op-batcher: bound every Espresso network call and drop the streamer on stop

[philippecamacho]

Closes #493. Stacked on #459 (espresso/batcher).

Problem

Three wedge modes, all the same family: a hung network call with no deadline parks a component forever.

1. Submit/verify workers: the Espresso SDK issues plain HTTP requests with no client-side timeout, and the workers called it with their long-lived loop contexts. One black-holed connection consumed a worker permanently; with all four wedged, submission stayed stopped even after the endpoint recovered.
2. Loading loop: Peek retries undecided batches' L1-backed validity checks (a contract call plus a header fetch) on the loop's shutdown context, so a hung L1 RPC silently stalled frame publication with no error logs.
3. Stop/start: l.espressoStreamer was never nil'ed, so the next StartBatchSubmitting ran clearState against the stale streamer, whose re-anchor gate retries every 5s with no deadline while holding the start mutex — the very mutex the stop that could cancel it needs. Against a resyncing op-node, a later SIGTERM blocked forever (the same start-mutex-hang pattern already fixed for waitForLocalSafeHead and registration).

Fix

Per the issue's suggestion, the SDK timeout is enforced once at a client-wrapper seam rather than per call site:

• The submitter's client dependency is narrowed to the three-method EspressoSubmitClient (the SDK's full interface includes streaming endpoints, which a per-call deadline would break), and the driver hands it a boundedEspressoClient wrapping every call with NetworkTimeout. This covers the block-height tracker too.
• Peek gets the same per-call bound as every other raw RPC on this path (via the existing networkTimeoutCtx); on expiry the batch stays undecided and is retried next tick, and the streamer's own warn logs now surface instead of silence.
• StopBatchSubmitting and rollbackFailedStart drop the stopped streamer, so every start reaches clearState streamer-free and the no-deadline re-anchor gate only ever runs from the loading loop, whose context a stop can cancel without taking the mutex.

Tests

• TestBoundedEspressoClientAppliesDeadline: a blocking fake (models the timeout-less SDK on a black-holed connection) called with a deadline-free parent context — each of the three wrapped methods must return DeadlineExceeded promptly.
• TestStopBatchSubmittingDropsStreamer / TestRollbackFailedStartDropsStreamer: both teardown paths must leave espressoStreamer nil (a zero-value Streamer is Stop()-safe, which makes the lifecycle directly unit-testable).
• The Peek bound is a context wrap with no seam a unit test can observe; it rides on the streamer's existing undecided-retry semantics.

go vet and the full op-batcher/batcher suite pass.

🤖 Generated with Claude Code

---

• To see the specific tasks where the Asana app for GitHub is being used, see below:
  • https://app.asana.com/0/0/1217525737014025

[palango]

The mechanism checks out. I put the real SDK behind a listener that accepts TCP and never answers, called it through a 500ms bound on a deadline-free parent, and every method came back on time: both the GETs (client.go:357) and the submit POST (query_submitter.go:63) go through http.NewRequestWithContext. Modes 1 and 2 are gone.

It's where the bound sits that I'd change. l.Espresso.Client is a MultipleNodesClient (espresso_driver.go:32), and NewMultipleNodesClient refuses fewer than two URLs, so multi-node is the shape we deploy. SubmitTransaction walks the nodes one at a time on a single ctx (multiple_nodes_client.go:156-177), and the wrapper puts one 10s budget around the whole walk. Black-hole node[0] and it eats all ten seconds; node[1] gets an expired context and fails instantly, every retry the same. My two-node probe against two black-holes came back at 501ms having "tried" both. The worker survives, which was the point, but the reason the multi-node client exists is bounded away. Either wrap each *Client, or size the wrapper NetworkTimeout * len(QueryServiceURLs).

I also don't think this closes #493. That wedge isn't the streamer, it's clearState under the start mutex: StartBatchSubmitting holds l.mutex from driver.go:200 across the clearState call at :206, and that loop only exits on success or on a context nothing but StopBatchSubmitting can cancel. Nil-ing the streamer removes the espressoReanchorTarget failure branch and leaves safeL1Origin (:890-893), so an op-node restart during admin_startBatcher still deadlocks until SIGKILL — the likelier trigger of the two. Upstream's branch, not yours, but teardownEspressoStreamer's comment reads like the class is closed, and #493 should stay open with the leftover written down.

Smaller stuff:

1. Nothing tests the wiring. Reverting newBoundedEspressoClient at its one call site (espresso_driver.go:285) leaves the package green. Better to make it unavoidable than to test it: wrap inside NewEspressoTransactionSubmitter.
2. The blocking fake waits on <-ctx.Done() under context.Background(), so a lost bound gives panic: test timed out and takes the test binary with it rather than naming the failure. select against time.After(time.Second).
3. Only the nil is pinned, not the Stop() — delete the Stop() call and both tests still pass, because the zero-value Streamer{} makes it a no-op. That hides a leaked poller set per stop/start cycle.
4. I don't buy that Peek has no seam. Five methods on that field, and the only other consumer (espresso_service.go:60) has no callers. Make it an interface and a twelve-line fake pins the deadline; that accessor also reads without the mutex and now returns nil after a stop, so I'd delete it.
5. A timing-out submit is invisible: the SDK wraps ctx failures as ErrEphemeral, so evaluateSubmission (espresso.go:352-357) skips its Warn and retries silently, and RecordFallbackAuthWindowExceeded is the only Espresso entry in Metricer. l.degradedLog is right there.

Two I chased and left: a re-sent submit is deduped by EspressoBatch.Hash() in batchStore.insert (worth a line in the PR body, the argument lives in a vendored dep), and Peek under a deadline maps to BatchUndecided and never BatchDrop, so nothing is lost.

[philippecamacho]

@palango

It's where the bound sits that I'd change. l.Espresso.Client is a MultipleNodesClient (espresso_driver.go:32), and NewMultipleNodesClient refuses fewer than two URLs, so multi-node is the shape we deploy. SubmitTransaction walks the nodes one at a time on a single ctx (multiple_nodes_client.go:156-177), and the wrapper puts one 10s budget around the whole walk. Black-hole node[0] and it eats all ten seconds; node[1] gets an expired context and fails instantly, every retry the same. My two-node probe against two black-holes came back at 501ms having "tried" both. The worker survives, which was the point, but the reason the multi-node client exists is bounded away. Either wrap each *Client, or size the wrapper NetworkTimeout * len(QueryServiceURLs).

I agree with the problem. Now the solution proposed is likely not enough as if the first node is failing it will eat up the whole time NetworkTimeout * len(QueryServiceURLs). So I would suggest to assign a timeout to each client.
It seems to me that the cleaner way to achieve this goal would be to make a change in the Espresso Go SDK as currently the http client used has its timeout set to 0.

Every HTTP path in sdks/go/client uses http.DefaultClient, which has Timeout: 0:

client.go:75 (NewClientFromOptions) and client.go:88 (NewClient)
query_submitter.go:25 (NewQuerySubmitter)
builder_submitter.go:34 (NewBuilderSubmitter, a []*http.Client whose entries are all the same global)

WDYT @jjeangal?

[jjeangal]

@palango

It's where the bound sits that I'd change. l.Espresso.Client is a MultipleNodesClient (espresso_driver.go:32), and NewMultipleNodesClient refuses fewer than two URLs, so multi-node is the shape we deploy. SubmitTransaction walks the nodes one at a time on a single ctx (multiple_nodes_client.go:156-177), and the wrapper puts one 10s budget around the whole walk. Black-hole node[0] and it eats all ten seconds; node[1] gets an expired context and fails instantly, every retry the same. My two-node probe against two black-holes came back at 501ms having "tried" both. The worker survives, which was the point, but the reason the multi-node client exists is bounded away. Either wrap each *Client, or size the wrapper NetworkTimeout * len(QueryServiceURLs).

I agree with the problem. Now the solution proposed is likely not enough as if the first node is failing it will eat up the whole time NetworkTimeout * len(QueryServiceURLs). So I would suggest to assign a timeout to each client. It seems to me that the cleaner way to achieve this goal would be to make a change in the Espresso Go SDK as currently the http client used has its timeout set to 0.

Every HTTP path in sdks/go/client uses http.DefaultClient, which has Timeout: 0:

client.go:75 (NewClientFromOptions) and client.go:88 (NewClient) query_submitter.go:25 (NewQuerySubmitter) builder_submitter.go:34 (NewBuilderSubmitter, a []*http.Client whose entries are all the same global)

WDYT @jjeangal?

@philippecamacho agreed, the SDK is the right place. One addition though, a client timeout alone doesn't fix what @palango raised. SubmitTransaction tries the nodes one after another with the same context, so a dead first node still burns everything before the second one is tried. The SDK change should also give each node its own share of the time, inside MultipleNodesClient.