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

Commit

Permalink
PaysFee for DispatchInfo (#4165)
Browse files Browse the repository at this point in the history 
* Add PaysFee trait

* bump version

* Apply suggestions from code review

Co-Authored-By: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* line width

* Apply suggestions from code review

Co-Authored-By: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* fix test

* fix test

* fix test
  • Loading branch information
@xlc @kianenigma
xlc authored and kianenigma committed Nov 25, 2019
1 parent 605852e commit 1078691
Show file tree
Hide file tree
Showing 7 changed files with 79 additions and 52 deletions.
10 changes: 5 additions & 5 deletions bin/node/executor/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ mod tests {
EventRecord {
phase: Phase::ApplyExtrinsic(0),
event: Event::system(system::Event::ExtrinsicSuccess(
DispatchInfo { weight: 10000, class: DispatchClass::Operational }
DispatchInfo { weight: 10000, class: DispatchClass::Operational, pays_fee: true }
)),
topics: vec![],
},
Expand All @@ -489,7 +489,7 @@ mod tests {
EventRecord {
phase: Phase::ApplyExtrinsic(1),
event: Event::system(system::Event::ExtrinsicSuccess(
DispatchInfo { weight: 1000000, class: DispatchClass::Normal }
DispatchInfo { weight: 1000000, class: DispatchClass::Normal, pays_fee: true }
)),
topics: vec![],
},
Expand Down Expand Up @@ -520,7 +520,7 @@ mod tests {
EventRecord {
phase: Phase::ApplyExtrinsic(0),
event: Event::system(system::Event::ExtrinsicSuccess(
DispatchInfo { weight: 10000, class: DispatchClass::Operational }
DispatchInfo { weight: 10000, class: DispatchClass::Operational, pays_fee: true }
)),
topics: vec![],
},
Expand All @@ -544,7 +544,7 @@ mod tests {
EventRecord {
phase: Phase::ApplyExtrinsic(1),
event: Event::system(system::Event::ExtrinsicSuccess(
DispatchInfo { weight: 1000000, class: DispatchClass::Normal }
DispatchInfo { weight: 1000000, class: DispatchClass::Normal, pays_fee: true }
)),
topics: vec![],
},
Expand All @@ -568,7 +568,7 @@ mod tests {
EventRecord {
phase: Phase::ApplyExtrinsic(2),
event: Event::system(system::Event::ExtrinsicSuccess(
DispatchInfo { weight: 1000000, class: DispatchClass::Normal }
DispatchInfo { weight: 1000000, class: DispatchClass::Normal, pays_fee: true }
)),
topics: vec![],
},
Expand Down
2 changes: 1 addition & 1 deletion frame/contracts/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2329,7 +2329,7 @@ fn cannot_self_destruct_in_constructor() {
#[test]
fn check_block_gas_limit_works() {
ExtBuilder::default().block_gas_limit(50).build().execute_with(|| {
let info = DispatchInfo { weight: 100, class: DispatchClass::Normal };
let info = DispatchInfo { weight: 100, class: DispatchClass::Normal, pays_fee: true };
let check = CheckBlockGasLimit::<Test>(Default::default());
let call: Call = crate::Call::put_code(1000, vec![]).into();

Expand Down
8 changes: 7 additions & 1 deletion frame/example/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@
use rstd::marker::PhantomData;
use support::{
dispatch::Result, decl_module, decl_storage, decl_event,
weights::{SimpleDispatchInfo, DispatchInfo, DispatchClass, ClassifyDispatch, WeighData, Weight},
weights::{SimpleDispatchInfo, DispatchInfo, DispatchClass, ClassifyDispatch, WeighData, Weight, PaysFee},
};
use system::{ensure_signed, ensure_root};
use codec::{Encode, Decode};
Expand Down Expand Up @@ -301,6 +301,12 @@ impl<T: balances::Trait> ClassifyDispatch<(&BalanceOf<T>,)> for WeightForSetDumm
}
}

impl<T: balances::Trait> PaysFee for WeightForSetDummy<T> {
fn pays_fee(&self) -> bool {
true
}
}

/// A type alias for the balance type from this module's point of view.
type BalanceOf<T> = <T as balances::Trait>::Balance;

Expand Down
18 changes: 12 additions & 6 deletions frame/support/src/dispatch.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub use frame_metadata::{
};
pub use crate::weights::{
SimpleDispatchInfo, GetDispatchInfo, DispatchInfo, WeighData, ClassifyDispatch,
TransactionPriority, Weight, WeighBlock,
TransactionPriority, Weight, WeighBlock, PaysFee,
};
pub use sr_primitives::{
traits::{Dispatchable, DispatchResult, ModuleDispatchError},
Expand Down Expand Up @@ -1321,7 +1321,10 @@ macro_rules! decl_module {
&$weight,
($( $param_name, )*)
);
return $crate::dispatch::DispatchInfo { weight, class };
let pays_fee = <dyn $crate::dispatch::PaysFee>::pays_fee(
&$weight
);
return $crate::dispatch::DispatchInfo { weight, class, pays_fee };
}
if let $call_type::__PhantomItem(_, _) = self { unreachable!("__PhantomItem should never be used.") }
)*
Expand All @@ -1337,7 +1340,10 @@ macro_rules! decl_module {
&$crate::dispatch::SimpleDispatchInfo::default(),
()
);
$crate::dispatch::DispatchInfo { weight, class }
let pays_fee = <dyn $crate::dispatch::PaysFee>::pays_fee(
&$crate::dispatch::SimpleDispatchInfo::default()
);
$crate::dispatch::DispatchInfo { weight, class, pays_fee }

}
}
Expand Down Expand Up @@ -2066,17 +2072,17 @@ mod tests {
// operational.
assert_eq!(
Call::<TraitImpl>::operational().get_dispatch_info(),
DispatchInfo { weight: 5, class: DispatchClass::Operational },
DispatchInfo { weight: 5, class: DispatchClass::Operational, pays_fee: true },
);
// default weight.
assert_eq!(
Call::<TraitImpl>::aux_0().get_dispatch_info(),
DispatchInfo { weight: 10_000, class: DispatchClass::Normal },
DispatchInfo { weight: 10_000, class: DispatchClass::Normal, pays_fee: true },
);
// custom basic
assert_eq!(
Call::<TraitImpl>::aux_3().get_dispatch_info(),
DispatchInfo { weight: 3, class: DispatchClass::Normal },
DispatchInfo { weight: 3, class: DispatchClass::Normal, pays_fee: true },
);
}

Expand Down
37 changes: 25 additions & 12 deletions frame/support/src/weights.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,14 @@ pub trait WeighBlock<BlockNumber> {
fn on_finalize(_: BlockNumber) -> Weight { Zero::zero() }
}

/// Indicates if dispatch function should pay fees or not.
/// If set to false, the block resource limits are applied, yet no fee is deducted.
pub trait PaysFee {
fn pays_fee(&self) -> bool {
true
}
}

/// Maybe I can do something to remove the duplicate code here.
#[impl_for_tuples(30)]
impl<BlockNumber: Copy> WeighBlock<BlockNumber> for SingleModule {
Expand Down Expand Up @@ -135,17 +143,8 @@ pub struct DispatchInfo {
pub weight: Weight,
/// Class of this transaction.
pub class: DispatchClass,
}

impl DispatchInfo {
/// Determine if this dispatch should pay the base length-related fee or not.
pub fn pay_length_fee(&self) -> bool {
match self.class {
DispatchClass::Normal => true,
// For now we assume all operational transactions don't pay the length fee.
DispatchClass::Operational => false,
}
}
/// Does this transaction pay fees.
pub pays_fee: bool,
}

/// A `Dispatchable` function (aka transaction) that can carry some static information along with
Expand Down Expand Up @@ -209,6 +208,20 @@ impl<T> ClassifyDispatch<T> for SimpleDispatchInfo {
}
}

impl PaysFee for SimpleDispatchInfo {
fn pays_fee(&self) -> bool {
match self {
SimpleDispatchInfo::FixedNormal(_) => true,
SimpleDispatchInfo::MaxNormal => true,
SimpleDispatchInfo::FreeNormal => true,

SimpleDispatchInfo::FixedOperational(_) => true,
SimpleDispatchInfo::MaxOperational => true,
SimpleDispatchInfo::FreeOperational => false,
}
}
}

impl Default for SimpleDispatchInfo {
fn default() -> Self {
// Default weight of all transactions.
Expand Down Expand Up @@ -253,8 +266,8 @@ impl<Call: Encode, Extra: Encode> GetDispatchInfo for sr_primitives::testing::Te
// for testing: weight == size.
DispatchInfo {
weight: self.encode().len() as _,
pays_fee: true,
..Default::default()
}
}
}

10 changes: 5 additions & 5 deletions frame/system/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1413,7 +1413,7 @@ mod tests {
fn signed_ext_check_weight_works_operational_tx() {
new_test_ext().execute_with(|| {
let normal = DispatchInfo { weight: 100, ..Default::default() };
let op = DispatchInfo { weight: 100, class: DispatchClass::Operational };
let op = DispatchInfo { weight: 100, class: DispatchClass::Operational, pays_fee: true };
let len = 0_usize;
let normal_limit = normal_weight_limit();

Expand All @@ -1435,8 +1435,8 @@ mod tests {
#[test]
fn signed_ext_check_weight_priority_works() {
new_test_ext().execute_with(|| {
let normal = DispatchInfo { weight: 100, class: DispatchClass::Normal };
let op = DispatchInfo { weight: 100, class: DispatchClass::Operational };
let normal = DispatchInfo { weight: 100, class: DispatchClass::Normal, pays_fee: true };
let op = DispatchInfo { weight: 100, class: DispatchClass::Operational, pays_fee: true };
let len = 0_usize;

let priority = CheckWeight::<Test>(PhantomData)
Expand Down Expand Up @@ -1469,7 +1469,7 @@ mod tests {
reset_check_weight(normal, normal_limit + 1, true);

// Operational ones don't have this limit.
let op = DispatchInfo { weight: 0, class: DispatchClass::Operational };
let op = DispatchInfo { weight: 0, class: DispatchClass::Operational, pays_fee: true };
reset_check_weight(op, normal_limit, false);
reset_check_weight(op, normal_limit + 100, false);
reset_check_weight(op, 1024, false);
Expand All @@ -1496,7 +1496,7 @@ mod tests {
#[test]
fn signed_ext_check_era_should_change_longevity() {
new_test_ext().execute_with(|| {
let normal = DispatchInfo { weight: 100, class: DispatchClass::Normal };
let normal = DispatchInfo { weight: 100, class: DispatchClass::Normal, pays_fee: true };
let len = 0_usize;
let ext = (
CheckWeight::<Test>(PhantomData),
Expand Down
46 changes: 24 additions & 22 deletions frame/transaction-payment/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ impl<T: Trait> Module<T> {
let dispatch_info = <Extrinsic as GetDispatchInfo>::get_dispatch_info(&unchecked_extrinsic);

let partial_fee = <ChargeTransactionPayment<T>>::compute_fee(len, dispatch_info, 0u32.into());
let DispatchInfo { weight, class } = dispatch_info;
let DispatchInfo { weight, class, .. } = dispatch_info;

RuntimeDispatchInfo { weight, class, partial_fee }
}
Expand Down Expand Up @@ -154,28 +154,28 @@ impl<T: Trait + Send + Sync> ChargeTransactionPayment<T> {
where
BalanceOf<T>: Sync + Send,
{
let len_fee = if info.pay_length_fee() {
if info.pays_fee {
let len = <BalanceOf<T>>::from(len);
let base = T::TransactionBaseFee::get();
let per_byte = T::TransactionByteFee::get();
base.saturating_add(per_byte.saturating_mul(len))
} else {
Zero::zero()
};
let len_fee = base.saturating_add(per_byte.saturating_mul(len));

let weight_fee = {
// cap the weight to the maximum defined in runtime, otherwise it will be the `Bounded`
// maximum of its data type, which is not desired.
let capped_weight = info.weight.min(<T as system::Trait>::MaximumBlockWeight::get());
T::WeightToFee::convert(capped_weight)
};
let weight_fee = {
// cap the weight to the maximum defined in runtime, otherwise it will be the `Bounded`
// maximum of its data type, which is not desired.
let capped_weight = info.weight.min(<T as system::Trait>::MaximumBlockWeight::get());
T::WeightToFee::convert(capped_weight)
};

// everything except for tip
let basic_fee = len_fee.saturating_add(weight_fee);
let fee_update = NextFeeMultiplier::get();
let adjusted_fee = fee_update.saturated_multiply_accumulate(basic_fee);
// everything except for tip
let basic_fee = len_fee.saturating_add(weight_fee);
let fee_update = NextFeeMultiplier::get();
let adjusted_fee = fee_update.saturated_multiply_accumulate(basic_fee);

adjusted_fee.saturating_add(tip)
adjusted_fee.saturating_add(tip)
} else {
tip
}
}
}

Expand Down Expand Up @@ -400,7 +400,7 @@ mod tests {

/// create a transaction info struct from weight. Handy to avoid building the whole struct.
pub fn info_from_weight(w: Weight) -> DispatchInfo {
DispatchInfo { weight: w, ..Default::default() }
DispatchInfo { weight: w, pays_fee: true, ..Default::default() }
}

#[test]
Expand Down Expand Up @@ -461,12 +461,14 @@ mod tests {
// 1 ain't have a penny.
assert_eq!(Balances::free_balance(&1), 0);

let len = 100;

// like a FreeOperational
let operational_transaction = DispatchInfo {
weight: 0,
class: DispatchClass::Operational
class: DispatchClass::Operational,
pays_fee: false,
};
let len = 100;
assert!(
ChargeTransactionPayment::<Runtime>::from(0)
.validate(&1, CALL, operational_transaction , len)
Expand All @@ -476,7 +478,8 @@ mod tests {
// like a FreeNormal
let free_transaction = DispatchInfo {
weight: 0,
class: DispatchClass::Normal
class: DispatchClass::Normal,
pays_fee: true,
};
assert!(
ChargeTransactionPayment::<Runtime>::from(0)
Expand Down Expand Up @@ -541,4 +544,3 @@ mod tests {
}
}


0 comments on commit 1078691

Please sign in to comment.