From 6303ddfa01109fbed1c415eb925dfb0384c0b8c0 Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Mon, 6 Feb 2023 14:13:29 -0700 Subject: [PATCH 01/10] Initial mods for dynamic fee on moonriver --- runtime/moonriver/src/lib.rs | 25 ++++++++++++++++++++----- runtime/moonriver/tests/common/mod.rs | 2 +- runtime/moonriver/tests/runtime_apis.rs | 2 +- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/runtime/moonriver/src/lib.rs b/runtime/moonriver/src/lib.rs index f0194afd095..2d14eddace6 100644 --- a/runtime/moonriver/src/lib.rs +++ b/runtime/moonriver/src/lib.rs @@ -380,7 +380,7 @@ parameter_types! { /// See `multiplier_can_grow_from_zero` in integration_tests.rs. /// This value is currently only used by pallet-transaction-payment as an assertion that the /// next multiplier is always > min value. - pub MinimumMultiplier: Multiplier = Multiplier::saturating_from_rational(1, 1_000_000u128); + pub MinimumMultiplier: Multiplier = Multiplier::from(10u128); /// Maximum multiplier. We pick a value that is expensive but not impossibly so; it should act /// as a safety net. pub MaximumMultiplier: Multiplier = Multiplier::from(100_000u128); @@ -388,11 +388,26 @@ parameter_types! { pub WeightPerGas: Weight = Weight::from_ref_time(WEIGHT_PER_GAS); } -pub struct FixedGasPrice; -impl FeeCalculator for FixedGasPrice { +pub struct TransactionPaymentAsGasPrice; +impl FeeCalculator for TransactionPaymentAsGasPrice { fn min_gas_price() -> (U256, Weight) { + // note: transaction-payment differs from EIP-1559 in that its tip and length fees are not + // scaled by the multiplier, which means its multiplier will be overstated when + // applied to an ethereum transaction + // note: transaction-payment uses both a congestion modifier (next_fee_multiplier, which is + // updated once per block in on_finalize) and a 'WeightToFee' implementation. Our + // runtime implements this as a 'ConstantModifier', so we can get away with a simple + // multiplication here. + // It is imperative that `saturating_mul_int` be performed as late as possible in the + // expression since it involves fixed point multiplication with a division by a fixed + // divisor. This leads to truncation and subsequent precision loss if performed too early. + // This can lead to min_gas_price being same across blocks even if the multiplier changes. + // There's still some precision loss when the final `gas_price` (used_gas * min_gas_price) + // is computed in frontier, but that's currently unavoidable. + let min_gas_price = TransactionPayment::next_fee_multiplier() + .saturating_mul_int(currency::WEIGHT_FEE.saturating_mul(WEIGHT_PER_GAS as u128)); ( - (1 * currency::GIGAWEI * currency::SUPPLY_FACTOR).into(), + min_gas_price.into(), ::DbWeight::get().reads(1), ) } @@ -440,7 +455,7 @@ where moonbeam_runtime_common::impl_on_charge_evm_transaction!(); impl pallet_evm::Config for Runtime { - type FeeCalculator = FixedGasPrice; + type FeeCalculator = TransactionPaymentAsGasPrice; type GasWeightMapping = pallet_evm::FixedGasWeightMapping; type WeightPerGas = WeightPerGas; type BlockHashMapping = pallet_ethereum::EthereumBlockHashMapping; diff --git a/runtime/moonriver/tests/common/mod.rs b/runtime/moonriver/tests/common/mod.rs index 639ecb3df79..0872d979cd4 100644 --- a/runtime/moonriver/tests/common/mod.rs +++ b/runtime/moonriver/tests/common/mod.rs @@ -28,7 +28,7 @@ pub use moonriver_runtime::{ currency::{GIGAWEI, MOVR, SUPPLY_FACTOR, WEI}, xcm_config::AssetType, AccountId, AssetId, AssetManager, Assets, AuthorInherent, Balance, Balances, CrowdloanRewards, - Ethereum, Executive, FixedGasPrice, InflationInfo, LocalAssets, ParachainStaking, Range, + Ethereum, Executive, TransactionPaymentAsGasPrice, InflationInfo, LocalAssets, ParachainStaking, Range, Runtime, RuntimeCall, RuntimeEvent, System, TransactionConverter, UncheckedExtrinsic, HOURS, WEEKS, }; diff --git a/runtime/moonriver/tests/runtime_apis.rs b/runtime/moonriver/tests/runtime_apis.rs index 450e0e4214c..47be97590b5 100644 --- a/runtime/moonriver/tests/runtime_apis.rs +++ b/runtime/moonriver/tests/runtime_apis.rs @@ -54,7 +54,7 @@ fn ethereum_runtime_rpc_api_account_basic() { #[test] fn ethereum_runtime_rpc_api_gas_price() { ExtBuilder::default().build().execute_with(|| { - assert_eq!(Runtime::gas_price(), FixedGasPrice::min_gas_price().0); + assert_eq!(Runtime::gas_price(), TransactionPaymentAsGasPrice::min_gas_price().0); }); } From d4c78125a3e39750f7eb88dad882081c903b2a3f Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Tue, 7 Feb 2023 10:31:05 -0700 Subject: [PATCH 02/10] Amplify AdjustmentVariable --- runtime/moonriver/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/moonriver/src/lib.rs b/runtime/moonriver/src/lib.rs index 2d14eddace6..652270a6c1e 100644 --- a/runtime/moonriver/src/lib.rs +++ b/runtime/moonriver/src/lib.rs @@ -374,7 +374,7 @@ parameter_types! { pub const TargetBlockFullness: Perquintill = Perquintill::from_percent(25); /// The adjustment variable of the runtime. Higher values will cause `TargetBlockFullness` to /// change the fees more rapidly. This low value causes changes to occur slowly over time. - pub AdjustmentVariable: Multiplier = Multiplier::saturating_from_rational(3, 100_000); + pub AdjustmentVariable: Multiplier = Multiplier::saturating_from_rational(4, 1_000); /// Minimum amount of the multiplier. This value cannot be too low. A test case should ensure /// that combined with `AdjustmentVariable`, we can recover from the minimum. /// See `multiplier_can_grow_from_zero` in integration_tests.rs. From d56536b572ff9a29c1269b808c9cc740de870ecb Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Tue, 7 Feb 2023 16:10:52 -0700 Subject: [PATCH 03/10] Fix (and add) moonriver fee tests --- node/service/src/chain_spec/moonbeam.rs | 2 +- runtime/moonriver/src/lib.rs | 2 +- runtime/moonriver/tests/common/mod.rs | 9 +++ runtime/moonriver/tests/integration_test.rs | 79 ++++++++++++--------- 4 files changed, 56 insertions(+), 36 deletions(-) diff --git a/node/service/src/chain_spec/moonbeam.rs b/node/service/src/chain_spec/moonbeam.rs index 829fa0e8fbc..52a9723e93c 100644 --- a/node/service/src/chain_spec/moonbeam.rs +++ b/node/service/src/chain_spec/moonbeam.rs @@ -315,7 +315,7 @@ pub fn testnet_genesis( // This should initialize it to whatever we have set in the pallet polkadot_xcm: PolkadotXcmConfig::default(), transaction_payment: TransactionPaymentConfig { - multiplier: Multiplier::from(8u128), + multiplier: Multiplier::from(10u128), }, } } diff --git a/runtime/moonriver/src/lib.rs b/runtime/moonriver/src/lib.rs index 652270a6c1e..6970a86026e 100644 --- a/runtime/moonriver/src/lib.rs +++ b/runtime/moonriver/src/lib.rs @@ -191,7 +191,7 @@ pub fn native_version() -> NativeVersion { } const NORMAL_DISPATCH_RATIO: Perbill = Perbill::from_percent(75); -const NORMAL_WEIGHT: Weight = MAXIMUM_BLOCK_WEIGHT.saturating_mul(3).saturating_div(4); +pub const NORMAL_WEIGHT: Weight = MAXIMUM_BLOCK_WEIGHT.saturating_mul(3).saturating_div(4); // Here we assume Ethereum's base fee of 21000 gas and convert to weight, but we // subtract roughly the cost of a balance transfer from it (about 1/3 the cost) // and some cost to account for per-byte-fee. diff --git a/runtime/moonriver/tests/common/mod.rs b/runtime/moonriver/tests/common/mod.rs index 0872d979cd4..6b5dce73750 100644 --- a/runtime/moonriver/tests/common/mod.rs +++ b/runtime/moonriver/tests/common/mod.rs @@ -39,6 +39,7 @@ use sp_runtime::{Digest, DigestItem, Perbill, Percent}; use std::collections::BTreeMap; use fp_rpc::ConvertTransaction; +use pallet_transaction_payment::Multiplier; // A valid signed Alice transfer. pub const VALID_ETH_TX: &str = @@ -303,6 +304,14 @@ impl ExtBuilder { ) .unwrap(); + >::assimilate_storage( + &pallet_transaction_payment::GenesisConfig { + multiplier: Multiplier::from(10u128), + }, + &mut t, + ) + .unwrap(); + let mut ext = sp_io::TestExternalities::new(t); let local_assets = self.local_assets.clone(); let xcm_assets = self.xcm_assets.clone(); diff --git a/runtime/moonriver/tests/integration_test.rs b/runtime/moonriver/tests/integration_test.rs index 3d54984638f..19c474d4d42 100644 --- a/runtime/moonriver/tests/integration_test.rs +++ b/runtime/moonriver/tests/integration_test.rs @@ -21,7 +21,7 @@ mod common; use common::*; -use fp_evm::{Context, FeeCalculator}; +use fp_evm::Context; use frame_support::{ assert_noop, assert_ok, dispatch::{DispatchClass, Dispatchable}, @@ -35,14 +35,16 @@ use frame_support::{ use moonriver_runtime::{ asset_config::LocalAssetInstance, xcm_config::{CurrencyId, SelfReserve, UnitWeightCost}, - AssetId, LocalAssets, PolkadotXcm, Precompiles, RuntimeBlockWeights, XTokens, XcmTransactor, - FOREIGN_ASSET_PRECOMPILE_ADDRESS_PREFIX, LOCAL_ASSET_PRECOMPILE_ADDRESS_PREFIX, + AssetId, LocalAssets, PolkadotXcm, Precompiles, RuntimeBlockWeights, TransactionPayment, + XTokens, XcmTransactor, FOREIGN_ASSET_PRECOMPILE_ADDRESS_PREFIX, + LOCAL_ASSET_PRECOMPILE_ADDRESS_PREFIX, }; use nimbus_primitives::NimbusId; use pallet_evm::PrecompileSet; use pallet_evm_precompileset_assets_erc20::{ AccountIdAssetIdConversion, IsLocal, SELECTOR_LOG_APPROVAL, SELECTOR_LOG_TRANSFER, }; +use pallet_transaction_payment::Multiplier; use pallet_xcm_transactor::{Currency, CurrencyPayment, TransactWeights}; use parity_scale_codec::Encode; use polkadot_parachain::primitives::Sibling; @@ -72,6 +74,8 @@ type LocalAssetsPCall = pallet_evm_precompileset_assets_erc20::Erc20AssetsPrecom type XcmTransactorV1PCall = pallet_evm_precompile_xcm_transactor::v1::XcmTransactorPrecompileV1Call; +const BASE_FEE_GENESIS: u128 = 100 * GIGAWEI; + #[test] fn xcmp_queue_controller_origin_is_root() { // important for the XcmExecutionManager impl of PauseExecution which uses root origin @@ -439,7 +443,7 @@ fn transfer_through_evm_to_stake() { assert_eq!(Balances::free_balance(AccountId::from(BOB)), 2_000 * MOVR); let gas_limit = 100000u64; - let gas_price: U256 = 1_000_000_000u64.into(); + let gas_price: U256 = BASE_FEE_GENESIS.into(); // Bob transfers 1000 MOVR to Charlie via EVM assert_ok!(RuntimeCall::EVM(pallet_evm::Call::::call { source: H160::from(BOB), @@ -845,7 +849,7 @@ fn claim_via_precompile() { // Alice uses the crowdloan precompile to claim through the EVM let gas_limit = 100000u64; - let gas_price: U256 = 1_000_000_000u64.into(); + let gas_price: U256 = BASE_FEE_GENESIS.into(); // Construct the call data (selector, amount) let mut call_data = Vec::::from([0u8; 4]); @@ -1091,7 +1095,7 @@ fn update_reward_address_via_precompile() { // Charlie uses the crowdloan precompile to update address through the EVM let gas_limit = 100000u64; - let gas_price: U256 = 1_000_000_000u64.into(); + let gas_price: U256 = BASE_FEE_GENESIS.into(); // Construct the input data to check if Bob is a contributor let mut call_data = Vec::::from([0u8; 36]); @@ -1211,6 +1215,24 @@ fn ethereum_invalid_transaction() { }); } +#[test] +fn initial_gas_fee_is_correct() { + use fp_evm::FeeCalculator; + + ExtBuilder::default().build().execute_with(|| { + let multiplier = TransactionPayment::next_fee_multiplier(); + assert_eq!(multiplier, Multiplier::from(10u128)); + + assert_eq!( + TransactionPaymentAsGasPrice::min_gas_price(), + ( + 12_500_000_000u128.into(), + Weight::from_ref_time(25_000_000u64) + ) + ); + }); +} + #[test] fn transfer_ed_0_substrate() { ExtBuilder::default() @@ -1237,7 +1259,7 @@ fn transfer_ed_0_evm() { .with_balances(vec![ ( AccountId::from(ALICE), - ((1 * MOVR) + (21_000 * 1_000_000_000)) + (1 * WEI), + ((1 * MOVR) + (21_000 * BASE_FEE_GENESIS)) + (1 * WEI), ), (AccountId::from(BOB), 0), ]) @@ -1250,8 +1272,8 @@ fn transfer_ed_0_evm() { input: Vec::new(), value: (1 * MOVR).into(), gas_limit: 21_000u64, - max_fee_per_gas: U256::from(1_000_000_000), - max_priority_fee_per_gas: None, + max_fee_per_gas: U256::from(BASE_FEE_GENESIS), + max_priority_fee_per_gas: Some(U256::from(BASE_FEE_GENESIS)), nonce: Some(U256::from(0)), access_list: Vec::new(), }) @@ -1267,7 +1289,7 @@ fn refund_ed_0_evm() { .with_balances(vec![ ( AccountId::from(ALICE), - ((1 * MOVR) + (21_777 * 1_000_000_000)), + ((1 * MOVR) + (21_777 * BASE_FEE_GENESIS)), ), (AccountId::from(BOB), 0), ]) @@ -1280,8 +1302,8 @@ fn refund_ed_0_evm() { input: Vec::new(), value: (1 * MOVR).into(), gas_limit: 21_777u64, - max_fee_per_gas: U256::from(1_000_000_000), - max_priority_fee_per_gas: None, + max_fee_per_gas: U256::from(BASE_FEE_GENESIS), + max_priority_fee_per_gas: Some(U256::from(BASE_FEE_GENESIS)), nonce: Some(U256::from(0)), access_list: Vec::new(), }) @@ -1289,7 +1311,7 @@ fn refund_ed_0_evm() { // ALICE is refunded assert_eq!( Balances::free_balance(AccountId::from(ALICE)), - 777 * 1_000_000_000, + 777 * BASE_FEE_GENESIS, ); }); } @@ -1333,7 +1355,7 @@ fn total_issuance_after_evm_transaction_with_priority_fee() { ExtBuilder::default() .with_balances(vec![( AccountId::from(BOB), - (1 * MOVR) + (21_000 * (2 * GIGAWEI)), + (1 * MOVR) + (21_000 * (2 * BASE_FEE_GENESIS)), )]) .build() .execute_with(|| { @@ -1345,16 +1367,15 @@ fn total_issuance_after_evm_transaction_with_priority_fee() { input: Vec::new(), value: (1 * MOVR).into(), gas_limit: 21_000u64, - max_fee_per_gas: U256::from(2 * GIGAWEI), - max_priority_fee_per_gas: Some(U256::from(1 * GIGAWEI)), + max_fee_per_gas: U256::from(2u128 * BASE_FEE_GENESIS), + max_priority_fee_per_gas: Some(U256::from(2u128 * BASE_FEE_GENESIS)), nonce: Some(U256::from(0)), access_list: Vec::new(), }) .dispatch(::RuntimeOrigin::root())); let issuance_after = ::Currency::total_issuance(); - // Fee is 1 GWEI base fee + 1 GWEI tip. - let fee = ((2 * GIGAWEI) * 21_000) as f64; + let fee = ((2 * BASE_FEE_GENESIS) * 21_000) as f64; // 80% was burned. let expected_burn = (fee * 0.8) as u128; assert_eq!(issuance_after, issuance_before - expected_burn,); @@ -1369,7 +1390,7 @@ fn total_issuance_after_evm_transaction_without_priority_fee() { ExtBuilder::default() .with_balances(vec![( AccountId::from(BOB), - (1 * MOVR) + (21_000 * (2 * GIGAWEI)), + (1 * MOVR) + (21_000 * (2 * BASE_FEE_GENESIS)), )]) .build() .execute_with(|| { @@ -1381,16 +1402,15 @@ fn total_issuance_after_evm_transaction_without_priority_fee() { input: Vec::new(), value: (1 * MOVR).into(), gas_limit: 21_000u64, - max_fee_per_gas: U256::from(1 * GIGAWEI), - max_priority_fee_per_gas: None, + max_fee_per_gas: U256::from(BASE_FEE_GENESIS), + max_priority_fee_per_gas: Some(U256::from(BASE_FEE_GENESIS)), nonce: Some(U256::from(0)), access_list: Vec::new(), }) .dispatch(::RuntimeOrigin::root())); let issuance_after = ::Currency::total_issuance(); - // Fee is 1 GWEI base fee. - let fee = ((1 * GIGAWEI) * 21_000) as f64; + let fee = ((1 * BASE_FEE_GENESIS) * 21_000) as f64; // 80% was burned. let expected_burn = (fee * 0.8) as u128; assert_eq!(issuance_after, issuance_before - expected_burn,); @@ -2804,15 +2824,6 @@ fn precompile_existence() { }); } -#[test] -fn base_fee_should_default_to_associate_type_value() { - ExtBuilder::default().build().execute_with(|| { - let (base_fee, _) = - ::FeeCalculator::min_gas_price(); - assert_eq!(base_fee, (1 * GIGAWEI * SUPPLY_FACTOR).into()); - }); -} - #[test] fn evm_revert_substrate_events() { ExtBuilder::default() @@ -2835,7 +2846,7 @@ fn evm_revert_substrate_events() { .into(), value: U256::zero(), // No value sent in EVM gas_limit: 500_000, - max_fee_per_gas: U256::from(1 * GIGAWEI), + max_fee_per_gas: U256::from(BASE_FEE_GENESIS), max_priority_fee_per_gas: None, nonce: Some(U256::from(0)), access_list: Vec::new(), @@ -2874,7 +2885,7 @@ fn evm_success_keeps_substrate_events() { .into(), value: U256::zero(), // No value sent in EVM gas_limit: 500_000, - max_fee_per_gas: U256::from(1 * GIGAWEI), + max_fee_per_gas: U256::from(BASE_FEE_GENESIS), max_priority_fee_per_gas: None, nonce: Some(U256::from(0)), access_list: Vec::new(), From ad170dfe5ae637ecdfe7a279ef9a069ec79b0fe7 Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Wed, 8 Feb 2023 13:43:42 -0700 Subject: [PATCH 04/10] Replace txn blobs with higher fees paid --- runtime/moonriver/tests/common/mod.rs | 6 +++--- runtime/moonriver/tests/runtime_apis.rs | 9 ++++++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/runtime/moonriver/tests/common/mod.rs b/runtime/moonriver/tests/common/mod.rs index 6b5dce73750..fec84199821 100644 --- a/runtime/moonriver/tests/common/mod.rs +++ b/runtime/moonriver/tests/common/mod.rs @@ -43,9 +43,9 @@ use pallet_transaction_payment::Multiplier; // A valid signed Alice transfer. pub const VALID_ETH_TX: &str = - "f86880843b9aca0083b71b0094111111111111111111111111111111111111111182020080820a26a\ - 08c69faf613b9f72dbb029bb5d5acf42742d214c79743507e75fdc8adecdee928a001be4f58ff278ac\ - 61125a81a582a717d9c5d6554326c01b878297c6522b12282"; + "02f86d8205018085174876e80085e8d4a5100082520894f24ff3a9cf04c71dbc94d0b566f7a27b9456\ + 6cac8080c001a0e1094e1a52520a75c0255db96132076dd0f1263089f838bea548cbdbfc64a4d19f031c\ + 92a8cb04e2d68d20a6158d542a07ac440cc8d07b6e36af02db046d92df"; // An invalid signed Alice transfer with a gas limit artifically set to 0. pub const INVALID_ETH_TX: &str = diff --git a/runtime/moonriver/tests/runtime_apis.rs b/runtime/moonriver/tests/runtime_apis.rs index 47be97590b5..6195652f11d 100644 --- a/runtime/moonriver/tests/runtime_apis.rs +++ b/runtime/moonriver/tests/runtime_apis.rs @@ -54,7 +54,10 @@ fn ethereum_runtime_rpc_api_account_basic() { #[test] fn ethereum_runtime_rpc_api_gas_price() { ExtBuilder::default().build().execute_with(|| { - assert_eq!(Runtime::gas_price(), TransactionPaymentAsGasPrice::min_gas_price().0); + assert_eq!( + Runtime::gas_price(), + TransactionPaymentAsGasPrice::min_gas_price().0 + ); }); } @@ -186,7 +189,7 @@ fn ethereum_runtime_rpc_api_create() { #[test] fn ethereum_runtime_rpc_api_current_transaction_statuses() { let alith = ::AddressMapping::into_account_id( - H160::from_str("6be02d1d3665660d22ff9624b7be0551ee1ac91b") + H160::from_str("f24ff3a9cf04c71dbc94d0b566f7a27b94566cac") .expect("internal H160 is valid; qed"), ); ExtBuilder::default() @@ -249,7 +252,7 @@ fn ethereum_runtime_rpc_api_current_block() { #[test] fn ethereum_runtime_rpc_api_current_receipts() { let alith = ::AddressMapping::into_account_id( - H160::from_str("6be02d1d3665660d22ff9624b7be0551ee1ac91b") + H160::from_str("f24ff3a9cf04c71dbc94d0b566f7a27b94566cac") .expect("internal H160 is valid; qed"), ); ExtBuilder::default() From f967f4f236365c94605c5c242c51bc91de4c2cdc Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Thu, 9 Feb 2023 15:01:48 -0700 Subject: [PATCH 05/10] fmt --- runtime/moonriver/tests/common/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/runtime/moonriver/tests/common/mod.rs b/runtime/moonriver/tests/common/mod.rs index fec84199821..3ad26be42a2 100644 --- a/runtime/moonriver/tests/common/mod.rs +++ b/runtime/moonriver/tests/common/mod.rs @@ -28,9 +28,9 @@ pub use moonriver_runtime::{ currency::{GIGAWEI, MOVR, SUPPLY_FACTOR, WEI}, xcm_config::AssetType, AccountId, AssetId, AssetManager, Assets, AuthorInherent, Balance, Balances, CrowdloanRewards, - Ethereum, Executive, TransactionPaymentAsGasPrice, InflationInfo, LocalAssets, ParachainStaking, Range, - Runtime, RuntimeCall, RuntimeEvent, System, TransactionConverter, UncheckedExtrinsic, HOURS, - WEEKS, + Ethereum, Executive, InflationInfo, LocalAssets, ParachainStaking, Range, Runtime, RuntimeCall, + RuntimeEvent, System, TransactionConverter, TransactionPaymentAsGasPrice, UncheckedExtrinsic, + HOURS, WEEKS, }; use nimbus_primitives::{NimbusId, NIMBUS_ENGINE_ID}; use sp_core::{Encode, H160}; From 56cf6b225f446bee44a5f42190d25b8affac0f27 Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Thu, 9 Feb 2023 16:03:31 -0700 Subject: [PATCH 06/10] Adjust moonriver's initial fee, not moonbeam's --- node/service/src/chain_spec/moonbeam.rs | 2 +- node/service/src/chain_spec/moonriver.rs | 2 +- tests/tests/test-fees/test-length-fees.ts | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/node/service/src/chain_spec/moonbeam.rs b/node/service/src/chain_spec/moonbeam.rs index 52a9723e93c..829fa0e8fbc 100644 --- a/node/service/src/chain_spec/moonbeam.rs +++ b/node/service/src/chain_spec/moonbeam.rs @@ -315,7 +315,7 @@ pub fn testnet_genesis( // This should initialize it to whatever we have set in the pallet polkadot_xcm: PolkadotXcmConfig::default(), transaction_payment: TransactionPaymentConfig { - multiplier: Multiplier::from(10u128), + multiplier: Multiplier::from(8u128), }, } } diff --git a/node/service/src/chain_spec/moonriver.rs b/node/service/src/chain_spec/moonriver.rs index e145b3baae1..ebc4c8abbe4 100644 --- a/node/service/src/chain_spec/moonriver.rs +++ b/node/service/src/chain_spec/moonriver.rs @@ -328,7 +328,7 @@ pub fn testnet_genesis( // This should initialize it to whatever we have set in the pallet polkadot_xcm: PolkadotXcmConfig::default(), transaction_payment: TransactionPaymentConfig { - multiplier: Multiplier::from(8u128), + multiplier: Multiplier::from(10u128), }, } } diff --git a/tests/tests/test-fees/test-length-fees.ts b/tests/tests/test-fees/test-length-fees.ts index fa7ecbd7ccb..d6d07437635 100644 --- a/tests/tests/test-fees/test-length-fees.ts +++ b/tests/tests/test-fees/test-length-fees.ts @@ -36,7 +36,7 @@ describeDevMoonbeam( (context) => { it("should have low balance transfer fees", async () => { const fee = await testBalanceTransfer(context); - expect(fee).to.equal(79_359_001_520_875n); + expect(fee).to.equal(96_045_001_520_875n); }); }, "Legacy", @@ -48,7 +48,7 @@ describeDevMoonbeam( (context) => { it("should have expensive runtime-upgrade fees", async () => { const fee = await testRuntimeUpgrade(context); - expect(fee).to.equal(9_226_801_665_723_667_008n); + expect(fee).to.equal(9_226_801_765_723_667_008n); }); }, "Legacy", From 155ceb39ad76305e00873977531266dec13f5fd9 Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Fri, 10 Feb 2023 15:20:04 -0700 Subject: [PATCH 07/10] Test min as base_fee --- runtime/moonriver/tests/integration_test.rs | 22 +++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/runtime/moonriver/tests/integration_test.rs b/runtime/moonriver/tests/integration_test.rs index 19c474d4d42..66c5ce1652c 100644 --- a/runtime/moonriver/tests/integration_test.rs +++ b/runtime/moonriver/tests/integration_test.rs @@ -1233,6 +1233,28 @@ fn initial_gas_fee_is_correct() { }); } +#[test] +fn min_gas_fee_is_correct() { + use fp_evm::FeeCalculator; + use frame_support::traits::Hooks; + + ExtBuilder::default().build().execute_with(|| { + pallet_transaction_payment::NextFeeMultiplier::::put(Multiplier::from(0)); + TransactionPayment::on_finalize(System::block_number()); // should trigger min to kick in + + let multiplier = TransactionPayment::next_fee_multiplier(); + assert_eq!(multiplier, Multiplier::from(10u128)); + + assert_eq!( + TransactionPaymentAsGasPrice::min_gas_price(), + ( + 12_500_000_000u128.into(), + Weight::from_ref_time(25_000_000u64) + ) + ); + }); +} + #[test] fn transfer_ed_0_substrate() { ExtBuilder::default() From d128e3c1a96fa577bad840a486ecc0ee58f47062 Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Fri, 10 Feb 2023 16:23:40 -0700 Subject: [PATCH 08/10] Add attempt at executeProposalWithCouncil --- .../test-fee-multiplier-moonriver.ts | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 tests/tests/test-fees/test-fee-multiplier-moonriver.ts diff --git a/tests/tests/test-fees/test-fee-multiplier-moonriver.ts b/tests/tests/test-fees/test-fee-multiplier-moonriver.ts new file mode 100644 index 00000000000..5d20497be8a --- /dev/null +++ b/tests/tests/test-fees/test-fee-multiplier-moonriver.ts @@ -0,0 +1,61 @@ +import "@moonbeam-network/api-augment/moonbase"; +import { expect } from "chai"; +import { BN, bnToHex } from "@polkadot/util"; +import { describeDevMoonbeam } from "../../util/setup-dev-tests"; +import { baltathar, BALTATHAR_ADDRESS, generateKeyringPair } from "../../util/accounts"; +import { alith } from "../../util/accounts"; +import { createContract, createContractExecution } from "../../util/transactions"; +import { + RawXcmMessage, + XcmFragment, + descendOriginFromAddress, + injectHrmpMessageAndSeal, +} from "../../util/xcm"; +import { expectOk } from "../../util/expect"; +import { KeyringPair } from "@substrate/txwrapper-core"; +import { TARGET_FILL_AMOUNT } from "../../util/constants"; +import { execTechnicalCommitteeProposal, executeProposalWithCouncil } from "../../util/governance"; +import { blake2AsHex } from "@polkadot/util-crypto"; + +// Note on the values from 'transactionPayment.nextFeeMultiplier': this storage item is actually a +// FixedU128, which is basically a u128 with an implicit denominator of 10^18. However, this +// denominator is omitted when it is queried through the API, leaving some very large numbers. +// +// To make sense of them, basically remove 18 zeroes (divide by 10^18). This will give you the +// number used internally by transaction-payment for fee calculations. + +describeDevMoonbeam("Max Fee Multiplier (Moonriver)", (context) => { + beforeEach("set to max multiplier (moonriver)", async function () { + this.timeout(20000); + const MULTIPLIER_STORAGE_KEY = context.polkadotApi.query.transactionPayment.nextFeeMultiplier + .key(0) + .toString(); + + const U128_MAX = new BN("340282366920938463463374607431768211455"); + + // set transaction-payment's multiplier to something above max in storage. on the next round, + // it should enforce its upper bound and reset it. + let proposal = context.polkadotApi.tx.system.setStorage([ + [MULTIPLIER_STORAGE_KEY, bnToHex(U128_MAX, { isLe: true, bitLength: 128 })], + ]); + let encodedProposal = proposal.method.toHex(); + let encodedHash = blake2AsHex(encodedProposal); + await executeProposalWithCouncil(context.polkadotApi, encodedHash); + }); + + it("should enforce upper bound (moonriver)", async function () { + this.timeout(20000); + // we set it to u128_max, but the max should have been enforced in on_finalize() + const multiplier = ( + await context.polkadotApi.query.transactionPayment.nextFeeMultiplier() + ).toBigInt(); + expect(multiplier).to.equal(100_000_000_000_000_000_000_000n); + + const result = await context.ethers.send("eth_gasPrice", []); + const gasPrice = BigInt(result); + expect(gasPrice).to.eq(125_000_000_000_000n); + }); + +}, +"Legacy", +"moonriver"); From 1f14645c4c1888c33c8e00369c9d598d7fde15f1 Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Mon, 13 Feb 2023 11:04:00 -0700 Subject: [PATCH 09/10] Revert "Add attempt at executeProposalWithCouncil" This reverts commit d128e3c1a96fa577bad840a486ecc0ee58f47062. --- .../test-fee-multiplier-moonriver.ts | 61 ------------------- 1 file changed, 61 deletions(-) delete mode 100644 tests/tests/test-fees/test-fee-multiplier-moonriver.ts diff --git a/tests/tests/test-fees/test-fee-multiplier-moonriver.ts b/tests/tests/test-fees/test-fee-multiplier-moonriver.ts deleted file mode 100644 index 5d20497be8a..00000000000 --- a/tests/tests/test-fees/test-fee-multiplier-moonriver.ts +++ /dev/null @@ -1,61 +0,0 @@ -import "@moonbeam-network/api-augment/moonbase"; -import { expect } from "chai"; -import { BN, bnToHex } from "@polkadot/util"; -import { describeDevMoonbeam } from "../../util/setup-dev-tests"; -import { baltathar, BALTATHAR_ADDRESS, generateKeyringPair } from "../../util/accounts"; -import { alith } from "../../util/accounts"; -import { createContract, createContractExecution } from "../../util/transactions"; -import { - RawXcmMessage, - XcmFragment, - descendOriginFromAddress, - injectHrmpMessageAndSeal, -} from "../../util/xcm"; -import { expectOk } from "../../util/expect"; -import { KeyringPair } from "@substrate/txwrapper-core"; -import { TARGET_FILL_AMOUNT } from "../../util/constants"; -import { execTechnicalCommitteeProposal, executeProposalWithCouncil } from "../../util/governance"; -import { blake2AsHex } from "@polkadot/util-crypto"; - -// Note on the values from 'transactionPayment.nextFeeMultiplier': this storage item is actually a -// FixedU128, which is basically a u128 with an implicit denominator of 10^18. However, this -// denominator is omitted when it is queried through the API, leaving some very large numbers. -// -// To make sense of them, basically remove 18 zeroes (divide by 10^18). This will give you the -// number used internally by transaction-payment for fee calculations. - -describeDevMoonbeam("Max Fee Multiplier (Moonriver)", (context) => { - beforeEach("set to max multiplier (moonriver)", async function () { - this.timeout(20000); - const MULTIPLIER_STORAGE_KEY = context.polkadotApi.query.transactionPayment.nextFeeMultiplier - .key(0) - .toString(); - - const U128_MAX = new BN("340282366920938463463374607431768211455"); - - // set transaction-payment's multiplier to something above max in storage. on the next round, - // it should enforce its upper bound and reset it. - let proposal = context.polkadotApi.tx.system.setStorage([ - [MULTIPLIER_STORAGE_KEY, bnToHex(U128_MAX, { isLe: true, bitLength: 128 })], - ]); - let encodedProposal = proposal.method.toHex(); - let encodedHash = blake2AsHex(encodedProposal); - await executeProposalWithCouncil(context.polkadotApi, encodedHash); - }); - - it("should enforce upper bound (moonriver)", async function () { - this.timeout(20000); - // we set it to u128_max, but the max should have been enforced in on_finalize() - const multiplier = ( - await context.polkadotApi.query.transactionPayment.nextFeeMultiplier() - ).toBigInt(); - expect(multiplier).to.equal(100_000_000_000_000_000_000_000n); - - const result = await context.ethers.send("eth_gasPrice", []); - const gasPrice = BigInt(result); - expect(gasPrice).to.eq(125_000_000_000_000n); - }); - -}, -"Legacy", -"moonriver"); From 000d5ac686138c1f9052b153b96213950f15f6c3 Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Mon, 13 Feb 2023 11:18:10 -0700 Subject: [PATCH 10/10] Reduce min multiplier by factor of 10 --- runtime/moonriver/src/lib.rs | 2 +- runtime/moonriver/tests/integration_test.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/runtime/moonriver/src/lib.rs b/runtime/moonriver/src/lib.rs index 6970a86026e..338e7ebe8c1 100644 --- a/runtime/moonriver/src/lib.rs +++ b/runtime/moonriver/src/lib.rs @@ -380,7 +380,7 @@ parameter_types! { /// See `multiplier_can_grow_from_zero` in integration_tests.rs. /// This value is currently only used by pallet-transaction-payment as an assertion that the /// next multiplier is always > min value. - pub MinimumMultiplier: Multiplier = Multiplier::from(10u128); + pub MinimumMultiplier: Multiplier = Multiplier::from(1u128); /// Maximum multiplier. We pick a value that is expensive but not impossibly so; it should act /// as a safety net. pub MaximumMultiplier: Multiplier = Multiplier::from(100_000u128); diff --git a/runtime/moonriver/tests/integration_test.rs b/runtime/moonriver/tests/integration_test.rs index 66c5ce1652c..f713e293424 100644 --- a/runtime/moonriver/tests/integration_test.rs +++ b/runtime/moonriver/tests/integration_test.rs @@ -1243,12 +1243,12 @@ fn min_gas_fee_is_correct() { TransactionPayment::on_finalize(System::block_number()); // should trigger min to kick in let multiplier = TransactionPayment::next_fee_multiplier(); - assert_eq!(multiplier, Multiplier::from(10u128)); + assert_eq!(multiplier, Multiplier::from(1u128)); assert_eq!( TransactionPaymentAsGasPrice::min_gas_price(), ( - 12_500_000_000u128.into(), + 1_250_000_000u128.into(), Weight::from_ref_time(25_000_000u64) ) );