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

xcm: Fixes for UnpaidLocalExporter #7126

Merged
merged 8 commits into from
Jan 13, 2025
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
6 changes: 4 additions & 2 deletions polkadot/xcm/xcm-builder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,13 @@ pub use routing::{
mod transactional;
pub use transactional::FrameTransactionalProcessor;

#[allow(deprecated)]
pub use universal_exports::UnpaidLocalExporter;
mod universal_exports;
pub use universal_exports::{
ensure_is_remote, BridgeBlobDispatcher, BridgeMessage, DispatchBlob, DispatchBlobError,
ExporterFor, HaulBlob, HaulBlobError, HaulBlobExporter, NetworkExportTable,
NetworkExportTableItem, SovereignPaidRemoteExporter, UnpaidLocalExporter, UnpaidRemoteExporter,
ExporterFor, HaulBlob, HaulBlobError, HaulBlobExporter, LocalExporter, NetworkExportTable,
NetworkExportTableItem, SovereignPaidRemoteExporter, UnpaidRemoteExporter,
};

mod weight;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ parameter_types! {
type TheBridge =
TestBridge<BridgeBlobDispatcher<TestRemoteIncomingRouter, RemoteUniversalLocation, ()>>;
type Router = TestTopic<
UnpaidLocalExporter<
LocalExporter<
HaulBlobExporter<TheBridge, RemoteNetwork, AlwaysLatest, Price>,
UniversalLocation,
>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ parameter_types! {
type TheBridge =
TestBridge<BridgeBlobDispatcher<TestRemoteIncomingRouter, RemoteUniversalLocation, ()>>;
type Router = TestTopic<
UnpaidLocalExporter<
LocalExporter<
HaulBlobExporter<TheBridge, RemoteNetwork, AlwaysLatest, Price>,
UniversalLocation,
>,
Expand Down
2 changes: 1 addition & 1 deletion polkadot/xcm/xcm-builder/src/tests/bridging/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ impl<Local: Get<Junctions>, Remote: Get<Junctions>, RemoteExporter: ExportXcm> S
let origin = Local::get().relative_to(&Remote::get());
AllowUnpaidFrom::set(vec![origin.clone()]);
set_exporter_override(price::<RemoteExporter>, deliver::<RemoteExporter>);
// The we execute it:
// Then we execute it:
let mut id = fake_id();
let outcome = XcmExecutor::<TestConfig>::prepare_and_execute(
origin,
Expand Down
62 changes: 57 additions & 5 deletions polkadot/xcm/xcm-builder/src/universal_exports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

//! Traits and utilities to help with origin mutation and bridging.

#![allow(deprecated)]

use crate::InspectMessageQueues;
use alloc::{vec, vec::Vec};
use codec::{Decode, Encode};
Expand Down Expand Up @@ -58,6 +60,8 @@ pub fn ensure_is_remote(
/// that the message sending cannot be abused in any way.
///
/// This is only useful when the local chain has bridging capabilities.
#[deprecated(note = "Will be removed after July 2025; It uses hard-coded channel `0`, \
use `xcm_builder::LocalExporter` directly instead.")]
pub struct UnpaidLocalExporter<Exporter, UniversalLocation>(
PhantomData<(Exporter, UniversalLocation)>,
);
Expand Down Expand Up @@ -100,6 +104,54 @@ impl<Exporter: ExportXcm, UniversalLocation: Get<InteriorLocation>> SendXcm
fn ensure_successful_delivery(_: Option<Location>) {}
}

/// Implementation of `SendXcm` which uses the given `ExportXcm` implementation in order to forward
/// the message over a bridge.
///
/// This is only useful when the local chain has bridging capabilities.
pub struct LocalExporter<Exporter, UniversalLocation>(PhantomData<(Exporter, UniversalLocation)>);
impl<Exporter: ExportXcm, UniversalLocation: Get<InteriorLocation>> SendXcm
for LocalExporter<Exporter, UniversalLocation>
{
type Ticket = Exporter::Ticket;

fn validate(
dest: &mut Option<Location>,
msg: &mut Option<Xcm<()>>,
) -> SendResult<Exporter::Ticket> {
// This `clone` ensures that `dest` is not consumed in any case.
let d = dest.clone().take().ok_or(MissingArgument)?;
let universal_source = UniversalLocation::get();
let devolved = ensure_is_remote(universal_source.clone(), d).map_err(|_| NotApplicable)?;
let (remote_network, remote_location) = devolved;
let xcm = msg.take().ok_or(MissingArgument)?;

let hash =
(Some(Location::here()), &remote_location).using_encoded(sp_io::hashing::blake2_128);
let channel = u32::decode(&mut hash.as_ref()).unwrap_or(0);

validate_export::<Exporter>(
remote_network,
channel,
universal_source,
remote_location,
xcm.clone(),
)
.inspect_err(|err| {
if let NotApplicable = err {
// We need to make sure that msg is not consumed in case of `NotApplicable`.
*msg = Some(xcm);
}
})
}

fn deliver(ticket: Exporter::Ticket) -> Result<XcmHash, SendError> {
Exporter::deliver(ticket)
}

#[cfg(feature = "runtime-benchmarks")]
fn ensure_successful_delivery(_: Option<Location>) {}
}

pub trait ExporterFor {
/// Return the locally-routable bridge (if any) capable of forwarding `message` to the
/// `remote_location` on the remote `network`, together with the payment which is required.
Expand Down Expand Up @@ -703,24 +755,24 @@ mod tests {
let local_dest: Location = (Parent, Parachain(5678)).into();
assert!(ensure_is_remote(UniversalLocation::get(), local_dest.clone()).is_err());

// UnpaidLocalExporter
// LocalExporter
ensure_validate_does_not_consume_dest_or_msg::<
UnpaidLocalExporter<RoutableBridgeExporter, UniversalLocation>,
LocalExporter<RoutableBridgeExporter, UniversalLocation>,
>(local_dest.clone(), |result| assert_eq!(Err(NotApplicable), result));

// 2. check with not applicable from the inner router (using `NotApplicableBridgeSender`)
let remote_dest: Location =
(Parent, Parent, DifferentRemote::get(), RemoteDestination::get()).into();
assert!(ensure_is_remote(UniversalLocation::get(), remote_dest.clone()).is_ok());

// UnpaidLocalExporter
// LocalExporter
ensure_validate_does_not_consume_dest_or_msg::<
UnpaidLocalExporter<NotApplicableBridgeExporter, UniversalLocation>,
LocalExporter<NotApplicableBridgeExporter, UniversalLocation>,
>(remote_dest.clone(), |result| assert_eq!(Err(NotApplicable), result));

// 3. Ok - deliver
// UnpaidRemoteExporter
assert_ok!(send_xcm::<UnpaidLocalExporter<RoutableBridgeExporter, UniversalLocation>>(
assert_ok!(send_xcm::<LocalExporter<RoutableBridgeExporter, UniversalLocation>>(
remote_dest,
Xcm::default()
));
Expand Down
7 changes: 7 additions & 0 deletions prdoc/pr_7126.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
title: 'xcm: Fixes for `UnpaidLocalExporter`'
doc:
- audience: Runtime Dev
description: This PR deprecates `UnpaidLocalExporter` in favor of the new `LocalExporter`. First, the name is misleading, as it can be used in both paid and unpaid scenarios. Second, it contains a hard-coded channel 0, whereas `LocalExporter` uses the same algorithm as `xcm-exporter`.
crates:
- name: staging-xcm-builder
bump: minor
Loading