Skip to content

Commit 8ea647f

Browse files
authored
fix: check for token address in delegation matching in upgrade step (MetaMask#9075)
## Explanation **Current state:** The `register-intents` step filters stored delegations by delegator address, delegate address, chain, and redeemer caveat before submitting them to CHOMP as intents. However, it does not filter by token address. This means that if the configured token addresses ever change (e.g. a boring vault contract is redeployed to a new address during development), any delegation written against the old address that still happens to carry a valid redeemer caveat will be picked up and registered as an intent against the new configuration — producing a stale, mismatched intent. **Solution:** A `matchesConfiguredToken` predicate is added to the `needsIntent` filter that checks the delegation's `tokenAddress` against the two currently-configured addresses (`musdTokenAddress` for deposits, `boringVaultAddress` for withdrawals). Only delegations whose token address matches one of these two values are eligible for intent registration. Delegations for any other token address are silently skipped. <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? Are there client or consumer pull requests to adopt any breaking changes? For example: * Fixes #12345 * Related to #67890 --> ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes which delegations are registered as CHOMP intents during account upgrade; incorrect filtering could skip valid intents or still allow mismatches, but the change is narrow and test-backed. > > **Overview** > **Fixes stale intent registration** when stored delegations still match delegator, delegate, chain, and redeemer caveats but point at an old token contract (e.g. a redeployed boring vault). > > The `register-intents` step now requires each delegation’s `metadata.tokenAddress` to match the current **mUSD** (`musdTokenAddress`) or **withdrawal vmUSD** (`boringVaultAddress`) before it is submitted to CHOMP. Other token addresses are skipped. A regression test covers a stale withdrawal delegation that would previously have been eligible. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit f8ed22f. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent fa2f9a1 commit 8ea647f

3 files changed

Lines changed: 41 additions & 0 deletions

File tree

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- Scope the `register-intents` step to the currently-configured token addresses (deposit mUSD / withdrawal vmUSD) so that stale delegations for previously-configured tokens are no longer registered as intents ([#9075](https://github.com/MetaMask/core/pull/9075))
13+
1014
## [2.0.4]
1115

1216
### Changed

packages/money-account-upgrade-controller/src/steps/register-intents.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,34 @@ describe('registerIntentsStep', () => {
432432
expect(mocks.createIntents).not.toHaveBeenCalled();
433433
});
434434

435+
it('ignores delegations for a token address that is no longer configured', async () => {
436+
const { messenger, mocks } = setup();
437+
mocks.listDelegations.mockResolvedValue([
438+
// Stale withdrawal delegation left over from a previous config (e.g. a
439+
// dev boring vault address) that still carries the current redeemer
440+
// caveat. It must not be registered against the current configuration.
441+
makeDelegationResponse({
442+
tokenAddress: OTHER_TOKEN,
443+
tokenSymbol: 'vmUSD',
444+
delegationHash: `0x${'04'.repeat(32)}`,
445+
type: 'cash-withdrawal',
446+
}),
447+
depositDelegation(),
448+
withdrawalDelegation(),
449+
]);
450+
451+
const result = await run(messenger);
452+
453+
expect(result).toBe('completed');
454+
const [submitted] = mocks.createIntents.mock.calls[0];
455+
expect(submitted).toHaveLength(2);
456+
expect(
457+
submitted.map(
458+
(intent: { delegationHash: Hex }) => intent.delegationHash,
459+
),
460+
).toStrictEqual([MOCK_MUSD_DELEGATION_HASH, MOCK_VMUSD_DELEGATION_HASH]);
461+
});
462+
435463
it('ignores delegations whose caveats do not include a redeemer caveat targeting the Veda vault adapter', async () => {
436464
const { messenger, mocks } = setup();
437465
mocks.listDelegations.mockResolvedValue([

packages/money-account-upgrade-controller/src/steps/register-intents.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,9 @@ export const registerIntentsStep: Step = {
5050
messenger,
5151
address,
5252
chainId,
53+
boringVaultAddress,
5354
delegateAddress,
55+
musdTokenAddress,
5456
redeemerEnforcer,
5557
vedaVaultAdapterAddress,
5658
}) {
@@ -70,10 +72,17 @@ export const registerIntentsStep: Step = {
7072
vedaVaultAdapterAddress,
7173
);
7274

75+
const configuredTokenAddresses = [musdTokenAddress, boringVaultAddress];
76+
const matchesConfiguredToken = (entry: DelegationResponse): boolean =>
77+
configuredTokenAddresses.some((tokenAddress) =>
78+
equalsIgnoreCase(entry.metadata.tokenAddress, tokenAddress),
79+
);
80+
7381
const needsIntent = (entry: DelegationResponse): boolean =>
7482
equalsIgnoreCase(entry.signedDelegation.delegator, address) &&
7583
equalsIgnoreCase(entry.signedDelegation.delegate, delegateAddress) &&
7684
equalsIgnoreCase(entry.metadata.chainIdHex, chainId) &&
85+
matchesConfiguredToken(entry) &&
7786
hasVedaRedeemerCaveat(entry) &&
7887
!activeIntentHashes.has(entry.metadata.delegationHash.toLowerCase());
7988

0 commit comments

Comments
 (0)