chore: Improve Keyring/Accounts error handling and logs

app/core/Engine/Engine.ts

@@ -213,6 +213,7 @@
   getGlobalChainId,
   getGlobalNetworkClientId,
 } from '../../util/networks/global-network';
+import { logEngineCreation } from './utils/logger';
 
 const NON_EMPTY = 'NON_EMPTY';
 
@@ -276,6 +277,8 @@
     initialState: Partial<EngineState> = {},
     initialKeyringState?: KeyringControllerState | null,
   ) {
+    logEngineCreation(initialState, initialKeyringState);
+
     this.controllerMessenger = new ExtendedControllerMessenger();
 
     const isBasicFunctionalityToggleEnabled = () =>
@@ -971,8 +974,8 @@
         disableSnaps: !isBasicFunctionalityToggleEnabled(),
       }),
       clientCryptography: {
-        pbkdf2Sha512: pbkdf2
-      }
+        pbkdf2Sha512: pbkdf2,
+      },
     });
 
     const authenticationController = new AuthenticationController.Controller({
@@ -1173,7 +1176,7 @@
 
           return Boolean(
             hasProperty(showIncomingTransactions, currentChainId) &&
-            showIncomingTransactions?.[currentHexChainId],
+              showIncomingTransactions?.[currentHexChainId],
           );
         },
         updateTransactions: true,
@@ -1305,7 +1308,9 @@
               .addProperties({
                 token_standard: 'ERC20',
                 asset_type: 'token',
-                chain_id: getDecimalChainId(getGlobalChainId(networkController)),
+                chain_id: getDecimalChainId(
+                  getGlobalChainId(networkController),
+                ),
               })
               .build(),
           ),
@@ -1517,7 +1522,7 @@
         if (
           state.networksMetadata[state.selectedNetworkClientId].status ===
             NetworkStatus.Available &&
-            getGlobalChainId(networkController) !== currentChainId
+          getGlobalChainId(networkController) !== currentChainId
         ) {
           // We should add a state or event emitter saying the provider changed
           setTimeout(() => {
@@ -1537,7 +1542,9 @@
         } catch (error) {
           console.error(
             error,
-            `Network ID not changed, current chainId: ${getGlobalChainId(networkController)}`,
+            `Network ID not changed, current chainId: ${getGlobalChainId(
+              networkController,
+            )}`,
           );
         }
       },
@@ -1775,7 +1782,7 @@
 
         const tokenBalances =
           allTokenBalances?.[selectedInternalAccount.address as Hex]?.[
-          chainId
+            chainId
           ] ?? {};
         tokens.forEach(
           (item: { address: string; balance?: string; decimals: number }) => {
@@ -1786,9 +1793,9 @@
               item.balance ||
               (item.address in tokenBalances
                 ? renderFromTokenMinimalUnit(
-                  tokenBalances[item.address as Hex],
-                  item.decimals,
-                )
+                    tokenBalances[item.address as Hex],
+                    item.decimals,
+                  )
                 : undefined);
             const tokenBalanceFiat = balanceToFiatNumber(
               // TODO: Fix this by handling or eliminating the undefined case
@@ -1871,12 +1878,12 @@
       const { tokenBalances } = backgroundState.TokenBalancesController;
 
       let tokenFound = false;
       tokenLoop: for (const chains of Object.values(tokenBalances)) {
         for (const tokens of Object.values(chains)) {
           for (const balance of Object.values(tokens)) {
             if (!isZero(balance)) {
               tokenFound = true;
               break tokenLoop;
             }
           }
         }
@@ -2148,12 +2155,15 @@
     return instance.resetState();
   },
 
-  destroyEngine() {
-    instance?.destroyEngineInstance();
+  destroyEngine: async () => {
+    await instance?.destroyEngineInstance();
     instance = null;
   },
 
-  init(state: Partial<EngineState> | undefined, keyringState = null) {
+  init(
+    state: Partial<EngineState> | undefined,
+    keyringState: KeyringControllerState | null = null,
+  ) {
     instance = Engine.instance || new Engine(state, keyringState);
     Object.freeze(instance);
     return instance;

app/core/Engine/controllers/AccountsController/logger.test.ts

@@ -0,0 +1,54 @@
+import Logger from '../../../../util/Logger';
+import { logAccountsControllerCreation } from './logger';
+import { defaultAccountsControllerState } from './utils';
+import { MOCK_ACCOUNTS_CONTROLLER_STATE } from '../../../../util/test/accountsControllerTestUtils';
+
+jest.mock('../../../../util/Logger');
+
+describe('logAccountsControllerCreation', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('logs creation with default state when no initial state provided', () => {
+    logAccountsControllerCreation();
+
+    expect(Logger.log).toHaveBeenCalledWith(
+      'Creating AccountsController with default state',
+      {
+        defaultState: defaultAccountsControllerState,
+      },
+    );
+  });
+
+  it('logs creation with empty initial state', () => {
+    const initialState = {
+      internalAccounts: {
+        accounts: {},
+        selectedAccount: '',
+      },
+    };
+
+    logAccountsControllerCreation(initialState);
+
+    expect(Logger.log).toHaveBeenCalledWith(
+      'Creating AccountsController with provided initial state',
+      {
+        hasSelectedAccount: false,
+        accountsCount: 0,
+      },
+    );
+  });
+
+  it('logs creation with populated initial state', () => {
+    logAccountsControllerCreation(MOCK_ACCOUNTS_CONTROLLER_STATE);
+
+    expect(Logger.log).toHaveBeenCalledWith(
+      'Creating AccountsController with provided initial state',
+      {
+        hasSelectedAccount: true,
+        accountsCount: 2,
+      },
+    );
+  });
+});

app/core/Engine/controllers/AccountsController/logger.ts

@@ -0,0 +1,19 @@
+import { AccountsControllerState } from '@metamask/accounts-controller';
+import Logger from '../../../../util/Logger';
+import { defaultAccountsControllerState } from './utils';
+
+export function logAccountsControllerCreation(
+  initialState?: AccountsControllerState,
+) {
+  if (!initialState) {
+    Logger.log('Creating AccountsController with default state', {
+      defaultState: defaultAccountsControllerState,
+    });
+  } else {
+    Logger.log('Creating AccountsController with provided initial state', {
+      hasSelectedAccount: !!initialState.internalAccounts?.selectedAccount,
+      accountsCount: Object.keys(initialState.internalAccounts?.accounts || {})
+        .length,
+    });
+  }
+}

app/core/Engine/controllers/AccountsController/utils.test.ts

@@ -10,11 +10,19 @@ import {
 import { withScope } from '@sentry/react-native';
 import { AGREED, METRICS_OPT_IN } from '../../../../constants/storage';
 import StorageWrapper from '../../../../store/storage-wrapper';
+import { logAccountsControllerCreation } from './logger';
 
 jest.mock('@sentry/react-native', () => ({
   withScope: jest.fn(),
 }));
+jest.mock('./logger', () => ({
+  logAccountsControllerCreation: jest.fn(),
+}));
+
 const mockedWithScope = jest.mocked(withScope);
+const mockedLogAccountsControllerCreation = jest.mocked(
+  logAccountsControllerCreation,
+);
 
 describe('accountControllersUtils', () => {
   describe('createAccountsController', () => {
@@ -50,12 +58,38 @@ describe('accountControllersUtils', () => {
       jest.resetAllMocks();
     });
 
+    it('logs creation with default state when no initial state provided', () => {
+      createAccountsController({
+        messenger: accountsControllerMessenger,
+      });
+      expect(mockedLogAccountsControllerCreation).toHaveBeenCalledWith(
+        undefined,
+      );
+    });
+
+    it('logs creation with provided initial state', () => {
+      const initialState = {
+        internalAccounts: {
+          accounts: {},
+          selectedAccount: '0x1',
+        },
+      };
+      createAccountsController({
+        messenger: accountsControllerMessenger,
+        initialState,
+      });
+      expect(mockedLogAccountsControllerCreation).toHaveBeenCalledWith(
+        initialState,
+      );
+    });
+
     it('AccountsController state should be default state when no initial state is passed in', () => {
       const accountsController = createAccountsController({
         messenger: accountsControllerMessenger,
       });
       expect(accountsController.state).toEqual(defaultAccountsControllerState);
     });
+
     it('AccountsController state should be initial state when initial state is passed in', () => {
       const initialAccountsControllerState: AccountsControllerState = {
         internalAccounts: {
@@ -69,13 +103,15 @@ describe('accountControllersUtils', () => {
       });
       expect(accountsController.state).toEqual(initialAccountsControllerState);
     });
+
     it('AccountsController name should be AccountsController', () => {
       const accountsControllerName = 'AccountsController';
       const accountsController = createAccountsController({
         messenger: accountsControllerMessenger,
       });
       expect(accountsController.name).toEqual(accountsControllerName);
     });
+
     it('should detect and log an error when controller fails to initialize', async () => {
       const brokenAccountsControllerMessenger =
         'controllerMessenger' as unknown as AccountsControllerMessenger;

app/core/Engine/controllers/AccountsController/utils.ts

@@ -4,6 +4,7 @@ import {
   AccountsControllerState,
 } from '@metamask/accounts-controller';
 import Logger from '../../../../util/Logger';
+import { logAccountsControllerCreation } from './logger';
 
 // Default AccountsControllerState
 export const defaultAccountsControllerState: AccountsControllerState = {
@@ -28,6 +29,7 @@ export const createAccountsController = ({
   initialState?: AccountsControllerState;
 }): AccountsController => {
   try {
+    logAccountsControllerCreation(initialState);
     const accountsController = new AccountsController({
       messenger,
       state: initialState ?? defaultAccountsControllerState,

app/core/Engine/utils/logger.ts

@@ -0,0 +1,20 @@
+import { KeyringControllerState } from '@metamask/keyring-controller';
+import { EngineState } from '../types';
+import Logger from '../../../util/Logger';
+
+export function logEngineCreation(
+  initialState: Partial<EngineState> = {},
+  initialKeyringState?: KeyringControllerState | null,
+) {
+  if (Object.keys(initialState).length === 0) {
+    Logger.log('Engine initialized with empty state', {
+      keyringStateFromBackup: !!initialKeyringState,
+    });
+  } else {
+    Logger.log('Engine initialized with non-empty state', {
+      hasAccountsState: !!initialState.AccountsController,
+      hasKeyringState: !!initialState.KeyringController,
+      keyringStateFromBackup: !!initialKeyringState,
+    });
+  }
+}