Skip to content

Commit

Permalink
Bridges testing improvements (paritytech#6536)
Browse files Browse the repository at this point in the history
This PR includes:  
- Refactored integrity tests to support standalone deployment of
`pallet-bridge-messages`.
- Refactored the `open_and_close_bridge_works` test case to support
multiple scenarios, such as:
  1. A local chain opening a bridge.  
  2. Sibling parachains opening a bridge.  
  3. The relay chain opening a bridge.  
- Previously, we added instance support for `pallet-bridge-relayer` but
overlooked updating the `DeliveryConfirmationPaymentsAdapter`.

---------

Co-authored-by: GitHub Action <[email protected]>
  • Loading branch information
2 people authored and Krayt78 committed Dec 18, 2024
1 parent cb58647 commit d59a224
Show file tree
Hide file tree
Showing 14 changed files with 251 additions and 127 deletions.
95 changes: 73 additions & 22 deletions bridges/bin/runtime-common/src/integrity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,11 @@ macro_rules! assert_bridge_messages_pallet_types(

/// Macro that combines four other macro calls - `assert_chain_types`, `assert_bridge_types`,
/// and `assert_bridge_messages_pallet_types`. It may be used
/// at the chain that is implementing complete standard messages bridge (i.e. with bridge GRANDPA
/// and messages pallets deployed).
/// at the chain that is implementing standard messages bridge with messages pallets deployed.
#[macro_export]
macro_rules! assert_complete_bridge_types(
(
runtime: $r:path,
with_bridged_chain_grandpa_instance: $gi:path,
with_bridged_chain_messages_instance: $mi:path,
this_chain: $this:path,
bridged_chain: $bridged:path,
Expand Down Expand Up @@ -186,34 +184,55 @@ where
);
}

/// Parameters for asserting bridge pallet names.
/// Parameters for asserting bridge GRANDPA pallet names.
#[derive(Debug)]
pub struct AssertBridgePalletNames<'a> {
struct AssertBridgeGrandpaPalletNames<'a> {
/// Name of the GRANDPA pallet, deployed at this chain and used to bridge with the bridged
/// chain.
pub with_bridged_chain_grandpa_pallet_name: &'a str,
/// Name of the messages pallet, deployed at this chain and used to bridge with the bridged
/// chain.
pub with_bridged_chain_messages_pallet_name: &'a str,
}

/// Tests that bridge pallet names used in `construct_runtime!()` macro call are matching constants
/// from chain primitives crates.
fn assert_bridge_pallet_names<R, GI, MI>(params: AssertBridgePalletNames)
fn assert_bridge_grandpa_pallet_names<R, GI>(params: AssertBridgeGrandpaPalletNames)
where
R: pallet_bridge_grandpa::Config<GI> + pallet_bridge_messages::Config<MI>,
R: pallet_bridge_grandpa::Config<GI>,
GI: 'static,
MI: 'static,
{
// check that the bridge GRANDPA pallet has required name
assert_eq!(
pallet_bridge_grandpa::PalletOwner::<R, GI>::storage_value_final_key().to_vec(),
pallet_bridge_grandpa::PalletOwner::<R, GI>::storage_value_final_key().to_vec(),
bp_runtime::storage_value_key(
params.with_bridged_chain_grandpa_pallet_name,
"PalletOwner",
)
.0,
);
assert_eq!(
pallet_bridge_grandpa::PalletOperatingMode::<R, GI>::storage_value_final_key().to_vec(),
bp_runtime::storage_value_key(
params.with_bridged_chain_grandpa_pallet_name,
"PalletOwner",
).0,
"PalletOperatingMode",
)
.0,
);
}

/// Parameters for asserting bridge messages pallet names.
#[derive(Debug)]
struct AssertBridgeMessagesPalletNames<'a> {
/// Name of the messages pallet, deployed at this chain and used to bridge with the bridged
/// chain.
pub with_bridged_chain_messages_pallet_name: &'a str,
}

/// Tests that bridge pallet names used in `construct_runtime!()` macro call are matching constants
/// from chain primitives crates.
fn assert_bridge_messages_pallet_names<R, MI>(params: AssertBridgeMessagesPalletNames)
where
R: pallet_bridge_messages::Config<MI>,
MI: 'static,
{
// check that the bridge messages pallet has required name
assert_eq!(
pallet_bridge_messages::PalletOwner::<R, MI>::storage_value_final_key().to_vec(),
Expand All @@ -223,6 +242,14 @@ where
)
.0,
);
assert_eq!(
pallet_bridge_messages::PalletOperatingMode::<R, MI>::storage_value_final_key().to_vec(),
bp_runtime::storage_value_key(
params.with_bridged_chain_messages_pallet_name,
"PalletOperatingMode",
)
.0,
);
}

/// Parameters for asserting complete standard messages bridge.
Expand All @@ -246,31 +273,55 @@ pub fn assert_complete_with_relay_chain_bridge_constants<R, GI, MI>(
assert_chain_constants::<R>(params.this_chain_constants);
assert_bridge_grandpa_pallet_constants::<R, GI>();
assert_bridge_messages_pallet_constants::<R, MI>();
assert_bridge_pallet_names::<R, GI, MI>(AssertBridgePalletNames {
assert_bridge_grandpa_pallet_names::<R, GI>(AssertBridgeGrandpaPalletNames {
with_bridged_chain_grandpa_pallet_name:
<R as pallet_bridge_grandpa::Config<GI>>::BridgedChain::WITH_CHAIN_GRANDPA_PALLET_NAME,
});
assert_bridge_messages_pallet_names::<R, MI>(AssertBridgeMessagesPalletNames {
with_bridged_chain_messages_pallet_name:
<R as pallet_bridge_messages::Config<MI>>::BridgedChain::WITH_CHAIN_MESSAGES_PALLET_NAME,
});
}

/// All bridge-related constants tests for the complete standard parachain messages bridge
/// (i.e. with bridge GRANDPA, parachains and messages pallets deployed).
pub fn assert_complete_with_parachain_bridge_constants<R, GI, MI, RelayChain>(
pub fn assert_complete_with_parachain_bridge_constants<R, PI, MI>(
params: AssertCompleteBridgeConstants,
) where
R: frame_system::Config
+ pallet_bridge_grandpa::Config<GI>
+ pallet_bridge_parachains::Config<PI>
+ pallet_bridge_messages::Config<MI>,
GI: 'static,
<R as pallet_bridge_parachains::BoundedBridgeGrandpaConfig<R::BridgesGrandpaPalletInstance>>::BridgedRelayChain: ChainWithGrandpa,
PI: 'static,
MI: 'static,
{
assert_chain_constants::<R>(params.this_chain_constants);
assert_bridge_grandpa_pallet_constants::<R, R::BridgesGrandpaPalletInstance>();
assert_bridge_messages_pallet_constants::<R, MI>();
assert_bridge_grandpa_pallet_names::<R, R::BridgesGrandpaPalletInstance>(
AssertBridgeGrandpaPalletNames {
with_bridged_chain_grandpa_pallet_name:
<<R as pallet_bridge_parachains::BoundedBridgeGrandpaConfig<
R::BridgesGrandpaPalletInstance,
>>::BridgedRelayChain>::WITH_CHAIN_GRANDPA_PALLET_NAME,
},
);
assert_bridge_messages_pallet_names::<R, MI>(AssertBridgeMessagesPalletNames {
with_bridged_chain_messages_pallet_name:
<R as pallet_bridge_messages::Config<MI>>::BridgedChain::WITH_CHAIN_MESSAGES_PALLET_NAME,
});
}

/// All bridge-related constants tests for the standalone messages bridge deployment (only with
/// messages pallets deployed).
pub fn assert_standalone_messages_bridge_constants<R, MI>(params: AssertCompleteBridgeConstants)
where
R: frame_system::Config + pallet_bridge_messages::Config<MI>,
MI: 'static,
RelayChain: ChainWithGrandpa,
{
assert_chain_constants::<R>(params.this_chain_constants);
assert_bridge_grandpa_pallet_constants::<R, GI>();
assert_bridge_messages_pallet_constants::<R, MI>();
assert_bridge_pallet_names::<R, GI, MI>(AssertBridgePalletNames {
with_bridged_chain_grandpa_pallet_name: RelayChain::WITH_CHAIN_GRANDPA_PALLET_NAME,
assert_bridge_messages_pallet_names::<R, MI>(AssertBridgeMessagesPalletNames {
with_bridged_chain_messages_pallet_name:
<R as pallet_bridge_messages::Config<MI>>::BridgedChain::WITH_CHAIN_MESSAGES_PALLET_NAME,
});
Expand Down
1 change: 1 addition & 0 deletions bridges/bin/runtime-common/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ impl pallet_bridge_messages::Config for TestRuntime {
type DeliveryConfirmationPayments = pallet_bridge_relayers::DeliveryConfirmationPaymentsAdapter<
TestRuntime,
(),
(),
ConstU64<100_000>,
>;
type OnMessagesDelivered = ();
Expand Down
5 changes: 3 additions & 2 deletions bridges/modules/relayers/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,17 @@

use bp_relayers::{
ExplicitOrAccountParams, PaymentProcedure, Registration, RelayerRewardsKeyProvider,
RewardsAccountParams, StakeAndSlash,
StakeAndSlash,
};
pub use bp_relayers::{RewardsAccountOwner, RewardsAccountParams};
use bp_runtime::StorageDoubleMapKeyProvider;
use frame_support::fail;
use sp_arithmetic::traits::{AtLeast32BitUnsigned, Zero};
use sp_runtime::{traits::CheckedSub, Saturating};
use sp_std::marker::PhantomData;

pub use pallet::*;
pub use payment_adapter::DeliveryConfirmationPaymentsAdapter;
pub use payment_adapter::{DeliveryConfirmationPaymentsAdapter, PayRewardFromAccount};
pub use stake_adapter::StakeAndSlashNamed;
pub use weights::WeightInfo;
pub use weights_ext::WeightInfoExt;
Expand Down
15 changes: 8 additions & 7 deletions bridges/modules/relayers/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,14 +171,14 @@ pub type TestStakeAndSlash = pallet_bridge_relayers::StakeAndSlashNamed<
frame_support::construct_runtime! {
pub enum TestRuntime
{
System: frame_system::{Pallet, Call, Config<T>, Storage, Event<T>},
System: frame_system,
Utility: pallet_utility,
Balances: pallet_balances::{Pallet, Call, Storage, Config<T>, Event<T>},
TransactionPayment: pallet_transaction_payment::{Pallet, Storage, Event<T>},
BridgeRelayers: pallet_bridge_relayers::{Pallet, Call, Storage, Event<T>},
BridgeGrandpa: pallet_bridge_grandpa::{Pallet, Call, Storage, Event<T>},
BridgeParachains: pallet_bridge_parachains::{Pallet, Call, Storage, Event<T>},
BridgeMessages: pallet_bridge_messages::{Pallet, Call, Storage, Event<T>, Config<T>},
Balances: pallet_balances,
TransactionPayment: pallet_transaction_payment,
BridgeRelayers: pallet_bridge_relayers,
BridgeGrandpa: pallet_bridge_grandpa,
BridgeParachains: pallet_bridge_parachains,
BridgeMessages: pallet_bridge_messages,
}
}

Expand Down Expand Up @@ -267,6 +267,7 @@ impl pallet_bridge_messages::Config for TestRuntime {
type DeliveryConfirmationPayments = pallet_bridge_relayers::DeliveryConfirmationPaymentsAdapter<
TestRuntime,
(),
(),
ConstU64<100_000>,
>;
type OnMessagesDelivered = ();
Expand Down
24 changes: 13 additions & 11 deletions bridges/modules/relayers/src/payment_adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use bp_messages::{
source_chain::{DeliveryConfirmationPayments, RelayersRewards},
MessageNonce,
};
pub use bp_relayers::PayRewardFromAccount;
use bp_relayers::{RewardsAccountOwner, RewardsAccountParams};
use bp_runtime::Chain;
use frame_support::{sp_runtime::SaturatedConversion, traits::Get};
Expand All @@ -31,15 +32,16 @@ use sp_std::{collections::vec_deque::VecDeque, marker::PhantomData, ops::RangeIn

/// Adapter that allows relayers pallet to be used as a delivery+dispatch payment mechanism
/// for the messages pallet.
pub struct DeliveryConfirmationPaymentsAdapter<T, MI, DeliveryReward>(
PhantomData<(T, MI, DeliveryReward)>,
pub struct DeliveryConfirmationPaymentsAdapter<T, MI, RI, DeliveryReward>(
PhantomData<(T, MI, RI, DeliveryReward)>,
);

impl<T, MI, DeliveryReward> DeliveryConfirmationPayments<T::AccountId, LaneIdOf<T, MI>>
for DeliveryConfirmationPaymentsAdapter<T, MI, DeliveryReward>
impl<T, MI, RI, DeliveryReward> DeliveryConfirmationPayments<T::AccountId, LaneIdOf<T, MI>>
for DeliveryConfirmationPaymentsAdapter<T, MI, RI, DeliveryReward>
where
T: Config + pallet_bridge_messages::Config<MI, LaneId = <T as Config>::LaneId>,
T: Config<RI> + pallet_bridge_messages::Config<MI, LaneId = <T as Config<RI>>::LaneId>,
MI: 'static,
RI: 'static,
DeliveryReward: Get<T::Reward>,
{
type Error = &'static str;
Expand All @@ -54,7 +56,7 @@ where
bp_messages::calc_relayers_rewards::<T::AccountId>(messages_relayers, received_range);
let rewarded_relayers = relayers_rewards.len();

register_relayers_rewards::<T>(
register_relayers_rewards::<T, RI>(
confirmation_relayer,
relayers_rewards,
RewardsAccountParams::new(
Expand All @@ -70,7 +72,7 @@ where
}

// Update rewards to given relayers, optionally rewarding confirmation relayer.
fn register_relayers_rewards<T: Config>(
fn register_relayers_rewards<T: Config<I>, I: 'static>(
confirmation_relayer: &T::AccountId,
relayers_rewards: RelayersRewards<T::AccountId>,
lane_id: RewardsAccountParams<T::LaneId>,
Expand All @@ -84,15 +86,15 @@ fn register_relayers_rewards<T: Config>(
let relayer_reward = T::Reward::saturated_from(messages).saturating_mul(delivery_fee);

if relayer != *confirmation_relayer {
Pallet::<T>::register_relayer_reward(lane_id, &relayer, relayer_reward);
Pallet::<T, I>::register_relayer_reward(lane_id, &relayer, relayer_reward);
} else {
confirmation_relayer_reward =
confirmation_relayer_reward.saturating_add(relayer_reward);
}
}

// finally - pay reward to confirmation relayer
Pallet::<T>::register_relayer_reward(
Pallet::<T, I>::register_relayer_reward(
lane_id,
confirmation_relayer,
confirmation_relayer_reward,
Expand All @@ -115,7 +117,7 @@ mod tests {
#[test]
fn confirmation_relayer_is_rewarded_if_it_has_also_delivered_messages() {
run_test(|| {
register_relayers_rewards::<TestRuntime>(
register_relayers_rewards::<TestRuntime, ()>(
&RELAYER_2,
relayers_rewards(),
test_reward_account_param(),
Expand All @@ -136,7 +138,7 @@ mod tests {
#[test]
fn confirmation_relayer_is_not_rewarded_if_it_has_not_delivered_any_messages() {
run_test(|| {
register_relayers_rewards::<TestRuntime>(
register_relayers_rewards::<TestRuntime, ()>(
&RELAYER_3,
relayers_rewards(),
test_reward_account_param(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,6 @@ mod tests {
fn ensure_bridge_integrity() {
assert_complete_bridge_types!(
runtime: Runtime,
with_bridged_chain_grandpa_instance: BridgeGrandpaRococoBulletinInstance,
with_bridged_chain_messages_instance: WithRococoBulletinMessagesInstance,
this_chain: bp_bridge_hub_rococo::BridgeHubRococo,
bridged_chain: bp_polkadot_bulletin::PolkadotBulletin,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ impl pallet_bridge_messages::Config<WithBridgeHubWestendMessagesInstance> for Ru
type DeliveryConfirmationPayments = pallet_bridge_relayers::DeliveryConfirmationPaymentsAdapter<
Runtime,
WithBridgeHubWestendMessagesInstance,
RelayersForLegacyLaneIdsMessagesInstance,
DeliveryRewardInBalance,
>;

Expand Down Expand Up @@ -256,7 +257,6 @@ mod tests {
fn ensure_bridge_integrity() {
assert_complete_bridge_types!(
runtime: Runtime,
with_bridged_chain_grandpa_instance: BridgeGrandpaWestendInstance,
with_bridged_chain_messages_instance: WithBridgeHubWestendMessagesInstance,
this_chain: bp_bridge_hub_rococo::BridgeHubRococo,
bridged_chain: bp_bridge_hub_westend::BridgeHubWestend,
Expand All @@ -266,7 +266,6 @@ mod tests {
Runtime,
BridgeGrandpaWestendInstance,
WithBridgeHubWestendMessagesInstance,
bp_westend::Westend,
>(AssertCompleteBridgeConstants {
this_chain_constants: AssertChainConstants {
block_length: bp_bridge_hub_rococo::BlockLength::get(),
Expand Down
Loading

0 comments on commit d59a224

Please sign in to comment.