Skip to content

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

DioBr4nd0
Copy link
Contributor

closes #15476 , updated the databaseproof and the databasestorageproof functions to use self parameters instead of <'a, TX>

Copy link
Member

@Rjected Rjected left a 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should remove this comment

Comment on lines -92 to +95
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,
Copy link
Member

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
Copy link
Member

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,
Copy link
Member

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

Copy link
Contributor Author

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

@github-project-automation github-project-automation bot moved this from Todo to In Progress in Reth Tracker Apr 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

Make DatabaseProof trait stateful
2 participants