Xdc : Add XdcTestBlockchain for Module testing#9596
Conversation
dont seal
There was a problem hiding this comment.
Pull Request Overview
This PR adds XdcTestBlockchain infrastructure for module testing and includes significant refactoring of the XDC consensus implementation. The changes improve testability, fix several bugs, and restructure the XDC module architecture.
Key changes:
- Introduced
XdcTestBlockchainand related test helpers for comprehensive XDC testing - Refactored
SnapshotManager.GetSnapshot()to accept block number and spec instead of hash - Changed
SwitchBlocktype fromUInt256tolongfor consistency - Fixed multiple issues in consensus logic, voting, and timeout handling
- Enhanced error handling and logging in
QuorumCertificateManager
Reviewed Changes
Copilot reviewed 73 out of 73 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| XdcModule.cs | Moved from XdcPlugin.cs; added new service registrations for BlockProcessor, GenesisBuilder, and managers |
| XdcPlugin.cs | Removed XdcModule class; added MustInitialize property |
| XdcTestBlockchain.cs | New comprehensive test blockchain implementation with XDC consensus support |
| SnapshotManager.cs | Refactored GetSnapshot to use block number instead of hash; simplified constructor |
| XdcHotStuff.cs | Multiple bug fixes in voting, round logic, and leader selection |
| XdcBlockHeader.cs | Added FromBlockHeader factory method; improved ExtraConsensusData caching |
| XdcGenesisBuilder.cs | New genesis builder with XDC-specific initialization |
| QuorumCertificateManager.cs | Enhanced error handling with detailed error messages |
| Spec/XdcReleaseSpec.cs | Changed SwitchBlock type to long; added GenesisMasterNodes and FromReleaseSpec |
| Test files | Added comprehensive test coverage for reorg scenarios, blockchain operations |
| Core changes | Made BlockProcessor methods virtual/protected for inheritance; fixed BasicTestBlockchain |
| if (_extraFieldsV2 is not null) | ||
| { | ||
| return _extraFieldsV2; | ||
| } | ||
|
|
||
| if (ExtraData is null || ExtraData.Length == 0) | ||
| return null; | ||
|
|
||
| if (_extraFieldsV2 == null) | ||
| { | ||
| //Check V2 consensus version in ExtraData field. | ||
| if (ExtraData.Length < 3 || ExtraData[0] != XdcConstants.ConsensusVersion) | ||
| return null; | ||
| Rlp.ValueDecoderContext valueDecoderContext = new Rlp.ValueDecoderContext(ExtraData.AsSpan(1)); | ||
| _extraFieldsV2 = _extraConsensusDataDecoder.Decode(ref valueDecoderContext); | ||
| } | ||
| //Check V2 consensus version in ExtraData field. | ||
| if (ExtraData.Length < 3 || ExtraData[0] != XdcConstants.ConsensusVersion) | ||
| return null; | ||
| Rlp.ValueDecoderContext valueDecoderContext = new Rlp.ValueDecoderContext(ExtraData.AsSpan(1)); | ||
| _extraFieldsV2 = _extraConsensusDataDecoder.Decode(ref valueDecoderContext); |
There was a problem hiding this comment.
[nitpick] The early return for cached _extraFieldsV2 at the beginning is good, but the original code checked for null and then decoded. The new code extracts the check out, which improves readability. However, consider adding thread-safety if this property can be accessed concurrently, as the decode operation could happen multiple times.
| var xdcHeader = _blockTree.Head?.Header as XdcBlockHeader; | ||
| IXdcReleaseSpec spec = _specProvider.GetXdcSpec(xdcHeader, xdcHeader.ExtraConsensusData.BlockRound); |
There was a problem hiding this comment.
Potential null reference exception: xdcHeader could be null from the cast, but line 189 accesses xdcHeader.ExtraConsensusData without null checking. Add a null check before accessing properties.
| XdcBlockHeader xdcHeader = _blockTree.Head?.Header as XdcBlockHeader; | ||
| IXdcReleaseSpec spec = _specProvider.GetXdcSpec(xdcHeader, timeout.Round); |
There was a problem hiding this comment.
Potential null reference exception: xdcHeader could be null from the cast, but is passed directly to GetXdcSpec without null checking. Add appropriate null validation before use.
| result.Should().BeEquivalentTo(snapshot); | ||
| } | ||
| } | ||
| //// SPDX-FileCopyrightText: 2025 Demerzel Solutions Limited |
There was a problem hiding this comment.
The entire test file has been commented out rather than removed or updated. This creates technical debt and should either be updated to work with the new SnapshotManager API or removed entirely. Commented-out code should not be committed.
ak88
left a comment
There was a problem hiding this comment.
Need to revisit how headers can be made more flexible at some point
* Change IPrecompile to use Result<byte[]> allowing to return errors from precompiles (#9561) * Change IPrecompile to use Result<byte[]> * whitespace * fix benchmark * fix tests * Move back DataGasCost to simple long * Some fixes * fixes * refactor * expose precompile error from EVM * ModExpPrecompile should fail on run not on DataGasCost * fixes * Fix tests and bubble up precompile errors * fix build * Another try * Error(2) * Choose substate error * fixes * Simplify handling errors * fix PairingCheckPrecompile * Add syntatic suggar, while preserving lazy evaluation * fix --------- Co-authored-by: lukasz.rozmej <lukasz.rozmej@gmail.com> * Update outdated link (#9654) * Feature/xdc prevent finalized reorg (#9655) * override in blocktree * fix * unittest * format * fix * Fix * remove test * cleanup * Do not insert block if invalid fork * format * review comments * format * Update src/Nethermind/Nethermind.Xdc/XdcBlockTree.cs Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com> --------- Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com> * fix: DynamicCallWithInput to skip value for DELEGATECALL/STATICCALL (#9672) * fix: DynamicCallWithInput to skip value for DELEGATECALL/STATICCALL * update test * Route ColumnDb.Remove through Set(null) to use wrapper semantics (#9636) * Changes for test blockchain (#9680) * test blockchain changes * format * Improve diag Docker container (#9635) * Fix bump-version action and adds reviewers as default (#9557) * feat: add reviewers to the bump-version action * feat: use checkout action * Fix free disk space check for non existent directories (HealthChecks) (#9666) * Create non existent db directory to get drive info * Add db folder to ignored to pass tests * Update OP Superchain chains (#9675) Co-authored-by: emlautarom1 <emlautarom1@users.noreply.github.com> Co-authored-by: Lautaro Emanuel <31224949+emlautarom1@users.noreply.github.com> * Remove Nethermind.Analytics (#9683) * Feature/block production blob limit (#9686) * Introduce BlockProductionBlobLimit to limit number of blobs picked when producing block * Add MaxBlobCount calculation tests * whitespace * Rename * Update src/Nethermind/Nethermind.Config/IBlocksConfig.cs Co-authored-by: Ahmad Bitar <33181301+smartprogrammer93@users.noreply.github.com> --------- Co-authored-by: Ahmad Bitar <33181301+smartprogrammer93@users.noreply.github.com> * Fix resource leak in HttpJsonRpcSubmitter (#9687) Update HttpJsonRpcSubmitter.cs * Remove unused BlockFilter.StartBlockNumber and simplify CreateBlockFilter (#9630) * Remove unused BlockFilter.StartBlockNumber and simplify CreateBlockFilter * fix ci * formatting * Revert "Improve Block related caching (#9571)" (#9688) * Revert "Improve Block related caching (#9571)" This reverts commit fe8adcc. * merge fix * Update Dockerfiles (#9689) Co-authored-by: rubo <rubo@users.noreply.github.com> * Fix Chainspec for Taiko-Hoodi (#9699) fix transition for taiko-hoodi * Xdc : Add XdcTestBlockchain for Module testing (#9596) * start * more dependencies * build * rename * voting * vote task * bit of refactor * invoke block produced dont seal * refactor context * format * timeout missing * block producer * format * basic incomplete setup * test * block production test * format * merge cleanup * merged * use the constant * use timestamp from attributes * Merged * Merged fix * cleanup * fixes * cleanup * commit cert before vote propose in background * merge and refactor * rename * format * comment * refactor to continuous loop * remove TCM * Format * fix * draft implmentation * cleanup * handle regenesis block in XdcTestBlockchain * refactor to inherit from TestBlockchain * register FromXdcContainer * handle _fromContainer from base * attempt rotating signers * test fix * test fix * test and container setup fix * decoder fix * bug fixes * init QC * test xdc sealer rotation * bug fixes and working now * Format * Update packages.lock.json * remove * total difficulty fix * format * override in blocktree * fix * unittest * format * fix build issues * fix * Fix * remove test * fixes * format * cleanup * undo merge and cleanup usings * add custom genesis builder for xdc * format whitespaces * Do not insert block if invalid fork * format * reorg test * revert block processor * revert cloning of header in PrepareBlockForProcessing * reorgs tests working * fix bad db * fix seal validator * fix * format * implement add block without commiting qc and generate extra keys for testing * fix and refactor * optional params * fix * one fix and a bit of refactor * reorg test * rename * return block instead * reorg test * fixes * dont set author * 10 seconds per add block * test blockchain changes * format * fixes for hotstuff module * fix test block producer * fixes for epoch master calculation * format * rename * rollback QC test blockchain order * snapshotmanager tests fixed * format * cleanup * remove TestBlockchainTester * cleanup * small fix in header validator test --------- Co-authored-by: ak88 <anders.holmbjerg@hotmail.com> Co-authored-by: cicr99 <kr1000a@gmail.com> * XDC - Snapshot Manager refactor (#9695) refactor of snapshot manager * Xdc : Port BlockInfoValidator Module Tests (#9650) * start * more dependencies * build * rename * voting * vote task * bit of refactor * invoke block produced dont seal * refactor context * format * timeout missing * block producer * format * basic incomplete setup * test * block production test * format * merge cleanup * merged * use the constant * use timestamp from attributes * Merged * Merged fix * cleanup * fixes * cleanup * commit cert before vote propose in background * merge and refactor * rename * format * comment * refactor to continuous loop * remove TCM * Format * fix * draft implmentation * cleanup * handle regenesis block in XdcTestBlockchain * refactor to inherit from TestBlockchain * register FromXdcContainer * handle _fromContainer from base * attempt rotating signers * test fix * test fix * test and container setup fix * decoder fix * bug fixes * init QC * test xdc sealer rotation * bug fixes and working now * Format * Update packages.lock.json * remove * draft impl of blockinfo validator tests * total difficulty fix * format * override in blocktree * fix * unittest * format * fix build issues * fix * Fix * remove test * fixes * format * cleanup * undo merge and cleanup usings * add custom genesis builder for xdc * format whitespaces * Do not insert block if invalid fork * format * reorg test * revert block processor * revert cloning of header in PrepareBlockForProcessing * reorgs tests working * fix bad db * fix seal validator * fix * format * implement add block without commiting qc and generate extra keys for testing * fix and refactor * optional params * fix * one fix and a bit of refactor * reorg test * rename * return block instead * reorg test * fixes * fix failing test * dont set author * 10 seconds per add block * test blockchain changes * format * ws fixes * fixes for hotstuff module * fix test block producer * fixes for epoch master calculation * format * rename * rollback QC test blockchain order * CLEANUP * remove unused --------- Co-authored-by: ak88 <anders.holmbjerg@hotmail.com> Co-authored-by: cicr99 <kr1000a@gmail.com> * Unit tests for commit in XDC QC manager (#9700) unittests * Xdc : Port HeaderValidator Module Tests (#9617) * start * more dependencies * build * rename * voting * vote task * bit of refactor * invoke block produced dont seal * refactor context * format * timeout missing * block producer * format * basic incomplete setup * test * block production test * format * merge cleanup * merged * use the constant * use timestamp from attributes * Merged * Merged fix * cleanup * fixes * cleanup * commit cert before vote propose in background * merge and refactor * rename * format * comment * refactor to continuous loop * remove TCM * Format * fix * draft implmentation * cleanup * handle regenesis block in XdcTestBlockchain * refactor to inherit from TestBlockchain * register FromXdcContainer * handle _fromContainer from base * attempt rotating signers * test fix * test fix * test and container setup fix * draft port of some HeaderValidatorTests * fix build issue * extra tests * decoder fix * handle merge issues * bug fixes * init QC * test xdc sealer rotation * bug fixes and working now * Format * Update packages.lock.json * remove * fix build issues * more build fixes * total difficulty fix * format * override in blocktree * fix * unittest * format * fix build issues * fix * Fix * remove test * fixes * format * cleanup * undo merge and cleanup usings * add custom genesis builder for xdc * format whitespaces * Do not insert block if invalid fork * format * reorg test * revert block processor * revert cloning of header in PrepareBlockForProcessing * reorgs tests working * fix bad db * fix seal validator * fix * format * implement add block without commiting qc and generate extra keys for testing * fix and refactor * optional params * fix * one fix and a bit of refactor * reorg test * rename * return block instead * minor changes and fixes * reorg test * fixes * bring back cache field in sealValidator * dont set author * 10 seconds per add block * test blockchain changes * format * ws fixes * fixes for hotstuff module * fix test block producer * fixes for epoch master calculation * format * rename * rollback QC test blockchain order * cleanup --------- Co-authored-by: ak88 <anders.holmbjerg@hotmail.com> Co-authored-by: cicr99 <kr1000a@gmail.com> * Update base-sepolia testnet matrix (#9705) chore: update base-sepolia testnet matrix * Comment out Linea in workflows (#9706) chore: comment linea on our workflows --------- Co-authored-by: Alexey Osipov <me@flcl.me> Co-authored-by: lukasz.rozmej <lukasz.rozmej@gmail.com> Co-authored-by: Jaye Turner <jayeturnerjatu@gmail.com> Co-authored-by: ak88 <anders@nethermind.io> Co-authored-by: Snezhkko <snezhkodaria38@gmail.com> Co-authored-by: radik878 <radikpadik76@gmail.com> Co-authored-by: Ruben Buniatyan <rubo@users.noreply.github.com> Co-authored-by: Marcos Antonio Maceo <35319980+stdevMac@users.noreply.github.com> Co-authored-by: Damian Orzechowski <114909782+damian-orzechowski@users.noreply.github.com> Co-authored-by: core-repository-dispatch-app[bot] <173070810+core-repository-dispatch-app[bot]@users.noreply.github.com> Co-authored-by: emlautarom1 <emlautarom1@users.noreply.github.com> Co-authored-by: Lautaro Emanuel <31224949+emlautarom1@users.noreply.github.com> Co-authored-by: Ahmad Bitar <33181301+smartprogrammer93@users.noreply.github.com> Co-authored-by: maradini77 <140460067+maradini77@users.noreply.github.com> Co-authored-by: GarmashAlex <garmasholeksii@gmail.com> Co-authored-by: Ben {chmark} Adams <thundercat@illyriad.co.uk> Co-authored-by: Diptanshu Kakwani <dipkakwani@gmail.com> Co-authored-by: Ayman Bouchareb <Ayman.bouchareb@outlook.fr> Co-authored-by: ak88 <anders.holmbjerg@hotmail.com> Co-authored-by: cicr99 <kr1000a@gmail.com> Co-authored-by: Carmen Irene Cabrera Rodríguez <49727740+cicr99@users.noreply.github.com>
Adds a lot of fixes and refactor for XDC.
Some Changes to NMC modules, for inherited behavior:
src/Nethermind/Nethermind.Consensus/Processing/BlockProcessor.cs
src/Nethermind/Nethermind.Consensus/Producers/BlockProducerBase.cs
Also fixes an issue with BasicTestBlockchain unrelated to XDC.