diff --git a/bridges/bin/runtime-common/src/mock.rs b/bridges/bin/runtime-common/src/mock.rs index fed0d15cc080b..a07bd8cc892dd 100644 --- a/bridges/bin/runtime-common/src/mock.rs +++ b/bridges/bin/runtime-common/src/mock.rs @@ -35,6 +35,7 @@ use pallet_transaction_payment::Multiplier; use sp_runtime::{ testing::H256, traits::{BlakeTwo256, ConstU32, ConstU64, ConstU8}, + transaction_validity::TransactionPriority, FixedPointNumber, Perquintill, StateVersion, }; @@ -124,6 +125,8 @@ parameter_types! { pub MinimumMultiplier: Multiplier = Multiplier::saturating_from_rational(1, 1_000_000u128); pub MaximumMultiplier: Multiplier = sp_runtime::traits::Bounded::max_value(); pub const ReserveId: [u8; 8] = *b"brdgrlrs"; + pub SlotLength: u32 = 16; + pub PriorityBoostForActiveLaneRelayer: TransactionPriority = 1; } #[derive_impl(frame_system::config_preludes::TestDefaultConfig)] @@ -212,6 +215,10 @@ impl pallet_bridge_relayers::Config for TestRuntime { type Reward = ThisChainBalance; type PaymentProcedure = TestPaymentProcedure; type StakeAndSlash = TestStakeAndSlash; + type MaxRelayersPerLane = ConstU32<16>; + type SlotLength = SlotLength; + type PriorityBoostPerItem = ConstU64<1>; + type PriorityBoostForActiveLaneRelayer = PriorityBoostForActiveLaneRelayer; type WeightInfo = (); } diff --git a/bridges/modules/relayers/src/extension/grandpa_adapter.rs b/bridges/modules/relayers/src/extension/grandpa_adapter.rs index 6c9ae1c2968c0..c3c17e561d644 100644 --- a/bridges/modules/relayers/src/extension/grandpa_adapter.rs +++ b/bridges/modules/relayers/src/extension/grandpa_adapter.rs @@ -33,9 +33,7 @@ use pallet_bridge_messages::{ CallSubType as BridgeMessagesCallSubType, Config as BridgeMessagesConfig, }; use sp_runtime::{ - traits::{Dispatchable, Get}, - transaction_validity::{TransactionPriority, TransactionValidityError}, - Saturating, + traits::Dispatchable, transaction_validity::TransactionValidityError, Saturating, }; use sp_std::marker::PhantomData; @@ -54,8 +52,6 @@ pub struct WithGrandpaChainExtensionConfig< BridgeGrandpaPalletInstance, // instance of BridgedChain `pallet-bridge-messages`, tracked by this extension BridgeMessagesPalletInstance, - // message delivery transaction priority boost for every additional message - PriorityBoostPerMessage, >( PhantomData<( IdProvider, @@ -63,12 +59,10 @@ pub struct WithGrandpaChainExtensionConfig< BatchCallUnpacker, BridgeGrandpaPalletInstance, BridgeMessagesPalletInstance, - PriorityBoostPerMessage, )>, ); -impl ExtensionConfig - for WithGrandpaChainExtensionConfig +impl ExtensionConfig for WithGrandpaChainExtensionConfig where ID: StaticStrProvider, R: BridgeRelayersConfig @@ -77,7 +71,6 @@ where BCU: BatchCallUnpacker, GI: 'static, MI: 'static, - P: Get, R::RuntimeCall: Dispatchable + BridgeGrandpaCallSubtype + BridgeMessagesCallSubType, @@ -85,7 +78,6 @@ where type IdProvider = ID; type Runtime = R; type BridgeMessagesPalletInstance = MI; - type PriorityBoostPerMessage = P; type Reward = R::Reward; type RemoteGrandpaChainBlockNumber = pallet_bridge_grandpa::BridgedBlockNumber; diff --git a/bridges/modules/relayers/src/extension/messages_adapter.rs b/bridges/modules/relayers/src/extension/messages_adapter.rs index f84e25ba5aa17..1d99d92813f09 100644 --- a/bridges/modules/relayers/src/extension/messages_adapter.rs +++ b/bridges/modules/relayers/src/extension/messages_adapter.rs @@ -63,7 +63,6 @@ where type IdProvider = ID; type Runtime = R; type BridgeMessagesPalletInstance = MI; - type PriorityBoostPerMessage = P; type Reward = R::Reward; type RemoteGrandpaChainBlockNumber = (); diff --git a/bridges/modules/relayers/src/extension/mod.rs b/bridges/modules/relayers/src/extension/mod.rs index 9abe129a22b5b..0a284447fc37c 100644 --- a/bridges/modules/relayers/src/extension/mod.rs +++ b/bridges/modules/relayers/src/extension/mod.rs @@ -35,7 +35,7 @@ use frame_support::{ dispatch::{DispatchInfo, PostDispatchInfo}, CloneNoBound, DefaultNoBound, EqNoBound, PartialEqNoBound, RuntimeDebugNoBound, }; -use frame_system::Config as SystemConfig; +use frame_system::{pallet_prelude::BlockNumberFor, Config as SystemConfig}; use pallet_bridge_messages::{CallHelper as MessagesCallHelper, Config as BridgeMessagesConfig}; use pallet_transaction_payment::{ Config as TransactionPaymentConfig, OnChargeTransaction, Pallet as TransactionPaymentPallet, @@ -125,6 +125,7 @@ where R::RuntimeCall: Dispatchable, ::OnChargeTransaction: OnChargeTransaction, + usize: TryFrom>, { /// Returns number of bundled messages `Some(_)`, if the given call info is a: /// @@ -136,16 +137,15 @@ where /// virtually boosted. The relayer registration (we only boost priority for registered /// relayer transactions) must be checked outside. fn bundled_messages_for_priority_boost( - call_info: Option<&ExtensionCallInfo>, + call_info: &ExtensionCallInfo, ) -> Option { // we only boost priority of message delivery transactions - let parsed_call = match call_info { - Some(parsed_call) if parsed_call.is_receive_messages_proof_call() => parsed_call, - _ => return None, - }; + if !call_info.is_receive_messages_proof_call() { + return None + } // compute total number of messages in transaction - let bundled_messages = parsed_call.messages_call_info().bundled_messages().saturating_len(); + let bundled_messages = call_info.messages_call_info().bundled_messages().saturating_len(); // a quick check to avoid invalid high-priority transactions let max_unconfirmed_messages_in_confirmation_tx = >::BridgedChain @@ -197,8 +197,7 @@ where // // - when relayer is registered after `validate` is called and priority is not boosted: // relayer should be ready for slashing after registration. - let may_slash_relayer = - Self::bundled_messages_for_priority_boost(Some(&call_info)).is_some(); + let may_slash_relayer = Self::bundled_messages_for_priority_boost(&call_info).is_some(); let slash_relayer_if_delivery_result = may_slash_relayer .then(|| RelayerAccountAction::Slash(relayer.clone(), reward_account_params)) .unwrap_or(RelayerAccountAction::None); @@ -273,6 +272,7 @@ where R::RuntimeCall: Dispatchable, ::OnChargeTransaction: OnChargeTransaction, + usize: TryFrom>, { const IDENTIFIER: &'static str = C::IdProvider::STR; type AccountId = R::AccountId; @@ -296,13 +296,15 @@ where // we're not calling `validate` from `pre_dispatch` directly because of performance // reasons, so if you're adding some code that may fail here, please check if it needs // to be added to the `pre_dispatch` as well - let parsed_call = C::parse_and_check_for_obsolete_call(call)?; + let parsed_call = match C::parse_and_check_for_obsolete_call(call)? { + Some(parsed_call) => parsed_call, + None => return Ok(Default::default()), + }; // the following code just plays with transaction priority and never returns an error // we only boost priority of presumably correct message delivery transactions - let bundled_messages = match Self::bundled_messages_for_priority_boost(parsed_call.as_ref()) - { + let bundled_messages = match Self::bundled_messages_for_priority_boost(&parsed_call) { Some(bundled_messages) => bundled_messages, None => return Ok(Default::default()), }; @@ -313,8 +315,11 @@ where } // compute priority boost - let priority_boost = - priority::compute_priority_boost::(bundled_messages); + let priority_boost = priority::compute_priority_boost::( + parsed_call.messages_call_info().lane_id(), + bundled_messages, + who, + ); let valid_transaction = ValidTransactionBuilder::default().priority(priority_boost); log::trace!( @@ -322,7 +327,7 @@ where "{}.{:?}: has boosted priority of message delivery transaction \ of relayer {:?}: {} messages -> {} priority", Self::IDENTIFIER, - parsed_call.as_ref().map(|p| p.messages_call_info().lane_id()), + parsed_call.messages_call_info().lane_id(), who, bundled_messages, priority_boost, @@ -447,7 +452,7 @@ mod tests { use pallet_bridge_parachains::{Call as ParachainsCall, Pallet as ParachainsPallet}; use pallet_utility::Call as UtilityCall; use sp_runtime::{ - traits::{ConstU64, Header as HeaderT}, + traits::Header as HeaderT, transaction_validity::{InvalidTransaction, ValidTransaction}, DispatchError, }; @@ -475,7 +480,6 @@ mod tests { RuntimeWithUtilityPallet, (), (), - ConstU64<1>, >; type TestGrandpaExtension = BridgeRelayersSignedExtension; @@ -485,7 +489,6 @@ mod tests { RuntimeWithUtilityPallet, (), (), - ConstU64<1>, >; type TestExtension = BridgeRelayersSignedExtension; diff --git a/bridges/modules/relayers/src/extension/parachain_adapter.rs b/bridges/modules/relayers/src/extension/parachain_adapter.rs index b6f57cebc309d..617fb1cf116bf 100644 --- a/bridges/modules/relayers/src/extension/parachain_adapter.rs +++ b/bridges/modules/relayers/src/extension/parachain_adapter.rs @@ -38,10 +38,7 @@ use pallet_bridge_parachains::{ CallSubType as BridgeParachainsCallSubtype, Config as BridgeParachainsConfig, SubmitParachainHeadsHelper, }; -use sp_runtime::{ - traits::{Dispatchable, Get}, - transaction_validity::{TransactionPriority, TransactionValidityError}, -}; +use sp_runtime::{traits::Dispatchable, transaction_validity::TransactionValidityError}; use sp_std::marker::PhantomData; /// Adapter to be used in signed extension configuration, when bridging with remote parachains. @@ -58,8 +55,6 @@ pub struct WithParachainExtensionConfig< BridgeParachainsPalletInstance, // instance of BridgedChain `pallet-bridge-messages`, tracked by this extension BridgeMessagesPalletInstance, - // message delivery transaction priority boost for every additional message - PriorityBoostPerMessage, >( PhantomData<( IdProvider, @@ -67,11 +62,10 @@ pub struct WithParachainExtensionConfig< BatchCallUnpacker, BridgeParachainsPalletInstance, BridgeMessagesPalletInstance, - PriorityBoostPerMessage, )>, ); -impl ExtensionConfig for WithParachainExtensionConfig +impl ExtensionConfig for WithParachainExtensionConfig where ID: StaticStrProvider, R: BridgeRelayersConfig @@ -81,7 +75,6 @@ where BCU: BatchCallUnpacker, PI: 'static, MI: 'static, - P: Get, R::RuntimeCall: Dispatchable + BridgeGrandpaCallSubtype + BridgeParachainsCallSubtype @@ -91,7 +84,6 @@ where type IdProvider = ID; type Runtime = R; type BridgeMessagesPalletInstance = MI; - type PriorityBoostPerMessage = P; type Reward = R::Reward; type RemoteGrandpaChainBlockNumber = pallet_bridge_grandpa::BridgedBlockNumber; diff --git a/bridges/modules/relayers/src/extension/priority.rs b/bridges/modules/relayers/src/extension/priority.rs index da188eaf5bdd4..57302681364f2 100644 --- a/bridges/modules/relayers/src/extension/priority.rs +++ b/bridges/modules/relayers/src/extension/priority.rs @@ -22,8 +22,15 @@ //! single message with nonce `N`, then the transaction with nonces `N..=N+100` will //! be rejected. This can lower bridge throughput down to one message per block. +use crate::{Config as RelayersConfig, Pallet as RelayersPallet}; + +use bp_messages::LaneId; use frame_support::traits::Get; -use sp_runtime::transaction_validity::TransactionPriority; +use frame_system::{pallet_prelude::BlockNumberFor, Pallet as SystemPallet}; +use sp_runtime::{ + traits::{One, Zero}, + transaction_validity::TransactionPriority, +}; // reexport everything from `integrity_tests` module #[allow(unused_imports)] @@ -33,23 +40,77 @@ pub use integrity_tests::*; /// To avoid being too verbose with generic code, let's just define a separate alias. pub type ItemCount = u64; -/// Compute priority boost for transaction that brings given number of bridge -/// items (messages, headers, ...), when every additional item adds `PriorityBoostPerItem` -/// to transaction priority. -pub fn compute_priority_boost(n_items: ItemCount) -> TransactionPriority +/// Compute total priority boost for message delivery transaction that brings given number of bridge +/// items (messages, headers, ...). +pub fn compute_priority_boost( + lane_id: LaneId, + n_items: ItemCount, + relayer: &R::AccountId, +) -> TransactionPriority where - PriorityBoostPerItem: Get, + usize: TryFrom>, { - // we don't want any boost for transaction with single (additional) item => minus one + compute_per_item_priority_boost::(n_items) + .saturating_add(compute_per_lane_priority_boost::(lane_id, relayer)) +} + +/// Compute priority boost for message delivery transaction, that depends on number +/// of bundled messages. +fn compute_per_item_priority_boost>( + n_items: ItemCount, +) -> TransactionPriority { + // we don't want any boost for transaction with single message => minus one PriorityBoostPerItem::get().saturating_mul(n_items.saturating_sub(1)) } +/// Compute priority boost for message delivery transaction, that depends on +/// the set of lane relayers and current slot. +fn compute_per_lane_priority_boost( + lane_id: LaneId, + relayer: &R::AccountId, +) -> TransactionPriority +where + usize: TryFrom>, +{ + // if there are no relayers, explicitly registered at this lane, noone gets additional + // priority boost + let lane_relayers = RelayersPallet::::lane_relayers(lane_id); + let lane_relayers_len: BlockNumberFor = (lane_relayers.len() as u32).into(); + if lane_relayers_len.is_zero() { + return 0 + } + + // we can't deal with slots shorter than 1 block + let slot_length: BlockNumberFor = R::SlotLength::get(); + if slot_length < One::one() { + return 0 + } + + // let's compute current slot number + let current_block_number = SystemPallet::::block_number(); + let slot = current_block_number / slot_length; + + // and then get the relayer for that slot + let slot_relayer = match usize::try_from(slot % lane_relayers_len) { + Ok(slot_relayer_index) => &lane_relayers[slot_relayer_index], + Err(_) => return 0, + }; + + // if message delivery transaction is submitted by the relayer, assigned to the current + // slot, let's boost the transaction priority + if relayer != slot_relayer { + return 0 + } + + R::PriorityBoostForActiveLaneRelayer::get() +} + #[cfg(not(feature = "integrity-test"))] mod integrity_tests {} #[cfg(feature = "integrity-test")] mod integrity_tests { - use super::{compute_priority_boost, ItemCount}; + use super::{compute_per_item_priority_boost, ItemCount}; use bp_messages::MessageNonce; use bp_runtime::PreComputedSize; @@ -89,7 +150,7 @@ mod integrity_tests { let priority_boost_per_item = PriorityBoostPerItem::get(); for n_items in 1..=max_items { let base_priority = estimate_priority(n_items, Zero::zero()); - let priority_boost = compute_priority_boost::(n_items); + let priority_boost = compute_per_item_priority_boost::(n_items); let priority_with_boost = base_priority .checked_add(priority_boost) .expect("priority overflow: try lowering `max_items` or `tip_boost_per_item`?"); @@ -417,3 +478,58 @@ mod integrity_tests { } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::{mock::*, LaneRelayers}; + + #[test] + fn compute_per_lane_priority_boost_works() { + run_test(|| { + // insert 3 relayers to the queue + let lane_id = LaneId::new(1, 2); + let relayer1 = 1_000; + let relayer2 = 2_000; + let relayer3 = 3_000; + LaneRelayers::::insert( + lane_id, + sp_runtime::BoundedVec::try_from(vec![relayer1, relayer2, relayer3]).unwrap(), + ); + + // at blocks 1..=SlotLength relayer1 gets the boost + System::set_block_number(0); + for _ in 1..SlotLength::get() { + System::set_block_number(System::block_number() + 1); + assert_eq!( + compute_per_lane_priority_boost::(lane_id, &relayer1), + PriorityBoostForActiveLaneRelayer::get(), + ); + assert_eq!(compute_per_lane_priority_boost::(lane_id, &relayer2), 0,); + assert_eq!(compute_per_lane_priority_boost::(lane_id, &relayer3), 0,); + } + + // at next slot, relayer2 gets the boost + for _ in 1..=SlotLength::get() { + System::set_block_number(System::block_number() + 1); + assert_eq!(compute_per_lane_priority_boost::(lane_id, &relayer1), 0,); + assert_eq!( + compute_per_lane_priority_boost::(lane_id, &relayer2), + PriorityBoostForActiveLaneRelayer::get(), + ); + assert_eq!(compute_per_lane_priority_boost::(lane_id, &relayer3), 0,); + } + + // at next slot, relayer3 gets the boost + for _ in 1..=SlotLength::get() { + System::set_block_number(System::block_number() + 1); + assert_eq!(compute_per_lane_priority_boost::(lane_id, &relayer1), 0,); + assert_eq!(compute_per_lane_priority_boost::(lane_id, &relayer2), 0,); + assert_eq!( + compute_per_lane_priority_boost::(lane_id, &relayer3), + PriorityBoostForActiveLaneRelayer::get(), + ); + } + }); + } +} diff --git a/bridges/modules/relayers/src/lib.rs b/bridges/modules/relayers/src/lib.rs index b9627774db1ec..889f1b13a5c15 100644 --- a/bridges/modules/relayers/src/lib.rs +++ b/bridges/modules/relayers/src/lib.rs @@ -20,6 +20,7 @@ #![cfg_attr(not(feature = "std"), no_std)] #![warn(missing_docs)] +use bp_messages::LaneId; use bp_relayers::{ ExplicitOrAccountParams, PaymentProcedure, Registration, RelayerRewardsKeyProvider, RewardsAccountParams, StakeAndSlash, @@ -66,8 +67,41 @@ pub mod pallet { type Reward: AtLeast32BitUnsigned + Copy + Member + Parameter + MaxEncodedLen; /// Pay rewards scheme. type PaymentProcedure: PaymentProcedure; + /// Stake and slash scheme. type StakeAndSlash: StakeAndSlash, Self::Reward>; + + /// Maximal number of relayers that can register themselves on a single lane. + #[pallet::constant] + type MaxRelayersPerLane: Get; + /// Length of slots in chain blocks. + /// + /// Registered relayer may explicitly register himself at some lane to get priority boost + /// for message delivery transactions on that lane (that is done using `register_at_lane` + /// pallet call). All relayers, registered at the lane form an ordered queue and only + /// relayer at the head of that queue receives a boost at his slot (which has a length of + /// `SlotLength` blocks). Then the "best" relayer is removed and pushed to the tail of the + /// queue and next relayer gets the boost during next `SlotLength` blocks. And so on... + /// + /// Shall not be too low to have an effect, because there's some (at least one block) lag + /// between moments when priority is computed and when active slot changes. + type SlotLength: Get>; + /// Priority boost that the registered relayer gets for every additional message in the + /// message delivery transaction. + type PriorityBoostPerItem: Get; + /// Additional priority boost, that is added to regular `PriorityBoostPerMessage` boost for + /// message delivery transactions, submitted by relayer at the head of the lane relayers + /// queue. + /// + /// In other words, if relayer has registered at some `lane`, using `register_at_lane` call + /// AND he is currently at the head of the lane relayers queue, his message delivery + /// transaction will get the following additional priority boost: + /// + /// ```nocompile + /// T::PriorityBoostForActiveLaneRelayer::get() + T::PriorityBoostPerMessage::get() * (msgs - 1) + /// ``` + type PriorityBoostForActiveLaneRelayer: Get; + /// Pallet call weights. type WeightInfo: WeightInfoExt; } @@ -464,6 +498,21 @@ pub mod pallet { Registration, T::Reward>, OptionQuery, >; + + /// A set of relayers that have explicitly registered themselves at a given lane. + /// + /// Every relayer inside this set receives additional priority boost when it submits + /// message delivers messages at given lane. The boost only happens inside the slot, + /// assigned to relayer. + #[pallet::storage] + #[pallet::getter(fn lane_relayers)] + pub type LaneRelayers = StorageMap< + _, + Identity, + LaneId, + BoundedVec, + ValueQuery, + >; } #[cfg(test)] diff --git a/bridges/modules/relayers/src/mock.rs b/bridges/modules/relayers/src/mock.rs index 43aeec8bfc3b4..d91817d2f2377 100644 --- a/bridges/modules/relayers/src/mock.rs +++ b/bridges/modules/relayers/src/mock.rs @@ -38,6 +38,7 @@ use pallet_transaction_payment::Multiplier; use sp_core::{ConstU64, ConstU8, H256}; use sp_runtime::{ traits::{BlakeTwo256, ConstU32}, + transaction_validity::TransactionPriority, BuildStorage, FixedPointNumber, Perquintill, StateVersion, }; @@ -191,13 +192,13 @@ parameter_types! { pub AdjustmentVariable: Multiplier = Multiplier::saturating_from_rational(3, 100_000); pub MinimumMultiplier: Multiplier = Multiplier::saturating_from_rational(1, 1_000_000u128); pub MaximumMultiplier: Multiplier = sp_runtime::traits::Bounded::max_value(); + pub SlotLength: u32 = 16; + pub PriorityBoostForActiveLaneRelayer: TransactionPriority = 1; } #[derive_impl(frame_system::config_preludes::TestDefaultConfig)] impl frame_system::Config for TestRuntime { type Block = ThisChainBlock; - // TODO: remove when https://github.com/paritytech/polkadot-sdk/pull/4543 merged - type BlockHashCount = ConstU32<10>; type AccountData = pallet_balances::AccountData; type DbWeight = DbWeight; } @@ -278,6 +279,10 @@ impl pallet_bridge_relayers::Config for TestRuntime { type Reward = ThisChainBalance; type PaymentProcedure = TestPaymentProcedure; type StakeAndSlash = TestStakeAndSlash; + type MaxRelayersPerLane = ConstU32<16>; + type SlotLength = SlotLength; + type PriorityBoostPerItem = ConstU64<1>; + type PriorityBoostForActiveLaneRelayer = PriorityBoostForActiveLaneRelayer; type WeightInfo = (); } diff --git a/bridges/primitives/relayers/src/extension.rs b/bridges/primitives/relayers/src/extension.rs index 5ab8e6cde96b4..7cc40a6f0155c 100644 --- a/bridges/primitives/relayers/src/extension.rs +++ b/bridges/primitives/relayers/src/extension.rs @@ -26,11 +26,7 @@ use frame_support::{ }; use frame_system::Config as SystemConfig; use pallet_utility::{Call as UtilityCall, Pallet as UtilityPallet}; -use sp_runtime::{ - traits::Get, - transaction_validity::{TransactionPriority, TransactionValidityError}, - RuntimeDebug, -}; +use sp_runtime::{transaction_validity::TransactionValidityError, RuntimeDebug}; use sp_std::{fmt::Debug, marker::PhantomData, vec, vec::Vec}; /// Type of the call that the signed extension recognizes. @@ -121,9 +117,6 @@ pub trait ExtensionConfig { type Runtime: frame_system::Config; /// Messages pallet instance. type BridgeMessagesPalletInstance: 'static; - /// Additional priority that is added to base message delivery transaction priority - /// for every additional bundled message. - type PriorityBoostPerMessage: Get; /// Type of reward, that the `pallet-bridge-relayers` is using. type Reward; /// Block number for the remote **GRANDPA chain**. Mind that this chain is not