Skip to content

Second iteration of weights v1.5 #1613

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions bin/runtime-common/src/messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ pub mod source {
let delivery_transaction = BridgedChain::<B>::estimate_delivery_transaction(
&payload.encode(),
true,
Weight::from_ref_time(0),
Weight::zero(),
);
let delivery_transaction_fee = BridgedChain::<B>::transaction_payment(delivery_transaction);

Expand Down Expand Up @@ -733,7 +733,7 @@ pub mod target {
payload.weight = Some(weight);
weight
},
_ => Weight::from_ref_time(0),
_ => Weight::zero(),
}
}

Expand Down Expand Up @@ -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,
}
}
Expand Down
2 changes: 1 addition & 1 deletion bin/runtime-common/src/messages_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion bin/runtime-common/src/messages_benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ where
nonces_start: *params.message_nonces.start(),
nonces_end: *params.message_nonces.end(),
},
Weight::from_ref_time(0),
Weight::zero(),
)
}

Expand Down
2 changes: 1 addition & 1 deletion bin/runtime-common/src/messages_extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ mod tests {
pallet_bridge_messages::Call::<Runtime, ()>::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![],
Expand Down
4 changes: 1 addition & 3 deletions modules/grandpa/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,7 @@ pub mod pallet {
fn on_initialize(_n: T::BlockNumber) -> frame_support::weights::Weight {
<RequestCount<T, I>>::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)
}
}

Expand Down
20 changes: 7 additions & 13 deletions modules/messages/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 })
}
Expand Down Expand Up @@ -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={}",
Expand Down Expand Up @@ -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
Expand All @@ -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()
},
);
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -880,7 +874,7 @@ fn send_message<T: Config<I>, 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,
Expand Down
42 changes: 20 additions & 22 deletions modules/messages/src/weights_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,26 +43,26 @@ pub fn ensure_weights_are_correct<W: WeightInfoExt>(
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(
&PreComputedSize(
(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,
Expand All @@ -71,16 +71,15 @@ pub fn ensure_weights_are_correct<W: WeightInfoExt>(
// 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
Expand All @@ -97,8 +96,8 @@ pub fn ensure_weights_are_correct<W: WeightInfoExt>(
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,
Expand All @@ -107,7 +106,7 @@ pub fn ensure_weights_are_correct<W: WeightInfoExt>(
// 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,
Expand Down Expand Up @@ -141,7 +140,7 @@ pub fn ensure_able_to_receive_message<W: WeightInfoExt>(
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,
Expand Down Expand Up @@ -180,7 +179,7 @@ pub fn ensure_able_to_receive_confirmation<W: WeightInfoExt>(
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,
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion modules/parachains/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -748,7 +748,7 @@ mod tests {
let db_weight = <TestRuntime as frame_system::Config>::DbWeight::get();
WeightInfoOf::<TestRuntime, ()>::submit_parachain_heads_weight(db_weight, proof, 1)
.saturating_sub(if prune_expected {
Weight::from_ref_time(0)
Weight::zero()
} else {
WeightInfoOf::<TestRuntime, ()>::parachain_head_pruning_weight(db_weight)
})
Expand Down
6 changes: 3 additions & 3 deletions primitives/messages/src/source_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ impl<SenderOrigin, Balance, Payload> MessagesBridge<SenderOrigin, Balance, Paylo
_message: Payload,
_delivery_and_dispatch_fee: Balance,
) -> Result<SendMessageArtifacts, Self::Error> {
Ok(SendMessageArtifacts { nonce: 0, weight: Weight::from_ref_time(0) })
Ok(SendMessageArtifacts { nonce: 0, weight: Weight::zero() })
}
}

Expand All @@ -237,7 +237,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));
Expand All @@ -255,7 +255,7 @@ pub trait OnMessageAccepted {

impl OnMessageAccepted for () {
fn on_messages_accepted(_lane: &LaneId, _message: &MessageNonce) -> Weight {
Weight::from_ref_time(0)
Weight::zero()
}
}

Expand Down
2 changes: 1 addition & 1 deletion primitives/messages/src/target_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ impl<AccountId, Fee> MessageDispatch<AccountId, Fee> for ForbidInboundMessages {
) -> MessageDispatchResult {
MessageDispatchResult {
dispatch_result: false,
unspent_weight: Weight::from_ref_time(0),
unspent_weight: Weight::zero(),
dispatch_fee_paid_during_dispatch: false,
}
}
Expand Down
21 changes: 20 additions & 1 deletion primitives/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -456,6 +457,24 @@ pub trait FilterCall<Call> {
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<u64>;
}

impl WeightExtraOps for Weight {
fn min_components_checked_div(&self, other: Weight) -> Option<u64> {
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::*;
Expand Down
17 changes: 7 additions & 10 deletions relays/lib-substrate-relay/src/messages_lane.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -462,15 +462,12 @@ pub fn select_delivery_transaction_limits<W: pallet_bridge_messages::WeightInfoE
W::receive_messages_proof_outbound_lane_state_overhead();
let delivery_tx_weight_rest = weight_for_delivery_tx - delivery_tx_base_weight;

let max_number_of_messages = if delivery_tx_weight_rest.ref_time() /
W::receive_messages_proof_messages_overhead(1).ref_time() <
max_unconfirmed_messages_at_inbound_lane
{
delivery_tx_weight_rest.ref_time() /
W::receive_messages_proof_messages_overhead(1).ref_time()
} else {
max_unconfirmed_messages_at_inbound_lane
};
let max_number_of_messages = std::cmp::min(
delivery_tx_weight_rest
.min_components_checked_div(W::receive_messages_proof_messages_overhead(1))
.unwrap_or(u64::MAX),
max_unconfirmed_messages_at_inbound_lane,
);

assert!(
max_number_of_messages > 0,
Expand Down
4 changes: 2 additions & 2 deletions relays/lib-substrate-relay/src/messages_source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -730,7 +730,7 @@ mod tests {
for (idx, _) in payload_sizes.iter().enumerate() {
out_msgs_details.push(OutboundMessageDetails::<BalanceOf<Rialto>> {
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,
Expand Down
2 changes: 1 addition & 1 deletion relays/messages/src/message_race_delivery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ impl<P: MessageLane, Strategy: RelayStrategy, SC, TC> MessageDeliveryStrategy<P,
.source_queue()
.iter()
.flat_map(|(_, range)| range.values().map(|details| details.dispatch_weight))
.fold(Weight::from_ref_time(0), |total, weight| total.saturating_add(weight))
.fold(Weight::zero(), |total, weight| total.saturating_add(weight))
}
}

Expand Down
Loading