From aac1305406ac68065c603f922efb1afe59277120 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Fri, 12 Aug 2022 12:05:46 +0300 Subject: [PATCH] Follow-up on #1518 (#1546) * Adjustments for the xcm messages sending logic Signed-off-by: Serban Iorga * Deduplicate XCM destination Signed-off-by: Serban Iorga * [send_message] small changes Signed-off-by: Serban Iorga * Define CustomNetworkId Right now we use some associations between Rialto, RialtoParachain and Millau chains and chains defined in the NetworkId enum. But if we are not carreful we might do mistakes like: In Millau: pub const ThisNetwork: NetworkId = Kusama; pub const RialtoNetwork: NetworkId = Polkadot; In Rialto: pub const ThisNetwork: NetworkId = Kusama; pub const MillauNetwork: NetworkId = Polkadot; We're introducing CustomNetworkId to have a centralized mapping between NetworkId chains and our custom chains. Signed-off-by: Serban Iorga * Revert "Deduplicate XCM destination" This reverts commit 3a0a950e1d7484e3ecac45f5c00b152f0485cd11. Signed-off-by: Serban Iorga --- bridges/bin/millau/runtime/src/xcm_config.rs | 17 ++++---- .../bin/rialto-parachain/runtime/src/lib.rs | 11 ++--- bridges/bin/rialto/runtime/src/xcm_config.rs | 13 +++--- bridges/bin/runtime-common/src/lib.rs | 23 +++++++++++ bridges/bin/runtime-common/src/messages.rs | 2 +- bridges/relays/bin-substrate/Cargo.toml | 1 - .../relays/bin-substrate/src/chains/millau.rs | 40 ++++++++----------- .../relays/bin-substrate/src/chains/rialto.rs | 19 +++++---- .../src/chains/rialto_parachain.rs | 21 +++++----- .../bin-substrate/src/cli/estimate_fee.rs | 10 ++--- .../bin-substrate/src/cli/send_message.rs | 4 +- 11 files changed, 89 insertions(+), 72 deletions(-) diff --git a/bridges/bin/millau/runtime/src/xcm_config.rs b/bridges/bin/millau/runtime/src/xcm_config.rs index 19fb68b66265b..57f28e1404444 100644 --- a/bridges/bin/millau/runtime/src/xcm_config.rs +++ b/bridges/bin/millau/runtime/src/xcm_config.rs @@ -27,7 +27,10 @@ use super::{ use bp_messages::LaneId; use bp_millau::WeightToFee; use bp_rialto_parachain::RIALTO_PARACHAIN_ID; -use bridge_runtime_common::messages::source::{XcmBridge, XcmBridgeAdapter}; +use bridge_runtime_common::{ + messages::source::{XcmBridge, XcmBridgeAdapter}, + CustomNetworkId, +}; use frame_support::{ parameter_types, traits::{Everything, Nothing}, @@ -45,12 +48,12 @@ parameter_types! { /// chain, we make it synonymous with it and thus it is the `Here` location, which means "equivalent to /// the context". pub const TokenLocation: MultiLocation = Here.into_location(); - /// The Millau network ID, associated with Kusama. - pub const ThisNetwork: NetworkId = Kusama; - /// The Rialto network ID, associated with Polkadot. - pub const RialtoNetwork: NetworkId = Polkadot; - /// The RialtoParachain network ID, associated with Westend. - pub const RialtoParachainNetwork: NetworkId = Westend; + /// The Millau network ID. + pub const ThisNetwork: NetworkId = CustomNetworkId::Millau.as_network_id(); + /// The Rialto network ID. + pub const RialtoNetwork: NetworkId = CustomNetworkId::Rialto.as_network_id(); + /// The RialtoParachain network ID. + pub const RialtoParachainNetwork: NetworkId = CustomNetworkId::RialtoParachain.as_network_id(); /// Our XCM location ancestry - i.e. our location within the Consensus Universe. /// diff --git a/bridges/bin/rialto-parachain/runtime/src/lib.rs b/bridges/bin/rialto-parachain/runtime/src/lib.rs index e96268d8aa8ca..5cc577281fdaa 100644 --- a/bridges/bin/rialto-parachain/runtime/src/lib.rs +++ b/bridges/bin/rialto-parachain/runtime/src/lib.rs @@ -78,6 +78,7 @@ pub use pallet_bridge_messages::Call as MessagesCall; pub use pallet_xcm::Call as XcmCall; // Polkadot & XCM imports +use bridge_runtime_common::CustomNetworkId; use pallet_xcm::XcmPassthrough; use polkadot_parachain::primitives::Sibling; use xcm::latest::prelude::*; @@ -304,13 +305,13 @@ impl pallet_randomness_collective_flip::Config for Runtime {} parameter_types! { pub const RelayLocation: MultiLocation = MultiLocation::parent(); - pub const RelayNetwork: NetworkId = NetworkId::Polkadot; + pub const RelayNetwork: NetworkId = CustomNetworkId::Rialto.as_network_id(); pub RelayOrigin: Origin = cumulus_pallet_xcm::Origin::Relay.into(); pub UniversalLocation: InteriorMultiLocation = X1(Parachain(ParachainInfo::parachain_id().into())); - /// The Millau network ID, associated with Kusama. - pub const MillauNetwork: NetworkId = Kusama; - /// The RialtoParachain network ID, associated with Westend. - pub const ThisNetwork: NetworkId = Westend; + /// The Millau network ID. + pub const MillauNetwork: NetworkId = CustomNetworkId::Millau.as_network_id(); + /// The RialtoParachain network ID. + pub const ThisNetwork: NetworkId = CustomNetworkId::RialtoParachain.as_network_id(); } /// Type for specifying how a `MultiLocation` can be converted into an `AccountId`. This is used diff --git a/bridges/bin/rialto/runtime/src/xcm_config.rs b/bridges/bin/rialto/runtime/src/xcm_config.rs index 63b2d0bead0f0..dbe8ad81c4fe1 100644 --- a/bridges/bin/rialto/runtime/src/xcm_config.rs +++ b/bridges/bin/rialto/runtime/src/xcm_config.rs @@ -21,7 +21,10 @@ use super::{ Event, Origin, Runtime, WithMillauMessagesInstance, XcmPallet, }; use bp_rialto::WeightToFee; -use bridge_runtime_common::messages::source::{XcmBridge, XcmBridgeAdapter}; +use bridge_runtime_common::{ + messages::source::{XcmBridge, XcmBridgeAdapter}, + CustomNetworkId, +}; use frame_support::{ parameter_types, traits::{Everything, Nothing}, @@ -39,10 +42,10 @@ parameter_types! { /// chain, we make it synonymous with it and thus it is the `Here` location, which means "equivalent to /// the context". pub const TokenLocation: MultiLocation = Here.into_location(); - /// The Rialto network ID, associated with Polkadot. - pub const ThisNetwork: NetworkId = Polkadot; - /// The Millau network ID, associated with Kusama. - pub const MillauNetwork: NetworkId = Kusama; + /// The Rialto network ID. + pub const ThisNetwork: NetworkId = CustomNetworkId::Rialto.as_network_id(); + /// The Millau network ID. + pub const MillauNetwork: NetworkId = CustomNetworkId::Millau.as_network_id(); /// Our XCM location ancestry - i.e. our location within the Consensus Universe. /// diff --git a/bridges/bin/runtime-common/src/lib.rs b/bridges/bin/runtime-common/src/lib.rs index 5b2cea8ddd7ff..a8f2434fc7d0b 100644 --- a/bridges/bin/runtime-common/src/lib.rs +++ b/bridges/bin/runtime-common/src/lib.rs @@ -20,6 +20,7 @@ use bp_runtime::FilterCall; use sp_runtime::transaction_validity::TransactionValidity; +use xcm::v3::NetworkId; pub mod messages; pub mod messages_api; @@ -119,6 +120,28 @@ macro_rules! generate_bridge_reject_obsolete_headers_and_messages { }; } +/// A mapping over `NetworkId`. +/// Since `NetworkId` doesn't include `Millau`, `Rialto` and `RialtoParachain`, we create some +/// synthetic associations between these chains and `NetworkId` chains. +pub enum CustomNetworkId { + /// The Millau network ID, associated with Kusama. + Millau, + /// The Rialto network ID, associated with Polkadot. + Rialto, + /// The RialtoParachain network ID, associated with Westend. + RialtoParachain, +} + +impl CustomNetworkId { + pub const fn as_network_id(&self) -> NetworkId { + match *self { + CustomNetworkId::Millau => NetworkId::Kusama, + CustomNetworkId::Rialto => NetworkId::Polkadot, + CustomNetworkId::RialtoParachain => NetworkId::Westend, + } + } +} + #[cfg(test)] mod tests { use crate::BridgeRuntimeFilterCall; diff --git a/bridges/bin/runtime-common/src/messages.rs b/bridges/bin/runtime-common/src/messages.rs index ef0722a80bce5..a7ad19dc1f168 100644 --- a/bridges/bin/runtime-common/src/messages.rs +++ b/bridges/bin/runtime-common/src/messages.rs @@ -569,7 +569,7 @@ pub mod source { } let route = T::build_destination(); - let msg = (route, msg.take().unwrap()).encode(); + let msg = (route, msg.take().ok_or(SendError::MissingArgument)?).encode(); let fee = estimate_message_dispatch_and_delivery_fee::( &msg, diff --git a/bridges/relays/bin-substrate/Cargo.toml b/bridges/relays/bin-substrate/Cargo.toml index 63ccb4f918e29..56d669e49735e 100644 --- a/bridges/relays/bin-substrate/Cargo.toml +++ b/bridges/relays/bin-substrate/Cargo.toml @@ -60,7 +60,6 @@ sp-version = { git = "https://github.com/paritytech/substrate", branch = "master # Polkadot Dependencies -#pallet-xcm = { git = "https://github.com/paritytech/polkadot", branch = "gav-xcm-v3", default-features = false } polkadot-parachain = { git = "https://github.com/paritytech/polkadot", branch = "gav-xcm-v3" } polkadot-primitives = { git = "https://github.com/paritytech/polkadot", branch = "gav-xcm-v3" } polkadot-runtime-common = { git = "https://github.com/paritytech/polkadot", branch = "gav-xcm-v3" } diff --git a/bridges/relays/bin-substrate/src/chains/millau.rs b/bridges/relays/bin-substrate/src/chains/millau.rs index 2cf0258d4325c..b71ecb288e9d0 100644 --- a/bridges/relays/bin-substrate/src/chains/millau.rs +++ b/bridges/relays/bin-substrate/src/chains/millau.rs @@ -34,35 +34,27 @@ impl CliEncodeMessage for Millau { message: xcm::VersionedXcm<()>, bridge_instance_index: u8, ) -> anyhow::Result> { - Ok(match bridge_instance_index { - bridge::MILLAU_TO_RIALTO_INDEX => { - let dest = - (Parent, X1(GlobalConsensus(millau_runtime::xcm_config::RialtoNetwork::get()))); - millau_runtime::Call::XcmPallet(millau_runtime::XcmCall::send { - dest: Box::new(dest.into()), - message: Box::new(message), - }) - .into() - }, - bridge::MILLAU_TO_RIALTO_PARACHAIN_INDEX => { - let dest = ( - Parent, - X2( - GlobalConsensus(millau_runtime::xcm_config::RialtoNetwork::get()), - Parachain(RIALTO_PARACHAIN_ID), - ), - ); - millau_runtime::Call::XcmPallet(millau_runtime::XcmCall::send { - dest: Box::new(dest.into()), - message: Box::new(message), - }) - .into() - }, + let dest = match bridge_instance_index { + bridge::MILLAU_TO_RIALTO_INDEX => + (Parent, X1(GlobalConsensus(millau_runtime::xcm_config::RialtoNetwork::get()))), + bridge::MILLAU_TO_RIALTO_PARACHAIN_INDEX => ( + Parent, + X2( + GlobalConsensus(millau_runtime::xcm_config::RialtoNetwork::get()), + Parachain(RIALTO_PARACHAIN_ID), + ), + ), _ => anyhow::bail!( "Unsupported target bridge pallet with instance index: {}", bridge_instance_index ), + }; + + Ok(millau_runtime::Call::XcmPallet(millau_runtime::XcmCall::send { + dest: Box::new(dest.into()), + message: Box::new(message), }) + .into()) } fn encode_send_message_call( diff --git a/bridges/relays/bin-substrate/src/chains/rialto.rs b/bridges/relays/bin-substrate/src/chains/rialto.rs index 2753a917ad24a..9dd86c9d95339 100644 --- a/bridges/relays/bin-substrate/src/chains/rialto.rs +++ b/bridges/relays/bin-substrate/src/chains/rialto.rs @@ -33,21 +33,20 @@ impl CliEncodeMessage for Rialto { message: xcm::VersionedXcm<()>, bridge_instance_index: u8, ) -> anyhow::Result> { - Ok(match bridge_instance_index { - bridge::RIALTO_TO_MILLAU_INDEX => { - let dest = - (Parent, X1(GlobalConsensus(rialto_runtime::xcm_config::MillauNetwork::get()))); - rialto_runtime::Call::XcmPallet(rialto_runtime::XcmCall::send { - dest: Box::new(dest.into()), - message: Box::new(message), - }) - .into() - }, + let dest = match bridge_instance_index { + bridge::RIALTO_TO_MILLAU_INDEX => + (Parent, X1(GlobalConsensus(rialto_runtime::xcm_config::MillauNetwork::get()))), _ => anyhow::bail!( "Unsupported target bridge pallet with instance index: {}", bridge_instance_index ), + }; + + Ok(rialto_runtime::Call::XcmPallet(rialto_runtime::XcmCall::send { + dest: Box::new(dest.into()), + message: Box::new(message), }) + .into()) } fn encode_send_message_call( diff --git a/bridges/relays/bin-substrate/src/chains/rialto_parachain.rs b/bridges/relays/bin-substrate/src/chains/rialto_parachain.rs index 09edeaecea02b..8cd7135c27a3c 100644 --- a/bridges/relays/bin-substrate/src/chains/rialto_parachain.rs +++ b/bridges/relays/bin-substrate/src/chains/rialto_parachain.rs @@ -33,23 +33,20 @@ impl CliEncodeMessage for RialtoParachain { message: xcm::VersionedXcm<()>, bridge_instance_index: u8, ) -> anyhow::Result> { - Ok(match bridge_instance_index { - bridge::RIALTO_PARACHAIN_TO_MILLAU_INDEX => { - let dest = - (Parent, X1(GlobalConsensus(rialto_parachain_runtime::MillauNetwork::get()))); - rialto_parachain_runtime::Call::PolkadotXcm( - rialto_parachain_runtime::XcmCall::send { - dest: Box::new(dest.into()), - message: Box::new(message), - }, - ) - .into() - }, + let dest = match bridge_instance_index { + bridge::RIALTO_PARACHAIN_TO_MILLAU_INDEX => + (Parent, X1(GlobalConsensus(rialto_parachain_runtime::MillauNetwork::get()))), _ => anyhow::bail!( "Unsupported target bridge pallet with instance index: {}", bridge_instance_index ), + }; + + Ok(rialto_parachain_runtime::Call::PolkadotXcm(rialto_parachain_runtime::XcmCall::send { + dest: Box::new(dest.into()), + message: Box::new(message), }) + .into()) } fn encode_send_message_call( diff --git a/bridges/relays/bin-substrate/src/cli/estimate_fee.rs b/bridges/relays/bin-substrate/src/cli/estimate_fee.rs index ed1e27bcf5f00..806df7f01f777 100644 --- a/bridges/relays/bin-substrate/src/cli/estimate_fee.rs +++ b/bridges/relays/bin-substrate/src/cli/estimate_fee.rs @@ -100,7 +100,7 @@ where data.conversion_rate_override, Self::ESTIMATE_MESSAGE_FEE_METHOD, lane, - payload, + &payload, ) .await?; @@ -141,7 +141,7 @@ pub(crate) async fn estimate_message_delivery_and_dispatch_fee< conversion_rate_override: Option, estimate_fee_method: &str, lane: bp_messages::LaneId, - payload: P, + payload: &P, ) -> anyhow::Result> { // actual conversion rate CAN be lesser than the rate stored in the runtime. So we may try to // pay lesser fee for the message delivery. But in this case, message may be rejected by the @@ -196,7 +196,7 @@ pub(crate) async fn estimate_message_delivery_and_dispatch_fee< client, estimate_fee_method, lane, - payload.clone(), + payload, None, ) .await?; @@ -204,7 +204,7 @@ pub(crate) async fn estimate_message_delivery_and_dispatch_fee< client, estimate_fee_method, lane, - payload.clone(), + payload, conversion_rate_override, ) .await?; @@ -227,7 +227,7 @@ async fn do_estimate_message_delivery_and_dispatch_fee client: &relay_substrate_client::Client, estimate_fee_method: &str, lane: bp_messages::LaneId, - payload: P, + payload: &P, conversion_rate_override: Option, ) -> anyhow::Result> { let encoded_response = client diff --git a/bridges/relays/bin-substrate/src/cli/send_message.rs b/bridges/relays/bin-substrate/src/cli/send_message.rs index 11625c3b8ea2a..a45235f0bec28 100644 --- a/bridges/relays/bin-substrate/src/cli/send_message.rs +++ b/bridges/relays/bin-substrate/src/cli/send_message.rs @@ -121,13 +121,13 @@ where conversion_rate_override, Self::ESTIMATE_MESSAGE_FEE_METHOD, lane, - payload.clone(), + &payload, ) .await? .into(), ), }; - let payload_len = payload.encode().len(); + let payload_len = payload.encoded_size(); let send_message_call = if data.use_xcm_pallet { Self::Source::encode_send_xcm( decode_xcm(payload)?,