Ensure xcm versions over bridge (on sending chains)

bridges/bin/runtime-common/src/messages_xcm_extension.rs

@@ -146,6 +146,8 @@ pub trait XcmBlobHauler {
 	type MessagesInstance: 'static;
 	/// Returns lane used by this hauler.
 	type SenderAndLane: Get<SenderAndLane>;
+	/// Checks the XCM version for the destination.
+	type DestinationVersion: GetVersion;
 
 	/// Actual XCM message sender (`HRMP` or `UMP`) to the source chain
 	/// location (`Self::SenderAndLane::get().location`).
@@ -202,6 +204,12 @@ where
 	}
 }
 
+impl<H: XcmBlobHauler> GetVersion for XcmBlobHaulerAdapter<H> {
+	fn get_version_for(dest: &MultiLocation) -> Option<XcmVersion> {
+		H::DestinationVersion::get_version_for(dest)
+	}
+}
+
 impl<H: XcmBlobHauler> OnMessagesDelivered for XcmBlobHaulerAdapter<H> {
 	fn on_messages_delivered(lane: LaneId, enqueued_messages: MessageNonce) {
 		let sender_and_lane = H::SenderAndLane::get();
@@ -342,6 +350,28 @@ impl<H: XcmBlobHauler> LocalXcmQueueManager<H> {
 	}
 }
 
+/// Adapter for the implementation of `GetVersion`, which attempts to find the minimal
+/// configured XCM version between the destination `dest` and the bridge hub location provided as
+/// `Get<Location>`.
+pub struct MinXcmVersionOfDestinationAndRemoteBridgeHub<Version, RemoteBridgeHub>(
+	sp_std::marker::PhantomData<(Version, RemoteBridgeHub)>,
+);
+impl<Version: GetVersion, RemoteBridgeHub: Get<MultiLocation>> GetVersion
+	for MinXcmVersionOfDestinationAndRemoteBridgeHub<Version, RemoteBridgeHub>
+{
+	fn get_version_for(dest: &MultiLocation) -> Option<XcmVersion> {
+		let dest_version = Version::get_version_for(dest);
+		let bridge_hub_version = Version::get_version_for(&RemoteBridgeHub::get());
+
+		match (dest_version, bridge_hub_version) {
+			(Some(dv), Some(bhv)) => Some(sp_std::cmp::min(dv, bhv)),
+			(Some(dv), None) => Some(dv),
+			(None, Some(bhv)) => Some(bhv),
+			(None, None) => None,
+		}
+	}
+}
+
 #[cfg(test)]
 mod tests {
 	use super::*;
@@ -390,6 +420,7 @@ mod tests {
 		type Runtime = TestRuntime;
 		type MessagesInstance = ();
 		type SenderAndLane = TestSenderAndLane;
+		type DestinationVersion = AlwaysLatest;
 
 		type ToSourceChainSender = DummySendXcm;
 		type CongestedMessage = DummyXcmMessage;

bridges/modules/xcm-bridge-hub-router/src/lib.rs

@@ -89,6 +89,8 @@ pub mod pallet {
 		/// **possible fee**. Allows to externalize better control over allowed **bridged
 		/// networks/locations**.
 		type Bridges: ExporterFor;
+		/// Checks the XCM version for the destination.
+		type DestinationVersion: GetVersion;
 
 		/// Origin of the sibling bridge hub that is allowed to report bridge status.
 		type BridgeHubOrigin: EnsureOrigin<Self::RuntimeOrigin>;
@@ -319,19 +321,32 @@ impl<T: Config<I>, I: 'static> SendXcm for Pallet<T, I> {
 		dest: &mut Option<MultiLocation>,
 		xcm: &mut Option<Xcm<()>>,
 	) -> SendResult<Self::Ticket> {
-		// we won't have an access to `dest` and `xcm` in the `delvier` method, so precompute
+		// `dest` and `xcm` are required here
+		let dest_ref = dest.as_ref().ok_or(SendError::MissingArgument)?;
+		let xcm_ref = xcm.as_ref().ok_or(SendError::MissingArgument)?;
+
+		// we won't have an access to `dest` and `xcm` in the `deliver` method, so precompute
 		// everything required here
-		let message_size = xcm
-			.as_ref()
-			.map(|xcm| xcm.encoded_size() as _)
-			.ok_or(SendError::MissingArgument)?;
+		let message_size = xcm_ref.encoded_size() as _;
 
 		// bridge doesn't support oversized/overweight messages now. So it is better to drop such
 		// messages here than at the bridge hub. Let's check the message size.
 		if message_size > HARD_MESSAGE_SIZE_LIMIT {
 			return Err(SendError::ExceedsMaxMessageSize)
 		}
 
+		// We need to ensure that the known `dest`'s XCM version can comprehend the current `xcm`
+		// program. This may seem like an additional, unnecessary check, but it is not. A similar
+		// check is probably performed by the `ViaBridgeHubExporter`, which attempts to send a
+		// versioned message to the sibling bridge hub. However, the local bridge hub may have a
+		// higher XCM version than the remote `dest`. Once again, it is better to discard such
+		// messages here than at the bridge hub (e.g., to avoid losing funds).
+		let destination_version = T::DestinationVersion::get_version_for(dest_ref)
+			.ok_or(SendError::DestinationUnsupported)?;
+		let _ = VersionedXcm::from(xcm_ref.clone())
+			.into_version(destination_version)
+			.map_err(|()| SendError::DestinationUnsupported)?;
+
 		// just use exporter to validate destination and insert instructions to pay message fee
 		// at the sibling/child bridge hub
 		//
@@ -358,6 +373,7 @@ impl<T: Config<I>, I: 'static> SendXcm for Pallet<T, I> {
 #[cfg(test)]
 mod tests {
 	use super::*;
+	use frame_support::assert_ok;
 	use mock::*;
 
 	use frame_support::traits::Hooks;
@@ -451,6 +467,19 @@ mod tests {
 		});
 	}
 
+	#[test]
+	fn destination_unsupported_if_wrap_version_fails() {
+		run_test(|| {
+			assert_eq!(
+				send_xcm::<XcmBridgeHubRouter>(
+					UnknownXcmVersionLocation::get(),
+					vec![ClearOrigin].into(),
+				),
+				Err(SendError::DestinationUnsupported),
+			);
+		});
+	}
+
 	#[test]
 	fn returns_proper_delivery_price() {
 		run_test(|| {
@@ -488,17 +517,14 @@ mod tests {
 	fn sent_message_doesnt_increase_factor_if_xcm_channel_is_uncongested() {
 		run_test(|| {
 			let old_bridge = XcmBridgeHubRouter::bridge();
-			assert_eq!(
-				send_xcm::<XcmBridgeHubRouter>(
-					MultiLocation::new(
-						2,
-						X2(GlobalConsensus(BridgedNetworkId::get()), Parachain(1000))
-					),
-					vec![ClearOrigin].into(),
-				)
-				.map(drop),
-				Ok(()),
-			);
+			assert_ok!(send_xcm::<XcmBridgeHubRouter>(
+				MultiLocation::new(
+					2,
+					X2(GlobalConsensus(BridgedNetworkId::get()), Parachain(1000))
+				),
+				vec![ClearOrigin].into(),
+			)
+			.map(drop));
 
 			assert!(TestToBridgeHubSender::is_message_sent());
 			assert_eq!(old_bridge, XcmBridgeHubRouter::bridge());
@@ -511,17 +537,14 @@ mod tests {
 			TestWithBridgeHubChannel::make_congested();
 
 			let old_bridge = XcmBridgeHubRouter::bridge();
-			assert_eq!(
-				send_xcm::<XcmBridgeHubRouter>(
-					MultiLocation::new(
-						2,
-						X2(GlobalConsensus(BridgedNetworkId::get()), Parachain(1000))
-					),
-					vec![ClearOrigin].into(),
-				)
-				.map(drop),
-				Ok(()),
-			);
+			assert_ok!(send_xcm::<XcmBridgeHubRouter>(
+				MultiLocation::new(
+					2,
+					X2(GlobalConsensus(BridgedNetworkId::get()), Parachain(1000))
+				),
+				vec![ClearOrigin].into(),
+			)
+			.map(drop));
 
 			assert!(TestToBridgeHubSender::is_message_sent());
 			assert!(
@@ -536,17 +559,14 @@ mod tests {
 			Bridge::<TestRuntime, ()>::put(congested_bridge(MINIMAL_DELIVERY_FEE_FACTOR));
 
 			let old_bridge = XcmBridgeHubRouter::bridge();
-			assert_eq!(
-				send_xcm::<XcmBridgeHubRouter>(
-					MultiLocation::new(
-						2,
-						X2(GlobalConsensus(BridgedNetworkId::get()), Parachain(1000))
-					),
-					vec![ClearOrigin].into(),
-				)
-				.map(drop),
-				Ok(()),
-			);
+			assert_ok!(send_xcm::<XcmBridgeHubRouter>(
+				MultiLocation::new(
+					2,
+					X2(GlobalConsensus(BridgedNetworkId::get()), Parachain(1000))
+				),
+				vec![ClearOrigin].into(),
+			)
+			.map(drop));
 
 			assert!(TestToBridgeHubSender::is_message_sent());
 			assert!(

bridges/modules/xcm-bridge-hub-router/src/mock.rs

@@ -19,7 +19,10 @@
 use crate as pallet_xcm_bridge_hub_router;
 
 use bp_xcm_bridge_hub_router::XcmChannelStatusProvider;
-use frame_support::{construct_runtime, derive_impl, parameter_types};
+use frame_support::{
+	construct_runtime, derive_impl, parameter_types,
+	traits::{Contains, Equals},
+};
 use frame_system::EnsureRoot;
 use sp_runtime::{traits::ConstU128, BuildStorage};
 use xcm::prelude::*;
@@ -58,6 +61,7 @@ parameter_types! {
 				Some((BridgeFeeAsset::get(), BASE_FEE).into())
 			)
 		];
+	pub UnknownXcmVersionLocation: MultiLocation = MultiLocation::new(2, X2(GlobalConsensus(BridgedNetworkId::get()), Parachain(9999)));
 }
 
 #[derive_impl(frame_system::config_preludes::TestDefaultConfig as frame_system::DefaultConfig)]
@@ -71,6 +75,8 @@ impl pallet_xcm_bridge_hub_router::Config<()> for TestRuntime {
 	type UniversalLocation = UniversalLocation;
 	type BridgedNetworkId = BridgedNetworkId;
 	type Bridges = NetworkExportTable<BridgeTable>;
+	type DestinationVersion =
+		LatestOrNoneForLocationVersionChecker<Equals<UnknownXcmVersionLocation>>;
 
 	type BridgeHubOrigin = EnsureRoot<AccountId>;
 	type ToBridgeHubSender = TestToBridgeHubSender;
@@ -80,6 +86,18 @@ impl pallet_xcm_bridge_hub_router::Config<()> for TestRuntime {
 	type FeeAsset = BridgeFeeAsset;
 }
 
+pub struct LatestOrNoneForLocationVersionChecker<Location>(sp_std::marker::PhantomData<Location>);
+impl<Location: Contains<MultiLocation>> GetVersion
+	for LatestOrNoneForLocationVersionChecker<Location>
+{
+	fn get_version_for(dest: &MultiLocation) -> Option<XcmVersion> {
+		if Location::contains(dest) {
+			return None
+		}
+		Some(XCM_VERSION)
+	}
+}
+
 pub struct TestToBridgeHubSender;
 
 impl TestToBridgeHubSender {

cumulus/parachains/integration-tests/emulated/chains/parachains/assets/asset-hub-rococo/src/lib.rs

@@ -21,8 +21,8 @@ use frame_support::traits::OnInitialize;
 // Cumulus
 use emulated_integration_tests_common::{
 	impl_accounts_helpers_for_parachain, impl_assert_events_helpers_for_parachain,
-	impl_assets_helpers_for_parachain, impl_foreign_assets_helpers_for_parachain, impls::Parachain,
-	xcm_emulator::decl_test_parachains,
+	impl_assets_helpers_for_parachain, impl_foreign_assets_helpers_for_parachain,
+	impl_xcm_helpers_for_parachain, impls::Parachain, xcm_emulator::decl_test_parachains,
 };
 use rococo_emulated_chain::Rococo;
 
@@ -55,3 +55,4 @@ impl_accounts_helpers_for_parachain!(AssetHubRococo);
 impl_assert_events_helpers_for_parachain!(AssetHubRococo);
 impl_assets_helpers_for_parachain!(AssetHubRococo, Rococo);
 impl_foreign_assets_helpers_for_parachain!(AssetHubRococo, Rococo);
+impl_xcm_helpers_for_parachain!(AssetHubRococo);

cumulus/parachains/integration-tests/emulated/chains/parachains/assets/asset-hub-westend/src/lib.rs

@@ -21,8 +21,8 @@ use frame_support::traits::OnInitialize;
 // Cumulus
 use emulated_integration_tests_common::{
 	impl_accounts_helpers_for_parachain, impl_assert_events_helpers_for_parachain,
-	impl_assets_helpers_for_parachain, impl_foreign_assets_helpers_for_parachain, impls::Parachain,
-	xcm_emulator::decl_test_parachains,
+	impl_assets_helpers_for_parachain, impl_foreign_assets_helpers_for_parachain,
+	impl_xcm_helpers_for_parachain, impls::Parachain, xcm_emulator::decl_test_parachains,
 };
 use westend_emulated_chain::Westend;
 
@@ -55,3 +55,4 @@ impl_accounts_helpers_for_parachain!(AssetHubWestend);
 impl_assert_events_helpers_for_parachain!(AssetHubWestend);
 impl_assets_helpers_for_parachain!(AssetHubWestend, Westend);
 impl_foreign_assets_helpers_for_parachain!(AssetHubWestend, Westend);
+impl_xcm_helpers_for_parachain!(AssetHubWestend);

cumulus/parachains/integration-tests/emulated/chains/parachains/bridges/bridge-hub-rococo/src/lib.rs

@@ -21,7 +21,7 @@ use frame_support::traits::OnInitialize;
 // Cumulus
 use emulated_integration_tests_common::{
 	impl_accounts_helpers_for_parachain, impl_assert_events_helpers_for_parachain,
-	impls::Parachain, xcm_emulator::decl_test_parachains,
+	impl_xcm_helpers_for_parachain, impls::Parachain, xcm_emulator::decl_test_parachains,
 };
 
 // BridgeHubRococo Parachain declaration
@@ -47,3 +47,4 @@ decl_test_parachains! {
 // BridgeHubRococo implementation
 impl_accounts_helpers_for_parachain!(BridgeHubRococo);
 impl_assert_events_helpers_for_parachain!(BridgeHubRococo);
+impl_xcm_helpers_for_parachain!(BridgeHubRococo);

cumulus/parachains/integration-tests/emulated/chains/parachains/bridges/bridge-hub-westend/src/lib.rs

@@ -21,7 +21,7 @@ use frame_support::traits::OnInitialize;
 // Cumulus
 use emulated_integration_tests_common::{
 	impl_accounts_helpers_for_parachain, impl_assert_events_helpers_for_parachain,
-	impls::Parachain, xcm_emulator::decl_test_parachains,
+	impl_xcm_helpers_for_parachain, impls::Parachain, xcm_emulator::decl_test_parachains,
 };
 
 // BridgeHubWestend Parachain declaration
@@ -47,3 +47,4 @@ decl_test_parachains! {
 // BridgeHubWestend implementation
 impl_accounts_helpers_for_parachain!(BridgeHubWestend);
 impl_assert_events_helpers_for_parachain!(BridgeHubWestend);
+impl_xcm_helpers_for_parachain!(BridgeHubWestend);

cumulus/parachains/integration-tests/emulated/common/src/impls.rs

@@ -38,7 +38,7 @@ pub use polkadot_runtime_parachains::{
 	inclusion::{AggregateMessageOrigin, UmpQueueId},
 };
 pub use xcm::{
-	prelude::{MultiLocation, OriginKind, Outcome, VersionedXcm},
+	prelude::{MultiLocation, OriginKind, Outcome, VersionedXcm, XcmVersion},
 	v3::Error,
 	DoubleEncoded,
 };
@@ -790,3 +790,23 @@ macro_rules! impl_foreign_assets_helpers_for_parachain {
 		}
 	};
 }
+
+#[macro_export]
+macro_rules! impl_xcm_helpers_for_parachain {
+	( $chain:ident ) => {
+		$crate::impls::paste::paste! {
+			impl<N: $crate::impls::Network> $chain<N> {
+				/// Set XCM version for destination
+				pub fn force_xcm_version(dest: $crate::impls::MultiLocation, version: $crate::impls::XcmVersion) {
+					<Self as $crate::impls::TestExt>::execute_with(|| {
+						$crate::impls::assert_ok!(<Self as [<$chain ParaPallet>]>::PolkadotXcm::force_xcm_version(
+							<Self as $crate::impls::Chain>::RuntimeOrigin::root(),
+							$crate::impls::bx!(dest),
+							version,
+						));
+					});
+				}
+			}
+		}
+	}
+}

cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/asset_transfers.rs

@@ -13,7 +13,13 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use crate::*;
+use crate::{
+	tests::{
+		asset_hub_rococo_set_xcm_version_for_asset_hub_westend,
+		bridge_hub_rococo_set_xcm_version_for_bridge_hub_westend,
+	},
+	*,
+};
 
 fn send_asset_from_asset_hub_rococo_to_asset_hub_westend(id: MultiLocation, amount: u128) {
 	let signed_origin =
@@ -34,6 +40,9 @@ fn send_asset_from_asset_hub_rococo_to_asset_hub_westend(id: MultiLocation, amou
 	let sov_ahr_on_bhr = BridgeHubRococo::sovereign_account_id_of(ahr_as_seen_by_bhr);
 	BridgeHubRococo::fund_accounts(vec![(sov_ahr_on_bhr.into(), 10_000_000_000_000u128)]);
 
+	asset_hub_rococo_set_xcm_version_for_asset_hub_westend(XCM_VERSION);
+	bridge_hub_rococo_set_xcm_version_for_bridge_hub_westend(XCM_VERSION);
+
 	AssetHubRococo::execute_with(|| {
 		assert_ok!(
 			<AssetHubRococo as AssetHubRococoPallet>::PolkadotXcm::limited_reserve_transfer_assets(