Repro: poller never backfills below its first head on an empty store (all node roles) - #587
Open
dazthecorgi wants to merge 1 commit into
Conversation
…n empty store (all node roles)
The global-frame poller's forward-fill guard
if config.forward_fill && last_frame > 0 && new_number > last_frame + 1 {
treats an empty store (last_frame == 0 — every fresh boot) as "nothing to
fill": the first successful head poll stores ONLY the head frame, fires
on_frame for it, and latches last_frame = head. Frames 1..head-1 are never
fetched, and no runtime path revisits frames below the cursor (the
record-gap scan runs at bootstrap only), so the hole is permanent and the
serial materializer wedges below it.
This is NOT archive-specific. master_node/archive_sync.rs sets
forward_fill: true for EVERY node role — deliberately, because regular
nodes need contiguity too: their app-shard storage attestations anchor
ρ_N to an exact global frame that must be present in the local clock
store ("anchored global frame unavailable for ρ_N" when it is not).
Regulars only shrink the window (gossip stores frames independently; a
far-behind boot state-jumps): a client whose store is empty at its first
head poll while gossip missed the early frames — boot-time partition,
late-forming mesh, gossip drops — hits the identical skip with the
identical consequences. Archives, with no gossip arm and no state-jump,
hit it deterministically.
Observed live in the devnet partition harness: an archive isolated while
frames 1-2 finalize returns NotFound for them forever, failing the
terminal committed-range safety check ("safety fetch incomplete — ready
nodes could not serve their committed range").
This commit adds an in-process repro and NO fix:
- frame_sync: extract the poller's fetch surface into PollerFrameSource
(impl'd by ArchiveClient) and split run_archive_poller into a thin mTLS
wrapper over run_archive_poller_with_connector, so tests can inject an
in-process source. The loop body is unchanged; the public signature and
the sole production caller (master_node/archive_sync.rs) are untouched.
- forward_fill_repro::empty_store_first_head_poll_must_backfill_full_history
(archive mode, gossip_freshness: None) drives the REAL poller loop over
a fresh Rocks store against a fake archive serving frames 1..=5 and
asserts contiguity — FAILS on this branch (only frame 5 is stored;
on_frame saw [5]).
- forward_fill_repro::client_mode_empty_store_hits_the_same_skip
(regular-node mode, gossip_freshness: Some with a quiet mesh) — FAILS
identically, proving the guard is role-independent.
- forward_fill_repro::seeded_store_gap_is_forward_filled is the passing
contrast: the same gap above a NON-empty cursor (store holds frame 1)
is filled correctly — the empty-store case is an inconsistency in the
guard, not a designed limitation of the loop.
Deleting `last_frame > 0 &&` from the guard turns both repros green
(verified locally), though a production fix likely wants a bounded
initial fill rather than an unconditional genesis-to-head crawl.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
blacks1ne
added a commit
to blacks1ne/ceremonyclient
that referenced
this pull request
Aug 18, 2026
Fixes the bug @dazthecorgi reported and reproduced in QuilibriumNetwork#587; his repro commit is included unchanged. The forward-fill guard read if config.forward_fill && last_frame > 0 && new_number > last_frame + 1 so an empty store — every fresh boot — was treated as "nothing to fill". The first successful head poll stored ONLY the head and latched the cursor past everything below it, and nothing revisits frames under the cursor at runtime (the record-gap scan is bootstrap-only), so the hole was permanent and a serial materializer wedged beneath it. Not archive-specific: `forward_fill` is set for every node role, because regulars need contiguity too — their app-shard storage attestations anchor ρ_N to an exact global frame that must be present locally. Regulars just have a smaller window to hit it in. An empty store is a gap like any other, so the `last_frame > 0` arm is gone. Removing it alone would let a fresh archive at mainnet height attempt a genesis-to-head crawl inside one tick — an unbounded loop with no cancellation check — so the fill is now bounded to MAX_FORWARD_FILL_PER_TICK (512) frames per tick. A truncated chunk does NOT fall through to head processing: storing the head there would latch the cursor past the unfetched remainder and reintroduce the very hole this fixes. It resumes below the head on the next tick instead. The bound costs no throughput — each frame is a serial round-trip, so the achievable rate is far below the ceiling — and it makes the loop responsive to `cancel` during a long catch-up, which it was not before. Behaviour change worth noting: a regular node whose store is empty and whose state-jump does not complete now crawls up from genesis instead of skipping to head. That is the contiguity ρ_N requires, and the runtime far-behind rescue still attempts the jump first. Tests: @dazthecorgi's two repros fail before this change (on_frame sees only [5]) and pass after; his seeded-cursor contrast still passes. Adds `gap_wider_than_one_chunk_fills_contiguously_across_ticks` for the new chunk boundary. Full -p quil-rpc suite: 129/129. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds repros for a bug where the global frame poller never backfills below its first head on an empty store.
I tried to fix it, but the fix ballooned and I realized I don't understand the protocol & the codebase well enough to confidently ship a fix, so I thought it'd be better to just add a repro and pass it on to you @CassOnMars .
I found the bug using the devnet crate which is currently broken in .25, because it needs changes to libp2p which is no longer vendored. I added the vendored libp2p back and fixed devnet it in this branch: https://github.com/dazthecorgi/monorepo/tree/devnet-app-shard. So you can run
crates/devnet/test.shthere to repro the issue if you want using production nodes.LLM write up of the bug and the repros below.
The global-frame poller's forward-fill guard
treats an empty store (last_frame == 0 — every fresh boot) as "nothing to fill": the first successful head poll stores ONLY the head frame, fires on_frame for it, and latches last_frame = head. Frames 1..head-1 are never fetched, and no runtime path revisits frames below the cursor (the record-gap scan runs at bootstrap only), so the hole is permanent and the serial materializer wedges below it.
This is NOT archive-specific. master_node/archive_sync.rs sets forward_fill: true for EVERY node role — deliberately, because regular nodes need contiguity too: their app-shard storage attestations anchor ρ_N to an exact global frame that must be present in the local clock store ("anchored global frame unavailable for ρ_N" when it is not). Regulars only shrink the window (gossip stores frames independently; a far-behind boot state-jumps): a client whose store is empty at its first head poll while gossip missed the early frames — boot-time partition, late-forming mesh, gossip drops — hits the identical skip with the identical consequences. Archives, with no gossip arm and no state-jump, hit it deterministically.
Observed live in the devnet partition harness: an archive isolated while frames 1-2 finalize returns NotFound for them forever, failing the terminal committed-range safety check ("safety fetch incomplete — ready nodes could not serve their committed range").
This commit adds an in-process repro and NO fix:
Deleting
last_frame > 0 &&from the guard turns both repros green (verified locally), though a production fix likely wants a bounded initial fill rather than an unconditional genesis-to-head crawl.