Skip to content

Commit b1c3a75

Browse files
authored
feat: [MUSD-445] complete money account upgrade controller flow (#30002)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until this PR meets the canonical Definition of Ready For Review in `docs/readme/ready-for-review.md`. In short: the template must be materially complete (not just section titles present), all status checks must be currently passing, and the only expected follow-up commits must be reviewer-driven. --> ## **Description** This PR upgrades to the final version of the money account upgrade controller - which can in theory run through the entire chomp upgrade flow. Things that we've changed in this PR in addition to bumping the package 1. Updated the config that is passed into the upgrade controller 2. Added a check which ensures the monad network is enabled before we start the upgrade process. This is necessary, because monad is enabled by default # TODO 1. ~~This PR builds on @MoMannn's PR #29897 - and it should be merged first.~~ 2. This PR requires us to change the [vault config feature flag](https://app.launchdarkly.com/projects/metamask-client-config-api-mobile/flags/money-account-vault-config/targeting?env=test&selected-env=test) before it will function correctly. 3. Chomp returns a 500 on the final step of the upgrade process. We need to figure out why this is happening and fix it 4. ~~We need to merge and publish [this core pr](MetaMask/core#8621) and update the preview packages that are currently used in this branch to the real published updates.~~ <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: Update to final version of money account upgrade controller ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** <!-- Every checklist item must be consciously assessed before marking this PR as "Ready for review". A checked box means you deliberately considered that responsibility, not that you literally performed every action listed. Unchecked boxes are ambiguous: they are not an implicit "N/A" and they are not a silent "skip". See `docs/readme/ready-for-review.md` for the full checklist semantics. --> - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I've included tests if applicable - [ ] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. #### Performance checks (if applicable) - [ ] I've tested on Android - Ideally on a mid-range device; emulator is acceptable - [ ] I've tested with a power user scenario - Use these [power-user SRPs](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/edit-v2/401401446401?draftShareId=9d77e1e1-4bdc-4be1-9ebb-ccd916988d93) to import wallets with many accounts and tokens - [ ] I've instrumented key operations with Sentry traces for production performance metrics - See [`trace()`](/app/util/trace.ts) for usage and [`addToken`](/app/components/Views/AddAsset/components/AddCustomToken/AddCustomToken.tsx#L274) for an example For performance guidelines and tooling, see the [Performance Guide](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/400085549067/Performance+Guide+for+Engineers). ## **Pre-merge reviewer checklist** <!-- Reviewer checklist items follow the same semantics as the author checklist: an unchecked box is ambiguous, a checked box means the reviewer consciously assessed that responsibility. See `docs/readme/ready-for-review.md`. --> - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches money account upgrade bootstrap logic and programmatically adds missing networks via `NetworkController.addNetwork`, which can affect upgrade flow behavior and user network state if misconfigured. Also bumps to `@metamask/money-account-upgrade-controller@^2.0.0` with related dependency updates. > > **Overview** > Updates `moneyAccountUpgradeControllerInit` to initialize `MoneyAccountUpgradeController` using the Money Account *vault config* (chainId + boring vault address) instead of deriving addresses from CHOMP service details / delegator environment. > > Adds a bootstrap guard (`ensureChainConfigured`) that checks whether the vault chain is present in the user’s `NetworkController` config and, if missing, auto-adds it from `PopularList` (or logs an error and aborts if unsupported). Tests are updated to cover the new init parameters, missing vault config handling, and the chain auto-add behavior. > > Expands the controller messenger permissions for the full upgrade flow (delegation + additional CHOMP actions) and bumps `@metamask/money-account-upgrade-controller` to `^2.0.0` plus related dependency versions in `package.json`/`yarn.lock`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 74a760e. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent e0c9dce commit b1c3a75

5 files changed

Lines changed: 236 additions & 141 deletions

File tree

Lines changed: 105 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { CHAIN_IDS } from '@metamask/transaction-controller';
1+
import type { Hex } from '@metamask/utils';
22
import { MoneyAccountUpgradeController } from '@metamask/money-account-upgrade-controller';
33
import { MOCK_ANY_NAMESPACE, MockAnyNamespace } from '@metamask/messenger';
44
import { ExtendedMessenger } from '../../ExtendedMessenger';
@@ -7,7 +7,13 @@ import {
77
getMoneyAccountUpgradeControllerInitMessenger,
88
getMoneyAccountUpgradeControllerMessenger,
99
} from '../messengers/money-account-upgrade-controller-messenger';
10-
import { getDeleGatorEnvironment } from '../../Delegation/environment';
10+
import Engine from '../../Engine';
11+
import ReduxService from '../../redux';
12+
import {
13+
type MoneyAccountVaultConfig,
14+
selectMoneyAccountVaultConfig,
15+
} from '../../../selectors/featureFlagController/moneyAccount';
16+
import { selectEvmNetworkConfigurationsByChainId } from '../../../selectors/networkController';
1117
import {
1218
__resetMoneyAccountUpgradeBootstrapForTesting,
1319
moneyAccountUpgradeControllerInit,
@@ -17,38 +23,42 @@ import Logger from '../../../util/Logger';
1723

1824
jest.mock('@metamask/money-account-upgrade-controller');
1925

20-
jest.mock('../../Delegation/environment', () => ({
21-
getDeleGatorEnvironment: jest.fn(),
26+
jest.mock('../../redux');
27+
28+
jest.mock('../../Engine', () => ({
29+
__esModule: true,
30+
default: {
31+
context: {
32+
NetworkController: {
33+
addNetwork: jest.fn().mockResolvedValue(undefined),
34+
},
35+
},
36+
},
37+
}));
38+
39+
jest.mock('../../../selectors/featureFlagController/moneyAccount');
40+
41+
jest.mock('../../../selectors/networkController', () => ({
42+
selectEvmNetworkConfigurationsByChainId: jest.fn(),
2243
}));
2344

2445
jest.mock('../../../util/Logger', () => ({
2546
error: jest.fn(),
2647
}));
2748

28-
const MUSD_TOKEN_ADDRESS = '0x000000000000000000000000000000000000dead';
29-
const DELEGATOR_IMPL = '0x0000000000000000000000000000000000000001';
30-
const REDEEMER_ENFORCER = '0x0000000000000000000000000000000000000002';
31-
const VALUE_LTE_ENFORCER = '0x0000000000000000000000000000000000000003';
32-
33-
const SERVICE_DETAILS = {
34-
chains: {
35-
[CHAIN_IDS.ARBITRUM]: {
36-
protocol: {
37-
vedaProtocol: {
38-
supportedTokens: [{ tokenAddress: MUSD_TOKEN_ADDRESS }],
39-
},
40-
},
41-
},
42-
},
49+
const VAULT_CHAIN_ID = '0x8f' as Hex;
50+
const BORING_VAULT_ADDRESS =
51+
'0x000000000000000000000000000000000000beef' as Hex;
52+
53+
const VAULT_CONFIG: MoneyAccountVaultConfig = {
54+
chainId: VAULT_CHAIN_ID,
55+
boringVault: BORING_VAULT_ADDRESS,
56+
tellerAddress: '0x0000000000000000000000000000000000000001',
57+
accountantAddress: '0x0000000000000000000000000000000000000002',
58+
lensAddress: '0x0000000000000000000000000000000000000003',
4359
};
4460

45-
function getInitRequestMock({
46-
isUnlocked,
47-
serviceDetails = SERVICE_DETAILS,
48-
}: {
49-
isUnlocked: boolean;
50-
serviceDetails?: unknown;
51-
}) {
61+
function getInitRequestMock({ isUnlocked }: { isUnlocked: boolean }) {
5262
const baseMessenger = new ExtendedMessenger<MockAnyNamespace, never, never>({
5363
namespace: MOCK_ANY_NAMESPACE,
5464
});
@@ -59,21 +69,14 @@ function getInitRequestMock({
5969
jest.fn().mockReturnValue({ isUnlocked }),
6070
);
6171

62-
const getServiceDetails = jest.fn().mockResolvedValue(serviceDetails);
63-
baseMessenger.registerActionHandler(
64-
// @ts-expect-error: Action not allowed on the mock messenger namespace.
65-
'ChompApiService:getServiceDetails',
66-
getServiceDetails,
67-
);
68-
6972
const requestMock = {
7073
...buildMessengerClientInitRequestMock(baseMessenger),
7174
controllerMessenger:
7275
getMoneyAccountUpgradeControllerMessenger(baseMessenger),
7376
initMessenger: getMoneyAccountUpgradeControllerInitMessenger(baseMessenger),
7477
};
7578

76-
return { requestMock, baseMessenger, getServiceDetails };
79+
return { requestMock, baseMessenger };
7780
}
7881

7982
const flushAsync = () => new Promise(process.nextTick);
@@ -92,15 +95,20 @@ describe('moneyAccountUpgradeControllerInit', () => {
9295
.mocked(MoneyAccountUpgradeController)
9396
.mockImplementation(() => mockedController);
9497

95-
jest.mocked(getDeleGatorEnvironment).mockReturnValue({
96-
EIP7702StatelessDeleGatorImpl: DELEGATOR_IMPL,
97-
caveatEnforcers: {
98-
RedeemerEnforcer: REDEEMER_ENFORCER,
99-
ValueLteEnforcer: VALUE_LTE_ENFORCER,
100-
},
101-
} as unknown as ReturnType<typeof getDeleGatorEnvironment>);
98+
(ReduxService as unknown as { store: { getState: jest.Mock } }).store = {
99+
getState: jest.fn().mockReturnValue({}),
100+
};
101+
jest.mocked(selectMoneyAccountVaultConfig).mockReturnValue(VAULT_CONFIG);
102+
jest.mocked(selectEvmNetworkConfigurationsByChainId).mockReturnValue({
103+
[VAULT_CHAIN_ID]: {
104+
chainId: VAULT_CHAIN_ID,
105+
} as never,
106+
});
102107
});
103108

109+
const mockAddNetwork = Engine.context.NetworkController
110+
.addNetwork as jest.Mock;
111+
104112
it('returns a MoneyAccountUpgradeController instance', () => {
105113
const { requestMock } = getInitRequestMock({ isUnlocked: false });
106114

@@ -126,23 +134,15 @@ describe('moneyAccountUpgradeControllerInit', () => {
126134
});
127135
});
128136

129-
it('initializes the controller with addresses from service details and the delegator environment when unlocked', async () => {
130-
const { requestMock, getServiceDetails } = getInitRequestMock({
131-
isUnlocked: true,
132-
});
137+
it('initializes the controller with the chainId and boring vault address from the vault config when unlocked', async () => {
138+
const { requestMock } = getInitRequestMock({ isUnlocked: true });
133139

134140
moneyAccountUpgradeControllerInit(requestMock);
135141
await flushAsync();
136142

137-
expect(getServiceDetails).toHaveBeenCalledWith([CHAIN_IDS.ARBITRUM]);
138-
expect(getDeleGatorEnvironment).toHaveBeenCalledWith(
139-
Number(CHAIN_IDS.ARBITRUM),
140-
);
141-
expect(mockedController.init).toHaveBeenCalledWith(CHAIN_IDS.ARBITRUM, {
142-
musdTokenAddress: MUSD_TOKEN_ADDRESS,
143-
delegatorImplAddress: DELEGATOR_IMPL,
144-
redeemerEnforcer: REDEEMER_ENFORCER,
145-
valueLteEnforcer: VALUE_LTE_ENFORCER,
143+
expect(mockedController.init).toHaveBeenCalledWith({
144+
chainId: VAULT_CHAIN_ID,
145+
boringVaultAddress: BORING_VAULT_ADDRESS,
146146
});
147147
});
148148

@@ -178,45 +178,71 @@ describe('moneyAccountUpgradeControllerInit', () => {
178178
expect(mockedController.init).toHaveBeenCalledTimes(1);
179179
});
180180

181-
it('logs an error when CHOMP service details are missing', async () => {
182-
const { requestMock } = getInitRequestMock({
183-
isUnlocked: true,
184-
serviceDetails: null,
185-
});
181+
it('logs an error when the vault config is missing', async () => {
182+
jest.mocked(selectMoneyAccountVaultConfig).mockReturnValue(undefined);
183+
const { requestMock } = getInitRequestMock({ isUnlocked: true });
186184

187185
moneyAccountUpgradeControllerInit(requestMock);
188186
await flushAsync();
189187

190188
expect(mockedController.init).not.toHaveBeenCalled();
191189
expect(Logger.error).toHaveBeenCalledWith(
192190
expect.objectContaining({
193-
message: `Missing CHOMP service details for chain ${CHAIN_IDS.ARBITRUM}`,
191+
message: 'Missing Money Account vault config',
194192
}),
195193
'MoneyAccountUpgradeController bootstrap',
196194
);
197195
});
198196

199-
it('logs an error when the MUSD token address is missing from service details', async () => {
200-
const { requestMock } = getInitRequestMock({
201-
isUnlocked: true,
202-
serviceDetails: {
203-
chains: {
204-
[CHAIN_IDS.ARBITRUM]: {
205-
protocol: { vedaProtocol: { supportedTokens: [] } },
206-
},
207-
},
208-
},
197+
describe('chain configuration', () => {
198+
it('does not add the chain if it is already configured', async () => {
199+
const { requestMock } = getInitRequestMock({ isUnlocked: true });
200+
201+
moneyAccountUpgradeControllerInit(requestMock);
202+
await flushAsync();
203+
204+
expect(mockAddNetwork).not.toHaveBeenCalled();
205+
expect(mockedController.init).toHaveBeenCalled();
209206
});
210207

211-
moneyAccountUpgradeControllerInit(requestMock);
212-
await flushAsync();
208+
it('adds the chain from PopularList when it is not configured', async () => {
209+
jest.mocked(selectEvmNetworkConfigurationsByChainId).mockReturnValue({});
213210

214-
expect(mockedController.init).not.toHaveBeenCalled();
215-
expect(Logger.error).toHaveBeenCalledWith(
216-
expect.objectContaining({
217-
message: `Missing MUSD token address for chain ${CHAIN_IDS.ARBITRUM} in CHOMP service details`,
218-
}),
219-
'MoneyAccountUpgradeController bootstrap',
220-
);
211+
const { requestMock } = getInitRequestMock({ isUnlocked: true });
212+
213+
moneyAccountUpgradeControllerInit(requestMock);
214+
await flushAsync();
215+
216+
expect(mockAddNetwork).toHaveBeenCalledWith(
217+
expect.objectContaining({
218+
chainId: VAULT_CHAIN_ID,
219+
name: 'Monad',
220+
}),
221+
);
222+
expect(mockedController.init).toHaveBeenCalled();
223+
});
224+
225+
it('logs an error when the missing chain is not in PopularList', async () => {
226+
const UNSUPPORTED_CHAIN_ID = '0xdeadbeef' as Hex;
227+
jest.mocked(selectMoneyAccountVaultConfig).mockReturnValue({
228+
...VAULT_CONFIG,
229+
chainId: UNSUPPORTED_CHAIN_ID,
230+
});
231+
jest.mocked(selectEvmNetworkConfigurationsByChainId).mockReturnValue({});
232+
233+
const { requestMock } = getInitRequestMock({ isUnlocked: true });
234+
235+
moneyAccountUpgradeControllerInit(requestMock);
236+
await flushAsync();
237+
238+
expect(mockAddNetwork).not.toHaveBeenCalled();
239+
expect(mockedController.init).not.toHaveBeenCalled();
240+
expect(Logger.error).toHaveBeenCalledWith(
241+
expect.objectContaining({
242+
message: expect.stringContaining(UNSUPPORTED_CHAIN_ID),
243+
}),
244+
'MoneyAccountUpgradeController bootstrap',
245+
);
246+
});
221247
});
222248
});

app/core/Engine/controllers/money-account-upgrade-controller-init.ts

Lines changed: 64 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,65 @@ import {
22
MoneyAccountUpgradeController,
33
type MoneyAccountUpgradeControllerMessenger,
44
} from '@metamask/money-account-upgrade-controller';
5-
import { type Hex, hexToNumber } from '@metamask/utils';
6-
import { CHAIN_IDS } from '@metamask/transaction-controller';
5+
import type { Hex } from '@metamask/utils';
6+
import { RpcEndpointType } from '@metamask/network-controller';
7+
import { toHex } from '@metamask/controller-utils';
78
import type { MessengerClientInitFunction } from '../types';
89
import type { MoneyAccountUpgradeControllerInitMessenger } from '../messengers/money-account-upgrade-controller-messenger';
9-
import { getDeleGatorEnvironment } from '../../Delegation/environment';
10+
import Engine from '../../Engine';
11+
import ReduxService from '../../redux';
12+
import type { RootState } from '../../../reducers';
13+
import { selectMoneyAccountVaultConfig } from '../../../selectors/featureFlagController/moneyAccount';
14+
import { selectEvmNetworkConfigurationsByChainId } from '../../../selectors/networkController';
15+
import { PopularList } from '../../../util/networks/customNetworks';
1016
import Logger from '../../../util/Logger';
1117

12-
// TODO: source this from a feature flag (parallel to ChompApiConfig.baseUrl)
13-
// so we can add/swap MUSD chains without a code deploy.
14-
const MUSD_CHAIN_ID: Hex = CHAIN_IDS.ARBITRUM;
18+
/**
19+
* Ensures the given chain exists in the user's NetworkController configuration.
20+
* If missing, adds it from `PopularList`. The upgrade flow's
21+
* `eip-7702-authorization` step calls
22+
* `NetworkController:findNetworkClientIdByChainId`, which throws if the chain
23+
* hasn't been configured, and Monad is not enabled by default
24+
* so we need to make sure it's there before init runs.
25+
*/
26+
const ensureChainConfigured = async (chainId: Hex): Promise<void> => {
27+
const networkConfigurations = selectEvmNetworkConfigurationsByChainId(
28+
ReduxService.store.getState() as RootState,
29+
);
30+
if (networkConfigurations[chainId]) {
31+
return;
32+
}
33+
34+
const popularEntry = PopularList.find(
35+
(network) => toHex(network.chainId as string) === chainId,
36+
);
37+
if (!popularEntry) {
38+
throw new Error(
39+
`Money Account upgrade chain ${chainId} is not in PopularList; cannot auto-add to NetworkController`,
40+
);
41+
}
42+
43+
await Engine.context.NetworkController.addNetwork({
44+
chainId,
45+
blockExplorerUrls: popularEntry.rpcPrefs?.blockExplorerUrl
46+
? [popularEntry.rpcPrefs.blockExplorerUrl]
47+
: [],
48+
defaultRpcEndpointIndex: 0,
49+
defaultBlockExplorerUrlIndex: popularEntry.rpcPrefs?.blockExplorerUrl
50+
? 0
51+
: undefined,
52+
name: popularEntry.nickname,
53+
nativeCurrency: popularEntry.ticker,
54+
rpcEndpoints: [
55+
{
56+
url: popularEntry.rpcUrl,
57+
failoverUrls: popularEntry.failoverRpcUrls,
58+
name: popularEntry.nickname,
59+
type: RpcEndpointType.Custom,
60+
},
61+
],
62+
});
63+
};
1564

1665
let bootstrapPromise: Promise<void> | null = null;
1766

@@ -61,33 +110,20 @@ export const moneyAccountUpgradeControllerInit: MessengerClientInitFunction<
61110
});
62111

63112
const bootstrap = async () => {
64-
const serviceDetails = await controllerMessenger.call(
65-
'ChompApiService:getServiceDetails',
66-
[MUSD_CHAIN_ID],
113+
const vaultConfig = selectMoneyAccountVaultConfig(
114+
ReduxService.store.getState() as RootState,
67115
);
68-
69-
if (!serviceDetails) {
70-
throw new Error(
71-
`Missing CHOMP service details for chain ${MUSD_CHAIN_ID}`,
72-
);
116+
if (!vaultConfig) {
117+
throw new Error('Missing Money Account vault config');
73118
}
74119

75-
const chain = serviceDetails.chains[MUSD_CHAIN_ID];
76-
const musdTokenAddress =
77-
chain?.protocol.vedaProtocol?.supportedTokens[0]?.tokenAddress;
78-
if (!musdTokenAddress) {
79-
throw new Error(
80-
`Missing MUSD token address for chain ${MUSD_CHAIN_ID} in CHOMP service details`,
81-
);
82-
}
120+
const chainId = vaultConfig.chainId as Hex;
83121

84-
const environment = getDeleGatorEnvironment(hexToNumber(MUSD_CHAIN_ID));
122+
await ensureChainConfigured(chainId);
85123

86-
await controller.init(MUSD_CHAIN_ID, {
87-
musdTokenAddress,
88-
delegatorImplAddress: environment.EIP7702StatelessDeleGatorImpl,
89-
redeemerEnforcer: environment.caveatEnforcers.RedeemerEnforcer,
90-
valueLteEnforcer: environment.caveatEnforcers.ValueLteEnforcer,
124+
await controller.init({
125+
chainId,
126+
boringVaultAddress: vaultConfig.boringVault as Hex,
91127
});
92128
};
93129

app/core/Engine/messengers/money-account-upgrade-controller-messenger.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,15 @@ export function getMoneyAccountUpgradeControllerMessenger(
3131
});
3232
rootMessenger.delegate({
3333
actions: [
34+
'AuthenticatedUserStorageService:createDelegation',
35+
'AuthenticatedUserStorageService:listDelegations',
3436
'ChompApiService:associateAddress',
37+
'ChompApiService:createIntents',
3538
'ChompApiService:createUpgrade',
39+
'ChompApiService:getIntentsByAddress',
3640
'ChompApiService:getServiceDetails',
41+
'ChompApiService:verifyDelegation',
42+
'DelegationController:signDelegation',
3743
'KeyringController:signEip7702Authorization',
3844
'KeyringController:signPersonalMessage',
3945
'NetworkController:findNetworkClientIdByChainId',

0 commit comments

Comments
 (0)