diff --git a/prdoc/pr_4571.prdoc b/prdoc/pr_4571.prdoc new file mode 100644 index 000000000000..b03fee8a5cc8 --- /dev/null +++ b/prdoc/pr_4571.prdoc @@ -0,0 +1,19 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: Ignore mandatory extrinsics in total PoV size check + +doc: + - audience: Runtime Dev + description: | + The `CheckWeight` extension is checking that extrinsic length and used storage proof + weight together do not exceed the PoV size limit. This lead to problems when + the PoV size was already reached before mandatory extrinsics were applied.The `CheckWeight` + extension will now allow extrinsics of `DispatchClass::Mandatory` to be applied even if + the limit is reached. + +crates: + - name: frame-system + bump: minor + - name: polkadot-sdk + bump: minor diff --git a/substrate/frame/system/src/extensions/check_weight.rs b/substrate/frame/system/src/extensions/check_weight.rs index 061d543f8c31..5d6c68989ed5 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::(&maximum_weight, all_weight, info)?; - check_combined_proof_size(&maximum_weight, next_len, &next_weight)?; + check_combined_proof_size::(info, &maximum_weight, next_len, &next_weight)?; Self::check_extrinsic_weight(info)?; crate::AllExtrinsicsLen::::put(next_len); @@ -131,22 +131,31 @@ 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( + info: &DispatchInfoOf, maximum_weight: &BlockWeights, next_len: u32, next_weight: &crate::ConsumedWeight, -) -> Result<(), TransactionValidityError> { +) -> Result<(), TransactionValidityError> +where + Call: Dispatchable, +{ // 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); if total_pov_size > maximum_weight.max_block.proof_size() { log::debug!( target: LOG_TARGET, - "Extrinsic exceeds total pov size: {}kb, limit: {}kb", + "Extrinsic exceeds total pov size. Still including if mandatory. size: {}kb, limit: {}kb, is_mandatory: {}", total_pov_size as f64/1024.0, - maximum_weight.max_block.proof_size() as f64/1024.0 + maximum_weight.max_block.proof_size() as f64/1024.0, + info.class == DispatchClass::Mandatory ); - return Err(InvalidTransaction::ExhaustsResources.into()) + return match info.class { + // Allow mandatory extrinsics + DispatchClass::Mandatory => Ok(()), + _ => 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::<::RuntimeCall>( + &info, + &maximum_weight, + 0, + &next_weight + )); + assert_ok!(check_combined_proof_size::<::RuntimeCall>( + &info, + &maximum_weight, + 5, + &next_weight + )); assert_err!( - check_combined_proof_size(&maximum_weight, 6, &next_weight), + check_combined_proof_size::<::RuntimeCall>( + &info, + &maximum_weight, + 6, + &next_weight + ), InvalidTransaction::ExhaustsResources ); + assert_ok!(check_combined_proof_size::<::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::<::RuntimeCall>( + &info, + &maximum_weight, + 0, + &next_weight + )); assert_err!( - check_combined_proof_size(&maximum_weight, 1, &next_weight), + check_combined_proof_size::<::RuntimeCall>( + &info, + &maximum_weight, + 1, + &next_weight + ), InvalidTransaction::ExhaustsResources ); + assert_ok!(check_combined_proof_size::<::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::<::RuntimeCall>( + &info, + &maximum_weight, + 0, + &next_weight + )); + assert_ok!(check_combined_proof_size::<::RuntimeCall>( + &info, + &maximum_weight, + 2, + &next_weight + )); assert_err!( - check_combined_proof_size(&maximum_weight, 3, &next_weight), + check_combined_proof_size::<::RuntimeCall>( + &info, + &maximum_weight, + 3, + &next_weight + ), InvalidTransaction::ExhaustsResources ); + assert_ok!(check_combined_proof_size::<::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::<::RuntimeCall>( + &info, + &maximum_weight, + 0, + &next_weight + )); + assert_ok!(check_combined_proof_size::<::RuntimeCall>( + &info, + &maximum_weight, + 5, + &next_weight + )); assert_err!( - check_combined_proof_size(&maximum_weight, 6, &next_weight), + check_combined_proof_size::<::RuntimeCall>( + &info, + &maximum_weight, + 6, + &next_weight + ), InvalidTransaction::ExhaustsResources ); + assert_ok!(check_combined_proof_size::<::RuntimeCall>( + &mandatory, + &maximum_weight, + 6, + &next_weight + )); } }