Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Fix the broken weight multiplier update function #6334

Merged
15 commits merged into from
Jun 17, 2020
Merged

Fix the broken weight multiplier update function #6334

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
63438d5
Initial draft, has some todos left
kianenigma Jun 11, 2020
8ee1b45
remove ununsed import
kianenigma Jun 11, 2020
340347e
Apply suggestions from code review
kianenigma Jun 11, 2020
a8e22d5
Some refactors with migration
kianenigma Jun 15, 2020
02a464d
Fix more test and cleanup
kianenigma Jun 15, 2020
71d96bd
Master.into()
kianenigma Jun 15, 2020
0eaae04
Fix for companion
kianenigma Jun 15, 2020
45912e1
Merge branch 'master' of github.com:paritytech/substrate into kiz-fix…
kianenigma Jun 16, 2020
106b6a7
Apply suggestions from code review
kianenigma Jun 17, 2020
5d27ad3
Merge branch 'master' of github.com:paritytech/substrate into kiz-fix…
kianenigma Jun 17, 2020
8fa2e7d
Update bin/node/runtime/src/impls.rs
kianenigma Jun 17, 2020
37704a6
Fix weight
kianenigma Jun 17, 2020
ad63fb3
Merge branch 'kiz-fix-tx-payment' of github.com:paritytech/substrate …
kianenigma Jun 17, 2020
9ce3281
Add integrity test
kianenigma Jun 17, 2020
0f964f9
length is not affected.
kianenigma Jun 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

55 changes: 24 additions & 31 deletions bin/node/executor/tests/basic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,11 @@ use codec::{Encode, Decode, Joiner};
use frame_support::{
StorageValue, StorageMap,
traits::Currency,
weights::{
GetDispatchInfo, DispatchInfo, DispatchClass, constants::ExtrinsicBaseWeight,
WeightToFeePolynomial,
},
weights::{GetDispatchInfo, DispatchInfo, DispatchClass},
};
use sp_core::{NeverNativeValue, traits::Externalities, storage::well_known_keys};
use sp_runtime::{
ApplyExtrinsicResult, FixedI128, FixedPointNumber,
ApplyExtrinsicResult,
traits::Hash as HashT,
transaction_validity::InvalidTransaction,
};
Expand All @@ -35,7 +32,7 @@ use frame_system::{self, EventRecord, Phase};

use node_runtime::{
Header, Block, UncheckedExtrinsic, CheckedExtrinsic, Call, Runtime, Balances,
System, TransactionPayment, Event, TransactionByteFee,
System, TransactionPayment, Event,
constants::currency::*,
};
use node_primitives::{Balance, Hash};
Expand All @@ -52,16 +49,17 @@ use self::common::{*, sign};
/// test code paths that differ between native and wasm versions.
pub const BLOATY_CODE: &[u8] = node_runtime::WASM_BINARY_BLOATY;

/// Default transfer fee
fn transfer_fee<E: Encode>(extrinsic: &E, fee_multiplier: FixedI128) -> Balance {
let length_fee = TransactionByteFee::get() * (extrinsic.encode().len() as Balance);

let base_weight = ExtrinsicBaseWeight::get();
let base_fee = <Runtime as pallet_transaction_payment::Trait>::WeightToFee::calc(&base_weight);
let weight = default_transfer_call().get_dispatch_info().weight;
let weight_fee = <Runtime as pallet_transaction_payment::Trait>::WeightToFee::calc(&weight);

base_fee + fee_multiplier.saturating_mul_acc_int(length_fee + weight_fee)
/// Default transfer fee. This will use the same logic that is implemented in transaction-payment module.
///
/// Note that reads the multiplier from storage directly, hence to get the fee of `extrinsic`
/// at block `n`, it must be called prior to executing block `n` to do the calculation with the
/// correct multiplier.
fn transfer_fee<E: Encode>(extrinsic: &E) -> Balance {
TransactionPayment::compute_fee(
extrinsic.encode().len() as u32,
&default_transfer_call().get_dispatch_info(),
0,
)
}

fn xt() -> UncheckedExtrinsic {
Expand Down Expand Up @@ -242,7 +240,7 @@ fn successful_execution_with_native_equivalent_code_gives_ok() {
).0;
assert!(r.is_ok());

let fm = t.execute_with(TransactionPayment::next_fee_multiplier);
let fees = t.execute_with(|| transfer_fee(&xt()));

let r = executor_call::<NeverNativeValue, fn() -> _>(
&mut t,
Expand All @@ -254,7 +252,6 @@ fn successful_execution_with_native_equivalent_code_gives_ok() {
assert!(r.is_ok());

t.execute_with(|| {
let fees = transfer_fee(&xt(), fm);
assert_eq!(Balances::total_balance(&alice()), 42 * DOLLARS - fees);
assert_eq!(Balances::total_balance(&bob()), 69 * DOLLARS);
});
Expand Down Expand Up @@ -286,7 +283,7 @@ fn successful_execution_with_foreign_code_gives_ok() {
).0;
assert!(r.is_ok());

let fm = t.execute_with(TransactionPayment::next_fee_multiplier);
let fees = t.execute_with(|| transfer_fee(&xt()));

let r = executor_call::<NeverNativeValue, fn() -> _>(
&mut t,
Expand All @@ -298,7 +295,6 @@ fn successful_execution_with_foreign_code_gives_ok() {
assert!(r.is_ok());

t.execute_with(|| {
let fees = transfer_fee(&xt(), fm);
assert_eq!(Balances::total_balance(&alice()), 42 * DOLLARS - fees);
assert_eq!(Balances::total_balance(&bob()), 69 * DOLLARS);
});
Expand All @@ -311,7 +307,7 @@ fn full_native_block_import_works() {
let (block1, block2) = blocks();

let mut alice_last_known_balance: Balance = Default::default();
let mut fm = t.execute_with(TransactionPayment::next_fee_multiplier);
let mut fees = t.execute_with(|| transfer_fee(&xt()));

executor_call::<NeverNativeValue, fn() -> _>(
&mut t,
Expand All @@ -322,7 +318,6 @@ fn full_native_block_import_works() {
).0.unwrap();

t.execute_with(|| {
let fees = transfer_fee(&xt(), fm);
assert_eq!(Balances::total_balance(&alice()), 42 * DOLLARS - fees);
assert_eq!(Balances::total_balance(&bob()), 169 * DOLLARS);
alice_last_known_balance = Balances::total_balance(&alice());
Expand Down Expand Up @@ -361,7 +356,7 @@ fn full_native_block_import_works() {
assert_eq!(System::events(), events);
});

fm = t.execute_with(TransactionPayment::next_fee_multiplier);
fees = t.execute_with(|| transfer_fee(&xt()));

executor_call::<NeverNativeValue, fn() -> _>(
&mut t,
Expand All @@ -372,7 +367,6 @@ fn full_native_block_import_works() {
).0.unwrap();

t.execute_with(|| {
let fees = transfer_fee(&xt(), fm);
assert_eq!(
Balances::total_balance(&alice()),
alice_last_known_balance - 10 * DOLLARS - fees,
Expand Down Expand Up @@ -450,7 +444,7 @@ fn full_wasm_block_import_works() {
let (block1, block2) = blocks();

let mut alice_last_known_balance: Balance = Default::default();
let mut fm = t.execute_with(TransactionPayment::next_fee_multiplier);
let mut fees = t.execute_with(|| transfer_fee(&xt()));

executor_call::<NeverNativeValue, fn() -> _>(
&mut t,
Expand All @@ -461,12 +455,12 @@ fn full_wasm_block_import_works() {
).0.unwrap();

t.execute_with(|| {
assert_eq!(Balances::total_balance(&alice()), 42 * DOLLARS - transfer_fee(&xt(), fm));
assert_eq!(Balances::total_balance(&alice()), 42 * DOLLARS - fees);
assert_eq!(Balances::total_balance(&bob()), 169 * DOLLARS);
alice_last_known_balance = Balances::total_balance(&alice());
});

fm = t.execute_with(TransactionPayment::next_fee_multiplier);
fees = t.execute_with(|| transfer_fee(&xt()));

executor_call::<NeverNativeValue, fn() -> _>(
&mut t,
Expand All @@ -479,11 +473,11 @@ fn full_wasm_block_import_works() {
t.execute_with(|| {
assert_eq!(
Balances::total_balance(&alice()),
alice_last_known_balance - 10 * DOLLARS - transfer_fee(&xt(), fm),
alice_last_known_balance - 10 * DOLLARS - fees,
);
assert_eq!(
Balances::total_balance(&bob()),
179 * DOLLARS - 1 * transfer_fee(&xt(), fm),
179 * DOLLARS - 1 * fees,
);
});
}
Expand Down Expand Up @@ -755,7 +749,7 @@ fn successful_execution_gives_ok() {
assert_eq!(Balances::total_balance(&alice()), 111 * DOLLARS);
});

let fm = t.execute_with(TransactionPayment::next_fee_multiplier);
let fees = t.execute_with(|| transfer_fee(&xt()));

let r = executor_call::<NeverNativeValue, fn() -> _>(
&mut t,
Expand All @@ -770,7 +764,6 @@ fn successful_execution_gives_ok() {
.expect("Extrinsic failed");

t.execute_with(|| {
let fees = transfer_fee(&xt(), fm);
assert_eq!(Balances::total_balance(&alice()), 42 * DOLLARS - fees);
assert_eq!(Balances::total_balance(&bob()), 69 * DOLLARS);
});
Expand Down
12 changes: 6 additions & 6 deletions bin/node/executor/tests/fees.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ use frame_support::{
weights::{GetDispatchInfo, constants::ExtrinsicBaseWeight, IdentityFee, WeightToFeePolynomial},
};
use sp_core::NeverNativeValue;
use sp_runtime::{FixedPointNumber, FixedI128, Perbill};
use sp_runtime::{Perbill, FixedPointNumber};
use node_runtime::{
CheckedExtrinsic, Call, Runtime, Balances, TransactionPayment,
CheckedExtrinsic, Call, Runtime, Balances, TransactionPayment, Multiplier,
TransactionByteFee,
constants::currency::*,
};
Expand All @@ -38,8 +38,8 @@ use self::common::{*, sign};
fn fee_multiplier_increases_and_decreases_on_big_weight() {
let mut t = new_test_ext(COMPACT_CODE, false);

// initial fee multiplier must be zero
let mut prev_multiplier = FixedI128::from_inner(0);
// initial fee multiplier must be one.
let mut prev_multiplier = Multiplier::one();

t.execute_with(|| {
assert_eq!(TransactionPayment::next_fee_multiplier(), prev_multiplier);
Expand All @@ -59,7 +59,7 @@ fn fee_multiplier_increases_and_decreases_on_big_weight() {
},
CheckedExtrinsic {
signed: Some((charlie(), signed_extra(0, 0))),
function: Call::System(frame_system::Call::fill_block(Perbill::from_percent(90))),
function: Call::System(frame_system::Call::fill_block(Perbill::from_percent(60))),
}
]
);
Expand Down Expand Up @@ -122,7 +122,7 @@ fn fee_multiplier_increases_and_decreases_on_big_weight() {
}

#[test]
fn transaction_fee_is_correct_ultimate() {
fn transaction_fee_is_correct() {
// This uses the exact values of substrate-node.
//
// weight of transfer call as of now: 1_000_000
Expand Down
Loading