remove hash of known value for leaf in compact proof. (#187)

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)

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<C: NodeCodec> {
 	/// The prefix is the nibble path to the node in the trie.
 	prefix: NibbleVec,
@@ -99,19 +101,16 @@ impl<C: NodeCodec> EncoderStackEntry<C> {
 	/// Generates the encoding of the subtrie rooted at this entry.
 	fn encode_node(&mut self) -> Result<Vec<u8>, 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<C: NodeCodec> EncoderStackEntry<C> {
 					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,
 				)
 			},
 		};

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<T> =
@@ -635,33 +635,33 @@ fn test_cache_internal<T: TrieLayout>() {
 
 #[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::<Layout>::default();
+	let mut memdb = PrefixedMemoryDB::<L>::default();
 	let mut root = Default::default();
 	{
-		let mut t = TrieDBMutBuilder::<Layout>::new(&mut memdb, &mut root).build();
+		let mut t = TrieDBMutBuilder::<L>::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::<Layout>::new();
+	// Value access would record a two nodes (branch and leaf with value 32 len inline).
+	let mut recorder = Recorder::<L>::new();
 	let overlay = memdb.clone();
 	let new_root = root;
 	{
-		let trie = TrieDBBuilder::<Layout>::new(&overlay, &new_root)
+		let trie = TrieDBBuilder::<L>::new(&overlay, &new_root)
 			.with_recorder(&mut recorder)
 			.build();
 
 		trie.get(key_value[0].0.as_slice()).unwrap();
 	}
 
-	let mut partial_db = MemoryDBProof::<Layout>::default();
+	let mut partial_db = MemoryDBProof::<L>::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 = <TrieDBBuilder<L>>::new(&partial_db, &root).build();
+		encode_compact::<L>(&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::<Layout>::new();
+	let mut recorder = Recorder::<L>::new();
 	let overlay = memdb.clone();
 	let new_root = root;
 	{
-		let trie = TrieDBBuilder::<Layout>::new(&overlay, &new_root)
+		let trie = TrieDBBuilder::<L>::new(&overlay, &new_root)
 			.with_recorder(&mut recorder)
 			.build();
 
 		trie.get(key_value[1].0.as_slice()).unwrap();
 	}
 
-	let mut partial_db = MemoryDBProof::<Layout>::default();
+	let mut partial_db = MemoryDBProof::<L>::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 = <TrieDBBuilder<L>>::new(&partial_db, &root).build();
+		encode_compact::<L>(&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::<Layout>::new();
+	let mut recorder = Recorder::<L>::new();
 	let overlay = memdb.clone();
 	let new_root = root;
 	{
-		let trie = TrieDBBuilder::<Layout>::new(&overlay, &new_root)
+		let trie = TrieDBBuilder::<L>::new(&overlay, &new_root)
 			.with_recorder(&mut recorder)
 			.build();
 
 		trie.get_hash(key_value[0].0.as_slice()).unwrap();
 	}
 
-	let mut partial_db = MemoryDBProof::<Layout>::default();
+	let mut partial_db = MemoryDBProof::<L>::default();
 	let mut count = 0;
 	for record in recorder.drain() {
 		count += 1;
@@ -712,24 +734,44 @@ fn test_record_value() {
 
 	assert_eq!(count, 2);
 
+	let compact_proof = {
+		let trie = <TrieDBBuilder<L>>::new(&partial_db, &root).build();
+		encode_compact::<L>(&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::<Layout>::new();
+	let mut recorder = Recorder::<L>::new();
 	let overlay = memdb.clone();
 	let new_root = root;
 	{
-		let trie = TrieDBBuilder::<Layout>::new(&overlay, &new_root)
+		let trie = TrieDBBuilder::<L>::new(&overlay, &new_root)
 			.with_recorder(&mut recorder)
 			.build();
 
 		trie.get_hash(key_value[1].0.as_slice()).unwrap();
 	}
 
-	let mut partial_db = MemoryDBProof::<Layout>::default();
+	let mut partial_db = MemoryDBProof::<L>::default();
 	let mut count = 0;
 	for record in recorder.drain() {
 		count += 1;
 		partial_db.insert(EMPTY_PREFIX, &record.data);
 	}
 
 	assert_eq!(count, 2);
+
+	let compact_proof = {
+		let trie = <TrieDBBuilder<L>>::new(&partial_db, &root).build();
+		encode_compact::<L>(&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);
 }