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.

Sign up for GitHub

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

feat: relative treasury inflation #1740

Merged
merged 10 commits into from
Feb 23, 2024
Merged

feat: relative treasury inflation #1740

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
02b7fe6
feat: relative treasury inflation
wischli Feb 20, 2024
195b14a
refactor: apply benchmarking v2 to block rewards
wischli Feb 20, 2024
420b1d1
feat: add migration
wischli Feb 21, 2024
45a5b09
fix: build + clippy
wischli Feb 21, 2024
95dbeae
Update pallets/block-rewards/src/mock.rs
wischli Feb 22, 2024
7d95ced
Merge remote-tracking branch 'origin/main' into feat/relative-treasur…
wischli Feb 22, 2024
76cea1f
chore: minor fixes
wischli Feb 22, 2024
24bc318
refactor: remove max changes per session
wischli Feb 22, 2024
63d37d1
refactor: remove more redundant trait bounds
wischli Feb 23, 2024
add78d8
Merge remote-tracking branch 'origin/main' into feat/relative-treasur…
wischli Feb 23, 2024
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
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 20 additions & 4 deletions node/src/chain_spec.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,10 @@ use sc_chain_spec::{ChainSpecExtension, ChainSpecGroup};
use sc_service::{ChainType, Properties};
use serde::{Deserialize, Serialize};
use sp_core::{crypto::UncheckedInto, sr25519, Encode, Pair, Public, H160};
use sp_runtime::traits::{IdentifyAccount, Verify};
use sp_runtime::{
traits::{IdentifyAccount, Verify},
FixedPointNumber,
};
use xcm::{
latest::MultiLocation,
prelude::{GeneralIndex, GeneralKey, PalletInstance, Parachain, X2, X3},
Expand All @@ -61,6 +64,7 @@ pub type DevelopmentChainSpec =

use altair_runtime::evm::AltairPrecompiles;
use centrifuge_runtime::evm::CentrifugePrecompiles;
use cfg_types::fixed_point::Rate;
use development_runtime::evm::DevelopmentPrecompiles;

/// Helper function to generate a crypto pair from seed
Expand Down Expand Up @@ -661,7 +665,11 @@ fn centrifuge_genesis(
.map(|(acc, _)| acc)
.collect(),
collator_reward: 8_325 * MILLI_CFG,
total_reward: 10_048 * CFG,
treasury_inflation_rate: Rate::saturating_from_rational(3, 100),
last_update: std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.expect("SystemTime before UNIX EPOCH!")
.as_secs(),
Comment on lines +669 to +672
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would mean that each node using this genesis will have a different configuration (different time machines). Is it ok or should they have the exact initial configuration?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, thats the implication. However, this only affects new networks, i.e. either Development if it needs to be reset or otherwise local ones. Therefore, it shouldn't have an impact. If we make it static, the first treasury inflation reward will huge.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But you're using the system clock, not the clock used on-chain (that changed each block by 12 secs). I guess we're okay with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point but it should be fine here. It only affects the first treasury reward and should never affect live chains/production.

},
block_rewards_base: Default::default(),
base_fee: Default::default(),
Expand Down Expand Up @@ -756,7 +764,11 @@ fn altair_genesis(
.map(|(acc, _)| acc)
.collect(),
collator_reward: 98_630 * MILLI_AIR,
total_reward: 98_630 * MILLI_AIR * 100,
treasury_inflation_rate: Rate::saturating_from_rational(3, 100),
last_update: std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.expect("SystemTime before UNIX EPOCH!")
.as_secs(),
},
block_rewards_base: Default::default(),
collator_allowlist: Default::default(),
Expand Down Expand Up @@ -939,7 +951,11 @@ fn development_genesis(
.map(|(acc, _)| acc)
.collect(),
collator_reward: 8_325 * MILLI_CFG,
total_reward: 10_048 * CFG,
treasury_inflation_rate: Rate::saturating_from_rational(3, 100),
last_update: std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.expect("SystemTime before UNIX EPOCH!")
.as_secs(),
},
base_fee: Default::default(),
evm_chain_id: development_runtime::EVMChainIdConfig {
Expand Down
1 change: 1 addition & 0 deletions pallets/block-rewards/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ sp-std = { workspace = true }
frame-benchmarking = { workspace = true, optional = true }

[dev-dependencies]
cfg-mocks = { workspace = true, default-features = true }
orml-tokens = { workspace = true, default-features = true }
orml-traits = { workspace = true, default-features = true }
pallet-balances = { workspace = true, default-features = true }
Expand Down
105 changes: 67 additions & 38 deletions pallets/block-rewards/src/benchmarking.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use cfg_primitives::CFG;
use cfg_types::tokens::CurrencyId;
use frame_benchmarking::{account, benchmarks, impl_benchmark_test_suite, whitelisted_caller};
use frame_benchmarking::v2::*;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating the benchmarking version!

use frame_support::{
assert_ok,
traits::{fungibles::Inspect, Currency as CurrencyT},
Expand All @@ -14,56 +14,85 @@ use crate::{pallet::Config, Pallet as BlockRewards};
const REWARD: u128 = 1 * CFG;
const SEED: u32 = 0;

benchmarks! {
where_clause {
where
#[benchmarks(
where
T::Balance: From<u128>,
T::BlockNumber: From<u32> + One,
T::Weight: From<u32>,
<T as Config>::Currency: frame_support::traits::fungibles::Inspect<T::AccountId> + CurrencyT<T::AccountId>,
<T as Config>::Tokens: Inspect<T::AccountId> + CurrencyT<T::AccountId>,
<T as Config>::CurrencyId: From<CurrencyId>,
}
)]
mod benchmarks {
use super::*;

claim_reward {
let caller = whitelisted_caller();
let beneficiary: T::AccountId = account("collator", 0, SEED);
#[benchmark]
fn claim_reward() -> Result<(), BenchmarkError> {
let caller: T::AccountId = account("caller", 0, SEED);
let beneficiary: T::AccountId = account("collator", 0, SEED);

assert_ok!(BlockRewards::<T>::do_init_collator(&beneficiary));
assert_ok!(T::Rewards::reward_group(T::StakeGroupId::get(), REWARD.into()));
assert_ok!(T::Rewards::reward_group(
T::StakeGroupId::get(),
REWARD.into()
));
assert!(T::Rewards::is_ready(T::StakeGroupId::get()));
assert!(
!T::Rewards::compute_reward(
T::StakeCurrencyId::get(),
&beneficiary,
).unwrap().is_zero()
!T::Rewards::compute_reward(T::StakeCurrencyId::get(), &beneficiary,)
.unwrap()
.is_zero()
);
let before =
<T::Tokens as Inspect<T::AccountId>>::balance(CurrencyId::Native.into(), &beneficiary);

#[extrinsic_call]
claim_reward(RawOrigin::Signed(caller), beneficiary.clone());

let num_collators: u128 = BlockRewards::<T>::next_session_changes()
.collator_count
.unwrap_or(BlockRewards::<T>::active_session_data().collator_count)
.into();
// Does not get entire reward since another collator is auto-staked via genesis
// config
assert_eq!(
<T::Tokens as Inspect<T::AccountId>>::balance(CurrencyId::Native.into(), &beneficiary)
.saturating_sub(before),
(REWARD / (num_collators + 1)).into()
);
let before = <T as Config>::Currency::balance(CurrencyId::Native.into(), &beneficiary);

}: _(RawOrigin::Signed(caller), beneficiary.clone())
verify {
let num_collators: u128 = BlockRewards::<T>::next_session_changes().collator_count.unwrap_or(
BlockRewards::<T>::active_session_data().collator_count
).into();
// Does not get entire reward since another collator is auto-staked via genesis config
assert_eq!(<T as Config>::Currency::balance(CurrencyId::Native.into(), &beneficiary).saturating_sub(before), (REWARD / (num_collators + 1)).into());

Ok(())
}

set_collator_reward {
assert_ok!(BlockRewards::<T>::set_total_reward(RawOrigin::Root.into(), u128::MAX.into()));
}: _(RawOrigin::Root, REWARD.into())
verify {
assert_eq!(BlockRewards::<T>::next_session_changes().collator_reward, Some(REWARD.into()));
#[benchmark]
fn set_collator_reward_per_session() -> Result<(), BenchmarkError> {
#[extrinsic_call]
set_collator_reward_per_session(RawOrigin::Root, REWARD.into());

assert_eq!(
BlockRewards::<T>::next_session_changes().collator_reward,
Some(REWARD.into())
);

Ok(())
}

set_total_reward {
}: _(RawOrigin::Root, u128::MAX.into())
verify {
assert_eq!(BlockRewards::<T>::next_session_changes().total_reward, Some(u128::MAX.into()));
#[benchmark]
fn set_annual_treasury_inflation_rate() -> Result<(), BenchmarkError> {
let rate = T::Rate::saturating_from_rational(1, 2);

#[extrinsic_call]
set_annual_treasury_inflation_rate(RawOrigin::Root, rate);

assert_eq!(
BlockRewards::<T>::next_session_changes().treasury_inflation_rate,
Some(rate)
);

Ok(())
}
}

impl_benchmark_test_suite!(
BlockRewards,
crate::mock::ExtBuilder::default().build(),
crate::mock::Test,
);
impl_benchmark_test_suite!(
BlockRewards,
crate::mock::ExtBuilder::default().build(),
crate::mock::Test,
);
}
Loading
Loading