Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

permissionless bond_extra in nomination pools #12608

Merged
merged 63 commits into from
Feb 22, 2023
Merged
Show file tree
Hide file tree
Changes from 59 commits
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
c5cde02
create enum
Doordashcon Nov 3, 2022
3ab9cd7
logic check
Doordashcon Nov 9, 2022
59958d5
add benchmarks
Doordashcon Nov 14, 2022
9d54b8a
-enum
Doordashcon Nov 18, 2022
8efecfe
Merge branch 'paritytech:master' into ddc-MCF-T1
Doordashcon Nov 19, 2022
f7b0a2d
update
Doordashcon Nov 20, 2022
8931433
bond extra other
Doordashcon Dec 3, 2022
9f83103
update
Doordashcon Dec 4, 2022
9d63fdb
update
Doordashcon Dec 4, 2022
a3718f6
update
Doordashcon Dec 5, 2022
4c6a4e7
cargo fmt
Doordashcon Dec 5, 2022
8207d51
Permissioned
Doordashcon Dec 7, 2022
0401174
Merge branch 'paritytech:master' into ddc-MCF-T1
Doordashcon Dec 7, 2022
a4b6ec2
update
Doordashcon Dec 15, 2022
db9c1b8
cargo fmt
Doordashcon Dec 15, 2022
4d8bc6f
Merge branch 'paritytech:master' into ddc-MCF-T1
Doordashcon Dec 15, 2022
b5eda08
update
Doordashcon Dec 18, 2022
58173e0
Merge branch 'paritytech:master' into ddc-MCF-T1
Doordashcon Jan 2, 2023
b110526
update index
Doordashcon Jan 2, 2023
126bc8d
Merge branch 'paritytech:master' into ddc-MCF-T1
Doordashcon Jan 10, 2023
a45d615
doc update
Doordashcon Jan 26, 2023
ab1e7bb
doc update
Doordashcon Jan 30, 2023
ab064f7
Merge branch 'paritytech:master' into ddc-MCF-T1
Doordashcon Feb 1, 2023
5f25374
cargo fmt
Doordashcon Feb 1, 2023
619e9b9
bond_extra auto compound
Doordashcon Feb 2, 2023
15c286f
bond_extra_other
Doordashcon Feb 7, 2023
1a1479d
Merge branch 'paritytech:master' into ddc-MCF-T1
Doordashcon Feb 7, 2023
b21097b
Merge branch 'paritytech:master' into ddc-MCF-T1
Doordashcon Feb 8, 2023
4ffe8f5
Merge branch 'paritytech:master' into ddc-MCF-T1
Doordashcon Feb 9, 2023
fcd63b2
Apply suggestions from code review
kianenigma Feb 13, 2023
f6254fa
Fixes from kian
kianenigma Feb 13, 2023
ce1ebc9
updates docs & test
Doordashcon Feb 14, 2023
62edda4
Merge remote-tracking branch 'origin/master' into ddc-MCF-T1
Feb 19, 2023
1211229
Update frame/nomination-pools/src/lib.rs
Feb 20, 2023
6e4f2b4
Update frame/nomination-pools/src/lib.rs
Feb 20, 2023
832281d
Update frame/nomination-pools/src/lib.rs
Feb 20, 2023
e242799
Update frame/nomination-pools/src/lib.rs
Feb 20, 2023
011d733
Update frame/nomination-pools/src/lib.rs
Feb 20, 2023
d9590b1
Update frame/nomination-pools/src/tests.rs
Feb 20, 2023
04d6b13
Update frame/nomination-pools/src/lib.rs
Feb 20, 2023
b8be98c
Update frame/nomination-pools/src/tests.rs
Feb 20, 2023
d8fac0e
Update frame/nomination-pools/src/tests.rs
Feb 20, 2023
79f9b7e
Update frame/nomination-pools/src/tests.rs
Feb 20, 2023
184d6f2
Update frame/nomination-pools/src/tests.rs
Feb 20, 2023
0e0cc6f
Update frame/nomination-pools/src/tests.rs
Feb 20, 2023
be3cc5e
Update frame/nomination-pools/src/lib.rs
Feb 20, 2023
768c63d
Update frame/nomination-pools/src/tests.rs
Feb 20, 2023
549a743
fixes + fmt
Feb 20, 2023
e823b43
Merge remote-tracking branch 'origin/master' into ddc-MCF-T1
Feb 21, 2023
1c17bcc
expand ClaimPermissions + add benchmarks
Feb 21, 2023
9b1eb00
".git/.scripts/commands/bench/bench.sh" pallet dev pallet_nomination_…
Feb 21, 2023
9996341
tidy up claim payout benches
Feb 21, 2023
6fa711c
fix
Feb 21, 2023
0f9f730
+ test: claim_payout_other_works
Feb 21, 2023
4bdf22a
comments, rename to set_claim_permission
Feb 21, 2023
2965fd9
fix comment
Feb 21, 2023
7c16c8e
remove ClaimPermission on leave pool
Feb 21, 2023
ef63fdf
fix test
Feb 21, 2023
f8cedba
".git/.scripts/commands/fmt/fmt.sh"
Feb 21, 2023
4ac673b
Merge remote-tracking branch 'origin/master' into ddc-MCF-T1
Feb 22, 2023
0ea2309
+ test for ClaimPermissions::remove()
Feb 22, 2023
a39573c
impl can_bond_extra & can_claim_payout
Feb 22, 2023
f7a6568
Merge remote-tracking branch 'origin/master' into ddc-MCF-T1
Feb 22, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 40 additions & 6 deletions frame/nomination-pools/benchmarking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +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, 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};
Expand Down Expand Up @@ -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::<T>::depositor_min_bond() * 2u32.into();
let scenario = ListScenario::<T>::new(origin_weight, true)?;
let extra = (scenario.dest_weight - origin_weight).max(CurrencyOf::<T>::minimum_balance());

// set claim preferences to `PermissionlessAll` to any account to bond extra on member's behalf.
let _ = Pools::<T>::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::<T>::create_reward_account(1);
assert!(extra >= CurrencyOf::<T>::minimum_balance());
CurrencyOf::<T>::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() >=
Expand All @@ -271,6 +276,8 @@ frame_benchmarking::benchmarks! {
}

claim_payout {
let claimer: T::AccountId = account("claimer", USER_SEED + 4, 0);

let origin_weight = Pools::<T>::depositor_min_bond() * 2u32.into();
let ed = CurrencyOf::<T>::minimum_balance();
let (depositor, pool_account) = create_pool_account::<T>(0, origin_weight);
Expand All @@ -279,14 +286,18 @@ frame_benchmarking::benchmarks! {
// Send funds to the reward account of the pool
CurrencyOf::<T>::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::<T>::set_claim_permission(RuntimeOrigin::Signed(depositor.clone()).into(), ClaimPermission::PermissionlessAll);

// Sanity check
assert_eq!(
CurrencyOf::<T>::free_balance(&depositor),
origin_weight
);

whitelist_account!(depositor);
}:_(RuntimeOrigin::Signed(depositor.clone()))
}:claim_payout_other(RuntimeOrigin::Signed(claimer), depositor.clone())
verify {
assert_eq!(
CurrencyOf::<T>::free_balance(&depositor),
Expand All @@ -298,6 +309,7 @@ frame_benchmarking::benchmarks! {
);
}


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).
Expand Down Expand Up @@ -654,6 +666,28 @@ frame_benchmarking::benchmarks! {
assert!(T::Staking::nominations(Pools::<T>::create_bonded_account(1)).is_none());
}

set_claim_permission {
// Create a pool
let min_create_bond = Pools::<T>::depositor_min_bond();
let (depositor, pool_account) = create_pool_account::<T>(0, min_create_bond);

// Join pool
let min_join_bond = MinJoinBond::<T>::get().max(CurrencyOf::<T>::minimum_balance());
let joiner = create_funded_user_with_balance::<T>("joiner", 0, min_join_bond * 4u32.into());
let joiner_lookup = T::Lookup::unlookup(joiner.clone());
Pools::<T>::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()), ClaimPermission::PermissionlessAll)
verify {
assert_eq!(ClaimPermissions::<T>::get(joiner), ClaimPermission::PermissionlessAll);
}

impl_benchmark_test_suite!(
Pallet,
crate::mock::new_test_ext(),
Expand Down
202 changes: 165 additions & 37 deletions frame/nomination-pools/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -411,6 +415,25 @@ enum AccountType {
Reward,
}

/// 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 rewards.
Permissioned,
/// 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 {
fn default() -> Self {
Self::Permissioned
}
}

/// A member in a pool.
#[derive(Encode, Decode, MaxEncodedLen, TypeInfo, RuntimeDebugNoBound, CloneNoBound)]
#[cfg_attr(feature = "std", derive(frame_support::PartialEqNoBound, DefaultNoBound))]
Expand Down Expand Up @@ -1316,6 +1339,11 @@ pub mod pallet {
pub type ReversePoolIdLookup<T: Config> =
CountedStorageMap<_, Twox64Concat, T::AccountId, PoolId, OptionQuery>;

/// Map from a pool member account to their opted claim permission.
#[pallet::storage]
pub type ClaimPermissions<T: Config> =
StorageMap<_, Twox64Concat, T::AccountId, ClaimPermission, ValueQuery>;

#[pallet::genesis_config]
pub struct GenesisConfig<T: Config> {
pub min_join_bond: BalanceOf<T>,
Expand Down Expand Up @@ -1473,6 +1501,8 @@ pub mod pallet {
PoolIdInUse,
/// Pool id provided is not correct/usable.
InvalidPoolId,
/// Bonding extra is restricted to the exact pending reward amount.
BondExtraRestricted,
}

#[derive(Encode, Decode, PartialEq, TypeInfo, frame_support::PalletError, RuntimeDebug)]
Expand Down Expand Up @@ -1563,44 +1593,18 @@ 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.
#[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<T>, extra: BondExtra<BalanceOf<T>>) -> DispatchResult {
let who = ensure_signed(origin)?;
let (mut member, mut bonded_pool, mut reward_pool) = Self::get_member_with_pools(&who)?;

// 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::<T>::OverflowRisk)?;

Self::deposit_event(Event::<T>::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.clone(), who, extra)
}

/// A bonded member can use this to claim their payout based on the rewards that the pool
Expand All @@ -1609,16 +1613,13 @@ 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<T>) -> 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
Expand Down Expand Up @@ -1718,7 +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);

Ok(())
}

Expand Down Expand Up @@ -1843,6 +1843,9 @@ pub mod pallet {
});

let post_info_weight = if member.total_points().is_zero() {
// remove any `ClaimPermission` associated with the member.
ClaimPermissions::<T>::remove(&member_account);

// member being reaped.
PoolMembers::<T>::remove(&member_account);
Self::deposit_event(Event::<T>::MemberRemoved {
Expand Down Expand Up @@ -2117,6 +2120,67 @@ pub mod pallet {
ensure!(bonded_pool.can_nominate(&who), Error::<T>::NotNominator);
T::Staking::chill(&bonded_pool.bonded_account())
}

/// `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`.
///
/// In the case of `origin != other`, `origin` can only bond extra pending rewards of
/// `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()
.max(T::WeightInfo::bond_extra_other())
)]
pub fn bond_extra_other(
origin: OriginFor<T>,
member: AccountIdLookupOf<T>,
extra: BondExtra<BalanceOf<T>>,
) -> DispatchResult {
let who = ensure_signed(origin)?;
Self::do_bond_extra(who, T::Lookup::lookup(member)?, extra)
}

/// Allows a pool member to set a claim permission to allow or disallow permissionless
/// bonding and withdrawing.
///
/// 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
///
/// * `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_claim_permission(
origin: OriginFor<T>,
permission: ClaimPermission,
) -> DispatchResult {
let who = ensure_signed(origin)?;

ensure!(PoolMembers::<T>::contains_key(&who), Error::<T>::PoolMemberNotFound);
ClaimPermissions::<T>::mutate(who, |source| {
*source = permission;
});
Ok(())
}

/// `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<T>, other: T::AccountId) -> DispatchResult {
let signer = ensure_signed(origin)?;
Self::do_claim_payout(signer, other)
}
}

#[pallet::hooks]
Expand Down Expand Up @@ -2419,6 +2483,70 @@ impl<T: Config> Pallet<T> {
Ok(())
}

fn do_bond_extra(
signer: T::AccountId,
who: T::AccountId,
extra: BondExtra<BalanceOf<T>>,
) -> DispatchResult {
if signer != who {
ensure!(
matches!(
ClaimPermissions::<T>::get(&who),
ClaimPermission::PermissionlessAll | ClaimPermission::PermissionlessCompound
rossbulat marked this conversation as resolved.
Show resolved Hide resolved
),
Error::<T>::DoesNotHavePermission
);
ensure!(extra == BondExtra::Rewards, Error::<T>::BondExtraRestricted);
}

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::<T>::OverflowRisk)?;

Self::deposit_event(Event::<T>::Bonded {
member: who.clone(),
pool_id: member.pool_id,
bonded,
joined: false,
});
Self::put_member_with_pools(&who, member, bonded_pool, reward_pool);

Ok(())
}

fn do_claim_payout(signer: T::AccountId, who: T::AccountId) -> DispatchResult {
if signer != who {
ensure!(
matches!(
ClaimPermissions::<T>::get(&who),
ClaimPermission::PermissionlessAll | ClaimPermission::PermissionlessWithdraw
),
Error::<T>::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.
Expand Down
Loading