From b26116e71f46436ece8d8e78ed29c4f3e502a3ff Mon Sep 17 00:00:00 2001 From: muraca Date: Tue, 24 Jan 2023 12:54:58 -0300 Subject: [PATCH 1/5] implemented `contains_prefix` for StorageDoubleMap and StorageNMap Signed-off-by: muraca --- .../src/storage/generator/double_map.rs | 7 ++++++ frame/support/src/storage/generator/nmap.rs | 7 ++++++ frame/support/src/storage/mod.rs | 24 +++++++++++++++++++ frame/support/src/storage/unhashed.rs | 4 ++++ 4 files changed, 42 insertions(+) diff --git a/frame/support/src/storage/generator/double_map.rs b/frame/support/src/storage/generator/double_map.rs index c95dcee9d7e5c..7fdcac561c093 100644 --- a/frame/support/src/storage/generator/double_map.rs +++ b/frame/support/src/storage/generator/double_map.rs @@ -233,6 +233,13 @@ where .into() } + fn contains_prefix(k1: KArg1) -> bool + where + KArg1: EncodeLike, + { + unhashed::contains_prefix(Self::storage_double_map_final_key1(k1).as_ref()) + } + fn iter_prefix_values(k1: KArg1) -> storage::PrefixIterator where KArg1: ?Sized + EncodeLike, diff --git a/frame/support/src/storage/generator/nmap.rs b/frame/support/src/storage/generator/nmap.rs index 79f3d72044e28..7e763e22c2029 100755 --- a/frame/support/src/storage/generator/nmap.rs +++ b/frame/support/src/storage/generator/nmap.rs @@ -208,6 +208,13 @@ where ) } + fn contains_prefix(partial_key: KP) -> bool + where + K: HasKeyPrefix, + { + unhashed::contains_prefix(&Self::storage_n_map_partial_key(partial_key)) + } + fn iter_prefix_values(partial_key: KP) -> PrefixIterator where K: HasKeyPrefix, diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index 8c0d6207c3f4d..a7247ee1ef4d8 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -557,6 +557,11 @@ pub trait StorageDoubleMap { where KArg1: ?Sized + EncodeLike; + /// Does any value under the first key `k1` (explicitly) exist in storage? + fn contains_prefix(k1: KArg1) -> bool + where + KArg1: EncodeLike; + /// Iterate over values that share the first key. fn iter_prefix_values(k1: KArg1) -> PrefixIterator where @@ -733,6 +738,11 @@ pub trait StorageNMap { where K: HasKeyPrefix; + /// Does any value under a `partial_key` prefix (explicitly) exist in storage? + fn contains_prefix(partial_key: KP) -> bool + where + K: HasKeyPrefix; + /// Iterate over values that share the partial prefix key. fn iter_prefix_values(partial_key: KP) -> PrefixIterator where @@ -1775,6 +1785,20 @@ mod test { type FooDoubleMap = StorageDoubleMap>>; + #[test] + fn contains_prefix_works() { + TestExternalities::default().execute_with(|| { + assert!(FooDoubleMap::iter_prefix_values(0).next().is_none()); + assert_eq!(FooDoubleMap::contains_prefix(0), false); + + assert_ok!(FooDoubleMap::try_append(1, 1, 4)); + assert!(FooDoubleMap::iter_prefix_values(1).next().is_some()); + assert!(FooDoubleMap::contains_prefix(1)); + FooDoubleMap::remove(1, 1); + assert_eq!(FooDoubleMap::contains_prefix(1), false); + }); + } + #[test] fn try_append_works() { TestExternalities::default().execute_with(|| { diff --git a/frame/support/src/storage/unhashed.rs b/frame/support/src/storage/unhashed.rs index 850e93e7d7fe4..acd610da9e51d 100644 --- a/frame/support/src/storage/unhashed.rs +++ b/frame/support/src/storage/unhashed.rs @@ -154,6 +154,10 @@ pub fn clear_prefix( MultiRemovalResults { maybe_cursor, backend: i, unique: i, loops: i } } +pub fn contains_prefix(prefix: &[u8]) -> bool { + sp_io::storage::next_key(prefix).is_some() +} + /// Get a Vec of bytes from storage. pub fn get_raw(key: &[u8]) -> Option> { sp_io::storage::get(key).map(|value| value.to_vec()) From 6fb5d13a8add4ce355962eeb7795f5389975754f Mon Sep 17 00:00:00 2001 From: muraca Date: Tue, 24 Jan 2023 14:08:15 -0300 Subject: [PATCH 2/5] match prefix to next_key Signed-off-by: muraca --- frame/support/src/storage/mod.rs | 1 + frame/support/src/storage/unhashed.rs | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index a7247ee1ef4d8..5404fe4f924d6 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -1792,6 +1792,7 @@ mod test { assert_eq!(FooDoubleMap::contains_prefix(0), false); assert_ok!(FooDoubleMap::try_append(1, 1, 4)); + assert_ok!(FooDoubleMap::try_append(2, 1, 4)); assert!(FooDoubleMap::iter_prefix_values(1).next().is_some()); assert!(FooDoubleMap::contains_prefix(1)); FooDoubleMap::remove(1, 1); diff --git a/frame/support/src/storage/unhashed.rs b/frame/support/src/storage/unhashed.rs index acd610da9e51d..086194d8473f8 100644 --- a/frame/support/src/storage/unhashed.rs +++ b/frame/support/src/storage/unhashed.rs @@ -155,7 +155,10 @@ pub fn clear_prefix( } pub fn contains_prefix(prefix: &[u8]) -> bool { - sp_io::storage::next_key(prefix).is_some() + match sp_io::storage::next_key(prefix) { + Some(key) => key.starts_with(prefix), + None => false, + } } /// Get a Vec of bytes from storage. From 5c1a05717ee0e28bf13aa6e0cc0c8d2214237e92 Mon Sep 17 00:00:00 2001 From: muraca Date: Thu, 26 Jan 2023 00:10:08 -0300 Subject: [PATCH 3/5] warning unexpected behaviour with empty keys Signed-off-by: muraca --- frame/support/src/storage/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index 5404fe4f924d6..d5a1211db494d 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -558,6 +558,7 @@ pub trait StorageDoubleMap { KArg1: ?Sized + EncodeLike; /// Does any value under the first key `k1` (explicitly) exist in storage? + /// Might have unexpected behaviour with empty keys, e.g. `[]`. fn contains_prefix(k1: KArg1) -> bool where KArg1: EncodeLike; @@ -739,6 +740,7 @@ pub trait StorageNMap { K: HasKeyPrefix; /// Does any value under a `partial_key` prefix (explicitly) exist in storage? + /// Might have unexpected behaviour with empty keys, e.g. `[]`. fn contains_prefix(partial_key: KP) -> bool where K: HasKeyPrefix; From 5e32f06af700feaa4519c9ed90bfbb34dcfdef5f Mon Sep 17 00:00:00 2001 From: muraca Date: Thu, 26 Jan 2023 11:24:34 -0300 Subject: [PATCH 4/5] clarifications for unhashed::contains_prefixed_key Signed-off-by: muraca --- frame/support/src/storage/generator/double_map.rs | 2 +- frame/support/src/storage/generator/nmap.rs | 2 +- frame/support/src/storage/unhashed.rs | 5 ++++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/frame/support/src/storage/generator/double_map.rs b/frame/support/src/storage/generator/double_map.rs index 7fdcac561c093..b60bb79566d51 100644 --- a/frame/support/src/storage/generator/double_map.rs +++ b/frame/support/src/storage/generator/double_map.rs @@ -237,7 +237,7 @@ where where KArg1: EncodeLike, { - unhashed::contains_prefix(Self::storage_double_map_final_key1(k1).as_ref()) + unhashed::contains_prefixed_key(Self::storage_double_map_final_key1(k1).as_ref()) } 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 7e763e22c2029..8cf1459431171 100755 --- a/frame/support/src/storage/generator/nmap.rs +++ b/frame/support/src/storage/generator/nmap.rs @@ -212,7 +212,7 @@ where where K: HasKeyPrefix, { - unhashed::contains_prefix(&Self::storage_n_map_partial_key(partial_key)) + unhashed::contains_prefixed_key(&Self::storage_n_map_partial_key(partial_key)) } fn iter_prefix_values(partial_key: KP) -> PrefixIterator diff --git a/frame/support/src/storage/unhashed.rs b/frame/support/src/storage/unhashed.rs index 086194d8473f8..8388c5f885c51 100644 --- a/frame/support/src/storage/unhashed.rs +++ b/frame/support/src/storage/unhashed.rs @@ -154,7 +154,10 @@ pub fn clear_prefix( MultiRemovalResults { maybe_cursor, backend: i, unique: i, loops: i } } -pub fn contains_prefix(prefix: &[u8]) -> bool { +/// Returns `true` if the storage contains any key, which starts with a certain prefix, +/// and is longer than said prefix. +/// This means that a key which equals the prefix will not be counted. +pub fn contains_prefixed_key(prefix: &[u8]) -> bool { match sp_io::storage::next_key(prefix) { Some(key) => key.starts_with(prefix), None => false, From f24a0ed7a4df67eee594a3bb6ef1c577d1507082 Mon Sep 17 00:00:00 2001 From: muraca Date: Mon, 30 Jan 2023 09:32:45 -0300 Subject: [PATCH 5/5] added tests for StorageNMap Signed-off-by: muraca --- frame/support/src/storage/mod.rs | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index d5a1211db494d..9cb06eef29f1c 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -1491,7 +1491,7 @@ pub fn storage_prefix(pallet_name: &[u8], storage_name: &[u8]) -> [u8; 32] { #[cfg(test)] mod test { use super::*; - use crate::{assert_ok, hash::Identity, Twox128}; + use crate::{assert_ok, hash::Identity, pallet_prelude::NMapKey, Twox128}; use bounded_vec::BoundedVec; use frame_support::traits::ConstU32; use generator::StorageValue as _; @@ -1786,12 +1786,19 @@ mod test { #[crate::storage_alias] type FooDoubleMap = StorageDoubleMap>>; + #[crate::storage_alias] + type FooTripleMap = StorageNMap< + Prefix, + (NMapKey, NMapKey, NMapKey), + u64, + >; #[test] fn contains_prefix_works() { TestExternalities::default().execute_with(|| { - assert!(FooDoubleMap::iter_prefix_values(0).next().is_none()); - assert_eq!(FooDoubleMap::contains_prefix(0), false); + // Test double maps + assert!(FooDoubleMap::iter_prefix_values(1).next().is_none()); + assert_eq!(FooDoubleMap::contains_prefix(1), false); assert_ok!(FooDoubleMap::try_append(1, 1, 4)); assert_ok!(FooDoubleMap::try_append(2, 1, 4)); @@ -1799,6 +1806,17 @@ mod test { assert!(FooDoubleMap::contains_prefix(1)); FooDoubleMap::remove(1, 1); assert_eq!(FooDoubleMap::contains_prefix(1), false); + + // Test N Maps + assert!(FooTripleMap::iter_prefix_values((1,)).next().is_none()); + assert_eq!(FooTripleMap::contains_prefix((1,)), false); + + FooTripleMap::insert((1, 1, 1), 4); + FooTripleMap::insert((2, 1, 1), 4); + assert!(FooTripleMap::iter_prefix_values((1,)).next().is_some()); + assert!(FooTripleMap::contains_prefix((1,))); + FooTripleMap::remove((1, 1, 1)); + assert_eq!(FooTripleMap::contains_prefix((1,)), false); }); }