Skip to content

[LWDM] fix(coin-framework): improve nonce check to avoid error at broadcast - #20289

Open
qperrot wants to merge 1 commit into
developfrom
fix/coin-framework-nonce-too-low
Open

[LWDM] fix(coin-framework): improve nonce check to avoid error at broadcast#20289
qperrot wants to merge 1 commit into
developfrom
fix/coin-framework-nonce-too-low

Conversation

@qperrot

@qperrot qperrot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

📝 Description

When two transactions are sent in quick succession, the second one can be
rejected by the node with "nonce too low" (or trigger an unintended
replacement). This happens because the sequence/nonce source (indexer or a
load-balanced RPC) lags behind a just-broadcast transaction and briefly returns
a nonce that was already consumed by the first send.

Separately, when an intent carries an explicit nonce (dapp / WalletConnect /
speed-up flows), a stale value was only rejected after signing — the user went
through the whole device flow before hitting the error.

What this PR does

  1. nextSequenceWithPending (generic-coin-framework) — at sign time, derive
    the next sequence from both the network source and the locally-tracked
    pending operations: max(networkSequence, highestPendingSequence + 1).
    Pending ops are tracked optimistically right after broadcast and carry their
    transactionSequenceNumber, so the next send no longer reuses a nonce. It is
    deterministic and self-corrects: once the network source catches up,
    networkSequence wins again.

  2. validateNonce (coin-evm validateIntent) — when an intent carries an
    explicit sequence, validate it against the sender's on-chain nonce before
    signing and surface "nonce is too low" up front. Best-effort: a failed nonce
    fetch never blocks the user. When no explicit sequence is set, nothing is
    checked (crafting fetches a fresh nonce).

Changes

  • generic-coin-framework/utils.ts — add nextSequenceWithPending()
  • generic-coin-framework/signOperation.ts — use it when the intent has no explicit sequence
  • coin-evm/logic/validateIntent.ts — add the pre-sign validateNonce() guard

🔗 Context

Copilot AI review requested due to automatic review settings July 31, 2026 14:02
@live-github-bot live-github-bot Bot added common Has changes in live-common coin-modules labels Jul 31, 2026
@live-github-bot live-github-bot Bot changed the title fix: improve nonce check to avoid error at broadcast [LWDM] fix: improve nonce check to avoid error at broadcast Jul 31, 2026

Copilot AI 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.

Pull request overview

Improves nonce/sequence handling for rapid consecutive sends and for explicit nonce intents (e.g., WalletConnect / dapps), aiming to prevent “nonce too low” failures and surface nonce issues earlier in the flow.

Changes:

  • Add nextSequenceWithPending() to compute the next nonce from max(networkNonce, highestPendingNonce + 1).
  • Use the new pending-aware nonce computation during signing when the intent does not carry an explicit sequence.
  • Add a pre-sign EVM intent nonce validation against the live on-chain nonce (best-effort).

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
libs/ledger-live-common/src/bridge/generic-coin-framework/utils.ts Adds nextSequenceWithPending() helper for pending-aware nonce derivation.
libs/ledger-live-common/src/bridge/generic-coin-framework/utils.test.ts Adds unit tests for nextSequenceWithPending().
libs/ledger-live-common/src/bridge/generic-coin-framework/signOperation.ts Uses pending-aware nonce derivation when intent nonce isn’t explicitly set.
libs/coin-modules/coin-evm/src/logic/validateIntent.ts Adds validateNonce() to detect “nonce too low” before signing (best-effort).
libs/coin-modules/coin-evm/src/logic/validateIntent.test.ts Adds test coverage for nonce validation behavior (error/skip/best-effort).
.changeset/quick-nonces-heal.md Declares minor bumps for live-common and coin-evm and documents the behavioral change.

Comment thread libs/ledger-live-common/src/bridge/generic-coin-framework/utils.ts
Comment thread libs/ledger-live-common/src/bridge/generic-coin-framework/utils.ts
Copilot AI review requested due to automatic review settings July 31, 2026 14:09
@qperrot
qperrot force-pushed the fix/coin-framework-nonce-too-low branch from 4b7504e to 683c459 Compare July 31, 2026 14:09

Copilot AI 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.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (1)

libs/ledger-live-common/src/bridge/generic-coin-framework/utils.ts:166

  • nextSequenceWithPending converts transactionSequenceNumber via BigInt(op.transactionSequenceNumber.toString()), which can throw at runtime (e.g., if the BigNumber is NaN, non-integer, or rendered in exponential notation like 1e+21). That would crash signing when an account has any malformed/edge pending op sequence. Consider guarding the BigNumber and converting via toFixed(0) like other code does (e.g. coin-multiversx safeStakeToBigInt).
    if (op.transactionSequenceNumber === undefined || op.transactionSequenceNumber === null) {
      continue;
    }
    const seq = BigInt(op.transactionSequenceNumber.toString());
    if (seq > highestPending) highestPending = seq;

@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Web Tools Build Status

Build Status Deployment
Web Tools Build ✅ Deployed https://web-tools-qe40rn3dt-ledger-hq-prd.vercel.app
Native Storybook Build ⏭️ Skipped
React Storybook Build ⏭️ Skipped

@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Rsdoctor Bundle Diff Analysis

Found 7 projects in monorepo, 2 projects with changes.

📊 Quick Summary
Project Total Size Change
desktop-main 2.3 MB 0
desktop-preloader 7.1 KB 0
desktop-renderer 80.7 MB +213.0 B (0.0%)
desktop-webviewDappPreloader 36.9 KB 0
desktop-webviewPreloader 200.0 B 0
desktop-workers 36.8 KB 0
mobile 261.6 MB +1.8 KB (0.0%)
📋 Detailed Reports (Click to expand)

📁 desktop-renderer

Path: rsdoctor/desktop-renderer/rsdoctor-data.json

📌 Baseline Commit: cbe58687d4 | PR: #20078

Metric Current Baseline Change
📊 Total Size 80.7 MB 80.7 MB +213.0 B (0.0%)
📄 JavaScript 29.3 MB 29.3 MB +213.0 B (0.0%)
🎨 CSS 183.2 KB 183.2 KB 0
🌐 HTML 1.8 KB 1.8 KB 0
📁 Other Assets 51.2 MB 51.2 MB 0

📦 Download Diff Report: desktop-renderer Bundle Diff

📁 mobile

Path: rsdoctor/mobile/rsdoctor-data.json

📌 Baseline Commit: cbe58687d4 | PR: #20078

Metric Current Baseline Change
📊 Total Size 261.6 MB 261.6 MB +1.8 KB (0.0%)
📄 JavaScript 110.5 MB 110.5 MB +873.0 B (0.0%)
🎨 CSS 0 B 0 B 0
🌐 HTML 0 B 0 B 0
📁 Other Assets 151.1 MB 151.1 MB +980.0 B (0.0%)

📦 Download Diff Report: mobile Bundle Diff

Generated by Rsdoctor GitHub Action

@qperrot
qperrot force-pushed the fix/coin-framework-nonce-too-low branch from 683c459 to d94edd8 Compare July 31, 2026 14:24
Copilot AI review requested due to automatic review settings July 31, 2026 14:24
@qperrot
qperrot marked this pull request as ready for review July 31, 2026 14:24
@qperrot
qperrot requested a review from a team as a code owner July 31, 2026 14:24

Copilot AI 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.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (3)

libs/ledger-live-common/src/bridge/generic-coin-framework/utils.test.ts:1382

  • This test uses pendingOp(1e21) as a JS number; 1e21 is outside the safe integer range, so the BigNumber constructed from it can be imprecise and make the assertion flaky. Use a string literal for the large nonce instead.
    it("handles a large pending sequence without throwing (fixed-point, not exponential)", () => {
      // BigNumber(1e21).toString() is "1e+21", which BigInt() cannot parse; .toFixed() must be used.
      expect(nextSequenceWithPending([pendingOp(1e21)], 5n)).toBe(1000000000000000000001n);
    });

libs/coin-modules/coin-evm/src/logic/validateIntent.ts:135

  • validateNonce uses getNextSequence(), which is backed by NodeApi.getTransactionCount(). For RPC nodes this uses the "pending" tag (libs/coin-modules/coin-evm/src/network/node/rpc.common.ts:299-305), so currentNonce is the next available nonce including pending txs. That means an explicit nonce meant to replace/speed-up an existing pending tx (same nonce as the pending one) will be < currentNonce and will be rejected as "nonce is too low", contradicting the stated support for speed-up flows.
  try {
    const currentNonce = await getNextSequence(currency, intent.sender);
    if (intent.sequence < currentNonce) {
      return {
        errors: { transaction: new InvalidTransactionError("nonce is too low") },
        warnings: {},
      };
    }

libs/ledger-live-common/src/bridge/generic-coin-framework/utils.test.ts:1351

  • The pendingOp helper takes a JS number and feeds it to new BigNumber(seq). For large values (e.g. nonces beyond Number.MAX_SAFE_INTEGER) this risks precision loss in the test setup; letting it accept BigNumber.Value enables passing a string for large nonces.

This issue also appears on line 1379 of the same file.

    const pendingOp = (seq: number | null): Operation =>
      ({
        transactionSequenceNumber: seq === null ? undefined : new BigNumber(seq),
      }) as Operation;

dilaouid
dilaouid previously approved these changes Jul 31, 2026
@qperrot qperrot changed the title [LWDM] fix: improve nonce check to avoid error at broadcast [LWDM] fix(coin-framework): improve nonce check to avoid error at broadcast Aug 3, 2026
@qperrot
qperrot force-pushed the fix/coin-framework-nonce-too-low branch from d94edd8 to a144cc3 Compare August 3, 2026 05:36
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

⚠️ E2E tests are required

Changes detected require e2e testing before merge (even before asking for any review).

🖥️ Desktop

-> Run Desktop E2E

  • Select "Run workflow"
  • Branch: fix/coin-framework-nonce-too-low
  • Device: nanoSP or stax

📱 Mobile

-> Run Mobile E2E

  • Select "Run workflow"
  • Branch: fix/coin-framework-nonce-too-low
  • Device: nanoX

Copilot AI review requested due to automatic review settings August 3, 2026 14:29
@qperrot
qperrot force-pushed the fix/coin-framework-nonce-too-low branch from 4f3959f to e50980f Compare August 3, 2026 14:30

Copilot AI 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.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (1)

libs/ledger-live-common/src/bridge/generic-coin-framework/signOperation.ts:72

  • PR description states a new pre-sign validateNonce() guard was added in libs/coin-modules/coin-evm/src/logic/validateIntent.ts, but this PR only changes the generic coin-framework (no EVM validateIntent changes are included). Either add the missing EVM nonce validation changes, or update the PR description/scope so it matches what’s actually being shipped.
          if (typeof transactionIntent.sequence !== "bigint" || transactionIntent.sequence < 0n) {
            // The network sequence source lags behind a just-broadcast tx, so combine it with
            // locally-tracked pending operations to avoid reusing a nonce on rapid consecutive sends.
            const networkSequence = await coinModuleApi.getNextSequence(transactionIntent.sender);
            transactionIntent.sequence = nextSequenceWithPending(
              account.pendingOperations ?? [],
              networkSequence,
            );

@sonarqubecloud

sonarqubecloud Bot commented Aug 3, 2026

Copy link
Copy Markdown

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

Labels

common Has changes in live-common

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants