Skip to content

Conversation

@weitingsun
Copy link
Contributor

@weitingsun weitingsun commented Dec 24, 2025

Description

Refine hardware signing flow

Changelog

CHANGELOG entry:null

Related issues

Fixes:

Manual testing steps

Feature: my feature name

  Scenario: user [verb for user action]
    Given [describe expected initial app state]

    When user [verb for user action]
    Then [describe expected outcome]

Screenshots/Recordings

Before

After

Performing unified swaps and bridge
https://github.com/user-attachments/assets/224ccca4-bf1a-421b-8c35-6e33489755dc

Pre-merge author checklist

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

Hardware signing flow refinement

  • In RootRPCMethodsUI.autoSign, detect Ledger and QR hardware accounts and only auto-sign for those; non-hardware accounts return early
  • Ledger transactions now navigate to the Ledger transaction confirmation modal; QR accounts proceed via Engine.acceptPendingApproval
  • Notes that unified swaps/bridge no longer use this path; maintains transaction finish/confirm listeners and swaps metrics handling

Changelog updates

  • Adds 7.61.3, 7.61.4, 7.61.5 sections and updates compare links

Written by Cursor Bugbot for commit 1761028. This will update automatically on new commits. Configure here.

@github-actions
Copy link
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.

@metamaskbot metamaskbot added the team-mobile-platform Mobile Platform team label Dec 24, 2025
@weitingsun weitingsun marked this pull request as ready for review December 24, 2025 17:19
@github-actions github-actions bot added size-S and removed size-XS labels Dec 24, 2025
Copy link
Contributor

@Cal-L Cal-L left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions
Copy link
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeSwaps, SmokeConfirmationsRedesigned
  • Risk Level: medium
  • AI Confidence: 82%
click to see 🤖 AI reasoning details

Analysis

Changed Files:

  1. CHANGELOG.md: Pure documentation update with release notes - no functional changes
  2. RootRPCMethodsUI.js: Targeted fix in the autoSign function

Code Change Impact:

The RootRPCMethodsUI.js change modifies the autoSign function to add an early return for non-hardware accounts (only Ledger and QR accounts should auto-sign).

Key observations:

  1. The function handles automatic signing for hardware wallets (Ledger/QR) in swap and bridge transactions
  2. The change adds a guard clause that prevents execution for regular accounts before event subscriptions are set up
  3. The comment indicates: "unified swaps and bridge no longer goes through this path" - suggesting this is legacy cleanup
  4. This is a performance optimization and correctness fix - prevents unnecessary event subscriptions for non-hardware accounts
  5. The autoSign function is specifically called for swap/bridge transactions (checked via getIsSwapApproveOrSwapTransaction and getIsBridgeTransaction)

Potential Impact Areas:

  • Transaction confirmations (particularly hardware wallet confirmations)
  • Swap transactions (especially with hardware wallets)
  • Bridge transactions (especially with hardware wallets)
  • The function manages transaction flow, notifications, and analytics tracking

Test Tags Selection:

SmokeSwaps: Selected because:

  • The autoSign function is explicitly called for swap transactions
  • The change affects how swap transactions are handled for hardware accounts
  • Swap analytics tracking and event handling are part of this code path

SmokeConfirmationsRedesigned: Selected because:

  • The autoSign function is part of the transaction confirmation flow
  • The change affects transaction approval handling
  • Hardware wallet transaction signing is a confirmation flow concern

Not selecting other tags:

  • SmokeAccounts: While hardware accounts are involved, this is more about transaction flow than account management
  • SmokeCore/SmokeWalletPlatform: Too broad - the change is specific to transaction signing
  • SmokeTrade: Related but SmokeSwaps is more specific
  • Other tags: Not directly related to the change

Risk Assessment:

Medium because:

  • This is a core transaction signing flow modification
  • Hardware wallet transactions are involved (more complex flow)
  • The change optimizes by preventing code execution, which could expose edge cases
  • However, the change is relatively small and defensive (adding guards)
  • The comment suggests this is legacy code path cleanup
  • CHANGELOG indicates this is part of a larger release with many changes

View GitHub Actions results

@weitingsun weitingsun added the skip-sonar-cloud Only used for bypassing sonar cloud when failures are not relevant to the changes. label Dec 24, 2025
@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.68%. Comparing base (ac7aa56) to head (1761028).
⚠️ Report is 2 commits behind head on release/7.61.5.

Files with missing lines Patch % Lines
app/components/Nav/Main/RootRPCMethodsUI.js 0.00% 4 Missing ⚠️
Additional details and impacted files
@@                 Coverage Diff                 @@
##           release/7.61.5   #24228       +/-   ##
===================================================
+ Coverage                0   78.68%   +78.68%     
===================================================
  Files                   0     4028     +4028     
  Lines                   0   105643   +105643     
  Branches                0    21209    +21209     
===================================================
+ Hits                    0    83121    +83121     
- Misses                  0    16753    +16753     
- Partials                0     5769     +5769     

☔ View full report in Codecov by Sentry.
📢 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

@weitingsun weitingsun merged commit 0645fe2 into release/7.61.5 Dec 24, 2025
194 of 234 checks passed
@weitingsun weitingsun deleted the wsun/refine-hardware-signing-flow branch December 24, 2025 19:34
@github-actions github-actions bot locked and limited conversation to collaborators Dec 24, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

size-S skip-sonar-cloud Only used for bypassing sonar cloud when failures are not relevant to the changes. team-mobile-platform Mobile Platform team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants