From 9331a6cc58650d8a3b68b5c4d520de0e463b2a50 Mon Sep 17 00:00:00 2001
From: Branislav Kontur <bkontur@gmail.com>
Date: Thu, 18 Apr 2024 10:59:38 +0200
Subject: [PATCH 1/6] Add `EnsureDecodableXcm` router (for testing purposes)
---
polkadot/xcm/xcm-builder/src/lib.rs | 2 +-
polkadot/xcm/xcm-builder/src/routing.rs | 66 +++++++++++++++++++++++++
2 files changed, 67 insertions(+), 1 deletion(-)
diff --git a/polkadot/xcm/xcm-builder/src/lib.rs b/polkadot/xcm/xcm-builder/src/lib.rs
index bd4a4c941c915..cdc663a0cc9b4 100644
--- a/polkadot/xcm/xcm-builder/src/lib.rs
+++ b/polkadot/xcm/xcm-builder/src/lib.rs
@@ -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;
diff --git a/polkadot/xcm/xcm-builder/src/routing.rs b/polkadot/xcm/xcm-builder/src/routing.rs
index 529ef80c15ff1..4b4dee9f1312f 100644
--- a/polkadot/xcm/xcm-builder/src/routing.rs
+++ b/polkadot/xcm/xcm-builder/src/routing.rs
@@ -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 testing purposes.
+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) {
+ 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) {
+ 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!"))
+ );
+}
From 5f53e19c22a0324a02b4ed76da74b3f743d60a0e Mon Sep 17 00:00:00 2001
From: Branislav Kontur <bkontur@gmail.com>
Date: Thu, 18 Apr 2024 11:15:26 +0200
Subject: [PATCH 2/6] Use `EnsureDecodableXcm` for `polkadot-test-runtime`
---
Cargo.lock | 8 --------
polkadot/runtime/test-runtime/Cargo.toml | 10 ----------
polkadot/runtime/test-runtime/constants/Cargo.toml | 6 ------
polkadot/runtime/test-runtime/src/xcm_config.rs | 10 ++++++----
.../xcm/pallet-xcm-benchmarks/src/fungible/mock.rs | 6 ++++--
.../xcm/pallet-xcm-benchmarks/src/generic/mock.rs | 5 +++--
polkadot/xcm/pallet-xcm/src/mock.rs | 12 +++++++-----
polkadot/xcm/xcm-builder/src/tests/mock.rs | 7 +++++--
polkadot/xcm/xcm-builder/tests/mock/mod.rs | 12 +++++++-----
polkadot/xcm/xcm-simulator/example/src/parachain.rs | 10 +++++-----
.../xcm/xcm-simulator/example/src/relay_chain.rs | 2 +-
11 files changed, 38 insertions(+), 50 deletions(-)
diff --git a/Cargo.lock b/Cargo.lock
index 951f2548d34da..910def85c60d4 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -14348,7 +14348,6 @@ dependencies = [
name = "polkadot-test-runtime"
version = "1.0.0"
dependencies = [
- "bitvec",
"frame-election-provider-support",
"frame-executive",
"frame-support",
@@ -14373,16 +14372,12 @@ dependencies = [
"pallet-vesting",
"pallet-xcm",
"parity-scale-codec",
- "polkadot-parachain-primitives",
"polkadot-primitives",
"polkadot-runtime-common",
"polkadot-runtime-parachains",
- "rustc-hex",
"scale-info",
"serde",
- "serde_derive",
"serde_json",
- "smallvec",
"sp-api",
"sp-authority-discovery",
"sp-block-builder",
@@ -21273,11 +21268,8 @@ version = "1.0.0"
dependencies = [
"frame-support",
"polkadot-primitives",
- "polkadot-runtime-common",
"smallvec",
- "sp-core",
"sp-runtime",
- "sp-weights",
]
[[package]]
diff --git a/polkadot/runtime/test-runtime/Cargo.toml b/polkadot/runtime/test-runtime/Cargo.toml
index 35fb684597e7d..6552ed4ef8aea 100644
--- a/polkadot/runtime/test-runtime/Cargo.toml
+++ b/polkadot/runtime/test-runtime/Cargo.toml
@@ -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 }
@@ -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 }
@@ -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",
@@ -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",
@@ -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",
diff --git a/polkadot/runtime/test-runtime/constants/Cargo.toml b/polkadot/runtime/test-runtime/constants/Cargo.toml
index 2b387bbd3072a..5b8a4d7a051af 100644
--- a/polkadot/runtime/test-runtime/constants/Cargo.toml
+++ b/polkadot/runtime/test-runtime/constants/Cargo.toml
@@ -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",
]
diff --git a/polkadot/runtime/test-runtime/src/xcm_config.rs b/polkadot/runtime/test-runtime/src/xcm_config.rs
index 8411b79f2529d..03e240e9c4846 100644
--- a/polkadot/runtime/test-runtime/src/xcm_config.rs
+++ b/polkadot/runtime/test-runtime/src/xcm_config.rs
@@ -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},
@@ -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>;
diff --git a/polkadot/xcm/pallet-xcm-benchmarks/src/fungible/mock.rs b/polkadot/xcm/pallet-xcm-benchmarks/src/fungible/mock.rs
index c831cd0246591..af23de17e3889 100644
--- a/polkadot/xcm/pallet-xcm-benchmarks/src/fungible/mock.rs
+++ b/polkadot/xcm/pallet-xcm-benchmarks/src/fungible/mock.rs
@@ -26,7 +26,9 @@ use frame_support::{
use sp_core::H256;
use sp_runtime::traits::{BlakeTwo256, IdentityLookup};
use xcm::latest::prelude::*;
-use xcm_builder::{AllowUnpaidExecutionFrom, FrameTransactionalProcessor, MintLocation};
+use xcm_builder::{
+ AllowUnpaidExecutionFrom, EnsureDecodableXcm, FrameTransactionalProcessor, MintLocation,
+};
type Block = frame_system::mocking::MockBlock<Test>;
@@ -121,7 +123,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;
diff --git a/polkadot/xcm/pallet-xcm-benchmarks/src/generic/mock.rs b/polkadot/xcm/pallet-xcm-benchmarks/src/generic/mock.rs
index 534f7d85ea2e9..f2ace94502337 100644
--- a/polkadot/xcm/pallet-xcm-benchmarks/src/generic/mock.rs
+++ b/polkadot/xcm/pallet-xcm-benchmarks/src/generic/mock.rs
@@ -30,7 +30,8 @@ use xcm_builder::{
AssetsInHolding, TestAssetExchanger, TestAssetLocker, TestAssetTrap,
TestSubscriptionService, TestUniversalAliases,
},
- AliasForeignAccountId32, AllowUnpaidExecutionFrom, FrameTransactionalProcessor,
+ AliasForeignAccountId32, AllowUnpaidExecutionFrom, EnsureDecodableXcm,
+ FrameTransactionalProcessor,
};
use xcm_executor::traits::ConvertOrigin;
@@ -110,7 +111,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;
diff --git a/polkadot/xcm/pallet-xcm/src/mock.rs b/polkadot/xcm/pallet-xcm/src/mock.rs
index 2cc228476ba8e..7fcbc881b438b 100644
--- a/polkadot/xcm/pallet-xcm/src/mock.rs
+++ b/polkadot/xcm/pallet-xcm/src/mock.rs
@@ -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},
@@ -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 {
diff --git a/polkadot/xcm/xcm-builder/src/tests/mock.rs b/polkadot/xcm/xcm-builder/src/tests/mock.rs
index 3d03ab054248d..7532b97d97b3b 100644
--- a/polkadot/xcm/xcm-builder/src/tests/mock.rs
+++ b/polkadot/xcm/xcm-builder/src/tests/mock.rs
@@ -19,6 +19,7 @@
use crate::{
barriers::{AllowSubscriptionsFrom, RespectSuspension, TrailingSetTopicAsId},
test_utils::*,
+ EnsureDecodableXcm,
};
pub use crate::{
AliasForeignAccountId32, AllowExplicitUnpaidExecutionFrom, AllowKnownQueryResponses,
@@ -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>,
@@ -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);
diff --git a/polkadot/xcm/xcm-builder/tests/mock/mod.rs b/polkadot/xcm/xcm-builder/tests/mock/mod.rs
index f3cf5ab264902..474aa4ff9bc9e 100644
--- a/polkadot/xcm/xcm-builder/tests/mock/mod.rs
+++ b/polkadot/xcm/xcm-builder/tests/mock/mod.rs
@@ -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;
@@ -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;
@@ -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 = ();
@@ -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;
diff --git a/polkadot/xcm/xcm-simulator/example/src/parachain.rs b/polkadot/xcm/xcm-simulator/example/src/parachain.rs
index 86401d756af3b..830293d0a1630 100644
--- a/polkadot/xcm/xcm-simulator/example/src/parachain.rs
+++ b/polkadot/xcm/xcm-simulator/example/src/parachain.rs
@@ -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},
@@ -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! {
diff --git a/polkadot/xcm/xcm-simulator/example/src/relay_chain.rs b/polkadot/xcm/xcm-simulator/example/src/relay_chain.rs
index 286d0038e187a..16a4eb6d3bc35 100644
--- a/polkadot/xcm/xcm-simulator/example/src/relay_chain.rs
+++ b/polkadot/xcm/xcm-simulator/example/src/relay_chain.rs
@@ -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;
From feaf731ef208e8522b0772c878594efe556a53fe Mon Sep 17 00:00:00 2001
From: Branislav Kontur <bkontur@gmail.com>
Date: Thu, 18 Apr 2024 11:55:28 +0200
Subject: [PATCH 3/6] Update polkadot/xcm/xcm-builder/src/routing.rs
Co-authored-by: Adrian Catangiu <adrian@parity.io>
---
polkadot/xcm/xcm-builder/src/routing.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/polkadot/xcm/xcm-builder/src/routing.rs b/polkadot/xcm/xcm-builder/src/routing.rs
index 4b4dee9f1312f..243f8040e6614 100644
--- a/polkadot/xcm/xcm-builder/src/routing.rs
+++ b/polkadot/xcm/xcm-builder/src/routing.rs
@@ -147,7 +147,7 @@ impl EnsureDelivery for Tuple {
/// 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 testing purposes.
+/// NOTE: For use in mock runtimes which don't have the DMP/UMP/HRMP XCM validations.
pub struct EnsureDecodableXcm<Inner>(sp_std::marker::PhantomData<Inner>);
impl<Inner: SendXcm> SendXcm for EnsureDecodableXcm<Inner> {
type Ticket = Inner::Ticket;
From 5080212a55868500a174ca4d6d0d6133a3e5b4b1 Mon Sep 17 00:00:00 2001
From: Branislav Kontur <bkontur@gmail.com>
Date: Thu, 18 Apr 2024 12:40:58 +0200
Subject: [PATCH 4/6] Fix import
---
polkadot/xcm/xcm-simulator/example/src/relay_chain.rs | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/polkadot/xcm/xcm-simulator/example/src/relay_chain.rs b/polkadot/xcm/xcm-simulator/example/src/relay_chain.rs
index 16a4eb6d3bc35..5273d47604100 100644
--- a/polkadot/xcm/xcm-simulator/example/src/relay_chain.rs
+++ b/polkadot/xcm/xcm-simulator/example/src/relay_chain.rs
@@ -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};
From f7e94f42cf592d3c8d580a1969aaf05b6e873793 Mon Sep 17 00:00:00 2001
From: Branislav Kontur <bkontur@gmail.com>
Date: Thu, 18 Apr 2024 22:17:18 +0200
Subject: [PATCH 5/6] Fixed XCM benchmarks to generate holding register data
import
---
.../src/fungible/benchmarking.rs | 26 ++++++++++----
.../src/fungible/mock.rs | 7 ++--
.../src/generic/benchmarking.rs | 36 ++++++++++++-------
.../pallet-xcm-benchmarks/src/generic/mock.rs | 5 ++-
polkadot/xcm/pallet-xcm-benchmarks/src/lib.rs | 13 ++++---
5 files changed, 57 insertions(+), 30 deletions(-)
diff --git a/polkadot/xcm/pallet-xcm-benchmarks/src/fungible/benchmarking.rs b/polkadot/xcm/pallet-xcm-benchmarks/src/fungible/benchmarking.rs
index 4b77199069d34..d99da9184b5d8 100644
--- a/polkadot/xcm/pallet-xcm-benchmarks/src/fungible/benchmarking.rs
+++ b/polkadot/xcm/pallet-xcm-benchmarks/src/fungible/benchmarking.rs
@@ -146,8 +146,6 @@ benchmarks_instance_pallet! {
initiate_reserve_withdraw {
let (sender_account, sender_location) = account_and_location::<T>(1);
- let holding = T::worst_case_holding(1);
- let assets_filter = AssetFilter::Definite(holding.clone().into_inner().into_iter().take(MAX_ITEMS_IN_ASSETS).collect::<Vec<_>>().into());
let reserve = T::valid_destination().map_err(|_| BenchmarkError::Skip)?;
let (expected_fees_mode, expected_assets_in_holding) = T::DeliveryHelper::ensure_successful_delivery(
@@ -157,15 +155,29 @@ benchmarks_instance_pallet! {
);
let sender_account_balance_before = T::TransactAsset::balance(&sender_account);
+ // generate holding and add possible required fees
+ let holding = if let Some(expected_assets_in_holding) = expected_assets_in_holding {
+ let mut holding = T::worst_case_holding(1 + expected_assets_in_holding.len() as u32);
+ for a in expected_assets_in_holding.into_inner() {
+ holding.push(a);
+ }
+ holding
+ } else {
+ T::worst_case_holding(1)
+ };
+
let mut executor = new_executor::<T>(sender_location);
- executor.set_holding(holding.into());
+ executor.set_holding(holding.clone().into());
if let Some(expected_fees_mode) = expected_fees_mode {
executor.set_fees_mode(expected_fees_mode);
}
- if let Some(expected_assets_in_holding) = expected_assets_in_holding {
- executor.set_holding(expected_assets_in_holding.into());
- }
- let instruction = Instruction::InitiateReserveWithdraw { assets: assets_filter, reserve, xcm: Xcm(vec![]) };
+
+ let instruction = Instruction::InitiateReserveWithdraw {
+ // Worst case is looking through all holdings for every asset explicitly - respecting the limit `MAX_ITEMS_IN_ASSETS`.
+ assets: Definite(holding.into_inner().into_iter().take(MAX_ITEMS_IN_ASSETS).collect::<Vec<_>>().into()),
+ reserve,
+ xcm: Xcm(vec![])
+ };
let xcm = Xcm(vec![instruction]);
}: {
executor.bench_process(xcm)?;
diff --git a/polkadot/xcm/pallet-xcm-benchmarks/src/fungible/mock.rs b/polkadot/xcm/pallet-xcm-benchmarks/src/fungible/mock.rs
index af23de17e3889..d93b60a6e0256 100644
--- a/polkadot/xcm/pallet-xcm-benchmarks/src/fungible/mock.rs
+++ b/polkadot/xcm/pallet-xcm-benchmarks/src/fungible/mock.rs
@@ -16,7 +16,7 @@
//! A mock runtime for XCM benchmarking.
-use crate::{fungible as xcm_balances_benchmark, mock::*};
+use crate::{fungible as xcm_balances_benchmark, generate_holding_assets, mock::*};
use frame_benchmarking::BenchmarkError;
use frame_support::{
derive_impl, parameter_types,
@@ -162,9 +162,8 @@ impl crate::Config for Test {
Ok(valid_destination)
}
fn worst_case_holding(depositable_count: u32) -> Assets {
- crate::mock_worst_case_holding(
- depositable_count,
- <XcmConfig as xcm_executor::Config>::MaxAssetsIntoHolding::get(),
+ generate_holding_assets(
+ <XcmConfig as xcm_executor::Config>::MaxAssetsIntoHolding::get() - depositable_count,
)
}
}
diff --git a/polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs b/polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs
index 8c6ed4b5d0e02..760b21f93566e 100644
--- a/polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs
+++ b/polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs
@@ -19,9 +19,9 @@ use crate::{account_and_location, new_executor, EnsureDelivery, XcmCallOf};
use codec::Encode;
use frame_benchmarking::{benchmarks, BenchmarkError};
use frame_support::{dispatch::GetDispatchInfo, traits::fungible::Inspect};
-use sp_std::vec;
+use sp_std::{prelude::*, vec};
use xcm::{
- latest::{prelude::*, MaxDispatchErrorLen, MaybeErrorCode, Weight},
+ latest::{prelude::*, MaxDispatchErrorLen, MaybeErrorCode, Weight, MAX_ITEMS_IN_ASSETS},
DoubleEncoded,
};
use xcm_executor::{
@@ -32,7 +32,6 @@ use xcm_executor::{
benchmarks! {
report_holding {
let (sender_account, sender_location) = account_and_location::<T>(1);
- let holding = T::worst_case_holding(0);
let destination = T::valid_destination().map_err(|_| BenchmarkError::Skip)?;
let (expected_fees_mode, expected_assets_in_holding) = T::DeliveryHelper::ensure_successful_delivery(
@@ -42,14 +41,22 @@ benchmarks! {
);
let sender_account_balance_before = T::TransactAsset::balance(&sender_account);
+ // generate holding and add possible required fees
+ let holding = if let Some(expected_assets_in_holding) = expected_assets_in_holding {
+ let mut holding = T::worst_case_holding(expected_assets_in_holding.len() as u32);
+ for a in expected_assets_in_holding.into_inner() {
+ holding.push(a);
+ }
+ holding
+ } else {
+ T::worst_case_holding(0)
+ };
+
let mut executor = new_executor::<T>(sender_location);
executor.set_holding(holding.clone().into());
if let Some(expected_fees_mode) = expected_fees_mode {
executor.set_fees_mode(expected_fees_mode);
}
- if let Some(expected_assets_in_holding) = expected_assets_in_holding {
- executor.set_holding(expected_assets_in_holding.into());
- }
let instruction = Instruction::<XcmCallOf<T>>::ReportHolding {
response_info: QueryResponseInfo {
@@ -57,8 +64,8 @@ benchmarks! {
query_id: Default::default(),
max_weight: Weight::MAX,
},
- // Worst case is looking through all holdings for every asset explicitly.
- assets: Definite(holding),
+ // Worst case is looking through all holdings for every asset explicitly - respecting the limit `MAX_ITEMS_IN_ASSETS`.
+ assets: Definite(holding.into_inner().into_iter().take(MAX_ITEMS_IN_ASSETS).collect::<Vec<_>>().into()),
};
let xcm = Xcm(vec![instruction]);
@@ -612,14 +619,19 @@ benchmarks! {
let sender_account = T::AccountIdConverter::convert_location(&owner).unwrap();
let sender_account_balance_before = T::TransactAsset::balance(&sender_account);
+ // generate holding and add possible required fees
+ let mut holding: Assets = asset.clone().into();
+ if let Some(expected_assets_in_holding) = expected_assets_in_holding {
+ for a in expected_assets_in_holding.into_inner() {
+ holding.push(a);
+ }
+ };
+
let mut executor = new_executor::<T>(owner);
- executor.set_holding(asset.clone().into());
+ executor.set_holding(holding.into());
if let Some(expected_fees_mode) = expected_fees_mode {
executor.set_fees_mode(expected_fees_mode);
}
- if let Some(expected_assets_in_holding) = expected_assets_in_holding {
- executor.set_holding(expected_assets_in_holding.into());
- }
let instruction = Instruction::LockAsset { asset, unlocker };
let xcm = Xcm(vec![instruction]);
diff --git a/polkadot/xcm/pallet-xcm-benchmarks/src/generic/mock.rs b/polkadot/xcm/pallet-xcm-benchmarks/src/generic/mock.rs
index f2ace94502337..5732d52962e3d 100644
--- a/polkadot/xcm/pallet-xcm-benchmarks/src/generic/mock.rs
+++ b/polkadot/xcm/pallet-xcm-benchmarks/src/generic/mock.rs
@@ -162,9 +162,8 @@ impl crate::Config for Test {
Ok(valid_destination)
}
fn worst_case_holding(depositable_count: u32) -> Assets {
- crate::mock_worst_case_holding(
- depositable_count,
- <XcmConfig as xcm_executor::Config>::MaxAssetsIntoHolding::get(),
+ generate_holding_assets(
+ <XcmConfig as xcm_executor::Config>::MaxAssetsIntoHolding::get() - depositable_count,
)
}
}
diff --git a/polkadot/xcm/pallet-xcm-benchmarks/src/lib.rs b/polkadot/xcm/pallet-xcm-benchmarks/src/lib.rs
index 63ed0ac0ca736..a43f27bf47e72 100644
--- a/polkadot/xcm/pallet-xcm-benchmarks/src/lib.rs
+++ b/polkadot/xcm/pallet-xcm-benchmarks/src/lib.rs
@@ -50,6 +50,8 @@ pub trait Config: frame_system::Config {
fn valid_destination() -> Result<Location, BenchmarkError>;
/// Worst case scenario for a holding account in this runtime.
+ /// - `depositable_count` specifies the count of assets we plan to add to the holding on top of
+ /// those generated by the `worst_case_holding` implementation.
fn worst_case_holding(depositable_count: u32) -> Assets;
}
@@ -64,19 +66,22 @@ pub type AssetTransactorOf<T> = <<T as Config>::XcmConfig as XcmConfig>::AssetTr
/// The call type of executor's config. Should eventually resolve to the same overarching call type.
pub type XcmCallOf<T> = <<T as Config>::XcmConfig as XcmConfig>::RuntimeCall;
-pub fn mock_worst_case_holding(depositable_count: u32, max_assets: u32) -> Assets {
+pub fn generate_holding_assets(max_assets: u32) -> Assets {
let fungibles_amount: u128 = 100;
- let holding_fungibles = max_assets / 2 - depositable_count;
- let holding_non_fungibles = holding_fungibles;
+ let holding_fungibles = max_assets / 2;
+ let holding_non_fungibles = max_assets - holding_fungibles - 1; // -1 because of adding `Here` asset
+ // add count of `holding_fungibles`
(0..holding_fungibles)
.map(|i| {
Asset {
id: AssetId(GeneralIndex(i as u128).into()),
- fun: Fungible(fungibles_amount * i as u128),
+ fun: Fungible(fungibles_amount * (i + 1) as u128), // non-zero amount
}
.into()
})
+ // add one more `Here` asset
.chain(core::iter::once(Asset { id: AssetId(Here.into()), fun: Fungible(u128::MAX) }))
+ // add count of `holding_non_fungibles`
.chain((0..holding_non_fungibles).map(|i| Asset {
id: AssetId(GeneralIndex(i as u128).into()),
fun: NonFungible(asset_instance_from(i)),
From d08c4404a5e1a93f710821bb91904572848f2811 Mon Sep 17 00:00:00 2001
From: Branislav Kontur <bkontur@gmail.com>
Date: Tue, 23 Apr 2024 12:26:37 +0200
Subject: [PATCH 6/6] Removed `ensure_encode_decode` and replaced by
`VersionedXcm::validate_xcm_nesting`
---
.../runtime/test-runtime/src/xcm_config.rs | 10 ++--
polkadot/xcm/xcm-builder/src/routing.rs | 46 +++----------------
2 files changed, 11 insertions(+), 45 deletions(-)
diff --git a/polkadot/runtime/test-runtime/src/xcm_config.rs b/polkadot/runtime/test-runtime/src/xcm_config.rs
index 03e240e9c4846..8411b79f2529d 100644
--- a/polkadot/runtime/test-runtime/src/xcm_config.rs
+++ b/polkadot/runtime/test-runtime/src/xcm_config.rs
@@ -24,8 +24,8 @@ use polkadot_runtime_parachains::FeeTracker;
use runtime_common::xcm_sender::{ChildParachainRouter, PriceForMessageDelivery};
use xcm::latest::prelude::*;
use xcm_builder::{
- AllowUnpaidExecutionFrom, EnsureDecodableXcm, EnsureXcmOrigin, FixedWeightBounds,
- FrameTransactionalProcessor, SignedAccountId32AsNative, SignedToAccountId32, WithUniqueTopic,
+ AllowUnpaidExecutionFrom, EnsureXcmOrigin, FixedWeightBounds, FrameTransactionalProcessor,
+ SignedAccountId32AsNative, SignedToAccountId32, WithUniqueTopic,
};
use xcm_executor::{
traits::{TransactAsset, WeightTrader},
@@ -82,10 +82,8 @@ 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<
- EnsureDecodableXcm<
- // Only one router so far - use DMP to communicate with child parachains.
- ChildParachainRouter<super::Runtime, super::Xcm, PriceForChildParachainDelivery>,
- >,
+ // Only one router so far - use DMP to communicate with child parachains.
+ ChildParachainRouter<super::Runtime, super::Xcm, PriceForChildParachainDelivery>,
>;
pub type Barrier = AllowUnpaidExecutionFrom<Everything>;
diff --git a/polkadot/xcm/xcm-builder/src/routing.rs b/polkadot/xcm/xcm-builder/src/routing.rs
index 243f8040e6614..921b9ac5922ea 100644
--- a/polkadot/xcm/xcm-builder/src/routing.rs
+++ b/polkadot/xcm/xcm-builder/src/routing.rs
@@ -157,8 +157,13 @@ impl<Inner: SendXcm> SendXcm for EnsureDecodableXcm<Inner> {
message: &mut Option<Xcm<()>>,
) -> SendResult<Self::Ticket> {
if let Some(msg) = message {
- if let Some(e) = Self::ensure_encode_decode(msg) {
- return Err(e)
+ let versioned_xcm = VersionedXcm::<()>::from(msg.clone());
+ if versioned_xcm.validate_xcm_nesting().is_err() {
+ log::error!(
+ target: "xcm::validate_xcm_nesting",
+ "EnsureDecodableXcm validate_xcm_nesting error for \nversioned_xcm: {versioned_xcm:?}\nbased on xcm: {msg:?}"
+ );
+ return Err(SendError::Transport("EnsureDecodableXcm validate_xcm_nesting error"))
}
}
Inner::validate(destination, message)
@@ -168,40 +173,3 @@ impl<Inner: SendXcm> SendXcm for EnsureDecodableXcm<Inner> {
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) {
- 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!"))
- );
-}