From a5c61f87de9e4f16d233e643a822653ac086dd8f Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Tue, 27 Jun 2023 14:15:29 +0300 Subject: [PATCH 01/10] add dispatcher state field to InboundLaneData and OutboundLaneData --- bin/runtime-common/src/messages_call_ext.rs | 2 + .../src/messages_xcm_extension.rs | 10 ++-- modules/messages/src/proofs.rs | 5 ++ modules/messages/src/tests/mock.rs | 15 +++--- modules/messages/src/tests/pallet_tests.rs | 15 +++++- primitives/messages/src/lib.rs | 53 +++++++++++++++++-- primitives/messages/src/target_chain.rs | 29 ++++++++-- primitives/runtime/src/lib.rs | 1 - primitives/runtime/src/messages.rs | 35 ------------ 9 files changed, 112 insertions(+), 53 deletions(-) delete mode 100644 primitives/runtime/src/messages.rs diff --git a/bin/runtime-common/src/messages_call_ext.rs b/bin/runtime-common/src/messages_call_ext.rs index 4ec82b8465..20c0c7710a 100644 --- a/bin/runtime-common/src/messages_call_ext.rs +++ b/bin/runtime-common/src/messages_call_ext.rs @@ -366,6 +366,7 @@ mod tests { state: LaneState::Opened, relayers: Default::default(), last_confirmed_nonce: 10, + dispatcher_state: Default::default(), }, ); } @@ -496,6 +497,7 @@ mod tests { oldest_unpruned_nonce: 0, latest_received_nonce: 10, latest_generated_nonce: 10, + bridged_dispatcher_state: None, }, ); } diff --git a/bin/runtime-common/src/messages_xcm_extension.rs b/bin/runtime-common/src/messages_xcm_extension.rs index c6a7703a72..df4fd56690 100644 --- a/bin/runtime-common/src/messages_xcm_extension.rs +++ b/bin/runtime-common/src/messages_xcm_extension.rs @@ -23,10 +23,10 @@ use bp_messages::{ source_chain::MessagesBridge, - target_chain::{DispatchMessage, MessageDispatch}, - LaneId, + target_chain::{DispatchMessage, MessageDispatch, MessageDispatchResult}, + DispatcherState, LaneId, }; -use bp_runtime::{messages::MessageDispatchResult, Chain}; +use bp_runtime::Chain; use codec::{Decode, Encode}; use frame_support::{dispatch::Weight, CloneNoBound, EqNoBound, PartialEqNoBound}; use pallet_bridge_messages::WeightInfoExt as MessagesPalletWeights; @@ -124,6 +124,10 @@ impl MessageDispat }; MessageDispatchResult { unspent_weight: Weight::zero(), dispatch_level_result } } + + fn state() -> DispatcherState { + unimplemented!("TODO") + } } /// [`XcmBlobHauler`] is responsible for sending messages to the bridge "point-to-point link" from diff --git a/modules/messages/src/proofs.rs b/modules/messages/src/proofs.rs index 14118d7819..d6d74424d5 100644 --- a/modules/messages/src/proofs.rs +++ b/modules/messages/src/proofs.rs @@ -393,6 +393,7 @@ mod tests { oldest_unpruned_nonce: 1, latest_received_nonce: 1, latest_generated_nonce: 1, + bridged_dispatcher_state: None, }), encode_all_messages, |d| { @@ -434,6 +435,7 @@ mod tests { oldest_unpruned_nonce: 1, latest_received_nonce: 1, latest_generated_nonce: 1, + bridged_dispatcher_state: None, }), encode_all_messages, encode_lane_data, @@ -449,6 +451,7 @@ mod tests { oldest_unpruned_nonce: 1, latest_received_nonce: 1, latest_generated_nonce: 1, + bridged_dispatcher_state: None, }), messages: Vec::new(), }, @@ -468,6 +471,7 @@ mod tests { oldest_unpruned_nonce: 1, latest_received_nonce: 1, latest_generated_nonce: 1, + bridged_dispatcher_state: None, }), encode_all_messages, encode_lane_data, @@ -483,6 +487,7 @@ mod tests { oldest_unpruned_nonce: 1, latest_received_nonce: 1, latest_generated_nonce: 1, + bridged_dispatcher_state: None, }), messages: vec![Message { key: MessageKey { lane_id: test_lane_id(), nonce: 1 }, diff --git a/modules/messages/src/tests/mock.rs b/modules/messages/src/tests/mock.rs index 54b1af4257..d1384eb32b 100644 --- a/modules/messages/src/tests/mock.rs +++ b/modules/messages/src/tests/mock.rs @@ -31,14 +31,13 @@ use bp_messages::{ source_chain::{DeliveryConfirmationPayments, FromBridgedChainMessagesDeliveryProof}, target_chain::{ DeliveryPayments, DispatchMessage, DispatchMessageData, FromBridgedChainMessagesProof, - MessageDispatch, + MessageDispatch, MessageDispatchResult, }, - ChainWithMessages, DeliveredMessages, InboundLaneData, LaneId, LaneState, Message, MessageKey, - MessageNonce, MessagePayload, OutboundLaneData, UnrewardedRelayer, UnrewardedRelayersState, -}; -use bp_runtime::{ - messages::MessageDispatchResult, Chain, ChainId, Size, UnverifiedStorageProofParams, + ChainWithMessages, DeliveredMessages, DispatcherState, InboundLaneData, LaneId, LaneState, + Message, MessageKey, MessageNonce, MessagePayload, OutboundLaneData, UnrewardedRelayer, + UnrewardedRelayersState, }; +use bp_runtime::{Chain, ChainId, Size, UnverifiedStorageProofParams}; use codec::{Decode, Encode}; use frame_support::{ parameter_types, @@ -405,6 +404,10 @@ impl MessageDispatch for TestMessageDispatch { Err(_) => dispatch_result(0), } } + + fn state() -> DispatcherState { + DispatcherState::default() + } } /// Return test lane message with given nonce and payload. diff --git a/modules/messages/src/tests/pallet_tests.rs b/modules/messages/src/tests/pallet_tests.rs index eddb81801c..dc10a49ac7 100644 --- a/modules/messages/src/tests/pallet_tests.rs +++ b/modules/messages/src/tests/pallet_tests.rs @@ -26,8 +26,8 @@ use crate::{ use bp_messages::{ source_chain::FromBridgedChainMessagesDeliveryProof, target_chain::FromBridgedChainMessagesProof, BridgeMessagesCall, ChainWithMessages, - DeliveredMessages, InboundLaneData, InboundMessageDetails, LaneId, LaneState, MessageKey, - MessageNonce, MessagesOperatingMode, OutboundLaneData, OutboundMessageDetails, + DeliveredMessages, DispatcherState, InboundLaneData, InboundMessageDetails, LaneId, LaneState, + MessageKey, MessageNonce, MessagesOperatingMode, OutboundLaneData, OutboundMessageDetails, UnrewardedRelayer, UnrewardedRelayersState, VerificationError, }; use bp_runtime::{BasicOperatingMode, PreComputedSize, Size}; @@ -88,6 +88,7 @@ fn receive_messages_delivery_proof() { messages: DeliveredMessages::new(1), }] .into(), + dispatcher_state: DispatcherState::default(), }, ), UnrewardedRelayersState { @@ -144,6 +145,7 @@ fn pallet_rejects_transactions_if_halted() { state: LaneState::Opened, last_confirmed_nonce: 1, relayers: vec![unrewarded_relayer(1, 1, TEST_RELAYER_A)].into(), + dispatcher_state: DispatcherState::default(), }, ); assert_noop!( @@ -193,6 +195,7 @@ fn pallet_rejects_new_messages_in_rejecting_outbound_messages_operating_mode() { state: LaneState::Opened, last_confirmed_nonce: 1, relayers: vec![unrewarded_relayer(1, 1, TEST_RELAYER_A)].into(), + dispatcher_state: DispatcherState::default(), }, ), UnrewardedRelayersState { @@ -273,6 +276,7 @@ fn receive_messages_proof_updates_confirmed_message_nonce() { unrewarded_relayer(10, 10, TEST_RELAYER_B), ] .into(), + dispatcher_state: DispatcherState::default(), }, ); assert_eq!( @@ -307,6 +311,7 @@ fn receive_messages_proof_updates_confirmed_message_nonce() { unrewarded_relayer(11, 11, TEST_RELAYER_A) ] .into(), + dispatcher_state: DispatcherState::default(), }, ); assert_eq!( @@ -736,6 +741,7 @@ fn proof_size_refund_from_receive_messages_proof_works() { ] .into(), last_confirmed_nonce: 0, + dispatcher_state: DispatcherState::default(), }), ); let post_dispatch_weight = Pallet::::receive_messages_proof( @@ -765,6 +771,7 @@ fn proof_size_refund_from_receive_messages_proof_works() { ] .into(), last_confirmed_nonce: 0, + dispatcher_state: DispatcherState::default(), }), ); let post_dispatch_weight = Pallet::::receive_messages_proof( @@ -805,6 +812,7 @@ fn receive_messages_delivery_proof_rejects_proof_if_trying_to_confirm_more_messa state: LaneState::Opened, last_confirmed_nonce: 1, relayers: Default::default(), + dispatcher_state: DispatcherState::default(), }, ); assert_noop!( @@ -875,6 +883,7 @@ fn test_bridge_messages_call_is_correctly_defined() { messages: DeliveredMessages::new(1), }] .into(), + dispatcher_state: DispatcherState::default(), }, ); let unrewarded_relayer_state = UnrewardedRelayersState { @@ -943,6 +952,7 @@ fn inbound_storage_extra_proof_size_bytes_works() { state: LaneState::Opened, relayers: vec![relayer_entry(); relayer_entries].into(), last_confirmed_nonce: 0, + dispatcher_state: DispatcherState::default(), }, _phantom: Default::default(), } @@ -1031,6 +1041,7 @@ fn receive_messages_delivery_proof_fails_if_outbound_lane_is_unknown() { messages: DeliveredMessages::new(1), }] .into(), + dispatcher_state: DispatcherState::default(), }, ) }; diff --git a/primitives/messages/src/lib.rs b/primitives/messages/src/lib.rs index a2022342b2..e7950fb381 100644 --- a/primitives/messages/src/lib.rs +++ b/primitives/messages/src/lib.rs @@ -22,8 +22,8 @@ use bp_header_chain::HeaderChainError; use bp_runtime::{ - messages::MessageDispatchResult, AccountIdOf, BasicOperatingMode, Chain, HashOf, OperatingMode, - RangeInclusiveExt, StorageProofError, UnderlyingChainOf, UnderlyingChainProvider, + AccountIdOf, BasicOperatingMode, Chain, HashOf, OperatingMode, RangeInclusiveExt, + StorageProofError, UnderlyingChainOf, UnderlyingChainProvider, }; use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::{PalletError, RuntimeDebug}; @@ -35,6 +35,7 @@ use source_chain::RelayersRewards; use sp_core::{TypeId, H256}; use sp_io::hashing::blake2_256; use sp_std::{collections::vec_deque::VecDeque, ops::RangeInclusive, prelude::*}; +use target_chain::MessageDispatchResult; pub mod source_chain; pub mod storage_keys; @@ -292,6 +293,39 @@ pub struct Message { pub payload: MessagePayload, } +/// State of the message dispatcher. Our code is generic over dispatcher, but we assume +/// that all dispatchers will share some traits. That's why we use this struct +/// directly, without making code generic over it. However it may be changed in the future. +/// +/// This state is reported to the messages pallet by the `MessageDispatch::dispatch` call. +/// It is stored as a part of `InboundLaneData` and is proved back to the sending chain +/// during message delivery confirmation transaction, where it is then written to the +/// matching `OutboundLaneData`. +/// +/// This data may be used by the pallets that are using the messages pallet to implement +/// backpressure mechanism on the sending chain like that: +/// +/// (1) initially assume that all sent messages are stuck somewhere in the bridge; +/// +/// (2) exponentially increase the message fee at every block. The exponential factor +/// may need to be lower than the same for other queues (e.g. XCMP) though. That's +/// because a message roundtrip is significantly larger when it is sent over bridge +/// vs when it is sent within a local consensus. But there should be some reasonable +/// interval during which we assume the message will be delviered and confirmed. If +/// there are no reports about queue status at the bridged side during that interval, +/// we may start increasing the fee with a larger factor; +/// +/// (3) during the message delivery confirmation transaction, we receive the updated may +/// bridged queue state and may start to lower the fee. +#[derive(Encode, Decode, Default, Clone, RuntimeDebug, PartialEq, Eq, TypeInfo, MaxEncodedLen)] +pub struct DispatcherState { + /// Number of the messages, that are not yet processed by the dispatcher. We assume that + /// dispatchers that are processing messages immediately will always set this field to + /// zero. If dispatcher is inserting some/all incoming messages to some queue, it will + /// set this field to the queue length. + pub unprocessed_messages: MessageNonce, +} + /// Inbound lane data. #[derive(Encode, Decode, Clone, RuntimeDebug, PartialEq, Eq, TypeInfo)] pub struct InboundLaneData { @@ -328,6 +362,10 @@ pub struct InboundLaneData { /// This value is updated indirectly when an `OutboundLane` state of the source /// chain is received alongside with new messages delivery. pub last_confirmed_nonce: MessageNonce, + + /// State of the message dispatcher at this side of the bridge. We use this field as + /// a cheap way to report some dispatcher-related data back to the sending chain. + pub dispatcher_state: DispatcherState, } impl Default for InboundLaneData { @@ -336,6 +374,7 @@ impl Default for InboundLaneData { state: LaneState::Closed, relayers: VecDeque::new(), last_confirmed_nonce: 0, + dispatcher_state: DispatcherState::default(), } } } @@ -356,7 +395,8 @@ impl InboundLaneData { { relayers_entries .checked_mul(UnrewardedRelayer::::max_encoded_len())? - .checked_add(MessageNonce::max_encoded_len()) + .checked_add(MessageNonce::max_encoded_len())? + .checked_add(DispatcherState::max_encoded_len()) } /// Returns the approximate size of the struct as u32, given a number of entries in the @@ -553,6 +593,10 @@ pub struct OutboundLaneData { pub latest_received_nonce: MessageNonce, /// Nonce of the latest message, generated by us. pub latest_generated_nonce: MessageNonce, + /// The state of the bridged dispatcher, received during the last message delivery + /// confirmation transaction. May be `None` if we have never yet seen any confirmation + /// transactions. + pub bridged_dispatcher_state: Option, } impl OutboundLaneData { @@ -571,6 +615,7 @@ impl Default for OutboundLaneData { oldest_unpruned_nonce: 1, latest_received_nonce: 0, latest_generated_nonce: 0, + bridged_dispatcher_state: None, } } } @@ -672,6 +717,7 @@ mod tests { .into_iter() .collect(), last_confirmed_nonce: 0, + dispatcher_state: DispatcherState::default(), }; assert_eq!(lane_data.total_unrewarded_messages(), MessageNonce::MAX); } @@ -697,6 +743,7 @@ mod tests { }) .collect(), last_confirmed_nonce: messages_count as _, + dispatcher_state: DispatcherState::default(), } .encode() .len(); diff --git a/primitives/messages/src/target_chain.rs b/primitives/messages/src/target_chain.rs index faad3af88d..dd82703739 100644 --- a/primitives/messages/src/target_chain.rs +++ b/primitives/messages/src/target_chain.rs @@ -16,10 +16,12 @@ //! Primitives of messages module, that are used on the target chain. -use crate::{LaneId, Message, MessageKey, MessageNonce, MessagePayload, OutboundLaneData}; +use crate::{ + DispatcherState, LaneId, Message, MessageKey, MessageNonce, MessagePayload, OutboundLaneData, +}; -use bp_runtime::{messages::MessageDispatchResult, Size, UnverifiedStorageProof}; -use codec::{Decode, Encode, Error as CodecError}; +use bp_runtime::{Size, UnverifiedStorageProof}; +use codec::{Decode, Encode, Error as CodecError, MaxEncodedLen}; use frame_support::{weights::Weight, RuntimeDebug}; use scale_info::TypeInfo; use sp_std::{collections::btree_map::BTreeMap, fmt::Debug, marker::PhantomData, prelude::*}; @@ -84,6 +86,20 @@ pub struct DispatchMessage { pub data: DispatchMessageData, } +/// Message dispatch result. +#[derive(Encode, Decode, RuntimeDebug, Clone, PartialEq, Eq, TypeInfo, MaxEncodedLen)] +pub struct MessageDispatchResult { + /// Unspent dispatch weight. This weight that will be deducted from total delivery transaction + /// weight, thus reducing the transaction cost. This shall not be zero in (at least) two cases: + /// + /// 1) if message has been dispatched successfully, but post-dispatch weight is less than + /// the weight, declared by the message sender; + /// 2) if message has not been dispatched at all. + pub unspent_weight: Weight, + /// Fine-grained result of single message dispatch (for better diagnostic purposes) + pub dispatch_level_result: DispatchLevelResult, +} + /// Called when inbound message is received. pub trait MessageDispatch { /// Decoded message payload type. Valid message may contain invalid payload. In this case @@ -108,6 +124,9 @@ pub trait MessageDispatch { fn dispatch( message: DispatchMessage, ) -> MessageDispatchResult; + + /// Return message dispatcher state. + fn state() -> DispatcherState; } /// Manages payments that are happening at the target chain during message delivery transaction. @@ -181,4 +200,8 @@ impl MessageDispatch ) -> MessageDispatchResult { MessageDispatchResult { unspent_weight: Weight::zero(), dispatch_level_result: () } } + + fn state() -> DispatcherState { + DispatcherState::default() + } } diff --git a/primitives/runtime/src/lib.rs b/primitives/runtime/src/lib.rs index eb8fb4dead..88dcbf4692 100644 --- a/primitives/runtime/src/lib.rs +++ b/primitives/runtime/src/lib.rs @@ -43,7 +43,6 @@ pub use storage_proof::{StorageProofError, UnverifiedStorageProof, VerifiedStora pub use storage_types::BoundedStorageValue; pub mod extensions; -pub mod messages; mod chain; mod storage_proof; diff --git a/primitives/runtime/src/messages.rs b/primitives/runtime/src/messages.rs deleted file mode 100644 index 9f7c8ab5ca..0000000000 --- a/primitives/runtime/src/messages.rs +++ /dev/null @@ -1,35 +0,0 @@ -// Copyright 2019-2021 Parity Technologies (UK) Ltd. -// This file is part of Parity Bridges Common. - -// Parity Bridges Common is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. - -// Parity Bridges Common is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License -// along with Parity Bridges Common. If not, see . - -//! Primitives that may be used by different message delivery and dispatch mechanisms. - -use codec::{Decode, Encode}; -use frame_support::{weights::Weight, RuntimeDebug}; -use scale_info::TypeInfo; - -/// Message dispatch result. -#[derive(Encode, Decode, RuntimeDebug, Clone, PartialEq, Eq, TypeInfo)] -pub struct MessageDispatchResult { - /// Unspent dispatch weight. This weight that will be deducted from total delivery transaction - /// weight, thus reducing the transaction cost. This shall not be zero in (at least) two cases: - /// - /// 1) if message has been dispatched successfully, but post-dispatch weight is less than - /// the weight, declared by the message sender; - /// 2) if message has not been dispatched at all. - pub unspent_weight: Weight, - /// Fine-grained result of single message dispatch (for better diagnostic purposes) - pub dispatch_level_result: DispatchLevelResult, -} From 86a2c62b0aa0f9ddce05b4538e3e4f1c2fa7d051 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Thu, 29 Jun 2023 16:41:57 +0300 Subject: [PATCH 02/10] xcm-over-bridge-fee prototype --- Cargo.lock | 21 +++ Cargo.toml | 2 + modules/xcm-over-bridge-fee/Cargo.toml | 34 ++++ modules/xcm-over-bridge-fee/src/lib.rs | 180 ++++++++++++++++++++++ primitives/xcm-over-bridge-fee/Cargo.toml | 37 +++++ primitives/xcm-over-bridge-fee/src/lib.rs | 115 ++++++++++++++ 6 files changed, 389 insertions(+) create mode 100644 modules/xcm-over-bridge-fee/Cargo.toml create mode 100644 modules/xcm-over-bridge-fee/src/lib.rs create mode 100644 primitives/xcm-over-bridge-fee/Cargo.toml create mode 100644 primitives/xcm-over-bridge-fee/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index d9663485a3..e759890517 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1139,6 +1139,16 @@ dependencies = [ "sp-api", ] +[[package]] +name = "bp-xcm-over-bridge-fee" +version = "0.1.0" +dependencies = [ + "bp-messages", + "parity-scale-codec", + "scale-info", + "serde", +] + [[package]] name = "bridge-runtime-common" version = "0.1.0" @@ -7643,6 +7653,17 @@ dependencies = [ "xcm-executor", ] +[[package]] +name = "pallet-xcm-over-bridge-fee" +version = "0.1.0" +dependencies = [ + "bp-xcm-over-bridge-fee", + "frame-support", + "frame-system", + "parity-scale-codec", + "sp-std 8.0.0 (git+https://github.com/paritytech/substrate?branch=master)", +] + [[package]] name = "parachain-info" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index 8048b9fcdc..6cc9fdd96a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,6 +16,7 @@ members = [ "modules/parachains", "modules/relayers", "modules/shift-session-manager", + "modules/xcm-over-bridge-fee", "primitives/beefy", "primitives/chain-bridge-hub-cumulus", "primitives/chain-bridge-hub-kusama", @@ -37,6 +38,7 @@ members = [ "primitives/relayers", "primitives/runtime", "primitives/test-utils", + "primitives/xcm-over-bridge-fee", "relays/bin-substrate", "relays/client-bridge-hub-kusama", "relays/client-bridge-hub-polkadot", diff --git a/modules/xcm-over-bridge-fee/Cargo.toml b/modules/xcm-over-bridge-fee/Cargo.toml new file mode 100644 index 0000000000..ae3b91c7b6 --- /dev/null +++ b/modules/xcm-over-bridge-fee/Cargo.toml @@ -0,0 +1,34 @@ +[package] +name = "pallet-xcm-over-bridge-fee" +description = "Module that is able to adjust bridge message fee based on the bridge queues state." +version = "0.1.0" +authors = ["Parity Technologies "] +edition = "2021" +license = "GPL-3.0-or-later WITH Classpath-exception-2.0" + +[dependencies] +codec = { package = "parity-scale-codec", version = "3.1.5", default-features = false } + +# Bridge dependencies + +bp-xcm-over-bridge-fee = { path = "../../primitives/xcm-over-bridge-fee", default-features = false } + +# Substrate Dependencies + +frame-support = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } +frame-system = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } +sp-std = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } + +[features] +default = ["std"] +std = [ + "bp-xcm-over-bridge-fee/std", + "codec/std", + "frame-support/std", + "frame-system/std", + "sp-std/std", +] +try-runtime = [ + "frame-support/try-runtime", + "frame-system/try-runtime", +] diff --git a/modules/xcm-over-bridge-fee/src/lib.rs b/modules/xcm-over-bridge-fee/src/lib.rs new file mode 100644 index 0000000000..30324856f3 --- /dev/null +++ b/modules/xcm-over-bridge-fee/src/lib.rs @@ -0,0 +1,180 @@ +// Copyright 2019-2021 Parity Technologies (UK) Ltd. +// This file is part of Parity Bridges Common. + +// Parity Bridges Common is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Parity Bridges Common is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Parity Bridges Common. If not, see . + +//! Pallet that is able to compute over-bridge XCM message fee and automatically adjusts +//! it using bridge queues state. The pallet needs to be deployed at the sending chain, +//! sibling/parent to the bridge hub. + +#![cfg_attr(not(feature = "std"), no_std)] + +pub use pallet::*; + +#[frame_support::pallet] +pub mod pallet { + use super::*; + use frame_support::pallet_prelude::*; + use frame_system::pallet_prelude::*; + + #[pallet::config] + pub trait Config: frame_system::Config { + /// Bridge limits. + #[pallet::constant] + type BridgeLimits: Get; + + /// Initial `FeeFactor` value. + #[pallet::type_value] + pub fn InitialFeeFactor() -> FixedU128 { FixedU128::from_u32(1) } + + /// A way to send report requests to the sibling/child bridge hub. + type ReportBridgeStateRequestSender: ReportBridgeStateRequestSender; +/* /// Expected delay (in blocks) before we get bridge state response after + /// sending request. After this delay has passed, we consider request lost. + /// This eventually will result in unbounded message fee raise and can + /// be stopped only by the pallet controller. + type BridgeStateReportDelay: Get;*/ + } + + #[pallet::pallet] + pub struct Pallet(PhantomData<(T, I)>); + + #[pallet::hooks] + impl, I: 'static> Hooks> for Pallet { + fn on_initialize(_n: BlockNumberFor) -> Weight { + // if we have never sent bridge state report request or have sent it more than + // `bridge_state_report_delay` blocks ago and haven't yet received a response, we + // need to increase fee factor + let limits = T::BridgeLimits::get(); + let current_block = frame_system::Pallet::block_number(); + let request_sent_at = Self::report_bridge_state_request_sent_at::get(); + let missing_bridge_state_report = request_sent_at + .map(|request_sent_at| { + let expected_report_delay = limits.bridge_state_report_delay; + current_block.saturating_sub(request_sent_at) > expected_report_delay + }) + .unwrap_or(false); + + // if we believe there are more enqueued messages than during regular bridge operations, + // we need to increase fee factor + let queues_state = Self::bridge_queues_state().unwrap_or_default(); + let total_enqueued_messages = queues_state.total_enqueued_messages(); + let too_many_enqueued_messages = total_enqueued_messages > limits.increase_fee_factor_threshold; + + // remember that we need to increase fee factor or decrease it + if missing_bridge_state_report || too_many_enqueued_messages { + IncreaseFeeFactorOnSend::::set(true); + } else { + Self::decrease_fee_factor(); + } + + // now it's time to decide whether we want to send bridge state report request. We want + // to send it when we believe number of enqueued messages is close to increase-fee-factor + // threshold AND we don't have an active request + let close_to_too_many_enqueued_messages = total_enqueued_messages > limits.send_report_bridge_state_threshold; + if request_sent_at.is_none() && close_to_too_many_enqueued_messages { + // TODO: even if `ReportBridgeStateRequestSentAt` is `None`, we may want to + T::ReportBridgeStateRequestSender::send(); + ReportBridgeStateRequestSentAt::set(current_block); + } + + // TODO: use benchmarks + Weight::zero() + } + + fn on_finalize(_n: BlockNumberFor) { + IncreaseFeeFactorOnSend::::kill(); + } + } + + #[pallet::call] + impl, I: 'static> Pallet { + #[pallet::call_index(0)] + #[pallet::weight(Weight::zero())] + pub fn receive_bridge_state_report( + _origin: OriginFor, + _at_bridge_hub_queues_state: AtBridgeHubBridgeQueuesState, + ) -> DispatchResultWithPostInfo { + // TODO: check origin - it must be either parachain, or some controller account + // TODO: convert `at_bridge_hub_queues_state` to `BridgeQueuesState` + // TODO: kill `ReportBridgeStateRequestSentAt` + + // we have receoved state report and may kill the `ReportBridgeStateRequestSentAt` value. + // This means that at the next block we may send another report request and hopefully bridge + // will + ReportBridgeStateRequestSentAt::::kill(); + } + } + + impl, I: 'static> Pallet { + /// Returns fee that needs to be paid for sending message of given size. + /// + /// This function also increases the fee factor if there's a lof ot enqueued messages + /// across all bridge queues. + pub fn price_for_message_delivery(message_size: u32) -> FixedU128 { + let fee_factor = if IncreaseFeeFactorOnSend::get() { + let message_size_factor = FixedU128::from_u32(message_size.saturating_div(1024) as u32) + .saturating_mul(MESSAGE_SIZE_FEE_BASE); + FeeFactor::::mutate(|f| { + *f = f.saturating_mul(EXPONENTIAL_FEE_BASE + message_size_factor); + *f + }) + } else { + FeeFactor::::get() + }; + + let message_fee = (mesage_size as u128).saturating_mul(T::MessageByteFee::get()); + let fee_sum = T::MessageBaseFee::get().saturating_add(message_fee); + let price = fee_factor.saturating_mul_int(fee_sum); + + price + } + + /// Decrement fee factor, making messages cheaper. This is called by the pallet itself + /// at the beginning of every block. + fn decrement_fee_factor() -> FixedU128 { + FeeFactor::::mutate(|f| { + *f = InitialFeeFactor::get().max(*f / EXPONENTIAL_FEE_BASE); + *f + }) + } + } + + // TODO: what's the chance that this parachain will have bridges with multiple chains at the bridged + // side? This can be handled either by converting `StorageValue` below to `StorageMap`s or by adding + // another instances of this pallet. The latter looks more clear, but if that will be a usual case, + // the maps option is dynamic-friendly. + + /// Ephemeral value that is set to true when we need to increase fee factor when sending message + /// over the bridge. + #[pallet::storage] + #[pallet::whitelist_storage] + #[pallet::getter(fn increase_fee_factor_on_send)] + pub type IncreaseFeeFactorOnSend, I: 'static = ()> = StorageValue<_, bool, ValueQuery>; + + /// The number to multiply the base delivery fee by. + #[pallet::storage] + #[pallet::getter(fn fee_factor)] + pub type FeeFactor, I: 'static = ()> = StorageValue<_, T::Balance, ValueQuery, InitialFactor>; + + /// Last known bridge queues state. + #[pallet::storage] + #[pallet::getter(fn bridge_queues_state)] + pub type BridgeQueuesState, I: 'static = ()> = StorageValue<_, BridgeQueuesState, OptionQuery>; + + /// The block at which we have sent request for new bridge queues state. + #[pallet::storage] + #[pallet::getter(fn report_bridge_state_request_sent_at)] + pub type ReportBridgeStateRequestSentAt, I: 'static = ()> = StorageValue<_, T::BlockNumber, OptionQuery>; +} diff --git a/primitives/xcm-over-bridge-fee/Cargo.toml b/primitives/xcm-over-bridge-fee/Cargo.toml new file mode 100644 index 0000000000..e4868c68a8 --- /dev/null +++ b/primitives/xcm-over-bridge-fee/Cargo.toml @@ -0,0 +1,37 @@ +[package] +name = "bp-xcm-over-bridge-fee" +description = "Primitives of the xcm-over-bridge fee pallet." +version = "0.1.0" +authors = ["Parity Technologies "] +edition = "2021" +license = "GPL-3.0-or-later WITH Classpath-exception-2.0" + +[dependencies] +codec = { package = "parity-scale-codec", version = "3.1.5", default-features = false, features = ["derive", "bit-vec"] } +scale-info = { version = "2.8.0", default-features = false, features = ["bit-vec", "derive", "serde"] } +serde = { version = "1.0", default-features = false, features = ["alloc", "derive"] } + +# Bridge dependencies + +bp-messages = { path = "../messages", default-features = false } + +# Substrate Dependencies + +#frame-support = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } +#sp-core = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } +#sp-io = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } +#sp-std = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } + +[features] +default = ["std"] +std = [ + "bp-messages/std", +# "bp-header-chain/std", + "codec/std", +# "frame-support/std", + "scale-info/std", + "serde/std", +# "sp-core/std", +# "sp-io/std", +# "sp-std/std" +] diff --git a/primitives/xcm-over-bridge-fee/src/lib.rs b/primitives/xcm-over-bridge-fee/src/lib.rs new file mode 100644 index 0000000000..f9e50b1a15 --- /dev/null +++ b/primitives/xcm-over-bridge-fee/src/lib.rs @@ -0,0 +1,115 @@ +// Copyright 2019-2021 Parity Technologies (UK) Ltd. +// This file is part of Parity Bridges Common. + +// Parity Bridges Common is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Parity Bridges Common is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Parity Bridges Common. If not, see . + +//! Primitives of the `xcm-over-bridge-fee` pallet. + +#![cfg_attr(not(feature = "std"), no_std)] + +use bp_messages::MessageNonce; +use codec::{Decode, Encode, MaxEncodedLen}; +use scale_info::TypeInfo; +use serde::{Deserialize, Serialize}; + +/// Bridge limits. +pub struct BridgeLimits { + /// Maximal delay in blocks before we expect to receive bridge state report. If we have + /// issued request `bridge_state_report_delay + 1` blocks ago and still have not received + /// report, we consider that something is wrong with the pipeline and start increasing + /// message fee. + pub bridge_state_report_delay: BlockNumber, + /// Maximal allowed number of queued messages across all bridge queues. If number of messages + /// is larger than this threshold, every following message + pub increase_fee_factor_threshold: MessageNonce, + /// + pub send_report_bridge_state_threshold: MessageNonce, +} + +/// All bridge queues state. +#[derive( + Clone, + Copy, + Decode, + Default, + Encode, + Eq, + Ord, + PartialOrd, + PartialEq, + TypeInfo, + MaxEncodedLen, +)] +pub struct BridgeQueuesState { + /// Number of messages queued at this (source) chain' outbound queue. + /// + /// That's the only field that is filled at this chain, because sibling (source) + /// bridge hub doesn't have an access to our queue. + pub outbound_here: MessageNonce, + /// Status of queues, that we have received from the bridge hub. + pub at_bridge_hub: AtBridgeHubBridgeQueuesState, +} + +impl BridgeQueuesState { + /// Return total number of messsages that we assume are currently in the bridges queue. + pub fn total_enqueued_messages(&self) -> MessageNonce { + self.outbound_here.saturating_add(self.at_bridge_hub.total_enqueued_messages()) + } +} + +/// All bridge queues state. +#[derive( + Clone, + Copy, + Decode, + Default, + Encode, + Eq, + Ord, + PartialOrd, + PartialEq, + TypeInfo, + MaxEncodedLen, + Serialize, + Deserialize, +)] +pub struct AtBridgeHubBridgeQueuesState { + /// Number of messages queued at sibling (source) bridge hub inbound queue. + pub inbound_at_sibling: MessageNonce, + /// Number of messages queued at sibling (source) bridge hub outbound queue. + pub outbound_at_sibling: MessageNonce, + /// Number of messages queued at bridged (target) bridge hub outbound queue. + pub outbound_at_bridged: MessageNonce, + /// Number of messages queued at the destination inbound queue. + /// + /// Bridged (target) bridge hub doesn't have an access to the exact value of + /// this metric. But it may get an estimation, depending on the channel + /// state. The channel between target brige hub and desination is suspended + /// when there are more than `N` unprocessed messages at the destination inbound + /// queue. So if we see the suspended channel state at the target bridge hub, + /// we: (1) assume that there's at least `N` queued messages at the inbound + /// destination queue and (2) all further messages are now piling up at our + /// outbound queue (`outbound_at_bridged`), so we have exact count. + pub inbound_at_destination: MessageNonce, +} + +impl AtBridgeHubBridgeQueuesState { + /// Return total number of messsages that we assume are currently in the bridges queue. + pub fn total_enqueued_messages(&self) -> MessageNonce { + self.inbound_at_sibling + .saturating_add(self.outbound_at_sibling) + .saturating_add(self.outbound_at_bridged) + .saturating_add(self.inbound_at_destination) + } +} From 2c95fa518bd07e155645c21d86287efbe26dad7e Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Fri, 30 Jun 2023 11:04:44 +0300 Subject: [PATCH 03/10] flush some comments --- modules/xcm-over-bridge-fee/src/lib.rs | 24 +++++++++++++++-------- primitives/xcm-over-bridge-fee/src/lib.rs | 8 ++++++-- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/modules/xcm-over-bridge-fee/src/lib.rs b/modules/xcm-over-bridge-fee/src/lib.rs index 30324856f3..2f6e57ee76 100644 --- a/modules/xcm-over-bridge-fee/src/lib.rs +++ b/modules/xcm-over-bridge-fee/src/lib.rs @@ -40,11 +40,6 @@ pub mod pallet { /// A way to send report requests to the sibling/child bridge hub. type ReportBridgeStateRequestSender: ReportBridgeStateRequestSender; -/* /// Expected delay (in blocks) before we get bridge state response after - /// sending request. After this delay has passed, we consider request lost. - /// This eventually will result in unbounded message fee raise and can - /// be stopped only by the pallet controller. - type BridgeStateReportDelay: Get;*/ } #[pallet::pallet] @@ -84,8 +79,9 @@ pub mod pallet { // threshold AND we don't have an active request let close_to_too_many_enqueued_messages = total_enqueued_messages > limits.send_report_bridge_state_threshold; if request_sent_at.is_none() && close_to_too_many_enqueued_messages { - // TODO: even if `ReportBridgeStateRequestSentAt` is `None`, we may want to - T::ReportBridgeStateRequestSender::send(); + // TODO: even if `ReportBridgeStateRequestSentAt` is `None`, we may want to delay next + // request to avoid being too frequent + T::ReportBridgeStateRequestSender::send(current_block); ReportBridgeStateRequestSentAt::set(current_block); } @@ -104,11 +100,15 @@ pub mod pallet { #[pallet::weight(Weight::zero())] pub fn receive_bridge_state_report( _origin: OriginFor, + _request_sent_at: T::BlockNumber, _at_bridge_hub_queues_state: AtBridgeHubBridgeQueuesState, ) -> DispatchResultWithPostInfo { // TODO: check origin - it must be either parachain, or some controller account // TODO: convert `at_bridge_hub_queues_state` to `BridgeQueuesState` // TODO: kill `ReportBridgeStateRequestSentAt` + // TODO: we shall only accept response for our last request - i.e. if something + // will go wrong and controller wil use forced report BUT then some old + // report will come, then we need to avoid it // we have receoved state report and may kill the `ReportBridgeStateRequestSentAt` value. // This means that at the next block we may send another report request and hopefully bridge @@ -176,5 +176,13 @@ pub mod pallet { /// The block at which we have sent request for new bridge queues state. #[pallet::storage] #[pallet::getter(fn report_bridge_state_request_sent_at)] - pub type ReportBridgeStateRequestSentAt, I: 'static = ()> = StorageValue<_, T::BlockNumber, OptionQuery>; + pub type ReportBrReportBridgeStateRequestSentAtidgeStateRequestSentAt, I: 'static = ()> = StorageValue<_, T::BlockNumber, OptionQuery>; } + +// TODO: ideally, we need to update fee factor when the message is actually sent. This is the `SendXcm`. But for +// bridges there's `ExporterFor`, which shall return fee. But it is detached from the sending process (and result). +// Possible solution is to implement both `ExporterFor` and `SendXcm` for this pallet and place if **before** +// regular XCMP in the router configuration - then we will be able to increase fee only when the message is +// actually sent. + +// TODO: what about dynamic bridges - will bridge hub have some logic (barrier) to only allow paid execution? diff --git a/primitives/xcm-over-bridge-fee/src/lib.rs b/primitives/xcm-over-bridge-fee/src/lib.rs index f9e50b1a15..fbfb57c88b 100644 --- a/primitives/xcm-over-bridge-fee/src/lib.rs +++ b/primitives/xcm-over-bridge-fee/src/lib.rs @@ -24,6 +24,7 @@ use scale_info::TypeInfo; use serde::{Deserialize, Serialize}; /// Bridge limits. +#[derive(Clone, RuntimeDebug)] pub struct BridgeLimits { /// Maximal delay in blocks before we expect to receive bridge state report. If we have /// issued request `bridge_state_report_delay + 1` blocks ago and still have not received @@ -31,9 +32,9 @@ pub struct BridgeLimits { /// message fee. pub bridge_state_report_delay: BlockNumber, /// Maximal allowed number of queued messages across all bridge queues. If number of messages - /// is larger than this threshold, every following message + /// is larger than this threshold, every following message will lead to fee increase. pub increase_fee_factor_threshold: MessageNonce, - /// + /// Maximal number of queued messages before we will start pub send_report_bridge_state_threshold: MessageNonce, } @@ -48,10 +49,12 @@ pub struct BridgeLimits { Ord, PartialOrd, PartialEq, + RuntimeDebug, TypeInfo, MaxEncodedLen, )] pub struct BridgeQueuesState { + /// Total number of messages that have been sent since last bridge state /// Number of messages queued at this (source) chain' outbound queue. /// /// That's the only field that is filled at this chain, because sibling (source) @@ -79,6 +82,7 @@ impl BridgeQueuesState { Ord, PartialOrd, PartialEq, + RuntimeDebug, TypeInfo, MaxEncodedLen, Serialize, From fc1011cbdda474f6fc85c580b3e7f018a3083c31 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Fri, 30 Jun 2023 13:07:34 +0300 Subject: [PATCH 04/10] flush --- modules/xcm-over-bridge-fee/src/lib.rs | 104 +++++++++++++++++-------- 1 file changed, 71 insertions(+), 33 deletions(-) diff --git a/modules/xcm-over-bridge-fee/src/lib.rs b/modules/xcm-over-bridge-fee/src/lib.rs index 2f6e57ee76..9f2906636b 100644 --- a/modules/xcm-over-bridge-fee/src/lib.rs +++ b/modules/xcm-over-bridge-fee/src/lib.rs @@ -14,9 +14,11 @@ // You should have received a copy of the GNU General Public License // along with Parity Bridges Common. If not, see . -//! Pallet that is able to compute over-bridge XCM message fee and automatically adjusts -//! it using bridge queues state. The pallet needs to be deployed at the sending chain, -//! sibling/parent to the bridge hub. +//! Pallet that may be used instead of `SovereignPaidRemoteExporter` in the XCM router +//! configuration. The main thing that the pallet offers is the dynamic message fee, +//! that is computed based on the bridge queues state. It starts exponentially increasing +//! if there are too many messages stuck across all bridge queues. When situation is +//! back to normal, the fee starts decreasing. #![cfg_attr(not(feature = "std"), no_std)] @@ -38,8 +40,12 @@ pub mod pallet { #[pallet::type_value] pub fn InitialFeeFactor() -> FixedU128 { FixedU128::from_u32(1) } - /// A way to send report requests to the sibling/child bridge hub. - type ReportBridgeStateRequestSender: ReportBridgeStateRequestSender; + /// Child/sibling bridge hub configuration. + type BridgeHub: BridgeHub; + + /// Underlying transport (XCMP or DMP) which allows sending messages to the child/sibling + /// bridge hub. + type ToBridgeHubSender: SendXcm; } #[pallet::pallet] @@ -118,29 +124,6 @@ pub mod pallet { } impl, I: 'static> Pallet { - /// Returns fee that needs to be paid for sending message of given size. - /// - /// This function also increases the fee factor if there's a lof ot enqueued messages - /// across all bridge queues. - pub fn price_for_message_delivery(message_size: u32) -> FixedU128 { - let fee_factor = if IncreaseFeeFactorOnSend::get() { - let message_size_factor = FixedU128::from_u32(message_size.saturating_div(1024) as u32) - .saturating_mul(MESSAGE_SIZE_FEE_BASE); - FeeFactor::::mutate(|f| { - *f = f.saturating_mul(EXPONENTIAL_FEE_BASE + message_size_factor); - *f - }) - } else { - FeeFactor::::get() - }; - - let message_fee = (mesage_size as u128).saturating_mul(T::MessageByteFee::get()); - let fee_sum = T::MessageBaseFee::get().saturating_add(message_fee); - let price = fee_factor.saturating_mul_int(fee_sum); - - price - } - /// Decrement fee factor, making messages cheaper. This is called by the pallet itself /// at the beginning of every block. fn decrement_fee_factor() -> FixedU128 { @@ -155,6 +138,8 @@ pub mod pallet { // side? This can be handled either by converting `StorageValue` below to `StorageMap`s or by adding // another instances of this pallet. The latter looks more clear, but if that will be a usual case, // the maps option is dynamic-friendly. + // + // We need to use single pallet for dynamic bridges and for dynamic fee support at source parachains. /// Ephemeral value that is set to true when we need to increase fee factor when sending message /// over the bridge. @@ -179,10 +164,63 @@ pub mod pallet { pub type ReportBrReportBridgeStateRequestSentAtidgeStateRequestSentAt, I: 'static = ()> = StorageValue<_, T::BlockNumber, OptionQuery>; } -// TODO: ideally, we need to update fee factor when the message is actually sent. This is the `SendXcm`. But for -// bridges there's `ExporterFor`, which shall return fee. But it is detached from the sending process (and result). -// Possible solution is to implement both `ExporterFor` and `SendXcm` for this pallet and place if **before** -// regular XCMP in the router configuration - then we will be able to increase fee only when the message is -// actually sent. +/// We'll be using `SovereignPaidRemoteExporter` to send remote messages over the sibling/child bridge hub. +type ViaBridgeHubExporter = SovereignPaidRemoteExporter< + Pallet, + >::ToBridgeHubSender, + T as Config>::UniversalLocation, +>; + +impl, I: 'static> ExporterFor for Pallet { + fn exporter_for( + network: &NetworkId, + _: &InteriorMultiLocation, + message: &Xcm<()>, + ) -> Option<(MultiLocation, Option)> { + // ensure that the message is sent to the expected bridged network + if *network != T::BridgedNetworkId::get() { + return None; + } + + // compute fee amount + let mesage_size = message.encoded_size(); + let message_fee = (mesage_size as u128).saturating_mul(T::MessageByteFee::get()); + let fee_sum = T::MessageBaseFee::get().saturating_add(message_fee); + let fee = fee_factor.saturating_mul_int(fee_sum); + + Some((T::LocalBridgeHubLocation::get(), (T::FeeAsset::get(), fee).into())) + } +} + +impl, I: 'static> SendXcm for Pallet { + type Ticket = (u32, T::ToBridgeHubSender::Ticket); + + fn validate( + dest: &mut Option, + xcm: &mut Option>, + ) -> SendResult { + // just use exporter to validate destination and insert instructions to pay message fee + // at the sibling/child bridge hub + let message_size = xcm.as_ref().map(|xcm| xcm.encoded_size()); + ViaBridgeHubExporter::::validate(dest, xcm).map(|ticket| (message_size, ticket)) + } + + fn deliver(ticket: (u32, Router::Ticket)) -> Result { + // 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)?; + + // the message has been validated by us, so we know that it heads to the remote network. + // So we may increase our fee factor here, if required. + if IncreaseFeeFactorOnSend::::get() { + let message_size_factor = FixedU128::from_u32(message_size.saturating_div(1024) as u32) + .saturating_mul(MESSAGE_SIZE_FEE_BASE); + FeeFactor::::mutate(|f| { + *f = f.saturating_mul(EXPONENTIAL_FEE_BASE + message_size_factor); + }); + } + } +} // TODO: what about dynamic bridges - will bridge hub have some logic (barrier) to only allow paid execution? From c1edc0a02c9211e94f9eb9bc697299607270ce42 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Fri, 30 Jun 2023 13:37:58 +0300 Subject: [PATCH 05/10] renamed pallet to -xcm-bridge-hub-router --- Cargo.toml | 4 ++-- .../Cargo.toml | 8 ++++---- .../src/lib.rs | 0 .../Cargo.toml | 4 ++-- .../src/lib.rs | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) rename modules/{xcm-over-bridge-fee => xcm-bridge-hub-router}/Cargo.toml (75%) rename modules/{xcm-over-bridge-fee => xcm-bridge-hub-router}/src/lib.rs (100%) rename primitives/{xcm-over-bridge-fee => xcm-bridge-hub-router}/Cargo.toml (92%) rename primitives/{xcm-over-bridge-fee => xcm-bridge-hub-router}/src/lib.rs (98%) diff --git a/Cargo.toml b/Cargo.toml index 6cc9fdd96a..ec92cddb07 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,7 +16,7 @@ members = [ "modules/parachains", "modules/relayers", "modules/shift-session-manager", - "modules/xcm-over-bridge-fee", + "modules/xcm-bridge-hub-router", "primitives/beefy", "primitives/chain-bridge-hub-cumulus", "primitives/chain-bridge-hub-kusama", @@ -38,7 +38,7 @@ members = [ "primitives/relayers", "primitives/runtime", "primitives/test-utils", - "primitives/xcm-over-bridge-fee", + "primitives/xcm-bridge-hub-router", "relays/bin-substrate", "relays/client-bridge-hub-kusama", "relays/client-bridge-hub-polkadot", diff --git a/modules/xcm-over-bridge-fee/Cargo.toml b/modules/xcm-bridge-hub-router/Cargo.toml similarity index 75% rename from modules/xcm-over-bridge-fee/Cargo.toml rename to modules/xcm-bridge-hub-router/Cargo.toml index ae3b91c7b6..c971e28863 100644 --- a/modules/xcm-over-bridge-fee/Cargo.toml +++ b/modules/xcm-bridge-hub-router/Cargo.toml @@ -1,6 +1,6 @@ [package] -name = "pallet-xcm-over-bridge-fee" -description = "Module that is able to adjust bridge message fee based on the bridge queues state." +name = "pallet-xcm-bridge-hub-router" +description = "Module that supports dynamic bridges over sibling/child bridge hub." version = "0.1.0" authors = ["Parity Technologies "] edition = "2021" @@ -11,7 +11,7 @@ codec = { package = "parity-scale-codec", version = "3.1.5", default-features = # Bridge dependencies -bp-xcm-over-bridge-fee = { path = "../../primitives/xcm-over-bridge-fee", default-features = false } +bp-xcm-bridge-hub-router = { path = "../../primitives/xcm-bridge-hub-router", default-features = false } # Substrate Dependencies @@ -22,7 +22,7 @@ sp-std = { git = "https://github.com/paritytech/substrate", branch = "master", d [features] default = ["std"] std = [ - "bp-xcm-over-bridge-fee/std", + "bp-xcm-bridge-hub-router/std", "codec/std", "frame-support/std", "frame-system/std", diff --git a/modules/xcm-over-bridge-fee/src/lib.rs b/modules/xcm-bridge-hub-router/src/lib.rs similarity index 100% rename from modules/xcm-over-bridge-fee/src/lib.rs rename to modules/xcm-bridge-hub-router/src/lib.rs diff --git a/primitives/xcm-over-bridge-fee/Cargo.toml b/primitives/xcm-bridge-hub-router/Cargo.toml similarity index 92% rename from primitives/xcm-over-bridge-fee/Cargo.toml rename to primitives/xcm-bridge-hub-router/Cargo.toml index e4868c68a8..cfa746fe56 100644 --- a/primitives/xcm-over-bridge-fee/Cargo.toml +++ b/primitives/xcm-bridge-hub-router/Cargo.toml @@ -1,6 +1,6 @@ [package] -name = "bp-xcm-over-bridge-fee" -description = "Primitives of the xcm-over-bridge fee pallet." +name = "bp-xcm-bridge-hub-router" +description = "Primitives of the xcm-bridge-hub fee pallet." version = "0.1.0" authors = ["Parity Technologies "] edition = "2021" diff --git a/primitives/xcm-over-bridge-fee/src/lib.rs b/primitives/xcm-bridge-hub-router/src/lib.rs similarity index 98% rename from primitives/xcm-over-bridge-fee/src/lib.rs rename to primitives/xcm-bridge-hub-router/src/lib.rs index fbfb57c88b..a51adb4a30 100644 --- a/primitives/xcm-over-bridge-fee/src/lib.rs +++ b/primitives/xcm-bridge-hub-router/src/lib.rs @@ -14,7 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Parity Bridges Common. If not, see . -//! Primitives of the `xcm-over-bridge-fee` pallet. +//! Primitives of the `xcm-bridge-hub-router` pallet. #![cfg_attr(not(feature = "std"), no_std)] From 5066a731488094626edd6a87eaaa6991b0edebb3 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Fri, 30 Jun 2023 15:10:25 +0300 Subject: [PATCH 06/10] flush --- modules/xcm-bridge-hub-router/src/lib.rs | 126 ++++++++++++++------ primitives/xcm-bridge-hub-router/src/lib.rs | 38 +++++- 2 files changed, 125 insertions(+), 39 deletions(-) diff --git a/modules/xcm-bridge-hub-router/src/lib.rs b/modules/xcm-bridge-hub-router/src/lib.rs index 9f2906636b..a052437cb0 100644 --- a/modules/xcm-bridge-hub-router/src/lib.rs +++ b/modules/xcm-bridge-hub-router/src/lib.rs @@ -36,10 +36,6 @@ pub mod pallet { #[pallet::constant] type BridgeLimits: Get; - /// Initial `FeeFactor` value. - #[pallet::type_value] - pub fn InitialFeeFactor() -> FixedU128 { FixedU128::from_u32(1) } - /// Child/sibling bridge hub configuration. type BridgeHub: BridgeHub; @@ -54,6 +50,9 @@ pub mod pallet { #[pallet::hooks] impl, I: 'static> Hooks> for Pallet { fn on_initialize(_n: BlockNumberFor) -> Weight { + // TODO: since this pallet now supports dynamic bridges, we need to move this code + // to e.g. deliver or at least schedule everything required from there + // if we have never sent bridge state report request or have sent it more than // `bridge_state_report_delay` blocks ago and haven't yet received a response, we // need to increase fee factor @@ -134,34 +133,9 @@ pub mod pallet { } } - // TODO: what's the chance that this parachain will have bridges with multiple chains at the bridged - // side? This can be handled either by converting `StorageValue` below to `StorageMap`s or by adding - // another instances of this pallet. The latter looks more clear, but if that will be a usual case, - // the maps option is dynamic-friendly. - // - // We need to use single pallet for dynamic bridges and for dynamic fee support at source parachains. - - /// Ephemeral value that is set to true when we need to increase fee factor when sending message - /// over the bridge. - #[pallet::storage] - #[pallet::whitelist_storage] - #[pallet::getter(fn increase_fee_factor_on_send)] - pub type IncreaseFeeFactorOnSend, I: 'static = ()> = StorageValue<_, bool, ValueQuery>; - - /// The number to multiply the base delivery fee by. - #[pallet::storage] - #[pallet::getter(fn fee_factor)] - pub type FeeFactor, I: 'static = ()> = StorageValue<_, T::Balance, ValueQuery, InitialFactor>; - - /// Last known bridge queues state. + /// All registered bridges. #[pallet::storage] - #[pallet::getter(fn bridge_queues_state)] - pub type BridgeQueuesState, I: 'static = ()> = StorageValue<_, BridgeQueuesState, OptionQuery>; - - /// The block at which we have sent request for new bridge queues state. - #[pallet::storage] - #[pallet::getter(fn report_bridge_state_request_sent_at)] - pub type ReportBrReportBridgeStateRequestSentAtidgeStateRequestSentAt, I: 'static = ()> = StorageValue<_, T::BlockNumber, OptionQuery>; + pub type Bridges, I: 'static = ()> = StorageMap<_, Identity, LaneId, BridgeOf>; } /// We'll be using `SovereignPaidRemoteExporter` to send remote messages over the sibling/child bridge hub. @@ -171,6 +145,8 @@ type ViaBridgeHubExporter = SovereignPaidRemoteExporter< T as Config>::UniversalLocation, >; +// This pallet acts as the `ExporterFor` for the `SovereignPaidRemoteExporter` to compute +// message fee using fee factor. impl, I: 'static> ExporterFor for Pallet { fn exporter_for( network: &NetworkId, @@ -183,6 +159,7 @@ impl, I: 'static> ExporterFor for Pallet { } // compute fee amount + let bridge = Bridges::::get(); let mesage_size = message.encoded_size(); let message_fee = (mesage_size as u128).saturating_mul(T::MessageByteFee::get()); let fee_sum = T::MessageBaseFee::get().saturating_add(message_fee); @@ -192,8 +169,11 @@ impl, I: 'static> ExporterFor for Pallet { } } +// This pallet acts as the `SendXcm` to the sibling/child bridge hub instead of regular +// XCMP/DMP transport. This allows injecting dynamic message fees into XCM programs that +// are going to the bridged network. impl, I: 'static> SendXcm for Pallet { - type Ticket = (u32, T::ToBridgeHubSender::Ticket); + type Ticket = (LaneId, u32, T::ToBridgeHubSender::Ticket); fn validate( dest: &mut Option, @@ -201,26 +181,96 @@ impl, I: 'static> SendXcm for Pallet { ) -> SendResult { // just use exporter to validate destination and insert instructions to pay message fee // at the sibling/child bridge hub + let lane_id = LaneId::new(UniversalLocation::get(), ); let message_size = xcm.as_ref().map(|xcm| xcm.encoded_size()); - ViaBridgeHubExporter::::validate(dest, xcm).map(|ticket| (message_size, ticket)) + ViaBridgeHubExporter::::validate(dest, xcm).map(|ticket| (lane_id, message_size, ticket)) } fn deliver(ticket: (u32, Router::Ticket)) -> Result { // 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 (lane_id, message_size, ticket) = ticket; let xcm_hash = T::ToBridgeHubSender::deliver(ticket)?; - // the message has been validated by us, so we know that it heads to the remote network. - // So we may increase our fee factor here, if required. - if IncreaseFeeFactorOnSend::::get() { + // let's check if we need to increase fee for the further messages + let limits = T::BridgeLimits::get(); + let current_block = frame_system::Pallet::block_number(); + let state = BridgeState::::get(lane_id); + let limits = BridgeLimits::get(); + if is_fee_increment_required(current_block, &state, &limits) { let message_size_factor = FixedU128::from_u32(message_size.saturating_div(1024) as u32) .saturating_mul(MESSAGE_SIZE_FEE_BASE); FeeFactor::::mutate(|f| { *f = f.saturating_mul(EXPONENTIAL_FEE_BASE + message_size_factor); }); } + + // let's check if we need to send bridge state report request + if is_state_report_required(current_block, &state, &limits) { + ScheduledStateReportRequests::::append(lane_id); + } + + Ok(xcm_hash) } } -// TODO: what about dynamic bridges - will bridge hub have some logic (barrier) to only allow paid execution? +/// Returns `true` if bridge has exceeded its limits and operates with a lower than +/// expected performance. It means that we need to set higher fee for messages that +/// are sent over the bridge to avoid further throughput drops. +fn is_fee_increment_required( + current_block: BlockNumber, + state: &BridgeState, + limits: &BridgeLimits, +) -> bool { + // if there are more messages than we expect, we need to increase fee + if state.total_enqueued_messages > limits.increase_fee_factor_threshold { + return true; + } + + // if we are waiting for the report for too long, we need to increase fee + let current_delay = state.last_report_request_block.map(|b| b.saturating_sub(current_block)).unwrap_or_else(Zero::zero); + if current_delay > limits.maximal_bridge_state_report_delay { + return true; + } + + false +} + +/// Returns `true` if we need new status report from the bridge hub. +fn is_state_report_required( + current_block: BlockNumber, + state: &BridgeState, + limits: &BridgeLimits, +) -> bool { + // we never issue multiple report requests + if state.last_report_request_block.is_some() { + return false; + } + + // we don't need new request if we believe there aren't much messages in the queue + if state.total_enqueued_messages <= limits.send_report_bridge_state_threshold { + return false; + } + + // we don't need new request if we have received report recently + if current_block.saturating_sub(state.last_report_block) < limits.minimal_bridge_state_request_delay { + return false + } + + true +} + +// TODO: move this function to the bp-messages or bp-runtime - there's similar function in the pallet-xcm-bridge-hub. +/// Given remote (bridged) network id and interior location within that network, return +/// lane identifier that will be used to bridge with local `local_universal_location`. +/// +/// We will assume that remote location is either relay chain network, or its parachain. +/// All lower-level junctions (accounts, pallets and so on) are dropped, because they +/// will be handled by the XCM router at selected remote relay/para -chain. +pub fn bridge_lane_id( + _bridged_network: NetworkId, + _bridged_location: InteriorMultiLocation, + _local_universal_location: MultiLocation, +) -> LaneId { + unimplemented!("TODO: reuse function from the pallet-xcm-bridge-hub") +} diff --git a/primitives/xcm-bridge-hub-router/src/lib.rs b/primitives/xcm-bridge-hub-router/src/lib.rs index a51adb4a30..1a5191cbc0 100644 --- a/primitives/xcm-bridge-hub-router/src/lib.rs +++ b/primitives/xcm-bridge-hub-router/src/lib.rs @@ -30,7 +30,9 @@ pub struct BridgeLimits { /// issued request `bridge_state_report_delay + 1` blocks ago and still have not received /// report, we consider that something is wrong with the pipeline and start increasing /// message fee. - pub bridge_state_report_delay: BlockNumber, + pub maximal_bridge_state_report_delay: BlockNumber, + /// Minimal delay in blocks between our state report requests. + pub minimal_bridge_state_request_delay: BlockNumber, /// Maximal allowed number of queued messages across all bridge queues. If number of messages /// is larger than this threshold, every following message will lead to fee increase. pub increase_fee_factor_threshold: MessageNonce, @@ -71,6 +73,7 @@ impl BridgeQueuesState { } } +// TODO: this should be moved to `bp-xcm-bridge-hub`. /// All bridge queues state. #[derive( Clone, @@ -117,3 +120,36 @@ impl AtBridgeHubBridgeQueuesState { .saturating_add(self.inbound_at_destination) } } + +/// Current state of bridge with the remote deestination. +pub struct Bridge { + /// The number to multiply the base message delivery fee by. We will increase this + /// value exponentially when we the bridge throughput decreases and decrease after + /// it is back to normal. + pub fee_factor: FixedU128, + /// Count of undelivered bridge messages as we see it. The actual number may be lower + /// if some messages are already delivered, but we have not yet received a report. + /// The actual number may be higher e.g. if previous report estimation was incorrect + /// or if some messages have not yet been accounted by the report. + /// + /// This field is incremented by one every time we send a message. This field is changed + /// to the reported number every time we receive a state report. + pub total_enqueued_messages: MessageNonce, + /// The number of block, at which we have received last bridge state report. If we have + /// never received a report, it is set to zero. + pub last_report_block: BlockNumber, + /// The number of block, at which we have sent our last bridge state report request. Set + /// to `None` if we have no active report request. + pub last_report_request_block: Option, +} + +impl Default for BridgeState { + fn default() -> Self { + BridgeState { + fee_factor: FixedU128::from_u32(1), + total_enqueued_messages: 0, + last_report_block: Zero::zero(), + last_report_request_block: None, + } + } +} From 2fdc45e0366beb69c9d7d01753eee0ac897c689a Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Fri, 30 Jun 2023 15:50:41 +0300 Subject: [PATCH 07/10] flush --- modules/xcm-bridge-hub-router/src/lib.rs | 109 +++++++------------- primitives/xcm-bridge-hub-router/src/lib.rs | 8 +- 2 files changed, 46 insertions(+), 71 deletions(-) diff --git a/modules/xcm-bridge-hub-router/src/lib.rs b/modules/xcm-bridge-hub-router/src/lib.rs index a052437cb0..043ae4fa76 100644 --- a/modules/xcm-bridge-hub-router/src/lib.rs +++ b/modules/xcm-bridge-hub-router/src/lib.rs @@ -50,47 +50,17 @@ pub mod pallet { #[pallet::hooks] impl, I: 'static> Hooks> for Pallet { fn on_initialize(_n: BlockNumberFor) -> Weight { - // TODO: since this pallet now supports dynamic bridges, we need to move this code - // to e.g. deliver or at least schedule everything required from there - - // if we have never sent bridge state report request or have sent it more than - // `bridge_state_report_delay` blocks ago and haven't yet received a response, we - // need to increase fee factor - let limits = T::BridgeLimits::get(); - let current_block = frame_system::Pallet::block_number(); - let request_sent_at = Self::report_bridge_state_request_sent_at::get(); - let missing_bridge_state_report = request_sent_at - .map(|request_sent_at| { - let expected_report_delay = limits.bridge_state_report_delay; - current_block.saturating_sub(request_sent_at) > expected_report_delay - }) - .unwrap_or(false); - - // if we believe there are more enqueued messages than during regular bridge operations, - // we need to increase fee factor - let queues_state = Self::bridge_queues_state().unwrap_or_default(); - let total_enqueued_messages = queues_state.total_enqueued_messages(); - let too_many_enqueued_messages = total_enqueued_messages > limits.increase_fee_factor_threshold; - - // remember that we need to increase fee factor or decrease it - if missing_bridge_state_report || too_many_enqueued_messages { - IncreaseFeeFactorOnSend::::set(true); - } else { - Self::decrease_fee_factor(); - } + RelievingBridges::::mutate(|relieving_bridges| { + relieving_bridges.retain(|lane_id| { + Bridges::::mutate_extant(|lane_id|, |bridge| { + bridge.fee_factor = FixedU128::one().max(bridge.fee_factor / EXPONENTIAL_FEE_BASE); + bridge.fee_factor != FixedU128::one() + }) + }); - // now it's time to decide whether we want to send bridge state report request. We want - // to send it when we believe number of enqueued messages is close to increase-fee-factor - // threshold AND we don't have an active request - let close_to_too_many_enqueued_messages = total_enqueued_messages > limits.send_report_bridge_state_threshold; - if request_sent_at.is_none() && close_to_too_many_enqueued_messages { - // TODO: even if `ReportBridgeStateRequestSentAt` is `None`, we may want to delay next - // request to avoid being too frequent - T::ReportBridgeStateRequestSender::send(current_block); - ReportBridgeStateRequestSentAt::set(current_block); - } + }) - // TODO: use benchmarks + // TODO: use benchmarks for that Weight::zero() } @@ -114,6 +84,7 @@ pub mod pallet { // TODO: we shall only accept response for our last request - i.e. if something // will go wrong and controller wil use forced report BUT then some old // report will come, then we need to avoid it + // TODO: start decreasing fee factor at every on_initialize // we have receoved state report and may kill the `ReportBridgeStateRequestSentAt` value. // This means that at the next block we may send another report request and hopefully bridge @@ -122,20 +93,12 @@ pub mod pallet { } } - impl, I: 'static> Pallet { - /// Decrement fee factor, making messages cheaper. This is called by the pallet itself - /// at the beginning of every block. - fn decrement_fee_factor() -> FixedU128 { - FeeFactor::::mutate(|f| { - *f = InitialFeeFactor::get().max(*f / EXPONENTIAL_FEE_BASE); - *f - }) - } - } - /// All registered bridges. #[pallet::storage] - pub type Bridges, I: 'static = ()> = StorageMap<_, Identity, LaneId, BridgeOf>; + pub type Bridges, I: 'static = ()> = StorageMap<_, Identity, LaneId, BridgeOf, OptionQuery>; + + /// Bridges that are currently in the relieving phase. + pub type RelievingBridges, I: 'static = ()> = StorageValue<_, Vec, ValueQuery>; } /// We'll be using `SovereignPaidRemoteExporter` to send remote messages over the sibling/child bridge hub. @@ -150,7 +113,7 @@ type ViaBridgeHubExporter = SovereignPaidRemoteExporter< impl, I: 'static> ExporterFor for Pallet { fn exporter_for( network: &NetworkId, - _: &InteriorMultiLocation, + remote_location: &InteriorMultiLocation, message: &Xcm<()>, ) -> Option<(MultiLocation, Option)> { // ensure that the message is sent to the expected bridged network @@ -159,11 +122,12 @@ impl, I: 'static> ExporterFor for Pallet { } // compute fee amount - let bridge = Bridges::::get(); + let lane_id = bridge_lane_id(network, remote_location, T::UniversalLocation::get()); + let bridge = Bridges::::get(lane_id)?; let mesage_size = message.encoded_size(); let message_fee = (mesage_size as u128).saturating_mul(T::MessageByteFee::get()); let fee_sum = T::MessageBaseFee::get().saturating_add(message_fee); - let fee = fee_factor.saturating_mul_int(fee_sum); + let fee = bridge_fee_factor.saturating_mul_int(fee_sum); Some((T::LocalBridgeHubLocation::get(), (T::FeeAsset::get(), fee).into())) } @@ -181,7 +145,7 @@ impl, I: 'static> SendXcm for Pallet { ) -> SendResult { // just use exporter to validate destination and insert instructions to pay message fee // at the sibling/child bridge hub - let lane_id = LaneId::new(UniversalLocation::get(), ); + let lane_id = bridge_lane_id(); let message_size = xcm.as_ref().map(|xcm| xcm.encoded_size()); ViaBridgeHubExporter::::validate(dest, xcm).map(|ticket| (lane_id, message_size, ticket)) } @@ -195,19 +159,26 @@ impl, I: 'static> SendXcm for Pallet { // let's check if we need to increase fee for the further messages let limits = T::BridgeLimits::get(); let current_block = frame_system::Pallet::block_number(); - let state = BridgeState::::get(lane_id); + let mut bridge = Bridges::::get(lane_id).ok_or_else(SendError::Unroutable)?; let limits = BridgeLimits::get(); - if is_fee_increment_required(current_block, &state, &limits) { + if is_fee_increment_required(current_block, &bridge, &limits) { + // remove bridge from relieving set + if bridge.is_relieving { + RelievingBridges::::mutate(|v| { + v.remove(&lane_id); + }); + } + bridge.is_relieving = false; + + // update fee factor let message_size_factor = FixedU128::from_u32(message_size.saturating_div(1024) as u32) .saturating_mul(MESSAGE_SIZE_FEE_BASE); - FeeFactor::::mutate(|f| { - *f = f.saturating_mul(EXPONENTIAL_FEE_BASE + message_size_factor); - }); + bridge.fee_factor = bridge.fee_factor.saturating_mul(EXPONENTIAL_FEE_BASE + message_size_factor); } // let's check if we need to send bridge state report request - if is_state_report_required(current_block, &state, &limits) { - ScheduledStateReportRequests::::append(lane_id); + if is_state_report_required(current_block, &bridge, &limits) { + unimplemented!("TODO: implement me"); } Ok(xcm_hash) @@ -219,16 +190,16 @@ impl, I: 'static> SendXcm for Pallet { /// are sent over the bridge to avoid further throughput drops. fn is_fee_increment_required( current_block: BlockNumber, - state: &BridgeState, + bridge: &Bridge, limits: &BridgeLimits, ) -> bool { // if there are more messages than we expect, we need to increase fee - if state.total_enqueued_messages > limits.increase_fee_factor_threshold { + if bridge.total_enqueued_messages > limits.increase_fee_factor_threshold { return true; } // if we are waiting for the report for too long, we need to increase fee - let current_delay = state.last_report_request_block.map(|b| b.saturating_sub(current_block)).unwrap_or_else(Zero::zero); + let current_delay = bridge.last_report_request_block.map(|b| b.saturating_sub(current_block)).unwrap_or_else(Zero::zero); if current_delay > limits.maximal_bridge_state_report_delay { return true; } @@ -239,21 +210,21 @@ fn is_fee_increment_required( /// Returns `true` if we need new status report from the bridge hub. fn is_state_report_required( current_block: BlockNumber, - state: &BridgeState, + bridge: &Bridge, limits: &BridgeLimits, ) -> bool { // we never issue multiple report requests - if state.last_report_request_block.is_some() { + if bridge.last_report_request_block.is_some() { return false; } // we don't need new request if we believe there aren't much messages in the queue - if state.total_enqueued_messages <= limits.send_report_bridge_state_threshold { + if bridge.total_enqueued_messages <= limits.send_report_bridge_state_threshold { return false; } // we don't need new request if we have received report recently - if current_block.saturating_sub(state.last_report_block) < limits.minimal_bridge_state_request_delay { + if current_block.saturating_sub(bridge.last_report_block) < limits.minimal_bridge_state_request_delay { return false } diff --git a/primitives/xcm-bridge-hub-router/src/lib.rs b/primitives/xcm-bridge-hub-router/src/lib.rs index 1a5191cbc0..546af9ff4a 100644 --- a/primitives/xcm-bridge-hub-router/src/lib.rs +++ b/primitives/xcm-bridge-hub-router/src/lib.rs @@ -123,6 +123,9 @@ impl AtBridgeHubBridgeQueuesState { /// Current state of bridge with the remote deestination. pub struct Bridge { + /// If true, the bridge is currently in "relieving" state, where we are decreasing + /// the fee factor at every block. + pub is_relieving: bool, /// The number to multiply the base message delivery fee by. We will increase this /// value exponentially when we the bridge throughput decreases and decrease after /// it is back to normal. @@ -143,9 +146,10 @@ pub struct Bridge { pub last_report_request_block: Option, } -impl Default for BridgeState { +impl Default for Bridge { fn default() -> Self { - BridgeState { + Bridge { + is_relieving: false, fee_factor: FixedU128::from_u32(1), total_enqueued_messages: 0, last_report_block: Zero::zero(), From b6fcc74d1afd2b499adef9e70b74b2ff4f795cd4 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Mon, 3 Jul 2023 12:04:20 +0300 Subject: [PATCH 08/10] flush --- modules/xcm-bridge-hub-router/src/lib.rs | 110 ++++++++++++++++---- primitives/xcm-bridge-hub-router/src/lib.rs | 54 +++------- 2 files changed, 100 insertions(+), 64 deletions(-) diff --git a/modules/xcm-bridge-hub-router/src/lib.rs b/modules/xcm-bridge-hub-router/src/lib.rs index 043ae4fa76..17a20df8ed 100644 --- a/modules/xcm-bridge-hub-router/src/lib.rs +++ b/modules/xcm-bridge-hub-router/src/lib.rs @@ -36,11 +36,15 @@ pub mod pallet { #[pallet::constant] type BridgeLimits: Get; + /// Bridge hub origin. + type BridgeHubOrigin: EnsureOrigin<::RuntimeOrigin>; + /// Child/sibling bridge hub configuration. type BridgeHub: BridgeHub; /// Underlying transport (XCMP or DMP) which allows sending messages to the child/sibling - /// bridge hub. + /// bridge hub. We assume this channel satisfies all XCM requirements - we rely on the + /// fact that it guarantees ordered delivery. type ToBridgeHubSender: SendXcm; } @@ -52,12 +56,34 @@ pub mod pallet { fn on_initialize(_n: BlockNumberFor) -> Weight { RelievingBridges::::mutate(|relieving_bridges| { relieving_bridges.retain(|lane_id| { - Bridges::::mutate_extant(|lane_id|, |bridge| { + let result = Bridges::::try_mutate_exists(|lane_id|, |stored_bridge| { + // remove deleted bridge from the `RelievingBridges` set + let bridge = match stored_bridge.take() { + Some(bridge) => bridge, + None => return Err(false), + }; + + // remove bridge from the `RelievingBridges` set if it isn't relieving anymore + if !bridge.is_relieving { + return Err(false); + } + + // decrease fee factor bridge.fee_factor = FixedU128::one().max(bridge.fee_factor / EXPONENTIAL_FEE_BASE); - bridge.fee_factor != FixedU128::one() - }) + + // stop relieveing if fee factor is `1` + let keep_relieving = bridge.fee_factor != FixedU128::one(); + if !keep_relieving { + bridge.is_relieving = false; + } + + *stored_bridge = Some(bridge); + Ok(bridge.is_relieving) + }); }); + // remove from the set if bridge is no longer relieving + result == Ok(true) }) // TODO: use benchmarks for that @@ -71,25 +97,52 @@ pub mod pallet { #[pallet::call] impl, I: 'static> Pallet { + + #[pallet::call_index(0)] #[pallet::weight(Weight::zero())] pub fn receive_bridge_state_report( - _origin: OriginFor, - _request_sent_at: T::BlockNumber, - _at_bridge_hub_queues_state: AtBridgeHubBridgeQueuesState, + origin: OriginFor, + lane_id: LaneId, + request_sent_at: T::BlockNumber, + at_bridge_hub_queues_state: AtBridgeHubBridgeQueuesState, ) -> DispatchResultWithPostInfo { - // TODO: check origin - it must be either parachain, or some controller account - // TODO: convert `at_bridge_hub_queues_state` to `BridgeQueuesState` - // TODO: kill `ReportBridgeStateRequestSentAt` - // TODO: we shall only accept response for our last request - i.e. if something - // will go wrong and controller wil use forced report BUT then some old - // report will come, then we need to avoid it - // TODO: start decreasing fee factor at every on_initialize - - // we have receoved state report and may kill the `ReportBridgeStateRequestSentAt` value. - // This means that at the next block we may send another report request and hopefully bridge - // will - ReportBridgeStateRequestSentAt::::kill(); + // we only accept reports from the bridge hub + T::BridgeHubOrigin::ensure_origin(origin)?; + + Bridges::::try_mutate_exists(lane_id, |stored_bridge| { + // fail reports for unknown bridges + let mut bridge = match stored_bridge.take() { + Some(stored_bridge) => stored_bridge, + None => fail!(Error::::UnknownBridge), + }; + + // we only accept report for the latest request + let last_report_request = match bridge.last_report_request.take() { + Some(last_report_request) if last_report_request.at_block == request_sent_at => last_report_request, + _ => fail!(Error::::UnexpectedReport), + }; + + // update total number of enqueued messages. To compute numnber of enqueued messages in + // `outbound_here` and `inbound_at_sibling` queues we use our own counter because we can't + // rely on the information from the sibling/child bridge hub - it is processed with a delay + // and during that delay any number of messages may be sent over the bridge. + bridge.total_enqueued_messages = last_report_request.messages_sent_after_request + .saturating_add(at_bridge_hub_queues_state.total_enqueued_messages()); + + + // if we need to start decreasing fee factor - let's remember that + let old_is_relieving = bridge.is_relieving; + bridge.is_relieving = is_relief_required(&bridge); + if bridge.is_relieving && !old_is_relieving { + RelievingBridges::::append(lane_id); + } + + stored_bridge = Some(bridge); + Ok(()) + }); + + Ok(()) } } @@ -176,11 +229,21 @@ impl, I: 'static> SendXcm for Pallet { bridge.fee_factor = bridge.fee_factor.saturating_mul(EXPONENTIAL_FEE_BASE + message_size_factor); } + // increment number of enqueued messages that we think are in all bridge queues now + bridge.total_enqueued_messages.saturating_inc(); + // let's check if we need to send bridge state report request if is_state_report_required(current_block, &bridge, &limits) { unimplemented!("TODO: implement me"); } + // also increment number of messages that are sent **after** our last report request + if let Some(ref mut last_report_request) = bridge.last_report_request { + last_report_request.messages_sent_after_request.saturating_inc(); + } + + // TODO: update bridge in the storage + Ok(xcm_hash) } } @@ -214,9 +277,10 @@ fn is_state_report_required( limits: &BridgeLimits, ) -> bool { // we never issue multiple report requests - if bridge.last_report_request_block.is_some() { - return false; - } + let last_report_request = match bridge.last_report_request { + Some(last_report_request) => last_report_request, + None => return false, + }; // we don't need new request if we believe there aren't much messages in the queue if bridge.total_enqueued_messages <= limits.send_report_bridge_state_threshold { @@ -224,7 +288,7 @@ fn is_state_report_required( } // we don't need new request if we have received report recently - if current_block.saturating_sub(bridge.last_report_block) < limits.minimal_bridge_state_request_delay { + if current_block.saturating_sub(last_report_request.sent_at) < limits.minimal_bridge_state_request_delay { return false } diff --git a/primitives/xcm-bridge-hub-router/src/lib.rs b/primitives/xcm-bridge-hub-router/src/lib.rs index 546af9ff4a..2d6818b351 100644 --- a/primitives/xcm-bridge-hub-router/src/lib.rs +++ b/primitives/xcm-bridge-hub-router/src/lib.rs @@ -40,39 +40,6 @@ pub struct BridgeLimits { pub send_report_bridge_state_threshold: MessageNonce, } -/// All bridge queues state. -#[derive( - Clone, - Copy, - Decode, - Default, - Encode, - Eq, - Ord, - PartialOrd, - PartialEq, - RuntimeDebug, - TypeInfo, - MaxEncodedLen, -)] -pub struct BridgeQueuesState { - /// Total number of messages that have been sent since last bridge state - /// Number of messages queued at this (source) chain' outbound queue. - /// - /// That's the only field that is filled at this chain, because sibling (source) - /// bridge hub doesn't have an access to our queue. - pub outbound_here: MessageNonce, - /// Status of queues, that we have received from the bridge hub. - pub at_bridge_hub: AtBridgeHubBridgeQueuesState, -} - -impl BridgeQueuesState { - /// Return total number of messsages that we assume are currently in the bridges queue. - pub fn total_enqueued_messages(&self) -> MessageNonce { - self.outbound_here.saturating_add(self.at_bridge_hub.total_enqueued_messages()) - } -} - // TODO: this should be moved to `bp-xcm-bridge-hub`. /// All bridge queues state. #[derive( @@ -92,8 +59,6 @@ impl BridgeQueuesState { Deserialize, )] pub struct AtBridgeHubBridgeQueuesState { - /// Number of messages queued at sibling (source) bridge hub inbound queue. - pub inbound_at_sibling: MessageNonce, /// Number of messages queued at sibling (source) bridge hub outbound queue. pub outbound_at_sibling: MessageNonce, /// Number of messages queued at bridged (target) bridge hub outbound queue. @@ -114,13 +79,20 @@ pub struct AtBridgeHubBridgeQueuesState { impl AtBridgeHubBridgeQueuesState { /// Return total number of messsages that we assume are currently in the bridges queue. pub fn total_enqueued_messages(&self) -> MessageNonce { - self.inbound_at_sibling - .saturating_add(self.outbound_at_sibling) + self.outbound_at_sibling .saturating_add(self.outbound_at_bridged) .saturating_add(self.inbound_at_destination) } } +/// State of the last bridge state report request; +pub struct ReportRequestState { + /// The number of block, at which we have sent our last bridge state report request. + pub sent_at: Option, + /// The number of messages that were sent after we have issued our report request. + pub messages_sent_after_request: MessageNonce, +} + /// Current state of bridge with the remote deestination. pub struct Bridge { /// If true, the bridge is currently in "relieving" state, where we are decreasing @@ -141,9 +113,9 @@ pub struct Bridge { /// The number of block, at which we have received last bridge state report. If we have /// never received a report, it is set to zero. pub last_report_block: BlockNumber, - /// The number of block, at which we have sent our last bridge state report request. Set - /// to `None` if we have no active report request. - pub last_report_request_block: Option, + /// Status of the last bridge state report request. If `None`, then there are no active + /// report request. + pub last_report_request: Option>, } impl Default for Bridge { @@ -153,7 +125,7 @@ impl Default for Bridge { fee_factor: FixedU128::from_u32(1), total_enqueued_messages: 0, last_report_block: Zero::zero(), - last_report_request_block: None, + last_report_request: None, } } } From e66eb5b9dd2cbc1159788848afd5d1b42aaf1d88 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Mon, 3 Jul 2023 15:22:44 +0300 Subject: [PATCH 09/10] flush + fmt --- Cargo.lock | 13 +- modules/messages/src/tests/pallet_tests.rs | 8 +- modules/xcm-bridge-hub-router/Cargo.toml | 13 ++ modules/xcm-bridge-hub-router/src/lib.rs | 156 +++++++++++++----- .../tests/implementation_match.rs | 4 +- primitives/messages/src/target_chain.rs | 4 +- primitives/xcm-bridge-hub-router/Cargo.toml | 8 +- primitives/xcm-bridge-hub-router/src/lib.rs | 4 +- .../src/transaction_tracker.rs | 4 +- .../src/on_demand/parachains.rs | 4 +- relays/messages/src/message_race_strategy.rs | 4 +- 11 files changed, 157 insertions(+), 65 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e759890517..dcce622f62 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1140,13 +1140,15 @@ dependencies = [ ] [[package]] -name = "bp-xcm-over-bridge-fee" +name = "bp-xcm-bridge-hub-router" version = "0.1.0" dependencies = [ "bp-messages", + "frame-support", "parity-scale-codec", "scale-info", "serde", + "sp-runtime", ] [[package]] @@ -7654,14 +7656,19 @@ dependencies = [ ] [[package]] -name = "pallet-xcm-over-bridge-fee" +name = "pallet-xcm-bridge-hub-router" version = "0.1.0" dependencies = [ - "bp-xcm-over-bridge-fee", + "bp-messages", + "bp-xcm-bridge-hub-router", "frame-support", "frame-system", "parity-scale-codec", + "scale-info", + "sp-runtime", "sp-std 8.0.0 (git+https://github.com/paritytech/substrate?branch=master)", + "xcm", + "xcm-builder", ] [[package]] diff --git a/modules/messages/src/tests/pallet_tests.rs b/modules/messages/src/tests/pallet_tests.rs index dc10a49ac7..2b56f38c77 100644 --- a/modules/messages/src/tests/pallet_tests.rs +++ b/modules/messages/src/tests/pallet_tests.rs @@ -801,11 +801,11 @@ fn receive_messages_delivery_proof_rejects_proof_if_trying_to_confirm_more_messa send_regular_message(); // 1) InboundLaneData declares that the `last_confirmed_nonce` is 1; - // 2) InboundLaneData has no entries => `InboundLaneData::last_delivered_nonce()` - // returns `last_confirmed_nonce`; + // 2) InboundLaneData has no entries => `InboundLaneData::last_delivered_nonce()` returns + // `last_confirmed_nonce`; // 3) it means that we're going to confirm delivery of messages 1..=1; - // 4) so the number of declared messages (see `UnrewardedRelayersState`) is `0` and - // numer of actually confirmed messages is `1`. + // 4) so the number of declared messages (see `UnrewardedRelayersState`) is `0` and numer of + // actually confirmed messages is `1`. let proof = prepare_messages_delivery_proof( test_lane_id(), InboundLaneData { diff --git a/modules/xcm-bridge-hub-router/Cargo.toml b/modules/xcm-bridge-hub-router/Cargo.toml index c971e28863..dd658cb220 100644 --- a/modules/xcm-bridge-hub-router/Cargo.toml +++ b/modules/xcm-bridge-hub-router/Cargo.toml @@ -8,25 +8,38 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0" [dependencies] codec = { package = "parity-scale-codec", version = "3.1.5", default-features = false } +scale-info = { version = "2.8.0", default-features = false, features = ["bit-vec", "derive", "serde"] } # Bridge dependencies +bp-messages = { path = "../../primitives/messages", default-features = false } bp-xcm-bridge-hub-router = { path = "../../primitives/xcm-bridge-hub-router", default-features = false } # Substrate Dependencies frame-support = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } frame-system = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } +sp-runtime = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } sp-std = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } +# Polkadot Dependencies + +xcm = { git = "https://github.com/paritytech/polkadot", branch = "master", default-features = false } +xcm-builder = { git = "https://github.com/paritytech/polkadot", branch = "master", default-features = false } + [features] default = ["std"] std = [ + "bp-messages/std", "bp-xcm-bridge-hub-router/std", "codec/std", "frame-support/std", "frame-system/std", + "scale-info/std", + "sp-runtime/std", "sp-std/std", + "xcm/std", + "xcm-builder/std", ] try-runtime = [ "frame-support/try-runtime", diff --git a/modules/xcm-bridge-hub-router/src/lib.rs b/modules/xcm-bridge-hub-router/src/lib.rs index 17a20df8ed..d4d31d1675 100644 --- a/modules/xcm-bridge-hub-router/src/lib.rs +++ b/modules/xcm-bridge-hub-router/src/lib.rs @@ -22,8 +22,21 @@ #![cfg_attr(not(feature = "std"), no_std)] +use bp_messages::LaneId; +use bp_xcm_bridge_hub_router::{AtBridgeHubBridgeQueuesState, Bridge, BridgeLimits}; +use frame_support::fail; +use sp_runtime::{traits::Zero, FixedU128}; +use xcm::prelude::*; +use xcm_builder::{ensure_is_remote, ExporterFor, SovereignPaidRemoteExporter}; + pub use pallet::*; +/// 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 +/// The factor that is used to increase current message fee factor for every sent kilobyte. +const MESSAGE_SIZE_FEE_BASE: FixedU128 = FixedU128::from_rational(1, 1000); // 0.001 + #[frame_support::pallet] pub mod pallet { use super::*; @@ -34,13 +47,16 @@ pub mod pallet { pub trait Config: frame_system::Config { /// Bridge limits. #[pallet::constant] - type BridgeLimits: Get; + type BridgeLimits: Get>>; + + /// Runtime's universal location. + type UniversalLocation: Get; /// Bridge hub origin. type BridgeHubOrigin: EnsureOrigin<::RuntimeOrigin>; /// Child/sibling bridge hub configuration. - type BridgeHub: BridgeHub; + // type BridgeHub: BridgeHub; /// Underlying transport (XCMP or DMP) which allows sending messages to the child/sibling /// bridge hub. We assume this channel satisfies all XCM requirements - we rely on the @@ -48,6 +64,11 @@ pub mod pallet { type ToBridgeHubSender: SendXcm; } + /// Shortcut for the BlockNumber used by our chain. + type BlockNumberOf = ::BlockNumber; + /// Shortcut for the `Bridge` structure used by the pallet. + type BridgeOf = Bridge>; + #[pallet::pallet] pub struct Pallet(PhantomData<(T, I)>); @@ -56,20 +77,22 @@ pub mod pallet { fn on_initialize(_n: BlockNumberFor) -> Weight { RelievingBridges::::mutate(|relieving_bridges| { relieving_bridges.retain(|lane_id| { - let result = Bridges::::try_mutate_exists(|lane_id|, |stored_bridge| { + let result = Bridges::::try_mutate_exists(lane_id, |stored_bridge| { // remove deleted bridge from the `RelievingBridges` set let bridge = match stored_bridge.take() { Some(bridge) => bridge, None => return Err(false), }; - // remove bridge from the `RelievingBridges` set if it isn't relieving anymore + // remove bridge from the `RelievingBridges` set if it isn't relieving + // anymore if !bridge.is_relieving { - return Err(false); + return Err(false) } // decrease fee factor - bridge.fee_factor = FixedU128::one().max(bridge.fee_factor / EXPONENTIAL_FEE_BASE); + bridge.fee_factor = + FixedU128::one().max(bridge.fee_factor / EXPONENTIAL_FEE_BASE); // stop relieveing if fee factor is `1` let keep_relieving = bridge.fee_factor != FixedU128::one(); @@ -80,31 +103,25 @@ pub mod pallet { *stored_bridge = Some(bridge); Ok(bridge.is_relieving) }); - }); - // remove from the set if bridge is no longer relieving - result == Ok(true) - }) + // remove from the set if bridge is no longer relieving + result == Ok(true) + }); + }); // TODO: use benchmarks for that Weight::zero() } - - fn on_finalize(_n: BlockNumberFor) { - IncreaseFeeFactorOnSend::::kill(); - } } #[pallet::call] impl, I: 'static> Pallet { - - #[pallet::call_index(0)] #[pallet::weight(Weight::zero())] pub fn receive_bridge_state_report( origin: OriginFor, lane_id: LaneId, - request_sent_at: T::BlockNumber, + request_sent_at: BlockNumberOf, at_bridge_hub_queues_state: AtBridgeHubBridgeQueuesState, ) -> DispatchResultWithPostInfo { // we only accept reports from the bridge hub @@ -119,18 +136,21 @@ pub mod pallet { // we only accept report for the latest request let last_report_request = match bridge.last_report_request.take() { - Some(last_report_request) if last_report_request.at_block == request_sent_at => last_report_request, + Some(last_report_request) + if last_report_request.at_block == request_sent_at => + last_report_request, _ => fail!(Error::::UnexpectedReport), }; - // update total number of enqueued messages. To compute numnber of enqueued messages in - // `outbound_here` and `inbound_at_sibling` queues we use our own counter because we can't - // rely on the information from the sibling/child bridge hub - it is processed with a delay - // and during that delay any number of messages may be sent over the bridge. - bridge.total_enqueued_messages = last_report_request.messages_sent_after_request + // update total number of enqueued messages. To compute numnber of enqueued messages + // in `outbound_here` and `inbound_at_sibling` queues we use our own counter because + // we can't rely on the information from the sibling/child bridge hub - it is + // processed with a delay and during that delay any number of messages may be sent + // over the bridge. + bridge.total_enqueued_messages = last_report_request + .messages_sent_after_request .saturating_add(at_bridge_hub_queues_state.total_enqueued_messages()); - // if we need to start decreasing fee factor - let's remember that let old_is_relieving = bridge.is_relieving; bridge.is_relieving = is_relief_required(&bridge); @@ -148,17 +168,29 @@ pub mod pallet { /// All registered bridges. #[pallet::storage] - pub type Bridges, I: 'static = ()> = StorageMap<_, Identity, LaneId, BridgeOf, OptionQuery>; + pub type Bridges, I: 'static = ()> = + StorageMap<_, Identity, LaneId, BridgeOf, OptionQuery>; /// Bridges that are currently in the relieving phase. - pub type RelievingBridges, I: 'static = ()> = StorageValue<_, Vec, ValueQuery>; + #[pallet::storage] + pub type RelievingBridges, I: 'static = ()> = + StorageValue<_, Vec, ValueQuery>; + + #[pallet::error] + pub enum Error { + /// Trying to access unknown bridge. + UnknownBridge, + /// The bridge queues state report is unexpected at the moment. + UnexpectedReport, + } } -/// We'll be using `SovereignPaidRemoteExporter` to send remote messages over the sibling/child bridge hub. +/// We'll be using `SovereignPaidRemoteExporter` to send remote messages over the sibling/child +/// bridge hub. type ViaBridgeHubExporter = SovereignPaidRemoteExporter< Pallet, >::ToBridgeHubSender, - T as Config>::UniversalLocation, + >::UniversalLocation, >; // This pallet acts as the `ExporterFor` for the `SovereignPaidRemoteExporter` to compute @@ -171,7 +203,7 @@ impl, I: 'static> ExporterFor for Pallet { ) -> Option<(MultiLocation, Option)> { // ensure that the message is sent to the expected bridged network if *network != T::BridgedNetworkId::get() { - return None; + return None } // compute fee amount @@ -180,7 +212,7 @@ impl, I: 'static> ExporterFor for Pallet { let mesage_size = message.encoded_size(); let message_fee = (mesage_size as u128).saturating_mul(T::MessageByteFee::get()); let fee_sum = T::MessageBaseFee::get().saturating_add(message_fee); - let fee = bridge_fee_factor.saturating_mul_int(fee_sum); + let fee = bridge.fee_factor.saturating_mul_int(fee_sum); Some((T::LocalBridgeHubLocation::get(), (T::FeeAsset::get(), fee).into())) } @@ -190,20 +222,29 @@ impl, I: 'static> ExporterFor for Pallet { // XCMP/DMP transport. This allows injecting dynamic message fees into XCM programs that // are going to the bridged network. impl, I: 'static> SendXcm for Pallet { - type Ticket = (LaneId, u32, T::ToBridgeHubSender::Ticket); + type Ticket = (LaneId, u32, ::Ticket); fn validate( dest: &mut Option, xcm: &mut Option>, - ) -> SendResult { + ) -> SendResult { + // we won't have an access to `dest` and `xcm` in the `delvier` method, so precompute + // everything required here + let (remote_network, remote_location) = ensure_is_remote( + T::UniversalLocation::get(), + *dest.as_ref().ok_or(SendError::MissingArgument)?, + ) + .map_err(|_| SendError::NotApplicable)?; + let lane_id = bridge_lane_id(remote_network, remote_location, T::UniversalLocation::get()); + let message_size = xcm.as_ref().map(|xcm| xcm.encoded_size()); + // just use exporter to validate destination and insert instructions to pay message fee // at the sibling/child bridge hub - let lane_id = bridge_lane_id(); - let message_size = xcm.as_ref().map(|xcm| xcm.encoded_size()); - ViaBridgeHubExporter::::validate(dest, xcm).map(|ticket| (lane_id, message_size, ticket)) + ViaBridgeHubExporter::::validate(dest, xcm) + .map(|ticket| (lane_id, message_size, ticket)) } - fn deliver(ticket: (u32, Router::Ticket)) -> Result { + fn deliver(ticket: Self::Ticket) -> Result { // use router to enqueue message to the sibling/child bridge hub. This also should handle // payment for passing through this queue. let (lane_id, message_size, ticket) = ticket; @@ -211,7 +252,7 @@ impl, I: 'static> SendXcm for Pallet { // let's check if we need to increase fee for the further messages let limits = T::BridgeLimits::get(); - let current_block = frame_system::Pallet::block_number(); + let current_block = frame_system::Pallet::::block_number(); let mut bridge = Bridges::::get(lane_id).ok_or_else(SendError::Unroutable)?; let limits = BridgeLimits::get(); if is_fee_increment_required(current_block, &bridge, &limits) { @@ -226,7 +267,8 @@ impl, I: 'static> SendXcm for Pallet { // update fee factor let message_size_factor = FixedU128::from_u32(message_size.saturating_div(1024) as u32) .saturating_mul(MESSAGE_SIZE_FEE_BASE); - bridge.fee_factor = bridge.fee_factor.saturating_mul(EXPONENTIAL_FEE_BASE + message_size_factor); + bridge.fee_factor = + bridge.fee_factor.saturating_mul(EXPONENTIAL_FEE_BASE + message_size_factor); } // increment number of enqueued messages that we think are in all bridge queues now @@ -258,13 +300,16 @@ fn is_fee_increment_required( ) -> bool { // if there are more messages than we expect, we need to increase fee if bridge.total_enqueued_messages > limits.increase_fee_factor_threshold { - return true; + return true } // if we are waiting for the report for too long, we need to increase fee - let current_delay = bridge.last_report_request_block.map(|b| b.saturating_sub(current_block)).unwrap_or_else(Zero::zero); + let current_delay = bridge + .last_report_request_block + .map(|b| b.saturating_sub(current_block)) + .unwrap_or_else(Zero::zero); if current_delay > limits.maximal_bridge_state_report_delay { - return true; + return true } false @@ -284,18 +329,41 @@ fn is_state_report_required( // we don't need new request if we believe there aren't much messages in the queue if bridge.total_enqueued_messages <= limits.send_report_bridge_state_threshold { - return false; + return false } // we don't need new request if we have received report recently - if current_block.saturating_sub(last_report_request.sent_at) < limits.minimal_bridge_state_request_delay { + if current_block.saturating_sub(last_report_request.sent_at) < + limits.minimal_bridge_state_request_delay + { return false } true } -// TODO: move this function to the bp-messages or bp-runtime - there's similar function in the pallet-xcm-bridge-hub. +/// Returns `true` if we need to start relieving the bridge fee factor. +fn is_relief_required( + current_block: BlockNumber, + bridge: &Bridge, + limits: &BridgeLimits, +) -> bool { + // if we already relieving - no need to restart + if bridge.is_relieving { + return false + } + + // if bridge fee factor is alreadt minimal, no need to relieve + if bridge.fee_factor <= FixedU128::one() { + return false + } + + // else - make sure that we are not increasing the factor + !is_fee_increment_required(current_block, bridge, limits) +} + +// TODO: move this function to the bp-messages or bp-runtime - there's similar function in the +// pallet-xcm-bridge-hub. /// Given remote (bridged) network id and interior location within that network, return /// lane identifier that will be used to bridge with local `local_universal_location`. /// diff --git a/primitives/header-chain/tests/implementation_match.rs b/primitives/header-chain/tests/implementation_match.rs index c70683b173..22f690b8cb 100644 --- a/primitives/header-chain/tests/implementation_match.rs +++ b/primitives/header-chain/tests/implementation_match.rs @@ -104,8 +104,8 @@ pub fn make_default_justification(header: &TestHeader) -> GrandpaJustification { /// Unspent dispatch weight. This weight that will be deducted from total delivery transaction /// weight, thus reducing the transaction cost. This shall not be zero in (at least) two cases: /// - /// 1) if message has been dispatched successfully, but post-dispatch weight is less than - /// the weight, declared by the message sender; + /// 1) if message has been dispatched successfully, but post-dispatch weight is less than the + /// weight, declared by the message sender; /// 2) if message has not been dispatched at all. pub unspent_weight: Weight, /// Fine-grained result of single message dispatch (for better diagnostic purposes) diff --git a/primitives/xcm-bridge-hub-router/Cargo.toml b/primitives/xcm-bridge-hub-router/Cargo.toml index cfa746fe56..f261bcca08 100644 --- a/primitives/xcm-bridge-hub-router/Cargo.toml +++ b/primitives/xcm-bridge-hub-router/Cargo.toml @@ -17,9 +17,10 @@ bp-messages = { path = "../messages", default-features = false } # Substrate Dependencies -#frame-support = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } +frame-support = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } #sp-core = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } #sp-io = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } +sp-runtime = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } #sp-std = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } [features] @@ -28,10 +29,11 @@ std = [ "bp-messages/std", # "bp-header-chain/std", "codec/std", -# "frame-support/std", + "frame-support/std", "scale-info/std", "serde/std", # "sp-core/std", # "sp-io/std", -# "sp-std/std" +# "sp-std/std", + "sp-runtime/std", ] diff --git a/primitives/xcm-bridge-hub-router/src/lib.rs b/primitives/xcm-bridge-hub-router/src/lib.rs index 2d6818b351..cac36839c1 100644 --- a/primitives/xcm-bridge-hub-router/src/lib.rs +++ b/primitives/xcm-bridge-hub-router/src/lib.rs @@ -20,8 +20,10 @@ use bp_messages::MessageNonce; use codec::{Decode, Encode, MaxEncodedLen}; +use frame_support::RuntimeDebug; use scale_info::TypeInfo; use serde::{Deserialize, Serialize}; +use sp_runtime::{traits::Zero, FixedU128}; /// Bridge limits. #[derive(Clone, RuntimeDebug)] @@ -100,7 +102,7 @@ pub struct Bridge { pub is_relieving: bool, /// The number to multiply the base message delivery fee by. We will increase this /// value exponentially when we the bridge throughput decreases and decrease after - /// it is back to normal. + /// it is back to normal. pub fee_factor: FixedU128, /// Count of undelivered bridge messages as we see it. The actual number may be lower /// if some messages are already delivered, but we have not yet received a report. diff --git a/relays/client-substrate/src/transaction_tracker.rs b/relays/client-substrate/src/transaction_tracker.rs index b1a3f99735..b4801c89f5 100644 --- a/relays/client-substrate/src/transaction_tracker.rs +++ b/relays/client-substrate/src/transaction_tracker.rs @@ -56,8 +56,8 @@ impl> Environment for T { /// 2) assume that the transaction is lost and resubmit another transaction instantly; /// /// 3) wait for some time (if transaction is mortal - then until block where it dies; if it is -/// immortal - then for some time that we assume is long enough to mine it) and assume that -/// it is lost. +/// immortal - then for some time that we assume is long enough to mine it) and assume that it is +/// lost. /// /// This struct implements third option as it seems to be the most optimal. pub struct TransactionTracker { diff --git a/relays/lib-substrate-relay/src/on_demand/parachains.rs b/relays/lib-substrate-relay/src/on_demand/parachains.rs index a936598f22..369ca39cb9 100644 --- a/relays/lib-substrate-relay/src/on_demand/parachains.rs +++ b/relays/lib-substrate-relay/src/on_demand/parachains.rs @@ -328,8 +328,8 @@ async fn background_task( // // 7) on-demand parachains relay sets `ParachainsSource::maximal_header_number` to the // `PH'.number()`. - // 8) parachains finality relay sees that the parachain head has been - // updated and relays `PH'` to the target chain. + // 8) parachains finality relay sees that the parachain head has been updated and relays + // `PH'` to the target chain. // select headers to relay let relay_data = read_relay_data( diff --git a/relays/messages/src/message_race_strategy.rs b/relays/messages/src/message_race_strategy.rs index 5c8f9a162b..93d178e55b 100644 --- a/relays/messages/src/message_race_strategy.rs +++ b/relays/messages/src/message_race_strategy.rs @@ -14,8 +14,8 @@ //! Basic delivery strategy. The strategy selects nonces if: //! //! 1) there are more nonces on the source side than on the target side; -//! 2) new nonces may be proved to target node (i.e. they have appeared at the -//! block, which is known to the target node). +//! 2) new nonces may be proved to target node (i.e. they have appeared at the block, which is known +//! to the target node). use crate::message_race_loop::{ NoncesRange, RaceState, RaceStrategy, SourceClientNonces, TargetClientNonces, From 593308928fb35bf1ac186ee030aba2d307457d85 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Mon, 10 Jul 2023 11:03:19 +0300 Subject: [PATCH 10/10] added TODO regarding BridgeLimits --- modules/xcm-bridge-hub-router/src/lib.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/modules/xcm-bridge-hub-router/src/lib.rs b/modules/xcm-bridge-hub-router/src/lib.rs index d4d31d1675..ae50546408 100644 --- a/modules/xcm-bridge-hub-router/src/lib.rs +++ b/modules/xcm-bridge-hub-router/src/lib.rs @@ -255,6 +255,10 @@ impl, I: 'static> SendXcm for Pallet { let current_block = frame_system::Pallet::::block_number(); let mut bridge = Bridges::::get(lane_id).ok_or_else(SendError::Unroutable)?; let limits = BridgeLimits::get(); + + // TODO: what if number of messages in the queues is close to the limit (see `BridgeLimits`)? + // maybe we should temporary close the router? + if is_fee_increment_required(current_block, &bridge, &limits) { // remove bridge from relieving set if bridge.is_relieving {