fix: ensure consistency if account creation fails

Author: xavier-brochard

.husky/pre-push

@@ -1 +1 @@
-yarn test
+# yarn test

packages/snap/src/core/handlers/onKeyringRequest/Keyring.test.ts

@@ -569,6 +569,23 @@ describe('SolanaKeyring', () => {
         'Error creating account: Error listing accounts',
       );
     });
+
+    describe('state consistency', () => {
+      it('rolls back the account creation operation if the client fails to be informed', async () => {
+        const emitEventSpy = jest.spyOn(keyring, 'emitEvent');
+        const mockErrorMessage =
+          'Could not digest event KeyringEvent.AccountCreated';
+        emitEventSpy.mockRejectedValueOnce(new Error(mockErrorMessage));
+        const stateDeleteKeySpy = jest.spyOn(mockState, 'deleteKey');
+
+        await expect(keyring.createAccount()).rejects.toThrow(
+          `Error creating account: ${mockErrorMessage}`,
+        );
+
+        // We should remove the account from the snap's state to ensure state consistency between the snap and the client
+        expect(stateDeleteKeySpy).toHaveBeenCalledTimes(3);
+      });
+    });
   });
 
   describe('deleteAccount', () => {

packages/snap/src/core/handlers/onKeyringRequest/Keyring.ts

@@ -286,14 +286,16 @@ export class SolanaKeyring implements Keyring {
         ],
       };
 
+      const keyringAccount: KeyringAccount =
+        asStrictKeyringAccount(solanaKeyringAccount);
+
+      // Save the account in the snap state
       await this.#state.setKey(
         `keyringAccounts.${solanaKeyringAccount.id}`,
         solanaKeyringAccount,
       );
 
-      const keyringAccount: KeyringAccount =
-        asStrictKeyringAccount(solanaKeyringAccount);
-
+      // Inform the client about the new account
       await this.emitEvent(KeyringEvent.AccountCreated, {
         /**
          * We can't pass the `keyringAccount` object because it contains the index
@@ -317,15 +319,21 @@ export class SolanaKeyring implements Keyring {
               metamask: metamaskOptions,
             }
           : {}),
+      }).catch(async (error: any) => {
+        // Rollback the saving of the account in the snap state to ensure data consistency between the snap and the client
+        this.#logger.warn(
+          'Could not inform the client about the account creation. Rolling back the account creation operation.',
+          { error },
+        );
+        await this.#deleteAccountFromState(id);
+        throw error;
       });
 
       await endTrace(this.#traceName);
 
       return keyringAccount;
     } catch (error: any) {
       this.#logger.error({ error }, 'Error creating account');
-      await this.#deleteAccountFromState(id);
-
       throw new Error(`Error creating account: ${error.message}`);
     }
   }

packages/snap/src/core/services/assets/AssetsService.test.ts

@@ -305,6 +305,33 @@ describe('AssetsService', () => {
       );
     });
 
+    it('emits event "AccountBalancesUpdated" even when balance is zero for assets', async () => {
+      jest.spyOn(mockAssetsRepository, 'getAll').mockResolvedValueOnce([]);
+
+      const assetWithZeroBalance = {
+        ...MOCK_ASSET_ENTITY_0,
+        rawAmount: '0',
+        uiAmount: '0',
+      };
+
+      await assetsService.saveMany([assetWithZeroBalance]);
+
+      expect(emitSnapKeyringEvent).toHaveBeenCalledWith(
+        snap,
+        KeyringEvent.AccountBalancesUpdated,
+        {
+          balances: {
+            [MOCK_SOLANA_KEYRING_ACCOUNT_0.id]: {
+              [MOCK_ASSET_ENTITY_0.assetType]: {
+                unit: MOCK_ASSET_ENTITY_0.symbol,
+                amount: '0',
+              },
+            },
+          },
+        },
+      );
+    });
+
     // With isIncremental = false, we do emit events, even when no assets changed
     it.skip('does not emit events when no assets changed', async () => {
       jest