Skip to content

Commit

Permalink
chore: improve Nibbles-related code (#5631)
Browse files Browse the repository at this point in the history
  • Loading branch information
DaniPopes authored Nov 30, 2023
1 parent a4ed76d commit 0d522e8
Show file tree
Hide file tree
Showing 23 changed files with 310 additions and 230 deletions.
2 changes: 1 addition & 1 deletion bin/reth/src/debug_cmd/in_memory_merkle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ impl Command {
(Some(in_mem), Some(incr)) => {
pretty_assertions::assert_eq!(in_mem.0, incr.0, "Nibbles don't match");
if in_mem.1 != incr.1 &&
matches!(in_mem.0, TrieKey::AccountNode(ref nibbles) if nibbles.inner.len() > self.skip_node_depth.unwrap_or_default())
matches!(in_mem.0, TrieKey::AccountNode(ref nibbles) if nibbles.len() > self.skip_node_depth.unwrap_or_default())
{
in_mem_mismatched.push(in_mem);
incremental_mismatched.push(incr);
Expand Down
5 changes: 2 additions & 3 deletions bin/reth/src/debug_cmd/merkle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ impl Command {
"Nibbles don't match"
);
if incremental.1 != clean.1 &&
clean.0.inner.len() > self.skip_node_depth.unwrap_or_default()
clean.0.len() > self.skip_node_depth.unwrap_or_default()
{
incremental_account_mismatched.push(incremental);
clean_account_mismatched.push(clean);
Expand Down Expand Up @@ -340,8 +340,7 @@ impl Command {
match (incremental_storage_trie_iter.next(), clean_storage_trie_iter.next()) {
(Some(incremental), Some(clean)) => {
if incremental != clean &&
clean.1.nibbles.inner.len() >
self.skip_node_depth.unwrap_or_default()
clean.1.nibbles.len() > self.skip_node_depth.unwrap_or_default()
{
first_mismatched_storage = Some((incremental, clean));
break
Expand Down
13 changes: 4 additions & 9 deletions crates/primitives/benches/nibbles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,12 @@ use reth_primitives::trie::Nibbles;

/// Benchmarks the nibble unpacking.
pub fn nibbles_benchmark(c: &mut Criterion) {
c.bench_function("Nibbles unpack", |b| {
let mut g = c.benchmark_group("nibbles");
g.bench_function("unpack", |b| {
let raw = (1..=32).collect::<Vec<u8>>();
b.iter(|| {
Nibbles::unpack(&raw);
})
b.iter(|| Nibbles::unpack(&raw))
});
}

criterion_group! {
name = benches;
config = Criterion::default();
targets = nibbles_benchmark
}
criterion_group!(benches, nibbles_benchmark);
criterion_main!(benches);
31 changes: 16 additions & 15 deletions crates/primitives/src/trie/hash_builder/mod.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use super::{
nodes::{rlp_hash, BranchNode, ExtensionNode, LeafNode},
nodes::{word_rlp, BranchNode, ExtensionNode, LeafNode},
BranchNodeCompact, Nibbles, TrieMask,
};
use crate::{constants::EMPTY_ROOT_HASH, keccak256, Bytes, B256};
use itertools::Itertools;
use std::{
collections::{BTreeMap, HashMap},
fmt::Debug,
Expand Down Expand Up @@ -62,7 +63,7 @@ pub struct HashBuilder {
impl From<HashBuilderState> for HashBuilder {
fn from(state: HashBuilderState) -> Self {
Self {
key: Nibbles::from_hex(state.key),
key: Nibbles::new_unchecked(state.key),
stack: state.stack,
value: state.value,
groups: state.groups,
Expand All @@ -79,7 +80,7 @@ impl From<HashBuilderState> for HashBuilder {
impl From<HashBuilder> for HashBuilderState {
fn from(state: HashBuilder) -> Self {
Self {
key: state.key.hex_data.to_vec(),
key: state.key.to_vec(),
stack: state.stack,
value: state.value,
groups: state.groups,
Expand Down Expand Up @@ -155,7 +156,7 @@ impl HashBuilder {
if !self.key.is_empty() {
self.update(&key);
} else if key.is_empty() {
self.stack.push(rlp_hash(value));
self.stack.push(word_rlp(&value));
}
self.set_key_value(key, value);
self.stored_in_database = stored_in_database;
Expand All @@ -166,7 +167,7 @@ impl HashBuilder {
// Clears the internal state
if !self.key.is_empty() {
self.update(&Nibbles::default());
self.key.hex_data.0.clear();
self.key.clear();
self.value = HashBuilderValue::Bytes(vec![]);
}
self.current_root()
Expand Down Expand Up @@ -209,15 +210,15 @@ impl HashBuilder {
tracing::Level::TRACE,
"loop",
i,
current = crate::hex::encode(&current.hex_data),
?current,
?build_extensions
);
let _enter = span.enter();

let preceding_exists = !self.groups.is_empty();
let preceding_len: usize = self.groups.len().saturating_sub(1);

let common_prefix_len = succeeding.common_prefix_length(&current);
let common_prefix_len = succeeding.common_prefix_length(current.as_slice());
let len = std::cmp::max(preceding_len, common_prefix_len);
assert!(len < current.len());

Expand All @@ -241,7 +242,7 @@ impl HashBuilder {
trace!(
target: "trie::hash_builder",
?extra_digit,
groups = self.groups.iter().map(|x| format!("{x:?}")).collect::<Vec<_>>().join(","),
groups = ?self.groups.iter().format(", "),
);

// Adjust the tree masks for exporting to the DB
Expand All @@ -256,7 +257,7 @@ impl HashBuilder {
trace!(target: "trie::hash_builder", "skipping {} nibbles", len_from);

// The key without the common prefix
let short_node_key = current.slice_from(len_from);
let short_node_key = current.slice(len_from..);
trace!(target: "trie::hash_builder", ?short_node_key);

// Concatenate the 2 nodes together
Expand All @@ -276,7 +277,7 @@ impl HashBuilder {
}
HashBuilderValue::Hash(hash) => {
trace!(target: "trie::hash_builder", ?hash, "pushing branch node hash");
self.stack.push(rlp_hash(*hash));
self.stack.push(word_rlp(hash));

if self.stored_in_database {
self.tree_masks[current.len() - 1] |=
Expand All @@ -302,7 +303,7 @@ impl HashBuilder {
}, "extension node rlp");
self.rlp_buf.clear();
self.stack.push(extension_node.rlp(&mut self.rlp_buf));
self.retain_proof_from_buf(&current.slice(0, len_from));
self.retain_proof_from_buf(&current.slice(..len_from));
self.resize_masks(len_from);
}

Expand Down Expand Up @@ -353,7 +354,7 @@ impl HashBuilder {

self.rlp_buf.clear();
let rlp = branch_node.rlp(state_mask, &mut self.rlp_buf);
self.retain_proof_from_buf(&current.slice(0, len));
self.retain_proof_from_buf(&current.slice(..len));

// Clears the stack from the branch node elements
let first_child_idx = self.stack.len() - state_mask.count_ones() as usize;
Expand Down Expand Up @@ -403,7 +404,7 @@ impl HashBuilder {
// Send it over to the provided channel which will handle it on the
// other side of the HashBuilder
trace!(target: "trie::hash_builder", node = ?n, "intermediate node");
let common_prefix = current.slice(0, len);
let common_prefix = current.slice(..len);
if let Some(nodes) = self.updated_branch_nodes.as_mut() {
nodes.insert(common_prefix, n);
}
Expand Down Expand Up @@ -563,7 +564,7 @@ mod tests {

let (_, updates) = hb.split();

let update = updates.get(&Nibbles::from(hex!("01").as_slice())).unwrap();
let update = updates.get(&Nibbles::new_unchecked(hex!("01"))).unwrap();
assert_eq!(update.state_mask, TrieMask::new(0b1111)); // 1st nibble: 0, 1, 2, 3
assert_eq!(update.tree_mask, TrieMask::new(0));
assert_eq!(update.hash_mask, TrieMask::new(6)); // in the 1st nibble, the ones with 1 and 2 are branches with `hashes`
Expand Down Expand Up @@ -633,7 +634,7 @@ mod tests {

let mut hb2 = HashBuilder::default();
// Insert the branch with the `0x6` shared prefix.
hb2.add_branch(Nibbles::from_hex(vec![0x6]), branch_node_hash, false);
hb2.add_branch(Nibbles::new_unchecked([0x6]), branch_node_hash, false);

let expected = trie_root(raw_input.clone());
assert_eq!(hb.root(), expected);
Expand Down
13 changes: 9 additions & 4 deletions crates/primitives/src/trie/mask.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,27 +32,32 @@ pub struct TrieMask(u16);

impl TrieMask {
/// Creates a new `TrieMask` from the given inner value.
#[inline]
pub fn new(inner: u16) -> Self {
Self(inner)
}

/// Creates a new `TrieMask` from the given nibble.
#[inline]
pub fn from_nibble(nibble: u8) -> Self {
Self(1u16 << nibble)
}

/// Returns `true` if the current `TrieMask` is a subset of `other`.
pub fn is_subset_of(&self, other: &Self) -> bool {
*self & *other == *self
#[inline]
pub fn is_subset_of(self, other: Self) -> bool {
self & other == self
}

/// Returns `true` if a given bit is set in a mask.
pub fn is_bit_set(&self, index: u8) -> bool {
#[inline]
pub fn is_bit_set(self, index: u8) -> bool {
self.0 & (1u16 << index) != 0
}

/// Returns `true` if the mask is empty.
pub fn is_empty(&self) -> bool {
#[inline]
pub fn is_empty(self) -> bool {
self.0 == 0
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/primitives/src/trie/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ mod subnode;

pub use self::{
mask::TrieMask,
nibbles::{Nibbles, StoredNibbles, StoredNibblesSubKey},
nibbles::{Nibbles, StoredNibblesSubKey},
storage::StorageTrieEntry,
subnode::StoredSubNode,
};
Loading

0 comments on commit 0d522e8

Please sign in to comment.