chore(trie): Remove ExecutedBlockWithTrieUpdates#18919
chore(trie): Remove ExecutedBlockWithTrieUpdates#18919mediocregopher merged 5 commits into18460-trie-changesetsfrom
Conversation
| .get_state(block.header().number())? | ||
| .ok_or_else(|| ProviderError::StateForNumberNotFound(block.header().number()))?; | ||
| let hashed_state = self.provider.hashed_post_state(execution_output.state()); | ||
| let trie_updates = self.provider.get_block_trie_updates(block.number())?; |
There was a problem hiding this comment.
This is where the new db method was required to be used, to properly populate ExecutedBlocks that aren't in memory
| /// block. | ||
| /// 3. Once in-memory blocks are collected and optionally filtered, we compute the | ||
| /// [`HashedPostState`] from them. | ||
| fn compute_trie_input<TP: DBProvider + BlockNumReader>( |
There was a problem hiding this comment.
We had an entire 1:1 copy of this function in this module for some reason. The version in payload_validator.rs remains.
| }; | ||
| input.append(revert_state); | ||
|
|
||
| input.append_cached(revert_trie.into(), revert_state); |
There was a problem hiding this comment.
append_cached specifically does not populate the prefix sets from the revert_state into the trie input. The trie state is fully reverted at this point, so the prefix set is not necessary.
| /// Map of hash to trie updates for canonical blocks that are persisted but not finalized. | ||
| /// | ||
| /// Contains the block number for easy removal. | ||
| pub(crate) persisted_trie_updates: HashMap<B256, (BlockNumber, Arc<TrieUpdates>)>, |
There was a problem hiding this comment.
This was only used (indirectly) by canonical_block_by_hash to construct historical ExecutedBlocks. We have the DB method for that now.
| } | ||
| } | ||
|
|
||
| impl From<TrieUpdatesSorted> for TrieUpdates { |
There was a problem hiding this comment.
Required because TrieInputs still uses TrieUpdates, but trie_reverts outputs a TrieUpdatesSorted. In a later PR I intend on making TrieInputs use TrieUpdatesSorted as well.
mediocregopher
requested review from
Rjected,
RomanHodulak,
fgimenez,
gakonst,
joshieDo,
mattsse,
rakita,
rkrasiuk,
shekhirin and
yongkangc
as code owners
Rjected
left a comment
Rjected
left a comment
There was a problem hiding this comment.
makes sense to me, glad that we can get rid of so much code
| // Try reinserting the reorged canonical chain. This is only possible if we have | ||
| // `persisted_trie_updates` for those blocks. | ||
| let old = old | ||
| .iter() | ||
| .filter_map(|block| { | ||
| let trie = self | ||
| .state | ||
| .tree_state | ||
| .persisted_trie_updates | ||
| .get(&block.recovered_block.hash())? | ||
| .1 | ||
| .clone(); | ||
| Some(ExecutedBlockWithTrieUpdates { | ||
| block: block.clone(), | ||
| trie: ExecutedTrieUpdates::Present(trie), | ||
| }) | ||
| }) | ||
| .collect::<Vec<_>>(); | ||
| self.reinsert_reorged_blocks(old); |
| /// | ||
| /// Also see [`reth_chain_state::ExecutedTrieUpdates::Missing`]. | ||
| #[error("Missing ancestor with hash {0}")] | ||
| MissingAncestor(B256), |
There was a problem hiding this comment.
nice, we can remove whole class of error because now we generate trie data for any block from the database on the fly and therefore the parent block's (ancestor's) trie data can never be "missing"
| /// calculation, and saves the trie updates. | ||
| /// | ||
| /// Returns an error if the state root calculation fails. | ||
| fn get_canonical_blocks_to_persist( |
There was a problem hiding this comment.
i guess this is the main crux? - where we no longer have to persist blocks missing trie updates because we can compute them on demand
3 participants
This PR removes the type
ExecutedBlockWithTrieUpdates, as well as the relatedExecutedTrieUpdatestype. Computation of theTrieInputis modified to take advantage of the trie changesets tables to generate reverts as necessary, the same as is done for hashed post state. This allows us to therefore always have validTrieUpdatesfor a block. With this in place we can remove a lot of extraneous logic and types.There is a new DB method added in this PR which was necessary to produce
ExecutedBlocksfor historical blocks without re-executing them. The new method calculates theTrieUpdates(not the reverts!) for a historical block based on the changesets.