Storage Layer for All FRAME Extrinsics

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

@@ -127,6 +127,14 @@ 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)
+				})
+			}
+		}
 
 		#(
 			impl #scrate::traits::IsSubType<#scrate::dispatch::CallableCallFor<#pallet_names, #runtime>> for Call {

frame/support/procedural/src/lib.rs

@@ -432,6 +432,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 {
@@ -509,12 +515,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,16 @@ 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;
+}
+
 /// Methods available on `frame_system::Config::Origin`.
 pub trait OriginTrait: Sized {
 	/// Runtime call type, as in `frame_system::Config::Call`

frame/support/test/tests/pallet.rs

@@ -208,8 +208,7 @@ pub mod pallet {
 
 		/// Doc comment put in metadata
 		#[pallet::weight(1)]
-		#[frame_support::transactional]
-		pub fn foo_transactional(
+		pub fn foo_storage_layer(
 			_origin: OriginFor<T>,
 			#[pallet::compact] foo: u32,
 		) -> DispatchResultWithPostInfo {
@@ -376,7 +375,7 @@ pub mod pallet {
 		fn validate_unsigned(_source: TransactionSource, call: &Self::Call) -> TransactionValidity {
 			let _ = T::AccountId::from(SomeType1); // Test for where clause
 			let _ = T::AccountId::from(SomeType5); // Test for where clause
-			if matches!(call, Call::foo_transactional { .. }) {
+			if matches!(call, Call::foo_storage_layer { .. }) {
 				return Ok(ValidTransaction::default())
 			}
 			Err(TransactionValidityError::Invalid(InvalidTransaction::Call))
@@ -614,13 +613,13 @@ fn transactional_works() {
 	TestExternalities::default().execute_with(|| {
 		frame_system::Pallet::<Runtime>::set_block_number(1);
 
-		pallet::Call::<Runtime>::foo_transactional { foo: 0 }
+		pallet::Call::<Runtime>::foo_storage_layer { foo: 0 }
 			.dispatch_bypass_filter(None.into())
 			.err()
 			.unwrap();
 		assert!(frame_system::Pallet::<Runtime>::events().is_empty());
 
-		pallet::Call::<Runtime>::foo_transactional { foo: 1 }
+		pallet::Call::<Runtime>::foo_storage_layer { foo: 1 }
 			.dispatch_bypass_filter(None.into())
 			.unwrap();
 		assert_eq!(
@@ -643,7 +642,7 @@ fn call_expand() {
 	assert_eq!(call_foo.get_call_name(), "foo");
 	assert_eq!(
 		pallet::Call::<Runtime>::get_call_names(),
-		&["foo", "foo_transactional", "foo_no_post_info"],
+		&["foo", "foo_storage_layer", "foo_no_post_info"],
 	);
 }
 
@@ -747,7 +746,7 @@ fn inherent_expand() {
 			Digest::default(),
 		),
 		vec![UncheckedExtrinsic {
-			function: Call::Example(pallet::Call::foo_transactional { foo: 0 }),
+			function: Call::Example(pallet::Call::foo_storage_layer { foo: 0 }),
 			signature: None,
 		}],
 	);
@@ -788,7 +787,7 @@ fn inherent_expand() {
 				signature: None,
 			},
 			UncheckedExtrinsic {
-				function: Call::Example(pallet::Call::foo_transactional { foo: 0 }),
+				function: Call::Example(pallet::Call::foo_storage_layer { foo: 0 }),
 				signature: None,
 			},
 		],
@@ -810,7 +809,7 @@ fn inherent_expand() {
 				signature: None,
 			},
 			UncheckedExtrinsic {
-				function: Call::Example(pallet::Call::foo_transactional { foo: 0 }),
+				function: Call::Example(pallet::Call::foo_storage_layer { foo: 0 }),
 				signature: None,
 			},
 			UncheckedExtrinsic {
@@ -860,7 +859,7 @@ fn validate_unsigned_expand() {
 	let validity = pallet::Pallet::validate_unsigned(TransactionSource::Local, &call).unwrap_err();
 	assert_eq!(validity, TransactionValidityError::Invalid(InvalidTransaction::Call));
 
-	let call = pallet::Call::<Runtime>::foo_transactional { foo: 0 };
+	let call = pallet::Call::<Runtime>::foo_storage_layer { foo: 0 };
 
 	let validity = pallet::Pallet::validate_unsigned(TransactionSource::External, &call).unwrap();
 	assert_eq!(validity, ValidTransaction::default());

frame/support/test/tests/pallet_instance.rs

@@ -92,8 +92,7 @@ pub mod pallet {
 
 		/// Doc comment put in metadata
 		#[pallet::weight(1)]
-		#[frame_support::transactional]
-		pub fn foo_transactional(
+		pub fn foo_storage_layer(
 			origin: OriginFor<T>,
 			#[pallet::compact] _foo: u32,
 		) -> DispatchResultWithPostInfo {
@@ -315,7 +314,7 @@ fn call_expand() {
 		DispatchInfo { weight: 3, class: DispatchClass::Normal, pays_fee: Pays::Yes }
 	);
 	assert_eq!(call_foo.get_call_name(), "foo");
-	assert_eq!(pallet::Call::<Runtime>::get_call_names(), &["foo", "foo_transactional"]);
+	assert_eq!(pallet::Call::<Runtime>::get_call_names(), &["foo", "foo_storage_layer"]);
 
 	let call_foo = pallet::Call::<Runtime, pallet::Instance1>::foo { foo: 3 };
 	assert_eq!(
@@ -325,7 +324,7 @@ fn call_expand() {
 	assert_eq!(call_foo.get_call_name(), "foo");
 	assert_eq!(
 		pallet::Call::<Runtime, pallet::Instance1>::get_call_names(),
-		&["foo", "foo_transactional"],
+		&["foo", "foo_storage_layer"],
 	);
 }