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

Ensure outbound XCMs are decodable with limits + add EnsureDecodableXcm router (for testing purposes) #4186

Merged
merged 8 commits into from
Apr 23, 2024
8 changes: 0 additions & 8 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 0 additions & 10 deletions polkadot/runtime/test-runtime/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,10 @@ license.workspace = true
workspace = true

[dependencies]
bitvec = { version = "1.0.0", default-features = false, features = ["alloc"] }
parity-scale-codec = { version = "3.6.1", default-features = false, features = ["derive"] }
log = { workspace = true }
rustc-hex = { version = "2.1.0", default-features = false }
scale-info = { version = "2.11.1", default-features = false, features = ["derive"] }
serde = { workspace = true }
serde_derive = { optional = true, workspace = true }
smallvec = "1.8.0"

authority-discovery-primitives = { package = "sp-authority-discovery", path = "../../../substrate/primitives/authority-discovery", default-features = false }
babe-primitives = { package = "sp-consensus-babe", path = "../../../substrate/primitives/consensus/babe", default-features = false }
Expand Down Expand Up @@ -63,7 +59,6 @@ pallet-vesting = { path = "../../../substrate/frame/vesting", default-features =
runtime-common = { package = "polkadot-runtime-common", path = "../common", default-features = false }
primitives = { package = "polkadot-primitives", path = "../../primitives", default-features = false }
pallet-xcm = { path = "../../xcm/pallet-xcm", default-features = false }
polkadot-parachain-primitives = { path = "../../parachain", default-features = false }
polkadot-runtime-parachains = { path = "../parachains", default-features = false }
xcm-builder = { package = "staging-xcm-builder", path = "../../xcm/xcm-builder", default-features = false }
xcm-executor = { package = "staging-xcm-executor", path = "../../xcm/xcm-executor", default-features = false }
Expand Down Expand Up @@ -92,7 +87,6 @@ std = [
"authority-discovery-primitives/std",
"babe-primitives/std",
"beefy-primitives/std",
"bitvec/std",
"block-builder-api/std",
"frame-election-provider-support/std",
"frame-executive/std",
Expand All @@ -118,14 +112,11 @@ std = [
"pallet-vesting/std",
"pallet-xcm/std",
"parity-scale-codec/std",
"polkadot-parachain-primitives/std",
"polkadot-runtime-parachains/std",
"primitives/std",
"runtime-common/std",
"rustc-hex/std",
"scale-info/std",
"serde/std",
"serde_derive",
"sp-api/std",
"sp-core/std",
"sp-genesis-builder/std",
Expand Down Expand Up @@ -157,7 +148,6 @@ runtime-benchmarks = [
"pallet-timestamp/runtime-benchmarks",
"pallet-vesting/runtime-benchmarks",
"pallet-xcm/runtime-benchmarks",
"polkadot-parachain-primitives/runtime-benchmarks",
"polkadot-runtime-parachains/runtime-benchmarks",
"primitives/runtime-benchmarks",
"runtime-common/runtime-benchmarks",
Expand Down
6 changes: 0 additions & 6 deletions polkadot/runtime/test-runtime/constants/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,12 @@ smallvec = "1.8.0"

frame-support = { path = "../../../../substrate/frame/support", default-features = false }
primitives = { package = "polkadot-primitives", path = "../../../primitives", default-features = false }
runtime-common = { package = "polkadot-runtime-common", path = "../../common", default-features = false }
sp-runtime = { path = "../../../../substrate/primitives/runtime", default-features = false }
sp-weights = { path = "../../../../substrate/primitives/weights", default-features = false }
sp-core = { path = "../../../../substrate/primitives/core", default-features = false }

[features]
default = ["std"]
std = [
"frame-support/std",
"primitives/std",
"runtime-common/std",
"sp-core/std",
"sp-runtime/std",
"sp-weights/std",
]
10 changes: 6 additions & 4 deletions polkadot/runtime/test-runtime/src/xcm_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ use polkadot_runtime_parachains::FeeTracker;
use runtime_common::xcm_sender::{ChildParachainRouter, PriceForMessageDelivery};
use xcm::latest::prelude::*;
use xcm_builder::{
AllowUnpaidExecutionFrom, EnsureXcmOrigin, FixedWeightBounds, FrameTransactionalProcessor,
SignedAccountId32AsNative, SignedToAccountId32, WithUniqueTopic,
AllowUnpaidExecutionFrom, EnsureDecodableXcm, EnsureXcmOrigin, FixedWeightBounds,
FrameTransactionalProcessor, SignedAccountId32AsNative, SignedToAccountId32, WithUniqueTopic,
};
use xcm_executor::{
traits::{TransactAsset, WeightTrader},
Expand Down Expand Up @@ -82,8 +82,10 @@ pub type PriceForChildParachainDelivery = TestDeliveryPrice<FeeAssetId, super::D
/// The XCM router. When we want to send an XCM message, we use this type. It amalgamates all of our
/// individual routers.
pub type XcmRouter = WithUniqueTopic<
// Only one router so far - use DMP to communicate with child parachains.
ChildParachainRouter<super::Runtime, super::Xcm, PriceForChildParachainDelivery>,
EnsureDecodableXcm<
// Only one router so far - use DMP to communicate with child parachains.
ChildParachainRouter<super::Runtime, super::Xcm, PriceForChildParachainDelivery>,
>,
>;

pub type Barrier = AllowUnpaidExecutionFrom<Everything>;
Expand Down
6 changes: 4 additions & 2 deletions polkadot/xcm/pallet-xcm-benchmarks/src/fungible/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ use frame_support::{
traits::{Everything, Nothing},
};
use xcm::latest::prelude::*;
use xcm_builder::{AllowUnpaidExecutionFrom, FrameTransactionalProcessor, MintLocation};
use xcm_builder::{
AllowUnpaidExecutionFrom, EnsureDecodableXcm, FrameTransactionalProcessor, MintLocation,
};

type Block = frame_system::mocking::MockBlock<Test>;

Expand Down Expand Up @@ -91,7 +93,7 @@ parameter_types! {
pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = RuntimeCall;
type XcmSender = DevNull;
type XcmSender = EnsureDecodableXcm<DevNull>;
type AssetTransactor = AssetTransactor;
type OriginConverter = ();
type IsReserve = TrustedReserves;
Expand Down
5 changes: 3 additions & 2 deletions polkadot/xcm/pallet-xcm-benchmarks/src/generic/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ use xcm_builder::{
AssetsInHolding, TestAssetExchanger, TestAssetLocker, TestAssetTrap,
TestSubscriptionService, TestUniversalAliases,
},
AliasForeignAccountId32, AllowUnpaidExecutionFrom, FrameTransactionalProcessor,
AliasForeignAccountId32, AllowUnpaidExecutionFrom, EnsureDecodableXcm,
FrameTransactionalProcessor,
};
use xcm_executor::traits::ConvertOrigin;

Expand Down Expand Up @@ -81,7 +82,7 @@ type Aliasers = AliasForeignAccountId32<OnlyParachains>;
pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = RuntimeCall;
type XcmSender = DevNull;
type XcmSender = EnsureDecodableXcm<DevNull>;
type AssetTransactor = NoAssetTransactor;
type OriginConverter = AlwaysSignedByDefault<RuntimeOrigin>;
type IsReserve = AllAssetLocationsPass;
Expand Down
12 changes: 7 additions & 5 deletions polkadot/xcm/pallet-xcm/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,11 @@ use xcm::prelude::*;
use xcm_builder::{
AccountId32Aliases, AllowKnownQueryResponses, AllowSubscriptionsFrom,
AllowTopLevelPaidExecutionFrom, Case, ChildParachainAsNative, ChildParachainConvertsVia,
ChildSystemParachainAsSuperuser, DescribeAllTerminal, FixedRateOfFungible, FixedWeightBounds,
FrameTransactionalProcessor, FungibleAdapter, FungiblesAdapter, HashedDescription, IsConcrete,
MatchedConvertedConcreteId, NoChecking, SignedAccountId32AsNative, SignedToAccountId32,
SovereignSignedViaLocation, TakeWeightCredit, XcmFeeManagerFromComponents, XcmFeeToAccount,
ChildSystemParachainAsSuperuser, DescribeAllTerminal, EnsureDecodableXcm, FixedRateOfFungible,
FixedWeightBounds, FrameTransactionalProcessor, FungibleAdapter, FungiblesAdapter,
HashedDescription, IsConcrete, MatchedConvertedConcreteId, NoChecking,
SignedAccountId32AsNative, SignedToAccountId32, SovereignSignedViaLocation, TakeWeightCredit,
XcmFeeManagerFromComponents, XcmFeeToAccount,
};
use xcm_executor::{
traits::{Identity, JustTry},
Expand Down Expand Up @@ -488,7 +489,8 @@ pub type Barrier = (
AllowSubscriptionsFrom<Everything>,
);

pub type XcmRouter = (TestPaidForPara3000SendXcm, TestSendXcmErrX8, TestSendXcm);
pub type XcmRouter =
EnsureDecodableXcm<(TestPaidForPara3000SendXcm, TestSendXcmErrX8, TestSendXcm)>;

pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
Expand Down
2 changes: 1 addition & 1 deletion polkadot/xcm/xcm-builder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ mod process_xcm_message;
pub use process_xcm_message::ProcessXcmMessage;

mod routing;
pub use routing::{EnsureDelivery, WithTopicSource, WithUniqueTopic};
pub use routing::{EnsureDecodableXcm, EnsureDelivery, WithTopicSource, WithUniqueTopic};

mod transactional;
pub use transactional::FrameTransactionalProcessor;
Expand Down
66 changes: 66 additions & 0 deletions polkadot/xcm/xcm-builder/src/routing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,3 +139,69 @@ impl EnsureDelivery for Tuple {
(None, None)
}
}

/// A wrapper router that attempts to *encode* and *decode* passed XCM `message` to ensure that the
/// receiving side will be able to decode, at least with the same XCM version.
///
/// This is designed to be at the top-level of any routers which do the real delivery. While other
/// routers can manipulate the `message`, we cannot access the final XCM due to the generic
/// `Inner::Ticket`. Therefore, this router aims to validate at least the passed `message`.
///
/// NOTE: For use in mock runtimes which don't have the DMP/UMP/HRMP XCM validations.
acatangiu marked this conversation as resolved.
Show resolved Hide resolved
pub struct EnsureDecodableXcm<Inner>(sp_std::marker::PhantomData<Inner>);
impl<Inner: SendXcm> SendXcm for EnsureDecodableXcm<Inner> {
type Ticket = Inner::Ticket;

fn validate(
destination: &mut Option<Location>,
message: &mut Option<Xcm<()>>,
) -> SendResult<Self::Ticket> {
if let Some(msg) = message {
if let Some(e) = Self::ensure_encode_decode(msg) {
acatangiu marked this conversation as resolved.
Show resolved Hide resolved
return Err(e)
}
}
Inner::validate(destination, message)
}

fn deliver(ticket: Self::Ticket) -> Result<XcmHash, SendError> {
Inner::deliver(ticket)
}
}
impl<Inner> EnsureDecodableXcm<Inner> {
fn ensure_encode_decode(xcm: &Xcm<()>) -> Option<SendError> {
use parity_scale_codec::Decode;
let encoded = xcm.encode();
match Xcm::<()>::decode(&mut &encoded[..]) {
Ok(decoded_xcm) => match decoded_xcm.eq(xcm) {
bkontur marked this conversation as resolved.
Show resolved Hide resolved
true => None,
false => {
log::error!(target: "xcm::test_utils", "EnsureDecodableXcm `decoded_xcm` does not match `xcm` - \ndecoded_xcm: {decoded_xcm:?} \nxcm:{xcm:?}");
Some(SendError::Transport("Decoded xcm does not match xcm!"))
},
},
Err(e) => {
log::error!(target: "xcm::test_utils", "EnsureDecodableXcm decode error: {e:?} occurred for xcm: {xcm:?}");
Some(SendError::Transport("Failed to encode/decode xcm!"))
},
}
}
}

#[test]
fn ensure_encode_decode_works() {
let good_fun = vec![(Here, 10).into(), (Parent, 10).into()];
assert!(Assets::from_sorted_and_deduplicated(good_fun.clone()).is_ok());

let good_xcm = Xcm(vec![ReserveAssetDeposited(Assets::from(good_fun.clone()))]);
assert!(EnsureDecodableXcm::<()>::ensure_encode_decode(&good_xcm).is_none());

let bad_xcm = Xcm(vec![
ReserveAssetDeposited(Assets::from(good_fun));
(xcm::latest::MAX_INSTRUCTIONS_TO_DECODE + 1) as usize
]);
assert_eq!(
EnsureDecodableXcm::<()>::ensure_encode_decode(&bad_xcm),
Some(SendError::Transport("Failed to encode/decode xcm!"))
);
}
7 changes: 5 additions & 2 deletions polkadot/xcm/xcm-builder/src/tests/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use crate::{
barriers::{AllowSubscriptionsFrom, RespectSuspension, TrailingSetTopicAsId},
test_utils::*,
EnsureDecodableXcm,
};
pub use crate::{
AliasForeignAccountId32, AllowExplicitUnpaidExecutionFrom, AllowKnownQueryResponses,
Expand Down Expand Up @@ -165,8 +166,8 @@ pub fn set_exporter_override(
pub fn clear_exporter_override() {
EXPORTER_OVERRIDE.with(|x| x.replace(None));
}
pub struct TestMessageSender;
impl SendXcm for TestMessageSender {
pub struct TestMessageSenderImpl;
impl SendXcm for TestMessageSenderImpl {
type Ticket = (Location, Xcm<()>, XcmHash);
fn validate(
dest: &mut Option<Location>,
Expand All @@ -183,6 +184,8 @@ impl SendXcm for TestMessageSender {
Ok(hash)
}
}
pub type TestMessageSender = EnsureDecodableXcm<TestMessageSenderImpl>;

pub struct TestMessageExporter;
impl ExportXcm for TestMessageExporter {
type Ticket = (NetworkId, u32, InteriorLocation, InteriorLocation, Xcm<()>, XcmHash);
Expand Down
12 changes: 7 additions & 5 deletions polkadot/xcm/xcm-builder/tests/mock/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ use staging_xcm_builder as xcm_builder;
use xcm_builder::{
AccountId32Aliases, AllowTopLevelPaidExecutionFrom, AllowUnpaidExecutionFrom,
ChildParachainAsNative, ChildParachainConvertsVia, ChildSystemParachainAsSuperuser,
FixedRateOfFungible, FixedWeightBounds, FungibleAdapter, IsChildSystemParachain, IsConcrete,
MintLocation, RespectSuspension, SignedAccountId32AsNative, SignedToAccountId32,
SovereignSignedViaLocation, TakeWeightCredit,
EnsureDecodableXcm, FixedRateOfFungible, FixedWeightBounds, FungibleAdapter,
IsChildSystemParachain, IsConcrete, MintLocation, RespectSuspension, SignedAccountId32AsNative,
SignedToAccountId32, SovereignSignedViaLocation, TakeWeightCredit,
};

pub type AccountId = AccountId32;
Expand Down Expand Up @@ -68,6 +68,8 @@ impl SendXcm for TestSendXcm {
}
}

pub type TestXcmRouter = EnsureDecodableXcm<TestSendXcm>;

// copied from kusama constants
pub const UNITS: Balance = 1_000_000_000_000;
pub const CENTS: Balance = UNITS / 30_000;
Expand Down Expand Up @@ -180,7 +182,7 @@ pub type TrustedTeleporters = (xcm_builder::Case<KusamaForAssetHub>,);
pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = RuntimeCall;
type XcmSender = TestSendXcm;
type XcmSender = TestXcmRouter;
type AssetTransactor = LocalAssetTransactor;
type OriginConverter = LocalOriginConverter;
type IsReserve = ();
Expand Down Expand Up @@ -215,7 +217,7 @@ impl pallet_xcm::Config for Runtime {
type RuntimeEvent = RuntimeEvent;
type UniversalLocation = UniversalLocation;
type SendXcmOrigin = xcm_builder::EnsureXcmOrigin<RuntimeOrigin, LocalOriginToLocation>;
type XcmRouter = TestSendXcm;
type XcmRouter = TestXcmRouter;
// Anyone can execute XCM messages locally...
type ExecuteXcmOrigin = xcm_builder::EnsureXcmOrigin<RuntimeOrigin, LocalOriginToLocation>;
type XcmExecuteFilter = Nothing;
Expand Down
10 changes: 5 additions & 5 deletions polkadot/xcm/xcm-simulator/example/src/parachain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ use polkadot_parachain_primitives::primitives::{
use xcm::{latest::prelude::*, VersionedXcm};
use xcm_builder::{
Account32Hash, AccountId32Aliases, AllowUnpaidExecutionFrom, ConvertedConcreteId,
EnsureXcmOrigin, FixedRateOfFungible, FixedWeightBounds, FrameTransactionalProcessor,
FungibleAdapter, IsConcrete, NativeAsset, NoChecking, NonFungiblesAdapter, ParentIsPreset,
SiblingParachainConvertsVia, SignedAccountId32AsNative, SignedToAccountId32,
SovereignSignedViaLocation,
EnsureDecodableXcm, EnsureXcmOrigin, FixedRateOfFungible, FixedWeightBounds,
FrameTransactionalProcessor, FungibleAdapter, IsConcrete, NativeAsset, NoChecking,
NonFungiblesAdapter, ParentIsPreset, SiblingParachainConvertsVia, SignedAccountId32AsNative,
SignedToAccountId32, SovereignSignedViaLocation,
};
use xcm_executor::{
traits::{ConvertLocation, JustTry},
Expand Down Expand Up @@ -212,7 +212,7 @@ pub type LocalAssetTransactor = (
>,
);

pub type XcmRouter = super::ParachainXcmRouter<MsgQueue>;
pub type XcmRouter = EnsureDecodableXcm<super::ParachainXcmRouter<MsgQueue>>;
pub type Barrier = AllowUnpaidExecutionFrom<Everything>;

parameter_types! {
Expand Down
8 changes: 4 additions & 4 deletions polkadot/xcm/xcm-simulator/example/src/relay_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ use xcm::latest::prelude::*;
use xcm_builder::{
Account32Hash, AccountId32Aliases, AllowUnpaidExecutionFrom, AsPrefixedGeneralIndex,
ChildParachainAsNative, ChildParachainConvertsVia, ChildSystemParachainAsSuperuser,
ConvertedConcreteId, FixedRateOfFungible, FixedWeightBounds, FrameTransactionalProcessor,
FungibleAdapter, IsConcrete, NoChecking, NonFungiblesAdapter, SignedAccountId32AsNative,
SignedToAccountId32, SovereignSignedViaLocation,
ConvertedConcreteId, EnsureDecodableXcm, FixedRateOfFungible, FixedWeightBounds,
FrameTransactionalProcessor, FungibleAdapter, IsConcrete, NoChecking, NonFungiblesAdapter,
SignedAccountId32AsNative, SignedToAccountId32, SovereignSignedViaLocation,
};
use xcm_executor::{traits::JustTry, Config, XcmExecutor};

Expand Down Expand Up @@ -168,7 +168,7 @@ parameter_types! {
pub const MaxAssetsIntoHolding: u32 = 64;
}

pub type XcmRouter = super::RelayChainXcmRouter;
pub type XcmRouter = EnsureDecodableXcm<super::RelayChainXcmRouter>;
pub type Barrier = AllowUnpaidExecutionFrom<Everything>;

pub struct XcmConfig;
Expand Down
Loading