fix: align extension storage 'set' and 'observeAll' with pouchdb stor…

…age behavior (#1597)

* fix: align extension storage 'set' and 'observeAll' with pouchdb storage behavior

deleting a wallet does:
1. delete it from repository
2. check remaining wallets, activate if some wallet exists

there was a bug where step 2. would find the wallet that was
just deleted and try to activate it, because extension storage
'observeAll' emits an slightly later

* fix: apply fromSerializableObject to objects emitted from storageChange$

set()/get() applies toSerializableObject/fromSerializableObject respectively
storageChange$ did not, resulting in emitting slightly different objects
that were not processed by fromSerializableObject util

this broke disabling accounts that were just enabled
and possibly some other functionality

* fix: activate any wallet when none is active

in order to recover from wallet activation bugs
for example, a crash after wallet deactivation but before
activating another wallet

fix: do not explicitly deactivate wallet when switching to another

explicitly deactivating will trigger a recovery mechanism
that will activate any wallet

apps/browser-extension-wallet/src/hooks/useWalletManager.ts

@@ -254,6 +254,7 @@ const connectHardwareWalletRevamped = async (usbDevice: USBDevice): Promise<Wall
 // eslint-disable-next-line max-statements
 export const useWalletManager = (): UseWalletManager => {
   const {
+    deletingWallet,
     walletLock,
     setWalletLock,
     cardanoWallet,
@@ -406,6 +407,42 @@ export const useWalletManager = (): UseWalletManager => {
     return firstValueFrom(walletRepository.wallets$);
   }, [resetWalletLock, backgroundService, setCardanoWallet, setWalletLock, getCurrentChainId]);
 
+  const activateWallet = useCallback(
+    async (props: ActivateWalletProps): Promise<void> => {
+      const [wallets, activeWallet] = await firstValueFrom(
+        combineLatest([walletRepository.wallets$, walletManager.activeWalletId$])
+      );
+      if (activeWallet?.walletId === props.walletId && activeWallet?.accountIndex === props.accountIndex) {
+        logger.debug('Wallet is already active');
+        return;
+      }
+      const updateWalletMetadataProps = {
+        walletId: props.walletId,
+        metadata: {
+          ...wallets.find(({ walletId }) => walletId === props.walletId).metadata,
+          lastActiveAccountIndex: props.accountIndex
+        }
+      };
+      await walletRepository.updateWalletMetadata(updateWalletMetadataProps);
+      await walletManager.activate({
+        ...props,
+        chainId: getCurrentChainId()
+      });
+    },
+    [getCurrentChainId]
+  );
+
+  const activateAnyWallet = useCallback(
+    async (wallets: AnyWallet<Wallet.WalletMetadata, Wallet.AccountMetadata>[]) => {
+      const anyWallet: ActivateWalletProps = {
+        walletId: wallets[0].walletId,
+        accountIndex: wallets[0].type === WalletType.Script ? undefined : wallets[0].accounts[0]?.accountIndex
+      };
+      await activateWallet(anyWallet);
+    },
+    [activateWallet]
+  );
+
   /**
    * Loads wallet from storage.
    * @returns resolves with wallet information or null when no wallet is found
@@ -427,6 +464,9 @@ export const useWalletManager = (): UseWalletManager => {
       // If there is no active wallet, activate the 1st one
       if (!activeWalletProps) {
         // deleting a wallet calls deactivateWallet(): do nothing, wallet will also be deleted from repository
+        if (!deletingWallet) {
+          await activateAnyWallet(wallets);
+        }
         return;
       }
 
@@ -484,6 +524,8 @@ export const useWalletManager = (): UseWalletManager => {
       cardanoWallet,
       currentChain,
       manageAccountsWallet,
+      activateAnyWallet,
+      deletingWallet,
       setCardanoCoin,
       setManageAccountsWallet
     ]
@@ -612,31 +654,6 @@ export const useWalletManager = (): UseWalletManager => {
     return createWallet({ name, passphrase, metadata, encryptedSecrets, extendedAccountPublicKey });
   };
 
-  const activateWallet = useCallback(
-    async (props: ActivateWalletProps): Promise<void> => {
-      const [wallets, activeWallet] = await firstValueFrom(
-        combineLatest([walletRepository.wallets$, walletManager.activeWalletId$])
-      );
-      if (activeWallet?.walletId === props.walletId && activeWallet?.accountIndex === props.accountIndex) {
-        logger.debug('Wallet is already active');
-        return;
-      }
-      await walletManager.deactivate();
-      await walletRepository.updateWalletMetadata({
-        walletId: props.walletId,
-        metadata: {
-          ...wallets.find(({ walletId }) => walletId === props.walletId).metadata,
-          lastActiveAccountIndex: props.accountIndex
-        }
-      });
-      await walletManager.activate({
-        ...props,
-        chainId: getCurrentChainId()
-      });
-    },
-    [getCurrentChainId]
-  );
-
   /**
    * Deletes Wallet from memory, all info from browser storage and destroys all stores
    *

apps/browser-extension-wallet/src/lib/scripts/background/storage/extension-document-store.ts

@@ -1,17 +1,19 @@
 /* eslint-disable @typescript-eslint/ban-types */
 import { storage as sdkStorage } from '@cardano-sdk/wallet';
-import { EMPTY, filter, from, map, mergeMap, Observable, of, share } from 'rxjs';
+import { EMPTY, filter, from, map, mergeMap, Observable, of, share, firstValueFrom } from 'rxjs';
 import { Logger } from 'ts-log';
 import { contextLogger, fromSerializableObject, toSerializableObject } from '@cardano-sdk/util';
 import { ExtensionStore } from './extension-store';
+import isEqual from 'lodash/isEqual';
 
 export type DocumentChange<T> = {
   oldValue?: T;
   newValue?: T;
 };
 
-const undefinedIfEmpty = <T>(value: T | undefined): T | undefined => {
-  if (typeof value === 'object' && (value === null || Object.keys(value).length === 0)) return undefined;
+const undefinedIfEmptyObj = <T>(value: T | undefined): T | undefined => {
+  if (typeof value === 'object' && !Array.isArray(value) && (value === null || Object.keys(value).length === 0))
+    return undefined;
   // eslint-disable-next-line consistent-return
   return value;
 };
@@ -34,8 +36,8 @@ export class ExtensionDocumentStore<T extends {}> extends ExtensionStore impleme
       filter(({ key }) => key === docId),
       map(
         ({ change }): DocumentChange<T> => ({
-          oldValue: undefinedIfEmpty(change.oldValue),
-          newValue: undefinedIfEmpty(change.newValue)
+          oldValue: undefinedIfEmptyObj(change.oldValue),
+          newValue: undefinedIfEmptyObj(change.newValue)
         })
       ),
       share()
@@ -55,11 +57,25 @@ export class ExtensionDocumentStore<T extends {}> extends ExtensionStore impleme
   set(doc: T): Observable<void> {
     this.logger.debug('set', doc);
     return from(
-      (this.idle = this.idle.then(() =>
-        this.storage.set({
+      (this.idle = this.idle.then(async () => {
+        const previousValueMap = await this.storage.get(this.docId);
+        const previousValue = fromSerializableObject(previousValueMap[this.docId]);
+        if (isEqual(previousValue, doc)) {
+          // if values are equal, then we would set to the same value and
+          // this.documentChange$ won't emit and this promise will never resolve
+          return;
+        }
+
+        const storageChange = firstValueFrom(this.documentChange$);
+        await this.storage.set({
           [this.docId]: toSerializableObject(doc)
-        })
-      ))
+        });
+        // do not emit until documentChange$ emits
+        // in order to avoid race conditions:
+        // users expect `observeAll` to emit new value when subscribing
+        // to it immediatelly after `set` emits
+        await storageChange;
+      }))
     );
   }
 

apps/browser-extension-wallet/src/lib/scripts/background/storage/extension-store.ts

@@ -2,6 +2,7 @@
 import { storage as extensionStorage, Storage } from 'webextension-polyfill';
 import { fromEventPattern, mergeMap, Observable, share } from 'rxjs';
 import { Logger } from 'ts-log';
+import { fromSerializableObject } from '@cardano-sdk/util';
 
 export abstract class ExtensionStore {
   protected readonly storageChange$: Observable<{ key: string; change: Storage.StorageChange }>;
@@ -13,7 +14,9 @@ export abstract class ExtensionStore {
       (handler) => this.storage.onChanged.addListener(handler),
       (handler) => this.storage.onChanged.removeListener(handler)
     ).pipe(
-      mergeMap((changes) => Object.entries(changes).map(([key, change]) => ({ key, change }))),
+      mergeMap((changes) =>
+        Object.entries(changes).map(([key, change]) => ({ key, change: fromSerializableObject(change) }))
+      ),
       share()
     );
   }

packages/e2e-tests/src/fixture/walletRepositoryInitializer.ts

@@ -16,11 +16,12 @@ export const clearWalletRepository = async (): Promise<void> => {
   await switchToWindowWithLace(0);
   await browser.execute(`
     return (async () => {
-      await window.walletManager.deactivate();
       const wallets = await window.firstValueFrom(window.walletRepository.wallets$);
-      for (const wallet of wallets) {
+      // reversing in order to delete shared wallets before dependent wallets
+      for (const wallet of wallets.reverse()) {
         await window.walletRepository.removeWallet(wallet.walletId);
       }
+      await window.walletManager.deactivate();
       return JSON.stringify(wallets);
     })()
   `);
@@ -32,23 +33,41 @@ export const getWalletsFromRepository = async (): Promise<any[]> =>
       return wallets;
   `);
 
-export const addAndActivateWalletInRepository = async (wallet: string): Promise<number> =>
+const addWalletInRepository = async (wallet: string): Promise<void> => {
+  await browser.execute(
+    `
+      return (async () => {
+        let walletsObj = JSON.parse('${wallet}');
+        await walletRepository.addWallet(walletsObj[0]);
+      })()
+    `
+  );
+};
+
+export const addAndActivateWalletInRepository = async (wallet: string): Promise<void> =>
   await browser.execute(
     `
-      let walletsObj = JSON.parse('${wallet}');
-      await walletRepository.addWallet(walletsObj[0]);
-       await walletManager.activate({
-        walletId: walletsObj[0].walletId,
-        accountIndex: walletsObj[0].accounts[0].accountIndex,
-        chainId: { networkId: 0, networkMagic: 1 }
-      })
+      return (async () => {
+        let walletsObj = JSON.parse('${wallet}');
+        await walletRepository.addWallet(walletsObj[0]);
+        // wait for Lace to auto-activate the added wallet,
+        // which might use a different network than we want to set here
+        // this is still a race and we should rework wallet initialization
+        await new Promise(resolve => setTimeout(resolve, 1000));
+        await walletManager.activate({
+          walletId: walletsObj[0].walletId,
+          accountIndex: walletsObj[0].accounts[0].accountIndex,
+          chainId: { networkId: 0, networkMagic: 1 }
+        });
+      })()
   `
   );
 
 export const addAndActivateWalletsInRepository = async (wallets: TestWalletName[]): Promise<void> => {
   const walletsRepositoryArray = wallets.map((wallet) => getTestWallet(wallet).repository as string);
 
-  for (const wallet of walletsRepositoryArray.reverse()) {
-    await addAndActivateWalletInRepository(wallet);
+  for (const wallet of walletsRepositoryArray.slice(1).reverse()) {
+    await addWalletInRepository(wallet);
   }
+  await addAndActivateWalletInRepository(walletsRepositoryArray[0]);
 };