Skip to content

Commit

Permalink
Split payload types (paritytech#383)
Browse files Browse the repository at this point in the history
* split payloads (inbound/outbound/opaque/dispatch) + fees (inbound/outbound)

* added tests

Co-authored-by: Hernando Castano <castano.ha@gmail.com>
  • Loading branch information
2 people authored and serban300 committed Apr 10, 2024
1 parent 5ffb526 commit 620694d
Show file tree
Hide file tree
Showing 6 changed files with 178 additions and 85 deletions.
15 changes: 7 additions & 8 deletions bridges/modules/message-lane/src/inbound_lane.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -47,10 +46,10 @@ impl<S: InboundLaneStorage> InboundLane<S> {
}

/// Receive new message.
pub fn receive_message<P: MessageDispatch<S::Payload, S::MessageFee>>(
pub fn receive_message<P: MessageDispatch<S::MessageFee>>(
&mut self,
nonce: MessageNonce,
message_data: MessageData<S::Payload, S::MessageFee>,
message_data: DispatchMessageData<P::DispatchPayload, S::MessageFee>,
) -> bool {
let mut data = self.storage.data();
let is_correct_message = nonce == data.latest_received_nonce + 1;
Expand All @@ -61,7 +60,7 @@ impl<S: InboundLaneStorage> InboundLane<S> {
data.latest_received_nonce = nonce;
self.storage.set_data(data);

P::dispatch(Message {
P::dispatch(DispatchMessage {
key: MessageKey {
lane_id: self.storage.id(),
nonce,
Expand All @@ -85,7 +84,7 @@ mod tests {
fn fails_to_receive_message_with_incorrect_nonce() {
run_test(|| {
let mut lane = inbound_lane::<TestRuntime, _>(TEST_LANE_ID);
assert!(!lane.receive_message::<TestMessageDispatch>(10, message_data(REGULAR_PAYLOAD)));
assert!(!lane.receive_message::<TestMessageDispatch>(10, message_data(REGULAR_PAYLOAD).into()));
assert_eq!(lane.storage.data().latest_received_nonce, 0);
});
}
Expand All @@ -94,7 +93,7 @@ mod tests {
fn correct_message_is_processed_instantly() {
run_test(|| {
let mut lane = inbound_lane::<TestRuntime, _>(TEST_LANE_ID);
assert!(lane.receive_message::<TestMessageDispatch>(1, message_data(REGULAR_PAYLOAD)));
assert!(lane.receive_message::<TestMessageDispatch>(1, message_data(REGULAR_PAYLOAD).into()));
assert_eq!(lane.storage.data().latest_received_nonce, 1);
});
}
Expand Down
121 changes: 75 additions & 46 deletions bridges/modules/message-lane/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -60,41 +61,45 @@ pub trait Trait<I = DefaultInstance>: frame_system::Trait {

/// They overarching event type.
type Event: From<Event<Self, I>> + Into<<Self as frame_system::Trait>::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<MessageNonce>;

/// 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<Self::Payload>;
type TargetHeaderChain: TargetHeaderChain<Self::OutboundPayload>;
/// Message payload verifier.
type LaneMessageVerifier: LaneMessageVerifier<Self::AccountId, Self::Payload, Self::MessageFee>;
type LaneMessageVerifier: LaneMessageVerifier<Self::AccountId, Self::OutboundPayload, Self::OutboundMessageFee>;
/// Message delivery payment.
type MessageDeliveryAndDispatchPayment: MessageDeliveryAndDispatchPayment<Self::AccountId, Self::MessageFee>;
type MessageDeliveryAndDispatchPayment: MessageDeliveryAndDispatchPayment<Self::AccountId, Self::OutboundMessageFee>;

// Types that are used by inbound_lane (on target chain).

/// Source header chain, as it is represented on target chain.
type SourceHeaderChain: SourceHeaderChain<Self::Payload, Self::MessageFee>;
type SourceHeaderChain: SourceHeaderChain<Self::InboundMessageFee>;
/// Message dispatch.
type MessageDispatch: MessageDispatch<Self::Payload, Self::MessageFee>;
type MessageDispatch: MessageDispatch<Self::InboundMessageFee, DispatchPayload = Self::InboundPayload>;
}

/// Shortcut to messages proof type for Trait.
type MessagesProofOf<T, I> = <<T as Trait<I>>::SourceHeaderChain as SourceHeaderChain<
<T as Trait<I>>::Payload,
<T as Trait<I>>::MessageFee,
>>::MessagesProof;
type MessagesProofOf<T, I> =
<<T as Trait<I>>::SourceHeaderChain as SourceHeaderChain<<T as Trait<I>>::InboundMessageFee>>::MessagesProof;
/// Shortcut to messages delivery proof type for Trait.
type MessagesDeliveryProofOf<T, I> =
<<T as Trait<I>>::TargetHeaderChain as TargetHeaderChain<<T as Trait<I>>::Payload>>::MessagesDeliveryProof;
<<T as Trait<I>>::TargetHeaderChain as TargetHeaderChain<<T as Trait<I>>::OutboundPayload>>::MessagesDeliveryProof;

decl_error! {
pub enum Error for Module<T: Trait<I>, I: Instance> {
Expand All @@ -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<MessageData<T::Payload, T::MessageFee>>;
OutboundMessages: map hasher(blake2_128_concat) MessageKey => Option<MessageData<T::OutboundMessageFee>>;
}
}

Expand All @@ -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)?;

Expand Down Expand Up @@ -201,7 +206,7 @@ decl_module! {
// finally, save message in outbound storage and emit event
let mut lane = outbound_lane::<T, I>(lane_id);
let nonce = lane.send_message(MessageData {
payload,
payload: payload.encode(),
fee: delivery_and_dispatch_fee,
});
lane.prune_messages(T::MaxMessagesToPruneAtOnce::get());
Expand Down Expand Up @@ -238,6 +243,9 @@ decl_module! {
Error::<T, I>::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()
Expand Down Expand Up @@ -329,8 +337,7 @@ struct RuntimeInboundLaneStorage<T, I = DefaultInstance> {
}

impl<T: Trait<I>, I: Instance> InboundLaneStorage for RuntimeInboundLaneStorage<T, I> {
type Payload = T::Payload;
type MessageFee = T::MessageFee;
type MessageFee = T::InboundMessageFee;

fn id(&self) -> LaneId {
self.lane_id
Expand All @@ -352,8 +359,7 @@ struct RuntimeOutboundLaneStorage<T, I = DefaultInstance> {
}

impl<T: Trait<I>, I: Instance> OutboundLaneStorage for RuntimeOutboundLaneStorage<T, I> {
type Payload = T::Payload;
type MessageFee = T::MessageFee;
type MessageFee = T::OutboundMessageFee;

fn id(&self) -> LaneId {
self.lane_id
Expand All @@ -368,14 +374,14 @@ impl<T: Trait<I>, I: Instance> OutboundLaneStorage for RuntimeOutboundLaneStorag
}

#[cfg(test)]
fn message(&self, nonce: &MessageNonce) -> Option<MessageData<T::Payload, T::MessageFee>> {
fn message(&self, nonce: &MessageNonce) -> Option<MessageData<T::OutboundMessageFee>> {
OutboundMessages::<T, I>::get(MessageKey {
lane_id: self.lane_id,
nonce: *nonce,
})
}

fn save_message(&mut self, nonce: MessageNonce, mesage_data: MessageData<T::Payload, T::MessageFee>) {
fn save_message(&mut self, nonce: MessageNonce, mesage_data: MessageData<T::OutboundMessageFee>) {
OutboundMessages::<T, I>::insert(
MessageKey {
lane_id: self.lane_id,
Expand All @@ -397,10 +403,9 @@ impl<T: Trait<I>, 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};

Expand Down Expand Up @@ -503,16 +508,7 @@ mod tests {
run_test(|| {
assert_ok!(Module::<TestRuntime>::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,
));

Expand All @@ -529,16 +525,7 @@ mod tests {
assert_noop!(
Module::<TestRuntime>::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::<TestRuntime, DefaultInstance>::InvalidMessagesDispatchWeight,
Expand Down Expand Up @@ -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::<TestRuntime, DefaultInstance>::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::<DefaultInstance>::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::<TestRuntime, DefaultInstance>::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::<DefaultInstance>::get(&TEST_LANE_ID).latest_received_nonce,
3,
);
});
}
}
52 changes: 37 additions & 15 deletions bridges/modules/message-lane/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -194,14 +199,12 @@ impl MessageDeliveryAndDispatchPayment<AccountId, TestMessageFee> for TestMessag
#[derive(Debug)]
pub struct TestSourceHeaderChain;

impl SourceHeaderChain<TestPayload, TestMessageFee> for TestSourceHeaderChain {
impl SourceHeaderChain<TestMessageFee> for TestSourceHeaderChain {
type Error = &'static str;

type MessagesProof = Result<Vec<Message<TestPayload, TestMessageFee>>, ()>;
type MessagesProof = Result<Vec<Message<TestMessageFee>>, ()>;

fn verify_messages_proof(
proof: Self::MessagesProof,
) -> Result<Vec<Message<TestPayload, TestMessageFee>>, Self::Error> {
fn verify_messages_proof(proof: Self::MessagesProof) -> Result<Vec<Message<TestMessageFee>>, Self::Error> {
proof.map_err(|_| TEST_ERROR)
}
}
Expand All @@ -210,17 +213,36 @@ impl SourceHeaderChain<TestPayload, TestMessageFee> for TestSourceHeaderChain {
#[derive(Debug)]
pub struct TestMessageDispatch;

impl MessageDispatch<TestPayload, TestMessageFee> for TestMessageDispatch {
fn dispatch_weight(message: &Message<TestPayload, TestMessageFee>) -> Weight {
message.data.payload.1
impl MessageDispatch<TestMessageFee> for TestMessageDispatch {
type DispatchPayload = TestPayload;

fn dispatch_weight(message: &DispatchMessage<TestPayload, TestMessageFee>) -> Weight {
match message.data.payload.as_ref() {
Ok(payload) => payload.1,
Err(_) => 0,
}
}

fn dispatch(_message: Message<TestPayload, TestMessageFee>) {}
fn dispatch(_message: DispatchMessage<TestPayload, TestMessageFee>) {}
}

/// Return test lane message with given nonce and payload.
pub fn message(nonce: MessageNonce, payload: TestPayload) -> Message<TestMessageFee> {
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<TestPayload, TestMessageFee> {
MessageData { payload, fee: 1 }
pub fn message_data(payload: TestPayload) -> MessageData<TestMessageFee> {
MessageData {
payload: payload.encode(),
fee: 1,
}
}

/// Run message lane test.
Expand Down
Loading

0 comments on commit 620694d

Please sign in to comment.