feat(money-account-upgrade-controller): persist upgrade status and add retry

packages/money-account-upgrade-controller/CHANGELOG.md

@@ -7,12 +7,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- **BREAKING:** Add persisted state tracking fully upgraded accounts ([#9500](https://github.com/MetaMask/core/pull/9500))
+  - `MoneyAccountUpgradeControllerState` changes from `Record<string, never>` to `{ upgradedAccounts }`, keyed by lowercased account address. Each entry records when the upgrade sequence completed and a fingerprint of the config it completed under (see new `MoneyAccountUpgradeStatus` type). Code constructing the state type (e.g. `{}` in tests or default-state maps) must include `upgradedAccounts`.
+  - The constructor now accepts an optional `state` option, merged with the defaults; add `getDefaultMoneyAccountUpgradeControllerState` to construct those defaults.
+- Add `TerminalUpgradeError` and `isTerminalMoneyAccountUpgradeError`, and a `terminal` property on `MoneyAccountUpgradeStepError`, marking failures that cannot resolve by retrying — currently an account delegated to a third-party EIP-7702 implementation, an account with unexpected on-chain code, or an address confirmed to be associated with a different CHOMP profile ([#9500](https://github.com/MetaMask/core/pull/9500))
+  - The controller does not retry on its own; clients implementing their own retry logic around `upgradeAccount` can use `isTerminalMoneyAccountUpgradeError` to stop retrying failures that cannot succeed.
+
 ### Changed
 
 - **BREAKING:** The `associate-address` upgrade step now checks the profile's existing address associations via `ChompApiService:getAssociatedAddresses` before signing, and reports `already-done` without signing or submitting anything when the address is already associated ([#9387](https://github.com/MetaMask/core/pull/9387))
   - `MoneyAccountUpgradeControllerMessenger` consumers must grant the `ChompApiService:getAssociatedAddresses` action alongside the previously required actions, and must provide a `@metamask/chomp-api-service` version that registers it (`>=4.0.0`).
   - The lookup is an optimization: if it fails, the step falls through to the previous sign-and-submit behavior.
   - A 409 conflict from the association request is disambiguated by re-fetching the associations, so a same-profile create race reports `already-done` instead of failing the upgrade; a genuine conflict (address associated with a different profile) still fails the step.
+- `upgradeAccount` now skips the step sequence entirely when the account is recorded in state as upgraded under the active config fingerprint, and records the account after a successful run. If the chain, CHOMP contract addresses, or Delegation Framework version change, the fingerprint no longer matches and the sequence re-runs ([#9500](https://github.com/MetaMask/core/pull/9500))
 - Bump `@metamask/messenger` from `^1.2.0` to `^2.0.0` ([#9392](https://github.com/MetaMask/core/pull/9392))
 - Bump `@metamask/authenticated-user-storage` from `^3.0.0` to `^3.0.1` ([#9458](https://github.com/MetaMask/core/pull/9458))
 

packages/money-account-upgrade-controller/src/MoneyAccountUpgradeController-method-action-types.ts

@@ -13,6 +13,12 @@ import type { MoneyAccountUpgradeController } from './MoneyAccountUpgradeControl
  * {@link MoneyAccountUpgradeStepError} that records which step failed (the
  * original error is preserved as `cause`).
  *
+ * A run that completes is recorded in state (keyed by lowercased address,
+ * fingerprinted against the active config); subsequent calls for a
+ * recorded account return immediately without running any steps. If the
+ * active config no longer matches the recorded fingerprint, the sequence
+ * re-runs.
+ *
  * @param address - The Money Account address to upgrade.
  */
 export type MoneyAccountUpgradeControllerUpgradeAccountAction = {

packages/money-account-upgrade-controller/src/MoneyAccountUpgradeController.test.ts

@@ -10,11 +10,14 @@ import type { Hex } from '@metamask/utils';
 
 import type {
   MoneyAccountUpgradeControllerMessenger,
+  MoneyAccountUpgradeControllerState,
   MoneyAccountUpgradeStepError,
 } from '.';
 import {
   MoneyAccountUpgradeController,
+  getDefaultMoneyAccountUpgradeControllerState,
   isMoneyAccountUpgradeStepError,
+  isTerminalMoneyAccountUpgradeError,
 } from '.';
 
 const MOCK_CHAIN_ID = '0x1' as Hex; // mainnet, supported in delegation-deployments@1.3.0
@@ -84,7 +87,11 @@ type Mocks = {
   createIntents: jest.Mock;
 };
 
-function setup(): {
+function setup({
+  state,
+}: {
+  state?: Partial<MoneyAccountUpgradeControllerState>;
+} = {}): {
   controller: MoneyAccountUpgradeController;
   rootMessenger: RootMessenger;
   messenger: MoneyAccountUpgradeControllerMessenger;
@@ -230,18 +237,53 @@ function setup(): {
 
   const controller = new MoneyAccountUpgradeController({
     messenger,
+    state,
   });
 
   return { controller, rootMessenger, messenger, mocks };
 }
 
+/**
+ * Resets the call history of every mock in the bag, preserving their
+ * configured implementations. Useful for asserting that a later
+ * `upgradeAccount` call performs no work.
+ *
+ * @param mocks - The mocks bag from `setup`.
+ */
+function clearMockCalls(mocks: Mocks): void {
+  for (const mock of Object.values(mocks)) {
+    mock.mockClear();
+  }
+}
+
 describe('MoneyAccountUpgradeController', () => {
   describe('constructor', () => {
     it('does not make async init calls when constructed', () => {
       const { mocks } = setup();
 
       expect(mocks.getServiceDetails).not.toHaveBeenCalled();
     });
+
+    it('starts with the default empty state', () => {
+      const { controller } = setup();
+
+      expect(controller.state).toStrictEqual(
+        getDefaultMoneyAccountUpgradeControllerState(),
+      );
+      expect(controller.state.upgradedAccounts).toStrictEqual({});
+    });
+
+    it('merges provided partial state with the defaults', () => {
+      const status = { configFingerprint: 'fingerprint', completedAt: 123 };
+
+      const { controller } = setup({
+        state: { upgradedAccounts: { [MOCK_ACCOUNT_ADDRESS]: status } },
+      });
+
+      expect(
+        controller.state.upgradedAccounts[MOCK_ACCOUNT_ADDRESS],
+      ).toStrictEqual(status);
+    });
   });
 
   describe('init', () => {
@@ -516,5 +558,180 @@ describe('MoneyAccountUpgradeController', () => {
         'Money Account upgrade failed at step "associate-address": plain string failure',
       );
     });
+
+    it('marks the failure terminal when the account is delegated to another implementation', async () => {
+      const { controller, mocks } = setup();
+      await controller.init({
+        chainId: MOCK_CHAIN_ID,
+        boringVaultAddress: MOCK_BORING_VAULT_ADDRESS,
+      });
+      // EIP-7702 delegation code pointing at a third-party impl.
+      mocks.providerRequest.mockImplementation(
+        async ({ method }: { method: string }) => {
+          if (method === 'eth_getCode') {
+            return `0xef0100${'9'.repeat(40)}`;
+          }
+          return '0x0';
+        },
+      );
+
+      const error = await controller
+        .upgradeAccount(MOCK_ACCOUNT_ADDRESS)
+        .catch((thrown: unknown) => thrown);
+
+      expect(isTerminalMoneyAccountUpgradeError(error)).toBe(true);
+    });
+
+    it('marks ordinary step failures as non-terminal', async () => {
+      const { controller, mocks } = setup();
+      await controller.init({
+        chainId: MOCK_CHAIN_ID,
+        boringVaultAddress: MOCK_BORING_VAULT_ADDRESS,
+      });
+      mocks.signPersonalMessage.mockRejectedValue(new Error('network down'));
+
+      const error = await controller
+        .upgradeAccount(MOCK_ACCOUNT_ADDRESS)
+        .catch((thrown: unknown) => thrown);
+
+      expect(isMoneyAccountUpgradeStepError(error)).toBe(true);
+      expect(isTerminalMoneyAccountUpgradeError(error)).toBe(false);
+    });
+  });
+
+  describe('upgrade status tracking', () => {
+    it('records a successful upgrade against the lowercased address', async () => {
+      const { controller, mocks } = setup();
+      await controller.init({
+        chainId: MOCK_CHAIN_ID,
+        boringVaultAddress: MOCK_BORING_VAULT_ADDRESS,
+      });
+      const mixedCaseAddress = MOCK_ACCOUNT_ADDRESS.replace(
+        '0xabc',
+        '0xABC',
+      ) as Hex;
+
+      await controller.upgradeAccount(mixedCaseAddress);
+
+      expect(mocks.signPersonalMessage).toHaveBeenCalled();
+      expect(
+        controller.state.upgradedAccounts[MOCK_ACCOUNT_ADDRESS],
+      ).toStrictEqual({
+        configFingerprint: expect.any(String),
+        completedAt: expect.any(Number),
+      });
+    });
+
+    it('skips the steps on a subsequent call for an already-upgraded account', async () => {
+      const { controller, mocks } = setup();
+      await controller.init({
+        chainId: MOCK_CHAIN_ID,
+        boringVaultAddress: MOCK_BORING_VAULT_ADDRESS,
+      });
+      await controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS);
+      clearMockCalls(mocks);
+
+      await controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS);
+
+      expect(mocks.signPersonalMessage).not.toHaveBeenCalled();
+      expect(mocks.providerRequest).not.toHaveBeenCalled();
+      expect(mocks.listDelegations).not.toHaveBeenCalled();
+      expect(mocks.getIntentsByAddress).not.toHaveBeenCalled();
+    });
+
+    it('treats recorded upgrades case-insensitively', async () => {
+      const { controller, mocks } = setup();
+      await controller.init({
+        chainId: MOCK_CHAIN_ID,
+        boringVaultAddress: MOCK_BORING_VAULT_ADDRESS,
+      });
+      await controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS);
+      clearMockCalls(mocks);
+
+      await controller.upgradeAccount(
+        MOCK_ACCOUNT_ADDRESS.replace('0xabc', '0xABC') as Hex,
+      );
+
+      expect(mocks.signPersonalMessage).not.toHaveBeenCalled();
+    });
+
+    it('skips the steps when constructed with state from a previous successful upgrade', async () => {
+      const first = setup();
+      await first.controller.init({
+        chainId: MOCK_CHAIN_ID,
+        boringVaultAddress: MOCK_BORING_VAULT_ADDRESS,
+      });
+      await first.controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS);
+
+      const second = setup({ state: first.controller.state });
+      await second.controller.init({
+        chainId: MOCK_CHAIN_ID,
+        boringVaultAddress: MOCK_BORING_VAULT_ADDRESS,
+      });
+      await second.controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS);
+
+      expect(second.mocks.signPersonalMessage).not.toHaveBeenCalled();
+      expect(second.mocks.providerRequest).not.toHaveBeenCalled();
+    });
+
+    it('does not record the account when a step fails, and re-runs on the next call', async () => {
+      const { controller, mocks } = setup();
+      await controller.init({
+        chainId: MOCK_CHAIN_ID,
+        boringVaultAddress: MOCK_BORING_VAULT_ADDRESS,
+      });
+      mocks.signPersonalMessage.mockRejectedValueOnce(
+        new Error('signing failed'),
+      );
+
+      await expect(
+        controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS),
+      ).rejects.toThrow('signing failed');
+
+      expect(controller.state.upgradedAccounts).toStrictEqual({});
+
+      await controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS);
+
+      expect(
+        controller.state.upgradedAccounts[MOCK_ACCOUNT_ADDRESS],
+      ).toBeDefined();
+    });
+
+    it('re-runs the sequence when the active config no longer matches the recorded fingerprint', async () => {
+      const { controller, mocks } = setup();
+      await controller.init({
+        chainId: MOCK_CHAIN_ID,
+        boringVaultAddress: MOCK_BORING_VAULT_ADDRESS,
+      });
+      await controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS);
+      const { configFingerprint: originalFingerprint } =
+        controller.state.upgradedAccounts[MOCK_ACCOUNT_ADDRESS];
+
+      // CHOMP rotates its delegate address — the recorded upgrade no longer
+      // reflects the active config.
+      mocks.getServiceDetails.mockResolvedValue({
+        ...MOCK_SERVICE_DETAILS_RESPONSE,
+        chains: {
+          [MOCK_CHAIN_ID]: {
+            ...MOCK_SERVICE_DETAILS_RESPONSE.chains[MOCK_CHAIN_ID],
+            autoDepositDelegate:
+              '0x2222222222222222222222222222222222222222' as Hex,
+          },
+        },
+      });
+      await controller.init({
+        chainId: MOCK_CHAIN_ID,
+        boringVaultAddress: MOCK_BORING_VAULT_ADDRESS,
+      });
+      clearMockCalls(mocks);
+
+      await controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS);
+
+      expect(mocks.signPersonalMessage).toHaveBeenCalled();
+      expect(
+        controller.state.upgradedAccounts[MOCK_ACCOUNT_ADDRESS]
+          .configFingerprint,
+      ).not.toBe(originalFingerprint);
+    });
   });
 });