Skip to content

Commit 77f1641

Browse files
authored
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
1 parent c23c4e4 commit 77f1641

File tree

10 files changed

+482
-65
lines changed

10 files changed

+482
-65
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

bin/millau/runtime/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ xcm-builder = { git = "https://github.com/paritytech/polkadot", branch = "master
6969
xcm-executor = { git = "https://github.com/paritytech/polkadot", branch = "master", default-features = false }
7070

7171
[dev-dependencies]
72-
bridge-runtime-common = { path = "../../runtime-common", features = ["integrity-test"] }
72+
bridge-runtime-common = { path = "../../runtime-common", features = ["integrity-test", "std"] }
7373
env_logger = "0.10"
7474
static_assertions = "1.1"
7575

bin/millau/runtime/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -589,11 +589,13 @@ generate_bridge_reject_obsolete_headers_and_messages! {
589589

590590
bp_runtime::generate_static_str_provider!(BridgeRefundRialtoPara2000Lane0Msgs);
591591
/// Signed extension that refunds relayers that are delivering messages from the Rialto parachain.
592+
pub type PriorityBoostPerMessage = ConstU64<921_900_294>;
592593
pub type BridgeRefundRialtoParachainMessages = RefundBridgedParachainMessages<
593594
Runtime,
594595
RefundableParachain<WithRialtoParachainsInstance, RialtoParachainId>,
595596
RefundableMessagesLane<WithRialtoParachainMessagesInstance, RialtoParachainMessagesLane>,
596597
ActualFeeRefund<Runtime>,
598+
PriorityBoostPerMessage,
597599
StrBridgeRefundRialtoPara2000Lane0Msgs,
598600
>;
599601

bin/millau/runtime/src/rialto_parachain_messages.rs

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,3 +137,73 @@ impl XcmBlobHauler for ToRialtoParachainXcmBlobHauler {
137137
XCM_LANE
138138
}
139139
}
140+
141+
#[cfg(test)]
142+
mod tests {
143+
use super::*;
144+
use crate::{
145+
PriorityBoostPerMessage, RialtoGrandpaInstance, Runtime,
146+
WithRialtoParachainMessagesInstance,
147+
};
148+
149+
use bridge_runtime_common::{
150+
assert_complete_bridge_types,
151+
integrity::{
152+
assert_complete_bridge_constants, check_message_lane_weights,
153+
AssertBridgeMessagesPalletConstants, AssertBridgePalletNames, AssertChainConstants,
154+
AssertCompleteBridgeConstants,
155+
},
156+
};
157+
158+
#[test]
159+
fn ensure_millau_message_lane_weights_are_correct() {
160+
check_message_lane_weights::<bp_millau::Millau, Runtime>(
161+
bp_rialto_parachain::EXTRA_STORAGE_PROOF_SIZE,
162+
bp_millau::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX,
163+
bp_millau::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX,
164+
);
165+
}
166+
167+
#[test]
168+
fn ensure_bridge_integrity() {
169+
assert_complete_bridge_types!(
170+
runtime: Runtime,
171+
with_bridged_chain_grandpa_instance: RialtoGrandpaInstance,
172+
with_bridged_chain_messages_instance: WithRialtoParachainMessagesInstance,
173+
bridge: WithRialtoParachainMessageBridge,
174+
this_chain: bp_millau::Millau,
175+
bridged_chain: bp_rialto::Rialto,
176+
);
177+
178+
assert_complete_bridge_constants::<
179+
Runtime,
180+
RialtoGrandpaInstance,
181+
WithRialtoParachainMessagesInstance,
182+
WithRialtoParachainMessageBridge,
183+
>(AssertCompleteBridgeConstants {
184+
this_chain_constants: AssertChainConstants {
185+
block_length: bp_millau::BlockLength::get(),
186+
block_weights: bp_millau::BlockWeights::get(),
187+
},
188+
messages_pallet_constants: AssertBridgeMessagesPalletConstants {
189+
max_unrewarded_relayers_in_bridged_confirmation_tx:
190+
bp_rialto_parachain::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX,
191+
max_unconfirmed_messages_in_bridged_confirmation_tx:
192+
bp_rialto_parachain::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX,
193+
bridged_chain_id: bp_runtime::RIALTO_PARACHAIN_CHAIN_ID,
194+
},
195+
pallet_names: AssertBridgePalletNames {
196+
with_this_chain_messages_pallet_name: bp_millau::WITH_MILLAU_MESSAGES_PALLET_NAME,
197+
with_bridged_chain_grandpa_pallet_name: bp_rialto::WITH_RIALTO_GRANDPA_PALLET_NAME,
198+
with_bridged_chain_messages_pallet_name:
199+
bp_rialto_parachain::WITH_RIALTO_PARACHAIN_MESSAGES_PALLET_NAME,
200+
},
201+
});
202+
203+
bridge_runtime_common::priority_calculator::ensure_priority_boost_is_sane::<
204+
Runtime,
205+
WithRialtoParachainMessagesInstance,
206+
PriorityBoostPerMessage,
207+
>(1_000_000);
208+
}
209+
}

bin/runtime-common/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ xcm-executor = { git = "https://github.com/paritytech/polkadot", branch = "maste
4747

4848
[dev-dependencies]
4949
bp-test-utils = { path = "../../primitives/test-utils" }
50+
bitvec = { version = "1", features = ["alloc"] }
5051
pallet-balances = { git = "https://github.com/paritytech/substrate", branch = "master" }
5152

5253
[features]

bin/runtime-common/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ pub mod messages_benchmarking;
3030
pub mod messages_call_ext;
3131
pub mod messages_xcm_extension;
3232
pub mod parachains_benchmarking;
33+
pub mod priority_calculator;
3334
pub mod refund_relayer_extension;
3435

3536
mod messages_generation;

bin/runtime-common/src/messages_call_ext.rs

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,22 @@ pub enum CallInfo {
104104
ReceiveMessagesDeliveryProof(ReceiveMessagesDeliveryProofInfo),
105105
}
106106

107+
impl CallInfo {
108+
/// Returns number of messages, bundled with this transaction.
109+
pub fn bundled_messages(&self) -> MessageNonce {
110+
let bundled_range = match *self {
111+
Self::ReceiveMessagesProof(ref info) => &info.0.bundled_range,
112+
Self::ReceiveMessagesDeliveryProof(ref info) => &info.0.bundled_range,
113+
};
114+
115+
bundled_range
116+
.end()
117+
.checked_sub(*bundled_range.start())
118+
.map(|d| d.saturating_add(1))
119+
.unwrap_or(0)
120+
}
121+
}
122+
107123
/// Helper struct that provides methods for working with a call supported by `CallInfo`.
108124
pub struct CallHelper<T: Config<I>, I: 'static> {
109125
pub _phantom_data: sp_std::marker::PhantomData<(T, I)>,
@@ -321,6 +337,7 @@ mod tests {
321337
TestRuntime, ThisChainRuntimeCall,
322338
},
323339
};
340+
use bitvec::prelude::*;
324341
use bp_messages::{DeliveredMessages, UnrewardedRelayer, UnrewardedRelayersState};
325342
use sp_std::ops::RangeInclusive;
326343

@@ -330,7 +347,11 @@ mod tests {
330347
for n in 0..MaxUnrewardedRelayerEntriesAtInboundLane::get() {
331348
inbound_lane_state.relayers.push_back(UnrewardedRelayer {
332349
relayer: Default::default(),
333-
messages: DeliveredMessages { begin: n + 1, end: n + 1 },
350+
messages: DeliveredMessages {
351+
begin: n + 1,
352+
end: n + 1,
353+
dispatch_results: bitvec![u8, Msb0; 1; 1],
354+
},
334355
});
335356
}
336357
pallet_bridge_messages::InboundLanes::<TestRuntime>::insert(
@@ -347,6 +368,7 @@ mod tests {
347368
messages: DeliveredMessages {
348369
begin: 1,
349370
end: MaxUnconfirmedMessagesAtInboundLane::get(),
371+
dispatch_results: bitvec![u8, Msb0; 1; MaxUnconfirmedMessagesAtInboundLane::get() as _],
350372
},
351373
});
352374
pallet_bridge_messages::InboundLanes::<TestRuntime>::insert(

bin/runtime-common/src/mock.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ parameter_types! {
127127
pub MinimumMultiplier: Multiplier = Multiplier::saturating_from_rational(1, 1_000_000u128);
128128
pub MaximumMultiplier: Multiplier = sp_runtime::traits::Bounded::max_value();
129129
pub const MaxUnrewardedRelayerEntriesAtInboundLane: MessageNonce = 16;
130-
pub const MaxUnconfirmedMessagesAtInboundLane: MessageNonce = 32;
130+
pub const MaxUnconfirmedMessagesAtInboundLane: MessageNonce = 1_000;
131131
}
132132

133133
impl frame_system::Config for TestRuntime {
Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
1+
// Copyright 2021 Parity Technologies (UK) Ltd.
2+
// This file is part of Parity Bridges Common.
3+
4+
// Parity Bridges Common is free software: you can redistribute it and/or modify
5+
// it under the terms of the GNU General Public License as published by
6+
// the Free Software Foundation, either version 3 of the License, or
7+
// (at your option) any later version.
8+
9+
// Parity Bridges Common is distributed in the hope that it will be useful,
10+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
// GNU General Public License for more details.
13+
14+
// You should have received a copy of the GNU General Public License
15+
// along with Parity Bridges Common. If not, see <http://www.gnu.org/licenses/>.
16+
17+
//! Bridge transaction priority calculator.
18+
//!
19+
//! We want to prioritize message delivery transactions with more messages over
20+
//! transactions with less messages. That's because we reject delivery transactions
21+
//! if it contains already delivered message. And if some transaction delivers
22+
//! single message with nonce `N`, then the transaction with nonces `N..=N+100` will
23+
//! be rejected. This can lower bridge throughput down to one message per block.
24+
25+
use bp_messages::MessageNonce;
26+
use frame_support::traits::Get;
27+
use sp_runtime::transaction_validity::TransactionPriority;
28+
29+
// reexport everything from `integrity_tests` module
30+
pub use integrity_tests::*;
31+
32+
/// Compute priority boost for message delivery transaction that delivers
33+
/// given number of messages.
34+
pub fn compute_priority_boost<PriorityBoostPerMessage>(
35+
messages: MessageNonce,
36+
) -> TransactionPriority
37+
where
38+
PriorityBoostPerMessage: Get<TransactionPriority>,
39+
{
40+
// we don't want any boost for transaction with single message => minus one
41+
PriorityBoostPerMessage::get().saturating_mul(messages - 1)
42+
}
43+
44+
#[cfg(not(feature = "integrity-test"))]
45+
mod integrity_tests {}
46+
47+
#[cfg(feature = "integrity-test")]
48+
mod integrity_tests {
49+
use super::compute_priority_boost;
50+
51+
use bp_messages::MessageNonce;
52+
use bp_runtime::PreComputedSize;
53+
use frame_support::{
54+
dispatch::{DispatchClass, DispatchInfo, Dispatchable, Pays, PostDispatchInfo},
55+
traits::Get,
56+
};
57+
use pallet_bridge_messages::WeightInfoExt;
58+
use pallet_transaction_payment::OnChargeTransaction;
59+
use sp_runtime::{
60+
traits::{UniqueSaturatedInto, Zero},
61+
transaction_validity::TransactionPriority,
62+
FixedPointOperand, SaturatedConversion, Saturating,
63+
};
64+
65+
type BalanceOf<T> =
66+
<<T as pallet_transaction_payment::Config>::OnChargeTransaction as OnChargeTransaction<
67+
T,
68+
>>::Balance;
69+
70+
/// Ensures that the value of `PriorityBoostPerMessage` matches the value of
71+
/// `tip_boost_per_message`.
72+
///
73+
/// We want two transactions, `TX1` with `N` messages and `TX2` with `N+1` messages, have almost
74+
/// the same priority if we'll add `tip_boost_per_message` tip to the `TX1`. We want to be sure
75+
/// that if we add plain `PriorityBoostPerMessage` priority to `TX1`, the priority will be close
76+
/// to `TX2` as well.
77+
pub fn ensure_priority_boost_is_sane<Runtime, MessagesInstance, PriorityBoostPerMessage>(
78+
tip_boost_per_message: BalanceOf<Runtime>,
79+
) where
80+
Runtime:
81+
pallet_transaction_payment::Config + pallet_bridge_messages::Config<MessagesInstance>,
82+
MessagesInstance: 'static,
83+
PriorityBoostPerMessage: Get<TransactionPriority>,
84+
Runtime::RuntimeCall: Dispatchable<Info = DispatchInfo, PostInfo = PostDispatchInfo>,
85+
BalanceOf<Runtime>: Send + Sync + FixedPointOperand,
86+
{
87+
let priority_boost_per_message = PriorityBoostPerMessage::get();
88+
let maximal_messages_in_delivery_transaction =
89+
Runtime::MaxUnconfirmedMessagesAtInboundLane::get();
90+
for messages in 1..=maximal_messages_in_delivery_transaction {
91+
let base_priority = estimate_message_delivery_transaction_priority::<
92+
Runtime,
93+
MessagesInstance,
94+
>(messages, Zero::zero());
95+
let priority_boost = compute_priority_boost::<PriorityBoostPerMessage>(messages);
96+
let priority_with_boost = base_priority + priority_boost;
97+
98+
let tip = tip_boost_per_message.saturating_mul((messages - 1).unique_saturated_into());
99+
let priority_with_tip =
100+
estimate_message_delivery_transaction_priority::<Runtime, MessagesInstance>(1, tip);
101+
102+
const ERROR_MARGIN: TransactionPriority = 5; // 5%
103+
if priority_with_boost.abs_diff(priority_with_tip).saturating_mul(100) /
104+
priority_with_tip >
105+
ERROR_MARGIN
106+
{
107+
panic!(
108+
"The PriorityBoostPerMessage value ({}) must be fixed to: {}",
109+
priority_boost_per_message,
110+
compute_priority_boost_per_message::<Runtime, MessagesInstance>(
111+
tip_boost_per_message
112+
),
113+
);
114+
}
115+
}
116+
}
117+
118+
/// Compute priority boost that we give to message delivery transaction for additional message.
119+
#[cfg(feature = "integrity-test")]
120+
fn compute_priority_boost_per_message<Runtime, MessagesInstance>(
121+
tip_boost_per_message: BalanceOf<Runtime>,
122+
) -> TransactionPriority
123+
where
124+
Runtime:
125+
pallet_transaction_payment::Config + pallet_bridge_messages::Config<MessagesInstance>,
126+
MessagesInstance: 'static,
127+
Runtime::RuntimeCall: Dispatchable<Info = DispatchInfo, PostInfo = PostDispatchInfo>,
128+
BalanceOf<Runtime>: Send + Sync + FixedPointOperand,
129+
{
130+
// esimate priority of transaction that delivers one message and has large tip
131+
let maximal_messages_in_delivery_transaction =
132+
Runtime::MaxUnconfirmedMessagesAtInboundLane::get();
133+
let small_with_tip_priority =
134+
estimate_message_delivery_transaction_priority::<Runtime, MessagesInstance>(
135+
1,
136+
tip_boost_per_message
137+
.saturating_mul(maximal_messages_in_delivery_transaction.saturated_into()),
138+
);
139+
// estimate priority of transaction that delivers maximal number of messages, but has no tip
140+
let large_without_tip_priority = estimate_message_delivery_transaction_priority::<
141+
Runtime,
142+
MessagesInstance,
143+
>(maximal_messages_in_delivery_transaction, Zero::zero());
144+
145+
small_with_tip_priority
146+
.saturating_sub(large_without_tip_priority)
147+
.saturating_div(maximal_messages_in_delivery_transaction - 1)
148+
}
149+
150+
/// Estimate message delivery transaction priority.
151+
#[cfg(feature = "integrity-test")]
152+
fn estimate_message_delivery_transaction_priority<Runtime, MessagesInstance>(
153+
messages: MessageNonce,
154+
tip: BalanceOf<Runtime>,
155+
) -> TransactionPriority
156+
where
157+
Runtime:
158+
pallet_transaction_payment::Config + pallet_bridge_messages::Config<MessagesInstance>,
159+
MessagesInstance: 'static,
160+
Runtime::RuntimeCall: Dispatchable<Info = DispatchInfo, PostInfo = PostDispatchInfo>,
161+
BalanceOf<Runtime>: Send + Sync + FixedPointOperand,
162+
{
163+
// just an estimation of extra transaction bytes that are added to every transaction
164+
// (including signature, signed extensions extra and etc + in our case it includes
165+
// all call arguments extept the proof itself)
166+
let base_tx_size = 512;
167+
// let's say we are relaying similar small messages and for every message we add more trie
168+
// nodes to the proof (x0.5 because we expect some nodes to be reused)
169+
let estimated_message_size = 512;
170+
// let's say all our messages have the same dispatch weight
171+
let estimated_message_dispatch_weight =
172+
Runtime::WeightInfo::message_dispatch_weight(estimated_message_size);
173+
// messages proof argument size is (for every message) messages size + some additional
174+
// trie nodes. Some of them are reused by different messages, so let's take 2/3 of default
175+
// "overhead" constant
176+
let messages_proof_size = Runtime::WeightInfo::expected_extra_storage_proof_size()
177+
.saturating_mul(2)
178+
.saturating_div(3)
179+
.saturating_add(estimated_message_size)
180+
.saturating_mul(messages as _);
181+
182+
// finally we are able to estimate transaction size and weight
183+
let transaction_size = base_tx_size.saturating_add(messages_proof_size);
184+
let transaction_weight = Runtime::WeightInfo::receive_messages_proof_weight(
185+
&PreComputedSize(transaction_size as _),
186+
messages as _,
187+
estimated_message_dispatch_weight.saturating_mul(messages),
188+
);
189+
190+
pallet_transaction_payment::ChargeTransactionPayment::<Runtime>::get_priority(
191+
&DispatchInfo {
192+
weight: transaction_weight,
193+
class: DispatchClass::Normal,
194+
pays_fee: Pays::Yes,
195+
},
196+
transaction_size as _,
197+
tip,
198+
Zero::zero(),
199+
)
200+
}
201+
}

0 commit comments

Comments
 (0)