Skip to content
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

Added receive_single_message_proof_with_dispatch benchmark #1990

Merged
merged 2 commits into from
Mar 24, 2023
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
4 changes: 2 additions & 2 deletions bin/millau/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1007,7 +1007,7 @@ impl_runtime_apis! {
Runtime,
WithRialtoParachainsInstance,
WithRialtoParachainMessageBridge,
>(params)
>(params, xcm::v3::Junctions::Here)
}

fn prepare_message_delivery_proof(
Expand Down Expand Up @@ -1038,7 +1038,7 @@ impl_runtime_apis! {
Runtime,
RialtoGrandpaInstance,
WithRialtoMessageBridge,
>(params)
>(params, xcm::v3::Junctions::Here)
}

fn prepare_message_delivery_proof(
Expand Down
2 changes: 1 addition & 1 deletion bin/millau/runtime/src/rialto_messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ pub type FromRialtoMessageDispatch =
bp_millau::Millau,
bp_rialto::Rialto,
crate::xcm_config::OnMillauBlobDispatcher,
bridge_runtime_common::messages_xcm_extension::XcmRouterWeigher<crate::DbWeight>,
(),
>;

/// Maximal outbound payload size of Millau -> Rialto messages.
Expand Down
2 changes: 1 addition & 1 deletion bin/millau/runtime/src/rialto_parachain_messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ pub type FromRialtoParachainMessageDispatch =
bp_millau::Millau,
bp_rialto::Rialto,
crate::xcm_config::OnMillauBlobDispatcher,
bridge_runtime_common::messages_xcm_extension::XcmRouterWeigher<crate::DbWeight>,
(),
>;

/// Maximal outbound payload size of Millau -> RialtoParachain messages.
Expand Down
21 changes: 5 additions & 16 deletions bin/millau/runtime/src/xcm_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,20 +233,16 @@ impl ExportXcm for ToRialtoOrRialtoParachainSwitchExporter {
mod tests {
use super::*;
use crate::{
rialto_messages::FromRialtoMessageDispatch,
rialto_parachain_messages::FromRialtoParachainMessageDispatch, DbWeight,
WithRialtoMessagesInstance, WithRialtoParachainMessagesInstance,
rialto_messages::FromRialtoMessageDispatch, WithRialtoMessagesInstance,
WithRialtoParachainMessagesInstance,
};
use bp_messages::{
target_chain::{DispatchMessage, DispatchMessageData, MessageDispatch},
LaneId, MessageKey,
};
use bridge_runtime_common::messages_xcm_extension::{
XcmBlobMessageDispatchResult, XcmRouterWeigher,
};
use bridge_runtime_common::messages_xcm_extension::XcmBlobMessageDispatchResult;
use codec::Encode;
use pallet_bridge_messages::OutboundLanes;
use sp_core::Get;
use xcm_executor::XcmExecutor;

fn new_test_ext() -> sp_io::TestExternalities {
Expand Down Expand Up @@ -347,10 +343,7 @@ mod tests {

#[test]
fn xcm_messages_from_rialto_are_dispatched() {
let mut incoming_message = prepare_inbound_bridge_message();

let dispatch_weight = FromRialtoMessageDispatch::dispatch_weight(&mut incoming_message);
assert_eq!(dispatch_weight, XcmRouterWeigher::<DbWeight>::get());
let incoming_message = prepare_inbound_bridge_message();

// we care only about handing message to the XCM dispatcher, so we don't care about its
// actual dispatch
Expand All @@ -364,11 +357,7 @@ mod tests {

#[test]
fn xcm_messages_from_rialto_parachain_are_dispatched() {
let mut incoming_message = prepare_inbound_bridge_message();

let dispatch_weight =
FromRialtoParachainMessageDispatch::dispatch_weight(&mut incoming_message);
assert_eq!(dispatch_weight, XcmRouterWeigher::<DbWeight>::get());
let incoming_message = prepare_inbound_bridge_message();

// we care only about handing message to the XCM dispatcher, so we don't care about its
// actual dispatch
Expand Down
9 changes: 2 additions & 7 deletions bin/rialto-parachain/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -844,12 +844,10 @@ mod tests {
LaneId, MessageKey,
};
use bridge_runtime_common::{
integrity::check_additional_signed,
messages_xcm_extension::{XcmBlobMessageDispatchResult, XcmRouterWeigher},
integrity::check_additional_signed, messages_xcm_extension::XcmBlobMessageDispatchResult,
};
use codec::Encode;
use pallet_bridge_messages::OutboundLanes;
use sp_core::Get;
use sp_runtime::generic::Era;
use xcm_executor::XcmExecutor;

Expand Down Expand Up @@ -914,10 +912,7 @@ mod tests {
#[test]
fn xcm_messages_from_millau_are_dispatched() {
new_test_ext().execute_with(|| {
let mut incoming_message = prepare_inbound_bridge_message();

let dispatch_weight = FromMillauMessageDispatch::dispatch_weight(&mut incoming_message);
assert_eq!(dispatch_weight, XcmRouterWeigher::<()>::get());
let incoming_message = prepare_inbound_bridge_message();

// we care only about handing message to the XCM dispatcher, so we don't care about its
// actual dispatch
Expand Down
2 changes: 1 addition & 1 deletion bin/rialto-parachain/runtime/src/millau_messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ pub type FromMillauMessageDispatch =
bp_rialto_parachain::RialtoParachain,
bp_millau::Millau,
crate::OnRialtoParachainBlobDispatcher,
bridge_runtime_common::messages_xcm_extension::XcmRouterWeigher<()>,
(),
>;

/// Messages proof for Millau -> RialtoParachain messages.
Expand Down
2 changes: 1 addition & 1 deletion bin/rialto/runtime/src/millau_messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ pub type FromMillauMessageDispatch =
bp_rialto::Rialto,
bp_millau::Millau,
crate::xcm_config::OnRialtoBlobDispatcher,
bridge_runtime_common::messages_xcm_extension::XcmRouterWeigher<crate::DbWeight>,
(),
>;

/// Messages proof for Millau -> Rialto messages.
Expand Down
12 changes: 3 additions & 9 deletions bin/rialto/runtime/src/xcm_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,18 +189,15 @@ mod tests {
use super::*;
use crate::{
millau_messages::{FromMillauMessageDispatch, XCM_LANE},
DbWeight, WithMillauMessagesInstance,
WithMillauMessagesInstance,
};
use bp_messages::{
target_chain::{DispatchMessage, DispatchMessageData, MessageDispatch},
LaneId, MessageKey,
};
use bridge_runtime_common::messages_xcm_extension::{
XcmBlobMessageDispatchResult, XcmRouterWeigher,
};
use bridge_runtime_common::messages_xcm_extension::XcmBlobMessageDispatchResult;
use codec::Encode;
use pallet_bridge_messages::OutboundLanes;
use sp_core::Get;
use xcm_executor::XcmExecutor;

fn new_test_ext() -> sp_io::TestExternalities {
Expand Down Expand Up @@ -263,10 +260,7 @@ mod tests {

#[test]
fn xcm_messages_from_millau_are_dispatched() {
let mut incoming_message = prepare_inbound_bridge_message();

let dispatch_weight = FromMillauMessageDispatch::dispatch_weight(&mut incoming_message);
assert_eq!(dispatch_weight, XcmRouterWeigher::<DbWeight>::get());
let incoming_message = prepare_inbound_bridge_message();

// we care only about handing message to the XCM dispatcher, so we don't care about its
// actual dispatch
Expand Down
53 changes: 41 additions & 12 deletions bin/runtime-common/src/messages_benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,39 @@ use pallet_bridge_messages::benchmarking::{MessageDeliveryProofParams, MessagePr
use sp_runtime::traits::{Header, Zero};
use sp_std::prelude::*;
use sp_trie::{trie_types::TrieDBMutBuilderV1, LayoutV1, MemoryDB, TrieMut};
use xcm::v3::prelude::*;

/// Prepare inbound bridge message according to given message proof parameters.
fn prepare_inbound_message(
params: &MessageProofParams,
destination: InteriorMultiLocation,
) -> Vec<u8> {
// we only care about **this** message size when message proof needs to be `Minimal`
let expected_size = match params.size {
StorageProofSize::Minimal(size) => size as usize,
_ => 0,
};

// if we don't need a correct message, then we may just return some random blob
if !params.is_successful_dispatch_expected {
return vec![0u8; expected_size]
}

// else let's prepare successful message. For XCM bridge hubs, it is the message that
// will be pushed further to some XCM queue (XCMP/UMP)
let location = xcm::VersionedInteriorMultiLocation::V3(destination);
let location_encoded_size = location.encoded_size();

// we don't need to be super-precise with `expected_size` here
let xcm_size = expected_size.saturating_sub(location_encoded_size);
let xcm = xcm::VersionedXcm::<()>::V3(vec![Instruction::ClearOrigin; xcm_size].into());

// this is the `BridgeMessage` from polkadot xcm builder, but it has no constructor
// or public fields, so just tuple
// (double encoding, because `.encode()` is called on original Xcm BLOB when it is pushed
// to the storage)
(location, xcm).encode().encode()
}

/// Prepare proof of messages for the `receive_messages_proof` call.
///
Expand All @@ -51,6 +84,7 @@ use sp_trie::{trie_types::TrieDBMutBuilderV1, LayoutV1, MemoryDB, TrieMut};
/// function.
pub fn prepare_message_proof_from_grandpa_chain<R, FI, B>(
params: MessageProofParams,
message_destination: InteriorMultiLocation,
) -> (FromBridgedChainMessagesProof<HashOf<BridgedChain<B>>>, Weight)
where
R: pallet_bridge_grandpa::Config<FI, BridgedChain = UnderlyingChainOf<BridgedChain<B>>>,
Expand All @@ -61,12 +95,9 @@ where
let (state_root, storage_proof) = prepare_messages_storage_proof::<B>(
params.lane,
params.message_nonces.clone(),
params.outbound_lane_data,
params.outbound_lane_data.clone(),
params.size,
match params.size {
StorageProofSize::Minimal(ref size) => vec![0u8; *size as _],
_ => vec![],
},
prepare_inbound_message(&params, message_destination),
encode_all_messages,
encode_lane_data,
);
Expand All @@ -82,7 +113,7 @@ where
nonces_start: *params.message_nonces.start(),
nonces_end: *params.message_nonces.end(),
},
Weight::zero(),
Weight::MAX / 1000,
)
}

Expand All @@ -96,6 +127,7 @@ where
/// `prepare_message_proof_from_grandpa_chain` function.
pub fn prepare_message_proof_from_parachain<R, PI, B>(
params: MessageProofParams,
message_destination: InteriorMultiLocation,
) -> (FromBridgedChainMessagesProof<HashOf<BridgedChain<B>>>, Weight)
where
R: pallet_bridge_parachains::Config<PI>,
Expand All @@ -107,12 +139,9 @@ where
let (state_root, storage_proof) = prepare_messages_storage_proof::<B>(
params.lane,
params.message_nonces.clone(),
params.outbound_lane_data,
params.outbound_lane_data.clone(),
params.size,
match params.size {
StorageProofSize::Minimal(ref size) => vec![0u8; *size as _],
_ => vec![],
},
prepare_inbound_message(&params, message_destination),
encode_all_messages,
encode_lane_data,
);
Expand All @@ -129,7 +158,7 @@ where
nonces_start: *params.message_nonces.start(),
nonces_end: *params.message_nonces.end(),
},
Weight::zero(),
Weight::MAX / 1000,
)
}

Expand Down
70 changes: 17 additions & 53 deletions bin/runtime-common/src/messages_xcm_extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,43 +28,15 @@ use bp_messages::{
};
use bp_runtime::{messages::MessageDispatchResult, AccountIdOf, Chain};
use codec::{Decode, Encode};
use frame_support::{
dispatch::Weight, traits::Get, weights::RuntimeDbWeight, CloneNoBound, EqNoBound,
PartialEqNoBound,
};
use frame_support::{dispatch::Weight, CloneNoBound, EqNoBound, PartialEqNoBound};
use pallet_bridge_messages::WeightInfoExt as MessagesPalletWeights;
use scale_info::TypeInfo;
use sp_std::marker::PhantomData;
use sp_runtime::SaturatedConversion;
use xcm_builder::{DispatchBlob, DispatchBlobError, HaulBlob, HaulBlobError};

/// Plain "XCM" payload, which we transfer through bridge
pub type XcmAsPlainPayload = sp_std::prelude::Vec<u8>;

// TODO: below are just rough estimations. Other things also happen there (including hashing and so
// on). Shall we do some benchmarking??? TODO: add proof_size component here
// https://github.com/paritytech/parity-bridges-common/issues/1986

/// Simple weigher for incoming XCM dispatch at **bridge hubs** to use with
/// `XcmBlobMessageDispatch`.
///
/// By our design, message at bridge hub is simply pushed to some other queue. This implementation
/// is for this case only. If your runtime performs some other actions with incoming XCM messages,
/// you shall use your own implementation.
///
/// If message is redirected to the relay chain, then `ParentAsUmp` is used and it roughly does
/// 1 db read and 1 db write (in its `send_upward_message` method).
///
/// If message is redirected to some sibling parachain, then `XcmpQueue` is used and
/// it roughly does 2 db reads and 2 db writes (in its `SendXcm` implementation).
///
/// The difference is not that big, so let's choose maximal.
pub struct XcmRouterWeigher<T>(PhantomData<T>);

impl<T: Get<RuntimeDbWeight>> Get<Weight> for XcmRouterWeigher<T> {
fn get() -> Weight {
T::get().reads_writes(2, 2)
}
}

/// Message dispatch result type for single message
#[derive(CloneNoBound, EqNoBound, PartialEqNoBound, Encode, Decode, Debug, TypeInfo)]
pub enum XcmBlobMessageDispatchResult {
Expand All @@ -74,38 +46,35 @@ pub enum XcmBlobMessageDispatchResult {
}

/// [`XcmBlobMessageDispatch`] is responsible for dispatching received messages
pub struct XcmBlobMessageDispatch<
SourceBridgeHubChain,
TargetBridgeHubChain,
DispatchBlob,
DispatchBlobWeigher,
> {
pub struct XcmBlobMessageDispatch<SourceBridgeHubChain, TargetBridgeHubChain, DispatchBlob, Weights>
{
_marker: sp_std::marker::PhantomData<(
SourceBridgeHubChain,
TargetBridgeHubChain,
DispatchBlob,
DispatchBlobWeigher,
Weights,
)>,
}

impl<
SourceBridgeHubChain: Chain,
TargetBridgeHubChain: Chain,
BlobDispatcher: DispatchBlob,
DispatchBlobWeigher: Get<Weight>,
Weights: MessagesPalletWeights,
> MessageDispatch<AccountIdOf<SourceBridgeHubChain>>
for XcmBlobMessageDispatch<
SourceBridgeHubChain,
TargetBridgeHubChain,
BlobDispatcher,
DispatchBlobWeigher,
>
for XcmBlobMessageDispatch<SourceBridgeHubChain, TargetBridgeHubChain, BlobDispatcher, Weights>
{
type DispatchPayload = XcmAsPlainPayload;
type DispatchLevelResult = XcmBlobMessageDispatchResult;

fn dispatch_weight(_message: &mut DispatchMessage<Self::DispatchPayload>) -> Weight {
DispatchBlobWeigher::get()
fn dispatch_weight(message: &mut DispatchMessage<Self::DispatchPayload>) -> Weight {
match message.data.payload {
Ok(ref payload) => {
let payload_size = payload.encoded_size().saturated_into();
Weights::message_dispatch_weight(payload_size)
},
Err(_) => Weight::zero(),
}
}

fn dispatch(
Expand All @@ -122,7 +91,6 @@ impl<
message.key.nonce
);
return MessageDispatchResult {
// TODO:check-parameter - setup uspent_weight? https://github.com/paritytech/polkadot/issues/6629
unspent_weight: Weight::zero(),
dispatch_level_result: XcmBlobMessageDispatchResult::InvalidPayload,
}
Expand Down Expand Up @@ -158,11 +126,7 @@ impl<
XcmBlobMessageDispatchResult::NotDispatched(e)
},
};
MessageDispatchResult {
// TODO:check-parameter - setup uspent_weight? https://github.com/paritytech/polkadot/issues/6629
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@svyatonik
Weights::message_dispatch_weight(payload_size) means now worst scenario, right?

so, now we dont need to fix DispatchBlob?
the final solution would be:
unspent_weight: Weights::message_dispatch_weight(payload_size) - spent_weight_by_blob_dispatch

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weights::message_dispatch_weight(payload_size) means now worst scenario, right?

Yes, it should be the worst scenarion. And in our case (we always push message to the XCMP queue), it is the only scenario. If we'll be adding more options for message routing, then it may change. But for now it is fine I believe

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, if we bridge message for relaychain as target then it would be DmpQueue, XcmpQueue is for parachain as target,
but I guess it is similar and XcmpQueue is worst case

ayway, do we still need unspent_weight?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah - that's a good point. If we'll be supporting to-relay routers or e.g. to-ethereum routers, then we'll probably want them to return actual dispatch weight (that's an ideal option). Then we may return actual unspent_weight and make things cheaper (and fit more messages into bridge hub block).

An alternate option may just use some runtime-level (or specific router-level) benchmarks to compute this kinda actual weight. So we'll be returning the worst possible value from dispatch_weight function and then from actual dispatch we will be looking at the XCM destination and return proper unspent_weight.

But for now, at least until we start work on dynamic bridging stuff, I think it is fine to have this ^^^ implementation

unspent_weight: Weight::zero(),
dispatch_level_result,
}
MessageDispatchResult { unspent_weight: Weight::zero(), dispatch_level_result }
}
}

Expand Down
Loading