-
Notifications
You must be signed in to change notification settings - Fork 1.6k
reth 15476:Make DatabaseProof trait stateful #15541
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?
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.
I have some potential simplifications and suggestions, let me know if you have any questions or are stuck on anything
pub trait DatabaseStorageProof<'a, TX> { | ||
/// Create a new [`StorageProof`] from database transaction and account address. | ||
fn from_tx(tx: &'a TX, address: Address) -> Self; | ||
// Add constructor for Proof |
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.
should remove this comment
address: Address, | ||
&self, | ||
slot: B256, | ||
storage: HashedStorage, | ||
) -> Result<reth_trie::StorageProof, StateProofError>; | ||
|
||
/// Generates the storage multiproof for target slots based on [`TrieInput`]. | ||
fn overlay_storage_multiproof( | ||
tx: &'a TX, | ||
address: Address, | ||
&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.
we need to keep address
in the trait I think? Even if we already have the address in the DatabaseStorageProof
struct, we should keep the behavior consistent with how we did it previously, which was hashing the input addr and returning the storage proof for that hashed addr
&state_sorted, | ||
)) | ||
.with_prefix_set_mut(prefix_set) | ||
.storage_multiproof(targets) | ||
} | ||
} | ||
|
||
// Add constructor for StorageProof |
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.
need to also rm this comment
input: TrieInput, | ||
address: Address, | ||
slots: &[B256], | ||
) -> Result<AccountProof, StateProofError>; | ||
|
||
/// Generates the state [`MultiProof`] for target hashed account and storage keys. | ||
fn overlay_multiproof( | ||
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.
I wonder if this should actually be self
given I think we create a new Proof
struct every time this method is called, then we don't have to do self.clone()
in the impls below
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.
sure @Rjected , I will resolve the issues in my next commit
closes #15476 , updated the databaseproof and the databasestorageproof functions to use self parameters instead of <'a, TX>