diff --git a/frame/bags-list/src/benchmarks.rs b/frame/bags-list/src/benchmarks.rs index b94a97093d001..573b467dc4586 100644 --- a/frame/bags-list/src/benchmarks.rs +++ b/frame/bags-list/src/benchmarks.rs @@ -175,10 +175,10 @@ frame_benchmarking::benchmarks! { vec![heavier, lighter, heavier_prev, heavier_next] ) } -} -frame_benchmarking::impl_benchmark_test_suite!( - Pallet, - crate::mock::ExtBuilder::default().skip_genesis_ids().build(), - crate::mock::Runtime -); + impl_benchmark_test_suite!( + Pallet, + crate::mock::ExtBuilder::default().skip_genesis_ids().build(), + crate::mock::Runtime + ); +} diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index 43d4ff61e34fd..18a7e802945c7 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -11,7 +11,7 @@ use frame_election_provider_support::SortedListProvider; use frame_support::{ensure, traits::Get}; use frame_system::RawOrigin as Origin; use pallet_nomination_pools::{ - BalanceOf, BondedPoolInner, BondedPools, ConfigOp, Delegators, MaxDelegators, + BalanceOf, BondExtra, BondedPoolInner, BondedPools, ConfigOp, Delegators, MaxDelegators, MaxDelegatorsPerPool, MaxPools, Metadata, MinCreateBond, MinJoinBond, Pallet as Pools, PoolRoles, PoolState, RewardPools, SubPoolsStorage, }; @@ -75,9 +75,11 @@ fn vote_to_balance( vote.try_into().map_err(|_| "could not convert u64 to Balance") } +#[allow(unused)] struct ListScenario { /// Stash/Controller that is expected to be moved. origin1: T::AccountId, + creator1: T::AccountId, dest_weight: BalanceOf, origin1_delegator: Option, } @@ -109,7 +111,7 @@ impl ListScenario { sp_std::mem::forget(i); // Create accounts with the origin weight - let (_, pool_origin1) = create_pool_account::(USER_SEED + 1, origin_weight); + let (pool_creator1, pool_origin1) = create_pool_account::(USER_SEED + 1, origin_weight); T::StakingInterface::nominate( pool_origin1.clone(), // NOTE: these don't really need to be validators. @@ -144,7 +146,12 @@ impl ListScenario { assert_eq!(vote_to_balance::(weight_of(&pool_origin2)).unwrap(), origin_weight); assert_eq!(vote_to_balance::(weight_of(&pool_dest1)).unwrap(), dest_weight); - Ok(ListScenario { origin1: pool_origin1, dest_weight, origin1_delegator: None }) + Ok(ListScenario { + origin1: pool_origin1, + creator1: pool_creator1, + dest_weight, + origin1_delegator: None, + }) } fn add_joiner(mut self, amount: BalanceOf) -> Self { @@ -214,6 +221,43 @@ frame_benchmarking::benchmarks! { ); } + bond_extra_transfer { + let origin_weight = pallet_nomination_pools::MinCreateBond::::get() + .max(CurrencyOf::::minimum_balance()) + * 2u32.into(); + let scenario = ListScenario::::new(origin_weight, true)?; + let extra = scenario.dest_weight.clone() - origin_weight; + + // creator of the src pool will bond-extra, bumping itself to dest bag. + + }: bond_extra(Origin::Signed(scenario.creator1.clone()), BondExtra::FreeBalance(extra)) + verify { + assert_eq!( + T::StakingInterface::active_stake(&scenario.origin1).unwrap(), + scenario.dest_weight + ); + } + + bond_extra_reward { + let origin_weight = pallet_nomination_pools::MinCreateBond::::get() + .max(CurrencyOf::::minimum_balance()) + * 2u32.into(); + let scenario = ListScenario::::new(origin_weight, true)?; + let extra = (scenario.dest_weight.clone() - origin_weight).max(CurrencyOf::::minimum_balance()); + + // 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(Origin::Signed(scenario.creator1.clone()), BondExtra::Rewards) + verify { + assert_eq!( + T::StakingInterface::active_stake(&scenario.origin1).unwrap(), + scenario.dest_weight + ); + } + claim_payout { let origin_weight = pallet_nomination_pools::MinCreateBond::::get().max(CurrencyOf::::minimum_balance()) * 2u32.into(); let ed = CurrencyOf::::minimum_balance(); @@ -560,12 +604,10 @@ frame_benchmarking::benchmarks! { assert_eq!(MaxDelegators::::get(), Some(u32::MAX)); assert_eq!(MaxDelegatorsPerPool::::get(), Some(u32::MAX)); } -} - -// TODO: consider benchmarking slashing logic with pools -frame_benchmarking::impl_benchmark_test_suite!( - Pallet, - crate::mock::new_test_ext(), - crate::mock::Runtime -); + impl_benchmark_test_suite!( + Pallet, + crate::mock::new_test_ext(), + crate::mock::Runtime + ); +} diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index fe5d2726cd93a..342d27eefa8d2 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -333,10 +333,21 @@ pub enum ConfigOp { Remove, } -/// Extrinsics that bond some funds to the pool. -enum PoolBond { +/// The type of binding that can happen to a pool. +enum BondType { + /// Someone is bonding into the pool upon creation. Create, - Join, + /// Someone is adding more funds later to this pool. + Later, +} + +/// How to increase the bond of a delegator. +#[derive(Encode, Decode, Clone, Copy, Debug, PartialEq, Eq, TypeInfo)] +pub enum BondExtra { + /// Take from the free balance. + FreeBalance(Balance), + /// Take the entire amount from the accumulated rewards. + Rewards, } /// The type of account being created. @@ -658,30 +669,26 @@ impl BondedPool { } } - /// Try to transfer a delegators funds to the bonded pool account and then try to bond them. - /// Additionally, this increments the delegator counter for the pool. + /// Bond exactly `amount` from `who`'s funds into this pool. /// - /// # Warning + /// If the bond type is `Create`, `StakingInterface::bond` is called, and `who` + /// is allowed to be killed. Otherwise, `StakingInterface::bond_extra` is called and `who` + /// cannot be killed. /// - /// This must only be used inside of transactional extrinsic, as funds are transferred prior to - /// attempting a fallible bond. - fn try_bond_delegator( + /// Returns `Ok(points_issues)`, `Err` otherwise. + fn try_bond_funds( &mut self, who: &T::AccountId, amount: BalanceOf, - ty: PoolBond, + ty: BondType, ) -> Result, DispatchError> { - self.try_inc_delegators()?; - - // Transfer the funds to be bonded from `who` to the pools account so the pool can then - // go bond them. T::Currency::transfer( &who, &self.bonded_account(), amount, match ty { - PoolBond::Create => ExistenceRequirement::AllowDeath, - PoolBond::Join => ExistenceRequirement::KeepAlive, + BondType::Create => ExistenceRequirement::AllowDeath, + BondType::Later => ExistenceRequirement::KeepAlive, }, )?; // We must calculate the points issued *before* we bond who's funds, else points:balance @@ -689,9 +696,8 @@ impl BondedPool { let points_issued = self.issue(amount); match ty { - PoolBond::Create => T::StakingInterface::bond( + BondType::Create => T::StakingInterface::bond( self.bonded_account(), - // We make the stash and controller the same for simplicity self.bonded_account(), amount, self.reward_account(), @@ -699,7 +705,7 @@ impl BondedPool { // The pool should always be created in such a way its in a state to bond extra, but if // the active balance is slashed below the minimum bonded or the account cannot be // found, we exit early. - PoolBond::Join => T::StakingInterface::bond_extra(self.bonded_account(), amount)?, + BondType::Later => T::StakingInterface::bond_extra(self.bonded_account(), amount)?, } Ok(points_issued) @@ -720,7 +726,10 @@ impl BondedPool { fn set_state(&mut self, state: PoolState) { if self.state != state { self.state = state; - Pallet::::deposit_event(Event::::State { pool_id: self.id, new_state: state }); + Pallet::::deposit_event(Event::::StateChanged { + pool_id: self.id, + new_state: state, + }); }; } } @@ -853,6 +862,7 @@ impl Get for TotalUnbondingPools { #[frame_support::pallet] pub mod pallet { use super::*; + use frame_support::transactional; use frame_system::{ensure_signed, pallet_prelude::*}; #[pallet::pallet] @@ -990,16 +1000,24 @@ pub mod pallet { } } + /// Events of this pallet. #[pallet::event] #[pallet::generate_deposit(pub(crate) fn deposit_event)] pub enum Event { + /// A pool has been created. Created { depositor: T::AccountId, pool_id: PoolId }, - Joined { delegator: T::AccountId, pool_id: PoolId, bonded: BalanceOf }, + /// A delegator has became bonded in a pool. + Bonded { delegator: T::AccountId, pool_id: PoolId, bonded: BalanceOf, joined: bool }, + /// A payout has been made to a delegator. PaidOut { delegator: T::AccountId, pool_id: PoolId, payout: BalanceOf }, + /// A delegator has unbonded from their pool. Unbonded { delegator: T::AccountId, pool_id: PoolId, amount: BalanceOf }, + /// A delegator has withdrawn from their pool. Withdrawn { delegator: T::AccountId, pool_id: PoolId, amount: BalanceOf }, + /// A pool has been destroyed. Destroyed { pool_id: PoolId }, - State { pool_id: PoolId, new_state: PoolState }, + /// The state of a pool has changed + StateChanged { pool_id: PoolId, new_state: PoolState }, } #[pallet::error] @@ -1085,7 +1103,8 @@ pub mod pallet { let reward_pool = RewardPool::::get_and_update(pool_id) .defensive_ok_or_else(|| Error::::RewardPoolNotFound)?; - let points_issued = bonded_pool.try_bond_delegator(&who, amount, PoolBond::Join)?; + bonded_pool.try_inc_delegators()?; + let points_issued = bonded_pool.try_bond_funds(&who, amount, BondType::Later)?; Delegators::insert( who.clone(), @@ -1103,7 +1122,52 @@ pub mod pallet { }, ); bonded_pool.put(); - Self::deposit_event(Event::::Joined { delegator: who, pool_id, bonded: amount }); + Self::deposit_event(Event::::Bonded { + delegator: who, + pool_id, + bonded: amount, + joined: true, + }); + + Ok(()) + } + + /// Bond `extra` more funds from `origin` into the pool to which they already belong. + /// + /// Additional funds can come from either the free balance of the account, of from the + /// accumulated rewards, see [`BondExtra`]. + // 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::weight(0)] + #[transactional] + pub fn bond_extra(origin: OriginFor, extra: BondExtra>) -> DispatchResult { + let who = ensure_signed(origin)?; + let (mut delegator, mut bonded_pool, mut reward_pool) = + Self::get_delegator_with_pools(&who)?; + + let (points_issued, bonded) = match extra { + BondExtra::FreeBalance(amount) => + (bonded_pool.try_bond_funds(&who, amount, BondType::Later)?, amount), + BondExtra::Rewards => { + let claimed = Self::do_reward_payout( + &who, + &mut delegator, + &mut bonded_pool, + &mut reward_pool, + )?; + (bonded_pool.try_bond_funds(&who, claimed, BondType::Later)?, claimed) + }, + }; + delegator.points = delegator.points.saturating_add(points_issued); + + Self::deposit_event(Event::::Bonded { + delegator: who.clone(), + pool_id: delegator.pool_id, + bonded, + joined: false, + }); + Self::put_delegator_with_pools(&who, delegator, bonded_pool, reward_pool); Ok(()) } @@ -1117,22 +1181,13 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::claim_payout())] pub fn claim_payout(origin: OriginFor) -> DispatchResult { let who = ensure_signed(origin)?; - let mut delegator = Delegators::::get(&who).ok_or(Error::::DelegatorNotFound)?; - let mut bonded_pool = BondedPool::::get(delegator.pool_id) - .defensive_ok_or_else(|| Error::::PoolNotFound)?; - let was_destroying = bonded_pool.is_destroying(); - - Self::do_reward_payout(who.clone(), &mut delegator, &mut bonded_pool)?; + let (mut delegator, mut bonded_pool, mut reward_pool) = + Self::get_delegator_with_pools(&who)?; - if bonded_pool.is_destroying() && !was_destroying { - Self::deposit_event(Event::::State { - pool_id: delegator.pool_id, - new_state: PoolState::Destroying, - }); - } - bonded_pool.put(); - Delegators::insert(who, delegator); + let _ = + Self::do_reward_payout(&who, &mut delegator, &mut bonded_pool, &mut reward_pool)?; + Self::put_delegator_with_pools(&who, delegator, bonded_pool, reward_pool); Ok(()) } @@ -1164,16 +1219,20 @@ pub mod pallet { delegator_account: T::AccountId, ) -> DispatchResult { let caller = ensure_signed(origin)?; - let mut delegator = - Delegators::::get(&delegator_account).ok_or(Error::::DelegatorNotFound)?; - let mut bonded_pool = BondedPool::::get(delegator.pool_id) - .defensive_ok_or_else(|| Error::::PoolNotFound)?; + let (mut delegator, mut bonded_pool, mut reward_pool) = + Self::get_delegator_with_pools(&delegator_account)?; + bonded_pool.ok_to_unbond_other_with(&caller, &delegator_account, &delegator)?; // Claim the the payout prior to unbonding. Once the user is unbonding their points // no longer exist in the bonded pool and thus they can no longer claim their payouts. // It is not strictly necessary to claim the rewards, but we do it here for UX. - Self::do_reward_payout(delegator_account.clone(), &mut delegator, &mut bonded_pool)?; + Self::do_reward_payout( + &delegator_account, + &mut delegator, + &mut bonded_pool, + &mut reward_pool, + )?; let balance_to_unbond = bonded_pool.balance_to_unbond(delegator.points); @@ -1217,9 +1276,8 @@ pub mod pallet { }); // Now that we know everything has worked write the items to storage. - bonded_pool.put(); SubPoolsStorage::insert(&delegator.pool_id, sub_pools); - Delegators::insert(delegator_account, delegator); + Self::put_delegator_with_pools(&delegator_account, delegator, bonded_pool, reward_pool); Ok(()) } @@ -1423,7 +1481,8 @@ pub mod pallet { PoolRoles { root, nominator, state_toggler, depositor: who.clone() }, ); - let points = bonded_pool.try_bond_delegator(&who, amount, PoolBond::Create)?; + bonded_pool.try_inc_delegators()?; + let points = bonded_pool.try_bond_funds(&who, amount, BondType::Create)?; T::Currency::transfer( &who, @@ -1577,7 +1636,6 @@ pub mod pallet { impl Pallet { /// Create the main, bonded account of a pool with the given id. - pub fn create_bonded_account(id: PoolId) -> T::AccountId { T::PalletId::get().into_sub_account((AccountType::Bonded, id)) } @@ -1589,6 +1647,31 @@ impl Pallet { T::PalletId::get().into_sub_account((AccountType::Reward, id)) } + /// Get the delegator with their associated bonded and reward pool. + fn get_delegator_with_pools( + who: &T::AccountId, + ) -> Result<(Delegator, BondedPool, RewardPool), Error> { + let delegator = Delegators::::get(&who).ok_or(Error::::DelegatorNotFound)?; + let bonded_pool = + BondedPool::::get(delegator.pool_id).defensive_ok_or(Error::::PoolNotFound)?; + let reward_pool = + RewardPools::::get(delegator.pool_id).defensive_ok_or(Error::::PoolNotFound)?; + Ok((delegator, bonded_pool, reward_pool)) + } + + /// Persist the delegator with their associated bonded and reward pool into storage, consuming + /// all of them. + fn put_delegator_with_pools( + delegator_account: &T::AccountId, + delegator: Delegator, + bonded_pool: BondedPool, + reward_pool: RewardPool, + ) { + bonded_pool.put(); + RewardPools::insert(delegator.pool_id, reward_pool); + Delegators::::insert(delegator_account, delegator); + } + /// Calculate the number of points to issue from a pool as `(current_points / current_balance) * /// new_funds` except for some zero edge cases; see logic and tests for details. fn points_to_issue( @@ -1717,21 +1800,19 @@ impl Pallet { } /// If the delegator has some rewards, transfer a payout from the reward pool to the delegator. - /// - /// # Note - /// This will persist updates for the reward pool to storage. But it will *not* persist updates - /// to the `delegator` or `bonded_pool` to storage, that is the responsibility of the caller. + // Emits events and potentially modifies pool state if any arithmetic saturates, but does + // not persist any of the mutable inputs to storage. fn do_reward_payout( - delegator_account: T::AccountId, + delegator_account: &T::AccountId, delegator: &mut Delegator, bonded_pool: &mut BondedPool, - ) -> DispatchResult { + reward_pool: &mut RewardPool, + ) -> Result, DispatchError> { debug_assert_eq!(delegator.pool_id, bonded_pool.id); - let mut reward_pool = RewardPools::::get(delegator.pool_id) - .defensive_ok_or_else(|| Error::::RewardPoolNotFound)?; + let was_destroying = bonded_pool.is_destroying(); let delegator_payout = - Self::calculate_delegator_payout(bonded_pool, &mut reward_pool, delegator)?; + Self::calculate_delegator_payout(bonded_pool, reward_pool, delegator)?; // Transfer payout to the delegator. T::Currency::transfer( @@ -1742,15 +1823,19 @@ impl Pallet { )?; Self::deposit_event(Event::::PaidOut { - delegator: delegator_account, + delegator: delegator_account.clone(), pool_id: delegator.pool_id, payout: delegator_payout, }); - // Write the reward pool to storage - RewardPools::insert(&delegator.pool_id, reward_pool); + if bonded_pool.is_destroying() && !was_destroying { + Self::deposit_event(Event::::StateChanged { + pool_id: delegator.pool_id, + new_state: PoolState::Destroying, + }); + } - Ok(()) + Ok(delegator_payout) } /// Ensure the correctness of the state of this pallet. diff --git a/frame/nomination-pools/src/mock.rs b/frame/nomination-pools/src/mock.rs index 08ed41e58369b..28b00c3488678 100644 --- a/frame/nomination-pools/src/mock.rs +++ b/frame/nomination-pools/src/mock.rs @@ -219,6 +219,9 @@ impl ExtBuilder { let mut ext = sp_io::TestExternalities::from(storage); ext.execute_with(|| { + // for events to be deposited. + frame_system::Pallet::::set_block_number(1); + // make a pool let amount_to_bond = ::StakingInterface::minimum_bond(); Balances::make_free_balance_be(&10, amount_to_bond * 2); @@ -250,6 +253,22 @@ pub(crate) fn unsafe_set_state(pool_id: PoolId, state: PoolState) -> Result<(), }) } +parameter_types! { + static ObservedEvents: usize = 0; +} + +/// All events of this pallet. +pub(crate) fn pool_events_since_last_call() -> Vec> { + let events = System::events() + .into_iter() + .map(|r| r.event) + .filter_map(|e| if let Event::Pools(inner) = e { Some(inner) } else { None }) + .collect::>(); + let already_seen = ObservedEvents::get(); + ObservedEvents::set(events.len()); + events.into_iter().skip(already_seen).collect() +} + #[cfg(test)] mod test { use super::*; diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index ee3abdb6cb031..8d690540f468a 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -599,6 +599,8 @@ mod join { } mod claim_payout { + use frame_support::storage::with_transaction; + use super::*; fn del(points: Balance, reward_pool_total_earnings: Balance) -> Delegator { @@ -794,38 +796,56 @@ mod claim_payout { #[test] fn do_reward_payout_correctly_sets_pool_state_to_destroying() { ExtBuilder::default().build_and_execute(|| { - let mut bonded_pool = BondedPool::::get(1).unwrap(); - let mut reward_pool = RewardPools::::get(1).unwrap(); - let mut delegator = Delegators::::get(10).unwrap(); + with_transaction(|| { + let mut bonded_pool = BondedPool::::get(1).unwrap(); + let mut reward_pool = RewardPools::::get(1).unwrap(); + let mut delegator = Delegators::::get(10).unwrap(); - // -- reward_pool.total_earnings saturates + // -- reward_pool.total_earnings saturates - // Given - Balances::make_free_balance_be(&default_reward_account(), Balance::MAX); + // Given + Balances::make_free_balance_be(&default_reward_account(), Balance::MAX); - // When - assert_ok!(Pools::do_reward_payout(10, &mut delegator, &mut bonded_pool)); + // When + assert_ok!(Pools::do_reward_payout( + &10, + &mut delegator, + &mut bonded_pool, + &mut reward_pool + )); - // Then - assert!(bonded_pool.is_destroying()); + // Then + assert!(bonded_pool.is_destroying()); + + storage::TransactionOutcome::Rollback(()) + }); // -- current_points saturates (reward_pool.points + new_earnings * bonded_pool.points) + with_transaction(|| { + // Given + let mut bonded_pool = BondedPool::::get(1).unwrap(); + let mut reward_pool = RewardPools::::get(1).unwrap(); + let mut delegator = Delegators::::get(10).unwrap(); + // Force new_earnings * bonded_pool.points == 100 + Balances::make_free_balance_be(&default_reward_account(), 5 + 10); + assert_eq!(bonded_pool.points, 10); + // Force reward_pool.points == U256::MAX - new_earnings * bonded_pool.points + reward_pool.points = U256::MAX - U256::from(100u32); + RewardPools::::insert(1, reward_pool.clone()); - // Given - let mut bonded_pool = BondedPool::::get(1).unwrap(); - let mut delegator = Delegators::::get(10).unwrap(); - // Force new_earnings * bonded_pool.points == 100 - Balances::make_free_balance_be(&default_reward_account(), 5 + 10); - assert_eq!(bonded_pool.points, 10); - // Force reward_pool.points == U256::MAX - new_earnings * bonded_pool.points - reward_pool.points = U256::MAX - U256::from(100u32); - RewardPools::::insert(1, reward_pool.clone()); + // When + assert_ok!(Pools::do_reward_payout( + &10, + &mut delegator, + &mut bonded_pool, + &mut reward_pool + )); - // When - assert_ok!(Pools::do_reward_payout(10, &mut delegator, &mut bonded_pool)); + // Then + assert!(bonded_pool.is_destroying()); - // Then - assert!(bonded_pool.is_destroying()); + storage::TransactionOutcome::Rollback(()) + }) }); } @@ -1158,6 +1178,7 @@ mod claim_payout { .add_delegators(vec![(40, 40), (50, 50)]) .build_and_execute(|| { let mut bonded_pool = BondedPool::::get(1).unwrap(); + let mut reward_pool = RewardPools::::get(1).unwrap(); let ed = Balances::minimum_balance(); // Given the bonded pool has 100 points @@ -1174,39 +1195,48 @@ mod claim_payout { let mut del_50 = Delegators::get(50).unwrap(); // When - assert_ok!(Pools::do_reward_payout(10, &mut del_10, &mut bonded_pool)); + assert_ok!(Pools::do_reward_payout( + &10, + &mut del_10, + &mut bonded_pool, + &mut reward_pool + )); // Then // Expect a payout of 10: (10 del virtual points / 100 pool points) * 100 pool // balance assert_eq!(del_10, del(10, 100)); - assert_eq!( - RewardPools::::get(&1).unwrap(), - rew(90, 100 * 100 - 100 * 10, 100) - ); + assert_eq!(reward_pool, rew(90, 100 * 100 - 100 * 10, 100)); assert_eq!(Balances::free_balance(&10), 10); assert_eq!(Balances::free_balance(&default_reward_account()), ed + 90); // When - assert_ok!(Pools::do_reward_payout(40, &mut del_40, &mut bonded_pool)); + assert_ok!(Pools::do_reward_payout( + &40, + &mut del_40, + &mut bonded_pool, + &mut reward_pool + )); // Then // Expect payout 40: (400 del virtual points / 900 pool points) * 90 pool balance assert_eq!(del_40, del(40, 100)); - assert_eq!( - RewardPools::::get(&1).unwrap(), - rew(50, 9_000 - 100 * 40, 100) - ); + assert_eq!(reward_pool, rew(50, 9_000 - 100 * 40, 100)); assert_eq!(Balances::free_balance(&40), 40); assert_eq!(Balances::free_balance(&default_reward_account()), ed + 50); // When - assert_ok!(Pools::do_reward_payout(50, &mut del_50, &mut bonded_pool)); + assert_ok!(Pools::do_reward_payout( + &50, + &mut del_50, + &mut bonded_pool, + &mut reward_pool + )); // Then // Expect payout 50: (50 del virtual points / 50 pool points) * 50 pool balance assert_eq!(del_50, del(50, 100)); - assert_eq!(RewardPools::::get(&1).unwrap(), rew(0, 0, 100)); + assert_eq!(reward_pool, rew(0, 0, 100)); assert_eq!(Balances::free_balance(&50), 50); assert_eq!(Balances::free_balance(&default_reward_account()), ed + 0); @@ -1214,23 +1244,33 @@ mod claim_payout { Balances::make_free_balance_be(&default_reward_account(), ed + 50); // When - assert_ok!(Pools::do_reward_payout(10, &mut del_10, &mut bonded_pool)); + assert_ok!(Pools::do_reward_payout( + &10, + &mut del_10, + &mut bonded_pool, + &mut reward_pool + )); // Then // Expect payout 5: (500 del virtual points / 5,000 pool points) * 50 pool balance assert_eq!(del_10, del(10, 150)); - assert_eq!(RewardPools::::get(&1).unwrap(), rew(45, 5_000 - 50 * 10, 150)); + assert_eq!(reward_pool, rew(45, 5_000 - 50 * 10, 150)); assert_eq!(Balances::free_balance(&10), 10 + 5); assert_eq!(Balances::free_balance(&default_reward_account()), ed + 45); // When - assert_ok!(Pools::do_reward_payout(40, &mut del_40, &mut bonded_pool)); + assert_ok!(Pools::do_reward_payout( + &40, + &mut del_40, + &mut bonded_pool, + &mut reward_pool + )); // Then // Expect payout 20: (2,000 del virtual points / 4,500 pool points) * 45 pool // balance assert_eq!(del_40, del(40, 150)); - assert_eq!(RewardPools::::get(1).unwrap(), rew(25, 4_500 - 50 * 40, 150)); + assert_eq!(reward_pool, rew(25, 4_500 - 50 * 40, 150)); assert_eq!(Balances::free_balance(&40), 40 + 20); assert_eq!(Balances::free_balance(&default_reward_account()), ed + 25); @@ -1239,14 +1279,19 @@ mod claim_payout { assert_eq!(Balances::free_balance(&default_reward_account()), ed + 75); // When - assert_ok!(Pools::do_reward_payout(50, &mut del_50, &mut bonded_pool)); + assert_ok!(Pools::do_reward_payout( + &50, + &mut del_50, + &mut bonded_pool, + &mut reward_pool + )); // Then // We expect a payout of 50: (5,000 del virtual points / 7,5000 pool points) * 75 // pool balance assert_eq!(del_50, del(50, 200)); assert_eq!( - RewardPools::::get(1).unwrap(), + reward_pool, rew( 25, // old pool points + points from new earnings - del points. @@ -1261,12 +1306,17 @@ mod claim_payout { assert_eq!(Balances::free_balance(&default_reward_account()), ed + 25); // When - assert_ok!(Pools::do_reward_payout(10, &mut del_10, &mut bonded_pool)); + assert_ok!(Pools::do_reward_payout( + &10, + &mut del_10, + &mut bonded_pool, + &mut reward_pool + )); // Then // We expect a payout of 5 assert_eq!(del_10, del(10, 200)); - assert_eq!(RewardPools::::get(1).unwrap(), rew(20, 2_500 - 10 * 50, 200)); + assert_eq!(reward_pool, rew(20, 2_500 - 10 * 50, 200)); assert_eq!(Balances::free_balance(&10), 15 + 5); assert_eq!(Balances::free_balance(&default_reward_account()), ed + 20); @@ -1275,13 +1325,18 @@ mod claim_payout { assert_eq!(Balances::free_balance(&default_reward_account()), ed + 420); // When - assert_ok!(Pools::do_reward_payout(10, &mut del_10, &mut bonded_pool)); + assert_ok!(Pools::do_reward_payout( + &10, + &mut del_10, + &mut bonded_pool, + &mut reward_pool + )); // Then // We expect a payout of 40 assert_eq!(del_10, del(10, 600)); assert_eq!( - RewardPools::::get(1).unwrap(), + reward_pool, rew( 380, // old pool points + points from new earnings - del points @@ -1300,40 +1355,49 @@ mod claim_payout { assert_eq!(Balances::free_balance(&default_reward_account()), ed + 400); // When - assert_ok!(Pools::do_reward_payout(10, &mut del_10, &mut bonded_pool)); + assert_ok!(Pools::do_reward_payout( + &10, + &mut del_10, + &mut bonded_pool, + &mut reward_pool + )); // Then // Expect a payout of 2: (200 del virtual points / 38,000 pool points) * 400 pool // balance assert_eq!(del_10, del(10, 620)); - assert_eq!( - RewardPools::::get(1).unwrap(), - rew(398, (38_000 + 20 * 100) - 10 * 20, 620) - ); + assert_eq!(reward_pool, rew(398, (38_000 + 20 * 100) - 10 * 20, 620)); assert_eq!(Balances::free_balance(&10), 60 + 2); assert_eq!(Balances::free_balance(&default_reward_account()), ed + 398); // When - assert_ok!(Pools::do_reward_payout(40, &mut del_40, &mut bonded_pool)); + assert_ok!(Pools::do_reward_payout( + &40, + &mut del_40, + &mut bonded_pool, + &mut reward_pool + )); // Then // Expect a payout of 188: (18,800 del virtual points / 39,800 pool points) * 399 // pool balance assert_eq!(del_40, del(40, 620)); - assert_eq!( - RewardPools::::get(1).unwrap(), - rew(210, 39_800 - 40 * 470, 620) - ); + assert_eq!(reward_pool, rew(210, 39_800 - 40 * 470, 620)); assert_eq!(Balances::free_balance(&40), 60 + 188); assert_eq!(Balances::free_balance(&default_reward_account()), ed + 210); // When - assert_ok!(Pools::do_reward_payout(50, &mut del_50, &mut bonded_pool)); + assert_ok!(Pools::do_reward_payout( + &50, + &mut del_50, + &mut bonded_pool, + &mut reward_pool + )); // Then // Expect payout of 210: (21,000 / 21,000) * 210 assert_eq!(del_50, del(50, 620)); - assert_eq!(RewardPools::::get(1).unwrap(), rew(0, 21_000 - 50 * 420, 620)); + assert_eq!(reward_pool, rew(0, 21_000 - 50 * 420, 620)); assert_eq!(Balances::free_balance(&50), 100 + 210); assert_eq!(Balances::free_balance(&default_reward_account()), ed + 0); }); @@ -2453,3 +2517,139 @@ mod set_configs { }); } } + +mod bond_extra { + use super::*; + use crate::Event; + + #[test] + fn bond_extra_from_free_balance_creator() { + ExtBuilder::default().build_and_execute(|| { + // 10 is the owner and a delegator in pool 1, give them some more funds. + Balances::make_free_balance_be(&10, 100); + + // given + assert_eq!(Delegators::::get(10).unwrap().points, 10); + assert_eq!(BondedPools::::get(1).unwrap().points, 10); + assert_eq!(Balances::free_balance(10), 100); + + // when + assert_ok!(Pools::bond_extra(Origin::signed(10), BondExtra::FreeBalance(10))); + + // then + assert_eq!(Balances::free_balance(10), 90); + assert_eq!(Delegators::::get(10).unwrap().points, 20); + assert_eq!(BondedPools::::get(1).unwrap().points, 20); + + assert_eq!( + pool_events_since_last_call(), + vec![ + Event::Created { depositor: 10, pool_id: 1 }, + Event::Bonded { delegator: 10, pool_id: 1, bonded: 10, joined: false } + ] + ); + + // when + assert_ok!(Pools::bond_extra(Origin::signed(10), BondExtra::FreeBalance(20))); + + // then + assert_eq!(Balances::free_balance(10), 70); + assert_eq!(Delegators::::get(10).unwrap().points, 40); + assert_eq!(BondedPools::::get(1).unwrap().points, 40); + + assert_eq!( + pool_events_since_last_call(), + vec![Event::Bonded { delegator: 10, pool_id: 1, bonded: 20, joined: false }] + ); + }) + } + + #[test] + fn bond_extra_from_rewards_creator() { + ExtBuilder::default().build_and_execute(|| { + // put some money in the reward account, all of which will belong to 10 as the only + // delegator of the pool. + Balances::make_free_balance_be(&default_reward_account(), 7); + // ... if which only 2 is claimable to make sure the reward account does not die. + let claimable_reward = 7 - ExistentialDeposit::get(); + + // given + assert_eq!(Delegators::::get(10).unwrap().points, 10); + assert_eq!(BondedPools::::get(1).unwrap().points, 10); + assert_eq!(Balances::free_balance(10), 5); + + // when + assert_ok!(Pools::bond_extra(Origin::signed(10), BondExtra::Rewards)); + + // then + assert_eq!(Balances::free_balance(10), 5); + assert_eq!(Delegators::::get(10).unwrap().points, 10 + claimable_reward); + assert_eq!(BondedPools::::get(1).unwrap().points, 10 + claimable_reward); + + assert_eq!( + pool_events_since_last_call(), + vec![ + Event::Created { depositor: 10, pool_id: 1 }, + Event::PaidOut { delegator: 10, pool_id: 1, payout: claimable_reward }, + Event::Bonded { + delegator: 10, + pool_id: 1, + bonded: claimable_reward, + joined: false + } + ] + ); + }) + } + + #[test] + fn bond_extra_from_rewards_joiner() { + ExtBuilder::default().add_delegators(vec![(20, 20)]).build_and_execute(|| { + // put some money in the reward account, all of which will belong to 10 as the only + // delegator of the pool. + 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!(Delegators::::get(10).unwrap().points, 10); + assert_eq!(Delegators::::get(20).unwrap().points, 20); + assert_eq!(BondedPools::::get(1).unwrap().points, 30); + assert_eq!(Balances::free_balance(10), 5); + assert_eq!(Balances::free_balance(20), 20); + + // when + assert_ok!(Pools::bond_extra(Origin::signed(10), BondExtra::Rewards)); + + // then + assert_eq!(Balances::free_balance(10), 5); + // 10's share of the reward is 1/3, since they gave 10/30 of the total shares. + assert_eq!(Delegators::::get(10).unwrap().points, 10 + 1); + assert_eq!(BondedPools::::get(1).unwrap().points, 30 + 1); + + // when + assert_ok!(Pools::bond_extra(Origin::signed(20), BondExtra::Rewards)); + + // 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!(Delegators::::get(20).unwrap().points, 20 + 2); + assert_eq!(BondedPools::::get(1).unwrap().points, 30 + 3); + + assert_eq!( + pool_events_since_last_call(), + vec![ + Event::Created { depositor: 10, pool_id: 1 }, + Event::Bonded { delegator: 20, pool_id: 1, bonded: 20, joined: true }, + Event::PaidOut { delegator: 10, pool_id: 1, payout: 1 }, + Event::Bonded { delegator: 10, pool_id: 1, bonded: 1, joined: false }, + Event::PaidOut { delegator: 20, pool_id: 1, payout: 2 }, + Event::Bonded { delegator: 20, pool_id: 1, bonded: 2, joined: false } + ] + ); + }) + } +} diff --git a/frame/support/src/traits/misc.rs b/frame/support/src/traits/misc.rs index 8c61874003bce..7af08f9d703e7 100644 --- a/frame/support/src/traits/misc.rs +++ b/frame/support/src/traits/misc.rs @@ -129,9 +129,13 @@ pub trait DefensiveOption { /// if `None`, which should never happen. fn defensive_map_or_else U, F: FnOnce(T) -> U>(self, default: D, f: F) -> U; - /// Defensively transform this option to a result. + /// Defensively transform this option to a result, mapping `None` to the return value of an + /// error closure. fn defensive_ok_or_else E>(self, err: F) -> Result; + /// Defensively transform this option to a result, mapping `None` to a default value. + fn defensive_ok_or(self, err: E) -> Result; + /// Exactly the same as `map`, but it prints the appropriate warnings if the value being mapped /// is `None`. fn defensive_map U>(self, f: F) -> Option; @@ -284,6 +288,13 @@ impl DefensiveOption for Option { }) } + fn defensive_ok_or(self, err: E) -> Result { + self.ok_or_else(|| { + defensive!(); + err + }) + } + fn defensive_map U>(self, f: F) -> Option { match self { Some(inner) => Some(f(inner)),