From ff7e2c88a460a0f9df26eb3f33d1c37f72508580 Mon Sep 17 00:00:00 2001
From: Branislav Kontur <bkontur@gmail.com>
Date: Mon, 22 Apr 2024 13:34:04 +0200
Subject: [PATCH] Sanitize `UniversalLocation` witth `GlobalConsensus` + XCM
 small nits and improvements (#4238)

This PR:
- sanitizes all `UniversalLocation`s with `GlobalConsensus` (when
possible) - addressing
[comment](https://github.com/paritytech/polkadot-sdk/pull/4025#discussion_r1557361473)
- adds `DefaultConfig` for `pallet-xcm-benchmarks` for `system`
---
 Cargo.lock                                    |  1 -
 .../contracts-rococo/src/xcm_config.rs        |  4 +--
 .../glutton/glutton-westend/src/xcm_config.rs |  4 +--
 .../runtimes/starters/shell/src/xcm_config.rs |  4 +--
 .../testing/rococo-parachain/src/lib.rs       |  4 +--
 polkadot/xcm/pallet-xcm-benchmarks/Cargo.toml |  1 -
 .../src/fungible/mock.rs                      | 32 +------------------
 .../pallet-xcm-benchmarks/src/generic/mock.rs | 31 +-----------------
 .../xcm/pallet-xcm-benchmarks/src/mock.rs     |  2 +-
 polkadot/xcm/pallet-xcm/src/mock.rs           |  2 +-
 .../xcm/xcm-builder/src/universal_exports.rs  |  4 +--
 polkadot/xcm/xcm-builder/tests/mock/mod.rs    |  2 +-
 .../xcm-simulator/example/src/parachain.rs    |  2 +-
 .../xcm-simulator/example/src/relay_chain.rs  |  2 +-
 .../xcm/xcm-simulator/fuzzer/src/parachain.rs |  2 +-
 .../xcm-simulator/fuzzer/src/relay_chain.rs   |  2 +-
 .../contracts/mock-network/src/parachain.rs   |  2 +-
 .../contracts/mock-network/src/relay_chain.rs |  2 +-
 .../runtime/src/configs/xcm_config.rs         |  2 ++
 19 files changed, 23 insertions(+), 82 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 951f2548d34..c932927a905 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -11777,7 +11777,6 @@ dependencies = [
  "polkadot-primitives",
  "polkadot-runtime-common",
  "scale-info",
- "sp-core",
  "sp-io",
  "sp-runtime",
  "sp-std 14.0.0",
diff --git a/cumulus/parachains/runtimes/contracts/contracts-rococo/src/xcm_config.rs b/cumulus/parachains/runtimes/contracts/contracts-rococo/src/xcm_config.rs
index 46fcbc6319c..9132b4e1760 100644
--- a/cumulus/parachains/runtimes/contracts/contracts-rococo/src/xcm_config.rs
+++ b/cumulus/parachains/runtimes/contracts/contracts-rococo/src/xcm_config.rs
@@ -51,9 +51,9 @@ use xcm_executor::XcmExecutor;
 
 parameter_types! {
 	pub const RelayLocation: Location = Location::parent();
-	pub const RelayNetwork: Option<NetworkId> = None;
+	pub const RelayNetwork: NetworkId = NetworkId::Rococo;
 	pub RelayChainOrigin: RuntimeOrigin = cumulus_pallet_xcm::Origin::Relay.into();
-	pub UniversalLocation: InteriorLocation = Parachain(ParachainInfo::parachain_id().into()).into();
+	pub UniversalLocation: InteriorLocation = [GlobalConsensus(RelayNetwork::get()), Parachain(ParachainInfo::parachain_id().into())].into();
 	pub const ExecutiveBody: BodyId = BodyId::Executive;
 	pub TreasuryAccount: AccountId = TREASURY_PALLET_ID.into_account_truncating();
 	pub RelayTreasuryLocation: Location = (Parent, PalletInstance(rococo_runtime_constants::TREASURY_PALLET_ID)).into();
diff --git a/cumulus/parachains/runtimes/glutton/glutton-westend/src/xcm_config.rs b/cumulus/parachains/runtimes/glutton/glutton-westend/src/xcm_config.rs
index 15bb519e115..9d438a41f8f 100644
--- a/cumulus/parachains/runtimes/glutton/glutton-westend/src/xcm_config.rs
+++ b/cumulus/parachains/runtimes/glutton/glutton-westend/src/xcm_config.rs
@@ -30,8 +30,8 @@ use xcm_builder::{
 
 parameter_types! {
 	pub const WestendLocation: Location = Location::parent();
-	pub const WestendNetwork: Option<NetworkId> = Some(NetworkId::Westend);
-	pub UniversalLocation: InteriorLocation = [Parachain(ParachainInfo::parachain_id().into())].into();
+	pub const WestendNetwork: NetworkId = NetworkId::Westend;
+	pub UniversalLocation: InteriorLocation = [GlobalConsensus(WestendNetwork::get()), Parachain(ParachainInfo::parachain_id().into())].into();
 }
 
 /// This is the type we use to convert an (incoming) XCM origin into a local `Origin` instance,
diff --git a/cumulus/parachains/runtimes/starters/shell/src/xcm_config.rs b/cumulus/parachains/runtimes/starters/shell/src/xcm_config.rs
index df89158729c..7f9de0f64b3 100644
--- a/cumulus/parachains/runtimes/starters/shell/src/xcm_config.rs
+++ b/cumulus/parachains/runtimes/starters/shell/src/xcm_config.rs
@@ -30,8 +30,8 @@ use xcm_builder::{
 
 parameter_types! {
 	pub const RococoLocation: Location = Location::parent();
-	pub const RococoNetwork: Option<NetworkId> = Some(NetworkId::Rococo);
-	pub UniversalLocation: InteriorLocation = [Parachain(ParachainInfo::parachain_id().into())].into();
+	pub const RococoNetwork: NetworkId = NetworkId::Rococo;
+	pub UniversalLocation: InteriorLocation = [GlobalConsensus(RococoNetwork::get()), Parachain(ParachainInfo::parachain_id().into())].into();
 }
 
 /// This is the type we use to convert an (incoming) XCM origin into a local `Origin` instance,
diff --git a/cumulus/parachains/runtimes/testing/rococo-parachain/src/lib.rs b/cumulus/parachains/runtimes/testing/rococo-parachain/src/lib.rs
index df335368be1..0ae93d1577c 100644
--- a/cumulus/parachains/runtimes/testing/rococo-parachain/src/lib.rs
+++ b/cumulus/parachains/runtimes/testing/rococo-parachain/src/lib.rs
@@ -327,9 +327,9 @@ impl cumulus_pallet_aura_ext::Config for Runtime {}
 
 parameter_types! {
 	pub const RocLocation: Location = Location::parent();
-	pub const RococoNetwork: Option<NetworkId> = Some(NetworkId::Rococo);
+	pub const RococoNetwork: NetworkId = NetworkId::Rococo;
 	pub RelayChainOrigin: RuntimeOrigin = cumulus_pallet_xcm::Origin::Relay.into();
-	pub UniversalLocation: InteriorLocation = [Parachain(ParachainInfo::parachain_id().into())].into();
+	pub UniversalLocation: InteriorLocation = [GlobalConsensus(RococoNetwork::get()), Parachain(ParachainInfo::parachain_id().into())].into();
 	pub CheckingAccount: AccountId = PolkadotXcm::check_account();
 }
 
diff --git a/polkadot/xcm/pallet-xcm-benchmarks/Cargo.toml b/polkadot/xcm/pallet-xcm-benchmarks/Cargo.toml
index 8c71426a6fa..9691ddd4816 100644
--- a/polkadot/xcm/pallet-xcm-benchmarks/Cargo.toml
+++ b/polkadot/xcm/pallet-xcm-benchmarks/Cargo.toml
@@ -29,7 +29,6 @@ log = { workspace = true, default-features = true }
 [dev-dependencies]
 pallet-balances = { path = "../../../substrate/frame/balances" }
 pallet-assets = { path = "../../../substrate/frame/assets" }
-sp-core = { path = "../../../substrate/primitives/core" }
 sp-tracing = { path = "../../../substrate/primitives/tracing" }
 xcm = { package = "staging-xcm", path = ".." }
 # temp
diff --git a/polkadot/xcm/pallet-xcm-benchmarks/src/fungible/mock.rs b/polkadot/xcm/pallet-xcm-benchmarks/src/fungible/mock.rs
index c831cd02465..d11f64e7494 100644
--- a/polkadot/xcm/pallet-xcm-benchmarks/src/fungible/mock.rs
+++ b/polkadot/xcm/pallet-xcm-benchmarks/src/fungible/mock.rs
@@ -20,11 +20,8 @@ use crate::{fungible as xcm_balances_benchmark, mock::*};
 use frame_benchmarking::BenchmarkError;
 use frame_support::{
 	derive_impl, parameter_types,
-	traits::{ConstU32, Everything, Nothing},
-	weights::Weight,
+	traits::{Everything, Nothing},
 };
-use sp_core::H256;
-use sp_runtime::traits::{BlakeTwo256, IdentityLookup};
 use xcm::latest::prelude::*;
 use xcm_builder::{AllowUnpaidExecutionFrom, FrameTransactionalProcessor, MintLocation};
 
@@ -40,37 +37,10 @@ frame_support::construct_runtime!(
 	}
 );
 
-parameter_types! {
-	pub const BlockHashCount: u64 = 250;
-	pub BlockWeights: frame_system::limits::BlockWeights =
-		frame_system::limits::BlockWeights::simple_max(Weight::from_parts(1024, u64::MAX));
-}
-
 #[derive_impl(frame_system::config_preludes::TestDefaultConfig)]
 impl frame_system::Config for Test {
-	type BaseCallFilter = Everything;
-	type BlockWeights = ();
-	type BlockLength = ();
-	type DbWeight = ();
-	type RuntimeOrigin = RuntimeOrigin;
-	type Nonce = u64;
-	type Hash = H256;
-	type RuntimeCall = RuntimeCall;
-	type Hashing = BlakeTwo256;
-	type AccountId = u64;
-	type Lookup = IdentityLookup<Self::AccountId>;
 	type Block = Block;
-	type RuntimeEvent = RuntimeEvent;
-	type BlockHashCount = BlockHashCount;
-	type Version = ();
-	type PalletInfo = PalletInfo;
 	type AccountData = pallet_balances::AccountData<u64>;
-	type OnNewAccount = ();
-	type OnKilledAccount = ();
-	type SystemWeightInfo = ();
-	type SS58Prefix = ();
-	type OnSetCode = ();
-	type MaxConsumers = ConstU32<16>;
 }
 
 parameter_types! {
diff --git a/polkadot/xcm/pallet-xcm-benchmarks/src/generic/mock.rs b/polkadot/xcm/pallet-xcm-benchmarks/src/generic/mock.rs
index 534f7d85ea2..f41df017b9d 100644
--- a/polkadot/xcm/pallet-xcm-benchmarks/src/generic/mock.rs
+++ b/polkadot/xcm/pallet-xcm-benchmarks/src/generic/mock.rs
@@ -21,10 +21,8 @@ use codec::Decode;
 use frame_support::{
 	derive_impl, parameter_types,
 	traits::{Contains, Everything, OriginTrait},
-	weights::Weight,
 };
-use sp_core::H256;
-use sp_runtime::traits::{BlakeTwo256, IdentityLookup, TrailingZeroInput};
+use sp_runtime::traits::TrailingZeroInput;
 use xcm_builder::{
 	test_utils::{
 		AssetsInHolding, TestAssetExchanger, TestAssetLocker, TestAssetTrap,
@@ -45,37 +43,10 @@ frame_support::construct_runtime!(
 	}
 );
 
-parameter_types! {
-	pub const BlockHashCount: u64 = 250;
-	pub BlockWeights: frame_system::limits::BlockWeights =
-		frame_system::limits::BlockWeights::simple_max(Weight::from_parts(1024, u64::MAX));
-}
-
 #[derive_impl(frame_system::config_preludes::TestDefaultConfig)]
 impl frame_system::Config for Test {
-	type BaseCallFilter = Everything;
-	type BlockWeights = ();
-	type BlockLength = ();
-	type DbWeight = ();
-	type RuntimeOrigin = RuntimeOrigin;
-	type Nonce = u64;
-	type Hash = H256;
-	type RuntimeCall = RuntimeCall;
-	type Hashing = BlakeTwo256;
-	type AccountId = u64;
-	type Lookup = IdentityLookup<Self::AccountId>;
 	type Block = Block;
-	type RuntimeEvent = RuntimeEvent;
-	type BlockHashCount = BlockHashCount;
-	type Version = ();
-	type PalletInfo = PalletInfo;
 	type AccountData = pallet_balances::AccountData<u64>;
-	type OnNewAccount = ();
-	type OnKilledAccount = ();
-	type SystemWeightInfo = ();
-	type SS58Prefix = ();
-	type OnSetCode = ();
-	type MaxConsumers = frame_support::traits::ConstU32<16>;
 }
 
 /// The benchmarks in this pallet should never need an asset transactor to begin with.
diff --git a/polkadot/xcm/pallet-xcm-benchmarks/src/mock.rs b/polkadot/xcm/pallet-xcm-benchmarks/src/mock.rs
index 78a9e5f8a01..be3af5d4a3f 100644
--- a/polkadot/xcm/pallet-xcm-benchmarks/src/mock.rs
+++ b/polkadot/xcm/pallet-xcm-benchmarks/src/mock.rs
@@ -58,7 +58,7 @@ impl xcm_executor::traits::ConvertLocation<u64> for AccountIdConverter {
 }
 
 parameter_types! {
-	pub UniversalLocation: InteriorLocation = Junction::Parachain(101).into();
+	pub UniversalLocation: InteriorLocation = [GlobalConsensus(ByGenesis([1; 32])), Junction::Parachain(101)].into();
 	pub UnitWeightCost: Weight = Weight::from_parts(10, 10);
 	pub WeightPrice: (AssetId, u128, u128) = (AssetId(Here.into()), 1_000_000, 1024);
 }
diff --git a/polkadot/xcm/pallet-xcm/src/mock.rs b/polkadot/xcm/pallet-xcm/src/mock.rs
index 2cc228476ba..e3680c530e2 100644
--- a/polkadot/xcm/pallet-xcm/src/mock.rs
+++ b/polkadot/xcm/pallet-xcm/src/mock.rs
@@ -413,7 +413,7 @@ parameter_types! {
 		)),
 	};
 	pub const AnyNetwork: Option<NetworkId> = None;
-	pub UniversalLocation: InteriorLocation = Here;
+	pub UniversalLocation: InteriorLocation = GlobalConsensus(ByGenesis([0; 32])).into();
 	pub UnitWeightCost: u64 = 1_000;
 	pub CheckingAccount: AccountId = XcmPallet::check_account();
 }
diff --git a/polkadot/xcm/xcm-builder/src/universal_exports.rs b/polkadot/xcm/xcm-builder/src/universal_exports.rs
index 6e031cdbc27..d0e3ef3032e 100644
--- a/polkadot/xcm/xcm-builder/src/universal_exports.rs
+++ b/polkadot/xcm/xcm-builder/src/universal_exports.rs
@@ -187,7 +187,7 @@ pub fn forward_id_for(original_id: &XcmHash) -> XcmHash {
 /// end with the `SetTopic` instruction.
 ///
 /// In the case that the message ends with a `SetTopic(T)` (as should be the case if the top-level
-/// router is `EnsureUniqueTopic`), then the forwarding message (i.e. the one carrying the
+/// router is `WithUniqueTopic`), then the forwarding message (i.e. the one carrying the
 /// export instruction *to* the bridge in local consensus) will also end with a `SetTopic` whose
 /// inner is `forward_id_for(T)`. If this is not the case then the onward message will not be given
 /// the `SetTopic` afterword.
@@ -254,7 +254,7 @@ impl<Bridges: ExporterFor, Router: SendXcm, UniversalLocation: Get<InteriorLocat
 /// end with the `SetTopic` instruction.
 ///
 /// In the case that the message ends with a `SetTopic(T)` (as should be the case if the top-level
-/// router is `EnsureUniqueTopic`), then the forwarding message (i.e. the one carrying the
+/// router is `WithUniqueTopic`), then the forwarding message (i.e. the one carrying the
 /// export instruction *to* the bridge in local consensus) will also end with a `SetTopic` whose
 /// inner is `forward_id_for(T)`. If this is not the case then the onward message will not be given
 /// the `SetTopic` afterword.
diff --git a/polkadot/xcm/xcm-builder/tests/mock/mod.rs b/polkadot/xcm/xcm-builder/tests/mock/mod.rs
index f3cf5ab2649..da38538b60c 100644
--- a/polkadot/xcm/xcm-builder/tests/mock/mod.rs
+++ b/polkadot/xcm/xcm-builder/tests/mock/mod.rs
@@ -137,7 +137,7 @@ impl configuration::Config for Runtime {
 parameter_types! {
 	pub const KsmLocation: Location = Location::here();
 	pub const KusamaNetwork: NetworkId = NetworkId::Kusama;
-	pub UniversalLocation: InteriorLocation = Here;
+	pub UniversalLocation: InteriorLocation = KusamaNetwork::get().into();
 	pub CheckAccount: (AccountId, MintLocation) = (XcmPallet::check_account(), MintLocation::Local);
 }
 
diff --git a/polkadot/xcm/xcm-simulator/example/src/parachain.rs b/polkadot/xcm/xcm-simulator/example/src/parachain.rs
index 86401d756af..c155ed5ab63 100644
--- a/polkadot/xcm/xcm-simulator/example/src/parachain.rs
+++ b/polkadot/xcm/xcm-simulator/example/src/parachain.rs
@@ -176,7 +176,7 @@ parameter_types! {
 parameter_types! {
 	pub const KsmLocation: Location = Location::parent();
 	pub const RelayNetwork: NetworkId = NetworkId::Kusama;
-	pub UniversalLocation: InteriorLocation = Parachain(MsgQueue::parachain_id().into()).into();
+	pub UniversalLocation: InteriorLocation = [GlobalConsensus(RelayNetwork::get()), Parachain(MsgQueue::parachain_id().into())].into();
 }
 
 pub type LocationToAccountId = (
diff --git a/polkadot/xcm/xcm-simulator/example/src/relay_chain.rs b/polkadot/xcm/xcm-simulator/example/src/relay_chain.rs
index 286d0038e18..4e8f1f68ebd 100644
--- a/polkadot/xcm/xcm-simulator/example/src/relay_chain.rs
+++ b/polkadot/xcm/xcm-simulator/example/src/relay_chain.rs
@@ -131,7 +131,7 @@ parameter_types! {
 	pub const TokenLocation: Location = Here.into_location();
 	pub RelayNetwork: NetworkId = ByGenesis([0; 32]);
 	pub const AnyNetwork: Option<NetworkId> = None;
-	pub UniversalLocation: InteriorLocation = Here;
+	pub UniversalLocation: InteriorLocation = RelayNetwork::get().into();
 	pub UnitWeightCost: u64 = 1_000;
 }
 
diff --git a/polkadot/xcm/xcm-simulator/fuzzer/src/parachain.rs b/polkadot/xcm/xcm-simulator/fuzzer/src/parachain.rs
index cadfc1e7200..d8d65fbf0ce 100644
--- a/polkadot/xcm/xcm-simulator/fuzzer/src/parachain.rs
+++ b/polkadot/xcm/xcm-simulator/fuzzer/src/parachain.rs
@@ -101,7 +101,7 @@ parameter_types! {
 parameter_types! {
 	pub const KsmLocation: Location = Location::parent();
 	pub const RelayNetwork: NetworkId = NetworkId::Kusama;
-	pub UniversalLocation: InteriorLocation = Parachain(MsgQueue::parachain_id().into()).into();
+	pub UniversalLocation: InteriorLocation = [GlobalConsensus(RelayNetwork::get()), Parachain(MsgQueue::parachain_id().into())].into();
 }
 
 pub type LocationToAccountId = (
diff --git a/polkadot/xcm/xcm-simulator/fuzzer/src/relay_chain.rs b/polkadot/xcm/xcm-simulator/fuzzer/src/relay_chain.rs
index 6790b535d16..47209b765d1 100644
--- a/polkadot/xcm/xcm-simulator/fuzzer/src/relay_chain.rs
+++ b/polkadot/xcm/xcm-simulator/fuzzer/src/relay_chain.rs
@@ -104,7 +104,7 @@ parameter_types! {
 	pub const TokenLocation: Location = Here.into_location();
 	pub const ThisNetwork: NetworkId = NetworkId::ByGenesis([0; 32]);
 	pub const AnyNetwork: Option<NetworkId> = None;
-	pub const UniversalLocation: InteriorLocation = Here;
+	pub UniversalLocation: InteriorLocation = ThisNetwork::get().into();
 }
 
 pub type SovereignAccountOf =
diff --git a/substrate/frame/contracts/mock-network/src/parachain.rs b/substrate/frame/contracts/mock-network/src/parachain.rs
index d4ad47581d1..843efab1502 100644
--- a/substrate/frame/contracts/mock-network/src/parachain.rs
+++ b/substrate/frame/contracts/mock-network/src/parachain.rs
@@ -144,7 +144,7 @@ parameter_types! {
 	pub const KsmLocation: Location = Location::parent();
 	pub const TokenLocation: Location = Here.into_location();
 	pub const RelayNetwork: NetworkId = ByGenesis([0; 32]);
-	pub UniversalLocation: InteriorLocation = Parachain(MsgQueue::parachain_id().into()).into();
+	pub UniversalLocation: InteriorLocation = [GlobalConsensus(RelayNetwork::get()), Parachain(MsgQueue::parachain_id().into())].into();
 }
 
 pub type XcmOriginToCallOrigin = (
diff --git a/substrate/frame/contracts/mock-network/src/relay_chain.rs b/substrate/frame/contracts/mock-network/src/relay_chain.rs
index 470304ed357..d5e0ec9c83f 100644
--- a/substrate/frame/contracts/mock-network/src/relay_chain.rs
+++ b/substrate/frame/contracts/mock-network/src/relay_chain.rs
@@ -107,7 +107,7 @@ impl configuration::Config for Runtime {
 parameter_types! {
 	pub RelayNetwork: NetworkId = ByGenesis([0; 32]);
 	pub const TokenLocation: Location = Here.into_location();
-	pub UniversalLocation: InteriorLocation = Here;
+	pub UniversalLocation: InteriorLocation = RelayNetwork::get().into();
 	pub UnitWeightCost: u64 = 1_000;
 }
 
diff --git a/templates/parachain/runtime/src/configs/xcm_config.rs b/templates/parachain/runtime/src/configs/xcm_config.rs
index 13da2363b05..c6b6e8da1b8 100644
--- a/templates/parachain/runtime/src/configs/xcm_config.rs
+++ b/templates/parachain/runtime/src/configs/xcm_config.rs
@@ -26,6 +26,8 @@ parameter_types! {
 	pub const RelayLocation: Location = Location::parent();
 	pub const RelayNetwork: Option<NetworkId> = None;
 	pub RelayChainOrigin: RuntimeOrigin = cumulus_pallet_xcm::Origin::Relay.into();
+	// For the real deployment, it is recommended to set `RelayNetwork` according to the relay chain
+	// and prepend `UniversalLocation` with `GlobalConsensus(RelayNetwork::get())`.
 	pub UniversalLocation: InteriorLocation = Parachain(ParachainInfo::parachain_id().into()).into();
 }
 
-- 
GitLab