chore: Improve Keyring/Accounts error handling and logs (#12822)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**
This PR does the following

1. unmasks non sensitive data from the Accounts Controller and Keyring
controller to help us better debug issues in prod. This means that more
data will be provided in the sentry state logs. I have ensured that all
addresses have remained masked
2. adds logs when creating key components such as the engine and the
accounts controller. This will help us debug issues with empty state.
3. adds validation to the migrations for key components. Currently we
are validating that the accounts controller and keyring controller data
are sound. This validation will not block the app from initializing but
will log the errors it finds.

## **Related issues**

Progresses : #12408

## **Manual testing steps**

1. this change only adds logs and error handling to the app so there
should be no functional changes. that being said, we should still verify
that the upgrade path is working.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

app/core/Engine/Engine.ts

@@ -213,6 +213,7 @@ import {
   getGlobalChainId,
   getGlobalNetworkClientId,
 } from '../../util/networks/global-network';
+import { logEngineCreation } from './utils/logger';
 
 const NON_EMPTY = 'NON_EMPTY';
 
@@ -276,6 +277,8 @@ export class Engine {
     initialState: Partial<EngineState> = {},
     initialKeyringState?: KeyringControllerState | null,
   ) {
+    logEngineCreation(initialState, initialKeyringState);
+
     this.controllerMessenger = new ExtendedControllerMessenger();
 
     const isBasicFunctionalityToggleEnabled = () =>
@@ -971,8 +974,8 @@ export class Engine {
         disableSnaps: !isBasicFunctionalityToggleEnabled(),
       }),
       clientCryptography: {
-        pbkdf2Sha512: pbkdf2
-      }
+        pbkdf2Sha512: pbkdf2,
+      },
     });
 
     const authenticationController = new AuthenticationController.Controller({
@@ -1173,7 +1176,7 @@ export class Engine {
 
           return Boolean(
             hasProperty(showIncomingTransactions, currentChainId) &&
-            showIncomingTransactions?.[currentHexChainId],
+              showIncomingTransactions?.[currentHexChainId],
           );
         },
         updateTransactions: true,
@@ -1305,7 +1308,9 @@ export class Engine {
               .addProperties({
                 token_standard: 'ERC20',
                 asset_type: 'token',
-                chain_id: getDecimalChainId(getGlobalChainId(networkController)),
+                chain_id: getDecimalChainId(
+                  getGlobalChainId(networkController),
+                ),
               })
               .build(),
           ),
@@ -1517,7 +1522,7 @@ export class Engine {
         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 @@ export class Engine {
         } 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 @@ export class Engine {
 
         const tokenBalances =
           allTokenBalances?.[selectedInternalAccount.address as Hex]?.[
-          chainId
+            chainId
           ] ?? {};
         tokens.forEach(
           (item: { address: string; balance?: string; decimals: number }) => {
@@ -1786,9 +1793,9 @@ export class Engine {
               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
@@ -2148,12 +2155,15 @@ export default {
     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,
+    });
+  }
+}