chore: Make SparseTrie crate no_std compatible
#15786
Conversation
| @@ -769,15 +778,15 @@ impl<P> RevealedSparseTrie<P> { | |||
| prefix_set: &mut PrefixSet, | |||
| buffers: &mut RlpNodeBuffers, | |||
| ) -> RlpNode { | |||
| let starting_path = buffers.path_stack.last().map(|item| item.path.clone()); | |||
| let _starting_path = buffers.path_stack.last().map(|item| item.path.clone()); | |||
There was a problem hiding this comment.
underscore because starting_path is only used in the trace! macro and in no_std this is a noop, so clippy warns of an unused variable
d91c417 to
5a02c05
Compare
5a02c05 to
e8ae2a0
Compare
| @@ -7,6 +7,7 @@ crates_to_check=( | |||
| reth-primitives-traits | |||
| reth-network-peers | |||
| reth-trie-common | |||
| reth-trie-sparse | |||
There was a problem hiding this comment.
reth already guarantees that certain crates should compile under no-std riscv, so just adding this line here
| @@ -769,15 +778,15 @@ impl<P> RevealedSparseTrie<P> { | |||
| prefix_set: &mut PrefixSet, | |||
| buffers: &mut RlpNodeBuffers, | |||
| ) -> RlpNode { | |||
| let starting_path = buffers.path_stack.last().map(|item| item.path.clone()); | |||
| let _starting_path = buffers.path_stack.last().map(|item| item.path.clone()); | |||
There was a problem hiding this comment.
this will still perform a clone if tracing is disabled, let's inline this in the trace macro itself
There was a problem hiding this comment.
Would it give the same behaviour? Since this takes the last item and its called in a method that mutates the path_stack:
'main: while let Some(RlpNodePathStackItem { level, path, mut is_in_prefix_set }) =
buffers.path_stack.pop()
{
let node = self.nodes.get_mut(&path).unwrap();
trace!(
target: "trie::sparse",
?_starting_path,
?level,
?path,
?is_in_prefix_set,
?node,
"Popped node from path stack"
);
Swapped out reth-tracing for tracing -- since that was the main issue. I can look into making reth-tracing no-std in a different PR (I think some features will need to be disabled) |
|
Was exploring making reth-tracing no_std and it boiled down to everything being feature gated except for tracing and tracing_subscriber (under "alloc" feature) so it was not very useful |
| @@ -16,24 +17,28 @@ pub(crate) struct SparseStateTrieMetrics { | |||
| /// Number of total storage nodes, including those that were skipped. | |||
| pub(crate) multiproof_total_storage_nodes: u64, | |||
There was a problem hiding this comment.
the way this is structured feels a bit strange, ideally this entire file is feature gated but this would require rewriting how metrics are recorded and we can do that separately.
There was a problem hiding this comment.
Yeah that was my initial thought too
| @@ -31,7 +31,7 @@ reth-stages-api.workspace = true | |||
| reth-tasks.workspace = true | |||
| reth-trie-db.workspace = true | |||
| reth-trie-parallel.workspace = true | |||
| reth-trie-sparse.workspace = true | |||
| reth-trie-sparse = { workspace = true, features = ["std", "metrics"] } | |||
There was a problem hiding this comment.
@shekhirin I enabled the metrics feature here so that we still record them.
I also forward activated them in trie-parallel but I noticed we never activate the metrics feature for trie-parallel?
Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
This PR makes the
SparseTrieandSparseStateTrieno_std compatible similar to revm