transactional: Wrap pallet::calls directly in storage layers

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(_, _) => {

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

@@ -48,8 +48,8 @@ pub struct CallDef {
 	pub docs: Vec<syn::Lit>,
 }
 
-#[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<Self> {
-		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<u8> = 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),
 		})
 	}
 }

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::<Runtime>::set_value(Origin::signed(1), 1), "Revert!");
 	});
 }