Adds syntax for marking calls feeless (#1926)

Fixes #1725 This PR adds the following changes: 1. An attribute `pallet::feeless_if` that can be optionally attached to a call like so: ```rust #[pallet::feeless_if(|_origin: &OriginFor<T>, something: &u32| -> bool { *something == 0 })] pub fn do_something(origin: OriginFor<T>, something: u32) -> DispatchResult { .... } ``` The closure passed accepts references to arguments as specified in the call fn. It returns a boolean that denotes the conditions required for this call to be "feeless". 2. A signed extension `SkipCheckIfFeeless<T: SignedExtension>` that wraps a transaction payment processor such as `pallet_transaction_payment::ChargeTransactionPayment`. It checks for all calls annotated with `pallet::feeless_if` to see if the conditions are met. If so, the wrapped signed extension is not called, essentially making the call feeless. In order to use this, you can simply replace your existing signed extension that manages transaction payment like so: ```diff - pallet_transaction_payment::ChargeTransactionPayment<Runtime>, + pallet_skip_feeless_payment::SkipCheckIfFeeless< + Runtime, + pallet_transaction_payment::ChargeTransactionPayment<Runtime>, + >, ``` ### Todo - [x] Tests - [x] Docs - [x] Prdoc --------- Co-authored-by: Nikhil Gupta <> Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: Francisco Aguirre <franciscoaguirreperez@gmail.com> Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>
paritytech · Nov 13, 2023 · 60c77a2 · 60c77a2
1 parent ebcf0a0
commit 60c77a2
Show file tree

Hide file tree

Showing 33 changed files with 874 additions and 54 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -382,6 +382,7 @@ members = [
 	"substrate/frame/transaction-payment/asset-tx-payment",
 	"substrate/frame/transaction-payment/rpc",
 	"substrate/frame/transaction-payment/rpc/runtime-api",
+	"substrate/frame/transaction-payment/skip-feeless-payment",
 	"substrate/frame/transaction-storage",
 	"substrate/frame/treasury",
 	"substrate/frame/try-runtime",

diff --git a/prdoc/pr_1926.prdoc b/prdoc/pr_1926.prdoc
@@ -0,0 +1,30 @@
+title: Adds syntax for marking calls feeless
+
+doc:
+  - audience: Core Dev
+    description: |
+      1. Adds an attribute `#[pallet::feeless_if]` that can be optionally attached to a `pallet::call`.
+      2. Adds a signed extension SkipCheckIfFeeless<T: SignedExtension> that wraps a transaction 
+      payment processor to potentially skip payment fees for such calls.
+      Note that both the attribute and the signed extension are needed to make the call feeless.
+
+migrations:
+  db: []
+
+  runtime: []
+
+crates:
+  - name: "frame-support-procedural"
+    semver: minor
+  - name: "pallet-skip-feeless-payment"
+    semver: major
+  - pallet-example-kitchensink
+    semver: patch
+  - kitchensink-runtime
+    semver: major
+  - node-testing
+    semver: patch
+  - node-cli
+    semver: patch
+
+host_functions: []
diff --git a/substrate/bin/node/cli/Cargo.toml b/substrate/bin/node/cli/Cargo.toml
@@ -96,6 +96,7 @@ pallet-assets = { path = "../../../frame/assets" }
 pallet-asset-conversion-tx-payment = { path = "../../../frame/transaction-payment/asset-conversion-tx-payment" }
 pallet-asset-tx-payment = { path = "../../../frame/transaction-payment/asset-tx-payment" }
 pallet-im-online = { path = "../../../frame/im-online", default-features = false}
+pallet-skip-feeless-payment = { path = "../../../frame/transaction-payment/skip-feeless-payment", default-features = false}
 
 # node-specific dependencies
 kitchensink-runtime = { path = "../runtime" }
@@ -168,6 +169,7 @@ runtime-benchmarks = [
 	"pallet-assets/runtime-benchmarks",
 	"pallet-balances/runtime-benchmarks",
 	"pallet-im-online/runtime-benchmarks",
+	"pallet-skip-feeless-payment/runtime-benchmarks",
 	"pallet-timestamp/runtime-benchmarks",
 	"sc-client-db/runtime-benchmarks",
 	"sc-service/runtime-benchmarks",
@@ -183,6 +185,7 @@ try-runtime = [
 	"pallet-assets/try-runtime",
 	"pallet-balances/try-runtime",
 	"pallet-im-online/try-runtime",
+	"pallet-skip-feeless-payment/try-runtime",
 	"pallet-timestamp/try-runtime",
 	"sp-runtime/try-runtime",
 	"substrate-cli-test-utils/try-runtime",

diff --git a/substrate/bin/node/cli/src/service.rs b/substrate/bin/node/cli/src/service.rs
@@ -91,21 +91,24 @@ pub fn create_extrinsic(
 		.map(|c| c / 2)
 		.unwrap_or(2) as u64;
 	let tip = 0;
-	let extra: kitchensink_runtime::SignedExtra = (
-		frame_system::CheckNonZeroSender::<kitchensink_runtime::Runtime>::new(),
-		frame_system::CheckSpecVersion::<kitchensink_runtime::Runtime>::new(),
-		frame_system::CheckTxVersion::<kitchensink_runtime::Runtime>::new(),
-		frame_system::CheckGenesis::<kitchensink_runtime::Runtime>::new(),
-		frame_system::CheckEra::<kitchensink_runtime::Runtime>::from(generic::Era::mortal(
-			period,
-			best_block.saturated_into(),
-		)),
-		frame_system::CheckNonce::<kitchensink_runtime::Runtime>::from(nonce),
-		frame_system::CheckWeight::<kitchensink_runtime::Runtime>::new(),
-		pallet_asset_conversion_tx_payment::ChargeAssetTxPayment::<kitchensink_runtime::Runtime>::from(
-			tip, None,
-		),
-	);
+	let extra: kitchensink_runtime::SignedExtra =
+		(
+			frame_system::CheckNonZeroSender::<kitchensink_runtime::Runtime>::new(),
+			frame_system::CheckSpecVersion::<kitchensink_runtime::Runtime>::new(),
+			frame_system::CheckTxVersion::<kitchensink_runtime::Runtime>::new(),
+			frame_system::CheckGenesis::<kitchensink_runtime::Runtime>::new(),
+			frame_system::CheckEra::<kitchensink_runtime::Runtime>::from(generic::Era::mortal(
+				period,
+				best_block.saturated_into(),
+			)),
+			frame_system::CheckNonce::<kitchensink_runtime::Runtime>::from(nonce),
+			frame_system::CheckWeight::<kitchensink_runtime::Runtime>::new(),
+			pallet_skip_feeless_payment::SkipCheckIfFeeless::from(
+				pallet_asset_conversion_tx_payment::ChargeAssetTxPayment::<
+					kitchensink_runtime::Runtime,
+				>::from(tip, None),
+			),
+		);
 
 	let raw_payload = kitchensink_runtime::SignedPayload::from_raw(
 		function.clone(),
@@ -879,8 +882,9 @@ mod tests {
 				let check_era = frame_system::CheckEra::from(Era::Immortal);
 				let check_nonce = frame_system::CheckNonce::from(index);
 				let check_weight = frame_system::CheckWeight::new();
-				let tx_payment =
-					pallet_asset_conversion_tx_payment::ChargeAssetTxPayment::from(0, None);
+				let tx_payment = pallet_skip_feeless_payment::SkipCheckIfFeeless::from(
+					pallet_asset_conversion_tx_payment::ChargeAssetTxPayment::from(0, None),
+				);
 				let extra = (
 					check_non_zero_sender,
 					check_spec_version,

diff --git a/substrate/bin/node/runtime/Cargo.toml b/substrate/bin/node/runtime/Cargo.toml
@@ -129,6 +129,7 @@ pallet-transaction-payment = { path = "../../../frame/transaction-payment", defa
 pallet-transaction-payment-rpc-runtime-api = { path = "../../../frame/transaction-payment/rpc/runtime-api", default-features = false}
 pallet-asset-conversion-tx-payment = { path = "../../../frame/transaction-payment/asset-conversion-tx-payment", default-features = false}
 pallet-asset-tx-payment = { path = "../../../frame/transaction-payment/asset-tx-payment", default-features = false}
+pallet-skip-feeless-payment = { path = "../../../frame/transaction-payment/skip-feeless-payment", default-features = false}
 pallet-transaction-storage = { path = "../../../frame/transaction-storage", default-features = false}
 pallet-uniques = { path = "../../../frame/uniques", default-features = false}
 pallet-vesting = { path = "../../../frame/vesting", default-features = false}
@@ -212,6 +213,7 @@ std = [
 	"pallet-scheduler/std",
 	"pallet-session-benchmarking?/std",
 	"pallet-session/std",
+	"pallet-skip-feeless-payment/std",
 	"pallet-society/std",
 	"pallet-staking-runtime-api/std",
 	"pallet-staking/std",
@@ -308,6 +310,7 @@ runtime-benchmarks = [
 	"pallet-salary/runtime-benchmarks",
 	"pallet-scheduler/runtime-benchmarks",
 	"pallet-session-benchmarking/runtime-benchmarks",
+	"pallet-skip-feeless-payment/runtime-benchmarks",
 	"pallet-society/runtime-benchmarks",
 	"pallet-staking/runtime-benchmarks",
 	"pallet-state-trie-migration/runtime-benchmarks",
@@ -381,6 +384,7 @@ try-runtime = [
 	"pallet-salary/try-runtime",
 	"pallet-scheduler/try-runtime",
 	"pallet-session/try-runtime",
+	"pallet-skip-feeless-payment/try-runtime",
 	"pallet-society/try-runtime",
 	"pallet-staking/try-runtime",
 	"pallet-state-trie-migration/try-runtime",

diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs
@@ -569,6 +569,10 @@ impl pallet_asset_conversion_tx_payment::Config for Runtime {
 		pallet_asset_conversion_tx_payment::AssetConversionAdapter<Balances, AssetConversion>;
 }
 
+impl pallet_skip_feeless_payment::Config for Runtime {
+	type RuntimeEvent = RuntimeEvent;
+}
+
 parameter_types! {
 	pub const MinimumPeriod: Moment = SLOT_DURATION / 2;
 }
@@ -1394,7 +1398,11 @@ where
 			frame_system::CheckEra::<Runtime>::from(era),
 			frame_system::CheckNonce::<Runtime>::from(nonce),
 			frame_system::CheckWeight::<Runtime>::new(),
-			pallet_asset_conversion_tx_payment::ChargeAssetTxPayment::<Runtime>::from(tip, None),
+			pallet_skip_feeless_payment::SkipCheckIfFeeless::from(
+				pallet_asset_conversion_tx_payment::ChargeAssetTxPayment::<Runtime>::from(
+					tip, None,
+				),
+			),
 		);
 		let raw_payload = SignedPayload::new(call, extra)
 			.map_err(|e| {
@@ -2134,6 +2142,7 @@ construct_runtime!(
 		Statement: pallet_statement,
 		Broker: pallet_broker,
 		Mixnet: pallet_mixnet,
+		SkipFeelessPayment: pallet_skip_feeless_payment,
 	}
 );
 
@@ -2160,7 +2169,10 @@ pub type SignedExtra = (
 	frame_system::CheckEra<Runtime>,
 	frame_system::CheckNonce<Runtime>,
 	frame_system::CheckWeight<Runtime>,
-	pallet_asset_conversion_tx_payment::ChargeAssetTxPayment<Runtime>,
+	pallet_skip_feeless_payment::SkipCheckIfFeeless<
+		Runtime,
+		pallet_asset_conversion_tx_payment::ChargeAssetTxPayment<Runtime>,
+	>,
 );
 
 /// Unchecked extrinsic type as expected by this runtime.

diff --git a/substrate/bin/node/testing/Cargo.toml b/substrate/bin/node/testing/Cargo.toml
@@ -26,6 +26,7 @@ pallet-asset-conversion = { path = "../../../frame/asset-conversion" }
 pallet-assets = { path = "../../../frame/assets" }
 pallet-asset-conversion-tx-payment = { path = "../../../frame/transaction-payment/asset-conversion-tx-payment" }
 pallet-asset-tx-payment = { path = "../../../frame/transaction-payment/asset-tx-payment" }
+pallet-skip-feeless-payment = { path = "../../../frame/transaction-payment/skip-feeless-payment" }
 sc-block-builder = { path = "../../../client/block-builder" }
 sc-client-api = { path = "../../../client/api" }
 sc-client-db = { path = "../../../client/db", features = ["rocksdb"]}

diff --git a/substrate/bin/node/testing/src/keyring.rs b/substrate/bin/node/testing/src/keyring.rs
@@ -78,7 +78,9 @@ pub fn signed_extra(nonce: Nonce, extra_fee: Balance) -> SignedExtra {
 		frame_system::CheckEra::from(Era::mortal(256, 0)),
 		frame_system::CheckNonce::from(nonce),
 		frame_system::CheckWeight::new(),
-		pallet_asset_conversion_tx_payment::ChargeAssetTxPayment::from(extra_fee, None),
+		pallet_skip_feeless_payment::SkipCheckIfFeeless::from(
+			pallet_asset_conversion_tx_payment::ChargeAssetTxPayment::from(extra_fee, None),
+		),
 	)
 }
 

diff --git a/substrate/frame/examples/kitchensink/src/lib.rs b/substrate/frame/examples/kitchensink/src/lib.rs
@@ -206,6 +206,10 @@ pub mod pallet {
 	impl<T: Config> Pallet<T> {
 		#[pallet::call_index(0)]
 		#[pallet::weight(T::WeightInfo::set_foo_benchmark())]
+		/// Marks this call as feeless if `new_foo` is zero.
+		#[pallet::feeless_if(|_origin: &OriginFor<T>, new_foo: &u32, _other_compact: &u128| -> bool {
+			*new_foo == 0
+		})]
 		pub fn set_foo(
 			_: OriginFor<T>,
 			new_foo: u32,

diff --git a/substrate/frame/support/procedural/src/construct_runtime/expand/call.rs b/substrate/frame/support/procedural/src/construct_runtime/expand/call.rs
@@ -124,6 +124,18 @@ pub fn expand_outer_dispatch(
 			}
 		}
 
+		impl #scrate::dispatch::CheckIfFeeless for RuntimeCall {
+			type Origin = #system_path::pallet_prelude::OriginFor<#runtime>;
+			fn is_feeless(&self, origin: &Self::Origin) -> bool {
+				match self {
+					#(
+						#pallet_attrs
+						#variant_patterns => call.is_feeless(origin),
+					)*
+				}
+			}
+		}
+
 		impl #scrate::traits::GetCallMetadata for RuntimeCall {
 			fn get_call_metadata(&self) -> #scrate::traits::CallMetadata {
 				use #scrate::traits::GetCallName;

diff --git a/substrate/frame/support/procedural/src/lib.rs b/substrate/frame/support/procedural/src/lib.rs
@@ -1157,6 +1157,36 @@ pub fn call_index(_: TokenStream, _: TokenStream) -> TokenStream {
 	pallet_macro_stub()
 }
 
+/// Each dispatchable may be annotated with the `#[pallet::feeless_if($closure)]` attribute,
+/// which explicitly defines the condition for the dispatchable to be feeless.
+///
+/// The arguments for the closure must be the referenced arguments of the dispatchable function.
+///
+/// The closure must return `bool`.
+///
+/// ### Example
+/// ```ignore
+/// #[pallet::feeless_if(|_origin: &OriginFor<T>, something: &u32| -> bool {
+/// 		*something == 0
+/// 	})]
+/// pub fn do_something(origin: OriginFor<T>, something: u32) -> DispatchResult {
+///     ....
+/// }
+/// ```
+///
+/// Please note that this only works for signed dispatchables and requires a signed extension
+/// such as `SkipCheckIfFeeless` as defined in `pallet-skip-feeless-payment` to wrap the existing
+/// payment extension. Else, this is completely ignored and the dispatchable is still charged.
+///
+/// ### Macro expansion
+///
+/// The macro implements the `CheckIfFeeless` trait on the dispatchable and calls the corresponding
+/// closure in the implementation.
+#[proc_macro_attribute]
+pub fn feeless_if(_: TokenStream, _: TokenStream) -> TokenStream {
+	pallet_macro_stub()
+}
+
 /// Allows you to define some extra constants to be added into constant metadata.
 ///
 /// Item must be defined as:

diff --git a/substrate/frame/support/procedural/src/pallet/expand/call.rs b/substrate/frame/support/procedural/src/pallet/expand/call.rs
@@ -241,6 +241,16 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
 		})
 		.collect::<Vec<_>>();
 
+	let feeless_check = methods.iter().map(|method| &method.feeless_check).collect::<Vec<_>>();
+	let feeless_check_result =
+		feeless_check.iter().zip(args_name.iter()).map(|(feeless_check, arg_name)| {
+			if let Some(feeless_check) = feeless_check {
+				quote::quote!(#feeless_check(origin, #( #arg_name, )*))
+			} else {
+				quote::quote!(false)
+			}
+		});
+
 	quote::quote_spanned!(span =>
 		mod warnings {
 			#(
@@ -347,6 +357,23 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
 			}
 		}
 
+		impl<#type_impl_gen> #frame_support::dispatch::CheckIfFeeless for #call_ident<#type_use_gen>
+			#where_clause
+		{
+			type Origin = #frame_system::pallet_prelude::OriginFor<T>;
+			#[allow(unused_variables)]
+			fn is_feeless(&self, origin: &Self::Origin) -> bool {
+				match *self {
+					#(
+						Self::#fn_name { #( #args_name_pattern_ref, )* } => {
+							#feeless_check_result
+						},
+					)*
+					Self::__Ignore(_, _) => unreachable!("__Ignore cannot be used"),
+				}
+			}
+		}
+
 		impl<#type_impl_gen> #frame_support::traits::GetCallName for #call_ident<#type_use_gen>
 			#where_clause
 		{