From 74da65922bb2875bcedf66c993dbabdcebfb271e Mon Sep 17 00:00:00 2001 From: georgepisaltu Date: Fri, 9 Aug 2024 01:23:39 +0300 Subject: [PATCH] Fix weight tests Signed-off-by: georgepisaltu --- substrate/bin/node/bench/src/import.rs | 7 +- substrate/bin/node/cli/tests/basic.rs | 102 ++++++++++++++++++------- substrate/bin/node/cli/tests/fees.rs | 4 +- 3 files changed, 80 insertions(+), 33 deletions(-) diff --git a/substrate/bin/node/bench/src/import.rs b/substrate/bin/node/bench/src/import.rs index 0b972650ef5a..8334d230a8d2 100644 --- a/substrate/bin/node/bench/src/import.rs +++ b/substrate/bin/node/bench/src/import.rs @@ -121,10 +121,10 @@ impl core::Benchmark for ImportBenchmark { .inspect_state(|| { match self.block_type { BlockType::RandomTransfersKeepAlive => { - // should be 9 per signed extrinsic + 1 per unsigned + // should be 10 per signed extrinsic + 1 per unsigned // we have 2 unsigned (timestamp and glutton bloat) while the rest are // signed in the block. - // those 9 events per signed are: + // those 10 events per signed are: // - transaction paid for the transaction payment // - withdraw (Balances::Withdraw) for charging the transaction fee // - new account (System::NewAccount) as we always transfer fund to @@ -132,12 +132,13 @@ impl core::Benchmark for ImportBenchmark { // - endowed (Balances::Endowed) for this new account // - issued (Balances::Issued) as total issuance is increased // - successful transfer (Event::Transfer) for this transfer operation + // - deposit (Balances::Deposit) for depositing the tx fee refund // - 2x deposit (Balances::Deposit and Treasury::Deposit) for depositing // the transaction fee into the treasury // - extrinsic success assert_eq!( kitchensink_runtime::System::events().len(), - (self.block.extrinsics.len() - 2) * 9 + 2, + (self.block.extrinsics.len() - 2) * 10 + 2, ); }, BlockType::Noop => { diff --git a/substrate/bin/node/cli/tests/basic.rs b/substrate/bin/node/cli/tests/basic.rs index 8735b84288c2..16ee2e073773 100644 --- a/substrate/bin/node/cli/tests/basic.rs +++ b/substrate/bin/node/cli/tests/basic.rs @@ -67,6 +67,14 @@ fn transfer_fee(extrinsic: &E) -> Balance { TransactionPayment::compute_fee(extrinsic.encode().len() as u32, &info, 0) } +/// Default transfer fee, same as `transfer_fee`, but with a weight refund factored in. +fn transfer_fee_with_refund(extrinsic: &E, weight_refund: Weight) -> Balance { + let mut info = default_transfer_call().get_dispatch_info(); + info.extension_weight = TxExtension::weight(); + let post_info = (Some(info.total_weight().saturating_sub(weight_refund)), info.pays_fee).into(); + TransactionPayment::compute_actual_fee(extrinsic.encode().len() as u32, &info, &post_info, 0) +} + fn xt() -> UncheckedExtrinsic { sign(CheckedExtrinsic { format: sp_runtime::generic::ExtrinsicFormat::Signed(alice(), tx_ext(0, 0)), @@ -257,13 +265,14 @@ fn successful_execution_with_native_equivalent_code_gives_ok() { let r = executor_call(&mut t, "Core_initialize_block", &vec![].and(&from_block_number(1u32))).0; assert!(r.is_ok()); - let fees = t.execute_with(|| transfer_fee(&xt())); + let weight_refund = pallet_asset_conversion_tx_payment::ChargeAssetTxPayment::::weight().saturating_sub(<::WeightInfo as pallet_asset_conversion_tx_payment::WeightInfo>::charge_asset_tx_payment_native()); + let fees_after_refund = t.execute_with(|| transfer_fee_with_refund(&xt(), weight_refund)); let r = executor_call(&mut t, "BlockBuilder_apply_extrinsic", &vec![].and(&xt())).0; assert!(r.is_ok()); t.execute_with(|| { - assert_eq!(Balances::total_balance(&alice()), 42 * DOLLARS - fees); + assert_eq!(Balances::total_balance(&alice()), 42 * DOLLARS - fees_after_refund); assert_eq!(Balances::total_balance(&bob()), 69 * DOLLARS); }); } @@ -297,13 +306,14 @@ fn successful_execution_with_foreign_code_gives_ok() { let r = executor_call(&mut t, "Core_initialize_block", &vec![].and(&from_block_number(1u32))).0; assert!(r.is_ok()); - let fees = t.execute_with(|| transfer_fee(&xt())); + let weight_refund = pallet_asset_conversion_tx_payment::ChargeAssetTxPayment::::weight().saturating_sub(<::WeightInfo as pallet_asset_conversion_tx_payment::WeightInfo>::charge_asset_tx_payment_native()); + let fees_after_refund = t.execute_with(|| transfer_fee_with_refund(&xt(), weight_refund)); let r = executor_call(&mut t, "BlockBuilder_apply_extrinsic", &vec![].and(&xt())).0; assert!(r.is_ok()); t.execute_with(|| { - assert_eq!(Balances::total_balance(&alice()), 42 * DOLLARS - fees); + assert_eq!(Balances::total_balance(&alice()), 42 * DOLLARS - fees_after_refund); assert_eq!(Balances::total_balance(&bob()), 69 * DOLLARS); }); } @@ -316,6 +326,8 @@ fn full_native_block_import_works() { let mut alice_last_known_balance: Balance = Default::default(); let mut fees = t.execute_with(|| transfer_fee(&xt())); + let weight_refund = pallet_asset_conversion_tx_payment::ChargeAssetTxPayment::::weight().saturating_sub(<::WeightInfo as pallet_asset_conversion_tx_payment::WeightInfo>::charge_asset_tx_payment_native()); + let fees_after_refund = t.execute_with(|| transfer_fee_with_refund(&xt(), weight_refund)); let transfer_weight = default_transfer_call().get_dispatch_info().call_weight.saturating_add( ::BlockWeights::get() @@ -334,7 +346,7 @@ fn full_native_block_import_works() { executor_call(&mut t, "Core_execute_block", &block1.0).0.unwrap(); t.execute_with(|| { - assert_eq!(Balances::total_balance(&alice()), 42 * DOLLARS - fees); + assert_eq!(Balances::total_balance(&alice()), 42 * DOLLARS - fees_after_refund); assert_eq!(Balances::total_balance(&bob()), 169 * DOLLARS); alice_last_known_balance = Balances::total_balance(&alice()); let events = vec![ @@ -364,25 +376,33 @@ fn full_native_block_import_works() { }), topics: vec![], }, + EventRecord { + phase: Phase::ApplyExtrinsic(1), + event: RuntimeEvent::Balances(pallet_balances::Event::Deposit { + who: alice().into(), + amount: fees - fees_after_refund, + }), + topics: vec![], + }, EventRecord { phase: Phase::ApplyExtrinsic(1), event: RuntimeEvent::Balances(pallet_balances::Event::Deposit { who: pallet_treasury::Pallet::::account_id(), - amount: fees * 8 / 10, + amount: fees_after_refund * 8 / 10, }), topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(1), event: RuntimeEvent::Treasury(pallet_treasury::Event::Deposit { - value: fees * 8 / 10, + value: fees_after_refund * 8 / 10, }), topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(1), event: RuntimeEvent::Balances(pallet_balances::Event::Rescinded { - amount: fees * 2 / 10, + amount: fees_after_refund * 2 / 10, }), topics: vec![], }, @@ -391,7 +411,7 @@ fn full_native_block_import_works() { event: RuntimeEvent::TransactionPayment( pallet_transaction_payment::Event::TransactionFeePaid { who: alice().into(), - actual_fee: fees, + actual_fee: fees_after_refund, tip: 0, }, ), @@ -400,7 +420,8 @@ fn full_native_block_import_works() { EventRecord { phase: Phase::ApplyExtrinsic(1), event: RuntimeEvent::System(frame_system::Event::ExtrinsicSuccess { - weight: transfer_weight.saturating_add(TxExtension::weight()), + weight: transfer_weight + .saturating_add(TxExtension::weight().saturating_sub(weight_refund)), class: Default::default(), pays: Default::default(), }), @@ -412,15 +433,17 @@ fn full_native_block_import_works() { fees = t.execute_with(|| transfer_fee(&xt())); let pot = t.execute_with(|| Treasury::pot()); + let weight_refund = pallet_asset_conversion_tx_payment::ChargeAssetTxPayment::::weight().saturating_sub(<::WeightInfo as pallet_asset_conversion_tx_payment::WeightInfo>::charge_asset_tx_payment_native()); + let fees_after_refund = t.execute_with(|| transfer_fee_with_refund(&xt(), weight_refund)); executor_call(&mut t, "Core_execute_block", &block2.0).0.unwrap(); t.execute_with(|| { assert_eq!( Balances::total_balance(&alice()), - alice_last_known_balance - 10 * DOLLARS - fees, + alice_last_known_balance - 10 * DOLLARS - fees_after_refund, ); - assert_eq!(Balances::total_balance(&bob()), 179 * DOLLARS - fees); + assert_eq!(Balances::total_balance(&bob()), 179 * DOLLARS - fees_after_refund); let events = vec![ EventRecord { phase: Phase::Initialization, @@ -456,25 +479,33 @@ fn full_native_block_import_works() { }), topics: vec![], }, + EventRecord { + phase: Phase::ApplyExtrinsic(1), + event: RuntimeEvent::Balances(pallet_balances::Event::Deposit { + who: bob().into(), + amount: fees - fees_after_refund, + }), + topics: vec![], + }, EventRecord { phase: Phase::ApplyExtrinsic(1), event: RuntimeEvent::Balances(pallet_balances::Event::Deposit { who: pallet_treasury::Pallet::::account_id(), - amount: fees * 8 / 10, + amount: fees_after_refund * 8 / 10, }), topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(1), event: RuntimeEvent::Treasury(pallet_treasury::Event::Deposit { - value: fees * 8 / 10, + value: fees_after_refund * 8 / 10, }), topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(1), event: RuntimeEvent::Balances(pallet_balances::Event::Rescinded { - amount: fees - fees * 8 / 10, + amount: fees_after_refund - fees_after_refund * 8 / 10, }), topics: vec![], }, @@ -483,7 +514,7 @@ fn full_native_block_import_works() { event: RuntimeEvent::TransactionPayment( pallet_transaction_payment::Event::TransactionFeePaid { who: bob().into(), - actual_fee: fees, + actual_fee: fees_after_refund, tip: 0, }, ), @@ -492,7 +523,8 @@ fn full_native_block_import_works() { EventRecord { phase: Phase::ApplyExtrinsic(1), event: RuntimeEvent::System(frame_system::Event::ExtrinsicSuccess { - weight: transfer_weight.saturating_add(TxExtension::weight()), + weight: transfer_weight + .saturating_add(TxExtension::weight().saturating_sub(weight_refund)), class: Default::default(), pays: Default::default(), }), @@ -515,25 +547,33 @@ fn full_native_block_import_works() { }), topics: vec![], }, + EventRecord { + phase: Phase::ApplyExtrinsic(2), + event: RuntimeEvent::Balances(pallet_balances::Event::Deposit { + who: alice().into(), + amount: fees - fees_after_refund, + }), + topics: vec![], + }, EventRecord { phase: Phase::ApplyExtrinsic(2), event: RuntimeEvent::Balances(pallet_balances::Event::Deposit { who: pallet_treasury::Pallet::::account_id(), - amount: fees * 8 / 10, + amount: fees_after_refund * 8 / 10, }), topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(2), event: RuntimeEvent::Treasury(pallet_treasury::Event::Deposit { - value: fees * 8 / 10, + value: fees_after_refund * 8 / 10, }), topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(2), event: RuntimeEvent::Balances(pallet_balances::Event::Rescinded { - amount: fees - fees * 8 / 10, + amount: fees_after_refund - fees_after_refund * 8 / 10, }), topics: vec![], }, @@ -542,7 +582,7 @@ fn full_native_block_import_works() { event: RuntimeEvent::TransactionPayment( pallet_transaction_payment::Event::TransactionFeePaid { who: alice().into(), - actual_fee: fees, + actual_fee: fees_after_refund, tip: 0, }, ), @@ -551,7 +591,8 @@ fn full_native_block_import_works() { EventRecord { phase: Phase::ApplyExtrinsic(2), event: RuntimeEvent::System(frame_system::Event::ExtrinsicSuccess { - weight: transfer_weight.saturating_add(TxExtension::weight()), + weight: transfer_weight + .saturating_add(TxExtension::weight().saturating_sub(weight_refund)), class: Default::default(), pays: Default::default(), }), @@ -569,26 +610,28 @@ fn full_wasm_block_import_works() { let (block1, block2) = blocks(); let mut alice_last_known_balance: Balance = Default::default(); - let mut fees = t.execute_with(|| transfer_fee(&xt())); + let weight_refund = pallet_asset_conversion_tx_payment::ChargeAssetTxPayment::::weight().saturating_sub(<::WeightInfo as pallet_asset_conversion_tx_payment::WeightInfo>::charge_asset_tx_payment_native()); + let fees_after_refund = t.execute_with(|| transfer_fee_with_refund(&xt(), weight_refund)); executor_call(&mut t, "Core_execute_block", &block1.0).0.unwrap(); t.execute_with(|| { - assert_eq!(Balances::total_balance(&alice()), 42 * DOLLARS - fees); + assert_eq!(Balances::total_balance(&alice()), 42 * DOLLARS - fees_after_refund); assert_eq!(Balances::total_balance(&bob()), 169 * DOLLARS); alice_last_known_balance = Balances::total_balance(&alice()); }); - fees = t.execute_with(|| transfer_fee(&xt())); + let weight_refund = pallet_asset_conversion_tx_payment::ChargeAssetTxPayment::::weight().saturating_sub(<::WeightInfo as pallet_asset_conversion_tx_payment::WeightInfo>::charge_asset_tx_payment_native()); + let fees_after_refund = t.execute_with(|| transfer_fee_with_refund(&xt(), weight_refund)); executor_call(&mut t, "Core_execute_block", &block2.0).0.unwrap(); t.execute_with(|| { assert_eq!( Balances::total_balance(&alice()), - alice_last_known_balance - 10 * DOLLARS - fees, + alice_last_known_balance - 10 * DOLLARS - fees_after_refund, ); - assert_eq!(Balances::total_balance(&bob()), 179 * DOLLARS - 1 * fees); + assert_eq!(Balances::total_balance(&bob()), 179 * DOLLARS - 1 * fees_after_refund); }); } @@ -830,7 +873,8 @@ fn successful_execution_gives_ok() { assert_eq!(Balances::total_balance(&alice()), 111 * DOLLARS); }); - let fees = t.execute_with(|| transfer_fee(&xt())); + let weight_refund = pallet_asset_conversion_tx_payment::ChargeAssetTxPayment::::weight().saturating_sub(<::WeightInfo as pallet_asset_conversion_tx_payment::WeightInfo>::charge_asset_tx_payment_native()); + let fees_after_refund = t.execute_with(|| transfer_fee_with_refund(&xt(), weight_refund)); let r = executor_call(&mut t, "BlockBuilder_apply_extrinsic", &vec![].and(&xt())) .0 @@ -841,7 +885,7 @@ fn successful_execution_gives_ok() { .expect("Extrinsic failed"); t.execute_with(|| { - assert_eq!(Balances::total_balance(&alice()), 42 * DOLLARS - fees); + assert_eq!(Balances::total_balance(&alice()), 42 * DOLLARS - fees_after_refund); assert_eq!(Balances::total_balance(&bob()), 69 * DOLLARS); }); } diff --git a/substrate/bin/node/cli/tests/fees.rs b/substrate/bin/node/cli/tests/fees.rs index d27c9c2ffd5c..906f35b94afb 100644 --- a/substrate/bin/node/cli/tests/fees.rs +++ b/substrate/bin/node/cli/tests/fees.rs @@ -179,7 +179,9 @@ fn transaction_fee_is_correct() { let mut info = default_transfer_call().get_dispatch_info(); info.extension_weight = TxExtension::weight(); - let weight = info.total_weight(); + let mut weight = info.total_weight(); + let weight_refund = pallet_asset_conversion_tx_payment::ChargeAssetTxPayment::::weight().saturating_sub(<::WeightInfo as pallet_asset_conversion_tx_payment::WeightInfo>::charge_asset_tx_payment_native()); + weight.saturating_reduce(weight_refund); let weight_fee = IdentityFee::::weight_to_fee(&weight); // we know that weight to fee multiplier is effect-less in block 1.