Skip to content

Commit

Permalink
Allow delivery confirmations on closed outbound lanes (#2257)
Browse files Browse the repository at this point in the history 
* allow delivery confirmations on closed outbound lanes

* fix benchmarks compilation
  • Loading branch information
@svyatonik @bkontur
svyatonik authored and bkontur committed May 17, 2024
1 parent b0202bd commit 3a76c27
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 46 deletions.
4 changes: 2 additions & 2 deletions bridges/modules/messages/src/benchmarking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
#![cfg(feature = "runtime-benchmarks")]

use crate::{
outbound_lane, weights_ext::EXPECTED_DEFAULT_MESSAGE_LENGTH, BridgedChainOf, Call,
active_outbound_lane, weights_ext::EXPECTED_DEFAULT_MESSAGE_LENGTH, BridgedChainOf, Call,
InboundLanes, OutboundLanes,
};

Expand Down Expand Up @@ -120,7 +120,7 @@ fn send_regular_message<T: Config<I>, I: 'static>() {
},
);

let mut outbound_lane = outbound_lane::<T, I>(T::bench_lane_id()).unwrap();
let mut outbound_lane = active_outbound_lane::<T, I>(T::bench_lane_id()).unwrap();
outbound_lane.send_message(BoundedVec::try_from(vec![]).expect("We craft valid messages"));
}

Expand Down
26 changes: 13 additions & 13 deletions bridges/modules/messages/src/inbound_lane.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ impl<S: InboundLaneStorage> InboundLane<S> {
#[cfg(test)]
mod tests {
use super::*;
use crate::{inbound_lane, lanes_manager::RuntimeInboundLaneStorage, tests::mock::*};
use crate::{active_inbound_lane, lanes_manager::RuntimeInboundLaneStorage, tests::mock::*};
use bp_messages::UnrewardedRelayersState;

fn receive_regular_message(
Expand All @@ -255,7 +255,7 @@ mod tests {
#[test]
fn receive_status_update_ignores_status_from_the_future() {
run_test(|| {
let mut lane = inbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
let mut lane = active_inbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
receive_regular_message(&mut lane, 1);
assert_eq!(
lane.receive_state_update(OutboundLaneData {
Expand All @@ -272,7 +272,7 @@ mod tests {
#[test]
fn receive_status_update_ignores_obsolete_status() {
run_test(|| {
let mut lane = inbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
let mut lane = active_inbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
receive_regular_message(&mut lane, 1);
receive_regular_message(&mut lane, 2);
receive_regular_message(&mut lane, 3);
Expand All @@ -299,7 +299,7 @@ mod tests {
#[test]
fn receive_status_update_works() {
run_test(|| {
let mut lane = inbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
let mut lane = active_inbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
receive_regular_message(&mut lane, 1);
receive_regular_message(&mut lane, 2);
receive_regular_message(&mut lane, 3);
Expand Down Expand Up @@ -337,7 +337,7 @@ mod tests {
#[test]
fn receive_status_update_works_with_batches_from_relayers() {
run_test(|| {
let mut lane = inbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
let mut lane = active_inbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
let mut seed_storage_data = lane.storage.data();
// Prepare data
seed_storage_data.last_confirmed_nonce = 0;
Expand Down Expand Up @@ -368,7 +368,7 @@ mod tests {
#[test]
fn fails_to_receive_message_with_incorrect_nonce() {
run_test(|| {
let mut lane = inbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
let mut lane = active_inbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
assert_eq!(
lane.receive_message::<TestMessageDispatch>(
&TEST_RELAYER_A,
Expand All @@ -384,7 +384,7 @@ mod tests {
#[test]
fn fails_to_receive_messages_above_unrewarded_relayer_entries_limit_per_lane() {
run_test(|| {
let mut lane = inbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
let mut lane = active_inbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
let max_nonce = BridgedChain::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX;
for current_nonce in 1..max_nonce + 1 {
assert_eq!(
Expand Down Expand Up @@ -420,7 +420,7 @@ mod tests {
#[test]
fn fails_to_receive_messages_above_unconfirmed_messages_limit_per_lane() {
run_test(|| {
let mut lane = inbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
let mut lane = active_inbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
let max_nonce = BridgedChain::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX;
for current_nonce in 1..=max_nonce {
assert_eq!(
Expand Down Expand Up @@ -456,7 +456,7 @@ mod tests {
#[test]
fn correctly_receives_following_messages_from_two_relayers_alternately() {
run_test(|| {
let mut lane = inbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
let mut lane = active_inbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
assert_eq!(
lane.receive_message::<TestMessageDispatch>(
&TEST_RELAYER_A,
Expand Down Expand Up @@ -495,7 +495,7 @@ mod tests {
#[test]
fn rejects_same_message_from_two_different_relayers() {
run_test(|| {
let mut lane = inbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
let mut lane = active_inbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
assert_eq!(
lane.receive_message::<TestMessageDispatch>(
&TEST_RELAYER_A,
Expand All @@ -518,7 +518,7 @@ mod tests {
#[test]
fn correct_message_is_processed_instantly() {
run_test(|| {
let mut lane = inbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
let mut lane = active_inbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
receive_regular_message(&mut lane, 1);
assert_eq!(lane.storage.data().last_delivered_nonce(), 1);
});
Expand All @@ -527,7 +527,7 @@ mod tests {
#[test]
fn unspent_weight_is_returned_by_receive_message() {
run_test(|| {
let mut lane = inbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
let mut lane = active_inbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
let mut payload = REGULAR_PAYLOAD;
*payload.dispatch_result.unspent_weight.ref_time_mut() = 1;
assert_eq!(
Expand All @@ -544,7 +544,7 @@ mod tests {
#[test]
fn first_message_is_confirmed_correctly() {
run_test(|| {
let mut lane = inbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
let mut lane = active_inbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
receive_regular_message(&mut lane, 1);
receive_regular_message(&mut lane, 2);
assert_eq!(
Expand Down
23 changes: 16 additions & 7 deletions bridges/modules/messages/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ pub mod pallet {
let mut messages_received_status = Vec::with_capacity(messages.len());
let mut dispatch_weight_left = dispatch_weight;
for (lane_id, lane_data) in messages {
let mut lane = inbound_lane::<T, I>(lane_id)?;
let mut lane = active_inbound_lane::<T, I>(lane_id)?;

// subtract extra storage proof bytes from the actual PoV size - there may be
// less unrewarded relayers than the maximal configured value
Expand Down Expand Up @@ -375,7 +375,7 @@ pub mod pallet {
);

// mark messages as delivered
let mut lane = outbound_lane::<T, I>(lane_id)?;
let mut lane = any_state_outbound_lane::<T, I>(lane_id)?;
let last_delivered_nonce = lane_data.last_delivered_nonce();
let confirmed_messages = lane
.confirm_delivery(
Expand Down Expand Up @@ -646,7 +646,7 @@ where
ensure_normal_operating_mode::<T, I>()?;

// check lane
let lane = outbound_lane::<T, I>(lane_id)?;
let lane = active_outbound_lane::<T, I>(lane_id)?;

Ok(SendMessageArgs {
lane_id,
Expand Down Expand Up @@ -691,24 +691,33 @@ fn ensure_normal_operating_mode<T: Config<I>, I: 'static>() -> Result<(), Error<
Err(Error::<T, I>::NotOperatingNormally)
}

/// Creates new inbound lane object, backed by runtime storage.
fn inbound_lane<T: Config<I>, I: 'static>(
/// Creates new inbound lane object, backed by runtime storage. Lane must be active.
fn active_inbound_lane<T: Config<I>, I: 'static>(
lane_id: LaneId,
) -> Result<InboundLane<RuntimeInboundLaneStorage<T, I>>, Error<T, I>> {
LanesManager::<T, I>::new()
.active_inbound_lane(lane_id)
.map_err(Error::LanesManager)
}

/// Creates new outbound lane object, backed by runtime storage.
fn outbound_lane<T: Config<I>, I: 'static>(
/// Creates new outbound lane object, backed by runtime storage. Lane must be active.
fn active_outbound_lane<T: Config<I>, I: 'static>(
lane_id: LaneId,
) -> Result<OutboundLane<RuntimeOutboundLaneStorage<T, I>>, Error<T, I>> {
LanesManager::<T, I>::new()
.active_outbound_lane(lane_id)
.map_err(Error::LanesManager)
}

/// Creates new outbound lane object, backed by runtime storage.
fn any_state_outbound_lane<T: Config<I>, I: 'static>(
lane_id: LaneId,
) -> Result<OutboundLane<RuntimeOutboundLaneStorage<T, I>>, Error<T, I>> {
LanesManager::<T, I>::new()
.any_state_outbound_lane(lane_id)
.map_err(Error::LanesManager)
}

/// Verify messages proof and return proved messages with decoded payload.
fn verify_and_decode_messages_proof<T: Config<I>, I: 'static>(
proof: FromBridgedChainMessagesProof<HashOf<BridgedChainOf<T, I>>>,
Expand Down
14 changes: 7 additions & 7 deletions bridges/modules/messages/src/outbound_lane.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ fn ensure_unrewarded_relayers_are_correct<RelayerId>(
mod tests {
use super::*;
use crate::{
outbound_lane,
active_outbound_lane,
tests::mock::{
outbound_message_data, run_test, test_lane_id, unrewarded_relayer, TestRelayer,
TestRuntime, REGULAR_PAYLOAD,
Expand All @@ -248,7 +248,7 @@ mod tests {
relayers: &VecDeque<UnrewardedRelayer<TestRelayer>>,
) -> Result<Option<DeliveredMessages>, ReceptionConfirmationError> {
run_test(|| {
let mut lane = outbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
let mut lane = active_outbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
Expand All @@ -264,7 +264,7 @@ mod tests {
#[test]
fn send_message_works() {
run_test(|| {
let mut lane = outbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
let mut lane = active_outbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
assert_eq!(lane.storage.data().latest_generated_nonce, 0);
assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), 1);
assert!(lane.storage.message(&1).is_some());
Expand All @@ -275,7 +275,7 @@ mod tests {
#[test]
fn confirm_delivery_works() {
run_test(|| {
let mut lane = outbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
let mut lane = active_outbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), 1);
assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), 2);
assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), 3);
Expand All @@ -295,7 +295,7 @@ mod tests {
#[test]
fn confirm_partial_delivery_works() {
run_test(|| {
let mut lane = outbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
let mut lane = active_outbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), 1);
assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), 2);
assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), 3);
Expand Down Expand Up @@ -324,7 +324,7 @@ mod tests {
#[test]
fn confirm_delivery_rejects_nonce_lesser_than_latest_received() {
run_test(|| {
let mut lane = outbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
let mut lane = active_outbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
Expand Down Expand Up @@ -404,7 +404,7 @@ mod tests {
#[test]
fn confirm_delivery_detects_when_more_than_expected_messages_are_confirmed() {
run_test(|| {
let mut lane = outbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
let mut lane = active_outbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
Expand Down
37 changes: 20 additions & 17 deletions bridges/modules/messages/src/tests/pallet_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
//! Pallet-level tests.

use crate::{
active_outbound_lane,
lanes_manager::RuntimeInboundLaneStorage,
outbound_lane,
outbound_lane::ReceptionConfirmationError,
tests::mock::{RuntimeEvent as TestEvent, *},
weights_ext::WeightInfoExt,
Expand Down Expand Up @@ -54,7 +54,7 @@ fn get_ready_for_events() {
fn send_regular_message(lane_id: LaneId) {
get_ready_for_events();

let outbound_lane = outbound_lane::<TestRuntime, ()>(lane_id).unwrap();
let outbound_lane = active_outbound_lane::<TestRuntime, ()>(lane_id).unwrap();
let message_nonce = outbound_lane.data().latest_generated_nonce + 1;
let prev_enqueued_messages = outbound_lane.data().queued_messages().saturating_len();
let valid_message = Pallet::<TestRuntime, ()>::validate_message(lane_id, &REGULAR_PAYLOAD)
Expand Down Expand Up @@ -401,6 +401,24 @@ fn receive_messages_delivery_proof_works() {
});
}

#[test]
fn receive_messages_delivery_proof_works_on_closed_outbound_lanes() {
run_test(|| {
send_regular_message(test_lane_id());
active_outbound_lane::<TestRuntime, ()>(test_lane_id())
.unwrap()
.set_state(LaneState::Closed);
receive_messages_delivery_proof();

assert_eq!(
OutboundLanes::<TestRuntime, ()>::get(test_lane_id())
.unwrap()
.latest_received_nonce,
1,
);
});
}

#[test]
fn receive_messages_delivery_proof_rewards_relayers() {
run_test(|| {
Expand Down Expand Up @@ -1055,20 +1073,5 @@ fn receive_messages_delivery_proof_fails_if_outbound_lane_is_unknown() {
),
Error::<TestRuntime, ()>::LanesManager(LanesManagerError::UnknownOutboundLane),
);

let proof = make_proof(closed_lane_id());
assert_noop!(
Pallet::<TestRuntime>::receive_messages_delivery_proof(
RuntimeOrigin::signed(1),
proof,
UnrewardedRelayersState {
unrewarded_relayer_entries: 1,
messages_in_oldest_entry: 1,
total_messages: 1,
last_delivered_nonce: 1,
},
),
Error::<TestRuntime, ()>::LanesManager(LanesManagerError::ClosedOutboundLane),
);
});
}

0 comments on commit 3a76c27

Please sign in to comment.