Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion bin/millau/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ use sp_std::prelude::*;
#[cfg(feature = "std")]
use sp_version::NativeVersion;
use sp_version::RuntimeVersion;
use xcm_builder::NetworkExportTable;

// to be able to use Millau runtime in `bridge-runtime-common` tests
pub use bridge_runtime_common;
Expand Down Expand Up @@ -529,17 +530,27 @@ impl pallet_utility::Config for Runtime {

// this config is totally incorrect - the pallet is not actually used at this runtime. We need
// it only to be able to run benchmarks and make required traits (and default weights for tests).

parameter_types! {
pub BridgeTable: Vec<(xcm::prelude::NetworkId, xcm::prelude::MultiLocation, Option<xcm::prelude::MultiAsset>)>
= vec![(
xcm_config::RialtoNetwork::get(),
xcm_config::TokenLocation::get(),
Some((xcm_config::TokenAssetId::get(), 1_000_000_000_u128).into()),
)];
}

impl pallet_xcm_bridge_hub_router::Config for Runtime {
type WeightInfo = ();

type UniversalLocation = xcm_config::UniversalLocation;
type SiblingBridgeHubLocation = xcm_config::TokenLocation;
type BridgedNetworkId = xcm_config::RialtoNetwork;
type Bridges = NetworkExportTable<BridgeTable>;

type ToBridgeHubSender = xcm_config::XcmRouter;
type LocalXcmChannelManager = xcm_config::EmulatedSiblingXcmpChannel;

type BaseFee = ConstU128<1_000_000_000>;
type ByteFee = ConstU128<1_000>;
type FeeAsset = xcm_config::TokenAssetId;
}
Expand Down
2 changes: 1 addition & 1 deletion modules/messages/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ pub mod pallet {
// notify others about messages delivery
T::OnMessagesDelivered::on_messages_delivered(
lane_id,
lane.queued_messages().checked_len().unwrap_or(0),
lane.queued_messages().saturating_len(),
);

// because of lags, the inbound lane state (`lane_data`) may have entries for
Expand Down
6 changes: 3 additions & 3 deletions modules/xcm-bridge-hub-router/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

#![cfg(feature = "runtime-benchmarks")]

use crate::{DeliveryFeeFactor, InitialFactor};
use crate::{DeliveryFeeFactor, InitialFactor, MINIMAL_DELIVERY_FEE_FACTOR};

use frame_benchmarking::benchmarks_instance_pallet;
use frame_support::traits::{Get, Hooks};
Expand All @@ -35,13 +35,13 @@ pub trait Config<I: 'static>: crate::Config<I> {

benchmarks_instance_pallet! {
on_initialize_when_non_congested {
DeliveryFeeFactor::<T, I>::put(InitialFactor::get() + InitialFactor::get());
DeliveryFeeFactor::<T, I>::put(MINIMAL_DELIVERY_FEE_FACTOR + MINIMAL_DELIVERY_FEE_FACTOR);
}: {
crate::Pallet::<T, I>::on_initialize(Zero::zero())
}

on_initialize_when_congested {
DeliveryFeeFactor::<T, I>::put(InitialFactor::get() + InitialFactor::get());
DeliveryFeeFactor::<T, I>::put(MINIMAL_DELIVERY_FEE_FACTOR + MINIMAL_DELIVERY_FEE_FACTOR);
T::make_congested();
}: {
crate::Pallet::<T, I>::on_initialize(Zero::zero())
Expand Down
141 changes: 105 additions & 36 deletions modules/xcm-bridge-hub-router/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,18 @@
//! All other bridge hub queues offer some backpressure mechanisms. So if at least one
//! of all queues is congested, it will eventually lead to the growth of the queue at
//! this chain.
//!
//! **A note on terminology**: when we mention the bridge hub here, we mean the chain that
//! has the messages pallet deployed (`pallet-bridge-grandpa`, `pallet-bridge-messages`,
//! `pallet-xcm-bridge-hub`, ...). It may be the system bridge hub parachain or any other
//! chain.

#![cfg_attr(not(feature = "std"), no_std)]

use bp_xcm_bridge_hub::LocalXcmChannelManager;
use codec::Encode;
use frame_support::traits::Get;
use sp_runtime::{traits::One, FixedPointNumber, FixedU128, Saturating};
use sp_runtime::{FixedPointNumber, FixedU128, Saturating};
use xcm::prelude::*;
use xcm_builder::{ExporterFor, SovereignPaidRemoteExporter};

Expand All @@ -40,6 +45,9 @@ pub mod weights;

mod mock;

/// Minimal delivery fee factor.
pub const MINIMAL_DELIVERY_FEE_FACTOR: FixedU128 = FixedU128::from_u32(1);

/// The factor that is used to increase current message fee factor when bridge experiencing
/// some lags.
const EXPONENTIAL_FEE_BASE: FixedU128 = FixedU128::from_rational(105, 100); // 1.05
Expand Down Expand Up @@ -75,16 +83,20 @@ pub mod pallet {
type UniversalLocation: Get<InteriorMultiLocation>;
/// Relative location of the sibling bridge hub.
type SiblingBridgeHubLocation: Get<MultiLocation>;
/// The bridged network that this config is for.
/// The bridged network that this config is for if specified.
/// Also used for filtering `Bridges` by `BridgedNetworkId`.
/// If not specified, allows all networks pass through.
type BridgedNetworkId: Get<NetworkId>;
/// Configuration for supported **bridged networks/locations** with **bridge location** and
/// **possible fee**. Allows to externalize better control over allowed **bridged
/// networks/locations**.
type Bridges: ExporterFor;

/// Actual message sender (`HRMP` or `DMP`) to the sibling bridge hub location.
type ToBridgeHubSender: SendXcm;
/// Local XCM channel manager.
type LocalXcmChannelManager: LocalXcmChannelManager;

/// Base bridge fee that is paid for every outbound message.
type BaseFee: Get<u128>;
/// Additional fee that is paid for every byte of the outbound message.
type ByteFee: Get<u128>;
/// Asset that is used to paid bridge fee.
Expand All @@ -97,37 +109,36 @@ pub mod pallet {
#[pallet::hooks]
impl<T: Config<I>, I: 'static> Hooks<BlockNumberFor<T>> for Pallet<T, I> {
fn on_initialize(_n: BlockNumberFor<T>) -> Weight {
// if XCM queue is still congested, we don't change anything
// if XCM channel is still congested, we don't change anything
if T::LocalXcmChannelManager::is_congested(&T::SiblingBridgeHubLocation::get()) {
return T::WeightInfo::on_initialize_when_congested()
}

DeliveryFeeFactor::<T, I>::mutate(|f| {
let previous_factor = *f;
*f = InitialFactor::get().max(*f / EXPONENTIAL_FEE_BASE);
if previous_factor != *f {
log::info!(
target: LOG_TARGET,
"Bridge queue is uncongested. Decreased fee factor from {} to {}",
previous_factor,
f,
);
// if bridge has reported congestion, we don't change anything
let mut delivery_fee_factor = Self::delivery_fee_factor();
if delivery_fee_factor == MINIMAL_DELIVERY_FEE_FACTOR {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Maybe <= would be a bit safer here. Even though < should never happen.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a valid concern. OTOH, if delivery_fee_factor is less than MINIMAL_DELIVERY_FEE_FACTOR then maybe we should change it to minimal (this what will happen with current implementation and won't happen if we apply your suggestion)? That could be if we'll change a constant after deployment for example. WDYT?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually you're right. Better to have it changed to minimal in this case.

return T::WeightInfo::on_initialize_when_congested()
}

T::WeightInfo::on_initialize_when_non_congested()
} else {
// we have not actually updated the `DeliveryFeeFactor`, so we may deduct
// single db write from maximal weight
T::WeightInfo::on_initialize_when_non_congested()
.saturating_sub(T::DbWeight::get().writes(1))
}
})
let previous_factor = delivery_fee_factor;
delivery_fee_factor =
MINIMAL_DELIVERY_FEE_FACTOR.max(delivery_fee_factor / EXPONENTIAL_FEE_BASE);
log::info!(
target: LOG_TARGET,
"Bridge channel is uncongested. Decreased fee factor from {} to {}",
previous_factor,
delivery_fee_factor,
);

DeliveryFeeFactor::<T, I>::put(delivery_fee_factor);
T::WeightInfo::on_initialize_when_non_congested()
}
}

/// Initialization value for the delivery fee factor.
#[pallet::type_value]
pub fn InitialFactor() -> FixedU128 {
FixedU128::one()
MINIMAL_DELIVERY_FEE_FACTOR
}

/// The number to multiply the base delivery fee by.
Expand Down Expand Up @@ -168,7 +179,7 @@ pub mod pallet {
impl<T: Config<I>, I: 'static> Pallet<T, I> {
/// Called when new message is sent (queued to local outbound XCM queue) over the bridge.
pub(crate) fn on_message_sent_to_bridge(message_size: u32) {
// if outbound queue is not congested, do nothing
// if outbound channel is not congested, do nothing
if !T::LocalXcmChannelManager::is_congested(&T::SiblingBridgeHubLocation::get()) {
return
}
Expand All @@ -182,7 +193,7 @@ pub mod pallet {
*f = f.saturating_mul(total_factor);
log::info!(
target: LOG_TARGET,
"Bridge queue is congested. Increased fee factor from {} to {}",
"Bridge channel is congested. Increased fee factor from {} to {}",
previous_factor,
f,
);
Expand All @@ -205,32 +216,89 @@ type ViaBridgeHubExporter<T, I> = SovereignPaidRemoteExporter<
impl<T: Config<I>, I: 'static> ExporterFor for Pallet<T, I> {
fn exporter_for(
network: &NetworkId,
_remote_location: &InteriorMultiLocation,
remote_location: &InteriorMultiLocation,
message: &Xcm<()>,
) -> Option<(MultiLocation, Option<MultiAsset>)> {
// ensure that the message is sent to the expected bridged network
// ensure that the message is sent to the expected bridged network (if specified).
if *network != T::BridgedNetworkId::get() {
log::trace!(
target: LOG_TARGET,
"Router with bridged_network_id {:?} does not support bridging to network {:?}!",
T::BridgedNetworkId::get(),
network,
);
return None
}

// ensure that the message is sent to the expected bridged network and location.
let Some((bridge_hub_location, maybe_payment)) =
T::Bridges::exporter_for(network, remote_location, message)
else {
log::trace!(
target: LOG_TARGET,
"Router with bridged_network_id {:?} does not support bridging to network {:?} \
and remote_location {:?}!",
T::BridgedNetworkId::get(),
network,
remote_location,
);
return None
};

// ensure that the bridge hub location is the expected one
if bridge_hub_location != T::SiblingBridgeHubLocation::get() {
log::trace!(
target: LOG_TARGET,
"Router with bridged_network_id {:?} is configured to use different bridge hub \
router {:?}. Expected: {:?}",
T::BridgedNetworkId::get(),
bridge_hub_location,
T::SiblingBridgeHubLocation::get(),
);
return None
}

// take `base_fee` from `T::Brides`, but it has to be the same `T::FeeAsset`
let base_fee = match maybe_payment {
Some(payment) => match payment {
MultiAsset { fun: Fungible(amount), id } if id.eq(&T::FeeAsset::get()) => amount,
invalid_asset => {
log::error!(
target: LOG_TARGET,
"Router with bridged_network_id {:?} is configured for `T::FeeAsset` {:?} \
which is not compatible with {:?} for bridge_hub_location: {:?} for bridging to {:?}/{:?}!",
T::BridgedNetworkId::get(),
T::FeeAsset::get(),
invalid_asset,
bridge_hub_location,
network,
remote_location,
);
return None
},
},
None => 0,
};

// compute fee amount. Keep in mind that this is only the bridge fee. The fee for sending
// message from this chain to child/sibling bridge hub is determined by the
// `Config::ToBridgeHubSender`
let message_size = message.encoded_size();
let message_fee = (message_size as u128).saturating_mul(T::ByteFee::get());
let fee_sum = T::BaseFee::get().saturating_add(message_fee);
let fee_sum = base_fee.saturating_add(message_fee);
let fee_factor = Self::delivery_fee_factor();
let fee = fee_factor.saturating_mul_int(fee_sum);
let fee = if fee > 0 { Some((T::FeeAsset::get(), fee).into()) } else { None };

log::info!(
target: LOG_TARGET,
"Going to send message ({} bytes) over bridge. Computed bridge fee {} using fee factor {}",
"Going to send message ({} bytes) over bridge. Computed bridge fee {:?} using fee factor {}",
message_size,
fee,
fee_factor,
message_size,
);

Some((T::SiblingBridgeHubLocation::get(), Some((T::FeeAsset::get(), fee).into())))
Some((bridge_hub_location, fee))
}
}

Expand Down Expand Up @@ -271,7 +339,7 @@ impl<T: Config<I>, I: 'static> SendXcm for Pallet<T, I> {
// use router to enqueue message to the sibling/child bridge hub. This also should handle
// payment for passing through this queue.
let (message_size, ticket) = ticket;
let xcm_hash = T::ToBridgeHubSender::deliver(ticket)?;
let xcm_hash = ViaBridgeHubExporter::<T, I>::deliver(ticket)?;

// increase delivery fee factor if required
Self::on_message_sent_to_bridge(message_size);
Expand All @@ -286,11 +354,12 @@ mod tests {
use mock::*;

use frame_support::traits::Hooks;
use sp_runtime::traits::One;

#[test]
fn initial_fee_factor_is_one() {
run_test(|| {
assert_eq!(DeliveryFeeFactor::<TestRuntime, ()>::get(), FixedU128::one());
assert_eq!(DeliveryFeeFactor::<TestRuntime, ()>::get(), MINIMAL_DELIVERY_FEE_FACTOR);
})
}

Expand All @@ -313,13 +382,13 @@ mod tests {
DeliveryFeeFactor::<TestRuntime, ()>::put(FixedU128::from_rational(125, 100));

// it shold eventually decreased to one
while XcmBridgeHubRouter::delivery_fee_factor() > FixedU128::one() {
while XcmBridgeHubRouter::delivery_fee_factor() > MINIMAL_DELIVERY_FEE_FACTOR {
XcmBridgeHubRouter::on_initialize(One::one());
}

// verify that it doesn't decreases anymore
XcmBridgeHubRouter::on_initialize(One::one());
assert_eq!(XcmBridgeHubRouter::delivery_fee_factor(), FixedU128::one());
assert_eq!(XcmBridgeHubRouter::delivery_fee_factor(), MINIMAL_DELIVERY_FEE_FACTOR);
})
}

Expand Down
8 changes: 7 additions & 1 deletion modules/xcm-bridge-hub-router/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use sp_runtime::{
BuildStorage,
};
use xcm::prelude::*;
use xcm_builder::NetworkExportTable;

pub type AccountId = u64;
type Block = frame_system::mocking::MockBlock<TestRuntime>;
Expand All @@ -51,6 +52,11 @@ parameter_types! {
pub UniversalLocation: InteriorMultiLocation = X2(GlobalConsensus(ThisNetworkId::get()), Parachain(1000));
pub SiblingBridgeHubLocation: MultiLocation = ParentThen(X1(Parachain(1002))).into();
pub BridgeFeeAsset: AssetId = MultiLocation::parent().into();
pub BridgeTable: Vec<(NetworkId, MultiLocation, Option<MultiAsset>)> = vec![(
BridgedNetworkId::get(),
SiblingBridgeHubLocation::get(),
Some((BridgeFeeAsset::get(), BASE_FEE).into()),
)];
}

impl frame_system::Config for TestRuntime {
Expand Down Expand Up @@ -85,11 +91,11 @@ impl pallet_xcm_bridge_hub_router::Config<()> for TestRuntime {
type UniversalLocation = UniversalLocation;
type SiblingBridgeHubLocation = SiblingBridgeHubLocation;
type BridgedNetworkId = BridgedNetworkId;
type Bridges = NetworkExportTable<BridgeTable>;

type ToBridgeHubSender = TestToBridgeHubSender;
type LocalXcmChannelManager = TestLocalXcmChannelManager;

type BaseFee = ConstU128<BASE_FEE>;
type ByteFee = ConstU128<BYTE_FEE>;
type FeeAsset = BridgeFeeAsset;
}
Expand Down