CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

Hey @caieu this is what i found

High severity

1. No safety timeout on isPaymentSelectorNavigationLocked

usePredictBuyConditions.ts (lines ~91–130)

The unlock pipeline is a strict blur → focus → schedule unlock sequence. The unlock is only scheduled when both isPaymentSelectorNavigationLockedRef.current === true and didBlurAfterPaymentSelectorOpenRef.current === true. The latter only flips inside the blur listener.

const unsubscribeFocus = navigation.addListener('focus', () => {
  if (
    isPaymentSelectorNavigationLockedRef.current &&
    didBlurAfterPaymentSelectorOpenRef.current
  ) {
    scheduleUnlock();
  }
});

Failure modes that leave the buy CTA permanently disabled:

• The Routes.CONFIRMATION_PAY_WITH_MODAL registration is changed to presentation: 'transparentModal' in the future — transparentModal does not always blur the screen behind it.
• The lock is set but the actual navigation.navigate(...) call is intercepted/throws/no-ops (e.g., guarded by another navigator).
• The user backgrounds the app between lock-set and modal-open; on resume some configurations skip the blur event for the predict screen.

Recovery from a stuck lock requires the user to back out of the screen entirely. There is no escape path within the buy flow.

Recommendation: schedule an unconditional safety unlock (e.g., 5 s) inside lockPaymentSelectorNavigation itself, in parallel with the existing blur/focus handshake. Whichever fires first wins. Or drop the didBlurAfterPaymentSelectorOpenRef gate from the focus handler and just unlock-on-focus when the lock is set.

Medium severity

2. Dead import: useTransactionPayQuotes

usePredictBuyInfo.ts (line 4)

import {
  useTransactionPayQuotes,
  useTransactionPayTotals,
} from '../../../../../Views/confirmations/hooks/pay/useTransactionPayData';

useTransactionPayQuotes is imported but never referenced anywhere in the file at PR HEAD. Either drop the import or wire up the consumer it was added for.

3. persistedBuyErrorBanner survives token changes

usePredictBuyError.ts (lines ~280–296)

The persisted banner is cleared only via clearBuyErrorBanner (user retries / closes / amount change reset). The PR description and Gherkin spec only list "user retries, changes the amount, or closes the sheet" as recovery paths — token change is not listed.

Practical scenario: user gets an order_failed banner for token A, opens the payment selector, picks token B. The banner referring to a token they no longer have selected stays visible.

Recommendation: clarify intent in the spec. If the banner should clear on token change, also reset persistedBuyErrorBanner when selectedPaymentToken identity changes.

4. Blocking pay-alert short-circuit no longer waits for fees to settle

usePredictBuyError.ts (lines ~80–91)

if (
  !isPlacingOrder &&
  !isConfirming &&
  !isPredictBalanceSelected &&
  !!blockingPayAlertMessage
) {
  return { status: 'error', error: blockingPayAlertMessage, source: 'blocking_pay_alert' };
}

Old code waited for !isPayFeesLoading before surfacing the alert. New code surfaces it eagerly. Intentional per PR description ("keeps transaction-pay alert text visible…including while quote/pay-fee loading is in progress") — but during a token switch the upstream alert hooks (useInsufficientPayTokenBalanceAlert, useNoPayTokenQuotesAlert) can briefly hold the previous token's alert text, so the user sees stale alert wording for one or two frames.

Recommendation: scope the alert hook input to the new selectedPaymentToken so it returns null until recomputed for the new token, or gate the message on a settled flag for the alert hooks themselves (mirroring what isPaySystemSettling does for the CTA).

5. computedDepositFee no longer guards on hasBlockingPayAlerts

usePredictBuyInfo.ts (lines 53–62)

Old behaviour: when there were blocking pay alerts, computedDepositFee returned 0 so the displayed Total didn't include a fee for an unsupported token.

New behaviour: deposit fee is added to Total even with a blocking alert.

This is intentional ("…the displayed Total now still includes the deposit fee so users can see the actual amount required by the selected payment token"), but the UX result is "Deposit fee: $X" displayed alongside an alert that says "We can't quote this token". Worth a quick design check that the row label or a contextual hint makes the situation unambiguous.

6. Edge case: lock can outlive its intended scope

usePredictBuyConditions.ts (lines ~91–130)

If the user opens the payment selector then leaves the Predict tab entirely (e.g., switches to Wallet or Activity), the blur fires for the predict screen, but focus may not re-fire on the predict screen for a while. Combined with the lack of a safety timeout (issue 1), the lock can persist for an unbounded duration. Not catastrophic on its own — covered by issue 1's recommendation.

Low severity

7. Deeply nested ternaries in CTA disabled / opacity

PredictBuyWithAnyToken.tsx (lines ~349–369)

const isBuyActionButtonDisabled = isPaymentSelectorNavigationLocked
  ? true
  : !isBannerActive && (isChangePaymentMode || isAddFundsMode)
    ? false
    : isBannerActive
      ? isRetrying || !preview
      : !canPlaceBet;

The two named consts isBuyActionButtonDisabled and showBuyActionButtonReducedOpacity follow the same shape but diverge subtly. Easy to drift out of sync in future edits. Consider a single getBuyButtonState() helper returning { disabled, reducedOpacity } so the two can't drift apart.

8. Magic-ish constant PAYMENT_SELECTOR_NAVIGATION_UNLOCK_DELAY_MS = 1000

usePredictBuyConditions.ts

At least named, but consider co-locating with other Predict timing constants (e.g., metamask-mobile/app/components/UI/Predict/constants/transactions.ts) and adding a one-line comment explaining why 1 s — long enough to outlast the new-quote loading flip, short enough to not feel laggy.

9. Optional chain on onPaymentSelectorOpen?.()

PredictPayWithRow.tsx (line 71)

Both call sites in this PR pass the callback. The ? is defensive only. If the callback is intended to be required for the row's correct lock behaviour, drop the ? and require the prop. Otherwise leave as-is and document that callers without it bypass the lock.

10. Type-coercion cleanup in hasValidDepositAmount

PredictPayWithAnyTokenInfo.tsx (line 91)

const hasValidDepositAmount = useMemo(
  () => depositAmount !== '' && Boolean(transactionMeta),
  [depositAmount, transactionMeta],
);

Codecov Report

❌ Patch coverage is 92.19858% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.62%. Comparing base (3751d9a) to head (88f4e70).
⚠️ Report is 51 commits behind head on main.

@@            Coverage Diff             @@
##             main   #30064      +/-   ##
==========================================
+ Coverage   81.54%   81.62%   +0.08%     
==========================================
  Files        5343     5369      +26     
  Lines      142128   143010     +882     
  Branches    32411    32644     +233     
==========================================
+ Hits       115899   116735     +836     
+ Misses      18299    18295       -4     
- Partials     7930     7980      +50     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🔍 Smart E2E Test Selection

• Selected E2E tags: SmokePredictions, SmokeWalletPlatform, SmokeConfirmations
• Selected Performance tags: None (no tests recommended)
• Risk Level: medium
• AI Confidence: 88%

View GitHub Actions results

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
96.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud