From a490e3af0c463480f095d3444627c67c9773c3b7 Mon Sep 17 00:00:00 2001 From: m2049r Date: Sun, 16 Jun 2019 21:22:56 +0200 Subject: [PATCH] better status & error message handling (#599) --- app/src/main/cpp/monerujo.cpp | 24 +++++++- .../xmrwallet/GenerateReviewFragment.java | 21 +++---- .../com/m2049r/xmrwallet/LoginActivity.java | 42 +++++--------- .../com/m2049r/xmrwallet/WalletActivity.java | 19 +++---- .../com/m2049r/xmrwallet/model/Wallet.java | 56 +++++++++++++++++-- .../m2049r/xmrwallet/model/WalletManager.java | 2 +- .../xmrwallet/service/WalletService.java | 35 ++++++------ 7 files changed, 126 insertions(+), 73 deletions(-) diff --git a/app/src/main/cpp/monerujo.cpp b/app/src/main/cpp/monerujo.cpp index 738c748..ad369d3 100644 --- a/app/src/main/cpp/monerujo.cpp +++ b/app/src/main/cpp/monerujo.cpp @@ -39,6 +39,7 @@ static jclass class_WalletListener; static jclass class_TransactionInfo; static jclass class_Transfer; static jclass class_Ledger; +static jclass class_WalletStatus; std::mutex _listenerMutex; @@ -61,6 +62,8 @@ JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM *jvm, void *reserved) { jenv->FindClass("com/m2049r/xmrwallet/model/WalletListener"))); class_Ledger = static_cast(jenv->NewGlobalRef( jenv->FindClass("com/m2049r/xmrwallet/ledger/Ledger"))); + class_WalletStatus = static_cast(jenv->NewGlobalRef( + jenv->FindClass("com/m2049r/xmrwallet/model/Wallet$Status"))); return JNI_VERSION_1_6; } #ifdef __cplusplus @@ -581,10 +584,25 @@ Java_com_m2049r_xmrwallet_model_Wallet_getStatusJ(JNIEnv *env, jobject instance) return wallet->status(); } -JNIEXPORT jstring JNICALL -Java_com_m2049r_xmrwallet_model_Wallet_getErrorString(JNIEnv *env, jobject instance) { +jobject newWalletStatusInstance(JNIEnv *env, int status, const std::string &errorString) { + jmethodID init = env->GetMethodID(class_WalletStatus, "", + "(ILjava/lang/String;)V"); + jstring _errorString = env->NewStringUTF(errorString.c_str()); + jobject instance = env->NewObject(class_WalletStatus, init, status, _errorString); + env->DeleteLocalRef(_errorString); + return instance; +} + + +JNIEXPORT jobject JNICALL +Java_com_m2049r_xmrwallet_model_Wallet_statusWithErrorString(JNIEnv *env, jobject instance) { Bitmonero::Wallet *wallet = getHandle(env, instance); - return env->NewStringUTF(wallet->errorString().c_str()); + + int status; + std::string errorString; + wallet->statusWithErrorString(status, errorString); + + return newWalletStatusInstance(env, status, errorString); } JNIEXPORT jboolean JNICALL diff --git a/app/src/main/java/com/m2049r/xmrwallet/GenerateReviewFragment.java b/app/src/main/java/com/m2049r/xmrwallet/GenerateReviewFragment.java index cc6d908..e5c7403 100644 --- a/app/src/main/java/com/m2049r/xmrwallet/GenerateReviewFragment.java +++ b/app/src/main/java/com/m2049r/xmrwallet/GenerateReviewFragment.java @@ -192,7 +192,7 @@ public class GenerateReviewFragment extends Fragment { String viewKey; String spendKey; boolean isWatchOnly; - Wallet.Status status; + Wallet.Status walletStatus; boolean dialogOpened = false; @@ -224,9 +224,9 @@ public class GenerateReviewFragment extends Fragment { closeWallet = true; } name = wallet.getName(); - status = wallet.getStatus(); - if (status != Wallet.Status.Status_Ok) { - Timber.e(wallet.getErrorString()); + walletStatus = wallet.getStatus(); + if (!walletStatus.isOk()) { + Timber.e(walletStatus.getErrorString()); if (closeWallet) wallet.close(); return false; } @@ -287,10 +287,10 @@ public class GenerateReviewFragment extends Fragment { GenerateReviewFragment.VIEW_TYPE_ACCEPT.equals(type) ? Toolbar.BUTTON_NONE : Toolbar.BUTTON_BACK); } else { // TODO show proper error message and/or end the fragment? - tvWalletAddress.setText(status.toString()); - tvWalletMnemonic.setText(status.toString()); - tvWalletViewKey.setText(status.toString()); - tvWalletSpendKey.setText(status.toString()); + tvWalletAddress.setText(walletStatus.toString()); + tvWalletMnemonic.setText(walletStatus.toString()); + tvWalletViewKey.setText(walletStatus.toString()); + tvWalletSpendKey.setText(walletStatus.toString()); } hideProgress(); } @@ -414,12 +414,13 @@ public class GenerateReviewFragment extends Fragment { } boolean ok = false; - if (wallet.getStatus() == Wallet.Status.Status_Ok) { + Wallet.Status walletStatus = wallet.getStatus(); + if (walletStatus.isOk()) { wallet.setPassword(newPassword); wallet.store(); ok = true; } else { - Timber.e(wallet.getErrorString()); + Timber.e(walletStatus.getErrorString()); } if (closeWallet) wallet.close(); return ok; diff --git a/app/src/main/java/com/m2049r/xmrwallet/LoginActivity.java b/app/src/main/java/com/m2049r/xmrwallet/LoginActivity.java index 159b7e8..5980fb5 100644 --- a/app/src/main/java/com/m2049r/xmrwallet/LoginActivity.java +++ b/app/src/main/java/com/m2049r/xmrwallet/LoginActivity.java @@ -918,6 +918,16 @@ public class LoginActivity extends BaseActivity } + boolean checkAndCloseWallet(Wallet aWallet) { + Wallet.Status walletStatus = aWallet.getStatus(); + if (!walletStatus.isOk()) { + Timber.e(walletStatus.getErrorString()); + toast(walletStatus.getErrorString()); + } + aWallet.close(); + return walletStatus.isOk(); + } + @Override public void onGenerate(final String name, final String password) { createWallet(name, password, @@ -934,13 +944,7 @@ public class LoginActivity extends BaseActivity (currentNode != null) ? currentNode.getHeight() - 20 : -1; Wallet newWallet = WalletManager.getInstance() .createWallet(aFile, password, MNEMONIC_LANGUAGE, restoreHeight); - boolean success = (newWallet.getStatus() == Wallet.Status.Status_Ok); - if (!success) { - Timber.e(newWallet.getErrorString()); - toast(newWallet.getErrorString()); - } - newWallet.close(); - return success; + return checkAndCloseWallet(newWallet); } }); } @@ -959,13 +963,7 @@ public class LoginActivity extends BaseActivity public boolean createWallet(File aFile, String password) { Wallet newWallet = WalletManager.getInstance() .recoveryWallet(aFile, password, seed, restoreHeight); - boolean success = (newWallet.getStatus() == Wallet.Status.Status_Ok); - if (!success) { - Timber.e(newWallet.getErrorString()); - toast(newWallet.getErrorString()); - } - newWallet.close(); - return success; + return checkAndCloseWallet(newWallet); } }); } @@ -985,13 +983,7 @@ public class LoginActivity extends BaseActivity Wallet newWallet = WalletManager.getInstance() .createWalletFromDevice(aFile, password, restoreHeight, "Ledger"); - boolean success = (newWallet.getStatus() == Wallet.Status.Status_Ok); - if (!success) { - Timber.e(newWallet.getErrorString()); - toast(newWallet.getErrorString()); - } - newWallet.close(); - return success; + return checkAndCloseWallet(newWallet); } }); } @@ -1012,13 +1004,7 @@ public class LoginActivity extends BaseActivity Wallet newWallet = WalletManager.getInstance() .createWalletWithKeys(aFile, password, MNEMONIC_LANGUAGE, restoreHeight, address, viewKey, spendKey); - boolean success = (newWallet.getStatus() == Wallet.Status.Status_Ok); - if (!success) { - Timber.e(newWallet.getErrorString()); - toast(newWallet.getErrorString()); - } - newWallet.close(); - return success; + return checkAndCloseWallet(newWallet); } }); } diff --git a/app/src/main/java/com/m2049r/xmrwallet/WalletActivity.java b/app/src/main/java/com/m2049r/xmrwallet/WalletActivity.java index 8488a86..b8e3429 100644 --- a/app/src/main/java/com/m2049r/xmrwallet/WalletActivity.java +++ b/app/src/main/java/com/m2049r/xmrwallet/WalletActivity.java @@ -628,23 +628,22 @@ public class WalletActivity extends BaseActivity implements WalletFragment.Liste } @Override - public void onWalletStarted(final Wallet.ConnectionStatus connStatus) { + public void onWalletStarted(final Wallet.Status walletStatus) { runOnUiThread(new Runnable() { public void run() { dismissProgressDialog(); - switch (connStatus) { - case ConnectionStatus_Disconnected: - Toast.makeText(WalletActivity.this, getString(R.string.status_wallet_connect_failed), Toast.LENGTH_LONG).show(); - break; - case ConnectionStatus_WrongVersion: + if (walletStatus == null) { + // guess what went wrong + Toast.makeText(WalletActivity.this, getString(R.string.status_wallet_connect_failed), Toast.LENGTH_LONG).show(); + } else { + if (Wallet.ConnectionStatus.ConnectionStatus_WrongVersion == walletStatus.getConnectionStatus()) Toast.makeText(WalletActivity.this, getString(R.string.status_wallet_connect_wrongversion), Toast.LENGTH_LONG).show(); - break; - case ConnectionStatus_Connected: - break; + else if (!walletStatus.isOk()) + Toast.makeText(WalletActivity.this, walletStatus.getErrorString(), Toast.LENGTH_LONG).show(); } } }); - if (connStatus != Wallet.ConnectionStatus.ConnectionStatus_Connected) { + if ((walletStatus == null) || (Wallet.ConnectionStatus.ConnectionStatus_Connected != walletStatus.getConnectionStatus())) { finish(); } else { haveWallet = true; diff --git a/app/src/main/java/com/m2049r/xmrwallet/model/Wallet.java b/app/src/main/java/com/m2049r/xmrwallet/model/Wallet.java index 1d3ed97..7812605 100644 --- a/app/src/main/java/com/m2049r/xmrwallet/model/Wallet.java +++ b/app/src/main/java/com/m2049r/xmrwallet/model/Wallet.java @@ -16,6 +16,9 @@ package com.m2049r.xmrwallet.model; +import android.support.annotation.NonNull; +import android.support.annotation.Nullable; + import com.m2049r.xmrwallet.data.TxData; import java.io.File; @@ -32,6 +35,47 @@ public class Wallet { System.loadLibrary("monerujo"); } + static public class Status { + Status(int status, String errorString) { + this.status = StatusEnum.values()[status]; + this.errorString = errorString; + } + + final private StatusEnum status; + final private String errorString; + @Nullable + private ConnectionStatus connectionStatus; // optional + + public StatusEnum getStatus() { + return status; + } + + public String getErrorString() { + return errorString; + } + + public void setConnectionStatus(@Nullable ConnectionStatus connectionStatus) { + this.connectionStatus = connectionStatus; + } + + @Nullable + public ConnectionStatus getConnectionStatus() { + return connectionStatus; + } + + public boolean isOk() { + return (getStatus() == StatusEnum.Status_Ok) + && ((getConnectionStatus() == null) || + (getConnectionStatus() == ConnectionStatus.ConnectionStatus_Connected)); + } + + @Override + @NonNull + public String toString() { + return "Wallet.Status: (" + status + "/" + errorString + ", " + connectionStatus; + } + } + private int accountIndex = 0; public int getAccountIndex() { @@ -66,7 +110,7 @@ public class Wallet { Device_Ledger } - public enum Status { + public enum StatusEnum { Status_Ok, Status_Error, Status_Critical @@ -85,12 +129,16 @@ public class Wallet { public native void setSeedLanguage(String language); public Status getStatus() { - return Wallet.Status.values()[getStatusJ()]; + return statusWithErrorString(); } - private native int getStatusJ(); + public Status getFullStatus() { + Wallet.Status walletStatus = statusWithErrorString(); + walletStatus.setConnectionStatus(getConnectionStatus()); + return walletStatus; + } - public native String getErrorString(); + private native Status statusWithErrorString(); public native boolean setPassword(String password); diff --git a/app/src/main/java/com/m2049r/xmrwallet/model/WalletManager.java b/app/src/main/java/com/m2049r/xmrwallet/model/WalletManager.java index bc7db4e..cbd821f 100644 --- a/app/src/main/java/com/m2049r/xmrwallet/model/WalletManager.java +++ b/app/src/main/java/com/m2049r/xmrwallet/model/WalletManager.java @@ -95,7 +95,7 @@ public class WalletManager { long walletHandle = createWalletJ(aFile.getAbsolutePath(), password, language, getNetworkType().getValue()); Wallet wallet = new Wallet(walletHandle); manageWallet(wallet); - if (wallet.getStatus() == Wallet.Status.Status_Ok) { + if (wallet.getStatus().isOk()) { // (Re-)Estimate restore height based on what we know final long oldHeight = wallet.getRestoreHeight(); final long restoreHeight = diff --git a/app/src/main/java/com/m2049r/xmrwallet/service/WalletService.java b/app/src/main/java/com/m2049r/xmrwallet/service/WalletService.java index b9ea930..1e66dc4 100644 --- a/app/src/main/java/com/m2049r/xmrwallet/service/WalletService.java +++ b/app/src/main/java/com/m2049r/xmrwallet/service/WalletService.java @@ -31,6 +31,7 @@ import android.os.IBinder; import android.os.Looper; import android.os.Message; import android.os.Process; +import android.support.annotation.Nullable; import android.support.annotation.RequiresApi; import android.support.v4.app.NotificationCompat; @@ -220,7 +221,7 @@ public class WalletService extends Service { void onSendTransactionFailed(String error); - void onWalletStarted(Wallet.ConnectionStatus walletStatus); + void onWalletStarted(Wallet.Status walletStatus); void onWalletOpen(Wallet.Device device); } @@ -287,9 +288,9 @@ public class WalletService extends Service { if (walletId != null) { showProgress(getString(R.string.status_wallet_loading)); showProgress(10); - Wallet.ConnectionStatus connStatus = start(walletId, walletPw); - if (observer != null) observer.onWalletStarted(connStatus); - if (connStatus != Wallet.ConnectionStatus.ConnectionStatus_Connected) { + Wallet.Status walletStatus = start(walletId, walletPw); + if (observer != null) observer.onWalletStarted(walletStatus); + if (!walletStatus.isOk()) { errorState = true; stop(); } @@ -300,7 +301,7 @@ public class WalletService extends Service { boolean rc = myWallet.store(); Timber.d("wallet stored: %s with rc=%b", myWallet.getName(), rc); if (!rc) { - Timber.w("Wallet store failed: %s", myWallet.getErrorString()); + Timber.w("Wallet store failed: %s", myWallet.getStatus().getErrorString()); } if (observer != null) observer.onWalletStored(rc); } else if (cmd.equals(REQUEST_CMD_TX)) { @@ -362,7 +363,7 @@ public class WalletService extends Service { boolean rc = myWallet.store(); Timber.d("wallet stored: %s with rc=%b", myWallet.getName(), rc); if (!rc) { - Timber.w("Wallet store failed: %s", myWallet.getErrorString()); + Timber.w("Wallet store failed: %s", myWallet.getStatus().getErrorString()); } if (observer != null) observer.onWalletStored(rc); listener.updated = true; @@ -464,7 +465,8 @@ public class WalletService extends Service { return true; // true is important so that onUnbind is also called next time } - private Wallet.ConnectionStatus start(String walletName, String walletPassword) { + @Nullable + private Wallet.Status start(String walletName, String walletPassword) { Timber.d("start()"); startNotfication(); showProgress(getString(R.string.status_wallet_loading)); @@ -472,11 +474,11 @@ public class WalletService extends Service { if (listener == null) { Timber.d("start() loadWallet"); Wallet aWallet = loadWallet(walletName, walletPassword); - Wallet.ConnectionStatus connStatus = Wallet.ConnectionStatus.ConnectionStatus_Disconnected; - if (aWallet != null) connStatus = aWallet.getConnectionStatus(); - if (connStatus != Wallet.ConnectionStatus.ConnectionStatus_Connected) { - if (aWallet != null) aWallet.close(); - return connStatus; + if (aWallet == null) return null; + Wallet.Status walletStatus = aWallet.getFullStatus(); + if (!walletStatus.isOk()) { + aWallet.close(); + return walletStatus; } listener = new MyWalletListener(); listener.start(); @@ -487,7 +489,7 @@ public class WalletService extends Service { // if we try to refresh the history here we get occasional segfaults! // doesnt matter since we update as soon as we get a new block anyway Timber.d("start() done"); - return Wallet.ConnectionStatus.ConnectionStatus_Connected; + return getWallet().getFullStatus(); } public void stop() { @@ -532,10 +534,9 @@ public class WalletService extends Service { wallet = walletMgr.openWallet(path, walletPassword); showProgress(60); Timber.d("wallet opened"); - Wallet.Status status = wallet.getStatus(); - Timber.d("wallet status is %s", status); - if (status != Wallet.Status.Status_Ok) { - Timber.d("wallet status is %s", status); + Wallet.Status walletStatus = wallet.getStatus(); + if (!walletStatus.isOk()) { + Timber.d("wallet status is %s", walletStatus); WalletManager.getInstance().close(wallet); // TODO close() failed? wallet = null; // TODO what do we do with the progress??