Skip to content

Commit 43b030d

Browse files
authored
fix(predict): prevent token selector from using stale approvals from other flows cp-7.73.0 (#28685)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **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? --> A user reported that the "Pay with" token selector on `PredictBuyWithAnyToken`wasn't allowing to select a different token. A transaction is needed to that work and the current logic could pick up pending approvals from unrelated flows (bridge, swap, send, etc.), allowing them to select tokens from the wrong transaction context. **Root cause:** `initPayWithAnyToken()` calls `addTransactionBatch()` directly without first rejecting existing pending approvals — unlike the standalone `predictDeposit` flow which goes through `useConfirmNavigation` and rejects all unapproved transactions before creating its own. When stale approvals existed, `useApprovalRequest()` returned the first (wrong) approval, and `PredictPayWithRow` enabled token selection based on a generic `transactionMeta` truthiness check. **Fix (defense-in-depth):** 1. **`usePredictBuyActions`** — added `rejectPendingTransactions()` in the `transitionEnd` handler before `initPayWithAnyToken()`, mirroring the cleanup pattern from `useConfirmNavigation`. 2. **`PredictPayWithRow`** — replaced the generic `transactionMeta` truthiness check with `hasTransactionType(transactionMeta, [TransactionType.predictDepositAndOrder])`, so the selector only enables for the correct transaction type. This branch also includes prior commits that handle no-quotes blocking alerts in the buy flow and disable the pay-with selector until transaction metadata is ready. ## **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** Fixes: ## **Manual testing steps** ```gherkin Feature: Predict buy-with-any-token token selector isolation Scenario: token selector stays disabled when a non-predict approval is pending Given user has an unconfirmed swap or bridge transaction pending And user navigates to a Predict market buy screen When the buy screen finishes loading Then the "Pay with" row does not show an arrow icon And tapping the "Pay with" row does not open the token selection modal Scenario: token selector enables after deposit-and-order batch is created Given user has no pending transactions And user navigates to a Predict market buy screen When the buy screen finishes loading and initPayWithAnyToken completes Then the "Pay with" row shows the arrow icon And tapping the "Pay with" row opens the token selection modal Scenario: stale approvals are rejected on buy screen entry Given user has an unconfirmed transaction from another flow And user navigates to a Predict market buy screen When the screen transition completes Then the stale unconfirmed transaction is rejected And a new deposit-and-order batch is created as the only pending approval ``` ## **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** - [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. ## **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. <!-- Generated with the help of the pr-description AI skill --> <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches transaction/approval handling in the Predict buy flow by programmatically rejecting unapproved transactions and tightening when the pay-with selector is enabled; regressions could affect in-flight approvals or block token selection. > > **Overview** > Prevents the `PredictBuyWithAnyToken` pay-with token selector from latching onto stale approvals from other flows. > > On screen entry, `usePredictBuyActions` now rejects all `unapproved` transactions before calling `initPayWithAnyToken`, and `PredictPayWithRow` only enables navigation/UI affordances when the current `transactionMeta` is a `TransactionType.predictDepositAndOrder` (otherwise it disables press, hides the arrow, and removes the muted background). The buy flow also propagates *blocking* pay alerts (insufficient balance / no quotes) via `usePredictBuyInfo` into `usePredictBuyConditions` and `usePredictBuyError` to disable placing bets and surface the correct blocking message, with updated tests. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 69b5397. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent e7ba37c commit 43b030d

File tree

11 files changed

+246
-102
lines changed

11 files changed

+246
-102
lines changed

app/components/UI/Predict/views/PredictBuyWithAnyToken/PredictBuyWithAnyToken.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,8 @@ const PredictBuyWithAnyToken = () => {
129129
depositFee,
130130
rewardsFeeAmount,
131131
totalPayForPredictBalance,
132+
hasBlockingPayAlerts,
133+
blockingPayAlertMessage,
132134
} = usePredictBuyInfo({
133135
currentValue,
134136
preview,
@@ -153,6 +155,7 @@ const PredictBuyWithAnyToken = () => {
153155
isConfirming,
154156
totalPayForPredictBalance,
155157
isInputFocused,
158+
hasBlockingPayAlerts,
156159
});
157160

158161
const { errorMessage, isOrderNotFilled, resetOrderNotFilled } =
@@ -166,6 +169,7 @@ const PredictBuyWithAnyToken = () => {
166169
isConfirming,
167170
isPayFeesLoading,
168171
isInputFocused,
172+
blockingPayAlertMessage,
169173
});
170174

171175
const { handleConfirm, placeOrder } = usePredictBuyActions({

app/components/UI/Predict/views/PredictBuyWithAnyToken/components/PredictPayWithRow/PredictPayWithRow.test.tsx

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,14 @@ jest.mock('../../../../../../../util/address', () => ({
5959
isHardwareAccount: jest.fn(() => false),
6060
}));
6161

62+
let mockHasTransactionType = true;
63+
jest.mock('../../../../../../Views/confirmations/utils/transaction', () => ({
64+
hasTransactionType: (transactionMeta: unknown) => {
65+
if (!transactionMeta) return false;
66+
return mockHasTransactionType;
67+
},
68+
}));
69+
6270
jest.mock('../../../../../../../../locales/i18n', () => ({
6371
strings: (key: string) => {
6472
if (key === 'confirm.label.pay_with') return 'Pay with';
@@ -93,6 +101,7 @@ describe('PredictPayWithRow', () => {
93101
mockIsPredictBalanceSelected = false;
94102
mockSelectedPaymentToken = null;
95103
mockIsHardwareAccount.mockReturnValue(false);
104+
mockHasTransactionType = true;
96105
});
97106

98107
it('renders label with payToken symbol', () => {
@@ -199,6 +208,56 @@ describe('PredictPayWithRow', () => {
199208
expect(screen.getByText('Pay with USDC')).toBeOnTheScreen();
200209
});
201210

211+
it('does not navigate when transactionMeta is null', () => {
212+
mockTransactionMeta = null;
213+
214+
renderWithProvider(<PredictPayWithRow />);
215+
fireEvent.press(screen.getByText('Pay with USDC'));
216+
217+
expect(mockNavigate).not.toHaveBeenCalled();
218+
});
219+
220+
it('hides arrow icon when transactionMeta is null', () => {
221+
mockTransactionMeta = null;
222+
223+
const { toJSON } = renderWithProvider(<PredictPayWithRow />);
224+
const tree = JSON.stringify(toJSON());
225+
226+
expect(tree).not.toContain('ArrowDown');
227+
});
228+
229+
it('applies muted background when canEdit is true', () => {
230+
const { toJSON } = renderWithProvider(<PredictPayWithRow />);
231+
const tree = JSON.stringify(toJSON());
232+
233+
expect(tree).toContain('backgroundColor');
234+
});
235+
236+
it('does not apply muted background when disabled', () => {
237+
const { toJSON } = renderWithProvider(<PredictPayWithRow disabled />);
238+
const tree = JSON.stringify(toJSON());
239+
240+
expect(tree).not.toContain('backgroundColor');
241+
});
242+
243+
it('does not apply muted background when transactionMeta is null', () => {
244+
mockTransactionMeta = null;
245+
246+
const { toJSON } = renderWithProvider(<PredictPayWithRow />);
247+
const tree = JSON.stringify(toJSON());
248+
249+
expect(tree).not.toContain('backgroundColor');
250+
});
251+
252+
it('does not apply muted background for hardware accounts', () => {
253+
mockIsHardwareAccount.mockReturnValue(true);
254+
255+
const { toJSON } = renderWithProvider(<PredictPayWithRow />);
256+
const tree = JSON.stringify(toJSON());
257+
258+
expect(tree).not.toContain('backgroundColor');
259+
});
260+
202261
it('renders predict balance first hint when external token selected', () => {
203262
mockIsPredictBalanceSelected = false;
204263

@@ -216,4 +275,22 @@ describe('PredictPayWithRow', () => {
216275
screen.queryByText('Predict balance used first'),
217276
).not.toBeOnTheScreen();
218277
});
278+
279+
it('does not navigate when transaction is not predictDepositAndOrder', () => {
280+
mockHasTransactionType = false;
281+
282+
renderWithProvider(<PredictPayWithRow />);
283+
fireEvent.press(screen.getByText('Pay with USDC'));
284+
285+
expect(mockNavigate).not.toHaveBeenCalled();
286+
});
287+
288+
it('hides arrow icon when transaction is not predictDepositAndOrder', () => {
289+
mockHasTransactionType = false;
290+
291+
const { toJSON } = renderWithProvider(<PredictPayWithRow />);
292+
const tree = JSON.stringify(toJSON());
293+
294+
expect(tree).not.toContain('ArrowDown');
295+
});
219296
});

app/components/UI/Predict/views/PredictBuyWithAnyToken/components/PredictPayWithRow/PredictPayWithRow.tsx

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
TextVariant,
1212
} from '@metamask/design-system-react-native';
1313
import { Hex } from '@metamask/utils';
14+
import { TransactionType } from '@metamask/transaction-controller';
1415
import { strings } from '../../../../../../../../locales/i18n';
1516
import Icon, {
1617
IconColor,
@@ -20,6 +21,7 @@ import Icon, {
2021
import Routes from '../../../../../../../constants/navigation/Routes';
2122
import { useTransactionPayToken } from '../../../../../../Views/confirmations/hooks/pay/useTransactionPayToken';
2223
import { useTransactionMetadataRequest } from '../../../../../../Views/confirmations/hooks/transactions/useTransactionMetadataRequest';
24+
import { hasTransactionType } from '../../../../../../Views/confirmations/utils/transaction';
2325
import { TokenIcon } from '../../../../../../Views/confirmations/components/token-icon';
2426
import { isHardwareAccount } from '../../../../../../../util/address';
2527
import { POLYGON_USDCE } from '../../../../../../Views/confirmations/constants/predict';
@@ -39,7 +41,13 @@ export function PredictPayWithRow({
3941
const { payToken } = useTransactionPayToken();
4042
const transactionMeta = useTransactionMetadataRequest();
4143
const from = transactionMeta?.txParams?.from;
42-
const canEdit = !isHardwareAccount((from as string) ?? '') && !disabled;
44+
const isPredictDepositAndOrder = hasTransactionType(transactionMeta, [
45+
TransactionType.predictDepositAndOrder,
46+
]);
47+
const canEdit =
48+
!isHardwareAccount((from as string) ?? '') &&
49+
!disabled &&
50+
isPredictDepositAndOrder;
4351
const { isPredictBalanceSelected, selectedPaymentToken } =
4452
usePredictPaymentToken();
4553

@@ -72,7 +80,7 @@ export function PredictPayWithRow({
7280
flexDirection={BoxFlexDirection.Row}
7381
alignItems={BoxAlignItems.Center}
7482
justifyContent={BoxJustifyContent.Center}
75-
twClassName={`rounded-full py-2 pl-[9px] pr-[16px] mt-2 ${disabled ? '' : 'bg-muted'} mx-auto`}
83+
twClassName={`rounded-full py-2 pl-[9px] pr-[16px] mt-2 ${!canEdit ? '' : 'bg-muted'} mx-auto`}
7684
gap={3}
7785
>
7886
{tokenIconAddress && tokenIconChainId && (

app/components/UI/Predict/views/PredictBuyWithAnyToken/hooks/usePredictBuyActions.test.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,8 @@ jest.mock('../../../../../Views/confirmations/hooks/useConfirmActions', () => ({
9090
}));
9191

9292
const mockClearActiveOrderTransactionId = jest.fn();
93+
const mockRejectRequest = jest.fn();
94+
const mockTransactions: { id: string; status: string }[] = [];
9395

9496
jest.mock('../../../hooks/usePredictActiveOrder', () => ({
9597
usePredictActiveOrder: () => ({
@@ -121,6 +123,16 @@ jest.mock('../../../../../../core/Engine', () => ({
121123
clearActiveOrderTransactionId: (...args: unknown[]) =>
122124
mockClearActiveOrderTransactionId(...args),
123125
},
126+
ApprovalController: {
127+
rejectRequest: (...args: unknown[]) => mockRejectRequest(...args),
128+
},
129+
TransactionController: {
130+
state: {
131+
get transactions() {
132+
return mockTransactions;
133+
},
134+
},
135+
},
124136
},
125137
}));
126138

@@ -154,6 +166,7 @@ describe('usePredictBuyActions', () => {
154166
mockTransitionEndCallbacks.length = 0;
155167
mockBeforeRemoveCallbacks.length = 0;
156168
mockAddListener.mockImplementation(createAddListenerMock());
169+
mockTransactions.length = 0;
157170
});
158171

159172
describe('mount effect', () => {
@@ -548,4 +561,52 @@ describe('usePredictBuyActions', () => {
548561
expect(mockDispatch).not.toHaveBeenCalledWith(StackActions.pop());
549562
});
550563
});
564+
565+
describe('pending transaction rejection', () => {
566+
it('rejects unapproved transactions before calling initPayWithAnyToken', () => {
567+
mockTransactions.push(
568+
{ id: 'tx-1', status: 'unapproved' },
569+
{ id: 'tx-2', status: 'unapproved' },
570+
);
571+
572+
renderHook(() => usePredictBuyActions(createDefaultParams()));
573+
574+
expect(mockRejectRequest).toHaveBeenCalledTimes(2);
575+
expect(mockRejectRequest).toHaveBeenCalledWith('tx-1', expect.anything());
576+
expect(mockRejectRequest).toHaveBeenCalledWith('tx-2', expect.anything());
577+
expect(mockInitPayWithAnyToken).toHaveBeenCalledTimes(1);
578+
});
579+
580+
it('does not reject already approved transactions', () => {
581+
mockTransactions.push(
582+
{ id: 'tx-1', status: 'confirmed' },
583+
{ id: 'tx-2', status: 'unapproved' },
584+
);
585+
586+
renderHook(() => usePredictBuyActions(createDefaultParams()));
587+
588+
expect(mockRejectRequest).toHaveBeenCalledTimes(1);
589+
expect(mockRejectRequest).toHaveBeenCalledWith('tx-2', expect.anything());
590+
});
591+
592+
it('proceeds with initPayWithAnyToken even if rejection throws', () => {
593+
mockTransactions.push({ id: 'tx-1', status: 'unapproved' });
594+
mockRejectRequest.mockImplementation(() => {
595+
throw new Error('Already resolved');
596+
});
597+
598+
renderHook(() => usePredictBuyActions(createDefaultParams()));
599+
600+
expect(mockInitPayWithAnyToken).toHaveBeenCalledTimes(1);
601+
});
602+
603+
it('does not reject transactions when pay with any token is disabled', () => {
604+
mockPayWithAnyTokenEnabled = false;
605+
mockTransactions.push({ id: 'tx-1', status: 'unapproved' });
606+
607+
renderHook(() => usePredictBuyActions(createDefaultParams()));
608+
609+
expect(mockRejectRequest).not.toHaveBeenCalled();
610+
});
611+
});
551612
});

app/components/UI/Predict/views/PredictBuyWithAnyToken/hooks/usePredictBuyActions.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import {
77
OrderPreview,
88
PlaceOrderParams,
99
} from '../../../types';
10+
import { TransactionStatus } from '@metamask/transaction-controller';
11+
import { providerErrors } from '@metamask/rpc-errors';
1012
import useApprovalRequest from '../../../../../Views/confirmations/hooks/useApprovalRequest';
1113
import { usePredictActiveOrder } from '../../../hooks/usePredictActiveOrder';
1214
import Engine from '../../../../../../core/Engine';
@@ -17,6 +19,28 @@ import { usePredictTrading } from '../../../hooks/usePredictTrading';
1719
import { PlaceOrderOutcome } from '../../../hooks/usePredictPlaceOrder';
1820
import { PREDICT_ERROR_CODES } from '../../../constants/errors';
1921
import { useConfirmActions } from '../../../../../Views/confirmations/hooks/useConfirmActions';
22+
23+
/**
24+
* Rejects all unapproved transactions to prevent stale approvals from
25+
* interfering with the new deposit-and-order transaction batch.
26+
* Mirrors the cleanup logic in useConfirmNavigation.
27+
*/
28+
function rejectPendingTransactions() {
29+
const { ApprovalController, TransactionController } = Engine.context;
30+
const unapprovedTxs = TransactionController.state.transactions.filter(
31+
(tx) => tx.status === TransactionStatus.unapproved,
32+
);
33+
for (const tx of unapprovedTxs) {
34+
try {
35+
ApprovalController.rejectRequest(
36+
tx.id,
37+
providerErrors.userRejectedRequest(),
38+
);
39+
} catch {
40+
// Approval may already be resolved
41+
}
42+
}
43+
}
2044
interface UsePredictBuyActionsParams {
2145
preview?: OrderPreview | null;
2246
analyticsProperties: PlaceOrderParams['analyticsProperties'];
@@ -65,6 +89,7 @@ export const usePredictBuyActions = ({
6589
const unsubscribe = navigation.addListener('transitionEnd', (e) => {
6690
if (!e.data.closing && !hasInitializedPayWithAnyTokenRef.current) {
6791
hasInitializedPayWithAnyTokenRef.current = true;
92+
rejectPendingTransactions();
6893
initPayWithAnyToken();
6994
}
7095
});

app/components/UI/Predict/views/PredictBuyWithAnyToken/hooks/usePredictBuyConditions.test.ts

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ let mockSelectedPaymentToken: {
2020
chainId?: string;
2121
} | null = null;
2222
let mockIsDepositPending = false;
23-
let mockInsufficientPayTokenBalanceAlert: { message: string } | null = null;
23+
2424
let mockPredictBalance = 0;
2525
const mockResetSelectedPaymentToken = jest.fn();
2626

@@ -58,15 +58,6 @@ jest.mock('../../../hooks/usePredictDeposit', () => ({
5858
}),
5959
}));
6060

61-
jest.mock(
62-
'../../../../../Views/confirmations/hooks/alerts/useInsufficientPayTokenBalanceAlert',
63-
() => ({
64-
useInsufficientPayTokenBalanceAlert: () => [
65-
mockInsufficientPayTokenBalanceAlert,
66-
],
67-
}),
68-
);
69-
7061
jest.mock(
7162
'../../../../../Views/confirmations/hooks/pay/useTransactionPayData',
7263
() => ({
@@ -91,6 +82,7 @@ const defaultParams = {
9182
isConfirming: false,
9283
totalPayForPredictBalance: 0,
9384
isInputFocused: false,
85+
hasBlockingPayAlerts: false,
9486
};
9587

9688
describe('usePredictBuyConditions', () => {
@@ -107,7 +99,7 @@ describe('usePredictBuyConditions', () => {
10799
mockIsPredictBalanceSelected = true;
108100
mockSelectedPaymentToken = null;
109101
mockIsDepositPending = false;
110-
mockInsufficientPayTokenBalanceAlert = null;
102+
111103
mockPredictBalance = 0;
112104
});
113105

@@ -362,12 +354,12 @@ describe('usePredictBuyConditions', () => {
362354

363355
it('returns false when external payment token balance is insufficient', () => {
364356
mockIsPredictBalanceSelected = false;
365-
mockInsufficientPayTokenBalanceAlert = {
366-
message: 'Insufficient payment token balance',
367-
};
368357

369358
const { result } = renderHook(() =>
370-
usePredictBuyConditions(defaultParams),
359+
usePredictBuyConditions({
360+
...defaultParams,
361+
hasBlockingPayAlerts: true,
362+
}),
371363
);
372364

373365
expect(result.current.canPlaceBet).toBe(false);

0 commit comments

Comments
 (0)