better status & error message handling (#599)

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<jclass>(jenv->NewGlobalRef(
             jenv->FindClass("com/m2049r/xmrwallet/ledger/Ledger")));
+    class_WalletStatus = static_cast<jclass>(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
+jobject newWalletStatusInstance(JNIEnv *env, int status, const std::string &errorString) {
-Java_com_m2049r_xmrwallet_model_Wallet_getErrorString(JNIEnv *env, jobject instance) {
+    jmethodID init = env->GetMethodID(class_WalletStatus, "<init>",
+                                      "(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<Bitmonero::Wallet>(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

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();
+            walletStatus = wallet.getStatus();
-            if (status != Wallet.Status.Status_Ok) {
+            if (!walletStatus.isOk()) {
-                Timber.e(wallet.getErrorString());
+                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());
+                tvWalletAddress.setText(walletStatus.toString());
-                tvWalletMnemonic.setText(status.toString());
+                tvWalletMnemonic.setText(walletStatus.toString());
-                tvWalletViewKey.setText(status.toString());
+                tvWalletViewKey.setText(walletStatus.toString());
-                tvWalletSpendKey.setText(status.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;

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);
+                        return checkAndCloseWallet(newWallet);
-                        if (!success) {
-                            Timber.e(newWallet.getErrorString());
-                            toast(newWallet.getErrorString());
-                        }
-                        newWallet.close();
-                        return success;
                     }
                 });
     }
@@ -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);
+                        return checkAndCloseWallet(newWallet);
-                        if (!success) {
-                            Timber.e(newWallet.getErrorString());
-                            toast(newWallet.getErrorString());
-                        }
-                        newWallet.close();
-                        return success;
                     }
                 });
     }
@@ -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);
+                        return checkAndCloseWallet(newWallet);
-                        if (!success) {
-                            Timber.e(newWallet.getErrorString());
-                            toast(newWallet.getErrorString());
-                        }
-                        newWallet.close();
-                        return success;
                     }
                 });
     }
@@ -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);
+                        return checkAndCloseWallet(newWallet);
-                        if (!success) {
-                            Timber.e(newWallet.getErrorString());
-                            toast(newWallet.getErrorString());
-                        }
-                        newWallet.close();
-                        return success;
                     }
                 });
     }

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) {
+                if (walletStatus == null) {
-                    case ConnectionStatus_Disconnected:
+                    // guess what went wrong
                     Toast.makeText(WalletActivity.this, getString(R.string.status_wallet_connect_failed), Toast.LENGTH_LONG).show();
-                        break;
+                } else {
-                    case ConnectionStatus_WrongVersion:
+                    if (Wallet.ConnectionStatus.ConnectionStatus_WrongVersion == walletStatus.getConnectionStatus())
                         Toast.makeText(WalletActivity.this, getString(R.string.status_wallet_connect_wrongversion), Toast.LENGTH_LONG).show();
-                        break;
+                    else if (!walletStatus.isOk())
-                    case ConnectionStatus_Connected:
+                        Toast.makeText(WalletActivity.this, walletStatus.getErrorString(), Toast.LENGTH_LONG).show();
-                        break;
                 }
             }
         });
-        if (connStatus != Wallet.ConnectionStatus.ConnectionStatus_Connected) {
+        if ((walletStatus == null) || (Wallet.ConnectionStatus.ConnectionStatus_Connected != walletStatus.getConnectionStatus())) {
             finish();
         } else {
             haveWallet = true;

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);
 

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 =

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);
+                            Wallet.Status walletStatus = start(walletId, walletPw);
-                            if (observer != null) observer.onWalletStarted(connStatus);
+                            if (observer != null) observer.onWalletStarted(walletStatus);
-                            if (connStatus != Wallet.ConnectionStatus.ConnectionStatus_Connected) {
+                            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) return null;
-            if (aWallet != null) connStatus = aWallet.getConnectionStatus();
+            Wallet.Status walletStatus = aWallet.getFullStatus();
-            if (connStatus != Wallet.ConnectionStatus.ConnectionStatus_Connected) {
+            if (!walletStatus.isOk()) {
-                if (aWallet != null) aWallet.close();
+                aWallet.close();
-                return connStatus;
+                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();
+            Wallet.Status walletStatus = wallet.getStatus();
-            Timber.d("wallet status is %s", status);
+            if (!walletStatus.isOk()) {
-            if (status != Wallet.Status.Status_Ok) {
+                Timber.d("wallet status is %s", walletStatus);
-                Timber.d("wallet status is %s", status);
                 WalletManager.getInstance().close(wallet); // TODO close() failed?
                 wallet = null;
                 // TODO what do we do with the progress??