From 025f0b7c3a6d2751ab9a0266a0b2f94952def873 Mon Sep 17 00:00:00 2001 From: Dino Pacandi Date: Tue, 17 Oct 2023 14:09:08 +0200 Subject: [PATCH] Refactoring finished --- pallets/dapp-staking-v3/src/test/mock.rs | 2 +- .../dapp-staking-v3/src/test/testing_utils.rs | 38 ++- pallets/dapp-staking-v3/src/test/tests.rs | 244 ++++++++++-------- .../dapp-staking-v3/src/test/tests_types.rs | 1 - pallets/dapp-staking-v3/src/types.rs | 11 +- 5 files changed, 161 insertions(+), 135 deletions(-) diff --git a/pallets/dapp-staking-v3/src/test/mock.rs b/pallets/dapp-staking-v3/src/test/mock.rs index ab88c11773..35196ce042 100644 --- a/pallets/dapp-staking-v3/src/test/mock.rs +++ b/pallets/dapp-staking-v3/src/test/mock.rs @@ -229,7 +229,7 @@ pub(crate) fn advance_to_next_period() { } /// Advance blocks until next period type has been reached. -pub(crate) fn _advance_to_next_period_type() { +pub(crate) fn advance_to_next_period_type() { let period_type = ActiveProtocolState::::get().period_type(); while ActiveProtocolState::::get().period_type() == period_type { run_for_blocks(1); diff --git a/pallets/dapp-staking-v3/src/test/testing_utils.rs b/pallets/dapp-staking-v3/src/test/testing_utils.rs index 6657e02c2b..1c328f852b 100644 --- a/pallets/dapp-staking-v3/src/test/testing_utils.rs +++ b/pallets/dapp-staking-v3/src/test/testing_utils.rs @@ -19,9 +19,8 @@ use crate::test::mock::*; use crate::types::*; use crate::{ - pallet as pallet_dapp_staking, ActiveProtocolState, BlockNumberFor, ContractStake, - CurrentEraInfo, DAppId, EraRewards, Event, IntegratedDApps, Ledger, NextDAppId, PeriodEnd, - PeriodEndInfo, StakeAmount, StakerInfo, + pallet::Config, ActiveProtocolState, BlockNumberFor, ContractStake, CurrentEraInfo, DAppId, + EraRewards, Event, IntegratedDApps, Ledger, NextDAppId, PeriodEnd, PeriodEndInfo, StakerInfo, }; use frame_support::{assert_ok, traits::Get}; @@ -36,23 +35,19 @@ pub(crate) struct MemorySnapshot { next_dapp_id: DAppId, current_era_info: EraInfo, integrated_dapps: HashMap< - ::SmartContract, + ::SmartContract, DAppInfo<::AccountId>, >, ledger: HashMap<::AccountId, AccountLedgerFor>, staker_info: HashMap< ( ::AccountId, - ::SmartContract, + ::SmartContract, ), SingularStakingInfo, >, - contract_stake: - HashMap<::SmartContract, ContractStakeAmountSeries>, - era_rewards: HashMap< - EraNumber, - EraRewardSpan<::EraRewardSpanLength>, - >, + contract_stake: HashMap<::SmartContract, ContractStakeAmountSeries>, + era_rewards: HashMap::EraRewardSpanLength>>, period_end: HashMap, } @@ -250,7 +245,7 @@ pub(crate) fn assert_unlock(account: AccountId, amount: Balance) { // When unlocking would take account below the minimum lock threshold, unlock everything let locked_amount = pre_ledger.active_locked_amount(); - let min_locked_amount = ::MinimumLockedAmount::get(); + let min_locked_amount = ::MinimumLockedAmount::get(); if locked_amount.saturating_sub(possible_unlock_amount) < min_locked_amount { locked_amount } else { @@ -506,7 +501,7 @@ pub(crate) fn assert_stake( amount, "Total staked amount must be equal to exactly the 'amount'" ); - assert!(amount >= ::MinimumStakeAmount::get()); + assert!(amount >= ::MinimumStakeAmount::get()); assert_eq!( post_staker_info.staked_amount(stake_period_type), amount, @@ -577,8 +572,7 @@ pub(crate) fn assert_unstake( let unstake_period = pre_snapshot.active_protocol_state.period_number(); let unstake_period_type = pre_snapshot.active_protocol_state.period_type(); - let minimum_stake_amount: Balance = - ::MinimumStakeAmount::get(); + let minimum_stake_amount: Balance = ::MinimumStakeAmount::get(); let is_full_unstake = pre_staker_info.total_staked_amount().saturating_sub(amount) < minimum_stake_amount; @@ -723,8 +717,8 @@ pub(crate) fn assert_unstake( pub(crate) fn assert_claim_staker_rewards(account: AccountId) { let pre_snapshot = MemorySnapshot::new(); let pre_ledger = pre_snapshot.ledger.get(&account).unwrap(); - let pre_total_issuance = ::Currency::total_issuance(); - let pre_free_balance = ::Currency::free_balance(&account); + let pre_total_issuance = ::Currency::total_issuance(); + let pre_free_balance = ::Currency::free_balance(&account); // Get the first eligible era for claiming rewards let first_claim_era = pre_ledger @@ -732,8 +726,7 @@ pub(crate) fn assert_claim_staker_rewards(account: AccountId) { .expect("Entry must exist, otherwise 'claim' is invalid."); // Get the apprropriate era rewards span for the 'first era' - let era_span_length: EraNumber = - ::EraRewardSpanLength::get(); + let era_span_length: EraNumber = ::EraRewardSpanLength::get(); let era_span_index = first_claim_era - (first_claim_era % era_span_length); let era_rewards_span = pre_snapshot .era_rewards @@ -820,14 +813,14 @@ pub(crate) fn assert_claim_staker_rewards(account: AccountId) { // Verify post state - let post_total_issuance = ::Currency::total_issuance(); + let post_total_issuance = ::Currency::total_issuance(); assert_eq!( post_total_issuance, pre_total_issuance + total_reward, "Total issuance must increase by the total reward amount." ); - let post_free_balance = ::Currency::free_balance(&account); + let post_free_balance = ::Currency::free_balance(&account); assert_eq!( post_free_balance, pre_free_balance + total_reward, @@ -881,8 +874,7 @@ pub(crate) fn required_number_of_reward_claims(account: AccountId) -> u32 { return 0; }; - let era_span_length: EraNumber = - ::EraRewardSpanLength::get(); + let era_span_length: EraNumber = ::EraRewardSpanLength::get(); let first = DappStaking::era_reward_span_index(range.0) .checked_div(era_span_length) .unwrap(); diff --git a/pallets/dapp-staking-v3/src/test/tests.rs b/pallets/dapp-staking-v3/src/test/tests.rs index 30d34d5543..bbae432895 100644 --- a/pallets/dapp-staking-v3/src/test/tests.rs +++ b/pallets/dapp-staking-v3/src/test/tests.rs @@ -19,8 +19,8 @@ use crate::test::mock::*; use crate::test::testing_utils::*; use crate::{ - pallet as pallet_dapp_staking, ActiveProtocolState, DAppId, EraNumber, EraRewards, Error, - IntegratedDApps, Ledger, NextDAppId, PeriodType, StakerInfo, + pallet::Config, ActiveProtocolState, DAppId, EraNumber, EraRewards, Error, IntegratedDApps, + Ledger, NextDAppId, PeriodNumber, PeriodType, }; use frame_support::{assert_noop, assert_ok, error::BadOrigin, traits::Get}; @@ -142,9 +142,8 @@ fn on_initialize_state_change_works() { // Advance eras just until we reach the next voting period let eras_per_bep_period: EraNumber = - ::StandardErasPerBuildAndEarnPeriod::get(); - let blocks_per_era: BlockNumber = - ::StandardEraLength::get(); + ::StandardErasPerBuildAndEarnPeriod::get(); + let blocks_per_era: BlockNumber = ::StandardEraLength::get(); for era in 2..(2 + eras_per_bep_period - 1) { let pre_block = System::block_number(); advance_to_next_era(); @@ -205,7 +204,7 @@ fn register_already_registered_contract_fails() { #[test] fn register_past_max_number_of_contracts_fails() { ExtBuilder::build().execute_with(|| { - let limit = ::MaxNumberOfContracts::get(); + let limit = ::MaxNumberOfContracts::get(); for id in 1..=limit { assert_register(1, &MockSmartContract::Wasm(id.into())); } @@ -385,10 +384,7 @@ fn lock_is_ok() { // Ensure minimum lock amount works let locker = 3; - assert_lock( - locker, - ::MinimumLockedAmount::get(), - ); + assert_lock(locker, ::MinimumLockedAmount::get()); }) } @@ -412,8 +408,7 @@ fn lock_with_incorrect_amount_fails() { // Locking just below the minimum amount should fail let locker = 2; - let minimum_locked_amount: Balance = - ::MinimumLockedAmount::get(); + let minimum_locked_amount: Balance = ::MinimumLockedAmount::get(); assert_noop!( DappStaking::lock(RuntimeOrigin::signed(locker), minimum_locked_amount - 1), Error::::LockedAmountBelowThreshold, @@ -455,8 +450,7 @@ fn unlock_with_remaining_amount_below_threshold_is_ok() { advance_to_era(ActiveProtocolState::::get().era + 3); // Unlock such amount that remaining amount is below threshold, resulting in full unlock - let minimum_locked_amount: Balance = - ::MinimumLockedAmount::get(); + let minimum_locked_amount: Balance = ::MinimumLockedAmount::get(); let ledger = Ledger::::get(&account); assert_unlock( account, @@ -530,8 +524,7 @@ fn unlock_everything_with_active_stake_fails() { advance_to_next_era(); // We stake so the amount is just below the minimum locked amount, causing full unlock impossible. - let minimum_locked_amount: Balance = - ::MinimumLockedAmount::get(); + let minimum_locked_amount: Balance = ::MinimumLockedAmount::get(); let stake_amount = minimum_locked_amount - 1; // Register contract & stake on it @@ -583,7 +576,7 @@ fn unlock_with_exceeding_unlocking_chunks_storage_limits_fails() { assert_lock(account, lock_amount); let unlock_amount = 3; - for _ in 0..::MaxUnlockingChunks::get() { + for _ in 0..::MaxUnlockingChunks::get() { run_for_blocks(1); assert_unlock(account, unlock_amount); } @@ -605,8 +598,7 @@ fn unlock_with_exceeding_unlocking_chunks_storage_limits_fails() { #[test] fn claim_unlocked_is_ok() { ExtBuilder::build().execute_with(|| { - let unlocking_blocks: BlockNumber = - ::UnlockingPeriod::get(); + let unlocking_blocks: BlockNumber = ::UnlockingPeriod::get(); // Lock some amount in a few eras let account = 2; @@ -620,8 +612,7 @@ fn claim_unlocked_is_ok() { assert_claim_unlocked(account); // Advanced example - let max_unlocking_chunks: u32 = - ::MaxUnlockingChunks::get(); + let max_unlocking_chunks: u32 = ::MaxUnlockingChunks::get(); for _ in 0..max_unlocking_chunks { run_for_blocks(1); assert_unlock(account, unlock_amount); @@ -651,8 +642,7 @@ fn claim_unlocked_no_eligible_chunks_fails() { // Cannot claim if unlock period hasn't passed yet let lock_amount = 103; assert_lock(account, lock_amount); - let unlocking_blocks: BlockNumber = - ::UnlockingPeriod::get(); + let unlocking_blocks: BlockNumber = ::UnlockingPeriod::get(); run_for_blocks(unlocking_blocks - 1); assert_noop!( DappStaking::claim_unlocked(RuntimeOrigin::signed(account)), @@ -677,8 +667,7 @@ fn relock_unlocking_is_ok() { assert_relock_unlocking(account); - let max_unlocking_chunks: u32 = - ::MaxUnlockingChunks::get(); + let max_unlocking_chunks: u32 = ::MaxUnlockingChunks::get(); for _ in 0..max_unlocking_chunks { run_for_blocks(1); assert_unlock(account, unlock_amount); @@ -701,8 +690,7 @@ fn relock_unlocking_no_chunks_fails() { #[test] fn relock_unlocking_insufficient_lock_amount_fails() { ExtBuilder::build().execute_with(|| { - let minimum_locked_amount: Balance = - ::MinimumLockedAmount::get(); + let minimum_locked_amount: Balance = ::MinimumLockedAmount::get(); // lock amount should be above the threshold let account = 2; @@ -732,8 +720,7 @@ fn relock_unlocking_insufficient_lock_amount_fails() { }); // Make sure only one chunk is left - let unlocking_blocks: BlockNumber = - ::UnlockingPeriod::get(); + let unlocking_blocks: BlockNumber = ::UnlockingPeriod::get(); run_for_blocks(unlocking_blocks - 1); assert_claim_unlocked(account); @@ -887,8 +874,7 @@ fn stake_fails_due_to_too_small_staking_amount() { assert_lock(account, 300); // Stake with too small amount, expect a failure - let min_stake_amount: Balance = - ::MinimumStakeAmount::get(); + let min_stake_amount: Balance = ::MinimumStakeAmount::get(); assert_noop!( DappStaking::stake( RuntimeOrigin::signed(account), @@ -953,8 +939,7 @@ fn unstake_with_leftover_amount_below_minimum_works() { let amount = 300; assert_lock(account, amount); - let min_stake_amount: Balance = - ::MinimumStakeAmount::get(); + let min_stake_amount: Balance = ::MinimumStakeAmount::get(); assert_stake(account, &smart_contract, min_stake_amount); // Unstake some amount, bringing it below the minimum @@ -1115,77 +1100,122 @@ fn unstake_from_past_period_fails() { }) } -// #[test] -// fn claim_staker_rewards_basic_example_is_ok() { -// ExtBuilder::build().execute_with(|| { -// // Register smart contract, lock&stake some amount -// let dev_account = 1; -// let smart_contract = MockSmartContract::default(); -// assert_register(dev_account, &smart_contract); - -// let account = 2; -// let lock_amount = 300; -// assert_lock(account, lock_amount); -// let stake_amount = 93; -// assert_stake(account, &smart_contract, stake_amount); - -// // Advance into Build&Earn period, and allow one era to pass. Claim reward for 1 era. -// advance_to_era(ActiveProtocolState::::get().era + 2); -// assert_claim_staker_rewards(account); - -// // Advance a few more eras, and claim multiple rewards this time. -// advance_to_era(ActiveProtocolState::::get().era + 3); -// assert_eq!( -// ActiveProtocolState::::get().period_number(), -// 1, -// "Sanity check, we must still be in the 1st period." -// ); -// assert_claim_staker_rewards(account); - -// // Advance into the next period, make sure we can still claim old rewards. -// advance_to_next_period(); -// for _ in 0..required_number_of_reward_claims(account) { -// assert_claim_staker_rewards(account); -// } -// }) -// } - -// #[test] -// fn claim_staker_rewards_no_claimable_rewards_fails() { -// ExtBuilder::build().execute_with(|| { -// // Register smart contract, lock&stake some amount -// let dev_account = 1; -// let smart_contract = MockSmartContract::default(); -// assert_register(dev_account, &smart_contract); - -// let account = 2; -// let lock_amount = 300; -// assert_lock(account, lock_amount); - -// // 1st scenario - try to claim with no stake at all. -// assert_noop!( -// DappStaking::claim_staker_rewards(RuntimeOrigin::signed(account)), -// Error::::NoClaimableRewards, -// ); - -// // 2nd scenario - stake some amount, and try to claim in the same era. -// // It's important this is the 1st era, when no `EraRewards` entry exists. -// assert_eq!(ActiveProtocolState::::get().era, 1, "Sanity check"); -// assert!(EraRewards::::iter().next().is_none(), "Sanity check"); -// let stake_amount = 93; -// assert_stake(account, &smart_contract, stake_amount); -// assert_noop!( -// DappStaking::claim_staker_rewards(RuntimeOrigin::signed(account)), -// Error::::NoClaimableRewards, -// ); - -// // 3rd scenario - move over to the next era, but we still expect failure because -// // stake is valid from era 2 (current era), and we're trying to claim rewards for era 1. -// advance_to_next_era(); -// assert!(EraRewards::::iter().next().is_some(), "Sanity check"); -// assert_noop!( -// DappStaking::claim_staker_rewards(RuntimeOrigin::signed(account)), -// Error::::NoClaimableRewards, -// ); -// }) -// } +#[test] +fn claim_staker_rewards_basic_example_is_ok() { + ExtBuilder::build().execute_with(|| { + // Register smart contract, lock&stake some amount + let dev_account = 1; + let smart_contract = MockSmartContract::default(); + assert_register(dev_account, &smart_contract); + + let account = 2; + let lock_amount = 300; + assert_lock(account, lock_amount); + let stake_amount = 93; + assert_stake(account, &smart_contract, stake_amount); + + // Advance into Build&Earn period, and allow one era to pass. Claim reward for 1 era. + advance_to_era(ActiveProtocolState::::get().era + 2); + assert_claim_staker_rewards(account); + + // Advance a few more eras, and claim multiple rewards this time. + advance_to_era(ActiveProtocolState::::get().era + 3); + assert_eq!( + ActiveProtocolState::::get().period_number(), + 1, + "Sanity check, we must still be in the 1st period." + ); + assert_claim_staker_rewards(account); + + // Advance into the next period, make sure we can still claim old rewards. + advance_to_next_period(); + for _ in 0..required_number_of_reward_claims(account) { + assert_claim_staker_rewards(account); + } + }) +} + +#[test] +fn claim_staker_rewards_no_claimable_rewards_fails() { + ExtBuilder::build().execute_with(|| { + // Register smart contract, lock&stake some amount + let dev_account = 1; + let smart_contract = MockSmartContract::default(); + assert_register(dev_account, &smart_contract); + + let account = 2; + let lock_amount = 300; + assert_lock(account, lock_amount); + + // 1st scenario - try to claim with no stake at all. + assert_noop!( + DappStaking::claim_staker_rewards(RuntimeOrigin::signed(account)), + Error::::NoClaimableRewards, + ); + + // 2nd scenario - stake some amount, and try to claim in the same era. + // It's important this is the 1st era, when no `EraRewards` entry exists. + assert_eq!(ActiveProtocolState::::get().era, 1, "Sanity check"); + assert!(EraRewards::::iter().next().is_none(), "Sanity check"); + let stake_amount = 93; + assert_stake(account, &smart_contract, stake_amount); + assert_noop!( + DappStaking::claim_staker_rewards(RuntimeOrigin::signed(account)), + Error::::NoClaimableRewards, + ); + + // 3rd scenario - move over to the next era, but we still expect failure because + // stake is valid from era 2 (current era), and we're trying to claim rewards for era 1. + advance_to_next_era(); + assert!(EraRewards::::iter().next().is_some(), "Sanity check"); + assert_noop!( + DappStaking::claim_staker_rewards(RuntimeOrigin::signed(account)), + Error::::NoClaimableRewards, + ); + }) +} + +#[test] +fn claim_staker_rewards_after_expiry_fails() { + ExtBuilder::build().execute_with(|| { + // Register smart contract, lock&stake some amount + let dev_account = 1; + let smart_contract = MockSmartContract::default(); + assert_register(dev_account, &smart_contract); + + let account = 2; + let lock_amount = 300; + assert_lock(account, lock_amount); + let stake_amount = 93; + assert_stake(account, &smart_contract, stake_amount); + + let reward_retention_in_periods: PeriodNumber = + ::RewardRetentionInPeriods::get(); + + // Advance to the block just before the 'expiry' period starts + advance_to_period( + ActiveProtocolState::::get().period_number() + reward_retention_in_periods, + ); + advance_to_next_period_type(); + advance_to_era(ActiveProtocolState::::get().period_info.ending_era - 1); + assert_claim_staker_rewards(account); + + // Ensure we're still in the first period for the sake of test validity + assert_eq!( + Ledger::::get(&account).staked.period, + 1, + "Sanity check." + ); + + // Trigger next period, rewards should be marked as expired + advance_to_next_era(); + assert_eq!( + ActiveProtocolState::::get().period_number(), + reward_retention_in_periods + 2 + ); + assert_noop!( + DappStaking::claim_staker_rewards(RuntimeOrigin::signed(account)), + Error::::StakerRewardsExpired, + ); + }) +} diff --git a/pallets/dapp-staking-v3/src/test/tests_types.rs b/pallets/dapp-staking-v3/src/test/tests_types.rs index 5a7399e4ea..f92cca8b8f 100644 --- a/pallets/dapp-staking-v3/src/test/tests_types.rs +++ b/pallets/dapp-staking-v3/src/test/tests_types.rs @@ -438,7 +438,6 @@ fn account_ledger_add_stake_amount_invalid_era_or_period_fails() { assert!(acc_ledger .add_stake_amount(stake_amount, first_era, period_info_1) .is_ok()); - let acc_ledger_snapshot = acc_ledger.clone(); // Try to add to the next era, it should fail. assert_eq!( diff --git a/pallets/dapp-staking-v3/src/types.rs b/pallets/dapp-staking-v3/src/types.rs index 907af2eb30..345f4c613c 100644 --- a/pallets/dapp-staking-v3/src/types.rs +++ b/pallets/dapp-staking-v3/src/types.rs @@ -537,10 +537,12 @@ where self.staked .subtract(amount, current_period_info.period_type); + // Convenience cleanup if self.staked.is_empty() { self.staked = Default::default(); } + if let Some(mut stake_amount) = self.staked_future { stake_amount.subtract(amount, current_period_info.period_type); @@ -586,7 +588,7 @@ where return Err(AccountLedgerError::NothingToClaim); } else if let Some(stake_amount) = self.staked_future { // Future entry exists, but era isn't 'in history' - if era <= stake_amount.era { + if era < stake_amount.era { return Err(AccountLedgerError::NothingToClaim); } } @@ -610,8 +612,11 @@ where let result = RewardsIter::new(span, maybe_other); - // Update latest 'staked' era - self.staked.era = era; + // Rollover future to 'current' stake amount + if let Some(stake_amount) = self.staked_future.take() { + self.staked = stake_amount; + } + self.staked.era = era.saturating_add(1); // Make sure to clean up the entries match period_end {