From 3a76c2703230603e826227da20062e857b708d96 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Tue, 11 Jul 2023 17:03:04 +0300 Subject: [PATCH] Allow delivery confirmations on closed outbound lanes (#2257) * allow delivery confirmations on closed outbound lanes * fix benchmarks compilation --- bridges/modules/messages/src/benchmarking.rs | 4 +- bridges/modules/messages/src/inbound_lane.rs | 26 ++++++------- bridges/modules/messages/src/lib.rs | 23 ++++++++---- bridges/modules/messages/src/outbound_lane.rs | 14 +++---- .../messages/src/tests/pallet_tests.rs | 37 ++++++++++--------- 5 files changed, 58 insertions(+), 46 deletions(-) diff --git a/bridges/modules/messages/src/benchmarking.rs b/bridges/modules/messages/src/benchmarking.rs index fec878f1fc743..eca558b6e2b4c 100644 --- a/bridges/modules/messages/src/benchmarking.rs +++ b/bridges/modules/messages/src/benchmarking.rs @@ -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, }; @@ -120,7 +120,7 @@ fn send_regular_message, I: 'static>() { }, ); - let mut outbound_lane = outbound_lane::(T::bench_lane_id()).unwrap(); + let mut outbound_lane = active_outbound_lane::(T::bench_lane_id()).unwrap(); outbound_lane.send_message(BoundedVec::try_from(vec![]).expect("We craft valid messages")); } diff --git a/bridges/modules/messages/src/inbound_lane.rs b/bridges/modules/messages/src/inbound_lane.rs index ccc1b1b60caa1..65240feb7194a 100644 --- a/bridges/modules/messages/src/inbound_lane.rs +++ b/bridges/modules/messages/src/inbound_lane.rs @@ -235,7 +235,7 @@ impl InboundLane { #[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( @@ -255,7 +255,7 @@ mod tests { #[test] fn receive_status_update_ignores_status_from_the_future() { run_test(|| { - let mut lane = inbound_lane::(test_lane_id()).unwrap(); + let mut lane = active_inbound_lane::(test_lane_id()).unwrap(); receive_regular_message(&mut lane, 1); assert_eq!( lane.receive_state_update(OutboundLaneData { @@ -272,7 +272,7 @@ mod tests { #[test] fn receive_status_update_ignores_obsolete_status() { run_test(|| { - let mut lane = inbound_lane::(test_lane_id()).unwrap(); + let mut lane = active_inbound_lane::(test_lane_id()).unwrap(); receive_regular_message(&mut lane, 1); receive_regular_message(&mut lane, 2); receive_regular_message(&mut lane, 3); @@ -299,7 +299,7 @@ mod tests { #[test] fn receive_status_update_works() { run_test(|| { - let mut lane = inbound_lane::(test_lane_id()).unwrap(); + let mut lane = active_inbound_lane::(test_lane_id()).unwrap(); receive_regular_message(&mut lane, 1); receive_regular_message(&mut lane, 2); receive_regular_message(&mut lane, 3); @@ -337,7 +337,7 @@ mod tests { #[test] fn receive_status_update_works_with_batches_from_relayers() { run_test(|| { - let mut lane = inbound_lane::(test_lane_id()).unwrap(); + let mut lane = active_inbound_lane::(test_lane_id()).unwrap(); let mut seed_storage_data = lane.storage.data(); // Prepare data seed_storage_data.last_confirmed_nonce = 0; @@ -368,7 +368,7 @@ mod tests { #[test] fn fails_to_receive_message_with_incorrect_nonce() { run_test(|| { - let mut lane = inbound_lane::(test_lane_id()).unwrap(); + let mut lane = active_inbound_lane::(test_lane_id()).unwrap(); assert_eq!( lane.receive_message::( &TEST_RELAYER_A, @@ -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::(test_lane_id()).unwrap(); + let mut lane = active_inbound_lane::(test_lane_id()).unwrap(); let max_nonce = BridgedChain::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX; for current_nonce in 1..max_nonce + 1 { assert_eq!( @@ -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::(test_lane_id()).unwrap(); + let mut lane = active_inbound_lane::(test_lane_id()).unwrap(); let max_nonce = BridgedChain::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX; for current_nonce in 1..=max_nonce { assert_eq!( @@ -456,7 +456,7 @@ mod tests { #[test] fn correctly_receives_following_messages_from_two_relayers_alternately() { run_test(|| { - let mut lane = inbound_lane::(test_lane_id()).unwrap(); + let mut lane = active_inbound_lane::(test_lane_id()).unwrap(); assert_eq!( lane.receive_message::( &TEST_RELAYER_A, @@ -495,7 +495,7 @@ mod tests { #[test] fn rejects_same_message_from_two_different_relayers() { run_test(|| { - let mut lane = inbound_lane::(test_lane_id()).unwrap(); + let mut lane = active_inbound_lane::(test_lane_id()).unwrap(); assert_eq!( lane.receive_message::( &TEST_RELAYER_A, @@ -518,7 +518,7 @@ mod tests { #[test] fn correct_message_is_processed_instantly() { run_test(|| { - let mut lane = inbound_lane::(test_lane_id()).unwrap(); + let mut lane = active_inbound_lane::(test_lane_id()).unwrap(); receive_regular_message(&mut lane, 1); assert_eq!(lane.storage.data().last_delivered_nonce(), 1); }); @@ -527,7 +527,7 @@ mod tests { #[test] fn unspent_weight_is_returned_by_receive_message() { run_test(|| { - let mut lane = inbound_lane::(test_lane_id()).unwrap(); + let mut lane = active_inbound_lane::(test_lane_id()).unwrap(); let mut payload = REGULAR_PAYLOAD; *payload.dispatch_result.unspent_weight.ref_time_mut() = 1; assert_eq!( @@ -544,7 +544,7 @@ mod tests { #[test] fn first_message_is_confirmed_correctly() { run_test(|| { - let mut lane = inbound_lane::(test_lane_id()).unwrap(); + let mut lane = active_inbound_lane::(test_lane_id()).unwrap(); receive_regular_message(&mut lane, 1); receive_regular_message(&mut lane, 2); assert_eq!( diff --git a/bridges/modules/messages/src/lib.rs b/bridges/modules/messages/src/lib.rs index bd18f592c930a..87dc3a52899c8 100644 --- a/bridges/modules/messages/src/lib.rs +++ b/bridges/modules/messages/src/lib.rs @@ -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::(lane_id)?; + let mut lane = active_inbound_lane::(lane_id)?; // subtract extra storage proof bytes from the actual PoV size - there may be // less unrewarded relayers than the maximal configured value @@ -375,7 +375,7 @@ pub mod pallet { ); // mark messages as delivered - let mut lane = outbound_lane::(lane_id)?; + let mut lane = any_state_outbound_lane::(lane_id)?; let last_delivered_nonce = lane_data.last_delivered_nonce(); let confirmed_messages = lane .confirm_delivery( @@ -646,7 +646,7 @@ where ensure_normal_operating_mode::()?; // check lane - let lane = outbound_lane::(lane_id)?; + let lane = active_outbound_lane::(lane_id)?; Ok(SendMessageArgs { lane_id, @@ -691,8 +691,8 @@ fn ensure_normal_operating_mode, I: 'static>() -> Result<(), Error< Err(Error::::NotOperatingNormally) } -/// Creates new inbound lane object, backed by runtime storage. -fn inbound_lane, I: 'static>( +/// Creates new inbound lane object, backed by runtime storage. Lane must be active. +fn active_inbound_lane, I: 'static>( lane_id: LaneId, ) -> Result>, Error> { LanesManager::::new() @@ -700,8 +700,8 @@ fn inbound_lane, I: 'static>( .map_err(Error::LanesManager) } -/// Creates new outbound lane object, backed by runtime storage. -fn outbound_lane, I: 'static>( +/// Creates new outbound lane object, backed by runtime storage. Lane must be active. +fn active_outbound_lane, I: 'static>( lane_id: LaneId, ) -> Result>, Error> { LanesManager::::new() @@ -709,6 +709,15 @@ fn outbound_lane, I: 'static>( .map_err(Error::LanesManager) } +/// Creates new outbound lane object, backed by runtime storage. +fn any_state_outbound_lane, I: 'static>( + lane_id: LaneId, +) -> Result>, Error> { + LanesManager::::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, I: 'static>( proof: FromBridgedChainMessagesProof>>, diff --git a/bridges/modules/messages/src/outbound_lane.rs b/bridges/modules/messages/src/outbound_lane.rs index 9dd8b4be6f83f..f71240ab7c703 100644 --- a/bridges/modules/messages/src/outbound_lane.rs +++ b/bridges/modules/messages/src/outbound_lane.rs @@ -223,7 +223,7 @@ fn ensure_unrewarded_relayers_are_correct( 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, @@ -248,7 +248,7 @@ mod tests { relayers: &VecDeque>, ) -> Result, ReceptionConfirmationError> { run_test(|| { - let mut lane = outbound_lane::(test_lane_id()).unwrap(); + let mut lane = active_outbound_lane::(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)); @@ -264,7 +264,7 @@ mod tests { #[test] fn send_message_works() { run_test(|| { - let mut lane = outbound_lane::(test_lane_id()).unwrap(); + let mut lane = active_outbound_lane::(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()); @@ -275,7 +275,7 @@ mod tests { #[test] fn confirm_delivery_works() { run_test(|| { - let mut lane = outbound_lane::(test_lane_id()).unwrap(); + let mut lane = active_outbound_lane::(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); @@ -295,7 +295,7 @@ mod tests { #[test] fn confirm_partial_delivery_works() { run_test(|| { - let mut lane = outbound_lane::(test_lane_id()).unwrap(); + let mut lane = active_outbound_lane::(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); @@ -324,7 +324,7 @@ mod tests { #[test] fn confirm_delivery_rejects_nonce_lesser_than_latest_received() { run_test(|| { - let mut lane = outbound_lane::(test_lane_id()).unwrap(); + let mut lane = active_outbound_lane::(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)); @@ -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::(test_lane_id()).unwrap(); + let mut lane = active_outbound_lane::(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)); diff --git a/bridges/modules/messages/src/tests/pallet_tests.rs b/bridges/modules/messages/src/tests/pallet_tests.rs index 581a0cfb75f6f..a9280711fc908 100644 --- a/bridges/modules/messages/src/tests/pallet_tests.rs +++ b/bridges/modules/messages/src/tests/pallet_tests.rs @@ -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, @@ -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::(lane_id).unwrap(); + let outbound_lane = active_outbound_lane::(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::::validate_message(lane_id, ®ULAR_PAYLOAD) @@ -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::(test_lane_id()) + .unwrap() + .set_state(LaneState::Closed); + receive_messages_delivery_proof(); + + assert_eq!( + OutboundLanes::::get(test_lane_id()) + .unwrap() + .latest_received_nonce, + 1, + ); + }); +} + #[test] fn receive_messages_delivery_proof_rewards_relayers() { run_test(|| { @@ -1055,20 +1073,5 @@ fn receive_messages_delivery_proof_fails_if_outbound_lane_is_unknown() { ), Error::::LanesManager(LanesManagerError::UnknownOutboundLane), ); - - let proof = make_proof(closed_lane_id()); - assert_noop!( - Pallet::::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::::LanesManager(LanesManagerError::ClosedOutboundLane), - ); }); }