backend/keystore: detect new keystores after a wrong one

With the current implementation, when the wrong keystore is connected after
a keystore request, the app is not able to automatically detect the
connection of the correct keystore. The only option for the user is to
cancel the 'connect keystore' popup and repeat the action to create a new
keystore connection request. This is counterintuitive and inefficient.

This adds a `connectkeystore.retryCallback` that allows to wait for a new
keystore to be connected and to check if that is the expected one.

Author: Beerosagos

backend/accounts.go

@@ -660,7 +660,7 @@ func (backend *Backend) createAndAddAccount(coin coinpkg.Coin, persistedConfig *
 			type data struct {
 				Type         string `json:"typ"`
 				KeystoreName string `json:"keystoreName"`
-				ErrorCode    string `json:"errorCode"`
+				ErrorCode    string `json:"errorCode,omitempty"`
 				ErrorMessage string `json:"errorMessage"`
 			}
 			accountRootFingerprint, err := persistedConfig.SigningConfigurations.RootFingerprint()
@@ -672,45 +672,81 @@ func (backend *Backend) createAndAddAccount(coin coinpkg.Coin, persistedConfig *
 			if err == nil {
 				keystoreName = persistedKeystore.Name
 			}
-			backend.Notify(observable.Event{
-				Subject: "connect-keystore",
-				Action:  action.Replace,
-				Object: data{
-					Type:         "connect",
-					KeystoreName: keystoreName,
-				},
-			})
-			ks, err := backend.connectKeystore.connect(
-				backend.Keystore(),
-				accountRootFingerprint,
-				20*time.Minute,
-			)
+			var ks keystore.Keystore
+			timeout := 20 * time.Minute
+		outerLoop:
+			for {
+				backend.Notify(observable.Event{
+					Subject: "connect-keystore",
+					Action:  action.Replace,
+					Object: data{
+						Type:         "connect",
+						KeystoreName: keystoreName,
+					},
+				})
+				ks, err = backend.connectKeystore.connect(
+					backend.Keystore(),
+					accountRootFingerprint,
+					timeout,
+				)
+				if err == nil || errp.Cause(err) != ErrWrongKeystore {
+					break
+				} else {
+					backend.Notify(observable.Event{
+						Subject: "connect-keystore",
+						Action:  action.Replace,
+						Object: data{
+							Type:         "error",
+							ErrorCode:    "wrongKeystore",
+							ErrorMessage: err.Error(),
+						},
+					})
+					c := make(chan bool)
+					// retryCallback is called when the current keystore is deregistered or when
+					// CancelConnectKeystore() is called.
+					// In the first case it allows to make a new connection attempt, in the last one
+					// it'll make this function return ErrUserAbort.
+					backend.connectKeystore.SetRetryConnect(func(retry bool) {
+						c <- retry
+					})
+					select {
+					case retry := <-c:
+						if !retry {
+							err = ErrUserAbort
+							break outerLoop
+						}
+					case <-time.After(timeout):
+						backend.connectKeystore.SetRetryConnect(nil)
+						err = errTimeout
+						break outerLoop
+					}
+				}
+			}
 			switch {
 			case errp.Cause(err) == errReplaced:
 				// If a previous connect-keystore request is in progress, the previous request is
 				// failed, but we don't dismiss the prompt, as the new prompt has already been shown
-				// by the above "connect" notification.y
-			case err == nil || errp.Cause(err) == errUserAbort:
+				// by the above "connect" notification.
+			case err == nil || errp.Cause(err) == ErrUserAbort:
 				// Dismiss prompt after success or upon user abort.
-
 				backend.Notify(observable.Event{
 					Subject: "connect-keystore",
 					Action:  action.Replace,
 					Object:  nil,
 				})
 			default:
-				// Display error to user.
-				errorCode := ""
-				if errp.Cause(err) == ErrWrongKeystore {
-					errorCode = "wrongKeystore"
+				var errorCode = ""
+				if errp.Cause(err) == errTimeout {
+					errorCode = "timeout"
 				}
+				// Display error to user.
 				backend.Notify(observable.Event{
 					Subject: "connect-keystore",
 					Action:  action.Replace,
 					Object: data{
 						Type:         "error",
-						ErrorCode:    errorCode,
 						ErrorMessage: err.Error(),
+						ErrorCode:    errorCode,
 					},
 				})
 			}

backend/backend.go

@@ -687,6 +687,7 @@ func (backend *Backend) DeregisterKeystore() {
 	// keystore. For now we just remove all, then re-add the rest.
 	backend.initPersistedAccounts()
 	backend.emitAccountsStatusChanged()
+	backend.connectKeystore.onDisconnect()
 }
 
 // Register registers the given device at this backend.
@@ -890,7 +891,7 @@ func (backend *Backend) GetAccountFromCode(acctCode accountsTypes.Code) (account
 
 // CancelConnectKeystore cancels a pending keystore connection request if one exists.
 func (backend *Backend) CancelConnectKeystore() {
-	backend.connectKeystore.cancel(errUserAbort)
+	backend.connectKeystore.cancel(ErrUserAbort)
 }
 
 // SetWatchonly sets the keystore's watchonly flag to `watchonly`.

backend/connectkeystore.go

@@ -26,16 +26,24 @@ import (
 
 // ErrWrongKeystore is returned if the connected keystore is not the expected one.
 var ErrWrongKeystore = errors.New("Wrong device/keystore connected.")
-var errUserAbort = errors.New("aborted by user")
+
+// ErrUserAbort is raised when a keystore connection is aborted calling CancelConnectKeystore().
+var ErrUserAbort = errors.New("aborted by user")
 var errReplaced = errors.New("replaced by new prompt")
 
+var errTimeout = errors.New("timeout")
+
 // connectKeystore is a helper struct to enable connecting to a keystore with a specific root
 // fingerprint.
 type connectKeystore struct {
 	locker.Locker
 	// connectKeystoreCallback, if not nil, is called when a keystore is registered.
 	connectKeystoreCallback func(keystore.Keystore)
-	cancelFunc              context.CancelCauseFunc
+	// retryCallback, if not nil, is called when a keystore is deregistered or when the cancel() is called.
+	// It allows to make a new connect attempt after a wrong keystore was connected, unless the request has
+	// been aborted.
+	retryCallback func(retry bool)
+	cancelFunc    context.CancelCauseFunc
 }
 
 func compareRootFingerprint(ks keystore.Keystore, rootFingerprint []byte) error {
@@ -108,6 +116,9 @@ func (c *connectKeystore) connect(
 	case r := <-resultCh:
 		return r.ks, r.err
 	case <-ctx.Done():
+		if context.Cause(ctx) == context.DeadlineExceeded {
+			return nil, errTimeout
+		}
 		return nil, context.Cause(ctx)
 	}
 }
@@ -123,6 +134,14 @@ func (c *connectKeystore) onConnect(keystore keystore.Keystore) {
 	}
 }
 
+func (c *connectKeystore) onDisconnect() {
+	defer c.Lock()()
+	if c.retryCallback != nil {
+		c.retryCallback(true)
+		c.retryCallback = nil
+	}
+}
+
 // cancel fails a pending call to `connect()`, making it return `cause` as the error.
 func (c *connectKeystore) cancel(cause error) {
 	defer c.Lock()()
@@ -131,4 +150,13 @@ func (c *connectKeystore) cancel(cause error) {
 		c.cancelFunc = nil
 		c.connectKeystoreCallback = nil
 	}
+	if c.retryCallback != nil {
+		c.retryCallback(false)
+		c.retryCallback = nil
+	}
+}
+
+func (c *connectKeystore) SetRetryConnect(f func(retry bool)) {
+	defer c.Lock()()
+	c.retryCallback = f
 }

backend/connectkeystore_test.go

@@ -15,7 +15,6 @@
 package backend
 
 import (
-	"context"
 	"sync"
 	"testing"
 	"time"
@@ -38,7 +37,7 @@ func TestConnectKeystore(t *testing.T) {
 
 	t.Run("timeout", func(t *testing.T) {
 		_, err := ck.connect(nil, fingerprint, time.Millisecond)
-		require.Equal(t, context.DeadlineExceeded, err)
+		require.Equal(t, errTimeout, err)
 	})
 
 	t.Run("already connected", func(t *testing.T) {
@@ -53,10 +52,10 @@ func TestConnectKeystore(t *testing.T) {
 		go func() {
 			defer wg.Done()
 			time.Sleep(50 * time.Millisecond)
-			ck.cancel(errUserAbort)
+			ck.cancel(ErrUserAbort)
 		}()
 		_, err := ck.connect(nil, fingerprint, time.Second)
-		require.Equal(t, errUserAbort, err)
+		require.Equal(t, ErrUserAbort, err)
 		wg.Wait()
 	})
 

frontends/web/src/api/backend.ts

@@ -76,13 +76,15 @@ export const socksProxyCheck = (proxyAddress: string): Promise<ISuccess> => {
   return apiPost('socksproxy/check', proxyAddress);
 };
 
+export type TConnectKeystoreErrorCode = 'wrongKeystore' | 'timeout';
+
 export type TSyncConnectKeystore = null | {
   typ: 'connect';
   keystoreName: string;
 } | {
   typ: 'error';
-  errorCode: 'wrongKeystore';
-  errorMessage: '';
+  errorCode?: TConnectKeystoreErrorCode;
+  errorMessage: string;
 };
 
 /**

frontends/web/src/components/keystoreconnectprompt.tsx

@@ -15,8 +15,9 @@
  */
 
 import { useTranslation } from 'react-i18next';
+import { ReactElement } from 'react';
 import { Button } from './forms';
-import { cancelConnectKeystore, syncConnectKeystore } from '../api/backend';
+import { TConnectKeystoreErrorCode, cancelConnectKeystore, syncConnectKeystore } from '../api/backend';
 import { useSubscribeReset } from '../hooks/api';
 import { Dialog, DialogButtons } from './dialog/dialog';
 import { BitBox02StylizedDark, BitBox02StylizedLight, Cancel, PointToBitBox02 } from './icon';
@@ -29,9 +30,35 @@ export function KeystoreConnectPrompt() {
   const { isDarkMode } = useDarkmode();
 
   const [data, reset] = useSubscribeReset(syncConnectKeystore());
+
+  const cancelAndReset = () => {
+    // This is needed to close the popup in case of timeout exception.
+    reset();
+    cancelConnectKeystore();
+  };
+
+  const errorMessage = (errorCode: TConnectKeystoreErrorCode | undefined): ReactElement | null => {
+    switch (errorCode) {
+    case 'wrongKeystore':
+      return (<>
+        {t('error.wrongKeystore')}
+        <br />
+        <br />
+        {t('error.wrongKeystore2')}
+      </>);
+    case 'timeout':
+      return (<>
+        {t('error.keystoreTimeout')}
+      </>);
+    default:
+      return null;
+    }
+  };
+
   if (!data) {
     return null;
   }
+
   switch (data.typ) {
   case 'connect':
     return (
@@ -51,35 +78,31 @@ export function KeystoreConnectPrompt() {
           <SkipForTesting />
         </div>
         <DialogButtons>
-          <Button secondary onClick={() => cancelConnectKeystore()}>{t('dialog.cancel')}</Button>
+          <Button secondary onClick={cancelConnectKeystore}>{t('dialog.cancel')}</Button>
         </DialogButtons>
       </Dialog>
     );
   case 'error':
+    const err = errorMessage(data.errorCode);
     return (
       <Dialog title={t('welcome.connect')} medium open>
         <p className={styles.text}>
-          {data.errorCode === 'wrongKeystore' ?
-            <>
-              {t('error.wrongKeystore')}
-              <br />
-              <br />
-              {t('error.wrongKeystore2')}
-            </> : data.errorMessage}
+          { err ? err : data.errorMessage }
         </p>
         <div className={`${styles.bitboxContainer} ${styles.failed}`}>
           <Cancel className={styles.cancelIcon} />
           {isDarkMode ?
             <BitBox02StylizedLight className={styles.bitboxImage} /> :
             <BitBox02StylizedDark className={styles.bitboxImage} />
           }
+          <SkipForTesting />
         </div>
         <DialogButtons>
-          <Button secondary onClick={() => reset()}>{t('dialog.cancel')}</Button>
+          <Button secondary onClick={cancelAndReset}>{t('dialog.cancel')}</Button>
         </DialogButtons>
       </Dialog>
     );
   default:
     return null;
   }
-}
+}

frontends/web/src/locales/en/app.json

@@ -673,6 +673,7 @@
     "aoppUnsupportedFormat": "There are no available accounts that support the requested address format.",
     "aoppUnsupportedKeystore": "The connected device cannot sign messages for this asset.",
     "aoppVersion": "Unknown version.",
+    "keystoreTimeout": "Wallet request expired. Please try again.",
     "wrongKeystore": "Wrong wallet connected. Please make sure to insert the correct device matching this account.",
     "wrongKeystore2": " If you are using the optional passphrase, make sure you have entered the correct passphrase for the account."
   },