chore(trie): Use trie changesets for engine unwinding #18878
+399
−194
Conversation
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
github-actions
bot
added
A-db
mediocregopher
force-pushed
the
mediocregopher/18517-trie-cs-unwind
branch
from
bdaeb41 to
70d24d4
Compare
mediocregopher
commented
Oct 6, 2025
| self.write_trie_updates_sorted(&trie_revert)?; | ||
|
|
||
| // Clear trie changesets which have been unwound. | ||
| self.clear_trie_changesets_from(*range.start())?; |
There was a problem hiding this comment.
@shekhirin you mentioned in a previous PR that we don't want to have deletion from changesets done on a range, but this method only accepts a range. Do you think it's fine to leave this like this? or better to re-introduce clear_trie_changesets_from?
mediocregopher
requested review from
rkrasiuk,
Rjected,
shekhirin,
joshieDo,
rakita and
mattsse
as code owners
Rjected
approved these changes
Oct 6, 2025
Comment on lines
+357
to
+358
| let trie_revert = self.trie_reverts(range.clone())?; | ||
| self.write_trie_updates_sorted(&trie_revert)?; |
There was a problem hiding this comment.
nice, i like that this is now just a call to a provider fn
| /// | ||
| /// Returns the number of entries modified. | ||
| fn write_trie_updates_sorted(&self, trie_updates: &TrieUpdatesSorted) -> ProviderResult<usize> { | ||
| if trie_updates.account_nodes.is_empty() && trie_updates.storage_tries.is_empty() { |
There was a problem hiding this comment.
could probably just introduce TrieUpdatesSorted::is_empty
Comment on lines
+62
to
+90
| // Sanity check that we have static files available to unwind to the target block. | ||
| let highest_static_file_block = | ||
| provider_factory.static_file_provider().get_highest_static_files().max_block_num(); | ||
| if highest_static_file_block | ||
| .filter(|highest_static_file_block| *highest_static_file_block > target) | ||
| .is_none() | ||
| { | ||
| return Err(eyre::eyre!("static files not available for target block {target}, highest is {highest_static_file_block:?}")); | ||
| } | ||
|
|
||
| // Move all applicable data from database to static files. | ||
| pipeline.move_to_static_files()?; | ||
| if self.offline { | ||
| info!(target: "reth::cli", "Performing an unwind for offline-only data!"); | ||
| } | ||
|
|
||
| pipeline.unwind(target, None)?; | ||
| if let Some(highest_static_file_block) = highest_static_file_block { | ||
| info!(target: "reth::cli", ?target, ?highest_static_file_block, "Executing a pipeline unwind."); | ||
| } else { | ||
| info!(target: "reth::cli", ?target, "Executing a database unwind."); | ||
| let provider = provider_factory.provider_rw()?; | ||
| info!(target: "reth::cli", ?target, "Executing a pipeline unwind."); | ||
| } | ||
| info!(target: "reth::cli", prune_config=?config.prune, "Using prune settings"); | ||
|
|
||
| provider | ||
| .remove_block_and_execution_above(target) | ||
| .map_err(|err| eyre::eyre!("Transaction error on unwind: {err}"))?; | ||
| // This will build an offline-only pipeline if the `offline` flag is enabled | ||
| let mut pipeline = | ||
| self.build_pipeline(config, provider_factory, components.evm_config().clone())?; | ||
|
|
||
| // update finalized block if needed | ||
| let last_saved_finalized_block_number = provider.last_finalized_block_number()?; | ||
| if last_saved_finalized_block_number.is_none_or(|f| f > target) { | ||
| provider.save_finalized_block_number(target)?; | ||
| } | ||
| // Move all applicable data from database to static files. | ||
| pipeline.move_to_static_files()?; | ||
|
|
||
| provider.commit()?; | ||
| } | ||
| pipeline.unwind(target, None)?; |
There was a problem hiding this comment.
is the reasoning behind this, that currently we pretty much always have static files and will now always be doing a pipeline unwind?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #18517
This PR primarily targets the
unwind_trie_state_rangeprovider method, which is used by the engine API to unwind the db during reorgs.This method was previously also potentially used by the unwind pipeline stage, but only in an
elseblock which should never get hit anymore. This else block was replaced with an error.unwind_trie_state_rangewas previously recomputing trie updates using a StateRoot calculation, but it is now able to skip that step and simply read trie updates from the trie changeset tables instead. To make applying these updates easier thewrite_trie_updatesmethod is superceded bywrite_trie_updates_sorted.