diff --git a/crates/common/trie/trie.rs b/crates/common/trie/trie.rs index b4df3ce7476..386a3f0e288 100644 --- a/crates/common/trie/trie.rs +++ b/crates/common/trie/trie.rs @@ -535,18 +535,20 @@ impl Trie { Trie::new(Box::new(NullTrieDB)) } - /// Obtain the encoded node given its path. - /// Allows usage of full paths (byte slice of 32 bytes) or compact-encoded nibble slices (with length lower than 32) + /// Maximum compact-encoded path length accepted by [`Self::get_node`]. + /// Longer inputs return an empty node (aligned with geth's snap path guard). + const MAX_GET_NODE_PATH_LEN: usize = 32; + + /// Obtain the RLP encoding of the trie node at a compact-encoded path. + /// + /// Empty path returns the root. Paths longer than [`Self::MAX_GET_NODE_PATH_LEN`] + /// return an empty vector. The path is snap `GetTrieNodes` compact encoding, + /// not a raw 32-byte account or storage key — use [`Self::get`] for values. pub fn get_node(&self, partial_path: &PathRLP) -> Result, TrieError> { - // Convert compact-encoded nibbles into a byte slice if necessary - let partial_path = match partial_path.len() { - // Compact-encoded nibbles - n if n < 32 => Nibbles::decode_compact(partial_path), - // Full path (No conversion needed) - 32 => Nibbles::from_bytes(partial_path), - // We won't handle paths with length over 32 - _ => return Ok(vec![]), - }; + if partial_path.len() > Self::MAX_GET_NODE_PATH_LEN { + return Ok(vec![]); + } + let partial_path = Nibbles::decode_compact(partial_path); fn get_node_inner( db: &dyn TrieDB, @@ -559,38 +561,52 @@ impl Trie { return Ok(node.encode_to_vec()); } match node { - Node::Branch(branch_node) => match partial_path.next_choice() { - Some(idx) => { - let child_ref = &branch_node.choices[idx]; - if child_ref.is_valid() { - let child_path = current_path.append_new(idx as u8); - let child_node = child_ref - .get_node_checked(db, child_path.clone())? - .ok_or_else(|| { - TrieError::InconsistentTree(Box::new( - InconsistentTreeError::NodeNotFoundOnBranchNode( - child_ref - .compute_hash(&NativeCrypto) - .finalize(&NativeCrypto), - branch_node - .compute_hash(&NativeCrypto) - .finalize(&NativeCrypto), - child_path.clone(), - ), - )) - })?; - get_node_inner(db, child_path, &child_node, partial_path) + Node::Branch(branch_node) => { + // Path is non-empty here; avoid `next_choice` (it treats 16 as a miss). + let nibble = partial_path + .next() + .expect("non-empty partial_path checked above"); + if nibble == 16 { + // Leaf terminator: return this branch only if the path ends here. + return if partial_path.is_empty() { + Ok(node.encode_to_vec()) } else { Ok(vec![]) - } + }; + } + if nibble > 15 { + return Ok(vec![]); } - _ => Ok(vec![]), - }, + let idx = nibble as usize; + let child_ref = &branch_node.choices[idx]; + if child_ref.is_valid() { + let child_path = current_path.append_new(nibble); + let child_node = child_ref + .get_node_checked(db, child_path.clone())? + .ok_or_else(|| { + TrieError::InconsistentTree(Box::new( + InconsistentTreeError::NodeNotFoundOnBranchNode( + child_ref + .compute_hash(&NativeCrypto) + .finalize(&NativeCrypto), + branch_node + .compute_hash(&NativeCrypto) + .finalize(&NativeCrypto), + child_path.clone(), + ), + )) + })?; + get_node_inner(db, child_path, &child_node, partial_path) + } else { + Ok(vec![]) + } + } Node::Extension(extension_node) => { if partial_path.skip_prefix(&extension_node.prefix) && extension_node.child.is_valid() { - let child_path = partial_path.concat(&extension_node.prefix); + // Child is keyed at current_path ++ prefix (see NodeRef::commit). + let child_path = current_path.concat(&extension_node.prefix); let child_node = extension_node .child .get_node_checked(db, child_path.clone())? diff --git a/crates/networking/p2p/rlpx/snap/messages.rs b/crates/networking/p2p/rlpx/snap/messages.rs index 699e592c5af..190456625fe 100644 --- a/crates/networking/p2p/rlpx/snap/messages.rs +++ b/crates/networking/p2p/rlpx/snap/messages.rs @@ -60,8 +60,8 @@ pub struct GetTrieNodes { pub id: u64, /// State root hash to query against pub root_hash: H256, - /// Paths to trie nodes: [[acc_path, slot_path_1, slot_path_2,...]...] - /// Paths can be full paths (hash) or partial paths (compact-encoded nibbles) + /// Pathsets `[[acc_path, slot_path, ...], ...]`: one entry is a compact account-trie + /// path; longer sets use a 32-byte account hash then compact storage-trie paths. pub paths: Vec>, /// Maximum response size in bytes pub bytes: u64, diff --git a/crates/storage/store.rs b/crates/storage/store.rs index b7592847ddc..9ff8dc42522 100644 --- a/crates/storage/store.rs +++ b/crates/storage/store.rs @@ -3695,12 +3695,13 @@ impl Store { Ok(Some(proof)) } - /// Receives the root of the state trie and a list of paths where the first path will correspond to a path in the state trie - /// (aka a hashed account address) and the following paths will be paths in the account's storage trie (aka hashed storage keys) - /// If only one hash (account) is received, then the state trie node containing the account will be returned. - /// If more than one hash is received, then the storage trie nodes where each storage key is stored will be returned - /// For more information check out snap capability message [`GetTrieNodes`](https://github.com/ethereum/devp2p/blob/master/caps/snap.md#gettrienodes-0x06) - /// The paths can be either full paths (hash) or partial paths (compact-encoded nibbles), if a partial path is given for the account this method will not return storage nodes for it + /// Serve snap [`GetTrieNodes`](https://github.com/ethereum/devp2p/blob/master/caps/snap.md#gettrienodes-0x06) + /// for one pathset against `state_root`. + /// + /// - One path: compact-encoded account-trie path; returns that node via [`Trie::get_node`]. + /// - Multiple paths: `paths[0]` is the 32-byte account hash used to open the account's + /// storage trie; the rest are compact-encoded storage-trie paths via [`Trie::get_node`]. + /// If `paths[0]` is not 32 bytes, returns an empty list. pub fn get_trie_nodes( &self, state_root: H256, diff --git a/docs/internal/l1/healing.md b/docs/internal/l1/healing.md index 0b7afb00c35..722b0a76ecf 100644 --- a/docs/internal/l1/healing.md +++ b/docs/internal/l1/healing.md @@ -42,9 +42,10 @@ The API used is the ethereum capability snap/1, documented at https://github.com pub struct GetTrieNodes { pub id: u64, pub root_hash: H256, - // [[acc_path, slot_path_1, slot_path_2,...]...] - // The paths can be either full paths (hash) or - // only the partial path (compact-encoded nibbles) + // [[acc_path, slot_path_1, slot_path_2,...]...] + // Account-trie node: one compact-encoded path (pathset length 1). + // Storage-trie nodes: pathset[0] = 32-byte account hash; + // pathset[1..] = compact-encoded paths in that storage trie. pub paths: Vec>, pub bytes: u64, } diff --git a/test/tests/trie/trie_tests.rs b/test/tests/trie/trie_tests.rs index 381bde822c5..a86d3be5fe1 100644 --- a/test/tests/trie/trie_tests.rs +++ b/test/tests/trie/trie_tests.rs @@ -3,7 +3,11 @@ use cita_trie::{MemoryDB as CitaMemoryDB, PatriciaTrie as CitaTrie, Trie as Cita use std::sync::Arc; use ethrex_crypto::NativeCrypto; -use ethrex_trie::Trie; +use ethrex_rlp::encode::RLPEncode; +use ethrex_trie::{ + InMemoryTrieDB, Nibbles, Node, NodeHash, NodeRef, Trie, + db::NodeMap, +}; use hasher::HasherKeccak; use hex_literal::hex; @@ -636,3 +640,188 @@ fn get_proof_removed_value() { let trie_proof = trie.get_proof(&a).unwrap(); assert_eq!(cita_proof, trie_proof); } + +// Trie::get_node (snap GetTrieNodes path decoding) + +/// Trie with a shared-prefix extension root and hashed branch/leaf children. +fn extension_rooted_trie() -> (Trie, Vec<[u8; 32]>) { + let keys: Vec<[u8; 32]> = (0u8..6) + .map(|i| { + let mut key = [0u8; 32]; + key[0] = 0xab; + key[1] = i << 4; + key[31] = i; + key + }) + .collect(); + + let db: NodeMap = Default::default(); + let mut trie = Trie::new(Box::new(InMemoryTrieDB::new(db.clone()))); + for key in &keys { + trie.insert(key.to_vec(), vec![0x11; 40]).unwrap(); + } + let root = trie.hash(&NativeCrypto).unwrap(); + (Trie::open(Box::new(InMemoryTrieDB::new(db)), root), keys) +} + +/// Extension child must load at `extension_path ++ prefix`. +#[test] +fn get_node_compact_path_crossing_extension_node() { + let (trie, _keys) = extension_rooted_trie(); + + let root = trie.root_node().unwrap().expect("trie should have a root"); + let Node::Extension(extension) = root.as_ref() else { + panic!("expected an extension node at the root, got {root:?}"); + }; + let branch_path = extension.prefix.clone(); + assert_eq!(branch_path, Nibbles::from_hex(vec![0xa, 0xb])); + assert!(matches!( + extension.child, + NodeRef::Hash(NodeHash::Hashed(_)) + )); + + let branch = extension + .child + .get_node(trie.db(), branch_path.clone()) + .unwrap() + .expect("extension child should be stored under its path"); + let Node::Branch(branch_node) = branch.as_ref() else { + panic!("expected a branch node below the extension, got {branch:?}"); + }; + let leaf_path = branch_path.append_new(0); + let leaf = branch_node.choices[0] + .get_node(trie.db(), leaf_path.clone()) + .unwrap() + .expect("branch child should be stored under its path"); + + assert_eq!( + trie.get_node(&branch_path.encode_compact()).unwrap(), + branch.as_ref().encode_to_vec() + ); + assert_eq!( + trie.get_node(&leaf_path.encode_compact()).unwrap(), + leaf.as_ref().encode_to_vec() + ); +} + +/// A 32-byte compact path must decode as compact and resolve the node. +#[test] +fn get_node_compact_path_length_32() { + // Two keys share 31 bytes plus the high nibble of the last byte → extension of + // 63 nibbles (compact length 32) then a branch. + let mut key_a = [0u8; 32]; + let mut key_b = [0u8; 32]; + key_a[31] = 0x01; + key_b[31] = 0x02; + + let db: NodeMap = Default::default(); + let mut trie = Trie::new(Box::new(InMemoryTrieDB::new(db.clone()))); + trie.insert(key_a.to_vec(), vec![0x11; 40]).unwrap(); + trie.insert(key_b.to_vec(), vec![0x22; 40]).unwrap(); + let root = trie.hash(&NativeCrypto).unwrap(); + let trie = Trie::open(Box::new(InMemoryTrieDB::new(db)), root); + + let root_node = trie.root_node().unwrap().expect("root"); + let Node::Extension(extension) = root_node.as_ref() else { + panic!("expected extension root, got {root_node:?}"); + }; + assert_eq!(extension.prefix.len(), 63); + + let branch_path = extension.prefix.clone(); + let compact = branch_path.encode_compact(); + assert_eq!( + compact.len(), + 32, + "expected 63-nibble extension path to encode to 32 compact bytes" + ); + + let branch = extension + .child + .get_node(trie.db(), branch_path.clone()) + .unwrap() + .expect("branch under extension"); + assert_eq!( + trie.get_node(&compact).unwrap(), + branch.as_ref().encode_to_vec() + ); +} + +/// Raw 32-byte keys are not a valid `get_node` path (use [`Trie::get`] for values). +#[test] +fn get_node_raw_32_byte_key_is_not_keybytes() { + let (trie, keys) = extension_rooted_trie(); + let path = keys[0].to_vec(); + + assert!(trie.get(&path).unwrap().is_some()); + assert!( + trie.get_node(&path).unwrap().is_empty(), + "raw 32-byte key must not resolve a trie node via get_node" + ); +} + +#[test] +fn get_node_empty_path_returns_root() { + let (trie, _keys) = extension_rooted_trie(); + let root = trie.root_node().unwrap().unwrap(); + assert_eq!( + trie.get_node(&Vec::new()).unwrap(), + root.as_ref().encode_to_vec() + ); +} + +#[test] +fn get_node_oversize_path_returns_empty() { + let (trie, _keys) = extension_rooted_trie(); + assert!(trie.get_node(&vec![0u8; 33]).unwrap().is_empty()); +} + +#[test] +fn get_node_missing_paths_return_empty() { + let (trie, _keys) = extension_rooted_trie(); + + let diverging = Nibbles::from_hex(vec![0xa, 0xc]).encode_compact(); + assert!(trie.get_node(&diverging).unwrap().is_empty()); + + let empty_choice = Nibbles::from_hex(vec![0xa, 0xb, 0xf]).encode_compact(); + assert!(trie.get_node(&empty_choice).unwrap().is_empty()); +} + +/// Leaf terminator at a branch with a value returns that branch node. +#[test] +fn get_node_branch_terminator_returns_branch() { + // Unequal key lengths so one value sits on the branch at the divergence. + let db: NodeMap = Default::default(); + let mut trie = Trie::new(Box::new(InMemoryTrieDB::new(db.clone()))); + trie.insert(vec![0x00], vec![0xaa; 40]).unwrap(); + trie.insert(vec![0x00, 0x11], vec![0xbb; 40]).unwrap(); + let root = trie.hash(&NativeCrypto).unwrap(); + let trie = Trie::open(Box::new(InMemoryTrieDB::new(db)), root); + + let root_node = trie.root_node().unwrap().expect("root"); + // Walk to the branch that owns the short key's value. + let (branch_path, branch_rlp) = match root_node.as_ref() { + Node::Branch(b) => (Nibbles::default(), b.as_ref().encode_to_vec()), + Node::Extension(ext) => { + let path = ext.prefix.clone(); + let child = ext + .child + .get_node(trie.db(), path.clone()) + .unwrap() + .expect("extension child"); + let Node::Branch(b) = child.as_ref() else { + panic!("expected branch under extension, got {child:?}"); + }; + assert!( + !b.value.is_empty(), + "expected branch to hold the short key's value" + ); + (path, b.as_ref().encode_to_vec()) + } + other => panic!("unexpected root {other:?}"), + }; + + let mut path_with_term = branch_path; + path_with_term.append(16); + let compact = path_with_term.encode_compact(); + assert_eq!(trie.get_node(&compact).unwrap(), branch_rlp); +}