Storage Layer for All FRAME Extrinsics

frame/balances/src/weights.rs

@@ -18,24 +18,22 @@
 //! Autogenerated weights for pallet_balances
 //!
 //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev
-//! DATE: 2022-01-30, STEPS: `50`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]`
+//! DATE: 2022-05-19, STEPS: `50`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]`
 //! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024
 
 // Executed Command:
-// ./target/production/substrate
+// target/production/substrate
 // benchmark
+// pallet
 // --chain=dev
 // --steps=50
 // --repeat=20
 // --pallet=pallet_balances
 // --extrinsic=*
 // --execution=wasm
 // --wasm-execution=compiled
-// --heap-pages=4096
 // --output=./frame/balances/src/weights.rs
-// --template=.maintain/frame-weight-template.hbs
-// --header=HEADER-APACHE2
-// --raw
+// --template=./.maintain/frame-weight-template.hbs
 
 #![cfg_attr(rustfmt, rustfmt_skip)]
 #![allow(unused_parens)]
@@ -60,43 +58,43 @@ pub struct SubstrateWeight<T>(PhantomData<T>);
 impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
 	// Storage: System Account (r:1 w:1)
 	fn transfer() -> Weight {
-		(34_200_000 as Weight)
+		(40_167_000 as Weight)
 			.saturating_add(T::DbWeight::get().reads(1 as Weight))
 			.saturating_add(T::DbWeight::get().writes(1 as Weight))
 	}
 	// Storage: System Account (r:1 w:1)
 	fn transfer_keep_alive() -> Weight {
-		(27_263_000 as Weight)
+		(31_669_000 as Weight)
 			.saturating_add(T::DbWeight::get().reads(1 as Weight))
 			.saturating_add(T::DbWeight::get().writes(1 as Weight))
 	}
 	// Storage: System Account (r:1 w:1)
 	fn set_balance_creating() -> Weight {
-		(17_425_000 as Weight)
+		(21_660_000 as Weight)
 			.saturating_add(T::DbWeight::get().reads(1 as Weight))
 			.saturating_add(T::DbWeight::get().writes(1 as Weight))
 	}
 	// Storage: System Account (r:1 w:1)
 	fn set_balance_killing() -> Weight {
-		(19_979_000 as Weight)
+		(24_558_000 as Weight)
 			.saturating_add(T::DbWeight::get().reads(1 as Weight))
 			.saturating_add(T::DbWeight::get().writes(1 as Weight))
 	}
 	// Storage: System Account (r:2 w:2)
 	fn force_transfer() -> Weight {
-		(34_783_000 as Weight)
+		(40_024_000 as Weight)
 			.saturating_add(T::DbWeight::get().reads(2 as Weight))
 			.saturating_add(T::DbWeight::get().writes(2 as Weight))
 	}
 	// Storage: System Account (r:1 w:1)
 	fn transfer_all() -> Weight {
-		(31_620_000 as Weight)
+		(36_806_000 as Weight)
 			.saturating_add(T::DbWeight::get().reads(1 as Weight))
 			.saturating_add(T::DbWeight::get().writes(1 as Weight))
 	}
 	// Storage: System Account (r:1 w:1)
 	fn force_unreserve() -> Weight {
-		(15_750_000 as Weight)
+		(19_292_000 as Weight)
 			.saturating_add(T::DbWeight::get().reads(1 as Weight))
 			.saturating_add(T::DbWeight::get().writes(1 as Weight))
 	}
@@ -106,43 +104,43 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
 impl WeightInfo for () {
 	// Storage: System Account (r:1 w:1)
 	fn transfer() -> Weight {
-		(34_200_000 as Weight)
+		(40_167_000 as Weight)
 			.saturating_add(RocksDbWeight::get().reads(1 as Weight))
 			.saturating_add(RocksDbWeight::get().writes(1 as Weight))
 	}
 	// Storage: System Account (r:1 w:1)
 	fn transfer_keep_alive() -> Weight {
-		(27_263_000 as Weight)
+		(31_669_000 as Weight)
 			.saturating_add(RocksDbWeight::get().reads(1 as Weight))
 			.saturating_add(RocksDbWeight::get().writes(1 as Weight))
 	}
 	// Storage: System Account (r:1 w:1)
 	fn set_balance_creating() -> Weight {
-		(17_425_000 as Weight)
+		(21_660_000 as Weight)
 			.saturating_add(RocksDbWeight::get().reads(1 as Weight))
 			.saturating_add(RocksDbWeight::get().writes(1 as Weight))
 	}
 	// Storage: System Account (r:1 w:1)
 	fn set_balance_killing() -> Weight {
-		(19_979_000 as Weight)
+		(24_558_000 as Weight)
 			.saturating_add(RocksDbWeight::get().reads(1 as Weight))
 			.saturating_add(RocksDbWeight::get().writes(1 as Weight))
 	}
 	// Storage: System Account (r:2 w:2)
 	fn force_transfer() -> Weight {
-		(34_783_000 as Weight)
+		(40_024_000 as Weight)
 			.saturating_add(RocksDbWeight::get().reads(2 as Weight))
 			.saturating_add(RocksDbWeight::get().writes(2 as Weight))
 	}
 	// Storage: System Account (r:1 w:1)
 	fn transfer_all() -> Weight {
-		(31_620_000 as Weight)
+		(36_806_000 as Weight)
 			.saturating_add(RocksDbWeight::get().reads(1 as Weight))
 			.saturating_add(RocksDbWeight::get().writes(1 as Weight))
 	}
 	// Storage: System Account (r:1 w:1)
 	fn force_unreserve() -> Weight {
-		(15_750_000 as Weight)
+		(19_292_000 as Weight)
 			.saturating_add(RocksDbWeight::get().reads(1 as Weight))
 			.saturating_add(RocksDbWeight::get().writes(1 as Weight))
 	}

frame/support/procedural/src/construct_runtime/expand/call.rs

@@ -127,6 +127,19 @@ pub fn expand_outer_dispatch(
 				}
 			}
 		}
+		impl #scrate::traits::DispatchableWithStorageLayer for Call {
+			type Origin = Origin;
+			fn dispatch_with_storage_layer(self, origin: Origin) -> #scrate::dispatch::DispatchResultWithPostInfo {
+				#scrate::storage::with_storage_layer(|| {
+					#scrate::dispatch::Dispatchable::dispatch(self, origin)
+				})
+			}
+			fn dispatch_bypass_filter_with_storage_layer(self, origin: Origin) -> #scrate::dispatch::DispatchResultWithPostInfo {
+				#scrate::storage::with_storage_layer(|| {
+					#scrate::traits::UnfilteredDispatchable::dispatch_bypass_filter(self, origin)
+				})
+			}
+		}
 
 		#(
 			impl #scrate::traits::IsSubType<#scrate::dispatch::CallableCallFor<#pallet_names, #runtime>> for Call {

frame/support/procedural/src/lib.rs

@@ -433,6 +433,12 @@ pub fn transactional(attr: TokenStream, input: TokenStream) -> TokenStream {
 	transactional::transactional(attr, input).unwrap_or_else(|e| e.to_compile_error().into())
 }
 
+#[proc_macro_attribute]
+pub fn require_transactional(attr: TokenStream, input: TokenStream) -> TokenStream {
+	transactional::require_transactional(attr, input)
+		.unwrap_or_else(|e| e.to_compile_error().into())
+}
+
 /// Derive [`Clone`] but do not bound any generic. Docs are at `frame_support::CloneNoBound`.
 #[proc_macro_derive(CloneNoBound)]
 pub fn derive_clone_no_bound(input: TokenStream) -> TokenStream {
@@ -510,12 +516,6 @@ pub fn derive_default_no_bound(input: TokenStream) -> TokenStream {
 	default_no_bound::derive_default_no_bound(input)
 }
 
-#[proc_macro_attribute]
-pub fn require_transactional(attr: TokenStream, input: TokenStream) -> TokenStream {
-	transactional::require_transactional(attr, input)
-		.unwrap_or_else(|e| e.to_compile_error().into())
-}
-
 #[proc_macro]
 pub fn crate_to_crate_version(input: TokenStream) -> TokenStream {
 	crate_version::crate_to_crate_version(input)

frame/support/procedural/src/pallet/expand/call.rs

@@ -265,8 +265,12 @@ 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))
 							);
-							<#pallet_ident<#type_use_gen>>::#fn_name(origin, #( #args_name, )* )
-								.map(Into::into).map_err(Into::into)
+							// We execute all dispatchable in at least one storage layer, allowing them
+							// to return an error at any point, and undoing any storage changes.
+							#frame_support::storage::in_storage_layer(|| {
+								<#pallet_ident<#type_use_gen>>::#fn_name(origin, #( #args_name, )* )
+									.map(Into::into).map_err(Into::into)
+							})
 						},
 					)*
 					Self::__Ignore(_, _) => {

frame/support/src/dispatch.rs

@@ -27,7 +27,8 @@ pub use crate::{
 		result,
 	},
 	traits::{
-		CallMetadata, GetCallMetadata, GetCallName, GetStorageVersion, UnfilteredDispatchable,
+		CallMetadata, DispatchableWithStorageLayer, GetCallMetadata, GetCallName,
+		GetStorageVersion, UnfilteredDispatchable,
 	},
 	weights::{
 		ClassifyDispatch, DispatchInfo, GetDispatchInfo, PaysFee, PostDispatchInfo,

frame/support/src/storage/mod.rs

@@ -30,7 +30,9 @@ use sp_runtime::generic::{Digest, DigestItem};
 use sp_std::prelude::*;
 
 pub use self::{
-	transactional::{with_transaction, with_transaction_unchecked},
+	transactional::{
+		in_storage_layer, with_storage_layer, with_transaction, with_transaction_unchecked,
+	},
 	types::StorageEntryMetadataBuilder,
 };
 pub use sp_runtime::TransactionOutcome;

frame/support/src/storage/transactional.rs

@@ -158,6 +158,40 @@ pub fn with_transaction_unchecked<R>(f: impl FnOnce() -> TransactionOutcome<R>)
 	}
 }
 
+/// Execute the supplied function, adding a new storage layer.
+///
+/// This is the same as `with_transaction`, but assuming that any function returning
+/// an `Err` should rollback, and any function returning `Ok` should commit. This
+/// provides a cleaner API to the developer who wants this behavior.
+pub fn with_storage_layer<T, E>(f: impl FnOnce() -> Result<T, E>) -> Result<T, E>
+where
+	E: From<DispatchError>,
+{
+	with_transaction(|| {
+		let r = f();
+		if r.is_ok() {
+			TransactionOutcome::Commit(r)
+		} else {
+			TransactionOutcome::Rollback(r)
+		}
+	})
+}
+
+/// Execute the supplied function, ensuring we are at least in one storage layer.
+///
+/// If we are already in a storage layer, we just execute the provided closure.
+/// If we are not, we execute the closure within a [`with_storage_layer`].
+pub fn in_storage_layer<T, E>(f: impl FnOnce() -> Result<T, E>) -> Result<T, E>
+where
+	E: From<DispatchError>,
+{
+	if is_transactional() {
+		f()
+	} else {
+		with_storage_layer(f)
+	}
+}
+
 #[cfg(test)]
 mod tests {
 	use super::*;
@@ -229,4 +263,33 @@ mod tests {
 			assert_eq!(get_transaction_level(), 0);
 		});
 	}
+
+	#[test]
+	fn in_storage_layer_works() {
+		TestExternalities::default().execute_with(|| {
+			assert_eq!(get_transaction_level(), 0);
+
+			let res = in_storage_layer(|| -> DispatchResult {
+				assert_eq!(get_transaction_level(), 1);
+				in_storage_layer(|| -> DispatchResult {
+					// We are still in the same layer :)
+					assert_eq!(get_transaction_level(), 1);
+					Ok(())
+				})
+			});
+
+			assert_ok!(res);
+
+			let res = in_storage_layer(|| -> DispatchResult {
+				assert_eq!(get_transaction_level(), 1);
+				in_storage_layer(|| -> DispatchResult {
+					// We are still in the same layer :)
+					assert_eq!(get_transaction_level(), 1);
+					Err("epic fail".into())
+				})
+			});
+
+			assert_noop!(res, "epic fail");
+		});
+	}
 }

frame/support/src/traits.rs

@@ -94,8 +94,8 @@ pub use storage::{
 
 mod dispatch;
 pub use dispatch::{
-	AsEnsureOriginWithArg, EnsureOneOf, EnsureOrigin, EnsureOriginWithArg, OriginTrait,
-	UnfilteredDispatchable,
+	AsEnsureOriginWithArg, DispatchableWithStorageLayer, EnsureOneOf, EnsureOrigin,
+	EnsureOriginWithArg, OriginTrait, UnfilteredDispatchable,
 };
 
 mod voting;

frame/support/src/traits/dispatch.rs

@@ -101,6 +101,23 @@ pub trait UnfilteredDispatchable {
 	fn dispatch_bypass_filter(self, origin: Self::Origin) -> DispatchResultWithPostInfo;
 }
 
+/// Type that can be dispatched with an additional storage layer which is used to execute the call.
+pub trait DispatchableWithStorageLayer {
+	/// The origin type of the runtime, (i.e. `frame_system::Config::Origin`).
+	type Origin;
+
+	/// Same as `dispatch` from the `Dispatchable` trait, but specifically spawns a new storage
+	/// layer to execute the call inside of.
+	fn dispatch_with_storage_layer(self, origin: Self::Origin) -> DispatchResultWithPostInfo;
+
+	/// Same as `dispatch_bypass_filter` from the `UnfilteredDispatchable` trait, but specifically
+	/// spawns a new storage layer to execute the call inside of.
+	fn dispatch_bypass_filter_with_storage_layer(
+		self,
+		origin: Self::Origin,
+	) -> DispatchResultWithPostInfo;
+}
+
 /// Methods available on `frame_system::Config::Origin`.
 pub trait OriginTrait: Sized {
 	/// Runtime call type, as in `frame_system::Config::Call`