From 008ead97aef5adf0671c295cd6175c08f7f93fcb Mon Sep 17 00:00:00 2001 From: Krayt78 Date: Tue, 17 Dec 2024 22:14:17 +0100 Subject: [PATCH 1/4] first pass need defensive --- Cargo.lock | 7 +---- substrate/frame/paged-list/Cargo.toml | 25 +++------------- substrate/frame/paged-list/src/lib.rs | 7 ++--- substrate/frame/paged-list/src/mock.rs | 10 +++---- substrate/frame/paged-list/src/paged_list.rs | 30 +++++++++----------- substrate/frame/paged-list/src/tests.rs | 5 ++-- 6 files changed, 29 insertions(+), 55 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 02d7da8f7657..7b2b39e91ae3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -14312,15 +14312,10 @@ name = "pallet-paged-list" version = "0.6.0" dependencies = [ "docify", - "frame-benchmarking 28.0.0", - "frame-support 28.0.0", - "frame-system 28.0.0", "parity-scale-codec", + "polkadot-sdk-frame 0.1.0", "scale-info", - "sp-core 28.0.0", - "sp-io 30.0.0", "sp-metadata-ir 0.6.0", - "sp-runtime 31.0.1", ] [[package]] diff --git a/substrate/frame/paged-list/Cargo.toml b/substrate/frame/paged-list/Cargo.toml index a680139c5fdc..e2ded479a11c 100644 --- a/substrate/frame/paged-list/Cargo.toml +++ b/substrate/frame/paged-list/Cargo.toml @@ -19,13 +19,7 @@ codec = { features = ["derive"], workspace = true } docify = { workspace = true } scale-info = { features = ["derive"], workspace = true } -frame-benchmarking = { optional = true, workspace = true } -frame-support = { workspace = true } -frame-system = { workspace = true } - -sp-runtime = { workspace = true } -sp-core = { workspace = true } -sp-io = { workspace = true } +frame = { workspace = true, features = ["experimental", "runtime"] } sp-metadata-ir = { optional = true, workspace = true } [features] @@ -33,27 +27,16 @@ default = ["std"] std = [ "codec/std", - "frame-benchmarking?/std", - "frame-support/std", - "frame-system/std", + "frame/std", "scale-info/std", - "sp-core/std", - "sp-io/std", - "sp-metadata-ir/std", - "sp-runtime/std", ] runtime-benchmarks = [ - "frame-benchmarking/runtime-benchmarks", - "frame-support/runtime-benchmarks", - "frame-system/runtime-benchmarks", - "sp-runtime/runtime-benchmarks", + "frame/runtime-benchmarks", ] try-runtime = [ - "frame-support/try-runtime", - "frame-system/try-runtime", - "sp-runtime/try-runtime", + "frame/try-runtime", ] frame-metadata = ["sp-metadata-ir"] diff --git a/substrate/frame/paged-list/src/lib.rs b/substrate/frame/paged-list/src/lib.rs index ed68dac63beb..c3cbdf040c3d 100644 --- a/substrate/frame/paged-list/src/lib.rs +++ b/substrate/frame/paged-list/src/lib.rs @@ -72,16 +72,15 @@ mod tests; extern crate alloc; use codec::FullCodec; -use frame_support::{ - pallet_prelude::StorageList, +use frame::{ + prelude::*, traits::{PalletInfoAccess, StorageInstance}, }; pub use paged_list::StoragePagedList; -#[frame_support::pallet] +#[frame::pallet] pub mod pallet { use super::*; - use frame_support::pallet_prelude::*; #[pallet::pallet] pub struct Pallet(_); diff --git a/substrate/frame/paged-list/src/mock.rs b/substrate/frame/paged-list/src/mock.rs index 3e4903200c3d..196beb4d09b8 100644 --- a/substrate/frame/paged-list/src/mock.rs +++ b/substrate/frame/paged-list/src/mock.rs @@ -20,13 +20,13 @@ #![cfg(feature = "std")] use crate::{paged_list::StoragePagedListMeta, Config, ListPrefix}; -use frame_support::derive_impl; -use sp_runtime::{traits::IdentityLookup, BuildStorage}; +use frame::{traits::IdentityLookup}; +use frame::testing_prelude::*; type Block = frame_system::mocking::MockBlock; // Configure a mock runtime to test the pallet. -frame_support::construct_runtime!( +construct_runtime!( pub enum Test { System: frame_system, PagedList: crate, @@ -43,7 +43,7 @@ impl frame_system::Config for Test { type RuntimeEvent = RuntimeEvent; } -frame_support::parameter_types! { +parameter_types! { pub storage ValuesPerNewPage: u32 = 5; pub const MaxPages: Option = Some(20); } @@ -62,7 +62,7 @@ pub type MetaOf = StoragePagedListMeta, ::Value, ::ValuesPerNewPage>; /// Build genesis storage according to the mock runtime. -pub fn new_test_ext() -> sp_io::TestExternalities { +pub fn new_test_ext() -> TestExternalities { frame_system::GenesisConfig::::default().build_storage().unwrap().into() } diff --git a/substrate/frame/paged-list/src/paged_list.rs b/substrate/frame/paged-list/src/paged_list.rs index bbd889e25218..3456a5d96e24 100644 --- a/substrate/frame/paged-list/src/paged_list.rs +++ b/substrate/frame/paged-list/src/paged_list.rs @@ -26,13 +26,14 @@ use alloc::vec::Vec; 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 frame::testing_prelude::*; +use frame::{ + traits::{Get, StorageInstance, Saturating}, }; -use sp_runtime::traits::Saturating; +use frame::runtime::prelude::storage; +use frame::runtime::prelude::storage::StoragePrefixedContainer; +use frame::runtime::prelude::storage::storage_noop_guard::StorageNoopGuard; +use frame::deps::sp_io; pub type PageIndex = u32; pub type ValueIndex = u32; @@ -60,10 +61,10 @@ pub type ValueIndex = u32; /// 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 +/// appending to the last page by utilizing [`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 -/// created through [`sp_io::storage::append`]. **Draining** advances the internal indices identical +/// created through [`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. /// @@ -111,7 +112,7 @@ pub struct StoragePagedListMeta { _phantom: PhantomData<(Prefix, Value, ValuesPerNewPage)>, } -impl frame_support::storage::StorageAppender +impl storage::StorageAppender for StoragePagedListMeta where Prefix: StorageInstance, @@ -311,7 +312,7 @@ where } } -impl frame_support::storage::StorageList +impl storage::StorageList for StoragePagedList where Prefix: StorageInstance, @@ -355,13 +356,13 @@ where /// Return the elements of the list. #[cfg(test)] fn as_vec() -> Vec { - >::iter().collect() + >::iter().collect() } /// Return and remove the elements of the list. #[cfg(test)] fn as_drained_vec() -> Vec { - >::drain().collect() + >::drain().collect() } } @@ -407,11 +408,6 @@ where #[allow(dead_code)] pub(crate) mod mock { pub use super::*; - pub use frame_support::parameter_types; - #[cfg(test)] - pub use frame_support::{storage::StorageList as _, StorageNoopGuard}; - #[cfg(test)] - pub use sp_io::TestExternalities; parameter_types! { pub const ValuesPerNewPage: u32 = 5; diff --git a/substrate/frame/paged-list/src/tests.rs b/substrate/frame/paged-list/src/tests.rs index becb4b23508e..291ba6220a92 100644 --- a/substrate/frame/paged-list/src/tests.rs +++ b/substrate/frame/paged-list/src/tests.rs @@ -21,7 +21,9 @@ #![cfg(test)] use crate::{mock::*, *}; -use frame_support::storage::{StorageList, StoragePrefixedContainer}; +use frame::testing_prelude::*; +use frame::prelude::storage::StorageAppender; +use frame::prelude::storage::StoragePrefixedContainer; #[docify::export] #[test] @@ -46,7 +48,6 @@ fn append_many_works() { #[docify::export] #[test] fn appender_works() { - use frame_support::storage::StorageAppender; test_closure(|| { let mut appender = PagedList::appender(); From 8780fa03b26f8bd90c9ab32f19af43d86f18c0a1 Mon Sep 17 00:00:00 2001 From: Krayt78 Date: Tue, 17 Dec 2024 22:35:37 +0100 Subject: [PATCH 2/4] fix defensive with framesupport --- substrate/frame/paged-list/src/paged_list.rs | 12 +++++------- substrate/frame/src/lib.rs | 2 +- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/substrate/frame/paged-list/src/paged_list.rs b/substrate/frame/paged-list/src/paged_list.rs index 3456a5d96e24..cc83fd5302a7 100644 --- a/substrate/frame/paged-list/src/paged_list.rs +++ b/substrate/frame/paged-list/src/paged_list.rs @@ -26,14 +26,12 @@ use alloc::vec::Vec; use codec::{Decode, Encode, EncodeLike, FullCodec}; use core::marker::PhantomData; + use frame::testing_prelude::*; -use frame::{ - traits::{Get, StorageInstance, Saturating}, -}; -use frame::runtime::prelude::storage; -use frame::runtime::prelude::storage::StoragePrefixedContainer; -use frame::runtime::prelude::storage::storage_noop_guard::StorageNoopGuard; -use frame::deps::sp_io; +use frame::deps::{sp_io, frame_support}; +use frame::traits::{Get, StorageInstance, Saturating}; +use frame::runtime::prelude::storage::{storage_noop_guard::StorageNoopGuard, StoragePrefixedContainer}; + pub type PageIndex = u32; pub type ValueIndex = u32; diff --git a/substrate/frame/src/lib.rs b/substrate/frame/src/lib.rs index 0ca36ca8545a..ac29159f89d8 100644 --- a/substrate/frame/src/lib.rs +++ b/substrate/frame/src/lib.rs @@ -308,7 +308,7 @@ pub mod testing_prelude { /// Other helper macros from `frame_support` that help with asserting in tests. pub use frame_support::{ assert_err, assert_err_ignore_postinfo, assert_error_encoded_size, assert_noop, assert_ok, - assert_storage_noop, storage_alias, + assert_storage_noop, storage_alias, defensive, }; pub use frame_system::{self, mocking::*}; From 880284bf546c61ab325866f8226ea5e41a99e1fe Mon Sep 17 00:00:00 2001 From: Krayt78 Date: Wed, 18 Dec 2024 09:26:16 +0100 Subject: [PATCH 3/4] cleanup & fmt --- substrate/frame/paged-list/src/mock.rs | 3 +-- substrate/frame/paged-list/src/paged_list.rs | 11 ++++++----- substrate/frame/paged-list/src/tests.rs | 7 ++++--- substrate/frame/src/lib.rs | 2 +- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/substrate/frame/paged-list/src/mock.rs b/substrate/frame/paged-list/src/mock.rs index 196beb4d09b8..e4561ef57eb9 100644 --- a/substrate/frame/paged-list/src/mock.rs +++ b/substrate/frame/paged-list/src/mock.rs @@ -20,8 +20,7 @@ #![cfg(feature = "std")] use crate::{paged_list::StoragePagedListMeta, Config, ListPrefix}; -use frame::{traits::IdentityLookup}; -use frame::testing_prelude::*; +use frame::{testing_prelude::*, traits::IdentityLookup}; type Block = frame_system::mocking::MockBlock; diff --git a/substrate/frame/paged-list/src/paged_list.rs b/substrate/frame/paged-list/src/paged_list.rs index cc83fd5302a7..772bc9e872d1 100644 --- a/substrate/frame/paged-list/src/paged_list.rs +++ b/substrate/frame/paged-list/src/paged_list.rs @@ -27,11 +27,12 @@ use alloc::vec::Vec; use codec::{Decode, Encode, EncodeLike, FullCodec}; use core::marker::PhantomData; -use frame::testing_prelude::*; -use frame::deps::{sp_io, frame_support}; -use frame::traits::{Get, StorageInstance, Saturating}; -use frame::runtime::prelude::storage::{storage_noop_guard::StorageNoopGuard, StoragePrefixedContainer}; - +use frame::{ + deps::{frame_support, sp_io}, + runtime::prelude::storage::{storage_noop_guard::StorageNoopGuard, StoragePrefixedContainer}, + testing_prelude::*, + traits::{Get, Saturating, StorageInstance}, +}; pub type PageIndex = u32; pub type ValueIndex = u32; diff --git a/substrate/frame/paged-list/src/tests.rs b/substrate/frame/paged-list/src/tests.rs index 291ba6220a92..259fe2165fbd 100644 --- a/substrate/frame/paged-list/src/tests.rs +++ b/substrate/frame/paged-list/src/tests.rs @@ -21,9 +21,10 @@ #![cfg(test)] use crate::{mock::*, *}; -use frame::testing_prelude::*; -use frame::prelude::storage::StorageAppender; -use frame::prelude::storage::StoragePrefixedContainer; +use frame::{ + prelude::storage::{StorageAppender, StoragePrefixedContainer}, + testing_prelude::*, +}; #[docify::export] #[test] diff --git a/substrate/frame/src/lib.rs b/substrate/frame/src/lib.rs index ac29159f89d8..9d781e797091 100644 --- a/substrate/frame/src/lib.rs +++ b/substrate/frame/src/lib.rs @@ -308,7 +308,7 @@ pub mod testing_prelude { /// Other helper macros from `frame_support` that help with asserting in tests. pub use frame_support::{ assert_err, assert_err_ignore_postinfo, assert_error_encoded_size, assert_noop, assert_ok, - assert_storage_noop, storage_alias, defensive, + assert_storage_noop, defensive, storage_alias, }; pub use frame_system::{self, mocking::*}; From 7af57ce533a23e87d6b2fc54fe08f925c0bc44c3 Mon Sep 17 00:00:00 2001 From: Krayt78 Date: Thu, 19 Dec 2024 16:44:37 +0100 Subject: [PATCH 4/4] removed warning and added deps to prelude --- substrate/frame/paged-list/src/paged_list.rs | 33 ++++++++++---------- substrate/frame/src/lib.rs | 2 ++ 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/substrate/frame/paged-list/src/paged_list.rs b/substrate/frame/paged-list/src/paged_list.rs index 772bc9e872d1..ac61d423647d 100644 --- a/substrate/frame/paged-list/src/paged_list.rs +++ b/substrate/frame/paged-list/src/paged_list.rs @@ -28,10 +28,10 @@ use codec::{Decode, Encode, EncodeLike, FullCodec}; use core::marker::PhantomData; use frame::{ - deps::{frame_support, sp_io}, - runtime::prelude::storage::{storage_noop_guard::StorageNoopGuard, StoragePrefixedContainer}, + deps::{frame_support}, + runtime::prelude::storage::StoragePrefixedContainer, testing_prelude::*, - traits::{Get, Saturating, StorageInstance}, + traits::{Saturating, StorageInstance}, }; pub type PageIndex = u32; @@ -135,7 +135,7 @@ where pub fn from_storage() -> Option { let key = Self::key(); - sp_io::storage::get(&key).and_then(|raw| Self::decode(&mut &raw[..]).ok()) + get(&key).and_then(|raw| Self::decode(&mut &raw[..]).ok()) } pub fn key() -> Vec { @@ -153,13 +153,13 @@ where } let key = page_key::(self.last_page); self.last_page_len.saturating_inc(); - sp_io::storage::append(&key, item.encode()); + append(&key, item.encode()); self.store(); } pub fn store(&self) { let key = Self::key(); - self.using_encoded(|enc| sp_io::storage::set(&key, enc)); + self.using_encoded(|enc| set(&key, enc)); } pub fn reset(&mut self) { @@ -168,7 +168,7 @@ where } pub fn delete() { - sp_io::storage::clear(&Self::key()); + clear(&Self::key()); } } @@ -187,7 +187,7 @@ impl Page { value_index: ValueIndex, ) -> Option { let key = page_key::(index); - let values = sp_io::storage::get(&key) + let values = get(&key) .and_then(|raw| alloc::vec::Vec::::decode(&mut &raw[..]).ok())?; if values.is_empty() { // Don't create empty pages. @@ -213,7 +213,7 @@ impl Page { // 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); + clear(&key); } /// Storage key of a page with `index`. @@ -440,7 +440,6 @@ mod tests { #[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::>()); @@ -494,13 +493,13 @@ mod tests { 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!(exists(&page_key::(0))); + assert!(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))); + assert!(!exists(&page_key::(0))); + assert!(exists(&page_key::(1))); }); } @@ -511,16 +510,16 @@ mod tests { List::append_many(0..9); let key = page_key::(0); - let raw = sp_io::storage::get(&key).expect("Page should be present"); + let raw = 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 raw = 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 = get(&meta_key::()).expect("Meta should be present"); let meta: StoragePagedListMeta = Decode::decode(&mut &meta[..]).unwrap(); assert_eq!(meta.first_page, 0); diff --git a/substrate/frame/src/lib.rs b/substrate/frame/src/lib.rs index 9d781e797091..c37204863d7b 100644 --- a/substrate/frame/src/lib.rs +++ b/substrate/frame/src/lib.rs @@ -317,6 +317,8 @@ pub mod testing_prelude { pub use sp_io::TestExternalities; pub use sp_io::TestExternalities as TestState; + + pub use sp_io::storage::*; } /// All of the types and tools needed to build FRAME-based runtimes.