From d07a92215cb40c830072aca8a22c32c1c55b0a56 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 28 Jul 2020 14:09:36 +0200 Subject: [PATCH 1/8] initial mock --- frame/contracts/src/gas.rs | 1 + frame/sudo/src/lib.rs | 27 ++++++++++++---- frame/support/src/weights.rs | 46 +++++++++++++++++++++++++++- frame/transaction-payment/src/lib.rs | 2 +- 4 files changed, 68 insertions(+), 8 deletions(-) diff --git a/frame/contracts/src/gas.rs b/frame/contracts/src/gas.rs index 0ae1952de0914..d6c7f82753ebd 100644 --- a/frame/contracts/src/gas.rs +++ b/frame/contracts/src/gas.rs @@ -194,6 +194,7 @@ impl GasMeter { { let post_info = PostDispatchInfo { actual_weight: Some(self.gas_spent()), + pays_fee: Default::default(), }; result diff --git a/frame/sudo/src/lib.rs b/frame/sudo/src/lib.rs index 0f614a46467f4..b49699abb9b4d 100644 --- a/frame/sudo/src/lib.rs +++ b/frame/sudo/src/lib.rs @@ -93,7 +93,11 @@ use sp_runtime::{DispatchResult, traits::StaticLookup}; use frame_support::{ Parameter, decl_module, decl_event, decl_storage, decl_error, ensure, }; -use frame_support::{weights::{Weight, GetDispatchInfo}, traits::UnfilteredDispatchable}; +use frame_support::{ + weights::{Weight, GetDispatchInfo, Pays}, + traits::UnfilteredDispatchable, + dispatch::DispatchResultWithPostInfo, +}; use frame_system::ensure_signed; #[cfg(test)] @@ -127,13 +131,15 @@ decl_module! { /// - Weight of derivative `call` execution + 10,000. /// # #[weight = (call.get_dispatch_info().weight + 10_000, call.get_dispatch_info().class)] - fn sudo(origin, call: Box<::Call>) { + fn sudo(origin, call: Box<::Call>) -> DispatchResultWithPostInfo{ // This is a public call, so we ensure that the origin is some signed account. let sender = ensure_signed(origin)?; ensure!(sender == Self::key(), Error::::RequireSudo); - let res = call.dispatch_bypass_filter(frame_system::RawOrigin::Root.into()); + let res = call.clone().dispatch_bypass_filter(frame_system::RawOrigin::Root.into()); Self::deposit_event(RawEvent::Sudid(res.map(|_| ()).map_err(|e| e.error))); + // Sudo user does not pay a fee. + Ok(Pays::No.into()) } /// Authenticates the sudo key and dispatches a function call with `Root` origin. @@ -147,13 +153,15 @@ decl_module! { /// - The weight of this call is defined by the caller. /// # #[weight = (*_weight, call.get_dispatch_info().class)] - fn sudo_unchecked_weight(origin, call: Box<::Call>, _weight: Weight) { + fn sudo_unchecked_weight(origin, call: Box<::Call>, _weight: Weight) -> DispatchResultWithPostInfo { // This is a public call, so we ensure that the origin is some signed account. let sender = ensure_signed(origin)?; ensure!(sender == Self::key(), Error::::RequireSudo); let res = call.dispatch_bypass_filter(frame_system::RawOrigin::Root.into()); Self::deposit_event(RawEvent::Sudid(res.map(|_| ()).map_err(|e| e.error))); + // Sudo user does not pay a fee. + Ok(Pays::No.into()) } /// Authenticates the current sudo key and sets the given AccountId (`new`) as the new sudo key. @@ -166,7 +174,7 @@ decl_module! { /// - One DB change. /// # #[weight = 0] - fn set_key(origin, new: ::Source) { + fn set_key(origin, new: ::Source) -> DispatchResultWithPostInfo { // This is a public call, so we ensure that the origin is some signed account. let sender = ensure_signed(origin)?; ensure!(sender == Self::key(), Error::::RequireSudo); @@ -174,6 +182,8 @@ decl_module! { Self::deposit_event(RawEvent::KeyChanged(Self::key())); >::put(new); + // Sudo user does not pay a fee. + Ok(Pays::No.into()) } /// Authenticates the sudo key and dispatches a function call with `Signed` origin from @@ -188,7 +198,10 @@ decl_module! { /// - Weight of derivative `call` execution + 10,000. /// # #[weight = (call.get_dispatch_info().weight + 10_000, call.get_dispatch_info().class)] - fn sudo_as(origin, who: ::Source, call: Box<::Call>) { + fn sudo_as(origin, + who: ::Source, + call: Box<::Call> + ) -> DispatchResultWithPostInfo { // This is a public call, so we ensure that the origin is some signed account. let sender = ensure_signed(origin)?; ensure!(sender == Self::key(), Error::::RequireSudo); @@ -204,6 +217,8 @@ decl_module! { }; Self::deposit_event(RawEvent::SudoAsDone(res)); + // Sudo user does not pay a fee. + Ok(Pays::No.into()) } } } diff --git a/frame/support/src/weights.rs b/frame/support/src/weights.rs index 595e84333bda7..36045ccc74348 100644 --- a/frame/support/src/weights.rs +++ b/frame/support/src/weights.rs @@ -263,10 +263,13 @@ pub trait GetDispatchInfo { } /// Weight information that is only available post dispatch. +/// NOTE: This can only be used to reduce the weight or fee, not increase it. #[derive(Clone, Copy, Eq, PartialEq, Default, RuntimeDebug, Encode, Decode)] pub struct PostDispatchInfo { /// Actual weight consumed by a call or `None` which stands for the worst case static weight. pub actual_weight: Option, + /// Whether this transaction should pay fees when all is said and done. + pub pays_fee: Pays, } impl PostDispatchInfo { @@ -283,6 +286,18 @@ impl PostDispatchInfo { info.weight } } + + /// Determine if user should actually pay fees at the end of the dispatch. + pub fn pays_fee(&self, info: &DispatchInfo) -> Pays { + // If they originally were not paying fees, or the post dispatch info + // says they should not pay fees, then they don't pay fees... + if info.pays_fee == Pays::No || self.pays_fee == Pays::No { + Pays::No + } else { + // Otherwise they pay. + Pays::Yes + } + } } /// Extract the actual weight from a dispatch result if any or fall back to the default weight. @@ -293,10 +308,30 @@ pub fn extract_actual_weight(result: &DispatchResultWithPostInfo, info: &Dispatc }.calc_actual_weight(info) } +impl From<(Option, Pays)> for PostDispatchInfo { + fn from(post_weight_info: (Option, Pays)) -> Self { + let (actual_weight, pays_fee) = post_weight_info; + Self { + actual_weight, + pays_fee, + } + } +} + +impl From for PostDispatchInfo { + fn from(pays_fee: Pays) -> Self { + Self { + actual_weight: None, + pays_fee, + } + } +} + impl From> for PostDispatchInfo { fn from(actual_weight: Option) -> Self { Self { actual_weight, + pays_fee: Default::default(), } } } @@ -305,6 +340,7 @@ impl From<()> for PostDispatchInfo { fn from(_: ()) -> Self { Self { actual_weight: None, + pays_fee: Default::default(), } } } @@ -315,6 +351,11 @@ impl sp_runtime::traits::Printable for PostDispatchInfo { match self.actual_weight { Some(weight) => weight.print(), None => "max-weight".print(), + }; + " pays_fee=".print(); + match self.pays_fee { + Pays::Yes => "true".print(), + Pays::No => "false".print(), } } } @@ -338,7 +379,10 @@ impl WithPostDispatchInfo for T where { fn with_weight(self, actual_weight: Weight) -> DispatchErrorWithPostInfo { DispatchErrorWithPostInfo { - post_info: PostDispatchInfo { actual_weight: Some(actual_weight) }, + post_info: PostDispatchInfo { + actual_weight: Some(actual_weight), + pays_fee: Default::default(), + }, error: self.into(), } } diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index 9c624df8ca355..bfac48b7787ac 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -371,7 +371,7 @@ impl Module where ) -> BalanceOf where T::Call: Dispatchable, { - Self::compute_fee_raw(len, post_info.calc_actual_weight(info), tip, info.pays_fee) + Self::compute_fee_raw(len, post_info.calc_actual_weight(info), tip, post_info.pays_fee(info)) } fn compute_fee_raw( From a4fc1209cdb2f16995251e0167743679ad7e07a9 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 28 Jul 2020 14:22:55 +0200 Subject: [PATCH 2/8] add test --- frame/transaction-payment/src/lib.rs | 51 ++++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index bfac48b7787ac..244b4280ade08 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -762,11 +762,24 @@ mod tests { } fn post_info_from_weight(w: Weight) -> PostDispatchInfo { - PostDispatchInfo { actual_weight: Some(w), } + PostDispatchInfo { + actual_weight: Some(w), + pays_fee: Default::default(), + } + } + + fn post_info_from_pays(p: Pays) -> PostDispatchInfo { + PostDispatchInfo { + actual_weight: None, + pays_fee: p, + } } fn default_post_info() -> PostDispatchInfo { - PostDispatchInfo { actual_weight: None, } + PostDispatchInfo { + actual_weight: None, + pays_fee: Default::default(), + } } #[test] @@ -1211,4 +1224,38 @@ mod tests { assert_eq!(refund_based_fee, actual_fee); }); } + + #[test] + fn post_info_can_change_pays_fee() { + ExtBuilder::default() + .balance_factor(10) + .base_weight(7) + .build() + .execute_with(|| + { + let info = info_from_weight(100); + let post_info = post_info_from_pays(Pays::No); + let prev_balance = Balances::free_balance(2); + let len = 10; + let tip = 5; + + NextFeeMultiplier::put(Multiplier::saturating_from_rational(5, 4)); + + let pre = ChargeTransactionPayment::::from(tip) + .pre_dispatch(&2, CALL, &info, len) + .unwrap(); + + ChargeTransactionPayment:: + ::post_dispatch(pre, &info, &post_info, len, &Ok(())) + .unwrap(); + + let refund_based_fee = prev_balance - Balances::free_balance(2); + let actual_fee = Module:: + ::compute_actual_fee(len as u32, &info, &post_info, tip); + + // Only 5 tip is paid + assert_eq!(actual_fee, 5); + assert_eq!(refund_based_fee, actual_fee); + }); + } } From 67e696ddb99299a3b0e386038a1302b5c9a17aea Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 28 Jul 2020 14:25:53 +0200 Subject: [PATCH 3/8] remove unneeded clone --- frame/sudo/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/sudo/src/lib.rs b/frame/sudo/src/lib.rs index b49699abb9b4d..fae1ccb2da51c 100644 --- a/frame/sudo/src/lib.rs +++ b/frame/sudo/src/lib.rs @@ -136,7 +136,7 @@ decl_module! { let sender = ensure_signed(origin)?; ensure!(sender == Self::key(), Error::::RequireSudo); - let res = call.clone().dispatch_bypass_filter(frame_system::RawOrigin::Root.into()); + let res = call.dispatch_bypass_filter(frame_system::RawOrigin::Root.into()); Self::deposit_event(RawEvent::Sudid(res.map(|_| ()).map_err(|e| e.error))); // Sudo user does not pay a fee. Ok(Pays::No.into()) From 2a5769165249935946e517d7540881b5094dfc75 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 28 Jul 2020 14:35:30 +0200 Subject: [PATCH 4/8] Update frame/support/src/weights.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Alexander Theißen --- frame/support/src/weights.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frame/support/src/weights.rs b/frame/support/src/weights.rs index 36045ccc74348..ee6ff43c307e9 100644 --- a/frame/support/src/weights.rs +++ b/frame/support/src/weights.rs @@ -290,7 +290,9 @@ impl PostDispatchInfo { /// Determine if user should actually pay fees at the end of the dispatch. pub fn pays_fee(&self, info: &DispatchInfo) -> Pays { // If they originally were not paying fees, or the post dispatch info - // says they should not pay fees, then they don't pay fees... + // says they should not pay fees, then they don't pay fees. + // This is because the pre dispatch information must contain the + // worst case for weight and fees paid. if info.pays_fee == Pays::No || self.pays_fee == Pays::No { Pays::No } else { From 7463977657fd630e11545ccb336904ead1d45bc1 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 28 Jul 2020 14:47:08 +0200 Subject: [PATCH 5/8] fix compile --- frame/contracts/src/tests.rs | 1 + frame/system/src/extensions/check_weight.rs | 10 ++++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index 0d2a2f7a3143e..792d29e152c94 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -295,6 +295,7 @@ fn returns_base_call_cost() { Ok( PostDispatchInfo { actual_weight: Some(67500000), + pays_fee: Default::default(), } ) ); diff --git a/frame/system/src/extensions/check_weight.rs b/frame/system/src/extensions/check_weight.rs index d52138b1e3bbe..1395aa87efbcf 100644 --- a/frame/system/src/extensions/check_weight.rs +++ b/frame/system/src/extensions/check_weight.rs @@ -575,7 +575,10 @@ mod tests { new_test_ext().execute_with(|| { // This is half of the max block weight let info = DispatchInfo { weight: 512, ..Default::default() }; - let post_info = PostDispatchInfo { actual_weight: Some(128), }; + let post_info = PostDispatchInfo { + actual_weight: Some(128), + pays_fee: Default::default(), + }; let len = 0_usize; // We allow 75% for normal transaction, so we put 25% - extrinsic base weight @@ -601,7 +604,10 @@ mod tests { fn signed_ext_check_weight_actual_weight_higher_than_max_is_capped() { new_test_ext().execute_with(|| { let info = DispatchInfo { weight: 512, ..Default::default() }; - let post_info = PostDispatchInfo { actual_weight: Some(700), }; + let post_info = PostDispatchInfo { + actual_weight: Some(700), + pays_fee: Default::default(), + }; let len = 0_usize; BlockWeight::mutate(|current_weight| { From f679652d882899cb459f49ea0b9cdd40c44cae98 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 28 Jul 2020 18:19:26 +0200 Subject: [PATCH 6/8] Update frame/support/src/weights.rs Co-authored-by: Alexander Popiak --- frame/support/src/weights.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/support/src/weights.rs b/frame/support/src/weights.rs index ee6ff43c307e9..bfb7ae5163fd3 100644 --- a/frame/support/src/weights.rs +++ b/frame/support/src/weights.rs @@ -356,8 +356,8 @@ impl sp_runtime::traits::Printable for PostDispatchInfo { }; " pays_fee=".print(); match self.pays_fee { - Pays::Yes => "true".print(), - Pays::No => "false".print(), + Pays::Yes => "Yes".print(), + Pays::No => "No".print(), } } } From 37a0851e7fa560010a1e0e8050ca469a15047c46 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 28 Jul 2020 18:19:34 +0200 Subject: [PATCH 7/8] Update frame/sudo/src/lib.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com> --- frame/sudo/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/sudo/src/lib.rs b/frame/sudo/src/lib.rs index fae1ccb2da51c..113fa0dccc6c7 100644 --- a/frame/sudo/src/lib.rs +++ b/frame/sudo/src/lib.rs @@ -131,7 +131,7 @@ decl_module! { /// - Weight of derivative `call` execution + 10,000. /// # #[weight = (call.get_dispatch_info().weight + 10_000, call.get_dispatch_info().class)] - fn sudo(origin, call: Box<::Call>) -> DispatchResultWithPostInfo{ + fn sudo(origin, call: Box<::Call>) -> DispatchResultWithPostInfo { // This is a public call, so we ensure that the origin is some signed account. let sender = ensure_signed(origin)?; ensure!(sender == Self::key(), Error::::RequireSudo); From 41fbaaf61d301f36c8c85573fe083ff9d044f2b4 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Wed, 29 Jul 2020 04:11:45 +0200 Subject: [PATCH 8/8] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- frame/support/src/weights.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/support/src/weights.rs b/frame/support/src/weights.rs index bfb7ae5163fd3..db1e25ca7ab2e 100644 --- a/frame/support/src/weights.rs +++ b/frame/support/src/weights.rs @@ -296,7 +296,7 @@ impl PostDispatchInfo { if info.pays_fee == Pays::No || self.pays_fee == Pays::No { Pays::No } else { - // Otherwise they pay. + // Otherwise they pay. Pays::Yes } } @@ -354,7 +354,7 @@ impl sp_runtime::traits::Printable for PostDispatchInfo { Some(weight) => weight.print(), None => "max-weight".print(), }; - " pays_fee=".print(); + "pays_fee=".print(); match self.pays_fee { Pays::Yes => "Yes".print(), Pays::No => "No".print(),