refactor(stages): extract IncrementalSettings struct from MerkleStage#20698
Closed
meetrick wants to merge 2 commits intoparadigmxyz:mainfrom
Closed
refactor(stages): extract IncrementalSettings struct from MerkleStage#20698meetrick wants to merge 2 commits intoparadigmxyz:mainfrom
meetrick wants to merge 2 commits intoparadigmxyz:mainfrom
Conversation
Extract `rebuild_threshold` and `incremental_threshold` fields into a shared `IncrementalSettings` struct to reduce duplication between `Execution` and `Both` variants. Signed-off-by: Hwangjae Lee <meetrick@gmail.com>
meetrick
force-pushed
the
refactor/merkle-incremental-settings
branch
from
61385d2 to
271d724
Compare
Signed-off-by: Hwangjae Lee <meetrick@gmail.com>
github-actions
bot
added
the
S-stale
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
refactor(stages): extract IncrementalSettings struct from MerkleStage
Problem
The
MerkleStageenum duplicated therebuild_thresholdandincremental_thresholdfields across both theExecutionandBothvariants. This required making the same changes in multiple places when modifying threshold-related behavior, increasing maintenance overhead and the risk of inconsistencies.Analysis
Both variants use these thresholds in exactly the same way inside
execute(): they determine when to rebuild the trie versus apply incremental updates, and they control the chunk size in incremental mode. Theunwind()path does not reference these values at all. The only behavioral difference between the variants is thatExecutionskips unwind, whileBothsupports both execution and unwind. No behavioral changes are introduced by this refactor, making it safe to consolidate the duplicated fields.Solution
Introduce a new
IncrementalSettingsstruct that encapsulates both threshold fields and provides aDefaultimplementation. The relevantMerkleStagevariants now store this struct instead of duplicating the fields. This refactor is purely structural and does not change stage semantics or public behavior.Changes
crates/stages/stages/src/stages/merkle.rs: addedIncrementalSettings, updated enum variants, and adjusted match patternscrates/cli/commands/src/stage/dump/merkle.rs: updatedMerkleStage::Executionconstructioncrates/stages/stages/benches/criterion.rs: updatedMerkleStage::BothconstructionCode paths using
new_execution()ordefault_unwind()required no changes.Verification
reth-stagespassed