[ndr-ds]

Motivation

A ChainClient listener repairs its validator notification streams (and runs the
circuit-breaker probes added in #5717) only inside update_notification_streams, which
ran exactly twice: at listen() startup and on the chain's own NewBlock notifications.
New blocks are heard through the very streams being repaired, so for a low-activity chain
this is circular: once its last stream dies during an idle stretch, the breaker records
next_probe_in=300s — and that probe can never execute, because the deadline is only
checked when the function runs and nothing is left to wake it. The chain stays deaf
until the process restarts.

This is not theoretical. On 2026-08-12 every PM worker's control chain was deaf to the
controller's UpdateService commands, fleet-wide. Worker logs show the arc: streams to
individual validators died on 07-30/07-31/08-01 (routine churn, each logged Validator notification stream ended, no repair possible), a block on 08-06 ran the update once and
armed breakers with next_probe_in=300s, then six days of silence — zero probes, zero
recoveries, deaf throughout. Restarting the worker "fixed" it by rebuilding the listener;
the bundles had been sitting on the validators all along.

Proposal

Give the repair path wake-up sources that do not depend on the chain's own traffic, and
judge validator health on evidence rather than on elapsed time.

The loop. listen() becomes a select! over notification arrival (unchanged
behaviour, including re-subscribing on NewBlock for committee changes), an individual
stream task finishing, and clock().sleep_until(earliest deadline). The timer is held
across iterations and rebuilt only when the deadline moves, or after it fires — it is a
Fuse, and select! skips a terminated arm in silence, so leaving it in place when the
recomputed deadline happens to be unchanged would cost the listener its only wake-up.
update_notification_streams returns its per-validator tasks individually rather than one
collect()ed composite, so a single stream death is observable — the composite only
completed once every stream in a batch had died. A stream that ends during an update
is still registered by that same update: await_while_polling drains the tasks
concurrently, and the update's only two await points (local_committee, admin_committee)
both precede its transition scan.

Health is judged on whether the stream reached the point of SERVING notifications.
The probe body is subscribe, then an initial sync, and nothing polls the notification
stream until that sync returns — so neither "the task is alive" nor "subscribe resolved"
means the validator is delivering anything. StreamHandle records the instant both
completed, and the transitions key off it:

stream state	action
serving, alive	recovered immediately — no interval to wait out
never served, deadline passed	stuck connecting or syncing: abort it and escalate
died after serving ≥ one interval	churn — re-arm at the initial interval
died after serving < one interval	persistent fault — escalate
died without ever serving	unreachable — escalate

The lifetime threshold is the correction to an earlier revision of this PR, which treated
every post-subscribe death as churn. That is wrong for the production transport:
GrpcClient::subscribe connects before returning and then reconnects internally with
jittered backoff, and is_retryable already covers Unavailable, Aborted,
DeadlineExceeded, Unknown and Internal for h2 protocol error / 502 Bad Gateway.
Routine churn is absorbed below this layer and never reaches the breaker, so a stream that
dies here has either exhausted its retries or hit a non-retryable status — for instance a
single undeserializable notification, which maps to Code::Internal and is not retryable.
Resetting those to the base cadence would re-probe a permanently broken validator every
300s forever, each probe a subscribe plus a full synchronize_chain_state_from.

Every launch arms a deadline, including a validator's first. Otherwise a subscribe
or sync that never returns leaves the senders entry occupied with nothing to abort it
and nothing to wake the timer on its account, and the Entry::Vacant guard skips that
validator for the life of the process. Recovery drops the breaker as soon as it serves.

Because every launch now arms one, breaker presence no longer distinguishes "this
stream has failed" from "this stream is new", so CircuitBreakerState carries that
explicitly. Only a breaker that actually tripped reports a recovery — gating that log on
the removal alone announced one for every healthy validator on its first update after it
started serving (measured: four per chain against a four-validator committee, in the
feature whose purpose is making silent deafness observable). And only a tripped breaker
escalates: charging a stream's first failure as a failed probe started the ladder at twice
the configured interval and made the error! that records a lost stream unreachable
(measured: a first death re-probed at 600 s on a 300 s setting, with zero occurrences of
that error line across the probe tests).

Failures arm a retry deadline, including at startup. Nothing retries listen() on
failure, so a transient committee-read failure there would otherwise leave no streams, no
breakers and nothing to wake the loop. A failed update returns before touching any
breaker, so defer_due_probes pushes out the deadlines that are already due — those
are what would keep winning min() and re-fire the timer into the same failing call.
Only those: deferring a probe that is legitimately due sooner would make taking the min
over the two deadlines a no-op.

Probes are spread per validator. next_probe_at carries a stable offset derived from
(chain_id, validator), up to an eighth of the interval. A fleet-wide stream loss would
otherwise re-converge every chain's probes on the same instant at every doubling, and
for a worker with thousands of chains each probe is a subscribe plus a full sync. The
offset is hashed rather than random so the timing tests stay deterministic.

The interval flags are validated where they are parsed. Both now reject 0 via
parse_millis_nonzero; MIN_PROBE_INTERVAL is a 1 s backstop for programmatic Options
rather than a guard against arithmetic, and the maximum is floored by the initial
interval so a 0 maximum cannot pin both ends together and delete escalation.

Boxed futures across the crate now share one MaybeSendBoxFuture<'a, T>, a cfg'd
re-export of BoxFuture/LocalBoxFuture, replacing three hand-written copies of the same
pair.

Test Plan

Each mutation below was applied to the source, the tests run against it, and the file
restored — so every cell is measured, not argued. Every mutation is killed. The bounds are
deliberately exact, or two-sided, rather than one-sided. assert!(interval > previous) is
also satisfied by jumping straight to the cap, and assert!(deadline > now) by any delay
at all — either would turn one transient failure into an hour of deafness while the suite
stayed green. Probe delays are bounded >= interval && <= interval * 9 / 8, which is what
PROBE_SPREAD_DIVISOR promises, and the spread itself is pinned by asserting that
validators launched together do not share one deadline.

mutation	repairs	backoff	in-flight	transitions	failed-update	retry-pacing	no-breakers	first-launch	stalled-sync	defer-due	parks
listener never wakes on a probe deadline	FAIL	FAIL	pass	pass	FAIL	FAIL	FAIL	pass	pass	pass	pass
delete the probe-launch re-arm	FAIL	pass	FAIL	pass	pass	pass	pass	pass	pass	pass	pass
delete the now < next_probe_at skip	pass	FAIL	pass	pass	pass	pass	pass	pass	pass	pass	pass
recovery gated on an elapsed deadline again	pass	pass	pass	FAIL	pass	pass	pass	pass	pass	pass	pass
recovery keyed on liveness, not on serving	pass	pass	FAIL	FAIL	pass	pass	pass	pass	FAIL	pass	pass
in-loop Err arm does not arm the retry deadline	pass	pass	pass	pass	pass	pass	FAIL	pass	pass	pass	pass
stalled probe never aborted	pass	pass	pass	FAIL	pass	pass	pass	pass	pass	pass	pass
a short-lived stream is treated as churn	pass	pass	pass	FAIL	pass	pass	pass	pass	pass	pass	pass
serving recorded before the sync instead of after	pass	pass	pass	pass	pass	pass	pass	pass	FAIL	pass	pass
first launch arms no breaker	pass	pass	pass	pass	pass	pass	pass	FAIL	FAIL	pass	pass
a launch-armed breaker counts as tripped	pass	pass	pass	pass	pass	pass	pass	FAIL	FAIL	pass	pass
a first stream death escalates off the launch breaker	pass	pass	pass	FAIL	pass	pass	pass	pass	pass	pass	pass
a first stall escalates off the launch breaker	pass	pass	pass	pass	pass	pass	pass	pass	FAIL	pass	pass
escalate jumps straight to the cap instead of doubling	pass	pass	pass	FAIL	pass	pass	pass	pass	pass	pass	pass
escalate triples instead of doubling	pass	pass	pass	FAIL	pass	pass	pass	pass	pass	pass	pass
churn re-arms at the 1s floor instead of the initial interval	pass	pass	pass	FAIL	pass	pass	pass	pass	pass	pass	pass
arm() re-arms at twice the interval	pass	pass	FAIL	pass	pass	pass	pass	FAIL	pass	pass	pass
first launch arms at the MAX interval, not the initial one	pass	pass	pass	pass	pass	pass	pass	FAIL	pass	pass	pass
probe_spread always returns zero	pass	pass	pass	pass	pass	pass	pass	FAIL	pass	pass	pass
a successful update never clears the retry deadline	pass	pass	pass	pass	pass	pass	pass	pass	pass	pass	FAIL
escalate has no cap	pass	pass	pass	FAIL	pass	pass	pass	pass	pass	pass	pass
startup Err arm does not arm the retry deadline	pass	pass	pass	pass	FAIL	pass	pass	pass	pass	pass	FAIL
clamp raises every breaker to the retry deadline	pass	pass	pass	pass	pass	pass	pass	pass	pass	FAIL	pass

• test_probe_timer_repairs_severed_notification_streams — an independent client (own
  local node, so validator streams are its only signal) listens to an idle chain; a
  test_utils hook severs every stream server-side. Two sever/repair rounds, each
  requiring a new subscribe on a healthy validator, so neither a notification still
  in flight from the previous round nor the faulty validator's own churn can satisfy it.
• test_probe_backoff_escalates_against_a_down_validator — validator 0 Offline; walks
  two simulated hours and bounds the probe count by the cumulative schedule.
• test_in_flight_probe_is_not_treated_as_recovered — drives the update directly without
  polling the tasks, so the probe stays in flight.
• test_breaker_transitions_key_off_serving_not_liveness — the transition table above,
  deterministically, including a short-lived stream escalating rather than resetting.
• test_a_first_launch_arms_a_probe_deadline — a launch with no prior breaker must still
  arm one, and that breaker must not count as tripped.
• test_a_stalled_sync_is_not_mistaken_for_a_serving_stream — holds every chain-info
  query at one validator so its probe clears subscribe and stops inside the sync, then
  asserts it is not recorded as recovered.
• test_failed_update_still_leaves_a_timer_armed,
  test_a_failing_update_is_paced_by_the_retry_deadline, and
  test_a_failed_update_with_no_breakers_still_arms_the_retry — the real failure path via
  fail_next_validator_set_builds, at startup, on a probe wake, and with an empty breaker
  map. The last is the incident's own shape and the only one that covers the in-loop
  arming; the first asserts the listener consumed the whole injected budget, so the
  failures cannot have been spent elsewhere.
• test_a_failed_update_defers_only_the_due_probes — an already-due probe is deferred, a
  probe due before the retry deadline is not.
• test_a_recovered_listener_parks_instead_of_spinning — fails one update, lets the retry
  succeed, then freezes the clock and bounds how often the listener rebuilds the validator
  set. A retry deadline left behind by the earlier failure would win min() permanently
  and re-fire on an elapsed deadline every pass. It is entered from the RECOVERED state on
  purpose: with failures still armed, each one re-arms the deadline into the future and
  hides the spin.

Test-harness changes: the injected validator-set failures are now private to the chain
client, since the validators' own export tasks resolve each other through a provider too
and could otherwise consume them; do_subscribe honours Offline | OfflineWithInfo; a
subscribe_calls counter and a wait_until barrier replace fixed sleeps where an
assertion depends on the listener having run; and a dropped notification stream now marks
its abort handle, so the handle vec prunes the real set rather than only severed streams.

• cargo test -p linera-core --lib: 206 passed, 0 failed.
• cargo clippy --locked --all-targets --all-features --workspace --exclude linera-web,
  -p linera-core --no-default-features, and
  --target wasm32-unknown-unknown --no-default-features --features web-default -p linera-client:
  clean.
• linera-bridge/tests/e2e (separate workspace) cargo check --all-targets: clean.
• Nightly cargo fmt -- --check: clean.

Release Plan

• These changes should be backported to the latest testnet branch — the PM workers
  (the fleet this incident hit) build from testnet_conway.

Links

• reviewer checklist

[ma2bd]

Reviewed the listener loop and the circuit-breaker state machine. The diagnosis is right and the timer is the right fix — next_probe_at finally having a reader closes the circular dependency where repair was gated on a signal that can only arrive over the streams being repaired.

Findings inline, roughly in severity order. The three I would want resolved before this merges:

1. The startup path still has the original bug (chain_client/mod.rs:3277) — a failed committee read at listen() leaves zero streams, zero breakers and no armed timer, and ChainListener never calls listen() again. Same absorbing state, reached from a different direction.
2. Recovery is inferred from the probe task not having died (:3453) — a slow synchronize_chain_state_from is declared recovered and has its backoff wiped; one that hangs forever leaves the senders entry Occupied and the breaker deleted, i.e. permanently unprobed.
3. The backoff only ever escalates unless a full interval elapses cleanly (:3450), so a validator behind a proxy with a ~30 min idle-stream timeout ratchets to the 1h cap and stays there.

Plus one behaviour change worth flagging despite the deliberate decision not to touch the flag: --notification-circuit-breaker-initial-probe-interval-ms=0 now spins rather than merely disabling the backoff (:3307).

The rest are smaller — a masked deadline, sync failures counted as connect failures, two test-determinism points, and some nits.

[ma2bd]

The startup update still only logs its error, so the listener can begin life permanently deaf.

If this call fails — a transient storage error in local_committee() / admin_committee() — senders and circuit_breakers stay empty, process_notifications is empty, and retry_update_at is initialised to None just below (3291). So next_probe_at is None, probe_due is future::pending(), and the only live select! branch is abortable_notifications — which can never yield a NewBlock, because there are no validator streams to carry one.

ChainListener calls listen() once per chain and never retries (linera-client/src/chain_listener.rs:541), so that chain stays deaf until the process restarts: the same absorbing state as the incident this PR fixes, reached from a different direction.

Arming retry_update_at here, the way the in-loop Err branch now does, closes it.

[ma2bd]

This infers health from "the probe task has not died within one interval", which is not the same as "a stream was established".

The probe body is stream::once(async { node.subscribe(..).await?; .. synchronize_chain_state_from(..) }). synchronize_chain_state_from has no overall deadline and calls download_certificates_from; for a chain that is far behind, exceeding a 300s probe interval is routine. When that happens the probe timer fires, the sender is still non-aborted (the task simply has not finished), so the breaker is dropped and the accumulated backoff reset — logging Validator recovered from circuit breaker for a validator that has never delivered a notification.

The worse tail: if subscribe/sync hangs indefinitely, the task never ends, the senders entry stays Occupied forever, the Entry::Vacant guard below skips it on every future update, and with the breaker now deleted there is no probe deadline left either. That validator is silently and permanently unreachable — the exact failure class this PR is closing.

Deriving recovery from the stream actually yielding (or at least from subscribe having resolved) rather than from the task still being alive would avoid both.

[ma2bd]

Because recovery now requires a full elapsed interval, any stream that dies sooner than the current interval escalates the backoff — so a healthy-but-churny validator ratchets to the cap and never escapes.

Consider a proxy or load balancer with a ~30 min idle-stream timeout, i.e. the same routine churn that produced the Validator notification stream ended lines in the incident. Probe at T succeeds, next_probe_at = T + I, stream dies at T + 30min. Once I has grown past 30 min, recovery is never declared, so the death lands in the escalation branch and doubles I instead of resetting it. Repeat, and probe_interval pins at max_probe_interval (1h) permanently: a fundamentally healthy validator is re-subscribed at most once an hour, and the client is deaf to it for up to 59 minutes after every routine reset.

The coupling is that recovery means "survived an interval" rather than "delivered something"; resetting probe_interval once a subscription actually produces a notification would break the ratchet.

[ma2bd]

With --notification-circuit-breaker-initial-probe-interval-ms=0 this becomes a CPU-bound busy loop, which is a regression over main.

I read the note about leaving the flag unclamped for consistency with --retry-delay-ms / --max-backoff-ms, and I agree the flag itself should not change here — but the meaning of 0 does change, and not in the "disable the throttle" direction the siblings have.

The breaker is created with probe_interval = 0, next_probe_at = now; sleep_until(now) returns immediately; now < next_probe_at is false so a probe launches; the re-arm sets next_probe_at = now + 0 = now; and escalation computes (ZERO * 2).min(max) == ZERO forever. The outer loop then spins at CPU speed, and the inner loop never satisfies !senders.values().any(is_aborted) for a down validator, so it hammers subscribe + synchronize_chain_state_from continuously.

On main, 0 merely meant "probe on every NewBlock" — bounded by chain activity. No caller sets it today, so this is not urgent, but a floor applied where the timer consumes the interval would keep 0 meaning "no backoff" instead of "spin", without touching the flag or its parser.

[ma2bd]

or_else masks the breaker deadlines rather than taking the minimum of both, so one failed committee read postpones every already-armed probe.

If a breaker has next_probe_at = now + 10s and an unrelated transient storage failure sets retry_update_at = now + 300s, Option::or_else short-circuits: the loop sleeps 300s and the probe that was due in 10s is 290s late. The comment argues probes cannot run while updates are failing, but the two are not causally linked — the retry deadline is set by one failed attempt, and the next attempt may well succeed.

min(retry_update_at, breaker deadlines) is strictly correct and lets the comment go away. Separately: retry_update_at re-arms at a flat initial_probe_interval on every failure, so unlike the probes it is masking, it never backs off.

[ma2bd]

notification_stream_aborts only ever grows: do_subscribe pushes (561) and nothing pops except disconnect_notification_subscribers. Any test with an active circuit breaker re-subscribes repeatedly, so the Vec grows monotonically inside the mutex-guarded LocalValidator, alongside the ChannelNotifier subscription that each dead stream leaves registered. A retain(|a| !a.is_aborted()) on push bounds both.

[ma2bd]

The timer future is rebuilt on every iteration, including the very common path where a non-NewBlock notification arrives, update_now is false and we continue.

Under TestClock each abandoned sleep_until leaves an orphaned oneshot::Sender in TestClockInner::sleeps, reaped only when the clock advances past that timestamp — so a test with an armed breaker and heavy notification traffic accumulates entries in the shared map. Hoisting a single timer outside the loop and resetting it only when next_probe_at actually changes avoids both the churn and the orphans.

[ma2bd]

These comments record history rather than the final state of the code: the (E0275, seen in the bridge e2e workspace) here, the 2026-08-12 PM control-chain incident at client_tests.rs:163-166, and the mutation-testing narratives at client_tests.rs:269-273 and 5842-5850.

The mechanism halves are worth keeping ("boxed because the nested futures overflow the trait solver when proving Send"). The archaeology dates the code — once the trait-solver limit, the incident, or the mutation set moves, these read as current and nothing signals that they need updating. The PR description is the right home for it, and it is already there.

[ma2bd]

NotificationStreamTask is documented just above as "a boxed per-validator notification stream task", but here it is the type of the whole listener loop — which is neither per-validator nor a stream task.

Worth a second alias, or a neutral name, so that a later refactor narrowing this one (say, to carry the validator key alongside the future) does not have to untangle the two uses first.

[ma2bd]

This is the third copy of the cfg(web) / cfg(not(web)) boxed-future alias pair in this crate: BatchFuture (worker.rs:593-595) is byte-for-byte identical, and CertificateBatchFuture (client/mod.rs:685-690) is the same shape with a different output. A single crate-level MaybeSendBoxFuture<T> would replace all three and keep the Send policy in one place.

[ma2bd]

Re-reviewed at e390115d. All fourteen points from the first round are addressed, several more thoroughly than I asked — the (died, subscribed) split, the pinned timer with the completed-Fuse subtlety handled, the crate-level MaybeSendBoxFuture, and the fault injection that closes the "not covered by the harness" gap.

First, a correction to my own round-one review, which sent the (true, true) branch the wrong way. I claimed stream deaths are mostly routine churn — proxy idle timeouts, rolling restarts — and that escalating on them ratchets a healthy validator to the cap. That is not true of the production transport. GrpcClient::subscribe (linera-rpc/src/grpc/client.rs:336) awaits the first connection and then wraps the stream in an endlessly-retrying unfold that re-subscribes internally with jittered backoff, and is_retryable (:110) covers exactly the cases I named — Unavailable, Aborted, Unknown, and Internal for h2 protocol error / 502 Bad Gateway. Churn is absorbed below this layer and never reaches the breaker; a stream that ends after subscribe resolved has usually failed for good. So resetting it to the initial interval and downgrading error! to info! is backwards for the case that actually occurs. Details and a suggested signal that separates the two cases are inline at that arm.

The rest, in rough order of how much I would want them addressed:

1. (true, true) misclassification, above (:3512).
2. A sync that stalls after subscribe now reads as healthy, with the breaker removed and no timer left (:3605) — the half of my round-one point that the subscribed flag does not reach, since the sync still has no deadline.
3. The stalled-probe abort only fires when a breaker already exists, so a first subscribe that never resolves is never aborted or retried (:3491).
4. The clamp in the Err arm makes the or_else → min change a no-op, reintroducing the masking I flagged last time (:3392).
5. The 1ms floor does not do what its comment says, and flooring max by the same constant lets --...-max-probe-interval-ms 0 silently disable escalation (:162).
6. The failure injection is shared with the validators' export tasks, so all three new tests can pass without exercising the path they name (test_utils.rs:845).

Then jitter, the duplicated breaker arithmetic, two comment/doc inaccuracies (one of them inherited from my own wording), and test-timing notes.

[ma2bd]

This classification is inverted for the production transport — and that is my fault: the round-one comment that prompted it assumed stream deaths are mostly churn, which the gRPC client makes false.

GrpcClient::subscribe (linera-rpc/src/grpc/client.rs:336) makes the first connection attempt before returning, then returns a stream that keeps re-calling client.subscribe(..) internally, sleeping jittered_backoff_delay between attempts, until either retry_count >= max_retries or is_retryable(status) is false. is_retryable (:110) covers precisely the churn I described — Unavailable, Aborted, DeadlineExceeded, Unknown, plus Internal when the message contains h2 protocol error or 502 Bad Gateway, which is there specifically for proxy restarts and connection resets.

So (true, true) does not mean "transient drop". Reaching it means one of:

• the internal retry loop exhausted max_retries, each with backoff — a sustained outage, not churn; or
• a non-retryable status. The most likely one is self-inflicted: subscribe maps a notification it cannot deserialize to Code::Internal ("Could not deserialize notification"), which is not retryable, so a single version-skewed notification from a newer validator ends the stream immediately after a perfectly successful subscribe.

Both are persistent. Resetting to initial_probe_interval and lowering error! to info! means such a validator is re-probed every 300s forever — each probe a subscribe plus a full synchronize_chain_state_from — with no escalation and no error-level signal that anything is wrong. For the case that actually occurs in production, that is worse than the behaviour at 245e3e5.

What I was reaching for is a stream that worked and then stopped, and subscribe resolving is too weak a proxy for it. Stream lifetime separates the two, and unlike "delivered a notification" it still works for an idle chain: record the subscribe instant in StreamHandle, and on death reset to the initial interval only if the stream survived some threshold — the initial interval is a natural one — otherwise escalate. A stream that lived an hour and died is churn; one that dies seconds after subscribing is not.

Worth noting the floor this leaves either way: after the internal retries give up, the client waits a further initial_probe_interval before trying again, so a validator is unheard for at least 300s past the transport's own exhaustion. That is still a large improvement on unbounded, but it is not the "seconds" a proxy restart actually takes.

[ma2bd]

subscribed is set before the initial sync, but the notification stream is not read until that sync returns — so a stalled sync now reads as a healthy validator with the breaker removed and no timer left.

The task is stream::once(async { subscribe; store(true); synchronize_chain_state_from(..); Ok(stream) }), so nothing polls the notification stream until the sync completes. The doc on StreamHandle::subscribed says "a sync still running counts as subscribed — the stream does exist", which is true, but existence is not the property the breaker needs; it needs notifications to be flowing.

Concretely: a probe subscribes, then the sync stalls — it still has no overall deadline, and download_certificates_from against a far-behind chain or a contended chain-worker lock is enough. The next update sees (false, true), removes the breaker, and if that was the last one next_probe_at is None and the timer is pending indefinitely. The senders entry stays Occupied, so the Entry::Vacant guard skips the validator permanently. Across the committee that is the chain deaf until restart — the state this PR exists to remove.

This is the half of my round-one comment that the subscribed flag does not reach: the fix addressed a probe stuck in subscribe, but a probe stuck in the sync afterwards is now classified as healthy rather than merely unknown. Either bound the sync, or hold off recovery until the stream has actually yielded (or at least until the sync has returned).

[ma2bd]

The stalled-probe abort only fires when a breaker already exists, so a first subscription that never resolves is never aborted and never retried.

At listen() startup — and after any recovery, since (false, true) removes the breaker — a validator's entry is created with subscribed = false and no breaker. If that subscribe does not resolve, if let Some(state) = circuit_breakers.get_mut(validator) is a no-op here: nothing aborts the task, nothing escalates, and the validator contributes no deadline, so the timer never wakes on its account. The senders entry stays Occupied and the Entry::Vacant guard skips it for the life of the process.

That is the same stuck state this arm was added to prevent, one step earlier in the lifecycle. The gRPC path awaits its first connect and will normally surface an error rather than hang — but this arm exists precisely because "normally" is not the case worth defending against, and the same reasoning applies a level up.

Minor and adjacent: the scan reads subscribed and abort_stalled_probes acts after the loop ends, so a probe that resolves inside that window is aborted anyway and its interval doubled. The window is tiny, but replacing the bool with a small state enum and a compare-and-swap would close this and give the no-breaker case above somewhere natural to hang its deadline.

[ma2bd]

This clamp makes the or_else → min change a no-op: it reintroduces exactly the masking I flagged in round one.

Once every breaker below retry has been raised to retry, min(breakers ∪ {retry}) is retry — the same wake-up the old retry_update_at.or_else(..) produced. A breaker due in 10s when an unrelated committee read fails is still postponed by a full initial_probe_interval.

The comment above states the correct rule — "every deadline it would have advanced is still in the past" — but the code clamps on < retry instead of on that. The deadlines a failed update would have advanced are exactly the already-due ones (the probe-launch re-arm and the escalations both key off now), so if state.next_probe_at <= now is both what the comment describes and enough to stop the spin, while leaving a probe legitimately due before the retry deadline alone.

[ma2bd]

A 1ms floor does not accomplish what the comment claims. It converts the measured ~3,600 subscribes/s into ~1,000/s — still a probe storm, and still a full synchronize_chain_state_from attempt per iteration against any validator that fails fast.

Worse, flooring max_probe_interval with the same constant means --notification-circuit-breaker-max-probe-interval-ms 0 floors both ends to 1ms, so (probe_interval * 2).min(max) is pinned at 1ms and escalation stops existing. That is a config which silently disables the backoff rather than being rejected — the same class of problem as the unclamped 0 this constant is answering.

The flags are still unvalidated both in linera-client's parser and in Options (0, and max < initial). Validating there fixes it once for every consumer, and would let this constant be a floor at a sane scale — seconds — instead of a guard against arithmetic.

[ma2bd]

The PR description flags the missing jitter as a known property equivalent to restarting the workers, which is fair for a one-shot event. Two things make it recur rather than fire once, though: every later doubling re-synchronises the whole fleet on the same instant, and with (true, true) resetting to the initial interval, a fleet-wide churn event re-converges on a 300s cadence instead of spreading out over time.

For a worker with thousands of chains, each probe being a subscribe plus a full synchronize_chain_state_from, that spike has the same shape as the sync storms that have saturated Scylla before. A few percent of jitter on next_probe_at is a small change and removes the recurrence rather than just the first spike.

[ma2bd]

The breaker arithmetic now exists in four copies: (probe_interval * 2).min(max) plus now + interval here and in (false, false); the fresh-breaker construction here and in (true, true); and the floored initial interval in both retry_update_deadline and at the top of this function.

CircuitBreakerState::new(now, initial) and escalate(&mut self, now, max), plus one accessor for the floored interval, would collapse them. This matters past tidiness: the jitter suggestion above, or any later change to the backoff rule, has to land in all four to be correct.

[ma2bd]

Two inaccuracies in this comment, the first of them inherited from my own round-one wording.

I wrote that ChainListener "calls listen() once per chain and never retries", which became "never calls listen() again" — stronger, and not true. ChainListener::listen (linera-client/src/chain_listener.rs:599) early-returns only when the existing mode is already sufficient (*m >= listening_mode); a chain whose mode is upgraded falls through and calls client.listen() again, building a fresh ListeningClient and discarding the previous listener's breaker state. The accurate claim — and the one that makes arming the deadline here load-bearing — is that nothing retries listen() on failure. The mode-upgrade path is worth knowing about separately, since accumulated backoff silently resets there.

Second, proof/notifications.rs:131 still states that update_notification_streams runs only on Reason::NewBlock, which this PR makes false; it now also runs on a stream death and on the probe timer.

[ma2bd]

MaybeSendBoxFuture<'a, T> is futures::future::BoxFuture<'a, T> / LocalBoxFuture<'a, T> spelled out, and chain_worker/export.rs:1846 already uses those directly with the same cfg split — so the crate still holds two spellings of one policy. A cfg'd re-export keeps the single-source-of-truth win without a second definition:

#[cfg(not(web))]
pub(crate) use futures::future::BoxFuture as MaybeSendBoxFuture;
#[cfg(web)]
pub(crate) use futures::future::LocalBoxFuture as MaybeSendBoxFuture;

Also, ReceiveSenderCertificateFuture (client/mod.rs:394-397) is still the same cfg pair written out by hand, in a file this PR already touches for exactly this conversion.

[ma2bd]

This prunes handles whose stream was aborted, but a stream the client simply drops is never marked aborted, so its handle stays — my round-one suggestion was incomplete on that point.

The vec therefore still grows across re-subscribes where the client drops its end rather than the validator severing it, and disconnect_notification_subscribers then walks and aborts handles for streams nobody holds. Tracking liveness of the stream itself rather than the abort flag would prune the real set.

[ma2bd]

Third pass, at d1904577. Every round-two point is addressed, including the ones I expected argument about: the health signal is now serving_since, stored only after the initial sync returns; every launch arms a breaker, so a first subscribe that never resolves can no longer sit unnoticed; defer_due_probes clamps only what is already due; the floor is 1s with parse_millis_nonzero rejecting 0 at the parser; and the spread is a deterministic per-(chain, validator) offset, which keeps tests reproducible. without_failure_injection and wait_until close the two test-infrastructure points. The tripped flag is a good addition and is doing real work in two places.

What is left is mostly a consequence of the new shape rather than a leftover:

1. now is sampled before the update's only await and every deadline is computed from it, so a slow committee read arms deadlines that are already in the past, and the Ok path then retries with no pacing at all (:3534). The same stale read inverts serving_for.
2. Recovery has no lifetime threshold while churn has one, so the backoff ladder is reset or kept according to whether an unrelated update happened to land inside the serving window (:3570).
3. In the healthy steady state there are no breakers and therefore no timer at all — and I could not find the transport keepalive the Proposal section relies on to make that safe (:3382).
4. The probe interval doubles as a hard abort deadline for the first sync, so the chains furthest behind get every validator's stream cut simultaneously (:3588).
5. A simultaneous N-stream loss can drive N separate updates for one chain (:3417).

Then a spurious "recovered" line for something this code itself classifies as routine, a few cleanups, and two test notes.

[ma2bd]

Every deadline this update arms is computed from a clock read taken before its only await, so a slow committee read arms deadlines that have already passed.

now is sampled here, and the block below then awaits local_committee() or admin_committee(). local_committee() can fall back to a full synchronize_chain_state() on BlobsNotFound, which is a quorum round-trip. If that await takes longer than initial_probe_interval, every state.arm(now, spread) and CircuitBreakerState::failed(now, ..) below sets next_probe_at to a moment that is in the past by the time the update returns.

Back in listen(), min() picks that elapsed deadline, sleep_until returns immediately, and the loop re-enters the same slow call. It is not a CPU spin — each iteration costs whatever the committee read costs — but it is an unpaced retry against validators that are, by construction, already struggling. That is what defer_due_probes exists to prevent on the Err path; the Ok path has no equivalent guard.

The same stale now also inverts serving_for: a stream that starts serving during the await stores serving_since > now, Timestamp::duration_since saturates to zero, and the scan reads Some(ZERO) — so a stream that genuinely served is charged as a persistent fault by the (true, _) arm.

Re-reading the clock after the committee read fixes both.

[ma2bd]

Recovery has no lifetime threshold while churn has one, so the ladder is reset or kept depending on whether an unrelated update happened to land while the stream was briefly alive.

This arm drops the breaker after any amount of serving, but (true, Some(lifetime)) only counts as churn above initial_probe_interval. Take a validator escalated to the 1h cap whose probe subscribes, syncs, serves for a few seconds and dies — the version-skew case from last round, where one notification the client cannot deserialize becomes a non-retryable Code::Internal and ends the stream:

• If any update lands inside that window — and the initial sync itself processes certificates and emits NewBlock, so on an active chain one usually does — this arm removes the breaker. The death then hits (true, _) with no breaker, takes the else branch, and inserts failed(now, initial, spread). The ladder is gone, and the validator is re-probed every 300s forever with a full synchronize_chain_state_from each time.
• If no update lands in that window, the breaker survives with tripped still set, and the death escalates.

Same stream behaviour, opposite outcome, decided by unrelated timing. Gating this arm on the threshold the churn arm already uses — (false, Some(lifetime)) if lifetime >= initial_probe_interval — makes the two sides symmetric and preserves the ladder; a shorter-lived serving stream simply keeps its armed breaker, whose deadline is in the future anyway.

The threshold's direction is right and I am not asking to lower it: subscribe resets retry_count on every successful internal reconnect (linera-rpc/src/grpc/client.rs:401), not only when a notification arrives, so idle-timeout churn never accumulates toward max_retries and anything that reaches this breaker really has failed persistently.

[ma2bd]

In the healthy steady state this min() is over an empty map, so the listener holds no timer at all — and I could not find the keepalive the Proposal section relies on to make that safe.

Once every stream is serving, the recovery arm has removed every breaker, retry_update_at is None, and probe_due is Either::Right(future::pending()). The only remaining wake-ups are a notification arriving and a task ending. A connection that is black-holed rather than closed — a dropped NAT mapping, a half-open TCP with no FIN or RST, a proxy that stops forwarding — produces neither: is_aborted() stays false, serving_since stays set, the task never completes. On an idle chain the update never runs again, and the client is deaf on that validator indefinitely.

The Proposal argues this away with "because streams end rather than silently stalling (transport keepalive covers the connection level)". I went looking for that keepalive and did not find it: linera-rpc's transport::Options exposes only connect_timeout and a request timeout (grpc/pool.rs:26-36), and nothing in the tree sets http2_keep_alive_interval, keep_alive_while_idle or tcp_keepalive on the channel. If the request timeout is set and does bound a server-streaming call, that would cover it — but it would also cap every healthy subscription, so it is worth confirming which is true.

The cheap fix is to never let the timer go silent: have this arm re-arm a breaker at the initial interval instead of removing it, so there is always a deadline at which the listener re-examines its streams. That also gives the lifetime threshold above something to be evaluated against on a schedule rather than only when something else wakes the loop. Tracking a last-notification time would be the more precise version if you want liveness rather than just a periodic look.

[ma2bd]

This makes the probe interval double as a hard timeout for subscribe plus the initial sync, and cuts every validator's stream at once when a legitimately slow first sync exceeds it.

A client starting on a chain that is far behind launches all its streams together, each with launched(now, 300s, spread). If the initial synchronize_chain_state_from needs longer than that — for a chain tens of thousands of certificates behind, it will — every handle is (false, None) past its deadline at the same moment, so all of them are aborted mid-sync, and the chain has no streams at all for the next interval, then two, then four as the breaker escalates.

It does converge, since the sync persists certificates as it goes and each attempt resumes from where the last got to rather than restarting the work. But the deaf window grows exactly while the client is furthest behind, and the tail of every attempt is thrown away.

These are really two different quantities: how often to re-probe a validator, and how long a first sync may take. The second is unbounded by nature. Giving a first launch a longer grace than a re-probe, or not aborting a stream that is demonstrably still making progress, would stop this from punishing the worst-behind chains hardest.

[ma2bd]

Now that the tasks are individual, a simultaneous loss of every stream on a chain lands N completions here, and each one drives its own update_notification_streams. Coalescing happens only if await_while_polling drains the others while the update is suspended; when local_committee() resolves without yielding, the remaining completions each run a full update of their own.

That is an Nx amplification per chain at precisely the moment a fleet-wide event has hit every chain on the worker — and each of those updates can, through the BlobsNotFound fallback, become a quorum synchronize_chain_state(). Draining the completions that are already ready before deciding to update would bound it to one pass.

[ma2bd]

This arm's own verdict is that the stream was healthy and the loss is routine — info!("Long-lived validator notification stream ended; re-probing") — but it stores CircuitBreakerState::failed(..), which sets tripped = true. The next successful probe therefore logs Validator recovered from circuit breaker for a breaker that, by this arm's own classification, never represented unhealthiness.

That is the log noise tripped was added to suppress, coming back through the other door: one rolling validator restart produces a spurious "recovered" line per chain per validator, which on a worker with thousands of chains is a lot of lines. A churned(..) constructor with tripped: false keeps the distinction the flag exists for, and still behaves correctly on a subsequent failure — the else branch in (true, _) would charge that first short-lived death at the configured interval, which is the rule you already established.

[ma2bd]

Three small things around this helper:

• The interval parameter is dead flexibility — all four call sites pass initial_probe_interval(). As written the signature reads as though the spread scales with the current, escalated interval, which it deliberately does not. Dropping the parameter and reading self.initial_probe_interval() inside removes the misleading degree of freedom.
• The call at 3567 runs for every sender on every update, including inside the (false, Some(_)) arm that discards it. It is a pure function of (chain_id, validator) that never changes over a listener's lifetime, so moving it into the arms that actually use it (or caching it alongside the breaker) avoids paying for hashes that are thrown away.
• CircuitBreakerState::armed writes next_probe_at: now and then overwrites it via arm on the next line. Constructing it with the final value avoids an intermediate state that is briefly wrong.

Separately: StreamHandle, CircuitBreakerState, NOT_SERVING and defer_due_probes are pub(super) purely so the tests can reach them — defer_due_probes has exactly one production caller, in this file. Not worth contorting the code over, but worth deciding deliberately whether that surface should be visible across client/ in release builds.

[ma2bd]

without_failure_injection fixes the export-task half of this, but attempts is still inferred from a provider-wide counter, and make_nodes is reached from other ChainClient paths on the same provider — not only from update_notification_streams.

So the count includes any such call that lands in the window, measured against a hard < 50. If it ever trips, the message reports "the listener is hot-looping" for something that may have nothing to do with the listener. A counter incremented only on the update path would measure what the assertion names.

[ma2bd]

This takes an async RwLock read on every chain-info query, at every validator, in every test — for a gate that exactly one test ever closes — and holds the guard across self.client.lock().await and the whole query.

tokio::sync::RwLock is write-preferring, so once stall_chain_info_queries takes the write guard, every in-flight and subsequent chain-info query at that validator queues behind it, including ones from unrelated background tasks in the same TestBuilder. In test_a_stalled_sync_is_not_mistaken_for_a_serving_stream the guard is held for the rest of the test, so anything later added there that needs validator 0 to answer an info query will hang rather than fail. Checking a flag and awaiting only when it is armed keeps both the cost and the blast radius on the one test that needs the gate.