From e0b42dfae7381c52bd46ac6bccc8b4f5bb167a9b 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 19fb68b662..57f28e1404 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 e96268d8aa..5cc577281f 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 63b2d0bead..dbe8ad81c4 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 5b2cea8ddd..a8f2434fc7 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 ef0722a80b..a7ad19dc1f 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 63ccb4f918..56d669e497 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 2cf0258d43..b71ecb288e 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 2753a917ad..9dd86c9d95 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 09edeaecea..8cd7135c27 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 ed1e27bcf5..806df7f01f 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 11625c3b8e..a45235f0be 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)?,