Skip to content

feat(autobahn): implement new lane ID for epoch (CON-358) - #3862

Open
wen-coding wants to merge 29 commits into
mainfrom
wen/lane_id_in_epoch
Open

feat(autobahn): implement new lane ID for epoch (CON-358)#3862
wen-coding wants to merge 29 commits into
mainfrom
wen/lane_id_in_epoch

Conversation

@wen-coding

@wen-coding wen-coding commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Replace LaneID = PublicKey with LaneID = (Validator, Joined): stay keeps Joined; leave ends that identity; rejoin allocates a new LaneID with Joined set to the join epoch.
  • Avail lane lifecycle (see package avail doc):
    • active — in the next-CommitQC committee (maps ensured at ApplyEpoch)
    • closing — left that committee, maps/WAL kept until closed at epochOfFirst
    • closedepochOfFirst.IsClosed → maps dropped; SyncLanes deletes the WAL
  • Persist: allowCreate decided in avail (active committee lane, or non-empty proposals so a leaver still flushes); SyncLanes is race-safe with MaybePruneAndPersistLane (map lock held across persist/delete).
  • Producer: per-session mempool via alignMempool / clearMempool (runMempool); sessions are WaitForLocalLane → produce until WaitUntilClosed. InsertTx waits for mempool align when LocalLane is already set; TryInsertTx fails fast; leave clears mempool and yields ErrNotProducing.
  • Giga StreamLaneProposals: bind by LaneID; server ends cleanly on ErrLaneClosed; client resumes from NextBlock and only excludes the old LaneID when the applied committee has dropped/replaced it (transient disconnect reconnects to the same identity).
  • Invalid/legacy lane dirs are warn-and-skipped and leak until wipe (intentional for this PR).

Compatibility

Autobahn has not launched. There is no production Autobahn state to migrate, and this PR does not attempt backward compatibility for persisted CommitQC/Block WALs, littblock records, or the wire encoding of BlockHeader/LaneRange (lanelane_id). Nodes must wipe persistent_state_dir (including blockdb) / do a coordinated reset when taking this change. Opaque decode failures on old epochs are acceptable in that context; a littblock serialization-version bump is not required for launch readiness.

Multi-epoch (#3736)

Production ApplyEpoch / ActivateEpoch wiring, neighbor VerifyInWindow, and accepting prior-epoch CommitQC while tip lags land in #3736. This PR ships LaneID + leave/rejoin scaffolding and unit coverage only; do not expect end-to-end multi-epoch production paths here.

Test note

  • TestMempool_EvmTxByHash: set MaxTxsPerBlock = 1 so a 1ms BlockInterval sealing between sequential InsertTx calls no longer trips env.Run's "block sealed too early" check (pre-existing flake under -race).

Made with Cursor

Comment thread sei-tendermint/internal/autobahn/consensus/persist/blocks.go
Comment thread sei-tendermint/internal/autobahn/avail/inner.go Outdated
@wen-coding
wen-coding force-pushed the wen/lane_id_in_epoch branch from 8d254d4 to 4f52a00 Compare August 5, 2026 22:23
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

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

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedAug 13, 2026, 9:42 PM

Comment thread sei-tendermint/internal/autobahn/avail/state.go Outdated
@codecov

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 74.42308% with 133 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.50%. Comparing base (cc95830) to head (8e093f7).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
sei-tendermint/internal/autobahn/avail/state.go 68.31% 17 Missing and 15 partials ⚠️
...i-tendermint/internal/autobahn/producer/mempool.go 70.42% 12 Missing and 9 partials ⚠️
...dermint/internal/autobahn/pb/autobahn.wireguard.go 37.50% 15 Missing ⚠️
...mint/internal/autobahn/consensus/persist/blocks.go 68.88% 7 Missing and 7 partials ⚠️
sei-tendermint/internal/autobahn/producer/state.go 77.77% 8 Missing and 4 partials ⚠️
sei-tendermint/internal/p2p/giga/avail.go 56.52% 6 Missing and 4 partials ⚠️
sei-tendermint/internal/autobahn/avail/inner.go 86.44% 4 Missing and 4 partials ⚠️
...ei-tendermint/internal/p2p/giga_router_fullnode.go 0.00% 6 Missing ⚠️
...endermint/internal/autobahn/avail/subscriptions.go 77.77% 2 Missing and 2 partials ⚠️
sei-tendermint/internal/autobahn/epoch/registry.go 76.92% 2 Missing and 1 partial ⚠️
... and 5 more
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3862      +/-   ##
==========================================
- Coverage   59.49%   58.50%   -0.99%     
==========================================
  Files        2325     2230      -95     
  Lines      198636   188317   -10319     
==========================================
- Hits       118172   110172    -8000     
+ Misses      69237    67727    -1510     
+ Partials    11227    10418     -809     
Flag Coverage Δ
sei-chain-pr 74.75% <74.23%> (?)
sei-db 70.41% <ø> (ø)
sei-db-state-db ?

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

Files with missing lines Coverage Δ
sei-tendermint/autobahn/types/block.go 83.33% <100.00%> (ø)
sei-tendermint/autobahn/types/epoch.go 100.00% <100.00%> (ø)
sei-tendermint/autobahn/types/proposal.go 92.43% <100.00%> (ø)
sei-tendermint/autobahn/types/testonly.go 96.12% <100.00%> (+0.06%) ⬆️
...-tendermint/internal/autobahn/avail/block_votes.go 90.90% <ø> (ø)
...t/internal/autobahn/consensus/persist/commitqcs.go 72.83% <ø> (ø)
...i-tendermint/internal/p2p/giga/pb/api.wireguard.go 100.00% <100.00%> (ø)
sei-tendermint/internal/p2p/giga/service.go 70.00% <100.00%> (ø)
sei-tendermint/internal/p2p/giga/types.go 100.00% <100.00%> (ø)
...i-tendermint/internal/p2p/giga_router_validator.go 93.10% <100.00%> (ø)
... and 16 more

... and 98 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.

Comment thread sei-tendermint/internal/p2p/giga/avail.go
seidroid[bot]
seidroid Bot previously requested changes Aug 5, 2026

@seidroid seidroid Bot 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.

The LaneID = (validator, e_join) refactor is coherent and well-tested at the type/proto layer, but the new lane-deletion path (pruneInactiveLanes + DeleteLane) breaks the "lanes are only added, never removed" invariant that three unguarded map reads in avail still rely on, and the leave/rejoin handling in SubscribeLaneProposals and ProduceLocalBlock has races that panic or permanently kill the producer. These are latent until epoch transitions are wired up (#3736), but they are defects in this PR's own feature and are not covered by the added tests.

Findings: 6 blocking | 13 non-blocking | 9 posted inline

Blockers

  • pruneInactiveLanes deletes entries from inner.blocks / inner.votes, but three call sites still index those maps without an ok check and will nil-deref once a leaver is pruned: avail/state.go:654 (headers: q := inner.votes[lr.Lane()] then q.first), avail/state.go:815 (PushQC loop: inner.blocks[lr.Lane()].q[n]), and avail/inner.go:189 (laneQC: i.votes[lane].q[n]). All three iterate the committee of the QC's epoch, which can be an older epoch that still contains the leaver. The comment removed from persist/blocks.go ("lanes are only added, never removed") was load-bearing for these too — every reader needs an ok-check (or lanes must be retained until the prune anchor passes them).
  • No test covers the interaction between tryPruneLeaveLanes and a lagging reader. TestApplyEpoch_AddsJoinerDefersLeaverUntilCommitQCWatermark verifies the leaver's maps/WAL disappear, but nothing exercises headers() / fullCommitQC / the s.data.PushQC loop against a previous-epoch CommitQC after the prune, which is exactly the crash path. Please add one.
  • Prune watermark choice needs justification: normal block retention is gated on the durable prune anchor (AppQC-derived, advancePersistedBlockStart), but a leaver's in-memory queues and WAL are dropped as soon as any durable CommitQC lands in the new epoch. Blocks that are committed but not yet executed/served can still be needed at that point. Either reuse the prune-anchor watermark or document why CommitQC-epoch is sufficient.
  • 3 blocking issue(s) flagged inline on specific lines.

Non-blocking

  • The Cursor review file (cursor-review.md) is empty — that pass produced no output, so this review merges only Claude's and Codex's findings.
  • Codex P1 #1 (legacy block WALs / wire format): the lane WAL directory name changes from hex(pubkey) (32B) to hex(pubkey||e_join) (40B), and BlockHeader.lane / LaneRange.lane change proto type from PublicKey to LaneID on the same field number. Both are hard breaks — existing WAL dirs are silently skipped on restart, and peers on the old binary cannot decode. Since ApplyEpoch/ActivateEpoch have no production callers yet, this is presumably pre-launch and acceptable; please confirm explicitly in the PR description rather than leaving it implicit.
  • ApplyEpoch never returns a non-nil error. Either drop the return value or note that it is reserved for the follow-up wiring.
  • tryPruneLeaveLanes re-Stores the identical latestCommitQC value after the disk delete purely to wake waiters. It is safe today only because markCommitQCsPersisted and tryPruneLeaveLanes are both on the runPersist goroutine — worth stating that in the comment, since a concurrent writer would make this a watermark regression. ctrl.Updated() alone may be enough.
  • markBlockPersisted writes inner.nextBlockToPersist[lane] = next unconditionally, so a pruned lane can be resurrected as a stale map entry (small leak, and it makes the map key sets diverge from blocks/votes).
  • LaneProposalsRecv.Recv allocates an errgroup and two goroutines per block received, plus a fresh LocalLaneUpdates() subscription per iteration. On the hot proposal path this is meaningful churn; consider hoisting the lane-change watcher out of the per-block loop.
  • alignMempoolForLane reads NextBlock(lane) before taking the mempool lock, so the tip can be stale by the time it is applied; and a rejoin silently discards all buffered evmTxs/evmNonces. Both are probably intended, but neither is documented.
  • 6 suggestion(s)/nit(s) flagged inline on specific lines.

Comment thread sei-tendermint/internal/autobahn/avail/state.go Outdated
Comment thread sei-tendermint/internal/autobahn/avail/subscriptions.go Outdated
Comment thread sei-tendermint/internal/autobahn/avail/state.go Outdated
Comment thread sei-tendermint/internal/autobahn/producer/state.go Outdated
Comment thread sei-tendermint/autobahn/types/committee.go
Comment thread sei-tendermint/autobahn/types/committee.go Outdated
Comment thread sei-tendermint/autobahn/types/lane_id.go Outdated
Comment thread sei-tendermint/internal/autobahn/consensus/persist/blocks.go
Comment thread sei-tendermint/internal/autobahn/avail/state.go Outdated
Comment thread sei-tendermint/internal/autobahn/producer/state.go Outdated
Comment thread sei-tendermint/internal/autobahn/avail/subscriptions.go Outdated
Comment thread sei-tendermint/internal/autobahn/avail/inner.go Outdated
seidroid[bot]
seidroid Bot previously requested changes Aug 6, 2026

@seidroid seidroid Bot 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.

Solid, well-documented reshaping of LaneID into (validator, e_join) with good test coverage of the stay/leave/rejoin state machine, but two correctness problems in the persistence/pruning paths are blocking: restored leave-lane queues are never positioned at the prune anchor (restart fails), and the leave-lane retention watermark is keyed on e_join rather than the leave epoch, so long-tenured leavers are dropped before their final tips are committed. Cursor's second-opinion pass produced no output; Codex's two findings are both confirmed and included.

Findings: 3 blocking | 12 non-blocking | 7 posted inline

Blockers

  • avail: no test covers the restart path that actually breaks — a persisted leave-lane WAL whose surviving blocks start above 0 (i.e. a prune anchor with a non-empty LaneRange for the leaver). TestApplyEpoch_AddsJoinerDefersLeaverUntilAppQCWatermark and TestTryPruneLeaveLanes_OrphanWALWithoutMaps both persist laneB at block 0 with no anchor, so they pass over the bug in newInner. Please add a restart test with an anchor whose leaver LaneRange.First() > 0.
  • 2 blocking issue(s) flagged inline on specific lines.

Non-blocking

  • Cursor's review file (cursor-review.md) is empty — that pass produced no output, so this review merges only Claude's and Codex's findings.
  • Epoch-boundary peer teardown for remote lanes is unhandled. Registry.VerifyInWindow still only checks the latest committee (existing TODO), and PushBlock/PushVote now return ErrBadLane when inner.blocks/inner.votes lacks the lane. A peer that applies epoch N+1 slightly before us and pushes a proposal on its new rejoin LaneID makes clientStreamLaneProposals return an error (giga/avail.go:194), tearing down that peer's RPC stream. The PR carefully protects the local lane via ErrLaneIdentityChanged; remote lanes need the equivalent soft-failure (or a widened verification window) before ApplyEpoch is wired up.
  • ApplyEpoch swaps inner.epoch wholesale, so inner.laneQC and PushVote immediately evaluate already-accumulated old-epoch votes against the new committee's LaneQuorum() and weights. Votes from departed validators are still in the queues and would be assembled into a LaneQC that fails verification against the new committee. The existing // TODO: filter votes per-epoch committee becomes load-bearing once ApplyEpoch is called in production — worth stating explicitly in the #3736 follow-up.
  • The BlockHeader.lane/LaneRange.lane proto type change (PublicKeyLaneID) alters block header hashes and makes previously persisted WAL entries undecodable, so any node with existing autobahn state needs a coordinated state reset. Autobahn is opt-in via autobahn-config-file, so this is likely fine, but the PR description doesn't mention it and there's no migration note.
  • producer.mempoolFirst() (producer/mempool.go:83) is now dead — alignMempoolForLane replaced its only caller. unused isn't enabled in .golangci.yml so it won't fail lint, but it should be removed.
  • types.GenCommittee (testonly.go) now bypasses the public constructors and calls normalizeWeights + finalizeCommittee directly to inject random e_join values. Randomizing e_join in tests is a genuine improvement (it catches code assuming e_join == 0), but duplicating the constructor body in test-only code means future changes to NewCommittee won't be reflected. Consider a newCommitteeWithEJoins(weights, func(PublicKey) EpochIndex) helper shared by all three.
  • State.ApplyEpoch always returns nil. The comment explains this is retained for #3736, which is reasonable, but every current caller has to handle an error that cannot occur.
  • 5 suggestion(s)/nit(s) flagged inline on specific lines.

Comment thread sei-tendermint/internal/autobahn/avail/inner.go Outdated
Comment thread sei-tendermint/internal/autobahn/avail/inner.go Outdated
Comment thread sei-tendermint/internal/autobahn/consensus/persist/blocks.go
Comment thread sei-tendermint/internal/autobahn/consensus/persist/blocks.go Outdated
Comment thread sei-tendermint/internal/autobahn/avail/subscriptions.go Outdated
Comment thread sei-tendermint/internal/autobahn/producer/state.go Outdated
Comment thread sei-tendermint/internal/autobahn/epoch/registry.go

@claude claude Bot 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.

No new bugs found this run. The latest commit (b3a591f, "harden LaneID leave/rejoin lifecycle") appears to resolve the checkBound panic, nil-map-deref, orphan-WAL, and producer-race issues flagged in earlier reviews on this PR (cursor, seidroid, and my own prior comments). Given the size and consensus-critical scope of this change, I'd still like a human to take a look before merge rather than shadow-approving.

What was reviewed:

  • checkBound/Recv no longer panics on leave+rejoin coalescing - now returns ErrLaneIdentityChanged (avail/subscriptions.go)
  • headers(), the PushQC block-collection loop, and laneQC() now guard missing lane maps instead of nil-dereferencing after a leave prune (avail/state.go, avail/inner.go)
  • produceLocalBlock/streakOpErr now bind to an explicit LaneID per streak, so a stale rejoin race resolves to ErrBadLane/context.Canceled instead of killing producer.Run (producer/state.go)
  • Checked the ruled-out mempool-reset-on-rejoin candidate - alignMempoolForLane only resets state when the streak's LaneID actually changes, so a same-lane stay keeps its tip and txs
Extended reasoning...

This run's bug hunter found no new issues. Cross-referencing the current HEAD (b3a591f, ahead of the PR diff shown) against the earlier cursor-bot, seidroid-bot, and my own prior claude[bot] comments on this thread, the latest commit appears to specifically address nearly every previously flagged blocker: the LaneID-coalescing panic in checkBound (now returns ErrLaneIdentityChanged), the nil-map-dereference in headers()/PushQC/laneQC() after a leave-lane prune (now guarded with ok-checks), the orphan leave-WAL-survives-restart issue (inner.go now reattaches leave-lane WALs into maps so tryPruneLeaveLanes/DeleteLane can reap them), the 'leave tears down peer RPC' issue (giga/avail.go now retries/resubscribes instead of propagating a fatal error out of the multiplexed RunServer scope), the producer lane-rederivation race (produceLocalBlock now takes an explicit bound LaneID and checks HasLane against it), and the HasLane O(n) regression (committee.go now keeps a byValidator map for O(1) lookup). I did not find a case where these fixes are incomplete.

This is nonetheless a large (36-file), consensus-critical change to how lanes are identified across the availability plane, persistence layer, producer, and wire protocol (BlockHeader.lane and LaneRange.lane both change proto type), including a protobuf wire-format change and WAL directory-naming change. ApplyEpoch/ActivateEpoch have no production callers yet (explicitly deferred to #3736), which reduces blast radius today, but the design decisions here (e.g. epoch-scoped LaneID reuse, AppQC-floor-gated lane pruning, coalescing-safe subscription semantics) are exactly the kind of judgment calls that warrant a human's sign-off before the epoch-transition wiring lands on top of them.

No security-sensitive auth/crypto/permission logic is touched beyond the existing signature verification already in place; the main risk surface is correctness/liveness of consensus (panics, stuck goroutines, dropped blocks) rather than exploitable vulnerabilities. Test coverage is substantial (new tests for committee activation, lane WAL orphan pruning, coalesced leave/rejoin subscription behavior, and a producer lifecycle test), which supports confidence in the fixes but doesn't substitute for a maintainer familiar with the epoch-transition roadmap reviewing the design.

seidroid[bot]
seidroid Bot previously requested changes Aug 6, 2026

@seidroid seidroid Bot 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.

Solid, well-tested refactor of LaneID from PublicKey to (validator, e_join) with careful prune/leave/rejoin bookkeeping. The blocking concern is that it silently breaks the on-disk autobahn WAL format (and the P2P wire format) with no migration or documented state-dir reset, so an in-place upgrade of an autobahn-enabled node fails to start; several smaller robustness/hot-path issues are noted below.

Findings: 2 blocking | 11 non-blocking | 7 posted inline

Blockers

  • On-disk WAL format break with no migration path. BlockHeader.lane and LaneRange.lane change from PublicKey to the new LaneID message. PublicKey{ed25519: <32 bytes>} and LaneID{validator, e_join} are not wire-compatible, so on an in-place upgrade of an autobahn-enabled node a persisted prune anchor decodes through PruneAnchorConv.Decode -> types.CommitQCConv.Decode -> LaneRangeConv.Decode and fails on the embedded LaneRange. loadPersistedState (sei-tendermint/internal/autobahn/avail/state.go:267) treats that as fatal, so NewState errors and the node will not start until <persistent_state_dir> is wiped. Same for the peer wire format: old and new binaries cannot exchange BlockHeader/LaneRange, and BlockHeader.Hash() changes. Autobahn is opt-in and off by default, so this is likely acceptable in substance -- but it needs to be explicit: either handle/skip the old encoding, or state the required state-dir reset and the no-mixed-version constraint in the PR description / release notes. Right now the only breakage signal is the non-app-hash-breaking label, which reads as the opposite.
  • 1 blocking issue(s) flagged inline on specific lines.

Non-blocking

  • Cursor's second-opinion pass produced no output (cursor-review.md is empty), so this review merges only Claude's and Codex's findings.
  • Codex's point, confirmed but downgraded: avail.State.ApplyEpoch, epoch.Registry.ActivateEpoch, and types.ActivateCommittee have no non-test callers, so the entire leave/rejoin/prune path this PR adds is unreachable in production until the advanceEpoch/onAdvance wiring in #3736 lands. That is consistent with the PR being explicitly incremental (the ApplyEpoch doc comment says so), so it is not a defect -- but it does mean the new gates get no production coverage in this PR, and the correctness of tryPruneLeaveLanes rests entirely on unit tests plus a chain of non-local invariants (see the inline notes on state.go:122 and blocks.go:347). Worth an integration test that drives a real epoch transition through producer.Run + runPersist before the wiring PR flips it on.
  • Registry.ActivateEpoch changing RWMutex[registryState] to RWMutex[*registryState] is the right fix (s.latest = next on a value copy would have been lost) -- good catch. Unrelated nit: ActivateEpoch doesn't touch prev's RoadRange, so callers passing OpenRoadRange() for every epoch (as all the new tests do) end up with overlapping open ranges. The doc comment acknowledges this; consider rejecting a roads that overlaps prev so the invalid state is unrepresentable rather than a caller contract.
  • The PR description says leavers are pruned at the "AppQC prune floor (lane.EJoin() < appQC.EpochIndex())", but the implemented gate is tipcut-committee membership (!current.HasLane(lane) && !tipcut.HasLane(lane)), with no EJoin() comparison anywhere. The code is fine; the description should match it so future readers don't go looking for the EJoin check.
  • serverStreamLaneProposals can now silently switch the stream to a different LaneID mid-connection (rejoin resets first = 0). clientStreamLaneProposals happens to tolerate this because PushBlock resolves the lane from the header, but the commented-out sanity check at giga/avail.go:191 (proposal lane == peer key) can no longer be a simple equality once it's uncommented -- it has to compare Lane().Validator(). Worth updating that comment now while the context is fresh.
  • 6 suggestion(s)/nit(s) flagged inline on specific lines.

Comment thread sei-tendermint/internal/autobahn/consensus/persist/blocks.go
Comment thread sei-tendermint/internal/autobahn/avail/state.go
Comment thread sei-tendermint/internal/autobahn/avail/subscriptions.go Outdated
Comment thread sei-tendermint/internal/autobahn/avail/state.go Outdated
}

lw, err := bp.getOrCreateLane(lane)
allowCreate := active.HasLane(lane) || len(proposals) > 0

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[suggestion] active is a full *types.Committee used for exactly one boolean (active.HasLane(lane)), which drags a consensus type into the persistence layer and forces every test call site to fabricate a synthetic committee (committeeForLane, plus ~15 utils.OrPanic1(types.NewCommittee(...)) calls in state_test.go/blocks_test.go). Passing allowCreate bool (or active bool) instead would keep the persister's dependency surface where it was and delete all that test boilerplate.

Also: active is dereferenced with no nil guard, so a nil committee panics on the nil-map read inside HasLane rather than returning an error. A bool parameter makes that unrepresentable.

Comment thread sei-tendermint/internal/autobahn/avail/state.go Outdated
Comment thread sei-tendermint/internal/autobahn/producer/state.go Outdated
Comment thread sei-tendermint/internal/autobahn/producer/mempool.go
Comment thread sei-tendermint/internal/autobahn/avail/inner.go Outdated
seidroid[bot]
seidroid Bot previously requested changes Aug 6, 2026

@seidroid seidroid Bot 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.

Solid, well-documented refactor of LaneID from a bare pubkey to (validator, e_join), with good test coverage for the stay/leave/rejoin state machine and correctly regenerated wireguard/proto size tables. Two blocking issues remain: existing autobahn persistent state becomes unreadable with no migration or version gate (commitQC WAL hard-fails, block WAL dirs are silently orphaned), and a restart inside a leave window can make the local PushQC loop permanently skip road indices whose retained CommitQCs still reference the departed lane.

Findings: 5 blocking | 13 non-blocking | 9 posted inline

Blockers

  • Upgrade path for an existing autobahn persistent_state_dir is undefined and handled inconsistently: the commitQC WAL / prune anchor hard-fail to decode (startup abort), while legacy block WAL dirs are warn-and-skipped (silent data loss + orphaned dirs that tryPruneLeaveLanes can never reach, since they are not in bp.lanes). Pick one policy — a state-dir version marker that fails fast with an actionable message, or an explicit migration/cleanup — and state it in the PR description. See the two inline comments on blocks.go:234 and autobahn.proto:131.
  • No test covers the upgrade path at all: there is no case that opens a BlockPersister over a 32-byte-hex lane dir, and none that feeds a pre-change (field-1 lane) LaneRange/CommitQC WAL entry through loadAllCommitQCs. Whatever policy is chosen for the item above should be pinned by a test, since this is exactly the failure that only shows up on a real operator's disk.
  • 3 blocking issue(s) flagged inline on specific lines.

Non-blocking

  • The Cursor pass (cursor-review.md) produced no output — the file is empty. Only Codex's three findings were available to merge; all three are reflected above (its P1 #1 as a blocker, P1 #2 as a blocker, P2 #3 as an inline suggestion).
  • PR description drift: it states the prune floor is the "in-memory AppQC prune floor (lane.EJoin() < appQC.EpochIndex())", but the implemented gate in tryPruneLeaveLanes is "the first retained CommitQC's committee no longer names the lane". The code comments are accurate; the description is not. Worth fixing since this is the subtlest invariant in the change.
  • producer.mempoolFirst() (producer/mempool.go:83) is now dead — Run was its only caller and it went through alignMempoolForLane. unused is not in .golangci.yml, so lint will not catch it.
  • tryPruneLeaveLanes allocates a full KnownLanes() snapshot and re-scans it after every persist batch, even when nothing has left the committee. Cheap fix: skip the whole body unless the epoch changed since the last check or len(inner.blocks) > current.Lanes().Len() (the guard already used for the in-memory half).
  • LaneID.Bytes() calls l.validator.Bytes() twice (once for the capacity, once for the append). Hoist it into a local.
  • epoch.GenRegistry still builds its genesis committee via NewCommittee (all e_join = 0), so the avail/producer/giga tests only ever exercise e_join = 0 lanes end-to-end; the random-e_join GenCommittee change is confined to the types package. An ActivateEpoch-based fixture with a non-zero e_join stayer would give the new WAL naming and map keying real coverage.
  • ApplyEpoch returns an error that is documented as always nil. Fine as a forward-compat hook for #3736, but every current caller has to require.NoError on something that cannot fail.
  • 6 suggestion(s)/nit(s) flagged inline on specific lines.

Comment thread sei-tendermint/internal/autobahn/consensus/persist/blocks.go
optional uint64 first = 2; // required
optional uint64 next = 3; // required
optional bytes last_hash = 4 [(wireguard.max_size) = 32]; // required
optional LaneID lane_id = 5; // required

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[blocker] reserved 1 + required lane_id = 5 makes the persisted commitQC WAL unreadable, not just the wire format. NewCommitQCPersister opens its WAL with types.CommitQCConv and loadAllCommitQCs decodes every entry eagerly; a pre-change entry has LaneRange.lane on field 1 and no lane_id, so LaneIDConv.DecodeReq(nil) returns e_join: missing. That error propagates out of NewCommitQCPersisterloadPersistedStateavail.NewState, so a node with an existing autobahn state dir refuses to start, with no migration and no version marker. The prune anchor (PersistedAvailPruneAnchor, which embeds a CommitQC) fails the same way.

Note this is the opposite behaviour from the block WAL, which silently skips legacy dirs (see the comment on blocks.go:234) — one path aborts startup, the other loses data quietly.

Autobahn is opt-in (autobahn-config-file), so "wipe the state dir" may well be an acceptable rollout. But that has to be a stated decision: add a state-dir format version that fails with an explicit "incompatible autobahn state, remove

" message, and say so in the PR description.

Comment thread sei-tendermint/internal/autobahn/avail/inner.go Outdated
Comment thread sei-tendermint/internal/autobahn/avail/state.go Outdated
Comment thread sei-tendermint/internal/autobahn/avail/state.go Outdated
Comment thread sei-tendermint/internal/autobahn/avail/subscriptions.go Outdated
Comment thread sei-tendermint/internal/autobahn/producer/state.go Outdated
Comment thread sei-tendermint/autobahn/types/committee.go Outdated
Comment thread sei-tendermint/internal/autobahn/epoch/registry.go
Comment thread sei-tendermint/internal/autobahn/avail/inner.go Outdated
Comment thread sei-tendermint/internal/autobahn/avail/state.go
Comment thread sei-tendermint/autobahn/types/testonly.go Outdated
@wen-coding wen-coding changed the title feat(autobahn): identify lanes as (validator, e_join) (CON-358) feat(autobahn): implement new lane ID for epoch (CON-358) Aug 6, 2026
@wen-coding wen-coding added the ai: skip-review Skips Seidroid AI review triggered via Unified CI. label Aug 6, 2026

@claude claude Bot 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.

This PR is a large, consensus-critical refactor (LaneID now (validator, e_join) instead of a bare pubkey, touching committee membership, avail WAL persistence, producer lifecycle, and the wire format) — well outside the scope for auto-approval regardless of this run's findings.

No new bugs were found this run. Beyond the reported findings, I also checked three additional candidates and ruled them out: a hang in Block/PushBlock/WaitForLocalCapacity if a lane is deleted mid-wait (the deployment model in this repo means ApplyEpoch/epoch transitions have no production callers yet, so this is dormant); tryPruneLeaveLanes turning a missing registry epoch into a NewState startup-abort (registry only holds genesis today, so unreachable until epoch persistence lands); and unbounded epoch registry growth (bounded in practice since nothing currently activates new epochs).

Extended reasoning...

Given the volume of prior findings on this thread (several genuine correctness gaps around leave-lane pruning windows, restart handling of in-flight leave WALs, and WAL/wire format compatibility flagged by other reviewers) and the fact that this changes core consensus lane-identity semantics, a human should review this rather than relying on automated approval.

Comment thread sei-tendermint/internal/autobahn/avail/inner.go Outdated
@wen-coding
wen-coding force-pushed the wen/lane_id_in_epoch branch from 3c335fa to fcee025 Compare August 8, 2026 00:30
Comment thread sei-tendermint/internal/autobahn/avail/state.go
Comment thread sei-tendermint/internal/autobahn/avail/subscriptions.go Outdated
Comment thread sei-tendermint/internal/autobahn/avail/state.go Outdated
@wen-coding
wen-coding force-pushed the wen/lane_id_in_epoch branch from 26a7c0b to d37679b Compare August 9, 2026 04:53
@wen-coding wen-coding removed the ai: skip-review Skips Seidroid AI review triggered via Unified CI. label Aug 9, 2026
Comment thread sei-tendermint/internal/autobahn/avail/state.go
seidroid[bot]
seidroid Bot previously requested changes Aug 9, 2026

@seidroid seidroid Bot 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.

Solid, well-documented reworking of LaneID into (validator, e_join) with good coverage on the persist/DeleteLane paths, but two blocking issues remain: PushBlock/PushVote don't re-check lane presence after waiting (the exact hazard the rest of the PR carefully fixes elsewhere), and four unrelated QC-verification tests were deleted with no replacement, leaving PrepareQC.Verify and AppQC.Verify weight/epoch-binding coverage at zero.

Findings: 4 blocking | 11 non-blocking | 8 posted inline

Blockers

  • Unexplained test deletions in sei-tendermint/autobahn/types/committee_test.go: TestPrepareQCVerifyChecksWeight, TestPrepareQCVerifyChecksEpochBinding, TestAppQCVerifyChecksWeight, and TestNewCommittee_RejectsEmptyWeights are removed with no replacement. TestCommitQCVerifyChecksWeight was not added — the old one was deleted and the PrepareQC test renamed into its place. After this PR there is no test anywhere in autobahn/types exercising PrepareQC.Verify, and none exercising AppQC.Verify weight thresholds. None of these depend on LaneID, so nothing in this change requires dropping them. Please restore them (mechanically updating to committee.Lane(...) where needed).
  • 3 blocking issue(s) flagged inline on specific lines.

Non-blocking

  • cursor-review.md is empty — the Cursor pass produced no output, so this review reflects only Claude + Codex findings.
  • inner.prune(c *types.Committee, ...) (inner.go:228) never references c — its body iterates i.votes. This PR now threads a carefully computed anchorCommittee into that ignored parameter, which reads as if the anchor committee scopes the prune when it does not. Either use it or drop the parameter.
  • Stale-leave pruning only runs when collectPersistBatch's WaitUntil predicate fires (new blocks / new commitQC / new appQC). A tip-stale leave lane with nothing pending and an otherwise idle chain lingers in inner.blocks and on disk until unrelated activity wakes the loop. Consider adding staleLaneDisposable to the wake predicate.
  • PushCommitQC still hard-rejects QCs whose EpochIndex differs from the applied epoch (state.go:435). Once ApplyEpoch has production callers, in-flight prior-epoch QCs will surface as stream errors to peers during a transition. The PR body defers this to #3736 — worth a TODO(#3736) at that comparison so it isn't lost.
  • NewBlockPersister logs and skips lane dirs whose names don't parse as the new 80-hex LaneID (old 64-hex pubkey dirs). Those directories are then never reclaimed. Given the PR requires a state wipe this is acceptable, but the skip path should say the entry leaks rather than implying it's handled.
  • LaneVotesRecv.next (subscriptions.go:55) is keyed by LaneID and never has entries removed when dropLanes fires, so it accumulates one entry per historical lane over the node's lifetime. Negligible in size, but easy to clean up alongside dropLanes.
  • 5 suggestion(s)/nit(s) flagged inline on specific lines.

func (s *State) PushBlock(ctx context.Context, p *types.Signed[*types.LaneProposal]) error {
h := p.Msg().Block().Header()
if p.Key() != h.Lane() {
if p.Key() != h.Lane().Validator() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[blocker] PushBlock captures q := inner.blocks[h.Lane()] before ctrl.WaitUntil (l.587–595) and never re-checks the lane afterwards — the same hazard Block, headers, and WaitForLocalCapacity were all restructured to handle in this PR.

After pruneStaleLeavedropLanes removes the lane, inner.persistedBlockStart[h.Lane()] reads back as the zero value, so the predicate becomes h.BlockNumber() <= min(q.next, BlocksPerLane-1):

  • For h.BlockNumber() >= BlocksPerLane the predicate is permanently false. The waiter re-evaluates on every unrelated ctrl.Updated() and only exits when the peer stream's ctx is cancelled.
  • For low block numbers the predicate passes and the block is pushed into a queue that is no longer reachable from inner.blocks — silently dropped, plus a spurious ctrl.Updated().

Move the inner.blocks[h.Lane()] lookup into the predicate (return true when absent) and re-check for ErrBadLane after the wait, mirroring WaitForLocalCapacity. (Also raised by Codex.)

q.pushBack(newBlockVotes())
}
if _, ok := q.q[h.BlockNumber()].pushVote(inner.epoch, vote); ok {
if _, ok := q.q[h.BlockNumber()].pushVote(inner.epoch.Load(), vote); ok {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[blocker] Same defect as PushBlock: q := inner.votes[h.Lane()] is captured at l.640 before ctrl.WaitUntil (l.644–648), and dropLanes can remove the lane while the caller waits. inner.persistedBlockStart[h.Lane()] then reads 0, so the predicate h.BlockNumber() < 0 + BlocksPerLane is permanently false for any block at or above BlocksPerLane — the waiter never exits until ctx cancellation — and below that it appends votes to a detached queue.

Fold the lane lookup into the predicate and return ErrBadLane after the wait when the lane is gone.

}

func TestPrepareQCVerifyChecksWeight(t *testing.T) {
func TestCommitQCVerifyChecksWeight(t *testing.T) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[blocker] This is the old TestPrepareQCVerifyChecksWeight rewritten to CommitQC, while the pre-existing TestCommitQCVerifyChecksWeight was deleted below — so this is a rename, not new coverage, and the PrepareQC weight check is now untested. Together with the deletions of TestPrepareQCVerifyChecksEpochBinding, TestAppQCVerifyChecksWeight, and TestNewCommittee_RejectsEmptyWeights, PrepareQC.Verify has no test left anywhere in this package. None of these depend on LaneID; please restore them.

return err
}

g, gctx := errgroup.WithContext(ctx)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[suggestion] sei-tendermint/AGENTS.md says to use libs/utils/scope.Run for structured concurrency; errgroup appears nowhere else under internal/autobahn and is introduced only here. scope.Run + scope.SpawnBg (which the new test in this same PR already uses) expresses "run the session until this background watcher fires" directly, and avoids returning context.Canceled as a control-flow sentinel from the WaitMustStop goroutine — a value that then has to be unwound by utils.IgnoreCancel(g.Wait()) and by sessionOpErr.

// so concurrent calls on the same lane serialize correctly.
func (bp *BlockPersister) MaybePruneAndPersistLane(
lane types.LaneID,
active *types.Committee,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[suggestion] Threading a whole *types.Committee into the WAL layer only to compute allowCreate := active.HasLane(lane) || len(proposals) > 0 pushes membership policy down into persistence. An allowCreate bool (decided in avail, which already holds the applied committee) would keep the decision at the layer that owns it, remove the nil-active panic hazard on active.HasLane in an exported API, and drop the committeeForLane scaffolding that ~12 test call sites now need.

anchorCommittee = ep.Committee()
}
for lane := range l.blocks {
if anchorCommittee != nil && lane.EJoin() <= anchorEpoch && !anchorCommittee.HasLane(lane) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[nit] This uses lane.EJoin() <= anchorEpoch while staleLaneDisposable (state.go:121) uses lane.EJoin() < ep.EpochIndex() for what the comments describe as the same predicate. They're equivalent given the invariant that e_join == e implies membership in epoch e, but the asymmetry looks like a typo. Align them or state the invariant that makes <= safe here.

blocksByLane[lane] = append(blocksByLane[lane], proposal)
}

active := s.epoch.Load().Committee()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[nit] active is read outside the inner lock, while batch was collected under it. ApplyEpoch can install a newer committee in between, flipping allowCreate for a lane whose blocks were collected against the older one. Benign today (worst case is a WAL that isn't created and gets deleted anyway), but snapshotting the committee inside collectPersistBatch alongside tipEpoch would make the batch self-consistent.

vs := slices.Collect(maps.Keys(weights))
slices.SortFunc(vs, PublicKey.Compare)
for _, v := range vs {
lanes = append(lanes, NewLaneID(v, GenEpochIndex(rng)))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[nit] Giving each member an independent random e_join produces committees that cannot occur in production — e_join can exceed the index of the epoch the committee is installed in. That's fine for round-tripping LaneIDConv, but it means staleLaneDisposable-style logic is never meaningfully exercised by any test built on GenCommittee, and it quietly breaks the e_join <= epochIndex invariant. Consider taking the epoch as a parameter (or bounding e_join by it) and keeping a separate generator for the deliberately-mixed case.

Comment thread sei-tendermint/internal/autobahn/avail/state.go Outdated

@seidroid seidroid Bot 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.

Solid, well-tested refactor of LaneID from PublicKey to (validator, e_join), with coherent leave/rejoin lifecycle across avail, persist, producer, and giga; the wire/state break is explicitly scoped and the wireguard MaxSize bumps line up with the RPC limits. No blocking correctness defects found — the main issues are an acknowledged-but-permanent leave-WAL leak on restart, a forward-looking startup dependency on registry epoch history that isn't persisted yet, and a few idiom/comment-style deviations from AGENTS.md.

Findings: 0 blocking | 11 non-blocking | 7 posted inline

Blockers

  • None at the file/PR level.

Non-blocking

  • Cursor's second-opinion pass (cursor-review.md) is empty — no findings were merged from it. Codex's single P2 finding is included below (inner.go stale-lane WAL leak) and I confirmed it.
  • Operator-facing hard break (WAL dirs hex(pubkey)hex(pubkey||e_join), BlockHeader/LaneRange wire change, block-hash change) is documented only in the PR description. Consider recording it in a package doc comment or sei-tendermint/AGENTS.md so the "wipe persistent_state_dir before upgrade" requirement survives the PR being merged.
  • Client-side asymmetry: the PR makes the giga server pause-and-resubscribe on lane pruning without tearing down peer RPC, but clientStreamLaneProposals (giga/avail.go:191) still bubbles any PushBlock error — including ErrBadLane from transient epoch skew between peers — which drops the whole connection and forces a DialInterval redial. Worth revisiting when #3736 wires real epoch transitions.
  • Several new comments are dense internal shorthand ("back-leash", "tipcut", "leave map", "avail passes true") that a reader new to the epoch design can't follow top-to-bottom, which is what AGENTS.md §Structural corrections asks the step name + doc comment to carry. state.go:125 and state.go:136 also start the sentence with a capitalized verb mid-phrase ("deleteStaleLaneWAL Deletes WALs…"); godoc convention is "deleteStaleLaneWAL deletes …".
  • 7 suggestion(s)/nit(s) flagged inline on specific lines.

Comment thread sei-tendermint/internal/autobahn/avail/inner.go Outdated
anchorEpoch = anchor.CommitQC.Proposal().EpochIndex()
ep, ok := registry.EpochByIndex(anchorEpoch)
if !ok {
return nil, fmt.Errorf("unknown epoch_index %d for prune anchor", anchorEpoch)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[suggestion] This turns a previously-tolerant path into a hard startup failure that depends on state the node does not persist yet. Before this PR, prune was called with epoch.Committee() (latest) regardless; now an anchor whose EpochIndex() is absent from the registry aborts NewState. tipEpochOf (state.go:109-111) has the same shape and kills runPersist/avail.Run.

NewRegistry seeds only epoch 0 and there is no epoch-history persistence, so once #3736 wires real transitions, any node restarting after an epoch bump will have an anchor at epoch N>0 and refuse to start. Harmless today (anchors are always epoch 0), but please make "registry epoch history must be rebuilt before NewState" an explicit prerequisite in #3736 — right now nothing in the tree records the coupling.

Comment thread sei-tendermint/internal/autobahn/avail/inner.go Outdated
if err != nil {
if errors.Is(err, avail.ErrLanePruned) {
logger.Info("StreamLaneProposals: leave-lane tipcut pruned; pausing until resubscribe")
first = 0

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[nit] Two things on the resubscribe path:

  1. No backoff. If ErrLanePruned were ever to recur immediately after resubscribe, this is a tight loop that also emits an Info log per iteration. My reading is that it isn't reachable today (ApplyEpoch adds the map before Store, and dropLanes only touches lanes absent from the tip committee), but the comment on subscriptions.go explicitly admits a "DeleteLane race" as a source — if that race is real, this spins.

  2. first = 0 replays from block 0. For a genuine rejoin that's correct (new LaneID starts at 0). For the race case, LaneProposalsRecv.Recv walks types.ErrPruned one block at a time up to q.first, taking the inner lock per step. Cheap on a fresh lane, O(pruned height) otherwise.

Comment thread sei-tendermint/internal/autobahn/producer/state.go Outdated
}
cur, hasLane := m.lane.Get()
if !hasLane || cur != local {
return nil, ErrNotProducing

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[nit] There's a window on rejoin where LocalLane() is Some(newLane) but m.lane is still None — between WaitProduce returning in Run and alignMempool setting it in produceSession. Inserts land here and get ErrNotProducing even though the node is a producer, so an RPC submitter sees a spurious rejection rather than a brief wait.

The distinguishable cases are worth splitting: LocalLane() is None → genuinely not producing, reject; LocalLane() is Some but m.lane doesn't match yet → wait on ctrl for alignment. (NewState seeds m.lane from LocalLane(), so this only bites after a clearMempool, not at boot.)

Comment thread sei-tendermint/internal/autobahn/consensus/persist/blocks.go Outdated
The Lock range variable must not reuse m; produce waits on the session
mempoolInner returned by alignMempool.

Co-authored-by: Cursor <cursoragent@cursor.com>
@wen-coding
wen-coding force-pushed the wen/lane_id_in_epoch branch from 4c990c0 to fefd1a1 Compare August 13, 2026 14:25
func hasClosedLane(inner *inner, ds *data.State) bool {
epOfFirst, err := epochOfFirst(inner, ds)
if err != nil {
return false

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Closed-lane cleanup can stall persist

Medium Severity

When the first retained CommitQC is gone, epochOfFirst treats a missing registry epoch as absent rather than failing, unlike the in-memory roads path. hasClosedLane also maps that error to false, so collectPersistBatch can wait indefinitely instead of aborting, and closed-lane maps and WALs are never dropped.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit fefd1a1. Configure here.

func hasClosedLane(inner *inner, ds *data.State) bool {
epOfFirst, err := epochOfFirst(inner, ds)
if err != nil {
return false

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Closed-lane cleanup can stall persist

Medium Severity

When the first retained CommitQC is gone, epochOfFirst treats a missing registry epoch as absent rather than failing, unlike the in-memory roads path. hasClosedLane also maps that error to false, so collectPersistBatch can wait indefinitely instead of aborting, and closed-lane maps and WALs are never dropped.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit fefd1a1. Configure here.

wen-coding and others added 6 commits August 13, 2026 08:25
Rename WaitForNextLane / ErrLaneClosed / setNextBlockToPersist, document
future-lane behavior, simplify giga client wait on current membership,
and tidy producer/avail nits from review.

Co-authored-by: Cursor <cursoragent@cursor.com>
Use roads[first].epoch when retained CommitQCs exist, otherwise
inner.epoch — no registry lookup or Option error path.

Co-authored-by: Cursor <cursoragent@cursor.com>
Deletion follows the Anchor epoch: prune takes that epoch from the
matching road (registry once at startup), and collectPersistBatch no
longer wakes or drops on hasClosedLane.

Co-authored-by: Cursor <cursoragent@cursor.com>
Accept LaneVotes under applied or Anchor; weigh LaneQCs under applied.
Pass the road epoch into headers and abort on IsClosed for that epoch.

Co-authored-by: Cursor <cursoragent@cursor.com>
PruneAndPersist matches main again (BlockNumber, always create). runEvict
only drops maps; runPersist SyncLanes is the sole runtime WAL deleter.

Co-authored-by: Cursor <cursoragent@cursor.com>
Record LaneQC vs FullCommitQC vote retention, note WaitForCapacity does
not wait on future lanes, and drop design history from the Consensus RPC.

Co-authored-by: Cursor <cursoragent@cursor.com>
}
inner.prune(anchor, r.epoch)
}
ctrl.Updated()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Evict dies when Anchor road is missing

High Severity

runEvict now requires inner.roads.q[idx] and returns a fatal error when the data Anchor sits at or beyond an empty tip (first == next after a tip prune, or data has moved ahead of avail). That used to fast-forward via prune and resolve the epoch from the registry, like newInner still does. The error tears down State.Run, so closed-lane maps are never dropped and subscribers never see ErrLaneClosed.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 95a9783. Configure here.

State.mempool is an atomic.Pointer to a per-session Watch[*mempoolInner]
with a closed flag, so produce/prune no longer use lock-protected state
outside Lock. insertTx waits only on full/closed.

Co-authored-by: Cursor <cursoragent@cursor.com>
}
// Session lane must still be this node's LocalLane.
if loc, ok := s.consensus.Avail().LocalLane().Get(); !ok || loc != m.lane {
return nil, ErrNotProducing

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

InsertTx no longer waits for alignment

Medium Severity

InsertTx now returns ErrNotProducing as soon as mempool is nil or the loaded session’s lane does not match LocalLane. The previous logic waited, when waitIfFull was set, until a session mempool existed for the current LocalLane. Switching State.mempool to atomic.Pointer removed that wait, so a node that is already in committee can reject txs during startup and leave/rejoin while alignMempool has not yet stored the new session.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit befa73d. Configure here.

wen-coding and others added 2 commits August 13, 2026 11:45
Prefer the sealed road's epoch while it is retained; fall back to the
registry once it is not, so empty-tip / data-ahead Anchors still prune
instead of tearing down State.Run.

Co-authored-by: Cursor <cursoragent@cursor.com>
Use AtomicSend so inserts can wait until a session is published, restoring
the pre-reshape split where InsertTx blocks and TryInsertTx fails fast.

Co-authored-by: Cursor <cursoragent@cursor.com>
@wen-coding wen-coding removed the ai: skip-review Skips Seidroid AI review triggered via Unified CI. label Aug 13, 2026

@cursor cursor Bot 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.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 7ff48f4. Configure here.

if _, ok := s.consensus.Avail().LocalLane().Get(); !wait || !ok {
return nil, ErrNotProducing
}
return s.mempool.Wait(ctx, func(mp *mempool) bool { return mp != nil })

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

InsertTx wait ignores lane leave

Medium Severity

session snapshots LocalLane once, then Waits only for a non-nil mempool. If the node leaves (or production ends) after that check, clearMempool stores nil and waiters never see ErrNotProducingInsertTx blocks until its context is cancelled instead of failing like the leave path that already holds a session.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 7ff48f4. Configure here.

Avoid the peer struct; shuffle ValidatorAddrs keys and look up the
dial addr, matching the review nit.

Co-authored-by: Cursor <cursoragent@cursor.com>

// anchorEpochOf returns the epoch of anchor's CommitQC, taken from the road it
// sealed while that road is retained and from the registry once it is not.
func (i *inner) anchorEpochOf(registry *epoch.Registry, anchor data.Anchor) (*types.Epoch, error) {

@pompon0 pompon0 Aug 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why not go straight to fallback? My previous comment was referring to the previous semantics, where epoch of the first CommitQC was requested, which is not the same as anchor epoch.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ah I see, done

// epoch is the applied (next-CommitQC) epoch. ApplyEpoch is the sole
// writer after construction.
epoch utils.AtomicSend[*types.Epoch]
// anchorEpoch is the epoch of data's Anchor CommitQC.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what if there is no anchor yet?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

doc updated

// Package avail is the Data Availability Plane and Ordered Event Log: lane
// blocks, CommitQC/AppQC buffers, and pruning.
//
// Lane maps and WALs outlive committee membership until the anchor epoch

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this should be State's godoc?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

moved


// headers collects headers for the given range.
func (s *State) headers(ctx context.Context, lr *types.LaneRange) ([]*types.BlockHeader, error) {
// laneVoteAccepted reports whether vote verifies under ep's committee.

@pompon0 pompon0 Aug 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

add a TODO to actually return a meaningful validation error, in case vote does not match either epoch, and in case verification fails despite the epoch matching

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

// consensus state to which published blocks will be reported.
cfg *Config
app *proxy.Proxy
mempool utils.AtomicSend[*mempool] // nil when not producing

@pompon0 pompon0 Aug 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AtomicSend[Option[*mempool]]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

// session returns the produce session to insert into. With wait set it blocks
// until a session is published, so that inserts racing the start of production
// are admitted rather than rejected.
func (s *State) session(ctx context.Context, wait bool) (*mempool, error) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

getMempool?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

actually it doesn't even make sense to call a wrapper in case wait == false, so perhaps inline?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

if m.closed {
return nil, ErrNotProducing
}
// Session lane must still be this node's LocalLane.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we really need this check?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed

if m.closed {
return nil, ErrNotProducing
}
if loc, ok := s.consensus.Avail().LocalLane().Get(); !ok || loc != m.lane {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ditto. We just grabbed a mempool and we should be good to go while at it. It is not like we double confirm if the shard matches.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed

wen-coding and others added 3 commits August 13, 2026 14:04
session() wait ends if LocalLane is gone, and clearMempool always
publishes nil so waiters re-evaluate instead of hanging until ctx cancel.

Co-authored-by: Cursor <cursoragent@cursor.com>
Move lane-lifecycle notes onto State (keeping the original godoc) and
TODO a real validation error for dual-epoch PushVote rejects.

Co-authored-by: Cursor <cursoragent@cursor.com>
Publish produce sessions as AtomicSend[Option]; wait only via getMempool.
Hoist headers IsClosed out of the wait loop under a fixed epoch.

Co-authored-by: Cursor <cursoragent@cursor.com>
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.

3 participants