diff --git a/bridges/bin/runtime-common/src/integrity.rs b/bridges/bin/runtime-common/src/integrity.rs index 2ff6c4c9165a..535f1a26e5e8 100644 --- a/bridges/bin/runtime-common/src/integrity.rs +++ b/bridges/bin/runtime-common/src/integrity.rs @@ -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, @@ -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(params: AssertBridgePalletNames) +fn assert_bridge_grandpa_pallet_names(params: AssertBridgeGrandpaPalletNames) where - R: pallet_bridge_grandpa::Config + pallet_bridge_messages::Config, + R: pallet_bridge_grandpa::Config, GI: 'static, - MI: 'static, { // check that the bridge GRANDPA pallet has required name assert_eq!( - pallet_bridge_grandpa::PalletOwner::::storage_value_final_key().to_vec(), + pallet_bridge_grandpa::PalletOwner::::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::::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(params: AssertBridgeMessagesPalletNames) +where + R: pallet_bridge_messages::Config, + MI: 'static, +{ // check that the bridge messages pallet has required name assert_eq!( pallet_bridge_messages::PalletOwner::::storage_value_final_key().to_vec(), @@ -223,6 +242,14 @@ where ) .0, ); + assert_eq!( + pallet_bridge_messages::PalletOperatingMode::::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. @@ -246,9 +273,11 @@ pub fn assert_complete_with_relay_chain_bridge_constants( assert_chain_constants::(params.this_chain_constants); assert_bridge_grandpa_pallet_constants::(); assert_bridge_messages_pallet_constants::(); - assert_bridge_pallet_names::(AssertBridgePalletNames { + assert_bridge_grandpa_pallet_names::(AssertBridgeGrandpaPalletNames { with_bridged_chain_grandpa_pallet_name: >::BridgedChain::WITH_CHAIN_GRANDPA_PALLET_NAME, + }); + assert_bridge_messages_pallet_names::(AssertBridgeMessagesPalletNames { with_bridged_chain_messages_pallet_name: >::BridgedChain::WITH_CHAIN_MESSAGES_PALLET_NAME, }); @@ -256,21 +285,43 @@ pub fn assert_complete_with_relay_chain_bridge_constants( /// 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( +pub fn assert_complete_with_parachain_bridge_constants( params: AssertCompleteBridgeConstants, ) where R: frame_system::Config - + pallet_bridge_grandpa::Config + + pallet_bridge_parachains::Config + pallet_bridge_messages::Config, - GI: 'static, + >::BridgedRelayChain: ChainWithGrandpa, + PI: 'static, + MI: 'static, +{ + assert_chain_constants::(params.this_chain_constants); + assert_bridge_grandpa_pallet_constants::(); + assert_bridge_messages_pallet_constants::(); + assert_bridge_grandpa_pallet_names::( + AssertBridgeGrandpaPalletNames { + with_bridged_chain_grandpa_pallet_name: + <>::BridgedRelayChain>::WITH_CHAIN_GRANDPA_PALLET_NAME, + }, + ); + assert_bridge_messages_pallet_names::(AssertBridgeMessagesPalletNames { + with_bridged_chain_messages_pallet_name: + >::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(params: AssertCompleteBridgeConstants) +where + R: frame_system::Config + pallet_bridge_messages::Config, MI: 'static, - RelayChain: ChainWithGrandpa, { assert_chain_constants::(params.this_chain_constants); - assert_bridge_grandpa_pallet_constants::(); assert_bridge_messages_pallet_constants::(); - assert_bridge_pallet_names::(AssertBridgePalletNames { - with_bridged_chain_grandpa_pallet_name: RelayChain::WITH_CHAIN_GRANDPA_PALLET_NAME, + assert_bridge_messages_pallet_names::(AssertBridgeMessagesPalletNames { with_bridged_chain_messages_pallet_name: >::BridgedChain::WITH_CHAIN_MESSAGES_PALLET_NAME, }); diff --git a/bridges/bin/runtime-common/src/mock.rs b/bridges/bin/runtime-common/src/mock.rs index 1d4043fc4b61..7bd6a7d3bffb 100644 --- a/bridges/bin/runtime-common/src/mock.rs +++ b/bridges/bin/runtime-common/src/mock.rs @@ -197,6 +197,7 @@ impl pallet_bridge_messages::Config for TestRuntime { type DeliveryConfirmationPayments = pallet_bridge_relayers::DeliveryConfirmationPaymentsAdapter< TestRuntime, (), + (), ConstU64<100_000>, >; type OnMessagesDelivered = (); diff --git a/bridges/modules/relayers/src/extension/mod.rs b/bridges/modules/relayers/src/extension/mod.rs index 9a248eb8e798..aaa66ee32ff7 100644 --- a/bridges/modules/relayers/src/extension/mod.rs +++ b/bridges/modules/relayers/src/extension/mod.rs @@ -126,7 +126,7 @@ pub struct BridgeRelayersSignedExtension( impl BridgeRelayersSignedExtension where Self: 'static + Send + Sync, - R: RelayersConfig + R: RelayersConfig + BridgeMessagesConfig + TransactionPaymentConfig, C: ExtensionConfig, @@ -250,7 +250,7 @@ where // let's also replace the weight of slashing relayer with the weight of rewarding relayer if call_info.is_receive_messages_proof_call() { post_info_weight = post_info_weight.saturating_sub( - ::WeightInfo::extra_weight_of_successful_receive_messages_proof_call(), + >::WeightInfo::extra_weight_of_successful_receive_messages_proof_call(), ); } @@ -277,7 +277,7 @@ where impl SignedExtension for BridgeRelayersSignedExtension where Self: 'static + Send + Sync, - R: RelayersConfig + R: RelayersConfig + BridgeMessagesConfig + TransactionPaymentConfig, C: ExtensionConfig, @@ -320,8 +320,8 @@ where }; // we only boost priority if relayer has staked required balance - if !RelayersPallet::::is_registration_active(who) { - return Ok(Default::default()) + if !RelayersPallet::::is_registration_active(who) { + return Ok(Default::default()); } // compute priority boost @@ -381,7 +381,11 @@ where match call_result { RelayerAccountAction::None => (), RelayerAccountAction::Reward(relayer, reward_account, reward) => { - RelayersPallet::::register_relayer_reward(reward_account, &relayer, reward); + RelayersPallet::::register_relayer_reward( + reward_account, + &relayer, + reward, + ); log::trace!( target: LOG_TARGET, @@ -393,7 +397,7 @@ where ); }, RelayerAccountAction::Slash(relayer, slash_account) => - RelayersPallet::::slash_and_deregister( + RelayersPallet::::slash_and_deregister( &relayer, ExplicitOrAccountParams::Params(slash_account), ), diff --git a/bridges/modules/relayers/src/lib.rs b/bridges/modules/relayers/src/lib.rs index f06c2e16ac24..d1c71b6d3051 100644 --- a/bridges/modules/relayers/src/lib.rs +++ b/bridges/modules/relayers/src/lib.rs @@ -22,8 +22,9 @@ 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}; @@ -31,7 +32,7 @@ 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; diff --git a/bridges/modules/relayers/src/mock.rs b/bridges/modules/relayers/src/mock.rs index d186e968e648..7dc213249379 100644 --- a/bridges/modules/relayers/src/mock.rs +++ b/bridges/modules/relayers/src/mock.rs @@ -171,14 +171,14 @@ pub type TestStakeAndSlash = pallet_bridge_relayers::StakeAndSlashNamed< frame_support::construct_runtime! { pub enum TestRuntime { - System: frame_system::{Pallet, Call, Config, Storage, Event}, + System: frame_system, Utility: pallet_utility, - Balances: pallet_balances::{Pallet, Call, Storage, Config, Event}, - TransactionPayment: pallet_transaction_payment::{Pallet, Storage, Event}, - BridgeRelayers: pallet_bridge_relayers::{Pallet, Call, Storage, Event}, - BridgeGrandpa: pallet_bridge_grandpa::{Pallet, Call, Storage, Event}, - BridgeParachains: pallet_bridge_parachains::{Pallet, Call, Storage, Event}, - BridgeMessages: pallet_bridge_messages::{Pallet, Call, Storage, Event, Config}, + Balances: pallet_balances, + TransactionPayment: pallet_transaction_payment, + BridgeRelayers: pallet_bridge_relayers, + BridgeGrandpa: pallet_bridge_grandpa, + BridgeParachains: pallet_bridge_parachains, + BridgeMessages: pallet_bridge_messages, } } @@ -267,6 +267,7 @@ impl pallet_bridge_messages::Config for TestRuntime { type DeliveryConfirmationPayments = pallet_bridge_relayers::DeliveryConfirmationPaymentsAdapter< TestRuntime, (), + (), ConstU64<100_000>, >; type OnMessagesDelivered = (); diff --git a/bridges/modules/relayers/src/payment_adapter.rs b/bridges/modules/relayers/src/payment_adapter.rs index 5383cba5ecbd..5af0d8f9dfbf 100644 --- a/bridges/modules/relayers/src/payment_adapter.rs +++ b/bridges/modules/relayers/src/payment_adapter.rs @@ -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}; @@ -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( - PhantomData<(T, MI, DeliveryReward)>, +pub struct DeliveryConfirmationPaymentsAdapter( + PhantomData<(T, MI, RI, DeliveryReward)>, ); -impl DeliveryConfirmationPayments> - for DeliveryConfirmationPaymentsAdapter +impl DeliveryConfirmationPayments> + for DeliveryConfirmationPaymentsAdapter where - T: Config + pallet_bridge_messages::Config::LaneId>, + T: Config + pallet_bridge_messages::Config>::LaneId>, MI: 'static, + RI: 'static, DeliveryReward: Get, { type Error = &'static str; @@ -54,7 +56,7 @@ where bp_messages::calc_relayers_rewards::(messages_relayers, received_range); let rewarded_relayers = relayers_rewards.len(); - register_relayers_rewards::( + register_relayers_rewards::( confirmation_relayer, relayers_rewards, RewardsAccountParams::new( @@ -70,7 +72,7 @@ where } // Update rewards to given relayers, optionally rewarding confirmation relayer. -fn register_relayers_rewards( +fn register_relayers_rewards, I: 'static>( confirmation_relayer: &T::AccountId, relayers_rewards: RelayersRewards, lane_id: RewardsAccountParams, @@ -84,7 +86,7 @@ fn register_relayers_rewards( let relayer_reward = T::Reward::saturated_from(messages).saturating_mul(delivery_fee); if relayer != *confirmation_relayer { - Pallet::::register_relayer_reward(lane_id, &relayer, relayer_reward); + Pallet::::register_relayer_reward(lane_id, &relayer, relayer_reward); } else { confirmation_relayer_reward = confirmation_relayer_reward.saturating_add(relayer_reward); @@ -92,7 +94,7 @@ fn register_relayers_rewards( } // finally - pay reward to confirmation relayer - Pallet::::register_relayer_reward( + Pallet::::register_relayer_reward( lane_id, confirmation_relayer, confirmation_relayer_reward, @@ -115,7 +117,7 @@ mod tests { #[test] fn confirmation_relayer_is_rewarded_if_it_has_also_delivered_messages() { run_test(|| { - register_relayers_rewards::( + register_relayers_rewards::( &RELAYER_2, relayers_rewards(), test_reward_account_param(), @@ -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::( + register_relayers_rewards::( &RELAYER_3, relayers_rewards(), test_reward_account_param(), diff --git a/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-rococo/src/lib.rs b/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-rococo/src/lib.rs index 706102a30676..4b907fc29de8 100644 --- a/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-rococo/src/lib.rs +++ b/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-rococo/src/lib.rs @@ -36,9 +36,9 @@ mod imports { pub use asset_test_utils::xcm_helpers; pub use emulated_integration_tests_common::{ accounts::DUMMY_EMPTY, - get_account_id_from_seed, - test_parachain_is_trusted_teleporter, test_parachain_is_trusted_teleporter_for_relay, - test_relay_is_trusted_teleporter, test_xcm_fee_querying_apis_work_for_asset_hub, + get_account_id_from_seed, test_parachain_is_trusted_teleporter, + test_parachain_is_trusted_teleporter_for_relay, test_relay_is_trusted_teleporter, + test_xcm_fee_querying_apis_work_for_asset_hub, xcm_emulator::{ assert_expected_events, bx, Chain, Parachain as Para, RelayChain as Relay, Test, TestArgs, TestContext, TestExt, diff --git a/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/lib.rs b/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/lib.rs index 2548f6a23de8..d9cb28d84737 100644 --- a/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/lib.rs +++ b/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/lib.rs @@ -33,9 +33,9 @@ mod imports { pub use asset_test_utils::xcm_helpers; pub use emulated_integration_tests_common::{ accounts::DUMMY_EMPTY, - get_account_id_from_seed, - test_parachain_is_trusted_teleporter, test_parachain_is_trusted_teleporter_for_relay, - test_relay_is_trusted_teleporter, test_xcm_fee_querying_apis_work_for_asset_hub, + get_account_id_from_seed, test_parachain_is_trusted_teleporter, + test_parachain_is_trusted_teleporter_for_relay, test_relay_is_trusted_teleporter, + test_xcm_fee_querying_apis_work_for_asset_hub, xcm_emulator::{ assert_expected_events, bx, Chain, Parachain as Para, RelayChain as Relay, Test, TestArgs, TestContext, TestExt, diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/bridge_to_bulletin_config.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/bridge_to_bulletin_config.rs index c971fa59c68d..ea927534e71d 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/bridge_to_bulletin_config.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/bridge_to_bulletin_config.rs @@ -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, diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/bridge_to_westend_config.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/bridge_to_westend_config.rs index 8fe045723107..eeb06d72a452 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/bridge_to_westend_config.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/bridge_to_westend_config.rs @@ -120,6 +120,7 @@ impl pallet_bridge_messages::Config for Ru type DeliveryConfirmationPayments = pallet_bridge_relayers::DeliveryConfirmationPaymentsAdapter< Runtime, WithBridgeHubWestendMessagesInstance, + RelayersForLegacyLaneIdsMessagesInstance, DeliveryRewardInBalance, >; @@ -253,7 +254,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, @@ -263,7 +263,6 @@ mod tests { Runtime, BridgeGrandpaWestendInstance, WithBridgeHubWestendMessagesInstance, - bp_westend::Westend, >(AssertCompleteBridgeConstants { this_chain_constants: AssertChainConstants { block_length: bp_bridge_hub_rococo::BlockLength::get(), diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/tests/tests.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/tests/tests.rs index 44ffd60dde70..29db12ab8b95 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/tests/tests.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/tests/tests.rs @@ -339,11 +339,12 @@ mod bridge_hub_westend_tests { >( SiblingParachainLocation::get(), BridgedUniversalLocation::get(), - |locations, fee| { + false, + |locations, _fee| { bridge_hub_test_utils::open_bridge_with_storage::< Runtime, XcmOverBridgeHubWestendInstance - >(locations, fee, LegacyLaneId([0, 0, 0, 1])) + >(locations, LegacyLaneId([0, 0, 0, 1])) } ).1 }, @@ -403,11 +404,12 @@ mod bridge_hub_westend_tests { >( SiblingParachainLocation::get(), BridgedUniversalLocation::get(), - |locations, fee| { + false, + |locations, _fee| { bridge_hub_test_utils::open_bridge_with_storage::< Runtime, XcmOverBridgeHubWestendInstance, - >(locations, fee, LegacyLaneId([0, 0, 0, 1])) + >(locations, LegacyLaneId([0, 0, 0, 1])) }, ) .1 @@ -437,11 +439,12 @@ mod bridge_hub_westend_tests { >( SiblingParachainLocation::get(), BridgedUniversalLocation::get(), - |locations, fee| { + false, + |locations, _fee| { bridge_hub_test_utils::open_bridge_with_storage::< Runtime, XcmOverBridgeHubWestendInstance, - >(locations, fee, LegacyLaneId([0, 0, 0, 1])) + >(locations, LegacyLaneId([0, 0, 0, 1])) }, ) .1 @@ -603,11 +606,12 @@ mod bridge_hub_bulletin_tests { >( SiblingPeopleParachainLocation::get(), BridgedBulletinLocation::get(), - |locations, fee| { + false, + |locations, _fee| { bridge_hub_test_utils::open_bridge_with_storage::< Runtime, XcmOverPolkadotBulletinInstance - >(locations, fee, HashedLaneId::try_new(1, 2).unwrap()) + >(locations, HashedLaneId::try_new(1, 2).unwrap()) } ).1 }, @@ -666,11 +670,12 @@ mod bridge_hub_bulletin_tests { >( SiblingPeopleParachainLocation::get(), BridgedBulletinLocation::get(), - |locations, fee| { + false, + |locations, _fee| { bridge_hub_test_utils::open_bridge_with_storage::< Runtime, XcmOverPolkadotBulletinInstance, - >(locations, fee, HashedLaneId::try_new(1, 2).unwrap()) + >(locations, HashedLaneId::try_new(1, 2).unwrap()) }, ) .1 @@ -699,11 +704,12 @@ mod bridge_hub_bulletin_tests { >( SiblingPeopleParachainLocation::get(), BridgedBulletinLocation::get(), - |locations, fee| { + false, + |locations, _fee| { bridge_hub_test_utils::open_bridge_with_storage::< Runtime, XcmOverPolkadotBulletinInstance, - >(locations, fee, HashedLaneId::try_new(1, 2).unwrap()) + >(locations, HashedLaneId::try_new(1, 2).unwrap()) }, ) .1 diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/bridge_to_rococo_config.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/bridge_to_rococo_config.rs index e45654bc62bd..0a21dbe52399 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/bridge_to_rococo_config.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/bridge_to_rococo_config.rs @@ -151,6 +151,7 @@ impl pallet_bridge_messages::Config for Run type DeliveryConfirmationPayments = pallet_bridge_relayers::DeliveryConfirmationPaymentsAdapter< Runtime, WithBridgeHubRococoMessagesInstance, + RelayersForLegacyLaneIdsMessagesInstance, DeliveryRewardInBalance, >; @@ -281,7 +282,6 @@ mod tests { fn ensure_bridge_integrity() { assert_complete_bridge_types!( runtime: Runtime, - with_bridged_chain_grandpa_instance: BridgeGrandpaRococoInstance, with_bridged_chain_messages_instance: WithBridgeHubRococoMessagesInstance, this_chain: bp_bridge_hub_westend::BridgeHubWestend, bridged_chain: bp_bridge_hub_rococo::BridgeHubRococo, @@ -291,7 +291,6 @@ mod tests { Runtime, BridgeGrandpaRococoInstance, WithBridgeHubRococoMessagesInstance, - bp_rococo::Rococo, >(AssertCompleteBridgeConstants { this_chain_constants: AssertChainConstants { block_length: bp_bridge_hub_westend::BlockLength::get(), diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/tests/tests.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/tests/tests.rs index e8895a9f433e..d559e20ace7f 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/tests/tests.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/tests/tests.rs @@ -243,10 +243,11 @@ fn handle_export_message_from_system_parachain_add_to_outbound_queue_works() { >( SiblingParachainLocation::get(), BridgedUniversalLocation::get(), - |locations, fee| { + false, + |locations, _fee| { bridge_hub_test_utils::open_bridge_with_storage::< Runtime, XcmOverBridgeHubRococoInstance - >(locations, fee, LegacyLaneId([0, 0, 0, 1])) + >(locations, LegacyLaneId([0, 0, 0, 1])) } ).1 }, @@ -301,12 +302,17 @@ fn relayed_incoming_message_works() { XcmOverBridgeHubRococoInstance, LocationToAccountId, WestendLocation, - >(SiblingParachainLocation::get(), BridgedUniversalLocation::get(), |locations, fee| { - bridge_hub_test_utils::open_bridge_with_storage::< - Runtime, - XcmOverBridgeHubRococoInstance, - >(locations, fee, LegacyLaneId([0, 0, 0, 1])) - }) + >( + SiblingParachainLocation::get(), + BridgedUniversalLocation::get(), + false, + |locations, _fee| { + bridge_hub_test_utils::open_bridge_with_storage::< + Runtime, + XcmOverBridgeHubRococoInstance, + >(locations, LegacyLaneId([0, 0, 0, 1])) + }, + ) .1 }, construct_and_apply_extrinsic, @@ -331,12 +337,17 @@ fn free_relay_extrinsic_works() { XcmOverBridgeHubRococoInstance, LocationToAccountId, WestendLocation, - >(SiblingParachainLocation::get(), BridgedUniversalLocation::get(), |locations, fee| { - bridge_hub_test_utils::open_bridge_with_storage::< - Runtime, - XcmOverBridgeHubRococoInstance, - >(locations, fee, LegacyLaneId([0, 0, 0, 1])) - }) + >( + SiblingParachainLocation::get(), + BridgedUniversalLocation::get(), + false, + |locations, _fee| { + bridge_hub_test_utils::open_bridge_with_storage::< + Runtime, + XcmOverBridgeHubRococoInstance, + >(locations, LegacyLaneId([0, 0, 0, 1])) + }, + ) .1 }, construct_and_apply_extrinsic, diff --git a/cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_cases/helpers.rs b/cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_cases/helpers.rs index aac60bba0b53..03ddc4313b45 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_cases/helpers.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_cases/helpers.rs @@ -29,7 +29,7 @@ use core::marker::PhantomData; use frame_support::{ assert_ok, dispatch::GetDispatchInfo, - traits::{fungible::Mutate, OnFinalize, OnInitialize, PalletInfoAccess}, + traits::{fungible::Mutate, Contains, OnFinalize, OnInitialize, PalletInfoAccess}, }; use frame_system::pallet_prelude::BlockNumberFor; use pallet_bridge_grandpa::{BridgedBlockHash, BridgedHeader}; @@ -395,7 +395,7 @@ pub fn ensure_opened_bridge< XcmOverBridgePalletInstance, LocationToAccountId, TokenLocation> -(source: Location, destination: InteriorLocation, bridge_opener: impl Fn(BridgeLocations, Asset)) -> (BridgeLocations, pallet_xcm_bridge_hub::LaneIdOf) +(source: Location, destination: InteriorLocation, is_paid_xcm_execution: bool, bridge_opener: impl Fn(BridgeLocations, Option)) -> (BridgeLocations, pallet_xcm_bridge_hub::LaneIdOf) where Runtime: BasicParachainRuntime + BridgeXcmOverBridgeConfig, XcmOverBridgePalletInstance: 'static, @@ -416,24 +416,37 @@ TokenLocation: Get{ ) .is_none()); - // required balance: ED + fee + BridgeDeposit - let bridge_deposit = - >::BridgeDeposit::get( - ); - // random high enough value for `BuyExecution` fees - let buy_execution_fee_amount = 5_000_000_000_000_u128; - let buy_execution_fee = (TokenLocation::get(), buy_execution_fee_amount).into(); - let balance_needed = ::ExistentialDeposit::get() + - buy_execution_fee_amount.into() + - bridge_deposit.into(); - // SA of source location needs to have some required balance - let source_account_id = LocationToAccountId::convert_location(&source).expect("valid location"); - let _ = >::mint_into(&source_account_id, balance_needed) - .expect("mint_into passes"); + if !>::AllowWithoutBridgeDeposit::contains(&source) { + // required balance: ED + fee + BridgeDeposit + let bridge_deposit = + >::BridgeDeposit::get( + ); + let balance_needed = ::ExistentialDeposit::get() + bridge_deposit.into(); + + let source_account_id = LocationToAccountId::convert_location(&source).expect("valid location"); + let _ = >::mint_into(&source_account_id, balance_needed) + .expect("mint_into passes"); + }; + + let maybe_paid_execution = if is_paid_xcm_execution { + // random high enough value for `BuyExecution` fees + let buy_execution_fee_amount = 5_000_000_000_000_u128; + let buy_execution_fee = (TokenLocation::get(), buy_execution_fee_amount).into(); + + let balance_needed = ::ExistentialDeposit::get() + + buy_execution_fee_amount.into(); + let source_account_id = + LocationToAccountId::convert_location(&source).expect("valid location"); + let _ = >::mint_into(&source_account_id, balance_needed) + .expect("mint_into passes"); + Some(buy_execution_fee) + } else { + None + }; // call the bridge opener - bridge_opener(*locations.clone(), buy_execution_fee); + bridge_opener(*locations.clone(), maybe_paid_execution); // check opened bridge let bridge = pallet_xcm_bridge_hub::Bridges::::get( @@ -452,8 +465,9 @@ TokenLocation: Get{ /// Utility for opening bridge with dedicated `pallet_xcm_bridge_hub`'s extrinsic. pub fn open_bridge_with_extrinsic( - locations: BridgeLocations, - buy_execution_fee: Asset, + (origin, origin_kind): (Location, OriginKind), + bridge_destination_universal_location: InteriorLocation, + maybe_paid_execution: Option, ) where Runtime: frame_system::Config + pallet_xcm_bridge_hub::Config @@ -469,15 +483,15 @@ pub fn open_bridge_with_extrinsic( XcmOverBridgePalletInstance, >::open_bridge { bridge_destination_universal_location: Box::new( - locations.bridge_destination_universal_location().clone().into(), + bridge_destination_universal_location.clone().into(), ), }); // execute XCM as source origin would do with `Transact -> Origin::Xcm` - assert_ok!(RuntimeHelper::::execute_as_origin_xcm( - locations.bridge_origin_relative_location().clone(), + assert_ok!(RuntimeHelper::::execute_as_origin( + (origin, origin_kind), open_bridge_call, - buy_execution_fee + maybe_paid_execution ) .ensure_complete()); } @@ -486,7 +500,6 @@ pub fn open_bridge_with_extrinsic( /// purposes). pub fn open_bridge_with_storage( locations: BridgeLocations, - _buy_execution_fee: Asset, lane_id: pallet_xcm_bridge_hub::LaneIdOf, ) where Runtime: pallet_xcm_bridge_hub::Config, @@ -503,8 +516,12 @@ pub fn open_bridge_with_storage( } /// Helper function to close the bridge/lane for `source` and `destination`. -pub fn close_bridge(source: Location, destination: InteriorLocation) -where +pub fn close_bridge( + expected_source: Location, + bridge_destination_universal_location: InteriorLocation, + (origin, origin_kind): (Location, OriginKind), + is_paid_xcm_execution: bool +) where Runtime: BasicParachainRuntime + BridgeXcmOverBridgeConfig, XcmOverBridgePalletInstance: 'static, ::RuntimeCall: GetDispatchInfo + From>, @@ -515,8 +532,8 @@ TokenLocation: Get{ // construct expected bridge configuration let locations = pallet_xcm_bridge_hub::Pallet::::bridge_locations( - source.clone().into(), - destination.clone().into(), + expected_source.clone().into(), + bridge_destination_universal_location.clone().into(), ) .expect("valid bridge locations"); assert!(pallet_xcm_bridge_hub::Bridges::::get( @@ -525,35 +542,38 @@ TokenLocation: Get{ .is_some()); // required balance: ED + fee + BridgeDeposit - let bridge_deposit = - >::BridgeDeposit::get( - ); - // random high enough value for `BuyExecution` fees - let buy_execution_fee_amount = 2_500_000_000_000_u128; - let buy_execution_fee = (TokenLocation::get(), buy_execution_fee_amount).into(); - let balance_needed = ::ExistentialDeposit::get() + - buy_execution_fee_amount.into() + - bridge_deposit.into(); - - // SA of source location needs to have some required balance - let source_account_id = LocationToAccountId::convert_location(&source).expect("valid location"); - let _ = >::mint_into(&source_account_id, balance_needed) - .expect("mint_into passes"); + let maybe_paid_execution = if is_paid_xcm_execution { + // random high enough value for `BuyExecution` fees + let buy_execution_fee_amount = 2_500_000_000_000_u128; + let buy_execution_fee = (TokenLocation::get(), buy_execution_fee_amount).into(); + + let balance_needed = ::ExistentialDeposit::get() + + buy_execution_fee_amount.into(); + let source_account_id = + LocationToAccountId::convert_location(&expected_source).expect("valid location"); + let _ = >::mint_into(&source_account_id, balance_needed) + .expect("mint_into passes"); + Some(buy_execution_fee) + } else { + None + }; // close bridge with `Transact` call let close_bridge_call = RuntimeCallOf::::from(BridgeXcmOverBridgeCall::< Runtime, XcmOverBridgePalletInstance, >::close_bridge { - bridge_destination_universal_location: Box::new(destination.into()), + bridge_destination_universal_location: Box::new( + bridge_destination_universal_location.into(), + ), may_prune_messages: 16, }); // execute XCM as source origin would do with `Transact -> Origin::Xcm` - assert_ok!(RuntimeHelper::::execute_as_origin_xcm( - source.clone(), + assert_ok!(RuntimeHelper::::execute_as_origin( + (origin, origin_kind), close_bridge_call, - buy_execution_fee + maybe_paid_execution ) .ensure_complete()); diff --git a/cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_cases/mod.rs b/cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_cases/mod.rs index 663558f5fd5c..84d2e76d14ea 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_cases/mod.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_cases/mod.rs @@ -659,8 +659,10 @@ where pub fn open_and_close_bridge_works( collator_session_key: CollatorSessionKeys, runtime_para_id: u32, - source: Location, + expected_source: Location, destination: InteriorLocation, + origin_with_origin_kind: (Location, OriginKind), + is_paid_xcm_execution: bool, ) where Runtime: BasicParachainRuntime + BridgeXcmOverBridgeConfig, XcmOverBridgePalletInstance: 'static, @@ -674,7 +676,7 @@ pub fn open_and_close_bridge_works(collator_session_key, runtime_para_id, vec![], || { // construct expected bridge configuration let locations = pallet_xcm_bridge_hub::Pallet::::bridge_locations( - source.clone().into(), + expected_source.clone().into(), destination.clone().into(), ).expect("valid bridge locations"); let expected_lane_id = @@ -709,7 +711,7 @@ pub fn open_and_close_bridge_works( - source.clone(), + expected_source.clone(), destination.clone(), - open_bridge_with_extrinsic:: + is_paid_xcm_execution, + |locations, maybe_paid_execution| open_bridge_with_extrinsic::< + Runtime, + XcmOverBridgePalletInstance, + >( + origin_with_origin_kind.clone(), + locations.bridge_destination_universal_location().clone(), + maybe_paid_execution + ) ) .0 .bridge_id(), @@ -732,7 +742,7 @@ pub fn open_and_close_bridge_works(source.clone(), destination); + >(expected_source, destination, origin_with_origin_kind, is_paid_xcm_execution); // check bridge/lane DOES not exist assert_eq!( diff --git a/cumulus/parachains/runtimes/test-utils/src/lib.rs b/cumulus/parachains/runtimes/test-utils/src/lib.rs index 3b38eee244f1..3584edad0b55 100644 --- a/cumulus/parachains/runtimes/test-utils/src/lib.rs +++ b/cumulus/parachains/runtimes/test-utils/src/lib.rs @@ -464,22 +464,30 @@ impl< ) } - pub fn execute_as_origin_xcm( - origin: Location, + pub fn execute_as_origin( + (origin, origin_kind): (Location, OriginKind), call: Call, - buy_execution_fee: Asset, + maybe_buy_execution_fee: Option, ) -> Outcome { + let mut instructions = if let Some(buy_execution_fee) = maybe_buy_execution_fee { + vec![ + WithdrawAsset(buy_execution_fee.clone().into()), + BuyExecution { fees: buy_execution_fee.clone(), weight_limit: Unlimited }, + ] + } else { + vec![UnpaidExecution { check_origin: None, weight_limit: Unlimited }] + }; + // prepare `Transact` xcm - let xcm = Xcm(vec![ - WithdrawAsset(buy_execution_fee.clone().into()), - BuyExecution { fees: buy_execution_fee.clone(), weight_limit: Unlimited }, + instructions.extend(vec![ Transact { - origin_kind: OriginKind::Xcm, + origin_kind, require_weight_at_most: call.get_dispatch_info().weight, call: call.encode().into(), }, ExpectTransactStatus(MaybeErrorCode::Success), ]); + let xcm = Xcm(instructions); // execute xcm as parent origin let mut hash = xcm.using_encoded(sp_io::hashing::blake2_256); diff --git a/polkadot/grafana/README.md b/polkadot/grafana/README.md index e909fdd29a75..0ecb0b70515b 100644 --- a/polkadot/grafana/README.md +++ b/polkadot/grafana/README.md @@ -90,4 +90,4 @@ and issue statement or initiate dispute. - **Assignment delay tranches**. Approval voting is designed such that validators assigned to check a specific candidate are split up into equal delay tranches (0.5 seconds each). All validators checks are ordered by the delay tranche index. Early tranches of validators have the opportunity to check the candidate first before later tranches -that act as as backups in case of no shows. +that act as backups in case of no shows. diff --git a/polkadot/grafana/parachains/status.json b/polkadot/grafana/parachains/status.json index 5942cbdf4479..22250967848d 100644 --- a/polkadot/grafana/parachains/status.json +++ b/polkadot/grafana/parachains/status.json @@ -1405,7 +1405,7 @@ "type": "prometheus", "uid": "$data_source" }, - "description": "Approval voting requires that validators which are assigned to check a specific \ncandidate are split up into delay tranches (0.5s each). Then, all validators checks are ordered by the delay \ntranche index. Early tranches of validators will check the candidate first and later tranches act as as backups in case of no shows.", + "description": "Approval voting requires that validators which are assigned to check a specific \ncandidate are split up into delay tranches (0.5s each). Then, all validators checks are ordered by the delay \ntranche index. Early tranches of validators will check the candidate first and later tranches act as backups in case of no shows.", "gridPos": { "h": 9, "w": 18, diff --git a/polkadot/xcm/pallet-xcm/src/lib.rs b/polkadot/xcm/pallet-xcm/src/lib.rs index 3056ae656e9a..5ccc122b891c 100644 --- a/polkadot/xcm/pallet-xcm/src/lib.rs +++ b/polkadot/xcm/pallet-xcm/src/lib.rs @@ -361,7 +361,10 @@ pub mod pallet { let message: Xcm<()> = (*message).try_into().map_err(|()| Error::::BadVersion)?; let message_id = Self::send_xcm(interior, dest.clone(), message.clone()) - .map_err(Error::::from)?; + .map_err(|error| { + tracing::error!(target: "xcm::pallet_xcm::send", ?error, ?dest, ?message, "XCM send failed with error"); + Error::::from(error) + })?; let e = Event::Sent { origin: origin_location, destination: dest, message, message_id }; Self::deposit_event(e); Ok(message_id) @@ -1805,7 +1808,10 @@ impl Pallet { if let Some(remote_xcm) = remote_xcm { let (ticket, price) = validate_send::(dest.clone(), remote_xcm.clone()) - .map_err(Error::::from)?; + .map_err(|error| { + tracing::error!(target: "xcm::pallet_xcm::execute_xcm_transfer", ?error, ?dest, ?remote_xcm, "XCM validate_send failed with error"); + Error::::from(error) + })?; if origin != Here.into_location() { Self::charge_fees(origin.clone(), price).map_err(|error| { log::error!( @@ -1815,7 +1821,11 @@ impl Pallet { Error::::FeesNotMet })?; } - let message_id = T::XcmRouter::deliver(ticket).map_err(Error::::from)?; + let message_id = T::XcmRouter::deliver(ticket) + .map_err(|error| { + tracing::error!(target: "xcm::pallet_xcm::execute_xcm_transfer", ?error, ?dest, ?remote_xcm, "XCM deliver failed with error"); + Error::::from(error) + })?; let e = Event::Sent { origin, destination: dest, message: remote_xcm, message_id }; Self::deposit_event(e); diff --git a/polkadot/xcm/src/v3/traits.rs b/polkadot/xcm/src/v3/traits.rs index 7fa8824c3568..80520b1962e4 100644 --- a/polkadot/xcm/src/v3/traits.rs +++ b/polkadot/xcm/src/v3/traits.rs @@ -535,13 +535,13 @@ impl SendXcm for Tuple { } /// Convenience function for using a `SendXcm` implementation. Just interprets the `dest` and wraps -/// both in `Some` before passing them as as mutable references into `T::send_xcm`. +/// both in `Some` before passing them as mutable references into `T::send_xcm`. pub fn validate_send(dest: MultiLocation, msg: Xcm<()>) -> SendResult { T::validate(&mut Some(dest), &mut Some(msg)) } /// Convenience function for using a `SendXcm` implementation. Just interprets the `dest` and wraps -/// both in `Some` before passing them as as mutable references into `T::send_xcm`. +/// both in `Some` before passing them as mutable references into `T::send_xcm`. /// /// Returns either `Ok` with the price of the delivery, or `Err` with the reason why the message /// could not be sent. diff --git a/polkadot/xcm/src/v4/traits.rs b/polkadot/xcm/src/v4/traits.rs index 351de92c80ed..c4eec522ed03 100644 --- a/polkadot/xcm/src/v4/traits.rs +++ b/polkadot/xcm/src/v4/traits.rs @@ -289,13 +289,13 @@ impl SendXcm for Tuple { } /// Convenience function for using a `SendXcm` implementation. Just interprets the `dest` and wraps -/// both in `Some` before passing them as as mutable references into `T::send_xcm`. +/// both in `Some` before passing them as mutable references into `T::send_xcm`. pub fn validate_send(dest: Location, msg: Xcm<()>) -> SendResult { T::validate(&mut Some(dest), &mut Some(msg)) } /// Convenience function for using a `SendXcm` implementation. Just interprets the `dest` and wraps -/// both in `Some` before passing them as as mutable references into `T::send_xcm`. +/// both in `Some` before passing them as mutable references into `T::send_xcm`. /// /// Returns either `Ok` with the price of the delivery, or `Err` with the reason why the message /// could not be sent. diff --git a/polkadot/xcm/xcm-builder/src/process_xcm_message.rs b/polkadot/xcm/xcm-builder/src/process_xcm_message.rs index 2e6f8c5fb566..129ce1e584ea 100644 --- a/polkadot/xcm/xcm-builder/src/process_xcm_message.rs +++ b/polkadot/xcm/xcm-builder/src/process_xcm_message.rs @@ -55,7 +55,7 @@ impl< let message = Xcm::::try_from(versioned_message).map_err(|_| { log::trace!( target: LOG_TARGET, - "Failed to convert `VersionedXcm` into `XcmV3`.", + "Failed to convert `VersionedXcm` into `xcm::prelude::Xcm`!", ); ProcessMessageError::Unsupported diff --git a/polkadot/xcm/xcm-builder/src/universal_exports.rs b/polkadot/xcm/xcm-builder/src/universal_exports.rs index 5c754f01ec0a..aae8438c78d2 100644 --- a/polkadot/xcm/xcm-builder/src/universal_exports.rs +++ b/polkadot/xcm/xcm-builder/src/universal_exports.rs @@ -68,20 +68,28 @@ impl> SendXcm fn validate( dest: &mut Option, - xcm: &mut Option>, + msg: &mut Option>, ) -> SendResult { - let d = dest.take().ok_or(MissingArgument)?; + // This `clone` ensures that `dest` is not consumed in any case. + let d = dest.clone().take().ok_or(MissingArgument)?; let universal_source = UniversalLocation::get(); - let devolved = match ensure_is_remote(universal_source.clone(), d) { - Ok(x) => x, - Err(d) => { - *dest = Some(d); - return Err(NotApplicable) - }, - }; - let (network, destination) = devolved; - let xcm = xcm.take().ok_or(SendError::MissingArgument)?; - validate_export::(network, 0, universal_source, destination, xcm) + let devolved = ensure_is_remote(universal_source.clone(), d).map_err(|_| NotApplicable)?; + let (remote_network, remote_location) = devolved; + let xcm = msg.take().ok_or(MissingArgument)?; + + validate_export::( + remote_network, + 0, + universal_source, + remote_location, + xcm.clone(), + ) + .inspect_err(|err| { + if let NotApplicable = err { + // We need to make sure that msg is not consumed in case of `NotApplicable`. + *msg = Some(xcm); + } + }) } fn deliver(ticket: Exporter::Ticket) -> Result { @@ -95,7 +103,7 @@ pub trait ExporterFor { /// /// The payment is specified from the local context, not the bridge chain. This is the /// total amount to withdraw in to Holding and should cover both payment for the execution on - /// the bridge chain as well as payment for the use of the `ExportMessage` instruction. + /// the bridge chain and payment for the use of the `ExportMessage` instruction. fn exporter_for( network: &NetworkId, remote_location: &InteriorLocation, @@ -205,7 +213,8 @@ impl, msg: &mut Option>, ) -> SendResult { - let d = dest.clone().ok_or(MissingArgument)?; + // This `clone` ensures that `dest` is not consumed in any case. + let d = dest.clone().take().ok_or(MissingArgument)?; let devolved = ensure_is_remote(UniversalLocation::get(), d).map_err(|_| NotApplicable)?; let (remote_network, remote_location) = devolved; let xcm = msg.take().ok_or(MissingArgument)?; @@ -216,7 +225,7 @@ impl(bridge, message) + validate_send::(bridge, message).inspect_err(|err| { + if let NotApplicable = err { + // We need to make sure that msg is not consumed in case of `NotApplicable`. + *msg = Some(xcm); + } + }) } fn deliver(validation: Self::Ticket) -> Result { @@ -272,9 +290,9 @@ impl, msg: &mut Option>, ) -> SendResult { - let d = dest.as_ref().ok_or(MissingArgument)?; - let devolved = - ensure_is_remote(UniversalLocation::get(), d.clone()).map_err(|_| NotApplicable)?; + // This `clone` ensures that `dest` is not consumed in any case. + let d = dest.clone().take().ok_or(MissingArgument)?; + let devolved = ensure_is_remote(UniversalLocation::get(), d).map_err(|_| NotApplicable)?; let (remote_network, remote_location) = devolved; let xcm = msg.take().ok_or(MissingArgument)?; @@ -284,7 +302,7 @@ impl(bridge, message)?; + let (v, mut cost) = validate_send::(bridge, message).inspect_err(|err| { + if let NotApplicable = err { + // We need to make sure that msg is not consumed in case of `NotApplicable`. + *msg = Some(xcm); + } + })?; if let Some(bridge_payment) = maybe_payment { cost.push(bridge_payment); } @@ -476,10 +502,10 @@ impl< let Location { parents, interior: mut junctions } = BridgedNetwork::get(); match junctions.take_first() { Some(GlobalConsensus(network)) => (network, parents), - _ => return Err(SendError::NotApplicable), + _ => return Err(NotApplicable), } }; - ensure!(&network == &bridged_network, SendError::NotApplicable); + ensure!(&network == &bridged_network, NotApplicable); // We don't/can't use the `channel` for this adapter. let dest = destination.take().ok_or(SendError::MissingArgument)?; @@ -496,7 +522,7 @@ impl< }, Err((dest, _)) => { *destination = Some(dest); - return Err(SendError::NotApplicable) + return Err(NotApplicable) }, }; @@ -540,6 +566,10 @@ impl< #[cfg(test)] mod tests { use super::*; + use frame_support::{ + assert_err, assert_ok, + traits::{Contains, Equals}, + }; #[test] fn ensure_is_remote_works() { @@ -564,21 +594,48 @@ mod tests { assert_eq!(x, Err((Parent, Polkadot, Parachain(1000)).into())); } - pub struct OkSender; - impl SendXcm for OkSender { + pub struct OkFor(PhantomData); + impl> SendXcm for OkFor { type Ticket = (); fn validate( - _destination: &mut Option, + destination: &mut Option, _message: &mut Option>, ) -> SendResult { - Ok(((), Assets::new())) + if let Some(d) = destination.as_ref() { + if Filter::contains(&d) { + return Ok(((), Assets::new())) + } + } + Err(NotApplicable) } fn deliver(_ticket: Self::Ticket) -> Result { Ok([0; 32]) } } + impl> ExportXcm for OkFor { + type Ticket = (); + + fn validate( + network: NetworkId, + _: u32, + _: &mut Option, + destination: &mut Option, + _: &mut Option>, + ) -> SendResult { + if let Some(d) = destination.as_ref() { + if Filter::contains(&(network, d.clone())) { + return Ok(((), Assets::new())) + } + } + Err(NotApplicable) + } + + fn deliver(_ticket: Self::Ticket) -> Result { + Ok([1; 32]) + } + } /// Generic test case asserting that dest and msg is not consumed by `validate` implementation /// of `SendXcm` in case of expected result. @@ -598,46 +655,168 @@ mod tests { } #[test] - fn remote_exporters_does_not_consume_dest_or_msg_on_not_applicable() { + fn local_exporters_works() { frame_support::parameter_types! { pub Local: NetworkId = ByGenesis([0; 32]); pub UniversalLocation: InteriorLocation = [GlobalConsensus(Local::get()), Parachain(1234)].into(); pub DifferentRemote: NetworkId = ByGenesis([22; 32]); - // no routers - pub BridgeTable: Vec = vec![]; + pub RemoteDestination: Junction = Parachain(9657); + pub RoutableBridgeFilter: (NetworkId, InteriorLocation) = (DifferentRemote::get(), RemoteDestination::get().into()); } + type RoutableBridgeExporter = OkFor>; + type NotApplicableBridgeExporter = OkFor<()>; + assert_ok!(validate_export::( + DifferentRemote::get(), + 0, + UniversalLocation::get(), + RemoteDestination::get().into(), + Xcm::default() + )); + assert_err!( + validate_export::( + DifferentRemote::get(), + 0, + UniversalLocation::get(), + RemoteDestination::get().into(), + Xcm::default() + ), + NotApplicable + ); - // check with local destination (should be remote) + // 1. check with local destination (should be remote) let local_dest: Location = (Parent, Parachain(5678)).into(); assert!(ensure_is_remote(UniversalLocation::get(), local_dest.clone()).is_err()); + // UnpaidLocalExporter ensure_validate_does_not_consume_dest_or_msg::< - UnpaidRemoteExporter, OkSender, UniversalLocation>, + UnpaidLocalExporter, >(local_dest.clone(), |result| assert_eq!(Err(NotApplicable), result)); + // 2. check with not applicable from the inner router (using `NotApplicableBridgeSender`) + let remote_dest: Location = + (Parent, Parent, DifferentRemote::get(), RemoteDestination::get()).into(); + assert!(ensure_is_remote(UniversalLocation::get(), remote_dest.clone()).is_ok()); + + // UnpaidLocalExporter + ensure_validate_does_not_consume_dest_or_msg::< + UnpaidLocalExporter, + >(remote_dest.clone(), |result| assert_eq!(Err(NotApplicable), result)); + + // 3. Ok - deliver + // UnpaidRemoteExporter + assert_ok!(send_xcm::>( + remote_dest, + Xcm::default() + )); + } + + #[test] + fn remote_exporters_works() { + frame_support::parameter_types! { + pub Local: NetworkId = ByGenesis([0; 32]); + pub UniversalLocation: InteriorLocation = [GlobalConsensus(Local::get()), Parachain(1234)].into(); + pub DifferentRemote: NetworkId = ByGenesis([22; 32]); + pub RoutableBridge: Location = Location::new(1, Parachain(9657)); + // not routable + pub NotApplicableBridgeTable: Vec = vec![]; + // routable + pub RoutableBridgeTable: Vec = vec![ + NetworkExportTableItem::new( + DifferentRemote::get(), + None, + RoutableBridge::get(), + None + ) + ]; + } + type RoutableBridgeSender = OkFor>; + type NotApplicableBridgeSender = OkFor<()>; + assert_ok!(validate_send::(RoutableBridge::get(), Xcm::default())); + assert_err!( + validate_send::(RoutableBridge::get(), Xcm::default()), + NotApplicable + ); + + // 1. check with local destination (should be remote) + let local_dest: Location = (Parent, Parachain(5678)).into(); + assert!(ensure_is_remote(UniversalLocation::get(), local_dest.clone()).is_err()); + + // UnpaidRemoteExporter + ensure_validate_does_not_consume_dest_or_msg::< + UnpaidRemoteExporter< + NetworkExportTable, + RoutableBridgeSender, + UniversalLocation, + >, + >(local_dest.clone(), |result| assert_eq!(Err(NotApplicable), result)); + // SovereignPaidRemoteExporter ensure_validate_does_not_consume_dest_or_msg::< SovereignPaidRemoteExporter< - NetworkExportTable, - OkSender, + NetworkExportTable, + RoutableBridgeSender, UniversalLocation, >, >(local_dest, |result| assert_eq!(Err(NotApplicable), result)); - // check with not applicable destination + // 2. check with not applicable destination (`NotApplicableBridgeTable`) let remote_dest: Location = (Parent, Parent, DifferentRemote::get()).into(); assert!(ensure_is_remote(UniversalLocation::get(), remote_dest.clone()).is_ok()); + // UnpaidRemoteExporter ensure_validate_does_not_consume_dest_or_msg::< - UnpaidRemoteExporter, OkSender, UniversalLocation>, + UnpaidRemoteExporter< + NetworkExportTable, + RoutableBridgeSender, + UniversalLocation, + >, >(remote_dest.clone(), |result| assert_eq!(Err(NotApplicable), result)); - + // SovereignPaidRemoteExporter ensure_validate_does_not_consume_dest_or_msg::< SovereignPaidRemoteExporter< - NetworkExportTable, - OkSender, + NetworkExportTable, + RoutableBridgeSender, UniversalLocation, >, >(remote_dest, |result| assert_eq!(Err(NotApplicable), result)); + + // 3. check with not applicable from the inner router (using `NotApplicableBridgeSender`) + let remote_dest: Location = (Parent, Parent, DifferentRemote::get()).into(); + assert!(ensure_is_remote(UniversalLocation::get(), remote_dest.clone()).is_ok()); + + // UnpaidRemoteExporter + ensure_validate_does_not_consume_dest_or_msg::< + UnpaidRemoteExporter< + NetworkExportTable, + NotApplicableBridgeSender, + UniversalLocation, + >, + >(remote_dest.clone(), |result| assert_eq!(Err(NotApplicable), result)); + // SovereignPaidRemoteExporter + ensure_validate_does_not_consume_dest_or_msg::< + SovereignPaidRemoteExporter< + NetworkExportTable, + NotApplicableBridgeSender, + UniversalLocation, + >, + >(remote_dest.clone(), |result| assert_eq!(Err(NotApplicable), result)); + + // 4. Ok - deliver + // UnpaidRemoteExporter + assert_ok!(send_xcm::< + UnpaidRemoteExporter< + NetworkExportTable, + RoutableBridgeSender, + UniversalLocation, + >, + >(remote_dest.clone(), Xcm::default())); + // SovereignPaidRemoteExporter + assert_ok!(send_xcm::< + SovereignPaidRemoteExporter< + NetworkExportTable, + RoutableBridgeSender, + UniversalLocation, + >, + >(remote_dest, Xcm::default())); } #[test] diff --git a/polkadot/xcm/xcm-executor/src/traits/export.rs b/polkadot/xcm/xcm-executor/src/traits/export.rs index b356e0da7df7..3e9275edab37 100644 --- a/polkadot/xcm/xcm-executor/src/traits/export.rs +++ b/polkadot/xcm/xcm-executor/src/traits/export.rs @@ -108,7 +108,7 @@ impl ExportXcm for Tuple { } /// Convenience function for using a `SendXcm` implementation. Just interprets the `dest` and wraps -/// both in `Some` before passing them as as mutable references into `T::send_xcm`. +/// both in `Some` before passing them as mutable references into `T::send_xcm`. pub fn validate_export( network: NetworkId, channel: u32, @@ -120,7 +120,7 @@ pub fn validate_export( } /// Convenience function for using a `SendXcm` implementation. Just interprets the `dest` and wraps -/// both in `Some` before passing them as as mutable references into `T::send_xcm`. +/// both in `Some` before passing them as mutable references into `T::send_xcm`. /// /// Returns either `Ok` with the price of the delivery, or `Err` with the reason why the message /// could not be sent. diff --git a/prdoc/pr_6536.prdoc b/prdoc/pr_6536.prdoc new file mode 100644 index 000000000000..676b5c131f17 --- /dev/null +++ b/prdoc/pr_6536.prdoc @@ -0,0 +1,24 @@ +title: Bridges testing improvements +doc: +- audience: Runtime Dev + description: |- + 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`. +crates: +- name: bridge-runtime-common + bump: patch +- name: pallet-bridge-relayers + bump: patch +- name: bridge-hub-rococo-runtime + bump: patch +- name: bridge-hub-westend-runtime + bump: patch +- name: bridge-hub-test-utils + bump: major +- name: parachains-runtimes-test-utils + bump: major diff --git a/prdoc/pr_6603.prdoc b/prdoc/pr_6603.prdoc new file mode 100644 index 000000000000..20c5e7294dfa --- /dev/null +++ b/prdoc/pr_6603.prdoc @@ -0,0 +1,16 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: Always provide main protocol name in litep2p responses + +doc: + - audience: [ Node Dev, Node Operator ] + description: | + This PR aligns litep2p behavior with libp2p. Previously, litep2p network backend + would provide the actual negotiated request-response protocol that produced a + response message. After this PR, only the main protocol name is reported to other + subsystems. + +crates: + - name: sc-network + bump: patch diff --git a/prdoc/pr_6645.prdoc b/prdoc/pr_6645.prdoc new file mode 100644 index 000000000000..f033cadc0b6e --- /dev/null +++ b/prdoc/pr_6645.prdoc @@ -0,0 +1,14 @@ +title: 'xcm: fix local/remote exports when inner routers return `NotApplicable`' +doc: +- audience: Runtime Dev + description: |- + Resolved a bug in the `local/remote exporters` used for bridging. Previously, they consumed `dest` and `msg` without returning them when inner routers/exporters failed with `NotApplicable`. This PR ensures compliance with the [`SendXcm`](https://github.com/paritytech/polkadot-sdk/blob/master/polkadot/xcm/src/v5/traits.rs#L449-L450) and [`ExportXcm`](https://github.com/paritytech/polkadot-sdk/blob/master/polkadot/xcm/xcm-executor/src/traits/export.rs#L44-L45) traits. +crates: +- name: staging-xcm-builder + bump: patch +- name: polkadot + bump: none +- name: staging-xcm + bump: none +- name: staging-xcm-executor + bump: none diff --git a/substrate/client/network/src/litep2p/shim/request_response/mod.rs b/substrate/client/network/src/litep2p/shim/request_response/mod.rs index a77acb464144..00ec425c9b76 100644 --- a/substrate/client/network/src/litep2p/shim/request_response/mod.rs +++ b/substrate/client/network/src/litep2p/shim/request_response/mod.rs @@ -318,7 +318,7 @@ impl RequestResponseProtocol { &mut self, peer: litep2p::PeerId, request_id: RequestId, - fallback: Option, + _fallback: Option, response: Vec, ) { match self.pending_inbound_responses.remove(&request_id) { @@ -335,10 +335,7 @@ impl RequestResponseProtocol { response.len(), ); - let _ = tx.send(Ok(( - response, - fallback.map_or_else(|| self.protocol.clone(), Into::into), - ))); + let _ = tx.send(Ok((response, self.protocol.clone()))); self.metrics.register_outbound_request_success(started.elapsed()); }, } diff --git a/substrate/frame/examples/default-config/src/lib.rs b/substrate/frame/examples/default-config/src/lib.rs index ccdcd4968598..f690bffe0998 100644 --- a/substrate/frame/examples/default-config/src/lib.rs +++ b/substrate/frame/examples/default-config/src/lib.rs @@ -62,10 +62,10 @@ pub mod pallet { type OverwrittenDefaultValue: Get; /// An input parameter that relies on `::AccountId`. This can - /// too have a default, as long as as it is present in `frame_system::DefaultConfig`. + /// too have a default, as long as it is present in `frame_system::DefaultConfig`. type CanDeriveDefaultFromSystem: Get; - /// We might chose to declare as one that doesn't have a default, for whatever semantical + /// We might choose to declare as one that doesn't have a default, for whatever semantical /// reason. #[pallet::no_default] type HasNoDefault: Get;