From d9a334584da8f5b6b3d5d63a0e60fdc38b5362b1 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Fri, 21 Oct 2022 13:43:07 +0300 Subject: [PATCH] weights v1.5: iteration 2 (#1613) --- bin/runtime-common/src/messages.rs | 15 ++++--- bin/runtime-common/src/messages_api.rs | 2 +- .../src/messages_benchmarking.rs | 2 +- bin/runtime-common/src/messages_extension.rs | 2 +- modules/grandpa/src/lib.rs | 4 +- modules/messages/src/lib.rs | 20 ++++----- modules/messages/src/weights_ext.rs | 42 +++++++++---------- modules/parachains/src/lib.rs | 2 +- primitives/messages/src/source_chain.rs | 6 +-- primitives/messages/src/target_chain.rs | 2 +- primitives/runtime/src/lib.rs | 21 +++++++++- .../lib-substrate-relay/src/messages_lane.rs | 17 ++++---- .../src/messages_source.rs | 4 +- relays/messages/src/message_race_delivery.rs | 2 +- .../relay_strategy/enforcement_strategy.rs | 10 ++--- 15 files changed, 81 insertions(+), 70 deletions(-) diff --git a/bin/runtime-common/src/messages.rs b/bin/runtime-common/src/messages.rs index 880bc6bc394..14d505f51b9 100644 --- a/bin/runtime-common/src/messages.rs +++ b/bin/runtime-common/src/messages.rs @@ -407,7 +407,7 @@ pub mod source { let delivery_transaction = BridgedChain::::estimate_delivery_transaction( &payload.encode(), true, - Weight::from_ref_time(0), + Weight::zero(), ); let delivery_transaction_fee = BridgedChain::::transaction_payment(delivery_transaction); @@ -733,7 +733,7 @@ pub mod target { payload.weight = Some(weight); weight }, - _ => Weight::from_ref_time(0), + _ => Weight::zero(), } } @@ -762,17 +762,22 @@ pub mod target { location, xcm, hash, - weight_limit.unwrap_or(Weight::from_ref_time(0)).ref_time(), + weight_limit.unwrap_or_else(Weight::zero).ref_time(), weight_credit.ref_time(), ); Ok(xcm_outcome) }; let xcm_outcome = do_dispatch(); - log::trace!(target: "runtime::bridge-dispatch", "Incoming message {:?} dispatched with result: {:?}", message_id, xcm_outcome); + log::trace!( + target: "runtime::bridge-dispatch", + "Incoming message {:?} dispatched with result: {:?}", + message_id, + xcm_outcome, + ); MessageDispatchResult { dispatch_result: true, - unspent_weight: Weight::from_ref_time(0), + unspent_weight: Weight::zero(), dispatch_fee_paid_during_dispatch: false, } } diff --git a/bin/runtime-common/src/messages_api.rs b/bin/runtime-common/src/messages_api.rs index acc4c6d9a6e..28090daee5a 100644 --- a/bin/runtime-common/src/messages_api.rs +++ b/bin/runtime-common/src/messages_api.rs @@ -39,7 +39,7 @@ where nonce, // dispatch message weight is always zero at the source chain, since we're paying for // dispatch at the target chain - dispatch_weight: frame_support::weights::Weight::from_ref_time(0), + dispatch_weight: frame_support::weights::Weight::zero(), size: message_data.payload.len() as _, delivery_and_dispatch_fee: message_data.fee, // we're delivering XCM messages here, so fee is always paid at the target chain diff --git a/bin/runtime-common/src/messages_benchmarking.rs b/bin/runtime-common/src/messages_benchmarking.rs index 2a384236aff..71eed881a06 100644 --- a/bin/runtime-common/src/messages_benchmarking.rs +++ b/bin/runtime-common/src/messages_benchmarking.rs @@ -94,7 +94,7 @@ where nonces_start: *params.message_nonces.start(), nonces_end: *params.message_nonces.end(), }, - Weight::from_ref_time(0), + Weight::zero(), ) } diff --git a/bin/runtime-common/src/messages_extension.rs b/bin/runtime-common/src/messages_extension.rs index 39d44ad11fb..c5fb897b926 100644 --- a/bin/runtime-common/src/messages_extension.rs +++ b/bin/runtime-common/src/messages_extension.rs @@ -125,7 +125,7 @@ mod tests { pallet_bridge_messages::Call::::receive_messages_proof { relayer_id_at_bridged_chain: [0u8; 32].into(), messages_count: (nonces_end - nonces_start + 1) as u32, - dispatch_weight: frame_support::weights::Weight::from_ref_time(0), + dispatch_weight: frame_support::weights::Weight::zero(), proof: FromBridgedChainMessagesProof { bridged_header_hash: Default::default(), storage_proof: vec![], diff --git a/modules/grandpa/src/lib.rs b/modules/grandpa/src/lib.rs index 97875e8fade..e061b702a11 100644 --- a/modules/grandpa/src/lib.rs +++ b/modules/grandpa/src/lib.rs @@ -135,9 +135,7 @@ pub mod pallet { fn on_initialize(_n: T::BlockNumber) -> frame_support::weights::Weight { >::mutate(|count| *count = count.saturating_sub(1)); - Weight::from_ref_time(0) - .saturating_add(T::DbWeight::get().reads(1)) - .saturating_add(T::DbWeight::get().writes(1)) + T::DbWeight::get().reads_writes(1, 1) } } diff --git a/modules/messages/src/lib.rs b/modules/messages/src/lib.rs index 61e6df211c7..66f568e698c 100644 --- a/modules/messages/src/lib.rs +++ b/modules/messages/src/lib.rs @@ -330,11 +330,8 @@ pub mod pallet { }); // compute actual dispatch weight that depends on the stored message size - let actual_weight = sp_std::cmp::min_by( - T::WeightInfo::maximal_increase_message_fee(), - T::WeightInfo::increase_message_fee(message_size as _), - |w1, w2| w1.ref_time().cmp(&w2.ref_time()), - ); + let actual_weight = T::WeightInfo::maximal_increase_message_fee() + .min(T::WeightInfo::increase_message_fee(message_size as _)); Ok(PostDispatchInfo { actual_weight: Some(actual_weight), pays_fee: Pays::Yes }) } @@ -416,7 +413,7 @@ pub mod pallet { // on this lane. We can't dispatch lane messages out-of-order, so if declared // weight is not enough, let's move to next lane let dispatch_weight = T::MessageDispatch::dispatch_weight(&mut message); - if dispatch_weight.ref_time() > dispatch_weight_left.ref_time() { + if dispatch_weight.any_gt(dispatch_weight_left) { log::trace!( target: LOG_TARGET, "Cannot dispatch any more messages on lane {:?}. Weight: declared={}, left={}", @@ -454,10 +451,7 @@ pub mod pallet { ReceivalResult::TooManyUnconfirmedMessages => (dispatch_weight, true), }; - let unspent_weight = - sp_std::cmp::min_by(unspent_weight, dispatch_weight, |w1, w2| { - w1.ref_time().cmp(&w2.ref_time()) - }); + let unspent_weight = unspent_weight.min(dispatch_weight); dispatch_weight_left -= dispatch_weight - unspent_weight; actual_weight = actual_weight.saturating_sub(unspent_weight).saturating_sub( // delivery call weight formula assumes that the fee is paid at @@ -466,7 +460,7 @@ pub mod pallet { if refund_pay_dispatch_fee { T::WeightInfo::pay_inbound_dispatch_fee_overhead() } else { - Weight::from_ref_time(0) + Weight::zero() }, ); } @@ -585,7 +579,7 @@ pub mod pallet { let actual_callback_weight = T::OnDeliveryConfirmed::on_messages_delivered(&lane_id, &confirmed_messages); match preliminary_callback_overhead.checked_sub(&actual_callback_weight) { - Some(difference) if difference.ref_time() == 0 => (), + Some(difference) if difference.is_zero() => (), Some(difference) => { log::trace!( target: LOG_TARGET, @@ -880,7 +874,7 @@ fn send_message, I: 'static>( T::WeightInfo::single_message_callback_overhead(T::DbWeight::get()); let actual_callback_weight = T::OnMessageAccepted::on_messages_accepted(&lane_id, &nonce); match single_message_callback_overhead.checked_sub(&actual_callback_weight) { - Some(difference) if difference.ref_time() == 0 => (), + Some(difference) if difference.is_zero() => (), Some(difference) => { log::trace!( target: LOG_TARGET, diff --git a/modules/messages/src/weights_ext.rs b/modules/messages/src/weights_ext.rs index eaf6bc1029e..80a1b9cdf8a 100644 --- a/modules/messages/src/weights_ext.rs +++ b/modules/messages/src/weights_ext.rs @@ -43,14 +43,14 @@ pub fn ensure_weights_are_correct( db_weight: RuntimeDbWeight, ) { // verify `send_message` weight components - assert_ne!(W::send_message_overhead(), Weight::from_ref_time(0)); - assert_ne!(W::send_message_size_overhead(0), Weight::from_ref_time(0)); + assert_ne!(W::send_message_overhead(), Weight::zero()); + assert_ne!(W::send_message_size_overhead(0), Weight::zero()); // verify `receive_messages_proof` weight components - assert_ne!(W::receive_messages_proof_overhead(), Weight::from_ref_time(0)); - assert_ne!(W::receive_messages_proof_messages_overhead(1), Weight::from_ref_time(0)); - assert_ne!(W::receive_messages_proof_outbound_lane_state_overhead(), Weight::from_ref_time(0)); - assert_ne!(W::storage_proof_size_overhead(1), Weight::from_ref_time(0)); + assert_ne!(W::receive_messages_proof_overhead(), Weight::zero()); + assert_ne!(W::receive_messages_proof_messages_overhead(1), Weight::zero()); + assert_ne!(W::receive_messages_proof_outbound_lane_state_overhead(), Weight::zero()); + assert_ne!(W::storage_proof_size_overhead(1), Weight::zero()); // verify that the hardcoded value covers `receive_messages_proof` weight let actual_single_regular_message_delivery_tx_weight = W::receive_messages_proof_weight( @@ -58,11 +58,11 @@ pub fn ensure_weights_are_correct( (EXPECTED_DEFAULT_MESSAGE_LENGTH + W::expected_extra_storage_proof_size()) as usize, ), 1, - Weight::from_ref_time(0), + Weight::zero(), ); assert!( - actual_single_regular_message_delivery_tx_weight.ref_time() <= - expected_default_message_delivery_tx_weight.ref_time(), + actual_single_regular_message_delivery_tx_weight + .all_lte(expected_default_message_delivery_tx_weight), "Default message delivery transaction weight {} is larger than expected weight {}", actual_single_regular_message_delivery_tx_weight, expected_default_message_delivery_tx_weight, @@ -71,16 +71,15 @@ pub fn ensure_weights_are_correct( // verify that hardcoded value covers additional byte length of `receive_messages_proof` weight let actual_additional_byte_delivery_weight = W::storage_proof_size_overhead(1); assert!( - actual_additional_byte_delivery_weight.ref_time() <= - expected_additional_byte_delivery_weight.ref_time(), + actual_additional_byte_delivery_weight.all_lte(expected_additional_byte_delivery_weight), "Single additional byte delivery weight {} is larger than expected weight {}", actual_additional_byte_delivery_weight, expected_additional_byte_delivery_weight, ); // verify `receive_messages_delivery_proof` weight components - assert_ne!(W::receive_messages_delivery_proof_overhead(), Weight::from_ref_time(0)); - assert_ne!(W::storage_proof_size_overhead(1), Weight::from_ref_time(0)); + assert_ne!(W::receive_messages_delivery_proof_overhead(), Weight::zero()); + assert_ne!(W::storage_proof_size_overhead(1), Weight::zero()); // `receive_messages_delivery_proof_messages_overhead` and // `receive_messages_delivery_proof_relayers_overhead` may return zero if rewards are not paid @@ -97,8 +96,8 @@ pub fn ensure_weights_are_correct( db_weight, ); assert!( - actual_messages_delivery_confirmation_tx_weight.ref_time() <= - expected_messages_delivery_confirmation_tx_weight.ref_time(), + actual_messages_delivery_confirmation_tx_weight + .all_lte(expected_messages_delivery_confirmation_tx_weight), "Messages delivery confirmation transaction weight {} is larger than expected weight {}", actual_messages_delivery_confirmation_tx_weight, expected_messages_delivery_confirmation_tx_weight, @@ -107,7 +106,7 @@ pub fn ensure_weights_are_correct( // verify pay-dispatch-fee overhead for inbound messages let actual_pay_inbound_dispatch_fee_weight = W::pay_inbound_dispatch_fee_overhead(); assert!( - actual_pay_inbound_dispatch_fee_weight.ref_time() <= expected_pay_inbound_dispatch_fee_weight.ref_time(), + actual_pay_inbound_dispatch_fee_weight.all_lte(expected_pay_inbound_dispatch_fee_weight), "Weight {} of pay-dispatch-fee overhead for inbound messages is larger than expected weight {}", actual_pay_inbound_dispatch_fee_weight, expected_pay_inbound_dispatch_fee_weight, @@ -141,7 +140,7 @@ pub fn ensure_able_to_receive_message( max_incoming_message_dispatch_weight, ); assert!( - max_delivery_transaction_dispatch_weight.ref_time() <= max_extrinsic_weight.ref_time(), + max_delivery_transaction_dispatch_weight.all_lte(max_extrinsic_weight), "Weight of maximal message delivery transaction + {} is larger than maximal possible transaction weight {}", max_delivery_transaction_dispatch_weight, max_extrinsic_weight, @@ -180,7 +179,7 @@ pub fn ensure_able_to_receive_confirmation( db_weight, ); assert!( - max_confirmation_transaction_dispatch_weight.ref_time() <= max_extrinsic_weight.ref_time(), + max_confirmation_transaction_dispatch_weight.all_lte(max_extrinsic_weight), "Weight of maximal confirmation transaction {} is larger than maximal possible transaction weight {}", max_confirmation_transaction_dispatch_weight, max_extrinsic_weight, @@ -263,15 +262,14 @@ pub trait WeightInfoExt: WeightInfo { // and cost of calling `OnDeliveryConfirmed::on_messages_delivered()` for every confirmed // message - let callback_overhead = relayers_state - .total_messages - .saturating_mul(Self::single_message_callback_overhead(db_weight).ref_time()); + let callback_overhead = Self::single_message_callback_overhead(db_weight) + .saturating_mul(relayers_state.total_messages); transaction_overhead .saturating_add(messages_overhead) .saturating_add(relayers_overhead) .saturating_add(proof_size_overhead) - .saturating_add(Weight::from_ref_time(callback_overhead)) + .saturating_add(callback_overhead) } // Functions that are used by extrinsics weights formulas. diff --git a/modules/parachains/src/lib.rs b/modules/parachains/src/lib.rs index e0c3ff8340f..4b2be1b7170 100644 --- a/modules/parachains/src/lib.rs +++ b/modules/parachains/src/lib.rs @@ -748,7 +748,7 @@ mod tests { let db_weight = ::DbWeight::get(); WeightInfoOf::::submit_parachain_heads_weight(db_weight, proof, 1) .saturating_sub(if prune_expected { - Weight::from_ref_time(0) + Weight::zero() } else { WeightInfoOf::::parachain_head_pruning_weight(db_weight) }) diff --git a/primitives/messages/src/source_chain.rs b/primitives/messages/src/source_chain.rs index 4eb79edd848..8050f994dd3 100644 --- a/primitives/messages/src/source_chain.rs +++ b/primitives/messages/src/source_chain.rs @@ -195,7 +195,7 @@ impl MessagesBridge Result { - Ok(SendMessageArtifacts { nonce: 0, weight: Weight::from_ref_time(0) }) + Ok(SendMessageArtifacts { nonce: 0, weight: Weight::zero() }) } } @@ -220,7 +220,7 @@ pub trait OnDeliveryConfirmed { #[impl_trait_for_tuples::impl_for_tuples(30)] impl OnDeliveryConfirmed for Tuple { fn on_messages_delivered(lane: &LaneId, messages: &DeliveredMessages) -> Weight { - let mut total_weight = Weight::from_ref_time(0); + let mut total_weight = Weight::zero(); for_tuples!( #( total_weight = total_weight.saturating_add(Tuple::on_messages_delivered(lane, messages)); @@ -238,7 +238,7 @@ pub trait OnMessageAccepted { impl OnMessageAccepted for () { fn on_messages_accepted(_lane: &LaneId, _message: &MessageNonce) -> Weight { - Weight::from_ref_time(0) + Weight::zero() } } diff --git a/primitives/messages/src/target_chain.rs b/primitives/messages/src/target_chain.rs index 05623d0efba..58419c398bb 100644 --- a/primitives/messages/src/target_chain.rs +++ b/primitives/messages/src/target_chain.rs @@ -167,7 +167,7 @@ impl MessageDispatch for ForbidInboundMessages { ) -> MessageDispatchResult { MessageDispatchResult { dispatch_result: false, - unspent_weight: Weight::from_ref_time(0), + unspent_weight: Weight::zero(), dispatch_fee_paid_during_dispatch: false, } } diff --git a/primitives/runtime/src/lib.rs b/primitives/runtime/src/lib.rs index cd395192049..9f525f5ea85 100644 --- a/primitives/runtime/src/lib.rs +++ b/primitives/runtime/src/lib.rs @@ -20,7 +20,8 @@ use codec::{Decode, Encode, FullCodec, MaxEncodedLen}; use frame_support::{ - log, pallet_prelude::DispatchResult, PalletError, RuntimeDebug, StorageHasher, StorageValue, + log, pallet_prelude::DispatchResult, weights::Weight, PalletError, RuntimeDebug, StorageHasher, + StorageValue, }; use frame_system::RawOrigin; use scale_info::TypeInfo; @@ -456,6 +457,24 @@ pub trait FilterCall { fn validate(call: &Call) -> TransactionValidity; } +/// All extra operations with weights that we need in bridges. +pub trait WeightExtraOps { + /// Checked division of individual components of two weights. + /// + /// Divides components and returns minimal division result. Returns `None` if one + /// of `other` weight components is zero. + fn min_components_checked_div(&self, other: Weight) -> Option; +} + +impl WeightExtraOps for Weight { + fn min_components_checked_div(&self, other: Weight) -> Option { + Some(sp_std::cmp::min( + self.ref_time().checked_div(other.ref_time())?, + self.proof_size().checked_div(other.proof_size())?, + )) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/relays/lib-substrate-relay/src/messages_lane.rs b/relays/lib-substrate-relay/src/messages_lane.rs index d821aa6f4cb..72da8f20220 100644 --- a/relays/lib-substrate-relay/src/messages_lane.rs +++ b/relays/lib-substrate-relay/src/messages_lane.rs @@ -27,7 +27,7 @@ use crate::{ use async_std::sync::Arc; use bp_messages::{LaneId, MessageNonce}; -use bp_runtime::{AccountIdOf, Chain as _}; +use bp_runtime::{AccountIdOf, Chain as _, WeightExtraOps}; use bridge_runtime_common::messages::{ source::FromBridgedChainMessagesDeliveryProof, target::FromBridgedChainMessagesProof, }; @@ -462,15 +462,12 @@ pub fn select_delivery_transaction_limits 0, diff --git a/relays/lib-substrate-relay/src/messages_source.rs b/relays/lib-substrate-relay/src/messages_source.rs index 25869da678a..c4e0f15f859 100644 --- a/relays/lib-substrate-relay/src/messages_source.rs +++ b/relays/lib-substrate-relay/src/messages_source.rs @@ -653,7 +653,7 @@ mod tests { .into_iter() .map(|nonce| bp_messages::OutboundMessageDetails { nonce, - dispatch_weight: Weight::from_ref_time(0), + dispatch_weight: Weight::zero(), size: 0, delivery_and_dispatch_fee: 0, dispatch_fee_payment: DispatchFeePayment::AtSourceChain, @@ -730,7 +730,7 @@ mod tests { for (idx, _) in payload_sizes.iter().enumerate() { out_msgs_details.push(OutboundMessageDetails::> { nonce: idx as MessageNonce, - dispatch_weight: Weight::from_ref_time(0), + dispatch_weight: Weight::zero(), size: 0, delivery_and_dispatch_fee: 0, dispatch_fee_payment: DispatchFeePayment::AtTargetChain, diff --git a/relays/messages/src/message_race_delivery.rs b/relays/messages/src/message_race_delivery.rs index a2c81d8cc30..9481e47faf8 100644 --- a/relays/messages/src/message_race_delivery.rs +++ b/relays/messages/src/message_race_delivery.rs @@ -295,7 +295,7 @@ impl MessageDeliveryStrategy EnforcementStrategy { let mut hard_selected_count = 0; let mut soft_selected_count = 0; - let mut selected_weight = Weight::from_ref_time(0); + let mut selected_weight = Weight::zero(); let mut selected_count: MessageNonce = 0; let hard_selected_begin_nonce = @@ -77,12 +77,12 @@ impl EnforcementStrategy { hard_selected_begin_nonce, selected_prepaid_nonces: 0, - selected_unpaid_weight: Weight::from_ref_time(0), + selected_unpaid_weight: Weight::zero(), index: 0, nonce: 0, details: MessageDetails { - dispatch_weight: Weight::from_ref_time(0), + dispatch_weight: Weight::zero(), size: 0, reward: P::SourceChainBalance::zero(), dispatch_fee_payment: DispatchFeePayment::AtSourceChain, @@ -107,8 +107,8 @@ impl EnforcementStrategy { // limit messages in the batch by weight let new_selected_weight = match selected_weight.checked_add(&details.dispatch_weight) { Some(new_selected_weight) - if new_selected_weight.ref_time() <= - reference.max_messages_weight_in_single_batch.ref_time() => + if new_selected_weight + .all_lte(reference.max_messages_weight_in_single_batch) => new_selected_weight, new_selected_weight if selected_count == 0 => { log::warn!(