-
Notifications
You must be signed in to change notification settings - Fork 1.6k
feat: Make DatabaseStateRoot stateful #15760
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: Make DatabaseStateRoot stateful #15760
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems fine, but unsure if this is what @Rjected had in mind
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the changes to DatabaseStateRoot
look fine, but I have some other suggestions that should make using StateRoot
simpler
/// Trait for creating a new [`StateRoot`] instance from a database transaction. | ||
pub trait StateRootFromTx<'a, TX> { | ||
/// Create a new [`StateRoot`] instance. | ||
fn from_tx(tx: &'a TX) -> Self; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a trait for this? I am not sure that we do, we should just be able to implement from_tx
on the things that implement DatabaseStateRoot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding from_tx
to the things that implement DatabaseStateRoot
(in this case StateRoot
) introduces a cyclic dependency between the reth-trie
and the reth-trie-db
crates.
If we want from_tx
implemented on StateRoot
, we'll probably need to separate out the cursor_factory
s into a separate crate
CodSpeed Performance ReportMerging #15760 will improve performances by 11.68%Comparing Summary
Benchmarks breakdown
|
This PR
DatabaseStateRoot
into a regular stateful trait that takes in&self
.from_tx
method to a new traitStateRootFromTx
implemented byStateRoot
tx ref
to the trait implementer (StateRoot
in this case)resolves: #15478