From d56f89b71bd9cccf713931930f0af5d23d2c406d Mon Sep 17 00:00:00 2001 From: Mara Broda Date: Tue, 4 Oct 2022 17:53:06 +0200 Subject: [PATCH 1/2] Revert "Add storage size component to weights (#12277)" This reverts commit 17c07af0b953b84dbe89341294e98e586f9b4591. --- frame/alliance/src/lib.rs | 62 +---- frame/babe/src/tests.rs | 3 +- frame/collective/src/lib.rs | 69 +----- frame/contracts/src/lib.rs | 179 +-------------- frame/contracts/src/wasm/mod.rs | 15 +- .../election-provider-multi-phase/src/lib.rs | 9 +- .../src/unsigned.rs | 9 +- frame/examples/basic/src/tests.rs | 6 +- frame/executive/src/lib.rs | 7 +- frame/grandpa/src/tests.rs | 3 +- .../procedural/src/pallet/expand/call.rs | 20 -- .../procedural/src/pallet/parse/call.rs | 3 - ...age_ensure_span_are_ok_on_wrong_gen.stderr | 6 +- ...re_span_are_ok_on_wrong_gen_unnamed.stderr | 6 +- .../pallet_ui/storage_info_unsatisfied.stderr | 2 +- .../storage_info_unsatisfied_nmap.stderr | 2 +- frame/system/src/limits.rs | 21 +- frame/transaction-payment/src/types.rs | 5 +- primitives/weights/src/lib.rs | 21 +- primitives/weights/src/weight_v2.rs | 211 ++++++------------ 20 files changed, 128 insertions(+), 531 deletions(-) diff --git a/frame/alliance/src/lib.rs b/frame/alliance/src/lib.rs index 24111b44ced9e..2ef6718538122 100644 --- a/frame/alliance/src/lib.rs +++ b/frame/alliance/src/lib.rs @@ -120,7 +120,7 @@ use frame_support::{ ChangeMembers, Currency, Get, InitializeMembers, IsSubType, OnUnbalanced, ReservableCurrency, }, - weights::{OldWeight, Weight}, + weights::Weight, }; use pallet_identity::IdentityField; @@ -620,22 +620,25 @@ pub mod pallet { .max(T::WeightInfo::close_early_disapproved(x, y, p2)) .max(T::WeightInfo::close_approved(b, x, y, p2)) .max(T::WeightInfo::close_disapproved(x, y, p2)) - .saturating_add(p1.into()) + .saturating_add(p1) })] - #[allow(deprecated)] - #[deprecated(note = "1D weight is used in this extrinsic, please migrate to use `close`")] - pub fn close_old_weight( + pub fn close( origin: OriginFor, proposal_hash: T::Hash, #[pallet::compact] index: ProposalIndex, - #[pallet::compact] proposal_weight_bound: OldWeight, + #[pallet::compact] proposal_weight_bound: Weight, #[pallet::compact] length_bound: u32, ) -> DispatchResultWithPostInfo { - let proposal_weight_bound: Weight = proposal_weight_bound.into(); let who = ensure_signed(origin)?; ensure!(Self::has_voting_rights(&who), Error::::NoVotingRights); - Self::do_close(proposal_hash, index, proposal_weight_bound, length_bound) + let info = T::ProposalProvider::close_proposal( + proposal_hash, + index, + proposal_weight_bound, + length_bound, + )?; + Ok(info.into()) } /// Initialize the Alliance, onboard founders, fellows, and allies. @@ -982,34 +985,6 @@ pub mod pallet { Self::deposit_event(Event::UnscrupulousItemRemoved { items }); Ok(()) } - - /// Close a vote that is either approved, disapproved, or whose voting period has ended. - /// - /// Requires the sender to be a founder or fellow. - #[pallet::weight({ - let b = *length_bound; - let x = T::MaxFounders::get(); - let y = T::MaxFellows::get(); - let p1 = *proposal_weight_bound; - let p2 = T::MaxProposals::get(); - T::WeightInfo::close_early_approved(b, x, y, p2) - .max(T::WeightInfo::close_early_disapproved(x, y, p2)) - .max(T::WeightInfo::close_approved(b, x, y, p2)) - .max(T::WeightInfo::close_disapproved(x, y, p2)) - .saturating_add(p1) - })] - pub fn close( - origin: OriginFor, - proposal_hash: T::Hash, - #[pallet::compact] index: ProposalIndex, - proposal_weight_bound: Weight, - #[pallet::compact] length_bound: u32, - ) -> DispatchResultWithPostInfo { - let who = ensure_signed(origin)?; - ensure!(Self::has_voting_rights(&who), Error::::NoVotingRights); - - Self::do_close(proposal_hash, index, proposal_weight_bound, length_bound) - } } } @@ -1222,19 +1197,4 @@ impl, I: 'static> Pallet { } res } - - fn do_close( - proposal_hash: T::Hash, - index: ProposalIndex, - proposal_weight_bound: Weight, - length_bound: u32, - ) -> DispatchResultWithPostInfo { - let info = T::ProposalProvider::close_proposal( - proposal_hash, - index, - proposal_weight_bound, - length_bound, - )?; - Ok(info.into()) - } } diff --git a/frame/babe/src/tests.rs b/frame/babe/src/tests.rs index d4132e6378540..8d2a9b326cd0f 100644 --- a/frame/babe/src/tests.rs +++ b/frame/babe/src/tests.rs @@ -852,8 +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. - // TODO: account for proof size weight - assert!(info.weight.ref_time() > 0); + assert!(info.weight.all_gt(Weight::zero())); assert_eq!(info.pays_fee, Pays::Yes); // report the equivocation. diff --git a/frame/collective/src/lib.rs b/frame/collective/src/lib.rs index 06d5b1fab78e7..ae68ae2fe3e16 100644 --- a/frame/collective/src/lib.rs +++ b/frame/collective/src/lib.rs @@ -57,7 +57,7 @@ use frame_support::{ traits::{ Backing, ChangeMembers, EnsureOrigin, Get, GetBacking, InitializeMembers, StorageVersion, }, - weights::{OldWeight, Weight}, + weights::Weight, }; #[cfg(test)] @@ -620,20 +620,17 @@ pub mod pallet { .max(T::WeightInfo::close_early_disapproved(m, p2)) .max(T::WeightInfo::close_approved(b, m, p2)) .max(T::WeightInfo::close_disapproved(m, p2)) - .saturating_add(p1.into()) + .saturating_add(p1) }, DispatchClass::Operational ))] - #[allow(deprecated)] - #[deprecated(note = "1D weight is used in this extrinsic, please migrate to `close`")] - pub fn close_old_weight( + pub fn close( origin: OriginFor, proposal_hash: T::Hash, #[pallet::compact] index: ProposalIndex, - #[pallet::compact] proposal_weight_bound: OldWeight, + #[pallet::compact] proposal_weight_bound: Weight, #[pallet::compact] length_bound: u32, ) -> DispatchResultWithPostInfo { - let proposal_weight_bound: Weight = proposal_weight_bound.into(); let _ = ensure_signed(origin)?; Self::do_close(proposal_hash, index, proposal_weight_bound, length_bound) @@ -662,64 +659,6 @@ pub mod pallet { let proposal_count = Self::do_disapprove_proposal(proposal_hash); Ok(Some(T::WeightInfo::disapprove_proposal(proposal_count)).into()) } - - /// Close a vote that is either approved, disapproved or whose voting period has ended. - /// - /// May be called by any signed account in order to finish voting and close the proposal. - /// - /// If called before the end of the voting period it will only close the vote if it is - /// has enough votes to be approved or disapproved. - /// - /// If called after the end of the voting period abstentions are counted as rejections - /// unless there is a prime member set and the prime member cast an approval. - /// - /// If the close operation completes successfully with disapproval, the transaction fee will - /// be waived. Otherwise execution of the approved operation will be charged to the caller. - /// - /// + `proposal_weight_bound`: The maximum amount of weight consumed by executing the closed - /// proposal. - /// + `length_bound`: The upper bound for the length of the proposal in storage. Checked via - /// `storage::read` so it is `size_of::() == 4` larger than the pure length. - /// - /// # - /// ## Weight - /// - `O(B + M + P1 + P2)` where: - /// - `B` is `proposal` size in bytes (length-fee-bounded) - /// - `M` is members-count (code- and governance-bounded) - /// - `P1` is the complexity of `proposal` preimage. - /// - `P2` is proposal-count (code-bounded) - /// - DB: - /// - 2 storage reads (`Members`: codec `O(M)`, `Prime`: codec `O(1)`) - /// - 3 mutations (`Voting`: codec `O(M)`, `ProposalOf`: codec `O(B)`, `Proposals`: codec - /// `O(P2)`) - /// - any mutations done while executing `proposal` (`P1`) - /// - up to 3 events - /// # - #[pallet::weight(( - { - let b = *length_bound; - let m = T::MaxMembers::get(); - let p1 = *proposal_weight_bound; - let p2 = T::MaxProposals::get(); - T::WeightInfo::close_early_approved(b, m, p2) - .max(T::WeightInfo::close_early_disapproved(m, p2)) - .max(T::WeightInfo::close_approved(b, m, p2)) - .max(T::WeightInfo::close_disapproved(m, p2)) - .saturating_add(p1) - }, - DispatchClass::Operational - ))] - pub fn close( - origin: OriginFor, - proposal_hash: T::Hash, - #[pallet::compact] index: ProposalIndex, - proposal_weight_bound: Weight, - #[pallet::compact] length_bound: u32, - ) -> DispatchResultWithPostInfo { - let _ = ensure_signed(origin)?; - - Self::do_close(proposal_hash, index, proposal_weight_bound, length_bound) - } } } diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index f9a1c8decf042..fc44e4507ca00 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -113,7 +113,7 @@ use frame_support::{ tokens::fungible::Inspect, ConstU32, Contains, Currency, Get, Randomness, ReservableCurrency, Time, }, - weights::{OldWeight, Weight}, + weights::Weight, BoundedVec, WeakBoundedVec, }; use frame_system::{limits::BlockWeights, Pallet as System}; @@ -429,18 +429,15 @@ pub mod pallet { /// * If the account is a regular account, any value will be transferred. /// * If no account exists and the call value is not less than `existential_deposit`, /// a regular account will be created and any value will be transferred. - #[pallet::weight(T::WeightInfo::call().saturating_add((*gas_limit).into()))] - #[allow(deprecated)] - #[deprecated(note = "1D weight is used in this extrinsic, please migrate to `call`")] - pub fn call_old_weight( + #[pallet::weight(T::WeightInfo::call().saturating_add(*gas_limit))] + pub fn call( origin: OriginFor, dest: AccountIdLookupOf, #[pallet::compact] value: BalanceOf, - #[pallet::compact] gas_limit: OldWeight, + #[pallet::compact] gas_limit: Weight, storage_deposit_limit: Option< as codec::HasCompact>::Type>, data: Vec, ) -> DispatchResultWithPostInfo { - let gas_limit: Weight = gas_limit.into(); let origin = ensure_signed(origin)?; let dest = T::Lookup::lookup(dest)?; let mut output = Self::internal_call( @@ -488,22 +485,17 @@ pub mod pallet { /// - The `deploy` function is executed in the context of the newly-created account. #[pallet::weight( T::WeightInfo::instantiate_with_code(code.len() as u32, salt.len() as u32) - .saturating_add((*gas_limit).into()) - )] - #[allow(deprecated)] - #[deprecated( - note = "1D weight is used in this extrinsic, please migrate to `instantiate_with_code`" + .saturating_add(*gas_limit) )] - pub fn instantiate_with_code_old_weight( + pub fn instantiate_with_code( origin: OriginFor, #[pallet::compact] value: BalanceOf, - #[pallet::compact] gas_limit: OldWeight, + #[pallet::compact] gas_limit: Weight, storage_deposit_limit: Option< as codec::HasCompact>::Type>, code: Vec, data: Vec, salt: Vec, ) -> DispatchResultWithPostInfo { - let gas_limit: Weight = gas_limit.into(); let origin = ensure_signed(origin)?; let code_len = code.len() as u32; let salt_len = salt.len() as u32; @@ -534,20 +526,17 @@ pub mod pallet { /// code deployment step. Instead, the `code_hash` of an on-chain deployed wasm binary /// must be supplied. #[pallet::weight( - T::WeightInfo::instantiate(salt.len() as u32).saturating_add((*gas_limit).into()) + T::WeightInfo::instantiate(salt.len() as u32).saturating_add(*gas_limit) )] - #[allow(deprecated)] - #[deprecated(note = "1D weight is used in this extrinsic, please migrate to `instantiate`")] - pub fn instantiate_old_weight( + pub fn instantiate( origin: OriginFor, #[pallet::compact] value: BalanceOf, - #[pallet::compact] gas_limit: OldWeight, + #[pallet::compact] gas_limit: Weight, storage_deposit_limit: Option< as codec::HasCompact>::Type>, code_hash: CodeHash, data: Vec, salt: Vec, ) -> DispatchResultWithPostInfo { - let gas_limit: Weight = gas_limit.into(); let origin = ensure_signed(origin)?; let salt_len = salt.len() as u32; let mut output = Self::internal_instantiate( @@ -650,154 +639,6 @@ pub mod pallet { Ok(()) }) } - - /// Makes a call to an account, optionally transferring some balance. - /// - /// # Parameters - /// - /// * `dest`: Address of the contract to call. - /// * `value`: The balance to transfer from the `origin` to `dest`. - /// * `gas_limit`: The gas limit enforced when executing the constructor. - /// * `storage_deposit_limit`: The maximum amount of balance that can be charged from the - /// caller to pay for the storage consumed. - /// * `data`: The input data to pass to the contract. - /// - /// * If the account is a smart-contract account, the associated code will be - /// executed and any value will be transferred. - /// * If the account is a regular account, any value will be transferred. - /// * If no account exists and the call value is not less than `existential_deposit`, - /// a regular account will be created and any value will be transferred. - #[pallet::weight(T::WeightInfo::call().saturating_add(*gas_limit))] - pub fn call( - origin: OriginFor, - dest: AccountIdLookupOf, - #[pallet::compact] value: BalanceOf, - gas_limit: Weight, - storage_deposit_limit: Option< as codec::HasCompact>::Type>, - data: Vec, - ) -> DispatchResultWithPostInfo { - let gas_limit: Weight = gas_limit.into(); - let origin = ensure_signed(origin)?; - let dest = T::Lookup::lookup(dest)?; - let mut output = Self::internal_call( - origin, - dest, - value, - gas_limit, - storage_deposit_limit.map(Into::into), - data, - None, - ); - if let Ok(retval) = &output.result { - if retval.did_revert() { - output.result = Err(>::ContractReverted.into()); - } - } - output.gas_meter.into_dispatch_result(output.result, T::WeightInfo::call()) - } - - /// Instantiates a new contract from the supplied `code` optionally transferring - /// some balance. - /// - /// This dispatchable has the same effect as calling [`Self::upload_code`] + - /// [`Self::instantiate`]. Bundling them together provides efficiency gains. Please - /// also check the documentation of [`Self::upload_code`]. - /// - /// # Parameters - /// - /// * `value`: The balance to transfer from the `origin` to the newly created contract. - /// * `gas_limit`: The gas limit enforced when executing the constructor. - /// * `storage_deposit_limit`: The maximum amount of balance that can be charged/reserved - /// from the caller to pay for the storage consumed. - /// * `code`: The contract code to deploy in raw bytes. - /// * `data`: The input data to pass to the contract constructor. - /// * `salt`: Used for the address derivation. See [`Pallet::contract_address`]. - /// - /// Instantiation is executed as follows: - /// - /// - The supplied `code` is instrumented, deployed, and a `code_hash` is created for that - /// code. - /// - If the `code_hash` already exists on the chain the underlying `code` will be shared. - /// - The destination address is computed based on the sender, code_hash and the salt. - /// - The smart-contract account is created at the computed address. - /// - The `value` is transferred to the new account. - /// - The `deploy` function is executed in the context of the newly-created account. - #[pallet::weight( - T::WeightInfo::instantiate_with_code(code.len() as u32, salt.len() as u32) - .saturating_add(*gas_limit) - )] - pub fn instantiate_with_code( - origin: OriginFor, - #[pallet::compact] value: BalanceOf, - gas_limit: Weight, - storage_deposit_limit: Option< as codec::HasCompact>::Type>, - code: Vec, - data: Vec, - salt: Vec, - ) -> DispatchResultWithPostInfo { - let origin = ensure_signed(origin)?; - let code_len = code.len() as u32; - let salt_len = salt.len() as u32; - let mut output = Self::internal_instantiate( - origin, - value, - gas_limit, - storage_deposit_limit.map(Into::into), - Code::Upload(Bytes(code)), - data, - salt, - None, - ); - if let Ok(retval) = &output.result { - if retval.1.did_revert() { - output.result = Err(>::ContractReverted.into()); - } - } - output.gas_meter.into_dispatch_result( - output.result.map(|(_address, result)| result), - T::WeightInfo::instantiate_with_code(code_len, salt_len), - ) - } - - /// Instantiates a contract from a previously deployed wasm binary. - /// - /// This function is identical to [`Self::instantiate_with_code`] but without the - /// code deployment step. Instead, the `code_hash` of an on-chain deployed wasm binary - /// must be supplied. - #[pallet::weight( - T::WeightInfo::instantiate(salt.len() as u32).saturating_add(*gas_limit) - )] - pub fn instantiate( - origin: OriginFor, - #[pallet::compact] value: BalanceOf, - gas_limit: Weight, - storage_deposit_limit: Option< as codec::HasCompact>::Type>, - code_hash: CodeHash, - data: Vec, - salt: Vec, - ) -> DispatchResultWithPostInfo { - let origin = ensure_signed(origin)?; - let salt_len = salt.len() as u32; - let mut output = Self::internal_instantiate( - origin, - value, - gas_limit, - storage_deposit_limit.map(Into::into), - Code::Existing(code_hash), - data, - salt, - None, - ); - if let Ok(retval) = &output.result { - if retval.1.did_revert() { - output.result = Err(>::ContractReverted.into()); - } - } - output.gas_meter.into_dispatch_result( - output.result.map(|(_address, output)| output), - T::WeightInfo::instantiate(salt_len), - ) - } } #[pallet::event] diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index d8b4cd245356e..126a37e9401ec 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -274,11 +274,7 @@ mod tests { BalanceOf, CodeHash, Error, Pallet as Contracts, }; use assert_matches::assert_matches; - use frame_support::{ - assert_ok, - dispatch::DispatchResultWithPostInfo, - weights::{OldWeight, Weight}, - }; + use frame_support::{assert_ok, dispatch::DispatchResultWithPostInfo, weights::Weight}; use pallet_contracts_primitives::{ExecReturnValue, ReturnFlags}; use pretty_assertions::assert_eq; use sp_core::{Bytes, H256}; @@ -1549,11 +1545,10 @@ mod tests { let output = execute(CODE_GAS_LEFT, vec![], &mut ext).unwrap(); - let OldWeight(gas_left) = OldWeight::decode(&mut &*output.data).unwrap(); + let gas_left = Weight::decode(&mut &*output.data).unwrap(); let actual_left = ext.gas_meter.gas_left(); - // TODO: account for proof size weight - assert!(gas_left < gas_limit.ref_time(), "gas_left must be less than initial"); - assert!(gas_left > actual_left.ref_time(), "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#" @@ -1951,7 +1946,7 @@ mod tests { )] ); - assert!(mock_ext.gas_meter.gas_left().ref_time() > 0); + assert!(mock_ext.gas_meter.gas_left().all_gt(Weight::zero())); } const CODE_DEPOSIT_EVENT_MAX_TOPICS: &str = r#" diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index bba8139f38f44..05353e5a3ac61 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -1008,10 +1008,8 @@ pub mod pallet { // unlikely to ever return an error: if phase is signed, snapshot will exist. let size = Self::snapshot_metadata().ok_or(Error::::MissingSnapshotMetadata)?; - // TODO: account for proof size weight ensure!( - Self::solution_weight_of(&raw_solution, size).ref_time() < - T::SignedMaxWeight::get().ref_time(), + Self::solution_weight_of(&raw_solution, size).all_lt(T::SignedMaxWeight::get()), Error::::SignedTooMuchWeight, ); @@ -2338,9 +2336,8 @@ mod tests { }; let mut active = 1; - // TODO: account for proof size weight - while weight_with(active).ref_time() <= - ::BlockWeights::get().max_block.ref_time() || + while weight_with(active) + .all_lte(::BlockWeights::get().max_block) || active == all_voters { active += 1; diff --git a/frame/election-provider-multi-phase/src/unsigned.rs b/frame/election-provider-multi-phase/src/unsigned.rs index 281ac37421174..833f80c90d13e 100644 --- a/frame/election-provider-multi-phase/src/unsigned.rs +++ b/frame/election-provider-multi-phase/src/unsigned.rs @@ -638,8 +638,7 @@ impl Miner { }; let next_voters = |current_weight: Weight, voters: u32, step: u32| -> Result { - // TODO: account for proof size weight - if current_weight.ref_time() < max_weight.ref_time() { + 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), @@ -674,8 +673,7 @@ impl Miner { // 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. - // TODO: account for proof size weight - while voters < max_voters && weight_with(voters + 1).ref_time() < max_weight.ref_time() { + while voters < max_voters && weight_with(voters + 1).all_lt(max_weight) { voters += 1; } while voters.checked_sub(1).is_some() && weight_with(voters).any_gt(max_weight) { @@ -683,9 +681,8 @@ impl Miner { } let final_decision = voters.min(size.voters); - // TODO: account for proof size weight debug_assert!( - weight_with(final_decision).ref_time() <= max_weight.ref_time(), + weight_with(final_decision).all_lte(max_weight), "weight_with({}) <= {}", final_decision, max_weight, diff --git a/frame/examples/basic/src/tests.rs b/frame/examples/basic/src/tests.rs index 97fbddfbc41e0..db4787eaa0faa 100644 --- a/frame/examples/basic/src/tests.rs +++ b/frame/examples/basic/src/tests.rs @@ -191,13 +191,11 @@ fn weights_work() { let default_call = pallet_example_basic::Call::::accumulate_dummy { increase_by: 10 }; let info1 = default_call.get_dispatch_info(); // aka. `let info = as GetDispatchInfo>::get_dispatch_info(&default_call);` - // TODO: account for proof size weight - assert!(info1.weight.ref_time() > 0); + 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::::set_dummy { new_value: 20 }; let info2 = custom_call.get_dispatch_info(); - // TODO: account for proof size weight - assert!(info1.weight.ref_time() > info2.weight.ref_time()); + assert!(info1.weight.all_gt(info2.weight)); } diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index 014c7a2bc02a6..a41c82da5757c 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -459,8 +459,7 @@ where let max_weight = >::get().max_block; let remaining_weight = max_weight.saturating_sub(weight.total()); - // TODO: account for proof size weight - if remaining_weight.ref_time() > 0 { + if remaining_weight.all_gt(Weight::zero()) { let used_weight = >::on_idle( block_number, remaining_weight, @@ -939,13 +938,13 @@ mod tests { block_import_works_inner( new_test_ext_v0(1), array_bytes::hex_n_into_unchecked( - "0d786e24c1f9e6ce237806a22c005bbbc7dee4edd6692b6c5442843d164392de", + "1039e1a4bd0cf5deefe65f313577e70169c41c7773d6acf31ca8d671397559f5", ), ); block_import_works_inner( new_test_ext(1), array_bytes::hex_n_into_unchecked( - "348485a4ab856467b440167e45f99b491385e8528e09b0e51f85f814a3021c93", + "75e7d8f360d375bbe91bcf8019c01ab6362448b4a89e3b329717eb9d910340e5", ), ); } diff --git a/frame/grandpa/src/tests.rs b/frame/grandpa/src/tests.rs index 5d2ebdf29cb6b..775eda58c03e0 100644 --- a/frame/grandpa/src/tests.rs +++ b/frame/grandpa/src/tests.rs @@ -856,8 +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. - // TODO: account for proof size weight - assert!(info.weight.ref_time() > 0); + assert!(info.weight.all_gt(Weight::zero())); assert_eq!(info.pays_fee, Pays::Yes); // report the equivocation. diff --git a/frame/support/procedural/src/pallet/expand/call.rs b/frame/support/procedural/src/pallet/expand/call.rs index 39d16109aa8fa..18d5adee63ad6 100644 --- a/frame/support/procedural/src/pallet/expand/call.rs +++ b/frame/support/procedural/src/pallet/expand/call.rs @@ -16,7 +16,6 @@ // limitations under the License. use crate::{pallet::Def, COUNTER}; -use quote::ToTokens; use syn::spanned::Spanned; /// @@ -159,24 +158,6 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream { }); } - // Extracts #[allow] attributes, necessary so that we don't run into compiler warnings - let maybe_allow_attrs = methods - .iter() - .map(|method| { - method - .attrs - .iter() - .find(|attr| { - if let Ok(syn::Meta::List(syn::MetaList { path, .. })) = attr.parse_meta() { - path.segments.last().map(|seg| seg.ident == "allow").unwrap_or(false) - } else { - false - } - }) - .map_or(proc_macro2::TokenStream::new(), |attr| attr.to_token_stream()) - }) - .collect::>(); - quote::quote_spanned!(span => #[doc(hidden)] pub mod __substrate_call_check { @@ -308,7 +289,6 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream { #frame_support::sp_tracing::enter_span!( #frame_support::sp_tracing::trace_span!(stringify!(#fn_name)) ); - #maybe_allow_attrs <#pallet_ident<#type_use_gen>>::#fn_name(origin, #( #args_name, )* ) .map(Into::into).map_err(Into::into) }, diff --git a/frame/support/procedural/src/pallet/parse/call.rs b/frame/support/procedural/src/pallet/parse/call.rs index f7b2c9544d831..336e08c3d39b7 100644 --- a/frame/support/procedural/src/pallet/parse/call.rs +++ b/frame/support/procedural/src/pallet/parse/call.rs @@ -61,8 +61,6 @@ pub struct CallVariantDef { pub call_index: u8, /// Docs, used for metadata. pub docs: Vec, - /// Attributes annotated at the top of the dispatchable function. - pub attrs: Vec, } /// Attributes for functions in call impl block. @@ -289,7 +287,6 @@ impl CallDef { call_index: final_index, args, docs, - attrs: method.attrs.clone(), }); } else { let msg = "Invalid pallet::call, only method accepted"; diff --git a/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen.stderr b/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen.stderr index b0716d569409c..5d159ec961c7f 100644 --- a/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen.stderr +++ b/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen.stderr @@ -28,7 +28,7 @@ error[E0277]: the trait bound `Bar: EncodeLike` is not satisfied <&[(T,)] as EncodeLike>> <&[(T,)] as EncodeLike>> <&[T] as EncodeLike>> - and 279 others + and 278 others = note: required because of the requirements on the impl of `FullEncode` for `Bar` = note: required because of the requirements on the impl of `FullCodec` for `Bar` = note: required because of the requirements on the impl of `PartialStorageInfoTrait` for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` @@ -69,7 +69,7 @@ error[E0277]: the trait bound `Bar: TypeInfo` is not satisfied (A, B, C, D) (A, B, C, D, E) (A, B, C, D, E, F) - and 160 others + and 159 others = note: required because of the requirements on the impl of `StaticTypeInfo` for `Bar` = note: required because of the requirements on the impl of `StorageEntryMetadataBuilder` for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` @@ -103,7 +103,7 @@ error[E0277]: the trait bound `Bar: EncodeLike` is not satisfied <&[(T,)] as EncodeLike>> <&[(T,)] as EncodeLike>> <&[T] as EncodeLike>> - and 279 others + and 278 others = note: required because of the requirements on the impl of `FullEncode` for `Bar` = note: required because of the requirements on the impl of `FullCodec` for `Bar` = note: required because of the requirements on the impl of `StorageEntryMetadataBuilder` for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` diff --git a/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen_unnamed.stderr b/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen_unnamed.stderr index 926dc92530659..4671855431b27 100644 --- a/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen_unnamed.stderr +++ b/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen_unnamed.stderr @@ -28,7 +28,7 @@ error[E0277]: the trait bound `Bar: EncodeLike` is not satisfied <&[(T,)] as EncodeLike>> <&[(T,)] as EncodeLike>> <&[T] as EncodeLike>> - and 279 others + and 278 others = note: required because of the requirements on the impl of `FullEncode` for `Bar` = note: required because of the requirements on the impl of `FullCodec` for `Bar` = note: required because of the requirements on the impl of `PartialStorageInfoTrait` for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` @@ -69,7 +69,7 @@ error[E0277]: the trait bound `Bar: TypeInfo` is not satisfied (A, B, C, D) (A, B, C, D, E) (A, B, C, D, E, F) - and 160 others + and 159 others = note: required because of the requirements on the impl of `StaticTypeInfo` for `Bar` = note: required because of the requirements on the impl of `StorageEntryMetadataBuilder` for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` @@ -103,7 +103,7 @@ error[E0277]: the trait bound `Bar: EncodeLike` is not satisfied <&[(T,)] as EncodeLike>> <&[(T,)] as EncodeLike>> <&[T] as EncodeLike>> - and 279 others + and 278 others = note: required because of the requirements on the impl of `FullEncode` for `Bar` = note: required because of the requirements on the impl of `FullCodec` for `Bar` = note: required because of the requirements on the impl of `StorageEntryMetadataBuilder` for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` diff --git a/frame/support/test/tests/pallet_ui/storage_info_unsatisfied.stderr b/frame/support/test/tests/pallet_ui/storage_info_unsatisfied.stderr index 563190a06f76f..d9cd20711403d 100644 --- a/frame/support/test/tests/pallet_ui/storage_info_unsatisfied.stderr +++ b/frame/support/test/tests/pallet_ui/storage_info_unsatisfied.stderr @@ -13,5 +13,5 @@ error[E0277]: the trait bound `Bar: MaxEncodedLen` is not satisfied (TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4, TupleElement5) (TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4, TupleElement5, TupleElement6) (TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4, TupleElement5, TupleElement6, TupleElement7) - and 77 others + and 76 others = note: required because of the requirements on the impl of `StorageInfoTrait` for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` diff --git a/frame/support/test/tests/pallet_ui/storage_info_unsatisfied_nmap.stderr b/frame/support/test/tests/pallet_ui/storage_info_unsatisfied_nmap.stderr index c10005223b674..9a4e8d740cb2c 100644 --- a/frame/support/test/tests/pallet_ui/storage_info_unsatisfied_nmap.stderr +++ b/frame/support/test/tests/pallet_ui/storage_info_unsatisfied_nmap.stderr @@ -13,6 +13,6 @@ error[E0277]: the trait bound `Bar: MaxEncodedLen` is not satisfied (TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4, TupleElement5) (TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4, TupleElement5, TupleElement6) (TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4, TupleElement5, TupleElement6, TupleElement7) - and 77 others + and 76 others = note: required because of the requirements on the impl of `KeyGeneratorMaxEncodedLen` for `Key` = note: required because of the requirements on the impl of `StorageInfoTrait` for `frame_support::pallet_prelude::StorageNMap<_GeneratedPrefixForStorageFoo, Key, u32>` diff --git a/frame/system/src/limits.rs b/frame/system/src/limits.rs index cfc1d261baa01..e182eb626424d 100644 --- a/frame/system/src/limits.rs +++ b/frame/system/src/limits.rs @@ -207,7 +207,7 @@ pub struct BlockWeights { impl Default for BlockWeights { fn default() -> Self { - Self::with_sensible_defaults(1u64 * constants::WEIGHT_PER_SECOND, DEFAULT_NORMAL_RATIO) + Self::with_sensible_defaults(1u32 * constants::WEIGHT_PER_SECOND, DEFAULT_NORMAL_RATIO) } } @@ -224,7 +224,6 @@ 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); @@ -233,16 +232,18 @@ impl BlockWeights { // Make sure that if total is set it's greater than base_block && // base_for_class error_assert!( - (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, + (max_for_class.all_gt(self.base_block) && max_for_class.all_gt(base_for_class)) + || max_for_class == Weight::zero(), &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()).ref_time() <= - max_for_class.saturating_sub(base_for_class).ref_time(), + weights + .max_extrinsic + .unwrap_or(Weight::zero()) + .all_lte(max_for_class.saturating_sub(base_for_class)), &mut error, "[{:?}] {:?} (max_extrinsic) can't be greater than {:?} (max for class)", class, @@ -251,14 +252,14 @@ impl BlockWeights { ); // Max extrinsic should not be 0 error_assert!( - weights.max_extrinsic.unwrap_or_else(Weight::max_value).ref_time() > 0, + weights.max_extrinsic.unwrap_or_else(Weight::max_value).all_gt(Weight::zero()), &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.ref_time() > base_for_class.ref_time() || reserved.ref_time() == 0, + reserved.all_gt(base_for_class) || reserved == Weight::zero(), &mut error, "[{:?}] {:?} (reserved) has to be greater than {:?} (base extrinsic) if set", class, @@ -267,7 +268,7 @@ impl BlockWeights { ); // Make sure max block is greater than max_total if it's set. error_assert!( - self.max_block.ref_time() >= weights.max_total.unwrap_or(Weight::zero()).ref_time(), + self.max_block.all_gte(weights.max_total.unwrap_or(Weight::zero())), &mut error, "[{:?}] {:?} (max block) has to be greater than {:?} (max for class)", class, @@ -276,7 +277,7 @@ impl BlockWeights { ); // Make sure we can fit at least one extrinsic. error_assert!( - self.max_block.ref_time() > (base_for_class + self.base_block).ref_time(), + self.max_block.all_gt(base_for_class + self.base_block), &mut error, "[{:?}] {:?} (max block) must fit at least one extrinsic {:?} (base weight)", class, diff --git a/frame/transaction-payment/src/types.rs b/frame/transaction-payment/src/types.rs index fff41ef6937f5..1f41ba7b0b72e 100644 --- a/frame/transaction-payment/src/types.rs +++ b/frame/transaction-payment/src/types.rs @@ -140,8 +140,7 @@ mod tests { partial_fee: 1_000_000_u64, }; - let json_str = - r#"{"weight":{"ref_time":5,"proof_size":0},"class":"normal","partialFee":"1000000"}"#; + let json_str = r#"{"weight":{"ref_time":5},"class":"normal","partialFee":"1000000"}"#; assert_eq!(serde_json::to_string(&info).unwrap(), json_str); assert_eq!(serde_json::from_str::>(json_str).unwrap(), info); @@ -158,7 +157,7 @@ mod tests { partial_fee: u128::max_value(), }; - let json_str = r#"{"weight":{"ref_time":5,"proof_size":0},"class":"normal","partialFee":"340282366920938463463374607431768211455"}"#; + let json_str = r#"{"weight":{"ref_time":5},"class":"normal","partialFee":"340282366920938463463374607431768211455"}"#; assert_eq!(serde_json::to_string(&info).unwrap(), json_str); assert_eq!(serde_json::from_str::>(json_str).unwrap(), info); diff --git a/primitives/weights/src/lib.rs b/primitives/weights/src/lib.rs index e1ac7fcd4e892..d260f73d41268 100644 --- a/primitives/weights/src/lib.rs +++ b/primitives/weights/src/lib.rs @@ -30,7 +30,7 @@ extern crate self as sp_weights; mod weight_v2; -use codec::{CompactAs, Decode, Encode, MaxEncodedLen}; +use codec::{Decode, Encode}; use scale_info::TypeInfo; #[cfg(feature = "std")] use serde::{Deserialize, Serialize}; @@ -53,25 +53,6 @@ pub mod constants { pub const WEIGHT_PER_NANOS: Weight = Weight::from_ref_time(1_000); } -/// The old weight type. -/// -/// NOTE: This type exists purely for compatibility purposes! Use [`weight_v2::Weight`] in all other -/// cases. -#[derive( - Decode, - Encode, - CompactAs, - PartialEq, - Eq, - Clone, - Copy, - RuntimeDebug, - Default, - MaxEncodedLen, - TypeInfo, -)] -pub struct OldWeight(pub u64); - /// The weight of database operations that the runtime can invoke. /// /// NOTE: This is currently only measured in computational time, and will probably diff --git a/primitives/weights/src/weight_v2.rs b/primitives/weights/src/weight_v2.rs index a8eaf79a28711..af0f469ebaaeb 100644 --- a/primitives/weights/src/weight_v2.rs +++ b/primitives/weights/src/weight_v2.rs @@ -15,7 +15,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use codec::{Decode, Encode, MaxEncodedLen}; +use codec::{CompactAs, Decode, Encode, MaxEncodedLen}; use core::ops::{Add, AddAssign, Div, Mul, Sub, SubAssign}; use sp_arithmetic::traits::{Bounded, CheckedAdd, CheckedSub, Zero}; use sp_debug_derive::RuntimeDebug; @@ -23,22 +23,22 @@ use sp_debug_derive::RuntimeDebug; use super::*; #[derive( - Encode, Decode, MaxEncodedLen, TypeInfo, Eq, PartialEq, Copy, Clone, RuntimeDebug, Default, + Encode, + Decode, + MaxEncodedLen, + TypeInfo, + Eq, + PartialEq, + Copy, + Clone, + RuntimeDebug, + Default, + CompactAs, )] #[cfg_attr(feature = "std", derive(Serialize, Deserialize))] pub struct Weight { - #[codec(compact)] /// The weight of computational time used based on some reference hardware. ref_time: u64, - #[codec(compact)] - /// The weight of storage space used by proof of validity. - proof_size: u64, -} - -impl From for Weight { - fn from(old: OldWeight) -> Self { - Weight::from_ref_time(old.0) - } } impl Weight { @@ -48,118 +48,71 @@ impl Weight { self } - /// Set the storage size part of the weight. - pub const fn set_proof_size(mut self, c: u64) -> Self { - self.proof_size = c; - self - } - /// Return the reference time part of the weight. pub const fn ref_time(&self) -> u64 { self.ref_time } - /// Return the storage size part of the weight. - pub const fn proof_size(&self) -> u64 { - self.proof_size - } - - /// Return a mutable reference to the reference time part of the weight. + /// Return a mutable reference time part of the weight. pub fn ref_time_mut(&mut self) -> &mut u64 { &mut self.ref_time } - /// Return a mutable reference to the storage size part of the weight. - pub fn proof_size_mut(&mut self) -> &mut u64 { - &mut self.proof_size - } - - pub const MAX: Self = Self { ref_time: u64::MAX, proof_size: u64::MAX }; + pub const MAX: Self = Self { ref_time: u64::MAX }; /// Get the conservative min of `self` and `other` weight. pub fn min(&self, other: Self) -> Self { - Self { - ref_time: self.ref_time.min(other.ref_time), - proof_size: self.proof_size.min(other.proof_size), - } + Self { ref_time: self.ref_time.min(other.ref_time) } } /// Get the aggressive max of `self` and `other` weight. pub fn max(&self, other: Self) -> Self { - Self { - ref_time: self.ref_time.max(other.ref_time), - proof_size: self.proof_size.max(other.proof_size), - } + Self { ref_time: self.ref_time.max(other.ref_time) } } /// Try to add some `other` weight while upholding the `limit`. pub fn try_add(&self, other: &Self, limit: &Self) -> Option { let total = self.checked_add(other)?; - if total.any_gt(*limit) { + if total.ref_time > limit.ref_time { None } else { Some(total) } } - /// Construct [`Weight`] with reference time weight and 0 storage size weight. + /// Construct [`Weight`] with reference time weight. pub const fn from_ref_time(ref_time: u64) -> Self { - Self { ref_time, proof_size: 0 } - } - - /// Construct [`Weight`] with storage size weight and 0 reference time weight. - pub const fn from_proof_size(proof_size: u64) -> Self { - Self { ref_time: 0, proof_size } - } - - /// Construct [`Weight`] with weight components, namely reference time and storage size weights. - pub const fn from_components(ref_time: u64, proof_size: u64) -> Self { - Self { ref_time, proof_size } + Self { ref_time } } /// Saturating [`Weight`] addition. Computes `self + rhs`, saturating at the numeric bounds of /// all fields instead of overflowing. pub const fn saturating_add(self, rhs: Self) -> Self { - Self { - ref_time: self.ref_time.saturating_add(rhs.ref_time), - proof_size: self.proof_size.saturating_add(rhs.proof_size), - } + Self { ref_time: self.ref_time.saturating_add(rhs.ref_time) } } /// Saturating [`Weight`] subtraction. Computes `self - rhs`, saturating at the numeric bounds /// of all fields instead of overflowing. pub const fn saturating_sub(self, rhs: Self) -> Self { - Self { - ref_time: self.ref_time.saturating_sub(rhs.ref_time), - proof_size: self.proof_size.saturating_sub(rhs.proof_size), - } + Self { ref_time: self.ref_time.saturating_sub(rhs.ref_time) } } /// Saturating [`Weight`] scalar multiplication. Computes `self.field * scalar` for all fields, /// saturating at the numeric bounds of all fields instead of overflowing. pub const fn saturating_mul(self, scalar: u64) -> Self { - Self { - ref_time: self.ref_time.saturating_mul(scalar), - proof_size: self.proof_size.saturating_mul(scalar), - } + Self { ref_time: self.ref_time.saturating_mul(scalar) } } /// Saturating [`Weight`] scalar division. Computes `self.field / scalar` for all fields, /// saturating at the numeric bounds of all fields instead of overflowing. pub const fn saturating_div(self, scalar: u64) -> Self { - Self { - ref_time: self.ref_time.saturating_div(scalar), - proof_size: self.proof_size.saturating_div(scalar), - } + Self { ref_time: self.ref_time.saturating_div(scalar) } } /// Saturating [`Weight`] scalar exponentiation. Computes `self.field.pow(exp)` for all fields, /// saturating at the numeric bounds of all fields instead of overflowing. pub const fn saturating_pow(self, exp: u32) -> Self { - Self { - ref_time: self.ref_time.saturating_pow(exp), - proof_size: self.proof_size.saturating_pow(exp), - } + Self { ref_time: self.ref_time.saturating_pow(exp) } } /// Increment [`Weight`] by `amount` via saturating addition. @@ -169,144 +122,124 @@ impl Weight { /// Checked [`Weight`] addition. Computes `self + rhs`, returning `None` if overflow occurred. pub const fn checked_add(&self, rhs: &Self) -> Option { - let ref_time = match self.ref_time.checked_add(rhs.ref_time) { - Some(t) => t, - None => return None, - }; - let proof_size = match self.proof_size.checked_add(rhs.proof_size) { - Some(s) => s, - None => return None, - }; - Some(Self { ref_time, proof_size }) + match self.ref_time.checked_add(rhs.ref_time) { + Some(ref_time) => Some(Self { ref_time }), + None => None, + } } /// Checked [`Weight`] subtraction. Computes `self - rhs`, returning `None` if overflow /// occurred. pub const fn checked_sub(&self, rhs: &Self) -> Option { - let ref_time = match self.ref_time.checked_sub(rhs.ref_time) { - Some(t) => t, - None => return None, - }; - let proof_size = match self.proof_size.checked_sub(rhs.proof_size) { - Some(s) => s, - None => return None, - }; - Some(Self { ref_time, proof_size }) + match self.ref_time.checked_sub(rhs.ref_time) { + Some(ref_time) => Some(Self { ref_time }), + None => None, + } } /// Checked [`Weight`] scalar multiplication. Computes `self.field * scalar` for each field, /// returning `None` if overflow occurred. pub const fn checked_mul(self, scalar: u64) -> Option { - let ref_time = match self.ref_time.checked_mul(scalar) { - Some(t) => t, - None => return None, - }; - let proof_size = match self.proof_size.checked_mul(scalar) { - Some(s) => s, - None => return None, - }; - Some(Self { ref_time, proof_size }) + match self.ref_time.checked_mul(scalar) { + Some(ref_time) => Some(Self { ref_time }), + None => None, + } } /// Checked [`Weight`] scalar division. Computes `self.field / scalar` for each field, returning /// `None` if overflow occurred. pub const fn checked_div(self, scalar: u64) -> Option { - let ref_time = match self.ref_time.checked_div(scalar) { - Some(t) => t, - None => return None, - }; - let proof_size = match self.proof_size.checked_div(scalar) { - Some(s) => s, - None => return None, - }; - Some(Self { ref_time, proof_size }) + match self.ref_time.checked_div(scalar) { + Some(ref_time) => Some(Self { ref_time }), + None => None, + } } /// Return a [`Weight`] where all fields are zero. pub const fn zero() -> Self { - Self { ref_time: 0, proof_size: 0 } + Self { ref_time: 0 } } /// Constant version of Add with u64. /// /// Is only overflow safe when evaluated at compile-time. pub const fn add(self, scalar: u64) -> Self { - Self { ref_time: self.ref_time + scalar, proof_size: self.proof_size + scalar } + Self { ref_time: self.ref_time + scalar } } /// Constant version of Sub with u64. /// /// Is only overflow safe when evaluated at compile-time. pub const fn sub(self, scalar: u64) -> Self { - Self { ref_time: self.ref_time - scalar, proof_size: self.proof_size - scalar } + Self { ref_time: self.ref_time - scalar } } /// Constant version of Div with u64. /// /// Is only overflow safe when evaluated at compile-time. pub const fn div(self, scalar: u64) -> Self { - Self { ref_time: self.ref_time / scalar, proof_size: self.proof_size / scalar } + Self { ref_time: self.ref_time / scalar } } /// Constant version of Mul with u64. /// /// Is only overflow safe when evaluated at compile-time. pub const fn mul(self, scalar: u64) -> Self { - Self { ref_time: self.ref_time * scalar, proof_size: self.proof_size * scalar } + Self { ref_time: self.ref_time * scalar } } /// Returns true if any of `self`'s constituent weights is strictly greater than that of the /// `other`'s, otherwise returns false. pub const fn any_gt(self, other: Self) -> bool { - self.ref_time > other.ref_time || self.proof_size > other.proof_size + self.ref_time > other.ref_time } /// Returns true if all of `self`'s constituent weights is strictly greater than that of the /// `other`'s, otherwise returns false. pub const fn all_gt(self, other: Self) -> bool { - self.ref_time > other.ref_time && self.proof_size > other.proof_size + self.ref_time > other.ref_time } /// Returns true if any of `self`'s constituent weights is strictly less than that of the /// `other`'s, otherwise returns false. pub const fn any_lt(self, other: Self) -> bool { - self.ref_time < other.ref_time || self.proof_size < other.proof_size + self.ref_time < other.ref_time } /// Returns true if all of `self`'s constituent weights is strictly less than that of the /// `other`'s, otherwise returns false. pub const fn all_lt(self, other: Self) -> bool { - self.ref_time < other.ref_time && self.proof_size < other.proof_size + self.ref_time < other.ref_time } /// Returns true if any of `self`'s constituent weights is greater than or equal to that of the /// `other`'s, otherwise returns false. pub const fn any_gte(self, other: Self) -> bool { - self.ref_time >= other.ref_time || self.proof_size >= other.proof_size + self.ref_time >= other.ref_time } /// Returns true if all of `self`'s constituent weights is greater than or equal to that of the /// `other`'s, otherwise returns false. pub const fn all_gte(self, other: Self) -> bool { - self.ref_time >= other.ref_time && self.proof_size >= other.proof_size + self.ref_time >= other.ref_time } /// Returns true if any of `self`'s constituent weights is less than or equal to that of the /// `other`'s, otherwise returns false. pub const fn any_lte(self, other: Self) -> bool { - self.ref_time <= other.ref_time || self.proof_size <= other.proof_size + self.ref_time <= other.ref_time } /// Returns true if all of `self`'s constituent weights is less than or equal to that of the /// `other`'s, otherwise returns false. pub const fn all_lte(self, other: Self) -> bool { - self.ref_time <= other.ref_time && self.proof_size <= other.proof_size + self.ref_time <= other.ref_time } /// Returns true if any of `self`'s constituent weights is equal to that of the `other`'s, /// otherwise returns false. pub const fn any_eq(self, other: Self) -> bool { - self.ref_time == other.ref_time || self.proof_size == other.proof_size + self.ref_time == other.ref_time } // NOTE: `all_eq` does not exist, as it's simply the `eq` method from the `PartialEq` trait. @@ -325,20 +258,14 @@ impl Zero for Weight { impl Add for Weight { type Output = Self; fn add(self, rhs: Self) -> Self { - Self { - ref_time: self.ref_time + rhs.ref_time, - proof_size: self.proof_size + rhs.proof_size, - } + Self { ref_time: self.ref_time + rhs.ref_time } } } impl Sub for Weight { type Output = Self; fn sub(self, rhs: Self) -> Self { - Self { - ref_time: self.ref_time - rhs.ref_time, - proof_size: self.proof_size - rhs.proof_size, - } + Self { ref_time: self.ref_time - rhs.ref_time } } } @@ -348,7 +275,7 @@ where { type Output = Self; fn mul(self, b: T) -> Self { - Self { ref_time: b * self.ref_time, proof_size: b * self.proof_size } + Self { ref_time: b * self.ref_time } } } @@ -358,10 +285,7 @@ macro_rules! weight_mul_per_impl { impl Mul for $t { type Output = Weight; fn mul(self, b: Weight) -> Weight { - Weight { - ref_time: self * b.ref_time, - proof_size: self * b.proof_size, - } + Weight { ref_time: self * b.ref_time } } } )* @@ -381,10 +305,7 @@ macro_rules! weight_mul_primitive_impl { impl Mul for $t { type Output = Weight; fn mul(self, b: Weight) -> Weight { - Weight { - ref_time: u64::from(self) * b.ref_time, - proof_size: u64::from(self) * b.proof_size, - } + Weight { ref_time: u64::from(self) * b.ref_time } } } )* @@ -399,7 +320,7 @@ where { type Output = Self; fn div(self, b: T) -> Self { - Self { ref_time: self.ref_time / b, proof_size: self.proof_size / b } + Self { ref_time: self.ref_time / b } } } @@ -417,7 +338,7 @@ impl CheckedSub for Weight { impl core::fmt::Display for Weight { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - write!(f, "Weight(ref_time: {}, proof_size: {})", self.ref_time, self.proof_size) + write!(f, "Weight(ref_time: {})", self.ref_time) } } @@ -432,18 +353,12 @@ impl Bounded for Weight { impl AddAssign for Weight { fn add_assign(&mut self, other: Self) { - *self = Self { - ref_time: self.ref_time + other.ref_time, - proof_size: self.proof_size + other.proof_size, - }; + *self = Self { ref_time: self.ref_time + other.ref_time }; } } impl SubAssign for Weight { fn sub_assign(&mut self, other: Self) { - *self = Self { - ref_time: self.ref_time - other.ref_time, - proof_size: self.proof_size - other.proof_size, - }; + *self = Self { ref_time: self.ref_time - other.ref_time }; } } From cd6397bab6c6501f54bc5f768bbceaef58a6ec8f Mon Sep 17 00:00:00 2001 From: Mara Broda Date: Tue, 4 Oct 2022 17:56:57 +0200 Subject: [PATCH 2/2] Revert "Properly set the max proof size weight on defaults and tests (#12383)" This reverts commit 61b9a4d1a8a9bf39c1d89a8dd02f82785c10860c. --- .../election-provider-multi-phase/src/lib.rs | 9 +++++--- .../src/unsigned.rs | 9 +++++--- frame/executive/src/lib.rs | 3 ++- frame/grandpa/src/tests.rs | 3 ++- frame/system/src/limits.rs | 21 +++++++++---------- 5 files changed, 26 insertions(+), 19 deletions(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 05353e5a3ac61..bba8139f38f44 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -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::::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::::SignedTooMuchWeight, ); @@ -2336,8 +2338,9 @@ mod tests { }; let mut active = 1; - while weight_with(active) - .all_lte(::BlockWeights::get().max_block) || + // TODO: account for proof size weight + while weight_with(active).ref_time() <= + ::BlockWeights::get().max_block.ref_time() || active == all_voters { active += 1; diff --git a/frame/election-provider-multi-phase/src/unsigned.rs b/frame/election-provider-multi-phase/src/unsigned.rs index 833f80c90d13e..281ac37421174 100644 --- a/frame/election-provider-multi-phase/src/unsigned.rs +++ b/frame/election-provider-multi-phase/src/unsigned.rs @@ -638,7 +638,8 @@ impl Miner { }; let next_voters = |current_weight: Weight, voters: u32, step: u32| -> Result { - 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), @@ -673,7 +674,8 @@ impl Miner { // 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) { @@ -681,8 +683,9 @@ impl Miner { } 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, diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index a41c82da5757c..dcdd6893fee1e 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -459,7 +459,8 @@ where let max_weight = >::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 = >::on_idle( block_number, remaining_weight, diff --git a/frame/grandpa/src/tests.rs b/frame/grandpa/src/tests.rs index 775eda58c03e0..5d2ebdf29cb6b 100644 --- a/frame/grandpa/src/tests.rs +++ b/frame/grandpa/src/tests.rs @@ -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. diff --git a/frame/system/src/limits.rs b/frame/system/src/limits.rs index e182eb626424d..cfc1d261baa01 100644 --- a/frame/system/src/limits.rs +++ b/frame/system/src/limits.rs @@ -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) } } @@ -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); @@ -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, @@ -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, @@ -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, @@ -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,