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/runtime/moonriver/src/lib.rs b/runtime/moonriver/src/lib.rs index 05b6c47d94d..12fc8ffec29 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. @@ -374,13 +374,13 @@ 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. /// 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(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); @@ -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 b13d545e489..28788fe10fd 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, FixedGasPrice, 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}; @@ -39,12 +39,13 @@ 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 = - "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 = @@ -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 82b73ab7f5d..5817a8e5370 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,46 @@ 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 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(1u128)); + + assert_eq!( + TransactionPaymentAsGasPrice::min_gas_price(), + ( + 1_250_000_000u128.into(), + Weight::from_ref_time(25_000_000u64) + ) + ); + }); +} + #[test] fn transfer_ed_0_substrate() { ExtBuilder::default() @@ -1237,7 +1281,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 +1294,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 +1311,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 +1324,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 +1333,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 +1377,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 +1389,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 +1412,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 +1424,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 +2846,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 +2868,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 +2907,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(), diff --git a/runtime/moonriver/tests/runtime_apis.rs b/runtime/moonriver/tests/runtime_apis.rs index 450e0e4214c..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(), FixedGasPrice::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() diff --git a/tests/tests/test-fees/test-length-fees.ts b/tests/tests/test-fees/test-length-fees.ts index 8bcb114f671..0699413fd83 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(82139401520875n); + expect(fee).to.equal(99_520_501_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",