From 442602ce3fc9cdd790f6a91ae3270f96127ef2c7 Mon Sep 17 00:00:00 2001
From: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Date: Tue, 31 May 2022 18:45:07 +0200
Subject: [PATCH] Clean up `#[transactional]` (#11546)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* 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 | 9 +--------
substrate/frame/nomination-pools/src/tests.rs | 12 ++++++++----
substrate/frame/support/procedural/src/lib.rs | 1 +
substrate/frame/support/src/dispatch.rs | 1 +
.../frame/support/test/tests/storage_transaction.rs | 3 +++
5 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/substrate/frame/nomination-pools/src/lib.rs b/substrate/frame/nomination-pools/src/lib.rs
index ff77949cf69..c9d811fa4a2 100644
--- a/substrate/frame/nomination-pools/src/lib.rs
+++ b/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>,
diff --git a/substrate/frame/nomination-pools/src/tests.rs b/substrate/frame/nomination-pools/src/tests.rs
index c16c1c9da9e..93218c78c83 100644
--- a/substrate/frame/nomination-pools/src/tests.rs
+++ b/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);
});
}
}
diff --git a/substrate/frame/support/procedural/src/lib.rs b/substrate/frame/support/procedural/src/lib.rs
index 00204b7a4d9..418fad56b76 100644
--- a/substrate/frame/support/procedural/src/lib.rs
+++ b/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())
}
diff --git a/substrate/frame/support/src/dispatch.rs b/substrate/frame/support/src/dispatch.rs
index 51504c9c36e..0a5092b4119 100644
--- a/substrate/frame/support/src/dispatch.rs
+++ b/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]
diff --git a/substrate/frame/support/test/tests/storage_transaction.rs b/substrate/frame/support/test/tests/storage_transaction.rs
index 848a91a7f5a..9e597969d6c 100644
--- a/substrate/frame/support/test/tests/storage_transaction.rs
+++ b/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},
--
GitLab