From d4b2e71c0f20c9d9cf50679d1a819e1378cd0645 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 17 Jul 2019 01:16:35 +0200 Subject: [PATCH 01/14] working poc added. --- .../src/generic/checked_extrinsic.rs | 16 ++--- core/sr-primitives/src/testing.rs | 17 +++-- core/sr-primitives/src/traits.rs | 32 ++++----- core/sr-primitives/src/weights.rs | 68 +++++++++++++++---- srml/balances/src/lib.rs | 5 +- srml/example/src/lib.rs | 15 +--- srml/executive/src/lib.rs | 8 +-- srml/support/src/dispatch.rs | 23 +++++-- srml/system/src/lib.rs | 33 +++++---- 9 files changed, 132 insertions(+), 85 deletions(-) diff --git a/core/sr-primitives/src/generic/checked_extrinsic.rs b/core/sr-primitives/src/generic/checked_extrinsic.rs index 3f73bca07a5f8..ba99576f307e8 100644 --- a/core/sr-primitives/src/generic/checked_extrinsic.rs +++ b/core/sr-primitives/src/generic/checked_extrinsic.rs @@ -22,7 +22,7 @@ use crate::traits::{ self, Member, MaybeDisplay, SignedExtension, DispatchError, Dispatchable, DispatchResult, ValidateUnsigned }; -use crate::weights::{Weigh, Weight}; +use crate::weights::{Weigh, TransactionInfo}; use crate::transaction_validity::TransactionValidity; /// Definition of something that the external world might want to say; its @@ -57,13 +57,13 @@ where } fn validate>(&self, - weight: crate::weights::Weight, + info: crate::weights::TransactionInfo, len: usize, ) -> TransactionValidity { if let Some((ref id, ref extra)) = self.signed { - Extra::validate(extra, id, weight, len).into() + Extra::validate(extra, id, info, len).into() } else { - match Extra::validate_unsigned(weight, len) { + match Extra::validate_unsigned(info, len) { Ok(extra) => match U::validate_unsigned(&self.function) { TransactionValidity::Valid(v) => TransactionValidity::Valid(v.combine_with(extra)), @@ -75,14 +75,14 @@ where } fn dispatch(self, - weight: crate::weights::Weight, + info: crate::weights::TransactionInfo, len: usize, ) -> Result { let maybe_who = if let Some((id, extra)) = self.signed { - Extra::pre_dispatch(extra, &id, weight, len)?; + Extra::pre_dispatch(extra, &id, info, len)?; Some(id) } else { - Extra::pre_dispatch_unsigned(weight, len)?; + Extra::pre_dispatch_unsigned(info, len)?; None }; Ok(self.function.dispatch(Origin::from(maybe_who))) @@ -93,7 +93,7 @@ impl Weigh for CheckedExtrinsic where Call: Weigh, { - fn weigh(&self) -> Weight { + fn weigh(&self) -> TransactionInfo { self.function.weigh() } } diff --git a/core/sr-primitives/src/testing.rs b/core/sr-primitives/src/testing.rs index 61e65a21d5b61..6540fa64fe887 100644 --- a/core/sr-primitives/src/testing.rs +++ b/core/sr-primitives/src/testing.rs @@ -24,7 +24,7 @@ use crate::traits::{ ValidateUnsigned, SignedExtension, Dispatchable, }; use crate::{generic, KeyTypeId}; -use crate::weights::{Weigh, Weight}; +use crate::weights::{Weigh, TransactionInfo, TransactionWeight}; pub use substrate_primitives::H256; use substrate_primitives::U256; use substrate_primitives::ed25519::{Public as AuthorityId}; @@ -240,7 +240,7 @@ impl Applyable for TestXt where /// Checks to see if this is a valid *transaction*. It returns information on it if so. fn validate>(&self, - _weight: Weight, + _info: TransactionInfo, _len: usize, ) -> TransactionValidity { TransactionValidity::Valid(Default::default()) @@ -249,14 +249,14 @@ impl Applyable for TestXt where /// Executes all necessary logic needed prior to dispatch and deconstructs into function call, /// index and sender. fn dispatch(self, - weight: Weight, + info: TransactionInfo, len: usize, ) -> Result { let maybe_who = if let Some(who) = self.0 { - Extra::pre_dispatch(self.2, &who, weight, len)?; + Extra::pre_dispatch(self.2, &who, info, len)?; Some(who) } else { - Extra::pre_dispatch_unsigned(weight, len)?; + Extra::pre_dispatch_unsigned(info, len)?; None }; Ok(self.1.dispatch(maybe_who.into())) @@ -264,8 +264,11 @@ impl Applyable for TestXt where } impl Weigh for TestXt { - fn weigh(&self) -> Weight { + fn weigh(&self) -> TransactionInfo { // for testing: weight == size. - self.0.using_encoded(|d| d.len() as Weight) + TransactionInfo { + weight: self.0.using_encoded(|d| d.len() as u32), + ..Default::default() + } } } diff --git a/core/sr-primitives/src/traits.rs b/core/sr-primitives/src/traits.rs index 0708e561f6c2a..430e1098f62b4 100644 --- a/core/sr-primitives/src/traits.rs +++ b/core/sr-primitives/src/traits.rs @@ -825,7 +825,7 @@ pub trait SignedExtension: fn validate( &self, _who: &Self::AccountId, - _weight: crate::weights::Weight, + _info: crate::weights::TransactionInfo, _len: usize, ) -> Result { Ok(Default::default()) } @@ -833,23 +833,23 @@ pub trait SignedExtension: fn pre_dispatch( self, who: &Self::AccountId, - weight: crate::weights::Weight, + info: crate::weights::TransactionInfo, len: usize, - ) -> Result<(), DispatchError> { self.validate(who, weight, len).map(|_| ()) } + ) -> Result<(), DispatchError> { self.validate(who, info, len).map(|_| ()) } /// Validate an unsigned transaction for the transaction queue. Normally the default /// implementation is fine since `ValidateUnsigned` is a better way of recognising and /// validating unsigned transactions. fn validate_unsigned( - _weight: crate::weights::Weight, + _info: crate::weights::TransactionInfo, _len: usize, ) -> Result { Ok(Default::default()) } /// Do any pre-flight stuff for a unsigned transaction. fn pre_dispatch_unsigned( - weight: crate::weights::Weight, + info: crate::weights::TransactionInfo, len: usize, - ) -> Result<(), DispatchError> { Self::validate_unsigned(weight, len).map(|_| ()) } + ) -> Result<(), DispatchError> { Self::validate_unsigned(info, len).map(|_| ()) } } macro_rules! tuple_impl_indexed { @@ -869,33 +869,33 @@ macro_rules! tuple_impl_indexed { fn validate( &self, who: &Self::AccountId, - weight: crate::weights::Weight, + info: crate::weights::TransactionInfo, len: usize, ) -> Result { - let aggregator = vec![$(<$direct as SignedExtension>::validate(&self.$index, who, weight, len)?),+]; + let aggregator = vec![$(<$direct as SignedExtension>::validate(&self.$index, who, info, len)?),+]; Ok(aggregator.into_iter().fold(ValidTransaction::default(), |acc, a| acc.combine_with(a))) } fn pre_dispatch( self, who: &Self::AccountId, - weight: crate::weights::Weight, + info: crate::weights::TransactionInfo, len: usize, ) -> Result<(), DispatchError> { - $(self.$index.pre_dispatch(who, weight, len)?;)+ + $(self.$index.pre_dispatch(who, info, len)?;)+ Ok(()) } fn validate_unsigned( - weight: crate::weights::Weight, + info: crate::weights::TransactionInfo, len: usize, ) -> Result { - let aggregator = vec![$($direct::validate_unsigned(weight, len)?),+]; + let aggregator = vec![$($direct::validate_unsigned(info, len)?),+]; Ok(aggregator.into_iter().fold(ValidTransaction::default(), |acc, a| acc.combine_with(a))) } fn pre_dispatch_unsigned( - weight: crate::weights::Weight, + info: crate::weights::TransactionInfo, len: usize, ) -> Result<(), DispatchError> { - $($direct::pre_dispatch_unsigned(weight, len)?;)+ + $($direct::pre_dispatch_unsigned(info, len)?;)+ Ok(()) } } @@ -944,14 +944,14 @@ pub trait Applyable: Sized + Send + Sync { /// Checks to see if this is a valid *transaction*. It returns information on it if so. fn validate>(&self, - weight: crate::weights::Weight, + info: crate::weights::TransactionInfo, len: usize, ) -> TransactionValidity; /// Executes all necessary logic needed prior to dispatch and deconstructs into function call, /// index and sender. fn dispatch(self, - weight: crate::weights::Weight, + info: crate::weights::TransactionInfo, len: usize, ) -> Result; } diff --git a/core/sr-primitives/src/weights.rs b/core/sr-primitives/src/weights.rs index fa60737bb89e9..2f92c860ecdda 100644 --- a/core/sr-primitives/src/weights.rs +++ b/core/sr-primitives/src/weights.rs @@ -24,18 +24,39 @@ //! Note that the decl_module macro _cannot_ enforce this and will simply fail //! if an invalid struct is passed in. -/// The final type that each `#[weight = $x:expr]`'s -/// expression must evaluate to. +pub use crate::transaction_validity::TransactionPriority; +use crate::traits::Bounded; + pub type Weight = u32; +#[derive(Copy, Clone, Default)] +pub struct TransactionInfo { + pub weight: Weight, + pub priority: TransactionPriority, + pub is_operational: bool, +} + +impl TransactionInfo { + pub fn will_cause_full_block(&self, + current_weight: Weight, + maximum_weight: Weight, + ) -> bool { + let new_weight = current_weight.saturating_add(self.weight); + let limit = if self.is_operational { maximum_weight / 4 } else { maximum_weight }; + new_weight > limit + } +} + /// A `Call` enum (aka transaction) that can be weighted using the custom weight attribute of /// its dispatchable functions. Is implemented by default in the `decl_module!`. /// /// Both the outer Call enum and the per-module individual ones will implement this. /// The outer enum simply calls the inner ones based on call type. +// TODO: rename this to sth that says: this traits returns a bunch of static meta-information about the tx, +// including but NOT only weight. pub trait Weigh { - /// Return the weight of this call. This is done independently of its encoded size. - fn weigh(&self) -> Weight; + /// Return the (weight, priority) of this call. This is done independently of its encoded size. + fn weigh(&self) -> TransactionInfo; } /// Means of weighing some particular kind of data (`T`). @@ -44,6 +65,16 @@ pub trait WeighData { fn weigh_data(&self, target: T) -> Weight; } +/// Means of prioritizing some transaction based on the input `T`. +pub trait PrioritizeData { + /// Compute the priority. + fn prioritize(&self, target: T) -> TransactionPriority; +} + +pub trait IsOperational { + fn is_operational(&self, targer: T) -> bool; +} + /// Default type used as the weight representative in a `#[weight = x]` attribute. /// /// A user may pass in any other type that implements [`Weigh`]. If not, the `Default` @@ -52,21 +83,32 @@ pub enum TransactionWeight { /// Basic weight (base, byte). /// The values contained are the base weight and byte weight respectively. Fixed(Weight), - /// Maximum fee. This implies that this transaction _might_ get included but - /// no more transaction can be added. This can be done by setting the - /// implementation to _maximum block weight_. - Max, - /// Free. The transaction does not increase the total weight - /// (i.e. is not included in weight calculation). - Free, + Operational(Weight), } impl WeighData for TransactionWeight { fn weigh_data(&self, _: T) -> Weight { match self { TransactionWeight::Fixed(w) => *w, - TransactionWeight::Max => 3 * 1024 * 1024, - TransactionWeight::Free => 0, + TransactionWeight::Operational(w) => *w, + } + } +} + +impl IsOperational for TransactionWeight { + fn is_operational(&self, _: T) -> bool { + match self { + TransactionWeight::Fixed(_) => false, + TransactionWeight::Operational(_) => true, + } + } +} + +impl PrioritizeData for TransactionWeight { + fn prioritize(&self, _: T) -> TransactionPriority { + match self { + TransactionWeight::Fixed(w) => TransactionPriority::from(*w), + TransactionWeight::Operational(_) => Bounded::max_value(), } } } diff --git a/srml/balances/src/lib.rs b/srml/balances/src/lib.rs index 24b414ee0a9eb..d3c7650693a30 100644 --- a/srml/balances/src/lib.rs +++ b/srml/balances/src/lib.rs @@ -1167,7 +1167,7 @@ impl, I: Instance> rstd::fmt::Debug for TakeFees { use primitives::traits::{DispatchError, SaturatedConversion}; use primitives::transaction_validity::ValidTransaction; -use primitives::weights::Weight; +use primitives::weights::TransactionInfo; impl, I: Instance + Clone + Eq> SignedExtension for TakeFees { type AccountId = T::AccountId; @@ -1177,9 +1177,10 @@ impl, I: Instance + Clone + Eq> SignedExtension for TakeFees { fn validate( &self, who: &Self::AccountId, - weight: Weight, + info: TransactionInfo, _len: usize, ) -> rstd::result::Result { + let weight = info.weight; let fee_x = T::Balance::from(weight); // TODO: should be weight_and_size_to_fee(weight, _len) let fee = T::TransactionBaseFee::get() + T::TransactionByteFee::get() * fee_x; diff --git a/srml/example/src/lib.rs b/srml/example/src/lib.rs index 37d091279e479..7fab590f0d552 100644 --- a/srml/example/src/lib.rs +++ b/srml/example/src/lib.rs @@ -396,19 +396,8 @@ decl_module! { // // If you don't respect these rules, it is likely that your chain will be attackable. // - // Each transaction can optionally indicate a weight. The weight is passed in as a - // custom attribute and the value can be anything that implements the `Weigh` - // trait. Most often using substrate's default `TransactionWeight` is enough for you. - // - // A basic weight is a tuple of `(base_weight, byte_weight)`. Upon including each transaction - // in a block, the final weight is calculated as `base_weight + byte_weight * tx_size`. - // If this value, added to the weight of all included transactions, exceeds `MAX_TRANSACTION_WEIGHT`, - // the transaction is not included. If no weight attribute is provided, the `::default()` - // implementation of `TransactionWeight` is used. - // - // The example below showcases a transaction which is relatively costly, but less dependent on - // the input, hence `byte_weight` is configured smaller. - #[weight = TransactionWeight::Basic(100_000, 10)] + // TODO: update weight doc. + #[weight = TransactionWeight::Fixed(0)] fn accumulate_dummy(origin, increase_by: T::Balance) -> Result { // This is a public call, so we ensure that the origin is some signed account. let _sender = ensure_signed(origin)?; diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index 47422d50f182f..7f75f21b39697 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -284,8 +284,8 @@ where // AUDIT: Under no circumstances may this function panic from here onwards. // Decode parameters and dispatch - let weight = xt.weigh(); - let r = Applyable::dispatch(xt, weight, encoded_len) + let transaction_info = xt.weigh(); + let r = Applyable::dispatch(xt, transaction_info, encoded_len) .map_err(internal::ApplyError::from)?; >::note_applied_extrinsic(&r, encoded_len as u32); @@ -339,8 +339,8 @@ where Err(_) => return TransactionValidity::Invalid(UNKNOWN_ERROR), }; - let weight = xt.weigh(); - xt.validate::(weight, encoded_len) + let transaction_info = xt.weigh(); + xt.validate::(transaction_info, encoded_len) } /// Start an offchain worker and generate extrinsics. diff --git a/srml/support/src/dispatch.rs b/srml/support/src/dispatch.rs index ac5e0604446e4..300f00b8ed0cb 100644 --- a/srml/support/src/dispatch.rs +++ b/srml/support/src/dispatch.rs @@ -26,7 +26,9 @@ pub use srml_metadata::{ ModuleConstantMetadata, DefaultByte, DefaultByteGetter, }; pub use sr_primitives::{ - weights::{TransactionWeight, Weigh, Weight, WeighData}, traits::{Dispatchable, DispatchResult} + weights::{TransactionWeight, Weigh, TransactionInfo, WeighData, PrioritizeData, + TransactionPriority, IsOperational}, + traits::{Dispatchable, DispatchResult} }; /// A type that cannot be instantiated. @@ -1111,12 +1113,23 @@ macro_rules! decl_module { impl<$trait_instance: $trait_name $(, $instance: $instantiable)?> $crate::dispatch::Weigh for $call_type<$trait_instance $(, $instance)?> where $( $other_where_bounds )* { - fn weigh(&self) -> $crate::dispatch::Weight { - match self { + fn weigh(&self) -> $crate::dispatch::TransactionInfo { + let weight = match self { $( $call_type::$fn_name($( ref $param_name ),*) => <$crate::dispatch::WeighData<( $( & $param, )* )>>::weigh_data(&$weight, ($( $param_name, )*)), )* $call_type::__PhantomItem(_, _) => { unreachable!("__PhantomItem should never be used.") }, - } + }; + let priority = match self { + $( $call_type::$fn_name($( ref $param_name ),*) => + <$crate::dispatch::PrioritizeData<( $( & $param, )* )>>::prioritize(&$weight, ($( $param_name, )*)), )* + $call_type::__PhantomItem(_, _) => { unreachable!("__PhantomItem should never be used.") }, + }; + let is_operational = match self { + $( $call_type::$fn_name($( ref $param_name ),*) => + <$crate::dispatch::IsOperational<( $( & $param, )* )>>::is_operational(&$weight, ($( $param_name, )*)), )* + $call_type::__PhantomItem(_, _) => { unreachable!("__PhantomItem should never be used.") }, + }; + $crate::dispatch::TransactionInfo { weight, priority, is_operational } } } @@ -1262,7 +1275,7 @@ macro_rules! impl_outer_dispatch { ,)* } impl $crate::dispatch::Weigh for $call_type { - fn weigh(&self) -> $crate::dispatch::Weight { + fn weigh(&self) -> $crate::dispatch::TransactionInfo { match self { $( $call_type::$camelcase(call) => call.weigh(), )* } diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index 897d7c727635f..d0577c066e15a 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -78,7 +78,7 @@ use rstd::prelude::*; use rstd::map; use rstd::marker::PhantomData; use primitives::{ - generic::{self, Era}, weights::Weight, traits::{ + generic::{self, Era}, weights::{Weight, TransactionInfo} , traits::{ self, CheckEqual, SimpleArithmetic, Zero, SignedExtension, SimpleBitOps, Hash, Member, MaybeDisplay, EnsureOrigin, CurrentHeight, BlockNumberToHash, MaybeSerializeDebugButNotDeserialize, MaybeSerializeDebug, StaticLookup, One, Bounded, @@ -755,19 +755,19 @@ impl Module { } } -/// Weight limit check and increment. +/// resource limit check. #[derive(Encode, Decode, Clone, Eq, PartialEq)] pub struct CheckWeight(PhantomData); impl CheckWeight { - fn internal_check_weight(weight: Weight) -> Result<(), DispatchError> { + fn internal_check_weight(info: TransactionInfo) -> Result<(), DispatchError> { let current_weight = Module::::all_extrinsics_weight(); - let next_weight = current_weight.saturating_add(weight); - if next_weight > T::MaximumBlockWeight::get() { - return Err(DispatchError::Payment) + if !info.will_cause_full_block(current_weight, T::MaximumBlockWeight::get()) { + // TODO: better error description. + Err(DispatchError::Payment) + } else { + Ok(()) } - AllExtrinsicsWeight::put(next_weight); - Ok(()) } } @@ -780,21 +780,20 @@ impl SignedExtension for CheckWeight { fn pre_dispatch( self, _who: &Self::AccountId, - weight: Weight, + info: TransactionInfo, _len: usize, ) -> Result<(), DispatchError> { - Self::internal_check_weight(weight) + Self::internal_check_weight(info) } fn validate( &self, _who: &Self::AccountId, - _weight: Weight, + info: TransactionInfo, _len: usize, ) -> Result { - // TODO: check for a maximum size and weight here as well. - // write priority based on tx weight type + tip. - Ok(ValidTransaction::default()) + Self::internal_check_weight(info)?; + Ok(ValidTransaction { priority: info.priority, ..Default::default() }) } } @@ -840,7 +839,7 @@ impl SignedExtension for CheckNonce { fn pre_dispatch( self, who: &Self::AccountId, - _weight: Weight, + _info: TransactionInfo, _len: usize, ) -> Result<(), DispatchError> { let expected = >::get(who); @@ -856,7 +855,7 @@ impl SignedExtension for CheckNonce { fn validate( &self, who: &Self::AccountId, - _weight: Weight, + info: TransactionInfo, _len: usize, ) -> Result { // check index @@ -873,7 +872,7 @@ impl SignedExtension for CheckNonce { }; Ok(ValidTransaction { - priority: _weight as TransactionPriority, + priority: info.weight as TransactionPriority, requires, provides, longevity: TransactionLongevity::max_value(), From c215161838ade8d49446e96ec814b30adcaca2eb Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 17 Jul 2019 11:04:12 +0200 Subject: [PATCH 02/14] some fixes. --- core/sr-primitives/src/testing.rs | 2 +- core/sr-primitives/src/weights.rs | 91 +++++++++++++++---------------- srml/support/src/dispatch.rs | 19 +++---- srml/system/src/lib.rs | 24 +++++--- 4 files changed, 70 insertions(+), 66 deletions(-) diff --git a/core/sr-primitives/src/testing.rs b/core/sr-primitives/src/testing.rs index 6540fa64fe887..bb238025f9a0e 100644 --- a/core/sr-primitives/src/testing.rs +++ b/core/sr-primitives/src/testing.rs @@ -24,7 +24,7 @@ use crate::traits::{ ValidateUnsigned, SignedExtension, Dispatchable, }; use crate::{generic, KeyTypeId}; -use crate::weights::{Weigh, TransactionInfo, TransactionWeight}; +use crate::weights::{Weigh, TransactionInfo}; pub use substrate_primitives::H256; use substrate_primitives::U256; use substrate_primitives::ed25519::{Public as AuthorityId}; diff --git a/core/sr-primitives/src/weights.rs b/core/sr-primitives/src/weights.rs index 2f92c860ecdda..2e431e009e8f1 100644 --- a/core/sr-primitives/src/weights.rs +++ b/core/sr-primitives/src/weights.rs @@ -16,44 +16,59 @@ //! Primitives for transaction weighting. //! -//! Each dispatch function within `decl_module!` can now have an optional -//! `#[weight = $x]` attribute. $x can be any object that implements the -//! `Weigh` trait. By default, All transactions are annotated by -//! `#[weight = TransactionWeight::default()]`. +//! Each dispatch function within `decl_module!` can have an optional `#[weight = $x]` attribute. $x +//! can be any object that implements the `ClassifyDispatch` and `WeighData` traits. By +//! default, All transactions are annotated by `#[weight = TransactionWeight::default()]`. //! //! Note that the decl_module macro _cannot_ enforce this and will simply fail //! if an invalid struct is passed in. pub use crate::transaction_validity::TransactionPriority; -use crate::traits::Bounded; +/// Numeric range of a transaction weight. pub type Weight = u32; -#[derive(Copy, Clone, Default)] -pub struct TransactionInfo { - pub weight: Weight, - pub priority: TransactionPriority, - pub is_operational: bool, +/// A broad range of dispatch types. This is only distinguishing normal, user-triggered transactions +/// and anything beyond which serves a higher purpose to the system (`Operational`). +#[derive(Clone, Copy)] +pub enum DispatchClass { + /// A normal dispatch. + User, + /// An operational dispatch. + Operational, +} + +impl Default for DispatchClass { + fn default() -> Self { + DispatchClass::User + } } -impl TransactionInfo { - pub fn will_cause_full_block(&self, - current_weight: Weight, - maximum_weight: Weight, - ) -> bool { - let new_weight = current_weight.saturating_add(self.weight); - let limit = if self.is_operational { maximum_weight / 4 } else { maximum_weight }; - new_weight > limit +impl From<&TransactionWeight> for DispatchClass { + fn from(tx: &TransactionWeight) -> Self { + match *tx { + TransactionWeight::Operational(_) => DispatchClass::Operational, + TransactionWeight::Fixed(_) => DispatchClass::User, + } } } +/// A bundle of static meta information collected from the `#[weight = $x]` meta tags. +#[derive(Clone, Copy, Default)] +pub struct TransactionInfo { + /// Weight of this transaction. + pub weight: Weight, + /// Class of this transaction. + pub class: DispatchClass, +} + /// A `Call` enum (aka transaction) that can be weighted using the custom weight attribute of /// its dispatchable functions. Is implemented by default in the `decl_module!`. /// /// Both the outer Call enum and the per-module individual ones will implement this. /// The outer enum simply calls the inner ones based on call type. -// TODO: rename this to sth that says: this traits returns a bunch of static meta-information about the tx, -// including but NOT only weight. +// TODO: rename this to sth that says: this traits returns a bunch of static meta-information about +// the tx, including but NOT only weight. Also rename #[weight] to #[meta]? pub trait Weigh { /// Return the (weight, priority) of this call. This is done independently of its encoded size. fn weigh(&self) -> TransactionInfo; @@ -65,24 +80,20 @@ pub trait WeighData { fn weigh_data(&self, target: T) -> Weight; } -/// Means of prioritizing some transaction based on the input `T`. -pub trait PrioritizeData { - /// Compute the priority. - fn prioritize(&self, target: T) -> TransactionPriority; -} - -pub trait IsOperational { - fn is_operational(&self, targer: T) -> bool; +/// Means of classifying a transaction. +pub trait ClassifyDispatch { + /// Classify transaction based on input data `target`. + fn class(&self, target: T) -> DispatchClass; } /// Default type used as the weight representative in a `#[weight = x]` attribute. /// -/// A user may pass in any other type that implements [`Weigh`]. If not, the `Default` +/// A user may pass in any other type that implements the correct traits. If not, the `Default` /// implementation of [`TransactionWeight`] is used. pub enum TransactionWeight { - /// Basic weight (base, byte). - /// The values contained are the base weight and byte weight respectively. + /// A fixed-weight transaction. No dependency on state or input. Fixed(Weight), + /// An operational transaction. Operational(Weight), } @@ -95,21 +106,9 @@ impl WeighData for TransactionWeight { } } -impl IsOperational for TransactionWeight { - fn is_operational(&self, _: T) -> bool { - match self { - TransactionWeight::Fixed(_) => false, - TransactionWeight::Operational(_) => true, - } - } -} - -impl PrioritizeData for TransactionWeight { - fn prioritize(&self, _: T) -> TransactionPriority { - match self { - TransactionWeight::Fixed(w) => TransactionPriority::from(*w), - TransactionWeight::Operational(_) => Bounded::max_value(), - } +impl ClassifyDispatch for TransactionWeight { + fn class(&self, _: T) -> DispatchClass { + DispatchClass::from(self) } } diff --git a/srml/support/src/dispatch.rs b/srml/support/src/dispatch.rs index 300f00b8ed0cb..3ac6a52ca6612 100644 --- a/srml/support/src/dispatch.rs +++ b/srml/support/src/dispatch.rs @@ -25,11 +25,11 @@ pub use srml_metadata::{ FunctionMetadata, DecodeDifferent, DecodeDifferentArray, FunctionArgumentMetadata, ModuleConstantMetadata, DefaultByte, DefaultByteGetter, }; -pub use sr_primitives::{ - weights::{TransactionWeight, Weigh, TransactionInfo, WeighData, PrioritizeData, - TransactionPriority, IsOperational}, - traits::{Dispatchable, DispatchResult} +pub use sr_primitives::weights::{TransactionWeight, Weigh, TransactionInfo, WeighData, + ClassifyDispatch, + TransactionPriority }; +pub use sr_primitives::traits::{Dispatchable, DispatchResult}; /// A type that cannot be instantiated. pub enum Never {} @@ -1119,17 +1119,12 @@ macro_rules! decl_module { <$crate::dispatch::WeighData<( $( & $param, )* )>>::weigh_data(&$weight, ($( $param_name, )*)), )* $call_type::__PhantomItem(_, _) => { unreachable!("__PhantomItem should never be used.") }, }; - let priority = match self { + let class = match self { $( $call_type::$fn_name($( ref $param_name ),*) => - <$crate::dispatch::PrioritizeData<( $( & $param, )* )>>::prioritize(&$weight, ($( $param_name, )*)), )* + <$crate::dispatch::ClassifyDispatch<( $( & $param, )* )>>::class(&$weight, ($( $param_name, )*)), )* $call_type::__PhantomItem(_, _) => { unreachable!("__PhantomItem should never be used.") }, }; - let is_operational = match self { - $( $call_type::$fn_name($( ref $param_name ),*) => - <$crate::dispatch::IsOperational<( $( & $param, )* )>>::is_operational(&$weight, ($( $param_name, )*)), )* - $call_type::__PhantomItem(_, _) => { unreachable!("__PhantomItem should never be used.") }, - }; - $crate::dispatch::TransactionInfo { weight, priority, is_operational } + $crate::dispatch::TransactionInfo { weight, class } } } diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index d0577c066e15a..9eba90962db29 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -78,7 +78,7 @@ use rstd::prelude::*; use rstd::map; use rstd::marker::PhantomData; use primitives::{ - generic::{self, Era}, weights::{Weight, TransactionInfo} , traits::{ + generic::{self, Era}, weights::{Weight, TransactionInfo, DispatchClass} , traits::{ self, CheckEqual, SimpleArithmetic, Zero, SignedExtension, SimpleBitOps, Hash, Member, MaybeDisplay, EnsureOrigin, CurrentHeight, BlockNumberToHash, MaybeSerializeDebugButNotDeserialize, MaybeSerializeDebug, StaticLookup, One, Bounded, @@ -762,11 +762,21 @@ pub struct CheckWeight(PhantomData); impl CheckWeight { fn internal_check_weight(info: TransactionInfo) -> Result<(), DispatchError> { let current_weight = Module::::all_extrinsics_weight(); - if !info.will_cause_full_block(current_weight, T::MaximumBlockWeight::get()) { - // TODO: better error description. - Err(DispatchError::Payment) - } else { - Ok(()) + let next_weight = current_weight.saturating_add(current_weight); + let limit = match info.class { + DispatchClass::Operational => T::MaximumBlockWeight::get(), + DispatchClass::User => T::MaximumBlockWeight::get() / 4 + }; + if next_weight > limit { + return Err(DispatchError::Payment) + } + Ok(()) + } + + fn internal_get_priority(info: TransactionInfo) -> TransactionPriority { + match info.class { + DispatchClass::User => info.weight.into(), + DispatchClass::Operational => Bounded::max_value() } } } @@ -793,7 +803,7 @@ impl SignedExtension for CheckWeight { _len: usize, ) -> Result { Self::internal_check_weight(info)?; - Ok(ValidTransaction { priority: info.priority, ..Default::default() }) + Ok(ValidTransaction { priority: Self::internal_get_priority(info), ..Default::default() }) } } From 34694e74211658e5b5fb7cd576bf9301f5913106 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 17 Jul 2019 11:10:50 +0200 Subject: [PATCH 03/14] Update doc. --- core/sr-primitives/src/weights.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/core/sr-primitives/src/weights.rs b/core/sr-primitives/src/weights.rs index 2e431e009e8f1..0752939aa7127 100644 --- a/core/sr-primitives/src/weights.rs +++ b/core/sr-primitives/src/weights.rs @@ -53,7 +53,7 @@ impl From<&TransactionWeight> for DispatchClass { } } -/// A bundle of static meta information collected from the `#[weight = $x]` meta tags. +/// A bundle of static meta information collected from the `#[weight = $x]` tags. #[derive(Clone, Copy, Default)] pub struct TransactionInfo { /// Weight of this transaction. @@ -70,7 +70,9 @@ pub struct TransactionInfo { // TODO: rename this to sth that says: this traits returns a bunch of static meta-information about // the tx, including but NOT only weight. Also rename #[weight] to #[meta]? pub trait Weigh { - /// Return the (weight, priority) of this call. This is done independently of its encoded size. + /// Return the `TransactionInfo` static information of this call. + /// + /// This is done independently of its encoded size. fn weigh(&self) -> TransactionInfo; } From 3acb9a70069e38568a11e03d6362eb400d48ad68 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 17 Jul 2019 13:42:12 +0200 Subject: [PATCH 04/14] Fix all tests + final logic. --- core/sr-primitives/src/testing.rs | 4 +- core/sr-primitives/src/weights.rs | 4 +- node/executor/src/lib.rs | 4 +- srml/balances/src/lib.rs | 13 ++++-- srml/balances/src/mock.rs | 8 +++- srml/balances/src/tests.rs | 28 ++++++++++--- srml/executive/src/lib.rs | 65 ++++++++++++++--------------- srml/generic-asset/src/lib.rs | 1 + srml/generic-asset/src/mock.rs | 2 + srml/support/Cargo.toml | 2 +- srml/support/src/dispatch.rs | 28 +++++++++---- srml/system/src/lib.rs | 68 +++++++++++++++++++++---------- 12 files changed, 144 insertions(+), 83 deletions(-) diff --git a/core/sr-primitives/src/testing.rs b/core/sr-primitives/src/testing.rs index bb238025f9a0e..90c47521430b5 100644 --- a/core/sr-primitives/src/testing.rs +++ b/core/sr-primitives/src/testing.rs @@ -263,11 +263,11 @@ impl Applyable for TestXt where } } -impl Weigh for TestXt { +impl Weigh for TestXt { fn weigh(&self) -> TransactionInfo { // for testing: weight == size. TransactionInfo { - weight: self.0.using_encoded(|d| d.len() as u32), + weight: self.encode().len() as u32, ..Default::default() } } diff --git a/core/sr-primitives/src/weights.rs b/core/sr-primitives/src/weights.rs index 0752939aa7127..b534867d988af 100644 --- a/core/sr-primitives/src/weights.rs +++ b/core/sr-primitives/src/weights.rs @@ -30,7 +30,8 @@ pub type Weight = u32; /// A broad range of dispatch types. This is only distinguishing normal, user-triggered transactions /// and anything beyond which serves a higher purpose to the system (`Operational`). -#[derive(Clone, Copy)] +#[cfg_attr(feature = "std", derive(Debug))] +#[derive(PartialEq, Eq, Clone, Copy)] pub enum DispatchClass { /// A normal dispatch. User, @@ -54,6 +55,7 @@ impl From<&TransactionWeight> for DispatchClass { } /// A bundle of static meta information collected from the `#[weight = $x]` tags. +#[cfg_attr(feature = "std", derive(PartialEq, Eq, Debug))] #[derive(Clone, Copy, Default)] pub struct TransactionInfo { /// Weight of this transaction. diff --git a/node/executor/src/lib.rs b/node/executor/src/lib.rs index 0f61a4ceade53..490a56850980b 100644 --- a/node/executor/src/lib.rs +++ b/node/executor/src/lib.rs @@ -83,8 +83,8 @@ mod tests { // TODO: fix for being charged based on weight now. fn transfer_fee(extrinsic: &E) -> Balance { >::get() + - >::get() * - (extrinsic.encode().len() as Balance) + >::get() * + (extrinsic.encode().len() as Balance) } fn creation_fee() -> Balance { diff --git a/srml/balances/src/lib.rs b/srml/balances/src/lib.rs index d3c7650693a30..dd827cdb9b55c 100644 --- a/srml/balances/src/lib.rs +++ b/srml/balances/src/lib.rs @@ -1178,13 +1178,18 @@ impl, I: Instance + Clone + Eq> SignedExtension for TakeFees { &self, who: &Self::AccountId, info: TransactionInfo, - _len: usize, + len: usize, ) -> rstd::result::Result { let weight = info.weight; - let fee_x = T::Balance::from(weight); // TODO: should be weight_and_size_to_fee(weight, _len) - let fee = T::TransactionBaseFee::get() + T::TransactionByteFee::get() * fee_x; - let fee = fee + self.0.clone(); + let _weight_fee = T::Balance::from(weight); + + let len = T::Balance::from(len as u32); + let len_fee = T::TransactionBaseFee::get() + .saturating_add(T::TransactionByteFee::get().saturating_mul(len)); + + let tip = self.0.clone(); + let fee = len_fee.saturating_add(tip) /*.saturating_add(weight_fee) */; let imbalance = >::withdraw( who, fee.clone(), diff --git a/srml/balances/src/mock.rs b/srml/balances/src/mock.rs index 3fb29058439d5..ee8cfd8f6dde0 100644 --- a/srml/balances/src/mock.rs +++ b/srml/balances/src/mock.rs @@ -18,7 +18,7 @@ #![cfg(test)] -use primitives::{traits::{IdentityLookup}, testing::Header}; +use primitives::{traits::{IdentityLookup}, testing::Header, weights::{TransactionInfo, Weight}}; use substrate_primitives::{H256, Blake2Hasher}; use runtime_io; use srml_support::{impl_outer_origin, parameter_types, traits::Get}; @@ -34,7 +34,7 @@ thread_local! { static TRANSFER_FEE: RefCell = RefCell::new(0); static CREATION_FEE: RefCell = RefCell::new(0); static TRANSACTION_BASE_FEE: RefCell = RefCell::new(0); - static TRANSACTION_BYTE_FEE: RefCell = RefCell::new(0); + static TRANSACTION_BYTE_FEE: RefCell = RefCell::new(1); } pub struct ExistentialDeposit; @@ -187,3 +187,7 @@ impl ExtBuilder { pub type System = system::Module; pub type Balances = Module; + +pub fn info_from_weight(w: Weight) -> TransactionInfo { + TransactionInfo { weight: w, ..Default::default() } +} \ No newline at end of file diff --git a/srml/balances/src/tests.rs b/srml/balances/src/tests.rs index 2174176b7af5c..f129e3beb94b7 100644 --- a/srml/balances/src/tests.rs +++ b/srml/balances/src/tests.rs @@ -19,7 +19,7 @@ #![cfg(test)] use super::*; -use mock::{Balances, ExtBuilder, Runtime, System}; +use mock::{Balances, ExtBuilder, Runtime, System, info_from_weight}; use runtime_io::with_externalities; use srml_support::{ assert_noop, assert_ok, assert_err, @@ -124,7 +124,12 @@ fn lock_reasons_should_work() { ); assert_ok!(>::reserve(&1, 1)); // NOTE: this causes a fee payment. - assert!( as SignedExtension>::validate(&TakeFees::from(1), &1, 1).is_ok()); + assert!( as SignedExtension>::validate( + &TakeFees::from(1), + &1, + info_from_weight(1), + 0, + ).is_ok()); Balances::set_lock(ID_1, &1, 10, u64::max_value(), WithdrawReason::Reserve.into()); assert_ok!(>::transfer(&1, &2, 1)); @@ -132,12 +137,22 @@ fn lock_reasons_should_work() { >::reserve(&1, 1), "account liquidity restrictions prevent withdrawal" ); - assert!( as SignedExtension>::validate(&TakeFees::from(1), &1, 1).is_ok()); + assert!( as SignedExtension>::validate( + &TakeFees::from(1), + &1, + info_from_weight(1), + 0, + ).is_ok()); Balances::set_lock(ID_1, &1, 10, u64::max_value(), WithdrawReason::TransactionPayment.into()); assert_ok!(>::transfer(&1, &2, 1)); assert_ok!(>::reserve(&1, 1)); - assert!( as SignedExtension>::validate(&TakeFees::from(1), &1, 1).is_err()); + assert!( as SignedExtension>::validate( + &TakeFees::from(1), + &1, + info_from_weight(1), + 0, + ).is_err()); } ); } @@ -741,9 +756,10 @@ fn signed_extension_take_fees_work() { .monied(true) .build(), || { - assert!(TakeFees::::from(0).validate(&1, 10).is_ok()); + let len = 10; + assert!(TakeFees::::from(0).validate(&1, info_from_weight(0), len).is_ok()); assert_eq!(Balances::free_balance(&1), 100 - 20); - assert!(TakeFees::::from(5 /* tipped */).validate(&1, 10).is_ok()); + assert!(TakeFees::::from(5 /* tipped */).validate(&1, info_from_weight(0), len).is_ok()); assert_eq!(Balances::free_balance(&1), 100 - 20 - 25); } ); diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index 7f75f21b39697..0271f619ffee4 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -548,40 +548,35 @@ mod tests { #[test] fn block_weight_limit_enforced() { - let run_test = |should_fail: bool| { - let mut t = new_test_ext(); - let xt = primitives::testing::TestXt(Some(1), Call::transfer(33, 69), extra(0, 0)); - let xt2 = primitives::testing::TestXt(Some(1), Call::transfer(33, 69), extra(1, 0)); - let encoded = xt2.encode(); - let len = if should_fail { ( ::MaximumBlockWeight::get() - 1) as usize } else { encoded.len() }; - let encoded_len = encoded.len() as u32; - with_externalities(&mut t, || { - Executive::initialize_block(&Header::new( - 1, - H256::default(), - H256::default(), - [69u8; 32].into(), - Digest::default(), - )); - assert_eq!(>::all_extrinsics_weight(), 0); - - Executive::apply_extrinsic(xt).unwrap(); - let res = Executive::apply_extrinsic_with_len(xt2, len, Some(encoded)); - - if should_fail { - assert!(res.is_err()); - assert_eq!(>::all_extrinsics_weight(), encoded_len); - assert_eq!(>::extrinsic_index(), Some(1)); + let mut t = new_test_ext(); + // given: TestXt uses the encoded len as fixed Len: + let xt = primitives::testing::TestXt(Some(1), Call::transfer::(33, 0), extra(0, 0)); + let encoded = xt.encode(); + let encoded_len = encoded.len() as u32; + let limit = >::get() / 4; + let num_to_exhaust_block = limit / encoded_len; + with_externalities(&mut t, || { + Executive::initialize_block(&Header::new( + 1, + H256::default(), + H256::default(), + [69u8; 32].into(), + Digest::default(), + )); + assert_eq!(>::all_extrinsics_weight(), 0); + + for nonce in 0..=num_to_exhaust_block { + let xt = primitives::testing::TestXt(Some(1), Call::transfer::(33, 0), extra(nonce.into(), 0)); + let res = Executive::apply_extrinsic(xt); + if nonce != num_to_exhaust_block { + assert_eq!(res.unwrap(), ApplyOutcome::Success); + assert_eq!(>::all_extrinsics_weight(), encoded_len * (nonce + 1)); + assert_eq!(>::extrinsic_index(), Some(nonce + 1)); } else { - assert!(res.is_ok()); - assert_eq!(>::all_extrinsics_weight(), encoded_len * 2); - assert_eq!(>::extrinsic_index(), Some(2)); + assert_eq!(res, Err(ApplyError::CantPay)); } - }); - }; - - run_test(false); - run_test(true); + } + }); } #[test] @@ -592,9 +587,9 @@ mod tests { let len = xt.clone().encode().len() as u32; let mut t = new_test_ext(); with_externalities(&mut t, || { - Executive::apply_extrinsic(xt.clone()).unwrap(); - Executive::apply_extrinsic(x1.clone()).unwrap(); - Executive::apply_extrinsic(x2.clone()).unwrap(); + assert_eq!(Executive::apply_extrinsic(xt.clone()).unwrap(), ApplyOutcome::Success); + assert_eq!(Executive::apply_extrinsic(x1.clone()).unwrap(), ApplyOutcome::Success); + assert_eq!(Executive::apply_extrinsic(x2.clone()).unwrap(), ApplyOutcome::Success); assert_eq!( >::all_extrinsics_weight(), 3 * (0 /*base*/ + len /*len*/ * 1 /*byte*/) diff --git a/srml/generic-asset/src/lib.rs b/srml/generic-asset/src/lib.rs index 60370600a69a6..a3b75e970e1d9 100644 --- a/srml/generic-asset/src/lib.rs +++ b/srml/generic-asset/src/lib.rs @@ -1056,6 +1056,7 @@ impl system::Trait for ElevatedTrait { type Lookup = T::Lookup; type Header = T::Header; type Event = (); + type MaximumBlockWeight = T::MaximumBlockWeight; type BlockHashCount = T::BlockHashCount; } impl Trait for ElevatedTrait { diff --git a/srml/generic-asset/src/mock.rs b/srml/generic-asset/src/mock.rs index 02e18fc335839..6b22f8860c167 100644 --- a/srml/generic-asset/src/mock.rs +++ b/srml/generic-asset/src/mock.rs @@ -40,6 +40,7 @@ impl_outer_origin! { pub struct Test; parameter_types! { pub const BlockHashCount: u64 = 250; + pub const MaximumBlockWeight: u32 = 1024; } impl system::Trait for Test { type Origin = Origin; @@ -51,6 +52,7 @@ impl system::Trait for Test { type Lookup = IdentityLookup; type Header = Header; type Event = TestEvent; + type MaximumBlockWeight = MaximumBlockWeight; type BlockHashCount = BlockHashCount; } diff --git a/srml/support/Cargo.toml b/srml/support/Cargo.toml index 88cad9651a81e..6abe1fb336481 100644 --- a/srml/support/Cargo.toml +++ b/srml/support/Cargo.toml @@ -20,7 +20,7 @@ bitmask = { version = "0.5", default-features = false } [dev-dependencies] pretty_assertions = "0.6.1" -srml-system = { path = "../system", default-features = false } +srml-system = { path = "../system" } [features] default = ["std"] diff --git a/srml/support/src/dispatch.rs b/srml/support/src/dispatch.rs index 3ac6a52ca6612..08b0daf907622 100644 --- a/srml/support/src/dispatch.rs +++ b/srml/support/src/dispatch.rs @@ -1116,12 +1116,12 @@ macro_rules! decl_module { fn weigh(&self) -> $crate::dispatch::TransactionInfo { let weight = match self { $( $call_type::$fn_name($( ref $param_name ),*) => - <$crate::dispatch::WeighData<( $( & $param, )* )>>::weigh_data(&$weight, ($( $param_name, )*)), )* + >::weigh_data(&$weight, ($( $param_name, )*)), )* $call_type::__PhantomItem(_, _) => { unreachable!("__PhantomItem should never be used.") }, }; let class = match self { $( $call_type::$fn_name($( ref $param_name ),*) => - <$crate::dispatch::ClassifyDispatch<( $( & $param, )* )>>::class(&$weight, ($( $param_name, )*)), )* + >::class(&$weight, ($( $param_name, )*)), )* $call_type::__PhantomItem(_, _) => { unreachable!("__PhantomItem should never be used.") }, }; $crate::dispatch::TransactionInfo { weight, class } @@ -1579,6 +1579,7 @@ macro_rules! __check_reserved_fn_name { mod tests { use super::*; use crate::runtime_primitives::traits::{OnInitialize, OnFinalize}; + use sr_primitives::weights::{TransactionInfo, DispatchClass}; pub trait Trait: system::Trait + Sized where Self::AccountId: From { type Origin; @@ -1604,7 +1605,7 @@ mod tests { fn aux_0(_origin) -> Result { unreachable!() } fn aux_1(_origin, #[compact] _data: u32) -> Result { unreachable!() } fn aux_2(_origin, _data: i32, _data2: String) -> Result { unreachable!() } - #[weight = TransactionWeight::Basic(10, 100)] + #[weight = TransactionWeight::Fixed(10)] fn aux_3(_origin) -> Result { unreachable!() } fn aux_4(_origin, _data: i32) -> Result { unreachable!() } fn aux_5(_origin, _data: i32, #[compact] _data2: u32) -> Result { unreachable!() } @@ -1613,8 +1614,8 @@ mod tests { fn on_finalize(n: T::BlockNumber) { if n.into() == 42 { panic!("on_finalize") } } fn offchain_worker() {} - #[weight = TransactionWeight::Max] - fn weighted(_origin) { unreachable!() } + #[weight = TransactionWeight::Operational(5)] + fn operational(_origin) { unreachable!() } } } @@ -1680,7 +1681,7 @@ mod tests { documentation: DecodeDifferent::Encode(&[]), }, FunctionMetadata { - name: DecodeDifferent::Encode("weighted"), + name: DecodeDifferent::Encode("operational"), arguments: DecodeDifferent::Encode(&[]), documentation: DecodeDifferent::Encode(&[]), }, @@ -1755,10 +1756,19 @@ mod tests { #[test] fn weight_should_attach_to_call_enum() { // max weight. not dependent on input. - assert_eq!(Call::::weighted().weight(100), 3 * 1024 * 1024); + assert_eq!( + Call::::operational().weigh(), + TransactionInfo { weight: 5, class: DispatchClass::Operational }, + ); // default weight. - assert_eq!(Call::::aux_0().weight(5), 5 /*tx-len*/); + assert_eq!( + Call::::aux_0().weigh(), + TransactionInfo { weight: 1, class: DispatchClass::User }, + ); // custom basic - assert_eq!(Call::::aux_3().weight(5), 10 + 100 * 5 ); + assert_eq!( + Call::::aux_3().weigh(), + TransactionInfo { weight: 10, class: DispatchClass::User }, + ); } } diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index 9eba90962db29..7e44fc2ce99d7 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -760,9 +760,10 @@ impl Module { pub struct CheckWeight(PhantomData); impl CheckWeight { - fn internal_check_weight(info: TransactionInfo) -> Result<(), DispatchError> { + fn internal_check_weight(info: TransactionInfo) -> Result { let current_weight = Module::::all_extrinsics_weight(); - let next_weight = current_weight.saturating_add(current_weight); + let added_weight = info.weight; + let next_weight = current_weight.saturating_add(added_weight); let limit = match info.class { DispatchClass::Operational => T::MaximumBlockWeight::get(), DispatchClass::User => T::MaximumBlockWeight::get() / 4 @@ -770,7 +771,7 @@ impl CheckWeight { if next_weight > limit { return Err(DispatchError::Payment) } - Ok(()) + Ok(next_weight) } fn internal_get_priority(info: TransactionInfo) -> TransactionPriority { @@ -793,7 +794,9 @@ impl SignedExtension for CheckWeight { info: TransactionInfo, _len: usize, ) -> Result<(), DispatchError> { - Self::internal_check_weight(info) + let next_weight = Self::internal_check_weight(info)?; + AllExtrinsicsWeight::put(next_weight); + Ok(()) } fn validate( @@ -802,7 +805,8 @@ impl SignedExtension for CheckWeight { info: TransactionInfo, _len: usize, ) -> Result { - Self::internal_check_weight(info)?; + let next_weight = Self::internal_check_weight(info)?; + AllExtrinsicsWeight::put(next_weight); Ok(ValidTransaction { priority: Self::internal_get_priority(info), ..Default::default() }) } } @@ -1138,31 +1142,53 @@ mod tests { fn signed_ext_check_nonce_works() { with_externalities(&mut new_test_ext(), || { >::insert(1, 1); + let info = TransactionInfo::default(); + let len = 0_usize; // stale - assert!(CheckNonce::(0).validate(&1, 0).is_err()); - assert!(CheckNonce::(0).pre_dispatch(&1, 0).is_err()); + assert!(CheckNonce::(0).validate(&1, info, len).is_err()); + assert!(CheckNonce::(0).pre_dispatch(&1, info, len).is_err()); // correct - assert!(CheckNonce::(1).validate(&1, 0).is_ok()); - assert!(CheckNonce::(1).pre_dispatch(&1, 0).is_ok()); + assert!(CheckNonce::(1).validate(&1, info, len).is_ok()); + assert!(CheckNonce::(1).pre_dispatch(&1, info, len).is_ok()); // future - assert!(CheckNonce::(5).validate(&1, 0).is_ok()); - assert!(CheckNonce::(5).pre_dispatch(&1, 0).is_err()); + assert!(CheckNonce::(5).validate(&1, info, len).is_ok()); + assert!(CheckNonce::(5).pre_dispatch(&1, info, len).is_err()); }) } #[test] - fn signed_ext_check_weight_works() { + fn signed_ext_check_weight_works_user_tx() { with_externalities(&mut new_test_ext(), || { - // small - AllExtrinsicsWeight::put(512); - assert!(CheckWeight::(PhantomData).pre_dispatch(&1, 100).is_ok()); - // almost - AllExtrinsicsWeight::put(512); - assert!(CheckWeight::(PhantomData).pre_dispatch(&1, 512).is_ok()); - // big - AllExtrinsicsWeight::put(512); - assert!(CheckWeight::(PhantomData).pre_dispatch(&1, 513).is_err()); + let small = TransactionInfo { weight: 100, ..Default::default() }; + let medium = TransactionInfo { weight: >::get() / 4 - 1, ..Default::default() }; + let big = TransactionInfo { weight: >::get() / 4 + 1, ..Default::default() }; + let len = 0_usize; + + let reset_check_weight = |i, f| { + AllExtrinsicsWeight::put(0); + let r = CheckWeight::(PhantomData).pre_dispatch(&1, i, len); + if f { assert!(r.is_err()) } else { assert!(r.is_ok()) } + }; + + reset_check_weight(small, false); + reset_check_weight(medium, false); + reset_check_weight(big, true); + }) + } + #[test] + fn signed_ext_check_weight_works_operational_tx() { + with_externalities(&mut new_test_ext(), || { + let normal = TransactionInfo { weight: 100, ..Default::default() }; + let op = TransactionInfo { weight: 100, class: DispatchClass::Operational }; + let len = 0_usize; + + // given almost full block + AllExtrinsicsWeight::put(>::get() / 4); + // will not fit. + assert!(CheckWeight::(PhantomData).pre_dispatch(&1, normal, len).is_err()); + // will fit. + assert!(CheckWeight::(PhantomData).pre_dispatch(&1, op, len).is_ok()); }) } From 18e898de5ce520253e20041ca029606abe532f6d Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 17 Jul 2019 16:32:49 +0200 Subject: [PATCH 05/14] more refactoring. --- .../src/generic/checked_extrinsic.rs | 10 ++-- core/sr-primitives/src/testing.rs | 6 +-- core/sr-primitives/src/weights.rs | 45 ++++++++--------- srml/example/src/lib.rs | 16 +++++-- srml/executive/src/lib.rs | 14 +++--- srml/support/src/dispatch.rs | 48 +++++++++++-------- 6 files changed, 77 insertions(+), 62 deletions(-) diff --git a/core/sr-primitives/src/generic/checked_extrinsic.rs b/core/sr-primitives/src/generic/checked_extrinsic.rs index ba99576f307e8..b27a11b0cc7c2 100644 --- a/core/sr-primitives/src/generic/checked_extrinsic.rs +++ b/core/sr-primitives/src/generic/checked_extrinsic.rs @@ -22,7 +22,7 @@ use crate::traits::{ self, Member, MaybeDisplay, SignedExtension, DispatchError, Dispatchable, DispatchResult, ValidateUnsigned }; -use crate::weights::{Weigh, TransactionInfo}; +use crate::weights::{DispatchInfo, TransactionInfo}; use crate::transaction_validity::TransactionValidity; /// Definition of something that the external world might want to say; its @@ -89,11 +89,11 @@ where } } -impl Weigh for CheckedExtrinsic +impl DispatchInfo for CheckedExtrinsic where - Call: Weigh, + Call: DispatchInfo, { - fn weigh(&self) -> TransactionInfo { - self.function.weigh() + fn dispatch_info(&self) -> TransactionInfo { + self.function.dispatch_info() } } diff --git a/core/sr-primitives/src/testing.rs b/core/sr-primitives/src/testing.rs index 90c47521430b5..9017937358bd0 100644 --- a/core/sr-primitives/src/testing.rs +++ b/core/sr-primitives/src/testing.rs @@ -24,7 +24,7 @@ use crate::traits::{ ValidateUnsigned, SignedExtension, Dispatchable, }; use crate::{generic, KeyTypeId}; -use crate::weights::{Weigh, TransactionInfo}; +use crate::weights::{DispatchInfo, TransactionInfo}; pub use substrate_primitives::H256; use substrate_primitives::U256; use substrate_primitives::ed25519::{Public as AuthorityId}; @@ -263,8 +263,8 @@ impl Applyable for TestXt where } } -impl Weigh for TestXt { - fn weigh(&self) -> TransactionInfo { +impl DispatchInfo for TestXt { + fn dispatch_info(&self) -> TransactionInfo { // for testing: weight == size. TransactionInfo { weight: self.encode().len() as u32, diff --git a/core/sr-primitives/src/weights.rs b/core/sr-primitives/src/weights.rs index b534867d988af..0b215976921b5 100644 --- a/core/sr-primitives/src/weights.rs +++ b/core/sr-primitives/src/weights.rs @@ -18,7 +18,7 @@ //! //! Each dispatch function within `decl_module!` can have an optional `#[weight = $x]` attribute. $x //! can be any object that implements the `ClassifyDispatch` and `WeighData` traits. By -//! default, All transactions are annotated by `#[weight = TransactionWeight::default()]`. +//! default, All transactions are annotated by `#[weight = WeightedTransaction::default()]`. //! //! Note that the decl_module macro _cannot_ enforce this and will simply fail //! if an invalid struct is passed in. @@ -45,11 +45,11 @@ impl Default for DispatchClass { } } -impl From<&TransactionWeight> for DispatchClass { - fn from(tx: &TransactionWeight) -> Self { +impl From<&WeightedTransaction> for DispatchClass { + fn from(tx: &WeightedTransaction) -> Self { match *tx { - TransactionWeight::Operational(_) => DispatchClass::Operational, - TransactionWeight::Fixed(_) => DispatchClass::User, + WeightedTransaction::Operational(_) => DispatchClass::Operational, + WeightedTransaction::Fixed(_) => DispatchClass::User, } } } @@ -64,18 +64,13 @@ pub struct TransactionInfo { pub class: DispatchClass, } -/// A `Call` enum (aka transaction) that can be weighted using the custom weight attribute of -/// its dispatchable functions. Is implemented by default in the `decl_module!`. -/// -/// Both the outer Call enum and the per-module individual ones will implement this. -/// The outer enum simply calls the inner ones based on call type. -// TODO: rename this to sth that says: this traits returns a bunch of static meta-information about -// the tx, including but NOT only weight. Also rename #[weight] to #[meta]? -pub trait Weigh { - /// Return the `TransactionInfo` static information of this call. +/// A `Call` enum (aka transaction) that can be carry some static information along with it using +/// the `#[weight]` tag. +pub trait DispatchInfo { + /// Return a `TransactionInfo`, containing relevant information of this call. /// /// This is done independently of its encoded size. - fn weigh(&self) -> TransactionInfo; + fn dispatch_info(&self) -> TransactionInfo; } /// Means of weighing some particular kind of data (`T`). @@ -87,40 +82,40 @@ pub trait WeighData { /// Means of classifying a transaction. pub trait ClassifyDispatch { /// Classify transaction based on input data `target`. - fn class(&self, target: T) -> DispatchClass; + fn classify_dispatch(&self, target: T) -> DispatchClass; } /// Default type used as the weight representative in a `#[weight = x]` attribute. /// /// A user may pass in any other type that implements the correct traits. If not, the `Default` -/// implementation of [`TransactionWeight`] is used. -pub enum TransactionWeight { +/// implementation of [`WeightedTransaction`] is used. +pub enum WeightedTransaction { /// A fixed-weight transaction. No dependency on state or input. Fixed(Weight), /// An operational transaction. Operational(Weight), } -impl WeighData for TransactionWeight { +impl WeighData for WeightedTransaction { fn weigh_data(&self, _: T) -> Weight { match self { - TransactionWeight::Fixed(w) => *w, - TransactionWeight::Operational(w) => *w, + WeightedTransaction::Fixed(w) => *w, + WeightedTransaction::Operational(w) => *w, } } } -impl ClassifyDispatch for TransactionWeight { - fn class(&self, _: T) -> DispatchClass { +impl ClassifyDispatch for WeightedTransaction { + fn classify_dispatch(&self, _: T) -> DispatchClass { DispatchClass::from(self) } } -impl Default for TransactionWeight { +impl Default for WeightedTransaction { fn default() -> Self { // This implies that the weight is currently equal to 100, nothing more // for all substrate transactions that do NOT explicitly annotate weight. // TODO #2431 needs to be updated with proper max values. - TransactionWeight::Fixed(1) + WeightedTransaction::Fixed(1) } } diff --git a/srml/example/src/lib.rs b/srml/example/src/lib.rs index 7fab590f0d552..4ba0703e0cddb 100644 --- a/srml/example/src/lib.rs +++ b/srml/example/src/lib.rs @@ -255,7 +255,7 @@ use srml_support::{StorageValue, dispatch::Result, decl_module, decl_storage, decl_event}; use system::{ensure_signed, ensure_root}; -use sr_primitives::weights::TransactionWeight; +use sr_primitives::weights::WeightedTransaction; /// Our module's configuration trait. All our types and consts go in here. If the /// module is dependent on specific other modules, then their configuration traits @@ -396,8 +396,18 @@ decl_module! { // // If you don't respect these rules, it is likely that your chain will be attackable. // - // TODO: update weight doc. - #[weight = TransactionWeight::Fixed(0)] + // Each transaction can define an optional `#[weight]` attribute to convey a set of static + // information about its dispatch. The `system` and `executive` module then use this + // information to properly execute the transaction, whilst keeping the total load of the + // chain in a moderate rate. + // + // the _right-hand-side_ value of the `#[weight]` attribute can be any type that implements + // a set of traits, namely [`WeighData`] and [`ClassifyDispatch`]. The former conveys the + // weight (a numeric representation of pure execution time and difficulty) of the + // transaction and the latter demonstrates the `DispatchClass` of the call. A higher weight + // means a larger transaction (less of which can be placed in a single block). See the + // `CheckWeight` signed extension struct in the `system` module for more information. + #[weight = WeightedTransaction::Fixed(10)] fn accumulate_dummy(origin, increase_by: T::Balance) -> Result { // This is a public call, so we ensure that the origin is some signed account. let _sender = ensure_signed(origin)?; diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index 0271f619ffee4..d3b491c3a1b7a 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -86,7 +86,7 @@ use parity_codec::{Codec, Encode}; use system::{extrinsics_root, DigestOf}; use primitives::{ApplyOutcome, ApplyError}; use primitives::transaction_validity::TransactionValidity; -use primitives::weights::Weigh; +use primitives::weights::DispatchInfo; mod internal { use primitives::traits::DispatchError; @@ -141,7 +141,7 @@ impl< > ExecuteBlock for Executive where Block::Extrinsic: Checkable + Codec, - CheckedOf: Applyable + Weigh, + CheckedOf: Applyable + DispatchInfo, CallOf: Dispatchable, OriginOf: From>, UnsignedValidator: ValidateUnsigned>, @@ -160,7 +160,7 @@ impl< > Executive where Block::Extrinsic: Checkable + Codec, - CheckedOf: Applyable + Weigh, + CheckedOf: Applyable + DispatchInfo, CallOf: Dispatchable, OriginOf: From>, UnsignedValidator: ValidateUnsigned>, @@ -284,8 +284,8 @@ where // AUDIT: Under no circumstances may this function panic from here onwards. // Decode parameters and dispatch - let transaction_info = xt.weigh(); - let r = Applyable::dispatch(xt, transaction_info, encoded_len) + let dispatch_info = xt.dispatch_info(); + let r = Applyable::dispatch(xt, dispatch_info, encoded_len) .map_err(internal::ApplyError::from)?; >::note_applied_extrinsic(&r, encoded_len as u32); @@ -339,8 +339,8 @@ where Err(_) => return TransactionValidity::Invalid(UNKNOWN_ERROR), }; - let transaction_info = xt.weigh(); - xt.validate::(transaction_info, encoded_len) + let dispatch_info = xt.dispatch_info(); + xt.validate::(dispatch_info, encoded_len) } /// Start an offchain worker and generate extrinsics. diff --git a/srml/support/src/dispatch.rs b/srml/support/src/dispatch.rs index 08b0daf907622..ec444815dcc75 100644 --- a/srml/support/src/dispatch.rs +++ b/srml/support/src/dispatch.rs @@ -25,7 +25,7 @@ pub use srml_metadata::{ FunctionMetadata, DecodeDifferent, DecodeDifferentArray, FunctionArgumentMetadata, ModuleConstantMetadata, DefaultByte, DefaultByteGetter, }; -pub use sr_primitives::weights::{TransactionWeight, Weigh, TransactionInfo, WeighData, +pub use sr_primitives::weights::{WeightedTransaction, DispatchInfo, TransactionInfo, WeighData, ClassifyDispatch, TransactionPriority }; @@ -586,7 +586,7 @@ macro_rules! decl_module { { $( $constants )* } [ $( $dispatchables )* ] $(#[doc = $doc_attr])* - #[weight = $crate::dispatch::TransactionWeight::default()] + #[weight = $crate::dispatch::WeightedTransaction::default()] $fn_vis fn $fn_name( $from $(, $(#[$codec_attr])* $param_name : $param )* ) $( -> $result )* { $( $impl )* } @@ -1110,21 +1110,31 @@ macro_rules! decl_module { } // Implement weight calculation function for Call - impl<$trait_instance: $trait_name $(, $instance: $instantiable)?> $crate::dispatch::Weigh + impl<$trait_instance: $trait_name $(, $instance: $instantiable)?> $crate::dispatch::DispatchInfo for $call_type<$trait_instance $(, $instance)?> where $( $other_where_bounds )* { - fn weigh(&self) -> $crate::dispatch::TransactionInfo { - let weight = match self { - $( $call_type::$fn_name($( ref $param_name ),*) => - >::weigh_data(&$weight, ($( $param_name, )*)), )* + fn dispatch_info(&self) -> $crate::dispatch::TransactionInfo { + let _weight = match self { + $( + $call_type::$fn_name($( ref $param_name ),*) => + >::weigh_data( + &$weight, + ($( $param_name, )*) + ), + )* $call_type::__PhantomItem(_, _) => { unreachable!("__PhantomItem should never be used.") }, }; - let class = match self { - $( $call_type::$fn_name($( ref $param_name ),*) => - >::class(&$weight, ($( $param_name, )*)), )* + let _class = match self { + $( + $call_type::$fn_name($( ref $param_name ),*) => + >::classify_dispatch( + &$weight, + ($( $param_name, )*) + ), + )* $call_type::__PhantomItem(_, _) => { unreachable!("__PhantomItem should never be used.") }, }; - $crate::dispatch::TransactionInfo { weight, class } + $crate::dispatch::TransactionInfo { weight: _weight, class: _class } } } @@ -1269,10 +1279,10 @@ macro_rules! impl_outer_dispatch { $camelcase ( $crate::dispatch::CallableCallFor<$camelcase, $runtime> ) ,)* } - impl $crate::dispatch::Weigh for $call_type { - fn weigh(&self) -> $crate::dispatch::TransactionInfo { + impl $crate::dispatch::DispatchInfo for $call_type { + fn dispatch_info(&self) -> $crate::dispatch::TransactionInfo { match self { - $( $call_type::$camelcase(call) => call.weigh(), )* + $( $call_type::$camelcase(call) => call.dispatch_info(), )* } } } @@ -1605,7 +1615,7 @@ mod tests { fn aux_0(_origin) -> Result { unreachable!() } fn aux_1(_origin, #[compact] _data: u32) -> Result { unreachable!() } fn aux_2(_origin, _data: i32, _data2: String) -> Result { unreachable!() } - #[weight = TransactionWeight::Fixed(10)] + #[weight = WeightedTransaction::Fixed(10)] fn aux_3(_origin) -> Result { unreachable!() } fn aux_4(_origin, _data: i32) -> Result { unreachable!() } fn aux_5(_origin, _data: i32, #[compact] _data2: u32) -> Result { unreachable!() } @@ -1614,7 +1624,7 @@ mod tests { fn on_finalize(n: T::BlockNumber) { if n.into() == 42 { panic!("on_finalize") } } fn offchain_worker() {} - #[weight = TransactionWeight::Operational(5)] + #[weight = WeightedTransaction::Operational(5)] fn operational(_origin) { unreachable!() } } } @@ -1757,17 +1767,17 @@ mod tests { fn weight_should_attach_to_call_enum() { // max weight. not dependent on input. assert_eq!( - Call::::operational().weigh(), + Call::::operational().dispatch_info(), TransactionInfo { weight: 5, class: DispatchClass::Operational }, ); // default weight. assert_eq!( - Call::::aux_0().weigh(), + Call::::aux_0().dispatch_info(), TransactionInfo { weight: 1, class: DispatchClass::User }, ); // custom basic assert_eq!( - Call::::aux_3().weigh(), + Call::::aux_3().dispatch_info(), TransactionInfo { weight: 10, class: DispatchClass::User }, ); } From fe5db51321c21ecb5c86af436f668143e4bcc108 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 17 Jul 2019 16:54:49 +0200 Subject: [PATCH 06/14] nits. --- core/sr-primitives/src/weights.rs | 25 +++++++++++++------------ node/executor/src/lib.rs | 1 - 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/core/sr-primitives/src/weights.rs b/core/sr-primitives/src/weights.rs index 0b215976921b5..ae3411e658356 100644 --- a/core/sr-primitives/src/weights.rs +++ b/core/sr-primitives/src/weights.rs @@ -16,9 +16,9 @@ //! Primitives for transaction weighting. //! -//! Each dispatch function within `decl_module!` can have an optional `#[weight = $x]` attribute. $x -//! can be any object that implements the `ClassifyDispatch` and `WeighData` traits. By -//! default, All transactions are annotated by `#[weight = WeightedTransaction::default()]`. +//! Each dispatch function within `decl_module!` can have an optional `#[weight = $x]` attribute. +//! `$x` can be any type that implements the `ClassifyDispatch` and `WeighData` traits. By +//! default, All transactions are annotated with `#[weight = WeightedTransaction::default()]`. //! //! Note that the decl_module macro _cannot_ enforce this and will simply fail //! if an invalid struct is passed in. @@ -45,9 +45,9 @@ impl Default for DispatchClass { } } -impl From<&WeightedTransaction> for DispatchClass { - fn from(tx: &WeightedTransaction) -> Self { - match *tx { +impl From for DispatchClass { + fn from(tx: WeightedTransaction) -> Self { + match tx { WeightedTransaction::Operational(_) => DispatchClass::Operational, WeightedTransaction::Fixed(_) => DispatchClass::User, } @@ -64,10 +64,10 @@ pub struct TransactionInfo { pub class: DispatchClass, } -/// A `Call` enum (aka transaction) that can be carry some static information along with it using -/// the `#[weight]` tag. +/// A `Call` (aka transaction) that can carry some static information along with it, using the +/// `#[weight]` attribute. pub trait DispatchInfo { - /// Return a `TransactionInfo`, containing relevant information of this call. + /// Return a `TransactionInfo`, containing relevant information of this dispatch. /// /// This is done independently of its encoded size. fn dispatch_info(&self) -> TransactionInfo; @@ -81,14 +81,15 @@ pub trait WeighData { /// Means of classifying a transaction. pub trait ClassifyDispatch { - /// Classify transaction based on input data `target`. + /// Classify the transaction based on input data `target`. fn classify_dispatch(&self, target: T) -> DispatchClass; } -/// Default type used as the weight representative in a `#[weight = x]` attribute. +/// Default type used with the `#[weight = x]` attribute in a substrate chain. /// /// A user may pass in any other type that implements the correct traits. If not, the `Default` /// implementation of [`WeightedTransaction`] is used. +#[derive(Clone, Copy)] pub enum WeightedTransaction { /// A fixed-weight transaction. No dependency on state or input. Fixed(Weight), @@ -107,7 +108,7 @@ impl WeighData for WeightedTransaction { impl ClassifyDispatch for WeightedTransaction { fn classify_dispatch(&self, _: T) -> DispatchClass { - DispatchClass::from(self) + DispatchClass::from(*self) } } diff --git a/node/executor/src/lib.rs b/node/executor/src/lib.rs index 490a56850980b..8f4fcc158d13a 100644 --- a/node/executor/src/lib.rs +++ b/node/executor/src/lib.rs @@ -80,7 +80,6 @@ mod tests { type TestExternalities = CoreTestExternalities; - // TODO: fix for being charged based on weight now. fn transfer_fee(extrinsic: &E) -> Balance { >::get() + >::get() * From e124c578f5fee18313f0ba618dbdbe3f1b0ec7e0 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 17 Jul 2019 22:26:23 +0200 Subject: [PATCH 07/14] System block limit in bytes. --- core/sr-primitives/src/traits.rs | 1 + node-template/runtime/src/lib.rs | 3 + node-template/runtime/src/template.rs | 2 + node/runtime/src/lib.rs | 4 +- srml/assets/src/lib.rs | 2 + srml/aura/src/mock.rs | 2 + srml/authorship/src/lib.rs | 2 + srml/balances/src/lib.rs | 1 + srml/balances/src/mock.rs | 2 + srml/collective/src/lib.rs | 2 + srml/contracts/src/tests.rs | 2 + srml/democracy/src/lib.rs | 2 + srml/elections/src/lib.rs | 2 + srml/example/src/lib.rs | 2 + srml/executive/src/lib.rs | 2 + srml/finality-tracker/src/lib.rs | 2 + srml/generic-asset/src/lib.rs | 1 + srml/generic-asset/src/mock.rs | 2 + srml/grandpa/src/mock.rs | 2 + srml/indices/src/mock.rs | 2 + srml/session/src/mock.rs | 2 + srml/staking/src/mock.rs | 2 + srml/system/src/lib.rs | 80 +++++++++++++++++++++++++-- srml/timestamp/src/lib.rs | 2 + srml/treasury/src/lib.rs | 2 + 25 files changed, 121 insertions(+), 7 deletions(-) diff --git a/core/sr-primitives/src/traits.rs b/core/sr-primitives/src/traits.rs index 430e1098f62b4..ea0b7a4ca356b 100644 --- a/core/sr-primitives/src/traits.rs +++ b/core/sr-primitives/src/traits.rs @@ -752,6 +752,7 @@ impl Checkable for T { /// An abstract error concerning an attempt to verify, check or dispatch the transaction. This /// cannot be more concrete because it's designed to work reasonably well over a broad range of /// possible transaction types. +#[cfg_attr(feature = "std", derive(Debug))] pub enum DispatchError { /// General error to do with the inability to pay some fees (e.g. account balance too low). Payment, diff --git a/node-template/runtime/src/lib.rs b/node-template/runtime/src/lib.rs index 92e64e2ea9ca9..aa309cf562476 100644 --- a/node-template/runtime/src/lib.rs +++ b/node-template/runtime/src/lib.rs @@ -113,6 +113,7 @@ pub fn native_version() -> NativeVersion { parameter_types! { pub const BlockHashCount: BlockNumber = 250; pub const MaximumBlockWeight: Weight = 4 * 1024 * 1024; + pub const MaximumBlockSize: u32 = 2 * 1024; } impl system::Trait for Runtime { @@ -138,6 +139,8 @@ impl system::Trait for Runtime { type BlockHashCount = BlockHashCount; /// Maximum weight of each block. With a default weight system of 1byte == 1weight, 4mb is ok. type MaximumBlockWeight = MaximumBlockWeight; + /// Maximum size of all encoded transactions (in bytes) that are allowed in one block. + type MaximumBlockSize = MaximumBlockSize; } impl aura::Trait for Runtime { diff --git a/node-template/runtime/src/template.rs b/node-template/runtime/src/template.rs index 97466d2b6dce5..4801403252022 100644 --- a/node-template/runtime/src/template.rs +++ b/node-template/runtime/src/template.rs @@ -87,6 +87,7 @@ mod tests { parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: Weight = 1024; + pub const MaximumBlockSize: u32 = 2 * 1024; } impl system::Trait for Test { type Origin = Origin; @@ -100,6 +101,7 @@ mod tests { type Event = (); type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; + type MaximumBlockSize = MaximumBlockSize; } impl Trait for Test { type Event = (); diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index 04bc02e46e3b4..58725013987db 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -112,7 +112,8 @@ pub const DAYS: Moment = HOURS * 24; parameter_types! { pub const BlockHashCount: BlockNumber = 250; - pub const MaximumBlockWeight: Weight = 4 * 1024 * 1024; + pub const MaximumBlockWeight: Weight = 4 * 1024; + pub const MaximumBlockSize: u32 = 4 * 1024 * 1024; } impl system::Trait for Runtime { @@ -127,6 +128,7 @@ impl system::Trait for Runtime { type Event = Event; type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; + type MaximumBlockSize = MaximumBlockSize; } impl aura::Trait for Runtime { diff --git a/srml/assets/src/lib.rs b/srml/assets/src/lib.rs index 1e4c06700abe1..437945db97c00 100644 --- a/srml/assets/src/lib.rs +++ b/srml/assets/src/lib.rs @@ -258,6 +258,7 @@ mod tests { parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; + pub const MaximumBlockSize: u32 = 2 * 1024; } impl system::Trait for Test { type Origin = Origin; @@ -271,6 +272,7 @@ mod tests { type Event = (); type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; + type MaximumBlockSize = MaximumBlockSize; } impl Trait for Test { type Event = (); diff --git a/srml/aura/src/mock.rs b/srml/aura/src/mock.rs index 7664405eb37e3..5a6e491a0389a 100644 --- a/srml/aura/src/mock.rs +++ b/srml/aura/src/mock.rs @@ -38,6 +38,7 @@ pub struct Test; parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; + pub const MaximumBlockSize: u32 = 2 * 1024; pub const MinimumPeriod: u64 = 1; } @@ -53,6 +54,7 @@ impl system::Trait for Test { type Event = (); type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; + type MaximumBlockSize = MaximumBlockSize; } impl timestamp::Trait for Test { diff --git a/srml/authorship/src/lib.rs b/srml/authorship/src/lib.rs index 5ba82f5daf2c2..ac46a428d3e78 100644 --- a/srml/authorship/src/lib.rs +++ b/srml/authorship/src/lib.rs @@ -338,6 +338,7 @@ mod tests { parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; + pub const MaximumBlockSize: u32 = 2 * 1024; } impl system::Trait for Test { @@ -352,6 +353,7 @@ mod tests { type Event = (); type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; + type MaximumBlockSize = MaximumBlockSize; } impl Trait for Test { diff --git a/srml/balances/src/lib.rs b/srml/balances/src/lib.rs index dd827cdb9b55c..7b431ca159047 100644 --- a/srml/balances/src/lib.rs +++ b/srml/balances/src/lib.rs @@ -762,6 +762,7 @@ impl, I: Instance> system::Trait for ElevatedTrait { type Event = (); type BlockHashCount = T::BlockHashCount; type MaximumBlockWeight = T::MaximumBlockWeight; + type MaximumBlockSize = T::MaximumBlockSize; } impl, I: Instance> Trait for ElevatedTrait { type Balance = T::Balance; diff --git a/srml/balances/src/mock.rs b/srml/balances/src/mock.rs index ee8cfd8f6dde0..3a8488d06d1c4 100644 --- a/srml/balances/src/mock.rs +++ b/srml/balances/src/mock.rs @@ -68,6 +68,7 @@ pub struct Runtime; parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; + pub const MaximumBlockSize: u32 = 2 * 1024; } impl system::Trait for Runtime { type Origin = Origin; @@ -81,6 +82,7 @@ impl system::Trait for Runtime { type Event = (); type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; + type MaximumBlockSize = MaximumBlockSize; } impl Trait for Runtime { type Balance = u64; diff --git a/srml/collective/src/lib.rs b/srml/collective/src/lib.rs index a8303501d46ea..6cd82eeca97e7 100644 --- a/srml/collective/src/lib.rs +++ b/srml/collective/src/lib.rs @@ -403,6 +403,7 @@ mod tests { parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; + pub const MaximumBlockSize: u32 = 2 * 1024; } impl system::Trait for Test { type Origin = Origin; @@ -416,6 +417,7 @@ mod tests { type Event = Event; type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; + type MaximumBlockSize = MaximumBlockSize; } impl Trait for Test { type Origin = Origin; diff --git a/srml/contracts/src/tests.rs b/srml/contracts/src/tests.rs index adcc90342662e..7459ac06e08fd 100644 --- a/srml/contracts/src/tests.rs +++ b/srml/contracts/src/tests.rs @@ -97,6 +97,7 @@ pub struct Test; parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; + pub const MaximumBlockSize: u32 = 2 * 1024; } impl system::Trait for Test { type Origin = Origin; @@ -110,6 +111,7 @@ impl system::Trait for Test { type Event = MetaEvent; type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; + type MaximumBlockSize = MaximumBlockSize; } parameter_types! { pub const BalancesTransactionBaseFee: u64 = 0; diff --git a/srml/democracy/src/lib.rs b/srml/democracy/src/lib.rs index 5c49f9519013d..188ff9fa81fd2 100644 --- a/srml/democracy/src/lib.rs +++ b/srml/democracy/src/lib.rs @@ -999,6 +999,7 @@ mod tests { parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; + pub const MaximumBlockSize: u32 = 2 * 1024; } impl system::Trait for Test { type Origin = Origin; @@ -1012,6 +1013,7 @@ mod tests { type Event = (); type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; + type MaximumBlockSize = MaximumBlockSize; } parameter_types! { pub const ExistentialDeposit: u64 = 0; diff --git a/srml/elections/src/lib.rs b/srml/elections/src/lib.rs index a1c139349ccee..63a388f7f4ca3 100644 --- a/srml/elections/src/lib.rs +++ b/srml/elections/src/lib.rs @@ -1109,6 +1109,7 @@ mod tests { parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; + pub const MaximumBlockSize: u32 = 2 * 1024; } impl system::Trait for Test { type Origin = Origin; @@ -1122,6 +1123,7 @@ mod tests { type Event = Event; type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; + type MaximumBlockSize = MaximumBlockSize; } parameter_types! { pub const ExistentialDeposit: u64 = 0; diff --git a/srml/example/src/lib.rs b/srml/example/src/lib.rs index 4ba0703e0cddb..9382ad3e9e097 100644 --- a/srml/example/src/lib.rs +++ b/srml/example/src/lib.rs @@ -525,6 +525,7 @@ mod tests { parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; + pub const MaximumBlockSize: u32 = 2 * 1024; } impl system::Trait for Test { type Origin = Origin; @@ -538,6 +539,7 @@ mod tests { type Event = (); type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; + type MaximumBlockSize = MaximumBlockSize; } parameter_types! { pub const ExistentialDeposit: u64 = 0; diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index d3b491c3a1b7a..c16d29157ea93 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -380,6 +380,7 @@ mod tests { parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; + pub const MaximumBlockSize: u32 = 2 * 1024; } impl system::Trait for Runtime { type Origin = Origin; @@ -393,6 +394,7 @@ mod tests { type Event = MetaEvent; type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; + type MaximumBlockSize = MaximumBlockSize; } parameter_types! { pub const ExistentialDeposit: u64 = 0; diff --git a/srml/finality-tracker/src/lib.rs b/srml/finality-tracker/src/lib.rs index 2e17d9688f1c9..3ab936233c1d7 100644 --- a/srml/finality-tracker/src/lib.rs +++ b/srml/finality-tracker/src/lib.rs @@ -300,6 +300,7 @@ mod tests { parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; + pub const MaximumBlockSize: u32 = 2 * 1024; } impl system::Trait for Test { type Origin = Origin; @@ -313,6 +314,7 @@ mod tests { type Event = (); type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; + type MaximumBlockSize = MaximumBlockSize; } parameter_types! { pub const WindowSize: u64 = 11; diff --git a/srml/generic-asset/src/lib.rs b/srml/generic-asset/src/lib.rs index a3b75e970e1d9..b40bd884aa3bc 100644 --- a/srml/generic-asset/src/lib.rs +++ b/srml/generic-asset/src/lib.rs @@ -1057,6 +1057,7 @@ impl system::Trait for ElevatedTrait { type Header = T::Header; type Event = (); type MaximumBlockWeight = T::MaximumBlockWeight; + type MaximumBlockSize = T::MaximumBlockSize; type BlockHashCount = T::BlockHashCount; } impl Trait for ElevatedTrait { diff --git a/srml/generic-asset/src/mock.rs b/srml/generic-asset/src/mock.rs index 6b22f8860c167..603944a6087cf 100644 --- a/srml/generic-asset/src/mock.rs +++ b/srml/generic-asset/src/mock.rs @@ -41,6 +41,7 @@ pub struct Test; parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; + pub const MaximumBlockSize: u32 = 2 * 1024; } impl system::Trait for Test { type Origin = Origin; @@ -53,6 +54,7 @@ impl system::Trait for Test { type Header = Header; type Event = TestEvent; type MaximumBlockWeight = MaximumBlockWeight; + type MaximumBlockSize = MaximumBlockSize; type BlockHashCount = BlockHashCount; } diff --git a/srml/grandpa/src/mock.rs b/srml/grandpa/src/mock.rs index 1420d65eb5576..301190a24d81b 100644 --- a/srml/grandpa/src/mock.rs +++ b/srml/grandpa/src/mock.rs @@ -44,6 +44,7 @@ impl Trait for Test { parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; + pub const MaximumBlockSize: u32 = 2 * 1024; } impl system::Trait for Test { type Origin = Origin; @@ -57,6 +58,7 @@ impl system::Trait for Test { type Event = TestEvent; type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; + type MaximumBlockSize = MaximumBlockSize; } mod grandpa { diff --git a/srml/indices/src/mock.rs b/srml/indices/src/mock.rs index 938aec2bc3311..fe81a889882b1 100644 --- a/srml/indices/src/mock.rs +++ b/srml/indices/src/mock.rs @@ -67,6 +67,7 @@ pub struct Runtime; parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; + pub const MaximumBlockSize: u32 = 2 * 1024; } impl system::Trait for Runtime { type Origin = Origin; @@ -80,6 +81,7 @@ impl system::Trait for Runtime { type Event = (); type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; + type MaximumBlockSize = MaximumBlockSize; } impl Trait for Runtime { type AccountIndex = u64; diff --git a/srml/session/src/mock.rs b/srml/session/src/mock.rs index 915c315b1eba6..e46429a866e33 100644 --- a/srml/session/src/mock.rs +++ b/srml/session/src/mock.rs @@ -110,6 +110,7 @@ pub struct Test; parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; + pub const MaximumBlockSize: u32 = 2 * 1024; pub const MinimumPeriod: u64 = 5; } impl system::Trait for Test { @@ -124,6 +125,7 @@ impl system::Trait for Test { type Event = (); type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; + type MaximumBlockSize = MaximumBlockSize; } impl timestamp::Trait for Test { type Moment = u64; diff --git a/srml/staking/src/mock.rs b/srml/staking/src/mock.rs index 1c420f6e1c47b..f6b0579cd56eb 100644 --- a/srml/staking/src/mock.rs +++ b/srml/staking/src/mock.rs @@ -88,6 +88,7 @@ pub struct Test; parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; + pub const MaximumBlockSize: u32 = 2 * 1024; } impl system::Trait for Test { type Origin = Origin; @@ -101,6 +102,7 @@ impl system::Trait for Test { type Event = (); type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; + type MaximumBlockSize = MaximumBlockSize; } parameter_types! { pub const TransferFee: u64 = 0; diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index 7e44fc2ce99d7..83bd103f9dc7f 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -194,6 +194,9 @@ pub trait Trait: 'static + Eq + Clone { /// The maximum weight of a block. type MaximumBlockWeight: Get; + + /// The maximum size of a block (in bytes). + type MaximumBlockSize: Get; } pub type DigestOf = generic::Digest<::Hash>; @@ -324,7 +327,9 @@ decl_storage! { /// Total extrinsics count for the current block. ExtrinsicCount: Option; /// Total weight for all extrinsics put together, for the current block. - AllExtrinsicsWeight: Option; + AllExtrinsicsWeight: Option; + /// Total length (in bytes) for all extrinsics put together, for the current block. + AllExtrinsicsLen: Option; /// Map of block numbers to block hashes. pub BlockHash get(block_hash) build(|_| vec![(T::BlockNumber::zero(), hash69())]): map T::BlockNumber => T::Hash; /// Extrinsics data for the current block (maps an extrinsic's index to its data). @@ -546,6 +551,11 @@ impl Module { AllExtrinsicsWeight::get().unwrap_or_default() } + /// Gets a total length of all executed extrinsics. + pub fn all_extrinsics_len() -> u32 { + AllExtrinsicsLen::get().unwrap_or_default() + } + /// Start the execution of a particular block. pub fn initialize( number: &T::BlockNumber, @@ -575,6 +585,7 @@ impl Module { pub fn finalize() -> T::Header { ExtrinsicCount::kill(); AllExtrinsicsWeight::kill(); + AllExtrinsicsLen::kill(); let number = >::take(); let parent_hash = >::take(); @@ -769,11 +780,21 @@ impl CheckWeight { DispatchClass::User => T::MaximumBlockWeight::get() / 4 }; if next_weight > limit { - return Err(DispatchError::Payment) + return Err(DispatchError::BadState) } Ok(next_weight) } + fn internal_check_block_size(_info: TransactionInfo, len: usize) -> Result { + let current_len = Module::::all_extrinsics_len(); + let added_len = len as u32; + let next_len = current_len.saturating_add(added_len); + if next_len > T::MaximumBlockSize::get() { + return Err(DispatchError::BadState) + } + Ok(next_len) + } + fn internal_get_priority(info: TransactionInfo) -> TransactionPriority { match info.class { DispatchClass::User => info.weight.into(), @@ -792,8 +813,10 @@ impl SignedExtension for CheckWeight { self, _who: &Self::AccountId, info: TransactionInfo, - _len: usize, + len: usize, ) -> Result<(), DispatchError> { + let next_len = Self::internal_check_block_size(info, len)?; + AllExtrinsicsLen::put(next_len); let next_weight = Self::internal_check_weight(info)?; AllExtrinsicsWeight::put(next_weight); Ok(()) @@ -803,8 +826,10 @@ impl SignedExtension for CheckWeight { &self, _who: &Self::AccountId, info: TransactionInfo, - _len: usize, + len: usize, ) -> Result { + let next_size = Self::internal_check_block_size(info, len)?; + AllExtrinsicsLen::put(next_size); let next_weight = Self::internal_check_weight(info)?; AllExtrinsicsWeight::put(next_weight); Ok(ValidTransaction { priority: Self::internal_get_priority(info), ..Default::default() }) @@ -973,6 +998,7 @@ mod tests { parameter_types! { pub const BlockHashCount: u64 = 10; pub const MaximumBlockWeight: Weight = 1024; + pub const MaximumBlockSize: u32 = 2 * 1024; } impl Trait for Test { @@ -987,6 +1013,7 @@ mod tests { type Event = u16; type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; + type MaximumBlockSize = MaximumBlockSize; } impl From for u16 { @@ -1160,8 +1187,14 @@ mod tests { fn signed_ext_check_weight_works_user_tx() { with_externalities(&mut new_test_ext(), || { let small = TransactionInfo { weight: 100, ..Default::default() }; - let medium = TransactionInfo { weight: >::get() / 4 - 1, ..Default::default() }; - let big = TransactionInfo { weight: >::get() / 4 + 1, ..Default::default() }; + let medium = TransactionInfo { + weight: >::get() / 4 - 1, + ..Default::default() + }; + let big = TransactionInfo { + weight: >::get() / 4 + 1, + ..Default::default() + }; let len = 0_usize; let reset_check_weight = |i, f| { @@ -1192,6 +1225,41 @@ mod tests { }) } + #[test] + fn signed_ext_check_weight_priority_works() { + with_externalities(&mut new_test_ext(), || { + let normal = TransactionInfo { weight: 100, class: DispatchClass::User }; + let op = TransactionInfo { weight: 100, class: DispatchClass::Operational }; + let len = 0_usize; + + assert_eq!( + CheckWeight::(PhantomData).validate(&1, normal, len).unwrap().priority, + 100, + ); + assert_eq!( + CheckWeight::(PhantomData).validate(&1, op, len).unwrap().priority, + Bounded::max_value(), + ); + }) + } + + #[test] + fn signed_ext_check_weight_block_size_works() { + with_externalities(&mut new_test_ext(), || { + let tx = TransactionInfo::default(); + + let reset_check_weight = |s, f| { + AllExtrinsicsLen::put(0); + let r = CheckWeight::(PhantomData).pre_dispatch(&1, tx, s); + if f { assert!(r.is_err()) } else { assert!(r.is_ok()) } + }; + + reset_check_weight(512, false); + reset_check_weight(2 * 1024, false); + reset_check_weight(3 * 1024, true); + }) + } + #[test] fn signed_ext_check_era_should_work() { with_externalities(&mut new_test_ext(), || { diff --git a/srml/timestamp/src/lib.rs b/srml/timestamp/src/lib.rs index b25cab3e03156..8fd1fda69d866 100644 --- a/srml/timestamp/src/lib.rs +++ b/srml/timestamp/src/lib.rs @@ -339,6 +339,7 @@ mod tests { parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; + pub const MaximumBlockSize: u32 = 2 * 1024; } impl system::Trait for Test { type Origin = Origin; @@ -352,6 +353,7 @@ mod tests { type Event = (); type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; + type MaximumBlockSize = MaximumBlockSize; } parameter_types! { pub const MinimumPeriod: u64 = 5; diff --git a/srml/treasury/src/lib.rs b/srml/treasury/src/lib.rs index aa49190d5abb2..00a91013ff575 100644 --- a/srml/treasury/src/lib.rs +++ b/srml/treasury/src/lib.rs @@ -371,6 +371,7 @@ mod tests { parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; + pub const MaximumBlockSize: u32 = 2 * 1024; } impl system::Trait for Test { type Origin = Origin; @@ -384,6 +385,7 @@ mod tests { type Event = (); type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; + type MaximumBlockSize = MaximumBlockSize; } parameter_types! { pub const ExistentialDeposit: u64 = 0; From d1d800457fbf265d47b707269f0a3d50f360c1a5 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 18 Jul 2019 10:12:31 +0200 Subject: [PATCH 08/14] Silent the storage macro warnings. --- srml/balances/src/lib.rs | 2 ++ srml/balances/src/mock.rs | 2 +- srml/council/src/lib.rs | 2 ++ srml/example/src/lib.rs | 2 +- srml/support/src/dispatch.rs | 49 ++++++++++++++++++++---------------- 5 files changed, 34 insertions(+), 23 deletions(-) diff --git a/srml/balances/src/lib.rs b/srml/balances/src/lib.rs index 7b431ca159047..30f95175fb3c9 100644 --- a/srml/balances/src/lib.rs +++ b/srml/balances/src/lib.rs @@ -1200,6 +1200,8 @@ impl, I: Instance + Clone + Eq> SignedExtension for TakeFees { T::TransactionPayment::on_unbalanced(imbalance); let mut r = ValidTransaction::default(); + // NOTE: we probably want to maximise the _fee (of any type) per weight unit_ here, which + // will be a bit more than setting the priority to tip. For now, this is enough. r.priority = fee.saturated_into::(); Ok(r) } diff --git a/srml/balances/src/mock.rs b/srml/balances/src/mock.rs index 3a8488d06d1c4..9675a312fdc52 100644 --- a/srml/balances/src/mock.rs +++ b/srml/balances/src/mock.rs @@ -192,4 +192,4 @@ pub type Balances = Module; pub fn info_from_weight(w: Weight) -> TransactionInfo { TransactionInfo { weight: w, ..Default::default() } -} \ No newline at end of file +} diff --git a/srml/council/src/lib.rs b/srml/council/src/lib.rs index 1baeb6fdfd34b..fec1ff7c43fe5 100644 --- a/srml/council/src/lib.rs +++ b/srml/council/src/lib.rs @@ -99,6 +99,7 @@ mod tests { parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; + pub const MaximumBlockSize: u32 = 2 * 1024; } impl system::Trait for Test { type Origin = Origin; @@ -112,6 +113,7 @@ mod tests { type Event = Event; type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; + type MaximumBlockSize = MaximumBlockSize; } parameter_types! { pub const ExistentialDeposit: u64 = 0; diff --git a/srml/example/src/lib.rs b/srml/example/src/lib.rs index 9382ad3e9e097..78cf4a314487a 100644 --- a/srml/example/src/lib.rs +++ b/srml/example/src/lib.rs @@ -401,7 +401,7 @@ decl_module! { // information to properly execute the transaction, whilst keeping the total load of the // chain in a moderate rate. // - // the _right-hand-side_ value of the `#[weight]` attribute can be any type that implements + // The _right-hand-side_ value of the `#[weight]` attribute can be any type that implements // a set of traits, namely [`WeighData`] and [`ClassifyDispatch`]. The former conveys the // weight (a numeric representation of pure execution time and difficulty) of the // transaction and the latter demonstrates the `DispatchClass` of the call. A higher weight diff --git a/srml/support/src/dispatch.rs b/srml/support/src/dispatch.rs index ec444815dcc75..f8774fd5f835b 100644 --- a/srml/support/src/dispatch.rs +++ b/srml/support/src/dispatch.rs @@ -1114,27 +1114,34 @@ macro_rules! decl_module { for $call_type<$trait_instance $(, $instance)?> where $( $other_where_bounds )* { fn dispatch_info(&self) -> $crate::dispatch::TransactionInfo { - let _weight = match self { - $( - $call_type::$fn_name($( ref $param_name ),*) => - >::weigh_data( - &$weight, - ($( $param_name, )*) - ), - )* - $call_type::__PhantomItem(_, _) => { unreachable!("__PhantomItem should never be used.") }, - }; - let _class = match self { - $( - $call_type::$fn_name($( ref $param_name ),*) => - >::classify_dispatch( - &$weight, - ($( $param_name, )*) - ), - )* - $call_type::__PhantomItem(_, _) => { unreachable!("__PhantomItem should never be used.") }, - }; - $crate::dispatch::TransactionInfo { weight: _weight, class: _class } + $( + if let $call_type::$fn_name($( ref $param_name ),*) = self { + let weight = >::weigh_data( + &$weight, + ($( $param_name, )*) + ); + let class = >::classify_dispatch( + &$weight, + ($( $param_name, )*) + ); + return $crate::dispatch::TransactionInfo { weight, class }; + } + if let $call_type::__PhantomItem(_, _) = self { unreachable!("__PhantomItem should never be used.") } + )* + // Defensive only: this function must have already returned at this point. + // all dispatchable function will have a weight which has the `::default` + // implementation of `WeightedTransaction`. Nonetheless, we create one if it does + // not exist. + let weight = >::weigh_data( + &$crate::dispatch::WeightedTransaction::default(), + () + ); + let class = >::classify_dispatch( + &$crate::dispatch::WeightedTransaction::default(), + () + ); + $crate::dispatch::TransactionInfo { weight, class } + } } From 9c03ff04be275227f2f3450c551fcbdc9925e3fe Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 18 Jul 2019 17:56:41 +0200 Subject: [PATCH 09/14] More logic more tests. --- .../src/generic/checked_extrinsic.rs | 4 +- core/sr-primitives/src/traits.rs | 21 ++--- core/sr-primitives/src/weights.rs | 10 +-- srml/balances/src/lib.rs | 76 +++++++++++++------ srml/balances/src/mock.rs | 4 +- srml/balances/src/tests.rs | 16 ++-- srml/executive/src/lib.rs | 18 +++-- srml/system/src/lib.rs | 41 +++++----- 8 files changed, 119 insertions(+), 71 deletions(-) diff --git a/core/sr-primitives/src/generic/checked_extrinsic.rs b/core/sr-primitives/src/generic/checked_extrinsic.rs index b27a11b0cc7c2..5138ac1cfdf4f 100644 --- a/core/sr-primitives/src/generic/checked_extrinsic.rs +++ b/core/sr-primitives/src/generic/checked_extrinsic.rs @@ -57,7 +57,7 @@ where } fn validate>(&self, - info: crate::weights::TransactionInfo, + info: TransactionInfo, len: usize, ) -> TransactionValidity { if let Some((ref id, ref extra)) = self.signed { @@ -75,7 +75,7 @@ where } fn dispatch(self, - info: crate::weights::TransactionInfo, + info: TransactionInfo, len: usize, ) -> Result { let maybe_who = if let Some((id, extra)) = self.signed { diff --git a/core/sr-primitives/src/traits.rs b/core/sr-primitives/src/traits.rs index ea0b7a4ca356b..02b1fcad776f6 100644 --- a/core/sr-primitives/src/traits.rs +++ b/core/sr-primitives/src/traits.rs @@ -25,6 +25,7 @@ use substrate_primitives::{self, Hasher, Blake2Hasher}; use crate::codec::{Codec, Encode, Decode, HasCompact}; use crate::transaction_validity::{ValidTransaction, TransactionValidity}; use crate::generic::{Digest, DigestItem}; +use TransactionInfo; pub use substrate_primitives::crypto::TypedKey; pub use integer_sqrt::IntegerSquareRoot; pub use num_traits::{ @@ -826,7 +827,7 @@ pub trait SignedExtension: fn validate( &self, _who: &Self::AccountId, - _info: crate::weights::TransactionInfo, + _info: TransactionInfo, _len: usize, ) -> Result { Ok(Default::default()) } @@ -834,7 +835,7 @@ pub trait SignedExtension: fn pre_dispatch( self, who: &Self::AccountId, - info: crate::weights::TransactionInfo, + info: TransactionInfo, len: usize, ) -> Result<(), DispatchError> { self.validate(who, info, len).map(|_| ()) } @@ -842,13 +843,13 @@ pub trait SignedExtension: /// implementation is fine since `ValidateUnsigned` is a better way of recognising and /// validating unsigned transactions. fn validate_unsigned( - _info: crate::weights::TransactionInfo, + _info: TransactionInfo, _len: usize, ) -> Result { Ok(Default::default()) } /// Do any pre-flight stuff for a unsigned transaction. fn pre_dispatch_unsigned( - info: crate::weights::TransactionInfo, + info: TransactionInfo, len: usize, ) -> Result<(), DispatchError> { Self::validate_unsigned(info, len).map(|_| ()) } } @@ -870,7 +871,7 @@ macro_rules! tuple_impl_indexed { fn validate( &self, who: &Self::AccountId, - info: crate::weights::TransactionInfo, + info: TransactionInfo, len: usize, ) -> Result { let aggregator = vec![$(<$direct as SignedExtension>::validate(&self.$index, who, info, len)?),+]; @@ -879,21 +880,21 @@ macro_rules! tuple_impl_indexed { fn pre_dispatch( self, who: &Self::AccountId, - info: crate::weights::TransactionInfo, + info: TransactionInfo, len: usize, ) -> Result<(), DispatchError> { $(self.$index.pre_dispatch(who, info, len)?;)+ Ok(()) } fn validate_unsigned( - info: crate::weights::TransactionInfo, + info: TransactionInfo, len: usize, ) -> Result { let aggregator = vec![$($direct::validate_unsigned(info, len)?),+]; Ok(aggregator.into_iter().fold(ValidTransaction::default(), |acc, a| acc.combine_with(a))) } fn pre_dispatch_unsigned( - info: crate::weights::TransactionInfo, + info: TransactionInfo, len: usize, ) -> Result<(), DispatchError> { $($direct::pre_dispatch_unsigned(info, len)?;)+ @@ -945,14 +946,14 @@ pub trait Applyable: Sized + Send + Sync { /// Checks to see if this is a valid *transaction*. It returns information on it if so. fn validate>(&self, - info: crate::weights::TransactionInfo, + info: TransactionInfo, len: usize, ) -> TransactionValidity; /// Executes all necessary logic needed prior to dispatch and deconstructs into function call, /// index and sender. fn dispatch(self, - info: crate::weights::TransactionInfo, + info: TransactionInfo, len: usize, ) -> Result; } diff --git a/core/sr-primitives/src/weights.rs b/core/sr-primitives/src/weights.rs index ae3411e658356..d15d6ae26b280 100644 --- a/core/sr-primitives/src/weights.rs +++ b/core/sr-primitives/src/weights.rs @@ -54,7 +54,7 @@ impl From for DispatchClass { } } -/// A bundle of static meta information collected from the `#[weight = $x]` tags. +/// A bundle of static information collected from the `#[weight = $x]` attributes. #[cfg_attr(feature = "std", derive(PartialEq, Eq, Debug))] #[derive(Clone, Copy, Default)] pub struct TransactionInfo { @@ -64,7 +64,7 @@ pub struct TransactionInfo { pub class: DispatchClass, } -/// A `Call` (aka transaction) that can carry some static information along with it, using the +/// A `Dispatchable` function (aka transaction) that can carry some static information along with it, using the /// `#[weight]` attribute. pub trait DispatchInfo { /// Return a `TransactionInfo`, containing relevant information of this dispatch. @@ -79,9 +79,9 @@ pub trait WeighData { fn weigh_data(&self, target: T) -> Weight; } -/// Means of classifying a transaction. +/// Means of classifying a dispatchable function. pub trait ClassifyDispatch { - /// Classify the transaction based on input data `target`. + /// Classify the dispatch function based on input data `target` of type `T`. fn classify_dispatch(&self, target: T) -> DispatchClass; } @@ -93,7 +93,7 @@ pub trait ClassifyDispatch { pub enum WeightedTransaction { /// A fixed-weight transaction. No dependency on state or input. Fixed(Weight), - /// An operational transaction. + /// An operational transaction. Still incurs a weight-fee but typically has a higher priority. Operational(Weight), } diff --git a/srml/balances/src/lib.rs b/srml/balances/src/lib.rs index 30f95175fb3c9..ed4a83be5f55f 100644 --- a/srml/balances/src/lib.rs +++ b/srml/balances/src/lib.rs @@ -156,13 +156,15 @@ use srml_support::{StorageValue, StorageMap, Parameter, decl_event, decl_storage use srml_support::traits::{ UpdateBalanceOutcome, Currency, OnFreeBalanceZero, OnUnbalanced, WithdrawReason, WithdrawReasons, LockIdentifier, LockableCurrency, ExistenceRequirement, - Imbalance, SignedImbalance, ReservableCurrency + Imbalance, SignedImbalance, ReservableCurrency, Get, }; -use srml_support::{dispatch::Result, traits::Get}; -use primitives::{transaction_validity::TransactionPriority, traits::{ - Zero, SimpleArithmetic, StaticLookup, Member, CheckedAdd, CheckedSub, - MaybeSerializeDebug, Saturating, Bounded, SignedExtension -}}; +use srml_support::dispatch::Result; +use primitives::traits::{ + Zero, SimpleArithmetic, StaticLookup, Member, CheckedAdd, CheckedSub, MaybeSerializeDebug, + Saturating, Bounded, SignedExtension, SaturatedConversion, DispatchError +}; +use primitives::transaction_validity::{TransactionPriority, ValidTransaction}; +use primitives::weights::{TransactionInfo, Weight}; use system::{IsDeadAccount, OnNewAccount, ensure_signed, ensure_root}; mod mock; @@ -1151,12 +1153,33 @@ where #[derive(Encode, Decode, Clone, Eq, PartialEq)] pub struct TakeFees, I: Instance = DefaultInstance>(#[codec(compact)] T::Balance); -#[cfg(feature = "std")] impl, I: Instance> TakeFees { /// utility constructor. Used only in client/factory code. + #[cfg(feature = "std")] pub fn from(fee: T::Balance) -> Self { Self(fee) } + + /// Compute the final fee value for a particular transaction. + /// + /// The final fee is composed of: + /// - _length-fee_: This is the amount paid merely to pay for size of the transaction. + /// - _weight-fee_: This amount is computer based on the weight of the transaction. Unlike + /// size-fee, this is not input dependent and reflects the _complexity_ of the execution + /// and the time it consumes. + /// - (optional) _tip_: if included in the transaction, it will be added on top. Only signed + /// transactions can have a tip. + fn compute_fee(len: usize, weight: Weight, tip: T::Balance) -> T::Balance { + // length fee + let len = T::Balance::from(len as u32); + let len_fee = T::TransactionBaseFee::get() + .saturating_add(T::TransactionByteFee::get().saturating_mul(len)); + + // weight fee + let _weight_fee = T::Balance::from(weight); // TODO: should be weight_and_size_to_fee(weight, _len) #2854 + + len_fee.saturating_add(tip)/*.saturating_add(_weight_fee)*/ + } } #[cfg(feature = "std")] @@ -1166,41 +1189,48 @@ impl, I: Instance> rstd::fmt::Debug for TakeFees { } } -use primitives::traits::{DispatchError, SaturatedConversion}; -use primitives::transaction_validity::ValidTransaction; -use primitives::weights::TransactionInfo; - impl, I: Instance + Clone + Eq> SignedExtension for TakeFees { type AccountId = T::AccountId; type AdditionalSigned = (); fn additional_signed(&self) -> rstd::result::Result<(), &'static str> { Ok(()) } + fn pre_dispatch( + self, + who: &Self::AccountId, + info: TransactionInfo, + len: usize, + ) -> rstd::result::Result<(), DispatchError> { + // pay any fees. + let fee = Self::compute_fee(len, info.weight, self.0); + let imbalance = >::withdraw( + who, + fee, + WithdrawReason::TransactionPayment, + ExistenceRequirement::KeepAlive, + ).map_err(|_| DispatchError::Payment)?; + T::TransactionPayment::on_unbalanced(imbalance); + Ok(()) + } + fn validate( &self, who: &Self::AccountId, info: TransactionInfo, len: usize, ) -> rstd::result::Result { - let weight = info.weight; - // TODO: should be weight_and_size_to_fee(weight, _len) - let _weight_fee = T::Balance::from(weight); - - let len = T::Balance::from(len as u32); - let len_fee = T::TransactionBaseFee::get() - .saturating_add(T::TransactionByteFee::get().saturating_mul(len)); - - let tip = self.0.clone(); - let fee = len_fee.saturating_add(tip) /*.saturating_add(weight_fee) */; + // check that they can pay all their fees. + let fee = Self::compute_fee(len, info.weight, self.0); + // TODO: just use `>::ensure_can_withdraw()`? and no need to handle imbalance imo. let imbalance = >::withdraw( who, fee.clone(), WithdrawReason::TransactionPayment, - ExistenceRequirement::KeepAlive + ExistenceRequirement::KeepAlive, ).map_err(|_| DispatchError::Payment)?; T::TransactionPayment::on_unbalanced(imbalance); let mut r = ValidTransaction::default(); - // NOTE: we probably want to maximise the _fee (of any type) per weight unit_ here, which + // NOTE: we probably want to maximize the _fee (of any type) per weight unit_ here, which // will be a bit more than setting the priority to tip. For now, this is enough. r.priority = fee.saturated_into::(); Ok(r) diff --git a/srml/balances/src/mock.rs b/srml/balances/src/mock.rs index 9675a312fdc52..bf8bb957f8e71 100644 --- a/srml/balances/src/mock.rs +++ b/srml/balances/src/mock.rs @@ -21,7 +21,8 @@ use primitives::{traits::{IdentityLookup}, testing::Header, weights::{TransactionInfo, Weight}}; use substrate_primitives::{H256, Blake2Hasher}; use runtime_io; -use srml_support::{impl_outer_origin, parameter_types, traits::Get}; +use srml_support::{impl_outer_origin, parameter_types}; +use srml_support::traits::Get; use std::cell::RefCell; use crate::{GenesisConfig, Module, Trait}; @@ -190,6 +191,7 @@ impl ExtBuilder { pub type System = system::Module; pub type Balances = Module; +/// create a transaction info struct from weight. Handy to avoid building the whole struct. pub fn info_from_weight(w: Weight) -> TransactionInfo { TransactionInfo { weight: w, ..Default::default() } } diff --git a/srml/balances/src/tests.rs b/srml/balances/src/tests.rs index f129e3beb94b7..2828d40e63ff8 100644 --- a/srml/balances/src/tests.rs +++ b/srml/balances/src/tests.rs @@ -124,8 +124,8 @@ fn lock_reasons_should_work() { ); assert_ok!(>::reserve(&1, 1)); // NOTE: this causes a fee payment. - assert!( as SignedExtension>::validate( - &TakeFees::from(1), + assert!( as SignedExtension>::pre_dispatch( + TakeFees::from(1), &1, info_from_weight(1), 0, @@ -137,8 +137,8 @@ fn lock_reasons_should_work() { >::reserve(&1, 1), "account liquidity restrictions prevent withdrawal" ); - assert!( as SignedExtension>::validate( - &TakeFees::from(1), + assert!( as SignedExtension>::pre_dispatch( + TakeFees::from(1), &1, info_from_weight(1), 0, @@ -147,8 +147,8 @@ fn lock_reasons_should_work() { Balances::set_lock(ID_1, &1, 10, u64::max_value(), WithdrawReason::TransactionPayment.into()); assert_ok!(>::transfer(&1, &2, 1)); assert_ok!(>::reserve(&1, 1)); - assert!( as SignedExtension>::validate( - &TakeFees::from(1), + assert!( as SignedExtension>::pre_dispatch( + TakeFees::from(1), &1, info_from_weight(1), 0, @@ -757,9 +757,9 @@ fn signed_extension_take_fees_work() { .build(), || { let len = 10; - assert!(TakeFees::::from(0).validate(&1, info_from_weight(0), len).is_ok()); + assert!(TakeFees::::from(0).pre_dispatch(&1, info_from_weight(0), len).is_ok()); assert_eq!(Balances::free_balance(&1), 100 - 20); - assert!(TakeFees::::from(5 /* tipped */).validate(&1, info_from_weight(0), len).is_ok()); + assert!(TakeFees::::from(5 /* tipped */).pre_dispatch(&1, info_from_weight(0), len).is_ok()); assert_eq!(Balances::free_balance(&1), 100 - 20 - 25); } ); diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index c16d29157ea93..6420c50e5e160 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -582,20 +582,28 @@ mod tests { } #[test] - fn default_block_weight_is_stored() { + fn block_weight_and_size_is_stored_per_tx() { let xt = primitives::testing::TestXt(Some(1), Call::transfer(33, 0), extra(0, 0)); let x1 = primitives::testing::TestXt(Some(1), Call::transfer(33, 0), extra(1, 0)); let x2 = primitives::testing::TestXt(Some(1), Call::transfer(33, 0), extra(2, 0)); let len = xt.clone().encode().len() as u32; let mut t = new_test_ext(); with_externalities(&mut t, || { + assert_eq!(>::all_extrinsics_weight(), 0); + assert_eq!(>::all_extrinsics_weight(), 0); + assert_eq!(Executive::apply_extrinsic(xt.clone()).unwrap(), ApplyOutcome::Success); assert_eq!(Executive::apply_extrinsic(x1.clone()).unwrap(), ApplyOutcome::Success); assert_eq!(Executive::apply_extrinsic(x2.clone()).unwrap(), ApplyOutcome::Success); - assert_eq!( - >::all_extrinsics_weight(), - 3 * (0 /*base*/ + len /*len*/ * 1 /*byte*/) - ); + + // default weight for `TestXt` == encoded length. + assert_eq!( >::all_extrinsics_weight(), 3 * len); + assert_eq!(>::all_extrinsics_len(), 3 * len); + + let _ = >::finalize(); + + assert_eq!(>::all_extrinsics_weight(), 0); + assert_eq!(>::all_extrinsics_weight(), 0); }); } diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index 83bd103f9dc7f..83c37e39ebbb8 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -771,7 +771,10 @@ impl Module { pub struct CheckWeight(PhantomData); impl CheckWeight { - fn internal_check_weight(info: TransactionInfo) -> Result { + /// Checks if the current extrinsic can fit into the block with respect to block weight limits. + /// + /// Upon successes, it returns the new block weight as a `Result`. + fn check_weight(info: TransactionInfo) -> Result { let current_weight = Module::::all_extrinsics_weight(); let added_weight = info.weight; let next_weight = current_weight.saturating_add(added_weight); @@ -785,7 +788,10 @@ impl CheckWeight { Ok(next_weight) } - fn internal_check_block_size(_info: TransactionInfo, len: usize) -> Result { + /// Checks if the current extrinsic can fit into the block with respect to block length limits. + /// + /// Upon successes, it returns the new block length as a `Result`. + fn check_block_length(_info: TransactionInfo, len: usize) -> Result { let current_len = Module::::all_extrinsics_len(); let added_len = len as u32; let next_len = current_len.saturating_add(added_len); @@ -795,12 +801,19 @@ impl CheckWeight { Ok(next_len) } - fn internal_get_priority(info: TransactionInfo) -> TransactionPriority { + /// get the priority of an extrinsic denoted by `info`. + fn get_priority(info: TransactionInfo) -> TransactionPriority { match info.class { DispatchClass::User => info.weight.into(), DispatchClass::Operational => Bounded::max_value() } } + + /// Utility constructor for tests and client code. + #[cfg(feature = "std")] + pub fn from() -> Self { + Self(PhantomData) + } } impl SignedExtension for CheckWeight { @@ -815,9 +828,9 @@ impl SignedExtension for CheckWeight { info: TransactionInfo, len: usize, ) -> Result<(), DispatchError> { - let next_len = Self::internal_check_block_size(info, len)?; + let next_len = Self::check_block_length(info, len)?; AllExtrinsicsLen::put(next_len); - let next_weight = Self::internal_check_weight(info)?; + let next_weight = Self::check_weight(info)?; AllExtrinsicsWeight::put(next_weight); Ok(()) } @@ -828,18 +841,12 @@ impl SignedExtension for CheckWeight { info: TransactionInfo, len: usize, ) -> Result { - let next_size = Self::internal_check_block_size(info, len)?; - AllExtrinsicsLen::put(next_size); - let next_weight = Self::internal_check_weight(info)?; - AllExtrinsicsWeight::put(next_weight); - Ok(ValidTransaction { priority: Self::internal_get_priority(info), ..Default::default() }) - } -} - -#[cfg(feature = "std")] -impl CheckWeight { - pub fn from() -> Self { - Self(PhantomData) + // There is no point in writing to storage here since changes are discarded. This basically + // discards any transaction which is bigger than the length or weight limit alone, which is + // a guarantee that it will fail in the pre-dispatch phase. + let _ = Self::check_block_length(info, len)?; + let _ = Self::check_weight(info)?; + Ok(ValidTransaction { priority: Self::get_priority(info), ..Default::default() }) } } From 0ef5cd008020df8cc8e7f67751cad7cf25c42708 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 18 Jul 2019 18:01:28 +0200 Subject: [PATCH 10/14] Fix import. --- core/sr-primitives/src/traits.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/sr-primitives/src/traits.rs b/core/sr-primitives/src/traits.rs index 02b1fcad776f6..8b02aae268828 100644 --- a/core/sr-primitives/src/traits.rs +++ b/core/sr-primitives/src/traits.rs @@ -25,7 +25,7 @@ use substrate_primitives::{self, Hasher, Blake2Hasher}; use crate::codec::{Codec, Encode, Decode, HasCompact}; use crate::transaction_validity::{ValidTransaction, TransactionValidity}; use crate::generic::{Digest, DigestItem}; -use TransactionInfo; +use crate::weights::TransactionInfo; pub use substrate_primitives::crypto::TypedKey; pub use integer_sqrt::IntegerSquareRoot; pub use num_traits::{ From 7a1416ea3386769d58b6e6ceda2537b75657f20f Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 18 Jul 2019 22:36:22 +0200 Subject: [PATCH 11/14] Refactor names. --- .../src/generic/checked_extrinsic.rs | 14 ++-- core/sr-primitives/src/testing.rs | 12 +-- core/sr-primitives/src/traits.rs | 22 +++--- core/sr-primitives/src/weights.rs | 60 ++++++++------ node/executor/src/lib.rs | 4 +- srml/balances/src/lib.rs | 6 +- srml/balances/src/mock.rs | 6 +- srml/example/src/lib.rs | 4 +- srml/executive/src/lib.rs | 10 +-- srml/support/src/dispatch.rs | 42 +++++----- srml/system/src/lib.rs | 79 +++++++++++++------ 11 files changed, 151 insertions(+), 108 deletions(-) diff --git a/core/sr-primitives/src/generic/checked_extrinsic.rs b/core/sr-primitives/src/generic/checked_extrinsic.rs index 5138ac1cfdf4f..04ccd1162c6c6 100644 --- a/core/sr-primitives/src/generic/checked_extrinsic.rs +++ b/core/sr-primitives/src/generic/checked_extrinsic.rs @@ -22,7 +22,7 @@ use crate::traits::{ self, Member, MaybeDisplay, SignedExtension, DispatchError, Dispatchable, DispatchResult, ValidateUnsigned }; -use crate::weights::{DispatchInfo, TransactionInfo}; +use crate::weights::{GetDispatchInfo, DispatchInfo}; use crate::transaction_validity::TransactionValidity; /// Definition of something that the external world might want to say; its @@ -57,7 +57,7 @@ where } fn validate>(&self, - info: TransactionInfo, + info: DispatchInfo, len: usize, ) -> TransactionValidity { if let Some((ref id, ref extra)) = self.signed { @@ -75,7 +75,7 @@ where } fn dispatch(self, - info: TransactionInfo, + info: DispatchInfo, len: usize, ) -> Result { let maybe_who = if let Some((id, extra)) = self.signed { @@ -89,11 +89,11 @@ where } } -impl DispatchInfo for CheckedExtrinsic +impl GetDispatchInfo for CheckedExtrinsic where - Call: DispatchInfo, + Call: GetDispatchInfo, { - fn dispatch_info(&self) -> TransactionInfo { - self.function.dispatch_info() + fn get_dispatch_info(&self) -> DispatchInfo { + self.function.get_dispatch_info() } } diff --git a/core/sr-primitives/src/testing.rs b/core/sr-primitives/src/testing.rs index 9017937358bd0..75f940eb313ad 100644 --- a/core/sr-primitives/src/testing.rs +++ b/core/sr-primitives/src/testing.rs @@ -24,7 +24,7 @@ use crate::traits::{ ValidateUnsigned, SignedExtension, Dispatchable, }; use crate::{generic, KeyTypeId}; -use crate::weights::{DispatchInfo, TransactionInfo}; +use crate::weights::{GetDispatchInfo, DispatchInfo}; pub use substrate_primitives::H256; use substrate_primitives::U256; use substrate_primitives::ed25519::{Public as AuthorityId}; @@ -240,7 +240,7 @@ impl Applyable for TestXt where /// Checks to see if this is a valid *transaction*. It returns information on it if so. fn validate>(&self, - _info: TransactionInfo, + _info: DispatchInfo, _len: usize, ) -> TransactionValidity { TransactionValidity::Valid(Default::default()) @@ -249,7 +249,7 @@ impl Applyable for TestXt where /// Executes all necessary logic needed prior to dispatch and deconstructs into function call, /// index and sender. fn dispatch(self, - info: TransactionInfo, + info: DispatchInfo, len: usize, ) -> Result { let maybe_who = if let Some(who) = self.0 { @@ -263,10 +263,10 @@ impl Applyable for TestXt where } } -impl DispatchInfo for TestXt { - fn dispatch_info(&self) -> TransactionInfo { +impl GetDispatchInfo for TestXt { + fn get_dispatch_info(&self) -> DispatchInfo { // for testing: weight == size. - TransactionInfo { + DispatchInfo { weight: self.encode().len() as u32, ..Default::default() } diff --git a/core/sr-primitives/src/traits.rs b/core/sr-primitives/src/traits.rs index 8b02aae268828..3539e2fcfd814 100644 --- a/core/sr-primitives/src/traits.rs +++ b/core/sr-primitives/src/traits.rs @@ -25,7 +25,7 @@ use substrate_primitives::{self, Hasher, Blake2Hasher}; use crate::codec::{Codec, Encode, Decode, HasCompact}; use crate::transaction_validity::{ValidTransaction, TransactionValidity}; use crate::generic::{Digest, DigestItem}; -use crate::weights::TransactionInfo; +use crate::weights::DispatchInfo; pub use substrate_primitives::crypto::TypedKey; pub use integer_sqrt::IntegerSquareRoot; pub use num_traits::{ @@ -827,7 +827,7 @@ pub trait SignedExtension: fn validate( &self, _who: &Self::AccountId, - _info: TransactionInfo, + _info: DispatchInfo, _len: usize, ) -> Result { Ok(Default::default()) } @@ -835,7 +835,7 @@ pub trait SignedExtension: fn pre_dispatch( self, who: &Self::AccountId, - info: TransactionInfo, + info: DispatchInfo, len: usize, ) -> Result<(), DispatchError> { self.validate(who, info, len).map(|_| ()) } @@ -843,13 +843,13 @@ pub trait SignedExtension: /// implementation is fine since `ValidateUnsigned` is a better way of recognising and /// validating unsigned transactions. fn validate_unsigned( - _info: TransactionInfo, + _info: DispatchInfo, _len: usize, ) -> Result { Ok(Default::default()) } /// Do any pre-flight stuff for a unsigned transaction. fn pre_dispatch_unsigned( - info: TransactionInfo, + info: DispatchInfo, len: usize, ) -> Result<(), DispatchError> { Self::validate_unsigned(info, len).map(|_| ()) } } @@ -871,7 +871,7 @@ macro_rules! tuple_impl_indexed { fn validate( &self, who: &Self::AccountId, - info: TransactionInfo, + info: DispatchInfo, len: usize, ) -> Result { let aggregator = vec![$(<$direct as SignedExtension>::validate(&self.$index, who, info, len)?),+]; @@ -880,21 +880,21 @@ macro_rules! tuple_impl_indexed { fn pre_dispatch( self, who: &Self::AccountId, - info: TransactionInfo, + info: DispatchInfo, len: usize, ) -> Result<(), DispatchError> { $(self.$index.pre_dispatch(who, info, len)?;)+ Ok(()) } fn validate_unsigned( - info: TransactionInfo, + info: DispatchInfo, len: usize, ) -> Result { let aggregator = vec![$($direct::validate_unsigned(info, len)?),+]; Ok(aggregator.into_iter().fold(ValidTransaction::default(), |acc, a| acc.combine_with(a))) } fn pre_dispatch_unsigned( - info: TransactionInfo, + info: DispatchInfo, len: usize, ) -> Result<(), DispatchError> { $($direct::pre_dispatch_unsigned(info, len)?;)+ @@ -946,14 +946,14 @@ pub trait Applyable: Sized + Send + Sync { /// Checks to see if this is a valid *transaction*. It returns information on it if so. fn validate>(&self, - info: TransactionInfo, + info: DispatchInfo, len: usize, ) -> TransactionValidity; /// Executes all necessary logic needed prior to dispatch and deconstructs into function call, /// index and sender. fn dispatch(self, - info: TransactionInfo, + info: DispatchInfo, len: usize, ) -> Result; } diff --git a/core/sr-primitives/src/weights.rs b/core/sr-primitives/src/weights.rs index d15d6ae26b280..6a1d857a63af9 100644 --- a/core/sr-primitives/src/weights.rs +++ b/core/sr-primitives/src/weights.rs @@ -18,18 +18,19 @@ //! //! Each dispatch function within `decl_module!` can have an optional `#[weight = $x]` attribute. //! `$x` can be any type that implements the `ClassifyDispatch` and `WeighData` traits. By -//! default, All transactions are annotated with `#[weight = WeightedTransaction::default()]`. +//! default, All transactions are annotated with `#[weight = SimpleDispatchInfo::default()]`. //! //! Note that the decl_module macro _cannot_ enforce this and will simply fail //! if an invalid struct is passed in. pub use crate::transaction_validity::TransactionPriority; +use crate::traits::Bounded; /// Numeric range of a transaction weight. pub type Weight = u32; /// A broad range of dispatch types. This is only distinguishing normal, user-triggered transactions -/// and anything beyond which serves a higher purpose to the system (`Operational`). +/// and anything beyond which serves a higher purpose to the system (`OperationalNormal`). #[cfg_attr(feature = "std", derive(Debug))] #[derive(PartialEq, Eq, Clone, Copy)] pub enum DispatchClass { @@ -45,11 +46,13 @@ impl Default for DispatchClass { } } -impl From for DispatchClass { - fn from(tx: WeightedTransaction) -> Self { +impl From for DispatchClass { + fn from(tx: SimpleDispatchInfo) -> Self { match tx { - WeightedTransaction::Operational(_) => DispatchClass::Operational, - WeightedTransaction::Fixed(_) => DispatchClass::User, + SimpleDispatchInfo::OperationalNormal(_) => DispatchClass::Operational, + SimpleDispatchInfo::FixedNormal(_) => DispatchClass::User, + SimpleDispatchInfo::MaxNormal => DispatchClass::User, + SimpleDispatchInfo::FreeNormal => DispatchClass::User, } } } @@ -57,7 +60,7 @@ impl From for DispatchClass { /// A bundle of static information collected from the `#[weight = $x]` attributes. #[cfg_attr(feature = "std", derive(PartialEq, Eq, Debug))] #[derive(Clone, Copy, Default)] -pub struct TransactionInfo { +pub struct DispatchInfo { /// Weight of this transaction. pub weight: Weight, /// Class of this transaction. @@ -66,11 +69,11 @@ pub struct TransactionInfo { /// A `Dispatchable` function (aka transaction) that can carry some static information along with it, using the /// `#[weight]` attribute. -pub trait DispatchInfo { - /// Return a `TransactionInfo`, containing relevant information of this dispatch. +pub trait GetDispatchInfo { + /// Return a `DispatchInfo`, containing relevant information of this dispatch. /// /// This is done independently of its encoded size. - fn dispatch_info(&self) -> TransactionInfo; + fn get_dispatch_info(&self) -> DispatchInfo; } /// Means of weighing some particular kind of data (`T`). @@ -88,35 +91,48 @@ pub trait ClassifyDispatch { /// Default type used with the `#[weight = x]` attribute in a substrate chain. /// /// A user may pass in any other type that implements the correct traits. If not, the `Default` -/// implementation of [`WeightedTransaction`] is used. +/// implementation of [`SimpleDispatchInfo`] is used. #[derive(Clone, Copy)] -pub enum WeightedTransaction { - /// A fixed-weight transaction. No dependency on state or input. - Fixed(Weight), - /// An operational transaction. Still incurs a weight-fee but typically has a higher priority. - Operational(Weight), +pub enum SimpleDispatchInfo { + /// A fixed-weight dispatch function. No dependency on state or input. + FixedNormal(Weight), + /// An operational dispatch function. Still incurs a weight-fee but typically has a higher + /// priority. + OperationalNormal(Weight), + /// A dispatch function with the maximum possible weight. This means: + /// - The weight-fee will be maximum possible, based on the system state. + /// - This transaction will also have a high priority (but not guaranteed to be included.) + /// - If included, no more _normal_ dispatches will be permitted in that block. Operational + /// dispatches might be included nonetheless. + MaxNormal, + /// Free. The transaction does not increase the total weight. This means: + /// - No weight-fee is charged. + /// - Block weight is not increased (very likely to be included, but not guaranteed). + FreeNormal, } -impl WeighData for WeightedTransaction { +impl WeighData for SimpleDispatchInfo { fn weigh_data(&self, _: T) -> Weight { match self { - WeightedTransaction::Fixed(w) => *w, - WeightedTransaction::Operational(w) => *w, + SimpleDispatchInfo::FixedNormal(w) => *w, + SimpleDispatchInfo::OperationalNormal(w) => *w, + SimpleDispatchInfo::MaxNormal => Bounded::max_value(), + SimpleDispatchInfo::FreeNormal => Bounded::min_value(), } } } -impl ClassifyDispatch for WeightedTransaction { +impl ClassifyDispatch for SimpleDispatchInfo { fn classify_dispatch(&self, _: T) -> DispatchClass { DispatchClass::from(*self) } } -impl Default for WeightedTransaction { +impl Default for SimpleDispatchInfo { fn default() -> Self { // This implies that the weight is currently equal to 100, nothing more // for all substrate transactions that do NOT explicitly annotate weight. // TODO #2431 needs to be updated with proper max values. - WeightedTransaction::Fixed(1) + SimpleDispatchInfo::FixedNormal(1) } } diff --git a/node/executor/src/lib.rs b/node/executor/src/lib.rs index 8f4fcc158d13a..5e142a64b681f 100644 --- a/node/executor/src/lib.rs +++ b/node/executor/src/lib.rs @@ -82,8 +82,8 @@ mod tests { fn transfer_fee(extrinsic: &E) -> Balance { >::get() + - >::get() * - (extrinsic.encode().len() as Balance) + >::get() * + (extrinsic.encode().len() as Balance) } fn creation_fee() -> Balance { diff --git a/srml/balances/src/lib.rs b/srml/balances/src/lib.rs index ed4a83be5f55f..11bcc9a5dab0b 100644 --- a/srml/balances/src/lib.rs +++ b/srml/balances/src/lib.rs @@ -164,7 +164,7 @@ use primitives::traits::{ Saturating, Bounded, SignedExtension, SaturatedConversion, DispatchError }; use primitives::transaction_validity::{TransactionPriority, ValidTransaction}; -use primitives::weights::{TransactionInfo, Weight}; +use primitives::weights::{DispatchInfo, Weight}; use system::{IsDeadAccount, OnNewAccount, ensure_signed, ensure_root}; mod mock; @@ -1197,7 +1197,7 @@ impl, I: Instance + Clone + Eq> SignedExtension for TakeFees { fn pre_dispatch( self, who: &Self::AccountId, - info: TransactionInfo, + info: DispatchInfo, len: usize, ) -> rstd::result::Result<(), DispatchError> { // pay any fees. @@ -1215,7 +1215,7 @@ impl, I: Instance + Clone + Eq> SignedExtension for TakeFees { fn validate( &self, who: &Self::AccountId, - info: TransactionInfo, + info: DispatchInfo, len: usize, ) -> rstd::result::Result { // check that they can pay all their fees. diff --git a/srml/balances/src/mock.rs b/srml/balances/src/mock.rs index bf8bb957f8e71..0f9e4a3a0c2cf 100644 --- a/srml/balances/src/mock.rs +++ b/srml/balances/src/mock.rs @@ -18,7 +18,7 @@ #![cfg(test)] -use primitives::{traits::{IdentityLookup}, testing::Header, weights::{TransactionInfo, Weight}}; +use primitives::{traits::{IdentityLookup}, testing::Header, weights::{DispatchInfo, Weight}}; use substrate_primitives::{H256, Blake2Hasher}; use runtime_io; use srml_support::{impl_outer_origin, parameter_types}; @@ -192,6 +192,6 @@ pub type System = system::Module; pub type Balances = Module; /// create a transaction info struct from weight. Handy to avoid building the whole struct. -pub fn info_from_weight(w: Weight) -> TransactionInfo { - TransactionInfo { weight: w, ..Default::default() } +pub fn info_from_weight(w: Weight) -> DispatchInfo { + DispatchInfo { weight: w, ..Default::default() } } diff --git a/srml/example/src/lib.rs b/srml/example/src/lib.rs index 78cf4a314487a..cb9f22a7094ee 100644 --- a/srml/example/src/lib.rs +++ b/srml/example/src/lib.rs @@ -255,7 +255,7 @@ use srml_support::{StorageValue, dispatch::Result, decl_module, decl_storage, decl_event}; use system::{ensure_signed, ensure_root}; -use sr_primitives::weights::WeightedTransaction; +use sr_primitives::weights::SimpleDispatchInfo; /// Our module's configuration trait. All our types and consts go in here. If the /// module is dependent on specific other modules, then their configuration traits @@ -407,7 +407,7 @@ decl_module! { // transaction and the latter demonstrates the `DispatchClass` of the call. A higher weight // means a larger transaction (less of which can be placed in a single block). See the // `CheckWeight` signed extension struct in the `system` module for more information. - #[weight = WeightedTransaction::Fixed(10)] + #[weight = SimpleDispatchInfo::FixedNormal(10_000)] fn accumulate_dummy(origin, increase_by: T::Balance) -> Result { // This is a public call, so we ensure that the origin is some signed account. let _sender = ensure_signed(origin)?; diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index 6420c50e5e160..a86786d684c39 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -86,7 +86,7 @@ use parity_codec::{Codec, Encode}; use system::{extrinsics_root, DigestOf}; use primitives::{ApplyOutcome, ApplyError}; use primitives::transaction_validity::TransactionValidity; -use primitives::weights::DispatchInfo; +use primitives::weights::GetDispatchInfo; mod internal { use primitives::traits::DispatchError; @@ -141,7 +141,7 @@ impl< > ExecuteBlock for Executive where Block::Extrinsic: Checkable + Codec, - CheckedOf: Applyable + DispatchInfo, + CheckedOf: Applyable + GetDispatchInfo, CallOf: Dispatchable, OriginOf: From>, UnsignedValidator: ValidateUnsigned>, @@ -160,7 +160,7 @@ impl< > Executive where Block::Extrinsic: Checkable + Codec, - CheckedOf: Applyable + DispatchInfo, + CheckedOf: Applyable + GetDispatchInfo, CallOf: Dispatchable, OriginOf: From>, UnsignedValidator: ValidateUnsigned>, @@ -284,7 +284,7 @@ where // AUDIT: Under no circumstances may this function panic from here onwards. // Decode parameters and dispatch - let dispatch_info = xt.dispatch_info(); + let dispatch_info = xt.get_dispatch_info(); let r = Applyable::dispatch(xt, dispatch_info, encoded_len) .map_err(internal::ApplyError::from)?; @@ -339,7 +339,7 @@ where Err(_) => return TransactionValidity::Invalid(UNKNOWN_ERROR), }; - let dispatch_info = xt.dispatch_info(); + let dispatch_info = xt.get_dispatch_info(); xt.validate::(dispatch_info, encoded_len) } diff --git a/srml/support/src/dispatch.rs b/srml/support/src/dispatch.rs index f8774fd5f835b..5f4a8861df495 100644 --- a/srml/support/src/dispatch.rs +++ b/srml/support/src/dispatch.rs @@ -25,7 +25,7 @@ pub use srml_metadata::{ FunctionMetadata, DecodeDifferent, DecodeDifferentArray, FunctionArgumentMetadata, ModuleConstantMetadata, DefaultByte, DefaultByteGetter, }; -pub use sr_primitives::weights::{WeightedTransaction, DispatchInfo, TransactionInfo, WeighData, +pub use sr_primitives::weights::{SimpleDispatchInfo, GetDispatchInfo, DispatchInfo, WeighData, ClassifyDispatch, TransactionPriority }; @@ -586,7 +586,7 @@ macro_rules! decl_module { { $( $constants )* } [ $( $dispatchables )* ] $(#[doc = $doc_attr])* - #[weight = $crate::dispatch::WeightedTransaction::default()] + #[weight = $crate::dispatch::SimpleDispatchInfo::default()] $fn_vis fn $fn_name( $from $(, $(#[$codec_attr])* $param_name : $param )* ) $( -> $result )* { $( $impl )* } @@ -1110,10 +1110,10 @@ macro_rules! decl_module { } // Implement weight calculation function for Call - impl<$trait_instance: $trait_name $(, $instance: $instantiable)?> $crate::dispatch::DispatchInfo + impl<$trait_instance: $trait_name $(, $instance: $instantiable)?> $crate::dispatch::GetDispatchInfo for $call_type<$trait_instance $(, $instance)?> where $( $other_where_bounds )* { - fn dispatch_info(&self) -> $crate::dispatch::TransactionInfo { + fn get_dispatch_info(&self) -> $crate::dispatch::DispatchInfo { $( if let $call_type::$fn_name($( ref $param_name ),*) = self { let weight = >::weigh_data( @@ -1124,23 +1124,23 @@ macro_rules! decl_module { &$weight, ($( $param_name, )*) ); - return $crate::dispatch::TransactionInfo { weight, class }; + return $crate::dispatch::DispatchInfo { weight, class }; } if let $call_type::__PhantomItem(_, _) = self { unreachable!("__PhantomItem should never be used.") } )* // Defensive only: this function must have already returned at this point. // all dispatchable function will have a weight which has the `::default` - // implementation of `WeightedTransaction`. Nonetheless, we create one if it does + // implementation of `SimpleDispatchInfo`. Nonetheless, we create one if it does // not exist. let weight = >::weigh_data( - &$crate::dispatch::WeightedTransaction::default(), + &$crate::dispatch::SimpleDispatchInfo::default(), () ); let class = >::classify_dispatch( - &$crate::dispatch::WeightedTransaction::default(), + &$crate::dispatch::SimpleDispatchInfo::default(), () ); - $crate::dispatch::TransactionInfo { weight, class } + $crate::dispatch::DispatchInfo { weight, class } } } @@ -1286,10 +1286,10 @@ macro_rules! impl_outer_dispatch { $camelcase ( $crate::dispatch::CallableCallFor<$camelcase, $runtime> ) ,)* } - impl $crate::dispatch::DispatchInfo for $call_type { - fn dispatch_info(&self) -> $crate::dispatch::TransactionInfo { + impl $crate::dispatch::GetDispatchInfo for $call_type { + fn get_dispatch_info(&self) -> $crate::dispatch::DispatchInfo { match self { - $( $call_type::$camelcase(call) => call.dispatch_info(), )* + $( $call_type::$camelcase(call) => call.get_dispatch_info(), )* } } } @@ -1596,7 +1596,7 @@ macro_rules! __check_reserved_fn_name { mod tests { use super::*; use crate::runtime_primitives::traits::{OnInitialize, OnFinalize}; - use sr_primitives::weights::{TransactionInfo, DispatchClass}; + use sr_primitives::weights::{DispatchInfo, DispatchClass}; pub trait Trait: system::Trait + Sized where Self::AccountId: From { type Origin; @@ -1622,7 +1622,7 @@ mod tests { fn aux_0(_origin) -> Result { unreachable!() } fn aux_1(_origin, #[compact] _data: u32) -> Result { unreachable!() } fn aux_2(_origin, _data: i32, _data2: String) -> Result { unreachable!() } - #[weight = WeightedTransaction::Fixed(10)] + #[weight = SimpleDispatchInfo::FixedNormal(10)] fn aux_3(_origin) -> Result { unreachable!() } fn aux_4(_origin, _data: i32) -> Result { unreachable!() } fn aux_5(_origin, _data: i32, #[compact] _data2: u32) -> Result { unreachable!() } @@ -1631,7 +1631,7 @@ mod tests { fn on_finalize(n: T::BlockNumber) { if n.into() == 42 { panic!("on_finalize") } } fn offchain_worker() {} - #[weight = WeightedTransaction::Operational(5)] + #[weight = SimpleDispatchInfo::OperationalNormal(5)] fn operational(_origin) { unreachable!() } } } @@ -1774,18 +1774,18 @@ mod tests { fn weight_should_attach_to_call_enum() { // max weight. not dependent on input. assert_eq!( - Call::::operational().dispatch_info(), - TransactionInfo { weight: 5, class: DispatchClass::Operational }, + Call::::operational().get_dispatch_info(), + DispatchInfo { weight: 5, class: DispatchClass::OperationalNormal }, ); // default weight. assert_eq!( - Call::::aux_0().dispatch_info(), - TransactionInfo { weight: 1, class: DispatchClass::User }, + Call::::aux_0().get_dispatch_info(), + DispatchInfo { weight: 1, class: DispatchClass::User }, ); // custom basic assert_eq!( - Call::::aux_3().dispatch_info(), - TransactionInfo { weight: 10, class: DispatchClass::User }, + Call::::aux_3().get_dispatch_info(), + DispatchInfo { weight: 10, class: DispatchClass::User }, ); } } diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index 83c37e39ebbb8..91c3cacaee006 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -78,7 +78,7 @@ use rstd::prelude::*; use rstd::map; use rstd::marker::PhantomData; use primitives::{ - generic::{self, Era}, weights::{Weight, TransactionInfo, DispatchClass} , traits::{ + generic::{self, Era}, weights::{Weight, DispatchInfo, DispatchClass} , traits::{ self, CheckEqual, SimpleArithmetic, Zero, SignedExtension, SimpleBitOps, Hash, Member, MaybeDisplay, EnsureOrigin, CurrentHeight, BlockNumberToHash, MaybeSerializeDebugButNotDeserialize, MaybeSerializeDebug, StaticLookup, One, Bounded, @@ -774,14 +774,15 @@ impl CheckWeight { /// Checks if the current extrinsic can fit into the block with respect to block weight limits. /// /// Upon successes, it returns the new block weight as a `Result`. - fn check_weight(info: TransactionInfo) -> Result { + fn check_weight(info: DispatchInfo) -> Result { let current_weight = Module::::all_extrinsics_weight(); - let added_weight = info.weight; - let next_weight = current_weight.saturating_add(added_weight); + let maximum_weight = T::MaximumBlockWeight::get(); let limit = match info.class { - DispatchClass::Operational => T::MaximumBlockWeight::get(), - DispatchClass::User => T::MaximumBlockWeight::get() / 4 + DispatchClass::Operational => maximum_weight, + DispatchClass::User => maximum_weight / 4 }; + let added_weight = info.weight.min(limit); + let next_weight = current_weight.saturating_add(added_weight); if next_weight > limit { return Err(DispatchError::BadState) } @@ -791,7 +792,7 @@ impl CheckWeight { /// Checks if the current extrinsic can fit into the block with respect to block length limits. /// /// Upon successes, it returns the new block length as a `Result`. - fn check_block_length(_info: TransactionInfo, len: usize) -> Result { + fn check_block_length(_info: DispatchInfo, len: usize) -> Result { let current_len = Module::::all_extrinsics_len(); let added_len = len as u32; let next_len = current_len.saturating_add(added_len); @@ -802,7 +803,7 @@ impl CheckWeight { } /// get the priority of an extrinsic denoted by `info`. - fn get_priority(info: TransactionInfo) -> TransactionPriority { + fn get_priority(info: DispatchInfo) -> TransactionPriority { match info.class { DispatchClass::User => info.weight.into(), DispatchClass::Operational => Bounded::max_value() @@ -825,7 +826,7 @@ impl SignedExtension for CheckWeight { fn pre_dispatch( self, _who: &Self::AccountId, - info: TransactionInfo, + info: DispatchInfo, len: usize, ) -> Result<(), DispatchError> { let next_len = Self::check_block_length(info, len)?; @@ -838,7 +839,7 @@ impl SignedExtension for CheckWeight { fn validate( &self, _who: &Self::AccountId, - info: TransactionInfo, + info: DispatchInfo, len: usize, ) -> Result { // There is no point in writing to storage here since changes are discarded. This basically @@ -885,7 +886,7 @@ impl SignedExtension for CheckNonce { fn pre_dispatch( self, who: &Self::AccountId, - _info: TransactionInfo, + _info: DispatchInfo, _len: usize, ) -> Result<(), DispatchError> { let expected = >::get(who); @@ -901,7 +902,7 @@ impl SignedExtension for CheckNonce { fn validate( &self, who: &Self::AccountId, - info: TransactionInfo, + info: DispatchInfo, _len: usize, ) -> Result { // check index @@ -1176,7 +1177,7 @@ mod tests { fn signed_ext_check_nonce_works() { with_externalities(&mut new_test_ext(), || { >::insert(1, 1); - let info = TransactionInfo::default(); + let info = DispatchInfo::default(); let len = 0_usize; // stale assert!(CheckNonce::(0).validate(&1, info, len).is_err()); @@ -1193,34 +1194,60 @@ mod tests { #[test] fn signed_ext_check_weight_works_user_tx() { with_externalities(&mut new_test_ext(), || { - let small = TransactionInfo { weight: 100, ..Default::default() }; - let medium = TransactionInfo { + let small = DispatchInfo { weight: 100, ..Default::default() }; + let medium = DispatchInfo { weight: >::get() / 4 - 1, ..Default::default() }; - let big = TransactionInfo { + let big = DispatchInfo { weight: >::get() / 4 + 1, ..Default::default() }; let len = 0_usize; - let reset_check_weight = |i, f| { - AllExtrinsicsWeight::put(0); + let reset_check_weight = |i, f, s| { + AllExtrinsicsWeight::put(s); let r = CheckWeight::(PhantomData).pre_dispatch(&1, i, len); if f { assert!(r.is_err()) } else { assert!(r.is_ok()) } }; - reset_check_weight(small, false); - reset_check_weight(medium, false); - reset_check_weight(big, true); + reset_check_weight(small, false, 0); + reset_check_weight(medium, false, 0); + reset_check_weight(big, true, 1); + }) + } + + #[test] + fn signed_ext_check_weight_fee_works() { + with_externalities(&mut new_test_ext(), || { + let free = DispatchInfo { weight: 0, ..Default::default() }; + let len = 0_usize; + + assert_eq!(System::all_extrinsics_weight(), 0); + let r = CheckWeight::(PhantomData).pre_dispatch(&1, free, len); + assert!(r.is_ok()); + assert_eq!(System::all_extrinsics_weight(), 0); + }) + } + + #[test] + fn signed_ext_check_weight_max_works() { + with_externalities(&mut new_test_ext(), || { + let max = DispatchInfo { weight: Weight::max_value(), ..Default::default() }; + let len = 0_usize; + + assert_eq!(System::all_extrinsics_weight(), 0); + let r = CheckWeight::(PhantomData).pre_dispatch(&1, max, len); + assert!(r.is_ok()); + assert_eq!(System::all_extrinsics_weight(), >::get() / 4); }) } #[test] fn signed_ext_check_weight_works_operational_tx() { with_externalities(&mut new_test_ext(), || { - let normal = TransactionInfo { weight: 100, ..Default::default() }; - let op = TransactionInfo { weight: 100, class: DispatchClass::Operational }; + let normal = DispatchInfo { weight: 100, ..Default::default() }; + let op = DispatchInfo { weight: 100, class: DispatchClass::Operational }; let len = 0_usize; // given almost full block @@ -1235,8 +1262,8 @@ mod tests { #[test] fn signed_ext_check_weight_priority_works() { with_externalities(&mut new_test_ext(), || { - let normal = TransactionInfo { weight: 100, class: DispatchClass::User }; - let op = TransactionInfo { weight: 100, class: DispatchClass::Operational }; + let normal = DispatchInfo { weight: 100, class: DispatchClass::User }; + let op = DispatchInfo { weight: 100, class: DispatchClass::Operational }; let len = 0_usize; assert_eq!( @@ -1253,7 +1280,7 @@ mod tests { #[test] fn signed_ext_check_weight_block_size_works() { with_externalities(&mut new_test_ext(), || { - let tx = TransactionInfo::default(); + let tx = DispatchInfo::default(); let reset_check_weight = |s, f| { AllExtrinsicsLen::put(0); From 1b22cf7de51650ad0b00e2f5b39192e59173a1ec Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 18 Jul 2019 22:48:32 +0200 Subject: [PATCH 12/14] Fix build. --- srml/support/src/dispatch.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/srml/support/src/dispatch.rs b/srml/support/src/dispatch.rs index 5f4a8861df495..65016316bba6e 100644 --- a/srml/support/src/dispatch.rs +++ b/srml/support/src/dispatch.rs @@ -1775,7 +1775,7 @@ mod tests { // max weight. not dependent on input. assert_eq!( Call::::operational().get_dispatch_info(), - DispatchInfo { weight: 5, class: DispatchClass::OperationalNormal }, + DispatchInfo { weight: 5, class: DispatchClass::Operational }, ); // default weight. assert_eq!( From a387225ba22fd321b4197e6027a5926e0cdd47ee Mon Sep 17 00:00:00 2001 From: Kian Peymani Date: Fri, 19 Jul 2019 09:17:28 +0200 Subject: [PATCH 13/14] Update srml/balances/src/lib.rs --- srml/balances/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/srml/balances/src/lib.rs b/srml/balances/src/lib.rs index 11bcc9a5dab0b..fbef2f210c4dd 100644 --- a/srml/balances/src/lib.rs +++ b/srml/balances/src/lib.rs @@ -1164,7 +1164,7 @@ impl, I: Instance> TakeFees { /// /// The final fee is composed of: /// - _length-fee_: This is the amount paid merely to pay for size of the transaction. - /// - _weight-fee_: This amount is computer based on the weight of the transaction. Unlike + /// - _weight-fee_: This amount is computed based on the weight of the transaction. Unlike /// size-fee, this is not input dependent and reflects the _complexity_ of the execution /// and the time it consumes. /// - (optional) _tip_: if included in the transaction, it will be added on top. Only signed From eabb1a559d9a5962eca395d89775de792ad0784a Mon Sep 17 00:00:00 2001 From: kianenigma Date: Fri, 19 Jul 2019 12:54:40 +0200 Subject: [PATCH 14/14] Final refactor. --- core/sr-primitives/src/weights.rs | 70 +++++++++++++++++++-------- node-template/runtime/src/lib.rs | 4 +- node-template/runtime/src/template.rs | 4 +- node/runtime/src/lib.rs | 4 +- srml/assets/src/lib.rs | 4 +- srml/aura/src/mock.rs | 4 +- srml/authorship/src/lib.rs | 4 +- srml/balances/src/lib.rs | 46 ++++++------------ srml/balances/src/mock.rs | 4 +- srml/collective/src/lib.rs | 4 +- srml/contracts/src/tests.rs | 4 +- srml/council/src/lib.rs | 4 +- srml/democracy/src/lib.rs | 4 +- srml/elections/src/lib.rs | 4 +- srml/example/src/lib.rs | 4 +- srml/executive/src/lib.rs | 4 +- srml/finality-tracker/src/lib.rs | 4 +- srml/generic-asset/src/lib.rs | 2 +- srml/generic-asset/src/mock.rs | 4 +- srml/grandpa/src/mock.rs | 4 +- srml/indices/src/mock.rs | 4 +- srml/session/src/mock.rs | 4 +- srml/staking/src/mock.rs | 4 +- srml/support/src/dispatch.rs | 6 +-- srml/system/src/lib.rs | 43 ++++++++++------ srml/timestamp/src/lib.rs | 4 +- srml/treasury/src/lib.rs | 4 +- 27 files changed, 143 insertions(+), 112 deletions(-) diff --git a/core/sr-primitives/src/weights.rs b/core/sr-primitives/src/weights.rs index 6a1d857a63af9..872a091a84523 100644 --- a/core/sr-primitives/src/weights.rs +++ b/core/sr-primitives/src/weights.rs @@ -30,29 +30,32 @@ use crate::traits::Bounded; pub type Weight = u32; /// A broad range of dispatch types. This is only distinguishing normal, user-triggered transactions -/// and anything beyond which serves a higher purpose to the system (`OperationalNormal`). +/// (`Normal`) and anything beyond which serves a higher purpose to the system (`Operational`). #[cfg_attr(feature = "std", derive(Debug))] #[derive(PartialEq, Eq, Clone, Copy)] pub enum DispatchClass { /// A normal dispatch. - User, + Normal, /// An operational dispatch. Operational, } impl Default for DispatchClass { fn default() -> Self { - DispatchClass::User + DispatchClass::Normal } } impl From for DispatchClass { fn from(tx: SimpleDispatchInfo) -> Self { match tx { - SimpleDispatchInfo::OperationalNormal(_) => DispatchClass::Operational, - SimpleDispatchInfo::FixedNormal(_) => DispatchClass::User, - SimpleDispatchInfo::MaxNormal => DispatchClass::User, - SimpleDispatchInfo::FreeNormal => DispatchClass::User, + SimpleDispatchInfo::FixedOperational(_) => DispatchClass::Operational, + SimpleDispatchInfo::MaxOperational => DispatchClass::Operational, + SimpleDispatchInfo::FreeOperational => DispatchClass::Operational, + + SimpleDispatchInfo::FixedNormal(_) => DispatchClass::Normal, + SimpleDispatchInfo::MaxNormal => DispatchClass::Normal, + SimpleDispatchInfo::FreeNormal => DispatchClass::Normal, } } } @@ -67,6 +70,17 @@ pub struct DispatchInfo { pub class: DispatchClass, } +impl DispatchInfo { + /// Determine if this dispatch should pay the base length-related fee or not. + pub fn pay_length_fee(&self) -> bool { + match self.class { + DispatchClass::Normal => true, + // For now we assume all operational transactions don't pay the length fee. + DispatchClass::Operational => false, + } + } +} + /// A `Dispatchable` function (aka transaction) that can carry some static information along with it, using the /// `#[weight]` attribute. pub trait GetDispatchInfo { @@ -92,32 +106,48 @@ pub trait ClassifyDispatch { /// /// A user may pass in any other type that implements the correct traits. If not, the `Default` /// implementation of [`SimpleDispatchInfo`] is used. +/// +/// For each broad group (`Normal` and `Operation`): +/// - A `Fixed` variant means weight fee is charged normally and the weight is the number +/// specified in the inner value of the variant. +/// - A `Free` variant is equal to `::Fixed(0)`. Note that this does not guarantee inclusion. +/// - A `Max` variant is equal to `::Fixed(Weight::max_value())`. +/// +/// Based on the final weight value, based on the above variants: +/// - A _weight-fee_ is deducted. +/// - The block weight is consumed proportionally. +/// +/// As for the broad groups themselves: +/// - `Normal` variants will be assigned a priority proportional to their weight. They can only +/// consume a portion (1/4) of the maximum block resource limits. +/// - `Operational` variants will be assigned the maximum priority. They can potentially consume +/// the entire block resource limit. #[derive(Clone, Copy)] pub enum SimpleDispatchInfo { - /// A fixed-weight dispatch function. No dependency on state or input. + /// A normal dispatch with fixed weight. FixedNormal(Weight), - /// An operational dispatch function. Still incurs a weight-fee but typically has a higher - /// priority. - OperationalNormal(Weight), - /// A dispatch function with the maximum possible weight. This means: - /// - The weight-fee will be maximum possible, based on the system state. - /// - This transaction will also have a high priority (but not guaranteed to be included.) - /// - If included, no more _normal_ dispatches will be permitted in that block. Operational - /// dispatches might be included nonetheless. + /// A normal dispatch with the maximum weight. MaxNormal, - /// Free. The transaction does not increase the total weight. This means: - /// - No weight-fee is charged. - /// - Block weight is not increased (very likely to be included, but not guaranteed). + /// A normal dispatch with no weight. FreeNormal, + /// An operational dispatch with fixed weight. + FixedOperational(Weight), + /// An operational dispatch with the maximum weight. + MaxOperational, + /// An operational dispatch with no weight. + FreeOperational, } impl WeighData for SimpleDispatchInfo { fn weigh_data(&self, _: T) -> Weight { match self { SimpleDispatchInfo::FixedNormal(w) => *w, - SimpleDispatchInfo::OperationalNormal(w) => *w, SimpleDispatchInfo::MaxNormal => Bounded::max_value(), SimpleDispatchInfo::FreeNormal => Bounded::min_value(), + + SimpleDispatchInfo::FixedOperational(w) => *w, + SimpleDispatchInfo::MaxOperational => Bounded::max_value(), + SimpleDispatchInfo::FreeOperational => Bounded::min_value(), } } } diff --git a/node-template/runtime/src/lib.rs b/node-template/runtime/src/lib.rs index aa309cf562476..b86af75ea8c8b 100644 --- a/node-template/runtime/src/lib.rs +++ b/node-template/runtime/src/lib.rs @@ -113,7 +113,7 @@ pub fn native_version() -> NativeVersion { parameter_types! { pub const BlockHashCount: BlockNumber = 250; pub const MaximumBlockWeight: Weight = 4 * 1024 * 1024; - pub const MaximumBlockSize: u32 = 2 * 1024; + pub const MaximumBlockLength: u32 = 2 * 1024; } impl system::Trait for Runtime { @@ -140,7 +140,7 @@ impl system::Trait for Runtime { /// Maximum weight of each block. With a default weight system of 1byte == 1weight, 4mb is ok. type MaximumBlockWeight = MaximumBlockWeight; /// Maximum size of all encoded transactions (in bytes) that are allowed in one block. - type MaximumBlockSize = MaximumBlockSize; + type MaximumBlockLength = MaximumBlockLength; } impl aura::Trait for Runtime { diff --git a/node-template/runtime/src/template.rs b/node-template/runtime/src/template.rs index 4801403252022..961ffaea4ffa6 100644 --- a/node-template/runtime/src/template.rs +++ b/node-template/runtime/src/template.rs @@ -87,7 +87,7 @@ mod tests { parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: Weight = 1024; - pub const MaximumBlockSize: u32 = 2 * 1024; + pub const MaximumBlockLength: u32 = 2 * 1024; } impl system::Trait for Test { type Origin = Origin; @@ -101,7 +101,7 @@ mod tests { type Event = (); type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; - type MaximumBlockSize = MaximumBlockSize; + type MaximumBlockLength = MaximumBlockLength; } impl Trait for Test { type Event = (); diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index 58725013987db..2b4f9322c7773 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -113,7 +113,7 @@ pub const DAYS: Moment = HOURS * 24; parameter_types! { pub const BlockHashCount: BlockNumber = 250; pub const MaximumBlockWeight: Weight = 4 * 1024; - pub const MaximumBlockSize: u32 = 4 * 1024 * 1024; + pub const MaximumBlockLength: u32 = 4 * 1024 * 1024; } impl system::Trait for Runtime { @@ -128,7 +128,7 @@ impl system::Trait for Runtime { type Event = Event; type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; - type MaximumBlockSize = MaximumBlockSize; + type MaximumBlockLength = MaximumBlockLength; } impl aura::Trait for Runtime { diff --git a/srml/assets/src/lib.rs b/srml/assets/src/lib.rs index 437945db97c00..962f4cfb4f0e0 100644 --- a/srml/assets/src/lib.rs +++ b/srml/assets/src/lib.rs @@ -258,7 +258,7 @@ mod tests { parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; - pub const MaximumBlockSize: u32 = 2 * 1024; + pub const MaximumBlockLength: u32 = 2 * 1024; } impl system::Trait for Test { type Origin = Origin; @@ -272,7 +272,7 @@ mod tests { type Event = (); type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; - type MaximumBlockSize = MaximumBlockSize; + type MaximumBlockLength = MaximumBlockLength; } impl Trait for Test { type Event = (); diff --git a/srml/aura/src/mock.rs b/srml/aura/src/mock.rs index 5a6e491a0389a..ffa0385761c68 100644 --- a/srml/aura/src/mock.rs +++ b/srml/aura/src/mock.rs @@ -38,7 +38,7 @@ pub struct Test; parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; - pub const MaximumBlockSize: u32 = 2 * 1024; + pub const MaximumBlockLength: u32 = 2 * 1024; pub const MinimumPeriod: u64 = 1; } @@ -54,7 +54,7 @@ impl system::Trait for Test { type Event = (); type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; - type MaximumBlockSize = MaximumBlockSize; + type MaximumBlockLength = MaximumBlockLength; } impl timestamp::Trait for Test { diff --git a/srml/authorship/src/lib.rs b/srml/authorship/src/lib.rs index ac46a428d3e78..8561a8428303b 100644 --- a/srml/authorship/src/lib.rs +++ b/srml/authorship/src/lib.rs @@ -338,7 +338,7 @@ mod tests { parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; - pub const MaximumBlockSize: u32 = 2 * 1024; + pub const MaximumBlockLength: u32 = 2 * 1024; } impl system::Trait for Test { @@ -353,7 +353,7 @@ mod tests { type Event = (); type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; - type MaximumBlockSize = MaximumBlockSize; + type MaximumBlockLength = MaximumBlockLength; } impl Trait for Test { diff --git a/srml/balances/src/lib.rs b/srml/balances/src/lib.rs index 11bcc9a5dab0b..67c8922d04a53 100644 --- a/srml/balances/src/lib.rs +++ b/srml/balances/src/lib.rs @@ -164,7 +164,7 @@ use primitives::traits::{ Saturating, Bounded, SignedExtension, SaturatedConversion, DispatchError }; use primitives::transaction_validity::{TransactionPriority, ValidTransaction}; -use primitives::weights::{DispatchInfo, Weight}; +use primitives::weights::DispatchInfo; use system::{IsDeadAccount, OnNewAccount, ensure_signed, ensure_root}; mod mock; @@ -764,7 +764,7 @@ impl, I: Instance> system::Trait for ElevatedTrait { type Event = (); type BlockHashCount = T::BlockHashCount; type MaximumBlockWeight = T::MaximumBlockWeight; - type MaximumBlockSize = T::MaximumBlockSize; + type MaximumBlockLength = T::MaximumBlockLength; } impl, I: Instance> Trait for ElevatedTrait { type Balance = T::Balance; @@ -1169,16 +1169,21 @@ impl, I: Instance> TakeFees { /// and the time it consumes. /// - (optional) _tip_: if included in the transaction, it will be added on top. Only signed /// transactions can have a tip. - fn compute_fee(len: usize, weight: Weight, tip: T::Balance) -> T::Balance { + fn compute_fee(len: usize, info: DispatchInfo, tip: T::Balance) -> T::Balance { // length fee - let len = T::Balance::from(len as u32); - let len_fee = T::TransactionBaseFee::get() - .saturating_add(T::TransactionByteFee::get().saturating_mul(len)); + let len_fee = if info.pay_length_fee() { + let len = T::Balance::from(len as u32); + let base = T::TransactionBaseFee::get(); + let byte = T::TransactionByteFee::get(); + base.saturating_add(byte.saturating_mul(len)) + } else { + Zero::zero() + }; // weight fee - let _weight_fee = T::Balance::from(weight); // TODO: should be weight_and_size_to_fee(weight, _len) #2854 + let _weight_fee = T::Balance::from(0); // TODO: should be weight_and_size_to_fee(weight, _len) #2854 - len_fee.saturating_add(tip)/*.saturating_add(_weight_fee)*/ + len_fee.saturating_add(_weight_fee).saturating_add(tip) } } @@ -1194,36 +1199,17 @@ impl, I: Instance + Clone + Eq> SignedExtension for TakeFees { type AdditionalSigned = (); fn additional_signed(&self) -> rstd::result::Result<(), &'static str> { Ok(()) } - fn pre_dispatch( - self, - who: &Self::AccountId, - info: DispatchInfo, - len: usize, - ) -> rstd::result::Result<(), DispatchError> { - // pay any fees. - let fee = Self::compute_fee(len, info.weight, self.0); - let imbalance = >::withdraw( - who, - fee, - WithdrawReason::TransactionPayment, - ExistenceRequirement::KeepAlive, - ).map_err(|_| DispatchError::Payment)?; - T::TransactionPayment::on_unbalanced(imbalance); - Ok(()) - } - fn validate( &self, who: &Self::AccountId, info: DispatchInfo, len: usize, ) -> rstd::result::Result { - // check that they can pay all their fees. - let fee = Self::compute_fee(len, info.weight, self.0); - // TODO: just use `>::ensure_can_withdraw()`? and no need to handle imbalance imo. + // pay any fees. + let fee = Self::compute_fee(len, info, self.0); let imbalance = >::withdraw( who, - fee.clone(), + fee, WithdrawReason::TransactionPayment, ExistenceRequirement::KeepAlive, ).map_err(|_| DispatchError::Payment)?; diff --git a/srml/balances/src/mock.rs b/srml/balances/src/mock.rs index 0f9e4a3a0c2cf..12eae9724172e 100644 --- a/srml/balances/src/mock.rs +++ b/srml/balances/src/mock.rs @@ -69,7 +69,7 @@ pub struct Runtime; parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; - pub const MaximumBlockSize: u32 = 2 * 1024; + pub const MaximumBlockLength: u32 = 2 * 1024; } impl system::Trait for Runtime { type Origin = Origin; @@ -83,7 +83,7 @@ impl system::Trait for Runtime { type Event = (); type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; - type MaximumBlockSize = MaximumBlockSize; + type MaximumBlockLength = MaximumBlockLength; } impl Trait for Runtime { type Balance = u64; diff --git a/srml/collective/src/lib.rs b/srml/collective/src/lib.rs index 6cd82eeca97e7..6e39d58e8d8b2 100644 --- a/srml/collective/src/lib.rs +++ b/srml/collective/src/lib.rs @@ -403,7 +403,7 @@ mod tests { parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; - pub const MaximumBlockSize: u32 = 2 * 1024; + pub const MaximumBlockLength: u32 = 2 * 1024; } impl system::Trait for Test { type Origin = Origin; @@ -417,7 +417,7 @@ mod tests { type Event = Event; type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; - type MaximumBlockSize = MaximumBlockSize; + type MaximumBlockLength = MaximumBlockLength; } impl Trait for Test { type Origin = Origin; diff --git a/srml/contracts/src/tests.rs b/srml/contracts/src/tests.rs index 7459ac06e08fd..71c6a7a31d7ed 100644 --- a/srml/contracts/src/tests.rs +++ b/srml/contracts/src/tests.rs @@ -97,7 +97,7 @@ pub struct Test; parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; - pub const MaximumBlockSize: u32 = 2 * 1024; + pub const MaximumBlockLength: u32 = 2 * 1024; } impl system::Trait for Test { type Origin = Origin; @@ -111,7 +111,7 @@ impl system::Trait for Test { type Event = MetaEvent; type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; - type MaximumBlockSize = MaximumBlockSize; + type MaximumBlockLength = MaximumBlockLength; } parameter_types! { pub const BalancesTransactionBaseFee: u64 = 0; diff --git a/srml/council/src/lib.rs b/srml/council/src/lib.rs index fec1ff7c43fe5..72feead5e1e33 100644 --- a/srml/council/src/lib.rs +++ b/srml/council/src/lib.rs @@ -99,7 +99,7 @@ mod tests { parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; - pub const MaximumBlockSize: u32 = 2 * 1024; + pub const MaximumBlockLength: u32 = 2 * 1024; } impl system::Trait for Test { type Origin = Origin; @@ -113,7 +113,7 @@ mod tests { type Event = Event; type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; - type MaximumBlockSize = MaximumBlockSize; + type MaximumBlockLength = MaximumBlockLength; } parameter_types! { pub const ExistentialDeposit: u64 = 0; diff --git a/srml/democracy/src/lib.rs b/srml/democracy/src/lib.rs index 188ff9fa81fd2..ab5d068928258 100644 --- a/srml/democracy/src/lib.rs +++ b/srml/democracy/src/lib.rs @@ -999,7 +999,7 @@ mod tests { parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; - pub const MaximumBlockSize: u32 = 2 * 1024; + pub const MaximumBlockLength: u32 = 2 * 1024; } impl system::Trait for Test { type Origin = Origin; @@ -1013,7 +1013,7 @@ mod tests { type Event = (); type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; - type MaximumBlockSize = MaximumBlockSize; + type MaximumBlockLength = MaximumBlockLength; } parameter_types! { pub const ExistentialDeposit: u64 = 0; diff --git a/srml/elections/src/lib.rs b/srml/elections/src/lib.rs index 63a388f7f4ca3..1060c7b9a7bf0 100644 --- a/srml/elections/src/lib.rs +++ b/srml/elections/src/lib.rs @@ -1109,7 +1109,7 @@ mod tests { parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; - pub const MaximumBlockSize: u32 = 2 * 1024; + pub const MaximumBlockLength: u32 = 2 * 1024; } impl system::Trait for Test { type Origin = Origin; @@ -1123,7 +1123,7 @@ mod tests { type Event = Event; type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; - type MaximumBlockSize = MaximumBlockSize; + type MaximumBlockLength = MaximumBlockLength; } parameter_types! { pub const ExistentialDeposit: u64 = 0; diff --git a/srml/example/src/lib.rs b/srml/example/src/lib.rs index cb9f22a7094ee..cd93d3a6b0735 100644 --- a/srml/example/src/lib.rs +++ b/srml/example/src/lib.rs @@ -525,7 +525,7 @@ mod tests { parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; - pub const MaximumBlockSize: u32 = 2 * 1024; + pub const MaximumBlockLength: u32 = 2 * 1024; } impl system::Trait for Test { type Origin = Origin; @@ -539,7 +539,7 @@ mod tests { type Event = (); type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; - type MaximumBlockSize = MaximumBlockSize; + type MaximumBlockLength = MaximumBlockLength; } parameter_types! { pub const ExistentialDeposit: u64 = 0; diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index a86786d684c39..0b2d9142a7d11 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -380,7 +380,7 @@ mod tests { parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; - pub const MaximumBlockSize: u32 = 2 * 1024; + pub const MaximumBlockLength: u32 = 2 * 1024; } impl system::Trait for Runtime { type Origin = Origin; @@ -394,7 +394,7 @@ mod tests { type Event = MetaEvent; type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; - type MaximumBlockSize = MaximumBlockSize; + type MaximumBlockLength = MaximumBlockLength; } parameter_types! { pub const ExistentialDeposit: u64 = 0; diff --git a/srml/finality-tracker/src/lib.rs b/srml/finality-tracker/src/lib.rs index 3ab936233c1d7..b6d59be474972 100644 --- a/srml/finality-tracker/src/lib.rs +++ b/srml/finality-tracker/src/lib.rs @@ -300,7 +300,7 @@ mod tests { parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; - pub const MaximumBlockSize: u32 = 2 * 1024; + pub const MaximumBlockLength: u32 = 2 * 1024; } impl system::Trait for Test { type Origin = Origin; @@ -314,7 +314,7 @@ mod tests { type Event = (); type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; - type MaximumBlockSize = MaximumBlockSize; + type MaximumBlockLength = MaximumBlockLength; } parameter_types! { pub const WindowSize: u64 = 11; diff --git a/srml/generic-asset/src/lib.rs b/srml/generic-asset/src/lib.rs index b40bd884aa3bc..d4a341c6ffd90 100644 --- a/srml/generic-asset/src/lib.rs +++ b/srml/generic-asset/src/lib.rs @@ -1057,7 +1057,7 @@ impl system::Trait for ElevatedTrait { type Header = T::Header; type Event = (); type MaximumBlockWeight = T::MaximumBlockWeight; - type MaximumBlockSize = T::MaximumBlockSize; + type MaximumBlockLength = T::MaximumBlockLength; type BlockHashCount = T::BlockHashCount; } impl Trait for ElevatedTrait { diff --git a/srml/generic-asset/src/mock.rs b/srml/generic-asset/src/mock.rs index 603944a6087cf..04c5b2d2ce157 100644 --- a/srml/generic-asset/src/mock.rs +++ b/srml/generic-asset/src/mock.rs @@ -41,7 +41,7 @@ pub struct Test; parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; - pub const MaximumBlockSize: u32 = 2 * 1024; + pub const MaximumBlockLength: u32 = 2 * 1024; } impl system::Trait for Test { type Origin = Origin; @@ -54,7 +54,7 @@ impl system::Trait for Test { type Header = Header; type Event = TestEvent; type MaximumBlockWeight = MaximumBlockWeight; - type MaximumBlockSize = MaximumBlockSize; + type MaximumBlockLength = MaximumBlockLength; type BlockHashCount = BlockHashCount; } diff --git a/srml/grandpa/src/mock.rs b/srml/grandpa/src/mock.rs index 301190a24d81b..71992655d1450 100644 --- a/srml/grandpa/src/mock.rs +++ b/srml/grandpa/src/mock.rs @@ -44,7 +44,7 @@ impl Trait for Test { parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; - pub const MaximumBlockSize: u32 = 2 * 1024; + pub const MaximumBlockLength: u32 = 2 * 1024; } impl system::Trait for Test { type Origin = Origin; @@ -58,7 +58,7 @@ impl system::Trait for Test { type Event = TestEvent; type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; - type MaximumBlockSize = MaximumBlockSize; + type MaximumBlockLength = MaximumBlockLength; } mod grandpa { diff --git a/srml/indices/src/mock.rs b/srml/indices/src/mock.rs index fe81a889882b1..ae6b31bb8cc86 100644 --- a/srml/indices/src/mock.rs +++ b/srml/indices/src/mock.rs @@ -67,7 +67,7 @@ pub struct Runtime; parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; - pub const MaximumBlockSize: u32 = 2 * 1024; + pub const MaximumBlockLength: u32 = 2 * 1024; } impl system::Trait for Runtime { type Origin = Origin; @@ -81,7 +81,7 @@ impl system::Trait for Runtime { type Event = (); type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; - type MaximumBlockSize = MaximumBlockSize; + type MaximumBlockLength = MaximumBlockLength; } impl Trait for Runtime { type AccountIndex = u64; diff --git a/srml/session/src/mock.rs b/srml/session/src/mock.rs index e46429a866e33..137642ca4cc69 100644 --- a/srml/session/src/mock.rs +++ b/srml/session/src/mock.rs @@ -110,7 +110,7 @@ pub struct Test; parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; - pub const MaximumBlockSize: u32 = 2 * 1024; + pub const MaximumBlockLength: u32 = 2 * 1024; pub const MinimumPeriod: u64 = 5; } impl system::Trait for Test { @@ -125,7 +125,7 @@ impl system::Trait for Test { type Event = (); type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; - type MaximumBlockSize = MaximumBlockSize; + type MaximumBlockLength = MaximumBlockLength; } impl timestamp::Trait for Test { type Moment = u64; diff --git a/srml/staking/src/mock.rs b/srml/staking/src/mock.rs index f6b0579cd56eb..2bf95bbf8b44b 100644 --- a/srml/staking/src/mock.rs +++ b/srml/staking/src/mock.rs @@ -88,7 +88,7 @@ pub struct Test; parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; - pub const MaximumBlockSize: u32 = 2 * 1024; + pub const MaximumBlockLength: u32 = 2 * 1024; } impl system::Trait for Test { type Origin = Origin; @@ -102,7 +102,7 @@ impl system::Trait for Test { type Event = (); type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; - type MaximumBlockSize = MaximumBlockSize; + type MaximumBlockLength = MaximumBlockLength; } parameter_types! { pub const TransferFee: u64 = 0; diff --git a/srml/support/src/dispatch.rs b/srml/support/src/dispatch.rs index 65016316bba6e..476c82a1136c7 100644 --- a/srml/support/src/dispatch.rs +++ b/srml/support/src/dispatch.rs @@ -1631,7 +1631,7 @@ mod tests { fn on_finalize(n: T::BlockNumber) { if n.into() == 42 { panic!("on_finalize") } } fn offchain_worker() {} - #[weight = SimpleDispatchInfo::OperationalNormal(5)] + #[weight = SimpleDispatchInfo::FixedOperational(5)] fn operational(_origin) { unreachable!() } } } @@ -1780,12 +1780,12 @@ mod tests { // default weight. assert_eq!( Call::::aux_0().get_dispatch_info(), - DispatchInfo { weight: 1, class: DispatchClass::User }, + DispatchInfo { weight: 1, class: DispatchClass::Normal }, ); // custom basic assert_eq!( Call::::aux_3().get_dispatch_info(), - DispatchInfo { weight: 10, class: DispatchClass::User }, + DispatchInfo { weight: 10, class: DispatchClass::Normal }, ); } } diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index 91c3cacaee006..3ed20f247f82f 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -195,8 +195,8 @@ pub trait Trait: 'static + Eq + Clone { /// The maximum weight of a block. type MaximumBlockWeight: Get; - /// The maximum size of a block (in bytes). - type MaximumBlockSize: Get; + /// The maximum length of a block (in bytes). + type MaximumBlockLength: Get; } pub type DigestOf = generic::Digest<::Hash>; @@ -771,16 +771,23 @@ impl Module { pub struct CheckWeight(PhantomData); impl CheckWeight { + + /// Get the quota divisor of each dispatch class type. This indicates that all operational + /// dispatches can use the full capacity of any resource, while user-triggered ones can consume + /// a quarter. + fn get_dispatch_limit_divisor(class: DispatchClass) -> Weight { + match class { + DispatchClass::Operational => 1, + DispatchClass::Normal => 4, + } + } /// Checks if the current extrinsic can fit into the block with respect to block weight limits. /// /// Upon successes, it returns the new block weight as a `Result`. fn check_weight(info: DispatchInfo) -> Result { let current_weight = Module::::all_extrinsics_weight(); let maximum_weight = T::MaximumBlockWeight::get(); - let limit = match info.class { - DispatchClass::Operational => maximum_weight, - DispatchClass::User => maximum_weight / 4 - }; + let limit = maximum_weight / Self::get_dispatch_limit_divisor(info.class); let added_weight = info.weight.min(limit); let next_weight = current_weight.saturating_add(added_weight); if next_weight > limit { @@ -792,11 +799,13 @@ impl CheckWeight { /// Checks if the current extrinsic can fit into the block with respect to block length limits. /// /// Upon successes, it returns the new block length as a `Result`. - fn check_block_length(_info: DispatchInfo, len: usize) -> Result { + fn check_block_length(info: DispatchInfo, len: usize) -> Result { let current_len = Module::::all_extrinsics_len(); + let maximum_len = T::MaximumBlockLength::get(); + let limit = maximum_len / Self::get_dispatch_limit_divisor(info.class); let added_len = len as u32; let next_len = current_len.saturating_add(added_len); - if next_len > T::MaximumBlockSize::get() { + if next_len > limit { return Err(DispatchError::BadState) } Ok(next_len) @@ -805,7 +814,7 @@ impl CheckWeight { /// get the priority of an extrinsic denoted by `info`. fn get_priority(info: DispatchInfo) -> TransactionPriority { match info.class { - DispatchClass::User => info.weight.into(), + DispatchClass::Normal => info.weight.into(), DispatchClass::Operational => Bounded::max_value() } } @@ -1006,7 +1015,7 @@ mod tests { parameter_types! { pub const BlockHashCount: u64 = 10; pub const MaximumBlockWeight: Weight = 1024; - pub const MaximumBlockSize: u32 = 2 * 1024; + pub const MaximumBlockLength: u32 = 2 * 1024; } impl Trait for Test { @@ -1021,7 +1030,7 @@ mod tests { type Event = u16; type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; - type MaximumBlockSize = MaximumBlockSize; + type MaximumBlockLength = MaximumBlockLength; } impl From for u16 { @@ -1256,13 +1265,19 @@ mod tests { assert!(CheckWeight::(PhantomData).pre_dispatch(&1, normal, len).is_err()); // will fit. assert!(CheckWeight::(PhantomData).pre_dispatch(&1, op, len).is_ok()); + + // likewise for length limit. + let len = 100_usize; + AllExtrinsicsLen::put(>::get() / 4); + assert!(CheckWeight::(PhantomData).pre_dispatch(&1, normal, len).is_err()); + assert!(CheckWeight::(PhantomData).pre_dispatch(&1, op, len).is_ok()); }) } #[test] fn signed_ext_check_weight_priority_works() { with_externalities(&mut new_test_ext(), || { - let normal = DispatchInfo { weight: 100, class: DispatchClass::User }; + let normal = DispatchInfo { weight: 100, class: DispatchClass::Normal }; let op = DispatchInfo { weight: 100, class: DispatchClass::Operational }; let len = 0_usize; @@ -1288,9 +1303,9 @@ mod tests { if f { assert!(r.is_err()) } else { assert!(r.is_ok()) } }; + reset_check_weight(128, false); reset_check_weight(512, false); - reset_check_weight(2 * 1024, false); - reset_check_weight(3 * 1024, true); + reset_check_weight(513, true); }) } diff --git a/srml/timestamp/src/lib.rs b/srml/timestamp/src/lib.rs index 8fd1fda69d866..9fdc05ee2a248 100644 --- a/srml/timestamp/src/lib.rs +++ b/srml/timestamp/src/lib.rs @@ -339,7 +339,7 @@ mod tests { parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; - pub const MaximumBlockSize: u32 = 2 * 1024; + pub const MaximumBlockLength: u32 = 2 * 1024; } impl system::Trait for Test { type Origin = Origin; @@ -353,7 +353,7 @@ mod tests { type Event = (); type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; - type MaximumBlockSize = MaximumBlockSize; + type MaximumBlockLength = MaximumBlockLength; } parameter_types! { pub const MinimumPeriod: u64 = 5; diff --git a/srml/treasury/src/lib.rs b/srml/treasury/src/lib.rs index 00a91013ff575..85edc89a30c83 100644 --- a/srml/treasury/src/lib.rs +++ b/srml/treasury/src/lib.rs @@ -371,7 +371,7 @@ mod tests { parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; - pub const MaximumBlockSize: u32 = 2 * 1024; + pub const MaximumBlockLength: u32 = 2 * 1024; } impl system::Trait for Test { type Origin = Origin; @@ -385,7 +385,7 @@ mod tests { type Event = (); type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; - type MaximumBlockSize = MaximumBlockSize; + type MaximumBlockLength = MaximumBlockLength; } parameter_types! { pub const ExistentialDeposit: u64 = 0;