Skip to content

feat(confirmations): block hardware wallets for money account deposits#32952

Open
dan437 wants to merge 6 commits into
mainfrom
money-account-deposit-hardware-wallet-alert
Open

feat(confirmations): block hardware wallets for money account deposits#32952
dan437 wants to merge 6 commits into
mainfrom
money-account-deposit-hardware-wallet-alert

Conversation

@dan437

@dan437 dan437 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Description

Money Account deposits require an EIP-7702 upgrade on the paying account, which hardware wallets don't support. The existing "Wallet not supported" blocking alert never fired for these deposits, so hardware wallet users hit confusing downstream errors (e.g. fee alerts) instead of a clear message.

Root cause: Money Account deposits are submitted with txParams.from set to the money account itself. The account that actually pays lives in TransactionPayController as accountOverride (seeded from the selected account, changeable via PayAccountSelector). useMMPayHardwareAccountAlert only checked txParams.from, which is never a hardware account.

Changes:

  • useMMPayHardwareAccountAlert now resolves the paying account as accountOverride ?? txParams.from, matching how TransactionPayController resolves it. Withdraw (post-quote) flows keep using txParams.from, because there the override is only the recipient and never signs.
  • The alert is suppressed whenever a fiat payment method is selected (any MM Pay flow, not just money account deposits): fiat purchases go directly to the destination, so the paying account never signs.
  • The hook is gated to MM Pay transaction types (PAY_TRANSACTION_TYPES) and registered in useConfirmationAlerts, so it now flows through the alerts context (hasBlockingAlerts, BlockingAlertMessage). The manual merges in custom-amount-info, musd-max-conversion-info, and usePendingAmountAlerts are removed.
  • The custom-amount keyboard keeps showing the hardware alert with priority over amount-level alerts (it can only be fixed by switching accounts).

Changelog

CHANGELOG entry: Added a blocking alert when a hardware wallet account is selected to pay for a Money Account deposit.

Related issues

Fixes:

Manual testing steps

Feature: Hardware wallet blocking alert on Money Account deposits

  Scenario: user opens a deposit with a hardware wallet selected
    Given the selected account is a Ledger account
    When user opens Money -> Add money
    Then the amount screen shows "Wallet not supported" instead of the Done button
    And the message "Hardware wallets aren't supported. Switch wallets to continue." is shown

  Scenario: user switches to a hot wallet
    Given the deposit amount screen shows the hardware wallet alert
    When user taps the From row and selects a hot wallet account
    Then the alert clears and the Done button returns

  Scenario: user switches to a hardware wallet after entering an amount
    Given user entered an amount with a hot wallet and pressed Done
    When user switches to a Ledger account in the account selector
    Then the Confirm button is disabled and shows "Wallet not supported"

  Scenario: user pays with a card
    Given the selected account is a Ledger account
    When user selects a fiat payment method for the deposit
    Then no hardware wallet alert is shown and the flow proceeds

  Scenario: user withdraws to a hardware wallet
    Given the user has a Money Account balance
    When user withdraws with a Ledger account as the recipient
    Then no hardware wallet alert is shown

  Scenario: regular send from a hardware wallet
    Given the selected account is a Ledger account
    When user sends ETH to another address
    Then no hardware wallet alert is shown

Screenshots/Recordings

Before

N/A

After

N/A

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

Medium Risk
Changes which accounts trigger blocking alerts across MM Pay confirmation and amount-entry flows; incorrect payer resolution could block or allow the wrong users, though behavior is covered by expanded unit tests.

Overview
Fixes the MM Pay hardware wallet blocking alert so it actually fires for Money Account deposits and other pay flows where the signer is accountOverride, not txParams.from.

useMMPayHardwareAccountAlert now resolves the paying account as accountOverride ?? from, except on pay withdraw flows where override is only the recipient. The alert is limited to PAY_TRANSACTION_TYPES, skipped when a fiat payment method is selected, and still respects the existing mUSD conversion hardware feature-flag / QR wallet rules.

The hook is wired into useConfirmationAlerts so blocking UI flows through the shared alerts context. Manual merges in musd-max-conversion-info and usePendingAmountAlerts are removed; useTransactionCustomAmountAlerts still prioritizes the hardware alert over amount-level messages on the custom amount keyboard.

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

@dan437 dan437 self-assigned this Jul 8, 2026
@github-actions

github-actions Bot commented Jul 8, 2026

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.

@metamask-ci metamask-ci Bot added the team-confirmations Push issues to confirmations team label Jul 8, 2026
@metamask-ci

metamask-ci Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

PR template — items to address before "Ready for review"

Warnings — informational, address before merging:

  • Related issues section is empty. Add Fixes: #123 / Closes: <URL> / Refs: <Jira key>, or write a short rationale after the colon.

See docs/readme/ready-for-review.md for the full Definition of Ready for Review.

@github-actions github-actions Bot added the size-S label Jul 8, 2026
@dan437 dan437 marked this pull request as ready for review July 8, 2026 13:41
@dan437 dan437 requested a review from a team as a code owner July 8, 2026 13:41
@github-actions github-actions Bot added the risk:medium AI analysis: medium risk label Jul 8, 2026
@matthewwalsh0 matthewwalsh0 self-requested a review July 9, 2026 11:00
Comment thread app/components/Views/confirmations/hooks/alerts/useMMPayHardwareAccountAlert.ts Outdated
Comment thread app/components/Views/confirmations/hooks/alerts/useMMPayHardwareAccountAlert.ts Outdated
dan437 added 2 commits July 15, 2026 14:44
- gate useMMPayHardwareAccountAlert to MM Pay transaction types
- check accountOverride over txParams.from regardless of type, except
  withdraws where the override is only the recipient
- suppress the alert for any fiat payment, not just money account deposits
- register the alert in useConfirmationAlerts so hasBlockingAlerts and
  BlockingAlertMessage pick it up without manual merges
@github-actions github-actions Bot added size-M and removed size-S labels Jul 15, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit b62ffaf. Configure here.

@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

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

E2E Test Selection:
All 9 changed files are within the confirmations subsystem (app/components/Views/confirmations/). The changes refactor how the hardware wallet alert (useMMPayHardwareAccountAlert) is wired into the confirmation alert pipeline:

  1. The alert is moved from usePendingAmountAlerts → useConfirmationAlerts (transaction alerts), making it available globally in the confirmation flow.
  2. musd-max-conversion-info.tsx no longer directly calls useMMPayHardwareAccountAlert since it's now in the global alerts system.
  3. useTransactionCustomAmountAlerts.ts adds priority ordering so hardware wallet alerts appear first.
  4. useMMPayHardwareAccountAlert itself is enhanced to correctly identify the paying account (handling accountOverride, withdraw flows, and fiat payment scenarios).

These changes affect the confirmation UI alert system for MetaMask Pay transactions involving hardware wallets, mUSD conversions, and fiat payments. SmokeConfirmations is the directly relevant tag as it covers transaction confirmation UI, smart contract interactions, token approvals, and transaction sending flows. No other feature areas (swaps, staking, browser, snaps, etc.) are impacted by these changes.

Performance Test Selection:
The changes are limited to alert hook logic within the confirmations subsystem. They refactor how hardware wallet alerts are wired and prioritized in the confirmation flow. This is purely UI/alert logic with no impact on app launch, asset loading, onboarding, login, swaps performance, or any other performance-sensitive flows.

View GitHub Actions results

@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

2 participants