From 751635463379e2c5caa87e6db82eb31320a58841 Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Wed, 17 May 2023 15:56:37 +0200 Subject: [PATCH] [xcm] Extend `haul_blob` with Channel + include `NetworkId` to channel (`origin/(network, destination)`) --- xcm/xcm-builder/src/tests/bridging/mod.rs | 4 +- xcm/xcm-builder/src/tests/origins.rs | 2 +- xcm/xcm-builder/src/universal_exports.rs | 16 +++--- xcm/xcm-executor/src/lib.rs | 15 ++---- xcm/xcm-executor/src/traits/export.rs | 62 +++++++++++++++++++++-- xcm/xcm-executor/src/traits/mod.rs | 2 +- 6 files changed, 75 insertions(+), 26 deletions(-) diff --git a/xcm/xcm-builder/src/tests/bridging/mod.rs b/xcm/xcm-builder/src/tests/bridging/mod.rs index 194e171cb620..28901cc29ee4 100644 --- a/xcm/xcm-builder/src/tests/bridging/mod.rs +++ b/xcm/xcm-builder/src/tests/bridging/mod.rs @@ -21,7 +21,7 @@ use crate::universal_exports::*; use frame_support::{parameter_types, traits::Get}; use std::{cell::RefCell, marker::PhantomData}; use xcm_executor::{ - traits::{export_xcm, validate_export}, + traits::{export_xcm, validate_export, Channel}, XcmExecutor, }; use SendError::*; @@ -51,7 +51,7 @@ impl TestBridge { } } impl HaulBlob for TestBridge { - fn haul_blob(blob: Vec) -> Result<(), HaulBlobError> { + fn haul_blob(blob: Vec, _channel: Channel) -> Result<(), HaulBlobError> { BRIDGE_TRAFFIC.with(|t| t.borrow_mut().push(blob)); Ok(()) } diff --git a/xcm/xcm-builder/src/tests/origins.rs b/xcm/xcm-builder/src/tests/origins.rs index d3d6278eff8e..a261decbf65f 100644 --- a/xcm/xcm-builder/src/tests/origins.rs +++ b/xcm/xcm-builder/src/tests/origins.rs @@ -94,7 +94,7 @@ fn export_message_should_work() { let uni_src = (ByGenesis([0; 32]), Parachain(42), Parachain(1)).into(); assert_eq!( exported_xcm(), - vec![(Polkadot, 403611790, uni_src, Here, expected_message, expected_hash)] + vec![(Polkadot, 470110423, uni_src, Here, expected_message, expected_hash)] ); } diff --git a/xcm/xcm-builder/src/universal_exports.rs b/xcm/xcm-builder/src/universal_exports.rs index 2d1c9f6d4c35..b44d509d8130 100644 --- a/xcm/xcm-builder/src/universal_exports.rs +++ b/xcm/xcm-builder/src/universal_exports.rs @@ -20,7 +20,7 @@ use frame_support::{ensure, traits::Get}; use parity_scale_codec::{Decode, Encode}; use sp_std::{convert::TryInto, marker::PhantomData, prelude::*}; use xcm::prelude::*; -use xcm_executor::traits::{validate_export, ExportXcm}; +use xcm_executor::traits::{validate_export, Channel, ExportXcm}; use SendError::*; /// Returns the network ID and consensus location within that network of the remote @@ -249,7 +249,7 @@ pub trait DispatchBlob { pub trait HaulBlob { /// Sends a blob over some point-to-point link. This will generally be implemented by a bridge. - fn haul_blob(blob: Vec) -> Result<(), HaulBlobError>; + fn haul_blob(blob: Vec, channel: Channel) -> Result<(), HaulBlobError>; } #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -319,15 +319,15 @@ pub struct HaulBlobExporter( impl, Price: Get> ExportXcm for HaulBlobExporter { - type Ticket = (Vec, XcmHash); + type Ticket = (Vec, XcmHash, Channel); fn validate( network: NetworkId, - _channel: u32, + channel: Channel, universal_source: &mut Option, destination: &mut Option, message: &mut Option>, - ) -> Result<((Vec, XcmHash), MultiAssets), SendError> { + ) -> Result<(Self::Ticket, MultiAssets), SendError> { let bridged_network = BridgedNetwork::get(); ensure!(&network == &bridged_network, SendError::NotApplicable); // We don't/can't use the `channel` for this adapter. @@ -354,11 +354,11 @@ impl, Price: Get> let message = VersionedXcm::from(inner); let hash = message.using_encoded(sp_io::hashing::blake2_256); let blob = BridgeMessage { universal_dest, message }.encode(); - Ok(((blob, hash), Price::get())) + Ok(((blob, hash, channel), Price::get())) } - fn deliver((blob, hash): (Vec, XcmHash)) -> Result { - Bridge::haul_blob(blob)?; + fn deliver((blob, hash, channel): Self::Ticket) -> Result { + Bridge::haul_blob(blob, channel)?; Ok(hash) } } diff --git a/xcm/xcm-executor/src/lib.rs b/xcm/xcm-executor/src/lib.rs index 749a63114d95..169fa3601106 100644 --- a/xcm/xcm-executor/src/lib.rs +++ b/xcm/xcm-executor/src/lib.rs @@ -21,18 +21,17 @@ use frame_support::{ ensure, traits::{Contains, ContainsPair, Get, PalletsInfoAccess}, }; -use parity_scale_codec::{Decode, Encode}; +use parity_scale_codec::Encode; use sp_core::defer; -use sp_io::hashing::blake2_128; use sp_std::{marker::PhantomData, prelude::*}; use sp_weights::Weight; use xcm::latest::prelude::*; pub mod traits; use traits::{ - validate_export, AssetExchange, AssetLock, CallDispatcher, ClaimAssets, ConvertOrigin, - DropAssets, Enact, ExportXcm, FeeManager, FeeReason, OnResponse, ShouldExecute, TransactAsset, - VersionChangeNotifier, WeightBounds, WeightTrader, + channel_from_params, validate_export, AssetExchange, AssetLock, CallDispatcher, ClaimAssets, + ConvertOrigin, DropAssets, Enact, ExportXcm, FeeManager, FeeReason, OnResponse, ShouldExecute, + TransactAsset, VersionChangeNotifier, WeightBounds, WeightTrader, }; mod assets; @@ -831,11 +830,7 @@ impl XcmExecutor { let universal_source = Config::UniversalLocation::get() .within_global(origin) .map_err(|()| XcmError::Unanchored)?; - let hash = (self.origin_ref(), &destination).using_encoded(blake2_128); - let channel = u32::decode(&mut hash.as_ref()).unwrap_or(0); - // Hash identifies the lane on the exporter which we use. We use the pairwise - // combination of the origin and destination to ensure origin/destination pairs will - // generally have their own lanes. + let channel = channel_from_params(self.origin_ref(), &network, &destination); let (ticket, fee) = validate_export::( network, channel, diff --git a/xcm/xcm-executor/src/traits/export.rs b/xcm/xcm-executor/src/traits/export.rs index 39667cec48ca..ffbac9d665ea 100644 --- a/xcm/xcm-executor/src/traits/export.rs +++ b/xcm/xcm-executor/src/traits/export.rs @@ -16,6 +16,24 @@ use xcm::latest::prelude::*; +/// An identifier for a channel. +pub type Channel = u32; + +/// Generates channel identifier for `origin/(network, destination)` combination to ensure using separated lanes. +pub fn channel_from_params( + origin_ref: Option<&MultiLocation>, + network: &NetworkId, + destination: &InteriorMultiLocation, +) -> Channel { + use parity_scale_codec::{Decode, Encode}; + // Hash identifies the lane on the exporter which we use. We use the pairwise + // combination of the origin and (network, destination) to ensure origin/(network, destination) pairs will + // generally have their own lanes. + let hash = (origin_ref, (network, destination)).using_encoded(sp_io::hashing::blake2_256); + let channel = u32::decode(&mut hash.as_ref()).unwrap_or(0); + channel +} + /// Utility for delivering a message to a system under a different (non-local) consensus with a /// spoofed origin. This essentially defines the behaviour of the `ExportMessage` XCM instruction. /// @@ -50,7 +68,7 @@ pub trait ExportXcm { /// early without trying alternative means of delivery. fn validate( network: NetworkId, - channel: u32, + channel: Channel, universal_source: &mut Option, destination: &mut Option, message: &mut Option>, @@ -70,7 +88,7 @@ impl ExportXcm for Tuple { fn validate( network: NetworkId, - channel: u32, + channel: Channel, universal_source: &mut Option, destination: &mut Option, message: &mut Option>, @@ -111,7 +129,7 @@ impl ExportXcm for Tuple { /// both in `Some` before passing them as as mutable references into `T::send_xcm`. pub fn validate_export( network: NetworkId, - channel: u32, + channel: Channel, universal_source: InteriorMultiLocation, dest: InteriorMultiLocation, msg: Xcm<()>, @@ -129,7 +147,7 @@ pub fn validate_export( /// before actually doing the delivery. pub fn export_xcm( network: NetworkId, - channel: u32, + channel: Channel, universal_source: InteriorMultiLocation, dest: InteriorMultiLocation, msg: Xcm<()>, @@ -144,3 +162,39 @@ pub fn export_xcm( let hash = T::deliver(ticket)?; Ok((hash, price)) } + +#[cfg(test)] +mod tests { + use super::*; + use std::collections::HashSet; + + #[test] + fn channel_from_params_works() { + let params: Vec<(Option, &NetworkId, &InteriorMultiLocation)> = vec![ + // different interior + (None, &ByGenesis([0; 32]), &X1(Parachain(1234))), + (None, &ByGenesis([0; 32]), &X1(Parachain(1235))), + // different networkId + (None, &ByGenesis([0; 32]), &X1(Parachain(1236))), + (None, &ByGenesis([1; 32]), &X1(Parachain(1236))), + // different origin + (None, &ByGenesis([0; 32]), &X1(Parachain(1237))), + (Some(MultiLocation::here()), &ByGenesis([0; 32]), &X1(Parachain(1237))), + (Some(MultiLocation::parent()), &ByGenesis([0; 32]), &X1(Parachain(1237))), + ]; + + // check unique + let channels: HashSet = + HashSet::from_iter(params.iter().map(|(origin, network, interior)| { + channel_from_params(origin.as_ref(), network, interior) + })); + assert_eq!(params.len(), channels.len()); + + // check not random + assert_eq!( + channel_from_params(None, &ByGenesis([0; 32]), &X1(Parachain(1234))), + channel_from_params(None, &ByGenesis([0; 32]), &X1(Parachain(1234))) + ); + assert_eq!(channel_from_params(Some(&Parachain(1).into()), &Polkadot, &Here), 470110423); + } +} diff --git a/xcm/xcm-executor/src/traits/mod.rs b/xcm/xcm-executor/src/traits/mod.rs index 64c77f2abf45..81246f8ae4c1 100644 --- a/xcm/xcm-executor/src/traits/mod.rs +++ b/xcm/xcm-executor/src/traits/mod.rs @@ -27,7 +27,7 @@ pub use asset_lock::{AssetLock, Enact, LockError}; mod asset_exchange; pub use asset_exchange::AssetExchange; mod export; -pub use export::{export_xcm, validate_export, ExportXcm}; +pub use export::{channel_from_params, export_xcm, validate_export, Channel, ExportXcm}; mod fee_manager; pub use fee_manager::{FeeManager, FeeReason}; mod filter_asset_location;