Skip to content

Commit 0e264ec

Browse files
refactor(card): B1 foundation – race-safe tx confirmation + Money Account delegation (#30000)
<!-- 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** <!-- 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? --> This is the **B1 foundation** branch of the Money Account ↔ Card linkage stack. It establishes clean controller and utility infrastructure that the subsequent hook/UX branches (B2, B3) will build on. ### Why The existing `useCardDelegation` hook contained two structural problems: 1. **Race condition** – the EVM approval flow subscribed to `TransactionController:transactionConfirmed` _after_ calling `addTransaction`. A fast-confirming transaction would fire the event before the subscription was registered, silently leaving the delegation incomplete. 2. **Inline SIWE message formatting** – the hook built the Sign-In-With-Ethereum message itself, duplicating provider-specific logic (EVM vs Solana wording, chain-ID mapping, expiration rules) that lives in the Baanx provider. Additionally, `CardController` had no way to orchestrate the full Money Account delegation flow (SIWE sign + background ERC-20 approval + `approveFunding` callback) without pushing that logic into a UI hook. ### What | Layer | Change | |---|---| | **`awaitTransactionConfirmed` util** | New utility that subscribes to `TransactionController:transactionConfirmed` **before** submitting the transaction, stashes events that arrive while waiting for the tx ID, then replays them once the ID is known. Handles timeout (5 min default) and graceful unsubscription. | | **`moneyAccountCardToken` util** | New utility to resolve the Monad USDC `CardFundingToken` out of the provider's delegation-settings response; drives `linkMoneyAccountCard` without touching the UI layer. | | **Messenger delegation** | `TransactionController:transactionConfirmed` is now delegated to `CardControllerMessenger` so `CardController` can subscribe natively without reaching through the root messenger. | | **`ICardProvider.generateCardDelegationSignatureMessage`** | New optional method on the provider interface; implemented in `BaanxProvider` with correct EVM (includes `Expiration Time`) and Solana (omits it, uses Solana wording) variants. | | **`CardController.generateCardDelegationSignatureMessage`** | Thin public wrapper that forwards to the active provider — the single source of SIWE message truth accessible to both the controller and UI hooks. | | **`CardController.linkMoneyAccountCard`** | New controller method that orchestrates the full background MA linkage: validates input → resolves token → fetches delegation challenge → signs SIWE message via `KeyringController` → submits ERC-20 approval with `requireApproval: false` using `awaitTransactionConfirmed` → calls `provider.approveFunding`. | | **`useCardDelegation` (EVM path)** | Replaces the `subscribeOnceIf`-after-`addTransaction` pattern with `awaitTransactionConfirmed`, eliminating the race condition. SIWE message generation now delegates to `CardController.generateCardDelegationSignatureMessage`. | All changes are covered by unit tests (190 passing). ## **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: null ## **Related issues** <!-- Link a real GitHub or Jira issue using `Fixes:`, `Closes:`, or `Refs:`. If no issue exists, write an explicit one-line rationale. --> No issue: internal refactor foundation for the Money Account ↔ Card linkage stack (B1 of a three-branch stack). ## **Manual testing steps** <!-- This branch contains no user-visible behaviour changes (no new UI, no flag changes). All observable paths are covered by unit tests. --> N/A ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** N/A ### **After** N/A ## **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. --> - [x] 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). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] 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. <!-- Generated with the help of the pr-description AI skill --> <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Medium risk because it changes transaction-confirmation waiting behavior (new subscribe/unsubscribe + failure/timeout handling) and adds a new background allowance-approval flow in `CardController` that signs messages and submits transactions without user confirmation. > > **Overview** > Introduces a new `awaitTransactionConfirmed` utility that subscribes to `TransactionController:transactionConfirmed` and `TransactionController:transactionFailed` *before* submitting a transaction, stashes early events, enforces a timeout, and always unsubscribes. > > Moves SIWE/delegation signature message construction out of `useCardDelegation` into the provider layer via `ICardProvider.generateCardDelegationSignatureMessage` (implemented in `BaanxProvider`) and exposes it through `CardController.generateCardDelegationSignatureMessage`; `useCardDelegation` now uses this plus `awaitTransactionConfirmed` for EVM approvals and listens for `transactionFailed` events. > > Adds `CardController.linkMoneyAccountCard` to orchestrate Money Account �4 Card linkage (resolve Monad USDC from delegation settings, fetch challenge, sign message, submit ERC-20 approve with `requireApproval:false`, then call `approveFunding`), plus a `moneyAccountCardToken` resolver util and messenger/event delegation updates. Also hardens spending-limit token list building by making the SDK token lookup callback non-optional, and skips notifications for `MMM_CARD`-origin transactions. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 26851d8. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 17e338a commit 0e264ec

19 files changed

Lines changed: 1679 additions & 619 deletions

app/components/UI/Card/hooks/useCardDelegation.test.ts

Lines changed: 97 additions & 131 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ jest.mock('../../../../util/address', () => ({
5252

5353
const mockFetchDelegationChallenge = jest.fn();
5454
const mockApproveFunding = jest.fn();
55+
const mockGenerateCardDelegationSignatureMessage = jest.fn();
5556

5657
jest.mock('../../../../core/Engine', () => ({
5758
context: {
@@ -65,6 +66,8 @@ jest.mock('../../../../core/Engine', () => ({
6566
fetchDelegationChallenge: (...args: unknown[]) =>
6667
mockFetchDelegationChallenge(...args),
6768
approveFunding: (...args: unknown[]) => mockApproveFunding(...args),
69+
generateCardDelegationSignatureMessage: (...args: unknown[]) =>
70+
mockGenerateCardDelegationSignatureMessage(...args),
6871
},
6972
},
7073
controllerMessenger: {
@@ -169,6 +172,13 @@ describe('useCardDelegation', () => {
169172
});
170173
mockApproveFunding.mockResolvedValue(undefined);
171174

175+
// Default SIWE message returned by the (provider-backed) controller.
176+
// Tests that need a different shape (e.g. Solana wording) override this.
177+
mockGenerateCardDelegationSignatureMessage.mockReset();
178+
mockGenerateCardDelegationSignatureMessage.mockReturnValue(
179+
'mocked-siwe-message',
180+
);
181+
172182
// Setup metrics mock
173183
mockBuild = jest.fn().mockReturnValue({ event: 'mock-event' });
174184
mockAddProperties = jest.fn().mockReturnValue({ build: mockBuild });
@@ -209,18 +219,21 @@ describe('useCardDelegation', () => {
209219
ensureNetworkExists: mockEnsureNetworkExists,
210220
});
211221

212-
// Setup controllerMessenger mock to simulate transaction confirmation
213-
Engine.controllerMessenger.subscribeOnceIf = jest
222+
// Setup controllerMessenger mock to simulate transaction confirmation via
223+
// the awaitTransactionConfirmed util (subscribe/unsubscribe pattern).
224+
Engine.controllerMessenger.subscribe = jest
214225
.fn()
215-
.mockImplementation((_eventName, callback, _filter) => {
216-
// Immediately call the callback with a confirmed transaction
217-
setImmediate(() => {
218-
callback({
219-
id: 'transaction-meta-id-123',
220-
status: TransactionStatus.confirmed,
226+
.mockImplementation((eventName, callback) => {
227+
if (eventName === 'TransactionController:transactionConfirmed') {
228+
setImmediate(() => {
229+
callback({
230+
id: 'transaction-meta-id-123',
231+
status: TransactionStatus.confirmed,
232+
});
221233
});
222-
});
234+
}
223235
});
236+
Engine.controllerMessenger.unsubscribe = jest.fn();
224237

225238
// Setup utility mocks
226239
mockToTokenMinimalUnit.mockReturnValue(100000000000000000000n);
@@ -602,17 +615,8 @@ describe('useCardDelegation', () => {
602615
const mockToken = createMockToken();
603616
const params = createMockDelegationParams();
604617

605-
Engine.controllerMessenger.subscribeOnceIf = jest
606-
.fn()
607-
.mockImplementation((_eventName, callback) => {
608-
// Immediately call the callback with a confirmed transaction
609-
setImmediate(() => {
610-
callback({
611-
id: 'transaction-meta-id-123',
612-
status: TransactionStatus.confirmed,
613-
});
614-
});
615-
});
618+
// Default beforeEach already emits a confirmed event on subscribe;
619+
// approveFunding will reject after confirmation completes.
616620

617621
const { result } = renderHook(() => useCardDelegation(mockToken));
618622

@@ -872,8 +876,8 @@ describe('useCardDelegation', () => {
872876
});
873877

874878
describe('generateSignatureMessage', () => {
875-
it('generates multi-line SIWE message for EVM networks', async () => {
876-
const mockToken = createMockToken();
879+
it('forwards EVM args to CardController.generateCardDelegationSignatureMessage and signs the returned message', async () => {
880+
const mockToken = createMockToken({ caipChainId: 'eip155:59144' });
877881
const params = createMockDelegationParams();
878882

879883
const { result } = renderHook(() => useCardDelegation(mockToken));
@@ -882,6 +886,13 @@ describe('useCardDelegation', () => {
882886
await result.current.submitDelegation(params);
883887
});
884888

889+
expect(mockGenerateCardDelegationSignatureMessage).toHaveBeenCalledWith({
890+
network: params.network,
891+
address: mockAddress,
892+
nonce: mockNonce,
893+
caipChainId: 'eip155:59144',
894+
});
895+
885896
const mockSignPersonalMessage = Engine.context.KeyringController
886897
.signPersonalMessage as jest.MockedFunction<
887898
typeof Engine.context.KeyringController.signPersonalMessage
@@ -892,26 +903,10 @@ describe('useCardDelegation', () => {
892903
signedMessageHex.slice(2),
893904
'hex',
894905
).toString('utf8');
895-
896-
// Verify content
897-
expect(signedMessage).toContain(`${mockAddress}`);
898-
expect(signedMessage).toContain('Chain ID: 59144');
899-
expect(signedMessage).toContain(`Nonce: ${mockNonce}`);
900-
expect(signedMessage).toContain(
901-
'metamask.app.link wants you to sign in with your Ethereum account:',
902-
);
903-
904-
// Verify multi-line format for EVM (contains newlines)
905-
expect(signedMessage).toContain('\n');
906-
expect(signedMessage).toContain('\nURI:');
907-
expect(signedMessage).toContain('\nVersion:');
908-
expect(signedMessage).toContain('\nChain ID:');
909-
expect(signedMessage).toContain('\nNonce:');
910-
expect(signedMessage).toContain('\nIssued At:');
911-
expect(signedMessage).toContain('\nExpiration Time:');
906+
expect(signedMessage).toBe('mocked-siwe-message');
912907
});
913908

914-
it('generates SIWE message for Solana network', async () => {
909+
it('forwards Solana args to CardController.generateCardDelegationSignatureMessage and signs via the Solana snap', async () => {
915910
const mockToken = createMockToken();
916911
const mockSolanaAddress = 'SolanaAddress123ABC';
917912
const mockAccountId = 'solana-account-uuid-123';
@@ -928,12 +923,14 @@ describe('useCardDelegation', () => {
928923
}),
929924
);
930925

931-
// Mock handleSnapRequest for both signCardMessage and approveCardAmount
926+
mockGenerateCardDelegationSignatureMessage.mockReturnValue(
927+
'mocked-solana-siwe',
928+
);
929+
932930
mockHandleSnapRequest
933-
.mockResolvedValueOnce({ signature: 'mock-solana-signature' }) // signCardMessage
934-
.mockResolvedValueOnce({ signature: mockTxSignature }); // approveCardAmount
931+
.mockResolvedValueOnce({ signature: 'mock-solana-signature' })
932+
.mockResolvedValueOnce({ signature: mockTxSignature });
935933

936-
// Mock controllerMessenger.subscribe for Solana stateChange
937934
(Engine.controllerMessenger.subscribe as jest.Mock).mockImplementation(
938935
(eventName: string, callback: (state: unknown) => void) => {
939936
if (eventName === 'MultichainTransactionsController:stateChange') {
@@ -963,34 +960,22 @@ describe('useCardDelegation', () => {
963960
await result.current.submitDelegation(params);
964961
});
965962

966-
// Get the message that was passed to signCardMessage
967-
// handleSnapRequest is called with (controllerMessenger, requestObject)
963+
expect(mockGenerateCardDelegationSignatureMessage).toHaveBeenCalledWith(
964+
expect.objectContaining({
965+
network: 'solana',
966+
address: mockSolanaAddress,
967+
nonce: mockNonce,
968+
}),
969+
);
970+
968971
const signCardMessageCall = mockHandleSnapRequest.mock.calls[0];
969972
const requestObject = signCardMessageCall[1];
970973
const base64Message = requestObject.request.params.message;
971-
972-
// Decode from base64
973974
const message = Buffer.from(base64Message, 'base64').toString('utf8');
974-
975-
// Verify content
976-
expect(message).toContain(mockSolanaAddress);
977-
expect(message).toContain(
978-
'metamask.app.link wants you to sign in with your Solana account:',
979-
);
980-
expect(message).toContain(`Nonce: ${mockNonce}`);
981-
expect(message).toContain('Chain ID: 1');
982-
expect(message).toContain('URI: https://metamask.app.link');
983-
expect(message).toContain('Version: 1');
984-
expect(message).toContain('Issued At:');
985-
986-
// Verify multi-line format (same structure as EVM, but without Expiration Time)
987-
expect(message).toContain('\nURI:');
988-
expect(message).toContain('\nVersion:');
989-
expect(message).toContain('\nChain ID:');
990-
expect(message).not.toContain('Expiration Time:');
975+
expect(message).toBe('mocked-solana-siwe');
991976
});
992977

993-
it('extracts chain ID from token caipChainId', async () => {
978+
it('forwards token caipChainId to the controller', async () => {
994979
const mockToken = createMockToken({ caipChainId: 'eip155:1' });
995980
const params = createMockDelegationParams();
996981

@@ -1000,21 +985,12 @@ describe('useCardDelegation', () => {
1000985
await result.current.submitDelegation(params);
1001986
});
1002987

1003-
const mockSignPersonalMessage = Engine.context.KeyringController
1004-
.signPersonalMessage as jest.MockedFunction<
1005-
typeof Engine.context.KeyringController.signPersonalMessage
1006-
>;
1007-
const signCallArgs = mockSignPersonalMessage.mock.calls[0][0];
1008-
const signedMessageHex = signCallArgs.data;
1009-
const signedMessage = Buffer.from(
1010-
signedMessageHex.slice(2),
1011-
'hex',
1012-
).toString('utf8');
1013-
1014-
expect(signedMessage).toContain('Chain ID: 1');
988+
expect(mockGenerateCardDelegationSignatureMessage).toHaveBeenCalledWith(
989+
expect.objectContaining({ caipChainId: 'eip155:1' }),
990+
);
1015991
});
1016992

1017-
it('uses default chain ID when caipChainId is not provided', async () => {
993+
it('forwards undefined caipChainId when the token has none', async () => {
1018994
const mockToken = createMockToken({ caipChainId: undefined });
1019995
const params = createMockDelegationParams();
1020996

@@ -1024,18 +1000,9 @@ describe('useCardDelegation', () => {
10241000
await result.current.submitDelegation(params);
10251001
});
10261002

1027-
const mockSignPersonalMessage = Engine.context.KeyringController
1028-
.signPersonalMessage as jest.MockedFunction<
1029-
typeof Engine.context.KeyringController.signPersonalMessage
1030-
>;
1031-
const signCallArgs = mockSignPersonalMessage.mock.calls[0][0];
1032-
const signedMessageHex = signCallArgs.data;
1033-
const signedMessage = Buffer.from(
1034-
signedMessageHex.slice(2),
1035-
'hex',
1036-
).toString('utf8');
1037-
1038-
expect(signedMessage).toContain('Chain ID: 59144');
1003+
expect(mockGenerateCardDelegationSignatureMessage).toHaveBeenCalledWith(
1004+
expect.objectContaining({ caipChainId: undefined }),
1005+
);
10391006
});
10401007
});
10411008

@@ -1094,9 +1061,12 @@ describe('useCardDelegation', () => {
10941061
await result.current.submitDelegation(params);
10951062
});
10961063

1097-
expect(Engine.controllerMessenger.subscribeOnceIf).toHaveBeenCalledWith(
1064+
expect(Engine.controllerMessenger.subscribe).toHaveBeenCalledWith(
10981065
'TransactionController:transactionConfirmed',
10991066
expect.any(Function),
1067+
);
1068+
expect(Engine.controllerMessenger.unsubscribe).toHaveBeenCalledWith(
1069+
'TransactionController:transactionConfirmed',
11001070
expect.any(Function),
11011071
);
11021072
});
@@ -1109,16 +1079,18 @@ describe('useCardDelegation', () => {
11091079
resolveConfirmation = resolve;
11101080
});
11111081

1112-
Engine.controllerMessenger.subscribeOnceIf = jest
1113-
.fn()
1114-
.mockImplementation((_eventName, callback) => {
1115-
confirmationPromise.then(() => {
1116-
callback({
1117-
id: 'transaction-meta-id-123',
1118-
status: TransactionStatus.confirmed,
1082+
(Engine.controllerMessenger.subscribe as jest.Mock).mockImplementation(
1083+
(eventName, callback) => {
1084+
if (eventName === 'TransactionController:transactionConfirmed') {
1085+
confirmationPromise.then(() => {
1086+
callback({
1087+
id: 'transaction-meta-id-123',
1088+
status: TransactionStatus.confirmed,
1089+
});
11191090
});
1120-
});
1121-
});
1091+
}
1092+
},
1093+
);
11221094

11231095
const { result } = renderHook(() => useCardDelegation(mockToken));
11241096

@@ -1150,24 +1122,22 @@ describe('useCardDelegation', () => {
11501122
expect(mockApproveFunding).toHaveBeenCalled();
11511123
});
11521124

1153-
it('handles transaction failure in confirmation listener', async () => {
1125+
it('handles transaction failure via transactionFailed event', async () => {
11541126
const mockToken = createMockToken();
11551127
const params = createMockDelegationParams();
11561128

1157-
Engine.controllerMessenger.subscribeOnceIf = jest
1158-
.fn()
1159-
.mockImplementation((_eventName, callback) => {
1160-
// Immediately call the callback with a failed transaction
1161-
setImmediate(() => {
1162-
callback({
1163-
id: 'transaction-meta-id-123',
1164-
status: TransactionStatus.failed,
1165-
error: {
1166-
message: 'Transaction execution failed',
1167-
},
1129+
(Engine.controllerMessenger.subscribe as jest.Mock).mockImplementation(
1130+
(eventName, callback) => {
1131+
if (eventName === 'TransactionController:transactionFailed') {
1132+
setImmediate(() => {
1133+
callback({
1134+
error: 'Transaction execution failed',
1135+
transactionMeta: { id: 'transaction-meta-id-123' },
1136+
});
11681137
});
1169-
});
1170-
});
1138+
}
1139+
},
1140+
);
11711141

11721142
const { result } = renderHook(() => useCardDelegation(mockToken));
11731143

@@ -1178,29 +1148,25 @@ describe('useCardDelegation', () => {
11781148
});
11791149

11801150
expect(result.current.error).toBe('Transaction execution failed');
1181-
expect(Logger.error).toHaveBeenCalledWith(
1182-
expect.any(Error),
1183-
'Transaction failed',
1184-
);
11851151
expect(mockApproveFunding).not.toHaveBeenCalled();
11861152
});
11871153

1188-
it('handles transaction failure with generic message when error details not provided', async () => {
1154+
it('handles transaction failure with generic message when no error string provided', async () => {
11891155
const mockToken = createMockToken();
11901156
const params = createMockDelegationParams();
11911157

1192-
Engine.controllerMessenger.subscribeOnceIf = jest
1193-
.fn()
1194-
.mockImplementation((_eventName, callback) => {
1195-
// Immediately call the callback with a failed transaction without error details
1196-
setImmediate(() => {
1197-
callback({
1198-
id: 'transaction-meta-id-123',
1199-
status: TransactionStatus.failed,
1200-
error: undefined,
1158+
(Engine.controllerMessenger.subscribe as jest.Mock).mockImplementation(
1159+
(eventName, callback) => {
1160+
if (eventName === 'TransactionController:transactionFailed') {
1161+
setImmediate(() => {
1162+
callback({
1163+
error: '',
1164+
transactionMeta: { id: 'transaction-meta-id-123' },
1165+
});
12011166
});
1202-
});
1203-
});
1167+
}
1168+
},
1169+
);
12041170

12051171
const { result } = renderHook(() => useCardDelegation(mockToken));
12061172

0 commit comments

Comments
 (0)