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

Commit

Permalink
Remove Ord impl for Weights V2 and add comparison fns (#12183)
Browse files Browse the repository at this point in the history
* Remove Ord impl for Weights V2 and add comparison fns

* Remove TODO

* Update frame/multisig/src/lib.rs

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Update frame/election-provider-multi-phase/src/unsigned.rs

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Remove unused import

* cargo fmt

* Fix tests

* Fix more tests

* cargo fmt

* Fix more tests

* Update frame/contracts/src/wasm/mod.rs

Co-authored-by: Alexander Theißen <alex.theissen@me.com>

* Update weight benchmarking templates

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Alexander Theißen <alex.theissen@me.com>
  • Loading branch information
3 people authored Sep 8, 2022
1 parent c0e1cdb commit 2619394
Show file tree
Hide file tree
Showing 23 changed files with 156 additions and 82 deletions.
4 changes: 2 additions & 2 deletions frame/babe/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,7 @@ fn report_equivocation_has_valid_weight() {
.map(<Test as Config>::WeightInfo::report_equivocation)
.collect::<Vec<_>>()
.windows(2)
.all(|w| w[0] < w[1]));
.all(|w| w[0].ref_time() < w[1].ref_time()));
}

#[test]
Expand Down Expand Up @@ -852,7 +852,7 @@ 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 > Weight::zero());
assert!(info.weight.all_gt(Weight::zero()));
assert_eq!(info.pays_fee, Pays::Yes);

// report the equivocation.
Expand Down
2 changes: 1 addition & 1 deletion frame/collective/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -881,7 +881,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
ensure!(proposal_len <= length_bound, Error::<T, I>::WrongProposalLength);
let proposal = ProposalOf::<T, I>::get(hash).ok_or(Error::<T, I>::ProposalMissing)?;
let proposal_weight = proposal.get_dispatch_info().weight;
ensure!(proposal_weight <= weight_bound, Error::<T, I>::WrongProposalWeight);
ensure!(proposal_weight.all_lte(weight_bound), Error::<T, I>::WrongProposalWeight);
Ok((proposal, proposal_len as usize))
}

Expand Down
2 changes: 1 addition & 1 deletion frame/contracts/src/gas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ where

// NOTE that it is ok to allocate all available gas since it still ensured
// by `charge` that it doesn't reach zero.
if self.gas_left < amount {
if self.gas_left.any_lt(amount) {
Err(<Error<T>>::OutOfGas.into())
} else {
self.gas_left -= amount;
Expand Down
6 changes: 3 additions & 3 deletions frame/contracts/src/wasm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1555,8 +1555,8 @@ mod tests {

let gas_left = Weight::decode(&mut &*output.data).unwrap();
let actual_left = ext.gas_meter.gas_left();
assert!(gas_left < gas_limit, "gas_left must be less than initial");
assert!(gas_left > actual_left, "gas_left must be greater than final");
assert!(gas_left.all_lt(gas_limit), "gas_left must be less than initial");
assert!(gas_left.all_gt(actual_left), "gas_left must be greater than final");
}

const CODE_VALUE_TRANSFERRED: &str = r#"
Expand Down Expand Up @@ -1953,7 +1953,7 @@ mod tests {
)]
);

assert!(mock_ext.gas_meter.gas_left() > Weight::zero());
assert!(mock_ext.gas_meter.gas_left().all_gt(Weight::zero()));
}

const CODE_DEPOSIT_EVENT_MAX_TOPICS: &str = r#"
Expand Down
6 changes: 3 additions & 3 deletions frame/election-provider-multi-phase/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -985,7 +985,7 @@ pub mod pallet {
let size = Self::snapshot_metadata().ok_or(Error::<T>::MissingSnapshotMetadata)?;

ensure!(
Self::solution_weight_of(&raw_solution, size) < T::SignedMaxWeight::get(),
Self::solution_weight_of(&raw_solution, size).all_lt(T::SignedMaxWeight::get()),
Error::<T>::SignedTooMuchWeight,
);

Expand Down Expand Up @@ -2299,8 +2299,8 @@ mod tests {
};

let mut active = 1;
while weight_with(active) <=
<Runtime as frame_system::Config>::BlockWeights::get().max_block ||
while weight_with(active)
.all_lte(<Runtime as frame_system::Config>::BlockWeights::get().max_block) ||
active == all_voters
{
active += 1;
Expand Down
29 changes: 15 additions & 14 deletions frame/election-provider-multi-phase/src/unsigned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use sp_runtime::{
offchain::storage::{MutateStorageError, StorageValueRef},
DispatchError, SaturatedConversion,
};
use sp_std::{cmp::Ordering, prelude::*};
use sp_std::prelude::*;

/// Storage key used to store the last block number at which offchain worker ran.
pub(crate) const OFFCHAIN_LAST_BLOCK: &[u8] = b"parity/multi-phase-unsigned-election";
Expand Down Expand Up @@ -638,16 +638,17 @@ impl<T: MinerConfig> Miner<T> {
};

let next_voters = |current_weight: Weight, voters: u32, step: u32| -> Result<u32, ()> {
match current_weight.cmp(&max_weight) {
Ordering::Less => {
let next_voters = voters.checked_add(step);
match next_voters {
Some(voters) if voters < max_voters => Ok(voters),
_ => Err(()),
}
},
Ordering::Greater => voters.checked_sub(step).ok_or(()),
Ordering::Equal => Ok(voters),
if current_weight.all_lt(max_weight) {
let next_voters = voters.checked_add(step);
match next_voters {
Some(voters) if voters < max_voters => Ok(voters),
_ => Err(()),
}
} else if current_weight.any_gt(max_weight) {
voters.checked_sub(step).ok_or(())
} else {
// If any of the constituent weights is equal to the max weight, we're at max
Ok(voters)
}
};

Expand All @@ -672,16 +673,16 @@ 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) < max_weight {
while voters < max_voters && weight_with(voters + 1).all_lt(max_weight) {
voters += 1;
}
while voters.checked_sub(1).is_some() && weight_with(voters) > max_weight {
while voters.checked_sub(1).is_some() && weight_with(voters).any_gt(max_weight) {
voters -= 1;
}

let final_decision = voters.min(size.voters);
debug_assert!(
weight_with(final_decision) <= max_weight,
weight_with(final_decision).all_lte(max_weight),
"weight_with({}) <= {}",
final_decision,
max_weight,
Expand Down
4 changes: 2 additions & 2 deletions frame/examples/basic/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,11 +190,11 @@ fn weights_work() {
let default_call = pallet_example_basic::Call::<Test>::accumulate_dummy { increase_by: 10 };
let info1 = default_call.get_dispatch_info();
// aka. `let info = <Call<Test> as GetDispatchInfo>::get_dispatch_info(&default_call);`
assert!(info1.weight > Weight::zero());
assert!(info1.weight.all_gt(Weight::zero()));

// `set_dummy` is simpler than `accumulate_dummy`, and the weight
// should be less.
let custom_call = pallet_example_basic::Call::<Test>::set_dummy { new_value: 20 };
let info2 = custom_call.get_dispatch_info();
assert!(info1.weight > info2.weight);
assert!(info1.weight.all_gt(info2.weight));
}
2 changes: 1 addition & 1 deletion frame/executive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ 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 > Weight::zero() {
if remaining_weight.all_gt(Weight::zero()) {
let used_weight = <AllPalletsWithSystem as OnIdle<System::BlockNumber>>::on_idle(
block_number,
remaining_weight,
Expand Down
4 changes: 2 additions & 2 deletions frame/grandpa/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,7 @@ fn report_equivocation_has_valid_weight() {
.map(<Test as Config>::WeightInfo::report_equivocation)
.collect::<Vec<_>>()
.windows(2)
.all(|w| w[0] < w[1]));
.all(|w| w[0].ref_time() < w[1].ref_time()));
}

#[test]
Expand Down Expand Up @@ -856,7 +856,7 @@ 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 > Weight::zero());
assert!(info.weight.all_gt(Weight::zero()));
assert_eq!(info.pays_fee, Pays::Yes);

// report the equivocation.
Expand Down
5 changes: 4 additions & 1 deletion frame/multisig/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,10 @@ impl<T: Config> Pallet<T> {

if let Some((call, call_len)) = maybe_approved_call {
// verify weight
ensure!(call.get_dispatch_info().weight <= max_weight, Error::<T>::MaxWeightTooLow);
ensure!(
call.get_dispatch_info().weight.all_lte(max_weight),
Error::<T>::MaxWeightTooLow
);

// Clean up storage before executing call to avoid an possibility of reentrancy
// attack.
Expand Down
2 changes: 1 addition & 1 deletion frame/scheduler/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ pub mod pallet {
let hard_deadline = s.priority <= schedule::HARD_DEADLINE;
let test_weight =
total_weight.saturating_add(call_weight).saturating_add(item_weight);
if !hard_deadline && order > 0 && test_weight > limit {
if !hard_deadline && order > 0 && test_weight.any_gt(limit) {
// Cannot be scheduled this block - postpone until next.
total_weight.saturating_accrue(T::WeightInfo::item(false, named, None));
if let Some(ref id) = s.maybe_id {
Expand Down
10 changes: 5 additions & 5 deletions frame/staking/src/pallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use sp_runtime::{
Perbill, Percent,
};
use sp_staking::{EraIndex, SessionIndex};
use sp_std::{cmp::max, prelude::*};
use sp_std::prelude::*;

mod impls;

Expand Down Expand Up @@ -1571,10 +1571,10 @@ pub mod pallet {
/// to kick people under the new limits, `chill_other` should be called.
// We assume the worst case for this call is either: all items are set or all items are
// removed.
#[pallet::weight(max(
T::WeightInfo::set_staking_configs_all_set(),
T::WeightInfo::set_staking_configs_all_remove()
))]
#[pallet::weight(
T::WeightInfo::set_staking_configs_all_set()
.max(T::WeightInfo::set_staking_configs_all_remove())
)]
pub fn set_staking_configs(
origin: OriginFor<T>,
min_nominator_bond: ConfigOp<BalanceOf<T>>,
Expand Down
16 changes: 8 additions & 8 deletions frame/staking/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3759,9 +3759,9 @@ fn payout_stakers_handles_weight_refund() {
let half_max_nom_rewarded_weight =
<Test as Config>::WeightInfo::payout_stakers_alive_staked(half_max_nom_rewarded);
let zero_nom_payouts_weight = <Test as Config>::WeightInfo::payout_stakers_alive_staked(0);
assert!(zero_nom_payouts_weight > Weight::zero());
assert!(half_max_nom_rewarded_weight > zero_nom_payouts_weight);
assert!(max_nom_rewarded_weight > half_max_nom_rewarded_weight);
assert!(zero_nom_payouts_weight.any_gt(Weight::zero()));
assert!(half_max_nom_rewarded_weight.any_gt(zero_nom_payouts_weight));
assert!(max_nom_rewarded_weight.any_gt(half_max_nom_rewarded_weight));

let balance = 1000;
bond_validator(11, 10, balance);
Expand Down Expand Up @@ -4238,7 +4238,7 @@ fn do_not_die_when_active_is_ed() {
fn on_finalize_weight_is_nonzero() {
ExtBuilder::default().build_and_execute(|| {
let on_finalize_weight = <Test as frame_system::Config>::DbWeight::get().reads(1);
assert!(<Staking as Hooks<u64>>::on_initialize(1) >= on_finalize_weight);
assert!(<Staking as Hooks<u64>>::on_initialize(1).all_gte(on_finalize_weight));
})
}

Expand All @@ -4249,8 +4249,8 @@ mod election_data_provider {
#[test]
fn targets_2sec_block() {
let mut validators = 1000;
while <Test as Config>::WeightInfo::get_npos_targets(validators) <
2u64 * frame_support::weights::constants::WEIGHT_PER_SECOND
while <Test as Config>::WeightInfo::get_npos_targets(validators)
.all_lt(2u64 * frame_support::weights::constants::WEIGHT_PER_SECOND)
{
validators += 1;
}
Expand All @@ -4267,8 +4267,8 @@ mod election_data_provider {
let slashing_spans = validators;
let mut nominators = 1000;

while <Test as Config>::WeightInfo::get_npos_voters(validators, nominators, slashing_spans) <
2u64 * frame_support::weights::constants::WEIGHT_PER_SECOND
while <Test as Config>::WeightInfo::get_npos_voters(validators, nominators, slashing_spans)
.all_lt(2u64 * frame_support::weights::constants::WEIGHT_PER_SECOND)
{
nominators += 1;
}
Expand Down
10 changes: 8 additions & 2 deletions frame/support/src/weights/block_weights.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,14 @@ mod test_weights {
let w = super::BlockExecutionWeight::get();

// At least 100 µs.
assert!(w >= 100u32 * constants::WEIGHT_PER_MICROS, "Weight should be at least 100 µs.");
assert!(
w.ref_time() >= 100u64 * constants::WEIGHT_PER_MICROS.ref_time(),
"Weight should be at least 100 µs."
);
// At most 50 ms.
assert!(w <= 50u32 * constants::WEIGHT_PER_MILLIS, "Weight should be at most 50 ms.");
assert!(
w.ref_time() <= 50u64 * constants::WEIGHT_PER_MILLIS.ref_time(),
"Weight should be at most 50 ms."
);
}
}
10 changes: 8 additions & 2 deletions frame/support/src/weights/extrinsic_weights.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,14 @@ mod test_weights {
let w = super::ExtrinsicBaseWeight::get();

// At least 10 µs.
assert!(w >= 10u32 * constants::WEIGHT_PER_MICROS, "Weight should be at least 10 µs.");
assert!(
w.ref_time() >= 10u64 * constants::WEIGHT_PER_MICROS.ref_time(),
"Weight should be at least 10 µs."
);
// At most 1 ms.
assert!(w <= constants::WEIGHT_PER_MILLIS, "Weight should be at most 1 ms.");
assert!(
w.ref_time() <= constants::WEIGHT_PER_MILLIS.ref_time(),
"Weight should be at most 1 ms."
);
}
}
8 changes: 4 additions & 4 deletions frame/support/src/weights/paritydb_weights.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,20 +42,20 @@ pub mod constants {
fn sane() {
// At least 1 µs.
assert!(
W::get().reads(1) >= constants::WEIGHT_PER_MICROS,
W::get().reads(1).ref_time() >= constants::WEIGHT_PER_MICROS.ref_time(),
"Read weight should be at least 1 µs."
);
assert!(
W::get().writes(1) >= constants::WEIGHT_PER_MICROS,
W::get().writes(1).ref_time() >= constants::WEIGHT_PER_MICROS.ref_time(),
"Write weight should be at least 1 µs."
);
// At most 1 ms.
assert!(
W::get().reads(1) <= constants::WEIGHT_PER_MILLIS,
W::get().reads(1).ref_time() <= constants::WEIGHT_PER_MILLIS.ref_time(),
"Read weight should be at most 1 ms."
);
assert!(
W::get().writes(1) <= constants::WEIGHT_PER_MILLIS,
W::get().writes(1).ref_time() <= constants::WEIGHT_PER_MILLIS.ref_time(),
"Write weight should be at most 1 ms."
);
}
Expand Down
8 changes: 4 additions & 4 deletions frame/support/src/weights/rocksdb_weights.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,20 +42,20 @@ pub mod constants {
fn sane() {
// At least 1 µs.
assert!(
W::get().reads(1) >= constants::WEIGHT_PER_MICROS,
W::get().reads(1).ref_time() >= constants::WEIGHT_PER_MICROS.ref_time(),
"Read weight should be at least 1 µs."
);
assert!(
W::get().writes(1) >= constants::WEIGHT_PER_MICROS,
W::get().writes(1).ref_time() >= constants::WEIGHT_PER_MICROS.ref_time(),
"Write weight should be at least 1 µs."
);
// At most 1 ms.
assert!(
W::get().reads(1) <= constants::WEIGHT_PER_MILLIS,
W::get().reads(1).ref_time() <= constants::WEIGHT_PER_MILLIS.ref_time(),
"Read weight should be at most 1 ms."
);
assert!(
W::get().writes(1) <= constants::WEIGHT_PER_MILLIS,
W::get().writes(1).ref_time() <= constants::WEIGHT_PER_MILLIS.ref_time(),
"Write weight should be at most 1 ms."
);
}
Expand Down
Loading

0 comments on commit 2619394

Please sign in to comment.