dApp staking v3 - claim staker benchmark fix (#1151)

pallets/dapp-staking-v3/src/benchmarking/mod.rs

@@ -426,7 +426,7 @@ mod benchmarks {
     }
 
     #[benchmark]
-    fn claim_staker_rewards_past_period(x: Linear<1, { T::EraRewardSpanLength::get() }>) {
+    fn claim_staker_rewards_past_period(x: Linear<1, { max_claim_size_past_period::<T>() }>) {
         initial_config::<T>();
 
         // Prepare staker & register smart contract
@@ -452,15 +452,16 @@ mod benchmarks {
             amount
         ));
 
-        // Advance to era just after the last era covered by the first span
-        force_advance_to_era::<T>(T::EraRewardSpanLength::get());
+        // Hacky era advancement to ensure we have the exact number of eras to claim, but are already in the next period.
+        force_advance_to_era::<T>(max_claim_size_past_period::<T>() - 1);
+        force_advance_to_next_period::<T>();
 
-        // Hack - modify staker's stake so it seems as if stake was valid from the 'first stake era'/
+        // Hack - modify staker's stake so it seems as if stake was valid from the 'first stake era'.
         // Also fill up the reward span.
         //
         // This allows us to easily control how many rewards are claimed, without having to advance large amount of blocks/eras/periods
         // to find an appropriate scenario.
-        let first_stake_era = T::EraRewardSpanLength::get() - x;
+        let first_stake_era = max_claim_size_past_period::<T>() - x;
         Ledger::<T>::mutate(&staker, |ledger| {
             ledger.staked = ledger.staked_future.unwrap();
             ledger.staked_future = None;
@@ -481,9 +482,6 @@ mod benchmarks {
         }
         EraRewards::<T>::insert(&0, reward_span);
 
-        // This ensures we claim from the past period.
-        force_advance_to_next_period::<T>();
-
         // For testing purposes
         System::<T>::reset_events();
 
@@ -499,7 +497,7 @@ mod benchmarks {
     }
 
     #[benchmark]
-    fn claim_staker_rewards_ongoing_period(x: Linear<1, { T::EraRewardSpanLength::get() }>) {
+    fn claim_staker_rewards_ongoing_period(x: Linear<1, { max_claim_size_ongoing_period::<T>() }>) {
         initial_config::<T>();
 
         // Prepare staker & register smart contract
@@ -525,16 +523,20 @@ mod benchmarks {
             amount
         ));
 
-        // Advance to era just after the last era covered by the first span
-        // This means we'll be able to claim all of the rewards from the previous span.
-        force_advance_to_era::<T>(T::EraRewardSpanLength::get());
+        // Advance to era at the end of the first period or first span.
+        force_advance_to_era::<T>(max_claim_size_ongoing_period::<T>());
+        assert_eq!(
+            ActiveProtocolState::<T>::get().period_number(),
+            1,
+            "Sanity check, we must still be in the first period."
+        );
 
         // Hack - modify staker's stake so it seems as if stake was valid from the 'first stake era'/
         // Also fill up the reward span.
         //
         // This allows us to easily control how many rewards are claimed, without having to advance large amount of blocks/eras/periods
         // to find an appropriate scenario.
-        let first_stake_era = T::EraRewardSpanLength::get() - x;
+        let first_stake_era = max_claim_size_ongoing_period::<T>() - x;
         Ledger::<T>::mutate(&staker, |ledger| {
             ledger.staked = ledger.staked_future.unwrap();
             ledger.staked_future = None;

pallets/dapp-staking-v3/src/benchmarking/utils.rs

@@ -53,7 +53,7 @@ pub(super) fn advance_to_era<T: Config>(era: EraNumber) {
 ///
 /// Relies on the `force` approach to advance one era per block.
 pub(super) fn force_advance_to_era<T: Config>(era: EraNumber) {
-    assert!(era >= ActiveProtocolState::<T>::get().era);
+    assert!(era > ActiveProtocolState::<T>::get().era);
     while ActiveProtocolState::<T>::get().era < era {
         assert_ok!(DappStaking::<T>::force(
             RawOrigin::Root.into(),
@@ -260,3 +260,18 @@ fn trivial_fisher_yates_shuffle<T>(vector: &mut Vec<T>, random_seed: u64) {
         rng = (rng.wrapping_mul(8427637) + 1) as usize; // Some random number generation
     }
 }
+
+/// Returns max amount of rewards that can be claimed in a single claim reward call from a past period.
+///
+/// Bounded by era reward span length & number of eras per period (not length but absolute number).
+pub(super) fn max_claim_size_past_period<T: Config>() -> u32 {
+    T::EraRewardSpanLength::get().min(T::CycleConfiguration::eras_per_build_and_earn_subperiod())
+}
+
+/// Returns max amount of rewards that can be claimed in a single claim reward call from an ongoing period.
+///
+/// Bounded by era reward span length & number of eras per period (not length but absolute number).
+pub(super) fn max_claim_size_ongoing_period<T: Config>() -> u32 {
+    T::EraRewardSpanLength::get()
+        .min(T::CycleConfiguration::eras_per_build_and_earn_subperiod() - 1)
+}

pallets/dapp-staking-v3/src/test/tests.rs

@@ -1459,7 +1459,7 @@ fn claim_staker_rewards_no_claimable_rewards_fails() {
 }
 
 #[test]
-fn claim_staker_rewards_after_expiry_fails() {
+fn claim_staker_rewards_era_after_expiry_works() {
     ExtBuilder::build().execute_with(|| {
         // Register smart contract, lock&stake some amount
         let dev_account = 1;
@@ -1486,17 +1486,34 @@ fn claim_staker_rewards_after_expiry_fails() {
                 .next_subperiod_start_era
                 - 1,
         );
+
+        // Claim must still work
         assert_claim_staker_rewards(account);
+    })
+}
 
-        // Ensure we're still in the first period for the sake of test validity
-        assert_eq!(
-            Ledger::<Test>::get(&account).staked.period,
-            1,
-            "Sanity check."
+#[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::wasm(1 as AccountId);
+        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 =
+            <Test as Config>::RewardRetentionInPeriods::get();
+
+        // Advance to the period at which rewards expire.
+        advance_to_period(
+            ActiveProtocolState::<Test>::get().period_number() + reward_retention_in_periods + 1,
         );
 
-        // Trigger next period, rewards should be marked as expired
-        advance_to_next_era();
         assert_eq!(
             ActiveProtocolState::<Test>::get().period_number(),
             reward_retention_in_periods + 2