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

@@ -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![],
 				},

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();
 

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;
 

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 },
 		);
 	}
 

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,
-}
+	/// Does this transaction pay fees.
-
+	pub pays_fee: bool,
-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,
-		}
-	}
 }
 
 /// 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()
 		}
 	}
 }
-

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 normal = DispatchInfo { weight: 100, class: DispatchClass::Normal, pays_fee: true };
-			let op = DispatchInfo { weight: 100, class: DispatchClass::Operational };
+			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),

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))
+			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)
+			};
+
+			// 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)
 		} else {
-			Zero::zero()
+			tip
-		};
+		}
-
-		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);
-
-		adjusted_fee.saturating_add(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 {
 	}
 }
 
-