-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat: Add option for returning TrieNodes instead of RLP encoded Bytes #14335
base: main
Are you sure you want to change the base?
Conversation
@Rjected please correct me if im going about this wrong |
crates/engine/tree/src/tree/root.rs
Outdated
@@ -229,7 +233,7 @@ impl ProofSequencer { | |||
|
|||
// return early if we don't have the next expected proof | |||
if !self.pending_proofs.contains_key(&self.next_to_deliver) { | |||
return Vec::new() | |||
return Vec::new(); |
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.
return Vec::new(); | |
return Vec::new() |
pls revert unrelated formatting changes. makes it hard to see the actual diff to review.
97ebde3
to
ce8fd4f
Compare
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.
much easier to read thx
crates/engine/tree/src/tree/root.rs
Outdated
@@ -66,13 +67,16 @@ pub struct StateRootComputeOutcome { | |||
/// A trie update that can be applied to sparse trie alongside the proofs for touched parts of the | |||
/// state. | |||
#[derive(Default, Debug)] | |||
#[allow(dead_code)] |
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 attribute shouldn't be needed
crates/engine/tree/src/tree/root.rs
Outdated
@@ -438,6 +443,25 @@ where | |||
} | |||
} | |||
|
|||
#[derive(Debug, Default)] | |||
struct DecodedProofs { | |||
#[allow(dead_code)] |
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.
#[allow(dead_code)] |
this attribute shouldn't be needed
… in proof retainer
ce8fd4f
to
862dbd2
Compare
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.
directionally we need to add the multiproof structs that contain DecodedProofNodes
first. Left some feedback, although heads up I may take this over early this week as it will soon be an urgent perf item
#[derive(Debug, Default)] | ||
struct DecodedProofs { | ||
pub decoded_nodes: Vec<TrieNode>, | ||
} | ||
|
||
fn decode_proofs(multiproof: MultiProof) -> Result<DecodedProofs, SparseStateTrieError> { | ||
let mut nodes: Vec<TrieNode> = vec![]; | ||
let account_subtree = multiproof.account_subtree.into_nodes_sorted(); | ||
let account_nodes = account_subtree.into_iter(); | ||
|
||
// Reveal the remaining proof nodes. | ||
for (_path, bytes) in account_nodes { | ||
let node = TrieNode::decode(&mut &bytes[..])?; | ||
nodes.push(node); | ||
} | ||
Ok(DecodedProofs { decoded_nodes: nodes }) | ||
} |
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.
Instead of this, let's just start by creating a struct like StorageMultiProof
:
reth/crates/trie/common/src/proofs.rs
Line 141 in 905fd37
pub struct StorageMultiProof { |
In trie-common, that uses alloy_trie::DecodedProofNodes
instead of ProofNodes
Ref #14313
Closely related to #14312