Merge branch 'backend-keystore-retry'

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:    err.Error(),
+							ErrorMessage: "",
+						},
+					})
+					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 = errp.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) == errp.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 = err.Error()
 				}
+				// 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(errp.ErrUserAbort)
 }
 
 // SetWatchonly sets the keystore's watchonly flag to `watchonly`.

backend/coins/btc/handlers/handlers.go

@@ -51,11 +51,6 @@ type Handlers struct {
 	log     *logrus.Entry
 }
 
-const (
-	// ErrUserAbort is returned if the user aborted the current operation.
-	errUserAbort errp.ErrorCode = "userAbort"
-)
-
 // NewHandlers creates a new Handlers instance.
 func NewHandlers(
 	handleFunc func(string, func(*http.Request) (interface{}, error)) *mux.Route, log *logrus.Entry) *Handlers {
@@ -739,10 +734,10 @@ func (handlers *Handlers) postSignBTCAddress(r *http.Request) (interface{}, erro
 		request.Format)
 	if err != nil {
 		if firmware.IsErrorAbort(err) {
-			return response{Success: false, ErrorCode: string(errUserAbort)}, nil
+			return response{Success: false, ErrorCode: errp.ErrUserAbort.Error()}, nil
 		}
 		if errp.Cause(err) == backend.ErrWrongKeystore {
-			return response{Success: false, ErrorCode: string("wrongKeystore")}, nil
+			return response{Success: false, ErrorCode: backend.ErrWrongKeystore.Error()}, nil
 		}
 
 		handlers.log.WithField("code", account.Config().Config.Code).Error(err)

backend/connectkeystore.go

@@ -17,25 +17,32 @@ package backend
 import (
 	"bytes"
 	"context"
-	"errors"
 	"time"
 
 	"github.com/digitalbitbox/bitbox-wallet-app/backend/keystore"
+	"github.com/digitalbitbox/bitbox-wallet-app/util/errp"
 	"github.com/digitalbitbox/bitbox-wallet-app/util/locker"
 )
 
-// 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")
-var errReplaced = errors.New("replaced by new prompt")
+const (
+	// ErrWrongKeystore is returned if the connected keystore is not the expected one.
+	ErrWrongKeystore errp.ErrorCode = "wrongKeystore"
+
+	errTimeout  errp.ErrorCode = "timeout"
+	errReplaced errp.ErrorCode = "connectReplaced"
+)
 
 // 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 +115,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 +133,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 +149,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,12 +15,12 @@
 package backend
 
 import (
-	"context"
 	"sync"
 	"testing"
 	"time"
 
 	keystoremock "github.com/digitalbitbox/bitbox-wallet-app/backend/keystore/mocks"
+	"github.com/digitalbitbox/bitbox-wallet-app/util/errp"
 
 	"github.com/stretchr/testify/require"
 )
@@ -38,7 +38,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 +53,10 @@ func TestConnectKeystore(t *testing.T) {
 		go func() {
 			defer wg.Done()
 			time.Sleep(50 * time.Millisecond)
-			ck.cancel(errUserAbort)
+			ck.cancel(errp.ErrUserAbort)
 		}()
 		_, err := ck.connect(nil, fingerprint, time.Second)
-		require.Equal(t, errUserAbort, err)
+		require.Equal(t, errp.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."
   },

util/errp/errp.go

@@ -65,6 +65,6 @@ func (e ErrorCode) Error() string {
 // The follwing error codes are defined here because they are shared between packages.
 // Package specific error codes should be defined inside the package itself.
 const (
-// ErrUserAbort is returned if the user aborted the current operation.
-// This is just an example: ErrUserAbort ErrorCode = "userAbort"
+	// ErrUserAbort is returned if the user aborted the current operation.
+	ErrUserAbort ErrorCode = "userAbort"
 )