Skip to content

Commit 2e88ce5

Browse files
skunertTarekkMA
skunert
authored and
TarekkMA
committed
check-weight: Disable total pov size check for mandatory extrinsics (paritytech#4571)
So in some pallets we like [here](https://github.com/paritytech/polkadot-sdk/blob/5dc522d02fe0b53be1517f8b8979176e489a388b/substrate/frame/session/src/lib.rs#L556) we use `max_block` as return value for `on_initialize` (ideally we would not). This means the block is already full when we try to apply the inherents, which lead to the error seen in paritytech#4559 because we are unable to include the required inherents. This was not erroring before paritytech#4326 because we were running into this branch: https://github.com/paritytech/polkadot-sdk/blob/e4b89cc50c8d17868d6c8b122f2e156d678c7525/substrate/frame/system/src/extensions/check_weight.rs#L222-L224 The inherents are of `DispatchClass::Mandatory` and therefore have a `reserved` value of `None` in all runtimes I have inspected. So they will always pass the normal check. So in this PR I adjust the `check_combined_proof_size` to return an early `Ok(())` for mandatory extrinsics. If we agree on this PR I will backport it to the 1.12.0 branch. closes paritytech#4559 --------- Co-authored-by: command-bot <>
1 parent 870b824 commit 2e88ce5

File tree

2 files changed

+129
-20
lines changed

2 files changed

+129
-20
lines changed

‎prdoc/pr_4571.prdoc‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
2+
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json
3+
4+
title: Ignore mandatory extrinsics in total PoV size check
5+
6+
doc:
7+
- audience: Runtime Dev
8+
description: |
9+
The `CheckWeight` extension is checking that extrinsic length and used storage proof
10+
weight together do not exceed the PoV size limit. This lead to problems when
11+
the PoV size was already reached before mandatory extrinsics were applied.The `CheckWeight`
12+
extension will now allow extrinsics of `DispatchClass::Mandatory` to be applied even if
13+
the limit is reached.
14+
15+
crates:
16+
- name: frame-system
17+
bump: minor
18+
- name: polkadot-sdk
19+
bump: minor

‎substrate/frame/system/src/extensions/check_weight.rs‎

Lines changed: 110 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
// See the License for the specific language governing permissions and
1616
// limitations under the License.
1717

18-
use crate::{limits::BlockWeights, Config, Pallet, LOG_TARGET};
18+
use crate::{limits::BlockWeights, Config, DispatchClass, Pallet, LOG_TARGET};
1919
use codec::{Decode, Encode};
2020
use frame_support::{
2121
dispatch::{DispatchInfo, PostDispatchInfo},
@@ -107,7 +107,7 @@ where
107107
let maximum_weight = T::BlockWeights::get();
108108
let next_weight =
109109
calculate_consumed_weight::<T::RuntimeCall>(&maximum_weight, all_weight, info)?;
110-
check_combined_proof_size(&maximum_weight, next_len, &next_weight)?;
110+
check_combined_proof_size::<T::RuntimeCall>(info, &maximum_weight, next_len, &next_weight)?;
111111
Self::check_extrinsic_weight(info)?;
112112

113113
crate::AllExtrinsicsLen::<T>::put(next_len);
@@ -131,22 +131,31 @@ where
131131
}
132132

133133
/// Check that the combined extrinsic length and proof size together do not exceed the PoV limit.
134-
pub fn check_combined_proof_size(
134+
pub fn check_combined_proof_size<Call>(
135+
info: &DispatchInfoOf<Call>,
135136
maximum_weight: &BlockWeights,
136137
next_len: u32,
137138
next_weight: &crate::ConsumedWeight,
138-
) -> Result<(), TransactionValidityError> {
139+
) -> Result<(), TransactionValidityError>
140+
where
141+
Call: Dispatchable<Info = DispatchInfo, PostInfo = PostDispatchInfo>,
142+
{
139143
// This extra check ensures that the extrinsic length does not push the
140144
// PoV over the limit.
141145
let total_pov_size = next_weight.total().proof_size().saturating_add(next_len as u64);
142146
if total_pov_size > maximum_weight.max_block.proof_size() {
143147
log::debug!(
144148
target: LOG_TARGET,
145-
"Extrinsic exceeds total pov size: {}kb, limit: {}kb",
149+
"Extrinsic exceeds total pov size. Still including if mandatory. size: {}kb, limit: {}kb, is_mandatory: {}",
146150
total_pov_size as f64/1024.0,
147-
maximum_weight.max_block.proof_size() as f64/1024.0
151+
maximum_weight.max_block.proof_size() as f64/1024.0,
152+
info.class == DispatchClass::Mandatory
148153
);
149-
return Err(InvalidTransaction::ExhaustsResources.into())
154+
return match info.class {
155+
// Allow mandatory extrinsics
156+
DispatchClass::Mandatory => Ok(()),
157+
_ => Err(InvalidTransaction::ExhaustsResources.into()),
158+
};
150159
}
151160
Ok(())
152161
}
@@ -190,7 +199,7 @@ where
190199
"Exceeded the per-class allowance.",
191200
);
192201

193-
return Err(InvalidTransaction::ExhaustsResources.into())
202+
return Err(InvalidTransaction::ExhaustsResources.into());
194203
},
195204
// There is no `max_total` limit (`None`),
196205
// or we are below the limit.
@@ -208,7 +217,7 @@ where
208217
"Total block weight is exceeded.",
209218
);
210219

211-
return Err(InvalidTransaction::ExhaustsResources.into())
220+
return Err(InvalidTransaction::ExhaustsResources.into());
212221
},
213222
// There is either no limit in reserved pool (`None`),
214223
// or we are below the limit.
@@ -791,6 +800,8 @@ mod tests {
791800

792801
assert_eq!(maximum_weight.max_block, Weight::from_parts(20, 10));
793802

803+
let info = DispatchInfo { class: DispatchClass::Normal, ..Default::default() };
804+
let mandatory = DispatchInfo { class: DispatchClass::Mandatory, ..Default::default() };
794805
// We have 10 reftime and 5 proof size left over.
795806
let next_weight = crate::ConsumedWeight::new(|class| match class {
796807
DispatchClass::Normal => Weight::from_parts(10, 5),
@@ -799,24 +810,61 @@ mod tests {
799810
});
800811

801812
// Simple checks for the length
802-
assert_ok!(check_combined_proof_size(&maximum_weight, 0, &next_weight));
803-
assert_ok!(check_combined_proof_size(&maximum_weight, 5, &next_weight));
813+
assert_ok!(check_combined_proof_size::<<Test as Config>::RuntimeCall>(
814+
&info,
815+
&maximum_weight,
816+
0,
817+
&next_weight
818+
));
819+
assert_ok!(check_combined_proof_size::<<Test as Config>::RuntimeCall>(
820+
&info,
821+
&maximum_weight,
822+
5,
823+
&next_weight
824+
));
804825
assert_err!(
805-
check_combined_proof_size(&maximum_weight, 6, &next_weight),
826+
check_combined_proof_size::<<Test as Config>::RuntimeCall>(
827+
&info,
828+
&maximum_weight,
829+
6,
830+
&next_weight
831+
),
806832
InvalidTransaction::ExhaustsResources
807833
);
834+
assert_ok!(check_combined_proof_size::<<Test as Config>::RuntimeCall>(
835+
&mandatory,
836+
&maximum_weight,
837+
6,
838+
&next_weight
839+
));
808840

809841
// We have 10 reftime and 0 proof size left over.
810842
let next_weight = crate::ConsumedWeight::new(|class| match class {
811843
DispatchClass::Normal => Weight::from_parts(10, 10),
812844
DispatchClass::Operational => Weight::from_parts(0, 0),
813845
DispatchClass::Mandatory => Weight::zero(),
814846
});
815-
assert_ok!(check_combined_proof_size(&maximum_weight, 0, &next_weight));
847+
assert_ok!(check_combined_proof_size::<<Test as Config>::RuntimeCall>(
848+
&info,
849+
&maximum_weight,
850+
0,
851+
&next_weight
852+
));
816853
assert_err!(
817-
check_combined_proof_size(&maximum_weight, 1, &next_weight),
854+
check_combined_proof_size::<<Test as Config>::RuntimeCall>(
855+
&info,
856+
&maximum_weight,
857+
1,
858+
&next_weight
859+
),
818860
InvalidTransaction::ExhaustsResources
819861
);
862+
assert_ok!(check_combined_proof_size::<<Test as Config>::RuntimeCall>(
863+
&mandatory,
864+
&maximum_weight,
865+
1,
866+
&next_weight
867+
));
820868

821869
// We have 10 reftime and 2 proof size left over.
822870
// Used weight is spread across dispatch classes this time.
@@ -825,12 +873,33 @@ mod tests {
825873
DispatchClass::Operational => Weight::from_parts(0, 3),
826874
DispatchClass::Mandatory => Weight::zero(),
827875
});
828-
assert_ok!(check_combined_proof_size(&maximum_weight, 0, &next_weight));
829-
assert_ok!(check_combined_proof_size(&maximum_weight, 2, &next_weight));
876+
assert_ok!(check_combined_proof_size::<<Test as Config>::RuntimeCall>(
877+
&info,
878+
&maximum_weight,
879+
0,
880+
&next_weight
881+
));
882+
assert_ok!(check_combined_proof_size::<<Test as Config>::RuntimeCall>(
883+
&info,
884+
&maximum_weight,
885+
2,
886+
&next_weight
887+
));
830888
assert_err!(
831-
check_combined_proof_size(&maximum_weight, 3, &next_weight),
889+
check_combined_proof_size::<<Test as Config>::RuntimeCall>(
890+
&info,
891+
&maximum_weight,
892+
3,
893+
&next_weight
894+
),
832895
InvalidTransaction::ExhaustsResources
833896
);
897+
assert_ok!(check_combined_proof_size::<<Test as Config>::RuntimeCall>(
898+
&mandatory,
899+
&maximum_weight,
900+
3,
901+
&next_weight
902+
));
834903

835904
// Ref time is over the limit. Should not happen, but we should make sure that it is
836905
// ignored.
@@ -839,11 +908,32 @@ mod tests {
839908
DispatchClass::Operational => Weight::from_parts(0, 0),
840909
DispatchClass::Mandatory => Weight::zero(),
841910
});
842-
assert_ok!(check_combined_proof_size(&maximum_weight, 0, &next_weight));
843-
assert_ok!(check_combined_proof_size(&maximum_weight, 5, &next_weight));
911+
assert_ok!(check_combined_proof_size::<<Test as Config>::RuntimeCall>(
912+
&info,
913+
&maximum_weight,
914+
0,
915+
&next_weight
916+
));
917+
assert_ok!(check_combined_proof_size::<<Test as Config>::RuntimeCall>(
918+
&info,
919+
&maximum_weight,
920+
5,
921+
&next_weight
922+
));
844923
assert_err!(
845-
check_combined_proof_size(&maximum_weight, 6, &next_weight),
924+
check_combined_proof_size::<<Test as Config>::RuntimeCall>(
925+
&info,
926+
&maximum_weight,
927+
6,
928+
&next_weight
929+
),
846930
InvalidTransaction::ExhaustsResources
847931
);
932+
assert_ok!(check_combined_proof_size::<<Test as Config>::RuntimeCall>(
933+
&mandatory,
934+
&maximum_weight,
935+
6,
936+
&next_weight
937+
));
848938
}
849939
}

0 commit comments

Comments
 (0)