diff --git a/pallets/dapp-staking-v3/src/lib.rs b/pallets/dapp-staking-v3/src/lib.rs index 2fd93007e1..a966d632fd 100644 --- a/pallets/dapp-staking-v3/src/lib.rs +++ b/pallets/dapp-staking-v3/src/lib.rs @@ -352,10 +352,10 @@ pub mod pallet { } let mut era_info = CurrentEraInfo::::get(); - let staker_reward_pool = Balance::from(1000_u128); // TODO: calculate this properly, inject it from outside (Tokenomics 2.0 pallet?) + let staker_reward_pool = Balance::from(1_000_000_000_000u128); // TODO: calculate this properly, inject it from outside (Tokenomics 2.0 pallet?) let era_reward = EraReward::new(staker_reward_pool, era_info.total_staked_amount()); - let ending_era = protocol_state.era; - let next_era = ending_era.saturating_add(1); + let current_era = protocol_state.era; + let next_era = current_era.saturating_add(1); let maybe_period_event = match protocol_state.period_type() { PeriodType::Voting => { // For the sake of consistency @@ -377,6 +377,17 @@ pub mod pallet { // Switch to `Voting` period if conditions are met. if protocol_state.period_info.is_next_period(next_era) { + // Store info about period end + let bonus_reward_pool = Balance::from(3_000_000_u32); // TODO: get this from Tokenomics 2.0 pallet + PeriodEnd::::insert( + &protocol_state.period_number(), + PeriodEndInfo { + bonus_reward_pool, + total_vp_stake: era_info.staked_amount(PeriodType::Voting), + final_era: current_era, + }, + ); + // For the sake of consistency we treat the whole `Voting` period as a single era. // This means no special handling is required for this period, it only lasts potentially longer than a single standard era. let ending_era = next_era.saturating_add(1); @@ -411,12 +422,12 @@ pub mod pallet { CurrentEraInfo::::put(era_info); - let era_span_index = Self::era_reward_span_index(ending_era); + let era_span_index = Self::era_reward_span_index(current_era); let mut span = EraRewards::::get(&era_span_index).unwrap_or(EraRewardSpan::new()); // TODO: error must not happen here. Log an error if it does. // The consequence will be that some rewards will be temporarily lost/unavailable, but nothing protocol breaking. // Will require a fix from the runtime team though. - let _ = span.push(ending_era, era_reward); + let _ = span.push(current_era, era_reward); EraRewards::::insert(&era_span_index, span); Self::deposit_event(Event::::NewEra { era: next_era }); @@ -1006,13 +1017,14 @@ pub mod pallet { ); // Calculate the reward claim span - let first_claim_era = ledger - .oldest_stake_era() - .ok_or(Error::::InternalClaimStakerError)?; - let era_rewards = EraRewards::::get(Self::era_reward_span_index(first_claim_era)) + 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::::InternalClaimStakerError)?; - // last_period_era or current_era - 1 + // The last era for which we can theoretically claim rewards. let last_period_era = if staked_period == protocol_state.period_number() { protocol_state.era.saturating_sub(1) } else { @@ -1020,10 +1032,20 @@ pub mod pallet { .ok_or(Error::::InternalClaimStakerError)? .final_era }; - let last_claim_era = era_rewards.last_era().min(last_period_era); - let is_full_period_claimed = - staked_period < protocol_state.period_number() && last_period_era == last_claim_era; + // 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 @@ -1032,10 +1054,17 @@ pub mod pallet { .map_err(|_| Error::::InternalClaimStakerError)?; ensure!(chunks_for_claim.0.len() > 0, Error::::NoClaimableRewards); + // Full reward claim is done if no more rewards are claimable. + // This can be either due to: 1) we've claimed the last period + // TODO: this is wrong, the first part of check needs to be improved + let is_full_period_claimed = staked_period < protocol_state.period_number() + && last_period_era == last_claim_era + || ledger.staked.0.is_empty(); + // Calculate rewards let mut rewards: Vec<_> = Vec::new(); let mut reward_sum = Balance::zero(); - for era in first_claim_era..=last_claim_era { + for era in first_chunk.get_era()..=last_claim_era { let era_reward = era_rewards .get(era) .ok_or(Error::::InternalClaimStakerError)?; @@ -1046,6 +1075,9 @@ pub mod pallet { let staker_reward = Perbill::from_rational(chunk.get_amount(), era_reward.staked()) * era_reward.staker_reward_pool(); + if staker_reward.is_zero() { + continue; + } rewards.push((era, staker_reward)); reward_sum.saturating_accrue(staker_reward); } @@ -1123,7 +1155,7 @@ pub mod pallet { } /// Calculates the `EraRewardSpan` index for the specified era. - fn era_reward_span_index(era: EraNumber) -> EraNumber { + pub fn era_reward_span_index(era: EraNumber) -> EraNumber { era.saturating_sub(era % T::EraRewardSpanLength::get()) } } diff --git a/pallets/dapp-staking-v3/src/test/testing_utils.rs b/pallets/dapp-staking-v3/src/test/testing_utils.rs index 0d6e1f0c8d..73043452a8 100644 --- a/pallets/dapp-staking-v3/src/test/testing_utils.rs +++ b/pallets/dapp-staking-v3/src/test/testing_utils.rs @@ -788,8 +788,6 @@ pub(crate) fn assert_claim_staker_rewards(account: AccountId) { let (last_claim_era, is_full_claim) = if is_current_period_stake { (pre_snapshot.active_protocol_state.era - 1, false) } else { - let last_claim_era = era_rewards_span.last_era(); - let claim_period = pre_ledger.staked_period.unwrap(); let period_end = pre_snapshot .period_end @@ -825,6 +823,9 @@ pub(crate) fn assert_claim_staker_rewards(account: AccountId) { .iter() .fold(Balance::zero(), |acc, (_, reward)| acc + reward); + //clean up possible leftover events + System::reset_events(); + // Unstake from smart contract & verify event(s) assert_ok!(DappStaking::claim_staker_rewards(RuntimeOrigin::signed( account @@ -874,3 +875,54 @@ pub(crate) fn assert_claim_staker_rewards(account: AccountId) { ); } } + +/// Returns from which starting era to which ending era can rewards be claimed for the specified account. +/// +/// If `None` is returned, there is nothing to claim. +/// Doesn't consider reward expiration. +pub(crate) fn claimable_reward_range(account: AccountId) -> Option<(EraNumber, EraNumber)> { + 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 + } 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 + 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(); + period_end.final_era + }; + + Some((first_chunk.get_era(), last_claim_era)) +} + +/// Number of times it's required to call `claim_staker_rewards` to claim all pending rewards. +/// +/// In case no rewards are pending, return **zero**. +pub(crate) fn required_number_of_reward_claims(account: AccountId) -> u32 { + let range = if let Some(range) = claimable_reward_range(account) { + range + } else { + return 0; + }; + + let era_span_length: EraNumber = + ::EraRewardSpanLength::get(); + let first = DappStaking::era_reward_span_index(range.0) + .checked_div(era_span_length) + .unwrap(); + let second = DappStaking::era_reward_span_index(range.1) + .checked_div(era_span_length) + .unwrap(); + + second - first + 1 +} diff --git a/pallets/dapp-staking-v3/src/test/tests.rs b/pallets/dapp-staking-v3/src/test/tests.rs index 27f76a6d62..9fb269ce8a 100644 --- a/pallets/dapp-staking-v3/src/test/tests.rs +++ b/pallets/dapp-staking-v3/src/test/tests.rs @@ -1235,3 +1235,39 @@ fn unstake_fails_due_to_too_many_chunks() { ); }) } + +#[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. + // TODO: I should calculate number of expected claims, so I know how much times `claim_staker_rewards` should be called. + advance_to_next_period(); + for _ in 0..required_number_of_reward_claims(account) { + assert_claim_staker_rewards(account); + } + }) +} diff --git a/pallets/dapp-staking-v3/src/types.rs b/pallets/dapp-staking-v3/src/types.rs index 875ece456c..07afb13f6a 100644 --- a/pallets/dapp-staking-v3/src/types.rs +++ b/pallets/dapp-staking-v3/src/types.rs @@ -243,6 +243,8 @@ where /// b) [1,2] -- split(4) --> [1,2],[5] /// c) [1,2,4,5] -- split(4) --> [1,2,4],[5] /// d) [1,2,4,6] -- split(4) --> [1,2,4],[5,6] + /// e) [1,2(0)] -- split(4) --> [1,2],[] // 2nd entry has 'zero' amount, so no need to keep it + // TODO: what if last element on the right is zero? We should cleanup the vector then. pub fn left_split(&mut self, era: EraNumber) -> Result { // TODO: this implementation can once again be optimized, sacrificing the code readability a bit. // But I don't think it's worth doing, since we're aiming for the safe side here. @@ -260,16 +262,24 @@ where if let Some(&last_l_chunk) = left.last() { // In case 'right' part is missing an entry covering the specified era, add it. - match right.first() { + let maybe_chunk = match right.first() { Some(first_r_chunk) if first_r_chunk.get_era() > era.saturating_add(1) => { let mut new_chunk = last_l_chunk.clone(); new_chunk.set_era(era.saturating_add(1)); - right.insert(0, new_chunk); + Some(new_chunk) } None => { let mut new_chunk = last_l_chunk.clone(); new_chunk.set_era(era.saturating_add(1)); - right.insert(0, new_chunk); + Some(new_chunk) + } + _ => None, + }; + + // Only insert the chunk if it's non-zero + match maybe_chunk { + Some(chunk) if chunk.get_amount() > Balance::zero() => { + right.insert(0, chunk); } _ => (), } @@ -775,18 +785,31 @@ where self.staked.subtract_amount(amount, era) } + // TODO: remove this /// Last era for which a stake entry exists. /// If no stake entries exist, returns `None`. pub fn last_stake_era(&self) -> Option { self.staked.0.last().map(|chunk| chunk.era) } + // TODO: remove this /// First entry for which a stake entry exists. /// If no stake entries exist, returns `None`. pub fn oldest_stake_era(&self) -> Option { self.staked.0.first().map(|chunk| chunk.era) } + // TODO + pub fn first_and_last_stake_chunks(&self) -> Option<(StakeChunk, StakeChunk)> { + let first = self.staked.0.first().map(|chunk| *chunk); + let last = self.staked.0.last().map(|chunk| *chunk); + + match (first, last) { + (Some(first), Some(last)) => Some((first, last)), + _ => None, + } + } + /// Notify ledger that all `stake` rewards have been claimed for the staked era. pub fn all_stake_rewards_claimed(&mut self) { // TODO: improve handling once bonus reward tracking is added