Skip to content

Commit

Permalink
Fix weight tests
Browse files Browse the repository at this point in the history 
Signed-off-by: georgepisaltu <[email protected]>
  • Loading branch information
@georgepisaltu
georgepisaltu committed Aug 8, 2024
1 parent 9a4ffcc commit 74da659
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 33 deletions.
7 changes: 4 additions & 3 deletions substrate/bin/node/bench/src/import.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,23 +121,24 @@ 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
// non-existent account
// - 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 => {
Expand Down
102 changes: 73 additions & 29 deletions substrate/bin/node/cli/tests/basic.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,14 @@ fn transfer_fee<E: Encode>(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<E: Encode>(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)),
Expand Down Expand Up @@ -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::<Runtime>::weight().saturating_sub(<<Runtime as pallet_asset_conversion_tx_payment::Config>::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);
});
}
Expand Down Expand Up @@ -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::<Runtime>::weight().saturating_sub(<<Runtime as pallet_asset_conversion_tx_payment::Config>::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);
});
}
Expand All @@ -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::<Runtime>::weight().saturating_sub(<<Runtime as pallet_asset_conversion_tx_payment::Config>::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(
<Runtime as frame_system::Config>::BlockWeights::get()
Expand All @@ -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![
Expand Down Expand Up @@ -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::<Runtime>::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![],
},
Expand All @@ -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,
},
),
Expand All @@ -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(),
}),
Expand All @@ -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::<Runtime>::weight().saturating_sub(<<Runtime as pallet_asset_conversion_tx_payment::Config>::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,
Expand Down Expand Up @@ -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::<Runtime>::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![],
},
Expand All @@ -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,
},
),
Expand All @@ -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(),
}),
Expand All @@ -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::<Runtime>::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![],
},
Expand All @@ -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,
},
),
Expand All @@ -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(),
}),
Expand All @@ -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::<Runtime>::weight().saturating_sub(<<Runtime as pallet_asset_conversion_tx_payment::Config>::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::<Runtime>::weight().saturating_sub(<<Runtime as pallet_asset_conversion_tx_payment::Config>::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);
});
}

Expand Down Expand Up @@ -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::<Runtime>::weight().saturating_sub(<<Runtime as pallet_asset_conversion_tx_payment::Config>::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
Expand All @@ -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);
});
}
Expand Down
4 changes: 3 additions & 1 deletion substrate/bin/node/cli/tests/fees.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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::<Runtime>::weight().saturating_sub(<<Runtime as pallet_asset_conversion_tx_payment::Config>::WeightInfo as pallet_asset_conversion_tx_payment::WeightInfo>::charge_asset_tx_payment_native());
weight.saturating_reduce(weight_refund);
let weight_fee = IdentityFee::<Balance>::weight_to_fee(&weight);

// we know that weight to fee multiplier is effect-less in block 1.
Expand Down

0 comments on commit 74da659

Please sign in to comment.