Skip to content

Commit

Permalink
Bridges improved tests and nits (#5128)
Browse files Browse the repository at this point in the history
This PR adds `exporter_is_compatible_with_pallet_xcm_bridge_hub_router`,
which ensures that our `pallet_xcm_bridge_hub` and
`pallet_xcm_bridge_hub_router` are compatible when handling
`ExportMessage`. Other changes are just small nits and cosmetics which
makes others stuff easier.

---------

Co-authored-by: Svyatoslav Nikolsky <[email protected]>
  • Loading branch information
bkontur and svyatonik authored Jul 25, 2024
1 parent 240b374 commit de6733b
Show file tree
Hide file tree
Showing 24 changed files with 284 additions and 147 deletions.
3 changes: 1 addition & 2 deletions Cargo.lock

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

21 changes: 15 additions & 6 deletions bridges/bin/runtime-common/src/integrity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,25 @@ use frame_support::{storage::generator::StorageValue, traits::Get, weights::Weig
use frame_system::limits;
use pallet_bridge_messages::WeightInfoExt as _;

// Re-export to avoid include all dependencies everywhere.
#[doc(hidden)]
pub mod __private {
pub use bp_xcm_bridge_hub;
pub use static_assertions;
}

/// Macro that ensures that the runtime configuration and chain primitives crate are sharing
/// the same types (nonce, block number, hash, hasher, account id and header).
#[macro_export]
macro_rules! assert_chain_types(
( runtime: $r:path, this_chain: $this:path ) => {
{
use frame_system::{Config as SystemConfig, pallet_prelude::{BlockNumberFor, HeaderFor}};
use $crate::integrity::__private::static_assertions::assert_type_eq_all;

// if one of asserts fail, then either bridge isn't configured properly (or alternatively - non-standard
// configuration is used), or something has broke existing configuration (meaning that all bridged chains
// and relays will stop functioning)
use frame_system::{Config as SystemConfig, pallet_prelude::{BlockNumberFor, HeaderFor}};
use static_assertions::assert_type_eq_all;

assert_type_eq_all!(<$r as SystemConfig>::Nonce, bp_runtime::NonceOf<$this>);
assert_type_eq_all!(BlockNumberFor<$r>, bp_runtime::BlockNumberOf<$this>);
Expand All @@ -60,14 +68,15 @@ macro_rules! assert_bridge_messages_pallet_types(
bridged_chain: $bridged:path,
) => {
{
// if one of asserts fail, then either bridge isn't configured properly (or alternatively - non-standard
// configuration is used), or something has broke existing configuration (meaning that all bridged chains
// and relays will stop functioning)
use $crate::messages_xcm_extension::XcmAsPlainPayload;
use $crate::integrity::__private::static_assertions::assert_type_eq_all;
use bp_messages::ChainWithMessages;
use bp_runtime::Chain;
use pallet_bridge_messages::Config as MessagesConfig;
use static_assertions::assert_type_eq_all;

// if one of asserts fail, then either bridge isn't configured properly (or alternatively - non-standard
// configuration is used), or something has broke existing configuration (meaning that all bridged chains
// and relays will stop functioning)

assert_type_eq_all!(<$r as MessagesConfig<$i>>::ThisChain, $this);
assert_type_eq_all!(<$r as MessagesConfig<$i>>::BridgedChain, $bridged);
Expand Down
2 changes: 1 addition & 1 deletion bridges/modules/messages/src/proofs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ pub fn verify_messages_proof<T: Config<I>, I: 'static>(
let nonces_range = nonces_start..=nonces_end;

// receiving proofs where end < begin is ok (if proof includes outbound lane state)
let messages_in_the_proof = nonces_range.checked_len().unwrap_or(0);
let messages_in_the_proof = nonces_range.saturating_len();
if messages_in_the_proof != MessageNonce::from(messages_count) {
return Err(VerificationError::MessagesCountMismatch)
}
Expand Down
2 changes: 1 addition & 1 deletion bridges/modules/messages/src/tests/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ impl crate::benchmarking::Config<()> for TestRuntime {
use bp_runtime::RangeInclusiveExt;

let dispatch_weight =
REGULAR_PAYLOAD.declared_weight * params.message_nonces.checked_len().unwrap_or(0);
REGULAR_PAYLOAD.declared_weight * params.message_nonces.saturating_len();
(
*prepare_messages_proof(
params.message_nonces.into_iter().map(|n| message(n, REGULAR_PAYLOAD)).collect(),
Expand Down
5 changes: 5 additions & 0 deletions bridges/modules/xcm-bridge-hub/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ xcm-executor = { workspace = true }
bp-header-chain = { workspace = true, default-features = true }
pallet-balances = { workspace = true, default-features = true }
sp-io = { workspace = true, default-features = true }
bp-runtime = { features = ["test-helpers"], workspace = true }
pallet-xcm-bridge-hub-router = { workspace = true }

[features]
default = ["std"]
Expand All @@ -51,6 +53,7 @@ std = [
"frame-system/std",
"log/std",
"pallet-bridge-messages/std",
"pallet-xcm-bridge-hub-router/std",
"scale-info/std",
"sp-core/std",
"sp-runtime/std",
Expand All @@ -65,6 +68,7 @@ runtime-benchmarks = [
"frame-system/runtime-benchmarks",
"pallet-balances/runtime-benchmarks",
"pallet-bridge-messages/runtime-benchmarks",
"pallet-xcm-bridge-hub-router/runtime-benchmarks",
"sp-runtime/runtime-benchmarks",
"xcm-builder/runtime-benchmarks",
"xcm-executor/runtime-benchmarks",
Expand All @@ -74,5 +78,6 @@ try-runtime = [
"frame-system/try-runtime",
"pallet-balances/try-runtime",
"pallet-bridge-messages/try-runtime",
"pallet-xcm-bridge-hub-router/try-runtime",
"sp-runtime/try-runtime",
]
44 changes: 39 additions & 5 deletions bridges/modules/xcm-bridge-hub/src/exporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,15 +131,16 @@ impl HaulBlob for DummyHaulBlob {
mod tests {
use super::*;
use crate::mock::*;
use bp_runtime::RangeInclusiveExt;
use frame_support::assert_ok;
use xcm_executor::traits::export_xcm;

fn universal_source() -> InteriorLocation {
[GlobalConsensus(RelayNetwork::get()), Parachain(SIBLING_ASSET_HUB_ID)].into()
}

fn universal_destination() -> InteriorLocation {
BridgedDestination::get()
fn bridged_relative_destination() -> InteriorLocation {
BridgedRelativeDestination::get()
}

#[test]
Expand All @@ -149,7 +150,7 @@ mod tests {
BridgedRelayNetwork::get(),
0,
universal_source(),
universal_destination(),
bridged_relative_destination(),
vec![Instruction::ClearOrigin].into(),
));
})
Expand All @@ -163,7 +164,7 @@ mod tests {
BridgedRelayNetwork::get(),
0,
&mut None,
&mut Some(universal_destination()),
&mut Some(bridged_relative_destination()),
&mut Some(Vec::new().into()),
),
Err(SendError::MissingArgument),
Expand Down Expand Up @@ -192,7 +193,7 @@ mod tests {
BridgedRelayNetwork::get(),
0,
&mut Some(universal_source()),
&mut Some(universal_destination()),
&mut Some(bridged_relative_destination()),
&mut Some(Vec::new().into()),
)
.unwrap()
Expand All @@ -203,4 +204,37 @@ mod tests {
);
})
}

#[test]
fn exporter_is_compatible_with_pallet_xcm_bridge_hub_router() {
run_test(|| {
// valid routable destination
let dest = Location::new(2, BridgedUniversalDestination::get());
let expected_lane_id = TEST_LANE_ID;

// check before - no messages
assert_eq!(
pallet_bridge_messages::Pallet::<TestRuntime, ()>::outbound_lane_data(
expected_lane_id
)
.queued_messages()
.saturating_len(),
0
);

// send `ExportMessage(message)` by `pallet_xcm_bridge_hub_router`.
TestExportXcmWithXcmOverBridge::set_origin_for_execute(SiblingLocation::get());
assert_ok!(send_xcm::<XcmOverBridgeRouter>(dest, Xcm::<()>::default()));

// check after - a message ready to be relayed
assert_eq!(
pallet_bridge_messages::Pallet::<TestRuntime, ()>::outbound_lane_data(
expected_lane_id
)
.queued_messages()
.saturating_len(),
1
);
})
}
}
150 changes: 139 additions & 11 deletions bridges/modules/xcm-bridge-hub/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,24 @@ use bp_messages::{
use bp_runtime::{messages::MessageDispatchResult, Chain, ChainId, HashOf};
use bridge_runtime_common::messages_xcm_extension::{SenderAndLane, XcmBlobHauler};
use codec::Encode;
use frame_support::{derive_impl, parameter_types, weights::RuntimeDbWeight};
use frame_support::{
assert_ok, derive_impl, parameter_types,
traits::{Everything, NeverEnsureOrigin},
weights::RuntimeDbWeight,
};
use sp_core::H256;
use sp_runtime::{
testing::Header as SubstrateHeader,
traits::{BlakeTwo256, IdentityLookup},
traits::{BlakeTwo256, ConstU128, ConstU32, IdentityLookup},
AccountId32, BuildStorage, StateVersion,
};
use sp_std::cell::RefCell;
use xcm::prelude::*;
use xcm_builder::{
AllowUnpaidExecutionFrom, FixedWeightBounds, InspectMessageQueues, NetworkExportTable,
NetworkExportTableItem,
};
use xcm_executor::XcmExecutor;

pub type AccountId = AccountId32;
pub type Balance = u64;
Expand All @@ -50,6 +60,7 @@ frame_support::construct_runtime! {
Balances: pallet_balances::{Pallet, Event<T>},
Messages: pallet_bridge_messages::{Pallet, Call, Event<T>},
XcmOverBridge: pallet_xcm_bridge_hub::{Pallet},
XcmOverBridgeRouter: pallet_xcm_bridge_hub_router,
}
}

Expand Down Expand Up @@ -137,15 +148,34 @@ impl pallet_bridge_messages::WeightInfoExt for TestMessagesWeights {

parameter_types! {
pub const RelayNetwork: NetworkId = NetworkId::Kusama;
pub const BridgedRelayNetwork: NetworkId = NetworkId::Polkadot;
pub BridgedRelayNetworkLocation: Location = (Parent, GlobalConsensus(BridgedRelayNetwork::get())).into();
pub const NonBridgedRelayNetwork: NetworkId = NetworkId::Rococo;
pub const BridgeReserve: Balance = 100_000;
pub UniversalLocation: InteriorLocation = [
GlobalConsensus(RelayNetwork::get()),
Parachain(THIS_BRIDGE_HUB_ID),
].into();
pub SiblingLocation: Location = Location::new(1, [Parachain(SIBLING_ASSET_HUB_ID)]);

pub const BridgedRelayNetwork: NetworkId = NetworkId::Polkadot;
pub BridgedRelayNetworkLocation: Location = (Parent, GlobalConsensus(BridgedRelayNetwork::get())).into();
pub BridgedRelativeDestination: InteriorLocation = [Parachain(BRIDGED_ASSET_HUB_ID)].into();
pub BridgedUniversalDestination: InteriorLocation = [GlobalConsensus(BridgedRelayNetwork::get()), Parachain(BRIDGED_ASSET_HUB_ID)].into();
pub const NonBridgedRelayNetwork: NetworkId = NetworkId::Rococo;

pub const BridgeDeposit: Balance = 100_000;
pub const Penalty: Balance = 1_000;

// configuration for pallet_xcm_bridge_hub_router
pub BridgeHubLocation: Location = Here.into();
pub BridgeFeeAsset: AssetId = Location::here().into();
pub BridgeTable: Vec<NetworkExportTableItem>
= vec![
NetworkExportTableItem::new(
BridgedRelayNetwork::get(),
None,
BridgeHubLocation::get(),
None
)
];
pub UnitWeightCost: Weight = Weight::from_parts(10, 10);
}

impl pallet_xcm_bridge_hub::Config for TestRuntime {
Expand All @@ -160,16 +190,114 @@ impl pallet_xcm_bridge_hub::Config for TestRuntime {
type LanesSupport = TestXcmBlobHauler;
}

impl pallet_xcm_bridge_hub_router::Config<()> for TestRuntime {
type WeightInfo = ();

type UniversalLocation = UniversalLocation;
type BridgedNetworkId = BridgedRelayNetwork;
type Bridges = NetworkExportTable<BridgeTable>;
type DestinationVersion = AlwaysLatest;

type BridgeHubOrigin = NeverEnsureOrigin<AccountId>;
type ToBridgeHubSender = TestExportXcmWithXcmOverBridge;

type ByteFee = ConstU128<0>;
type FeeAsset = BridgeFeeAsset;

type WithBridgeHubChannel = ();
}

pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = RuntimeCall;
type XcmSender = ();
type AssetTransactor = ();
type OriginConverter = ();
type IsReserve = ();
type IsTeleporter = ();
type UniversalLocation = UniversalLocation;
type Barrier = AllowUnpaidExecutionFrom<Everything>;
type Weigher = FixedWeightBounds<UnitWeightCost, RuntimeCall, ConstU32<100>>;
type Trader = ();
type ResponseHandler = ();
type AssetTrap = ();
type AssetClaims = ();
type SubscriptionService = ();
type PalletInstancesInfo = ();
type MaxAssetsIntoHolding = ();
type AssetLocker = ();
type AssetExchanger = ();
type FeeManager = ();
// We just set `MessageExporter` as our `pallet_xcm_bridge_hub` instance.
type MessageExporter = (XcmOverBridge,);
type UniversalAliases = ();
type CallDispatcher = RuntimeCall;
type SafeCallFilter = Everything;
type Aliasers = ();
type TransactionalProcessor = ();
type HrmpNewChannelOpenRequestHandler = ();
type HrmpChannelAcceptedHandler = ();
type HrmpChannelClosingHandler = ();
type XcmRecorder = ();
}

thread_local! {
pub static EXECUTE_XCM_ORIGIN: RefCell<Option<Location>> = RefCell::new(None);
}

/// The `SendXcm` implementation directly executes XCM using `XcmExecutor`.
///
/// We ensure that the `ExportMessage` produced by `pallet_xcm_bridge_hub_router` is compatible with
/// the `ExportXcm` implementation of `pallet_xcm_bridge_hub`.
///
/// Note: The crucial part is that `ExportMessage` is processed by `XcmExecutor`, which calls the
/// `ExportXcm` implementation of `pallet_xcm_bridge_hub` as `MessageExporter`.
pub struct TestExportXcmWithXcmOverBridge;
impl SendXcm for TestExportXcmWithXcmOverBridge {
type Ticket = Xcm<()>;

fn validate(
_: &mut Option<Location>,
message: &mut Option<Xcm<()>>,
) -> SendResult<Self::Ticket> {
Ok((message.take().unwrap(), Assets::new()))
}

fn deliver(ticket: Self::Ticket) -> Result<XcmHash, SendError> {
let xcm: Xcm<RuntimeCall> = ticket.into();

let origin = EXECUTE_XCM_ORIGIN.with(|o| o.borrow().clone().unwrap());
let mut hash = xcm.using_encoded(sp_io::hashing::blake2_256);
let outcome = XcmExecutor::<XcmConfig>::prepare_and_execute(
origin,
xcm,
&mut hash,
Weight::MAX,
Weight::zero(),
);
assert_ok!(outcome.ensure_complete());

Ok(hash)
}
}
impl InspectMessageQueues for TestExportXcmWithXcmOverBridge {
fn get_messages() -> Vec<(VersionedLocation, Vec<VersionedXcm<()>>)> {
todo!()
}
}
impl TestExportXcmWithXcmOverBridge {
pub fn set_origin_for_execute(origin: Location) {
EXECUTE_XCM_ORIGIN.with(|o| *o.borrow_mut() = Some(origin));
}
}

parameter_types! {
pub TestSenderAndLane: SenderAndLane = SenderAndLane {
location: Location::new(1, [Parachain(SIBLING_ASSET_HUB_ID)]),
location: SiblingLocation::get(),
lane: TEST_LANE_ID,
};
pub BridgedDestination: InteriorLocation = [
Parachain(BRIDGED_ASSET_HUB_ID)
].into();
pub TestLanes: sp_std::vec::Vec<(SenderAndLane, (NetworkId, InteriorLocation))> = sp_std::vec![
(TestSenderAndLane::get(), (BridgedRelayNetwork::get(), BridgedDestination::get()))
(TestSenderAndLane::get(), (BridgedRelayNetwork::get(), BridgedRelativeDestination::get()))
];
}

Expand Down
Loading

0 comments on commit de6733b

Please sign in to comment.