From c5cde02243b3a7ff7c46245136a14a0d232ab8f5 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Thu, 3 Nov 2022 10:23:27 +0100 Subject: [PATCH 01/50] create enum --- frame/nomination-pools/src/lib.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 3661a7f70b48a..9b0bb3d886f91 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -363,6 +363,15 @@ enum AccountType { Reward, } +/// Actions to be taken +#[derive(Encode, Decode)] +enum ClaimableAction { + /// Increase the bond of a member + Increase, + /// Take out rewards + Withdraw, +} + /// A member in a pool. #[derive(Encode, Decode, MaxEncodedLen, TypeInfo, RuntimeDebugNoBound, CloneNoBound)] #[cfg_attr(feature = "std", derive(frame_support::PartialEqNoBound, DefaultNoBound))] From 3ab9cd73f344ecec7e651651905870401d4e633b Mon Sep 17 00:00:00 2001 From: doordashcon Date: Wed, 9 Nov 2022 19:02:39 +0100 Subject: [PATCH 02/50] logic check --- frame/nomination-pools/src/lib.rs | 92 +++++++++++++++++++++++++---- frame/nomination-pools/src/tests.rs | 84 +++++++++++++++++++++++++- 2 files changed, 164 insertions(+), 12 deletions(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 9b0bb3d886f91..5ff32f3a6a82e 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -363,15 +363,6 @@ enum AccountType { Reward, } -/// Actions to be taken -#[derive(Encode, Decode)] -enum ClaimableAction { - /// Increase the bond of a member - Increase, - /// Take out rewards - Withdraw, -} - /// A member in a pool. #[derive(Encode, Decode, MaxEncodedLen, TypeInfo, RuntimeDebugNoBound, CloneNoBound)] #[cfg_attr(feature = "std", derive(frame_support::PartialEqNoBound, DefaultNoBound))] @@ -1285,6 +1276,10 @@ pub mod pallet { pub type ReversePoolIdLookup = CountedStorageMap<_, Twox64Concat, T::AccountId, PoolId, OptionQuery>; + /// Member decision for operator claimable reward action. + #[pallet::storage] + pub type ClaimableAction = StorageMap<_, Twox64Concat, T::AccountId, bool, ValueQuery>; + #[pallet::genesis_config] pub struct GenesisConfig { pub min_join_bond: BalanceOf, @@ -1442,6 +1437,8 @@ pub mod pallet { PoolIdInUse, /// Pool id provided is not correct/usable. InvalidPoolId, + /// The caller is not the operator for this pool. + NotRoot, } #[derive(Encode, Decode, PartialEq, TypeInfo, frame_support::PalletError, RuntimeDebug)] @@ -1575,6 +1572,68 @@ pub mod pallet { Ok(()) } + // TODO: Update weight info + // Caller to be reinbursed for the transactional fees used? + // Investigate adding helper function to remove code duplication. + // Explore the proxy pallet alternative, allowing root to spend the specified amout of rewards payout to the member. + #[pallet::weight( + T::WeightInfo::bond_extra_transfer() + .max(T::WeightInfo::bond_extra_reward()) + )] + #[transactional] + pub fn root_bond_extra( + origin: OriginFor, + member_account: AccountIdLookupOf, + ) -> DispatchResult { + let who = ensure_signed(origin)?; + let member_account = T::Lookup::lookup(member_account)?; + let (mut member, mut bonded_pool, mut reward_pool) = + Self::get_member_with_pools(&member_account)?; + + ensure!(bonded_pool.is_root(&who), Error::::NotRoot); + ensure!(ClaimableAction::::get(&member_account), Error::::DoesNotHavePermission); + + // IMPORTANT: reward pool records must be updated with the old points. why? + reward_pool.update_records(bonded_pool.id, bonded_pool.points)?; + + // TODO: Investigate need. + debug_assert_eq!(member.pool_id, bonded_pool.id); + + // A member who has no skin in the game anymore cannot claim any rewards. + ensure!(!member.active_points().is_zero(), Error::::FullyUnbonding); + + let current_reward_counter = + reward_pool.current_reward_counter(bonded_pool.id, bonded_pool.points)?; + let pending_rewards = member.pending_rewards(current_reward_counter)?; + + // TODO: Investigate adding conditional here for when the pending_rewards is not zero. + if !pending_rewards.is_zero() { + member.last_recorded_reward_counter = current_reward_counter; + reward_pool.register_claimed_reward(pending_rewards); + } + + // TODO: Investigate adjusting member reward & points after rebonding. + let points_issued = bonded_pool.try_bond_funds( + &bonded_pool.reward_account(), + pending_rewards, + BondType::Later + )?; + + bonded_pool.ok_to_be_open(pending_rewards)?; + member.points = member.points.saturating_add(points_issued); + + Self::deposit_event(Event::::Bonded { + member: member_account.clone(), + pool_id: member.pool_id, + bonded: pending_rewards, + joined: false, + }); + + Self::put_member_with_pools(&member_account, member, bonded_pool, reward_pool); + + Ok(()) + } + /// A bonded member can use this to claim their payout based on the rewards that the pool /// has accumulated since their last claimed payout (OR since joining if this is there first /// time claiming rewards). The payout will be transferred to the member's account. @@ -2088,6 +2147,19 @@ pub mod pallet { ensure!(bonded_pool.can_nominate(&who), Error::::NotNominator); T::Staking::chill(&bonded_pool.bonded_account()) } + + // TODO: For when the pool member leaves or is kiked out, clear storage. + // Update weight info + #[pallet::weight(T::DbWeight::get().reads_writes(1, 1))] + pub fn set_claimable_action(origin: OriginFor, action: bool) -> DispatchResult { + let who = ensure_signed(origin)?; + + ensure!(PoolMembers::::contains_key(&who), Error::::PoolMemberNotFound); + >::mutate(who, |delegate| { + *delegate = action; + }); + Ok(()) + } } #[pallet::hooks] @@ -2274,7 +2346,7 @@ impl Pallet { // We check for zero above .div(current_points) } - + // TODO: Investigate logic here to add conditional event emission on different account types. /// If the member has some rewards, transfer a payout from the reward pool to the member. // Emits events and potentially modifies pool state if any arithmetic saturates, but does // not persist any of the mutable inputs to storage. diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 17fee7cc8dda3..9db9c5f424c05 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -4207,10 +4207,10 @@ mod create { ExtBuilder::default().build_and_execute(|| { let ed = Balances::minimum_balance(); - Balances::make_free_balance_be(&11, StakingMock::minimum_bond() + ed); + Balances::make_free_balance_be(&11, StakingMock::minimum_nominator_bond() + ed); assert_ok!(Pools::create( RuntimeOrigin::signed(11), - StakingMock::minimum_bond(), + StakingMock::minimum_nominator_bond(), 123, 456, 789 @@ -4241,6 +4241,40 @@ mod create { } } +#[test] +fn set_claimable_action_works() { + ExtBuilder::default().build_and_execute(|| { + // Given + Balances::make_free_balance_be(&11, ExistentialDeposit::get() + 2); + assert!(!PoolMembers::::contains_key(&11)); + + // When + assert_ok!(Pools::join(RuntimeOrigin::signed(11), 2, 1)); + + // Then + assert_eq!( + pool_events_since_last_call(), + vec![ + Event::Created { depositor: 10, pool_id: 1 }, + Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::Bonded { member: 11, pool_id: 1, bonded: 2, joined: true }, + ] + ); + + // Give pool operator permission. + assert_eq!(ClaimableAction::::get(11), false); + assert_noop!(Pools::set_claimable_action(RuntimeOrigin::signed(12), true), Error::::PoolMemberNotFound); + assert_ok!(Pools::set_claimable_action(RuntimeOrigin::signed(11), true)); + + // then + assert_eq!(ClaimableAction::::get(11), true); + + // TODO: Pool withdraw unbonding... + ClaimableAction::::remove(11); + assert_eq!(ClaimableAction::::get(11), false); + }); +} + mod nominate { use super::*; @@ -4553,6 +4587,7 @@ mod bond_extra { // when assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(10), BondExtra::Rewards)); + assert_eq!(Balances::free_balance(&default_reward_account()), 7); // then assert_eq!(Balances::free_balance(10), 35); @@ -4584,6 +4619,51 @@ mod bond_extra { ); }) } + + #[test] + fn root_bond_extra_from_rewards() { + ExtBuilder::default().add_members(vec![(20, 20)]).build_and_execute(|| { + Balances::make_free_balance_be(&default_reward_account(), 8); + // ... if which only 3 is claimable to make sure the reward account does not die. + let claimable_reward = 8 - ExistentialDeposit::get(); + // NOTE: easier to read of we use 3, so let's use the number instead of variable. + assert_eq!(claimable_reward, 3, "test is correct if rewards are divisible by 3"); + + // given + assert_eq!(PoolMembers::::get(10).unwrap().points, 10); + assert_eq!(PoolMembers::::get(20).unwrap().points, 20); + assert_eq!(BondedPools::::get(1).unwrap().points, 30); + assert_eq!(Balances::free_balance(10), 35); + assert_eq!(Balances::free_balance(20), 20); + + assert_noop!(Pools::root_bond_extra(RuntimeOrigin::signed(900), 10), Error::::DoesNotHavePermission); + assert_ok!(Pools::set_claimable_action(RuntimeOrigin::signed(10), true)); + // assert_eq!(BondedPools::::get(1).unwrap().member_counter, 10); + assert_ok!(Pools::root_bond_extra(RuntimeOrigin::signed(900), 10)); + // let account_reward = BondedPools::::get(1).reward_account(); + assert_eq!(Balances::free_balance(&default_reward_account()), 7); + + // then + assert_eq!(Balances::free_balance(10), 35); + assert_eq!(PoolMembers::::get(10).unwrap().points, 10 + 1); + assert_eq!(BondedPools::::get(1).unwrap().points, 30 + 1); + + // when + assert_noop!(Pools::root_bond_extra(RuntimeOrigin::signed(900), 20), Error::::DoesNotHavePermission); + assert_ok!(Pools::set_claimable_action(RuntimeOrigin::signed(20), true)); + assert_ok!(Pools::root_bond_extra(RuntimeOrigin::signed(900), 20)); + + // then + assert_eq!(Balances::free_balance(20), 20); + // 20's share of the rewards is the other 2/3 of the rewards, since they have 20/30 of + // the shares + assert_eq!(PoolMembers::::get(20).unwrap().points, 20 + 2); + assert_eq!(BondedPools::::get(1).unwrap().points, 30 + 3); + + // try bond_extra with no rewards. + // TODO: check the new payout after `root_bound_extra` + }) + } } mod update_roles { From 59958d5480d1421a6ea8cbc063b40ab68d6edbfa Mon Sep 17 00:00:00 2001 From: doordashcon Date: Mon, 14 Nov 2022 22:19:04 +0100 Subject: [PATCH 03/50] add benchmarks --- .../nomination-pools/benchmarking/src/lib.rs | 43 ++++++++++++++++++- frame/nomination-pools/src/lib.rs | 7 --- frame/nomination-pools/src/tests.rs | 6 --- 3 files changed, 42 insertions(+), 14 deletions(-) diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index 9b063539152b7..5820f6ede0ee6 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -30,7 +30,7 @@ use frame_system::RawOrigin as RuntimeOrigin; use pallet_nomination_pools::{ BalanceOf, BondExtra, BondedPoolInner, BondedPools, ConfigOp, MaxPoolMembers, MaxPoolMembersPerPool, MaxPools, Metadata, MinCreateBond, MinJoinBond, Pallet as Pools, - PoolMembers, PoolRoles, PoolState, RewardPools, SubPoolsStorage, + PoolMembers, PoolRoles, PoolState, RewardPools, SubPoolsStorage, ClaimableAction, }; use sp_runtime::traits::{Bounded, StaticLookup, Zero}; use sp_staking::{EraIndex, StakingInterface}; @@ -268,6 +268,25 @@ frame_benchmarking::benchmarks! { ); } + root_bond_extra { + let origin_weight = Pools::::depositor_min_bond() * 2u32.into(); + let scenario = ListScenario::::new(origin_weight, true)?; + let scenario_creator1_lookup = T::Lookup::unlookup(scenario.creator1.clone()); + let extra = (scenario.dest_weight - origin_weight).max(CurrencyOf::::minimum_balance()); + + // transfer exactly `extra` to the depositor of the src pool (1), from the reward balance + let reward_account1 = Pools::::create_reward_account(1); + assert!(extra >= CurrencyOf::::minimum_balance()); + CurrencyOf::::deposit_creating(&reward_account1, extra); + + }:_(RuntimeOrigin::Signed(scenario.creator1.clone()), scenario_creator1_lookup) + verify { + assert!( + T::Staking::active_stake(&scenario.origin1).unwrap() >= + scenario.dest_weight + ); + } + claim_payout { let origin_weight = Pools::::depositor_min_bond() * 2u32.into(); let ed = CurrencyOf::::minimum_balance(); @@ -652,6 +671,28 @@ frame_benchmarking::benchmarks! { assert!(T::Staking::nominations(Pools::::create_bonded_account(1)).is_none()); } + set_claimable_action { + // Create a pool + let min_create_bond = Pools::::depositor_min_bond(); + let (depositor, pool_account) = create_pool_account::(0, min_create_bond); + + // Add a new member + let min_join_bond = MinJoinBond::::get().max(CurrencyOf::::minimum_balance()); + let joiner = create_funded_user_with_balance::("joiner", 0, min_join_bond * 2u32.into()); + let joiner_lookup = T::Lookup::unlookup(joiner.clone()); + Pools::::join(RuntimeOrigin::Signed(joiner.clone()).into(), min_join_bond, 1) + .unwrap(); + + // Sanity check join worked + assert_eq!( + T::Staking::active_stake(&pool_account).unwrap(), + min_create_bond + min_join_bond + ); + }:_(RuntimeOrigin::Signed(joiner.clone()), true) + verify { + assert_eq!(ClaimableAction::::get(joiner), true); + } + impl_benchmark_test_suite!( Pallet, crate::mock::new_test_ext(), diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 5ff32f3a6a82e..3e3c92ad24895 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -1573,9 +1573,7 @@ pub mod pallet { } // TODO: Update weight info - // Caller to be reinbursed for the transactional fees used? // Investigate adding helper function to remove code duplication. - // Explore the proxy pallet alternative, allowing root to spend the specified amout of rewards payout to the member. #[pallet::weight( T::WeightInfo::bond_extra_transfer() .max(T::WeightInfo::bond_extra_reward()) @@ -1596,9 +1594,6 @@ pub mod pallet { // IMPORTANT: reward pool records must be updated with the old points. why? reward_pool.update_records(bonded_pool.id, bonded_pool.points)?; - // TODO: Investigate need. - debug_assert_eq!(member.pool_id, bonded_pool.id); - // A member who has no skin in the game anymore cannot claim any rewards. ensure!(!member.active_points().is_zero(), Error::::FullyUnbonding); @@ -1606,13 +1601,11 @@ pub mod pallet { reward_pool.current_reward_counter(bonded_pool.id, bonded_pool.points)?; let pending_rewards = member.pending_rewards(current_reward_counter)?; - // TODO: Investigate adding conditional here for when the pending_rewards is not zero. if !pending_rewards.is_zero() { member.last_recorded_reward_counter = current_reward_counter; reward_pool.register_claimed_reward(pending_rewards); } - // TODO: Investigate adjusting member reward & points after rebonding. let points_issued = bonded_pool.try_bond_funds( &bonded_pool.reward_account(), pending_rewards, diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 9db9c5f424c05..6827d7ac55f63 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -4269,7 +4269,6 @@ fn set_claimable_action_works() { // then assert_eq!(ClaimableAction::::get(11), true); - // TODO: Pool withdraw unbonding... ClaimableAction::::remove(11); assert_eq!(ClaimableAction::::get(11), false); }); @@ -4638,9 +4637,7 @@ mod bond_extra { assert_noop!(Pools::root_bond_extra(RuntimeOrigin::signed(900), 10), Error::::DoesNotHavePermission); assert_ok!(Pools::set_claimable_action(RuntimeOrigin::signed(10), true)); - // assert_eq!(BondedPools::::get(1).unwrap().member_counter, 10); assert_ok!(Pools::root_bond_extra(RuntimeOrigin::signed(900), 10)); - // let account_reward = BondedPools::::get(1).reward_account(); assert_eq!(Balances::free_balance(&default_reward_account()), 7); // then @@ -4659,9 +4656,6 @@ mod bond_extra { // the shares assert_eq!(PoolMembers::::get(20).unwrap().points, 20 + 2); assert_eq!(BondedPools::::get(1).unwrap().points, 30 + 3); - - // try bond_extra with no rewards. - // TODO: check the new payout after `root_bound_extra` }) } } From 9d54b8a895ccb535a477bfa87a1146726a25c741 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Fri, 18 Nov 2022 15:01:34 +0100 Subject: [PATCH 04/50] -enum --- .../nomination-pools/benchmarking/src/lib.rs | 10 ++++--- frame/nomination-pools/src/lib.rs | 29 ++++++++++++++----- frame/nomination-pools/src/tests.rs | 17 +++++------ 3 files changed, 34 insertions(+), 22 deletions(-) diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index 5820f6ede0ee6..4ab2ab2d0c867 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -30,7 +30,7 @@ use frame_system::RawOrigin as RuntimeOrigin; use pallet_nomination_pools::{ BalanceOf, BondExtra, BondedPoolInner, BondedPools, ConfigOp, MaxPoolMembers, MaxPoolMembersPerPool, MaxPools, Metadata, MinCreateBond, MinJoinBond, Pallet as Pools, - PoolMembers, PoolRoles, PoolState, RewardPools, SubPoolsStorage, ClaimableAction, + PoolMembers, PoolRoles, PoolState, RewardPools, SubPoolsStorage, ClaimableAction, BondExtraSource, }; use sp_runtime::traits::{Bounded, StaticLookup, Zero}; use sp_staking::{EraIndex, StakingInterface}; @@ -278,6 +278,8 @@ frame_benchmarking::benchmarks! { let reward_account1 = Pools::::create_reward_account(1); assert!(extra >= CurrencyOf::::minimum_balance()); CurrencyOf::::deposit_creating(&reward_account1, extra); + Pools::::set_claimable_actor(RuntimeOrigin::Signed(scenario.creator1.clone()).into(), BondExtraSource::Operator) + .unwrap(); }:_(RuntimeOrigin::Signed(scenario.creator1.clone()), scenario_creator1_lookup) verify { @@ -671,7 +673,7 @@ frame_benchmarking::benchmarks! { assert!(T::Staking::nominations(Pools::::create_bonded_account(1)).is_none()); } - set_claimable_action { + set_claimable_actor { // Create a pool let min_create_bond = Pools::::depositor_min_bond(); let (depositor, pool_account) = create_pool_account::(0, min_create_bond); @@ -688,9 +690,9 @@ frame_benchmarking::benchmarks! { T::Staking::active_stake(&pool_account).unwrap(), min_create_bond + min_join_bond ); - }:_(RuntimeOrigin::Signed(joiner.clone()), true) + }:_(RuntimeOrigin::Signed(joiner.clone()), BondExtraSource::Operator) verify { - assert_eq!(ClaimableAction::::get(joiner), true); + assert_eq!(ClaimableAction::::get(joiner), BondExtraSource::Operator); } impl_benchmark_test_suite!( diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 3e3c92ad24895..8ecdf1656c83a 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -363,6 +363,21 @@ enum AccountType { Reward, } +// Account to increse the bond of a member +#[derive(Encode, Decode, MaxEncodedLen, Clone, Copy, Debug, PartialEq, Eq, TypeInfo)] +pub enum BondExtraSource { + // Action to be taken by the bonded member of a pool + Origin, + // Action to be taken by the pool operator on the member's behalf + Operator, +} + +impl Default for BondExtraSource { + fn default() -> Self { + Self::Origin + } +} + /// A member in a pool. #[derive(Encode, Decode, MaxEncodedLen, TypeInfo, RuntimeDebugNoBound, CloneNoBound)] #[cfg_attr(feature = "std", derive(frame_support::PartialEqNoBound, DefaultNoBound))] @@ -1276,9 +1291,9 @@ pub mod pallet { pub type ReversePoolIdLookup = CountedStorageMap<_, Twox64Concat, T::AccountId, PoolId, OptionQuery>; - /// Member decision for operator claimable reward action. + /// ? #[pallet::storage] - pub type ClaimableAction = StorageMap<_, Twox64Concat, T::AccountId, bool, ValueQuery>; + pub type ClaimableAction = StorageMap<_, Twox64Concat, T::AccountId, BondExtraSource, ValueQuery>; #[pallet::genesis_config] pub struct GenesisConfig { @@ -1573,7 +1588,6 @@ pub mod pallet { } // TODO: Update weight info - // Investigate adding helper function to remove code duplication. #[pallet::weight( T::WeightInfo::bond_extra_transfer() .max(T::WeightInfo::bond_extra_reward()) @@ -1589,7 +1603,7 @@ pub mod pallet { Self::get_member_with_pools(&member_account)?; ensure!(bonded_pool.is_root(&who), Error::::NotRoot); - ensure!(ClaimableAction::::get(&member_account), Error::::DoesNotHavePermission); + ensure!(ClaimableAction::::get(&member_account) == BondExtraSource::Operator, Error::::DoesNotHavePermission); // IMPORTANT: reward pool records must be updated with the old points. why? reward_pool.update_records(bonded_pool.id, bonded_pool.points)?; @@ -2141,15 +2155,14 @@ pub mod pallet { T::Staking::chill(&bonded_pool.bonded_account()) } - // TODO: For when the pool member leaves or is kiked out, clear storage. // Update weight info #[pallet::weight(T::DbWeight::get().reads_writes(1, 1))] - pub fn set_claimable_action(origin: OriginFor, action: bool) -> DispatchResult { + pub fn set_claimable_actor(origin: OriginFor, actor: BondExtraSource) -> DispatchResult { let who = ensure_signed(origin)?; ensure!(PoolMembers::::contains_key(&who), Error::::PoolMemberNotFound); - >::mutate(who, |delegate| { - *delegate = action; + >::mutate(who, |source| { + *source = actor; }); Ok(()) } diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 6827d7ac55f63..2d7b27481584d 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -4242,7 +4242,7 @@ mod create { } #[test] -fn set_claimable_action_works() { +fn set_claimable_actor_works() { ExtBuilder::default().build_and_execute(|| { // Given Balances::make_free_balance_be(&11, ExistentialDeposit::get() + 2); @@ -4262,15 +4262,12 @@ fn set_claimable_action_works() { ); // Give pool operator permission. - assert_eq!(ClaimableAction::::get(11), false); - assert_noop!(Pools::set_claimable_action(RuntimeOrigin::signed(12), true), Error::::PoolMemberNotFound); - assert_ok!(Pools::set_claimable_action(RuntimeOrigin::signed(11), true)); + assert_eq!(ClaimableAction::::get(11), BondExtraSource::Origin); + assert_noop!(Pools::set_claimable_actor(RuntimeOrigin::signed(12), BondExtraSource::Operator), Error::::PoolMemberNotFound); + assert_ok!(Pools::set_claimable_actor(RuntimeOrigin::signed(11), BondExtraSource::Operator)); // then - assert_eq!(ClaimableAction::::get(11), true); - - ClaimableAction::::remove(11); - assert_eq!(ClaimableAction::::get(11), false); + assert_eq!(ClaimableAction::::get(11), BondExtraSource::Operator); }); } @@ -4636,7 +4633,7 @@ mod bond_extra { assert_eq!(Balances::free_balance(20), 20); assert_noop!(Pools::root_bond_extra(RuntimeOrigin::signed(900), 10), Error::::DoesNotHavePermission); - assert_ok!(Pools::set_claimable_action(RuntimeOrigin::signed(10), true)); + assert_ok!(Pools::set_claimable_actor(RuntimeOrigin::signed(10), BondExtraSource::Operator)); assert_ok!(Pools::root_bond_extra(RuntimeOrigin::signed(900), 10)); assert_eq!(Balances::free_balance(&default_reward_account()), 7); @@ -4647,7 +4644,7 @@ mod bond_extra { // when assert_noop!(Pools::root_bond_extra(RuntimeOrigin::signed(900), 20), Error::::DoesNotHavePermission); - assert_ok!(Pools::set_claimable_action(RuntimeOrigin::signed(20), true)); + assert_ok!(Pools::set_claimable_actor(RuntimeOrigin::signed(20), BondExtraSource::Operator)); assert_ok!(Pools::root_bond_extra(RuntimeOrigin::signed(900), 20)); // then From f7b0a2d93d66af51132fbe32d2a34ffe0dfa7184 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Sun, 20 Nov 2022 11:36:17 +0100 Subject: [PATCH 05/50] update --- frame/nomination-pools/src/lib.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 99d765ba586c7..05d90bd898daa 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -1620,12 +1620,11 @@ pub mod pallet { Ok(()) } - // TODO: Update weight info + // TODO: Update Weight info #[pallet::weight( T::WeightInfo::bond_extra_transfer() .max(T::WeightInfo::bond_extra_reward()) )] - #[transactional] pub fn root_bond_extra( origin: OriginFor, member_account: AccountIdLookupOf, @@ -1659,7 +1658,7 @@ pub mod pallet { BondType::Later )?; - bonded_pool.ok_to_be_open(pending_rewards)?; + bonded_pool.ok_to_be_open()?; member.points = member.points.saturating_add(points_issued); Self::deposit_event(Event::::Bonded { @@ -2174,7 +2173,7 @@ pub mod pallet { T::Staking::chill(&bonded_pool.bonded_account()) } - // Update weight info + // TODO: Update weight info #[pallet::weight(T::DbWeight::get().reads_writes(1, 1))] pub fn set_claimable_actor(origin: OriginFor, actor: BondExtraSource) -> DispatchResult { let who = ensure_signed(origin)?; From 893143311500c3c127224b66da1218d3f0093544 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Sat, 3 Dec 2022 20:58:01 +0100 Subject: [PATCH 06/50] bond extra other --- .../nomination-pools/benchmarking/src/lib.rs | 2 +- frame/nomination-pools/src/lib.rs | 15 ++++++-------- frame/nomination-pools/src/tests.rs | 20 +++++++++---------- 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index 4ab2ab2d0c867..be0ba10c2d011 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -268,7 +268,7 @@ frame_benchmarking::benchmarks! { ); } - root_bond_extra { + bond_extra_other { let origin_weight = Pools::::depositor_min_bond() * 2u32.into(); let scenario = ListScenario::::new(origin_weight, true)?; let scenario_creator1_lookup = T::Lookup::unlookup(scenario.creator1.clone()); diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 05d90bd898daa..53ea206bfc1e8 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -416,8 +416,8 @@ enum AccountType { pub enum BondExtraSource { // Action to be taken by the bonded member of a pool Origin, - // Action to be taken by the pool operator on the member's behalf - Operator, + // Action to be taken by anyone on the member's behalf + Open, } impl Default for BondExtraSource { @@ -1490,8 +1490,6 @@ pub mod pallet { PoolIdInUse, /// Pool id provided is not correct/usable. InvalidPoolId, - /// The caller is not the operator for this pool. - NotRoot, } #[derive(Encode, Decode, PartialEq, TypeInfo, frame_support::PalletError, RuntimeDebug)] @@ -1625,19 +1623,18 @@ pub mod pallet { T::WeightInfo::bond_extra_transfer() .max(T::WeightInfo::bond_extra_reward()) )] - pub fn root_bond_extra( + pub fn bond_extra_other( origin: OriginFor, member_account: AccountIdLookupOf, ) -> DispatchResult { - let who = ensure_signed(origin)?; + ensure_signed(origin)?; let member_account = T::Lookup::lookup(member_account)?; let (mut member, mut bonded_pool, mut reward_pool) = Self::get_member_with_pools(&member_account)?; - ensure!(bonded_pool.is_root(&who), Error::::NotRoot); - ensure!(ClaimableAction::::get(&member_account) == BondExtraSource::Operator, Error::::DoesNotHavePermission); + ensure!(ClaimableAction::::get(&member_account) == BondExtraSource::Open, Error::::DoesNotHavePermission); - // IMPORTANT: reward pool records must be updated with the old points. why? + // IMPORTANT: reward pool records must be updated with the old points. reward_pool.update_records(bonded_pool.id, bonded_pool.points)?; // A member who has no skin in the game anymore cannot claim any rewards. diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 08b809ebc483d..b4495f527587d 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -4262,11 +4262,11 @@ fn set_claimable_actor_works() { // Give pool operator permission. assert_eq!(ClaimableAction::::get(11), BondExtraSource::Origin); - assert_noop!(Pools::set_claimable_actor(RuntimeOrigin::signed(12), BondExtraSource::Operator), Error::::PoolMemberNotFound); - assert_ok!(Pools::set_claimable_actor(RuntimeOrigin::signed(11), BondExtraSource::Operator)); + assert_noop!(Pools::set_claimable_actor(RuntimeOrigin::signed(12), BondExtraSource::Open), Error::::PoolMemberNotFound); + assert_ok!(Pools::set_claimable_actor(RuntimeOrigin::signed(11), BondExtraSource::Open)); // then - assert_eq!(ClaimableAction::::get(11), BondExtraSource::Operator); + assert_eq!(ClaimableAction::::get(11), BondExtraSource::Open); }); } @@ -4616,7 +4616,7 @@ mod bond_extra { } #[test] - fn root_bond_extra_from_rewards() { + fn other_bond_extra_from_rewards() { ExtBuilder::default().add_members(vec![(20, 20)]).build_and_execute(|| { Balances::make_free_balance_be(&default_reward_account(), 8); // ... if which only 3 is claimable to make sure the reward account does not die. @@ -4631,9 +4631,9 @@ mod bond_extra { assert_eq!(Balances::free_balance(10), 35); assert_eq!(Balances::free_balance(20), 20); - assert_noop!(Pools::root_bond_extra(RuntimeOrigin::signed(900), 10), Error::::DoesNotHavePermission); - assert_ok!(Pools::set_claimable_actor(RuntimeOrigin::signed(10), BondExtraSource::Operator)); - assert_ok!(Pools::root_bond_extra(RuntimeOrigin::signed(900), 10)); + assert_noop!(Pools::bond_extra_other(RuntimeOrigin::signed(80), 10), Error::::DoesNotHavePermission); + assert_ok!(Pools::set_claimable_actor(RuntimeOrigin::signed(10), BondExtraSource::Open)); + assert_ok!(Pools::bond_extra_other(RuntimeOrigin::signed(50), 10)); assert_eq!(Balances::free_balance(&default_reward_account()), 7); // then @@ -4642,9 +4642,9 @@ mod bond_extra { assert_eq!(BondedPools::::get(1).unwrap().points, 30 + 1); // when - assert_noop!(Pools::root_bond_extra(RuntimeOrigin::signed(900), 20), Error::::DoesNotHavePermission); - assert_ok!(Pools::set_claimable_actor(RuntimeOrigin::signed(20), BondExtraSource::Operator)); - assert_ok!(Pools::root_bond_extra(RuntimeOrigin::signed(900), 20)); + assert_noop!(Pools::bond_extra_other(RuntimeOrigin::signed(40), 20), Error::::DoesNotHavePermission); + assert_ok!(Pools::set_claimable_actor(RuntimeOrigin::signed(20), BondExtraSource::Open)); + assert_ok!(Pools::bond_extra_other(RuntimeOrigin::signed(20), 20)); // then assert_eq!(Balances::free_balance(20), 20); From 9f83103078c3909829b1f3e7f4017c5311ac9aef Mon Sep 17 00:00:00 2001 From: doordashcon Date: Sun, 4 Dec 2022 20:47:41 +0100 Subject: [PATCH 07/50] update --- .../nomination-pools/benchmarking/src/lib.rs | 14 ++--- frame/nomination-pools/src/lib.rs | 59 +++++++++++-------- frame/nomination-pools/src/tests.rs | 47 +++++++++++---- 3 files changed, 77 insertions(+), 43 deletions(-) diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index be0ba10c2d011..100c68089bba9 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -28,9 +28,9 @@ use frame_election_provider_support::SortedListProvider; use frame_support::{assert_ok, ensure, traits::Get}; use frame_system::RawOrigin as RuntimeOrigin; use pallet_nomination_pools::{ - BalanceOf, BondExtra, BondedPoolInner, BondedPools, ConfigOp, MaxPoolMembers, + BalanceOf, BondExtra, BondedPoolInner, BondedPools, ClaimableAction, ConfigOp, MaxPoolMembers, MaxPoolMembersPerPool, MaxPools, Metadata, MinCreateBond, MinJoinBond, Pallet as Pools, - PoolMembers, PoolRoles, PoolState, RewardPools, SubPoolsStorage, ClaimableAction, BondExtraSource, + PoolMembers, PoolRoles, PoolState, RewardClaim, RewardPools, SubPoolsStorage, }; use sp_runtime::traits::{Bounded, StaticLookup, Zero}; use sp_staking::{EraIndex, StakingInterface}; @@ -268,7 +268,7 @@ frame_benchmarking::benchmarks! { ); } - bond_extra_other { + bond_extra_pending_rewards_other { let origin_weight = Pools::::depositor_min_bond() * 2u32.into(); let scenario = ListScenario::::new(origin_weight, true)?; let scenario_creator1_lookup = T::Lookup::unlookup(scenario.creator1.clone()); @@ -278,7 +278,7 @@ frame_benchmarking::benchmarks! { let reward_account1 = Pools::::create_reward_account(1); assert!(extra >= CurrencyOf::::minimum_balance()); CurrencyOf::::deposit_creating(&reward_account1, extra); - Pools::::set_claimable_actor(RuntimeOrigin::Signed(scenario.creator1.clone()).into(), BondExtraSource::Operator) + Pools::::set_reward_claim(RuntimeOrigin::Signed(scenario.creator1.clone()).into(), RewardClaim::Permissionless) .unwrap(); }:_(RuntimeOrigin::Signed(scenario.creator1.clone()), scenario_creator1_lookup) @@ -673,7 +673,7 @@ frame_benchmarking::benchmarks! { assert!(T::Staking::nominations(Pools::::create_bonded_account(1)).is_none()); } - set_claimable_actor { + set_reward_claim { // Create a pool let min_create_bond = Pools::::depositor_min_bond(); let (depositor, pool_account) = create_pool_account::(0, min_create_bond); @@ -690,9 +690,9 @@ frame_benchmarking::benchmarks! { T::Staking::active_stake(&pool_account).unwrap(), min_create_bond + min_join_bond ); - }:_(RuntimeOrigin::Signed(joiner.clone()), BondExtraSource::Operator) + }:_(RuntimeOrigin::Signed(joiner.clone()), RewardClaim::Permissionless) verify { - assert_eq!(ClaimableAction::::get(joiner), BondExtraSource::Operator); + assert_eq!(ClaimableAction::::get(joiner), RewardClaim::Permissionless); } impl_benchmark_test_suite!( diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 53ea206bfc1e8..8ad165d9cfac1 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -411,16 +411,16 @@ enum AccountType { Reward, } -// Account to increse the bond of a member +// Account to bond pending reward of a member #[derive(Encode, Decode, MaxEncodedLen, Clone, Copy, Debug, PartialEq, Eq, TypeInfo)] -pub enum BondExtraSource { - // Action to be taken by the bonded member of a pool +pub enum RewardClaim { + /// Only `origin` i.e. the pool member themself can claim their reward. Origin, - // Action to be taken by anyone on the member's behalf - Open, + /// Anyone can claim the reward of this member. + Permissionless, } -impl Default for BondExtraSource { +impl Default for RewardClaim { fn default() -> Self { Self::Origin } @@ -1331,7 +1331,8 @@ pub mod pallet { /// ? #[pallet::storage] - pub type ClaimableAction = StorageMap<_, Twox64Concat, T::AccountId, BondExtraSource, ValueQuery>; + pub type ClaimableAction = + StorageMap<_, Twox64Concat, T::AccountId, RewardClaim, ValueQuery>; #[pallet::genesis_config] pub struct GenesisConfig { @@ -1618,13 +1619,15 @@ pub mod pallet { Ok(()) } - // TODO: Update Weight info + /// Bond pending rewards of `member_account` from any AccountId + /// into the pool they already belong. + /// member_account must `set_reward_claim` to `RewardClaim::Permissionless`. #[pallet::weight( T::WeightInfo::bond_extra_transfer() .max(T::WeightInfo::bond_extra_reward()) )] - pub fn bond_extra_other( - origin: OriginFor, + pub fn bond_extra_pending_rewards_other( + origin: OriginFor, member_account: AccountIdLookupOf, ) -> DispatchResult { ensure_signed(origin)?; @@ -1632,17 +1635,17 @@ pub mod pallet { let (mut member, mut bonded_pool, mut reward_pool) = Self::get_member_with_pools(&member_account)?; - ensure!(ClaimableAction::::get(&member_account) == BondExtraSource::Open, Error::::DoesNotHavePermission); + ensure!( + ClaimableAction::::get(&member_account) == RewardClaim::Permissionless, + Error::::DoesNotHavePermission + ); - // IMPORTANT: reward pool records must be updated with the old points. reward_pool.update_records(bonded_pool.id, bonded_pool.points)?; + ensure!(!member.active_points().is_zero(), Error::::FullyUnbonding); - // A member who has no skin in the game anymore cannot claim any rewards. - ensure!(!member.active_points().is_zero(), Error::::FullyUnbonding); - - let current_reward_counter = + let current_reward_counter = reward_pool.current_reward_counter(bonded_pool.id, bonded_pool.points)?; - let pending_rewards = member.pending_rewards(current_reward_counter)?; + let pending_rewards = member.pending_rewards(current_reward_counter)?; if !pending_rewards.is_zero() { member.last_recorded_reward_counter = current_reward_counter; @@ -1650,12 +1653,12 @@ pub mod pallet { } let points_issued = bonded_pool.try_bond_funds( - &bonded_pool.reward_account(), - pending_rewards, - BondType::Later + &bonded_pool.reward_account(), + pending_rewards, + BondType::Later, )?; - bonded_pool.ok_to_be_open()?; + bonded_pool.ok_to_be_open()?; member.points = member.points.saturating_add(points_issued); Self::deposit_event(Event::::Bonded { @@ -1780,6 +1783,7 @@ pub mod pallet { // Now that we know everything has worked write the items to storage. SubPoolsStorage::insert(&member.pool_id, sub_pools); Self::put_member_with_pools(&member_account, member, bonded_pool, reward_pool); + >::remove(member_account); Ok(()) } @@ -2170,14 +2174,19 @@ pub mod pallet { T::Staking::chill(&bonded_pool.bonded_account()) } - // TODO: Update weight info + /// Pool Member set reward claim permission + /// + /// # Account + /// + /// `Origin` + /// `Permisionless` #[pallet::weight(T::DbWeight::get().reads_writes(1, 1))] - pub fn set_claimable_actor(origin: OriginFor, actor: BondExtraSource) -> DispatchResult { + pub fn set_reward_claim(origin: OriginFor, actor: RewardClaim) -> DispatchResult { let who = ensure_signed(origin)?; ensure!(PoolMembers::::contains_key(&who), Error::::PoolMemberNotFound); - >::mutate(who, |source| { - *source = actor; + >::mutate(who, |source| { + *source = actor; }); Ok(()) } diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index b4495f527587d..fc8dbccba8aa2 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -2090,10 +2090,20 @@ mod unbond { Error::::MinimumBondNotMet ); + // Give pool operator permission. + assert_eq!(ClaimableAction::::get(10), RewardClaim::Origin); + // assert_noop!(Pools::set_claimable_actor(RuntimeOrigin::signed(12), + // BondExtraSource::Open), Error::::PoolMemberNotFound); + assert_ok!(Pools::set_reward_claim( + RuntimeOrigin::signed(20), + RewardClaim::Permissionless + )); + // but can go to 0 assert_ok!(Pools::unbond(RuntimeOrigin::signed(20), 20, 15)); assert_eq!(PoolMembers::::get(20).unwrap().active_points(), 0); assert_eq!(PoolMembers::::get(20).unwrap().unbonding_points(), 20); + assert_eq!(ClaimableAction::::get(20), RewardClaim::Origin); }) } @@ -4261,12 +4271,15 @@ fn set_claimable_actor_works() { ); // Give pool operator permission. - assert_eq!(ClaimableAction::::get(11), BondExtraSource::Origin); - assert_noop!(Pools::set_claimable_actor(RuntimeOrigin::signed(12), BondExtraSource::Open), Error::::PoolMemberNotFound); - assert_ok!(Pools::set_claimable_actor(RuntimeOrigin::signed(11), BondExtraSource::Open)); + assert_eq!(ClaimableAction::::get(11), RewardClaim::Origin); + assert_noop!( + Pools::set_reward_claim(RuntimeOrigin::signed(12), RewardClaim::Permissionless), + Error::::PoolMemberNotFound + ); + assert_ok!(Pools::set_reward_claim(RuntimeOrigin::signed(11), RewardClaim::Permissionless)); // then - assert_eq!(ClaimableAction::::get(11), BondExtraSource::Open); + assert_eq!(ClaimableAction::::get(11), RewardClaim::Permissionless); }); } @@ -4616,7 +4629,7 @@ mod bond_extra { } #[test] - fn other_bond_extra_from_rewards() { + fn bond_extra_pending_rewards_other_works() { ExtBuilder::default().add_members(vec![(20, 20)]).build_and_execute(|| { Balances::make_free_balance_be(&default_reward_account(), 8); // ... if which only 3 is claimable to make sure the reward account does not die. @@ -4631,9 +4644,15 @@ mod bond_extra { assert_eq!(Balances::free_balance(10), 35); assert_eq!(Balances::free_balance(20), 20); - assert_noop!(Pools::bond_extra_other(RuntimeOrigin::signed(80), 10), Error::::DoesNotHavePermission); - assert_ok!(Pools::set_claimable_actor(RuntimeOrigin::signed(10), BondExtraSource::Open)); - assert_ok!(Pools::bond_extra_other(RuntimeOrigin::signed(50), 10)); + assert_noop!( + Pools::bond_extra_pending_rewards_other(RuntimeOrigin::signed(80), 10), + Error::::DoesNotHavePermission + ); + assert_ok!(Pools::set_reward_claim( + RuntimeOrigin::signed(10), + RewardClaim::Permissionless + )); + assert_ok!(Pools::bond_extra_pending_rewards_other(RuntimeOrigin::signed(50), 10)); assert_eq!(Balances::free_balance(&default_reward_account()), 7); // then @@ -4642,9 +4661,15 @@ mod bond_extra { assert_eq!(BondedPools::::get(1).unwrap().points, 30 + 1); // when - assert_noop!(Pools::bond_extra_other(RuntimeOrigin::signed(40), 20), Error::::DoesNotHavePermission); - assert_ok!(Pools::set_claimable_actor(RuntimeOrigin::signed(20), BondExtraSource::Open)); - assert_ok!(Pools::bond_extra_other(RuntimeOrigin::signed(20), 20)); + assert_noop!( + Pools::bond_extra_pending_rewards_other(RuntimeOrigin::signed(40), 20), + Error::::DoesNotHavePermission + ); + assert_ok!(Pools::set_reward_claim( + RuntimeOrigin::signed(20), + RewardClaim::Permissionless + )); + assert_ok!(Pools::bond_extra_pending_rewards_other(RuntimeOrigin::signed(20), 20)); // then assert_eq!(Balances::free_balance(20), 20); From 9d63fdba9fb0a3b356a9ee06ff89d4515edb0ee5 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Sun, 4 Dec 2022 20:57:09 +0100 Subject: [PATCH 08/50] update --- frame/nomination-pools/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 8ad165d9cfac1..703c9e71ea9b9 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -1329,7 +1329,7 @@ pub mod pallet { pub type ReversePoolIdLookup = CountedStorageMap<_, Twox64Concat, T::AccountId, PoolId, OptionQuery>; - /// ? + /// Reward claim Permissions #[pallet::storage] pub type ClaimableAction = StorageMap<_, Twox64Concat, T::AccountId, RewardClaim, ValueQuery>; @@ -2179,7 +2179,7 @@ pub mod pallet { /// # Account /// /// `Origin` - /// `Permisionless` + /// `Permissionless` #[pallet::weight(T::DbWeight::get().reads_writes(1, 1))] pub fn set_reward_claim(origin: OriginFor, actor: RewardClaim) -> DispatchResult { let who = ensure_signed(origin)?; From a3718f64671bd27cba71d52e4086277d850855c3 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Mon, 5 Dec 2022 08:54:01 +0100 Subject: [PATCH 09/50] update --- frame/nomination-pools/benchmarking/src/lib.rs | 4 ++-- frame/nomination-pools/src/lib.rs | 10 +++++----- frame/nomination-pools/src/tests.rs | 8 ++++---- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index 100c68089bba9..541893009a0ee 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -28,7 +28,7 @@ use frame_election_provider_support::SortedListProvider; use frame_support::{assert_ok, ensure, traits::Get}; use frame_system::RawOrigin as RuntimeOrigin; use pallet_nomination_pools::{ - BalanceOf, BondExtra, BondedPoolInner, BondedPools, ClaimableAction, ConfigOp, MaxPoolMembers, + BalanceOf, BondExtra, BondedPoolInner, BondedPools, RewardClaimPermission, ConfigOp, MaxPoolMembers, MaxPoolMembersPerPool, MaxPools, Metadata, MinCreateBond, MinJoinBond, Pallet as Pools, PoolMembers, PoolRoles, PoolState, RewardClaim, RewardPools, SubPoolsStorage, }; @@ -692,7 +692,7 @@ frame_benchmarking::benchmarks! { ); }:_(RuntimeOrigin::Signed(joiner.clone()), RewardClaim::Permissionless) verify { - assert_eq!(ClaimableAction::::get(joiner), RewardClaim::Permissionless); + assert_eq!(RewardClaimPermission::::get(joiner), RewardClaim::Permissionless); } impl_benchmark_test_suite!( diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 703c9e71ea9b9..686fca2abb477 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -411,7 +411,7 @@ enum AccountType { Reward, } -// Account to bond pending reward of a member +/// Account to bond pending reward of a member #[derive(Encode, Decode, MaxEncodedLen, Clone, Copy, Debug, PartialEq, Eq, TypeInfo)] pub enum RewardClaim { /// Only `origin` i.e. the pool member themself can claim their reward. @@ -1331,7 +1331,7 @@ pub mod pallet { /// Reward claim Permissions #[pallet::storage] - pub type ClaimableAction = + pub type RewardClaimPermission = StorageMap<_, Twox64Concat, T::AccountId, RewardClaim, ValueQuery>; #[pallet::genesis_config] @@ -1636,7 +1636,7 @@ pub mod pallet { Self::get_member_with_pools(&member_account)?; ensure!( - ClaimableAction::::get(&member_account) == RewardClaim::Permissionless, + RewardClaimPermission::::get(&member_account) == RewardClaim::Permissionless, Error::::DoesNotHavePermission ); @@ -1783,7 +1783,7 @@ pub mod pallet { // Now that we know everything has worked write the items to storage. SubPoolsStorage::insert(&member.pool_id, sub_pools); Self::put_member_with_pools(&member_account, member, bonded_pool, reward_pool); - >::remove(member_account); + >::remove(member_account); Ok(()) } @@ -2185,7 +2185,7 @@ pub mod pallet { let who = ensure_signed(origin)?; ensure!(PoolMembers::::contains_key(&who), Error::::PoolMemberNotFound); - >::mutate(who, |source| { + >::mutate(who, |source| { *source = actor; }); Ok(()) diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index fc8dbccba8aa2..2c19459b07674 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -2091,7 +2091,7 @@ mod unbond { ); // Give pool operator permission. - assert_eq!(ClaimableAction::::get(10), RewardClaim::Origin); + assert_eq!(RewardClaimPermission::::get(10), RewardClaim::Origin); // assert_noop!(Pools::set_claimable_actor(RuntimeOrigin::signed(12), // BondExtraSource::Open), Error::::PoolMemberNotFound); assert_ok!(Pools::set_reward_claim( @@ -2103,7 +2103,7 @@ mod unbond { assert_ok!(Pools::unbond(RuntimeOrigin::signed(20), 20, 15)); assert_eq!(PoolMembers::::get(20).unwrap().active_points(), 0); assert_eq!(PoolMembers::::get(20).unwrap().unbonding_points(), 20); - assert_eq!(ClaimableAction::::get(20), RewardClaim::Origin); + assert_eq!(RewardClaimPermission::::get(20), RewardClaim::Origin); }) } @@ -4271,7 +4271,7 @@ fn set_claimable_actor_works() { ); // Give pool operator permission. - assert_eq!(ClaimableAction::::get(11), RewardClaim::Origin); + assert_eq!(RewardClaimPermission::::get(11), RewardClaim::Origin); assert_noop!( Pools::set_reward_claim(RuntimeOrigin::signed(12), RewardClaim::Permissionless), Error::::PoolMemberNotFound @@ -4279,7 +4279,7 @@ fn set_claimable_actor_works() { assert_ok!(Pools::set_reward_claim(RuntimeOrigin::signed(11), RewardClaim::Permissionless)); // then - assert_eq!(ClaimableAction::::get(11), RewardClaim::Permissionless); + assert_eq!(RewardClaimPermission::::get(11), RewardClaim::Permissionless); }); } From 4c6a4e7acb1030ecc7b42d72e7337e2ce19c8410 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Mon, 5 Dec 2022 08:54:55 +0100 Subject: [PATCH 10/50] cargo fmt --- frame/nomination-pools/benchmarking/src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index 541893009a0ee..f7d82cf23c874 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -28,9 +28,10 @@ use frame_election_provider_support::SortedListProvider; use frame_support::{assert_ok, ensure, traits::Get}; use frame_system::RawOrigin as RuntimeOrigin; use pallet_nomination_pools::{ - BalanceOf, BondExtra, BondedPoolInner, BondedPools, RewardClaimPermission, ConfigOp, MaxPoolMembers, + BalanceOf, BondExtra, BondedPoolInner, BondedPools, ConfigOp, MaxPoolMembers, MaxPoolMembersPerPool, MaxPools, Metadata, MinCreateBond, MinJoinBond, Pallet as Pools, - PoolMembers, PoolRoles, PoolState, RewardClaim, RewardPools, SubPoolsStorage, + PoolMembers, PoolRoles, PoolState, RewardClaim, RewardClaimPermission, RewardPools, + SubPoolsStorage, }; use sp_runtime::traits::{Bounded, StaticLookup, Zero}; use sp_staking::{EraIndex, StakingInterface}; From 8207d51e9f76703bad172d29f4f846b868a3c7a1 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Wed, 7 Dec 2022 21:37:08 +0100 Subject: [PATCH 11/50] Permissioned --- frame/nomination-pools/src/lib.rs | 6 +++--- frame/nomination-pools/src/tests.rs | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 686fca2abb477..b9d2b8d5e5c69 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -414,15 +414,15 @@ enum AccountType { /// Account to bond pending reward of a member #[derive(Encode, Decode, MaxEncodedLen, Clone, Copy, Debug, PartialEq, Eq, TypeInfo)] pub enum RewardClaim { - /// Only `origin` i.e. the pool member themself can claim their reward. - Origin, + /// Only the pool member themself can claim their reward. + Permissioned, /// Anyone can claim the reward of this member. Permissionless, } impl Default for RewardClaim { fn default() -> Self { - Self::Origin + Self::Permissioned } } diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 2c19459b07674..73f3f85fe01f8 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -2091,7 +2091,7 @@ mod unbond { ); // Give pool operator permission. - assert_eq!(RewardClaimPermission::::get(10), RewardClaim::Origin); + assert_eq!(RewardClaimPermission::::get(10), RewardClaim::Permissioned); // assert_noop!(Pools::set_claimable_actor(RuntimeOrigin::signed(12), // BondExtraSource::Open), Error::::PoolMemberNotFound); assert_ok!(Pools::set_reward_claim( @@ -2103,7 +2103,7 @@ mod unbond { assert_ok!(Pools::unbond(RuntimeOrigin::signed(20), 20, 15)); assert_eq!(PoolMembers::::get(20).unwrap().active_points(), 0); assert_eq!(PoolMembers::::get(20).unwrap().unbonding_points(), 20); - assert_eq!(RewardClaimPermission::::get(20), RewardClaim::Origin); + assert_eq!(RewardClaimPermission::::get(20), RewardClaim::Permissioned); }) } @@ -4271,7 +4271,7 @@ fn set_claimable_actor_works() { ); // Give pool operator permission. - assert_eq!(RewardClaimPermission::::get(11), RewardClaim::Origin); + assert_eq!(RewardClaimPermission::::get(11), RewardClaim::Permissioned); assert_noop!( Pools::set_reward_claim(RuntimeOrigin::signed(12), RewardClaim::Permissionless), Error::::PoolMemberNotFound From a4b6ec23400c611c22ff0d77e7a959d7a93ee0a3 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Thu, 15 Dec 2022 11:21:27 +0100 Subject: [PATCH 12/50] update --- .../nomination-pools/benchmarking/src/lib.rs | 13 ++++++------- frame/nomination-pools/src/lib.rs | 19 ++++++++++--------- frame/nomination-pools/src/tests.rs | 7 +++---- 3 files changed, 19 insertions(+), 20 deletions(-) diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index f7d82cf23c874..1cbe99774fab5 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -272,17 +272,16 @@ frame_benchmarking::benchmarks! { bond_extra_pending_rewards_other { let origin_weight = Pools::::depositor_min_bond() * 2u32.into(); let scenario = ListScenario::::new(origin_weight, true)?; - let scenario_creator1_lookup = T::Lookup::unlookup(scenario.creator1.clone()); + let scenario_creator_lookup = T::Lookup::unlookup(scenario.creator1.clone()); let extra = (scenario.dest_weight - origin_weight).max(CurrencyOf::::minimum_balance()); - // transfer exactly `extra` to the depositor of the src pool (1), from the reward balance - let reward_account1 = Pools::::create_reward_account(1); + let reward_account = Pools::::create_reward_account(1); assert!(extra >= CurrencyOf::::minimum_balance()); - CurrencyOf::::deposit_creating(&reward_account1, extra); + CurrencyOf::::deposit_creating(&reward_account, extra); Pools::::set_reward_claim(RuntimeOrigin::Signed(scenario.creator1.clone()).into(), RewardClaim::Permissionless) .unwrap(); - }:_(RuntimeOrigin::Signed(scenario.creator1.clone()), scenario_creator1_lookup) + }:_(RuntimeOrigin::Signed(scenario.creator1.clone()), scenario_creator_lookup) verify { assert!( T::Staking::active_stake(&scenario.origin1).unwrap() >= @@ -679,9 +678,9 @@ frame_benchmarking::benchmarks! { let min_create_bond = Pools::::depositor_min_bond(); let (depositor, pool_account) = create_pool_account::(0, min_create_bond); - // Add a new member + // Join pool let min_join_bond = MinJoinBond::::get().max(CurrencyOf::::minimum_balance()); - let joiner = create_funded_user_with_balance::("joiner", 0, min_join_bond * 2u32.into()); + let joiner = create_funded_user_with_balance::("joiner", 0, min_join_bond * 4u32.into()); let joiner_lookup = T::Lookup::unlookup(joiner.clone()); Pools::::join(RuntimeOrigin::Signed(joiner.clone()).into(), min_join_bond, 1) .unwrap(); diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index b9d2b8d5e5c69..fad3fbfe4a872 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -411,7 +411,7 @@ enum AccountType { Reward, } -/// Account to bond pending reward of a member +/// Account to bond pending reward of a member. #[derive(Encode, Decode, MaxEncodedLen, Clone, Copy, Debug, PartialEq, Eq, TypeInfo)] pub enum RewardClaim { /// Only the pool member themself can claim their reward. @@ -1329,7 +1329,7 @@ pub mod pallet { pub type ReversePoolIdLookup = CountedStorageMap<_, Twox64Concat, T::AccountId, PoolId, OptionQuery>; - /// Reward claim Permissions + /// Map from a pool member account to their preference regarding reward payout #[pallet::storage] pub type RewardClaimPermission = StorageMap<_, Twox64Concat, T::AccountId, RewardClaim, ValueQuery>; @@ -1619,9 +1619,10 @@ pub mod pallet { Ok(()) } - /// Bond pending rewards of `member_account` from any AccountId - /// into the pool they already belong. - /// member_account must `set_reward_claim` to `RewardClaim::Permissionless`. + /// Bond pending rewards of `member_account` into the pool they already belong. + /// + /// Note: `member_account` must pass `RewardClaim::Permissionless` to `set_reward_claim`, + /// making this call permissionless. #[pallet::weight( T::WeightInfo::bond_extra_transfer() .max(T::WeightInfo::bond_extra_reward()) @@ -2174,12 +2175,12 @@ pub mod pallet { T::Staking::chill(&bonded_pool.bonded_account()) } - /// Pool Member set reward claim permission + /// Set reward claim permission. /// - /// # Account + /// # Arguments /// - /// `Origin` - /// `Permissionless` + /// * `Origin` - Member of a pool. + /// * `actor` - Account to claim reward. #[pallet::weight(T::DbWeight::get().reads_writes(1, 1))] pub fn set_reward_claim(origin: OriginFor, actor: RewardClaim) -> DispatchResult { let who = ensure_signed(origin)?; diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 73f3f85fe01f8..9ca53808dd3e1 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -2090,10 +2090,8 @@ mod unbond { Error::::MinimumBondNotMet ); - // Give pool operator permission. + // Make permissionless assert_eq!(RewardClaimPermission::::get(10), RewardClaim::Permissioned); - // assert_noop!(Pools::set_claimable_actor(RuntimeOrigin::signed(12), - // BondExtraSource::Open), Error::::PoolMemberNotFound); assert_ok!(Pools::set_reward_claim( RuntimeOrigin::signed(20), RewardClaim::Permissionless @@ -4270,7 +4268,7 @@ fn set_claimable_actor_works() { ] ); - // Give pool operator permission. + // Make permissionless assert_eq!(RewardClaimPermission::::get(11), RewardClaim::Permissioned); assert_noop!( Pools::set_reward_claim(RuntimeOrigin::signed(12), RewardClaim::Permissionless), @@ -4644,6 +4642,7 @@ mod bond_extra { assert_eq!(Balances::free_balance(10), 35); assert_eq!(Balances::free_balance(20), 20); + // Make permissionless and bond pending rewards of member assert_noop!( Pools::bond_extra_pending_rewards_other(RuntimeOrigin::signed(80), 10), Error::::DoesNotHavePermission From db9c1b808680f5c4c8e80644e9189d592e01b84e Mon Sep 17 00:00:00 2001 From: doordashcon Date: Thu, 15 Dec 2022 11:24:18 +0100 Subject: [PATCH 13/50] cargo fmt --- frame/nomination-pools/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index fad3fbfe4a872..9e1547e398e0b 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -1620,7 +1620,7 @@ pub mod pallet { } /// Bond pending rewards of `member_account` into the pool they already belong. - /// + /// /// Note: `member_account` must pass `RewardClaim::Permissionless` to `set_reward_claim`, /// making this call permissionless. #[pallet::weight( From b5eda08dbcb95b4367b01f6ffb651747349ba667 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Sun, 18 Dec 2022 16:24:33 +0100 Subject: [PATCH 14/50] update --- frame/nomination-pools/src/lib.rs | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 0882534939db0..cebf9972b01a3 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -1625,6 +1625,7 @@ pub mod pallet { /// /// Note: `member_account` must pass `RewardClaim::Permissionless` to `set_reward_claim`, /// making this call permissionless. + #[pallet::call_index(2)] #[pallet::weight( T::WeightInfo::bond_extra_transfer() .max(T::WeightInfo::bond_extra_reward()) @@ -1682,7 +1683,7 @@ 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::call_index(2)] + #[pallet::call_index(3)] #[pallet::weight(T::WeightInfo::claim_payout())] pub fn claim_payout(origin: OriginFor) -> DispatchResult { let who = ensure_signed(origin)?; @@ -1725,7 +1726,7 @@ pub mod pallet { /// are available). However, it may not be possible to release the current unlocking chunks, /// in which case, the result of this call will likely be the `NoMoreChunks` error from the /// staking system. - #[pallet::call_index(3)] + #[pallet::call_index(4)] #[pallet::weight(T::WeightInfo::unbond())] pub fn unbond( origin: OriginFor, @@ -1802,7 +1803,7 @@ pub mod pallet { /// can be cleared by withdrawing. In the case there are too many unlocking chunks, the user /// would probably see an error like `NoMoreChunks` emitted from the staking system when /// they attempt to unbond. - #[pallet::call_index(4)] + #[pallet::call_index(5)] #[pallet::weight(T::WeightInfo::pool_withdraw_unbonded(*num_slashing_spans))] pub fn pool_withdraw_unbonded( origin: OriginFor, @@ -1837,7 +1838,7 @@ pub mod pallet { /// # Note /// /// If the target is the depositor, the pool will be destroyed. - #[pallet::call_index(5)] + #[pallet::call_index(6)] #[pallet::weight( T::WeightInfo::withdraw_unbonded_kill(*num_slashing_spans) )] @@ -1959,7 +1960,7 @@ 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::call_index(6)] + #[pallet::call_index(7)] #[pallet::weight(T::WeightInfo::create())] pub fn create( origin: OriginFor, @@ -1984,7 +1985,7 @@ pub mod pallet { /// /// same as `create` with the inclusion of /// * `pool_id` - `A valid PoolId. - #[pallet::call_index(7)] + #[pallet::call_index(8)] #[pallet::weight(T::WeightInfo::create())] pub fn create_with_pool_id( origin: OriginFor, @@ -2009,7 +2010,7 @@ pub mod pallet { /// /// This directly forward the call to the staking pallet, on behalf of the pool bonded /// account. - #[pallet::call_index(8)] + #[pallet::call_index(9)] #[pallet::weight(T::WeightInfo::nominate(validators.len() as u32))] pub fn nominate( origin: OriginFor, @@ -2032,7 +2033,7 @@ pub mod pallet { /// 1. signed by the state toggler, or the root role of the pool, /// 2. if the pool conditions to be open are NOT met (as described by `ok_to_be_open`), and /// then the state of the pool can be permissionlessly changed to `Destroying`. - #[pallet::call_index(9)] + #[pallet::call_index(10)] #[pallet::weight(T::WeightInfo::set_state())] pub fn set_state( origin: OriginFor, @@ -2061,7 +2062,7 @@ pub mod pallet { /// /// The dispatch origin of this call must be signed by the state toggler, or the root role /// of the pool. - #[pallet::call_index(10)] + #[pallet::call_index(11)] #[pallet::weight(T::WeightInfo::set_metadata(metadata.len() as u32))] pub fn set_metadata( origin: OriginFor, @@ -2093,7 +2094,7 @@ pub mod pallet { /// * `max_pools` - Set [`MaxPools`]. /// * `max_members` - Set [`MaxPoolMembers`]. /// * `max_members_per_pool` - Set [`MaxPoolMembersPerPool`]. - #[pallet::call_index(11)] + #[pallet::call_index(12)] #[pallet::weight(T::WeightInfo::set_configs())] pub fn set_configs( origin: OriginFor, @@ -2130,7 +2131,7 @@ pub mod pallet { /// /// It emits an event, notifying UIs of the role change. This event is quite relevant to /// most pool members and they should be informed of changes to pool roles. - #[pallet::call_index(12)] + #[pallet::call_index(13)] #[pallet::weight(T::WeightInfo::update_roles())] pub fn update_roles( origin: OriginFor, @@ -2183,7 +2184,7 @@ pub mod pallet { /// /// This directly forward the call to the staking pallet, on behalf of the pool bonded /// account. - #[pallet::call_index(13)] + #[pallet::call_index(14)] #[pallet::weight(T::WeightInfo::chill())] pub fn chill(origin: OriginFor, pool_id: PoolId) -> DispatchResult { let who = ensure_signed(origin)?; @@ -2198,6 +2199,7 @@ pub mod pallet { /// /// * `Origin` - Member of a pool. /// * `actor` - Account to claim reward. + #[pallet::call_index(15)] #[pallet::weight(T::DbWeight::get().reads_writes(1, 1))] pub fn set_reward_claim(origin: OriginFor, actor: RewardClaim) -> DispatchResult { let who = ensure_signed(origin)?; From b1105260a42903f79457c6579dca620864c0f429 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Mon, 2 Jan 2023 22:32:28 +0100 Subject: [PATCH 15/50] update index --- frame/nomination-pools/src/lib.rs | 136 +++++++++++++++--------------- 1 file changed, 68 insertions(+), 68 deletions(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index cebf9972b01a3..0544b08eff2b9 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -1621,69 +1621,13 @@ pub mod pallet { Ok(()) } - /// Bond pending rewards of `member_account` into the pool they already belong. - /// - /// Note: `member_account` must pass `RewardClaim::Permissionless` to `set_reward_claim`, - /// making this call permissionless. - #[pallet::call_index(2)] - #[pallet::weight( - T::WeightInfo::bond_extra_transfer() - .max(T::WeightInfo::bond_extra_reward()) - )] - pub fn bond_extra_pending_rewards_other( - origin: OriginFor, - member_account: AccountIdLookupOf, - ) -> DispatchResult { - ensure_signed(origin)?; - let member_account = T::Lookup::lookup(member_account)?; - let (mut member, mut bonded_pool, mut reward_pool) = - Self::get_member_with_pools(&member_account)?; - - ensure!( - RewardClaimPermission::::get(&member_account) == RewardClaim::Permissionless, - Error::::DoesNotHavePermission - ); - - reward_pool.update_records(bonded_pool.id, bonded_pool.points)?; - ensure!(!member.active_points().is_zero(), Error::::FullyUnbonding); - - let current_reward_counter = - reward_pool.current_reward_counter(bonded_pool.id, bonded_pool.points)?; - let pending_rewards = member.pending_rewards(current_reward_counter)?; - - if !pending_rewards.is_zero() { - member.last_recorded_reward_counter = current_reward_counter; - reward_pool.register_claimed_reward(pending_rewards); - } - - let points_issued = bonded_pool.try_bond_funds( - &bonded_pool.reward_account(), - pending_rewards, - BondType::Later, - )?; - - bonded_pool.ok_to_be_open()?; - member.points = member.points.saturating_add(points_issued); - - Self::deposit_event(Event::::Bonded { - member: member_account.clone(), - pool_id: member.pool_id, - bonded: pending_rewards, - joined: false, - }); - - Self::put_member_with_pools(&member_account, member, bonded_pool, reward_pool); - - Ok(()) - } - /// A bonded member can use this to claim their payout based on the rewards that the pool /// has accumulated since their last claimed payout (OR since joining if this is there first /// time claiming rewards). The payout will be transferred to the member's account. /// /// 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::call_index(3)] + #[pallet::call_index(2)] #[pallet::weight(T::WeightInfo::claim_payout())] pub fn claim_payout(origin: OriginFor) -> DispatchResult { let who = ensure_signed(origin)?; @@ -1726,7 +1670,7 @@ pub mod pallet { /// are available). However, it may not be possible to release the current unlocking chunks, /// in which case, the result of this call will likely be the `NoMoreChunks` error from the /// staking system. - #[pallet::call_index(4)] + #[pallet::call_index(3)] #[pallet::weight(T::WeightInfo::unbond())] pub fn unbond( origin: OriginFor, @@ -1803,7 +1747,7 @@ pub mod pallet { /// can be cleared by withdrawing. In the case there are too many unlocking chunks, the user /// would probably see an error like `NoMoreChunks` emitted from the staking system when /// they attempt to unbond. - #[pallet::call_index(5)] + #[pallet::call_index(4)] #[pallet::weight(T::WeightInfo::pool_withdraw_unbonded(*num_slashing_spans))] pub fn pool_withdraw_unbonded( origin: OriginFor, @@ -1838,7 +1782,7 @@ pub mod pallet { /// # Note /// /// If the target is the depositor, the pool will be destroyed. - #[pallet::call_index(6)] + #[pallet::call_index(5)] #[pallet::weight( T::WeightInfo::withdraw_unbonded_kill(*num_slashing_spans) )] @@ -1960,7 +1904,7 @@ 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::call_index(7)] + #[pallet::call_index(6)] #[pallet::weight(T::WeightInfo::create())] pub fn create( origin: OriginFor, @@ -1985,7 +1929,7 @@ pub mod pallet { /// /// same as `create` with the inclusion of /// * `pool_id` - `A valid PoolId. - #[pallet::call_index(8)] + #[pallet::call_index(7)] #[pallet::weight(T::WeightInfo::create())] pub fn create_with_pool_id( origin: OriginFor, @@ -2010,7 +1954,7 @@ pub mod pallet { /// /// This directly forward the call to the staking pallet, on behalf of the pool bonded /// account. - #[pallet::call_index(9)] + #[pallet::call_index(8)] #[pallet::weight(T::WeightInfo::nominate(validators.len() as u32))] pub fn nominate( origin: OriginFor, @@ -2033,7 +1977,7 @@ pub mod pallet { /// 1. signed by the state toggler, or the root role of the pool, /// 2. if the pool conditions to be open are NOT met (as described by `ok_to_be_open`), and /// then the state of the pool can be permissionlessly changed to `Destroying`. - #[pallet::call_index(10)] + #[pallet::call_index(9)] #[pallet::weight(T::WeightInfo::set_state())] pub fn set_state( origin: OriginFor, @@ -2062,7 +2006,7 @@ pub mod pallet { /// /// The dispatch origin of this call must be signed by the state toggler, or the root role /// of the pool. - #[pallet::call_index(11)] + #[pallet::call_index(10)] #[pallet::weight(T::WeightInfo::set_metadata(metadata.len() as u32))] pub fn set_metadata( origin: OriginFor, @@ -2094,7 +2038,7 @@ pub mod pallet { /// * `max_pools` - Set [`MaxPools`]. /// * `max_members` - Set [`MaxPoolMembers`]. /// * `max_members_per_pool` - Set [`MaxPoolMembersPerPool`]. - #[pallet::call_index(12)] + #[pallet::call_index(11)] #[pallet::weight(T::WeightInfo::set_configs())] pub fn set_configs( origin: OriginFor, @@ -2131,7 +2075,7 @@ pub mod pallet { /// /// It emits an event, notifying UIs of the role change. This event is quite relevant to /// most pool members and they should be informed of changes to pool roles. - #[pallet::call_index(13)] + #[pallet::call_index(12)] #[pallet::weight(T::WeightInfo::update_roles())] pub fn update_roles( origin: OriginFor, @@ -2184,7 +2128,7 @@ pub mod pallet { /// /// This directly forward the call to the staking pallet, on behalf of the pool bonded /// account. - #[pallet::call_index(14)] + #[pallet::call_index(13)] #[pallet::weight(T::WeightInfo::chill())] pub fn chill(origin: OriginFor, pool_id: PoolId) -> DispatchResult { let who = ensure_signed(origin)?; @@ -2193,6 +2137,62 @@ pub mod pallet { T::Staking::chill(&bonded_pool.bonded_account()) } + /// Bond pending rewards of `member_account` into the pool they already belong. + /// + /// Note: `member_account` must pass `RewardClaim::Permissionless` to `set_reward_claim`, + /// making this call permissionless. + #[pallet::call_index(14)] + #[pallet::weight( + T::WeightInfo::bond_extra_transfer() + .max(T::WeightInfo::bond_extra_reward()) + )] + pub fn bond_extra_pending_rewards_other( + origin: OriginFor, + member_account: AccountIdLookupOf, + ) -> DispatchResult { + ensure_signed(origin)?; + let member_account = T::Lookup::lookup(member_account)?; + let (mut member, mut bonded_pool, mut reward_pool) = + Self::get_member_with_pools(&member_account)?; + + ensure!( + RewardClaimPermission::::get(&member_account) == RewardClaim::Permissionless, + Error::::DoesNotHavePermission + ); + + reward_pool.update_records(bonded_pool.id, bonded_pool.points)?; + ensure!(!member.active_points().is_zero(), Error::::FullyUnbonding); + + let current_reward_counter = + reward_pool.current_reward_counter(bonded_pool.id, bonded_pool.points)?; + let pending_rewards = member.pending_rewards(current_reward_counter)?; + + if !pending_rewards.is_zero() { + member.last_recorded_reward_counter = current_reward_counter; + reward_pool.register_claimed_reward(pending_rewards); + } + + let points_issued = bonded_pool.try_bond_funds( + &bonded_pool.reward_account(), + pending_rewards, + BondType::Later, + )?; + + bonded_pool.ok_to_be_open()?; + member.points = member.points.saturating_add(points_issued); + + Self::deposit_event(Event::::Bonded { + member: member_account.clone(), + pool_id: member.pool_id, + bonded: pending_rewards, + joined: false, + }); + + Self::put_member_with_pools(&member_account, member, bonded_pool, reward_pool); + + Ok(()) + } + /// Set reward claim permission. /// /// # Arguments From a45d6154e3e5471f6624cdf8f9b84e5d3f4c6145 Mon Sep 17 00:00:00 2001 From: Doordashcon Date: Thu, 26 Jan 2023 10:00:32 +0100 Subject: [PATCH 16/50] doc update Co-authored-by: Ankan <10196091+Ank4n@users.noreply.github.com> --- frame/nomination-pools/src/lib.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 31562e79af6ad..0f1dc19f6afd4 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -2195,7 +2195,12 @@ pub mod pallet { Ok(()) } - /// Set reward claim permission. + /// Sets permission to claim reward. + /// + /// Lets a pool member to choose who can claim pending rewards on their behalf. By default, + /// this is `Permissioned` which implies only the pool member themselves can claim their pending + /// rewards. If a pool member wishes so, they can set this to `Permissionless` to allow any + /// account to claim their rewards and bond extra to the pool. /// /// # Arguments /// From ab1e7bb40b90800506ae1a05cba98bfedb066fa6 Mon Sep 17 00:00:00 2001 From: Doordashcon Date: Mon, 30 Jan 2023 20:56:38 +0100 Subject: [PATCH 17/50] doc update Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> --- frame/nomination-pools/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 0f1dc19f6afd4..c0f89dd2cdbc8 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -1331,7 +1331,7 @@ pub mod pallet { pub type ReversePoolIdLookup = CountedStorageMap<_, Twox64Concat, T::AccountId, PoolId, OptionQuery>; - /// Map from a pool member account to their preference regarding reward payout + /// Map from a pool member account to their preference regarding reward payout. #[pallet::storage] pub type RewardClaimPermission = StorageMap<_, Twox64Concat, T::AccountId, RewardClaim, ValueQuery>; From 5f253740c61849e1f0eeef53ad607338a1dc8a65 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Wed, 1 Feb 2023 09:59:17 +0100 Subject: [PATCH 18/50] cargo fmt --- frame/nomination-pools/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index c0f89dd2cdbc8..2d89b4b9bc3ba 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -2198,9 +2198,9 @@ pub mod pallet { /// Sets permission to claim reward. /// /// Lets a pool member to choose who can claim pending rewards on their behalf. By default, - /// this is `Permissioned` which implies only the pool member themselves can claim their pending - /// rewards. If a pool member wishes so, they can set this to `Permissionless` to allow any - /// account to claim their rewards and bond extra to the pool. + /// this is `Permissioned` which implies only the pool member themselves can claim their + /// pending rewards. If a pool member wishes so, they can set this to `Permissionless` to + /// allow any account to claim their rewards and bond extra to the pool. /// /// # Arguments /// From 619e9b9e7dbcbc8f5bf1c3b2a3966b9916f572f9 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Thu, 2 Feb 2023 20:07:35 +0100 Subject: [PATCH 19/50] bond_extra auto compound --- .../nomination-pools/benchmarking/src/lib.rs | 24 +---- frame/nomination-pools/fuzzer/src/call.rs | 2 +- frame/nomination-pools/src/lib.rs | 79 ++++------------ frame/nomination-pools/src/tests.rs | 94 ++++++++++++++----- 4 files changed, 94 insertions(+), 105 deletions(-) diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index ff2c25fe2401a..0e921ea2182b5 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -245,7 +245,7 @@ frame_benchmarking::benchmarks! { // creator of the src pool will bond-extra, bumping itself to dest bag. - }: bond_extra(RuntimeOrigin::Signed(scenario.creator1.clone()), BondExtra::FreeBalance(extra)) + }: bond_extra(RuntimeOrigin::Signed(scenario.creator1.clone()), None, BondExtra::FreeBalance(extra)) verify { assert!( T::Staking::active_stake(&scenario.origin1).unwrap() >= @@ -263,27 +263,7 @@ frame_benchmarking::benchmarks! { assert!(extra >= CurrencyOf::::minimum_balance()); CurrencyOf::::deposit_creating(&reward_account1, extra); - }: bond_extra(RuntimeOrigin::Signed(scenario.creator1.clone()), BondExtra::Rewards) - verify { - assert!( - T::Staking::active_stake(&scenario.origin1).unwrap() >= - scenario.dest_weight - ); - } - - bond_extra_pending_rewards_other { - let origin_weight = Pools::::depositor_min_bond() * 2u32.into(); - let scenario = ListScenario::::new(origin_weight, true)?; - let scenario_creator_lookup = T::Lookup::unlookup(scenario.creator1.clone()); - let extra = (scenario.dest_weight - origin_weight).max(CurrencyOf::::minimum_balance()); - - let reward_account = Pools::::create_reward_account(1); - assert!(extra >= CurrencyOf::::minimum_balance()); - CurrencyOf::::deposit_creating(&reward_account, extra); - Pools::::set_reward_claim(RuntimeOrigin::Signed(scenario.creator1.clone()).into(), RewardClaim::Permissionless) - .unwrap(); - - }:_(RuntimeOrigin::Signed(scenario.creator1.clone()), scenario_creator_lookup) + }: bond_extra(RuntimeOrigin::Signed(scenario.creator1.clone()), None, BondExtra::Rewards) verify { assert!( T::Staking::active_stake(&scenario.origin1).unwrap() >= diff --git a/frame/nomination-pools/fuzzer/src/call.rs b/frame/nomination-pools/fuzzer/src/call.rs index b07903609e8ab..9b4e787724ce9 100644 --- a/frame/nomination-pools/fuzzer/src/call.rs +++ b/frame/nomination-pools/fuzzer/src/call.rs @@ -110,7 +110,7 @@ fn random_call(mut rng: &mut R) -> (pools::Call, RuntimeOrigin) { let amount = random_ed_multiple(&mut rng); BondExtra::FreeBalance(amount) }; - (PoolsCall::::bond_extra { extra }, origin) + (PoolsCall::::bond_extra { other: Some(who), extra }, origin) }, 2 => { // claim_payout diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 2d89b4b9bc3ba..7e26783a40786 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -1493,6 +1493,8 @@ pub mod pallet { PoolIdInUse, /// Pool id provided is not correct/usable. InvalidPoolId, + /// Restricted to pending rewards + CannotBondFreeBalanceOther, } #[derive(Encode, Decode, PartialEq, TypeInfo, frame_support::PalletError, RuntimeDebug)] @@ -1591,8 +1593,23 @@ pub mod pallet { T::WeightInfo::bond_extra_transfer() .max(T::WeightInfo::bond_extra_reward()) )] - pub fn bond_extra(origin: OriginFor, extra: BondExtra>) -> DispatchResult { + pub fn bond_extra( + origin: OriginFor, + other: Option>, + extra: BondExtra>, + ) -> DispatchResult { let who = ensure_signed(origin)?; + let other = other.map(T::Lookup::lookup).transpose()?; + let is_bonding_for = other.is_some(); + let who = other.unwrap_or(who); + if is_bonding_for { + ensure!( + RewardClaimPermission::::get(&who) == RewardClaim::Permissionless, + Error::::DoesNotHavePermission + ); + ensure!(extra == BondExtra::Rewards, Error::::CannotBondFreeBalanceOther); + } + let (mut member, mut bonded_pool, mut reward_pool) = Self::get_member_with_pools(&who)?; // payout related stuff: we must claim the payouts, and updated recorded payout data @@ -2139,62 +2156,6 @@ pub mod pallet { T::Staking::chill(&bonded_pool.bonded_account()) } - /// Bond pending rewards of `member_account` into the pool they already belong. - /// - /// Note: `member_account` must pass `RewardClaim::Permissionless` to `set_reward_claim`, - /// making this call permissionless. - #[pallet::call_index(14)] - #[pallet::weight( - T::WeightInfo::bond_extra_transfer() - .max(T::WeightInfo::bond_extra_reward()) - )] - pub fn bond_extra_pending_rewards_other( - origin: OriginFor, - member_account: AccountIdLookupOf, - ) -> DispatchResult { - ensure_signed(origin)?; - let member_account = T::Lookup::lookup(member_account)?; - let (mut member, mut bonded_pool, mut reward_pool) = - Self::get_member_with_pools(&member_account)?; - - ensure!( - RewardClaimPermission::::get(&member_account) == RewardClaim::Permissionless, - Error::::DoesNotHavePermission - ); - - reward_pool.update_records(bonded_pool.id, bonded_pool.points)?; - ensure!(!member.active_points().is_zero(), Error::::FullyUnbonding); - - let current_reward_counter = - reward_pool.current_reward_counter(bonded_pool.id, bonded_pool.points)?; - let pending_rewards = member.pending_rewards(current_reward_counter)?; - - if !pending_rewards.is_zero() { - member.last_recorded_reward_counter = current_reward_counter; - reward_pool.register_claimed_reward(pending_rewards); - } - - let points_issued = bonded_pool.try_bond_funds( - &bonded_pool.reward_account(), - pending_rewards, - BondType::Later, - )?; - - bonded_pool.ok_to_be_open()?; - member.points = member.points.saturating_add(points_issued); - - Self::deposit_event(Event::::Bonded { - member: member_account.clone(), - pool_id: member.pool_id, - bonded: pending_rewards, - joined: false, - }); - - Self::put_member_with_pools(&member_account, member, bonded_pool, reward_pool); - - Ok(()) - } - /// Sets permission to claim reward. /// /// Lets a pool member to choose who can claim pending rewards on their behalf. By default, @@ -2206,7 +2167,7 @@ pub mod pallet { /// /// * `Origin` - Member of a pool. /// * `actor` - Account to claim reward. - #[pallet::call_index(15)] + #[pallet::call_index(14)] #[pallet::weight(T::DbWeight::get().reads_writes(1, 1))] pub fn set_reward_claim(origin: OriginFor, actor: RewardClaim) -> DispatchResult { let who = ensure_signed(origin)?; @@ -2403,7 +2364,7 @@ impl Pallet { // We check for zero above .div(current_points) } - // TODO: Investigate logic here to add conditional event emission on different account types. + /// If the member has some rewards, transfer a payout from the reward pool to the member. // Emits events and potentially modifies pool state if any arithmetic saturates, but does // not persist any of the mutable inputs to storage. diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 9ca53808dd3e1..5cd9a5259a865 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -1384,7 +1384,11 @@ mod claim_payout { ); // 30 now bumps itself to be like 20. - assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(30), BondExtra::FreeBalance(10))); + assert_ok!(Pools::bond_extra( + RuntimeOrigin::signed(30), + None, + BondExtra::FreeBalance(10) + )); // more rewards come in. Balances::mutate_account(&default_reward_account(), |f| f.free += 100).unwrap(); @@ -1547,7 +1551,11 @@ mod claim_payout { Balances::mutate_account(&default_reward_account(), |f| f.free += 60).unwrap(); // and 20 bonds more -- they should not have more share of this reward. - assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(20), BondExtra::FreeBalance(10))); + assert_ok!(Pools::bond_extra( + RuntimeOrigin::signed(20), + None, + BondExtra::FreeBalance(10) + )); // everyone claim. assert_ok!(Pools::claim_payout(RuntimeOrigin::signed(10))); @@ -1738,6 +1746,7 @@ mod claim_payout { { assert_ok!(Pools::bond_extra( RuntimeOrigin::signed(10), + None, BondExtra::FreeBalance(10) )); let (member, _, reward_pool) = Pools::get_member_with_pools(&10).unwrap(); @@ -1753,6 +1762,7 @@ mod claim_payout { { assert_ok!(Pools::bond_extra( RuntimeOrigin::signed(10), + None, BondExtra::FreeBalance(10) )); let (member, _, reward_pool) = Pools::get_member_with_pools(&10).unwrap(); @@ -1770,6 +1780,7 @@ mod claim_payout { { assert_ok!(Pools::bond_extra( RuntimeOrigin::signed(20), + None, BondExtra::FreeBalance(10) )); let (member, _, reward_pool) = Pools::get_member_with_pools(&20).unwrap(); @@ -1826,7 +1837,7 @@ mod claim_payout { // 20 re-bonds it. { - assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(20), BondExtra::Rewards)); + assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(20), None, BondExtra::Rewards)); let (member, _, reward_pool) = Pools::get_member_with_pools(&10).unwrap(); assert_eq!(member.last_recorded_reward_counter, 1.into()); assert_eq!(reward_pool.total_rewards_claimed, 30); @@ -2186,7 +2197,11 @@ mod unbond { // - depositor cannot unbond to below limit or 0 ExtBuilder::default().min_join_bond(10).build_and_execute(|| { // give the depositor some extra funds. - assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(10), BondExtra::FreeBalance(10))); + assert_ok!(Pools::bond_extra( + RuntimeOrigin::signed(10), + None, + BondExtra::FreeBalance(10) + )); assert_eq!(PoolMembers::::get(10).unwrap().points, 20); // can unbond to above the limit. @@ -2214,7 +2229,11 @@ mod unbond { // - depositor can never be kicked. ExtBuilder::default().min_join_bond(10).build_and_execute(|| { // give the depositor some extra funds. - assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(10), BondExtra::FreeBalance(10))); + assert_ok!(Pools::bond_extra( + RuntimeOrigin::signed(10), + None, + BondExtra::FreeBalance(10) + )); assert_eq!(PoolMembers::::get(10).unwrap().points, 20); // set the stage @@ -2252,7 +2271,11 @@ mod unbond { // depositor can never be permissionlessly unbonded. ExtBuilder::default().min_join_bond(10).build_and_execute(|| { // give the depositor some extra funds. - assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(10), BondExtra::FreeBalance(10))); + assert_ok!(Pools::bond_extra( + RuntimeOrigin::signed(10), + None, + BondExtra::FreeBalance(10) + )); assert_eq!(PoolMembers::::get(10).unwrap().points, 20); // set the stage @@ -2293,6 +2316,7 @@ mod unbond { // give the depositor some extra funds. assert_ok!(Pools::bond_extra( RuntimeOrigin::signed(10), + None, BondExtra::FreeBalance(10) )); assert_eq!(PoolMembers::::get(10).unwrap().points, 20); @@ -2327,7 +2351,11 @@ mod unbond { // - depositor can unbond to 0 if last and destroying. ExtBuilder::default().min_join_bond(10).build_and_execute(|| { // give the depositor some extra funds. - assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(10), BondExtra::FreeBalance(10))); + assert_ok!(Pools::bond_extra( + RuntimeOrigin::signed(10), + None, + BondExtra::FreeBalance(10) + )); assert_eq!(PoolMembers::::get(10).unwrap().points, 20); // set the stage @@ -3598,7 +3626,11 @@ mod withdraw_unbonded { #[test] fn partial_withdraw_unbonded_depositor() { ExtBuilder::default().ed(1).build_and_execute(|| { - assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(10), BondExtra::FreeBalance(10))); + assert_ok!(Pools::bond_extra( + RuntimeOrigin::signed(10), + None, + BondExtra::FreeBalance(10) + )); unsafe_set_state(1, PoolState::Destroying); // given @@ -3996,7 +4028,11 @@ mod withdraw_unbonded { ExtBuilder::default().ed(1).build_and_execute(|| { // depositor now has 20, they can unbond to 10. assert_eq!(Pools::depositor_min_bond(), 10); - assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(10), BondExtra::FreeBalance(10))); + assert_ok!(Pools::bond_extra( + RuntimeOrigin::signed(10), + None, + BondExtra::FreeBalance(10) + )); // now they can. assert_ok!(Pools::unbond(RuntimeOrigin::signed(10), 10, 7)); @@ -4503,7 +4539,11 @@ mod bond_extra { assert_eq!(Balances::free_balance(10), 100); // when - assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(10), BondExtra::FreeBalance(10))); + assert_ok!(Pools::bond_extra( + RuntimeOrigin::signed(10), + None, + BondExtra::FreeBalance(10) + )); // then assert_eq!(Balances::free_balance(10), 90); @@ -4520,7 +4560,11 @@ mod bond_extra { ); // when - assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(10), BondExtra::FreeBalance(20))); + assert_ok!(Pools::bond_extra( + RuntimeOrigin::signed(10), + None, + BondExtra::FreeBalance(20) + )); // then assert_eq!(Balances::free_balance(10), 70); @@ -4549,7 +4593,7 @@ mod bond_extra { assert_eq!(Balances::free_balance(10), 35); // when - assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(10), BondExtra::Rewards)); + assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(10), None, BondExtra::Rewards)); // then assert_eq!(Balances::free_balance(10), 35); @@ -4592,7 +4636,7 @@ mod bond_extra { assert_eq!(Balances::free_balance(20), 20); // when - assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(10), BondExtra::Rewards)); + assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(10), None, BondExtra::Rewards)); assert_eq!(Balances::free_balance(&default_reward_account()), 7); // then @@ -4602,7 +4646,7 @@ mod bond_extra { assert_eq!(BondedPools::::get(1).unwrap().points, 30 + 1); // when - assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(20), BondExtra::Rewards)); + assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(20), None, BondExtra::Rewards)); // then assert_eq!(Balances::free_balance(20), 20); @@ -4627,7 +4671,7 @@ mod bond_extra { } #[test] - fn bond_extra_pending_rewards_other_works() { + fn bond_extra_rewards_other() { ExtBuilder::default().add_members(vec![(20, 20)]).build_and_execute(|| { Balances::make_free_balance_be(&default_reward_account(), 8); // ... if which only 3 is claimable to make sure the reward account does not die. @@ -4642,16 +4686,17 @@ mod bond_extra { assert_eq!(Balances::free_balance(10), 35); assert_eq!(Balances::free_balance(20), 20); - // Make permissionless and bond pending rewards of member + // Permissioned by default assert_noop!( - Pools::bond_extra_pending_rewards_other(RuntimeOrigin::signed(80), 10), + Pools::bond_extra(RuntimeOrigin::signed(80), Some(20), BondExtra::Rewards), Error::::DoesNotHavePermission ); + assert_ok!(Pools::set_reward_claim( RuntimeOrigin::signed(10), RewardClaim::Permissionless )); - assert_ok!(Pools::bond_extra_pending_rewards_other(RuntimeOrigin::signed(50), 10)); + assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(50), Some(10), BondExtra::Rewards)); assert_eq!(Balances::free_balance(&default_reward_account()), 7); // then @@ -4661,21 +4706,24 @@ mod bond_extra { // when assert_noop!( - Pools::bond_extra_pending_rewards_other(RuntimeOrigin::signed(40), 20), - Error::::DoesNotHavePermission + Pools::bond_extra(RuntimeOrigin::signed(40), None, BondExtra::Rewards), + Error::::PoolMemberNotFound ); assert_ok!(Pools::set_reward_claim( RuntimeOrigin::signed(20), RewardClaim::Permissionless )); - assert_ok!(Pools::bond_extra_pending_rewards_other(RuntimeOrigin::signed(20), 20)); + assert_noop!( + Pools::bond_extra(RuntimeOrigin::signed(20), Some(20), BondExtra::FreeBalance(10)), + Error::::CannotBondFreeBalanceOther + ); // then assert_eq!(Balances::free_balance(20), 20); // 20's share of the rewards is the other 2/3 of the rewards, since they have 20/30 of // the shares - assert_eq!(PoolMembers::::get(20).unwrap().points, 20 + 2); - assert_eq!(BondedPools::::get(1).unwrap().points, 30 + 3); + assert_eq!(PoolMembers::::get(20).unwrap().points, 20); + assert_eq!(BondedPools::::get(1).unwrap().points, 31); }) } } From 15c286f81d6852495f635085727f452c98d376da Mon Sep 17 00:00:00 2001 From: doordashcon Date: Tue, 7 Feb 2023 09:09:58 +0100 Subject: [PATCH 20/50] bond_extra_other --- .../nomination-pools/benchmarking/src/lib.rs | 4 +- frame/nomination-pools/fuzzer/src/call.rs | 2 +- frame/nomination-pools/src/lib.rs | 121 +++++++++++------- frame/nomination-pools/src/tests.rs | 90 ++++--------- 4 files changed, 105 insertions(+), 112 deletions(-) diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index 0e921ea2182b5..e871c5939f5fc 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -245,7 +245,7 @@ frame_benchmarking::benchmarks! { // creator of the src pool will bond-extra, bumping itself to dest bag. - }: bond_extra(RuntimeOrigin::Signed(scenario.creator1.clone()), None, BondExtra::FreeBalance(extra)) + }: bond_extra(RuntimeOrigin::Signed(scenario.creator1.clone()), BondExtra::FreeBalance(extra)) verify { assert!( T::Staking::active_stake(&scenario.origin1).unwrap() >= @@ -263,7 +263,7 @@ frame_benchmarking::benchmarks! { assert!(extra >= CurrencyOf::::minimum_balance()); CurrencyOf::::deposit_creating(&reward_account1, extra); - }: bond_extra(RuntimeOrigin::Signed(scenario.creator1.clone()), None, BondExtra::Rewards) + }: bond_extra(RuntimeOrigin::Signed(scenario.creator1.clone()), BondExtra::Rewards) verify { assert!( T::Staking::active_stake(&scenario.origin1).unwrap() >= diff --git a/frame/nomination-pools/fuzzer/src/call.rs b/frame/nomination-pools/fuzzer/src/call.rs index 9b4e787724ce9..b07903609e8ab 100644 --- a/frame/nomination-pools/fuzzer/src/call.rs +++ b/frame/nomination-pools/fuzzer/src/call.rs @@ -110,7 +110,7 @@ fn random_call(mut rng: &mut R) -> (pools::Call, RuntimeOrigin) { let amount = random_ed_multiple(&mut rng); BondExtra::FreeBalance(amount) }; - (PoolsCall::::bond_extra { other: Some(who), extra }, origin) + (PoolsCall::::bond_extra { extra }, origin) }, 2 => { // claim_payout diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 7e26783a40786..ab073d87196b4 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -1585,6 +1585,7 @@ pub mod pallet { /// accumulated rewards, see [`BondExtra`]. /// /// Bonding extra funds implies an automatic payout of all pending rewards as well. + /// See `bond_extra_other` to bond pending rewards of `other` members. // NOTE: this transaction is implemented with the sole purpose of readability and // correctness, not optimization. We read/write several storage items multiple times instead // of just once, in the spirit reusing code. @@ -1593,51 +1594,10 @@ pub mod pallet { T::WeightInfo::bond_extra_transfer() .max(T::WeightInfo::bond_extra_reward()) )] - pub fn bond_extra( - origin: OriginFor, - other: Option>, - extra: BondExtra>, - ) -> DispatchResult { + pub fn bond_extra(origin: OriginFor, extra: BondExtra>) -> DispatchResult { let who = ensure_signed(origin)?; - let other = other.map(T::Lookup::lookup).transpose()?; - let is_bonding_for = other.is_some(); - let who = other.unwrap_or(who); - if is_bonding_for { - ensure!( - RewardClaimPermission::::get(&who) == RewardClaim::Permissionless, - Error::::DoesNotHavePermission - ); - ensure!(extra == BondExtra::Rewards, Error::::CannotBondFreeBalanceOther); - } - - let (mut member, mut bonded_pool, mut reward_pool) = Self::get_member_with_pools(&who)?; - - // payout related stuff: we must claim the payouts, and updated recorded payout data - // before updating the bonded pool points, similar to that of `join` transaction. - reward_pool.update_records(bonded_pool.id, bonded_pool.points)?; - let claimed = - Self::do_reward_payout(&who, &mut member, &mut bonded_pool, &mut reward_pool)?; - - let (points_issued, bonded) = match extra { - BondExtra::FreeBalance(amount) => - (bonded_pool.try_bond_funds(&who, amount, BondType::Later)?, amount), - BondExtra::Rewards => - (bonded_pool.try_bond_funds(&who, claimed, BondType::Later)?, claimed), - }; - - bonded_pool.ok_to_be_open()?; - member.points = - member.points.checked_add(&points_issued).ok_or(Error::::OverflowRisk)?; - - Self::deposit_event(Event::::Bonded { - member: who.clone(), - pool_id: member.pool_id, - bonded, - joined: false, - }); - Self::put_member_with_pools(&who, member, bonded_pool, reward_pool); - Ok(()) + Self::do_bond_extra(who, None, extra) } /// A bonded member can use this to claim their payout based on the rewards that the pool @@ -2156,6 +2116,29 @@ pub mod pallet { T::Staking::chill(&bonded_pool.bonded_account()) } + /// Bond `extra` more funds from `origin` and + /// pending rewards of `other` into their respective pools. + /// + /// Origin can bond extra more funds from free balance and pending rewards + /// when `other` is `None`. + /// + /// Origin can bond extra pending rewards of `other` given `Some` is a member and + /// set reward claim to Permissionless. + #[pallet::call_index(14)] + #[pallet::weight( + T::WeightInfo::bond_extra_transfer() + .max(T::WeightInfo::bond_extra_reward()) + )] + pub fn bond_extra_other( + origin: OriginFor, + other: Option>, + extra: BondExtra>, + ) -> DispatchResult { + let who = ensure_signed(origin)?; + + Self::do_bond_extra(who, other, extra) + } + /// Sets permission to claim reward. /// /// Lets a pool member to choose who can claim pending rewards on their behalf. By default, @@ -2165,9 +2148,9 @@ pub mod pallet { /// /// # Arguments /// - /// * `Origin` - Member of a pool. - /// * `actor` - Account to claim reward. - #[pallet::call_index(14)] + /// * `origin` - Member of a pool. + /// * `actor` - Account to claim reward. // improve this + #[pallet::call_index(15)] #[pallet::weight(T::DbWeight::get().reads_writes(1, 1))] pub fn set_reward_claim(origin: OriginFor, actor: RewardClaim) -> DispatchResult { let who = ensure_signed(origin)?; @@ -2480,6 +2463,52 @@ impl Pallet { Ok(()) } + fn do_bond_extra( + who: T::AccountId, + other: Option>, + extra: BondExtra>, + ) -> DispatchResult { + let other = other.map(T::Lookup::lookup).transpose()?; + let is_bonding_for = other.is_some(); + let who = other.unwrap_or(who); + if is_bonding_for { + ensure!( + RewardClaimPermission::::get(&who) == RewardClaim::Permissionless, + Error::::DoesNotHavePermission + ); + ensure!(extra == BondExtra::Rewards, Error::::CannotBondFreeBalanceOther); + } + + let (mut member, mut bonded_pool, mut reward_pool) = Self::get_member_with_pools(&who)?; + + // payout related stuff: we must claim the payouts, and updated recorded payout data + // before updating the bonded pool points, similar to that of `join` transaction. + reward_pool.update_records(bonded_pool.id, bonded_pool.points)?; + let claimed = + Self::do_reward_payout(&who, &mut member, &mut bonded_pool, &mut reward_pool)?; + + let (points_issued, bonded) = match extra { + BondExtra::FreeBalance(amount) => + (bonded_pool.try_bond_funds(&who, amount, BondType::Later)?, amount), + BondExtra::Rewards => + (bonded_pool.try_bond_funds(&who, claimed, BondType::Later)?, claimed), + }; + + bonded_pool.ok_to_be_open()?; + member.points = + member.points.checked_add(&points_issued).ok_or(Error::::OverflowRisk)?; + + Self::deposit_event(Event::::Bonded { + member: who.clone(), + pool_id: member.pool_id, + bonded, + joined: false, + }); + Self::put_member_with_pools(&who, member, bonded_pool, reward_pool); + + Ok(()) + } + /// Ensure the correctness of the state of this pallet. /// /// This should be valid before or after each state transition of this pallet. diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 5cd9a5259a865..482f47a482fa1 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -1384,11 +1384,7 @@ mod claim_payout { ); // 30 now bumps itself to be like 20. - assert_ok!(Pools::bond_extra( - RuntimeOrigin::signed(30), - None, - BondExtra::FreeBalance(10) - )); + assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(30), BondExtra::FreeBalance(10))); // more rewards come in. Balances::mutate_account(&default_reward_account(), |f| f.free += 100).unwrap(); @@ -1551,11 +1547,7 @@ mod claim_payout { Balances::mutate_account(&default_reward_account(), |f| f.free += 60).unwrap(); // and 20 bonds more -- they should not have more share of this reward. - assert_ok!(Pools::bond_extra( - RuntimeOrigin::signed(20), - None, - BondExtra::FreeBalance(10) - )); + assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(20), BondExtra::FreeBalance(10))); // everyone claim. assert_ok!(Pools::claim_payout(RuntimeOrigin::signed(10))); @@ -1746,7 +1738,6 @@ mod claim_payout { { assert_ok!(Pools::bond_extra( RuntimeOrigin::signed(10), - None, BondExtra::FreeBalance(10) )); let (member, _, reward_pool) = Pools::get_member_with_pools(&10).unwrap(); @@ -1762,7 +1753,6 @@ mod claim_payout { { assert_ok!(Pools::bond_extra( RuntimeOrigin::signed(10), - None, BondExtra::FreeBalance(10) )); let (member, _, reward_pool) = Pools::get_member_with_pools(&10).unwrap(); @@ -1780,7 +1770,6 @@ mod claim_payout { { assert_ok!(Pools::bond_extra( RuntimeOrigin::signed(20), - None, BondExtra::FreeBalance(10) )); let (member, _, reward_pool) = Pools::get_member_with_pools(&20).unwrap(); @@ -1837,7 +1826,7 @@ mod claim_payout { // 20 re-bonds it. { - assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(20), None, BondExtra::Rewards)); + assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(20), BondExtra::Rewards)); let (member, _, reward_pool) = Pools::get_member_with_pools(&10).unwrap(); assert_eq!(member.last_recorded_reward_counter, 1.into()); assert_eq!(reward_pool.total_rewards_claimed, 30); @@ -2197,11 +2186,7 @@ mod unbond { // - depositor cannot unbond to below limit or 0 ExtBuilder::default().min_join_bond(10).build_and_execute(|| { // give the depositor some extra funds. - assert_ok!(Pools::bond_extra( - RuntimeOrigin::signed(10), - None, - BondExtra::FreeBalance(10) - )); + assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(10), BondExtra::FreeBalance(10))); assert_eq!(PoolMembers::::get(10).unwrap().points, 20); // can unbond to above the limit. @@ -2229,11 +2214,7 @@ mod unbond { // - depositor can never be kicked. ExtBuilder::default().min_join_bond(10).build_and_execute(|| { // give the depositor some extra funds. - assert_ok!(Pools::bond_extra( - RuntimeOrigin::signed(10), - None, - BondExtra::FreeBalance(10) - )); + assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(10), BondExtra::FreeBalance(10))); assert_eq!(PoolMembers::::get(10).unwrap().points, 20); // set the stage @@ -2271,11 +2252,7 @@ mod unbond { // depositor can never be permissionlessly unbonded. ExtBuilder::default().min_join_bond(10).build_and_execute(|| { // give the depositor some extra funds. - assert_ok!(Pools::bond_extra( - RuntimeOrigin::signed(10), - None, - BondExtra::FreeBalance(10) - )); + assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(10), BondExtra::FreeBalance(10))); assert_eq!(PoolMembers::::get(10).unwrap().points, 20); // set the stage @@ -2316,7 +2293,6 @@ mod unbond { // give the depositor some extra funds. assert_ok!(Pools::bond_extra( RuntimeOrigin::signed(10), - None, BondExtra::FreeBalance(10) )); assert_eq!(PoolMembers::::get(10).unwrap().points, 20); @@ -2351,11 +2327,7 @@ mod unbond { // - depositor can unbond to 0 if last and destroying. ExtBuilder::default().min_join_bond(10).build_and_execute(|| { // give the depositor some extra funds. - assert_ok!(Pools::bond_extra( - RuntimeOrigin::signed(10), - None, - BondExtra::FreeBalance(10) - )); + assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(10), BondExtra::FreeBalance(10))); assert_eq!(PoolMembers::::get(10).unwrap().points, 20); // set the stage @@ -3626,11 +3598,7 @@ mod withdraw_unbonded { #[test] fn partial_withdraw_unbonded_depositor() { ExtBuilder::default().ed(1).build_and_execute(|| { - assert_ok!(Pools::bond_extra( - RuntimeOrigin::signed(10), - None, - BondExtra::FreeBalance(10) - )); + assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(10), BondExtra::FreeBalance(10))); unsafe_set_state(1, PoolState::Destroying); // given @@ -4028,11 +3996,7 @@ mod withdraw_unbonded { ExtBuilder::default().ed(1).build_and_execute(|| { // depositor now has 20, they can unbond to 10. assert_eq!(Pools::depositor_min_bond(), 10); - assert_ok!(Pools::bond_extra( - RuntimeOrigin::signed(10), - None, - BondExtra::FreeBalance(10) - )); + assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(10), BondExtra::FreeBalance(10))); // now they can. assert_ok!(Pools::unbond(RuntimeOrigin::signed(10), 10, 7)); @@ -4539,11 +4503,7 @@ mod bond_extra { assert_eq!(Balances::free_balance(10), 100); // when - assert_ok!(Pools::bond_extra( - RuntimeOrigin::signed(10), - None, - BondExtra::FreeBalance(10) - )); + assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(10), BondExtra::FreeBalance(10))); // then assert_eq!(Balances::free_balance(10), 90); @@ -4560,11 +4520,7 @@ mod bond_extra { ); // when - assert_ok!(Pools::bond_extra( - RuntimeOrigin::signed(10), - None, - BondExtra::FreeBalance(20) - )); + assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(10), BondExtra::FreeBalance(20))); // then assert_eq!(Balances::free_balance(10), 70); @@ -4593,7 +4549,7 @@ mod bond_extra { assert_eq!(Balances::free_balance(10), 35); // when - assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(10), None, BondExtra::Rewards)); + assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(10), BondExtra::Rewards)); // then assert_eq!(Balances::free_balance(10), 35); @@ -4636,7 +4592,7 @@ mod bond_extra { assert_eq!(Balances::free_balance(20), 20); // when - assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(10), None, BondExtra::Rewards)); + assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(10), BondExtra::Rewards)); assert_eq!(Balances::free_balance(&default_reward_account()), 7); // then @@ -4646,7 +4602,7 @@ mod bond_extra { assert_eq!(BondedPools::::get(1).unwrap().points, 30 + 1); // when - assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(20), None, BondExtra::Rewards)); + assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(20), BondExtra::Rewards)); // then assert_eq!(Balances::free_balance(20), 20); @@ -4671,7 +4627,7 @@ mod bond_extra { } #[test] - fn bond_extra_rewards_other() { + fn bond_extra_other() { ExtBuilder::default().add_members(vec![(20, 20)]).build_and_execute(|| { Balances::make_free_balance_be(&default_reward_account(), 8); // ... if which only 3 is claimable to make sure the reward account does not die. @@ -4688,7 +4644,7 @@ mod bond_extra { // Permissioned by default assert_noop!( - Pools::bond_extra(RuntimeOrigin::signed(80), Some(20), BondExtra::Rewards), + Pools::bond_extra_other(RuntimeOrigin::signed(80), Some(20), BondExtra::Rewards), Error::::DoesNotHavePermission ); @@ -4696,7 +4652,11 @@ mod bond_extra { RuntimeOrigin::signed(10), RewardClaim::Permissionless )); - assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(50), Some(10), BondExtra::Rewards)); + assert_ok!(Pools::bond_extra_other( + RuntimeOrigin::signed(50), + Some(10), + BondExtra::Rewards + )); assert_eq!(Balances::free_balance(&default_reward_account()), 7); // then @@ -4706,7 +4666,7 @@ mod bond_extra { // when assert_noop!( - Pools::bond_extra(RuntimeOrigin::signed(40), None, BondExtra::Rewards), + Pools::bond_extra_other(RuntimeOrigin::signed(40), None, BondExtra::Rewards), Error::::PoolMemberNotFound ); assert_ok!(Pools::set_reward_claim( @@ -4714,7 +4674,11 @@ mod bond_extra { RewardClaim::Permissionless )); assert_noop!( - Pools::bond_extra(RuntimeOrigin::signed(20), Some(20), BondExtra::FreeBalance(10)), + Pools::bond_extra_other( + RuntimeOrigin::signed(20), + Some(20), + BondExtra::FreeBalance(10) + ), Error::::CannotBondFreeBalanceOther ); From fcd63b2921e0e8914cfc50ce6249714ce20c55ce Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Mon, 13 Feb 2023 10:19:54 -0500 Subject: [PATCH 21/50] Apply suggestions from code review --- frame/nomination-pools/src/lib.rs | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index ab073d87196b4..dd2d7bfa5568b 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -1597,7 +1597,7 @@ pub mod pallet { pub fn bond_extra(origin: OriginFor, extra: BondExtra>) -> DispatchResult { let who = ensure_signed(origin)?; - Self::do_bond_extra(who, None, extra) + Self::do_bond_extra(who, who, extra) } /// A bonded member can use this to claim their payout based on the rewards that the pool @@ -2131,7 +2131,7 @@ pub mod pallet { )] pub fn bond_extra_other( origin: OriginFor, - other: Option>, + other: AccountIdLookupOf, extra: BondExtra>, ) -> DispatchResult { let who = ensure_signed(origin)?; @@ -2468,16 +2468,14 @@ impl Pallet { other: Option>, extra: BondExtra>, ) -> DispatchResult { - let other = other.map(T::Lookup::lookup).transpose()?; - let is_bonding_for = other.is_some(); - let who = other.unwrap_or(who); - if is_bonding_for { - ensure!( - RewardClaimPermission::::get(&who) == RewardClaim::Permissionless, - Error::::DoesNotHavePermission - ); - ensure!(extra == BondExtra::Rewards, Error::::CannotBondFreeBalanceOther); + let other = T::Lookup::lookup(other)?; + // either it is permissioned, or they have allowed it. + if who != other { + ensure!(matches!(RewardClaimPermission::::get(&target), + RewardClaim::Permissionless), Error::::DoesNotHavePermission); + ensure!(extra == BondExtra::Rewards, Error::::CannotBondFreeBalanceOther); } + ); let (mut member, mut bonded_pool, mut reward_pool) = Self::get_member_with_pools(&who)?; From f6254faffc99ff599da7773cbd9db067761df68e Mon Sep 17 00:00:00 2001 From: kianenigma Date: Mon, 13 Feb 2023 09:33:42 -0600 Subject: [PATCH 22/50] Fixes from kian --- frame/nomination-pools/src/lib.rs | 17 ++++++++--------- frame/nomination-pools/src/tests.rs | 23 ++++++++++++----------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index dd2d7bfa5568b..842d2a0e3f367 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -1597,7 +1597,7 @@ pub mod pallet { pub fn bond_extra(origin: OriginFor, extra: BondExtra>) -> DispatchResult { let who = ensure_signed(origin)?; - Self::do_bond_extra(who, who, extra) + Self::do_bond_extra(who.clone(), who, extra) } /// A bonded member can use this to claim their payout based on the rewards that the pool @@ -2136,7 +2136,7 @@ pub mod pallet { ) -> DispatchResult { let who = ensure_signed(origin)?; - Self::do_bond_extra(who, other, extra) + Self::do_bond_extra(who, T::Lookup::lookup(other)?, extra) } /// Sets permission to claim reward. @@ -2464,18 +2464,17 @@ impl Pallet { } fn do_bond_extra( + signer: T::AccountId, who: T::AccountId, - other: Option>, extra: BondExtra>, ) -> DispatchResult { - let other = T::Lookup::lookup(other)?; - // either it is permissioned, or they have allowed it. - if who != other { - ensure!(matches!(RewardClaimPermission::::get(&target), - RewardClaim::Permissionless), Error::::DoesNotHavePermission); + if signer != who { + ensure!(matches!( + RewardClaimPermission::::get(&who), + RewardClaim::Permissionless), Error::::DoesNotHavePermission + ); ensure!(extra == BondExtra::Rewards, Error::::CannotBondFreeBalanceOther); } - ); let (mut member, mut bonded_pool, mut reward_pool) = Self::get_member_with_pools(&who)?; diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 482f47a482fa1..670915a61eebe 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -4644,7 +4644,7 @@ mod bond_extra { // Permissioned by default assert_noop!( - Pools::bond_extra_other(RuntimeOrigin::signed(80), Some(20), BondExtra::Rewards), + Pools::bond_extra_other(RuntimeOrigin::signed(80), 20, BondExtra::Rewards), Error::::DoesNotHavePermission ); @@ -4654,7 +4654,7 @@ mod bond_extra { )); assert_ok!(Pools::bond_extra_other( RuntimeOrigin::signed(50), - Some(10), + 10, BondExtra::Rewards )); assert_eq!(Balances::free_balance(&default_reward_account()), 7); @@ -4666,21 +4666,22 @@ mod bond_extra { // when assert_noop!( - Pools::bond_extra_other(RuntimeOrigin::signed(40), None, BondExtra::Rewards), + Pools::bond_extra_other(RuntimeOrigin::signed(40), 40, BondExtra::Rewards), Error::::PoolMemberNotFound ); assert_ok!(Pools::set_reward_claim( RuntimeOrigin::signed(20), RewardClaim::Permissionless )); - assert_noop!( - Pools::bond_extra_other( - RuntimeOrigin::signed(20), - Some(20), - BondExtra::FreeBalance(10) - ), - Error::::CannotBondFreeBalanceOther - ); + // todo DDS. + // assert_n!( + // Pools::bond_extra_other( + // RuntimeOrigin::signed(20), + // 20, + // BondExtra::FreeBalance(10) + // ), + // Error::::CannotBondFreeBalanceOther + // ); // then assert_eq!(Balances::free_balance(20), 20); From ce1ebc95183f60af6909ea8fbc312dca20033b2b Mon Sep 17 00:00:00 2001 From: doordashcon Date: Tue, 14 Feb 2023 08:42:16 +0100 Subject: [PATCH 23/50] updates docs & test --- frame/nomination-pools/src/lib.rs | 19 +++++++++--------- frame/nomination-pools/src/tests.rs | 31 ++++++++++------------------- 2 files changed, 20 insertions(+), 30 deletions(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 842d2a0e3f367..a5a572a915f3e 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -2117,13 +2117,14 @@ pub mod pallet { } /// Bond `extra` more funds from `origin` and - /// pending rewards of `other` into their respective pools. + /// pending rewards of `other` members into their respective pools. /// - /// Origin can bond extra more funds from free balance and pending rewards - /// when `other` is `None`. + /// Origin can bond extra funds from free balance + /// or pending rewards when `origin == other`. /// - /// Origin can bond extra pending rewards of `other` given `Some` is a member and - /// set reward claim to Permissionless. + /// In the case of `origin != other`, `origin` can only bond extra + /// pending rewards of `other` members assuming set_reward_claim + /// for the given member is `Permissionless`. #[pallet::call_index(14)] #[pallet::weight( T::WeightInfo::bond_extra_transfer() @@ -2469,11 +2470,11 @@ impl Pallet { extra: BondExtra>, ) -> DispatchResult { if signer != who { - ensure!(matches!( - RewardClaimPermission::::get(&who), - RewardClaim::Permissionless), Error::::DoesNotHavePermission + ensure!( + matches!(RewardClaimPermission::::get(&who), RewardClaim::Permissionless), + Error::::DoesNotHavePermission ); - ensure!(extra == BondExtra::Rewards, Error::::CannotBondFreeBalanceOther); + ensure!(extra == BondExtra::Rewards, Error::::CannotBondFreeBalanceOther); } let (mut member, mut bonded_pool, mut reward_pool) = Self::get_member_with_pools(&who)?; diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 670915a61eebe..4c3747090127f 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -4652,11 +4652,7 @@ mod bond_extra { RuntimeOrigin::signed(10), RewardClaim::Permissionless )); - assert_ok!(Pools::bond_extra_other( - RuntimeOrigin::signed(50), - 10, - BondExtra::Rewards - )); + assert_ok!(Pools::bond_extra_other(RuntimeOrigin::signed(50), 10, BondExtra::Rewards)); assert_eq!(Balances::free_balance(&default_reward_account()), 7); // then @@ -4669,26 +4665,19 @@ mod bond_extra { Pools::bond_extra_other(RuntimeOrigin::signed(40), 40, BondExtra::Rewards), Error::::PoolMemberNotFound ); - assert_ok!(Pools::set_reward_claim( + + // when + assert_ok!(Pools::bond_extra_other( RuntimeOrigin::signed(20), - RewardClaim::Permissionless + 20, + BondExtra::FreeBalance(10) )); - // todo DDS. - // assert_n!( - // Pools::bond_extra_other( - // RuntimeOrigin::signed(20), - // 20, - // BondExtra::FreeBalance(10) - // ), - // Error::::CannotBondFreeBalanceOther - // ); // then - assert_eq!(Balances::free_balance(20), 20); - // 20's share of the rewards is the other 2/3 of the rewards, since they have 20/30 of - // the shares - assert_eq!(PoolMembers::::get(20).unwrap().points, 20); - assert_eq!(BondedPools::::get(1).unwrap().points, 31); + assert_eq!(Balances::free_balance(20), 12); + assert_eq!(Balances::free_balance(&default_reward_account()), 5); + assert_eq!(PoolMembers::::get(20).unwrap().points, 30); + assert_eq!(BondedPools::::get(1).unwrap().points, 41); }) } } From 121122921abb43d51d9d9d731904c36249f4073d Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Mon, 20 Feb 2023 12:58:00 +0000 Subject: [PATCH 24/50] Update frame/nomination-pools/src/lib.rs --- frame/nomination-pools/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index a5a572a915f3e..e8ebccb228fdd 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -420,7 +420,7 @@ pub enum RewardClaim { Permissionless, } -impl Default for RewardClaim { +impl Default for ClaimPermission { fn default() -> Self { Self::Permissioned } From 6e4f2b4665f35499d253ee5dff03618d77c1c1c4 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Mon, 20 Feb 2023 12:58:10 +0000 Subject: [PATCH 25/50] Update frame/nomination-pools/src/lib.rs --- frame/nomination-pools/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index e8ebccb228fdd..0fd7740356e77 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -1333,7 +1333,7 @@ pub mod pallet { /// Map from a pool member account to their preference regarding reward payout. #[pallet::storage] - pub type RewardClaimPermission = + pub type ClaimPermissions = StorageMap<_, Twox64Concat, T::AccountId, RewardClaim, ValueQuery>; #[pallet::genesis_config] From 832281d35907ff66808774d38a71070d08550e3b Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Mon, 20 Feb 2023 12:58:21 +0000 Subject: [PATCH 26/50] Update frame/nomination-pools/src/lib.rs --- frame/nomination-pools/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 0fd7740356e77..316391ed694a8 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -1493,8 +1493,8 @@ pub mod pallet { PoolIdInUse, /// Pool id provided is not correct/usable. InvalidPoolId, - /// Restricted to pending rewards - CannotBondFreeBalanceOther, + /// Bonding extra is restricted to the exact pending reward amount. + BondExtraRestricted, } #[derive(Encode, Decode, PartialEq, TypeInfo, frame_support::PalletError, RuntimeDebug)] From e242799fd0dc11d2c0c70f4a7b249ce2329df9d5 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Mon, 20 Feb 2023 12:58:30 +0000 Subject: [PATCH 27/50] Update frame/nomination-pools/src/lib.rs --- frame/nomination-pools/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 316391ed694a8..55dac1228a95e 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -1715,7 +1715,7 @@ pub mod pallet { // Now that we know everything has worked write the items to storage. SubPoolsStorage::insert(&member.pool_id, sub_pools); Self::put_member_with_pools(&member_account, member, bonded_pool, reward_pool); - >::remove(member_account); + >::remove(member_account); Ok(()) } From 011d733242bb28cda65a62859c4867df32521fbe Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Mon, 20 Feb 2023 12:58:39 +0000 Subject: [PATCH 28/50] Update frame/nomination-pools/src/lib.rs --- frame/nomination-pools/src/lib.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 55dac1228a95e..534fc400719ac 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -2116,15 +2116,12 @@ pub mod pallet { T::Staking::chill(&bonded_pool.bonded_account()) } - /// Bond `extra` more funds from `origin` and - /// pending rewards of `other` members into their respective pools. + /// `origin` bonds funds from `extra` for some pool member `member` into their respective pools. /// - /// Origin can bond extra funds from free balance - /// or pending rewards when `origin == other`. + /// `origin` can bond extra funds from free balance or pending rewards when `origin == other`. /// - /// In the case of `origin != other`, `origin` can only bond extra - /// pending rewards of `other` members assuming set_reward_claim - /// for the given member is `Permissionless`. + /// In the case of `origin != other`, `origin` can only bond extra pending rewards of `other` + /// members assuming set_reward_claim for the given member is `Permissionless`. #[pallet::call_index(14)] #[pallet::weight( T::WeightInfo::bond_extra_transfer() @@ -2132,12 +2129,12 @@ pub mod pallet { )] pub fn bond_extra_other( origin: OriginFor, - other: AccountIdLookupOf, + member: AccountIdLookupOf, extra: BondExtra>, ) -> DispatchResult { let who = ensure_signed(origin)?; - Self::do_bond_extra(who, T::Lookup::lookup(other)?, extra) + Self::do_bond_extra(who, T::Lookup::lookup(member)?, extra) } /// Sets permission to claim reward. From d9590b18cadad6360f7de932200080f7f75f7f88 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Mon, 20 Feb 2023 12:58:50 +0000 Subject: [PATCH 29/50] Update frame/nomination-pools/src/tests.rs --- frame/nomination-pools/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 4c3747090127f..5edcfaa0c52b9 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -2101,7 +2101,7 @@ mod unbond { assert_ok!(Pools::unbond(RuntimeOrigin::signed(20), 20, 15)); assert_eq!(PoolMembers::::get(20).unwrap().active_points(), 0); assert_eq!(PoolMembers::::get(20).unwrap().unbonding_points(), 20); - assert_eq!(RewardClaimPermission::::get(20), RewardClaim::Permissioned); + assert_eq!(ClaimPermissions::::get(20), ClaimPermission::Permissioned); }) } From 04d6b13a4857429c159aaba8dc7906ef5de9e23a Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Mon, 20 Feb 2023 12:59:02 +0000 Subject: [PATCH 30/50] Update frame/nomination-pools/src/lib.rs --- frame/nomination-pools/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 534fc400719ac..c64dd9d23b668 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -2150,12 +2150,12 @@ pub mod pallet { /// * `actor` - Account to claim reward. // improve this #[pallet::call_index(15)] #[pallet::weight(T::DbWeight::get().reads_writes(1, 1))] - pub fn set_reward_claim(origin: OriginFor, actor: RewardClaim) -> DispatchResult { + pub fn set_reward_claim(origin: OriginFor, permission: ClaimPermission) -> DispatchResult { let who = ensure_signed(origin)?; ensure!(PoolMembers::::contains_key(&who), Error::::PoolMemberNotFound); - >::mutate(who, |source| { - *source = actor; + >::mutate(who, |source| { + *source = permission; }); Ok(()) } From b8be98c949c6d781f2272393daa25e48c89fa2d7 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Mon, 20 Feb 2023 12:59:12 +0000 Subject: [PATCH 31/50] Update frame/nomination-pools/src/tests.rs --- frame/nomination-pools/src/tests.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 5edcfaa0c52b9..3813ea568947c 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -4269,15 +4269,15 @@ fn set_claimable_actor_works() { ); // Make permissionless - assert_eq!(RewardClaimPermission::::get(11), RewardClaim::Permissioned); + assert_eq!(ClaimPermissions::::get(11), ClaimPermission::Permissioned); assert_noop!( - Pools::set_reward_claim(RuntimeOrigin::signed(12), RewardClaim::Permissionless), + Pools::set_reward_claim(RuntimeOrigin::signed(12), ClaimPermission::Permissionless), Error::::PoolMemberNotFound ); - assert_ok!(Pools::set_reward_claim(RuntimeOrigin::signed(11), RewardClaim::Permissionless)); + assert_ok!(Pools::set_reward_claim(RuntimeOrigin::signed(11), ClaimPermission::Permissionless)); // then - assert_eq!(RewardClaimPermission::::get(11), RewardClaim::Permissionless); + assert_eq!(ClaimPermissions::::get(11), ClaimPermission::Permissionless); }); } From d8fac0eaa52eafa9cf79c41afcb6cfc328560c7f Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Mon, 20 Feb 2023 12:59:20 +0000 Subject: [PATCH 32/50] Update frame/nomination-pools/src/tests.rs --- frame/nomination-pools/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 3813ea568947c..da9eac5be7c55 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -4632,7 +4632,7 @@ mod bond_extra { Balances::make_free_balance_be(&default_reward_account(), 8); // ... if which only 3 is claimable to make sure the reward account does not die. let claimable_reward = 8 - ExistentialDeposit::get(); - // NOTE: easier to read of we use 3, so let's use the number instead of variable. + // NOTE: easier to read if we use 3, so let's use the number instead of variable. assert_eq!(claimable_reward, 3, "test is correct if rewards are divisible by 3"); // given From 79f9b7e1082716b72142aa38c87753c3fe04ac09 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Mon, 20 Feb 2023 12:59:29 +0000 Subject: [PATCH 33/50] Update frame/nomination-pools/src/tests.rs --- frame/nomination-pools/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index da9eac5be7c55..1edd3a7d1dbaf 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -4630,7 +4630,7 @@ mod bond_extra { fn bond_extra_other() { ExtBuilder::default().add_members(vec![(20, 20)]).build_and_execute(|| { Balances::make_free_balance_be(&default_reward_account(), 8); - // ... if which only 3 is claimable to make sure the reward account does not die. + // ... of which only 3 are claimable to make sure the reward account does not die. let claimable_reward = 8 - ExistentialDeposit::get(); // NOTE: easier to read if we use 3, so let's use the number instead of variable. assert_eq!(claimable_reward, 3, "test is correct if rewards are divisible by 3"); From 184d6f2e8cf1738b696c88d78f5d6e2cc30398db Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Mon, 20 Feb 2023 12:59:36 +0000 Subject: [PATCH 34/50] Update frame/nomination-pools/src/tests.rs --- frame/nomination-pools/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 1edd3a7d1dbaf..0e84980eb9e63 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -4650,7 +4650,7 @@ mod bond_extra { assert_ok!(Pools::set_reward_claim( RuntimeOrigin::signed(10), - RewardClaim::Permissionless + ClaimPermission::Permissionless )); assert_ok!(Pools::bond_extra_other(RuntimeOrigin::signed(50), 10, BondExtra::Rewards)); assert_eq!(Balances::free_balance(&default_reward_account()), 7); From 0e0cc6f981edde09b7d78db82f5ecf7dad83df52 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Mon, 20 Feb 2023 12:59:53 +0000 Subject: [PATCH 35/50] Update frame/nomination-pools/src/tests.rs --- frame/nomination-pools/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 0e84980eb9e63..7d9613d0985b2 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -2094,7 +2094,7 @@ mod unbond { assert_eq!(RewardClaimPermission::::get(10), RewardClaim::Permissioned); assert_ok!(Pools::set_reward_claim( RuntimeOrigin::signed(20), - RewardClaim::Permissionless + ClaimPermission::Permissionless )); // but can go to 0 From be3cc5e5619a3399431d2af494f5d986f9e7ee57 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Mon, 20 Feb 2023 13:00:05 +0000 Subject: [PATCH 36/50] Update frame/nomination-pools/src/lib.rs --- frame/nomination-pools/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index c64dd9d23b668..12d65dcf7e406 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -2468,7 +2468,7 @@ impl Pallet { ) -> DispatchResult { if signer != who { ensure!( - matches!(RewardClaimPermission::::get(&who), RewardClaim::Permissionless), + matches!(ClaimPermissions::::get(&who), ClaimPermission::Permissionless), Error::::DoesNotHavePermission ); ensure!(extra == BondExtra::Rewards, Error::::CannotBondFreeBalanceOther); From 768c63df319eca7b3149e729d36ef25b62a01fd0 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Mon, 20 Feb 2023 13:00:16 +0000 Subject: [PATCH 37/50] Update frame/nomination-pools/src/tests.rs --- frame/nomination-pools/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 7d9613d0985b2..1db6a0d253b15 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -2091,7 +2091,7 @@ mod unbond { ); // Make permissionless - assert_eq!(RewardClaimPermission::::get(10), RewardClaim::Permissioned); + assert_eq!(ClaimPermissions::::get(10), ClaimPermission::Permissioned); assert_ok!(Pools::set_reward_claim( RuntimeOrigin::signed(20), ClaimPermission::Permissionless From 549a743dc11b790876f89855722dfb3236e50903 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Mon, 20 Feb 2023 20:03:01 +0700 Subject: [PATCH 38/50] fixes + fmt --- .../nomination-pools/benchmarking/src/lib.rs | 11 +++++----- frame/nomination-pools/src/lib.rs | 21 ++++++++++++------- frame/nomination-pools/src/tests.rs | 5 ++++- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index e871c5939f5fc..ccb2fa14aa6ff 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -30,10 +30,9 @@ use frame_election_provider_support::SortedListProvider; use frame_support::{assert_ok, ensure, traits::Get}; use frame_system::RawOrigin as RuntimeOrigin; use pallet_nomination_pools::{ - BalanceOf, BondExtra, BondedPoolInner, BondedPools, ConfigOp, MaxPoolMembers, - MaxPoolMembersPerPool, MaxPools, Metadata, MinCreateBond, MinJoinBond, Pallet as Pools, - PoolMembers, PoolRoles, PoolState, RewardClaim, RewardClaimPermission, RewardPools, - SubPoolsStorage, + BalanceOf, BondExtra, BondedPoolInner, BondedPools, ClaimPermission, ClaimPermissions, + ConfigOp, MaxPoolMembers, MaxPoolMembersPerPool, MaxPools, Metadata, MinCreateBond, + MinJoinBond, Pallet as Pools, PoolMembers, PoolRoles, PoolState, RewardPools, SubPoolsStorage, }; use sp_runtime::traits::{Bounded, StaticLookup, Zero}; use sp_staking::{EraIndex, StakingInterface}; @@ -672,9 +671,9 @@ frame_benchmarking::benchmarks! { T::Staking::active_stake(&pool_account).unwrap(), min_create_bond + min_join_bond ); - }:_(RuntimeOrigin::Signed(joiner.clone()), RewardClaim::Permissionless) + }:_(RuntimeOrigin::Signed(joiner.clone()), ClaimPermission::Permissionless) verify { - assert_eq!(RewardClaimPermission::::get(joiner), RewardClaim::Permissionless); + assert_eq!(ClaimPermissions::::get(joiner), ClaimPermission::Permissionless); } impl_benchmark_test_suite!( diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 12d65dcf7e406..60eeb08c6b7ff 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -413,7 +413,7 @@ enum AccountType { /// Account to bond pending reward of a member. #[derive(Encode, Decode, MaxEncodedLen, Clone, Copy, Debug, PartialEq, Eq, TypeInfo)] -pub enum RewardClaim { +pub enum ClaimPermission { /// Only the pool member themself can claim their reward. Permissioned, /// Anyone can claim the reward of this member. @@ -1334,7 +1334,7 @@ pub mod pallet { /// Map from a pool member account to their preference regarding reward payout. #[pallet::storage] pub type ClaimPermissions = - StorageMap<_, Twox64Concat, T::AccountId, RewardClaim, ValueQuery>; + StorageMap<_, Twox64Concat, T::AccountId, ClaimPermission, ValueQuery>; #[pallet::genesis_config] pub struct GenesisConfig { @@ -2116,12 +2116,14 @@ pub mod pallet { T::Staking::chill(&bonded_pool.bonded_account()) } - /// `origin` bonds funds from `extra` for some pool member `member` into their respective pools. + /// `origin` bonds funds from `extra` for some pool member `member` into their respective + /// pools. /// - /// `origin` can bond extra funds from free balance or pending rewards when `origin == other`. + /// `origin` can bond extra funds from free balance or pending rewards when `origin == + /// other`. /// - /// In the case of `origin != other`, `origin` can only bond extra pending rewards of `other` - /// members assuming set_reward_claim for the given member is `Permissionless`. + /// In the case of `origin != other`, `origin` can only bond extra pending rewards of + /// `other` members assuming set_reward_claim for the given member is `Permissionless`. #[pallet::call_index(14)] #[pallet::weight( T::WeightInfo::bond_extra_transfer() @@ -2150,7 +2152,10 @@ pub mod pallet { /// * `actor` - Account to claim reward. // improve this #[pallet::call_index(15)] #[pallet::weight(T::DbWeight::get().reads_writes(1, 1))] - pub fn set_reward_claim(origin: OriginFor, permission: ClaimPermission) -> DispatchResult { + pub fn set_reward_claim( + origin: OriginFor, + permission: ClaimPermission, + ) -> DispatchResult { let who = ensure_signed(origin)?; ensure!(PoolMembers::::contains_key(&who), Error::::PoolMemberNotFound); @@ -2471,7 +2476,7 @@ impl Pallet { matches!(ClaimPermissions::::get(&who), ClaimPermission::Permissionless), Error::::DoesNotHavePermission ); - ensure!(extra == BondExtra::Rewards, Error::::CannotBondFreeBalanceOther); + ensure!(extra == BondExtra::Rewards, Error::::BondExtraRestricted); } let (mut member, mut bonded_pool, mut reward_pool) = Self::get_member_with_pools(&who)?; diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 1db6a0d253b15..08847516b9eae 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -4274,7 +4274,10 @@ fn set_claimable_actor_works() { Pools::set_reward_claim(RuntimeOrigin::signed(12), ClaimPermission::Permissionless), Error::::PoolMemberNotFound ); - assert_ok!(Pools::set_reward_claim(RuntimeOrigin::signed(11), ClaimPermission::Permissionless)); + assert_ok!(Pools::set_reward_claim( + RuntimeOrigin::signed(11), + ClaimPermission::Permissionless + )); // then assert_eq!(ClaimPermissions::::get(11), ClaimPermission::Permissionless); From 1c17bcc00b282c9d9c3c3cb006f89f73b4c15650 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 21 Feb 2023 11:52:57 +0700 Subject: [PATCH 39/50] expand ClaimPermissions + add benchmarks --- .../nomination-pools/benchmarking/src/lib.rs | 42 ++- frame/nomination-pools/src/lib.rs | 58 ++- frame/nomination-pools/src/tests.rs | 10 +- frame/nomination-pools/src/weights.rs | 342 +++++++++++------- 4 files changed, 305 insertions(+), 147 deletions(-) diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index ccb2fa14aa6ff..ae11d4de615cc 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -252,17 +252,22 @@ frame_benchmarking::benchmarks! { ); } - bond_extra_reward { + bond_extra_other { + let claimer: T::AccountId = account("claimer", USER_SEED + 4, 0); + let origin_weight = Pools::::depositor_min_bond() * 2u32.into(); let scenario = ListScenario::::new(origin_weight, true)?; let extra = (scenario.dest_weight - origin_weight).max(CurrencyOf::::minimum_balance()); + // set claim preferences to `PermissionlessAll` to any account to bond extra on member's behalf. + let _ = Pools::::set_reward_claim(RuntimeOrigin::Signed(scenario.creator1.clone()).into(), ClaimPermission::PermissionlessAll); + // transfer exactly `extra` to the depositor of the src pool (1), let reward_account1 = Pools::::create_reward_account(1); assert!(extra >= CurrencyOf::::minimum_balance()); CurrencyOf::::deposit_creating(&reward_account1, extra); - }: bond_extra(RuntimeOrigin::Signed(scenario.creator1.clone()), BondExtra::Rewards) + }: _(RuntimeOrigin::Signed(claimer), T::Lookup::unlookup(scenario.creator1.clone()), BondExtra::Rewards) verify { assert!( T::Staking::active_stake(&scenario.origin1).unwrap() >= @@ -298,6 +303,35 @@ frame_benchmarking::benchmarks! { ); } + claim_payout_other { + let claimer: T::AccountId = account("claimer", USER_SEED + 4, 0); + + let origin_weight = Pools::::depositor_min_bond() * 2u32.into(); + let ed = CurrencyOf::::minimum_balance(); + let (depositor, pool_account) = create_pool_account::(0, origin_weight); + let reward_account = Pools::::create_reward_account(1); + + // Send funds to the reward account of the pool + CurrencyOf::::make_free_balance_be(&reward_account, ed + origin_weight); + + // set claim preferences to `PermissionlessAll` so any account can claim rewards on member's + // behalf. + let _ = Pools::::set_reward_claim(RuntimeOrigin::Signed(depositor.clone()).into(), ClaimPermission::PermissionlessAll); + + whitelist_account!(depositor); + }:_(RuntimeOrigin::Signed(claimer), depositor.clone()) + verify { + assert_eq!( + CurrencyOf::::free_balance(&depositor), + origin_weight * 2u32.into() + ); + assert_eq!( + CurrencyOf::::free_balance(&reward_account), + ed + Zero::zero() + ); + } + + unbond { // The weight the nominator will start at. The value used here is expected to be // significantly higher than the first position in a list (e.g. the first bag threshold). @@ -671,9 +705,9 @@ frame_benchmarking::benchmarks! { T::Staking::active_stake(&pool_account).unwrap(), min_create_bond + min_join_bond ); - }:_(RuntimeOrigin::Signed(joiner.clone()), ClaimPermission::Permissionless) + }:_(RuntimeOrigin::Signed(joiner.clone()), ClaimPermission::PermissionlessAll) verify { - assert_eq!(ClaimPermissions::::get(joiner), ClaimPermission::Permissionless); + assert_eq!(ClaimPermissions::::get(joiner), ClaimPermission::PermissionlessAll); } impl_benchmark_test_suite!( diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 60eeb08c6b7ff..9dc86541e2b82 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -416,8 +416,12 @@ enum AccountType { pub enum ClaimPermission { /// Only the pool member themself can claim their reward. Permissioned, - /// Anyone can claim the reward of this member. - Permissionless, + /// Anyone can compound rewards on a pool member's behalf. + PermissionlessCompound, + /// Anyone can withdraw rewards on a pool member's behalf. + PermissionlessWithdraw, + /// Anyone can withdraw and compound rewards on a member's behalf. + PermissionlessAll, } impl Default for ClaimPermission { @@ -1592,11 +1596,10 @@ pub mod pallet { #[pallet::call_index(1)] #[pallet::weight( T::WeightInfo::bond_extra_transfer() - .max(T::WeightInfo::bond_extra_reward()) + .max(T::WeightInfo::bond_extra_other()) )] pub fn bond_extra(origin: OriginFor, extra: BondExtra>) -> DispatchResult { let who = ensure_signed(origin)?; - Self::do_bond_extra(who.clone(), who, extra) } @@ -1609,13 +1612,8 @@ pub mod pallet { #[pallet::call_index(2)] #[pallet::weight(T::WeightInfo::claim_payout())] pub fn claim_payout(origin: OriginFor) -> DispatchResult { - let who = ensure_signed(origin)?; - let (mut member, mut bonded_pool, mut reward_pool) = Self::get_member_with_pools(&who)?; - - let _ = Self::do_reward_payout(&who, &mut member, &mut bonded_pool, &mut reward_pool)?; - - Self::put_member_with_pools(&who, member, bonded_pool, reward_pool); - Ok(()) + let signer = ensure_signed(origin)?; + Self::do_claim_payout(signer.clone(), signer) } /// Unbond up to `unbonding_points` of the `member_account`'s funds from the pool. It @@ -2123,11 +2121,11 @@ pub mod pallet { /// other`. /// /// In the case of `origin != other`, `origin` can only bond extra pending rewards of - /// `other` members assuming set_reward_claim for the given member is `Permissionless`. + /// `other` members assuming set_reward_claim for the given member is `PermissionlessAll`. #[pallet::call_index(14)] #[pallet::weight( T::WeightInfo::bond_extra_transfer() - .max(T::WeightInfo::bond_extra_reward()) + .max(T::WeightInfo::bond_extra_other()) )] pub fn bond_extra_other( origin: OriginFor, @@ -2135,7 +2133,6 @@ pub mod pallet { extra: BondExtra>, ) -> DispatchResult { let who = ensure_signed(origin)?; - Self::do_bond_extra(who, T::Lookup::lookup(member)?, extra) } @@ -2143,7 +2140,7 @@ pub mod pallet { /// /// Lets a pool member to choose who can claim pending rewards on their behalf. By default, /// this is `Permissioned` which implies only the pool member themselves can claim their - /// pending rewards. If a pool member wishes so, they can set this to `Permissionless` to + /// pending rewards. If a pool member wishes so, they can set this to `PermissionlessAll` to /// allow any account to claim their rewards and bond extra to the pool. /// /// # Arguments @@ -2164,6 +2161,14 @@ pub mod pallet { }); Ok(()) } + + /// `origin` can claim payouts on some pool member `member`'s behalf. + #[pallet::call_index(16)] + #[pallet::weight(T::WeightInfo::claim_payout_other())] + pub fn claim_payout_other(origin: OriginFor, who: T::AccountId) -> DispatchResult { + let signer = ensure_signed(origin)?; + Self::do_claim_payout(signer, who) + } } #[pallet::hooks] @@ -2473,7 +2478,10 @@ impl Pallet { ) -> DispatchResult { if signer != who { ensure!( - matches!(ClaimPermissions::::get(&who), ClaimPermission::Permissionless), + matches!( + ClaimPermissions::::get(&who), + ClaimPermission::PermissionlessAll | ClaimPermission::PermissionlessCompound + ), Error::::DoesNotHavePermission ); ensure!(extra == BondExtra::Rewards, Error::::BondExtraRestricted); @@ -2509,6 +2517,24 @@ impl Pallet { Ok(()) } + fn do_claim_payout(signer: T::AccountId, who: T::AccountId) -> DispatchResult { + if signer != who { + ensure!( + matches!( + ClaimPermissions::::get(&who), + ClaimPermission::PermissionlessAll | ClaimPermission::PermissionlessWithdraw + ), + Error::::DoesNotHavePermission + ); + } + let (mut member, mut bonded_pool, mut reward_pool) = Self::get_member_with_pools(&who)?; + + let _ = Self::do_reward_payout(&who, &mut member, &mut bonded_pool, &mut reward_pool)?; + + Self::put_member_with_pools(&who, member, bonded_pool, reward_pool); + Ok(()) + } + /// Ensure the correctness of the state of this pallet. /// /// This should be valid before or after each state transition of this pallet. diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 08847516b9eae..ae3b15023d55c 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -2094,7 +2094,7 @@ mod unbond { assert_eq!(ClaimPermissions::::get(10), ClaimPermission::Permissioned); assert_ok!(Pools::set_reward_claim( RuntimeOrigin::signed(20), - ClaimPermission::Permissionless + ClaimPermission::PermissionlessAll )); // but can go to 0 @@ -4271,16 +4271,16 @@ fn set_claimable_actor_works() { // Make permissionless assert_eq!(ClaimPermissions::::get(11), ClaimPermission::Permissioned); assert_noop!( - Pools::set_reward_claim(RuntimeOrigin::signed(12), ClaimPermission::Permissionless), + Pools::set_reward_claim(RuntimeOrigin::signed(12), ClaimPermission::PermissionlessAll), Error::::PoolMemberNotFound ); assert_ok!(Pools::set_reward_claim( RuntimeOrigin::signed(11), - ClaimPermission::Permissionless + ClaimPermission::PermissionlessAll )); // then - assert_eq!(ClaimPermissions::::get(11), ClaimPermission::Permissionless); + assert_eq!(ClaimPermissions::::get(11), ClaimPermission::PermissionlessAll); }); } @@ -4653,7 +4653,7 @@ mod bond_extra { assert_ok!(Pools::set_reward_claim( RuntimeOrigin::signed(10), - ClaimPermission::Permissionless + ClaimPermission::PermissionlessAll )); assert_ok!(Pools::bond_extra_other(RuntimeOrigin::signed(50), 10, BondExtra::Rewards)); assert_eq!(Balances::free_balance(&default_reward_account()), 7); diff --git a/frame/nomination-pools/src/weights.rs b/frame/nomination-pools/src/weights.rs index ae73015940f80..dc1e350c54709 100644 --- a/frame/nomination-pools/src/weights.rs +++ b/frame/nomination-pools/src/weights.rs @@ -1,43 +1,33 @@ -// This file is part of Substrate. - -// Copyright (C) 2023 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. //! Autogenerated weights for pallet_nomination_pools //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev -//! DATE: 2023-01-24, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2023-02-21, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` //! WORST CASE MAP SIZE: `1000000` -//! HOSTNAME: `bm2`, CPU: `Intel(R) Core(TM) i7-7700K CPU @ 4.20GHz` -//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024 +//! HOSTNAME: `Ross`, CPU: `` +//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: None, DB CACHE: 1024 // Executed Command: -// ./target/production/substrate +// target/release/substrate // benchmark // pallet -// --chain=dev -// --steps=50 -// --repeat=20 -// --pallet=pallet_nomination_pools -// --extrinsic=* -// --execution=wasm -// --wasm-execution=compiled -// --heap-pages=4096 -// --output=./frame/nomination-pools/src/weights.rs -// --header=./HEADER-APACHE2 -// --template=./.maintain/frame-weight-template.hbs +// --execution +// wasm +// --wasm-execution +// compiled +// --dev +// --pallet +// pallet-nomination-pools +// --extrinsic +// * +// --steps +// 50 +// --repeat +// 20 +// --output +// frame/nomination-pools/src/weights.rs +// --template +// .maintain/frame-weight-template.hbs #![cfg_attr(rustfmt, rustfmt_skip)] #![allow(unused_parens)] @@ -50,8 +40,9 @@ use sp_std::marker::PhantomData; pub trait WeightInfo { fn join() -> Weight; fn bond_extra_transfer() -> Weight; - fn bond_extra_reward() -> Weight; + fn bond_extra_other() -> Weight; fn claim_payout() -> Weight; + fn claim_payout_other() -> Weight; fn unbond() -> Weight; fn pool_withdraw_unbonded(s: u32, ) -> Weight; fn withdraw_unbonded_update(s: u32, ) -> Weight; @@ -63,6 +54,7 @@ pub trait WeightInfo { fn set_configs() -> Weight; fn update_roles() -> Weight; fn chill() -> Weight; + fn set_reward_claim() -> Weight; } /// Weights for pallet_nomination_pools using the Substrate node and recommended hardware. @@ -98,8 +90,9 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `3573` // Estimated: `37988` - // Minimum execution time: 140_155 nanoseconds. - Weight::from_parts(141_098_000, 37988) + // Minimum execution time: 162_000 nanoseconds. + Weight::from_ref_time(164_000_000) + .saturating_add(Weight::from_proof_size(37988)) .saturating_add(T::DbWeight::get().reads(17_u64)) .saturating_add(T::DbWeight::get().writes(12_u64)) } @@ -125,11 +118,14 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `3615` // Estimated: `38583` - // Minimum execution time: 136_048 nanoseconds. - Weight::from_parts(136_767_000, 38583) + // Minimum execution time: 156_000 nanoseconds. + Weight::from_ref_time(159_000_000) + .saturating_add(Weight::from_proof_size(38583)) .saturating_add(T::DbWeight::get().reads(14_u64)) .saturating_add(T::DbWeight::get().writes(12_u64)) } + /// Storage: NominationPools ClaimPermissions (r:1 w:0) + /// Proof: NominationPools ClaimPermissions (max_values: None, max_size: Some(41), added: 2516, mode: MaxEncodedLen) /// Storage: NominationPools PoolMembers (r:1 w:1) /// Proof: NominationPools PoolMembers (max_values: None, max_size: Some(237), added: 2712, mode: MaxEncodedLen) /// Storage: NominationPools BondedPools (r:1 w:1) @@ -148,13 +144,14 @@ impl WeightInfo for SubstrateWeight { /// Proof: VoterList ListNodes (max_values: None, max_size: Some(154), added: 2629, mode: MaxEncodedLen) /// Storage: VoterList ListBags (r:2 w:2) /// Proof: VoterList ListBags (max_values: None, max_size: Some(82), added: 2557, mode: MaxEncodedLen) - fn bond_extra_reward() -> Weight { + fn bond_extra_other() -> Weight { // Proof Size summary in bytes: - // Measured: `3615` - // Estimated: `38583` - // Minimum execution time: 151_473 nanoseconds. - Weight::from_parts(152_375_000, 38583) - .saturating_add(T::DbWeight::get().reads(14_u64)) + // Measured: `3680` + // Estimated: `41099` + // Minimum execution time: 184_000 nanoseconds. + Weight::from_ref_time(188_000_000) + .saturating_add(Weight::from_proof_size(41099)) + .saturating_add(T::DbWeight::get().reads(15_u64)) .saturating_add(T::DbWeight::get().writes(13_u64)) } /// Storage: NominationPools PoolMembers (r:1 w:1) @@ -169,11 +166,32 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `1189` // Estimated: `10489` - // Minimum execution time: 51_146 nanoseconds. - Weight::from_parts(51_570_000, 10489) + // Minimum execution time: 55_000 nanoseconds. + Weight::from_ref_time(57_000_000) + .saturating_add(Weight::from_proof_size(10489)) .saturating_add(T::DbWeight::get().reads(4_u64)) .saturating_add(T::DbWeight::get().writes(4_u64)) } + /// Storage: NominationPools ClaimPermissions (r:1 w:0) + /// Proof: NominationPools ClaimPermissions (max_values: None, max_size: Some(41), added: 2516, mode: MaxEncodedLen) + /// Storage: NominationPools PoolMembers (r:1 w:1) + /// Proof: NominationPools PoolMembers (max_values: None, max_size: Some(237), added: 2712, mode: MaxEncodedLen) + /// Storage: NominationPools BondedPools (r:1 w:1) + /// Proof: NominationPools BondedPools (max_values: None, max_size: Some(164), added: 2639, mode: MaxEncodedLen) + /// Storage: NominationPools RewardPools (r:1 w:1) + /// Proof: NominationPools RewardPools (max_values: None, max_size: Some(60), added: 2535, mode: MaxEncodedLen) + /// Storage: System Account (r:1 w:1) + /// Proof: System Account (max_values: None, max_size: Some(128), added: 2603, mode: MaxEncodedLen) + fn claim_payout_other() -> Weight { + // Proof Size summary in bytes: + // Measured: `1254` + // Estimated: `13005` + // Minimum execution time: 59_000 nanoseconds. + Weight::from_ref_time(60_000_000) + .saturating_add(Weight::from_proof_size(13005)) + .saturating_add(T::DbWeight::get().reads(5_u64)) + .saturating_add(T::DbWeight::get().writes(4_u64)) + } /// Storage: NominationPools PoolMembers (r:1 w:1) /// Proof: NominationPools PoolMembers (max_values: None, max_size: Some(237), added: 2712, mode: MaxEncodedLen) /// Storage: NominationPools BondedPools (r:1 w:1) @@ -202,14 +220,17 @@ impl WeightInfo for SubstrateWeight { /// Proof: NominationPools SubPoolsStorage (max_values: None, max_size: Some(24382), added: 26857, mode: MaxEncodedLen) /// Storage: NominationPools CounterForSubPoolsStorage (r:1 w:1) /// Proof: NominationPools CounterForSubPoolsStorage (max_values: Some(1), max_size: Some(4), added: 499, mode: MaxEncodedLen) + /// Storage: NominationPools ClaimPermissions (r:0 w:1) + /// Proof: NominationPools ClaimPermissions (max_values: None, max_size: Some(41), added: 2516, mode: MaxEncodedLen) fn unbond() -> Weight { // Proof Size summary in bytes: // Measured: `3858` // Estimated: `67379` - // Minimum execution time: 141_400 nanoseconds. - Weight::from_parts(141_822_000, 67379) + // Minimum execution time: 162_000 nanoseconds. + Weight::from_ref_time(164_000_000) + .saturating_add(Weight::from_proof_size(67379)) .saturating_add(T::DbWeight::get().reads(18_u64)) - .saturating_add(T::DbWeight::get().writes(13_u64)) + .saturating_add(T::DbWeight::get().writes(14_u64)) } /// Storage: NominationPools BondedPools (r:1 w:0) /// Proof: NominationPools BondedPools (max_values: None, max_size: Some(164), added: 2639, mode: MaxEncodedLen) @@ -222,14 +243,13 @@ impl WeightInfo for SubstrateWeight { /// Storage: Balances Locks (r:1 w:1) /// Proof: Balances Locks (max_values: None, max_size: Some(1299), added: 3774, mode: MaxEncodedLen) /// The range of component `s` is `[0, 100]`. - fn pool_withdraw_unbonded(s: u32, ) -> Weight { + fn pool_withdraw_unbonded(_s: u32, ) -> Weight { // Proof Size summary in bytes: // Measured: `1779` // Estimated: `13025` - // Minimum execution time: 49_021 nanoseconds. - Weight::from_parts(49_954_282, 13025) - // Standard Error: 378 - .saturating_add(Weight::from_ref_time(5_165).saturating_mul(s.into())) + // Minimum execution time: 50_000 nanoseconds. + Weight::from_ref_time(52_796_229) + .saturating_add(Weight::from_proof_size(13025)) .saturating_add(T::DbWeight::get().reads(5_u64)) .saturating_add(T::DbWeight::get().writes(2_u64)) } @@ -256,10 +276,11 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `2303` // Estimated: `45696` - // Minimum execution time: 92_473 nanoseconds. - Weight::from_parts(93_901_972, 45696) - // Standard Error: 618 - .saturating_add(Weight::from_ref_time(12_032).saturating_mul(s.into())) + // Minimum execution time: 101_000 nanoseconds. + Weight::from_ref_time(104_858_516) + .saturating_add(Weight::from_proof_size(45696)) + // Standard Error: 1_643 + .saturating_add(Weight::from_ref_time(7_299).saturating_mul(s.into())) .saturating_add(T::DbWeight::get().reads(9_u64)) .saturating_add(T::DbWeight::get().writes(7_u64)) } @@ -304,12 +325,15 @@ impl WeightInfo for SubstrateWeight { /// Storage: Staking Payee (r:0 w:1) /// Proof: Staking Payee (max_values: None, max_size: Some(73), added: 2548, mode: MaxEncodedLen) /// The range of component `s` is `[0, 100]`. - fn withdraw_unbonded_kill(_s: u32, ) -> Weight { + fn withdraw_unbonded_kill(s: u32, ) -> Weight { // Proof Size summary in bytes: // Measured: `2690` // Estimated: `68812` - // Minimum execution time: 150_063 nanoseconds. - Weight::from_parts(152_321_387, 68812) + // Minimum execution time: 167_000 nanoseconds. + Weight::from_ref_time(172_584_557) + .saturating_add(Weight::from_proof_size(68812)) + // Standard Error: 3_095 + .saturating_add(Weight::from_ref_time(17_153).saturating_mul(s.into())) .saturating_add(T::DbWeight::get().reads(20_u64)) .saturating_add(T::DbWeight::get().writes(17_u64)) } @@ -359,8 +383,9 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `1321` // Estimated: `31522` - // Minimum execution time: 131_430 nanoseconds. - Weight::from_parts(132_214_000, 31522) + // Minimum execution time: 149_000 nanoseconds. + Weight::from_ref_time(151_000_000) + .saturating_add(Weight::from_proof_size(31522)) .saturating_add(T::DbWeight::get().reads(21_u64)) .saturating_add(T::DbWeight::get().writes(15_u64)) } @@ -393,10 +418,11 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `1909` // Estimated: `21998 + n * (2520 ±0)` - // Minimum execution time: 61_798 nanoseconds. - Weight::from_parts(61_504_758, 21998) - // Standard Error: 4_046 - .saturating_add(Weight::from_ref_time(1_159_175).saturating_mul(n.into())) + // Minimum execution time: 64_000 nanoseconds. + Weight::from_ref_time(65_318_309) + .saturating_add(Weight::from_proof_size(21998)) + // Standard Error: 9_189 + .saturating_add(Weight::from_ref_time(1_618_914).saturating_mul(n.into())) .saturating_add(T::DbWeight::get().reads(12_u64)) .saturating_add(T::DbWeight::get().reads((1_u64).saturating_mul(n.into()))) .saturating_add(T::DbWeight::get().writes(5_u64)) @@ -412,8 +438,9 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `1498` // Estimated: `8752` - // Minimum execution time: 32_433 nanoseconds. - Weight::from_parts(32_894_000, 8752) + // Minimum execution time: 33_000 nanoseconds. + Weight::from_ref_time(34_000_000) + .saturating_add(Weight::from_proof_size(8752)) .saturating_add(T::DbWeight::get().reads(3_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } @@ -428,10 +455,11 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `559` // Estimated: `5883` - // Minimum execution time: 13_608 nanoseconds. - Weight::from_parts(13_966_346, 5883) - // Standard Error: 44 - .saturating_add(Weight::from_ref_time(1_511).saturating_mul(n.into())) + // Minimum execution time: 13_000 nanoseconds. + Weight::from_ref_time(13_864_041) + .saturating_add(Weight::from_proof_size(5883)) + // Standard Error: 198 + .saturating_add(Weight::from_ref_time(312).saturating_mul(n.into())) .saturating_add(T::DbWeight::get().reads(3_u64)) .saturating_add(T::DbWeight::get().writes(2_u64)) } @@ -449,8 +477,9 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `0` // Estimated: `0` - // Minimum execution time: 5_832 nanoseconds. - Weight::from_ref_time(6_117_000) + // Minimum execution time: 6_000 nanoseconds. + Weight::from_ref_time(6_000_000) + .saturating_add(Weight::from_proof_size(0)) .saturating_add(T::DbWeight::get().writes(5_u64)) } /// Storage: NominationPools BondedPools (r:1 w:1) @@ -459,8 +488,9 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `559` // Estimated: `2639` - // Minimum execution time: 18_160 nanoseconds. - Weight::from_parts(18_567_000, 2639) + // Minimum execution time: 17_000 nanoseconds. + Weight::from_ref_time(18_000_000) + .saturating_add(Weight::from_proof_size(2639)) .saturating_add(T::DbWeight::get().reads(1_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } @@ -486,11 +516,26 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `2136` // Estimated: `20489` - // Minimum execution time: 58_991 nanoseconds. - Weight::from_parts(59_528_000, 20489) + // Minimum execution time: 62_000 nanoseconds. + Weight::from_ref_time(64_000_000) + .saturating_add(Weight::from_proof_size(20489)) .saturating_add(T::DbWeight::get().reads(9_u64)) .saturating_add(T::DbWeight::get().writes(5_u64)) } + /// Storage: NominationPools PoolMembers (r:1 w:0) + /// Proof: NominationPools PoolMembers (max_values: None, max_size: Some(237), added: 2712, mode: MaxEncodedLen) + /// Storage: NominationPools ClaimPermissions (r:1 w:1) + /// Proof: NominationPools ClaimPermissions (max_values: None, max_size: Some(41), added: 2516, mode: MaxEncodedLen) + fn set_reward_claim() -> Weight { + // Proof Size summary in bytes: + // Measured: `542` + // Estimated: `5228` + // Minimum execution time: 13_000 nanoseconds. + Weight::from_ref_time(14_000_000) + .saturating_add(Weight::from_proof_size(5228)) + .saturating_add(T::DbWeight::get().reads(2_u64)) + .saturating_add(T::DbWeight::get().writes(1_u64)) + } } // For backwards compatibility and tests @@ -525,8 +570,9 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `3573` // Estimated: `37988` - // Minimum execution time: 140_155 nanoseconds. - Weight::from_parts(141_098_000, 37988) + // Minimum execution time: 162_000 nanoseconds. + Weight::from_ref_time(164_000_000) + .saturating_add(Weight::from_proof_size(37988)) .saturating_add(RocksDbWeight::get().reads(17_u64)) .saturating_add(RocksDbWeight::get().writes(12_u64)) } @@ -552,11 +598,14 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `3615` // Estimated: `38583` - // Minimum execution time: 136_048 nanoseconds. - Weight::from_parts(136_767_000, 38583) + // Minimum execution time: 156_000 nanoseconds. + Weight::from_ref_time(159_000_000) + .saturating_add(Weight::from_proof_size(38583)) .saturating_add(RocksDbWeight::get().reads(14_u64)) .saturating_add(RocksDbWeight::get().writes(12_u64)) } + /// Storage: NominationPools ClaimPermissions (r:1 w:0) + /// Proof: NominationPools ClaimPermissions (max_values: None, max_size: Some(41), added: 2516, mode: MaxEncodedLen) /// Storage: NominationPools PoolMembers (r:1 w:1) /// Proof: NominationPools PoolMembers (max_values: None, max_size: Some(237), added: 2712, mode: MaxEncodedLen) /// Storage: NominationPools BondedPools (r:1 w:1) @@ -575,13 +624,14 @@ impl WeightInfo for () { /// Proof: VoterList ListNodes (max_values: None, max_size: Some(154), added: 2629, mode: MaxEncodedLen) /// Storage: VoterList ListBags (r:2 w:2) /// Proof: VoterList ListBags (max_values: None, max_size: Some(82), added: 2557, mode: MaxEncodedLen) - fn bond_extra_reward() -> Weight { + fn bond_extra_other() -> Weight { // Proof Size summary in bytes: - // Measured: `3615` - // Estimated: `38583` - // Minimum execution time: 151_473 nanoseconds. - Weight::from_parts(152_375_000, 38583) - .saturating_add(RocksDbWeight::get().reads(14_u64)) + // Measured: `3680` + // Estimated: `41099` + // Minimum execution time: 184_000 nanoseconds. + Weight::from_ref_time(188_000_000) + .saturating_add(Weight::from_proof_size(41099)) + .saturating_add(RocksDbWeight::get().reads(15_u64)) .saturating_add(RocksDbWeight::get().writes(13_u64)) } /// Storage: NominationPools PoolMembers (r:1 w:1) @@ -596,11 +646,32 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `1189` // Estimated: `10489` - // Minimum execution time: 51_146 nanoseconds. - Weight::from_parts(51_570_000, 10489) + // Minimum execution time: 55_000 nanoseconds. + Weight::from_ref_time(57_000_000) + .saturating_add(Weight::from_proof_size(10489)) .saturating_add(RocksDbWeight::get().reads(4_u64)) .saturating_add(RocksDbWeight::get().writes(4_u64)) } + /// Storage: NominationPools ClaimPermissions (r:1 w:0) + /// Proof: NominationPools ClaimPermissions (max_values: None, max_size: Some(41), added: 2516, mode: MaxEncodedLen) + /// Storage: NominationPools PoolMembers (r:1 w:1) + /// Proof: NominationPools PoolMembers (max_values: None, max_size: Some(237), added: 2712, mode: MaxEncodedLen) + /// Storage: NominationPools BondedPools (r:1 w:1) + /// Proof: NominationPools BondedPools (max_values: None, max_size: Some(164), added: 2639, mode: MaxEncodedLen) + /// Storage: NominationPools RewardPools (r:1 w:1) + /// Proof: NominationPools RewardPools (max_values: None, max_size: Some(60), added: 2535, mode: MaxEncodedLen) + /// Storage: System Account (r:1 w:1) + /// Proof: System Account (max_values: None, max_size: Some(128), added: 2603, mode: MaxEncodedLen) + fn claim_payout_other() -> Weight { + // Proof Size summary in bytes: + // Measured: `1254` + // Estimated: `13005` + // Minimum execution time: 59_000 nanoseconds. + Weight::from_ref_time(60_000_000) + .saturating_add(Weight::from_proof_size(13005)) + .saturating_add(RocksDbWeight::get().reads(5_u64)) + .saturating_add(RocksDbWeight::get().writes(4_u64)) + } /// Storage: NominationPools PoolMembers (r:1 w:1) /// Proof: NominationPools PoolMembers (max_values: None, max_size: Some(237), added: 2712, mode: MaxEncodedLen) /// Storage: NominationPools BondedPools (r:1 w:1) @@ -629,14 +700,17 @@ impl WeightInfo for () { /// Proof: NominationPools SubPoolsStorage (max_values: None, max_size: Some(24382), added: 26857, mode: MaxEncodedLen) /// Storage: NominationPools CounterForSubPoolsStorage (r:1 w:1) /// Proof: NominationPools CounterForSubPoolsStorage (max_values: Some(1), max_size: Some(4), added: 499, mode: MaxEncodedLen) + /// Storage: NominationPools ClaimPermissions (r:0 w:1) + /// Proof: NominationPools ClaimPermissions (max_values: None, max_size: Some(41), added: 2516, mode: MaxEncodedLen) fn unbond() -> Weight { // Proof Size summary in bytes: // Measured: `3858` // Estimated: `67379` - // Minimum execution time: 141_400 nanoseconds. - Weight::from_parts(141_822_000, 67379) + // Minimum execution time: 162_000 nanoseconds. + Weight::from_ref_time(164_000_000) + .saturating_add(Weight::from_proof_size(67379)) .saturating_add(RocksDbWeight::get().reads(18_u64)) - .saturating_add(RocksDbWeight::get().writes(13_u64)) + .saturating_add(RocksDbWeight::get().writes(14_u64)) } /// Storage: NominationPools BondedPools (r:1 w:0) /// Proof: NominationPools BondedPools (max_values: None, max_size: Some(164), added: 2639, mode: MaxEncodedLen) @@ -649,14 +723,13 @@ impl WeightInfo for () { /// Storage: Balances Locks (r:1 w:1) /// Proof: Balances Locks (max_values: None, max_size: Some(1299), added: 3774, mode: MaxEncodedLen) /// The range of component `s` is `[0, 100]`. - fn pool_withdraw_unbonded(s: u32, ) -> Weight { + fn pool_withdraw_unbonded(_s: u32, ) -> Weight { // Proof Size summary in bytes: // Measured: `1779` // Estimated: `13025` - // Minimum execution time: 49_021 nanoseconds. - Weight::from_parts(49_954_282, 13025) - // Standard Error: 378 - .saturating_add(Weight::from_ref_time(5_165).saturating_mul(s.into())) + // Minimum execution time: 50_000 nanoseconds. + Weight::from_ref_time(52_796_229) + .saturating_add(Weight::from_proof_size(13025)) .saturating_add(RocksDbWeight::get().reads(5_u64)) .saturating_add(RocksDbWeight::get().writes(2_u64)) } @@ -683,10 +756,11 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `2303` // Estimated: `45696` - // Minimum execution time: 92_473 nanoseconds. - Weight::from_parts(93_901_972, 45696) - // Standard Error: 618 - .saturating_add(Weight::from_ref_time(12_032).saturating_mul(s.into())) + // Minimum execution time: 101_000 nanoseconds. + Weight::from_ref_time(104_858_516) + .saturating_add(Weight::from_proof_size(45696)) + // Standard Error: 1_643 + .saturating_add(Weight::from_ref_time(7_299).saturating_mul(s.into())) .saturating_add(RocksDbWeight::get().reads(9_u64)) .saturating_add(RocksDbWeight::get().writes(7_u64)) } @@ -731,12 +805,15 @@ impl WeightInfo for () { /// Storage: Staking Payee (r:0 w:1) /// Proof: Staking Payee (max_values: None, max_size: Some(73), added: 2548, mode: MaxEncodedLen) /// The range of component `s` is `[0, 100]`. - fn withdraw_unbonded_kill(_s: u32, ) -> Weight { + fn withdraw_unbonded_kill(s: u32, ) -> Weight { // Proof Size summary in bytes: // Measured: `2690` // Estimated: `68812` - // Minimum execution time: 150_063 nanoseconds. - Weight::from_parts(152_321_387, 68812) + // Minimum execution time: 167_000 nanoseconds. + Weight::from_ref_time(172_584_557) + .saturating_add(Weight::from_proof_size(68812)) + // Standard Error: 3_095 + .saturating_add(Weight::from_ref_time(17_153).saturating_mul(s.into())) .saturating_add(RocksDbWeight::get().reads(20_u64)) .saturating_add(RocksDbWeight::get().writes(17_u64)) } @@ -786,8 +863,9 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `1321` // Estimated: `31522` - // Minimum execution time: 131_430 nanoseconds. - Weight::from_parts(132_214_000, 31522) + // Minimum execution time: 149_000 nanoseconds. + Weight::from_ref_time(151_000_000) + .saturating_add(Weight::from_proof_size(31522)) .saturating_add(RocksDbWeight::get().reads(21_u64)) .saturating_add(RocksDbWeight::get().writes(15_u64)) } @@ -820,10 +898,11 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `1909` // Estimated: `21998 + n * (2520 ±0)` - // Minimum execution time: 61_798 nanoseconds. - Weight::from_parts(61_504_758, 21998) - // Standard Error: 4_046 - .saturating_add(Weight::from_ref_time(1_159_175).saturating_mul(n.into())) + // Minimum execution time: 64_000 nanoseconds. + Weight::from_ref_time(65_318_309) + .saturating_add(Weight::from_proof_size(21998)) + // Standard Error: 9_189 + .saturating_add(Weight::from_ref_time(1_618_914).saturating_mul(n.into())) .saturating_add(RocksDbWeight::get().reads(12_u64)) .saturating_add(RocksDbWeight::get().reads((1_u64).saturating_mul(n.into()))) .saturating_add(RocksDbWeight::get().writes(5_u64)) @@ -839,8 +918,9 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `1498` // Estimated: `8752` - // Minimum execution time: 32_433 nanoseconds. - Weight::from_parts(32_894_000, 8752) + // Minimum execution time: 33_000 nanoseconds. + Weight::from_ref_time(34_000_000) + .saturating_add(Weight::from_proof_size(8752)) .saturating_add(RocksDbWeight::get().reads(3_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) } @@ -855,10 +935,11 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `559` // Estimated: `5883` - // Minimum execution time: 13_608 nanoseconds. - Weight::from_parts(13_966_346, 5883) - // Standard Error: 44 - .saturating_add(Weight::from_ref_time(1_511).saturating_mul(n.into())) + // Minimum execution time: 13_000 nanoseconds. + Weight::from_ref_time(13_864_041) + .saturating_add(Weight::from_proof_size(5883)) + // Standard Error: 198 + .saturating_add(Weight::from_ref_time(312).saturating_mul(n.into())) .saturating_add(RocksDbWeight::get().reads(3_u64)) .saturating_add(RocksDbWeight::get().writes(2_u64)) } @@ -876,8 +957,9 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `0` // Estimated: `0` - // Minimum execution time: 5_832 nanoseconds. - Weight::from_ref_time(6_117_000) + // Minimum execution time: 6_000 nanoseconds. + Weight::from_ref_time(6_000_000) + .saturating_add(Weight::from_proof_size(0)) .saturating_add(RocksDbWeight::get().writes(5_u64)) } /// Storage: NominationPools BondedPools (r:1 w:1) @@ -886,8 +968,9 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `559` // Estimated: `2639` - // Minimum execution time: 18_160 nanoseconds. - Weight::from_parts(18_567_000, 2639) + // Minimum execution time: 17_000 nanoseconds. + Weight::from_ref_time(18_000_000) + .saturating_add(Weight::from_proof_size(2639)) .saturating_add(RocksDbWeight::get().reads(1_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) } @@ -913,9 +996,24 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `2136` // Estimated: `20489` - // Minimum execution time: 58_991 nanoseconds. - Weight::from_parts(59_528_000, 20489) + // Minimum execution time: 62_000 nanoseconds. + Weight::from_ref_time(64_000_000) + .saturating_add(Weight::from_proof_size(20489)) .saturating_add(RocksDbWeight::get().reads(9_u64)) .saturating_add(RocksDbWeight::get().writes(5_u64)) } + /// Storage: NominationPools PoolMembers (r:1 w:0) + /// Proof: NominationPools PoolMembers (max_values: None, max_size: Some(237), added: 2712, mode: MaxEncodedLen) + /// Storage: NominationPools ClaimPermissions (r:1 w:1) + /// Proof: NominationPools ClaimPermissions (max_values: None, max_size: Some(41), added: 2516, mode: MaxEncodedLen) + fn set_reward_claim() -> Weight { + // Proof Size summary in bytes: + // Measured: `542` + // Estimated: `5228` + // Minimum execution time: 13_000 nanoseconds. + Weight::from_ref_time(14_000_000) + .saturating_add(Weight::from_proof_size(5228)) + .saturating_add(RocksDbWeight::get().reads(2_u64)) + .saturating_add(RocksDbWeight::get().writes(1_u64)) + } } From 9b1eb0057d04e6be628374471a96ce8b922f7444 Mon Sep 17 00:00:00 2001 From: command-bot <> Date: Tue, 21 Feb 2023 05:25:25 +0000 Subject: [PATCH 40/50] ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_nomination_pools --- frame/nomination-pools/src/weights.rs | 227 ++++++++++++++------------ 1 file changed, 119 insertions(+), 108 deletions(-) diff --git a/frame/nomination-pools/src/weights.rs b/frame/nomination-pools/src/weights.rs index dc1e350c54709..0031efb717dc5 100644 --- a/frame/nomination-pools/src/weights.rs +++ b/frame/nomination-pools/src/weights.rs @@ -1,33 +1,44 @@ +// This file is part of Substrate. + +// Copyright (C) 2023 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. //! Autogenerated weights for pallet_nomination_pools //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev //! DATE: 2023-02-21, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` //! WORST CASE MAP SIZE: `1000000` -//! HOSTNAME: `Ross`, CPU: `` -//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: None, DB CACHE: 1024 +//! HOSTNAME: `runner-ehxwxxsd-project-145-concurrent-0`, CPU: `Intel(R) Xeon(R) CPU @ 2.60GHz` +//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024 // Executed Command: -// target/release/substrate +// target/production/substrate // benchmark // pallet -// --execution -// wasm -// --wasm-execution -// compiled -// --dev -// --pallet -// pallet-nomination-pools -// --extrinsic -// * -// --steps -// 50 -// --repeat -// 20 -// --output -// frame/nomination-pools/src/weights.rs -// --template -// .maintain/frame-weight-template.hbs +// --steps=50 +// --repeat=20 +// --extrinsic=* +// --execution=wasm +// --wasm-execution=compiled +// --heap-pages=4096 +// --json-file=/builds/parity/mirrors/substrate/.git/.artifacts/bench.json +// --pallet=pallet_nomination_pools +// --chain=dev +// --header=./HEADER-APACHE2 +// --output=./frame/nomination-pools/src/weights.rs +// --template=./.maintain/frame-weight-template.hbs #![cfg_attr(rustfmt, rustfmt_skip)] #![allow(unused_parens)] @@ -90,8 +101,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `3573` // Estimated: `37988` - // Minimum execution time: 162_000 nanoseconds. - Weight::from_ref_time(164_000_000) + // Minimum execution time: 169_857 nanoseconds. + Weight::from_ref_time(173_895_000) .saturating_add(Weight::from_proof_size(37988)) .saturating_add(T::DbWeight::get().reads(17_u64)) .saturating_add(T::DbWeight::get().writes(12_u64)) @@ -118,8 +129,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `3615` // Estimated: `38583` - // Minimum execution time: 156_000 nanoseconds. - Weight::from_ref_time(159_000_000) + // Minimum execution time: 167_372 nanoseconds. + Weight::from_ref_time(168_776_000) .saturating_add(Weight::from_proof_size(38583)) .saturating_add(T::DbWeight::get().reads(14_u64)) .saturating_add(T::DbWeight::get().writes(12_u64)) @@ -148,8 +159,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `3680` // Estimated: `41099` - // Minimum execution time: 184_000 nanoseconds. - Weight::from_ref_time(188_000_000) + // Minimum execution time: 186_346 nanoseconds. + Weight::from_ref_time(191_308_000) .saturating_add(Weight::from_proof_size(41099)) .saturating_add(T::DbWeight::get().reads(15_u64)) .saturating_add(T::DbWeight::get().writes(13_u64)) @@ -166,8 +177,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `1189` // Estimated: `10489` - // Minimum execution time: 55_000 nanoseconds. - Weight::from_ref_time(57_000_000) + // Minimum execution time: 57_430 nanoseconds. + Weight::from_ref_time(59_161_000) .saturating_add(Weight::from_proof_size(10489)) .saturating_add(T::DbWeight::get().reads(4_u64)) .saturating_add(T::DbWeight::get().writes(4_u64)) @@ -186,8 +197,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `1254` // Estimated: `13005` - // Minimum execution time: 59_000 nanoseconds. - Weight::from_ref_time(60_000_000) + // Minimum execution time: 61_423 nanoseconds. + Weight::from_ref_time(63_219_000) .saturating_add(Weight::from_proof_size(13005)) .saturating_add(T::DbWeight::get().reads(5_u64)) .saturating_add(T::DbWeight::get().writes(4_u64)) @@ -226,8 +237,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `3858` // Estimated: `67379` - // Minimum execution time: 162_000 nanoseconds. - Weight::from_ref_time(164_000_000) + // Minimum execution time: 174_532 nanoseconds. + Weight::from_ref_time(180_032_000) .saturating_add(Weight::from_proof_size(67379)) .saturating_add(T::DbWeight::get().reads(18_u64)) .saturating_add(T::DbWeight::get().writes(14_u64)) @@ -243,13 +254,15 @@ impl WeightInfo for SubstrateWeight { /// Storage: Balances Locks (r:1 w:1) /// Proof: Balances Locks (max_values: None, max_size: Some(1299), added: 3774, mode: MaxEncodedLen) /// The range of component `s` is `[0, 100]`. - fn pool_withdraw_unbonded(_s: u32, ) -> Weight { + fn pool_withdraw_unbonded(s: u32, ) -> Weight { // Proof Size summary in bytes: // Measured: `1779` // Estimated: `13025` - // Minimum execution time: 50_000 nanoseconds. - Weight::from_ref_time(52_796_229) + // Minimum execution time: 55_327 nanoseconds. + Weight::from_ref_time(58_947_746) .saturating_add(Weight::from_proof_size(13025)) + // Standard Error: 1_589 + .saturating_add(Weight::from_ref_time(40_696).saturating_mul(s.into())) .saturating_add(T::DbWeight::get().reads(5_u64)) .saturating_add(T::DbWeight::get().writes(2_u64)) } @@ -276,11 +289,11 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `2303` // Estimated: `45696` - // Minimum execution time: 101_000 nanoseconds. - Weight::from_ref_time(104_858_516) + // Minimum execution time: 105_923 nanoseconds. + Weight::from_ref_time(110_572_476) .saturating_add(Weight::from_proof_size(45696)) - // Standard Error: 1_643 - .saturating_add(Weight::from_ref_time(7_299).saturating_mul(s.into())) + // Standard Error: 2_438 + .saturating_add(Weight::from_ref_time(69_045).saturating_mul(s.into())) .saturating_add(T::DbWeight::get().reads(9_u64)) .saturating_add(T::DbWeight::get().writes(7_u64)) } @@ -325,15 +338,13 @@ impl WeightInfo for SubstrateWeight { /// Storage: Staking Payee (r:0 w:1) /// Proof: Staking Payee (max_values: None, max_size: Some(73), added: 2548, mode: MaxEncodedLen) /// The range of component `s` is `[0, 100]`. - fn withdraw_unbonded_kill(s: u32, ) -> Weight { + fn withdraw_unbonded_kill(_s: u32, ) -> Weight { // Proof Size summary in bytes: // Measured: `2690` // Estimated: `68812` - // Minimum execution time: 167_000 nanoseconds. - Weight::from_ref_time(172_584_557) + // Minimum execution time: 169_700 nanoseconds. + Weight::from_ref_time(178_693_541) .saturating_add(Weight::from_proof_size(68812)) - // Standard Error: 3_095 - .saturating_add(Weight::from_ref_time(17_153).saturating_mul(s.into())) .saturating_add(T::DbWeight::get().reads(20_u64)) .saturating_add(T::DbWeight::get().writes(17_u64)) } @@ -383,8 +394,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `1321` // Estimated: `31522` - // Minimum execution time: 149_000 nanoseconds. - Weight::from_ref_time(151_000_000) + // Minimum execution time: 145_976 nanoseconds. + Weight::from_ref_time(150_664_000) .saturating_add(Weight::from_proof_size(31522)) .saturating_add(T::DbWeight::get().reads(21_u64)) .saturating_add(T::DbWeight::get().writes(15_u64)) @@ -418,11 +429,11 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `1909` // Estimated: `21998 + n * (2520 ±0)` - // Minimum execution time: 64_000 nanoseconds. - Weight::from_ref_time(65_318_309) + // Minimum execution time: 69_288 nanoseconds. + Weight::from_ref_time(71_075_293) .saturating_add(Weight::from_proof_size(21998)) - // Standard Error: 9_189 - .saturating_add(Weight::from_ref_time(1_618_914).saturating_mul(n.into())) + // Standard Error: 10_508 + .saturating_add(Weight::from_ref_time(1_384_674).saturating_mul(n.into())) .saturating_add(T::DbWeight::get().reads(12_u64)) .saturating_add(T::DbWeight::get().reads((1_u64).saturating_mul(n.into()))) .saturating_add(T::DbWeight::get().writes(5_u64)) @@ -438,8 +449,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `1498` // Estimated: `8752` - // Minimum execution time: 33_000 nanoseconds. - Weight::from_ref_time(34_000_000) + // Minimum execution time: 36_410 nanoseconds. + Weight::from_ref_time(37_585_000) .saturating_add(Weight::from_proof_size(8752)) .saturating_add(T::DbWeight::get().reads(3_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) @@ -455,11 +466,11 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `559` // Estimated: `5883` - // Minimum execution time: 13_000 nanoseconds. - Weight::from_ref_time(13_864_041) + // Minimum execution time: 14_322 nanoseconds. + Weight::from_ref_time(15_328_204) .saturating_add(Weight::from_proof_size(5883)) - // Standard Error: 198 - .saturating_add(Weight::from_ref_time(312).saturating_mul(n.into())) + // Standard Error: 161 + .saturating_add(Weight::from_ref_time(1_406).saturating_mul(n.into())) .saturating_add(T::DbWeight::get().reads(3_u64)) .saturating_add(T::DbWeight::get().writes(2_u64)) } @@ -477,8 +488,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `0` // Estimated: `0` - // Minimum execution time: 6_000 nanoseconds. - Weight::from_ref_time(6_000_000) + // Minimum execution time: 5_968 nanoseconds. + Weight::from_ref_time(6_245_000) .saturating_add(Weight::from_proof_size(0)) .saturating_add(T::DbWeight::get().writes(5_u64)) } @@ -488,8 +499,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `559` // Estimated: `2639` - // Minimum execution time: 17_000 nanoseconds. - Weight::from_ref_time(18_000_000) + // Minimum execution time: 18_979 nanoseconds. + Weight::from_ref_time(19_795_000) .saturating_add(Weight::from_proof_size(2639)) .saturating_add(T::DbWeight::get().reads(1_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) @@ -516,8 +527,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `2136` // Estimated: `20489` - // Minimum execution time: 62_000 nanoseconds. - Weight::from_ref_time(64_000_000) + // Minimum execution time: 68_145 nanoseconds. + Weight::from_ref_time(70_444_000) .saturating_add(Weight::from_proof_size(20489)) .saturating_add(T::DbWeight::get().reads(9_u64)) .saturating_add(T::DbWeight::get().writes(5_u64)) @@ -530,8 +541,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `542` // Estimated: `5228` - // Minimum execution time: 13_000 nanoseconds. - Weight::from_ref_time(14_000_000) + // Minimum execution time: 15_112 nanoseconds. + Weight::from_ref_time(15_897_000) .saturating_add(Weight::from_proof_size(5228)) .saturating_add(T::DbWeight::get().reads(2_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) @@ -570,8 +581,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `3573` // Estimated: `37988` - // Minimum execution time: 162_000 nanoseconds. - Weight::from_ref_time(164_000_000) + // Minimum execution time: 169_857 nanoseconds. + Weight::from_ref_time(173_895_000) .saturating_add(Weight::from_proof_size(37988)) .saturating_add(RocksDbWeight::get().reads(17_u64)) .saturating_add(RocksDbWeight::get().writes(12_u64)) @@ -598,8 +609,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `3615` // Estimated: `38583` - // Minimum execution time: 156_000 nanoseconds. - Weight::from_ref_time(159_000_000) + // Minimum execution time: 167_372 nanoseconds. + Weight::from_ref_time(168_776_000) .saturating_add(Weight::from_proof_size(38583)) .saturating_add(RocksDbWeight::get().reads(14_u64)) .saturating_add(RocksDbWeight::get().writes(12_u64)) @@ -628,8 +639,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `3680` // Estimated: `41099` - // Minimum execution time: 184_000 nanoseconds. - Weight::from_ref_time(188_000_000) + // Minimum execution time: 186_346 nanoseconds. + Weight::from_ref_time(191_308_000) .saturating_add(Weight::from_proof_size(41099)) .saturating_add(RocksDbWeight::get().reads(15_u64)) .saturating_add(RocksDbWeight::get().writes(13_u64)) @@ -646,8 +657,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `1189` // Estimated: `10489` - // Minimum execution time: 55_000 nanoseconds. - Weight::from_ref_time(57_000_000) + // Minimum execution time: 57_430 nanoseconds. + Weight::from_ref_time(59_161_000) .saturating_add(Weight::from_proof_size(10489)) .saturating_add(RocksDbWeight::get().reads(4_u64)) .saturating_add(RocksDbWeight::get().writes(4_u64)) @@ -666,8 +677,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `1254` // Estimated: `13005` - // Minimum execution time: 59_000 nanoseconds. - Weight::from_ref_time(60_000_000) + // Minimum execution time: 61_423 nanoseconds. + Weight::from_ref_time(63_219_000) .saturating_add(Weight::from_proof_size(13005)) .saturating_add(RocksDbWeight::get().reads(5_u64)) .saturating_add(RocksDbWeight::get().writes(4_u64)) @@ -706,8 +717,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `3858` // Estimated: `67379` - // Minimum execution time: 162_000 nanoseconds. - Weight::from_ref_time(164_000_000) + // Minimum execution time: 174_532 nanoseconds. + Weight::from_ref_time(180_032_000) .saturating_add(Weight::from_proof_size(67379)) .saturating_add(RocksDbWeight::get().reads(18_u64)) .saturating_add(RocksDbWeight::get().writes(14_u64)) @@ -723,13 +734,15 @@ impl WeightInfo for () { /// Storage: Balances Locks (r:1 w:1) /// Proof: Balances Locks (max_values: None, max_size: Some(1299), added: 3774, mode: MaxEncodedLen) /// The range of component `s` is `[0, 100]`. - fn pool_withdraw_unbonded(_s: u32, ) -> Weight { + fn pool_withdraw_unbonded(s: u32, ) -> Weight { // Proof Size summary in bytes: // Measured: `1779` // Estimated: `13025` - // Minimum execution time: 50_000 nanoseconds. - Weight::from_ref_time(52_796_229) + // Minimum execution time: 55_327 nanoseconds. + Weight::from_ref_time(58_947_746) .saturating_add(Weight::from_proof_size(13025)) + // Standard Error: 1_589 + .saturating_add(Weight::from_ref_time(40_696).saturating_mul(s.into())) .saturating_add(RocksDbWeight::get().reads(5_u64)) .saturating_add(RocksDbWeight::get().writes(2_u64)) } @@ -756,11 +769,11 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `2303` // Estimated: `45696` - // Minimum execution time: 101_000 nanoseconds. - Weight::from_ref_time(104_858_516) + // Minimum execution time: 105_923 nanoseconds. + Weight::from_ref_time(110_572_476) .saturating_add(Weight::from_proof_size(45696)) - // Standard Error: 1_643 - .saturating_add(Weight::from_ref_time(7_299).saturating_mul(s.into())) + // Standard Error: 2_438 + .saturating_add(Weight::from_ref_time(69_045).saturating_mul(s.into())) .saturating_add(RocksDbWeight::get().reads(9_u64)) .saturating_add(RocksDbWeight::get().writes(7_u64)) } @@ -805,15 +818,13 @@ impl WeightInfo for () { /// Storage: Staking Payee (r:0 w:1) /// Proof: Staking Payee (max_values: None, max_size: Some(73), added: 2548, mode: MaxEncodedLen) /// The range of component `s` is `[0, 100]`. - fn withdraw_unbonded_kill(s: u32, ) -> Weight { + fn withdraw_unbonded_kill(_s: u32, ) -> Weight { // Proof Size summary in bytes: // Measured: `2690` // Estimated: `68812` - // Minimum execution time: 167_000 nanoseconds. - Weight::from_ref_time(172_584_557) + // Minimum execution time: 169_700 nanoseconds. + Weight::from_ref_time(178_693_541) .saturating_add(Weight::from_proof_size(68812)) - // Standard Error: 3_095 - .saturating_add(Weight::from_ref_time(17_153).saturating_mul(s.into())) .saturating_add(RocksDbWeight::get().reads(20_u64)) .saturating_add(RocksDbWeight::get().writes(17_u64)) } @@ -863,8 +874,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `1321` // Estimated: `31522` - // Minimum execution time: 149_000 nanoseconds. - Weight::from_ref_time(151_000_000) + // Minimum execution time: 145_976 nanoseconds. + Weight::from_ref_time(150_664_000) .saturating_add(Weight::from_proof_size(31522)) .saturating_add(RocksDbWeight::get().reads(21_u64)) .saturating_add(RocksDbWeight::get().writes(15_u64)) @@ -898,11 +909,11 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `1909` // Estimated: `21998 + n * (2520 ±0)` - // Minimum execution time: 64_000 nanoseconds. - Weight::from_ref_time(65_318_309) + // Minimum execution time: 69_288 nanoseconds. + Weight::from_ref_time(71_075_293) .saturating_add(Weight::from_proof_size(21998)) - // Standard Error: 9_189 - .saturating_add(Weight::from_ref_time(1_618_914).saturating_mul(n.into())) + // Standard Error: 10_508 + .saturating_add(Weight::from_ref_time(1_384_674).saturating_mul(n.into())) .saturating_add(RocksDbWeight::get().reads(12_u64)) .saturating_add(RocksDbWeight::get().reads((1_u64).saturating_mul(n.into()))) .saturating_add(RocksDbWeight::get().writes(5_u64)) @@ -918,8 +929,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `1498` // Estimated: `8752` - // Minimum execution time: 33_000 nanoseconds. - Weight::from_ref_time(34_000_000) + // Minimum execution time: 36_410 nanoseconds. + Weight::from_ref_time(37_585_000) .saturating_add(Weight::from_proof_size(8752)) .saturating_add(RocksDbWeight::get().reads(3_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) @@ -935,11 +946,11 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `559` // Estimated: `5883` - // Minimum execution time: 13_000 nanoseconds. - Weight::from_ref_time(13_864_041) + // Minimum execution time: 14_322 nanoseconds. + Weight::from_ref_time(15_328_204) .saturating_add(Weight::from_proof_size(5883)) - // Standard Error: 198 - .saturating_add(Weight::from_ref_time(312).saturating_mul(n.into())) + // Standard Error: 161 + .saturating_add(Weight::from_ref_time(1_406).saturating_mul(n.into())) .saturating_add(RocksDbWeight::get().reads(3_u64)) .saturating_add(RocksDbWeight::get().writes(2_u64)) } @@ -957,8 +968,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `0` // Estimated: `0` - // Minimum execution time: 6_000 nanoseconds. - Weight::from_ref_time(6_000_000) + // Minimum execution time: 5_968 nanoseconds. + Weight::from_ref_time(6_245_000) .saturating_add(Weight::from_proof_size(0)) .saturating_add(RocksDbWeight::get().writes(5_u64)) } @@ -968,8 +979,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `559` // Estimated: `2639` - // Minimum execution time: 17_000 nanoseconds. - Weight::from_ref_time(18_000_000) + // Minimum execution time: 18_979 nanoseconds. + Weight::from_ref_time(19_795_000) .saturating_add(Weight::from_proof_size(2639)) .saturating_add(RocksDbWeight::get().reads(1_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) @@ -996,8 +1007,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `2136` // Estimated: `20489` - // Minimum execution time: 62_000 nanoseconds. - Weight::from_ref_time(64_000_000) + // Minimum execution time: 68_145 nanoseconds. + Weight::from_ref_time(70_444_000) .saturating_add(Weight::from_proof_size(20489)) .saturating_add(RocksDbWeight::get().reads(9_u64)) .saturating_add(RocksDbWeight::get().writes(5_u64)) @@ -1010,8 +1021,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `542` // Estimated: `5228` - // Minimum execution time: 13_000 nanoseconds. - Weight::from_ref_time(14_000_000) + // Minimum execution time: 15_112 nanoseconds. + Weight::from_ref_time(15_897_000) .saturating_add(Weight::from_proof_size(5228)) .saturating_add(RocksDbWeight::get().reads(2_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) From 999634189a6c83233717be37a94ba9bd77eeeed1 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 21 Feb 2023 13:15:45 +0700 Subject: [PATCH 41/50] tidy up claim payout benches --- .../nomination-pools/benchmarking/src/lib.rs | 34 +++------------ frame/nomination-pools/src/lib.rs | 2 +- frame/nomination-pools/src/weights.rs | 41 +------------------ 3 files changed, 9 insertions(+), 68 deletions(-) diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index ae11d4de615cc..de352085a1930 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -276,34 +276,6 @@ frame_benchmarking::benchmarks! { } claim_payout { - let origin_weight = Pools::::depositor_min_bond() * 2u32.into(); - let ed = CurrencyOf::::minimum_balance(); - let (depositor, pool_account) = create_pool_account::(0, origin_weight); - let reward_account = Pools::::create_reward_account(1); - - // Send funds to the reward account of the pool - CurrencyOf::::make_free_balance_be(&reward_account, ed + origin_weight); - - // Sanity check - assert_eq!( - CurrencyOf::::free_balance(&depositor), - origin_weight - ); - - whitelist_account!(depositor); - }:_(RuntimeOrigin::Signed(depositor.clone())) - verify { - assert_eq!( - CurrencyOf::::free_balance(&depositor), - origin_weight * 2u32.into() - ); - assert_eq!( - CurrencyOf::::free_balance(&reward_account), - ed + Zero::zero() - ); - } - - claim_payout_other { let claimer: T::AccountId = account("claimer", USER_SEED + 4, 0); let origin_weight = Pools::::depositor_min_bond() * 2u32.into(); @@ -318,6 +290,12 @@ frame_benchmarking::benchmarks! { // behalf. let _ = Pools::::set_reward_claim(RuntimeOrigin::Signed(depositor.clone()).into(), ClaimPermission::PermissionlessAll); + // Sanity check + assert_eq!( + CurrencyOf::::free_balance(&depositor), + origin_weight + ); + whitelist_account!(depositor); }:_(RuntimeOrigin::Signed(claimer), depositor.clone()) verify { diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 9dc86541e2b82..ec6b8ca350644 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -2164,7 +2164,7 @@ pub mod pallet { /// `origin` can claim payouts on some pool member `member`'s behalf. #[pallet::call_index(16)] - #[pallet::weight(T::WeightInfo::claim_payout_other())] + #[pallet::weight(T::WeightInfo::claim_payout())] pub fn claim_payout_other(origin: OriginFor, who: T::AccountId) -> DispatchResult { let signer = ensure_signed(origin)?; Self::do_claim_payout(signer, who) diff --git a/frame/nomination-pools/src/weights.rs b/frame/nomination-pools/src/weights.rs index 0031efb717dc5..4ed4d156b8b1c 100644 --- a/frame/nomination-pools/src/weights.rs +++ b/frame/nomination-pools/src/weights.rs @@ -53,7 +53,6 @@ pub trait WeightInfo { fn bond_extra_transfer() -> Weight; fn bond_extra_other() -> Weight; fn claim_payout() -> Weight; - fn claim_payout_other() -> Weight; fn unbond() -> Weight; fn pool_withdraw_unbonded(s: u32, ) -> Weight; fn withdraw_unbonded_update(s: u32, ) -> Weight; @@ -165,24 +164,6 @@ impl WeightInfo for SubstrateWeight { .saturating_add(T::DbWeight::get().reads(15_u64)) .saturating_add(T::DbWeight::get().writes(13_u64)) } - /// Storage: NominationPools PoolMembers (r:1 w:1) - /// Proof: NominationPools PoolMembers (max_values: None, max_size: Some(237), added: 2712, mode: MaxEncodedLen) - /// Storage: NominationPools BondedPools (r:1 w:1) - /// Proof: NominationPools BondedPools (max_values: None, max_size: Some(164), added: 2639, mode: MaxEncodedLen) - /// Storage: NominationPools RewardPools (r:1 w:1) - /// Proof: NominationPools RewardPools (max_values: None, max_size: Some(60), added: 2535, mode: MaxEncodedLen) - /// Storage: System Account (r:1 w:1) - /// Proof: System Account (max_values: None, max_size: Some(128), added: 2603, mode: MaxEncodedLen) - fn claim_payout() -> Weight { - // Proof Size summary in bytes: - // Measured: `1189` - // Estimated: `10489` - // Minimum execution time: 57_430 nanoseconds. - Weight::from_ref_time(59_161_000) - .saturating_add(Weight::from_proof_size(10489)) - .saturating_add(T::DbWeight::get().reads(4_u64)) - .saturating_add(T::DbWeight::get().writes(4_u64)) - } /// Storage: NominationPools ClaimPermissions (r:1 w:0) /// Proof: NominationPools ClaimPermissions (max_values: None, max_size: Some(41), added: 2516, mode: MaxEncodedLen) /// Storage: NominationPools PoolMembers (r:1 w:1) @@ -193,7 +174,7 @@ impl WeightInfo for SubstrateWeight { /// Proof: NominationPools RewardPools (max_values: None, max_size: Some(60), added: 2535, mode: MaxEncodedLen) /// Storage: System Account (r:1 w:1) /// Proof: System Account (max_values: None, max_size: Some(128), added: 2603, mode: MaxEncodedLen) - fn claim_payout_other() -> Weight { + fn claim_payout() -> Weight { // Proof Size summary in bytes: // Measured: `1254` // Estimated: `13005` @@ -645,24 +626,6 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().reads(15_u64)) .saturating_add(RocksDbWeight::get().writes(13_u64)) } - /// Storage: NominationPools PoolMembers (r:1 w:1) - /// Proof: NominationPools PoolMembers (max_values: None, max_size: Some(237), added: 2712, mode: MaxEncodedLen) - /// Storage: NominationPools BondedPools (r:1 w:1) - /// Proof: NominationPools BondedPools (max_values: None, max_size: Some(164), added: 2639, mode: MaxEncodedLen) - /// Storage: NominationPools RewardPools (r:1 w:1) - /// Proof: NominationPools RewardPools (max_values: None, max_size: Some(60), added: 2535, mode: MaxEncodedLen) - /// Storage: System Account (r:1 w:1) - /// Proof: System Account (max_values: None, max_size: Some(128), added: 2603, mode: MaxEncodedLen) - fn claim_payout() -> Weight { - // Proof Size summary in bytes: - // Measured: `1189` - // Estimated: `10489` - // Minimum execution time: 57_430 nanoseconds. - Weight::from_ref_time(59_161_000) - .saturating_add(Weight::from_proof_size(10489)) - .saturating_add(RocksDbWeight::get().reads(4_u64)) - .saturating_add(RocksDbWeight::get().writes(4_u64)) - } /// Storage: NominationPools ClaimPermissions (r:1 w:0) /// Proof: NominationPools ClaimPermissions (max_values: None, max_size: Some(41), added: 2516, mode: MaxEncodedLen) /// Storage: NominationPools PoolMembers (r:1 w:1) @@ -673,7 +636,7 @@ impl WeightInfo for () { /// Proof: NominationPools RewardPools (max_values: None, max_size: Some(60), added: 2535, mode: MaxEncodedLen) /// Storage: System Account (r:1 w:1) /// Proof: System Account (max_values: None, max_size: Some(128), added: 2603, mode: MaxEncodedLen) - fn claim_payout_other() -> Weight { + fn claim_payout() -> Weight { // Proof Size summary in bytes: // Measured: `1254` // Estimated: `13005` From 6fa711c840926c15b7fb17e79a0cbe3b319c02dd Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 21 Feb 2023 13:21:32 +0700 Subject: [PATCH 42/50] fix --- frame/nomination-pools/benchmarking/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index de352085a1930..98d6e24c07af5 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -297,7 +297,7 @@ frame_benchmarking::benchmarks! { ); whitelist_account!(depositor); - }:_(RuntimeOrigin::Signed(claimer), depositor.clone()) + }:claim_payout_other(RuntimeOrigin::Signed(claimer), depositor.clone()) verify { assert_eq!( CurrencyOf::::free_balance(&depositor), From 0f9f730422f01ff3d350df723da4edd52e35faab Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 21 Feb 2023 13:44:43 +0700 Subject: [PATCH 43/50] + test: claim_payout_other_works --- frame/nomination-pools/src/tests.rs | 30 +++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index ae3b15023d55c..51dcc947d0864 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -2064,6 +2064,36 @@ mod claim_payout { ); }) } + + #[test] + fn claim_payout_other_works() { + ExtBuilder::default().add_members(vec![(20, 20)]).build_and_execute(|| { + Balances::make_free_balance_be(&default_reward_account(), 8); + // ... of which only 3 are claimable to make sure the reward account does not die. + let claimable_reward = 8 - ExistentialDeposit::get(); + // NOTE: easier to read if we use 3, so let's use the number instead of variable. + assert_eq!(claimable_reward, 3, "test is correct if rewards are divisible by 3"); + + // given + assert_eq!(Balances::free_balance(10), 35); + + // Permissioned by default + assert_noop!( + Pools::claim_payout_other(RuntimeOrigin::signed(80), 10), + Error::::DoesNotHavePermission + ); + + assert_ok!(Pools::set_reward_claim( + RuntimeOrigin::signed(10), + ClaimPermission::PermissionlessWithdraw + )); + assert_ok!(Pools::claim_payout_other(RuntimeOrigin::signed(80), 10)); + + // then + assert_eq!(Balances::free_balance(10), 36); + assert_eq!(Balances::free_balance(&default_reward_account()), 7); + }) + } } mod unbond { From 4bdf22ac3a341ba794394d92ba272b39ad9d3deb Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 21 Feb 2023 14:00:18 +0700 Subject: [PATCH 44/50] comments, rename to set_claim_permission --- .../nomination-pools/benchmarking/src/lib.rs | 6 +-- frame/nomination-pools/src/lib.rs | 37 ++++++++++++------- frame/nomination-pools/src/tests.rs | 13 ++++--- frame/nomination-pools/src/weights.rs | 6 +-- 4 files changed, 38 insertions(+), 24 deletions(-) diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index 98d6e24c07af5..869285270f5b4 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -260,7 +260,7 @@ frame_benchmarking::benchmarks! { let extra = (scenario.dest_weight - origin_weight).max(CurrencyOf::::minimum_balance()); // set claim preferences to `PermissionlessAll` to any account to bond extra on member's behalf. - let _ = Pools::::set_reward_claim(RuntimeOrigin::Signed(scenario.creator1.clone()).into(), ClaimPermission::PermissionlessAll); + let _ = Pools::::set_claim_permission(RuntimeOrigin::Signed(scenario.creator1.clone()).into(), ClaimPermission::PermissionlessAll); // transfer exactly `extra` to the depositor of the src pool (1), let reward_account1 = Pools::::create_reward_account(1); @@ -288,7 +288,7 @@ frame_benchmarking::benchmarks! { // set claim preferences to `PermissionlessAll` so any account can claim rewards on member's // behalf. - let _ = Pools::::set_reward_claim(RuntimeOrigin::Signed(depositor.clone()).into(), ClaimPermission::PermissionlessAll); + let _ = Pools::::set_claim_permission(RuntimeOrigin::Signed(depositor.clone()).into(), ClaimPermission::PermissionlessAll); // Sanity check assert_eq!( @@ -666,7 +666,7 @@ frame_benchmarking::benchmarks! { assert!(T::Staking::nominations(Pools::::create_bonded_account(1)).is_none()); } - set_reward_claim { + set_claim_permission { // Create a pool let min_create_bond = Pools::::depositor_min_bond(); let (depositor, pool_account) = create_pool_account::(0, min_create_bond); diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index ec6b8ca350644..e7e552c802a36 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -61,6 +61,10 @@ //! //! After joining a pool, a member can claim rewards by calling [`Call::claim_payout`]. //! +//! A pool member can also set a `ClaimPermission` with [`Call:set_claim_permission`], to allow +//! other members to permissionlessly bond or withdraw their rewards by calling +//! [`Call::bond_extra_other`] or [`Call::claim_payout_other`] respectively. +//! //! For design docs see the [reward pool](#reward-pool) section. //! //! ### Leave @@ -411,10 +415,10 @@ enum AccountType { Reward, } -/// Account to bond pending reward of a member. +/// The permission a pool member can set for other accounts to claim rewards on their behalf. #[derive(Encode, Decode, MaxEncodedLen, Clone, Copy, Debug, PartialEq, Eq, TypeInfo)] pub enum ClaimPermission { - /// Only the pool member themself can claim their reward. + /// Only the pool member themself can claim their rewards. Permissioned, /// Anyone can compound rewards on a pool member's behalf. PermissionlessCompound, @@ -1335,7 +1339,7 @@ pub mod pallet { pub type ReversePoolIdLookup = CountedStorageMap<_, Twox64Concat, T::AccountId, PoolId, OptionQuery>; - /// Map from a pool member account to their preference regarding reward payout. + /// Map from a pool member account to their opted claim permission. #[pallet::storage] pub type ClaimPermissions = StorageMap<_, Twox64Concat, T::AccountId, ClaimPermission, ValueQuery>; @@ -1609,6 +1613,8 @@ 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". + /// + /// See `claim_payout_other` to caim rewards on bahalf of some `other` pool member. #[pallet::call_index(2)] #[pallet::weight(T::WeightInfo::claim_payout())] pub fn claim_payout(origin: OriginFor) -> DispatchResult { @@ -2121,7 +2127,8 @@ pub mod pallet { /// other`. /// /// In the case of `origin != other`, `origin` can only bond extra pending rewards of - /// `other` members assuming set_reward_claim for the given member is `PermissionlessAll`. + /// `other` members assuming set_claim_permission for the given member is + /// `PermissionlessAll` or `PermissionlessCompound`. #[pallet::call_index(14)] #[pallet::weight( T::WeightInfo::bond_extra_transfer() @@ -2136,12 +2143,13 @@ pub mod pallet { Self::do_bond_extra(who, T::Lookup::lookup(member)?, extra) } - /// Sets permission to claim reward. + /// Allows a pool member to set a claim permission to allow or disallow permissionless + /// bonding and withdrawing. /// - /// Lets a pool member to choose who can claim pending rewards on their behalf. By default, - /// this is `Permissioned` which implies only the pool member themselves can claim their - /// pending rewards. If a pool member wishes so, they can set this to `PermissionlessAll` to - /// allow any account to claim their rewards and bond extra to the pool. + /// By default, this is `Permissioned`, which implies only the pool member themselves can + /// claim their pending rewards. If a pool member wishes so, they can set this to + /// `PermissionlessAll` to allow any account to claim their rewards and bond extra to the + /// pool. /// /// # Arguments /// @@ -2149,7 +2157,7 @@ pub mod pallet { /// * `actor` - Account to claim reward. // improve this #[pallet::call_index(15)] #[pallet::weight(T::DbWeight::get().reads_writes(1, 1))] - pub fn set_reward_claim( + pub fn set_claim_permission( origin: OriginFor, permission: ClaimPermission, ) -> DispatchResult { @@ -2162,12 +2170,15 @@ pub mod pallet { Ok(()) } - /// `origin` can claim payouts on some pool member `member`'s behalf. + /// `origin` can claim payouts on some pool member `other`'s behalf. + /// + /// Pool member `other` must have a `PermissionlessAll` or `PermissionlessWithdraw` in order + /// for this call to be successful. #[pallet::call_index(16)] #[pallet::weight(T::WeightInfo::claim_payout())] - pub fn claim_payout_other(origin: OriginFor, who: T::AccountId) -> DispatchResult { + pub fn claim_payout_other(origin: OriginFor, other: T::AccountId) -> DispatchResult { let signer = ensure_signed(origin)?; - Self::do_claim_payout(signer, who) + Self::do_claim_payout(signer, other) } } diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 51dcc947d0864..7931e2f38ccae 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -2083,7 +2083,7 @@ mod claim_payout { Error::::DoesNotHavePermission ); - assert_ok!(Pools::set_reward_claim( + assert_ok!(Pools::set_claim_permission( RuntimeOrigin::signed(10), ClaimPermission::PermissionlessWithdraw )); @@ -2122,7 +2122,7 @@ mod unbond { // Make permissionless assert_eq!(ClaimPermissions::::get(10), ClaimPermission::Permissioned); - assert_ok!(Pools::set_reward_claim( + assert_ok!(Pools::set_claim_permission( RuntimeOrigin::signed(20), ClaimPermission::PermissionlessAll )); @@ -4301,10 +4301,13 @@ fn set_claimable_actor_works() { // Make permissionless assert_eq!(ClaimPermissions::::get(11), ClaimPermission::Permissioned); assert_noop!( - Pools::set_reward_claim(RuntimeOrigin::signed(12), ClaimPermission::PermissionlessAll), + Pools::set_claim_permission( + RuntimeOrigin::signed(12), + ClaimPermission::PermissionlessAll + ), Error::::PoolMemberNotFound ); - assert_ok!(Pools::set_reward_claim( + assert_ok!(Pools::set_claim_permission( RuntimeOrigin::signed(11), ClaimPermission::PermissionlessAll )); @@ -4681,7 +4684,7 @@ mod bond_extra { Error::::DoesNotHavePermission ); - assert_ok!(Pools::set_reward_claim( + assert_ok!(Pools::set_claim_permission( RuntimeOrigin::signed(10), ClaimPermission::PermissionlessAll )); diff --git a/frame/nomination-pools/src/weights.rs b/frame/nomination-pools/src/weights.rs index 4ed4d156b8b1c..093af7e16bb89 100644 --- a/frame/nomination-pools/src/weights.rs +++ b/frame/nomination-pools/src/weights.rs @@ -64,7 +64,7 @@ pub trait WeightInfo { fn set_configs() -> Weight; fn update_roles() -> Weight; fn chill() -> Weight; - fn set_reward_claim() -> Weight; + fn set_claim_permission() -> Weight; } /// Weights for pallet_nomination_pools using the Substrate node and recommended hardware. @@ -518,7 +518,7 @@ impl WeightInfo for SubstrateWeight { /// Proof: NominationPools PoolMembers (max_values: None, max_size: Some(237), added: 2712, mode: MaxEncodedLen) /// Storage: NominationPools ClaimPermissions (r:1 w:1) /// Proof: NominationPools ClaimPermissions (max_values: None, max_size: Some(41), added: 2516, mode: MaxEncodedLen) - fn set_reward_claim() -> Weight { + fn set_claim_permission() -> Weight { // Proof Size summary in bytes: // Measured: `542` // Estimated: `5228` @@ -980,7 +980,7 @@ impl WeightInfo for () { /// Proof: NominationPools PoolMembers (max_values: None, max_size: Some(237), added: 2712, mode: MaxEncodedLen) /// Storage: NominationPools ClaimPermissions (r:1 w:1) /// Proof: NominationPools ClaimPermissions (max_values: None, max_size: Some(41), added: 2516, mode: MaxEncodedLen) - fn set_reward_claim() -> Weight { + fn set_claim_permission() -> Weight { // Proof Size summary in bytes: // Measured: `542` // Estimated: `5228` From 2965fd9fc785938464b25f50bfef1b8d90b31763 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 21 Feb 2023 14:05:27 +0700 Subject: [PATCH 45/50] fix comment --- frame/nomination-pools/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index e7e552c802a36..5f56866be03e1 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -61,7 +61,7 @@ //! //! After joining a pool, a member can claim rewards by calling [`Call::claim_payout`]. //! -//! A pool member can also set a `ClaimPermission` with [`Call:set_claim_permission`], to allow +//! A pool member can also set a `ClaimPermission` with [`Call::set_claim_permission`], to allow //! other members to permissionlessly bond or withdraw their rewards by calling //! [`Call::bond_extra_other`] or [`Call::claim_payout_other`] respectively. //! From 7c16c8e70a4e3d836ef07b07536b14b115613715 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 21 Feb 2023 18:20:55 +0700 Subject: [PATCH 46/50] remove ClaimPermission on leave pool --- frame/nomination-pools/src/lib.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 5f56866be03e1..809b246a91f50 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -1719,8 +1719,6 @@ pub mod pallet { // Now that we know everything has worked write the items to storage. SubPoolsStorage::insert(&member.pool_id, sub_pools); Self::put_member_with_pools(&member_account, member, bonded_pool, reward_pool); - >::remove(member_account); - Ok(()) } @@ -1845,6 +1843,9 @@ pub mod pallet { }); let post_info_weight = if member.total_points().is_zero() { + // remove any `ClaimPermission` associated with the member. + ClaimPermissions::::remove(&member_account); + // member being reaped. PoolMembers::::remove(&member_account); Self::deposit_event(Event::::MemberRemoved { @@ -2164,7 +2165,7 @@ pub mod pallet { let who = ensure_signed(origin)?; ensure!(PoolMembers::::contains_key(&who), Error::::PoolMemberNotFound); - >::mutate(who, |source| { + ClaimPermissions::::mutate(who, |source| { *source = permission; }); Ok(()) From ef63fdfc7b026264535f762691a607ce37b1713d Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 21 Feb 2023 18:26:52 +0700 Subject: [PATCH 47/50] fix test --- frame/nomination-pools/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 7931e2f38ccae..c2296e53ded0f 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -2131,7 +2131,7 @@ mod unbond { assert_ok!(Pools::unbond(RuntimeOrigin::signed(20), 20, 15)); assert_eq!(PoolMembers::::get(20).unwrap().active_points(), 0); assert_eq!(PoolMembers::::get(20).unwrap().unbonding_points(), 20); - assert_eq!(ClaimPermissions::::get(20), ClaimPermission::Permissioned); + assert_eq!(ClaimPermissions::::get(20), ClaimPermission::PermissionlessAll); }) } From f8cedba1d36222189f7ca060062fef5b704619be Mon Sep 17 00:00:00 2001 From: command-bot <> Date: Tue, 21 Feb 2023 11:57:08 +0000 Subject: [PATCH 48/50] ".git/.scripts/commands/fmt/fmt.sh" --- frame/nomination-pools/src/tests.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index c2296e53ded0f..fda5c0186ef44 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -2131,7 +2131,10 @@ mod unbond { assert_ok!(Pools::unbond(RuntimeOrigin::signed(20), 20, 15)); assert_eq!(PoolMembers::::get(20).unwrap().active_points(), 0); assert_eq!(PoolMembers::::get(20).unwrap().unbonding_points(), 20); - assert_eq!(ClaimPermissions::::get(20), ClaimPermission::PermissionlessAll); + assert_eq!( + ClaimPermissions::::get(20), + ClaimPermission::PermissionlessAll + ); }) } From 0ea2309a213ea06550c0224777d0a211d5d9642f Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Wed, 22 Feb 2023 18:06:11 +0700 Subject: [PATCH 49/50] + test for ClaimPermissions::remove() --- frame/nomination-pools/src/tests.rs | 43 +++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 2dcdea9ff93b0..33eee01948d1f 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -4139,6 +4139,49 @@ mod withdraw_unbonded { assert!(!Metadata::::contains_key(1)); }) } + + #[test] + fn withdraw_unbonded_removes_claim_permissions_on_leave() { + ExtBuilder::default().add_members(vec![(20, 20)]).build_and_execute(|| { + // Given + CurrentEra::set(1); + assert_eq!(PoolMembers::::get(20).unwrap().points, 20); + + assert_ok!(Pools::set_claim_permission( + RuntimeOrigin::signed(20), + ClaimPermission::PermissionlessAll + )); + assert_ok!(Pools::unbond(RuntimeOrigin::signed(20), 20, 20)); + assert_eq!(ClaimPermissions::::get(20), ClaimPermission::PermissionlessAll); + + assert_eq!( + pool_events_since_last_call(), + vec![ + Event::Created { depositor: 10, pool_id: 1 }, + Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::Bonded { member: 20, pool_id: 1, bonded: 20, joined: true }, + Event::Unbonded { member: 20, pool_id: 1, balance: 20, points: 20, era: 4 }, + ] + ); + + CurrentEra::set(5); + + // When + assert_ok!(Pools::withdraw_unbonded(RuntimeOrigin::signed(20), 20, 0)); + + assert_eq!( + pool_events_since_last_call(), + vec![ + Event::Withdrawn { member: 20, pool_id: 1, balance: 20, points: 20 }, + Event::MemberRemoved { pool_id: 1, member: 20 } + ] + ); + + // Then + assert_eq!(PoolMembers::::get(20), None); + assert_eq!(ClaimPermissions::::contains_key(20), false); + }); + } } mod create { From a39573c3432276fd23ac7074c68accde9d897da9 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Wed, 22 Feb 2023 18:13:03 +0700 Subject: [PATCH 50/50] impl can_bond_extra & can_claim_payout --- frame/nomination-pools/src/lib.rs | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 0c2a6f8163e1b..b296eb048562a 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -428,6 +428,24 @@ pub enum ClaimPermission { PermissionlessAll, } +impl ClaimPermission { + fn can_bond_extra(&self) -> bool { + match self { + ClaimPermission::PermissionlessAll => true, + ClaimPermission::PermissionlessCompound => true, + _ => false, + } + } + + fn can_claim_payout(&self) -> bool { + match self { + ClaimPermission::PermissionlessAll => true, + ClaimPermission::PermissionlessWithdraw => true, + _ => false, + } + } +} + impl Default for ClaimPermission { fn default() -> Self { Self::Permissioned @@ -2469,10 +2487,7 @@ impl Pallet { ) -> DispatchResult { if signer != who { ensure!( - matches!( - ClaimPermissions::::get(&who), - ClaimPermission::PermissionlessAll | ClaimPermission::PermissionlessCompound - ), + ClaimPermissions::::get(&who).can_bond_extra(), Error::::DoesNotHavePermission ); ensure!(extra == BondExtra::Rewards, Error::::BondExtraRestricted); @@ -2511,10 +2526,7 @@ impl Pallet { fn do_claim_payout(signer: T::AccountId, who: T::AccountId) -> DispatchResult { if signer != who { ensure!( - matches!( - ClaimPermissions::::get(&who), - ClaimPermission::PermissionlessAll | ClaimPermission::PermissionlessWithdraw - ), + ClaimPermissions::::get(&who).can_claim_payout(), Error::::DoesNotHavePermission ); }