Skip to content

Commit

Permalink
Fixed off-by-one when confirming rewards in messages pallet (#2075)
Browse files Browse the repository at this point in the history
* fixed off-by-one when confirming rewards in messages pallet

* Update modules/messages/src/inbound_lane.rs
  • Loading branch information
svyatonik authored Apr 26, 2023
1 parent a298be9 commit c01a63e
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 30 deletions.
34 changes: 30 additions & 4 deletions modules/messages/src/inbound_lane.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
// 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;
},
_ => {},
Expand Down Expand Up @@ -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<RuntimeInboundLaneStorage<TestRuntime, ()>>,
Expand Down Expand Up @@ -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,
},
);
});
}
}
29 changes: 6 additions & 23 deletions modules/messages/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -949,12 +949,12 @@ fn verify_and_decode_messages_proof<Chain: SourceHeaderChain, DispatchPayload: D
mod tests {
use super::*;
use crate::mock::{
message, message_payload, run_test, unrewarded_relayer, AccountId, DbWeight,
RuntimeEvent as TestEvent, RuntimeOrigin, TestDeliveryConfirmationPayments,
TestDeliveryPayments, TestMessagesDeliveryProof, TestMessagesProof, TestRelayer,
TestRuntime, TestWeightInfo, MAX_OUTBOUND_PAYLOAD_SIZE, PAYLOAD_REJECTED_BY_TARGET_CHAIN,
REGULAR_PAYLOAD, TEST_LANE_ID, TEST_LANE_ID_2, TEST_LANE_ID_3, TEST_RELAYER_A,
TEST_RELAYER_B,
inbound_unrewarded_relayers_state, message, message_payload, run_test, unrewarded_relayer,
AccountId, DbWeight, RuntimeEvent as TestEvent, RuntimeOrigin,
TestDeliveryConfirmationPayments, TestDeliveryPayments, TestMessagesDeliveryProof,
TestMessagesProof, TestRelayer, TestRuntime, TestWeightInfo, MAX_OUTBOUND_PAYLOAD_SIZE,
PAYLOAD_REJECTED_BY_TARGET_CHAIN, REGULAR_PAYLOAD, TEST_LANE_ID, TEST_LANE_ID_2,
TEST_LANE_ID_3, TEST_RELAYER_A, TEST_RELAYER_B,
};
use bp_messages::{BridgeMessagesCall, UnrewardedRelayer, UnrewardedRelayersState};
use bp_test_utils::generate_owned_bridge_module_tests;
Expand All @@ -973,23 +973,6 @@ mod tests {
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() {
get_ready_for_events();

Expand Down
22 changes: 19 additions & 3 deletions modules/messages/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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];
}
Expand Down Expand Up @@ -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::<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.
pub fn new_test_ext() -> sp_io::TestExternalities {
let mut t = frame_system::GenesisConfig::default().build_storage::<TestRuntime>().unwrap();
Expand Down

0 comments on commit c01a63e

Please sign in to comment.