From f3f8bf4ec6c1acc9925f0f1c7b73b2a0fdc4b14a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 28 Mar 2023 16:53:31 +0200 Subject: [PATCH 01/12] TrieRecorder: Start adding support for transactions --- primitives/trie/src/recorder.rs | 103 ++++++++++++++++++++++++-------- 1 file changed, 77 insertions(+), 26 deletions(-) diff --git a/primitives/trie/src/recorder.rs b/primitives/trie/src/recorder.rs index 3bdfda01532cc..f6c04470bd69f 100644 --- a/primitives/trie/src/recorder.rs +++ b/primitives/trie/src/recorder.rs @@ -25,7 +25,7 @@ use codec::Encode; use hash_db::Hasher; use parking_lot::Mutex; use std::{ - collections::HashMap, + collections::{HashMap, HashSet}, marker::PhantomData, mem, ops::DerefMut, @@ -41,14 +41,28 @@ const LOG_TARGET: &str = "trie-recorder"; /// The internals of [`Recorder`]. struct RecorderInner { /// The keys for that we have recorded the trie nodes and if we have recorded up to the value. - recorded_keys: HashMap, RecordedForKey>>, + /// + /// Mapping: `StorageRoot -> (Key -> RecordedForKey)`. + recorded_keys: HashMap, RecordedForKey>>, + + recorded_keys_transactions: Vec, Option>>>, + /// The encoded nodes we accessed while recording. + /// + /// Mapping: `Hash(Node) -> Node`. accessed_nodes: HashMap>, + + accessed_nodes_transactions: Vec>, } impl Default for RecorderInner { fn default() -> Self { - Self { recorded_keys: Default::default(), accessed_nodes: Default::default() } + Self { + recorded_keys: Default::default(), + accessed_nodes: Default::default(), + recorded_keys_transactions: Vec::new(), + accessed_nodes_transactions: Vec::new(), + } } } @@ -145,6 +159,42 @@ struct TrieRecorder { _phantom: PhantomData, } +impl>> TrieRecorder { + fn update_recorded_keys(&mut self, full_key: &[u8], access: RecordedForKey) { + let inner = self.inner.deref_mut(); + + let entry = inner + .recorded_keys + .entry(self.storage_root) + .or_default() + .entry(full_key.into()); + + let key = entry.key().clone(); + + let entry = if matches!(access, RecordedForKey::Value) { + entry.and_modify(|e| { + if let Some(tx) = inner.recorded_keys_transactions.last_mut() { + tx.entry(self.storage_root) + .or_default() + .entry(key.clone()) + .or_insert(Some(*e)); + } + + *e = access; + }) + } else { + entry + }; + entry.or_insert_with(|| { + if let Some(tx) = inner.recorded_keys_transactions.last_mut() { + tx.entry(self.storage_root).or_default().entry(key).or_insert(None); + } + + access + }); + } +} + impl>> trie_db::TrieRecorder for TrieRecorder { @@ -159,11 +209,17 @@ impl>> trie_db::TrieRecord "Recording node", ); - self.inner.accessed_nodes.entry(hash).or_insert_with(|| { + let inner = self.inner.deref_mut(); + + inner.accessed_nodes.entry(hash).or_insert_with(|| { let node = node_owned.to_encoded::>(); encoded_size_update += node.encoded_size(); + if let Some(tx) = inner.accessed_nodes_transactions.last_mut() { + tx.insert(hash); + } + node }); }, @@ -174,11 +230,17 @@ impl>> trie_db::TrieRecord "Recording node", ); - self.inner.accessed_nodes.entry(hash).or_insert_with(|| { + let inner = self.inner.deref_mut(); + + inner.accessed_nodes.entry(hash).or_insert_with(|| { let node = encoded_node.into_owned(); encoded_size_update += node.encoded_size(); + if let Some(tx) = inner.accessed_nodes_transactions.last_mut() { + tx.insert(hash); + } + node }); }, @@ -190,21 +252,21 @@ impl>> trie_db::TrieRecord "Recording value", ); - self.inner.accessed_nodes.entry(hash).or_insert_with(|| { + let inner = self.inner.deref_mut(); + + inner.accessed_nodes.entry(hash).or_insert_with(|| { let value = value.into_owned(); encoded_size_update += value.encoded_size(); + if let Some(tx) = inner.accessed_nodes_transactions.last_mut() { + tx.insert(hash); + } + value }); - self.inner - .recorded_keys - .entry(self.storage_root) - .or_default() - .entry(full_key.to_vec()) - .and_modify(|e| *e = RecordedForKey::Value) - .or_insert(RecordedForKey::Value); + self.update_recorded_keys(full_key, RecordedForKey::Value); }, TrieAccess::Hash { full_key } => { tracing::trace!( @@ -215,12 +277,7 @@ impl>> trie_db::TrieRecord // We don't need to update the `encoded_size_update` as the hash was already // accounted for by the recorded node that holds the hash. - self.inner - .recorded_keys - .entry(self.storage_root) - .or_default() - .entry(full_key.to_vec()) - .or_insert(RecordedForKey::Hash); + self.update_recorded_keys(full_key, RecordedForKey::Hash); }, TrieAccess::NonExisting { full_key } => { tracing::trace!( @@ -232,13 +289,7 @@ impl>> trie_db::TrieRecord // Non-existing access means we recorded all trie nodes up to the value. // Not the actual value, as it doesn't exist, but all trie nodes to know // that the value doesn't exist in the trie. - self.inner - .recorded_keys - .entry(self.storage_root) - .or_default() - .entry(full_key.to_vec()) - .and_modify(|e| *e = RecordedForKey::Value) - .or_insert(RecordedForKey::Value); + self.update_recorded_keys(full_key, RecordedForKey::Value); }, }; From 0e9f8ad233401245e7b1a22a0eada6ba0769c2ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 29 Mar 2023 17:22:28 +0200 Subject: [PATCH 02/12] Adds `transactions` functions and some test --- primitives/trie/src/recorder.rs | 132 +++++++++++++++++++++++++++++--- 1 file changed, 122 insertions(+), 10 deletions(-) diff --git a/primitives/trie/src/recorder.rs b/primitives/trie/src/recorder.rs index f6c04470bd69f..2f584120d5d17 100644 --- a/primitives/trie/src/recorder.rs +++ b/primitives/trie/src/recorder.rs @@ -149,6 +149,54 @@ impl Recorder { mem::take(&mut *self.inner.lock()); self.encoded_size_estimation.store(0, Ordering::Relaxed); } + + pub fn start_transaction(&self) { + let mut inner = self.inner.lock(); + inner.accessed_nodes_transactions.push(Default::default()); + inner.recorded_keys_transactions.push(Default::default()); + } + + pub fn rollback_transaction(&self) { + let mut inner = self.inner.lock(); + + if let Some(nodes) = inner.accessed_nodes_transactions.pop() { + nodes.into_iter().for_each(|n| { + inner.accessed_nodes.remove(&n); + }) + } + + if let Some(keys_per_storage_root) = inner.recorded_keys_transactions.pop() { + keys_per_storage_root.into_iter().for_each(|(storage_root, keys)| { + keys.into_iter().for_each(|(k, old_state)| { + if let Some(state) = old_state { + inner.recorded_keys.entry(storage_root).or_default().insert(k, state); + } else { + inner.recorded_keys.entry(storage_root).or_default().remove(&k); + } + }); + }) + } + } + + pub fn commit_transaction(&self) { + let mut inner = self.inner.lock(); + + if let Some(nodes) = inner.accessed_nodes_transactions.pop() { + if let Some(parent) = inner.accessed_nodes_transactions.last_mut() { + parent.extend(nodes); + } + } + + if let Some(keys_per_storage_root) = inner.recorded_keys_transactions.pop() { + if let Some(parent) = inner.recorded_keys_transactions.last_mut() { + keys_per_storage_root.into_iter().for_each(|(storage_root, keys)| { + keys.into_iter().for_each(|(k, old_state)| { + parent.entry(storage_root).or_default().entry(k).or_insert(old_state); + }) + }); + } + } + } } /// The [`TrieRecorder`](trie_db::TrieRecorder) implementation. @@ -163,21 +211,15 @@ impl>> TrieRecorder fn update_recorded_keys(&mut self, full_key: &[u8], access: RecordedForKey) { let inner = self.inner.deref_mut(); - let entry = inner - .recorded_keys - .entry(self.storage_root) - .or_default() - .entry(full_key.into()); + let entry = + inner.recorded_keys.entry(self.storage_root).or_default().entry(full_key.into()); let key = entry.key().clone(); let entry = if matches!(access, RecordedForKey::Value) { entry.and_modify(|e| { if let Some(tx) = inner.recorded_keys_transactions.last_mut() { - tx.entry(self.storage_root) - .or_default() - .entry(key.clone()) - .or_insert(Some(*e)); + tx.entry(self.storage_root).or_default().entry(key.clone()).or_insert(Some(*e)); } *e = access; @@ -314,7 +356,7 @@ mod tests { type Recorder = super::Recorder; const TEST_DATA: &[(&[u8], &[u8])] = - &[(b"key1", b"val1"), (b"key2", b"val2"), (b"key3", b"val3"), (b"key4", b"val4")]; + &[(b"key1", &[1; 64]), (b"key2", &[2; 64]), (b"key3", &[3; 64]), (b"key4", &[4; 64])]; fn create_trie() -> (MemoryDB, TrieHash) { let mut db = MemoryDB::default(); @@ -351,4 +393,74 @@ mod tests { let trie = TrieDBBuilder::::new(&memory_db, &root).build(); assert_eq!(TEST_DATA[0].1.to_vec(), trie.get(TEST_DATA[0].0).unwrap().unwrap()); } + + #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] + struct RecorderStats { + accessed_nodes: usize, + recorded_keys: usize, + } + + impl RecorderStats { + fn extract(recorder: &Recorder) -> Self { + let inner = recorder.inner.lock(); + + let recorded_keys = + inner.recorded_keys.iter().map(|(_, keys)| keys.keys()).flatten().count(); + + Self { recorded_keys, accessed_nodes: inner.accessed_nodes.len() } + } + } + + #[test] + fn recorder_transactions_work() { + let (db, root) = create_trie(); + + let recorder = Recorder::default(); + let mut stats = Vec::new(); + + for i in 0..4 { + recorder.start_transaction(); + { + let mut trie_recorder = recorder.as_trie_recorder(root); + let trie = TrieDBBuilder::::new(&db, &root) + .with_recorder(&mut trie_recorder) + .build(); + + assert_eq!(TEST_DATA[i].1.to_vec(), trie.get(TEST_DATA[i].0).unwrap().unwrap()); + } + stats.push(RecorderStats::extract(&recorder)); + } + + assert_eq!(4, recorder.inner.lock().accessed_nodes_transactions.len()); + assert_eq!(4, recorder.inner.lock().recorded_keys_transactions.len()); + + for i in 0..5 { + if i == 4 { + assert_eq!(RecorderStats::default(), RecorderStats::extract(&recorder)); + } else { + assert_eq!(stats[3 - i], RecorderStats::extract(&recorder)); + } + + let storage_proof = recorder.to_storage_proof(); + let memory_db: MemoryDB = storage_proof.into_memory_db(); + + // Check that we recorded the required data + let trie = TrieDBBuilder::::new(&memory_db, &root).build(); + + // Check that the required data is still present. + for a in 0..4 { + if a < 4 - i { + assert_eq!(TEST_DATA[a].1.to_vec(), trie.get(TEST_DATA[a].0).unwrap().unwrap()); + } else { + // All the data that we already rolled back, should be gone! + assert!(trie.get(TEST_DATA[a].0).is_err()); + } + } + + recorder.rollback_transaction(); + } + + assert_eq!(0, recorder.inner.lock().accessed_nodes_transactions.len()); + assert_eq!(0, recorder.inner.lock().recorded_keys_transactions.len()); + } } From 5b64a4090cb02c47db31310cb2120da6ede6452e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 29 Mar 2023 22:39:49 +0200 Subject: [PATCH 03/12] More tests --- primitives/trie/src/recorder.rs | 221 +++++++++++++++++++++++++++++++- 1 file changed, 217 insertions(+), 4 deletions(-) diff --git a/primitives/trie/src/recorder.rs b/primitives/trie/src/recorder.rs index 2f584120d5d17..65a8e1ed51beb 100644 --- a/primitives/trie/src/recorder.rs +++ b/primitives/trie/src/recorder.rs @@ -159,11 +159,19 @@ impl Recorder { pub fn rollback_transaction(&self) { let mut inner = self.inner.lock(); + // We locked `inner` and can just update the encoded size locally and then store it back to + // the atomic. + let mut new_encoded_size_estimation = self.encoded_size_estimation.load(Ordering::Relaxed); if let Some(nodes) = inner.accessed_nodes_transactions.pop() { nodes.into_iter().for_each(|n| { - inner.accessed_nodes.remove(&n); + if let Some(old) = inner.accessed_nodes.remove(&n) { + new_encoded_size_estimation = + new_encoded_size_estimation.saturating_sub(old.encoded_size()); + } }) } + self.encoded_size_estimation + .store(new_encoded_size_estimation, Ordering::Relaxed); if let Some(keys_per_storage_root) = inner.recorded_keys_transactions.pop() { keys_per_storage_root.into_iter().for_each(|(storage_root, keys)| { @@ -227,6 +235,7 @@ impl>> TrieRecorder } else { entry }; + entry.or_insert_with(|| { if let Some(tx) = inner.recorded_keys_transactions.last_mut() { tx.entry(self.storage_root).or_default().entry(key).or_insert(None); @@ -349,7 +358,8 @@ impl>> trie_db::TrieRecord #[cfg(test)] mod tests { - use trie_db::{Trie, TrieDBBuilder, TrieDBMutBuilder, TrieHash, TrieMut}; + use super::*; + use trie_db::{Trie, TrieDBBuilder, TrieDBMutBuilder, TrieHash, TrieMut, TrieRecorder}; type MemoryDB = crate::MemoryDB; type Layout = crate::LayoutV1; @@ -398,6 +408,7 @@ mod tests { struct RecorderStats { accessed_nodes: usize, recorded_keys: usize, + estimated_size: usize, } impl RecorderStats { @@ -407,12 +418,16 @@ mod tests { let recorded_keys = inner.recorded_keys.iter().map(|(_, keys)| keys.keys()).flatten().count(); - Self { recorded_keys, accessed_nodes: inner.accessed_nodes.len() } + Self { + recorded_keys, + accessed_nodes: inner.accessed_nodes.len(), + estimated_size: recorder.estimate_encoded_size(), + } } } #[test] - fn recorder_transactions_work() { + fn recorder_transactions_rollback_work() { let (db, root) = create_trie(); let recorder = Recorder::default(); @@ -463,4 +478,202 @@ mod tests { assert_eq!(0, recorder.inner.lock().accessed_nodes_transactions.len()); assert_eq!(0, recorder.inner.lock().recorded_keys_transactions.len()); } + + #[test] + fn recorder_transactions_commit_work() { + let (db, root) = create_trie(); + + let recorder = Recorder::default(); + + for i in 0..4 { + recorder.start_transaction(); + { + let mut trie_recorder = recorder.as_trie_recorder(root); + let trie = TrieDBBuilder::::new(&db, &root) + .with_recorder(&mut trie_recorder) + .build(); + + assert_eq!(TEST_DATA[i].1.to_vec(), trie.get(TEST_DATA[i].0).unwrap().unwrap()); + } + } + + let stats = RecorderStats::extract(&recorder); + assert_eq!(4, recorder.inner.lock().accessed_nodes_transactions.len()); + assert_eq!(4, recorder.inner.lock().recorded_keys_transactions.len()); + + for _ in 0..4 { + recorder.commit_transaction(); + } + assert_eq!(stats, RecorderStats::extract(&recorder)); + + let storage_proof = recorder.to_storage_proof(); + let memory_db: MemoryDB = storage_proof.into_memory_db(); + + // Check that we recorded the required data + let trie = TrieDBBuilder::::new(&memory_db, &root).build(); + + // Check that the required data is still present. + for i in 0..4 { + assert_eq!(TEST_DATA[i].1.to_vec(), trie.get(TEST_DATA[i].0).unwrap().unwrap()); + } + } + + #[test] + fn recorder_transactions_commit_and_rollback_work() { + let (db, root) = create_trie(); + + let recorder = Recorder::default(); + + for i in 0..2 { + recorder.start_transaction(); + { + let mut trie_recorder = recorder.as_trie_recorder(root); + let trie = TrieDBBuilder::::new(&db, &root) + .with_recorder(&mut trie_recorder) + .build(); + + assert_eq!(TEST_DATA[i].1.to_vec(), trie.get(TEST_DATA[i].0).unwrap().unwrap()); + } + } + + recorder.rollback_transaction(); + + for i in 2..4 { + recorder.start_transaction(); + { + let mut trie_recorder = recorder.as_trie_recorder(root); + let trie = TrieDBBuilder::::new(&db, &root) + .with_recorder(&mut trie_recorder) + .build(); + + assert_eq!(TEST_DATA[i].1.to_vec(), trie.get(TEST_DATA[i].0).unwrap().unwrap()); + } + } + + recorder.rollback_transaction(); + + assert_eq!(2, recorder.inner.lock().accessed_nodes_transactions.len()); + assert_eq!(2, recorder.inner.lock().recorded_keys_transactions.len()); + + for _ in 0..2 { + recorder.commit_transaction(); + } + + assert_eq!(0, recorder.inner.lock().accessed_nodes_transactions.len()); + assert_eq!(0, recorder.inner.lock().recorded_keys_transactions.len()); + + let storage_proof = recorder.to_storage_proof(); + let memory_db: MemoryDB = storage_proof.into_memory_db(); + + // Check that we recorded the required data + let trie = TrieDBBuilder::::new(&memory_db, &root).build(); + + // Check that the required data is still present. + for i in 0..4 { + if i % 2 == 0 { + assert_eq!(TEST_DATA[i].1.to_vec(), trie.get(TEST_DATA[i].0).unwrap().unwrap()); + } else { + assert!(trie.get(TEST_DATA[i].0).is_err()); + } + } + } + + #[test] + fn recorder_transaction_accessed_keys_works() { + let key = TEST_DATA[0].0; + let (db, root) = create_trie(); + + let recorder = Recorder::default(); + + { + let trie_recorder = recorder.as_trie_recorder(root); + assert!(matches!(trie_recorder.trie_nodes_recorded_for_key(key), RecordedForKey::None)); + } + + recorder.start_transaction(); + { + let mut trie_recorder = recorder.as_trie_recorder(root); + let trie = TrieDBBuilder::::new(&db, &root) + .with_recorder(&mut trie_recorder) + .build(); + + assert_eq!( + sp_core::Blake2Hasher::hash(TEST_DATA[0].1), + trie.get_hash(TEST_DATA[0].0).unwrap().unwrap() + ); + assert!(matches!(trie_recorder.trie_nodes_recorded_for_key(key), RecordedForKey::Hash)); + } + + recorder.start_transaction(); + { + let mut trie_recorder = recorder.as_trie_recorder(root); + let trie = TrieDBBuilder::::new(&db, &root) + .with_recorder(&mut trie_recorder) + .build(); + + assert_eq!(TEST_DATA[0].1.to_vec(), trie.get(TEST_DATA[0].0).unwrap().unwrap()); + assert!(matches!( + trie_recorder.trie_nodes_recorded_for_key(key), + RecordedForKey::Value, + )); + } + + recorder.rollback_transaction(); + { + let trie_recorder = recorder.as_trie_recorder(root); + assert!(matches!(trie_recorder.trie_nodes_recorded_for_key(key), RecordedForKey::Hash)); + } + + recorder.rollback_transaction(); + { + let trie_recorder = recorder.as_trie_recorder(root); + assert!(matches!(trie_recorder.trie_nodes_recorded_for_key(key), RecordedForKey::None)); + } + + recorder.start_transaction(); + { + let mut trie_recorder = recorder.as_trie_recorder(root); + let trie = TrieDBBuilder::::new(&db, &root) + .with_recorder(&mut trie_recorder) + .build(); + + assert_eq!(TEST_DATA[0].1.to_vec(), trie.get(TEST_DATA[0].0).unwrap().unwrap()); + assert!(matches!( + trie_recorder.trie_nodes_recorded_for_key(key), + RecordedForKey::Value, + )); + } + + recorder.start_transaction(); + { + let mut trie_recorder = recorder.as_trie_recorder(root); + let trie = TrieDBBuilder::::new(&db, &root) + .with_recorder(&mut trie_recorder) + .build(); + + assert_eq!( + sp_core::Blake2Hasher::hash(TEST_DATA[0].1), + trie.get_hash(TEST_DATA[0].0).unwrap().unwrap() + ); + assert!(matches!( + trie_recorder.trie_nodes_recorded_for_key(key), + RecordedForKey::Value + )); + } + + recorder.rollback_transaction(); + { + let trie_recorder = recorder.as_trie_recorder(root); + assert!(matches!( + trie_recorder.trie_nodes_recorded_for_key(key), + RecordedForKey::Value + )); + } + + recorder.rollback_transaction(); + { + let trie_recorder = recorder.as_trie_recorder(root); + assert!(matches!(trie_recorder.trie_nodes_recorded_for_key(key), RecordedForKey::None)); + } + } } From 53f5bef318c563f3aa27b55e8c46a82e8e3d961f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 29 Mar 2023 23:49:30 +0200 Subject: [PATCH 04/12] Docs --- primitives/trie/src/recorder.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/primitives/trie/src/recorder.rs b/primitives/trie/src/recorder.rs index 65a8e1ed51beb..30af7b8ec4c06 100644 --- a/primitives/trie/src/recorder.rs +++ b/primitives/trie/src/recorder.rs @@ -45,6 +45,10 @@ struct RecorderInner { /// Mapping: `StorageRoot -> (Key -> RecordedForKey)`. recorded_keys: HashMap, RecordedForKey>>, + /// Stores transaction information about [`Self::recorded_keys`]. + /// + /// For each transaction we only store the `storage_root` and the old states per key. `None` + /// state means that the key wasn't recorded before. recorded_keys_transactions: Vec, Option>>>, /// The encoded nodes we accessed while recording. @@ -52,6 +56,9 @@ struct RecorderInner { /// Mapping: `Hash(Node) -> Node`. accessed_nodes: HashMap>, + /// Stores transaction information about [`Self::accessed_nodes`]. + /// + /// For each transaction we only store the hashes of added nodes. accessed_nodes_transactions: Vec>, } @@ -97,6 +104,8 @@ impl Recorder { /// /// - `storage_root`: The storage root of the trie for which accesses are recorded. This is /// important when recording access to different tries at once (like top and child tries). + /// + ///NOTE: This locks a mutex that stays locked until the return value is dropped. #[inline] pub fn as_trie_recorder( &self, @@ -150,12 +159,16 @@ impl Recorder { self.encoded_size_estimation.store(0, Ordering::Relaxed); } + /// Stat a new transaction. pub fn start_transaction(&self) { let mut inner = self.inner.lock(); inner.accessed_nodes_transactions.push(Default::default()); inner.recorded_keys_transactions.push(Default::default()); } + /// Rollback the latest transaction. + /// + /// If there isn't any transaction, nothing gonna happen. pub fn rollback_transaction(&self) { let mut inner = self.inner.lock(); @@ -186,6 +199,9 @@ impl Recorder { } } + /// Commit the latest transaction. + /// + /// If there isn't any transaction, nothing gonna happen. pub fn commit_transaction(&self) { let mut inner = self.inner.lock(); @@ -216,6 +232,7 @@ struct TrieRecorder { } impl>> TrieRecorder { + /// Update the recorded keys entry for the given `full_key`. fn update_recorded_keys(&mut self, full_key: &[u8], access: RecordedForKey) { let inner = self.inner.deref_mut(); @@ -224,9 +241,12 @@ impl>> TrieRecorder let key = entry.key().clone(); + // We don't need to update the record if we only accessed the `Hash` for the given + // `full_key`. Only `Value` access can be an upgrade from `Hash`. let entry = if matches!(access, RecordedForKey::Value) { entry.and_modify(|e| { if let Some(tx) = inner.recorded_keys_transactions.last_mut() { + // Store the previous state only once per transaction. tx.entry(self.storage_root).or_default().entry(key.clone()).or_insert(Some(*e)); } @@ -238,6 +258,7 @@ impl>> TrieRecorder entry.or_insert_with(|| { if let Some(tx) = inner.recorded_keys_transactions.last_mut() { + // The key wasn't yet recorded, so there isn't any old state. tx.entry(self.storage_root).or_default().entry(key).or_insert(None); } From 93b861fdd2cde9c7f4c3485c97190425a654fdfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Thu, 30 Mar 2023 14:14:25 +0200 Subject: [PATCH 05/12] Ensure that we rollback failed transactions in the storage proof --- client/block-builder/src/lib.rs | 60 ++++++++++++++++++- .../api/proc-macro/src/impl_runtime_apis.rs | 30 ++++++++-- test-utils/runtime/src/lib.rs | 12 +++- test-utils/runtime/src/system.rs | 26 ++++++++ 4 files changed, 120 insertions(+), 8 deletions(-) diff --git a/client/block-builder/src/lib.rs b/client/block-builder/src/lib.rs index d97afadd40156..2f41dca27dca7 100644 --- a/client/block-builder/src/lib.rs +++ b/client/block-builder/src/lib.rs @@ -312,7 +312,9 @@ mod tests { use sp_blockchain::HeaderBackend; use sp_core::Blake2Hasher; use sp_state_machine::Backend; - use substrate_test_runtime_client::{DefaultTestClientBuilderExt, TestClientBuilderExt}; + use substrate_test_runtime_client::{ + runtime::Extrinsic, DefaultTestClientBuilderExt, TestClientBuilderExt, + }; #[test] fn block_building_storage_proof_does_not_include_runtime_by_default() { @@ -345,4 +347,60 @@ mod tests { .unwrap_err() .contains("Database missing expected key"),); } + + #[test] + fn failing_extrinsic_rolls_back_changes_in_storage_proof() { + let builder = substrate_test_runtime_client::TestClientBuilder::new(); + let backend = builder.backend(); + let client = builder.build(); + + let mut block_builder = BlockBuilder::new( + &client, + client.info().best_hash, + client.info().best_number, + RecordProof::Yes, + Default::default(), + &*backend, + ) + .unwrap(); + + block_builder.push(Extrinsic::ReadAndPanic(8)).unwrap_err(); + + let block = block_builder.build().unwrap(); + + let proof_with_panic = block.proof.expect("Proof is build on request").encoded_size(); + + let mut block_builder = BlockBuilder::new( + &client, + client.info().best_hash, + client.info().best_number, + RecordProof::Yes, + Default::default(), + &*backend, + ) + .unwrap(); + + block_builder.push(Extrinsic::Read(8)).unwrap(); + + let block = block_builder.build().unwrap(); + + let proof_without_panic = block.proof.expect("Proof is build on request").encoded_size(); + + let block = BlockBuilder::new( + &client, + client.info().best_hash, + client.info().best_number, + RecordProof::Yes, + Default::default(), + &*backend, + ) + .unwrap().build().unwrap(); + + let proof_empty_block = block.proof.expect("Proof is build on request").encoded_size(); + + // Ensure that we rolled back the changes of the panicked transaction. + assert!(proof_without_panic > proof_with_panic); + assert!(proof_without_panic > proof_empty_block); + assert_eq!(proof_empty_block, proof_with_panic); + } } diff --git a/primitives/api/proc-macro/src/impl_runtime_apis.rs b/primitives/api/proc-macro/src/impl_runtime_apis.rs index 5ac07975df0f7..2fffcaca56371 100644 --- a/primitives/api/proc-macro/src/impl_runtime_apis.rs +++ b/primitives/api/proc-macro/src/impl_runtime_apis.rs @@ -236,7 +236,8 @@ fn generate_runtime_api_base_structures() -> Result { &self, call: F, ) -> R where Self: Sized { - #crate_::OverlayedChanges::start_transaction(&mut std::cell::RefCell::borrow_mut(&self.changes)); + self.start_transaction(); + *std::cell::RefCell::borrow_mut(&self.commit_on_success) = false; let res = call(self); *std::cell::RefCell::borrow_mut(&self.commit_on_success) = true; @@ -340,10 +341,18 @@ fn generate_runtime_api_base_structures() -> Result { transactions; qed"; if *std::cell::RefCell::borrow(&self.commit_on_success) { let res = if commit { + if let Some(recorder) = &self.recorder { + #crate_::ProofRecorder::::commit_transaction(&recorder); + } + #crate_::OverlayedChanges::commit_transaction( &mut std::cell::RefCell::borrow_mut(&self.changes) ) } else { + if let Some(recorder) = &self.recorder { + #crate_::ProofRecorder::::rollback_transaction(&recorder); + } + #crate_::OverlayedChanges::rollback_transaction( &mut std::cell::RefCell::borrow_mut(&self.changes) ) @@ -352,6 +361,19 @@ fn generate_runtime_api_base_structures() -> Result { std::result::Result::expect(res, proof); } } + + fn start_transaction(&self) { + if !*std::cell::RefCell::borrow(&self.commit_on_success) { + return + } + + #crate_::OverlayedChanges::start_transaction( + &mut std::cell::RefCell::borrow_mut(&self.changes) + ); + if let Some(recorder) = &self.recorder { + #crate_::ProofRecorder::::start_transaction(&recorder); + } + } } )) } @@ -443,11 +465,7 @@ impl<'a> ApiRuntimeImplToApiRuntimeApiImpl<'a> { params: std::vec::Vec, fn_name: &dyn Fn(#crate_::RuntimeVersion) -> &'static str, ) -> std::result::Result, #crate_::ApiError> { - if *std::cell::RefCell::borrow(&self.commit_on_success) { - #crate_::OverlayedChanges::start_transaction( - &mut std::cell::RefCell::borrow_mut(&self.changes) - ); - } + self.start_transaction(); let res = (|| { let version = #crate_::CallApiAt::<__SrApiBlock__>::runtime_version_at( diff --git a/test-utils/runtime/src/lib.rs b/test-utils/runtime/src/lib.rs index c9a0ac04d63ba..b5600843c2749 100644 --- a/test-utils/runtime/src/lib.rs +++ b/test-utils/runtime/src/lib.rs @@ -164,13 +164,21 @@ pub enum Extrinsic { OffchainIndexSet(Vec, Vec), OffchainIndexClear(Vec), Store(Vec), + /// Read X times from the state some data and then panic! + /// + /// Returns `Ok` if it didn't read anything. + ReadAndPanic(u32), + /// Read X times from the state some data. + /// + /// Panics if it can not read `X` times. + Read(u32), } #[cfg(feature = "std")] impl serde::Serialize for Extrinsic { fn serialize(&self, seq: S) -> Result where - S: ::serde::Serializer, + S: serde::Serializer, { self.using_encoded(|bytes| seq.serialize_bytes(bytes)) } @@ -210,6 +218,8 @@ impl BlindCheckable for Extrinsic { Extrinsic::OffchainIndexSet(key, value) => Ok(Extrinsic::OffchainIndexSet(key, value)), Extrinsic::OffchainIndexClear(key) => Ok(Extrinsic::OffchainIndexClear(key)), Extrinsic::Store(data) => Ok(Extrinsic::Store(data)), + Extrinsic::ReadAndPanic(i) => Ok(Extrinsic::ReadAndPanic(i)), + Extrinsic::Read(i) => Ok(Extrinsic::Read(i)), } } } diff --git a/test-utils/runtime/src/system.rs b/test-utils/runtime/src/system.rs index 12ebf486bb1b9..fc750531529b6 100644 --- a/test-utils/runtime/src/system.rs +++ b/test-utils/runtime/src/system.rs @@ -275,6 +275,32 @@ fn execute_transaction_backend(utx: &Extrinsic, extrinsic_index: u32) -> ApplyEx Ok(Ok(())) }, Extrinsic::Store(data) => execute_store(data.clone()), + Extrinsic::ReadAndPanic(i) => execute_read(*i, true), + Extrinsic::Read(i) => execute_read(*i, false), + } +} + +fn execute_read(read: u32, panic_at_end: bool) -> ApplyExtrinsicResult { + let mut next_key = vec![]; + for _ in 0..(read as usize) { + if let Some(next) = sp_io::storage::next_key(&next_key) { + // Read the value + sp_io::storage::get(&next); + + next_key = next; + } else { + if panic_at_end { + return Ok(Ok(())) + } else { + panic!("Could not read {read} times from the state"); + } + } + } + + if panic_at_end { + panic!("BYE") + } else { + Ok(Ok(())) } } From 5b36ad065fd490e5d4d15677094fb1189696f6cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Thu, 30 Mar 2023 16:18:58 +0200 Subject: [PATCH 06/12] FMT --- client/block-builder/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/client/block-builder/src/lib.rs b/client/block-builder/src/lib.rs index 2f41dca27dca7..21f8e6fdd3999 100644 --- a/client/block-builder/src/lib.rs +++ b/client/block-builder/src/lib.rs @@ -394,7 +394,9 @@ mod tests { Default::default(), &*backend, ) - .unwrap().build().unwrap(); + .unwrap() + .build() + .unwrap(); let proof_empty_block = block.proof.expect("Proof is build on request").encoded_size(); From 79102451fb8d29077681d4c930c135758987a27a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Fri, 31 Mar 2023 11:45:33 +0200 Subject: [PATCH 07/12] Update primitives/trie/src/recorder.rs Co-authored-by: Dmitry Markin --- primitives/trie/src/recorder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/primitives/trie/src/recorder.rs b/primitives/trie/src/recorder.rs index 30af7b8ec4c06..abf99ebba2958 100644 --- a/primitives/trie/src/recorder.rs +++ b/primitives/trie/src/recorder.rs @@ -105,7 +105,7 @@ impl Recorder { /// - `storage_root`: The storage root of the trie for which accesses are recorded. This is /// important when recording access to different tries at once (like top and child tries). /// - ///NOTE: This locks a mutex that stays locked until the return value is dropped. + /// NOTE: This locks a mutex that stays locked until the return value is dropped. #[inline] pub fn as_trie_recorder( &self, From 78242800f0ee25fa0e3e6e19c17daa082a213fd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Sat, 1 Apr 2023 23:54:06 +0200 Subject: [PATCH 08/12] Review comments --- .../api/proc-macro/src/impl_runtime_apis.rs | 32 +++- primitives/trie/src/recorder.rs | 177 +++++++++--------- 2 files changed, 115 insertions(+), 94 deletions(-) diff --git a/primitives/api/proc-macro/src/impl_runtime_apis.rs b/primitives/api/proc-macro/src/impl_runtime_apis.rs index 2fffcaca56371..7e2b36bceaae4 100644 --- a/primitives/api/proc-macro/src/impl_runtime_apis.rs +++ b/primitives/api/proc-macro/src/impl_runtime_apis.rs @@ -341,21 +341,33 @@ fn generate_runtime_api_base_structures() -> Result { transactions; qed"; if *std::cell::RefCell::borrow(&self.commit_on_success) { let res = if commit { - if let Some(recorder) = &self.recorder { - #crate_::ProofRecorder::::commit_transaction(&recorder); - } + let res = if let Some(recorder) = &self.recorder { + #crate_::ProofRecorder::::commit_transaction(&recorder) + } else { + Ok(()) + }; - #crate_::OverlayedChanges::commit_transaction( + let res2 = #crate_::OverlayedChanges::commit_transaction( &mut std::cell::RefCell::borrow_mut(&self.changes) - ) + ); + + // Will panic on an `Err` below, however we should call commit + // on the recorder and the changes together. + std::result::Result::and(res, std::result::Result::map_err(res2, drop)) } else { - if let Some(recorder) = &self.recorder { - #crate_::ProofRecorder::::rollback_transaction(&recorder); - } + let res = if let Some(recorder) = &self.recorder { + #crate_::ProofRecorder::::rollback_transaction(&recorder) + } else { + Ok(()) + }; - #crate_::OverlayedChanges::rollback_transaction( + let res2 = #crate_::OverlayedChanges::rollback_transaction( &mut std::cell::RefCell::borrow_mut(&self.changes) - ) + ); + + // Will panic on an `Err` below, however we should call commit + // on the recorder and the changes together. + std::result::Result::and(res, std::result::Result::map_err(res2, drop)) }; std::result::Result::expect(res, proof); diff --git a/primitives/trie/src/recorder.rs b/primitives/trie/src/recorder.rs index abf99ebba2958..8edfca36631f8 100644 --- a/primitives/trie/src/recorder.rs +++ b/primitives/trie/src/recorder.rs @@ -38,6 +38,20 @@ use trie_db::{RecordedForKey, TrieAccess}; const LOG_TARGET: &str = "trie-recorder"; +/// Stores all the information per transaction. +#[derive(Default)] +struct Transaction { + /// Stores transaction information about [`RecorderInner::recorded_keys`]. + /// + /// For each transaction we only store the `storage_root` and the old states per key. `None` + /// state means that the key wasn't recorded before. + recorded_keys: HashMap, Option>>, + /// Stores transaction information about [`RecorderInner::accessed_nodes`]. + /// + /// For each transaction we only store the hashes of added nodes. + accessed_nodes: HashSet, +} + /// The internals of [`Recorder`]. struct RecorderInner { /// The keys for that we have recorded the trie nodes and if we have recorded up to the value. @@ -45,21 +59,13 @@ struct RecorderInner { /// Mapping: `StorageRoot -> (Key -> RecordedForKey)`. recorded_keys: HashMap, RecordedForKey>>, - /// Stores transaction information about [`Self::recorded_keys`]. - /// - /// For each transaction we only store the `storage_root` and the old states per key. `None` - /// state means that the key wasn't recorded before. - recorded_keys_transactions: Vec, Option>>>, + /// Currently active transactions. + transactions: Vec>, /// The encoded nodes we accessed while recording. /// /// Mapping: `Hash(Node) -> Node`. accessed_nodes: HashMap>, - - /// Stores transaction information about [`Self::accessed_nodes`]. - /// - /// For each transaction we only store the hashes of added nodes. - accessed_nodes_transactions: Vec>, } impl Default for RecorderInner { @@ -67,8 +73,7 @@ impl Default for RecorderInner { Self { recorded_keys: Default::default(), accessed_nodes: Default::default(), - recorded_keys_transactions: Vec::new(), - accessed_nodes_transactions: Vec::new(), + transactions: Vec::new(), } } } @@ -162,64 +167,67 @@ impl Recorder { /// Stat a new transaction. pub fn start_transaction(&self) { let mut inner = self.inner.lock(); - inner.accessed_nodes_transactions.push(Default::default()); - inner.recorded_keys_transactions.push(Default::default()); + inner.transactions.push(Default::default()); } /// Rollback the latest transaction. /// - /// If there isn't any transaction, nothing gonna happen. - pub fn rollback_transaction(&self) { + /// Returns an error if there wasn't any active transaction. + pub fn rollback_transaction(&self) -> Result<(), ()> { let mut inner = self.inner.lock(); // We locked `inner` and can just update the encoded size locally and then store it back to // the atomic. let mut new_encoded_size_estimation = self.encoded_size_estimation.load(Ordering::Relaxed); - if let Some(nodes) = inner.accessed_nodes_transactions.pop() { - nodes.into_iter().for_each(|n| { - if let Some(old) = inner.accessed_nodes.remove(&n) { - new_encoded_size_estimation = - new_encoded_size_estimation.saturating_sub(old.encoded_size()); + let transaction = inner.transactions.pop().ok_or(())?; + + transaction.accessed_nodes.into_iter().for_each(|n| { + if let Some(old) = inner.accessed_nodes.remove(&n) { + new_encoded_size_estimation = + new_encoded_size_estimation.saturating_sub(old.encoded_size()); + } + }); + + transaction.recorded_keys.into_iter().for_each(|(storage_root, keys)| { + keys.into_iter().for_each(|(k, old_state)| { + if let Some(state) = old_state { + inner.recorded_keys.entry(storage_root).or_default().insert(k, state); + } else { + inner.recorded_keys.entry(storage_root).or_default().remove(&k); } - }) - } + }); + }); + self.encoded_size_estimation .store(new_encoded_size_estimation, Ordering::Relaxed); - if let Some(keys_per_storage_root) = inner.recorded_keys_transactions.pop() { - keys_per_storage_root.into_iter().for_each(|(storage_root, keys)| { - keys.into_iter().for_each(|(k, old_state)| { - if let Some(state) = old_state { - inner.recorded_keys.entry(storage_root).or_default().insert(k, state); - } else { - inner.recorded_keys.entry(storage_root).or_default().remove(&k); - } - }); - }) - } + Ok(()) } /// Commit the latest transaction. /// - /// If there isn't any transaction, nothing gonna happen. - pub fn commit_transaction(&self) { + /// Returns an error if there wasn't any active transaction. + pub fn commit_transaction(&self) -> Result<(), ()> { let mut inner = self.inner.lock(); - if let Some(nodes) = inner.accessed_nodes_transactions.pop() { - if let Some(parent) = inner.accessed_nodes_transactions.last_mut() { - parent.extend(nodes); - } - } + let transaction = inner.transactions.pop().ok_or(())?; - if let Some(keys_per_storage_root) = inner.recorded_keys_transactions.pop() { - if let Some(parent) = inner.recorded_keys_transactions.last_mut() { - keys_per_storage_root.into_iter().for_each(|(storage_root, keys)| { - keys.into_iter().for_each(|(k, old_state)| { - parent.entry(storage_root).or_default().entry(k).or_insert(old_state); - }) - }); - } + if let Some(parent_transaction) = inner.transactions.last_mut() { + parent_transaction.accessed_nodes.extend(transaction.accessed_nodes); + + transaction.recorded_keys.into_iter().for_each(|(storage_root, keys)| { + keys.into_iter().for_each(|(k, old_state)| { + parent_transaction + .recorded_keys + .entry(storage_root) + .or_default() + .entry(k) + .or_insert(old_state); + }) + }); } + + Ok(()) } } @@ -245,9 +253,13 @@ impl>> TrieRecorder // `full_key`. Only `Value` access can be an upgrade from `Hash`. let entry = if matches!(access, RecordedForKey::Value) { entry.and_modify(|e| { - if let Some(tx) = inner.recorded_keys_transactions.last_mut() { + if let Some(tx) = inner.transactions.last_mut() { // Store the previous state only once per transaction. - tx.entry(self.storage_root).or_default().entry(key.clone()).or_insert(Some(*e)); + tx.recorded_keys + .entry(self.storage_root) + .or_default() + .entry(key.clone()) + .or_insert(Some(*e)); } *e = access; @@ -257,9 +269,13 @@ impl>> TrieRecorder }; entry.or_insert_with(|| { - if let Some(tx) = inner.recorded_keys_transactions.last_mut() { + if let Some(tx) = inner.transactions.last_mut() { // The key wasn't yet recorded, so there isn't any old state. - tx.entry(self.storage_root).or_default().entry(key).or_insert(None); + tx.recorded_keys + .entry(self.storage_root) + .or_default() + .entry(key) + .or_insert(None); } access @@ -288,8 +304,8 @@ impl>> trie_db::TrieRecord encoded_size_update += node.encoded_size(); - if let Some(tx) = inner.accessed_nodes_transactions.last_mut() { - tx.insert(hash); + if let Some(tx) = inner.transactions.last_mut() { + tx.accessed_nodes.insert(hash); } node @@ -309,8 +325,8 @@ impl>> trie_db::TrieRecord encoded_size_update += node.encoded_size(); - if let Some(tx) = inner.accessed_nodes_transactions.last_mut() { - tx.insert(hash); + if let Some(tx) = inner.transactions.last_mut() { + tx.accessed_nodes.insert(hash); } node @@ -331,8 +347,8 @@ impl>> trie_db::TrieRecord encoded_size_update += value.encoded_size(); - if let Some(tx) = inner.accessed_nodes_transactions.last_mut() { - tx.insert(hash); + if let Some(tx) = inner.transactions.last_mut() { + tx.accessed_nodes.insert(hash); } value @@ -452,7 +468,7 @@ mod tests { let (db, root) = create_trie(); let recorder = Recorder::default(); - let mut stats = Vec::new(); + let mut stats = vec![RecorderStats::default()]; for i in 0..4 { recorder.start_transaction(); @@ -467,15 +483,10 @@ mod tests { stats.push(RecorderStats::extract(&recorder)); } - assert_eq!(4, recorder.inner.lock().accessed_nodes_transactions.len()); - assert_eq!(4, recorder.inner.lock().recorded_keys_transactions.len()); + assert_eq!(4, recorder.inner.lock().transactions.len()); for i in 0..5 { - if i == 4 { - assert_eq!(RecorderStats::default(), RecorderStats::extract(&recorder)); - } else { - assert_eq!(stats[3 - i], RecorderStats::extract(&recorder)); - } + assert_eq!(stats[4 - i], RecorderStats::extract(&recorder)); let storage_proof = recorder.to_storage_proof(); let memory_db: MemoryDB = storage_proof.into_memory_db(); @@ -493,11 +504,12 @@ mod tests { } } - recorder.rollback_transaction(); + if i < 4 { + recorder.rollback_transaction().unwrap(); + } } - assert_eq!(0, recorder.inner.lock().accessed_nodes_transactions.len()); - assert_eq!(0, recorder.inner.lock().recorded_keys_transactions.len()); + assert_eq!(0, recorder.inner.lock().transactions.len()); } #[test] @@ -519,11 +531,10 @@ mod tests { } let stats = RecorderStats::extract(&recorder); - assert_eq!(4, recorder.inner.lock().accessed_nodes_transactions.len()); - assert_eq!(4, recorder.inner.lock().recorded_keys_transactions.len()); + assert_eq!(4, recorder.inner.lock().transactions.len()); for _ in 0..4 { - recorder.commit_transaction(); + recorder.commit_transaction().unwrap(); } assert_eq!(stats, RecorderStats::extract(&recorder)); @@ -557,7 +568,7 @@ mod tests { } } - recorder.rollback_transaction(); + recorder.rollback_transaction().unwrap(); for i in 2..4 { recorder.start_transaction(); @@ -571,17 +582,15 @@ mod tests { } } - recorder.rollback_transaction(); + recorder.rollback_transaction().unwrap(); - assert_eq!(2, recorder.inner.lock().accessed_nodes_transactions.len()); - assert_eq!(2, recorder.inner.lock().recorded_keys_transactions.len()); + assert_eq!(2, recorder.inner.lock().transactions.len()); for _ in 0..2 { - recorder.commit_transaction(); + recorder.commit_transaction().unwrap(); } - assert_eq!(0, recorder.inner.lock().accessed_nodes_transactions.len()); - assert_eq!(0, recorder.inner.lock().recorded_keys_transactions.len()); + assert_eq!(0, recorder.inner.lock().transactions.len()); let storage_proof = recorder.to_storage_proof(); let memory_db: MemoryDB = storage_proof.into_memory_db(); @@ -639,13 +648,13 @@ mod tests { )); } - recorder.rollback_transaction(); + recorder.rollback_transaction().unwrap(); { let trie_recorder = recorder.as_trie_recorder(root); assert!(matches!(trie_recorder.trie_nodes_recorded_for_key(key), RecordedForKey::Hash)); } - recorder.rollback_transaction(); + recorder.rollback_transaction().unwrap(); { let trie_recorder = recorder.as_trie_recorder(root); assert!(matches!(trie_recorder.trie_nodes_recorded_for_key(key), RecordedForKey::None)); @@ -682,7 +691,7 @@ mod tests { )); } - recorder.rollback_transaction(); + recorder.rollback_transaction().unwrap(); { let trie_recorder = recorder.as_trie_recorder(root); assert!(matches!( @@ -691,7 +700,7 @@ mod tests { )); } - recorder.rollback_transaction(); + recorder.rollback_transaction().unwrap(); { let trie_recorder = recorder.as_trie_recorder(root); assert!(matches!(trie_recorder.trie_nodes_recorded_for_key(key), RecordedForKey::None)); From 622a633c1771a878e94b402594798cdb3742bb1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 3 Apr 2023 22:05:12 +0200 Subject: [PATCH 09/12] Update primitives/trie/src/recorder.rs Co-authored-by: Sebastian Kunert --- primitives/trie/src/recorder.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/primitives/trie/src/recorder.rs b/primitives/trie/src/recorder.rs index 8edfca36631f8..addc35f16e8dc 100644 --- a/primitives/trie/src/recorder.rs +++ b/primitives/trie/src/recorder.rs @@ -536,6 +536,7 @@ mod tests { for _ in 0..4 { recorder.commit_transaction().unwrap(); } + assert_eq!(0, recorder.inner.lock().transactions.len()); assert_eq!(stats, RecorderStats::extract(&recorder)); let storage_proof = recorder.to_storage_proof(); From 99f41d2e24c5f09a949abf64a40c2ddb7d2b44b4 Mon Sep 17 00:00:00 2001 From: command-bot <> Date: Mon, 3 Apr 2023 20:28:20 +0000 Subject: [PATCH 10/12] ".git/.scripts/commands/fmt/fmt.sh" --- primitives/trie/src/recorder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/primitives/trie/src/recorder.rs b/primitives/trie/src/recorder.rs index addc35f16e8dc..fea209a558fa3 100644 --- a/primitives/trie/src/recorder.rs +++ b/primitives/trie/src/recorder.rs @@ -536,7 +536,7 @@ mod tests { for _ in 0..4 { recorder.commit_transaction().unwrap(); } - assert_eq!(0, recorder.inner.lock().transactions.len()); + assert_eq!(0, recorder.inner.lock().transactions.len()); assert_eq!(stats, RecorderStats::extract(&recorder)); let storage_proof = recorder.to_storage_proof(); From 3bc9cbe5b2e81ddba32a598999222d6079f2c278 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 5 Apr 2023 16:07:51 +0200 Subject: [PATCH 11/12] For the holy clippy! --- primitives/trie/src/recorder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/primitives/trie/src/recorder.rs b/primitives/trie/src/recorder.rs index fea209a558fa3..a24e85bb86d74 100644 --- a/primitives/trie/src/recorder.rs +++ b/primitives/trie/src/recorder.rs @@ -453,7 +453,7 @@ mod tests { let inner = recorder.inner.lock(); let recorded_keys = - inner.recorded_keys.iter().map(|(_, keys)| keys.keys()).flatten().count(); + inner.recorded_keys.iter().flat_map(|(_, keys)| keys.keys()).count(); Self { recorded_keys, From a346fcada075787e8d06f62b7a5501ce308555a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 5 Apr 2023 16:08:11 +0200 Subject: [PATCH 12/12] Update primitives/trie/src/recorder.rs Co-authored-by: Anton --- primitives/trie/src/recorder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/primitives/trie/src/recorder.rs b/primitives/trie/src/recorder.rs index a24e85bb86d74..728dc836205b5 100644 --- a/primitives/trie/src/recorder.rs +++ b/primitives/trie/src/recorder.rs @@ -164,7 +164,7 @@ impl Recorder { self.encoded_size_estimation.store(0, Ordering::Relaxed); } - /// Stat a new transaction. + /// Start a new transaction. pub fn start_transaction(&self) { let mut inner = self.inner.lock(); inner.transactions.push(Default::default());