From dd7aa33b9760f93b178f9c3ecbf181173b281fcd Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 2 Aug 2023 21:08:32 +0200 Subject: [PATCH 1/4] Use proper heap and add item counting Signed-off-by: Oliver Tale-Yazdi --- frame/paged-list/fuzzer/src/paged_list.rs | 7 +- frame/paged-list/src/lib.rs | 13 ++-- frame/paged-list/src/mock.rs | 8 +- frame/paged-list/src/paged_list.rs | 95 ++++++++++++----------- frame/paged-list/src/tests.rs | 6 ++ frame/support/src/storage/mod.rs | 2 + 6 files changed, 73 insertions(+), 58 deletions(-) diff --git a/frame/paged-list/fuzzer/src/paged_list.rs b/frame/paged-list/fuzzer/src/paged_list.rs index 43b797eee6bfb..814345164ad14 100644 --- a/frame/paged-list/fuzzer/src/paged_list.rs +++ b/frame/paged-list/fuzzer/src/paged_list.rs @@ -37,7 +37,7 @@ type Meta = MetaOf; fn main() { loop { - fuzz!(|data: (Vec, u8)| { + fuzz!(|data: (Vec, u16)| { drain_append_work(data.0, data.1); }); } @@ -46,13 +46,13 @@ fn main() { /// Appends and drains random number of elements in random order and checks storage invariants. /// /// It also changes the maximal number of elements per page dynamically, hence the `page_size`. -fn drain_append_work(ops: Vec, page_size: u8) { +fn drain_append_work(ops: Vec, page_size: u16) { if page_size == 0 { return } TestExternalities::default().execute_with(|| { - ValuesPerNewPage::set(&page_size.into()); + HeapSize::set(&(page_size as u32 * 4)); // `* 4` to convert from items to byte size. let _g = StorageNoopGuard::default(); let mut total: i64 = 0; @@ -70,6 +70,7 @@ fn drain_append_work(ops: Vec, page_size: u8) { } assert_eq!(List::drain().count(), total as usize); + assert_eq!(List::len(), total as u64); // `StorageNoopGuard` checks that there is no storage leaked. }); } diff --git a/frame/paged-list/src/lib.rs b/frame/paged-list/src/lib.rs index 9aa089bb09273..8cb74c5929a61 100644 --- a/frame/paged-list/src/lib.rs +++ b/frame/paged-list/src/lib.rs @@ -94,16 +94,13 @@ pub mod pallet { /// Note that this does not retroactively affect already created pages. This value can be /// changed at any time without requiring a runtime migration. #[pallet::constant] - type ValuesPerNewPage: Get; + type HeapSize: Get; } /// A storage paged list akin to what the FRAME macros would generate. // Note that FRAME does natively support paged lists in storage. - pub type List = StoragePagedList< - ListPrefix, - >::Value, - >::ValuesPerNewPage, - >; + pub type List = + StoragePagedList, >::Value, >::HeapSize>; } // This exposes the list functionality to other pallets. @@ -111,6 +108,10 @@ impl, I: 'static> StorageList for Pallet { type Iterator = as StorageList>::Iterator; type Appender = as StorageList>::Appender; + fn len() -> u64 { + List::::len() + } + fn iter() -> Self::Iterator { List::::iter() } diff --git a/frame/paged-list/src/mock.rs b/frame/paged-list/src/mock.rs index 390b4a8530dce..88a926490310a 100644 --- a/frame/paged-list/src/mock.rs +++ b/frame/paged-list/src/mock.rs @@ -65,22 +65,22 @@ impl frame_system::Config for Test { } frame_support::parameter_types! { - pub storage ValuesPerNewPage: u32 = 5; + pub storage HeapSize: u32 = 20; // 5 u32 per page pub const MaxPages: Option = Some(20); } impl crate::Config for Test { type Value = u32; - type ValuesPerNewPage = ValuesPerNewPage; + type HeapSize = HeapSize; } impl crate::Config for Test { type Value = u32; - type ValuesPerNewPage = ValuesPerNewPage; + type HeapSize = HeapSize; } pub type MetaOf = - StoragePagedListMeta, ::Value, ::ValuesPerNewPage>; + StoragePagedListMeta, ::Value, ::HeapSize>; /// Build genesis storage according to the mock runtime. pub fn new_test_ext() -> sp_io::TestExternalities { diff --git a/frame/paged-list/src/paged_list.rs b/frame/paged-list/src/paged_list.rs index 37ebe80d93448..64da516dbf85a 100644 --- a/frame/paged-list/src/paged_list.rs +++ b/frame/paged-list/src/paged_list.rs @@ -52,8 +52,8 @@ pub type ValueIndex = u32; /// The metadata of this struct is stored in [`StoragePagedListMeta`]. The data is stored in /// [`Page`]s. /// -/// Each [`Page`] holds at most `ValuesPerNewPage` values in its `values` vector. The last page is -/// the only one that could have less than `ValuesPerNewPage` values. +/// Each [`Page`] holds at most `HeapSize` values in its `values` vector. The last page is +/// the only one that could have less than `HeapSize` values. /// **Iteration** happens by starting /// at [`first_page`][StoragePagedListMeta::first_page]/ /// [`first_value_offset`][StoragePagedListMeta::first_value_offset] and incrementing these indices @@ -62,7 +62,7 @@ pub type ValueIndex = u32; /// reduces the number of storage `read` calls on the overlay. **Appending** to the list happens by /// appending to the last page by utilizing [`sp_io::storage::append`]. It allows to directly extend /// the elements of `values` vector of the page without loading the whole vector from storage. A new -/// page is instantiated once [`Page::next`] overflows `ValuesPerNewPage`. Its vector will also be +/// page is instantiated once [`Page::next`] overflows `HeapSize`. Its vector will also be /// created through [`sp_io::storage::append`]. **Draining** advances the internal indices identical /// to Iteration. It additionally persists the increments to storage and thereby 'drains' elements. /// Completely drained pages are deleted from storage. @@ -77,8 +77,8 @@ pub type ValueIndex = u32; /// asymptotic case when using an `Appender`, and worse in all. For example when appending just /// one value. /// - It does incur a read overhead on the host side as compared to a `StorageValue>`. -pub struct StoragePagedList { - _phantom: PhantomData<(Prefix, Value, ValuesPerNewPage)>, +pub struct StoragePagedList { + _phantom: PhantomData<(Prefix, Value, HeapSize)>, } /// The state of a [`StoragePagedList`]. @@ -88,7 +88,7 @@ pub struct StoragePagedList { Encode, Decode, CloneNoBound, PartialEqNoBound, EqNoBound, DebugNoBound, DefaultNoBound, )] // todo ignore scale bounds -pub struct StoragePagedListMeta { +pub struct StoragePagedListMeta { /// The first page that could contain a value. /// /// Can be >0 when pages were deleted. @@ -106,17 +106,19 @@ pub struct StoragePagedListMeta { /// /// Appending starts at this index. If the page does not hold a value at this index, then the /// whole list is empty. The only case where this can happen is when both are `0`. - pub last_page_len: ValueIndex, + pub last_page_byte_offset: u32, - _phantom: PhantomData<(Prefix, Value, ValuesPerNewPage)>, + pub total_items: u64, + + _phantom: PhantomData<(Prefix, Value, HeapSize)>, } -impl frame_support::storage::StorageAppender - for StoragePagedListMeta +impl frame_support::storage::StorageAppender + for StoragePagedListMeta where Prefix: StorageInstance, Value: FullCodec, - ValuesPerNewPage: Get, + HeapSize: Get, { fn append(&mut self, item: EncodeLikeValue) where @@ -126,11 +128,11 @@ where } } -impl StoragePagedListMeta +impl StoragePagedListMeta where Prefix: StorageInstance, Value: FullCodec, - ValuesPerNewPage: Get, + HeapSize: Get, { pub fn from_storage() -> Option { let key = Self::key(); @@ -146,13 +148,14 @@ where where EncodeLikeValue: EncodeLike, { - // Note: we use >= here in case someone decreased it in a runtime upgrade. - if self.last_page_len >= ValuesPerNewPage::get() { + let enc_size = item.encoded_size() as u32; + if (self.last_page_byte_offset.saturating_add(enc_size)) > HeapSize::get() { self.last_page.saturating_inc(); - self.last_page_len = 0; + self.last_page_byte_offset = 0; } let key = page_key::(self.last_page); - self.last_page_len.saturating_inc(); + self.last_page_byte_offset.saturating_accrue(enc_size); + self.total_items.saturating_inc(); sp_io::storage::append(&key, item.encode()); self.store(); } @@ -237,7 +240,7 @@ impl Iterator for Page { /// Iterates over values of a [`StoragePagedList`]. /// /// Can optionally drain the iterated values. -pub struct StoragePagedListIterator { +pub struct StoragePagedListIterator { // Design: we put the Page into the iterator to have fewer storage look-ups. Yes, these // look-ups would be cached anyway, but bugging the overlay on each `.next` call still seems // like a poor trade-off than caching it in the iterator directly. Iterating and modifying is @@ -245,31 +248,27 @@ pub struct StoragePagedListIterator { // the iterator did not find any data upon setup or ran out of pages. page: Option>, drain: bool, - meta: StoragePagedListMeta, + meta: StoragePagedListMeta, } -impl StoragePagedListIterator +impl StoragePagedListIterator where Prefix: StorageInstance, Value: FullCodec, - ValuesPerNewPage: Get, + HeapSize: Get, { /// Read self from the storage. - pub fn from_meta( - meta: StoragePagedListMeta, - drain: bool, - ) -> Self { + pub fn from_meta(meta: StoragePagedListMeta, drain: bool) -> Self { let page = Page::::from_storage::(meta.first_page, meta.first_value_offset); Self { page, drain, meta } } } -impl Iterator - for StoragePagedListIterator +impl Iterator for StoragePagedListIterator where Prefix: StorageInstance, Value: FullCodec, - ValuesPerNewPage: Get, + HeapSize: Get, { type Item = Value; @@ -290,6 +289,7 @@ where page.delete::(); self.meta.first_value_offset = 0; self.meta.first_page.saturating_inc(); + self.meta.total_items.saturating_dec(); } debug_assert!(!self.drain || self.meta.first_page == page.index + 1); @@ -304,6 +304,7 @@ where } else { if self.drain { self.meta.first_value_offset.saturating_inc(); + self.meta.total_items.saturating_dec(); self.meta.store(); } } @@ -311,15 +312,19 @@ where } } -impl frame_support::storage::StorageList - for StoragePagedList +impl frame_support::storage::StorageList + for StoragePagedList where Prefix: StorageInstance, Value: FullCodec, - ValuesPerNewPage: Get, + HeapSize: Get, { - type Iterator = StoragePagedListIterator; - type Appender = StoragePagedListMeta; + type Iterator = StoragePagedListIterator; + type Appender = StoragePagedListMeta; + + fn len() -> u64 { + Self::read_meta().total_items + } fn iter() -> Self::Iterator { StoragePagedListIterator::from_meta(Self::read_meta(), false) @@ -334,13 +339,13 @@ where } } -impl StoragePagedList +impl StoragePagedList where Prefix: StorageInstance, Value: FullCodec, - ValuesPerNewPage: Get, + HeapSize: Get, { - fn read_meta() -> StoragePagedListMeta { + fn read_meta() -> StoragePagedListMeta { // Use default here to not require a setup migration. StoragePagedListMeta::from_storage().unwrap_or_default() } @@ -348,7 +353,7 @@ where /// Provides a fast append iterator. /// /// The list should not be modified while appending. Also don't call it recursively. - fn appender() -> StoragePagedListMeta { + fn appender() -> StoragePagedListMeta { Self::read_meta() } @@ -386,12 +391,12 @@ where } } -impl frame_support::storage::StoragePrefixedContainer - for StoragePagedList +impl frame_support::storage::StoragePrefixedContainer + for StoragePagedList where Prefix: StorageInstance, Value: FullCodec, - ValuesPerNewPage: Get, + HeapSize: Get, { fn module_prefix() -> &'static [u8] { StoragePagedListPrefix::::module_prefix() @@ -416,7 +421,7 @@ pub(crate) mod mock { pub use sp_io::{hashing::twox_128, TestExternalities}; parameter_types! { - pub const ValuesPerNewPage: u32 = 5; + pub const HeapSize: u32 = 20; pub const MaxPages: Option = Some(20); } @@ -428,7 +433,7 @@ pub(crate) mod mock { const STORAGE_PREFIX: &'static str = "foo"; } - pub type List = StoragePagedList; + pub type List = StoragePagedList; } #[cfg(test)] @@ -457,7 +462,7 @@ mod tests { // all gone assert_eq!(List::as_vec(), Vec::::new()); // Need to delete the metadata manually. - StoragePagedListMeta::::delete(); + StoragePagedListMeta::::delete(); }); } @@ -528,12 +533,12 @@ mod tests { assert_eq!(as_vec.len(), 4, "Second page contains 4"); let meta = sp_io::storage::get(&meta_key::()).expect("Meta should be present"); - let meta: StoragePagedListMeta = + let meta: StoragePagedListMeta = Decode::decode(&mut &meta[..]).unwrap(); assert_eq!(meta.first_page, 0); assert_eq!(meta.first_value_offset, 0); assert_eq!(meta.last_page, 1); - assert_eq!(meta.last_page_len, 4); + assert_eq!(meta.last_page_byte_offset, 16); let page = Page::::from_storage::(0, 0).unwrap(); assert_eq!(page.index, 0); diff --git a/frame/paged-list/src/tests.rs b/frame/paged-list/src/tests.rs index becb4b23508ef..76da1316dc482 100644 --- a/frame/paged-list/src/tests.rs +++ b/frame/paged-list/src/tests.rs @@ -30,6 +30,7 @@ fn append_one_works() { PagedList::append_one(1); assert_eq!(PagedList::iter().collect::>(), vec![1]); + assert_eq!(PagedList::len(), 1); }); } @@ -40,6 +41,7 @@ fn append_many_works() { PagedList::append_many(0..3); assert_eq!(PagedList::iter().collect::>(), vec![0, 1, 2]); + assert_eq!(PagedList::len(), 3); }); } @@ -55,6 +57,7 @@ fn appender_works() { appender.append_many(2..4); assert_eq!(PagedList::iter().collect::>(), vec![0, 1, 2, 3]); + assert_eq!(PagedList::len(), 4); }); } @@ -68,6 +71,7 @@ fn iter_works() { assert_eq!(iter.next(), Some(0)); assert_eq!(iter.next(), Some(1)); assert_eq!(iter.collect::>(), (2..10).collect::>()); + assert_eq!(PagedList::len(), 10); }); } @@ -78,8 +82,10 @@ fn drain_works() { PagedList::append_many(0..3); PagedList::drain().next(); assert_eq!(PagedList::iter().collect::>(), vec![1, 2], "0 is drained"); + assert_eq!(PagedList::len(), 2); PagedList::drain().peekable().peek(); assert_eq!(PagedList::iter().collect::>(), vec![2], "Peeking removed 1"); + assert_eq!(PagedList::len(), 1); }); } diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index 4eecd4febf007..ea8f1877f7cb4 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -168,6 +168,8 @@ pub trait StorageList { /// Append iterator for fast append operations. type Appender: StorageAppender; + fn len() -> u64; + /// List the elements in append order. fn iter() -> Self::Iterator; From d7a111a1e0aa575b63786c61c07aea58928ee8f2 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 3 Aug 2023 11:55:44 +0200 Subject: [PATCH 2/4] Somewhat works but i still dont like it... Signed-off-by: Oliver Tale-Yazdi --- frame/paged-list/src/lib.rs | 45 +++++++++++++----------------- frame/paged-list/src/mock.rs | 14 +++------- frame/paged-list/src/paged_list.rs | 6 ++++ frame/paged-list/src/tests.rs | 12 +++++--- 4 files changed, 37 insertions(+), 40 deletions(-) diff --git a/frame/paged-list/src/lib.rs b/frame/paged-list/src/lib.rs index 8cb74c5929a61..8490c2ab96304 100644 --- a/frame/paged-list/src/lib.rs +++ b/frame/paged-list/src/lib.rs @@ -74,6 +74,7 @@ use frame_support::{ pallet_prelude::StorageList, traits::{PalletInfoAccess, StorageInstance}, }; +use sp_core::Get; pub use paged_list::StoragePagedList; #[frame_support::pallet] @@ -82,56 +83,48 @@ pub mod pallet { use frame_support::pallet_prelude::*; #[pallet::pallet] - pub struct Pallet(_); + pub struct Pallet(_); #[pallet::config] - pub trait Config: frame_system::Config { - /// The value type that can be stored in the list. - type Value: FullCodec; - - /// The number of values that can be put into newly created pages. - /// - /// Note that this does not retroactively affect already created pages. This value can be - /// changed at any time without requiring a runtime migration. - #[pallet::constant] - type HeapSize: Get; - } + pub trait Config: frame_system::Config {} - /// A storage paged list akin to what the FRAME macros would generate. + // A storage paged list akin to what the FRAME macros would generate. // Note that FRAME does natively support paged lists in storage. - pub type List = - StoragePagedList, >::Value, >::HeapSize>; + /*pub type List = + StoragePagedList, >::Value, >::HeapSize>;*/ } +pub struct List(core::marker::PhantomData<(T, Value, HeapSize, I)>); + // This exposes the list functionality to other pallets. -impl, I: 'static> StorageList for Pallet { - type Iterator = as StorageList>::Iterator; - type Appender = as StorageList>::Appender; +impl, Value: FullCodec, HeapSize: Get, const I: u8> StorageList for List { + type Iterator = , Value, HeapSize> as StorageList>::Iterator; + type Appender = , Value, HeapSize> as StorageList>::Appender; fn len() -> u64 { - List::::len() + StoragePagedList::, Value, HeapSize>::len() } fn iter() -> Self::Iterator { - List::::iter() + StoragePagedList::, Value, HeapSize>::iter() } fn drain() -> Self::Iterator { - List::::drain() + StoragePagedList::, Value, HeapSize>::drain() } fn appender() -> Self::Appender { - List::::appender() + StoragePagedList::, Value, HeapSize>::appender() } } /// Generates a unique storage prefix for each instance of the pallet. pub struct ListPrefix(core::marker::PhantomData<(T, I)>); -impl, I: 'static> StorageInstance for ListPrefix { - fn pallet_prefix() -> &'static str { - crate::Pallet::::name() +impl, const I: u8> StorageInstance for ListPrefix { + fn pallet_prefix() -> &[u8] { + crate::Pallet::::name() } - const STORAGE_PREFIX: &'static str = "paged_list"; + const STORAGE_PREFIX: &[u8] = b"paged_list"; } diff --git a/frame/paged-list/src/mock.rs b/frame/paged-list/src/mock.rs index 88a926490310a..161e3480bbb37 100644 --- a/frame/paged-list/src/mock.rs +++ b/frame/paged-list/src/mock.rs @@ -69,18 +69,12 @@ frame_support::parameter_types! { pub const MaxPages: Option = Some(20); } -impl crate::Config for Test { - type Value = u32; - type HeapSize = HeapSize; -} +impl crate::Config for Test {} -impl crate::Config for Test { - type Value = u32; - type HeapSize = HeapSize; -} +impl crate::Config for Test {} -pub type MetaOf = - StoragePagedListMeta, ::Value, ::HeapSize>; +pub type MetaOf = + StoragePagedListMeta, Value, HeapSize>; /// Build genesis storage according to the mock runtime. pub fn new_test_ext() -> sp_io::TestExternalities { diff --git a/frame/paged-list/src/paged_list.rs b/frame/paged-list/src/paged_list.rs index 64da516dbf85a..1417e04ae5218 100644 --- a/frame/paged-list/src/paged_list.rs +++ b/frame/paged-list/src/paged_list.rs @@ -113,6 +113,12 @@ pub struct StoragePagedListMeta { _phantom: PhantomData<(Prefix, Value, HeapSize)>, } +/// Similar to [`StorageInstance`] but allows for runtime-generated prefix. +pub trait StoragePrefix { + fn pallet_prefix() -> &[u8]; + fn instance_prefix() -> &[u8]; +} + impl frame_support::storage::StorageAppender for StoragePagedListMeta where diff --git a/frame/paged-list/src/tests.rs b/frame/paged-list/src/tests.rs index 76da1316dc482..a34aab0e180ff 100644 --- a/frame/paged-list/src/tests.rs +++ b/frame/paged-list/src/tests.rs @@ -21,8 +21,12 @@ #![cfg(test)] use crate::{mock::*, *}; +use sp_core::ConstU32; use frame_support::storage::{StorageList, StoragePrefixedContainer}; +type PagedList = List>; +type PagedList2 = List>; + #[docify::export] #[test] fn append_one_works() { @@ -105,10 +109,10 @@ fn iter_independent_works() { assert_eq!(PagedList::iter().count(), 0); }); } - +/* #[test] fn prefix_distinct() { - let p1 = List::::final_prefix(); - let p2 = List::::final_prefix(); + let p1 = List::, ()>::final_prefix(); + let p2 = List::, crate::Instance2>::final_prefix(); assert_ne!(p1, p2); -} +}*/ From 9ff838f81f083bbfe88099981875dc9756062dae Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 10 Aug 2023 14:29:56 +0200 Subject: [PATCH 3/4] Add paginated storage primitives Signed-off-by: Oliver Tale-Yazdi --- Cargo.lock | 54 +- Cargo.toml | 3 +- frame/benchmarking/pov/Cargo.toml | 24 +- frame/benchmarking/pov/src/benchmarking.rs | 12 +- frame/benchmarking/pov/src/lib.rs | 3 + frame/examples/basic/src/tests.rs | 20 + frame/paged-list/Cargo.toml | 2 +- frame/paged-list/fuzzer/Cargo.toml | 2 +- frame/paged-list/fuzzer/src/paged_list.rs | 15 +- frame/paged-list/src/lib.rs | 61 +- frame/paged-list/src/mock.rs | 23 +- frame/paged-list/src/paged_list.rs | 592 ------------- frame/paged-list/src/tests.rs | 33 +- frame/support/Cargo.toml | 39 +- frame/support/fuzzer/Cargo.toml | 29 + .../fuzzer/src/bin/paged_all_in_one.rs | 88 ++ frame/support/fuzzer/src/bin/paged_list.rs | 73 ++ frame/support/fuzzer/src/bin/paged_nmap.rs | 77 ++ frame/support/fuzzer/src/lib.rs | 120 +++ .../procedural/src/pallet/expand/storage.rs | 30 +- .../procedural/src/pallet/parse/storage.rs | 39 + frame/support/src/lib.rs | 5 +- frame/support/src/storage/lists.rs | 141 ++++ frame/support/src/storage/mod.rs | 74 +- frame/support/src/storage/types/mod.rs | 4 + frame/support/src/storage/types/paged_list.rs | 474 +++++++++++ frame/support/src/storage/types/paged_nmap.rs | 795 ++++++++++++++++++ frame/support/test/tests/final_keys.rs | 258 +++++- primitives/metadata-ir/Cargo.toml | 2 +- 29 files changed, 2263 insertions(+), 829 deletions(-) delete mode 100644 frame/paged-list/src/paged_list.rs create mode 100644 frame/support/fuzzer/Cargo.toml create mode 100644 frame/support/fuzzer/src/bin/paged_all_in_one.rs create mode 100644 frame/support/fuzzer/src/bin/paged_list.rs create mode 100644 frame/support/fuzzer/src/bin/paged_nmap.rs create mode 100644 frame/support/fuzzer/src/lib.rs create mode 100644 frame/support/src/storage/lists.rs create mode 100644 frame/support/src/storage/types/paged_list.rs create mode 100644 frame/support/src/storage/types/paged_nmap.rs diff --git a/Cargo.lock b/Cargo.lock index 8effc229ca332..7500ca633ed85 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1874,44 +1874,18 @@ checksum = "fea41bba32d969b513997752735605054bc0dfa92b4c56bf1189f2e174be7a10" [[package]] name = "docify" -version = "0.1.16" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "af1b04e6ef3d21119d3eb7b032bca17f99fe041e9c072f30f32cc0e1a2b1f3c4" -dependencies = [ - "docify_macros 0.1.16", -] - -[[package]] -name = "docify" -version = "0.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f6491709f76fb7ceb951244daf624d480198b427556084391d6e3c33d3ae74b9" -dependencies = [ - "docify_macros 0.2.0", -] - -[[package]] -name = "docify_macros" -version = "0.1.16" +version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8b5610df7f2acf89a1bb5d1a66ae56b1c7fcdcfe3948856fb3ace3f644d70eb7" +checksum = "029de870d175d11969524d91a3fb2cbf6d488b853bff99d41cf65e533ac7d9d2" dependencies = [ - "common-path", - "derive-syn-parse", - "lazy_static", - "proc-macro2", - "quote", - "regex", - "syn 2.0.18", - "termcolor", - "walkdir", + "docify_macros", ] [[package]] name = "docify_macros" -version = "0.2.0" +version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ffc5338a9f72ce29a81377d9039798fcc926fb471b2004666caf48e446dffbbf" +checksum = "cac43324656a1b05eb0186deb51f27d2d891c704c37f34de281ef6297ba193e5" dependencies = [ "common-path", "derive-syn-parse", @@ -1921,6 +1895,7 @@ dependencies = [ "regex", "syn 2.0.18", "termcolor", + "toml 0.7.4", "walkdir", ] @@ -2487,8 +2462,7 @@ dependencies = [ [[package]] name = "frame-metadata" version = "16.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "87cf1549fba25a6fcac22785b61698317d958e96cac72a59102ea45b9ae64692" +source = "git+https://github.com/ggwpez/frame-metadata?branch=main#a53a427c9f93bf0acb42092d5782c78550940eee" dependencies = [ "cfg-if", "parity-scale-codec", @@ -2596,6 +2570,16 @@ dependencies = [ "syn 2.0.18", ] +[[package]] +name = "frame-support-storage-fuzzer" +version = "0.1.0" +dependencies = [ + "arbitrary", + "frame-support", + "honggfuzz", + "sp-io", +] + [[package]] name = "frame-support-test" version = "3.0.0" @@ -6274,7 +6258,7 @@ dependencies = [ name = "pallet-fast-unstake" version = "4.0.0-dev" dependencies = [ - "docify 0.2.0", + "docify", "frame-benchmarking", "frame-election-provider-support", "frame-support", @@ -6738,7 +6722,7 @@ dependencies = [ name = "pallet-paged-list" version = "0.1.0" dependencies = [ - "docify 0.1.16", + "docify", "frame-benchmarking", "frame-support", "frame-system", diff --git a/Cargo.toml b/Cargo.toml index 9ee8142e23e76..27d25534be334 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -180,9 +180,9 @@ members = [ "frame/nomination-pools/benchmarking", "frame/nomination-pools/test-staking", "frame/nomination-pools/runtime-api", + "frame/insecure-randomness-collective-flip", "frame/paged-list", "frame/paged-list/fuzzer", - "frame/insecure-randomness-collective-flip", "frame/ranked-collective", "frame/recovery", "frame/referenda", @@ -209,6 +209,7 @@ members = [ "frame/support/test", "frame/support/test/compile_pass", "frame/support/test/pallet", + "frame/support/fuzzer", "frame/system", "frame/system/benchmarking", "frame/system/rpc/runtime-api", diff --git a/frame/benchmarking/pov/Cargo.toml b/frame/benchmarking/pov/Cargo.toml index c0ba8285519a5..cb8b6bc7a401d 100644 --- a/frame/benchmarking/pov/Cargo.toml +++ b/frame/benchmarking/pov/Cargo.toml @@ -23,24 +23,6 @@ sp-std = { version = "8.0.0", default-features = false, path = "../../../primiti [features] default = ["std"] -std = [ - "codec/std", - "frame-benchmarking/std", - "frame-support/std", - "frame-system/std", - "scale-info/std", - "sp-io/std", - "sp-runtime/std", - "sp-std/std", -] -runtime-benchmarks = [ - "frame-system/runtime-benchmarks", - "frame-benchmarking/runtime-benchmarks", - "frame-support/runtime-benchmarks", - "sp-runtime/runtime-benchmarks" -] -try-runtime = [ - "frame-support/try-runtime", - "frame-system/try-runtime", - "sp-runtime/try-runtime" -] +std = ["codec/std", "frame-benchmarking/std", "frame-support/std", "frame-system/std", "scale-info/std", "sp-io/std", "sp-runtime/std", "sp-std/std"] +runtime-benchmarks = ["frame-system/runtime-benchmarks", "frame-benchmarking/runtime-benchmarks", "frame-support/runtime-benchmarks", "sp-runtime/runtime-benchmarks"] +try-runtime = ["frame-support/try-runtime", "frame-system/try-runtime", "sp-runtime/try-runtime"] diff --git a/frame/benchmarking/pov/src/benchmarking.rs b/frame/benchmarking/pov/src/benchmarking.rs index 473947b171ac5..3937d07708129 100644 --- a/frame/benchmarking/pov/src/benchmarking.rs +++ b/frame/benchmarking/pov/src/benchmarking.rs @@ -21,7 +21,7 @@ use super::*; -use frame_support::traits::UnfilteredDispatchable; +use frame_support::{pallet_prelude::*, traits::UnfilteredDispatchable}; use frame_system::{Pallet as System, RawOrigin}; use sp_runtime::traits::Hash; @@ -330,6 +330,16 @@ frame_benchmarking::benchmarks! { } } + paged_list_iter_pages { + let n in 0 .. 20; + PagedList64k::::append_many(0u32..((1<<14) * 20)); + }: { + let mut iter = PagedList64k::::iter(); + for i in 0 .. ((1<<14) * n) { + assert_eq!(iter.next(), Some(i)); + } + } + impl_benchmark_test_suite!( Pallet, mock::new_test_ext(), diff --git a/frame/benchmarking/pov/src/lib.rs b/frame/benchmarking/pov/src/lib.rs index eb02ccc983c09..47b229aa2c4bf 100644 --- a/frame/benchmarking/pov/src/lib.rs +++ b/frame/benchmarking/pov/src/lib.rs @@ -111,6 +111,9 @@ pub mod pallet { pub(crate) type UnboundedMapTwox = StorageMap, QueryKind = OptionQuery>; + #[pallet::storage] + pub(super) type PagedList64k = StoragePagedList<_, u32>; + #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event { diff --git a/frame/examples/basic/src/tests.rs b/frame/examples/basic/src/tests.rs index addf219dc3c39..bc13df432254d 100644 --- a/frame/examples/basic/src/tests.rs +++ b/frame/examples/basic/src/tests.rs @@ -21,6 +21,7 @@ use crate::*; use frame_support::{ assert_ok, dispatch::{DispatchInfo, GetDispatchInfo}, + pallet_prelude::*, traits::{ConstU64, OnInitialize}, }; use sp_core::H256; @@ -195,3 +196,22 @@ fn weights_work() { // TODO: account for proof size weight assert!(info1.weight.ref_time() > info2.weight.ref_time()); } + +#[test] +fn paged_nmap_works() { + new_test_ext().execute_with(|| { + use frame_support::storage::StorageKeyedList; + + for x in 0..10 { + for y in 0..10 { + PagedNMap::::append_many((x, y), 0..3); + } + } + + for x in 0..10 { + for y in 0..10 { + assert_eq!(PagedNMap::::iter((x, y)).collect::>(), vec![0, 1, 2]); + } + } + }); +} diff --git a/frame/paged-list/Cargo.toml b/frame/paged-list/Cargo.toml index 71af167d78913..3d7010d331b2e 100644 --- a/frame/paged-list/Cargo.toml +++ b/frame/paged-list/Cargo.toml @@ -13,7 +13,7 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false, features = [ "derive"] } -docify = "0.1.13" +docify = "0.2.1" scale-info = { version = "2.0.0", default-features = false, features = ["derive"] } frame-benchmarking = { version = "4.0.0-dev", default-features = false, optional = true, path = "../benchmarking" } diff --git a/frame/paged-list/fuzzer/Cargo.toml b/frame/paged-list/fuzzer/Cargo.toml index 9402ca8a48477..cd8f5879ec216 100644 --- a/frame/paged-list/fuzzer/Cargo.toml +++ b/frame/paged-list/fuzzer/Cargo.toml @@ -10,7 +10,7 @@ description = "Fuzz storage types of pallet-paged-list" publish = false [[bin]] -name = "pallet-paged-list" +name = "pallet-paged-list-fuzzer" path = "src/paged_list.rs" [dependencies] diff --git a/frame/paged-list/fuzzer/src/paged_list.rs b/frame/paged-list/fuzzer/src/paged_list.rs index 814345164ad14..0362612008fab 100644 --- a/frame/paged-list/fuzzer/src/paged_list.rs +++ b/frame/paged-list/fuzzer/src/paged_list.rs @@ -21,7 +21,8 @@ //! //! # Debugging a panic //! Once a panic is found, it can be debugged with -//! `cargo hfuzz run-debug pallet-paged-list hfuzz_workspace/pallet-paged-list/*.fuzz`. +//! `cargo hfuzz run-debug pallet-paged-list-fuzzer +//! hfuzz_workspace/pallet-paged-list-fuzzer/*.fuzz`. //! //! # More information //! More information about `honggfuzz` can be found @@ -37,7 +38,7 @@ type Meta = MetaOf; fn main() { loop { - fuzz!(|data: (Vec, u16)| { + fuzz!(|data: (Vec, u8)| { drain_append_work(data.0, data.1); }); } @@ -46,13 +47,13 @@ fn main() { /// Appends and drains random number of elements in random order and checks storage invariants. /// /// It also changes the maximal number of elements per page dynamically, hence the `page_size`. -fn drain_append_work(ops: Vec, page_size: u16) { - if page_size == 0 { +fn drain_append_work(ops: Vec, page_size: u8) { + if page_size < 4 { return } TestExternalities::default().execute_with(|| { - HeapSize::set(&(page_size as u32 * 4)); // `* 4` to convert from items to byte size. + ValuesPerNewPage::set(&page_size.into()); let _g = StorageNoopGuard::default(); let mut total: i64 = 0; @@ -61,16 +62,16 @@ fn drain_append_work(ops: Vec, page_size: u16) { assert!(total >= 0); assert_eq!(List::iter().count(), total as usize); + assert_eq!(total as u64, List::len()); // We have the assumption that the queue removes the metadata when empty. if total == 0 { assert_eq!(List::drain().count(), 0); - assert_eq!(Meta::from_storage().unwrap_or_default(), Default::default()); + assert_eq!(Meta::from_storage(((),)).unwrap_or_default(), Default::default()); } } assert_eq!(List::drain().count(), total as usize); - assert_eq!(List::len(), total as u64); // `StorageNoopGuard` checks that there is no storage leaked. }); } diff --git a/frame/paged-list/src/lib.rs b/frame/paged-list/src/lib.rs index 8490c2ab96304..ffcc7bef2e9eb 100644 --- a/frame/paged-list/src/lib.rs +++ b/frame/paged-list/src/lib.rs @@ -39,18 +39,18 @@ //! ## Examples //! //! 1. **Appending** some data to the list can happen either by [`Pallet::append_one`]: -#![doc = docify::embed!("frame/paged-list/src/tests.rs", append_one_works)] +#![doc = docify::embed!("src/tests.rs", append_one_works)] //! 2. or by [`Pallet::append_many`]. This should always be preferred to repeated calls to //! [`Pallet::append_one`]: -#![doc = docify::embed!("frame/paged-list/src/tests.rs", append_many_works)] +#![doc = docify::embed!("src/tests.rs", append_many_works)] //! 3. If you want to append many values (ie. in a loop), then best use the [`Pallet::appender`]: -#![doc = docify::embed!("frame/paged-list/src/tests.rs", appender_works)] +#![doc = docify::embed!("src/tests.rs", appender_works)] //! 4. **Iterating** over the list can be done with [`Pallet::iter`]. It uses the standard //! `Iterator` trait: -#![doc = docify::embed!("frame/paged-list/src/tests.rs", iter_works)] +#![doc = docify::embed!("src/tests.rs", iter_works)] //! 5. **Draining** elements happens through the [`Pallet::drain`] iterator. Note that even //! *peeking* a value will already remove it. -#![doc = docify::embed!("frame/paged-list/src/tests.rs", drain_works)] +#![doc = docify::embed!("src/tests.rs", drain_works)] //! //! ## Pallet API //! @@ -66,16 +66,14 @@ pub use pallet::*; pub mod mock; -mod paged_list; mod tests; use codec::FullCodec; use frame_support::{ pallet_prelude::StorageList, + storage::types::StoragePagedList, traits::{PalletInfoAccess, StorageInstance}, }; -use sp_core::Get; -pub use paged_list::StoragePagedList; #[frame_support::pallet] pub mod pallet { @@ -83,48 +81,59 @@ pub mod pallet { use frame_support::pallet_prelude::*; #[pallet::pallet] - pub struct Pallet(_); + pub struct Pallet(_); #[pallet::config] - pub trait Config: frame_system::Config {} + pub trait Config: frame_system::Config { + /// The value type that can be stored in the list. + type Value: FullCodec; + + /// The number of values that can be put into newly created pages. + /// + /// Note that this does not retroactively affect already created pages. This value can be + /// changed at any time without requiring a runtime migration. + #[pallet::constant] + type ValuesPerNewPage: Get; + } - // A storage paged list akin to what the FRAME macros would generate. + /// A storage paged list akin to what the FRAME macros would generate. // Note that FRAME does natively support paged lists in storage. - /*pub type List = - StoragePagedList, >::Value, >::HeapSize>;*/ + pub type List = StoragePagedList< + ListPrefix, + >::Value, + >::ValuesPerNewPage, + >; } -pub struct List(core::marker::PhantomData<(T, Value, HeapSize, I)>); - // This exposes the list functionality to other pallets. -impl, Value: FullCodec, HeapSize: Get, const I: u8> StorageList for List { - type Iterator = , Value, HeapSize> as StorageList>::Iterator; - type Appender = , Value, HeapSize> as StorageList>::Appender; +impl, I: 'static> StorageList for Pallet { + type Iterator = as StorageList>::Iterator; + type Appender = as StorageList>::Appender; fn len() -> u64 { - StoragePagedList::, Value, HeapSize>::len() + List::::len() } fn iter() -> Self::Iterator { - StoragePagedList::, Value, HeapSize>::iter() + List::::iter() } fn drain() -> Self::Iterator { - StoragePagedList::, Value, HeapSize>::drain() + List::::drain() } fn appender() -> Self::Appender { - StoragePagedList::, Value, HeapSize>::appender() + List::::appender() } } /// Generates a unique storage prefix for each instance of the pallet. pub struct ListPrefix(core::marker::PhantomData<(T, I)>); -impl, const I: u8> StorageInstance for ListPrefix { - fn pallet_prefix() -> &[u8] { - crate::Pallet::::name() +impl, I: 'static> StorageInstance for ListPrefix { + fn pallet_prefix() -> &'static str { + crate::Pallet::::name() } - const STORAGE_PREFIX: &[u8] = b"paged_list"; + const STORAGE_PREFIX: &'static str = "paged_list"; } diff --git a/frame/paged-list/src/mock.rs b/frame/paged-list/src/mock.rs index 161e3480bbb37..6b45666ed8e7b 100644 --- a/frame/paged-list/src/mock.rs +++ b/frame/paged-list/src/mock.rs @@ -19,8 +19,11 @@ #![cfg(feature = "std")] -use crate::{paged_list::StoragePagedListMeta, Config, ListPrefix}; -use frame_support::traits::{ConstU16, ConstU64}; +use crate::{Config, ListPrefix}; +use frame_support::{ + storage::types::StoragePagedListMeta, + traits::{ConstU16, ConstU64}, +}; use sp_core::H256; use sp_runtime::{ traits::{BlakeTwo256, IdentityLookup}, @@ -65,16 +68,22 @@ impl frame_system::Config for Test { } frame_support::parameter_types! { - pub storage HeapSize: u32 = 20; // 5 u32 per page + pub storage ValuesPerNewPage: u32 = 5; pub const MaxPages: Option = Some(20); } -impl crate::Config for Test {} +impl crate::Config for Test { + type Value = u32; + type ValuesPerNewPage = ValuesPerNewPage; +} -impl crate::Config for Test {} +impl crate::Config for Test { + type Value = u32; + type ValuesPerNewPage = ValuesPerNewPage; +} -pub type MetaOf = - StoragePagedListMeta, Value, HeapSize>; +pub type MetaOf = + StoragePagedListMeta, ::Value, ::ValuesPerNewPage>; /// Build genesis storage according to the mock runtime. pub fn new_test_ext() -> sp_io::TestExternalities { diff --git a/frame/paged-list/src/paged_list.rs b/frame/paged-list/src/paged_list.rs deleted file mode 100644 index 1417e04ae5218..0000000000000 --- a/frame/paged-list/src/paged_list.rs +++ /dev/null @@ -1,592 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) Parity Technologies (UK) Ltd. -// SPDX-License-Identifier: Apache-2.0 - -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -//! Paged storage list. - -// links are better than no links - even when they refer to private stuff. -#![allow(rustdoc::private_intra_doc_links)] -#![deny(rustdoc::broken_intra_doc_links)] -#![deny(missing_docs)] -#![deny(unsafe_code)] - -use codec::{Decode, Encode, EncodeLike, FullCodec}; -use core::marker::PhantomData; -use frame_support::{ - defensive, - storage::StoragePrefixedContainer, - traits::{Get, StorageInstance}, - CloneNoBound, DebugNoBound, DefaultNoBound, EqNoBound, PartialEqNoBound, -}; -use sp_runtime::traits::Saturating; -use sp_std::prelude::*; - -pub type PageIndex = u32; -pub type ValueIndex = u32; - -/// A paginated storage list. -/// -/// # Motivation -/// -/// This type replaces `StorageValue>` in situations where only iteration and appending is -/// needed. There are a few places where this is the case. A paginated structure reduces the memory -/// usage when a storage transactions needs to be rolled back. The main motivation is therefore a -/// reduction of runtime memory on storage transaction rollback. Should be configured such that the -/// size of a page is about 64KiB. This can only be ensured when `V` implements `MaxEncodedLen`. -/// -/// # Implementation -/// -/// The metadata of this struct is stored in [`StoragePagedListMeta`]. The data is stored in -/// [`Page`]s. -/// -/// Each [`Page`] holds at most `HeapSize` values in its `values` vector. The last page is -/// the only one that could have less than `HeapSize` values. -/// **Iteration** happens by starting -/// at [`first_page`][StoragePagedListMeta::first_page]/ -/// [`first_value_offset`][StoragePagedListMeta::first_value_offset] and incrementing these indices -/// as long as there are elements in the page and there are pages in storage. All elements of a page -/// are loaded once a page is read from storage. Iteration then happens on the cached elements. This -/// reduces the number of storage `read` calls on the overlay. **Appending** to the list happens by -/// appending to the last page by utilizing [`sp_io::storage::append`]. It allows to directly extend -/// the elements of `values` vector of the page without loading the whole vector from storage. A new -/// page is instantiated once [`Page::next`] overflows `HeapSize`. Its vector will also be -/// created through [`sp_io::storage::append`]. **Draining** advances the internal indices identical -/// to Iteration. It additionally persists the increments to storage and thereby 'drains' elements. -/// Completely drained pages are deleted from storage. -/// -/// # Further Observations -/// -/// - The encoded layout of a page is exactly its [`Page::values`]. The [`Page::next`] offset is -/// stored in the [`StoragePagedListMeta`] instead. There is no particular reason for this, -/// besides having all management state handy in one location. -/// - The PoV complexity of iterating compared to a `StorageValue>` is improved for -/// "shortish" iterations and worse for total iteration. The append complexity is identical in the -/// asymptotic case when using an `Appender`, and worse in all. For example when appending just -/// one value. -/// - It does incur a read overhead on the host side as compared to a `StorageValue>`. -pub struct StoragePagedList { - _phantom: PhantomData<(Prefix, Value, HeapSize)>, -} - -/// The state of a [`StoragePagedList`]. -/// -/// This struct doubles as [`frame_support::storage::StorageList::Appender`]. -#[derive( - Encode, Decode, CloneNoBound, PartialEqNoBound, EqNoBound, DebugNoBound, DefaultNoBound, -)] -// todo ignore scale bounds -pub struct StoragePagedListMeta { - /// The first page that could contain a value. - /// - /// Can be >0 when pages were deleted. - pub first_page: PageIndex, - /// The first index inside `first_page` that could contain a value. - /// - /// Can be >0 when values were deleted. - pub first_value_offset: ValueIndex, - - /// The last page that could contain data. - /// - /// Appending starts at this page index. - pub last_page: PageIndex, - /// The last value inside `last_page` that could contain a value. - /// - /// Appending starts at this index. If the page does not hold a value at this index, then the - /// whole list is empty. The only case where this can happen is when both are `0`. - pub last_page_byte_offset: u32, - - pub total_items: u64, - - _phantom: PhantomData<(Prefix, Value, HeapSize)>, -} - -/// Similar to [`StorageInstance`] but allows for runtime-generated prefix. -pub trait StoragePrefix { - fn pallet_prefix() -> &[u8]; - fn instance_prefix() -> &[u8]; -} - -impl frame_support::storage::StorageAppender - for StoragePagedListMeta -where - Prefix: StorageInstance, - Value: FullCodec, - HeapSize: Get, -{ - fn append(&mut self, item: EncodeLikeValue) - where - EncodeLikeValue: EncodeLike, - { - self.append_one(item); - } -} - -impl StoragePagedListMeta -where - Prefix: StorageInstance, - Value: FullCodec, - HeapSize: Get, -{ - pub fn from_storage() -> Option { - let key = Self::key(); - - sp_io::storage::get(&key).and_then(|raw| Self::decode(&mut &raw[..]).ok()) - } - - pub fn key() -> Vec { - meta_key::() - } - - pub fn append_one(&mut self, item: EncodeLikeValue) - where - EncodeLikeValue: EncodeLike, - { - let enc_size = item.encoded_size() as u32; - if (self.last_page_byte_offset.saturating_add(enc_size)) > HeapSize::get() { - self.last_page.saturating_inc(); - self.last_page_byte_offset = 0; - } - let key = page_key::(self.last_page); - self.last_page_byte_offset.saturating_accrue(enc_size); - self.total_items.saturating_inc(); - sp_io::storage::append(&key, item.encode()); - self.store(); - } - - pub fn store(&self) { - let key = Self::key(); - self.using_encoded(|enc| sp_io::storage::set(&key, enc)); - } - - pub fn reset(&mut self) { - *self = Default::default(); - Self::delete(); - } - - pub fn delete() { - sp_io::storage::clear(&Self::key()); - } -} - -/// A page that was decoded from storage and caches its values. -pub struct Page { - /// The index of the page. - index: PageIndex, - /// The remaining values of the page, to be drained by [`Page::next`]. - values: sp_std::iter::Skip>, -} - -impl Page { - /// Read the page with `index` from storage and assume the first value at `value_index`. - pub fn from_storage( - index: PageIndex, - value_index: ValueIndex, - ) -> Option { - let key = page_key::(index); - let values = sp_io::storage::get(&key) - .and_then(|raw| sp_std::vec::Vec::::decode(&mut &raw[..]).ok())?; - if values.is_empty() { - // Dont create empty pages. - return None - } - let values = values.into_iter().skip(value_index as usize); - - Some(Self { index, values }) - } - - /// Whether no more values can be read from this page. - pub fn is_eof(&self) -> bool { - self.values.len() == 0 - } - - /// Delete this page from storage. - pub fn delete(&self) { - delete_page::(self.index); - } -} - -/// Delete a page with `index` from storage. -// Does not live under `Page` since it does not require the `Value` generic. -pub(crate) fn delete_page(index: PageIndex) { - let key = page_key::(index); - sp_io::storage::clear(&key); -} - -/// Storage key of a page with `index`. -// Does not live under `Page` since it does not require the `Value` generic. -pub(crate) fn page_key(index: PageIndex) -> Vec { - (StoragePagedListPrefix::::final_prefix(), b"page", index).encode() -} - -pub(crate) fn meta_key() -> Vec { - (StoragePagedListPrefix::::final_prefix(), b"meta").encode() -} - -impl Iterator for Page { - type Item = V; - - fn next(&mut self) -> Option { - self.values.next() - } -} - -/// Iterates over values of a [`StoragePagedList`]. -/// -/// Can optionally drain the iterated values. -pub struct StoragePagedListIterator { - // Design: we put the Page into the iterator to have fewer storage look-ups. Yes, these - // look-ups would be cached anyway, but bugging the overlay on each `.next` call still seems - // like a poor trade-off than caching it in the iterator directly. Iterating and modifying is - // not allowed at the same time anyway, just like with maps. Note: if Page is empty then - // the iterator did not find any data upon setup or ran out of pages. - page: Option>, - drain: bool, - meta: StoragePagedListMeta, -} - -impl StoragePagedListIterator -where - Prefix: StorageInstance, - Value: FullCodec, - HeapSize: Get, -{ - /// Read self from the storage. - pub fn from_meta(meta: StoragePagedListMeta, drain: bool) -> Self { - let page = Page::::from_storage::(meta.first_page, meta.first_value_offset); - Self { page, drain, meta } - } -} - -impl Iterator for StoragePagedListIterator -where - Prefix: StorageInstance, - Value: FullCodec, - HeapSize: Get, -{ - type Item = Value; - - fn next(&mut self) -> Option { - let page = self.page.as_mut()?; - let value = match page.next() { - Some(value) => value, - None => { - defensive!("There are no empty pages in storage; nuking the list"); - self.meta.reset(); - self.page = None; - return None - }, - }; - - if page.is_eof() { - if self.drain { - page.delete::(); - self.meta.first_value_offset = 0; - self.meta.first_page.saturating_inc(); - self.meta.total_items.saturating_dec(); - } - - debug_assert!(!self.drain || self.meta.first_page == page.index + 1); - self.page = Page::from_storage::(page.index.saturating_add(1), 0); - if self.drain { - if self.page.is_none() { - self.meta.reset(); - } else { - self.meta.store(); - } - } - } else { - if self.drain { - self.meta.first_value_offset.saturating_inc(); - self.meta.total_items.saturating_dec(); - self.meta.store(); - } - } - Some(value) - } -} - -impl frame_support::storage::StorageList - for StoragePagedList -where - Prefix: StorageInstance, - Value: FullCodec, - HeapSize: Get, -{ - type Iterator = StoragePagedListIterator; - type Appender = StoragePagedListMeta; - - fn len() -> u64 { - Self::read_meta().total_items - } - - fn iter() -> Self::Iterator { - StoragePagedListIterator::from_meta(Self::read_meta(), false) - } - - fn drain() -> Self::Iterator { - StoragePagedListIterator::from_meta(Self::read_meta(), true) - } - - fn appender() -> Self::Appender { - Self::appender() - } -} - -impl StoragePagedList -where - Prefix: StorageInstance, - Value: FullCodec, - HeapSize: Get, -{ - fn read_meta() -> StoragePagedListMeta { - // Use default here to not require a setup migration. - StoragePagedListMeta::from_storage().unwrap_or_default() - } - - /// Provides a fast append iterator. - /// - /// The list should not be modified while appending. Also don't call it recursively. - fn appender() -> StoragePagedListMeta { - Self::read_meta() - } - - /// Return the elements of the list. - #[cfg(test)] - fn as_vec() -> Vec { - >::iter().collect() - } - - /// Return and remove the elements of the list. - #[cfg(test)] - fn as_drained_vec() -> Vec { - >::drain().collect() - } -} - -/// Provides the final prefix for a [`StoragePagedList`]. -/// -/// It solely exists so that when re-using it from the iterator and meta struct, none of the un-used -/// generics bleed through. Otherwise when only having the `StoragePrefixedContainer` implementation -/// on the list directly, the iterator and metadata need to muster *all* generics, even the ones -/// that are completely useless for prefix calculation. -struct StoragePagedListPrefix(PhantomData); - -impl frame_support::storage::StoragePrefixedContainer for StoragePagedListPrefix -where - Prefix: StorageInstance, -{ - fn module_prefix() -> &'static [u8] { - Prefix::pallet_prefix().as_bytes() - } - - fn storage_prefix() -> &'static [u8] { - Prefix::STORAGE_PREFIX.as_bytes() - } -} - -impl frame_support::storage::StoragePrefixedContainer - for StoragePagedList -where - Prefix: StorageInstance, - Value: FullCodec, - HeapSize: Get, -{ - fn module_prefix() -> &'static [u8] { - StoragePagedListPrefix::::module_prefix() - } - - fn storage_prefix() -> &'static [u8] { - StoragePagedListPrefix::::storage_prefix() - } -} - -/// Prelude for (doc)tests. -#[cfg(feature = "std")] -#[allow(dead_code)] -pub(crate) mod mock { - pub use super::*; - pub use frame_support::{ - metadata_ir::{StorageEntryModifierIR, StorageEntryTypeIR, StorageHasherIR}, - parameter_types, - storage::{types::ValueQuery, StorageList as _}, - StorageNoopGuard, - }; - pub use sp_io::{hashing::twox_128, TestExternalities}; - - parameter_types! { - pub const HeapSize: u32 = 20; - pub const MaxPages: Option = Some(20); - } - - pub struct Prefix; - impl StorageInstance for Prefix { - fn pallet_prefix() -> &'static str { - "test" - } - const STORAGE_PREFIX: &'static str = "foo"; - } - - pub type List = StoragePagedList; -} - -#[cfg(test)] -mod tests { - use super::mock::*; - - #[test] - fn append_works() { - TestExternalities::default().execute_with(|| { - List::append_many(0..1000); - assert_eq!(List::as_vec(), (0..1000).collect::>()); - }); - } - - /// Draining all works. - #[test] - fn simple_drain_works() { - TestExternalities::default().execute_with(|| { - let _g = StorageNoopGuard::default(); // All in all a No-Op - List::append_many(0..1000); - - assert_eq!(List::as_drained_vec(), (0..1000).collect::>()); - - assert_eq!(List::read_meta(), Default::default()); - - // all gone - assert_eq!(List::as_vec(), Vec::::new()); - // Need to delete the metadata manually. - StoragePagedListMeta::::delete(); - }); - } - - /// Drain half of the elements and iterator the rest. - #[test] - fn partial_drain_works() { - TestExternalities::default().execute_with(|| { - List::append_many(0..100); - - let vals = List::drain().take(50).collect::>(); - assert_eq!(vals, (0..50).collect::>()); - - let meta = List::read_meta(); - // Will switch over to `10/0`, but will in the next call. - assert_eq!((meta.first_page, meta.first_value_offset), (10, 0)); - - // 50 gone, 50 to go - assert_eq!(List::as_vec(), (50..100).collect::>()); - }); - } - - /// Draining, appending and iterating work together. - #[test] - fn drain_append_iter_works() { - TestExternalities::default().execute_with(|| { - for r in 1..=100 { - List::append_many(0..12); - List::append_many(0..12); - - let dropped = List::drain().take(12).collect::>(); - assert_eq!(dropped, (0..12).collect::>()); - - assert_eq!(List::as_vec(), (0..12).cycle().take(r * 12).collect::>()); - } - }); - } - - /// Pages are removed ASAP. - #[test] - fn drain_eager_page_removal() { - TestExternalities::default().execute_with(|| { - List::append_many(0..9); - - assert!(sp_io::storage::exists(&page_key::(0))); - assert!(sp_io::storage::exists(&page_key::(1))); - - assert_eq!(List::drain().take(5).count(), 5); - // Page 0 is eagerly removed. - assert!(!sp_io::storage::exists(&page_key::(0))); - assert!(sp_io::storage::exists(&page_key::(1))); - }); - } - - /// Appending encodes pages as `Vec`. - #[test] - fn append_storage_layout() { - TestExternalities::default().execute_with(|| { - List::append_many(0..9); - - let key = page_key::(0); - let raw = sp_io::storage::get(&key).expect("Page should be present"); - let as_vec = Vec::::decode(&mut &raw[..]).unwrap(); - assert_eq!(as_vec.len(), 5, "First page contains 5"); - - let key = page_key::(1); - let raw = sp_io::storage::get(&key).expect("Page should be present"); - let as_vec = Vec::::decode(&mut &raw[..]).unwrap(); - assert_eq!(as_vec.len(), 4, "Second page contains 4"); - - let meta = sp_io::storage::get(&meta_key::()).expect("Meta should be present"); - let meta: StoragePagedListMeta = - Decode::decode(&mut &meta[..]).unwrap(); - assert_eq!(meta.first_page, 0); - assert_eq!(meta.first_value_offset, 0); - assert_eq!(meta.last_page, 1); - assert_eq!(meta.last_page_byte_offset, 16); - - let page = Page::::from_storage::(0, 0).unwrap(); - assert_eq!(page.index, 0); - assert_eq!(page.values.count(), 5); - - let page = Page::::from_storage::(1, 0).unwrap(); - assert_eq!(page.index, 1); - assert_eq!(page.values.count(), 4); - }); - } - - #[test] - fn page_key_correct() { - let got = page_key::(0); - let pallet_prefix = StoragePagedListPrefix::::final_prefix(); - let want = (pallet_prefix, b"page", 0).encode(); - - assert_eq!(want.len(), 32 + 4 + 4); - assert!(want.starts_with(&pallet_prefix[..])); - assert_eq!(got, want); - } - - #[test] - fn meta_key_correct() { - let got = meta_key::(); - let pallet_prefix = StoragePagedListPrefix::::final_prefix(); - let want = (pallet_prefix, b"meta").encode(); - - assert_eq!(want.len(), 32 + 4); - assert!(want.starts_with(&pallet_prefix[..])); - assert_eq!(got, want); - } - - #[test] - fn peekable_drain_also_deletes() { - TestExternalities::default().execute_with(|| { - List::append_many(0..10); - - let mut iter = List::drain().peekable(); - assert_eq!(iter.peek(), Some(&0)); - // `peek` does remove one element... - assert_eq!(List::iter().count(), 9); - }); - } -} diff --git a/frame/paged-list/src/tests.rs b/frame/paged-list/src/tests.rs index a34aab0e180ff..f1a2b3c7d923c 100644 --- a/frame/paged-list/src/tests.rs +++ b/frame/paged-list/src/tests.rs @@ -21,12 +21,8 @@ #![cfg(test)] use crate::{mock::*, *}; -use sp_core::ConstU32; use frame_support::storage::{StorageList, StoragePrefixedContainer}; -type PagedList = List>; -type PagedList2 = List>; - #[docify::export] #[test] fn append_one_works() { @@ -34,7 +30,6 @@ fn append_one_works() { PagedList::append_one(1); assert_eq!(PagedList::iter().collect::>(), vec![1]); - assert_eq!(PagedList::len(), 1); }); } @@ -45,14 +40,13 @@ fn append_many_works() { PagedList::append_many(0..3); assert_eq!(PagedList::iter().collect::>(), vec![0, 1, 2]); - assert_eq!(PagedList::len(), 3); }); } #[docify::export] #[test] fn appender_works() { - use frame_support::storage::StorageAppender; + use frame_support::storage::StorageListAppender; test_closure(|| { let mut appender = PagedList::appender(); @@ -61,7 +55,6 @@ fn appender_works() { appender.append_many(2..4); assert_eq!(PagedList::iter().collect::>(), vec![0, 1, 2, 3]); - assert_eq!(PagedList::len(), 4); }); } @@ -75,7 +68,6 @@ fn iter_works() { assert_eq!(iter.next(), Some(0)); assert_eq!(iter.next(), Some(1)); assert_eq!(iter.collect::>(), (2..10).collect::>()); - assert_eq!(PagedList::len(), 10); }); } @@ -86,33 +78,32 @@ fn drain_works() { PagedList::append_many(0..3); PagedList::drain().next(); assert_eq!(PagedList::iter().collect::>(), vec![1, 2], "0 is drained"); - assert_eq!(PagedList::len(), 2); PagedList::drain().peekable().peek(); assert_eq!(PagedList::iter().collect::>(), vec![2], "Peeking removed 1"); - assert_eq!(PagedList::len(), 1); }); } #[test] fn iter_independent_works() { test_closure(|| { - PagedList::append_many(0..1000); - PagedList2::append_many(0..1000); + PagedList::append_many(0..100); + PagedList2::append_many(0..200); - assert_eq!(PagedList::iter().collect::>(), (0..1000).collect::>()); - assert_eq!(PagedList::iter().collect::>(), (0..1000).collect::>()); + assert_eq!(PagedList::iter().collect::>(), (0..100).collect::>()); + assert_eq!(PagedList2::iter().collect::>(), (0..200).collect::>()); // drain - assert_eq!(PagedList::drain().collect::>(), (0..1000).collect::>()); - assert_eq!(PagedList2::iter().collect::>(), (0..1000).collect::>()); + assert_eq!(PagedList::drain().collect::>(), (0..100).collect::>()); + assert_eq!(PagedList2::drain().collect::>(), (0..200).collect::>()); assert_eq!(PagedList::iter().count(), 0); + assert_eq!(PagedList2::iter().count(), 0); }); } -/* + #[test] fn prefix_distinct() { - let p1 = List::, ()>::final_prefix(); - let p2 = List::, crate::Instance2>::final_prefix(); + let p1 = MetaOf::::storage_key(((),)); + let p2 = MetaOf::::storage_key(((),)); assert_ne!(p1, p2); -}*/ +} diff --git a/frame/support/Cargo.toml b/frame/support/Cargo.toml index fab6167f3d50f..5276c5ccebf1e 100644 --- a/frame/support/Cargo.toml +++ b/frame/support/Cargo.toml @@ -16,7 +16,7 @@ targets = ["x86_64-unknown-linux-gnu"] serde = { version = "1.0.163", default-features = false, features = ["alloc", "derive"] } codec = { package = "parity-scale-codec", version = "3.6.1", default-features = false, features = ["derive", "max-encoded-len"] } scale-info = { version = "2.5.0", default-features = false, features = ["derive"] } -frame-metadata = { version = "16.0.0", default-features = false, features = ["current"] } +frame-metadata = { version = "16.0.0", default-features = false, features = ["current"], git = "https://github.com/ggwpez/frame-metadata", branch = "main" } # FAIL-CI sp-api = { version = "4.0.0-dev", default-features = false, path = "../../primitives/api", features = [ "frame-metadata" ] } sp-std = { version = "8.0.0", default-features = false, path = "../../primitives/std" } sp-io = { version = "23.0.0", default-features = false, path = "../../primitives/io" } @@ -53,40 +53,9 @@ array-bytes = "6.1" [features] default = ["std"] -std = [ - "sp-core/std", - "k256/std", - "serde/std", - "sp-api/std", - "sp-io/std", - "codec/std", - "scale-info/std", - "sp-std/std", - "sp-runtime/std", - "sp-tracing/std", - "sp-arithmetic/std", - "frame-metadata/std", - "sp-inherents/std", - "sp-staking/std", - "sp-state-machine/std", - "sp-weights/std", - "frame-support-procedural/std", - "log/std", - "environmental/std", - "sp-genesis-builder/std", - "frame-system/std", - "sp-debug-derive/std" -] -runtime-benchmarks = [ - "frame-system/runtime-benchmarks", - "sp-runtime/runtime-benchmarks", - "sp-staking/runtime-benchmarks" -] -try-runtime = [ - "sp-debug-derive/force-debug", - "frame-system/try-runtime", - "sp-runtime/try-runtime" -] +std = ["sp-core/std", "k256/std", "serde/std", "sp-api/std", "sp-io/std", "codec/std", "scale-info/std", "sp-std/std", "sp-runtime/std", "sp-tracing/std", "sp-arithmetic/std", "frame-metadata/std", "sp-inherents/std", "sp-staking/std", "sp-state-machine/std", "sp-weights/std", "frame-support-procedural/std", "log/std", "environmental/std", "sp-genesis-builder/std", "frame-system/std", "sp-debug-derive/std"] +runtime-benchmarks = ["frame-system/runtime-benchmarks", "sp-runtime/runtime-benchmarks", "sp-staking/runtime-benchmarks"] +try-runtime = ["sp-debug-derive/force-debug", "frame-system/try-runtime", "sp-runtime/try-runtime"] experimental = [] # By default some types have documentation, `no-metadata-docs` allows to reduce the documentation # in the metadata. diff --git a/frame/support/fuzzer/Cargo.toml b/frame/support/fuzzer/Cargo.toml new file mode 100644 index 0000000000000..728f3020ddd3e --- /dev/null +++ b/frame/support/fuzzer/Cargo.toml @@ -0,0 +1,29 @@ +[package] +name = "frame-support-storage-fuzzer" +version = "0.1.0" +authors = ["Parity Technologies "] +edition = "2021" +license = "Apache-2.0" +homepage = "https://substrate.io" +repository = "https://github.com/paritytech/substrate/" +description = "Fuzz storage types of frame-support" +publish = false + +[[bin]] +name = "paged-storage-list-fuzzer" +path = "src/bin/paged_list.rs" + +[[bin]] +name = "paged-storage-nmap-fuzzer" +path = "src/bin/paged_nmap.rs" + +[[bin]] +name = "paged-storage-all-in-one-fuzzer" +path = "src/bin/paged_all_in_one.rs" + +[dependencies] +arbitrary = "1.3.0" +honggfuzz = "0.5.49" + +frame-support = { version = "4.0.0-dev", default-features = false, features = [ "std" ], path = "../../support" } +sp-io = { path = "../../../primitives/io", default-features = false, features = [ "std" ] } diff --git a/frame/support/fuzzer/src/bin/paged_all_in_one.rs b/frame/support/fuzzer/src/bin/paged_all_in_one.rs new file mode 100644 index 0000000000000..42a6c04201c30 --- /dev/null +++ b/frame/support/fuzzer/src/bin/paged_all_in_one.rs @@ -0,0 +1,88 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Fuzzer to test all paged data structures in one go to ensure that they won't interfere with +//! each other. + +use frame_support::{storage::StorageKeyedList, StorageNoopGuard}; +use honggfuzz::fuzz; +use std::collections::BTreeMap; + +use frame_support_storage_fuzzer::*; + +fn main() { + loop { + fuzz!(|data: (Vec, u16)| { + drain_append_work(data.0, data.1); + }); + } +} + +/// Appends and drains random number of elements in random order and checks storage invariants. +/// +/// It also changes the maximal number of elements per page dynamically, hence the `page_size`. +fn drain_append_work(ops: Vec, page_size: u16) { + if page_size < 4 { + return + } + + TestExternalities::default().execute_with(|| { + // Changing the heapsize should be fine at any point in time - even to non-multiple of 4, + HeapSize::set(&page_size.into()); + + let _g = StorageNoopGuard::default(); + let mut map_totals: BTreeMap = BTreeMap::new(); + let mut list_total = 0i64; + + for op in ops.into_iter() { + match op { + AllInOneOp::NMap(op) => { + let total = map_totals.entry(op.key).or_insert(0); + *total += op.op.exec_map(op.key); + + assert!(*total >= 0); + assert_eq!(NMap::iter((op.key,)).count(), *total as usize); + assert_eq!(*total as u64, NMap::len((op.key,))); + + // We have the assumption that the queue removes the metadata when empty. + if *total == 0 { + assert_eq!(NMap::drain((op.key,)).count(), 0); + assert_eq!(NMap::meta((op.key,)), Default::default()); + } + }, + AllInOneOp::List(op) => { + list_total += op.exec_list::(); + + assert!(list_total >= 0); + assert_eq!(List::iter().count(), list_total as usize); + assert_eq!(list_total as u64, List::len()); + + if list_total == 0 { + assert_eq!(List::drain().count(), 0); + assert_eq!(List::meta(), Default::default()); + } + }, + } + } + + for (key, total) in map_totals { + assert_eq!(NMap::drain((key,)).count(), total as usize); + } + assert_eq!(List::drain().count(), list_total as usize); + // `StorageNoopGuard` checks that there is no storage leaked. + }); +} diff --git a/frame/support/fuzzer/src/bin/paged_list.rs b/frame/support/fuzzer/src/bin/paged_list.rs new file mode 100644 index 0000000000000..58baae437a0b1 --- /dev/null +++ b/frame/support/fuzzer/src/bin/paged_list.rs @@ -0,0 +1,73 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! # Running +//! Running this fuzzer can be done with `cargo hfuzz run pallet-paged-list`. `honggfuzz` CLI +//! options can be used by setting `HFUZZ_RUN_ARGS`, such as `-n 4` to use 4 threads. +//! +//! # Debugging a panic +//! Once a panic is found, it can be debugged with +//! `cargo hfuzz run-debug pallet-paged-list hfuzz_workspace/pallet-paged-list/*.fuzz`. +//! +//! # More information +//! More information about `honggfuzz` can be found +//! [here](https://docs.rs/honggfuzz/). + +use frame_support::StorageNoopGuard; +use honggfuzz::fuzz; + +use frame_support_storage_fuzzer::*; + +fn main() { + loop { + fuzz!(|data: (Vec, u8)| { + drain_append_work(data.0, data.1); + }); + } +} + +/// Appends and drains random number of elements in random order and checks storage invariants. +/// +/// It also changes the maximal number of elements per page dynamically, hence the `page_size`. +fn drain_append_work(ops: Vec, page_size: u8) { + if page_size == 0 { + return + } + + TestExternalities::default().execute_with(|| { + //ValuesPerNewPage::set(&page_size.into()); + let _g = StorageNoopGuard::default(); + let mut total: i64 = 0; + + for op in ops.into_iter() { + total += op.exec_list::(); + + assert!(total >= 0); + assert_eq!(List::iter().count(), total as usize); + assert_eq!(total as u64, List::len()); + + // We have the assumption that the queue removes the metadata when empty. + if total == 0 { + assert_eq!(List::drain().count(), 0); + assert_eq!(List::meta(), Default::default()); + } + } + + assert_eq!(List::drain().count(), total as usize); + // `StorageNoopGuard` checks that there is no storage leaked. + }); +} diff --git a/frame/support/fuzzer/src/bin/paged_nmap.rs b/frame/support/fuzzer/src/bin/paged_nmap.rs new file mode 100644 index 0000000000000..256fe19428cb2 --- /dev/null +++ b/frame/support/fuzzer/src/bin/paged_nmap.rs @@ -0,0 +1,77 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! # Running +//! Running this fuzzer can be done with `cargo hfuzz run pallet-paged-list`. `honggfuzz` CLI +//! options can be used by setting `HFUZZ_RUN_ARGS`, such as `-n 4` to use 4 threads. +//! +//! # Debugging a panic +//! Once a panic is found, it can be debugged with +//! `cargo hfuzz run-debug pallet-paged-list hfuzz_workspace/pallet-paged-list/*.fuzz`. +//! +//! # More information +//! More information about `honggfuzz` can be found +//! [here](https://docs.rs/honggfuzz/). + +use frame_support::{storage::StorageKeyedList, StorageNoopGuard}; +use honggfuzz::fuzz; +use std::collections::BTreeMap; + +use frame_support_storage_fuzzer::*; + +fn main() { + loop { + fuzz!(|data: (Vec, u8)| { + drain_append_work(data.0, data.1); + }); + } +} + +/// Appends and drains random number of elements in random order and checks storage invariants. +/// +/// It also changes the maximal number of elements per page dynamically, hence the `page_size`. +fn drain_append_work(ops: Vec, page_size: u8) { + if page_size == 0 { + return + } + + TestExternalities::default().execute_with(|| { + //ValuesPerNewPage::set(&page_size.into()); + let _g = StorageNoopGuard::default(); + let mut totals: BTreeMap = BTreeMap::new(); + + for op in ops.into_iter() { + let total = totals.entry(op.key).or_insert(0); + *total += op.op.exec_map(op.key); + + assert!(*total >= 0); + assert_eq!(NMap::iter((op.key,)).count(), *total as usize); + assert_eq!(*total as u64, NMap::len((op.key,))); + + // We have the assumption that the queue removes the metadata when empty. + if *total == 0 { + assert_eq!(NMap::drain((op.key,)).count(), 0); + assert_eq!(NMap::meta((op.key,)), Default::default()); + } + } + + for (key, total) in totals { + assert_eq!(NMap::drain((key,)).count(), total as usize); + } + // `StorageNoopGuard` checks that there is no storage leaked. + }); +} diff --git a/frame/support/fuzzer/src/lib.rs b/frame/support/fuzzer/src/lib.rs new file mode 100644 index 0000000000000..7a21e44880fe5 --- /dev/null +++ b/frame/support/fuzzer/src/lib.rs @@ -0,0 +1,120 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Shared code for fuzzers. + +pub use frame_support::{ + metadata_ir::{StorageEntryModifierIR, StorageEntryTypeIR, StorageHasherIR}, + pallet_prelude::NMapKey, + parameter_types, + storage::{types::ValueQuery, StorageList as _}, + Blake2_128Concat, +}; +use frame_support::{ + pallet_prelude::{StorageList, StoragePagedNMap}, + storage::{types::StoragePagedList, StorageKeyedList}, + traits::StorageInstance, +}; +pub use sp_io::{hashing::twox_128, TestExternalities}; + +pub enum Op { + Append(Vec), + Drain(u8), +} + +impl arbitrary::Arbitrary<'_> for Op { + fn arbitrary(u: &mut arbitrary::Unstructured<'_>) -> arbitrary::Result { + if u.arbitrary::()? { + Ok(Op::Append(Vec::::arbitrary(u)?)) + } else { + Ok(Op::Drain(u.arbitrary::()?)) + } + } +} + +impl Op { + pub fn exec_list>(self) -> i64 { + match self { + Op::Append(v) => { + let l = v.len(); + List::append_many(v); + l as i64 + }, + Op::Drain(v) => -(List::drain().take(v as usize).count() as i64), + } + } + + pub fn exec_map(self, key: u32) -> i64 { + match self { + Op::Append(v) => { + let l = v.len(); + NMap::append_many((key,), v); + l as i64 + }, + Op::Drain(v) => -(NMap::drain((key,)).take(v as usize).count() as i64), + } + } +} + +pub struct KeyedOp { + pub op: Op, + pub key: u32, +} + +pub enum AllInOneOp { + List(Op), + NMap(KeyedOp), +} + +impl arbitrary::Arbitrary<'_> for KeyedOp { + fn arbitrary(u: &mut arbitrary::Unstructured<'_>) -> arbitrary::Result { + Ok(KeyedOp { op: Op::arbitrary(u)?, key: u.arbitrary::()? }) + } +} + +impl arbitrary::Arbitrary<'_> for AllInOneOp { + fn arbitrary(u: &mut arbitrary::Unstructured<'_>) -> arbitrary::Result { + if u.arbitrary::()? { + Ok(AllInOneOp::List(Op::arbitrary(u)?)) + } else { + Ok(AllInOneOp::NMap(KeyedOp::arbitrary(u)?)) + } + } +} + +parameter_types! { + pub storage HeapSize: u32 = 20; + pub const MaxPages: Option = Some(20); +} + +pub struct Prefix; +impl StorageInstance for Prefix { + fn pallet_prefix() -> &'static str { + "test" + } + const STORAGE_PREFIX: &'static str = "foo"; +} +pub struct Prefix2; +impl StorageInstance for Prefix2 { + fn pallet_prefix() -> &'static str { + "test" + } + const STORAGE_PREFIX: &'static str = "foo2"; +} + +pub type List = StoragePagedList; +pub type NMap = StoragePagedNMap,), u32, HeapSize>; diff --git a/frame/support/procedural/src/pallet/expand/storage.rs b/frame/support/procedural/src/pallet/expand/storage.rs index 253b429bb6eb3..65407e6927233 100644 --- a/frame/support/procedural/src/pallet/expand/storage.rs +++ b/frame/support/procedural/src/pallet/expand/storage.rs @@ -154,7 +154,7 @@ pub fn process_generics(def: &mut Def) -> syn::Result syn::Result<()> { if let Some(QueryKind::ResultQuery(error_path, _)) = storage_def.query_kind.as_ref() { @@ -203,9 +203,17 @@ pub fn process_generics(def: &mut Def) -> syn::Result { + args.args.push(syn::GenericArgument::Type(value.clone())); + let heap_size = heap_size.unwrap_or_else(|| get_default.clone()); + args.args.push(syn::GenericArgument::Type(heap_size)); + let max_pages = max_pages.unwrap_or_else(|| get_default.clone()); + args.args.push(syn::GenericArgument::Type(max_pages)); + }, StorageGenerics::CountedMap { hasher, key, @@ -222,7 +230,7 @@ pub fn process_generics(def: &mut Def) -> syn::Result syn::Result { @@ -256,7 +264,7 @@ pub fn process_generics(def: &mut Def) -> syn::Result syn::Result (1, 2, 3), + Metadata::PagedList { .. } => (1, 2, 3), // FAIL-CI Metadata::NMap { .. } => (2, 3, 4), Metadata::Map { .. } | Metadata::CountedMap { .. } => (3, 4, 5), Metadata::DoubleMap { .. } => (5, 6, 7), @@ -339,6 +348,14 @@ fn augment_final_docs(def: &mut Def) { ); push_string_literal(&doc_line, storage); }, + // FAIL-CI + Metadata::PagedList { value } => { + let doc_line = format!( + "Storage type is [`StoragePagedList`] with value type `{}`.", + value.to_token_stream() + ); + push_string_literal(&doc_line, storage); + }, Metadata::DoubleMap { key1, key2, value } => { let doc_line = format!( "Storage type is [`StorageDoubleMap`] with key1 type {}, key2 type {} and value type {}.", @@ -494,6 +511,9 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { } ) }, + Metadata::PagedList { .. } => { + unreachable!("Getters are forbidden for storage type 'PagedList'.") // FAIL-CI + }, Metadata::CountedMap { key, value } => { let query = match storage.query_kind.as_ref().expect("Checked by def") { QueryKind::OptionQuery => quote::quote_spanned!(storage.attr_span => diff --git a/frame/support/procedural/src/pallet/parse/storage.rs b/frame/support/procedural/src/pallet/parse/storage.rs index 12e06b214b6b6..4fe6167ad9e6a 100644 --- a/frame/support/procedural/src/pallet/parse/storage.rs +++ b/frame/support/procedural/src/pallet/parse/storage.rs @@ -135,6 +135,7 @@ impl PalletStorageAttrInfo { pub enum Metadata { Value { value: syn::Type }, Map { value: syn::Type, key: syn::Type }, + PagedList { value: syn::Type }, CountedMap { value: syn::Type, key: syn::Type }, DoubleMap { value: syn::Type, key1: syn::Type, key2: syn::Type }, NMap { keys: Vec, keygen: syn::Type, value: syn::Type }, @@ -210,6 +211,12 @@ pub enum StorageGenerics { on_empty: Option, max_values: Option, }, + // FAIL-CI + PagedList { + value: syn::Type, + heap_size: Option, + max_pages: Option, + }, CountedMap { hasher: syn::Type, key: syn::Type, @@ -238,6 +245,7 @@ impl StorageGenerics { let res = match self.clone() { Self::DoubleMap { value, key1, key2, .. } => Metadata::DoubleMap { value, key1, key2 }, Self::Map { value, key, .. } => Metadata::Map { value, key }, + Self::PagedList { value, .. } => Metadata::PagedList { value }, Self::CountedMap { value, key, .. } => Metadata::CountedMap { value, key }, Self::Value { value, .. } => Metadata::Value { value }, Self::NMap { keygen, value, .. } => @@ -255,6 +263,8 @@ impl StorageGenerics { Self::CountedMap { query_kind, .. } | Self::Value { query_kind, .. } | Self::NMap { query_kind, .. } => query_kind.clone(), + // A list cannot be queried - only iterated. + Self::PagedList { .. } => None, } } } @@ -262,6 +272,7 @@ impl StorageGenerics { enum StorageKind { Value, Map, + PagedList, CountedMap, DoubleMap, NMap, @@ -401,6 +412,25 @@ fn process_named_generics( max_values: parsed.remove("MaxValues").map(|binding| binding.ty), } }, + StorageKind::PagedList => { + check_generics( + &parsed, + &map_mandatory_generics, + &map_optional_generics, + "StoragePagedList", + args_span, + )?; + + StorageGenerics::PagedList { + // FAIL-CI + value: parsed + .remove("Value") + .map(|binding| binding.ty) + .expect("checked above as mandatory generic"), + heap_size: parsed.remove("HeapSize").map(|binding| binding.ty), + max_pages: parsed.remove("MaxPagex").map(|binding| binding.ty), + } + }, StorageKind::CountedMap => { check_generics( &parsed, @@ -552,6 +582,13 @@ fn process_unnamed_generics( retrieve_arg(4).ok(), use_default_hasher(1)?, ), + StorageKind::PagedList => ( + // FAIL-CI double check + None, + Metadata::PagedList { value: retrieve_arg(1)? }, + None, + true, + ), StorageKind::CountedMap => ( None, Metadata::CountedMap { key: retrieve_arg(2)?, value: retrieve_arg(3)? }, @@ -591,6 +628,8 @@ fn process_generics( let storage_kind = match &*segment.ident.to_string() { "StorageValue" => StorageKind::Value, "StorageMap" => StorageKind::Map, + "StoragePagedList" => StorageKind::PagedList, + "StoragePagedNMap" => StorageKind::PagedList, "CountedStorageMap" => StorageKind::CountedMap, "StorageDoubleMap" => StorageKind::DoubleMap, "StorageNMap" => StorageKind::NMap, diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 230a3ab5a314a..cd43038f1c264 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -1546,9 +1546,10 @@ pub mod pallet_prelude { bounded_vec::BoundedVec, types::{ CountedStorageMap, Key as NMapKey, OptionQuery, ResultQuery, StorageDoubleMap, - StorageMap, StorageNMap, StorageValue, ValueQuery, + StorageMap, StorageNMap, StoragePagedList, StoragePagedNMap, StorageValue, + ValueQuery, }, - StorageList, + StorageKeyedList, StorageList, }, traits::{ BuildGenesisConfig, ConstU32, EnsureOrigin, Get, GetDefault, GetStorageVersion, Hooks, diff --git a/frame/support/src/storage/lists.rs b/frame/support/src/storage/lists.rs new file mode 100644 index 0000000000000..632b66ad0a348 --- /dev/null +++ b/frame/support/src/storage/lists.rs @@ -0,0 +1,141 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Traits and types for non-continous storage types (lists). + +use codec::{EncodeLike, FullCodec}; + +/// A non-continuous container type that can only be iterated. +pub trait StorageList { + /// Iterator for normal and draining iteration. + type Iterator: Iterator; + + /// Append iterator for fast append operations. + type Appender: StorageListAppender; + + /// Number of elements in the list. + fn len() -> u64; + + /// List the elements in append order. + fn iter() -> Self::Iterator; + + /// Drain the elements in append order. + /// + /// Note that this drains a value as soon as it is being inspected. For example `take_while(|_| + /// false)` still drains the first element. This also applies to `peek()`. + fn drain() -> Self::Iterator; + + /// A fast append iterator. + fn appender() -> Self::Appender; + + /// Append a single element. + /// + /// Should not be called repeatedly; use `append_many` instead. + /// Worst case linear `O(len)` with `len` being the number if elements in the list. + fn append_one(item: EncodeLikeValue) + where + EncodeLikeValue: EncodeLike, + { + Self::append_many(core::iter::once(item)); + } + + /// Append many elements. + /// + /// Should not be called repeatedly; use `appender` instead. + /// Worst case linear `O(len + items.count())` with `len` beings the number if elements in the + /// list. + fn append_many(items: I) + where + EncodeLikeValue: EncodeLike, + I: IntoIterator, + { + let mut ap = Self::appender(); + ap.append_many(items); + } +} + +/// A non-continuous container type with a key that can only be iterated. +pub trait StorageKeyedList { + /// Iterator for normal and draining iteration. + type Iterator: Iterator; + + /// Append iterator for fast append operations. + type Appender: StorageListAppender; + + /// Number of elements in the list. + fn len(key: K) -> u64; + + /// List the elements in append order. + fn iter(key: K) -> Self::Iterator; + + /// Drain the elements in append order. + /// + /// Note that this drains a value as soon as it is being inspected. For example `take_while(|_| + /// false)` still drains the first element. This also applies to `peek()`. + fn drain(key: K) -> Self::Iterator; + + /// A fast append iterator. + fn appender(key: K) -> Self::Appender; + + /// Append a single element. + /// + /// Should not be called repeatedly; use `append_many` instead. + /// Worst case linear `O(len)` with `len` being the number if elements in the list. + fn append_one(key: K, item: EncodeLikeValue) + where + EncodeLikeValue: EncodeLike, + { + Self::append_many(key, core::iter::once(item)); + } + + /// Append many elements. + /// + /// Should not be called repeatedly; use `appender` instead. + /// Worst case linear `O(len + items.count())` with `len` beings the number if elements in the + /// list. + fn append_many(key: K, items: I) + where + EncodeLikeValue: EncodeLike, + I: IntoIterator, + { + let mut ap = Self::appender(key); + ap.append_many(items); + } +} + +/// Append iterator to append values to a storage struct. +/// +/// Can be used in situations where appending does not have constant time complexity. +pub trait StorageListAppender { + /// Append a single item in constant time `O(1)`. + fn append(&mut self, item: EncodeLikeValue) + where + EncodeLikeValue: EncodeLike; + + /// Append many items in linear time `O(items.count())`. + // Note: a default impl is provided since `Self` is already assumed to be optimal for single + // append operations. + fn append_many(&mut self, items: I) + where + EncodeLikeValue: EncodeLike, + I: IntoIterator, + { + for item in items.into_iter() { + self.append(item); + } + } +} diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index ea8f1877f7cb4..88be59995fe92 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -46,6 +46,7 @@ pub mod child; #[doc(hidden)] pub mod generator; pub mod hashed; +pub mod lists; pub mod migration; pub mod storage_noop_guard; mod stream_iter; @@ -54,6 +55,8 @@ pub mod types; pub mod unhashed; pub mod weak_bounded_vec; +pub use lists::*; + /// Utility type for converting a storage map into a `Get` impl which returns the maximum /// key size. pub struct KeyLenOf(PhantomData); @@ -160,77 +163,6 @@ pub trait StorageValue { } } -/// A non-continuous container type. -pub trait StorageList { - /// Iterator for normal and draining iteration. - type Iterator: Iterator; - - /// Append iterator for fast append operations. - type Appender: StorageAppender; - - fn len() -> u64; - - /// List the elements in append order. - fn iter() -> Self::Iterator; - - /// Drain the elements in append order. - /// - /// Note that this drains a value as soon as it is being inspected. For example `take_while(|_| - /// false)` still drains the first element. This also applies to `peek()`. - fn drain() -> Self::Iterator; - - /// A fast append iterator. - fn appender() -> Self::Appender; - - /// Append a single element. - /// - /// Should not be called repeatedly; use `append_many` instead. - /// Worst case linear `O(len)` with `len` being the number if elements in the list. - fn append_one(item: EncodeLikeValue) - where - EncodeLikeValue: EncodeLike, - { - Self::append_many(core::iter::once(item)); - } - - /// Append many elements. - /// - /// Should not be called repeatedly; use `appender` instead. - /// Worst case linear `O(len + items.count())` with `len` beings the number if elements in the - /// list. - fn append_many(items: I) - where - EncodeLikeValue: EncodeLike, - I: IntoIterator, - { - let mut ap = Self::appender(); - ap.append_many(items); - } -} - -/// Append iterator to append values to a storage struct. -/// -/// Can be used in situations where appending does not have constant time complexity. -pub trait StorageAppender { - /// Append a single item in constant time `O(1)`. - fn append(&mut self, item: EncodeLikeValue) - where - EncodeLikeValue: EncodeLike; - - /// Append many items in linear time `O(items.count())`. - // Note: a default impl is provided since `Self` is already assumed to be optimal for single - // append operations. - fn append_many(&mut self, items: I) - where - EncodeLikeValue: EncodeLike, - I: IntoIterator, - { - for item in items.into_iter() { - self.append(item); - } - } -} - /// A strongly-typed map in storage. /// /// Details on implementation can be found at [`generator::StorageMap`]. diff --git a/frame/support/src/storage/types/mod.rs b/frame/support/src/storage/types/mod.rs index 3a5bae2e608b7..0cc8fcc752873 100644 --- a/frame/support/src/storage/types/mod.rs +++ b/frame/support/src/storage/types/mod.rs @@ -27,6 +27,8 @@ mod double_map; mod key; mod map; mod nmap; +mod paged_list; +mod paged_nmap; mod value; pub use counted_map::{CountedStorageMap, CountedStorageMapInstance}; @@ -37,6 +39,8 @@ pub use key::{ }; pub use map::StorageMap; pub use nmap::StorageNMap; +pub use paged_list::{StoragePagedList, StoragePagedListMeta}; +pub use paged_nmap::{StoragePagedNMap, StoragePagedNMapMeta}; pub use value::StorageValue; /// Trait implementing how the storage optional value is converted into the queried type. diff --git a/frame/support/src/storage/types/paged_list.rs b/frame/support/src/storage/types/paged_list.rs new file mode 100644 index 0000000000000..4190e14831af5 --- /dev/null +++ b/frame/support/src/storage/types/paged_list.rs @@ -0,0 +1,474 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Paged storage list. + +// links are better than no links - even when they refer to private stuff. +#![allow(rustdoc::private_intra_doc_links)] +#![deny(rustdoc::broken_intra_doc_links)] +#![deny(missing_docs)] +#![deny(unsafe_code)] + +use super::{ + key::KeyGenerator, + paged_nmap::{StoragePagedNMap, StoragePagedNMapAppender, StoragePagedNMapIterator}, +}; +#[cfg(feature = "std")] +use crate::storage::types::paged_nmap::StoragePagedNMapMeta; +use crate::{ + hash::StorageHasher, + metadata_ir::{StorageEntryMetadataIR, StorageEntryTypeIR}, + storage::{ + lists::*, types::paged_nmap::StoragePagedNMapPrefix, EncodeLikeTuple, KeyLenOf, + StorageEntryMetadataBuilder, StoragePrefixedContainer, TupleToEncodedIter, + }, + traits::{StorageInfo, StorageInstance}, + DefaultNoBound, +}; +use codec::{EncodeLike, FullCodec}; +use core::marker::PhantomData; +use frame_support::traits::Get; +use sp_std::prelude::*; + +/// A list in storage that stores items in a paged manner. +#[derive(DefaultNoBound)] +pub struct StoragePagedList { + /// Phantom data. + pub _phantom: PhantomData<(Prefix, Value, HeapSize, MaxPages)>, +} + +/// The metadata of a [`StoragePagedList`]. +pub type StoragePagedListMeta = + StoragePagedNMapMeta; + +/// Iterator type to inspect elements of a [`StoragePagedList`]. +pub struct StoragePagedListIterator { + inner: StoragePagedNMapIterator, +} + +/// Iterator type to append elements to a [`StoragePagedList`]. +pub struct StoragePagedListAppender { + inner: StoragePagedNMapAppender, +} + +/// An implementation of [`KeyGenerator`] that uses `()` as key type and does nothing. +pub struct EmptyKeyGen; + +/// The key type of [`EmptyKeyGen`]. +pub type EmptyKey = ((),); + +impl KeyGenerator for EmptyKeyGen { + type Key = (); + type KArg = EmptyKey; + type HashFn = Box Vec>; + type HArg = (); + + const HASHER_METADATA: &'static [crate::metadata_ir::StorageHasherIR] = &[]; + + fn final_key + TupleToEncodedIter>(_key: KArg) -> Vec { + Vec::new() + } + + fn migrate_key + TupleToEncodedIter>( + _key: &KArg, + _hash_fns: Self::HArg, + ) -> Vec { + Vec::new() + } +} + +impl StorageList + for StoragePagedList +where + Prefix: StorageInstance, + Value: FullCodec, + HeapSize: Get, +{ + type Iterator = StoragePagedListIterator; + type Appender = StoragePagedListAppender; + + fn len() -> u64 { + as StorageKeyedList< + ((),), + Value, + >>::len(((),)) + } + + fn iter() -> Self::Iterator { + StoragePagedListIterator { inner: as StorageKeyedList>::iter(((),)) } + } + + fn drain() -> Self::Iterator { + StoragePagedListIterator { inner: as StorageKeyedList>::drain(((),)) } + } + + fn appender() -> Self::Appender { + StoragePagedListAppender { inner: as StorageKeyedList>::appender(((),)) } + } +} + +impl Iterator + for StoragePagedListIterator +where + Prefix: StorageInstance, + Value: FullCodec, + HeapSize: Get, +{ + type Item = Value; + + fn next(&mut self) -> Option { + self.inner.next() + } +} + +impl StorageListAppender + for StoragePagedListAppender +where + Prefix: StorageInstance, + Value: FullCodec, + HeapSize: Get, +{ + fn append(&mut self, item: EncodeLikeValue) + where + EncodeLikeValue: EncodeLike, + { + self.inner.append(item) + } +} + +// Needed for FRAME +impl crate::traits::StorageInfoTrait + for StoragePagedList +where + Prefix: StorageInstance, + Value: FullCodec, + HeapSize: Get, + MaxPages: Get>, +{ + fn storage_info() -> Vec { + vec![StorageInfo { + pallet_name: ::module_prefix().to_vec(), + storage_name: Self::storage_prefix().to_vec(), + prefix: Self::final_prefix().to_vec(), + max_values: MaxPages::get(), + max_size: Some(HeapSize::get()), + }] + } +} + +// Needed for FRAME +impl StoragePrefixedContainer + for StoragePagedList +where + Prefix: StorageInstance, + Value: FullCodec, + HeapSize: Get, +{ + fn module_prefix() -> &'static [u8] { + // There is no difference between the prefices of a List and NMap. + StoragePagedNMapPrefix::::module_prefix() + } + + fn storage_prefix() -> &'static [u8] { + StoragePagedNMapPrefix::::storage_prefix() + } +} + +// Needed for FRAME +impl StorageEntryMetadataBuilder + for StoragePagedList +where + Prefix: StorageInstance, + Value: FullCodec + scale_info::StaticTypeInfo, +{ + fn build_metadata(docs: Vec<&'static str>, entries: &mut Vec) { + let docs = if cfg!(feature = "no-metadata-docs") { vec![] } else { docs }; + + let entry = StorageEntryMetadataIR { + name: Prefix::STORAGE_PREFIX, + modifier: crate::storage::types::StorageEntryModifierIR::Optional, // FAIL-CI + ty: StorageEntryTypeIR::Map { + hashers: vec![crate::Twox64Concat::METADATA], + key: scale_info::meta_type::(), + value: scale_info::meta_type::>(), + }, + default: vec![], // FAIL-CI + docs, + }; + + entries.push(entry); + } +} + +// Needed for FRAME +impl Get + for KeyLenOf> +where + Prefix: StorageInstance, + Value: FullCodec, + HeapSize: Get, +{ + fn get() -> u32 { + super::paged_nmap::page_key::(((),), u32::MAX).len() as u32 + } +} + +// Test helpers: +#[cfg(feature = "std")] +#[allow(dead_code)] +impl StoragePagedList +where + Prefix: StorageInstance, + Value: FullCodec, + HeapSize: Get, +{ + /// Return the metadata struct of this list. + pub fn meta() -> StoragePagedNMapMeta { + // Use default here to not require a setup migration. + StoragePagedNMap::::meta(((),)) + } + + /// Return the elements of the list. + pub fn as_vec() -> Vec { + >::iter().collect() + } + + /// Return and remove the elements of the list. + pub fn as_drained_vec() -> Vec { + >::drain().collect() + } +} + +/// Prelude for (doc)tests. +#[cfg(feature = "std")] +#[allow(dead_code)] +pub(crate) mod mock { + pub use super::*; + pub use crate::storage::{types::paged_nmap::*, StoragePrefixedContainer}; + pub use codec::{Compact, Decode, Encode}; + pub use frame_support::{ + metadata_ir::{StorageEntryModifierIR, StorageEntryTypeIR, StorageHasherIR}, + parameter_types, + storage::{types::ValueQuery, StorageList as _}, + StorageNoopGuard, + }; + pub use sp_io::{hashing::twox_128, TestExternalities}; + + pub fn page_key(page: PageIndex) -> Vec { + crate::storage::types::paged_nmap::page_key::(((),), page) + } + + pub fn meta_key() -> Vec { + crate::storage::types::paged_nmap::meta_key::(((),)) + } + + parameter_types! { + pub const HeapSize: u32 = 20; + pub const MaxPages: Option = Some(20); + } + + pub struct Prefix; + impl StorageInstance for Prefix { + fn pallet_prefix() -> &'static str { + "FinalKeysNone" + } + const STORAGE_PREFIX: &'static str = "PagedList"; + } + pub struct Prefix2; + impl StorageInstance for Prefix2 { + fn pallet_prefix() -> &'static str { + "FinalKeysNone" + } + const STORAGE_PREFIX: &'static str = "PagedList"; + } + + pub type List = StoragePagedList; + pub type KeyGen = EmptyKeyGen; + pub type Key = ((),); + pub type ListCompact = StoragePagedList, HeapSize>; +} + +#[cfg(test)] +mod tests { + use super::mock::*; + + #[test] + fn append_works() { + TestExternalities::default().execute_with(|| { + List::append_many(0..1000); + assert_eq!(List::as_vec(), (0..1000).collect::>()); + }); + } + + /// Draining all works. + #[test] + fn simple_drain_works() { + TestExternalities::default().execute_with(|| { + let _g = StorageNoopGuard::default(); // All in all a No-Op + List::append_many(0..1000); + + assert_eq!(List::as_drained_vec(), (0..1000).collect::>()); + + assert_eq!(List::meta(), Default::default()); + + // all gone + assert_eq!(List::as_vec(), Vec::::new()); + // Need to delete the metadata manually. + StoragePagedNMapMeta::::delete(((),)); + }); + } + + /// Drain half of the elements and iterator the rest. + #[test] + fn partial_drain_works() { + TestExternalities::default().execute_with(|| { + List::append_many(0..100); + + let vals = List::drain().take(50).collect::>(); + assert_eq!(vals, (0..50).collect::>()); + + let meta = List::meta(); + // Will switch over to `10/0`, but will in the next call. + assert_eq!((meta.first_page, meta.first_value_offset), (10, 0)); + assert_eq!(List::len(), 50); + + // 50 gone, 50 to go + assert_eq!(List::as_vec(), (50..100).collect::>()); + }); + } + + /// Draining, appending and iterating work together. + #[test] + fn drain_append_iter_works() { + TestExternalities::default().execute_with(|| { + for r in 1..=100 { + List::append_many(0..12); + List::append_many(0..12); + + let dropped = List::drain().take(12).collect::>(); + assert_eq!(dropped, (0..12).collect::>()); + + assert_eq!(List::as_vec(), (0..12).cycle().take(r * 12).collect::>()); + assert_eq!(List::len() as usize, r * 12); + } + }); + } + + /// Pages are removed ASAP. + #[test] + fn drain_eager_page_removal() { + TestExternalities::default().execute_with(|| { + List::append_many(0..9); + + assert!(sp_io::storage::exists(&page_key::(0))); + assert!(sp_io::storage::exists(&page_key::(1))); + + assert_eq!(List::drain().take(5).count(), 5); + // Page 0 is eagerly removed. + assert!(!sp_io::storage::exists(&page_key::(0))); + assert!(sp_io::storage::exists(&page_key::(1))); + }); + } + + /// Appending encodes pages as `Vec`. + #[test] + fn append_storage_layout() { + TestExternalities::default().execute_with(|| { + List::append_many(0..9); + + let key = page_key::(0); + let raw = sp_io::storage::get(&key).expect("Page should be present"); + let as_vec = Vec::::decode(&mut &raw[..]).unwrap(); + assert_eq!(as_vec.len(), 5, "First page contains 5"); + + let key = page_key::(1); + let raw = sp_io::storage::get(&key).expect("Page should be present"); + let as_vec = Vec::::decode(&mut &raw[..]).unwrap(); + assert_eq!(as_vec.len(), 4, "Second page contains 4"); + + let meta = sp_io::storage::get(&meta_key::()).expect("Meta should be present"); + let meta: StoragePagedNMapMeta = + codec::Decode::decode(&mut &meta[..]).unwrap(); + assert_eq!(meta.first_page, 0); + assert_eq!(meta.first_value_offset, 0); + assert_eq!(meta.last_page, 1); + assert_eq!(meta.last_page_byte_offset, 16); + + let page = Page::::from_storage::(((),), 0, 0).unwrap(); + assert_eq!(page.index, 0); + assert_eq!(page.values.count(), 5); + + let page = Page::::from_storage::(((),), 1, 0).unwrap(); + assert_eq!(page.index, 1); + assert_eq!(page.values.count(), 4); + }); + } + + #[test] + fn final_prefix_correct() { + let got = StoragePagedNMapPrefix::::final_prefix(); + let want = [twox_128(b"FinalKeysNone"), twox_128(b"PagedList")].concat(); + + assert_eq!(want, got); + } + + #[test] + fn page_key_correct() { + let got = page_key::(0); + let pallet_prefix = StoragePagedNMapPrefix::::final_prefix(); + let want = (pallet_prefix, b"page", 0u32).encode(); + + assert_eq!(want.len(), 32 + 4 + 4); + assert!(want.starts_with(&pallet_prefix[..])); + assert_eq!(got, want); + } + + #[test] + fn meta_key_correct() { + let got = meta_key::(); + let pallet_prefix = StoragePagedNMapPrefix::::final_prefix(); + let want = (pallet_prefix, b"meta").encode(); + + assert_eq!(want.len(), 32 + 4); + assert!(want.starts_with(&pallet_prefix[..])); + assert_eq!(got, want); + } + + #[test] + fn peekable_drain_also_deletes() { + TestExternalities::default().execute_with(|| { + List::append_many(0..10); + + let mut iter = List::drain().peekable(); + assert_eq!(iter.peek(), Some(&0)); + // `peek` does remove one element... + assert_eq!(List::iter().count(), 9); + }); + } + + #[test] + fn heap_size_works() { + TestExternalities::default().execute_with(|| { + List::append_many(0..100); + ListCompact::append_many((0..100).map(|i| Compact(i))); + + // They should have the same number of items: + assert_eq!(List::meta().total_items, ListCompact::meta().total_items); + // But the compact variant should have more values per page: + }); + } +} diff --git a/frame/support/src/storage/types/paged_nmap.rs b/frame/support/src/storage/types/paged_nmap.rs new file mode 100644 index 0000000000000..badd03a2ede09 --- /dev/null +++ b/frame/support/src/storage/types/paged_nmap.rs @@ -0,0 +1,795 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Paged storage n-map. + +// links are better than no links - even when they refer to private stuff. +#![allow(rustdoc::private_intra_doc_links)] +#![deny(rustdoc::broken_intra_doc_links)] +#![deny(missing_docs)] +#![deny(unsafe_code)] + +use super::key::KeyGenerator; +use crate::{ + metadata_ir::{StorageEntryMetadataIR, StorageEntryTypeIR}, + storage::{types::StorageEntryMetadataBuilder, EncodeLikeTuple, TupleToEncodedIter}, + traits::{StorageInfo, StorageInstance}, + StorageHasher, +}; +use codec::{Decode, Encode, EncodeLike, FullCodec}; +use core::marker::PhantomData; +use frame_support::{ + defensive, storage::StoragePrefixedContainer, traits::Get, CloneNoBound, DebugNoBound, + DefaultNoBound, EqNoBound, PartialEqNoBound, +}; +use sp_runtime::traits::Saturating; +use sp_std::prelude::*; + +pub type PageIndex = u32; +pub type ValueIndex = u32; + +/// A paginated storage list. +/// +/// # Motivation +/// +/// This type replaces `StorageValue>` in situations where only iteration and appending is +/// needed. There are a few places where this is the case. A paginated structure reduces the memory +/// usage when a storage transactions needs to be rolled back. The main motivation is therefore a +/// reduction of runtime memory on storage transaction rollback. Should be configured such that the +/// size of a page is about 64KiB. This can only be ensured when `V` implements `MaxEncodedLen`. +/// +/// # Implementation +/// +/// The metadata of this struct is stored in [`StoragePagedNMapMeta`]. The data is stored in +/// [`Page`]s. +/// +/// Each [`Page`] holds at most `HeapSize` values in its `values` vector. The last page is +/// the only one that could have less than `HeapSize` values. +/// **Iteration** happens by starting +/// at [`first_page`][StoragePagedNMapMeta::first_page]/ +/// [`first_value_offset`][StoragePagedNMapMeta::first_value_offset] and incrementing these indices +/// as long as there are elements in the page and there are pages in storage. All elements of a page +/// are loaded once a page is read from storage. Iteration then happens on the cached elements. This +/// reduces the number of storage `read` calls on the overlay. **Appending** to the list happens by +/// appending to the last page by utilizing [`sp_io::storage::append`]. It allows to directly extend +/// the elements of `values` vector of the page without loading the whole vector from storage. A new +/// page is instantiated once [`Page::next`] overflows `HeapSize`. Its vector will also be +/// created through [`sp_io::storage::append`]. **Draining** advances the internal indices identical +/// to Iteration. It additionally persists the increments to storage and thereby 'drains' elements. +/// Completely drained pages are deleted from storage. +/// +/// # Further Observations +/// +/// - The encoded layout of a page is exactly its [`Page::values`]. The [`Page::next`] offset is +/// stored in the [`StoragePagedNMapMeta`] instead. There is no particular reason for this, +/// besides having all management state handy in one location. +/// - The PoV complexity of iterating compared to a `StorageValue>` is improved for +/// "shortish" iterations and worse for total iteration. The append complexity is identical in the +/// asymptotic case when using an `Appender`, and worse in all. For example when appending just +/// one value. +/// - It does incur a read overhead on the host side as compared to a `StorageValue>`. +#[derive(Default)] +pub struct StoragePagedNMap { + /// Phantom data. + pub _phantom: PhantomData<(Prefix, Key, Value, HeapSize, MaxPages)>, +} + +// FAIL-CI: TODO add test for Value MEL bound to be <= HeapSize + +/// The state of a [`StoragePagedNMap`]. +/// +/// This struct doubles as [`frame_support::storage::StorageList::Appender`]. +#[derive( + Encode, Decode, CloneNoBound, PartialEqNoBound, EqNoBound, DebugNoBound, DefaultNoBound, +)] +pub struct StoragePagedNMapMeta { + /// The first page that could contain a value. + /// + /// Can be >0 when pages were deleted. + pub first_page: PageIndex, + /// The first index inside `first_page` that could contain a value. + /// + /// Can be >0 when values were deleted. + pub first_value_offset: ValueIndex, + + /// The last page that could contain data. + /// + /// Appending starts at this page index. + pub last_page: PageIndex, + /// The last value inside `last_page` that could contain a value. + /// + /// Appending starts at this index. If the page does not hold a value at this index, then the + /// whole list is empty. The only case where this can happen is when both are `0`. + pub last_page_byte_offset: u32, + + /// The total number of items currently present in the list. + pub total_items: u64, + /// Phantom data. + pub _phantom: PhantomData<(Prefix, Key, KArg, Value, HeapSize, MaxPages)>, +} + +pub struct StoragePagedNMapAppender { + /// The inner metadata. + pub meta: StoragePagedNMapMeta, + /// The key that values will be appended to. + pub key: KArg, +} + +impl + frame_support::storage::StorageListAppender + for StoragePagedNMapAppender +where + Prefix: StorageInstance, + Key: KeyGenerator, + KArg: EncodeLikeTuple + TupleToEncodedIter + Clone, + Value: FullCodec, + HeapSize: Get, +{ + fn append(&mut self, item: EncodeLikeValue) + where + EncodeLikeValue: EncodeLike, + { + self.append_one(item); + } +} + +impl + StoragePagedNMapMeta +where + Prefix: StorageInstance, + Key: KeyGenerator, + KArg: EncodeLikeTuple + TupleToEncodedIter + Clone, + Value: FullCodec, + HeapSize: Get, +{ + /// Read the metadata from storage. + pub fn from_storage(key: KArg) -> Option { + let mk = Self::storage_key(key); + + sp_io::storage::get(&mk).and_then(|raw| Self::decode(&mut &raw[..]).ok()) + } + + /// The key under which the metadata is stored. + pub fn storage_key(key: KArg) -> Vec { + meta_key::(key) + } + + /// Write the metadata to storage. + pub fn store(&self, k: KArg) { + let key = Self::storage_key(k); + self.using_encoded(|enc| sp_io::storage::set(&key, enc)); + } + + /// Reset the metadata to default and delete it from storage. + pub fn reset(&mut self, key: KArg) { + *self = Default::default(); + Self::delete(key); + } + + /// Delete the metadata from storage. + pub fn delete(key: KArg) { + sp_io::storage::clear(&Self::storage_key(key)); + } +} + +impl + StoragePagedNMapAppender +where + Prefix: StorageInstance, + Key: KeyGenerator, + KArg: EncodeLikeTuple + TupleToEncodedIter + Clone, + Value: FullCodec, + HeapSize: Get, +{ + pub fn append_one(&mut self, item: EncodeLikeValue) + where + EncodeLikeValue: EncodeLike, + { + let enc_size = item.encoded_size() as u32; + if (self.meta.last_page_byte_offset.saturating_add(enc_size)) > HeapSize::get() { + self.meta.last_page.saturating_inc(); + self.meta.last_page_byte_offset = 0; + } + let pk = page_key::(self.key.clone(), self.meta.last_page); + self.meta.last_page_byte_offset.saturating_accrue(enc_size); + self.meta.total_items.saturating_inc(); + sp_io::storage::append(&pk, item.encode()); + self.meta.store(self.key.clone()); + } +} + +/// A page that was decoded from storage and caches its values. +pub struct Page { + /// The index of the page. + pub(crate) index: PageIndex, + /// The remaining values of the page, to be drained by [`Page::next`]. + pub(crate) values: sp_std::iter::Skip>, + + pub(crate) key: KArg, + _phantom: PhantomData, +} + +impl< + Key: KeyGenerator, + KArg: EncodeLikeTuple + TupleToEncodedIter + Clone, + V: FullCodec, + > Page +{ + /// Read the page with `index` from storage and assume the first value at `value_index`. + pub fn from_storage( + k: KArg, + index: PageIndex, + value_index: ValueIndex, + ) -> Option { + let key = page_key::(k.clone(), index); + let values = sp_io::storage::get(&key) + .and_then(|raw| sp_std::vec::Vec::::decode(&mut &raw[..]).ok())?; + if values.is_empty() { + // Dont create empty pages. + return None + } + let values = values.into_iter().skip(value_index as usize); + + Some(Self { index, values, key: k, _phantom: PhantomData }) + } + + /// Whether no more values can be read from this page. + pub fn is_eof(&self) -> bool { + self.values.len() == 0 + } + + /// Delete this page from storage. + pub fn delete(&self) { + delete_page::(self.key.clone(), self.index); + } +} + +/// Delete a page with `index` from storage. +// Does not live under `Page` since it does not require the `Value` generic. +pub(crate) fn delete_page< + Prefix: StorageInstance, + Key: KeyGenerator, + KArg: EncodeLikeTuple + TupleToEncodedIter + Clone, +>( + key: KArg, + index: PageIndex, +) { + let key = page_key::(key, index); + sp_io::storage::clear(&key); +} + +/// Storage key of a page with `index`. +// Does not live under `Page` since it does not require the `Value` generic. +pub(crate) fn page_key< + Prefix: StorageInstance, + Key: KeyGenerator, + KArg: EncodeLikeTuple + TupleToEncodedIter + Clone, +>( + key: KArg, + index: PageIndex, +) -> Vec { + let k1 = StoragePagedNMapPrefix::::final_prefix(); + let k2 = Key::final_key(key); + + [&k1[..], &k2[..], b"page", &index.encode()[..]].concat() +} + +pub(crate) fn meta_key< + Prefix: StorageInstance, + Key: KeyGenerator, + KArg: EncodeLikeTuple + TupleToEncodedIter + Clone, +>( + key: KArg, +) -> Vec { + let k1 = StoragePagedNMapPrefix::::final_prefix(); + let k2 = Key::final_key(key); + + [&k1[..], &k2[..], b"meta"].concat() +} + +impl Iterator for Page { + type Item = V; + + fn next(&mut self) -> Option { + self.values.next() + } +} + +/// Iterates over values of a [`StoragePagedNMap`]. +/// +/// Can optionally drain the iterated values. +pub struct StoragePagedNMapIterator { + // Design: we put the Page into the iterator to have fewer storage look-ups. Yes, these + // look-ups would be cached anyway, but bugging the overlay on each `.next` call still seems + // like a poor trade-off than caching it in the iterator directly. Iterating and modifying is + // not allowed at the same time anyway, just like with maps. Note: if Page is empty then + // the iterator did not find any data upon setup or ran out of pages. + page: Option>, + drain: bool, + meta: StoragePagedNMapMeta, + key: KArg, +} + +impl + StoragePagedNMapIterator +where + Prefix: StorageInstance, + Key: KeyGenerator, + KArg: EncodeLikeTuple + TupleToEncodedIter + Clone, + Value: FullCodec, + HeapSize: Get, +{ + /// Read self from the storage. + pub fn from_meta( + meta: StoragePagedNMapMeta, + key: KArg, + drain: bool, + ) -> Self { + let page = Page::::from_storage::( + key.clone(), + meta.first_page, + meta.first_value_offset, + ); + Self { page, drain, meta, key } + } +} + +impl Iterator + for StoragePagedNMapIterator +where + Prefix: StorageInstance, + Key: KeyGenerator, + KArg: EncodeLikeTuple + TupleToEncodedIter + Clone, + Value: FullCodec, + HeapSize: Get, +{ + type Item = Value; + + fn next(&mut self) -> Option { + let page = self.page.as_mut()?; + let value = match page.next() { + Some(value) => value, + None => { + defensive!("There are no empty pages in storage; nuking the list"); + self.meta.reset(self.key.clone()); + self.page = None; + return None + }, + }; + + if page.is_eof() { + if self.drain { + page.delete::(); + self.meta.first_value_offset = 0; + self.meta.first_page.saturating_inc(); + self.meta.total_items.saturating_dec(); + } + + debug_assert!(!self.drain || self.meta.first_page == page.index + 1); + self.page = + Page::from_storage::(self.key.clone(), page.index.saturating_add(1), 0); + if self.drain { + if self.page.is_none() { + self.meta.reset(self.key.clone()); + } else { + self.meta.store(self.key.clone()); + } + } + } else { + if self.drain { + self.meta.first_value_offset.saturating_inc(); + self.meta.total_items.saturating_dec(); + self.meta.store(self.key.clone()); + } + } + Some(value) + } +} + +impl + frame_support::storage::StorageKeyedList + for StoragePagedNMap +where + Prefix: StorageInstance, + Key: KeyGenerator, + KArg: EncodeLikeTuple + TupleToEncodedIter + Clone, + Value: FullCodec, + HeapSize: Get, +{ + type Iterator = StoragePagedNMapIterator; + type Appender = StoragePagedNMapAppender; + + fn len(key: KArg) -> u64 { + Self::meta(key).total_items + } + + fn iter(key: KArg) -> Self::Iterator { + StoragePagedNMapIterator::from_meta(Self::meta(key.clone()), key, false) + } + + fn drain(key: KArg) -> Self::Iterator { + StoragePagedNMapIterator::from_meta(Self::meta(key.clone()), key, true) + } + + fn appender(key: KArg) -> Self::Appender { + Self::appender(key) + } +} + +impl + StoragePagedNMap +where + Prefix: StorageInstance, + Key: KeyGenerator, + Value: FullCodec, + HeapSize: Get, +{ + /// Return the metadata of the map. + #[cfg(feature = "std")] + pub fn meta + TupleToEncodedIter + Clone>( + key: KArg, + ) -> StoragePagedNMapMeta { + // Use default here to not require a setup migration. + StoragePagedNMapMeta::from_storage(key).unwrap_or_default() + } + + /// Provides a fast append iterator. + /// + /// The list should not be modified while appending. Also don't call it recursively. + fn appender + TupleToEncodedIter + Clone>( + key: KArg, + ) -> StoragePagedNMapAppender { + StoragePagedNMapAppender { meta: Self::meta(key.clone()), key } + } + + /// Return the elements of the list. + #[cfg(feature = "std")] + pub fn as_vec + TupleToEncodedIter + Clone>( + key: KArg, + ) -> Vec { + >::iter(key).collect() + } + + /// Return and remove the elements of the list. + #[cfg(feature = "std")] + pub fn as_drained_vec + TupleToEncodedIter + Clone>( + key: KArg, + ) -> Vec { + >::drain(key).collect() + } +} + +/// Provides the final prefix for a [`StoragePagedNMap`]. +/// +/// It solely exists so that when re-using it from the iterator and meta struct, none of the un-used +/// generics bleed through. Otherwise when only having the `StoragePrefixedContainer` implementation +/// on the list directly, the iterator and metadata need to muster *all* generics, even the ones +/// that are completely useless for prefix calculation. +pub(crate) struct StoragePagedNMapPrefix(PhantomData); + +impl frame_support::storage::StoragePrefixedContainer for StoragePagedNMapPrefix +where + Prefix: StorageInstance, +{ + fn module_prefix() -> &'static [u8] { + Prefix::pallet_prefix().as_bytes() + } + + fn storage_prefix() -> &'static [u8] { + Prefix::STORAGE_PREFIX.as_bytes() + } +} + +impl frame_support::storage::StoragePrefixedContainer + for StoragePagedNMap +where + Prefix: StorageInstance, + Value: FullCodec, + HeapSize: Get, +{ + fn module_prefix() -> &'static [u8] { + StoragePagedNMapPrefix::::module_prefix() + } + + fn storage_prefix() -> &'static [u8] { + StoragePagedNMapPrefix::::storage_prefix() + } +} + +// Needed for FRAME +impl crate::traits::StorageInfoTrait + for StoragePagedNMap +where + Prefix: StorageInstance, + Value: FullCodec + codec::MaxEncodedLen, + HeapSize: Get, + MaxPages: Get>, +{ + fn storage_info() -> Vec { + vec![StorageInfo { + pallet_name: Self::module_prefix().to_vec(), + storage_name: Self::storage_prefix().to_vec(), + prefix: Self::final_prefix().to_vec(), + max_values: MaxPages::get(), + max_size: Some(HeapSize::get()), + }] + } +} + +// Needed for FRAME +impl StorageEntryMetadataBuilder + for StoragePagedNMap +where + Prefix: StorageInstance, + Value: FullCodec + scale_info::StaticTypeInfo, +{ + fn build_metadata(docs: Vec<&'static str>, entries: &mut Vec) { + let docs = if cfg!(feature = "no-metadata-docs") { vec![] } else { docs }; + + let entry = StorageEntryMetadataIR { + name: Prefix::STORAGE_PREFIX, + modifier: crate::storage::types::StorageEntryModifierIR::Optional, // FAIL-CI + ty: StorageEntryTypeIR::Map { + hashers: vec![crate::Twox64Concat::METADATA], + key: scale_info::meta_type::(), + value: scale_info::meta_type::>(), + }, + default: vec![], // FAIL-CI + docs, + }; + + entries.push(entry); + } +} + +// Needed for FRAME +/*impl Get + for KeyLenOf> +where + Prefix: StorageInstance, + HeapSize: Get, + MaxPages: Get>, +{ + fn get() -> u32 { + page_key::(&None, u32::MAX).len() as u32 + } +}*/ + +/// Prelude for (doc)tests. +#[cfg(feature = "std")] +#[allow(dead_code)] +pub(crate) mod mock { + pub use super::*; + pub use crate::{ + pallet_prelude::{ConstU32, NMapKey}, + storage::StorageKeyedList, + }; + use codec::Compact; + pub use frame_support::{ + metadata_ir::{StorageEntryModifierIR, StorageEntryTypeIR, StorageHasherIR}, + parameter_types, + storage::{types::ValueQuery, StorageList as _}, + Blake2_128Concat, StorageNoopGuard, + }; + pub use sp_io::{hashing::twox_128, TestExternalities}; + + parameter_types! { + pub const HeapSize: u32 = 20; + pub const MaxPages: Option = Some(20); + } + + pub struct Prefix; + impl StorageInstance for Prefix { + fn pallet_prefix() -> &'static str { + "FinalKeysNone" + } + const STORAGE_PREFIX: &'static str = "PagedMap"; + } + pub struct Prefix2; + impl StorageInstance for Prefix2 { + fn pallet_prefix() -> &'static str { + "FinalKeysNone" + } + const STORAGE_PREFIX: &'static str = "PagedMap2"; + } + + pub type NMap = StoragePagedNMap; + pub type KeyGen = (NMapKey,); + pub type Key = (u32,); + + pub type NMapCompact = + StoragePagedNMap,), Compact, HeapSize>; +} + +#[cfg(test)] +mod tests { + use super::mock::*; + + #[test] + fn append_works() { + TestExternalities::default().execute_with(|| { + >::append_many((123,), 0..1000); + assert_eq!(NMap::as_vec((123,)), (0..1000).collect::>()); + }); + } + + /// Draining all works. + #[test] + fn simple_drain_works() { + TestExternalities::default().execute_with(|| { + let _g = StorageNoopGuard::default(); // All in all a No-Op + NMap::append_many((123,), 0..1000); + + assert_eq!(NMap::as_drained_vec((123,)), (0..1000).collect::>()); + + assert_eq!(NMap::meta((123,)), Default::default()); + + // all gone + assert_eq!(NMap::as_vec((123,)), Vec::::new()); + // Need to delete the metadata manually. + StoragePagedNMapMeta::,), (u32,), u32, (), ()>::delete((123,)); + }); + } + + /// Drain half of the elements and iterator the rest. + #[test] + fn partial_drain_works() { + TestExternalities::default().execute_with(|| { + NMap::append_many((123,), 0..100); + + let vals = NMap::drain((123,)).take(50).collect::>(); + assert_eq!(vals, (0..50).collect::>()); + + let meta = NMap::meta((123,)); + // Will switch over to `10/0`, but will in the next call. + assert_eq!((meta.first_page, meta.first_value_offset), (10, 0)); + assert_eq!(NMap::len((123,)), 50); + + // 50 gone, 50 to go + assert_eq!(NMap::as_vec((123,)), (50..100).collect::>()); + }); + } + + /// Draining, appending and iterating work together. + #[test] + fn drain_append_iter_works() { + TestExternalities::default().execute_with(|| { + for r in 1..=100 { + NMap::append_many((123,), 0..12); + NMap::append_many((123,), 0..12); + + let dropped = NMap::drain((123,)).take(12).collect::>(); + assert_eq!(dropped, (0..12).collect::>()); + + assert_eq!(NMap::as_vec((123,)), (0..12).cycle().take(r * 12).collect::>()); + assert_eq!(NMap::len((123,)) as usize, r * 12); + } + }); + } + + /// Pages are removed ASAP. + #[test] + fn drain_eager_page_removal() { + TestExternalities::default().execute_with(|| { + NMap::append_many((123,), 0..9); + + assert!(sp_io::storage::exists(&page_key::((123,), 0))); + assert!(sp_io::storage::exists(&page_key::((123,), 1))); + + assert_eq!(NMap::drain((123,)).take(5).count(), 5); + // Page 0 is eagerly removed. + assert!(!sp_io::storage::exists(&page_key::((123,), 0))); + assert!(sp_io::storage::exists(&page_key::((123,), 1))); + }); + } + + /// Appending encodes pages as `Vec`. + #[test] + fn append_storage_layout() { + TestExternalities::default().execute_with(|| { + NMap::append_many((123,), 0..9); + + let key = page_key::((123,), 0); + let raw = sp_io::storage::get(&key).expect("Page should be present"); + let as_vec = Vec::::decode(&mut &raw[..]).unwrap(); + assert_eq!(as_vec.len(), 5, "First page contains 5"); + + let key = page_key::((123,), 1); + let raw = sp_io::storage::get(&key).expect("Page should be present"); + let as_vec = Vec::::decode(&mut &raw[..]).unwrap(); + assert_eq!(as_vec.len(), 4, "Second page contains 4"); + + let meta = sp_io::storage::get(&meta_key::((123,))) + .expect("Meta should be present"); + let meta: StoragePagedNMapMeta = + Decode::decode(&mut &meta[..]).unwrap(); + assert_eq!(meta.first_page, 0); + assert_eq!(meta.first_value_offset, 0); + assert_eq!(meta.last_page, 1); + assert_eq!(meta.last_page_byte_offset, 16); + + let page = Page::::from_storage::((123,), 0, 0).unwrap(); + assert_eq!(page.index, 0); + assert_eq!(page.values.count(), 5); + + let page = Page::::from_storage::((123,), 1, 0).unwrap(); + assert_eq!(page.index, 1); + assert_eq!(page.values.count(), 4); + }); + } + + #[test] + fn final_prefix_correct() { + let got = StoragePagedNMapPrefix::::final_prefix(); + let want = [twox_128(b"FinalKeysNone"), twox_128(b"PagedMap")].concat(); + + assert_eq!(want, got); + } + + #[test] + fn page_key_correct() { + let got = page_key::((123,), 11); + let pallet_prefix = StoragePagedNMapPrefix::::final_prefix(); + let hashed_key = Blake2_128Concat::hash(&123u32.encode()); + let want = [&pallet_prefix[..], &hashed_key, b"page", 11u32.encode().as_slice()].concat(); + + assert_eq!(want.len(), 32 + (16 + 4) + 4 + 4); + assert!(want.starts_with(&pallet_prefix[..])); + assert_eq!(got, want); + } + + #[test] + fn meta_key_correct() { + let got = meta_key::((123,)); + let pallet_prefix = StoragePagedNMapPrefix::::final_prefix(); + let hashed_key = Blake2_128Concat::hash(&123u32.encode()); + let want = [&pallet_prefix[..], hashed_key.as_slice(), b"meta"].concat(); + + assert_eq!(want.len(), 32 + (16 + 4) + 4); + assert!(want.starts_with(&pallet_prefix[..])); + assert_eq!(got.len(), want.len()); + assert_eq!(got, want); + } + + #[test] + fn peekable_drain_also_deletes() { + TestExternalities::default().execute_with(|| { + NMap::append_many((123,), 0..10); + + let mut iter = NMap::drain((123,)).peekable(); + assert_eq!(iter.peek(), Some(&0)); + // `peek` does remove one element... + assert_eq!(NMap::iter((123,)).count(), 9); + }); + } + + #[test] + fn heap_size_works() { + use codec::Compact; + TestExternalities::default().execute_with(|| { + NMap::append_many((123,), 0..100); + // FAIL-CI add more prefix + NMapCompact::append_many((123,), (0..100).map(|i| Compact(i))); + + // They should have the same number of items: + assert_eq!(NMap::meta((123,)).total_items, NMapCompact::meta((123,)).total_items); // FAIL-CI check tracking + assert_eq!(NMap::meta((123,)).total_items, 100); + // But the compact variant should have more values per page: + }); + } +} diff --git a/frame/support/test/tests/final_keys.rs b/frame/support/test/tests/final_keys.rs index 765afaf1e6604..11a60b4e2bf9b 100644 --- a/frame/support/test/tests/final_keys.rs +++ b/frame/support/test/tests/final_keys.rs @@ -19,6 +19,11 @@ use codec::Encode; use frame_support::{derive_impl, storage::unhashed, StoragePrefixedMap}; use frame_system::pallet_prelude::BlockNumberFor; +use frame_support::{ + pallet_prelude::*, + storage::{types::StoragePagedListMeta, StoragePrefixedContainer}, +}; + use sp_core::{sr25519, ConstU32}; use sp_io::{ hashing::{blake2_128, twox_128, twox_64}, @@ -32,7 +37,6 @@ use sp_runtime::{ #[frame_support::pallet] mod no_instance { use super::*; - use frame_support::pallet_prelude::*; #[pallet::pallet] pub struct Pallet(_); @@ -73,6 +77,17 @@ mod no_instance { ValueQuery, >; + #[pallet::storage] + pub type PagedList = StoragePagedList<_, u32, ConstU32<40>>; + + #[pallet::storage] + pub type PagedNMap = StoragePagedNMap< + _, + (NMapKey, NMapKey), + u32, + ConstU32<40>, + >; + #[pallet::genesis_config] pub struct GenesisConfig { pub value: u32, @@ -105,7 +120,6 @@ mod no_instance { #[frame_support::pallet] mod instance { use super::*; - use frame_support::pallet_prelude::*; #[pallet::pallet] pub struct Pallet(PhantomData<(T, I)>); @@ -149,6 +163,17 @@ mod instance { ValueQuery, >; + #[pallet::storage] + pub type PagedList, I: 'static = ()> = StoragePagedList<_, u32, ConstU32<40>>; + + #[pallet::storage] + pub type PagedNMap, I: 'static = ()> = StoragePagedNMap< + _, + (NMapKey, NMapKey), + u32, + ConstU32<40>, + >; + #[pallet::genesis_config] pub struct GenesisConfig, I: 'static = ()> { pub value: u32, @@ -260,6 +285,78 @@ fn final_keys_no_instance() { assert_eq!(unhashed::get::(&k), Some(3u32)); assert_eq!(&k[..32], &>::final_prefix()); }); + // The metadata key of a PagedList is correct. + TestExternalities::default().execute_with(|| { + >::append_many(0..123); + let mut k = [twox_128(b"FinalKeysNone"), twox_128(b"PagedList")].concat(); + k.extend(b"meta"); + assert_eq!( + unhashed::get::, u32, ()>>(&k), + Some(StoragePagedListMeta { + total_items: 123, + last_page: 12, + last_page_byte_offset: 12, + ..Default::default() + }) + ); + assert_eq!(&k[..32], &>::final_prefix()); + }); + // The page key of a PagedList is correct. + TestExternalities::default().execute_with(|| { + >::append_many(0..400); + for page in 0u32..10 { + let items = page * 10; + let mut k = [twox_128(b"FinalKeysNone"), twox_128(b"PagedList")].concat(); + k.extend(b"page"); + k.extend(page.encode()); + assert_eq!( + unhashed::get_raw(&k), + Some((items..items + 10).collect::>().encode()) + ); + assert_eq!(&k[..32], &>::final_prefix()); + } + }); + // The metadata key of a PagedMap is correct. + TestExternalities::default().execute_with(|| { + for key in 0..10 { + let key2 = key * 123; // random... + >::append_many((key, key2), 0..123); + let mut k = [twox_128(b"FinalKeysNone"), twox_128(b"PagedNMap")].concat(); + k.extend(key.using_encoded(blake2_128_concat)); + k.extend(key2.using_encoded(twox_64_concat)); + k.extend(b"meta"); + assert_eq!( + unhashed::get::, u32, ()>>(&k), + Some(StoragePagedListMeta { + total_items: 123, + last_page: 12, + last_page_byte_offset: 12, + ..Default::default() + }) + ); + assert_eq!(&k[..32], &>::final_prefix()); + } + }); + // The page key of a PagedMap is correct. + TestExternalities::default().execute_with(|| { + for key in 0..10 { + let key2 = key * 123; // random... + >::append_many((key, key2), 0..400); + for page in 0u32..10 { + let items = page * 10; + let mut k = [twox_128(b"FinalKeysNone"), twox_128(b"PagedNMap")].concat(); + k.extend(key.using_encoded(blake2_128_concat)); + k.extend(key2.using_encoded(twox_64_concat)); + k.extend(b"page"); + k.extend(page.encode()); + assert_eq!( + unhashed::get_raw(&k), + Some((items..items + 10).collect::>().encode()) + ); + assert_eq!(&k[..32], &>::final_prefix()); + } + } + }); } #[test] @@ -295,6 +392,78 @@ fn final_keys_default_instance() { assert_eq!(unhashed::get::(&k), Some(3u32)); assert_eq!(&k[..32], &>::final_prefix()); }); + // The metadata key of a PagedList is correct. + TestExternalities::default().execute_with(|| { + >::append_many(0..123); + let mut k = [twox_128(b"FinalKeysSome"), twox_128(b"PagedList")].concat(); + k.extend(b"meta"); + assert_eq!( + unhashed::get::, u32, ()>>(&k), + Some(StoragePagedListMeta { + total_items: 123, + last_page: 12, + last_page_byte_offset: 12, + ..Default::default() + }) + ); + assert_eq!(&k[..32], &>::final_prefix()); + }); + // The page key of a PagedList is correct. + TestExternalities::default().execute_with(|| { + >::append_many(0..400); + for page in 0u32..10 { + let items = page * 10; + let mut k = [twox_128(b"FinalKeysSome"), twox_128(b"PagedList")].concat(); + k.extend(b"page"); + k.extend(page.encode()); + assert_eq!( + unhashed::get_raw(&k), + Some((items..items + 10).collect::>().encode()) + ); + assert_eq!(&k[..32], &>::final_prefix()); + } + }); + // The metadata key of a PagedMap is correct. + TestExternalities::default().execute_with(|| { + for key in 0..10 { + let key2 = key * 123; // random... + >::append_many((key, key2), 0..123); + let mut k = [twox_128(b"FinalKeysSome"), twox_128(b"PagedNMap")].concat(); + k.extend(key.using_encoded(blake2_128_concat)); + k.extend(key2.using_encoded(twox_64_concat)); + k.extend(b"meta"); + assert_eq!( + unhashed::get::, u32, ()>>(&k), + Some(StoragePagedListMeta { + total_items: 123, + last_page: 12, + last_page_byte_offset: 12, + ..Default::default() + }) + ); + assert_eq!(&k[..32], &>::final_prefix()); + } + }); + // The page key of a PagedMap is correct. + TestExternalities::default().execute_with(|| { + for key in 0..10 { + let key2 = key * 123; // random... + >::append_many((key, key2), 0..400); + for page in 0u32..10 { + let items = page * 10; + let mut k = [twox_128(b"FinalKeysSome"), twox_128(b"PagedNMap")].concat(); + k.extend(key.using_encoded(blake2_128_concat)); + k.extend(key2.using_encoded(twox_64_concat)); + k.extend(b"page"); + k.extend(page.encode()); + assert_eq!( + unhashed::get_raw(&k), + Some((items..items + 10).collect::>().encode()) + ); + assert_eq!(&k[..32], &>::final_prefix()); + } + } + }); } #[test] @@ -330,4 +499,89 @@ fn final_keys_instance_2() { assert_eq!(unhashed::get::(&k), Some(3u32)); assert_eq!(&k[..32], &>::final_prefix()); }); + // The metadata key of a PagedList is correct. + TestExternalities::default().execute_with(|| { + >::append_many(0..123); + let mut k = [twox_128(b"Instance2FinalKeysSome"), twox_128(b"PagedList")].concat(); + k.extend(b"meta"); + assert_eq!( + unhashed::get::< + StoragePagedListMeta, u32, ()>, + >(&k), + Some(StoragePagedListMeta { + total_items: 123, + last_page: 12, + last_page_byte_offset: 12, + ..Default::default() + }) + ); + assert_eq!(&k[..32], &>::final_prefix()); + }); + // The page key of a PagedList is correct. + TestExternalities::default().execute_with(|| { + >::append_many(0..400); + for page in 0u32..10 { + let items = page * 10; + let mut k = [twox_128(b"Instance2FinalKeysSome"), twox_128(b"PagedList")].concat(); + k.extend(b"page"); + k.extend(page.encode()); + assert_eq!( + unhashed::get_raw(&k), + Some((items..items + 10).collect::>().encode()) + ); + assert_eq!( + &k[..32], + &>::final_prefix() + ); + } + }); + // The metadata key of a PagedMap is correct. + TestExternalities::default().execute_with(|| { + for key in 0..10 { + let key2 = key * 123; // random... + >::append_many((key, key2), 0..123); + let mut k = [twox_128(b"Instance2FinalKeysSome"), twox_128(b"PagedNMap")].concat(); + k.extend(key.using_encoded(blake2_128_concat)); + k.extend(key2.using_encoded(twox_64_concat)); + k.extend(b"meta"); + assert_eq!( + unhashed::get::< + StoragePagedListMeta, u32, ()>, + >(&k), + Some(StoragePagedListMeta { + total_items: 123, + last_page: 12, + last_page_byte_offset: 12, + ..Default::default() + }) + ); + assert_eq!( + &k[..32], + &>::final_prefix() + ); + } + }); + // The page key of a PagedMap is correct. + TestExternalities::default().execute_with(|| { + for key in 0..10 { + let key2 = key * 123; // random... + >::append_many((key, key2), 0..400); + for page in 0u32..10 { + let items = page * 10; + let mut k = [twox_128(b"Instance2FinalKeysSome"), twox_128(b"PagedNMap")].concat(); + k.extend(key.using_encoded(blake2_128_concat)); + k.extend(key2.using_encoded(twox_64_concat)); + k.extend(b"page"); + k.extend(page.encode()); + assert_eq!( + unhashed::get_raw(&k), + Some((items..items + 10).collect::>().encode()) + ); + assert_eq!( + &k[..32], + &>::final_prefix() + ); + } + } + }); } diff --git a/primitives/metadata-ir/Cargo.toml b/primitives/metadata-ir/Cargo.toml index 49fd23e208e41..ea93807cce484 100644 --- a/primitives/metadata-ir/Cargo.toml +++ b/primitives/metadata-ir/Cargo.toml @@ -14,7 +14,7 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] codec = { package = "parity-scale-codec", version = "3.6.1", default-features = false } -frame-metadata = { version = "16.0.0", default-features = false, features = ["current"] } +frame-metadata = { version = "16.0.0", default-features = false, features = ["current"], git = "https://github.com/ggwpez/frame-metadata", branch = "main" } # FAIL-CI scale-info = { version = "2.1.1", default-features = false, features = ["derive"] } sp-std = { version = "8.0.0", default-features = false, path = "../std" } From 511f31ed490c620a6fcee08284fff563cf37b160 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 10 Aug 2023 14:37:34 +0200 Subject: [PATCH 4/4] Fixup merge Signed-off-by: Oliver Tale-Yazdi --- frame/support/procedural/src/pallet/parse/storage.rs | 4 ++-- frame/support/src/lib.rs | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/frame/support/procedural/src/pallet/parse/storage.rs b/frame/support/procedural/src/pallet/parse/storage.rs index ac49de30f1402..5574b540d6f77 100644 --- a/frame/support/procedural/src/pallet/parse/storage.rs +++ b/frame/support/procedural/src/pallet/parse/storage.rs @@ -272,10 +272,10 @@ impl StorageGenerics { Self::Map { query_kind, .. } | Self::CountedMap { query_kind, .. } | Self::Value { query_kind, .. } | - // A list cannot be queried - only iterated. - Self::PagedList { .. } => None, Self::NMap { query_kind, .. } | Self::CountedNMap { query_kind, .. } => query_kind.clone(), + // A list cannot be queried - only iterated. + Self::PagedList { .. } => None, } } } diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 972fb70bc86df..c2d6c1a1ddaeb 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -1545,9 +1545,9 @@ pub mod pallet_prelude { storage::{ bounded_vec::BoundedVec, types::{ - CountedStorageMap, CountedStorageNMap, Key as NMapKey, OptionQuery, ResultQuery, StorageDoubleMap, - StorageMap, StorageNMap, StoragePagedList, StoragePagedNMap, StorageValue, - ValueQuery, + CountedStorageMap, CountedStorageNMap, Key as NMapKey, OptionQuery, ResultQuery, + StorageDoubleMap, StorageMap, StorageNMap, StoragePagedList, StoragePagedNMap, + StorageValue, ValueQuery, }, StorageKeyedList, StorageList, },