fix(consensus/XDPoS,core): stop fabricating verify batch blocks, close #2253#2298
fix(consensus/XDPoS,core): stop fabricating verify batch blocks, close #2253#2298gzliudan wants to merge 1 commit intoXinFinOrg:dev-upgradefrom
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 |
There was a problem hiding this comment.
Pull request overview
Fixes XDPoS header-batch verification so consensus code can’t observe “fabricated” empty blocks for in-flight headers during VerifyHeaders, while still allowing full block imports to provide real batch blocks for GetBlock lookups.
Changes:
- Introduces an XDPoS verify-batch
ChainReaderwrapper that shadows headers but does not fabricate placeholder blocks (unless real blocks are explicitly provided). - Wires the wrapper into header-only verification (
HeaderChain.ValidateHeaderChain) and full block imports (BlockChain.insertChain) with engine-specific handling for XDPoS. - Adds unit tests for the verify-batch reader and an engine v2 regression test to ensure
GetBlockdoesn’t return fabricated batch blocks during header verification (HookPenalty path).
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 | Uses an XDPoS-specific verify-batch ChainReader when validating header chains. |
| core/blockchain.go | Passes real batch blocks to the verify-batch ChainReader during full block imports. |
| consensus/XDPoS/XDPoS.go | Switches to the new exported verify-batch reader constructor. |
| consensus/XDPoS/verify_chain_reader.go | Stops creating header-only placeholder blocks; optionally exposes real blocks when provided. |
| consensus/XDPoS/verify_chain_reader_test.go | Expands unit coverage for header shadowing and block lookup behavior. |
| consensus/tests/engine_v2_tests/verify_header_test.go | Adds regression coverage ensuring batch GetBlock lookups don’t see fabricated blocks during verification. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…XinFinOrg#2253 Avoid exposing in-flight verify batch headers as empty placeholder blocks. Wire real batch blocks into full block imports while keeping header-only verification limited to header lookups, so consensus code cannot observe fabricated bodies during VerifyHeaders. Add unit coverage for verifyChainReader and an engine v2 regression test for block lookups during batch verification.
ce1e38d to
7ccdbd8
Compare
Proposed changes
Avoid exposing in-flight verify batch headers as empty placeholder blocks. Wire real batch blocks into full block imports while keeping header-only verification limited to header lookups, so consensus code cannot observe fabricated bodies during VerifyHeaders.
Add unit coverage for verifyChainReader and an engine v2 regression test for block lookups during batch verification.
fix #2253
test: sync testnet from genesis block, passed all blocks
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