From fe6e4734901fdea3484c2c6aad288f52490cbdde Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Mon, 21 Nov 2022 13:29:26 +0300 Subject: [PATCH] Prune messages from on-idle callback (#1650) * prune messages from on-idle callback * no more secondary lanes at deployments * clippy * Update modules/messages/src/lib.rs Co-authored-by: Adrian Catangiu * sub -> add * more tests + check that message is sent using one of ActiveOutboundLanes * ensure spent_weight is correct Co-authored-by: Adrian Catangiu --- bin/millau/runtime/src/lib.rs | 6 +- bin/millau/runtime/src/rialto_messages.rs | 24 +- .../runtime/src/rialto_parachain_messages.rs | 6 +- bin/millau/runtime/src/xcm_config.rs | 10 +- bin/rialto-parachain/runtime/src/lib.rs | 7 +- .../runtime/src/millau_messages.rs | 23 +- bin/rialto/runtime/src/lib.rs | 3 +- bin/rialto/runtime/src/millau_messages.rs | 24 +- bin/runtime-common/src/integrity.rs | 6 +- modules/messages/src/lib.rs | 253 ++++++++++++++++-- modules/messages/src/mock.rs | 9 +- modules/messages/src/outbound_lane.rs | 58 +++- modules/relayers/src/mock.rs | 3 +- 13 files changed, 322 insertions(+), 110 deletions(-) diff --git a/bin/millau/runtime/src/lib.rs b/bin/millau/runtime/src/lib.rs index 49cd16834bb7..b1f1e33d18a4 100644 --- a/bin/millau/runtime/src/lib.rs +++ b/bin/millau/runtime/src/lib.rs @@ -469,6 +469,8 @@ parameter_types! { pub const RootAccountForPayments: Option = None; pub const RialtoChainId: bp_runtime::ChainId = bp_runtime::RIALTO_CHAIN_ID; pub const RialtoParachainChainId: bp_runtime::ChainId = bp_runtime::RIALTO_PARACHAIN_CHAIN_ID; + pub RialtoActiveOutboundLanes: &'static [bp_messages::LaneId] = &[rialto_messages::XCM_LANE]; + pub RialtoParachainActiveOutboundLanes: &'static [bp_messages::LaneId] = &[rialto_parachain_messages::XCM_LANE]; } /// Instance of the messages pallet used to relay messages to/from Rialto chain. @@ -477,7 +479,7 @@ pub type WithRialtoMessagesInstance = (); impl pallet_bridge_messages::Config for Runtime { type RuntimeEvent = RuntimeEvent; type WeightInfo = pallet_bridge_messages::weights::BridgeWeight; - type MaxMessagesToPruneAtOnce = MaxMessagesToPruneAtOnce; + type ActiveOutboundLanes = RialtoActiveOutboundLanes; type MaxUnrewardedRelayerEntriesAtInboundLane = MaxUnrewardedRelayerEntriesAtInboundLane; type MaxUnconfirmedMessagesAtInboundLane = MaxUnconfirmedMessagesAtInboundLane; @@ -506,7 +508,7 @@ pub type WithRialtoParachainMessagesInstance = pallet_bridge_messages::Instance1 impl pallet_bridge_messages::Config for Runtime { type RuntimeEvent = RuntimeEvent; type WeightInfo = pallet_bridge_messages::weights::BridgeWeight; - type MaxMessagesToPruneAtOnce = MaxMessagesToPruneAtOnce; + type ActiveOutboundLanes = RialtoParachainActiveOutboundLanes; type MaxUnrewardedRelayerEntriesAtInboundLane = MaxUnrewardedRelayerEntriesAtInboundLane; type MaxUnconfirmedMessagesAtInboundLane = MaxUnconfirmedMessagesAtInboundLane; diff --git a/bin/millau/runtime/src/rialto_messages.rs b/bin/millau/runtime/src/rialto_messages.rs index 5d7b189db2af..e78d69229e18 100644 --- a/bin/millau/runtime/src/rialto_messages.rs +++ b/bin/millau/runtime/src/rialto_messages.rs @@ -16,7 +16,7 @@ //! Everything required to serve Millau <-> Rialto messages. -use crate::{OriginCaller, RialtoGrandpaInstance, Runtime, RuntimeCall, RuntimeOrigin}; +use crate::{RialtoGrandpaInstance, Runtime, RuntimeCall, RuntimeOrigin}; use bp_messages::{ source_chain::TargetHeaderChain, @@ -28,7 +28,7 @@ use bridge_runtime_common::messages::{self, MessageBridge}; use frame_support::{parameter_types, weights::Weight, RuntimeDebug}; /// Default lane that is used to send messages to Rialto. -pub const DEFAULT_XCM_LANE_TO_RIALTO: LaneId = [0, 0, 0, 0]; +pub const XCM_LANE: LaneId = [0, 0, 0, 0]; /// Weight of 2 XCM instructions is for simple `Trap(42)` program, coming through bridge /// (it is prepended with `UniversalOrigin` instruction). It is used just for simplest manual /// tests, confirming that we don't break encoding somewhere between. @@ -98,24 +98,8 @@ impl messages::ThisChainWithMessages for Millau { type RuntimeOrigin = RuntimeOrigin; type RuntimeCall = RuntimeCall; - fn is_message_accepted(send_origin: &Self::RuntimeOrigin, lane: &LaneId) -> bool { - let here_location = - xcm::v3::MultiLocation::from(crate::xcm_config::UniversalLocation::get()); - match send_origin.caller { - OriginCaller::XcmPallet(pallet_xcm::Origin::Xcm(ref location)) - if *location == here_location => - { - log::trace!(target: "runtime::bridge", "Verifying message sent using XCM pallet to Rialto"); - }, - _ => { - // keep in mind that in this case all messages are free (in term of fees) - // => it's just to keep testing bridge on our test deployments until we'll have a - // better option - log::trace!(target: "runtime::bridge", "Verifying message sent using messages pallet to Rialto"); - }, - } - - *lane == DEFAULT_XCM_LANE_TO_RIALTO || *lane == [0, 0, 0, 1] + fn is_message_accepted(_send_origin: &Self::RuntimeOrigin, _lane: &LaneId) -> bool { + true } fn maximal_pending_messages_at_outbound_lane() -> MessageNonce { diff --git a/bin/millau/runtime/src/rialto_parachain_messages.rs b/bin/millau/runtime/src/rialto_parachain_messages.rs index 8655edd59f49..b6e8f52d342e 100644 --- a/bin/millau/runtime/src/rialto_parachain_messages.rs +++ b/bin/millau/runtime/src/rialto_parachain_messages.rs @@ -28,7 +28,7 @@ use bridge_runtime_common::messages::{self, MessageBridge}; use frame_support::{parameter_types, weights::Weight, RuntimeDebug}; /// Default lane that is used to send messages to Rialto parachain. -pub const DEFAULT_XCM_LANE_TO_RIALTO_PARACHAIN: LaneId = [0, 0, 0, 0]; +pub const XCM_LANE: LaneId = [0, 0, 0, 0]; /// Weight of 2 XCM instructions is for simple `Trap(42)` program, coming through bridge /// (it is prepended with `UniversalOrigin` instruction). It is used just for simplest manual /// tests, confirming that we don't break encoding somewhere between. @@ -103,8 +103,8 @@ impl messages::ThisChainWithMessages for Millau { type RuntimeCall = RuntimeCall; type RuntimeOrigin = RuntimeOrigin; - fn is_message_accepted(_send_origin: &Self::RuntimeOrigin, lane: &LaneId) -> bool { - *lane == DEFAULT_XCM_LANE_TO_RIALTO_PARACHAIN || *lane == [0, 0, 0, 1] + fn is_message_accepted(_send_origin: &Self::RuntimeOrigin, _lane: &LaneId) -> bool { + true } fn maximal_pending_messages_at_outbound_lane() -> MessageNonce { diff --git a/bin/millau/runtime/src/xcm_config.rs b/bin/millau/runtime/src/xcm_config.rs index 32b14f27a3b8..13304d2671f4 100644 --- a/bin/millau/runtime/src/xcm_config.rs +++ b/bin/millau/runtime/src/xcm_config.rs @@ -17,10 +17,8 @@ //! XCM configurations for the Millau runtime. use super::{ - rialto_messages::{WithRialtoMessageBridge, DEFAULT_XCM_LANE_TO_RIALTO}, - rialto_parachain_messages::{ - WithRialtoParachainMessageBridge, DEFAULT_XCM_LANE_TO_RIALTO_PARACHAIN, - }, + rialto_messages::{WithRialtoMessageBridge, XCM_LANE}, + rialto_parachain_messages::{WithRialtoParachainMessageBridge, XCM_LANE as XCM_LANE_PARACHAIN}, AccountId, AllPalletsWithSystem, Balances, Runtime, RuntimeCall, RuntimeEvent, RuntimeOrigin, WithRialtoMessagesInstance, WithRialtoParachainMessagesInstance, XcmPallet, }; @@ -218,7 +216,7 @@ impl XcmBridge for ToRialtoBridge { } fn xcm_lane() -> LaneId { - DEFAULT_XCM_LANE_TO_RIALTO + XCM_LANE } } @@ -245,7 +243,7 @@ impl XcmBridge for ToRialtoParachainBridge { } fn xcm_lane() -> LaneId { - DEFAULT_XCM_LANE_TO_RIALTO_PARACHAIN + XCM_LANE_PARACHAIN } } diff --git a/bin/rialto-parachain/runtime/src/lib.rs b/bin/rialto-parachain/runtime/src/lib.rs index 7487e68f8f95..fcf83c822945 100644 --- a/bin/rialto-parachain/runtime/src/lib.rs +++ b/bin/rialto-parachain/runtime/src/lib.rs @@ -26,7 +26,7 @@ #[cfg(feature = "std")] include!(concat!(env!("OUT_DIR"), "/wasm_binary.rs")); -use crate::millau_messages::{WithMillauMessageBridge, DEFAULT_XCM_LANE_TO_MILLAU}; +use crate::millau_messages::{WithMillauMessageBridge, XCM_LANE}; use bridge_runtime_common::messages::source::{XcmBridge, XcmBridgeAdapter}; use cumulus_pallet_parachain_system::AnyRelayNumber; @@ -455,7 +455,7 @@ impl XcmBridge for ToMillauBridge { } fn xcm_lane() -> bp_messages::LaneId { - DEFAULT_XCM_LANE_TO_MILLAU + XCM_LANE } } @@ -554,6 +554,7 @@ parameter_types! { bp_millau::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX; pub const RootAccountForPayments: Option = None; pub const BridgedChainId: bp_runtime::ChainId = bp_runtime::MILLAU_CHAIN_ID; + pub ActiveOutboundLanes: &'static [bp_messages::LaneId] = &[millau_messages::XCM_LANE]; } /// Instance of the messages pallet used to relay messages to/from Millau chain. @@ -562,7 +563,7 @@ pub type WithMillauMessagesInstance = (); impl pallet_bridge_messages::Config for Runtime { type RuntimeEvent = RuntimeEvent; type WeightInfo = pallet_bridge_messages::weights::BridgeWeight; - type MaxMessagesToPruneAtOnce = MaxMessagesToPruneAtOnce; + type ActiveOutboundLanes = ActiveOutboundLanes; type MaxUnrewardedRelayerEntriesAtInboundLane = MaxUnrewardedRelayerEntriesAtInboundLane; type MaxUnconfirmedMessagesAtInboundLane = MaxUnconfirmedMessagesAtInboundLane; diff --git a/bin/rialto-parachain/runtime/src/millau_messages.rs b/bin/rialto-parachain/runtime/src/millau_messages.rs index ceabcd57b666..25d37f159883 100644 --- a/bin/rialto-parachain/runtime/src/millau_messages.rs +++ b/bin/rialto-parachain/runtime/src/millau_messages.rs @@ -19,7 +19,7 @@ // TODO: this is almost exact copy of `millau_messages.rs` from Rialto runtime. // Should be extracted to a separate crate and reused here. -use crate::{MillauGrandpaInstance, OriginCaller, Runtime, RuntimeCall, RuntimeOrigin}; +use crate::{MillauGrandpaInstance, Runtime, RuntimeCall, RuntimeOrigin}; use bp_messages::{ source_chain::TargetHeaderChain, @@ -31,7 +31,7 @@ use bridge_runtime_common::messages::{self, MessageBridge}; use frame_support::{parameter_types, weights::Weight, RuntimeDebug}; /// Default lane that is used to send messages to Millau. -pub const DEFAULT_XCM_LANE_TO_MILLAU: LaneId = [0, 0, 0, 0]; +pub const XCM_LANE: LaneId = [0, 0, 0, 0]; /// Weight of 2 XCM instructions is for simple `Trap(42)` program, coming through bridge /// (it is prepended with `UniversalOrigin` instruction). It is used just for simplest manual /// tests, confirming that we don't break encoding somewhere between. @@ -102,23 +102,8 @@ impl messages::ThisChainWithMessages for RialtoParachain { type RuntimeCall = RuntimeCall; type RuntimeOrigin = RuntimeOrigin; - fn is_message_accepted(send_origin: &Self::RuntimeOrigin, lane: &LaneId) -> bool { - let here_location = xcm::v3::MultiLocation::from(crate::UniversalLocation::get()); - match send_origin.caller { - OriginCaller::PolkadotXcm(pallet_xcm::Origin::Xcm(ref location)) - if *location == here_location => - { - log::trace!(target: "runtime::bridge", "Verifying message sent using XCM pallet to Millau"); - }, - _ => { - // keep in mind that in this case all messages are free (in term of fees) - // => it's just to keep testing bridge on our test deployments until we'll have a - // better option - log::trace!(target: "runtime::bridge", "Verifying message sent using messages pallet to Millau"); - }, - } - - *lane == [0, 0, 0, 0] || *lane == [0, 0, 0, 1] + fn is_message_accepted(_send_origin: &Self::RuntimeOrigin, _lane: &LaneId) -> bool { + true } fn maximal_pending_messages_at_outbound_lane() -> MessageNonce { diff --git a/bin/rialto/runtime/src/lib.rs b/bin/rialto/runtime/src/lib.rs index 378a471325d6..048be3ba25ef 100644 --- a/bin/rialto/runtime/src/lib.rs +++ b/bin/rialto/runtime/src/lib.rs @@ -442,6 +442,7 @@ parameter_types! { bp_millau::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX; pub const RootAccountForPayments: Option = None; pub const BridgedChainId: bp_runtime::ChainId = bp_runtime::MILLAU_CHAIN_ID; + pub ActiveOutboundLanes: &'static [bp_messages::LaneId] = &[millau_messages::XCM_LANE]; } /// Instance of the messages pallet used to relay messages to/from Millau chain. @@ -450,7 +451,7 @@ pub type WithMillauMessagesInstance = (); impl pallet_bridge_messages::Config for Runtime { type RuntimeEvent = RuntimeEvent; type WeightInfo = pallet_bridge_messages::weights::BridgeWeight; - type MaxMessagesToPruneAtOnce = MaxMessagesToPruneAtOnce; + type ActiveOutboundLanes = ActiveOutboundLanes; type MaxUnrewardedRelayerEntriesAtInboundLane = MaxUnrewardedRelayerEntriesAtInboundLane; type MaxUnconfirmedMessagesAtInboundLane = MaxUnconfirmedMessagesAtInboundLane; diff --git a/bin/rialto/runtime/src/millau_messages.rs b/bin/rialto/runtime/src/millau_messages.rs index 98030ac3ee52..221590e9f812 100644 --- a/bin/rialto/runtime/src/millau_messages.rs +++ b/bin/rialto/runtime/src/millau_messages.rs @@ -16,7 +16,7 @@ //! Everything required to serve Millau <-> Rialto messages. -use crate::{MillauGrandpaInstance, OriginCaller, Runtime, RuntimeCall, RuntimeOrigin}; +use crate::{MillauGrandpaInstance, Runtime, RuntimeCall, RuntimeOrigin}; use bp_messages::{ source_chain::TargetHeaderChain, @@ -27,6 +27,8 @@ use bp_runtime::{ChainId, MILLAU_CHAIN_ID, RIALTO_CHAIN_ID}; use bridge_runtime_common::messages::{self, MessageBridge}; use frame_support::{parameter_types, weights::Weight, RuntimeDebug}; +/// Lane that is used for XCM messages exchange. +pub const XCM_LANE: LaneId = [0, 0, 0, 0]; /// Weight of 2 XCM instructions is for simple `Trap(42)` program, coming through bridge /// (it is prepended with `UniversalOrigin` instruction). It is used just for simplest manual /// tests, confirming that we don't break encoding somewhere between. @@ -96,24 +98,8 @@ impl messages::ThisChainWithMessages for Rialto { type RuntimeOrigin = RuntimeOrigin; type RuntimeCall = RuntimeCall; - fn is_message_accepted(send_origin: &Self::RuntimeOrigin, lane: &LaneId) -> bool { - let here_location = - xcm::v3::MultiLocation::from(crate::xcm_config::UniversalLocation::get()); - match send_origin.caller { - OriginCaller::XcmPallet(pallet_xcm::Origin::Xcm(ref location)) - if *location == here_location => - { - log::trace!(target: "runtime::bridge", "Verifying message sent using XCM pallet to Millau"); - }, - _ => { - // keep in mind that in this case all messages are free (in term of fees) - // => it's just to keep testing bridge on our test deployments until we'll have a - // better option - log::trace!(target: "runtime::bridge", "Verifying message sent using messages pallet to Millau"); - }, - } - - *lane == [0, 0, 0, 0] || *lane == [0, 0, 0, 1] + fn is_message_accepted(_send_origin: &Self::RuntimeOrigin, _lane: &LaneId) -> bool { + true } fn maximal_pending_messages_at_outbound_lane() -> MessageNonce { diff --git a/bin/runtime-common/src/integrity.rs b/bin/runtime-common/src/integrity.rs index a9a2c06470c9..4c69a29b8217 100644 --- a/bin/runtime-common/src/integrity.rs +++ b/bin/runtime-common/src/integrity.rs @@ -204,9 +204,9 @@ where MI: 'static, { assert!( - R::MaxMessagesToPruneAtOnce::get() > 0, - "MaxMessagesToPruneAtOnce ({}) must be larger than zero", - R::MaxMessagesToPruneAtOnce::get(), + !R::ActiveOutboundLanes::get().is_empty(), + "ActiveOutboundLanes ({:?}) must not be empty", + R::ActiveOutboundLanes::get(), ); assert!( R::MaxUnrewardedRelayerEntriesAtInboundLane::get() <= params.max_unrewarded_relayers_in_bridged_confirmation_tx, diff --git a/modules/messages/src/lib.rs b/modules/messages/src/lib.rs index 8324aa75bc83..e2e3bef48558 100644 --- a/modules/messages/src/lib.rs +++ b/modules/messages/src/lib.rs @@ -65,6 +65,7 @@ use bp_messages::{ use bp_runtime::{BasicOperatingMode, ChainId, OwnedBridgeModule, Size}; use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::{dispatch::PostDispatchInfo, ensure, fail, traits::Get}; +use sp_runtime::traits::UniqueSaturatedFrom; use sp_std::{ cell::RefCell, collections::vec_deque::VecDeque, marker::PhantomData, ops::RangeInclusive, prelude::*, @@ -107,10 +108,8 @@ pub mod pallet { #[pallet::constant] type BridgedChainId: Get; - /// Maximal number of messages that may be pruned during maintenance. Maintenance occurs - /// whenever new message is sent. The reason is that if you want to use lane, you should - /// be ready to pay for its maintenance. - type MaxMessagesToPruneAtOnce: Get; + /// Get all active outbound lanes that the message pallet is serving. + type ActiveOutboundLanes: Get<&'static [LaneId]>; /// Maximal number of unrewarded relayer entries at inbound lane. Unrewarded means that the /// relayer has delivered messages, but either confirmations haven't been delivered back to /// the source chain, or we haven't received reward confirmations yet. @@ -191,6 +190,40 @@ pub mod pallet { type OperatingModeStorage = PalletOperatingMode; } + #[pallet::hooks] + impl, I: 'static> Hooks> for Pallet + where + u32: TryFrom<::BlockNumber>, + { + fn on_idle(_block: T::BlockNumber, remaining_weight: Weight) -> Weight { + // we'll need at least to read outbound lane state, kill a message and update lane state + let db_weight = T::DbWeight::get(); + if !remaining_weight.all_gte(db_weight.reads_writes(1, 2)) { + return Weight::zero() + } + + // messages from lane with index `i` in `ActiveOutboundLanes` are pruned when + // `System::block_number() % lanes.len() == i`. Otherwise we need to read lane states on + // every block, wasting the whole `remaining_weight` for nothing and causing starvation + // of the last lane pruning + let active_lanes = T::ActiveOutboundLanes::get(); + let active_lanes_len = (active_lanes.len() as u32).into(); + let active_lane_index = u32::unique_saturated_from( + frame_system::Pallet::::block_number() % active_lanes_len, + ); + let active_lane_id = active_lanes[active_lane_index as usize]; + + // first db read - outbound lane state + let mut active_lane = outbound_lane::(active_lane_id); + let mut used_weight = db_weight.reads(1); + // and here we'll have writes + used_weight += active_lane.prune_messages(db_weight, remaining_weight - used_weight); + + // we already checked we have enough `remaining_weight` to cover this `used_weight` + used_weight + } + } + #[pallet::call] impl, I: 'static> Pallet { /// Change `PalletOwner`. @@ -469,6 +502,8 @@ pub mod pallet { pub enum Error { /// Pallet is not in Normal operating mode. NotOperatingNormally, + /// The outbound lane is inactive. + InactiveOutboundLane, /// The message is too large to be sent over the bridge. MessageIsTooLarge, /// Message has been treated as invalid by chain verifier. @@ -614,8 +649,8 @@ fn send_message, I: 'static>( > { ensure_normal_operating_mode::()?; - // initially, actual (post-dispatch) weight is equal to pre-dispatch weight - let mut actual_weight = frame_support::weights::Weight::zero(); // TODO (https://github.com/paritytech/parity-bridges-common/issues/1647): remove this + // let's check if outbound lane is active + ensure!(T::ActiveOutboundLanes::get().contains(&lane_id), Error::::InactiveOutboundLane,); // let's first check if message can be delivered to target chain T::TargetHeaderChain::verify_message(&payload).map_err(|err| { @@ -653,15 +688,6 @@ fn send_message, I: 'static>( ); let nonce = lane.send_message(encoded_payload); - // message sender pays for pruning at most `MaxMessagesToPruneAtOnce` messages - // the cost of pruning every message is roughly single db write - // => lets refund sender if less than `MaxMessagesToPruneAtOnce` messages pruned - let max_messages_to_prune = T::MaxMessagesToPruneAtOnce::get(); - let pruned_messages = lane.prune_messages(max_messages_to_prune); - if let Some(extra_messages) = max_messages_to_prune.checked_sub(pruned_messages) { - actual_weight = actual_weight.saturating_sub(T::DbWeight::get().writes(extra_messages)); - } - log::trace!( target: LOG_TARGET, "Accepted message {} to lane {:?}. Message size: {:?}", @@ -672,6 +698,14 @@ fn send_message, I: 'static>( Pallet::::deposit_event(Event::MessageAccepted { lane_id, nonce }); + // we may introduce benchmarks for that, but no heavy ops planned here apart from + // db reads and writes. There are currently 2 db reads and 2 db writes: + // - one db read for operation mode check (`ensure_normal_operating_mode`); + // - one db read for outbound lane state (`outbound_lane`); + // - one db write for outbound lane state (`send_message`); + // - one db write for the message (`send_message`); + let actual_weight = T::DbWeight::get().reads_writes(2, 2); + Ok(SendMessageArtifacts { nonce, weight: actual_weight }) } @@ -851,17 +885,18 @@ fn verify_and_decode_messages_proof::receive_messages_delivery_proof( + RuntimeOrigin::signed(1), + TestMessagesDeliveryProof(Ok(( + TEST_LANE_ID, + InboundLaneData { + last_confirmed_nonce: 4, + relayers: vec![unrewarded_relayer(1, 4, TEST_RELAYER_A)] + .into_iter() + .collect(), + }, + ))), + UnrewardedRelayersState { + unrewarded_relayer_entries: 1, + messages_in_oldest_entry: 4, + total_messages: 4, + last_delivered_nonce: 4, + }, + )); + + // all 4 messages may be pruned now + assert_eq!( + outbound_lane::(TEST_LANE_ID).data().latest_received_nonce, + 4 + ); + assert_eq!( + outbound_lane::(TEST_LANE_ID).data().oldest_unpruned_nonce, + 1 + ); + System::::set_block_number(2); + + // if passed wight is too low to do anything + let dbw = DbWeight::get(); + assert_eq!( + Pallet::::on_idle(0, dbw.reads_writes(1, 1)), + Weight::zero(), + ); + assert_eq!( + outbound_lane::(TEST_LANE_ID).data().oldest_unpruned_nonce, + 1 + ); + + // if passed wight is enough to prune single message + assert_eq!( + Pallet::::on_idle(0, dbw.reads_writes(1, 2)), + dbw.reads_writes(1, 2), + ); + assert_eq!( + outbound_lane::(TEST_LANE_ID).data().oldest_unpruned_nonce, + 2 + ); + + // if passed wight is enough to prune two more messages + assert_eq!( + Pallet::::on_idle(0, dbw.reads_writes(1, 3)), + dbw.reads_writes(1, 3), + ); + assert_eq!( + outbound_lane::(TEST_LANE_ID).data().oldest_unpruned_nonce, + 4 + ); + + // if passed wight is enough to prune many messages + assert_eq!( + Pallet::::on_idle(0, dbw.reads_writes(100, 100)), + dbw.reads_writes(1, 2), + ); + assert_eq!( + outbound_lane::(TEST_LANE_ID).data().oldest_unpruned_nonce, + 5 + ); + }); + } + + #[test] + fn on_idle_callback_is_rotating_lanes_to_prune() { + run_test(|| { + // send + receive confirmation for lane 1 + send_regular_message(); + receive_messages_delivery_proof(); + // send + receive confirmation for lane 2 + assert_ok!(send_message::( + RuntimeOrigin::signed(1), + TEST_LANE_ID_2, + REGULAR_PAYLOAD, + )); + assert_ok!(Pallet::::receive_messages_delivery_proof( + RuntimeOrigin::signed(1), + TestMessagesDeliveryProof(Ok(( + TEST_LANE_ID_2, + InboundLaneData { + last_confirmed_nonce: 1, + relayers: vec![unrewarded_relayer(1, 1, TEST_RELAYER_A)] + .into_iter() + .collect(), + }, + ))), + UnrewardedRelayersState { + unrewarded_relayer_entries: 1, + messages_in_oldest_entry: 1, + total_messages: 1, + last_delivered_nonce: 1, + }, + )); + + // nothing is pruned yet + assert_eq!( + outbound_lane::(TEST_LANE_ID).data().latest_received_nonce, + 1 + ); + assert_eq!( + outbound_lane::(TEST_LANE_ID).data().oldest_unpruned_nonce, + 1 + ); + assert_eq!( + outbound_lane::(TEST_LANE_ID_2).data().latest_received_nonce, + 1 + ); + assert_eq!( + outbound_lane::(TEST_LANE_ID_2).data().oldest_unpruned_nonce, + 1 + ); + + // in block#2.on_idle lane messages of lane 1 are pruned + let dbw = DbWeight::get(); + System::::set_block_number(2); + assert_eq!( + Pallet::::on_idle(0, dbw.reads_writes(100, 100)), + dbw.reads_writes(1, 2), + ); + assert_eq!( + outbound_lane::(TEST_LANE_ID).data().oldest_unpruned_nonce, + 2 + ); + assert_eq!( + outbound_lane::(TEST_LANE_ID_2).data().oldest_unpruned_nonce, + 1 + ); + + // in block#3.on_idle lane messages of lane 2 are pruned + System::::set_block_number(3); + + assert_eq!( + Pallet::::on_idle(0, dbw.reads_writes(100, 100)), + dbw.reads_writes(1, 2), + ); + assert_eq!( + outbound_lane::(TEST_LANE_ID).data().oldest_unpruned_nonce, + 2 + ); + assert_eq!( + outbound_lane::(TEST_LANE_ID_2).data().oldest_unpruned_nonce, + 2 + ); + }); + } + + #[test] + fn outbound_message_from_unconfigured_lane_is_rejected() { + run_test(|| { + assert_noop!( + send_message::( + RuntimeOrigin::signed(1), + TEST_LANE_ID_3, + REGULAR_PAYLOAD, + ), + Error::::InactiveOutboundLane, + ); + }); + } + generate_owned_bridge_module_tests!( MessagesOperatingMode::Basic(BasicOperatingMode::Normal), MessagesOperatingMode::Basic(BasicOperatingMode::Halted) diff --git a/modules/messages/src/mock.rs b/modules/messages/src/mock.rs index 5d58883d0a6f..861f273d7e7c 100644 --- a/modules/messages/src/mock.rs +++ b/modules/messages/src/mock.rs @@ -141,12 +141,13 @@ parameter_types! { pub const MaxUnrewardedRelayerEntriesAtInboundLane: u64 = 16; pub const MaxUnconfirmedMessagesAtInboundLane: u64 = 32; pub const TestBridgedChainId: bp_runtime::ChainId = *b"test"; + pub const ActiveOutboundLanes: &'static [LaneId] = &[TEST_LANE_ID, TEST_LANE_ID_2]; } impl Config for TestRuntime { type RuntimeEvent = RuntimeEvent; type WeightInfo = (); - type MaxMessagesToPruneAtOnce = MaxMessagesToPruneAtOnce; + type ActiveOutboundLanes = ActiveOutboundLanes; type MaxUnrewardedRelayerEntriesAtInboundLane = MaxUnrewardedRelayerEntriesAtInboundLane; type MaxUnconfirmedMessagesAtInboundLane = MaxUnconfirmedMessagesAtInboundLane; @@ -192,6 +193,12 @@ pub const TEST_ERROR: &str = "Test error"; /// Lane that we're using in tests. pub const TEST_LANE_ID: LaneId = [0, 0, 0, 1]; +/// Secondary lane that we're using in tests. +pub const TEST_LANE_ID_2: LaneId = [0, 0, 0, 2]; + +/// Inactive outbound lane. +pub const TEST_LANE_ID_3: LaneId = [0, 0, 0, 3]; + /// Regular message payload. pub const REGULAR_PAYLOAD: TestPayload = message_payload(0, 50); diff --git a/modules/messages/src/outbound_lane.rs b/modules/messages/src/outbound_lane.rs index 890b85072ab9..3774dc7ff366 100644 --- a/modules/messages/src/outbound_lane.rs +++ b/modules/messages/src/outbound_lane.rs @@ -23,7 +23,11 @@ use bp_messages::{ DeliveredMessages, DispatchResultsBitVec, LaneId, MessageNonce, MessagePayload, OutboundLaneData, UnrewardedRelayer, }; -use frame_support::{BoundedVec, RuntimeDebug}; +use frame_support::{ + weights::{RuntimeDbWeight, Weight}, + BoundedVec, RuntimeDebug, +}; +use num_traits::Zero; use sp_std::collections::vec_deque::VecDeque; /// Outbound lane storage. @@ -147,24 +151,32 @@ impl OutboundLane { /// Prune at most `max_messages_to_prune` already received messages. /// - /// Returns number of pruned messages. - pub fn prune_messages(&mut self, max_messages_to_prune: MessageNonce) -> MessageNonce { - let mut pruned_messages = 0; + /// Returns weight, consumed by messages pruning and lane state update. + pub fn prune_messages( + &mut self, + db_weight: RuntimeDbWeight, + mut remaining_weight: Weight, + ) -> Weight { + let write_weight = db_weight.writes(1); + let two_writes_weight = write_weight + write_weight; + let mut spent_weight = Weight::zero(); let mut data = self.storage.data(); - while pruned_messages < max_messages_to_prune && + while remaining_weight.all_gte(two_writes_weight) && data.oldest_unpruned_nonce <= data.latest_received_nonce { self.storage.remove_message(&data.oldest_unpruned_nonce); - pruned_messages += 1; + spent_weight += write_weight; + remaining_weight -= write_weight; data.oldest_unpruned_nonce += 1; } - if pruned_messages > 0 { + if !spent_weight.is_zero() { + spent_weight += write_weight; self.storage.set_data(data); } - pruned_messages + spent_weight } } @@ -246,6 +258,7 @@ mod tests { }, outbound_lane, }; + use frame_support::weights::constants::RocksDbWeight; use sp_std::ops::RangeInclusive; fn unrewarded_relayers( @@ -413,27 +426,48 @@ mod tests { run_test(|| { let mut lane = outbound_lane::(TEST_LANE_ID); // when lane is empty, nothing is pruned - assert_eq!(lane.prune_messages(100), 0); + assert_eq!( + lane.prune_messages(RocksDbWeight::get(), RocksDbWeight::get().writes(101)), + Weight::zero() + ); assert_eq!(lane.storage.data().oldest_unpruned_nonce, 1); // when nothing is confirmed, nothing is pruned lane.send_message(outbound_message_data(REGULAR_PAYLOAD)); lane.send_message(outbound_message_data(REGULAR_PAYLOAD)); lane.send_message(outbound_message_data(REGULAR_PAYLOAD)); - assert_eq!(lane.prune_messages(100), 0); + assert!(lane.storage.message(&1).is_some()); + assert!(lane.storage.message(&2).is_some()); + assert!(lane.storage.message(&3).is_some()); + assert_eq!( + lane.prune_messages(RocksDbWeight::get(), RocksDbWeight::get().writes(101)), + Weight::zero() + ); assert_eq!(lane.storage.data().oldest_unpruned_nonce, 1); // after confirmation, some messages are received assert_eq!( lane.confirm_delivery(2, 2, &unrewarded_relayers(1..=2)), ReceivalConfirmationResult::ConfirmedMessages(delivered_messages(1..=2)), ); - assert_eq!(lane.prune_messages(100), 2); + assert_eq!( + lane.prune_messages(RocksDbWeight::get(), RocksDbWeight::get().writes(101)), + RocksDbWeight::get().writes(3), + ); + assert!(lane.storage.message(&1).is_none()); + assert!(lane.storage.message(&2).is_none()); + assert!(lane.storage.message(&3).is_some()); assert_eq!(lane.storage.data().oldest_unpruned_nonce, 3); // after last message is confirmed, everything is pruned assert_eq!( lane.confirm_delivery(1, 3, &unrewarded_relayers(3..=3)), ReceivalConfirmationResult::ConfirmedMessages(delivered_messages(3..=3)), ); - assert_eq!(lane.prune_messages(100), 1); + assert_eq!( + lane.prune_messages(RocksDbWeight::get(), RocksDbWeight::get().writes(101)), + RocksDbWeight::get().writes(2), + ); + assert!(lane.storage.message(&1).is_none()); + assert!(lane.storage.message(&2).is_none()); + assert!(lane.storage.message(&3).is_none()); assert_eq!(lane.storage.data().oldest_unpruned_nonce, 4); }); } diff --git a/modules/relayers/src/mock.rs b/modules/relayers/src/mock.rs index acb709407dca..7cf5575f5f38 100644 --- a/modules/relayers/src/mock.rs +++ b/modules/relayers/src/mock.rs @@ -91,13 +91,14 @@ impl pallet_balances::Config for TestRuntime { parameter_types! { pub const TestBridgedChainId: bp_runtime::ChainId = *b"test"; + pub ActiveOutboundLanes: &'static [bp_messages::LaneId] = &[[0, 0, 0, 0]]; } // we're not testing messages pallet here, so values in this config might be crazy impl pallet_bridge_messages::Config for TestRuntime { type RuntimeEvent = RuntimeEvent; type WeightInfo = (); - type MaxMessagesToPruneAtOnce = frame_support::traits::ConstU64<0>; + type ActiveOutboundLanes = ActiveOutboundLanes; type MaxUnrewardedRelayerEntriesAtInboundLane = frame_support::traits::ConstU64<8>; type MaxUnconfirmedMessagesAtInboundLane = frame_support::traits::ConstU64<8>;