Skip to content

improve(tx-client): typed error when broadcast tx cannot be confirmed#3467

Draft
droplet-rl wants to merge 1 commit into
masterfrom
droplet/T90K0AL22-C0AB30NK4LX-1780657234-684319-confirm-typed
Draft

improve(tx-client): typed error when broadcast tx cannot be confirmed#3467
droplet-rl wants to merge 1 commit into
masterfrom
droplet/T90K0AL22-C0AB30NK4LX-1780657234-684319-confirm-typed

Conversation

@droplet-rl

Copy link
Copy Markdown
Contributor

Summary

Slack discussion: continuation of the gasless stuck-queue reporting thread (PRs #3464, #3465, #3466). Opening as a draft because this changes TransactionClient.submit's contract slightly — worth a sanity check before merging.

TransactionClient._submit's confirmation loop previously warned and returned the broadcast TransactionResponse when wait() never produced a receipt within maxTries retries. Two problems flow from that:

  1. Callers couldn't tell broadcast-only from confirmed. sendAndConfirmTransaction would call .wait() again on the same response, potentially polling indefinitely against the RPC. The bot's state machine treated the tx as confirmed.
  2. No signal for the gasless stuck-queue counter (PR feat(gasless): typed transaction outcomes + stuck-queue submission reporting #3464). A tx that broadcasts but never confirms is precisely the "stuck queue" the user wants to count, but today it looks like a success internally.

Change

  • New TransactionConfirmationFailedError (in src/utils/TransactionUtils.ts) — extends Error, carries the submitted TransactionResponse so callers can correlate to the broadcast tx hash, and exposes a kind: "confirmation_failed" literal for future discriminated unions.
  • _submit throws it after the maxTries loop completes without a receipt, in addition to the existing warn-level log.
  • TransactionClient.submit whitelists it in the catch and re-throws (with its own warn log), so it propagates out rather than being swallowed alongside generic submission errors. All other error categories continue to be logged at info and swallowed at submit's existing surface — no regression for the ~15 direct callers that depend on submit's lenient contract.

Behavior change worth noting

Today: _submit with ensureConfirmation: true and a wait() that keeps failing eventually returns the broadcast response — sendAndConfirmTransaction's outer txResponse.wait() then potentially retries forever or returns late, and the caller proceeds as if confirmed.

After this change: the same scenario throws → sendAndConfirmTransaction (which catches all errors and returns undefined) treats it as a failure → caller's existing !isDefined(receipt) branch fires and logs "Failed to submit X tx". For the DepositAddressHandler and (post-#3464) GaslessRelayer callers, this is the desired behavior — silent give-up becomes loud failure.

Only callers that set ensureConfirmation: true are affected. In production today, only sendAndConfirmTransaction sets it (DepositAddressHandler × 3 sites + gasless × 2 sites).

Doc updates considered

No AGENTS.md / README.md shifts. The error class is a new export but the module map and runtime flow stay the same.

Test plan

  • yarn typecheck — clean.
  • yarn lint — clean.
  • yarn hardhat test test/TransactionClient.ts — 11/11 passing.
    • Replaced the existing "Gives up after maxTries exhausted" test (which asserted the silent-fall-through) with "Throws TransactionConfirmationFailedError when maxTries is exhausted".
    • Added "Generic submit errors are still swallowed (no regression)" to lock in the contract for non-confirmation errors.
  • yarn hardhat test test/TransactionClient.ts test/DepositAddressHandler.ts test/MultiCallerClient.ts test/TryMulticallClient.ts — 40/40 passing (broader sanity check on adjacent surfaces).
  • Reviewer sanity-check: any non-test caller relying on the old "broadcast-but-unconfirmed treated as success" semantics? Best place to look is anywhere setting ensureConfirmation: true outside of sendAndConfirmTransaction.

Stacking

🤖 Generated with Claude Code

`TransactionClient._submit`'s confirmation loop previously warned and
returned the broadcast `TransactionResponse` when `wait()` never produced
a receipt within `maxTries` retries. Callers couldn't distinguish a
confirmed tx from one that was broadcast but stuck unconfirmed —
`sendAndConfirmTransaction` would then call `.wait()` again on the same
response, potentially polling forever, while the gasless relayer's
stuck-queue reporting (PR #3464) had no signal to count it against.

Introduce `TransactionConfirmationFailedError`, throw it from `_submit`
after exhausting retries, and whitelist it in `TransactionClient.submit`
so it propagates out rather than being swallowed alongside generic
submission errors. Other error categories continue to be logged and
swallowed at submit's existing surface.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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