Skip to content

Commit

Permalink
permissionless bond_extra in nomination pools (paritytech#12608)
Browse files Browse the repository at this point in the history 
* create enum

* logic check

* add benchmarks

* -enum

* update

* bond extra other

* update

* update

* update

* cargo fmt

* Permissioned

* update

* cargo fmt

* update

* update index

* doc update

Co-authored-by: Ankan <10196091+Ank4n@users.noreply.github.com>

* doc update

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* cargo fmt

* bond_extra auto compound

* bond_extra_other

* Apply suggestions from code review

* Fixes from kian

* updates docs & test

* Update frame/nomination-pools/src/lib.rs

* Update frame/nomination-pools/src/lib.rs

* Update frame/nomination-pools/src/lib.rs

* Update frame/nomination-pools/src/lib.rs

* Update frame/nomination-pools/src/lib.rs

* Update frame/nomination-pools/src/tests.rs

* Update frame/nomination-pools/src/lib.rs

* Update frame/nomination-pools/src/tests.rs

* Update frame/nomination-pools/src/tests.rs

* Update frame/nomination-pools/src/tests.rs

* Update frame/nomination-pools/src/tests.rs

* Update frame/nomination-pools/src/tests.rs

* Update frame/nomination-pools/src/lib.rs

* Update frame/nomination-pools/src/tests.rs

* fixes + fmt

* expand ClaimPermissions + add benchmarks

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_nomination_pools

* tidy up claim payout benches

* fix

* + test: claim_payout_other_works

* comments, rename to set_claim_permission

* fix comment

* remove ClaimPermission on leave pool

* fix test

* ".git/.scripts/commands/fmt/fmt.sh"

* + test for ClaimPermissions::remove()

* impl can_bond_extra & can_claim_payout

---------

Co-authored-by: Ankan <10196091+Ank4n@users.noreply.github.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: kianenigma <kian@parity.io>
Co-authored-by: parity-processbot <>
Co-authored-by: Ross Bulat <ross@parity.io>
  • Loading branch information
@Doordashcon @Ank4n
@kianenigma @ark0f
5 people authored and ark0f committed Feb 27, 2023
1 parent cec0a35 commit f7f3d96
Show file tree
Hide file tree
Showing 4 changed files with 567 additions and 142 deletions.
46 changes: 40 additions & 6 deletions frame/nomination-pools/benchmarking/src/lib.rs
View file
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
214 changes: 177 additions & 37 deletions frame/nomination-pools/src/lib.rs
View file
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,43 @@ 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 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
}
}

/// 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 @@ -1313,6 +1354,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 @@ -1470,6 +1516,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 @@ -1560,44 +1608,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 @@ -1606,16 +1628,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 @@ -1715,7 +1734,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 @@ -1840,6 +1858,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 @@ -2114,6 +2135,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 @@ -2398,6 +2480,64 @@ 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!(
ClaimPermissions::<T>::get(&who).can_bond_extra(),
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!(
ClaimPermissions::<T>::get(&who).can_claim_payout(),
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

0 comments on commit f7f3d96

Please sign in to comment.