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
This commit is contained in:
Svyatoslav Nikolsky
2023-04-26 12:46:11 +03:00
committed by Bastian Köcher
parent 97ef70c67a
commit dc61bcdc01
3 changed files with 55 additions and 30 deletions
+30 -4
View File
@@ -153,7 +153,7 @@ impl<S: InboundLaneStorage> InboundLane<S> {
// Note: There will be max. 1 record to update as we don't allow messages from relayers to // Note: There will be max. 1 record to update as we don't allow messages from relayers to
// overlap. // overlap.
match data.relayers.front_mut() { 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; entry.messages.begin = new_confirmed_nonce + 1;
}, },
_ => {}, _ => {},
@@ -221,12 +221,13 @@ mod tests {
use crate::{ use crate::{
inbound_lane, inbound_lane,
mock::{ mock::{
dispatch_result, inbound_message_data, run_test, unrewarded_relayer, dispatch_result, inbound_message_data, inbound_unrewarded_relayers_state, run_test,
TestMessageDispatch, TestRuntime, REGULAR_PAYLOAD, TEST_LANE_ID, TEST_RELAYER_A, unrewarded_relayer, TestMessageDispatch, TestRuntime, REGULAR_PAYLOAD, TEST_LANE_ID,
TEST_RELAYER_B, TEST_RELAYER_C, TEST_RELAYER_A, TEST_RELAYER_B, TEST_RELAYER_C,
}, },
RuntimeInboundLaneStorage, RuntimeInboundLaneStorage,
}; };
use bp_messages::UnrewardedRelayersState;
fn receive_regular_message( fn receive_regular_message(
lane: &mut InboundLane<RuntimeInboundLaneStorage<TestRuntime, ()>>, lane: &mut InboundLane<RuntimeInboundLaneStorage<TestRuntime, ()>>,
@@ -545,4 +546,29 @@ mod tests {
); );
}); });
} }
#[test]
fn first_message_is_confirmed_correctly() {
run_test(|| {
let mut lane = inbound_lane::<TestRuntime, _>(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,
},
);
});
}
} }
+6 -23
View File
@@ -949,12 +949,12 @@ fn verify_and_decode_messages_proof<Chain: SourceHeaderChain, DispatchPayload: D
mod tests { mod tests {
use super::*; use super::*;
use crate::mock::{ use crate::mock::{
message, message_payload, run_test, unrewarded_relayer, AccountId, DbWeight, inbound_unrewarded_relayers_state, message, message_payload, run_test, unrewarded_relayer,
RuntimeEvent as TestEvent, RuntimeOrigin, TestDeliveryConfirmationPayments, AccountId, DbWeight, RuntimeEvent as TestEvent, RuntimeOrigin,
TestDeliveryPayments, TestMessagesDeliveryProof, TestMessagesProof, TestRelayer, TestDeliveryConfirmationPayments, TestDeliveryPayments, TestMessagesDeliveryProof,
TestRuntime, TestWeightInfo, MAX_OUTBOUND_PAYLOAD_SIZE, PAYLOAD_REJECTED_BY_TARGET_CHAIN, TestMessagesProof, TestRelayer, TestRuntime, TestWeightInfo, MAX_OUTBOUND_PAYLOAD_SIZE,
REGULAR_PAYLOAD, TEST_LANE_ID, TEST_LANE_ID_2, TEST_LANE_ID_3, TEST_RELAYER_A, PAYLOAD_REJECTED_BY_TARGET_CHAIN, REGULAR_PAYLOAD, TEST_LANE_ID, TEST_LANE_ID_2,
TEST_RELAYER_B, TEST_LANE_ID_3, TEST_RELAYER_A, TEST_RELAYER_B,
}; };
use bp_messages::{BridgeMessagesCall, UnrewardedRelayer, UnrewardedRelayersState}; use bp_messages::{BridgeMessagesCall, UnrewardedRelayer, UnrewardedRelayersState};
use bp_test_utils::generate_owned_bridge_module_tests; use bp_test_utils::generate_owned_bridge_module_tests;
@@ -973,23 +973,6 @@ mod tests {
System::<TestRuntime>::reset_events(); System::<TestRuntime>::reset_events();
} }
fn inbound_unrewarded_relayers_state(
lane: bp_messages::LaneId,
) -> bp_messages::UnrewardedRelayersState {
let inbound_lane_data = InboundLanes::<TestRuntime, ()>::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() { fn send_regular_message() {
get_ready_for_events(); get_ready_for_events();
+19 -3
View File
@@ -26,8 +26,8 @@ use bp_messages::{
DeliveryPayments, DispatchMessage, DispatchMessageData, MessageDispatch, DeliveryPayments, DispatchMessage, DispatchMessageData, MessageDispatch,
ProvedLaneMessages, ProvedMessages, SourceHeaderChain, ProvedLaneMessages, ProvedMessages, SourceHeaderChain,
}, },
DeliveredMessages, InboundLaneData, LaneId, Message, MessageKey, MessageNonce, MessagePayload, total_unrewarded_messages, DeliveredMessages, InboundLaneData, LaneId, Message, MessageKey,
OutboundLaneData, UnrewardedRelayer, MessageNonce, MessagePayload, OutboundLaneData, UnrewardedRelayer, UnrewardedRelayersState,
}; };
use bp_runtime::{messages::MessageDispatchResult, Size}; use bp_runtime::{messages::MessageDispatchResult, Size};
use codec::{Decode, Encode}; use codec::{Decode, Encode};
@@ -142,7 +142,7 @@ impl pallet_balances::Config for TestRuntime {
parameter_types! { parameter_types! {
pub const MaxMessagesToPruneAtOnce: u64 = 10; pub const MaxMessagesToPruneAtOnce: u64 = 10;
pub const MaxUnrewardedRelayerEntriesAtInboundLane: u64 = 16; 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 TestBridgedChainId: bp_runtime::ChainId = *b"test";
pub const ActiveOutboundLanes: &'static [LaneId] = &[TEST_LANE_ID, TEST_LANE_ID_2]; 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 } } 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::<TestRuntime, ()>::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. /// Return test externalities to use in tests.
pub fn new_test_ext() -> sp_io::TestExternalities { pub fn new_test_ext() -> sp_io::TestExternalities {
let mut t = frame_system::GenesisConfig::default().build_storage::<TestRuntime>().unwrap(); let mut t = frame_system::GenesisConfig::default().build_storage::<TestRuntime>().unwrap();