Implement 'transactional' annotation for runtime functions. (#6763)

* Implement 'transactional' annotation for runtime functions.

* Allow function attributes for dispatchable calls in decl_module.

* decl_module docs: add transactional function example.

* decl_module docs: add function attributes notes.

* Fix license header.

substrate/frame/support/procedural/src/lib.rs

@@ -23,6 +23,7 @@
 
 mod storage;
 mod construct_runtime;
+mod transactional;
 
 use proc_macro::TokenStream;
 
@@ -289,3 +290,28 @@ pub fn decl_storage(input: TokenStream) -> TokenStream {
 pub fn construct_runtime(input: TokenStream) -> TokenStream {
 	construct_runtime::construct_runtime(input)
 }
+
+/// Execute the annotated function in a new storage transaction.
+///
+/// The return type of the annotated function must be `Result`. All changes to storage performed
+/// by the annotated function are discarded if it returns `Err`, or committed if `Ok`.
+///
+/// #Example
+///
+/// ```nocompile
+/// #[transactional]
+/// fn value_commits(v: u32) -> result::Result<u32, &'static str> {
+/// 	Value::set(v);
+/// 	Ok(v)
+/// }
+///
+/// #[transactional]
+/// fn value_rollbacks(v: u32) -> result::Result<u32, &'static str> {
+/// 	Value::set(v);
+/// 	Err("nah")
+/// }
+/// ```
+#[proc_macro_attribute]
+pub fn transactional(attr: TokenStream, input: TokenStream) -> TokenStream {
+	transactional::transactional(attr, input)
+}

substrate/frame/support/procedural/src/transactional.rs

@@ -0,0 +1,40 @@
+// This file is part of Substrate.
+
+// Copyright (C) 2020 Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: Apache-2.0
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use proc_macro::TokenStream;
+use quote::quote;
+use syn::{parse_macro_input, ItemFn};
+
+pub fn transactional(_attr: TokenStream, input: TokenStream) -> TokenStream {
+	let ItemFn { attrs, vis, sig, block } = parse_macro_input!(input as ItemFn);
+
+	let output = quote! {
+		#(#attrs)*
+        #vis #sig {
+			use frame_support::storage::{with_transaction, TransactionOutcome};
+			with_transaction(|| {
+				let r = #block;
+				if r.is_ok() {
+					TransactionOutcome::Commit(r)
+				} else {
+					TransactionOutcome::Rollback(r)
+				}
+			})
+        }
+    };
+    output.into()
+}

substrate/frame/support/src/dispatch.rs

@@ -167,6 +167,28 @@ impl<T> Parameter for T where T: Codec + EncodeLike + Clone + Eq + fmt::Debug {}
 /// # fn main() {}
 /// ```
 ///
+/// ### Transactional Function Example
+///
+/// Transactional function discards all changes to storage if it returns `Err`, or commits if
+/// `Ok`, via the #\[transactional\] attribute. Note the attribute must be after #\[weight\].
+///
+/// ```
+/// # #[macro_use]
+/// # extern crate frame_support;
+/// # use frame_support::transactional;
+/// # use frame_system::Trait;
+/// decl_module! {
+/// 	pub struct Module<T: Trait> for enum Call where origin: T::Origin {
+/// 		#[weight = 0]
+/// 		#[transactional]
+/// 		fn my_short_function(origin) {
+///				// Your implementation
+/// 		}
+/// 	}
+/// }
+/// # fn main() {}
+/// ```
+///
 /// ### Privileged Function Example
 ///
 /// A privileged function checks that the origin of the call is `ROOT`.
@@ -189,6 +211,14 @@ impl<T> Parameter for T where T: Codec + EncodeLike + Clone + Eq + fmt::Debug {}
 /// # fn main() {}
 /// ```
 ///
+/// ### Attributes on Functions
+///
+/// Attributes on functions are supported, but must be in the order of:
+/// 1. Optional #\[doc\] attribute.
+/// 2. #\[weight\] attribute.
+/// 3. Optional function attributes, for instance #\[transactional\]. Those function attributes will be written
+/// only on the dispatchable functions implemented on `Module`, not on the `Call` enum variant.
+///
 /// ## Multiple Module Instances Example
 ///
 /// A Substrate module can be built such that multiple instances of the same module can be used within a single
@@ -1015,6 +1045,7 @@ macro_rules! decl_module {
 		[ $( $dispatchables:tt )* ]
 		$(#[doc = $doc_attr:tt])*
 		#[weight = $weight:expr]
+		$(#[$fn_attr:meta])*
 		$fn_vis:vis fn $fn_name:ident(
 			$origin:ident $( , $(#[$codec_attr:ident])* $param_name:ident : $param:ty )* $(,)?
 		) $( -> $result:ty )* { $( $impl:tt )* }
@@ -1039,6 +1070,7 @@ macro_rules! decl_module {
 				$( $dispatchables )*
 				$(#[doc = $doc_attr])*
 				#[weight = $weight]
+				$(#[$fn_attr])*
 				$fn_vis fn $fn_name(
 					$origin $( , $(#[$codec_attr])* $param_name : $param )*
 				) $( -> $result )* { $( $impl )* }
@@ -1066,6 +1098,7 @@ macro_rules! decl_module {
 		{ $( $integrity_test:tt )* }
 		[ $( $dispatchables:tt )* ]
 		$(#[doc = $doc_attr:tt])*
+		$(#[$fn_attr:meta])*
 		$fn_vis:vis fn $fn_name:ident(
 			$from:ident $( , $( #[$codec_attr:ident] )* $param_name:ident : $param:ty )* $(,)?
 		) $( -> $result:ty )* { $( $impl:tt )* }
@@ -1094,6 +1127,7 @@ macro_rules! decl_module {
 		[ $( $dispatchables:tt )* ]
 		$(#[doc = $doc_attr:tt])*
 		$(#[weight = $weight:expr])?
+		$(#[$fn_attr:meta])*
 		$fn_vis:vis fn $fn_name:ident(
 			$origin:ident : T::Origin $( , $( #[$codec_attr:ident] )* $param_name:ident : $param:ty )* $(,)?
 		) $( -> $result:ty )* { $( $impl:tt )* }
@@ -1121,6 +1155,7 @@ macro_rules! decl_module {
 		[ $( $dispatchables:tt )* ]
 		$(#[doc = $doc_attr:tt])*
 		$(#[weight = $weight:expr])?
+		$(#[$fn_attr:meta])*
 		$fn_vis:vis fn $fn_name:ident(
 			origin : $origin:ty $( , $( #[$codec_attr:ident] )* $param_name:ident : $param:ty )* $(,)?
 		) $( -> $result:ty )* { $( $impl:tt )* }
@@ -1148,6 +1183,7 @@ macro_rules! decl_module {
 		[ $( $dispatchables:tt )* ]
 		$(#[doc = $doc_attr:tt])*
 		$(#[weight = $weight:expr])?
+		$(#[$fn_attr:meta])*
 		$fn_vis:vis fn $fn_name:ident(
 			$( $(#[$codec_attr:ident])* $param_name:ident : $param:ty ),* $(,)?
 		) $( -> $result:ty )* { $( $impl:tt )* }
@@ -1410,13 +1446,13 @@ macro_rules! decl_module {
 		$origin_ty:ty;
 		$error_type:ty;
 		$ignore:ident;
-		$(#[doc = $doc_attr:tt])*
+		$(#[$fn_attr:meta])*
 		$vis:vis fn $name:ident (
 			$origin:ident $(, $param:ident : $param_ty:ty )*
 		) { $( $impl:tt )* }
 	) => {
-		$(#[doc = $doc_attr])*
 		#[allow(unreachable_code)]
+		$(#[$fn_attr])*
 		$vis fn $name(
 			$origin: $origin_ty $(, $param: $param_ty )*
 		) -> $crate::dispatch::DispatchResult {
@@ -1432,12 +1468,12 @@ macro_rules! decl_module {
 		$origin_ty:ty;
 		$error_type:ty;
 		$ignore:ident;
-		$(#[doc = $doc_attr:tt])*
+		$(#[$fn_attr:meta])*
 		$vis:vis fn $name:ident (
 			$origin:ident $(, $param:ident : $param_ty:ty )*
 		) -> $result:ty { $( $impl:tt )* }
 	) => {
-		$(#[doc = $doc_attr])*
+		$(#[$fn_attr])*
 		$vis fn $name($origin: $origin_ty $(, $param: $param_ty )* ) -> $result {
 			$crate::sp_tracing::enter_span!(stringify!($name));
 			$( $impl )*
@@ -1569,6 +1605,7 @@ macro_rules! decl_module {
 			$(
 				$(#[doc = $doc_attr:tt])*
 				#[weight = $weight:expr]
+				$(#[$fn_attr:meta])*
 				$fn_vis:vis fn $fn_name:ident(
 					$from:ident $( , $(#[$codec_attr:ident])* $param_name:ident : $param:ty)*
 				) $( -> $result:ty )* { $( $impl:tt )* }
@@ -1654,6 +1691,7 @@ macro_rules! decl_module {
 					$(#[doc = $doc_attr])*
 					///
 					/// NOTE: Calling this function will bypass origin filters.
+					$(#[$fn_attr])*
 					$fn_vis fn $fn_name (
 						$from $(, $param_name : $param )*
 					) $( -> $result )* { $( $impl )* }

substrate/frame/support/src/lib.rs

@@ -267,7 +267,7 @@ macro_rules! ord_parameter_types {
 }
 
 #[doc(inline)]
-pub use frame_support_procedural::{decl_storage, construct_runtime};
+pub use frame_support_procedural::{decl_storage, construct_runtime, transactional};
 
 /// Return Err of the expression: `return Err($expression);`.
 ///

substrate/frame/support/test/tests/storage_transaction.rs

@@ -17,9 +17,11 @@
 
 use codec::{Encode, Decode, EncodeLike};
 use frame_support::{
-	StorageMap, StorageValue, storage::{with_transaction, TransactionOutcome::*},
+	assert_ok, assert_noop, dispatch::{DispatchError, DispatchResult}, transactional, StorageMap, StorageValue,
+	storage::{with_transaction, TransactionOutcome::*},
 };
 use sp_io::TestExternalities;
+use sp_std::result;
 
 pub trait Trait {
 	type Origin;
@@ -27,7 +29,20 @@ pub trait Trait {
 }
 
 frame_support::decl_module! {
-	pub struct Module<T: Trait> for enum Call where origin: T::Origin {}
+	pub struct Module<T: Trait> for enum Call where origin: T::Origin {
+		#[weight = 0]
+		#[transactional]
+		fn value_commits(_origin, v: u32) {
+			Value::set(v);
+		}
+
+		#[weight = 0]
+		#[transactional]
+		fn value_rollbacks(_origin, v: u32) -> DispatchResult {
+			Value::set(v);
+			Err(DispatchError::Other("nah"))
+		}
+	}
 }
 
 frame_support::decl_storage!{
@@ -37,6 +52,11 @@ frame_support::decl_storage!{
 	}
 }
 
+struct Runtime;
+impl Trait for Runtime {
+	type Origin = u32;
+	type BlockNumber = u32;
+}
 
 #[test]
 fn storage_transaction_basic_commit() {
@@ -157,3 +177,36 @@ fn storage_transaction_commit_then_rollback() {
 		assert_eq!(Map::get("val3"), 0);
 	});
 }
+
+#[test]
+fn transactional_annotation() {
+	#[transactional]
+	fn value_commits(v: u32) -> result::Result<u32, &'static str> {
+		Value::set(v);
+		Ok(v)
+	}
+
+	#[transactional]
+	fn value_rollbacks(v: u32) -> result::Result<u32, &'static str> {
+		Value::set(v);
+		Err("nah")
+	}
+
+	TestExternalities::default().execute_with(|| {
+		assert_ok!(value_commits(2), 2);
+		assert_eq!(Value::get(), 2);
+
+		assert_noop!(value_rollbacks(3), "nah");
+	});
+}
+
+#[test]
+fn transactional_annotation_in_decl_module() {
+	TestExternalities::default().execute_with(|| {
+		let origin = 0;
+		assert_ok!(<Module<Runtime>>::value_commits(origin, 2));
+		assert_eq!(Value::get(), 2);
+
+		assert_noop!(<Module<Runtime>>::value_rollbacks(origin, 3), "nah");
+	});
+}