From 0e48dc9debe1c8db24eb9f056b006816f4061765 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Wed, 18 May 2022 17:30:57 +0100 Subject: [PATCH 01/38] Fix overlay prefix removal result --- primitives/state-machine/src/ext.rs | 5 +++-- .../state-machine/src/overlayed_changes/changeset.rs | 7 +++++-- primitives/state-machine/src/overlayed_changes/mod.rs | 4 ++-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/primitives/state-machine/src/ext.rs b/primitives/state-machine/src/ext.rs index e33569e2a1f67..2d446b9c3b9a0 100644 --- a/primitives/state-machine/src/ext.rs +++ b/primitives/state-machine/src/ext.rs @@ -464,8 +464,9 @@ where } self.mark_dirty(); - self.overlay.clear_prefix(prefix); - self.limit_remove_from_backend(None, Some(prefix), limit) + let overlay_count = self.overlay.clear_prefix(prefix); + let (all, count) = self.limit_remove_from_backend(None, Some(prefix), limit); + (all, count + overlay_count) } fn clear_child_prefix( diff --git a/primitives/state-machine/src/overlayed_changes/changeset.rs b/primitives/state-machine/src/overlayed_changes/changeset.rs index 9e7f6ffeddfd7..1d9eb0dea2b1b 100644 --- a/primitives/state-machine/src/overlayed_changes/changeset.rs +++ b/primitives/state-machine/src/overlayed_changes/changeset.rs @@ -410,10 +410,13 @@ impl OverlayedChangeSet { &mut self, predicate: impl Fn(&[u8], &OverlayedValue) -> bool, at_extrinsic: Option, - ) { + ) -> u32 { + let mut count = 0; for (key, val) in self.changes.iter_mut().filter(|(k, v)| predicate(k, v)) { - val.set(None, insert_dirty(&mut self.dirty_keys, key.clone()), at_extrinsic); + val.set(None, insert_dirty(&mut self.dirty_keys, key.clone()), at_extrinsic); + count += 1; } + count } /// Get the iterator over all changes that follow the supplied `key`. diff --git a/primitives/state-machine/src/overlayed_changes/mod.rs b/primitives/state-machine/src/overlayed_changes/mod.rs index 2161b343711c9..2bb09e98bff62 100644 --- a/primitives/state-machine/src/overlayed_changes/mod.rs +++ b/primitives/state-machine/src/overlayed_changes/mod.rs @@ -315,8 +315,8 @@ impl OverlayedChanges { /// Removes all key-value pairs which keys share the given prefix. /// /// Can be rolled back or committed when called inside a transaction. - pub(crate) fn clear_prefix(&mut self, prefix: &[u8]) { - self.top.clear_where(|key, _| key.starts_with(prefix), self.extrinsic_index()); + pub(crate) fn clear_prefix(&mut self, prefix: &[u8]) -> u32 { + self.top.clear_where(|key, _| key.starts_with(prefix), self.extrinsic_index()) } /// Removes all key-value pairs which keys share the given prefix. From ae9b285855a98ef0a765a2045e619843e90a7937 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Wed, 18 May 2022 17:48:21 +0100 Subject: [PATCH 02/38] Second part of the overlay prefix removal fix. --- primitives/state-machine/src/overlayed_changes/changeset.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/primitives/state-machine/src/overlayed_changes/changeset.rs b/primitives/state-machine/src/overlayed_changes/changeset.rs index 1d9eb0dea2b1b..ae5d47f6bb943 100644 --- a/primitives/state-machine/src/overlayed_changes/changeset.rs +++ b/primitives/state-machine/src/overlayed_changes/changeset.rs @@ -413,8 +413,10 @@ impl OverlayedChangeSet { ) -> u32 { let mut count = 0; for (key, val) in self.changes.iter_mut().filter(|(k, v)| predicate(k, v)) { - val.set(None, insert_dirty(&mut self.dirty_keys, key.clone()), at_extrinsic); + if val.value_ref().is_some() { count += 1; + } + val.set(None, insert_dirty(&mut self.dirty_keys, key.clone()), at_extrinsic); } count } From 7f80f123a732276d0aa722ac23ff28d49be166c6 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Fri, 20 May 2022 12:25:22 +0100 Subject: [PATCH 03/38] Report only items deleted from storage in clear_prefix --- primitives/state-machine/src/ext.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/primitives/state-machine/src/ext.rs b/primitives/state-machine/src/ext.rs index 2d446b9c3b9a0..57201d093756a 100644 --- a/primitives/state-machine/src/ext.rs +++ b/primitives/state-machine/src/ext.rs @@ -464,9 +464,9 @@ where } self.mark_dirty(); - let overlay_count = self.overlay.clear_prefix(prefix); + let _overlay_count = self.overlay.clear_prefix(prefix); let (all, count) = self.limit_remove_from_backend(None, Some(prefix), limit); - (all, count + overlay_count) + (all, count) } fn clear_child_prefix( @@ -485,8 +485,9 @@ where let _guard = guard(); self.mark_dirty(); - self.overlay.clear_child_prefix(child_info, prefix); - self.limit_remove_from_backend(Some(child_info), Some(prefix), limit) + let _overlay_count = self.overlay.clear_child_prefix(child_info, prefix); + let (all, count) = self.limit_remove_from_backend(Some(child_info), Some(prefix), limit); + (all, count) } fn storage_append(&mut self, key: Vec, value: Vec) { From 3e580e532cc0b3fb8592c507350f1632fe006db9 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Fri, 20 May 2022 14:01:08 +0100 Subject: [PATCH 04/38] Fix kill_prefix --- primitives/io/src/lib.rs | 5 +++++ primitives/state-machine/src/ext.rs | 16 ++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index 8f62b03b62ea9..90c43ac517b2a 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -1766,6 +1766,11 @@ mod tests { assert!(storage::get(b":abdd").is_some()); assert!(storage::get(b":abcd").is_none()); assert!(storage::get(b":abc").is_none()); + + assert!(matches!( + storage::clear_prefix(b":abc", None), + KillStorageResult::AllRemoved(0) + )); }); } diff --git a/primitives/state-machine/src/ext.rs b/primitives/state-machine/src/ext.rs index 57201d093756a..0a156e1ae854b 100644 --- a/primitives/state-machine/src/ext.rs +++ b/primitives/state-machine/src/ext.rs @@ -465,6 +465,7 @@ where self.mark_dirty(); let _overlay_count = self.overlay.clear_prefix(prefix); +//, Some(&self.overlay.changes) let (all, count) = self.limit_remove_from_backend(None, Some(prefix), limit); (all, count) } @@ -486,6 +487,7 @@ where self.mark_dirty(); let _overlay_count = self.overlay.clear_child_prefix(child_info, prefix); +//, Some(&self.overlay.changes) let (all, count) = self.limit_remove_from_backend(Some(child_info), Some(prefix), limit); (all, count) } @@ -743,6 +745,13 @@ where all_deleted = false; return false } + if let Some(None) = match child_info { + Some(child_info) => self.overlay.child_storage(child_info, key), + None => self.overlay.storage(key), + } { + // already deleted. + return true; + } if let Some(num) = num_deleted.checked_add(1) { num_deleted = num; } else { @@ -759,6 +768,13 @@ where (all_deleted, num_deleted) } else { self.backend.apply_to_keys_while(child_info, prefix, |key| { + if let Some(None) = match child_info { + Some(child_info) => self.overlay.child_storage(child_info, key), + None => self.overlay.storage(key), + } { + // already deleted. + return true; + } num_deleted = num_deleted.saturating_add(1); if let Some(child_info) = child_info { self.overlay.set_child_storage(child_info, key.to_vec(), None); From 4a5ff62437c131cf5f04ad5f759d2f97d1a813de Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Fri, 20 May 2022 14:04:17 +0100 Subject: [PATCH 05/38] Formatting --- primitives/state-machine/src/ext.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/primitives/state-machine/src/ext.rs b/primitives/state-machine/src/ext.rs index 0a156e1ae854b..357b4afb9b2fd 100644 --- a/primitives/state-machine/src/ext.rs +++ b/primitives/state-machine/src/ext.rs @@ -465,7 +465,7 @@ where self.mark_dirty(); let _overlay_count = self.overlay.clear_prefix(prefix); -//, Some(&self.overlay.changes) + //, Some(&self.overlay.changes) let (all, count) = self.limit_remove_from_backend(None, Some(prefix), limit); (all, count) } @@ -487,7 +487,7 @@ where self.mark_dirty(); let _overlay_count = self.overlay.clear_child_prefix(child_info, prefix); -//, Some(&self.overlay.changes) + //, Some(&self.overlay.changes) let (all, count) = self.limit_remove_from_backend(Some(child_info), Some(prefix), limit); (all, count) } @@ -750,7 +750,7 @@ where None => self.overlay.storage(key), } { // already deleted. - return true; + return true } if let Some(num) = num_deleted.checked_add(1) { num_deleted = num; @@ -773,7 +773,7 @@ where None => self.overlay.storage(key), } { // already deleted. - return true; + return true } num_deleted = num_deleted.saturating_add(1); if let Some(child_info) = child_info { From 8d56c139f1feed60ebd3c59b1b1215a9ffb29250 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Fri, 20 May 2022 14:06:55 +0100 Subject: [PATCH 06/38] Remove unused code --- primitives/state-machine/src/ext.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/primitives/state-machine/src/ext.rs b/primitives/state-machine/src/ext.rs index 357b4afb9b2fd..6a9635bd26638 100644 --- a/primitives/state-machine/src/ext.rs +++ b/primitives/state-machine/src/ext.rs @@ -464,8 +464,8 @@ where } self.mark_dirty(); + // In the future, we should report this back to the caller. let _overlay_count = self.overlay.clear_prefix(prefix); - //, Some(&self.overlay.changes) let (all, count) = self.limit_remove_from_backend(None, Some(prefix), limit); (all, count) } @@ -486,8 +486,8 @@ where let _guard = guard(); self.mark_dirty(); + // In the future, we should report this back to the caller. let _overlay_count = self.overlay.clear_child_prefix(child_info, prefix); - //, Some(&self.overlay.changes) let (all, count) = self.limit_remove_from_backend(Some(child_info), Some(prefix), limit); (all, count) } From 91362cd60b6c5453209bae0b7e09dd0eb84f986f Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Fri, 20 May 2022 14:09:26 +0100 Subject: [PATCH 07/38] Fixes --- primitives/state-machine/src/overlayed_changes/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/primitives/state-machine/src/overlayed_changes/mod.rs b/primitives/state-machine/src/overlayed_changes/mod.rs index 2bb09e98bff62..fdaa5ce787861 100644 --- a/primitives/state-machine/src/overlayed_changes/mod.rs +++ b/primitives/state-machine/src/overlayed_changes/mod.rs @@ -322,7 +322,7 @@ impl OverlayedChanges { /// Removes all key-value pairs which keys share the given prefix. /// /// Can be rolled back or committed when called inside a transaction - pub(crate) fn clear_child_prefix(&mut self, child_info: &ChildInfo, prefix: &[u8]) { + pub(crate) fn clear_child_prefix(&mut self, child_info: &ChildInfo, prefix: &[u8]) -> u32 { let extrinsic_index = self.extrinsic_index(); let storage_key = child_info.storage_key().to_vec(); let top = &self.top; @@ -332,7 +332,7 @@ impl OverlayedChanges { .or_insert_with(|| (top.spawn_child(), child_info.clone())); let updatable = info.try_update(child_info); debug_assert!(updatable); - changeset.clear_where(|key, _| key.starts_with(prefix), extrinsic_index); + changeset.clear_where(|key, _| key.starts_with(prefix), extrinsic_index) } /// Returns the current nesting depth of the transaction stack. From 47675384e1526ff469666afd66371c1578b526e9 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Fri, 20 May 2022 15:33:37 +0100 Subject: [PATCH 08/38] Fixes --- primitives/state-machine/src/lib.rs | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/primitives/state-machine/src/lib.rs b/primitives/state-machine/src/lib.rs index 97a9ae8d88cd2..89764888146b3 100644 --- a/primitives/state-machine/src/lib.rs +++ b/primitives/state-machine/src/lib.rs @@ -1675,11 +1675,25 @@ mod tests { let mut ext = Ext::new(&mut overlay, &mut cache, &backend, None); assert_eq!(ext.kill_child_storage(&child_info, Some(0)), (false, 0)); assert_eq!(ext.kill_child_storage(&child_info, Some(1)), (false, 1)); - assert_eq!(ext.kill_child_storage(&child_info, Some(2)), (false, 2)); - assert_eq!(ext.kill_child_storage(&child_info, Some(3)), (false, 3)); - assert_eq!(ext.kill_child_storage(&child_info, Some(4)), (true, 4)); - // Only 4 items to remove - assert_eq!(ext.kill_child_storage(&child_info, Some(5)), (true, 4)); + // Only 3 items remaining to remove + assert_eq!(ext.kill_child_storage(&child_info, Some(4)), (true, 3)); + } + + #[test] + fn limited_child_kill_off_by_one_works_without_limit() { + let child_info = ChildInfo::new_default(b"sub1"); + let initial: HashMap<_, BTreeMap<_, _>> = map![ + Some(child_info.clone()) => map![ + b"a".to_vec() => b"0".to_vec(), + b"b".to_vec() => b"1".to_vec(), + b"c".to_vec() => b"2".to_vec(), + b"d".to_vec() => b"3".to_vec() + ], + ]; + let backend = InMemoryBackend::::from((initial, StateVersion::default())); + let mut overlay = OverlayedChanges::default(); + let mut cache = StorageTransactionCache::default(); + let mut ext = Ext::new(&mut overlay, &mut cache, &backend, None); assert_eq!(ext.kill_child_storage(&child_info, None), (true, 4)); } From 08b898cf882f2d5014f94f580bcaae704497092a Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Fri, 20 May 2022 15:27:12 +0100 Subject: [PATCH 09/38] Introduce clear_prefix host function v3 --- frame/support/src/lib.rs | 4 +- .../src/storage/generator/double_map.rs | 9 +- frame/support/src/storage/generator/nmap.rs | 9 +- frame/support/src/storage/migration.rs | 2 +- frame/support/src/storage/mod.rs | 56 +++++++++++- .../support/src/storage/types/counted_map.rs | 13 ++- frame/support/src/storage/types/double_map.rs | 46 +++++++++- frame/support/src/storage/types/map.rs | 22 ++++- frame/support/src/storage/types/nmap.rs | 52 +++++++++-- frame/support/src/storage/unhashed.rs | 6 ++ frame/system/src/lib.rs | 4 +- primitives/externalities/src/lib.rs | 4 +- primitives/io/src/lib.rs | 91 ++++++++++++++++++- primitives/state-machine/src/basic.rs | 12 +-- primitives/state-machine/src/ext.rs | 16 ++-- primitives/state-machine/src/lib.rs | 4 +- primitives/state-machine/src/read_only.rs | 4 +- primitives/tasks/src/async_externalities.rs | 4 +- 18 files changed, 301 insertions(+), 57 deletions(-) diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index d73a01187bfcc..c9329a872e1d5 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -1097,8 +1097,8 @@ pub mod tests { DoubleMap::insert(&(key1 + 1), &key2, &4u64); DoubleMap::insert(&(key1 + 1), &(key2 + 1), &4u64); assert!(matches!( - DoubleMap::remove_prefix(&key1, None), - sp_io::KillStorageResult::AllRemoved(0), // all in overlay + DoubleMap::clear_prefix(&key1, None), + sp_io::ClearPrefixResult::NoneLeft { db: 0, total: 2 }, )); assert_eq!(DoubleMap::get(&key1, &key2), 0u64); assert_eq!(DoubleMap::get(&key1, &(key2 + 1)), 0u64); diff --git a/frame/support/src/storage/generator/double_map.rs b/frame/support/src/storage/generator/double_map.rs index 1c308bc351902..a10f3dc6bcc9a 100644 --- a/frame/support/src/storage/generator/double_map.rs +++ b/frame/support/src/storage/generator/double_map.rs @@ -206,7 +206,14 @@ where where KArg1: EncodeLike, { - unhashed::kill_prefix(Self::storage_double_map_final_key1(k1).as_ref(), limit) + Self::clear_prefix(k1, limit).into() + } + + fn clear_prefix(k1: KArg1, limit: Option) -> sp_io::ClearPrefixResult + where + KArg1: EncodeLike, + { + unhashed::clear_prefix(Self::storage_double_map_final_key1(k1).as_ref(), limit) } fn iter_prefix_values(k1: KArg1) -> storage::PrefixIterator diff --git a/frame/support/src/storage/generator/nmap.rs b/frame/support/src/storage/generator/nmap.rs index 18c912cf294ef..0778783fd564a 100755 --- a/frame/support/src/storage/generator/nmap.rs +++ b/frame/support/src/storage/generator/nmap.rs @@ -183,7 +183,14 @@ where where K: HasKeyPrefix, { - unhashed::kill_prefix(&Self::storage_n_map_partial_key(partial_key), limit) + Self::clear_prefix(partial_key, limit).into() + } + + fn clear_prefix(partial_key: KP, limit: Option) -> sp_io::ClearPrefixResult + where + K: HasKeyPrefix, + { + unhashed::clear_prefix(&Self::storage_n_map_partial_key(partial_key), limit) } fn iter_prefix_values(partial_key: KP) -> PrefixIterator diff --git a/frame/support/src/storage/migration.rs b/frame/support/src/storage/migration.rs index 713c2b0f3fe01..ca9cc7983ab1f 100644 --- a/frame/support/src/storage/migration.rs +++ b/frame/support/src/storage/migration.rs @@ -261,7 +261,7 @@ pub fn remove_storage_prefix(module: &[u8], item: &[u8], hash: &[u8]) { let storage_prefix = storage_prefix(module, item); key[0..32].copy_from_slice(&storage_prefix); key[32..].copy_from_slice(hash); - frame_support::storage::unhashed::kill_prefix(&key, None); + frame_support::storage::unhashed::clear_prefix(&key, None); } /// Take a particular item in storage by the `module`, the map's `item` name and the key `hash`. diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index 7135ead850b6d..0fb7b44939533 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -514,10 +514,27 @@ pub trait StorageDoubleMap { /// Calling this multiple times per block with a `limit` set leads always to the same keys being /// removed and the same result being returned. This happens because the keys to delete in the /// overlay are not taken into account when deleting keys in the backend. + #[deprecated = "Use `clear_prefix` instead"] fn remove_prefix(k1: KArg1, limit: Option) -> sp_io::KillStorageResult where KArg1: ?Sized + EncodeLike; + /// Remove all values under the first key `k1` in the overlay and up to `limit` in the + /// backend. + /// + /// All values in the client overlay will be deleted, if there is some `limit` then up to + /// `limit` values are deleted from the client backend, if `limit` is none then all values in + /// the client backend are deleted. + /// + /// # Note + /// + /// Calling this multiple times per block with a `limit` set leads always to the same keys being + /// removed and the same result being returned. This happens because the keys to delete in the + /// overlay are not taken into account when deleting keys in the backend. + fn clear_prefix(k1: KArg1, limit: Option) -> sp_io::ClearPrefixResult + where + KArg1: ?Sized + EncodeLike; + /// Iterate over values that share the first key. fn iter_prefix_values(k1: KArg1) -> PrefixIterator where @@ -655,10 +672,27 @@ pub trait StorageNMap { /// Calling this multiple times per block with a `limit` set leads always to the same keys being /// removed and the same result being returned. This happens because the keys to delete in the /// overlay are not taken into account when deleting keys in the backend. + #[deprecated = "Use `clear_prefix` instead"] fn remove_prefix(partial_key: KP, limit: Option) -> sp_io::KillStorageResult where K: HasKeyPrefix; + /// Remove all values starting with `partial_key` in the overlay and up to `limit` in the + /// backend. + /// + /// All values in the client overlay will be deleted, if there is some `limit` then up to + /// `limit` values are deleted from the client backend, if `limit` is none then all values in + /// the client backend are deleted. + /// + /// # Note + /// + /// Calling this multiple times per block with a `limit` set leads always to the same keys being + /// removed and the same result being returned. This happens because the keys to delete in the + /// overlay are not taken into account when deleting keys in the backend. + fn clear_prefix(partial_key: KP, limit: Option) -> sp_io::ClearPrefixResult + where + K: HasKeyPrefix; + /// Iterate over values that share the partial prefix key. fn iter_prefix_values(partial_key: KP) -> PrefixIterator where @@ -1109,7 +1143,23 @@ pub trait StoragePrefixedMap { /// Calling this multiple times per block with a `limit` set leads always to the same keys being /// removed and the same result being returned. This happens because the keys to delete in the /// overlay are not taken into account when deleting keys in the backend. + #[deprecated = "Use `clear` instead"] fn remove_all(limit: Option) -> sp_io::KillStorageResult { + Self::clear(limit).into() + } + + /// Remove all values in the overlay and up to `limit` in the backend. + /// + /// All values in the client overlay will be deleted, if there is some `limit` then up to + /// `limit` values are deleted from the client backend, if `limit` is none then all values in + /// the client backend are deleted. + /// + /// # Note + /// + /// Calling this multiple times per block with a `limit` set leads always to the same keys being + /// removed and the same result being returned. This happens because the keys to delete in the + /// overlay are not taken into account when deleting keys in the backend. + fn clear(limit: Option) -> sp_io::ClearPrefixResult { sp_io::storage::clear_prefix(&Self::final_prefix(), limit) } @@ -1425,7 +1475,7 @@ mod test { assert_eq!(MyStorage::iter_values().collect::>(), vec![1, 2, 3, 4]); // test removal - MyStorage::remove_all(None); + MyStorage::clear(None); assert!(MyStorage::iter_values().collect::>().is_empty()); // test migration @@ -1435,7 +1485,7 @@ mod test { assert!(MyStorage::iter_values().collect::>().is_empty()); MyStorage::translate_values(|v: u32| Some(v as u64)); assert_eq!(MyStorage::iter_values().collect::>(), vec![1, 2]); - MyStorage::remove_all(None); + MyStorage::clear(None); // test migration 2 unhashed::put(&[&k[..], &vec![1][..]].concat(), &1u128); @@ -1447,7 +1497,7 @@ mod test { assert_eq!(MyStorage::iter_values().collect::>(), vec![1, 2, 3]); MyStorage::translate_values(|v: u128| Some(v as u64)); assert_eq!(MyStorage::iter_values().collect::>(), vec![1, 2, 3]); - MyStorage::remove_all(None); + MyStorage::clear(None); // test that other values are not modified. assert_eq!(unhashed::get(&key_before[..]), Some(32u64)); diff --git a/frame/support/src/storage/types/counted_map.rs b/frame/support/src/storage/types/counted_map.rs index a2160ede52fe9..c616495d27e54 100644 --- a/frame/support/src/storage/types/counted_map.rs +++ b/frame/support/src/storage/types/counted_map.rs @@ -31,6 +31,7 @@ use crate::{ Never, }; use codec::{Decode, Encode, EncodeLike, FullCodec, MaxEncodedLen, Ref}; +use sp_io::ClearPrefixResult; use sp_runtime::traits::Saturating; use sp_std::prelude::*; @@ -274,12 +275,18 @@ where } /// Remove all value of the storage. + #[deprecated = "Use `clear` instead"] pub fn remove_all() { + Self::clear(None); + } + + /// Remove all value of the storage. + pub fn clear(limit: Option) -> ClearPrefixResult { // NOTE: it is not possible to remove up to some limit because // `sp_io::storage::clear_prefix` and `StorageMap::remove_all` don't give the number of // value removed from the overlay. CounterFor::::set(0u32); - ::Map::remove_all(None); + ::Map::clear(limit) } /// Iter over all value of the storage. @@ -691,7 +698,7 @@ mod test { assert_eq!(A::count(), 2); // Remove all. - A::remove_all(); + A::clear(None); assert_eq!(A::count(), 0); assert_eq!(A::initialize_counter(), 0); @@ -922,7 +929,7 @@ mod test { assert_eq!(B::count(), 2); // Remove all. - B::remove_all(); + B::clear(None); assert_eq!(B::count(), 0); assert_eq!(B::initialize_counter(), 0); diff --git a/frame/support/src/storage/types/double_map.rs b/frame/support/src/storage/types/double_map.rs index 42b903128934d..095d0a3f382c2 100644 --- a/frame/support/src/storage/types/double_map.rs +++ b/frame/support/src/storage/types/double_map.rs @@ -229,11 +229,31 @@ where /// Calling this multiple times per block with a `limit` set leads always to the same keys being /// removed and the same result being returned. This happens because the keys to delete in the /// overlay are not taken into account when deleting keys in the backend. + #[deprecated = "Use `clear_prefix` instead"] pub fn remove_prefix(k1: KArg1, limit: Option) -> sp_io::KillStorageResult where KArg1: ?Sized + EncodeLike, { - >::remove_prefix(k1, limit) + Self::clear_prefix(k1, limit).into() + } + + /// Remove all values under `k1` in the overlay and up to `limit` in the + /// backend. + /// + /// All values in the client overlay will be deleted, if there is some `limit` then up to + /// `limit` values are deleted from the client backend, if `limit` is none then all values in + /// the client backend are deleted. + /// + /// # Note + /// + /// Calling this multiple times per block with a `limit` set leads always to the same keys being + /// removed and the same result being returned. This happens because the keys to delete in the + /// overlay are not taken into account when deleting keys in the backend. + pub fn clear_prefix(k1: KArg1, limit: Option) -> sp_io::ClearPrefixResult + where + KArg1: ?Sized + EncodeLike, + { + >::clear_prefix(k1, limit) } /// Iterate over values that share the first key. @@ -359,8 +379,24 @@ where /// Calling this multiple times per block with a `limit` set leads always to the same keys being /// removed and the same result being returned. This happens because the keys to delete in the /// overlay are not taken into account when deleting keys in the backend. + #[deprecated = "Use `clear` instead"] pub fn remove_all(limit: Option) -> sp_io::KillStorageResult { - >::remove_all(limit) + Self::clear(limit).into() + } + + /// Remove all values in the overlay and up to `limit` in the backend. + /// + /// All values in the client overlay will be deleted, if there is some `limit` then up to + /// `limit` values are deleted from the client backend, if `limit` is none then all values in + /// the client backend are deleted. + /// + /// # Note + /// + /// Calling this multiple times per block with a `limit` set leads always to the same keys being + /// removed and the same result being returned. This happens because the keys to delete in the + /// overlay are not taken into account when deleting keys in the backend. + pub fn clear(limit: Option) -> sp_io::ClearPrefixResult { + >::clear(limit) } /// Iter over all value of the storage. @@ -768,7 +804,7 @@ mod test { A::insert(3, 30, 10); A::insert(4, 40, 10); - A::remove_all(None); + A::clear(None); assert_eq!(A::contains_key(3, 30), false); assert_eq!(A::contains_key(4, 40), false); @@ -829,7 +865,7 @@ mod test { ] ); - WithLen::remove_all(None); + WithLen::clear(None); assert_eq!(WithLen::decode_len(3, 30), None); WithLen::append(0, 100, 10); assert_eq!(WithLen::decode_len(0, 100), Some(1)); @@ -843,7 +879,7 @@ mod test { assert_eq!(A::iter_prefix_values(4).collect::>(), vec![13, 14]); assert_eq!(A::iter_prefix(4).collect::>(), vec![(40, 13), (41, 14)]); - A::remove_prefix(3, None); + A::clear_prefix(3, None); assert_eq!(A::iter_prefix(3).collect::>(), vec![]); assert_eq!(A::iter_prefix(4).collect::>(), vec![(40, 13), (41, 14)]); diff --git a/frame/support/src/storage/types/map.rs b/frame/support/src/storage/types/map.rs index bc32c58da8db6..679247da19115 100644 --- a/frame/support/src/storage/types/map.rs +++ b/frame/support/src/storage/types/map.rs @@ -247,8 +247,24 @@ where /// Calling this multiple times per block with a `limit` set leads always to the same keys being /// removed and the same result being returned. This happens because the keys to delete in the /// overlay are not taken into account when deleting keys in the backend. + #[deprecated = "Use `clear` instead"] pub fn remove_all(limit: Option) -> sp_io::KillStorageResult { - >::remove_all(limit) + Self::clear(limit).into() + } + + /// Remove all values in the overlay and up to `limit` in the backend. + /// + /// All values in the client overlay will be deleted, if there is some `limit` then up to + /// `limit` values are deleted from the client backend, if `limit` is none then all values in + /// the client backend are deleted. + /// + /// # Note + /// + /// Calling this multiple times per block with a `limit` set leads always to the same keys being + /// removed and the same result being returned. This happens because the keys to delete in the + /// overlay are not taken into account when deleting keys in the backend. + pub fn clear(limit: Option) -> sp_io::ClearPrefixResult { + >::clear(limit) } /// Iter over all value of the storage. @@ -563,7 +579,7 @@ mod test { A::insert(3, 10); A::insert(4, 10); - A::remove_all(None); + A::clear(None); assert_eq!(A::contains_key(3), false); assert_eq!(A::contains_key(4), false); @@ -618,7 +634,7 @@ mod test { ] ); - WithLen::remove_all(None); + WithLen::clear(None); assert_eq!(WithLen::decode_len(3), None); WithLen::append(0, 10); assert_eq!(WithLen::decode_len(0), Some(1)); diff --git a/frame/support/src/storage/types/nmap.rs b/frame/support/src/storage/types/nmap.rs index 5faeb5d8cac28..c6c322019710d 100755 --- a/frame/support/src/storage/types/nmap.rs +++ b/frame/support/src/storage/types/nmap.rs @@ -185,11 +185,31 @@ where /// Calling this multiple times per block with a `limit` set leads always to the same keys being /// removed and the same result being returned. This happens because the keys to delete in the /// overlay are not taken into account when deleting keys in the backend. + #[deprecated = "Use `clear_prefix` instead"] pub fn remove_prefix(partial_key: KP, limit: Option) -> sp_io::KillStorageResult where Key: HasKeyPrefix, { - >::remove_prefix(partial_key, limit) + Self::clear_prefix(partial_key, limit).into() + } + + /// Remove all values starting with `partial_key` in the overlay and up to `limit` in the + /// backend. + /// + /// All values in the client overlay will be deleted, if there is some `limit` then up to + /// `limit` values are deleted from the client backend, if `limit` is none then all values in + /// the client backend are deleted. + /// + /// # Note + /// + /// Calling this multiple times per block with a `limit` set leads always to the same keys being + /// removed and the same result being returned. This happens because the keys to delete in the + /// overlay are not taken into account when deleting keys in the backend. + pub fn clear_prefix(partial_key: KP, limit: Option) -> sp_io::ClearPrefixResult + where + Key: HasKeyPrefix, + { + >::clear_prefix(partial_key, limit) } /// Iterate over values that share the first key. @@ -299,8 +319,24 @@ where /// Calling this multiple times per block with a `limit` set leads always to the same keys being /// removed and the same result being returned. This happens because the keys to delete in the /// overlay are not taken into account when deleting keys in the backend. + #[deprecated = "Use `clear` instead"] pub fn remove_all(limit: Option) -> sp_io::KillStorageResult { - >::remove_all(limit) + Self::clear(limit).into() + } + + /// Remove all values in the overlay and up to `limit` in the backend. + /// + /// All values in the client overlay will be deleted, if there is some `limit` then up to + /// `limit` values are deleted from the client backend, if `limit` is none then all values in + /// the client backend are deleted. + /// + /// # Note + /// + /// Calling this multiple times per block with a `limit` set leads always to the same keys being + /// removed and the same result being returned. This happens because the keys to delete in the + /// overlay are not taken into account when deleting keys in the backend. + pub fn clear(limit: Option) -> sp_io::ClearPrefixResult { + >::clear(limit) } /// Iter over all value of the storage. @@ -684,7 +720,7 @@ mod test { A::insert((3,), 10); A::insert((4,), 10); - A::remove_all(None); + A::clear(None); assert_eq!(A::contains_key((3,)), false); assert_eq!(A::contains_key((4,)), false); @@ -739,7 +775,7 @@ mod test { ] ); - WithLen::remove_all(None); + WithLen::clear(None); assert_eq!(WithLen::decode_len((3,)), None); WithLen::append((0,), 10); assert_eq!(WithLen::decode_len((0,)), Some(1)); @@ -877,7 +913,7 @@ mod test { A::insert((3, 30), 10); A::insert((4, 40), 10); - A::remove_all(None); + A::clear(None); assert_eq!(A::contains_key((3, 30)), false); assert_eq!(A::contains_key((4, 40)), false); @@ -938,7 +974,7 @@ mod test { ] ); - WithLen::remove_all(None); + WithLen::clear(None); assert_eq!(WithLen::decode_len((3, 30)), None); WithLen::append((0, 100), 10); assert_eq!(WithLen::decode_len((0, 100)), Some(1)); @@ -1093,7 +1129,7 @@ mod test { A::insert((3, 30, 300), 10); A::insert((4, 40, 400), 10); - A::remove_all(None); + A::clear(None); assert_eq!(A::contains_key((3, 30, 300)), false); assert_eq!(A::contains_key((4, 40, 400)), false); @@ -1161,7 +1197,7 @@ mod test { ] ); - WithLen::remove_all(None); + WithLen::clear(None); assert_eq!(WithLen::decode_len((3, 30, 300)), None); WithLen::append((0, 100, 1000), 10); assert_eq!(WithLen::decode_len((0, 100, 1000)), Some(1)); diff --git a/frame/support/src/storage/unhashed.rs b/frame/support/src/storage/unhashed.rs index adabc9c14a3d5..65f5f268a1ca7 100644 --- a/frame/support/src/storage/unhashed.rs +++ b/frame/support/src/storage/unhashed.rs @@ -96,7 +96,13 @@ pub fn kill(key: &[u8]) { } /// Ensure keys with the given `prefix` have no entries in storage. +#[deprecated = "Use `clear_prefix` instead"] pub fn kill_prefix(prefix: &[u8], limit: Option) -> sp_io::KillStorageResult { + clear_prefix(prefix, limit).into() +} + +/// Ensure keys with the given `prefix` have no entries in storage. +pub fn clear_prefix(prefix: &[u8], limit: Option) -> sp_io::ClearPrefixResult { sp_io::storage::clear_prefix(prefix, limit) } diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index ba494dfbd9f8c..7d4579d3b9726 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -475,7 +475,7 @@ pub mod pallet { _subkeys: u32, ) -> DispatchResultWithPostInfo { ensure_root(origin)?; - storage::unhashed::kill_prefix(&prefix, None); + storage::unhashed::clear_prefix(&prefix, None); Ok(().into()) } @@ -1427,7 +1427,7 @@ impl Pallet { pub fn reset_events() { >::kill(); EventCount::::kill(); - >::remove_all(None); + >::clear(None); } /// Assert the given `event` exists. diff --git a/primitives/externalities/src/lib.rs b/primitives/externalities/src/lib.rs index 49b190b4ae260..d3b30cdaa6759 100644 --- a/primitives/externalities/src/lib.rs +++ b/primitives/externalities/src/lib.rs @@ -133,7 +133,7 @@ pub trait Externalities: ExtensionStore { /// Clear storage entries which keys are start with the given prefix. /// /// `limit` and result works as for `kill_child_storage`. - fn clear_prefix(&mut self, prefix: &[u8], limit: Option) -> (bool, u32); + fn clear_prefix(&mut self, prefix: &[u8], limit: Option) -> (bool, u32, u32); /// Clear child storage entries which keys are start with the given prefix. /// @@ -143,7 +143,7 @@ pub trait Externalities: ExtensionStore { child_info: &ChildInfo, prefix: &[u8], limit: Option, - ) -> (bool, u32); + ) -> (bool, u32, u32); /// Set or clear a storage entry (`key`) of current contract being called (effective /// immediately). diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index 90c43ac517b2a..faf5f906bd8bc 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -99,12 +99,41 @@ pub enum EcdsaVerifyError { /// removed from the backend from making the `storage_kill` call. #[derive(PassByCodec, Encode, Decode)] pub enum KillStorageResult { - /// All key to remove were removed, return number of key removed from backend. + /// All keys to remove were removed, return number of key removed from backend. AllRemoved(u32), /// Not all key to remove were removed, return number of key removed from backend. SomeRemaining(u32), } +impl From for KillStorageResult { + fn from(r: ClearPrefixResult) -> Self { + match r { + ClearPrefixResult::NoneLeft { db, .. } => Self::AllRemoved(db), + ClearPrefixResult::SomeLeft { db, .. } => Self::SomeRemaining(db), + } + } +} + +/// The outcome of calling `clear_prefix`. Returned value is the number of storage items +/// removed from the backend from making the `storage_kill` call. +#[derive(PassByCodec, Encode, Decode)] +pub enum ClearPrefixResult { + /// All keys to be removed were removed. + NoneLeft { + /// The number of keys removed from persistent storage. + db: u32, + /// The number of keys removed in total (including non-persistent storage). + total: u32, + }, + /// Not all keys to be removed were removed. + SomeLeft { + /// The number of keys removed from persistent storage. + db: u32, + /// The number of keys removed in total (including non-persistent storage). + total: u32, + }, +} + /// Interface for accessing the storage from within the runtime. #[runtime_interface] pub trait Storage { @@ -175,13 +204,47 @@ pub trait Storage { /// backend. #[version(2)] fn clear_prefix(&mut self, prefix: &[u8], limit: Option) -> KillStorageResult { - let (all_removed, num_removed) = Externalities::clear_prefix(*self, prefix, limit); + let (all_removed, num_removed, _) = Externalities::clear_prefix(*self, prefix, limit); match all_removed { true => KillStorageResult::AllRemoved(num_removed), false => KillStorageResult::SomeRemaining(num_removed), } } + /// Clear the storage of each key-value pair where the key starts with the given `prefix`. + /// + /// # Limit + /// + /// Deletes all keys from the overlay and up to `limit` keys from the backend if + /// it is set to `Some`. No limit is applied when `limit` is set to `None`. + /// + /// The limit can be used to partially delete a prefix storage in case it is too large + /// to delete in one go (block). + /// + /// Returns [`KillStorageResult`] to inform about the result. + /// + /// # Note + /// + /// Please note that keys that are residing in the overlay for that prefix when + /// issuing this call are all deleted without counting towards the `limit`. Only keys + /// written during the current block are part of the overlay. Deleting with a `limit` + /// mostly makes sense with an empty overlay for that prefix. + /// + /// Calling this function multiple times per block for the same `prefix` does + /// not make much sense because it is not cumulative when called inside the same block. + /// The deletion would always start from `prefix` resulting in the same keys being deleted + /// every time this function is called with the exact same arguments per block. This happens + /// because the keys in the overlay are not taken into account when deleting keys in the + /// backend. + #[version(3)] + fn clear_prefix(&mut self, prefix: &[u8], limit: Option) -> ClearPrefixResult { + let (all_removed, db, total) = Externalities::clear_prefix(*self, prefix, limit); + match all_removed { + true => ClearPrefixResult::NoneLeft { db, total }, + false => ClearPrefixResult::SomeLeft { db, total }, + } + } + /// Append the encoded `value` to the storage item at `key`. /// /// The storage item needs to implement [`EncodeAppend`](codec::EncodeAppend). @@ -376,13 +439,31 @@ pub trait DefaultChildStorage { limit: Option, ) -> KillStorageResult { let child_info = ChildInfo::new_default(storage_key); - let (all_removed, num_removed) = self.clear_child_prefix(&child_info, prefix, limit); + let (all_removed, num_removed, ..) = self.clear_child_prefix(&child_info, prefix, limit); match all_removed { true => KillStorageResult::AllRemoved(num_removed), false => KillStorageResult::SomeRemaining(num_removed), } } + /// Clear the child storage of each key-value pair where the key starts with the given `prefix`. + /// + /// See `Storage` module `clear_prefix` documentation for `limit` usage. + #[version(3)] + fn clear_prefix( + &mut self, + storage_key: &[u8], + prefix: &[u8], + limit: Option, + ) -> ClearPrefixResult { + let child_info = ChildInfo::new_default(storage_key); + let (all_removed, db, total) = self.clear_child_prefix(&child_info, prefix, limit); + match all_removed { + true => ClearPrefixResult::NoneLeft { db, total }, + false => ClearPrefixResult::SomeLeft { db, total }, + } + } + /// Default child root calculation. /// /// "Commit" all existing operations and compute the resulting child storage root. @@ -1759,7 +1840,7 @@ mod tests { t.execute_with(|| { assert!(matches!( storage::clear_prefix(b":abc", None), - KillStorageResult::AllRemoved(2) + ClearPrefixResult::NoneLeft { db: 2, total: 2 } )); assert!(storage::get(b":a").is_some()); @@ -1769,7 +1850,7 @@ mod tests { assert!(matches!( storage::clear_prefix(b":abc", None), - KillStorageResult::AllRemoved(0) + ClearPrefixResult::NoneLeft { db: 0, total: 0 } )); }); } diff --git a/primitives/state-machine/src/basic.rs b/primitives/state-machine/src/basic.rs index f1c09251faa5e..b10426d77add7 100644 --- a/primitives/state-machine/src/basic.rs +++ b/primitives/state-machine/src/basic.rs @@ -219,13 +219,13 @@ impl Externalities for BasicExternalities { (true, num_removed as u32) } - fn clear_prefix(&mut self, prefix: &[u8], _limit: Option) -> (bool, u32) { + fn clear_prefix(&mut self, prefix: &[u8], _limit: Option) -> (bool, u32, u32) { if is_child_storage_key(prefix) { warn!( target: "trie", "Refuse to clear prefix that is part of child storage key via main storage" ); - return (false, 0) + return (false, 0, 0) } let to_remove = self @@ -241,7 +241,7 @@ impl Externalities for BasicExternalities { for key in to_remove { self.inner.top.remove(&key); } - (true, num_removed as u32) + (true, num_removed as u32, num_removed as u32) } fn clear_child_prefix( @@ -249,7 +249,7 @@ impl Externalities for BasicExternalities { child_info: &ChildInfo, prefix: &[u8], _limit: Option, - ) -> (bool, u32) { + ) -> (bool, u32, u32) { if let Some(child) = self.inner.children_default.get_mut(child_info.storage_key()) { let to_remove = child .data @@ -263,9 +263,9 @@ impl Externalities for BasicExternalities { for key in to_remove { child.data.remove(&key); } - (true, num_removed as u32) + (true, num_removed as u32, num_removed as u32) } else { - (true, 0) + (true, 0, 0) } } diff --git a/primitives/state-machine/src/ext.rs b/primitives/state-machine/src/ext.rs index 6a9635bd26638..733e49c85b808 100644 --- a/primitives/state-machine/src/ext.rs +++ b/primitives/state-machine/src/ext.rs @@ -446,7 +446,7 @@ where self.limit_remove_from_backend(Some(child_info), None, limit) } - fn clear_prefix(&mut self, prefix: &[u8], limit: Option) -> (bool, u32) { + fn clear_prefix(&mut self, prefix: &[u8], limit: Option) -> (bool, u32, u32) { trace!( target: "state", method = "ClearPrefix", @@ -460,14 +460,13 @@ where target: "trie", "Refuse to directly clear prefix that is part or contains of child storage key", ); - return (false, 0) + return (false, 0, 0) } self.mark_dirty(); - // In the future, we should report this back to the caller. - let _overlay_count = self.overlay.clear_prefix(prefix); + let overlay_count = self.overlay.clear_prefix(prefix); let (all, count) = self.limit_remove_from_backend(None, Some(prefix), limit); - (all, count) + (all, count, count + overlay_count) } fn clear_child_prefix( @@ -475,7 +474,7 @@ where child_info: &ChildInfo, prefix: &[u8], limit: Option, - ) -> (bool, u32) { + ) -> (bool, u32, u32) { trace!( target: "state", method = "ChildClearPrefix", @@ -486,10 +485,9 @@ where let _guard = guard(); self.mark_dirty(); - // In the future, we should report this back to the caller. - let _overlay_count = self.overlay.clear_child_prefix(child_info, prefix); + let overlay_count = self.overlay.clear_child_prefix(child_info, prefix); let (all, count) = self.limit_remove_from_backend(Some(child_info), Some(prefix), limit); - (all, count) + (all, count, count + overlay_count) } fn storage_append(&mut self, key: Vec, value: Vec) { diff --git a/primitives/state-machine/src/lib.rs b/primitives/state-machine/src/lib.rs index 89764888146b3..1f63e4dcfd046 100644 --- a/primitives/state-machine/src/lib.rs +++ b/primitives/state-machine/src/lib.rs @@ -1596,7 +1596,7 @@ mod tests { { let mut cache = StorageTransactionCache::default(); let mut ext = Ext::new(&mut overlay, &mut cache, backend, None); - assert_eq!((false, 1), ext.clear_prefix(b"ab", Some(1))); + assert_eq!((false, 1, 3), ext.clear_prefix(b"ab", Some(1))); } overlay.commit_transaction().unwrap(); @@ -1678,7 +1678,7 @@ mod tests { // Only 3 items remaining to remove assert_eq!(ext.kill_child_storage(&child_info, Some(4)), (true, 3)); } - + #[test] fn limited_child_kill_off_by_one_works_without_limit() { let child_info = ChildInfo::new_default(b"sub1"); diff --git a/primitives/state-machine/src/read_only.rs b/primitives/state-machine/src/read_only.rs index 3c3e779c32f3a..596bbf867266f 100644 --- a/primitives/state-machine/src/read_only.rs +++ b/primitives/state-machine/src/read_only.rs @@ -128,7 +128,7 @@ impl<'a, H: Hasher, B: 'a + Backend> Externalities for ReadOnlyExternalities< unimplemented!("kill_child_storage is not supported in ReadOnlyExternalities") } - fn clear_prefix(&mut self, _prefix: &[u8], _limit: Option) -> (bool, u32) { + fn clear_prefix(&mut self, _prefix: &[u8], _limit: Option) -> (bool, u32, u32) { unimplemented!("clear_prefix is not supported in ReadOnlyExternalities") } @@ -137,7 +137,7 @@ impl<'a, H: Hasher, B: 'a + Backend> Externalities for ReadOnlyExternalities< _child_info: &ChildInfo, _prefix: &[u8], _limit: Option, - ) -> (bool, u32) { + ) -> (bool, u32, u32) { unimplemented!("clear_child_prefix is not supported in ReadOnlyExternalities") } diff --git a/primitives/tasks/src/async_externalities.rs b/primitives/tasks/src/async_externalities.rs index 70e3437538e8f..7fa624f4177d6 100644 --- a/primitives/tasks/src/async_externalities.rs +++ b/primitives/tasks/src/async_externalities.rs @@ -109,7 +109,7 @@ impl Externalities for AsyncExternalities { panic!("`kill_child_storage`: should not be used in async externalities!") } - fn clear_prefix(&mut self, _prefix: &[u8], _limit: Option) -> (bool, u32) { + fn clear_prefix(&mut self, _prefix: &[u8], _limit: Option) -> (bool, u32, u32) { panic!("`clear_prefix`: should not be used in async externalities!") } @@ -118,7 +118,7 @@ impl Externalities for AsyncExternalities { _child_info: &ChildInfo, _prefix: &[u8], _limit: Option, - ) -> (bool, u32) { + ) -> (bool, u32, u32) { panic!("`clear_child_prefix`: should not be used in async externalities!") } From aee799346e21eb66503ae9a0ffca31bc58133e09 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Fri, 20 May 2022 15:35:26 +0100 Subject: [PATCH 10/38] Formatting --- primitives/state-machine/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/primitives/state-machine/src/lib.rs b/primitives/state-machine/src/lib.rs index 1f63e4dcfd046..f3feacf324728 100644 --- a/primitives/state-machine/src/lib.rs +++ b/primitives/state-machine/src/lib.rs @@ -1678,7 +1678,7 @@ mod tests { // Only 3 items remaining to remove assert_eq!(ext.kill_child_storage(&child_info, Some(4)), (true, 3)); } - + #[test] fn limited_child_kill_off_by_one_works_without_limit() { let child_info = ChildInfo::new_default(b"sub1"); From a1982d1d5e32085a36a1ff25441390d4b4bc63bc Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Fri, 20 May 2022 15:45:18 +0100 Subject: [PATCH 11/38] Use v2 for now --- frame/support/src/storage/mod.rs | 2 +- frame/support/src/storage/unhashed.rs | 14 ++++++++++++-- primitives/io/src/lib.rs | 2 +- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index 0fb7b44939533..9b07190d25c02 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -1160,7 +1160,7 @@ pub trait StoragePrefixedMap { /// removed and the same result being returned. This happens because the keys to delete in the /// overlay are not taken into account when deleting keys in the backend. fn clear(limit: Option) -> sp_io::ClearPrefixResult { - sp_io::storage::clear_prefix(&Self::final_prefix(), limit) + unhashed::clear_prefix(&Self::final_prefix(), limit) } /// Iter over all value of the storage. diff --git a/frame/support/src/storage/unhashed.rs b/frame/support/src/storage/unhashed.rs index 65f5f268a1ca7..c3d5d0e13cd50 100644 --- a/frame/support/src/storage/unhashed.rs +++ b/frame/support/src/storage/unhashed.rs @@ -98,12 +98,22 @@ pub fn kill(key: &[u8]) { /// Ensure keys with the given `prefix` have no entries in storage. #[deprecated = "Use `clear_prefix` instead"] pub fn kill_prefix(prefix: &[u8], limit: Option) -> sp_io::KillStorageResult { - clear_prefix(prefix, limit).into() + // TODO: Once the network has upgraded to include the new host functions, this code can be + // enabled. + // clear_prefix(prefix, limit).into() + sp_io::storage::clear_prefix(prefix, limit) } /// Ensure keys with the given `prefix` have no entries in storage. pub fn clear_prefix(prefix: &[u8], limit: Option) -> sp_io::ClearPrefixResult { - sp_io::storage::clear_prefix(prefix, limit) + // TODO: Once the network has upgraded to include the new host functions, this code can be + // enabled. + // sp_io::storage::clear_prefix(prefix, limit) + use sp_io::{KillStorageResult::*, ClearPrefixResult::*}; + match kill_prefix(prefix, limit) { + AllRemoved(db) => NoneLeft { db, total: db }, + SomeRemaining(db) => SomeLeft { db, total: db }, + } } /// Get a Vec of bytes from storage. diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index faf5f906bd8bc..7c61eae1f5c3f 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -236,7 +236,7 @@ pub trait Storage { /// every time this function is called with the exact same arguments per block. This happens /// because the keys in the overlay are not taken into account when deleting keys in the /// backend. - #[version(3)] + #[version(3, register_only)] fn clear_prefix(&mut self, prefix: &[u8], limit: Option) -> ClearPrefixResult { let (all_removed, db, total) = Externalities::clear_prefix(*self, prefix, limit); match all_removed { From ed2698563dec3f6c531f364d98d9fbe13352180c Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Fri, 20 May 2022 15:49:41 +0100 Subject: [PATCH 12/38] Fixes --- frame/support/src/lib.rs | 6 +++++- frame/support/src/storage/unhashed.rs | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index c9329a872e1d5..8c708e2806c81 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -1098,7 +1098,11 @@ pub mod tests { DoubleMap::insert(&(key1 + 1), &(key2 + 1), &4u64); assert!(matches!( DoubleMap::clear_prefix(&key1, None), - sp_io::ClearPrefixResult::NoneLeft { db: 0, total: 2 }, + // Note this is the incorrect answer (for now), since we are using v2 of + // `clear_prefix`. + // When we switch to v3, then this will become: + // sp_io::ClearPrefixResult::NoneLeft { db: 0, total: 2 }, + sp_io::ClearPrefixResult::NoneLeft { db: 0, total: 0 }, )); assert_eq!(DoubleMap::get(&key1, &key2), 0u64); assert_eq!(DoubleMap::get(&key1, &(key2 + 1)), 0u64); diff --git a/frame/support/src/storage/unhashed.rs b/frame/support/src/storage/unhashed.rs index c3d5d0e13cd50..53966229322ea 100644 --- a/frame/support/src/storage/unhashed.rs +++ b/frame/support/src/storage/unhashed.rs @@ -110,6 +110,7 @@ pub fn clear_prefix(prefix: &[u8], limit: Option) -> sp_io::ClearPrefixResu // enabled. // sp_io::storage::clear_prefix(prefix, limit) use sp_io::{KillStorageResult::*, ClearPrefixResult::*}; + #[allow(deprecated)] match kill_prefix(prefix, limit) { AllRemoved(db) => NoneLeft { db, total: db }, SomeRemaining(db) => SomeLeft { db, total: db }, From 72f8fdd3cdc14baecf66b2afcb9f635b300da6e9 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Fri, 20 May 2022 15:49:57 +0100 Subject: [PATCH 13/38] Formatting --- frame/support/src/storage/unhashed.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/src/storage/unhashed.rs b/frame/support/src/storage/unhashed.rs index 53966229322ea..9c0ce8e2254fe 100644 --- a/frame/support/src/storage/unhashed.rs +++ b/frame/support/src/storage/unhashed.rs @@ -109,7 +109,7 @@ pub fn clear_prefix(prefix: &[u8], limit: Option) -> sp_io::ClearPrefixResu // TODO: Once the network has upgraded to include the new host functions, this code can be // enabled. // sp_io::storage::clear_prefix(prefix, limit) - use sp_io::{KillStorageResult::*, ClearPrefixResult::*}; + use sp_io::{ClearPrefixResult::*, KillStorageResult::*}; #[allow(deprecated)] match kill_prefix(prefix, limit) { AllRemoved(db) => NoneLeft { db, total: db }, From d97aa894b82474ff8348037236b3d6cef34f6d43 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Fri, 20 May 2022 15:53:14 +0100 Subject: [PATCH 14/38] Docs --- primitives/io/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index 7c61eae1f5c3f..07e13a757fc64 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -114,8 +114,7 @@ impl From for KillStorageResult { } } -/// The outcome of calling `clear_prefix`. Returned value is the number of storage items -/// removed from the backend from making the `storage_kill` call. +/// The outcome of calling `clear_prefix` or some function which uses it. #[derive(PassByCodec, Encode, Decode)] pub enum ClearPrefixResult { /// All keys to be removed were removed. From f4bbf8d8cc47bc58afc4eae89e69ab96c7ec5930 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Fri, 20 May 2022 15:55:40 +0100 Subject: [PATCH 15/38] Child prefix removal should also hide v3 for now --- primitives/io/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index 07e13a757fc64..f16618af0c168 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -448,7 +448,7 @@ pub trait DefaultChildStorage { /// Clear the child storage of each key-value pair where the key starts with the given `prefix`. /// /// See `Storage` module `clear_prefix` documentation for `limit` usage. - #[version(3)] + #[version(3, register_only)] fn clear_prefix( &mut self, storage_key: &[u8], From e82b1ca7c2548ea7f99beb78ec09238b605abf89 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Fri, 20 May 2022 15:57:53 +0100 Subject: [PATCH 16/38] Fixes --- primitives/io/src/lib.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index f16618af0c168..5d5cb130c0605 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -1837,9 +1837,14 @@ mod tests { }); t.execute_with(|| { + // We can switch to this once we enable v3 of the `clear_prefix`. + //assert!(matches!( + // storage::clear_prefix(b":abc", None), + // ClearPrefixResult::NoneLeft { db: 2, total: 2 } + //)); assert!(matches!( storage::clear_prefix(b":abc", None), - ClearPrefixResult::NoneLeft { db: 2, total: 2 } + KillStorageResult::AllRemoved(2), )); assert!(storage::get(b":a").is_some()); @@ -1847,9 +1852,14 @@ mod tests { assert!(storage::get(b":abcd").is_none()); assert!(storage::get(b":abc").is_none()); + // We can switch to this once we enable v3 of the `clear_prefix`. + //assert!(matches!( + // storage::clear_prefix(b":abc", None), + // ClearPrefixResult::NoneLeft { db: 0, total: 0 } + //)); assert!(matches!( storage::clear_prefix(b":abc", None), - ClearPrefixResult::NoneLeft { db: 0, total: 0 } + KillStorageResult::AllRemoved(0), )); }); } From 73294b0e44d085145d0bc16cfdf036f00ba12452 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Fri, 20 May 2022 16:11:39 +0100 Subject: [PATCH 17/38] Fixes --- frame/contracts/src/storage.rs | 8 +++---- frame/support/src/storage/child.rs | 34 +++++++++++++++++++++++++++++- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/frame/contracts/src/storage.rs b/frame/contracts/src/storage.rs index a9a78f12581d8..12e9938b859ca 100644 --- a/frame/contracts/src/storage.rs +++ b/frame/contracts/src/storage.rs @@ -27,7 +27,7 @@ use crate::{ use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::{ dispatch::{DispatchError, DispatchResult}, - storage::child::{self, ChildInfo, KillStorageResult}, + storage::child::{self, ChildInfo, ClearPrefixResult}, weights::Weight, }; use scale_info::TypeInfo; @@ -270,12 +270,12 @@ where child::kill_storage(&child_trie_info(&trie.trie_id), Some(remaining_key_budget)); let keys_removed = match outcome { // This happens when our budget wasn't large enough to remove all keys. - KillStorageResult::SomeRemaining(count) => count, - KillStorageResult::AllRemoved(count) => { + ClearPrefixResult::SomeLeft { db, .. } => db, + ClearPrefixResult::NoneLeft { db, .. } => { // We do not care to preserve order. The contract is deleted already and // no one waits for the trie to be deleted. queue.swap_remove(0); - count + db }, }; remaining_key_budget = remaining_key_budget.saturating_sub(keys_removed); diff --git a/frame/support/src/storage/child.rs b/frame/support/src/storage/child.rs index e20d4fe0cd4d3..0b61a427c9d15 100644 --- a/frame/support/src/storage/child.rs +++ b/frame/support/src/storage/child.rs @@ -21,7 +21,7 @@ // NOTE: could replace unhashed by having only one kind of storage (top trie being the child info // of null length parent storage key). -pub use crate::sp_io::KillStorageResult; +pub use crate::sp_io::{KillStorageResult, ClearPrefixResult}; use crate::sp_std::prelude::*; use codec::{Codec, Decode, Encode}; pub use sp_core::storage::{ChildInfo, ChildType, StateVersion}; @@ -136,6 +136,7 @@ pub fn exists(child_info: &ChildInfo, key: &[u8]) -> bool { /// not make much sense because it is not cumulative when called inside the same block. /// Use this function to distribute the deletion of a single child trie across multiple /// blocks. +#[deprecated = "Use `clear_storage` instead"] pub fn kill_storage(child_info: &ChildInfo, limit: Option) -> KillStorageResult { match child_info.child_type() { ChildType::ParentKeyId => @@ -143,6 +144,37 @@ pub fn kill_storage(child_info: &ChildInfo, limit: Option) -> KillStorageRe } } +/// Remove all `storage_key` key/values +/// +/// Deletes all keys from the overlay and up to `limit` keys from the backend if +/// it is set to `Some`. No limit is applied when `limit` is set to `None`. +/// +/// The limit can be used to partially delete a child trie in case it is too large +/// to delete in one go (block). +/// +/// # Note +/// +/// Please note that keys that are residing in the overlay for that child trie when +/// issuing this call are all deleted without counting towards the `limit`. Only keys +/// written during the current block are part of the overlay. Deleting with a `limit` +/// mostly makes sense with an empty overlay for that child trie. +/// +/// Calling this function multiple times per block for the same `storage_key` does +/// not make much sense because it is not cumulative when called inside the same block. +/// Use this function to distribute the deletion of a single child trie across multiple +/// blocks. +pub fn clear_storage(child_info: &ChildInfo, limit: Option) -> ClearPrefixResult { + let r = match child_info.child_type() { + ChildType::ParentKeyId => + sp_io::default_child_storage::storage_kill(child_info.storage_key(), limit), + }; + use sp_io::{ClearPrefixResult::*, KillStorageResult::*}; + match r { + AllRemoved(db) => NoneLeft { db, total: db }, + SomeRemaining(db) => SomeLeft { db, total: db }, + } +} + /// Ensure `key` has no explicit entry in storage. pub fn kill(child_info: &ChildInfo, key: &[u8]) { match child_info.child_type() { From 8a8efb11a6ac00a0de779eb55b5d93e2bedb03f9 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Fri, 20 May 2022 16:11:44 +0100 Subject: [PATCH 18/38] Formatting --- frame/support/src/storage/child.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/src/storage/child.rs b/frame/support/src/storage/child.rs index 0b61a427c9d15..6e8e1d6aa1961 100644 --- a/frame/support/src/storage/child.rs +++ b/frame/support/src/storage/child.rs @@ -21,7 +21,7 @@ // NOTE: could replace unhashed by having only one kind of storage (top trie being the child info // of null length parent storage key). -pub use crate::sp_io::{KillStorageResult, ClearPrefixResult}; +pub use crate::sp_io::{ClearPrefixResult, KillStorageResult}; use crate::sp_std::prelude::*; use codec::{Codec, Decode, Encode}; pub use sp_core::storage::{ChildInfo, ChildType, StateVersion}; From a626d54a1f3554fe2f8f5077b7fbd94a7b2bba66 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Sat, 21 May 2022 15:25:30 +0100 Subject: [PATCH 19/38] Fixes --- frame/bags-list/src/list/mod.rs | 4 ++-- frame/elections-phragmen/src/benchmarking.rs | 2 +- frame/im-online/src/lib.rs | 4 ++-- frame/society/src/lib.rs | 6 +++--- frame/staking/src/pallet/impls.rs | 18 +++++++++--------- frame/staking/src/slashing.rs | 4 ++-- frame/staking/src/testing_utils.rs | 4 ++-- frame/support/src/storage/types/counted_map.rs | 18 ++++++++++-------- frame/uniques/src/functions.rs | 4 ++-- 9 files changed, 33 insertions(+), 31 deletions(-) diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index d95c5de8132ea..26891c9bb239c 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -94,8 +94,8 @@ impl, I: 'static> List { /// this function should generally not be used in production as it could lead to a very large /// number of storage accesses. pub(crate) fn unsafe_clear() { - crate::ListBags::::remove_all(None); - crate::ListNodes::::remove_all(); + crate::ListBags::::clear(None); + crate::ListNodes::::clear(None); } /// Regenerate all of the data from the given ids. diff --git a/frame/elections-phragmen/src/benchmarking.rs b/frame/elections-phragmen/src/benchmarking.rs index 493f20924203c..ae4bf0f6a3f28 100644 --- a/frame/elections-phragmen/src/benchmarking.rs +++ b/frame/elections-phragmen/src/benchmarking.rs @@ -148,7 +148,7 @@ fn clean() { >::kill(); >::kill(); >::kill(); - >::remove_all(None); + >::clear(None); } benchmarks! { diff --git a/frame/im-online/src/lib.rs b/frame/im-online/src/lib.rs index 05b7618b286a6..10f0a923f274a 100644 --- a/frame/im-online/src/lib.rs +++ b/frame/im-online/src/lib.rs @@ -900,8 +900,8 @@ impl OneSessionHandler for Pallet { // Remove all received heartbeats and number of authored blocks from the // current session, they have already been processed and won't be needed // anymore. - ReceivedHeartbeats::::remove_prefix(&T::ValidatorSet::session_index(), None); - AuthoredBlocks::::remove_prefix(&T::ValidatorSet::session_index(), None); + ReceivedHeartbeats::::clear_prefix(&T::ValidatorSet::session_index(), None); + AuthoredBlocks::::clear_prefix(&T::ValidatorSet::session_index(), None); if offenders.is_empty() { Self::deposit_event(Event::::AllGood); diff --git a/frame/society/src/lib.rs b/frame/society/src/lib.rs index 2973ecd68092e..e8e429807258d 100644 --- a/frame/society/src/lib.rs +++ b/frame/society/src/lib.rs @@ -1067,7 +1067,7 @@ pub mod pallet { Founder::::kill(); Rules::::kill(); Candidates::::kill(); - SuspendedCandidates::::remove_all(None); + SuspendedCandidates::::clear(None); Self::deposit_event(Event::::Unfounded { founder }); Ok(()) } @@ -1511,7 +1511,7 @@ impl, I: 'static> Pallet { .collect::>(); // Clean up all votes. - >::remove_all(None); + >::clear(None); // Reward one of the voters who voted the right way. if !total_slash.is_zero() { @@ -1695,7 +1695,7 @@ impl, I: 'static> Pallet { } // Clean up all votes. - >::remove_all(None); + >::clear(None); } // Avoid challenging if there's only two members since we never challenge the Head or diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 4de1ea36cb591..efb537b88294c 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -585,9 +585,9 @@ impl Pallet { /// Clear all era information for given era. pub(crate) fn clear_era_information(era_index: EraIndex) { - >::remove_prefix(era_index, None); - >::remove_prefix(era_index, None); - >::remove_prefix(era_index, None); + >::clear_prefix(era_index, None); + >::clear_prefix(era_index, None); + >::clear_prefix(era_index, None); >::remove(era_index); >::remove(era_index); >::remove(era_index); @@ -984,10 +984,10 @@ impl ElectionDataProvider for Pallet { #[cfg(feature = "runtime-benchmarks")] fn clear() { - >::remove_all(None); - >::remove_all(None); - >::remove_all(); - >::remove_all(); + >::clear(None); + >::clear(None); + >::clear(None); + >::clear(None); T::VoterList::unsafe_clear(); } @@ -1368,8 +1368,8 @@ impl SortedListProvider for UseNominatorsAndValidatorsM fn unsafe_clear() { // NOTE: Caller must ensure this doesn't lead to too many storage accesses. This is a // condition of SortedListProvider::unsafe_clear. - Nominators::::remove_all(); - Validators::::remove_all(); + Nominators::::clear(None); + Validators::::clear(None); } } diff --git a/frame/staking/src/slashing.rs b/frame/staking/src/slashing.rs index 25954a3699b31..1fd1790149cc8 100644 --- a/frame/staking/src/slashing.rs +++ b/frame/staking/src/slashing.rs @@ -557,8 +557,8 @@ impl<'a, T: 'a + Config> Drop for InspectingSpans<'a, T> { /// Clear slashing metadata for an obsolete era. pub(crate) fn clear_era_metadata(obsolete_era: EraIndex) { - as Store>::ValidatorSlashInEra::remove_prefix(&obsolete_era, None); - as Store>::NominatorSlashInEra::remove_prefix(&obsolete_era, None); + as Store>::ValidatorSlashInEra::clear_prefix(&obsolete_era, None); + as Store>::NominatorSlashInEra::clear_prefix(&obsolete_era, None); } /// Clear slashing metadata for a dead account. diff --git a/frame/staking/src/testing_utils.rs b/frame/staking/src/testing_utils.rs index f30020597db45..3e6c109df28aa 100644 --- a/frame/staking/src/testing_utils.rs +++ b/frame/staking/src/testing_utils.rs @@ -36,10 +36,10 @@ const SEED: u32 = 0; /// This function removes all validators and nominators from storage. pub fn clear_validators_and_nominators() { - Validators::::remove_all(); + Validators::::clear(); // whenever we touch nominators counter we should update `T::VoterList` as well. - Nominators::::remove_all(); + Nominators::::clear(); // NOTE: safe to call outside block production T::VoterList::unsafe_clear(); diff --git a/frame/support/src/storage/types/counted_map.rs b/frame/support/src/storage/types/counted_map.rs index c616495d27e54..5939ff2f1b799 100644 --- a/frame/support/src/storage/types/counted_map.rs +++ b/frame/support/src/storage/types/counted_map.rs @@ -274,19 +274,21 @@ where ::Map::migrate_key::(key) } - /// Remove all value of the storage. + /// Remove all values in the map. #[deprecated = "Use `clear` instead"] pub fn remove_all() { - Self::clear(None); + Self::clear(); } - /// Remove all value of the storage. - pub fn clear(limit: Option) -> ClearPrefixResult { - // NOTE: it is not possible to remove up to some limit because - // `sp_io::storage::clear_prefix` and `StorageMap::remove_all` don't give the number of - // value removed from the overlay. + /// Remove all values in the map. + pub fn clear() { +/* match ::Map::clear(limit) { + ClearPrefixResult::NoneLeft { .. } => CounterFor::::set(0u32), + ClearPrefixResult::SomeLeft { total, .. } => + CounterFor::::mutate(|x| x.saturating_reduce(total)), + }*/ CounterFor::::set(0u32); - ::Map::clear(limit) + ::Map::clear(None); } /// Iter over all value of the storage. diff --git a/frame/uniques/src/functions.rs b/frame/uniques/src/functions.rs index cdaa31d4e1c8a..d40a36b14d96c 100644 --- a/frame/uniques/src/functions.rs +++ b/frame/uniques/src/functions.rs @@ -110,9 +110,9 @@ impl, I: 'static> Pallet { for (item, details) in Item::::drain_prefix(&collection) { Account::::remove((&details.owner, &collection, &item)); } - ItemMetadataOf::::remove_prefix(&collection, None); + ItemMetadataOf::::clear_prefix(&collection, None); CollectionMetadataOf::::remove(&collection); - Attribute::::remove_prefix((&collection,), None); + Attribute::::clear_prefix((&collection,), None); CollectionAccount::::remove(&collection_details.owner, &collection); T::Currency::unreserve(&collection_details.owner, collection_details.total_deposit); From 6867534846934a7ea7172a0e23858749b7446ef3 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Sat, 21 May 2022 17:18:52 +0100 Subject: [PATCH 20/38] apply_to_keys_whle takes start_at --- client/db/src/bench.rs | 3 ++- client/db/src/lib.rs | 3 ++- client/db/src/storage_cache.rs | 6 ++++-- primitives/state-machine/src/backend.rs | 1 + primitives/state-machine/src/ext.rs | 4 ++-- .../state-machine/src/proving_backend.rs | 3 ++- primitives/state-machine/src/trie_backend.rs | 3 ++- .../state-machine/src/trie_backend_essence.rs | 20 +++++++++++-------- 8 files changed, 27 insertions(+), 16 deletions(-) diff --git a/client/db/src/bench.rs b/client/db/src/bench.rs index 8bc4a0f4893c7..d3d43e742d026 100644 --- a/client/db/src/bench.rs +++ b/client/db/src/bench.rs @@ -393,10 +393,11 @@ impl StateBackend> for BenchmarkingState { &self, child_info: Option<&ChildInfo>, prefix: Option<&[u8]>, + start_at: Option<&[u8]>, f: F, ) { if let Some(ref state) = *self.state.borrow() { - state.apply_to_keys_while(child_info, prefix, f) + state.apply_to_keys_while(child_info, prefix, start_at, f) } } diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index a32a666c3c980..5f15b2d6fe969 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -226,9 +226,10 @@ impl StateBackend> for RefTrackingState { &self, child_info: Option<&ChildInfo>, prefix: Option<&[u8]>, + start_at: Option<&[u8]>, f: F, ) { - self.state.apply_to_keys_while(child_info, prefix, f) + self.state.apply_to_keys_while(child_info, prefix, start_at, f) } fn for_child_keys_with_prefix( diff --git a/client/db/src/storage_cache.rs b/client/db/src/storage_cache.rs index 9dada92b066ea..8326946999946 100644 --- a/client/db/src/storage_cache.rs +++ b/client/db/src/storage_cache.rs @@ -639,9 +639,10 @@ impl>, B: BlockT> StateBackend> for Cachin &self, child_info: Option<&ChildInfo>, prefix: Option<&[u8]>, + start_at: Option<&[u8]>, f: F, ) { - self.state.apply_to_keys_while(child_info, prefix, f) + self.state.apply_to_keys_while(child_info, prefix, start_at, f) } fn next_storage_key(&self, key: &[u8]) -> Result>, Self::Error> { @@ -839,9 +840,10 @@ impl>, B: BlockT> StateBackend> &self, child_info: Option<&ChildInfo>, prefix: Option<&[u8]>, + start_at: Option<&[u8]>, f: F, ) { - self.caching_state().apply_to_keys_while(child_info, prefix, f) + self.caching_state().apply_to_keys_while(child_info, prefix, start_at, f) } fn next_storage_key(&self, key: &[u8]) -> Result>, Self::Error> { diff --git a/primitives/state-machine/src/backend.rs b/primitives/state-machine/src/backend.rs index e9d4ac0fb8224..505b53c800f9e 100644 --- a/primitives/state-machine/src/backend.rs +++ b/primitives/state-machine/src/backend.rs @@ -112,6 +112,7 @@ pub trait Backend: sp_std::fmt::Debug { &self, child_info: Option<&ChildInfo>, prefix: Option<&[u8]>, + start_at: Option<&[u8]>, f: F, ); diff --git a/primitives/state-machine/src/ext.rs b/primitives/state-machine/src/ext.rs index 733e49c85b808..8488d214fab49 100644 --- a/primitives/state-machine/src/ext.rs +++ b/primitives/state-machine/src/ext.rs @@ -738,7 +738,7 @@ where if let Some(limit) = limit { let mut all_deleted = true; - self.backend.apply_to_keys_while(child_info, prefix, |key| { + self.backend.apply_to_keys_while(child_info, prefix, None, |key| { if num_deleted == limit { all_deleted = false; return false @@ -765,7 +765,7 @@ where }); (all_deleted, num_deleted) } else { - self.backend.apply_to_keys_while(child_info, prefix, |key| { + self.backend.apply_to_keys_while(child_info, prefix, None, |key| { if let Some(None) = match child_info { Some(child_info) => self.overlay.child_storage(child_info, key), None => self.overlay.storage(key), diff --git a/primitives/state-machine/src/proving_backend.rs b/primitives/state-machine/src/proving_backend.rs index b47e6c693a3e8..f5cf542908b10 100644 --- a/primitives/state-machine/src/proving_backend.rs +++ b/primitives/state-machine/src/proving_backend.rs @@ -288,9 +288,10 @@ where &self, child_info: Option<&ChildInfo>, prefix: Option<&[u8]>, + start_at: Option<&[u8]>, f: F, ) { - self.0.apply_to_keys_while(child_info, prefix, f) + self.0.apply_to_keys_while(child_info, prefix, start_at, f) } fn next_storage_key(&self, key: &[u8]) -> Result>, Self::Error> { diff --git a/primitives/state-machine/src/trie_backend.rs b/primitives/state-machine/src/trie_backend.rs index c0a620120bff2..130b4bf178202 100644 --- a/primitives/state-machine/src/trie_backend.rs +++ b/primitives/state-machine/src/trie_backend.rs @@ -123,9 +123,10 @@ where &self, child_info: Option<&ChildInfo>, prefix: Option<&[u8]>, + start_at: Option<&[u8]>, f: F, ) { - self.essence.apply_to_keys_while(child_info, prefix, f) + self.essence.apply_to_keys_while(child_info, prefix, start_at, f) } fn for_child_keys_with_prefix( diff --git a/primitives/state-machine/src/trie_backend_essence.rs b/primitives/state-machine/src/trie_backend_essence.rs index 11cac92efd2f4..391a5d84ef22c 100644 --- a/primitives/state-machine/src/trie_backend_essence.rs +++ b/primitives/state-machine/src/trie_backend_essence.rs @@ -267,6 +267,7 @@ where &self, child_info: Option<&ChildInfo>, prefix: Option<&[u8]>, + start_at: Option<&[u8]>, mut f: F, ) { let root = if let Some(child_info) = child_info.as_ref() { @@ -283,7 +284,7 @@ where self.root }; - self.trie_iter_key_inner(&root, prefix, |k| f(k), child_info) + self.trie_iter_key_inner(&root, prefix, f, child_info, start_at) } /// Execute given closure for all keys starting with prefix. @@ -311,6 +312,7 @@ where true }, Some(child_info), + None, ) } @@ -324,28 +326,30 @@ where true }, None, + None, ) } fn trie_iter_key_inner bool>( &self, root: &H::Out, - prefix: Option<&[u8]>, + maybe_prefix: Option<&[u8]>, mut f: F, child_info: Option<&ChildInfo>, + maybe_start_at: Option<&[u8]>, ) { let mut iter = move |db| -> sp_std::result::Result<(), Box>> { let trie = TrieDB::::new(db, root)?; - let iter = if let Some(prefix) = prefix.as_ref() { - TrieDBKeyIterator::new_prefixed(&trie, prefix)? - } else { - TrieDBKeyIterator::new(&trie)? - }; + let prefix = maybe_prefix.unwrap_or(&[]); + let iter = match maybe_start_at { + Some(start_at) => TrieDBKeyIterator::new_prefixed_then_seek(&trie, prefix, start_at), + None => TrieDBKeyIterator::new_prefixed(&trie, prefix), + }?; for x in iter { let key = x?; - debug_assert!(prefix + debug_assert!(maybe_prefix .as_ref() .map(|prefix| key.starts_with(prefix)) .unwrap_or(true)); From 94e885e59158fb5a7528ed61dadde6b2c0b27a16 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Sat, 21 May 2022 18:03:00 +0100 Subject: [PATCH 21/38] apply_to_keys_whle takes start_at --- primitives/io/src/lib.rs | 53 +++++++------ primitives/state-machine/src/basic.rs | 29 ++++--- primitives/state-machine/src/ext.rs | 107 +++++++++++--------------- 3 files changed, 91 insertions(+), 98 deletions(-) diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index 5d5cb130c0605..e423a6e4d418e 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -116,21 +116,15 @@ impl From for KillStorageResult { /// The outcome of calling `clear_prefix` or some function which uses it. #[derive(PassByCodec, Encode, Decode)] -pub enum ClearPrefixResult { - /// All keys to be removed were removed. - NoneLeft { - /// The number of keys removed from persistent storage. - db: u32, - /// The number of keys removed in total (including non-persistent storage). - total: u32, - }, - /// Not all keys to be removed were removed. - SomeLeft { - /// The number of keys removed from persistent storage. - db: u32, - /// The number of keys removed in total (including non-persistent storage). - total: u32, - }, +pub struct ClearPrefixResult { + /// The number of keys removed from persistent storage. This is what is relevant for weight + /// calculation. + db: u32, + /// The number of keys removed in total (including from non-persistent storage). + total: u32, + /// `None` if all keys to be removed were removed; `Some` if some keys are remaining, in which + /// case, the following call should pass this value in as the cursor. + maybe_cursor: Option>, } /// Interface for accessing the storage from within the runtime. @@ -236,12 +230,19 @@ pub trait Storage { /// because the keys in the overlay are not taken into account when deleting keys in the /// backend. #[version(3, register_only)] - fn clear_prefix(&mut self, prefix: &[u8], limit: Option) -> ClearPrefixResult { - let (all_removed, db, total) = Externalities::clear_prefix(*self, prefix, limit); - match all_removed { - true => ClearPrefixResult::NoneLeft { db, total }, - false => ClearPrefixResult::SomeLeft { db, total }, - } + fn clear_prefix( + &mut self, + maybe_prefix: &[u8], + maybe_limit: Option, + maybe_cursor: Option<&[u8]>, + ) -> ClearPrefixResult { + let (maybe_cursor, db, total) = Externalities::clear_prefix( + *self, + maybe_prefix, + maybe_limit, + maybe_cursor, + ); + ClearPrefixResult { db, total, maybe_cursor } } /// Append the encoded `value` to the storage item at `key`. @@ -453,14 +454,12 @@ pub trait DefaultChildStorage { &mut self, storage_key: &[u8], prefix: &[u8], - limit: Option, + maybe_limit: Option, + maybe_cursor: Option<&[u8]>, ) -> ClearPrefixResult { let child_info = ChildInfo::new_default(storage_key); - let (all_removed, db, total) = self.clear_child_prefix(&child_info, prefix, limit); - match all_removed { - true => ClearPrefixResult::NoneLeft { db, total }, - false => ClearPrefixResult::SomeLeft { db, total }, - } + let (maybe_cursor, db, total) = self.clear_child_prefix(&child_info, prefix, maybe_limit, maybe_cursor); + ClearPrefixResult { db, total, maybe_cursor } } /// Default child root calculation. diff --git a/primitives/state-machine/src/basic.rs b/primitives/state-machine/src/basic.rs index b10426d77add7..5d6e6dd50afe5 100644 --- a/primitives/state-machine/src/basic.rs +++ b/primitives/state-machine/src/basic.rs @@ -209,23 +209,33 @@ impl Externalities for BasicExternalities { } } - fn kill_child_storage(&mut self, child_info: &ChildInfo, _limit: Option) -> (bool, u32) { + fn kill_child_storage( + &mut self, + child_info: &ChildInfo, + _maybe_limit: Option, + _maybe_cursor: Option<&[u8]>, + ) -> (Option>, u32) { let num_removed = self .inner .children_default .remove(child_info.storage_key()) .map(|c| c.data.len()) .unwrap_or(0); - (true, num_removed as u32) + (None, num_removed as u32) } - fn clear_prefix(&mut self, prefix: &[u8], _limit: Option) -> (bool, u32, u32) { + fn clear_prefix( + &mut self, + prefix: &[u8], + _maybe_limit: Option, + _maybe_cursor: Option<&[u8]>, + ) -> (Option>, u32, u32) { if is_child_storage_key(prefix) { warn!( target: "trie", "Refuse to clear prefix that is part of child storage key via main storage" ); - return (false, 0, 0) + return (Some(prefix.to_vec()), 0, 0) } let to_remove = self @@ -241,15 +251,16 @@ impl Externalities for BasicExternalities { for key in to_remove { self.inner.top.remove(&key); } - (true, num_removed as u32, num_removed as u32) + (None, num_removed as u32, num_removed as u32) } fn clear_child_prefix( &mut self, child_info: &ChildInfo, prefix: &[u8], - _limit: Option, - ) -> (bool, u32, u32) { + _maybe_limit: Option, + _maybe_cursor: Option<&[u8]>, + ) -> (Option>, u32, u32) { if let Some(child) = self.inner.children_default.get_mut(child_info.storage_key()) { let to_remove = child .data @@ -263,9 +274,9 @@ impl Externalities for BasicExternalities { for key in to_remove { child.data.remove(&key); } - (true, num_removed as u32, num_removed as u32) + (None, num_removed as u32, num_removed as u32) } else { - (true, 0, 0) + (None, 0, 0) } } diff --git a/primitives/state-machine/src/ext.rs b/primitives/state-machine/src/ext.rs index 8488d214fab49..8e385b396f2bd 100644 --- a/primitives/state-machine/src/ext.rs +++ b/primitives/state-machine/src/ext.rs @@ -443,10 +443,15 @@ where let _guard = guard(); self.mark_dirty(); self.overlay.clear_child_storage(child_info); - self.limit_remove_from_backend(Some(child_info), None, limit) + self.limit_remove_from_backend(Some(child_info), None, limit, None) } - fn clear_prefix(&mut self, prefix: &[u8], limit: Option) -> (bool, u32, u32) { + fn clear_prefix( + &mut self, + prefix: &[u8], + maybe_limit: Option, + maybe_cursor: Option<&[u8]>, + ) -> (Option>, u32, u32) { trace!( target: "state", method = "ClearPrefix", @@ -460,21 +465,22 @@ where target: "trie", "Refuse to directly clear prefix that is part or contains of child storage key", ); - return (false, 0, 0) + return (None, 0, 0) } self.mark_dirty(); let overlay_count = self.overlay.clear_prefix(prefix); - let (all, count) = self.limit_remove_from_backend(None, Some(prefix), limit); - (all, count, count + overlay_count) + let (next_cursor, backend_count) = self.limit_remove_from_backend(None, Some(prefix), maybe_limit, maybe_cursor); + (next_cursor, backend_count, overlay_count + backend_count) } fn clear_child_prefix( &mut self, child_info: &ChildInfo, prefix: &[u8], - limit: Option, - ) -> (bool, u32, u32) { + maybe_limit: Option, + maybe_cursor: Option<&[u8]>, + ) -> (Option>, u32, u32) { trace!( target: "state", method = "ChildClearPrefix", @@ -486,8 +492,8 @@ where self.mark_dirty(); let overlay_count = self.overlay.clear_child_prefix(child_info, prefix); - let (all, count) = self.limit_remove_from_backend(Some(child_info), Some(prefix), limit); - (all, count, count + overlay_count) + let (next_cursor, backend_count) = self.limit_remove_from_backend(Some(child_info), Some(prefix), maybe_limit, maybe_cursor); + (next_cursor, backend_count, backend_count + overlay_count) } fn storage_append(&mut self, key: Vec, value: Vec) { @@ -730,59 +736,36 @@ where { fn limit_remove_from_backend( &mut self, - child_info: Option<&ChildInfo>, - prefix: Option<&[u8]>, - limit: Option, - ) -> (bool, u32) { - let mut num_deleted: u32 = 0; - - if let Some(limit) = limit { - let mut all_deleted = true; - self.backend.apply_to_keys_while(child_info, prefix, None, |key| { - if num_deleted == limit { - all_deleted = false; - return false - } - if let Some(None) = match child_info { - Some(child_info) => self.overlay.child_storage(child_info, key), - None => self.overlay.storage(key), - } { - // already deleted. - return true - } - if let Some(num) = num_deleted.checked_add(1) { - num_deleted = num; - } else { - all_deleted = false; - return false - } - if let Some(child_info) = child_info { - self.overlay.set_child_storage(child_info, key.to_vec(), None); - } else { - self.overlay.set_storage(key.to_vec(), None); - } - true - }); - (all_deleted, num_deleted) - } else { - self.backend.apply_to_keys_while(child_info, prefix, None, |key| { - if let Some(None) = match child_info { - Some(child_info) => self.overlay.child_storage(child_info, key), - None => self.overlay.storage(key), - } { - // already deleted. - return true - } - num_deleted = num_deleted.saturating_add(1); - if let Some(child_info) = child_info { - self.overlay.set_child_storage(child_info, key.to_vec(), None); - } else { - self.overlay.set_storage(key.to_vec(), None); - } - true - }); - (true, num_deleted) - } + maybe_child: Option<&ChildInfo>, + maybe_prefix: Option<&[u8]>, + maybe_limit: Option, + maybe_cursor: Option<&[u8]>, + ) -> (Option>, u32) { + let mut delete_count: u32 = 0; + let mut loop_count: u32 = 0; + let mut maybe_next_key = None; + self.backend.apply_to_keys_while(maybe_child, maybe_prefix, maybe_cursor, |key| { + if maybe_limit.map_or(false, |limit| limit == loop_count) { + maybe_next_key = Some(key.to_vec()); + return false + } + if let Some(None) = match maybe_child { + Some(child_info) => self.overlay.child_storage(child_info, key), + None => self.overlay.storage(key), + } { + // already pending deletion from the backend - no need to count it again. + return true + } + delete_count = delete_count.saturating_add(1); + loop_count = loop_count.saturating_add(1); + if let Some(child_info) = maybe_child { + self.overlay.set_child_storage(child_info, key.to_vec(), None); + } else { + self.overlay.set_storage(key.to_vec(), None); + } + true + }); + (maybe_next_key, delete_count) } } From 2566a4d4b8b35a62351737681338ff53a82bf934 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Mon, 23 May 2022 12:56:46 +0100 Subject: [PATCH 22/38] apply_to_keys_whle takes start_at --- primitives/externalities/src/lib.rs | 38 +++++++--- primitives/io/src/lib.rs | 73 ++++++++++++------- primitives/state-machine/src/basic.rs | 28 +++---- primitives/state-machine/src/ext.rs | 53 ++++++++------ primitives/state-machine/src/lib.rs | 18 +++-- .../src/overlayed_changes/mod.rs | 4 +- primitives/state-machine/src/read_only.rs | 19 ++++- .../state-machine/src/trie_backend_essence.rs | 2 +- primitives/tasks/src/async_externalities.rs | 19 ++++- 9 files changed, 164 insertions(+), 90 deletions(-) diff --git a/primitives/externalities/src/lib.rs b/primitives/externalities/src/lib.rs index d3b30cdaa6759..b60b5c59acb07 100644 --- a/primitives/externalities/src/lib.rs +++ b/primitives/externalities/src/lib.rs @@ -118,32 +118,48 @@ pub trait Externalities: ExtensionStore { /// Clear an entire child storage. /// - /// Deletes all keys from the overlay and up to `limit` keys from the backend. No - /// limit is applied if `limit` is `None`. Returned boolean is `true` if the child trie was - /// removed completely and `false` if there are remaining keys after the function - /// returns. Returned `u32` is the number of keys that was removed at the end of the - /// operation. + /// Deletes all keys from the overlay and up to `maybe_limit` keys from the backend. No + /// limit is applied if `maybe_limit` is `None`. Returns the cursor for the next call as `Some` + /// if the child trie deletion operation is incomplete. In this case, it should be passed into + /// the next call to avoid unaccounted iterations on the backend. Returns also the the number + /// of keys that were removed from the backend, the number of unique keys removed in total + /// (including from the overlay) and the number of backend iterations done. + /// + /// As long as `maybe_cursor` is passed from the result of the previous call, then the number of + /// iterations done will only ever be one more than the number of keys removed. + /// than `maybe_limit` (if `Some`). /// /// # Note /// /// An implementation is free to delete more keys than the specified limit as long as /// it is able to do that in constant time. - fn kill_child_storage(&mut self, child_info: &ChildInfo, limit: Option) -> (bool, u32); + fn kill_child_storage( + &mut self, + child_info: &ChildInfo, + maybe_limit: Option, + maybe_cursor: Option<&[u8]>, + ) -> (Option>, u32, u32, u32); /// Clear storage entries which keys are start with the given prefix. /// - /// `limit` and result works as for `kill_child_storage`. - fn clear_prefix(&mut self, prefix: &[u8], limit: Option) -> (bool, u32, u32); + /// `maybe_limit`, `maybe_cursor` and result works as for `kill_child_storage`. + fn clear_prefix( + &mut self, + prefix: &[u8], + maybe_limit: Option, + maybe_cursor: Option<&[u8]>, + ) -> (Option>, u32, u32, u32); /// Clear child storage entries which keys are start with the given prefix. /// - /// `limit` and result works as for `kill_child_storage`. + /// `maybe_limit`, `maybe_cursor` and result works as for `kill_child_storage`. fn clear_child_prefix( &mut self, child_info: &ChildInfo, prefix: &[u8], - limit: Option, - ) -> (bool, u32, u32); + maybe_limit: Option, + maybe_cursor: Option<&[u8]>, + ) -> (Option>, u32, u32, u32); /// Set or clear a storage entry (`key`) of current contract being called (effective /// immediately). diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index e423a6e4d418e..14c4267fa96fd 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -108,8 +108,8 @@ pub enum KillStorageResult { impl From for KillStorageResult { fn from(r: ClearPrefixResult) -> Self { match r { - ClearPrefixResult::NoneLeft { db, .. } => Self::AllRemoved(db), - ClearPrefixResult::SomeLeft { db, .. } => Self::SomeRemaining(db), + ClearPrefixResult { maybe_cursor: None, db, .. } => Self::AllRemoved(db), + ClearPrefixResult { maybe_cursor: Some(..), db, .. } => Self::SomeRemaining(db), } } } @@ -122,6 +122,8 @@ pub struct ClearPrefixResult { db: u32, /// The number of keys removed in total (including from non-persistent storage). total: u32, + /// The number of backend iterations done for this operation. + loops: u32, /// `None` if all keys to be removed were removed; `Some` if some keys are remaining, in which /// case, the following call should pass this value in as the cursor. maybe_cursor: Option>, @@ -167,7 +169,7 @@ pub trait Storage { /// Clear the storage of each key-value pair where the key starts with the given `prefix`. fn clear_prefix(&mut self, prefix: &[u8]) { - let _ = Externalities::clear_prefix(*self, prefix, None); + let _ = Externalities::clear_prefix(*self, prefix, None, None); } /// Clear the storage of each key-value pair where the key starts with the given `prefix`. @@ -197,10 +199,10 @@ pub trait Storage { /// backend. #[version(2)] fn clear_prefix(&mut self, prefix: &[u8], limit: Option) -> KillStorageResult { - let (all_removed, num_removed, _) = Externalities::clear_prefix(*self, prefix, limit); - match all_removed { - true => KillStorageResult::AllRemoved(num_removed), - false => KillStorageResult::SomeRemaining(num_removed), + let (maybe_cursor, num_removed, ..) = Externalities::clear_prefix(*self, prefix, limit, None); + match maybe_cursor { + None => KillStorageResult::AllRemoved(num_removed), + Some(_) => KillStorageResult::SomeRemaining(num_removed), } } @@ -234,15 +236,15 @@ pub trait Storage { &mut self, maybe_prefix: &[u8], maybe_limit: Option, - maybe_cursor: Option<&[u8]>, + maybe_cursor: Option>, //< TODO Make work or just Option>? ) -> ClearPrefixResult { - let (maybe_cursor, db, total) = Externalities::clear_prefix( + let (maybe_cursor, db, total, loops) = Externalities::clear_prefix( *self, maybe_prefix, maybe_limit, - maybe_cursor, + maybe_cursor.as_ref().map(|x| &x[..]), ); - ClearPrefixResult { db, total, maybe_cursor } + ClearPrefixResult { db, total, loops, maybe_cursor } } /// Append the encoded `value` to the storage item at `key`. @@ -386,7 +388,7 @@ pub trait DefaultChildStorage { /// is removed. fn storage_kill(&mut self, storage_key: &[u8]) { let child_info = ChildInfo::new_default(storage_key); - self.kill_child_storage(&child_info, None); + self.kill_child_storage(&child_info, None, None); } /// Clear a child storage key. @@ -395,8 +397,8 @@ pub trait DefaultChildStorage { #[version(2)] fn storage_kill(&mut self, storage_key: &[u8], limit: Option) -> bool { let child_info = ChildInfo::new_default(storage_key); - let (all_removed, _num_removed) = self.kill_child_storage(&child_info, limit); - all_removed + let (maybe_cursor, ..) = self.kill_child_storage(&child_info, limit, None); + maybe_cursor.is_none() } /// Clear a child storage key. @@ -405,13 +407,27 @@ pub trait DefaultChildStorage { #[version(3)] fn storage_kill(&mut self, storage_key: &[u8], limit: Option) -> KillStorageResult { let child_info = ChildInfo::new_default(storage_key); - let (all_removed, num_removed) = self.kill_child_storage(&child_info, limit); - match all_removed { - true => KillStorageResult::AllRemoved(num_removed), - false => KillStorageResult::SomeRemaining(num_removed), + let (maybe_cursor, num_removed, ..) = self.kill_child_storage(&child_info, limit, None); + match maybe_cursor { + None => KillStorageResult::AllRemoved(num_removed), + Some(..) => KillStorageResult::SomeRemaining(num_removed), } } + /// Clear a child storage key. + /// + /// See `Storage` module `clear_prefix` documentation for `limit` usage. + #[version(4, register_only)] + fn storage_kill(&mut self, storage_key: &[u8], maybe_limit: Option, maybe_cursor: Option>) -> ClearPrefixResult { + let child_info = ChildInfo::new_default(storage_key); + let (maybe_cursor, db, total, loops) = self.kill_child_storage( + &child_info, + maybe_limit, + maybe_cursor.as_ref().map(|x| &x[..]) + ); + ClearPrefixResult { maybe_cursor, db, total, loops } + } + /// Check a child storage key. /// /// Check whether the given `key` exists in default child defined at `storage_key`. @@ -425,7 +441,7 @@ pub trait DefaultChildStorage { /// Clear the child storage of each key-value pair where the key starts with the given `prefix`. fn clear_prefix(&mut self, storage_key: &[u8], prefix: &[u8]) { let child_info = ChildInfo::new_default(storage_key); - let _ = self.clear_child_prefix(&child_info, prefix, None); + let _ = self.clear_child_prefix(&child_info, prefix, None, None); } /// Clear the child storage of each key-value pair where the key starts with the given `prefix`. @@ -439,10 +455,10 @@ pub trait DefaultChildStorage { limit: Option, ) -> KillStorageResult { let child_info = ChildInfo::new_default(storage_key); - let (all_removed, num_removed, ..) = self.clear_child_prefix(&child_info, prefix, limit); - match all_removed { - true => KillStorageResult::AllRemoved(num_removed), - false => KillStorageResult::SomeRemaining(num_removed), + let (maybe_cursor, num_removed, ..) = self.clear_child_prefix(&child_info, prefix, limit, None); + match maybe_cursor { + None => KillStorageResult::AllRemoved(num_removed), + Some(..) => KillStorageResult::SomeRemaining(num_removed), } } @@ -455,11 +471,16 @@ pub trait DefaultChildStorage { storage_key: &[u8], prefix: &[u8], maybe_limit: Option, - maybe_cursor: Option<&[u8]>, + maybe_cursor: Option>, ) -> ClearPrefixResult { let child_info = ChildInfo::new_default(storage_key); - let (maybe_cursor, db, total) = self.clear_child_prefix(&child_info, prefix, maybe_limit, maybe_cursor); - ClearPrefixResult { db, total, maybe_cursor } + let (maybe_cursor, db, total, loops) = self.clear_child_prefix( + &child_info, + prefix, + maybe_limit, + maybe_cursor.as_ref().map(|x| &x[..]), + ); + ClearPrefixResult { db, total, maybe_cursor, loops } } /// Default child root calculation. diff --git a/primitives/state-machine/src/basic.rs b/primitives/state-machine/src/basic.rs index 5d6e6dd50afe5..437ed9a25314c 100644 --- a/primitives/state-machine/src/basic.rs +++ b/primitives/state-machine/src/basic.rs @@ -214,14 +214,14 @@ impl Externalities for BasicExternalities { child_info: &ChildInfo, _maybe_limit: Option, _maybe_cursor: Option<&[u8]>, - ) -> (Option>, u32) { + ) -> (Option>, u32, u32, u32) { let num_removed = self .inner .children_default .remove(child_info.storage_key()) .map(|c| c.data.len()) - .unwrap_or(0); - (None, num_removed as u32) + .unwrap_or(0) as u32; + (None, num_removed, num_removed, num_removed) } fn clear_prefix( @@ -229,13 +229,13 @@ impl Externalities for BasicExternalities { prefix: &[u8], _maybe_limit: Option, _maybe_cursor: Option<&[u8]>, - ) -> (Option>, u32, u32) { + ) -> (Option>, u32, u32, u32) { if is_child_storage_key(prefix) { warn!( target: "trie", "Refuse to clear prefix that is part of child storage key via main storage" ); - return (Some(prefix.to_vec()), 0, 0) + return (Some(prefix.to_vec()), 0, 0, 0) } let to_remove = self @@ -247,11 +247,11 @@ impl Externalities for BasicExternalities { .cloned() .collect::>(); - let num_removed = to_remove.len(); + let num_removed = to_remove.len() as u32; for key in to_remove { self.inner.top.remove(&key); } - (None, num_removed as u32, num_removed as u32) + (None, num_removed, num_removed, num_removed) } fn clear_child_prefix( @@ -260,7 +260,7 @@ impl Externalities for BasicExternalities { prefix: &[u8], _maybe_limit: Option, _maybe_cursor: Option<&[u8]>, - ) -> (Option>, u32, u32) { + ) -> (Option>, u32, u32, u32) { if let Some(child) = self.inner.children_default.get_mut(child_info.storage_key()) { let to_remove = child .data @@ -270,13 +270,13 @@ impl Externalities for BasicExternalities { .cloned() .collect::>(); - let num_removed = to_remove.len(); + let num_removed = to_remove.len() as u32; for key in to_remove { child.data.remove(&key); } - (None, num_removed as u32, num_removed as u32) + (None, num_removed, num_removed, num_removed) } else { - (None, 0, 0) + (None, 0, 0, 0) } } @@ -445,7 +445,7 @@ mod tests { ext.clear_child_storage(child_info, b"dog"); assert_eq!(ext.child_storage(child_info, b"dog"), None); - ext.kill_child_storage(child_info, None); + ext.kill_child_storage(child_info, None, None); assert_eq!(ext.child_storage(child_info, b"doe"), None); } @@ -467,8 +467,8 @@ mod tests { ], }); - let res = ext.kill_child_storage(child_info, None); - assert_eq!(res, (true, 3)); + let res = ext.kill_child_storage(child_info, None, None); + assert_eq!(res, (None, 3, 3, 3)); } #[test] diff --git a/primitives/state-machine/src/ext.rs b/primitives/state-machine/src/ext.rs index 8e385b396f2bd..b780a0b06ab9f 100644 --- a/primitives/state-machine/src/ext.rs +++ b/primitives/state-machine/src/ext.rs @@ -433,7 +433,12 @@ where self.overlay.set_child_storage(child_info, key, value); } - fn kill_child_storage(&mut self, child_info: &ChildInfo, limit: Option) -> (bool, u32) { + fn kill_child_storage( + &mut self, + child_info: &ChildInfo, + maybe_limit: Option, + maybe_cursor: Option<&[u8]>, + ) -> (Option>, u32, u32, u32) { trace!( target: "state", method = "ChildKill", @@ -442,8 +447,10 @@ where ); let _guard = guard(); self.mark_dirty(); - self.overlay.clear_child_storage(child_info); - self.limit_remove_from_backend(Some(child_info), None, limit, None) + let overlay_count = self.overlay.clear_child_storage(child_info); + let (next_cursor, backend_count, iterations) = + self.limit_remove_from_backend(Some(child_info), None, maybe_limit, maybe_cursor); + (next_cursor, backend_count, overlay_count + backend_count, iterations) } fn clear_prefix( @@ -451,7 +458,7 @@ where prefix: &[u8], maybe_limit: Option, maybe_cursor: Option<&[u8]>, - ) -> (Option>, u32, u32) { + ) -> (Option>, u32, u32, u32) { trace!( target: "state", method = "ClearPrefix", @@ -465,13 +472,14 @@ where target: "trie", "Refuse to directly clear prefix that is part or contains of child storage key", ); - return (None, 0, 0) + return (None, 0, 0, 0) } self.mark_dirty(); let overlay_count = self.overlay.clear_prefix(prefix); - let (next_cursor, backend_count) = self.limit_remove_from_backend(None, Some(prefix), maybe_limit, maybe_cursor); - (next_cursor, backend_count, overlay_count + backend_count) + let (next_cursor, backend_count, iterations) = + self.limit_remove_from_backend(None, Some(prefix), maybe_limit, maybe_cursor); + (next_cursor, backend_count, overlay_count + backend_count, iterations) } fn clear_child_prefix( @@ -480,7 +488,7 @@ where prefix: &[u8], maybe_limit: Option, maybe_cursor: Option<&[u8]>, - ) -> (Option>, u32, u32) { + ) -> (Option>, u32, u32, u32) { trace!( target: "state", method = "ChildClearPrefix", @@ -492,8 +500,9 @@ where self.mark_dirty(); let overlay_count = self.overlay.clear_child_prefix(child_info, prefix); - let (next_cursor, backend_count) = self.limit_remove_from_backend(Some(child_info), Some(prefix), maybe_limit, maybe_cursor); - (next_cursor, backend_count, backend_count + overlay_count) + let (next_cursor, backend_count, iterations) = + self.limit_remove_from_backend(Some(child_info), Some(prefix), maybe_limit, maybe_cursor); + (next_cursor, backend_count, backend_count + overlay_count, iterations) } fn storage_append(&mut self, key: Vec, value: Vec) { @@ -740,7 +749,7 @@ where maybe_prefix: Option<&[u8]>, maybe_limit: Option, maybe_cursor: Option<&[u8]>, - ) -> (Option>, u32) { + ) -> (Option>, u32, u32) { let mut delete_count: u32 = 0; let mut loop_count: u32 = 0; let mut maybe_next_key = None; @@ -749,23 +758,23 @@ where maybe_next_key = Some(key.to_vec()); return false } - if let Some(None) = match maybe_child { + let overlay = match maybe_child { Some(child_info) => self.overlay.child_storage(child_info, key), None => self.overlay.storage(key), - } { - // already pending deletion from the backend - no need to count it again. - return true + }; + if !matches!(overlay, Some(None)) { + // already pending deletion from the backend - no need to delete it again. + if let Some(child_info) = maybe_child { + self.overlay.set_child_storage(child_info, key.to_vec(), None); + } else { + self.overlay.set_storage(key.to_vec(), None); + } + delete_count = delete_count.saturating_add(1); } - delete_count = delete_count.saturating_add(1); loop_count = loop_count.saturating_add(1); - if let Some(child_info) = maybe_child { - self.overlay.set_child_storage(child_info, key.to_vec(), None); - } else { - self.overlay.set_storage(key.to_vec(), None); - } true }); - (maybe_next_key, delete_count) + (maybe_next_key, delete_count, loop_count) } } diff --git a/primitives/state-machine/src/lib.rs b/primitives/state-machine/src/lib.rs index f3feacf324728..1d390dcb1073f 100644 --- a/primitives/state-machine/src/lib.rs +++ b/primitives/state-machine/src/lib.rs @@ -1357,7 +1357,8 @@ mod tests { NativeOrEncoded, NeverNativeValue, }; use sp_runtime::traits::BlakeTwo256; - use std::{ + use core::assert_matches; +use std::{ collections::{BTreeMap, HashMap}, panic::UnwindSafe, result, @@ -1673,10 +1674,15 @@ mod tests { let mut overlay = OverlayedChanges::default(); let mut cache = StorageTransactionCache::default(); let mut ext = Ext::new(&mut overlay, &mut cache, &backend, None); - assert_eq!(ext.kill_child_storage(&child_info, Some(0)), (false, 0)); - assert_eq!(ext.kill_child_storage(&child_info, Some(1)), (false, 1)); + let r = ext.kill_child_storage(&child_info, Some(0), None); + assert_matches!(r, (0, 0, 1, Some(_))); + let r = ext.kill_child_storage(&child_info, Some(1), r.3); + assert_matches!(r, (1, 1, 2, Some(_))); + let r = ext.kill_child_storage(&child_info, Some(4), r.3); // Only 3 items remaining to remove - assert_eq!(ext.kill_child_storage(&child_info, Some(4)), (true, 3)); + assert_matches!(r, (3, 3, 4, None)); + let r = ext.kill_child_storage(&child_info, Some(1), None); + assert_matches!(r, (0, 0, 1, None)); } #[test] @@ -1694,7 +1700,7 @@ mod tests { let mut overlay = OverlayedChanges::default(); let mut cache = StorageTransactionCache::default(); let mut ext = Ext::new(&mut overlay, &mut cache, &backend, None); - assert_eq!(ext.kill_child_storage(&child_info, None), (true, 4)); + assert_eq!(ext.kill_child_storage(&child_info, None, None), (None, 4, 4, 5)); } #[test] @@ -1709,7 +1715,7 @@ mod tests { ext.set_child_storage(child_info, b"abc".to_vec(), b"def".to_vec()); assert_eq!(ext.child_storage(child_info, b"abc"), Some(b"def".to_vec())); - ext.kill_child_storage(child_info, None); + ext.kill_child_storage(child_info, None, None); assert_eq!(ext.child_storage(child_info, b"abc"), None); } diff --git a/primitives/state-machine/src/overlayed_changes/mod.rs b/primitives/state-machine/src/overlayed_changes/mod.rs index fdaa5ce787861..746599a6768e6 100644 --- a/primitives/state-machine/src/overlayed_changes/mod.rs +++ b/primitives/state-machine/src/overlayed_changes/mod.rs @@ -299,7 +299,7 @@ impl OverlayedChanges { /// Clear child storage of given storage key. /// /// Can be rolled back or committed when called inside a transaction. - pub(crate) fn clear_child_storage(&mut self, child_info: &ChildInfo) { + pub(crate) fn clear_child_storage(&mut self, child_info: &ChildInfo) -> u32 { let extrinsic_index = self.extrinsic_index(); let storage_key = child_info.storage_key().to_vec(); let top = &self.top; @@ -309,7 +309,7 @@ impl OverlayedChanges { .or_insert_with(|| (top.spawn_child(), child_info.clone())); let updatable = info.try_update(child_info); debug_assert!(updatable); - changeset.clear_where(|_, _| true, extrinsic_index); + changeset.clear_where(|_, _| true, extrinsic_index) } /// Removes all key-value pairs which keys share the given prefix. diff --git a/primitives/state-machine/src/read_only.rs b/primitives/state-machine/src/read_only.rs index 596bbf867266f..1a84c1c7af8f5 100644 --- a/primitives/state-machine/src/read_only.rs +++ b/primitives/state-machine/src/read_only.rs @@ -124,11 +124,21 @@ impl<'a, H: Hasher, B: 'a + Backend> Externalities for ReadOnlyExternalities< unimplemented!("place_child_storage not supported in ReadOnlyExternalities") } - fn kill_child_storage(&mut self, _child_info: &ChildInfo, _limit: Option) -> (bool, u32) { + fn kill_child_storage( + &mut self, + _child_info: &ChildInfo, + _maybe_limit: Option, + _maybe_cursor: Option<&[u8]>, + ) -> (Option>, u32, u32, u32) { unimplemented!("kill_child_storage is not supported in ReadOnlyExternalities") } - fn clear_prefix(&mut self, _prefix: &[u8], _limit: Option) -> (bool, u32, u32) { + fn clear_prefix( + &mut self, + _prefix: &[u8], + _maybe_limit: Option, + _maybe_cursor: Option<&[u8]>, + ) -> (Option>, u32, u32, u32) { unimplemented!("clear_prefix is not supported in ReadOnlyExternalities") } @@ -136,8 +146,9 @@ impl<'a, H: Hasher, B: 'a + Backend> Externalities for ReadOnlyExternalities< &mut self, _child_info: &ChildInfo, _prefix: &[u8], - _limit: Option, - ) -> (bool, u32, u32) { + _maybe_limit: Option, + _maybe_cursor: Option<&[u8]>, + ) -> (Option>, u32, u32, u32) { unimplemented!("clear_child_prefix is not supported in ReadOnlyExternalities") } diff --git a/primitives/state-machine/src/trie_backend_essence.rs b/primitives/state-machine/src/trie_backend_essence.rs index 391a5d84ef22c..49f18e3148cf1 100644 --- a/primitives/state-machine/src/trie_backend_essence.rs +++ b/primitives/state-machine/src/trie_backend_essence.rs @@ -268,7 +268,7 @@ where child_info: Option<&ChildInfo>, prefix: Option<&[u8]>, start_at: Option<&[u8]>, - mut f: F, + f: F, ) { let root = if let Some(child_info) = child_info.as_ref() { match self.child_root(child_info) { diff --git a/primitives/tasks/src/async_externalities.rs b/primitives/tasks/src/async_externalities.rs index 7fa624f4177d6..852be406eecae 100644 --- a/primitives/tasks/src/async_externalities.rs +++ b/primitives/tasks/src/async_externalities.rs @@ -105,11 +105,21 @@ impl Externalities for AsyncExternalities { panic!("`place_child_storage`: should not be used in async externalities!") } - fn kill_child_storage(&mut self, _child_info: &ChildInfo, _limit: Option) -> (bool, u32) { + fn kill_child_storage( + &mut self, + _child_info: &ChildInfo, + _maybe_limit: Option, + _maybe_cursor: Option<&[u8]>, + ) -> (Option>, u32, u32, u32) { panic!("`kill_child_storage`: should not be used in async externalities!") } - fn clear_prefix(&mut self, _prefix: &[u8], _limit: Option) -> (bool, u32, u32) { + fn clear_prefix( + &mut self, + _prefix: &[u8], + _maybe_limit: Option, + _maybe_cursor: Option<&[u8]>, + ) -> (Option>, u32, u32, u32) { panic!("`clear_prefix`: should not be used in async externalities!") } @@ -117,8 +127,9 @@ impl Externalities for AsyncExternalities { &mut self, _child_info: &ChildInfo, _prefix: &[u8], - _limit: Option, - ) -> (bool, u32, u32) { + _maybe_limit: Option, + _maybe_cursor: Option<&[u8]>, + ) -> (Option>, u32, u32, u32) { panic!("`clear_child_prefix`: should not be used in async externalities!") } From a71f9a08c46a78507f598ee4f5aa3b6cb35ad4e3 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Mon, 23 May 2022 14:41:17 +0100 Subject: [PATCH 23/38] Cursor API; force limits --- Cargo.lock | 1 + frame/support/src/lib.rs | 4 +- frame/support/src/storage/child.rs | 55 +++++---- .../src/storage/generator/double_map.rs | 16 ++- frame/support/src/storage/generator/nmap.rs | 6 +- frame/support/src/storage/migration.rs | 32 +++++- frame/support/src/storage/mod.rs | 104 ++++++++++++------ .../support/src/storage/types/counted_map.rs | 47 ++++++-- frame/support/src/storage/types/double_map.rs | 77 ++++++++----- frame/support/src/storage/types/map.rs | 39 ++++--- frame/support/src/storage/types/nmap.rs | 83 +++++++++----- frame/support/src/storage/unhashed.rs | 49 +++++++-- frame/system/src/lib.rs | 4 +- primitives/io/src/lib.rs | 49 +++++---- primitives/state-machine/Cargo.toml | 1 + primitives/state-machine/src/ext.rs | 10 +- primitives/state-machine/src/lib.rs | 24 ++-- primitives/state-machine/src/testing.rs | 2 +- 18 files changed, 410 insertions(+), 193 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 03bc44a0a848a..7dbf83a29ee9a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10071,6 +10071,7 @@ dependencies = [ name = "sp-state-machine" version = "0.12.0" dependencies = [ + "assert_matches", "hash-db", "hex-literal", "log", diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 8c708e2806c81..5c64f90ef23fb 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -1097,12 +1097,12 @@ pub mod tests { DoubleMap::insert(&(key1 + 1), &key2, &4u64); DoubleMap::insert(&(key1 + 1), &(key2 + 1), &4u64); assert!(matches!( - DoubleMap::clear_prefix(&key1, None), + DoubleMap::clear_prefix(&key1, u32::max_value(), None), // Note this is the incorrect answer (for now), since we are using v2 of // `clear_prefix`. // When we switch to v3, then this will become: // sp_io::ClearPrefixResult::NoneLeft { db: 0, total: 2 }, - sp_io::ClearPrefixResult::NoneLeft { db: 0, total: 0 }, + sp_io::ClearPrefixResult { maybe_cursor: None, db: 0, total: 0, loops: 0 }, )); assert_eq!(DoubleMap::get(&key1, &key2), 0u64); assert_eq!(DoubleMap::get(&key1, &(key2 + 1)), 0u64); diff --git a/frame/support/src/storage/child.rs b/frame/support/src/storage/child.rs index 6e8e1d6aa1961..f3255ce9ef9be 100644 --- a/frame/support/src/storage/child.rs +++ b/frame/support/src/storage/child.rs @@ -144,34 +144,51 @@ pub fn kill_storage(child_info: &ChildInfo, limit: Option) -> KillStorageRe } } -/// Remove all `storage_key` key/values +/// Partially clear the child storage of each key-value pair. /// -/// Deletes all keys from the overlay and up to `limit` keys from the backend if -/// it is set to `Some`. No limit is applied when `limit` is set to `None`. +/// # Limit /// -/// The limit can be used to partially delete a child trie in case it is too large -/// to delete in one go (block). +/// A *limit* should always be provided through `maybe_limit`. This is one fewer than the +/// maximum number of backend iterations which may be done by this operation and as such +/// represents the maximum number of backend deletions which may happen. A *limit* of zero +/// implies that no keys will be deleted, through there may be a single iteration done. /// -/// # Note +/// The limit can be used to partially delete storage items in case it is too large or costly +/// to delete all in a single operation. /// -/// Please note that keys that are residing in the overlay for that child trie when -/// issuing this call are all deleted without counting towards the `limit`. Only keys -/// written during the current block are part of the overlay. Deleting with a `limit` -/// mostly makes sense with an empty overlay for that child trie. +/// # Cursor /// -/// Calling this function multiple times per block for the same `storage_key` does -/// not make much sense because it is not cumulative when called inside the same block. -/// Use this function to distribute the deletion of a single child trie across multiple -/// blocks. -pub fn clear_storage(child_info: &ChildInfo, limit: Option) -> ClearPrefixResult { +/// A *cursor* may be passed in to this operation with `maybe_cursor`. `None` should only be +/// passed once (in the initial call) for any attempt to clear storage. Subsequent calls +/// operating on the same prefix should always pass `Some`, and this should be equal to the +/// previous call result's `maybe_prefix` field. +/// +/// Returns [`ClearPrefixResult`] to inform about the result. Once the resultant `maybe_prefix` +/// field is `None`, then no further items remain to be deleted. +/// +/// NOTE: After the initial call for any given child storage, it is important that no keys further +/// keys are inserted. If so, then they may or may not be deleted by subsequent calls. +/// +/// # Note +/// +/// Please note that keys which are residing in the overlay for the child are deleted without +/// counting towards the `limit`. +pub fn clear_storage( + child_info: &ChildInfo, + maybe_limit: Option, + _maybe_cursor: Option<&[u8]>, +) -> ClearPrefixResult { + // TODO: Once the network has upgraded to include the new host functions, this code can be + // enabled. + // sp_io::default_child_storage::storage_kill(prefix, maybe_limit, maybe_cursor) let r = match child_info.child_type() { ChildType::ParentKeyId => - sp_io::default_child_storage::storage_kill(child_info.storage_key(), limit), + sp_io::default_child_storage::storage_kill(child_info.storage_key(), maybe_limit), }; - use sp_io::{ClearPrefixResult::*, KillStorageResult::*}; + use sp_io::KillStorageResult::*; match r { - AllRemoved(db) => NoneLeft { db, total: db }, - SomeRemaining(db) => SomeLeft { db, total: db }, + AllRemoved(db) => ClearPrefixResult { maybe_cursor: None, db, total: db, loops: db }, + SomeRemaining(db) => ClearPrefixResult { maybe_cursor: None, db, total: db, loops: db }, } } diff --git a/frame/support/src/storage/generator/double_map.rs b/frame/support/src/storage/generator/double_map.rs index a10f3dc6bcc9a..c1bca318ae68b 100644 --- a/frame/support/src/storage/generator/double_map.rs +++ b/frame/support/src/storage/generator/double_map.rs @@ -202,18 +202,26 @@ where unhashed::kill(&Self::storage_double_map_final_key(k1, k2)) } - fn remove_prefix(k1: KArg1, limit: Option) -> sp_io::KillStorageResult + fn remove_prefix(k1: KArg1, maybe_limit: Option) -> sp_io::KillStorageResult where KArg1: EncodeLike, { - Self::clear_prefix(k1, limit).into() + unhashed::clear_prefix( + Self::storage_double_map_final_key1(k1).as_ref(), + maybe_limit, + None, + ).into() } - fn clear_prefix(k1: KArg1, limit: Option) -> sp_io::ClearPrefixResult + fn clear_prefix(k1: KArg1, limit: u32, maybe_cursor: Option<&[u8]>) -> sp_io::ClearPrefixResult where KArg1: EncodeLike, { - unhashed::clear_prefix(Self::storage_double_map_final_key1(k1).as_ref(), limit) + unhashed::clear_prefix( + Self::storage_double_map_final_key1(k1).as_ref(), + Some(limit), + maybe_cursor, + ).into() } fn iter_prefix_values(k1: KArg1) -> storage::PrefixIterator diff --git a/frame/support/src/storage/generator/nmap.rs b/frame/support/src/storage/generator/nmap.rs index 0778783fd564a..a4bc9ecb20652 100755 --- a/frame/support/src/storage/generator/nmap.rs +++ b/frame/support/src/storage/generator/nmap.rs @@ -183,14 +183,14 @@ where where K: HasKeyPrefix, { - Self::clear_prefix(partial_key, limit).into() + unhashed::clear_prefix(&Self::storage_n_map_partial_key(partial_key), limit, None).into() } - fn clear_prefix(partial_key: KP, limit: Option) -> sp_io::ClearPrefixResult + fn clear_prefix(partial_key: KP, limit: u32, maybe_cursor: Option<&[u8]>) -> sp_io::ClearPrefixResult where K: HasKeyPrefix, { - unhashed::clear_prefix(&Self::storage_n_map_partial_key(partial_key), limit) + unhashed::clear_prefix(&Self::storage_n_map_partial_key(partial_key), Some(limit), maybe_cursor) } fn iter_prefix_values(partial_key: KP) -> PrefixIterator diff --git a/frame/support/src/storage/migration.rs b/frame/support/src/storage/migration.rs index ca9cc7983ab1f..93dcb3184e060 100644 --- a/frame/support/src/storage/migration.rs +++ b/frame/support/src/storage/migration.rs @@ -256,12 +256,42 @@ pub fn put_storage_value(module: &[u8], item: &[u8], hash: &[u8], val /// Remove all items under a storage prefix by the `module`, the map's `item` name and the key /// `hash`. +#[deprecated = "Use `clear_storage_prefix` instead"] pub fn remove_storage_prefix(module: &[u8], item: &[u8], hash: &[u8]) { let mut key = vec![0u8; 32 + hash.len()]; let storage_prefix = storage_prefix(module, item); key[0..32].copy_from_slice(&storage_prefix); key[32..].copy_from_slice(hash); - frame_support::storage::unhashed::clear_prefix(&key, None); + let _ = frame_support::storage::unhashed::clear_prefix(&key, None, None); +} + +/// Attmept to remove all values under a storage prefix by the `module`, the map's `item` name and +/// the key `hash`. +/// +/// All values in the client overlay will be deleted, if `maybe_limit` is `Some` then up to +/// that number of values are deleted from the client backend, otherwise all values in the +/// client backend are deleted. +/// +/// ## Cursors +/// +/// The `maybe_cursor` parameter should be `None` for the first call to initial removal. +/// If the resultant `maybe_cursor` is `Some`, then another call is required to complete the +/// removal operation. This value must be passed in as the subsequent call's `maybe_cursor` +/// parameter. If the resultant `maybe_cursor` is `None`, then the operation is complete and no +/// items remain in storage provided that no items were added between the first calls and the +/// final call. +pub fn clear_storage_prefix( + module: &[u8], + item: &[u8], + hash: &[u8], + maybe_limit: Option, + maybe_cursor: Option<&[u8]> +) -> sp_io::ClearPrefixResult { + let mut key = vec![0u8; 32 + hash.len()]; + let storage_prefix = storage_prefix(module, item); + key[0..32].copy_from_slice(&storage_prefix); + key[32..].copy_from_slice(hash); + frame_support::storage::unhashed::clear_prefix(&key, maybe_limit, maybe_cursor) } /// Take a particular item in storage by the `module`, the map's `item` name and the key `hash`. diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index 9b07190d25c02..b8ad47a474058 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -519,19 +519,26 @@ pub trait StorageDoubleMap { where KArg1: ?Sized + EncodeLike; - /// Remove all values under the first key `k1` in the overlay and up to `limit` in the + /// Remove all values under the first key `k1` in the overlay and up to `maybe_limit` in the /// backend. /// - /// All values in the client overlay will be deleted, if there is some `limit` then up to - /// `limit` values are deleted from the client backend, if `limit` is none then all values in - /// the client backend are deleted. - /// - /// # Note - /// - /// Calling this multiple times per block with a `limit` set leads always to the same keys being - /// removed and the same result being returned. This happens because the keys to delete in the - /// overlay are not taken into account when deleting keys in the backend. - fn clear_prefix(k1: KArg1, limit: Option) -> sp_io::ClearPrefixResult + /// All values in the client overlay will be deleted, if `maybe_limit` is `Some` then up to + /// that number of values are deleted from the client backend, otherwise all values in the + /// client backend are deleted. + /// + /// ## Cursors + /// + /// The `maybe_cursor` parameter should be `None` for the first call to initial removal. + /// If the resultant `maybe_cursor` is `Some`, then another call is required to complete the + /// removal operation. This value must be passed in as the subsequent call's `maybe_cursor` + /// parameter. If the resultant `maybe_cursor` is `None`, then the operation is complete and no + /// items remain in storage provided that no items were added between the first calls and the + /// final call. + fn clear_prefix( + k1: KArg1, + limit: u32, + maybe_cursor: Option<&[u8]>, + ) -> sp_io::ClearPrefixResult where KArg1: ?Sized + EncodeLike; @@ -677,19 +684,34 @@ pub trait StorageNMap { where K: HasKeyPrefix; - /// Remove all values starting with `partial_key` in the overlay and up to `limit` in the - /// backend. + /// Attempt to remove items from the map matching a `partial_key` prefix. /// - /// All values in the client overlay will be deleted, if there is some `limit` then up to - /// `limit` values are deleted from the client backend, if `limit` is none then all values in - /// the client backend are deleted. + /// Returns [`ClearPrefixResult`] to inform about the result. Once the resultant `maybe_cursor` + /// field is `None`, then no further items remain to be deleted. /// - /// # Note + /// NOTE: After the initial call for any given map, it is important that no further items + /// are inserted into the map which match the `partial key`. If so, then the map may not be + /// empty when the resultant `maybe_cursor` is `None`. /// - /// Calling this multiple times per block with a `limit` set leads always to the same keys being - /// removed and the same result being returned. This happens because the keys to delete in the - /// overlay are not taken into account when deleting keys in the backend. - fn clear_prefix(partial_key: KP, limit: Option) -> sp_io::ClearPrefixResult + /// # Limit + /// + /// A `limit` must be provided in order to cap the maximum + /// amount of deletions done in a single call. This is one fewer than the + /// maximum number of backend iterations which may be done by this operation and as such + /// represents the maximum number of backend deletions which may happen. A *limit* of zero + /// implies that no keys will be deleted, through there may be a single iteration done. + /// + /// # Cursor + /// + /// A *cursor* may be passed in to this operation with `maybe_cursor`. `None` should only be + /// passed once (in the initial call) for any given storage map and `partial_key`. Subsequent + /// calls operating on the same map/`partial_key` should always pass `Some`, and this should be + /// equal to the previous call result's `maybe_cursor` field. + fn clear_prefix( + partial_key: KP, + limit: u32, + maybe_cursor: Option<&[u8]>, + ) -> sp_io::ClearPrefixResult where K: HasKeyPrefix; @@ -1145,22 +1167,34 @@ pub trait StoragePrefixedMap { /// overlay are not taken into account when deleting keys in the backend. #[deprecated = "Use `clear` instead"] fn remove_all(limit: Option) -> sp_io::KillStorageResult { - Self::clear(limit).into() + unhashed::clear_prefix(&Self::final_prefix(), limit, None).into() } - /// Remove all values in the overlay and up to `limit` in the backend. + /// Attempt to remove all items from the map. /// - /// All values in the client overlay will be deleted, if there is some `limit` then up to - /// `limit` values are deleted from the client backend, if `limit` is none then all values in - /// the client backend are deleted. + /// Returns [`ClearPrefixResult`] to inform about the result. Once the resultant `maybe_cursor` + /// field is `None`, then no further items remain to be deleted. /// - /// # Note + /// NOTE: After the initial call for any given map, it is important that no further items + /// are inserted into the map. If so, then the map may not be empty when the resultant + /// `maybe_cursor` is `None`. /// - /// Calling this multiple times per block with a `limit` set leads always to the same keys being - /// removed and the same result being returned. This happens because the keys to delete in the - /// overlay are not taken into account when deleting keys in the backend. - fn clear(limit: Option) -> sp_io::ClearPrefixResult { - unhashed::clear_prefix(&Self::final_prefix(), limit) + /// # Limit + /// + /// A `limit` must always be provided through in order to cap the maximum + /// amount of deletions done in a single call. This is one fewer than the + /// maximum number of backend iterations which may be done by this operation and as such + /// represents the maximum number of backend deletions which may happen. A *limit* of zero + /// implies that no keys will be deleted, through there may be a single iteration done. + /// + /// # Cursor + /// + /// A *cursor* may be passed in to this operation with `maybe_cursor`. `None` should only be + /// passed once (in the initial call) for any given storage map. Subsequent calls + /// operating on the same map should always pass `Some`, and this should be equal to the + /// previous call result's `maybe_cursor` field. + fn clear(limit: u32, maybe_cursor: Option<&[u8]>) -> sp_io::ClearPrefixResult { + unhashed::clear_prefix(&Self::final_prefix(), Some(limit), maybe_cursor) } /// Iter over all value of the storage. @@ -1475,7 +1509,7 @@ mod test { assert_eq!(MyStorage::iter_values().collect::>(), vec![1, 2, 3, 4]); // test removal - MyStorage::clear(None); + let _ = MyStorage::clear(u32::max_value(), None); assert!(MyStorage::iter_values().collect::>().is_empty()); // test migration @@ -1485,7 +1519,7 @@ mod test { assert!(MyStorage::iter_values().collect::>().is_empty()); MyStorage::translate_values(|v: u32| Some(v as u64)); assert_eq!(MyStorage::iter_values().collect::>(), vec![1, 2]); - MyStorage::clear(None); + let _ = MyStorage::clear(u32::max_value(), None); // test migration 2 unhashed::put(&[&k[..], &vec![1][..]].concat(), &1u128); @@ -1497,7 +1531,7 @@ mod test { assert_eq!(MyStorage::iter_values().collect::>(), vec![1, 2, 3]); MyStorage::translate_values(|v: u128| Some(v as u64)); assert_eq!(MyStorage::iter_values().collect::>(), vec![1, 2, 3]); - MyStorage::clear(None); + let _ = MyStorage::clear(u32::max_value(), None); // test that other values are not modified. assert_eq!(unhashed::get(&key_before[..]), Some(32u64)); diff --git a/frame/support/src/storage/types/counted_map.rs b/frame/support/src/storage/types/counted_map.rs index 5939ff2f1b799..fdfa92c366014 100644 --- a/frame/support/src/storage/types/counted_map.rs +++ b/frame/support/src/storage/types/counted_map.rs @@ -277,18 +277,41 @@ where /// Remove all values in the map. #[deprecated = "Use `clear` instead"] pub fn remove_all() { - Self::clear(); + #[allow(deprecated)] + ::Map::remove_all(None); + CounterFor::::kill(); } - /// Remove all values in the map. - pub fn clear() { -/* match ::Map::clear(limit) { - ClearPrefixResult::NoneLeft { .. } => CounterFor::::set(0u32), - ClearPrefixResult::SomeLeft { total, .. } => - CounterFor::::mutate(|x| x.saturating_reduce(total)), - }*/ - CounterFor::::set(0u32); - ::Map::clear(None); + /// Attempt to remove all items from the map. + /// + /// Returns [`ClearPrefixResult`] to inform about the result. Once the resultant `maybe_cursor` + /// field is `None`, then no further items remain to be deleted. + /// + /// NOTE: After the initial call for any given map, it is important that no further items + /// are inserted into the map. If so, then the map may not be empty when the resultant + /// `maybe_cursor` is `None`. + /// + /// # Limit + /// + /// A *limit* should always be provided through `maybe_limit` in order to cap the maximum + /// amount of deletions done in a single call. This is one fewer than the + /// maximum number of backend iterations which may be done by this operation and as such + /// represents the maximum number of backend deletions which may happen. A *limit* of zero + /// implies that no keys will be deleted, through there may be a single iteration done. + /// + /// # Cursor + /// + /// A *cursor* may be passed in to this operation with `maybe_cursor`. `None` should only be + /// passed once (in the initial call) for any given storage map. Subsequent calls + /// operating on the same map should always pass `Some`, and this should be equal to the + /// previous call result's `maybe_cursor` field. + pub fn clear(limit: u32, maybe_cursor: Option<&[u8]>) -> ClearPrefixResult { + let result = ::Map::clear(limit, maybe_cursor); + match result.maybe_cursor { + None => CounterFor::::kill(), + Some(_) => CounterFor::::mutate(|x| x.saturating_reduce(result.total)), + } + result } /// Iter over all value of the storage. @@ -700,7 +723,7 @@ mod test { assert_eq!(A::count(), 2); // Remove all. - A::clear(None); + let _ = A::clear(u32::max_value(), None); assert_eq!(A::count(), 0); assert_eq!(A::initialize_counter(), 0); @@ -931,7 +954,7 @@ mod test { assert_eq!(B::count(), 2); // Remove all. - B::clear(None); + let _ = B::clear(u32::max_value(), None); assert_eq!(B::count(), 0); assert_eq!(B::initialize_counter(), 0); diff --git a/frame/support/src/storage/types/double_map.rs b/frame/support/src/storage/types/double_map.rs index 095d0a3f382c2..07695c1854830 100644 --- a/frame/support/src/storage/types/double_map.rs +++ b/frame/support/src/storage/types/double_map.rs @@ -234,26 +234,38 @@ where where KArg1: ?Sized + EncodeLike, { - Self::clear_prefix(k1, limit).into() + #[allow(deprecated)] + >::remove_prefix(k1, limit) } - /// Remove all values under `k1` in the overlay and up to `limit` in the - /// backend. + /// Attempt to remove items from the map matching a `first_key` prefix. /// - /// All values in the client overlay will be deleted, if there is some `limit` then up to - /// `limit` values are deleted from the client backend, if `limit` is none then all values in - /// the client backend are deleted. + /// Returns [`ClearPrefixResult`] to inform about the result. Once the resultant `maybe_cursor` + /// field is `None`, then no further items remain to be deleted. /// - /// # Note + /// NOTE: After the initial call for any given map, it is important that no further items + /// are inserted into the map which match the `first_key`. If so, then the map may not be + /// empty when the resultant `maybe_cursor` is `None`. /// - /// Calling this multiple times per block with a `limit` set leads always to the same keys being - /// removed and the same result being returned. This happens because the keys to delete in the - /// overlay are not taken into account when deleting keys in the backend. - pub fn clear_prefix(k1: KArg1, limit: Option) -> sp_io::ClearPrefixResult + /// # Limit + /// + /// A *limit* should always be provided through `maybe_limit` in order to cap the maximum + /// amount of deletions done in a single call. This is one fewer than the + /// maximum number of backend iterations which may be done by this operation and as such + /// represents the maximum number of backend deletions which may happen. A *limit* of zero + /// implies that no keys will be deleted, through there may be a single iteration done. + /// + /// # Cursor + /// + /// A *cursor* may be passed in to this operation with `maybe_cursor`. `None` should only be + /// passed once (in the initial call) for any given storage map and `first_key`. Subsequent + /// calls operating on the same map/`first_key` should always pass `Some`, and this should be + /// equal to the previous call result's `maybe_cursor` field. + pub fn clear_prefix(first_key: KArg1, limit: u32, maybe_cursor: Option<&[u8]>) -> sp_io::ClearPrefixResult where KArg1: ?Sized + EncodeLike, { - >::clear_prefix(k1, limit) + >::clear_prefix(first_key, limit, maybe_cursor) } /// Iterate over values that share the first key. @@ -381,22 +393,35 @@ where /// overlay are not taken into account when deleting keys in the backend. #[deprecated = "Use `clear` instead"] pub fn remove_all(limit: Option) -> sp_io::KillStorageResult { - Self::clear(limit).into() + #[allow(deprecated)] + >::remove_all(limit) } - /// Remove all values in the overlay and up to `limit` in the backend. + /// Attempt to remove all items from the map. /// - /// All values in the client overlay will be deleted, if there is some `limit` then up to - /// `limit` values are deleted from the client backend, if `limit` is none then all values in - /// the client backend are deleted. + /// Returns [`ClearPrefixResult`] to inform about the result. Once the resultant `maybe_cursor` + /// field is `None`, then no further items remain to be deleted. /// - /// # Note + /// NOTE: After the initial call for any given map, it is important that no further items + /// are inserted into the map. If so, then the map may not be empty when the resultant + /// `maybe_cursor` is `None`. /// - /// Calling this multiple times per block with a `limit` set leads always to the same keys being - /// removed and the same result being returned. This happens because the keys to delete in the - /// overlay are not taken into account when deleting keys in the backend. - pub fn clear(limit: Option) -> sp_io::ClearPrefixResult { - >::clear(limit) + /// # Limit + /// + /// A `limit` must always be provided through in order to cap the maximum + /// amount of deletions done in a single call. This is one fewer than the + /// maximum number of backend iterations which may be done by this operation and as such + /// represents the maximum number of backend deletions which may happen. A *limit* of zero + /// implies that no keys will be deleted, through there may be a single iteration done. + /// + /// # Cursor + /// + /// A *cursor* may be passed in to this operation with `maybe_cursor`. `None` should only be + /// passed once (in the initial call) for any given storage map. Subsequent calls + /// operating on the same map should always pass `Some`, and this should be equal to the + /// previous call result's `maybe_cursor` field. + pub fn clear(limit: u32, maybe_cursor: Option<&[u8]>) -> sp_io::ClearPrefixResult { + >::clear(limit, maybe_cursor) } /// Iter over all value of the storage. @@ -804,7 +829,7 @@ mod test { A::insert(3, 30, 10); A::insert(4, 40, 10); - A::clear(None); + let _ = A::clear(u32::max_value(), None); assert_eq!(A::contains_key(3, 30), false); assert_eq!(A::contains_key(4, 40), false); @@ -865,7 +890,7 @@ mod test { ] ); - WithLen::clear(None); + let _ = WithLen::clear(u32::max_value(), None); assert_eq!(WithLen::decode_len(3, 30), None); WithLen::append(0, 100, 10); assert_eq!(WithLen::decode_len(0, 100), Some(1)); @@ -879,7 +904,7 @@ mod test { assert_eq!(A::iter_prefix_values(4).collect::>(), vec![13, 14]); assert_eq!(A::iter_prefix(4).collect::>(), vec![(40, 13), (41, 14)]); - A::clear_prefix(3, None); + let _ = A::clear_prefix(3, u32::max_value(), None); assert_eq!(A::iter_prefix(3).collect::>(), vec![]); assert_eq!(A::iter_prefix(4).collect::>(), vec![(40, 13), (41, 14)]); diff --git a/frame/support/src/storage/types/map.rs b/frame/support/src/storage/types/map.rs index 679247da19115..1c046318a2197 100644 --- a/frame/support/src/storage/types/map.rs +++ b/frame/support/src/storage/types/map.rs @@ -249,22 +249,35 @@ where /// overlay are not taken into account when deleting keys in the backend. #[deprecated = "Use `clear` instead"] pub fn remove_all(limit: Option) -> sp_io::KillStorageResult { - Self::clear(limit).into() + #[allow(deprecated)] + >::remove_all(limit) } - /// Remove all values in the overlay and up to `limit` in the backend. + /// Attempt to remove all items from the map. /// - /// All values in the client overlay will be deleted, if there is some `limit` then up to - /// `limit` values are deleted from the client backend, if `limit` is none then all values in - /// the client backend are deleted. + /// Returns [`ClearPrefixResult`] to inform about the result. Once the resultant `maybe_cursor` + /// field is `None`, then no further items remain to be deleted. /// - /// # Note + /// NOTE: After the initial call for any given map, it is important that no further items + /// are inserted into the map. If so, then the map may not be empty when the resultant + /// `maybe_cursor` is `None`. /// - /// Calling this multiple times per block with a `limit` set leads always to the same keys being - /// removed and the same result being returned. This happens because the keys to delete in the - /// overlay are not taken into account when deleting keys in the backend. - pub fn clear(limit: Option) -> sp_io::ClearPrefixResult { - >::clear(limit) + /// # Limit + /// + /// A *limit* should always be provided through `maybe_limit` in order to cap the maximum + /// amount of deletions done in a single call. This is one fewer than the + /// maximum number of backend iterations which may be done by this operation and as such + /// represents the maximum number of backend deletions which may happen. A *limit* of zero + /// implies that no keys will be deleted, through there may be a single iteration done. + /// + /// # Cursor + /// + /// A *cursor* may be passed in to this operation with `maybe_cursor`. `None` should only be + /// passed once (in the initial call) for any given storage map. Subsequent calls + /// operating on the same map should always pass `Some`, and this should be equal to the + /// previous call result's `maybe_cursor` field. + pub fn clear(limit: u32, maybe_cursor: Option<&[u8]>) -> sp_io::ClearPrefixResult { + >::clear(limit, maybe_cursor) } /// Iter over all value of the storage. @@ -579,7 +592,7 @@ mod test { A::insert(3, 10); A::insert(4, 10); - A::clear(None); + let _ = A::clear(u32::max_value(), None); assert_eq!(A::contains_key(3), false); assert_eq!(A::contains_key(4), false); @@ -634,7 +647,7 @@ mod test { ] ); - WithLen::clear(None); + let _ = WithLen::clear(u32::max_value(), None); assert_eq!(WithLen::decode_len(3), None); WithLen::append(0, 10); assert_eq!(WithLen::decode_len(0), Some(1)); diff --git a/frame/support/src/storage/types/nmap.rs b/frame/support/src/storage/types/nmap.rs index c6c322019710d..a517e3281b672 100755 --- a/frame/support/src/storage/types/nmap.rs +++ b/frame/support/src/storage/types/nmap.rs @@ -190,26 +190,38 @@ where where Key: HasKeyPrefix, { - Self::clear_prefix(partial_key, limit).into() + #[allow(deprecated)] + >::remove_prefix(partial_key, limit) } - /// Remove all values starting with `partial_key` in the overlay and up to `limit` in the - /// backend. + /// Attempt to remove items from the map matching a `partial_key` prefix. /// - /// All values in the client overlay will be deleted, if there is some `limit` then up to - /// `limit` values are deleted from the client backend, if `limit` is none then all values in - /// the client backend are deleted. + /// Returns [`ClearPrefixResult`] to inform about the result. Once the resultant `maybe_cursor` + /// field is `None`, then no further items remain to be deleted. /// - /// # Note + /// NOTE: After the initial call for any given map, it is important that no further items + /// are inserted into the map which match the `partial key`. If so, then the map may not be + /// empty when the resultant `maybe_cursor` is `None`. /// - /// Calling this multiple times per block with a `limit` set leads always to the same keys being - /// removed and the same result being returned. This happens because the keys to delete in the - /// overlay are not taken into account when deleting keys in the backend. - pub fn clear_prefix(partial_key: KP, limit: Option) -> sp_io::ClearPrefixResult + /// # Limit + /// + /// A `limit` must be provided in order to cap the maximum + /// amount of deletions done in a single call. This is one fewer than the + /// maximum number of backend iterations which may be done by this operation and as such + /// represents the maximum number of backend deletions which may happen. A *limit* of zero + /// implies that no keys will be deleted, through there may be a single iteration done. + /// + /// # Cursor + /// + /// A *cursor* may be passed in to this operation with `maybe_cursor`. `None` should only be + /// passed once (in the initial call) for any given storage map and `partial_key`. Subsequent + /// calls operating on the same map/`partial_key` should always pass `Some`, and this should be + /// equal to the previous call result's `maybe_cursor` field. + pub fn clear_prefix(partial_key: KP, limit: u32, maybe_cursor: Option<&[u8]>) -> sp_io::ClearPrefixResult where Key: HasKeyPrefix, { - >::clear_prefix(partial_key, limit) + >::clear_prefix(partial_key, limit, maybe_cursor) } /// Iterate over values that share the first key. @@ -321,22 +333,35 @@ where /// overlay are not taken into account when deleting keys in the backend. #[deprecated = "Use `clear` instead"] pub fn remove_all(limit: Option) -> sp_io::KillStorageResult { - Self::clear(limit).into() + #[allow(deprecated)] + >::remove_all(limit).into() } - /// Remove all values in the overlay and up to `limit` in the backend. + /// Attempt to remove all items from the map. /// - /// All values in the client overlay will be deleted, if there is some `limit` then up to - /// `limit` values are deleted from the client backend, if `limit` is none then all values in - /// the client backend are deleted. + /// Returns [`ClearPrefixResult`] to inform about the result. Once the resultant `maybe_cursor` + /// field is `None`, then no further items remain to be deleted. /// - /// # Note + /// NOTE: After the initial call for any given map, it is important that no further items + /// are inserted into the map. If so, then the map may not be empty when the resultant + /// `maybe_cursor` is `None`. /// - /// Calling this multiple times per block with a `limit` set leads always to the same keys being - /// removed and the same result being returned. This happens because the keys to delete in the - /// overlay are not taken into account when deleting keys in the backend. - pub fn clear(limit: Option) -> sp_io::ClearPrefixResult { - >::clear(limit) + /// # Limit + /// + /// A `limit` must always be provided through in order to cap the maximum + /// amount of deletions done in a single call. This is one fewer than the + /// maximum number of backend iterations which may be done by this operation and as such + /// represents the maximum number of backend deletions which may happen. A *limit* of zero + /// implies that no keys will be deleted, through there may be a single iteration done. + /// + /// # Cursor + /// + /// A *cursor* may be passed in to this operation with `maybe_cursor`. `None` should only be + /// passed once (in the initial call) for any given storage map. Subsequent calls + /// operating on the same map should always pass `Some`, and this should be equal to the + /// previous call result's `maybe_cursor` field. + pub fn clear(limit: u32, maybe_cursor: Option<&[u8]>) -> sp_io::ClearPrefixResult { + >::clear(limit, maybe_cursor) } /// Iter over all value of the storage. @@ -720,7 +745,7 @@ mod test { A::insert((3,), 10); A::insert((4,), 10); - A::clear(None); + let _ = A::clear(u32::max_value(), None); assert_eq!(A::contains_key((3,)), false); assert_eq!(A::contains_key((4,)), false); @@ -775,7 +800,7 @@ mod test { ] ); - WithLen::clear(None); + let _ = WithLen::clear(u32::max_value(), None); assert_eq!(WithLen::decode_len((3,)), None); WithLen::append((0,), 10); assert_eq!(WithLen::decode_len((0,)), Some(1)); @@ -913,7 +938,7 @@ mod test { A::insert((3, 30), 10); A::insert((4, 40), 10); - A::clear(None); + let _ = A::clear(u32::max_value(), None); assert_eq!(A::contains_key((3, 30)), false); assert_eq!(A::contains_key((4, 40)), false); @@ -974,7 +999,7 @@ mod test { ] ); - WithLen::clear(None); + let _ = WithLen::clear(u32::max_value(), None); assert_eq!(WithLen::decode_len((3, 30)), None); WithLen::append((0, 100), 10); assert_eq!(WithLen::decode_len((0, 100)), Some(1)); @@ -1129,7 +1154,7 @@ mod test { A::insert((3, 30, 300), 10); A::insert((4, 40, 400), 10); - A::clear(None); + let _ = A::clear(u32::max_value(), None); assert_eq!(A::contains_key((3, 30, 300)), false); assert_eq!(A::contains_key((4, 40, 400)), false); @@ -1197,7 +1222,7 @@ mod test { ] ); - WithLen::clear(None); + let _ = WithLen::clear(u32::max_value(), None); assert_eq!(WithLen::decode_len((3, 30, 300)), None); WithLen::append((0, 100, 1000), 10); assert_eq!(WithLen::decode_len((0, 100, 1000)), Some(1)); diff --git a/frame/support/src/storage/unhashed.rs b/frame/support/src/storage/unhashed.rs index 9c0ce8e2254fe..68b32cc6e3e39 100644 --- a/frame/support/src/storage/unhashed.rs +++ b/frame/support/src/storage/unhashed.rs @@ -104,17 +104,50 @@ pub fn kill_prefix(prefix: &[u8], limit: Option) -> sp_io::KillStorageResul sp_io::storage::clear_prefix(prefix, limit) } -/// Ensure keys with the given `prefix` have no entries in storage. -pub fn clear_prefix(prefix: &[u8], limit: Option) -> sp_io::ClearPrefixResult { +/// Partially clear the storage of all keys under a common `prefix`. +/// +/// # Limit +/// +/// A *limit* should always be provided through `maybe_limit`. This is one fewer than the +/// maximum number of backend iterations which may be done by this operation and as such +/// represents the maximum number of backend deletions which may happen. A *limit* of zero +/// implies that no keys will be deleted, through there may be a single iteration done. +/// +/// The limit can be used to partially delete storage items in case it is too large or costly +/// to delete all in a single operation. +/// +/// # Cursor +/// +/// A *cursor* may be passed in to this operation with `maybe_cursor`. `None` should only be +/// passed once (in the initial call) for any attempt to clear storage. Subsequent calls +/// operating on the same prefix should always pass `Some`, and this should be equal to the +/// previous call result's `maybe_prefix` field. +/// +/// Returns [`ClearPrefixResult`] to inform about the result. Once the resultant `maybe_prefix` +/// field is `None`, then no further items remain to be deleted. +/// +/// NOTE: After the initial call for any given child storage, it is important that no keys further +/// keys are inserted. If so, then they may or may not be deleted by subsequent calls. +/// +/// # Note +/// +/// Please note that keys which are residing in the overlay for the child are deleted without +/// counting towards the `limit`. +pub fn clear_prefix( + prefix: &[u8], + maybe_limit: Option, + _maybe_cursor: Option<&[u8]>, +) -> sp_io::ClearPrefixResult { // TODO: Once the network has upgraded to include the new host functions, this code can be // enabled. - // sp_io::storage::clear_prefix(prefix, limit) - use sp_io::{ClearPrefixResult::*, KillStorageResult::*}; + // sp_io::storage::clear_prefix(prefix, maybe_limit, maybe_cursor) + use sp_io::{ClearPrefixResult, KillStorageResult::*}; #[allow(deprecated)] - match kill_prefix(prefix, limit) { - AllRemoved(db) => NoneLeft { db, total: db }, - SomeRemaining(db) => SomeLeft { db, total: db }, - } + let (maybe_cursor, i) = match kill_prefix(prefix, maybe_limit) { + AllRemoved(i) => (None, i), + SomeRemaining(i) => (Some(prefix.to_vec()), i), + }; + ClearPrefixResult { maybe_cursor, db: i, total: i, loops: i } } /// Get a Vec of bytes from storage. diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index 7d4579d3b9726..f2999f945c5c2 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -475,7 +475,7 @@ pub mod pallet { _subkeys: u32, ) -> DispatchResultWithPostInfo { ensure_root(origin)?; - storage::unhashed::clear_prefix(&prefix, None); + let _ = storage::unhashed::clear_prefix(&prefix, None, None); Ok(().into()) } @@ -1427,7 +1427,7 @@ impl Pallet { pub fn reset_events() { >::kill(); EventCount::::kill(); - >::clear(None); + let _ = >::clear(u32::max_value(), None); } /// Assert the given `event` exists. diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index 14c4267fa96fd..95f1f038de46f 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -116,17 +116,18 @@ impl From for KillStorageResult { /// The outcome of calling `clear_prefix` or some function which uses it. #[derive(PassByCodec, Encode, Decode)] +#[must_use] pub struct ClearPrefixResult { /// The number of keys removed from persistent storage. This is what is relevant for weight /// calculation. - db: u32, + pub db: u32, /// The number of keys removed in total (including from non-persistent storage). - total: u32, + pub total: u32, /// The number of backend iterations done for this operation. - loops: u32, + pub loops: u32, /// `None` if all keys to be removed were removed; `Some` if some keys are remaining, in which /// case, the following call should pass this value in as the cursor. - maybe_cursor: Option>, + pub maybe_cursor: Option>, } /// Interface for accessing the storage from within the runtime. @@ -206,31 +207,37 @@ pub trait Storage { } } - /// Clear the storage of each key-value pair where the key starts with the given `prefix`. + /// Partially clear the storage of each key-value pair where the key starts with the given + /// prefix. /// /// # Limit /// - /// Deletes all keys from the overlay and up to `limit` keys from the backend if - /// it is set to `Some`. No limit is applied when `limit` is set to `None`. + /// A *limit* should always be provided through `maybe_limit`. This is one fewer than the + /// maximum number of backend iterations which may be done by this operation and as such + /// represents the maximum number of backend deletions which may happen. A *limit* of zero + /// implies that no keys will be deleted, through there may be a single iteration done. /// - /// The limit can be used to partially delete a prefix storage in case it is too large - /// to delete in one go (block). + /// The limit can be used to partially delete a prefix storage in case it is too large or costly + /// to delete in a single operation. /// - /// Returns [`KillStorageResult`] to inform about the result. + /// # Cursor /// - /// # Note + /// A *cursor* may be passed in to this operation with `maybe_cursor`. `None` should only be + /// passed once (in the initial call) for any given `maybe_prefix` value. Subsequent calls + /// operating on the same prefix should always pass `Some`, and this should be equal to the + /// previous call result's `maybe_prefix` field. /// - /// Please note that keys that are residing in the overlay for that prefix when - /// issuing this call are all deleted without counting towards the `limit`. Only keys - /// written during the current block are part of the overlay. Deleting with a `limit` - /// mostly makes sense with an empty overlay for that prefix. + /// Returns [`ClearPrefixResult`] to inform about the result. Once the resultant `maybe_prefix` + /// field is `None`, then no further items remain to be deleted. /// - /// Calling this function multiple times per block for the same `prefix` does - /// not make much sense because it is not cumulative when called inside the same block. - /// The deletion would always start from `prefix` resulting in the same keys being deleted - /// every time this function is called with the exact same arguments per block. This happens - /// because the keys in the overlay are not taken into account when deleting keys in the - /// backend. + /// NOTE: After the initial call for any given prefix, it is important that no keys further + /// keys under the same prefix are inserted. If so, then they may or may not be deleted by + /// subsequent calls. + /// + /// # Note + /// + /// Please note that keys which are residing in the overlay for that prefix when + /// issuing this call are deleted without counting towards the `limit`. #[version(3, register_only)] fn clear_prefix( &mut self, diff --git a/primitives/state-machine/Cargo.toml b/primitives/state-machine/Cargo.toml index e472b66c5d9ee..1c652966180a7 100644 --- a/primitives/state-machine/Cargo.toml +++ b/primitives/state-machine/Cargo.toml @@ -35,6 +35,7 @@ hex-literal = "0.3.4" pretty_assertions = "1.2.1" rand = "0.7.2" sp-runtime = { version = "6.0.0", path = "../runtime" } +assert_matches = "1.5" [features] default = ["std"] diff --git a/primitives/state-machine/src/ext.rs b/primitives/state-machine/src/ext.rs index b780a0b06ab9f..2de60c6202d98 100644 --- a/primitives/state-machine/src/ext.rs +++ b/primitives/state-machine/src/ext.rs @@ -754,7 +754,7 @@ where let mut loop_count: u32 = 0; let mut maybe_next_key = None; self.backend.apply_to_keys_while(maybe_child, maybe_prefix, maybe_cursor, |key| { - if maybe_limit.map_or(false, |limit| limit == loop_count) { + if maybe_limit.map_or(false, |limit| loop_count == limit) { maybe_next_key = Some(key.to_vec()); return false } @@ -1093,14 +1093,14 @@ mod tests { not_under_prefix.extend(b"path"); ext.set_storage(not_under_prefix.clone(), vec![10]); - ext.clear_prefix(&[], None); - ext.clear_prefix(&well_known_keys::CHILD_STORAGE_KEY_PREFIX[..4], None); + ext.clear_prefix(&[], None, None); + ext.clear_prefix(&well_known_keys::CHILD_STORAGE_KEY_PREFIX[..4], None, None); let mut under_prefix = well_known_keys::CHILD_STORAGE_KEY_PREFIX.to_vec(); under_prefix.extend(b"path"); - ext.clear_prefix(&well_known_keys::CHILD_STORAGE_KEY_PREFIX[..4], None); + ext.clear_prefix(&well_known_keys::CHILD_STORAGE_KEY_PREFIX[..4], None, None); assert_eq!(ext.child_storage(child_info, &[30]), Some(vec![40])); assert_eq!(ext.storage(not_under_prefix.as_slice()), Some(vec![10])); - ext.clear_prefix(¬_under_prefix[..5], None); + ext.clear_prefix(¬_under_prefix[..5], None, None); assert_eq!(ext.storage(not_under_prefix.as_slice()), None); } diff --git a/primitives/state-machine/src/lib.rs b/primitives/state-machine/src/lib.rs index 1d390dcb1073f..fcf6f159e2cf5 100644 --- a/primitives/state-machine/src/lib.rs +++ b/primitives/state-machine/src/lib.rs @@ -1357,8 +1357,8 @@ mod tests { NativeOrEncoded, NeverNativeValue, }; use sp_runtime::traits::BlakeTwo256; - use core::assert_matches; -use std::{ + use assert_matches::assert_matches; + use std::{ collections::{BTreeMap, HashMap}, panic::UnwindSafe, result, @@ -1573,7 +1573,7 @@ use std::{ { let mut cache = StorageTransactionCache::default(); let mut ext = Ext::new(&mut overlay, &mut cache, backend, None); - ext.clear_prefix(b"ab", None); + ext.clear_prefix(b"ab", None, None); } overlay.commit_transaction().unwrap(); @@ -1597,7 +1597,7 @@ use std::{ { let mut cache = StorageTransactionCache::default(); let mut ext = Ext::new(&mut overlay, &mut cache, backend, None); - assert_eq!((false, 1, 3), ext.clear_prefix(b"ab", Some(1))); + assert_matches!(ext.clear_prefix(b"ab", Some(1), None), (Some(_), 1, 3, 1)); } overlay.commit_transaction().unwrap(); @@ -1639,7 +1639,7 @@ use std::{ { let mut cache = StorageTransactionCache::default(); let mut ext = Ext::new(&mut overlay, &mut cache, &backend, None); - assert_eq!(ext.kill_child_storage(&child_info, Some(2)), (false, 2)); + assert_matches!(ext.kill_child_storage(&child_info, Some(2), None), (Some(_), 2, 6, 2)); } assert_eq!( @@ -1675,14 +1675,14 @@ use std::{ let mut cache = StorageTransactionCache::default(); let mut ext = Ext::new(&mut overlay, &mut cache, &backend, None); let r = ext.kill_child_storage(&child_info, Some(0), None); - assert_matches!(r, (0, 0, 1, Some(_))); - let r = ext.kill_child_storage(&child_info, Some(1), r.3); - assert_matches!(r, (1, 1, 2, Some(_))); - let r = ext.kill_child_storage(&child_info, Some(4), r.3); + assert_matches!(r, (Some(_), 0, 0, 0)); + let r = ext.kill_child_storage(&child_info, Some(1), r.0.as_ref().map(|x| &x[..])); + assert_matches!(r, (Some(_), 1, 1, 1)); + let r = ext.kill_child_storage(&child_info, Some(4), r.0.as_ref().map(|x| &x[..])); // Only 3 items remaining to remove - assert_matches!(r, (3, 3, 4, None)); + assert_matches!(r, (None, 3, 3, 3)); let r = ext.kill_child_storage(&child_info, Some(1), None); - assert_matches!(r, (0, 0, 1, None)); + assert_matches!(r, (Some(_), 0, 0, 1)); } #[test] @@ -1700,7 +1700,7 @@ use std::{ let mut overlay = OverlayedChanges::default(); let mut cache = StorageTransactionCache::default(); let mut ext = Ext::new(&mut overlay, &mut cache, &backend, None); - assert_eq!(ext.kill_child_storage(&child_info, None, None), (None, 4, 4, 5)); + assert_eq!(ext.kill_child_storage(&child_info, None, None), (None, 4, 4, 4)); } #[test] diff --git a/primitives/state-machine/src/testing.rs b/primitives/state-machine/src/testing.rs index bc462ae01ab26..450abca03c69f 100644 --- a/primitives/state-machine/src/testing.rs +++ b/primitives/state-machine/src/testing.rs @@ -381,7 +381,7 @@ mod tests { { let mut ext = ext.ext(); - assert!(!ext.kill_child_storage(&child_info, Some(2)).0, "Should not delete all keys"); + assert!(ext.kill_child_storage(&child_info, Some(2), None).0.is_some(), "Should not delete all keys"); assert!(ext.child_storage(&child_info, &b"doe"[..]).is_none()); assert!(ext.child_storage(&child_info, &b"dog"[..]).is_none()); From f435107e0b1e06d5e3174bfee8e231e2eb78355f Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Mon, 23 May 2022 14:59:43 +0100 Subject: [PATCH 24/38] Use unsafe deprecated functions --- frame/bags-list/src/list/mod.rs | 6 ++++-- frame/contracts/src/migration.rs | 1 + frame/contracts/src/storage.rs | 11 ++++++----- frame/im-online/src/lib.rs | 6 ++++-- frame/scheduler/src/lib.rs | 2 ++ frame/society/src/lib.rs | 9 ++++++--- frame/staking/src/pallet/impls.rs | 27 ++++++++++++++++++--------- frame/staking/src/slashing.rs | 6 ++++-- frame/staking/src/testing_utils.rs | 6 ++++-- frame/uniques/src/functions.rs | 6 ++++-- 10 files changed, 53 insertions(+), 27 deletions(-) diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index 26891c9bb239c..b4f852685842d 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -94,8 +94,10 @@ impl, I: 'static> List { /// this function should generally not be used in production as it could lead to a very large /// number of storage accesses. pub(crate) fn unsafe_clear() { - crate::ListBags::::clear(None); - crate::ListNodes::::clear(None); + #[allow(deprecated)] + crate::ListBags::::remove_all(None); + #[allow(deprecated)] + crate::ListNodes::::remove_all(); } /// Regenerate all of the data from the given ids. diff --git a/frame/contracts/src/migration.rs b/frame/contracts/src/migration.rs index 0832ebadac9c6..19e699a855461 100644 --- a/frame/contracts/src/migration.rs +++ b/frame/contracts/src/migration.rs @@ -56,6 +56,7 @@ mod v4 { use super::*; pub fn migrate() -> Weight { + #[allow(deprecated)] migration::remove_storage_prefix(>::name().as_bytes(), b"CurrentSchedule", b""); T::DbWeight::get().writes(1) } diff --git a/frame/contracts/src/storage.rs b/frame/contracts/src/storage.rs index 12e9938b859ca..e99dd26058752 100644 --- a/frame/contracts/src/storage.rs +++ b/frame/contracts/src/storage.rs @@ -27,12 +27,12 @@ use crate::{ use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::{ dispatch::{DispatchError, DispatchResult}, - storage::child::{self, ChildInfo, ClearPrefixResult}, + storage::child::{self, ChildInfo}, weights::Weight, }; use scale_info::TypeInfo; use sp_core::crypto::UncheckedFrom; -use sp_io::hashing::blake2_256; +use sp_io::{hashing::blake2_256, KillStorageResult}; use sp_runtime::{ traits::{Hash, Zero}, RuntimeDebug, @@ -266,16 +266,17 @@ where while !queue.is_empty() && remaining_key_budget > 0 { // Cannot panic due to loop condition let trie = &mut queue[0]; + #[allow(deprecated)] let outcome = child::kill_storage(&child_trie_info(&trie.trie_id), Some(remaining_key_budget)); let keys_removed = match outcome { // This happens when our budget wasn't large enough to remove all keys. - ClearPrefixResult::SomeLeft { db, .. } => db, - ClearPrefixResult::NoneLeft { db, .. } => { + KillStorageResult::SomeRemaining(c) => c, + KillStorageResult::AllRemoved(c) => { // We do not care to preserve order. The contract is deleted already and // no one waits for the trie to be deleted. queue.swap_remove(0); - db + c }, }; remaining_key_budget = remaining_key_budget.saturating_sub(keys_removed); diff --git a/frame/im-online/src/lib.rs b/frame/im-online/src/lib.rs index 10f0a923f274a..f190f6672f309 100644 --- a/frame/im-online/src/lib.rs +++ b/frame/im-online/src/lib.rs @@ -900,8 +900,10 @@ impl OneSessionHandler for Pallet { // Remove all received heartbeats and number of authored blocks from the // current session, they have already been processed and won't be needed // anymore. - ReceivedHeartbeats::::clear_prefix(&T::ValidatorSet::session_index(), None); - AuthoredBlocks::::clear_prefix(&T::ValidatorSet::session_index(), None); + #[allow(deprecated)] + ReceivedHeartbeats::::remove_prefix(&T::ValidatorSet::session_index(), None); + #[allow(deprecated)] + AuthoredBlocks::::remove_prefix(&T::ValidatorSet::session_index(), None); if offenders.is_empty() { Self::deposit_event(Event::::AllGood); diff --git a/frame/scheduler/src/lib.rs b/frame/scheduler/src/lib.rs index 9b0fc87587e3a..a005c051a1abc 100644 --- a/frame/scheduler/src/lib.rs +++ b/frame/scheduler/src/lib.rs @@ -565,6 +565,7 @@ impl Pallet { }, ); + #[allow(deprecated)] frame_support::storage::migration::remove_storage_prefix( Self::name().as_bytes(), b"StorageVersion", @@ -601,6 +602,7 @@ impl Pallet { ) }); + #[allow(deprecated)] frame_support::storage::migration::remove_storage_prefix( Self::name().as_bytes(), b"StorageVersion", diff --git a/frame/society/src/lib.rs b/frame/society/src/lib.rs index e8e429807258d..5a993f72f32d2 100644 --- a/frame/society/src/lib.rs +++ b/frame/society/src/lib.rs @@ -1067,7 +1067,8 @@ pub mod pallet { Founder::::kill(); Rules::::kill(); Candidates::::kill(); - SuspendedCandidates::::clear(None); + #[allow(deprecated)] + SuspendedCandidates::::remove_all(None); Self::deposit_event(Event::::Unfounded { founder }); Ok(()) } @@ -1511,7 +1512,8 @@ impl, I: 'static> Pallet { .collect::>(); // Clean up all votes. - >::clear(None); + #[allow(deprecated)] + >::remove_all(None); // Reward one of the voters who voted the right way. if !total_slash.is_zero() { @@ -1695,7 +1697,8 @@ impl, I: 'static> Pallet { } // Clean up all votes. - >::clear(None); + #[allow(deprecated)] + >::remove_all(None); } // Avoid challenging if there's only two members since we never challenge the Head or diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index efb537b88294c..66265ab1f135e 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -585,9 +585,12 @@ impl Pallet { /// Clear all era information for given era. pub(crate) fn clear_era_information(era_index: EraIndex) { - >::clear_prefix(era_index, None); - >::clear_prefix(era_index, None); - >::clear_prefix(era_index, None); + #[allow(deprecated)] + >::remove_prefix(era_index, None); + #[allow(deprecated)] + >::remove_prefix(era_index, None); + #[allow(deprecated)] + >::remove_prefix(era_index, None); >::remove(era_index); >::remove(era_index); >::remove(era_index); @@ -984,10 +987,14 @@ impl ElectionDataProvider for Pallet { #[cfg(feature = "runtime-benchmarks")] fn clear() { - >::clear(None); - >::clear(None); - >::clear(None); - >::clear(None); + #[allow(deprecated)] + >::remove_all(None); + #[allow(deprecated)] + >::remove_all(None); + #[allow(deprecated)] + >::remove_all(); + #[allow(deprecated)] + >::remove_all(); T::VoterList::unsafe_clear(); } @@ -1368,8 +1375,10 @@ impl SortedListProvider for UseNominatorsAndValidatorsM fn unsafe_clear() { // NOTE: Caller must ensure this doesn't lead to too many storage accesses. This is a // condition of SortedListProvider::unsafe_clear. - Nominators::::clear(None); - Validators::::clear(None); + #[allow(deprecated)] + Nominators::::remove_all(); + #[allow(deprecated)] + Validators::::remove_all(); } } diff --git a/frame/staking/src/slashing.rs b/frame/staking/src/slashing.rs index 1fd1790149cc8..7372c4390f816 100644 --- a/frame/staking/src/slashing.rs +++ b/frame/staking/src/slashing.rs @@ -557,8 +557,10 @@ impl<'a, T: 'a + Config> Drop for InspectingSpans<'a, T> { /// Clear slashing metadata for an obsolete era. pub(crate) fn clear_era_metadata(obsolete_era: EraIndex) { - as Store>::ValidatorSlashInEra::clear_prefix(&obsolete_era, None); - as Store>::NominatorSlashInEra::clear_prefix(&obsolete_era, None); + #[allow(deprecated)] + as Store>::ValidatorSlashInEra::remove_prefix(&obsolete_era, None); + #[allow(deprecated)] + as Store>::NominatorSlashInEra::remove_prefix(&obsolete_era, None); } /// Clear slashing metadata for a dead account. diff --git a/frame/staking/src/testing_utils.rs b/frame/staking/src/testing_utils.rs index 3e6c109df28aa..ba67292ddc434 100644 --- a/frame/staking/src/testing_utils.rs +++ b/frame/staking/src/testing_utils.rs @@ -36,10 +36,12 @@ const SEED: u32 = 0; /// This function removes all validators and nominators from storage. pub fn clear_validators_and_nominators() { - Validators::::clear(); + #[allow(deprecated)] + Validators::::remove_all(); // whenever we touch nominators counter we should update `T::VoterList` as well. - Nominators::::clear(); + #[allow(deprecated)] + Nominators::::remove_all(); // NOTE: safe to call outside block production T::VoterList::unsafe_clear(); diff --git a/frame/uniques/src/functions.rs b/frame/uniques/src/functions.rs index d40a36b14d96c..c5220b562b172 100644 --- a/frame/uniques/src/functions.rs +++ b/frame/uniques/src/functions.rs @@ -110,9 +110,11 @@ impl, I: 'static> Pallet { for (item, details) in Item::::drain_prefix(&collection) { Account::::remove((&details.owner, &collection, &item)); } - ItemMetadataOf::::clear_prefix(&collection, None); + #[allow(deprecated)] + ItemMetadataOf::::remove_prefix(&collection, None); CollectionMetadataOf::::remove(&collection); - Attribute::::clear_prefix((&collection,), None); + #[allow(deprecated)] + Attribute::::remove_prefix((&collection,), None); CollectionAccount::::remove(&collection_details.owner, &collection); T::Currency::unreserve(&collection_details.owner, collection_details.total_deposit); From bc181a0a829ebdc9f38d08210d84b4a3ce2e5983 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Mon, 23 May 2022 16:12:59 +0200 Subject: [PATCH 25/38] Formatting --- frame/contracts/src/storage.rs | 3 +- .../src/storage/generator/double_map.rs | 16 +++--- frame/support/src/storage/generator/nmap.rs | 12 ++++- frame/support/src/storage/migration.rs | 2 +- frame/support/src/storage/types/double_map.rs | 12 ++++- frame/support/src/storage/types/nmap.rs | 12 ++++- primitives/io/src/lib.rs | 17 +++++-- primitives/state-machine/src/ext.rs | 51 ++++++++++--------- primitives/state-machine/src/lib.rs | 2 +- primitives/state-machine/src/testing.rs | 5 +- .../state-machine/src/trie_backend_essence.rs | 3 +- 11 files changed, 88 insertions(+), 47 deletions(-) diff --git a/frame/contracts/src/storage.rs b/frame/contracts/src/storage.rs index e99dd26058752..fe220cc42975c 100644 --- a/frame/contracts/src/storage.rs +++ b/frame/contracts/src/storage.rs @@ -267,8 +267,7 @@ where // Cannot panic due to loop condition let trie = &mut queue[0]; #[allow(deprecated)] - let outcome = - child::kill_storage(&child_trie_info(&trie.trie_id), Some(remaining_key_budget)); + let outcome = child::kill_storage(&child_trie_info(&trie.trie_id), Some(remaining_key_budget)); let keys_removed = match outcome { // This happens when our budget wasn't large enough to remove all keys. KillStorageResult::SomeRemaining(c) => c, diff --git a/frame/support/src/storage/generator/double_map.rs b/frame/support/src/storage/generator/double_map.rs index c1bca318ae68b..99cd305a319e0 100644 --- a/frame/support/src/storage/generator/double_map.rs +++ b/frame/support/src/storage/generator/double_map.rs @@ -206,14 +206,15 @@ where where KArg1: EncodeLike, { - unhashed::clear_prefix( - Self::storage_double_map_final_key1(k1).as_ref(), - maybe_limit, - None, - ).into() + unhashed::clear_prefix(Self::storage_double_map_final_key1(k1).as_ref(), maybe_limit, None) + .into() } - fn clear_prefix(k1: KArg1, limit: u32, maybe_cursor: Option<&[u8]>) -> sp_io::ClearPrefixResult + fn clear_prefix( + k1: KArg1, + limit: u32, + maybe_cursor: Option<&[u8]>, + ) -> sp_io::ClearPrefixResult where KArg1: EncodeLike, { @@ -221,7 +222,8 @@ where Self::storage_double_map_final_key1(k1).as_ref(), Some(limit), maybe_cursor, - ).into() + ) + .into() } fn iter_prefix_values(k1: KArg1) -> storage::PrefixIterator diff --git a/frame/support/src/storage/generator/nmap.rs b/frame/support/src/storage/generator/nmap.rs index a4bc9ecb20652..d127ca4b07cf2 100755 --- a/frame/support/src/storage/generator/nmap.rs +++ b/frame/support/src/storage/generator/nmap.rs @@ -186,11 +186,19 @@ where unhashed::clear_prefix(&Self::storage_n_map_partial_key(partial_key), limit, None).into() } - fn clear_prefix(partial_key: KP, limit: u32, maybe_cursor: Option<&[u8]>) -> sp_io::ClearPrefixResult + fn clear_prefix( + partial_key: KP, + limit: u32, + maybe_cursor: Option<&[u8]>, + ) -> sp_io::ClearPrefixResult where K: HasKeyPrefix, { - unhashed::clear_prefix(&Self::storage_n_map_partial_key(partial_key), Some(limit), maybe_cursor) + unhashed::clear_prefix( + &Self::storage_n_map_partial_key(partial_key), + Some(limit), + maybe_cursor, + ) } fn iter_prefix_values(partial_key: KP) -> PrefixIterator diff --git a/frame/support/src/storage/migration.rs b/frame/support/src/storage/migration.rs index 93dcb3184e060..b4f16eeab20e5 100644 --- a/frame/support/src/storage/migration.rs +++ b/frame/support/src/storage/migration.rs @@ -285,7 +285,7 @@ pub fn clear_storage_prefix( item: &[u8], hash: &[u8], maybe_limit: Option, - maybe_cursor: Option<&[u8]> + maybe_cursor: Option<&[u8]>, ) -> sp_io::ClearPrefixResult { let mut key = vec![0u8; 32 + hash.len()]; let storage_prefix = storage_prefix(module, item); diff --git a/frame/support/src/storage/types/double_map.rs b/frame/support/src/storage/types/double_map.rs index 07695c1854830..39b9b5a57455c 100644 --- a/frame/support/src/storage/types/double_map.rs +++ b/frame/support/src/storage/types/double_map.rs @@ -261,11 +261,19 @@ where /// passed once (in the initial call) for any given storage map and `first_key`. Subsequent /// calls operating on the same map/`first_key` should always pass `Some`, and this should be /// equal to the previous call result's `maybe_cursor` field. - pub fn clear_prefix(first_key: KArg1, limit: u32, maybe_cursor: Option<&[u8]>) -> sp_io::ClearPrefixResult + pub fn clear_prefix( + first_key: KArg1, + limit: u32, + maybe_cursor: Option<&[u8]>, + ) -> sp_io::ClearPrefixResult where KArg1: ?Sized + EncodeLike, { - >::clear_prefix(first_key, limit, maybe_cursor) + >::clear_prefix( + first_key, + limit, + maybe_cursor, + ) } /// Iterate over values that share the first key. diff --git a/frame/support/src/storage/types/nmap.rs b/frame/support/src/storage/types/nmap.rs index a517e3281b672..e548523d5f78d 100755 --- a/frame/support/src/storage/types/nmap.rs +++ b/frame/support/src/storage/types/nmap.rs @@ -217,11 +217,19 @@ where /// passed once (in the initial call) for any given storage map and `partial_key`. Subsequent /// calls operating on the same map/`partial_key` should always pass `Some`, and this should be /// equal to the previous call result's `maybe_cursor` field. - pub fn clear_prefix(partial_key: KP, limit: u32, maybe_cursor: Option<&[u8]>) -> sp_io::ClearPrefixResult + pub fn clear_prefix( + partial_key: KP, + limit: u32, + maybe_cursor: Option<&[u8]>, + ) -> sp_io::ClearPrefixResult where Key: HasKeyPrefix, { - >::clear_prefix(partial_key, limit, maybe_cursor) + >::clear_prefix( + partial_key, + limit, + maybe_cursor, + ) } /// Iterate over values that share the first key. diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index 95f1f038de46f..f5d3e658c3e79 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -200,7 +200,8 @@ pub trait Storage { /// backend. #[version(2)] fn clear_prefix(&mut self, prefix: &[u8], limit: Option) -> KillStorageResult { - let (maybe_cursor, num_removed, ..) = Externalities::clear_prefix(*self, prefix, limit, None); + let (maybe_cursor, num_removed, ..) = + Externalities::clear_prefix(*self, prefix, limit, None); match maybe_cursor { None => KillStorageResult::AllRemoved(num_removed), Some(_) => KillStorageResult::SomeRemaining(num_removed), @@ -243,7 +244,7 @@ pub trait Storage { &mut self, maybe_prefix: &[u8], maybe_limit: Option, - maybe_cursor: Option>, //< TODO Make work or just Option>? + maybe_cursor: Option>, //< TODO Make work or just Option>? ) -> ClearPrefixResult { let (maybe_cursor, db, total, loops) = Externalities::clear_prefix( *self, @@ -425,12 +426,17 @@ pub trait DefaultChildStorage { /// /// See `Storage` module `clear_prefix` documentation for `limit` usage. #[version(4, register_only)] - fn storage_kill(&mut self, storage_key: &[u8], maybe_limit: Option, maybe_cursor: Option>) -> ClearPrefixResult { + fn storage_kill( + &mut self, + storage_key: &[u8], + maybe_limit: Option, + maybe_cursor: Option>, + ) -> ClearPrefixResult { let child_info = ChildInfo::new_default(storage_key); let (maybe_cursor, db, total, loops) = self.kill_child_storage( &child_info, maybe_limit, - maybe_cursor.as_ref().map(|x| &x[..]) + maybe_cursor.as_ref().map(|x| &x[..]), ); ClearPrefixResult { maybe_cursor, db, total, loops } } @@ -462,7 +468,8 @@ pub trait DefaultChildStorage { limit: Option, ) -> KillStorageResult { let child_info = ChildInfo::new_default(storage_key); - let (maybe_cursor, num_removed, ..) = self.clear_child_prefix(&child_info, prefix, limit, None); + let (maybe_cursor, num_removed, ..) = + self.clear_child_prefix(&child_info, prefix, limit, None); match maybe_cursor { None => KillStorageResult::AllRemoved(num_removed), Some(..) => KillStorageResult::SomeRemaining(num_removed), diff --git a/primitives/state-machine/src/ext.rs b/primitives/state-machine/src/ext.rs index 2de60c6202d98..b241365480fd9 100644 --- a/primitives/state-machine/src/ext.rs +++ b/primitives/state-machine/src/ext.rs @@ -449,7 +449,7 @@ where self.mark_dirty(); let overlay_count = self.overlay.clear_child_storage(child_info); let (next_cursor, backend_count, iterations) = - self.limit_remove_from_backend(Some(child_info), None, maybe_limit, maybe_cursor); + self.limit_remove_from_backend(Some(child_info), None, maybe_limit, maybe_cursor); (next_cursor, backend_count, overlay_count + backend_count, iterations) } @@ -500,8 +500,12 @@ where self.mark_dirty(); let overlay_count = self.overlay.clear_child_prefix(child_info, prefix); - let (next_cursor, backend_count, iterations) = - self.limit_remove_from_backend(Some(child_info), Some(prefix), maybe_limit, maybe_cursor); + let (next_cursor, backend_count, iterations) = self.limit_remove_from_backend( + Some(child_info), + Some(prefix), + maybe_limit, + maybe_cursor, + ); (next_cursor, backend_count, backend_count + overlay_count, iterations) } @@ -753,27 +757,28 @@ where let mut delete_count: u32 = 0; let mut loop_count: u32 = 0; let mut maybe_next_key = None; - self.backend.apply_to_keys_while(maybe_child, maybe_prefix, maybe_cursor, |key| { - if maybe_limit.map_or(false, |limit| loop_count == limit) { - maybe_next_key = Some(key.to_vec()); - return false - } - let overlay = match maybe_child { - Some(child_info) => self.overlay.child_storage(child_info, key), - None => self.overlay.storage(key), - }; - if !matches!(overlay, Some(None)) { - // already pending deletion from the backend - no need to delete it again. - if let Some(child_info) = maybe_child { - self.overlay.set_child_storage(child_info, key.to_vec(), None); - } else { - self.overlay.set_storage(key.to_vec(), None); + self.backend + .apply_to_keys_while(maybe_child, maybe_prefix, maybe_cursor, |key| { + if maybe_limit.map_or(false, |limit| loop_count == limit) { + maybe_next_key = Some(key.to_vec()); + return false } - delete_count = delete_count.saturating_add(1); - } - loop_count = loop_count.saturating_add(1); - true - }); + let overlay = match maybe_child { + Some(child_info) => self.overlay.child_storage(child_info, key), + None => self.overlay.storage(key), + }; + if !matches!(overlay, Some(None)) { + // already pending deletion from the backend - no need to delete it again. + if let Some(child_info) = maybe_child { + self.overlay.set_child_storage(child_info, key.to_vec(), None); + } else { + self.overlay.set_storage(key.to_vec(), None); + } + delete_count = delete_count.saturating_add(1); + } + loop_count = loop_count.saturating_add(1); + true + }); (maybe_next_key, delete_count, loop_count) } } diff --git a/primitives/state-machine/src/lib.rs b/primitives/state-machine/src/lib.rs index fcf6f159e2cf5..57bdcec65a847 100644 --- a/primitives/state-machine/src/lib.rs +++ b/primitives/state-machine/src/lib.rs @@ -1348,6 +1348,7 @@ mod execution { mod tests { use super::{ext::Ext, *}; use crate::execution::CallResult; + use assert_matches::assert_matches; use codec::{Decode, Encode}; use sp_core::{ map, @@ -1357,7 +1358,6 @@ mod tests { NativeOrEncoded, NeverNativeValue, }; use sp_runtime::traits::BlakeTwo256; - use assert_matches::assert_matches; use std::{ collections::{BTreeMap, HashMap}, panic::UnwindSafe, diff --git a/primitives/state-machine/src/testing.rs b/primitives/state-machine/src/testing.rs index 450abca03c69f..c09cecc614621 100644 --- a/primitives/state-machine/src/testing.rs +++ b/primitives/state-machine/src/testing.rs @@ -381,7 +381,10 @@ mod tests { { let mut ext = ext.ext(); - assert!(ext.kill_child_storage(&child_info, Some(2), None).0.is_some(), "Should not delete all keys"); + assert!( + ext.kill_child_storage(&child_info, Some(2), None).0.is_some(), + "Should not delete all keys" + ); assert!(ext.child_storage(&child_info, &b"doe"[..]).is_none()); assert!(ext.child_storage(&child_info, &b"dog"[..]).is_none()); diff --git a/primitives/state-machine/src/trie_backend_essence.rs b/primitives/state-machine/src/trie_backend_essence.rs index 49f18e3148cf1..c55a6b7e80481 100644 --- a/primitives/state-machine/src/trie_backend_essence.rs +++ b/primitives/state-machine/src/trie_backend_essence.rs @@ -342,7 +342,8 @@ where let trie = TrieDB::::new(db, root)?; let prefix = maybe_prefix.unwrap_or(&[]); let iter = match maybe_start_at { - Some(start_at) => TrieDBKeyIterator::new_prefixed_then_seek(&trie, prefix, start_at), + Some(start_at) => + TrieDBKeyIterator::new_prefixed_then_seek(&trie, prefix, start_at), None => TrieDBKeyIterator::new_prefixed(&trie, prefix), }?; From 0e51dbe7607a909a891e7bdf4bea9a253e8611de Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Mon, 23 May 2022 17:53:11 +0200 Subject: [PATCH 26/38] Fixes --- frame/elections-phragmen/src/benchmarking.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frame/elections-phragmen/src/benchmarking.rs b/frame/elections-phragmen/src/benchmarking.rs index ae4bf0f6a3f28..44dd6aff09f0c 100644 --- a/frame/elections-phragmen/src/benchmarking.rs +++ b/frame/elections-phragmen/src/benchmarking.rs @@ -148,7 +148,8 @@ fn clean() { >::kill(); >::kill(); >::kill(); - >::clear(None); + #[allow(deprecated)] + >::remove_all(None); } benchmarks! { From ee53604123175c688140ba7f2104d61864bb62fd Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Wed, 25 May 2022 12:36:16 +0200 Subject: [PATCH 27/38] Grumbles --- frame/support/src/storage/child.rs | 22 ++++--- frame/support/src/storage/migration.rs | 2 +- frame/support/src/storage/mod.rs | 8 +-- .../support/src/storage/types/counted_map.rs | 6 +- frame/support/src/storage/types/double_map.rs | 10 +-- frame/support/src/storage/types/map.rs | 6 +- frame/support/src/storage/types/nmap.rs | 8 +-- frame/support/src/storage/unhashed.rs | 13 ++-- primitives/externalities/src/lib.rs | 26 ++++++-- primitives/io/src/lib.rs | 63 ++++++++++--------- primitives/state-machine/src/basic.rs | 27 ++++---- primitives/state-machine/src/ext.rs | 30 ++++----- primitives/state-machine/src/lib.rs | 15 ++--- primitives/state-machine/src/read_only.rs | 7 ++- primitives/state-machine/src/testing.rs | 2 +- primitives/tasks/src/async_externalities.rs | 6 +- 16 files changed, 143 insertions(+), 108 deletions(-) diff --git a/frame/support/src/storage/child.rs b/frame/support/src/storage/child.rs index f3255ce9ef9be..28f04ad7eb9ec 100644 --- a/frame/support/src/storage/child.rs +++ b/frame/support/src/storage/child.rs @@ -151,7 +151,7 @@ pub fn kill_storage(child_info: &ChildInfo, limit: Option) -> KillStorageRe /// A *limit* should always be provided through `maybe_limit`. This is one fewer than the /// maximum number of backend iterations which may be done by this operation and as such /// represents the maximum number of backend deletions which may happen. A *limit* of zero -/// implies that no keys will be deleted, through there may be a single iteration done. +/// implies that no keys will be deleted, though there may be a single iteration done. /// /// The limit can be used to partially delete storage items in case it is too large or costly /// to delete all in a single operation. @@ -159,11 +159,14 @@ pub fn kill_storage(child_info: &ChildInfo, limit: Option) -> KillStorageRe /// # Cursor /// /// A *cursor* may be passed in to this operation with `maybe_cursor`. `None` should only be -/// passed once (in the initial call) for any attempt to clear storage. Subsequent calls -/// operating on the same prefix should always pass `Some`, and this should be equal to the -/// previous call result's `maybe_prefix` field. +/// passed once (in the initial call) for any attempt to clear storage. In general, subsequent calls +/// operating on the same prefix should pass `Some` and this value should be equal to the +/// previous call result's `maybe_cursor` field. The only exception to this is when you can +/// guarantee that the subsequent call is in a new block; in this case the previous call's result +/// cursor need not be passed in an a `None` may be passed instead. This exception may be useful +/// then making this call solely from a block-hook such as `on_initialize`. /// -/// Returns [`ClearPrefixResult`] to inform about the result. Once the resultant `maybe_prefix` +/// Returns [`ClearPrefixResult`] to inform about the result. Once the resultant `maybe_cursor` /// field is `None`, then no further items remain to be deleted. /// /// NOTE: After the initial call for any given child storage, it is important that no keys further @@ -186,10 +189,11 @@ pub fn clear_storage( sp_io::default_child_storage::storage_kill(child_info.storage_key(), maybe_limit), }; use sp_io::KillStorageResult::*; - match r { - AllRemoved(db) => ClearPrefixResult { maybe_cursor: None, db, total: db, loops: db }, - SomeRemaining(db) => ClearPrefixResult { maybe_cursor: None, db, total: db, loops: db }, - } + let (maybe_cursor, db) = match r { + AllRemoved(db) => (None, db), + SomeRemaining(db) => (Some(child_info.storage_key().to_vec()), db), + }; + ClearPrefixResult { maybe_cursor, db, total: db, loops: db } } /// Ensure `key` has no explicit entry in storage. diff --git a/frame/support/src/storage/migration.rs b/frame/support/src/storage/migration.rs index b4f16eeab20e5..5a6e88f94f3c7 100644 --- a/frame/support/src/storage/migration.rs +++ b/frame/support/src/storage/migration.rs @@ -265,7 +265,7 @@ pub fn remove_storage_prefix(module: &[u8], item: &[u8], hash: &[u8]) { let _ = frame_support::storage::unhashed::clear_prefix(&key, None, None); } -/// Attmept to remove all values under a storage prefix by the `module`, the map's `item` name and +/// Attempt to remove all values under a storage prefix by the `module`, the map's `item` name and /// the key `hash`. /// /// All values in the client overlay will be deleted, if `maybe_limit` is `Some` then up to diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index b8ad47a474058..954dd69a3db8f 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -698,8 +698,8 @@ pub trait StorageNMap { /// A `limit` must be provided in order to cap the maximum /// amount of deletions done in a single call. This is one fewer than the /// maximum number of backend iterations which may be done by this operation and as such - /// represents the maximum number of backend deletions which may happen. A *limit* of zero - /// implies that no keys will be deleted, through there may be a single iteration done. + /// represents the maximum number of backend deletions which may happen. A `limit` of zero + /// implies that no keys will be deleted, though there may be a single iteration done. /// /// # Cursor /// @@ -1184,8 +1184,8 @@ pub trait StoragePrefixedMap { /// A `limit` must always be provided through in order to cap the maximum /// amount of deletions done in a single call. This is one fewer than the /// maximum number of backend iterations which may be done by this operation and as such - /// represents the maximum number of backend deletions which may happen. A *limit* of zero - /// implies that no keys will be deleted, through there may be a single iteration done. + /// represents the maximum number of backend deletions which may happen. A `limit` of zero + /// implies that no keys will be deleted, though there may be a single iteration done. /// /// # Cursor /// diff --git a/frame/support/src/storage/types/counted_map.rs b/frame/support/src/storage/types/counted_map.rs index fdfa92c366014..9ae3d3c575870 100644 --- a/frame/support/src/storage/types/counted_map.rs +++ b/frame/support/src/storage/types/counted_map.rs @@ -293,11 +293,11 @@ where /// /// # Limit /// - /// A *limit* should always be provided through `maybe_limit` in order to cap the maximum + /// A `limit` must always be provided through in order to cap the maximum /// amount of deletions done in a single call. This is one fewer than the /// maximum number of backend iterations which may be done by this operation and as such - /// represents the maximum number of backend deletions which may happen. A *limit* of zero - /// implies that no keys will be deleted, through there may be a single iteration done. + /// represents the maximum number of backend deletions which may happen. A `limit` of zero + /// implies that no keys will be deleted, though there may be a single iteration done. /// /// # Cursor /// diff --git a/frame/support/src/storage/types/double_map.rs b/frame/support/src/storage/types/double_map.rs index 39b9b5a57455c..6a36d8ce3e3b9 100644 --- a/frame/support/src/storage/types/double_map.rs +++ b/frame/support/src/storage/types/double_map.rs @@ -249,11 +249,11 @@ where /// /// # Limit /// - /// A *limit* should always be provided through `maybe_limit` in order to cap the maximum + /// A `limit` must always be provided through in order to cap the maximum /// amount of deletions done in a single call. This is one fewer than the /// maximum number of backend iterations which may be done by this operation and as such - /// represents the maximum number of backend deletions which may happen. A *limit* of zero - /// implies that no keys will be deleted, through there may be a single iteration done. + /// represents the maximum number of backend deletions which may happen. A `limit` of zero + /// implies that no keys will be deleted, though there may be a single iteration done. /// /// # Cursor /// @@ -419,8 +419,8 @@ where /// A `limit` must always be provided through in order to cap the maximum /// amount of deletions done in a single call. This is one fewer than the /// maximum number of backend iterations which may be done by this operation and as such - /// represents the maximum number of backend deletions which may happen. A *limit* of zero - /// implies that no keys will be deleted, through there may be a single iteration done. + /// represents the maximum number of backend deletions which may happen.A `limit` of zero + /// implies that no keys will be deleted, though there may be a single iteration done. /// /// # Cursor /// diff --git a/frame/support/src/storage/types/map.rs b/frame/support/src/storage/types/map.rs index 1c046318a2197..53b5f5f7467a1 100644 --- a/frame/support/src/storage/types/map.rs +++ b/frame/support/src/storage/types/map.rs @@ -264,11 +264,11 @@ where /// /// # Limit /// - /// A *limit* should always be provided through `maybe_limit` in order to cap the maximum + /// A `limit` must always be provided through in order to cap the maximum /// amount of deletions done in a single call. This is one fewer than the /// maximum number of backend iterations which may be done by this operation and as such - /// represents the maximum number of backend deletions which may happen. A *limit* of zero - /// implies that no keys will be deleted, through there may be a single iteration done. + /// represents the maximum number of backend deletions which may happen. A `limit` of zero + /// implies that no keys will be deleted, though there may be a single iteration done. /// /// # Cursor /// diff --git a/frame/support/src/storage/types/nmap.rs b/frame/support/src/storage/types/nmap.rs index e548523d5f78d..d049e25c6bd8a 100755 --- a/frame/support/src/storage/types/nmap.rs +++ b/frame/support/src/storage/types/nmap.rs @@ -208,8 +208,8 @@ where /// A `limit` must be provided in order to cap the maximum /// amount of deletions done in a single call. This is one fewer than the /// maximum number of backend iterations which may be done by this operation and as such - /// represents the maximum number of backend deletions which may happen. A *limit* of zero - /// implies that no keys will be deleted, through there may be a single iteration done. + /// represents the maximum number of backend deletions which may happen. A `limit` of zero + /// implies that no keys will be deleted, though there may be a single iteration done. /// /// # Cursor /// @@ -359,8 +359,8 @@ where /// A `limit` must always be provided through in order to cap the maximum /// amount of deletions done in a single call. This is one fewer than the /// maximum number of backend iterations which may be done by this operation and as such - /// represents the maximum number of backend deletions which may happen. A *limit* of zero - /// implies that no keys will be deleted, through there may be a single iteration done. + /// represents the maximum number of backend deletions which may happen. A `limit` of zero + /// implies that no keys will be deleted, though there may be a single iteration done. /// /// # Cursor /// diff --git a/frame/support/src/storage/unhashed.rs b/frame/support/src/storage/unhashed.rs index 68b32cc6e3e39..a2a4a267b739d 100644 --- a/frame/support/src/storage/unhashed.rs +++ b/frame/support/src/storage/unhashed.rs @@ -111,7 +111,7 @@ pub fn kill_prefix(prefix: &[u8], limit: Option) -> sp_io::KillStorageResul /// A *limit* should always be provided through `maybe_limit`. This is one fewer than the /// maximum number of backend iterations which may be done by this operation and as such /// represents the maximum number of backend deletions which may happen. A *limit* of zero -/// implies that no keys will be deleted, through there may be a single iteration done. +/// implies that no keys will be deleted, though there may be a single iteration done. /// /// The limit can be used to partially delete storage items in case it is too large or costly /// to delete all in a single operation. @@ -119,11 +119,14 @@ pub fn kill_prefix(prefix: &[u8], limit: Option) -> sp_io::KillStorageResul /// # Cursor /// /// A *cursor* may be passed in to this operation with `maybe_cursor`. `None` should only be -/// passed once (in the initial call) for any attempt to clear storage. Subsequent calls -/// operating on the same prefix should always pass `Some`, and this should be equal to the -/// previous call result's `maybe_prefix` field. +/// passed once (in the initial call) for any attempt to clear storage. In general, subsequent calls +/// operating on the same prefix should pass `Some` and this value should be equal to the +/// previous call result's `maybe_cursor` field. The only exception to this is when you can +/// guarantee that the subsequent call is in a new block; in this case the previous call's result +/// cursor need not be passed in an a `None` may be passed instead. This exception may be useful +/// then making this call solely from a block-hook such as `on_initialize`. /// -/// Returns [`ClearPrefixResult`] to inform about the result. Once the resultant `maybe_prefix` +/// Returns [`ClearPrefixResult`] to inform about the result. Once the resultant `maybe_cursor` /// field is `None`, then no further items remain to be deleted. /// /// NOTE: After the initial call for any given child storage, it is important that no keys further diff --git a/primitives/externalities/src/lib.rs b/primitives/externalities/src/lib.rs index b60b5c59acb07..7b568bc640190 100644 --- a/primitives/externalities/src/lib.rs +++ b/primitives/externalities/src/lib.rs @@ -52,6 +52,25 @@ pub enum Error { StorageUpdateFailed(&'static str), } +/// Results concerning an operation to remove many keys. +pub struct MultiRemovalResults { + /// A continuation cursor which, if `Some` must be provided to the subsequent removal call. + /// If `None` then all removals are complete and no further calls are needed. + pub maybe_cursor: Option>, + /// The number of items removed from the backend database. + pub backend: u32, + /// The number of unique keys removed, taking into account both the backend and the overlay. + pub unique: u32, + /// The number of iterations (each requiring a storage seek/read) which were done. + pub loops: u32, +} + +impl MultiRemovalResults { + pub fn decon(self) -> (Option>, u32, u32, u32) { + (self.maybe_cursor, self.backend, self.unique, self.loops) + } +} + /// The Substrate externalities. /// /// Provides access to the storage and to other registered extensions. @@ -127,7 +146,6 @@ pub trait Externalities: ExtensionStore { /// /// As long as `maybe_cursor` is passed from the result of the previous call, then the number of /// iterations done will only ever be one more than the number of keys removed. - /// than `maybe_limit` (if `Some`). /// /// # Note /// @@ -138,7 +156,7 @@ pub trait Externalities: ExtensionStore { child_info: &ChildInfo, maybe_limit: Option, maybe_cursor: Option<&[u8]>, - ) -> (Option>, u32, u32, u32); + ) -> MultiRemovalResults; /// Clear storage entries which keys are start with the given prefix. /// @@ -148,7 +166,7 @@ pub trait Externalities: ExtensionStore { prefix: &[u8], maybe_limit: Option, maybe_cursor: Option<&[u8]>, - ) -> (Option>, u32, u32, u32); + ) -> MultiRemovalResults; /// Clear child storage entries which keys are start with the given prefix. /// @@ -159,7 +177,7 @@ pub trait Externalities: ExtensionStore { prefix: &[u8], maybe_limit: Option, maybe_cursor: Option<&[u8]>, - ) -> (Option>, u32, u32, u32); + ) -> MultiRemovalResults; /// Set or clear a storage entry (`key`) of current contract being called (effective /// immediately). diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index f5d3e658c3e79..d62f4dfa9a3e8 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -44,6 +44,7 @@ use sp_core::{ }; #[cfg(feature = "std")] use sp_keystore::{KeystoreExt, SyncCryptoStore}; +use sp_externalities::MultiRemovalResults; use sp_core::{ crypto::KeyTypeId, @@ -130,6 +131,17 @@ pub struct ClearPrefixResult { pub maybe_cursor: Option>, } +impl From for ClearPrefixResult { + fn from(r: MultiRemovalResults) -> Self { + Self { + db: r.backend, + total: r.unique, + loops: r.loops, + maybe_cursor: r.maybe_cursor, + } + } +} + /// Interface for accessing the storage from within the runtime. #[runtime_interface] pub trait Storage { @@ -200,11 +212,10 @@ pub trait Storage { /// backend. #[version(2)] fn clear_prefix(&mut self, prefix: &[u8], limit: Option) -> KillStorageResult { - let (maybe_cursor, num_removed, ..) = - Externalities::clear_prefix(*self, prefix, limit, None); - match maybe_cursor { - None => KillStorageResult::AllRemoved(num_removed), - Some(_) => KillStorageResult::SomeRemaining(num_removed), + let r = Externalities::clear_prefix(*self, prefix, limit, None); + match r.maybe_cursor { + None => KillStorageResult::AllRemoved(r.loops), + Some(_) => KillStorageResult::SomeRemaining(r.loops), } } @@ -216,7 +227,7 @@ pub trait Storage { /// A *limit* should always be provided through `maybe_limit`. This is one fewer than the /// maximum number of backend iterations which may be done by this operation and as such /// represents the maximum number of backend deletions which may happen. A *limit* of zero - /// implies that no keys will be deleted, through there may be a single iteration done. + /// implies that no keys will be deleted, though there may be a single iteration done. /// /// The limit can be used to partially delete a prefix storage in case it is too large or costly /// to delete in a single operation. @@ -226,9 +237,9 @@ pub trait Storage { /// A *cursor* may be passed in to this operation with `maybe_cursor`. `None` should only be /// passed once (in the initial call) for any given `maybe_prefix` value. Subsequent calls /// operating on the same prefix should always pass `Some`, and this should be equal to the - /// previous call result's `maybe_prefix` field. + /// previous call result's `maybe_cursor` field. /// - /// Returns [`ClearPrefixResult`] to inform about the result. Once the resultant `maybe_prefix` + /// Returns [`ClearPrefixResult`] to inform about the result. Once the resultant `maybe_cursor` /// field is `None`, then no further items remain to be deleted. /// /// NOTE: After the initial call for any given prefix, it is important that no keys further @@ -246,13 +257,12 @@ pub trait Storage { maybe_limit: Option, maybe_cursor: Option>, //< TODO Make work or just Option>? ) -> ClearPrefixResult { - let (maybe_cursor, db, total, loops) = Externalities::clear_prefix( + Externalities::clear_prefix( *self, maybe_prefix, maybe_limit, maybe_cursor.as_ref().map(|x| &x[..]), - ); - ClearPrefixResult { db, total, loops, maybe_cursor } + ).into() } /// Append the encoded `value` to the storage item at `key`. @@ -405,8 +415,8 @@ pub trait DefaultChildStorage { #[version(2)] fn storage_kill(&mut self, storage_key: &[u8], limit: Option) -> bool { let child_info = ChildInfo::new_default(storage_key); - let (maybe_cursor, ..) = self.kill_child_storage(&child_info, limit, None); - maybe_cursor.is_none() + let r = self.kill_child_storage(&child_info, limit, None); + r.maybe_cursor.is_none() } /// Clear a child storage key. @@ -415,10 +425,10 @@ pub trait DefaultChildStorage { #[version(3)] fn storage_kill(&mut self, storage_key: &[u8], limit: Option) -> KillStorageResult { let child_info = ChildInfo::new_default(storage_key); - let (maybe_cursor, num_removed, ..) = self.kill_child_storage(&child_info, limit, None); - match maybe_cursor { - None => KillStorageResult::AllRemoved(num_removed), - Some(..) => KillStorageResult::SomeRemaining(num_removed), + let r = self.kill_child_storage(&child_info, limit, None); + match r.maybe_cursor { + None => KillStorageResult::AllRemoved(r.loops), + Some(..) => KillStorageResult::SomeRemaining(r.loops), } } @@ -433,12 +443,11 @@ pub trait DefaultChildStorage { maybe_cursor: Option>, ) -> ClearPrefixResult { let child_info = ChildInfo::new_default(storage_key); - let (maybe_cursor, db, total, loops) = self.kill_child_storage( + self.kill_child_storage( &child_info, maybe_limit, maybe_cursor.as_ref().map(|x| &x[..]), - ); - ClearPrefixResult { maybe_cursor, db, total, loops } + ).into() } /// Check a child storage key. @@ -468,11 +477,10 @@ pub trait DefaultChildStorage { limit: Option, ) -> KillStorageResult { let child_info = ChildInfo::new_default(storage_key); - let (maybe_cursor, num_removed, ..) = - self.clear_child_prefix(&child_info, prefix, limit, None); - match maybe_cursor { - None => KillStorageResult::AllRemoved(num_removed), - Some(..) => KillStorageResult::SomeRemaining(num_removed), + let r = self.clear_child_prefix(&child_info, prefix, limit, None); + match r.maybe_cursor { + None => KillStorageResult::AllRemoved(r.loops), + Some(..) => KillStorageResult::SomeRemaining(r.loops), } } @@ -488,13 +496,12 @@ pub trait DefaultChildStorage { maybe_cursor: Option>, ) -> ClearPrefixResult { let child_info = ChildInfo::new_default(storage_key); - let (maybe_cursor, db, total, loops) = self.clear_child_prefix( + self.clear_child_prefix( &child_info, prefix, maybe_limit, maybe_cursor.as_ref().map(|x| &x[..]), - ); - ClearPrefixResult { db, total, maybe_cursor, loops } + ).into() } /// Default child root calculation. diff --git a/primitives/state-machine/src/basic.rs b/primitives/state-machine/src/basic.rs index 267a830c77ec2..706d648f9d984 100644 --- a/primitives/state-machine/src/basic.rs +++ b/primitives/state-machine/src/basic.rs @@ -29,7 +29,7 @@ use sp_core::{ traits::Externalities, Blake2Hasher, }; -use sp_externalities::{Extension, Extensions}; +use sp_externalities::{Extension, Extensions, MultiRemovalResults}; use sp_trie::{empty_child_trie_root, HashKey, LayoutV0, LayoutV1, TrieConfiguration}; use std::{ any::{Any, TypeId}, @@ -214,14 +214,14 @@ impl Externalities for BasicExternalities { child_info: &ChildInfo, _maybe_limit: Option, _maybe_cursor: Option<&[u8]>, - ) -> (Option>, u32, u32, u32) { - let num_removed = self + ) -> MultiRemovalResults { + let count = self .inner .children_default .remove(child_info.storage_key()) .map(|c| c.data.len()) .unwrap_or(0) as u32; - (None, num_removed, num_removed, num_removed) + MultiRemovalResults { maybe_cursor: None, backend: count, unique: count, loops: count } } fn clear_prefix( @@ -229,13 +229,14 @@ impl Externalities for BasicExternalities { prefix: &[u8], _maybe_limit: Option, _maybe_cursor: Option<&[u8]>, - ) -> (Option>, u32, u32, u32) { + ) -> MultiRemovalResults { if is_child_storage_key(prefix) { warn!( target: "trie", "Refuse to clear prefix that is part of child storage key via main storage" ); - return (Some(prefix.to_vec()), 0, 0, 0) + let maybe_cursor = Some(prefix.to_vec()); + return MultiRemovalResults { maybe_cursor, backend: 0, unique: 0, loops: 0 } } let to_remove = self @@ -247,11 +248,11 @@ impl Externalities for BasicExternalities { .cloned() .collect::>(); - let num_removed = to_remove.len() as u32; + let count = to_remove.len() as u32; for key in to_remove { self.inner.top.remove(&key); } - (None, num_removed, num_removed, num_removed) + MultiRemovalResults { maybe_cursor: None, backend: count, unique: count, loops: count } } fn clear_child_prefix( @@ -260,7 +261,7 @@ impl Externalities for BasicExternalities { prefix: &[u8], _maybe_limit: Option, _maybe_cursor: Option<&[u8]>, - ) -> (Option>, u32, u32, u32) { + ) -> MultiRemovalResults { if let Some(child) = self.inner.children_default.get_mut(child_info.storage_key()) { let to_remove = child .data @@ -270,13 +271,13 @@ impl Externalities for BasicExternalities { .cloned() .collect::>(); - let num_removed = to_remove.len() as u32; + let count = to_remove.len() as u32; for key in to_remove { child.data.remove(&key); } - (None, num_removed, num_removed, num_removed) + MultiRemovalResults { maybe_cursor: None, backend: count, unique: count, loops: count } } else { - (None, 0, 0, 0) + MultiRemovalResults { maybe_cursor: None, backend: 0, unique: 0, loops: 0 } } } @@ -468,7 +469,7 @@ mod tests { }); let res = ext.kill_child_storage(child_info, None, None); - assert_eq!(res, (None, 3, 3, 3)); + assert_eq!(res.decon(), (None, 3, 3, 3)); } #[test] diff --git a/primitives/state-machine/src/ext.rs b/primitives/state-machine/src/ext.rs index b241365480fd9..14275aa90437f 100644 --- a/primitives/state-machine/src/ext.rs +++ b/primitives/state-machine/src/ext.rs @@ -27,7 +27,7 @@ use sp_core::hexdisplay::HexDisplay; use sp_core::storage::{ well_known_keys::is_child_storage_key, ChildInfo, StateVersion, TrackedStorageKey, }; -use sp_externalities::{Extension, ExtensionStore, Externalities}; +use sp_externalities::{Extension, ExtensionStore, Externalities, MultiRemovalResults}; use sp_trie::{empty_child_trie_root, LayoutV1}; use crate::{log_error, trace, warn, StorageTransactionCache}; @@ -438,7 +438,7 @@ where child_info: &ChildInfo, maybe_limit: Option, maybe_cursor: Option<&[u8]>, - ) -> (Option>, u32, u32, u32) { + ) -> MultiRemovalResults { trace!( target: "state", method = "ChildKill", @@ -447,10 +447,10 @@ where ); let _guard = guard(); self.mark_dirty(); - let overlay_count = self.overlay.clear_child_storage(child_info); - let (next_cursor, backend_count, iterations) = + let overlay = self.overlay.clear_child_storage(child_info); + let (maybe_cursor, backend, loops) = self.limit_remove_from_backend(Some(child_info), None, maybe_limit, maybe_cursor); - (next_cursor, backend_count, overlay_count + backend_count, iterations) + MultiRemovalResults { maybe_cursor, backend, unique: overlay + backend, loops } } fn clear_prefix( @@ -458,7 +458,7 @@ where prefix: &[u8], maybe_limit: Option, maybe_cursor: Option<&[u8]>, - ) -> (Option>, u32, u32, u32) { + ) -> MultiRemovalResults { trace!( target: "state", method = "ClearPrefix", @@ -472,14 +472,14 @@ where target: "trie", "Refuse to directly clear prefix that is part or contains of child storage key", ); - return (None, 0, 0, 0) + return MultiRemovalResults { maybe_cursor: None, backend: 0, unique: 0, loops: 0 } } self.mark_dirty(); - let overlay_count = self.overlay.clear_prefix(prefix); - let (next_cursor, backend_count, iterations) = + let overlay = self.overlay.clear_prefix(prefix); + let (maybe_cursor, backend, loops) = self.limit_remove_from_backend(None, Some(prefix), maybe_limit, maybe_cursor); - (next_cursor, backend_count, overlay_count + backend_count, iterations) + MultiRemovalResults { maybe_cursor, backend, unique: overlay + backend, loops } } fn clear_child_prefix( @@ -488,7 +488,7 @@ where prefix: &[u8], maybe_limit: Option, maybe_cursor: Option<&[u8]>, - ) -> (Option>, u32, u32, u32) { + ) -> MultiRemovalResults { trace!( target: "state", method = "ChildClearPrefix", @@ -499,14 +499,14 @@ where let _guard = guard(); self.mark_dirty(); - let overlay_count = self.overlay.clear_child_prefix(child_info, prefix); - let (next_cursor, backend_count, iterations) = self.limit_remove_from_backend( + let overlay = self.overlay.clear_child_prefix(child_info, prefix); + let (maybe_cursor, backend, loops) = self.limit_remove_from_backend( Some(child_info), Some(prefix), maybe_limit, maybe_cursor, ); - (next_cursor, backend_count, backend_count + overlay_count, iterations) + MultiRemovalResults { maybe_cursor, backend, unique: overlay + backend, loops } } fn storage_append(&mut self, key: Vec, value: Vec) { @@ -768,7 +768,7 @@ where None => self.overlay.storage(key), }; if !matches!(overlay, Some(None)) { - // already pending deletion from the backend - no need to delete it again. + // not pending deletion from the backend - delete it. if let Some(child_info) = maybe_child { self.overlay.set_child_storage(child_info, key.to_vec(), None); } else { diff --git a/primitives/state-machine/src/lib.rs b/primitives/state-machine/src/lib.rs index 3db2af3b95cd3..b15b7ac940836 100644 --- a/primitives/state-machine/src/lib.rs +++ b/primitives/state-machine/src/lib.rs @@ -1597,7 +1597,7 @@ mod tests { { let mut cache = StorageTransactionCache::default(); let mut ext = Ext::new(&mut overlay, &mut cache, backend, None); - assert_matches!(ext.clear_prefix(b"ab", Some(1), None), (Some(_), 1, 3, 1)); + assert_matches!(ext.clear_prefix(b"ab", Some(1), None).decon(), (Some(_), 1, 3, 1)); } overlay.commit_transaction().unwrap(); @@ -1639,7 +1639,8 @@ mod tests { { let mut cache = StorageTransactionCache::default(); let mut ext = Ext::new(&mut overlay, &mut cache, &backend, None); - assert_matches!(ext.kill_child_storage(&child_info, Some(2), None), (Some(_), 2, 6, 2)); + let r = ext.kill_child_storage(&child_info, Some(2), None); + assert_matches!(r.decon(), (Some(_), 2, 6, 2)); } assert_eq!( @@ -1674,14 +1675,14 @@ mod tests { let mut overlay = OverlayedChanges::default(); let mut cache = StorageTransactionCache::default(); let mut ext = Ext::new(&mut overlay, &mut cache, &backend, None); - let r = ext.kill_child_storage(&child_info, Some(0), None); + let r = ext.kill_child_storage(&child_info, Some(0), None).decon(); assert_matches!(r, (Some(_), 0, 0, 0)); - let r = ext.kill_child_storage(&child_info, Some(1), r.0.as_ref().map(|x| &x[..])); + let r = ext.kill_child_storage(&child_info, Some(1), r.0.as_ref().map(|x| &x[..])).decon(); assert_matches!(r, (Some(_), 1, 1, 1)); - let r = ext.kill_child_storage(&child_info, Some(4), r.0.as_ref().map(|x| &x[..])); + let r = ext.kill_child_storage(&child_info, Some(4), r.0.as_ref().map(|x| &x[..])).decon(); // Only 3 items remaining to remove assert_matches!(r, (None, 3, 3, 3)); - let r = ext.kill_child_storage(&child_info, Some(1), None); + let r = ext.kill_child_storage(&child_info, Some(1), None).decon(); assert_matches!(r, (Some(_), 0, 0, 1)); } @@ -1700,7 +1701,7 @@ mod tests { let mut overlay = OverlayedChanges::default(); let mut cache = StorageTransactionCache::default(); let mut ext = Ext::new(&mut overlay, &mut cache, &backend, None); - assert_eq!(ext.kill_child_storage(&child_info, None, None), (None, 4, 4, 4)); + assert_eq!(ext.kill_child_storage(&child_info, None, None).decon(), (None, 4, 4, 4)); } #[test] diff --git a/primitives/state-machine/src/read_only.rs b/primitives/state-machine/src/read_only.rs index 1a84c1c7af8f5..622915a2d0525 100644 --- a/primitives/state-machine/src/read_only.rs +++ b/primitives/state-machine/src/read_only.rs @@ -25,6 +25,7 @@ use sp_core::{ traits::Externalities, Blake2Hasher, }; +use sp_externalities::MultiRemovalResults; use std::{ any::{Any, TypeId}, marker::PhantomData, @@ -129,7 +130,7 @@ impl<'a, H: Hasher, B: 'a + Backend> Externalities for ReadOnlyExternalities< _child_info: &ChildInfo, _maybe_limit: Option, _maybe_cursor: Option<&[u8]>, - ) -> (Option>, u32, u32, u32) { + ) -> MultiRemovalResults { unimplemented!("kill_child_storage is not supported in ReadOnlyExternalities") } @@ -138,7 +139,7 @@ impl<'a, H: Hasher, B: 'a + Backend> Externalities for ReadOnlyExternalities< _prefix: &[u8], _maybe_limit: Option, _maybe_cursor: Option<&[u8]>, - ) -> (Option>, u32, u32, u32) { + ) -> MultiRemovalResults { unimplemented!("clear_prefix is not supported in ReadOnlyExternalities") } @@ -148,7 +149,7 @@ impl<'a, H: Hasher, B: 'a + Backend> Externalities for ReadOnlyExternalities< _prefix: &[u8], _maybe_limit: Option, _maybe_cursor: Option<&[u8]>, - ) -> (Option>, u32, u32, u32) { + ) -> MultiRemovalResults { unimplemented!("clear_child_prefix is not supported in ReadOnlyExternalities") } diff --git a/primitives/state-machine/src/testing.rs b/primitives/state-machine/src/testing.rs index c09cecc614621..57d4f0b4898eb 100644 --- a/primitives/state-machine/src/testing.rs +++ b/primitives/state-machine/src/testing.rs @@ -382,7 +382,7 @@ mod tests { let mut ext = ext.ext(); assert!( - ext.kill_child_storage(&child_info, Some(2), None).0.is_some(), + ext.kill_child_storage(&child_info, Some(2), None).maybe_cursor.is_some(), "Should not delete all keys" ); diff --git a/primitives/tasks/src/async_externalities.rs b/primitives/tasks/src/async_externalities.rs index 852be406eecae..85c6339008bad 100644 --- a/primitives/tasks/src/async_externalities.rs +++ b/primitives/tasks/src/async_externalities.rs @@ -110,7 +110,7 @@ impl Externalities for AsyncExternalities { _child_info: &ChildInfo, _maybe_limit: Option, _maybe_cursor: Option<&[u8]>, - ) -> (Option>, u32, u32, u32) { + ) -> MultiRemovalResults { panic!("`kill_child_storage`: should not be used in async externalities!") } @@ -119,7 +119,7 @@ impl Externalities for AsyncExternalities { _prefix: &[u8], _maybe_limit: Option, _maybe_cursor: Option<&[u8]>, - ) -> (Option>, u32, u32, u32) { + ) -> MultiRemovalResults { panic!("`clear_prefix`: should not be used in async externalities!") } @@ -129,7 +129,7 @@ impl Externalities for AsyncExternalities { _prefix: &[u8], _maybe_limit: Option, _maybe_cursor: Option<&[u8]>, - ) -> (Option>, u32, u32, u32) { + ) -> MultiRemovalResults { panic!("`clear_child_prefix`: should not be used in async externalities!") } From 1fb2ef5d7cba2be1b3cfd8b49d7b32a41b9e780f Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Wed, 25 May 2022 12:44:18 +0200 Subject: [PATCH 28/38] Fixes --- primitives/io/Cargo.toml | 4 ++-- primitives/tasks/src/async_externalities.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/primitives/io/Cargo.toml b/primitives/io/Cargo.toml index bd288dd896d1b..09a087d509416 100644 --- a/primitives/io/Cargo.toml +++ b/primitives/io/Cargo.toml @@ -25,7 +25,7 @@ sp-state-machine = { version = "0.12.0", optional = true, path = "../state-machi sp-wasm-interface = { version = "6.0.0", path = "../wasm-interface", default-features = false } sp-runtime-interface = { version = "6.0.0", default-features = false, path = "../runtime-interface" } sp-trie = { version = "6.0.0", optional = true, path = "../trie" } -sp-externalities = { version = "0.12.0", optional = true, path = "../externalities" } +sp-externalities = { version = "0.12.0", default-features = false, path = "../externalities" } sp-tracing = { version = "5.0.0", default-features = false, path = "../tracing" } log = { version = "0.4.17", optional = true } futures = { version = "0.3.21", features = ["thread-pool"], optional = true } @@ -37,6 +37,7 @@ tracing-core = { version = "0.1.26", default-features = false} [features] default = ["std"] std = [ + "sp-externalities/std", "sp-core/std", "sp-keystore", "codec/std", @@ -47,7 +48,6 @@ std = [ "libsecp256k1", "secp256k1", "sp-runtime-interface/std", - "sp-externalities", "sp-wasm-interface/std", "sp-tracing/std", "tracing/std", diff --git a/primitives/tasks/src/async_externalities.rs b/primitives/tasks/src/async_externalities.rs index 85c6339008bad..008955a714b21 100644 --- a/primitives/tasks/src/async_externalities.rs +++ b/primitives/tasks/src/async_externalities.rs @@ -22,7 +22,7 @@ use sp_core::{ storage::{ChildInfo, StateVersion, TrackedStorageKey}, traits::{Externalities, RuntimeSpawn, RuntimeSpawnExt, SpawnNamed, TaskExecutorExt}, }; -use sp_externalities::{Extensions, ExternalitiesExt as _}; +use sp_externalities::{Extensions, ExternalitiesExt as _, MultiRemovalResults}; use std::any::{Any, TypeId}; /// Simple state-less externalities for use in async context. From 0338fce7fcd5cd92044b989769b1bafafd39c48f Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Wed, 25 May 2022 12:14:23 +0100 Subject: [PATCH 29/38] Docs --- frame/support/src/storage/migration.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frame/support/src/storage/migration.rs b/frame/support/src/storage/migration.rs index 5a6e88f94f3c7..26f059f997f05 100644 --- a/frame/support/src/storage/migration.rs +++ b/frame/support/src/storage/migration.rs @@ -269,8 +269,9 @@ pub fn remove_storage_prefix(module: &[u8], item: &[u8], hash: &[u8]) { /// the key `hash`. /// /// All values in the client overlay will be deleted, if `maybe_limit` is `Some` then up to -/// that number of values are deleted from the client backend, otherwise all values in the -/// client backend are deleted. +/// that number of values are deleted from the client backend by seeking and reading that number of +/// storage values plus one. If `maybe_limit` is `None` then all values in the client backend are +/// deleted. This is potentially unsafe since it's an unbounded operation. /// /// ## Cursors /// From 73d1fc6ff617cb0cb04a346794c145d768663510 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 25 May 2022 13:58:25 +0200 Subject: [PATCH 30/38] Some nitpicks :see_no_evil: --- frame/support/src/lib.rs | 4 +- frame/support/src/storage/child.rs | 12 ++-- .../src/storage/generator/double_map.rs | 2 +- frame/support/src/storage/generator/nmap.rs | 2 +- frame/support/src/storage/migration.rs | 2 +- frame/support/src/storage/mod.rs | 14 ++-- .../support/src/storage/types/counted_map.rs | 10 +-- frame/support/src/storage/types/double_map.rs | 12 ++-- frame/support/src/storage/types/map.rs | 6 +- frame/support/src/storage/types/nmap.rs | 12 ++-- frame/support/src/storage/unhashed.rs | 10 +-- primitives/externalities/src/lib.rs | 2 + primitives/io/src/lib.rs | 65 ++++++------------- primitives/runtime-interface/Cargo.toml | 4 +- primitives/runtime-interface/src/impls.rs | 4 ++ primitives/state-machine/src/lib.rs | 8 ++- 16 files changed, 76 insertions(+), 93 deletions(-) diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 5c64f90ef23fb..85e88292f041a 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -1101,8 +1101,8 @@ pub mod tests { // Note this is the incorrect answer (for now), since we are using v2 of // `clear_prefix`. // When we switch to v3, then this will become: - // sp_io::ClearPrefixResult::NoneLeft { db: 0, total: 2 }, - sp_io::ClearPrefixResult { maybe_cursor: None, db: 0, total: 0, loops: 0 }, + // sp_io::MultiRemovalResults::NoneLeft { db: 0, total: 2 }, + sp_io::MultiRemovalResults { maybe_cursor: None, backend: 0, unique: 0, loops: 0 }, )); assert_eq!(DoubleMap::get(&key1, &key2), 0u64); assert_eq!(DoubleMap::get(&key1, &(key2 + 1)), 0u64); diff --git a/frame/support/src/storage/child.rs b/frame/support/src/storage/child.rs index 28f04ad7eb9ec..397daaa82a677 100644 --- a/frame/support/src/storage/child.rs +++ b/frame/support/src/storage/child.rs @@ -21,7 +21,7 @@ // NOTE: could replace unhashed by having only one kind of storage (top trie being the child info // of null length parent storage key). -pub use crate::sp_io::{ClearPrefixResult, KillStorageResult}; +pub use crate::sp_io::{KillStorageResult, MultiRemovalResults}; use crate::sp_std::prelude::*; use codec::{Codec, Decode, Encode}; pub use sp_core::storage::{ChildInfo, ChildType, StateVersion}; @@ -166,8 +166,8 @@ pub fn kill_storage(child_info: &ChildInfo, limit: Option) -> KillStorageRe /// cursor need not be passed in an a `None` may be passed instead. This exception may be useful /// then making this call solely from a block-hook such as `on_initialize`. /// -/// Returns [`ClearPrefixResult`] to inform about the result. Once the resultant `maybe_cursor` -/// field is `None`, then no further items remain to be deleted. +/// Returns [`MultiRemovalResults`](sp_io::MultiRemovalResults) to inform about the result. Once the +/// resultant `maybe_cursor` field is `None`, then no further items remain to be deleted. /// /// NOTE: After the initial call for any given child storage, it is important that no keys further /// keys are inserted. If so, then they may or may not be deleted by subsequent calls. @@ -180,7 +180,7 @@ pub fn clear_storage( child_info: &ChildInfo, maybe_limit: Option, _maybe_cursor: Option<&[u8]>, -) -> ClearPrefixResult { +) -> MultiRemovalResults { // TODO: Once the network has upgraded to include the new host functions, this code can be // enabled. // sp_io::default_child_storage::storage_kill(prefix, maybe_limit, maybe_cursor) @@ -189,11 +189,11 @@ pub fn clear_storage( sp_io::default_child_storage::storage_kill(child_info.storage_key(), maybe_limit), }; use sp_io::KillStorageResult::*; - let (maybe_cursor, db) = match r { + let (maybe_cursor, backend) = match r { AllRemoved(db) => (None, db), SomeRemaining(db) => (Some(child_info.storage_key().to_vec()), db), }; - ClearPrefixResult { maybe_cursor, db, total: db, loops: db } + MultiRemovalResults { maybe_cursor, backend, unique: backend, loops: backend } } /// Ensure `key` has no explicit entry in storage. diff --git a/frame/support/src/storage/generator/double_map.rs b/frame/support/src/storage/generator/double_map.rs index 99cd305a319e0..90ec92c622b1f 100644 --- a/frame/support/src/storage/generator/double_map.rs +++ b/frame/support/src/storage/generator/double_map.rs @@ -214,7 +214,7 @@ where k1: KArg1, limit: u32, maybe_cursor: Option<&[u8]>, - ) -> sp_io::ClearPrefixResult + ) -> sp_io::MultiRemovalResults where KArg1: EncodeLike, { diff --git a/frame/support/src/storage/generator/nmap.rs b/frame/support/src/storage/generator/nmap.rs index d127ca4b07cf2..0175d681df1d4 100755 --- a/frame/support/src/storage/generator/nmap.rs +++ b/frame/support/src/storage/generator/nmap.rs @@ -190,7 +190,7 @@ where partial_key: KP, limit: u32, maybe_cursor: Option<&[u8]>, - ) -> sp_io::ClearPrefixResult + ) -> sp_io::MultiRemovalResults where K: HasKeyPrefix, { diff --git a/frame/support/src/storage/migration.rs b/frame/support/src/storage/migration.rs index 26f059f997f05..67001fc4e1f42 100644 --- a/frame/support/src/storage/migration.rs +++ b/frame/support/src/storage/migration.rs @@ -287,7 +287,7 @@ pub fn clear_storage_prefix( hash: &[u8], maybe_limit: Option, maybe_cursor: Option<&[u8]>, -) -> sp_io::ClearPrefixResult { +) -> sp_io::MultiRemovalResults { let mut key = vec![0u8; 32 + hash.len()]; let storage_prefix = storage_prefix(module, item); key[0..32].copy_from_slice(&storage_prefix); diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index 954dd69a3db8f..c9b4cf75f8e3b 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -538,7 +538,7 @@ pub trait StorageDoubleMap { k1: KArg1, limit: u32, maybe_cursor: Option<&[u8]>, - ) -> sp_io::ClearPrefixResult + ) -> sp_io::MultiRemovalResults where KArg1: ?Sized + EncodeLike; @@ -686,8 +686,8 @@ pub trait StorageNMap { /// Attempt to remove items from the map matching a `partial_key` prefix. /// - /// Returns [`ClearPrefixResult`] to inform about the result. Once the resultant `maybe_cursor` - /// field is `None`, then no further items remain to be deleted. + /// Returns [`MultiRemovalResults`](sp_io::MultiRemovalResults) to inform about the result. Once + /// the resultant `maybe_cursor` field is `None`, then no further items remain to be deleted. /// /// NOTE: After the initial call for any given map, it is important that no further items /// are inserted into the map which match the `partial key`. If so, then the map may not be @@ -711,7 +711,7 @@ pub trait StorageNMap { partial_key: KP, limit: u32, maybe_cursor: Option<&[u8]>, - ) -> sp_io::ClearPrefixResult + ) -> sp_io::MultiRemovalResults where K: HasKeyPrefix; @@ -1172,8 +1172,8 @@ pub trait StoragePrefixedMap { /// Attempt to remove all items from the map. /// - /// Returns [`ClearPrefixResult`] to inform about the result. Once the resultant `maybe_cursor` - /// field is `None`, then no further items remain to be deleted. + /// Returns [`MultiRemovalResults`](sp_io::MultiRemovalResults) to inform about the result. Once + /// the resultant `maybe_cursor` field is `None`, then no further items remain to be deleted. /// /// NOTE: After the initial call for any given map, it is important that no further items /// are inserted into the map. If so, then the map may not be empty when the resultant @@ -1193,7 +1193,7 @@ pub trait StoragePrefixedMap { /// passed once (in the initial call) for any given storage map. Subsequent calls /// operating on the same map should always pass `Some`, and this should be equal to the /// previous call result's `maybe_cursor` field. - fn clear(limit: u32, maybe_cursor: Option<&[u8]>) -> sp_io::ClearPrefixResult { + fn clear(limit: u32, maybe_cursor: Option<&[u8]>) -> sp_io::MultiRemovalResults { unhashed::clear_prefix(&Self::final_prefix(), Some(limit), maybe_cursor) } diff --git a/frame/support/src/storage/types/counted_map.rs b/frame/support/src/storage/types/counted_map.rs index 9ae3d3c575870..44fcbf607935f 100644 --- a/frame/support/src/storage/types/counted_map.rs +++ b/frame/support/src/storage/types/counted_map.rs @@ -31,7 +31,7 @@ use crate::{ Never, }; use codec::{Decode, Encode, EncodeLike, FullCodec, MaxEncodedLen, Ref}; -use sp_io::ClearPrefixResult; +use sp_io::MultiRemovalResults; use sp_runtime::traits::Saturating; use sp_std::prelude::*; @@ -284,8 +284,8 @@ where /// Attempt to remove all items from the map. /// - /// Returns [`ClearPrefixResult`] to inform about the result. Once the resultant `maybe_cursor` - /// field is `None`, then no further items remain to be deleted. + /// Returns [`MultiRemovalResults`](sp_io::MultiRemovalResults) to inform about the result. Once + /// the resultant `maybe_cursor` field is `None`, then no further items remain to be deleted. /// /// NOTE: After the initial call for any given map, it is important that no further items /// are inserted into the map. If so, then the map may not be empty when the resultant @@ -305,11 +305,11 @@ where /// passed once (in the initial call) for any given storage map. Subsequent calls /// operating on the same map should always pass `Some`, and this should be equal to the /// previous call result's `maybe_cursor` field. - pub fn clear(limit: u32, maybe_cursor: Option<&[u8]>) -> ClearPrefixResult { + pub fn clear(limit: u32, maybe_cursor: Option<&[u8]>) -> MultiRemovalResults { let result = ::Map::clear(limit, maybe_cursor); match result.maybe_cursor { None => CounterFor::::kill(), - Some(_) => CounterFor::::mutate(|x| x.saturating_reduce(result.total)), + Some(_) => CounterFor::::mutate(|x| x.saturating_reduce(result.unique)), } result } diff --git a/frame/support/src/storage/types/double_map.rs b/frame/support/src/storage/types/double_map.rs index 6a36d8ce3e3b9..5662087cc896f 100644 --- a/frame/support/src/storage/types/double_map.rs +++ b/frame/support/src/storage/types/double_map.rs @@ -240,8 +240,8 @@ where /// Attempt to remove items from the map matching a `first_key` prefix. /// - /// Returns [`ClearPrefixResult`] to inform about the result. Once the resultant `maybe_cursor` - /// field is `None`, then no further items remain to be deleted. + /// Returns [`MultiRemovalResults`](sp_io::MultiRemovalResults) to inform about the result. Once + /// the resultant `maybe_cursor` field is `None`, then no further items remain to be deleted. /// /// NOTE: After the initial call for any given map, it is important that no further items /// are inserted into the map which match the `first_key`. If so, then the map may not be @@ -265,7 +265,7 @@ where first_key: KArg1, limit: u32, maybe_cursor: Option<&[u8]>, - ) -> sp_io::ClearPrefixResult + ) -> sp_io::MultiRemovalResults where KArg1: ?Sized + EncodeLike, { @@ -407,8 +407,8 @@ where /// Attempt to remove all items from the map. /// - /// Returns [`ClearPrefixResult`] to inform about the result. Once the resultant `maybe_cursor` - /// field is `None`, then no further items remain to be deleted. + /// Returns [`MultiRemovalResults`](sp_io::MultiRemovalResults) to inform about the result. Once + /// the resultant `maybe_cursor` field is `None`, then no further items remain to be deleted. /// /// NOTE: After the initial call for any given map, it is important that no further items /// are inserted into the map. If so, then the map may not be empty when the resultant @@ -428,7 +428,7 @@ where /// passed once (in the initial call) for any given storage map. Subsequent calls /// operating on the same map should always pass `Some`, and this should be equal to the /// previous call result's `maybe_cursor` field. - pub fn clear(limit: u32, maybe_cursor: Option<&[u8]>) -> sp_io::ClearPrefixResult { + pub fn clear(limit: u32, maybe_cursor: Option<&[u8]>) -> sp_io::MultiRemovalResults { >::clear(limit, maybe_cursor) } diff --git a/frame/support/src/storage/types/map.rs b/frame/support/src/storage/types/map.rs index 53b5f5f7467a1..6dac5b3756598 100644 --- a/frame/support/src/storage/types/map.rs +++ b/frame/support/src/storage/types/map.rs @@ -255,8 +255,8 @@ where /// Attempt to remove all items from the map. /// - /// Returns [`ClearPrefixResult`] to inform about the result. Once the resultant `maybe_cursor` - /// field is `None`, then no further items remain to be deleted. + /// Returns [`MultiRemovalResults`](sp_io::MultiRemovalResults) to inform about the result. Once + /// the resultant `maybe_cursor` field is `None`, then no further items remain to be deleted. /// /// NOTE: After the initial call for any given map, it is important that no further items /// are inserted into the map. If so, then the map may not be empty when the resultant @@ -276,7 +276,7 @@ where /// passed once (in the initial call) for any given storage map. Subsequent calls /// operating on the same map should always pass `Some`, and this should be equal to the /// previous call result's `maybe_cursor` field. - pub fn clear(limit: u32, maybe_cursor: Option<&[u8]>) -> sp_io::ClearPrefixResult { + pub fn clear(limit: u32, maybe_cursor: Option<&[u8]>) -> sp_io::MultiRemovalResults { >::clear(limit, maybe_cursor) } diff --git a/frame/support/src/storage/types/nmap.rs b/frame/support/src/storage/types/nmap.rs index d049e25c6bd8a..1e7e2c9734f20 100755 --- a/frame/support/src/storage/types/nmap.rs +++ b/frame/support/src/storage/types/nmap.rs @@ -196,8 +196,8 @@ where /// Attempt to remove items from the map matching a `partial_key` prefix. /// - /// Returns [`ClearPrefixResult`] to inform about the result. Once the resultant `maybe_cursor` - /// field is `None`, then no further items remain to be deleted. + /// Returns [`MultiRemovalResults`](sp_io::MultiRemovalResults) to inform about the result. Once + /// the resultant `maybe_cursor` field is `None`, then no further items remain to be deleted. /// /// NOTE: After the initial call for any given map, it is important that no further items /// are inserted into the map which match the `partial key`. If so, then the map may not be @@ -221,7 +221,7 @@ where partial_key: KP, limit: u32, maybe_cursor: Option<&[u8]>, - ) -> sp_io::ClearPrefixResult + ) -> sp_io::MultiRemovalResults where Key: HasKeyPrefix, { @@ -347,8 +347,8 @@ where /// Attempt to remove all items from the map. /// - /// Returns [`ClearPrefixResult`] to inform about the result. Once the resultant `maybe_cursor` - /// field is `None`, then no further items remain to be deleted. + /// Returns [`MultiRemovalResults`](sp_io::MultiRemovalResults) to inform about the result. Once + /// the resultant `maybe_cursor` field is `None`, then no further items remain to be deleted. /// /// NOTE: After the initial call for any given map, it is important that no further items /// are inserted into the map. If so, then the map may not be empty when the resultant @@ -368,7 +368,7 @@ where /// passed once (in the initial call) for any given storage map. Subsequent calls /// operating on the same map should always pass `Some`, and this should be equal to the /// previous call result's `maybe_cursor` field. - pub fn clear(limit: u32, maybe_cursor: Option<&[u8]>) -> sp_io::ClearPrefixResult { + pub fn clear(limit: u32, maybe_cursor: Option<&[u8]>) -> sp_io::MultiRemovalResults { >::clear(limit, maybe_cursor) } diff --git a/frame/support/src/storage/unhashed.rs b/frame/support/src/storage/unhashed.rs index a2a4a267b739d..fc6d8ae79c57d 100644 --- a/frame/support/src/storage/unhashed.rs +++ b/frame/support/src/storage/unhashed.rs @@ -126,8 +126,8 @@ pub fn kill_prefix(prefix: &[u8], limit: Option) -> sp_io::KillStorageResul /// cursor need not be passed in an a `None` may be passed instead. This exception may be useful /// then making this call solely from a block-hook such as `on_initialize`. /// -/// Returns [`ClearPrefixResult`] to inform about the result. Once the resultant `maybe_cursor` -/// field is `None`, then no further items remain to be deleted. +/// Returns [`MultiRemovalResults`](sp_io::MultiRemovalResults) to inform about the result. Once the +/// resultant `maybe_cursor` field is `None`, then no further items remain to be deleted. /// /// NOTE: After the initial call for any given child storage, it is important that no keys further /// keys are inserted. If so, then they may or may not be deleted by subsequent calls. @@ -140,17 +140,17 @@ pub fn clear_prefix( prefix: &[u8], maybe_limit: Option, _maybe_cursor: Option<&[u8]>, -) -> sp_io::ClearPrefixResult { +) -> sp_io::MultiRemovalResults { // TODO: Once the network has upgraded to include the new host functions, this code can be // enabled. // sp_io::storage::clear_prefix(prefix, maybe_limit, maybe_cursor) - use sp_io::{ClearPrefixResult, KillStorageResult::*}; + use sp_io::{KillStorageResult::*, MultiRemovalResults}; #[allow(deprecated)] let (maybe_cursor, i) = match kill_prefix(prefix, maybe_limit) { AllRemoved(i) => (None, i), SomeRemaining(i) => (Some(prefix.to_vec()), i), }; - ClearPrefixResult { maybe_cursor, db: i, total: i, loops: i } + MultiRemovalResults { maybe_cursor, backend: i, unique: i, loops: i } } /// Get a Vec of bytes from storage. diff --git a/primitives/externalities/src/lib.rs b/primitives/externalities/src/lib.rs index 7b568bc640190..15474183d532e 100644 --- a/primitives/externalities/src/lib.rs +++ b/primitives/externalities/src/lib.rs @@ -53,6 +53,8 @@ pub enum Error { } /// Results concerning an operation to remove many keys. +#[derive(codec::Encode, codec::Decode)] +#[must_use] pub struct MultiRemovalResults { /// A continuation cursor which, if `Some` must be provided to the subsequent removal call. /// If `None` then all removals are complete and no further calls are needed. diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index d62f4dfa9a3e8..803fae044ccf7 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -44,7 +44,6 @@ use sp_core::{ }; #[cfg(feature = "std")] use sp_keystore::{KeystoreExt, SyncCryptoStore}; -use sp_externalities::MultiRemovalResults; use sp_core::{ crypto::KeyTypeId, @@ -82,6 +81,8 @@ mod batch_verifier; #[cfg(feature = "std")] use batch_verifier::BatchVerifier; +pub use sp_externalities::MultiRemovalResults; + #[cfg(feature = "std")] const LOG_TARGET: &str = "runtime::io"; @@ -106,38 +107,11 @@ pub enum KillStorageResult { SomeRemaining(u32), } -impl From for KillStorageResult { - fn from(r: ClearPrefixResult) -> Self { - match r { - ClearPrefixResult { maybe_cursor: None, db, .. } => Self::AllRemoved(db), - ClearPrefixResult { maybe_cursor: Some(..), db, .. } => Self::SomeRemaining(db), - } - } -} - -/// The outcome of calling `clear_prefix` or some function which uses it. -#[derive(PassByCodec, Encode, Decode)] -#[must_use] -pub struct ClearPrefixResult { - /// The number of keys removed from persistent storage. This is what is relevant for weight - /// calculation. - pub db: u32, - /// The number of keys removed in total (including from non-persistent storage). - pub total: u32, - /// The number of backend iterations done for this operation. - pub loops: u32, - /// `None` if all keys to be removed were removed; `Some` if some keys are remaining, in which - /// case, the following call should pass this value in as the cursor. - pub maybe_cursor: Option>, -} - -impl From for ClearPrefixResult { +impl From for KillStorageResult { fn from(r: MultiRemovalResults) -> Self { - Self { - db: r.backend, - total: r.unique, - loops: r.loops, - maybe_cursor: r.maybe_cursor, + match r { + MultiRemovalResults { maybe_cursor: None, backend, .. } => Self::AllRemoved(backend), + MultiRemovalResults { maybe_cursor: Some(..), backend, .. } => Self::SomeRemaining(backend), } } } @@ -239,8 +213,8 @@ pub trait Storage { /// operating on the same prefix should always pass `Some`, and this should be equal to the /// previous call result's `maybe_cursor` field. /// - /// Returns [`ClearPrefixResult`] to inform about the result. Once the resultant `maybe_cursor` - /// field is `None`, then no further items remain to be deleted. + /// Returns [`MultiRemovalResults`](sp_io::MultiRemovalResults) to inform about the result. Once + /// the resultant `maybe_cursor` field is `None`, then no further items remain to be deleted. /// /// NOTE: After the initial call for any given prefix, it is important that no keys further /// keys under the same prefix are inserted. If so, then they may or may not be deleted by @@ -256,13 +230,14 @@ pub trait Storage { maybe_prefix: &[u8], maybe_limit: Option, maybe_cursor: Option>, //< TODO Make work or just Option>? - ) -> ClearPrefixResult { + ) -> MultiRemovalResults { Externalities::clear_prefix( *self, maybe_prefix, maybe_limit, maybe_cursor.as_ref().map(|x| &x[..]), - ).into() + ) + .into() } /// Append the encoded `value` to the storage item at `key`. @@ -441,13 +416,10 @@ pub trait DefaultChildStorage { storage_key: &[u8], maybe_limit: Option, maybe_cursor: Option>, - ) -> ClearPrefixResult { + ) -> MultiRemovalResults { let child_info = ChildInfo::new_default(storage_key); - self.kill_child_storage( - &child_info, - maybe_limit, - maybe_cursor.as_ref().map(|x| &x[..]), - ).into() + self.kill_child_storage(&child_info, maybe_limit, maybe_cursor.as_ref().map(|x| &x[..])) + .into() } /// Check a child storage key. @@ -494,14 +466,15 @@ pub trait DefaultChildStorage { prefix: &[u8], maybe_limit: Option, maybe_cursor: Option>, - ) -> ClearPrefixResult { + ) -> MultiRemovalResults { let child_info = ChildInfo::new_default(storage_key); self.clear_child_prefix( &child_info, prefix, maybe_limit, maybe_cursor.as_ref().map(|x| &x[..]), - ).into() + ) + .into() } /// Default child root calculation. @@ -1881,7 +1854,7 @@ mod tests { // We can switch to this once we enable v3 of the `clear_prefix`. //assert!(matches!( // storage::clear_prefix(b":abc", None), - // ClearPrefixResult::NoneLeft { db: 2, total: 2 } + // MultiRemovalResults::NoneLeft { db: 2, total: 2 } //)); assert!(matches!( storage::clear_prefix(b":abc", None), @@ -1896,7 +1869,7 @@ mod tests { // We can switch to this once we enable v3 of the `clear_prefix`. //assert!(matches!( // storage::clear_prefix(b":abc", None), - // ClearPrefixResult::NoneLeft { db: 0, total: 0 } + // MultiRemovalResults::NoneLeft { db: 0, total: 0 } //)); assert!(matches!( storage::clear_prefix(b":abc", None), diff --git a/primitives/runtime-interface/Cargo.toml b/primitives/runtime-interface/Cargo.toml index c9419f73722c6..8cd31bab559ea 100644 --- a/primitives/runtime-interface/Cargo.toml +++ b/primitives/runtime-interface/Cargo.toml @@ -18,7 +18,7 @@ sp-wasm-interface = { version = "6.0.0", path = "../wasm-interface", default-fea sp-std = { version = "4.0.0", default-features = false, path = "../std" } sp-tracing = { version = "5.0.0", default-features = false, path = "../tracing" } sp-runtime-interface-proc-macro = { version = "5.0.0", path = "proc-macro" } -sp-externalities = { version = "0.12.0", optional = true, path = "../externalities" } +sp-externalities = { version = "0.12.0", default-features = false, path = "../externalities" } codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false } static_assertions = "1.0.0" primitive-types = { version = "0.11.1", default-features = false } @@ -40,7 +40,7 @@ std = [ "sp-std/std", "sp-tracing/std", "codec/std", - "sp-externalities", + "sp-externalities/std", "primitive-types/std", ] diff --git a/primitives/runtime-interface/src/impls.rs b/primitives/runtime-interface/src/impls.rs index 3c1927d6b361a..e801931c306cf 100644 --- a/primitives/runtime-interface/src/impls.rs +++ b/primitives/runtime-interface/src/impls.rs @@ -548,3 +548,7 @@ impl PassBy for sp_storage::TrackedStorageKey { impl PassBy for sp_storage::StateVersion { type PassBy = Enum; } + +impl PassBy for sp_externalities::MultiRemovalResults { + type PassBy = Codec; +} diff --git a/primitives/state-machine/src/lib.rs b/primitives/state-machine/src/lib.rs index b15b7ac940836..3982a66bc3ef7 100644 --- a/primitives/state-machine/src/lib.rs +++ b/primitives/state-machine/src/lib.rs @@ -1677,9 +1677,13 @@ mod tests { let mut ext = Ext::new(&mut overlay, &mut cache, &backend, None); let r = ext.kill_child_storage(&child_info, Some(0), None).decon(); assert_matches!(r, (Some(_), 0, 0, 0)); - let r = ext.kill_child_storage(&child_info, Some(1), r.0.as_ref().map(|x| &x[..])).decon(); + let r = ext + .kill_child_storage(&child_info, Some(1), r.0.as_ref().map(|x| &x[..])) + .decon(); assert_matches!(r, (Some(_), 1, 1, 1)); - let r = ext.kill_child_storage(&child_info, Some(4), r.0.as_ref().map(|x| &x[..])).decon(); + let r = ext + .kill_child_storage(&child_info, Some(4), r.0.as_ref().map(|x| &x[..])) + .decon(); // Only 3 items remaining to remove assert_matches!(r, (None, 3, 3, 3)); let r = ext.kill_child_storage(&child_info, Some(1), None).decon(); From ef7f475b49450070f5f9ae63a37c36dd008f0ee4 Mon Sep 17 00:00:00 2001 From: Gavin Wood Date: Wed, 25 May 2022 13:42:22 +0100 Subject: [PATCH 31/38] Update primitives/externalities/src/lib.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- primitives/externalities/src/lib.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/primitives/externalities/src/lib.rs b/primitives/externalities/src/lib.rs index 15474183d532e..b343b8c393e00 100644 --- a/primitives/externalities/src/lib.rs +++ b/primitives/externalities/src/lib.rs @@ -68,7 +68,10 @@ pub struct MultiRemovalResults { } impl MultiRemovalResults { - pub fn decon(self) -> (Option>, u32, u32, u32) { + /// Deconstruct into the internal components. + /// + /// Returns `(maybe_cursor, backend, unique, loops)`. + pub fn deconstruct(self) -> (Option>, u32, u32, u32) { (self.maybe_cursor, self.backend, self.unique, self.loops) } } From 7db73b18448c0e4d12958f0302899ba68f193717 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Wed, 25 May 2022 13:44:43 +0100 Subject: [PATCH 32/38] Formatting --- primitives/io/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index 803fae044ccf7..1afd8fd92313c 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -111,7 +111,8 @@ impl From for KillStorageResult { fn from(r: MultiRemovalResults) -> Self { match r { MultiRemovalResults { maybe_cursor: None, backend, .. } => Self::AllRemoved(backend), - MultiRemovalResults { maybe_cursor: Some(..), backend, .. } => Self::SomeRemaining(backend), + MultiRemovalResults { maybe_cursor: Some(..), backend, .. } => + Self::SomeRemaining(backend), } } } From 6aeb3a32f90d39b20ec35b45790ea7a7a1fd9cc2 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Wed, 25 May 2022 16:19:40 +0100 Subject: [PATCH 33/38] Fixes --- primitives/state-machine/src/basic.rs | 2 +- primitives/state-machine/src/lib.rs | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/primitives/state-machine/src/basic.rs b/primitives/state-machine/src/basic.rs index 706d648f9d984..07011d8ab6e66 100644 --- a/primitives/state-machine/src/basic.rs +++ b/primitives/state-machine/src/basic.rs @@ -469,7 +469,7 @@ mod tests { }); let res = ext.kill_child_storage(child_info, None, None); - assert_eq!(res.decon(), (None, 3, 3, 3)); + assert_eq!(res.deconstruct(), (None, 3, 3, 3)); } #[test] diff --git a/primitives/state-machine/src/lib.rs b/primitives/state-machine/src/lib.rs index 3982a66bc3ef7..69cbf7af12fb3 100644 --- a/primitives/state-machine/src/lib.rs +++ b/primitives/state-machine/src/lib.rs @@ -1597,7 +1597,7 @@ mod tests { { let mut cache = StorageTransactionCache::default(); let mut ext = Ext::new(&mut overlay, &mut cache, backend, None); - assert_matches!(ext.clear_prefix(b"ab", Some(1), None).decon(), (Some(_), 1, 3, 1)); + assert_matches!(ext.clear_prefix(b"ab", Some(1), None).deconstruct(), (Some(_), 1, 3, 1)); } overlay.commit_transaction().unwrap(); @@ -1640,7 +1640,7 @@ mod tests { let mut cache = StorageTransactionCache::default(); let mut ext = Ext::new(&mut overlay, &mut cache, &backend, None); let r = ext.kill_child_storage(&child_info, Some(2), None); - assert_matches!(r.decon(), (Some(_), 2, 6, 2)); + assert_matches!(r.deconstruct(), (Some(_), 2, 6, 2)); } assert_eq!( @@ -1675,18 +1675,18 @@ mod tests { let mut overlay = OverlayedChanges::default(); let mut cache = StorageTransactionCache::default(); let mut ext = Ext::new(&mut overlay, &mut cache, &backend, None); - let r = ext.kill_child_storage(&child_info, Some(0), None).decon(); + let r = ext.kill_child_storage(&child_info, Some(0), None).deconstruct(); assert_matches!(r, (Some(_), 0, 0, 0)); let r = ext .kill_child_storage(&child_info, Some(1), r.0.as_ref().map(|x| &x[..])) - .decon(); + .deconstruct(); assert_matches!(r, (Some(_), 1, 1, 1)); let r = ext .kill_child_storage(&child_info, Some(4), r.0.as_ref().map(|x| &x[..])) - .decon(); + .deconstruct(); // Only 3 items remaining to remove assert_matches!(r, (None, 3, 3, 3)); - let r = ext.kill_child_storage(&child_info, Some(1), None).decon(); + let r = ext.kill_child_storage(&child_info, Some(1), None).deconstruct(); assert_matches!(r, (Some(_), 0, 0, 1)); } @@ -1705,7 +1705,7 @@ mod tests { let mut overlay = OverlayedChanges::default(); let mut cache = StorageTransactionCache::default(); let mut ext = Ext::new(&mut overlay, &mut cache, &backend, None); - assert_eq!(ext.kill_child_storage(&child_info, None, None).decon(), (None, 4, 4, 4)); + assert_eq!(ext.kill_child_storage(&child_info, None, None).deconstruct(), (None, 4, 4, 4)); } #[test] From 4692d0652c310937fc53b35d71e2eb634d730c84 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Wed, 25 May 2022 16:28:26 +0100 Subject: [PATCH 34/38] cargo fmt --- frame/support/src/storage/types/nmap.rs | 12 ++++++------ primitives/state-machine/src/lib.rs | 5 ++++- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/frame/support/src/storage/types/nmap.rs b/frame/support/src/storage/types/nmap.rs index 1e7e2c9734f20..894d3a42a4bc1 100755 --- a/frame/support/src/storage/types/nmap.rs +++ b/frame/support/src/storage/types/nmap.rs @@ -613,7 +613,7 @@ mod test { use crate::{ hash::{StorageHasher as _, *}, metadata::{StorageEntryModifier, StorageHasher}, - storage::types::{Key, Key as NMapKey, ValueQuery}, + storage::types::{Key as NMapKey, ValueQuery}, }; use sp_io::{hashing::twox_128, TestExternalities}; @@ -634,12 +634,12 @@ mod test { #[test] fn test_1_key() { - type A = StorageNMap, u32, OptionQuery>; + type A = StorageNMap, u32, OptionQuery>; type AValueQueryWithAnOnEmpty = - StorageNMap, u32, ValueQuery, ADefault>; - type B = StorageNMap, u32, ValueQuery>; - type C = StorageNMap, u8, ValueQuery>; - type WithLen = StorageNMap, Vec>; + StorageNMap, u32, ValueQuery, ADefault>; + type B = StorageNMap, u32, ValueQuery>; + type C = StorageNMap, u8, ValueQuery>; + type WithLen = StorageNMap, Vec>; TestExternalities::default().execute_with(|| { let mut k: Vec = vec![]; diff --git a/primitives/state-machine/src/lib.rs b/primitives/state-machine/src/lib.rs index 69cbf7af12fb3..94ac3d47f3b72 100644 --- a/primitives/state-machine/src/lib.rs +++ b/primitives/state-machine/src/lib.rs @@ -1597,7 +1597,10 @@ mod tests { { let mut cache = StorageTransactionCache::default(); let mut ext = Ext::new(&mut overlay, &mut cache, backend, None); - assert_matches!(ext.clear_prefix(b"ab", Some(1), None).deconstruct(), (Some(_), 1, 3, 1)); + assert_matches!( + ext.clear_prefix(b"ab", Some(1), None).deconstruct(), + (Some(_), 1, 3, 1) + ); } overlay.commit_transaction().unwrap(); From 7e8a8c202a142db20b5e929c0336ea4b7f5d3ece Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Wed, 25 May 2022 16:38:39 +0100 Subject: [PATCH 35/38] Fixes --- frame/support/src/storage/types/nmap.rs | 26 ++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/frame/support/src/storage/types/nmap.rs b/frame/support/src/storage/types/nmap.rs index 894d3a42a4bc1..75397623c2217 100755 --- a/frame/support/src/storage/types/nmap.rs +++ b/frame/support/src/storage/types/nmap.rs @@ -665,7 +665,7 @@ mod test { assert_eq!(Foo::get((3,)), Some(10)); } - A::swap::, _, _>((3,), (2,)); + A::swap::, _, _>((3,), (2,)); assert_eq!(A::contains_key((3,)), false); assert_eq!(A::contains_key((2,)), true); assert_eq!(A::get((3,)), None); @@ -819,26 +819,26 @@ mod test { fn test_2_keys() { type A = StorageNMap< Prefix, - (Key, Key), + (NMapKey, NMapKey), u32, OptionQuery, >; type AValueQueryWithAnOnEmpty = StorageNMap< Prefix, - (Key, Key), + (NMapKey, NMapKey), u32, ValueQuery, ADefault, >; - type B = StorageNMap, Key), u32, ValueQuery>; + type B = StorageNMap, NMapKey), u32, ValueQuery>; type C = StorageNMap< Prefix, - (Key, Key), + (NMapKey, NMapKey), u8, ValueQuery, >; type WithLen = - StorageNMap, Key), Vec>; + StorageNMap, NMapKey), Vec>; TestExternalities::default().execute_with(|| { let mut k: Vec = vec![]; @@ -857,7 +857,7 @@ mod test { assert_eq!(A::get((3, 30)), Some(10)); assert_eq!(AValueQueryWithAnOnEmpty::get((3, 30)), 10); - A::swap::<(Key, Key), _, _>((3, 30), (2, 20)); + A::swap::<(NMapKey, NMapKey), _, _>((3, 30), (2, 20)); assert_eq!(A::contains_key((3, 30)), false); assert_eq!(A::contains_key((2, 20)), true); assert_eq!(A::get((3, 30)), None); @@ -1025,32 +1025,32 @@ mod test { fn test_3_keys() { type A = StorageNMap< Prefix, - (Key, Key, Key), + (NMapKey, NMapKey, NMapKey), u32, OptionQuery, >; type AValueQueryWithAnOnEmpty = StorageNMap< Prefix, - (Key, Key, Key), + (NMapKey, NMapKey, NMapKey), u32, ValueQuery, ADefault, >; type B = StorageNMap< Prefix, - (Key, Key, Key), + (NMapKey, NMapKey, NMapKey), u32, ValueQuery, >; type C = StorageNMap< Prefix, - (Key, Key, Key), + (NMapKey, NMapKey, NMapKey), u8, ValueQuery, >; type WithLen = StorageNMap< Prefix, - (Key, Key, Key), + (NMapKey, NMapKey, NMapKey), Vec, >; @@ -1073,7 +1073,7 @@ mod test { assert_eq!(AValueQueryWithAnOnEmpty::get((1, 10, 100)), 30); A::swap::< - (Key, Key, Key), + (NMapKey, NMapKey, NMapKey), _, _, >((1, 10, 100), (2, 20, 200)); From edf7ce01958d22f2b1d571bf42332ef6ecc48290 Mon Sep 17 00:00:00 2001 From: Gavin Wood Date: Wed, 25 May 2022 18:33:23 +0100 Subject: [PATCH 36/38] Update primitives/io/src/lib.rs Co-authored-by: Keith Yeung --- primitives/io/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index 3e7a58a34154e..044b93b28cb79 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -101,9 +101,9 @@ pub enum EcdsaVerifyError { /// removed from the backend from making the `storage_kill` call. #[derive(PassByCodec, Encode, Decode)] pub enum KillStorageResult { - /// All keys to remove were removed, return number of key removed from backend. + /// All keys to remove were removed, return number of iterations performed during the operation. AllRemoved(u32), - /// Not all key to remove were removed, return number of key removed from backend. + /// Not all key to remove were removed, return number of iterations performed during the operation. SomeRemaining(u32), } From 33224f20ae693b53220483ce41ed32eb58a5d8c7 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Wed, 25 May 2022 18:34:25 +0100 Subject: [PATCH 37/38] Formatting --- frame/support/src/storage/types/nmap.rs | 45 ++++++++++++++++++++----- primitives/io/src/lib.rs | 6 ++-- 2 files changed, 40 insertions(+), 11 deletions(-) diff --git a/frame/support/src/storage/types/nmap.rs b/frame/support/src/storage/types/nmap.rs index 75397623c2217..1f303525b7476 100755 --- a/frame/support/src/storage/types/nmap.rs +++ b/frame/support/src/storage/types/nmap.rs @@ -830,15 +830,19 @@ mod test { ValueQuery, ADefault, >; - type B = StorageNMap, NMapKey), u32, ValueQuery>; + type B = + StorageNMap, NMapKey), u32, ValueQuery>; type C = StorageNMap< Prefix, (NMapKey, NMapKey), u8, ValueQuery, >; - type WithLen = - StorageNMap, NMapKey), Vec>; + type WithLen = StorageNMap< + Prefix, + (NMapKey, NMapKey), + Vec, + >; TestExternalities::default().execute_with(|| { let mut k: Vec = vec![]; @@ -857,7 +861,10 @@ mod test { assert_eq!(A::get((3, 30)), Some(10)); assert_eq!(AValueQueryWithAnOnEmpty::get((3, 30)), 10); - A::swap::<(NMapKey, NMapKey), _, _>((3, 30), (2, 20)); + A::swap::<(NMapKey, NMapKey), _, _>( + (3, 30), + (2, 20), + ); assert_eq!(A::contains_key((3, 30)), false); assert_eq!(A::contains_key((2, 20)), true); assert_eq!(A::get((3, 30)), None); @@ -1025,13 +1032,21 @@ mod test { fn test_3_keys() { type A = StorageNMap< Prefix, - (NMapKey, NMapKey, NMapKey), + ( + NMapKey, + NMapKey, + NMapKey, + ), u32, OptionQuery, >; type AValueQueryWithAnOnEmpty = StorageNMap< Prefix, - (NMapKey, NMapKey, NMapKey), + ( + NMapKey, + NMapKey, + NMapKey, + ), u32, ValueQuery, ADefault, @@ -1044,13 +1059,21 @@ mod test { >; type C = StorageNMap< Prefix, - (NMapKey, NMapKey, NMapKey), + ( + NMapKey, + NMapKey, + NMapKey, + ), u8, ValueQuery, >; type WithLen = StorageNMap< Prefix, - (NMapKey, NMapKey, NMapKey), + ( + NMapKey, + NMapKey, + NMapKey, + ), Vec, >; @@ -1073,7 +1096,11 @@ mod test { assert_eq!(AValueQueryWithAnOnEmpty::get((1, 10, 100)), 30); A::swap::< - (NMapKey, NMapKey, NMapKey), + ( + NMapKey, + NMapKey, + NMapKey, + ), _, _, >((1, 10, 100), (2, 20, 200)); diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index 044b93b28cb79..0a931919437af 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -101,9 +101,11 @@ pub enum EcdsaVerifyError { /// removed from the backend from making the `storage_kill` call. #[derive(PassByCodec, Encode, Decode)] pub enum KillStorageResult { - /// All keys to remove were removed, return number of iterations performed during the operation. + /// All keys to remove were removed, return number of iterations performed during the + /// operation. AllRemoved(u32), - /// Not all key to remove were removed, return number of iterations performed during the operation. + /// Not all key to remove were removed, return number of iterations performed during the + /// operation. SomeRemaining(u32), } From a5f349ba9620979c3c95b65547ffd106d9660d9d Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Thu, 26 May 2022 14:33:03 +0100 Subject: [PATCH 38/38] Fixes --- primitives/state-machine/src/basic.rs | 2 +- primitives/state-machine/src/ext.rs | 8 ++++---- primitives/state-machine/src/lib.rs | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/primitives/state-machine/src/basic.rs b/primitives/state-machine/src/basic.rs index 07011d8ab6e66..6efc847bfbdb7 100644 --- a/primitives/state-machine/src/basic.rs +++ b/primitives/state-machine/src/basic.rs @@ -446,7 +446,7 @@ mod tests { ext.clear_child_storage(child_info, b"dog"); assert_eq!(ext.child_storage(child_info, b"dog"), None); - ext.kill_child_storage(child_info, None, None); + let _ = ext.kill_child_storage(child_info, None, None); assert_eq!(ext.child_storage(child_info, b"doe"), None); } diff --git a/primitives/state-machine/src/ext.rs b/primitives/state-machine/src/ext.rs index 14275aa90437f..1db0ec517015b 100644 --- a/primitives/state-machine/src/ext.rs +++ b/primitives/state-machine/src/ext.rs @@ -1098,14 +1098,14 @@ mod tests { not_under_prefix.extend(b"path"); ext.set_storage(not_under_prefix.clone(), vec![10]); - ext.clear_prefix(&[], None, None); - ext.clear_prefix(&well_known_keys::CHILD_STORAGE_KEY_PREFIX[..4], None, None); + let _ = ext.clear_prefix(&[], None, None); + let _ = ext.clear_prefix(&well_known_keys::CHILD_STORAGE_KEY_PREFIX[..4], None, None); let mut under_prefix = well_known_keys::CHILD_STORAGE_KEY_PREFIX.to_vec(); under_prefix.extend(b"path"); - ext.clear_prefix(&well_known_keys::CHILD_STORAGE_KEY_PREFIX[..4], None, None); + let _ = ext.clear_prefix(&well_known_keys::CHILD_STORAGE_KEY_PREFIX[..4], None, None); assert_eq!(ext.child_storage(child_info, &[30]), Some(vec![40])); assert_eq!(ext.storage(not_under_prefix.as_slice()), Some(vec![10])); - ext.clear_prefix(¬_under_prefix[..5], None, None); + let _ = ext.clear_prefix(¬_under_prefix[..5], None, None); assert_eq!(ext.storage(not_under_prefix.as_slice()), None); } diff --git a/primitives/state-machine/src/lib.rs b/primitives/state-machine/src/lib.rs index 94ac3d47f3b72..edc3db7a60e36 100644 --- a/primitives/state-machine/src/lib.rs +++ b/primitives/state-machine/src/lib.rs @@ -1573,7 +1573,7 @@ mod tests { { let mut cache = StorageTransactionCache::default(); let mut ext = Ext::new(&mut overlay, &mut cache, backend, None); - ext.clear_prefix(b"ab", None, None); + let _ = ext.clear_prefix(b"ab", None, None); } overlay.commit_transaction().unwrap(); @@ -1723,7 +1723,7 @@ mod tests { ext.set_child_storage(child_info, b"abc".to_vec(), b"def".to_vec()); assert_eq!(ext.child_storage(child_info, b"abc"), Some(b"def".to_vec())); - ext.kill_child_storage(child_info, None, None); + let _ = ext.kill_child_storage(child_info, None, None); assert_eq!(ext.child_storage(child_info, b"abc"), None); }