fix: KMS stale-connection retry, verified broadcast, real signer errors

[lukemacauley]

Motivation

Root-caused from a 21-minute production outage in the gator liquidator on 2026-07-09: every batch-liquidation send failed, strictly alternating error.SigningFailed / error.RpcError, then self-recovered. Tracing both error paths to source turned up three distinct defects in this repo's send path.

Changes

1. KMS: retry once on a stale keep-alive connection (src/kms.zig)

Signing calls are often minutes apart, and KMS closes idle keep-alive connections well before that. The pooled connection is therefore routinely dead when the next kms:Sign reuses it, so the first write fails; the failed fetch discards that connection and one retry dials fresh. This was the SigningFailed half of the outage — it fired on every other attempt, which is exactly the strict alternation seen in the logs.

2. Wallet: propagate the signer's real error (src/wallet.zig)

signHash catch return error.SigningFailed collapsed every signer failure into one opaque error. For a KMS signer this hid whether the cause was a failed HTTPS call (RequestFailed), bad credentials (Unauthorized), or a recovery mismatch (AddressMismatch). Now propagated. Removes the now-unused WalletError.SigningFailed.

3. Wallet: verify ambiguous broadcasts before reporting failure (src/wallet.zig)

A JSON-RPC error on eth_sendRawTransaction does not prove the transaction missed the chain: a redelivered request can be answered already known while the first copy mines. This actually happened in the outage — a send logged as RpcError liquidated its position on-chain, and the caller wrongly treated it as failed (put the position in cooldown instead of marking it liquidated). The transaction hash is fully determined by the signed bytes, so on RpcError we now poll briefly for that hash's receipt and report success if it landed.

Adds a test that cross-checks the pre-broadcast hash against viem (independent implementation) for the well-known Anvil key #0, locking the keccak256(signed_bytes) == canonical-tx-hash invariant the receipt poll depends on.

Testing

• zig build test — 856/856 pass (was 855; +1 new cross-validation test)
• zig fmt --check src/ tests/ — clean

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Transaction sending is more resilient when a broadcast error occurs, with automatic confirmation checking before reporting failure.

• Bug Fixes

  • Improved handling of flaky network connections when calling KMS services by retrying failed requests.
  • Transaction hashes are now derived consistently from signed transaction bytes before broadcast.
  • Signing errors are now reported more accurately, and transaction-not-found cases are handled explicitly.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
eth-zig	Ready	Preview, Comment	Jul 9, 2026 3:52pm

[coderabbitai]

📝 Walkthrough

Walkthrough

KMS client requests now retry once on HTTP fetch failure with per-attempt response body handling and status-based error mapping. Wallet's sendTransaction handles ambiguous JSON-RPC broadcast errors by polling for a receipt via a new transactionLanded helper, signTransaction propagates signer errors directly, and WalletError set is updated accordingly with a new test.

Changes

KMS Request Retry

Layer / File(s)	Summary
Retry loop and response handling src/kms.zig	Response body allocation moves into a per-attempt retry loop; a failed fetch is retried once, and HTTP status is mapped to .ok (owned body), Unauthorized (forbidden/unauthorized), or RequestFailed (other statuses).

Estimated code review effort: 3 (Moderate) | ~20 minutes

Wallet Transaction Broadcast and Signing

Layer / File(s)	Summary
Wallet error set update src/wallet.zig	WalletError removes SigningFailed and adds ReceiptNotFound.
Ambiguous broadcast handling and receipt polling src/wallet.zig	sendTransaction computes the tx hash before broadcasting, catches error.RpcError, and uses new transactionLanded helper polling getTransactionReceipt up to three times to decide whether to return the tx hash or the original error.
Signing error propagation and canonical hash test src/wallet.zig	signTransaction now directly propagates the signer's signHash error instead of converting it to SigningFailed; a new test verifies signed tx bytes and keccak hash against canonical values.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Wallet
  participant Provider
  Wallet->>Wallet: compute tx_hash from signed_bytes
  Wallet->>Provider: sendRawTransaction(signed_bytes)
  Provider--xWallet: error.RpcError
  Wallet->>Wallet: transactionLanded(tx_hash)
  loop up to 3 attempts
    Wallet->>Provider: getTransactionReceipt(tx_hash)
    Provider->>Wallet: receipt or none
  end
  alt receipt found
    Wallet->>Wallet: return tx_hash
  else no receipt
    Wallet->>Wallet: return original RpcError
  end

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the three main fixes: KMS retry, broadcast verification, and propagating real signer errors.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/kms-retry-and-send-verify

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

src/wallet.zig (1)

157-171: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Short landing window (~2s) means a still-pending in-mempool tx triggers a nonce rollback.

When transactionLanded returns false, the errdefer at Lines 117–119 rolls back the managed nonce. If the ambiguous broadcast actually reached the mempool but has not mined within the ~2s poll window, the nonce is returned to the manager and can be reissued, colliding with the pending tx. This matches the "strictly no worse than before" contract, but since the whole point is disambiguation, consider widening/parameterizing the poll window (or only rolling back when a receipt lookup is authoritatively negative).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/wallet.zig` around lines 157 - 171, The broadcast path in wallet.zig is
rolling back the managed nonce too early when the receipt check is inconclusive.
Update the `sendRawTransaction`/`transactionLanded` flow so `transactionLanded`
is not treated as a hard negative after the short poll window; instead, widen or
parameterize the polling window in `transactionLanded` and only allow the
`errdefer` nonce rollback in the caller when a receipt lookup is definitively
negative. Keep the ambiguity-handling logic around `tx_hash`,
`provider.sendRawTransaction`, and `transactionLanded` aligned so
pending-in-mempool transactions do not get reissued.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/wallet.zig`:
- Around line 157-171: The broadcast path in wallet.zig is rolling back the
managed nonce too early when the receipt check is inconclusive. Update the
`sendRawTransaction`/`transactionLanded` flow so `transactionLanded` is not
treated as a hard negative after the short poll window; instead, widen or
parameterize the polling window in `transactionLanded` and only allow the
`errdefer` nonce rollback in the caller when a receipt lookup is definitively
negative. Keep the ambiguity-handling logic around `tx_hash`,
`provider.sendRawTransaction`, and `transactionLanded` aligned so
pending-in-mempool transactions do not get reissued.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0ef998d2-f45f-4dd0-af11-227a8f88ba0d

📥 Commits

Reviewing files that changed from the base of the PR and between 8c9a20d and 16ac046.

📒 Files selected for processing (2)

• src/kms.zig
• src/wallet.zig