fix(consensus/XDPoS,core): fix verifyheaders parent lookups for epoch switches#2276
fix(consensus/XDPoS,core): fix verifyheaders parent lookups for epoch switches#2276gzliudan wants to merge 1 commit intoXinFinOrg:v2.6.x-betafrom
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
… switches Problem: VerifyHeaders could fail on epoch-switch batches when consensus hooks looked up parent headers through ChainReader before the batch had been written to disk. Cause: the import paths passed the raw chain reader into VerifyHeaders, so helper lookups could only see DB-backed state and missed in-flight headers during block and header-only imports. Solution: wrap the VerifyHeaders entry points with a batch-aware verifyChainReader so current-batch headers and blocks are visible during validation, add focused regression tests for epoch-switch parent resolution, and keep wrapper reuse as a no-op instead of mutating an existing cache.
a9a8405 to
ab8dc39
Compare
There was a problem hiding this comment.
Pull request overview
Fixes XDPoS header batch verification during epoch switches by ensuring consensus parent/header lookups can see in-flight (not-yet-persisted) headers/blocks during VerifyHeaders, preventing false unknown ancestor-style failures during imports.
Changes:
- Introduces a batch-aware
consensus.ChainReaderwrapper (verifyChainReader) that shadows header/block lookups with current-batch entries. - Updates block and header-only import paths to pass a batch-aware reader into
VerifyHeadersfor XDPoS. - Adds regression tests covering epoch-switch parent resolution during
VerifyHeadersandInsertHeaderChain.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| core/headerchain.go | Wraps VerifyHeaders calls with a batch-aware reader for XDPoS during header-chain validation. |
| core/blockchain.go | Wraps VerifyHeaders calls with a batch-aware reader (including blocks) for XDPoS during block insertion. |
| consensus/XDPoS/XDPoS.go | Ensures XDPoS VerifyHeaders uses a batch-aware reader so helper lookups can see in-batch headers. |
| consensus/XDPoS/verify_chain_reader.go | Adds verifyChainReader implementation to shadow header/block lookups with batch data. |
| consensus/XDPoS/verify_chain_reader_test.go | Adds unit tests validating wrapper behavior (nil-safety, shadowing, reuse semantics). |
| consensus/tests/engine_v2_tests/verify_chain_reader_test.go | Adds focused regression tests for epoch-switch parent resolution in VerifyHeaders and header-only import. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Proposed changes
Problem: VerifyHeaders could fail on epoch-switch batches when consensus hooks looked up parent headers through ChainReader before the batch had been written to disk.
Cause: the import paths passed the raw chain reader into VerifyHeaders, so helper lookups could only see DB-backed state and missed in-flight headers during block and header-only imports.
Solution: wrap the VerifyHeaders entry points with a batch-aware verifyChainReader so current-batch headers and blocks are visible during validation, add focused regression tests for epoch-switch parent resolution, and keep wrapper reuse as a no-op instead of mutating an existing cache.
Fix: XinFinOrg/XinFin-Node#258
Types of changes
What types of changes does your code introduce to XDC network?
Put an
✅in the boxes that applyImpacted Components
Which parts of the codebase does this PR touch?
Put an
✅in the boxes that applyChecklist
Put an
✅in the boxes once you have confirmed below actions (or provide reasons on not doing so) that