Skip to content

Commit 7f14e66

Browse files
serban300serban300
committed
FeeTracker deduplications
1 parent 4b83d24 commit 7f14e66

File tree

8 files changed

+118
-105
lines changed

8 files changed

+118
-105
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.

‎bridges/modules/xcm-bridge-hub-router/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ sp-std = { workspace = true }
3030
xcm = { workspace = true }
3131
xcm-builder = { workspace = true }
3232

33+
polkadot-runtime-parachains = { workspace = true }
34+
3335
[dev-dependencies]
3436
sp-io = { workspace = true, default-features = true }
3537

‎bridges/modules/xcm-bridge-hub-router/src/lib.rs

Lines changed: 28 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,9 @@
3333
pub use bp_xcm_bridge_hub_router::{BridgeState, XcmChannelStatusProvider};
3434
use codec::Encode;
3535
use frame_support::traits::Get;
36+
use polkadot_runtime_parachains::FeeTracker;
3637
use sp_core::H256;
37-
use sp_runtime::{FixedPointNumber, FixedU128, Saturating};
38+
use sp_runtime::{FixedPointNumber, FixedU128};
3839
use sp_std::vec::Vec;
3940
use xcm::prelude::*;
4041
use xcm_builder::{ExporterFor, InspectMessageQueues, SovereignPaidRemoteExporter};
@@ -50,12 +51,6 @@ mod mock;
5051
/// Minimal delivery fee factor.
5152
pub const MINIMAL_DELIVERY_FEE_FACTOR: FixedU128 = FixedU128::from_u32(1);
5253

53-
/// The factor that is used to increase current message fee factor when bridge experiencing
54-
/// some lags.
55-
const EXPONENTIAL_FEE_BASE: FixedU128 = FixedU128::from_rational(105, 100); // 1.05
56-
/// The factor that is used to increase current message fee factor for every sent kilobyte.
57-
const MESSAGE_SIZE_FEE_BASE: FixedU128 = FixedU128::from_rational(1, 1000); // 0.001
58-
5954
/// Maximal size of the XCM message that may be sent over bridge.
6055
///
6156
/// This should be less than the maximal size, allowed by the messages pallet, because
@@ -130,15 +125,12 @@ pub mod pallet {
130125
return T::WeightInfo::on_initialize_when_congested();
131126
}
132127

128+
let previous_factor = Self::get_fee_factor(());
133129
// if we can't decrease the delivery fee factor anymore, we don't change anything
134-
if bridge.delivery_fee_factor == MINIMAL_DELIVERY_FEE_FACTOR {
130+
if !Self::do_decrease_fee_factor(&mut bridge.delivery_fee_factor) {
135131
return T::WeightInfo::on_initialize_when_congested();
136132
}
137133

138-
let previous_factor = bridge.delivery_fee_factor;
139-
bridge.delivery_fee_factor =
140-
MINIMAL_DELIVERY_FEE_FACTOR.max(bridge.delivery_fee_factor / EXPONENTIAL_FEE_BASE);
141-
142134
log::info!(
143135
target: LOG_TARGET,
144136
"Bridge channel is uncongested. Decreased fee factor from {} to {}",
@@ -216,13 +208,12 @@ pub mod pallet {
216208
return Err(());
217209
}
218210

211+
let previous_factor = Self::get_fee_factor(());
219212
// ok - we need to increase the fee factor, let's do that
220-
let message_size_factor = FixedU128::from_u32(message_size.saturating_div(1024))
221-
.saturating_mul(MESSAGE_SIZE_FEE_BASE);
222-
let total_factor = EXPONENTIAL_FEE_BASE.saturating_add(message_size_factor);
223-
let previous_factor = bridge.delivery_fee_factor;
224-
bridge.delivery_fee_factor =
225-
bridge.delivery_fee_factor.saturating_mul(total_factor);
213+
<Self as FeeTracker>::do_increase_fee_factor(
214+
&mut bridge.delivery_fee_factor,
215+
message_size as u128,
216+
);
226217

227218
log::info!(
228219
target: LOG_TARGET,
@@ -335,7 +326,7 @@ impl<T: Config<I>, I: 'static> ExporterFor for Pallet<T, I> {
335326
let message_size = message.encoded_size();
336327
let message_fee = (message_size as u128).saturating_mul(T::ByteFee::get());
337328
let fee_sum = base_fee.saturating_add(message_fee);
338-
let fee_factor = Self::bridge().delivery_fee_factor;
329+
let fee_factor = Self::get_fee_factor(());
339330
let fee = fee_factor.saturating_mul_int(fee_sum);
340331

341332
let fee = if fee > 0 { Some((T::FeeAsset::get(), fee).into()) } else { None };
@@ -442,6 +433,24 @@ impl<T: Config<I>, I: 'static> InspectMessageQueues for Pallet<T, I> {
442433
}
443434
}
444435

436+
impl<T: Config<I>, I: 'static> FeeTracker for Pallet<T, I> {
437+
type Id = ();
438+
439+
fn get_min_fee_factor() -> FixedU128 {
440+
MINIMAL_DELIVERY_FEE_FACTOR
441+
}
442+
443+
fn get_fee_factor(_id: Self::Id) -> FixedU128 {
444+
Self::bridge().delivery_fee_factor
445+
}
446+
447+
fn set_fee_factor(_id: Self::Id, val: FixedU128) {
448+
let mut bridge = Self::bridge();
449+
bridge.delivery_fee_factor = val;
450+
Bridge::<T, I>::put(bridge);
451+
}
452+
}
453+
445454
#[cfg(test)]
446455
mod tests {
447456
use super::*;

‎cumulus/pallets/parachain-system/src/lib.rs

Lines changed: 8 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ use polkadot_runtime_parachains::FeeTracker;
5757
use scale_info::TypeInfo;
5858
use sp_runtime::{
5959
traits::{Block as BlockT, BlockNumberProvider, Hash, One},
60-
BoundedSlice, FixedU128, RuntimeDebug, Saturating,
60+
BoundedSlice, FixedU128, RuntimeDebug,
6161
};
6262
use xcm::{latest::XcmHash, VersionedLocation, VersionedXcm, MAX_XCM_DECODE_DEPTH};
6363
use xcm_builder::InspectMessageQueues;
@@ -177,17 +177,10 @@ impl CheckAssociatedRelayNumber for RelayNumberMonotonicallyIncreases {
177177
pub type MaxDmpMessageLenOf<T> = <<T as Config>::DmpQueue as HandleMessage>::MaxMessageLen;
178178

179179
pub mod ump_constants {
180-
use super::FixedU128;
181-
182180
/// `host_config.max_upward_queue_size / THRESHOLD_FACTOR` is the threshold after which delivery
183181
/// starts getting exponentially more expensive.
184182
/// `2` means the price starts to increase when queue is half full.
185183
pub const THRESHOLD_FACTOR: u32 = 2;
186-
/// The base number the delivery fee factor gets multiplied by every time it is increased.
187-
/// Also the number it gets divided by when decreased.
188-
pub const EXPONENTIAL_FEE_BASE: FixedU128 = FixedU128::from_rational(105, 100); // 1.05
189-
/// The base number message size in KB is multiplied by before increasing the fee factor.
190-
pub const MESSAGE_SIZE_FEE_BASE: FixedU128 = FixedU128::from_rational(1, 1000); // 0.001
191184
}
192185

193186
/// Trait for selecting the next core to build the candidate for.
@@ -1015,25 +1008,16 @@ impl<T: Config> Pallet<T> {
10151008
impl<T: Config> FeeTracker for Pallet<T> {
10161009
type Id = ();
10171010

1018-
fn get_fee_factor(_: Self::Id) -> FixedU128 {
1019-
UpwardDeliveryFeeFactor::<T>::get()
1011+
fn get_min_fee_factor() -> FixedU128 {
1012+
UpwardInitialDeliveryFeeFactor::get()
10201013
}
10211014

1022-
fn increase_fee_factor(_: Self::Id, message_size_factor: FixedU128) -> FixedU128 {
1023-
<UpwardDeliveryFeeFactor<T>>::mutate(|f| {
1024-
*f = f.saturating_mul(
1025-
ump_constants::EXPONENTIAL_FEE_BASE.saturating_add(message_size_factor),
1026-
);
1027-
*f
1028-
})
1015+
fn get_fee_factor(_id: Self::Id) -> FixedU128 {
1016+
UpwardDeliveryFeeFactor::<T>::get()
10291017
}
10301018

1031-
fn decrease_fee_factor(_: Self::Id) -> FixedU128 {
1032-
<UpwardDeliveryFeeFactor<T>>::mutate(|f| {
1033-
*f =
1034-
UpwardInitialDeliveryFeeFactor::get().max(*f / ump_constants::EXPONENTIAL_FEE_BASE);
1035-
*f
1036-
})
1019+
fn set_fee_factor(_id: Self::Id, val: FixedU128) {
1020+
UpwardDeliveryFeeFactor::<T>::set(val);
10371021
}
10381022
}
10391023

@@ -1613,9 +1597,7 @@ impl<T: Config> Pallet<T> {
16131597
let total_size: usize = pending_messages.iter().map(UpwardMessage::len).sum();
16141598
if total_size > threshold as usize {
16151599
// We increase the fee factor by a factor based on the new message's size in KB
1616-
let message_size_factor = FixedU128::from((message_len / 1024) as u128)
1617-
.saturating_mul(ump_constants::MESSAGE_SIZE_FEE_BASE);
1618-
Self::increase_fee_factor((), message_size_factor);
1600+
Self::increase_fee_factor((), message_len as u128);
16191601
}
16201602
} else {
16211603
// This storage field should carry over from the previous block. So if it's None

‎cumulus/pallets/xcmp-queue/src/lib.rs

Lines changed: 21 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ use polkadot_runtime_common::xcm_sender::PriceForMessageDelivery;
7979
use polkadot_runtime_parachains::FeeTracker;
8080
use scale_info::TypeInfo;
8181
use sp_core::MAX_POSSIBLE_ALLOCATION;
82-
use sp_runtime::{FixedU128, RuntimeDebug, Saturating, WeakBoundedVec};
82+
use sp_runtime::{FixedU128, RuntimeDebug, WeakBoundedVec};
8383
use xcm::{latest::prelude::*, VersionedLocation, VersionedXcm, WrapVersion, MAX_XCM_DECODE_DEPTH};
8484
use xcm_builder::InspectMessageQueues;
8585
use xcm_executor::traits::ConvertOrigin;
@@ -99,15 +99,8 @@ pub const XCM_BATCH_SIZE: usize = 250;
9999

100100
/// Constants related to delivery fee calculation
101101
pub mod delivery_fee_constants {
102-
use super::FixedU128;
103-
104102
/// Fees will start increasing when queue is half full
105103
pub const THRESHOLD_FACTOR: u32 = 2;
106-
/// The base number the delivery fee factor gets multiplied by every time it is increased.
107-
/// Also, the number it gets divided by when decreased.
108-
pub const EXPONENTIAL_FEE_BASE: FixedU128 = FixedU128::from_rational(105, 100); // 1.05
109-
/// The contribution of each KB to a fee factor increase
110-
pub const MESSAGE_SIZE_FEE_BASE: FixedU128 = FixedU128::from_rational(1, 1000); // 0.001
111104
}
112105

113106
#[frame_support::pallet]
@@ -583,9 +576,7 @@ impl<T: Config> Pallet<T> {
583576
number_of_pages.saturating_sub(1) * max_message_size as u32 + last_page_size as u32;
584577
let threshold = channel_info.max_total_size / delivery_fee_constants::THRESHOLD_FACTOR;
585578
if total_size > threshold {
586-
let message_size_factor = FixedU128::from((encoded_fragment.len() / 1024) as u128)
587-
.saturating_mul(delivery_fee_constants::MESSAGE_SIZE_FEE_BASE);
588-
Self::increase_fee_factor(recipient, message_size_factor);
579+
Self::increase_fee_factor(recipient, encoded_fragment.len() as u128);
589580
}
590581

591582
Ok(number_of_pages)
@@ -1005,17 +996,24 @@ impl<T: Config> XcmpMessageSource for Pallet<T> {
1005996
result.push((para_id, page.into_inner()));
1006997
}
1007998

1008-
let max_total_size = match T::ChannelInfo::get_channel_info(para_id) {
1009-
Some(channel_info) => channel_info.max_total_size,
999+
let (max_total_size, max_message_size) = match T::ChannelInfo::get_channel_info(para_id)
1000+
{
1001+
Some(channel_info) => (
1002+
channel_info.max_total_size,
1003+
channel_info.max_message_size.min(T::MaxPageSize::get()) as usize,
1004+
),
10101005
None => {
10111006
log::warn!("calling `get_channel_info` with no RelevantMessagingState?!");
1012-
MAX_POSSIBLE_ALLOCATION // We use this as a fallback in case the messaging state is not present
1007+
// We use this as a fallback in case the messaging state is not present
1008+
(MAX_POSSIBLE_ALLOCATION, T::MaxPageSize::get() as usize)
10131009
},
10141010
};
10151011
let threshold = max_total_size.saturating_div(delivery_fee_constants::THRESHOLD_FACTOR);
1016-
let remaining_total_size: usize = (first_index..last_index)
1017-
.map(|index| OutboundXcmpMessages::<T>::decode_len(para_id, index).unwrap())
1018-
.sum();
1012+
// We have to count the total size here since `channel_info.total_size` is not updated
1013+
// at this point in time. We assume all previous pages are filled, which, in
1014+
// practice, is not always the case.
1015+
let num_pages = (last_index - first_index) as usize;
1016+
let remaining_total_size = num_pages.saturating_mul(max_message_size);
10191017
if remaining_total_size <= threshold as usize {
10201018
Self::decrease_fee_factor(para_id);
10211019
}
@@ -1153,23 +1151,15 @@ impl<T: Config> InspectMessageQueues for Pallet<T> {
11531151
impl<T: Config> FeeTracker for Pallet<T> {
11541152
type Id = ParaId;
11551153

1156-
fn get_fee_factor(id: Self::Id) -> FixedU128 {
1157-
<DeliveryFeeFactor<T>>::get(id)
1154+
fn get_min_fee_factor() -> FixedU128 {
1155+
InitialFactor::get()
11581156
}
11591157

1160-
fn increase_fee_factor(id: Self::Id, message_size_factor: FixedU128) -> FixedU128 {
1161-
<DeliveryFeeFactor<T>>::mutate(id, |f| {
1162-
*f = f.saturating_mul(
1163-
delivery_fee_constants::EXPONENTIAL_FEE_BASE.saturating_add(message_size_factor),
1164-
);
1165-
*f
1166-
})
1158+
fn get_fee_factor(id: Self::Id) -> FixedU128 {
1159+
<DeliveryFeeFactor<T>>::get(id)
11671160
}
11681161

1169-
fn decrease_fee_factor(id: Self::Id) -> FixedU128 {
1170-
<DeliveryFeeFactor<T>>::mutate(id, |f| {
1171-
*f = InitialFactor::get().max(*f / delivery_fee_constants::EXPONENTIAL_FEE_BASE);
1172-
*f
1173-
})
1162+
fn set_fee_factor(id: Self::Id, val: FixedU128) {
1163+
<DeliveryFeeFactor<T>>::set(id, val);
11741164
}
11751165
}

‎cumulus/pallets/xcmp-queue/src/tests.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -997,9 +997,9 @@ fn verify_fee_factor_increase_and_decrease() {
997997
// We take 5 100 byte pages
998998
XcmpQueue::take_outbound_messages(1);
999999
}
1000-
assert!(DeliveryFeeFactor::<Test>::get(sibling_para_id) < FixedU128::from_float(1.72));
1000+
assert!(DeliveryFeeFactor::<Test>::get(sibling_para_id) < FixedU128::from_float(1.80));
10011001
XcmpQueue::take_outbound_messages(1);
1002-
assert!(DeliveryFeeFactor::<Test>::get(sibling_para_id) < FixedU128::from_float(1.63));
1002+
assert!(DeliveryFeeFactor::<Test>::get(sibling_para_id) < FixedU128::from_float(1.72));
10031003
});
10041004
}
10051005

‎polkadot/runtime/parachains/src/dmp.rs

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ use polkadot_primitives::{DownwardMessage, Hash, Id as ParaId, InboundDownwardMe
5454
use sp_core::MAX_POSSIBLE_ALLOCATION;
5555
use sp_runtime::{
5656
traits::{BlakeTwo256, Hash as HashT, SaturatedConversion},
57-
FixedU128, Saturating,
57+
FixedU128,
5858
};
5959
use xcm::latest::SendError;
6060

@@ -64,8 +64,6 @@ pub use pallet::*;
6464
mod tests;
6565

6666
const THRESHOLD_FACTOR: u32 = 2;
67-
const EXPONENTIAL_FEE_BASE: FixedU128 = FixedU128::from_rational(105, 100); // 1.05
68-
const MESSAGE_SIZE_FEE_BASE: FixedU128 = FixedU128::from_rational(1, 1000); // 0.001
6967

7068
/// An error sending a downward message.
7169
#[cfg_attr(test, derive(Debug))]
@@ -224,7 +222,7 @@ impl<T: Config> Pallet<T> {
224222
para: ParaId,
225223
msg: DownwardMessage,
226224
) -> Result<(), QueueDownwardMessageError> {
227-
let serialized_len = msg.len() as u32;
225+
let serialized_len = msg.len();
228226
Self::can_queue_downward_message(config, &para, &msg)?;
229227

230228
let inbound =
@@ -245,9 +243,7 @@ impl<T: Config> Pallet<T> {
245243
let threshold =
246244
Self::dmq_max_length(config.max_downward_message_size).saturating_div(THRESHOLD_FACTOR);
247245
if q_len > (threshold as usize) {
248-
let message_size_factor = FixedU128::from((serialized_len / 1024) as u128)
249-
.saturating_mul(MESSAGE_SIZE_FEE_BASE);
250-
Self::increase_fee_factor(para, message_size_factor);
246+
Self::increase_fee_factor(para, serialized_len as u128);
251247
}
252248

253249
Ok(())
@@ -351,22 +347,16 @@ impl<T: Config> Pallet<T> {
351347
impl<T: Config> FeeTracker for Pallet<T> {
352348
type Id = ParaId;
353349

354-
fn get_fee_factor(id: Self::Id) -> FixedU128 {
355-
DeliveryFeeFactor::<T>::get(id)
350+
fn get_min_fee_factor() -> FixedU128 {
351+
InitialFactor::get()
356352
}
357353

358-
fn increase_fee_factor(id: Self::Id, message_size_factor: FixedU128) -> FixedU128 {
359-
DeliveryFeeFactor::<T>::mutate(id, |f| {
360-
*f = f.saturating_mul(EXPONENTIAL_FEE_BASE.saturating_add(message_size_factor));
361-
*f
362-
})
354+
fn get_fee_factor(id: Self::Id) -> FixedU128 {
355+
DeliveryFeeFactor::<T>::get(id)
363356
}
364357

365-
fn decrease_fee_factor(id: Self::Id) -> FixedU128 {
366-
DeliveryFeeFactor::<T>::mutate(id, |f| {
367-
*f = InitialFactor::get().max(*f / EXPONENTIAL_FEE_BASE);
368-
*f
369-
})
358+
fn set_fee_factor(id: Self::Id, val: FixedU128) {
359+
<DeliveryFeeFactor<T>>::set(id, val);
370360
}
371361
}
372362

0 commit comments

Comments
 (0)