From 61695cf6f979b4379cb17aebeabf6f4a8f0e31c8 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Wed, 27 Sep 2023 10:10:11 +0530 Subject: [PATCH 01/41] Initial setup --- .../node-template/pallets/template/src/lib.rs | 4 ++ .../bin/node-template/runtime/src/lib.rs | 1 + .../src/construct_runtime/expand/call.rs | 49 +++++++++++++++++++ .../procedural/src/pallet/expand/call.rs | 40 +++++++++++++++ .../procedural/src/pallet/parse/call.rs | 33 +++++++++---- 5 files changed, 117 insertions(+), 10 deletions(-) diff --git a/substrate/bin/node-template/pallets/template/src/lib.rs b/substrate/bin/node-template/pallets/template/src/lib.rs index 4a2e53baa774..db2da61afb60 100644 --- a/substrate/bin/node-template/pallets/template/src/lib.rs +++ b/substrate/bin/node-template/pallets/template/src/lib.rs @@ -154,6 +154,10 @@ pub mod pallet { /// error if it isn't. Learn more about origins here: #[pallet::call_index(0)] #[pallet::weight(T::WeightInfo::do_something())] + #[pallet::feeless_if(|origin: &OriginFor, something: &u32| -> bool { + let account = ensure_signed(origin.clone())?; + *something == 0 + })] pub fn do_something(origin: OriginFor, something: u32) -> DispatchResult { // Check that the extrinsic was signed and get the signer. let who = ensure_signed(origin)?; diff --git a/substrate/bin/node-template/runtime/src/lib.rs b/substrate/bin/node-template/runtime/src/lib.rs index 216be9588bca..607609729604 100644 --- a/substrate/bin/node-template/runtime/src/lib.rs +++ b/substrate/bin/node-template/runtime/src/lib.rs @@ -309,6 +309,7 @@ pub type SignedExtra = ( frame_system::CheckNonce, frame_system::CheckWeight, pallet_transaction_payment::ChargeTransactionPayment, + RuntimeSignedExtension ); /// Unchecked extrinsic type as expected by this runtime. diff --git a/substrate/frame/support/procedural/src/construct_runtime/expand/call.rs b/substrate/frame/support/procedural/src/construct_runtime/expand/call.rs index 859b9a327e48..b5ce992a5d13 100644 --- a/substrate/frame/support/procedural/src/construct_runtime/expand/call.rs +++ b/substrate/frame/support/procedural/src/construct_runtime/expand/call.rs @@ -207,5 +207,54 @@ pub fn expand_outer_dispatch( } } )* + + /// RuntimeSignedExtension + #[derive( + #scrate::RuntimeDebugNoBound, + #scrate::CloneNoBound, + #scrate::EqNoBound, + #scrate::PartialEqNoBound, + #scrate::__private::codec::Encode, + #scrate::__private::codec::Decode, + #scrate::__private::scale_info::TypeInfo, + )] + #[codec(encode_bound())] + #[codec(decode_bound())] + pub struct RuntimeSignedExtension; + + impl #scrate::sp_runtime::traits::SignedExtension for RuntimeSignedExtension + // where + // T::RuntimeCall: #scrate::sp_runtime::traits::Dispatchable + { + type AccountId = <#runtime as #system_path::Config>::AccountId; + type Call = RuntimeCall; + type AdditionalSigned = (); + type Pre = (); + const IDENTIFIER: &'static str = "RuntimeSignedExtension"; + + fn additional_signed(&self) -> #scrate::__private::sp_std::result::Result<(), #scrate::sp_runtime::transaction_validity::TransactionValidityError> { + Ok(()) + } + + fn pre_dispatch( + self, + _who: &Self::AccountId, + _call: &Self::Call, + _info: &#scrate::sp_runtime::traits::DispatchInfoOf, + _len: usize, + ) -> Result<(), #scrate::sp_runtime::transaction_validity::TransactionValidityError> { + Ok(()) + } + + fn post_dispatch( + _pre: Option, + _info: &#scrate::sp_runtime::traits::DispatchInfoOf, + _post_info: &#scrate::sp_runtime::traits::PostDispatchInfoOf, + _len: usize, + _result: &#scrate::dispatch::DispatchResult, + ) -> Result<(), #scrate::sp_runtime::transaction_validity::TransactionValidityError> { + Ok(()) + } + } } } diff --git a/substrate/frame/support/procedural/src/pallet/expand/call.rs b/substrate/frame/support/procedural/src/pallet/expand/call.rs index 3ed5509863e9..b6e7b32b3040 100644 --- a/substrate/frame/support/procedural/src/pallet/expand/call.rs +++ b/substrate/frame/support/procedural/src/pallet/expand/call.rs @@ -423,5 +423,45 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream { #frame_support::__private::scale_info::meta_type::<#call_ident<#type_use_gen>>().into() } } + + // /// RuntimeSignedExtension + // #[derive( + // #frame_support::RuntimeDebugNoBound, + // #frame_support::CloneNoBound, + // #frame_support::EqNoBound, + // #frame_support::PartialEqNoBound, + // #frame_support::__private::codec::Encode, + // #frame_support::__private::codec::Decode, + // #frame_support::__private::scale_info::TypeInfo, + // )] + // #[codec(encode_bound())] + // #[codec(decode_bound())] + // #[scale_info(skip_type_params(T))] + // pub struct RuntimeSignedExtension(#frame_support::__private::sp_std::marker::PhantomData); + + // impl #frame_support::sp_runtime::traits::SignedExtension for RuntimeSignedExtension + // // where + // // T::RuntimeCall: #frame_support::sp_runtime::traits::Dispatchable + // { + // type AccountId = T::AccountId; + // type Call = RuntimeCall; + // type AdditionalSigned = (); + // type Pre = (); + // const IDENTIFIER: &'static str = "RuntimeSignedExtension"; + + // fn additional_signed(&self) -> #frame_support::__private::sp_std::result::Result<(), TransactionValidityError> { + // Ok(()) + // } + + // fn pre_dispatch( + // self, + // _who: &Self::AccountId, + // _call: &Self::Call, + // _info: &#frame_support::sp_runtime::traits::DispatchInfoOf, + // _len: usize, + // ) -> Result<(), TransactionValidityError> { + // Ok(()) + // } + // } ) } diff --git a/substrate/frame/support/procedural/src/pallet/parse/call.rs b/substrate/frame/support/procedural/src/pallet/parse/call.rs index 90631f264b92..c570d27c6321 100644 --- a/substrate/frame/support/procedural/src/pallet/parse/call.rs +++ b/substrate/frame/support/procedural/src/pallet/parse/call.rs @@ -30,6 +30,7 @@ mod keyword { syn::custom_keyword!(compact); syn::custom_keyword!(T); syn::custom_keyword!(pallet); + syn::custom_keyword!(feeless_if); } /// Definition of dispatchables typically `impl Pallet { ... }` @@ -89,6 +90,7 @@ pub struct CallVariantDef { pub enum FunctionAttr { CallIndex(u8), Weight(syn::Expr), + FeelessIf(syn::ExprClosure) } impl syn::parse::Parse for FunctionAttr { @@ -115,6 +117,11 @@ impl syn::parse::Parse for FunctionAttr { return Err(syn::Error::new(index.span(), msg)) } Ok(FunctionAttr::CallIndex(index.base10_parse()?)) + } else if lookahead.peek(keyword::feeless_if) { + content.parse::()?; + let closure_content; + syn::parenthesized!(closure_content in content); + Ok(FunctionAttr::FeelessIf(closure_content.parse::()?)) } else { Err(lookahead.error()) } @@ -227,16 +234,22 @@ impl CallDef { return Err(syn::Error::new(method.sig.span(), msg)) } - let (mut weight_attrs, mut call_idx_attrs): (Vec, Vec) = - helper::take_item_pallet_attrs(&mut method.attrs)?.into_iter().partition( - |attr| { - if let FunctionAttr::Weight(_) = attr { - true - } else { - false - } - }, - ); + let mut call_idx_attrs = vec![]; + let mut weight_attrs = vec![]; + let mut feeless_if_attrs = vec![]; + for attr in helper::take_item_pallet_attrs(&mut method.attrs)?.into_iter() { + match attr { + FunctionAttr::CallIndex(_) => { + call_idx_attrs.push(attr); + } + FunctionAttr::Weight(_) => { + weight_attrs.push(attr); + } + FunctionAttr::FeelessIf(_) => { + feeless_if_attrs.push(attr); + } + } + } if weight_attrs.is_empty() && dev_mode { // inject a default O(1) weight when dev mode is enabled and no weight has From d199795974e9860bfcbc2f9b67c34f5cfd7dbf9e Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Wed, 18 Oct 2023 10:43:19 +0530 Subject: [PATCH 02/41] Fixes end-to-end --- .../node-template/pallets/template/src/lib.rs | 3 +- .../bin/node-template/runtime/src/lib.rs | 3 +- .../src/construct_runtime/expand/call.rs | 63 +++++++++++------ .../procedural/src/pallet/expand/call.rs | 67 ++++++++----------- .../procedural/src/pallet/parse/call.rs | 32 ++++++--- substrate/frame/support/src/dispatch.rs | 6 ++ substrate/frame/system/src/lib.rs | 2 + 7 files changed, 101 insertions(+), 75 deletions(-) diff --git a/substrate/bin/node-template/pallets/template/src/lib.rs b/substrate/bin/node-template/pallets/template/src/lib.rs index db2da61afb60..a87b5224e124 100644 --- a/substrate/bin/node-template/pallets/template/src/lib.rs +++ b/substrate/bin/node-template/pallets/template/src/lib.rs @@ -154,8 +154,7 @@ pub mod pallet { /// error if it isn't. Learn more about origins here: #[pallet::call_index(0)] #[pallet::weight(T::WeightInfo::do_something())] - #[pallet::feeless_if(|origin: &OriginFor, something: &u32| -> bool { - let account = ensure_signed(origin.clone())?; + #[pallet::feeless_if(|_who: &AccountIdFor, something: &u32| -> bool { *something == 0 })] pub fn do_something(origin: OriginFor, something: u32) -> DispatchResult { diff --git a/substrate/bin/node-template/runtime/src/lib.rs b/substrate/bin/node-template/runtime/src/lib.rs index 1f54da9c6d65..16780bce38b2 100644 --- a/substrate/bin/node-template/runtime/src/lib.rs +++ b/substrate/bin/node-template/runtime/src/lib.rs @@ -309,8 +309,7 @@ pub type SignedExtra = ( frame_system::CheckEra, frame_system::CheckNonce, frame_system::CheckWeight, - pallet_transaction_payment::ChargeTransactionPayment, - RuntimeSignedExtension + SkipCheckIfFeeless>, ); /// Unchecked extrinsic type as expected by this runtime. diff --git a/substrate/frame/support/procedural/src/construct_runtime/expand/call.rs b/substrate/frame/support/procedural/src/construct_runtime/expand/call.rs index b5ce992a5d13..9955e546c445 100644 --- 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 AccountId = #system_path::pallet_prelude::AccountIdFor<#runtime>; + fn is_feeless(&self, account_id: &Self::AccountId) -> bool { + match self { + #( + #pallet_attrs + #variant_patterns => call.is_feeless(account_id), + )* + } + } + } + impl #scrate::traits::GetCallMetadata for RuntimeCall { fn get_call_metadata(&self) -> #scrate::traits::CallMetadata { use #scrate::traits::GetCallName; @@ -208,7 +220,7 @@ pub fn expand_outer_dispatch( } )* - /// RuntimeSignedExtension + /// SkipCheckIfFeeless #[derive( #scrate::RuntimeDebugNoBound, #scrate::CloneNoBound, @@ -218,19 +230,17 @@ pub fn expand_outer_dispatch( #scrate::__private::codec::Decode, #scrate::__private::scale_info::TypeInfo, )] - #[codec(encode_bound())] - #[codec(decode_bound())] - pub struct RuntimeSignedExtension; + pub struct SkipCheckIfFeeless(T); - impl #scrate::sp_runtime::traits::SignedExtension for RuntimeSignedExtension - // where - // T::RuntimeCall: #scrate::sp_runtime::traits::Dispatchable + impl #scrate::sp_runtime::traits::SignedExtension for SkipCheckIfFeeless + where + T::Call: #scrate::dispatch::CheckIfFeeless { - type AccountId = <#runtime as #system_path::Config>::AccountId; - type Call = RuntimeCall; + type AccountId = T::AccountId; + type Call = T::Call; type AdditionalSigned = (); - type Pre = (); - const IDENTIFIER: &'static str = "RuntimeSignedExtension"; + type Pre = Option; + const IDENTIFIER: &'static str = "SkipCheckIfFeeless"; fn additional_signed(&self) -> #scrate::__private::sp_std::result::Result<(), #scrate::sp_runtime::transaction_validity::TransactionValidityError> { Ok(()) @@ -238,21 +248,30 @@ pub fn expand_outer_dispatch( fn pre_dispatch( self, - _who: &Self::AccountId, - _call: &Self::Call, - _info: &#scrate::sp_runtime::traits::DispatchInfoOf, - _len: usize, - ) -> Result<(), #scrate::sp_runtime::transaction_validity::TransactionValidityError> { - Ok(()) + who: &Self::AccountId, + call: &Self::Call, + info: &#scrate::sp_runtime::traits::DispatchInfoOf, + len: usize, + ) -> Result { + use #scrate::dispatch::CheckIfFeeless; + match call.is_feeless(who) { + true => Ok(None), + false => Ok(Some(self.0.pre_dispatch(who, call, info, len)?)), + } } fn post_dispatch( - _pre: Option, - _info: &#scrate::sp_runtime::traits::DispatchInfoOf, - _post_info: &#scrate::sp_runtime::traits::PostDispatchInfoOf, - _len: usize, - _result: &#scrate::dispatch::DispatchResult, + pre: Option, + info: &#scrate::sp_runtime::traits::DispatchInfoOf, + post_info: &#scrate::sp_runtime::traits::PostDispatchInfoOf, + len: usize, + result: &#scrate::dispatch::DispatchResult, ) -> Result<(), #scrate::sp_runtime::transaction_validity::TransactionValidityError> { + if let Some(pre) = pre { + if let Some(pre) = pre { + T::post_dispatch(Some(pre), info, post_info, len, result)?; + } + } Ok(()) } } diff --git a/substrate/frame/support/procedural/src/pallet/expand/call.rs b/substrate/frame/support/procedural/src/pallet/expand/call.rs index 397a5950e300..effbf4f6cb02 100644 --- 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::>(); + let feeless_check = methods.iter().map(|method| &method.feeless_check).collect::>(); + 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(account_id, #( #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 AccountId = #frame_system::pallet_prelude::AccountIdFor; + #[allow(unused_variables)] + fn is_feeless(&self, account_id: &Self::AccountId) -> 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 { @@ -419,45 +446,5 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream { #frame_support::__private::scale_info::meta_type::<#call_ident<#type_use_gen>>().into() } } - - // /// RuntimeSignedExtension - // #[derive( - // #frame_support::RuntimeDebugNoBound, - // #frame_support::CloneNoBound, - // #frame_support::EqNoBound, - // #frame_support::PartialEqNoBound, - // #frame_support::__private::codec::Encode, - // #frame_support::__private::codec::Decode, - // #frame_support::__private::scale_info::TypeInfo, - // )] - // #[codec(encode_bound())] - // #[codec(decode_bound())] - // #[scale_info(skip_type_params(T))] - // pub struct RuntimeSignedExtension(#frame_support::__private::sp_std::marker::PhantomData); - - // impl #frame_support::sp_runtime::traits::SignedExtension for RuntimeSignedExtension - // // where - // // T::RuntimeCall: #frame_support::sp_runtime::traits::Dispatchable - // { - // type AccountId = T::AccountId; - // type Call = RuntimeCall; - // type AdditionalSigned = (); - // type Pre = (); - // const IDENTIFIER: &'static str = "RuntimeSignedExtension"; - - // fn additional_signed(&self) -> #frame_support::__private::sp_std::result::Result<(), TransactionValidityError> { - // Ok(()) - // } - - // fn pre_dispatch( - // self, - // _who: &Self::AccountId, - // _call: &Self::Call, - // _info: &#frame_support::sp_runtime::traits::DispatchInfoOf, - // _len: usize, - // ) -> Result<(), TransactionValidityError> { - // Ok(()) - // } - // } ) } diff --git a/substrate/frame/support/procedural/src/pallet/parse/call.rs b/substrate/frame/support/procedural/src/pallet/parse/call.rs index c570d27c6321..b9716184f3b8 100644 --- a/substrate/frame/support/procedural/src/pallet/parse/call.rs +++ b/substrate/frame/support/procedural/src/pallet/parse/call.rs @@ -19,7 +19,7 @@ use super::{helper, InheritedCallWeightAttr}; use frame_support_procedural_tools::get_doc_literals; use quote::ToTokens; use std::collections::HashMap; -use syn::spanned::Spanned; +use syn::{spanned::Spanned, ExprClosure}; /// List of additional token to be used for parsing. mod keyword { @@ -83,14 +83,17 @@ pub struct CallVariantDef { pub docs: Vec, /// Attributes annotated at the top of the dispatchable function. pub attrs: Vec, + /// The optional `feeless_if` attribute on the `pallet::call`. + pub feeless_check: Option, } /// Attributes for functions in call impl block. -/// Parse for `#[pallet::weight(expr)]` or `#[pallet::call_index(expr)] +/// Parse for `#[pallet::weight(expr)]` or `#[pallet::call_index(expr)] or +/// `#[pallet::feeless_if(expr)]` pub enum FunctionAttr { CallIndex(u8), Weight(syn::Expr), - FeelessIf(syn::ExprClosure) + FeelessIf(syn::ExprClosure), } impl syn::parse::Parse for FunctionAttr { @@ -117,7 +120,7 @@ impl syn::parse::Parse for FunctionAttr { return Err(syn::Error::new(index.span(), msg)) } Ok(FunctionAttr::CallIndex(index.base10_parse()?)) - } else if lookahead.peek(keyword::feeless_if) { + } else if lookahead.peek(keyword::feeless_if) { content.parse::()?; let closure_content; syn::parenthesized!(closure_content in content); @@ -236,18 +239,18 @@ impl CallDef { let mut call_idx_attrs = vec![]; let mut weight_attrs = vec![]; - let mut feeless_if_attrs = vec![]; + let mut feeless_attrs = vec![]; for attr in helper::take_item_pallet_attrs(&mut method.attrs)?.into_iter() { match attr { FunctionAttr::CallIndex(_) => { call_idx_attrs.push(attr); - } + }, FunctionAttr::Weight(_) => { weight_attrs.push(attr); - } + }, FunctionAttr::FeelessIf(_) => { - feeless_if_attrs.push(attr); - } + feeless_attrs.push(attr); + }, } } @@ -336,6 +339,16 @@ impl CallDef { let docs = get_doc_literals(&method.attrs); + if feeless_attrs.len() > 1 { + let msg = "Invalid pallet::call, too many feeless_if attributes given"; + return Err(syn::Error::new(method.sig.span(), msg)) + } + let feeless_check: Option = + feeless_attrs.pop().map(|attr| match attr { + FunctionAttr::FeelessIf(closure) => closure, + _ => unreachable!("checked during creation of the let binding"), + }); + methods.push(CallVariantDef { name: method.sig.ident.clone(), weight, @@ -344,6 +357,7 @@ impl CallDef { args, docs, attrs: method.attrs.clone(), + feeless_check, }); } else { let msg = "Invalid pallet::call, only method accepted"; diff --git a/substrate/frame/support/src/dispatch.rs b/substrate/frame/support/src/dispatch.rs index 75e94827fea8..7893d216a245 100644 --- a/substrate/frame/support/src/dispatch.rs +++ b/substrate/frame/support/src/dispatch.rs @@ -54,6 +54,12 @@ pub trait Callable { // https://github.com/rust-lang/rust/issues/51331 pub type CallableCallFor = >::RuntimeCall; +pub trait CheckIfFeeless { + type AccountId; + + fn is_feeless(&self, account_d: &Self::AccountId) -> bool; +} + /// Origin for the System pallet. #[derive(PartialEq, Eq, Clone, RuntimeDebug, Encode, Decode, TypeInfo, MaxEncodedLen)] pub enum RawOrigin { diff --git a/substrate/frame/system/src/lib.rs b/substrate/frame/system/src/lib.rs index 897d3bd7ce91..183534a560b2 100644 --- a/substrate/frame/system/src/lib.rs +++ b/substrate/frame/system/src/lib.rs @@ -1811,6 +1811,8 @@ pub mod pallet_prelude { /// Type alias for the `Origin` associated type of system config. pub type OriginFor = ::RuntimeOrigin; + pub type AccountIdFor = ::AccountId; + /// Type alias for the `Header`. pub type HeaderFor = <::Block as sp_runtime::traits::HeaderProvider>::HeaderT; From b0390bfc0de4b32e77a27a39f7699252f7a9ce89 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Fri, 20 Oct 2023 11:22:34 +0530 Subject: [PATCH 03/41] Adds pallet-skip-feeless-payment --- Cargo.lock | 13 ++ Cargo.toml | 1 + .../bin/node-template/runtime/Cargo.toml | 2 + .../bin/node-template/runtime/src/lib.rs | 10 +- .../src/construct_runtime/expand/call.rs | 56 --------- .../skip-feeless-payment/Cargo.toml | 46 +++++++ .../skip-feeless-payment/src/lib.rs | 112 ++++++++++++++++++ 7 files changed, 183 insertions(+), 57 deletions(-) create mode 100644 substrate/frame/transaction-payment/skip-feeless-payment/Cargo.toml create mode 100644 substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index d8e7d055d770..24bf069a3f07 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8467,6 +8467,7 @@ dependencies = [ "pallet-aura", "pallet-balances", "pallet-grandpa", + "pallet-skip-feeless-payment", "pallet-sudo", "pallet-template", "pallet-timestamp", @@ -10427,6 +10428,18 @@ dependencies = [ "sp-std", ] +[[package]] +name = "pallet-skip-feeless-payment" +version = "4.0.0-dev" +dependencies = [ + "frame-support", + "frame-system", + "parity-scale-codec", + "scale-info", + "sp-runtime", + "sp-std", +] + [[package]] name = "pallet-society" version = "4.0.0-dev" diff --git a/Cargo.toml b/Cargo.toml index 75da6681465d..5bab7f0d7c95 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -355,6 +355,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/substrate/bin/node-template/runtime/Cargo.toml b/substrate/bin/node-template/runtime/Cargo.toml index caca54ce2ba4..12146f2c2d01 100644 --- a/substrate/bin/node-template/runtime/Cargo.toml +++ b/substrate/bin/node-template/runtime/Cargo.toml @@ -25,6 +25,7 @@ frame-system = { path = "../../../frame/system", default-features = false} frame-try-runtime = { path = "../../../frame/try-runtime", default-features = false, optional = true } pallet-timestamp = { path = "../../../frame/timestamp", default-features = false} pallet-transaction-payment = { path = "../../../frame/transaction-payment", default-features = false} +pallet-skip-feeless-payment = { path = "../../../frame/transaction-payment/skip-feeless-payment", default-features = false} frame-executive = { path = "../../../frame/executive", default-features = false} sp-api = { path = "../../../primitives/api", default-features = false} sp-block-builder = { path = "../../../primitives/block-builder", default-features = false} @@ -74,6 +75,7 @@ std = [ "pallet-timestamp/std", "pallet-transaction-payment-rpc-runtime-api/std", "pallet-transaction-payment/std", + "pallet-skip-feeless-payment/std", "scale-info/std", "sp-api/std", "sp-block-builder/std", diff --git a/substrate/bin/node-template/runtime/src/lib.rs b/substrate/bin/node-template/runtime/src/lib.rs index 16780bce38b2..9c23ce039c29 100644 --- a/substrate/bin/node-template/runtime/src/lib.rs +++ b/substrate/bin/node-template/runtime/src/lib.rs @@ -267,6 +267,10 @@ impl pallet_transaction_payment::Config for Runtime { type FeeMultiplierUpdate = ConstFeeMultiplier; } +impl pallet_skip_feeless_payment::Config for Runtime { + type RuntimeEvent = RuntimeEvent; +} + impl pallet_sudo::Config for Runtime { type RuntimeEvent = RuntimeEvent; type RuntimeCall = RuntimeCall; @@ -288,6 +292,7 @@ construct_runtime!( Grandpa: pallet_grandpa, Balances: pallet_balances, TransactionPayment: pallet_transaction_payment, + SkipFeelessPayment: pallet_skip_feeless_payment, Sudo: pallet_sudo, // Include the custom logic from the pallet-template in the runtime. TemplateModule: pallet_template, @@ -309,7 +314,10 @@ pub type SignedExtra = ( frame_system::CheckEra, frame_system::CheckNonce, frame_system::CheckWeight, - SkipCheckIfFeeless>, + pallet_skip_feeless_payment::SkipCheckIfFeeless< + Runtime, + pallet_transaction_payment::ChargeTransactionPayment, + >, ); /// Unchecked extrinsic type as expected by this runtime. diff --git a/substrate/frame/support/procedural/src/construct_runtime/expand/call.rs b/substrate/frame/support/procedural/src/construct_runtime/expand/call.rs index 9955e546c445..a9ca00a4e19b 100644 --- a/substrate/frame/support/procedural/src/construct_runtime/expand/call.rs +++ b/substrate/frame/support/procedural/src/construct_runtime/expand/call.rs @@ -219,61 +219,5 @@ pub fn expand_outer_dispatch( } } )* - - /// SkipCheckIfFeeless - #[derive( - #scrate::RuntimeDebugNoBound, - #scrate::CloneNoBound, - #scrate::EqNoBound, - #scrate::PartialEqNoBound, - #scrate::__private::codec::Encode, - #scrate::__private::codec::Decode, - #scrate::__private::scale_info::TypeInfo, - )] - pub struct SkipCheckIfFeeless(T); - - impl #scrate::sp_runtime::traits::SignedExtension for SkipCheckIfFeeless - where - T::Call: #scrate::dispatch::CheckIfFeeless - { - type AccountId = T::AccountId; - type Call = T::Call; - type AdditionalSigned = (); - type Pre = Option; - const IDENTIFIER: &'static str = "SkipCheckIfFeeless"; - - fn additional_signed(&self) -> #scrate::__private::sp_std::result::Result<(), #scrate::sp_runtime::transaction_validity::TransactionValidityError> { - Ok(()) - } - - fn pre_dispatch( - self, - who: &Self::AccountId, - call: &Self::Call, - info: &#scrate::sp_runtime::traits::DispatchInfoOf, - len: usize, - ) -> Result { - use #scrate::dispatch::CheckIfFeeless; - match call.is_feeless(who) { - true => Ok(None), - false => Ok(Some(self.0.pre_dispatch(who, call, info, len)?)), - } - } - - fn post_dispatch( - pre: Option, - info: &#scrate::sp_runtime::traits::DispatchInfoOf, - post_info: &#scrate::sp_runtime::traits::PostDispatchInfoOf, - len: usize, - result: &#scrate::dispatch::DispatchResult, - ) -> Result<(), #scrate::sp_runtime::transaction_validity::TransactionValidityError> { - if let Some(pre) = pre { - if let Some(pre) = pre { - T::post_dispatch(Some(pre), info, post_info, len, result)?; - } - } - Ok(()) - } - } } } diff --git a/substrate/frame/transaction-payment/skip-feeless-payment/Cargo.toml b/substrate/frame/transaction-payment/skip-feeless-payment/Cargo.toml new file mode 100644 index 000000000000..6e6ca3bd37e8 --- /dev/null +++ b/substrate/frame/transaction-payment/skip-feeless-payment/Cargo.toml @@ -0,0 +1,46 @@ +[package] +name = "pallet-skip-feeless-payment" +version = "4.0.0-dev" +authors.workspace = true +edition.workspace = true +license = "Apache-2.0" +homepage = "https://substrate.io" +repository.workspace = true +description = "pallet to skip feeless payments" +readme = "README.md" + +[package.metadata.docs.rs] +targets = ["x86_64-unknown-linux-gnu"] + +[dependencies] +# Substrate dependencies +sp-runtime = { path = "../../../primitives/runtime", default-features = false} +sp-std = { path = "../../../primitives/std", default-features = false} + +frame-support = { path = "../../support", default-features = false} +frame-system = { path = "../../system", default-features = false} + +# Other dependencies +codec = { package = "parity-scale-codec", version = "3.6.1", default-features = false, features = ["derive"] } +scale-info = { version = "2.5.0", default-features = false, features = ["derive"] } + +[features] +default = [ "std" ] +std = [ + "codec/std", + "frame-support/std", + "frame-system/std", + "scale-info/std", + "sp-runtime/std", + "sp-std/std", +] +runtime-benchmarks = [ + "frame-support/runtime-benchmarks", + "frame-system/runtime-benchmarks", + "sp-runtime/runtime-benchmarks", +] +try-runtime = [ + "frame-support/try-runtime", + "frame-system/try-runtime", + "sp-runtime/try-runtime", +] diff --git a/substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs b/substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs new file mode 100644 index 000000000000..0f9f58624131 --- /dev/null +++ b/substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs @@ -0,0 +1,112 @@ +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#![cfg_attr(not(feature = "std"), no_std)] + +use codec::{Decode, Encode}; +use frame_support::{ + dispatch::{CheckIfFeeless, DispatchResult}, + traits::IsType, +}; +use scale_info::TypeInfo; +use sp_runtime::{ + traits::{DispatchInfoOf, PostDispatchInfoOf, SignedExtension}, + transaction_validity::TransactionValidityError, +}; + +pub use pallet::*; + +#[frame_support::pallet] +pub mod pallet { + use super::*; + + #[pallet::config] + pub trait Config: frame_system::Config { + /// The overarching event type. + type RuntimeEvent: From> + IsType<::RuntimeEvent>; + } + + #[pallet::pallet] + pub struct Pallet(_); + + #[pallet::event] + #[pallet::generate_deposit(pub(super) fn deposit_event)] + pub enum Event { + /// A transaction fee was skipped. + FeeSkipped { who: T::AccountId }, + } +} + +#[derive(Encode, Decode, Clone, Eq, PartialEq, TypeInfo)] +#[scale_info(skip_type_params(T))] +pub struct SkipCheckIfFeeless(S, sp_std::marker::PhantomData); + +impl sp_std::fmt::Debug for SkipCheckIfFeeless { + #[cfg(feature = "std")] + fn fmt(&self, f: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result { + write!(f, "SkipCheckIfFeeless<{:?}>", self.0.encode()) + } + #[cfg(not(feature = "std"))] + fn fmt(&self, _: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result { + Ok(()) + } +} + +impl> SignedExtension + for SkipCheckIfFeeless +where + S::Call: CheckIfFeeless, +{ + type AccountId = T::AccountId; + type Call = S::Call; + type AdditionalSigned = (); + type Pre = (Self::AccountId, Option<::Pre>); + const IDENTIFIER: &'static str = "SkipCheckIfFeeless"; + + fn additional_signed(&self) -> Result<(), TransactionValidityError> { + Ok(()) + } + + fn pre_dispatch( + self, + who: &Self::AccountId, + call: &Self::Call, + info: &DispatchInfoOf, + len: usize, + ) -> Result { + if call.is_feeless(who) { + Ok((who.clone(), None)) + } else { + Ok((who.clone(), Some(self.0.pre_dispatch(who, call, info, len)?))) + } + } + + fn post_dispatch( + pre: Option, + info: &DispatchInfoOf, + post_info: &PostDispatchInfoOf, + len: usize, + result: &DispatchResult, + ) -> Result<(), TransactionValidityError> { + if let Some(pre) = pre { + if let Some(pre) = pre.1 { + S::post_dispatch(Some(pre), info, post_info, len, result)?; + } else { + Pallet::::deposit_event(Event::::FeeSkipped { who: pre.0 }); + } + } + Ok(()) + } +} From 7e0450b9c0a4ebdf9212583bb147e56293d2915d Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Fri, 20 Oct 2023 13:16:30 +0530 Subject: [PATCH 04/41] Adds AccountIdFor in test --- substrate/frame/support/test/src/lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/substrate/frame/support/test/src/lib.rs b/substrate/frame/support/test/src/lib.rs index 6b38d42d33d0..47e90fa18208 100644 --- a/substrate/frame/support/test/src/lib.rs +++ b/substrate/frame/support/test/src/lib.rs @@ -119,6 +119,9 @@ pub mod pallet_prelude { /// Type alias for the `Origin` associated type of system config. pub type OriginFor = ::RuntimeOrigin; + /// Type alias for the `AccountId` associated type of system config. + pub type AccountIdFor = ::AccountId; + /// Type alias for the `BlockNumber` associated type of system config. pub type BlockNumberFor = ::BlockNumber; } From 6802f4d1b7fc5f0f88dbfdadfdb1a0b3f5b80f4b Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Fri, 20 Oct 2023 14:09:06 +0530 Subject: [PATCH 05/41] Fixes node build --- Cargo.lock | 1 + substrate/bin/node-template/node/Cargo.toml | 1 + substrate/bin/node-template/node/src/benchmarking.rs | 4 +++- .../transaction-payment/skip-feeless-payment/src/lib.rs | 9 ++++++++- 4 files changed, 13 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b8f97ad5b4e6..a2733f92061e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8519,6 +8519,7 @@ dependencies = [ "futures", "jsonrpsee", "node-template-runtime", + "pallet-skip-feeless-payment", "pallet-transaction-payment", "pallet-transaction-payment-rpc", "sc-basic-authorship", diff --git a/substrate/bin/node-template/node/Cargo.toml b/substrate/bin/node-template/node/Cargo.toml index 23840cce2229..09d7b874e2a8 100644 --- a/substrate/bin/node-template/node/Cargo.toml +++ b/substrate/bin/node-template/node/Cargo.toml @@ -42,6 +42,7 @@ sp-inherents = { path = "../../../primitives/inherents" } sp-keyring = { path = "../../../primitives/keyring" } frame-system = { path = "../../../frame/system" } pallet-transaction-payment = { path = "../../../frame/transaction-payment", default-features = false} +pallet-skip-feeless-payment = { path = "../../../frame/transaction-payment/skip-feeless-payment", default-features = false} # These dependencies are used for the node template's RPCs jsonrpsee = { version = "0.16.2", features = ["server"] } diff --git a/substrate/bin/node-template/node/src/benchmarking.rs b/substrate/bin/node-template/node/src/benchmarking.rs index 6e29ad1a1231..7c0b8cbb76b6 100644 --- a/substrate/bin/node-template/node/src/benchmarking.rs +++ b/substrate/bin/node-template/node/src/benchmarking.rs @@ -120,7 +120,9 @@ pub fn create_benchmark_extrinsic( )), frame_system::CheckNonce::::from(nonce), frame_system::CheckWeight::::new(), - pallet_transaction_payment::ChargeTransactionPayment::::from(0), + pallet_skip_feeless_payment::SkipCheckIfFeeless::from( + pallet_transaction_payment::ChargeTransactionPayment::::from(0), + ), ); let raw_payload = runtime::SignedPayload::from_raw( diff --git a/substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs b/substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs index 0f9f58624131..08a1858276d4 100644 --- a/substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs +++ b/substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs @@ -51,7 +51,7 @@ pub mod pallet { #[derive(Encode, Decode, Clone, Eq, PartialEq, TypeInfo)] #[scale_info(skip_type_params(T))] -pub struct SkipCheckIfFeeless(S, sp_std::marker::PhantomData); +pub struct SkipCheckIfFeeless(pub S, sp_std::marker::PhantomData); impl sp_std::fmt::Debug for SkipCheckIfFeeless { #[cfg(feature = "std")] @@ -64,6 +64,13 @@ impl sp_std::fmt::Debug for SkipCheckIfFeeless SkipCheckIfFeeless { + /// utility constructor. Used only in client/factory code. + pub fn from(s: S) -> Self { + Self(s, sp_std::marker::PhantomData) + } +} + impl> SignedExtension for SkipCheckIfFeeless where From 093fa63f7b487814290a9e95f85fef69c39bde95 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Fri, 20 Oct 2023 14:26:02 +0530 Subject: [PATCH 06/41] Fixes prelude in tests --- substrate/frame/support/src/dispatch.rs | 2 ++ substrate/frame/support/src/storage/generator/mod.rs | 2 ++ substrate/frame/support/src/tests/mod.rs | 2 ++ substrate/frame/system/src/lib.rs | 1 + 4 files changed, 7 insertions(+) diff --git a/substrate/frame/support/src/dispatch.rs b/substrate/frame/support/src/dispatch.rs index 7893d216a245..59f45b97d5be 100644 --- a/substrate/frame/support/src/dispatch.rs +++ b/substrate/frame/support/src/dispatch.rs @@ -747,6 +747,8 @@ mod weight_tests { pub mod pallet_prelude { pub type OriginFor = ::RuntimeOrigin; + pub type AccountIdFor = ::AccountId; + pub type HeaderFor = <::Block as sp_runtime::traits::HeaderProvider>::HeaderT; diff --git a/substrate/frame/support/src/storage/generator/mod.rs b/substrate/frame/support/src/storage/generator/mod.rs index bac9f642e37d..806a6280d51a 100644 --- a/substrate/frame/support/src/storage/generator/mod.rs +++ b/substrate/frame/support/src/storage/generator/mod.rs @@ -103,6 +103,8 @@ mod tests { pub mod pallet_prelude { pub type OriginFor = ::RuntimeOrigin; + pub type AccountIdFor = ::AccountId; + pub type HeaderFor = <::Block as sp_runtime::traits::HeaderProvider>::HeaderT; diff --git a/substrate/frame/support/src/tests/mod.rs b/substrate/frame/support/src/tests/mod.rs index 3690159c5994..d55eeaf6ab22 100644 --- a/substrate/frame/support/src/tests/mod.rs +++ b/substrate/frame/support/src/tests/mod.rs @@ -172,6 +172,8 @@ pub mod frame_system { pub mod pallet_prelude { pub type OriginFor = ::RuntimeOrigin; + pub type AccountIdFor = ::AccountId; + pub type HeaderFor = <::Block as sp_runtime::traits::HeaderProvider>::HeaderT; diff --git a/substrate/frame/system/src/lib.rs b/substrate/frame/system/src/lib.rs index 183534a560b2..aea7ca953368 100644 --- a/substrate/frame/system/src/lib.rs +++ b/substrate/frame/system/src/lib.rs @@ -1811,6 +1811,7 @@ pub mod pallet_prelude { /// Type alias for the `Origin` associated type of system config. pub type OriginFor = ::RuntimeOrigin; + /// Type alias for the `AccountId`. pub type AccountIdFor = ::AccountId; /// Type alias for the `Header`. From 9cf74018c218d74002fad419d037da40e36f12bf Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Fri, 20 Oct 2023 14:47:07 +0530 Subject: [PATCH 07/41] Minor fixes --- substrate/frame/support/src/dispatch.rs | 2 +- substrate/frame/support/src/storage/generator/mod.rs | 4 ++-- substrate/frame/support/src/tests/mod.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/substrate/frame/support/src/dispatch.rs b/substrate/frame/support/src/dispatch.rs index 59f45b97d5be..e8c5f7fa89c2 100644 --- a/substrate/frame/support/src/dispatch.rs +++ b/substrate/frame/support/src/dispatch.rs @@ -747,7 +747,7 @@ mod weight_tests { pub mod pallet_prelude { pub type OriginFor = ::RuntimeOrigin; - pub type AccountIdFor = ::AccountId; + pub type AccountIdFor = ::AccountId; pub type HeaderFor = <::Block as sp_runtime::traits::HeaderProvider>::HeaderT; diff --git a/substrate/frame/support/src/storage/generator/mod.rs b/substrate/frame/support/src/storage/generator/mod.rs index 806a6280d51a..6e3f67f75e55 100644 --- a/substrate/frame/support/src/storage/generator/mod.rs +++ b/substrate/frame/support/src/storage/generator/mod.rs @@ -103,8 +103,8 @@ mod tests { pub mod pallet_prelude { pub type OriginFor = ::RuntimeOrigin; - pub type AccountIdFor = ::AccountId; - + pub type AccountIdFor = ::AccountId; + pub type HeaderFor = <::Block as sp_runtime::traits::HeaderProvider>::HeaderT; diff --git a/substrate/frame/support/src/tests/mod.rs b/substrate/frame/support/src/tests/mod.rs index d55eeaf6ab22..bd09735f5d5f 100644 --- a/substrate/frame/support/src/tests/mod.rs +++ b/substrate/frame/support/src/tests/mod.rs @@ -172,7 +172,7 @@ pub mod frame_system { pub mod pallet_prelude { pub type OriginFor = ::RuntimeOrigin; - pub type AccountIdFor = ::AccountId; + pub type AccountIdFor = ::AccountId; pub type HeaderFor = <::Block as sp_runtime::traits::HeaderProvider>::HeaderT; From 25f57f5019749d77dc9271fa5335d4b5d257b465 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Fri, 20 Oct 2023 15:09:02 +0530 Subject: [PATCH 08/41] Fixes feature propagation --- substrate/bin/node-template/node/Cargo.toml | 1 + substrate/bin/node-template/runtime/Cargo.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/substrate/bin/node-template/node/Cargo.toml b/substrate/bin/node-template/node/Cargo.toml index 09d7b874e2a8..905f60556fe2 100644 --- a/substrate/bin/node-template/node/Cargo.toml +++ b/substrate/bin/node-template/node/Cargo.toml @@ -84,6 +84,7 @@ try-runtime = [ "frame-system/try-runtime", "node-template-runtime/try-runtime", "pallet-transaction-payment/try-runtime", + "pallet-skip-feeless-payment/try-runtime", "sp-runtime/try-runtime", "try-runtime-cli/try-runtime", ] diff --git a/substrate/bin/node-template/runtime/Cargo.toml b/substrate/bin/node-template/runtime/Cargo.toml index 730d8ef26eac..367d3171f822 100644 --- a/substrate/bin/node-template/runtime/Cargo.toml +++ b/substrate/bin/node-template/runtime/Cargo.toml @@ -118,5 +118,6 @@ try-runtime = [ "pallet-timestamp/try-runtime", "pallet-transaction-payment/try-runtime", "sp-runtime/try-runtime", + "pallet-skip-feeless-payment/try-runtime", ] experimental = [ "pallet-aura/experimental" ] From de68ccc784b22982d442ef11c8e177c0bbe2b471 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Fri, 20 Oct 2023 16:16:15 +0530 Subject: [PATCH 09/41] Fixes feature propagation --- substrate/bin/node-template/node/Cargo.toml | 1 + substrate/bin/node-template/runtime/Cargo.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/substrate/bin/node-template/node/Cargo.toml b/substrate/bin/node-template/node/Cargo.toml index 905f60556fe2..0d89d988af32 100644 --- a/substrate/bin/node-template/node/Cargo.toml +++ b/substrate/bin/node-template/node/Cargo.toml @@ -75,6 +75,7 @@ runtime-benchmarks = [ "frame-benchmarking/runtime-benchmarks", "frame-system/runtime-benchmarks", "node-template-runtime/runtime-benchmarks", + "pallet-skip-feeless-payment/runtime-benchmarks", "sc-service/runtime-benchmarks", "sp-runtime/runtime-benchmarks", ] diff --git a/substrate/bin/node-template/runtime/Cargo.toml b/substrate/bin/node-template/runtime/Cargo.toml index 367d3171f822..e647c3dd2a93 100644 --- a/substrate/bin/node-template/runtime/Cargo.toml +++ b/substrate/bin/node-template/runtime/Cargo.toml @@ -103,6 +103,7 @@ runtime-benchmarks = [ "pallet-sudo/runtime-benchmarks", "pallet-template/runtime-benchmarks", "pallet-timestamp/runtime-benchmarks", + "pallet-skip-feeless-payment/runtime-benchmarks", "sp-runtime/runtime-benchmarks", ] try-runtime = [ From fac68c15181dc5a27af094b015c1e7a264c5b9a1 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Sat, 21 Oct 2023 11:16:12 +0530 Subject: [PATCH 10/41] Formats features --- substrate/bin/node-template/node/Cargo.toml | 2 +- substrate/bin/node-template/runtime/Cargo.toml | 6 +++--- .../transaction-payment/skip-feeless-payment/Cargo.toml | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/substrate/bin/node-template/node/Cargo.toml b/substrate/bin/node-template/node/Cargo.toml index 0d89d988af32..a25cd3ec655f 100644 --- a/substrate/bin/node-template/node/Cargo.toml +++ b/substrate/bin/node-template/node/Cargo.toml @@ -84,8 +84,8 @@ runtime-benchmarks = [ try-runtime = [ "frame-system/try-runtime", "node-template-runtime/try-runtime", - "pallet-transaction-payment/try-runtime", "pallet-skip-feeless-payment/try-runtime", + "pallet-transaction-payment/try-runtime", "sp-runtime/try-runtime", "try-runtime-cli/try-runtime", ] diff --git a/substrate/bin/node-template/runtime/Cargo.toml b/substrate/bin/node-template/runtime/Cargo.toml index 7dae22a02d09..27a20da1d295 100644 --- a/substrate/bin/node-template/runtime/Cargo.toml +++ b/substrate/bin/node-template/runtime/Cargo.toml @@ -70,12 +70,12 @@ std = [ "pallet-aura/std", "pallet-balances/std", "pallet-grandpa/std", + "pallet-skip-feeless-payment/std", "pallet-sudo/std", "pallet-template/std", "pallet-timestamp/std", "pallet-transaction-payment-rpc-runtime-api/std", "pallet-transaction-payment/std", - "pallet-skip-feeless-payment/std", "scale-info/std", "sp-api/std", "sp-block-builder/std", @@ -100,10 +100,10 @@ runtime-benchmarks = [ "frame-system/runtime-benchmarks", "pallet-balances/runtime-benchmarks", "pallet-grandpa/runtime-benchmarks", + "pallet-skip-feeless-payment/runtime-benchmarks", "pallet-sudo/runtime-benchmarks", "pallet-template/runtime-benchmarks", "pallet-timestamp/runtime-benchmarks", - "pallet-skip-feeless-payment/runtime-benchmarks", "sp-runtime/runtime-benchmarks", ] try-runtime = [ @@ -114,11 +114,11 @@ try-runtime = [ "pallet-aura/try-runtime", "pallet-balances/try-runtime", "pallet-grandpa/try-runtime", + "pallet-skip-feeless-payment/try-runtime", "pallet-sudo/try-runtime", "pallet-template/try-runtime", "pallet-timestamp/try-runtime", "pallet-transaction-payment/try-runtime", "sp-runtime/try-runtime", - "pallet-skip-feeless-payment/try-runtime", ] experimental = [ "pallet-aura/experimental" ] diff --git a/substrate/frame/transaction-payment/skip-feeless-payment/Cargo.toml b/substrate/frame/transaction-payment/skip-feeless-payment/Cargo.toml index 6e6ca3bd37e8..3673d5276abe 100644 --- a/substrate/frame/transaction-payment/skip-feeless-payment/Cargo.toml +++ b/substrate/frame/transaction-payment/skip-feeless-payment/Cargo.toml @@ -32,7 +32,7 @@ std = [ "frame-system/std", "scale-info/std", "sp-runtime/std", - "sp-std/std", + "sp-std/std", ] runtime-benchmarks = [ "frame-support/runtime-benchmarks", From b0fdbc1b0eb2ed6228e4d85c08b96cf4b35be489 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Sat, 21 Oct 2023 11:17:22 +0530 Subject: [PATCH 11/41] Updates UI test --- .../frame/support/test/tests/pallet_ui/call_invalid_attr.stderr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/support/test/tests/pallet_ui/call_invalid_attr.stderr b/substrate/frame/support/test/tests/pallet_ui/call_invalid_attr.stderr index eec5e33ccbd9..1809fcb6ed99 100644 --- a/substrate/frame/support/test/tests/pallet_ui/call_invalid_attr.stderr +++ b/substrate/frame/support/test/tests/pallet_ui/call_invalid_attr.stderr @@ -1,4 +1,4 @@ -error: expected `weight` or `call_index` +error: expected one of: `weight`, `call_index`, `feeless_if` --> tests/pallet_ui/call_invalid_attr.rs:31:13 | 31 | #[pallet::weird_attr] From 73c7bcafd7710b7c9a0f0b0af4017ec650e67d48 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Mon, 23 Oct 2023 11:29:54 +0530 Subject: [PATCH 12/41] Adds tests for skip fee payment pallet --- .../skip-feeless-payment/src/lib.rs | 11 ++- .../skip-feeless-payment/src/mock.rs | 92 +++++++++++++++++++ .../skip-feeless-payment/src/tests.rs | 33 +++++++ 3 files changed, 133 insertions(+), 3 deletions(-) create mode 100644 substrate/frame/transaction-payment/skip-feeless-payment/src/mock.rs create mode 100644 substrate/frame/transaction-payment/skip-feeless-payment/src/tests.rs diff --git a/substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs b/substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs index 08a1858276d4..eb1f0184ba6b 100644 --- a/substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs +++ b/substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs @@ -26,6 +26,11 @@ use sp_runtime::{ transaction_validity::TransactionValidityError, }; +#[cfg(test)] +mod mock; +#[cfg(test)] +mod tests; + pub use pallet::*; #[frame_support::pallet] @@ -78,12 +83,12 @@ where { type AccountId = T::AccountId; type Call = S::Call; - type AdditionalSigned = (); + type AdditionalSigned = S::AdditionalSigned; type Pre = (Self::AccountId, Option<::Pre>); const IDENTIFIER: &'static str = "SkipCheckIfFeeless"; - fn additional_signed(&self) -> Result<(), TransactionValidityError> { - Ok(()) + fn additional_signed(&self) -> Result { + self.0.additional_signed() } fn pre_dispatch( diff --git a/substrate/frame/transaction-payment/skip-feeless-payment/src/mock.rs b/substrate/frame/transaction-payment/skip-feeless-payment/src/mock.rs new file mode 100644 index 000000000000..20861e9942a6 --- /dev/null +++ b/substrate/frame/transaction-payment/skip-feeless-payment/src/mock.rs @@ -0,0 +1,92 @@ +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use super::*; +use crate as pallet_skip_feeless_payment; + +use frame_support::{derive_impl, parameter_types}; +use frame_system as system; + +type Block = frame_system::mocking::MockBlock; +type AccountId = u64; + +#[derive_impl(frame_system::config_preludes::TestDefaultConfig as frame_system::DefaultConfig)] +impl frame_system::Config for Runtime { + type Block = Block; +} + +impl Config for Runtime { + type RuntimeEvent = RuntimeEvent; +} + +parameter_types! { + pub static PreDispatchCount: u32 = 0; +} + +#[derive(Clone, Eq, PartialEq, Debug, Encode, Decode, TypeInfo)] +pub struct DummyExtension; + +impl SignedExtension for DummyExtension { + type AccountId = AccountId; + type Call = RuntimeCall; + type AdditionalSigned = (); + type Pre = (); + const IDENTIFIER: &'static str = "DummyExtension"; + fn additional_signed(&self) -> sp_std::result::Result<(), TransactionValidityError> { + Ok(()) + } + fn pre_dispatch( + self, + _who: &Self::AccountId, + _call: &Self::Call, + _info: &DispatchInfoOf, + _len: usize, + ) -> Result { + PreDispatchCount::mutate(|c| *c += 1); + Ok(()) + } +} + +#[frame_support::pallet(dev_mode)] +pub mod pallet_dummy { + use frame_support::pallet_prelude::*; + use frame_system::pallet_prelude::*; + + #[pallet::pallet] + pub struct Pallet(_); + + #[pallet::config] + pub trait Config: frame_system::Config {} + + #[pallet::call] + impl Pallet { + #[pallet::feeless_if(|_who: &AccountIdFor, data: &u32| -> bool { + *data == 0 + })] + pub fn aux(_origin: OriginFor, #[pallet::compact] _data: u32) -> DispatchResult { + unreachable!() + } + } +} + +impl pallet_dummy::Config for Runtime {} + +frame_support::construct_runtime!( + pub struct Runtime { + System: system, + SkipFeeless: pallet_skip_feeless_payment, + DummyPallet: pallet_dummy, + } +); diff --git a/substrate/frame/transaction-payment/skip-feeless-payment/src/tests.rs b/substrate/frame/transaction-payment/skip-feeless-payment/src/tests.rs new file mode 100644 index 000000000000..4b4dd6997418 --- /dev/null +++ b/substrate/frame/transaction-payment/skip-feeless-payment/src/tests.rs @@ -0,0 +1,33 @@ +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use super::*; +use crate::mock::{pallet_dummy::Call, DummyExtension, PreDispatchCount, Runtime, RuntimeCall}; +use frame_support::dispatch::DispatchInfo; + +#[test] +fn skip_feeless_payment_works() { + let call = RuntimeCall::DummyPallet(Call::::aux { data: 1 }); + SkipCheckIfFeeless::::from(DummyExtension) + .pre_dispatch(&0, &call, &DispatchInfo::default(), 0) + .unwrap(); + assert_eq!(PreDispatchCount::get(), 1); + + let call = RuntimeCall::DummyPallet(Call::::aux { data: 0 }); + SkipCheckIfFeeless::::from(DummyExtension) + .pre_dispatch(&0, &call, &DispatchInfo::default(), 0) + .unwrap(); + assert_eq!(PreDispatchCount::get(), 1); +} From 84bb8fa0d625efecb6efaa11dccd7996f4548561 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Tue, 24 Oct 2023 09:58:08 +0530 Subject: [PATCH 13/41] Adds UI Tests for feeless args --- .../procedural/src/pallet/parse/call.rs | 61 +++++++++++++++++++ .../call_feeless_invalid_closure_arg1.rs | 37 +++++++++++ .../call_feeless_invalid_closure_arg1.stderr | 5 ++ .../call_feeless_invalid_closure_arg2.rs | 37 +++++++++++ .../call_feeless_invalid_closure_arg2.stderr | 11 ++++ .../call_feeless_invalid_closure_arg3.rs | 38 ++++++++++++ .../call_feeless_invalid_closure_arg3.stderr | 15 +++++ 7 files changed, 204 insertions(+) create mode 100644 substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg1.rs create mode 100644 substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg1.stderr create mode 100644 substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg2.rs create mode 100644 substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg2.stderr create mode 100644 substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg3.rs create mode 100644 substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg3.stderr diff --git a/substrate/frame/support/procedural/src/pallet/parse/call.rs b/substrate/frame/support/procedural/src/pallet/parse/call.rs index b9716184f3b8..afbd1392461d 100644 --- a/substrate/frame/support/procedural/src/pallet/parse/call.rs +++ b/substrate/frame/support/procedural/src/pallet/parse/call.rs @@ -31,6 +31,7 @@ mod keyword { syn::custom_keyword!(T); syn::custom_keyword!(pallet); syn::custom_keyword!(feeless_if); + syn::custom_keyword!(AccountIdFor); } /// Definition of dispatchables typically `impl Pallet { ... }` @@ -172,6 +173,31 @@ pub fn check_dispatchable_first_arg_type(ty: &syn::Type) -> syn::Result<()> { Ok(()) } +/// Check the syntax is `AccountIdFor` +pub fn check_feeless_first_arg_type(ty: &syn::Type) -> syn::Result<()> { + pub struct CheckFeelessFirstArg; + impl syn::parse::Parse for CheckFeelessFirstArg { + fn parse(input: syn::parse::ParseStream) -> syn::Result { + input.parse::()?; + input.parse::()?; + input.parse::()?; + input.parse::()?; + input.parse::]>()?; + + Ok(Self) + } + } + + syn::parse2::(ty.to_token_stream()).map_err(|e| { + let msg = "Invalid type: expected `&AccountIdFor`"; + let mut err = syn::Error::new(ty.span(), msg); + err.combine(e); + err + })?; + + Ok(()) +} + impl CallDef { pub fn try_from( attr_span: proc_macro2::Span, @@ -349,6 +375,41 @@ impl CallDef { _ => unreachable!("checked during creation of the let binding"), }); + if let Some(ref feeless_check) = feeless_check { + if feeless_check.inputs.len() != args.len() + 1 { + let msg = "Invalid pallet::call, feeless_if closure must have same \ + number of arguments as the dispatchable function"; + return Err(syn::Error::new(feeless_check.span(), msg)) + } + + match feeless_check.inputs.first() { + Some(syn::Pat::Type(arg)) => { + check_feeless_first_arg_type(&arg.ty)?; + }, + _ => { + let msg = "Invalid pallet::call, feeless_if closure's first argument must be a typed argument, \ + e.g. `who: &AccountIdFor`"; + return Err(syn::Error::new(feeless_check.inputs.span(), msg)) + }, + } + + let valid_return = match &feeless_check.output { + syn::ReturnType::Type(_, type_) => { + match *(type_.clone()) { + syn::Type::Path(syn::TypePath { path, .. }) => { + path.is_ident("bool") + }, + _ => false + } + }, + _ => false + }; + if !valid_return { + let msg = "Invalid pallet::call, feeless_if closure must return `bool`"; + return Err(syn::Error::new(feeless_check.output.span(), msg)) + } + } + methods.push(CallVariantDef { name: method.sig.ident.clone(), weight, diff --git a/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg1.rs b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg1.rs new file mode 100644 index 000000000000..08aaf06a7ef2 --- /dev/null +++ b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg1.rs @@ -0,0 +1,37 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#[frame_support::pallet(dev_mode)] +mod pallet { + use frame_support::pallet_prelude::DispatchResult; + use frame_system::pallet_prelude::OriginFor; + + #[pallet::config] + pub trait Config: frame_system::Config {} + + #[pallet::pallet] + pub struct Pallet(core::marker::PhantomData); + + #[pallet::call] + impl Pallet { + #[pallet::feeless_if(|| -> bool { true })] + pub fn foo(_: OriginFor) -> DispatchResult { Ok(()) } + } +} + +fn main() { +} diff --git a/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg1.stderr b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg1.stderr new file mode 100644 index 000000000000..9c13d59d7932 --- /dev/null +++ b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg1.stderr @@ -0,0 +1,5 @@ +error: Invalid pallet::call, feeless_if closure must have same number of arguments as the dispatchable function + --> tests/pallet_ui/call_feeless_invalid_closure_arg1.rs:31:24 + | +31 | #[pallet::feeless_if(|| -> bool { true })] + | ^ diff --git a/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg2.rs b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg2.rs new file mode 100644 index 000000000000..b16b4b3ffd94 --- /dev/null +++ b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg2.rs @@ -0,0 +1,37 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#[frame_support::pallet(dev_mode)] +mod pallet { + use frame_support::pallet_prelude::DispatchResult; + use frame_system::pallet_prelude::OriginFor; + + #[pallet::config] + pub trait Config: frame_system::Config {} + + #[pallet::pallet] + pub struct Pallet(core::marker::PhantomData); + + #[pallet::call] + impl Pallet { + #[pallet::feeless_if(|_: bool| -> bool { true })] + pub fn foo(_: OriginFor) -> DispatchResult { Ok(()) } + } +} + +fn main() { +} diff --git a/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg2.stderr b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg2.stderr new file mode 100644 index 000000000000..e07d67c9b6f0 --- /dev/null +++ b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg2.stderr @@ -0,0 +1,11 @@ +error: Invalid type: expected `&AccountIdFor` + --> tests/pallet_ui/call_feeless_invalid_closure_arg2.rs:31:28 + | +31 | #[pallet::feeless_if(|_: bool| -> bool { true })] + | ^^^^ + +error: expected `&` + --> tests/pallet_ui/call_feeless_invalid_closure_arg2.rs:31:28 + | +31 | #[pallet::feeless_if(|_: bool| -> bool { true })] + | ^^^^ diff --git a/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg3.rs b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg3.rs new file mode 100644 index 000000000000..1fe3d4b779ab --- /dev/null +++ b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg3.rs @@ -0,0 +1,38 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#[frame_support::pallet(dev_mode)] +mod pallet { + use frame_support::pallet_prelude::DispatchResult; + use frame_system::pallet_prelude::OriginFor; + use frame_system::pallet_prelude::AccountIdFor; + + #[pallet::config] + pub trait Config: frame_system::Config {} + + #[pallet::pallet] + pub struct Pallet(core::marker::PhantomData); + + #[pallet::call] + impl Pallet { + #[pallet::feeless_if(|_: &AccountIdFor, _s: &u32| -> bool { true })] + pub fn foo(_: OriginFor, _something: u64) -> DispatchResult { Ok(()) } + } +} + +fn main() { +} diff --git a/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg3.stderr b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg3.stderr new file mode 100644 index 000000000000..eda9f2694c05 --- /dev/null +++ b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg3.stderr @@ -0,0 +1,15 @@ +error[E0308]: mismatched types + --> tests/pallet_ui/call_feeless_invalid_closure_arg3.rs:33:31 + | +32 | #[pallet::feeless_if(|_: &AccountIdFor, _s: &u32| -> bool { true })] + | ------------------------------------------------ arguments to this function are incorrect +33 | pub fn foo(_: OriginFor, _something: u64) -> DispatchResult { Ok(()) } + | ^^^^^^^^^^ expected `&u32`, found `&u64` + | + = note: expected reference `&u32` + found reference `&u64` +note: closure parameter defined here + --> tests/pallet_ui/call_feeless_invalid_closure_arg3.rs:32:46 + | +32 | #[pallet::feeless_if(|_: &AccountIdFor, _s: &u32| -> bool { true })] + | ^^^^^^^^ From 948207a60a34c80c4e4d6856f5d02b4b69f6c199 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Tue, 24 Oct 2023 10:49:52 +0530 Subject: [PATCH 14/41] Adds UI Tests for feeless input type --- .../pallet_ui/call_feeless_invalid_type.rs | 37 +++++++++++++++++++ .../call_feeless_invalid_type.stderr | 5 +++ 2 files changed, 42 insertions(+) create mode 100644 substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_type.rs create mode 100644 substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_type.stderr diff --git a/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_type.rs b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_type.rs new file mode 100644 index 000000000000..26bd8a600ab9 --- /dev/null +++ b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_type.rs @@ -0,0 +1,37 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#[frame_support::pallet(dev_mode)] +mod pallet { + use frame_support::pallet_prelude::DispatchResult; + use frame_system::pallet_prelude::OriginFor; + + #[pallet::config] + pub trait Config: frame_system::Config {} + + #[pallet::pallet] + pub struct Pallet(core::marker::PhantomData); + + #[pallet::call] + impl Pallet { + #[pallet::feeless_if(0)] + pub fn foo(_: OriginFor) -> DispatchResult { Ok(()) } + } +} + +fn main() { +} diff --git a/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_type.stderr b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_type.stderr new file mode 100644 index 000000000000..beb19fdb8fad --- /dev/null +++ b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_type.stderr @@ -0,0 +1,5 @@ +error: expected `|` + --> tests/pallet_ui/call_feeless_invalid_type.rs:31:24 + | +31 | #[pallet::feeless_if(0)] + | ^ From be7a0fc27a82881455ec2177e365def6c2d9b2a2 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Tue, 24 Oct 2023 12:03:10 +0530 Subject: [PATCH 15/41] Adds UI Tests for feeless return type --- .../procedural/src/pallet/parse/call.rs | 12 ++---- .../call_feeless_invalid_closure_return.rs | 38 +++++++++++++++++++ ...call_feeless_invalid_closure_return.stderr | 5 +++ 3 files changed, 47 insertions(+), 8 deletions(-) create mode 100644 substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_return.rs create mode 100644 substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_return.stderr diff --git a/substrate/frame/support/procedural/src/pallet/parse/call.rs b/substrate/frame/support/procedural/src/pallet/parse/call.rs index afbd1392461d..5314535c3875 100644 --- a/substrate/frame/support/procedural/src/pallet/parse/call.rs +++ b/substrate/frame/support/procedural/src/pallet/parse/call.rs @@ -394,15 +394,11 @@ impl CallDef { } let valid_return = match &feeless_check.output { - syn::ReturnType::Type(_, type_) => { - match *(type_.clone()) { - syn::Type::Path(syn::TypePath { path, .. }) => { - path.is_ident("bool") - }, - _ => false - } + syn::ReturnType::Type(_, type_) => match *(type_.clone()) { + syn::Type::Path(syn::TypePath { path, .. }) => path.is_ident("bool"), + _ => false, }, - _ => false + _ => false, }; if !valid_return { let msg = "Invalid pallet::call, feeless_if closure must return `bool`"; diff --git a/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_return.rs b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_return.rs new file mode 100644 index 000000000000..d408449e97ec --- /dev/null +++ b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_return.rs @@ -0,0 +1,38 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#[frame_support::pallet(dev_mode)] +mod pallet { + use frame_support::pallet_prelude::DispatchResult; + use frame_system::pallet_prelude::OriginFor; + use frame_system::pallet_prelude::AccountIdFor; + + #[pallet::config] + pub trait Config: frame_system::Config {} + + #[pallet::pallet] + pub struct Pallet(core::marker::PhantomData); + + #[pallet::call] + impl Pallet { + #[pallet::feeless_if(|_: &AccountIdFor| -> u32 { 0 })] + pub fn foo(_: OriginFor) -> DispatchResult { Ok(()) } + } +} + +fn main() { +} diff --git a/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_return.stderr b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_return.stderr new file mode 100644 index 000000000000..681064279294 --- /dev/null +++ b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_return.stderr @@ -0,0 +1,5 @@ +error: Invalid pallet::call, feeless_if closure must return `bool` + --> tests/pallet_ui/call_feeless_invalid_closure_return.rs:32:46 + | +32 | #[pallet::feeless_if(|_: &AccountIdFor| -> u32 { 0 })] + | ^ From 23cfd9153b54a5de1087673c318eed406052b176 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Tue, 24 Oct 2023 14:19:21 +0530 Subject: [PATCH 16/41] Adds UI Test for passing case --- .../test/tests/pallet_ui/pass/feeless_call.rs | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 substrate/frame/support/test/tests/pallet_ui/pass/feeless_call.rs diff --git a/substrate/frame/support/test/tests/pallet_ui/pass/feeless_call.rs b/substrate/frame/support/test/tests/pallet_ui/pass/feeless_call.rs new file mode 100644 index 000000000000..12b9aeaa75f1 --- /dev/null +++ b/substrate/frame/support/test/tests/pallet_ui/pass/feeless_call.rs @@ -0,0 +1,38 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#[frame_support::pallet(dev_mode)] +mod pallet { + use frame_support::pallet_prelude::DispatchResult; + use frame_system::pallet_prelude::OriginFor; + use frame_system::pallet_prelude::AccountIdFor; + + #[pallet::config] + pub trait Config: frame_system::Config {} + + #[pallet::pallet] + pub struct Pallet(core::marker::PhantomData); + + #[pallet::call] + impl Pallet { + #[pallet::feeless_if(|_: &AccountIdFor| -> bool { true })] + pub fn foo(_: OriginFor) -> DispatchResult { Ok(()) } + } +} + +fn main() { +} From e87a1395ad523e6386aa4d9cb13c7814a27e1f44 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Wed, 25 Oct 2023 10:20:41 +0530 Subject: [PATCH 17/41] Moves feeless extension to kitchensink --- Cargo.lock | 2 +- substrate/bin/node-template/node/Cargo.toml | 3 --- .../bin/node-template/node/src/benchmarking.rs | 4 +--- .../bin/node-template/pallets/template/src/lib.rs | 3 --- substrate/bin/node-template/runtime/Cargo.toml | 3 --- substrate/bin/node-template/runtime/src/lib.rs | 10 +--------- substrate/bin/node/runtime/Cargo.toml | 4 ++++ substrate/bin/node/runtime/src/lib.rs | 14 ++++++++++++-- 8 files changed, 19 insertions(+), 24 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 82828c84dfdc..32059a535f11 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6898,6 +6898,7 @@ dependencies = [ "pallet-scheduler", "pallet-session", "pallet-session-benchmarking", + "pallet-skip-feeless-payment", "pallet-society", "pallet-staking", "pallet-staking-reward-curve", @@ -8519,7 +8520,6 @@ dependencies = [ "futures", "jsonrpsee", "node-template-runtime", - "pallet-skip-feeless-payment", "pallet-transaction-payment", "pallet-transaction-payment-rpc", "sc-basic-authorship", diff --git a/substrate/bin/node-template/node/Cargo.toml b/substrate/bin/node-template/node/Cargo.toml index a25cd3ec655f..23840cce2229 100644 --- a/substrate/bin/node-template/node/Cargo.toml +++ b/substrate/bin/node-template/node/Cargo.toml @@ -42,7 +42,6 @@ sp-inherents = { path = "../../../primitives/inherents" } sp-keyring = { path = "../../../primitives/keyring" } frame-system = { path = "../../../frame/system" } pallet-transaction-payment = { path = "../../../frame/transaction-payment", default-features = false} -pallet-skip-feeless-payment = { path = "../../../frame/transaction-payment/skip-feeless-payment", default-features = false} # These dependencies are used for the node template's RPCs jsonrpsee = { version = "0.16.2", features = ["server"] } @@ -75,7 +74,6 @@ runtime-benchmarks = [ "frame-benchmarking/runtime-benchmarks", "frame-system/runtime-benchmarks", "node-template-runtime/runtime-benchmarks", - "pallet-skip-feeless-payment/runtime-benchmarks", "sc-service/runtime-benchmarks", "sp-runtime/runtime-benchmarks", ] @@ -84,7 +82,6 @@ runtime-benchmarks = [ try-runtime = [ "frame-system/try-runtime", "node-template-runtime/try-runtime", - "pallet-skip-feeless-payment/try-runtime", "pallet-transaction-payment/try-runtime", "sp-runtime/try-runtime", "try-runtime-cli/try-runtime", diff --git a/substrate/bin/node-template/node/src/benchmarking.rs b/substrate/bin/node-template/node/src/benchmarking.rs index 7c0b8cbb76b6..6e29ad1a1231 100644 --- a/substrate/bin/node-template/node/src/benchmarking.rs +++ b/substrate/bin/node-template/node/src/benchmarking.rs @@ -120,9 +120,7 @@ pub fn create_benchmark_extrinsic( )), frame_system::CheckNonce::::from(nonce), frame_system::CheckWeight::::new(), - pallet_skip_feeless_payment::SkipCheckIfFeeless::from( - pallet_transaction_payment::ChargeTransactionPayment::::from(0), - ), + pallet_transaction_payment::ChargeTransactionPayment::::from(0), ); let raw_payload = runtime::SignedPayload::from_raw( diff --git a/substrate/bin/node-template/pallets/template/src/lib.rs b/substrate/bin/node-template/pallets/template/src/lib.rs index a87b5224e124..4a2e53baa774 100644 --- a/substrate/bin/node-template/pallets/template/src/lib.rs +++ b/substrate/bin/node-template/pallets/template/src/lib.rs @@ -154,9 +154,6 @@ pub mod pallet { /// error if it isn't. Learn more about origins here: #[pallet::call_index(0)] #[pallet::weight(T::WeightInfo::do_something())] - #[pallet::feeless_if(|_who: &AccountIdFor, something: &u32| -> bool { - *something == 0 - })] pub fn do_something(origin: OriginFor, something: u32) -> DispatchResult { // Check that the extrinsic was signed and get the signer. let who = ensure_signed(origin)?; diff --git a/substrate/bin/node-template/runtime/Cargo.toml b/substrate/bin/node-template/runtime/Cargo.toml index 27a20da1d295..1db5b47a4b6c 100644 --- a/substrate/bin/node-template/runtime/Cargo.toml +++ b/substrate/bin/node-template/runtime/Cargo.toml @@ -70,7 +70,6 @@ std = [ "pallet-aura/std", "pallet-balances/std", "pallet-grandpa/std", - "pallet-skip-feeless-payment/std", "pallet-sudo/std", "pallet-template/std", "pallet-timestamp/std", @@ -100,7 +99,6 @@ runtime-benchmarks = [ "frame-system/runtime-benchmarks", "pallet-balances/runtime-benchmarks", "pallet-grandpa/runtime-benchmarks", - "pallet-skip-feeless-payment/runtime-benchmarks", "pallet-sudo/runtime-benchmarks", "pallet-template/runtime-benchmarks", "pallet-timestamp/runtime-benchmarks", @@ -114,7 +112,6 @@ try-runtime = [ "pallet-aura/try-runtime", "pallet-balances/try-runtime", "pallet-grandpa/try-runtime", - "pallet-skip-feeless-payment/try-runtime", "pallet-sudo/try-runtime", "pallet-template/try-runtime", "pallet-timestamp/try-runtime", diff --git a/substrate/bin/node-template/runtime/src/lib.rs b/substrate/bin/node-template/runtime/src/lib.rs index 9c23ce039c29..4653b49bf2c3 100644 --- a/substrate/bin/node-template/runtime/src/lib.rs +++ b/substrate/bin/node-template/runtime/src/lib.rs @@ -267,10 +267,6 @@ impl pallet_transaction_payment::Config for Runtime { type FeeMultiplierUpdate = ConstFeeMultiplier; } -impl pallet_skip_feeless_payment::Config for Runtime { - type RuntimeEvent = RuntimeEvent; -} - impl pallet_sudo::Config for Runtime { type RuntimeEvent = RuntimeEvent; type RuntimeCall = RuntimeCall; @@ -292,7 +288,6 @@ construct_runtime!( Grandpa: pallet_grandpa, Balances: pallet_balances, TransactionPayment: pallet_transaction_payment, - SkipFeelessPayment: pallet_skip_feeless_payment, Sudo: pallet_sudo, // Include the custom logic from the pallet-template in the runtime. TemplateModule: pallet_template, @@ -314,10 +309,7 @@ pub type SignedExtra = ( frame_system::CheckEra, frame_system::CheckNonce, frame_system::CheckWeight, - pallet_skip_feeless_payment::SkipCheckIfFeeless< - Runtime, - pallet_transaction_payment::ChargeTransactionPayment, - >, + pallet_transaction_payment::ChargeTransactionPayment, ); /// Unchecked extrinsic type as expected by this runtime. diff --git a/substrate/bin/node/runtime/Cargo.toml b/substrate/bin/node/runtime/Cargo.toml index bf6c540774c1..ff88f6f74fca 100644 --- a/substrate/bin/node/runtime/Cargo.toml +++ b/substrate/bin/node/runtime/Cargo.toml @@ -128,6 +128,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} @@ -211,6 +212,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", @@ -305,6 +307,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", @@ -378,6 +381,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 index 2070e3f12d04..5d7a3a217c95 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -567,6 +567,10 @@ impl pallet_asset_conversion_tx_payment::Config for Runtime { pallet_asset_conversion_tx_payment::AssetConversionAdapter; } +impl pallet_skip_feeless_payment::Config for Runtime { + type RuntimeEvent = RuntimeEvent; +} + parameter_types! { pub const MinimumPeriod: Moment = SLOT_DURATION / 2; } @@ -1392,7 +1396,9 @@ where frame_system::CheckEra::::from(era), frame_system::CheckNonce::::from(nonce), frame_system::CheckWeight::::new(), - pallet_asset_conversion_tx_payment::ChargeAssetTxPayment::::from(tip, None), + pallet_skip_feeless_payment::SkipCheckIfFeeless::from( + pallet_asset_conversion_tx_payment::ChargeAssetTxPayment::::from(tip, None), + ), ); let raw_payload = SignedPayload::new(call, extra) .map_err(|e| { @@ -2129,6 +2135,7 @@ construct_runtime!( Statement: pallet_statement, Broker: pallet_broker, Mixnet: pallet_mixnet, + SkipFeelessPayment: pallet_skip_feeless_payment, } ); @@ -2155,7 +2162,10 @@ pub type SignedExtra = ( frame_system::CheckEra, frame_system::CheckNonce, frame_system::CheckWeight, - pallet_asset_conversion_tx_payment::ChargeAssetTxPayment, + pallet_skip_feeless_payment::SkipCheckIfFeeless< + Runtime, + pallet_asset_conversion_tx_payment::ChargeAssetTxPayment, + >, ); /// Unchecked extrinsic type as expected by this runtime. From 1c95fdf218a74ebe797b6e69d52a7ea81611ffb9 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Wed, 25 Oct 2023 10:35:06 +0530 Subject: [PATCH 18/41] Fixes node-testing build --- Cargo.lock | 1 + substrate/bin/node/testing/Cargo.toml | 1 + substrate/bin/node/testing/src/keyring.rs | 4 +++- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 32059a535f11..3a56734dce9a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8620,6 +8620,7 @@ dependencies = [ "pallet-asset-conversion-tx-payment", "pallet-asset-tx-payment", "pallet-assets", + "pallet-skip-feeless-payment", "parity-scale-codec", "sc-block-builder", "sc-client-api", diff --git a/substrate/bin/node/testing/Cargo.toml b/substrate/bin/node/testing/Cargo.toml index f5a39693301c..048782a11471 100644 --- 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 index 22a8f5deb19f..9940077c9da6 100644 --- 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), + ), ) } From ff860e6d5d60c28d3b828cf8254d0d155860888d Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Wed, 25 Oct 2023 10:58:25 +0530 Subject: [PATCH 19/41] Fixes node-cli build --- Cargo.lock | 1 + substrate/bin/node/cli/Cargo.toml | 3 +++ substrate/bin/node/cli/src/service.rs | 6 ++++-- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8c500c17d4df..f8f198909176 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8338,6 +8338,7 @@ dependencies = [ "pallet-assets", "pallet-balances", "pallet-im-online", + "pallet-skip-feeless-payment", "pallet-timestamp", "parity-scale-codec", "platforms", diff --git a/substrate/bin/node/cli/Cargo.toml b/substrate/bin/node/cli/Cargo.toml index 49dc39099be0..a1404b6dd5db 100644 --- 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" } @@ -169,6 +170,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", @@ -184,6 +186,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 index 5a85f4cde0ae..ff04f929c5d0 100644 --- a/substrate/bin/node/cli/src/service.rs +++ b/substrate/bin/node/cli/src/service.rs @@ -102,8 +102,10 @@ pub fn create_extrinsic( )), frame_system::CheckNonce::::from(nonce), frame_system::CheckWeight::::new(), - pallet_asset_conversion_tx_payment::ChargeAssetTxPayment::::from( - tip, None, + pallet_skip_feeless_payment::SkipCheckIfFeeless::from( + pallet_asset_conversion_tx_payment::ChargeAssetTxPayment::::from( + tip, None, + ), ), ); From a9074a5b79101d8483a88f243a6b9f7716905f41 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Wed, 25 Oct 2023 10:58:46 +0530 Subject: [PATCH 20/41] Adds feeless macro in kitchensink example --- substrate/frame/examples/kitchensink/src/lib.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/substrate/frame/examples/kitchensink/src/lib.rs b/substrate/frame/examples/kitchensink/src/lib.rs index 56117c59dc6d..f89afa0cc9b5 100644 --- a/substrate/frame/examples/kitchensink/src/lib.rs +++ b/substrate/frame/examples/kitchensink/src/lib.rs @@ -206,6 +206,10 @@ pub mod pallet { impl Pallet { #[pallet::call_index(0)] #[pallet::weight(T::WeightInfo::set_foo_benchmark())] + /// Marks this call as feeless if `new_foo` is zero. + #[pallet::feeless_if(|_who: &AccountIdFor, new_foo: &u32, _other_compact: &u128| -> bool { + *new_foo == 0 + })] pub fn set_foo( _: OriginFor, new_foo: u32, From 6706bc29a9c5ef832312c07fda2b4e6ee2205153 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Wed, 25 Oct 2023 10:59:36 +0530 Subject: [PATCH 21/41] Fmt --- substrate/bin/node/cli/src/service.rs | 33 ++++++++++++++------------- substrate/bin/node/runtime/src/lib.rs | 4 +++- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/substrate/bin/node/cli/src/service.rs b/substrate/bin/node/cli/src/service.rs index ff04f929c5d0..87b17ef5a9e7 100644 --- a/substrate/bin/node/cli/src/service.rs +++ b/substrate/bin/node/cli/src/service.rs @@ -91,23 +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::::new(), - frame_system::CheckSpecVersion::::new(), - frame_system::CheckTxVersion::::new(), - frame_system::CheckGenesis::::new(), - frame_system::CheckEra::::from(generic::Era::mortal( - period, - best_block.saturated_into(), - )), - frame_system::CheckNonce::::from(nonce), - frame_system::CheckWeight::::new(), - pallet_skip_feeless_payment::SkipCheckIfFeeless::from( - pallet_asset_conversion_tx_payment::ChargeAssetTxPayment::::from( - tip, None, + let extra: kitchensink_runtime::SignedExtra = + ( + frame_system::CheckNonZeroSender::::new(), + frame_system::CheckSpecVersion::::new(), + frame_system::CheckTxVersion::::new(), + frame_system::CheckGenesis::::new(), + frame_system::CheckEra::::from(generic::Era::mortal( + period, + best_block.saturated_into(), + )), + frame_system::CheckNonce::::from(nonce), + frame_system::CheckWeight::::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(), diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index a59658857384..985b8823deb5 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -1399,7 +1399,9 @@ where frame_system::CheckNonce::::from(nonce), frame_system::CheckWeight::::new(), pallet_skip_feeless_payment::SkipCheckIfFeeless::from( - pallet_asset_conversion_tx_payment::ChargeAssetTxPayment::::from(tip, None), + pallet_asset_conversion_tx_payment::ChargeAssetTxPayment::::from( + tip, None, + ), ), ); let raw_payload = SignedPayload::new(call, extra) From edaf143131929e2ec7eb1bc3681acb1d88fe8a75 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Wed, 25 Oct 2023 11:10:32 +0530 Subject: [PATCH 22/41] fixes build --- substrate/bin/node/cli/src/service.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/substrate/bin/node/cli/src/service.rs b/substrate/bin/node/cli/src/service.rs index 87b17ef5a9e7..40345e580983 100644 --- a/substrate/bin/node/cli/src/service.rs +++ b/substrate/bin/node/cli/src/service.rs @@ -878,8 +878,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, From 9987b88727e0b38f92404a28bf3fe6d2aff26226 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Wed, 25 Oct 2023 11:17:59 +0530 Subject: [PATCH 23/41] Adds docs --- substrate/frame/support/procedural/src/lib.rs | 27 +++++++++++++++++++ substrate/frame/support/src/dispatch.rs | 9 ++++++- substrate/frame/support/src/lib.rs | 8 +++--- .../skip-feeless-payment/src/lib.rs | 21 +++++++++++++++ 4 files changed, 60 insertions(+), 5 deletions(-) diff --git a/substrate/frame/support/procedural/src/lib.rs b/substrate/frame/support/procedural/src/lib.rs index 9c551b9f2306..c0ace47ee186 100644 --- a/substrate/frame/support/procedural/src/lib.rs +++ b/substrate/frame/support/procedural/src/lib.rs @@ -1142,6 +1142,33 @@ pub fn call_index(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() } +/// Each dispatchable may also be annotated with the `#[pallet::feeless_if($closure)]` attribute, +/// which explicitly defines the condition for the dispatchable function to be feeless. +/// +/// The first argument for the closure must be `account_id: &AccountIdFor`, while the rest +/// should be the references to arguments of the dispatchable function. +/// +/// The closure must return `bool`. +/// +/// ### Example +/// ```ignore +/// #[pallet::feeless_if(|_who: &AccountIdFor, something: &u32| -> bool { +/// *something == 0 +/// })] +/// pub fn do_something(origin: OriginFor, something: u32) -> DispatchResult { +/// .... +/// } +/// ``` +/// +/// ### 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/src/dispatch.rs b/substrate/frame/support/src/dispatch.rs index 6c933fff8fc9..0e5afd2a9aab 100644 --- a/substrate/frame/support/src/dispatch.rs +++ b/substrate/frame/support/src/dispatch.rs @@ -54,10 +54,17 @@ pub trait Callable { // https://github.com/rust-lang/rust/issues/51331 pub type CallableCallFor = >::RuntimeCall; +/// Means to checks if the dispatchable is feeless. +/// This is automatically implemented for all dispatchables during pallet expansion. +/// If a call is marked by [`#[pallet::feeless_if]`](`macro@frame_support_procedural::feeless_if`) +/// attribute, the corresponding closure is checked. pub trait CheckIfFeeless { + /// The account id type of the runtime. type AccountId; - fn is_feeless(&self, account_d: &Self::AccountId) -> bool; + /// Checks if the dispatchable satisfies the feeless condition as defined by + /// [`#[pallet::feeless_if]`](`macro@frame_support_procedural::feeless_if`) + fn is_feeless(&self, account_id: &Self::AccountId) -> bool; } /// Origin for the System pallet. diff --git a/substrate/frame/support/src/lib.rs b/substrate/frame/support/src/lib.rs index f54252ff9d61..c04bfcdc3185 100644 --- a/substrate/frame/support/src/lib.rs +++ b/substrate/frame/support/src/lib.rs @@ -2196,10 +2196,10 @@ pub use frame_support_procedural::pallet; pub mod pallet_macros { pub use frame_support_procedural::{ call_index, compact, composite_enum, config, constant, - disable_frame_system_supertrait_check, error, event, extra_constants, generate_deposit, - generate_store, getter, hooks, import_section, inherent, no_default, no_default_bounds, - origin, pallet_section, storage, storage_prefix, storage_version, type_value, unbounded, - validate_unsigned, weight, whitelist_storage, + disable_frame_system_supertrait_check, error, event, extra_constants, feeless_if, + generate_deposit, generate_store, getter, hooks, import_section, inherent, no_default, + no_default_bounds, origin, pallet_section, storage, storage_prefix, storage_version, + type_value, unbounded, validate_unsigned, weight, whitelist_storage, }; /// Allows you to define the genesis configuration for the pallet. diff --git a/substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs b/substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs index eb1f0184ba6b..77151470f362 100644 --- a/substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs +++ b/substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs @@ -13,6 +13,26 @@ // See the License for the specific language governing permissions and // limitations under the License. +//! # Skip Feeless Payment Pallet +//! +//! This pallet allows runtimes that include it to skip payment of transaction fees for +//! dispatchables marked by [`#[pallet::feeless_if]`](`macro@ +//! frame_support::pallet_prelude::feeless_if`). +//! +//! ## Overview + +//! It does this by wrapping an existing [`SignedExtension`] implementation (e.g. +//! [`pallet-transaction-payment`]) and checking if the dispatchable is feeless before applying the +//! wrapped extension. If the dispatchable is indeed feeless, the extension is skipped and a custom +//! event is emitted instead. Otherwise, the extension is applied as usual. +//! +//! +//! ## Integration + +//! This pallet wraps an existing transaction payment pallet. This means you should both pallets +//! in your `construct_runtime` macro and include this pallet's +//! [`SignedExtension`] ([`SkipCheckIfFeeless`]) that would accept the existing one as an argument. + #![cfg_attr(not(feature = "std"), no_std)] use codec::{Decode, Encode}; @@ -54,6 +74,7 @@ pub mod pallet { } } +/// A [`SignedExtension`] that skips the wrapped extension if the dispatchable is feeless. #[derive(Encode, Decode, Clone, Eq, PartialEq, TypeInfo)] #[scale_info(skip_type_params(T))] pub struct SkipCheckIfFeeless(pub S, sp_std::marker::PhantomData); From 864b50c149ef4a5f8a138af60b6c78666d61630c Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Wed, 25 Oct 2023 11:40:25 +0530 Subject: [PATCH 24/41] Adds prdoc --- prdoc/pr_1926.prdoc | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 prdoc/pr_1926.prdoc diff --git a/prdoc/pr_1926.prdoc b/prdoc/pr_1926.prdoc new file mode 100644 index 000000000000..3e456f75d94f --- /dev/null +++ b/prdoc/pr_1926.prdoc @@ -0,0 +1,29 @@ +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 that wraps a transaction + payment processor to skip payment fess for all such calls. + +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: [] From 646fe0f5609e04fed07b472a304f462770239cc0 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Wed, 25 Oct 2023 11:52:37 +0530 Subject: [PATCH 25/41] Minor fix --- Cargo.lock | 1 - substrate/bin/node-template/runtime/Cargo.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f8f198909176..8850cbbb302e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8578,7 +8578,6 @@ dependencies = [ "pallet-aura", "pallet-balances", "pallet-grandpa", - "pallet-skip-feeless-payment", "pallet-sudo", "pallet-template", "pallet-timestamp", diff --git a/substrate/bin/node-template/runtime/Cargo.toml b/substrate/bin/node-template/runtime/Cargo.toml index 1db5b47a4b6c..c6d751484237 100644 --- a/substrate/bin/node-template/runtime/Cargo.toml +++ b/substrate/bin/node-template/runtime/Cargo.toml @@ -25,7 +25,6 @@ frame-system = { path = "../../../frame/system", default-features = false} frame-try-runtime = { path = "../../../frame/try-runtime", default-features = false, optional = true } pallet-timestamp = { path = "../../../frame/timestamp", default-features = false} pallet-transaction-payment = { path = "../../../frame/transaction-payment", default-features = false} -pallet-skip-feeless-payment = { path = "../../../frame/transaction-payment/skip-feeless-payment", default-features = false} frame-executive = { path = "../../../frame/executive", default-features = false} sp-api = { path = "../../../primitives/api", default-features = false} sp-block-builder = { path = "../../../primitives/block-builder", default-features = false} From 5049e9423811d966b8044711b31e112cb05ba1cc Mon Sep 17 00:00:00 2001 From: gupnik <17176722+gupnik@users.noreply.github.com> Date: Fri, 3 Nov 2023 12:10:38 +0100 Subject: [PATCH 26/41] Update substrate/frame/transaction-payment/skip-feeless-payment/Cargo.toml Co-authored-by: Oliver Tale-Yazdi --- .../frame/transaction-payment/skip-feeless-payment/Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/frame/transaction-payment/skip-feeless-payment/Cargo.toml b/substrate/frame/transaction-payment/skip-feeless-payment/Cargo.toml index 3673d5276abe..7f60053717eb 100644 --- a/substrate/frame/transaction-payment/skip-feeless-payment/Cargo.toml +++ b/substrate/frame/transaction-payment/skip-feeless-payment/Cargo.toml @@ -3,8 +3,8 @@ name = "pallet-skip-feeless-payment" version = "4.0.0-dev" authors.workspace = true edition.workspace = true -license = "Apache-2.0" -homepage = "https://substrate.io" +license.workspace = true +homepage.workspace = true repository.workspace = true description = "pallet to skip feeless payments" readme = "README.md" From 649708a9725282f0242a7d6b9786c141587de589 Mon Sep 17 00:00:00 2001 From: gupnik <17176722+gupnik@users.noreply.github.com> Date: Fri, 3 Nov 2023 12:10:53 +0100 Subject: [PATCH 27/41] Update substrate/frame/transaction-payment/skip-feeless-payment/Cargo.toml Co-authored-by: Oliver Tale-Yazdi --- .../frame/transaction-payment/skip-feeless-payment/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/transaction-payment/skip-feeless-payment/Cargo.toml b/substrate/frame/transaction-payment/skip-feeless-payment/Cargo.toml index 7f60053717eb..a129f3968572 100644 --- a/substrate/frame/transaction-payment/skip-feeless-payment/Cargo.toml +++ b/substrate/frame/transaction-payment/skip-feeless-payment/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pallet-skip-feeless-payment" -version = "4.0.0-dev" +version = "1.0.0-dev" authors.workspace = true edition.workspace = true license.workspace = true From b12fa28470145b9df1f4210e46f64de691eb6ed0 Mon Sep 17 00:00:00 2001 From: gupnik <17176722+gupnik@users.noreply.github.com> Date: Fri, 3 Nov 2023 12:11:09 +0100 Subject: [PATCH 28/41] Update substrate/frame/transaction-payment/skip-feeless-payment/Cargo.toml Co-authored-by: Oliver Tale-Yazdi --- .../frame/transaction-payment/skip-feeless-payment/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/substrate/frame/transaction-payment/skip-feeless-payment/Cargo.toml b/substrate/frame/transaction-payment/skip-feeless-payment/Cargo.toml index a129f3968572..0e42bdba24c1 100644 --- a/substrate/frame/transaction-payment/skip-feeless-payment/Cargo.toml +++ b/substrate/frame/transaction-payment/skip-feeless-payment/Cargo.toml @@ -7,7 +7,6 @@ license.workspace = true homepage.workspace = true repository.workspace = true description = "pallet to skip feeless payments" -readme = "README.md" [package.metadata.docs.rs] targets = ["x86_64-unknown-linux-gnu"] From c8f99a68bc7f655b843bb3ac225ae00d1ebf6339 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Tue, 7 Nov 2023 12:00:57 +0530 Subject: [PATCH 29/41] Addresses review comments --- Cargo.lock | 2 +- substrate/frame/support/procedural/src/lib.rs | 4 ++++ .../frame/transaction-payment/skip-feeless-payment/Cargo.toml | 3 +-- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8850cbbb302e..5678e1b3c851 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10541,7 +10541,7 @@ dependencies = [ [[package]] name = "pallet-skip-feeless-payment" -version = "4.0.0-dev" +version = "1.0.0-dev" dependencies = [ "frame-support", "frame-system", diff --git a/substrate/frame/support/procedural/src/lib.rs b/substrate/frame/support/procedural/src/lib.rs index c0ace47ee186..c867d7d7276f 100644 --- a/substrate/frame/support/procedural/src/lib.rs +++ b/substrate/frame/support/procedural/src/lib.rs @@ -1160,6 +1160,10 @@ pub fn call_index(_: TokenStream, _: TokenStream) -> TokenStream { /// } /// ``` /// +/// 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 diff --git a/substrate/frame/transaction-payment/skip-feeless-payment/Cargo.toml b/substrate/frame/transaction-payment/skip-feeless-payment/Cargo.toml index 0e42bdba24c1..cfb814e2e380 100644 --- a/substrate/frame/transaction-payment/skip-feeless-payment/Cargo.toml +++ b/substrate/frame/transaction-payment/skip-feeless-payment/Cargo.toml @@ -4,9 +4,8 @@ version = "1.0.0-dev" authors.workspace = true edition.workspace = true license.workspace = true -homepage.workspace = true repository.workspace = true -description = "pallet to skip feeless payments" +description = "Pallet to skip payments for calls annotated with `feeless_if` if the respective conditions are satisfied." [package.metadata.docs.rs] targets = ["x86_64-unknown-linux-gnu"] From c11a5e8a3da3ad6037eabc9bc72b67b4c27908b6 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Tue, 7 Nov 2023 12:39:59 +0530 Subject: [PATCH 30/41] Uses Origin as the first arg --- .../frame/examples/kitchensink/src/lib.rs | 2 +- .../src/construct_runtime/expand/call.rs | 6 +-- substrate/frame/support/procedural/src/lib.rs | 5 +-- .../procedural/src/pallet/expand/call.rs | 6 +-- .../procedural/src/pallet/parse/call.rs | 37 ------------------- substrate/frame/support/src/dispatch.rs | 8 ++-- .../support/src/storage/generator/mod.rs | 2 - substrate/frame/support/src/tests/mod.rs | 2 - substrate/frame/support/test/src/lib.rs | 3 -- .../call_feeless_invalid_closure_arg2.stderr | 20 ++++++---- .../call_feeless_invalid_closure_arg3.rs | 3 +- .../call_feeless_invalid_closure_arg3.stderr | 14 +++---- .../call_feeless_invalid_closure_return.rs | 3 +- ...call_feeless_invalid_closure_return.stderr | 6 +-- .../test/tests/pallet_ui/pass/feeless_call.rs | 3 +- substrate/frame/system/src/lib.rs | 3 -- .../skip-feeless-payment/src/lib.rs | 6 +-- .../skip-feeless-payment/src/mock.rs | 2 +- 18 files changed, 42 insertions(+), 89 deletions(-) diff --git a/substrate/frame/examples/kitchensink/src/lib.rs b/substrate/frame/examples/kitchensink/src/lib.rs index f89afa0cc9b5..89759dd0bf63 100644 --- a/substrate/frame/examples/kitchensink/src/lib.rs +++ b/substrate/frame/examples/kitchensink/src/lib.rs @@ -207,7 +207,7 @@ pub mod pallet { #[pallet::call_index(0)] #[pallet::weight(T::WeightInfo::set_foo_benchmark())] /// Marks this call as feeless if `new_foo` is zero. - #[pallet::feeless_if(|_who: &AccountIdFor, new_foo: &u32, _other_compact: &u128| -> bool { + #[pallet::feeless_if(|_origin: &OriginFor, new_foo: &u32, _other_compact: &u128| -> bool { *new_foo == 0 })] pub fn set_foo( diff --git a/substrate/frame/support/procedural/src/construct_runtime/expand/call.rs b/substrate/frame/support/procedural/src/construct_runtime/expand/call.rs index a9ca00a4e19b..ce2aa0942794 100644 --- a/substrate/frame/support/procedural/src/construct_runtime/expand/call.rs +++ b/substrate/frame/support/procedural/src/construct_runtime/expand/call.rs @@ -125,12 +125,12 @@ pub fn expand_outer_dispatch( } impl #scrate::dispatch::CheckIfFeeless for RuntimeCall { - type AccountId = #system_path::pallet_prelude::AccountIdFor<#runtime>; - fn is_feeless(&self, account_id: &Self::AccountId) -> bool { + 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(account_id), + #variant_patterns => call.is_feeless(origin), )* } } diff --git a/substrate/frame/support/procedural/src/lib.rs b/substrate/frame/support/procedural/src/lib.rs index 14b14313e2dc..052101306e0e 100644 --- a/substrate/frame/support/procedural/src/lib.rs +++ b/substrate/frame/support/procedural/src/lib.rs @@ -1160,14 +1160,13 @@ pub fn call_index(_: TokenStream, _: TokenStream) -> TokenStream { /// Each dispatchable may also be annotated with the `#[pallet::feeless_if($closure)]` attribute, /// which explicitly defines the condition for the dispatchable function to be feeless. /// -/// The first argument for the closure must be `account_id: &AccountIdFor`, while the rest -/// should be the references to arguments of the dispatchable function. +/// The arguments for the closure must be the references to arguments of the dispatchable function. /// /// The closure must return `bool`. /// /// ### Example /// ```ignore -/// #[pallet::feeless_if(|_who: &AccountIdFor, something: &u32| -> bool { +/// #[pallet::feeless_if(|_origin: &OriginFor, something: &u32| -> bool { /// *something == 0 /// })] /// pub fn do_something(origin: OriginFor, something: u32) -> DispatchResult { diff --git a/substrate/frame/support/procedural/src/pallet/expand/call.rs b/substrate/frame/support/procedural/src/pallet/expand/call.rs index effbf4f6cb02..cf302faafc78 100644 --- a/substrate/frame/support/procedural/src/pallet/expand/call.rs +++ b/substrate/frame/support/procedural/src/pallet/expand/call.rs @@ -245,7 +245,7 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream { 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(account_id, #( #arg_name, )*)) + quote::quote!(#feeless_check(origin, #( #arg_name, )*)) } else { quote::quote!(false) } @@ -360,9 +360,9 @@ 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 AccountId = #frame_system::pallet_prelude::AccountIdFor; + type Origin = #frame_system::pallet_prelude::OriginFor; #[allow(unused_variables)] - fn is_feeless(&self, account_id: &Self::AccountId) -> bool { + fn is_feeless(&self, origin: &Self::Origin) -> bool { match *self { #( Self::#fn_name { #( #args_name_pattern_ref, )* } => { diff --git a/substrate/frame/support/procedural/src/pallet/parse/call.rs b/substrate/frame/support/procedural/src/pallet/parse/call.rs index 5314535c3875..64fd95309eb4 100644 --- a/substrate/frame/support/procedural/src/pallet/parse/call.rs +++ b/substrate/frame/support/procedural/src/pallet/parse/call.rs @@ -31,7 +31,6 @@ mod keyword { syn::custom_keyword!(T); syn::custom_keyword!(pallet); syn::custom_keyword!(feeless_if); - syn::custom_keyword!(AccountIdFor); } /// Definition of dispatchables typically `impl Pallet { ... }` @@ -173,31 +172,6 @@ pub fn check_dispatchable_first_arg_type(ty: &syn::Type) -> syn::Result<()> { Ok(()) } -/// Check the syntax is `AccountIdFor` -pub fn check_feeless_first_arg_type(ty: &syn::Type) -> syn::Result<()> { - pub struct CheckFeelessFirstArg; - impl syn::parse::Parse for CheckFeelessFirstArg { - fn parse(input: syn::parse::ParseStream) -> syn::Result { - input.parse::()?; - input.parse::()?; - input.parse::()?; - input.parse::()?; - input.parse::]>()?; - - Ok(Self) - } - } - - syn::parse2::(ty.to_token_stream()).map_err(|e| { - let msg = "Invalid type: expected `&AccountIdFor`"; - let mut err = syn::Error::new(ty.span(), msg); - err.combine(e); - err - })?; - - Ok(()) -} - impl CallDef { pub fn try_from( attr_span: proc_macro2::Span, @@ -382,17 +356,6 @@ impl CallDef { return Err(syn::Error::new(feeless_check.span(), msg)) } - match feeless_check.inputs.first() { - Some(syn::Pat::Type(arg)) => { - check_feeless_first_arg_type(&arg.ty)?; - }, - _ => { - let msg = "Invalid pallet::call, feeless_if closure's first argument must be a typed argument, \ - e.g. `who: &AccountIdFor`"; - return Err(syn::Error::new(feeless_check.inputs.span(), msg)) - }, - } - let valid_return = match &feeless_check.output { syn::ReturnType::Type(_, type_) => match *(type_.clone()) { syn::Type::Path(syn::TypePath { path, .. }) => path.is_ident("bool"), diff --git a/substrate/frame/support/src/dispatch.rs b/substrate/frame/support/src/dispatch.rs index 0e5afd2a9aab..4c6b4c1beacd 100644 --- a/substrate/frame/support/src/dispatch.rs +++ b/substrate/frame/support/src/dispatch.rs @@ -59,12 +59,12 @@ pub type CallableCallFor = >::RuntimeCall; /// If a call is marked by [`#[pallet::feeless_if]`](`macro@frame_support_procedural::feeless_if`) /// attribute, the corresponding closure is checked. pub trait CheckIfFeeless { - /// The account id type of the runtime. - type AccountId; + /// The Origin type of the runtime. + type Origin; /// Checks if the dispatchable satisfies the feeless condition as defined by /// [`#[pallet::feeless_if]`](`macro@frame_support_procedural::feeless_if`) - fn is_feeless(&self, account_id: &Self::AccountId) -> bool; + fn is_feeless(&self, origin: &Self::Origin) -> bool; } /// Origin for the System pallet. @@ -754,8 +754,6 @@ mod weight_tests { pub mod pallet_prelude { pub type OriginFor = ::RuntimeOrigin; - pub type AccountIdFor = ::AccountId; - pub type HeaderFor = <::Block as sp_runtime::traits::HeaderProvider>::HeaderT; diff --git a/substrate/frame/support/src/storage/generator/mod.rs b/substrate/frame/support/src/storage/generator/mod.rs index 88191a7ac962..2b2abdc2e830 100644 --- a/substrate/frame/support/src/storage/generator/mod.rs +++ b/substrate/frame/support/src/storage/generator/mod.rs @@ -103,8 +103,6 @@ mod tests { pub mod pallet_prelude { pub type OriginFor = ::RuntimeOrigin; - pub type AccountIdFor = ::AccountId; - pub type HeaderFor = <::Block as sp_runtime::traits::HeaderProvider>::HeaderT; diff --git a/substrate/frame/support/src/tests/mod.rs b/substrate/frame/support/src/tests/mod.rs index bd09735f5d5f..3690159c5994 100644 --- a/substrate/frame/support/src/tests/mod.rs +++ b/substrate/frame/support/src/tests/mod.rs @@ -172,8 +172,6 @@ pub mod frame_system { pub mod pallet_prelude { pub type OriginFor = ::RuntimeOrigin; - pub type AccountIdFor = ::AccountId; - pub type HeaderFor = <::Block as sp_runtime::traits::HeaderProvider>::HeaderT; diff --git a/substrate/frame/support/test/src/lib.rs b/substrate/frame/support/test/src/lib.rs index 47e90fa18208..6b38d42d33d0 100644 --- a/substrate/frame/support/test/src/lib.rs +++ b/substrate/frame/support/test/src/lib.rs @@ -119,9 +119,6 @@ pub mod pallet_prelude { /// Type alias for the `Origin` associated type of system config. pub type OriginFor = ::RuntimeOrigin; - /// Type alias for the `AccountId` associated type of system config. - pub type AccountIdFor = ::AccountId; - /// Type alias for the `BlockNumber` associated type of system config. pub type BlockNumberFor = ::BlockNumber; } diff --git a/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg2.stderr b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg2.stderr index e07d67c9b6f0..627358a7ebbd 100644 --- a/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg2.stderr +++ b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg2.stderr @@ -1,11 +1,17 @@ -error: Invalid type: expected `&AccountIdFor` - --> tests/pallet_ui/call_feeless_invalid_closure_arg2.rs:31:28 +error[E0308]: mismatched types + --> tests/pallet_ui/call_feeless_invalid_closure_arg2.rs:18:1 | +18 | #[frame_support::pallet(dev_mode)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `bool`, found `&::RuntimeOrigin` +... 31 | #[pallet::feeless_if(|_: bool| -> bool { true })] - | ^^^^ - -error: expected `&` - --> tests/pallet_ui/call_feeless_invalid_closure_arg2.rs:31:28 + | -------------------------- arguments to this function are incorrect + | + = note: expected type `bool` + found reference `&::RuntimeOrigin` +note: closure parameter defined here + --> tests/pallet_ui/call_feeless_invalid_closure_arg2.rs:31:25 | 31 | #[pallet::feeless_if(|_: bool| -> bool { true })] - | ^^^^ + | ^^^^^^^ + = note: this error originates in the attribute macro `frame_support::pallet` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg3.rs b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg3.rs index 1fe3d4b779ab..5f2230744ff8 100644 --- a/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg3.rs +++ b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg3.rs @@ -19,7 +19,6 @@ mod pallet { use frame_support::pallet_prelude::DispatchResult; use frame_system::pallet_prelude::OriginFor; - use frame_system::pallet_prelude::AccountIdFor; #[pallet::config] pub trait Config: frame_system::Config {} @@ -29,7 +28,7 @@ mod pallet { #[pallet::call] impl Pallet { - #[pallet::feeless_if(|_: &AccountIdFor, _s: &u32| -> bool { true })] + #[pallet::feeless_if(|_: &OriginFor, _s: &u32| -> bool { true })] pub fn foo(_: OriginFor, _something: u64) -> DispatchResult { Ok(()) } } } diff --git a/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg3.stderr b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg3.stderr index eda9f2694c05..eaf4df140d98 100644 --- a/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg3.stderr +++ b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg3.stderr @@ -1,15 +1,15 @@ error[E0308]: mismatched types - --> tests/pallet_ui/call_feeless_invalid_closure_arg3.rs:33:31 + --> tests/pallet_ui/call_feeless_invalid_closure_arg3.rs:32:31 | -32 | #[pallet::feeless_if(|_: &AccountIdFor, _s: &u32| -> bool { true })] - | ------------------------------------------------ arguments to this function are incorrect -33 | pub fn foo(_: OriginFor, _something: u64) -> DispatchResult { Ok(()) } +31 | #[pallet::feeless_if(|_: &OriginFor, _s: &u32| -> bool { true })] + | --------------------------------------------- arguments to this function are incorrect +32 | pub fn foo(_: OriginFor, _something: u64) -> DispatchResult { Ok(()) } | ^^^^^^^^^^ expected `&u32`, found `&u64` | = note: expected reference `&u32` found reference `&u64` note: closure parameter defined here - --> tests/pallet_ui/call_feeless_invalid_closure_arg3.rs:32:46 + --> tests/pallet_ui/call_feeless_invalid_closure_arg3.rs:31:43 | -32 | #[pallet::feeless_if(|_: &AccountIdFor, _s: &u32| -> bool { true })] - | ^^^^^^^^ +31 | #[pallet::feeless_if(|_: &OriginFor, _s: &u32| -> bool { true })] + | ^^^^^^^^ diff --git a/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_return.rs b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_return.rs index d408449e97ec..1f0399a123ca 100644 --- a/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_return.rs +++ b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_return.rs @@ -19,7 +19,6 @@ mod pallet { use frame_support::pallet_prelude::DispatchResult; use frame_system::pallet_prelude::OriginFor; - use frame_system::pallet_prelude::AccountIdFor; #[pallet::config] pub trait Config: frame_system::Config {} @@ -29,7 +28,7 @@ mod pallet { #[pallet::call] impl Pallet { - #[pallet::feeless_if(|_: &AccountIdFor| -> u32 { 0 })] + #[pallet::feeless_if(|_: &OriginFor| -> u32 { 0 })] pub fn foo(_: OriginFor) -> DispatchResult { Ok(()) } } } diff --git a/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_return.stderr b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_return.stderr index 681064279294..a8c05242bde8 100644 --- a/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_return.stderr +++ b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_return.stderr @@ -1,5 +1,5 @@ error: Invalid pallet::call, feeless_if closure must return `bool` - --> tests/pallet_ui/call_feeless_invalid_closure_return.rs:32:46 + --> tests/pallet_ui/call_feeless_invalid_closure_return.rs:31:43 | -32 | #[pallet::feeless_if(|_: &AccountIdFor| -> u32 { 0 })] - | ^ +31 | #[pallet::feeless_if(|_: &OriginFor| -> u32 { 0 })] + | ^ diff --git a/substrate/frame/support/test/tests/pallet_ui/pass/feeless_call.rs b/substrate/frame/support/test/tests/pallet_ui/pass/feeless_call.rs index 12b9aeaa75f1..566b7c65cc71 100644 --- a/substrate/frame/support/test/tests/pallet_ui/pass/feeless_call.rs +++ b/substrate/frame/support/test/tests/pallet_ui/pass/feeless_call.rs @@ -19,7 +19,6 @@ mod pallet { use frame_support::pallet_prelude::DispatchResult; use frame_system::pallet_prelude::OriginFor; - use frame_system::pallet_prelude::AccountIdFor; #[pallet::config] pub trait Config: frame_system::Config {} @@ -29,7 +28,7 @@ mod pallet { #[pallet::call] impl Pallet { - #[pallet::feeless_if(|_: &AccountIdFor| -> bool { true })] + #[pallet::feeless_if(|_: &OriginFor| -> bool { true })] pub fn foo(_: OriginFor) -> DispatchResult { Ok(()) } } } diff --git a/substrate/frame/system/src/lib.rs b/substrate/frame/system/src/lib.rs index 2645e7d826d4..0e394a110411 100644 --- a/substrate/frame/system/src/lib.rs +++ b/substrate/frame/system/src/lib.rs @@ -1854,9 +1854,6 @@ pub mod pallet_prelude { /// Type alias for the `Origin` associated type of system config. pub type OriginFor = ::RuntimeOrigin; - /// Type alias for the `AccountId`. - pub type AccountIdFor = ::AccountId; - /// Type alias for the `Header`. pub type HeaderFor = <::Block as sp_runtime::traits::HeaderProvider>::HeaderT; diff --git a/substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs b/substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs index 77151470f362..38d097ce83ef 100644 --- a/substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs +++ b/substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs @@ -38,7 +38,7 @@ use codec::{Decode, Encode}; use frame_support::{ dispatch::{CheckIfFeeless, DispatchResult}, - traits::IsType, + traits::{IsType, OriginTrait}, }; use scale_info::TypeInfo; use sp_runtime::{ @@ -100,7 +100,7 @@ impl SkipCheckIfFeeless { impl> SignedExtension for SkipCheckIfFeeless where - S::Call: CheckIfFeeless, + S::Call: CheckIfFeeless>, { type AccountId = T::AccountId; type Call = S::Call; @@ -119,7 +119,7 @@ where info: &DispatchInfoOf, len: usize, ) -> Result { - if call.is_feeless(who) { + if call.is_feeless(&::RuntimeOrigin::signed(who.clone())) { Ok((who.clone(), None)) } else { Ok((who.clone(), Some(self.0.pre_dispatch(who, call, info, len)?))) diff --git a/substrate/frame/transaction-payment/skip-feeless-payment/src/mock.rs b/substrate/frame/transaction-payment/skip-feeless-payment/src/mock.rs index 20861e9942a6..5c540c3e4595 100644 --- a/substrate/frame/transaction-payment/skip-feeless-payment/src/mock.rs +++ b/substrate/frame/transaction-payment/skip-feeless-payment/src/mock.rs @@ -72,7 +72,7 @@ pub mod pallet_dummy { #[pallet::call] impl Pallet { - #[pallet::feeless_if(|_who: &AccountIdFor, data: &u32| -> bool { + #[pallet::feeless_if(|_origin: &OriginFor, data: &u32| -> bool { *data == 0 })] pub fn aux(_origin: OriginFor, #[pallet::compact] _data: u32) -> DispatchResult { From ff51a941292bf7bd878790f3018f768ee64821af Mon Sep 17 00:00:00 2001 From: gupnik <17176722+gupnik@users.noreply.github.com> Date: Sat, 11 Nov 2023 11:13:03 +0530 Subject: [PATCH 31/41] Update substrate/frame/support/src/dispatch.rs Co-authored-by: Oliver Tale-Yazdi --- substrate/frame/support/src/dispatch.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/substrate/frame/support/src/dispatch.rs b/substrate/frame/support/src/dispatch.rs index 4c6b4c1beacd..e57227f9b401 100644 --- a/substrate/frame/support/src/dispatch.rs +++ b/substrate/frame/support/src/dispatch.rs @@ -55,6 +55,7 @@ pub trait Callable { pub type CallableCallFor = >::RuntimeCall; /// Means to checks if the dispatchable is feeless. +/// /// This is automatically implemented for all dispatchables during pallet expansion. /// If a call is marked by [`#[pallet::feeless_if]`](`macro@frame_support_procedural::feeless_if`) /// attribute, the corresponding closure is checked. From a1e2ea9ec3cf6d1f131c8282277a9d375317ef14 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Sat, 11 Nov 2023 11:20:56 +0530 Subject: [PATCH 32/41] Addresses review comments --- .../procedural/src/pallet/parse/call.rs | 23 +++++++++++-------- .../skip-feeless-payment/src/lib.rs | 6 ++--- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/substrate/frame/support/procedural/src/pallet/parse/call.rs b/substrate/frame/support/procedural/src/pallet/parse/call.rs index 64fd95309eb4..ed21dd488c5e 100644 --- a/substrate/frame/support/procedural/src/pallet/parse/call.rs +++ b/substrate/frame/support/procedural/src/pallet/parse/call.rs @@ -17,6 +17,7 @@ use super::{helper, InheritedCallWeightAttr}; use frame_support_procedural_tools::get_doc_literals; +use proc_macro2::Span; use quote::ToTokens; use std::collections::HashMap; use syn::{spanned::Spanned, ExprClosure}; @@ -88,12 +89,13 @@ pub struct CallVariantDef { } /// Attributes for functions in call impl block. -/// Parse for `#[pallet::weight(expr)]` or `#[pallet::call_index(expr)] or -/// `#[pallet::feeless_if(expr)]` pub enum FunctionAttr { + /// Parse for `#[pallet::call_index(expr)]` CallIndex(u8), + /// Parse for `#[pallet::weight(expr)]` Weight(syn::Expr), - FeelessIf(syn::ExprClosure), + /// Parse for `#[pallet::feeless_if(expr)]` + FeelessIf(Span, syn::ExprClosure), } impl syn::parse::Parse for FunctionAttr { @@ -124,7 +126,10 @@ impl syn::parse::Parse for FunctionAttr { content.parse::()?; let closure_content; syn::parenthesized!(closure_content in content); - Ok(FunctionAttr::FeelessIf(closure_content.parse::()?)) + Ok(FunctionAttr::FeelessIf( + closure_content.span(), + closure_content.parse::()?, + )) } else { Err(lookahead.error()) } @@ -248,8 +253,8 @@ impl CallDef { FunctionAttr::Weight(_) => { weight_attrs.push(attr); }, - FunctionAttr::FeelessIf(_) => { - feeless_attrs.push(attr); + FunctionAttr::FeelessIf(span, _) => { + feeless_attrs.push((span, attr)); }, } } @@ -341,11 +346,11 @@ impl CallDef { if feeless_attrs.len() > 1 { let msg = "Invalid pallet::call, too many feeless_if attributes given"; - return Err(syn::Error::new(method.sig.span(), msg)) + return Err(syn::Error::new(feeless_attrs[1].0, msg)) } let feeless_check: Option = - feeless_attrs.pop().map(|attr| match attr { - FunctionAttr::FeelessIf(closure) => closure, + feeless_attrs.pop().map(|(_, attr)| match attr { + FunctionAttr::FeelessIf(_, closure) => closure, _ => unreachable!("checked during creation of the let binding"), }); diff --git a/substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs b/substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs index 38d097ce83ef..923c7e7ebc29 100644 --- a/substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs +++ b/substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs @@ -12,7 +12,7 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. - +// //! # Skip Feeless Payment Pallet //! //! This pallet allows runtimes that include it to skip payment of transaction fees for @@ -20,7 +20,7 @@ //! frame_support::pallet_prelude::feeless_if`). //! //! ## Overview - +//! //! It does this by wrapping an existing [`SignedExtension`] implementation (e.g. //! [`pallet-transaction-payment`]) and checking if the dispatchable is feeless before applying the //! wrapped extension. If the dispatchable is indeed feeless, the extension is skipped and a custom @@ -28,7 +28,7 @@ //! //! //! ## Integration - +//! //! This pallet wraps an existing transaction payment pallet. This means you should both pallets //! in your `construct_runtime` macro and include this pallet's //! [`SignedExtension`] ([`SkipCheckIfFeeless`]) that would accept the existing one as an argument. From c6f10916f5dc779d0a9b4865768baba22669150a Mon Sep 17 00:00:00 2001 From: gupnik <17176722+gupnik@users.noreply.github.com> Date: Sat, 11 Nov 2023 11:22:41 +0530 Subject: [PATCH 33/41] Update substrate/frame/support/procedural/src/lib.rs Co-authored-by: Francisco Aguirre --- substrate/frame/support/procedural/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/support/procedural/src/lib.rs b/substrate/frame/support/procedural/src/lib.rs index 052101306e0e..f558c3a26cf2 100644 --- a/substrate/frame/support/procedural/src/lib.rs +++ b/substrate/frame/support/procedural/src/lib.rs @@ -1160,7 +1160,7 @@ pub fn call_index(_: TokenStream, _: TokenStream) -> TokenStream { /// Each dispatchable may also be annotated with the `#[pallet::feeless_if($closure)]` attribute, /// which explicitly defines the condition for the dispatchable function to be feeless. /// -/// The arguments for the closure must be the references to arguments of the dispatchable function. +/// The arguments for the closure must be the referenced arguments of the dispatchable function. /// /// The closure must return `bool`. /// From 69193c648018b9f6ed46da7b833f53d503007e30 Mon Sep 17 00:00:00 2001 From: gupnik <17176722+gupnik@users.noreply.github.com> Date: Sat, 11 Nov 2023 11:22:56 +0530 Subject: [PATCH 34/41] Update substrate/frame/support/procedural/src/pallet/parse/call.rs Co-authored-by: Francisco Aguirre --- substrate/frame/support/procedural/src/pallet/parse/call.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/support/procedural/src/pallet/parse/call.rs b/substrate/frame/support/procedural/src/pallet/parse/call.rs index ed21dd488c5e..2c64bc221f3e 100644 --- a/substrate/frame/support/procedural/src/pallet/parse/call.rs +++ b/substrate/frame/support/procedural/src/pallet/parse/call.rs @@ -345,7 +345,7 @@ impl CallDef { let docs = get_doc_literals(&method.attrs); if feeless_attrs.len() > 1 { - let msg = "Invalid pallet::call, too many feeless_if attributes given"; + let msg = "Invalid pallet::call, there can only be one feeless_if attribute"; return Err(syn::Error::new(feeless_attrs[1].0, msg)) } let feeless_check: Option = From 5a44ab41e1772f944e1421cea5dab9cca47dda0e Mon Sep 17 00:00:00 2001 From: gupnik <17176722+gupnik@users.noreply.github.com> Date: Sat, 11 Nov 2023 11:26:28 +0530 Subject: [PATCH 35/41] Update prdoc/pr_1926.prdoc Co-authored-by: Liam Aharon --- prdoc/pr_1926.prdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prdoc/pr_1926.prdoc b/prdoc/pr_1926.prdoc index 3e456f75d94f..9147a60de2da 100644 --- a/prdoc/pr_1926.prdoc +++ b/prdoc/pr_1926.prdoc @@ -5,7 +5,7 @@ doc: description: | 1. Adds an attribute `#[pallet::feeless_if]` that can be optionally attached to a `pallet::call`. 2. Adds a signed extension SkipCheckIfFeeless that wraps a transaction - payment processor to skip payment fess for all such calls. + payment processor to potentially skip payment fess for such calls. migrations: db: [] From 90f2cf824c578ade8b2928259e26a1c3f462bcce Mon Sep 17 00:00:00 2001 From: gupnik <17176722+gupnik@users.noreply.github.com> Date: Sat, 11 Nov 2023 11:27:15 +0530 Subject: [PATCH 36/41] Update substrate/frame/support/procedural/src/lib.rs Co-authored-by: Liam Aharon --- substrate/frame/support/procedural/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/support/procedural/src/lib.rs b/substrate/frame/support/procedural/src/lib.rs index f558c3a26cf2..5b72c7438864 100644 --- a/substrate/frame/support/procedural/src/lib.rs +++ b/substrate/frame/support/procedural/src/lib.rs @@ -1157,7 +1157,7 @@ pub fn call_index(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() } -/// Each dispatchable may also be annotated with the `#[pallet::feeless_if($closure)]` attribute, +/// Each dispatchable may be annotated with the `#[pallet::feeless_if($closure)]` attribute, /// which explicitly defines the condition for the dispatchable function to be feeless. /// /// The arguments for the closure must be the referenced arguments of the dispatchable function. From f031785ec5812565d7e0d1e88c9a18e486fc29de Mon Sep 17 00:00:00 2001 From: gupnik <17176722+gupnik@users.noreply.github.com> Date: Sat, 11 Nov 2023 11:27:29 +0530 Subject: [PATCH 37/41] Update substrate/frame/support/procedural/src/lib.rs Co-authored-by: Liam Aharon --- substrate/frame/support/procedural/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/support/procedural/src/lib.rs b/substrate/frame/support/procedural/src/lib.rs index 5b72c7438864..ec4118918856 100644 --- a/substrate/frame/support/procedural/src/lib.rs +++ b/substrate/frame/support/procedural/src/lib.rs @@ -1158,7 +1158,7 @@ pub fn call_index(_: TokenStream, _: TokenStream) -> TokenStream { } /// Each dispatchable may be annotated with the `#[pallet::feeless_if($closure)]` attribute, -/// which explicitly defines the condition for the dispatchable function to be feeless. +/// 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. /// From e21094130b7bc6f968ae5658ae595a6c6b927586 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Sat, 11 Nov 2023 11:29:57 +0530 Subject: [PATCH 38/41] Addresses review comments --- prdoc/pr_1926.prdoc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/prdoc/pr_1926.prdoc b/prdoc/pr_1926.prdoc index 9147a60de2da..9dc656f1260d 100644 --- a/prdoc/pr_1926.prdoc +++ b/prdoc/pr_1926.prdoc @@ -5,7 +5,8 @@ doc: description: | 1. Adds an attribute `#[pallet::feeless_if]` that can be optionally attached to a `pallet::call`. 2. Adds a signed extension SkipCheckIfFeeless that wraps a transaction - payment processor to potentially skip payment fess for such calls. + 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: [] From d79cc22d18cf70acb89825ca845d303fa1b2d95a Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Sat, 11 Nov 2023 11:39:12 +0530 Subject: [PATCH 39/41] Fixes error description for invalid attr type --- .../frame/support/procedural/src/pallet/parse/call.rs | 7 ++++++- .../test/tests/pallet_ui/call_feeless_invalid_type.stderr | 6 ++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/substrate/frame/support/procedural/src/pallet/parse/call.rs b/substrate/frame/support/procedural/src/pallet/parse/call.rs index 2c64bc221f3e..3703e0adf654 100644 --- a/substrate/frame/support/procedural/src/pallet/parse/call.rs +++ b/substrate/frame/support/procedural/src/pallet/parse/call.rs @@ -128,7 +128,12 @@ impl syn::parse::Parse for FunctionAttr { syn::parenthesized!(closure_content in content); Ok(FunctionAttr::FeelessIf( closure_content.span(), - closure_content.parse::()?, + closure_content.parse::().map_err(|e| { + let msg = "Invalid feeless_if attribute: expected a closure"; + let mut err = syn::Error::new(closure_content.span(), msg); + err.combine(e); + err + })? )) } else { Err(lookahead.error()) diff --git a/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_type.stderr b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_type.stderr index beb19fdb8fad..add3decbf160 100644 --- a/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_type.stderr +++ b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_type.stderr @@ -1,3 +1,9 @@ +error: Invalid feeless_if attribute: expected a closure + --> tests/pallet_ui/call_feeless_invalid_type.rs:31:24 + | +31 | #[pallet::feeless_if(0)] + | ^ + error: expected `|` --> tests/pallet_ui/call_feeless_invalid_type.rs:31:24 | From 0ab5492e31c907c222b6ac0818b37861385f5408 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Sat, 11 Nov 2023 12:22:36 +0530 Subject: [PATCH 40/41] Fixes error description and span for closure args --- .../procedural/src/pallet/parse/call.rs | 70 +++++++++++++++---- .../call_feeless_invalid_closure_arg2.stderr | 18 +---- .../call_feeless_invalid_closure_arg3.stderr | 14 +--- 3 files changed, 61 insertions(+), 41 deletions(-) diff --git a/substrate/frame/support/procedural/src/pallet/parse/call.rs b/substrate/frame/support/procedural/src/pallet/parse/call.rs index 3703e0adf654..519e1e618954 100644 --- a/substrate/frame/support/procedural/src/pallet/parse/call.rs +++ b/substrate/frame/support/procedural/src/pallet/parse/call.rs @@ -133,7 +133,7 @@ impl syn::parse::Parse for FunctionAttr { let mut err = syn::Error::new(closure_content.span(), msg); err.combine(e); err - })? + })?, )) } else { Err(lookahead.error()) @@ -158,28 +158,33 @@ impl syn::parse::Parse for ArgAttrIsCompact { } } -/// Check the syntax is `OriginFor` -pub fn check_dispatchable_first_arg_type(ty: &syn::Type) -> syn::Result<()> { - pub struct CheckDispatchableFirstArg; +/// Check the syntax is `OriginFor` or `&OriginFor`. +pub fn check_dispatchable_first_arg_type(ty: &syn::Type, is_ref: bool) -> syn::Result<()> { + pub struct CheckDispatchableFirstArg(bool); impl syn::parse::Parse for CheckDispatchableFirstArg { fn parse(input: syn::parse::ParseStream) -> syn::Result { + let is_ref = input.parse::().is_ok(); input.parse::()?; input.parse::()?; input.parse::()?; input.parse::]>()?; - Ok(Self) + Ok(Self(is_ref)) } } - syn::parse2::(ty.to_token_stream()).map_err(|e| { - let msg = "Invalid type: expected `OriginFor`"; - let mut err = syn::Error::new(ty.span(), msg); - err.combine(e); - err - })?; - - Ok(()) + let result = syn::parse2::(ty.to_token_stream()); + return match result { + Ok(CheckDispatchableFirstArg(has_ref)) if is_ref == has_ref => Ok(()), + _ => { + let msg = if is_ref { + "Invalid type: expected `&OriginFor`" + } else { + "Invalid type: expected `OriginFor`" + }; + return Err(syn::Error::new(ty.span(), msg)) + }, + } } impl CallDef { @@ -235,7 +240,7 @@ impl CallDef { return Err(syn::Error::new(method.sig.span(), msg)) }, Some(syn::FnArg::Typed(arg)) => { - check_dispatchable_first_arg_type(&arg.ty)?; + check_dispatchable_first_arg_type(&arg.ty, false)?; }, } @@ -366,6 +371,43 @@ impl CallDef { return Err(syn::Error::new(feeless_check.span(), msg)) } + match feeless_check.inputs.first() { + None => { + let msg = "Invalid pallet::call, feeless_if closure must have at least origin arg"; + return Err(syn::Error::new(feeless_check.span(), msg)) + }, + Some(syn::Pat::Type(arg)) => { + check_dispatchable_first_arg_type(&arg.ty, true)?; + }, + _ => { + let msg = "Invalid pallet::call, feeless_if closure first argument must be a typed argument, \ + e.g. `origin: OriginFor`"; + return Err(syn::Error::new(feeless_check.span(), msg)) + }, + } + + for (feeless_arg, arg) in feeless_check.inputs.iter().skip(1).zip(args.iter()) { + let feeless_arg_type = + if let syn::Pat::Type(syn::PatType { ty, .. }) = feeless_arg.clone() { + if let syn::Type::Reference(pat) = *ty { + pat.elem.clone() + } else { + let msg = "Invalid pallet::call, feeless_if closure argument must be a reference"; + return Err(syn::Error::new(ty.span(), msg)) + } + } else { + let msg = "Invalid pallet::call, feeless_if closure argument must be a type ascription pattern"; + return Err(syn::Error::new(feeless_arg.span(), msg)) + }; + + if feeless_arg_type != arg.2 { + let msg = + "Invalid pallet::call, feeless_if closure argument must have \ + a reference to the same type as the dispatchable function argument"; + return Err(syn::Error::new(feeless_arg.span(), msg)) + } + } + let valid_return = match &feeless_check.output { syn::ReturnType::Type(_, type_) => match *(type_.clone()) { syn::Type::Path(syn::TypePath { path, .. }) => path.is_ident("bool"), diff --git a/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg2.stderr b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg2.stderr index 627358a7ebbd..1c38ec236836 100644 --- a/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg2.stderr +++ b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg2.stderr @@ -1,17 +1,5 @@ -error[E0308]: mismatched types - --> tests/pallet_ui/call_feeless_invalid_closure_arg2.rs:18:1 +error: Invalid type: expected `&OriginFor` + --> tests/pallet_ui/call_feeless_invalid_closure_arg2.rs:31:28 | -18 | #[frame_support::pallet(dev_mode)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `bool`, found `&::RuntimeOrigin` -... 31 | #[pallet::feeless_if(|_: bool| -> bool { true })] - | -------------------------- arguments to this function are incorrect - | - = note: expected type `bool` - found reference `&::RuntimeOrigin` -note: closure parameter defined here - --> tests/pallet_ui/call_feeless_invalid_closure_arg2.rs:31:25 - | -31 | #[pallet::feeless_if(|_: bool| -> bool { true })] - | ^^^^^^^ - = note: this error originates in the attribute macro `frame_support::pallet` (in Nightly builds, run with -Z macro-backtrace for more info) + | ^^^^ diff --git a/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg3.stderr b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg3.stderr index eaf4df140d98..1ad9588cead6 100644 --- a/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg3.stderr +++ b/substrate/frame/support/test/tests/pallet_ui/call_feeless_invalid_closure_arg3.stderr @@ -1,15 +1,5 @@ -error[E0308]: mismatched types - --> tests/pallet_ui/call_feeless_invalid_closure_arg3.rs:32:31 - | -31 | #[pallet::feeless_if(|_: &OriginFor, _s: &u32| -> bool { true })] - | --------------------------------------------- arguments to this function are incorrect -32 | pub fn foo(_: OriginFor, _something: u64) -> DispatchResult { Ok(()) } - | ^^^^^^^^^^ expected `&u32`, found `&u64` - | - = note: expected reference `&u32` - found reference `&u64` -note: closure parameter defined here +error: Invalid pallet::call, feeless_if closure argument must have a reference to the same type as the dispatchable function argument --> tests/pallet_ui/call_feeless_invalid_closure_arg3.rs:31:43 | 31 | #[pallet::feeless_if(|_: &OriginFor, _s: &u32| -> bool { true })] - | ^^^^^^^^ + | ^^ From 3f9faad3192827a1844c8c7d26fa4b6fbfeb3813 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Sat, 11 Nov 2023 12:47:18 +0530 Subject: [PATCH 41/41] Updates error output in UI test --- .../test/tests/pallet_ui/call_invalid_origin_type.stderr | 6 ------ 1 file changed, 6 deletions(-) diff --git a/substrate/frame/support/test/tests/pallet_ui/call_invalid_origin_type.stderr b/substrate/frame/support/test/tests/pallet_ui/call_invalid_origin_type.stderr index 99146c0563a9..c04729a24386 100644 --- a/substrate/frame/support/test/tests/pallet_ui/call_invalid_origin_type.stderr +++ b/substrate/frame/support/test/tests/pallet_ui/call_invalid_origin_type.stderr @@ -3,9 +3,3 @@ error: Invalid type: expected `OriginFor` | 34 | pub fn foo(origin: u8) {} | ^^ - -error: expected `OriginFor` - --> tests/pallet_ui/call_invalid_origin_type.rs:34:22 - | -34 | pub fn foo(origin: u8) {} - | ^^