fix(consensus): prev hash byron genesis support#1630
Conversation
📝 WalkthroughWalkthroughThe pull request introduces validation logic to require the presence of PrevHeaderHash for non-genesis blocks across the consensus validation code. Changes are made in both the Byron-specific and generic validate modules, with corresponding test cases added to verify the new validation behavior. The validation now returns an explicit error when non-genesis blocks lack a PrevHeaderHash, while genesis blocks remain unaffected. Code formatting adjustments collapse multi-line statements into single-line format without altering functionality. Possibly related issues
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@consensus/byron/validate.go`:
- Around line 231-234: The exported Byron header wrappers
(ValidateByronBlockHeader / ValidateByronMainBlockHeader /
ValidateByronEBBHeader) still assume a non-nil prevHeader and unconditionally
call prevHeader.SlotNumber(), prevHeader.BlockNumber(), and prevHeader.Hash(),
preventing genesis/no-prev validation; update these wrappers to accept a nil
prevHeader when input represents genesis (e.g., PrevBlockNumber == 0 or empty
PrevHeaderHash) and short-circuit any reads from prevHeader (skip
slot/number/hash comparisons) so the lower-level validatePrevHash path can
handle the no-prev case; ensure ValidateHeaderInput can carry a nil PrevHeader
and that callers pass nil instead of a dummy header so validatePrevHash and
related checks operate correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f67780cb-1758-422e-b898-75e49b76650f
📒 Files selected for processing (4)
consensus/byron/validate.goconsensus/byron/validate_test.goconsensus/validate.goconsensus/validate_test.go
Signed-off-by: Chris Gianelloni <wolf31o2@blinklabs.io>
b5c12d1 to
bc42598
Compare
This replaces #1623 and enhances #1628's implementation
Summary by cubic
Fixes previous-hash validation and header handling for Byron genesis transitions. Non-genesis blocks now require a previous header hash; genesis transitions may omit it, with consistent mismatch errors.
consensus/byron, detect genesis viaPrevBlockNumber; requirePrevHeaderHashonly for non-genesis; set prev header fields only whenprevHeader != nilinValidateByronBlockHeader.consensus, enforce the same rule viaBlockNumberand keep a single error message.Written for commit bc42598. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests