From 2984b23ce58465eb94d7765a65659a2ce0573410 Mon Sep 17 00:00:00 2001 From: Dino Pacandi Date: Mon, 25 Sep 2023 12:49:46 +0200 Subject: [PATCH] Updates to logic, more comments, more tests --- pallets/dynamic-evm-base-fee/src/lib.rs | 39 ++++++--- pallets/dynamic-evm-base-fee/src/mock.rs | 1 - pallets/dynamic-evm-base-fee/src/tests.rs | 98 ++++++++++++++++++++++- 3 files changed, 126 insertions(+), 12 deletions(-) diff --git a/pallets/dynamic-evm-base-fee/src/lib.rs b/pallets/dynamic-evm-base-fee/src/lib.rs index d4f475a42b..494ce047e1 100644 --- a/pallets/dynamic-evm-base-fee/src/lib.rs +++ b/pallets/dynamic-evm-base-fee/src/lib.rs @@ -18,6 +18,18 @@ //! TODO: Rustdoc!!! +// TODO: remove this comment later +// Max amount that adjustment factor will be able to change on live networks using the new tokenomics will be: +// +// c_n = c_n-1 * (1 + adjustment + adjustment^2/2) +// +// adjustment = v * (s - s*) +// +// Biggest possible adjustment between 2 blocks is: 0.000015 * (1 - 0.25) = 0.000_011_25 +// Expressed as ratio: 11_250 / 1_000_000_000. +// This is a much smaller change compared to the max step limit ratio we'll use to limit bfpg adaptation. +// This means that once equilibrium is reached, the `StepLimitRatio` will be larger than the max possible adjustment, essentially eliminating it's effect. + #![cfg_attr(not(feature = "std"), no_std)] use frame_support::{traits::Get, weights::Weight}; @@ -90,8 +102,6 @@ pub mod pallet { db_weight.reads_writes(2, 1) } - // TODO: it's super important to do double-check possible loss of precision here. - // Do some tests, compare to benchmark values. fn on_finalize(_n: ::BlockNumber) { BaseFeePerGas::::mutate(|base_fee_per_gas| { let old_bfpg = *base_fee_per_gas; @@ -103,13 +113,24 @@ pub mod pallet { U256::from(step) }; - // TODO: maybe add a DB entry to check until when should we apply max step adjustment? - // Once 'equilibrium' is reached, it's safe to just follow the formula without limit updates. - // Or we could abuse the sudo for this. - - // Lower & upper limit between which the new base fee per gas should be clamped. - let lower_limit = T::MinBaseFeePerGas::get().max(old_bfpg.saturating_sub(max_step)); - let upper_limit = T::MaxBaseFeePerGas::get().min(old_bfpg.saturating_add(max_step)); + // It's possible current base fee per gas is outside of the allowed range. + // This can & will happen when this solution is deployed on live networks. + // + // In such scenario, we will discard the lower & upper bounds configured in the runtime. + // Once these bounds are reached ONCE, the runtime logic will prevent them from going out of bounds again. + let apply_configured_bounds = old_bfpg >= T::MinBaseFeePerGas::get() + && old_bfpg <= T::MaxBaseFeePerGas::get(); + let (lower_limit, upper_limit) = if apply_configured_bounds { + ( + T::MinBaseFeePerGas::get().max(old_bfpg.saturating_sub(max_step)), + T::MaxBaseFeePerGas::get().min(old_bfpg.saturating_add(max_step)), + ) + } else { + ( + old_bfpg.saturating_sub(max_step), + old_bfpg.saturating_add(max_step), + ) + }; // Calculate ideal new 'base_fee_per_gas' according to the formula let ideal_new_bfpg = T::AdjustmentFactor::get() diff --git a/pallets/dynamic-evm-base-fee/src/mock.rs b/pallets/dynamic-evm-base-fee/src/mock.rs index 7a6a60f854..452f87c2ff 100644 --- a/pallets/dynamic-evm-base-fee/src/mock.rs +++ b/pallets/dynamic-evm-base-fee/src/mock.rs @@ -95,7 +95,6 @@ impl pallet_timestamp::Config for TestRuntime { type WeightInfo = (); } -// TODO: maybe expand this so all params are configurable? Should make for more dynamic testing. parameter_types! { pub DefaultBaseFeePerGas: U256 = U256::from(1_500_000_000_000_u128); pub MinBaseFeePerGas: U256 = U256::from(800_000_000_000_u128); diff --git a/pallets/dynamic-evm-base-fee/src/tests.rs b/pallets/dynamic-evm-base-fee/src/tests.rs index 76dc929235..c26b0096ae 100644 --- a/pallets/dynamic-evm-base-fee/src/tests.rs +++ b/pallets/dynamic-evm-base-fee/src/tests.rs @@ -23,7 +23,10 @@ use mock::*; use frame_support::{assert_noop, assert_ok, traits::OnFinalize}; use num_traits::Bounded; -use sp_runtime::traits::{BadOrigin, One, Zero}; +use sp_runtime::{ + traits::{BadOrigin, One, Zero}, + FixedU128, +}; use fp_evm::FeeCalculator; @@ -224,6 +227,97 @@ fn bfpg_full_spectrum_change_works() { DynamicEvmBaseFee::on_finalize(counter); counter += 1; } - assert_eq!(BaseFeePerGas::::get(), target_bfpg); + + assert_eq!(BaseFeePerGas::::get(), target_bfpg, + "bfpg upper bound not reached - either it's not enough iterations or some precision loss occurs."); + }); +} + +#[test] +fn bfpg_matches_expected_value_for_so_called_average_transaction() { + ExtBuilder::build().execute_with(|| { + // The new proposed models suggests to use the following formula to calculate the base fee per gas: + // + // bfpg = (adj_factor * weight_factor * 25_000) / 9_897_4000 + let init_bfpg = get_ideal_bfpg(); + BaseFeePerGas::::set(init_bfpg); + let init_adj_factor = ::AdjustmentFactor::get(); + + // Slighly increase the adjustment factor, and calculate the new base fee per gas + // + // To keep it closer to reality, let's assume we're using the proposed variability factor of 0.000_015. + // Let's also assume that block fullness difference is 0.01 (1%). + // This should result in the adjustment factor of 0.000_001_5. + // + // NOTE: it's important to keep the increase small so that the step doesn't saturate + let change = FixedU128::from_rational(1500, 1_000_000_000); + let new_adj_factor = init_adj_factor + change; + assert!(new_adj_factor > init_adj_factor, "Sanity check"); + set_adjustment_factor(new_adj_factor); + + // Calculate the new expected base fee per gas + let weight_factor: u128 = ::WeightFactor::get(); + let expected_bfpg = + U256::from(new_adj_factor.saturating_mul_int(weight_factor) * 25_000 / 9_897_4000); + + // Calculate the new base fee per gas in the pallet + DynamicEvmBaseFee::on_finalize(1); + + // Assert calculated value is as expected + let new_bfpg = BaseFeePerGas::::get(); + assert!(new_bfpg > init_bfpg, "Sanity check"); + assert_eq!(new_bfpg, expected_bfpg); + + // Also check the opposite direction + let new_adj_factor = init_adj_factor - change; + set_adjustment_factor(new_adj_factor); + let expected_bfpg = + U256::from(new_adj_factor.saturating_mul_int(weight_factor) * 25_000 / 9_897_4000); + + // Calculate the new base fee per gas in the pallet + DynamicEvmBaseFee::on_finalize(2); + // Assert calculated value is as expected + let new_bfpg = BaseFeePerGas::::get(); + assert!(new_bfpg < init_bfpg, "Sanity check"); + assert_eq!(new_bfpg, expected_bfpg); + }); +} + +#[test] +fn lower_upper_bounds_ignored_if_bfpg_is_outside() { + ExtBuilder::build().execute_with(|| { + // Set the initial bfpg to be outside of the allowed range. + // It's important reduction is sufficient so we're still below the minimum limit after the adjustment. + let delta = 100_000_000; + + // First test when bfpg is too little + let too_small_bfpg = ::MinBaseFeePerGas::get() - delta; + BaseFeePerGas::::set(too_small_bfpg); + DynamicEvmBaseFee::on_finalize(1); + + assert!( + BaseFeePerGas::::get() > too_small_bfpg, + "Bfpg should have increased slightly." + ); + assert!( + BaseFeePerGas::::get() + < ::MinBaseFeePerGas::get(), + "For this test, bfpg should still be below the minimum limit." + ); + + // Repeat the same test but this time bfpg is too big + let too_big_bfpg = ::MaxBaseFeePerGas::get() + delta; + BaseFeePerGas::::set(too_big_bfpg); + DynamicEvmBaseFee::on_finalize(2); + + assert!( + BaseFeePerGas::::get() < too_big_bfpg, + "Bfpg should have decreased slightly." + ); + assert!( + BaseFeePerGas::::get() + < ::MaxBaseFeePerGas::get(), + "For this test, bfpg should still be above the maximum limit." + ); }); }