From 7c0914453e7455313dae2197668776634077d65f Mon Sep 17 00:00:00 2001 From: PierreOssun Date: Fri, 3 Nov 2023 15:11:20 +0100 Subject: [PATCH] PR comments --- .../block-rewards-hybrid/src/benchmarking.rs | 27 +++-- pallets/block-rewards-hybrid/src/lib.rs | 4 +- pallets/block-rewards-hybrid/src/mock.rs | 16 ++- pallets/block-rewards-hybrid/src/tests.rs | 108 +++--------------- runtime/local/src/lib.rs | 4 +- runtime/shibuya/src/lib.rs | 4 +- 6 files changed, 51 insertions(+), 112 deletions(-) diff --git a/pallets/block-rewards-hybrid/src/benchmarking.rs b/pallets/block-rewards-hybrid/src/benchmarking.rs index 5f1a9c1a47..d24f80f049 100644 --- a/pallets/block-rewards-hybrid/src/benchmarking.rs +++ b/pallets/block-rewards-hybrid/src/benchmarking.rs @@ -20,7 +20,7 @@ use super::*; -use frame_benchmarking::{benchmarks, impl_benchmark_test_suite}; +use frame_benchmarking::v2::*; use frame_system::{Pallet as System, RawOrigin}; /// Assert that the last event equals the provided one. @@ -28,15 +28,26 @@ fn assert_last_event(generic_event: ::RuntimeEvent) { System::::assert_last_event(generic_event.into()); } -benchmarks! { +#[benchmarks(where T: Config)] +mod benchmarks { + use super::*; - set_configuration { + #[benchmark] + fn set_configuration() { let reward_config = RewardDistributionConfig::default(); assert!(reward_config.is_consistent()); - }: _(RawOrigin::Root, reward_config.clone()) - verify { + + #[extrinsic_call] + _(RawOrigin::Root, reward_config.clone()); + assert_last_event::(Event::::DistributionConfigurationChanged(reward_config).into()); } + + impl_benchmark_test_suite!( + Pallet, + crate::benchmarking::tests::new_test_ext(), + crate::mock::TestRuntime, + ); } #[cfg(test)] @@ -48,9 +59,3 @@ mod tests { mock::ExternalityBuilder::build() } } - -impl_benchmark_test_suite!( - Pallet, - crate::benchmarking::tests::new_test_ext(), - crate::mock::TestRuntime, -); diff --git a/pallets/block-rewards-hybrid/src/lib.rs b/pallets/block-rewards-hybrid/src/lib.rs index 89c205e4b7..336e740006 100644 --- a/pallets/block-rewards-hybrid/src/lib.rs +++ b/pallets/block-rewards-hybrid/src/lib.rs @@ -126,7 +126,7 @@ pub mod pallet { /// The amount of issuance for each block. #[pallet::constant] - type RewardAmount: Get; + type MaxBlockRewardAmount: Get; /// The overarching event type. type RuntimeEvent: From> + IsType<::RuntimeEvent>; @@ -202,7 +202,7 @@ pub mod pallet { impl OnTimestampSet for Pallet { fn on_timestamp_set(_moment: Moment) { - let rewards = Self::calculate_rewards(T::RewardAmount::get()); + let rewards = Self::calculate_rewards(T::MaxBlockRewardAmount::get()); let inflation = T::Currency::issue(rewards.sum()); Self::distribute_rewards(inflation, rewards); } diff --git a/pallets/block-rewards-hybrid/src/mock.rs b/pallets/block-rewards-hybrid/src/mock.rs index f185e55c56..80e10e6eeb 100644 --- a/pallets/block-rewards-hybrid/src/mock.rs +++ b/pallets/block-rewards-hybrid/src/mock.rs @@ -32,6 +32,7 @@ use sp_runtime::{ testing::Header, traits::{AccountIdConversion, BlakeTwo256, IdentityLookup}, }; +use sp_std::cell::RefCell; pub(crate) type AccountId = u64; pub(crate) type BlockNumber = u64; @@ -126,23 +127,28 @@ impl pallet_timestamp::Config for TestRuntime { // due to TVL changes. pub(crate) const BLOCK_REWARD: Balance = 1_000_000; -// This gives us enough flexibility to get valid percentages by controlling issuance. -pub(crate) const TVL: Balance = 1_000_000_000; - // Fake accounts used to simulate reward beneficiaries balances pub(crate) const TREASURY_POT: PalletId = PalletId(*b"moktrsry"); pub(crate) const COLLATOR_POT: PalletId = PalletId(*b"mokcolat"); pub(crate) const STAKERS_POT: PalletId = PalletId(*b"mokstakr"); pub(crate) const DAPPS_POT: PalletId = PalletId(*b"mokdapps"); +thread_local! { + static TVL: RefCell = RefCell::new(1_000_000_000); +} + // Type used as TVL provider pub struct TvlProvider(); impl Get for TvlProvider { fn get() -> Balance { - TVL + TVL.with(|t| t.borrow().clone()) } } +pub(crate) fn set_tvl(v: Balance) { + TVL.with(|t| *t.borrow_mut() = v) +} + // Type used as beneficiary payout handle pub struct BeneficiaryPayout(); impl pallet_block_reward::BeneficiaryPayout> @@ -172,7 +178,7 @@ parameter_types! { impl pallet_block_reward::Config for TestRuntime { type RuntimeEvent = RuntimeEvent; type Currency = Balances; - type RewardAmount = RewardAmount; + type MaxBlockRewardAmount = RewardAmount; type DappsStakingTvlProvider = TvlProvider; type BeneficiaryPayout = BeneficiaryPayout; type WeightInfo = (); diff --git a/pallets/block-rewards-hybrid/src/tests.rs b/pallets/block-rewards-hybrid/src/tests.rs index 9170a1f2df..99b1e0a111 100644 --- a/pallets/block-rewards-hybrid/src/tests.rs +++ b/pallets/block-rewards-hybrid/src/tests.rs @@ -212,7 +212,7 @@ pub fn reward_distribution_as_expected() { )); // Initial adjustment of TVL - adjust_tvl_percentage(Perbill::from_percent(30)); + set_tvl(30); // Issue rewards a couple of times and verify distribution is as expected // also ensure that the non distributed reward amount is burn @@ -257,65 +257,22 @@ pub fn non_distributed_reward_amount_is_burned() { reward_config.clone() )); - // Initial adjustment of TVL - adjust_tvl_percentage(Perbill::from_percent(30)); + for tvl in [30, 50, 70, 100] { + // Initial adjustment of TVL + set_tvl(tvl); - for _block in 1..=100 { - let total_issuance_before = ::Currency::total_issuance(); - let distributed_rewards = Rewards::calculate(&reward_config); - let burned_amount = BLOCK_REWARD - distributed_rewards.sum(); + for _block in 1..=100 { + let total_issuance_before = ::Currency::total_issuance(); + let distributed_rewards = Rewards::calculate(&reward_config); + let burned_amount = BLOCK_REWARD - distributed_rewards.sum(); - BlockReward::on_timestamp_set(0); + BlockReward::on_timestamp_set(0); - assert_eq!( - ::Currency::total_issuance(), - total_issuance_before + BLOCK_REWARD - burned_amount - ); - } - - adjust_tvl_percentage(Perbill::from_percent(50)); - - for _block in 1..=100 { - let total_issuance_before = ::Currency::total_issuance(); - let distributed_rewards = Rewards::calculate(&reward_config); - let burned_amount = BLOCK_REWARD - distributed_rewards.sum(); - - BlockReward::on_timestamp_set(0); - - assert_eq!( - ::Currency::total_issuance(), - total_issuance_before + BLOCK_REWARD - burned_amount - ); - } - - adjust_tvl_percentage(Perbill::from_percent(70)); - - for _block in 1..=100 { - let total_issuance_before = ::Currency::total_issuance(); - let distributed_rewards = Rewards::calculate(&reward_config); - let burned_amount = BLOCK_REWARD - distributed_rewards.sum(); - - BlockReward::on_timestamp_set(0); - - assert_eq!( - ::Currency::total_issuance(), - total_issuance_before + BLOCK_REWARD - burned_amount - ); - } - - adjust_tvl_percentage(Perbill::from_percent(100)); - - for _block in 1..=100 { - let total_issuance_before = ::Currency::total_issuance(); - let distributed_rewards = Rewards::calculate(&reward_config); - let burned_amount = BLOCK_REWARD - distributed_rewards.sum(); - - BlockReward::on_timestamp_set(0); - - assert_eq!( - ::Currency::total_issuance(), - total_issuance_before + BLOCK_REWARD - burned_amount - ); + assert_eq!( + ::Currency::total_issuance(), + total_issuance_before + BLOCK_REWARD - burned_amount + ); + } } }) } @@ -493,38 +450,9 @@ impl Rewards { fn sum(&self) -> Balance { self.base_staker_reward - .saturating_add(self.adjustable_staker_reward) - .saturating_add(self.collators_reward) - .saturating_add(self.dapps_reward) - .saturating_add(self.treasury_reward) - } -} - -/// Adjusts total_issuance in order to try-and-match the requested TVL percentage -fn adjust_tvl_percentage(desired_tvl_percentage: Perbill) { - // Calculate the required total issuance - let tvl = ::DappsStakingTvlProvider::get(); - let required_total_issuance = desired_tvl_percentage.saturating_reciprocal_mul(tvl); - - // Calculate how much more we need to issue in order to get the desired TVL percentage - let init_total_issuance = ::Currency::total_issuance(); - - // issue to if issuance should be greater - // or burn if it should be lower - if required_total_issuance > init_total_issuance { - let to_issue = required_total_issuance.saturating_sub(init_total_issuance); - ::Currency::resolve_creating( - &1, - ::Currency::issue(to_issue), - ); - } else { - let to_burn = init_total_issuance.saturating_sub(required_total_issuance); - _ = ::Currency::slash(&1, to_burn); + + self.adjustable_staker_reward + + self.collators_reward + + self.dapps_reward + + self.treasury_reward } - - // Sanity check - assert_eq!( - ::Currency::total_issuance(), - required_total_issuance - ); } diff --git a/runtime/local/src/lib.rs b/runtime/local/src/lib.rs index d3d9b3107d..1c1fb8f087 100644 --- a/runtime/local/src/lib.rs +++ b/runtime/local/src/lib.rs @@ -438,14 +438,14 @@ impl block_rewards_hybrid::BeneficiaryPayout for BeneficiaryP } parameter_types! { - pub const RewardAmount: Balance = 2_664 * MILLIAST; + pub const MaxBlockRewardAmount: Balance = 2_664 * MILLIAST; } impl block_rewards_hybrid::Config for Runtime { type Currency = Balances; type DappsStakingTvlProvider = DappsStakingTvlProvider; type BeneficiaryPayout = BeneficiaryPayout; - type RewardAmount = RewardAmount; + type MaxBlockRewardAmount = MaxBlockRewardAmount; type RuntimeEvent = RuntimeEvent; type WeightInfo = block_rewards_hybrid::weights::SubstrateWeight; } diff --git a/runtime/shibuya/src/lib.rs b/runtime/shibuya/src/lib.rs index 8357979052..e5e2e1e44c 100644 --- a/runtime/shibuya/src/lib.rs +++ b/runtime/shibuya/src/lib.rs @@ -552,14 +552,14 @@ impl block_rewards_hybrid::BeneficiaryPayout for BeneficiaryP } parameter_types! { - pub const RewardAmount: Balance = 2_530 * MILLISBY; + pub const MaxBlockRewardAmount: Balance = 2_530 * MILLISBY; } impl block_rewards_hybrid::Config for Runtime { type Currency = Balances; type DappsStakingTvlProvider = DappsStakingTvlProvider; type BeneficiaryPayout = BeneficiaryPayout; - type RewardAmount = RewardAmount; + type MaxBlockRewardAmount = MaxBlockRewardAmount; type RuntimeEvent = RuntimeEvent; type WeightInfo = block_rewards_hybrid::weights::SubstrateWeight; }