diff --git a/bin/node/runtime/src/impls.rs b/bin/node/runtime/src/impls.rs index f613dc5af5048..878d4b6f0c266 100644 --- a/bin/node/runtime/src/impls.rs +++ b/bin/node/runtime/src/impls.rs @@ -70,8 +70,8 @@ pub struct TargetedFeeAdjustment(sp_std::marker::PhantomData); impl> Convert for TargetedFeeAdjustment { fn convert(multiplier: Fixed128) -> Fixed128 { - let block_weight = System::all_extrinsics_weight(); let max_weight = MaximumBlockWeight::get(); + let block_weight = System::all_extrinsics_weight().total().min(max_weight); let target_weight = (T::get() * max_weight) as u128; let block_weight = block_weight as u128; @@ -132,11 +132,12 @@ mod tests { // poc reference implementation. fn fee_multiplier_update(block_weight: Weight, previous: Fixed128) -> Fixed128 { - let block_weight = block_weight as f64; - let v: f64 = 0.00004; - // maximum tx weight let m = max() as f64; + // block weight always truncated to max weight + let block_weight = (block_weight as f64).min(m); + let v: f64 = 0.00004; + // Ideal saturation in terms of weight let ss = target() as f64; // Current saturation in terms of weight diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index 742397b624763..9c46d86d74429 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -116,7 +116,7 @@ use sp_std::{prelude::*, marker::PhantomData}; use frame_support::{ - storage::StorageValue, weights::{GetDispatchInfo, DispatchInfo}, + storage::StorageValue, weights::{GetDispatchInfo, DispatchInfo, DispatchClass}, traits::{OnInitialize, OnFinalize, OnRuntimeUpgrade, OffchainWorker}, }; use sp_runtime::{ @@ -235,7 +235,7 @@ where let mut weight = as OnRuntimeUpgrade>::on_runtime_upgrade(); weight = weight.saturating_add(COnRuntimeUpgrade::on_runtime_upgrade()); weight = weight.saturating_add(::on_runtime_upgrade()); - >::register_extra_weight_unchecked(weight); + >::register_extra_weight_unchecked(weight, DispatchClass::Mandatory); } >::initialize( block_number, @@ -247,7 +247,7 @@ where as OnInitialize>::on_initialize(*block_number); let weight = >::on_initialize(*block_number) .saturating_add(>::get()); - >::register_extra_weight_unchecked(weight); + >::register_extra_weight_unchecked(weight, DispatchClass::Mandatory); frame_system::Module::::note_finished_initialize(); } @@ -785,7 +785,7 @@ mod tests { Digest::default(), )); // Base block execution weight + `on_initialize` weight from the custom module. - assert_eq!(>::all_extrinsics_weight(), base_block_weight); + assert_eq!(>::all_extrinsics_weight().total(), base_block_weight); for nonce in 0..=num_to_exhaust_block { let xt = TestXt::new( @@ -795,7 +795,7 @@ mod tests { if nonce != num_to_exhaust_block { assert!(res.is_ok()); assert_eq!( - >::all_extrinsics_weight(), + >::all_extrinsics_weight().total(), //--------------------- on_initialize + block_execution + extrinsic_base weight (encoded_len + 5) * (nonce + 1) + base_block_weight, ); @@ -815,7 +815,7 @@ mod tests { let len = xt.clone().encode().len() as u32; let mut t = new_test_ext(1); t.execute_with(|| { - assert_eq!(>::all_extrinsics_weight(), 0); + assert_eq!(>::all_extrinsics_weight().total(), 0); assert_eq!(>::all_extrinsics_len(), 0); assert!(Executive::apply_extrinsic(xt.clone()).unwrap().is_ok()); @@ -824,14 +824,14 @@ mod tests { // default weight for `TestXt` == encoded length. assert_eq!( - >::all_extrinsics_weight(), + >::all_extrinsics_weight().total(), 3 * (len as Weight + ::ExtrinsicBaseWeight::get()), ); assert_eq!(>::all_extrinsics_len(), 3 * len); let _ = >::finalize(); - assert_eq!(>::all_extrinsics_weight(), 0); + assert_eq!(>::all_extrinsics_weight().total(), 0); assert_eq!(>::all_extrinsics_len(), 0); }); } @@ -903,7 +903,7 @@ mod tests { // NOTE: might need updates over time if new weights are introduced. // For now it only accounts for the base block execution weight and // the `on_initialize` weight defined in the custom test module. - assert_eq!(>::all_extrinsics_weight(), 175 + 10); + assert_eq!(>::all_extrinsics_weight().total(), 175 + 10); }) } diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index 7219b0f5180fe..ff6893d6290e8 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -357,6 +357,60 @@ impl From for LastRuntimeUpgradeInfo { } } +/// An object to track the currently used extrinsic weight in a block. +#[derive(Clone, Eq, PartialEq, Default, RuntimeDebug, Encode, Decode)] +pub struct ExtrinsicsWeight { + normal: Weight, + operational: Weight, +} + +impl ExtrinsicsWeight { + /// Returns the total weight consumed by all extrinsics in the block. + pub fn total(&self) -> Weight { + self.normal.saturating_add(self.operational) + } + + /// Add some weight of a specific dispatch class, saturating at the numeric bounds of `Weight`. + pub fn add(&mut self, weight: Weight, class: DispatchClass) { + let value = self.get_mut(class); + *value = value.saturating_add(weight); + } + + /// Try to add some weight of a specific dispatch class, returning Err(()) if overflow would occur. + pub fn checked_add(&mut self, weight: Weight, class: DispatchClass) -> Result<(), ()> { + let value = self.get_mut(class); + *value = value.checked_add(weight).ok_or(())?; + Ok(()) + } + + /// Subtract some weight of a specific dispatch class, saturating at the numeric bounds of `Weight`. + pub fn sub(&mut self, weight: Weight, class: DispatchClass) { + let value = self.get_mut(class); + *value = value.saturating_sub(weight); + } + + /// Get the current weight of a specific dispatch class. + pub fn get(&self, class: DispatchClass) -> Weight { + match class { + DispatchClass::Operational => self.operational, + DispatchClass::Normal | DispatchClass::Mandatory => self.normal, + } + } + + /// Get a mutable reference to the current weight of a specific dispatch class. + fn get_mut(&mut self, class: DispatchClass) -> &mut Weight { + match class { + DispatchClass::Operational => &mut self.operational, + DispatchClass::Normal | DispatchClass::Mandatory => &mut self.normal, + } + } + + /// Set the weight of a specific dispatch class. + pub fn put(&mut self, new: Weight, class: DispatchClass) { + *self.get_mut(class) = new; + } +} + decl_storage! { trait Store for Module as System { /// The full account information for a particular account ID. @@ -366,8 +420,8 @@ decl_storage! { /// Total extrinsics count for the current block. ExtrinsicCount: Option; - /// Total weight for all extrinsics put together, for the current block. - AllExtrinsicsWeight: Option; + /// Total weight for all extrinsics for the current block. + AllExtrinsicsWeight: ExtrinsicsWeight; /// Total length (in bytes) for all extrinsics put together, for the current block. AllExtrinsicsLen: Option; @@ -915,9 +969,9 @@ impl Module { ExtrinsicCount::get().unwrap_or_default() } - /// Gets a total weight of all executed extrinsics. - pub fn all_extrinsics_weight() -> Weight { - AllExtrinsicsWeight::get().unwrap_or_default() + /// Gets the weight of all executed extrinsics. + pub fn all_extrinsics_weight() -> ExtrinsicsWeight { + AllExtrinsicsWeight::get() } pub fn all_extrinsics_len() -> u32 { @@ -939,12 +993,10 @@ impl Module { /// of block weight is more than the block weight limit. This is what the _unchecked_. /// /// Another potential use-case could be for the `on_initialize` and `on_finalize` hooks. - /// - /// If no previous weight exists, the function initializes the weight to zero. - pub fn register_extra_weight_unchecked(weight: Weight) { - let current_weight = AllExtrinsicsWeight::get().unwrap_or_default(); - let next_weight = current_weight.saturating_add(weight); - AllExtrinsicsWeight::put(next_weight); + pub fn register_extra_weight_unchecked(weight: Weight, class: DispatchClass) { + AllExtrinsicsWeight::mutate(|current_weight| { + current_weight.add(weight, class); + }); } /// Start the execution of a particular block. @@ -1065,7 +1117,9 @@ impl Module { /// Set the current block weight. This should only be used in some integration tests. #[cfg(any(feature = "std", test))] pub fn set_block_limits(weight: Weight, len: usize) { - AllExtrinsicsWeight::put(weight); + AllExtrinsicsWeight::mutate(|current_weight| { + current_weight.put(weight, DispatchClass::Normal) + }); AllExtrinsicsLen::put(len as u32); } @@ -1298,24 +1352,49 @@ impl CheckWeight where /// Upon successes, it returns the new block weight as a `Result`. fn check_weight( info: &DispatchInfoOf, - ) -> Result { - let current_weight = Module::::all_extrinsics_weight(); + ) -> Result { let maximum_weight = T::MaximumBlockWeight::get(); - let limit = Self::get_dispatch_limit_ratio(info.class) * maximum_weight; - if info.class == DispatchClass::Mandatory { + let mut all_weight = Module::::all_extrinsics_weight(); + match info.class { // If we have a dispatch that must be included in the block, it ignores all the limits. - let extrinsic_weight = info.weight.saturating_add(T::ExtrinsicBaseWeight::get()); - let next_weight = current_weight.saturating_add(extrinsic_weight); - Ok(next_weight) - } else { - let extrinsic_weight = info.weight.checked_add(T::ExtrinsicBaseWeight::get()) - .ok_or(InvalidTransaction::ExhaustsResources)?; - let next_weight = current_weight.checked_add(extrinsic_weight) - .ok_or(InvalidTransaction::ExhaustsResources)?; - if next_weight > limit { - Err(InvalidTransaction::ExhaustsResources.into()) - } else { - Ok(next_weight) + DispatchClass::Mandatory => { + let extrinsic_weight = info.weight.saturating_add(T::ExtrinsicBaseWeight::get()); + all_weight.add(extrinsic_weight, DispatchClass::Mandatory); + Ok(all_weight) + }, + // If we have a normal dispatch, we follow all the normal rules and limits. + DispatchClass::Normal => { + let normal_limit = Self::get_dispatch_limit_ratio(DispatchClass::Normal) * maximum_weight; + let extrinsic_weight = info.weight.checked_add(T::ExtrinsicBaseWeight::get()) + .ok_or(InvalidTransaction::ExhaustsResources)?; + all_weight.checked_add(extrinsic_weight, DispatchClass::Normal) + .map_err(|_| InvalidTransaction::ExhaustsResources)?; + if all_weight.get(DispatchClass::Normal) > normal_limit { + Err(InvalidTransaction::ExhaustsResources.into()) + } else { + Ok(all_weight) + } + }, + // If we have an operational dispatch, allow it if we have not used our full + // "operational space" (independent of existing fullness). + DispatchClass::Operational => { + let operational_limit = Self::get_dispatch_limit_ratio(DispatchClass::Operational) * maximum_weight; + let normal_limit = Self::get_dispatch_limit_ratio(DispatchClass::Normal) * maximum_weight; + let operational_space = operational_limit.saturating_sub(normal_limit); + + let extrinsic_weight = info.weight.checked_add(T::ExtrinsicBaseWeight::get()) + .ok_or(InvalidTransaction::ExhaustsResources)?; + all_weight.checked_add(extrinsic_weight, DispatchClass::Operational) + .map_err(|_| InvalidTransaction::ExhaustsResources)?; + + // If it would fit in normally, its okay + if all_weight.total() <= maximum_weight || + // If we have not used our operational space + all_weight.get(DispatchClass::Operational) <= operational_space { + Ok(all_weight) + } else { + Err(InvalidTransaction::ExhaustsResources.into()) + } } } } @@ -1452,8 +1531,8 @@ impl SignedExtension for CheckWeight where let unspent = post_info.calc_unspent(info); if unspent > 0 { - AllExtrinsicsWeight::mutate(|weight| { - *weight = weight.map(|w| w.saturating_sub(unspent)); + AllExtrinsicsWeight::mutate(|current_weight| { + current_weight.sub(unspent, info.class); }) } @@ -1713,7 +1792,7 @@ pub(crate) mod tests { use sp_std::cell::RefCell; use sp_core::H256; use sp_runtime::{traits::{BlakeTwo256, IdentityLookup, SignedExtension}, testing::Header, DispatchError}; - use frame_support::{impl_outer_origin, parameter_types, assert_ok}; + use frame_support::{impl_outer_origin, parameter_types, assert_ok, assert_noop}; impl_outer_origin! { pub enum Origin for Test where system = super {} @@ -1810,7 +1889,7 @@ pub(crate) mod tests { fn new_test_ext() -> sp_io::TestExternalities { let mut ext: sp_io::TestExternalities = GenesisConfig::default().build_storage::().unwrap().into(); // Add to each test the initial weight of a block - ext.execute_with(|| System::register_extra_weight_unchecked(::BlockExecutionWeight::get())); + ext.execute_with(|| System::register_extra_weight_unchecked(::BlockExecutionWeight::get(), DispatchClass::Mandatory)); ext } @@ -2036,7 +2115,9 @@ pub(crate) mod tests { let len = 0_usize; let reset_check_weight = |i, f, s| { - AllExtrinsicsWeight::put(s); + AllExtrinsicsWeight::mutate(|current_weight| { + current_weight.put(s, DispatchClass::Normal) + }); let r = CheckWeight::(PhantomData).pre_dispatch(&1, CALL, i, len); if f { assert!(r.is_err()) } else { assert!(r.is_ok()) } }; @@ -2056,17 +2137,19 @@ pub(crate) mod tests { let len = 0_usize; // We allow 75% for normal transaction, so we put 25% - extrinsic base weight - AllExtrinsicsWeight::put(256 - ::ExtrinsicBaseWeight::get()); + AllExtrinsicsWeight::mutate(|current_weight| { + current_weight.put(256 - ::ExtrinsicBaseWeight::get(), DispatchClass::Normal) + }); let pre = CheckWeight::(PhantomData).pre_dispatch(&1, CALL, &info, len).unwrap(); - assert_eq!(AllExtrinsicsWeight::get().unwrap(), info.weight + 256); + assert_eq!(AllExtrinsicsWeight::get().total(), info.weight + 256); assert!( CheckWeight::::post_dispatch(pre, &info, &post_info, len, &Ok(())) .is_ok() ); assert_eq!( - AllExtrinsicsWeight::get().unwrap(), + AllExtrinsicsWeight::get().total(), post_info.actual_weight.unwrap() + 256, ); }) @@ -2079,11 +2162,13 @@ pub(crate) mod tests { let post_info = PostDispatchInfo { actual_weight: Some(700), }; let len = 0_usize; - AllExtrinsicsWeight::put(128); + AllExtrinsicsWeight::mutate(|current_weight| { + current_weight.put(128, DispatchClass::Normal) + }); let pre = CheckWeight::(PhantomData).pre_dispatch(&1, CALL, &info, len).unwrap(); assert_eq!( - AllExtrinsicsWeight::get().unwrap(), + AllExtrinsicsWeight::get().total(), info.weight + 128 + ::ExtrinsicBaseWeight::get(), ); @@ -2092,7 +2177,7 @@ pub(crate) mod tests { .is_ok() ); assert_eq!( - AllExtrinsicsWeight::get().unwrap(), + AllExtrinsicsWeight::get().total(), info.weight + 128 + ::ExtrinsicBaseWeight::get(), ); }) @@ -2105,11 +2190,11 @@ pub(crate) mod tests { let len = 0_usize; // Initial weight from `BlockExecutionWeight` - assert_eq!(System::all_extrinsics_weight(), ::BlockExecutionWeight::get()); + assert_eq!(System::all_extrinsics_weight().total(), ::BlockExecutionWeight::get()); let r = CheckWeight::(PhantomData).pre_dispatch(&1, CALL, &free, len); assert!(r.is_ok()); assert_eq!( - System::all_extrinsics_weight(), + System::all_extrinsics_weight().total(), ::ExtrinsicBaseWeight::get() + ::BlockExecutionWeight::get() ); }) @@ -2126,17 +2211,17 @@ pub(crate) mod tests { let len = 0_usize; assert_ok!(CheckWeight::::do_pre_dispatch(&max, len)); - assert_eq!(System::all_extrinsics_weight(), Weight::max_value()); - assert!(System::all_extrinsics_weight() > ::MaximumBlockWeight::get()); + assert_eq!(System::all_extrinsics_weight().total(), Weight::max_value()); + assert!(System::all_extrinsics_weight().total() > ::MaximumBlockWeight::get()); }); } #[test] fn register_extra_weight_unchecked_doesnt_care_about_limits() { new_test_ext().execute_with(|| { - System::register_extra_weight_unchecked(Weight::max_value()); - assert_eq!(System::all_extrinsics_weight(), Weight::max_value()); - assert!(System::all_extrinsics_weight() > ::MaximumBlockWeight::get()); + System::register_extra_weight_unchecked(Weight::max_value(), DispatchClass::Normal); + assert_eq!(System::all_extrinsics_weight().total(), Weight::max_value()); + assert!(System::all_extrinsics_weight().total() > ::MaximumBlockWeight::get()); }); } @@ -2154,10 +2239,45 @@ pub(crate) mod tests { let len = 0_usize; assert_ok!(CheckWeight::::do_pre_dispatch(&max_normal, len)); - assert_eq!(System::all_extrinsics_weight(), 768); + assert_eq!(System::all_extrinsics_weight().total(), 768); assert_ok!(CheckWeight::::do_pre_dispatch(&rest_operational, len)); assert_eq!(::MaximumBlockWeight::get(), 1024); - assert_eq!(System::all_extrinsics_weight(), ::MaximumBlockWeight::get()); + assert_eq!(System::all_extrinsics_weight().total(), ::MaximumBlockWeight::get()); + }); + } + + #[test] + fn dispatch_order_does_not_effect_weight_logic() { + new_test_ext().execute_with(|| { + // We switch the order of `full_block_with_normal_and_operational` + let max_normal = DispatchInfo { weight: 753, ..Default::default() }; + let rest_operational = DispatchInfo { weight: 251, class: DispatchClass::Operational, ..Default::default() }; + + let len = 0_usize; + + assert_ok!(CheckWeight::::do_pre_dispatch(&rest_operational, len)); + // Extra 15 here from block execution + base extrinsic weight + assert_eq!(System::all_extrinsics_weight().total(), 266); + assert_ok!(CheckWeight::::do_pre_dispatch(&max_normal, len)); + assert_eq!(::MaximumBlockWeight::get(), 1024); + assert_eq!(System::all_extrinsics_weight().total(), ::MaximumBlockWeight::get()); + }); + } + + #[test] + fn operational_works_on_full_block() { + new_test_ext().execute_with(|| { + // An on_initialize takes up the whole block! (Every time!) + System::register_extra_weight_unchecked(Weight::max_value(), DispatchClass::Mandatory); + let dispatch_normal = DispatchInfo { weight: 251, class: DispatchClass::Normal, ..Default::default() }; + let dispatch_operational = DispatchInfo { weight: 251, class: DispatchClass::Operational, ..Default::default() }; + let len = 0_usize; + + assert_noop!(CheckWeight::::do_pre_dispatch(&dispatch_normal, len), InvalidTransaction::ExhaustsResources); + // Thank goodness we can still do an operational transaction to possibly save the blockchain. + assert_ok!(CheckWeight::::do_pre_dispatch(&dispatch_operational, len)); + // Not too much though + assert_noop!(CheckWeight::::do_pre_dispatch(&dispatch_operational, len), InvalidTransaction::ExhaustsResources); }); } @@ -2170,7 +2290,9 @@ pub(crate) mod tests { let normal_limit = normal_weight_limit(); // given almost full block - AllExtrinsicsWeight::put(normal_limit); + AllExtrinsicsWeight::mutate(|current_weight| { + current_weight.put(normal_limit, DispatchClass::Normal) + }); // will not fit. assert!(CheckWeight::(PhantomData).pre_dispatch(&1, CALL, &normal, len).is_err()); // will fit.