From fafe1f1b47c39e1e49befe7f241d758cfc1d25ed Mon Sep 17 00:00:00 2001
From: Squirrel <gilescope@gmail.com>
Date: Wed, 20 Apr 2022 10:14:08 +0100
Subject: [PATCH] Deny using relay chain for reserve transfers (#1169)
* Deny using relay chain for reserve transfers
* match on junction as well.
* Update polkadot-parachains/parachains-common/src/xcm_config.rs
* Use Err to signal deny
* Deny depisiting reserved assets + fmt
* Allow DepositReserveAssets from relay chain
Deny InitiateReserveWithdrawal
* Rather than allow DepositReserveAsset,
it was ReservAssetDeposited that should be allowed but logged.
* don't reference common parachains.
* Update parachain-template/runtime/src/xcm_config.rs
* Warn if reserve asset deposited msg detected.
Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
---
.../runtime/src/xcm_config.rs | 83 +++++++++++++++++--
.../canvas-kusama/src/xcm_config.rs | 24 +++---
.../parachains-common/src/lib.rs | 2 +
.../parachains-common/src/xcm_config.rs | 67 +++++++++++++++
.../statemine/src/xcm_config.rs | 28 ++++---
.../statemint/src/xcm_config.rs | 28 ++++---
.../westmint/src/xcm_config.rs | 28 ++++---
7 files changed, 209 insertions(+), 51 deletions(-)
create mode 100644 cumulus/polkadot-parachains/parachains-common/src/xcm_config.rs
diff --git a/cumulus/parachain-template/runtime/src/xcm_config.rs b/cumulus/parachain-template/runtime/src/xcm_config.rs
index 75f203f97cd..3c77efba37e 100644
--- a/cumulus/parachain-template/runtime/src/xcm_config.rs
+++ b/cumulus/parachain-template/runtime/src/xcm_config.rs
@@ -2,8 +2,9 @@ use super::{
AccountId, Balances, Call, Event, Origin, ParachainInfo, ParachainSystem, PolkadotXcm, Runtime,
WeightToFee, XcmpQueue,
};
+use core::marker::PhantomData;
use frame_support::{
- match_types, parameter_types,
+ log, match_types, parameter_types,
traits::{Everything, Nothing},
weights::Weight,
};
@@ -18,7 +19,7 @@ use xcm_builder::{
SignedAccountId32AsNative, SignedToAccountId32, SovereignSignedViaLocation, TakeWeightCredit,
UsingComponents,
};
-use xcm_executor::XcmExecutor;
+use xcm_executor::{traits::ShouldExecute, XcmExecutor};
parameter_types! {
pub const RelayLocation: MultiLocation = MultiLocation::parent();
@@ -87,12 +88,78 @@ match_types! {
};
}
-pub type Barrier = (
- TakeWeightCredit,
- AllowTopLevelPaidExecutionFrom<Everything>,
- AllowUnpaidExecutionFrom<ParentOrParentsExecutivePlurality>,
- // ^^^ Parent and its exec plurality get free execution
-);
+//TODO: move DenyThenTry to polkadot's xcm module.
+/// Deny executing the xcm message if it matches any of the Deny filter regardless of anything else.
+/// 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,
+ Allow: ShouldExecute;
+
+impl<Deny, Allow> ShouldExecute for DenyThenTry<Deny, Allow>
+where
+ Deny: ShouldExecute,
+ Allow: ShouldExecute,
+{
+ fn should_execute<Call>(
+ origin: &MultiLocation,
+ message: &mut Xcm<Call>,
+ max_weight: Weight,
+ weight_credit: &mut Weight,
+ ) -> Result<(), ()> {
+ Deny::should_execute(origin, message, max_weight, weight_credit)?;
+ Allow::should_execute(origin, message, max_weight, weight_credit)
+ }
+}
+
+// See issue #5233
+pub struct DenyReserveTransferToRelayChain;
+impl ShouldExecute for DenyReserveTransferToRelayChain {
+ fn should_execute<Call>(
+ origin: &MultiLocation,
+ message: &mut Xcm<Call>,
+ _max_weight: Weight,
+ _weight_credit: &mut Weight,
+ ) -> Result<(), ()> {
+ if message.0.iter().any(|inst| {
+ matches!(
+ inst,
+ InitiateReserveWithdraw {
+ reserve: MultiLocation { parents: 1, interior: Here },
+ ..
+ } | DepositReserveAsset { dest: MultiLocation { parents: 1, interior: Here }, .. } |
+ TransferReserveAsset {
+ dest: MultiLocation { parents: 1, interior: Here },
+ ..
+ }
+ )
+ }) {
+ return Err(()) // Deny
+ }
+
+ // allow reserve transfers to arrive from relay chain
+ if matches!(origin, MultiLocation { parents: 1, interior: Here }) &&
+ message.0.iter().any(|inst| matches!(inst, ReserveAssetDeposited { .. }))
+ {
+ log::warn!(
+ target: "xcm::barriers",
+ "Unexpected ReserveAssetDeposited from the relay chain",
+ );
+ }
+ // Permit everything else
+ Ok(())
+ }
+}
+
+pub type Barrier = DenyThenTry<
+ DenyReserveTransferToRelayChain,
+ (
+ TakeWeightCredit,
+ AllowTopLevelPaidExecutionFrom<Everything>,
+ AllowUnpaidExecutionFrom<ParentOrParentsExecutivePlurality>,
+ // ^^^ Parent and its exec plurality get free execution
+ ),
+>;
pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
diff --git a/cumulus/polkadot-parachains/canvas-kusama/src/xcm_config.rs b/cumulus/polkadot-parachains/canvas-kusama/src/xcm_config.rs
index c129d4d0162..6a33909332c 100644
--- a/cumulus/polkadot-parachains/canvas-kusama/src/xcm_config.rs
+++ b/cumulus/polkadot-parachains/canvas-kusama/src/xcm_config.rs
@@ -24,6 +24,7 @@ use frame_support::{
};
use frame_system::EnsureRoot;
use pallet_xcm::{EnsureXcm, IsMajorityOfBody, XcmPassthrough};
+use parachains_common::xcm_config::{DenyReserveTransferToRelayChain, DenyThenTry};
use polkadot_parachain::primitives::Sibling;
use xcm::latest::prelude::*;
use xcm_builder::{
@@ -117,16 +118,19 @@ match_types! {
};
}
-pub type Barrier = (
- TakeWeightCredit,
- AllowTopLevelPaidExecutionFrom<Everything>,
- // Parent and its exec plurality get free execution
- AllowUnpaidExecutionFrom<ParentOrParentsExecutivePlurality>,
- // Expected responses are OK.
- AllowKnownQueryResponses<PolkadotXcm>,
- // Subscriptions for version tracking are OK.
- AllowSubscriptionsFrom<ParentOrSiblings>,
-);
+pub type Barrier = DenyThenTry<
+ DenyReserveTransferToRelayChain,
+ (
+ TakeWeightCredit,
+ AllowTopLevelPaidExecutionFrom<Everything>,
+ // Parent and its exec plurality get free execution
+ AllowUnpaidExecutionFrom<ParentOrParentsExecutivePlurality>,
+ // Expected responses are OK.
+ AllowKnownQueryResponses<PolkadotXcm>,
+ // Subscriptions for version tracking are OK.
+ AllowSubscriptionsFrom<ParentOrSiblings>,
+ ),
+>;
pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
diff --git a/cumulus/polkadot-parachains/parachains-common/src/lib.rs b/cumulus/polkadot-parachains/parachains-common/src/lib.rs
index c04f0a37798..fcec189a210 100644
--- a/cumulus/polkadot-parachains/parachains-common/src/lib.rs
+++ b/cumulus/polkadot-parachains/parachains-common/src/lib.rs
@@ -16,9 +16,11 @@
#![cfg_attr(not(feature = "std"), no_std)]
pub mod impls;
+pub mod xcm_config;
pub use constants::*;
pub use opaque::*;
pub use types::*;
+
/// Common types of parachains.
mod types {
use sp_runtime::traits::{IdentifyAccount, Verify};
diff --git a/cumulus/polkadot-parachains/parachains-common/src/xcm_config.rs b/cumulus/polkadot-parachains/parachains-common/src/xcm_config.rs
new file mode 100644
index 00000000000..63b388acaeb
--- /dev/null
+++ b/cumulus/polkadot-parachains/parachains-common/src/xcm_config.rs
@@ -0,0 +1,67 @@
+use core::marker::PhantomData;
+use frame_support::{log, weights::Weight};
+use xcm::latest::prelude::*;
+use xcm_executor::traits::ShouldExecute;
+
+//TODO: move DenyThenTry to polkadot's xcm module.
+/// Deny executing the XCM if it matches any of the Deny filter regardless of anything else.
+/// 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,
+ Allow: ShouldExecute;
+
+impl<Deny, Allow> ShouldExecute for DenyThenTry<Deny, Allow>
+where
+ Deny: ShouldExecute,
+ Allow: ShouldExecute,
+{
+ fn should_execute<Call>(
+ origin: &MultiLocation,
+ message: &mut Xcm<Call>,
+ max_weight: Weight,
+ weight_credit: &mut Weight,
+ ) -> Result<(), ()> {
+ Deny::should_execute(origin, message, max_weight, weight_credit)?;
+ Allow::should_execute(origin, message, max_weight, weight_credit)
+ }
+}
+
+// See issue #5233
+pub struct DenyReserveTransferToRelayChain;
+impl ShouldExecute for DenyReserveTransferToRelayChain {
+ fn should_execute<Call>(
+ origin: &MultiLocation,
+ message: &mut Xcm<Call>,
+ _max_weight: Weight,
+ _weight_credit: &mut Weight,
+ ) -> Result<(), ()> {
+ if message.0.iter().any(|inst| {
+ matches!(
+ inst,
+ InitiateReserveWithdraw {
+ reserve: MultiLocation { parents: 1, interior: Here },
+ ..
+ } | DepositReserveAsset { dest: MultiLocation { parents: 1, interior: Here }, .. } |
+ TransferReserveAsset {
+ dest: MultiLocation { parents: 1, interior: Here },
+ ..
+ }
+ )
+ }) {
+ return Err(()) // Deny
+ }
+
+ // allow reserve transfers to arrive from relay chain
+ if matches!(origin, MultiLocation { parents: 1, interior: Here }) &&
+ message.0.iter().any(|inst| matches!(inst, ReserveAssetDeposited { .. }))
+ {
+ log::info!(
+ target: "runtime::xcm-barier",
+ "Unexpected Reserve Assets Deposited on the relay chain",
+ );
+ }
+ // Permit everything else
+ Ok(())
+ }
+}
diff --git a/cumulus/polkadot-parachains/statemine/src/xcm_config.rs b/cumulus/polkadot-parachains/statemine/src/xcm_config.rs
index 9bf8b827f1f..7ace1bf2b32 100644
--- a/cumulus/polkadot-parachains/statemine/src/xcm_config.rs
+++ b/cumulus/polkadot-parachains/statemine/src/xcm_config.rs
@@ -23,7 +23,10 @@ use frame_support::{
weights::Weight,
};
use pallet_xcm::XcmPassthrough;
-use parachains_common::impls::ToStakingPot;
+use parachains_common::{
+ impls::ToStakingPot,
+ xcm_config::{DenyReserveTransferToRelayChain, DenyThenTry},
+};
use polkadot_parachain::primitives::Sibling;
use xcm::latest::prelude::*;
use xcm_builder::{
@@ -139,16 +142,19 @@ match_types! {
};
}
-pub type Barrier = (
- TakeWeightCredit,
- AllowTopLevelPaidExecutionFrom<Everything>,
- // Parent and its exec plurality get free execution
- AllowUnpaidExecutionFrom<ParentOrParentsExecutivePlurality>,
- // Expected responses are OK.
- AllowKnownQueryResponses<PolkadotXcm>,
- // Subscriptions for version tracking are OK.
- AllowSubscriptionsFrom<ParentOrSiblings>,
-);
+pub type Barrier = DenyThenTry<
+ DenyReserveTransferToRelayChain,
+ (
+ TakeWeightCredit,
+ AllowTopLevelPaidExecutionFrom<Everything>,
+ // Parent and its exec plurality get free execution
+ AllowUnpaidExecutionFrom<ParentOrParentsExecutivePlurality>,
+ // Expected responses are OK.
+ AllowKnownQueryResponses<PolkadotXcm>,
+ // Subscriptions for version tracking are OK.
+ AllowSubscriptionsFrom<ParentOrSiblings>,
+ ),
+>;
pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
diff --git a/cumulus/polkadot-parachains/statemint/src/xcm_config.rs b/cumulus/polkadot-parachains/statemint/src/xcm_config.rs
index bb4d16b4746..f7f72065937 100644
--- a/cumulus/polkadot-parachains/statemint/src/xcm_config.rs
+++ b/cumulus/polkadot-parachains/statemint/src/xcm_config.rs
@@ -23,7 +23,10 @@ use frame_support::{
weights::Weight,
};
use pallet_xcm::XcmPassthrough;
-use parachains_common::impls::ToStakingPot;
+use parachains_common::{
+ impls::ToStakingPot,
+ xcm_config::{DenyReserveTransferToRelayChain, DenyThenTry},
+};
use polkadot_parachain::primitives::Sibling;
use xcm::latest::prelude::*;
use xcm_builder::{
@@ -139,16 +142,19 @@ match_types! {
};
}
-pub type Barrier = (
- TakeWeightCredit,
- AllowTopLevelPaidExecutionFrom<Everything>,
- // Parent and its exec plurality get free execution
- AllowUnpaidExecutionFrom<ParentOrParentsExecutivePlurality>,
- // Expected responses are OK.
- AllowKnownQueryResponses<PolkadotXcm>,
- // Subscriptions for version tracking are OK.
- AllowSubscriptionsFrom<ParentOrSiblings>,
-);
+pub type Barrier = DenyThenTry<
+ DenyReserveTransferToRelayChain,
+ (
+ TakeWeightCredit,
+ AllowTopLevelPaidExecutionFrom<Everything>,
+ // Parent and its exec plurality get free execution
+ AllowUnpaidExecutionFrom<ParentOrParentsExecutivePlurality>,
+ // Expected responses are OK.
+ AllowKnownQueryResponses<PolkadotXcm>,
+ // Subscriptions for version tracking are OK.
+ AllowSubscriptionsFrom<ParentOrSiblings>,
+ ),
+>;
pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
diff --git a/cumulus/polkadot-parachains/westmint/src/xcm_config.rs b/cumulus/polkadot-parachains/westmint/src/xcm_config.rs
index d95cbe7cfd0..c0dd1da1fd7 100644
--- a/cumulus/polkadot-parachains/westmint/src/xcm_config.rs
+++ b/cumulus/polkadot-parachains/westmint/src/xcm_config.rs
@@ -23,7 +23,10 @@ use frame_support::{
weights::Weight,
};
use pallet_xcm::XcmPassthrough;
-use parachains_common::impls::ToStakingPot;
+use parachains_common::{
+ impls::ToStakingPot,
+ xcm_config::{DenyReserveTransferToRelayChain, DenyThenTry},
+};
use polkadot_parachain::primitives::Sibling;
use xcm::latest::prelude::*;
use xcm_builder::{
@@ -135,16 +138,19 @@ match_types! {
};
}
-pub type Barrier = (
- TakeWeightCredit,
- AllowTopLevelPaidExecutionFrom<Everything>,
- // Parent and its plurality get free execution
- AllowUnpaidExecutionFrom<ParentOrParentsPlurality>,
- // Expected responses are OK.
- AllowKnownQueryResponses<PolkadotXcm>,
- // Subscriptions for version tracking are OK.
- AllowSubscriptionsFrom<Everything>,
-);
+pub type Barrier = DenyThenTry<
+ DenyReserveTransferToRelayChain,
+ (
+ TakeWeightCredit,
+ AllowTopLevelPaidExecutionFrom<Everything>,
+ // Parent and its plurality get free execution
+ AllowUnpaidExecutionFrom<ParentOrParentsPlurality>,
+ // Expected responses are OK.
+ AllowKnownQueryResponses<PolkadotXcm>,
+ // Subscriptions for version tracking are OK.
+ AllowSubscriptionsFrom<Everything>,
+ ),
+>;
pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
--
GitLab