From 3a6f89bed31b8059742ea92eb4c5d9693d802aee Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Tue, 19 Apr 2022 11:28:37 -0600 Subject: [PATCH 01/15] Use FixedGasPrice for transaction-payment fee modifier --- runtime/moonbase/src/lib.rs | 69 ++++++++++++++++++++++--------------- 1 file changed, 42 insertions(+), 27 deletions(-) diff --git a/runtime/moonbase/src/lib.rs b/runtime/moonbase/src/lib.rs index 320785da7ab..82f2b7cdb17 100644 --- a/runtime/moonbase/src/lib.rs +++ b/runtime/moonbase/src/lib.rs @@ -69,7 +69,7 @@ use pallet_evm::{ Account as EVMAccount, EVMCurrencyAdapter, EnsureAddressNever, EnsureAddressRoot, FeeCalculator, GasWeightMapping, OnChargeEVMTransaction as OnChargeEVMTransactionT, Runner, }; -use pallet_transaction_payment::{CurrencyAdapter, Multiplier, TargetedFeeAdjustment}; +use pallet_transaction_payment::{CurrencyAdapter, Multiplier, MultiplierUpdate}; pub use parachain_staking::{InflationInfo, Range}; use parity_scale_codec::{Decode, Encode, MaxEncodedLen}; use scale_info::TypeInfo; @@ -77,7 +77,7 @@ use sp_api::impl_runtime_apis; use sp_core::{OpaqueMetadata, H160, H256, U256}; use sp_runtime::{ create_runtime_str, generic, impl_opaque_keys, - traits::{BlakeTwo256, Block as BlockT, Dispatchable, IdentityLookup, PostDispatchInfoOf}, + traits::{BlakeTwo256, Block as BlockT, Convert, Dispatchable, IdentityLookup, PostDispatchInfoOf}, transaction_validity::{ InvalidTransaction, TransactionSource, TransactionValidity, TransactionValidityError, }, @@ -323,7 +323,7 @@ impl pallet_transaction_payment::Config for Runtime { type TransactionByteFee = ConstU128<{ currency::TRANSACTION_BYTE_FEE }>; type OperationalFeeMultiplier = ConstU8<5>; type WeightToFee = IdentityFee; - type FeeMultiplierUpdate = SlowAdjustingFeeUpdate; + type FeeMultiplierUpdate = FixedGasPrice; } impl pallet_sudo::Config for Runtime { @@ -359,41 +359,56 @@ impl pallet_evm::GasWeightMapping for MoonbeamGasWeightMapping { parameter_types! { pub BlockGasLimit: U256 = U256::from(NORMAL_DISPATCH_RATIO * MAXIMUM_BLOCK_WEIGHT / WEIGHT_PER_GAS); - /// The portion of the `NORMAL_DISPATCH_RATIO` that we adjust the fees with. Blocks filled less - /// than this will decrease the weight and more will increase. - 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); - /// 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 PrecompilesValue: MoonbasePrecompiles = MoonbasePrecompiles::<_>::new(); } +/// Implementation of both Ethereum and Substrate congestion-based fee modifiers (EIP-1559 and +/// pallet-transaction-payment) which returns a fixed fee. pub struct FixedGasPrice; +impl FixedGasPrice { + fn gas_price() -> U256 { + (1 * currency::GIGAWEI * currency::SUPPLY_FACTOR).into() + } +} + impl FeeCalculator for FixedGasPrice { fn min_gas_price() -> U256 { - (1 * currency::GIGAWEI * currency::SUPPLY_FACTOR).into() + Self::gas_price() } } -/// Parameterized slow adjusting fee updated based on -/// https://w3f-research.readthedocs.io/en/latest/polkadot/overview/2-token-economics.html#-2.-slow-adjusting-mechanism // editorconfig-checker-disable-line +/// Implementation of MultiplierUpdate which uses FixedGasPrice to update +/// pallet-transaction-payment's fee modifier. /// -/// The adjustment algorithm boils down to: +/// Note that the MultiplierUpdate trait itself is currently only used in transaction-payment's +/// integrity_test() hook. The important conversion occurs in its on_finalize() hook and uses the +/// Convert trait. /// -/// diff = (previous_block_weight - target) / maximum_block_weight -/// next_multiplier = prev_multiplier * (1 + (v * diff) + ((v * diff)^2 / 2)) -/// assert(next_multiplier > min) -/// where: v is AdjustmentVariable -/// target is TargetBlockFullness -/// min is MinimumMultiplier -pub type SlowAdjustingFeeUpdate = - TargetedFeeAdjustment; +/// Reminder: FixedU128 is a fixed point unsigned in the range +/// [0.000000000000000000, 340282366920938463463.374607431768211455] +impl MultiplierUpdate for FixedGasPrice { + fn min() -> Multiplier { + Self::gas_price() + .saturating_mul(WEIGHT_PER_GAS.into()) + .as_u128() // TODO: this panics. we should enforce the upper bound from FixedU128 + .into() + } + fn target() -> Perquintill { + Perquintill::from_percent(0) + } + fn variability() -> Multiplier { + 0.into() + } +} + +impl Convert for FixedGasPrice { + fn convert(_previous: Multiplier) -> Multiplier { + Self::gas_price() + .saturating_mul(WEIGHT_PER_GAS.into()) + .as_u128() // TODO: this panics. we should enforce the upper bound from FixedU128 + .into() + } +} /// The author inherent provides an AccountId, but pallet evm needs an H160. /// This simple adapter makes the conversion for any types T, U such that T: Into From c76e7abfd8968295c1a1457a971dd57afd3dfc48 Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Tue, 19 Apr 2022 11:45:40 -0600 Subject: [PATCH 02/15] fmt --- runtime/moonbase/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/runtime/moonbase/src/lib.rs b/runtime/moonbase/src/lib.rs index 82f2b7cdb17..e9a93dd314b 100644 --- a/runtime/moonbase/src/lib.rs +++ b/runtime/moonbase/src/lib.rs @@ -77,7 +77,9 @@ use sp_api::impl_runtime_apis; use sp_core::{OpaqueMetadata, H160, H256, U256}; use sp_runtime::{ create_runtime_str, generic, impl_opaque_keys, - traits::{BlakeTwo256, Block as BlockT, Convert, Dispatchable, IdentityLookup, PostDispatchInfoOf}, + traits::{ + BlakeTwo256, Block as BlockT, Convert, Dispatchable, IdentityLookup, PostDispatchInfoOf, + }, transaction_validity::{ InvalidTransaction, TransactionSource, TransactionValidity, TransactionValidityError, }, From 7f30077637a2c3a9ff37a451996c00647cc0441a Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Wed, 18 May 2022 15:17:06 +0000 Subject: [PATCH 03/15] Clean up --- runtime/moonbase/src/lib.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/runtime/moonbase/src/lib.rs b/runtime/moonbase/src/lib.rs index e9a93dd314b..9299580af4f 100644 --- a/runtime/moonbase/src/lib.rs +++ b/runtime/moonbase/src/lib.rs @@ -83,7 +83,7 @@ use sp_runtime::{ transaction_validity::{ InvalidTransaction, TransactionSource, TransactionValidity, TransactionValidityError, }, - ApplyExtrinsicResult, FixedPointNumber, Perbill, Percent, Permill, Perquintill, + ApplyExtrinsicResult, Perbill, Percent, Permill, Perquintill, SaturatedConversion, }; use sp_std::{ @@ -371,6 +371,12 @@ impl FixedGasPrice { fn gas_price() -> U256 { (1 * currency::GIGAWEI * currency::SUPPLY_FACTOR).into() } + fn weight_multiplier() -> Multiplier { + Self::gas_price() + .saturating_mul(WEIGHT_PER_GAS.into()) + .as_u128() // TODO: this panics. a simple test case should suffice to check this + .into() + } } impl FeeCalculator for FixedGasPrice { @@ -386,14 +392,11 @@ impl FeeCalculator for FixedGasPrice { /// integrity_test() hook. The important conversion occurs in its on_finalize() hook and uses the /// Convert trait. /// -/// Reminder: FixedU128 is a fixed point unsigned in the range +/// Reminder: Multiplier is a FixedU128, which is a fixed point unsigned in the range /// [0.000000000000000000, 340282366920938463463.374607431768211455] impl MultiplierUpdate for FixedGasPrice { fn min() -> Multiplier { - Self::gas_price() - .saturating_mul(WEIGHT_PER_GAS.into()) - .as_u128() // TODO: this panics. we should enforce the upper bound from FixedU128 - .into() + Self::weight_multiplier() } fn target() -> Perquintill { Perquintill::from_percent(0) @@ -405,10 +408,7 @@ impl MultiplierUpdate for FixedGasPrice { impl Convert for FixedGasPrice { fn convert(_previous: Multiplier) -> Multiplier { - Self::gas_price() - .saturating_mul(WEIGHT_PER_GAS.into()) - .as_u128() // TODO: this panics. we should enforce the upper bound from FixedU128 - .into() + Self::weight_multiplier() } } From 2fae10b61aeca0a6fb43b5cc3e7efb66339acd03 Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Wed, 18 May 2022 16:36:14 +0000 Subject: [PATCH 04/15] Add test, remove old ones --- runtime/moonbase/src/lib.rs | 10 +++++ runtime/moonbase/tests/integration_test.rs | 46 ---------------------- 2 files changed, 10 insertions(+), 46 deletions(-) diff --git a/runtime/moonbase/src/lib.rs b/runtime/moonbase/src/lib.rs index 54d54670ac8..2175b054f3b 100644 --- a/runtime/moonbase/src/lib.rs +++ b/runtime/moonbase/src/lib.rs @@ -1481,4 +1481,14 @@ mod tests { 50 ); } + + #[test] + fn fixed_gas_price_is_sane() { + let expected: Multiplier = 25000000000000.into(); + + // this also tests that the conversions do not panic (assuming it uses constants) + assert_eq!( + FixedGasPrice::weight_multiplier(), + expected); + } } diff --git a/runtime/moonbase/tests/integration_test.rs b/runtime/moonbase/tests/integration_test.rs index 74b1cd904db..6e999e9a0bc 100644 --- a/runtime/moonbase/tests/integration_test.rs +++ b/runtime/moonbase/tests/integration_test.rs @@ -2299,52 +2299,6 @@ where }); } -#[test] -fn multiplier_can_grow_from_zero() { - let minimum_multiplier = moonbase_runtime::MinimumMultiplier::get(); - let target = moonbase_runtime::TargetBlockFullness::get() - * BlockWeights::get() - .get(DispatchClass::Normal) - .max_total - .unwrap(); - // if the min is too small, then this will not change, and we are doomed forever. - // the weight is 1/100th bigger than target. - run_with_system_weight(target * 101 / 100, || { - let next = moonbase_runtime::SlowAdjustingFeeUpdate::::convert(minimum_multiplier); - assert!( - next > minimum_multiplier, - "{:?} !>= {:?}", - next, - minimum_multiplier - ); - }) -} - -#[test] -#[ignore] // test runs for a very long time -fn multiplier_growth_simulator() { - // assume the multiplier is initially set to its minimum. We update it with values twice the - //target (target is 25%, thus 50%) and we see at which point it reaches 1. - let mut multiplier = moonbase_runtime::MinimumMultiplier::get(); - let block_weight = moonbase_runtime::TargetBlockFullness::get() - * BlockWeights::get() - .get(DispatchClass::Normal) - .max_total - .unwrap() - * 2; - let mut blocks = 0; - while multiplier <= Multiplier::one() { - run_with_system_weight(block_weight, || { - let next = moonbase_runtime::SlowAdjustingFeeUpdate::::convert(multiplier); - // ensure that it is growing as well. - assert!(next > multiplier, "{:?} !>= {:?}", next, multiplier); - multiplier = next; - }); - blocks += 1; - println!("block = {} multiplier {:?}", blocks, multiplier); - } -} - #[test] fn ethereum_invalid_transaction() { ExtBuilder::default().build().execute_with(|| { From fc150357abefe2c8ba298cf90da805204a3a9496 Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Wed, 18 May 2022 16:47:01 +0000 Subject: [PATCH 05/15] fmt --- runtime/moonbase/src/lib.rs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/runtime/moonbase/src/lib.rs b/runtime/moonbase/src/lib.rs index 2175b054f3b..3d100c2f4e0 100644 --- a/runtime/moonbase/src/lib.rs +++ b/runtime/moonbase/src/lib.rs @@ -83,8 +83,7 @@ use sp_runtime::{ transaction_validity::{ InvalidTransaction, TransactionSource, TransactionValidity, TransactionValidityError, }, - ApplyExtrinsicResult, Perbill, Percent, Permill, Perquintill, - SaturatedConversion, + ApplyExtrinsicResult, Perbill, Percent, Permill, Perquintill, SaturatedConversion, }; use sp_std::{ convert::{From, Into, TryFrom}, @@ -1482,13 +1481,11 @@ mod tests { ); } - #[test] - fn fixed_gas_price_is_sane() { - let expected: Multiplier = 25000000000000.into(); + #[test] + fn fixed_gas_price_is_sane() { + let expected: Multiplier = 25000000000000.into(); - // this also tests that the conversions do not panic (assuming it uses constants) - assert_eq!( - FixedGasPrice::weight_multiplier(), - expected); - } + // this also tests that the conversions do not panic (assuming it uses constants) + assert_eq!(FixedGasPrice::weight_multiplier(), expected); + } } From f903172f9c66a1d0018c2f81aac352334c6e334a Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Wed, 18 May 2022 18:07:29 +0000 Subject: [PATCH 06/15] Remove unused code --- runtime/moonbase/tests/integration_test.rs | 27 ++++------------------ 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/runtime/moonbase/tests/integration_test.rs b/runtime/moonbase/tests/integration_test.rs index 6e999e9a0bc..696a1411200 100644 --- a/runtime/moonbase/tests/integration_test.rs +++ b/runtime/moonbase/tests/integration_test.rs @@ -29,14 +29,13 @@ use frame_support::{ fungible::Inspect, fungibles::Inspect as FungiblesInspect, EnsureOrigin, PalletInfo, StorageInfo, StorageInfoTrait, }, - weights::{DispatchClass, Weight}, StorageHasher, Twox128, }; use moonbase_runtime::{ asset_config::AssetRegistrarMetadata, asset_config::LocalAssetInstance, currency::UNIT, get, - xcm_config::AssetType, AccountId, AssetId, AssetManager, Assets, Balances, BaseFee, - BlockWeights, Call, CrowdloanRewards, Event, LocalAssets, ParachainStaking, PolkadotXcm, - Precompiles, Runtime, System, XTokens, XcmTransactor, FOREIGN_ASSET_PRECOMPILE_ADDRESS_PREFIX, + xcm_config::AssetType, AccountId, AssetId, AssetManager, Assets, Balances, BaseFee, Call, + CrowdloanRewards, Event, LocalAssets, ParachainStaking, PolkadotXcm, Precompiles, Runtime, + System, XTokens, XcmTransactor, FOREIGN_ASSET_PRECOMPILE_ADDRESS_PREFIX, LOCAL_ASSET_PRECOMPILE_ADDRESS_PREFIX, }; @@ -48,14 +47,10 @@ use pallet_evm_precompile_assets_erc20::{ }; use xtokens_precompiles::Action as XtokensAction; -use pallet_transaction_payment::Multiplier; use parity_scale_codec::Encode; use sha3::{Digest, Keccak256}; use sp_core::{crypto::UncheckedFrom, ByteArray, Pair, H160, U256}; -use sp_runtime::{ - traits::{Convert, One}, - DispatchError, ModuleError, TokenError, -}; +use sp_runtime::{DispatchError, ModuleError, TokenError}; use xcm::latest::prelude::*; #[test] @@ -2285,20 +2280,6 @@ fn xtokens_precompile_transfer_local_asset() { }) } -fn run_with_system_weight(w: Weight, mut assertions: F) -where - F: FnMut() -> (), -{ - let mut t: sp_io::TestExternalities = frame_system::GenesisConfig::default() - .build_storage::() - .unwrap() - .into(); - t.execute_with(|| { - System::set_block_consumed_resources(w, 0); - assertions() - }); -} - #[test] fn ethereum_invalid_transaction() { ExtBuilder::default().build().execute_with(|| { From 41b787217f10ec41ff3bb180b2d4a1f2990adfcc Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Thu, 19 May 2022 18:37:27 +0000 Subject: [PATCH 07/15] Correct gas -> weight conversion --- runtime/moonbase/src/lib.rs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/runtime/moonbase/src/lib.rs b/runtime/moonbase/src/lib.rs index 3d100c2f4e0..144afdad756 100644 --- a/runtime/moonbase/src/lib.rs +++ b/runtime/moonbase/src/lib.rs @@ -372,14 +372,13 @@ parameter_types! { /// pallet-transaction-payment) which returns a fixed fee. pub struct FixedGasPrice; impl FixedGasPrice { - fn gas_price() -> U256 { + pub fn gas_price() -> U256 { (1 * currency::GIGAWEI * currency::SUPPLY_FACTOR).into() } - fn weight_multiplier() -> Multiplier { - Self::gas_price() - .saturating_mul(WEIGHT_PER_GAS.into()) - .as_u128() // TODO: this panics. a simple test case should suffice to check this - .into() + pub fn weight_multiplier() -> Multiplier { + // note that 'as_u128' will panic if gas_price() overflows a u128 + let weight_price = Self::gas_price() / U256::from(WEIGHT_PER_GAS); + weight_price.as_u128().into() } } @@ -1482,10 +1481,8 @@ mod tests { } #[test] - fn fixed_gas_price_is_sane() { - let expected: Multiplier = 25000000000000.into(); - + fn fixed_gas_price_as_weight_multiplier_is_known() { // this also tests that the conversions do not panic (assuming it uses constants) - assert_eq!(FixedGasPrice::weight_multiplier(), expected); + assert_eq!(FixedGasPrice::weight_multiplier(), 40_000.into()); } } From 76fb9ebd170303141b963c1b7efaacc00e916f0e Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Thu, 19 May 2022 18:37:51 +0000 Subject: [PATCH 08/15] Starting point for calling query_fee_details --- runtime/moonbase/tests/integration_test.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/runtime/moonbase/tests/integration_test.rs b/runtime/moonbase/tests/integration_test.rs index 696a1411200..c3b21731864 100644 --- a/runtime/moonbase/tests/integration_test.rs +++ b/runtime/moonbase/tests/integration_test.rs @@ -2792,3 +2792,25 @@ fn base_fee_should_default_to_associate_type_value() { ); }); } + +#[test] +fn fixed_gas_price_is_sane() { + use pallet_transaction_payment::FeeDetails; + use sp_runtime::testing::TestXt; + + let remark = frame_system::Call::::remark { remark: vec![] }; + let signed_xt = TestXt::<_, ()>::new(remark, Some((1, ()))); + + ExtBuilder::default().build().execute_with(|| { + + let fee_details = moonbase_runtime::TransactionPayment::query_fee_details(signed_xt, 1000); + + assert_eq!( + fee_details, + FeeDetails { + inclusion_fee: None, + tip: 0u32.into(), + } + ); + }); +} From b4cad225758f1f5107ca41c2d052a9f1dd39d268 Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Thu, 19 May 2022 20:44:00 +0000 Subject: [PATCH 09/15] Incremental work on fees testing --- runtime/moonbase/tests/integration_test.rs | 37 ++++++++++++++++++---- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/runtime/moonbase/tests/integration_test.rs b/runtime/moonbase/tests/integration_test.rs index c3b21731864..58f345f4a94 100644 --- a/runtime/moonbase/tests/integration_test.rs +++ b/runtime/moonbase/tests/integration_test.rs @@ -2794,22 +2794,45 @@ fn base_fee_should_default_to_associate_type_value() { } #[test] -fn fixed_gas_price_is_sane() { - use pallet_transaction_payment::FeeDetails; +fn substrate_based_fees_are_known() { + use pallet_transaction_payment::{FeeDetails, InclusionFee}; use sp_runtime::testing::TestXt; - let remark = frame_system::Call::::remark { remark: vec![] }; - let signed_xt = TestXt::<_, ()>::new(remark, Some((1, ()))); + let generate_xt = |weight: u128| { + let remark = frame_system::Call::::remark { remark: vec![1; weight as usize] }; + let signed_xt = TestXt::<_, ()>::new(remark, Some((0, ()))); + signed_xt + }; ExtBuilder::default().build().execute_with(|| { - let fee_details = moonbase_runtime::TransactionPayment::query_fee_details(signed_xt, 1000); + let query_fees = |weight: u128, len: u32| { + let signed_xt = generate_xt(weight); + + // TODO: this returned weight doesn't seem to follow the weight fn declared for + // remark... + // + // NOTE: another alternative would be to call pallet_transaction_payment's + // compute_actual_fee_details() which gives better control over inputs + // + // use frame_support::dispatch::GetDispatchInfo; + // let w = signed_xt.get_dispatch_info().weight; + // assert_eq!(w as u128, weight); + + moonbase_runtime::TransactionPayment::query_fee_details(signed_xt, len) + }; + + let fee_details = query_fees(1, 1000); assert_eq!( fee_details, FeeDetails { - inclusion_fee: None, - tip: 0u32.into(), + inclusion_fee: Some(InclusionFee { + base_fee: 125000000, + len_fee: 10000000000000000, + adjusted_weight_fee: 12, // TODO: why 12 (see above)? + }), + tip: 0, } ); }); From 967ab7baf137d7d81d46e43c97f7b3e2ca29b5bb Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Tue, 7 Jun 2022 16:36:08 +0000 Subject: [PATCH 10/15] Fix and also break test --- runtime/moonbase/tests/integration_test.rs | 30 +++++++++++++++++----- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/runtime/moonbase/tests/integration_test.rs b/runtime/moonbase/tests/integration_test.rs index 37283e7c549..ef8fc8993ba 100644 --- a/runtime/moonbase/tests/integration_test.rs +++ b/runtime/moonbase/tests/integration_test.rs @@ -2527,26 +2527,42 @@ fn substrate_based_fees_are_known() { // // NOTE: another alternative would be to call pallet_transaction_payment's // compute_actual_fee_details() which gives better control over inputs - // - // use frame_support::dispatch::GetDispatchInfo; - // let w = signed_xt.get_dispatch_info().weight; - // assert_eq!(w as u128, weight); moonbase_runtime::TransactionPayment::query_fee_details(signed_xt, len) }; + // TODO: derive this + let expected_base_fee = 6250000000000; + let fee_details = query_fees(1, 1000); + assert_eq!( + fee_details, + FeeDetails { + inclusion_fee: Some(InclusionFee { + base_fee: expected_base_fee, + len_fee: 1001000000000, + adjusted_weight_fee: 600000, // TODO: why 600000 (see above)? + }), + tip: 0, + } + ); + let fee_details = query_fees(10, 10000); assert_eq!( fee_details, FeeDetails { inclusion_fee: Some(InclusionFee { - base_fee: 125000000, - len_fee: 10000000000000000, - adjusted_weight_fee: 12, // TODO: why 12 (see above)? + base_fee: expected_base_fee, + len_fee: 11000000000000, + adjusted_weight_fee: 1050000, }), tip: 0, } ); + + // see notes above. this test should be able to calculate fees given runtime + // constants, that is, it should demonstrate that we know precisely how our fees + // are derived + panic!("Test fails because it doesn't demonstrate how fees are calculated"); }); } From ed002e72a32fc51ca4b817e787d5ac428d4e18d7 Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Tue, 7 Jun 2022 17:31:04 +0000 Subject: [PATCH 11/15] fmt --- runtime/moonbase/tests/integration_test.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/runtime/moonbase/tests/integration_test.rs b/runtime/moonbase/tests/integration_test.rs index ef8fc8993ba..b981cd48b1c 100644 --- a/runtime/moonbase/tests/integration_test.rs +++ b/runtime/moonbase/tests/integration_test.rs @@ -2512,13 +2512,14 @@ fn substrate_based_fees_are_known() { use sp_runtime::testing::TestXt; let generate_xt = |weight: u128| { - let remark = frame_system::Call::::remark { remark: vec![1; weight as usize] }; + let remark = frame_system::Call::::remark { + remark: vec![1; weight as usize], + }; let signed_xt = TestXt::<_, ()>::new(remark, Some((0, ()))); signed_xt }; ExtBuilder::default().build().execute_with(|| { - let query_fees = |weight: u128, len: u32| { let signed_xt = generate_xt(weight); From e4cd61a3e8abeffd57f90da5fd001cd5f0dfea86 Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Tue, 7 Jun 2022 18:14:29 +0000 Subject: [PATCH 12/15] Derive base_extrinsic in tests --- runtime/moonbase/tests/integration_test.rs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/runtime/moonbase/tests/integration_test.rs b/runtime/moonbase/tests/integration_test.rs index b981cd48b1c..11f7e12cf18 100644 --- a/runtime/moonbase/tests/integration_test.rs +++ b/runtime/moonbase/tests/integration_test.rs @@ -2520,6 +2520,9 @@ fn substrate_based_fees_are_known() { }; ExtBuilder::default().build().execute_with(|| { + use frame_support::weights::DispatchClass; + use moonbase_runtime::currency; + let query_fees = |weight: u128, len: u32| { let signed_xt = generate_xt(weight); @@ -2532,8 +2535,13 @@ fn substrate_based_fees_are_known() { moonbase_runtime::TransactionPayment::query_fee_details(signed_xt, len) }; - // TODO: derive this - let expected_base_fee = 6250000000000; + let base_extrinsic = + moonbase_runtime::BlockWeights::get() + .per_class + .get(DispatchClass::Normal) + .base_extrinsic; + let expected_base_fee = base_extrinsic as u128 * currency::WEIGHT_FEE; + assert_eq!(6250000000000, expected_base_fee); let fee_details = query_fees(1, 1000); assert_eq!( @@ -2564,6 +2572,6 @@ fn substrate_based_fees_are_known() { // see notes above. this test should be able to calculate fees given runtime // constants, that is, it should demonstrate that we know precisely how our fees // are derived - panic!("Test fails because it doesn't demonstrate how fees are calculated"); + // panic!("Test fails because it doesn't demonstrate how fees are calculated"); }); } From 4a5173e487642304c8e6d320274cd3c187d5be83 Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Tue, 7 Jun 2022 21:15:29 +0000 Subject: [PATCH 13/15] Messy test --- runtime/moonbase/tests/integration_test.rs | 55 +++++++++++++++------- 1 file changed, 38 insertions(+), 17 deletions(-) diff --git a/runtime/moonbase/tests/integration_test.rs b/runtime/moonbase/tests/integration_test.rs index 11f7e12cf18..1b3f54a3668 100644 --- a/runtime/moonbase/tests/integration_test.rs +++ b/runtime/moonbase/tests/integration_test.rs @@ -33,9 +33,9 @@ use frame_support::{ }; use moonbase_runtime::{ asset_config::AssetRegistrarMetadata, asset_config::LocalAssetInstance, get, - xcm_config::AssetType, AccountId, AssetId, AssetManager, Assets, Balances, BaseFee, - BlockWeights, Call, CrowdloanRewards, Event, LocalAssets, ParachainStaking, PolkadotXcm, - Precompiles, Runtime, System, XTokens, XcmTransactor, FOREIGN_ASSET_PRECOMPILE_ADDRESS_PREFIX, + xcm_config::AssetType, AccountId, AssetId, AssetManager, Assets, Balances, BaseFee, Call, + CrowdloanRewards, Event, LocalAssets, ParachainStaking, PolkadotXcm, Precompiles, Runtime, + System, XTokens, XcmTransactor, FOREIGN_ASSET_PRECOMPILE_ADDRESS_PREFIX, LOCAL_ASSET_PRECOMPILE_ADDRESS_PREFIX, }; @@ -2507,13 +2507,27 @@ fn base_fee_should_default_to_associate_type_value() { } #[test] -fn substrate_based_fees_are_known() { +fn substrate_based_weight_fees_are_known() { use pallet_transaction_payment::{FeeDetails, InclusionFee}; use sp_runtime::testing::TestXt; let generate_xt = |weight: u128| { + + // an empty remark has an encoded size of 12 in this case. we want to generate a txn with + // a specific weight and remark's weight is its encoded size, so we simply adjust for this + // (and don't allow a size less than 12). + // + // TODO: there may be a better way to do this (use a different pub fn from + // transaction-payment for testing?) + let known_encoded_overhead_bytes = 11; + assert!( + weight >= known_encoded_overhead_bytes, + "this test only supports weight >= {}", + known_encoded_overhead_bytes + ); + let remark = frame_system::Call::::remark { - remark: vec![1; weight as usize], + remark: vec![1; (weight - known_encoded_overhead_bytes) as usize], }; let signed_xt = TestXt::<_, ()>::new(remark, Some((0, ()))); signed_xt @@ -2535,43 +2549,50 @@ fn substrate_based_fees_are_known() { moonbase_runtime::TransactionPayment::query_fee_details(signed_xt, len) }; - let base_extrinsic = - moonbase_runtime::BlockWeights::get() + let base_extrinsic = moonbase_runtime::BlockWeights::get() .per_class .get(DispatchClass::Normal) .base_extrinsic; let expected_base_fee = base_extrinsic as u128 * currency::WEIGHT_FEE; assert_eq!(6250000000000, expected_base_fee); - let fee_details = query_fees(1, 1000); + let fee_details = query_fees(11, 0); assert_eq!( fee_details, FeeDetails { inclusion_fee: Some(InclusionFee { base_fee: expected_base_fee, - len_fee: 1001000000000, - adjusted_weight_fee: 600000, // TODO: why 600000 (see above)? + len_fee: 0, + adjusted_weight_fee: 11 * currency::WEIGHT_FEE, }), tip: 0, } ); - let fee_details = query_fees(10, 10000); + let fee_details = query_fees(12, 0); assert_eq!( fee_details, FeeDetails { inclusion_fee: Some(InclusionFee { base_fee: expected_base_fee, - len_fee: 11000000000000, - adjusted_weight_fee: 1050000, + len_fee: 0, + adjusted_weight_fee: 12 * currency::WEIGHT_FEE, }), tip: 0, } ); - // see notes above. this test should be able to calculate fees given runtime - // constants, that is, it should demonstrate that we know precisely how our fees - // are derived - // panic!("Test fails because it doesn't demonstrate how fees are calculated"); + let fee_details = query_fees(64, 0); + assert_eq!( + fee_details, + FeeDetails { + inclusion_fee: Some(InclusionFee { + base_fee: expected_base_fee, + len_fee: 0, + adjusted_weight_fee: 64 * currency::WEIGHT_FEE, + }), + tip: 0, + } + ); }); } From b497e82839c41809dcb4000d3971236e525e3bd2 Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Tue, 7 Jun 2022 21:29:34 +0000 Subject: [PATCH 14/15] fmt --- runtime/moonbase/tests/integration_test.rs | 25 +++++++++++----------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/runtime/moonbase/tests/integration_test.rs b/runtime/moonbase/tests/integration_test.rs index 1b3f54a3668..efab909d73b 100644 --- a/runtime/moonbase/tests/integration_test.rs +++ b/runtime/moonbase/tests/integration_test.rs @@ -2512,19 +2512,18 @@ fn substrate_based_weight_fees_are_known() { use sp_runtime::testing::TestXt; let generate_xt = |weight: u128| { - - // an empty remark has an encoded size of 12 in this case. we want to generate a txn with - // a specific weight and remark's weight is its encoded size, so we simply adjust for this - // (and don't allow a size less than 12). - // - // TODO: there may be a better way to do this (use a different pub fn from - // transaction-payment for testing?) - let known_encoded_overhead_bytes = 11; - assert!( - weight >= known_encoded_overhead_bytes, - "this test only supports weight >= {}", - known_encoded_overhead_bytes - ); + // an empty remark has an encoded size of 12 in this case. we want to generate a txn with + // a specific weight and remark's weight is its encoded size, so we simply adjust for this + // (and don't allow a size less than 12). + // + // TODO: there may be a better way to do this (use a different pub fn from + // transaction-payment for testing?) + let known_encoded_overhead_bytes = 11; + assert!( + weight >= known_encoded_overhead_bytes, + "this test only supports weight >= {}", + known_encoded_overhead_bytes + ); let remark = frame_system::Call::::remark { remark: vec![1; (weight - known_encoded_overhead_bytes) as usize], From 24d373f76018561ac4d3b45bd30debeacdb074e3 Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Mon, 15 Aug 2022 14:26:22 -0600 Subject: [PATCH 15/15] Restore missing import --- runtime/moonbase/tests/integration_test.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/runtime/moonbase/tests/integration_test.rs b/runtime/moonbase/tests/integration_test.rs index c66505e1649..47d128b1151 100644 --- a/runtime/moonbase/tests/integration_test.rs +++ b/runtime/moonbase/tests/integration_test.rs @@ -30,6 +30,7 @@ use frame_support::{ EnsureOrigin, PalletInfo, StorageInfo, StorageInfoTrait, }, StorageHasher, Twox128, + weights::DispatchClass, }; use moonbase_runtime::{ asset_config::AssetRegistrarMetadata, asset_config::LocalAssetInstance, get,