Clean up #[transactional] (#11546)

* Deprecate #[transactional] attribute

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Remove #[transactional] from nomination pools

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Review fix

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Fix NOOP test

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Suppress warnings

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

substrate/frame/nomination-pools/src/lib.rs

@@ -1082,7 +1082,7 @@ impl<T: Config> Get<u32> for TotalUnbondingPools<T> {
 #[frame_support::pallet]
 pub mod pallet {
 	use super::*;
-	use frame_support::{traits::StorageVersion, transactional};
+	use frame_support::traits::StorageVersion;
 	use frame_system::{ensure_signed, pallet_prelude::*};
 
 	/// The current storage version.
@@ -1349,7 +1349,6 @@ pub mod pallet {
 		///   `existential deposit + amount` in their account.
 		/// * Only a pool with [`PoolState::Open`] can be joined
 		#[pallet::weight(T::WeightInfo::join())]
-		#[transactional]
 		pub fn join(
 			origin: OriginFor<T>,
 			#[pallet::compact] amount: BalanceOf<T>,
@@ -1410,7 +1409,6 @@ pub mod pallet {
 			T::WeightInfo::bond_extra_transfer()
 			.max(T::WeightInfo::bond_extra_reward())
 		)]
-		#[transactional]
 		pub fn bond_extra(origin: OriginFor<T>, extra: BondExtra<BalanceOf<T>>) -> DispatchResult {
 			let who = ensure_signed(origin)?;
 			let (mut member, mut bonded_pool, mut reward_pool) = Self::get_member_with_pools(&who)?;
@@ -1449,7 +1447,6 @@ pub mod pallet {
 		/// The member will earn rewards pro rata based on the members stake vs the sum of the
 		/// members in the pools stake. Rewards do not "expire".
 		#[pallet::weight(T::WeightInfo::claim_payout())]
-		#[transactional]
 		pub fn claim_payout(origin: OriginFor<T>) -> DispatchResult {
 			let who = ensure_signed(origin)?;
 			let (mut member, mut bonded_pool, mut reward_pool) = Self::get_member_with_pools(&who)?;
@@ -1489,7 +1486,6 @@ pub mod pallet {
 		/// there are too many unlocking chunks, the result of this call will likely be the
 		/// `NoMoreChunks` error from the staking system.
 		#[pallet::weight(T::WeightInfo::unbond())]
-		#[transactional]
 		pub fn unbond(
 			origin: OriginFor<T>,
 			member_account: T::AccountId,
@@ -1564,7 +1560,6 @@ pub mod pallet {
 		/// would probably see an error like `NoMoreChunks` emitted from the staking system when
 		/// they attempt to unbond.
 		#[pallet::weight(T::WeightInfo::pool_withdraw_unbonded(*num_slashing_spans))]
-		#[transactional]
 		pub fn pool_withdraw_unbonded(
 			origin: OriginFor<T>,
 			pool_id: PoolId,
@@ -1601,7 +1596,6 @@ pub mod pallet {
 		#[pallet::weight(
 			T::WeightInfo::withdraw_unbonded_kill(*num_slashing_spans)
 		)]
-		#[transactional]
 		pub fn withdraw_unbonded(
 			origin: OriginFor<T>,
 			member_account: T::AccountId,
@@ -1717,7 +1711,6 @@ pub mod pallet {
 		/// In addition to `amount`, the caller will transfer the existential deposit; so the caller
 		/// needs at have at least `amount + existential_deposit` transferrable.
 		#[pallet::weight(T::WeightInfo::create())]
-		#[transactional]
 		pub fn create(
 			origin: OriginFor<T>,
 			#[pallet::compact] amount: BalanceOf<T>,

substrate/frame/nomination-pools/src/tests.rs

@@ -21,6 +21,7 @@ use frame_support::{
 	assert_noop, assert_ok, assert_storage_noop, bounded_btree_map,
 	storage::{with_transaction, TransactionOutcome},
 };
+use sp_runtime::traits::Dispatchable;
 
 macro_rules! unbonding_pools_with_era {
 	($($k:expr => $v:expr),* $(,)?) => {{
@@ -3256,10 +3257,13 @@ mod create {
 			Balances::make_free_balance_be(&11, 5 + 20);
 
 			// Then
-			assert_noop!(
-				Pools::create(Origin::signed(11), 20, 11, 11, 11),
-				Error::<Runtime>::MaxPoolMembers
-			);
+			let create = Call::Pools(crate::Call::<Runtime>::create {
+				amount: 20,
+				root: 11,
+				nominator: 11,
+				state_toggler: 11,
+			});
+			assert_noop!(create.dispatch(Origin::signed(11)), Error::<Runtime>::MaxPoolMembers);
 		});
 	}
 }

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

@@ -429,6 +429,7 @@ pub fn pallet(attr: TokenStream, item: TokenStream) -> TokenStream {
 /// }
 /// ```
 #[proc_macro_attribute]
+#[deprecated(note = "This is now the default behaviour for all extrinsics.")]
 pub fn transactional(attr: TokenStream, input: TokenStream) -> TokenStream {
 	transactional::transactional(attr, input).unwrap_or_else(|e| e.to_compile_error().into())
 }

substrate/frame/support/src/dispatch.rs

@@ -200,6 +200,7 @@ impl<T> Parameter for T where T: Codec + EncodeLike + Clone + Eq + fmt::Debug +
 ///
 /// 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\].
+/// The #\[transactional\] attribute is deprecated since it is the default behaviour.
 ///
 /// ```
 /// # #[macro_use]

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

@@ -15,6 +15,9 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+// Disable warnings for #\[transactional\] being deprecated.
+#![allow(deprecated)]
+
 use frame_support::{
 	assert_noop, assert_ok, assert_storage_noop,
 	dispatch::{DispatchError, DispatchResult},