Skip to content

fix(bot-kit): confirm mined replacements and fix premature stuck bumps - #98

Closed
cashd wants to merge 4 commits into
mainfrom
claude/crtr-2851-pending-queue-tx-confirmation
Closed

fix(bot-kit): confirm mined replacements and fix premature stuck bumps#98
cashd wants to merge 4 commits into
mainfrom
claude/crtr-2851-pending-queue-tx-confirmation

Conversation

@cashd

@cashd cashd commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Problem

Production incident (July 17, Base, midnight-liquidation, nonce 236):

  • Original tx 0xaedfb547…73cca was broadcast at 22:35:24.326 UTC and mined successfully in block 48,769,189.
  • Only 286 ms later the queue broadcast a fee-bump replacement 0xc95d215a…6dbee, which never landed.
  • The bot then emitted tx.dropped with reason: nonce_consumed instead of tx.confirmed.

Two defects combined:

  1. Stale stuck-age baseline. Both bots passed the chain head captured before quoting as the queue's submittedAtBlock. Quoting took ~10 s (~5 Base blocks), so the tx was born already > STUCK_BLOCKS old and got fee-bumped immediately — the 286 ms replacement.
  2. Replacement overwrote the tracked hash. pending-queue.ts kept only the latest hash per nonce; receipt checks and the nonce-consumed reconciler never looked at earlier broadcasts. When the original hash mined, the queue had forgotten it and misclassified the settlement as nonce_consumed.

Fix

  • Pending.txHashtxHashes[]: every hash broadcast for a nonce is retained; the last one remains the fee-replacement target (snapshot/logs still expose the latest hash).
  • Receipt sweeps and the reconciler scan all hashes (newest first). Any mined hash settles the entry as tx.confirmed / tx.reverted with the actually-mined hash and block; tx.dropped: nonce_consumed fires only when the nonce is consumed and none of our hashes has a receipt.
  • A per-hash receipt read failure no longer aborts the scan — an RPC hiccup on the replacement hash cannot mask a mined original, while a receipt-free scan that hit an error still propagates as transient.
  • Stuck-age is measured from the first onBlock that observes the broadcast (observedAtBlock), not from the tick's pre-quote head. queue.submit no longer takes blockNumber; both liquidation bots updated.

Tests

packages/bot-kit/test/queue/pending-queue.test.ts (40 pass):

  • original hash confirms / reverts after a replacement was broadcast (sweep path)
  • replacement hash confirms
  • consumed nonce settles as confirmed via the reconciler when an earlier hash mined
  • nonce_consumed only when no tracked hash has a receipt
  • a slow pre-submit quote does not cause an immediate fee bump (first sighting stamps the baseline)
  • existing replacement / cooldown / nonce-hole / snapshot behavior re-verified under the new baseline semantics

Per repo convention, one assertion was temporarily broken to confirm the suite fails, then restored.

Validation: @repo/bot-kit + both bots typecheck clean, bun lint 0 warnings, bun format clean, bun test green except pre-existing environment-dependent failures (midnight fork suite needs RPC_URL_8453 + anvil ≥1.5 — local anvil is 1.4.4, the version the CI comment documents as rejecting same-nonce replacements; crossed-books failures reproduce identically on origin/main).

Unresolved operational finding (documented, not fixed)

A second liquidation tx at nonce 237 (0x0017b634…d86fe) from the same signer repeated the liquidation in the next block, but neither the Midnight nor Blue Better Stack source has a corresponding tx.sent. The code offers no unlogged send path: the signer performs exactly one sendTransaction per send, and the queue logs tx.sent synchronously after every successful first-send. Two consistent-but-unproven explanations: (a) drop() deliberately skips the settle cooldown, so after the incorrect nonce_consumed drop the position was immediately re-eligible and a normal re-submit could have fired whose tx.sent line was lost by the opt-in in-process BetterStack shipper; (b) an external send from the same key. No fix is included because the code provides no concrete evidence of the source.

Fixes CRTR-2851 — https://linear.app/morpho-labs/issue/CRTR-2851/fixbot-kit-confirm-mined-replacements-and-fix-premature-stuck-bumps

The pending queue overwrote entry.txHash on fee replacement, so receipt
checks and nonce reconciliation only ever saw the latest hash. When the
ORIGINAL broadcast mined after a replacement was sent, the queue
misreported tx.dropped: nonce_consumed instead of tx.confirmed
(production, July 17, Base nonce 236). It also aged transactions from
the chain head the tick captured BEFORE quoting, so a slow (~10s) quote
made a fresh broadcast look >STUCK_BLOCKS old and triggered an
immediate, pointless fee bump.

- Track every hash broadcast for a nonce; scan them all (newest first)
  before replacing or classifying the nonce as consumed, and settle
  with the actually-mined hash and block.
- Emit tx.dropped: nonce_consumed only when the nonce is consumed and
  none of our hashes has a receipt.
- Measure stuck-age from the first onBlock that observes the broadcast;
  drop the pre-quote blockNumber from queue.submit (both bots updated).

Fixes CRTR-2851
@cashd cashd self-assigned this Jul 24, 2026
@cashd

cashd commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator Author

Simplify pass (140002c):

  1. Trimmed the STUCK_BLOCKS doc — it was narrating the bug this PR fixes; kept just the constraint (age is measured from first onBlock sighting, not a caller block).
  2. Deduped the "any hash can mine" explanation to one home on the txHashes field; dropped the findReceipt comment and tightened reconcile's to the settle-vs-drop rule.
  3. The "settles via the reconciler" test wasn't actually exercising the reconciler — the sweep found the receipt first. Now the receipt lands inside getConsumedNonce (which runs after the sweep), so the reconciler path is the one under test.

@cashd

cashd commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator Author

Review pass (244a2bd):

  1. findReceipt now scans past a per-hash getReceipt failure instead of aborting — a transient RPC error on the replacement hash was masking an already-mined original for that block. A receipt-free scan that hit an error still throws, so an RPC outage is never misread as "still pending". Added a test for it.

Two things surfaced but intentionally not changed: the reconciler settling a mined tx now applies the settled-cooldown (previously the nonce_consumed drop skipped it) — that's desired, a genuinely-mined tx should cool its label down; and the multi-hash sweep costs up to attempt+1 getReceipt reads per stuck entry per block, bounded by maxBumpAttempts — fine for now, can revisit if it shows up in RPC budgets.

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