From 479cabea3d3249dfaa5527c020f407766294b741 Mon Sep 17 00:00:00 2001 From: Jayesh Bhole <54071350+jayeshbhole@users.noreply.github.com> Date: Tue, 25 Aug 2026 20:14:05 +0530 Subject: [PATCH 1/7] fix: reconcile pending bundles on a timer when blocks stall handleBlock is the only path that resolves a submitted bundle and returns its executor wallet to the sender pool, but watchBlocks only fires it on a new block. On a chain that produces blocks only when it receives a transaction, a bundler that stops submitting waits for a block that only it would have caused: no wallet is freed, every later bundle blocks forever in getWallet(), and userOps are accepted but never included. Keep the block-driven receipt polling (a receipt cannot change without a new block) and add a watchdog that runs handleBlock once no reconcile has happened within resubmitStuckTimeout while bundles are still pending. No RPC work on a healthy chain. Also hold currentlyHandlingBlock in try/finally: a throw in handleBlockInner left it set, wedging every later tick at the overlap guard. --- src/executor/executorManager.test.ts | 150 +++++++++++++++++++++++++++ src/executor/executorManager.ts | 92 ++++++++++++++-- 2 files changed, 231 insertions(+), 11 deletions(-) create mode 100644 src/executor/executorManager.test.ts diff --git a/src/executor/executorManager.test.ts b/src/executor/executorManager.test.ts new file mode 100644 index 00000000..3c09008f --- /dev/null +++ b/src/executor/executorManager.test.ts @@ -0,0 +1,150 @@ +import { beforeEach, describe, expect, it, vi } from "vitest" +import { ExecutorManager } from "./executorManager" + +// Chains that only produce a block when they receive a transaction (Arbitrum +// Orbit and friends) deadlock a purely block-driven reconciler: the bundler +// stops submitting, so no block is produced, so handleBlock never runs, so no +// executor wallet is ever freed. These cover the timer that breaks that loop. + +const RESUBMIT_STUCK_TIMEOUT = 10_000 +const BLOCK_TIME = 1000 + +const noopLogger = () => { + const logger = { + trace: vi.fn(), + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn() + } + return logger +} + +const createHarness = () => { + // Captured so a test can emit a block the way watchBlocks would. + let onBlock: ((block: unknown) => Promise) | undefined + const unwatch = vi.fn() + + const getBundleStatuses = vi.fn().mockResolvedValue([]) + + const bundleManager = { + // One bundle pending for the whole test: handleBlock must keep + // reconciling it, and must not stop the watcher. + getPendingBundles: vi.fn().mockReturnValue([{ uid: "0xbundle" }]), + getBundleStatuses + } + + const config = { + bundleMode: "manual", + blockTime: BLOCK_TIME, + resubmitStuckTimeout: RESUBMIT_STUCK_TIMEOUT, + flashblocksPreconfirmationTime: undefined, + // Keeps getBaseFee() off the network. + legacyTransactions: true, + logLevel: "info", + executorLogLevel: "info", + getLogger: () => noopLogger(), + publicClient: { + watchBlocks: (args: { + onBlock: (block: unknown) => Promise + }) => { + onBlock = args.onBlock + return unwatch + } + } + } + + const executorManager = new ExecutorManager({ + // biome-ignore lint/suspicious/noExplicitAny: narrow stubs, only the + // block-reconcile path is under test. + config: config as any, + executor: {} as any, + mempool: {} as any, + metrics: {} as any, + gasPriceManager: { + tryGetNetworkGasPrice: vi.fn().mockResolvedValue({ + maxFeePerGas: 1n, + maxPriorityFeePerGas: 1n + }) + } as any, + senderManager: {} as any, + bundleManager: bundleManager as any + }) + + return { + executorManager, + getBundleStatuses, + unwatch, + emitBlock: async () => { + await onBlock?.({ number: 1n, baseFeePerGas: 1n }) + } + } +} + +describe("ExecutorManager stale block watchdog", () => { + beforeEach(() => { + vi.useFakeTimers() + return () => vi.useRealTimers() + }) + + it("reconciles pending bundles when no new block arrives", async () => { + const { executorManager, getBundleStatuses } = createHarness() + + executorManager.startWatchingBlocks() + + // No block is ever emitted. Before the watchdog this was a permanent + // stall: pending bundles held their wallets forever. + expect(getBundleStatuses).not.toHaveBeenCalled() + + await vi.advanceTimersByTimeAsync(RESUBMIT_STUCK_TIMEOUT + BLOCK_TIME) + + expect(getBundleStatuses).toHaveBeenCalled() + }) + + it("stays idle while blocks keep arriving", async () => { + const { executorManager, getBundleStatuses, emitBlock } = + createHarness() + + executorManager.startWatchingBlocks() + + // A block every blockTime keeps lastReconcileAt fresh, so the watchdog + // must add no reconciles (and therefore no RPC) of its own. + for (let i = 0; i < RESUBMIT_STUCK_TIMEOUT / BLOCK_TIME + 2; i++) { + await emitBlock() + await vi.advanceTimersByTimeAsync(BLOCK_TIME) + } + + expect(getBundleStatuses).toHaveBeenCalledTimes( + RESUBMIT_STUCK_TIMEOUT / BLOCK_TIME + 2 + ) + }) + + it("keeps reconciling after a failed tick", async () => { + const { executorManager, getBundleStatuses, emitBlock } = + createHarness() + + getBundleStatuses.mockRejectedValueOnce(new Error("store unavailable")) + + executorManager.startWatchingBlocks() + + // A throw used to leave currentlyHandlingBlock set, so every later tick + // returned early at the guard and reconciliation never resumed. + await emitBlock().catch(() => undefined) + await emitBlock() + + expect(getBundleStatuses).toHaveBeenCalledTimes(2) + }) + + it("clears the watchdog when the watcher stops", async () => { + const { executorManager, getBundleStatuses, unwatch } = createHarness() + + executorManager.startWatchingBlocks() + executorManager.stopWatchingBlocks() + + expect(unwatch).toHaveBeenCalled() + + await vi.advanceTimersByTimeAsync(RESUBMIT_STUCK_TIMEOUT * 3) + + expect(getBundleStatuses).not.toHaveBeenCalled() + }) +}) diff --git a/src/executor/executorManager.ts b/src/executor/executorManager.ts index ccbeff33..3b5de77f 100644 --- a/src/executor/executorManager.ts +++ b/src/executor/executorManager.ts @@ -57,6 +57,11 @@ export class ExecutorManager { private currentlyHandlingBlock = false + // When handleBlock last reconciled pending bundles, from either trigger + // (a new block, or the stale-block watchdog below). Drives the watchdog. + private lastReconcileAt = Date.now() + private staleBlockTimer: NodeJS.Timeout | undefined + // Executor wallets rotated away from a stuck bundle whose cancel did not // confirm. Held out of the sender pool (markWalletProcessed deferred) until // their on-chain nonce advances past the stuck nonce, so no other instance @@ -219,12 +224,64 @@ export class ExecutorManager { includeTransactions: false, emitMissed: false }) + this.startStaleBlockWatchdog() } this.logger.debug("started watching blocks") }) } + // handleBlock is the only path that resolves a submitted bundle: it reads + // receipts, applies resubmitStuckTimeout, and returns the executor wallet + // to the sender pool. watchBlocks only fires it on a NEW block, which + // deadlocks a chain that produces blocks only when it receives a + // transaction (Arbitrum Orbit and friends): the bundler stops submitting -> + // no blocks -> handleBlock never runs -> no wallet is ever freed -> every + // later bundle blocks forever in getWallet(). Nothing inside that loop can + // break it; in practice only an unrelated third-party transaction did. + // + // Receipt polling itself is correctly block-driven (a receipt cannot change + // without a new block, and each tick costs one lookup per pending bundle + // plus gas price reads), so the watcher stays and the interval below does + // no RPC work on a healthy chain. This only re-arms the *time*-based stuck + // check that the block gate otherwise makes unreachable — the same fix + // reconcileQuarantinedWallets already applies one layer down. + private startStaleBlockWatchdog(): void { + if (this.staleBlockTimer) { + return + } + + this.lastReconcileAt = Date.now() + this.staleBlockTimer = setInterval(() => { + const msSinceReconcile = Date.now() - this.lastReconcileAt + + if (msSinceReconcile < this.config.resubmitStuckTimeout) { + return + } + + const pendingBundles = this.bundleManager.getPendingBundles().length + if (pendingBundles === 0) { + return + } + + this.logger.warn( + { + event: "staleBlockWatchdogFired", + msSinceReconcile, + pendingBundles + }, + "no new block within resubmitStuckTimeout, reconciling pending bundles on a timer" + ) + + this.handleBlock().catch((err) => + this.logger.error( + { err }, + "stale block watchdog failed to reconcile pending bundles" + ) + ) + }, this.config.blockTime) + } + async getBaseFee(): Promise { if (this.config.legacyTransactions) { return 0n @@ -479,6 +536,10 @@ export class ExecutorManager { this.unWatch() this.unWatch = undefined } + if (this.staleBlockTimer) { + clearInterval(this.staleBlockTimer) + this.staleBlockTimer = undefined + } } private async updateTransactionCostMetrics( @@ -537,29 +598,40 @@ export class ExecutorManager { return } + // Held in try/finally rather than reset at the end of handleBlockInner: + // an unexpected throw in there (a store error in freeSubmittedBundle, + // say) would otherwise leave the flag set and make every later tick + // return early at the guard above — wedging bundle reconciliation for + // the lifetime of the process. Same guard shape as + // reconcileQuarantinedWallets. + this.currentlyHandlingBlock = true + // startWatchingBlocks() registers its timers inside whichever flow // first started the watcher, so those timers inherit that flow's log // context on every future tick. Open a fresh context here so block // handling is never attributed to one arbitrary bundle. - await runWithLogContext({ flow: "block" }, () => - timed( - this.logger, - "handleBlock", - { blockNumber: block ? Number(block.number) : undefined }, - () => this.handleBlockInner(block) + try { + await runWithLogContext({ flow: "block" }, () => + timed( + this.logger, + "handleBlock", + { blockNumber: block ? Number(block.number) : undefined }, + () => this.handleBlockInner(block) + ) ) - ) + } finally { + this.currentlyHandlingBlock = false + } } private async handleBlockInner(block?: Block) { - this.currentlyHandlingBlock = true const blockReceivedTimestamp = Date.now() + this.lastReconcileAt = blockReceivedTimestamp const pendingBundles = this.bundleManager.getPendingBundles() if (pendingBundles.length === 0) { this.stopWatchingBlocks() - this.currentlyHandlingBlock = false return } @@ -623,8 +695,6 @@ export class ExecutorManager { } }) ) - - this.currentlyHandlingBlock = false } potentiallyResubmitBundle({ From 076209538334be5b83f088ea44a5bee8f4096723 Mon Sep 17 00:00:00 2001 From: Jayesh Bhole <54071350+jayeshbhole@users.noreply.github.com> Date: Thu, 3 Sep 2026 17:50:51 +0530 Subject: [PATCH 2/7] fix: address review on stale block watchdog - stop watcher when watchdog finds no pending bundles - align isStuck to >= to match watchdog boundary - test not_found -> watchdog -> included -> bundle released --- src/executor/executorManager.test.ts | 121 +++++++++++++++++++++++---- src/executor/executorManager.ts | 33 +++----- 2 files changed, 115 insertions(+), 39 deletions(-) diff --git a/src/executor/executorManager.test.ts b/src/executor/executorManager.test.ts index 3c09008f..f4cf3c4f 100644 --- a/src/executor/executorManager.test.ts +++ b/src/executor/executorManager.test.ts @@ -1,10 +1,8 @@ import { beforeEach, describe, expect, it, vi } from "vitest" import { ExecutorManager } from "./executorManager" -// Chains that only produce a block when they receive a transaction (Arbitrum -// Orbit and friends) deadlock a purely block-driven reconciler: the bundler -// stops submitting, so no block is produced, so handleBlock never runs, so no -// executor wallet is ever freed. These cover the timer that breaks that loop. +// Chains that only produce a block per transaction deadlock a block-driven +// reconciler. These cover the timer that breaks that loop. const RESUBMIT_STUCK_TIMEOUT = 10_000 const BLOCK_TIME = 1000 @@ -25,13 +23,38 @@ const createHarness = () => { let onBlock: ((block: unknown) => Promise) | undefined const unwatch = vi.fn() + const submittedBundle = { + uid: "0xbundle", + transactionHash: "0xtx", + previousTransactionHashes: [], + // Matches networkGasPrice, so isGasPriceTooLow stays false. + transactionRequest: { + maxFeePerGas: 1n, + maxPriorityFeePerGas: 1n, + nonce: 0 + }, + bundle: { + entryPoint: "0xentrypoint", + version: "0.7", + userOps: [{ userOpHash: "0xuserop" }], + submissionAttempts: 1 + }, + executor: { address: "0xexecutor" }, + lastReplaced: Date.now() + } + + // Mutable: production drops the bundle in processIncludedBundle. + let pendingBundles: unknown[] = [submittedBundle] + const getBundleStatuses = vi.fn().mockResolvedValue([]) + const processIncludedBundle = vi.fn(() => { + pendingBundles = [] + }) const bundleManager = { - // One bundle pending for the whole test: handleBlock must keep - // reconciling it, and must not stop the watcher. - getPendingBundles: vi.fn().mockReturnValue([{ uid: "0xbundle" }]), - getBundleStatuses + getPendingBundles: vi.fn(() => pendingBundles), + getBundleStatuses, + processIncludedBundle } const config = { @@ -50,13 +73,16 @@ const createHarness = () => { }) => { onBlock = args.onBlock return unwatch - } + }, + // Only cost metrics reach this, and they swallow failures. + getTransactionReceipt: vi + .fn() + .mockRejectedValue(new Error("no receipt in test")) } } const executorManager = new ExecutorManager({ - // biome-ignore lint/suspicious/noExplicitAny: narrow stubs, only the - // block-reconcile path is under test. + // Narrow stubs: only the block-reconcile path is under test. config: config as any, executor: {} as any, mempool: {} as any, @@ -74,13 +100,23 @@ const createHarness = () => { return { executorManager, getBundleStatuses, + processIncludedBundle, + submittedBundle, unwatch, + getPending: () => pendingBundles, emitBlock: async () => { await onBlock?.({ number: 1n, baseFeePerGas: 1n }) } } } +const includedStatus = { + status: "included", + userOpReceipts: {}, + transactionHash: "0xtx", + blockNumber: 2n +} + describe("ExecutorManager stale block watchdog", () => { beforeEach(() => { vi.useFakeTimers() @@ -92,8 +128,7 @@ describe("ExecutorManager stale block watchdog", () => { executorManager.startWatchingBlocks() - // No block is ever emitted. Before the watchdog this was a permanent - // stall: pending bundles held their wallets forever. + // No block ever emitted. Was a permanent stall before the watchdog. expect(getBundleStatuses).not.toHaveBeenCalled() await vi.advanceTimersByTimeAsync(RESUBMIT_STUCK_TIMEOUT + BLOCK_TIME) @@ -101,14 +136,67 @@ describe("ExecutorManager stale block watchdog", () => { expect(getBundleStatuses).toHaveBeenCalled() }) + it("releases a bundle that lands while blocks are stalled", async () => { + const { + executorManager, + getBundleStatuses, + processIncludedBundle, + submittedBundle, + emitBlock, + getPending + } = createHarness() + + // Not yet included, and this is the last block the chain produces. + getBundleStatuses + .mockResolvedValueOnce([{ status: "not_found" }]) + .mockResolvedValueOnce([includedStatus]) + + executorManager.startWatchingBlocks() + + await emitBlock() + + expect(processIncludedBundle).not.toHaveBeenCalled() + expect(getPending()).toHaveLength(1) + + // Only the watchdog can see the inclusion now. + await vi.advanceTimersByTimeAsync(RESUBMIT_STUCK_TIMEOUT) + + expect(processIncludedBundle).toHaveBeenCalledWith( + expect.objectContaining({ submittedBundle }) + ) + // Left the pending set -> wallet released. + expect(getPending()).toHaveLength(0) + }) + + it("stops watching once the last pending bundle is resolved", async () => { + const { executorManager, getBundleStatuses, emitBlock, unwatch } = + createHarness() + + getBundleStatuses + .mockResolvedValueOnce([{ status: "not_found" }]) + .mockResolvedValueOnce([includedStatus]) + + executorManager.startWatchingBlocks() + + await emitBlock() + await vi.advanceTimersByTimeAsync(RESUBMIT_STUCK_TIMEOUT) + + expect(unwatch).not.toHaveBeenCalled() + + // Nothing pending and no block coming, so the watchdog must clean up + // or both it and the block watcher poll forever. + await vi.advanceTimersByTimeAsync(RESUBMIT_STUCK_TIMEOUT) + + expect(unwatch).toHaveBeenCalled() + }) + it("stays idle while blocks keep arriving", async () => { const { executorManager, getBundleStatuses, emitBlock } = createHarness() executorManager.startWatchingBlocks() - // A block every blockTime keeps lastReconcileAt fresh, so the watchdog - // must add no reconciles (and therefore no RPC) of its own. + // Fresh lastReconcileAt -> watchdog adds no reconciles of its own. for (let i = 0; i < RESUBMIT_STUCK_TIMEOUT / BLOCK_TIME + 2; i++) { await emitBlock() await vi.advanceTimersByTimeAsync(BLOCK_TIME) @@ -127,8 +215,7 @@ describe("ExecutorManager stale block watchdog", () => { executorManager.startWatchingBlocks() - // A throw used to leave currentlyHandlingBlock set, so every later tick - // returned early at the guard and reconciliation never resumed. + // A throw used to leave currentlyHandlingBlock set forever. await emitBlock().catch(() => undefined) await emitBlock() diff --git a/src/executor/executorManager.ts b/src/executor/executorManager.ts index 3b5de77f..d44155d0 100644 --- a/src/executor/executorManager.ts +++ b/src/executor/executorManager.ts @@ -231,21 +231,11 @@ export class ExecutorManager { }) } - // handleBlock is the only path that resolves a submitted bundle: it reads - // receipts, applies resubmitStuckTimeout, and returns the executor wallet - // to the sender pool. watchBlocks only fires it on a NEW block, which - // deadlocks a chain that produces blocks only when it receives a - // transaction (Arbitrum Orbit and friends): the bundler stops submitting -> - // no blocks -> handleBlock never runs -> no wallet is ever freed -> every - // later bundle blocks forever in getWallet(). Nothing inside that loop can - // break it; in practice only an unrelated third-party transaction did. - // - // Receipt polling itself is correctly block-driven (a receipt cannot change - // without a new block, and each tick costs one lookup per pending bundle - // plus gas price reads), so the watcher stays and the interval below does - // no RPC work on a healthy chain. This only re-arms the *time*-based stuck - // check that the block gate otherwise makes unreachable — the same fix - // reconcileQuarantinedWallets already applies one layer down. + // handleBlock is the only path that frees an executor wallet, and + // watchBlocks only fires it on a new block. On a chain that produces blocks + // only when it receives a transaction, that deadlocks: no submissions -> no + // blocks -> no wallets freed. Re-arms the time-based stuck check the block + // gate makes unreachable; no RPC on a healthy chain. private startStaleBlockWatchdog(): void { if (this.staleBlockTimer) { return @@ -261,6 +251,8 @@ export class ExecutorManager { const pendingBundles = this.bundleManager.getPendingBundles().length if (pendingBundles === 0) { + // No block will arrive to run handleBlockInner's cleanup. + this.stopWatchingBlocks() return } @@ -598,12 +590,8 @@ export class ExecutorManager { return } - // Held in try/finally rather than reset at the end of handleBlockInner: - // an unexpected throw in there (a store error in freeSubmittedBundle, - // say) would otherwise leave the flag set and make every later tick - // return early at the guard above — wedging bundle reconciliation for - // the lifetime of the process. Same guard shape as - // reconcileQuarantinedWallets. + // try/finally, else a throw in handleBlockInner leaves this set and + // every later tick bails at the guard above. this.currentlyHandlingBlock = true // startWatchingBlocks() registers its timers inside whichever flow @@ -718,8 +706,9 @@ export class ExecutorManager { maxFeePerGas < networkGasPrice.maxFeePerGas || maxPriorityFeePerGas < networkGasPrice.maxPriorityFeePerGas + // >= to match the stale block watchdog's boundary. const isStuck = - Date.now() - lastReplaced > this.config.resubmitStuckTimeout + Date.now() - lastReplaced >= this.config.resubmitStuckTimeout if (!(isGasPriceTooLow || isStuck)) { return From 2df6b56cdfa3a0d89d4f0ed14a18cddc957598d7 Mon Sep 17 00:00:00 2001 From: Jayesh Bhole <54071350+jayeshbhole@users.noreply.github.com> Date: Thu, 3 Sep 2026 18:37:04 +0530 Subject: [PATCH 3/7] probe: revert both review fixes to isolate e2e failure Temporary. Local e2e passes 3/3 on the review-fix commit while CI fails 2/2 on the same 5 parallel-op tests, so bisecting in CI. --- src/executor/executorManager.test.ts | 3 ++- src/executor/executorManager.ts | 5 +---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/executor/executorManager.test.ts b/src/executor/executorManager.test.ts index f4cf3c4f..8a3e81d6 100644 --- a/src/executor/executorManager.test.ts +++ b/src/executor/executorManager.test.ts @@ -168,7 +168,8 @@ describe("ExecutorManager stale block watchdog", () => { expect(getPending()).toHaveLength(0) }) - it("stops watching once the last pending bundle is resolved", async () => { + // biome-ignore lint/suspicious/noSkippedTests: temporary e2e bisect probe + it.skip("stops watching once the last pending bundle is resolved", async () => { const { executorManager, getBundleStatuses, emitBlock, unwatch } = createHarness() diff --git a/src/executor/executorManager.ts b/src/executor/executorManager.ts index d44155d0..b253cae1 100644 --- a/src/executor/executorManager.ts +++ b/src/executor/executorManager.ts @@ -251,8 +251,6 @@ export class ExecutorManager { const pendingBundles = this.bundleManager.getPendingBundles().length if (pendingBundles === 0) { - // No block will arrive to run handleBlockInner's cleanup. - this.stopWatchingBlocks() return } @@ -706,9 +704,8 @@ export class ExecutorManager { maxFeePerGas < networkGasPrice.maxFeePerGas || maxPriorityFeePerGas < networkGasPrice.maxPriorityFeePerGas - // >= to match the stale block watchdog's boundary. const isStuck = - Date.now() - lastReplaced >= this.config.resubmitStuckTimeout + Date.now() - lastReplaced > this.config.resubmitStuckTimeout if (!(isGasPriceTooLow || isStuck)) { return From f36d0867a385083426c44e52c0fcf8e2498b4e07 Mon Sep 17 00:00:00 2001 From: Jayesh Bhole <54071350+jayeshbhole@users.noreply.github.com> Date: Thu, 3 Sep 2026 19:24:49 +0530 Subject: [PATCH 4/7] probe: re-apply only the isStuck >= change Isolates which of the two review fixes breaks the parallel-op e2e tests. --- src/executor/executorManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/executor/executorManager.ts b/src/executor/executorManager.ts index b253cae1..0f6d8c78 100644 --- a/src/executor/executorManager.ts +++ b/src/executor/executorManager.ts @@ -705,7 +705,7 @@ export class ExecutorManager { maxPriorityFeePerGas < networkGasPrice.maxPriorityFeePerGas const isStuck = - Date.now() - lastReplaced > this.config.resubmitStuckTimeout + Date.now() - lastReplaced >= this.config.resubmitStuckTimeout if (!(isGasPriceTooLow || isStuck)) { return From ad2fff02a80ddf339757683304ed144ad322e971 Mon Sep 17 00:00:00 2001 From: Jayesh Bhole <54071350+jayeshbhole@users.noreply.github.com> Date: Thu, 3 Sep 2026 20:06:12 +0530 Subject: [PATCH 5/7] fix: drop the watcher-stop from the stale block watchdog Stopping watchBlocks when no bundles are pending failed 5 parallel-op e2e tests. Bisected in CI: fixes 1+2 red, neither green, fix 2 alone green. The idle-watcher leak predates this PR. --- src/executor/executorManager.test.ts | 23 ----------------------- src/executor/executorManager.ts | 3 +++ 2 files changed, 3 insertions(+), 23 deletions(-) diff --git a/src/executor/executorManager.test.ts b/src/executor/executorManager.test.ts index 8a3e81d6..77759df3 100644 --- a/src/executor/executorManager.test.ts +++ b/src/executor/executorManager.test.ts @@ -168,29 +168,6 @@ describe("ExecutorManager stale block watchdog", () => { expect(getPending()).toHaveLength(0) }) - // biome-ignore lint/suspicious/noSkippedTests: temporary e2e bisect probe - it.skip("stops watching once the last pending bundle is resolved", async () => { - const { executorManager, getBundleStatuses, emitBlock, unwatch } = - createHarness() - - getBundleStatuses - .mockResolvedValueOnce([{ status: "not_found" }]) - .mockResolvedValueOnce([includedStatus]) - - executorManager.startWatchingBlocks() - - await emitBlock() - await vi.advanceTimersByTimeAsync(RESUBMIT_STUCK_TIMEOUT) - - expect(unwatch).not.toHaveBeenCalled() - - // Nothing pending and no block coming, so the watchdog must clean up - // or both it and the block watcher poll forever. - await vi.advanceTimersByTimeAsync(RESUBMIT_STUCK_TIMEOUT) - - expect(unwatch).toHaveBeenCalled() - }) - it("stays idle while blocks keep arriving", async () => { const { executorManager, getBundleStatuses, emitBlock } = createHarness() diff --git a/src/executor/executorManager.ts b/src/executor/executorManager.ts index 0f6d8c78..58cf0dab 100644 --- a/src/executor/executorManager.ts +++ b/src/executor/executorManager.ts @@ -251,6 +251,9 @@ export class ExecutorManager { const pendingBundles = this.bundleManager.getPendingBundles().length if (pendingBundles === 0) { + // Deliberately not stopWatchingBlocks(): re-arming a fresh + // watcher can miss the block the next submission triggers, so + // that bundle never reconciles and its wallet never frees. return } From b8b8c16b66c2957380cf12f7fcdd212b6d805cda Mon Sep 17 00:00:00 2001 From: Jayesh Bhole <54071350+jayeshbhole@users.noreply.github.com> Date: Mon, 7 Sep 2026 19:29:58 +0530 Subject: [PATCH 6/7] fix: restore watchdog cleanup and fix e2e deploy receipt race - watchdog calls stopWatchingBlocks() when no pending bundles remain, instead of leaking the watcher and its timer - await the deploy op receipt before building follow-up ops, which were racing it and hitting AA10 sender already constructed - expand watchdog unit tests to 13 cases: not_found replacement, both sides of the >= boundary, watchdog-path failure recovery, overlap guard, mixed batches, cleanup, restart idempotency - run src unit tests in CI, which nothing invoked before Part of SUP-1552 --- .github/workflows/lint.yaml | 3 + .gitignore | 3 +- package.json | 1 + src/executor/executorManager.test.ts | 296 +++++++++++++++---- src/executor/executorManager.ts | 7 +- test/e2e/tests/eth_sendUserOperation.test.ts | 10 +- 6 files changed, 252 insertions(+), 68 deletions(-) diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index 816248c6..0dcfc558 100644 --- a/.github/workflows/lint.yaml +++ b/.github/workflows/lint.yaml @@ -22,6 +22,9 @@ jobs: - name: Run format check run: pnpm run lint:fix && pnpm run lint + - name: Run unit tests + run: pnpm run test:unit + - uses: stefanzweifel/git-auto-commit-action@v5 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.gitignore b/.gitignore index 6c16ca25..1678df42 100644 --- a/.gitignore +++ b/.gitignore @@ -27,4 +27,5 @@ src/contracts/* !src/contracts/EntryPointGasEstimationOverride.sol/ !src/contracts/EntryPointGasEstimationOverride.sol/EntryPointGasEstimationOverride06.json -docs/superpowers/* \ No newline at end of file +docs/superpowers/* +.claude diff --git a/package.json b/package.json index 356245a0..5bf63cf9 100644 --- a/package.json +++ b/package.json @@ -26,6 +26,7 @@ "dev": "nodemon --ext ts,js,json --watch src --exec DOTENV_CONFIG_PATH=$(pwd)/.env tsx --tsconfig src/tsconfig.json src/cli/alto.ts run", "test": "pnpm --filter e2e run test", "test:ci": "pnpm --filter e2e run test:ci", + "test:unit": "pnpm --filter @pimlico/alto run test:unit", "test:spec": "./test/spec-tests/run-spec-tests.sh", "lint": "biome check .", "lint:fix": "pnpm run lint --apply", diff --git a/src/executor/executorManager.test.ts b/src/executor/executorManager.test.ts index 77759df3..0778f406 100644 --- a/src/executor/executorManager.test.ts +++ b/src/executor/executorManager.test.ts @@ -3,77 +3,89 @@ import { ExecutorManager } from "./executorManager" // Chains that only produce a block per transaction deadlock a block-driven // reconciler. These cover the timer that breaks that loop. +// +// Wallet release lives in BundleManager.processIncludedBundle. +// These tests assert that timer-driven reconciliation delegates included +// bundles to that boundary. const RESUBMIT_STUCK_TIMEOUT = 10_000 const BLOCK_TIME = 1000 -const noopLogger = () => { - const logger = { - trace: vi.fn(), - debug: vi.fn(), - info: vi.fn(), - warn: vi.fn(), - error: vi.fn() - } - return logger -} +const noopLogger = () => ({ + trace: vi.fn(), + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn() +}) + +// Matches networkGasPrice below, so isGasPriceTooLow stays false and only the +// stuck path can fire. +const createBundle = (overrides: Record = {}) => ({ + uid: "0xbundle", + transactionHash: "0xtx", + previousTransactionHashes: [], + transactionRequest: { + maxFeePerGas: 1n, + maxPriorityFeePerGas: 1n, + nonce: 0 + }, + bundle: { + entryPoint: "0xentrypoint", + version: "0.7", + userOps: [{ userOpHash: "0xuserop" }], + submissionAttempts: 1 + }, + executor: { address: "0xexecutor" }, + lastReplaced: Date.now(), + ...overrides +}) -const createHarness = () => { +const createHarness = (bundles: unknown[] = [createBundle()]) => { // Captured so a test can emit a block the way watchBlocks would. let onBlock: ((block: unknown) => Promise) | undefined const unwatch = vi.fn() - const submittedBundle = { - uid: "0xbundle", - transactionHash: "0xtx", - previousTransactionHashes: [], - // Matches networkGasPrice, so isGasPriceTooLow stays false. - transactionRequest: { - maxFeePerGas: 1n, - maxPriorityFeePerGas: 1n, - nonce: 0 - }, - bundle: { - entryPoint: "0xentrypoint", - version: "0.7", - userOps: [{ userOpHash: "0xuserop" }], - submissionAttempts: 1 - }, - executor: { address: "0xexecutor" }, - lastReplaced: Date.now() - } - - // Mutable: production drops the bundle in processIncludedBundle. - let pendingBundles: unknown[] = [submittedBundle] + let pendingBundles = [...bundles] const getBundleStatuses = vi.fn().mockResolvedValue([]) - const processIncludedBundle = vi.fn(() => { - pendingBundles = [] - }) + const processIncludedBundle = vi.fn( + ({ submittedBundle }: { submittedBundle: unknown }) => { + pendingBundles = pendingBundles.filter( + (b) => b !== submittedBundle + ) + } + ) const bundleManager = { getPendingBundles: vi.fn(() => pendingBundles), getBundleStatuses, - processIncludedBundle + processIncludedBundle, + processRevertedBundle: vi.fn(), + stopTrackingBundle: vi.fn() } + const watchBlocks = vi.fn( + (args: { onBlock: (block: unknown) => Promise }) => { + onBlock = args.onBlock + return unwatch + } + ) + const config = { bundleMode: "manual", blockTime: BLOCK_TIME, resubmitStuckTimeout: RESUBMIT_STUCK_TIMEOUT, flashblocksPreconfirmationTime: undefined, + maxStuckAttemptsBeforeRotation: 3, + maxBundlingGasPrice: undefined, // Keeps getBaseFee() off the network. legacyTransactions: true, logLevel: "info", executorLogLevel: "info", getLogger: () => noopLogger(), publicClient: { - watchBlocks: (args: { - onBlock: (block: unknown) => Promise - }) => { - onBlock = args.onBlock - return unwatch - }, + watchBlocks, // Only cost metrics reach this, and they swallow failures. getTransactionReceipt: vi .fn() @@ -93,19 +105,26 @@ const createHarness = () => { maxPriorityFeePerGas: 1n }) } as any, - senderManager: {} as any, + senderManager: { getAllWallets: () => [] } as any, bundleManager: bundleManager as any }) + // Stops at the replacement decision: everything past it needs a real + // executor and store. + const replaceTransaction = vi + .spyOn(executorManager as any, "replaceTransaction") + .mockImplementation(() => undefined) + return { executorManager, getBundleStatuses, processIncludedBundle, - submittedBundle, + replaceTransaction, + watchBlocks, unwatch, getPending: () => pendingBundles, - emitBlock: async () => { - await onBlock?.({ number: 1n, baseFeePerGas: 1n }) + emitBlock: async (number = 1n) => { + await onBlock?.({ number, baseFeePerGas: 1n }) } } } @@ -123,8 +142,11 @@ describe("ExecutorManager stale block watchdog", () => { return () => vi.useRealTimers() }) - it("reconciles pending bundles when no new block arrives", async () => { - const { executorManager, getBundleStatuses } = createHarness() + it("replaces a stuck bundle when no new block arrives", async () => { + const { executorManager, getBundleStatuses, replaceTransaction } = + createHarness() + + getBundleStatuses.mockResolvedValue([{ status: "not_found" }]) executorManager.startWatchingBlocks() @@ -133,7 +155,9 @@ describe("ExecutorManager stale block watchdog", () => { await vi.advanceTimersByTimeAsync(RESUBMIT_STUCK_TIMEOUT + BLOCK_TIME) - expect(getBundleStatuses).toHaveBeenCalled() + expect(replaceTransaction).toHaveBeenCalledWith( + expect.objectContaining({ reason: "stuck" }) + ) }) it("releases a bundle that lands while blocks are stalled", async () => { @@ -141,7 +165,6 @@ describe("ExecutorManager stale block watchdog", () => { executorManager, getBundleStatuses, processIncludedBundle, - submittedBundle, emitBlock, getPending } = createHarness() @@ -152,7 +175,6 @@ describe("ExecutorManager stale block watchdog", () => { .mockResolvedValueOnce([includedStatus]) executorManager.startWatchingBlocks() - await emitBlock() expect(processIncludedBundle).not.toHaveBeenCalled() @@ -161,10 +183,7 @@ describe("ExecutorManager stale block watchdog", () => { // Only the watchdog can see the inclusion now. await vi.advanceTimersByTimeAsync(RESUBMIT_STUCK_TIMEOUT) - expect(processIncludedBundle).toHaveBeenCalledWith( - expect.objectContaining({ submittedBundle }) - ) - // Left the pending set -> wallet released. + expect(processIncludedBundle).toHaveBeenCalled() expect(getPending()).toHaveLength(0) }) @@ -175,17 +194,66 @@ describe("ExecutorManager stale block watchdog", () => { executorManager.startWatchingBlocks() // Fresh lastReconcileAt -> watchdog adds no reconciles of its own. - for (let i = 0; i < RESUBMIT_STUCK_TIMEOUT / BLOCK_TIME + 2; i++) { - await emitBlock() + const ticks = RESUBMIT_STUCK_TIMEOUT / BLOCK_TIME + 2 + for (let i = 0; i < ticks; i++) { + await emitBlock(BigInt(i)) await vi.advanceTimersByTimeAsync(BLOCK_TIME) } - expect(getBundleStatuses).toHaveBeenCalledTimes( - RESUBMIT_STUCK_TIMEOUT / BLOCK_TIME + 2 - ) + expect(getBundleStatuses).toHaveBeenCalledTimes(ticks) }) - it("keeps reconciling after a failed tick", async () => { + it("lets a block just before the deadline push it out", async () => { + const { executorManager, getBundleStatuses, emitBlock } = + createHarness() + + executorManager.startWatchingBlocks() + + await vi.advanceTimersByTimeAsync(RESUBMIT_STUCK_TIMEOUT - BLOCK_TIME) + expect(getBundleStatuses).not.toHaveBeenCalled() + + await emitBlock() + expect(getBundleStatuses).toHaveBeenCalledTimes(1) + + // Deadline moved with the block, so no watchdog reconcile yet. + await vi.advanceTimersByTimeAsync(RESUBMIT_STUCK_TIMEOUT - BLOCK_TIME) + expect(getBundleStatuses).toHaveBeenCalledTimes(1) + }) + + it("treats elapsed exactly equal to the timeout as stuck", async () => { + const { executorManager, getBundleStatuses, replaceTransaction } = + createHarness([createBundle({ lastReplaced: Date.now() })]) + + getBundleStatuses.mockResolvedValue([{ status: "not_found" }]) + + executorManager.startWatchingBlocks() + + // lastReconcileAt and lastReplaced are seeded from the same + // Date.now(), so the first tick past the watchdog deadline has + // elapsed == timeout exactly. `>` skips it, and the reconcile it + // performs resets lastReconcileAt, pushing the retry a full window + // out rather than losing it. + await vi.advanceTimersByTimeAsync(RESUBMIT_STUCK_TIMEOUT) + + expect(replaceTransaction).toHaveBeenCalled() + }) + + it("leaves a bundle one ms short of the timeout alone", async () => { + const { executorManager, getBundleStatuses, replaceTransaction } = + createHarness([createBundle({ lastReplaced: Date.now() + 1 })]) + + getBundleStatuses.mockResolvedValue([{ status: "not_found" }]) + + executorManager.startWatchingBlocks() + + // Watchdog fires (its own deadline is met) but elapsed is timeout - 1. + await vi.advanceTimersByTimeAsync(RESUBMIT_STUCK_TIMEOUT) + + expect(getBundleStatuses).toHaveBeenCalled() + expect(replaceTransaction).not.toHaveBeenCalled() + }) + + it("keeps reconciling after a failed block tick", async () => { const { executorManager, getBundleStatuses, emitBlock } = createHarness() @@ -200,6 +268,93 @@ describe("ExecutorManager stale block watchdog", () => { expect(getBundleStatuses).toHaveBeenCalledTimes(2) }) + it("keeps reconciling after a failed watchdog tick", async () => { + const { executorManager, getBundleStatuses } = createHarness() + + getBundleStatuses.mockRejectedValueOnce(new Error("store unavailable")) + + executorManager.startWatchingBlocks() + + await vi.advanceTimersByTimeAsync(RESUBMIT_STUCK_TIMEOUT + BLOCK_TIME) + expect(getBundleStatuses).toHaveBeenCalledTimes(1) + + // The rejection is caught inside the timer, so the guard must clear. + await vi.advanceTimersByTimeAsync(RESUBMIT_STUCK_TIMEOUT + BLOCK_TIME) + expect(getBundleStatuses).toHaveBeenCalledTimes(2) + }) + + it("runs one reconcile when a block lands mid watchdog tick", async () => { + const { executorManager, getBundleStatuses, emitBlock } = + createHarness() + + // Never settles, so the watchdog's reconcile is still in flight. + getBundleStatuses.mockReturnValue(new Promise(() => undefined)) + + executorManager.startWatchingBlocks() + + await vi.advanceTimersByTimeAsync(RESUBMIT_STUCK_TIMEOUT + BLOCK_TIME) + expect(getBundleStatuses).toHaveBeenCalledTimes(1) + + await emitBlock() + + // currentlyHandlingBlock must gate the overlapping tick. + expect(getBundleStatuses).toHaveBeenCalledTimes(1) + }) + + it("handles a mixed batch of pending bundles", async () => { + const included = createBundle({ uid: "0xincluded" }) + const stuck = createBundle({ uid: "0xstuck" }) + + const { + executorManager, + getBundleStatuses, + processIncludedBundle, + replaceTransaction, + getPending + } = createHarness([included, stuck]) + + // Index-aligned with getPendingBundles(). + getBundleStatuses.mockResolvedValue([ + includedStatus, + { status: "not_found" } + ]) + + executorManager.startWatchingBlocks() + await vi.advanceTimersByTimeAsync(RESUBMIT_STUCK_TIMEOUT + BLOCK_TIME) + + expect(processIncludedBundle).toHaveBeenCalledWith( + expect.objectContaining({ submittedBundle: included }) + ) + expect(replaceTransaction).toHaveBeenCalledWith( + expect.objectContaining({ submittedBundle: stuck }) + ) + expect(getPending()).toEqual([stuck]) + }) + + it("stops watching once the last pending bundle is resolved", async () => { + const { executorManager, getBundleStatuses, unwatch, getPending } = + createHarness() + + getBundleStatuses.mockResolvedValueOnce([includedStatus]) + + executorManager.startWatchingBlocks() + + // First window reconciles and the bundle lands. + await vi.advanceTimersByTimeAsync(RESUBMIT_STUCK_TIMEOUT + BLOCK_TIME) + expect(getPending()).toHaveLength(0) + + const callsAfterDrain = getBundleStatuses.mock.calls.length + + // That tick entered with a pending bundle, so the empty-set branch + // only runs a window later. + await vi.advanceTimersByTimeAsync(RESUBMIT_STUCK_TIMEOUT + BLOCK_TIME) + expect(unwatch).toHaveBeenCalled() + + // Timer cleared too, else it spins for the process lifetime. + await vi.advanceTimersByTimeAsync(RESUBMIT_STUCK_TIMEOUT * 3) + expect(getBundleStatuses.mock.calls).toHaveLength(callsAfterDrain) + }) + it("clears the watchdog when the watcher stops", async () => { const { executorManager, getBundleStatuses, unwatch } = createHarness() @@ -212,4 +367,21 @@ describe("ExecutorManager stale block watchdog", () => { expect(getBundleStatuses).not.toHaveBeenCalled() }) + + it("does not stack watchers or timers on restart", async () => { + const { executorManager, getBundleStatuses, watchBlocks } = + createHarness() + + executorManager.startWatchingBlocks() + executorManager.startWatchingBlocks() + expect(watchBlocks).toHaveBeenCalledTimes(1) + + executorManager.stopWatchingBlocks() + executorManager.startWatchingBlocks() + expect(watchBlocks).toHaveBeenCalledTimes(2) + + // Two timers would reconcile twice in one window. + await vi.advanceTimersByTimeAsync(RESUBMIT_STUCK_TIMEOUT + BLOCK_TIME) + expect(getBundleStatuses).toHaveBeenCalledTimes(1) + }) }) diff --git a/src/executor/executorManager.ts b/src/executor/executorManager.ts index 58cf0dab..d18b457c 100644 --- a/src/executor/executorManager.ts +++ b/src/executor/executorManager.ts @@ -251,9 +251,10 @@ export class ExecutorManager { const pendingBundles = this.bundleManager.getPendingBundles().length if (pendingBundles === 0) { - // Deliberately not stopWatchingBlocks(): re-arming a fresh - // watcher can miss the block the next submission triggers, so - // that bundle never reconciles and its wallet never frees. + // Nothing to reconcile: drop the watcher and this timer. + // Submission re-arms both, and a missed submission block is + // caught by the fresh watchdog. + this.stopWatchingBlocks() return } diff --git a/test/e2e/tests/eth_sendUserOperation.test.ts b/test/e2e/tests/eth_sendUserOperation.test.ts index 54966935..f3875cc2 100644 --- a/test/e2e/tests/eth_sendUserOperation.test.ts +++ b/test/e2e/tests/eth_sendUserOperation.test.ts @@ -276,7 +276,7 @@ describe.each([ privateKey }) - await client.sendUserOperation({ + const deployHash = await client.sendUserOperation({ calls: [ { to: client.account.address, @@ -287,6 +287,9 @@ describe.each([ }) await sendBundleNow({ altoRpc }) + // Ops below are prepared against the deployed account, so the + // factory args must be gone before they are built, else AA10. + await client.waitForUserOperationReceipt({ hash: deployHash }) const opHashes = await Promise.all( nonceKeys.map((nonceKey) => @@ -372,7 +375,7 @@ describe.each([ privateKey }) - await client.sendUserOperation({ + const deployHash = await client.sendUserOperation({ calls: [ { to: client.account.address, @@ -383,6 +386,9 @@ describe.each([ }) await sendBundleNow({ altoRpc }) + // Ops below are prepared against the deployed account, so the + // factory args must be gone before they are built, else AA10. + await client.waitForUserOperationReceipt({ hash: deployHash }) const nonceKey = 100n const nonceValueDiffs = [0n, 1n, 2n] From 65f3b976a75d9cc03f397586df06f8db499cfd5b Mon Sep 17 00:00:00 2001 From: jayeshbhole <54071350+jayeshbhole@users.noreply.github.com> Date: Mon, 7 Sep 2026 14:00:43 +0000 Subject: [PATCH 7/7] chore: format --- src/executor/executorManager.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/executor/executorManager.test.ts b/src/executor/executorManager.test.ts index 0778f406..aa67f779 100644 --- a/src/executor/executorManager.test.ts +++ b/src/executor/executorManager.test.ts @@ -51,9 +51,7 @@ const createHarness = (bundles: unknown[] = [createBundle()]) => { const getBundleStatuses = vi.fn().mockResolvedValue([]) const processIncludedBundle = vi.fn( ({ submittedBundle }: { submittedBundle: unknown }) => { - pendingBundles = pendingBundles.filter( - (b) => b !== submittedBundle - ) + pendingBundles = pendingBundles.filter((b) => b !== submittedBundle) } )