diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index 9f48ab7e3ef3c..13558998d6440 100644 --- a/bin/node/cli/src/service.rs +++ b/bin/node/cli/src/service.rs @@ -817,7 +817,7 @@ mod tests { let check_genesis = frame_system::CheckGenesis::new(); 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 check_weight = frame_system::CheckWeight::new().into(); let payment = pallet_transaction_payment::ChargeTransactionPayment::from(0); let extra = ( check_spec_version, diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index d7257a9ea71b5..479659b619ddf 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -25,6 +25,7 @@ use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::{ construct_runtime, parameter_types, + signed_extensions::{AdjustPriority, Divide}, traits::{ Currency, Everything, Imbalance, InstanceFilter, KeyOwnerProofSystem, LockIdentifier, Nothing, OnUnbalanced, U128CurrencyToVote, @@ -927,7 +928,7 @@ where frame_system::CheckGenesis::::new(), frame_system::CheckEra::::from(era), frame_system::CheckNonce::::from(nonce), - frame_system::CheckWeight::::new(), + frame_system::CheckWeight::::new().into(), pallet_transaction_payment::ChargeTransactionPayment::::from(tip), ); let raw_payload = SignedPayload::new(call, extra) @@ -1268,7 +1269,7 @@ pub type SignedExtra = ( frame_system::CheckGenesis, frame_system::CheckEra, frame_system::CheckNonce, - frame_system::CheckWeight, + AdjustPriority, Divide, CHECK_WEIGHT_PRIORITY_DIVISOR>, pallet_transaction_payment::ChargeTransactionPayment, ); /// Unchecked extrinsic type as expected by this runtime. @@ -1297,6 +1298,22 @@ mod mmr { pub type Hashing = ::Hashing; } +/// There are two extensions returning the priority: +/// 1. The `CheckWeight` extension. +/// 2. The `TransactionPayment` extension. +/// +/// The first one gives a significant bump to `Operational` transactions, but for `Normal` +/// it's within `[0..MAXIMUM_BLOCK_WEIGHT]` range. +/// +/// The second one roughly represents the amount of fees being paid (and the tip) with +/// size-adjustment coefficient. I.e. we are interested to maximize `fee/consumed_weight` or +/// `fee/size_limit`. The returned value is potentially unbounded though. +/// +/// The idea for the adjustment is scale the priority coming from `CheckWeight` for +/// `Normal` transactions down to zero, leaving the priority bump for `Operational` and +/// `Mandatory` though. +const CHECK_WEIGHT_PRIORITY_DIVISOR: TransactionPriority = MAXIMUM_BLOCK_WEIGHT; + impl_runtime_apis! { impl sp_api::Core for Runtime { fn version() -> RuntimeVersion { diff --git a/bin/node/test-runner-example/src/lib.rs b/bin/node/test-runner-example/src/lib.rs index e7fe1ee002423..886c40d5f1473 100644 --- a/bin/node/test-runner-example/src/lib.rs +++ b/bin/node/test-runner-example/src/lib.rs @@ -76,7 +76,7 @@ impl ChainInfo for NodeTemplateChainInfo { frame_system::CheckNonce::::from( frame_system::Pallet::::account_nonce(from), ), - frame_system::CheckWeight::::new(), + frame_system::CheckWeight::::new().into(), pallet_transaction_payment::ChargeTransactionPayment::::from(0), ) } diff --git a/bin/node/testing/src/keyring.rs b/bin/node/testing/src/keyring.rs index 4e2d88b4bba33..059533400f409 100644 --- a/bin/node/testing/src/keyring.rs +++ b/bin/node/testing/src/keyring.rs @@ -75,7 +75,7 @@ pub fn signed_extra(nonce: Index, extra_fee: Balance) -> SignedExtra { frame_system::CheckGenesis::new(), frame_system::CheckEra::from(Era::mortal(256, 0)), frame_system::CheckNonce::from(nonce), - frame_system::CheckWeight::new(), + frame_system::CheckWeight::new().into(), pallet_transaction_payment::ChargeTransactionPayment::from(extra_fee), ) } diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 9dee6da89b256..a0350bb1da82a 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -60,6 +60,7 @@ pub mod inherent; pub mod error; pub mod instances; pub mod migrations; +pub mod signed_extensions; pub mod traits; pub mod weights; diff --git a/frame/support/src/signed_extensions.rs b/frame/support/src/signed_extensions.rs new file mode 100644 index 0000000000000..eed2554fee0e0 --- /dev/null +++ b/frame/support/src/signed_extensions.rs @@ -0,0 +1,327 @@ +// This file is part of Substrate. + +// Copyright (C) 2017-2021 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. + +//! Signed Extensions utilities for FRAME. + +use sp_runtime::{ + codec::{Decode, Encode}, + traits::{DispatchInfoOf, PostDispatchInfoOf, SignedExtension}, + transaction_validity::{TransactionPriority, TransactionValidity, TransactionValidityError}, + DispatchResult, +}; +use sp_std::{fmt::Debug, marker::PhantomData, vec::Vec}; + +/// Adjust transaction `priority` returned by the signed extension. +/// +/// The idea for this type is to be able to compose multiple signed extensions +/// and adjust the priority they are returning so that the summed up priority +/// makes sense for a particular runtime. +/// +/// Example: +/// We combine [`frame_system::CheckWeight`] extension and +/// [`frame_transaction_payment::ChargeTransactionPayment`] extensions. +/// +/// Each of them add to `priority`, but the weight is much less important +/// than the fee payment, so we adjust the fee payment by given factor, +/// by multiplying the priority returned by the second extension. +#[derive(Default)] +pub struct AdjustPriority { + ext: S, + adjuster: PhantomData, +} + +impl From for AdjustPriority { + fn from(ext: S) -> Self { + Self { ext, adjuster: Default::default() } + } +} + +impl Debug for AdjustPriority { + #[cfg(feature = "std")] + fn fmt(&self, f: &mut sp_std::fmt::Formatter<'_>) -> sp_std::fmt::Result { + write!(f, "{:?}::priority_adjustment({})", self.ext, V) + } + #[cfg(not(feature = "std"))] + fn fmt(&self, _f: &mut sp_std::fmt::Formatter<'_>) -> sp_std::fmt::Result { + Ok(()) + } +} + +impl Clone for AdjustPriority { + fn clone(&self) -> Self { + Self { ext: self.ext.clone(), adjuster: self.adjuster.clone() } + } +} + +impl PartialEq for AdjustPriority { + fn eq(&self, other: &Self) -> bool { + self.ext == other.ext && self.adjuster == other.adjuster + } +} + +impl Eq for AdjustPriority {} + +impl Encode for AdjustPriority { + fn encode(&self) -> Vec { + self.ext.encode() + } + fn using_encoded R>(&self, f: F) -> R { + self.ext.using_encoded(f) + } + + fn size_hint(&self) -> usize { + self.ext.size_hint() + } + + fn encode_to(&self, dest: &mut T) { + self.ext.encode_to(dest) + } + + fn encoded_size(&self) -> usize { + self.ext.encoded_size() + } +} + +impl Decode for AdjustPriority { + fn decode(input: &mut I) -> Result { + Ok(Self { ext: S::decode(input)?, adjuster: Default::default() }) + } + + fn encoded_fixed_size() -> Option { + S::encoded_fixed_size() + } +} + +impl SignedExtension for AdjustPriority +where + S: SignedExtension, + M: Mode + Send + Sync, +{ + type AccountId = S::AccountId; + type AdditionalSigned = S::AdditionalSigned; + type Call = S::Call; + type Pre = S::Pre; + + const IDENTIFIER: &'static str = S::IDENTIFIER; + + fn additional_signed(&self) -> Result { + self.ext.additional_signed() + } + + fn validate( + &self, + who: &Self::AccountId, + call: &Self::Call, + info: &DispatchInfoOf, + len: usize, + ) -> TransactionValidity { + let mut validity = self.ext.validate(who, call, info, len)?; + validity.priority = M::combine_priority(validity.priority, V); + Ok(validity) + } + + fn pre_dispatch( + self, + who: &Self::AccountId, + call: &Self::Call, + info: &DispatchInfoOf, + len: usize, + ) -> Result { + self.ext.pre_dispatch(who, call, info, len) + } + + fn validate_unsigned( + call: &Self::Call, + info: &DispatchInfoOf, + len: usize, + ) -> TransactionValidity { + let mut validity = S::validate_unsigned(call, info, len)?; + validity.priority = M::combine_priority(validity.priority, V); + Ok(validity) + } + + fn pre_dispatch_unsigned( + call: &Self::Call, + info: &DispatchInfoOf, + len: usize, + ) -> Result { + S::pre_dispatch_unsigned(call, info, len) + } + + fn post_dispatch( + pre: Self::Pre, + info: &DispatchInfoOf, + post_info: &PostDispatchInfoOf, + len: usize, + result: &DispatchResult, + ) -> Result<(), TransactionValidityError> { + S::post_dispatch(pre, info, post_info, len, result) + } + + fn identifier() -> Vec<&'static str> { + S::identifier() + } +} + +/// Combination mode for the adjuster. +pub trait Mode { + /// Return a combination of two transaction priorities. + fn combine_priority(a: TransactionPriority, b: TransactionPriority) -> TransactionPriority; +} + +/// Adding mode for the adjuster. +/// +/// The priorities are added without an overflow. +#[derive(Default)] +pub struct Add; +impl Mode for Add { + fn combine_priority(a: TransactionPriority, b: TransactionPriority) -> TransactionPriority { + a.saturating_add(b) + } +} + +/// Multiplication mode for the adjuster. +/// +/// The priorities are multiplied without an overflow. +#[derive(Default)] +pub struct Multiply; +impl Mode for Multiply { + fn combine_priority(a: TransactionPriority, b: TransactionPriority) -> TransactionPriority { + a.saturating_mul(b) + } +} + +/// Divisive mode for the adjuster. +/// +/// The priorities are divided without an overflow. +/// In case the divisor is 0 we return 0. +#[derive(Default)] +pub struct Divide; +impl Mode for Divide { + fn combine_priority(a: TransactionPriority, b: TransactionPriority) -> TransactionPriority { + if b == 0 { + 0 + } else { + a / b + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use sp_runtime::transaction_validity::ValidTransaction; + + #[derive(PartialEq, Eq, Clone, Debug, Encode, Decode)] + struct TestExtension(TransactionPriority); + + impl Default for TestExtension { + fn default() -> Self { + Self(50) + } + } + + impl SignedExtension for TestExtension { + const IDENTIFIER: &'static str = "TestExtension"; + + type AccountId = (); + type Call = (); + type AdditionalSigned = (); + type Pre = (); + + fn validate( + &self, + _who: &Self::AccountId, + _call: &Self::Call, + _info: &DispatchInfoOf, + _len: usize, + ) -> TransactionValidity { + Ok(ValidTransaction { priority: self.0, ..Default::default() }) + } + + fn validate_unsigned( + _call: &Self::Call, + _info: &DispatchInfoOf, + _len: usize, + ) -> TransactionValidity { + Ok(ValidTransaction { priority: 30, ..Default::default() }) + } + + fn additional_signed(&self) -> Result { + Ok(()) + } + } + + const ADJUSTMENT: TransactionPriority = 10; + + #[test] + fn should_mul_priority_of_signed_transaction() { + type Adjusted = AdjustPriority; + let adj = Adjusted::default(); + + let got = adj.validate(&(), &(), &(), 5).unwrap(); + + assert_eq!(got.priority, 500); + } + + #[test] + fn should_mul_priority_of_unsigned_transaction() { + type Adjusted = AdjustPriority; + + let got = Adjusted::validate_unsigned(&(), &(), 0).unwrap(); + + assert_eq!(got.priority, 300); + } + + #[test] + fn should_add_priority_of_signed_transaction() { + type Adjusted = AdjustPriority; + let adj = Adjusted::default(); + + let got = adj.validate(&(), &(), &(), 5).unwrap(); + + assert_eq!(got.priority, 60); + } + + #[test] + fn should_add_priority_of_unsigned_transaction() { + type Adjusted = AdjustPriority; + + let got = Adjusted::validate_unsigned(&(), &(), 0).unwrap(); + + assert_eq!(got.priority, 40); + } + + #[test] + fn should_div_priority_of_signed_transaction() { + type Adjusted = AdjustPriority; + let adj = Adjusted::default(); + + let got = adj.validate(&(), &(), &(), 5).unwrap(); + + assert_eq!(got.priority, 5); + } + + #[test] + fn should_div_priority_of_unsigned_transaction() { + type Adjusted = AdjustPriority; + + let got = Adjusted::validate_unsigned(&(), &(), 0).unwrap(); + + assert_eq!(got.priority, 3); + } +} diff --git a/frame/system/src/extensions/check_genesis.rs b/frame/system/src/extensions/check_genesis.rs index 4f561f17c3564..1528b4e390727 100644 --- a/frame/system/src/extensions/check_genesis.rs +++ b/frame/system/src/extensions/check_genesis.rs @@ -23,6 +23,11 @@ use sp_runtime::{ }; /// Genesis hash check to provide replay protection between different networks. +/// +/// # Transaction Validity +/// +/// Note that while a transaction with invalid `genesis_hash` will fail to be decoded, +/// the extension does not affect any other fields of `TransactionValidity` directly. #[derive(Encode, Decode, Clone, Eq, PartialEq)] pub struct CheckGenesis(sp_std::marker::PhantomData); diff --git a/frame/system/src/extensions/check_mortality.rs b/frame/system/src/extensions/check_mortality.rs index 6596939eb9d68..1443d2d181c18 100644 --- a/frame/system/src/extensions/check_mortality.rs +++ b/frame/system/src/extensions/check_mortality.rs @@ -26,6 +26,10 @@ use sp_runtime::{ }; /// Check for transaction mortality. +/// +/// # Transaction Validity +/// +/// The extension affects `longevity` of the transaction according to the [`Era`] definition. #[derive(Encode, Decode, Clone, Eq, PartialEq)] pub struct CheckMortality(Era, sp_std::marker::PhantomData); diff --git a/frame/system/src/extensions/check_nonce.rs b/frame/system/src/extensions/check_nonce.rs index 6eaa9f9e02a4b..6a917629c60ce 100644 --- a/frame/system/src/extensions/check_nonce.rs +++ b/frame/system/src/extensions/check_nonce.rs @@ -29,8 +29,11 @@ use sp_std::vec; /// Nonce check and increment to give replay protection for transactions. /// -/// Note that this does not set any priority by default. Make sure that AT LEAST one of the signed -/// extension sets some kind of priority upon validating transactions. +/// # Transaction Validity +/// +/// This extension affects `requires` and `provides` tags of validity, but DOES NOT +/// set the `priority` field. Make sure that AT LEAST one of the signed extension sets +/// some kind of priority upon validating transactions. #[derive(Encode, Decode, Clone, Eq, PartialEq)] pub struct CheckNonce(#[codec(compact)] T::Index); diff --git a/frame/system/src/extensions/check_spec_version.rs b/frame/system/src/extensions/check_spec_version.rs index 7f5629fefa924..658dbb1a410ca 100644 --- a/frame/system/src/extensions/check_spec_version.rs +++ b/frame/system/src/extensions/check_spec_version.rs @@ -20,6 +20,11 @@ use codec::{Decode, Encode}; use sp_runtime::{traits::SignedExtension, transaction_validity::TransactionValidityError}; /// Ensure the runtime version registered in the transaction is the same as at present. +/// +/// # Transaction Validity +/// +/// The transaction with incorrect `spec_version` are considered invalid. The validity +/// is not affected in any other way. #[derive(Encode, Decode, Clone, Eq, PartialEq)] pub struct CheckSpecVersion(sp_std::marker::PhantomData); diff --git a/frame/system/src/extensions/check_tx_version.rs b/frame/system/src/extensions/check_tx_version.rs index badf0292601b6..ae06b3ff2eb30 100644 --- a/frame/system/src/extensions/check_tx_version.rs +++ b/frame/system/src/extensions/check_tx_version.rs @@ -20,6 +20,11 @@ use codec::{Decode, Encode}; use sp_runtime::{traits::SignedExtension, transaction_validity::TransactionValidityError}; /// Ensure the transaction version registered in the transaction is the same as at present. +/// +/// # Transaction Validity +/// +/// The transaction with incorrect `transaction_version` are considered invalid. The validity +/// is not affected in any other way. #[derive(Encode, Decode, Clone, Eq, PartialEq)] pub struct CheckTxVersion(sp_std::marker::PhantomData); diff --git a/frame/system/src/extensions/check_weight.rs b/frame/system/src/extensions/check_weight.rs index 1e7ad9454b4c9..049e4646bdd2a 100644 --- a/frame/system/src/extensions/check_weight.rs +++ b/frame/system/src/extensions/check_weight.rs @@ -31,6 +31,16 @@ use sp_runtime::{ }; /// Block resource (weight) limit check. +/// +/// # Transaction Validity +/// +/// The extension affects `priority` field of `TransationValidity`. The priority depends +/// on the `DispatchClass` and declared `weight`. +/// +/// Note that usually it's desireable to include some fee payment mechanism and use that +/// as a primary source of `priority`. Take a look at +/// [`frame_support::signed_extensions::AdjustPriority`] utility to adjust priority coming +/// from multiple extensions. #[derive(Encode, Decode, Clone, Eq, PartialEq, Default)] pub struct CheckWeight(sp_std::marker::PhantomData); diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index 9e8dbf6cb5d1d..1ad704ae1309d 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -522,6 +522,12 @@ where /// Require the transactor pay for themselves and maybe include a tip to gain additional priority /// in the queue. +/// +/// # Transaction Validity +/// +/// This extension sets the `priority` field of `TransactionValidity` depending on the amount +/// of fee being paid (including the tip) and byte size. Transactions with high per-weight-unit +/// fee or per-byte fee (depending which is greater) will have higher priority. #[derive(Encode, Decode, Clone, Eq, PartialEq)] pub struct ChargeTransactionPayment(#[codec(compact)] BalanceOf);