consensus/bor, eth, miner, internal/cli: add sequence store publisher - #2355
consensus/bor, eth, miner, internal/cli: add sequence store publisher#2355cffls wants to merge 17 commits into
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (89.09%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## sequencing #2355 +/- ##
=============================================
Coverage ? 55.88%
=============================================
Files ? 929
Lines ? 169336
Branches ? 0
=============================================
Hits ? 94634
Misses ? 69138
Partials ? 5564
🚀 New features to boost your workflow:
|
|
@claude review |
| var succession int | ||
| // if signer is not empty | ||
| if currentSigner.signer != (common.Address{}) { | ||
| // A signer outside the active set (post-Rio: outside the span's | ||
| // producer set) does not build at all — its candidate could never | ||
| // seal, and its sequence must never reach the store. Nodes without | ||
| // a signer (RPC) skip this check and keep their pending snapshot | ||
| // fresh. | ||
| succession, err = snap.GetSignerSuccessionNumber(currentSigner.signer) | ||
| if err != nil { | ||
| // If the signer is not in the active validator set, use succession 0 | ||
| // so that the pending block header is still valid for RPC queries. | ||
| // Seal() will independently reject the block if unauthorized. | ||
| succession = 0 | ||
| return err | ||
| } |
There was a problem hiding this comment.
🔴 consensus/bor/bor.go's Prepare() reverts the fix from #2183 (commit 2f0229d): it now returns UnauthorizedSignerError unconditionally whenever the local signer falls outside the active validator set (span rotation, jailing), instead of defaulting succession to 0 as before. Because Seal() already independently rejects unauthorized blocks, that fallback was safe and existed specifically so prepareWork()/commitWork() keep refreshing w.snapshotBlock/w.snapshotState; without it, the pending snapshot freezes and RPC calls (eth_call, eth_estimateGas, eth_getBlockByNumber at "pending") can read stale or zeroed state once the old pathdb trie layers are GC'd — the exact "insufficient funds for transfer" bug #2183 fixed. The change is on the shared, sequencer-agnostic Prepare() path (not gated by sequencer.enabled), so it affects every mining-configured validator node, and the regression test TestPendingStateNotStaleForNonValidator that covered this is deleted in the same diff.
Extended reasoning...
What the bug is. consensus/bor/bor.go's Prepare() (lines 1129-1140) replaces the fallback succession = 0 with an unconditional return err whenever snap.GetSignerSuccessionNumber(currentSigner.signer) fails - which happens precisely when the local signer is not currently in the active validator set. This is not a hypothetical: span rotation and validator jailing are routine, non-attacker-triggered consensus-state transitions that put a mining-configured node's own signer temporarily outside the set.
Why this is a reintroduced regression, not new behavior. This exact code path was deliberately changed in the other direction by commit 2f0229d (PR #2183, "consensus/bor, miner: fix stale pending state for non-validator nodes"), whose commit message states the failure mode precisely: "Prepare() returned UnauthorizedSignerError when the signer was not in the validator set, which caused prepareWork() to fail on every block. Since commit() was never called, the pending snapshot was never refreshed, and its underlying pathdb trie layers were eventually garbage-collected - making all state reads return zero." That PR's fix, and the comment this PR deletes, are worded almost identically to what is being undone here: default succession to 0 so "the pending block header is still valid for RPC queries. Seal() will independently reject the block if unauthorized." This PR removes both the fallback and the accompanying regression test (TestPendingStateNotStaleForNonValidator), with no mention of the tradeoff in the PR description.
The propagation path, verified in the current tree. GetSignerSuccessionNumber (snapshot.go) returns *UnauthorizedSignerError only when the signer is genuinely absent from s.ValidatorSet.Validators - not for a mere backup. makeHeader (miner/worker.go:2039) propagates this error straight up through prepareWork/buildAndCommitBlock, which returns early without ever calling commit()/updateSnapshot. worker.pending()/pendingBlock() (worker.go:706,719) then keep serving whatever snapshot was taken before the signer fell out of the set - frozen at that block height while the chain keeps advancing underneath it.
Why existing tests don't catch it. consensus/bor/bor_test.go has no test that calls Prepare() with a signer absent from the validator set (the one test that did, TestPendingStateNotStaleForNonValidator, is deleted by this PR), so CI is blind to the regression.
Scope: broader than the PR's own stated intent. The PR's rationale (visible in the new comment in bor.go) is that "a signer outside the active producer set ... does not build at all - its candidate could never seal, and its sequence must never reach the store." That's a real requirement for the sequencer/publisher feature this PR adds. But the check sits on Prepare(), which is shared and unconditional - it is not gated on sequencer.enabled or on the node's role. So it also fires for every ordinary mining-configured validator node that isn't running the sequencer at all, degrading their RPC-served pending state exactly as #2183 originally found and fixed.
Step-by-step proof of the failure:
- A validator node has authorizedSigner.signer set (mining configured), and is currently in the active validator set - Prepare() succeeds, succession is computed normally, blocks build and seal, pending() stays fresh.
- A span rotation or jailing event removes this node's signer from the active validator set (routine, expected, no attacker involved).
- On the next block, Prepare() calls snap.GetSignerSuccessionNumber(signer), which now returns *UnauthorizedSignerError because the signer isn't in s.ValidatorSet.Validators.
- With this PR's change, Prepare() immediately returns err - it no longer falls back to succession = 0.
- makeHeader receives that error and returns it up through prepareWork/buildAndCommitBlock; commit()/updateSnapshot is never reached for this or any subsequent block while the signer stays out of the set.
- w.snapshotBlock/w.snapshotState stop advancing. eth_call/eth_estimateGas/eth_getBlockByNumber("pending") on this node now serve state from the block height at which the signer fell out - and once the underlying pathdb trie layers for that old state are garbage-collected, state reads return zero, producing exactly the "insufficient funds for transfer"-style wrong answers consensus/bor, miner: fix stale pending state for non-validator nodes #2183 was written to fix.
Suggested fix. Reconcile the sequencer's requirement ("an out-of-set signer must never publish to the sequence store") with RPC freshness by gating the hard-fail behind the sequencer/producer role (e.g. only return err when sequencer.enabled and this node is the active producer), rather than changing the shared, always-on Prepare() behavior for every mining-configured node. Restoring the succession = 0 fallback for the non-sequencer path - and keeping (or adapting) TestPendingStateNotStaleForNonValidator - would close the regression.
There was a problem hiding this comment.
Intentional, keeping it. Post-Rio a signer outside the span's producer set must not build at all: with the sequencer attached, a non-producer build would open store windows for heights it can never seal — the two-generations-per-height contention the read-first design exists to prevent. Seal() rejecting later is too late; the store write happens at build time. Signerless nodes (RPC — the #2183 population) skip this branch entirely and keep a fresh pending snapshot. The residual case is a mining-configured node whose signer is outside the active set; the operator answer there is to not configure a signer on a non-validator node.
There was a problem hiding this comment.
The reply defends this as a sequencer-driven requirement ("with the sequencer attached..."), but the code in Prepare() has no gating on sequencer.enabled or on the node's producer role — it is unconditional for every mining-configured node, exactly as the original finding described. grep -n "[Ss]equencer" consensus/bor/bor.go returns nothing: a mining node that never configured [sequencer] at all (i.e. does not have "the sequencer attached") hits this same return err path the moment its signer rotates out of the active validator set, with the same stale-pending-state consequence #2183 fixed.
Separately, the "residual case" is described as operator misconfiguration ("the operator answer there is to not configure a signer on a non-validator node"), but that is not the actual population affected. A signer is configured precisely because the node is a validator; span rotation and jailing routinely and temporarily move a validator out of the active set without any misconfiguration on the operator's part. That is exactly the scenario #2183's commit message describes and fixes. If the intent is to restrict this hard-fail to sequencer-producer builds only, the code needs an explicit check (e.g. c.sequencer != nil && isProducer) rather than relying on "the sequencer is attached" as an implicit precondition that isn't actually enforced in Prepare().
|
@claude review |
There was a problem hiding this comment.
Beyond the new RATE_LIMITED finding in the inline comments, I also checked whether payload-building/pending-snapshot paths (generateWork, RPC eth_call-style snapshots) could leak into the sequence store: commitTransaction gates publishing on w.sequencingActive(env.header.Number), which is only true for actual block production (the production flag is set solely by commitWork), so non-production commits never call PublishTx. That path is fine.
Extended reasoning...
This run's automated review found one new issue (RATE_LIMITED handling in the seal gate transport, eth/sequencer/stream.go) which is already attached as an inline comment. I additionally verified a candidate concern raised by a finder agent — that generateWork/payload-building paths might publish transactions to the sequence store even outside real block production — and confirmed it is not an issue: commitTransaction only calls sequencer.PublishTx when w.sequencingActive(env.header.Number) is true, and that flag is derived from the production bool which commitWork alone sets (payload-building/pending snapshot paths never set it). This is purely a record of what else was examined; it is not a substitute for addressing the RATE_LIMITED finding.
| case pb.AckStatus_ACK_STATUS_RATE_LIMITED: | ||
| // The rejected entry never advanced the store head and everything | ||
| // pipelined behind it failed the head check too — a fresh stream | ||
| // resending in order is exact. | ||
| return streamResult{reason: endTransport}, true |
There was a problem hiding this comment.
🔴 handleAck maps ACK_STATUS_RATE_LIMITED to endTransport (eth/sequencer/stream.go:320-324), and step() treats every endTransport the same as a genuine connection failure by calling p.unreachable.Store(true) and backing off (stream.go:502-505). RATE_LIMITED just means the store is alive but declining this write under load, not that it's unreachable - yet while unreachable is set, ConfirmSeal short-circuits to SealUnknown (skipping even the store-independent chainVerdict rival check) and sealMirror/coverage skips its read entirely, both meant only for a genuine outage. Since rate-limiting is most likely under multi-producer contention, this defeats the one-broadcast-per-height guarantee exactly when it matters most. Fix: back off and retry on RATE_LIMITED without marking the store unreachable, so the gate keeps using chainVerdict/gateRecheck.
Extended reasoning...
The bug. handleAck (eth/sequencer/stream.go:320-324) maps ACK_STATUS_RATE_LIMITED to streamResult{reason: endTransport}, with a comment noting that a fresh resend is exact for this status. But step() (stream.go:489-505) does not distinguish why the transport ended: any endTransport reason unconditionally sets p.unreachable.Store(true) and enters degradedSleep backoff (1s doubling to 5s) before reconnecting. RATE_LIMITED is fundamentally different from a dropped connection or a dead store — the store received the write, evaluated it, and explicitly rejected it because of load. It is alive and reachable for every other producer, and quite possibly still reachable for reads from this producer too.\n\nWhy this matters. While p.unreachable is true, two safety mechanisms are deliberately disabled, because they're designed around the assumption 'the store is down for everyone, so blind broadcast is safe':\n- ConfirmSeal (gate.go) returns p.settle(miner.SealUnknown) immediately, broadcasting without waiting for a verdict, and skipping even the local, store-independent chainVerdict check (whether a rival's block already imported at our height).\n- sealMirror/coverage (barrier.go) returns true (skip the check) without attempting a read.\n\nThat assumption is false for RATE_LIMITED: the store is up for everyone else, so a rival producer may hold a live, readable window or a rejection verdict at this exact height. Treating a load-shedding response as a full outage throws away the one check (chainVerdict) that costs nothing and needs no store access, plus the mirror/coverage read that would have caught a rival's window.\n\nWhy this is not just theoretical. Rate-limiting is most likely to trigger precisely under multi-producer contention on one store (the M:N Bor:Heimdall topology this repo explicitly designs for) — i.e., exactly when the one-broadcast-per-height guarantee is needed most. Because RATE_LIMITED is rejected before the store's head CAS (per the code comment, the write 'never advanced the store head'), a losing producer can receive RATE_LIMITED without ever getting a STALE verdict (which would call markGateLost), so it never learns it lost the race through the normal channel — and now, for the backoff window, its safety net is also switched off.\n\nStep-by-step proof:\n1. Producer A and Producer B are both trying to publish at height N against the same sequence store, which is under load.\n2. The store accepts B's write (advances its head) and responds to A's write with RATE_LIMITED, because A's request landed under contention/load.\n3. handleAck maps this to endTransport; step() sets p.unreachable = true on A and starts a 1-5s backoff.\n4. A's block for height N seals locally. ConfirmSeal checks p.unreachable.Load() first and returns SealUnknown immediately — it never calls chainVerdict (which would have checked whether B's block already landed on A's local canonical chain) and never waits for a store verdict.\n5. A broadcasts its block for height N. If B's block also gets accepted and propagates, two divergent blocks now exist at height N, defeating the exact guarantee ("one broadcast per height") this whole subsystem exists to provide.\n\nWhy existing tests miss it. stream_test.go only asserts that handleAck returns endTransport for RATE_LIMITED; nothing exercises the resulting p.unreachable state or its effect on ConfirmSeal/AwaitSequenced.\n\nSuggested fix. Give RATE_LIMITED its own streamResult reason (or a flag) so step() backs off and retries the send without calling p.unreachable.Store(true) — i.e., treat it as "resend, do not disable the gate," which matches what the code's own comment already says about RATE_LIMITED's resend semantics.
51bd895 to
9967e98
Compare
…2370) ci: bump kurtosis-pos pin and Actions versions in kurtosis workflows
#2369) * tests/bor: harden test peering against simultaneous-connect collisions connectAndWaitForPeers dialed from both sides at once, which invites simultaneous-connect collisions: each server can drop the other's inbound connection as a duplicate of its own in-flight dial, and with aligned retry backoff the collision can repeat until the 120s deadline ("nodes failed to peer within deadline: peers a=0 b=0"). Hit twice on 2026-08-24 across two different callers on busy runners, including the develop push run. Dial from one side only, re-schedule a fresh static dial every 10s so a stuck dial state can't consume the whole deadline, and include both enodes in the timeout message. Both previously-flaked tests pass 3x consecutively with the change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * tests/bor: drop the redial ticker, single-sided dial suffices Review caught that RemovePeer/AddPeer never clears the dial scheduler's per-node history (remStaticCh handling deletes the task only; checkDial keeps rejecting with errRecentlyDialed for ~35s), so the ticker could not bypass backoff as its comment claimed — and the scheduler already retries static targets on its own after each history expiry. The single-sided dial is the whole fix; the comment now states the collision-plus-history mechanism accurately. Both tests still pass 3x consecutively. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* ci: add pipeline-enabled kurtosis e2e leg (POS-3697) pipeline.enable-import-src ships default-off, so no existing CI job exercises the pipelined import path on a real network. This leg runs the stateless-sync topology with the flag injected into the bor config template for every node: full-sync bor:local nodes import via the pipelined SRC, stateless-sync nodes self-gate the pipeline off and sync from witnesses the pipelined SRC produced, and a released-image baseline validator (participant 10, ignores the unknown [pipeline] section) independently validates every block the pipelined producers seal. Runs the unmodified pos-workflows stateless suite (participants 1-9 keep their exact indices), then role-based checks: pipeline active and root_mismatch == 0 on full-sync nodes, src == 0 on stateless nodes, witness/SRC count parity on the non-mining witness-producing RPC (every witness off the pipelined completion path), and the baseline's block hash against the pipelined reference. Validated end to end on a local enclave with the identical setup: all checks green, baseline hash-identical at the target block. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: source pipeline leg config and checks from pos-workflows Per review, the args file and checks script move to pos-workflows (0xPolygon/pos-workflows#48) alongside the other legs' configs and suites; only the workflow file stays in bor. The leg's behavior is unchanged — same topology, same template injection, same assertions. Depends on pos-workflows#48; this leg's CI run stays red on the missing config until that merges. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: bump kurtosis-pos pin to v1.4.1 Matches the pin pos-workflows moved to in 0xPolygon/pos-workflows#49; this leg consumes its config and test suites from pos-workflows@main. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
The first run built bor from sequencing, which does not carry the publisher yet (0xPolygon/bor#2355 is unmerged) — no sequencer metrics exist on such a build and test 1 reads 0/4 forever. Build from the PR branch until it merges (then sequencing, then develop), and on a test-1 failure print each validator's sequencer series count and publish state, telling a sequencer-less image apart from a wrong-state publisher. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Local codex reviews left the following points. Let me know if you need the expanded version
|
|
Cross verified |
|
Thanks for the thorough review, @pratikspatil024 — these were high-signal. Dispositions below; the fixes are in 0b5d8c7. Fixed in this PR (0b5d8c7):
All three were validated end-to-end on a 4-validator kurtosis rig: before the fix, non-producing validators returned Deferred / not blocking:
|
foundry-toolchain@c7450ba (v1.8.0), pinned inside kurtosis-pos's shared setup action, is broken against Foundry's current install infra (foundryup: cannot execute binary file — foundry-rs/foundry-toolchain#170). v1.4.2 carries the fix (bumped to foundry-toolchain v1.9.1).
… and consumer Block producers publish each block's lifecycle (open context, per-tx records, sealed header) to the sequence store as it happens; RPC nodes follow the stream, re-execute it deterministically, and hold preconfirmation receipts for the block being built. Design doc: docs/sequencer-bor.md. Producer (eth/sequencer.Publisher, miner hooks): read-before-write follow model — a foreign unsealed window on our tip is followed, not superseded; the only supersede is the seal flush that makes the store match sealed truth. Pre-seal barrier awaits sequencing; the post-seal gate turns store acks into broadcast verdicts (foreign seal refuses, budget expiry broadcasts for liveness after a recheck). Recovery is the reconcile position ladder (anchor, block-anchor probe, floor read) with delta-only re-anchors; producer rotation adopts the dangling window instead of revoking it. Transport hardening: bounded in-flight sends with ack refill, ack-stall watchdog, and a self-heal redial after prolonged channel silence. Consensus-side: a signer outside the active producer set no longer builds at all, so its sequence can never reach the store. Consumer (eth/sequencer.Consumer): follows the gateway stream with warm/cold resume, verifies the commitment chain per entry, re-executes on canonical or parked speculative state (author-nil EVM context, speculative BLOCKHASH, EIP-2935), cross-checks seals (context, gas, receipts root, state root), voids-and-skips on divergence, and fills a capped receipt index evicted on canonical import. The RPC read path that serves these receipts ships separately. Everything is gated behind the [sequencer] config section; the role derives from the sealer flag (mining node publishes, non-mining node consumes). With the section unset there is no behavior change. Validated on kurtosis devnets: a 12-phase chaos campaign (store component restarts, pauses, 200s outages, flapping, partitions, producer and heimdall kills) ended with zero store gaps, zero revoked or reordered preconfirmations, and zero absent heights across 859k entries; preconfirmation receipts measured at p50 ~100ms against ~2.5-2.9s canonical inclusion at 4s blocks, byte-consistent with canonical receipts after import. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ed functions Raise the patch coverage of the sequence-store integration from ~81% to ~91% (local measurement). The consumer gains an execution-level harness: a real imported chain re-executes speculative blocks through the session (canonical and parked parents, every void-and-skip path, the checkSeal divergence matrix, speculative BLOCKHASH resolution), and a stream-level suite follows a live devstore end to end — receipts served pre-seal, evicted on canonical import, and an exact warm resume across a store restart. The worker's barrier-refusal and fill-halt cycles and the backend's role dispatch are covered directly. Decompose the functions the complexity gate flagged — ConfirmSeal, backfillLocked, reader.walk, restoreAbandonedDebtLocked, floorRead, runStream — and split AdoptWindow and SealBlock into their orchestration and resolution halves. The backfill debt machinery moves to debt.go, the build-start read and sealed-height recovery to buildstart.go, and the worker's sequencer integration to miner/sequencer.go, bringing classify.go, adoption.go, and worker.go back under the size gates. No behavior changes. Mutation tiers hold at T1 84.9%, T2 61.7%. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… edge rows Drive the build-start read through every store shape it classifies — unreadable, empty, behind (live window above, seal edge below), sealed with the chain's block, sealed inside and past the recovery grace — and pin the recovered window's exact content. Cover the gate's last-look recheck against sealed generations and live windows, the refusal streak cap and flush unwind, the backfill drain's pruned-jump, byte-budget, and undrainable-debt rows, prime-and-merge debt bounds, the reoffer and regrown-window paths, walk absorb hooks, probe edges, and the worker's store-owned-height discard mid-fill. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Re-arm the finality grace when sync completes: commitWork returns on syncing before it consults the gate, so a resync outliving the grace consumed it silently and opened the gate the moment sync ended — the restart window the gate exists to cover. The anchor is now re-armed in the miner's DoneEvent/FailedEvent handling, before builds unblock. Give the consumer's cold resume ladder distinct rungs (block anchor, then earliest) instead of retrying the identical block anchor before falling back. Fail the build-start read loudly when probeDown violates its contract instead of misclassifying the height as sealed past. Register sequencer flags against defaults when an HCL/JSON config has no sequencer block, instead of dereferencing nil at startup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Heights at or below a canonical whitelisted milestone are final: immutable and permanently served by the canonical chain, so their store copies serve no consumer. The backfill drain now starts past the milestone — a 1-hour store outage owes seconds of blocks, not the hour — with the skipped prefix crossed by the jump open as a counted forward jump. Without a usable milestone (Heimdall down alongside the store, or a milestone naming a chain we don't hold) a 40-block depth cap below the tip bounds the drain instead. The floor is a fact about our own chain, never a belief about the store: storeSealedTip remains untrusted, so the devnet failure class that removed the earlier freshness bound (skipping heights the store had actually shed) does not reopen — below finality the hole is now intentional. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…arm wiring The drain with a floor inside the pending range (finalized prefix dropped, remainder rebuilt) and the miner's sync-completion re-arm of the finality grace were the two untested branches of the previous commits; codecov's patch gate flagged both. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The two adoption-floor tests swapped a live worker's chainConfig to vary the bor config, racing the worker's newWorkLoop, which re-reads chainConfig.Bor on every veblop tick — CI segfaulted in CalculatePeriod when a tick landed inside the nil-Bor swap window. The floor computation moves to a pure helper (adoptionMinTime) the tests exercise directly; the end-to-end applyAdoption coverage keeps the worker's real config. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nd pending state Addresses three review findings on the sequence-store publisher: - eth/sequencer: a live store window on a displaced parent (canonical reorg) now mutes the build instead of arming a sticky hold that could refuse to seal forever. The pre-seal barrier lets a muted build seal without a store mirror, and the seal flush supersedes the dead-parent window with sealed truth. - eth/sequencer: a partially adopted window whose transaction proves unexecutable on canonical state now seals on the executable prefix and lets the seal flush supersede the remainder, instead of the barrier refusing the block and re-adopting the same window forever. Adopted window timestamps are also bounded above (mirroring the consensus future-block limit) so a far-future timestamp cannot stall the build. - consensus/bor, miner: a signer outside the active producer set keeps its pending snapshot fresh again, so eth_call/eth_estimateGas against the pending block work on non-producing nodes. The build is instead kept out of the sequence store at the miner via a muted-build flag gated on IsAuthorizedSigner, so it publishes nothing and never contends the store's per-height election. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
0b5d8c7 to
4f2727b
Compare
The test drove a store-owned height with only the fill-loop resync abort (resyncN) guarding the seal, while AwaitSequenced still reported the height uncontested — so under constrained CI scheduling the build could seal before the abort fired (or the started worker's background loop could seal on the shared recorder), failing "must never reach the seal hook". Marking the height contested makes the seal barrier refuse unconditionally, which is what a store-owned height means, so the invariant no longer races the scheduler. Reproduced the old flake at GOMAXPROCS=1 (120/120 fail); green 200/200 after. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…used tasks Two fixes from a devnet load-test freeze: a producer rotated out mid-span kept sealing, its network-rejected block was flushed to the store as sealed truth, and the rotated-in producer then discarded its own valid block against that seal while the chain sat frozen a height below. - eth/sequencer: the broadcast gate now consensus-verifies a foreign store seal before honoring it as ownership of a height, on all three refusal paths (the timeout recheck, a tail-read loss carrying the decoded header, and a header-less STALE loss, which fetches the standing seal once). A seal whose signer the engine rejects can never become canonical, so it is noise, not ownership — the block broadcasts. A consensus-valid seal (an in-flight winner, a twin) keeps its refusal, as does one that cannot be inspected, bounded by the existing refusal cap. The verifier is the engine's VerifySeal, wired through Publisher.SetSealVerifier; a node without one behaves exactly as before. - miner: a gate-refused block now clears its pendingTasks entry. Nothing else ever could — clearPending needs chain progress a refused height never makes — and the leaked entry read as sealing-in-flight to the veblop stall fallback (decideVeblopFallback), disabling the only recovery path while the chain was stalled at that exact height. One refusal became a permanent production stop. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
5 participants
Block producers publish each block's lifecycle (open context, per-tx records, sealed header) to the sequence store as it happens; RPC nodes follow the stream, re-execute it deterministically, and hold preconfirmation receipts for the block being built. Design doc: docs/sequencer-bor.md.
Producer (eth/sequencer.Publisher, miner hooks): read-before-write follow model — a foreign unsealed window on our tip is followed, not superseded; the only supersede is the seal flush that makes the store match sealed truth. Pre-seal barrier awaits sequencing; the post-seal gate turns store acks into broadcast verdicts (foreign seal refuses, budget expiry broadcasts for liveness after a recheck). Recovery is the reconcile position ladder (anchor, block-anchor probe, floor read) with delta-only re-anchors; producer rotation adopts the dangling window instead of revoking it. Transport hardening: bounded in-flight sends with ack refill, ack-stall watchdog, and a self-heal redial after prolonged channel silence. Consensus-side: a signer outside the active producer set no longer builds at all, so its sequence can never reach the store.
Consumer (eth/sequencer.Consumer): follows the gateway stream with warm/cold resume, verifies the commitment chain per entry, re-executes on canonical or parked speculative state (author-nil EVM context, speculative BLOCKHASH, EIP-2935), cross-checks seals (context, gas, receipts root, state root), voids-and-skips on divergence, and fills a capped receipt index evicted on canonical import. The RPC read path that serves these receipts ships separately.
Everything is gated behind the [sequencer] config section; the role derives from the sealer flag (mining node publishes, non-mining node consumes). With the section unset there is no behavior change.
Validated on kurtosis devnets: a 12-phase chaos campaign (store component restarts, pauses, 200s outages, flapping, partitions, producer and heimdall kills) ended with zero store gaps, zero revoked or reordered preconfirmations, and zero absent heights across 859k entries; preconfirmation receipts measured at p50 ~100ms against ~2.5-2.9s canonical inclusion at 4s blocks, byte-consistent with canonical receipts after import.