diff --git a/bridges/modules/message-lane/src/inbound_lane.rs b/bridges/modules/message-lane/src/inbound_lane.rs index 9fedf043194da..0c88b7796998d 100644 --- a/bridges/modules/message-lane/src/inbound_lane.rs +++ b/bridges/modules/message-lane/src/inbound_lane.rs @@ -17,13 +17,12 @@ //! Everything about incoming messages receival. use bp_message_lane::{ - target_chain::MessageDispatch, InboundLaneData, LaneId, Message, MessageData, MessageKey, MessageNonce, + target_chain::{DispatchMessage, DispatchMessageData, MessageDispatch}, + InboundLaneData, LaneId, MessageKey, MessageNonce, }; /// Inbound lane storage. pub trait InboundLaneStorage { - /// Message payload. - type Payload; /// Delivery and dispatch fee type on source chain. type MessageFee; @@ -47,10 +46,10 @@ impl InboundLane { } /// Receive new message. - pub fn receive_message>( + pub fn receive_message>( &mut self, nonce: MessageNonce, - message_data: MessageData, + message_data: DispatchMessageData, ) -> bool { let mut data = self.storage.data(); let is_correct_message = nonce == data.latest_received_nonce + 1; @@ -61,7 +60,7 @@ impl InboundLane { data.latest_received_nonce = nonce; self.storage.set_data(data); - P::dispatch(Message { + P::dispatch(DispatchMessage { key: MessageKey { lane_id: self.storage.id(), nonce, @@ -85,7 +84,7 @@ mod tests { fn fails_to_receive_message_with_incorrect_nonce() { run_test(|| { let mut lane = inbound_lane::(TEST_LANE_ID); - assert!(!lane.receive_message::(10, message_data(REGULAR_PAYLOAD))); + assert!(!lane.receive_message::(10, message_data(REGULAR_PAYLOAD).into())); assert_eq!(lane.storage.data().latest_received_nonce, 0); }); } @@ -94,7 +93,7 @@ mod tests { fn correct_message_is_processed_instantly() { run_test(|| { let mut lane = inbound_lane::(TEST_LANE_ID); - assert!(lane.receive_message::(1, message_data(REGULAR_PAYLOAD))); + assert!(lane.receive_message::(1, message_data(REGULAR_PAYLOAD).into())); assert_eq!(lane.storage.data().latest_received_nonce, 1); }); } diff --git a/bridges/modules/message-lane/src/lib.rs b/bridges/modules/message-lane/src/lib.rs index 5f99eee756b63..e79ccb6d483a3 100644 --- a/bridges/modules/message-lane/src/lib.rs +++ b/bridges/modules/message-lane/src/lib.rs @@ -37,6 +37,7 @@ use bp_message_lane::{ target_chain::{MessageDispatch, SourceHeaderChain}, InboundLaneData, LaneId, MessageData, MessageKey, MessageNonce, OutboundLaneData, }; +use codec::{Decode, Encode}; use frame_support::{ decl_error, decl_event, decl_module, decl_storage, sp_runtime::DispatchResult, traits::Get, weights::Weight, Parameter, StorageMap, @@ -60,41 +61,45 @@ pub trait Trait: frame_system::Trait { /// They overarching event type. type Event: From> + Into<::Event>; - /// Message payload. - type Payload: Parameter; /// Maximal number of messages that may be pruned during maintenance. Maintenance occurs /// whenever outbound lane is updated - i.e. when new message is sent, or receival is /// confirmed. The reason is that if you want to use lane, you should be ready to pay /// for it. type MaxMessagesToPruneAtOnce: Get; + /// Payload type of outbound messages. This payload is dispatched on the bridged chain. + type OutboundPayload: Parameter; + /// Message fee type of outbound messages. This fee is paid on this chain. + type OutboundMessageFee: Parameter; + + /// Payload type of inbound messages. This payload is dispatched on this chain. + type InboundPayload: Decode; + /// Message fee type of inbound messages. This fee is paid on the bridged chain. + type InboundMessageFee: Decode; + // Types that are used by outbound_lane (on source chain). - /// Type of delivery_and_dispatch_fee on source chain. - type MessageFee: Parameter; /// Target header chain. - type TargetHeaderChain: TargetHeaderChain; + type TargetHeaderChain: TargetHeaderChain; /// Message payload verifier. - type LaneMessageVerifier: LaneMessageVerifier; + type LaneMessageVerifier: LaneMessageVerifier; /// Message delivery payment. - type MessageDeliveryAndDispatchPayment: MessageDeliveryAndDispatchPayment; + type MessageDeliveryAndDispatchPayment: MessageDeliveryAndDispatchPayment; // Types that are used by inbound_lane (on target chain). /// Source header chain, as it is represented on target chain. - type SourceHeaderChain: SourceHeaderChain; + type SourceHeaderChain: SourceHeaderChain; /// Message dispatch. - type MessageDispatch: MessageDispatch; + type MessageDispatch: MessageDispatch; } /// Shortcut to messages proof type for Trait. -type MessagesProofOf = <>::SourceHeaderChain as SourceHeaderChain< - >::Payload, - >::MessageFee, ->>::MessagesProof; +type MessagesProofOf = + <>::SourceHeaderChain as SourceHeaderChain<>::InboundMessageFee>>::MessagesProof; /// Shortcut to messages delivery proof type for Trait. type MessagesDeliveryProofOf = - <>::TargetHeaderChain as TargetHeaderChain<>::Payload>>::MessagesDeliveryProof; + <>::TargetHeaderChain as TargetHeaderChain<>::OutboundPayload>>::MessagesDeliveryProof; decl_error! { pub enum Error for Module, I: Instance> { @@ -120,7 +125,7 @@ decl_storage! { /// Map of lane id => outbound lane data. OutboundLanes: map hasher(blake2_128_concat) LaneId => OutboundLaneData; /// All queued outbound messages. - OutboundMessages: map hasher(blake2_128_concat) MessageKey => Option>; + OutboundMessages: map hasher(blake2_128_concat) MessageKey => Option>; } } @@ -147,8 +152,8 @@ decl_module! { pub fn send_message( origin, lane_id: LaneId, - payload: T::Payload, - delivery_and_dispatch_fee: T::MessageFee, + payload: T::OutboundPayload, + delivery_and_dispatch_fee: T::OutboundMessageFee, ) -> DispatchResult { let submitter = ensure_signed(origin)?; @@ -201,7 +206,7 @@ decl_module! { // finally, save message in outbound storage and emit event let mut lane = outbound_lane::(lane_id); let nonce = lane.send_message(MessageData { - payload, + payload: payload.encode(), fee: delivery_and_dispatch_fee, }); lane.prune_messages(T::MaxMessagesToPruneAtOnce::get()); @@ -238,6 +243,9 @@ decl_module! { Error::::InvalidMessagesProof })?; + // try to decode message payloads + let messages: Vec<_> = messages.into_iter().map(Into::into).collect(); + // verify that relayer is paying actual dispatch weight let actual_dispatch_weight: Weight = messages .iter() @@ -329,8 +337,7 @@ struct RuntimeInboundLaneStorage { } impl, I: Instance> InboundLaneStorage for RuntimeInboundLaneStorage { - type Payload = T::Payload; - type MessageFee = T::MessageFee; + type MessageFee = T::InboundMessageFee; fn id(&self) -> LaneId { self.lane_id @@ -352,8 +359,7 @@ struct RuntimeOutboundLaneStorage { } impl, I: Instance> OutboundLaneStorage for RuntimeOutboundLaneStorage { - type Payload = T::Payload; - type MessageFee = T::MessageFee; + type MessageFee = T::OutboundMessageFee; fn id(&self) -> LaneId { self.lane_id @@ -368,14 +374,14 @@ impl, I: Instance> OutboundLaneStorage for RuntimeOutboundLaneStorag } #[cfg(test)] - fn message(&self, nonce: &MessageNonce) -> Option> { + fn message(&self, nonce: &MessageNonce) -> Option> { OutboundMessages::::get(MessageKey { lane_id: self.lane_id, nonce: *nonce, }) } - fn save_message(&mut self, nonce: MessageNonce, mesage_data: MessageData) { + fn save_message(&mut self, nonce: MessageNonce, mesage_data: MessageData) { OutboundMessages::::insert( MessageKey { lane_id: self.lane_id, @@ -397,10 +403,9 @@ impl, I: Instance> OutboundLaneStorage for RuntimeOutboundLaneStorag mod tests { use super::*; use crate::mock::{ - run_test, Origin, TestEvent, TestMessageDeliveryAndDispatchPayment, TestRuntime, + message, run_test, Origin, TestEvent, TestMessageDeliveryAndDispatchPayment, TestRuntime, PAYLOAD_REJECTED_BY_TARGET_CHAIN, REGULAR_PAYLOAD, TEST_LANE_ID, }; - use bp_message_lane::Message; use frame_support::{assert_noop, assert_ok}; use frame_system::{EventRecord, Module as System, Phase}; @@ -503,16 +508,7 @@ mod tests { run_test(|| { assert_ok!(Module::::receive_messages_proof( Origin::signed(1), - Ok(vec![Message { - key: MessageKey { - lane_id: TEST_LANE_ID, - nonce: 1, - }, - data: MessageData { - payload: REGULAR_PAYLOAD, - fee: 0, - }, - }]), + Ok(vec![message(1, REGULAR_PAYLOAD)]), REGULAR_PAYLOAD.1, )); @@ -529,16 +525,7 @@ mod tests { assert_noop!( Module::::receive_messages_proof( Origin::signed(1), - Ok(vec![Message { - key: MessageKey { - lane_id: TEST_LANE_ID, - nonce: 1, - }, - data: MessageData { - payload: REGULAR_PAYLOAD, - fee: 0, - }, - }]), + Ok(vec![message(1, REGULAR_PAYLOAD)]), REGULAR_PAYLOAD.1 - 1, ), Error::::InvalidMessagesDispatchWeight, @@ -578,4 +565,46 @@ mod tests { ); }); } + + #[test] + fn receive_messages_accepts_single_message_with_invalid_payload() { + run_test(|| { + let mut invalid_message = message(1, REGULAR_PAYLOAD); + invalid_message.data.payload = Vec::new(); + + assert_ok!(Module::::receive_messages_proof( + Origin::signed(1), + Ok(vec![invalid_message]), + 0, // weight may be zero in this case (all messages are improperly encoded) + ),); + + assert_eq!( + InboundLanes::::get(&TEST_LANE_ID).latest_received_nonce, + 1, + ); + }); + } + + #[test] + fn receive_messages_accepts_batch_with_message_with_invalid_payload() { + run_test(|| { + let mut invalid_message = message(2, REGULAR_PAYLOAD); + invalid_message.data.payload = Vec::new(); + + assert_ok!(Module::::receive_messages_proof( + Origin::signed(1), + Ok(vec![ + message(1, REGULAR_PAYLOAD), + invalid_message, + message(3, REGULAR_PAYLOAD), + ]), + REGULAR_PAYLOAD.1 + REGULAR_PAYLOAD.1, + ),); + + assert_eq!( + InboundLanes::::get(&TEST_LANE_ID).latest_received_nonce, + 3, + ); + }); + } } diff --git a/bridges/modules/message-lane/src/mock.rs b/bridges/modules/message-lane/src/mock.rs index b8b71c5db19b4..69faeb7cdcc87 100644 --- a/bridges/modules/message-lane/src/mock.rs +++ b/bridges/modules/message-lane/src/mock.rs @@ -18,9 +18,10 @@ use crate::Trait; use bp_message_lane::{ source_chain::{LaneMessageVerifier, MessageDeliveryAndDispatchPayment, TargetHeaderChain}, - target_chain::{MessageDispatch, SourceHeaderChain}, - LaneId, Message, MessageData, MessageNonce, + target_chain::{DispatchMessage, MessageDispatch, SourceHeaderChain}, + LaneId, Message, MessageData, MessageKey, MessageNonce, }; +use codec::Encode; use frame_support::{impl_outer_event, impl_outer_origin, parameter_types, weights::Weight}; use sp_core::H256; use sp_runtime::{ @@ -92,10 +93,14 @@ parameter_types! { impl Trait for TestRuntime { type Event = TestEvent; - type Payload = TestPayload; type MaxMessagesToPruneAtOnce = MaxMessagesToPruneAtOnce; - type MessageFee = TestMessageFee; + type OutboundPayload = TestPayload; + type OutboundMessageFee = TestMessageFee; + + type InboundPayload = TestPayload; + type InboundMessageFee = TestMessageFee; + type TargetHeaderChain = TestTargetHeaderChain; type LaneMessageVerifier = TestLaneMessageVerifier; type MessageDeliveryAndDispatchPayment = TestMessageDeliveryAndDispatchPayment; @@ -194,14 +199,12 @@ impl MessageDeliveryAndDispatchPayment for TestMessag #[derive(Debug)] pub struct TestSourceHeaderChain; -impl SourceHeaderChain for TestSourceHeaderChain { +impl SourceHeaderChain for TestSourceHeaderChain { type Error = &'static str; - type MessagesProof = Result>, ()>; + type MessagesProof = Result>, ()>; - fn verify_messages_proof( - proof: Self::MessagesProof, - ) -> Result>, Self::Error> { + fn verify_messages_proof(proof: Self::MessagesProof) -> Result>, Self::Error> { proof.map_err(|_| TEST_ERROR) } } @@ -210,17 +213,36 @@ impl SourceHeaderChain for TestSourceHeaderChain { #[derive(Debug)] pub struct TestMessageDispatch; -impl MessageDispatch for TestMessageDispatch { - fn dispatch_weight(message: &Message) -> Weight { - message.data.payload.1 +impl MessageDispatch for TestMessageDispatch { + type DispatchPayload = TestPayload; + + fn dispatch_weight(message: &DispatchMessage) -> Weight { + match message.data.payload.as_ref() { + Ok(payload) => payload.1, + Err(_) => 0, + } } - fn dispatch(_message: Message) {} + fn dispatch(_message: DispatchMessage) {} +} + +/// Return test lane message with given nonce and payload. +pub fn message(nonce: MessageNonce, payload: TestPayload) -> Message { + Message { + key: MessageKey { + lane_id: TEST_LANE_ID, + nonce, + }, + data: message_data(payload), + } } /// Return message data with valid fee for given payload. -pub fn message_data(payload: TestPayload) -> MessageData { - MessageData { payload, fee: 1 } +pub fn message_data(payload: TestPayload) -> MessageData { + MessageData { + payload: payload.encode(), + fee: 1, + } } /// Run message lane test. diff --git a/bridges/modules/message-lane/src/outbound_lane.rs b/bridges/modules/message-lane/src/outbound_lane.rs index 8afcd0702869b..dc1ec413012d0 100644 --- a/bridges/modules/message-lane/src/outbound_lane.rs +++ b/bridges/modules/message-lane/src/outbound_lane.rs @@ -20,8 +20,6 @@ use bp_message_lane::{LaneId, MessageData, MessageNonce, OutboundLaneData}; /// Outbound lane storage. pub trait OutboundLaneStorage { - /// Message payload. - type Payload; /// Delivery and dispatch fee type on source chain. type MessageFee; @@ -33,9 +31,9 @@ pub trait OutboundLaneStorage { fn set_data(&mut self, data: OutboundLaneData); /// Returns saved outbound message payload. #[cfg(test)] - fn message(&self, nonce: &MessageNonce) -> Option>; + fn message(&self, nonce: &MessageNonce) -> Option>; /// Save outbound message in the storage. - fn save_message(&mut self, nonce: MessageNonce, message_data: MessageData); + fn save_message(&mut self, nonce: MessageNonce, message_data: MessageData); /// Remove outbound message from the storage. fn remove_message(&mut self, nonce: &MessageNonce); } @@ -54,7 +52,7 @@ impl OutboundLane { /// Send message over lane. /// /// Returns new message nonce. - pub fn send_message(&mut self, message_data: MessageData) -> MessageNonce { + pub fn send_message(&mut self, message_data: MessageData) -> MessageNonce { let mut data = self.storage.data(); let nonce = data.latest_generated_nonce + 1; data.latest_generated_nonce = nonce; diff --git a/bridges/primitives/message-lane/src/lib.rs b/bridges/primitives/message-lane/src/lib.rs index a2e3e41cfd40c..a3216439958b1 100644 --- a/bridges/primitives/message-lane/src/lib.rs +++ b/bridges/primitives/message-lane/src/lib.rs @@ -39,6 +39,9 @@ pub type MessageNonce = u64; /// Message id as a tuple. pub type MessageId = (LaneId, MessageNonce); +/// Opaque message payload. We only decode this payload when it is dispatched. +pub type MessagePayload = Vec; + /// Message key (unique message identifier) as it is stored in the storage. #[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug)] pub struct MessageKey { @@ -50,20 +53,20 @@ pub struct MessageKey { /// Message data as it is stored in the storage. #[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug)] -pub struct MessageData { +pub struct MessageData { /// Message payload. - pub payload: Payload, + pub payload: MessagePayload, /// Message delivery and dispatch fee, paid by the submitter. pub fee: Fee, } /// Message as it is stored in the storage. #[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug)] -pub struct Message { +pub struct Message { /// Message key. pub key: MessageKey, /// Message data. - pub data: MessageData, + pub data: MessageData, } /// Inbound lane data. diff --git a/bridges/primitives/message-lane/src/target_chain.rs b/bridges/primitives/message-lane/src/target_chain.rs index 7287b26ae5713..8bc71ee830066 100644 --- a/bridges/primitives/message-lane/src/target_chain.rs +++ b/bridges/primitives/message-lane/src/target_chain.rs @@ -16,17 +16,36 @@ //! Primitives of message lane module, that are used on the target chain. -use crate::Message; +use crate::{Message, MessageData, MessageKey}; -use frame_support::{weights::Weight, Parameter}; +use codec::{Decode, Error as CodecError}; +use frame_support::{weights::Weight, Parameter, RuntimeDebug}; use sp_std::{fmt::Debug, prelude::*}; +/// Message data with decoded dispatch payload. +#[derive(RuntimeDebug)] +pub struct DispatchMessageData { + /// Result of dispatch payload decoding. + pub payload: Result, + /// Message delivery and dispatch fee, paid by the submitter. + pub fee: Fee, +} + +/// Message with decoded dispatch payload. +#[derive(RuntimeDebug)] +pub struct DispatchMessage { + /// Message key. + pub key: MessageKey, + /// Message data with decoded dispatch payload. + pub data: DispatchMessageData, +} + /// Source chain API. Used by target chain, to verify source chain proofs. /// /// All implementations of this trait should only work with finalized data that /// can't change. Wrong implementation may lead to invalid lane states (i.e. lane /// that's stuck) and/or processing messages without paying fees. -pub trait SourceHeaderChain { +pub trait SourceHeaderChain { /// Error type. type Error: Debug + Into<&'static str>; @@ -37,20 +56,43 @@ pub trait SourceHeaderChain { /// /// Messages vector is required to be sorted by nonce within each lane. Out-of-order /// messages will be rejected. - fn verify_messages_proof(proof: Self::MessagesProof) -> Result>, Self::Error>; + fn verify_messages_proof(proof: Self::MessagesProof) -> Result>, Self::Error>; } /// Called when inbound message is received. -pub trait MessageDispatch { +pub trait MessageDispatch { + /// Decoded message payload type. Valid message may contain invalid payload. In this case + /// message is delivered, but dispatch fails. Therefore, two separate types of payload + /// (opaque `MessagePayload` used in delivery and this `DispatchPayload` used in dispatch). + type DispatchPayload: Decode; + /// Estimate dispatch weight. /// /// This function must: (1) be instant and (2) return correct upper bound /// of dispatch weight. - fn dispatch_weight(message: &Message) -> Weight; + fn dispatch_weight(message: &DispatchMessage) -> Weight; /// Called when inbound message is received. /// /// It is up to the implementers of this trait to determine whether the message /// is invalid (i.e. improperly encoded, has too large weight, ...) or not. - fn dispatch(message: Message); + fn dispatch(message: DispatchMessage); +} + +impl From> for DispatchMessage { + fn from(message: Message) -> Self { + DispatchMessage { + key: message.key, + data: message.data.into(), + } + } +} + +impl From> for DispatchMessageData { + fn from(data: MessageData) -> Self { + DispatchMessageData { + payload: DispatchPayload::decode(&mut &data.payload[..]), + fee: data.fee, + } + } }