From 04571d958b9d98e8ea2dc171b5bf3dcf65c8f09a Mon Sep 17 00:00:00 2001
From: Xiliang Chen <xlchen1291@gmail.com>
Date: Mon, 25 Nov 2019 19:42:51 +1300
Subject: [PATCH] PaysFee for DispatchInfo (#4165)
* 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
---
substrate/bin/node/executor/src/lib.rs | 10 ++--
substrate/frame/contracts/src/tests.rs | 2 +-
substrate/frame/example/src/lib.rs | 8 +++-
substrate/frame/support/src/dispatch.rs | 18 +++++---
substrate/frame/support/src/weights.rs | 37 ++++++++++-----
substrate/frame/system/src/lib.rs | 10 ++--
.../frame/transaction-payment/src/lib.rs | 46 ++++++++++---------
7 files changed, 79 insertions(+), 52 deletions(-)
diff --git a/substrate/bin/node/executor/src/lib.rs b/substrate/bin/node/executor/src/lib.rs
index e6360304d82..b1e5e24ffd3 100644
--- a/substrate/bin/node/executor/src/lib.rs
+++ b/substrate/bin/node/executor/src/lib.rs
@@ -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![],
},
@@ -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![],
},
@@ -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![],
},
@@ -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![],
},
@@ -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![],
},
diff --git a/substrate/frame/contracts/src/tests.rs b/substrate/frame/contracts/src/tests.rs
index 5532a0cc9aa..d06565ed8ec 100644
--- a/substrate/frame/contracts/src/tests.rs
+++ b/substrate/frame/contracts/src/tests.rs
@@ -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();
diff --git a/substrate/frame/example/src/lib.rs b/substrate/frame/example/src/lib.rs
index 8956de7249c..cf22d38f694 100644
--- a/substrate/frame/example/src/lib.rs
+++ b/substrate/frame/example/src/lib.rs
@@ -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};
@@ -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;
diff --git a/substrate/frame/support/src/dispatch.rs b/substrate/frame/support/src/dispatch.rs
index 025ee67dd14..cde1300831d 100644
--- a/substrate/frame/support/src/dispatch.rs
+++ b/substrate/frame/support/src/dispatch.rs
@@ -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},
@@ -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.") }
)*
@@ -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 }
}
}
@@ -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 },
);
}
diff --git a/substrate/frame/support/src/weights.rs b/substrate/frame/support/src/weights.rs
index 3a5863ee372..8af418fc740 100644
--- a/substrate/frame/support/src/weights.rs
+++ b/substrate/frame/support/src/weights.rs
@@ -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 {
@@ -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
@@ -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.
@@ -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()
}
}
}
-
diff --git a/substrate/frame/system/src/lib.rs b/substrate/frame/system/src/lib.rs
index 699ff67d2c5..611ec2ff160 100644
--- a/substrate/frame/system/src/lib.rs
+++ b/substrate/frame/system/src/lib.rs
@@ -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();
@@ -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)
@@ -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);
@@ -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),
diff --git a/substrate/frame/transaction-payment/src/lib.rs b/substrate/frame/transaction-payment/src/lib.rs
index c3fa13c0ba8..c7310ac587d 100644
--- a/substrate/frame/transaction-payment/src/lib.rs
+++ b/substrate/frame/transaction-payment/src/lib.rs
@@ -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 }
}
@@ -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
+ }
}
}
@@ -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]
@@ -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)
@@ -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)
@@ -541,4 +544,3 @@ mod tests {
}
}
-
--
GitLab