Follow-up on paritytech#1518 (paritytech#1546)

* Adjustments for the xcm messages sending logic

Signed-off-by: Serban Iorga <serban@parity.io>

* Deduplicate XCM destination

Signed-off-by: Serban Iorga <serban@parity.io>

* [send_message] small changes

Signed-off-by: Serban Iorga <serban@parity.io>

* 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 <serban@parity.io>

* Revert "Deduplicate XCM destination"

This reverts commit 3a0a950e1d7484e3ecac45f5c00b152f0485cd11.

Signed-off-by: Serban Iorga <serban@parity.io>

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.
 	///

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

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.
 	///

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;

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::<T::MessageBridge>(
 				&msg,

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" }

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<EncodedOrDecodedCall<Self::Call>> {
-		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(

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<EncodedOrDecodedCall<Self::Call>> {
-		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(

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<EncodedOrDecodedCall<Self::Call>> {
-		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(

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<ConversionRateOverride>,
 	estimate_fee_method: &str,
 	lane: bp_messages::LaneId,
-	payload: P,
+	payload: &P,
 ) -> anyhow::Result<BalanceOf<Source>> {
 	// 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,15 +196,15 @@ pub(crate) async fn estimate_message_delivery_and_dispatch_fee<
 		client,
 		estimate_fee_method,
 		lane,
-		payload.clone(),
+		payload,
 		None,
 	)
 	.await?;
 	let with_override = do_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<Source: Chain, P: Encode>
 	client: &relay_substrate_client::Client<Source>,
 	estimate_fee_method: &str,
 	lane: bp_messages::LaneId,
-	payload: P,
+	payload: &P,
 	conversion_rate_override: Option<FixedU128>,
 ) -> anyhow::Result<BalanceOf<Source>> {
 	let encoded_response = client

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)?,