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
• Ran the actual CLI (node src/index.js trade quote / trade execute) against a local mock trading-api + RPC server: a correct broadcaster confirms normally, a broadcaster returning a mismatched hash is rejected with the fail-closed error, before any success is reported

🤖 Generated with Claude Code

[nansen-pr-reviewer]

pr-reviewer Summary for #65fc4c8

📝 1 finding

Review completed. Please address the findings below.

Findings by Severity

Severity	Count
🟡 Medium	1

Review effort: 4/5 (Complex)

Summary

This PR correctly addresses a real security gap: the CLI was polling the receipt for whatever hash the broadcaster reported, meaning a compromised broadcaster could supply a hash for a different transaction and the CLI would proceed as if its own transaction confirmed. The fix — deriving the canonical hash locally from the signed bytes and failing closed on a mismatch — is well-reasoned, correctly applied across all three signing paths (Privy, WalletConnect fallthrough, local-key), and the gasless carve-out is sound. Test coverage is thorough. Changeset level (patch) is appropriate for a bug fix.

Findings (1 medium)

src/trading.js — Misleading error message when evmTxHash throws before the receipt poll

Severity: medium

Location: Lines 3163–3182 (non-gasless EVM swap broadcast path)

} else {
  txId = evmTxHash(signedTransaction);          // <-- line 3164: throws plain Error if signedTransaction is bad hex
  explorerUrl = chainConfig.explorer + txId;
  const { hash } = await confirmEvmBroadcast(chain, signedTransaction, result.txHash);
  txId = hash;
  explorerUrl = chainConfig.explorer + txId;
}

evmTxHash(signedTransaction) at line 3164 is inside the try block whose catch at line 3170 prints "⚠ Transaction was broadcast but REVERTED on-chain!". If signedTransaction is somehow not valid hex (e.g. empty string from an unexpected Privy or WalletConnect response), the Error thrown by evmTxHash — before any receipt poll has occurred — will be caught here, and the user will see a misleading revert message for a transaction that was never polled, not confirmed, and may not even have been broadcast correctly.

This is a low-probability path (all three signing branches produce valid hex), but the consequence of hitting it is a confusing, incorrect diagnostic that obscures the real failure.

Suggested fix: Move the evmTxHash pre-computation outside (or before) the receipt-polling try, or add a TXHASH_MISMATCH-style rethrow guard for evmTxHash's validation errors so they surface with their real message:

} else {
  // Derive our local hash up-front; let hex-validation errors surface directly.
  let localHash;
  try {
    localHash = evmTxHash(signedTransaction);
  } catch (hashErr) {
    throw new CommandError(`Cannot derive local tx hash: ${hashErr.message}`, 'INVALID_SIGNED_TX');
  }
  txId = localHash;
  explorerUrl = chainConfig.explorer + txId;
  try {
    const { hash } = await confirmEvmBroadcast(chain, signedTransaction, result.txHash);
    txId = hash;
    explorerUrl = chainConfig.explorer + txId;
  } catch (receiptErr) {
    if (receiptErr.code === 'TXHASH_MISMATCH') throw receiptErr;
    // ... existing revert handling ...
  }
}

---

Token usage: 803 input, 6,700 output, 1,134,390 cache read, 63,847 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.