diff --git a/trie-db/CHANGELOG.md b/trie-db/CHANGELOG.md index efa09e43..21826032 100644 --- a/trie-db/CHANGELOG.md +++ b/trie-db/CHANGELOG.md @@ -6,6 +6,8 @@ The format is based on [Keep a Changelog]. ## [Unreleased] +- Fix compact proof to skip including value node hashes [#187](https://github.com/paritytech/trie/pull/187) + ## [0.26.0] - 2023-02-24 - `TrieDBRawIterator` and `TrieDBNodeIterator` now return a node inside of an `Arc` instead of `Rc` [#185](https://github.com/paritytech/trie/pull/185) - `TrieDBRawIterator` is now `Send` and `Sync` [#185](https://github.com/paritytech/trie/pull/185) diff --git a/trie-db/src/trie_codec.rs b/trie-db/src/trie_codec.rs index 41f64fff..9a1f51b3 100644 --- a/trie-db/src/trie_codec.rs +++ b/trie-db/src/trie_codec.rs @@ -34,6 +34,8 @@ use crate::{ }; use hash_db::{HashDB, Prefix}; +const OMIT_VALUE_HASH: crate::node::Value<'static> = crate::node::Value::Inline(&[]); + struct EncoderStackEntry { /// The prefix is the nibble path to the node in the trie. prefix: NibbleVec, @@ -99,19 +101,16 @@ impl EncoderStackEntry { /// Generates the encoding of the subtrie rooted at this entry. fn encode_node(&mut self) -> Result, C::HashOut, C::Error> { let node_data = self.node.data(); - let mut modified_node_plan; - let node_plan = if self.omit_value { - modified_node_plan = self.node.node_plan().clone(); - if let Some(value) = modified_node_plan.value_plan_mut() { - // 0 length value. - *value = ValuePlan::Inline(0..0); - } - &modified_node_plan - } else { - self.node.node_plan() - }; + let node_plan = self.node.node_plan(); let mut encoded = match node_plan { - NodePlan::Empty | NodePlan::Leaf { .. } => node_data.to_vec(), + NodePlan::Empty => node_data.to_vec(), + NodePlan::Leaf { partial, value: _ } => + if self.omit_value { + let partial = partial.build(node_data); + C::leaf_node(partial.right_iter(), partial.len(), OMIT_VALUE_HASH) + } else { + node_data.to_vec() + }, NodePlan::Extension { partial, child: _ } => if !self.omit_children[0] { node_data.to_vec() @@ -120,17 +119,29 @@ impl EncoderStackEntry { let empty_child = ChildReference::Inline(C::HashOut::default(), 0); C::extension_node(partial.right_iter(), partial.len(), empty_child) }, - NodePlan::Branch { value, children } => C::branch_node( - Self::branch_children(node_data, &children, &self.omit_children)?.iter(), - value.as_ref().map(|v| v.build(node_data)), - ), + NodePlan::Branch { value, children } => { + let value = if self.omit_value { + value.is_some().then_some(OMIT_VALUE_HASH) + } else { + value.as_ref().map(|v| v.build(node_data)) + }; + C::branch_node( + Self::branch_children(node_data, &children, &self.omit_children)?.iter(), + value, + ) + }, NodePlan::NibbledBranch { partial, value, children } => { let partial = partial.build(node_data); + let value = if self.omit_value { + value.is_some().then_some(OMIT_VALUE_HASH) + } else { + value.as_ref().map(|v| v.build(node_data)) + }; C::branch_node_nibbled( partial.right_iter(), partial.len(), Self::branch_children(node_data, &children, &self.omit_children)?.iter(), - value.as_ref().map(|v| v.build(node_data)), + value, ) }, }; diff --git a/trie-db/test/src/triedb.rs b/trie-db/test/src/triedb.rs index 520db1a5..e4420ae3 100644 --- a/trie-db/test/src/triedb.rs +++ b/trie-db/test/src/triedb.rs @@ -19,8 +19,8 @@ use hex_literal::hex; use memory_db::{HashKey, MemoryDB, PrefixedKey}; use reference_trie::{test_layouts, HashedValueNoExtThreshold, TestTrieCache}; use trie_db::{ - CachedValue, DBValue, Lookup, NibbleSlice, Recorder, Trie, TrieCache, TrieDBBuilder, - TrieDBMutBuilder, TrieLayout, TrieMut, + encode_compact, CachedValue, DBValue, Lookup, NibbleSlice, Recorder, Trie, TrieCache, + TrieDBBuilder, TrieDBMutBuilder, TrieLayout, TrieMut, }; type PrefixedMemoryDB = @@ -635,33 +635,33 @@ fn test_cache_internal() { #[test] fn test_record_value() { - type Layout = HashedValueNoExtThreshold<33>; + type L = HashedValueNoExtThreshold<33>; // one root branch and two leaf, one with inline value, the other with node value. let key_value = vec![(b"A".to_vec(), vec![1; 32]), (b"B".to_vec(), vec![1; 33])]; // Add some initial data to the trie - let mut memdb = PrefixedMemoryDB::::default(); + let mut memdb = PrefixedMemoryDB::::default(); let mut root = Default::default(); { - let mut t = TrieDBMutBuilder::::new(&mut memdb, &mut root).build(); + let mut t = TrieDBMutBuilder::::new(&mut memdb, &mut root).build(); for (key, value) in key_value.iter() { t.insert(key, value).unwrap(); } } - // Value access would record a tow node (branch and leaf with value 32 len inline). - let mut recorder = Recorder::::new(); + // Value access would record a two nodes (branch and leaf with value 32 len inline). + let mut recorder = Recorder::::new(); let overlay = memdb.clone(); let new_root = root; { - let trie = TrieDBBuilder::::new(&overlay, &new_root) + let trie = TrieDBBuilder::::new(&overlay, &new_root) .with_recorder(&mut recorder) .build(); trie.get(key_value[0].0.as_slice()).unwrap(); } - let mut partial_db = MemoryDBProof::::default(); + let mut partial_db = MemoryDBProof::::default(); let mut count = 0; for record in recorder.drain() { count += 1; @@ -670,19 +670,29 @@ fn test_record_value() { assert_eq!(count, 2); + let compact_proof = { + let trie = >::new(&partial_db, &root).build(); + encode_compact::(&trie).unwrap() + }; + assert_eq!(compact_proof.len(), 2); + // two child branch with only one child accessed + assert_eq!(compact_proof[0].len(), 38); + // leaf node with inline 32 byte value + assert_eq!(compact_proof[1].len(), 34); + // Value access on node returns three items: a branch a leaf and a value node - let mut recorder = Recorder::::new(); + let mut recorder = Recorder::::new(); let overlay = memdb.clone(); let new_root = root; { - let trie = TrieDBBuilder::::new(&overlay, &new_root) + let trie = TrieDBBuilder::::new(&overlay, &new_root) .with_recorder(&mut recorder) .build(); trie.get(key_value[1].0.as_slice()).unwrap(); } - let mut partial_db = MemoryDBProof::::default(); + let mut partial_db = MemoryDBProof::::default(); let mut count = 0; for record in recorder.drain() { count += 1; @@ -691,19 +701,31 @@ fn test_record_value() { assert_eq!(count, 3); + let compact_proof = { + let trie = >::new(&partial_db, &root).build(); + encode_compact::(&trie).unwrap() + }; + assert_eq!(compact_proof.len(), 3); + // two child branch with only one child accessed + assert_eq!(compact_proof[0].len(), 38); + // leaf with ommited hash value and escape header + assert_eq!(compact_proof[1].len(), 3); + // value node 33 bytes + assert_eq!(compact_proof[2].len(), 33); + // Hash access would record two node (branch and leaf with value 32 len inline). - let mut recorder = Recorder::::new(); + let mut recorder = Recorder::::new(); let overlay = memdb.clone(); let new_root = root; { - let trie = TrieDBBuilder::::new(&overlay, &new_root) + let trie = TrieDBBuilder::::new(&overlay, &new_root) .with_recorder(&mut recorder) .build(); trie.get_hash(key_value[0].0.as_slice()).unwrap(); } - let mut partial_db = MemoryDBProof::::default(); + let mut partial_db = MemoryDBProof::::default(); let mut count = 0; for record in recorder.drain() { count += 1; @@ -712,19 +734,29 @@ fn test_record_value() { assert_eq!(count, 2); + let compact_proof = { + let trie = >::new(&partial_db, &root).build(); + encode_compact::(&trie).unwrap() + }; + assert_eq!(compact_proof.len(), 2); + // two child branch with only one child accessed + assert_eq!(compact_proof[0].len(), 38); + // leaf node with inline 32 byte value + assert_eq!(compact_proof[1].len(), 34); + // Hash access would record two node (branch and leaf with value 32 len inline). - let mut recorder = Recorder::::new(); + let mut recorder = Recorder::::new(); let overlay = memdb.clone(); let new_root = root; { - let trie = TrieDBBuilder::::new(&overlay, &new_root) + let trie = TrieDBBuilder::::new(&overlay, &new_root) .with_recorder(&mut recorder) .build(); trie.get_hash(key_value[1].0.as_slice()).unwrap(); } - let mut partial_db = MemoryDBProof::::default(); + let mut partial_db = MemoryDBProof::::default(); let mut count = 0; for record in recorder.drain() { count += 1; @@ -732,4 +764,14 @@ fn test_record_value() { } assert_eq!(count, 2); + + let compact_proof = { + let trie = >::new(&partial_db, &root).build(); + encode_compact::(&trie).unwrap() + }; + assert_eq!(compact_proof.len(), 2); + // two child branch with only one child accessed + assert_eq!(compact_proof[0].len(), 38); + // leaf with value hash only. + assert_eq!(compact_proof[1].len(), 33); }