From fd705a882d6dfeded376e580a7c744ed43c26206 Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Tue, 5 Sep 2023 12:40:03 +0200 Subject: [PATCH] Update review comments from Adrian Update cumulus/parachains/runtimes/bridge-hubs/bridge-hub-polkadot/src/xcm_config.rs Co-authored-by: Adrian Catangiu Update cumulus/parachains/runtimes/bridge-hubs/bridge-hub-polkadot/src/bridge_hub_config.rs Co-authored-by: Adrian Catangiu Update cumulus/parachains/runtimes/bridge-hubs/bridge-hub-polkadot/src/bridge_hub_config.rs Co-authored-by: Adrian Catangiu Update cumulus/parachains/runtimes/bridge-hubs/bridge-hub-kusama/src/xcm_config.rs Co-authored-by: Adrian Catangiu Update cumulus/parachains/runtimes/bridge-hubs/bridge-hub-kusama/src/bridge_hub_config.rs Co-authored-by: Adrian Catangiu Update cumulus/parachains/runtimes/assets/common/src/matching.rs Co-authored-by: Adrian Catangiu Update cumulus/parachains/runtimes/assets/common/src/matching.rs Co-authored-by: Adrian Catangiu Update cumulus/parachains/runtimes/assets/common/src/matching.rs Co-authored-by: Adrian Catangiu Update cumulus/parachains/runtimes/assets/common/src/matching.rs Co-authored-by: Adrian Catangiu Update cumulus/parachains/runtimes/assets/common/src/matching.rs Co-authored-by: Adrian Catangiu Update cumulus/parachains/runtimes/assets/common/src/matching.rs Co-authored-by: Adrian Catangiu Update cumulus/parachains/runtimes/assets/common/src/matching.rs Co-authored-by: Adrian Catangiu Update cumulus/parachains/runtimes/assets/asset-hub-kusama/src/xcm_config.rs Co-authored-by: Adrian Catangiu Update cumulus/parachains/runtimes/assets/asset-hub-kusama/src/xcm_config.rs Co-authored-by: Adrian Catangiu Update cumulus/parachains/runtimes/assets/asset-hub-polkadot/src/xcm_config.rs Co-authored-by: Adrian Catangiu Update cumulus/parachains/runtimes/assets/asset-hub-polkadot/src/xcm_config.rs Co-authored-by: Adrian Catangiu Update cumulus/parachains/runtimes/assets/common/src/matching.rs Co-authored-by: Adrian Catangiu Fmt + one any to all Co-authored-by: Adrian Catangiu --- .../assets/asset-hub-kusama/src/xcm_config.rs | 10 +++--- .../asset-hub-polkadot/src/xcm_config.rs | 10 +++--- .../runtimes/assets/common/src/matching.rs | 34 +++++++++---------- .../src/bridge_hub_config.rs | 2 +- .../bridge-hub-kusama/src/xcm_config.rs | 2 +- .../src/bridge_hub_config.rs | 5 ++- .../bridge-hub-polkadot/src/xcm_config.rs | 2 +- .../bridge-hub-rococo/src/xcm_config.rs | 2 +- 8 files changed, 33 insertions(+), 34 deletions(-) diff --git a/cumulus/parachains/runtimes/assets/asset-hub-kusama/src/xcm_config.rs b/cumulus/parachains/runtimes/assets/asset-hub-kusama/src/xcm_config.rs index 183940956f523..c81b7a632dbfa 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-kusama/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-kusama/src/xcm_config.rs @@ -608,8 +608,7 @@ impl pallet_xcm::Config for Runtime { type XcmTeleportFilter = Everything; // Allow reserve based transfer to everywhere except for bridging, here we strictly check what // assets are allowed. - type XcmReserveTransferFilter = - EverythingBut; + type XcmReserveTransferFilter = EverythingBut; type Weigher = WeightInfoBounds< crate::weights::xcm::AssetHubKusamaXcmWeight, RuntimeCall, @@ -775,11 +774,12 @@ pub mod bridging { >; /// Filter out those assets which are not allowed for bridged reserve based transfer. - /// (asset, location) filter for `pallet_xcm::Config::XcmReserveTransferFilter`. - pub type IsNotAllowedExplicitlyForReserveTransfer = ExcludeOnlyForRemoteDestination< + /// Filter is made of (asset, location) and used by + /// `pallet_xcm::Config::XcmReserveTransferFilter`. + pub type DisallowForReserveTransfer = ExcludeOnlyForRemoteDestination< UniversalLocation, FilteredNetworkExportTable, - IsNotAllowedConcreteAssetBy, + DisallowConcreteAssetUnless, >; impl Contains for ToPolkadotXcmRouter { diff --git a/cumulus/parachains/runtimes/assets/asset-hub-polkadot/src/xcm_config.rs b/cumulus/parachains/runtimes/assets/asset-hub-polkadot/src/xcm_config.rs index 5479eb82d70e8..860cdbfd50a5d 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-polkadot/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-polkadot/src/xcm_config.rs @@ -528,8 +528,7 @@ impl pallet_xcm::Config for Runtime { type XcmTeleportFilter = Everything; // Allow reserve based transfer to everywhere except for bridging, here we strictly check what // assets are allowed. - type XcmReserveTransferFilter = - EverythingBut; + type XcmReserveTransferFilter = EverythingBut; type Weigher = WeightInfoBounds< crate::weights::xcm::AssetHubPolkadotXcmWeight, RuntimeCall, @@ -682,11 +681,12 @@ pub mod bridging { >; /// Filter out those assets which are not allowed for bridged reserve based transfer. - /// (asset, location) filter for `pallet_xcm::Config::XcmReserveTransferFilter`. - pub type IsNotAllowedExplicitlyForReserveTransfer = ExcludeOnlyForRemoteDestination< + /// Filter is made of (asset, location) and used by + /// `pallet_xcm::Config::XcmReserveTransferFilter`. + pub type DisallowForReserveTransfer = ExcludeOnlyForRemoteDestination< UniversalLocation, FilteredNetworkExportTable, - IsNotAllowedConcreteAssetBy, + DisallowConcreteAssetUnless, >; impl Contains for ToKusamaXcmRouter { diff --git a/cumulus/parachains/runtimes/assets/common/src/matching.rs b/cumulus/parachains/runtimes/assets/common/src/matching.rs index 0402e7a560ed2..9a4e203264c91 100644 --- a/cumulus/parachains/runtimes/assets/common/src/matching.rs +++ b/cumulus/parachains/runtimes/assets/common/src/matching.rs @@ -126,10 +126,8 @@ impl< // check asset according to the configured reserve locations for (reserve_location, asset_filter) in Reserves::get() { - if origin.eq(&reserve_location) { - if asset_filter.matches(asset_location) { - return true - } + if origin.eq(&reserve_location) && asset_filter.matches(asset_location) { + return true } } @@ -137,20 +135,22 @@ impl< } } -/// Filter assets that are explicitly not allowed for destination. +/// Disallow all assets the are either not `Concrete`, or not explicitly allowed by +/// `LocationAssetFilters`, iff `dest` matches any location in `LocationAssetFilters`. /// -/// Returns true if asset is not `Concrete` or is explicitly not allowed by `LocationAssetFilters`. -/// Returns false if `dest` does not match any location in `LocationAssetFilters`. -pub struct IsNotAllowedConcreteAssetBy( +/// Returns `false` regardless of `assets`, if `dest` does not match any location in +/// `LocationAssetFilters`. Otherwise, returns `true` if asset is either not `Concrete` or is not +/// explicitly allowed by `LocationAssetFilters`, otherwise returns `false`. +pub struct DisallowConcreteAssetUnless( sp_std::marker::PhantomData, ); impl>> Contains<(MultiLocation, sp_std::vec::Vec)> - for IsNotAllowedConcreteAssetBy + for DisallowConcreteAssetUnless { fn contains((dest, assets): &(MultiLocation, sp_std::vec::Vec)) -> bool { for (allowed_dest, asset_filter) in LocationAssetFilters::get().iter() { - // we care only about explicitly configured destinations + // we only disallow `assets` on explicitly configured destinations if !allowed_dest.eq(dest) { continue } @@ -162,22 +162,21 @@ impl>> _ => return true, }; - // filter have to match for all assets if !asset_filter.matches(asset_location) { - // if asset does not match filter, we found explicitly not allowed asset + // if asset does not match filter, disallow it return true } } } - // by default, everything is allowed + // if we got here, allow it false } } -/// Adapter for `Contains<(MultiLocation, sp_std::vec::Vec)>` which checks -/// if `Exporters` contains exporter for **remote** `MultiLocation` and iff so, then checks -/// `Filter`, anyway return false. +/// Adapter for `Contains<(MultiLocation, sp_std::vec::Vec)>` which returns `true` +/// iff `Exporters` contains exporter for **remote** `MultiLocation` _and_ +///`assets` also pass`Filter`, otherwise returns `false`. /// /// Note: Assumes that `Exporters` do not depend on `XCM program` and works for `Xcm::default()`. pub struct ExcludeOnlyForRemoteDestination( @@ -204,6 +203,7 @@ where if Exporters::exporter_for(&remote_network, &remote_destination, &Xcm::default()) .is_some() { + // destination is remote, and has configured exporter, now check filter Exclude::contains(dest_and_assets) } else { log::trace!( @@ -221,7 +221,7 @@ where "CheckOnlyForRemoteDestination dest: {:?} is not remote to the universal_source: {:?}", dest_and_assets.0, universal_source ); - // + // not a remote destination, do not exclude false }, } diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-kusama/src/bridge_hub_config.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-kusama/src/bridge_hub_config.rs index 7d973c2cebade..1ffe668818a8a 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-kusama/src/bridge_hub_config.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-kusama/src/bridge_hub_config.rs @@ -14,7 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Cumulus. If not, see . -//! Bridge definitions. +//! Kusama BridgeHub definitions. use crate::{ BridgeParachainPolkadotInstance, BridgePolkadotMessages, Runtime, diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-kusama/src/xcm_config.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-kusama/src/xcm_config.rs index 7b8e3fa5aada1..75e7454b727db 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-kusama/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-kusama/src/xcm_config.rs @@ -134,7 +134,7 @@ impl Contains for SafeCallFilter { // Allow to change dedicated storage items (called by governance-like) match call { RuntimeCall::System(frame_system::Call::set_storage { items }) - if items.iter().any(|(k, _)| { + if items.iter().all(|(k, _)| { k.eq(&DeliveryRewardInBalance::key()) | k.eq(&RequiredStakeForStakeAndSlash::key()) }) => diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-polkadot/src/bridge_hub_config.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-polkadot/src/bridge_hub_config.rs index 26a4a2c2175e3..538c056b18af5 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-polkadot/src/bridge_hub_config.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-polkadot/src/bridge_hub_config.rs @@ -14,7 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Cumulus. If not, see . -//! Bridge definitions. +//! Polkadot BridgeHub definitions. use crate::{ BridgeKusamaMessages, BridgeParachainKusamaInstance, Runtime, @@ -168,8 +168,7 @@ pub type BridgeRefundBridgeHubKusamaMessages = RefundBridgedParachainMessages< bp_runtime::generate_static_str_provider!(BridgeRefundBridgeHubKusamaMessages); // TODO: rework once dynamic lanes are supported (https://github.com/paritytech/parity-bridges-common/issues/1760) -// now we support only StatemineToStatemint -/// Lanes setup +// now we support only AHP<>AHK Lanes setup pub const ASSET_HUB_POLKADOT_TO_ASSET_HUB_KUSAMA_LANE_ID: LaneId = LaneId([0, 0, 0, 0]); parameter_types! { pub ActiveOutboundLanesToBridgeHubKusama: &'static [bp_messages::LaneId] = &[ASSET_HUB_POLKADOT_TO_ASSET_HUB_KUSAMA_LANE_ID]; diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-polkadot/src/xcm_config.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-polkadot/src/xcm_config.rs index ab6643995efa8..5154a28a1dcd4 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-polkadot/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-polkadot/src/xcm_config.rs @@ -137,7 +137,7 @@ impl Contains for SafeCallFilter { // Allow to change dedicated storage items (called by governance-like) match call { RuntimeCall::System(frame_system::Call::set_storage { items }) - if items.iter().any(|(k, _)| { + if items.iter().all(|(k, _)| { k.eq(&DeliveryRewardInBalance::key()) | k.eq(&RequiredStakeForStakeAndSlash::key()) }) => diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/xcm_config.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/xcm_config.rs index e3d8645d49e7e..55064af74a8e6 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/xcm_config.rs @@ -154,7 +154,7 @@ impl Contains for SafeCallFilter { // Allow to change dedicated storage items (called by governance-like) match call { RuntimeCall::System(frame_system::Call::set_storage { items }) - if items.iter().any(|(k, _)| { + if items.iter().all(|(k, _)| { k.eq(&DeliveryRewardInBalance::key()) | k.eq(&RequiredStakeForStakeAndSlash::key()) }) =>