From 3480405b10ecbdf19a72ae8ce802e8719b43014c Mon Sep 17 00:00:00 2001 From: Aramik Date: Tue, 6 Aug 2024 13:12:46 -0700 Subject: [PATCH 1/4] some minor refactoring --- designdocs/provider_boosting_implementation.md | 2 +- pallets/capacity/src/benchmarking.rs | 2 +- pallets/capacity/src/lib.rs | 11 ++++------- pallets/capacity/src/tests/mock.rs | 9 ++------- pallets/capacity/src/types.rs | 11 +---------- pallets/frequency-tx-payment/src/tests/mock.rs | 2 +- runtime/frequency/src/lib.rs | 2 +- 7 files changed, 11 insertions(+), 28 deletions(-) diff --git a/designdocs/provider_boosting_implementation.md b/designdocs/provider_boosting_implementation.md index 1337db9d7a..21f73638d0 100644 --- a/designdocs/provider_boosting_implementation.md +++ b/designdocs/provider_boosting_implementation.md @@ -178,7 +178,7 @@ pub trait Config: frame_system::Config { type MaxRetargetsPerRewardEra: Get; /// The fixed size of the reward pool in each Reward Era. - type RewardPoolEachEra: Get>; + type RewardPoolPerEra: Get>; /// the percentage cap per era of an individual Provider Boost reward type RewardPercentCap: Get; diff --git a/pallets/capacity/src/benchmarking.rs b/pallets/capacity/src/benchmarking.rs index 0f0358dbb7..0e40c3eb2d 100644 --- a/pallets/capacity/src/benchmarking.rs +++ b/pallets/capacity/src/benchmarking.rs @@ -163,7 +163,7 @@ benchmarks! { start_new_reward_era_if_needed { let current_block: BlockNumberFor = 1_209_600u32.into(); let history_limit: u32 = ::ProviderBoostHistoryLimit::get(); - let total_reward_pool: BalanceOf = ::RewardPoolEachEra::get(); + let total_reward_pool: BalanceOf = ::RewardPoolPerEra::get(); let unclaimed_balance: BalanceOf = 5_000u32.into(); let total_staked_token: BalanceOf = 5_000u32.into(); let started_at: BlockNumberFor = current_block.saturating_sub(::EraLength::get().into()); diff --git a/pallets/capacity/src/lib.rs b/pallets/capacity/src/lib.rs index 76e99705b7..3cc5f9dc2c 100644 --- a/pallets/capacity/src/lib.rs +++ b/pallets/capacity/src/lib.rs @@ -127,7 +127,7 @@ pub mod pallet { /// The maximum number of unlocking chunks a StakingAccountLedger can have. /// It determines how many concurrent unstaked chunks may exist. #[pallet::constant] - type MaxUnlockingChunks: Get + Clone; + type MaxUnlockingChunks: Get; #[cfg(feature = "runtime-benchmarks")] /// A set of helper functions for benchmarking. @@ -178,7 +178,7 @@ pub mod pallet { /// The fixed size of the reward pool in each Reward Era. #[pallet::constant] - type RewardPoolEachEra: Get>; + type RewardPoolPerEra: Get>; /// the percentage cap per era of an individual Provider Boost reward #[pallet::constant] @@ -1117,7 +1117,7 @@ impl Pallet { let earned_amount = ::RewardsProvider::era_staking_reward( eligible_amount, total_for_era, - T::RewardPoolEachEra::get(), + T::RewardPoolPerEra::get(), ); unclaimed_rewards .try_push(UnclaimedRewardInfo { @@ -1315,13 +1315,10 @@ impl Replenishable for Pallet { } impl ProviderBoostRewardsProvider for Pallet { - type AccountId = T::AccountId; - type RewardEra = common_primitives::capacity::RewardEra; - type Hash = T::Hash; type Balance = BalanceOf; fn reward_pool_size(_total_staked: Self::Balance) -> Self::Balance { - T::RewardPoolEachEra::get() + T::RewardPoolPerEra::get() } /// Calculate the reward for a single era. We don't care about the era number, diff --git a/pallets/capacity/src/tests/mock.rs b/pallets/capacity/src/tests/mock.rs index 25ab94bdb3..a82608bf47 100644 --- a/pallets/capacity/src/tests/mock.rs +++ b/pallets/capacity/src/tests/mock.rs @@ -5,7 +5,7 @@ use crate::{ ProviderBoostRewardsProvider, RewardPoolHistoryChunk, STAKED_PERCENTAGE_TO_BOOST, }; use common_primitives::{ - node::{AccountId, Hash, ProposalProvider}, + node::{AccountId, ProposalProvider}, schema::{SchemaId, SchemaValidator}, }; use frame_support::{ @@ -145,12 +145,7 @@ impl pallet_msa::Config for Test { // not used yet pub struct TestRewardsProvider {} -type TestRewardEra = u32; - impl ProviderBoostRewardsProvider for TestRewardsProvider { - type AccountId = u64; - type RewardEra = TestRewardEra; - type Hash = Hash; // use what's in common_primitives::node type Balance = BalanceOf; // To reflect new economic model behavior of having a constant RewardPool amount. @@ -199,7 +194,7 @@ impl pallet_capacity::Config for Test { type ProviderBoostHistoryLimit = ConstU32<12>; type RewardsProvider = Capacity; type MaxRetargetsPerRewardEra = ConstU32<5>; - type RewardPoolEachEra = ConstU64<10_000>; + type RewardPoolPerEra = ConstU64<10_000>; type RewardPercentCap = TestRewardCap; type RewardPoolChunkLength = ConstU32<3>; } diff --git a/pallets/capacity/src/types.rs b/pallets/capacity/src/types.rs index 3bacd269b8..361a7d1ca8 100644 --- a/pallets/capacity/src/types.rs +++ b/pallets/capacity/src/types.rs @@ -15,7 +15,7 @@ use sp_runtime::{ use sp_std::vec::Vec; /// How much, as a percentage of staked token, to boost a targeted Provider when staking. -/// this value should be between [0,100] +/// this value should be between 0 and 100 pub const STAKED_PERCENTAGE_TO_BOOST: u32 = 50; #[derive( @@ -520,15 +520,6 @@ impl RetargetInfo { /// A trait that provides the Economic Model for Provider Boosting. pub trait ProviderBoostRewardsProvider { - /// the AccountId this provider is using - type AccountId; - - /// the range of blocks over which a Reward Pool is determined and rewards are paid out - type RewardEra; - - /// The hasher to use for proofs - type Hash; - /// The type for currency type Balance; diff --git a/pallets/frequency-tx-payment/src/tests/mock.rs b/pallets/frequency-tx-payment/src/tests/mock.rs index e0c8c440df..d699fabe6f 100644 --- a/pallets/frequency-tx-payment/src/tests/mock.rs +++ b/pallets/frequency-tx-payment/src/tests/mock.rs @@ -230,7 +230,7 @@ impl pallet_capacity::Config for Test { type ProviderBoostHistoryLimit = ConstU32<6>; type RewardsProvider = Capacity; type MaxRetargetsPerRewardEra = ConstU32<5>; - type RewardPoolEachEra = ConstU64<10_000>; + type RewardPoolPerEra = ConstU64<10_000>; type RewardPercentCap = TestRewardCap; type RewardPoolChunkLength = ConstU32<2>; } diff --git a/runtime/frequency/src/lib.rs b/runtime/frequency/src/lib.rs index 50acda7c1b..449bfcec2c 100644 --- a/runtime/frequency/src/lib.rs +++ b/runtime/frequency/src/lib.rs @@ -539,7 +539,7 @@ impl pallet_capacity::Config for Runtime { type RewardsProvider = Capacity; type MaxRetargetsPerRewardEra = ConstU32<2>; // Value determined by desired inflation rate limits for chosen economic model - type RewardPoolEachEra = ConstU128<{ currency::CENTS.saturating_mul(172_602_740u128) }>; + type RewardPoolPerEra = ConstU128<{ currency::CENTS.saturating_mul(172_602_740u128) }>; type RewardPercentCap = CapacityRewardCap; // Must evenly divide ProviderBoostHistoryLimit type RewardPoolChunkLength = ConstU32<5>; From 56df16dda3b02147b11b107c0b213fb3e31cad6f Mon Sep 17 00:00:00 2001 From: Shannon Wells Date: Tue, 6 Aug 2024 13:49:33 -0700 Subject: [PATCH 2/4] Make RewardEras zero-indexed (#2115) # Goal Reward eras were 1-indexed but this made things too confusing. --- e2e/capacity/provider_boost.test.ts | 8 +- e2e/package-lock.json | 2 +- e2e/scaffolding/helpers.ts | 1 - pallets/capacity/src/lib.rs | 30 +---- .../src/migration/provider_boost_init.rs | 8 +- .../src/tests/claim_staking_rewards_tests.rs | 21 +-- pallets/capacity/src/tests/eras_tests.rs | 125 +++++++++++------- pallets/capacity/src/tests/mock.rs | 2 +- .../src/tests/provider_boost_history_tests.rs | 4 +- .../capacity/src/tests/reward_pool_tests.rs | 43 +++--- .../src/tests/rewards_provider_tests.rs | 101 ++++++++++---- pallets/capacity/src/tests/testing_utils.rs | 10 +- pallets/capacity/src/tests/unstaking_tests.rs | 57 ++++---- 13 files changed, 244 insertions(+), 168 deletions(-) diff --git a/e2e/capacity/provider_boost.test.ts b/e2e/capacity/provider_boost.test.ts index 949ea368b9..546e8ceccf 100644 --- a/e2e/capacity/provider_boost.test.ts +++ b/e2e/capacity/provider_boost.test.ts @@ -7,10 +7,11 @@ import { CENTS, DOLLARS, createAndFundKeypair, - boostProvider, + boostProvider, stakeToProvider, } from '../scaffolding/helpers'; const fundingSource = getFundingSource('capacity-provider-boost'); +const tokenMinStake: bigint = 1n * CENTS; describe('Capacity: provider_boost extrinsic', function () { const providerBalance = 2n * DOLLARS; @@ -25,14 +26,15 @@ describe('Capacity: provider_boost extrinsic', function () { it('fails when staker is a Maximized Capacity staker', async function () { const stakeKeys = createKeys('booster'); const provider = await createMsaAndProvider(fundingSource, stakeKeys, 'Provider1', providerBalance); - await assert.rejects(boostProvider(fundingSource, stakeKeys, provider, 1n * DOLLARS), {name: "CannotChangeStakingType"}); + await assert.doesNotReject(stakeToProvider(fundingSource, stakeKeys, provider, tokenMinStake)); + await assert.rejects(boostProvider(fundingSource, stakeKeys, provider, tokenMinStake), {name: "CannotChangeStakingType"}); }); it("fails when staker doesn't have enough token", async function () { const stakeKeys = createKeys('booster'); const provider = await createMsaAndProvider(fundingSource, stakeKeys, 'Provider1', providerBalance); const booster = await createAndFundKeypair(fundingSource, 1n * DOLLARS, 'booster'); - await assert.rejects(boostProvider(booster, booster, provider, 1n * DOLLARS), {name: "InsufficientCapacityBalance"}); + await assert.rejects(boostProvider(booster, booster, provider, 1n * DOLLARS), {name: "BalanceTooLowtoStake"}); }); it('staker can boost multiple times', async function () { diff --git a/e2e/package-lock.json b/e2e/package-lock.json index 29dc74bfa3..08aea7a79f 100644 --- a/e2e/package-lock.json +++ b/e2e/package-lock.json @@ -2471,7 +2471,7 @@ "node_modules/@frequency-chain/api-augment": { "version": "0.0.0", "resolved": "file:../js/api-augment/dist/frequency-chain-api-augment-0.0.0.tgz", - "integrity": "sha512-7prkQiSHJ0aCa9vqR6g5HwVZ55rstu94m1FjQvJQO7MIt8BgSQYAsP0N1pc2CmtN0k72wYeGRV3+nLnWkfKG1w==", + "integrity": "sha512-c30YAG6cOgSQPYQFJ5MLPUNR1mn0y0QTxf5YauEtafALIOXodrVCBCd+63R10k1X+fYNEKiuqved60kCmAabcw==", "dependencies": { "@polkadot/api": "^12.2.3", "@polkadot/rpc-provider": "^12.2.3", diff --git a/e2e/scaffolding/helpers.ts b/e2e/scaffolding/helpers.ts index 3e0854ba2d..48fc952151 100644 --- a/e2e/scaffolding/helpers.ts +++ b/e2e/scaffolding/helpers.ts @@ -448,7 +448,6 @@ export async function boostProvider( const stakeOp = ExtrinsicHelper.providerBoost(keys, providerId, tokensToStake); const { target: stakeEvent } = await stakeOp.fundAndSend(source); assert.notEqual(stakeEvent, undefined, 'stakeToProvider: should have returned Stake event'); - if (stakeEvent) { const stakedCapacity = stakeEvent.data.capacity; diff --git a/pallets/capacity/src/lib.rs b/pallets/capacity/src/lib.rs index 3cc5f9dc2c..95bfb437f4 100644 --- a/pallets/capacity/src/lib.rs +++ b/pallets/capacity/src/lib.rs @@ -268,24 +268,6 @@ pub mod pallet { pub type ProviderBoostHistories = StorageMap<_, Twox64Concat, T::AccountId, ProviderBoostHistory>; - #[pallet::genesis_config] - pub struct GenesisConfig { - /// Phantom type - #[serde(skip)] - pub _config: PhantomData, - } - - #[pallet::genesis_build] - impl BuildGenesisConfig for GenesisConfig { - fn build(&self) { - CurrentEraInfo::::set(RewardEraInfo { - era_index: 1u32.into(), - started_at: 0u32.into(), - }); - CurrentEraProviderBoostTotal::::set(0u32.into()); - } - } - // Simple declaration of the `Pallet` type. It is placeholder we use to implement traits and // method. #[pallet::pallet] @@ -1096,12 +1078,14 @@ impl Pallet { let max_history: u32 = T::ProviderBoostHistoryLimit::get(); let start_era = current_era_info.era_index.saturating_sub((max_history).into()); - let end_era = current_era_info.era_index.saturating_sub(One::one()); + let end_era = current_era_info.era_index.saturating_sub(One::one()); // stop at previous era // start with how much was staked in the era before the earliest for which there are eligible rewards. - let mut previous_amount: BalanceOf = - staking_history.get_amount_staked_for_era(&(start_era.saturating_sub(1u32.into()))); - + let mut previous_amount: BalanceOf = match start_era { + 0 => 0u32.into(), + _ => + staking_history.get_amount_staked_for_era(&(start_era.saturating_sub(1u32.into()))), + }; let mut unclaimed_rewards: BoundedVec< UnclaimedRewardInfo, BlockNumberFor>, T::ProviderBoostHistoryLimit, @@ -1183,7 +1167,7 @@ impl Pallet { let history_limit: u32 = T::ProviderBoostHistoryLimit::get(); let chunk_len = T::RewardPoolChunkLength::get(); // Remove one because eras are 1 indexed - let era_u32: u32 = era.saturating_sub(One::one()).into(); + let era_u32: u32 = era; // Add one chunk so that we always have the full history limit in our chunks let cycle: u32 = era_u32 % history_limit.saturating_add(chunk_len); diff --git a/pallets/capacity/src/migration/provider_boost_init.rs b/pallets/capacity/src/migration/provider_boost_init.rs index a5934a07bc..3a4747beda 100644 --- a/pallets/capacity/src/migration/provider_boost_init.rs +++ b/pallets/capacity/src/migration/provider_boost_init.rs @@ -4,7 +4,6 @@ use frame_support::{ traits::{Get, OnRuntimeUpgrade}, }; -use common_primitives::capacity::RewardEra; #[cfg(feature = "try-runtime")] use sp_std::vec::Vec; @@ -15,9 +14,10 @@ impl OnRuntimeUpgrade for ProviderBoostInit { fn on_runtime_upgrade() -> Weight { let current_era_info = CurrentEraInfo::::get(); // 1r if current_era_info.eq(&RewardEraInfo::default()) { - let current_block = frame_system::Pallet::::block_number(); // Whitelisted - let era_index: RewardEra = 1u32.into(); - CurrentEraInfo::::set(RewardEraInfo { era_index, started_at: current_block }); // 1w + CurrentEraInfo::::set(RewardEraInfo { + era_index: 0u32.into(), + started_at: frame_system::Pallet::::block_number(), + }); // 1w CurrentEraProviderBoostTotal::::set(0u32.into()); // 1w T::DbWeight::get().reads_writes(2, 1) } else { diff --git a/pallets/capacity/src/tests/claim_staking_rewards_tests.rs b/pallets/capacity/src/tests/claim_staking_rewards_tests.rs index 648c0985f6..d66d8dbf29 100644 --- a/pallets/capacity/src/tests/claim_staking_rewards_tests.rs +++ b/pallets/capacity/src/tests/claim_staking_rewards_tests.rs @@ -21,11 +21,11 @@ fn claim_staking_rewards_leaves_one_history_item_for_current_era() { setup_provider(&account, &target, &amount, ProviderBoost); run_to_block(31); - assert_eq!(CurrentEraInfo::::get().era_index, 4u32); + assert_eq!(CurrentEraInfo::::get().era_index, 3u32); let current_history = ProviderBoostHistories::::get(account).unwrap(); assert_eq!(current_history.count(), 1usize); - let history_item = current_history.get_entry_for_era(&1u32).unwrap(); + let history_item = current_history.get_entry_for_era(&0u32).unwrap(); assert_eq!(*history_item, amount); }) } @@ -57,27 +57,28 @@ fn claim_staking_rewards_mints_and_transfers_expected_total() { setup_provider(&account, &target, &amount, ProviderBoost); run_to_block(31); - assert_eq!(CurrentEraInfo::::get().era_index, 4u32); + assert_eq!(CurrentEraInfo::::get().era_index, 3u32); assert_ok!(Capacity::claim_staking_rewards(RuntimeOrigin::signed(account))); System::assert_last_event( Event::::ProviderBoostRewardClaimed { account, reward_amount: 8u64 }.into(), ); - // should have 2 era's worth of payouts: 4 each for eras 2, 3 + // should have 2 era's worth of payouts: 4 each for eras 1, 2 assert_eq!(get_balance::(&account), 10_008u64); // the reward value is unlocked assert_transferable::(&account, 8u64); - run_to_block(51); - assert_eq!(CurrentEraInfo::::get().era_index, 6u32); + run_to_block(41); + assert_eq!(CurrentEraInfo::::get().era_index, 4u32); assert_ok!(Capacity::claim_staking_rewards(RuntimeOrigin::signed(account))); + // rewards available for one more era System::assert_last_event( - ProviderBoostRewardClaimed { account, reward_amount: 8u64 }.into(), + ProviderBoostRewardClaimed { account, reward_amount: 4u64 }.into(), ); - // should have 4 for eras 2-5 - assert_eq!(get_balance::(&account), 10_016u64); - assert_transferable::(&account, 16u64); + // should have 4 for eras 1-3 + assert_eq!(get_balance::(&account), 10_012u64); + assert_transferable::(&account, 12u64); }) } diff --git a/pallets/capacity/src/tests/eras_tests.rs b/pallets/capacity/src/tests/eras_tests.rs index 26732d4f72..b1d2d1de7d 100644 --- a/pallets/capacity/src/tests/eras_tests.rs +++ b/pallets/capacity/src/tests/eras_tests.rs @@ -1,10 +1,10 @@ use super::mock::*; use crate::{ - tests::testing_utils::*, BalanceOf, CurrentEraInfo, CurrentEraProviderBoostTotal, + tests::testing_utils::*, BalanceOf, Config, CurrentEraInfo, CurrentEraProviderBoostTotal, ProviderBoostRewardPools, RewardEraInfo, }; use common_primitives::{capacity::RewardEra, msa::MessageSourceId}; -use frame_support::assert_ok; +use frame_support::{assert_ok, traits::Get}; pub fn boost_provider_and_run_to_end_of_era( staker: u64, @@ -13,10 +13,14 @@ pub fn boost_provider_and_run_to_end_of_era( era: u32, ) { assert_ok!(Capacity::provider_boost(RuntimeOrigin::signed(staker), provider_msa, stake_amount)); - let block_decade: u32 = (era * 10) + 1; - run_to_block(block_decade); - assert_eq!(CurrentEraProviderBoostTotal::::get(), stake_amount * (era as u64)); - system_run_to_block(block_decade + 9); + // want to run to the first block of era so we get the on_initialize updates + let first_block_of_era: u32 = ((era + 1u32) * 10) + 1; + run_to_block(first_block_of_era); + let expected_stake = stake_amount * ((era + 1u32) as u64); + assert_eq!(CurrentEraProviderBoostTotal::::get(), expected_stake); + // system_run to end of era. we don't need the on_initialize updates + let era_length: RewardEra = ::EraLength::get(); + system_run_to_block(first_block_of_era + era_length - 1); } #[test] @@ -29,7 +33,7 @@ fn start_new_era_if_needed_updates_era_info() { let current_era_info = CurrentEraInfo::::get(); - let expected_era = i + 1; + let expected_era = i; assert_eq!( current_era_info, RewardEraInfo { era_index: expected_era, started_at: block_decade } @@ -48,9 +52,15 @@ fn assert_chunk_is_full_and_has_earliest_era_total( total: BalanceOf, ) { let chunk = ProviderBoostRewardPools::::get(chunk_index).unwrap(); - assert_eq!(chunk.is_full(), is_full, "{:?}", chunk); - assert_eq!(chunk.earliest_era(), Some(&era), "{:?}", chunk); - assert_eq!(chunk.total_for_era(&era), Some(&total), "{:?}", chunk); + assert_eq!(chunk.is_full(), is_full, "Chunk {:?} should be full: {:?}", chunk_index, chunk); + assert_eq!(chunk.earliest_era(), Some(&era), "Earliest era should be {:?}, {:?}", era, chunk); + assert_eq!( + chunk.total_for_era(&era), + Some(&total), + "Chunk total should be {:?}, {:?}", + total, + chunk + ); } // gets the last (i.e. latest non-current) stored reward pool era, which is in chunk 0. @@ -62,15 +72,27 @@ fn assert_last_era_total(era: RewardEra, total: BalanceOf) { let chunk = chunk_opt.unwrap(); let (_earliest, latest) = chunk.era_range(); assert_eq!(latest, era); - assert_eq!(chunk.total_for_era(&era), Some(&total)); + assert_eq!( + chunk.total_for_era(&era), + Some(&total), + "Chunk total should be {:?}, {:?}", + total, + chunk + ); } fn assert_chunk_is_empty(chunk_index: u32) { let chunk_opt = ProviderBoostRewardPools::::get(chunk_index); if chunk_opt.is_some() { - assert!(chunk_opt.unwrap().earliest_era().is_none()) + let chunk = chunk_opt.unwrap(); + assert!( + chunk.earliest_era().is_none(), + "Earliest era for chunk {:?} should be None but it is {:?}", + chunk_index, + chunk + ) } else { - assert!(chunk_opt.is_none()) + assert!(chunk_opt.is_none(), "Expected chunk {:?} to be empty, but it's not", chunk_index); } } @@ -85,49 +107,49 @@ fn start_new_era_if_needed_updates_reward_pool() { register_provider(provider_msa, "Binky".to_string()); - for i in [1u32, 2u32, 3u32] { + for i in 0u32..=2u32 { boost_provider_and_run_to_end_of_era(staker, provider_msa, stake_amount, i); } // check that first chunk is filled correctly. - assert_chunk_is_full_and_has_earliest_era_total(0, true, 1, 100); - assert_chunk_is_empty(1); - assert_chunk_is_empty(2); - assert_last_era_total(3, 300); + assert_chunk_is_full_and_has_earliest_era_total(0, true, 0, 100); + for i in 1u32..=4u32 { + assert_chunk_is_empty(i); + } - for i in [4u32, 5u32, 6u32] { + for i in [3u32, 4u32, 5u32] { boost_provider_and_run_to_end_of_era(staker, provider_msa, stake_amount, i); } // No change - assert_chunk_is_full_and_has_earliest_era_total(0, true, 1, 100); + assert_chunk_is_full_and_has_earliest_era_total(0, true, 0, 100); // New Chunk - assert_chunk_is_full_and_has_earliest_era_total(1, true, 4, 400); + assert_chunk_is_full_and_has_earliest_era_total(1, true, 3, 400); assert_chunk_is_empty(2); - assert_last_era_total(6, 600); + assert_last_era_total(5, 600); - for i in [7u32, 8u32, 9u32, 10u32, 11u32, 12u32, 13u32, 14u32, 15u32] { + for i in [6u32, 7u32, 8u32, 9u32, 10u32, 11u32, 12u32, 13u32, 14u32] { boost_provider_and_run_to_end_of_era(staker, provider_msa, stake_amount, i); } // No changes - assert_chunk_is_full_and_has_earliest_era_total(0, true, 1, 100); - assert_chunk_is_full_and_has_earliest_era_total(1, true, 4, 400); + assert_chunk_is_full_and_has_earliest_era_total(0, true, 0, 100); + assert_chunk_is_full_and_has_earliest_era_total(1, true, 3, 400); // New - assert_chunk_is_full_and_has_earliest_era_total(2, true, 7, 700); - assert_chunk_is_full_and_has_earliest_era_total(3, true, 10, 1000); - assert_chunk_is_full_and_has_earliest_era_total(4, true, 13, 1300); - assert_last_era_total(15, 1500); + assert_chunk_is_full_and_has_earliest_era_total(2, true, 6, 700); + assert_chunk_is_full_and_has_earliest_era_total(3, true, 9, 1000); + assert_chunk_is_full_and_has_earliest_era_total(4, true, 12, 1300); + assert_last_era_total(14, 1500); // check that it all rolls over properly. - for i in [16u32, 17u32] { + for i in [15u32, 16u32] { boost_provider_and_run_to_end_of_era(staker, provider_msa, stake_amount, i); } - // No Changes - assert_chunk_is_full_and_has_earliest_era_total(1, true, 4, 400); - assert_chunk_is_full_and_has_earliest_era_total(2, true, 7, 700); - assert_chunk_is_full_and_has_earliest_era_total(3, true, 10, 1000); - assert_chunk_is_full_and_has_earliest_era_total(4, true, 13, 1300); + // No Changes in chunk content for these eras + assert_chunk_is_full_and_has_earliest_era_total(1, true, 3, 400); + assert_chunk_is_full_and_has_earliest_era_total(2, true, 6, 700); + assert_chunk_is_full_and_has_earliest_era_total(3, true, 9, 1000); + assert_chunk_is_full_and_has_earliest_era_total(4, true, 12, 1300); // New - assert_chunk_is_full_and_has_earliest_era_total(0, false, 16, 1600); - assert_last_era_total(17, 1700); + assert_chunk_is_full_and_has_earliest_era_total(0, false, 15, 1600); + assert_last_era_total(16, 1700); // There shouldn't be a chunk 5 ever with this config assert_chunk_is_empty(5); }); @@ -136,12 +158,12 @@ fn start_new_era_if_needed_updates_reward_pool() { #[test] fn get_expiration_block_for_era_works() { new_test_ext().execute_with(|| { - assert_eq!(CurrentEraInfo::::get().era_index, 1u32); - assert_eq!(Capacity::block_at_end_of_era(10u32), 100); + assert_eq!(CurrentEraInfo::::get().era_index, 0u32); + assert_eq!(Capacity::block_at_end_of_era(9u32), 100); - set_era_and_reward_pool(7, 61, 0); - assert_eq!(Capacity::block_at_end_of_era(7u32), 70); - assert_eq!(Capacity::block_at_end_of_era(10u32), 100); + set_era_and_reward_pool(7, 63, 0); + assert_eq!(Capacity::block_at_end_of_era(7u32), 72); + assert_eq!(Capacity::block_at_end_of_era(10u32), 102); set_era_and_reward_pool(10, 91, 0); assert_eq!(Capacity::block_at_end_of_era(10u32), 100); @@ -166,27 +188,28 @@ fn get_chunk_index_for_era_works() { // [16],[4,5,6],[7,8,9],[10,11,12],[13,14,15] for test in { vec![ + TestCase { era: 0, expected: 0 }, TestCase { era: 1, expected: 0 }, TestCase { era: 2, expected: 0 }, - TestCase { era: 3, expected: 0 }, + TestCase { era: 3, expected: 1 }, TestCase { era: 4, expected: 1 }, TestCase { era: 5, expected: 1 }, - TestCase { era: 6, expected: 1 }, + TestCase { era: 6, expected: 2 }, TestCase { era: 7, expected: 2 }, TestCase { era: 8, expected: 2 }, - TestCase { era: 9, expected: 2 }, + TestCase { era: 9, expected: 3 }, TestCase { era: 10, expected: 3 }, TestCase { era: 11, expected: 3 }, - TestCase { era: 12, expected: 3 }, - TestCase { era: 13, expected: 4 }, // This is not wrong; there is an extra chunk to leave space for cycling + TestCase { era: 12, expected: 4 }, // This is not wrong; there is an extra chunk to leave space for cycling + TestCase { era: 13, expected: 4 }, TestCase { era: 14, expected: 4 }, - TestCase { era: 15, expected: 4 }, - TestCase { era: 16, expected: 0 }, // So cycle restarts here, not at 13. + TestCase { era: 15, expected: 0 }, // So cycle restarts here, not at 12. + TestCase { era: 16, expected: 0 }, TestCase { era: 17, expected: 0 }, - TestCase { era: 18, expected: 0 }, + TestCase { era: 18, expected: 1 }, TestCase { era: 22, expected: 2 }, TestCase { era: 55, expected: 3 }, - TestCase { era: 999, expected: 2 }, + TestCase { era: 998, expected: 2 }, ] } { assert_eq!(Capacity::get_chunk_index_for_era(test.era), test.expected, "{:?}", test); diff --git a/pallets/capacity/src/tests/mock.rs b/pallets/capacity/src/tests/mock.rs index a82608bf47..00c793445e 100644 --- a/pallets/capacity/src/tests/mock.rs +++ b/pallets/capacity/src/tests/mock.rs @@ -236,7 +236,7 @@ pub fn new_test_ext() -> sp_io::TestExternalities { ext.execute_with(|| { System::set_block_number(1); initialize_reward_pool(); - set_era_and_reward_pool(1, 1, 0); + set_era_and_reward_pool(0, 1, 0); }); ext } diff --git a/pallets/capacity/src/tests/provider_boost_history_tests.rs b/pallets/capacity/src/tests/provider_boost_history_tests.rs index 0fa790dc0e..ebd12ff05a 100644 --- a/pallets/capacity/src/tests/provider_boost_history_tests.rs +++ b/pallets/capacity/src/tests/provider_boost_history_tests.rs @@ -34,7 +34,7 @@ fn multiple_provider_boosts_updates_history_correctly() { // should update era 1 history let mut history = ProviderBoostHistories::::get(staker).unwrap(); assert_eq!(history.count(), 1); - assert_eq!(history.get_entry_for_era(&1u32).unwrap(), &700u64); + assert_eq!(history.get_entry_for_era(&0u32).unwrap(), &700u64); system_run_to_block(10); run_to_block(11); @@ -44,7 +44,7 @@ fn multiple_provider_boosts_updates_history_correctly() { // should add an era 2 history history = ProviderBoostHistories::::get(staker).unwrap(); assert_eq!(history.count(), 2); - assert_eq!(history.get_entry_for_era(&2u32).unwrap(), &900u64); + assert_eq!(history.get_entry_for_era(&1u32).unwrap(), &900u64); }) } diff --git a/pallets/capacity/src/tests/reward_pool_tests.rs b/pallets/capacity/src/tests/reward_pool_tests.rs index 2f5347d705..849c6475ea 100644 --- a/pallets/capacity/src/tests/reward_pool_tests.rs +++ b/pallets/capacity/src/tests/reward_pool_tests.rs @@ -6,8 +6,9 @@ use common_primitives::capacity::RewardEra; use frame_support::{assert_ok, traits::Get}; use std::ops::Add; -// Check eras_tests for how reward pool chunks are expected to be filled during -// runtime. +// Check eras_tests for how reward pool chunks are expected to be filled during runtime. + +// Fill up a reward pool history chunk by adding 100 in each new era from the starting value. fn fill_reward_pool_history_chunk( chunk_index: u32, starting_era: RewardEra, @@ -67,11 +68,11 @@ fn get_total_stake_for_past_era_works_with_1_full_chunk() { new_test_ext().execute_with(|| { System::set_block_number(52); set_era_and_reward_pool(6, 51, 1000); - fill_reward_pool_history_chunk(0, 1, 3, 100); // eras 1-3 - fill_reward_pool_history_chunk(1, 4, 2, 400); // eras 4,5 - for i in 3u32..=5u32 { + fill_reward_pool_history_chunk(0, 0, 3, 0); // eras 1-3 + fill_reward_pool_history_chunk(1, 3, 2, 300); // eras 4,5 + for i in 2u32..=4u32 { let expected_total: BalanceOf = (i * 100u32).into(); - let actual = Capacity::get_total_stake_for_past_era(i, 6); + let actual = Capacity::get_total_stake_for_past_era(i, 5); assert_eq!(actual, Ok(expected_total)); } assert!(Capacity::get_total_stake_for_past_era(6, 6).is_err()); @@ -82,15 +83,15 @@ fn get_total_stake_for_past_era_works_with_1_full_chunk() { fn get_total_stake_for_past_era_works_with_2_full_chunks() { new_test_ext().execute_with(|| { System::set_block_number(72); - set_era_and_reward_pool(8, 71, 1000); - fill_reward_pool_history_chunk(0, 1, 3, 100); - fill_reward_pool_history_chunk(1, 4, 3, 400); - fill_reward_pool_history_chunk(2, 7, 1, 700); - for i in 1u32..=7u32 { + set_era_and_reward_pool(7, 71, 1000); + fill_reward_pool_history_chunk(0, 0, 3, 0); + fill_reward_pool_history_chunk(1, 3, 3, 300); + fill_reward_pool_history_chunk(2, 6, 1, 600); + for i in 0u32..=6u32 { let expected_total: BalanceOf = (i * 100u32).into(); - assert_eq!(Capacity::get_total_stake_for_past_era(i, 8), Ok(expected_total)); + assert_eq!(Capacity::get_total_stake_for_past_era(i, 7), Ok(expected_total)); } - assert!(Capacity::get_total_stake_for_past_era(8, 8).is_err()); + assert!(Capacity::get_total_stake_for_past_era(7, 7).is_err()); }) } @@ -99,17 +100,17 @@ fn get_total_stake_for_past_era_works_with_full_reward_pool() { new_test_ext().execute_with(|| { System::set_block_number(121); let history_limit: u32 = ::ProviderBoostHistoryLimit::get(); - set_era_and_reward_pool(13, 121, (2000u32).into()); + set_era_and_reward_pool(12, 121, (2000u32).into()); - fill_reward_pool_history_chunk(0, 1, 3, 101); - fill_reward_pool_history_chunk(1, 4, 3, 401); - fill_reward_pool_history_chunk(2, 7, 3, 701); - fill_reward_pool_history_chunk(3, 10, 3, 1001); + fill_reward_pool_history_chunk(0, 0, 3, 1); + fill_reward_pool_history_chunk(1, 3, 3, 301); + fill_reward_pool_history_chunk(2, 6, 3, 601); + fill_reward_pool_history_chunk(3, 9, 3, 901); - (1u32..=history_limit).for_each(|era| { + (0u32..history_limit).for_each(|era| { let expected_total: BalanceOf = ((era * 100u32) + 1u32).into(); - assert_eq!(Capacity::get_total_stake_for_past_era(era, 13), Ok(expected_total)); + assert_eq!(Capacity::get_total_stake_for_past_era(era, 12), Ok(expected_total)); }); - assert!(Capacity::get_total_stake_for_past_era(13, 13).is_err()); + assert!(Capacity::get_total_stake_for_past_era(12, 12).is_err()); }) } diff --git a/pallets/capacity/src/tests/rewards_provider_tests.rs b/pallets/capacity/src/tests/rewards_provider_tests.rs index 0e65f06ceb..674d2b0674 100644 --- a/pallets/capacity/src/tests/rewards_provider_tests.rs +++ b/pallets/capacity/src/tests/rewards_provider_tests.rs @@ -112,69 +112,70 @@ fn list_unclaimed_rewards_has_eligible_rewards() { run_to_block(31); assert_ok!(Capacity::provider_boost(RuntimeOrigin::signed(account), target, amount)); - run_to_block(51); - assert_eq!(CurrentEraInfo::::get().era_index, 6u32); + run_to_block(51); // era 5 + assert_eq!(CurrentEraInfo::::get().era_index, 5u32); assert_eq!(CurrentEraInfo::::get().started_at, 51u32); // rewards for era 6 should not be returned; era 6 is current era and therefore ineligible. // eligible amounts for rewards for eras should be: 1=0, 2=1k, 3=2k, 4=2k, 5=3k let rewards = Capacity::list_unclaimed_rewards(&account).unwrap(); assert_eq!(rewards.len(), 5usize); - let expected_info: [UnclaimedRewardInfo, BlockNumberFor>; 5] = [ + let expected_info: Vec, BlockNumberFor>> = [ UnclaimedRewardInfo { - reward_era: 1u32, + reward_era: 0u32, expires_at_block: 130, staked_amount: 1000, eligible_amount: 0, earned_amount: 0, }, UnclaimedRewardInfo { - reward_era: 2u32, + reward_era: 1u32, expires_at_block: 140, staked_amount: 2000, eligible_amount: 1000, earned_amount: 4, }, UnclaimedRewardInfo { - reward_era: 3u32, + reward_era: 2u32, expires_at_block: 150, staked_amount: 2000, eligible_amount: 2_000, earned_amount: 8, }, UnclaimedRewardInfo { - reward_era: 4u32, + reward_era: 3u32, expires_at_block: 160, staked_amount: 3000, eligible_amount: 2000, earned_amount: 8, }, UnclaimedRewardInfo { - reward_era: 5u32, + reward_era: 4u32, expires_at_block: 170, staked_amount: 3000, eligible_amount: 3_000, earned_amount: 11, }, - ]; - for i in 0..5 { + ] + .to_vec(); + for i in 0..expected_info.len() { assert_eq!(rewards.get(i).unwrap(), &expected_info[i]); } - run_to_block(131); + run_to_block(141); // current era = 14 let rewards = Capacity::list_unclaimed_rewards(&account).unwrap(); let max_history: u32 = ::ProviderBoostHistoryLimit::get(); - // the earliest era should no longer be stored. + // the earliest eras, 0 and 1, should no longer be stored. assert_eq!(rewards.len(), max_history as usize); assert_eq!(rewards.get(0).unwrap().reward_era, 2u32); - // there was no change in stake, so the eligible and earned amounts should be the same as in - // reward era 5. + // there was no change in stake, so the eligible and earned amounts + // for era 13 should be the same as in reward era 5. assert_eq!( rewards.get((max_history - 1) as usize).unwrap(), &UnclaimedRewardInfo { reward_era: 13u32, - expires_at_block: 250, + expires_at_block: 260, staked_amount: 3_000, eligible_amount: 3_000, earned_amount: 11, @@ -183,7 +184,8 @@ fn list_unclaimed_rewards_has_eligible_rewards() { }) } -// check that if an account boosted and then let it run for more than the number +// "Set and forget" test. +// Check that if an account boosted and then let it run for more than the number // of history retention eras, eligible rewards are correct. #[test] fn list_unclaimed_rewards_returns_correctly_for_old_single_boost() { @@ -198,31 +200,82 @@ fn list_unclaimed_rewards_returns_correctly_for_old_single_boost() { setup_provider(&account, &target, &amount, ProviderBoost); assert!(!Capacity::has_unclaimed_rewards(&account)); - run_to_block(131); - assert_eq!(CurrentEraInfo::::get().era_index, 14u32); + run_to_block(131); // era 13 + assert_eq!(CurrentEraInfo::::get().era_index, 13u32); assert_eq!(CurrentEraInfo::::get().started_at, 131u32); + let boost_history = ProviderBoostHistories::::get(account).unwrap(); + assert!(boost_history.get_entry_for_era(&0u32).is_some()); + let rewards = Capacity::list_unclaimed_rewards(&account).unwrap(); let max_history: u32 = ::ProviderBoostHistoryLimit::get(); assert_eq!(rewards.len(), max_history as usize); - // the earliest era should no longer be stored. - for i in 0u32..max_history { - let era = i + 2u32; + // the earliest era should not be returned. + assert_eq!(rewards.get(0).unwrap().reward_era, 1u32); + + for era in 1u32..=max_history { + let expires_at_era = era.saturating_add(max_history.into()); + let expires_at_block = Capacity::block_at_end_of_era(expires_at_era); let expected_info: UnclaimedRewardInfo, BlockNumberFor> = UnclaimedRewardInfo { - reward_era: era.into(), - expires_at_block: (era * 10u32 + 120u32).into(), + reward_era: era, + expires_at_block, staked_amount: 1000, eligible_amount: 1000, earned_amount: 4, }; - assert_eq!(rewards.get(i as usize).unwrap(), &expected_info); + let era_index: usize = (era - 1u32) as usize; + assert_eq!(rewards.get(era_index).unwrap(), &expected_info); } }) } +// this is to check that our 0-indexed era + when a Current Era starts at something besides one, +// that the calculations are still correct +#[test] +fn list_unclaimed_rewards_current_era_starts_at_later_block_works() { + new_test_ext().execute_with(|| { + let account = 10_000u64; + let target: MessageSourceId = 10; + let amount = 1000u64; + + System::set_block_number(9900); + set_era_and_reward_pool(0, 9898, 10_000); + setup_provider(&account, &target, &amount, ProviderBoost); + + run_to_block(9910); // middle of era 1 + assert_eq!(CurrentEraInfo::::get().era_index, 1u32); + assert_eq!(CurrentEraInfo::::get().started_at, 9908u32); + + run_to_block(9920); // middle of era 2, now some rewards can be claimed + assert_eq!(CurrentEraInfo::::get().era_index, 2u32); + assert_eq!(CurrentEraInfo::::get().started_at, 9918u32); + + let expected_info_era_0: UnclaimedRewardInfo, BlockNumberFor> = + UnclaimedRewardInfo { + reward_era: 0, + expires_at_block: 9898u32 + 129u32, + staked_amount: 1000, + eligible_amount: 0, + earned_amount: 0, + }; + let expected_info_era_1: UnclaimedRewardInfo, BlockNumberFor> = + UnclaimedRewardInfo { + reward_era: 1, + expires_at_block: 9908u32 + 129u32, + staked_amount: 1000, + eligible_amount: 1000, + earned_amount: 4, + }; + + let rewards = Capacity::list_unclaimed_rewards(&account).unwrap(); + assert_eq!(rewards.get(0).unwrap(), &expected_info_era_0); + assert_eq!(rewards.get(1).unwrap(), &expected_info_era_1); + }) +} + #[test] fn has_unclaimed_rewards_works() { new_test_ext().execute_with(|| { diff --git a/pallets/capacity/src/tests/testing_utils.rs b/pallets/capacity/src/tests/testing_utils.rs index a76e97d31d..f751006032 100644 --- a/pallets/capacity/src/tests/testing_utils.rs +++ b/pallets/capacity/src/tests/testing_utils.rs @@ -14,7 +14,15 @@ pub fn capacity_events() -> Vec> { let result = System::events() .into_iter() .map(|r| r.event) - .filter_map(|e| if let RuntimeEvent::Capacity(inner) = e { Some(inner) } else { None }) + .filter_map(|e| { + if let RuntimeEvent::Capacity(inner) = e { + log::warn!("inner: {:?}", inner); + Some(inner) + } else { + log::warn!("nothing"); + None + } + }) .collect::>(); System::reset_events(); diff --git a/pallets/capacity/src/tests/unstaking_tests.rs b/pallets/capacity/src/tests/unstaking_tests.rs index fde771a136..1f1852cd32 100644 --- a/pallets/capacity/src/tests/unstaking_tests.rs +++ b/pallets/capacity/src/tests/unstaking_tests.rs @@ -366,17 +366,27 @@ fn unstake_by_a_booster_updates_provider_boost_history_with_correct_amount() { register_provider(target1, String::from("Test Target")); assert_ok!(Capacity::provider_boost(RuntimeOrigin::signed(staker), target1, 1_000)); - let pbh = ProviderBoostHistories::::get(staker).unwrap(); + let mut pbh = ProviderBoostHistories::::get(staker).unwrap(); assert_eq!(pbh.count(), 1); // If unstaking in the next era, this should add a new staking history entry. - system_run_to_block(9); - run_to_block(51); + system_run_to_block(10); // last block of era 0 + run_to_block(41); // beginning of era 4 assert_ok!(Capacity::claim_staking_rewards(RuntimeOrigin::signed(staker))); + pbh = ProviderBoostHistories::::get(staker).unwrap(); + assert_eq!(pbh.count(), 1); + + // This adds a new history item for the unstake, in current era, 4 assert_ok!(Capacity::unstake(RuntimeOrigin::signed(staker), target1, 400u64)); - assert_eq!(get_balance::(&staker), 10_016u64); - assert_transferable::(&staker, 16u64); + // earned 4 in rewards for eras 3,2,1 + assert_eq!(get_balance::(&staker), 10_012u64); + assert_transferable::(&staker, 12u64); + + pbh = ProviderBoostHistories::::get(staker).unwrap(); + assert_eq!(pbh.count(), 2); + let entry = pbh.get_entry_for_era(&4u32.into()).unwrap(); + assert_eq!(entry, &600u64); }) } @@ -387,23 +397,25 @@ fn unstake_all_by_booster_reaps_boost_history() { let target1 = 1; register_provider(target1, String::from("Test Target")); + // Era 0, block 1. assert_ok!(Capacity::provider_boost(RuntimeOrigin::signed(staker), target1, 1_000)); let pbh = ProviderBoostHistories::::get(staker).unwrap(); assert_eq!(pbh.count(), 1); // If unstaking in the next era, this should add a new staking history entry. - system_run_to_block(9); - run_to_block(51); + system_run_to_block(10); // last block of era 0 + run_to_block(41); // First block of Era 4. assert_ok!(Capacity::claim_staking_rewards(RuntimeOrigin::signed(staker))); assert_ok!(Capacity::unstake(RuntimeOrigin::signed(staker), target1, 1_000)); assert!(ProviderBoostHistories::::get(staker).is_none()); - assert_eq!(get_balance::(&staker), 10_016u64); - assert_transferable::(&staker, 16u64); + // earn 4 each for 3 past eras, 3,2,1 + assert_eq!(get_balance::(&staker), 10_012u64); + assert_transferable::(&staker, 12u64); }) } #[test] -fn unstake_maximum_does_not_change_provider_boost_history() { +fn unstake_maximum_immediately_after_staking_does_not_create_provider_boost_history() { new_test_ext().execute_with(|| { let staker = 10_000; let target1 = 1; @@ -421,6 +433,15 @@ fn unstake_maximum_does_not_change_provider_boost_history() { fn get_amount_staked_for_era_works() { let mut staking_history: ProviderBoostHistory = ProviderBoostHistory::new(); + for i in 0u32..5u32 { + staking_history.add_era_balance(&i.into(), &10u64); + } + assert_eq!(staking_history.get_amount_staked_for_era(&0u32), 10u64); + assert_eq!(staking_history.get_amount_staked_for_era(&4u32), 50u64); + + staking_history.subtract_era_balance(&4u32.into(), &50u64); + assert_eq!(staking_history.get_amount_staked_for_era(&5u32), 0u64); + for i in 10u32..=13u32 { staking_history.add_era_balance(&i.into(), &5u64); } @@ -479,19 +500,3 @@ fn unstake_fails_if_provider_boosted_and_have_unclaimed_rewards() { ); }) } - -#[test] -fn unstake_all_if_no_unclaimed_rewards_removes_provider_boost_history() { - new_test_ext().execute_with(|| { - let account = 10_000u64; - let target: MessageSourceId = 10; - let amount = 1_000u64; - - // staking 1k as of block 1, era 1 - setup_provider(&account, &target, &amount, ProviderBoost); - assert!(ProviderBoostHistories::::get(account).is_some()); - run_to_block(10); - assert_ok!(Capacity::unstake(RuntimeOrigin::signed(account), target, amount)); - assert!(ProviderBoostHistories::::get(account).is_none()); - }); -} From c2bef1959006604119da5d8187ff3ec5aadf0e7a Mon Sep 17 00:00:00 2001 From: Aramik Date: Tue, 6 Aug 2024 13:58:22 -0700 Subject: [PATCH 3/4] adding more checks --- Cargo.lock | 1 + Cargo.toml | 1 + pallets/capacity/src/lib.rs | 2 +- runtime/frequency/Cargo.toml | 1 + runtime/frequency/src/lib.rs | 15 ++++++++++++--- 5 files changed, 16 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 68fe8682d0..17c2a8481f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3736,6 +3736,7 @@ dependencies = [ "sp-transaction-pool", "sp-version", "staging-parachain-info", + "static_assertions", "substrate-wasm-builder", "system-runtime-api", ] diff --git a/Cargo.toml b/Cargo.toml index 8e53fe308f..54c538935a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -118,6 +118,7 @@ serde_json = { version = "1.0.86", default-features = false } tokio = { version = "1.25.0", default-features = false } unicode-normalization = { version = "0.1.22", default-features = false } clap = { version = "4.2.5", features = ["derive"] } +static_assertions = { version = "1.1.0", default-features = false } frame-benchmarking-cli = { git = "https://github.com/paritytech/polkadot-sdk", branch = "release-polkadot-v1.10.0" } pallet-transaction-payment-rpc = { git = "https://github.com/paritytech/polkadot-sdk", branch = "release-polkadot-v1.10.0" } diff --git a/pallets/capacity/src/lib.rs b/pallets/capacity/src/lib.rs index 95bfb437f4..c317156c39 100644 --- a/pallets/capacity/src/lib.rs +++ b/pallets/capacity/src/lib.rs @@ -185,7 +185,7 @@ pub mod pallet { type RewardPercentCap: Get; /// The number of chunks of Reward Pool history we expect to store - /// MUST be a divisor of [`Self::ProviderBoostHistoryLimit`] + /// Is a divisor of [`Self::ProviderBoostHistoryLimit`] #[pallet::constant] type RewardPoolChunkLength: Get; } diff --git a/runtime/frequency/Cargo.toml b/runtime/frequency/Cargo.toml index 4fc8cf3258..6182af9a35 100644 --- a/runtime/frequency/Cargo.toml +++ b/runtime/frequency/Cargo.toml @@ -56,6 +56,7 @@ sp-core = { workspace = true } sp-inherents = { workspace = true } sp-io = { workspace = true } sp-genesis-builder = { workspace = true } +static_assertions = { workspace = true } sp-offchain = { workspace = true } sp-runtime = { workspace = true } diff --git a/runtime/frequency/src/lib.rs b/runtime/frequency/src/lib.rs index 449bfcec2c..6a53ff81b1 100644 --- a/runtime/frequency/src/lib.rs +++ b/runtime/frequency/src/lib.rs @@ -32,11 +32,11 @@ use pallet_collective::Members; use pallet_collective::ProposalCount; use parity_scale_codec::Encode; - use sp_std::prelude::*; #[cfg(feature = "std")] use sp_version::NativeVersion; use sp_version::RuntimeVersion; +use static_assertions::const_assert; use common_primitives::node::{ AccountId, Address, Balance, BlockNumber, Hash, Header, Index, ProposalProvider, Signature, @@ -519,6 +519,15 @@ impl pallet_msa::Config for Runtime { >; } +parameter_types! { + /// The maximum number of eras over which one can claim rewards + pub const ProviderBoostHistoryLimit : u32 = 30; + /// The number of chunks of Reward Pool history we expect to store + pub const RewardPoolChunkLength: u32 = 5; +} +// RewardPoolChunkLength MUST be a divisor of ProviderBoostHistoryLimit +const_assert!(ProviderBoostHistoryLimit::get() % RewardPoolChunkLength::get() == 0); + impl pallet_capacity::Config for Runtime { type RuntimeEvent = RuntimeEvent; type WeightInfo = pallet_capacity::weights::SubstrateWeight; @@ -535,14 +544,14 @@ impl pallet_capacity::Config for Runtime { type CapacityPerToken = CapacityPerToken; type RuntimeFreezeReason = RuntimeFreezeReason; type EraLength = CapacityRewardEraLength; - type ProviderBoostHistoryLimit = ConstU32<30u32>; + type ProviderBoostHistoryLimit = ProviderBoostHistoryLimit; type RewardsProvider = Capacity; type MaxRetargetsPerRewardEra = ConstU32<2>; // Value determined by desired inflation rate limits for chosen economic model type RewardPoolPerEra = ConstU128<{ currency::CENTS.saturating_mul(172_602_740u128) }>; type RewardPercentCap = CapacityRewardCap; // Must evenly divide ProviderBoostHistoryLimit - type RewardPoolChunkLength = ConstU32<5>; + type RewardPoolChunkLength = RewardPoolChunkLength; } impl pallet_schemas::Config for Runtime { From abe50669542bf19df982ca423601f337c0b5144e Mon Sep 17 00:00:00 2001 From: shannonwells Date: Tue, 6 Aug 2024 14:16:08 -0700 Subject: [PATCH 4/4] * New alias for ChunkIndex = u32 * Use unwrap_or_default for getting a reward pool chunk * warning for inserting a new chunk failure * code golf * linting in e2e * remove unnecessary change in helpers --- e2e/capacity/list_unclaimed_rewards.test.ts | 6 +++--- e2e/capacity/provider_boost.test.ts | 9 ++++++--- e2e/scaffolding/helpers.ts | 5 ++--- pallets/capacity/src/lib.rs | 11 ++++++----- 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/e2e/capacity/list_unclaimed_rewards.test.ts b/e2e/capacity/list_unclaimed_rewards.test.ts index 0c296cd0e2..9f77eb055c 100644 --- a/e2e/capacity/list_unclaimed_rewards.test.ts +++ b/e2e/capacity/list_unclaimed_rewards.test.ts @@ -34,9 +34,9 @@ describe('Capacity: list_unclaimed_rewards', function () { }); it('returns correct rewards after enough eras have passed', async function () { - if (isTestnet()) { - this.skip(); - } + // this will be too long if run against testnet + if (isTestnet()) this.skip(); + const [_provider, booster] = await setUpForBoosting('booster2', 'provider2'); console.debug(`Booster pubkey: ${booster.address}`); diff --git a/e2e/capacity/provider_boost.test.ts b/e2e/capacity/provider_boost.test.ts index 546e8ceccf..f64c93e034 100644 --- a/e2e/capacity/provider_boost.test.ts +++ b/e2e/capacity/provider_boost.test.ts @@ -7,7 +7,8 @@ import { CENTS, DOLLARS, createAndFundKeypair, - boostProvider, stakeToProvider, + boostProvider, + stakeToProvider, } from '../scaffolding/helpers'; const fundingSource = getFundingSource('capacity-provider-boost'); @@ -27,14 +28,16 @@ describe('Capacity: provider_boost extrinsic', function () { const stakeKeys = createKeys('booster'); const provider = await createMsaAndProvider(fundingSource, stakeKeys, 'Provider1', providerBalance); await assert.doesNotReject(stakeToProvider(fundingSource, stakeKeys, provider, tokenMinStake)); - await assert.rejects(boostProvider(fundingSource, stakeKeys, provider, tokenMinStake), {name: "CannotChangeStakingType"}); + await assert.rejects(boostProvider(fundingSource, stakeKeys, provider, tokenMinStake), { + name: 'CannotChangeStakingType', + }); }); it("fails when staker doesn't have enough token", async function () { const stakeKeys = createKeys('booster'); const provider = await createMsaAndProvider(fundingSource, stakeKeys, 'Provider1', providerBalance); const booster = await createAndFundKeypair(fundingSource, 1n * DOLLARS, 'booster'); - await assert.rejects(boostProvider(booster, booster, provider, 1n * DOLLARS), {name: "BalanceTooLowtoStake"}); + await assert.rejects(boostProvider(booster, booster, provider, 1n * DOLLARS), { name: 'BalanceTooLowtoStake' }); }); it('staker can boost multiple times', async function () { diff --git a/e2e/scaffolding/helpers.ts b/e2e/scaffolding/helpers.ts index 48fc952151..d06c2a27db 100644 --- a/e2e/scaffolding/helpers.ts +++ b/e2e/scaffolding/helpers.ts @@ -497,15 +497,14 @@ export async function getOrCreateGraphChangeSchema(source: KeyringPair): Promise if (existingSchemaId) { return new u16(ExtrinsicHelper.api.registry, existingSchemaId); } else { - const op = ExtrinsicHelper.createSchemaV3( + const { target: createSchemaEvent, eventMap } = await ExtrinsicHelper.createSchemaV3( source, AVRO_GRAPH_CHANGE, 'AvroBinary', 'OnChain', [], 'test.graphChangeSchema' - ); - const { target: createSchemaEvent, eventMap } = await op.fundAndSend(source); + ).fundAndSend(source); assertExtrinsicSuccess(eventMap); if (createSchemaEvent) { return createSchemaEvent.data.schemaId; diff --git a/pallets/capacity/src/lib.rs b/pallets/capacity/src/lib.rs index c317156c39..f22b339d2e 100644 --- a/pallets/capacity/src/lib.rs +++ b/pallets/capacity/src/lib.rs @@ -72,6 +72,7 @@ pub mod migration; pub mod weights; type BalanceOf = <::Currency as InspectFungible<::AccountId>>::Balance; +type ChunkIndex = u32; #[frame_support::pallet] pub mod pallet { @@ -257,7 +258,7 @@ pub mod pallet { /// chunk number. #[pallet::storage] pub type ProviderBoostRewardPools = - StorageMap<_, Twox64Concat, u32, RewardPoolHistoryChunk>; + StorageMap<_, Twox64Concat, ChunkIndex, RewardPoolHistoryChunk>; /// How much is staked this era #[pallet::storage] @@ -1146,7 +1147,7 @@ impl Pallet { Error::::EraOutOfRange ); - let chunk_idx: u32 = Self::get_chunk_index_for_era(reward_era); + let chunk_idx: ChunkIndex = Self::get_chunk_index_for_era(reward_era); let reward_pool_chunk = ProviderBoostRewardPools::::get(chunk_idx).unwrap_or_default(); // 1r let total_for_era = reward_pool_chunk.total_for_era(&reward_era).ok_or(Error::::EraOutOfRange)?; @@ -1177,9 +1178,8 @@ impl Pallet { // This is where the reward pool gets updated. pub(crate) fn update_provider_boost_reward_pool(era: RewardEra, boost_total: BalanceOf) { // Current era is this era - let chunk_idx: u32 = Self::get_chunk_index_for_era(era); - let mut new_chunk = - ProviderBoostRewardPools::::get(chunk_idx).unwrap_or(RewardPoolHistoryChunk::new()); // 1r + let chunk_idx: ChunkIndex = Self::get_chunk_index_for_era(era); + let mut new_chunk = ProviderBoostRewardPools::::get(chunk_idx).unwrap_or_default(); // 1r // If it is full we are resetting. // This assumes that the chunk length is a divisor of the history limit @@ -1189,6 +1189,7 @@ impl Pallet { if new_chunk.try_insert(era, boost_total).is_err() { // Handle the error case that should never happen + log::warn!("could not insert a new chunk into provider boost reward pool") } ProviderBoostRewardPools::::set(chunk_idx, Some(new_chunk)); // 1w }