diff --git a/pallets/dapp-staking-v3/src/lib.rs b/pallets/dapp-staking-v3/src/lib.rs index 8df02a4f90..3e3eef7acd 100644 --- a/pallets/dapp-staking-v3/src/lib.rs +++ b/pallets/dapp-staking-v3/src/lib.rs @@ -806,10 +806,11 @@ pub mod pallet { ); let protocol_state = ActiveProtocolState::::get(); - // Staker always stakes from the NEXT era - let stake_era = protocol_state.era.saturating_add(1); + let stake_era = protocol_state.era; ensure!( - !protocol_state.period_info.is_next_period(stake_era), + !protocol_state + .period_info + .is_next_period(stake_era.saturating_add(1)), Error::::PeriodEndsInNextEra ); @@ -820,7 +821,7 @@ pub mod pallet { ledger .add_stake_amount(amount, stake_era, protocol_state.period_info) .map_err(|err| match err { - AccountLedgerError::InvalidPeriod => { + AccountLedgerError::InvalidPeriod | AccountLedgerError::InvalidEra => { Error::::UnclaimedRewardsFromPastPeriods } AccountLedgerError::UnavailableStakeFunds => Error::::UnavailableStakeFunds, @@ -952,7 +953,9 @@ pub mod pallet { ledger .unstake_amount(amount, unstake_era, protocol_state.period_info) .map_err(|err| match err { - AccountLedgerError::InvalidPeriod => Error::::UnstakeFromPastPeriod, + AccountLedgerError::InvalidPeriod | AccountLedgerError::InvalidEra => { + Error::::UnclaimedRewardsFromPastPeriods + } AccountLedgerError::UnstakeAmountLargerThanStake => { Error::::UnstakeAmountTooLarge } @@ -995,109 +998,97 @@ pub mod pallet { Ok(()) } - // /// TODO - // #[pallet::call_index(11)] - // #[pallet::weight(Weight::zero())] - // pub fn claim_staker_rewards(origin: OriginFor) -> DispatchResult { - // Self::ensure_pallet_enabled()?; - // let account = ensure_signed(origin)?; - - // let protocol_state = ActiveProtocolState::::get(); - // let mut ledger = Ledger::::get(&account); - - // // TODO: how do we handle expired rewards? Add an additional call to clean them up? - // // Putting this logic inside existing calls will add even more complexity. - - // // Check if the rewards have expired - // let staked_period = ledger.staked_period.ok_or(Error::::NoClaimableRewards)?; - // ensure!( - // staked_period - // >= protocol_state - // .period_number() - // .saturating_sub(T::RewardRetentionInPeriods::get()), - // Error::::StakerRewardsExpired - // ); - - // // Calculate the reward claim span - // let (first_chunk, last_chunk) = ledger - // .first_and_last_stake_chunks() - // .ok_or(Error::::InternalClaimStakerError)?; - // let era_rewards = - // EraRewards::::get(Self::era_reward_span_index(first_chunk.get_era())) - // .ok_or(Error::::NoClaimableRewards)?; - - // // The last era for which we can theoretically claim rewards. - // // And indicator if we know the period's ending era. - // let (last_period_era, period_end) = if staked_period == protocol_state.period_number() { - // (protocol_state.era.saturating_sub(1), None) - // } else { - // PeriodEnd::::get(&staked_period) - // .map(|info| (info.final_era, Some(info.final_era))) - // .ok_or(Error::::InternalClaimStakerError)? - // }; - - // // The last era for which we can claim rewards for this account. - // // Limiting factors are: - // // 1. era reward span entries - // // 2. last era in period limit - // // 3. last era in which staker had non-zero stake - // let last_claim_era = if last_chunk.get_amount().is_zero() { - // era_rewards - // .last_era() - // .min(last_period_era) - // .min(last_chunk.get_era()) - // } else { - // era_rewards.last_era().min(last_period_era) - // }; - - // // Get chunks for reward claiming - // let chunks_for_claim = - // ledger - // .claim_up_to_era(last_claim_era, period_end) - // .map_err(|err| match err { - // AccountLedgerError::SplitEraInvalid => Error::::NoClaimableRewards, - // _ => Error::::InternalClaimStakerError, - // })?; - - // // Calculate rewards - // let mut rewards: Vec<_> = Vec::new(); - // let mut reward_sum = Balance::zero(); - // for era in first_chunk.get_era()..=last_claim_era { - // // TODO: this should be zipped, and values should be fetched only once - // let era_reward = era_rewards - // .get(era) - // .ok_or(Error::::InternalClaimStakerError)?; - - // let chunk = chunks_for_claim - // .get(era) - // .ok_or(Error::::InternalClaimStakerError)?; - - // // Optimization, and zero-division protection - // if chunk.get_amount().is_zero() || era_reward.staked().is_zero() { - // continue; - // } - // let staker_reward = Perbill::from_rational(chunk.get_amount(), era_reward.staked()) - // * era_reward.staker_reward_pool(); - - // rewards.push((era, staker_reward)); - // reward_sum.saturating_accrue(staker_reward); - // } - - // T::Currency::deposit_into_existing(&account, reward_sum) - // .map_err(|_| Error::::InternalClaimStakerError)?; - - // Self::update_ledger(&account, ledger); - - // rewards.into_iter().for_each(|(era, reward)| { - // Self::deposit_event(Event::::Reward { - // account: account.clone(), - // era, - // amount: reward, - // }); - // }); - - // Ok(()) - // } + /// TODO + #[pallet::call_index(11)] + #[pallet::weight(Weight::zero())] + pub fn claim_staker_rewards(origin: OriginFor) -> DispatchResult { + Self::ensure_pallet_enabled()?; + let account = ensure_signed(origin)?; + + let protocol_state = ActiveProtocolState::::get(); + let mut ledger = Ledger::::get(&account); + + // TODO: how do we handle expired rewards? Add an additional call to clean them up? + // Putting this logic inside existing calls will add even more complexity. + + // Check if the rewards have expired + let staked_period = ledger + .staked_period() + .ok_or(Error::::NoClaimableRewards)?; + ensure!( + staked_period + >= protocol_state + .period_number() + .saturating_sub(T::RewardRetentionInPeriods::get()), + Error::::StakerRewardsExpired + ); + + // Calculate the reward claim span + let earliest_staked_era = ledger + .earliest_staked_era() + .ok_or(Error::::InternalClaimStakerError)?; + let era_rewards = + EraRewards::::get(Self::era_reward_span_index(earliest_staked_era)) + .ok_or(Error::::NoClaimableRewards)?; + + // The last era for which we can theoretically claim rewards. + // And indicator if we know the period's ending era. + let (last_period_era, period_end) = if staked_period == protocol_state.period_number() { + (protocol_state.era.saturating_sub(1), None) + } else { + PeriodEnd::::get(&staked_period) + .map(|info| (info.final_era, Some(info.final_era))) + .ok_or(Error::::InternalClaimStakerError)? + }; + + // The last era for which we can claim rewards for this account. + let last_claim_era = era_rewards.last_era().min(last_period_era); + + // Get chunks for reward claiming + let rewards_iter = + ledger + .claim_up_to_era(last_claim_era, period_end) + .map_err(|err| match err { + AccountLedgerError::NothingToClaim => Error::::NoClaimableRewards, + _ => Error::::InternalClaimStakerError, + })?; + + // Calculate rewards + let mut rewards: Vec<_> = Vec::new(); + let mut reward_sum = Balance::zero(); + for (era, amount) in rewards_iter { + // TODO: this should be zipped, and values should be fetched only once + let era_reward = era_rewards + .get(era) + .ok_or(Error::::InternalClaimStakerError)?; + + // Optimization, and zero-division protection + if amount.is_zero() || era_reward.staked().is_zero() { + continue; + } + let staker_reward = Perbill::from_rational(amount, era_reward.staked()) + * era_reward.staker_reward_pool(); + + rewards.push((era, staker_reward)); + reward_sum.saturating_accrue(staker_reward); + } + + // TODO: add negative test for this. + T::Currency::deposit_into_existing(&account, reward_sum) + .map_err(|_| Error::::InternalClaimStakerError)?; + + Self::update_ledger(&account, ledger); + + rewards.into_iter().for_each(|(era, reward)| { + Self::deposit_event(Event::::Reward { + account: account.clone(), + era, + amount: reward, + }); + }); + + Ok(()) + } } impl Pallet { diff --git a/pallets/dapp-staking-v3/src/test/mod.rs b/pallets/dapp-staking-v3/src/test/mod.rs index d4f5974862..94a090243c 100644 --- a/pallets/dapp-staking-v3/src/test/mod.rs +++ b/pallets/dapp-staking-v3/src/test/mod.rs @@ -17,6 +17,6 @@ // along with Astar. If not, see . mod mock; -// mod testing_utils; -// mod tests; +mod testing_utils; +mod tests; mod tests_types; diff --git a/pallets/dapp-staking-v3/src/test/testing_utils.rs b/pallets/dapp-staking-v3/src/test/testing_utils.rs index ccc171a46b..6657e02c2b 100644 --- a/pallets/dapp-staking-v3/src/test/testing_utils.rs +++ b/pallets/dapp-staking-v3/src/test/testing_utils.rs @@ -21,7 +21,7 @@ use crate::types::*; use crate::{ pallet as pallet_dapp_staking, ActiveProtocolState, BlockNumberFor, ContractStake, CurrentEraInfo, DAppId, EraRewards, Event, IntegratedDApps, Ledger, NextDAppId, PeriodEnd, - PeriodEndInfo, StakerInfo, + PeriodEndInfo, StakeAmount, StakerInfo, }; use frame_support::{assert_ok, traits::Get}; @@ -48,7 +48,7 @@ pub(crate) struct MemorySnapshot { SingularStakingInfo, >, contract_stake: - HashMap<::SmartContract, ContractStakingInfoSeries>, + HashMap<::SmartContract, ContractStakeAmountSeries>, era_rewards: HashMap< EraNumber, EraRewardSpan<::EraRewardSpanLength>, @@ -422,7 +422,7 @@ pub(crate) fn assert_stake( let pre_contract_stake = pre_snapshot .contract_stake .get(&smart_contract) - .map_or(ContractStakingInfoSeries::default(), |series| { + .map_or(ContractStakeAmountSeries::default(), |series| { series.clone() }); let pre_era_info = pre_snapshot.current_era_info; @@ -459,7 +459,11 @@ pub(crate) fn assert_stake( // 1. verify ledger // ===================== // ===================== - assert_eq!(post_ledger.staked_period, Some(stake_period)); + assert_eq!( + post_ledger.staked, pre_ledger.staked, + "Must remain exactly the same." + ); + assert_eq!(post_ledger.staked_future.unwrap().period, stake_period); assert_eq!( post_ledger.staked_amount(stake_period), pre_ledger.staked_amount(stake_period) + amount, @@ -470,22 +474,7 @@ pub(crate) fn assert_stake( pre_ledger.stakeable_amount(stake_period) - amount, "Stakeable amount must decrease by the 'amount'" ); - match pre_ledger.last_stake_era() { - Some(last_stake_era) if last_stake_era == stake_era => { - assert_eq!( - post_ledger.staked.0.len(), - pre_ledger.staked.0.len(), - "Existing entry must be modified." - ); - } - _ => { - assert_eq!( - post_ledger.staked.0.len(), - pre_ledger.staked.0.len() + 1, - "Additional entry must be added." - ); - } - } + // TODO: maybe expand checks here? // 2. verify staker info // ===================== @@ -534,39 +523,20 @@ pub(crate) fn assert_stake( // 3. verify contract stake // ========================= // ========================= - // TODO: since default value is all zeros, maybe we can just skip the branching code and do it once? - match pre_contract_stake.last_stake_period() { - Some(last_stake_period) if last_stake_period == stake_period => { - assert_eq!( - post_contract_stake.total_staked_amount(stake_period), - pre_contract_stake.total_staked_amount(stake_period) + amount, - "Staked amount must increase by the 'amount'" - ); - assert_eq!( - post_contract_stake.staked_amount(stake_period, stake_period_type), - pre_contract_stake.staked_amount(stake_period, stake_period_type) + amount, - "Staked amount must increase by the 'amount'" - ); - } - _ => { - assert_eq!(post_contract_stake.len(), 1); - assert_eq!( - post_contract_stake.total_staked_amount(stake_period), - amount, - "Total staked amount must be equal to exactly the 'amount'" - ); - assert_eq!( - post_contract_stake.staked_amount(stake_period, stake_period_type), - amount, - "Staked amount must be equal to exactly the 'amount'" - ); - } - } + assert_eq!( + post_contract_stake.total_staked_amount(stake_period), + pre_contract_stake.total_staked_amount(stake_period) + amount, + "Staked amount must increase by the 'amount'" + ); + assert_eq!( + post_contract_stake.staked_amount(stake_period, stake_period_type), + pre_contract_stake.staked_amount(stake_period, stake_period_type) + amount, + "Staked amount must increase by the 'amount'" + ); + assert_eq!(post_contract_stake.last_stake_period(), Some(stake_period)); assert_eq!(post_contract_stake.last_stake_era(), Some(stake_era)); - // TODO: expand this check to compare inner slices as well! - // 4. verify era info // ========================= // ========================= @@ -643,7 +613,6 @@ pub(crate) fn assert_unstake( // 1. verify ledger // ===================== // ===================== - assert_eq!(post_ledger.staked_period, Some(unstake_period)); assert_eq!( post_ledger.staked_amount(unstake_period), pre_ledger.staked_amount(unstake_period) - amount, @@ -654,7 +623,7 @@ pub(crate) fn assert_unstake( pre_ledger.stakeable_amount(unstake_period) + amount, "Stakeable amount must increase by the 'amount'" ); - // TODO: maybe extend check with concrete value checks? E.g. if we modify past entry, we should check past & current entries are properly adjusted. + // TODO: expand with more detailed checks of staked and staked_future // 2. verify staker info // ===================== @@ -709,7 +678,6 @@ pub(crate) fn assert_unstake( .saturating_sub(amount), "Staked amount must decreased by the 'amount'" ); - // TODO: extend with concrete value checks later // 4. verify era info // ========================= @@ -760,11 +728,8 @@ pub(crate) fn assert_claim_staker_rewards(account: AccountId) { // Get the first eligible era for claiming rewards let first_claim_era = pre_ledger - .staked - .0 - .first() - .expect("Entry must exist, otherwise 'claim' is invalid.") - .get_era(); + .earliest_staked_era() + .expect("Entry must exist, otherwise 'claim' is invalid."); // Get the apprropriate era rewards span for the 'first era' let era_span_length: EraNumber = @@ -776,19 +741,24 @@ pub(crate) fn assert_claim_staker_rewards(account: AccountId) { .expect("Entry must exist, otherwise 'claim' is invalid."); // Calculate the final era for claiming rewards. Also determine if this will fully claim all staked period rewards. - let is_current_period_stake = match pre_ledger.staked_period { - Some(staked_period) - if staked_period == pre_snapshot.active_protocol_state.period_number() => - { - true - } - _ => false, + let claim_period_end = if pre_ledger.staked_period().unwrap() + == pre_snapshot.active_protocol_state.period_number() + { + None + } else { + Some( + pre_snapshot + .period_end + .get(&pre_ledger.staked_period().unwrap()) + .expect("Entry must exist, since it's the current period.") + .final_era, + ) }; - let (last_claim_era, is_full_claim) = if is_current_period_stake { + let (last_claim_era, is_full_claim) = if claim_period_end.is_none() { (pre_snapshot.active_protocol_state.era - 1, false) } else { - let claim_period = pre_ledger.staked_period.unwrap(); + let claim_period = pre_ledger.staked_period().unwrap(); let period_end = pre_snapshot .period_end .get(&claim_period) @@ -806,16 +776,16 @@ pub(crate) fn assert_claim_staker_rewards(account: AccountId) { // Calculate the expected rewards let mut rewards = Vec::new(); - for era in first_claim_era..=last_claim_era { + for (era, amount) in pre_ledger + .clone() + .claim_up_to_era(last_claim_era, claim_period_end) + .unwrap() + { let era_reward_info = era_rewards_span .get(era) .expect("Entry must exist, otherwise 'claim' is invalid."); - let stake_chunk = pre_ledger - .staked - .get(era) - .expect("Entry must exist, otherwise 'claim' is invalid."); - let reward = Perbill::from_rational(stake_chunk.amount, era_reward_info.staked()) + let reward = Perbill::from_rational(amount, era_reward_info.staked()) * era_reward_info.staker_reward_pool(); if reward.is_zero() { continue; @@ -868,15 +838,11 @@ pub(crate) fn assert_claim_staker_rewards(account: AccountId) { let post_ledger = post_snapshot.ledger.get(&account).unwrap(); if is_full_claim { - assert!(post_ledger.staked.0.is_empty()); - assert!(post_ledger.staked_period.is_none()); + assert!(post_ledger.staked.is_empty()); + assert!(post_ledger.staked_future.is_none()); } else { - let stake_chunk = post_ledger.staked.0.first().expect("Entry must exist"); - assert_eq!(stake_chunk.era, last_claim_era + 1); - assert_eq!( - stake_chunk.amount, - pre_ledger.staked.get(last_claim_era).unwrap().amount - ); + assert_eq!(post_ledger.staked.era, last_claim_era + 1); + // TODO: expand check? } } @@ -888,25 +854,21 @@ pub(crate) fn claimable_reward_range(account: AccountId) -> Option<(EraNumber, E let ledger = Ledger::::get(&account); let protocol_state = ActiveProtocolState::::get(); - let (first_chunk, last_chunk) = if let Some(chunks) = ledger.first_and_last_stake_chunks() { - chunks + let earliest_stake_era = if let Some(era) = ledger.earliest_staked_era() { + era } else { return None; }; - // Full unstake happened, no rewards past this. - let last_claim_era = if last_chunk.get_amount().is_zero() { - last_chunk.get_era() - 1 - } else if ledger.staked_period == Some(protocol_state.period_number()) { - // Staked in the ongoing period, best we can do is claim up to last era + let last_claim_era = if ledger.staked_period() == Some(protocol_state.period_number()) { protocol_state.era - 1 } else { // Period finished, we can claim up to its final era - let period_end = PeriodEnd::::get(ledger.staked_period.unwrap()).unwrap(); + let period_end = PeriodEnd::::get(ledger.staked_period().unwrap()).unwrap(); period_end.final_era }; - Some((first_chunk.get_era(), last_claim_era)) + Some((earliest_stake_era, last_claim_era)) } /// Number of times it's required to call `claim_staker_rewards` to claim all pending rewards. diff --git a/pallets/dapp-staking-v3/src/test/tests.rs b/pallets/dapp-staking-v3/src/test/tests.rs index 56fe48118f..30d34d5543 100644 --- a/pallets/dapp-staking-v3/src/test/tests.rs +++ b/pallets/dapp-staking-v3/src/test/tests.rs @@ -756,27 +756,10 @@ fn stake_basic_example_is_ok() { let lock_amount = 300; assert_lock(account, lock_amount); - // 1st scenario - stake some amount, and then some more + // Stake some amount, and then some more let (stake_amount_1, stake_amount_2) = (31, 29); assert_stake(account, &smart_contract, stake_amount_1); assert_stake(account, &smart_contract, stake_amount_2); - - // 2nd scenario - stake in the next era - advance_to_next_era(); - let stake_amount_3 = 23; - assert_stake(account, &smart_contract, stake_amount_3); - - // 3rd scenario - advance era again but create a gap, and then stake - advance_to_era(ActiveProtocolState::::get().era + 2); - let stake_amount_4 = 19; - assert_stake(account, &smart_contract, stake_amount_4); - - // 4th scenario - advance period, and stake - // advance_to_next_era(); - // advance_to_next_period(); - // let stake_amount_5 = 17; - // assert_stake(account, &smart_contract, stake_amount_5); - // TODO: this can only be tested after reward claiming has been implemented!!! }) } @@ -892,34 +875,6 @@ fn stake_fails_if_not_enough_stakeable_funds_available() { }) } -#[test] -fn stake_fails_due_to_too_many_chunks() { - ExtBuilder::build().execute_with(|| { - // Register smart contract & lock some amount - let smart_contract = MockSmartContract::default(); - let account = 3; - assert_register(1, &smart_contract); - let lock_amount = 500; - assert_lock(account, lock_amount); - - // Keep on staking & creating chunks until capacity is reached - for _ in 0..(::MaxStakingChunks::get()) { - advance_to_next_era(); - assert_stake(account, &smart_contract, 10); - } - - // Ensure we can still stake in the current era since an entry exists - assert_stake(account, &smart_contract, 10); - - // Staking in the next era results in error due to too many chunks - advance_to_next_era(); - assert_noop!( - DappStaking::stake(RuntimeOrigin::signed(account), smart_contract.clone(), 10), - Error::::TooManyStakeChunks - ); - }) -} - #[test] fn stake_fails_due_to_too_small_staking_amount() { ExtBuilder::build().execute_with(|| { @@ -960,6 +915,8 @@ fn stake_fails_due_to_too_small_staking_amount() { }) } +// TODO: add tests to cover staking & unstaking with unclaimed rewards! + #[test] fn unstake_basic_example_is_ok() { ExtBuilder::build().execute_with(|| { @@ -976,33 +933,11 @@ fn unstake_basic_example_is_ok() { let stake_amount_1 = 83; assert_stake(account, &smart_contract, stake_amount_1); - // 1st scenario - unstake some amount, in the current era. + // Unstake some amount, in the current era. let unstake_amount_1 = 3; assert_unstake(account, &smart_contract, unstake_amount_1); - // 2nd scenario - advance to next era/period type, and unstake some more - let unstake_amount_2 = 7; - let unstake_amount_3 = 11; - advance_to_next_era(); - assert_eq!( - ActiveProtocolState::::get().period_type(), - PeriodType::BuildAndEarn, - "Sanity check, period type change must happe." - ); - assert_unstake(account, &smart_contract, unstake_amount_2); - assert_unstake(account, &smart_contract, unstake_amount_3); - - // 3rd scenario - advance few eras to create a gap, and unstake some more - advance_to_era(ActiveProtocolState::::get().era + 3); - assert_unstake(account, &smart_contract, unstake_amount_3); - assert_unstake(account, &smart_contract, unstake_amount_2); - - // 4th scenario - perform a full unstake - advance_to_next_era(); - let full_unstake_amount = StakerInfo::::get(&account, &smart_contract) - .unwrap() - .total_staked_amount(); - assert_unstake(account, &smart_contract, full_unstake_amount); + // TODO: scenario where we unstake AFTER advancing an era and claiming rewards }) } @@ -1027,33 +962,6 @@ fn unstake_with_leftover_amount_below_minimum_works() { }) } -#[test] -fn unstake_with_entry_overflow_attempt_works() { - ExtBuilder::build().execute_with(|| { - // Register smart contract & lock some amount - let dev_account = 1; - let smart_contract = MockSmartContract::default(); - assert_register(dev_account, &smart_contract); - - let account = 2; - let amount = 300; - assert_lock(account, amount); - - assert_stake(account, &smart_contract, amount); - - // Advance one era, unstake some amount. The goal is to make a new entry. - advance_to_next_era(); - assert_unstake(account, &smart_contract, 11); - - // Advance 2 eras, stake some amount. This should create a new entry for the next era. - advance_to_era(ActiveProtocolState::::get().era + 2); - assert_stake(account, &smart_contract, 3); - - // Unstake some amount, which should result in the creation of the 4th entry, but the oldest one should be prunned. - assert_unstake(account, &smart_contract, 1); - }) -} - #[test] fn unstake_with_zero_amount_fails() { ExtBuilder::build().execute_with(|| { @@ -1207,106 +1115,77 @@ fn unstake_from_past_period_fails() { }) } -#[test] -fn unstake_fails_due_to_too_many_chunks() { - ExtBuilder::build().execute_with(|| { - // Register smart contract,lock & stake some amount - let smart_contract = MockSmartContract::default(); - let account = 2; - assert_register(1, &smart_contract); - let lock_amount = 1000; - assert_lock(account, lock_amount); - assert_stake(account, &smart_contract, lock_amount); - - // Keep on unstaking & creating chunks until capacity is reached - for _ in 0..(::MaxStakingChunks::get()) { - advance_to_next_era(); - assert_unstake(account, &smart_contract, 11); - } - - // Ensure we can still unstake in the current era since an entry exists - assert_unstake(account, &smart_contract, 10); - - // Staking in the next era results in error due to too many chunks - advance_to_next_era(); - assert_noop!( - DappStaking::unstake(RuntimeOrigin::signed(account), smart_contract.clone(), 10), - Error::::TooManyStakeChunks - ); - }) -} - -#[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, +// ); +// }) +// } diff --git a/pallets/dapp-staking-v3/src/test/tests_types.rs b/pallets/dapp-staking-v3/src/test/tests_types.rs index 5ad99cb1a2..5a7399e4ea 100644 --- a/pallets/dapp-staking-v3/src/test/tests_types.rs +++ b/pallets/dapp-staking-v3/src/test/tests_types.rs @@ -1188,6 +1188,7 @@ fn contract_stake_amount_info_series_stake_is_ok() { // 1st scenario - stake some amount and verify state change let era_1 = 3; + let stake_era_1 = era_1 + 1; let period_1 = 5; let period_info_1 = PeriodInfo::new(period_1, PeriodType::Voting, 20); let amount_1 = 31; @@ -1196,8 +1197,15 @@ fn contract_stake_amount_info_series_stake_is_ok() { assert_eq!(series.len(), 1); assert!(!series.is_empty()); - let entry_1_1 = series.get(era_1, period_1).unwrap(); - assert_eq!(entry_1_1.era, era_1); + assert!( + series.get(era_1, period_1).is_none(), + "Entry for current era must not exist." + ); + let entry_1_1 = series.get(stake_era_1, period_1).unwrap(); + assert_eq!( + entry_1_1.era, stake_era_1, + "Stake is only valid from next era." + ); assert_eq!(entry_1_1.total(), amount_1); // 2nd scenario - stake some more to the same era but different period type, and verify state change. @@ -1208,19 +1216,20 @@ fn contract_stake_amount_info_series_stake_is_ok() { 1, "No new entry should be created since it's the same era." ); - let entry_1_2 = series.get(era_1, period_1).unwrap(); - assert_eq!(entry_1_2.era, era_1); + let entry_1_2 = series.get(stake_era_1, period_1).unwrap(); + assert_eq!(entry_1_2.era, stake_era_1); assert_eq!(entry_1_2.total(), amount_1 * 2); // 3rd scenario - stake more to the next era, while still in the same period. let era_2 = era_1 + 2; + let stake_era_2 = era_2 + 1; let amount_2 = 37; assert!(series.stake(amount_2, period_info_1, era_2).is_ok()); assert_eq!(series.len(), 2); - let entry_2_1 = series.get(era_1, period_1).unwrap(); - let entry_2_2 = series.get(era_2, period_1).unwrap(); + let entry_2_1 = series.get(stake_era_1, period_1).unwrap(); + let entry_2_2 = series.get(stake_era_2, period_1).unwrap(); assert_eq!(entry_2_1, entry_1_2, "Old entry must remain unchanged."); - assert_eq!(entry_2_2.era, era_2); + assert_eq!(entry_2_2.era, stake_era_2); assert_eq!(entry_2_2.period, period_1); assert_eq!( entry_2_2.total(), @@ -1230,18 +1239,19 @@ fn contract_stake_amount_info_series_stake_is_ok() { // 4th scenario - stake some more to the next era, but this time also bump the period. let era_3 = era_2 + 3; + let stake_era_3 = era_3 + 1; let period_2 = period_1 + 1; let period_info_2 = PeriodInfo::new(period_2, PeriodType::BuildAndEarn, 20); let amount_3 = 41; assert!(series.stake(amount_3, period_info_2, era_3).is_ok()); assert_eq!(series.len(), 3); - let entry_3_1 = series.get(era_1, period_1).unwrap(); - let entry_3_2 = series.get(era_2, period_1).unwrap(); - let entry_3_3 = series.get(era_3, period_2).unwrap(); + let entry_3_1 = series.get(stake_era_1, period_1).unwrap(); + let entry_3_2 = series.get(stake_era_2, period_1).unwrap(); + let entry_3_3 = series.get(stake_era_3, period_2).unwrap(); assert_eq!(entry_3_1, entry_2_1, "Old entry must remain unchanged."); assert_eq!(entry_3_2, entry_2_2, "Old entry must remain unchanged."); - assert_eq!(entry_3_3.era, era_3); + assert_eq!(entry_3_3.era, stake_era_3); assert_eq!(entry_3_3.period, period_2); assert_eq!( entry_3_3.total(), @@ -1251,15 +1261,16 @@ fn contract_stake_amount_info_series_stake_is_ok() { // 5th scenario - stake to the next era, expect cleanup of oldest entry let era_4 = era_3 + 1; + let stake_era_4 = era_4 + 1; let amount_4 = 5; assert!(series.stake(amount_4, period_info_2, era_4).is_ok()); assert_eq!(series.len(), 3); - let entry_4_1 = series.get(era_2, period_1).unwrap(); - let entry_4_2 = series.get(era_3, period_2).unwrap(); - let entry_4_3 = series.get(era_4, period_2).unwrap(); + let entry_4_1 = series.get(stake_era_2, period_1).unwrap(); + let entry_4_2 = series.get(stake_era_3, period_2).unwrap(); + let entry_4_3 = series.get(stake_era_4, period_2).unwrap(); assert_eq!(entry_4_1, entry_3_2, "Old entry must remain unchanged."); assert_eq!(entry_4_2, entry_3_3, "Old entry must remain unchanged."); - assert_eq!(entry_4_3.era, era_4); + assert_eq!(entry_4_3.era, stake_era_4); assert_eq!(entry_4_3.period, period_2); assert_eq!(entry_4_3.total(), amount_3 + amount_4); } @@ -1296,6 +1307,7 @@ fn contract_stake_amount_info_series_unstake_is_ok() { // Prep action - create a stake entry let era_1 = 2; + let stake_era_1 = era_1 + 1; let period = 3; let period_info = PeriodInfo::new(period, PeriodType::Voting, 20); let stake_amount = 100; @@ -1344,7 +1356,7 @@ fn contract_stake_amount_info_series_unstake_is_ok() { // Check concrete entries assert_eq!( - series.get(era_1, period).unwrap().total(), + series.get(stake_era_1, period).unwrap().total(), stake_amount - amount_1, "Oldest entry must remain unchanged." ); @@ -1416,6 +1428,7 @@ fn contract_stake_amount_info_unstake_with_worst_case_scenario_for_capacity_over fn contract_stake_amount_info_series_unstake_with_inconsistent_data_fails() { let mut series = ContractStakeAmountSeries::default(); let era = 5; + let stake_era = era + 1; let period = 2; let period_info = PeriodInfo { number: period, @@ -1435,11 +1448,11 @@ fn contract_stake_amount_info_series_unstake_with_inconsistent_data_fails() { temp.number -= 1; temp }; - assert!(series.unstake(1, old_period_info, era - 1).is_err()); + assert!(series.unstake(1, old_period_info, stake_era).is_err()); // 3rd - Unstake with 'too' old era - assert!(series.unstake(1, period_info, era - 2).is_err()); - assert!(series.unstake(1, period_info, era - 1).is_ok()); + assert!(series.unstake(1, period_info, stake_era - 2).is_err()); + assert!(series.unstake(1, period_info, stake_era - 1).is_ok()); } #[test] diff --git a/pallets/dapp-staking-v3/src/types.rs b/pallets/dapp-staking-v3/src/types.rs index 8c418e60ba..907af2eb30 100644 --- a/pallets/dapp-staking-v3/src/types.rs +++ b/pallets/dapp-staking-v3/src/types.rs @@ -554,6 +554,24 @@ where Ok(()) } + // TODO + pub fn staked_period(&self) -> Option { + if self.staked.is_empty() { + self.staked_future.map(|stake_amount| stake_amount.period) + } else { + Some(self.staked.period) + } + } + + // TODO + pub fn earliest_staked_era(&self) -> Option { + if self.staked.is_empty() { + self.staked_future.map(|stake_amount| stake_amount.era) + } else { + Some(self.staked.era) + } + } + /// Claim up stake chunks up to the specified `era`. /// Returns the vector describing claimable chunks. /// @@ -562,22 +580,44 @@ where &mut self, era: EraNumber, period_end: Option, - ) -> Result<(EraNumber, EraNumber, Balance), AccountLedgerError> { - // TODO: the check also needs to ensure that future entry is covered!!! - // TODO2: the return type won't work since we can have 2 distinct values - one from staked, one from staked_future - if era <= self.staked.era || self.staked.total().is_zero() { + ) -> Result { + // Main entry exists, but era isn't 'in history' + if !self.staked.is_empty() && era <= self.staked.era { 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 { + return Err(AccountLedgerError::NothingToClaim); + } } - let result = (self.staked.era, era, self.staked.total()); + // There are multiple options: + // 1. We only have future entry, no current entry + // 2. We have both current and future entry + // 3. We only have current entry, no future entry + let (span, maybe_other) = if let Some(stake_amount) = self.staked_future { + if self.staked.is_empty() { + ((stake_amount.era, era, stake_amount.total()), None) + } else { + ( + (stake_amount.era, era, stake_amount.total()), + Some((self.staked.era, self.staked.total())), + ) + } + } else { + ((self.staked.era, era, self.staked.total()), None) + }; + + let result = RewardsIter::new(span, maybe_other); // Update latest 'staked' era self.staked.era = era; - // Make sure to clean + // Make sure to clean up the entries match period_end { Some(ending_era) if era >= ending_era => { self.staker_rewards_claimed = true; + // TODO: probably not the right thing to do, need to check if pending bonus rewards need to be claimed as well self.staked = Default::default(); self.staked_future = None; } @@ -588,6 +628,56 @@ where } } +// TODO: docs & test +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub struct RewardsIter { + maybe_other: Option<(EraNumber, Balance)>, + start_era: EraNumber, + end_era: EraNumber, + amount: Balance, +} + +impl RewardsIter { + pub fn new( + span: (EraNumber, EraNumber, Balance), + maybe_other: Option<(EraNumber, Balance)>, + ) -> Self { + if let Some((era, _amount)) = maybe_other { + debug_assert!( + span.0 == era + 1, + "The 'other', if it exists, must cover era preceding the span." + ); + } + + Self { + maybe_other, + start_era: span.0, + end_era: span.1, + amount: span.2, + } + } +} + +impl Iterator for RewardsIter { + type Item = (EraNumber, Balance); + + fn next(&mut self) -> Option { + // Fist cover the scenario where we have a unique first value + if let Some((era, amount)) = self.maybe_other.take() { + return Some((era, amount)); + } + + // Afterwards, just keep returning the same amount for different eras + if self.start_era <= self.end_era { + let value = (self.start_era, self.amount); + self.start_era.saturating_accrue(1); + return Some(value); + } else { + None + } + } +} + // TODO #[derive(Encode, Decode, MaxEncodedLen, Copy, Clone, Debug, PartialEq, Eq, TypeInfo, Default)] pub struct StakeAmount { @@ -869,6 +959,16 @@ impl ContractStakeAmountSeries { self.0.is_empty() } + // TODO + pub fn last_stake_period(&self) -> Option { + self.0.last().map(|x| x.period) + } + + // TODO + pub fn last_stake_era(&self) -> Option { + self.0.last().map(|x| x.era) + } + /// Returns the `StakeAmount` type for the specified era & period, if it exists. pub fn get(&self, era: EraNumber, period: PeriodNumber) -> Option { let idx = self @@ -923,18 +1023,20 @@ impl ContractStakeAmountSeries { &mut self, amount: Balance, period_info: PeriodInfo, - era: EraNumber, + current_era: EraNumber, ) -> Result<(), ()> { + let stake_era = current_era.saturating_add(1); + // Defensive check to ensure we don't end up in a corrupted state. Should never happen. if let Some(stake_amount) = self.0.last() { - if stake_amount.era > era || stake_amount.period > period_info.number { + if stake_amount.era > stake_era || stake_amount.period > period_info.number { return Err(()); } } // Get the most relevant `StakeAmount` instance let mut stake_amount = if let Some(stake_amount) = self.0.last() { - if stake_amount.era == era { + if stake_amount.era == stake_era { // Era matches, so we just update the last element. let stake_amount = *stake_amount; let _ = self.0.pop(); @@ -942,15 +1044,25 @@ impl ContractStakeAmountSeries { } else if stake_amount.period == period_info.number { // Periods match so we should 'copy' the last element to get correct staking amount let mut temp = *stake_amount; - temp.era = era; + temp.era = stake_era; temp } else { // It's a new period, so we need a completely new instance - StakeAmount::new(Balance::zero(), Balance::zero(), era, period_info.number) + StakeAmount::new( + Balance::zero(), + Balance::zero(), + stake_era, + period_info.number, + ) } } else { // It's a new period, so we need a completely new instance - StakeAmount::new(Balance::zero(), Balance::zero(), era, period_info.number) + StakeAmount::new( + Balance::zero(), + Balance::zero(), + stake_era, + period_info.number, + ) }; // Update the stake amount