Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bridge fixes for stable2409 #7

Merged
Show file tree
Hide file tree
Changes from all commits
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
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
Comment on lines +338 to +340

Choose a reason for hiding this comment

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

I recommend option 1: wait for and use patch-release stable2409-3 2024-12-12

If there are other time-sensitive deliverables to be deployed with this upgrade and we can't wait for stable2409-3, then we should do 3. go with XcmpQueue::send_signal workaround.

I would not go with option 2 Ok(()) / Err(())


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
Loading