Skip to content

Commit bd94e75

Browse files
bkonturacatangiu
bkontur
and
acatangiu
committed
Update review comments from Adrian
Update cumulus/parachains/runtimes/bridge-hubs/bridge-hub-polkadot/src/xcm_config.rs Co-authored-by: Adrian Catangiu <[email protected]> Update cumulus/parachains/runtimes/bridge-hubs/bridge-hub-polkadot/src/bridge_hub_config.rs Co-authored-by: Adrian Catangiu <[email protected]> Update cumulus/parachains/runtimes/bridge-hubs/bridge-hub-polkadot/src/bridge_hub_config.rs Co-authored-by: Adrian Catangiu <[email protected]> Update cumulus/parachains/runtimes/bridge-hubs/bridge-hub-kusama/src/xcm_config.rs Co-authored-by: Adrian Catangiu <[email protected]> Update cumulus/parachains/runtimes/bridge-hubs/bridge-hub-kusama/src/bridge_hub_config.rs Co-authored-by: Adrian Catangiu <[email protected]> Update cumulus/parachains/runtimes/assets/common/src/matching.rs Co-authored-by: Adrian Catangiu <[email protected]> Update cumulus/parachains/runtimes/assets/common/src/matching.rs Co-authored-by: Adrian Catangiu <[email protected]> Update cumulus/parachains/runtimes/assets/common/src/matching.rs Co-authored-by: Adrian Catangiu <[email protected]> Update cumulus/parachains/runtimes/assets/common/src/matching.rs Co-authored-by: Adrian Catangiu <[email protected]> Update cumulus/parachains/runtimes/assets/common/src/matching.rs Co-authored-by: Adrian Catangiu <[email protected]> Update cumulus/parachains/runtimes/assets/common/src/matching.rs Co-authored-by: Adrian Catangiu <[email protected]> Update cumulus/parachains/runtimes/assets/common/src/matching.rs Co-authored-by: Adrian Catangiu <[email protected]> Update cumulus/parachains/runtimes/assets/asset-hub-kusama/src/xcm_config.rs Co-authored-by: Adrian Catangiu <[email protected]> Update cumulus/parachains/runtimes/assets/asset-hub-kusama/src/xcm_config.rs Co-authored-by: Adrian Catangiu <[email protected]> Update cumulus/parachains/runtimes/assets/asset-hub-polkadot/src/xcm_config.rs Co-authored-by: Adrian Catangiu <[email protected]> Update cumulus/parachains/runtimes/assets/asset-hub-polkadot/src/xcm_config.rs Co-authored-by: Adrian Catangiu <[email protected]> Update cumulus/parachains/runtimes/assets/common/src/matching.rs Co-authored-by: Adrian Catangiu <[email protected]> Fmt + one any to all Co-authored-by: Adrian Catangiu <[email protected]>
1 parent 21037dc commit bd94e75

File tree

9 files changed

+39
-41
lines changed

9 files changed

+39
-41
lines changed

‎cumulus/parachains/runtimes/assets/asset-hub-kusama/src/xcm_config.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -608,8 +608,7 @@ impl pallet_xcm::Config for Runtime {
608608
type XcmTeleportFilter = Everything;
609609
// Allow reserve based transfer to everywhere except for bridging, here we strictly check what
610610
// assets are allowed.
611-
type XcmReserveTransferFilter =
612-
EverythingBut<bridging::IsNotAllowedExplicitlyForReserveTransfer>;
611+
type XcmReserveTransferFilter = EverythingBut<bridging::DisallowForReserveTransfer>;
613612
type Weigher = WeightInfoBounds<
614613
crate::weights::xcm::AssetHubKusamaXcmWeight<RuntimeCall>,
615614
RuntimeCall,
@@ -775,11 +774,12 @@ pub mod bridging {
775774
>;
776775

777776
/// Filter out those assets which are not allowed for bridged reserve based transfer.
778-
/// (asset, location) filter for `pallet_xcm::Config::XcmReserveTransferFilter`.
779-
pub type IsNotAllowedExplicitlyForReserveTransfer = ExcludeOnlyForRemoteDestination<
777+
/// Filter is made of (asset, location) and used by
778+
/// `pallet_xcm::Config::XcmReserveTransferFilter`.
779+
pub type DisallowForReserveTransfer = ExcludeOnlyForRemoteDestination<
780780
UniversalLocation,
781781
FilteredNetworkExportTable,
782-
IsNotAllowedConcreteAssetBy<AllowedReserveTransferAssetsLocations>,
782+
DisallowConcreteAssetUnless<AllowedReserveTransferAssetsLocations>,
783783
>;
784784

785785
impl Contains<RuntimeCall> for ToPolkadotXcmRouter {

‎cumulus/parachains/runtimes/assets/asset-hub-polkadot/src/xcm_config.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -528,8 +528,7 @@ impl pallet_xcm::Config for Runtime {
528528
type XcmTeleportFilter = Everything;
529529
// Allow reserve based transfer to everywhere except for bridging, here we strictly check what
530530
// assets are allowed.
531-
type XcmReserveTransferFilter =
532-
EverythingBut<bridging::IsNotAllowedExplicitlyForReserveTransfer>;
531+
type XcmReserveTransferFilter = EverythingBut<bridging::DisallowForReserveTransfer>;
533532
type Weigher = WeightInfoBounds<
534533
crate::weights::xcm::AssetHubPolkadotXcmWeight<RuntimeCall>,
535534
RuntimeCall,
@@ -682,11 +681,12 @@ pub mod bridging {
682681
>;
683682

684683
/// Filter out those assets which are not allowed for bridged reserve based transfer.
685-
/// (asset, location) filter for `pallet_xcm::Config::XcmReserveTransferFilter`.
686-
pub type IsNotAllowedExplicitlyForReserveTransfer = ExcludeOnlyForRemoteDestination<
684+
/// Filter is made of (asset, location) and used by
685+
/// `pallet_xcm::Config::XcmReserveTransferFilter`.
686+
pub type DisallowForReserveTransfer = ExcludeOnlyForRemoteDestination<
687687
UniversalLocation,
688688
FilteredNetworkExportTable,
689-
IsNotAllowedConcreteAssetBy<AllowedReserveTransferAssetsLocations>,
689+
DisallowConcreteAssetUnless<AllowedReserveTransferAssetsLocations>,
690690
>;
691691

692692
impl Contains<RuntimeCall> for ToKusamaXcmRouter {

‎cumulus/parachains/runtimes/assets/common/src/matching.rs

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -126,31 +126,31 @@ impl<
126126

127127
// check asset according to the configured reserve locations
128128
for (reserve_location, asset_filter) in Reserves::get() {
129-
if origin.eq(&reserve_location) {
130-
if asset_filter.matches(asset_location) {
131-
return true
132-
}
129+
if origin.eq(&reserve_location) && asset_filter.matches(asset_location) {
130+
return true
133131
}
134132
}
135133

136134
false
137135
}
138136
}
139137

140-
/// Filter assets that are explicitly not allowed for destination.
138+
/// Disallow all assets the are either not `Concrete`, or not explicitly allowed by
139+
/// `LocationAssetFilters`, iff `dest` matches any location in `LocationAssetFilters`.
141140
///
142-
/// Returns true if asset is not `Concrete` or is explicitly not allowed by `LocationAssetFilters`.
143-
/// Returns false if `dest` does not match any location in `LocationAssetFilters`.
144-
pub struct IsNotAllowedConcreteAssetBy<LocationAssetFilters>(
141+
/// Returns `false` regardless of `assets`, if `dest` does not match any location in
142+
/// `LocationAssetFilters`. Otherwise, returns `true` if asset is either not `Concrete` or is not
143+
/// explicitly allowed by `LocationAssetFilters`, otherwise returns `false`.
144+
pub struct DisallowConcreteAssetUnless<LocationAssetFilters>(
145145
sp_std::marker::PhantomData<LocationAssetFilters>,
146146
);
147147
impl<LocationAssetFilters: Get<sp_std::vec::Vec<FilteredLocation>>>
148148
Contains<(MultiLocation, sp_std::vec::Vec<MultiAsset>)>
149-
for IsNotAllowedConcreteAssetBy<LocationAssetFilters>
149+
for DisallowConcreteAssetUnless<LocationAssetFilters>
150150
{
151151
fn contains((dest, assets): &(MultiLocation, sp_std::vec::Vec<MultiAsset>)) -> bool {
152152
for (allowed_dest, asset_filter) in LocationAssetFilters::get().iter() {
153-
// we care only about explicitly configured destinations
153+
// we only disallow `assets` on explicitly configured destinations
154154
if !allowed_dest.eq(dest) {
155155
continue
156156
}
@@ -162,22 +162,21 @@ impl<LocationAssetFilters: Get<sp_std::vec::Vec<FilteredLocation>>>
162162
_ => return true,
163163
};
164164

165-
// filter have to match for all assets
166165
if !asset_filter.matches(asset_location) {
167-
// if asset does not match filter, we found explicitly not allowed asset
166+
// if asset does not match filter, disallow it
168167
return true
169168
}
170169
}
171170
}
172171

173-
// by default, everything is allowed
172+
// if we got here, allow it
174173
false
175174
}
176175
}
177176

178-
/// Adapter for `Contains<(MultiLocation, sp_std::vec::Vec<MultiAsset>)>` which checks
179-
/// if `Exporters` contains exporter for **remote** `MultiLocation` and iff so, then checks
180-
/// `Filter`, anyway return false.
177+
/// Adapter for `Contains<(MultiLocation, sp_std::vec::Vec<MultiAsset>)>` which returns `true`
178+
/// iff `Exporters` contains exporter for **remote** `MultiLocation` _and_
179+
///`assets` also pass`Filter`, otherwise returns `false`.
181180
///
182181
/// Note: Assumes that `Exporters` do not depend on `XCM program` and works for `Xcm::default()`.
183182
pub struct ExcludeOnlyForRemoteDestination<UniversalLocation, Exporters, Exclude>(
@@ -204,6 +203,7 @@ where
204203
if Exporters::exporter_for(&remote_network, &remote_destination, &Xcm::default())
205204
.is_some()
206205
{
206+
// destination is remote, and has configured exporter, now check filter
207207
Exclude::contains(dest_and_assets)
208208
} else {
209209
log::trace!(
@@ -221,7 +221,7 @@ where
221221
"CheckOnlyForRemoteDestination dest: {:?} is not remote to the universal_source: {:?}",
222222
dest_and_assets.0, universal_source
223223
);
224-
//
224+
// not a remote destination, do not exclude
225225
false
226226
},
227227
}

‎cumulus/parachains/runtimes/bridge-hubs/bridge-hub-kusama/src/bridge_hub_config.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
// You should have received a copy of the GNU General Public License
1515
// along with Cumulus. If not, see <http://www.gnu.org/licenses/>.
1616

17-
//! Bridge definitions.
17+
//! Kusama BridgeHub definitions.
1818
1919
use crate::{
2020
BridgeParachainPolkadotInstance, BridgePolkadotMessages, Runtime,
@@ -179,7 +179,7 @@ parameter_types! {
179179
#[cfg(test)]
180180
mod tests {
181181
use super::*;
182-
use crate::{constants, BridgeGrandpaPolkadotInstance};
182+
use crate::BridgeGrandpaPolkadotInstance;
183183
use bridge_runtime_common::{
184184
assert_complete_bridge_types,
185185
integrity::{
@@ -188,7 +188,7 @@ mod tests {
188188
AssertCompleteBridgeConstants,
189189
},
190190
};
191-
use parachains_common::Balance;
191+
use parachains_common::{kusama, Balance};
192192

193193
/// Every additional message in the message delivery transaction boosts its priority.
194194
/// So the priority of transaction with `N+1` messages is larger than priority of
@@ -199,7 +199,7 @@ mod tests {
199199
///
200200
/// We want this tip to be large enough (delivery transactions with more messages = less
201201
/// operational costs and a faster bridge), so this value should be significant.
202-
const FEE_BOOST_PER_MESSAGE: Balance = constants::currency::UNITS;
202+
const FEE_BOOST_PER_MESSAGE: Balance = kusama::currency::UNITS;
203203

204204
#[test]
205205
fn ensure_lane_weights_are_correct() {

‎cumulus/parachains/runtimes/bridge-hubs/bridge-hub-kusama/src/xcm_config.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ impl Contains<RuntimeCall> for SafeCallFilter {
134134
// Allow to change dedicated storage items (called by governance-like)
135135
match call {
136136
RuntimeCall::System(frame_system::Call::set_storage { items })
137-
if items.iter().any(|(k, _)| {
137+
if items.iter().all(|(k, _)| {
138138
k.eq(&DeliveryRewardInBalance::key()) |
139139
k.eq(&RequiredStakeForStakeAndSlash::key())
140140
}) =>

‎cumulus/parachains/runtimes/bridge-hubs/bridge-hub-polkadot/src/bridge_hub_config.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
// You should have received a copy of the GNU General Public License
1515
// along with Cumulus. If not, see <http://www.gnu.org/licenses/>.
1616

17-
//! Bridge definitions.
17+
//! Polkadot BridgeHub definitions.
1818
1919
use crate::{
2020
BridgeKusamaMessages, BridgeParachainKusamaInstance, Runtime,
@@ -168,8 +168,7 @@ pub type BridgeRefundBridgeHubKusamaMessages = RefundBridgedParachainMessages<
168168
bp_runtime::generate_static_str_provider!(BridgeRefundBridgeHubKusamaMessages);
169169

170170
// TODO: rework once dynamic lanes are supported (https://github.com/paritytech/parity-bridges-common/issues/1760)
171-
// now we support only StatemineToStatemint
172-
/// Lanes setup
171+
// now we support only AHP<>AHK Lanes setup
173172
pub const ASSET_HUB_POLKADOT_TO_ASSET_HUB_KUSAMA_LANE_ID: LaneId = LaneId([0, 0, 0, 0]);
174173
parameter_types! {
175174
pub ActiveOutboundLanesToBridgeHubKusama: &'static [bp_messages::LaneId] = &[ASSET_HUB_POLKADOT_TO_ASSET_HUB_KUSAMA_LANE_ID];
@@ -179,7 +178,7 @@ parameter_types! {
179178
#[cfg(test)]
180179
mod tests {
181180
use super::*;
182-
use crate::{constants, BridgeGrandpaKusamaInstance};
181+
use crate::BridgeGrandpaKusamaInstance;
183182
use bridge_runtime_common::{
184183
assert_complete_bridge_types,
185184
integrity::{
@@ -188,7 +187,7 @@ mod tests {
188187
AssertCompleteBridgeConstants,
189188
},
190189
};
191-
use parachains_common::Balance;
190+
use parachains_common::{polkadot, Balance};
192191

193192
/// Every additional message in the message delivery transaction boosts its priority.
194193
/// So the priority of transaction with `N+1` messages is larger than priority of
@@ -199,7 +198,7 @@ mod tests {
199198
///
200199
/// We want this tip to be large enough (delivery transactions with more messages = less
201200
/// operational costs and a faster bridge), so this value should be significant.
202-
const FEE_BOOST_PER_MESSAGE: Balance = 5 * constants::currency::UNITS;
201+
const FEE_BOOST_PER_MESSAGE: Balance = 5 * polkadot::currency::UNITS;
203202

204203
#[test]
205204
fn ensure_lane_weights_are_correct() {

‎cumulus/parachains/runtimes/bridge-hubs/bridge-hub-polkadot/src/xcm_config.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ impl Contains<RuntimeCall> for SafeCallFilter {
137137
// Allow to change dedicated storage items (called by governance-like)
138138
match call {
139139
RuntimeCall::System(frame_system::Call::set_storage { items })
140-
if items.iter().any(|(k, _)| {
140+
if items.iter().all(|(k, _)| {
141141
k.eq(&DeliveryRewardInBalance::key()) |
142142
k.eq(&RequiredStakeForStakeAndSlash::key())
143143
}) =>

‎cumulus/parachains/runtimes/bridge-hubs/bridge-hub-polkadot/tests/tests.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
use bp_polkadot_core::Signature;
1818
pub use bridge_hub_polkadot_runtime::{
1919
bridge_hub_config,
20-
constants::fee::WeightToFee,
2120
xcm_config::{DotRelayLocation, RelayNetwork, XcmConfig},
2221
AllPalletsWithoutSystem, Balances, BridgeGrandpaKusamaInstance,
2322
BridgeRejectObsoleteHeadersAndMessages, ExistentialDeposit, ParachainSystem, PolkadotXcm,

‎cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/xcm_config.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ impl Contains<RuntimeCall> for SafeCallFilter {
154154
// Allow to change dedicated storage items (called by governance-like)
155155
match call {
156156
RuntimeCall::System(frame_system::Call::set_storage { items })
157-
if items.iter().any(|(k, _)| {
157+
if items.iter().all(|(k, _)| {
158158
k.eq(&DeliveryRewardInBalance::key()) |
159159
k.eq(&RequiredStakeForStakeAndSlash::key())
160160
}) =>

0 commit comments

Comments
 (0)