From f7171f17a20a5593879554dbba22d137fac3c62b Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Tue, 27 Apr 2021 12:30:34 +0300 Subject: [PATCH 1/9] relay dispatch result flags back to the source chain --- Cargo.lock | 2 + bin/millau/runtime/src/lib.rs | 1 + bin/millau/runtime/src/rialto_messages.rs | 9 +- bin/rialto/runtime/src/lib.rs | 1 + bin/rialto/runtime/src/millau_messages.rs | 9 +- modules/dispatch/src/lib.rs | 45 +++- modules/messages/Cargo.toml | 1 + modules/messages/README.md | 9 +- modules/messages/src/inbound_lane.rs | 86 +++++--- modules/messages/src/lib.rs | 120 ++++++---- modules/messages/src/mock.rs | 26 ++- modules/messages/src/outbound_lane.rs | 258 +++++++++++++++++++--- primitives/messages/Cargo.toml | 3 +- primitives/messages/src/lib.rs | 168 +++++++++++--- primitives/messages/src/target_chain.rs | 1 + primitives/runtime/src/messages.rs | 6 + 16 files changed, 617 insertions(+), 128 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5d4475d8c0..c491730f16 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -725,6 +725,7 @@ dependencies = [ name = "bp-messages" version = "0.1.0" dependencies = [ + "bitvec", "bp-runtime", "frame-support", "frame-system", @@ -4762,6 +4763,7 @@ dependencies = [ name = "pallet-bridge-messages" version = "0.1.0" dependencies = [ + "bitvec", "bp-message-dispatch", "bp-messages", "bp-rialto", diff --git a/bin/millau/runtime/src/lib.rs b/bin/millau/runtime/src/lib.rs index cbbe7c92ee..15e8bc42d9 100644 --- a/bin/millau/runtime/src/lib.rs +++ b/bin/millau/runtime/src/lib.rs @@ -702,6 +702,7 @@ mod tests { let max_incoming_inbound_lane_data_proof_size = bp_messages::InboundLaneData::<()>::encoded_size_hint( bp_millau::MAXIMAL_ENCODED_ACCOUNT_ID_SIZE, bp_rialto::MAX_UNREWARDED_RELAYER_ENTRIES_AT_INBOUND_LANE as _, + bp_rialto::MAX_UNCONFIRMED_MESSAGES_AT_INBOUND_LANE as _, ) .unwrap_or(u32::MAX); pallet_bridge_messages::ensure_able_to_receive_confirmation::( diff --git a/bin/millau/runtime/src/rialto_messages.rs b/bin/millau/runtime/src/rialto_messages.rs index b2754dcbf7..644ddc6421 100644 --- a/bin/millau/runtime/src/rialto_messages.rs +++ b/bin/millau/runtime/src/rialto_messages.rs @@ -113,9 +113,12 @@ impl messages::ThisChainWithMessages for Millau { } fn estimate_delivery_confirmation_transaction() -> MessageTransaction { - let inbound_data_size = - InboundLaneData::::encoded_size_hint(bp_millau::MAXIMAL_ENCODED_ACCOUNT_ID_SIZE, 1) - .unwrap_or(u32::MAX); + let inbound_data_size = InboundLaneData::::encoded_size_hint( + bp_millau::MAXIMAL_ENCODED_ACCOUNT_ID_SIZE, + 1, + 1, + ) + .unwrap_or(u32::MAX); MessageTransaction { dispatch_weight: bp_millau::MAX_SINGLE_MESSAGE_DELIVERY_CONFIRMATION_TX_WEIGHT, diff --git a/bin/rialto/runtime/src/lib.rs b/bin/rialto/runtime/src/lib.rs index a2e9578c1d..ebb29ab3f6 100644 --- a/bin/rialto/runtime/src/lib.rs +++ b/bin/rialto/runtime/src/lib.rs @@ -1144,6 +1144,7 @@ mod tests { let max_incoming_inbound_lane_data_proof_size = bp_messages::InboundLaneData::<()>::encoded_size_hint( bp_rialto::MAXIMAL_ENCODED_ACCOUNT_ID_SIZE, bp_millau::MAX_UNREWARDED_RELAYER_ENTRIES_AT_INBOUND_LANE as _, + bp_millau::MAX_UNCONFIRMED_MESSAGES_AT_INBOUND_LANE as _, ) .unwrap_or(u32::MAX); pallet_bridge_messages::ensure_able_to_receive_confirmation::( diff --git a/bin/rialto/runtime/src/millau_messages.rs b/bin/rialto/runtime/src/millau_messages.rs index 4945d2a7df..edcef4447f 100644 --- a/bin/rialto/runtime/src/millau_messages.rs +++ b/bin/rialto/runtime/src/millau_messages.rs @@ -113,9 +113,12 @@ impl messages::ThisChainWithMessages for Rialto { } fn estimate_delivery_confirmation_transaction() -> MessageTransaction { - let inbound_data_size = - InboundLaneData::::encoded_size_hint(bp_rialto::MAXIMAL_ENCODED_ACCOUNT_ID_SIZE, 1) - .unwrap_or(u32::MAX); + let inbound_data_size = InboundLaneData::::encoded_size_hint( + bp_rialto::MAXIMAL_ENCODED_ACCOUNT_ID_SIZE, + 1, + 1, + ) + .unwrap_or(u32::MAX); MessageTransaction { dispatch_weight: bp_rialto::MAX_SINGLE_MESSAGE_DELIVERY_CONFIRMATION_TX_WEIGHT, diff --git a/modules/dispatch/src/lib.rs b/modules/dispatch/src/lib.rs index eeb5d5c17d..4f61a3b935 100644 --- a/modules/dispatch/src/lib.rs +++ b/modules/dispatch/src/lib.rs @@ -154,6 +154,7 @@ impl, I: Instance> MessageDispatch for ); Self::deposit_event(RawEvent::MessageRejected(source_chain, id)); return MessageDispatchResult { + dispatch_result: false, unspent_weight: 0, dispatch_fee_paid_during_dispatch: false, }; @@ -163,6 +164,7 @@ impl, I: Instance> MessageDispatch for // verify spec version // (we want it to be the same, because otherwise we may decode Call improperly) let mut dispatch_result = MessageDispatchResult { + dispatch_result: false, unspent_weight: message.weight, dispatch_fee_paid_during_dispatch: false, }; @@ -303,6 +305,7 @@ impl, I: Instance> MessageDispatch for log::trace!(target: "runtime::bridge-dispatch", "Message being dispatched is: {:.4096?}", &call); let result = call.dispatch(origin); let actual_call_weight = extract_actual_weight(&result, &dispatch_info); + dispatch_result.dispatch_result = result.is_ok(); dispatch_result.unspent_weight = message.weight.saturating_sub(actual_call_weight); log::trace!( @@ -573,6 +576,7 @@ mod tests { System::set_block_number(1); let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| unreachable!()); assert_eq!(result.unspent_weight, weight); + assert!(!result.dispatch_result); assert_eq!( System::events(), @@ -601,6 +605,7 @@ mod tests { System::set_block_number(1); let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| unreachable!()); assert_eq!(result.unspent_weight, 7); + assert!(!result.dispatch_result); assert_eq!( System::events(), @@ -633,6 +638,7 @@ mod tests { System::set_block_number(1); let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| unreachable!()); assert_eq!(result.unspent_weight, weight); + assert!(!result.dispatch_result); assert_eq!( System::events(), @@ -683,6 +689,7 @@ mod tests { System::set_block_number(1); let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| unreachable!()); assert_eq!(result.unspent_weight, weight); + assert!(!result.dispatch_result); assert_eq!( System::events(), @@ -711,6 +718,7 @@ mod tests { System::set_block_number(1); let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| unreachable!()); assert_eq!(result.unspent_weight, weight); + assert!(!result.dispatch_result); assert_eq!( System::events(), @@ -739,12 +747,13 @@ mod tests { System::set_block_number(1); let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| Err(())); assert_eq!(result.unspent_weight, weight); + assert!(!result.dispatch_result); assert_eq!( System::events(), vec![EventRecord { phase: Phase::Initialization, - event: Event::call_dispatch(call_dispatch::Event::::MessageDispatchPaymentFailed( + event: Event::Dispatch(call_dispatch::Event::::MessageDispatchPaymentFailed( SOURCE_CHAIN_ID, id, AccountIdConverter::convert(derive_account_id::( @@ -771,12 +780,13 @@ mod tests { System::set_block_number(1); let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| Ok(())); assert!(result.dispatch_fee_paid_during_dispatch); + assert!(result.dispatch_result); assert_eq!( System::events(), vec![EventRecord { phase: Phase::Initialization, - event: Event::call_dispatch(call_dispatch::Event::::MessageDispatched( + event: Event::Dispatch(call_dispatch::Event::::MessageDispatched( SOURCE_CHAIN_ID, id, Ok(()) @@ -787,6 +797,34 @@ mod tests { }); } + #[test] + fn should_return_dispatch_failed_flag_if_dispatch_happened_but_failed() { + new_test_ext().execute_with(|| { + let id = [0; 4]; + + let call = Call::System(>::set_heap_pages(1)); + let message = prepare_target_message(call); + + System::set_block_number(1); + let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| unreachable!()); + assert!(!result.dispatch_fee_paid_during_dispatch); + assert!(!result.dispatch_result); + + assert_eq!( + System::events(), + vec![EventRecord { + phase: Phase::Initialization, + event: Event::Dispatch(call_dispatch::Event::::MessageDispatched( + SOURCE_CHAIN_ID, + id, + Err(sp_runtime::DispatchError::BadOrigin) + )), + topics: vec![], + }], + ); + }) + } + #[test] fn should_dispatch_bridge_message_from_root_origin() { new_test_ext().execute_with(|| { @@ -796,6 +834,7 @@ mod tests { System::set_block_number(1); let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| unreachable!()); assert!(!result.dispatch_fee_paid_during_dispatch); + assert!(result.dispatch_result); assert_eq!( System::events(), @@ -823,6 +862,7 @@ mod tests { System::set_block_number(1); let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| unreachable!()); assert!(!result.dispatch_fee_paid_during_dispatch); + assert!(result.dispatch_result); assert_eq!( System::events(), @@ -850,6 +890,7 @@ mod tests { System::set_block_number(1); let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| unreachable!()); assert!(!result.dispatch_fee_paid_during_dispatch); + assert!(result.dispatch_result); assert_eq!( System::events(), diff --git a/modules/messages/Cargo.toml b/modules/messages/Cargo.toml index 983a74fe75..a26cf65c02 100644 --- a/modules/messages/Cargo.toml +++ b/modules/messages/Cargo.toml @@ -7,6 +7,7 @@ edition = "2018" license = "GPL-3.0-or-later WITH Classpath-exception-2.0" [dependencies] +bitvec = { version = "0.20", default-features = false, features = ["alloc"] } codec = { package = "parity-scale-codec", version = "2.0.0", default-features = false } log = { version = "0.4.14", default-features = false } num-traits = { version = "0.2", default-features = false } diff --git a/modules/messages/README.md b/modules/messages/README.md index 47bcacdee7..973e251083 100644 --- a/modules/messages/README.md +++ b/modules/messages/README.md @@ -101,7 +101,14 @@ the `MessageAccepted` event is emitted in the `send_message()` transaction. The message lane identifier and nonce that has been assigned to the message. When a message is delivered to the target chain, the `MessagesDelivered` event is emitted from the `receive_messages_delivery_proof()` transaction. The `MessagesDelivered` contains the message lane -identifier and inclusive range of delivered message nonces. +identifier, inclusive range of delivered message nonces and their single-bit dispatch results. + +Please note that the meaning of the 'dispatch result' is determined by the message dispatcher at +the target chain. For example, in case of immediate call dispatcher it will be the `true` if call +has been successfully dispatched and `false` if it has only been delivered. This simple mechanism +built into the messages module allows building basic bridge applications, which only care whether +their messages have been successfully dispatched or not. More sophisticated applications may use +their own dispatch result delivery mechanism to deliver something larger than single bit. ### How to plug-in Messages Module to Send Messages to the Bridged Chain? diff --git a/modules/messages/src/inbound_lane.rs b/modules/messages/src/inbound_lane.rs index 92eb8d665d..83d17dc3c0 100644 --- a/modules/messages/src/inbound_lane.rs +++ b/modules/messages/src/inbound_lane.rs @@ -18,7 +18,7 @@ use bp_messages::{ target_chain::{DispatchMessage, DispatchMessageData, MessageDispatch}, - InboundLaneData, LaneId, MessageKey, MessageNonce, OutboundLaneData, + DeliveredMessages, InboundLaneData, LaneId, MessageKey, MessageNonce, OutboundLaneData, UnrewardedRelayer, }; use bp_runtime::messages::MessageDispatchResult; use frame_support::RuntimeDebug; @@ -89,7 +89,7 @@ impl InboundLane { while data .relayers .front() - .map(|(_, nonce_high, _)| *nonce_high <= new_confirmed_nonce) + .map(|entry| entry.messages.end <= new_confirmed_nonce) .unwrap_or(false) { data.relayers.pop_front(); @@ -97,8 +97,12 @@ impl InboundLane { // Secondly, update the next record with lower nonce equal to new confirmed nonce if needed. // 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((nonce_low, _, _)) if *nonce_low < new_confirmed_nonce => { - *nonce_low = new_confirmed_nonce + 1; + Some(entry) if entry.messages.begin < new_confirmed_nonce => { + entry.messages.dispatch_results = entry + .messages + .dispatch_results + .split_off((new_confirmed_nonce + 1 - entry.messages.begin) as _); + entry.messages.begin = new_confirmed_nonce + 1; } _ => {} } @@ -132,30 +136,37 @@ impl InboundLane { return ReceivalResult::TooManyUnconfirmedMessages; } + // dispatch message before updating anything in the storage. If dispatch would panic, + // (which should not happen in the runtime) then we simply won't consider message as + // delivered (no changes to the inbound lane storage have been made). + let dispatch_result = P::dispatch( + relayer_at_this_chain, + DispatchMessage { + key: MessageKey { + lane_id: self.storage.id(), + nonce, + }, + data: message_data, + }, + ); + + // now let's update inbound lane storage let push_new = match data.relayers.back_mut() { - Some((_, nonce_high, last_relayer)) if last_relayer == relayer_at_bridged_chain => { - *nonce_high = nonce; + Some(entry) if entry.relayer == *relayer_at_bridged_chain => { + entry.messages.note_dispatched_message(dispatch_result.dispatch_result); false } _ => true, }; if push_new { - data.relayers - .push_back((nonce, nonce, (*relayer_at_bridged_chain).clone())); + data.relayers.push_back(UnrewardedRelayer { + relayer: (*relayer_at_bridged_chain).clone(), + messages: DeliveredMessages::new(nonce, dispatch_result.dispatch_result), + }); } - self.storage.set_data(data); - ReceivalResult::Dispatched(P::dispatch( - relayer_at_this_chain, - DispatchMessage { - key: MessageKey { - lane_id: self.storage.id(), - nonce, - }, - data: message_data, - }, - )) + ReceivalResult::Dispatched(dispatch_result) } } @@ -165,8 +176,8 @@ mod tests { use crate::{ inbound_lane, mock::{ - dispatch_result, message_data, run_test, TestMessageDispatch, TestRuntime, REGULAR_PAYLOAD, TEST_LANE_ID, - TEST_RELAYER_A, TEST_RELAYER_B, TEST_RELAYER_C, + dispatch_result, message_data, run_test, unrewarded_relayer, TestMessageDispatch, TestRuntime, + REGULAR_PAYLOAD, TEST_LANE_ID, TEST_RELAYER_A, TEST_RELAYER_B, TEST_RELAYER_C, }, DefaultInstance, RuntimeInboundLaneStorage, }; @@ -238,7 +249,10 @@ mod tests { receive_regular_message(&mut lane, 2); receive_regular_message(&mut lane, 3); assert_eq!(lane.storage.data().last_confirmed_nonce, 0); - assert_eq!(lane.storage.data().relayers, vec![(1, 3, TEST_RELAYER_A)]); + assert_eq!( + lane.storage.data().relayers, + vec![unrewarded_relayer(1, 3, TEST_RELAYER_A)] + ); assert_eq!( lane.receive_state_update(OutboundLaneData { @@ -248,7 +262,10 @@ mod tests { Some(2), ); assert_eq!(lane.storage.data().last_confirmed_nonce, 2); - assert_eq!(lane.storage.data().relayers, vec![(3, 3, TEST_RELAYER_A)]); + assert_eq!( + lane.storage.data().relayers, + vec![unrewarded_relayer(3, 3, TEST_RELAYER_A)] + ); assert_eq!( lane.receive_state_update(OutboundLaneData { @@ -269,10 +286,16 @@ mod tests { let mut seed_storage_data = lane.storage.data(); // Prepare data seed_storage_data.last_confirmed_nonce = 0; - seed_storage_data.relayers.push_back((1, 1, TEST_RELAYER_A)); + seed_storage_data + .relayers + .push_back(unrewarded_relayer(1, 1, TEST_RELAYER_A)); // Simulate messages batch (2, 3, 4) from relayer #2 - seed_storage_data.relayers.push_back((2, 4, TEST_RELAYER_B)); - seed_storage_data.relayers.push_back((5, 5, TEST_RELAYER_C)); + seed_storage_data + .relayers + .push_back(unrewarded_relayer(2, 4, TEST_RELAYER_B)); + seed_storage_data + .relayers + .push_back(unrewarded_relayer(5, 5, TEST_RELAYER_C)); lane.storage.set_data(seed_storage_data); // Check assert_eq!( @@ -285,7 +308,10 @@ mod tests { assert_eq!(lane.storage.data().last_confirmed_nonce, 3); assert_eq!( lane.storage.data().relayers, - vec![(4, 4, TEST_RELAYER_B), (5, 5, TEST_RELAYER_C)] + vec![ + unrewarded_relayer(4, 4, TEST_RELAYER_B), + unrewarded_relayer(5, 5, TEST_RELAYER_C) + ] ); }); } @@ -418,7 +444,11 @@ mod tests { ); assert_eq!( lane.storage.data().relayers, - vec![(1, 1, TEST_RELAYER_A), (2, 2, TEST_RELAYER_B), (3, 3, TEST_RELAYER_A)] + vec![ + unrewarded_relayer(1, 1, TEST_RELAYER_A), + unrewarded_relayer(2, 2, TEST_RELAYER_B), + unrewarded_relayer(3, 3, TEST_RELAYER_A) + ] ); }); } diff --git a/modules/messages/src/lib.rs b/modules/messages/src/lib.rs index 5b0c609d85..9892db32e9 100644 --- a/modules/messages/src/lib.rs +++ b/modules/messages/src/lib.rs @@ -43,21 +43,21 @@ pub use crate::weights_ext::{ }; use crate::inbound_lane::{InboundLane, InboundLaneStorage, ReceivalResult}; -use crate::outbound_lane::{OutboundLane, OutboundLaneStorage}; +use crate::outbound_lane::{OutboundLane, OutboundLaneStorage, ReceivalConfirmationResult}; use crate::weights::WeightInfo; use bp_messages::{ source_chain::{LaneMessageVerifier, MessageDeliveryAndDispatchPayment, RelayersRewards, TargetHeaderChain}, target_chain::{DispatchMessage, MessageDispatch, ProvedLaneMessages, ProvedMessages, SourceHeaderChain}, - total_unrewarded_messages, InboundLaneData, LaneId, MessageData, MessageKey, MessageNonce, OperatingMode, - OutboundLaneData, Parameter as MessagesParameter, UnrewardedRelayersState, + total_unrewarded_messages, DeliveredMessages, InboundLaneData, LaneId, MessageData, MessageKey, MessageNonce, + OperatingMode, OutboundLaneData, Parameter as MessagesParameter, UnrewardedRelayersState, }; use bp_runtime::Size; use codec::{Decode, Encode}; use frame_support::{ decl_error, decl_event, decl_module, decl_storage, dispatch::DispatchResultWithPostInfo, - ensure, + ensure, fail, traits::Get, weights::{DispatchClass, Pays, PostDispatchInfo, Weight}, Parameter, StorageMap, @@ -186,6 +186,8 @@ decl_error! { InvalidMessagesDispatchWeight, /// Invalid messages delivery proof has been submitted. InvalidMessagesDeliveryProof, + /// The bridged chain has invalid `UnrewardedRelayers` in its storage (fatal for the lane). + InvalidUnrewardedRelayers, /// The relayer has declared invalid unrewarded relayers state in the `receive_messages_delivery_proof` call. InvalidUnrewardedRelayersState, /// The message someone is trying to work with (i.e. increase fee) is already-delivered. @@ -236,8 +238,8 @@ decl_event!( ParameterUpdated(Parameter), /// Message has been accepted and is waiting to be delivered. MessageAccepted(LaneId, MessageNonce), - /// Messages in the inclusive range have been delivered and processed by the bridged chain. - MessagesDelivered(LaneId, MessageNonce, MessageNonce), + /// Messages in the inclusive range have been delivered to the bridged chain. + MessagesDelivered(LaneId, DeliveredMessages), /// Phantom member, never used. Dummy(PhantomData<(AccountId, I)>), } @@ -620,19 +622,32 @@ decl_module! { let mut lane = outbound_lane::(lane_id); let mut relayers_rewards: RelayersRewards<_, T::OutboundMessageFee> = RelayersRewards::new(); let last_delivered_nonce = lane_data.last_delivered_nonce(); - let received_range = lane.confirm_delivery(last_delivered_nonce); - if let Some(received_range) = received_range { - Self::deposit_event(RawEvent::MessagesDelivered(lane_id, received_range.0, received_range.1)); + let confirmed_messages = match lane.confirm_delivery(last_delivered_nonce, &lane_data.relayers) { + ReceivalConfirmationResult::ConfirmedMessages(confirmed_messages) => Some(confirmed_messages), + ReceivalConfirmationResult::NoNewConfirmations => None, + error => { + log::trace!( + target: "runtime::bridge-messages", + "Messages delivery proof contains invalid unrewarded relayers vec: {:?}", + error, + ); + + fail!(Error::::InvalidUnrewardedRelayers); + }, + }; + if let Some(confirmed_messages) = confirmed_messages { + let received_range = confirmed_messages.begin..=confirmed_messages.end; + Self::deposit_event(RawEvent::MessagesDelivered(lane_id, confirmed_messages)); // remember to reward relayers that have delivered messages // this loop is bounded by `T::MaxUnrewardedRelayerEntriesAtInboundLane` on the bridged chain - for (nonce_low, nonce_high, relayer) in lane_data.relayers { - let nonce_begin = sp_std::cmp::max(nonce_low, received_range.0); - let nonce_end = sp_std::cmp::min(nonce_high, received_range.1); + for entry in lane_data.relayers { + let nonce_begin = sp_std::cmp::max(entry.messages.begin, *received_range.start()); + let nonce_end = sp_std::cmp::min(entry.messages.end, *received_range.end()); // loop won't proceed if current entry is ahead of received range (begin > end). // this loop is bound by `T::MaxUnconfirmedMessagesAtInboundLane` on the bridged chain - let mut relayer_reward = relayers_rewards.entry(relayer).or_default(); + let mut relayer_reward = relayers_rewards.entry(entry.relayer).or_default(); for nonce in nonce_begin..nonce_end + 1 { let message_data = OutboundMessages::::get(MessageKey { lane_id, @@ -697,7 +712,10 @@ impl, I: Instance> Pallet { let relayers = InboundLanes::::get(&lane).relayers; bp_messages::UnrewardedRelayersState { unrewarded_relayer_entries: relayers.len() as _, - messages_in_oldest_entry: relayers.front().map(|(begin, end, _)| 1 + end - begin).unwrap_or(0), + 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), } } @@ -920,11 +938,12 @@ fn verify_and_decode_messages_proof, Fee, Dispatch mod tests { use super::*; use crate::mock::{ - message, message_payload, run_test, Event as TestEvent, Origin, TestMessageDeliveryAndDispatchPayment, - TestMessagesDeliveryProof, TestMessagesParameter, TestMessagesProof, TestRuntime, TokenConversionRate, - PAYLOAD_REJECTED_BY_TARGET_CHAIN, REGULAR_PAYLOAD, TEST_LANE_ID, TEST_RELAYER_A, TEST_RELAYER_B, + message, message_payload, run_test, unrewarded_relayer, Event as TestEvent, Origin, + TestMessageDeliveryAndDispatchPayment, TestMessagesDeliveryProof, TestMessagesParameter, TestMessagesProof, + TestRuntime, TokenConversionRate, PAYLOAD_REJECTED_BY_TARGET_CHAIN, REGULAR_PAYLOAD, TEST_LANE_ID, + TEST_RELAYER_A, TEST_RELAYER_B, }; - use bp_messages::UnrewardedRelayersState; + use bp_messages::{UnrewardedRelayer, UnrewardedRelayersState}; use frame_support::{assert_noop, assert_ok}; use frame_system::{EventRecord, Pallet as System, Phase}; use hex_literal::hex; @@ -972,17 +991,29 @@ mod tests { TEST_LANE_ID, InboundLaneData { last_confirmed_nonce: 1, - ..Default::default() + relayers: vec![UnrewardedRelayer { + relayer: 0, + messages: DeliveredMessages::new(1, true), + }] + .into_iter() + .collect(), }, ))), - Default::default(), + UnrewardedRelayersState { + unrewarded_relayer_entries: 1, + total_messages: 1, + ..Default::default() + }, )); assert_eq!( System::::events(), vec![EventRecord { phase: Phase::Initialization, - event: TestEvent::Messages(RawEvent::MessagesDelivered(TEST_LANE_ID, 1, 1)), + event: TestEvent::Messages(RawEvent::MessagesDelivered( + TEST_LANE_ID, + DeliveredMessages::new(1, true), + )), topics: vec![], }], ); @@ -1333,9 +1364,12 @@ mod tests { TEST_LANE_ID, InboundLaneData { last_confirmed_nonce: 8, - relayers: vec![(9, 9, TEST_RELAYER_A), (10, 10, TEST_RELAYER_B)] - .into_iter() - .collect(), + relayers: vec![ + unrewarded_relayer(9, 9, TEST_RELAYER_A), + unrewarded_relayer(10, 10, TEST_RELAYER_B), + ] + .into_iter() + .collect(), }, ); assert_eq!( @@ -1366,9 +1400,12 @@ mod tests { InboundLanes::::get(TEST_LANE_ID), InboundLaneData { last_confirmed_nonce: 9, - relayers: vec![(10, 10, TEST_RELAYER_B), (11, 11, TEST_RELAYER_A)] - .into_iter() - .collect(), + relayers: vec![ + unrewarded_relayer(10, 10, TEST_RELAYER_B), + unrewarded_relayer(11, 11, TEST_RELAYER_A) + ] + .into_iter() + .collect(), }, ); assert_eq!( @@ -1465,7 +1502,7 @@ mod tests { TestMessagesDeliveryProof(Ok(( TEST_LANE_ID, InboundLaneData { - relayers: vec![(1, 1, TEST_RELAYER_A)].into_iter().collect(), + relayers: vec![unrewarded_relayer(1, 1, TEST_RELAYER_A)].into_iter().collect(), ..Default::default() } ))), @@ -1490,9 +1527,12 @@ mod tests { TestMessagesDeliveryProof(Ok(( TEST_LANE_ID, InboundLaneData { - relayers: vec![(1, 1, TEST_RELAYER_A), (2, 2, TEST_RELAYER_B)] - .into_iter() - .collect(), + relayers: vec![ + unrewarded_relayer(1, 1, TEST_RELAYER_A), + unrewarded_relayer(2, 2, TEST_RELAYER_B) + ] + .into_iter() + .collect(), ..Default::default() } ))), @@ -1537,9 +1577,12 @@ mod tests { TestMessagesDeliveryProof(Ok(( TEST_LANE_ID, InboundLaneData { - relayers: vec![(1, 1, TEST_RELAYER_A), (2, 2, TEST_RELAYER_B)] - .into_iter() - .collect(), + relayers: vec![ + unrewarded_relayer(1, 1, TEST_RELAYER_A), + unrewarded_relayer(2, 2, TEST_RELAYER_B) + ] + .into_iter() + .collect(), ..Default::default() } ))), @@ -1559,9 +1602,12 @@ mod tests { TestMessagesDeliveryProof(Ok(( TEST_LANE_ID, InboundLaneData { - relayers: vec![(1, 1, TEST_RELAYER_A), (2, 2, TEST_RELAYER_B)] - .into_iter() - .collect(), + relayers: vec![ + unrewarded_relayer(1, 1, TEST_RELAYER_A), + unrewarded_relayer(2, 2, TEST_RELAYER_B) + ] + .into_iter() + .collect(), ..Default::default() } ))), diff --git a/modules/messages/src/mock.rs b/modules/messages/src/mock.rs index 823ec13dd7..77813711e8 100644 --- a/modules/messages/src/mock.rs +++ b/modules/messages/src/mock.rs @@ -19,13 +19,14 @@ use crate::Config; +use bitvec::prelude::*; use bp_messages::{ source_chain::{ LaneMessageVerifier, MessageDeliveryAndDispatchPayment, RelayersRewards, Sender, TargetHeaderChain, }, target_chain::{DispatchMessage, MessageDispatch, ProvedLaneMessages, ProvedMessages, SourceHeaderChain}, - InboundLaneData, LaneId, Message, MessageData, MessageKey, MessageNonce, OutboundLaneData, - Parameter as MessagesParameter, + DeliveredMessages, InboundLaneData, LaneId, Message, MessageData, MessageKey, MessageNonce, OutboundLaneData, + Parameter as MessagesParameter, UnrewardedRelayer, }; use bp_runtime::{messages::MessageDispatchResult, Size}; use codec::{Decode, Encode}; @@ -421,11 +422,32 @@ pub fn message_data(payload: TestPayload) -> MessageData { /// Returns message dispatch result with given unspent weight. pub const fn dispatch_result(unspent_weight: Weight) -> MessageDispatchResult { MessageDispatchResult { + dispatch_result: true, unspent_weight, dispatch_fee_paid_during_dispatch: true, } } +/// Constructs unrewarded relayer entry from nonces range and relayer id. +pub fn unrewarded_relayer( + begin: MessageNonce, + end: MessageNonce, + relayer: TestRelayer, +) -> UnrewardedRelayer { + UnrewardedRelayer { + relayer, + messages: DeliveredMessages { + begin, + end, + dispatch_results: if end >= begin { + bitvec![Msb0, u8; 1; (end - begin + 1) as _] + } else { + Default::default() + }, + }, + } +} + /// Run pallet test. pub fn run_test(test: impl FnOnce() -> T) -> T { let mut t = frame_system::GenesisConfig::default() diff --git a/modules/messages/src/outbound_lane.rs b/modules/messages/src/outbound_lane.rs index 47616c33ea..461c053302 100644 --- a/modules/messages/src/outbound_lane.rs +++ b/modules/messages/src/outbound_lane.rs @@ -16,7 +16,12 @@ //! Everything about outgoing messages sending. -use bp_messages::{LaneId, MessageData, MessageNonce, OutboundLaneData}; +use bitvec::prelude::*; +use bp_messages::{ + DeliveredMessages, DispatchResultsBitVec, LaneId, MessageData, MessageNonce, OutboundLaneData, UnrewardedRelayer, +}; +use frame_support::RuntimeDebug; +use sp_std::collections::vec_deque::VecDeque; /// Outbound lane storage. pub trait OutboundLaneStorage { @@ -38,6 +43,28 @@ pub trait OutboundLaneStorage { fn remove_message(&mut self, nonce: &MessageNonce); } +/// Result of messages receival confirmation. +#[derive(RuntimeDebug, PartialEq, Eq)] +pub enum ReceivalConfirmationResult { + /// New messages have been confirmed by the confirmation transaction. + ConfirmedMessages(DeliveredMessages), + /// Confirmation transaction brings no new confirmation. This may be a result of relayer + /// error or several relayers runnng. + NoNewConfirmations, + /// Bridged chain is trying to confirm more messages than we have generated. May be a result + /// of invalid bridged chain storage. + FailedToConfirmFutureMessages, + /// The unrewarded relayers vec contains an empty entry. May be a result of invalid bridged + /// chain storage. + EmptyUnrewardedRelayerEntry, + /// The unrewarded relayers vec contains non-consecutive entries. May be a result of invalid bridged + /// chain storage. + NonConsecutiveUnrewardedRelayerEntries, + /// The unrewarded relayers vec contains entry with mismatched number of dispatch results. May be + /// a result of invalid bridged chain storage. + InvalidNumberOfDispatchResults, +} + /// Outbound messages lane. pub struct OutboundLane { storage: S, @@ -69,20 +96,34 @@ impl OutboundLane { } /// Confirm messages delivery. - /// - /// Returns `None` if confirmation is wrong/duplicate. - /// Returns `Some` with inclusive ranges of message nonces that have been received. - pub fn confirm_delivery(&mut self, latest_received_nonce: MessageNonce) -> Option<(MessageNonce, MessageNonce)> { + pub fn confirm_delivery( + &mut self, + latest_received_nonce: MessageNonce, + relayers: &VecDeque>, + ) -> ReceivalConfirmationResult { let mut data = self.storage.data(); - if latest_received_nonce <= data.latest_received_nonce || latest_received_nonce > data.latest_generated_nonce { - return None; + if latest_received_nonce <= data.latest_received_nonce { + return ReceivalConfirmationResult::NoNewConfirmations; + } + if latest_received_nonce > data.latest_generated_nonce { + return ReceivalConfirmationResult::FailedToConfirmFutureMessages; } + let dispatch_results = + match extract_dispatch_results(data.latest_received_nonce, latest_received_nonce, relayers) { + Ok(dispatch_results) => dispatch_results, + Err(extract_error) => return extract_error, + }; + let prev_latest_received_nonce = data.latest_received_nonce; data.latest_received_nonce = latest_received_nonce; self.storage.set_data(data); - Some((prev_latest_received_nonce + 1, latest_received_nonce)) + ReceivalConfirmationResult::ConfirmedMessages(DeliveredMessages { + begin: prev_latest_received_nonce + 1, + end: latest_received_nonce, + dispatch_results, + }) } /// Prune at most `max_messages_to_prune` already received messages. @@ -108,13 +149,108 @@ impl OutboundLane { } } +/// Extract new dispatch results from the unrewarded relayers vec. +/// +/// Returns `Err(_)` if unrewarded relayers vec contains invalid data, meaning that the bridged +/// chain has invalid runtime storage. +fn extract_dispatch_results( + prev_latest_received_nonce: MessageNonce, + latest_received_nonce: MessageNonce, + relayers: &VecDeque>, +) -> Result { + // the only caller of this functions checks that the prev_latest_received_nonce..=latest_received_nonce + // is valid, so we're ready to accept messages in this range + // => with_capacity call must succeed here or we'll be unable to receive confirmations at all + let mut received_dispatch_result = + BitVec::with_capacity((latest_received_nonce - prev_latest_received_nonce + 1) as _); + let mut last_entry_end: Option = None; + for entry in relayers { + // unrewarded relayer entry must have at least 1 unconfirmed message + // (guaranteed by the `InboundLane::receive_message()`) + if entry.messages.end < entry.messages.begin { + return Err(ReceivalConfirmationResult::EmptyUnrewardedRelayerEntry); + } + // every entry must confirm range of messages that follows previous entry range + // (guaranteed by the `InboundLane::receive_message()`) + if let Some(last_entry_end) = last_entry_end { + let expected_entry_begin = last_entry_end.checked_add(1); + if expected_entry_begin != Some(entry.messages.begin) { + return Err(ReceivalConfirmationResult::NonConsecutiveUnrewardedRelayerEntries); + } + } + last_entry_end = Some(entry.messages.end); + // entry can't confirm messages larger than `inbound_lane_data.latest_received_nonce()` + // (guaranteed by the `InboundLane::receive_message()`) + if entry.messages.end > latest_received_nonce { + // technically this will be detected in the next loop iteration as `InvalidNumberOfDispatchResults` + // but to guarantee safety of loop operations below this is detected now + return Err(ReceivalConfirmationResult::FailedToConfirmFutureMessages); + } + // entry must have single dispatch result for every message + // (guaranteed by the `InboundLane::receive_message()`) + if entry.messages.dispatch_results.len() as MessageNonce != entry.messages.end - entry.messages.begin + 1 { + return Err(ReceivalConfirmationResult::InvalidNumberOfDispatchResults); + } + + // now we know that the entry is valid + // => let's check if it brings new confirmations + let new_messages_begin = sp_std::cmp::max(entry.messages.begin, prev_latest_received_nonce + 1); + let new_messages_end = sp_std::cmp::min(entry.messages.end, latest_received_nonce); + let new_messages_range = new_messages_begin..=new_messages_end; + if new_messages_range.is_empty() { + continue; + } + + // now we know that entry brings new confirmations + // => let's extract dispatch results + received_dispatch_result.extend_from_bitslice( + &entry.messages.dispatch_results[(new_messages_begin - entry.messages.begin) as usize..], + ); + } + + Ok(received_dispatch_result) +} + #[cfg(test)] mod tests { use super::*; use crate::{ - mock::{message_data, run_test, TestRuntime, REGULAR_PAYLOAD, TEST_LANE_ID}, + mock::{message_data, run_test, unrewarded_relayer, TestRelayer, TestRuntime, REGULAR_PAYLOAD, TEST_LANE_ID}, outbound_lane, }; + use sp_std::ops::RangeInclusive; + + fn unrewarded_relayers(nonces: RangeInclusive) -> VecDeque> { + vec![unrewarded_relayer(*nonces.start(), *nonces.end(), 0)] + .into_iter() + .collect() + } + + fn delivered_messages(nonces: RangeInclusive) -> DeliveredMessages { + DeliveredMessages { + begin: *nonces.start(), + end: *nonces.end(), + dispatch_results: bitvec![Msb0, u8; 1; (nonces.end() - nonces.start() + 1) as _], + } + } + + fn assert_3_messages_confirmation_fails( + latest_received_nonce: MessageNonce, + relayers: &VecDeque>, + ) -> ReceivalConfirmationResult { + run_test(|| { + let mut lane = outbound_lane::(TEST_LANE_ID); + lane.send_message(message_data(REGULAR_PAYLOAD)); + lane.send_message(message_data(REGULAR_PAYLOAD)); + lane.send_message(message_data(REGULAR_PAYLOAD)); + assert_eq!(lane.storage.data().latest_generated_nonce, 3); + assert_eq!(lane.storage.data().latest_received_nonce, 0); + let result = lane.confirm_delivery(latest_received_nonce, relayers); + assert_eq!(lane.storage.data().latest_generated_nonce, 3); + assert_eq!(lane.storage.data().latest_received_nonce, 0); + result + }) + } #[test] fn send_message_works() { @@ -136,7 +272,10 @@ mod tests { assert_eq!(lane.send_message(message_data(REGULAR_PAYLOAD)), 3); assert_eq!(lane.storage.data().latest_generated_nonce, 3); assert_eq!(lane.storage.data().latest_received_nonce, 0); - assert_eq!(lane.confirm_delivery(3), Some((1, 3))); + assert_eq!( + lane.confirm_delivery(3, &unrewarded_relayers(1..=3)), + ReceivalConfirmationResult::ConfirmedMessages(delivered_messages(1..=3)), + ); assert_eq!(lane.storage.data().latest_generated_nonce, 3); assert_eq!(lane.storage.data().latest_received_nonce, 3); }); @@ -151,12 +290,21 @@ mod tests { lane.send_message(message_data(REGULAR_PAYLOAD)); assert_eq!(lane.storage.data().latest_generated_nonce, 3); assert_eq!(lane.storage.data().latest_received_nonce, 0); - assert_eq!(lane.confirm_delivery(3), Some((1, 3))); - assert_eq!(lane.confirm_delivery(3), None); + assert_eq!( + lane.confirm_delivery(3, &unrewarded_relayers(1..=3)), + ReceivalConfirmationResult::ConfirmedMessages(delivered_messages(1..=3)), + ); + assert_eq!( + lane.confirm_delivery(3, &unrewarded_relayers(1..=3)), + ReceivalConfirmationResult::NoNewConfirmations, + ); assert_eq!(lane.storage.data().latest_generated_nonce, 3); assert_eq!(lane.storage.data().latest_received_nonce, 3); - assert_eq!(lane.confirm_delivery(2), None); + assert_eq!( + lane.confirm_delivery(2, &unrewarded_relayers(1..=1)), + ReceivalConfirmationResult::NoNewConfirmations, + ); assert_eq!(lane.storage.data().latest_generated_nonce, 3); assert_eq!(lane.storage.data().latest_received_nonce, 3); }); @@ -164,17 +312,69 @@ mod tests { #[test] fn confirm_delivery_rejects_nonce_larger_than_last_generated() { - run_test(|| { - let mut lane = outbound_lane::(TEST_LANE_ID); - lane.send_message(message_data(REGULAR_PAYLOAD)); - lane.send_message(message_data(REGULAR_PAYLOAD)); - lane.send_message(message_data(REGULAR_PAYLOAD)); - assert_eq!(lane.storage.data().latest_generated_nonce, 3); - assert_eq!(lane.storage.data().latest_received_nonce, 0); - assert_eq!(lane.confirm_delivery(10), None); - assert_eq!(lane.storage.data().latest_generated_nonce, 3); - assert_eq!(lane.storage.data().latest_received_nonce, 0); - }); + assert_eq!( + assert_3_messages_confirmation_fails(10, &unrewarded_relayers(1..=10),), + ReceivalConfirmationResult::FailedToConfirmFutureMessages, + ); + } + + #[test] + fn confirm_delivery_fails_if_entry_confirms_future_messages() { + assert_eq!( + assert_3_messages_confirmation_fails( + 3, + &unrewarded_relayers(1..=1) + .into_iter() + .chain(unrewarded_relayers(2..=30).into_iter()) + .chain(unrewarded_relayers(3..=3).into_iter()) + .collect(), + ), + ReceivalConfirmationResult::FailedToConfirmFutureMessages, + ); + } + + #[test] + fn confirm_delivery_fails_if_entry_is_empty() { + assert_eq!( + assert_3_messages_confirmation_fails( + 3, + &unrewarded_relayers(1..=1) + .into_iter() + .chain(unrewarded_relayers(2..=1).into_iter()) + .chain(unrewarded_relayers(2..=3).into_iter()) + .collect(), + ), + ReceivalConfirmationResult::EmptyUnrewardedRelayerEntry, + ); + } + + #[test] + fn confirm_delivery_fails_if_entries_are_non_consecutive() { + assert_eq!( + assert_3_messages_confirmation_fails( + 3, + &unrewarded_relayers(1..=1) + .into_iter() + .chain(unrewarded_relayers(3..=3).into_iter()) + .chain(unrewarded_relayers(2..=2).into_iter()) + .collect(), + ), + ReceivalConfirmationResult::NonConsecutiveUnrewardedRelayerEntries, + ); + } + + #[test] + fn confirm_delivery_fails_if_number_of_dispatch_results_in_entry_is_invalid() { + let mut relayers: VecDeque<_> = unrewarded_relayers(1..=1) + .into_iter() + .chain(unrewarded_relayers(2..=2).into_iter()) + .chain(unrewarded_relayers(3..=3).into_iter()) + .collect(); + relayers[0].messages.dispatch_results.clear(); + assert_eq!( + assert_3_messages_confirmation_fails(3, &relayers), + ReceivalConfirmationResult::InvalidNumberOfDispatchResults, + ); } #[test] @@ -191,11 +391,17 @@ mod tests { assert_eq!(lane.prune_messages(100), 0); assert_eq!(lane.storage.data().oldest_unpruned_nonce, 1); // after confirmation, some messages are received - assert_eq!(lane.confirm_delivery(2), Some((1, 2))); + assert_eq!( + lane.confirm_delivery(2, &unrewarded_relayers(1..=2)), + ReceivalConfirmationResult::ConfirmedMessages(delivered_messages(1..=2)), + ); assert_eq!(lane.prune_messages(100), 2); assert_eq!(lane.storage.data().oldest_unpruned_nonce, 3); // after last message is confirmed, everything is pruned - assert_eq!(lane.confirm_delivery(3), Some((3, 3))); + assert_eq!( + lane.confirm_delivery(3, &unrewarded_relayers(3..=3)), + ReceivalConfirmationResult::ConfirmedMessages(delivered_messages(3..=3)), + ); assert_eq!(lane.prune_messages(100), 1); assert_eq!(lane.storage.data().oldest_unpruned_nonce, 4); }); diff --git a/primitives/messages/Cargo.toml b/primitives/messages/Cargo.toml index 9b79a3bdb5..263fea1a70 100644 --- a/primitives/messages/Cargo.toml +++ b/primitives/messages/Cargo.toml @@ -7,7 +7,8 @@ edition = "2018" license = "GPL-3.0-or-later WITH Classpath-exception-2.0" [dependencies] -codec = { package = "parity-scale-codec", version = "2.0.0", default-features = false, features = ["derive"] } +bitvec = { version = "0.20", default-features = false, features = ["alloc"] } +codec = { package = "parity-scale-codec", version = "2.0.0", default-features = false, features = ["derive", "bit-vec"] } serde = { version = "1.0.101", optional = true, features = ["derive"] } # Bridge dependencies diff --git a/primitives/messages/src/lib.rs b/primitives/messages/src/lib.rs index 211f4e7175..d51200273a 100644 --- a/primitives/messages/src/lib.rs +++ b/primitives/messages/src/lib.rs @@ -22,6 +22,7 @@ // Generated by `DecodeLimit::decode_with_depth_limit` #![allow(clippy::unnecessary_mut_passed)] +use bitvec::prelude::*; use codec::{Decode, Encode}; use frame_support::RuntimeDebug; use sp_std::{collections::vec_deque::VecDeque, prelude::*}; @@ -120,7 +121,7 @@ pub struct InboundLaneData { /// When a relayer sends a single message, both of MessageNonces are the same. /// When relayer sends messages in a batch, the first arg is the lowest nonce, second arg the highest nonce. /// Multiple dispatches from the same relayer are allowed. - pub relayers: VecDeque<(MessageNonce, MessageNonce, RelayerId)>, + pub relayers: VecDeque>, /// Nonce of the last message that /// a) has been delivered to the target (this) chain and @@ -147,18 +148,22 @@ impl InboundLaneData { /// size of each entry. /// /// Returns `None` if size overflows `u32` limits. - pub fn encoded_size_hint(relayer_id_encoded_size: u32, relayers_entries: u32) -> Option { + pub fn encoded_size_hint(relayer_id_encoded_size: u32, relayers_entries: u32, messages_count: u32) -> Option { let message_nonce_size = 8; let relayers_entry_size = relayer_id_encoded_size.checked_add(2 * message_nonce_size)?; let relayers_size = relayers_entries.checked_mul(relayers_entry_size)?; - relayers_size.checked_add(message_nonce_size) + let dispatch_results_per_byte = 8; + let dispatch_result_size = sp_std::cmp::max(relayers_entries, messages_count / dispatch_results_per_byte); + relayers_size + .checked_add(message_nonce_size) + .and_then(|result| result.checked_add(dispatch_result_size)) } /// Nonce of the last message that has been delivered to this (target) chain. pub fn last_delivered_nonce(&self) -> MessageNonce { self.relayers .back() - .map(|(_, last_nonce, _)| *last_nonce) + .map(|entry| entry.messages.end) .unwrap_or(self.last_confirmed_nonce) } } @@ -176,6 +181,71 @@ pub struct MessageDetails { pub delivery_and_dispatch_fee: OutboundMessageFee, } +/// Bit vector of message dispatch results. +pub type DispatchResultsBitVec = BitVec; + +/// Unrewarded relayer entry stored in the inbound lane data. +/// +/// This struct represents a continuous range of messages that have been delivered by the same relayer +/// and whose confirmations are still pending. +#[derive(Encode, Decode, Clone, RuntimeDebug, PartialEq, Eq)] +pub struct UnrewardedRelayer { + /// Identifier of the relayer. + pub relayer: RelayerId, + /// Messages range, delivered by this relayer. + pub messages: DeliveredMessages, +} + +/// Delivered messages with their dispatch result. +#[derive(Clone, Default, Encode, Decode, RuntimeDebug, PartialEq, Eq)] +pub struct DeliveredMessages { + /// Nonce of the first message that has been delivered (inclusive). + pub begin: MessageNonce, + /// Nonce of the last message that has been delivered (inclusive). + pub end: MessageNonce, + /// Dispatch result (`false`/`true`), returned by the message dispatcher for every + /// message in the `[begin; end]` range. See `dispatch_result` field of the + /// `bp_runtime::messages::MessageDispatchResult` structure for more information. + pub dispatch_results: DispatchResultsBitVec, +} + +impl DeliveredMessages { + /// Create new `DeliveredMessages` struct that confirms delivery of single nonce with given dispatch result. + pub fn new(nonce: MessageNonce, dispatch_result: bool) -> Self { + DeliveredMessages { + begin: nonce, + end: nonce, + dispatch_results: bitvec![Msb0, u8; if dispatch_result { 1 } else { 0 }], + } + } + + /// Note new dispatched message. + pub fn note_dispatched_message(&mut self, dispatch_result: bool) { + self.end += 1; + self.dispatch_results.push(dispatch_result); + } + + /// Returns true if delivered messages contain message with given nonce. + pub fn contains_message(&self, nonce: MessageNonce) -> bool { + (self.begin..=self.end).contains(&nonce) + } + + /// Get dispatch result flag by message nonce. + /// + /// Dispatch result flag must be interpreted using the knowledge of dispatch mechanism + /// at the target chain. See `dispatch_result` field of the + /// `bp_runtime::messages::MessageDispatchResult` structure for more information. + /// + /// Panics if message nonce is not in the `begin..=end` range. Typically you'll first + /// check if message is within the range by calling `contains_message`. + pub fn message_dispatch_result(&self, nonce: MessageNonce) -> bool { + const INVALID_NONCE: &'static str = "Invalid nonce used to index dispatch_results"; + + let index = nonce.checked_sub(self.begin).expect(INVALID_NONCE) as usize; + *self.dispatch_results.get(index).expect(INVALID_NONCE) + } +} + /// Gist of `InboundLaneData::relayers` field used by runtime APIs. #[derive(Clone, Default, Encode, Decode, RuntimeDebug, PartialEq, Eq)] pub struct UnrewardedRelayersState { @@ -214,12 +284,10 @@ impl Default for OutboundLaneData { /// Returns total number of messages in the `InboundLaneData::relayers` vector. /// /// Returns `None` if there are more messages that `MessageNonce` may fit (i.e. `MessageNonce + 1`). -pub fn total_unrewarded_messages( - relayers: &VecDeque<(MessageNonce, MessageNonce, RelayerId)>, -) -> Option { +pub fn total_unrewarded_messages(relayers: &VecDeque>) -> Option { match (relayers.front(), relayers.back()) { - (Some((begin, _, _)), Some((_, end, _))) => { - if let Some(difference) = end.checked_sub(*begin) { + (Some(front), Some(back)) => { + if let Some(difference) = back.messages.end.checked_sub(front.messages.begin) { difference.checked_add(1) } else { Some(0) @@ -237,9 +305,18 @@ mod tests { fn total_unrewarded_messages_does_not_overflow() { assert_eq!( total_unrewarded_messages( - &vec![(0, 0, 1), (MessageNonce::MAX, MessageNonce::MAX, 2)] - .into_iter() - .collect() + &vec![ + UnrewardedRelayer { + relayer: 1, + messages: DeliveredMessages::new(0, true) + }, + UnrewardedRelayer { + relayer: 2, + messages: DeliveredMessages::new(MessageNonce::MAX, true) + }, + ] + .into_iter() + .collect() ), None, ); @@ -247,19 +324,60 @@ mod tests { #[test] fn inbound_lane_data_returns_correct_hint() { - let expected_size = InboundLaneData::::encoded_size_hint(1, 13); - let actual_size = InboundLaneData { - relayers: (1u8..=13u8).map(|i| (i as _, i as _, i)).collect(), - last_confirmed_nonce: 13, + let test_cases = vec![ + // single relayer, multiple messages + (1, 128u8), + // multiple relayers, single message per relayer + (128u8, 128u8), + // several messages per relayer + (13u8, 128u8), + ]; + for (relayer_entries, messages_count) in test_cases { + let expected_size = InboundLaneData::::encoded_size_hint(1, relayer_entries as _, messages_count as _); + let actual_size = InboundLaneData { + relayers: (1u8..=relayer_entries) + .map(|i| { + let mut entry = UnrewardedRelayer { + relayer: i, + messages: DeliveredMessages::new(i as _, true), + }; + entry.messages.dispatch_results = bitvec![ + Msb0, u8; + 1; + (messages_count / relayer_entries) as _ + ]; + entry + }) + .collect(), + last_confirmed_nonce: messages_count as _, + } + .encode() + .len(); + let difference = (expected_size.unwrap() as f64 - actual_size as f64).abs(); + assert!( + difference / (std::cmp::min(actual_size, expected_size.unwrap() as usize) as f64) < 0.1, + "Too large difference between actual ({}) and expected ({:?}) inbound lane data size. Test case: {}+{}", + actual_size, + expected_size, + relayer_entries, + messages_count, + ); } - .encode() - .len(); - let difference = (expected_size.unwrap() as f64 - actual_size as f64).abs(); - assert!( - difference / (std::cmp::min(actual_size, expected_size.unwrap() as usize) as f64) < 0.1, - "Too large difference between actual ({}) and expected ({:?}) inbound lane data size", - actual_size, - expected_size, - ); + } + + #[test] + fn message_dispatch_result_works() { + let delivered_messages = DeliveredMessages { + begin: 100, + end: 150, + dispatch_results: bitvec![Msb0, u8; 1; 151], + }; + + assert_eq!(delivered_messages.contains_message(99), false); + assert_eq!(delivered_messages.contains_message(100), true); + assert_eq!(delivered_messages.contains_message(150), true); + assert_eq!(delivered_messages.contains_message(151), false); + + assert_eq!(delivered_messages.message_dispatch_result(125), true); } } diff --git a/primitives/messages/src/target_chain.rs b/primitives/messages/src/target_chain.rs index 2a649f54d7..d1b87fd023 100644 --- a/primitives/messages/src/target_chain.rs +++ b/primitives/messages/src/target_chain.rs @@ -164,6 +164,7 @@ impl MessageDispatch for ForbidInboundMessages { fn dispatch(_: &AccountId, _: DispatchMessage) -> MessageDispatchResult { MessageDispatchResult { + dispatch_result: false, unspent_weight: 0, dispatch_fee_paid_during_dispatch: false, } diff --git a/primitives/runtime/src/messages.rs b/primitives/runtime/src/messages.rs index e97430c5b0..053b44ff04 100644 --- a/primitives/runtime/src/messages.rs +++ b/primitives/runtime/src/messages.rs @@ -36,6 +36,12 @@ pub enum DispatchFeePayment { /// Message dispatch result. #[derive(Encode, Decode, RuntimeDebug, Clone, PartialEq, Eq)] pub struct MessageDispatchResult { + /// Dispatch result flag. This flag is relayed back to the source chain and, generally + /// speaking, may bring any (that fits in single bit) information from the dispatcher at + /// the target chain to the message submitter at the source chain. If you're using immediate + /// call dispatcher, then it'll be result of the dispatch - `true` if dispatch has succeeded + /// and `false` otherwise. + pub dispatch_result: bool, /// Unspent dispatch weight. This weight that will be deducted from total delivery transaction /// weight, thus reducing the transaction cost. This shall not be zero in (at least) two cases: /// From 61ec89e9bfde5403cb4f83570e8bf430b6d121e9 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Wed, 28 Apr 2021 09:39:04 +0300 Subject: [PATCH 2/9] OnMessagesDelivered callback --- Cargo.lock | 1 + bin/millau/runtime/src/lib.rs | 1 + bin/rialto/runtime/src/lib.rs | 1 + modules/messages/src/lib.rs | 82 ++++++++++++++++++++++++- modules/messages/src/mock.rs | 42 ++++++++++++- primitives/messages/Cargo.toml | 1 + primitives/messages/src/source_chain.rs | 11 +++- 7 files changed, 135 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c491730f16..110800befe 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -729,6 +729,7 @@ dependencies = [ "bp-runtime", "frame-support", "frame-system", + "impl-trait-for-tuples", "parity-scale-codec", "serde", "sp-std", diff --git a/bin/millau/runtime/src/lib.rs b/bin/millau/runtime/src/lib.rs index 15e8bc42d9..8ccb80c523 100644 --- a/bin/millau/runtime/src/lib.rs +++ b/bin/millau/runtime/src/lib.rs @@ -384,6 +384,7 @@ impl pallet_bridge_messages::Config for Runtime { GetDeliveryConfirmationTransactionFee, RootAccountForPayments, >; + type OnMessagesDelivered = (); type SourceHeaderChain = crate::rialto_messages::Rialto; type MessageDispatch = crate::rialto_messages::FromRialtoMessageDispatch; diff --git a/bin/rialto/runtime/src/lib.rs b/bin/rialto/runtime/src/lib.rs index ebb29ab3f6..a2b41515c6 100644 --- a/bin/rialto/runtime/src/lib.rs +++ b/bin/rialto/runtime/src/lib.rs @@ -491,6 +491,7 @@ impl pallet_bridge_messages::Config for Runtime { GetDeliveryConfirmationTransactionFee, RootAccountForPayments, >; + type OnMessagesDelivered = (); type SourceHeaderChain = crate::millau_messages::Millau; type MessageDispatch = crate::millau_messages::FromMillauMessageDispatch; diff --git a/modules/messages/src/lib.rs b/modules/messages/src/lib.rs index 9892db32e9..ad35f299ff 100644 --- a/modules/messages/src/lib.rs +++ b/modules/messages/src/lib.rs @@ -47,7 +47,9 @@ use crate::outbound_lane::{OutboundLane, OutboundLaneStorage, ReceivalConfirmati use crate::weights::WeightInfo; use bp_messages::{ - source_chain::{LaneMessageVerifier, MessageDeliveryAndDispatchPayment, RelayersRewards, TargetHeaderChain}, + source_chain::{ + LaneMessageVerifier, MessageDeliveryAndDispatchPayment, OnMessagesDelivered, RelayersRewards, TargetHeaderChain, + }, target_chain::{DispatchMessage, MessageDispatch, ProvedLaneMessages, ProvedMessages, SourceHeaderChain}, total_unrewarded_messages, DeliveredMessages, InboundLaneData, LaneId, MessageData, MessageKey, MessageNonce, OperatingMode, OutboundLaneData, Parameter as MessagesParameter, UnrewardedRelayersState, @@ -146,6 +148,8 @@ pub trait Config: frame_system::Config { type LaneMessageVerifier: LaneMessageVerifier; /// Message delivery payment. type MessageDeliveryAndDispatchPayment: MessageDeliveryAndDispatchPayment; + /// Handler for delivered messages. + type OnMessagesDelivered: OnMessagesDelivered; // Types that are used by inbound_lane (on target chain). @@ -636,6 +640,10 @@ decl_module! { }, }; if let Some(confirmed_messages) = confirmed_messages { + // handle messages delivery + T::OnMessagesDelivered::on_messages_delivered(&confirmed_messages); + + // emit 'delivered' event let received_range = confirmed_messages.begin..=confirmed_messages.end; Self::deposit_event(RawEvent::MessagesDelivered(lane_id, confirmed_messages)); @@ -957,6 +965,10 @@ mod tests { fn send_regular_message() { get_ready_for_events(); + let message_nonce = outbound_lane::(TEST_LANE_ID) + .data() + .latest_generated_nonce + + 1; assert_ok!(Pallet::::send_message( Origin::signed(1), TEST_LANE_ID, @@ -969,7 +981,7 @@ mod tests { System::::events(), vec![EventRecord { phase: Phase::Initialization, - event: TestEvent::Messages(RawEvent::MessageAccepted(TEST_LANE_ID, 1)), + event: TestEvent::Messages(RawEvent::MessageAccepted(TEST_LANE_ID, message_nonce)), topics: vec![], }], ); @@ -1837,4 +1849,70 @@ mod tests { ); }); } + + #[test] + fn messages_delivered_callbacks_are_called() { + run_test(|| { + send_regular_message(); + send_regular_message(); + send_regular_message(); + + // messages 1+2 are confirmed in 1 tx, message 3 in a separate tx + // dispatch of message 2 has failed + let mut delivered_messages_1_and_2 = DeliveredMessages::new(1, true); + delivered_messages_1_and_2.note_dispatched_message(false); + let messages_1_and_2_proof = Ok(( + TEST_LANE_ID, + InboundLaneData { + last_confirmed_nonce: 0, + relayers: vec![UnrewardedRelayer { + relayer: 0, + messages: delivered_messages_1_and_2.clone(), + }] + .into_iter() + .collect(), + }, + )); + let delivered_message_3 = DeliveredMessages::new(3, true); + let messages_3_proof = Ok(( + TEST_LANE_ID, + InboundLaneData { + last_confirmed_nonce: 0, + relayers: vec![UnrewardedRelayer { + relayer: 0, + messages: delivered_message_3.clone(), + }] + .into_iter() + .collect(), + }, + )); + + // first tx with messages 1+2 + assert_ok!(Pallet::::receive_messages_delivery_proof( + Origin::signed(1), + TestMessagesDeliveryProof(messages_1_and_2_proof), + UnrewardedRelayersState { + unrewarded_relayer_entries: 1, + total_messages: 2, + ..Default::default() + }, + )); + // second tx with message 3 + assert_ok!(Pallet::::receive_messages_delivery_proof( + Origin::signed(1), + TestMessagesDeliveryProof(messages_3_proof), + UnrewardedRelayersState { + unrewarded_relayer_entries: 1, + total_messages: 1, + ..Default::default() + }, + )); + + // ensure that both callbacks have been called twice: for 1+2, then for 3 + crate::mock::TestOnMessagesDelivered1::ensure_called(&delivered_messages_1_and_2); + crate::mock::TestOnMessagesDelivered1::ensure_called(&delivered_message_3); + crate::mock::TestOnMessagesDelivered2::ensure_called(&delivered_messages_1_and_2); + crate::mock::TestOnMessagesDelivered2::ensure_called(&delivered_message_3); + }); + } } diff --git a/modules/messages/src/mock.rs b/modules/messages/src/mock.rs index 77813711e8..ca10f7d5a9 100644 --- a/modules/messages/src/mock.rs +++ b/modules/messages/src/mock.rs @@ -22,7 +22,8 @@ use crate::Config; use bitvec::prelude::*; use bp_messages::{ source_chain::{ - LaneMessageVerifier, MessageDeliveryAndDispatchPayment, RelayersRewards, Sender, TargetHeaderChain, + LaneMessageVerifier, MessageDeliveryAndDispatchPayment, OnMessagesDelivered, RelayersRewards, Sender, + TargetHeaderChain, }, target_chain::{DispatchMessage, MessageDispatch, ProvedLaneMessages, ProvedMessages, SourceHeaderChain}, DeliveredMessages, InboundLaneData, LaneId, Message, MessageData, MessageKey, MessageNonce, OutboundLaneData, @@ -170,6 +171,7 @@ impl Config for TestRuntime { type TargetHeaderChain = TestTargetHeaderChain; type LaneMessageVerifier = TestLaneMessageVerifier; type MessageDeliveryAndDispatchPayment = TestMessageDeliveryAndDispatchPayment; + type OnMessagesDelivered = (TestOnMessagesDelivered1, TestOnMessagesDelivered2); type SourceHeaderChain = TestSourceHeaderChain; type MessageDispatch = TestMessageDispatch; @@ -346,6 +348,44 @@ impl MessageDeliveryAndDispatchPayment for TestMessag } } +/// First on-messages-delivered callback. +#[derive(Debug)] +pub struct TestOnMessagesDelivered1; + +impl TestOnMessagesDelivered1 { + /// Verify that the callback has been called with given delivered messages. + pub fn ensure_called(messages: &DeliveredMessages) { + let key = (b"TestOnMessagesDelivered1", messages).encode(); + assert_eq!(frame_support::storage::unhashed::get(&key), Some(true)); + } +} + +impl OnMessagesDelivered for TestOnMessagesDelivered1 { + fn on_messages_delivered(messages: &DeliveredMessages) { + let key = (b"TestOnMessagesDelivered1", messages).encode(); + frame_support::storage::unhashed::put(&key, &true); + } +} + +/// Seconde on-messages-delivered callback. +#[derive(Debug)] +pub struct TestOnMessagesDelivered2; + +impl TestOnMessagesDelivered2 { + /// Verify that the callback has been called with given delivered messages. + pub fn ensure_called(messages: &DeliveredMessages) { + let key = (b"TestOnMessagesDelivered2", messages).encode(); + assert_eq!(frame_support::storage::unhashed::get(&key), Some(true)); + } +} + +impl OnMessagesDelivered for TestOnMessagesDelivered2 { + fn on_messages_delivered(messages: &DeliveredMessages) { + let key = (b"TestOnMessagesDelivered2", messages).encode(); + frame_support::storage::unhashed::put(&key, &true); + } +} + /// Source header chain that is used in tests. #[derive(Debug)] pub struct TestSourceHeaderChain; diff --git a/primitives/messages/Cargo.toml b/primitives/messages/Cargo.toml index 263fea1a70..b5b68220a4 100644 --- a/primitives/messages/Cargo.toml +++ b/primitives/messages/Cargo.toml @@ -9,6 +9,7 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0" [dependencies] bitvec = { version = "0.20", default-features = false, features = ["alloc"] } codec = { package = "parity-scale-codec", version = "2.0.0", default-features = false, features = ["derive", "bit-vec"] } +impl-trait-for-tuples = "0.2" serde = { version = "1.0.101", optional = true, features = ["derive"] } # Bridge dependencies diff --git a/primitives/messages/src/source_chain.rs b/primitives/messages/src/source_chain.rs index 1d313634bc..54118b4f0f 100644 --- a/primitives/messages/src/source_chain.rs +++ b/primitives/messages/src/source_chain.rs @@ -16,7 +16,7 @@ //! Primitives of messages module, that are used on the source chain. -use crate::{InboundLaneData, LaneId, MessageNonce, OutboundLaneData}; +use crate::{DeliveredMessages, InboundLaneData, LaneId, MessageNonce, OutboundLaneData}; use bp_runtime::Size; use frame_support::{Parameter, RuntimeDebug}; @@ -135,6 +135,15 @@ pub trait MessageDeliveryAndDispatchPayment { } } +/// Handler for delivered messages. +#[impl_trait_for_tuples::impl_for_tuples(30)] +pub trait OnMessagesDelivered { + /// Called when we receive confirmation that our messages have been delivered to the + /// target chain. The confirmation aso has single bit dispatch result for every + /// confirmed message (see `DeliveredMessages` for details). + fn on_messages_delivered(_messages: &DeliveredMessages) {} +} + /// Structure that may be used in place of `TargetHeaderChain`, `LaneMessageVerifier` and /// `MessageDeliveryAndDispatchPayment` on chains, where outbound messages are forbidden. pub struct ForbidOutboundMessages; From 2110b9afb42263f63c305192cb5603ca5c4c4a8c Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Wed, 28 Apr 2021 16:23:57 +0300 Subject: [PATCH 3/9] add lane id to OnDeliveredMessages callback --- modules/messages/src/lib.rs | 10 +++++----- modules/messages/src/mock.rs | 16 ++++++++-------- primitives/messages/src/source_chain.rs | 2 +- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/modules/messages/src/lib.rs b/modules/messages/src/lib.rs index ad35f299ff..88ccd7d5d6 100644 --- a/modules/messages/src/lib.rs +++ b/modules/messages/src/lib.rs @@ -641,7 +641,7 @@ decl_module! { }; if let Some(confirmed_messages) = confirmed_messages { // handle messages delivery - T::OnMessagesDelivered::on_messages_delivered(&confirmed_messages); + T::OnMessagesDelivered::on_messages_delivered(&lane_id, &confirmed_messages); // emit 'delivered' event let received_range = confirmed_messages.begin..=confirmed_messages.end; @@ -1909,10 +1909,10 @@ mod tests { )); // ensure that both callbacks have been called twice: for 1+2, then for 3 - crate::mock::TestOnMessagesDelivered1::ensure_called(&delivered_messages_1_and_2); - crate::mock::TestOnMessagesDelivered1::ensure_called(&delivered_message_3); - crate::mock::TestOnMessagesDelivered2::ensure_called(&delivered_messages_1_and_2); - crate::mock::TestOnMessagesDelivered2::ensure_called(&delivered_message_3); + crate::mock::TestOnMessagesDelivered1::ensure_called(&TEST_LANE_ID, &delivered_messages_1_and_2); + crate::mock::TestOnMessagesDelivered1::ensure_called(&TEST_LANE_ID, &delivered_message_3); + crate::mock::TestOnMessagesDelivered2::ensure_called(&TEST_LANE_ID, &delivered_messages_1_and_2); + crate::mock::TestOnMessagesDelivered2::ensure_called(&TEST_LANE_ID, &delivered_message_3); }); } } diff --git a/modules/messages/src/mock.rs b/modules/messages/src/mock.rs index ca10f7d5a9..ae1422333d 100644 --- a/modules/messages/src/mock.rs +++ b/modules/messages/src/mock.rs @@ -354,15 +354,15 @@ pub struct TestOnMessagesDelivered1; impl TestOnMessagesDelivered1 { /// Verify that the callback has been called with given delivered messages. - pub fn ensure_called(messages: &DeliveredMessages) { - let key = (b"TestOnMessagesDelivered1", messages).encode(); + pub fn ensure_called(lane: &LaneId, messages: &DeliveredMessages) { + let key = (b"TestOnMessagesDelivered1", lane, messages).encode(); assert_eq!(frame_support::storage::unhashed::get(&key), Some(true)); } } impl OnMessagesDelivered for TestOnMessagesDelivered1 { - fn on_messages_delivered(messages: &DeliveredMessages) { - let key = (b"TestOnMessagesDelivered1", messages).encode(); + fn on_messages_delivered(lane: &LaneId, messages: &DeliveredMessages) { + let key = (b"TestOnMessagesDelivered1", lane, messages).encode(); frame_support::storage::unhashed::put(&key, &true); } } @@ -373,15 +373,15 @@ pub struct TestOnMessagesDelivered2; impl TestOnMessagesDelivered2 { /// Verify that the callback has been called with given delivered messages. - pub fn ensure_called(messages: &DeliveredMessages) { - let key = (b"TestOnMessagesDelivered2", messages).encode(); + pub fn ensure_called(lane: &LaneId, messages: &DeliveredMessages) { + let key = (b"TestOnMessagesDelivered2", lane, messages).encode(); assert_eq!(frame_support::storage::unhashed::get(&key), Some(true)); } } impl OnMessagesDelivered for TestOnMessagesDelivered2 { - fn on_messages_delivered(messages: &DeliveredMessages) { - let key = (b"TestOnMessagesDelivered2", messages).encode(); + fn on_messages_delivered(lane: &LaneId, messages: &DeliveredMessages) { + let key = (b"TestOnMessagesDelivered2", lane, messages).encode(); frame_support::storage::unhashed::put(&key, &true); } } diff --git a/primitives/messages/src/source_chain.rs b/primitives/messages/src/source_chain.rs index 54118b4f0f..5d0bdf70ff 100644 --- a/primitives/messages/src/source_chain.rs +++ b/primitives/messages/src/source_chain.rs @@ -141,7 +141,7 @@ pub trait OnMessagesDelivered { /// Called when we receive confirmation that our messages have been delivered to the /// target chain. The confirmation aso has single bit dispatch result for every /// confirmed message (see `DeliveredMessages` for details). - fn on_messages_delivered(_messages: &DeliveredMessages) {} + fn on_messages_delivered(_lane: &LaneId, _messages: &DeliveredMessages) {} } /// Structure that may be used in place of `TargetHeaderChain`, `LaneMessageVerifier` and From 7377557f4c0d48a51cd5df950a0881c66d78fae1 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Fri, 18 Jun 2021 13:52:54 +0300 Subject: [PATCH 4/9] fix benchmarks && upate weights --- bin/rialto/runtime/src/lib.rs | 2 +- modules/messages/src/benchmarking.rs | 72 ++++++++++++--- modules/messages/src/weights.rs | 102 +++++++++++----------- relays/bin-substrate/src/messages_lane.rs | 2 +- 4 files changed, 113 insertions(+), 65 deletions(-) diff --git a/bin/rialto/runtime/src/lib.rs b/bin/rialto/runtime/src/lib.rs index a2b41515c6..89ad855478 100644 --- a/bin/rialto/runtime/src/lib.rs +++ b/bin/rialto/runtime/src/lib.rs @@ -1029,7 +1029,7 @@ impl_runtime_apis! { .map(|event_record| event_record.event) .any(|event| matches!( event, - Event::pallet_bridge_dispatch(pallet_bridge_dispatch::Event::::MessageDispatched( + Event::BridgeDispatch(pallet_bridge_dispatch::Event::::MessageDispatched( _, ([0, 0, 0, 0], nonce_from_event), _, )) if nonce_from_event == nonce )) diff --git a/modules/messages/src/benchmarking.rs b/modules/messages/src/benchmarking.rs index 5fc04bb2b2..6af28e8c5c 100644 --- a/modules/messages/src/benchmarking.rs +++ b/modules/messages/src/benchmarking.rs @@ -17,17 +17,25 @@ //! Messages pallet benchmarking. use crate::weights_ext::EXPECTED_DEFAULT_MESSAGE_LENGTH; -use crate::{inbound_lane::InboundLaneStorage, inbound_lane_storage, outbound_lane, Call, Instance}; +use crate::{ + inbound_lane::InboundLaneStorage, inbound_lane_storage, outbound_lane, outbound_lane::ReceivalConfirmationResult, + Call, Instance, +}; use bp_messages::{ - source_chain::TargetHeaderChain, target_chain::SourceHeaderChain, InboundLaneData, LaneId, MessageData, - MessageNonce, OutboundLaneData, UnrewardedRelayersState, + source_chain::TargetHeaderChain, target_chain::SourceHeaderChain, DeliveredMessages, InboundLaneData, LaneId, + MessageData, MessageNonce, OutboundLaneData, UnrewardedRelayer, UnrewardedRelayersState, }; use bp_runtime::messages::DispatchFeePayment; use frame_benchmarking::{account, benchmarks_instance}; use frame_support::{traits::Get, weights::Weight}; use frame_system::RawOrigin; -use sp_std::{collections::btree_map::BTreeMap, convert::TryInto, ops::RangeInclusive, prelude::*}; +use sp_std::{ + collections::{btree_map::BTreeMap, vec_deque::VecDeque}, + convert::TryInto, + ops::RangeInclusive, + prelude::*, +}; /// Fee paid by submitter for single message delivery. pub const MESSAGE_FEE: u64 = 10_000_000_000; @@ -471,7 +479,10 @@ benchmarks_instance! { let proof = T::prepare_message_delivery_proof(MessageDeliveryProofParams { lane: T::bench_lane_id(), inbound_lane_data: InboundLaneData { - relayers: vec![(1, 1, relayer_id.clone())].into_iter().collect(), + relayers: vec![UnrewardedRelayer { + relayer: relayer_id.clone(), + messages: DeliveredMessages::new(1, true), + }].into_iter().collect(), last_confirmed_nonce: 0, }, size: ProofSize::Minimal(0), @@ -506,10 +517,15 @@ benchmarks_instance! { messages_in_oldest_entry: 2, total_messages: 2, }; + let mut delivered_messages = DeliveredMessages::new(1, true); + delivered_messages.note_dispatched_message(true); let proof = T::prepare_message_delivery_proof(MessageDeliveryProofParams { lane: T::bench_lane_id(), inbound_lane_data: InboundLaneData { - relayers: vec![(1, 2, relayer_id.clone())].into_iter().collect(), + relayers: vec![UnrewardedRelayer { + relayer: relayer_id.clone(), + messages: delivered_messages, + }].into_iter().collect(), last_confirmed_nonce: 0, }, size: ProofSize::Minimal(0), @@ -547,8 +563,14 @@ benchmarks_instance! { lane: T::bench_lane_id(), inbound_lane_data: InboundLaneData { relayers: vec![ - (1, 1, relayer1_id.clone()), - (2, 2, relayer2_id.clone()), + UnrewardedRelayer { + relayer: relayer1_id.clone(), + messages: DeliveredMessages::new(1, true), + }, + UnrewardedRelayer { + relayer: relayer2_id.clone(), + messages: DeliveredMessages::new(2, true), + }, ].into_iter().collect(), last_confirmed_nonce: 0, }, @@ -783,10 +805,17 @@ benchmarks_instance! { messages_in_oldest_entry: 1, total_messages: i as MessageNonce, }; + let mut delivered_messages = DeliveredMessages::new(1, true); + for nonce in 2..=i { + delivered_messages.note_dispatched_message(true); + } let proof = T::prepare_message_delivery_proof(MessageDeliveryProofParams { lane: T::bench_lane_id(), inbound_lane_data: InboundLaneData { - relayers: vec![(1, i as MessageNonce, relayer_id.clone())].into_iter().collect(), + relayers: vec![UnrewardedRelayer { + relayer: relayer_id.clone(), + messages: delivered_messages, + }].into_iter().collect(), last_confirmed_nonce: 0, }, size: ProofSize::Minimal(0), @@ -831,7 +860,10 @@ benchmarks_instance! { relayers: relayers .keys() .enumerate() - .map(|(j, relayer_id)| (j as MessageNonce + 1, j as MessageNonce + 1, relayer_id.clone())) + .map(|(j, relayer)| UnrewardedRelayer { + relayer: relayer.clone(), + messages: DeliveredMessages::new(j as MessageNonce + 1, true), + }) .collect(), last_confirmed_nonce: 0, }, @@ -863,13 +895,29 @@ fn send_regular_message_with_payload, I: Instance>(payload: Vec fn confirm_message_delivery, I: Instance>(nonce: MessageNonce) { let mut outbound_lane = outbound_lane::(T::bench_lane_id()); - assert!(outbound_lane.confirm_delivery(nonce).is_some()); + let latest_received_nonce = outbound_lane.data().latest_received_nonce; + let mut relayers = VecDeque::with_capacity((nonce - latest_received_nonce) as usize); + for nonce in latest_received_nonce + 1..=nonce { + relayers.push_back(UnrewardedRelayer { + relayer: (), + messages: DeliveredMessages::new(nonce, true), + }); + } + assert!(matches!( + outbound_lane.confirm_delivery(nonce, &relayers), + ReceivalConfirmationResult::ConfirmedMessages(_), + )); } fn receive_messages, I: Instance>(nonce: MessageNonce) { let mut inbound_lane_storage = inbound_lane_storage::(T::bench_lane_id()); inbound_lane_storage.set_data(InboundLaneData { - relayers: vec![(1, nonce, T::bridged_relayer_id())].into_iter().collect(), + relayers: vec![UnrewardedRelayer { + relayer: T::bridged_relayer_id(), + messages: DeliveredMessages::new(nonce, true), + }] + .into_iter() + .collect(), last_confirmed_nonce: 0, }); } diff --git a/modules/messages/src/weights.rs b/modules/messages/src/weights.rs index ffe89c58bd..9b65c8217a 100644 --- a/modules/messages/src/weights.rs +++ b/modules/messages/src/weights.rs @@ -17,7 +17,7 @@ //! Autogenerated weights for pallet_bridge_messages //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 3.0.0 -//! DATE: 2021-06-15, STEPS: [50, ], REPEAT: 20 +//! DATE: 2021-06-18, STEPS: [50, ], REPEAT: 20 //! LOW RANGE: [], HIGH RANGE: [] //! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled //! CHAIN: Some("dev"), DB CACHE: 128 @@ -74,110 +74,110 @@ pub trait WeightInfo { pub struct RialtoWeight(PhantomData); impl WeightInfo for RialtoWeight { fn send_minimal_message_worst_case() -> Weight { - (154_371_000 as Weight) + (159_305_000 as Weight) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(12 as Weight)) } fn send_1_kb_message_worst_case() -> Weight { - (157_479_000 as Weight) + (164_394_000 as Weight) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(12 as Weight)) } fn send_16_kb_message_worst_case() -> Weight { - (186_840_000 as Weight) + (223_521_000 as Weight) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(12 as Weight)) } fn increase_message_fee() -> Weight { - (4_377_567_000 as Weight) + (6_709_925_000 as Weight) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn receive_single_message_proof() -> Weight { - (205_350_000 as Weight) + (206_769_000 as Weight) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn receive_two_messages_proof() -> Weight { - (337_102_000 as Weight) + (343_982_000 as Weight) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn receive_single_message_proof_with_outbound_lane_state() -> Weight { - (218_825_000 as Weight) + (223_738_000 as Weight) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn receive_single_message_proof_1_kb() -> Weight { - (230_759_000 as Weight) + (235_369_000 as Weight) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn receive_single_message_proof_16_kb() -> Weight { - (490_522_000 as Weight) + (510_338_000 as Weight) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn receive_single_prepaid_message_proof() -> Weight { - (136_550_000 as Weight) + (141_536_000 as Weight) .saturating_add(T::DbWeight::get().reads(3 as Weight)) .saturating_add(T::DbWeight::get().writes(1 as Weight)) } fn receive_delivery_proof_for_single_message() -> Weight { - (131_397_000 as Weight) + (128_805_000 as Weight) .saturating_add(T::DbWeight::get().reads(6 as Weight)) .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn receive_delivery_proof_for_two_messages_by_single_relayer() -> Weight { - (137_946_000 as Weight) + (137_143_000 as Weight) .saturating_add(T::DbWeight::get().reads(7 as Weight)) .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn receive_delivery_proof_for_two_messages_by_two_relayers() -> Weight { - (194_448_000 as Weight) + (193_108_000 as Weight) .saturating_add(T::DbWeight::get().reads(8 as Weight)) .saturating_add(T::DbWeight::get().writes(4 as Weight)) } fn send_messages_of_various_lengths(i: u32) -> Weight { - (142_576_000 as Weight) - .saturating_add((2_000 as Weight).saturating_mul(i as Weight)) + (133_632_000 as Weight) + .saturating_add((4_000 as Weight).saturating_mul(i as Weight)) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(12 as Weight)) } fn receive_multiple_messages_proof(i: u32) -> Weight { (0 as Weight) - .saturating_add((138_341_000 as Weight).saturating_mul(i as Weight)) + .saturating_add((145_006_000 as Weight).saturating_mul(i as Weight)) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn receive_message_proofs_with_extra_nodes(i: u32) -> Weight { - (472_752_000 as Weight) - .saturating_add((9_000 as Weight).saturating_mul(i as Weight)) + (486_301_000 as Weight) + .saturating_add((10_000 as Weight).saturating_mul(i as Weight)) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn receive_message_proofs_with_large_leaf(i: u32) -> Weight { - (175_300_000 as Weight) - .saturating_add((6_000 as Weight).saturating_mul(i as Weight)) + (178_139_000 as Weight) + .saturating_add((7_000 as Weight).saturating_mul(i as Weight)) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn receive_multiple_messages_proof_with_outbound_lane_state(i: u32) -> Weight { (0 as Weight) - .saturating_add((142_176_000 as Weight).saturating_mul(i as Weight)) + .saturating_add((150_844_000 as Weight).saturating_mul(i as Weight)) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn receive_delivery_proof_for_multiple_messages_by_single_relayer(i: u32) -> Weight { - (127_372_000 as Weight) - .saturating_add((7_927_000 as Weight).saturating_mul(i as Weight)) + (113_140_000 as Weight) + .saturating_add((7_656_000 as Weight).saturating_mul(i as Weight)) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().reads((1 as Weight).saturating_mul(i as Weight))) .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn receive_delivery_proof_for_multiple_messages_by_multiple_relayers(i: u32) -> Weight { - (99_781_000 as Weight) - .saturating_add((64_001_000 as Weight).saturating_mul(i as Weight)) + (97_424_000 as Weight) + .saturating_add((63_128_000 as Weight).saturating_mul(i as Weight)) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().reads((2 as Weight).saturating_mul(i as Weight))) .saturating_add(T::DbWeight::get().writes(3 as Weight)) @@ -188,110 +188,110 @@ impl WeightInfo for RialtoWeight { // For backwards compatibility and tests impl WeightInfo for () { fn send_minimal_message_worst_case() -> Weight { - (154_371_000 as Weight) + (159_305_000 as Weight) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(12 as Weight)) } fn send_1_kb_message_worst_case() -> Weight { - (157_479_000 as Weight) + (164_394_000 as Weight) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(12 as Weight)) } fn send_16_kb_message_worst_case() -> Weight { - (186_840_000 as Weight) + (223_521_000 as Weight) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(12 as Weight)) } fn increase_message_fee() -> Weight { - (4_377_567_000 as Weight) + (6_709_925_000 as Weight) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_single_message_proof() -> Weight { - (205_350_000 as Weight) + (206_769_000 as Weight) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_two_messages_proof() -> Weight { - (337_102_000 as Weight) + (343_982_000 as Weight) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_single_message_proof_with_outbound_lane_state() -> Weight { - (218_825_000 as Weight) + (223_738_000 as Weight) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_single_message_proof_1_kb() -> Weight { - (230_759_000 as Weight) + (235_369_000 as Weight) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_single_message_proof_16_kb() -> Weight { - (490_522_000 as Weight) + (510_338_000 as Weight) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_single_prepaid_message_proof() -> Weight { - (136_550_000 as Weight) + (141_536_000 as Weight) .saturating_add(RocksDbWeight::get().reads(3 as Weight)) .saturating_add(RocksDbWeight::get().writes(1 as Weight)) } fn receive_delivery_proof_for_single_message() -> Weight { - (131_397_000 as Weight) + (128_805_000 as Weight) .saturating_add(RocksDbWeight::get().reads(6 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_delivery_proof_for_two_messages_by_single_relayer() -> Weight { - (137_946_000 as Weight) + (137_143_000 as Weight) .saturating_add(RocksDbWeight::get().reads(7 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_delivery_proof_for_two_messages_by_two_relayers() -> Weight { - (194_448_000 as Weight) + (193_108_000 as Weight) .saturating_add(RocksDbWeight::get().reads(8 as Weight)) .saturating_add(RocksDbWeight::get().writes(4 as Weight)) } fn send_messages_of_various_lengths(i: u32) -> Weight { - (142_576_000 as Weight) - .saturating_add((2_000 as Weight).saturating_mul(i as Weight)) + (133_632_000 as Weight) + .saturating_add((4_000 as Weight).saturating_mul(i as Weight)) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(12 as Weight)) } fn receive_multiple_messages_proof(i: u32) -> Weight { (0 as Weight) - .saturating_add((138_341_000 as Weight).saturating_mul(i as Weight)) + .saturating_add((145_006_000 as Weight).saturating_mul(i as Weight)) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_message_proofs_with_extra_nodes(i: u32) -> Weight { - (472_752_000 as Weight) - .saturating_add((9_000 as Weight).saturating_mul(i as Weight)) + (486_301_000 as Weight) + .saturating_add((10_000 as Weight).saturating_mul(i as Weight)) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_message_proofs_with_large_leaf(i: u32) -> Weight { - (175_300_000 as Weight) - .saturating_add((6_000 as Weight).saturating_mul(i as Weight)) + (178_139_000 as Weight) + .saturating_add((7_000 as Weight).saturating_mul(i as Weight)) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_multiple_messages_proof_with_outbound_lane_state(i: u32) -> Weight { (0 as Weight) - .saturating_add((142_176_000 as Weight).saturating_mul(i as Weight)) + .saturating_add((150_844_000 as Weight).saturating_mul(i as Weight)) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_delivery_proof_for_multiple_messages_by_single_relayer(i: u32) -> Weight { - (127_372_000 as Weight) - .saturating_add((7_927_000 as Weight).saturating_mul(i as Weight)) + (113_140_000 as Weight) + .saturating_add((7_656_000 as Weight).saturating_mul(i as Weight)) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().reads((1 as Weight).saturating_mul(i as Weight))) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_delivery_proof_for_multiple_messages_by_multiple_relayers(i: u32) -> Weight { - (99_781_000 as Weight) - .saturating_add((64_001_000 as Weight).saturating_mul(i as Weight)) + (97_424_000 as Weight) + .saturating_add((63_128_000 as Weight).saturating_mul(i as Weight)) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().reads((2 as Weight).saturating_mul(i as Weight))) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) diff --git a/relays/bin-substrate/src/messages_lane.rs b/relays/bin-substrate/src/messages_lane.rs index 5bb64ed077..7efea545f9 100644 --- a/relays/bin-substrate/src/messages_lane.rs +++ b/relays/bin-substrate/src/messages_lane.rs @@ -204,7 +204,7 @@ mod tests { // reserved for messages dispatch allows dispatch of non-trivial messages. // // Any significant change in this values should attract additional attention. - (814, 216_583_333_334), + (782, 216_583_333_334), ); } } From b43bc214a1d4132448caa6a14e253017b3767a5e Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Fri, 18 Jun 2021 14:10:48 +0300 Subject: [PATCH 5/9] clippy --- primitives/messages/src/lib.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/primitives/messages/src/lib.rs b/primitives/messages/src/lib.rs index d51200273a..e1e7522181 100644 --- a/primitives/messages/src/lib.rs +++ b/primitives/messages/src/lib.rs @@ -239,7 +239,7 @@ impl DeliveredMessages { /// Panics if message nonce is not in the `begin..=end` range. Typically you'll first /// check if message is within the range by calling `contains_message`. pub fn message_dispatch_result(&self, nonce: MessageNonce) -> bool { - const INVALID_NONCE: &'static str = "Invalid nonce used to index dispatch_results"; + const INVALID_NONCE: &str = "Invalid nonce used to index dispatch_results"; let index = nonce.checked_sub(self.begin).expect(INVALID_NONCE) as usize; *self.dispatch_results.get(index).expect(INVALID_NONCE) @@ -373,11 +373,11 @@ mod tests { dispatch_results: bitvec![Msb0, u8; 1; 151], }; - assert_eq!(delivered_messages.contains_message(99), false); - assert_eq!(delivered_messages.contains_message(100), true); - assert_eq!(delivered_messages.contains_message(150), true); - assert_eq!(delivered_messages.contains_message(151), false); + assert!(!delivered_messages.contains_message(99)); + assert!(delivered_messages.contains_message(100)); + assert!(delivered_messages.contains_message(150)); + assert!(!delivered_messages.contains_message(151)); - assert_eq!(delivered_messages.message_dispatch_result(125), true); + assert!(delivered_messages.message_dispatch_result(125)); } } From 71be9edb27f72563a0d4aac0513f29f8db8cfb99 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Fri, 18 Jun 2021 14:19:52 +0300 Subject: [PATCH 6/9] clippy --- modules/messages/src/outbound_lane.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/messages/src/outbound_lane.rs b/modules/messages/src/outbound_lane.rs index 461c053302..d85ed8e982 100644 --- a/modules/messages/src/outbound_lane.rs +++ b/modules/messages/src/outbound_lane.rs @@ -335,6 +335,7 @@ mod tests { #[test] fn confirm_delivery_fails_if_entry_is_empty() { + #[allow(clippy::reversed_empty_ranges)] assert_eq!( assert_3_messages_confirmation_fails( 3, From bd88510208a40ac0f87e32f85523cc9363726b86 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Fri, 18 Jun 2021 14:46:25 +0300 Subject: [PATCH 7/9] clipy another try --- modules/messages/src/outbound_lane.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/messages/src/outbound_lane.rs b/modules/messages/src/outbound_lane.rs index d85ed8e982..44061d984e 100644 --- a/modules/messages/src/outbound_lane.rs +++ b/modules/messages/src/outbound_lane.rs @@ -334,8 +334,8 @@ mod tests { } #[test] + #[allow(clippy::reversed_empty_ranges)] fn confirm_delivery_fails_if_entry_is_empty() { - #[allow(clippy::reversed_empty_ranges)] assert_eq!( assert_3_messages_confirmation_fails( 3, From 15b3acf52b8c8c70156e4316bd8b3b8c0ef97f8e Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Mon, 21 Jun 2021 10:56:00 +0300 Subject: [PATCH 8/9] OnMessagesDelivered -> OnDeliveryConfirmed --- bin/millau/runtime/src/lib.rs | 2 +- bin/rialto/runtime/src/lib.rs | 2 +- modules/messages/src/lib.rs | 16 ++++++++-------- modules/messages/src/mock.rs | 24 ++++++++++++------------ primitives/messages/src/source_chain.rs | 4 ++-- 5 files changed, 24 insertions(+), 24 deletions(-) diff --git a/bin/millau/runtime/src/lib.rs b/bin/millau/runtime/src/lib.rs index 8ccb80c523..84d62c2e30 100644 --- a/bin/millau/runtime/src/lib.rs +++ b/bin/millau/runtime/src/lib.rs @@ -384,7 +384,7 @@ impl pallet_bridge_messages::Config for Runtime { GetDeliveryConfirmationTransactionFee, RootAccountForPayments, >; - type OnMessagesDelivered = (); + type OnDeliveryConfirmed = (); type SourceHeaderChain = crate::rialto_messages::Rialto; type MessageDispatch = crate::rialto_messages::FromRialtoMessageDispatch; diff --git a/bin/rialto/runtime/src/lib.rs b/bin/rialto/runtime/src/lib.rs index 89ad855478..46c01baf4f 100644 --- a/bin/rialto/runtime/src/lib.rs +++ b/bin/rialto/runtime/src/lib.rs @@ -491,7 +491,7 @@ impl pallet_bridge_messages::Config for Runtime { GetDeliveryConfirmationTransactionFee, RootAccountForPayments, >; - type OnMessagesDelivered = (); + type OnDeliveryConfirmed = (); type SourceHeaderChain = crate::millau_messages::Millau; type MessageDispatch = crate::millau_messages::FromMillauMessageDispatch; diff --git a/modules/messages/src/lib.rs b/modules/messages/src/lib.rs index 88ccd7d5d6..054c0e2a02 100644 --- a/modules/messages/src/lib.rs +++ b/modules/messages/src/lib.rs @@ -48,7 +48,7 @@ use crate::weights::WeightInfo; use bp_messages::{ source_chain::{ - LaneMessageVerifier, MessageDeliveryAndDispatchPayment, OnMessagesDelivered, RelayersRewards, TargetHeaderChain, + LaneMessageVerifier, MessageDeliveryAndDispatchPayment, OnDeliveryConfirmed, RelayersRewards, TargetHeaderChain, }, target_chain::{DispatchMessage, MessageDispatch, ProvedLaneMessages, ProvedMessages, SourceHeaderChain}, total_unrewarded_messages, DeliveredMessages, InboundLaneData, LaneId, MessageData, MessageKey, MessageNonce, @@ -149,7 +149,7 @@ pub trait Config: frame_system::Config { /// Message delivery payment. type MessageDeliveryAndDispatchPayment: MessageDeliveryAndDispatchPayment; /// Handler for delivered messages. - type OnMessagesDelivered: OnMessagesDelivered; + type OnDeliveryConfirmed: OnDeliveryConfirmed; // Types that are used by inbound_lane (on target chain). @@ -640,8 +640,8 @@ decl_module! { }, }; if let Some(confirmed_messages) = confirmed_messages { - // handle messages delivery - T::OnMessagesDelivered::on_messages_delivered(&lane_id, &confirmed_messages); + // handle messages delivery confirmation + T::OnDeliveryConfirmed::on_messages_delivered(&lane_id, &confirmed_messages); // emit 'delivered' event let received_range = confirmed_messages.begin..=confirmed_messages.end; @@ -1909,10 +1909,10 @@ mod tests { )); // ensure that both callbacks have been called twice: for 1+2, then for 3 - crate::mock::TestOnMessagesDelivered1::ensure_called(&TEST_LANE_ID, &delivered_messages_1_and_2); - crate::mock::TestOnMessagesDelivered1::ensure_called(&TEST_LANE_ID, &delivered_message_3); - crate::mock::TestOnMessagesDelivered2::ensure_called(&TEST_LANE_ID, &delivered_messages_1_and_2); - crate::mock::TestOnMessagesDelivered2::ensure_called(&TEST_LANE_ID, &delivered_message_3); + crate::mock::TestOnDeliveryConfirmed1::ensure_called(&TEST_LANE_ID, &delivered_messages_1_and_2); + crate::mock::TestOnDeliveryConfirmed1::ensure_called(&TEST_LANE_ID, &delivered_message_3); + crate::mock::TestOnDeliveryConfirmed2::ensure_called(&TEST_LANE_ID, &delivered_messages_1_and_2); + crate::mock::TestOnDeliveryConfirmed2::ensure_called(&TEST_LANE_ID, &delivered_message_3); }); } } diff --git a/modules/messages/src/mock.rs b/modules/messages/src/mock.rs index ae1422333d..2e184dda15 100644 --- a/modules/messages/src/mock.rs +++ b/modules/messages/src/mock.rs @@ -22,7 +22,7 @@ use crate::Config; use bitvec::prelude::*; use bp_messages::{ source_chain::{ - LaneMessageVerifier, MessageDeliveryAndDispatchPayment, OnMessagesDelivered, RelayersRewards, Sender, + LaneMessageVerifier, MessageDeliveryAndDispatchPayment, OnDeliveryConfirmed, RelayersRewards, Sender, TargetHeaderChain, }, target_chain::{DispatchMessage, MessageDispatch, ProvedLaneMessages, ProvedMessages, SourceHeaderChain}, @@ -171,7 +171,7 @@ impl Config for TestRuntime { type TargetHeaderChain = TestTargetHeaderChain; type LaneMessageVerifier = TestLaneMessageVerifier; type MessageDeliveryAndDispatchPayment = TestMessageDeliveryAndDispatchPayment; - type OnMessagesDelivered = (TestOnMessagesDelivered1, TestOnMessagesDelivered2); + type OnDeliveryConfirmed = (TestOnDeliveryConfirmed1, TestOnDeliveryConfirmed2); type SourceHeaderChain = TestSourceHeaderChain; type MessageDispatch = TestMessageDispatch; @@ -350,38 +350,38 @@ impl MessageDeliveryAndDispatchPayment for TestMessag /// First on-messages-delivered callback. #[derive(Debug)] -pub struct TestOnMessagesDelivered1; +pub struct TestOnDeliveryConfirmed1; -impl TestOnMessagesDelivered1 { +impl TestOnDeliveryConfirmed1 { /// Verify that the callback has been called with given delivered messages. pub fn ensure_called(lane: &LaneId, messages: &DeliveredMessages) { - let key = (b"TestOnMessagesDelivered1", lane, messages).encode(); + let key = (b"TestOnDeliveryConfirmed1", lane, messages).encode(); assert_eq!(frame_support::storage::unhashed::get(&key), Some(true)); } } -impl OnMessagesDelivered for TestOnMessagesDelivered1 { +impl OnDeliveryConfirmed for TestOnDeliveryConfirmed1 { fn on_messages_delivered(lane: &LaneId, messages: &DeliveredMessages) { - let key = (b"TestOnMessagesDelivered1", lane, messages).encode(); + let key = (b"TestOnDeliveryConfirmed1", lane, messages).encode(); frame_support::storage::unhashed::put(&key, &true); } } /// Seconde on-messages-delivered callback. #[derive(Debug)] -pub struct TestOnMessagesDelivered2; +pub struct TestOnDeliveryConfirmed2; -impl TestOnMessagesDelivered2 { +impl TestOnDeliveryConfirmed2 { /// Verify that the callback has been called with given delivered messages. pub fn ensure_called(lane: &LaneId, messages: &DeliveredMessages) { - let key = (b"TestOnMessagesDelivered2", lane, messages).encode(); + let key = (b"TestOnDeliveryConfirmed2", lane, messages).encode(); assert_eq!(frame_support::storage::unhashed::get(&key), Some(true)); } } -impl OnMessagesDelivered for TestOnMessagesDelivered2 { +impl OnDeliveryConfirmed for TestOnDeliveryConfirmed2 { fn on_messages_delivered(lane: &LaneId, messages: &DeliveredMessages) { - let key = (b"TestOnMessagesDelivered2", lane, messages).encode(); + let key = (b"TestOnDeliveryConfirmed2", lane, messages).encode(); frame_support::storage::unhashed::put(&key, &true); } } diff --git a/primitives/messages/src/source_chain.rs b/primitives/messages/src/source_chain.rs index 5d0bdf70ff..0bed86a1b2 100644 --- a/primitives/messages/src/source_chain.rs +++ b/primitives/messages/src/source_chain.rs @@ -135,9 +135,9 @@ pub trait MessageDeliveryAndDispatchPayment { } } -/// Handler for delivered messages. +/// Handler for messages delivery confirmation. #[impl_trait_for_tuples::impl_for_tuples(30)] -pub trait OnMessagesDelivered { +pub trait OnDeliveryConfirmed { /// Called when we receive confirmation that our messages have been delivered to the /// target chain. The confirmation aso has single bit dispatch result for every /// confirmed message (see `DeliveredMessages` for details). From d231c82004620f68a2e846a670dffb83e52c61bd Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Mon, 21 Jun 2021 10:58:13 +0300 Subject: [PATCH 9/9] Update primitives/messages/src/source_chain.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Tomasz Drwięga --- primitives/messages/src/source_chain.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/primitives/messages/src/source_chain.rs b/primitives/messages/src/source_chain.rs index 5d0bdf70ff..61d044b307 100644 --- a/primitives/messages/src/source_chain.rs +++ b/primitives/messages/src/source_chain.rs @@ -139,7 +139,7 @@ pub trait MessageDeliveryAndDispatchPayment { #[impl_trait_for_tuples::impl_for_tuples(30)] pub trait OnMessagesDelivered { /// Called when we receive confirmation that our messages have been delivered to the - /// target chain. The confirmation aso has single bit dispatch result for every + /// target chain. The confirmation also has single bit dispatch result for every /// confirmed message (see `DeliveredMessages` for details). fn on_messages_delivered(_lane: &LaneId, _messages: &DeliveredMessages) {} }