Skip to content

fix(eth): passthrough JSON-RPC reads + tip-aware fee warning + drop-check diagnostic#55

Merged
BitHighlander merged 7 commits intodevelopfrom
fix/eth-swap-dropped-tx
Apr 30, 2026
Merged

fix(eth): passthrough JSON-RPC reads + tip-aware fee warning + drop-check diagnostic#55
BitHighlander merged 7 commits intodevelopfrom
fix/eth-swap-dropped-tx

Conversation

@BitHighlander
Copy link
Copy Markdown
Collaborator

Summary

This PR is opened draft for auditing. It bundles four related fixes that together unblock the Uniswap "Confirm in wallet" hang plus the recurring stuck-pending-tx pattern. The three commits stack cleanly so reviewers can audit each independently.

The headline fix is the JSON-RPC shape passthrough — that's the smell that breaks the dApp's ability to track a tx after we hand back its hash, and it would still bite us on a perfect public-mempool RPC. See docs/RPC_PASSTHROUGH_AUDIT.md (vendored in this PR) for the field-by-field analysis.

Commits

  1. fix(eth): passthrough eth_getTransaction* + eth_getBlockByNumber to preserve JSON-RPC shape (3256fce)

    • handleEthGetTransactionByHash, handleEthGetTransactionReceipt, handleEthGetBlockByNumber were calling ethers v6 wrapper methods that return class instances with renamed/dropped fields (gas/gasLimit, input/data, transactionIndex/index, flat vs nested v/r/s). Replaced with provider.send('<method>', params) passthroughs that return the spec shape verbatim. Mirrors the existing pattern in 9cf2bbe (eth_call + eth_estimateGas) on the read side.
    • Same commit also adds a post-broadcast drop-check (8s + 45s setTimeouts) that logs [DROP-CHECK] and emits chrome.runtime tx_drop_warning when the broadcast hash is invisible to our RPC. Diagnostic for the low-tip eviction failure mode.
  2. feat(eth): tip-aware fee-warning + smart-contract detection fix (2f9a777)

    • Fee-warning now flags maxPriorityFeePerGas below per-chain tip floor (previously only checked maxFeePerGas floor). Tip economics matter independently — high maxFee with low tip = single-mempool acceptance, no propagation, eviction.
    • FeeWarning shape extended with trigger, priorityFloorWei, effectiveTipWei. Banner title varies by trigger.
    • RequestMethodCard was reading transaction.request.data, but transaction.request is the JSON-RPC params arrayrequest.data is always undefined, so Universal Router calls (data: 0x3593564c…) rendered as a green-check 'Simple transfer'. Now reads transaction.unsignedTx.data to match the Details tab.
  3. docs: RPC passthrough audit + dropped-tx retro (32b9399)

    • docs/RPC_PASSTHROUGH_AUDIT.md — the audit document, vendored as the source of truth for reviewers. Field-divergence tables, structured-clone consequences, list of handlers audited, audit candidates outside ethereumHandler.ts.
    • RETRO_uniswap_swap_dropped_tx.md — failure-mode retro that drove this work.

Audit asks

The reviewers I'd particularly like eyes from:

  • Confirm the field-divergence table in docs/RPC_PASSTHROUGH_AUDIT.md against ethers v6 source. I derived it from inspection; a sanity-check pass would be valuable.
  • Sanity-check the per-chain tip-floor numbers in feeFloors.ts (STATIC_PRIORITY_FLOOR_WEI). The mainnet 1 gwei is the load-bearing one for the failing case; others (Polygon 25 gwei, Avalanche 1 gwei, BSC 1 gwei, sequencer-driven L2s = 0) are conservative defaults that may want refinement.
  • The drop-check uses fixed 8s + 45s timeouts. Reasonable, but if there's a preference for telemetry/configuration we can wire that through.
  • Out of scope, separately tracked: EVM RPC URL bootstrap is still hardcoded in chains.ts and index.ts:665. Per discussion this should migrate to Pioneer (single source of truth for non-EVM already). Not patched here; captured in retro + memory. If this PR's reviewers can confirm the migration approach we'll start that workstream next.

What this PR does not fix

  • The "tx drops from mempool" routing problem itself. Diagnostic only via the drop-check; root cause is upstream of what we control (RPC routing through providers like llamarpc that forward to private mempools).
  • Vault firmware nonce display. Out of repo.

Test plan

  • pnpm build clean (verified locally, all 9 packages green).
  • Reload extension at chrome://extensions, swap small amount on Uniswap mainnet.
  • After [HANDOFF] dApp ← KeepKey (ethereum/eth_sendTransaction) RESOLVE, expect the polling eth_getTransactionByHash response value to have JSON-RPC field names (gas, input, transactionIndex, flat v/r/s) — not the ethers wrapper shape (gasLimit, data, index, nested signature).
  • Approval card should now read "Smart Contract — Interacts with smart contract — review carefully" (yellow) for Universal Router calls instead of green-check "Simple transfer".
  • If tip is below 1 gwei (mainnet), expect orange banner with "⚠ Low miner tip — tx may be dropped" + footer line showing tip floor and effective tip.
  • Background console after broadcast: [DROP-CHECK] tx 0x… visible on RPC at 8000ms (block=...) if mined, or [DROP-CHECK] tx 0x… not visible on RPC 45000ms after broadcast if dropped.

🤖 Generated with Claude Code

BitHighlander and others added 7 commits April 28, 2026 16:26
…reserve JSON-RPC shape

Three read handlers were calling ethers v6 wrapper methods (provider.getTransaction,
provider.getTransactionReceipt, provider.getBlock) and returning the resulting class
instances to the dApp. Field names diverge from the JSON-RPC spec (gasLimit vs gas,
data vs input, index vs transactionIndex, signature.{v,r,s} vs flat) and methods
are stripped during chrome.runtime structured-clone. dApps parsing the spec shape
silently fail to reconcile the response — Uniswap's "Confirm in wallet" hangs even
after the tx is on-chain because eth_getTransactionByHash never resolves to the
expected shape.

Mirrors the pattern in 9cf2bbe (eth_call + eth_estimateGas) on the read side.

Also adds a post-broadcast drop-check (8s + 45s setTimeouts) that logs and emits
chrome.runtime tx_drop_warning when the broadcast hash is invisible to our RPC —
diagnostic for the low-tip eviction failure mode tracked in
RETRO_uniswap_swap_dropped_tx.md.

See docs/RPC_PASSTHROUGH_AUDIT.md.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fee-warning now flags two distinct failure modes instead of one:
- maxFeePerGas below floor (existing behavior, preserved)
- maxPriorityFeePerGas below the per-chain tip floor (new)

Tip floor matters because dApps often pad maxFee well above floor while
keeping tip below 1 gwei. Such txs are accepted by an entry node, never
gossiped to miners, and silently evicted — see RETRO_uniswap_swap_dropped_tx.md.

The FeeWarning shape now carries trigger ('maxFee' | 'tip' | 'both'),
priorityFloorWei, and effectiveTipWei so the side-panel banner can title
+ explain the warning correctly. Existing 'dapp/suggested/custom' override
paths are unchanged.

Side-panel RequestMethodCard previously read transaction.request.data,
but transaction.request is the raw JSON-RPC params *array* — request.data
is always undefined, so Universal Router calls (data 0x3593564c…) rendered
as a green-check 'Simple transfer'. Now reads transaction.unsignedTx.data
(matching what the Details tab uses) so contract calls correctly render as
yellow 'Smart Contract — review carefully'.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
RPC_PASSTHROUGH_AUDIT.md vendored to anchor reviewer auditing of the
read-side handlers. Captures the field-name divergence (gas/gasLimit,
input/data, transactionIndex/index, flat-vs-nested signature) between
ethers v6 wrappers and the JSON-RPC spec, the chrome.runtime
structured-clone consequences, the three handlers fixed in this PR, and
the audit candidates left for follow-up review.

RETRO_uniswap_swap_dropped_tx.md captures the failure mode that drove
this work: hash returned to dApp → tx briefly visible on etherscan →
evicted → eth_getTransactionByHash returns null → Uniswap UI hangs.
Includes the audit findings on hardcoded EVM RPC bootstrap (separate
Pioneer-migration workstream) and the fix taxonomy.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After the [HANDOFF] BEX → RPC broadcast log, parse the signed bytes via
ethers.Transaction.from(signedTx) and log every parsed field plus the
ECDSA-recovered signer. Compare to the dApp-supplied `from`. If they
mismatch, log [DECODE] ❌ MALFORMED-HEX at error level.

Threaded an `expectedFrom` argument through both broadcastTransaction
call sites (sendTransaction at line 1318 of approved-event flow, and
the alt path at line 825) so the comparison has the canonical
expected signer.

This instrumentation surfaced a release-blocker bug in EIP-1559 signing
upstream of the BEX — see RETRO_evm_tx_1559_signing_chain.md and
HANDOFF_evm_tx_1559_signing_chain.md. Imports `Transaction` from ethers
which is already a transitive dep.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- RETRO_evm_tx_1559_signing_chain.md: the [DECODE] log added in the
  previous commit caught a release-blocker bug. sdk.eth.ethSignTransaction
  is returning serialized type-2 envelopes whose ECDSA signature does not
  recover to the device's address — recovers to a wrong-but-deterministic
  one (e.g. 0xEB152892… for the captured tx). 30+ candidate pre-image
  hashes ruled out (chainId variants, legacy fallback, missing 0x02
  prefix, field swap, data truncation/extension). Bug is in a non-canonical
  field encoding inside keepkey-vault-sdk and/or vault firmware. EIP-712
  signing through the same SDK works correctly, so the corruption is
  EIP-1559-specific.

- HANDOFF_evm_tx_1559_signing_chain.md: self-contained next-session
  handoff with reproducer commands, where to look in the SDK/firmware,
  what to ignore (the routing/Blink theories I hallucinated earlier),
  and a verification checklist for the eventual fix.

- RETRO_uniswap_swap_dropped_tx.md: marked SUPERSEDED with a banner
  pointing at the new retro. The fee-warning/drop-check/passthrough
  cleanups captured there are still real fixes for real other smells,
  but the framing of mempool eviction as the root cause was wrong —
  the real cause is the malformed-hex signature.

Companion test scaffolding lives in keepkey-vault-sdk (separate repo),
not committed here:
  tests/evm-tx-1559/recover-fixture.js
  tests/evm-tx-1559/sign-and-recover.js
  tests/fixtures/evm-tx-1559-regression.json

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tx fees, alarm-based drop check

- pnpm-lock.yaml: realign @types/chrome to ^0.0.280 so CI install passes
- broadcastTransaction: throw before broadcast when recovered signer ≠ expectedFrom
  (was logging only); pass-through ProviderRpcError so dApp sees the precise reason
- eth_signTransaction: pin chainId/from from active provider and apply feeChoice from
  the side-panel banner; previously 'Use suggested' / 'Custom' were no-ops on this path
- feeFloors.buildFeeWarning: enforce suggestedMax ≥ baseFee + suggestedPriority so
  'Use suggested' can never produce an effective tip below priorityFloor when the
  oracle's maxFee wins the choice
- drop-check: 45s probe migrated to chrome.alarms (survives MV3 SW suspension);
  manifest gains 'alarms' permission. 8s probe stays on setTimeout (best-effort,
  alarms minimum is 30s in production)
- FeeWarningBanner: drop unused @ts-expect-error flagged by reviewer

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
RETRO documented diagnostic dead-ends. The actual bug lives in
keepkey-firmware/lib/firmware/ethereum.c:891: when EIP-1559 tx-data exceeds
the 1024-byte single-USB-chunk threshold, the access-list 0xC0 byte is
hashed between the first chunk and the remainder, producing a non-canonical
pre-image. Single-chunk txs escape because the misplaced byte happens to
land at the end anyway. Universal Router swaps / Permit2 batches /
multicalls all hit it.

RESOLUTION_evm_tx_1559_signing_chain.md captures the unlock move
(EthereumTxRequest.hash field), the fix sketch, and the verification path.
Retro is kept with a banner pointing forward.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@BitHighlander BitHighlander marked this pull request as ready for review April 30, 2026 00:15
@BitHighlander BitHighlander merged commit 08ee279 into develop Apr 30, 2026
4 of 5 checks passed
@BitHighlander BitHighlander deleted the fix/eth-swap-dropped-tx branch April 30, 2026 00:16
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.

1 participant