Repository navigation
refactored epoch registry - #4697
Conversation
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4697 +/- ##
==========================================
- Coverage 60.31% 60.31% -0.01%
==========================================
Files 2029 2029
Lines 158351 158352 +1
==========================================
- Hits 95515 95512 -3
- Misses 62831 62835 +4
Partials 5 5
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
PR SummaryHigh Risk Overview Consensus timing moves genesis timestamp off Availability startup is refactored: Mechanical cleanup replaces Reviewed by Cursor Bugbot for commit c5e641d. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
This PR replaces utils.Alloc and the proto scalar-pointer helpers with new(), rebuilds the epoch registry around one immutable persistedRegistry stored as an Epoch with its road range, lets the genesis epochs be pruned, and moves the genesis timestamp into ConsensusSpec. It blocks on one issue: snapshots written by the current release can no longer be read, so any node that already has one fails to start after upgrading. That is codex's finding, which I verified and kept; codex raised nothing else.
Blocking
1 finding on the changed lines, as inline comments.
1 nit, not posted on the code
sei-tendermint/internal/autobahn/avail/inner.go:205— With thefrontierguard gone, logLocalBlock runs after every restored block, but it only ever checks the block at q.first. When that block does not extend the certified tip, the same line is logged once per restored block in the lane, up to BlocksPerLane times.
seidroid review · decision request_changes · session 8b0f848604aa4d298d61598b8b628daf · turn resp_claude_55537b80af335b207d0fc81449d9416c · item e0bfbcee605359719615ac11843d87ed
Findings: 1 blocking | 0 non-blocking | 1 posted inline
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit be3fe31. Configure here.
giga code doesn't need backward compatibility
|
@seidroid review |
There was a problem hiding this comment.
Nothing has changed since my last review: the diff is byte-identical and the head and merge commits are the same. The snapshot-compatibility blocker is still in the code even though its thread is marked resolved and the 'wai' reply gives no reason. codex raised the same point, and I kept it as one finding; codex contributed nothing else.
Blocking
1 finding on the changed lines, as inline comments.
seidroid review · decision request_changes · session 8b0f848604aa4d298d61598b8b628daf · turn resp_claude_11ce188905bf54575380d50ed3a78b48 · item 47e267b0de2c53709a27ffd043caf5b0
Findings: 1 blocking | 0 non-blocking | 1 posted inline
|
@seidroid review |
There was a problem hiding this comment.
The code has not changed since my last review (same diff, same commits); what changed is the author's explicit statement that giga persisted state carries no backward-compatibility promise yet, and the tree supports that: Autobahn only runs when an autobahn-config-file is set, REVIEW.md sets no compatibility rule for it, and the reserved fields and _v2 names show the format break is deliberate. I have downgraded the snapshot-format finding, raised by me and by codex (the only scout, with nothing else to add), from blocker to a suggestion, because the failure is loud at startup rather than silent and nothing else blocks.
Non-blocking
1 finding on the changed lines, as inline comments.
seidroid review · decision approve · session 8b0f848604aa4d298d61598b8b628daf · turn resp_claude_64b8c663dab4ce8e0105c7a83043efd2 · item 0eb44c2d18545af18d331d324e796002
Findings: 0 blocking | 1 non-blocking | 1 posted inline
| if err != nil { | ||
| return nil, fmt.Errorf("pending: %w", err) | ||
| } | ||
| if len(live) == 0 { |
There was a problem hiding this comment.
suggestion — Breaking the format is a deliberate choice, but a node with a pre-upgrade snapshot will fail here with "persisted registry has no live epochs", which reads like corruption rather than a format change. Deleting only epochs/ does not recover either, because the blockstore and avail state still reference epochs ≥2. Consider detecting the legacy fields 1/2 and returning an explicit "incompatible epoch snapshot format; reset Autobahn persistent_state_dir" error, so giga-testnet operators know to reset the whole persistent state. (Also raised by codex.)
Superseded: the latest review found nothing blocking in this change.

Uh oh!
There was an error while loading. Please reload this page.