diff --git a/README.md b/README.md index 27f8d326fe2..4efea42c40f 100644 --- a/README.md +++ b/README.md @@ -451,7 +451,9 @@ linkStyle default opacity:0.5 money_account_upgrade_controller --> delegation_controller; money_account_upgrade_controller --> keyring_controller; money_account_upgrade_controller --> messenger; + money_account_upgrade_controller --> money_account_utils; money_account_upgrade_controller --> network_controller; + money_account_upgrade_controller --> remote_feature_flag_controller; money_account_utils --> transaction_controller; multichain_account_service --> accounts_controller; multichain_account_service --> base_controller; diff --git a/packages/money-account-upgrade-controller/CHANGELOG.md b/packages/money-account-upgrade-controller/CHANGELOG.md index d9fe3406497..bfeb007c927 100644 --- a/packages/money-account-upgrade-controller/CHANGELOG.md +++ b/packages/money-account-upgrade-controller/CHANGELOG.md @@ -7,8 +7,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add a public `sync()` method that re-evaluates the bootstrap gates against live state, for clients whose `isEnabled` hook reads client-only signals (onboarding, preferences) that should also re-trigger the bootstrap ([#10072](https://github.com/MetaMask/core/pull/10072)) +- Add `MissingMoneyAccountVaultConfigError`, reported through the `onBootstrapError` hook (once per controller lifetime) when the enable flag is on but `moneyAccountVaultConfig` is unserved or malformed ([#10072](https://github.com/MetaMask/core/pull/10072)) + ### Changed +- **BREAKING:** The controller now owns its bootstrap: it subscribes to `RemoteFeatureFlagController:stateChanged` and `KeyringController:stateChanged`, gates on an unlocked wallet with an HD keyring, parses the `moneyAccountVaultConfig` remote feature flag, and runs a serialized bootstrap that re-checks its gates across `await` points and re-runs when the vault config changes ([#10072](https://github.com/MetaMask/core/pull/10072)) + - `init()` is now the no-argument lifecycle entry point (subscribe and first sync), to be called once after all controllers and services the messenger reaches are constructed. The former `init({ chainId, boringVaultAddress })` config-arming routine is internal and reads the chain id and boring vault from the flag; clients that called it must remove that orchestration. + - The constructor requires a `hooks` option carrying the client-specific parts of the bootstrap: `isEnabled(remoteFeatureFlags)` (required — version-gated flag evaluation depends on the client version, and clients may add gates such as a basic-functionality toggle), plus optional `isEligible()` (an async pre-bootstrap gate, e.g. fail-closed geolocation), `ensureChainConfigured(vaultConfig)` (client-specific network adding), and `onBootstrapError(error)`. + - The messenger must now allow the `RemoteFeatureFlagController:getState` and `KeyringController:getState` actions and the `RemoteFeatureFlagController:stateChanged` and `KeyringController:stateChanged` events. +- **BREAKING:** `upgradeAccount()` now waits for the in-flight bootstrap chain to settle (including runs scheduled while waiting) instead of throwing, and throws a new not-bootstrapped error message when no bootstrap has armed a config or the wallet is locked. Scheduling a bootstrap for a changed vault config — or `isEnabled` flipping off — disarms the previous config, so an upgrade can never sign against a superseded vault, including after a failed re-bootstrap ([#10072](https://github.com/MetaMask/core/pull/10072)) + - The armed config is re-checked before every step; if it is disarmed or superseded while the sequence is running, `upgradeAccount()` throws an upgrade-aborted error before the next step signs anything and records nothing + - An `onBootstrapError` hook that throws is contained: the failed bootstrap is still forgotten and retried on the next trigger, and the throw does not escape `init()` or `sync()` +- Add `@metamask/money-account-utils` and `@metamask/remote-feature-flag-controller` as dependencies ([#10072](https://github.com/MetaMask/core/pull/10072)) - Bump `@metamask/authenticated-user-storage` from `^3.0.1` to `^3.0.2` ([#9972](https://github.com/MetaMask/core/pull/9972)) - Bump `@metamask/chomp-api-service` from `^4.0.0` to `^4.0.1` ([#9972](https://github.com/MetaMask/core/pull/9972)) - Bump `@metamask/utils` from `^11.11.0` to `^11.12.0` ([#10076](https://github.com/MetaMask/core/pull/10076)) diff --git a/packages/money-account-upgrade-controller/package.json b/packages/money-account-upgrade-controller/package.json index a1cb6ce7539..8ecb848d0b7 100644 --- a/packages/money-account-upgrade-controller/package.json +++ b/packages/money-account-upgrade-controller/package.json @@ -63,7 +63,9 @@ "@metamask/delegation-deployments": "^1.4.0", "@metamask/keyring-controller": "^27.1.1", "@metamask/messenger": "^2.0.0", + "@metamask/money-account-utils": "^1.1.0", "@metamask/network-controller": "^36.0.0", + "@metamask/remote-feature-flag-controller": "^6.1.0", "@metamask/utils": "^11.12.0" }, "devDependencies": { 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 7aa0061afdc..c302b2fe05e 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 @@ -19,6 +19,17 @@ import type { MoneyAccountUpgradeController } from './MoneyAccountUpgradeControl * active config no longer matches the recorded fingerprint, the sequence * re-runs. * + * A call that arrives while the bootstrap chain is still in flight — + * including runs scheduled while waiting — waits for it to settle rather + * than failing, so the upgrade always runs against the latest armed + * config. Scheduling a bootstrap for a changed vault config disarms the + * previous one, so it throws when no bootstrap has armed a config (feature + * disabled or the last bootstrap failed) or when the wallet is locked. + * + * The armed config is re-checked before every step: if a sync disarms or + * supersedes it while the sequence is running, the sequence aborts before + * the next step signs anything, and nothing is recorded. + * * @param address - The Money Account address to upgrade. */ export type MoneyAccountUpgradeControllerUpgradeAccountAction = { diff --git a/packages/money-account-upgrade-controller/src/MoneyAccountUpgradeController.test.ts b/packages/money-account-upgrade-controller/src/MoneyAccountUpgradeController.test.ts index 50f9932e884..2ff79762030 100644 --- a/packages/money-account-upgrade-controller/src/MoneyAccountUpgradeController.test.ts +++ b/packages/money-account-upgrade-controller/src/MoneyAccountUpgradeController.test.ts @@ -1,10 +1,13 @@ import { DELEGATOR_CONTRACTS } from '@metamask/delegation-deployments'; +import type { KeyringControllerState } from '@metamask/keyring-controller'; import { Messenger, MOCK_ANY_NAMESPACE } from '@metamask/messenger'; import type { MockAnyNamespace, MessengerActions, MessengerEvents, } from '@metamask/messenger'; +import type { MoneyAccountVaultConfig } from '@metamask/money-account-utils'; +import type { RemoteFeatureFlagControllerState } from '@metamask/remote-feature-flag-controller'; import { hexToNumber } from '@metamask/utils'; import type { Hex } from '@metamask/utils'; @@ -14,6 +17,7 @@ import type { MoneyAccountUpgradeStepError, } from './index.js'; import { + MissingMoneyAccountVaultConfigError, MoneyAccountUpgradeController, getDefaultMoneyAccountUpgradeControllerState, isMoneyAccountUpgradeStepError, @@ -27,6 +31,22 @@ const MOCK_ACCOUNT_ADDRESS = const MOCK_BORING_VAULT_ADDRESS = '0xA20f97813014129E7609171d2D3AA3da5206259e' as Hex; +const VAULT_CONFIG: MoneyAccountVaultConfig = { + chainId: MOCK_CHAIN_ID, + boringVault: MOCK_BORING_VAULT_ADDRESS, + tellerAddress: '0x2D49EA58A4C70b62c8B56DE971310d9e999c8117', + accountantAddress: '0x7382c5b8B51B8C4f127B3123C1039581BAA5A06B', + lensAddress: '0xA816ECd922de94c6879AD23B9A884dB257F20947', + underlyingToken: '0xacA92E438df0B2401fF60dA7E4337B687a2435DA', +}; + +// The same vault, but published under a fresher flag payload with a different +// vmUSD token address — must be treated as a config change. +const CHANGED_VAULT_CONFIG: MoneyAccountVaultConfig = { + ...VAULT_CONFIG, + underlyingToken: '0x1111111111111111111111111111111111111111', +}; + // CHOMP-API-derived values. const MOCK_DELEGATE_ADDRESS = '0x1111111111111111111111111111111111111111' as Hex; @@ -37,8 +57,8 @@ const MOCK_VEDA_VAULT_ADAPTER_ADDRESS = // Delegation Framework deployment for mainnet @ 1.3.0 — the controller resolves // these from `@metamask/delegation-deployments` rather than accepting them via -// `init()`. We re-read from the same source here so the test does not drift if -// the deployment registry is bumped. +// the vault config. We re-read from the same source here so the test does not +// drift if the deployment registry is bumped. const MAINNET_CONTRACTS = DELEGATOR_CONTRACTS['1.3.0'][hexToNumber(MOCK_CHAIN_ID)]; @@ -69,6 +89,13 @@ type AllEvents = MessengerEvents; type RootMessenger = Messenger; +/** + * Flush the microtask queue so scheduled bootstraps settle. + */ +const flushPromises = async (): Promise => { + await new Promise((resolve) => setImmediate(resolve)); +}; + type Mocks = { getServiceDetails: jest.Mock; signPersonalMessage: jest.Mock; @@ -85,18 +112,58 @@ type Mocks = { verifyDelegation: jest.Mock; getIntentsByAddress: jest.Mock; createIntents: jest.Mock; + isEnabled: jest.Mock; + isEligible: jest.Mock; + ensureChainConfigured: jest.Mock; + onBootstrapError: jest.Mock; +}; + +/** + * The mutable gate state the messenger and hook mocks read on every call, so + * tests can flip a gate and re-trigger a sync. + */ +type GateConfig = { + isEnabled: boolean; + isUnlocked: boolean; + hasHdKeyring: boolean; + isEligible: boolean; + vaultConfig: unknown; }; function setup({ state, + isEnabled = true, + isUnlocked = true, + hasHdKeyring = true, + isEligible = true, + vaultConfig = VAULT_CONFIG, + withOptionalHooks = true, }: { state?: Partial; + isEnabled?: boolean; + isUnlocked?: boolean; + hasHdKeyring?: boolean; + isEligible?: boolean; + vaultConfig?: unknown; + withOptionalHooks?: boolean; } = {}): { controller: MoneyAccountUpgradeController; rootMessenger: RootMessenger; messenger: MoneyAccountUpgradeControllerMessenger; mocks: Mocks; + config: GateConfig; + bootstrap: () => Promise; + triggerFlagChange: () => Promise; + triggerKeyringChange: () => Promise; } { + const config: GateConfig = { + isEnabled, + isUnlocked, + hasHdKeyring, + isEligible, + vaultConfig, + }; + // 65-byte signature — r (32 bytes) + s (32 bytes) + v = 0x1c (28). const signature = `0x${'1'.repeat(64)}${'2'.repeat(64)}1c`; @@ -146,12 +213,37 @@ function setup({ verifyDelegation: jest.fn().mockResolvedValue({ valid: true }), getIntentsByAddress: jest.fn().mockResolvedValue([]), createIntents: jest.fn().mockResolvedValue([]), + isEnabled: jest.fn().mockImplementation(() => config.isEnabled), + isEligible: jest.fn().mockImplementation(async () => config.isEligible), + ensureChainConfigured: jest.fn().mockResolvedValue(undefined), + onBootstrapError: jest.fn(), }; const rootMessenger = new Messenger({ namespace: MOCK_ANY_NAMESPACE, }); + rootMessenger.registerActionHandler( + 'RemoteFeatureFlagController:getState', + (): RemoteFeatureFlagControllerState => + ({ + remoteFeatureFlags: { + moneyAccountVaultConfig: config.vaultConfig, + }, + cacheTimestamp: 0, + }) as RemoteFeatureFlagControllerState, + ); + rootMessenger.registerActionHandler( + 'KeyringController:getState', + (): KeyringControllerState => + ({ + isUnlocked: config.isUnlocked, + keyrings: + config.isUnlocked && config.hasHdKeyring + ? [{ type: 'HD Key Tree', accounts: [], metadata: { id: 'hd' } }] + : [], + }) as unknown as KeyringControllerState, + ); rootMessenger.registerActionHandler( 'ChompApiService:getServiceDetails', mocks.getServiceDetails, @@ -217,6 +309,7 @@ function setup({ rootMessenger.delegate({ actions: [ 'ChompApiService:getServiceDetails', + 'KeyringController:getState', 'KeyringController:signPersonalMessage', 'ChompApiService:associateAddress', 'ChompApiService:getAssociatedAddresses', @@ -230,17 +323,61 @@ function setup({ 'ChompApiService:verifyDelegation', 'ChompApiService:getIntentsByAddress', 'ChompApiService:createIntents', + 'RemoteFeatureFlagController:getState', + ], + events: [ + 'KeyringController:stateChanged', + 'RemoteFeatureFlagController:stateChanged', ], - events: [], messenger, }); const controller = new MoneyAccountUpgradeController({ messenger, state, + hooks: withOptionalHooks + ? { + isEnabled: mocks.isEnabled, + isEligible: mocks.isEligible, + ensureChainConfigured: mocks.ensureChainConfigured, + onBootstrapError: mocks.onBootstrapError, + } + : { isEnabled: mocks.isEnabled }, }); - return { controller, rootMessenger, messenger, mocks }; + const bootstrap = async (): Promise => { + controller.init(); + await flushPromises(); + }; + + const triggerFlagChange = async (): Promise => { + rootMessenger.publish( + 'RemoteFeatureFlagController:stateChanged', + {} as RemoteFeatureFlagControllerState, + [], + ); + await flushPromises(); + }; + + const triggerKeyringChange = async (): Promise => { + rootMessenger.publish( + 'KeyringController:stateChanged', + {} as KeyringControllerState, + [], + ); + await flushPromises(); + }; + + return { + controller, + rootMessenger, + messenger, + mocks, + config, + bootstrap, + triggerFlagChange, + triggerKeyringChange, + }; } /** @@ -258,10 +395,11 @@ function clearMockCalls(mocks: Mocks): void { describe('MoneyAccountUpgradeController', () => { describe('constructor', () => { - it('does not make async init calls when constructed', () => { + it('makes no messenger calls before init()', () => { const { mocks } = setup(); expect(mocks.getServiceDetails).not.toHaveBeenCalled(); + expect(mocks.isEnabled).not.toHaveBeenCalled(); }); it('starts with the default empty state', () => { @@ -286,75 +424,465 @@ describe('MoneyAccountUpgradeController', () => { }); }); - describe('init', () => { - it('fetches service details and builds config', async () => { - const { controller, mocks } = setup(); + describe('bootstrap gating', () => { + it('bootstraps at init when the gates are open', async () => { + const { mocks, bootstrap } = setup(); - await controller.init({ - chainId: MOCK_CHAIN_ID, - boringVaultAddress: MOCK_BORING_VAULT_ADDRESS, - }); + await bootstrap(); expect(mocks.getServiceDetails).toHaveBeenCalledWith([MOCK_CHAIN_ID]); }); - it('throws when the chain has no Delegation Framework deployment', async () => { - const { controller, mocks } = setup(); + it('configures the chain before fetching service details', async () => { + const order: string[] = []; + const { mocks, bootstrap } = setup(); + mocks.ensureChainConfigured.mockImplementation(async () => { + order.push('ensureChainConfigured'); + }); + mocks.getServiceDetails.mockImplementation(async () => { + order.push('getServiceDetails'); + return MOCK_SERVICE_DETAILS_RESPONSE; + }); - await expect( - controller.init({ - chainId: UNSUPPORTED_CHAIN_ID, - boringVaultAddress: MOCK_BORING_VAULT_ADDRESS, + await bootstrap(); + + expect(order).toStrictEqual([ + 'ensureChainConfigured', + 'getServiceDetails', + ]); + expect(mocks.ensureChainConfigured).toHaveBeenCalledWith(VAULT_CONFIG); + }); + + it('is idempotent: a second init() does not re-subscribe or re-bootstrap', async () => { + const { controller, mocks, bootstrap, triggerFlagChange } = setup(); + await bootstrap(); + const syncsAfterFirstInit = mocks.isEnabled.mock.calls.length; + + controller.init(); + await flushPromises(); + expect(mocks.isEnabled).toHaveBeenCalledTimes(syncsAfterFirstInit); + + await triggerFlagChange(); + expect(mocks.isEnabled).toHaveBeenCalledTimes(syncsAfterFirstInit + 1); + expect(mocks.getServiceDetails).toHaveBeenCalledTimes(1); + }); + + it('does not bootstrap when the isEnabled hook returns false', async () => { + const { mocks, bootstrap } = setup({ isEnabled: false }); + + await bootstrap(); + + expect(mocks.getServiceDetails).not.toHaveBeenCalled(); + }); + + it('passes the current remote feature flags to the isEnabled hook', async () => { + const { mocks, bootstrap } = setup(); + + await bootstrap(); + + expect(mocks.isEnabled).toHaveBeenCalledWith( + expect.objectContaining({ + moneyAccountVaultConfig: VAULT_CONFIG, }), - ).rejects.toThrow( - `Delegation Framework 1.3.0 is not deployed on chain ${UNSUPPORTED_CHAIN_ID}`, ); + }); + + it('does not bootstrap while the wallet is locked', async () => { + const { mocks, bootstrap } = setup({ isUnlocked: false }); + + await bootstrap(); + expect(mocks.getServiceDetails).not.toHaveBeenCalled(); }); - it('uses the supplied boring vault address as the withdrawal-side delegation token', async () => { - const { controller, mocks } = setup(); + it('does not bootstrap while the keyring list has no HD keyring', async () => { + const { mocks, bootstrap } = setup({ hasHdKeyring: false }); + + await bootstrap(); + + expect(mocks.getServiceDetails).not.toHaveBeenCalled(); + }); - await controller.init({ - chainId: MOCK_CHAIN_ID, - boringVaultAddress: MOCK_BORING_VAULT_ADDRESS, + it('bootstraps on unlock via the keyring state change', async () => { + const { config, mocks, bootstrap, triggerKeyringChange } = setup({ + isUnlocked: false, }); - await controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS); + await bootstrap(); + expect(mocks.getServiceDetails).not.toHaveBeenCalled(); - // Both delegations were signed; the boring-vault address shows up in the - // ABI-encoded ERC20TransferAmount caveat terms of one of them. - expect(mocks.signDelegation).toHaveBeenCalledTimes(2); - const allCaveatTerms = mocks.verifyDelegation.mock.calls - .flatMap(([{ signedDelegation }]) => signedDelegation.caveats) - .map((caveat) => caveat.terms.toLowerCase()); + config.isUnlocked = true; + await triggerKeyringChange(); + + expect(mocks.getServiceDetails).toHaveBeenCalledTimes(1); + }); + + it('bootstraps when the isEnabled hook flips to true on a flag change', async () => { + const { config, mocks, bootstrap, triggerFlagChange } = setup({ + isEnabled: false, + }); + await bootstrap(); + expect(mocks.getServiceDetails).not.toHaveBeenCalled(); + + config.isEnabled = true; + await triggerFlagChange(); + + expect(mocks.getServiceDetails).toHaveBeenCalledTimes(1); + }); + + it('bootstraps when an external sync() reports a client gate reopened', async () => { + const { config, controller, mocks, bootstrap } = setup({ + isEnabled: false, + }); + await bootstrap(); + expect(mocks.getServiceDetails).not.toHaveBeenCalled(); + + config.isEnabled = true; + controller.sync(); + await flushPromises(); + + expect(mocks.getServiceDetails).toHaveBeenCalledTimes(1); + }); + + it('does not bootstrap while ineligible, then bootstraps once eligible', async () => { + const { config, mocks, bootstrap, triggerFlagChange } = setup({ + isEligible: false, + }); + await bootstrap(); + + expect(mocks.ensureChainConfigured).not.toHaveBeenCalled(); + expect(mocks.getServiceDetails).not.toHaveBeenCalled(); + + config.isEligible = true; + await triggerFlagChange(); + + expect(mocks.ensureChainConfigured).toHaveBeenCalledTimes(1); + expect(mocks.getServiceDetails).toHaveBeenCalledTimes(1); + }); + + it('skips the CHOMP call when the wallet locks while the chain is being configured', async () => { + let resolveEnsure: (value?: unknown) => void = () => undefined; + const { config, mocks, bootstrap, triggerKeyringChange } = setup(); + mocks.ensureChainConfigured + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveEnsure = resolve; + }), + ) + .mockResolvedValue(undefined); + + await bootstrap(); + expect(mocks.ensureChainConfigured).toHaveBeenCalledTimes(1); + + config.isUnlocked = false; + resolveEnsure(); + await flushPromises(); + + expect(mocks.getServiceDetails).not.toHaveBeenCalled(); + + config.isUnlocked = true; + await triggerKeyringChange(); + + expect(mocks.getServiceDetails).toHaveBeenCalledTimes(1); + }); + + it('skips the CHOMP call when isEnabled flips off while the chain is being configured', async () => { + let resolveEnsure: (value?: unknown) => void = () => undefined; + const { config, mocks, bootstrap, triggerFlagChange } = setup(); + mocks.ensureChainConfigured + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveEnsure = resolve; + }), + ) + .mockResolvedValue(undefined); + + await bootstrap(); + + config.isEnabled = false; + resolveEnsure(); + await flushPromises(); + + expect(mocks.getServiceDetails).not.toHaveBeenCalled(); + + config.isEnabled = true; + await triggerFlagChange(); + + expect(mocks.getServiceDetails).toHaveBeenCalledTimes(1); + }); + + it('skips a chained run entirely when the wallet locks before it starts', async () => { + let resolveServiceDetails: (value?: unknown) => void = () => undefined; + const { + config, + mocks, + bootstrap, + triggerFlagChange, + triggerKeyringChange, + } = setup(); + mocks.getServiceDetails + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveServiceDetails = resolve; + }), + ) + .mockResolvedValue(MOCK_SERVICE_DETAILS_RESPONSE); + await bootstrap(); + + config.vaultConfig = CHANGED_VAULT_CONFIG; + await triggerFlagChange(); + config.isUnlocked = false; + await triggerKeyringChange(); + resolveServiceDetails(MOCK_SERVICE_DETAILS_RESPONSE); + await flushPromises(); + + expect(mocks.isEligible).toHaveBeenCalledTimes(1); + expect(mocks.ensureChainConfigured).toHaveBeenCalledTimes(1); + expect(mocks.getServiceDetails).toHaveBeenCalledTimes(1); + + config.isUnlocked = true; + await triggerKeyringChange(); + + expect(mocks.ensureChainConfigured).toHaveBeenLastCalledWith( + CHANGED_VAULT_CONFIG, + ); + expect(mocks.getServiceDetails).toHaveBeenCalledTimes(2); + }); + + it('reports a missing vault config through onBootstrapError only once', async () => { + const { mocks, bootstrap, triggerFlagChange } = setup({ + vaultConfig: null, + }); + await bootstrap(); + await triggerFlagChange(); + await triggerFlagChange(); + + expect(mocks.getServiceDetails).not.toHaveBeenCalled(); + expect(mocks.onBootstrapError).toHaveBeenCalledTimes(1); + expect(mocks.onBootstrapError).toHaveBeenCalledWith( + expect.any(MissingMoneyAccountVaultConfigError), + ); + }); + + it('reports a malformed vault config the same as a missing one', async () => { + const { mocks, bootstrap } = setup({ + vaultConfig: { ...VAULT_CONFIG, chainId: 'not-hex' }, + }); + await bootstrap(); + + expect(mocks.onBootstrapError).toHaveBeenCalledWith( + expect.any(MissingMoneyAccountVaultConfigError), + ); + }); + + it('does not re-bootstrap when triggers repeat with the same config', async () => { + const { mocks, bootstrap, triggerFlagChange, triggerKeyringChange } = + setup(); + await bootstrap(); + await triggerFlagChange(); + await triggerKeyringChange(); + + expect(mocks.getServiceDetails).toHaveBeenCalledTimes(1); + }); + + it('re-bootstraps when the vault config changes', async () => { + const { config, mocks, bootstrap, triggerFlagChange } = setup(); + await bootstrap(); + + config.vaultConfig = CHANGED_VAULT_CONFIG; + await triggerFlagChange(); + + expect(mocks.getServiceDetails).toHaveBeenCalledTimes(2); + }); + + it('serializes a config-change bootstrap after the in-flight one', async () => { + let resolveFirst: (value?: unknown) => void = () => undefined; + const { config, mocks, bootstrap, triggerFlagChange } = setup(); + mocks.getServiceDetails + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveFirst = resolve; + }), + ) + .mockResolvedValue(MOCK_SERVICE_DETAILS_RESPONSE); + + await bootstrap(); + config.vaultConfig = CHANGED_VAULT_CONFIG; + await triggerFlagChange(); + expect(mocks.getServiceDetails).toHaveBeenCalledTimes(1); + + resolveFirst(MOCK_SERVICE_DETAILS_RESPONSE); + await flushPromises(); + + expect(mocks.getServiceDetails).toHaveBeenCalledTimes(2); + }); + + it('keeps a newer scheduled config when the superseded bootstrap fails', async () => { + let rejectFirst: (error: Error) => void = () => undefined; + const { config, mocks, bootstrap, triggerFlagChange } = setup(); + mocks.getServiceDetails + .mockImplementationOnce( + () => + new Promise((_resolve, reject) => { + rejectFirst = reject; + }), + ) + .mockResolvedValue(MOCK_SERVICE_DETAILS_RESPONSE); + + await bootstrap(); + config.vaultConfig = CHANGED_VAULT_CONFIG; + await triggerFlagChange(); + + rejectFirst(new Error('CHOMP outage')); + await flushPromises(); + expect(mocks.getServiceDetails).toHaveBeenCalledTimes(2); + + // The failure of the superseded run must not forget the newer config: + // a repeat trigger with the same config schedules nothing new. + await triggerFlagChange(); + expect(mocks.getServiceDetails).toHaveBeenCalledTimes(2); + }); + + it('retries a failed bootstrap on the next trigger and reports the failure', async () => { + const { mocks, bootstrap, triggerKeyringChange } = setup(); + const failure = new Error('CHOMP outage'); + mocks.getServiceDetails + .mockRejectedValueOnce(failure) + .mockResolvedValue(MOCK_SERVICE_DETAILS_RESPONSE); + + await bootstrap(); + await triggerKeyringChange(); + + expect(mocks.getServiceDetails).toHaveBeenCalledTimes(2); + expect(mocks.onBootstrapError).toHaveBeenCalledWith(failure); + }); + + it('reports a bootstrap failure from the chain configuration hook', async () => { + const { mocks, bootstrap } = setup(); + const failure = new Error('addNetwork failed'); + mocks.ensureChainConfigured.mockRejectedValueOnce(failure); + + await bootstrap(); + + expect(mocks.onBootstrapError).toHaveBeenCalledWith(failure); + expect(mocks.getServiceDetails).not.toHaveBeenCalled(); + }); + + it('bootstraps with only the required isEnabled hook, defaulting the others', async () => { + const { controller, mocks, bootstrap } = setup({ + withOptionalHooks: false, + }); + + await bootstrap(); + + expect(mocks.getServiceDetails).toHaveBeenCalledTimes(1); expect( - allCaveatTerms.some((terms) => - terms.includes(MOCK_BORING_VAULT_ADDRESS.toLowerCase().slice(2)), - ), - ).toBe(true); + await controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS), + ).toBeUndefined(); }); - it('throws when the chain is not found in service details', async () => { - const { controller, mocks } = setup(); + it('swallows a bootstrap failure when no onBootstrapError hook is given', async () => { + const { controller, mocks, bootstrap } = setup({ + withOptionalHooks: false, + }); + mocks.getServiceDetails.mockRejectedValueOnce(new Error('CHOMP outage')); + await bootstrap(); + + await expect( + controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS), + ).rejects.toThrow('MoneyAccountUpgradeController is not bootstrapped'); + }); + + it('swallows a missing vault config when no onBootstrapError hook is given', async () => { + const { mocks, bootstrap } = setup({ + withOptionalHooks: false, + vaultConfig: null, + }); + + await bootstrap(); + + expect(mocks.getServiceDetails).not.toHaveBeenCalled(); + }); + + it('survives a throwing messenger call during sync', async () => { + const { controller, mocks, bootstrap } = setup(); + await bootstrap(); + const failure = new Error('handler not registered'); + mocks.isEnabled.mockImplementationOnce(() => { + throw failure; + }); + + expect(() => controller.sync()).not.toThrow(); + expect(mocks.onBootstrapError).toHaveBeenCalledWith(failure); + }); + + it('still retries a failed bootstrap when the onBootstrapError hook throws', async () => { + const { mocks, bootstrap, triggerKeyringChange } = setup(); + mocks.getServiceDetails + .mockRejectedValueOnce(new Error('CHOMP outage')) + .mockResolvedValue(MOCK_SERVICE_DETAILS_RESPONSE); + mocks.onBootstrapError.mockImplementationOnce(() => { + throw new Error('reporter is broken'); + }); + + await bootstrap(); + await triggerKeyringChange(); + + expect(mocks.getServiceDetails).toHaveBeenCalledTimes(2); + }); + + it('does not let a throwing onBootstrapError hook escape sync', async () => { + const { controller, mocks, bootstrap } = setup(); + await bootstrap(); + mocks.isEnabled.mockImplementationOnce(() => { + throw new Error('handler not registered'); + }); + mocks.onBootstrapError.mockImplementationOnce(() => { + throw new Error('reporter is broken'); + }); + + expect(() => controller.sync()).not.toThrow(); + }); + }); + + describe('bootstrap failures', () => { + it('reports when the chain has no Delegation Framework deployment', async () => { + const { mocks, bootstrap } = setup({ + vaultConfig: { ...VAULT_CONFIG, chainId: UNSUPPORTED_CHAIN_ID }, + }); + + await bootstrap(); + + expect(mocks.onBootstrapError).toHaveBeenCalledWith( + new Error( + `Delegation Framework 1.3.0 is not deployed on chain ${UNSUPPORTED_CHAIN_ID}`, + ), + ); + expect(mocks.getServiceDetails).not.toHaveBeenCalled(); + }); + + it('reports when the chain is not found in service details', async () => { + const { mocks, bootstrap } = setup(); mocks.getServiceDetails.mockResolvedValue({ auth: { message: 'CHOMP Authentication' }, chains: {}, }); - await expect( - controller.init({ - chainId: MOCK_CHAIN_ID, - boringVaultAddress: MOCK_BORING_VAULT_ADDRESS, - }), - ).rejects.toThrow( - `Chain ${MOCK_CHAIN_ID} not found in service details response`, + await bootstrap(); + + expect(mocks.onBootstrapError).toHaveBeenCalledWith( + new Error( + `Chain ${MOCK_CHAIN_ID} not found in service details response`, + ), ); }); - it('throws when vedaProtocol is not found', async () => { - const { controller, mocks } = setup(); - + it('reports when vedaProtocol is not found', async () => { + const { mocks, bootstrap } = setup(); mocks.getServiceDetails.mockResolvedValue({ auth: { message: 'CHOMP Authentication' }, chains: { @@ -365,19 +893,17 @@ describe('MoneyAccountUpgradeController', () => { }, }); - await expect( - controller.init({ - chainId: MOCK_CHAIN_ID, - boringVaultAddress: MOCK_BORING_VAULT_ADDRESS, - }), - ).rejects.toThrow( - `vedaProtocol not found for chain ${MOCK_CHAIN_ID} in service details response`, + await bootstrap(); + + expect(mocks.onBootstrapError).toHaveBeenCalledWith( + new Error( + `vedaProtocol not found for chain ${MOCK_CHAIN_ID} in service details response`, + ), ); }); - it('throws when supportedTokens is empty', async () => { - const { controller, mocks } = setup(); - + it('reports when supportedTokens is empty', async () => { + const { mocks, bootstrap } = setup(); mocks.getServiceDetails.mockResolvedValue({ auth: { message: 'CHOMP Authentication' }, chains: { @@ -394,55 +920,257 @@ describe('MoneyAccountUpgradeController', () => { }, }); - await expect( - controller.init({ - chainId: MOCK_CHAIN_ID, - boringVaultAddress: MOCK_BORING_VAULT_ADDRESS, - }), - ).rejects.toThrow( - `No supported tokens found for vedaProtocol on chain ${MOCK_CHAIN_ID}`, + await bootstrap(); + + expect(mocks.onBootstrapError).toHaveBeenCalledWith( + new Error( + `No supported tokens found for vedaProtocol on chain ${MOCK_CHAIN_ID}`, + ), ); }); }); describe('upgradeAccount', () => { - it('throws when called before init', async () => { - const { controller } = setup(); + it('throws when no bootstrap has been scheduled', async () => { + const { controller } = setup({ isEnabled: false }); + controller.init(); await expect( controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS), - ).rejects.toThrow( - 'MoneyAccountUpgradeController must be initialized via init() before upgradeAccount() can be called', - ); + ).rejects.toThrow('MoneyAccountUpgradeController is not bootstrapped'); }); - it('throws when a previous init attempt failed', async () => { - const { controller, mocks } = setup(); - mocks.getServiceDetails.mockResolvedValueOnce({ + it('throws when the bootstrap failed', async () => { + const { controller, mocks, bootstrap } = setup(); + mocks.getServiceDetails.mockResolvedValue({ auth: { message: 'CHOMP Authentication' }, chains: {}, }); + await bootstrap(); + await expect( - controller.init({ - chainId: MOCK_CHAIN_ID, - boringVaultAddress: MOCK_BORING_VAULT_ADDRESS, - }), - ).rejects.toThrow('Chain 0x1 not found in service details response'); + controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS), + ).rejects.toThrow('MoneyAccountUpgradeController is not bootstrapped'); + }); + + it('waits for an in-flight bootstrap instead of throwing', async () => { + let resolveServiceDetails: (value?: unknown) => void = () => undefined; + const { controller, mocks } = setup(); + mocks.getServiceDetails.mockImplementationOnce( + () => + new Promise((resolve) => { + resolveServiceDetails = resolve; + }), + ); + controller.init(); + await flushPromises(); + + const upgrade = controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS); + resolveServiceDetails(MOCK_SERVICE_DETAILS_RESPONSE); + + expect(await upgrade).toBeUndefined(); + expect(mocks.signPersonalMessage).toHaveBeenCalled(); + }); + + it('waits for an in-flight re-bootstrap instead of using the superseded config', async () => { + let resolveSecond: (value?: unknown) => void = () => undefined; + const { controller, config, mocks, bootstrap, triggerFlagChange } = + setup(); + await bootstrap(); + mocks.getServiceDetails.mockImplementationOnce( + () => + new Promise((resolve) => { + resolveSecond = resolve; + }), + ); + config.vaultConfig = CHANGED_VAULT_CONFIG; + await triggerFlagChange(); + + const upgrade = controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS); + await flushPromises(); + expect(mocks.signPersonalMessage).not.toHaveBeenCalled(); + + resolveSecond(MOCK_SERVICE_DETAILS_RESPONSE); + + expect(await upgrade).toBeUndefined(); + expect(mocks.signPersonalMessage).toHaveBeenCalled(); + }); + + it('waits for a run chained on while it was already waiting', async () => { + let resolveFirst: (value?: unknown) => void = () => undefined; + const { controller, config, mocks, triggerFlagChange } = setup(); + mocks.getServiceDetails.mockImplementationOnce( + () => + new Promise((resolve) => { + resolveFirst = resolve; + }), + ); + controller.init(); + await flushPromises(); + + // The upgrade captures the first run's promise; a config change then + // chains a second run, which supersedes the first. + const upgrade = controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS); + config.vaultConfig = CHANGED_VAULT_CONFIG; + await triggerFlagChange(); + resolveFirst(MOCK_SERVICE_DETAILS_RESPONSE); + + expect(await upgrade).toBeUndefined(); + expect(mocks.getServiceDetails).toHaveBeenCalledTimes(2); + }); + + it('throws instead of using the superseded config when a re-bootstrap fails', async () => { + const { + controller, + config, + mocks, + bootstrap, + triggerFlagChange, + triggerKeyringChange, + } = setup(); + await bootstrap(); + mocks.getServiceDetails.mockRejectedValueOnce(new Error('CHOMP outage')); + config.vaultConfig = CHANGED_VAULT_CONFIG; + await triggerFlagChange(); + + await expect( + controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS), + ).rejects.toThrow('MoneyAccountUpgradeController is not bootstrapped'); + + // The next trigger retries the bootstrap; once it succeeds the + // upgrade runs against the new config. + await triggerKeyringChange(); + + expect( + await controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS), + ).toBeUndefined(); + }); + + it('throws after the isEnabled hook flips off and a sync disarms the controller', async () => { + const { controller, config, mocks, bootstrap, triggerFlagChange } = + setup(); + await bootstrap(); + expect(mocks.getServiceDetails).toHaveBeenCalledTimes(1); + + config.isEnabled = false; + await triggerFlagChange(); + + await expect( + controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS), + ).rejects.toThrow('MoneyAccountUpgradeController is not bootstrapped'); + }); + + it('does not re-arm when disarmed while the CHOMP call is in flight', async () => { + let resolveServiceDetails: (value?: unknown) => void = () => undefined; + const { controller, config, mocks, bootstrap, triggerFlagChange } = + setup(); + mocks.getServiceDetails.mockImplementationOnce( + () => + new Promise((resolve) => { + resolveServiceDetails = resolve; + }), + ); + await bootstrap(); + + config.isEnabled = false; + await triggerFlagChange(); + resolveServiceDetails(MOCK_SERVICE_DETAILS_RESPONSE); + await flushPromises(); + + await expect( + controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS), + ).rejects.toThrow('MoneyAccountUpgradeController is not bootstrapped'); + }); + + it('re-bootstraps from scratch after being disarmed', async () => { + const { config, mocks, bootstrap, triggerFlagChange } = setup(); + await bootstrap(); + + config.isEnabled = false; + await triggerFlagChange(); + config.isEnabled = true; + await triggerFlagChange(); + + expect(mocks.getServiceDetails).toHaveBeenCalledTimes(2); + }); + + it('throws without running any step while the wallet is locked, and resumes on unlock without re-bootstrapping', async () => { + const { controller, config, mocks, bootstrap, triggerKeyringChange } = + setup(); + await bootstrap(); + + config.isUnlocked = false; + await triggerKeyringChange(); await expect( controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS), ).rejects.toThrow( - 'MoneyAccountUpgradeController must be initialized via init() before upgradeAccount() can be called', + 'MoneyAccountUpgradeController is not bootstrapped: upgradeAccount() requires the feature flag on, the wallet unlocked, and a successful bootstrap', ); + expect(mocks.signPersonalMessage).not.toHaveBeenCalled(); + + config.isUnlocked = true; + await triggerKeyringChange(); + await controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS); + + expect(mocks.getServiceDetails).toHaveBeenCalledTimes(1); + expect( + controller.state.upgradedAccounts[MOCK_ACCOUNT_ADDRESS], + ).toBeDefined(); }); - it('runs each step against the deployment-derived contract addresses', async () => { - const { controller, mocks } = setup(); - await controller.init({ - chainId: MOCK_CHAIN_ID, - boringVaultAddress: MOCK_BORING_VAULT_ADDRESS, + it('aborts the sequence without signing further when disarmed mid-run', async () => { + const { controller, config, mocks, bootstrap, triggerFlagChange } = + setup(); + await bootstrap(); + mocks.signPersonalMessage.mockImplementationOnce(async () => { + config.isEnabled = false; + await triggerFlagChange(); + return '0xdeadbeef'; + }); + + await expect( + controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS), + ).rejects.toThrow( + 'MoneyAccountUpgradeController upgrade aborted: the upgrade config was disarmed or superseded while the sequence was running', + ); + + expect(mocks.signEip7702Authorization).not.toHaveBeenCalled(); + expect(mocks.signDelegation).not.toHaveBeenCalled(); + expect(controller.state.upgradedAccounts).toStrictEqual({}); + }); + + it('aborts the sequence when the vault config changes mid-run, then upgrades against the new config', async () => { + const { controller, config, mocks, bootstrap, triggerFlagChange } = + setup(); + await bootstrap(); + mocks.signEip7702Authorization.mockImplementationOnce(async () => { + config.vaultConfig = CHANGED_VAULT_CONFIG; + await triggerFlagChange(); + return `0x${'1'.repeat(64)}${'2'.repeat(64)}1c`; }); + await expect( + controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS), + ).rejects.toThrow('MoneyAccountUpgradeController upgrade aborted'); + + expect(mocks.signDelegation).not.toHaveBeenCalled(); + expect(controller.state.upgradedAccounts).toStrictEqual({}); + expect(mocks.getServiceDetails).toHaveBeenCalledTimes(2); + + clearMockCalls(mocks); + await controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS); + + expect(mocks.signDelegation).toHaveBeenCalled(); + expect( + controller.state.upgradedAccounts[MOCK_ACCOUNT_ADDRESS], + ).toBeDefined(); + }); + + it('runs each step against the deployment-derived contract addresses', async () => { + const { controller, mocks, bootstrap } = setup(); + await bootstrap(); + await controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS); expect(mocks.signPersonalMessage).toHaveBeenCalledWith( @@ -466,12 +1194,28 @@ describe('MoneyAccountUpgradeController', () => { ); }); + it('uses the vault config boring vault as the withdrawal-side delegation token', async () => { + const { controller, mocks, bootstrap } = setup(); + await bootstrap(); + + await controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS); + + // Both delegations were signed; the boring-vault address shows up in the + // ABI-encoded ERC20TransferAmount caveat terms of one of them. + expect(mocks.signDelegation).toHaveBeenCalledTimes(2); + const allCaveatTerms = mocks.verifyDelegation.mock.calls + .flatMap(([{ signedDelegation }]) => signedDelegation.caveats) + .map((caveat) => caveat.terms.toLowerCase()); + expect( + allCaveatTerms.some((terms) => + terms.includes(MOCK_BORING_VAULT_ADDRESS.toLowerCase().slice(2)), + ), + ).toBe(true); + }); + it('is callable via the messenger', async () => { - const { controller, rootMessenger } = setup(); - await controller.init({ - chainId: MOCK_CHAIN_ID, - boringVaultAddress: MOCK_BORING_VAULT_ADDRESS, - }); + const { rootMessenger, bootstrap } = setup(); + await bootstrap(); expect( await rootMessenger.call( @@ -482,11 +1226,8 @@ describe('MoneyAccountUpgradeController', () => { }); it('propagates errors thrown by a step', async () => { - const { controller, mocks } = setup(); - await controller.init({ - chainId: MOCK_CHAIN_ID, - boringVaultAddress: MOCK_BORING_VAULT_ADDRESS, - }); + const { controller, mocks, bootstrap } = setup(); + await bootstrap(); mocks.signPersonalMessage.mockRejectedValue(new Error('signing failed')); await expect( @@ -495,11 +1236,8 @@ describe('MoneyAccountUpgradeController', () => { }); it('wraps a step failure in a MoneyAccountUpgradeStepError that records the step and cause', async () => { - const { controller, mocks } = setup(); - await controller.init({ - chainId: MOCK_CHAIN_ID, - boringVaultAddress: MOCK_BORING_VAULT_ADDRESS, - }); + const { controller, mocks, bootstrap } = setup(); + await bootstrap(); const cause = new Error('signing failed'); // The associate-address step (first in the sequence) signs a personal // message before calling CHOMP, so failing this surfaces that step. @@ -520,11 +1258,8 @@ describe('MoneyAccountUpgradeController', () => { }); it('records the name of the specific step that failed', async () => { - const { controller, mocks } = setup(); - await controller.init({ - chainId: MOCK_CHAIN_ID, - boringVaultAddress: MOCK_BORING_VAULT_ADDRESS, - }); + const { controller, mocks, bootstrap } = setup(); + await bootstrap(); // The first step (associate-address) passes; fail at the second step // (eip-7702-authorization), which signs the authorization. mocks.signEip7702Authorization.mockRejectedValue( @@ -539,11 +1274,8 @@ describe('MoneyAccountUpgradeController', () => { }); it('wraps a non-Error thrown by a step, stringifying it as the cause message', async () => { - const { controller, mocks } = setup(); - await controller.init({ - chainId: MOCK_CHAIN_ID, - boringVaultAddress: MOCK_BORING_VAULT_ADDRESS, - }); + const { controller, mocks, bootstrap } = setup(); + await bootstrap(); mocks.signPersonalMessage.mockRejectedValue('plain string failure'); const error = await controller @@ -560,11 +1292,8 @@ describe('MoneyAccountUpgradeController', () => { }); 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, - }); + const { controller, mocks, bootstrap } = setup(); + await bootstrap(); // EIP-7702 delegation code pointing at a third-party impl. mocks.providerRequest.mockImplementation( async ({ method }: { method: string }) => { @@ -583,11 +1312,8 @@ describe('MoneyAccountUpgradeController', () => { }); 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, - }); + const { controller, mocks, bootstrap } = setup(); + await bootstrap(); mocks.signPersonalMessage.mockRejectedValue(new Error('network down')); const error = await controller @@ -601,11 +1327,8 @@ describe('MoneyAccountUpgradeController', () => { 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 { controller, mocks, bootstrap } = setup(); + await bootstrap(); const mixedCaseAddress = MOCK_ACCOUNT_ADDRESS.replace( '0xabc', '0xABC', @@ -623,11 +1346,8 @@ describe('MoneyAccountUpgradeController', () => { }); 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, - }); + const { controller, mocks, bootstrap } = setup(); + await bootstrap(); await controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS); clearMockCalls(mocks); @@ -640,11 +1360,8 @@ describe('MoneyAccountUpgradeController', () => { }); it('treats recorded upgrades case-insensitively', async () => { - const { controller, mocks } = setup(); - await controller.init({ - chainId: MOCK_CHAIN_ID, - boringVaultAddress: MOCK_BORING_VAULT_ADDRESS, - }); + const { controller, mocks, bootstrap } = setup(); + await bootstrap(); await controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS); clearMockCalls(mocks); @@ -657,17 +1374,11 @@ describe('MoneyAccountUpgradeController', () => { 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.bootstrap(); 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.bootstrap(); await second.controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS); expect(second.mocks.signPersonalMessage).not.toHaveBeenCalled(); @@ -675,11 +1386,8 @@ describe('MoneyAccountUpgradeController', () => { }); 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, - }); + const { controller, mocks, bootstrap } = setup(); + await bootstrap(); mocks.signPersonalMessage.mockRejectedValueOnce( new Error('signing failed'), ); @@ -698,17 +1406,16 @@ describe('MoneyAccountUpgradeController', () => { }); 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, - }); + const { controller, config, mocks, bootstrap, triggerFlagChange } = + setup(); + await bootstrap(); 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. + // CHOMP rotates its delegate address, published alongside a vault + // config refresh — the recorded upgrade no longer reflects the active + // config. mocks.getServiceDetails.mockResolvedValue({ ...MOCK_SERVICE_DETAILS_RESPONSE, chains: { @@ -719,10 +1426,8 @@ describe('MoneyAccountUpgradeController', () => { }, }, }); - await controller.init({ - chainId: MOCK_CHAIN_ID, - boringVaultAddress: MOCK_BORING_VAULT_ADDRESS, - }); + config.vaultConfig = CHANGED_VAULT_CONFIG; + await triggerFlagChange(); clearMockCalls(mocks); await controller.upgradeAccount(MOCK_ACCOUNT_ADDRESS); diff --git a/packages/money-account-upgrade-controller/src/MoneyAccountUpgradeController.ts b/packages/money-account-upgrade-controller/src/MoneyAccountUpgradeController.ts index e912f408424..5042b51d195 100644 --- a/packages/money-account-upgrade-controller/src/MoneyAccountUpgradeController.ts +++ b/packages/money-account-upgrade-controller/src/MoneyAccountUpgradeController.ts @@ -19,19 +19,35 @@ import type { } from '@metamask/chomp-api-service'; import type { DelegationControllerSignDelegationAction } from '@metamask/delegation-controller'; import { DELEGATOR_CONTRACTS } from '@metamask/delegation-deployments'; +import { KeyringTypes } from '@metamask/keyring-controller'; import type { + KeyringControllerGetStateAction, KeyringControllerSignEip7702AuthorizationAction, KeyringControllerSignPersonalMessageAction, + KeyringControllerState, } from '@metamask/keyring-controller'; import type { Messenger } from '@metamask/messenger'; +import { + areMoneyAccountVaultConfigsEqual, + getMoneyAccountVaultConfig, +} from '@metamask/money-account-utils'; +import type { MoneyAccountVaultConfig } from '@metamask/money-account-utils'; import type { NetworkControllerFindNetworkClientIdByChainIdAction, NetworkControllerGetNetworkClientByIdAction, } from '@metamask/network-controller'; +import type { + FeatureFlags, + RemoteFeatureFlagControllerGetStateAction, + RemoteFeatureFlagControllerState, +} from '@metamask/remote-feature-flag-controller'; import { hexToNumber } from '@metamask/utils'; import type { Hex } from '@metamask/utils'; -import { MoneyAccountUpgradeStepError } from './errors.js'; +import { + MissingMoneyAccountVaultConfigError, + MoneyAccountUpgradeStepError, +} from './errors.js'; import type { MoneyAccountUpgradeControllerMethodActions } from './MoneyAccountUpgradeController-method-action-types.js'; import { associateAddressStep } from './steps/associate-address.js'; import { buildDelegationStep } from './steps/build-delegations.js'; @@ -117,10 +133,12 @@ type AllowedActions = | ChompApiServiceGetServiceDetailsAction | ChompApiServiceVerifyDelegationAction | DelegationControllerSignDelegationAction + | KeyringControllerGetStateAction | KeyringControllerSignEip7702AuthorizationAction | KeyringControllerSignPersonalMessageAction | NetworkControllerFindNetworkClientIdByChainIdAction - | NetworkControllerGetNetworkClientByIdAction; + | NetworkControllerGetNetworkClientByIdAction + | RemoteFeatureFlagControllerGetStateAction; export type MoneyAccountUpgradeControllerStateChangedEvent = ControllerStateChangedEvent< @@ -131,7 +149,12 @@ export type MoneyAccountUpgradeControllerStateChangedEvent = export type MoneyAccountUpgradeControllerEvents = MoneyAccountUpgradeControllerStateChangedEvent; -type AllowedEvents = never; +type AllowedEvents = + | ControllerStateChangedEvent<'KeyringController', KeyringControllerState> + | ControllerStateChangedEvent< + 'RemoteFeatureFlagController', + RemoteFeatureFlagControllerState + >; export type MoneyAccountUpgradeControllerMessenger = Messenger< typeof controllerName, @@ -140,7 +163,72 @@ export type MoneyAccountUpgradeControllerMessenger = Messenger< >; /** - * Controller that orchestrates the Money Account upgrade sequence. + * These hooks must be provided by the client - they provide functions that required to bootstrap + * the controller, which rely on client specific information. + */ +export type MoneyAccountUpgradeControllerHooks = { + /** + * Whether the Money Account feature is enabled for this client. + * + * The controller will call the function with the current state of + * the remote feature flags. It gets caleld on every sync, and re-checked + * when the bootstrap `awaits`. + * + * The isEnabled function should re-read any client state it depends + * on (e.g. a version-gated flag, a "basic functionality" toggle) rather + * than caching it. Returning `false` after a successful bootstrap disarms + * the controller: `upgradeAccount` refuses to run until a later sync + * re-bootstraps. + */ + isEnabled: (remoteFeatureFlags: FeatureFlags) => boolean; + + /** + * An asynchronous client gate checked once per bootstrap run, before any + * network is added or external service is called — e.g. a fail-closed + * geolocation check. A run skipped here is forgotten and retried on the + * next sync trigger. If the function is not provided we assume the client is eligible. + */ + isEligible?: () => Promise; + + /** + * Ensure the vault chain exists in the client's NetworkController before + * the bootstrap validates it. Adding a network is client-specific, so + * the controller awaits this before calling + * `NetworkController:findNetworkClientIdByChainId` consumers. Defaults to a + * no-op. + */ + ensureChainConfigured?: ( + vaultConfig: MoneyAccountVaultConfig, + ) => Promise; + + /** + * Called when a bootstrap run fails or cannot be scheduled. Receives a + * {@link MissingMoneyAccountVaultConfigError} (once per controller + * lifetime) when the enable flag is on but `moneyAccountVaultConfig` is + * unserved or malformed. + */ + onBootstrapError?: (error: unknown) => void; +}; + +/** + * Controller that owns the Money Account upgrade sequence and its own + * bootstrap. + * + * After {@link MoneyAccountUpgradeController.init} is called, the controller watches + * `RemoteFeatureFlagController` and `KeyringController` state and bootstraps + * itself when: + * + * 1. the client's `isEnabled` hook returns `true` for the current flags, + * 2. the wallet is unlocked with an HD keyring, + * 3. the client's `isEligible` hook (if any) resolves `true`, and + * 4. the `moneyAccountVaultConfig` flag parses. + * + * The bootstrap awaits the client's `ensureChainConfigured` hook and then + * fetches CHOMP service details to get the upgrade config. We recheck points + * 1–2 when awaiting in the bootstrap so a lock or an `isEnabled` will stop the process. + * + * The bootstrap re-runs whenever the vault config changes. + * `isEnabled` going `false` disables the controller. */ export class MoneyAccountUpgradeController extends BaseController< typeof controllerName, @@ -149,6 +237,28 @@ export class MoneyAccountUpgradeController extends BaseController< > { #config?: UpgradeConfig & { chainId: Hex }; + readonly #isEnabled: MoneyAccountUpgradeControllerHooks['isEnabled']; + + readonly #isEligible: NonNullable< + MoneyAccountUpgradeControllerHooks['isEligible'] + >; + + readonly #ensureChainConfigured: NonNullable< + MoneyAccountUpgradeControllerHooks['ensureChainConfigured'] + >; + + readonly #onBootstrapError: NonNullable< + MoneyAccountUpgradeControllerHooks['onBootstrapError'] + >; + + #initialized = false; + + #bootstrap?: Promise; + + #bootstrappedConfig?: MoneyAccountVaultConfig; + + #missingConfigReported = false; + readonly #steps: Step[] = [ associateAddressStep, eip7702AuthorizationStep, @@ -162,13 +272,17 @@ 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. + * @param options.hooks - The client hooks for the bootstrap; see + * {@link MoneyAccountUpgradeControllerHooks}. */ constructor({ messenger, state, + hooks, }: { messenger: MoneyAccountUpgradeControllerMessenger; state?: Partial; + hooks: MoneyAccountUpgradeControllerHooks; }) { super({ messenger, @@ -180,31 +294,214 @@ export class MoneyAccountUpgradeController extends BaseController< }, }); + this.#isEnabled = hooks.isEnabled; + this.#isEligible = hooks.isEligible ?? (async (): Promise => true); + this.#ensureChainConfigured = + hooks.ensureChainConfigured ?? (async (): Promise => undefined); + this.#onBootstrapError = hooks.onBootstrapError ?? ((): void => undefined); + this.messenger.registerMethodActionHandlers( this, MESSENGER_EXPOSED_METHODS, ); } + /** + * Start the controller's bootstrap: subscribe to the feature-flag and + * keyring triggers and run an initial sync. Call once, after all + * controllers and services the messenger reaches are constructed — this is + * the only method that may be called before the controller is bootstrapped, + * and it is the reason the constructor performs no messenger calls. + */ + init(): void { + if (this.#initialized) { + return; + } + this.#initialized = true; + + this.messenger.subscribe('RemoteFeatureFlagController:stateChanged', () => + this.sync(), + ); + this.messenger.subscribe('KeyringController:stateChanged', () => + this.sync(), + ); + this.sync(); + } + + /** + * Re-evaluate the bootstrap checks against live state and schedule a + * bootstrap run when they are open and the vault config is new. Runs on + * every feature-flag and keyring state change. + * + * If sync fails a failure is reported through `onBootstrapError` and retried + * on the next trigger. + */ + sync(): void { + try { + const { remoteFeatureFlags } = this.messenger.call( + 'RemoteFeatureFlagController:getState', + ); + + if (!this.#isEnabled(remoteFeatureFlags)) { + // Disarm so a later `upgradeAccount` cannot run against a config + // armed while the feature was still enabled. Re-enabling re-runs the + // bootstrap from scratch. + this.#config = undefined; + this.#bootstrappedConfig = undefined; + return; + } + + if (!this.#isWalletReady()) { + return; + } + + const vaultConfig = getMoneyAccountVaultConfig(remoteFeatureFlags); + if (!vaultConfig) { + this.#reportMissingConfig(); + return; + } + + if ( + this.#bootstrappedConfig && + areMoneyAccountVaultConfigsEqual(vaultConfig, this.#bootstrappedConfig) + ) { + return; + } + + this.#scheduleBootstrap(vaultConfig); + } catch (error) { + this.#reportBootstrapError(error); + } + } + + /** + * Hand a bootstrap failure to the client's `onBootstrapError` hook. A hook + * that throws must not break the controller: from a subscription it would + * surface as an unhandled rejection, and from `init()` it would abort the + * caller's startup. + * + * @param error - The failure to report. + */ + #reportBootstrapError(error: unknown): void { + try { + this.#onBootstrapError(error); + } catch { + // The hook is the client's error sink; there is nowhere else to send its + // own failure without recursing into it. + } + } + + /** + * Whether the wallet can sign right now: unlocked with an HD keyring. The + * keyring list matters because during a vault restore the unlock flips + * before the keyrings land in state. + * + * @returns Whether the wallet is ready. + */ + #isWalletReady(): boolean { + const { isUnlocked, keyrings } = this.messenger.call( + 'KeyringController:getState', + ); + return ( + isUnlocked && keyrings.some((keyring) => keyring.type === KeyringTypes.hd) + ); + } + + /** + * Whether the synchronous bootstrap gates are open right now. Re-read from + * live state on every call because the bootstrap re-validates them across + * its `await` points. + * + * @returns Whether the bootstrap may proceed. + */ + #areGatesOpen(): boolean { + const { remoteFeatureFlags } = this.messenger.call( + 'RemoteFeatureFlagController:getState', + ); + return this.#isEnabled(remoteFeatureFlags) && this.#isWalletReady(); + } + + #scheduleBootstrap(vaultConfig: MoneyAccountVaultConfig): void { + this.#bootstrappedConfig = vaultConfig; + + // Scheduling means the served vault config no longer matches whatever is + // armed, so disarm now: until this run succeeds, `upgradeAccount` must + // wait for it (or refuse if it fails) rather than sign delegations + // against the superseded vault. + this.#config = undefined; + + const run = async (): Promise => { + // The gates were checked when this run was scheduled, but it may start + // much later, chained behind an in-flight bootstrap. Eligibility comes + // after the gates so a disabled client also skips the (possibly + // external) eligibility lookup. + if (!this.#areGatesOpen() || !(await this.#isEligible())) { + this.#forget(vaultConfig); + return; + } + + await this.#ensureChainConfigured(vaultConfig); + + // Configuring the chain can suspend for a while, so re-check the gates + // before the external CHOMP call. + if (!this.#areGatesOpen()) { + this.#forget(vaultConfig); + return; + } + + await this.#applyVaultConfig(vaultConfig); + }; + + const bootstrap = this.#bootstrap + ? this.#bootstrap.catch(() => undefined).then(run) + : run(); + this.#bootstrap = bootstrap; + + bootstrap.catch((error) => { + this.#forget(vaultConfig); + this.#reportBootstrapError(error); + }); + } + + /** + * Forget a scheduled bootstrap that was skipped or failed, so the next + * trigger re-runs it — but only if no newer config has been scheduled + * meanwhile: a newer config supersedes this run, success or failure. + * + * @param vaultConfig - The config the abandoned run was scheduled with. + */ + #forget(vaultConfig: MoneyAccountVaultConfig): void { + if (this.#bootstrappedConfig === vaultConfig) { + this.#bootstrappedConfig = undefined; + } + } + + /** + * Report a served enable flag without a usable `moneyAccountVaultConfig` — + * a flag misconfiguration that silently disables upgrades. Reported once + * per controller lifetime; flag refreshes arrive continuously and would + * otherwise spam. + */ + #reportMissingConfig(): void { + if (!this.#missingConfigReported) { + this.#missingConfigReported = true; + this.#reportBootstrapError(new MissingMoneyAccountVaultConfigError()); + } + } + /** * Fetches service details and validates the controller can operate on the - * given chain. Resolves the Delegation Framework contract addresses for the - * chain from `@metamask/delegation-deployments`. + * vault's chain, arming the upgrade config `upgradeAccount` runs against. + * Resolves the Delegation Framework contract addresses for the chain from + * `@metamask/delegation-deployments`. * - * @param params - The parameters for initialization. - * @param params.chainId - The chain to initialize for. - * @param params.boringVaultAddress - The Veda boring vault contract - * (vmUSD) for the given chain. Used as the withdrawal-side delegation - * token. Supplied by the consumer until the CHOMP service-details API - * exposes it. + * @param vaultConfig - The vault config to arm; its `boringVault` is the + * withdrawal-side delegation token (vmUSD), supplied via the flag until + * the CHOMP service-details API exposes it. */ - async init({ - chainId, - boringVaultAddress, - }: { - chainId: Hex; - boringVaultAddress: Hex; - }): Promise { + async #applyVaultConfig(vaultConfig: MoneyAccountVaultConfig): Promise { + const { chainId, boringVault: boringVaultAddress } = vaultConfig; + const contracts = DELEGATOR_CONTRACTS[DELEGATION_FRAMEWORK_VERSION][hexToNumber(chainId)]; if (!contracts) { @@ -236,6 +533,13 @@ export class MoneyAccountUpgradeController extends BaseController< ); } + // A disarm (isEnabled flipping off) or a newer scheduled config during + // the CHOMP call supersedes this run: arming now would resurrect a config + // the controller just dropped, or briefly shadow the newer one. + if (this.#bootstrappedConfig !== vaultConfig) { + return; + } + this.#config = { chainId, delegateAddress: chain.autoDepositDelegate, @@ -263,12 +567,32 @@ export class MoneyAccountUpgradeController extends BaseController< * active config no longer matches the recorded fingerprint, the sequence * re-runs. * + * A call that arrives while the bootstrap chain is still in flight — + * including runs scheduled while waiting — waits for it to settle rather + * than failing, so the upgrade always runs against the latest armed + * config. Scheduling a bootstrap for a changed vault config disarms the + * previous one, so it throws when no bootstrap has armed a config (feature + * disabled or the last bootstrap failed) or when the wallet is locked. + * + * The armed config is re-checked before every step: if a sync disarms or + * supersedes it while the sequence is running, the sequence aborts before + * the next step signs anything, and nothing is recorded. + * * @param address - The Money Account address to upgrade. */ async upgradeAccount(address: Hex): Promise { - if (!this.#config) { + let bootstrap = this.#bootstrap; + while (bootstrap) { + await bootstrap.catch(() => undefined); + // A newer run may have been chained on while we waited; wait for that + // one too, until the chain settles. + bootstrap = this.#bootstrap === bootstrap ? undefined : this.#bootstrap; + } + // A lock does not disarm the config (unlocking must not cost a CHOMP + // re-fetch), so the wallet has to be checked here as well. + if (!this.#config || !this.#areGatesOpen()) { throw new Error( - 'MoneyAccountUpgradeController must be initialized via init() before upgradeAccount() can be called', + 'MoneyAccountUpgradeController is not bootstrapped: upgradeAccount() requires the feature flag on, the wallet unlocked, and a successful bootstrap', ); } const config = this.#config; @@ -283,6 +607,11 @@ export class MoneyAccountUpgradeController extends BaseController< } for (const step of this.#steps) { + if (this.#config !== config) { + throw new Error( + 'MoneyAccountUpgradeController upgrade aborted: the upgrade config was disarmed or superseded while the sequence was running', + ); + } try { await step.run({ messenger: this.messenger, diff --git a/packages/money-account-upgrade-controller/src/errors.ts b/packages/money-account-upgrade-controller/src/errors.ts index 1aef34ff3c8..a1779905559 100644 --- a/packages/money-account-upgrade-controller/src/errors.ts +++ b/packages/money-account-upgrade-controller/src/errors.ts @@ -52,6 +52,20 @@ export class TerminalUpgradeError extends Error { } } +/** + * Error reported through the `onBootstrapError` hook when the Money Account + * enable flag is on but the `moneyAccountVaultConfig` flag is unserved or + * malformed. + */ +export class MissingMoneyAccountVaultConfigError extends Error { + constructor() { + super( + 'Money Account upgrade bootstrap skipped: vault configuration is unavailable', + ); + this.name = 'MissingMoneyAccountVaultConfigError'; + } +} + /** * Type guard for {@link MoneyAccountUpgradeStepError}. * diff --git a/packages/money-account-upgrade-controller/src/index.ts b/packages/money-account-upgrade-controller/src/index.ts index ddb674d4114..8db25bd1d9a 100644 --- a/packages/money-account-upgrade-controller/src/index.ts +++ b/packages/money-account-upgrade-controller/src/index.ts @@ -1,5 +1,6 @@ export type { UpgradeConfig } from './types.js'; export { + MissingMoneyAccountVaultConfigError, MoneyAccountUpgradeStepError, TerminalUpgradeError, isMoneyAccountUpgradeStepError, @@ -13,6 +14,7 @@ export type { MoneyAccountUpgradeControllerState, MoneyAccountUpgradeControllerGetStateAction, MoneyAccountUpgradeControllerActions, + MoneyAccountUpgradeControllerHooks, MoneyAccountUpgradeControllerStateChangedEvent, MoneyAccountUpgradeControllerEvents, MoneyAccountUpgradeControllerMessenger, diff --git a/packages/money-account-upgrade-controller/tsconfig.build.json b/packages/money-account-upgrade-controller/tsconfig.build.json index 66f113f221d..dbc68c02926 100644 --- a/packages/money-account-upgrade-controller/tsconfig.build.json +++ b/packages/money-account-upgrade-controller/tsconfig.build.json @@ -24,8 +24,14 @@ { "path": "../messenger/tsconfig.build.json" }, + { + "path": "../money-account-utils/tsconfig.build.json" + }, { "path": "../network-controller/tsconfig.build.json" + }, + { + "path": "../remote-feature-flag-controller/tsconfig.build.json" } ], "include": ["../../types", "./src"] diff --git a/packages/money-account-upgrade-controller/tsconfig.json b/packages/money-account-upgrade-controller/tsconfig.json index 1273483ee1d..171a6c971a2 100644 --- a/packages/money-account-upgrade-controller/tsconfig.json +++ b/packages/money-account-upgrade-controller/tsconfig.json @@ -22,8 +22,14 @@ { "path": "../messenger" }, + { + "path": "../money-account-utils" + }, { "path": "../network-controller" + }, + { + "path": "../remote-feature-flag-controller" } ], "include": ["../../types", "./src"] diff --git a/packages/money-account-utils/CHANGELOG.md b/packages/money-account-utils/CHANGELOG.md index 5518297bf6b..ec684bab192 100644 --- a/packages/money-account-utils/CHANGELOG.md +++ b/packages/money-account-utils/CHANGELOG.md @@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add Money Account vault-config helpers, shared by the upgrade controller and clients ([#10072](https://github.com/MetaMask/core/pull/10072)) + - `getMoneyAccountVaultConfig` and `parseMoneyAccountVaultConfig` read and validate the `moneyAccountVaultConfig` remote feature flag into a `MoneyAccountVaultConfig` whose chain id and addresses are known-good `Hex`; `underlyingToken` is optional so flags deployed before that field existed still parse + - `areMoneyAccountVaultConfigsEqual` compares configs field by field so consumers keyed on the config (bootstraps, caches) can detect changes + - `MONEY_ACCOUNT_VAULT_CONFIG_FLAG_NAME` names the flag + ### Changed - Bump `@metamask/transaction-controller` from `^69.4.0` to `^69.7.0` ([#9780](https://github.com/MetaMask/core/pull/9780), [#9798](https://github.com/MetaMask/core/pull/9798), [#9823](https://github.com/MetaMask/core/pull/9823), [#9960](https://github.com/MetaMask/core/pull/9960), [#9969](https://github.com/MetaMask/core/pull/9969), [#10046](https://github.com/MetaMask/core/pull/10046)) diff --git a/packages/money-account-utils/src/index.ts b/packages/money-account-utils/src/index.ts index cb74ffbacee..fa6762dd53d 100644 --- a/packages/money-account-utils/src/index.ts +++ b/packages/money-account-utils/src/index.ts @@ -22,6 +22,13 @@ export { getMoneyAccountDepositAssetId, getSharesForWithdrawal, } from './transactions.js'; +export { + MONEY_ACCOUNT_VAULT_CONFIG_FLAG_NAME, + areMoneyAccountVaultConfigsEqual, + getMoneyAccountVaultConfig, + parseMoneyAccountVaultConfig, +} from './vault-config.js'; +export type { MoneyAccountVaultConfig } from './vault-config.js'; export type { BuildMoneyAccountDepositBatchOptions, BuildMoneyAccountDepositPlaceholderBatchOptions, diff --git a/packages/money-account-utils/src/vault-config.test.ts b/packages/money-account-utils/src/vault-config.test.ts new file mode 100644 index 00000000000..32bc6a63659 --- /dev/null +++ b/packages/money-account-utils/src/vault-config.test.ts @@ -0,0 +1,198 @@ +import type { MoneyAccountVaultConfig } from './vault-config.js'; +import { + areMoneyAccountVaultConfigsEqual, + getMoneyAccountVaultConfig, + parseMoneyAccountVaultConfig, +} from './vault-config.js'; + +const VALID_CONFIG = { + chainId: '0x8f', + boringVault: '0xb4563bcD3B7764CCBf497f515585f70B6C3EA5Ae', + tellerAddress: '0x2D49EA58A4C70b62c8B56DE971310d9e999c8117', + accountantAddress: '0x7382c5b8B51B8C4f127B3123C1039581BAA5A06B', + lensAddress: '0xA816ECd922de94c6879AD23B9A884dB257F20947', + underlyingToken: '0xacA92E438df0B2401fF60dA7E4337B687a2435DA', +} as const; + +const REQUIRED_ADDRESS_KEYS = [ + 'boringVault', + 'tellerAddress', + 'accountantAddress', + 'lensAddress', +] as const; + +const ADDRESS_KEYS = [...REQUIRED_ADDRESS_KEYS, 'underlyingToken'] as const; + +const INVALID_CHAIN_IDS: [string, unknown][] = [ + ['is not prefixed', '8f'], + ['has non-hex digits', '0xmonad'], + ['is missing', undefined], + ['is a number', 143], + ['is null', null], + ['is empty', ''], +]; + +const INVALID_ADDRESSES: [string, unknown][] = [ + ['is not hex', 'not-an-address'], + ['is truncated', '0xb4563bcD3B7764CCBf497f5'], + // A valid address but with its final byte upper-cased: still 20 hex bytes, + // but no longer a valid ERC-55 checksum. + ['has a bad ERC-55 checksum', '0xb4563bcD3B7764CCBf497f515585f70B6C3EA5AE'], + ['is missing', undefined], + ['is not a string', 1234], + ['is null', null], + ['is empty', ''], +]; + +const NON_OBJECT_FLAGS: [string, unknown][] = [ + ['a string', 'moneyAccountVaultConfig'], + ['a number', 1], + ['null', null], + ['undefined', undefined], + ['an array', [VALID_CONFIG]], + ['an empty object', {}], +]; + +describe('parseMoneyAccountVaultConfig', () => { + it('parses a well-formed config', () => { + expect(parseMoneyAccountVaultConfig(VALID_CONFIG)).toStrictEqual( + VALID_CONFIG, + ); + }); + + it('passes checksummed addresses through unchanged, without normalising', () => { + const parsed = parseMoneyAccountVaultConfig(VALID_CONFIG); + + expect(parsed?.boringVault).toBe(VALID_CONFIG.boringVault); + expect(parsed?.tellerAddress).toBe(VALID_CONFIG.tellerAddress); + expect(parsed?.accountantAddress).toBe(VALID_CONFIG.accountantAddress); + expect(parsed?.lensAddress).toBe(VALID_CONFIG.lensAddress); + expect(parsed?.underlyingToken).toBe(VALID_CONFIG.underlyingToken); + }); + + it('accepts all-lowercase addresses', () => { + const lowercased = { + ...VALID_CONFIG, + boringVault: VALID_CONFIG.boringVault.toLowerCase(), + }; + + expect(parseMoneyAccountVaultConfig(lowercased)).toStrictEqual(lowercased); + }); + + it('ignores unknown extra fields', () => { + expect( + parseMoneyAccountVaultConfig({ ...VALID_CONFIG, someFutureField: 1 }), + ).toStrictEqual(VALID_CONFIG); + }); + + for (const [description, chainId] of INVALID_CHAIN_IDS) { + it(`rejects a config whose chain id ${description}`, () => { + expect( + parseMoneyAccountVaultConfig({ ...VALID_CONFIG, chainId }), + ).toBeUndefined(); + }); + } + + for (const key of REQUIRED_ADDRESS_KEYS) { + for (const [description, value] of INVALID_ADDRESSES) { + it(`rejects a config whose ${key} ${description}`, () => { + expect( + parseMoneyAccountVaultConfig({ ...VALID_CONFIG, [key]: value }), + ).toBeUndefined(); + }); + } + } + + // Flags deployed before `underlyingToken` existed must still parse, matching + // the balance service's schema for the same flag. + it('accepts a config without an underlyingToken', () => { + const { underlyingToken: _omitted, ...withoutUnderlyingToken } = + VALID_CONFIG; + + expect(parseMoneyAccountVaultConfig(withoutUnderlyingToken)).toStrictEqual( + withoutUnderlyingToken, + ); + }); + + for (const [description, value] of INVALID_ADDRESSES.filter( + ([, invalid]) => invalid !== undefined, + )) { + it(`rejects a config whose underlyingToken ${description}`, () => { + expect( + parseMoneyAccountVaultConfig({ + ...VALID_CONFIG, + underlyingToken: value, + }), + ).toBeUndefined(); + }); + } + + for (const [description, raw] of NON_OBJECT_FLAGS) { + it(`rejects a flag that is ${description}`, () => { + expect(parseMoneyAccountVaultConfig(raw)).toBeUndefined(); + }); + } +}); + +describe('getMoneyAccountVaultConfig', () => { + it('parses the config out of the remote feature flags', () => { + expect( + getMoneyAccountVaultConfig({ + moneyAccountVaultConfig: { ...VALID_CONFIG }, + }), + ).toStrictEqual(VALID_CONFIG); + }); + + it('returns undefined when the flag is unserved', () => { + expect(getMoneyAccountVaultConfig({})).toBeUndefined(); + }); + + it('returns undefined when there are no flags at all', () => { + expect(getMoneyAccountVaultConfig(undefined)).toBeUndefined(); + }); + + it('returns undefined when the flag is malformed', () => { + expect( + getMoneyAccountVaultConfig({ + moneyAccountVaultConfig: { ...VALID_CONFIG, lensAddress: '0x0' }, + }), + ).toBeUndefined(); + }); +}); + +describe('areMoneyAccountVaultConfigsEqual', () => { + const config = VALID_CONFIG as MoneyAccountVaultConfig; + + it('treats identical configs as equal', () => { + expect(areMoneyAccountVaultConfigsEqual(config, { ...config })).toBe(true); + }); + + for (const key of ['chainId', ...ADDRESS_KEYS] as const) { + it(`treats configs differing in ${key} as unequal`, () => { + expect( + areMoneyAccountVaultConfigsEqual(config, { + ...config, + [key]: '0x1111111111111111111111111111111111111111', + }), + ).toBe(false); + }); + } + + it('treats configs that both omit the underlyingToken as equal', () => { + const { underlyingToken: _omitted, ...withoutUnderlyingToken } = config; + + expect( + areMoneyAccountVaultConfigsEqual(withoutUnderlyingToken, { + ...withoutUnderlyingToken, + }), + ).toBe(true); + }); + + it('treats a config that gains an underlyingToken as changed', () => { + const { underlyingToken: _omitted, ...withoutUnderlyingToken } = config; + + expect( + areMoneyAccountVaultConfigsEqual(withoutUnderlyingToken, config), + ).toBe(false); + }); +}); diff --git a/packages/money-account-utils/src/vault-config.ts b/packages/money-account-utils/src/vault-config.ts new file mode 100644 index 00000000000..32f99865a64 --- /dev/null +++ b/packages/money-account-utils/src/vault-config.ts @@ -0,0 +1,128 @@ +import { + isObject, + isStrictHexString, + isValidHexAddress, +} from '@metamask/utils'; +import type { Hex } from '@metamask/utils'; + +/** + * The LaunchDarkly flag carrying the Money Account vault contracts. The same + * flag `@metamask/money-account-balance-service` reads, so the parsed chain id + * here is the chain the balance service talks to. + */ +export const MONEY_ACCOUNT_VAULT_CONFIG_FLAG_NAME = 'moneyAccountVaultConfig'; + +/** + * The Money Account vault contracts served via remote feature flags, with the + * chain id and every address validated as known-good `Hex`. + * + * `underlyingToken` is optional because flags deployed before the field + * existed must still validate, matching the balance service's schema for the + * same flag. + */ +export type MoneyAccountVaultConfig = { + chainId: Hex; + boringVault: Hex; + tellerAddress: Hex; + accountantAddress: Hex; + lensAddress: Hex; + underlyingToken?: Hex; +}; + +/** + * Parses one raw vault-config field into an address. + * + * `isValidHexAddress` accepts an all-lowercase address or a valid ERC-55 + * checksum, which is what ethers accepts when it encodes the calldata. The + * address is deliberately not normalised, so a checksummed address passes + * through unchanged. + * + * @param value - The raw field value. + * @returns The address, or `undefined` if it is missing or malformed. + */ +const parseAddress = (value: unknown): Hex | undefined => + isStrictHexString(value) && isValidHexAddress(value) ? value : undefined; + +/** + * Parses the raw `moneyAccountVaultConfig` remote feature flag into a config + * whose chain id and addresses are known-good `Hex`. + * + * @param raw - The raw remote feature flag value. + * @returns The parsed vault config, or `undefined` if any field is missing or + * malformed. + */ +export const parseMoneyAccountVaultConfig = ( + raw: unknown, +): MoneyAccountVaultConfig | undefined => { + if (!isObject(raw)) { + return undefined; + } + + const { chainId } = raw; + if (!isStrictHexString(chainId)) { + return undefined; + } + + const boringVault = parseAddress(raw.boringVault); + const tellerAddress = parseAddress(raw.tellerAddress); + const accountantAddress = parseAddress(raw.accountantAddress); + const lensAddress = parseAddress(raw.lensAddress); + + if (!boringVault || !tellerAddress || !accountantAddress || !lensAddress) { + return undefined; + } + + const config: MoneyAccountVaultConfig = { + chainId, + boringVault, + tellerAddress, + accountantAddress, + lensAddress, + }; + + if (raw.underlyingToken === undefined) { + return config; + } + + const underlyingToken = parseAddress(raw.underlyingToken); + return underlyingToken ? { ...config, underlyingToken } : undefined; +}; + +/** + * Reads and parses the Money Account vault config out of the remote feature + * flags. + * + * @param remoteFeatureFlags - The remote feature flags. + * @returns The parsed vault config, or `undefined` when the flag is unserved + * or malformed. + */ +export function getMoneyAccountVaultConfig( + remoteFeatureFlags: Record | undefined, +): MoneyAccountVaultConfig | undefined { + return parseMoneyAccountVaultConfig( + remoteFeatureFlags?.[MONEY_ACCOUNT_VAULT_CONFIG_FLAG_NAME], + ); +} + +/** + * Compares vault configs field by field. Any difference means the vault the + * config points at has changed and consumers keyed on it (bootstraps, caches) + * must re-run. + * + * @param a - One vault config. + * @param b - The other vault config. + * @returns Whether the configs are equal. + */ +export function areMoneyAccountVaultConfigsEqual( + a: MoneyAccountVaultConfig, + b: MoneyAccountVaultConfig, +): boolean { + return ( + a.chainId === b.chainId && + a.boringVault === b.boringVault && + a.tellerAddress === b.tellerAddress && + a.accountantAddress === b.accountantAddress && + a.lensAddress === b.lensAddress && + a.underlyingToken === b.underlyingToken + ); +} diff --git a/yarn.lock b/yarn.lock index 1e88d4058a7..16331b66344 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7792,7 +7792,9 @@ __metadata: "@metamask/delegation-deployments": "npm:^1.4.0" "@metamask/keyring-controller": "npm:^27.1.1" "@metamask/messenger": "npm:^2.0.0" + "@metamask/money-account-utils": "npm:^1.1.0" "@metamask/network-controller": "npm:^36.0.0" + "@metamask/remote-feature-flag-controller": "npm:^6.1.0" "@metamask/utils": "npm:^11.12.0" "@ts-bridge/cli": "npm:^0.6.4" "@types/jest": "npm:^30.0.0" @@ -7807,7 +7809,7 @@ __metadata: languageName: unknown linkType: soft -"@metamask/money-account-utils@workspace:packages/money-account-utils": +"@metamask/money-account-utils@npm:^1.1.0, @metamask/money-account-utils@workspace:packages/money-account-utils": version: 0.0.0-use.local resolution: "@metamask/money-account-utils@workspace:packages/money-account-utils" dependencies: