Skip to content

Commit

Permalink
Boost message delivery transaction priority (paritytech#2023)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
svyatonik authored Apr 10, 2023
1 parent 06d5c75 commit 2781ed3
Show file tree
Hide file tree
Showing 9 changed files with 481 additions and 65 deletions.
2 changes: 1 addition & 1 deletion bridges/bin/millau/runtime/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
2 changes: 2 additions & 0 deletions bridges/bin/millau/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
>;

Expand Down
70 changes: 70 additions & 0 deletions bridges/bin/millau/runtime/src/rialto_parachain_messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
1 change: 1 addition & 0 deletions bridges/bin/runtime-common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
1 change: 1 addition & 0 deletions bridges/bin/runtime-common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
24 changes: 23 additions & 1 deletion bridges/bin/runtime-common/src/messages_call_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)>,
Expand Down Expand Up @@ -321,6 +337,7 @@ mod tests {
TestRuntime, ThisChainRuntimeCall,
},
};
use bitvec::prelude::*;
use bp_messages::{DeliveredMessages, UnrewardedRelayer, UnrewardedRelayersState};
use sp_std::ops::RangeInclusive;

Expand All @@ -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(
Expand All @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion bridges/bin/runtime-common/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
201 changes: 201 additions & 0 deletions bridges/bin/runtime-common/src/priority_calculator.rs
Original file line number Diff line number Diff line change
@@ -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(),
)
}
}
Loading

0 comments on commit 2781ed3

Please sign in to comment.