Skip to content

fix(chainfold): various bugs discovered during formal verification - #53

Merged
rymnc merged 9 commits into
mainfrom
fix/chainfold-bugs
Oct 13, 2026
Merged

rymnc merged 9 commits into
mainfrom
fix/chainfold-bugs

Conversation

@rymnc

@rymnc rymnc commented Oct 9, 2026 •

Copy link
Copy Markdown
Member

Breaking: Source::head returns BlockRef, SnapshotSink::reset is required, DivergenceCause::ForkBeyondWindow is removed. Bumps to chainfold 0.4.0.

  • Trust boundaries: decode refuses a cursorless snapshot that carries a ring. Checkpoint slots are capped at 1 << 16. Only the engine's own offer can move the durable watermark. A queue depth of 0 no longer blocks.
  • Panics poison: a caught Fold::apply panic poisons the engine instead of leaving a half-applied span. Poisoned engines refuse to encode. Caught up is false unless Active.
  • Rollback and reset bookkeeping: a rollback drops every checkpoint newer than the restored cursor and rewinds the write position. Cadence marks are clamped each tick. Sinks reset with the engine. A recovered engine checkpoints at the recovery point.
  • One chain per fold: the scan reads the head first, and every batch must match it. Torn answers, out-of-window entries and two hashes at one height are refused. A head below the cursor is Idle, not a fork. The walk stops at u64::MAX.
  • Bounded walks: a tick reads at most span windows (starts at 64, halves on error, doubles otherwise). A long empty walk keeps its progress, so a first sync finishes under rate limits.
  • Docs and tests: README fork/terminal semantics corrected. apply_alloc counts per thread. ScriptedChain::events_in includes u64::MAX.

rymnc added 6 commits October 9, 2026 22:40
- decode refuses a cursorless snapshot that carries a ring, a state the
  engine never produces and cannot fold from
- EngineConfig rejects more than 1 << 16 checkpoint slots with a typed
  error; Engine::new and decode_snapshot panicked on a huge count
- Flusher::submit is private and DurabilityToken is gone, so only an
  engine's own offer moves the durable watermark; a caller holding the
  sink through Driver::sink could push it past the engine cursor
- Flusher::spawn counts a queue depth of 0 as 1; 0 blocked the first
  offer forever
A panic out of Fold::apply that the caller caught left the engine Active
with a half-applied span. The next batch then folded its events twice, a
snapshot of it decoded as a trusted engine, the harness loop died without
a terminal status, and the driver kept reporting caught up.

- apply_batch marks the engine Poisoned once per batch, before the span
  loop, and restores Active on every exit that is not a panic
- encode_snapshot refuses a Poisoned engine with SnapshotError::Poisoned;
  a Halted engine still encodes
- the harness catches a fold panic, publishes the Poisoned status, ends
  the loop, and shutdown still returns the driver; a panic that leaves the
  engine Active is rethrown
- is_caught_up and DriverStatus::caught_up are false while the engine is
  not Active, so a Halt, a Poison and a caught panic stop reading as
  caught up
… reset

- a rollback drops every checkpoint newer than the restored cursor, not
  only those above the fork block, so an expired checkpoint cannot come
  back to life when the refold reaches its block again. It also moves the
  write position back over the dropped slots, so the refold refills them
  before it evicts a live checkpoint, and a head that flaps between two
  siblings no longer wears the slots down to a resync
- both cadence marks are clamped to the cursor block at the start of every
  tick, so a rollback, whoever ran it, no longer suppresses the next
  checkpoint or offer; a resync clears them the same way
- SnapshotSink gains a required reset. After it, durable_cursor is None
  until an offer made after the call commits. The driver calls it when
  the engine was reset, whether by a resync or by hand, and when it takes
  over a sink that is ahead of its engine. DriverStatus::durable_cursor is
  None while a reset is unseen. The Flusher keeps a reset epoch beside its
  watermark, so a job queued before a reset commits without raising it.
  NoSink does nothing and WatermarkSink forgets its offers
- a driver over a recovered engine that holds no checkpoint takes one at
  the recovery point, so a one-block reorg of the first tip rolls back
  instead of resyncing from the start block
- chainfold 0.4.0, since SnapshotSink::reset is required
The scan read the cursor block's header before the head and the events, so a
reorg landing between the reads let new-chain events fold on an old-chain
boundary. It also folded whatever it was handed: events a reorg skipped between
two windows, a torn answer, entries outside the window, two hashes at the cursor
block, and the rest of a block the cursor stopped inside were folded wrongly or
lost without a signal. A head below the cursor resynced the whole fold, and a
head at u64::MAX never ended the walk.

`Source::head` now returns the head block's header. The scan reads it first and
vouches for everything after it:
- the walk starts on the cursor block, so the answer carries the boundary
  header; two hashes there, or any entry outside its window, refuse the answer
- entries at or before the cursor are dropped, so the rest of a partly folded
  cursor block is folded
- a batch with events re-reads the head block and is dropped when it moved
- a head below the cursor on the chain the ring observed is Idle, not a fork
- `group_into` splits spans on any block change, so two hashes for one height
  are a source error
- the walk stops at u64::MAX
The dead `scanned_to` mark is gone.

BREAKING: `Source::head` returns `BlockRef`, not `u64`.
- the README said a fork deeper than the ring is ForkBeyondWindow and
  that with zero slots Halt and Poison escalate to a resync; such a fork
  resyncs, and Halt and Poison are terminal for every slot count
- DivergenceCause::ForkBeyondWindow was never built, so it is gone
- the apply_alloc counters were process wide, so a measurement could
  count the test harness's own threads and fail under cargo test; they
  count per thread now
- ScriptedChain::events_in left out the block numbered u64::MAX from a
  range that ends there

BREAKING: DivergenceCause::ForkBeyondWindow is removed.
A walk that found nothing ran to the head in one tick, however long, and any
failed call dropped everything it had read. A first sync over a long empty
stretch never finished under a rate limit or a few failed calls.

- a tick reads at most `span` windows. The span starts at 64, halves after a
  source error and doubles after any other tick
- a walk that stops at the cap with nothing to fold remembers the block it
  ended on, read between two reads of the head block so it lies on the chain
  the head names. The next tick checks that block against the head and goes
  on above it, with the ring's cursor block as the boundary. A mark off the
  head's chain is dropped, and the walk starts at the cursor in the same tick
- the mark is set only once the tick's boundary check passes, and it never
  moves to the head. Folding a batch, a fork, a rollback, a reset and
  `source_mut` clear it
- a head below the mark is Idle, as a head below the cursor is, unless it
  contradicts the ring
- a capped tick is `Progressed` with a zero summary, is not caught up, and asks
  for no delay
- a head that a folding or capped tick read before and after its walk, once
  its boundary check passed, names the same chain at the next tick. That tick
  takes the engine's own cursor block as the boundary and walks above it, and
  above the mark without checking it, so a catch-up at a small window reads no
  more than before walks started on the cursor block
- `engine_mut` clears nothing. The driver records the engine's cursor and reset
  count at the end of each tick that leaves it active; a different pair at the
  next tick means the caller moved the engine, which drops the mark and the
  remembered head and reads as not caught up. A caller that only reaches the
  fold, as the merkle_log example does to prune it, keeps all of it

A walk of up to 64 windows, 32,000 blocks at the default window, costs what it
did.
@rymnc
rymnc force-pushed the fix/chainfold-bugs branch from 73d5b82 to 8e5006d Compare October 9, 2026 17:12
@rymnc rymnc self-assigned this Oct 9, 2026
@rymnc
rymnc marked this pull request as ready for review October 9, 2026 17:41
@rymnc
rymnc requested review from cygnusv and oskarth as code owners October 9, 2026 17:41
Copilot AI balanced review requested due to automatic review settings October 9, 2026 17:41

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@oskarth

oskarth commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator

The lagging shortcut in scan (driver.rs:578) only checks the ring for a block at the head's height, but the ring holds event and boundary blocks only. With events at 10 and 20 and a fork that replaces 10..20 with a chain ending at 15, every tick is Idle with caught_up = true and the orphaned events stay folded until the fork outgrows 20. Suggest confirming the newest observed block at or below the head with one header_at before returning Lagging (one extra read per lagging tick; a_head_below_the_mark_is_idle_and_changes_nothing then expects 2 reads), and documenting the residual window in the scan doc comment.

Repro, passes on this branch as crates/chainfold/tests/lagging_repro.rs:

use chainfold::{Driver, DriverConfig, EngineConfig, Position, Tick, test_util::{RecordingFold, ScriptedChain}};

#[test]
fn a_shorter_fork_with_no_ring_entry_at_its_head_is_reported_idle() {
    // given events at blocks 10 and 20 only, folded to block 20
    let mut chain = ScriptedChain::new(1);
    for n in 1..=20u64 {
        let events: Vec<u64> = if n == 10 || n == 20 { vec![n] } else { vec![] };
        chain.push_block(&events);
    }
    let config = EngineConfig { ring_capacity: 64, checkpoint_slots: 4 };
    let mut driver = Driver::new(RecordingFold::default(), chain, config, DriverConfig::from_block(1)).unwrap();
    while driver.tick() != Tick::Idle {}
    assert_eq!(driver.engine().cursor(), Some(Position::new(20, 0)));
    // when blocks 10..=20 are replaced by a fork ending at 15, with an event at 12
    let blocks: Vec<Vec<u64>> = (10..=15u64).map(|n| if n == 12 { vec![12] } else { vec![] }).collect();
    let replacements: Vec<&[u64]> = blocks.iter().map(Vec::as_slice).collect();
    driver.source_mut().reorg(11, &replacements);
    let ticks: Vec<Tick> = (0..5).map(|_| driver.tick()).collect();
    // then every tick is Idle, caught_up is true, the orphaned events stay, 12 is never folded
    assert!(ticks.iter().all(|t| *t == Tick::Idle), "{ticks:?}");
    assert!(driver.status().caught_up);
    let applied: Vec<u64> = driver.engine().fold().applied.iter().map(|(_, e)| *e).collect();
    assert_eq!(applied, vec![10, 20]);
}

@rymnc

rymnc commented Oct 13, 2026

Copy link
Copy Markdown
Member Author

addressed in 9cb78f8 @oskarth

@rymnc
rymnc enabled auto-merge (squash) October 13, 2026 11:54
@rymnc
rymnc merged commit 7b49420 into main Oct 13, 2026
14 checks passed
@rymnc
rymnc deleted the fix/chainfold-bugs branch October 13, 2026 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants