Skip to content

Commit 46e048f

Browse files
fix(transaction-pay-controller): guard fiat post-ramp second leg against locked keyring and CHOMP races (MetaMask#9267)
## Explanation Two failure modes for the direct mUSD Money Account vault deposit are addressed in this PR. **Locked keyring at submit time** When a fiat order completes and the post-ramp second leg is triggered (direct mUSD vault deposit, Relay nested-calldata, or simple relay), the wallet may be locked — for example if the user backgrounded the app during Apple Pay checkout and the auto-lock timer fired. In that state `KeyringController.keyrings` is empty, `doesAccountSupportEIP7702` returns `false` for the Money Account address, and `addTransactionBatch` (called with `disableHook` and `disableSequential`) throws `Account does not support EIP-7702`, failing the deposit. A `waitForKeyringUnlock` guard is added at the top of `submitRelayAfterFiatCompletion` in `fiat-submit.ts`. Before any second-leg logic runs the function checks `KeyringController:getState().isUnlocked`. If already unlocked it proceeds immediately; if locked it subscribes to `KeyringController:unlock` and waits indefinitely, resuming only after the user authenticates. `KeyringControllerUnlockEvent` is added to `AllowedEvents` in `types.ts` to allow the subscription. **CHOMP auto-vault race** CHOMP is a backend service that may auto-vault mUSD from the Money Account independently of the extension/mobile submit path. If CHOMP runs concurrently with the checkout flow the vault deposit that `addTransactionBatch` tries to submit may conflict with a deposit CHOMP already completed, causing a double-vault. A CHOMP idempotency check is added in `submitDirectMusdVaultDeposit` using a single `eth_getLogs` call that scans for recent mUSD Transfer-out events from the Money Account. The baseline block is derived from the ramps settlement tx receipt (already fetched for the amount) so no additional network request is needed. The check runs before `addTransactionBatch` (skip the submit entirely) and again in the catch path (CHOMP may have won the race just before submit). Detection errors are swallowed so they never break the normal vault path. ## References ## 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 - [ ] I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes the fiat post-ramp and direct mUSD vault submission path (signing, batch submit, and on-chain idempotency) and adds a new messenger event permission for clients; incorrect CHOMP matching could skip or mis-attribute a vault hash. > > **Overview** > Fixes two post–fiat-ramp failure modes for the **direct mUSD Money Account vault** path and other fiat second legs. > > **Locked keyring:** After the on-ramp order completes, the controller now calls `waitForKeyringUnlock` before `submitRelayAfterFiatCompletion` (relay, nested calldata, and direct mUSD). If the wallet is locked it subscribes to `KeyringController:unlock` and resumes only after unlock, avoiding false EIP-7702 / `addTransactionBatch` failures when the user was away during checkout. `KeyringControllerUnlockEvent` is added to `AllowedEvents` so clients must grant that subscription. > > **CHOMP race:** New `findRecentChompVaultDeposit` scans Monad `eth_getLogs` for mUSD `Transfer` events from the Money Account (newest first, amount ≥ settled raw). `submitDirectMusdVaultDeposit` runs this **before** `addTransactionBatch` (skip submit and return the CHOMP tx hash) and **again on batch failure** (treat CHOMP as success). Scan errors are swallowed so the normal vault path still runs. The log window starts at the ramps settlement receipt block: `resolveSourceAmountRaw` / `getTransferredAmountFromTxHash` now return `{ amountRaw, fromBlock }` / `{ amountRaw, blockNumber }` instead of a bare string, with no extra RPC when the receipt was already fetched for ERC-20 amounts. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit f307c21. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 13f811b commit 46e048f

12 files changed

Lines changed: 811 additions & 63 deletions

File tree

packages/transaction-pay-controller/CHANGELOG.md

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

88
## [Unreleased]
99

10+
### Added
11+
12+
- Add CHOMP idempotency for direct mUSD vault deposits ([#9267](https://github.com/MetaMask/core/pull/9267))
13+
14+
### Fixed
15+
16+
- Wait for keyring unlock before executing fiat post-ramp second leg ([#9267](https://github.com/MetaMask/core/pull/9267))
17+
1018
## [23.16.1]
1119

1220
### Changed
Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
import type { Hex } from '@metamask/utils';
2+
3+
import { CHAIN_ID_MONAD, MUSD_MONAD_ADDRESS } from '../../constants';
4+
import type { TransactionPayControllerMessenger } from '../../types';
5+
import { rpcRequest } from '../../utils/provider';
6+
import { findRecentChompVaultDeposit } from './chomp';
7+
8+
jest.mock('../../utils/provider');
9+
10+
const MONEY_ACCOUNT_ADDRESS =
11+
'0x1111111111111111111111111111111111111111' as Hex;
12+
const CHOMP_TX_HASH =
13+
'0xdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef' as Hex;
14+
const FROM_BLOCK = '0x100' as Hex;
15+
const SOURCE_AMOUNT_RAW = '5000000'; // 5 mUSD (6 decimals)
16+
// uint256 hex for 5000000 (>= source amount)
17+
const TRANSFER_DATA_SUFFICIENT =
18+
'0x00000000000000000000000000000000000000000000000000000000004c4b40';
19+
// uint256 hex for 4999999 (< source amount)
20+
const TRANSFER_DATA_INSUFFICIENT =
21+
'0x00000000000000000000000000000000000000000000000000000000004c4b3f';
22+
23+
const ERC20_TRANSFER_TOPIC =
24+
'0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef';
25+
26+
function padAddress(address: string): string {
27+
return `0x${address.replace(/^0x/u, '').toLowerCase().padStart(64, '0')}`;
28+
}
29+
30+
const MONEY_ACCOUNT_PADDED = padAddress(MONEY_ACCOUNT_ADDRESS);
31+
32+
function buildMusdTransferLog(
33+
txHash: Hex = CHOMP_TX_HASH,
34+
data: string = TRANSFER_DATA_SUFFICIENT,
35+
): {
36+
address: string;
37+
topics: string[];
38+
data: string;
39+
transactionHash: Hex;
40+
} {
41+
return {
42+
address: MUSD_MONAD_ADDRESS,
43+
data,
44+
topics: [
45+
ERC20_TRANSFER_TOPIC,
46+
MONEY_ACCOUNT_PADDED,
47+
padAddress('0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'),
48+
],
49+
transactionHash: txHash,
50+
};
51+
}
52+
53+
function buildMessenger(): TransactionPayControllerMessenger {
54+
return {} as TransactionPayControllerMessenger;
55+
}
56+
57+
describe('chomp', () => {
58+
const rpcRequestMock = jest.mocked(rpcRequest);
59+
60+
beforeEach(() => {
61+
jest.resetAllMocks();
62+
});
63+
64+
describe('findRecentChompVaultDeposit', () => {
65+
it('returns the CHOMP tx hash when a Transfer log with sufficient amount is found', async () => {
66+
rpcRequestMock.mockResolvedValueOnce([buildMusdTransferLog()]);
67+
68+
const result = await findRecentChompVaultDeposit({
69+
fromBlock: FROM_BLOCK,
70+
messenger: buildMessenger(),
71+
moneyAccountAddress: MONEY_ACCOUNT_ADDRESS,
72+
sourceAmountRaw: SOURCE_AMOUNT_RAW,
73+
});
74+
75+
expect(result).toBe(CHOMP_TX_HASH);
76+
// Only eth_getLogs should have been called.
77+
expect(rpcRequestMock).toHaveBeenCalledTimes(1);
78+
});
79+
80+
it('returns undefined when the mUSD transfer amount is below the required amount', async () => {
81+
rpcRequestMock.mockResolvedValueOnce([
82+
buildMusdTransferLog(CHOMP_TX_HASH, TRANSFER_DATA_INSUFFICIENT),
83+
]);
84+
85+
const result = await findRecentChompVaultDeposit({
86+
fromBlock: FROM_BLOCK,
87+
messenger: buildMessenger(),
88+
moneyAccountAddress: MONEY_ACCOUNT_ADDRESS,
89+
sourceAmountRaw: SOURCE_AMOUNT_RAW,
90+
});
91+
92+
expect(result).toBeUndefined();
93+
expect(rpcRequestMock).toHaveBeenCalledTimes(1);
94+
});
95+
96+
it('returns undefined when no mUSD Transfer logs are found', async () => {
97+
rpcRequestMock.mockResolvedValueOnce([]);
98+
99+
const result = await findRecentChompVaultDeposit({
100+
fromBlock: FROM_BLOCK,
101+
messenger: buildMessenger(),
102+
moneyAccountAddress: MONEY_ACCOUNT_ADDRESS,
103+
sourceAmountRaw: SOURCE_AMOUNT_RAW,
104+
});
105+
106+
expect(result).toBeUndefined();
107+
expect(rpcRequestMock).toHaveBeenCalledTimes(1);
108+
});
109+
110+
it('queries eth_getLogs with the correct filter', async () => {
111+
rpcRequestMock.mockResolvedValueOnce([]);
112+
113+
await findRecentChompVaultDeposit({
114+
fromBlock: FROM_BLOCK,
115+
messenger: buildMessenger(),
116+
moneyAccountAddress: MONEY_ACCOUNT_ADDRESS,
117+
sourceAmountRaw: SOURCE_AMOUNT_RAW,
118+
});
119+
120+
expect(rpcRequestMock).toHaveBeenCalledWith(
121+
expect.objectContaining({
122+
chainId: CHAIN_ID_MONAD,
123+
method: 'eth_getLogs',
124+
params: [
125+
expect.objectContaining({
126+
address: MUSD_MONAD_ADDRESS,
127+
fromBlock: FROM_BLOCK,
128+
toBlock: 'latest',
129+
topics: [ERC20_TRANSFER_TOPIC, MONEY_ACCOUNT_PADDED, null],
130+
}),
131+
],
132+
}),
133+
);
134+
});
135+
136+
it('processes logs newest-first and returns the most recent match', async () => {
137+
const olderHash =
138+
'0x0000000000000000000000000000000000000000000000000000000000000001' as Hex;
139+
const newerHash =
140+
'0x0000000000000000000000000000000000000000000000000000000000000002' as Hex;
141+
142+
rpcRequestMock.mockResolvedValueOnce([
143+
buildMusdTransferLog(olderHash),
144+
buildMusdTransferLog(newerHash),
145+
]);
146+
147+
const result = await findRecentChompVaultDeposit({
148+
fromBlock: FROM_BLOCK,
149+
messenger: buildMessenger(),
150+
moneyAccountAddress: MONEY_ACCOUNT_ADDRESS,
151+
sourceAmountRaw: SOURCE_AMOUNT_RAW,
152+
});
153+
154+
expect(result).toBe(newerHash);
155+
expect(rpcRequestMock).toHaveBeenCalledTimes(1);
156+
});
157+
158+
it('skips logs with insufficient amount and returns the first sufficient one', async () => {
159+
const insufficientHash =
160+
'0x0000000000000000000000000000000000000000000000000000000000000001' as Hex;
161+
162+
rpcRequestMock.mockResolvedValueOnce([
163+
buildMusdTransferLog(insufficientHash, TRANSFER_DATA_INSUFFICIENT),
164+
buildMusdTransferLog(CHOMP_TX_HASH),
165+
]);
166+
167+
const result = await findRecentChompVaultDeposit({
168+
fromBlock: FROM_BLOCK,
169+
messenger: buildMessenger(),
170+
moneyAccountAddress: MONEY_ACCOUNT_ADDRESS,
171+
sourceAmountRaw: SOURCE_AMOUNT_RAW,
172+
});
173+
174+
// Logs reversed: CHOMP_TX_HASH checked first (newer), passes amount check.
175+
expect(result).toBe(CHOMP_TX_HASH);
176+
expect(rpcRequestMock).toHaveBeenCalledTimes(1);
177+
});
178+
179+
it('treats a log with data "0x" as zero amount and skips it', async () => {
180+
rpcRequestMock.mockResolvedValueOnce([
181+
buildMusdTransferLog(CHOMP_TX_HASH, '0x'),
182+
]);
183+
184+
const result = await findRecentChompVaultDeposit({
185+
fromBlock: FROM_BLOCK,
186+
messenger: buildMessenger(),
187+
moneyAccountAddress: MONEY_ACCOUNT_ADDRESS,
188+
sourceAmountRaw: SOURCE_AMOUNT_RAW,
189+
});
190+
191+
expect(result).toBeUndefined();
192+
});
193+
});
194+
});
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
import type { Hex } from '@metamask/utils';
2+
import { createModuleLogger } from '@metamask/utils';
3+
4+
import { CHAIN_ID_MONAD, MUSD_MONAD_ADDRESS } from '../../constants';
5+
import { projectLogger } from '../../logger';
6+
import type { TransactionPayControllerMessenger } from '../../types';
7+
import { rpcRequest } from '../../utils/provider';
8+
9+
const log = createModuleLogger(projectLogger, 'chomp');
10+
11+
/** keccak256('Transfer(address,address,uint256)') */
12+
const ERC20_TRANSFER_TOPIC =
13+
'0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef';
14+
15+
type RpcLog = {
16+
address: string;
17+
topics: string[];
18+
data: string;
19+
transactionHash: Hex;
20+
};
21+
22+
export async function findRecentChompVaultDeposit({
23+
messenger,
24+
moneyAccountAddress,
25+
sourceAmountRaw,
26+
fromBlock,
27+
}: {
28+
messenger: TransactionPayControllerMessenger;
29+
moneyAccountAddress: Hex;
30+
sourceAmountRaw: string;
31+
fromBlock: Hex;
32+
}): Promise<Hex | undefined> {
33+
const fromPadded = padAddress(moneyAccountAddress);
34+
35+
const logs = await rpcRequest<RpcLog[]>({
36+
messenger,
37+
chainId: CHAIN_ID_MONAD,
38+
method: 'eth_getLogs',
39+
params: [
40+
{
41+
address: MUSD_MONAD_ADDRESS,
42+
fromBlock,
43+
toBlock: 'latest',
44+
topics: [ERC20_TRANSFER_TOPIC, fromPadded, null],
45+
},
46+
],
47+
});
48+
49+
log('CHOMP scan: mUSD Transfer logs found', {
50+
count: logs.length,
51+
fromBlock,
52+
moneyAccountAddress,
53+
});
54+
55+
const requiredAmount = BigInt(sourceAmountRaw);
56+
57+
// Examine newest logs first so we return the most recent CHOMP match.
58+
for (const txLog of [...logs].reverse()) {
59+
const transferAmount = BigInt(txLog.data === '0x' ? '0x0' : txLog.data);
60+
61+
if (transferAmount < requiredAmount) {
62+
log('CHOMP scan: skipping log — transfer amount below required', {
63+
requiredAmount: requiredAmount.toString(),
64+
transferAmount: transferAmount.toString(),
65+
txHash: txLog.transactionHash,
66+
});
67+
continue;
68+
}
69+
70+
log('CHOMP scan: match found', {
71+
moneyAccountAddress,
72+
sourceAmountRaw,
73+
transferAmount: transferAmount.toString(),
74+
txHash: txLog.transactionHash,
75+
});
76+
77+
return txLog.transactionHash;
78+
}
79+
80+
log('CHOMP scan: no match found', { fromBlock, moneyAccountAddress });
81+
return undefined;
82+
}
83+
84+
function padAddress(address: Hex): string {
85+
return `0x${address.replace(/^0x/u, '').toLowerCase().padStart(64, '0')}`;
86+
}

0 commit comments

Comments
 (0)