Skip to content

consensus: persist AppQC, blocks, and CommitQCs with async persistence#2896

Open
wen-coding wants to merge 40 commits intomainfrom
wen/persist_appqc_and_blocks
Open

consensus: persist AppQC, blocks, and CommitQCs with async persistence#2896
wen-coding wants to merge 40 commits intomainfrom
wen/persist_appqc_and_blocks

Conversation

@wen-coding
Copy link
Contributor

@wen-coding wen-coding commented Feb 16, 2026

Summary

Crash-safe persistence for availability state (AppQC, signed lane proposals, and CommitQCs). All I/O is fully asynchronous — no disk operations on the critical path or under locks.

  • Extract consensus/persist/ sub-package: Generic A/B file persistence (Persister[T]) with crash-safe alternating writes. BlockPersister manages per-block files in a blocks/ subdirectory. CommitQCPersister manages per-road-index CommitQC files.
  • Generic Persister[T proto.Message] interface: Strongly-typed persistence API; concrete abPersister[T] handles A/B file strategy. A/B suffixes are unexported; WriteRawFile helper for corruption tests.
  • Block-file persistence (persist/blocks.go): Each signed lane proposal stored as <lane_hex>_<blocknum>.pb. On load, blocks are sorted and truncated at the first gap. PersistBatch encapsulates the I/O: persist blocks, update tips, clean up old files.
  • CommitQC persistence (persist/commitqcs.go): Each CommitQC stored as commitqc_<roadindex>.pb. On load, QCs are sorted and truncated at the first gap. Needed for reconstructing FullCommitQCs on restart.
  • Async persistence — goroutine reads inner state directly: A background goroutine (persistLoop) watches inner.blocks and inner.commitQCs directly (the in-memory state is the queue). collectPersistBatch acquires the lock, waits for new data, clamps cursors past pruned entries, and collects the batch. I/O runs with no lock held. No channel, no backpressure.
  • Cursor clamp for prune safety: Between persist iterations (lock not held), prune can delete map entries. Cursors are clamped to q.first before reading to prevent nil pointer dereference. Regression test (TestStateWithPersistence) reliably catches this.
  • Async AppQC persistence: latestAppQC is published via AtomicSend; a separate goroutine subscribes and persists asynchronously. No I/O under the inner lock.
  • Wire persistence into availability state (avail/state.go): NewState accepts stateDir, initialises both the A/B persister (for AppQC), BlockPersister, and CommitQCPersister, and loads persisted data on restart.
  • Restore state on restart (avail/inner.go): Uses queue.reset() to set starting positions, then pushBack to reload entries. Finally calls inner.prune() with the persisted AppQC to advance all queues — same code path as runtime. Returns error for corrupt state (AppQC without matching CommitQC on disk).
  • Gate voting on persistence (avail/subscriptions.go): RecvBatch only yields blocks below the nextBlockToPersist watermark, so votes are only signed for durably written blocks.
  • Clean up orphaned block files: DeleteBefore removes files for pruned blocks and orphaned lanes (from previous committees). Driven by the persist goroutine observing laneFirsts.
  • prune() returns lane firsts map: Eliminates the separate snapshot helper; nil return means no pruning occurred.
  • Test injection via private constructor: newState() accepts a custom Persister for test mocks, avoiding fragile field mutation after construction.
  • queue.reset() method: Clearly sets the starting position of an empty queue, replacing misleading prune() calls during initialization.

Ref: sei-protocol/sei-v3#512

Test plan

  • persist/blocks_test.go: load/store, gap truncation, DeleteBefore, orphaned lane cleanup, header mismatch, corrupt files
  • persist/commitqcs_test.go: load/store, gap truncation, DeleteBefore, corrupt files
  • persist/persist_test.go: A/B file crash safety, seq management, corrupt fallback
  • avail/state_test.go: fresh start, load AppQC, load blocks, load both, load commitQCs, load commitQCs with AppQC, corrupt data, headers returns ErrPruned for blocks before loaded range
  • avail/state_test.go (TestStateWithPersistence): end-to-end persist + prune race regression test (5 iterations with disk persistence; reliably catches cursor race without the clamp fix)
  • avail/inner_test.go: newInner with loaded state, newInner with all three (AppQC + CommitQCs + blocks), newInner error cases (AppQC without matching CommitQC), nextBlockToPersist reconstruction, votes queue advancement
  • avail/queue_test.go: newQueue, pushBack, reset, prune, stale prune, prune past next
  • consensus/inner_test.go: consensus inner persistence round-trip, persist error propagation via newState injection
  • data/state_test.go: data state tests
  • types/proposal_test.go: proposal verification tests

@github-actions
Copy link

github-actions bot commented Feb 16, 2026

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedFeb 24, 2026, 11:59 PM

@codecov
Copy link

codecov bot commented Feb 16, 2026

Codecov Report

❌ Patch coverage is 82.68293% with 71 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.16%. Comparing base (37f1b8d) to head (c2caac4).

Files with missing lines Patch % Lines
...mint/internal/autobahn/consensus/persist/blocks.go 77.41% 14 Missing and 14 partials ⚠️
...t/internal/autobahn/consensus/persist/commitqcs.go 80.21% 9 Missing and 9 partials ⚠️
sei-tendermint/internal/autobahn/avail/state.go 86.55% 8 Missing and 8 partials ⚠️
...ei-tendermint/internal/autobahn/consensus/state.go 58.33% 4 Missing and 1 partial ⚠️
sei-tendermint/internal/autobahn/avail/inner.go 94.59% 1 Missing and 1 partial ⚠️
...endermint/internal/autobahn/avail/subscriptions.go 60.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #2896       +/-   ##
===========================================
+ Coverage   58.12%   77.16%   +19.03%     
===========================================
  Files        2109       20     -2089     
  Lines      173381     1747   -171634     
===========================================
- Hits       100780     1348    -99432     
+ Misses      63659      259    -63400     
+ Partials     8942      140     -8802     
Flag Coverage Δ
sei-chain 79.96% <82.68%> (+21.87%) ⬆️
sei-db 69.50% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
sei-tendermint/internal/autobahn/avail/queue.go 100.00% <100.00%> (ø)
...ei-tendermint/internal/autobahn/consensus/inner.go 63.21% <100.00%> (ø)
...int/internal/autobahn/consensus/persist/persist.go 79.13% <100.00%> (ø)
sei-tendermint/internal/autobahn/avail/inner.go 92.42% <94.59%> (+0.99%) ⬆️
...endermint/internal/autobahn/avail/subscriptions.go 82.97% <60.00%> (-3.39%) ⬇️
...ei-tendermint/internal/autobahn/consensus/state.go 83.03% <58.33%> (+0.10%) ⬆️
sei-tendermint/internal/autobahn/avail/state.go 77.74% <86.55%> (+4.00%) ⬆️
...t/internal/autobahn/consensus/persist/commitqcs.go 80.21% <80.21%> (ø)
...mint/internal/autobahn/consensus/persist/blocks.go 77.41% <77.41%> (ø)

... and 2091 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@wen-coding wen-coding force-pushed the wen/persist_appqc_and_blocks branch from ebf93df to f4a9c1e Compare February 17, 2026 04:50
@wen-coding wen-coding changed the title Port sei-v3 PR #512: persist AppQC and blocks to disk consensus: persist AppQC and blocks to disk Feb 18, 2026
@wen-coding wen-coding changed the title consensus: persist AppQC and blocks to disk consensus: persist AppQC and blocks, async block fsync Feb 18, 2026
@wen-coding wen-coding changed the title consensus: persist AppQC and blocks, async block fsync consensus: persist AppQC and blocks in avail Feb 18, 2026
wen-coding and others added 7 commits February 20, 2026 10:15
Extract generic A/B file persistence into a reusable consensus/persist/
sub-package and add block-file persistence for crash-safe availability
state recovery.

Changes:
- Move persist.go and persist_test.go into consensus/persist/ (git mv to
  preserve history), exporting Persister, NewPersister, WriteAndSync,
  SuffixA, SuffixB.
- Add persist/blocks.go: per-block file persistence using
  <lane_hex>_<blocknum>.pb files in a blocks/ subdirectory, with load,
  delete-before, and header-mismatch validation.
- Wire avail.NewState to accept stateDir, create A/B persister for
  AppQC and BlockPersister for signed lane proposals, and restore both
  on restart (contiguous block runs, queue alignment).
- Update avail/state.go to persist AppQC on prune and delete obsolete
  block files after each AppQC advance.
- Thread PersistentStateDir from consensus.Config through to
  avail.NewState.
- Expand consensus/inner.go doc comment with full persistence design
  (what, why, recovery, write behavior, rebroadcasting).
- Move TestRunOutputsPersistErrorPropagates to consensus/inner_test.go
  for proper package alignment.
- Add comprehensive tests for blocks persistence (empty dir, multi-lane,
  corrupt/mismatched skip, DeleteBefore, filename roundtrip).

Ref: sei-protocol/sei-v3#512
Co-authored-by: Cursor <cursoragent@cursor.com>
Move persisted data loading (AppQC deserialization and block loading)
into a dedicated function for readability.

Co-authored-by: Cursor <cursoragent@cursor.com>
Move block sorting, contiguous-prefix extraction, and gap truncation
from avail/inner.go into persist/blocks.go so all disk-recovery logic
lives in one place. This isolates storage concerns in the persistence
layer, simplifying newInner and preparing for a future storage backend
swap.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
PushBlock and ProduceBlock now add blocks to the in-memory queue
immediately and send a persist job to a background goroutine via a
buffered channel. The background writer fsyncs each block to disk
and advances a per-lane blockPersisted cursor under the inner lock.

RecvBatch gates on this cursor so votes are only signed for blocks
that have been durably written to disk. When persistence is disabled
(testing), the cursor is nil and RecvBatch falls back to bq.next.

Co-authored-by: Cursor <cursoragent@cursor.com>
newInner no longer takes a separate persistEnabled bool; loaded != nil
already implies persistence is enabled. Tests with loaded data now
correctly reflect this.

Co-authored-by: Cursor <cursoragent@cursor.com>
blockPersisted is reconstructed from disk on restart, not persisted
itself. Move its creation to just above the block restoration loop
(past the loaded==nil early return) so the code reads top-down.

Co-authored-by: Cursor <cursoragent@cursor.com>
@wen-coding wen-coding force-pushed the wen/persist_appqc_and_blocks branch from 05beddb to 2f0bbad Compare February 20, 2026 18:46
wen-coding and others added 6 commits February 20, 2026 10:48
Co-authored-by: Cursor <cursoragent@cursor.com>
Move persistCh, persistJob, and the writer loop from avail/State into
BlockPersister.Queue + BlockPersister.Run, so callers just call Queue()
and the persist layer owns the channel, buffer sizing, and drain loop.

Queue blocks with context to avoid holes in the sequential
blockPersisted cursor (which would permanently stall voting).
Call sites use utils.IgnoreAfterCancel to swallow shutdown errors.

Co-authored-by: Cursor <cursoragent@cursor.com>
…l/sei-chain into wen/persist_appqc_and_blocks
@wen-coding wen-coding changed the title consensus: persist AppQC and blocks in avail consensus: persist AppQC and blocks with async block persistence Feb 20, 2026
wen-coding and others added 5 commits February 22, 2026 09:27
Remove redundant loop that explicitly zeroed every lane. Map zero-values
handle lanes without loaded blocks; only lanes with blocks on disk need
an explicit write. Add comment explaining why starting at 0 is safe.

Co-authored-by: Cursor <cursoragent@cursor.com>
- Add comment explaining why votes are not persisted and why the votes
  queue must be advanced past loaded blocks on restart.
- Consolidate redundant tests: fold blockPersisted assertions into
  existing tests, remove TestNewInnerLoadedBlocksContiguousPrefix.
- Add test that headers() returns ErrPruned for blocks before the
  loaded range (verifies votes queue advancement prevents hangs).

Co-authored-by: Cursor <cursoragent@cursor.com>
Replace require.Contains(err.Error(), "...") with require.Error(err).
Callers don't branch on specific error messages, so string matching
adds no value; the test name already documents what is being rejected.

Co-authored-by: Cursor <cursoragent@cursor.com>
BlockPersister now owns the per-lane contiguous persistence cursor and
passes the exclusive upper bound to the callback. The caller no longer
needs to compute n+1 or guard against out-of-order completion.

This localizes the ordering assumption (FIFO queue) inside
BlockPersister, so switching to parallel storage only requires changing
BlockPersister.Run.

Co-authored-by: Cursor <cursoragent@cursor.com>
Also add TODO for retry on persistence failure.

Co-authored-by: Cursor <cursoragent@cursor.com>
@wen-coding wen-coding force-pushed the wen/persist_appqc_and_blocks branch from a6a3e19 to 1581831 Compare February 24, 2026 00:31
wen-coding and others added 2 commits February 23, 2026 16:58
- Add appQCSend AtomicSend to publish latestAppQC updates.
- PushAppVote/PushAppQC store to appQCSend instead of calling
  persistAppQC under the lock.
- Run() spawns appQCPersist goroutine that watches and persists
  asynchronously, matching the pattern used for block tips.
- Remove persistAppQC method.
- Use q.pushBack() instead of direct queue field manipulation.
- Update persistQueueSize comment for mixed job types.

Co-authored-by: Cursor <cursoragent@cursor.com>
BlockPersister no longer owns a channel or Run loop. Instead, a single
goroutine in state.go watches inner.blocks for new entries, collects a
batch under the lock, then calls bp.PersistBatch (no lock held) which
persists blocks, updates tips, and cleans up old files.

This removes backpressure on PushBlock/produceBlock — they just add to
inner.blocks and return. Cleanup (DeleteBefore) is driven by the persist
goroutine observing laneFirsts, removing explicit QueueDeleteBefore
calls from PushAppVote/PushAppQC.

Also: use q.pushBack() instead of direct queue field writes, remove
dead Tips() method, simplify Queue drop comments.

Co-authored-by: Cursor <cursoragent@cursor.com>
On restart, the node previously lost all CommitQCs between the last
AppQC and the current head, forcing it to wait for re-gossip before
fullCommitQC could push to DataState. This adds async CommitQC
persistence alongside the existing block persistence:

- New CommitQCPersister (persist/commitqcs.go): individual files per
  road index, same load/sort/truncate-at-gap pattern as BlockPersister.
- Restore loaded CommitQCs into inner.commitQCs queue on restart,
  filtering by the AppQC-derived first index.
- Extend the persist goroutine to collect and persist new CommitQCs
  alongside blocks, with lazy cleanup of old files.

Co-authored-by: Cursor <cursoragent@cursor.com>
wen-coding and others added 3 commits February 23, 2026 21:23
Restructure newInner to load all persisted data (commitQCs, blocks)
first, then call inner.prune(appQC, commitQC) at the end — same code
path as runtime. This eliminates manual queue.first/next assignments
and ensures init and normal operation use identical pruning logic.

newInner now returns an error: if the persisted AppQC has no matching
commitQC on disk, the state is corrupt (the AppQC is always persisted
before old commitQC files are eligible for deletion).

Co-authored-by: Cursor <cursoragent@cursor.com>
Between persist iterations (lock released), prune can delete map entries
below q.first. Without clamping, the persist goroutine reads nil from
the map, causing a nil pointer dereference. Clamp blockCur and
commitQCCur to q.first before collecting batches.

Add TestStateWithPersistence (5 iterations) that reliably triggers the
crash without the fix.

Co-authored-by: Cursor <cursoragent@cursor.com>
wen-coding and others added 2 commits February 24, 2026 13:44
prune() on an empty queue works but reads as "deleting entries" when
the intent is "set starting position." Add reset(start) for clarity
and replace the three prune() calls in newInner that operate on
freshly created queues. Add unit tests for queue.

Co-authored-by: Cursor <cursoragent@cursor.com>
Extract the inline persist closure into two methods for readability.
Move latestCommitQC.Store into the commitQC loading loop in newInner.
Remove dead blockNum variable in produceBlock; use q.next directly.

Return clamped commitQCCur via persistBatch to prevent busy-loop when
prune empties the queue (value type not visible to caller otherwise).

Co-authored-by: Cursor <cursoragent@cursor.com>
Comment on lines +633 to +635
for lane, next := range blockCur {
inner.nextBlockToPersist[lane] = next
}

Check warning

Code scanning / CodeQL

Iteration over map Warning

Iteration over map may be a possible source of non-determinism
Comment on lines +652 to +656
for lane, q := range inner.blocks {
if blockCur[lane] < q.next {
return true
}
}

Check warning

Code scanning / CodeQL

Iteration over map Warning

Iteration over map may be a possible source of non-determinism
Comment on lines +662 to +670
for lane, q := range inner.blocks {
// Clamp cursor: prune may have deleted entries below q.first
// between iterations while the lock was not held.
blockCur[lane] = max(blockCur[lane], q.first)
for n := blockCur[lane]; n < q.next; n++ {
b.blocks = append(b.blocks, q.q[n])
}
b.laneFirsts[lane] = q.first
}

Check warning

Code scanning / CodeQL

Iteration over map Warning

Iteration over map may be a possible source of non-determinism
@wen-coding wen-coding changed the title consensus: persist AppQC and blocks with async block persistence consensus: persist AppQC, blocks, and CommitQCs with async persistence Feb 24, 2026
@wen-coding wen-coding requested a review from pompon0 February 24, 2026 23:48
if len(l.commitQCs) > 0 {
i.commitQCs.reset(l.commitQCs[0].Index)
for _, lqc := range l.commitQCs {
if lqc.Index == i.commitQCs.next {
Copy link
Contributor

Choose a reason for hiding this comment

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

return error in case loaded state is not a list of consecutive commitQCs.

return uint64(q.next) - uint64(q.first)
}

// reset sets the starting position of an empty queue.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: you should just recreate the queue if you want to reset the state. Or you can call prune(start) if you want to move 0 -> start.

inner utils.Watch[*inner]

// persister writes avail inner state to disk using A/B files; None when persistence is disabled.
persister utils.Option[persist.Persister[*apb.AppQC]]
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: group the persisters under a single Option. It would be kind of weird to allow for an arbitrary subset of persisters.

const innerFile = "avail_inner"

// loadPersistedState loads persisted avail state from disk and creates persisters for ongoing writes.
func loadPersistedState(dir string) (*loadedAvailState, persist.Persister[*apb.AppQC], *persist.BlockPersister, *persist.CommitQCPersister, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

oof, this result type deserves a struct. It will combine nicely with the previous comment.

return nil
}
updated, err := inner.prune(appQC, qc)
laneFirsts, err := inner.prune(appQC, qc)
Copy link
Contributor

Choose a reason for hiding this comment

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

ugh, so the laneFirsts are not always used (wasted computation), and now we check for non-nil map (which are proper empty maps). Perhaps it was not such a good idea :/ . Up to you, if you want to revert it to the previous version.

if bp, ok := s.blockPersist.Get(); ok {
cp, _ := s.commitQCPersist.Get()
scope.SpawnNamed("persist", func() error {
return s.persistLoop(ctx, bp, cp)
Copy link
Contributor

Choose a reason for hiding this comment

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

once you get all persisterts into a single struct, it can have its own Run() method to encapsulate these.

func (s *State) persistLoop(ctx context.Context, bp *persist.BlockPersister, cp *persist.CommitQCPersister) error {
blockCur := bp.LoadTips()
var commitQCCur types.RoadIndex
if cp != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

either assume cp is non-nil, or make it Optional


// Update nextBlockToPersist under lock.
if len(batch.blocks) > 0 {
for inner, ctrl := range s.inner.Lock() {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I would make it a method of State, MarkAsPersisted, or similar.

return true
}
}
return cp != nil && commitQCCur < inner.commitQCs.next
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

// collectPersistBatch waits for new blocks or commitQCs and collects them under lock.
func (s *State) collectPersistBatch(
ctx context.Context,
cp *persist.CommitQCPersister,
Copy link
Contributor

Choose a reason for hiding this comment

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

cp is only passed to check whether commitQCs update should also be considered. This should be a given always (see comments above)

if !ok {
return nil
}
if err := p.Persist(types.AppQCConv.Encode(appQC)); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

If I understand correctly, it is possible that appQC persistance will outdistance commitQCs persistance, in which case we will end up with inconsistent state (AppQC for a future CommitQC).

if len(batch.blocks) > 0 {
for inner, ctrl := range s.inner.Lock() {
for lane, next := range blockCur {
inner.nextBlockToPersist[lane] = next
Copy link
Contributor

Choose a reason for hiding this comment

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

we need a similar mechanism for commitQCs - in particular consensus.State should not start voting for height H+1 until commitQC for H is persisted. Otherwise we can lose commitQCs.

commitQCCur types.RoadIndex // clamped cursor (may have advanced past pruned entries)
}

func (s *State) persistLoop(ctx context.Context, bp *persist.BlockPersister, cp *persist.CommitQCPersister) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

this loop looks very similar to lanevote subscription (which is intentional). Perhaps we could deduplicate this logic in an elegant way? (this is not a request for change, it is just a thought).

Copy link
Contributor

@pompon0 pompon0 left a comment

Choose a reason for hiding this comment

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

The updated version looks very nice! Good work! There are still a few missing pieces. I've left comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants