From c01a63efd8900c66d0af313beeb482e0cd334064 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Wed, 26 Apr 2023 12:46:11 +0300 Subject: [PATCH] Fixed off-by-one when confirming rewards in messages pallet (#2075) * fixed off-by-one when confirming rewards in messages pallet * Update modules/messages/src/inbound_lane.rs --- modules/messages/src/inbound_lane.rs | 34 ++++++++++++++++++++++++---- modules/messages/src/lib.rs | 29 +++++------------------- modules/messages/src/mock.rs | 22 +++++++++++++++--- 3 files changed, 55 insertions(+), 30 deletions(-) diff --git a/modules/messages/src/inbound_lane.rs b/modules/messages/src/inbound_lane.rs index 59ff5667195..fa6480d962f 100644 --- a/modules/messages/src/inbound_lane.rs +++ b/modules/messages/src/inbound_lane.rs @@ -153,7 +153,7 @@ impl InboundLane { // Note: There will be max. 1 record to update as we don't allow messages from relayers to // overlap. match data.relayers.front_mut() { - Some(entry) if entry.messages.begin < new_confirmed_nonce => { + Some(entry) if entry.messages.begin <= new_confirmed_nonce => { entry.messages.begin = new_confirmed_nonce + 1; }, _ => {}, @@ -221,12 +221,13 @@ mod tests { use crate::{ inbound_lane, mock::{ - dispatch_result, inbound_message_data, run_test, unrewarded_relayer, - TestMessageDispatch, TestRuntime, REGULAR_PAYLOAD, TEST_LANE_ID, TEST_RELAYER_A, - TEST_RELAYER_B, TEST_RELAYER_C, + dispatch_result, inbound_message_data, inbound_unrewarded_relayers_state, run_test, + unrewarded_relayer, TestMessageDispatch, TestRuntime, REGULAR_PAYLOAD, TEST_LANE_ID, + TEST_RELAYER_A, TEST_RELAYER_B, TEST_RELAYER_C, }, RuntimeInboundLaneStorage, }; + use bp_messages::UnrewardedRelayersState; fn receive_regular_message( lane: &mut InboundLane>, @@ -545,4 +546,29 @@ mod tests { ); }); } + + #[test] + fn first_message_is_confirmed_correctly() { + run_test(|| { + let mut lane = inbound_lane::(TEST_LANE_ID); + receive_regular_message(&mut lane, 1); + receive_regular_message(&mut lane, 2); + assert_eq!( + lane.receive_state_update(OutboundLaneData { + latest_received_nonce: 1, + ..Default::default() + }), + Some(1), + ); + assert_eq!( + inbound_unrewarded_relayers_state(TEST_LANE_ID), + UnrewardedRelayersState { + unrewarded_relayer_entries: 1, + messages_in_oldest_entry: 1, + total_messages: 1, + last_delivered_nonce: 2, + }, + ); + }); + } } diff --git a/modules/messages/src/lib.rs b/modules/messages/src/lib.rs index c94f5ffa752..6985b9018e7 100644 --- a/modules/messages/src/lib.rs +++ b/modules/messages/src/lib.rs @@ -949,12 +949,12 @@ fn verify_and_decode_messages_proof::reset_events(); } - fn inbound_unrewarded_relayers_state( - lane: bp_messages::LaneId, - ) -> bp_messages::UnrewardedRelayersState { - let inbound_lane_data = InboundLanes::::get(lane).0; - let last_delivered_nonce = inbound_lane_data.last_delivered_nonce(); - let relayers = inbound_lane_data.relayers; - bp_messages::UnrewardedRelayersState { - unrewarded_relayer_entries: relayers.len() as _, - messages_in_oldest_entry: relayers - .front() - .map(|entry| 1 + entry.messages.end - entry.messages.begin) - .unwrap_or(0), - total_messages: total_unrewarded_messages(&relayers).unwrap_or(MessageNonce::MAX), - last_delivered_nonce, - } - } - fn send_regular_message() { get_ready_for_events(); diff --git a/modules/messages/src/mock.rs b/modules/messages/src/mock.rs index 75f05b4820a..270222a1fd6 100644 --- a/modules/messages/src/mock.rs +++ b/modules/messages/src/mock.rs @@ -26,8 +26,8 @@ use bp_messages::{ DeliveryPayments, DispatchMessage, DispatchMessageData, MessageDispatch, ProvedLaneMessages, ProvedMessages, SourceHeaderChain, }, - DeliveredMessages, InboundLaneData, LaneId, Message, MessageKey, MessageNonce, MessagePayload, - OutboundLaneData, UnrewardedRelayer, + total_unrewarded_messages, DeliveredMessages, InboundLaneData, LaneId, Message, MessageKey, + MessageNonce, MessagePayload, OutboundLaneData, UnrewardedRelayer, UnrewardedRelayersState, }; use bp_runtime::{messages::MessageDispatchResult, Size}; use codec::{Decode, Encode}; @@ -142,7 +142,7 @@ impl pallet_balances::Config for TestRuntime { parameter_types! { pub const MaxMessagesToPruneAtOnce: u64 = 10; pub const MaxUnrewardedRelayerEntriesAtInboundLane: u64 = 16; - pub const MaxUnconfirmedMessagesAtInboundLane: u64 = 32; + pub const MaxUnconfirmedMessagesAtInboundLane: u64 = 128; pub const TestBridgedChainId: bp_runtime::ChainId = *b"test"; pub const ActiveOutboundLanes: &'static [LaneId] = &[TEST_LANE_ID, TEST_LANE_ID_2]; } @@ -483,6 +483,22 @@ pub fn unrewarded_relayer( UnrewardedRelayer { relayer, messages: DeliveredMessages { begin, end } } } +/// Returns unrewarded relayers state at given lane. +pub fn inbound_unrewarded_relayers_state(lane: bp_messages::LaneId) -> UnrewardedRelayersState { + let inbound_lane_data = crate::InboundLanes::::get(lane).0; + let last_delivered_nonce = inbound_lane_data.last_delivered_nonce(); + let relayers = inbound_lane_data.relayers; + UnrewardedRelayersState { + unrewarded_relayer_entries: relayers.len() as _, + messages_in_oldest_entry: relayers + .front() + .map(|entry| 1 + entry.messages.end - entry.messages.begin) + .unwrap_or(0), + total_messages: total_unrewarded_messages(&relayers).unwrap_or(MessageNonce::MAX), + last_delivered_nonce, + } +} + /// Return test externalities to use in tests. pub fn new_test_ext() -> sp_io::TestExternalities { let mut t = frame_system::GenesisConfig::default().build_storage::().unwrap();