Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CheckWeight: account for extrinsic len as proof size #4765

Merged
merged 14 commits into from
Jun 14, 2024
6 changes: 1 addition & 5 deletions cumulus/client/consensus/aura/src/collators/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,11 +246,7 @@ where
None,
(parachain_inherent_data, other_inherent_data),
params.authoring_duration,
// Set the block limit to 50% of the maximum PoV size.
//
// TODO: If we got benchmarking that includes the proof size,
// we should be able to use the maximum pov size.
(validation_data.max_pov_size / 2) as usize,
validation_data.max_pov_size,
sandreim marked this conversation as resolved.
Show resolved Hide resolved
)
.await
);
Expand Down
6 changes: 1 addition & 5 deletions cumulus/client/consensus/aura/src/collators/lookahead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -387,11 +387,7 @@ where
None,
(parachain_inherent_data, other_inherent_data),
params.authoring_duration,
// Set the block limit to 50% of the maximum PoV size.
//
// TODO: If we got benchmarking that includes the proof size,
// we should be able to use the maximum pov size.
(validation_data.max_pov_size / 2) as usize,
validation_data.max_pov_size,
)
.await
{
Expand Down
50 changes: 49 additions & 1 deletion cumulus/primitives/storage-weight-reclaim/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ mod tests {
use super::*;
use frame_support::{
assert_ok,
dispatch::DispatchClass,
dispatch::{DispatchClass, PerDispatchClass},
weights::{Weight, WeightMeter},
};
use frame_system::{BlockWeight, CheckWeight};
Expand Down Expand Up @@ -256,6 +256,10 @@ mod tests {
});
}

fn get_current_storage_weight() -> PerDispatchClass<Weight> {
sandreim marked this conversation as resolved.
Show resolved Hide resolved
BlockWeight::<Test>::get()
}

#[test]
fn basic_refund() {
// The real cost will be 100 bytes of storage size
Expand Down Expand Up @@ -471,6 +475,50 @@ mod tests {
})
}

#[test]
fn test_block_len_weight_is_not_reclaimed() {
let mut test_ext = setup_test_externalities(&[100, 200]);

test_ext.execute_with(|| {
set_current_storage_weight(0);
// Benchmarked storage weight: 100
let info = DispatchInfo { weight: Weight::from_parts(100, 100), ..Default::default() };

// Actual proof size is 100
let post_info = PostDispatchInfo {
actual_weight: Some(Weight::from_parts(50, 100)),
pays_fee: Default::default(),
};

assert_ok!(CheckWeight::<Test>::do_pre_dispatch(&info, 200));
// Weight should go up by 200 len + 100 proof size weight
assert_eq!(get_current_storage_weight().total().proof_size(), 300);

let pre = StorageWeightReclaim::<Test>(PhantomData)
.pre_dispatch(&ALICE, CALL, &info, 200)
.unwrap();
// Should return `setup_test_externalities` proof recorder value: 100.
assert_eq!(pre, Some(100));

// The `CheckWeight` extension will refund `actual_weight` from `PostDispatchInfo`
// we always need to call `post_dispatch` to verify that they interoperate correctly.
assert_ok!(CheckWeight::<Test>::post_dispatch(None, &info, &post_info, 200, &Ok(())));
// `setup_test_externalities` proof recorder value: 200, so this means the extrinsic
// actually used 100 proof size.
assert_ok!(StorageWeightReclaim::<Test>::post_dispatch(
Some(pre),
&info,
&post_info,
200,
&Ok(())
));

// Check block len weight was not reclaimed:
// 100 weight + 200 extrinsic len == 300 proof size
assert_eq!(get_current_storage_weight().total().proof_size(), 300);
sandreim marked this conversation as resolved.
Show resolved Hide resolved
})
}

#[test]
fn test_incorporates_check_weight_unspent_weight_reverse_order() {
let mut test_ext = setup_test_externalities(&[100, 300]);
Expand Down
14 changes: 14 additions & 0 deletions prdoc/pr_4765.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
title: Cumulus - remove defensive 50% PoV size limit

doc:
- audience: Runtime Dev
description: |
This changes how CheckWeight extension works. It will now account for the extrinsic length
as proof size. When `on_idle` is called, the remaining weight parameter reflects this.

crates:
- name: frame-system
bump: patch
- name: cumulus-client-consensus-aura
bump: minor

Loading
Loading