diff --git a/packages/money-account-upgrade-controller/CHANGELOG.md b/packages/money-account-upgrade-controller/CHANGELOG.md index eecd67b75d..152546957c 100644 --- a/packages/money-account-upgrade-controller/CHANGELOG.md +++ b/packages/money-account-upgrade-controller/CHANGELOG.md @@ -7,8 +7,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add persisted state tracking fully upgraded accounts ([#9500](https://github.com/MetaMask/core/pull/9500)) + - `MoneyAccountUpgradeControllerState` now contains `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). + - The constructor now accepts an optional `state` option, merged with the defaults; add `getDefaultMoneyAccountUpgradeControllerState` to construct those defaults. +- Add `upgradeAccountWithRetry` method and matching `MoneyAccountUpgradeController:upgradeAccountWithRetry` messenger action ([#9500](https://github.com/MetaMask/core/pull/9500)) + - Retries failed `upgradeAccount` attempts with capped exponential backoff (10s, 20s, 40s, then 60s between attempts; 5 attempts by default). Terminal failures and non-step errors are rethrown without retrying. Accepts an `AbortSignal` to cancel waiting between attempts. +- 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, or an account with unexpected on-chain code ([#9500](https://github.com/MetaMask/core/pull/9500)) + ### Changed +- `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 ([#9999](https://github.com/MetaMask/core/pull/9999)) - 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)) diff --git a/packages/money-account-upgrade-controller/src/MoneyAccountUpgradeController-method-action-types.ts b/packages/money-account-upgrade-controller/src/MoneyAccountUpgradeController-method-action-types.ts index 23d5c72362..b40cbd0f9d 100644 --- a/packages/money-account-upgrade-controller/src/MoneyAccountUpgradeController-method-action-types.ts +++ b/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 = { @@ -20,8 +26,30 @@ export type MoneyAccountUpgradeControllerUpgradeAccountAction = { handler: MoneyAccountUpgradeController['upgradeAccount']; }; +/** + * Runs the upgrade sequence via + * {@link MoneyAccountUpgradeController.upgradeAccount}, retrying failed + * attempts with capped exponential backoff (10s, 20s, 40s, then 60s + * between attempts). Rethrows the last error without further attempts when + * the failure is terminal (see `isTerminalMoneyAccountUpgradeError`), when + * it is not a step failure at all, or when `maxAttempts` is exhausted. + * + * @param address - The Money Account address to upgrade. + * @param options - Retry options. + * @param options.signal - Aborts waiting between attempts and prevents + * further attempts. An aborted run rejects with an error stating the retry + * was aborted. + * @param options.maxAttempts - Maximum number of attempts, including the + * first. Defaults to 5. + */ +export type MoneyAccountUpgradeControllerUpgradeAccountWithRetryAction = { + type: `MoneyAccountUpgradeController:upgradeAccountWithRetry`; + handler: MoneyAccountUpgradeController['upgradeAccountWithRetry']; +}; + /** * Union of all MoneyAccountUpgradeController action types. */ export type MoneyAccountUpgradeControllerMethodActions = - MoneyAccountUpgradeControllerUpgradeAccountAction; + | MoneyAccountUpgradeControllerUpgradeAccountAction + | MoneyAccountUpgradeControllerUpgradeAccountWithRetryAction; diff --git a/packages/money-account-upgrade-controller/src/MoneyAccountUpgradeController.test.ts b/packages/money-account-upgrade-controller/src/MoneyAccountUpgradeController.test.ts index b617238242..c9797f59c7 100644 --- a/packages/money-account-upgrade-controller/src/MoneyAccountUpgradeController.test.ts +++ b/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 @@ -83,7 +86,11 @@ type Mocks = { createIntents: jest.Mock; }; -function setup(): { +function setup({ + state, +}: { + state?: Partial; +} = {}): { controller: MoneyAccountUpgradeController; rootMessenger: RootMessenger; messenger: MoneyAccountUpgradeControllerMessenger; @@ -223,11 +230,25 @@ 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', () => { @@ -235,6 +256,27 @@ describe('MoneyAccountUpgradeController', () => { 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', () => { @@ -509,5 +551,379 @@ 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); + }); + }); + + describe('upgradeAccountWithRetry', () => { + afterEach(() => { + jest.useRealTimers(); + }); + + it('resolves after a single attempt when the upgrade succeeds', async () => { + const { controller, mocks } = setup(); + await controller.init({ + chainId: MOCK_CHAIN_ID, + boringVaultAddress: MOCK_BORING_VAULT_ADDRESS, + }); + + await controller.upgradeAccountWithRetry(MOCK_ACCOUNT_ADDRESS); + + expect(mocks.signPersonalMessage).toHaveBeenCalledTimes(1); + expect( + controller.state.upgradedAccounts[MOCK_ACCOUNT_ADDRESS], + ).toBeDefined(); + }); + + it('retries a failed attempt after 10 seconds', async () => { + const { controller, mocks } = setup(); + await controller.init({ + chainId: MOCK_CHAIN_ID, + boringVaultAddress: MOCK_BORING_VAULT_ADDRESS, + }); + mocks.signPersonalMessage.mockRejectedValueOnce( + new Error('network down'), + ); + jest.useFakeTimers(); + + const promise = controller.upgradeAccountWithRetry(MOCK_ACCOUNT_ADDRESS); + await jest.advanceTimersByTimeAsync(0); + expect(mocks.signPersonalMessage).toHaveBeenCalledTimes(1); + + await jest.advanceTimersByTimeAsync(9_999); + expect(mocks.signPersonalMessage).toHaveBeenCalledTimes(1); + + await jest.advanceTimersByTimeAsync(1); + await promise; + + expect(mocks.signPersonalMessage).toHaveBeenCalledTimes(2); + expect( + controller.state.upgradedAccounts[MOCK_ACCOUNT_ADDRESS], + ).toBeDefined(); + }); + + it('backs off exponentially between attempts, capped at 60 seconds', async () => { + const { controller, mocks } = setup(); + await controller.init({ + chainId: MOCK_CHAIN_ID, + boringVaultAddress: MOCK_BORING_VAULT_ADDRESS, + }); + mocks.signPersonalMessage.mockRejectedValue(new Error('network down')); + jest.useFakeTimers(); + + const promise = controller.upgradeAccountWithRetry(MOCK_ACCOUNT_ADDRESS, { + maxAttempts: 6, + }); + // Swallow the eventual rejection so advancing timers doesn't surface an + // unhandled rejection; the real assertion happens below. + promise.catch(() => undefined); + + await jest.advanceTimersByTimeAsync(0); + expect(mocks.signPersonalMessage).toHaveBeenCalledTimes(1); + await jest.advanceTimersByTimeAsync(10_000); + expect(mocks.signPersonalMessage).toHaveBeenCalledTimes(2); + await jest.advanceTimersByTimeAsync(20_000); + expect(mocks.signPersonalMessage).toHaveBeenCalledTimes(3); + await jest.advanceTimersByTimeAsync(40_000); + expect(mocks.signPersonalMessage).toHaveBeenCalledTimes(4); + await jest.advanceTimersByTimeAsync(60_000); + expect(mocks.signPersonalMessage).toHaveBeenCalledTimes(5); + // The cap repeats once the schedule is exhausted. + await jest.advanceTimersByTimeAsync(60_000); + expect(mocks.signPersonalMessage).toHaveBeenCalledTimes(6); + + await expect(promise).rejects.toThrow('network down'); + }); + + it('gives up after maxAttempts and rethrows the last step error', async () => { + const { controller, mocks } = setup(); + await controller.init({ + chainId: MOCK_CHAIN_ID, + boringVaultAddress: MOCK_BORING_VAULT_ADDRESS, + }); + mocks.signPersonalMessage.mockRejectedValue(new Error('network down')); + jest.useFakeTimers(); + + const promise = controller.upgradeAccountWithRetry(MOCK_ACCOUNT_ADDRESS, { + maxAttempts: 2, + }); + promise.catch(() => undefined); + await jest.advanceTimersByTimeAsync(10_000); + + await expect(promise).rejects.toMatchObject({ + step: 'associate-address', + }); + expect(mocks.signPersonalMessage).toHaveBeenCalledTimes(2); + expect(controller.state.upgradedAccounts).toStrictEqual({}); + }); + + it('does not retry terminal failures', async () => { + const { controller, mocks } = setup(); + await controller.init({ + chainId: MOCK_CHAIN_ID, + boringVaultAddress: MOCK_BORING_VAULT_ADDRESS, + }); + // Account delegated to a third-party impl — retrying cannot help. + mocks.providerRequest.mockImplementation( + async ({ method }: { method: string }) => { + if (method === 'eth_getCode') { + return `0xef0100${'9'.repeat(40)}`; + } + return '0x0'; + }, + ); + + await expect( + controller.upgradeAccountWithRetry(MOCK_ACCOUNT_ADDRESS), + ).rejects.toThrow('already upgraded to another smart account'); + + expect(mocks.signEip7702Authorization).not.toHaveBeenCalled(); + expect(mocks.signPersonalMessage).toHaveBeenCalledTimes(1); + }); + + it('does not retry non-step errors such as calling before init', async () => { + const { controller, mocks } = setup(); + + await expect( + controller.upgradeAccountWithRetry(MOCK_ACCOUNT_ADDRESS), + ).rejects.toThrow( + 'MoneyAccountUpgradeController must be initialized via init() before upgradeAccount() can be called', + ); + + expect(mocks.signPersonalMessage).not.toHaveBeenCalled(); + }); + + it('throws without attempting when the signal is already aborted', async () => { + const { controller, mocks } = setup(); + await controller.init({ + chainId: MOCK_CHAIN_ID, + boringVaultAddress: MOCK_BORING_VAULT_ADDRESS, + }); + const abortController = new AbortController(); + abortController.abort(); + + await expect( + controller.upgradeAccountWithRetry(MOCK_ACCOUNT_ADDRESS, { + signal: abortController.signal, + }), + ).rejects.toThrow('Money Account upgrade retry aborted'); + + expect(mocks.signPersonalMessage).not.toHaveBeenCalled(); + }); + + it('stops retrying when the signal aborts during the backoff wait', async () => { + const { controller, mocks } = setup(); + await controller.init({ + chainId: MOCK_CHAIN_ID, + boringVaultAddress: MOCK_BORING_VAULT_ADDRESS, + }); + mocks.signPersonalMessage.mockRejectedValue(new Error('network down')); + jest.useFakeTimers(); + const abortController = new AbortController(); + + const promise = controller.upgradeAccountWithRetry(MOCK_ACCOUNT_ADDRESS, { + signal: abortController.signal, + }); + promise.catch(() => undefined); + await jest.advanceTimersByTimeAsync(0); + expect(mocks.signPersonalMessage).toHaveBeenCalledTimes(1); + + abortController.abort(); + await expect(promise).rejects.toThrow( + 'Money Account upgrade retry aborted', + ); + + // The pending wait was cancelled — advancing time runs no further attempts. + await jest.advanceTimersByTimeAsync(120_000); + expect(mocks.signPersonalMessage).toHaveBeenCalledTimes(1); + }); + + it('is callable via the messenger', async () => { + const { controller, rootMessenger } = setup(); + await controller.init({ + chainId: MOCK_CHAIN_ID, + boringVaultAddress: MOCK_BORING_VAULT_ADDRESS, + }); + + expect( + await rootMessenger.call( + 'MoneyAccountUpgradeController:upgradeAccountWithRetry', + MOCK_ACCOUNT_ADDRESS, + ), + ).toBeUndefined(); + }); }); }); diff --git a/packages/money-account-upgrade-controller/src/MoneyAccountUpgradeController.ts b/packages/money-account-upgrade-controller/src/MoneyAccountUpgradeController.ts index 7d0556e5ec..c217d6b119 100644 --- a/packages/money-account-upgrade-controller/src/MoneyAccountUpgradeController.ts +++ b/packages/money-account-upgrade-controller/src/MoneyAccountUpgradeController.ts @@ -30,7 +30,11 @@ import type { import { hexToNumber } from '@metamask/utils'; import type { Hex } from '@metamask/utils'; -import { MoneyAccountUpgradeStepError } from './errors'; +import { + MoneyAccountUpgradeStepError, + isMoneyAccountUpgradeStepError, + isTerminalMoneyAccountUpgradeError, +} from './errors'; import type { MoneyAccountUpgradeControllerMethodActions } from './MoneyAccountUpgradeController-method-action-types'; import { associateAddressStep } from './steps/associate-address'; import { buildDelegationStep } from './steps/build-delegations'; @@ -47,12 +51,67 @@ const DELEGATION_FRAMEWORK_VERSION = '1.3.0'; export const controllerName = 'MoneyAccountUpgradeController'; -export type MoneyAccountUpgradeControllerState = Record; +/** + * Delays between retry attempts in + * {@link MoneyAccountUpgradeController.upgradeAccountWithRetry}. Once the + * schedule is exhausted the last delay repeats. + */ +const RETRY_DELAYS_MS = [10_000, 20_000, 40_000, 60_000]; + +const DEFAULT_MAX_RETRY_ATTEMPTS = 5; + +const RETRY_ABORTED_MESSAGE = 'Money Account upgrade retry aborted'; + +/** + * Record of a Money Account upgrade sequence that ran to completion. + */ +export type MoneyAccountUpgradeStatus = { + /** + * Fingerprint of the upgrade config the sequence completed under. The + * record is only trusted while the active config produces the same + * fingerprint — if the chain, CHOMP contracts, or Delegation Framework + * version change, the sequence re-runs. + */ + configFingerprint: string; + /** Unix timestamp (in milliseconds) when the sequence completed. */ + completedAt: number; +}; -const moneyAccountUpgradeControllerMetadata = - {} satisfies StateMetadata; +export type MoneyAccountUpgradeControllerState = { + /** + * Accounts whose upgrade sequence has fully completed, keyed by lowercased + * account address. + */ + upgradedAccounts: { [address: Hex]: MoneyAccountUpgradeStatus }; +}; -const MESSENGER_EXPOSED_METHODS = ['upgradeAccount'] as const; +const moneyAccountUpgradeControllerMetadata = { + upgradedAccounts: { + includeInDebugSnapshot: false, + includeInStateLogs: true, + persist: true, + usedInUi: false, + }, +} satisfies StateMetadata; + +/** + * Constructs the default {@link MoneyAccountUpgradeController} state. This + * allows consumers to provide a partial state object when initializing the + * controller and also helps in constructing complete state objects for this + * controller in tests. + * + * @returns The default {@link MoneyAccountUpgradeController} state. + */ +export function getDefaultMoneyAccountUpgradeControllerState(): MoneyAccountUpgradeControllerState { + return { + upgradedAccounts: {}, + }; +} + +const MESSENGER_EXPOSED_METHODS = [ + 'upgradeAccount', + 'upgradeAccountWithRetry', +] as const; export type MoneyAccountUpgradeControllerGetStateAction = ControllerGetStateAction< @@ -118,17 +177,23 @@ export class MoneyAccountUpgradeController extends BaseController< * * @param options - The options for constructing the controller. * @param options.messenger - The messenger to use for inter-controller communication. + * @param options.state - The initial state, merged with the defaults. */ constructor({ messenger, + state, }: { messenger: MoneyAccountUpgradeControllerMessenger; + state?: Partial; }) { super({ messenger, metadata: moneyAccountUpgradeControllerMetadata, name: controllerName, - state: {}, + state: { + ...getDefaultMoneyAccountUpgradeControllerState(), + ...state, + }, }); this.messenger.registerMethodActionHandlers( @@ -208,6 +273,12 @@ export class MoneyAccountUpgradeController extends BaseController< * {@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. */ async upgradeAccount(address: Hex): Promise { @@ -216,17 +287,140 @@ export class MoneyAccountUpgradeController extends BaseController< 'MoneyAccountUpgradeController must be initialized via init() before upgradeAccount() can be called', ); } + const config = this.#config; + + const accountKey = address.toLowerCase() as Hex; + const configFingerprint = computeConfigFingerprint(config); + if ( + this.state.upgradedAccounts[accountKey]?.configFingerprint === + configFingerprint + ) { + return; + } for (const step of this.#steps) { try { await step.run({ messenger: this.messenger, address, - ...this.#config, + ...config, }); } catch (error) { throw new MoneyAccountUpgradeStepError(step.name, error); } } + + this.update((state) => { + state.upgradedAccounts[accountKey] = { + configFingerprint, + completedAt: Date.now(), + }; + }); + } + + /** + * Runs the upgrade sequence via + * {@link MoneyAccountUpgradeController.upgradeAccount}, retrying failed + * attempts with capped exponential backoff (10s, 20s, 40s, then 60s + * between attempts). Rethrows the last error without further attempts when + * the failure is terminal (see `isTerminalMoneyAccountUpgradeError`), when + * it is not a step failure at all, or when `maxAttempts` is exhausted. + * + * @param address - The Money Account address to upgrade. + * @param options - Retry options. + * @param options.signal - Aborts waiting between attempts and prevents + * further attempts. An aborted run rejects with an error stating the retry + * was aborted. + * @param options.maxAttempts - Maximum number of attempts, including the + * first. Defaults to 5. + */ + async upgradeAccountWithRetry( + address: Hex, + { + signal, + maxAttempts = DEFAULT_MAX_RETRY_ATTEMPTS, + }: { signal?: AbortSignal; maxAttempts?: number } = {}, + ): Promise { + for (let attempt = 1; ; attempt++) { + if (signal?.aborted) { + throw new Error(RETRY_ABORTED_MESSAGE); + } + try { + await this.upgradeAccount(address); + return; + } catch (error) { + if ( + attempt >= maxAttempts || + !isMoneyAccountUpgradeStepError(error) || + isTerminalMoneyAccountUpgradeError(error) + ) { + throw error; + } + await waitUnlessAborted(retryDelayMs(attempt), signal); + } + } } } + +/** + * Derives a stable fingerprint of the config fields that define what + * "upgraded" means for an account. A recorded upgrade is only trusted while + * the active config produces the same fingerprint. + * + * @param config - The active upgrade config. + * @returns A canonical string over the config's identifying fields. + */ +function computeConfigFingerprint( + config: UpgradeConfig & { chainId: Hex }, +): string { + return [ + DELEGATION_FRAMEWORK_VERSION, + config.chainId, + config.delegateAddress, + config.musdTokenAddress, + config.boringVaultAddress, + config.vedaVaultAdapterAddress, + config.delegatorImplAddress, + config.erc20TransferAmountEnforcer, + config.redeemerEnforcer, + config.valueLteEnforcer, + ] + .map((value) => value.toLowerCase()) + .join('|'); +} + +/** + * The backoff delay to wait after the given (1-indexed) failed attempt. Once + * the schedule is exhausted, the last delay repeats. + * + * @param attempt - The attempt that just failed. + * @returns The delay in milliseconds. + */ +function retryDelayMs(attempt: number): number { + return RETRY_DELAYS_MS[Math.min(attempt, RETRY_DELAYS_MS.length) - 1]; +} + +/** + * Waits for the given duration, rejecting early if `signal` aborts. + * + * @param durationMs - How long to wait. + * @param signal - Abort signal that cancels the wait. + * @returns A promise that resolves after the wait, or rejects on abort. + */ +async function waitUnlessAborted( + durationMs: number, + signal?: AbortSignal, +): Promise { + return new Promise((resolve, reject) => { + const timer = setTimeout(() => { + signal?.removeEventListener('abort', onAbort); + resolve(); + }, durationMs); + + function onAbort(): void { + clearTimeout(timer); + reject(new Error(RETRY_ABORTED_MESSAGE)); + } + signal?.addEventListener('abort', onAbort, { once: true }); + }); +} diff --git a/packages/money-account-upgrade-controller/src/errors.test.ts b/packages/money-account-upgrade-controller/src/errors.test.ts index 53b3f1a885..7bfa63f6ab 100644 --- a/packages/money-account-upgrade-controller/src/errors.test.ts +++ b/packages/money-account-upgrade-controller/src/errors.test.ts @@ -1,6 +1,8 @@ import { MoneyAccountUpgradeStepError, + TerminalUpgradeError, isMoneyAccountUpgradeStepError, + isTerminalMoneyAccountUpgradeError, } from './errors'; describe('MoneyAccountUpgradeStepError', () => { @@ -26,6 +28,87 @@ describe('MoneyAccountUpgradeStepError', () => { 'Money Account upgrade failed at step "register-intents": 42', ); }); + + it('is non-terminal when the cause is a plain Error', () => { + const error = new MoneyAccountUpgradeStepError( + 'associate-address', + new Error('network down'), + ); + + expect(error.terminal).toBe(false); + }); + + it('is terminal when the cause is a TerminalUpgradeError', () => { + const error = new MoneyAccountUpgradeStepError( + 'eip-7702-authorization', + new TerminalUpgradeError('delegated elsewhere'), + ); + + expect(error.terminal).toBe(true); + }); + + it('is terminal when the cause is a structurally-terminal error from another realm', () => { + const cause = new Error('delegated elsewhere'); + (cause as unknown as { terminal: boolean }).terminal = true; + + const error = new MoneyAccountUpgradeStepError( + 'eip-7702-authorization', + cause, + ); + + expect(error.terminal).toBe(true); + }); + + it('is non-terminal when the cause is a non-Error carrying a terminal property', () => { + const error = new MoneyAccountUpgradeStepError('associate-address', { + terminal: true, + }); + + expect(error.terminal).toBe(false); + }); +}); + +describe('TerminalUpgradeError', () => { + it('is an Error marked as terminal', () => { + const error = new TerminalUpgradeError('cannot recover'); + + expect(error).toBeInstanceOf(Error); + expect(error.name).toBe('TerminalUpgradeError'); + expect(error.message).toBe('cannot recover'); + expect(error.terminal).toBe(true); + }); +}); + +describe('isTerminalMoneyAccountUpgradeError', () => { + it('returns true for a step error with a terminal cause', () => { + expect( + isTerminalMoneyAccountUpgradeError( + new MoneyAccountUpgradeStepError( + 'eip-7702-authorization', + new TerminalUpgradeError('delegated elsewhere'), + ), + ), + ).toBe(true); + }); + + it('returns false for a step error with a non-terminal cause', () => { + expect( + isTerminalMoneyAccountUpgradeError( + new MoneyAccountUpgradeStepError('associate-address', new Error('x')), + ), + ).toBe(false); + }); + + it('returns false for an unwrapped TerminalUpgradeError', () => { + expect( + isTerminalMoneyAccountUpgradeError(new TerminalUpgradeError('x')), + ).toBe(false); + }); + + it('returns false for non-step-error values', () => { + expect(isTerminalMoneyAccountUpgradeError(undefined)).toBe(false); + expect(isTerminalMoneyAccountUpgradeError(new Error('x'))).toBe(false); + }); }); describe('isMoneyAccountUpgradeStepError', () => { diff --git a/packages/money-account-upgrade-controller/src/errors.ts b/packages/money-account-upgrade-controller/src/errors.ts index 0fc386d60c..1aef34ff3c 100644 --- a/packages/money-account-upgrade-controller/src/errors.ts +++ b/packages/money-account-upgrade-controller/src/errors.ts @@ -14,6 +14,13 @@ export class MoneyAccountUpgradeStepError extends Error { /** The underlying error thrown by the step. */ readonly cause: unknown; + /** + * Whether the failure is terminal — the condition will not resolve on its + * own, so retrying the upgrade sequence cannot succeed. Derived from the + * cause (see {@link TerminalUpgradeError}). + */ + readonly terminal: boolean; + constructor(step: string, cause: unknown) { const causeMessage = cause instanceof Error ? cause.message : String(cause); super(`Money Account upgrade failed at step "${step}": ${causeMessage}`); @@ -21,6 +28,27 @@ export class MoneyAccountUpgradeStepError extends Error { this.name = 'MoneyAccountUpgradeStepError'; this.step = step; this.cause = cause; + this.terminal = + cause instanceof Error && + (cause as { terminal?: unknown }).terminal === true; + } +} + +/** + * Error a step throws to mark a failure as terminal: the condition will not + * resolve on its own, so retrying the upgrade sequence is pointless — e.g. + * the account is already delegated to a third-party implementation. + * + * Detected structurally via the `terminal` property (rather than + * `instanceof`) so the marking survives module-realm duplication. + */ +export class TerminalUpgradeError extends Error { + /** Marks the failure as not retryable. */ + readonly terminal = true; + + constructor(message: string) { + super(message); + this.name = 'TerminalUpgradeError'; } } @@ -43,3 +71,23 @@ export function isMoneyAccountUpgradeStepError( typeof (error as { step?: unknown }).step === 'string' ); } + +/** + * Whether `error` is a {@link MoneyAccountUpgradeStepError} marked as + * terminal — a failure that will not resolve on its own, so retrying the + * upgrade sequence cannot succeed. + * + * Uses the same structural checks as {@link isMoneyAccountUpgradeStepError} + * so it holds across module realm boundaries. + * + * @param error - The value to test. + * @returns Whether `error` is a terminal `MoneyAccountUpgradeStepError`. + */ +export function isTerminalMoneyAccountUpgradeError( + error: unknown, +): error is MoneyAccountUpgradeStepError { + return ( + isMoneyAccountUpgradeStepError(error) && + (error as { terminal?: unknown }).terminal === true + ); +} diff --git a/packages/money-account-upgrade-controller/src/index.ts b/packages/money-account-upgrade-controller/src/index.ts index 784e62cfd5..3b64f38a2b 100644 --- a/packages/money-account-upgrade-controller/src/index.ts +++ b/packages/money-account-upgrade-controller/src/index.ts @@ -1,9 +1,14 @@ export type { UpgradeConfig } from './types'; export { MoneyAccountUpgradeStepError, + TerminalUpgradeError, isMoneyAccountUpgradeStepError, + isTerminalMoneyAccountUpgradeError, } from './errors'; -export { MoneyAccountUpgradeController } from './MoneyAccountUpgradeController'; +export { + MoneyAccountUpgradeController, + getDefaultMoneyAccountUpgradeControllerState, +} from './MoneyAccountUpgradeController'; export type { MoneyAccountUpgradeControllerState, MoneyAccountUpgradeControllerGetStateAction, @@ -11,5 +16,9 @@ export type { MoneyAccountUpgradeControllerStateChangedEvent, MoneyAccountUpgradeControllerEvents, MoneyAccountUpgradeControllerMessenger, + MoneyAccountUpgradeStatus, } from './MoneyAccountUpgradeController'; -export type { MoneyAccountUpgradeControllerUpgradeAccountAction } from './MoneyAccountUpgradeController-method-action-types'; +export type { + MoneyAccountUpgradeControllerUpgradeAccountAction, + MoneyAccountUpgradeControllerUpgradeAccountWithRetryAction, +} from './MoneyAccountUpgradeController-method-action-types'; diff --git a/packages/money-account-upgrade-controller/src/steps/eip-7702-authorization.test.ts b/packages/money-account-upgrade-controller/src/steps/eip-7702-authorization.test.ts index e1c179fc8f..24de94126e 100644 --- a/packages/money-account-upgrade-controller/src/steps/eip-7702-authorization.test.ts +++ b/packages/money-account-upgrade-controller/src/steps/eip-7702-authorization.test.ts @@ -202,6 +202,13 @@ describe('eip7702AuthorizationStep', () => { expect(mocks.signEip7702Authorization).not.toHaveBeenCalled(); expect(mocks.createUpgrade).not.toHaveBeenCalled(); }); + + it('marks the failure as terminal', async () => { + const { messenger, mocks } = setup(); + configureProvider(mocks, delegationCode(MOCK_THIRD_PARTY_IMPL)); + + await expect(run(messenger)).rejects.toMatchObject({ terminal: true }); + }); }); describe('when the account has unexpected non-delegation code', () => { @@ -217,6 +224,13 @@ describe('eip7702AuthorizationStep', () => { expect(mocks.createUpgrade).not.toHaveBeenCalled(); }); + it('marks the failure as terminal', async () => { + const { messenger, mocks } = setup(); + configureProvider(mocks, '0x6080604052' as Hex); + + await expect(run(messenger)).rejects.toMatchObject({ terminal: true }); + }); + it('throws when eth_getCode returns a non-hex value', async () => { const { messenger, mocks } = setup(); mocks.providerRequest.mockImplementation(async ({ method }) => { diff --git a/packages/money-account-upgrade-controller/src/steps/eip-7702-authorization.ts b/packages/money-account-upgrade-controller/src/steps/eip-7702-authorization.ts index f494a67945..1c217c17d6 100644 --- a/packages/money-account-upgrade-controller/src/steps/eip-7702-authorization.ts +++ b/packages/money-account-upgrade-controller/src/steps/eip-7702-authorization.ts @@ -2,6 +2,7 @@ import type { Provider } from '@metamask/network-controller'; import { add0x, isStrictHexString } from '@metamask/utils'; import type { Hex } from '@metamask/utils'; +import { TerminalUpgradeError } from '../errors'; import type { Step, StepContext } from './step'; const EIP_7702_DELEGATION_PREFIX = '0xef0100'; @@ -47,7 +48,7 @@ export const eip7702AuthorizationStep: Step = { if (existingDelegation === delegatorImplAddress.toLowerCase()) { return 'already-done'; } - throw new Error( + throw new TerminalUpgradeError( `Account ${address} is already upgraded to another smart account: ${existingDelegation}.`, ); } @@ -184,7 +185,7 @@ async function fetchDelegationAddress( return add0x(normalized.slice(EIP_7702_DELEGATION_PREFIX.length)); } - throw new Error( + throw new TerminalUpgradeError( `Account ${address} has unexpected on-chain code; expected either no code or an EIP-7702 delegation.`, ); }