From 0fd5b85061e6aba6c1ca33c72501bba2aa9f5230 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Thu, 28 Jul 2022 00:53:42 +0200 Subject: [PATCH 01/14] transactional: Wrap `pallet::calls` directly in storage layers Before this pr we only wrapped `pallet::calls` into storage layers when executing the calls with `dispatch`. This pr is solving that by wrapping each call function inside a storage layer. --- .../procedural/src/pallet/expand/call.rs | 26 ++++++++++++++----- .../procedural/src/pallet/parse/call.rs | 20 +++++++------- frame/support/test/tests/storage_layers.rs | 2 ++ 3 files changed, 32 insertions(+), 16 deletions(-) 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..bff462d8ed919 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_impl.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/test/tests/storage_layers.rs b/frame/support/test/tests/storage_layers.rs index 05ed60fe90196..8c5d0ff6d7dea 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!(Pallet::::set_value(Origin::signed(1), 1), "Revert!"); }); } From 028cf635574fee70760bac9196fb62c605d1f842 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Thu, 28 Jul 2022 14:01:35 +0200 Subject: [PATCH 02/14] Teach `BasicExternalities` transactions support --- primitives/state-machine/src/basic.rs | 192 +++++++----------- .../src/overlayed_changes/changeset.rs | 29 ++- .../src/overlayed_changes/mod.rs | 15 ++ 3 files changed, 117 insertions(+), 119 deletions(-) diff --git a/primitives/state-machine/src/basic.rs b/primitives/state-machine/src/basic.rs index 6efc847bfbdb7..8cde7c084de8a 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)| (iter.map(|(k, v)| (k, v.value())).collect::>(), i)) + .collect::>() == + other + .overlay + .children() + .map(|(iter, i)| { + (iter.map(|(k, v)| (k, v.value())).collect::>(), i) + }) + .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(|v| v.1.value().map(|_| v.0.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(|v| v.1.value().map(|_| v.0.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..a804479f65e67 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,29 @@ 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(), + ..Default::default() + } + } +} + 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 From d80af32f24169269120393b6969e8778cd3b87b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Thu, 28 Jul 2022 16:53:26 +0200 Subject: [PATCH 03/14] Fix crates --- frame/state-trie-migration/src/lib.rs | 105 +++++++++++--------------- frame/transaction-storage/src/lib.rs | 4 +- 2 files changed, 49 insertions(+), 60 deletions(-) diff --git a/frame/state-trie-migration/src/lib.rs b/frame/state-trie-migration/src/lib.rs index 94f6c1f223b9c..45aee106f7859 100644 --- a/frame/state-trie-migration/src/lib.rs +++ b/frame/state-trie-migration/src/lib.rs @@ -235,7 +235,7 @@ 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 +262,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 +301,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 +350,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 +431,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 +516,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 +530,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 +618,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 { @@ -637,8 +638,8 @@ pub mod pallet { match migration { Ok(_) => Ok(post_info), Err(error) => { - Self::halt(&error); - Err(DispatchErrorWithPostInfo { post_info, error }) + Self::halt(error); + Ok(().into()) }, } } @@ -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,13 +742,7 @@ 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(().into()) } else { Self::deposit_event(Event::::Migrated { top: 0, @@ -806,7 +801,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 +844,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 +866,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() } @@ -1285,18 +1280,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 +1315,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 +1475,7 @@ mod test { ..Default::default() } ), - Error::::BadWitness + Error::::BadWitness, ); // migrate all keys in a series of submissions @@ -1547,14 +1538,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 +1572,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/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; From ab7fa66df433f45e61cf3391e67efd33ed9d7faa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Thu, 28 Jul 2022 16:55:56 +0200 Subject: [PATCH 04/14] FMT --- frame/state-trie-migration/src/lib.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frame/state-trie-migration/src/lib.rs b/frame/state-trie-migration/src/lib.rs index 45aee106f7859..22b12e0ffe324 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) -> Result<(), Error> { + 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() { From f24d062ba49460e3e5bee5b72d6478ba3c809dda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 1 Aug 2022 15:50:30 +0200 Subject: [PATCH 05/14] Fix benchmarking tests --- frame/state-trie-migration/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/state-trie-migration/src/lib.rs b/frame/state-trie-migration/src/lib.rs index 22b12e0ffe324..c4ee9ba9dbad5 100644 --- a/frame/state-trie-migration/src/lib.rs +++ b/frame/state-trie-migration/src/lib.rs @@ -959,7 +959,7 @@ mod benchmarks { frame_system::RawOrigin::Signed(caller.clone()).into(), vec![b"foo".to_vec()], 1, - ).is_err() + ).is_ok() ) } verify { @@ -1003,7 +1003,7 @@ mod benchmarks { StateTrieMigration::::childify("top"), vec![b"foo".to_vec()], 1, - ).is_err() + ).is_ok() ) } verify { From fe15f9d9a41bcc0a14f0a92dd24fefa34ec3183a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 2 Aug 2022 12:33:07 +0200 Subject: [PATCH 06/14] Use correct span --- frame/support/procedural/src/pallet/parse/call.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/procedural/src/pallet/parse/call.rs b/frame/support/procedural/src/pallet/parse/call.rs index bff462d8ed919..336e08c3d39b7 100644 --- a/frame/support/procedural/src/pallet/parse/call.rs +++ b/frame/support/procedural/src/pallet/parse/call.rs @@ -290,7 +290,7 @@ impl CallDef { }); } else { let msg = "Invalid pallet::call, only method accepted"; - return Err(syn::Error::new(item_impl.span(), msg)) + return Err(syn::Error::new(item.span(), msg)) } } From 9a82ad555ae9487edd55cfdb5d3b588ceb6644a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 2 Aug 2022 12:45:39 +0200 Subject: [PATCH 07/14] Support old decl macros --- frame/support/src/dispatch.rs | 14 +- frame/support/test/tests/construct_runtime.rs | 178 +++++++++--------- 2 files changed, 99 insertions(+), 93 deletions(-) diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index ae4230efc63f8..7cb848cf4004d 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] From b6095fd45f4eacba93f948e9aa563eba2dddf009 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 2 Aug 2022 13:48:35 +0200 Subject: [PATCH 08/14] Fix test --- frame/support/test/tests/storage_layers.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/test/tests/storage_layers.rs b/frame/support/test/tests/storage_layers.rs index 8c5d0ff6d7dea..7404ccace2c09 100644 --- a/frame/support/test/tests/storage_layers.rs +++ b/frame/support/test/tests/storage_layers.rs @@ -277,6 +277,6 @@ 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!(Pallet::::set_value(Origin::signed(1), 1), "Revert!"); + assert_noop!(decl_pallet::Module::::set_value(Origin::signed(1), 1), "Revert!"); }); } From ad18fa6b9fd70de0b48aa8ef3a92cfabcaaee4cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 9 Aug 2022 21:49:06 +0200 Subject: [PATCH 09/14] Apply suggestions from code review Co-authored-by: Oliver Tale-Yazdi --- primitives/state-machine/src/basic.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/primitives/state-machine/src/basic.rs b/primitives/state-machine/src/basic.rs index 8cde7c084de8a..e31db4afd0f27 100644 --- a/primitives/state-machine/src/basic.rs +++ b/primitives/state-machine/src/basic.rs @@ -177,13 +177,13 @@ impl Externalities for BasicExternalities { } fn next_storage_key(&self, key: &[u8]) -> Option { - self.overlay.iter_after(key).find_map(|v| v.1.value().map(|_| v.0.to_vec())) + 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 { self.overlay .child_iter_after(child_info.storage_key(), key) - .find_map(|v| v.1.value().map(|_| v.0.to_vec())) + .find_map(|(k, v)| v.value().map(|_| k.to_vec())) } fn place_storage(&mut self, key: StorageKey, maybe_value: Option) { From cec575ffd404b518c3148b258fc5c47e3a28e4e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 9 Aug 2022 21:55:14 +0200 Subject: [PATCH 10/14] Update frame/state-trie-migration/src/lib.rs --- frame/state-trie-migration/src/lib.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/frame/state-trie-migration/src/lib.rs b/frame/state-trie-migration/src/lib.rs index c4ee9ba9dbad5..f53293cf329c2 100644 --- a/frame/state-trie-migration/src/lib.rs +++ b/frame/state-trie-migration/src/lib.rs @@ -638,9 +638,7 @@ pub mod pallet { MigrationProcess::::put(task); let post_info = PostDispatchInfo { actual_weight, pays_fee: Pays::No }; - match migration { - Ok(_) => Ok(post_info), - Err(error) => { + if let Err(error) = migration { Self::halt(error); Ok(().into()) }, From 7f400d14ad657c1de663b79ad91b08e3ebdba505 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 9 Aug 2022 21:55:20 +0200 Subject: [PATCH 11/14] Update frame/state-trie-migration/src/lib.rs --- frame/state-trie-migration/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frame/state-trie-migration/src/lib.rs b/frame/state-trie-migration/src/lib.rs index f53293cf329c2..33c92a8ae5401 100644 --- a/frame/state-trie-migration/src/lib.rs +++ b/frame/state-trie-migration/src/lib.rs @@ -641,8 +641,7 @@ pub mod pallet { if let Err(error) = migration { Self::halt(error); Ok(().into()) - }, - } + Ok(post_info) } /// Migrate the list of top keys by iterating each of them one by one. From 1807e1db417c7f2d87274fe03d1e87bc270eff6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 9 Aug 2022 21:55:27 +0200 Subject: [PATCH 12/14] Update frame/state-trie-migration/src/lib.rs --- frame/state-trie-migration/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/state-trie-migration/src/lib.rs b/frame/state-trie-migration/src/lib.rs index 33c92a8ae5401..6532ea586c9d0 100644 --- a/frame/state-trie-migration/src/lib.rs +++ b/frame/state-trie-migration/src/lib.rs @@ -639,8 +639,8 @@ pub mod pallet { MigrationProcess::::put(task); let post_info = PostDispatchInfo { actual_weight, pays_fee: Pays::No }; if let Err(error) = migration { - Self::halt(error); - Ok(().into()) + Self::halt(error); + } Ok(post_info) } From 22e5fab19291859c0327fb8d54e179931e5db99c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 10 Aug 2022 21:51:58 +0200 Subject: [PATCH 13/14] Feedback --- frame/state-trie-migration/src/lib.rs | 15 +++++++++++++-- .../src/overlayed_changes/changeset.rs | 4 +++- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/frame/state-trie-migration/src/lib.rs b/frame/state-trie-migration/src/lib.rs index 6532ea586c9d0..353bc93e5ab94 100644 --- a/frame/state-trie-migration/src/lib.rs +++ b/frame/state-trie-migration/src/lib.rs @@ -742,7 +742,10 @@ pub mod pallet { let (_imbalance, _remainder) = T::Currency::slash(&who, deposit); debug_assert!(_remainder.is_zero()); Self::deposit_event(Event::::Slashed { who, amount: deposit }); - Ok(().into()) + Ok(PostDispatchInfo { + actual_weight: Some(T::WeightInfo::migrate_custom_child_fail()), + pays_fee: Pays::Yes, + }) } else { Self::deposit_event(Event::::Migrated { top: 0, @@ -957,7 +960,15 @@ mod benchmarks { vec![b"foo".to_vec()], 1, ).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()); diff --git a/primitives/state-machine/src/overlayed_changes/changeset.rs b/primitives/state-machine/src/overlayed_changes/changeset.rs index a804479f65e67..e5dad7157c731 100644 --- a/primitives/state-machine/src/overlayed_changes/changeset.rs +++ b/primitives/state-machine/src/overlayed_changes/changeset.rs @@ -139,7 +139,9 @@ impl From for OverlayedMap Date: Wed, 10 Aug 2022 21:52:45 +0200 Subject: [PATCH 14/14] Apply suggestions from code review Co-authored-by: cheme --- primitives/state-machine/src/basic.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/primitives/state-machine/src/basic.rs b/primitives/state-machine/src/basic.rs index e31db4afd0f27..236a515a2412d 100644 --- a/primitives/state-machine/src/basic.rs +++ b/primitives/state-machine/src/basic.rs @@ -125,13 +125,13 @@ impl PartialEq for BasicExternalities { other.overlay.changes().map(|(k, v)| (k, v.value())).collect::>() && self.overlay .children() - .map(|(iter, i)| (iter.map(|(k, v)| (k, v.value())).collect::>(), i)) + .map(|(iter, i)| (i, iter.map(|(k, v)| (k, v.value())).collect::>())) .collect::>() == other .overlay .children() .map(|(iter, i)| { - (iter.map(|(k, v)| (k, v.value())).collect::>(), i) + (i, iter.map(|(k, v)| (k, v.value())).collect::>()) }) .collect::>() }