trade execute: bind EVM receipt confirmation to the locally-derived tx hash

[kome12]

Summary

• trade execute confirmed a broadcast EVM transaction by polling waitForReceipt with whatever txHash the broadcaster reported, without ever checking it against the transaction the CLI actually signed. A compromised or buggy broadcaster could report success for a different transaction, and the CLI would proceed to the next step (e.g. broadcasting a swap after a "confirmed" allowance revoke that never happened on-chain).
• Add evmTxHash (keccak256 over the raw signed tx bytes) and confirmEvmBroadcast, which polls the receipt on our own locally-derived hash and fails closed with a clear, actionable error if the broadcaster's reported hash disagrees.
• Apply this at every EVM executeTransaction/waitForReceipt pair in trade execute — the swap broadcast, ordinary approvals, and the revoke-then-reapprove flow — across the Privy, WalletConnect, and local-key signing paths.
• Carve out gasless swaps: the Relay solver broadcasts its own on-chain transaction there, so the returned hash is legitimately different from the bytes we signed; that path still confirms on the broadcaster's hash as before.
• TXHASH_MISMATCH is now fatal (re-thrown) everywhere it can occur, instead of being swallowed into "try the next quote" — a broadcaster-integrity failure shouldn't be treated like a bad quote.
• Left the WalletConnect wallet-broadcast swap path and src/bridge.js unchanged (out of scope — no raw signed bytes available in the former; different broadcast mechanism in the latter).

Test plan

• npm test — 2127 tests passing, including new evmTxHash unit tests (known-vector, hex normalization, rejection cases, EIP-1559/legacy signing round-trips), a fail-closed mismatch test, and a gasless-regression test
• npm run lint
• Existing EVM execute test mocks updated to echo the real hash of the signed bytes, as a correct broadcaster would

🤖 Generated with Claude Code

[nansen-pr-reviewer]

pr-reviewer Summary for #55ab7db

📝 2 findings

Review completed. Please address the findings below.

Findings by Severity

Severity	Count
🟡 Medium	2

Review effort: 3/5 (Moderate)

Summary

This PR closes a genuine security gap: EVM receipts are now polled on the CLI's own locally-derived keccak256 hash of the signed transaction rather than on whatever hash the broadcaster reported, failing hard (TXHASH_MISMATCH) if the two differ. The approach is sound, the gasless carve-out is correctly scoped, and the test coverage is thorough. Two medium-severity issues are worth addressing.

Findings (2 medium)

---

src/trading.js — WalletConnect "signed-tx fallback" receipt confirmation still uses waitForReceipt, not confirmEvmBroadcast

Severity: medium

The WalletConnect approval path has a branch where the wallet returns a signedTransaction instead of a txHash (lines 2683-2705). The PR correctly adds the mismatch check inline and then sets approvalTxHash = localApprovalHash. At that point approvalTxHash is already the local hash, so polling it via waitForReceipt at line 2727 is safe.

However, the revoke equivalent (lines 2618-2639 / poll at line 2653) follows the same pattern: it also sets revokeTxHash = localRevokeHash and then calls plain waitForReceipt(chain, revokeTxHash) at line 2653, which is fine. But these two sub-paths perform the mismatch check via bespoke inline code rather than confirmEvmBroadcast, making them harder to audit and easier to accidentally regress when refactoring. Consider extracting these inline checks into a call to confirmEvmBroadcast (or at minimum a TODO comment noting why it isn't used) to keep all mismatch logic in one place.

Suggested fix: Replace the inline if (broadcastResult.txHash && localApprovalHash.toLowerCase() !== broadcastResult.txHash.toLowerCase()) { throw new CommandError(...) blocks (lines 2695-2704 and 2629-2638) with await confirmEvmBroadcast(chain, approvalResult.signedTransaction, broadcastResult.txHash) so the mismatch path stays canonical.

---

src/trading.js — Success log in Privy path prints broadcaster hash instead of local hash

Severity: medium

At lines 2314 and 2373, after confirmEvmBroadcast succeeds, the confirmation log is:

log(`  ✓ Allowance revoked in block ${parseInt(receipt.blockNumber, 16)}: ${revokeResult.txHash}`);
//                                                                           ^^^^^^^^^^^^^^^^^^^
//                                                                    this is the BROADCASTER's hash

Because confirmEvmBroadcast only returns if revokeResult.txHash already matches the local hash, the value shown is correct in practice — but it's semantically misleading. If the broadcaster returns null/undefined, confirmEvmBroadcast still polls on localHash, succeeds, and then the log emits undefined or null to the user. This is an actionable cosmetic/UX bug: replace revokeResult.txHash / approvalResult.txHash in the success logs with evmTxHash(signedRevoke) / evmTxHash(signedApproval) (or capture localHash from the return value if you extend the function to return it).

---

Token usage: 8,577 input, 4,878 output, 490,682 cache read, 47,655 cache write | Usage Guide

New pushes are reviewed automatically with a 10-minute cooldown between reviews. To request a review at any time, comment @nansen-pr-reviewer re-review.