xcm: fix for DenyThenTry Barrier

cumulus/parachains/runtimes/bridge-hubs/common/Cargo.toml

@@ -19,6 +19,8 @@ sp-core = { workspace = true }
 sp-runtime = { workspace = true }
 sp-std = { workspace = true }
 xcm = { workspace = true }
+xcm-builder = { workspace = true }
+xcm-executor = { workspace = true }
 
 [features]
 default = ["std"]
@@ -32,6 +34,8 @@ std = [
 	"sp-core/std",
 	"sp-runtime/std",
 	"sp-std/std",
+	"xcm-builder/std",
+	"xcm-executor/std",
 	"xcm/std",
 ]
 
@@ -41,5 +45,7 @@ runtime-benchmarks = [
 	"pallet-message-queue/runtime-benchmarks",
 	"snowbridge-core/runtime-benchmarks",
 	"sp-runtime/runtime-benchmarks",
+	"xcm-builder/runtime-benchmarks",
+	"xcm-executor/runtime-benchmarks",
 	"xcm/runtime-benchmarks",
 ]

cumulus/parachains/runtimes/bridge-hubs/common/src/barriers.rs

@@ -0,0 +1,59 @@
+// Copyright (C) Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: Apache-2.0
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use core::{marker::PhantomData, ops::ControlFlow};
+use cumulus_primitives_core::Weight;
+use frame_support::traits::{Contains, ProcessMessageError};
+use xcm::prelude::{ExportMessage, Instruction, Location, NetworkId};
+
+use xcm_builder::{CreateMatcher, MatchXcm};
+use xcm_executor::traits::{DenyExecution, Properties};
+
+/// Deny execution if the message contains instruction `ExportMessage` with
+/// a. origin is contained in `FromOrigin` (i.e.`FromOrigin::Contains(origin)`)
+/// b. network is contained in `ToGlobalConsensus`, (i.e. `ToGlobalConsensus::contains(network)`)
+pub struct DenyExportMessageFrom<FromOrigin, ToGlobalConsensus>(
+	PhantomData<(FromOrigin, ToGlobalConsensus)>,
+);
+
+impl<FromOrigin, ToGlobalConsensus> DenyExecution
+	for DenyExportMessageFrom<FromOrigin, ToGlobalConsensus>
+where
+	FromOrigin: Contains<Location>,
+	ToGlobalConsensus: Contains<NetworkId>,
+{
+	fn deny_execution<RuntimeCall>(
+		origin: &Location,
+		message: &mut [Instruction<RuntimeCall>],
+		_max_weight: Weight,
+		_properties: &mut Properties,
+	) -> Option<ProcessMessageError> {
+		match message.matcher().match_next_inst_while(
+			|_| true,
+			|inst| match inst {
+				ExportMessage { network, .. } =>
+					if ToGlobalConsensus::contains(network) && FromOrigin::contains(origin) {
+						return Err(ProcessMessageError::Unsupported)
+					} else {
+						Ok(ControlFlow::Continue(()))
+					},
+				_ => Ok(ControlFlow::Continue(())),
+			},
+		) {
+			Ok(_) => None,
+			Err(error) => Some(error),
+		}
+	}
+}

cumulus/parachains/runtimes/bridge-hubs/common/src/lib.rs

@@ -14,9 +14,11 @@
 // limitations under the License.
 #![cfg_attr(not(feature = "std"), no_std)]
 
+pub mod barriers;
 pub mod digest_item;
 pub mod message_queue;
 pub mod xcm_version;
 
+pub use barriers::DenyExportMessageFrom;
 pub use digest_item::CustomDigestItem;
 pub use message_queue::{AggregateMessageOrigin, BridgeHubMessageRouter};

cumulus/parachains/runtimes/bridge-hubs/common/tests/tests.rs

@@ -0,0 +1,82 @@
+// Copyright (C) Parity Technologies (UK) Ltd.
+// This file is part of Cumulus.
+
+// Cumulus is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Cumulus is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Cumulus.  If not, see <http://www.gnu.org/licenses/>.
+
+#![cfg(test)]
+use bridge_hub_common::DenyExportMessageFrom;
+use frame_support::{
+	parameter_types,
+	traits::{Equals, EverythingBut, ProcessMessageError::Unsupported},
+};
+use xcm::prelude::{
+	AliasOrigin, ByGenesis, ExportMessage, Here, Instruction, Location, NetworkId, Parachain,
+	Weight,
+};
+use xcm_executor::traits::{DenyExecution, Properties};
+
+#[test]
+fn test_deny_export_message_from() {
+	parameter_types! {
+		pub Source1: Location = Location::new(1, Parachain(1));
+		pub Source2: Location = Location::new(1, Parachain(2));
+		pub Remote1: NetworkId = ByGenesis([1;32]);
+		pub Remote2: NetworkId = ByGenesis([2;32]);
+	}
+
+	pub type Denied = DenyExportMessageFrom<EverythingBut<Equals<Source1>>, Equals<Remote1>>;
+
+	let assert_deny_execution = |mut xcm: Vec<Instruction<()>>, origin, expected_result| {
+		assert_eq!(
+			Denied::deny_execution(
+				&origin,
+				&mut xcm,
+				Weight::zero(),
+				&mut Properties { weight_credit: Weight::zero(), message_id: None }
+			),
+			expected_result
+		);
+	};
+
+	// No `ExportMessage` should pass
+	assert_deny_execution(vec![AliasOrigin(Here.into())], Source1::get(), None);
+
+	// `ExportMessage` from source1 and remote1 should pass
+	assert_deny_execution(
+		vec![ExportMessage { network: Remote1::get(), destination: Here, xcm: Default::default() }],
+		Source1::get(),
+		None,
+	);
+
+	// `ExportMessage` from source1 and remote2 should pass
+	assert_deny_execution(
+		vec![ExportMessage { network: Remote2::get(), destination: Here, xcm: Default::default() }],
+		Source1::get(),
+		None,
+	);
+
+	// `ExportMessage` from source2 and remote2 should pass
+	assert_deny_execution(
+		vec![ExportMessage { network: Remote2::get(), destination: Here, xcm: Default::default() }],
+		Source2::get(),
+		None,
+	);
+
+	// `ExportMessage` from source2 and remote1 should be banned
+	assert_deny_execution(
+		vec![ExportMessage { network: Remote1::get(), destination: Here, xcm: Default::default() }],
+		Source2::get(),
+		Some(Unsupported),
+	);
+}

polkadot/xcm/xcm-builder/src/barriers.rs

@@ -24,7 +24,7 @@ use frame_support::{
 };
 use polkadot_parachain_primitives::primitives::IsSystem;
 use xcm::prelude::*;
-use xcm_executor::traits::{CheckSuspension, OnResponse, Properties, ShouldExecute};
+use xcm_executor::traits::{CheckSuspension, DenyExecution, OnResponse, Properties, ShouldExecute};
 
 /// Execution barrier that just takes `max_weight` from `properties.weight_credit`.
 ///
@@ -444,12 +444,12 @@ impl ShouldExecute for AllowHrmpNotificationsFromRelayChain {
 /// If it passes the Deny, and matches one of the Allow cases then it is let through.
 pub struct DenyThenTry<Deny, Allow>(PhantomData<Deny>, PhantomData<Allow>)
 where
-	Deny: ShouldExecute,
+	Deny: DenyExecution,
 	Allow: ShouldExecute;
 
 impl<Deny, Allow> ShouldExecute for DenyThenTry<Deny, Allow>
 where
-	Deny: ShouldExecute,
+	Deny: DenyExecution,
 	Allow: ShouldExecute,
 {
 	fn should_execute<RuntimeCall>(
@@ -458,21 +458,23 @@ where
 		max_weight: Weight,
 		properties: &mut Properties,
 	) -> Result<(), ProcessMessageError> {
-		Deny::should_execute(origin, message, max_weight, properties)?;
-		Allow::should_execute(origin, message, max_weight, properties)
+		match Deny::deny_execution(origin, message, max_weight, properties) {
+			Some(err) => Err(err),
+			None => Allow::should_execute(origin, message, max_weight, properties),
+		}
 	}
 }
 
 // See issue <https://github.com/paritytech/polkadot/issues/5233>
 pub struct DenyReserveTransferToRelayChain;
-impl ShouldExecute for DenyReserveTransferToRelayChain {
-	fn should_execute<RuntimeCall>(
+impl DenyExecution for DenyReserveTransferToRelayChain {
+	fn deny_execution<RuntimeCall>(
 		origin: &Location,
 		message: &mut [Instruction<RuntimeCall>],
 		_max_weight: Weight,
 		_properties: &mut Properties,
-	) -> Result<(), ProcessMessageError> {
-		message.matcher().match_next_inst_while(
+	) -> Option<ProcessMessageError> {
+		match message.matcher().match_next_inst_while(
 			|_| true,
 			|inst| match inst {
 				InitiateReserveWithdraw {
@@ -498,9 +500,9 @@ impl ShouldExecute for DenyReserveTransferToRelayChain {
 
 				_ => Ok(ControlFlow::Continue(())),
 			},
-		)?;
-
-		// Permit everything else
-		Ok(())
+		) {
+			Ok(_) => None,
+			Err(err) => Some(err),
+		}
 	}
 }