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

xcm: fix for DenyThenTry Barrier #7169

Merged
merged 52 commits into from
Jan 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
faa431a
Fix for DenyThenTry
yrong Jan 15, 2025
d0748cf
Apply suggestions from code review
bkontur Jan 15, 2025
bf669bb
Rename to DenyExportMessageFrom with comments
yrong Jan 15, 2025
716ccdc
Move DenyExportMessageFrom to bridge-hub-common
yrong Jan 15, 2025
17c9b14
Merge branch 'fix-for-deny-then-try' of https://github.com/yrong/polk…
yrong Jan 15, 2025
e10587b
Update comments
yrong Jan 15, 2025
d83dadb
Add the prdoc
yrong Jan 15, 2025
0b4c955
Merge branch 'master' into fix-for-deny-then-try
yrong Jan 15, 2025
ac810f7
Refactor DenyExecution to be more explicit
yrong Jan 15, 2025
8f0af5b
Update prdoc/pr_7169.prdoc
yrong Jan 15, 2025
67e7991
Merge branch 'master' into fix-for-deny-then-try
yrong Jan 15, 2025
ee8de3e
Merge branch 'master' into fix-for-deny-then-try
yrong Jan 16, 2025
2b496e5
Update polkadot/xcm/xcm-executor/src/traits/should_execute.rs
bkontur Jan 16, 2025
81ec7f1
Update polkadot/xcm/xcm-executor/src/traits/should_execute.rs
bkontur Jan 16, 2025
17c4972
Improve tests
yrong Jan 16, 2025
58d7990
Test for DenyReserveTransferToRelayChain
yrong Jan 16, 2025
2258ab1
Merge branch 'fix-for-deny-then-try' of https://github.com/yrong/polk…
yrong Jan 16, 2025
dbfe244
Merge branch 'master' into fix-for-deny-then-try
yrong Jan 17, 2025
462c049
Apply suggestions from code review
bkontur Jan 17, 2025
6e578bf
Update polkadot/xcm/xcm-builder/src/tests/barriers.rs
bkontur Jan 17, 2025
e4acb39
Update from bkontur running command 'fmt'
Jan 17, 2025
8b94963
Move dummy impls to tests
yrong Jan 17, 2025
b2f7338
More tests
yrong Jan 17, 2025
a8a7ea3
Merge branch 'fix-for-deny-then-try' of https://github.com/yrong/polk…
yrong Jan 17, 2025
a78449e
Fix comments
yrong Jan 17, 2025
ddba5dc
Apply suggestions from code review
bkontur Jan 17, 2025
66814c9
Update polkadot/xcm/xcm-executor/src/traits/should_execute.rs
bkontur Jan 21, 2025
02912a6
Update cumulus/parachains/runtimes/bridge-hubs/common/tests/tests.rs
bkontur Jan 21, 2025
5d3a5e7
Merge branch 'master' into fix-for-deny-then-try
bkontur Jan 21, 2025
85b06c2
Add comments
yrong Jan 21, 2025
dfbafc5
Merge branch 'master' into fix-for-deny-then-try
yrong Jan 21, 2025
a842c4d
Fix clippy
yrong Jan 22, 2025
c7cd51d
Merge branch 'fix-for-deny-then-try' of https://github.com/yrong/polk…
yrong Jan 22, 2025
551afc1
Update polkadot/xcm/xcm-executor/src/traits/should_execute.rs
yrong Jan 22, 2025
cd33044
Update polkadot/xcm/xcm-executor/src/traits/should_execute.rs
yrong Jan 22, 2025
639177b
Update polkadot/xcm/xcm-executor/src/traits/should_execute.rs
yrong Jan 22, 2025
7564e23
Update cumulus/parachains/runtimes/bridge-hubs/common/src/barriers.rs
yrong Jan 22, 2025
d9c2f0e
Update cumulus/parachains/runtimes/bridge-hubs/common/src/barriers.rs
yrong Jan 22, 2025
a04aae2
Merge branch 'fix-for-deny-then-try' of https://github.com/yrong/polk…
yrong Jan 22, 2025
c038e86
Merge branch 'master' into fix-for-deny-then-try
yrong Jan 22, 2025
234a5e7
Merge branch 'master' into fix-for-deny-then-try
franciscoaguirre Jan 24, 2025
b39f07e
Update polkadot/xcm/xcm-builder/src/tests/barriers.rs
yrong Jan 25, 2025
110d42e
Update polkadot/xcm/xcm-builder/src/tests/barriers.rs
yrong Jan 25, 2025
8699575
Update cumulus/parachains/runtimes/bridge-hubs/common/src/barriers.rs
yrong Jan 25, 2025
3cd6bf5
Merge branch 'master' into fix-for-deny-then-try
yrong Jan 25, 2025
6b65c81
Change return to Result<(), ProcessMessageError>
yrong Jan 25, 2025
03ff441
Fix tests
yrong Jan 25, 2025
8d61bfb
Merge branch 'master' into fix-for-deny-then-try
yrong Jan 26, 2025
c2719f9
Update polkadot/xcm/xcm-executor/src/traits/should_execute.rs
bkontur Jan 27, 2025
83835fc
Merge branch 'master' into fix-for-deny-then-try
bkontur Jan 27, 2025
54dadec
Update polkadot/xcm/xcm-executor/src/traits/should_execute.rs
acatangiu Jan 27, 2025
90153db
Merge branch 'master' into fix-for-deny-then-try
acatangiu Jan 27, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock

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

6 changes: 6 additions & 0 deletions cumulus/parachains/runtimes/bridge-hubs/common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand All @@ -32,6 +34,8 @@ std = [
"sp-core/std",
"sp-runtime/std",
"sp-std/std",
"xcm-builder/std",
"xcm-executor/std",
"xcm/std",
]

Expand All @@ -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",
]
57 changes: 57 additions & 0 deletions cumulus/parachains/runtimes/bridge-hubs/common/src/barriers.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
// 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,
) -> Result<(), ProcessMessageError> {
// This barrier only cares about messages with `origin` matching `FromOrigin`.
if !FromOrigin::contains(origin) {
return Ok(())
}
message.matcher().match_next_inst_while(
|_| true,
|inst| match inst {
ExportMessage { network, .. } if ToGlobalConsensus::contains(network) =>
Err(ProcessMessageError::Unsupported),
_ => Ok(ControlFlow::Continue(())),
},
)?;
Ok(())
}
}
2 changes: 2 additions & 0 deletions cumulus/parachains/runtimes/bridge-hubs/common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
85 changes: 85 additions & 0 deletions cumulus/parachains/runtimes/bridge-hubs/common/tests/tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// 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]);
}

// Deny ExportMessage when both of the conditions met:
// 1: source != Source1
// 2: network == Remote1
pub type Denied = DenyExportMessageFrom<EverythingBut<Equals<Source1>>, Equals<Remote1>>;
bkontur marked this conversation as resolved.
Show resolved Hide resolved

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
);
};

// A message without an `ExportMessage` should pass
assert_deny_execution(vec![AliasOrigin(Here.into())], Source1::get(), Ok(()));

// `ExportMessage` from source1 and remote1 should pass
acatangiu marked this conversation as resolved.
Show resolved Hide resolved
assert_deny_execution(
vec![ExportMessage { network: Remote1::get(), destination: Here, xcm: Default::default() }],
Source1::get(),
Ok(()),
);

// `ExportMessage` from source1 and remote2 should pass
assert_deny_execution(
vec![ExportMessage { network: Remote2::get(), destination: Here, xcm: Default::default() }],
Source1::get(),
Ok(()),
);

// `ExportMessage` from source2 and remote2 should pass
assert_deny_execution(
vec![ExportMessage { network: Remote2::get(), destination: Here, xcm: Default::default() }],
Source2::get(),
Ok(()),
);

// `ExportMessage` from source2 and remote1 should be banned
assert_deny_execution(
vec![ExportMessage { network: Remote1::get(), destination: Here, xcm: Default::default() }],
Source2::get(),
Err(Unsupported),
);
}
14 changes: 6 additions & 8 deletions polkadot/xcm/xcm-builder/src/barriers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
///
Expand Down Expand Up @@ -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>(
Expand All @@ -458,15 +458,15 @@ where
max_weight: Weight,
properties: &mut Properties,
) -> Result<(), ProcessMessageError> {
Deny::should_execute(origin, message, max_weight, properties)?;
Deny::deny_execution(origin, message, max_weight, properties)?;
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,
Expand Down Expand Up @@ -499,8 +499,6 @@ impl ShouldExecute for DenyReserveTransferToRelayChain {
_ => Ok(ControlFlow::Continue(())),
},
)?;

// Permit everything else
Ok(())
}
}
Loading
Loading