diff --git a/pallets/dapp-staking-v3/src/lib.rs b/pallets/dapp-staking-v3/src/lib.rs index aae0dd082b..7596bfe0df 100644 --- a/pallets/dapp-staking-v3/src/lib.rs +++ b/pallets/dapp-staking-v3/src/lib.rs @@ -223,6 +223,7 @@ pub mod pallet { }, BonusReward { account: T::AccountId, + smart_contract: T::SmartContract, period: PeriodNumber, amount: Balance, }, @@ -289,14 +290,10 @@ pub mod pallet { InternalUnstakeError, /// Rewards are no longer claimable since they are too old. RewardExpired, - /// All staker rewards for the period have been claimed. - StakerRewardsAlreadyClaimed, /// There are no claimable rewards. NoClaimableRewards, /// An unexpected error occured while trying to claim staker rewards. InternalClaimStakerError, - /// Bonus rewards have already been claimed. - BonusRewardAlreadyClaimed, /// Account is has no eligible stake amount for bonus reward. NotEligibleForBonusReward, /// An unexpected error occured while trying to claim bonus reward. @@ -840,7 +837,7 @@ pub mod pallet { 0 }; - // TODO: discussion point - it's possible that this will "kill" users ability to withdraw past rewards. + // TODO: discussion point - this will "kill" users ability to withdraw past rewards. // This can be handled by the frontend though. Self::update_ledger(&account, ledger); @@ -1130,10 +1127,7 @@ pub mod pallet { // Check if the rewards have expired let protocol_state = ActiveProtocolState::::get(); ensure!( - staked_period - >= protocol_state - .period_number() - .saturating_sub(T::RewardRetentionInPeriods::get()), + staked_period >= Self::oldest_claimable_period(protocol_state.period_number()), Error::::RewardExpired ); @@ -1203,8 +1197,7 @@ pub mod pallet { Ok(()) } - // TODO: perhaps this should be changed to include smart contract from which rewards are being claimed. - /// Used to claim bonus reward for the eligible era, if applicable. + /// Used to claim bonus reward for a smart contract, if eligible. #[pallet::call_index(12)] #[pallet::weight(Weight::zero())] pub fn claim_bonus_reward( @@ -1218,7 +1211,10 @@ pub mod pallet { .ok_or(Error::::NoClaimableRewards)?; let protocol_state = ActiveProtocolState::::get(); - // Check if period has ended + // Ensure: + // 1. Period for which rewards are being claimed has ended. + // 2. Account has been a loyal staker. + // 3. Rewards haven't expired. let staked_period = staker_info.period_number(); ensure!( staked_period < protocol_state.period_number(), @@ -1230,15 +1226,10 @@ pub mod pallet { ); ensure!( staker_info.period_number() - >= protocol_state - .period_number() - .saturating_sub(T::RewardRetentionInPeriods::get()), + >= Self::oldest_claimable_period(protocol_state.period_number()), Error::::RewardExpired ); - // Check if user is applicable for bonus reward - let eligible_amount = staker_info.staked_amount(PeriodType::Voting); - let period_end_info = PeriodEnd::::get(&staked_period).ok_or(Error::::InternalClaimBonusError)?; // Defensive check - we should never get this far in function if no voting period stake exists. @@ -1247,6 +1238,7 @@ pub mod pallet { Error::::InternalClaimBonusError ); + let eligible_amount = staker_info.staked_amount(PeriodType::Voting); let bonus_reward = Perbill::from_rational(eligible_amount, period_end_info.total_vp_stake) * period_end_info.bonus_reward_pool; @@ -1260,6 +1252,7 @@ pub mod pallet { Self::deposit_event(Event::::BonusReward { account: account.clone(), + smart_contract, period: staked_period, amount: bonus_reward, }); @@ -1287,6 +1280,11 @@ pub mod pallet { // 'Consume' dApp reward for the specified era, if possible. let mut dapp_tiers = DAppTiers::::get(&era).ok_or(Error::::NoDAppTierInfo)?; + ensure!( + Self::oldest_claimable_period(dapp_tiers.period) <= protocol_state.period_number(), + Error::::RewardExpired + ); + let (amount, tier_id) = dapp_tiers .try_consume(dapp_info.id) @@ -1408,8 +1406,7 @@ pub mod pallet { } // Remove expired ledger stake entries, if needed. - let threshold_period = - current_period.saturating_sub(T::RewardRetentionInPeriods::get()); + let threshold_period = Self::oldest_claimable_period(current_period); let mut ledger = Ledger::::get(&account); if ledger.maybe_cleanup_expired(threshold_period) { Self::update_ledger(&account, ledger); @@ -1476,6 +1473,12 @@ pub mod pallet { era.saturating_sub(era % T::EraRewardSpanLength::get()) } + /// Return the oldest period for which rewards can be claimed. + /// All rewards before that period are considered to be expired. + pub fn oldest_claimable_period(current_period: PeriodNumber) -> PeriodNumber { + current_period.saturating_sub(T::RewardRetentionInPeriods::get()) + } + /// Assign eligible dApps into appropriate tiers, and calculate reward for each tier. /// /// The returned object contains information about each dApp that made it into a tier. @@ -1541,7 +1544,7 @@ pub mod pallet { // 2. dApp doesn't satisfy the tier threshold (since they're sorted, none of the following dApps will satisfy the condition either) for (dapp_id, stake_amount) in dapp_stakes[global_idx..max_idx].iter() { if tier_threshold.is_satisfied(*stake_amount) { - global_idx.saturating_accrue(1); + global_idx.saturating_inc(); dapp_tiers.push(DAppTier { dapp_id: *dapp_id, tier_id: Some(tier_id), @@ -1551,7 +1554,7 @@ pub mod pallet { } } - tier_id.saturating_accrue(1); + tier_id.saturating_inc(); } // 4. @@ -1571,6 +1574,7 @@ pub mod pallet { DAppTierRewards::, T::NumberOfTiers>::new( dapp_tiers, tier_rewards, + period, ) .unwrap_or_default() } diff --git a/pallets/dapp-staking-v3/src/test/mock.rs b/pallets/dapp-staking-v3/src/test/mock.rs index d42874cb54..6de7dc0b61 100644 --- a/pallets/dapp-staking-v3/src/test/mock.rs +++ b/pallets/dapp-staking-v3/src/test/mock.rs @@ -199,7 +199,7 @@ impl ExtBuilder { maintenance: false, }); - // TODO: improve this laterm should be handled via genesis? + // TODO: improve this later, should be handled via genesis? let tier_params = TierParameters::<::NumberOfTiers> { reward_portion: BoundedVec::try_from(vec![ Permill::from_percent(40), diff --git a/pallets/dapp-staking-v3/src/test/testing_utils.rs b/pallets/dapp-staking-v3/src/test/testing_utils.rs index 9cb1463be3..c3c06f8534 100644 --- a/pallets/dapp-staking-v3/src/test/testing_utils.rs +++ b/pallets/dapp-staking-v3/src/test/testing_utils.rs @@ -484,7 +484,7 @@ pub(crate) fn assert_stake( pre_ledger.stakeable_amount(stake_period) - amount, "Stakeable amount must decrease by the 'amount'" ); - // TODO: maybe expand checks here? + // TODO: expand with more detailed checks of staked and staked_future // 2. verify staker info // ===================== @@ -885,6 +885,7 @@ pub(crate) fn assert_claim_bonus_reward(account: AccountId, smart_contract: &Moc )); System::assert_last_event(RuntimeEvent::DappStaking(Event::BonusReward { account, + smart_contract: *smart_contract, period: staked_period, amount: reward, })); diff --git a/pallets/dapp-staking-v3/src/test/tests.rs b/pallets/dapp-staking-v3/src/test/tests.rs index 0f5801de52..ab33654c49 100644 --- a/pallets/dapp-staking-v3/src/test/tests.rs +++ b/pallets/dapp-staking-v3/src/test/tests.rs @@ -102,6 +102,33 @@ fn maintenace_mode_call_filtering_works() { DappStaking::unstake(RuntimeOrigin::signed(1), MockSmartContract::default(), 100), Error::::Disabled ); + assert_noop!( + DappStaking::claim_staker_rewards(RuntimeOrigin::signed(1)), + Error::::Disabled + ); + assert_noop!( + DappStaking::claim_bonus_reward(RuntimeOrigin::signed(1), MockSmartContract::default()), + Error::::Disabled + ); + assert_noop!( + DappStaking::claim_dapp_reward( + RuntimeOrigin::signed(1), + MockSmartContract::default(), + 1 + ), + Error::::Disabled + ); + assert_noop!( + DappStaking::unstake_from_unregistered( + RuntimeOrigin::signed(1), + MockSmartContract::default() + ), + Error::::Disabled + ); + assert_noop!( + DappStaking::cleanup_expired_entries(RuntimeOrigin::signed(1)), + Error::::Disabled + ); }) } diff --git a/pallets/dapp-staking-v3/src/test/tests_types.rs b/pallets/dapp-staking-v3/src/test/tests_types.rs index 62cd148911..1211792180 100644 --- a/pallets/dapp-staking-v3/src/test/tests_types.rs +++ b/pallets/dapp-staking-v3/src/test/tests_types.rs @@ -335,9 +335,6 @@ fn account_ledger_add_stake_amount_basic_example_works() { .is_ok()); assert!(acc_ledger.staked.is_empty()); assert!(acc_ledger.staked_future.is_none()); - assert!(acc_ledger - .staked_amount_for_type(PeriodType::Voting, period_number) - .is_zero()); // 1st scenario - stake some amount, and ensure values are as expected. let first_era = 1; diff --git a/pallets/dapp-staking-v3/src/types.rs b/pallets/dapp-staking-v3/src/types.rs index 04b3366915..fec4351457 100644 --- a/pallets/dapp-staking-v3/src/types.rs +++ b/pallets/dapp-staking-v3/src/types.rs @@ -740,7 +740,7 @@ impl Iterator for EraStakePairIter { // 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); + self.start_era.saturating_inc(); return Some(value); } else { None @@ -1559,7 +1559,9 @@ pub struct DAppTierRewards, NT: Get> { pub dapps: BoundedVec, /// Rewards for each tier. First entry refers to the first tier, and so on. pub rewards: BoundedVec, - // TODO: perhaps I can add 'PeriodNumber' here so it's easy to identify expired rewards? + /// Period during which this struct was created. + #[codec(compact)] + pub period: PeriodNumber, } impl, NT: Get> Default for DAppTierRewards { @@ -1567,6 +1569,7 @@ impl, NT: Get> Default for DAppTierRewards { Self { dapps: BoundedVec::default(), rewards: BoundedVec::default(), + period: 0, } } } @@ -1574,10 +1577,18 @@ impl, NT: Get> Default for DAppTierRewards { impl, NT: Get> DAppTierRewards { /// Attempt to construct `DAppTierRewards` struct. /// If the provided arguments exceed the allowed capacity, return an error. - pub fn new(dapps: Vec, rewards: Vec) -> Result { + pub fn new( + dapps: Vec, + rewards: Vec, + period: PeriodNumber, + ) -> Result { let dapps = BoundedVec::try_from(dapps).map_err(|_| ())?; let rewards = BoundedVec::try_from(rewards).map_err(|_| ())?; - Ok(Self { dapps, rewards }) + Ok(Self { + dapps, + rewards, + period, + }) } /// Consume reward for the specified dapp id, returning its amount and tier Id.