Skip to content

fix(money): skip insufficient-funds alert on Max money account deposit cp-8.5.0 - #33836

Merged
jpuri merged 4 commits into
mainfrom
fix/money-account-max-deposit-insufficient-funds
Jul 27, 2026
Merged

fix(money): skip insufficient-funds alert on Max money account deposit cp-8.5.0#33836
jpuri merged 4 commits into
mainfrom
fix/money-account-max-deposit-insufficient-funds

Conversation

@jpuri

@jpuri jpuri commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Description

A Max money account deposit derives its amount from a balance snapshot while the insufficient-funds alert compares against a live balance. Fiat rounding between the two could push the amount marginally above the live balance and falsely show "Insufficient funds", even though the submitted token amount is clamped to the actual raw balance.

Track the Max deposit selection via ConfirmationContext (isMaxDeposit) and have useInsufficientPayTokenBalanceAlert skip the input check for a Max money account deposit.

Changelog

CHANGELOG entry:

Related issues

Fixes: https://consensyssoftware.atlassian.net/browse/CONF-1730

Manual testing steps

NA

Screenshots/Recordings

NA

Pre-merge author checklist

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 to import wallets with many accounts and tokens
  • I've instrumented key operations with Sentry traces for production performance metrics

For performance guidelines and tooling, see the Performance Guide.

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.

Note

Low Risk
Localized confirmation/money-deposit UX fix with tests; no auth, payment submission, or broad alert logic changes beyond the max-deposit exception.

Overview
Fixes a false Insufficient funds blocking alert when users choose Max on a money account deposit. The Max amount comes from a balance snapshot, but the alert compares fiat against a live balance, so tiny rounding gaps could block confirm even though the on-chain amount is clamped to the real balance.

ConfirmationContext now exposes isMaxDeposit / setIsMaxDeposit. useTransactionCustomAmount sets the flag when 100% is chosen for a money account deposit and clears it on manual edits or pay-token changes. useInsufficientPayTokenBalanceAlert skips the input-balance check only for moneyAccountDeposit when isMaxDeposit is true; other flows are unchanged.

Tests cover context state, alert behavior for max vs non-max deposits, and updated confirmation mocks.

Reviewed by Cursor Bugbot for commit ac519e2. Bugbot is set up for automated code reviews on this repo. Configure here.

A Max money account deposit derives its amount from a balance snapshot
while the insufficient-funds alert compares against a live balance. Fiat
rounding between the two could push the amount marginally above the live
balance and falsely show "Insufficient funds", even though the submitted
token amount is clamped to the actual raw balance.

Track the Max deposit selection via ConfirmationContext (isMaxDeposit)
and have useInsufficientPayTokenBalanceAlert skip the input check for a
Max money account deposit.
@jpuri jpuri added the team-confirmations Push issues to confirmations team label Jul 27, 2026
@jpuri
jpuri requested a review from a team as a code owner July 27, 2026 10:40
@jpuri jpuri added the no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed label Jul 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

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.

@jpuri
jpuri enabled auto-merge July 27, 2026 10:40
@github-actions github-actions Bot added the risk:low AI analysis: low risk label Jul 27, 2026
jpuri added 2 commits July 27, 2026 16:20
A Max money account deposit derives its amount from a balance snapshot
while the insufficient-funds alert compares against a live balance. Fiat
rounding between the two could push the amount marginally above the live
balance and falsely show "Insufficient funds", even though the submitted
token amount is clamped to the actual raw balance.

Track the Max deposit selection via ConfirmationContext (isMaxDeposit)
and have useInsufficientPayTokenBalanceAlert skip the input check for a
Max money account deposit.
@github-actions github-actions Bot added risk:medium AI analysis: medium risk and removed risk:low AI analysis: low risk labels Jul 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeConfirmations, SmokeMoney
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: medium
  • AI Confidence: 90%
click to see 🤖 AI reasoning details

E2E Test Selection:
The PR makes targeted changes to the confirmation system for money account deposits:

  1. confirmation-context.tsx: Adds isMaxDeposit boolean state and setIsMaxDeposit setter to the shared ConfirmationContext. This is a new field in a widely-used context, so any component consuming this context could be affected.

  2. useTransactionCustomAmount.ts: Integrates setIsMaxDeposit to flag when a user selects the maximum amount for a money account deposit (100% of balance). Resets the flag on manual edits or pay token changes.

  3. useInsufficientPayTokenBalanceAlert.ts: Uses the new isMaxDeposit flag to skip the insufficient-funds alert for max money account deposits, preventing false positives caused by fiat rounding drift between the balance snapshot and live balance.

Tag Selection:

  • SmokeMoney: Directly affected — the fix targets money account deposit flows (MetaMask Card/fiat on-ramp). The max deposit amount selection and insufficient-funds alert behavior are core to this flow.
  • SmokeConfirmations: The ConfirmationContext is shared across all confirmation types. The alert system (useInsufficientPayTokenBalanceAlert) and custom amount hook (useTransactionCustomAmount) are used in confirmation flows broadly. Changes to the context interface need validation across confirmation types.

Not selected:

  • Other tags (SmokeSwap, SmokeStake, SmokeNetworkExpansion, etc.) are not directly impacted — the changes are scoped to money account deposit logic and the confirmation context extension, which doesn't affect non-deposit confirmation flows in a breaking way.

Performance tests: No performance-sensitive code paths are changed. The changes are purely logic/state management for alert suppression in a specific deposit scenario.

Performance Test Selection:
The changes are limited to state management in the confirmation context (adding isMaxDeposit flag) and alert logic (skipping insufficient-funds alert for max deposits). No rendering performance, asset loading, or app launch paths are affected. No performance test files were changed.

View GitHub Actions results

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Flaky unit test detection

Run history flaky detection

View recent run history

Historical failure rate is a hint, not proof — review each suggestion in context. See the flaky-test-detection skill for the full pattern reference and manual audit workflow.

Failures / runs sampled per window:

File 7d 15d 30d
app/components/Views/confirmations/components/footer/footer.test.tsx 0/176 0/294 0/294

AI-detected flaky patterns

app/components/Views/confirmations/components/footer/footer.test.tsx

  • J10 — jest.spyOn without restoreAllMocks in afterEach (medium)
    • Multiple tests (e.g. the QR signing and camera permission tests) use jest.spyOn(QRHardwareHook, 'useQRHardwareContext') to mock the hook but there is no afterEach that calls jest.restoreAllMocks(). Spies persist across tests and can cause non-deterministic behavior depending on execution order (matches J10 exactly; history showed 0 failures but pattern is present). No other J1-J10 patterns matched in any of the 4 files (all use beforeEach with clearAllMocks or resetAllMocks, waitFor calls contain assertions, no fake timers with waitFor, no arbitrary sleeps, no mutable module lets, no incomplete stores, no non-deterministic APIs).
    • Suggested fix in app/components/Views/confirmations/components/footer/footer.test.tsx:
      -  beforeEach(() => {
      -    jest.clearAllMocks();
      -
      -    mockUseConfirmationContext.mockReturnValue({
      -      mmPayRequestInProgressNavHandler: { current: false },
      -      headlessBuyError: undefined,
      -      isFooterVisible: true,
      -      isConfirmationSubmitting: false,
      -      isConfirmationSubmittingRef: { current: false },
      -      isHeadlessBuyInProgress: false,
      -      isTransactionDataUpdating: false,
      -      isTransactionValueUpdating: false,
      -      setHeadlessBuyError: jest.fn(),
      -      setIsFooterVisible: jest.fn(),
      -      setIsConfirmationSubmitting: jest.fn(),
      -      setIsHeadlessBuyInProgress: jest.fn(),
      -      setIsTransactionDataUpdating: jest.fn(),
      -      setIsTransactionValueUpdating: jest.fn(),
      -      isMaxDeposit: false,
      -      setIsMaxDeposit: jest.fn(),
      -    });
      -
      -    (useAlerts as jest.Mock).mockReturnValue({
      -      fieldAlerts: [],
      -      hasDangerAlerts: false,
      -      setAlertConfirmed: jest.fn(),
      -    });
      -
      -    (useAlertsConfirmed as jest.Mock).mockReturnValue({
      -      hasUnconfirmedDangerAlerts: false,
      -    });
      -
      -    useIsTransactionPayLoadingMock.mockReturnValue(false);
      -    useIsGaslessLoadingMock.mockReturnValue({ isGaslessLoading: false });
      -  });
      +  beforeEach(() => {
      +    jest.clearAllMocks();
      +
      +    mockUseConfirmationContext.mockReturnValue({
      +      mmPayRequestInProgressNavHandler: { current: false },
      +      headlessBuyError: undefined,
      +      isFooterVisible: true,
      +      isConfirmationSubmitting: false,
      +      isConfirmationSubmittingRef: { current: false },
      +      isHeadlessBuyInProgress: false,
      +      isTransactionDataUpdating: false,
      +      isTransactionValueUpdating: false,
      +      setHeadlessBuyError: jest.fn(),
      +      setIsFooterVisible: jest.fn(),
      +      setIsConfirmationSubmitting: jest.fn(),
      +      setIsHeadlessBuyInProgress: jest.fn(),
      +      setIsTransactionDataUpdating: jest.fn(),
      +      setIsTransactionValueUpdating: jest.fn(),
      +      isMaxDeposit: false,
      +      setIsMaxDeposit: jest.fn(),
      +    });
      +
      +    (useAlerts as jest.Mock).mockReturnValue({
      +      fieldAlerts: [],
      +      hasDangerAlerts: false,
      +      setAlertConfirmed: jest.fn(),
      +    });
      +
      +    (useAlertsConfirmed as jest.Mock).mockReturnValue({
      +      hasUnconfirmedDangerAlerts: false,
      +    });
      +
      +    useIsTransactionPayLoadingMock.mockReturnValue(false);
      +    useIsGaslessLoadingMock.mockReturnValue({ isGaslessLoading: false });
      +  });
      +
      +  afterEach(() => {
      +    jest.restoreAllMocks();
      +  });

This check is informational only and does not block merging.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.77%. Comparing base (a3e7109) to head (ac519e2).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff            @@
##             main   #33836    +/-   ##
========================================
  Coverage   84.76%   84.77%            
========================================
  Files        6244     6245     +1     
  Lines      168009   168125   +116     
  Branches    41120    41145    +25     
========================================
+ Hits       142420   142528   +108     
- Misses      15848    15855     +7     
- Partials     9741     9742     +1     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sonarqubecloud

Copy link
Copy Markdown

@jpuri
jpuri added this pull request to the merge queue Jul 27, 2026
Merged via the queue into main with commit e8c08ba Jul 27, 2026
135 checks passed
@jpuri
jpuri deleted the fix/money-account-max-deposit-insufficient-funds branch July 27, 2026 11:38
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 27, 2026
@metamask-ci metamask-ci Bot added the release-8.6.0 Issue or pull request that will be included in release 8.6.0 label Jul 27, 2026
@jpuri jpuri changed the title fix(money): skip insufficient-funds alert on Max money account deposit fix(money): skip insufficient-funds alert on Max money account deposit cp-8.5.0 Jul 27, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed release-8.6.0 Issue or pull request that will be included in release 8.6.0 risk:medium AI analysis: medium risk size-M team-confirmations Push issues to confirmations team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants