Skip to content

Fix Ledger transaction approval race condition and improve error handling#3525

Closed
Copilot wants to merge 1 commit intoledgerUiFixfrom
copilot/sub-pr-3524
Closed

Fix Ledger transaction approval race condition and improve error handling#3525
Copilot wants to merge 1 commit intoledgerUiFixfrom
copilot/sub-pr-3524

Conversation

Copy link

Copilot AI commented Jan 29, 2026

Description

Ticket: CP-13165 CP-13167 CP-13173

Critical Fix

  • Race condition in transaction approval: The useEffect in LedgerReviewTransactionScreen was triggering onApprove() multiple times whenever dependencies changed (deviceForWallet, isConnected, ledgerAppName, onApprove). This caused duplicate transaction signing attempts and unpredictable behavior.

Changes Made

  • Added useRef to track approval state and prevent multiple invocations
  • Ensured openApp() completes before approval flow begins
  • Added explicit error handling for openApp() failures in transaction flow
  • Improved type safety by properly typing error objects instead of using @ts-ignore
  • Made error message handling consistent across all error types

Technical Details

The root cause was an unbounded useEffect hook:

// Before: Ran on every dependency change
useEffect(() => {
  const handleApproveTransaction = async (): Promise<void> => {
    if (deviceForWallet && isConnected) {
      await LedgerService.openApp(ledgerAppName)
      onApprove()
    }
  }
  handleApproveTransaction()
}, [deviceForWallet, onApprove, isConnected, ledgerAppName])

// After: Runs once when conditions are met
const approvalTriggeredRef = useRef(false)
useEffect(() => {
  if (approvalTriggeredRef.current) return
  // ... safe single invocation
}, [...])

Screenshots/Videos

Include relevant screenshots or screen recordings of iOS and Android.

Testing

Dev Testing (if applicable)

  • Test transaction signing flow: connect Ledger → initiate transaction → verify single approval prompt
  • Test error cases: wrong app open, device locked, user rejection
  • Test race conditions: rapid navigation/reconnection during approval flow
  • Verify transport race condition error displays correct message
  • Test auto-open app functionality for both Avalanche and Solana

QA Testing (if applicable)

  • Transaction approval should only trigger once per transaction
  • Error messages should be user-friendly and actionable
  • No duplicate transaction attempts when Ledger reconnects during approval
  • Transport busy errors show "Ledger is processing another request" message

Checklist

Please check all that apply (if applicable)

  • I have performed a self-review of my code
  • I have verified the code works
  • I have included screenshots / videos of android and ios
  • I have added testing steps
  • I have added/updated necessary unit tests
  • I have updated the documentation

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copy link
Author

Copilot AI commented Jan 29, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • chromedriver.storage.googleapis.com
    • Triggering command: /usr/local/bin/node /usr/local/bin/node install-npm.js (dns block)
  • https://api.github.com/repos/yarnpkg/yarn/releases
    • Triggering command: /usr/local/bin/node /usr/local/bin/node /tmp/xfs-ab978b7a/.yarn/releases/yarn-1.22.19.cjs set version classic --only-if-needed (http block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title [WIP] Enhance ledger UI for better usability Fix Ledger transaction approval race condition and improve error handling Jan 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants