From 82291fc477b40aa0a4fd8a04e33c5ecedf33d819 Mon Sep 17 00:00:00 2001 From: Hayden Shively Date: Thu, 3 Sep 2026 02:40:50 -0500 Subject: [PATCH 1/6] fix(bot-kit): confirm mined replacements and fix premature stuck bumps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects in the shared pending queue compounded into liquidators fee-bumping transactions milliseconds after broadcast and then reporting the ones that mined as failures. The stuck-age baseline was whatever block the caller passed, and all five callers passed the head captured at tick entry — before discovery, quoting and simulation. A slow tick is several blocks, so an entry was born past `stuckBlocks` and the next `onBlock` replaced it. `blockNumber` now leaves `SubmitArgs` entirely: the queue stamps `submittedAtBlock` from the first `onBlock` that sights an entry, a head it has actually seen and therefore necessarily after the broadcast. `replaceStuck` resets it the same way, so a replacement is re-sighted rather than aged from a maintenance block that already spent a receipt read and a gas estimation. A fee bump also overwrote the hash it replaced, and both the receipt sweep and the nonce-consumed reconciler only ever looked at the latest one. When the original mined and the replacement did not, the queue saw a consumed nonce with no receipt of its own and logged `tx.dropped/nonce_consumed` for a transaction that had succeeded. Entries now keep every hash broadcast for a nonce, newest first and bounded by `maxBumpAttempts`; `scanReceipts` settles on the first hash with a receipt, and `nonce_consumed` fires only when a clean read of all of them found none. A read failure yields `unknown`, which never retires an entry — otherwise a transient error on the replacement would mask a mined original. Only `tx.confirmed` / `tx.reverted` change shape, to name the hash that actually mined; every other event still names the latest broadcast. Verified against blue-liquidation on Base, 2026-08-19: six `tx.bumped` events fired 176-422ms after their own `tx.sent` on a 2s-block chain, five of which then logged `nonce_consumed` for transactions that mined. Co-Authored-By: Claude Opus 5 (1M context) --- bots/blue-liquidation/src/index.ts | 5 +- bots/blue-liquidation/src/runner/tick.ts | 2 - .../blue-liquidation/test/runner/tick.test.ts | 7 +- .../application/crossed-books-bot.service.ts | 7 +- bots/midnight-crossed-books/src/bootstrap.ts | 4 +- .../resolver/resolver.service.ts | 4 +- .../resolver/resolver.transport.ts | 8 +- .../crossed-books-bot.service.test.ts | 24 +- .../resolver/resolver.service.test.ts | 4 +- .../resolver/resolver.transport.test.ts | 2 +- bots/midnight-liquidation/README.md | 6 + bots/midnight-liquidation/src/index.ts | 5 +- bots/midnight-liquidation/src/runner/tick.ts | 2 - .../test/fork/queue.test.ts | 5 +- .../test/runner/tick.test.ts | 7 +- bots/vault-v1-reallocation/src/index.ts | 5 +- bots/vault-v2-reallocation/src/index.ts | 5 +- packages/bot-kit/src/queue/pending-queue.ts | 117 ++++--- packages/bot-kit/src/queue/receipt.utils.ts | 42 +++ .../bot-kit/test/queue/pending-queue.test.ts | 314 ++++++++++++++---- 20 files changed, 409 insertions(+), 166 deletions(-) create mode 100644 packages/bot-kit/src/queue/receipt.utils.ts diff --git a/bots/blue-liquidation/src/index.ts b/bots/blue-liquidation/src/index.ts index 7fef2462..c46c2055 100644 --- a/bots/blue-liquidation/src/index.ts +++ b/bots/blue-liquidation/src/index.ts @@ -276,7 +276,7 @@ async function main() { eoa, data: encodeExec(market, borrower, plan, swapPlan) }), - submit: async ({ market, borrower, plan, swapPlan, blockNumber, label }) => { + submit: async ({ market, borrower, plan, swapPlan, label }) => { const fees = initialFees(await signer.getBaseFee(), config.maxFeeWei) return queue.submit({ request: { @@ -285,8 +285,7 @@ async function main() { }, label, maxFeePerGas: fees.maxFeePerGas, - maxPriorityFeePerGas: fees.maxPriorityFeePerGas, - blockNumber + maxPriorityFeePerGas: fees.maxPriorityFeePerGas }) }, backoff, diff --git a/bots/blue-liquidation/src/runner/tick.ts b/bots/blue-liquidation/src/runner/tick.ts index c7c9744d..e1101622 100644 --- a/bots/blue-liquidation/src/runner/tick.ts +++ b/bots/blue-liquidation/src/runner/tick.ts @@ -119,7 +119,6 @@ export async function runTick(deps: { borrower: Address plan: LiquidationPlan swapPlan: SwapPlan - blockNumber: bigint label: string }) => Promise /** Per-position exponential backoff suppressing repeated quote/simulate failures (rate-limit defense). */ @@ -269,7 +268,6 @@ export async function runTick(deps: { borrower: pair.borrower, plan: liquidationPlan, swapPlan, - blockNumber: chainHead, label }) if (outcome.sent) { diff --git a/bots/blue-liquidation/test/runner/tick.test.ts b/bots/blue-liquidation/test/runner/tick.test.ts index 10bac914..87ff2e5e 100644 --- a/bots/blue-liquidation/test/runner/tick.test.ts +++ b/bots/blue-liquidation/test/runner/tick.test.ts @@ -119,7 +119,7 @@ function runWith(opts: { /** Shared spy, so a caller can observe the tick's and the queue's events in ONE stream. */ spy?: ReturnType /** Replaces the stub `submit` — used to broadcast through a real pending queue. */ - submitWith?: (args: { label: string; blockNumber: bigint }) => Promise + submitWith?: (args: { label: string }) => Promise }) { const { logger, events } = opts.spy ?? spyLogger() let simulateCalls = 0 @@ -468,13 +468,12 @@ describe('runTick', () => { }) await runWith({ spy, - submitWith: ({ label, blockNumber }) => + submitWith: ({ label }) => queue.submit({ request: { to: ROUTER, data: '0x' }, label, maxFeePerGas: 1000n, - maxPriorityFeePerGas: 1000n, - blockNumber + maxPriorityFeePerGas: 1000n }) }) const planBuilt = spy.events.find(e => e.event === 'plan.built') diff --git a/bots/midnight-crossed-books/src/application/crossed-books-bot.service.ts b/bots/midnight-crossed-books/src/application/crossed-books-bot.service.ts index f9494820..a163f79f 100644 --- a/bots/midnight-crossed-books/src/application/crossed-books-bot.service.ts +++ b/bots/midnight-crossed-books/src/application/crossed-books-bot.service.ts @@ -20,7 +20,7 @@ export interface OrderBookService { export interface ResolverService { simulate(matches: readonly CrossedMatch[]): Promise - submit(prepared: PreparedResolution, blockNumber: bigint): Promise + submit(prepared: PreparedResolution): Promise } interface BotLogger { @@ -53,11 +53,10 @@ export class CrossedBooksBotService { /** * Computes the first profitable crossed resolution for one block. - * @param blockNumber - Block label used only when queueing a write-mode transaction. * @returns Submission status and the number of listed markets inspected. * @remarks Always simulates first. Readonly mode logs `match.computed` and performs no submission. */ - async run({ blockNumber }: { blockNumber: bigint }) { + async run() { const markets = await this.markets.listListedActiveMarkets() const inflight = this.inflightMarketIds() let computed = false @@ -95,7 +94,7 @@ export class CrossedBooksBotService { continue } - await this.resolver.submit(simulation.prepared, blockNumber) + await this.resolver.submit(simulation.prepared) this.logger.info('match.submitted', fields) return { submitted: true, markets: markets.length } diff --git a/bots/midnight-crossed-books/src/bootstrap.ts b/bots/midnight-crossed-books/src/bootstrap.ts index de119d52..3e628f32 100644 --- a/bots/midnight-crossed-books/src/bootstrap.ts +++ b/bots/midnight-crossed-books/src/bootstrap.ts @@ -146,10 +146,10 @@ export async function createApplication( let nextScanAt = 0 const runner = createRunner({ getBlockNumber: () => getBlockNumber(chainClient), - tick: async blockNumber => { + tick: async () => { if (Date.now() < nextScanAt || (queue?.size ?? 0) > 0) return nextScanAt = Date.now() + config.scanIntervalMs - await bot.run({ blockNumber }) + await bot.run() }, maintain: async blockNumber => { await queue?.onBlock(blockNumber) diff --git a/bots/midnight-crossed-books/src/infrastructure/resolver/resolver.service.ts b/bots/midnight-crossed-books/src/infrastructure/resolver/resolver.service.ts index 76fde26b..aba87940 100644 --- a/bots/midnight-crossed-books/src/infrastructure/resolver/resolver.service.ts +++ b/bots/midnight-crossed-books/src/infrastructure/resolver/resolver.service.ts @@ -29,7 +29,7 @@ export class ResolverExecutionService implements ResolverService { } } - submit(prepared: PreparedResolution, blockNumber: bigint) { - return this.transport.submit(prepared, blockNumber) + submit(prepared: PreparedResolution) { + return this.transport.submit(prepared) } } diff --git a/bots/midnight-crossed-books/src/infrastructure/resolver/resolver.transport.ts b/bots/midnight-crossed-books/src/infrastructure/resolver/resolver.transport.ts index b40bd26b..03670962 100644 --- a/bots/midnight-crossed-books/src/infrastructure/resolver/resolver.transport.ts +++ b/bots/midnight-crossed-books/src/infrastructure/resolver/resolver.transport.ts @@ -16,7 +16,7 @@ export type ResolverSimulation = export interface ResolverTransport { simulate(data: Hex): Promise - submit(prepared: PreparedResolution, blockNumber: bigint): Promise + submit(prepared: PreparedResolution): Promise } export class ViemResolverTransport implements ResolverTransport { @@ -62,19 +62,17 @@ export class ViemResolverTransport implements ResolverTransport { /** * Queues the immutable request prepared by simulation. * @param prepared - Resolver target calldata and market label. - * @param blockNumber - Block used to seed queue fee and replacement policy. * @returns A promise that resolves once the request is accepted by the queue. * @throws `ReadonlyMutationError` when submission dependencies were intentionally omitted. */ - async submit(prepared: PreparedResolution, blockNumber: bigint) { + async submit(prepared: PreparedResolution) { if (!this.submission) throw new ReadonlyMutationError() const fees = initialFees(await this.submission.signer.getBaseFee(), this.submission.maxFeeWei) await this.submission.queue.submit({ request: { to: this.resolver, data: prepared.data }, label: prepared.marketId, - ...fees, - blockNumber + ...fees }) } } diff --git a/bots/midnight-crossed-books/test/application/crossed-books-bot.service.test.ts b/bots/midnight-crossed-books/test/application/crossed-books-bot.service.test.ts index 8963c5bc..062484a0 100644 --- a/bots/midnight-crossed-books/test/application/crossed-books-bot.service.test.ts +++ b/bots/midnight-crossed-books/test/application/crossed-books-bot.service.test.ts @@ -89,7 +89,7 @@ describe('CrossedBooksBotService', () => { test('loads listed active markets once per run', async () => { const { service, listListedActiveMarkets } = setup() - await service.run({ blockNumber: 10n }) + await service.run() expect(listListedActiveMarkets).toHaveBeenCalledTimes(1) }) @@ -99,7 +99,7 @@ describe('CrossedBooksBotService', () => { inflight: new Set([MARKET_ID]) }) - const result = await service.run({ blockNumber: 10n }) + const result = await service.run() expect(result).toEqual({ submitted: false, markets: 1 }) expect(getTakeableBook).not.toHaveBeenCalled() @@ -109,7 +109,7 @@ describe('CrossedBooksBotService', () => { test('does not simulate or submit when books do not cross', async () => { const { service, simulate, submit } = setup({ matches: [] }) - await service.run({ blockNumber: 10n }) + await service.run() expect(simulate).not.toHaveBeenCalled() expect(submit).not.toHaveBeenCalled() @@ -120,7 +120,7 @@ describe('CrossedBooksBotService', () => { simulation: { status: 'revert', reason: 'InsufficientProfit' } }) - await service.run({ blockNumber: 10n }) + await service.run() expect(submit).not.toHaveBeenCalled() }) @@ -128,7 +128,7 @@ describe('CrossedBooksBotService', () => { test('simulates every crossed offer in one resolution', async () => { const { service, match, simulate } = setup() - await service.run({ blockNumber: 10n }) + await service.run() expect(match).toHaveBeenCalledWith({ asks: [MATCH.ask], @@ -141,7 +141,7 @@ describe('CrossedBooksBotService', () => { test('uses the configured match cap', async () => { const { service, match } = setup({ maxMatches: 3 }) - await service.run({ blockNumber: 10n }) + await service.run() expect(match).toHaveBeenCalledWith({ asks: [MATCH.ask], @@ -158,9 +158,9 @@ describe('CrossedBooksBotService', () => { } const { service, submit } = setup({ simulation: { status: 'ok', prepared } }) - const result = await service.run({ blockNumber: 10n }) + const result = await service.run() - expect(submit).toHaveBeenCalledWith(prepared, 10n) + expect(submit).toHaveBeenCalledWith(prepared) expect(result).toEqual({ submitted: true, markets: 1 }) }) @@ -175,7 +175,7 @@ describe('CrossedBooksBotService', () => { simulation: { status: 'ok', prepared } }) - const result = await service.run({ blockNumber: 10n }) + const result = await service.run() expect(submit).not.toHaveBeenCalled() expect(logger.info).toHaveBeenCalledWith('match.computed', { @@ -192,7 +192,7 @@ describe('CrossedBooksBotService', () => { markets: [MARKET, OTHER_MARKET] }) - const result = await service.run({ blockNumber: 10n }) + const result = await service.run() expect(getTakeableBook).toHaveBeenCalledTimes(2) expect(simulate).toHaveBeenCalledTimes(2) @@ -221,7 +221,7 @@ describe('CrossedBooksBotService', () => { } }) - const result = await service.run({ blockNumber: 10n }) + const result = await service.run() expect(calls).toBe(2) expect(submit).toHaveBeenCalledTimes(1) @@ -231,7 +231,7 @@ describe('CrossedBooksBotService', () => { test('stops after the first successful submission', async () => { const { service, getTakeableBook, submit } = setup({ markets: [MARKET, OTHER_MARKET] }) - await service.run({ blockNumber: 10n }) + await service.run() expect(getTakeableBook).toHaveBeenCalledTimes(1) expect(submit).toHaveBeenCalledTimes(1) diff --git a/bots/midnight-crossed-books/test/infrastructure/resolver/resolver.service.test.ts b/bots/midnight-crossed-books/test/infrastructure/resolver/resolver.service.test.ts index 7f323f8b..cdffadd6 100644 --- a/bots/midnight-crossed-books/test/infrastructure/resolver/resolver.service.test.ts +++ b/bots/midnight-crossed-books/test/infrastructure/resolver/resolver.service.test.ts @@ -73,8 +73,8 @@ describe('ResolverExecutionService', () => { profit: 42n } - await service.submit(prepared, 99n) + await service.submit(prepared) - expect(submit).toHaveBeenCalledWith(prepared, 99n) + expect(submit).toHaveBeenCalledWith(prepared) }) }) diff --git a/bots/midnight-crossed-books/test/infrastructure/resolver/resolver.transport.test.ts b/bots/midnight-crossed-books/test/infrastructure/resolver/resolver.transport.test.ts index 9b5a9d91..5e99d109 100644 --- a/bots/midnight-crossed-books/test/infrastructure/resolver/resolver.transport.test.ts +++ b/bots/midnight-crossed-books/test/infrastructure/resolver/resolver.transport.test.ts @@ -46,7 +46,7 @@ describe('ViemResolverTransport', () => { const transport = new ViemResolverTransport(client, ADDRESS, ADDRESS) await expect( - transport.submit({ marketId: MARKET_ID, data: '0x1234', profit: 42n }, 10n) + transport.submit({ marketId: MARKET_ID, data: '0x1234', profit: 42n }) ).rejects.toBeInstanceOf(ReadonlyMutationError) }) }) diff --git a/bots/midnight-liquidation/README.md b/bots/midnight-liquidation/README.md index 53dd11e4..9e0b34ff 100644 --- a/bots/midnight-liquidation/README.md +++ b/bots/midnight-liquidation/README.md @@ -141,6 +141,12 @@ same behaviour keeps the same timing on a ~2s and a ~12s chain. | `reconcileEveryBlocks` / `balanceEveryBlocks` | `3` / `30n` | `1` / `5n` | Same, for nonce reconciliation and the gas metric. | | `maxBumpAttempts` | `3` | `6` | Mainnet basefee can climb 12.5% per block. | +`stuckBlocks` is counted from the block that first _sights_ a broadcast, not from the block the tick +started on, so the wait before a bump is one block longer than the number itself — on mainnet's `1n` +that is the difference between one block and two. The baseline is deliberately sourced this way: a +tick's head is captured before quoting and simulation, and ageing against it once fee-bumped fresh +transactions within a second of sending them. + **Env-overridable** — the chain's row supplies the default, the env var wins: | Default | Base (8453) | Mainnet (1) | Why it differs | diff --git a/bots/midnight-liquidation/src/index.ts b/bots/midnight-liquidation/src/index.ts index 78a08a2e..ea3f6fc4 100644 --- a/bots/midnight-liquidation/src/index.ts +++ b/bots/midnight-liquidation/src/index.ts @@ -365,7 +365,7 @@ async function main() { eoa, data: encodeExec(market, borrower, plan, swapPlan) }), - submit: async ({ market, borrower, plan, swapPlan, blockNumber, label }) => { + submit: async ({ market, borrower, plan, swapPlan, label }) => { const fees = initialFees(await signer.getBaseFee(), config.maxFeeWei, config.priorityFeeWei) return queue.submit({ request: { @@ -381,8 +381,7 @@ async function main() { postMaturityMode: plan.postMaturityMode }, maxFeePerGas: fees.maxFeePerGas, - maxPriorityFeePerGas: fees.maxPriorityFeePerGas, - blockNumber + maxPriorityFeePerGas: fees.maxPriorityFeePerGas }) }, backoff, diff --git a/bots/midnight-liquidation/src/runner/tick.ts b/bots/midnight-liquidation/src/runner/tick.ts index f1e46dbe..3a965f6e 100644 --- a/bots/midnight-liquidation/src/runner/tick.ts +++ b/bots/midnight-liquidation/src/runner/tick.ts @@ -616,7 +616,6 @@ export async function runTick(deps: { borrower: Address plan: LiquidationPlan swapPlan: SwapPlan | null - blockNumber: bigint label: string }) => Promise /** Per-position exponential backoff suppressing repeated quote/simulate failures (rate-limit defense). */ @@ -1017,7 +1016,6 @@ export async function runTick(deps: { borrower: pair.borrower, plan: liquidationPlan, swapPlan, - blockNumber: chainHead, label }) if (outcome.sent) { diff --git a/bots/midnight-liquidation/test/fork/queue.test.ts b/bots/midnight-liquidation/test/fork/queue.test.ts index 6135916e..a3130965 100644 --- a/bots/midnight-liquidation/test/fork/queue.test.ts +++ b/bots/midnight-liquidation/test/fork/queue.test.ts @@ -61,8 +61,7 @@ describe('fork: pending-queue bump + replacement against a real node', () => { request: { to: LIQUIDATOR, data: '0x' }, label: 'queue-fork', maxFeePerGas: fees.maxFeePerGas, - maxPriorityFeePerGas: fees.maxPriorityFeePerGas, - blockNumber: 0n + maxPriorityFeePerGas: fees.maxPriorityFeePerGas }) expect(queue.size).toBe(1) const original = queue.snapshot()[0] @@ -71,7 +70,7 @@ describe('fork: pending-queue bump + replacement against a real node', () => { // 2. Advance past STUCK_BLOCKS (4) without mining → real getReceipt returns null, so onBlock detects // the stuck tx and replaces it at the same nonce with ≥12.5% higher fees (a fresh hash). - for (let block = 1n; block <= 5n; block++) await queue.onBlock(block) + for (let block = 1n; block <= 6n; block++) await queue.onBlock(block) const bumped = queue.snapshot()[0] if (!bumped) throw new Error('expected the bumped entry to remain pending') expect(queue.size).toBe(1) diff --git a/bots/midnight-liquidation/test/runner/tick.test.ts b/bots/midnight-liquidation/test/runner/tick.test.ts index d1d6f856..00e795f4 100644 --- a/bots/midnight-liquidation/test/runner/tick.test.ts +++ b/bots/midnight-liquidation/test/runner/tick.test.ts @@ -231,7 +231,7 @@ function runWith(opts: { /** Shared spy, so a caller can observe the tick's and the queue's events in ONE stream. */ spy?: ReturnType /** Replaces the stub `submit` — used to broadcast through a real pending queue. */ - submitWith?: (args: { label: string; blockNumber: bigint }) => Promise + submitWith?: (args: { label: string }) => Promise }) { const { logger, events } = opts.spy ?? spyLogger() const order: Address[] = [] @@ -1815,13 +1815,12 @@ describe('runTick', () => { }) await runWith({ spy, - submitWith: ({ label, blockNumber }) => + submitWith: ({ label }) => queue.submit({ request: { to: ROUTER, data: '0x' }, label, maxFeePerGas: 1000n, - maxPriorityFeePerGas: 1000n, - blockNumber + maxPriorityFeePerGas: 1000n }) }) const planBuilt = spy.events.find(e => e.event === 'plan.built') diff --git a/bots/vault-v1-reallocation/src/index.ts b/bots/vault-v1-reallocation/src/index.ts index c15354c1..1aab760f 100644 --- a/bots/vault-v1-reallocation/src/index.ts +++ b/bots/vault-v1-reallocation/src/index.ts @@ -149,14 +149,13 @@ async function main() { // withdrawals/deposits, market removed from the queue — means do not send; the tick gates on // `ok` only. simulate: (vault, data) => simulateCall(client, { eoa, to: vault, data }), - submit: async ({ vault, data, blockNumber }) => { + submit: async ({ vault, data }) => { const fees = initialFees(await signer.getBaseFee(), config.maxFeeWei) return queue.submit({ request: { to: vault, data }, label: vault, maxFeePerGas: fees.maxFeePerGas, - maxPriorityFeePerGas: fees.maxPriorityFeePerGas, - blockNumber + maxPriorityFeePerGas: fees.maxPriorityFeePerGas }) }, dryRun: config.dryRun, diff --git a/bots/vault-v2-reallocation/src/index.ts b/bots/vault-v2-reallocation/src/index.ts index 16682681..42003bec 100644 --- a/bots/vault-v2-reallocation/src/index.ts +++ b/bots/vault-v2-reallocation/src/index.ts @@ -143,14 +143,13 @@ async function main() { // Byte-for-byte what gets broadcast. A revert here — role revoked, cap exceeded, insufficient // idle, market no longer enabled — means do not send; the tick gates on `ok` only. simulate: (vault, data) => simulateCall(client, { eoa, to: vault, data }), - submit: async ({ vault, data, blockNumber }) => { + submit: async ({ vault, data }) => { const fees = initialFees(await signer.getBaseFee(), config.maxFeeWei) return queue.submit({ request: { to: vault, data }, label: vault, maxFeePerGas: fees.maxFeePerGas, - maxPriorityFeePerGas: fees.maxPriorityFeePerGas, - blockNumber + maxPriorityFeePerGas: fees.maxPriorityFeePerGas }) }, dryRun: config.dryRun, diff --git a/packages/bot-kit/src/queue/pending-queue.ts b/packages/bot-kit/src/queue/pending-queue.ts index cbe92b0d..1c7230bd 100644 --- a/packages/bot-kit/src/queue/pending-queue.ts +++ b/packages/bot-kit/src/queue/pending-queue.ts @@ -13,6 +13,7 @@ import { } from '../revert.utils' import { TxSendError } from '../tx-send.error' import { bumpFees } from './fee-policy' +import { scanReceipts } from './receipt.utils' /** Default blocks a pending tx may sit unconfirmed before the queue bumps its fee and replaces it. */ export const STUCK_BLOCKS = 4n @@ -72,7 +73,6 @@ export type SubmitArgs = { correlation?: Readonly> maxFeePerGas: bigint maxPriorityFeePerGas: bigint - blockNumber: bigint } /** @@ -100,10 +100,23 @@ export type SubmitOutcome = /** One tracked tx — the queue's full per-nonce record. */ type Pending = { nonce: number - txHash: Hex + /** + * Every hash broadcast for this nonce, newest first — `[0]` is the live replacement target and the + * one the send-side log events name. A fee bump cannot un-broadcast what it replaced, so any hash + * here may be the one that mines; retiring the nonce requires {@link scanReceipts} to clear them + * all. Non-empty by construction, and bounded by `maxBumpAttempts`. + */ + txHashes: [Hex, ...Hex[]] request: TxRequest label: string - submittedAtBlock: bigint + /** + * Head at which the queue FIRST OBSERVED this broadcast, which is what `stuckBlocks` ages against. + * `null` until an `onBlock` sights it: a caller's block is captured before quoting and simulation + * and can already be several blocks stale by the time the send resolves, which would make a fresh + * transaction look stuck and bump it before it could possibly mine. Sighting can only run late, + * never early, so the error is always in the safe direction. + */ + submittedAtBlock: bigint | null maxFeePerGas: bigint maxPriorityFeePerGas: bigint /** Gas the node estimated for this tx — the other half of what a bump is allowed to cost. */ @@ -266,6 +279,24 @@ export function createPendingQueue({ } } + // Retires an entry the chain settled, naming the hash that ACTUALLY mined — which after a fee bump + // need not be the latest one broadcast, and is the whole point of tracking every hash per nonce. + function settleMined( + entry: Pending, + mined: { txHash: Hex; receipt: TxReceiptLite }, + blockNumber: bigint + ): void { + settle(entry, blockNumber) + const fields = { + id: entry.label, + nonce: entry.nonce, + txHash: mined.txHash, + blockNumber: mined.receipt.blockNumber + } + if (mined.receipt.status === 'success') logger.info('tx.confirmed', fields) + else logger.warn('tx.reverted', fields) + } + // The nonce-critical section, always entered under `submitMutex`: the latch checks, the empty-queue // `syncNonce`, the `send` that claims a nonce, and the tracking insert must all observe the same // `pending` snapshot. In particular the `pending.size === 0` test has to sit INSIDE the lock — a @@ -337,10 +368,10 @@ export function createPendingQueue({ const { nonce, txHash, gas } = sent.data pending.set(nonce, { nonce, - txHash, + txHashes: [txHash], request: args.request, label: args.label, - submittedAtBlock: args.blockNumber, + submittedAtBlock: null, maxFeePerGas: args.maxFeePerGas, maxPriorityFeePerGas: args.maxPriorityFeePerGas, gas, @@ -381,7 +412,7 @@ export function createPendingQueue({ logger.warn('tx.dropped', { id: entry.label, nonce: entry.nonce, - txHash: entry.txHash, + txHash: entry.txHashes[0], reason: 'max_bump_attempts' }) return @@ -398,7 +429,7 @@ export function createPendingQueue({ logger.warn('tx.dropped', { id: entry.label, nonce: entry.nonce, - txHash: entry.txHash, + txHash: entry.txHashes[0], reason: 'fee_ceiling' }) return @@ -414,7 +445,7 @@ export function createPendingQueue({ logger.warn('tx.dropped', { id: entry.label, nonce: entry.nonce, - txHash: entry.txHash, + txHash: entry.txHashes[0], reason: 'reverts_on_replace', detail: revertReason(replaced.error) }) @@ -423,18 +454,20 @@ export function createPendingQueue({ logger.warn('tx.replace_failed', { id: entry.label, nonce: entry.nonce, - txHash: entry.txHash, + txHash: entry.txHashes[0], attempt: entry.attempt, reason: revertReason(replaced.error) }) } return } - const oldHash = entry.txHash - entry.txHash = replaced.data.txHash + const oldHash = entry.txHashes[0] + entry.txHashes.unshift(replaced.data.txHash) entry.maxFeePerGas = result.fees.maxFeePerGas entry.maxPriorityFeePerGas = result.fees.maxPriorityFeePerGas - entry.submittedAtBlock = blockNumber + // Re-sighted like a first send: this pass already spent a receipt read per entry and a gas + // estimation on the replacement, so the block it started from can itself be stale. + entry.submittedAtBlock = null entry.attempt += 1 logger.info('tx.bumped', { id: entry.label, @@ -449,7 +482,7 @@ export function createPendingQueue({ // Drops tracked txs whose nonce is already consumed on-chain but that never produced a receipt for // us — an external send, competing signer, or reorg claimed the nonce, so our tx can never mine. - async function reconcile(): Promise { + async function reconcile(blockNumber: bigint): Promise { if (!getConsumedNonce || pending.size === 0) return const count = await tryCatch(getConsumedNonce()) if (count.error) { @@ -459,8 +492,12 @@ export function createPendingQueue({ // Deleting the current key mid-iteration (via `drop`) is well-defined for a Map. for (const entry of pending.values()) { if (entry.nonce >= count.data) continue - const receipt = await tryCatch(getReceipt(entry.txHash)) - if (!receipt.error && !receipt.data) drop(entry.nonce, 'nonce_consumed') + const scan = await scanReceipts(getReceipt, entry.txHashes) + // A consumed nonce whose receipt is ours is a settlement, not a loss — the sweep can miss it + // when the receipt lands between the two passes, or when its read failed and this one didn't. + if (scan.kind === 'mined') settleMined(entry, scan, blockNumber) + // `unknown` leaves "none of our hashes mined" unproven, so it must never retire the nonce. + else if (scan.kind === 'none') drop(entry.nonce, 'nonce_consumed') } } @@ -480,9 +517,9 @@ export function createPendingQueue({ // Nonce-consumed reconciliation on a fixed block cadence (chain truth cleaning up entries the // receipt loop can't see — an external/competing send under the same nonce). - async function reconcileOnCadence(): Promise { + async function reconcileOnCadence(blockNumber: bigint): Promise { blocksSeen += 1 - if (blocksSeen % reconcileEveryBlocks === 0) await reconcile() + if (blocksSeen % reconcileEveryBlocks === 0) await reconcile(blockNumber) } // Expire cooldowns so a position the bot acted on long ago is eligible again. By now the read RPC @@ -505,8 +542,20 @@ export function createPendingQueue({ // Per-entry isolation: one entry's transient read failure (getReceipt/getBaseFee) must not // abort the sweep for the rest of the queue. replaceStuck owns its own send-error handling. try { - const receipt = await getReceipt(entry.txHash) - if (receipt) { + const scan = await scanReceipts(getReceipt, entry.txHashes) + if (scan.kind === 'unknown') { + // Every hash read failed, so this entry's fate is unknown this pass. Skipping the stuck + // check is deliberate: bumping on an unreadable receipt would replace a tx that may have + // already mined. + logger.warn('tx.onblock_error', { + id: entry.label, + nonce: entry.nonce, + txHash: entry.txHashes[0], + reason: revertReason(scan.error) + }) + continue + } + if (scan.kind === 'mined') { // One-block receipt finality: a receipt is treated as terminal (confirm or revert) the // moment it appears. Free on the L2s most of these bots target (Base, Robinhood / // Arbitrum-Orbit), which do not reorg confirmed transactions in practice; Ethereum @@ -515,22 +564,12 @@ export function createPendingQueue({ // re-liquidated — never a queue entry stuck waiting on a vanished tx — but the re-plan // can broadcast a second tx while the first is still pending, and whichever lands second // reverts on-chain at the cost of its gas. - settle(entry, blockNumber) - if (receipt.status === 'success') { - logger.info('tx.confirmed', { - id: entry.label, - nonce: entry.nonce, - txHash: entry.txHash, - blockNumber: receipt.blockNumber - }) - } else { - logger.warn('tx.reverted', { - id: entry.label, - nonce: entry.nonce, - txHash: entry.txHash, - blockNumber: receipt.blockNumber - }) - } + settleMined(entry, scan, blockNumber) + continue + } + if (entry.submittedAtBlock === null) { + // First sighting: stamp the baseline and give the tx a full stuck window from here. + entry.submittedAtBlock = blockNumber continue } if (blockNumber - entry.submittedAtBlock > stuckBlocks) { @@ -541,12 +580,12 @@ export function createPendingQueue({ logger.warn('tx.onblock_error', { id: entry.label, nonce: entry.nonce, - txHash: entry.txHash, + txHash: entry.txHashes[0], reason: revertReason(error) }) } } - await reconcileOnCadence() + await reconcileOnCadence(blockNumber) // While a nonce hole is latched, check every block whether the chain has caught up past it. await clearNonceHoleIfFilled() pruneSettledCooldowns(blockNumber) @@ -559,7 +598,7 @@ export function createPendingQueue({ logger.warn('tx.dropped', { id: entry.label, nonce: entry.nonce, - txHash: entry.txHash, + txHash: entry.txHashes[0], reason }) settle(entry) @@ -575,7 +614,7 @@ export function createPendingQueue({ snapshot() { return [...pending.values()].map(entry => ({ nonce: entry.nonce, - txHash: entry.txHash, + txHash: entry.txHashes[0], attempt: entry.attempt })) }, diff --git a/packages/bot-kit/src/queue/receipt.utils.ts b/packages/bot-kit/src/queue/receipt.utils.ts new file mode 100644 index 00000000..0c798717 --- /dev/null +++ b/packages/bot-kit/src/queue/receipt.utils.ts @@ -0,0 +1,42 @@ +import type { Hex } from 'viem' + +import { tryCatch } from '@repo/utils' + +import type { GetReceipt, TxReceiptLite } from './pending-queue' + +/** + * Outcome of scanning every hash broadcast for one nonce. + * + * The `none`/`unknown` split is load-bearing: only `none` proves our transaction cannot have mined, + * so only `none` may retire an entry. `unknown` means a read failed and the scan therefore cannot + * distinguish "not mined" from "mined but unreadable" — a caller must treat it as transient and try + * again on the next pass. + */ +type ReceiptScan = + | { kind: 'mined'; txHash: Hex; receipt: TxReceiptLite } + | { kind: 'none' } + | { kind: 'unknown'; error: unknown } + +/** + * Finds the hash that mined among every hash broadcast for one nonce, newest first. + * + * A fee bump replaces a transaction but cannot un-broadcast it, so any hash in the list may be the + * one the chain kept. A per-hash read failure is recorded and the scan CONTINUES rather than + * aborting — otherwise a transient error on the newest hash would mask a mined original and the + * queue would report a successful transaction as dropped. + */ +export const scanReceipts = async ( + getReceipt: GetReceipt, + txHashes: readonly Hex[] +): Promise => { + let failure: { error: unknown } | null = null + for (const txHash of txHashes) { + const receipt = await tryCatch(getReceipt(txHash)) + if (receipt.error) { + failure ??= { error: receipt.error } + continue + } + if (receipt.data) return { kind: 'mined', txHash, receipt: receipt.data } + } + return failure ? { kind: 'unknown', error: failure.error } : { kind: 'none' } +} diff --git a/packages/bot-kit/test/queue/pending-queue.test.ts b/packages/bot-kit/test/queue/pending-queue.test.ts index 08f6f479..edc759f7 100644 --- a/packages/bot-kit/test/queue/pending-queue.test.ts +++ b/packages/bot-kit/test/queue/pending-queue.test.ts @@ -100,16 +100,26 @@ function setup( } } -function submitOne(queue: PendingQueue, blockNumber = 0n) { +function submitOne(queue: PendingQueue) { return queue.submit({ request: REQUEST, label: 'market:borrower', maxFeePerGas: 1000n, - maxPriorityFeePerGas: 1000n, - blockNumber + maxPriorityFeePerGas: 1000n }) } +/** + * Submits, then lets one `onBlock` sight the entry so its stuck-age baseline is stamped at `at`. + * The queue no longer takes a caller block, so a test that wants an entry aged from a known height + * has to give it that sighting pass first. + */ +async function submitSighted(queue: PendingQueue, at = 0n) { + const outcome = await submitOne(queue) + await queue.onBlock(at) + return outcome +} + describe('createPendingQueue', () => { it('records a submitted tx with the signer-assigned nonce', async () => { const { queue, sends } = setup() @@ -128,7 +138,7 @@ describe('createPendingQueue', () => { it('leaves a tx pending until it is stuck past stuckBlocks', async () => { const { queue, sends } = setup() - await submitOne(queue, 0n) + await submitSighted(queue, 0n) await queue.onBlock(4n) // age 4, not yet > 4 expect(queue.size).toBe(1) expect(sends).toHaveLength(1) // no replacement @@ -136,7 +146,7 @@ describe('createPendingQueue', () => { it('bumps and replaces a stuck tx at the same nonce', async () => { const { queue, sends } = setup({ baseFee: 100n }) - await submitOne(queue, 0n) + await submitSighted(queue, 0n) await queue.onBlock(5n) // age 5 > 4 → bump expect(sends).toHaveLength(2) expect(sends[1]?.nonce).toBe(7) // replacement pins the original nonce @@ -156,13 +166,16 @@ describe('createPendingQueue', () => { maxFeeWei: 1_000_000_000n, maxSpendWei: 1_400n * STUB_GAS }) - await submitOne(queue, 0n) + await submitSighted(queue, 0n) await queue.onBlock(5n) expect(sends).toHaveLength(2) // 1325 wei/gas x 1e6 gas = 1.325e9 <= budget expect(sends[1]?.maxFeePerGas).toBe(1325n) - await queue.onBlock(10n) + // A replacement is re-sighted like a first send, so each further ladder step needs its own + // sighting block before the block that finds it stuck. + await queue.onBlock(10n) // sights the replacement + await queue.onBlock(15n) expect(sends).toHaveLength(2) // 1490 breaches the budget, so nothing is broadcast expect(queue.size).toBe(0) expect(events.some(e => e.event === 'tx.dropped' && e.fields?.reason === 'fee_ceiling')).toBe( @@ -178,7 +191,7 @@ describe('createPendingQueue', () => { maxFeeWei: 1_000_000_000n, maxSpendWei: 1_000_000_000n * STUB_GAS }) - await submitOne(queue, 0n) + await submitSighted(queue, 0n) await queue.onBlock(5n) expect(sends).toHaveLength(2) @@ -187,18 +200,21 @@ describe('createPendingQueue', () => { it('drops a tx after maxBumpAttempts bumps', async () => { const { queue } = setup() - await submitOne(queue, 0n) + await submitSighted(queue, 0n) await queue.onBlock(5n) // attempt 1 - await queue.onBlock(10n) // attempt 2 - await queue.onBlock(15n) // attempt 3 + await queue.onBlock(10n) // sights the replacement + await queue.onBlock(15n) // attempt 2 + await queue.onBlock(20n) // sights the replacement + await queue.onBlock(25n) // attempt 3 expect(queue.size).toBe(1) - await queue.onBlock(20n) // attempt already 3 → drop + await queue.onBlock(30n) // sights the replacement + await queue.onBlock(35n) // attempt already 3 → drop expect(queue.size).toBe(0) }) it('drops a stuck tx when the bump would breach the fee ceiling', async () => { const { queue } = setup({ maxFeeWei: 1000n }) - await submitOne(queue, 0n) + await submitSighted(queue, 0n) await queue.onBlock(5n) expect(queue.size).toBe(0) }) @@ -227,8 +243,7 @@ describe('createPendingQueue', () => { label: 'market:borrower', correlation: { collateralIndex: 2, postMaturityMode: true }, maxFeePerGas: 1000n, - maxPriorityFeePerGas: 1000n, - blockNumber: 0n + maxPriorityFeePerGas: 1000n }) expect(events.find(e => e.event === 'tx.submit_failed')?.fields).toMatchObject({ id: 'market:borrower', @@ -257,7 +272,7 @@ describe('createPendingQueue', () => { throw new ExecutionRevertedError({}) // the re-broadcast reverts } const { queue } = setup({ send, logger }) - await submitOne(queue, 0n) + await submitSighted(queue, 0n) await queue.onBlock(5n) // stuck → replace → reverts → drop expect(queue.size).toBe(0) expect(events.find(e => e.event === 'tx.dropped')?.fields?.reason).toBe('reverts_on_replace') @@ -272,7 +287,7 @@ describe('createPendingQueue', () => { throw new Error('connection reset') // every replacement is a transient failure } const { queue } = setup({ send, logger }) - await submitOne(queue, 0n) + await submitSighted(queue, 0n) await queue.onBlock(5n) // attempt 1 await queue.onBlock(6n) // attempt 2 await queue.onBlock(7n) // attempt 3 @@ -299,15 +314,13 @@ describe('createPendingQueue', () => { request: REQUEST, label: 'a', maxFeePerGas: 1000n, - maxPriorityFeePerGas: 1000n, - blockNumber: 0n + maxPriorityFeePerGas: 1000n }) await queue.submit({ request: REQUEST, label: 'b', maxFeePerGas: 1000n, - maxPriorityFeePerGas: 1000n, - blockNumber: 0n + maxPriorityFeePerGas: 1000n }) expect(queue.size).toBe(2) await queue.onBlock(1n) // entry #1 getReceipt throws (caught), entry #2 confirms → evicted @@ -332,15 +345,13 @@ describe('createPendingQueue', () => { request: REQUEST, label: 'a', maxFeePerGas: 1000n, - maxPriorityFeePerGas: 1000n, - blockNumber: 0n + maxPriorityFeePerGas: 1000n }) await ctx.queue.submit({ request: REQUEST, label: 'b', maxFeePerGas: 1000n, - maxPriorityFeePerGas: 1000n, - blockNumber: 0n + maxPriorityFeePerGas: 1000n }) // First submit synced (queue was empty); the second must NOT — the cursor is legitimately ahead // by the in-flight tx, and re-reading chain would hand out a colliding nonce. @@ -395,7 +406,7 @@ describe('createPendingQueue', () => { logger, getReceipt: async () => ({ status: 'success', blockNumber: 10n }) }) - await submitOne(queue, 0n) + await submitSighted(queue, 0n) await queue.onBlock(1n) expect(events.find(e => e.event === 'tx.sent')?.fields?.id).toBe('market:borrower') expect(events.find(e => e.event === 'tx.confirmed')?.fields?.id).toBe('market:borrower') @@ -403,7 +414,7 @@ describe('createPendingQueue', () => { it('keeps a confirmed label in the backpressure set for the cooldown, then releases it', async () => { const { queue } = setup({ getReceipt: async () => ({ status: 'success', blockNumber: 10n }) }) - await submitOne(queue, 0n) + await submitOne(queue) await queue.onBlock(1n) // confirms → leaves `pending`, enters cooldown expect(queue.size).toBe(0) // Still suppressed: the read RPC may not yet reflect the cleared position, so re-submitting now @@ -417,7 +428,7 @@ describe('createPendingQueue', () => { it('also cools down a reverted label', async () => { const { queue } = setup({ getReceipt: async () => ({ status: 'reverted', blockNumber: 10n }) }) - await submitOne(queue, 0n) + await submitOne(queue) await queue.onBlock(1n) expect(queue.size).toBe(0) expect(queue.inflightLabels().has('market:borrower')).toBe(true) @@ -425,11 +436,14 @@ describe('createPendingQueue', () => { it('also cools down a dropped (max-bump) label', async () => { const { queue } = setup() - await submitOne(queue, 0n) + await submitSighted(queue, 0n) await queue.onBlock(5n) // attempt 1 - await queue.onBlock(10n) // attempt 2 - await queue.onBlock(15n) // attempt 3 - await queue.onBlock(20n) // attempt already 3 → drop → cooldown + await queue.onBlock(10n) + await queue.onBlock(15n) // attempt 2 + await queue.onBlock(20n) + await queue.onBlock(25n) // attempt 3 + await queue.onBlock(30n) + await queue.onBlock(35n) // attempt already 3 → drop → cooldown expect(queue.size).toBe(0) expect(queue.inflightLabels().has('market:borrower')).toBe(true) }) @@ -439,7 +453,7 @@ describe('createPendingQueue', () => { getReceipt: async () => ({ status: 'success', blockNumber: 10n }), withCooldown: false }) - await submitOne(queue, 0n) + await submitOne(queue) await queue.onBlock(1n) // confirms → with no cooldown the label leaves the set right away expect(queue.size).toBe(0) expect(queue.inflightLabels().size).toBe(0) @@ -450,7 +464,7 @@ describe('drop', () => { it('settles the tracked nonce as dropped and logs tx.dropped with the reason', async () => { const { logger, events } = captureLogger() const { queue } = setup({ logger }) - await submitOne(queue, 0n) + await submitSighted(queue, 0n) expect(queue.drop(7, 'nonce_consumed')).toBe(true) expect(queue.size).toBe(0) const dropped = events.find(e => e.event === 'tx.dropped') @@ -465,7 +479,7 @@ describe('drop', () => { it('releases the label from the inflight set', async () => { const { queue } = setup() - await submitOne(queue, 0n) + await submitSighted(queue, 0n) queue.drop(7, 'nonce_consumed') expect(queue.inflightLabels().has('market:borrower')).toBe(false) }) @@ -473,7 +487,7 @@ describe('drop', () => { it('returns false for a nonce that is not tracked (nothing to reconcile)', async () => { const { logger, events } = captureLogger() const { queue } = setup({ logger }) - await submitOne(queue, 0n) + await submitSighted(queue, 0n) expect(queue.drop(999, 'nonce_consumed')).toBe(false) expect(queue.size).toBe(1) // the real entry is untouched expect(events.some(e => e.event === 'tx.dropped')).toBe(false) // nothing dropped for a phantom nonce @@ -490,7 +504,7 @@ describe('nonce-consumed reconciliation', () => { reconcileEveryBlocks: 1, logger }) - await submitOne(queue, 0n) + await submitSighted(queue, 0n) await queue.onBlock(1n) expect(queue.size).toBe(0) expect(events.find(e => e.event === 'tx.dropped')?.fields).toMatchObject({ @@ -503,7 +517,7 @@ describe('nonce-consumed reconciliation', () => { it('keeps a tracked tx whose nonce is not yet consumed', async () => { const { queue } = setup({ getConsumedNonce: async () => 7, reconcileEveryBlocks: 1 }) - await submitOne(queue, 0n) // nonce 7; consumed count 7 means nonce 7 is not yet mined + await submitSighted(queue, 0n) // nonce 7; consumed count 7 means nonce 7 is not yet mined await queue.onBlock(1n) expect(queue.size).toBe(1) }) @@ -517,7 +531,7 @@ describe('nonce-consumed reconciliation', () => { }, reconcileEveryBlocks: 3 }) - await submitOne(queue, 0n) + await submitOne(queue) await queue.onBlock(1n) // block 1 — no reconcile await queue.onBlock(2n) // block 2 — no reconcile expect(calls).toBe(0) @@ -527,12 +541,171 @@ describe('nonce-consumed reconciliation', () => { it('does not reconcile when no getConsumedNonce hook is provided', async () => { const { queue } = setup({ reconcileEveryBlocks: 1 }) - await submitOne(queue, 0n) + await submitSighted(queue, 0n) await queue.onBlock(1n) // no hook → nothing to reconcile, tx stays expect(queue.size).toBe(1) }) }) +describe('stuck-age baseline', () => { + it('does not bump on the pass that first sights a broadcast, however stale the head', async () => { + const { queue, sends } = setup() + await submitOne(queue) + // A caller's block used to set this baseline, so a tick that spent blocks quoting made a fresh + // broadcast look stuck and bumped it 300ms after it was sent. + await queue.onBlock(1_000n) + expect(sends).toHaveLength(1) + expect(queue.snapshot()[0]?.attempt).toBe(0) + }) + + it('bumps only once stuckBlocks pass beyond the sighting', async () => { + const { queue, sends } = setup() + await submitOne(queue) + await queue.onBlock(1_000n) // sighting stamps the baseline here + await queue.onBlock(1_004n) // age 4 — not yet past stuckBlocks + expect(sends).toHaveLength(1) + await queue.onBlock(1_005n) // age 5 + expect(sends).toHaveLength(2) + }) + + it('re-sights a replacement rather than ageing it from the block that bumped it', async () => { + const { queue, sends } = setup() + await submitSighted(queue, 0n) + await queue.onBlock(5n) + expect(sends).toHaveLength(2) + await queue.onBlock(100n) // sights the replacement; a stale maintenance block must not re-bump + expect(sends).toHaveLength(2) + await queue.onBlock(105n) + expect(sends).toHaveLength(3) + }) +}) + +describe('multi-hash settlement', () => { + // The stub signer hands out hashOf(1) for the first send and hashOf(2) for its replacement. + const ORIGINAL = hashOf(1) + const REPLACEMENT = hashOf(2) + + /** + * A queue whose receipt source and consumed-nonce cursor can both be moved mid-test, so a receipt + * can land AFTER a bump has already replaced the hash that carries it. + */ + function setupSwappable(opts: { withReconciler?: boolean } = {}) { + let receipt: GetReceipt = async () => null + const consumedRef = { value: 7 } // 7 == our nonce, i.e. not yet consumed + const ctx = setup({ + getReceipt: async txHash => receipt(txHash), + ...(opts.withReconciler ? { getConsumedNonce: async () => consumedRef.value } : {}), + reconcileEveryBlocks: 1 + }) + return { + ...ctx, + consumedRef, + setReceipt(next: GetReceipt) { + receipt = next + } + } + } + + const minedAt = + (hash: Hex, status: 'success' | 'reverted'): GetReceipt => + async txHash => + txHash === hash ? { status, blockNumber: 42n } : null + + it('confirms on the original hash when the bump that replaced it never landed', async () => { + const ctx = setupSwappable() + await submitSighted(ctx.queue, 0n) + await ctx.queue.onBlock(5n) // bump: REPLACEMENT is now latest, ORIGINAL still broadcast + ctx.setReceipt(minedAt(ORIGINAL, 'success')) + await ctx.queue.onBlock(6n) + + expect(ctx.queue.size).toBe(0) + const confirmed = ctx.events.find(e => e.event === 'tx.confirmed') + expect(confirmed?.fields?.txHash).toBe(ORIGINAL) + expect(confirmed?.fields?.blockNumber).toBe(42n) + expect(ctx.events.some(e => e.event === 'tx.dropped')).toBe(false) + }) + + it('reverts on the original hash with that hash, not the replacement', async () => { + const ctx = setupSwappable() + await submitSighted(ctx.queue, 0n) + await ctx.queue.onBlock(5n) + ctx.setReceipt(minedAt(ORIGINAL, 'reverted')) + await ctx.queue.onBlock(6n) + + expect(ctx.events.find(e => e.event === 'tx.reverted')?.fields?.txHash).toBe(ORIGINAL) + }) + + it('still settles on the replacement when that is what mined', async () => { + const ctx = setupSwappable() + await submitSighted(ctx.queue, 0n) + await ctx.queue.onBlock(5n) + ctx.setReceipt(minedAt(REPLACEMENT, 'success')) + await ctx.queue.onBlock(6n) + + expect(ctx.events.find(e => e.event === 'tx.confirmed')?.fields?.txHash).toBe(REPLACEMENT) + }) + + it('reads past a failure on the newest hash to find a mined original', async () => { + const ctx = setupSwappable() + await submitSighted(ctx.queue, 0n) + await ctx.queue.onBlock(5n) + ctx.setReceipt(async txHash => { + if (txHash === REPLACEMENT) throw new Error('rpc down') + return { status: 'success', blockNumber: 42n } + }) + await ctx.queue.onBlock(6n) + + expect(ctx.events.find(e => e.event === 'tx.confirmed')?.fields?.txHash).toBe(ORIGINAL) + }) + + it('settles a consumed nonce whose earlier hash mined instead of dropping it', async () => { + // The reconciler used to look only at the latest hash, so a mined original read as an external + // send taking our nonce — the misreport this whole record exists to prevent. + const ctx = setupSwappable({ withReconciler: true }) + await submitSighted(ctx.queue, 0n) + await ctx.queue.onBlock(5n) // bump; nonce not yet consumed, so nothing is retired + ctx.setReceipt(minedAt(ORIGINAL, 'success')) + ctx.consumedRef.value = 8 // the chain moved past our nonce — because OUR original mined + await ctx.queue.onBlock(6n) + + expect(ctx.queue.size).toBe(0) + expect(ctx.events.find(e => e.event === 'tx.confirmed')?.fields?.txHash).toBe(ORIGINAL) + expect( + ctx.events.some(e => e.event === 'tx.dropped' && e.fields?.reason === 'nonce_consumed') + ).toBe(false) + }) + + it('still reports nonce_consumed when no tracked hash has a receipt', async () => { + const ctx = setupSwappable({ withReconciler: true }) + await submitSighted(ctx.queue, 0n) + await ctx.queue.onBlock(5n) + ctx.consumedRef.value = 8 // consumed by someone else: no hash of ours ever gets a receipt + await ctx.queue.onBlock(6n) + + expect(ctx.queue.size).toBe(0) + expect( + ctx.events.some(e => e.event === 'tx.dropped' && e.fields?.reason === 'nonce_consumed') + ).toBe(true) + }) + + it('keeps an entry whose every hash failed to read, and never bumps or drops it', async () => { + const ctx = setupSwappable({ withReconciler: true }) + await submitSighted(ctx.queue, 0n) + await ctx.queue.onBlock(5n) + const sendsAfterBump = ctx.sends.length + ctx.setReceipt(async () => { + throw new Error('rpc down') + }) + ctx.consumedRef.value = 8 // even with the nonce consumed, an unreadable receipt proves nothing + await ctx.queue.onBlock(20n) + + expect(ctx.queue.size).toBe(1) // an unreadable receipt is not proof the tx is gone + expect(ctx.sends.length).toBe(sendsAfterBump) // nor grounds to replace it + expect(ctx.events.some(e => e.event === 'tx.onblock_error')).toBe(true) + expect(ctx.events.some(e => e.event === 'tx.dropped')).toBe(false) + }) +}) + describe('send-aborted latch', () => { it('latches sends after a hashless TxSendError until the next onBlock clears it', async () => { let calls = 0 @@ -543,15 +716,15 @@ describe('send-aborted latch', () => { } const { queue } = setup({ send }) // First submit claims a nonce but fails hashless → rethrows and latches. - await expect(submitOne(queue, 0n)).rejects.toThrow(/rpc timeout after broadcast/) + await expect(submitSighted(queue, 0n)).rejects.toThrow(/rpc timeout after broadcast/) expect(calls).toBe(1) // While latched, further submits are skipped (no new send attempts). - await submitOne(queue, 0n) + await submitSighted(queue, 0n) expect(calls).toBe(1) expect(queue.size).toBe(0) // The settlement pass clears the latch. await queue.onBlock(1n) - await submitOne(queue, 0n) + await submitSighted(queue, 0n) expect(calls).toBe(2) expect(queue.size).toBe(1) }) @@ -562,8 +735,8 @@ describe('send-aborted latch', () => { throw new TxSendError(new Error('broadcast lost'), 7) } const { queue } = setup({ send, logger }) - await expect(submitOne(queue, 0n)).rejects.toThrow() - await submitOne(queue, 0n) // latched → skipped + await expect(submitSighted(queue, 0n)).rejects.toThrow() + await submitSighted(queue, 0n) // latched → skipped expect(events.find(e => e.event === 'tx.send_aborted')?.level).toBe('warn') }) }) @@ -597,13 +770,12 @@ describe('nonce-hole latch', () => { maxFeeWei: opts.maxFeeWei ?? 10_000_000_000_000n, logger }) - const submit = (label: string, blockNumber: bigint) => + const submit = (label: string) => queue.submit({ request: REQUEST, label, maxFeePerGas: 1000n, - maxPriorityFeePerGas: 1000n, - blockNumber + maxPriorityFeePerGas: 1000n }) return { queue, @@ -619,11 +791,12 @@ describe('nonce-hole latch', () => { it('refuses new first-sends after dropping an unconsumed nonce, then resumes once the chain consumes past it', async () => { const ctx = setupSeq({ maxFeeWei: 1000n }) // low ceiling so a bump breaches → fee_ceiling drop - await ctx.submit('a', 0n) // nonce 7 at block 0 - await ctx.submit('b', 3n) // nonce 8 at block 3 (queue not empty → no extra sync) + await ctx.submit('a') // nonce 7 + await ctx.queue.onBlock(0n) // sights nonce 7 at block 0 + await ctx.submit('b') // nonce 8 (queue not empty → no extra sync) expect(ctx.queue.size).toBe(2) // Block 5: nonce 7 (age 5 > 4) is stuck; its bump breaches the ceiling → dropped fee_ceiling and - // its (still-unconsumed) nonce is latched as a hole. nonce 8 (age 2) is not stuck → stays. + // its (still-unconsumed) nonce is latched as a hole. nonce 8 is only sighted here → stays. await ctx.queue.onBlock(5n) expect(ctx.queue.size).toBe(1) expect( @@ -633,7 +806,7 @@ describe('nonce-hole latch', () => { // A NEW first-send is refused while the hole is latched (nonce 8 still pending → queue not empty, // so the empty-queue sync can't clear it). // Queue-wide, not this label's fault: `refused` is what tells a caller not to back the position off. - expect(await ctx.submit('c', 6n)).toEqual({ sent: false, reason: 'refused' }) + expect(await ctx.submit('c')).toEqual({ sent: false, reason: 'refused' }) expect(ctx.sends.length).toBe(sendsAfterDrop) // no new broadcast expect(ctx.queue.size).toBe(1) expect(ctx.events.some(e => e.event === 'queue.nonce_hole' && e.fields?.id === 'c')).toBe(true) @@ -642,18 +815,19 @@ describe('nonce-hole latch', () => { await ctx.queue.onBlock(7n) expect(ctx.events.some(e => e.event === 'queue.nonce_hole_cleared')).toBe(true) // Sends flow again. - await ctx.submit('d', 8n) + await ctx.submit('d') expect(ctx.sends.length).toBe(sendsAfterDrop + 1) }) it('clears the latch when the queue empties and syncNonce re-derives the cursor', async () => { const ctx = setupSeq({ maxFeeWei: 1000n }) - await ctx.submit('a', 0n) // nonce 7, sole entry (sync #1 on the empty queue) + await ctx.submit('a') // nonce 7, sole entry (sync #1 on the empty queue) + await ctx.queue.onBlock(0n) // sights it at block 0 await ctx.queue.onBlock(5n) // stuck → fee_ceiling drop → latched; queue now empty expect(ctx.queue.size).toBe(0) const syncsBefore = ctx.syncNonceCalls // Queue empty → the next first-send syncs the cursor from chain and clears the hole in one step. - await ctx.submit('b', 6n) + await ctx.submit('b') expect(ctx.syncNonceCalls).toBe(syncsBefore + 1) expect( ctx.events.some(e => e.event === 'queue.nonce_hole_cleared' && e.fields?.via === 'sync') @@ -663,8 +837,8 @@ describe('nonce-hole latch', () => { it('does not latch a hole when the reconciler drops a consumed nonce', async () => { const ctx = setupSeq() - await ctx.submit('a', 0n) // nonce 7 - await ctx.submit('b', 0n) // nonce 8 (queue not empty → no extra sync) + await ctx.submit('a') // nonce 7 + await ctx.submit('b') // nonce 8 (queue not empty → no extra sync) expect(ctx.queue.size).toBe(2) // Chain shows nonce 7 consumed by an external send (count 8); the reconciler drops our tracked // nonce 7 as nonce_consumed — a BY-DEFINITION-consumed retirement that must NOT latch a hole. @@ -676,15 +850,17 @@ describe('nonce-hole latch', () => { ).toBe(true) const sendsBefore = ctx.sends.length // No hole latched → a new first-send proceeds (it would be refused had the reconciler latched). - await ctx.submit('c', 2n) + await ctx.submit('c') expect(ctx.sends.length).toBe(sendsBefore + 1) expect(ctx.events.some(e => e.event === 'queue.nonce_hole')).toBe(false) }) it('widens the hole span across multiple drops and clears only past the highest', async () => { const ctx = setupSeq({ maxFeeWei: 1000n }) - await ctx.submit('a', 0n) // nonce 7 at block 0 - await ctx.submit('b', 2n) // nonce 8 at block 2 + await ctx.submit('a') // nonce 7 + await ctx.queue.onBlock(0n) // sights nonce 7 at block 0 + await ctx.submit('b') // nonce 8 + await ctx.queue.onBlock(2n) // sights nonce 8 at block 2 await ctx.queue.onBlock(5n) // a (age 5) stuck → drop → hole {7} await ctx.queue.onBlock(7n) // b (age 5) stuck → drop → hole widens to {7, 8} expect(ctx.queue.size).toBe(0) @@ -808,15 +984,13 @@ describe('submit serialization', () => { request: REQUEST, label: 'vault:a', maxFeePerGas: 1000n, - maxPriorityFeePerGas: 1000n, - blockNumber: 1n + maxPriorityFeePerGas: 1000n }) const second = queue.submit({ request: REQUEST, label: 'vault:b', maxFeePerGas: 1000n, - maxPriorityFeePerGas: 1000n, - blockNumber: 1n + maxPriorityFeePerGas: 1000n }) // Only the leader may be inside `send`; the follower is still queued behind the lock. A macrotask @@ -854,15 +1028,13 @@ describe('submit serialization', () => { request: REQUEST, label: 'vault:a', maxFeePerGas: 1000n, - maxPriorityFeePerGas: 1000n, - blockNumber: 1n + maxPriorityFeePerGas: 1000n }), queue.submit({ request: REQUEST, label: 'vault:b', maxFeePerGas: 1000n, - maxPriorityFeePerGas: 1000n, - blockNumber: 1n + maxPriorityFeePerGas: 1000n }) ]) @@ -890,15 +1062,13 @@ describe('submit serialization', () => { request: REQUEST, label: 'vault:a', maxFeePerGas: 1000n, - maxPriorityFeePerGas: 1000n, - blockNumber: 1n + maxPriorityFeePerGas: 1000n }) const second = queue.submit({ request: REQUEST, label: 'vault:b', maxFeePerGas: 1000n, - maxPriorityFeePerGas: 1000n, - blockNumber: 1n + maxPriorityFeePerGas: 1000n }) await expect(first).rejects.toBeInstanceOf(TxSendError) From 0185434d273419ebe5003e3282c81a9fb89763ac Mon Sep 17 00:00:00 2001 From: Hayden Shively Date: Thu, 3 Sep 2026 02:59:45 -0500 Subject: [PATCH 2/6] address the review of the pending-queue fix - vault-v1/v2 reallocation were left half-migrated: their tick `submit` dep type still declared `blockNumber` and still threaded `deps.chainHead` into it. TypeScript accepted the narrower handler in `index.ts`, so this compiled while shipping the exact tick-entry-stale head the change exists to delete. Both seams and their pinned test assertions updated; the `tick.end` log field keeps its own `chainHead`. - Add `test/queue/receipt.utils.test.ts`, mirroring the sibling modules. It covers the branch `pending-queue.test.ts` could not reach: a read failure on an OLDER hash while the newest reads clean must still yield `unknown`, since the unreadable hash is the one that may have mined. Verified by reverting the failure accumulation, not the assertion. - Amend TIB-2026-05-28, which specified both the single-`txHash` record and `currentBlock - submittedAtBlock` stuck detection. - `txHashes` is bounded by `maxBumpAttempts + 1`, not `maxBumpAttempts`: `attempt` also increments on a transient `tx.replace_failed`. - Give each rule one home and drop the incident narration from comments. Co-Authored-By: Claude Opus 5 (1M context) --- .../test/fork/queue.test.ts | 5 +- bots/vault-v1-reallocation/src/runner/tick.ts | 4 +- .../test/runner/tick.test.ts | 2 +- bots/vault-v2-reallocation/src/runner/tick.ts | 4 +- .../test/runner/tick.test.ts | 2 +- ...TIB-2026-05-28-midnight-liquidation-bot.md | 23 ++++++ packages/bot-kit/src/queue/pending-queue.ts | 30 ++++--- .../bot-kit/test/queue/pending-queue.test.ts | 7 +- .../bot-kit/test/queue/receipt.utils.test.ts | 78 +++++++++++++++++++ 9 files changed, 126 insertions(+), 29 deletions(-) create mode 100644 packages/bot-kit/test/queue/receipt.utils.test.ts diff --git a/bots/midnight-liquidation/test/fork/queue.test.ts b/bots/midnight-liquidation/test/fork/queue.test.ts index a3130965..34f0f96d 100644 --- a/bots/midnight-liquidation/test/fork/queue.test.ts +++ b/bots/midnight-liquidation/test/fork/queue.test.ts @@ -68,8 +68,9 @@ describe('fork: pending-queue bump + replacement against a real node', () => { if (!original) throw new Error('expected a pending entry') expect(original.attempt).toBe(0) - // 2. Advance past STUCK_BLOCKS (4) without mining → real getReceipt returns null, so onBlock detects - // the stuck tx and replaces it at the same nonce with ≥12.5% higher fees (a fresh hash). + // 2. Advance without mining → real getReceipt returns null, so onBlock detects the stuck tx and + // replaces it at the same nonce with ≥12.5% higher fees (a fresh hash). Block 1 only sights + // the entry, so the bump lands at sighting + STUCK_BLOCKS (4) + 1 = block 6. for (let block = 1n; block <= 6n; block++) await queue.onBlock(block) const bumped = queue.snapshot()[0] if (!bumped) throw new Error('expected the bumped entry to remain pending') diff --git a/bots/vault-v1-reallocation/src/runner/tick.ts b/bots/vault-v1-reallocation/src/runner/tick.ts index 61fcde6f..b08db14a 100644 --- a/bots/vault-v1-reallocation/src/runner/tick.ts +++ b/bots/vault-v1-reallocation/src/runner/tick.ts @@ -17,7 +17,7 @@ export type TickDeps = { encodeReallocate: (allocations: MarketAllocation[]) => Hex simulate: (vault: Address, data: Hex) => Promise /** Resolves true only when the transaction was actually broadcast. */ - submit: (params: { vault: Address; data: Hex; blockNumber: bigint }) => Promise + submit: (params: { vault: Address; data: Hex }) => Promise /** When true, a sim-ok plan is logged (`reallocation.dry_run`) instead of submitted. */ dryRun: boolean /** Labels (vault addresses) with an in-flight or cooling-down tx — skipped this tick. */ @@ -95,7 +95,7 @@ const processVault = async (deps: TickDeps, vault: Address): Promise { it('submits a sim-ok reallocation and counts it', async () => { const { deps, events } = makeDeps({ strategy: vi.fn(() => someAllocations()) }) await runTick(deps) - expect(deps.submit).toHaveBeenCalledWith({ vault: VAULT_A, data: DATA, blockNumber: 100n }) + expect(deps.submit).toHaveBeenCalledWith({ vault: VAULT_A, data: DATA }) expect(tickEnd(events)).toMatchObject({ reallocations_found: 1, submitted: 1, errors: 0 }) expect(events.some(e => e.event === 'reallocation.found')).toBe(true) }) diff --git a/bots/vault-v2-reallocation/src/runner/tick.ts b/bots/vault-v2-reallocation/src/runner/tick.ts index 05462a4c..ba646aae 100644 --- a/bots/vault-v2-reallocation/src/runner/tick.ts +++ b/bots/vault-v2-reallocation/src/runner/tick.ts @@ -21,7 +21,7 @@ export type TickDeps = { encodeReallocation: (vaultData: VaultV2Data, reallocation: Reallocation) => Hex simulate: (vault: Address, data: Hex) => Promise /** Resolves true only when the transaction was actually broadcast. */ - submit: (params: { vault: Address; data: Hex; blockNumber: bigint }) => Promise + submit: (params: { vault: Address; data: Hex }) => Promise /** When true, a sim-ok plan is logged (`reallocation.dry_run`) instead of submitted. */ dryRun: boolean /** Labels (vault addresses) with an in-flight or cooling-down tx — skipped this tick. */ @@ -116,7 +116,7 @@ const processVault = async (deps: TickDeps, vault: Address): Promise { it('submits a sim-ok reallocation and counts it', async () => { const { deps, events } = makeDeps({ strategy: vi.fn(() => someReallocation()) }) await runTick(deps) - expect(deps.submit).toHaveBeenCalledWith({ vault: VAULT_A, data: DATA, blockNumber: 100n }) + expect(deps.submit).toHaveBeenCalledWith({ vault: VAULT_A, data: DATA }) expect(tickEnd(events)).toMatchObject({ reallocations_found: 1, submitted: 1, errors: 0 }) expect(events.some(e => e.event === 'reallocation.found')).toBe(true) }) diff --git a/docs/decisions/TIB-2026-05-28-midnight-liquidation-bot.md b/docs/decisions/TIB-2026-05-28-midnight-liquidation-bot.md index 8afad3ce..4f62a780 100644 --- a/docs/decisions/TIB-2026-05-28-midnight-liquidation-bot.md +++ b/docs/decisions/TIB-2026-05-28-midnight-liquidation-bot.md @@ -942,3 +942,26 @@ Alternative 5 ("Persistent queue state across runner restarts", rejected here) i required by the one-shot model**: a per-run process with no memory of its pending txs could never fee-bump a stuck one. The spirit survives — persisted state is a hint reconciled against chain truth, and losing the file degrades to this TIB's restart semantics. + +### 2026-09-03 — stuck detection ages from a sighting, and a nonce keeps every hash it broadcast + +Two of this TIB's queue specifications are superseded (BOTS-50). + +**Stuck detection** was specified as `currentBlock - submittedAtBlock > STUCK_BLOCKS (4)`, with +`submittedAtBlock` supplied by the caller. Every caller supplied the head captured at tick entry — +before discovery, quoting and simulation — so a slow tick produced an entry that was already older +than `STUCK_BLOCKS` the moment it was tracked, and the next block replaced it. In production this +fee-bumped transactions 176–422ms after broadcast on a 2s-block chain. `submittedAtBlock` is now +stamped by the first `onBlock` that sights an entry and `blockNumber` is gone from `SubmitArgs`: a +baseline the queue observed cannot be stale, whereas one it is handed always can. The cost is that a +bump waits one block longer than `stuckBlocks` alone suggests. + +**`Pending` carries `txHashes`, not `txHash`.** This TIB gave each entry one hash and had replacement +overwrite it. A fee bump cannot un-broadcast what it replaced, so when the original mined and the +replacement did not, the queue held no hash with a receipt and reported a settled liquidation as +`tx.dropped / nonce_consumed`. An entry now retains every hash broadcast for its nonce; the receipt +sweep and the reconciler settle on the first that mined and log it by that hash. `nonce_consumed` +requires a clean read of every hash finding none — a read failure proves nothing and never retires an +entry. + +The **drift / gap awareness** and **fee bump** specifications above are unchanged. diff --git a/packages/bot-kit/src/queue/pending-queue.ts b/packages/bot-kit/src/queue/pending-queue.ts index 1c7230bd..40f74e4b 100644 --- a/packages/bot-kit/src/queue/pending-queue.ts +++ b/packages/bot-kit/src/queue/pending-queue.ts @@ -102,19 +102,18 @@ type Pending = { nonce: number /** * Every hash broadcast for this nonce, newest first — `[0]` is the live replacement target and the - * one the send-side log events name. A fee bump cannot un-broadcast what it replaced, so any hash - * here may be the one that mines; retiring the nonce requires {@link scanReceipts} to clear them - * all. Non-empty by construction, and bounded by `maxBumpAttempts`. + * one the send-side log events name. A fee bump cannot un-broadcast what it replaced, so ANY hash + * here may be the one that mines. Non-empty by construction; bounded by `maxBumpAttempts + 1`. */ txHashes: [Hex, ...Hex[]] request: TxRequest label: string /** - * Head at which the queue FIRST OBSERVED this broadcast, which is what `stuckBlocks` ages against. - * `null` until an `onBlock` sights it: a caller's block is captured before quoting and simulation - * and can already be several blocks stale by the time the send resolves, which would make a fresh - * transaction look stuck and bump it before it could possibly mine. Sighting can only run late, - * never early, so the error is always in the safe direction. + * Head at which the queue FIRST OBSERVED this broadcast — what `stuckBlocks` ages against, and + * `null` until an `onBlock` sights it. Deliberately not a caller's block: that is captured before + * quoting and simulation and can be stale by the time the send resolves, which ages a fresh + * transaction into an immediate replacement. A sighting can only run late, never early, so the + * error is always toward waiting too long rather than bumping too soon. */ submittedAtBlock: bigint | null maxFeePerGas: bigint @@ -279,8 +278,8 @@ export function createPendingQueue({ } } - // Retires an entry the chain settled, naming the hash that ACTUALLY mined — which after a fee bump - // need not be the latest one broadcast, and is the whole point of tracking every hash per nonce. + // Retires an entry the chain settled, naming the hash that mined — which after a fee bump need not + // be the latest broadcast. See {@link Pending.txHashes}. function settleMined( entry: Pending, mined: { txHash: Hex; receipt: TxReceiptLite }, @@ -465,8 +464,8 @@ export function createPendingQueue({ entry.txHashes.unshift(replaced.data.txHash) entry.maxFeePerGas = result.fees.maxFeePerGas entry.maxPriorityFeePerGas = result.fees.maxPriorityFeePerGas - // Re-sighted like a first send: this pass already spent a receipt read per entry and a gas - // estimation on the replacement, so the block it started from can itself be stale. + // Re-sighted like a first send; see {@link Pending.submittedAtBlock}. This pass already spent a + // receipt read per entry and a gas estimation, so its own block can be stale too. entry.submittedAtBlock = null entry.attempt += 1 logger.info('tx.bumped', { @@ -496,7 +495,6 @@ export function createPendingQueue({ // A consumed nonce whose receipt is ours is a settlement, not a loss — the sweep can miss it // when the receipt lands between the two passes, or when its read failed and this one didn't. if (scan.kind === 'mined') settleMined(entry, scan, blockNumber) - // `unknown` leaves "none of our hashes mined" unproven, so it must never retire the nonce. else if (scan.kind === 'none') drop(entry.nonce, 'nonce_consumed') } } @@ -544,9 +542,8 @@ export function createPendingQueue({ try { const scan = await scanReceipts(getReceipt, entry.txHashes) if (scan.kind === 'unknown') { - // Every hash read failed, so this entry's fate is unknown this pass. Skipping the stuck - // check is deliberate: bumping on an unreadable receipt would replace a tx that may have - // already mined. + // Skipping the stuck check is deliberate: replacing a tx whose receipt we could not read + // may replace one that already mined. logger.warn('tx.onblock_error', { id: entry.label, nonce: entry.nonce, @@ -568,7 +565,6 @@ export function createPendingQueue({ continue } if (entry.submittedAtBlock === null) { - // First sighting: stamp the baseline and give the tx a full stuck window from here. entry.submittedAtBlock = blockNumber continue } diff --git a/packages/bot-kit/test/queue/pending-queue.test.ts b/packages/bot-kit/test/queue/pending-queue.test.ts index edc759f7..30625b85 100644 --- a/packages/bot-kit/test/queue/pending-queue.test.ts +++ b/packages/bot-kit/test/queue/pending-queue.test.ts @@ -551,8 +551,7 @@ describe('stuck-age baseline', () => { it('does not bump on the pass that first sights a broadcast, however stale the head', async () => { const { queue, sends } = setup() await submitOne(queue) - // A caller's block used to set this baseline, so a tick that spent blocks quoting made a fresh - // broadcast look stuck and bumped it 300ms after it was sent. + // The sighting pass may not bump, however stale the head it runs on. await queue.onBlock(1_000n) expect(sends).toHaveLength(1) expect(queue.snapshot()[0]?.attempt).toBe(0) @@ -659,8 +658,8 @@ describe('multi-hash settlement', () => { }) it('settles a consumed nonce whose earlier hash mined instead of dropping it', async () => { - // The reconciler used to look only at the latest hash, so a mined original read as an external - // send taking our nonce — the misreport this whole record exists to prevent. + // A consumed nonce plus a receipt on one of our hashes is our own settlement, not an external + // send claiming the nonce. const ctx = setupSwappable({ withReconciler: true }) await submitSighted(ctx.queue, 0n) await ctx.queue.onBlock(5n) // bump; nonce not yet consumed, so nothing is retired diff --git a/packages/bot-kit/test/queue/receipt.utils.test.ts b/packages/bot-kit/test/queue/receipt.utils.test.ts new file mode 100644 index 00000000..7b2230d7 --- /dev/null +++ b/packages/bot-kit/test/queue/receipt.utils.test.ts @@ -0,0 +1,78 @@ +import type { Hex } from 'viem' + +import { describe, expect, it } from 'vitest' + +import type { GetReceipt, TxReceiptLite } from '../../src/queue/pending-queue' + +import { scanReceipts } from '../../src/queue/receipt.utils' + +const NEWEST: Hex = `0x${'a'.repeat(64)}` +const MIDDLE: Hex = `0x${'b'.repeat(64)}` +const OLDEST: Hex = `0x${'c'.repeat(64)}` +const ALL = [NEWEST, MIDDLE, OLDEST] + +const MINED: TxReceiptLite = { status: 'success', blockNumber: 42n } + +/** Reads that resolve per hash: a `TxReceiptLite` mines, an `Error` fails, `null` is still pending. */ +const reads = + (by: Record): GetReceipt => + async txHash => { + const answer = by[txHash] ?? null + if (answer instanceof Error) throw answer + return answer + } + +describe('scanReceipts', () => { + it('reports the newest hash when it is the one that mined', async () => { + const scan = await scanReceipts(reads({ [NEWEST]: MINED }), ALL) + expect(scan).toEqual({ kind: 'mined', txHash: NEWEST, receipt: MINED }) + }) + + it('reports an older hash when a bump replaced the one that actually mined', async () => { + const scan = await scanReceipts(reads({ [OLDEST]: MINED }), ALL) + expect(scan).toEqual({ kind: 'mined', txHash: OLDEST, receipt: MINED }) + }) + + it('prefers the newest of several receipts', async () => { + const older: TxReceiptLite = { status: 'reverted', blockNumber: 7n } + const scan = await scanReceipts(reads({ [NEWEST]: MINED, [OLDEST]: older }), ALL) + expect(scan).toEqual({ kind: 'mined', txHash: NEWEST, receipt: MINED }) + }) + + it('reports none only when every hash read cleanly and none had a receipt', async () => { + expect(await scanReceipts(reads({}), ALL)).toEqual({ kind: 'none' }) + }) + + it('reads past a failure on the newest hash to reach a mined older one', async () => { + const scan = await scanReceipts( + reads({ [NEWEST]: new Error('rpc down'), [OLDEST]: MINED }), + ALL + ) + expect(scan).toEqual({ kind: 'mined', txHash: OLDEST, receipt: MINED }) + }) + + it('reports unknown when an OLDER hash failed, even though the newest read cleanly', async () => { + // The mirror of the misreport this module exists to prevent: the unreadable hash is the one that + // may have mined, so a clean `null` on the newest is not proof the nonce is ours to retire. + const scan = await scanReceipts(reads({ [OLDEST]: new Error('rpc down') }), ALL) + expect(scan).toEqual({ kind: 'unknown', error: expect.any(Error) }) + }) + + it('surfaces the newest failure when several hashes failed', async () => { + const newest = new Error('newest') + const scan = await scanReceipts(reads({ [NEWEST]: newest, [OLDEST]: new Error('oldest') }), ALL) + expect(scan).toEqual({ kind: 'unknown', error: newest }) + }) + + it('lets a mined hash win over a failure on a hash that never mined', async () => { + const scan = await scanReceipts( + reads({ [NEWEST]: MINED, [OLDEST]: new Error('rpc down') }), + ALL + ) + expect(scan).toEqual({ kind: 'mined', txHash: NEWEST, receipt: MINED }) + }) + + it('reports none for an empty hash list', async () => { + expect(await scanReceipts(reads({}), [])).toEqual({ kind: 'none' }) + }) +}) From 1694a2b64b73f7676d6bd409897283427f3de51f Mon Sep 17 00:00:00 2001 From: Hayden Shively Date: Thu, 3 Sep 2026 03:16:16 -0500 Subject: [PATCH 3/6] address the automated review of the pending-queue fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `reverts_on_replace` retired an entry on the revert alone, discarding every retained hash unread. The original is a likely reason the replacement reverted: it can mine during that send's gas estimation, after the sweep read it as pending. The path now re-scans first — settling a mined hash, dropping only on a clean negative read, and keeping the entry when the scan itself failed. Both branches covered by tests verified against the pre-fix behaviour. - Scan the retained hashes concurrently. They are independent, the common case reads all of them anyway, and this runs in the maintenance pass the tick waits on, so serial reads multiplied RPC latency by hash count. - Restore JSDoc on `ResolverExecutionService.submit`, whose signature this branch changed. - Four comments in both liquidators still described the polled tick height as the queue's `submittedAtBlock` — the exact behaviour this branch removes. - `settleMined` becomes an arrow constant. Its `function` siblings in the same closure predate this branch and are left alone. Co-Authored-By: Claude Opus 5 (1M context) --- bots/blue-liquidation/src/index.ts | 4 +- bots/blue-liquidation/src/runner/tick.ts | 2 +- .../resolver/resolver.service.ts | 7 +++ bots/midnight-liquidation/src/index.ts | 8 +-- bots/midnight-liquidation/src/runner/tick.ts | 2 +- packages/bot-kit/src/queue/pending-queue.ts | 40 ++++++++------ packages/bot-kit/src/queue/receipt.utils.ts | 14 +++-- .../bot-kit/test/queue/pending-queue.test.ts | 53 +++++++++++++++++++ 8 files changed, 102 insertions(+), 28 deletions(-) diff --git a/bots/blue-liquidation/src/index.ts b/bots/blue-liquidation/src/index.ts index c46c2055..db1e5fee 100644 --- a/bots/blue-liquidation/src/index.ts +++ b/bots/blue-liquidation/src/index.ts @@ -260,8 +260,8 @@ async function main() { }) void heartbeatMonitor.start() - // An HTTP block-poll watcher drives one tick per new block (coalescing backlog), passing the polled - // height as the queue's submittedAtBlock. Each liquidatable position resolves its swap, simulates + // An HTTP block-poll watcher drives one tick per new block (coalescing backlog). Each + // liquidatable position resolves its swap, simulates // the real `exec_606BaXt`, and — on a sim-ok result — broadcasts that same exec via the Executor // singleton. Pending-queue upkeep runs in `maintain`. const tick = (chainHead: bigint) => diff --git a/bots/blue-liquidation/src/runner/tick.ts b/bots/blue-liquidation/src/runner/tick.ts index e1101622..35a7e07d 100644 --- a/bots/blue-liquidation/src/runner/tick.ts +++ b/bots/blue-liquidation/src/runner/tick.ts @@ -92,7 +92,7 @@ const LEVEL_BY_REASON: Record = { */ export async function runTick(deps: { discover: () => Promise - /** Chain head the runner just polled — the queue's `submittedAtBlock`. */ + /** Chain head the runner just polled — the tick-constant height backoff windows are measured in. */ chainHead: bigint readLens: (pairs: LensInput[]) => Promise> /** diff --git a/bots/midnight-crossed-books/src/infrastructure/resolver/resolver.service.ts b/bots/midnight-crossed-books/src/infrastructure/resolver/resolver.service.ts index aba87940..0e6eef11 100644 --- a/bots/midnight-crossed-books/src/infrastructure/resolver/resolver.service.ts +++ b/bots/midnight-crossed-books/src/infrastructure/resolver/resolver.service.ts @@ -29,6 +29,13 @@ export class ResolverExecutionService implements ResolverService { } } + /** + * Hands the simulated resolution to the pending queue for broadcast. + * @param prepared - Resolver target calldata and market label, exactly as simulation produced it. + * @returns A promise resolving once the queue has accepted (or declined) the request. + * @throws `ReadonlyMutationError` when the transport was composed without submission dependencies, + * and propagates any signer or queue failure the transport raises. + */ submit(prepared: PreparedResolution) { return this.transport.submit(prepared) } diff --git a/bots/midnight-liquidation/src/index.ts b/bots/midnight-liquidation/src/index.ts index ea3f6fc4..cd0ba983 100644 --- a/bots/midnight-liquidation/src/index.ts +++ b/bots/midnight-liquidation/src/index.ts @@ -345,10 +345,10 @@ async function main() { }) void heartbeatMonitor.start() - // Phase-4 runner: an HTTP block-poll watcher drives one tick per new block (coalescing backlog), - // passing the polled height as the queue's submittedAtBlock. Each liquidatable position resolves its - // swap step, simulates the real `exec_606BaXt`, and — on a sim-ok result — broadcasts that same exec - // via the Executor singleton. Pending-queue upkeep runs in `maintain`. + // Phase-4 runner: an HTTP block-poll watcher drives one tick per new block (coalescing backlog). + // Each liquidatable position resolves its swap step, simulates the real `exec_606BaXt`, and — on a + // sim-ok result — broadcasts that same exec via the Executor singleton. Pending-queue upkeep runs + // in `maintain`. const tick = (chainHead: bigint) => runTick({ discover, diff --git a/bots/midnight-liquidation/src/runner/tick.ts b/bots/midnight-liquidation/src/runner/tick.ts index 3a965f6e..84a362e3 100644 --- a/bots/midnight-liquidation/src/runner/tick.ts +++ b/bots/midnight-liquidation/src/runner/tick.ts @@ -575,7 +575,7 @@ const capPerPosition = ( */ export async function runTick(deps: { discover: () => Promise - /** Chain head the runner just polled — the queue's `submittedAtBlock`. */ + /** Chain head the runner just polled — the tick-constant height backoff windows are measured in. */ chainHead: bigint /** The Executor singleton — the `liquidate` msg.sender whose gate the lens checks. */ caller: Address diff --git a/packages/bot-kit/src/queue/pending-queue.ts b/packages/bot-kit/src/queue/pending-queue.ts index 40f74e4b..e9113107 100644 --- a/packages/bot-kit/src/queue/pending-queue.ts +++ b/packages/bot-kit/src/queue/pending-queue.ts @@ -280,11 +280,11 @@ export function createPendingQueue({ // Retires an entry the chain settled, naming the hash that mined — which after a fee bump need not // be the latest broadcast. See {@link Pending.txHashes}. - function settleMined( + const settleMined = ( entry: Pending, mined: { txHash: Hex; receipt: TxReceiptLite }, blockNumber: bigint - ): void { + ): void => { settle(entry, blockNumber) const fields = { id: entry.label, @@ -435,10 +435,18 @@ export function createPendingQueue({ } const replaced = await tryCatch(send({ ...entry.request, ...result.fees, nonce: entry.nonce })) if (replaced.error) { - // A re-broadcast that reverts means the liquidation is no longer valid (e.g. the position was - // cleared while our tx was in flight) — bumping it forever is futile, so drop it. A transient - // RPC error instead counts as a spent attempt, so `maxBumpAttempts` still bounds the retries. - if (isExecutionRevert(replaced.error)) { + // A re-broadcast that reverts means the liquidation is no longer valid — bumping it forever is + // futile, so drop it. But the ORIGINAL is a likely reason it is no longer valid: it can mine + // during the gas estimation this send just did, after the sweep's own scan read it as pending. + // Retiring on the revert alone would discard the receipt for work that succeeded. + const settled = isExecutionRevert(replaced.error) + ? await scanReceipts(getReceipt, entry.txHashes) + : null + if (settled?.kind === 'mined') { + settleMined(entry, settled, blockNumber) + return + } + if (settled?.kind === 'none') { settle(entry, blockNumber) latchNonceHole(entry.nonce) logger.warn('tx.dropped', { @@ -448,16 +456,18 @@ export function createPendingQueue({ reason: 'reverts_on_replace', detail: revertReason(replaced.error) }) - } else { - entry.attempt += 1 - logger.warn('tx.replace_failed', { - id: entry.label, - nonce: entry.nonce, - txHash: entry.txHashes[0], - attempt: entry.attempt, - reason: revertReason(replaced.error) - }) + return } + // A transient failure — of the send, or of the scan that would have justified retiring the + // entry — counts as a spent attempt, so `maxBumpAttempts` still bounds the retries. + entry.attempt += 1 + logger.warn('tx.replace_failed', { + id: entry.label, + nonce: entry.nonce, + txHash: entry.txHashes[0], + attempt: entry.attempt, + reason: revertReason(settled?.error ?? replaced.error) + }) return } const oldHash = entry.txHashes[0] diff --git a/packages/bot-kit/src/queue/receipt.utils.ts b/packages/bot-kit/src/queue/receipt.utils.ts index 0c798717..50591589 100644 --- a/packages/bot-kit/src/queue/receipt.utils.ts +++ b/packages/bot-kit/src/queue/receipt.utils.ts @@ -21,17 +21,21 @@ type ReceiptScan = * Finds the hash that mined among every hash broadcast for one nonce, newest first. * * A fee bump replaces a transaction but cannot un-broadcast it, so any hash in the list may be the - * one the chain kept. A per-hash read failure is recorded and the scan CONTINUES rather than - * aborting — otherwise a transient error on the newest hash would mask a mined original and the - * queue would report a successful transaction as dropped. + * one the chain kept. A read failure on one hash never decides the scan — otherwise a transient + * error on the newest would mask a mined original and report a successful transaction as dropped. + * + * The reads are issued together: they are independent, the common case (nothing mined) reads all of + * them anyway, and this runs in the per-block maintenance pass that the tick waits on. */ export const scanReceipts = async ( getReceipt: GetReceipt, txHashes: readonly Hex[] ): Promise => { + const reads = await Promise.all( + txHashes.map(async txHash => ({ txHash, receipt: await tryCatch(getReceipt(txHash)) })) + ) let failure: { error: unknown } | null = null - for (const txHash of txHashes) { - const receipt = await tryCatch(getReceipt(txHash)) + for (const { txHash, receipt } of reads) { if (receipt.error) { failure ??= { error: receipt.error } continue diff --git a/packages/bot-kit/test/queue/pending-queue.test.ts b/packages/bot-kit/test/queue/pending-queue.test.ts index 30625b85..8f8924cf 100644 --- a/packages/bot-kit/test/queue/pending-queue.test.ts +++ b/packages/bot-kit/test/queue/pending-queue.test.ts @@ -278,6 +278,59 @@ describe('createPendingQueue', () => { expect(events.find(e => e.event === 'tx.dropped')?.fields?.reason).toBe('reverts_on_replace') }) + it('confirms the original when the replacement reverts because that original mined', async () => { + // The revert is the position being gone, and the original is a likely reason it is gone: it can + // mine during the replacement's gas estimation, after the sweep read it as pending. + const { logger, events } = captureLogger() + let calls = 0 + const send: SendTx = async request => { + calls += 1 + if (calls === 1) return { nonce: request.nonce ?? 7, txHash: hashOf(1), gas: STUB_GAS } + // The sweep already read this nonce as pending; the original mines while the replacement is + // being estimated, and the estimation is what surfaces the revert. + mined = true + throw new ExecutionRevertedError({}) + } + let mined = false + const { queue } = setup({ + send, + logger, + getReceipt: async () => (mined ? { status: 'success', blockNumber: 42n } : null) + }) + await submitSighted(queue, 0n) + await queue.onBlock(5n) + + expect(queue.size).toBe(0) + expect(events.find(e => e.event === 'tx.confirmed')?.fields?.txHash).toBe(hashOf(1)) + expect(events.some(e => e.event === 'tx.dropped')).toBe(false) + }) + + it('keeps a stuck tx whose replacement reverted but whose own receipt could not be read', async () => { + const { logger, events } = captureLogger() + let calls = 0 + const send: SendTx = async request => { + calls += 1 + if (calls === 1) return { nonce: request.nonce ?? 7, txHash: hashOf(1), gas: STUB_GAS } + readable = false // the RPC goes away between the sweep's scan and the retire decision + throw new ExecutionRevertedError({}) + } + let readable = true + const { queue } = setup({ + send, + logger, + getReceipt: async () => { + if (readable) return null + throw new Error('rpc down') + } + }) + await submitSighted(queue, 0n) + await queue.onBlock(5n) + + expect(queue.size).toBe(1) // an unreadable receipt is not grounds to retire the nonce + expect(events.some(e => e.event === 'tx.dropped')).toBe(false) + expect(events.find(e => e.event === 'tx.replace_failed')?.fields?.attempt).toBe(1) + }) + it('retries a stuck tx on transient send failures, then drops it at maxBumpAttempts', async () => { const { logger, events } = captureLogger() let calls = 0 From ca16d8a88dc6d2c5a94b6988b6fdff6578796203 Mon Sep 17 00:00:00 2001 From: Hayden Shively Date: Thu, 3 Sep 2026 03:27:54 -0500 Subject: [PATCH 4/6] document run's side effects and failure contract Codex P1 on the crossed-books service: `run()` documented its return value but not what it emits, what it broadcasts, or what it propagates. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/application/crossed-books-bot.service.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/bots/midnight-crossed-books/src/application/crossed-books-bot.service.ts b/bots/midnight-crossed-books/src/application/crossed-books-bot.service.ts index a163f79f..547bdfbe 100644 --- a/bots/midnight-crossed-books/src/application/crossed-books-bot.service.ts +++ b/bots/midnight-crossed-books/src/application/crossed-books-bot.service.ts @@ -54,7 +54,11 @@ export class CrossedBooksBotService { /** * Computes the first profitable crossed resolution for one block. * @returns Submission status and the number of listed markets inspected. - * @remarks Always simulates first. Readonly mode logs `match.computed` and performs no submission. + * @throws Whatever market discovery, book reads, simulation, or submission raise — none are caught + * here, so the caller's tick is what isolates them. + * @remarks Always simulates first, and submits at most one resolution per call. Logs + * `match.not_profitable` / `match.computed` / `match.submitted`. Readonly mode performs no + * submission; write mode broadcasts through the pending queue, consuming a nonce. */ async run() { const markets = await this.markets.listListedActiveMarkets() From ca8feb60ffbf0e2b5ee1a3bbc5ac2bc0c29f290d Mon Sep 17 00:00:00 2001 From: Hayden Shively Date: Thu, 3 Sep 2026 03:50:11 -0500 Subject: [PATCH 5/6] give the settlement check one home MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The scan → settle-if-mined → hand-back rule had three call sites: the per-block sweep, the nonce-consumed reconciler, and the replacement- reverted path added while addressing review. Each re-derived it, and each was a place a caller could discard a retained hash unread — which is the defect this branch exists to fix. `settleIfMined` is now the only caller of `scanReceipts`. It settles a mined entry itself and returns what is left to decide, with `unreadable` distinct from `unmined` so no caller can retire an entry on a read that failed. `settleMined` folds into it, so this is a net removal. Co-Authored-By: Claude Opus 5 (1M context) --- packages/bot-kit/src/queue/pending-queue.ts | 75 +++++++++++---------- 1 file changed, 39 insertions(+), 36 deletions(-) diff --git a/packages/bot-kit/src/queue/pending-queue.ts b/packages/bot-kit/src/queue/pending-queue.ts index e9113107..ab6b61da 100644 --- a/packages/bot-kit/src/queue/pending-queue.ts +++ b/packages/bot-kit/src/queue/pending-queue.ts @@ -278,22 +278,40 @@ export function createPendingQueue({ } } - // Retires an entry the chain settled, naming the hash that mined — which after a fee bump need not - // be the latest broadcast. See {@link Pending.txHashes}. - const settleMined = ( - entry: Pending, - mined: { txHash: Hex; receipt: TxReceiptLite }, - blockNumber: bigint - ): void => { + /** + * What {@link settleIfMined} left for its caller to decide. `unreadable` NEVER justifies retiring + * an entry — the hash that could not be read is the one that may have mined — so `unmined` is the + * only answer that hands the entry back for retirement. + */ + type SettlementCheck = + | { kind: 'settled' } + | { kind: 'unmined' } + | { kind: 'unreadable'; error: unknown } + + // The one place a tracked entry is tested against the chain: every caller that would retire an + // entry goes through here first, so none of them can discard a hash unread. A settled entry is + // logged by the hash that MINED, which after a fee bump need not be the latest broadcast. + // + // A receipt is terminal the moment it appears. Free on the L2s most of these bots target (Base, + // Robinhood / Arbitrum-Orbit), which do not reorg confirmed transactions in practice; Ethereum + // mainnet does, so there a `tx.confirmed` may name a tx a short reorg orphans. The consequence + // stays bounded: the position reappears in a later discovery pass and is re-liquidated — never a + // queue entry stuck waiting on a vanished tx — but the re-plan can broadcast a second tx while the + // first is still pending, and whichever lands second reverts on-chain at the cost of its gas. + const settleIfMined = async (entry: Pending, blockNumber: bigint): Promise => { + const scan = await scanReceipts(getReceipt, entry.txHashes) + if (scan.kind === 'unknown') return { kind: 'unreadable', error: scan.error } + if (scan.kind === 'none') return { kind: 'unmined' } settle(entry, blockNumber) const fields = { id: entry.label, nonce: entry.nonce, - txHash: mined.txHash, - blockNumber: mined.receipt.blockNumber + txHash: scan.txHash, + blockNumber: scan.receipt.blockNumber } - if (mined.receipt.status === 'success') logger.info('tx.confirmed', fields) + if (scan.receipt.status === 'success') logger.info('tx.confirmed', fields) else logger.warn('tx.reverted', fields) + return { kind: 'settled' } } // The nonce-critical section, always entered under `submitMutex`: the latch checks, the empty-queue @@ -439,14 +457,11 @@ export function createPendingQueue({ // futile, so drop it. But the ORIGINAL is a likely reason it is no longer valid: it can mine // during the gas estimation this send just did, after the sweep's own scan read it as pending. // Retiring on the revert alone would discard the receipt for work that succeeded. - const settled = isExecutionRevert(replaced.error) - ? await scanReceipts(getReceipt, entry.txHashes) + const checked = isExecutionRevert(replaced.error) + ? await settleIfMined(entry, blockNumber) : null - if (settled?.kind === 'mined') { - settleMined(entry, settled, blockNumber) - return - } - if (settled?.kind === 'none') { + if (checked?.kind === 'settled') return + if (checked?.kind === 'unmined') { settle(entry, blockNumber) latchNonceHole(entry.nonce) logger.warn('tx.dropped', { @@ -466,7 +481,7 @@ export function createPendingQueue({ nonce: entry.nonce, txHash: entry.txHashes[0], attempt: entry.attempt, - reason: revertReason(settled?.error ?? replaced.error) + reason: revertReason(checked?.kind === 'unreadable' ? checked.error : replaced.error) }) return } @@ -501,11 +516,10 @@ export function createPendingQueue({ // Deleting the current key mid-iteration (via `drop`) is well-defined for a Map. for (const entry of pending.values()) { if (entry.nonce >= count.data) continue - const scan = await scanReceipts(getReceipt, entry.txHashes) // A consumed nonce whose receipt is ours is a settlement, not a loss — the sweep can miss it // when the receipt lands between the two passes, or when its read failed and this one didn't. - if (scan.kind === 'mined') settleMined(entry, scan, blockNumber) - else if (scan.kind === 'none') drop(entry.nonce, 'nonce_consumed') + const checked = await settleIfMined(entry, blockNumber) + if (checked.kind === 'unmined') drop(entry.nonce, 'nonce_consumed') } } @@ -550,30 +564,19 @@ export function createPendingQueue({ // Per-entry isolation: one entry's transient read failure (getReceipt/getBaseFee) must not // abort the sweep for the rest of the queue. replaceStuck owns its own send-error handling. try { - const scan = await scanReceipts(getReceipt, entry.txHashes) - if (scan.kind === 'unknown') { + const checked = await settleIfMined(entry, blockNumber) + if (checked.kind === 'settled') continue + if (checked.kind === 'unreadable') { // Skipping the stuck check is deliberate: replacing a tx whose receipt we could not read // may replace one that already mined. logger.warn('tx.onblock_error', { id: entry.label, nonce: entry.nonce, txHash: entry.txHashes[0], - reason: revertReason(scan.error) + reason: revertReason(checked.error) }) continue } - if (scan.kind === 'mined') { - // One-block receipt finality: a receipt is treated as terminal (confirm or revert) the - // moment it appears. Free on the L2s most of these bots target (Base, Robinhood / - // Arbitrum-Orbit), which do not reorg confirmed transactions in practice; Ethereum - // mainnet does, so there a `tx.confirmed` may name a tx that a short reorg orphans. - // The consequence stays bounded: the position reappears in a later discovery pass and is - // re-liquidated — never a queue entry stuck waiting on a vanished tx — but the re-plan - // can broadcast a second tx while the first is still pending, and whichever lands second - // reverts on-chain at the cost of its gas. - settleMined(entry, scan, blockNumber) - continue - } if (entry.submittedAtBlock === null) { entry.submittedAtBlock = blockNumber continue From 6df560e296c2a0b8e0515226db754f955d4b2d3e Mon Sep 17 00:00:00 2001 From: Hayden Date: Thu, 3 Sep 2026 18:01:10 +0000 Subject: [PATCH 6/6] docs(bot-kit, midnight-crossed-books): clarify onBlock and run JSDoc - Add substantive JSDoc for PendingQueue.onBlock per Codex review. - Clarify CrossedBooksBotService.run() documents attempted queue submission rather than unconditional broadcast. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../src/application/crossed-books-bot.service.ts | 7 +++++-- packages/bot-kit/src/queue/pending-queue.ts | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/bots/midnight-crossed-books/src/application/crossed-books-bot.service.ts b/bots/midnight-crossed-books/src/application/crossed-books-bot.service.ts index 547bdfbe..0d03697b 100644 --- a/bots/midnight-crossed-books/src/application/crossed-books-bot.service.ts +++ b/bots/midnight-crossed-books/src/application/crossed-books-bot.service.ts @@ -53,12 +53,15 @@ export class CrossedBooksBotService { /** * Computes the first profitable crossed resolution for one block. - * @returns Submission status and the number of listed markets inspected. + * @returns Whether a profitable resolution was found and handed to the resolver for attempted + * queue submission, and the number of listed markets inspected. * @throws Whatever market discovery, book reads, simulation, or submission raise — none are caught * here, so the caller's tick is what isolates them. * @remarks Always simulates first, and submits at most one resolution per call. Logs * `match.not_profitable` / `match.computed` / `match.submitted`. Readonly mode performs no - * submission; write mode broadcasts through the pending queue, consuming a nonce. + * submission; write mode hands the prepared resolution to the resolver, which submits it through + * the pending queue. `match.submitted` records acceptance by the resolver, not an on-chain + * broadcast guarantee — the pending queue may still decline the transaction. */ async run() { const markets = await this.markets.listListedActiveMarkets() diff --git a/packages/bot-kit/src/queue/pending-queue.ts b/packages/bot-kit/src/queue/pending-queue.ts index ab6b61da..52ffcd12 100644 --- a/packages/bot-kit/src/queue/pending-queue.ts +++ b/packages/bot-kit/src/queue/pending-queue.ts @@ -141,6 +141,22 @@ export type PendingQueue = { * rewind the cursor past an in-flight send. */ submit(args: SubmitArgs): Promise + /** + * Advances the queue by one block. For every pending transaction it scans all tracked hashes for + * receipts; if any hash mined, the entry is settled and removed. For entries that have not yet + * been sighted, it records the current block as `submittedAtBlock` — this first-sighting block + * becomes the baseline for stuck detection. Entries whose first-sighting block is older than + * `stuckBlocks` are replaced by `replaceStuck`. After the per-entry pass it reconciles consumed + * nonces on cadence, clears any nonce-hole latch if the chain has caught up, prunes settled + * cooldowns, and releases the send latch. + * + * @param blockNumber - The observed chain head. Used to age entries, bound cooldowns, and pass to + * `replaceStuck`. This is the queue's own observation of the head, not the value supplied to + * `submit`. + * @returns Resolves once the sweep and all side effects are complete; never rejects — per-entry + * receipt/base-fee read failures are isolated and logged as `tx.onblock_error`, and + * `replaceStuck` owns its own send-error handling. + */ onBlock(blockNumber: bigint): Promise readonly size: number snapshot(): { nonce: number; txHash: Hex; attempt: number }[]