Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dApp staking v3 - claim staker benchmark fix #1151

Merged
merged 1 commit into from
Jan 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 15 additions & 13 deletions pallets/dapp-staking-v3/src/benchmarking/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand All @@ -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();

Expand All @@ -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
Expand All @@ -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;
Expand Down
17 changes: 16 additions & 1 deletion pallets/dapp-staking-v3/src/benchmarking/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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)
}
33 changes: 25 additions & 8 deletions pallets/dapp-staking-v3/src/test/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down
Loading