From e4b89cc50c8d17868d6c8b122f2e156d678c7525 Mon Sep 17 00:00:00 2001 From: Sebastian Kunert <skunert49@gmail.com> Date: Thu, 23 May 2024 21:21:32 +0200 Subject: [PATCH] Disable total pov size check for mandatory extrinsics --- .../system/src/extensions/check_weight.rs | 126 +++++++++++++++--- 1 file changed, 108 insertions(+), 18 deletions(-) diff --git a/substrate/frame/system/src/extensions/check_weight.rs b/substrate/frame/system/src/extensions/check_weight.rs index 061d543f8c3..a6da11648e1 100644 --- a/substrate/frame/system/src/extensions/check_weight.rs +++ b/substrate/frame/system/src/extensions/check_weight.rs @@ -15,7 +15,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::{limits::BlockWeights, Config, Pallet, LOG_TARGET}; +use crate::{limits::BlockWeights, Config, DispatchClass, Pallet, LOG_TARGET}; use codec::{Decode, Encode}; use frame_support::{ dispatch::{DispatchInfo, PostDispatchInfo}, @@ -107,7 +107,7 @@ where let maximum_weight = T::BlockWeights::get(); let next_weight = calculate_consumed_weight::<T::RuntimeCall>(&maximum_weight, all_weight, info)?; - check_combined_proof_size(&maximum_weight, next_len, &next_weight)?; + check_combined_proof_size::<T::RuntimeCall>(info, &maximum_weight, next_len, &next_weight)?; Self::check_extrinsic_weight(info)?; crate::AllExtrinsicsLen::<T>::put(next_len); @@ -131,11 +131,20 @@ where } /// Check that the combined extrinsic length and proof size together do not exceed the PoV limit. -pub fn check_combined_proof_size( +pub fn check_combined_proof_size<Call>( + info: &DispatchInfoOf<Call>, maximum_weight: &BlockWeights, next_len: u32, next_weight: &crate::ConsumedWeight, -) -> Result<(), TransactionValidityError> { +) -> Result<(), TransactionValidityError> +where + Call: Dispatchable<Info = DispatchInfo, PostInfo = PostDispatchInfo>, +{ + if matches!(info.class, DispatchClass::Mandatory) { + // Allow mandatory extrinsics + return Ok(()); + } + // This extra check ensures that the extrinsic length does not push the // PoV over the limit. let total_pov_size = next_weight.total().proof_size().saturating_add(next_len as u64); @@ -146,7 +155,7 @@ pub fn check_combined_proof_size( total_pov_size as f64/1024.0, maximum_weight.max_block.proof_size() as f64/1024.0 ); - return Err(InvalidTransaction::ExhaustsResources.into()) + return Err(InvalidTransaction::ExhaustsResources.into()); } Ok(()) } @@ -190,7 +199,7 @@ where "Exceeded the per-class allowance.", ); - return Err(InvalidTransaction::ExhaustsResources.into()) + return Err(InvalidTransaction::ExhaustsResources.into()); }, // There is no `max_total` limit (`None`), // or we are below the limit. @@ -208,7 +217,7 @@ where "Total block weight is exceeded.", ); - return Err(InvalidTransaction::ExhaustsResources.into()) + return Err(InvalidTransaction::ExhaustsResources.into()); }, // There is either no limit in reserved pool (`None`), // or we are below the limit. @@ -791,6 +800,8 @@ mod tests { assert_eq!(maximum_weight.max_block, Weight::from_parts(20, 10)); + let info = DispatchInfo { class: DispatchClass::Normal, ..Default::default() }; + let mandatory = DispatchInfo { class: DispatchClass::Mandatory, ..Default::default() }; // We have 10 reftime and 5 proof size left over. let next_weight = crate::ConsumedWeight::new(|class| match class { DispatchClass::Normal => Weight::from_parts(10, 5), @@ -799,12 +810,33 @@ mod tests { }); // Simple checks for the length - assert_ok!(check_combined_proof_size(&maximum_weight, 0, &next_weight)); - assert_ok!(check_combined_proof_size(&maximum_weight, 5, &next_weight)); + assert_ok!(check_combined_proof_size::<<Test as Config>::RuntimeCall>( + &info, + &maximum_weight, + 0, + &next_weight + )); + assert_ok!(check_combined_proof_size::<<Test as Config>::RuntimeCall>( + &info, + &maximum_weight, + 5, + &next_weight + )); assert_err!( - check_combined_proof_size(&maximum_weight, 6, &next_weight), + check_combined_proof_size::<<Test as Config>::RuntimeCall>( + &info, + &maximum_weight, + 6, + &next_weight + ), InvalidTransaction::ExhaustsResources ); + assert_ok!(check_combined_proof_size::<<Test as Config>::RuntimeCall>( + &mandatory, + &maximum_weight, + 6, + &next_weight + )); // We have 10 reftime and 0 proof size left over. let next_weight = crate::ConsumedWeight::new(|class| match class { @@ -812,11 +844,27 @@ mod tests { DispatchClass::Operational => Weight::from_parts(0, 0), DispatchClass::Mandatory => Weight::zero(), }); - assert_ok!(check_combined_proof_size(&maximum_weight, 0, &next_weight)); + assert_ok!(check_combined_proof_size::<<Test as Config>::RuntimeCall>( + &info, + &maximum_weight, + 0, + &next_weight + )); assert_err!( - check_combined_proof_size(&maximum_weight, 1, &next_weight), + check_combined_proof_size::<<Test as Config>::RuntimeCall>( + &info, + &maximum_weight, + 1, + &next_weight + ), InvalidTransaction::ExhaustsResources ); + assert_ok!(check_combined_proof_size::<<Test as Config>::RuntimeCall>( + &mandatory, + &maximum_weight, + 1, + &next_weight + )); // We have 10 reftime and 2 proof size left over. // Used weight is spread across dispatch classes this time. @@ -825,12 +873,33 @@ mod tests { DispatchClass::Operational => Weight::from_parts(0, 3), DispatchClass::Mandatory => Weight::zero(), }); - assert_ok!(check_combined_proof_size(&maximum_weight, 0, &next_weight)); - assert_ok!(check_combined_proof_size(&maximum_weight, 2, &next_weight)); + assert_ok!(check_combined_proof_size::<<Test as Config>::RuntimeCall>( + &info, + &maximum_weight, + 0, + &next_weight + )); + assert_ok!(check_combined_proof_size::<<Test as Config>::RuntimeCall>( + &info, + &maximum_weight, + 2, + &next_weight + )); assert_err!( - check_combined_proof_size(&maximum_weight, 3, &next_weight), + check_combined_proof_size::<<Test as Config>::RuntimeCall>( + &info, + &maximum_weight, + 3, + &next_weight + ), InvalidTransaction::ExhaustsResources ); + assert_ok!(check_combined_proof_size::<<Test as Config>::RuntimeCall>( + &mandatory, + &maximum_weight, + 3, + &next_weight + )); // Ref time is over the limit. Should not happen, but we should make sure that it is // ignored. @@ -839,11 +908,32 @@ mod tests { DispatchClass::Operational => Weight::from_parts(0, 0), DispatchClass::Mandatory => Weight::zero(), }); - assert_ok!(check_combined_proof_size(&maximum_weight, 0, &next_weight)); - assert_ok!(check_combined_proof_size(&maximum_weight, 5, &next_weight)); + assert_ok!(check_combined_proof_size::<<Test as Config>::RuntimeCall>( + &info, + &maximum_weight, + 0, + &next_weight + )); + assert_ok!(check_combined_proof_size::<<Test as Config>::RuntimeCall>( + &info, + &maximum_weight, + 5, + &next_weight + )); assert_err!( - check_combined_proof_size(&maximum_weight, 6, &next_weight), + check_combined_proof_size::<<Test as Config>::RuntimeCall>( + &info, + &maximum_weight, + 6, + &next_weight + ), InvalidTransaction::ExhaustsResources ); + assert_ok!(check_combined_proof_size::<<Test as Config>::RuntimeCall>( + &mandatory, + &maximum_weight, + 6, + &next_weight + )); } } -- GitLab