Skip to content

Commit b3e7eb8

Browse files
fix(notifications): suppress duplicate toasts for quickbuy trades (MetaMask#31462)
## **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? --> QuickBuy surfaces its own pending/complete/failed toasts. When a trade is submitted via an EIP-7702 smart account or a gas-included (`gasIncluded7702`) quote, `BridgeStatusController.submitTx` wraps the trade as a `TransactionType.batch` transaction. `batch` is in `REDESIGNED_TRANSACTION_TYPES`, so `NotificationManager` also fired the generic "Transaction submitted" and "Transaction #N complete" toasts on top of QuickBuy's own — the user saw **4 toasts instead of 2**. (This is why it doesn't reproduce on a plain EOA: those trades are typed `swap`/`bridge`, which are not redesigned types.) This PR scopes the suppression strictly to QuickBuy via inversion of control, so the main Swap/Bridge flows are unaffected: - New dependency-free module `app/core/notificationSkipPredicates.ts` holds a registry of "skip predicates". `NotificationManager.#shouldSkipNotification` consults it (guarded by try/catch so a throwing predicate can never break notifications). Keeping it in its own module means feature code can register without importing the heavy `NotificationManager` graph. - QuickBuy registers a predicate at the app root (`useQuickBuyToastRegistrations`) that matches: - **tracked trade ids** — covers the terminal complete/failed toast, and - **in-flight submissions** via a pre-submit marker (`beginQuickBuySubmission`/`endQuickBuySubmission`) — covers the *pending* toast, which fires mid-`submitTx` before the tx id is known. ## **Changelog** <!-- mms-check: type=changelog required=true --> CHANGELOG entry: Fixed QuickBuy showing duplicate transaction toasts for smart-account and gas-included trades ## **Related issues** <!-- mms-check: type=issue-link required=true --> Fixes: https://consensyssoftware.atlassian.net/browse/TSA-627 ## **Manual testing steps** <!-- mms-check: type=manual-testing required=true --> ```gherkin Feature: QuickBuy transaction toasts Scenario: User runs a QuickBuy with a smart (EIP-7702) account Given I have an EIP-7702 smart account enabled on the trade network And I open a token from the Social Leaderboard trader position view When I confirm a QuickBuy trade Then I see exactly one QuickBuy "Buying..." pending toast And after the trade settles I see exactly one QuickBuy "Bought" (or "Failed") toast And I do not see the generic "Transaction submitted" or "Transaction complete" toasts Scenario: User runs a QuickBuy on a plain EOA (regression) Given I have a standard externally-owned account When I confirm a QuickBuy trade Then I still see only the two QuickBuy toasts (pending, then terminal) Scenario: Main Swap/Bridge flows are unaffected (regression) Given I start a swap or bridge from the main Swap/Bridge UI When the transaction is submitted and confirmed Then the generic transaction toasts still appear as before ``` ## **Screenshots/Recordings** <!-- mms-check: type=screenshot required=true --> ### **Before** In some rare cases, this happens: https://github.com/user-attachments/assets/838826be-99a7-4101-873e-88173059fbc1 ### **After** This is what we want in all cases: https://github.com/user-attachments/assets/b49de0d5-a0e2-47cb-8eef-6b372ad0f79f ## **Pre-merge author checklist** <!-- mms-check: type=checklist required=true --> - [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) - [x] I've tested on Android - Ideally on a mid-range device; emulator is acceptable - [x] 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 - [x] 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** - [ ] 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. Made with [Cursor](https://cursor.com) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes global notification gating and briefly suppresses swap/bridge/batch notifications during QuickBuy submit; predicates are isolated and errors are caught so unrelated flows should still get generic toasts. > > **Overview** > Adds a **dependency-free notification skip registry** (`notificationSkipPredicates`) and wires **`NotificationManager`** to consult registered predicates (with try/catch) so feature flows can opt out of generic transaction toasts without core importing UI code. > > **QuickBuy** registers `isQuickBuyTransaction` at the app root and extends the trade tracker with: a **ref-counted in-flight submission marker** (`begin`/`end` around `submitTx`) for pending notifications before the tx id exists; **tracked and recently-settled tx ids** so suppression still applies when `NotificationManager` re-checks ~2s after confirm/fail; and **type-narrowed matching** (swap/bridge/batch) during submission so unrelated txs are not hidden. Terminal handling now **settles** trades via `markQuickBuyTradeSettled` instead of a plain untrack so duplicate generic success/error toasts do not appear on top of QuickBuy’s own toasts (notably EIP-7702 `batch` trades). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 270b0ec. 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 8f251d4 commit b3e7eb8

11 files changed

Lines changed: 560 additions & 10 deletions

app/components/Views/SocialLeaderboard/TraderPositionView/components/QuickBuy/hooks/useQuickBuyController.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,11 @@ import {
9595
SocialLeaderboardEventValues,
9696
} from '../../../../analytics';
9797
import { buildQuickBuyToastOptions } from '../quickBuyToastOptions';
98-
import { trackQuickBuyTrade } from '../quickBuyTradeTracker';
98+
import {
99+
trackQuickBuyTrade,
100+
beginQuickBuySubmission,
101+
endQuickBuySubmission,
102+
} from '../quickBuyTradeTracker';
99103
import { resolveQuickBuyTerminalToast } from '../resolveQuickBuyTerminalToast';
100104

101105
export type QuickBuyButtonError =
@@ -1061,6 +1065,11 @@ export function useQuickBuyController(
10611065
submitStartedAtRef.current ? Date.now() - submitStartedAtRef.current : 0;
10621066

10631067
try {
1068+
// Mark a QuickBuy submission as in flight BEFORE submitTx so the generic
1069+
// transaction notification (which fires mid-submit, before the tx id is
1070+
// known) is suppressed. The id-based tracker takes over for the terminal
1071+
// notification once submitTx resolves.
1072+
beginQuickBuySubmission();
10641073
dispatch(setIsSubmittingTx(true));
10651074
const submitResult = await Engine.context.BridgeStatusController.submitTx(
10661075
walletAddress,
@@ -1132,6 +1141,10 @@ export function useQuickBuyController(
11321141
});
11331142
}
11341143
} finally {
1144+
// Cleared after `trackQuickBuyTrade` (in the try block) has registered the
1145+
// tx id, so the predicate transitions from marker-based to id-based
1146+
// suppression with no coverage gap.
1147+
endQuickBuySubmission();
11351148
dispatch(setIsSubmittingTx(false));
11361149
}
11371150
}, [

app/components/Views/SocialLeaderboard/TraderPositionView/components/QuickBuy/hooks/useQuickBuyToastRegistrations.test.tsx

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,21 @@ import {
88
} from '../../../../../../../util/haptics';
99
import { buildQuickBuyToastOptions } from '../quickBuyToastOptions';
1010
import {
11+
clearSettledQuickBuyTrades,
1112
getTrackedQuickBuyTradeIds,
13+
isQuickBuyTransaction,
1214
trackQuickBuyTrade,
1315
untrackQuickBuyTrade,
1416
type TrackedQuickBuyTrade,
1517
} from '../quickBuyTradeTracker';
18+
import { registerNotificationSkipPredicate } from '../../../../../../../core/notificationSkipPredicates';
1619
import { useQuickBuyToastRegistrations } from './useQuickBuyToastRegistrations';
1720

21+
jest.mock('../../../../../../../core/notificationSkipPredicates', () => ({
22+
__esModule: true,
23+
registerNotificationSkipPredicate: jest.fn(() => jest.fn()),
24+
}));
25+
1826
jest.mock('../quickBuyToastOptions', () => ({
1927
buildQuickBuyToastOptions: jest.fn((kind: string) => ({ kind })),
2028
}));
@@ -101,10 +109,29 @@ describe('useQuickBuyToastRegistrations', () => {
101109
beforeEach(() => {
102110
jest.clearAllMocks();
103111
getTrackedQuickBuyTradeIds().forEach(untrackQuickBuyTrade);
112+
clearSettledQuickBuyTrades();
104113
Engine.context.MultichainTransactionsController.state.nonEvmTransactions =
105114
{};
106115
});
107116

117+
it('registers the QuickBuy notification skip predicate on mount and unregisters on unmount', () => {
118+
const unregister = jest.fn();
119+
(registerNotificationSkipPredicate as jest.Mock).mockReturnValue(
120+
unregister,
121+
);
122+
123+
const { unmount } = renderHook(() => useQuickBuyToastRegistrations());
124+
125+
expect(registerNotificationSkipPredicate).toHaveBeenCalledWith(
126+
isQuickBuyTransaction,
127+
);
128+
expect(unregister).not.toHaveBeenCalled();
129+
130+
unmount();
131+
132+
expect(unregister).toHaveBeenCalledTimes(1);
133+
});
134+
108135
it('subscribes to both the bridge and multichain state change events', () => {
109136
const { result } = renderHook(() => useQuickBuyToastRegistrations());
110137

app/components/Views/SocialLeaderboard/TraderPositionView/components/QuickBuy/hooks/useQuickBuyToastRegistrations.tsx

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
1-
import { useCallback, useMemo } from 'react';
1+
import { useCallback, useEffect, useMemo } from 'react';
22
import type { ToastRef } from '../../../../../../../component-library/components/Toast/Toast.types';
33
import { useAppThemeFromContext } from '../../../../../../../util/theme';
4+
import { registerNotificationSkipPredicate } from '../../../../../../../core/notificationSkipPredicates';
45
import type { ToastRegistration } from '../../../../../../Nav/App/ControllerEventToastBridge';
5-
import { getTrackedQuickBuyTradeIds } from '../quickBuyTradeTracker';
6+
import {
7+
getTrackedQuickBuyTradeIds,
8+
isQuickBuyTransaction,
9+
} from '../quickBuyTradeTracker';
610
import { resolveQuickBuyTerminalToast } from '../resolveQuickBuyTerminalToast';
711

812
/**
@@ -19,6 +23,11 @@ import { resolveQuickBuyTerminalToast } from '../resolveQuickBuyTerminalToast';
1923
export const useQuickBuyToastRegistrations = (): ToastRegistration[] => {
2024
const theme = useAppThemeFromContext();
2125

26+
// Opt QuickBuy-initiated transactions out of the generic transaction
27+
// notifications so the user only sees QuickBuy's own toasts. Registered at
28+
// the app root so it covers submissions regardless of sheet lifecycle.
29+
useEffect(() => registerNotificationSkipPredicate(isQuickBuyTransaction), []);
30+
2231
// Shared by both controller subscriptions: `resolveQuickBuyTerminalToast`
2332
// checks each tracked trade against whichever controller is authoritative for
2433
// it, so we can fan out the same scan regardless of which event fired.

app/components/Views/SocialLeaderboard/TraderPositionView/components/QuickBuy/quickBuyTradeTracker.test.ts

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,24 @@
11
import {
2+
TransactionType,
3+
type TransactionMeta,
4+
} from '@metamask/transaction-controller';
5+
import {
6+
beginQuickBuySubmission,
7+
clearSettledQuickBuyTrades,
8+
endQuickBuySubmission,
29
getTrackedQuickBuyTrade,
310
getTrackedQuickBuyTradeIds,
11+
hasPendingQuickBuySubmission,
12+
isQuickBuyTransaction,
13+
markQuickBuyTradeSettled,
414
trackQuickBuyTrade,
515
untrackQuickBuyTrade,
616
type TrackedQuickBuyTrade,
717
} from './quickBuyTradeTracker';
818

19+
const txMeta = (overrides: Partial<TransactionMeta>): TransactionMeta =>
20+
overrides as TransactionMeta;
21+
922
const buyTrade: TrackedQuickBuyTrade = {
1023
tradeMode: 'buy',
1124
tokenSymbol: 'PEPE',
@@ -24,6 +37,10 @@ const sellTrade: TrackedQuickBuyTrade = {
2437
describe('quickBuyTradeTracker', () => {
2538
afterEach(() => {
2639
getTrackedQuickBuyTradeIds().forEach(untrackQuickBuyTrade);
40+
clearSettledQuickBuyTrades();
41+
while (hasPendingQuickBuySubmission()) {
42+
endQuickBuySubmission();
43+
}
2744
});
2845

2946
it('stores and returns a tracked trade by tx meta id', () => {
@@ -84,4 +101,146 @@ describe('quickBuyTradeTracker', () => {
84101
expect(() => untrackQuickBuyTrade('missing')).not.toThrow();
85102
expect(getTrackedQuickBuyTradeIds()).toEqual(['tx-1']);
86103
});
104+
105+
describe('submission marker', () => {
106+
it('reflects in-flight submissions via the counter', () => {
107+
expect(hasPendingQuickBuySubmission()).toBe(false);
108+
109+
beginQuickBuySubmission();
110+
expect(hasPendingQuickBuySubmission()).toBe(true);
111+
112+
endQuickBuySubmission();
113+
expect(hasPendingQuickBuySubmission()).toBe(false);
114+
});
115+
116+
it('stays active until all overlapping submissions end', () => {
117+
beginQuickBuySubmission();
118+
beginQuickBuySubmission();
119+
120+
endQuickBuySubmission();
121+
expect(hasPendingQuickBuySubmission()).toBe(true);
122+
123+
endQuickBuySubmission();
124+
expect(hasPendingQuickBuySubmission()).toBe(false);
125+
});
126+
127+
it('never drops below zero on unbalanced ends', () => {
128+
endQuickBuySubmission();
129+
130+
expect(hasPendingQuickBuySubmission()).toBe(false);
131+
132+
beginQuickBuySubmission();
133+
expect(hasPendingQuickBuySubmission()).toBe(true);
134+
});
135+
});
136+
137+
describe('isQuickBuyTransaction', () => {
138+
it('matches a transaction already tracked by id', () => {
139+
trackQuickBuyTrade('tx-1', buyTrade);
140+
141+
expect(isQuickBuyTransaction(txMeta({ id: 'tx-1' }))).toBe(true);
142+
});
143+
144+
it('matches an in-flight swap/bridge/batch transaction during submission', () => {
145+
beginQuickBuySubmission();
146+
147+
expect(
148+
isQuickBuyTransaction(
149+
txMeta({ id: 'tx-9', type: TransactionType.batch }),
150+
),
151+
).toBe(true);
152+
expect(
153+
isQuickBuyTransaction(
154+
txMeta({ id: 'tx-9', type: TransactionType.swap }),
155+
),
156+
).toBe(true);
157+
expect(
158+
isQuickBuyTransaction(
159+
txMeta({ id: 'tx-9', type: TransactionType.bridge }),
160+
),
161+
).toBe(true);
162+
});
163+
164+
it('matches a batch whose nested transaction is a swap during submission', () => {
165+
beginQuickBuySubmission();
166+
167+
expect(
168+
isQuickBuyTransaction(
169+
txMeta({
170+
id: 'tx-9',
171+
type: TransactionType.batch,
172+
nestedTransactions: [{ type: TransactionType.swap }],
173+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
174+
} as any),
175+
),
176+
).toBe(true);
177+
});
178+
179+
it('does not match an unrelated in-flight transaction type', () => {
180+
beginQuickBuySubmission();
181+
182+
expect(
183+
isQuickBuyTransaction(
184+
txMeta({ id: 'tx-9', type: TransactionType.simpleSend }),
185+
),
186+
).toBe(false);
187+
});
188+
189+
it('does not match when there is no marker and no tracked id', () => {
190+
expect(
191+
isQuickBuyTransaction(
192+
txMeta({ id: 'tx-9', type: TransactionType.swap }),
193+
),
194+
).toBe(false);
195+
});
196+
197+
it('still matches a trade after it has settled (covers the delayed re-check)', () => {
198+
trackQuickBuyTrade('tx-1', buyTrade);
199+
markQuickBuyTradeSettled('tx-1');
200+
201+
expect(getTrackedQuickBuyTradeIds()).toEqual([]);
202+
expect(isQuickBuyTransaction(txMeta({ id: 'tx-1' }))).toBe(true);
203+
});
204+
205+
it('stops matching a settled trade once it is explicitly untracked', () => {
206+
trackQuickBuyTrade('tx-1', buyTrade);
207+
markQuickBuyTradeSettled('tx-1');
208+
untrackQuickBuyTrade('tx-1');
209+
210+
expect(isQuickBuyTransaction(txMeta({ id: 'tx-1' }))).toBe(false);
211+
});
212+
});
213+
214+
describe('markQuickBuyTradeSettled', () => {
215+
it('removes the trade from the active registry so the terminal toast does not repeat', () => {
216+
trackQuickBuyTrade('tx-1', buyTrade);
217+
218+
markQuickBuyTradeSettled('tx-1');
219+
220+
expect(getTrackedQuickBuyTrade('tx-1')).toBeUndefined();
221+
expect(getTrackedQuickBuyTradeIds()).toEqual([]);
222+
});
223+
224+
it('evicts the oldest settled id once the retention cap is exceeded', () => {
225+
for (let i = 0; i < 51; i += 1) {
226+
markQuickBuyTradeSettled(`tx-${i}`);
227+
}
228+
229+
expect(isQuickBuyTransaction(txMeta({ id: 'tx-0' }))).toBe(false);
230+
expect(isQuickBuyTransaction(txMeta({ id: 'tx-1' }))).toBe(true);
231+
expect(isQuickBuyTransaction(txMeta({ id: 'tx-50' }))).toBe(true);
232+
});
233+
234+
it('refreshes a re-settled id so it survives further eviction', () => {
235+
markQuickBuyTradeSettled('keep-me');
236+
for (let i = 0; i < 49; i += 1) {
237+
markQuickBuyTradeSettled(`tx-${i}`);
238+
}
239+
// Re-settle to move it to the newest position before overflowing.
240+
markQuickBuyTradeSettled('keep-me');
241+
markQuickBuyTradeSettled('overflow');
242+
243+
expect(isQuickBuyTransaction(txMeta({ id: 'keep-me' }))).toBe(true);
244+
});
245+
});
87246
});

0 commit comments

Comments
 (0)