Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Commit

Permalink
Revert "Properly set the max proof size weight on defaults and tests (#…
Browse files Browse the repository at this point in the history
…12383)"

This reverts commit 61b9a4d.
  • Loading branch information
coderobe committed Oct 4, 2022
1 parent d56f89b commit cd6397b
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 19 deletions.
9 changes: 6 additions & 3 deletions frame/election-provider-multi-phase/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1008,8 +1008,10 @@ pub mod pallet {
// unlikely to ever return an error: if phase is signed, snapshot will exist.
let size = Self::snapshot_metadata().ok_or(Error::<T>::MissingSnapshotMetadata)?;

// TODO: account for proof size weight
ensure!(
Self::solution_weight_of(&raw_solution, size).all_lt(T::SignedMaxWeight::get()),
Self::solution_weight_of(&raw_solution, size).ref_time() <
T::SignedMaxWeight::get().ref_time(),
Error::<T>::SignedTooMuchWeight,
);

Expand Down Expand Up @@ -2336,8 +2338,9 @@ mod tests {
};

let mut active = 1;
while weight_with(active)
.all_lte(<Runtime as frame_system::Config>::BlockWeights::get().max_block) ||
// TODO: account for proof size weight
while weight_with(active).ref_time() <=
<Runtime as frame_system::Config>::BlockWeights::get().max_block.ref_time() ||
active == all_voters
{
active += 1;
Expand Down
9 changes: 6 additions & 3 deletions frame/election-provider-multi-phase/src/unsigned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,8 @@ impl<T: MinerConfig> Miner<T> {
};

let next_voters = |current_weight: Weight, voters: u32, step: u32| -> Result<u32, ()> {
if current_weight.all_lt(max_weight) {
// TODO: account for proof size weight
if current_weight.ref_time() < max_weight.ref_time() {
let next_voters = voters.checked_add(step);
match next_voters {
Some(voters) if voters < max_voters => Ok(voters),
Expand Down Expand Up @@ -673,16 +674,18 @@ impl<T: MinerConfig> Miner<T> {

// Time to finish. We might have reduced less than expected due to rounding error. Increase
// one last time if we have any room left, the reduce until we are sure we are below limit.
while voters < max_voters && weight_with(voters + 1).all_lt(max_weight) {
// TODO: account for proof size weight
while voters < max_voters && weight_with(voters + 1).ref_time() < max_weight.ref_time() {
voters += 1;
}
while voters.checked_sub(1).is_some() && weight_with(voters).any_gt(max_weight) {
voters -= 1;
}

let final_decision = voters.min(size.voters);
// TODO: account for proof size weight
debug_assert!(
weight_with(final_decision).all_lte(max_weight),
weight_with(final_decision).ref_time() <= max_weight.ref_time(),
"weight_with({}) <= {}",
final_decision,
max_weight,
Expand Down
3 changes: 2 additions & 1 deletion frame/executive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,8 @@ where
let max_weight = <System::BlockWeights as frame_support::traits::Get<_>>::get().max_block;
let remaining_weight = max_weight.saturating_sub(weight.total());

if remaining_weight.all_gt(Weight::zero()) {
// TODO: account for proof size weight
if remaining_weight.ref_time() > 0 {
let used_weight = <AllPalletsWithSystem as OnIdle<System::BlockNumber>>::on_idle(
block_number,
remaining_weight,
Expand Down
3 changes: 2 additions & 1 deletion frame/grandpa/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -856,7 +856,8 @@ fn valid_equivocation_reports_dont_pay_fees() {
.get_dispatch_info();

// it should have non-zero weight and the fee has to be paid.
assert!(info.weight.all_gt(Weight::zero()));
// TODO: account for proof size weight
assert!(info.weight.ref_time() > 0);
assert_eq!(info.pays_fee, Pays::Yes);

// report the equivocation.
Expand Down
21 changes: 10 additions & 11 deletions frame/system/src/limits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ pub struct BlockWeights {

impl Default for BlockWeights {
fn default() -> Self {
Self::with_sensible_defaults(1u32 * constants::WEIGHT_PER_SECOND, DEFAULT_NORMAL_RATIO)
Self::with_sensible_defaults(1u64 * constants::WEIGHT_PER_SECOND, DEFAULT_NORMAL_RATIO)
}
}

Expand All @@ -224,6 +224,7 @@ impl BlockWeights {
}
let mut error = ValidationErrors::default();

// TODO: account for proof size weight in the assertions below
for class in DispatchClass::all() {
let weights = self.per_class.get(*class);
let max_for_class = or_max(weights.max_total);
Expand All @@ -232,18 +233,16 @@ impl BlockWeights {
// Make sure that if total is set it's greater than base_block &&
// base_for_class
error_assert!(
(max_for_class.all_gt(self.base_block) && max_for_class.all_gt(base_for_class))
|| max_for_class == Weight::zero(),
(max_for_class.ref_time() > self.base_block.ref_time() && max_for_class.ref_time() > base_for_class.ref_time())
|| max_for_class.ref_time() == 0,
&mut error,
"[{:?}] {:?} (total) has to be greater than {:?} (base block) & {:?} (base extrinsic)",
class, max_for_class, self.base_block, base_for_class,
);
// Max extrinsic can't be greater than max_for_class.
error_assert!(
weights
.max_extrinsic
.unwrap_or(Weight::zero())
.all_lte(max_for_class.saturating_sub(base_for_class)),
weights.max_extrinsic.unwrap_or(Weight::zero()).ref_time() <=
max_for_class.saturating_sub(base_for_class).ref_time(),
&mut error,
"[{:?}] {:?} (max_extrinsic) can't be greater than {:?} (max for class)",
class,
Expand All @@ -252,14 +251,14 @@ impl BlockWeights {
);
// Max extrinsic should not be 0
error_assert!(
weights.max_extrinsic.unwrap_or_else(Weight::max_value).all_gt(Weight::zero()),
weights.max_extrinsic.unwrap_or_else(Weight::max_value).ref_time() > 0,
&mut error,
"[{:?}] {:?} (max_extrinsic) must not be 0. Check base cost and average initialization cost.",
class, weights.max_extrinsic,
);
// Make sure that if reserved is set it's greater than base_for_class.
error_assert!(
reserved.all_gt(base_for_class) || reserved == Weight::zero(),
reserved.ref_time() > base_for_class.ref_time() || reserved.ref_time() == 0,
&mut error,
"[{:?}] {:?} (reserved) has to be greater than {:?} (base extrinsic) if set",
class,
Expand All @@ -268,7 +267,7 @@ impl BlockWeights {
);
// Make sure max block is greater than max_total if it's set.
error_assert!(
self.max_block.all_gte(weights.max_total.unwrap_or(Weight::zero())),
self.max_block.ref_time() >= weights.max_total.unwrap_or(Weight::zero()).ref_time(),
&mut error,
"[{:?}] {:?} (max block) has to be greater than {:?} (max for class)",
class,
Expand All @@ -277,7 +276,7 @@ impl BlockWeights {
);
// Make sure we can fit at least one extrinsic.
error_assert!(
self.max_block.all_gt(base_for_class + self.base_block),
self.max_block.ref_time() > (base_for_class + self.base_block).ref_time(),
&mut error,
"[{:?}] {:?} (max block) must fit at least one extrinsic {:?} (base weight)",
class,
Expand Down

0 comments on commit cd6397b

Please sign in to comment.