Skip to content

Commit

Permalink
Bridge fixes for stable2409 (#7)
Browse files Browse the repository at this point in the history
* Remove unused

* Added `LocalXcmChannelManager` for bridges

* Fixed `BridgeDeposit` TODO

* Compilation fix + don't need to fund system para

* fmt
  • Loading branch information
bkontur authored Dec 3, 2024
1 parent 0cc2571 commit ac46d87
Show file tree
Hide file tree
Showing 11 changed files with 307 additions and 84 deletions.
4 changes: 4 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ bp-polkadot-bulletin = { version = "0.15.0", default-features = false }
bp-polkadot-core = { version = "0.18.0", default-features = false }
bp-relayers = { version = "0.18.0", default-features = false }
bp-runtime = { version = "0.18.0", default-features = false }
bp-xcm-bridge-hub = { version = "0.4.0", default-features = false }
bp-xcm-bridge-hub-router = { version = "0.14.1", default-features = false }
bridge-hub-common = { version = "0.10.0", default-features = false }
bridge-hub-kusama-emulated-chain = { path = "integration-tests/emulated/chains/parachains/bridges/bridge-hub-kusama" }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,30 +46,9 @@ frame_support::parameter_types! {
/// Some sane weight to execute `xcm::Transact(pallet-xcm-bridge-hub-router::Call::report_bridge_status)`.
pub const XcmBridgeHubRouterTransactCallMaxWeight: Weight = Weight::from_parts(200_000_000, 6144);

/// Message that is sent to the sibling Kusama Asset Hub when the with-Polkadot bridge becomes congested.
pub CongestedMessage: Xcm<()> = build_congestion_message(true).into();
/// Message that is sent to the sibling Kusama Asset Hub when the with-Polkadot bridge becomes uncongested.
pub UncongestedMessage: Xcm<()> = build_congestion_message(false).into();

/// Should match the `AssetDeposit` of the `ForeignAssets` pallet on Asset Hub.
pub const CreateForeignAssetDeposit: u128 = system_para_deposit(1, 190);
}

fn build_congestion_message(is_congested: bool) -> sp_std::vec::Vec<Instruction<()>> {
sp_std::vec![
UnpaidExecution { weight_limit: Unlimited, check_origin: None },
Transact {
origin_kind: OriginKind::Xcm,
require_weight_at_most: XcmBridgeHubRouterTransactCallMaxWeight::get(),
call: Call::ToPolkadotXcmRouter(XcmBridgeHubRouterCall::report_bridge_status {
bridge_id: Default::default(),
is_congested,
})
.encode()
.into(),
}
]
}

/// Identifier of AssetHubKusama in the Kusama relay chain.
pub const ASSET_HUB_KUSAMA_PARACHAIN_ID: u32 = 1000;
Original file line number Diff line number Diff line change
Expand Up @@ -46,30 +46,9 @@ frame_support::parameter_types! {
/// Some sane weight to execute `xcm::Transact(pallet-xcm-bridge-hub-router::Call::report_bridge_status)`.
pub const XcmBridgeHubRouterTransactCallMaxWeight: Weight = Weight::from_parts(200_000_000, 6144);

/// Message that is sent to the sibling Kusama Asset Hub when the with-Polkadot bridge becomes congested.
pub CongestedMessage: Xcm<()> = build_congestion_message(true).into();
/// Message that is sent to the sibling Kusama Asset Hub when the with-Polkadot bridge becomes uncongested.
pub UncongestedMessage: Xcm<()> = build_congestion_message(false).into();

/// Should match the `AssetDeposit` of the `ForeignAssets` pallet on Asset Hub.
pub const CreateForeignAssetDeposit: u128 = system_para_deposit(1, 190);
}

fn build_congestion_message(is_congested: bool) -> sp_std::vec::Vec<Instruction<()>> {
sp_std::vec![
UnpaidExecution { weight_limit: Unlimited, check_origin: None },
Transact {
origin_kind: OriginKind::Xcm,
require_weight_at_most: XcmBridgeHubRouterTransactCallMaxWeight::get(),
call: Call::ToKusamaXcmRouter(XcmBridgeHubRouterCall::report_bridge_status {
bridge_id: Default::default(),
is_congested,
})
.encode()
.into(),
}
]
}

/// Identifier of AssetHubPolkadot in the Polkadot relay chain.
pub const ASSET_HUB_POLKADOT_PARACHAIN_ID: u32 = 1000;
1 change: 0 additions & 1 deletion system-parachains/asset-hubs/asset-hub-polkadot/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -860,7 +860,6 @@ impl pallet_xcm_bridge_hub_router::Config<ToKusamaXcmRouterInstance> for Runtime

type ByteFee = xcm_config::bridging::XcmBridgeHubRouterByteFee;
type FeeAsset = xcm_config::bridging::XcmBridgeHubRouterFeeAssetId;
// TODO: @bkontur - change to `report_bridge_status` when patched - FAIL-CI
type LocalXcmChannelManager =
cumulus_pallet_xcmp_queue::bridging::InAndOutXcmpChannelStatusProvider<Runtime>;
}
Expand Down
4 changes: 4 additions & 0 deletions system-parachains/bridge-hubs/bridge-hub-kusama/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ bp-relayers = { workspace = true }
bp-runtime = { workspace = true }
bp-kusama = { workspace = true }
bp-polkadot = { workspace = true }
bp-xcm-bridge-hub = { workspace = true }
bp-xcm-bridge-hub-router = { workspace = true }
bridge-hub-common = { workspace = true }
bridge-runtime-common = { workspace = true }
pallet-bridge-grandpa = { workspace = true }
Expand Down Expand Up @@ -129,6 +131,8 @@ std = [
"bp-polkadot/std",
"bp-relayers/std",
"bp-runtime/std",
"bp-xcm-bridge-hub-router/std",
"bp-xcm-bridge-hub/std",
"bridge-hub-common/std",
"bridge-runtime-common/std",
"codec/std",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ use bp_messages::{
use bp_parachains::SingleParaStoredHeaderDataBuilder;
use bp_runtime::Chain;
use bridge_hub_common::xcm_version::XcmVersionOfDestAndRemoteBridge;
use frame_support::{parameter_types, traits::PalletInfoAccess};
use frame_support::{
parameter_types,
traits::{ConstU128, PalletInfoAccess},
};
use frame_system::{EnsureNever, EnsureRoot};
use kusama_runtime_constants as constants;
use pallet_bridge_messages::LaneIdOf;
Expand Down Expand Up @@ -140,8 +143,6 @@ parameter_types! {
pub PriorityBoostPerParachainHeader: u64 = 920_224_664_224_664;
// see the `FEE_BOOST_PER_MESSAGE` constant to get the meaning of this value
pub PriorityBoostPerMessage: u64 = 182_044_444_444_444;
// TODO: What's the correct value? - FAIL-CI
pub storage BridgeDeposit: Balance = constants::currency::UNITS;
}

/// Proof of messages, coming from Polkadot.
Expand Down Expand Up @@ -246,19 +247,156 @@ impl pallet_xcm_bridge_hub::Config<XcmOverBridgeHubPolkadotInstance> for Runtime
type BridgeOriginAccountIdConverter =
(ParentIsPreset<AccountId>, SiblingParachainConvertsVia<Sibling, AccountId>);

type BridgeDeposit = BridgeDeposit;
// We do not allow creating bridges here (see `T::OpenBridgeOrigin` above), so there is no need
// to set a deposit.
type BridgeDeposit = ConstU128<0>;
type Currency = Balances;
type RuntimeHoldReason = RuntimeHoldReason;
// Do not require deposit from system parachains or relay chain
type AllowWithoutBridgeDeposit =
RelayOrOtherSystemParachains<AllSiblingSystemParachains, Runtime>;

// TODO: @acatangiu (bridges-v2) - add `LocalXcmChannelManager` impl - https://github.com/paritytech/parity-bridges-common/issues/3047
// @acatangiu
type LocalXcmChannelManager = ();
type LocalXcmChannelManager = XcmpQueueChannelManager;
type BlobDispatcher = FromPolkadotMessageBlobDispatcher;
}

/// Implementation `bp_xcm_bridge_hub::LocalXcmChannelManager`.
pub struct XcmpQueueChannelManager;
impl bp_xcm_bridge_hub::LocalXcmChannelManager for XcmpQueueChannelManager {
type Error = ();

fn is_congested(with: &Location) -> bool {
// This is used to check the inbound queue/messages to determine if they can be dispatched
// and sent to the sibling parachain. Therefore, checking `OutXcmp` is sufficient.
use bp_xcm_bridge_hub_router::XcmChannelStatusProvider;
cumulus_pallet_xcmp_queue::bridging::OutXcmpChannelStatusProvider::<Runtime>::is_congested(
with,
)
}

fn suspend_bridge(
_local_origin: &Location,
_: pallet_xcm_bridge_hub::BridgeId,
) -> Result<(), Self::Error> {
// IMPORTANT NOTE:
//
// Unfortunately, `https://github.com/paritytech/polkadot-sdk/pull/6231` reworked congestion is not yet released.
//
// And unfortunately, we don't have access to `XcmpQueue::send_signal(para,
// ChannelSignal::Suspend)` here (which would require patch release), we can add this
// hacky workaround/tmp/implementation that should trigger `ChannelSignal::Suspend`, e.g.:
/*
use crate::{MessageQueue, XcmpQueue};
use bridge_hub_common::message_queue::AggregateMessageOrigin;
use codec::{Decode, Encode, MaxEncodedLen};
use frame_support::traits::EnqueueMessage;
use frame_support::pallet_prelude::OptionQuery;
use pallet_message_queue::OnQueueChanged;
use scale_info::TypeInfo;
// get sibling para id
let local_origin_para_id: crate::ParaId = match local_origin.unpack() {
(1, [Parachain(id)]) => (*id).into(),
_ => return Err(())
};
// read `suspend_threshold` from `XcmpQueue` storage
#[derive(Copy, Clone, Eq, PartialEq, Encode, Decode, TypeInfo, MaxEncodedLen)]
struct QueueConfigData {
suspend_threshold: u32,
drop_threshold: u32,
resume_threshold: u32,
}
#[frame_support::storage_alias]
type QueueConfig = StorageValue<XcmpQueue, QueueConfigData, OptionQuery>;
let suspend_threshold = match QueueConfig::get() {
Some(qc) => qc.suspend_threshold,
None => return Err(())
};
// Now, this should trigger `XcmpQueue::send_signal(para, ChannelSignal::Suspend)`
let mut qf = MessageQueue::footprint(AggregateMessageOrigin::Sibling(local_origin_para_id));
qf.ready_pages = suspend_threshold;
XcmpQueue::on_queue_changed(local_origin_para_id.into(), qf);
*/

// IMPORTANT NOTE2:
//
// In the current setup, this code is likely triggered only for the hard-coded AHK<>AHP
// lane, as we do not support any other bridge lanes on BridgeHubs. It is triggered only
// when `pallet_bridge_messages::OutboundMessages` reaches 8,192 undelivered messages. The
// potential risk of keeping `Ok(())` or `Err(())` here is that
// `pallet_bridge_messages::OutboundMessages` may continue to grow:
//
// ```
// #[pallet::storage]
// pub type OutboundMessages<T: Config<I>, I: 'static = ()> =
// StorageMap<_, Blake2_128Concat, MessageKey<T::LaneId>, StoredMessagePayload<T, I>>;
// ```

// TODO: decide:
// 1. wait for patch-release stable2409-3 2024-12-12
// 2. go with `Ok(())` / `Err(())`
// 3. go with `XcmpQueue::send_signal` temporary workaround till patch release

Ok(())
}

fn resume_bridge(
_local_origin: &Location,
_: pallet_xcm_bridge_hub::BridgeId,
) -> Result<(), Self::Error> {
// IMPORTANT NOTE:
//
// Unfortunately, `https://github.com/paritytech/polkadot-sdk/pull/6231` reworked congestion is not yet released.
//
// And unfortunately, we don't have access to `XcmpQueue::send_signal(para,
// ChannelSignal::Resume)` here (which would require patch release), we can add this hacky
// workaround/tmp/implementation that should trigger `ChannelSignal::Resume`, e.g.:
/*
use crate::{MessageQueue, XcmpQueue};
use bridge_hub_common::message_queue::AggregateMessageOrigin;
use codec::{Decode, Encode, MaxEncodedLen};
use frame_support::traits::EnqueueMessage;
use frame_support::pallet_prelude::OptionQuery;
use pallet_message_queue::OnQueueChanged;
use scale_info::TypeInfo;
// get sibling para id
let local_origin_para_id: crate::ParaId = match local_origin.unpack() {
(1, [Parachain(id)]) => (*id).into(),
_ => return Err(())
};
// read `resume_threshold` from `XcmpQueue` storage
#[derive(Copy, Clone, Eq, PartialEq, Encode, Decode, TypeInfo, MaxEncodedLen)]
struct QueueConfigData {
suspend_threshold: u32,
drop_threshold: u32,
resume_threshold: u32,
}
#[frame_support::storage_alias]
type QueueConfig = StorageValue<XcmpQueue, QueueConfigData, OptionQuery>;
let resume_threshold = match QueueConfig::get() {
Some(qc) => qc.resume_threshold,
None => return Err(())
};
// Now, this should trigger `XcmpQueue::send_signal(para, ChannelSignal::Resume)`
let mut qf = MessageQueue::footprint(AggregateMessageOrigin::Sibling(local_origin_para_id));
qf.ready_pages = resume_threshold;
XcmpQueue::on_queue_changed(local_origin_para_id.into(), qf);
*/

// TODO: decide:
// 1. wait for patch-release stable2409-3 2024-12-12
// 2. go with `Ok(())` / `Err(())`
// 3. go with `XcmpQueue::send_signal` temporary workaround till patch release

Ok(())
}
}

#[cfg(feature = "runtime-benchmarks")]
pub(crate) fn open_bridge_for_benchmarks<R, XBHI, C>(
with: pallet_xcm_bridge_hub::LaneIdOf<R, XBHI>,
Expand Down
16 changes: 2 additions & 14 deletions system-parachains/bridge-hubs/bridge-hub-kusama/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1128,21 +1128,9 @@ impl_runtime_apis! {
BenchmarkError::Stop("XcmVersion was not stored!")
})?;

let sibling_parachain_id = Parachain(8765);
let sibling_system_parachain_id = Parachain(1000);
let remote_parachain_id = Parachain(5678);
let sibling_parachain_location = Location::new(1, [sibling_parachain_id]);

// fund SA
use frame_support::traits::fungible::Mutate;
use xcm_executor::traits::ConvertLocation;
frame_support::assert_ok!(
Balances::mint_into(
&xcm_config::LocationToAccountId::convert_location(&sibling_parachain_location).expect("valid AccountId"),
bridge_to_polkadot_config::BridgeDeposit::get()
.saturating_add(ExistentialDeposit::get())
.saturating_add(UNITS * 5)
)
);
let sibling_parachain_location = Location::new(1, [sibling_system_parachain_id]);

// open bridge
let bridge_destination_universal_location: InteriorLocation = [GlobalConsensus(NetworkId::Polkadot), remote_parachain_id].into();
Expand Down
4 changes: 4 additions & 0 deletions system-parachains/bridge-hubs/bridge-hub-polkadot/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ bp-relayers = { workspace = true }
bp-runtime = { workspace = true }
bp-kusama = { workspace = true }
bp-polkadot = { workspace = true }
bp-xcm-bridge-hub = { workspace = true }
bp-xcm-bridge-hub-router = { workspace = true }
bridge-hub-common = { workspace = true }
bridge-runtime-common = { workspace = true }
pallet-bridge-grandpa = { workspace = true }
Expand Down Expand Up @@ -143,6 +145,8 @@ std = [
"bp-polkadot/std",
"bp-relayers/std",
"bp-runtime/std",
"bp-xcm-bridge-hub-router/std",
"bp-xcm-bridge-hub/std",
"bridge-hub-common/std",
"bridge-runtime-common/std",
"codec/std",
Expand Down
Loading

0 comments on commit ac46d87

Please sign in to comment.