Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow delivery confirmations on closed outbound lanes #2257

Merged
merged 2 commits into from
Jul 11, 2023
Merged

Allow delivery confirmations on closed outbound lanes #2257

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
93c9bd3
allow delivery confirmations on closed outbound lanes
svyatonik Jul 11, 2023
b24bd35
fix benchmarks compilation
svyatonik Jul 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions modules/messages/src/benchmarking.rs
View file
Open in desktop
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(vec![]).expect("We craft valid messages");
}

Expand Down
26 changes: 13 additions & 13 deletions modules/messages/src/inbound_lane.rs
View file
Open in desktop
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 modules/messages/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,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 @@ -369,7 +369,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 @@ -599,7 +599,7 @@ fn send_message<T: Config<I>, I: 'static>(
ensure_normal_operating_mode::<T, I>()?;

// finally, save message in outbound storage and emit event
let mut lane = outbound_lane::<T, I>(lane_id)?;
let mut lane = active_outbound_lane::<T, I>(lane_id)?;
let encoded_payload = payload.encode();
let encoded_payload_len = encoded_payload.len();

Expand Down Expand Up @@ -633,24 +633,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 modules/messages/src/outbound_lane.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ fn ensure_unrewarded_relayers_are_correct<RelayerId>(
#[cfg(test)]
mod tests {
use super::*;
use crate::{outbound_lane, tests::mock::*};
use crate::{active_outbound_lane, tests::mock::*};
use frame_support::assert_ok;
use sp_std::ops::RangeInclusive;

Expand All @@ -246,7 +246,7 @@ mod tests {
relayers: &VecDeque<UnrewardedRelayer<TestRelayer>>,
) -> Result<Option<DeliveredMessages>, ReceivalConfirmationError> {
run_test(|| {
let mut lane = outbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
let mut lane = active_outbound_lane::<TestRuntime, _>(test_lane_id()).unwrap();
assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
Expand All @@ -262,7 +262,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)), Ok(1));
assert!(lane.storage.message(&1).is_some());
Expand All @@ -273,7 +273,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)), Ok(1));
assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), Ok(2));
assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), Ok(3));
Expand All @@ -293,7 +293,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)), Ok(1));
assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), Ok(2));
assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), Ok(3));
Expand Down Expand Up @@ -322,7 +322,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();
assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
Expand Down Expand Up @@ -402,7 +402,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();
assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
Expand Down
37 changes: 20 additions & 17 deletions modules/messages/src/tests/pallet_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
//! Pallet-level tests.

use crate::{
lanes_manager::RuntimeInboundLaneStorage, outbound_lane,
active_outbound_lane, lanes_manager::RuntimeInboundLaneStorage,
outbound_lane::ReceivalConfirmationError, send_message, tests::mock::*,
weights_ext::WeightInfoExt, Call, Config, Error, Event, InboundLanes, LanesManagerError,
OutboundLanes, OutboundMessages, Pallet, PalletOperatingMode, PalletOwner,
Expand Down Expand Up @@ -51,7 +51,7 @@ fn get_ready_for_events() {
fn send_regular_message() {
get_ready_for_events();

let message_nonce = outbound_lane::<TestRuntime, ()>(test_lane_id())
let message_nonce = active_outbound_lane::<TestRuntime, ()>(test_lane_id())
.unwrap()
.data()
.latest_generated_nonce +
Expand Down Expand Up @@ -397,6 +397,24 @@ fn receive_messages_delivery_proof_works() {
});
}

#[test]
fn receive_messages_delivery_proof_works_on_closed_outbound_lanes() {
run_test(|| {
send_regular_message();
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 @@ -1050,20 +1068,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),
);
});
}