diff --git a/frame/state-trie-migration/src/lib.rs b/frame/state-trie-migration/src/lib.rs index 94f6c1f223b9c..353bc93e5ab94 100644 --- a/frame/state-trie-migration/src/lib.rs +++ b/frame/state-trie-migration/src/lib.rs @@ -235,7 +235,10 @@ pub mod pallet { /// reading a key, we simply cannot know how many bytes it is. In other words, this should /// not be used in any environment where resources are strictly bounded (e.g. a parachain), /// but it is acceptable otherwise (relay chain, offchain workers). - pub fn migrate_until_exhaustion(&mut self, limits: MigrationLimits) -> DispatchResult { + pub fn migrate_until_exhaustion( + &mut self, + limits: MigrationLimits, + ) -> Result<(), Error> { log!(debug, "running migrations on top of {:?} until {:?}", self, limits); if limits.item.is_zero() || limits.size.is_zero() { @@ -262,7 +265,7 @@ pub mod pallet { /// Migrate AT MOST ONE KEY. This can be either a top or a child key. /// /// This function is *the* core of this entire pallet. - fn migrate_tick(&mut self) -> DispatchResult { + fn migrate_tick(&mut self) -> Result<(), Error> { match (&self.progress_top, &self.progress_child) { (Progress::ToStart, _) => self.migrate_top(), (Progress::LastKey(_), Progress::LastKey(_)) => { @@ -301,7 +304,7 @@ pub mod pallet { /// Migrate the current child key, setting it to its new value, if one exists. /// /// It updates the dynamic counters. - fn migrate_child(&mut self) -> DispatchResult { + fn migrate_child(&mut self) -> Result<(), Error> { use sp_io::default_child_storage as child_io; let (maybe_current_child, child_root) = match (&self.progress_child, &self.progress_top) { @@ -350,7 +353,7 @@ pub mod pallet { /// Migrate the current top key, setting it to its new value, if one exists. /// /// It updates the dynamic counters. - fn migrate_top(&mut self) -> DispatchResult { + fn migrate_top(&mut self) -> Result<(), Error> { let maybe_current_top = match &self.progress_top { Progress::LastKey(last_top) => { let maybe_top: Option> = @@ -431,7 +434,7 @@ pub mod pallet { /// The auto migration task finished. AutoMigrationFinished, /// Migration got halted due to an error or miss-configuration. - Halted, + Halted { error: Error }, } /// The outer Pallet struct. @@ -516,8 +519,9 @@ pub mod pallet { pub type SignedMigrationMaxLimits = StorageValue<_, MigrationLimits, OptionQuery>; #[pallet::error] + #[derive(Clone, PartialEq)] pub enum Error { - /// max signed limits not respected. + /// Max signed limits not respected. MaxSignedLimits, /// A key was longer than the configured maximum. /// @@ -529,12 +533,12 @@ pub mod pallet { KeyTooLong, /// submitter does not have enough funds. NotEnoughFunds, - /// bad witness data provided. + /// Bad witness data provided. BadWitness, - /// upper bound of size is exceeded, - SizeUpperBoundExceeded, /// Signed migration is not allowed because the maximum limit is not set yet. SignedMigrationNotAllowed, + /// Bad child root provided. + BadChildRoot, } #[pallet::call] @@ -617,7 +621,7 @@ pub mod pallet { let (_imbalance, _remainder) = T::Currency::slash(&who, deposit); Self::deposit_event(Event::::Slashed { who, amount: deposit }); debug_assert!(_remainder.is_zero()); - return Err(Error::::SizeUpperBoundExceeded.into()) + return Ok(().into()) } Self::deposit_event(Event::::Migrated { @@ -634,13 +638,10 @@ pub mod pallet { MigrationProcess::::put(task); let post_info = PostDispatchInfo { actual_weight, pays_fee: Pays::No }; - match migration { - Ok(_) => Ok(post_info), - Err(error) => { - Self::halt(&error); - Err(DispatchErrorWithPostInfo { post_info, error }) - }, + if let Err(error) = migration { + Self::halt(error); } + Ok(post_info) } /// Migrate the list of top keys by iterating each of them one by one. @@ -679,7 +680,7 @@ pub mod pallet { let (_imbalance, _remainder) = T::Currency::slash(&who, deposit); Self::deposit_event(Event::::Slashed { who, amount: deposit }); debug_assert!(_remainder.is_zero()); - Err("wrong witness data".into()) + Ok(().into()) } else { Self::deposit_event(Event::::Migrated { top: keys.len() as u32, @@ -741,12 +742,9 @@ pub mod pallet { let (_imbalance, _remainder) = T::Currency::slash(&who, deposit); debug_assert!(_remainder.is_zero()); Self::deposit_event(Event::::Slashed { who, amount: deposit }); - Err(DispatchErrorWithPostInfo { - error: "bad witness".into(), - post_info: PostDispatchInfo { - actual_weight: Some(T::WeightInfo::migrate_custom_child_fail()), - pays_fee: Pays::Yes, - }, + Ok(PostDispatchInfo { + actual_weight: Some(T::WeightInfo::migrate_custom_child_fail()), + pays_fee: Pays::Yes, }) } else { Self::deposit_event(Event::::Migrated { @@ -806,7 +804,7 @@ pub mod pallet { if let Some(limits) = Self::auto_limits() { let mut task = Self::migration_process(); if let Err(e) = task.migrate_until_exhaustion(limits) { - Self::halt(&e); + Self::halt(e); } let weight = Self::dynamic_weight(task.dyn_total_items(), task.dyn_size); @@ -849,10 +847,10 @@ pub mod pallet { } /// Put a stop to all ongoing migrations and logs an error. - fn halt(msg: &E) { - log!(error, "migration halted due to: {:?}", msg); + fn halt(error: Error) { + log!(error, "migration halted due to: {:?}", error); AutoLimits::::kill(); - Self::deposit_event(Event::::Halted); + Self::deposit_event(Event::::Halted { error }); } /// Convert a child root key, aka. "Child-bearing top key" into the proper format. @@ -871,7 +869,7 @@ pub mod pallet { fn transform_child_key_or_halt(root: &Vec) -> &[u8] { let key = Self::transform_child_key(root); if key.is_none() { - Self::halt("bad child root key"); + Self::halt(Error::::BadChildRoot); } key.unwrap_or_default() } @@ -961,8 +959,16 @@ mod benchmarks { frame_system::RawOrigin::Signed(caller.clone()).into(), vec![b"foo".to_vec()], 1, - ).is_err() - ) + ).is_ok() + ); + + frame_system::Pallet::::assert_last_event( + ::Event::from(crate::Event::Slashed { + who: caller.clone(), + amount: T::SignedDepositBase::get() + .saturating_add(T::SignedDepositPerItem::get().saturating_mul(1u32.into())), + }).into(), + ); } verify { assert_eq!(StateTrieMigration::::migration_process(), Default::default()); @@ -1005,7 +1011,7 @@ mod benchmarks { StateTrieMigration::::childify("top"), vec![b"foo".to_vec()], 1, - ).is_err() + ).is_ok() ) } verify { @@ -1285,18 +1291,16 @@ mod test { SignedMigrationMaxLimits::::put(MigrationLimits { size: 1 << 20, item: 50 }); // fails if the top key is too long. - frame_support::assert_err_with_weight!( - StateTrieMigration::continue_migrate( - Origin::signed(1), - MigrationLimits { item: 50, size: 1 << 20 }, - Bounded::max_value(), - MigrationProcess::::get() - ), - Error::::KeyTooLong, - Some(2000000), - ); + frame_support::assert_ok!(StateTrieMigration::continue_migrate( + Origin::signed(1), + MigrationLimits { item: 50, size: 1 << 20 }, + Bounded::max_value(), + MigrationProcess::::get() + ),); // The auto migration halted. - System::assert_last_event(crate::Event::Halted {}.into()); + System::assert_last_event( + crate::Event::Halted { error: Error::::KeyTooLong }.into(), + ); // Limits are killed. assert!(AutoLimits::::get().is_none()); @@ -1322,18 +1326,16 @@ mod test { SignedMigrationMaxLimits::::put(MigrationLimits { size: 1 << 20, item: 50 }); // fails if the top key is too long. - frame_support::assert_err_with_weight!( - StateTrieMigration::continue_migrate( - Origin::signed(1), - MigrationLimits { item: 50, size: 1 << 20 }, - Bounded::max_value(), - MigrationProcess::::get() - ), - Error::::KeyTooLong, - Some(2000000), - ); + frame_support::assert_ok!(StateTrieMigration::continue_migrate( + Origin::signed(1), + MigrationLimits { item: 50, size: 1 << 20 }, + Bounded::max_value(), + MigrationProcess::::get() + )); // The auto migration halted. - System::assert_last_event(crate::Event::Halted {}.into()); + System::assert_last_event( + crate::Event::Halted { error: Error::::KeyTooLong }.into(), + ); // Limits are killed. assert!(AutoLimits::::get().is_none()); @@ -1484,7 +1486,7 @@ mod test { ..Default::default() } ), - Error::::BadWitness + Error::::BadWitness, ); // migrate all keys in a series of submissions @@ -1547,14 +1549,11 @@ mod test { assert_eq!(Balances::free_balance(&1), 1000); // note that we don't expect this to be a noop -- we do slash. - frame_support::assert_err!( - StateTrieMigration::migrate_custom_top( - Origin::signed(1), - vec![b"key1".to_vec(), b"key2".to_vec(), b"key3".to_vec()], - correct_witness - 1, - ), - "wrong witness data" - ); + frame_support::assert_ok!(StateTrieMigration::migrate_custom_top( + Origin::signed(1), + vec![b"key1".to_vec(), b"key2".to_vec(), b"key3".to_vec()], + correct_witness - 1, + ),); // no funds should remain reserved. assert_eq!(Balances::reserved_balance(&1), 0); @@ -1584,13 +1583,12 @@ mod test { assert_eq!(Balances::free_balance(&1), 1000); // note that we don't expect this to be a noop -- we do slash. - assert!(StateTrieMigration::migrate_custom_child( + frame_support::assert_ok!(StateTrieMigration::migrate_custom_child( Origin::signed(1), StateTrieMigration::childify("chk1"), vec![b"key1".to_vec(), b"key2".to_vec()], 999999, // wrong witness - ) - .is_err()); + )); // no funds should remain reserved. assert_eq!(Balances::reserved_balance(&1), 0); diff --git a/frame/support/procedural/src/pallet/expand/call.rs b/frame/support/procedural/src/pallet/expand/call.rs index fe7589a8275d2..a9468451ad1d4 100644 --- a/frame/support/procedural/src/pallet/expand/call.rs +++ b/frame/support/procedural/src/pallet/expand/call.rs @@ -140,6 +140,24 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream { let capture_docs = if cfg!(feature = "no-metadata-docs") { "never" } else { "always" }; + // Wrap all calls inside of storage layers + if let Some(syn::Item::Impl(item_impl)) = def + .call + .as_ref() + .map(|c| &mut def.item.content.as_mut().expect("Checked by def parser").1[c.index]) + { + item_impl.items.iter_mut().for_each(|i| { + if let syn::ImplItem::Method(method) = i { + let block = &method.block; + method.block = syn::parse_quote! {{ + // We execute all dispatchable in a new storage layer, allowing them + // to return an error at any point, and undoing any storage changes. + #frame_support::storage::with_storage_layer(|| #block) + }}; + } + }); + } + quote::quote_spanned!(span => #[doc(hidden)] pub mod __substrate_call_check { @@ -267,12 +285,8 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream { #frame_support::sp_tracing::enter_span!( #frame_support::sp_tracing::trace_span!(stringify!(#fn_name)) ); - // We execute all dispatchable in a new storage layer, allowing them - // to return an error at any point, and undoing any storage changes. - #frame_support::storage::with_storage_layer(|| { - <#pallet_ident<#type_use_gen>>::#fn_name(origin, #( #args_name, )* ) - .map(Into::into).map_err(Into::into) - }) + <#pallet_ident<#type_use_gen>>::#fn_name(origin, #( #args_name, )* ) + .map(Into::into).map_err(Into::into) }, )* Self::__Ignore(_, _) => { diff --git a/frame/support/procedural/src/pallet/parse/call.rs b/frame/support/procedural/src/pallet/parse/call.rs index d8a81d699b8c2..336e08c3d39b7 100644 --- a/frame/support/procedural/src/pallet/parse/call.rs +++ b/frame/support/procedural/src/pallet/parse/call.rs @@ -48,8 +48,8 @@ pub struct CallDef { pub docs: Vec, } -#[derive(Clone)] /// Definition of dispatchable typically: `#[weight...] fn foo(origin .., param1: ...) -> ..` +#[derive(Clone)] pub struct CallVariantDef { /// Function name. pub name: syn::Ident, @@ -143,18 +143,18 @@ impl CallDef { index: usize, item: &mut syn::Item, ) -> syn::Result { - let item = if let syn::Item::Impl(item) = item { + let item_impl = if let syn::Item::Impl(item) = item { item } else { return Err(syn::Error::new(item.span(), "Invalid pallet::call, expected item impl")) }; let instances = vec![ - helper::check_impl_gen(&item.generics, item.impl_token.span())?, - helper::check_pallet_struct_usage(&item.self_ty)?, + helper::check_impl_gen(&item_impl.generics, item_impl.impl_token.span())?, + helper::check_pallet_struct_usage(&item_impl.self_ty)?, ]; - if let Some((_, _, for_)) = item.trait_ { + if let Some((_, _, for_)) = item_impl.trait_ { let msg = "Invalid pallet::call, expected no trait ident as in \ `impl<..> Pallet<..> { .. }`"; return Err(syn::Error::new(for_.span(), msg)) @@ -163,8 +163,8 @@ impl CallDef { let mut methods = vec![]; let mut indices = HashMap::new(); let mut last_index: Option = None; - for impl_item in &mut item.items { - if let syn::ImplItem::Method(method) = impl_item { + for item in &mut item_impl.items { + if let syn::ImplItem::Method(method) = item { if !matches!(method.vis, syn::Visibility::Public(_)) { let msg = "Invalid pallet::call, dispatchable function must be public: \ `pub fn`"; @@ -290,7 +290,7 @@ impl CallDef { }); } else { let msg = "Invalid pallet::call, only method accepted"; - return Err(syn::Error::new(impl_item.span(), msg)) + return Err(syn::Error::new(item.span(), msg)) } } @@ -299,8 +299,8 @@ impl CallDef { attr_span, instances, methods, - where_clause: item.generics.where_clause.clone(), - docs: get_doc_literals(&item.attrs), + where_clause: item_impl.generics.where_clause.clone(), + docs: get_doc_literals(&item_impl.attrs), }) } } diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index 2b05478e0b02a..fb0b658cd9303 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -1787,9 +1787,11 @@ macro_rules! decl_module { $vis fn $name( $origin: $origin_ty $(, $param: $param_ty )* ) -> $crate::dispatch::DispatchResult { - $crate::sp_tracing::enter_span!($crate::sp_tracing::trace_span!(stringify!($name))); - { $( $impl )* } - Ok(()) + $crate::storage::with_storage_layer(|| { + $crate::sp_tracing::enter_span!($crate::sp_tracing::trace_span!(stringify!($name))); + { $( $impl )* } + Ok(()) + }) } }; @@ -1805,8 +1807,10 @@ macro_rules! decl_module { ) => { $(#[$fn_attr])* $vis fn $name($origin: $origin_ty $(, $param: $param_ty )* ) -> $result { - $crate::sp_tracing::enter_span!($crate::sp_tracing::trace_span!(stringify!($name))); - $( $impl )* + $crate::storage::with_storage_layer(|| { + $crate::sp_tracing::enter_span!($crate::sp_tracing::trace_span!(stringify!($name))); + $( $impl )* + }) } }; diff --git a/frame/support/test/tests/construct_runtime.rs b/frame/support/test/tests/construct_runtime.rs index 63747a9d560dc..d388127f29abd 100644 --- a/frame/support/test/tests/construct_runtime.rs +++ b/frame/support/test/tests/construct_runtime.rs @@ -282,94 +282,96 @@ pub type UncheckedExtrinsic = generic::UncheckedExtrinsic::Root.into()), - Err(DispatchError::Module(ModuleError { - index: 31, - error: [0; 4], - message: Some("Something") - })), - ); - assert_eq!( - Module2::fail(system::Origin::::Root.into()), - Err(DispatchError::Module(ModuleError { - index: 32, - error: [0; 4], - message: Some("Something") - })), - ); - assert_eq!( - Module1_2::fail(system::Origin::::Root.into()), - Err(DispatchError::Module(ModuleError { - index: 33, - error: [0; 4], - message: Some("Something") - })), - ); - assert_eq!( - NestedModule3::fail(system::Origin::::Root.into()), - Err(DispatchError::Module(ModuleError { - index: 34, - error: [0; 4], - message: Some("Something") - })), - ); - assert_eq!( - Module1_3::fail(system::Origin::::Root.into()), - Err(DispatchError::Module(ModuleError { - index: 6, - error: [0; 4], - message: Some("Something") - })), - ); - assert_eq!( - Module1_4::fail(system::Origin::::Root.into()), - Err(DispatchError::Module(ModuleError { - index: 3, - error: [0; 4], - message: Some("Something") - })), - ); - assert_eq!( - Module1_5::fail(system::Origin::::Root.into()), - Err(DispatchError::Module(ModuleError { - index: 4, - error: [0; 4], - message: Some("Something") - })), - ); - assert_eq!( - Module1_6::fail(system::Origin::::Root.into()), - Err(DispatchError::Module(ModuleError { - index: 1, - error: [0; 4], - message: Some("Something") - })), - ); - assert_eq!( - Module1_7::fail(system::Origin::::Root.into()), - Err(DispatchError::Module(ModuleError { - index: 2, - error: [0; 4], - message: Some("Something") - })), - ); - assert_eq!( - Module1_8::fail(system::Origin::::Root.into()), - Err(DispatchError::Module(ModuleError { - index: 12, - error: [0; 4], - message: Some("Something") - })), - ); - assert_eq!( - Module1_9::fail(system::Origin::::Root.into()), - Err(DispatchError::Module(ModuleError { - index: 13, - error: [0; 4], - message: Some("Something") - })), - ); + sp_io::TestExternalities::default().execute_with(|| { + assert_eq!( + Module1_1::fail(system::Origin::::Root.into()), + Err(DispatchError::Module(ModuleError { + index: 31, + error: [0; 4], + message: Some("Something") + })), + ); + assert_eq!( + Module2::fail(system::Origin::::Root.into()), + Err(DispatchError::Module(ModuleError { + index: 32, + error: [0; 4], + message: Some("Something") + })), + ); + assert_eq!( + Module1_2::fail(system::Origin::::Root.into()), + Err(DispatchError::Module(ModuleError { + index: 33, + error: [0; 4], + message: Some("Something") + })), + ); + assert_eq!( + NestedModule3::fail(system::Origin::::Root.into()), + Err(DispatchError::Module(ModuleError { + index: 34, + error: [0; 4], + message: Some("Something") + })), + ); + assert_eq!( + Module1_3::fail(system::Origin::::Root.into()), + Err(DispatchError::Module(ModuleError { + index: 6, + error: [0; 4], + message: Some("Something") + })), + ); + assert_eq!( + Module1_4::fail(system::Origin::::Root.into()), + Err(DispatchError::Module(ModuleError { + index: 3, + error: [0; 4], + message: Some("Something") + })), + ); + assert_eq!( + Module1_5::fail(system::Origin::::Root.into()), + Err(DispatchError::Module(ModuleError { + index: 4, + error: [0; 4], + message: Some("Something") + })), + ); + assert_eq!( + Module1_6::fail(system::Origin::::Root.into()), + Err(DispatchError::Module(ModuleError { + index: 1, + error: [0; 4], + message: Some("Something") + })), + ); + assert_eq!( + Module1_7::fail(system::Origin::::Root.into()), + Err(DispatchError::Module(ModuleError { + index: 2, + error: [0; 4], + message: Some("Something") + })), + ); + assert_eq!( + Module1_8::fail(system::Origin::::Root.into()), + Err(DispatchError::Module(ModuleError { + index: 12, + error: [0; 4], + message: Some("Something") + })), + ); + assert_eq!( + Module1_9::fail(system::Origin::::Root.into()), + Err(DispatchError::Module(ModuleError { + index: 13, + error: [0; 4], + message: Some("Something") + })), + ); + }); } #[test] diff --git a/frame/support/test/tests/storage_layers.rs b/frame/support/test/tests/storage_layers.rs index 05ed60fe90196..7404ccace2c09 100644 --- a/frame/support/test/tests/storage_layers.rs +++ b/frame/support/test/tests/storage_layers.rs @@ -276,5 +276,7 @@ fn storage_layer_in_decl_pallet_call() { let call2 = Call::DeclPallet(decl_pallet::Call::set_value { value: 1 }); assert_noop!(call2.dispatch(Origin::signed(0)), "Revert!"); + // Calling the function directly also works with storage layers. + assert_noop!(decl_pallet::Module::::set_value(Origin::signed(1), 1), "Revert!"); }); } diff --git a/frame/transaction-storage/src/lib.rs b/frame/transaction-storage/src/lib.rs index f16b8f029662b..681cd29af8222 100644 --- a/frame/transaction-storage/src/lib.rs +++ b/frame/transaction-storage/src/lib.rs @@ -245,9 +245,11 @@ pub mod pallet { let sender = ensure_signed(origin)?; let transactions = >::get(block).ok_or(Error::::RenewedNotFound)?; let info = transactions.get(index as usize).ok_or(Error::::RenewedNotFound)?; + let extrinsic_index = + >::extrinsic_index().ok_or(Error::::BadContext)?; + Self::apply_fee(sender, info.size)?; - let extrinsic_index = >::extrinsic_index().unwrap(); sp_io::transaction_index::renew(extrinsic_index, info.content_hash.into()); let mut index = 0; diff --git a/primitives/state-machine/src/basic.rs b/primitives/state-machine/src/basic.rs index 6efc847bfbdb7..236a515a2412d 100644 --- a/primitives/state-machine/src/basic.rs +++ b/primitives/state-machine/src/basic.rs @@ -17,14 +17,13 @@ //! Basic implementation for Externalities. -use crate::{Backend, StorageKey, StorageValue}; +use crate::{Backend, OverlayedChanges, StorageKey, StorageValue}; use codec::Encode; use hash_db::Hasher; use log::warn; use sp_core::{ storage::{ - well_known_keys::is_child_storage_key, ChildInfo, StateVersion, Storage, StorageChild, - TrackedStorageKey, + well_known_keys::is_child_storage_key, ChildInfo, StateVersion, Storage, TrackedStorageKey, }, traits::Externalities, Blake2Hasher, @@ -35,20 +34,19 @@ use std::{ any::{Any, TypeId}, collections::BTreeMap, iter::FromIterator, - ops::Bound, }; /// Simple Map-based Externalities impl. #[derive(Debug)] pub struct BasicExternalities { - inner: Storage, + overlay: OverlayedChanges, extensions: Extensions, } impl BasicExternalities { /// Create a new instance of `BasicExternalities` pub fn new(inner: Storage) -> Self { - BasicExternalities { inner, extensions: Default::default() } + BasicExternalities { overlay: inner.into(), extensions: Default::default() } } /// New basic externalities with empty storage. @@ -57,13 +55,34 @@ impl BasicExternalities { } /// Insert key/value - pub fn insert(&mut self, k: StorageKey, v: StorageValue) -> Option { - self.inner.top.insert(k, v) + pub fn insert(&mut self, k: StorageKey, v: StorageValue) { + self.overlay.set_storage(k, Some(v)); } /// Consume self and returns inner storages pub fn into_storages(self) -> Storage { - self.inner + Storage { + top: self + .overlay + .changes() + .filter_map(|(k, v)| v.value().map(|v| (k.to_vec(), v.to_vec()))) + .collect(), + children_default: self + .overlay + .children() + .map(|(iter, i)| { + ( + i.storage_key().to_vec(), + sp_core::storage::StorageChild { + data: iter + .filter_map(|(k, v)| v.value().map(|v| (k.to_vec(), v.to_vec()))) + .collect(), + child_info: i.clone(), + }, + ) + }) + .collect(), + } } /// Execute the given closure `f` with the externalities set and initialized with `storage`. @@ -73,13 +92,7 @@ impl BasicExternalities { storage: &mut sp_core::storage::Storage, f: impl FnOnce() -> R, ) -> R { - let mut ext = Self { - inner: Storage { - top: std::mem::take(&mut storage.top), - children_default: std::mem::take(&mut storage.children_default), - }, - extensions: Default::default(), - }; + let mut ext = Self::new(std::mem::take(storage)); let r = ext.execute_with(f); @@ -108,15 +121,26 @@ impl BasicExternalities { impl PartialEq for BasicExternalities { fn eq(&self, other: &BasicExternalities) -> bool { - self.inner.top.eq(&other.inner.top) && - self.inner.children_default.eq(&other.inner.children_default) + self.overlay.changes().map(|(k, v)| (k, v.value())).collect::>() == + other.overlay.changes().map(|(k, v)| (k, v.value())).collect::>() && + self.overlay + .children() + .map(|(iter, i)| (i, iter.map(|(k, v)| (k, v.value())).collect::>())) + .collect::>() == + other + .overlay + .children() + .map(|(iter, i)| { + (i, iter.map(|(k, v)| (k, v.value())).collect::>()) + }) + .collect::>() } } impl FromIterator<(StorageKey, StorageValue)> for BasicExternalities { fn from_iter>(iter: I) -> Self { let mut t = Self::default(); - t.inner.top.extend(iter); + iter.into_iter().for_each(|(k, v)| t.insert(k, v)); t } } @@ -128,11 +152,8 @@ impl Default for BasicExternalities { } impl From> for BasicExternalities { - fn from(hashmap: BTreeMap) -> Self { - BasicExternalities { - inner: Storage { top: hashmap, children_default: Default::default() }, - extensions: Default::default(), - } + fn from(map: BTreeMap) -> Self { + Self::from_iter(map.into_iter()) } } @@ -140,7 +161,7 @@ impl Externalities for BasicExternalities { fn set_offchain_storage(&mut self, _key: &[u8], _value: Option<&[u8]>) {} fn storage(&self, key: &[u8]) -> Option { - self.inner.top.get(key).cloned() + self.overlay.storage(key).and_then(|v| v.map(|v| v.to_vec())) } fn storage_hash(&self, key: &[u8]) -> Option> { @@ -148,11 +169,7 @@ impl Externalities for BasicExternalities { } fn child_storage(&self, child_info: &ChildInfo, key: &[u8]) -> Option { - self.inner - .children_default - .get(child_info.storage_key()) - .and_then(|child| child.data.get(key)) - .cloned() + self.overlay.child_storage(child_info, key).and_then(|v| v.map(|v| v.to_vec())) } fn child_storage_hash(&self, child_info: &ChildInfo, key: &[u8]) -> Option> { @@ -160,16 +177,13 @@ impl Externalities for BasicExternalities { } fn next_storage_key(&self, key: &[u8]) -> Option { - let range = (Bound::Excluded(key), Bound::Unbounded); - self.inner.top.range::<[u8], _>(range).next().map(|(k, _)| k).cloned() + self.overlay.iter_after(key).find_map(|(k, v)| v.value().map(|_| k.to_vec())) } fn next_child_storage_key(&self, child_info: &ChildInfo, key: &[u8]) -> Option { - let range = (Bound::Excluded(key), Bound::Unbounded); - self.inner - .children_default - .get(child_info.storage_key()) - .and_then(|child| child.data.range::<[u8], _>(range).next().map(|(k, _)| k).cloned()) + self.overlay + .child_iter_after(child_info.storage_key(), key) + .find_map(|(k, v)| v.value().map(|_| k.to_vec())) } fn place_storage(&mut self, key: StorageKey, maybe_value: Option) { @@ -178,14 +192,7 @@ impl Externalities for BasicExternalities { return } - match maybe_value { - Some(value) => { - self.inner.top.insert(key, value); - }, - None => { - self.inner.top.remove(&key); - }, - } + self.overlay.set_storage(key, maybe_value) } fn place_child_storage( @@ -194,19 +201,7 @@ impl Externalities for BasicExternalities { key: StorageKey, value: Option, ) { - let child_map = self - .inner - .children_default - .entry(child_info.storage_key().to_vec()) - .or_insert_with(|| StorageChild { - data: Default::default(), - child_info: child_info.to_owned(), - }); - if let Some(value) = value { - child_map.data.insert(key, value); - } else { - child_map.data.remove(&key); - } + self.overlay.set_child_storage(child_info, key, value); } fn kill_child_storage( @@ -215,12 +210,7 @@ impl Externalities for BasicExternalities { _maybe_limit: Option, _maybe_cursor: Option<&[u8]>, ) -> MultiRemovalResults { - let count = self - .inner - .children_default - .remove(child_info.storage_key()) - .map(|c| c.data.len()) - .unwrap_or(0) as u32; + let count = self.overlay.clear_child_storage(child_info); MultiRemovalResults { maybe_cursor: None, backend: count, unique: count, loops: count } } @@ -239,19 +229,7 @@ impl Externalities for BasicExternalities { return MultiRemovalResults { maybe_cursor, backend: 0, unique: 0, loops: 0 } } - let to_remove = self - .inner - .top - .range::<[u8], _>((Bound::Included(prefix), Bound::Unbounded)) - .map(|(k, _)| k) - .take_while(|k| k.starts_with(prefix)) - .cloned() - .collect::>(); - - let count = to_remove.len() as u32; - for key in to_remove { - self.inner.top.remove(&key); - } + let count = self.overlay.clear_prefix(prefix); MultiRemovalResults { maybe_cursor: None, backend: count, unique: count, loops: count } } @@ -262,56 +240,37 @@ impl Externalities for BasicExternalities { _maybe_limit: Option, _maybe_cursor: Option<&[u8]>, ) -> MultiRemovalResults { - if let Some(child) = self.inner.children_default.get_mut(child_info.storage_key()) { - let to_remove = child - .data - .range::<[u8], _>((Bound::Included(prefix), Bound::Unbounded)) - .map(|(k, _)| k) - .take_while(|k| k.starts_with(prefix)) - .cloned() - .collect::>(); - - let count = to_remove.len() as u32; - for key in to_remove { - child.data.remove(&key); - } - MultiRemovalResults { maybe_cursor: None, backend: count, unique: count, loops: count } - } else { - MultiRemovalResults { maybe_cursor: None, backend: 0, unique: 0, loops: 0 } - } + let count = self.overlay.clear_child_prefix(child_info, prefix); + MultiRemovalResults { maybe_cursor: None, backend: count, unique: count, loops: count } } fn storage_append(&mut self, key: Vec, value: Vec) { - let current = self.inner.top.entry(key).or_default(); - crate::ext::StorageAppend::new(current).append(value); + let current_value = self.overlay.value_mut_or_insert_with(&key, || Default::default()); + crate::ext::StorageAppend::new(current_value).append(value); } fn storage_root(&mut self, state_version: StateVersion) -> Vec { - let mut top = self.inner.top.clone(); - let prefixed_keys: Vec<_> = self - .inner - .children_default - .iter() - .map(|(_k, v)| (v.child_info.prefixed_storage_key(), v.child_info.clone())) - .collect(); + let mut top = self + .overlay + .changes() + .filter_map(|(k, v)| v.value().map(|v| (k.clone(), v.clone()))) + .collect::>(); // Single child trie implementation currently allows using the same child // empty root for all child trie. Using null storage key until multiple // type of child trie support. let empty_hash = empty_child_trie_root::>(); - for (prefixed_storage_key, child_info) in prefixed_keys { + for child_info in self.overlay.children().map(|d| d.1.clone()).collect::>() { let child_root = self.child_storage_root(&child_info, state_version); if empty_hash[..] == child_root[..] { - top.remove(prefixed_storage_key.as_slice()); + top.remove(child_info.prefixed_storage_key().as_slice()); } else { - top.insert(prefixed_storage_key.into_inner(), child_root); + top.insert(child_info.prefixed_storage_key().into_inner(), child_root); } } match state_version { - StateVersion::V0 => - LayoutV0::::trie_root(self.inner.top.clone()).as_ref().into(), - StateVersion::V1 => - LayoutV1::::trie_root(self.inner.top.clone()).as_ref().into(), + StateVersion::V0 => LayoutV0::::trie_root(top).as_ref().into(), + StateVersion::V1 => LayoutV1::::trie_root(top).as_ref().into(), } } @@ -320,10 +279,11 @@ impl Externalities for BasicExternalities { child_info: &ChildInfo, state_version: StateVersion, ) -> Vec { - if let Some(child) = self.inner.children_default.get(child_info.storage_key()) { - let delta = child.data.iter().map(|(k, v)| (k.as_ref(), Some(v.as_ref()))); + if let Some((data, child_info)) = self.overlay.child_changes(child_info.storage_key()) { + let delta = + data.into_iter().map(|(k, v)| (k.as_ref(), v.value().map(|v| v.as_slice()))); crate::in_memory_backend::new_in_mem::>() - .child_storage_root(&child.child_info, delta, state_version) + .child_storage_root(&child_info, delta, state_version) .0 } else { empty_child_trie_root::>() @@ -332,15 +292,15 @@ impl Externalities for BasicExternalities { } fn storage_start_transaction(&mut self) { - unimplemented!("Transactions are not supported by BasicExternalities"); + self.overlay.start_transaction() } fn storage_rollback_transaction(&mut self) -> Result<(), ()> { - unimplemented!("Transactions are not supported by BasicExternalities"); + self.overlay.rollback_transaction().map_err(drop) } fn storage_commit_transaction(&mut self) -> Result<(), ()> { - unimplemented!("Transactions are not supported by BasicExternalities"); + self.overlay.commit_transaction().map_err(drop) } fn wipe(&mut self) {} diff --git a/primitives/state-machine/src/overlayed_changes/changeset.rs b/primitives/state-machine/src/overlayed_changes/changeset.rs index ae5d47f6bb943..e5dad7157c731 100644 --- a/primitives/state-machine/src/overlayed_changes/changeset.rs +++ b/primitives/state-machine/src/overlayed_changes/changeset.rs @@ -57,7 +57,7 @@ pub struct NotInRuntime; /// Describes in which mode the node is currently executing. #[derive(Debug, Clone, Copy)] pub enum ExecutionMode { - /// Exeuting in client mode: Removal of all transactions possible. + /// Executing in client mode: Removal of all transactions possible. Client, /// Executing in runtime mode: Transactions started by the client are protected. Runtime, @@ -95,7 +95,7 @@ pub type OverlayedChangeSet = OverlayedMap>; /// Holds a set of changes with the ability modify them using nested transactions. #[derive(Debug, Clone)] -pub struct OverlayedMap { +pub struct OverlayedMap { /// Stores the changes that this overlay constitutes. changes: BTreeMap>, /// Stores which keys are dirty per transaction. Needed in order to determine which @@ -110,7 +110,7 @@ pub struct OverlayedMap { execution_mode: ExecutionMode, } -impl Default for OverlayedMap { +impl Default for OverlayedMap { fn default() -> Self { Self { changes: BTreeMap::new(), @@ -121,6 +121,31 @@ impl Default for OverlayedMap { } } +#[cfg(feature = "std")] +impl From for OverlayedMap> { + fn from(storage: sp_core::storage::StorageMap) -> Self { + Self { + changes: storage + .into_iter() + .map(|(k, v)| { + ( + k, + OverlayedEntry { + transactions: SmallVec::from_iter([InnerValue { + value: Some(v), + extrinsics: Default::default(), + }]), + }, + ) + }) + .collect(), + dirty_keys: Default::default(), + num_client_transactions: 0, + execution_mode: ExecutionMode::Client, + } + } +} + impl Default for ExecutionMode { fn default() -> Self { Self::Client diff --git a/primitives/state-machine/src/overlayed_changes/mod.rs b/primitives/state-machine/src/overlayed_changes/mod.rs index 746599a6768e6..001b4b656c34e 100644 --- a/primitives/state-machine/src/overlayed_changes/mod.rs +++ b/primitives/state-machine/src/overlayed_changes/mod.rs @@ -638,6 +638,21 @@ impl OverlayedChanges { } } +#[cfg(feature = "std")] +impl From for OverlayedChanges { + fn from(storage: sp_core::storage::Storage) -> Self { + Self { + top: storage.top.into(), + children: storage + .children_default + .into_iter() + .map(|(k, v)| (k, (v.data.into(), v.child_info))) + .collect(), + ..Default::default() + } + } +} + #[cfg(feature = "std")] fn retain_map(map: &mut Map, f: F) where