From f7380490c0245daa917f906c9e518ff6f663b968 Mon Sep 17 00:00:00 2001
From: Svyatoslav Nikolsky <svyatonik@gmail.com>
Date: Mon, 10 Apr 2023 13:43:37 +0300
Subject: [PATCH] Boost message delivery transaction priority (#2023)
* reject delivery transactions with at least one obsolete message
* clippy
* boost priority of message delivery transactions: transaction with more messages has larger priority than the transaction with less messages
* apply review suggestion
* CallInfo::bundled_messages
* validate_does_not_boost_priority_of_message_delivery_transactons_with_too_many_messages
* clippy
---
bridges/bin/millau/runtime/Cargo.toml | 2 +-
bridges/bin/millau/runtime/src/lib.rs | 2 +
.../runtime/src/rialto_parachain_messages.rs | 70 +++++
bridges/bin/runtime-common/Cargo.toml | 1 +
bridges/bin/runtime-common/src/lib.rs | 1 +
.../runtime-common/src/messages_call_ext.rs | 24 +-
bridges/bin/runtime-common/src/mock.rs | 2 +-
.../runtime-common/src/priority_calculator.rs | 201 +++++++++++++++
.../src/refund_relayer_extension.rs | 243 +++++++++++++-----
9 files changed, 481 insertions(+), 65 deletions(-)
create mode 100644 bridges/bin/runtime-common/src/priority_calculator.rs
diff --git a/bridges/bin/millau/runtime/Cargo.toml b/bridges/bin/millau/runtime/Cargo.toml
index ce1484fecad..e1a55ea6f24 100644
--- a/bridges/bin/millau/runtime/Cargo.toml
+++ b/bridges/bin/millau/runtime/Cargo.toml
@@ -69,7 +69,7 @@ xcm-builder = { git = "https://github.com/paritytech/polkadot", branch = "master
xcm-executor = { git = "https://github.com/paritytech/polkadot", branch = "master", default-features = false }
[dev-dependencies]
-bridge-runtime-common = { path = "../../runtime-common", features = ["integrity-test"] }
+bridge-runtime-common = { path = "../../runtime-common", features = ["integrity-test", "std"] }
env_logger = "0.10"
static_assertions = "1.1"
diff --git a/bridges/bin/millau/runtime/src/lib.rs b/bridges/bin/millau/runtime/src/lib.rs
index deff859a816..4e6f1e43e8c 100644
--- a/bridges/bin/millau/runtime/src/lib.rs
+++ b/bridges/bin/millau/runtime/src/lib.rs
@@ -589,11 +589,13 @@ generate_bridge_reject_obsolete_headers_and_messages! {
bp_runtime::generate_static_str_provider!(BridgeRefundRialtoPara2000Lane0Msgs);
/// Signed extension that refunds relayers that are delivering messages from the Rialto parachain.
+pub type PriorityBoostPerMessage = ConstU64<921_900_294>;
pub type BridgeRefundRialtoParachainMessages = RefundBridgedParachainMessages<
Runtime,
RefundableParachain<WithRialtoParachainsInstance, RialtoParachainId>,
RefundableMessagesLane<WithRialtoParachainMessagesInstance, RialtoParachainMessagesLane>,
ActualFeeRefund<Runtime>,
+ PriorityBoostPerMessage,
StrBridgeRefundRialtoPara2000Lane0Msgs,
>;
diff --git a/bridges/bin/millau/runtime/src/rialto_parachain_messages.rs b/bridges/bin/millau/runtime/src/rialto_parachain_messages.rs
index b3d33c1cefd..bef8a281188 100644
--- a/bridges/bin/millau/runtime/src/rialto_parachain_messages.rs
+++ b/bridges/bin/millau/runtime/src/rialto_parachain_messages.rs
@@ -137,3 +137,73 @@ impl XcmBlobHauler for ToRialtoParachainXcmBlobHauler {
XCM_LANE
}
}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use crate::{
+ PriorityBoostPerMessage, RialtoGrandpaInstance, Runtime,
+ WithRialtoParachainMessagesInstance,
+ };
+
+ use bridge_runtime_common::{
+ assert_complete_bridge_types,
+ integrity::{
+ assert_complete_bridge_constants, check_message_lane_weights,
+ AssertBridgeMessagesPalletConstants, AssertBridgePalletNames, AssertChainConstants,
+ AssertCompleteBridgeConstants,
+ },
+ };
+
+ #[test]
+ fn ensure_millau_message_lane_weights_are_correct() {
+ check_message_lane_weights::<bp_millau::Millau, Runtime>(
+ bp_rialto_parachain::EXTRA_STORAGE_PROOF_SIZE,
+ bp_millau::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX,
+ bp_millau::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX,
+ );
+ }
+
+ #[test]
+ fn ensure_bridge_integrity() {
+ assert_complete_bridge_types!(
+ runtime: Runtime,
+ with_bridged_chain_grandpa_instance: RialtoGrandpaInstance,
+ with_bridged_chain_messages_instance: WithRialtoParachainMessagesInstance,
+ bridge: WithRialtoParachainMessageBridge,
+ this_chain: bp_millau::Millau,
+ bridged_chain: bp_rialto::Rialto,
+ );
+
+ assert_complete_bridge_constants::<
+ Runtime,
+ RialtoGrandpaInstance,
+ WithRialtoParachainMessagesInstance,
+ WithRialtoParachainMessageBridge,
+ >(AssertCompleteBridgeConstants {
+ this_chain_constants: AssertChainConstants {
+ block_length: bp_millau::BlockLength::get(),
+ block_weights: bp_millau::BlockWeights::get(),
+ },
+ messages_pallet_constants: AssertBridgeMessagesPalletConstants {
+ max_unrewarded_relayers_in_bridged_confirmation_tx:
+ bp_rialto_parachain::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX,
+ max_unconfirmed_messages_in_bridged_confirmation_tx:
+ bp_rialto_parachain::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX,
+ bridged_chain_id: bp_runtime::RIALTO_PARACHAIN_CHAIN_ID,
+ },
+ pallet_names: AssertBridgePalletNames {
+ with_this_chain_messages_pallet_name: bp_millau::WITH_MILLAU_MESSAGES_PALLET_NAME,
+ with_bridged_chain_grandpa_pallet_name: bp_rialto::WITH_RIALTO_GRANDPA_PALLET_NAME,
+ with_bridged_chain_messages_pallet_name:
+ bp_rialto_parachain::WITH_RIALTO_PARACHAIN_MESSAGES_PALLET_NAME,
+ },
+ });
+
+ bridge_runtime_common::priority_calculator::ensure_priority_boost_is_sane::<
+ Runtime,
+ WithRialtoParachainMessagesInstance,
+ PriorityBoostPerMessage,
+ >(1_000_000);
+ }
+}
diff --git a/bridges/bin/runtime-common/Cargo.toml b/bridges/bin/runtime-common/Cargo.toml
index 3db4ae9abca..9d616bf588e 100644
--- a/bridges/bin/runtime-common/Cargo.toml
+++ b/bridges/bin/runtime-common/Cargo.toml
@@ -47,6 +47,7 @@ xcm-executor = { git = "https://github.com/paritytech/polkadot", branch = "maste
[dev-dependencies]
bp-test-utils = { path = "../../primitives/test-utils" }
+bitvec = { version = "1", features = ["alloc"] }
pallet-balances = { git = "https://github.com/paritytech/substrate", branch = "master" }
[features]
diff --git a/bridges/bin/runtime-common/src/lib.rs b/bridges/bin/runtime-common/src/lib.rs
index ed486c04abc..e8a2d2470fa 100644
--- a/bridges/bin/runtime-common/src/lib.rs
+++ b/bridges/bin/runtime-common/src/lib.rs
@@ -30,6 +30,7 @@ pub mod messages_benchmarking;
pub mod messages_call_ext;
pub mod messages_xcm_extension;
pub mod parachains_benchmarking;
+pub mod priority_calculator;
pub mod refund_relayer_extension;
mod messages_generation;
diff --git a/bridges/bin/runtime-common/src/messages_call_ext.rs b/bridges/bin/runtime-common/src/messages_call_ext.rs
index b299d758a32..8d53c4844c8 100644
--- a/bridges/bin/runtime-common/src/messages_call_ext.rs
+++ b/bridges/bin/runtime-common/src/messages_call_ext.rs
@@ -104,6 +104,22 @@ pub enum CallInfo {
ReceiveMessagesDeliveryProof(ReceiveMessagesDeliveryProofInfo),
}
+impl CallInfo {
+ /// Returns number of messages, bundled with this transaction.
+ pub fn bundled_messages(&self) -> MessageNonce {
+ let bundled_range = match *self {
+ Self::ReceiveMessagesProof(ref info) => &info.0.bundled_range,
+ Self::ReceiveMessagesDeliveryProof(ref info) => &info.0.bundled_range,
+ };
+
+ bundled_range
+ .end()
+ .checked_sub(*bundled_range.start())
+ .map(|d| d.saturating_add(1))
+ .unwrap_or(0)
+ }
+}
+
/// Helper struct that provides methods for working with a call supported by `CallInfo`.
pub struct CallHelper<T: Config<I>, I: 'static> {
pub _phantom_data: sp_std::marker::PhantomData<(T, I)>,
@@ -321,6 +337,7 @@ mod tests {
TestRuntime, ThisChainRuntimeCall,
},
};
+ use bitvec::prelude::*;
use bp_messages::{DeliveredMessages, UnrewardedRelayer, UnrewardedRelayersState};
use sp_std::ops::RangeInclusive;
@@ -330,7 +347,11 @@ mod tests {
for n in 0..MaxUnrewardedRelayerEntriesAtInboundLane::get() {
inbound_lane_state.relayers.push_back(UnrewardedRelayer {
relayer: Default::default(),
- messages: DeliveredMessages { begin: n + 1, end: n + 1 },
+ messages: DeliveredMessages {
+ begin: n + 1,
+ end: n + 1,
+ dispatch_results: bitvec![u8, Msb0; 1; 1],
+ },
});
}
pallet_bridge_messages::InboundLanes::<TestRuntime>::insert(
@@ -347,6 +368,7 @@ mod tests {
messages: DeliveredMessages {
begin: 1,
end: MaxUnconfirmedMessagesAtInboundLane::get(),
+ dispatch_results: bitvec![u8, Msb0; 1; MaxUnconfirmedMessagesAtInboundLane::get() as _],
},
});
pallet_bridge_messages::InboundLanes::<TestRuntime>::insert(
diff --git a/bridges/bin/runtime-common/src/mock.rs b/bridges/bin/runtime-common/src/mock.rs
index 967682ccbc5..036813f6fd5 100644
--- a/bridges/bin/runtime-common/src/mock.rs
+++ b/bridges/bin/runtime-common/src/mock.rs
@@ -127,7 +127,7 @@ parameter_types! {
pub MinimumMultiplier: Multiplier = Multiplier::saturating_from_rational(1, 1_000_000u128);
pub MaximumMultiplier: Multiplier = sp_runtime::traits::Bounded::max_value();
pub const MaxUnrewardedRelayerEntriesAtInboundLane: MessageNonce = 16;
- pub const MaxUnconfirmedMessagesAtInboundLane: MessageNonce = 32;
+ pub const MaxUnconfirmedMessagesAtInboundLane: MessageNonce = 1_000;
}
impl frame_system::Config for TestRuntime {
diff --git a/bridges/bin/runtime-common/src/priority_calculator.rs b/bridges/bin/runtime-common/src/priority_calculator.rs
new file mode 100644
index 00000000000..590de05fb1c
--- /dev/null
+++ b/bridges/bin/runtime-common/src/priority_calculator.rs
@@ -0,0 +1,201 @@
+// Copyright 2021 Parity Technologies (UK) Ltd.
+// This file is part of Parity Bridges Common.
+
+// Parity Bridges Common is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Parity Bridges Common is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Parity Bridges Common. If not, see <http://www.gnu.org/licenses/>.
+
+//! Bridge transaction priority calculator.
+//!
+//! We want to prioritize message delivery transactions with more messages over
+//! transactions with less messages. That's because we reject delivery transactions
+//! if it contains already delivered message. And if some transaction delivers
+//! 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 bp_messages::MessageNonce;
+use frame_support::traits::Get;
+use sp_runtime::transaction_validity::TransactionPriority;
+
+// reexport everything from `integrity_tests` module
+pub use integrity_tests::*;
+
+/// Compute priority boost for message delivery transaction that delivers
+/// given number of messages.
+pub fn compute_priority_boost<PriorityBoostPerMessage>(
+ messages: MessageNonce,
+) -> TransactionPriority
+where
+ PriorityBoostPerMessage: Get<TransactionPriority>,
+{
+ // we don't want any boost for transaction with single message => minus one
+ PriorityBoostPerMessage::get().saturating_mul(messages - 1)
+}
+
+#[cfg(not(feature = "integrity-test"))]
+mod integrity_tests {}
+
+#[cfg(feature = "integrity-test")]
+mod integrity_tests {
+ use super::compute_priority_boost;
+
+ use bp_messages::MessageNonce;
+ use bp_runtime::PreComputedSize;
+ use frame_support::{
+ dispatch::{DispatchClass, DispatchInfo, Dispatchable, Pays, PostDispatchInfo},
+ traits::Get,
+ };
+ use pallet_bridge_messages::WeightInfoExt;
+ use pallet_transaction_payment::OnChargeTransaction;
+ use sp_runtime::{
+ traits::{UniqueSaturatedInto, Zero},
+ transaction_validity::TransactionPriority,
+ FixedPointOperand, SaturatedConversion, Saturating,
+ };
+
+ type BalanceOf<T> =
+ <<T as pallet_transaction_payment::Config>::OnChargeTransaction as OnChargeTransaction<
+ T,
+ >>::Balance;
+
+ /// Ensures that the value of `PriorityBoostPerMessage` matches the value of
+ /// `tip_boost_per_message`.
+ ///
+ /// We want two transactions, `TX1` with `N` messages and `TX2` with `N+1` messages, have almost
+ /// the same priority if we'll add `tip_boost_per_message` tip to the `TX1`. We want to be sure
+ /// that if we add plain `PriorityBoostPerMessage` priority to `TX1`, the priority will be close
+ /// to `TX2` as well.
+ pub fn ensure_priority_boost_is_sane<Runtime, MessagesInstance, PriorityBoostPerMessage>(
+ tip_boost_per_message: BalanceOf<Runtime>,
+ ) where
+ Runtime:
+ pallet_transaction_payment::Config + pallet_bridge_messages::Config<MessagesInstance>,
+ MessagesInstance: 'static,
+ PriorityBoostPerMessage: Get<TransactionPriority>,
+ Runtime::RuntimeCall: Dispatchable<Info = DispatchInfo, PostInfo = PostDispatchInfo>,
+ BalanceOf<Runtime>: Send + Sync + FixedPointOperand,
+ {
+ let priority_boost_per_message = PriorityBoostPerMessage::get();
+ let maximal_messages_in_delivery_transaction =
+ Runtime::MaxUnconfirmedMessagesAtInboundLane::get();
+ for messages in 1..=maximal_messages_in_delivery_transaction {
+ let base_priority = estimate_message_delivery_transaction_priority::<
+ Runtime,
+ MessagesInstance,
+ >(messages, Zero::zero());
+ let priority_boost = compute_priority_boost::<PriorityBoostPerMessage>(messages);
+ let priority_with_boost = base_priority + priority_boost;
+
+ let tip = tip_boost_per_message.saturating_mul((messages - 1).unique_saturated_into());
+ let priority_with_tip =
+ estimate_message_delivery_transaction_priority::<Runtime, MessagesInstance>(1, tip);
+
+ const ERROR_MARGIN: TransactionPriority = 5; // 5%
+ if priority_with_boost.abs_diff(priority_with_tip).saturating_mul(100) /
+ priority_with_tip >
+ ERROR_MARGIN
+ {
+ panic!(
+ "The PriorityBoostPerMessage value ({}) must be fixed to: {}",
+ priority_boost_per_message,
+ compute_priority_boost_per_message::<Runtime, MessagesInstance>(
+ tip_boost_per_message
+ ),
+ );
+ }
+ }
+ }
+
+ /// Compute priority boost that we give to message delivery transaction for additional message.
+ #[cfg(feature = "integrity-test")]
+ fn compute_priority_boost_per_message<Runtime, MessagesInstance>(
+ tip_boost_per_message: BalanceOf<Runtime>,
+ ) -> TransactionPriority
+ where
+ Runtime:
+ pallet_transaction_payment::Config + pallet_bridge_messages::Config<MessagesInstance>,
+ MessagesInstance: 'static,
+ Runtime::RuntimeCall: Dispatchable<Info = DispatchInfo, PostInfo = PostDispatchInfo>,
+ BalanceOf<Runtime>: Send + Sync + FixedPointOperand,
+ {
+ // esimate priority of transaction that delivers one message and has large tip
+ let maximal_messages_in_delivery_transaction =
+ Runtime::MaxUnconfirmedMessagesAtInboundLane::get();
+ let small_with_tip_priority =
+ estimate_message_delivery_transaction_priority::<Runtime, MessagesInstance>(
+ 1,
+ tip_boost_per_message
+ .saturating_mul(maximal_messages_in_delivery_transaction.saturated_into()),
+ );
+ // estimate priority of transaction that delivers maximal number of messages, but has no tip
+ let large_without_tip_priority = estimate_message_delivery_transaction_priority::<
+ Runtime,
+ MessagesInstance,
+ >(maximal_messages_in_delivery_transaction, Zero::zero());
+
+ small_with_tip_priority
+ .saturating_sub(large_without_tip_priority)
+ .saturating_div(maximal_messages_in_delivery_transaction - 1)
+ }
+
+ /// Estimate message delivery transaction priority.
+ #[cfg(feature = "integrity-test")]
+ fn estimate_message_delivery_transaction_priority<Runtime, MessagesInstance>(
+ messages: MessageNonce,
+ tip: BalanceOf<Runtime>,
+ ) -> TransactionPriority
+ where
+ Runtime:
+ pallet_transaction_payment::Config + pallet_bridge_messages::Config<MessagesInstance>,
+ MessagesInstance: 'static,
+ Runtime::RuntimeCall: Dispatchable<Info = DispatchInfo, PostInfo = PostDispatchInfo>,
+ BalanceOf<Runtime>: Send + Sync + FixedPointOperand,
+ {
+ // just an estimation of extra transaction bytes that are added to every transaction
+ // (including signature, signed extensions extra and etc + in our case it includes
+ // all call arguments extept the proof itself)
+ let base_tx_size = 512;
+ // let's say we are relaying similar small messages and for every message we add more trie
+ // nodes to the proof (x0.5 because we expect some nodes to be reused)
+ let estimated_message_size = 512;
+ // let's say all our messages have the same dispatch weight
+ let estimated_message_dispatch_weight =
+ Runtime::WeightInfo::message_dispatch_weight(estimated_message_size);
+ // messages proof argument size is (for every message) messages size + some additional
+ // trie nodes. Some of them are reused by different messages, so let's take 2/3 of default
+ // "overhead" constant
+ let messages_proof_size = Runtime::WeightInfo::expected_extra_storage_proof_size()
+ .saturating_mul(2)
+ .saturating_div(3)
+ .saturating_add(estimated_message_size)
+ .saturating_mul(messages as _);
+
+ // finally we are able to estimate transaction size and weight
+ let transaction_size = base_tx_size.saturating_add(messages_proof_size);
+ let transaction_weight = Runtime::WeightInfo::receive_messages_proof_weight(
+ &PreComputedSize(transaction_size as _),
+ messages as _,
+ estimated_message_dispatch_weight.saturating_mul(messages),
+ );
+
+ pallet_transaction_payment::ChargeTransactionPayment::<Runtime>::get_priority(
+ &DispatchInfo {
+ weight: transaction_weight,
+ class: DispatchClass::Normal,
+ pays_fee: Pays::Yes,
+ },
+ transaction_size as _,
+ tip,
+ Zero::zero(),
+ )
+ }
+}
diff --git a/bridges/bin/runtime-common/src/refund_relayer_extension.rs b/bridges/bin/runtime-common/src/refund_relayer_extension.rs
index ebfc0b33466..83eafa1a36a 100644
--- a/bridges/bin/runtime-common/src/refund_relayer_extension.rs
+++ b/bridges/bin/runtime-common/src/refund_relayer_extension.rs
@@ -46,7 +46,9 @@ use pallet_utility::{Call as UtilityCall, Config as UtilityConfig, Pallet as Uti
use scale_info::TypeInfo;
use sp_runtime::{
traits::{DispatchInfoOf, Get, PostDispatchInfoOf, SignedExtension, Zero},
- transaction_validity::{TransactionValidity, TransactionValidityError, ValidTransaction},
+ transaction_validity::{
+ TransactionPriority, TransactionValidity, TransactionValidityError, ValidTransactionBuilder,
+ },
DispatchResult, FixedPointOperand,
};
use sp_std::{marker::PhantomData, vec, vec::Vec};
@@ -58,7 +60,7 @@ type CallOf<R> = <R as frame_system::Config>::RuntimeCall;
/// Trait identifying a bridged parachain. A relayer might be refunded for delivering messages
/// coming from this parachain.
-trait RefundableParachainId {
+pub trait RefundableParachainId {
/// The instance of the bridge parachains pallet.
type Instance;
/// The parachain Id.
@@ -78,7 +80,7 @@ where
/// Trait identifying a bridged messages lane. A relayer might be refunded for delivering messages
/// coming from this lane.
-trait RefundableMessagesLaneId {
+pub trait RefundableMessagesLaneId {
/// The instance of the bridge messages pallet.
type Instance;
/// The messages lane id.
@@ -201,36 +203,75 @@ impl CallInfo {
RuntimeDebugNoBound,
TypeInfo,
)]
-#[scale_info(skip_type_params(Runtime, Para, Msgs, Refund, Id))]
-pub struct RefundBridgedParachainMessages<Runtime, Para, Msgs, Refund, Id>(
- PhantomData<(Runtime, Para, Msgs, Refund, Id)>,
+#[scale_info(skip_type_params(Runtime, Para, Msgs, Refund, Priority, Id))]
+pub struct RefundBridgedParachainMessages<Runtime, Para, Msgs, Refund, Priority, Id>(
+ PhantomData<(Runtime, Para, Msgs, Refund, Priority, Id)>,
);
-impl<Runtime, Para, Msgs, Refund, Id>
- RefundBridgedParachainMessages<Runtime, Para, Msgs, Refund, Id>
+impl<Runtime, Para, Msgs, Refund, Priority, Id>
+ RefundBridgedParachainMessages<Runtime, Para, Msgs, Refund, Priority, Id>
where
- Runtime: UtilityConfig<RuntimeCall = CallOf<Runtime>>,
- CallOf<Runtime>: IsSubType<CallableCallFor<UtilityPallet<Runtime>, Runtime>>,
+ Self: 'static + Send + Sync,
+ Runtime: UtilityConfig<RuntimeCall = CallOf<Runtime>>
+ + BoundedBridgeGrandpaConfig<Runtime::BridgesGrandpaPalletInstance>
+ + ParachainsConfig<Para::Instance>
+ + MessagesConfig<Msgs::Instance>,
+ Para: RefundableParachainId,
+ Msgs: RefundableMessagesLaneId,
+ CallOf<Runtime>: Dispatchable<Info = DispatchInfo, PostInfo = PostDispatchInfo>
+ + IsSubType<CallableCallFor<UtilityPallet<Runtime>, Runtime>>
+ + GrandpaCallSubType<Runtime, Runtime::BridgesGrandpaPalletInstance>
+ + ParachainsCallSubType<Runtime, Para::Instance>
+ + MessagesCallSubType<Runtime, Msgs::Instance>,
{
- fn expand_call<'a>(&self, call: &'a CallOf<Runtime>) -> Option<Vec<&'a CallOf<Runtime>>> {
- let calls = match call.is_sub_type() {
- Some(UtilityCall::<Runtime>::batch_all { ref calls }) => {
- if calls.len() > 3 {
- return None
- }
-
- calls.iter().collect()
- },
- Some(_) => return None,
+ fn expand_call<'a>(&self, call: &'a CallOf<Runtime>) -> Vec<&'a CallOf<Runtime>> {
+ match call.is_sub_type() {
+ Some(UtilityCall::<Runtime>::batch_all { ref calls }) if calls.len() <= 3 =>
+ calls.iter().collect(),
+ Some(_) => vec![],
None => vec![call],
- };
+ }
+ }
+
+ fn parse_and_check_for_obsolete_call(
+ &self,
+ call: &CallOf<Runtime>,
+ ) -> Result<Option<CallInfo>, TransactionValidityError> {
+ let calls = self.expand_call(call);
+ let total_calls = calls.len();
+ let mut calls = calls.into_iter().map(Self::check_obsolete_call).rev();
+
+ let msgs_call = calls.next().transpose()?.and_then(|c| c.call_info_for(Msgs::Id::get()));
+ let para_finality_call = calls
+ .next()
+ .transpose()?
+ .and_then(|c| c.submit_parachain_heads_info_for(Para::Id::get()));
+ let relay_finality_call =
+ calls.next().transpose()?.and_then(|c| c.submit_finality_proof_info());
+
+ Ok(match (total_calls, relay_finality_call, para_finality_call, msgs_call) {
+ (3, Some(relay_finality_call), Some(para_finality_call), Some(msgs_call)) => Some(
+ CallInfo::AllFinalityAndMsgs(relay_finality_call, para_finality_call, msgs_call),
+ ),
+ (2, None, Some(para_finality_call), Some(msgs_call)) =>
+ Some(CallInfo::ParachainFinalityAndMsgs(para_finality_call, msgs_call)),
+ (1, None, None, Some(msgs_call)) => Some(CallInfo::Msgs(msgs_call)),
+ _ => None,
+ })
+ }
- Some(calls)
+ fn check_obsolete_call(
+ call: &CallOf<Runtime>,
+ ) -> Result<&CallOf<Runtime>, TransactionValidityError> {
+ call.check_obsolete_submit_finality_proof()?;
+ call.check_obsolete_submit_parachain_heads()?;
+ call.check_obsolete_call()?;
+ Ok(call)
}
}
-impl<Runtime, Para, Msgs, Refund, Id> SignedExtension
- for RefundBridgedParachainMessages<Runtime, Para, Msgs, Refund, Id>
+impl<Runtime, Para, Msgs, Refund, Priority, Id> SignedExtension
+ for RefundBridgedParachainMessages<Runtime, Para, Msgs, Refund, Priority, Id>
where
Self: 'static + Send + Sync,
Runtime: UtilityConfig<RuntimeCall = CallOf<Runtime>>
@@ -241,6 +282,7 @@ where
Para: RefundableParachainId,
Msgs: RefundableMessagesLaneId,
Refund: RefundCalculator<Balance = Runtime::Reward>,
+ Priority: Get<TransactionPriority>,
Id: StaticStrProvider,
CallOf<Runtime>: Dispatchable<Info = DispatchInfo, PostInfo = PostDispatchInfo>
+ IsSubType<CallableCallFor<UtilityPallet<Runtime>, Runtime>>
@@ -265,46 +307,46 @@ where
_info: &DispatchInfoOf<Self::Call>,
_len: usize,
) -> TransactionValidity {
- if let Some(calls) = self.expand_call(call) {
- for nested_call in calls {
- nested_call.check_obsolete_submit_finality_proof()?;
- nested_call.check_obsolete_submit_parachain_heads()?;
- nested_call.check_obsolete_call()?;
+ // this is the only relevant line of code for the `pre_dispatch`
+ //
+ // we're not calling `validato` 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 = self.parse_and_check_for_obsolete_call(call)?;
+
+ // the following code just plays with transaction priority and never returns an error
+ let mut valid_transaction = ValidTransactionBuilder::default();
+ if let Some(parsed_call) = parsed_call {
+ // we give delivery transactions some boost, that depends on number of messages inside
+ let messages_call_info = parsed_call.messages_call_info();
+ if let MessagesCallInfo::ReceiveMessagesProof(_) = messages_call_info {
+ // compute total number of messages in transaction
+ let bundled_messages = messages_call_info.bundled_messages();
+
+ // a quick check to avoid invalid high-priority transactions
+ if bundled_messages <= Runtime::MaxUnconfirmedMessagesAtInboundLane::get() {
+ let priority_boost = crate::priority_calculator::compute_priority_boost::<
+ Priority,
+ >(bundled_messages);
+ valid_transaction = valid_transaction.priority(priority_boost);
+ }
}
}
- Ok(ValidTransaction::default())
+ valid_transaction.build()
}
fn pre_dispatch(
self,
who: &Self::AccountId,
call: &Self::Call,
- info: &DispatchInfoOf<Self::Call>,
- len: usize,
+ _info: &DispatchInfoOf<Self::Call>,
+ _len: usize,
) -> Result<Self::Pre, TransactionValidityError> {
- // reject batch transactions with obsolete headers
- self.validate(who, call, info, len).map(drop)?;
-
- // Try to check if the tx matches one of types we support.
- let parse_call = || {
- let mut calls = self.expand_call(call)?.into_iter();
- match calls.len() {
- 3 => Some(CallInfo::AllFinalityAndMsgs(
- calls.next()?.submit_finality_proof_info()?,
- calls.next()?.submit_parachain_heads_info_for(Para::Id::get())?,
- calls.next()?.call_info_for(Msgs::Id::get())?,
- )),
- 2 => Some(CallInfo::ParachainFinalityAndMsgs(
- calls.next()?.submit_parachain_heads_info_for(Para::Id::get())?,
- calls.next()?.call_info_for(Msgs::Id::get())?,
- )),
- 1 => Some(CallInfo::Msgs(calls.next()?.call_info_for(Msgs::Id::get())?)),
- _ => None,
- }
- };
+ // this is a relevant piece of `validate` that we need here (in `pre_dispatch`)
+ let parsed_call = self.parse_and_check_for_obsolete_call(call)?;
- Ok(parse_call().map(|call_info| {
+ Ok(parsed_call.map(|call_info| {
log::trace!(
target: "runtime::bridge",
"{} from parachain {} via {:?} parsed bridge transaction in pre-dispatch: {:?}",
@@ -475,14 +517,24 @@ mod tests {
use pallet_bridge_messages::Call as MessagesCall;
use pallet_bridge_parachains::{Call as ParachainsCall, RelayBlockHash};
use sp_runtime::{
- traits::Header as HeaderT, transaction_validity::InvalidTransaction, DispatchError,
+ traits::{ConstU64, Header as HeaderT},
+ transaction_validity::{InvalidTransaction, ValidTransaction},
+ DispatchError,
};
parameter_types! {
TestParachain: u32 = 1000;
pub TestLaneId: LaneId = TEST_LANE_ID;
- pub MsgProofsRewardsAccount: RewardsAccountParams = RewardsAccountParams::new(TEST_LANE_ID, TEST_BRIDGED_CHAIN_ID, RewardsAccountOwner::ThisChain);
- pub MsgDeliveryProofsRewardsAccount: RewardsAccountParams = RewardsAccountParams::new(TEST_LANE_ID, TEST_BRIDGED_CHAIN_ID, RewardsAccountOwner::BridgedChain);
+ pub MsgProofsRewardsAccount: RewardsAccountParams = RewardsAccountParams::new(
+ TEST_LANE_ID,
+ TEST_BRIDGED_CHAIN_ID,
+ RewardsAccountOwner::ThisChain,
+ );
+ pub MsgDeliveryProofsRewardsAccount: RewardsAccountParams = RewardsAccountParams::new(
+ TEST_LANE_ID,
+ TEST_BRIDGED_CHAIN_ID,
+ RewardsAccountOwner::BridgedChain,
+ );
}
bp_runtime::generate_static_str_provider!(TestExtension);
@@ -491,6 +543,7 @@ mod tests {
RefundableParachain<(), TestParachain>,
RefundableMessagesLane<(), TestLaneId>,
ActualFeeRefund<TestRuntime>,
+ ConstU64<1>,
StrTestExtension,
>;
@@ -831,32 +884,98 @@ mod tests {
)
}
+ #[test]
+ fn validate_boosts_priority_of_message_delivery_transactons() {
+ run_test(|| {
+ initialize_environment(100, 100, Default::default(), 100);
+
+ let priority_of_100_messages_delivery =
+ run_validate(message_delivery_call(200)).unwrap().priority;
+ let priority_of_200_messages_delivery =
+ run_validate(message_delivery_call(300)).unwrap().priority;
+ assert!(
+ priority_of_200_messages_delivery > priority_of_100_messages_delivery,
+ "Invalid priorities: {} for 200 messages vs {} for 100 messages",
+ priority_of_200_messages_delivery,
+ priority_of_100_messages_delivery,
+ );
+
+ let priority_of_100_messages_confirmation =
+ run_validate(message_confirmation_call(200)).unwrap().priority;
+ let priority_of_200_messages_confirmation =
+ run_validate(message_confirmation_call(300)).unwrap().priority;
+ assert_eq!(
+ priority_of_100_messages_confirmation,
+ priority_of_200_messages_confirmation
+ );
+ });
+ }
+
+ #[test]
+ fn validate_does_not_boost_priority_of_message_delivery_transactons_with_too_many_messages() {
+ run_test(|| {
+ initialize_environment(100, 100, Default::default(), 100);
+
+ let priority_of_max_messages_delivery = run_validate(message_delivery_call(
+ 100 + MaxUnconfirmedMessagesAtInboundLane::get(),
+ ))
+ .unwrap()
+ .priority;
+ let priority_of_more_than_max_messages_delivery = run_validate(message_delivery_call(
+ 100 + MaxUnconfirmedMessagesAtInboundLane::get() + 1,
+ ))
+ .unwrap()
+ .priority;
+
+ assert!(
+ priority_of_max_messages_delivery > priority_of_more_than_max_messages_delivery,
+ "Invalid priorities: {} for MAX messages vs {} for MAX+1 messages",
+ priority_of_max_messages_delivery,
+ priority_of_more_than_max_messages_delivery,
+ );
+ });
+ }
+
#[test]
fn validate_allows_non_obsolete_transactions() {
run_test(|| {
initialize_environment(100, 100, Default::default(), 100);
- assert_eq!(run_validate(message_delivery_call(200)), Ok(ValidTransaction::default()),);
+ fn run_validate_ignore_priority(call: RuntimeCall) -> TransactionValidity {
+ run_validate(call).map(|mut tx| {
+ tx.priority = 0;
+ tx
+ })
+ }
+
assert_eq!(
- run_validate(message_confirmation_call(200)),
+ run_validate_ignore_priority(message_delivery_call(200)),
+ Ok(ValidTransaction::default()),
+ );
+ assert_eq!(
+ run_validate_ignore_priority(message_confirmation_call(200)),
Ok(ValidTransaction::default()),
);
assert_eq!(
- run_validate(parachain_finality_and_delivery_batch_call(200, 200)),
+ run_validate_ignore_priority(parachain_finality_and_delivery_batch_call(200, 200)),
Ok(ValidTransaction::default()),
);
assert_eq!(
- run_validate(parachain_finality_and_confirmation_batch_call(200, 200)),
+ run_validate_ignore_priority(parachain_finality_and_confirmation_batch_call(
+ 200, 200
+ )),
Ok(ValidTransaction::default()),
);
assert_eq!(
- run_validate(all_finality_and_delivery_batch_call(200, 200, 200)),
+ run_validate_ignore_priority(all_finality_and_delivery_batch_call(200, 200, 200)),
Ok(ValidTransaction::default()),
);
assert_eq!(
- run_validate(all_finality_and_confirmation_batch_call(200, 200, 200)),
+ run_validate_ignore_priority(all_finality_and_confirmation_batch_call(
+ 200, 200, 200
+ )),
Ok(ValidTransaction::default()),
);
});
--
GitLab