From 07479e7a78452026d2781c291779acdee4ae8fb0 Mon Sep 17 00:00:00 2001
From: Serban Iorga <serban@parity.io>
Date: Thu, 18 Jan 2024 10:20:18 +0100
Subject: [PATCH] [`polkadot-staging`] Separate `send_message()` validations
(#2795)
* Fix xcm-bridge-hub build
* Move send_message definition to MessagesBridge impl
* Make send_message infallible
* Split `MessagesBridge` into 2 phases
Split `MessagesBridge` into 2 phases:
- message validation
- message sending
---
bridges/modules/messages/src/benchmarking.rs | 4 +-
bridges/modules/messages/src/lib.rs | 185 +++++++++---------
bridges/modules/messages/src/mock.rs | 8 +-
bridges/modules/messages/src/outbound_lane.rs | 58 +++---
.../modules/xcm-bridge-hub/src/exporter.rs | 71 ++++---
bridges/modules/xcm-bridge-hub/src/mock.rs | 22 +--
.../primitives/messages/src/source_chain.rs | 26 +--
bridges/primitives/runtime/src/lib.rs | 2 +-
8 files changed, 180 insertions(+), 196 deletions(-)
diff --git a/bridges/modules/messages/src/benchmarking.rs b/bridges/modules/messages/src/benchmarking.rs
index 8c4e6fbf00c..4f13c440967 100644
--- a/bridges/modules/messages/src/benchmarking.rs
+++ b/bridges/modules/messages/src/benchmarking.rs
@@ -31,7 +31,7 @@ use codec::Decode;
use frame_benchmarking::{account, benchmarks_instance_pallet};
use frame_support::weights::Weight;
use frame_system::RawOrigin;
-use sp_runtime::traits::TrailingZeroInput;
+use sp_runtime::{traits::TrailingZeroInput, BoundedVec};
use sp_std::{ops::RangeInclusive, prelude::*};
const SEED: u32 = 0;
@@ -443,7 +443,7 @@ benchmarks_instance_pallet! {
fn send_regular_message<T: Config<I>, I: 'static>() {
let mut outbound_lane = outbound_lane::<T, I>(T::bench_lane_id());
- outbound_lane.send_message(vec![]).expect("We craft valid messages");
+ outbound_lane.send_message(BoundedVec::try_from(vec![]).expect("We craft valid messages"));
}
fn receive_messages<T: Config<I>, I: 'static>(nonce: MessageNonce) {
diff --git a/bridges/modules/messages/src/lib.rs b/bridges/modules/messages/src/lib.rs
index 401ec185485..a86cb326cf0 100644
--- a/bridges/modules/messages/src/lib.rs
+++ b/bridges/modules/messages/src/lib.rs
@@ -534,6 +534,7 @@ pub mod pallet {
}
#[pallet::error]
+ #[derive(PartialEq, Eq)]
pub enum Error<T, I = ()> {
/// Pallet is not in Normal operating mode.
NotOperatingNormally,
@@ -543,8 +544,6 @@ pub mod pallet {
MessageDispatchInactive,
/// Message has been treated as invalid by chain verifier.
MessageRejectedByChainVerifier(VerificationError),
- /// Message has been treated as invalid by lane verifier.
- MessageRejectedByLaneVerifier(VerificationError),
/// Message has been treated as invalid by the pallet logic.
MessageRejectedByPallet(VerificationError),
/// Submitter has failed to pay fee for delivering and dispatching messages.
@@ -690,68 +689,72 @@ pub mod pallet {
}
}
+/// Structure, containing a validated message payload and all the info required
+/// to send it on the bridge.
+#[derive(Debug, PartialEq, Eq)]
+pub struct SendMessageArgs<T: Config<I>, I: 'static> {
+ lane_id: LaneId,
+ payload: StoredMessagePayload<T, I>,
+}
+
impl<T, I> bp_messages::source_chain::MessagesBridge<T::OutboundPayload> for Pallet<T, I>
where
T: Config<I>,
I: 'static,
{
- type Error = sp_runtime::DispatchErrorWithPostInfo<PostDispatchInfo>;
+ type Error = Error<T, I>;
+ type SendMessageArgs = SendMessageArgs<T, I>;
- fn send_message(
+ fn validate_message(
lane: LaneId,
- message: T::OutboundPayload,
- ) -> Result<SendMessageArtifacts, Self::Error> {
- crate::send_message::<T, I>(lane, message)
+ message: &T::OutboundPayload,
+ ) -> Result<SendMessageArgs<T, I>, Self::Error> {
+ ensure_normal_operating_mode::<T, I>()?;
+
+ // let's check if outbound lane is active
+ ensure!(T::ActiveOutboundLanes::get().contains(&lane), Error::<T, I>::InactiveOutboundLane);
+
+ // let's first check if message can be delivered to target chain
+ T::TargetHeaderChain::verify_message(message).map_err(|err| {
+ log::trace!(
+ target: LOG_TARGET,
+ "Message to lane {:?} is rejected by target chain: {:?}",
+ lane,
+ err,
+ );
+
+ Error::<T, I>::MessageRejectedByChainVerifier(err)
+ })?;
+
+ Ok(SendMessageArgs {
+ lane_id: lane,
+ payload: StoredMessagePayload::<T, I>::try_from(message.encode()).map_err(|_| {
+ Error::<T, I>::MessageRejectedByPallet(VerificationError::MessageTooLarge)
+ })?,
+ })
}
-}
-/// Function that actually sends message.
-fn send_message<T: Config<I>, I: 'static>(
- lane_id: LaneId,
- payload: T::OutboundPayload,
-) -> sp_std::result::Result<
- SendMessageArtifacts,
- sp_runtime::DispatchErrorWithPostInfo<PostDispatchInfo>,
-> {
- ensure_normal_operating_mode::<T, I>()?;
-
- // let's check if outbound lane is active
- ensure!(T::ActiveOutboundLanes::get().contains(&lane_id), Error::<T, I>::InactiveOutboundLane,);
-
- // let's first check if message can be delivered to target chain
- T::TargetHeaderChain::verify_message(&payload).map_err(|err| {
+ fn send_message(args: SendMessageArgs<T, I>) -> SendMessageArtifacts {
+ // save message in outbound storage and emit event
+ let mut lane = outbound_lane::<T, I>(args.lane_id);
+ let message_len = args.payload.len();
+ let nonce = lane.send_message(args.payload);
+
+ // return number of messages in the queue to let sender know about its state
+ let enqueued_messages = lane.data().queued_messages().saturating_len();
+
log::trace!(
target: LOG_TARGET,
- "Message to lane {:?} is rejected by target chain: {:?}",
- lane_id,
- err,
+ "Accepted message {} to lane {:?}. Message size: {:?}",
+ nonce,
+ args.lane_id,
+ message_len,
);
- Error::<T, I>::MessageRejectedByChainVerifier(err)
- })?;
-
- // finally, save message in outbound storage and emit event
- let mut lane = outbound_lane::<T, I>(lane_id);
- let encoded_payload = payload.encode();
- let encoded_payload_len = encoded_payload.len();
- let nonce = lane
- .send_message(encoded_payload)
- .map_err(Error::<T, I>::MessageRejectedByPallet)?;
-
- // return number of messages in the queue to let sender know about its state
- let enqueued_messages = lane.data().queued_messages().saturating_len();
-
- log::trace!(
- target: LOG_TARGET,
- "Accepted message {} to lane {:?}. Message size: {:?}",
- nonce,
- lane_id,
- encoded_payload_len,
- );
-
- Pallet::<T, I>::deposit_event(Event::MessageAccepted { lane_id, nonce });
+ Pallet::<T, I>::deposit_event(Event::MessageAccepted { lane_id: args.lane_id, nonce });
- Ok(SendMessageArtifacts { nonce, enqueued_messages })
+ SendMessageArtifacts { nonce, enqueued_messages }
+ }
}
/// Ensure that the pallet is in normal operational mode.
@@ -852,6 +855,8 @@ struct RuntimeOutboundLaneStorage<T, I = ()> {
}
impl<T: Config<I>, I: 'static> OutboundLaneStorage for RuntimeOutboundLaneStorage<T, I> {
+ type StoredMessagePayload = StoredMessagePayload<T, I>;
+
fn id(&self) -> LaneId {
self.lane_id
}
@@ -865,22 +870,15 @@ impl<T: Config<I>, I: 'static> OutboundLaneStorage for RuntimeOutboundLaneStorag
}
#[cfg(test)]
- fn message(&self, nonce: &MessageNonce) -> Option<MessagePayload> {
+ fn message(&self, nonce: &MessageNonce) -> Option<Self::StoredMessagePayload> {
OutboundMessages::<T, I>::get(MessageKey { lane_id: self.lane_id, nonce: *nonce })
- .map(Into::into)
}
- fn save_message(
- &mut self,
- nonce: MessageNonce,
- message_payload: MessagePayload,
- ) -> Result<(), VerificationError> {
+ fn save_message(&mut self, nonce: MessageNonce, message_payload: Self::StoredMessagePayload) {
OutboundMessages::<T, I>::insert(
MessageKey { lane_id: self.lane_id, nonce },
- StoredMessagePayload::<T, I>::try_from(message_payload)
- .map_err(|_| VerificationError::MessageTooLarge)?,
+ message_payload,
);
- Ok(())
}
fn remove_message(&mut self, nonce: &MessageNonce) {
@@ -927,7 +925,10 @@ mod tests {
},
outbound_lane::ReceivalConfirmationError,
};
- use bp_messages::{BridgeMessagesCall, UnrewardedRelayer, UnrewardedRelayersState};
+ use bp_messages::{
+ source_chain::MessagesBridge, BridgeMessagesCall, UnrewardedRelayer,
+ UnrewardedRelayersState,
+ };
use bp_test_utils::generate_owned_bridge_module_tests;
use frame_support::{
assert_noop, assert_ok,
@@ -944,14 +945,15 @@ mod tests {
System::<TestRuntime>::reset_events();
}
- fn send_regular_message() {
+ fn send_regular_message(lane_id: LaneId) {
get_ready_for_events();
- let outbound_lane = outbound_lane::<TestRuntime, ()>(TEST_LANE_ID);
+ let outbound_lane = outbound_lane::<TestRuntime, ()>(lane_id);
let message_nonce = outbound_lane.data().latest_generated_nonce + 1;
let prev_enqueud_messages = outbound_lane.data().queued_messages().saturating_len();
- let artifacts = send_message::<TestRuntime, ()>(TEST_LANE_ID, REGULAR_PAYLOAD)
- .expect("send_message has failed");
+ let valid_message = Pallet::<TestRuntime, ()>::validate_message(lane_id, ®ULAR_PAYLOAD)
+ .expect("validate_message has failed");
+ let artifacts = Pallet::<TestRuntime, ()>::send_message(valid_message);
assert_eq!(artifacts.enqueued_messages, prev_enqueud_messages + 1);
// check event with assigned nonce
@@ -960,7 +962,7 @@ mod tests {
vec![EventRecord {
phase: Phase::Initialization,
event: TestEvent::Messages(Event::MessageAccepted {
- lane_id: TEST_LANE_ID,
+ lane_id,
nonce: message_nonce
}),
topics: vec![],
@@ -1011,14 +1013,14 @@ mod tests {
fn pallet_rejects_transactions_if_halted() {
run_test(|| {
// send message first to be able to check that delivery_proof fails later
- send_regular_message();
+ send_regular_message(TEST_LANE_ID);
PalletOperatingMode::<TestRuntime, ()>::put(MessagesOperatingMode::Basic(
BasicOperatingMode::Halted,
));
assert_noop!(
- send_message::<TestRuntime, ()>(TEST_LANE_ID, REGULAR_PAYLOAD,),
+ Pallet::<TestRuntime, ()>::validate_message(TEST_LANE_ID, ®ULAR_PAYLOAD),
Error::<TestRuntime, ()>::NotOperatingNormally,
);
@@ -1061,14 +1063,14 @@ mod tests {
fn pallet_rejects_new_messages_in_rejecting_outbound_messages_operating_mode() {
run_test(|| {
// send message first to be able to check that delivery_proof fails later
- send_regular_message();
+ send_regular_message(TEST_LANE_ID);
PalletOperatingMode::<TestRuntime, ()>::put(
MessagesOperatingMode::RejectingOutboundMessages,
);
assert_noop!(
- send_message::<TestRuntime, ()>(TEST_LANE_ID, REGULAR_PAYLOAD,),
+ Pallet::<TestRuntime, ()>::validate_message(TEST_LANE_ID, ®ULAR_PAYLOAD),
Error::<TestRuntime, ()>::NotOperatingNormally,
);
@@ -1104,7 +1106,7 @@ mod tests {
#[test]
fn send_message_works() {
run_test(|| {
- send_regular_message();
+ send_regular_message(TEST_LANE_ID);
});
}
@@ -1118,7 +1120,7 @@ mod tests {
.extra
.extend_from_slice(&[0u8; MAX_OUTBOUND_PAYLOAD_SIZE as usize]);
assert_noop!(
- send_message::<TestRuntime, ()>(TEST_LANE_ID, message_payload.clone(),),
+ Pallet::<TestRuntime, ()>::validate_message(TEST_LANE_ID, &message_payload.clone(),),
Error::<TestRuntime, ()>::MessageRejectedByPallet(
VerificationError::MessageTooLarge
),
@@ -1129,7 +1131,11 @@ mod tests {
message_payload.extra.pop();
}
assert_eq!(message_payload.encoded_size() as u32, MAX_OUTBOUND_PAYLOAD_SIZE);
- assert_ok!(send_message::<TestRuntime, ()>(TEST_LANE_ID, message_payload,),);
+
+ let valid_message =
+ Pallet::<TestRuntime, ()>::validate_message(TEST_LANE_ID, &message_payload)
+ .expect("validate_message has failed");
+ Pallet::<TestRuntime, ()>::send_message(valid_message);
})
}
@@ -1138,7 +1144,10 @@ mod tests {
run_test(|| {
// messages with this payload are rejected by target chain verifier
assert_noop!(
- send_message::<TestRuntime, ()>(TEST_LANE_ID, PAYLOAD_REJECTED_BY_TARGET_CHAIN,),
+ Pallet::<TestRuntime, ()>::validate_message(
+ TEST_LANE_ID,
+ &PAYLOAD_REJECTED_BY_TARGET_CHAIN,
+ ),
Error::<TestRuntime, ()>::MessageRejectedByChainVerifier(VerificationError::Other(
mock::TEST_ERROR
)),
@@ -1298,7 +1307,7 @@ mod tests {
#[test]
fn receive_messages_delivery_proof_works() {
run_test(|| {
- send_regular_message();
+ send_regular_message(TEST_LANE_ID);
receive_messages_delivery_proof();
assert_eq!(
@@ -1311,8 +1320,8 @@ mod tests {
#[test]
fn receive_messages_delivery_proof_rewards_relayers() {
run_test(|| {
- assert_ok!(send_message::<TestRuntime, ()>(TEST_LANE_ID, REGULAR_PAYLOAD,));
- assert_ok!(send_message::<TestRuntime, ()>(TEST_LANE_ID, REGULAR_PAYLOAD,));
+ send_regular_message(TEST_LANE_ID);
+ send_regular_message(TEST_LANE_ID);
// this reports delivery of message 1 => reward is paid to TEST_RELAYER_A
let single_message_delivery_proof = TestMessagesDeliveryProof(Ok((
@@ -1698,9 +1707,9 @@ mod tests {
#[test]
fn messages_delivered_callbacks_are_called() {
run_test(|| {
- send_regular_message();
- send_regular_message();
- send_regular_message();
+ send_regular_message(TEST_LANE_ID);
+ send_regular_message(TEST_LANE_ID);
+ send_regular_message(TEST_LANE_ID);
// messages 1+2 are confirmed in 1 tx, message 3 in a separate tx
// dispatch of message 2 has failed
@@ -1759,7 +1768,7 @@ mod tests {
) {
run_test(|| {
// send message first to be able to check that delivery_proof fails later
- send_regular_message();
+ send_regular_message(TEST_LANE_ID);
// 1) InboundLaneData declares that the `last_confirmed_nonce` is 1;
// 2) InboundLaneData has no entries => `InboundLaneData::last_delivered_nonce()`
@@ -1826,10 +1835,10 @@ mod tests {
#[test]
fn on_idle_callback_respects_remaining_weight() {
run_test(|| {
- send_regular_message();
- send_regular_message();
- send_regular_message();
- send_regular_message();
+ send_regular_message(TEST_LANE_ID);
+ send_regular_message(TEST_LANE_ID);
+ send_regular_message(TEST_LANE_ID);
+ send_regular_message(TEST_LANE_ID);
assert_ok!(Pallet::<TestRuntime>::receive_messages_delivery_proof(
RuntimeOrigin::signed(1),
@@ -1908,10 +1917,10 @@ mod tests {
fn on_idle_callback_is_rotating_lanes_to_prune() {
run_test(|| {
// send + receive confirmation for lane 1
- send_regular_message();
+ send_regular_message(TEST_LANE_ID);
receive_messages_delivery_proof();
// send + receive confirmation for lane 2
- assert_ok!(send_message::<TestRuntime, ()>(TEST_LANE_ID_2, REGULAR_PAYLOAD,));
+ send_regular_message(TEST_LANE_ID_2);
assert_ok!(Pallet::<TestRuntime>::receive_messages_delivery_proof(
RuntimeOrigin::signed(1),
TestMessagesDeliveryProof(Ok((
@@ -1987,7 +1996,7 @@ mod tests {
fn outbound_message_from_unconfigured_lane_is_rejected() {
run_test(|| {
assert_noop!(
- send_message::<TestRuntime, ()>(TEST_LANE_ID_3, REGULAR_PAYLOAD,),
+ Pallet::<TestRuntime, ()>::validate_message(TEST_LANE_ID_3, ®ULAR_PAYLOAD,),
Error::<TestRuntime, ()>::InactiveOutboundLane,
);
});
diff --git a/bridges/modules/messages/src/mock.rs b/bridges/modules/messages/src/mock.rs
index 303e641ea2d..af921205398 100644
--- a/bridges/modules/messages/src/mock.rs
+++ b/bridges/modules/messages/src/mock.rs
@@ -17,7 +17,7 @@
// From construct_runtime macro
#![allow(clippy::from_over_into)]
-use crate::Config;
+use crate::{Config, StoredMessagePayload};
use bp_messages::{
calc_relayers_rewards,
@@ -26,7 +26,7 @@ use bp_messages::{
DeliveryPayments, DispatchMessage, DispatchMessageData, MessageDispatch,
ProvedLaneMessages, ProvedMessages, SourceHeaderChain,
},
- DeliveredMessages, InboundLaneData, LaneId, Message, MessageKey, MessageNonce, MessagePayload,
+ DeliveredMessages, InboundLaneData, LaneId, Message, MessageKey, MessageNonce,
UnrewardedRelayer, UnrewardedRelayersState, VerificationError,
};
use bp_runtime::{messages::MessageDispatchResult, Size};
@@ -402,8 +402,8 @@ pub fn message(nonce: MessageNonce, payload: TestPayload) -> Message {
}
/// Return valid outbound message data, constructed from given payload.
-pub fn outbound_message_data(payload: TestPayload) -> MessagePayload {
- payload.encode()
+pub fn outbound_message_data(payload: TestPayload) -> StoredMessagePayload<TestRuntime, ()> {
+ StoredMessagePayload::<TestRuntime, ()>::try_from(payload.encode()).expect("payload too large")
}
/// Return valid inbound (dispatch) message data, constructed from given payload.
diff --git a/bridges/modules/messages/src/outbound_lane.rs b/bridges/modules/messages/src/outbound_lane.rs
index f92e9ccfd95..431c2cfb7ee 100644
--- a/bridges/modules/messages/src/outbound_lane.rs
+++ b/bridges/modules/messages/src/outbound_lane.rs
@@ -18,10 +18,7 @@
use crate::{Config, LOG_TARGET};
-use bp_messages::{
- DeliveredMessages, LaneId, MessageNonce, MessagePayload, OutboundLaneData, UnrewardedRelayer,
- VerificationError,
-};
+use bp_messages::{DeliveredMessages, LaneId, MessageNonce, OutboundLaneData, UnrewardedRelayer};
use codec::{Decode, Encode};
use frame_support::{
weights::{RuntimeDbWeight, Weight},
@@ -34,6 +31,8 @@ use sp_std::collections::vec_deque::VecDeque;
/// Outbound lane storage.
pub trait OutboundLaneStorage {
+ type StoredMessagePayload;
+
/// Lane id.
fn id(&self) -> LaneId;
/// Get lane data from the storage.
@@ -42,13 +41,9 @@ pub trait OutboundLaneStorage {
fn set_data(&mut self, data: OutboundLaneData);
/// Returns saved outbound message payload.
#[cfg(test)]
- fn message(&self, nonce: &MessageNonce) -> Option<MessagePayload>;
+ fn message(&self, nonce: &MessageNonce) -> Option<Self::StoredMessagePayload>;
/// Save outbound message in the storage.
- fn save_message(
- &mut self,
- nonce: MessageNonce,
- message_payload: MessagePayload,
- ) -> Result<(), VerificationError>;
+ fn save_message(&mut self, nonce: MessageNonce, message_payload: Self::StoredMessagePayload);
/// Remove outbound message from the storage.
fn remove_message(&mut self, nonce: &MessageNonce);
}
@@ -91,18 +86,15 @@ impl<S: OutboundLaneStorage> OutboundLane<S> {
/// Send message over lane.
///
/// Returns new message nonce.
- pub fn send_message(
- &mut self,
- message_payload: MessagePayload,
- ) -> Result<MessageNonce, VerificationError> {
+ pub fn send_message(&mut self, message_payload: S::StoredMessagePayload) -> MessageNonce {
let mut data = self.storage.data();
let nonce = data.latest_generated_nonce + 1;
data.latest_generated_nonce = nonce;
- self.storage.save_message(nonce, message_payload)?;
+ self.storage.save_message(nonce, message_payload);
self.storage.set_data(data);
- Ok(nonce)
+ nonce
}
/// Confirm messages delivery.
@@ -218,7 +210,7 @@ mod tests {
},
outbound_lane,
};
- use frame_support::{assert_ok, weights::constants::RocksDbWeight};
+ use frame_support::weights::constants::RocksDbWeight;
use sp_std::ops::RangeInclusive;
fn unrewarded_relayers(
@@ -239,9 +231,9 @@ mod tests {
) -> Result<Option<DeliveredMessages>, ReceivalConfirmationError> {
run_test(|| {
let mut lane = outbound_lane::<TestRuntime, _>(TEST_LANE_ID);
- assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
- assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
- assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
+ lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
+ lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
+ lane.send_message(outbound_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(3, latest_received_nonce, relayers);
@@ -256,7 +248,7 @@ mod tests {
run_test(|| {
let mut lane = outbound_lane::<TestRuntime, _>(TEST_LANE_ID);
assert_eq!(lane.storage.data().latest_generated_nonce, 0);
- assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), Ok(1));
+ assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), 1);
assert!(lane.storage.message(&1).is_some());
assert_eq!(lane.storage.data().latest_generated_nonce, 1);
});
@@ -266,9 +258,9 @@ mod tests {
fn confirm_delivery_works() {
run_test(|| {
let mut lane = outbound_lane::<TestRuntime, _>(TEST_LANE_ID);
- assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), Ok(1));
- assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), Ok(2));
- assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), Ok(3));
+ assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), 1);
+ assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), 2);
+ assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), 3);
assert_eq!(lane.storage.data().latest_generated_nonce, 3);
assert_eq!(lane.storage.data().latest_received_nonce, 0);
assert_eq!(
@@ -284,9 +276,9 @@ mod tests {
fn confirm_delivery_rejects_nonce_lesser_than_latest_received() {
run_test(|| {
let mut lane = outbound_lane::<TestRuntime, _>(TEST_LANE_ID);
- assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
- assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
- assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
+ lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
+ lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
+ lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
assert_eq!(lane.storage.data().latest_generated_nonce, 3);
assert_eq!(lane.storage.data().latest_received_nonce, 0);
assert_eq!(
@@ -368,9 +360,9 @@ mod tests {
);
assert_eq!(lane.storage.data().oldest_unpruned_nonce, 1);
// when nothing is confirmed, nothing is pruned
- assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
- assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
- assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
+ lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
+ lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
+ lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
assert!(lane.storage.message(&1).is_some());
assert!(lane.storage.message(&2).is_some());
assert!(lane.storage.message(&3).is_some());
@@ -412,9 +404,9 @@ mod tests {
fn confirm_delivery_detects_when_more_than_expected_messages_are_confirmed() {
run_test(|| {
let mut lane = outbound_lane::<TestRuntime, _>(TEST_LANE_ID);
- assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
- assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
- assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
+ lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
+ lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
+ lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
assert_eq!(
lane.confirm_delivery(0, 3, &unrewarded_relayers(1..=3)),
Err(ReceivalConfirmationError::TryingToConfirmMoreMessagesThanExpected),
diff --git a/bridges/modules/xcm-bridge-hub/src/exporter.rs b/bridges/modules/xcm-bridge-hub/src/exporter.rs
index 5318b222c54..4e899dfa011 100644
--- a/bridges/modules/xcm-bridge-hub/src/exporter.rs
+++ b/bridges/modules/xcm-bridge-hub/src/exporter.rs
@@ -42,12 +42,13 @@ type MessagesPallet<T, I> = BridgeMessagesPallet<T, <T as Config<I>>::BridgeMess
impl<T: Config<I>, I: 'static> ExportXcm for Pallet<T, I>
where
- T: BridgeMessagesConfig<
- <T as Config<I>>::BridgeMessagesPalletInstance,
- OutboundPayload = XcmAsPlainPayload,
- >,
+ T: BridgeMessagesConfig<T::BridgeMessagesPalletInstance, OutboundPayload = XcmAsPlainPayload>,
{
- type Ticket = (SenderAndLane, XcmAsPlainPayload, XcmHash);
+ type Ticket = (
+ SenderAndLane,
+ <MessagesPallet<T, I> as MessagesBridge<T::OutboundPayload>>::SendMessageArgs,
+ XcmHash,
+ );
fn validate(
network: NetworkId,
@@ -74,42 +75,38 @@ where
message,
)?;
- Ok(((sender_and_lane, blob, id), price))
- }
-
- fn deliver(
- (sender_and_lane, blob, id): (SenderAndLane, XcmAsPlainPayload, XcmHash),
- ) -> Result<XcmHash, SendError> {
- let lane_id = sender_and_lane.lane;
- let send_result = MessagesPallet::<T, I>::send_message(lane_id, blob);
-
- match send_result {
- Ok(artifacts) => {
- log::info!(
- target: LOG_TARGET,
- "XCM message {:?} has been enqueued at bridge {:?} with nonce {}",
- id,
- lane_id,
- artifacts.nonce,
- );
-
- // notify XCM queue manager about updated lane state
- LocalXcmQueueManager::<T::LanesSupport>::on_bridge_message_enqueued(
- &sender_and_lane,
- artifacts.enqueued_messages,
- );
- },
- Err(error) => {
+ let bridge_message = MessagesPallet::<T, I>::validate_message(sender_and_lane.lane, &blob)
+ .map_err(|e| {
log::debug!(
target: LOG_TARGET,
- "XCM message {:?} has been dropped because of bridge error {:?} on bridge {:?}",
+ "XCM message {:?} cannot be exported because of bridge error {:?} on bridge {:?}",
id,
- error,
- lane_id,
+ e,
+ sender_and_lane.lane,
);
- return Err(SendError::Transport("BridgeSendError"))
- },
- }
+ SendError::Transport("BridgeValidateError")
+ })?;
+
+ Ok(((sender_and_lane, bridge_message, id), price))
+ }
+
+ fn deliver((sender_and_lane, bridge_message, id): Self::Ticket) -> Result<XcmHash, SendError> {
+ let lane_id = sender_and_lane.lane;
+ let artifacts = MessagesPallet::<T, I>::send_message(bridge_message);
+
+ log::info!(
+ target: LOG_TARGET,
+ "XCM message {:?} has been enqueued at bridge {:?} with nonce {}",
+ id,
+ lane_id,
+ artifacts.nonce,
+ );
+
+ // notify XCM queue manager about updated lane state
+ LocalXcmQueueManager::<T::LanesSupport>::on_bridge_message_enqueued(
+ &sender_and_lane,
+ artifacts.enqueued_messages,
+ );
Ok(id)
}
diff --git a/bridges/modules/xcm-bridge-hub/src/mock.rs b/bridges/modules/xcm-bridge-hub/src/mock.rs
index 8edd4b1f7aa..97fc7741e40 100644
--- a/bridges/modules/xcm-bridge-hub/src/mock.rs
+++ b/bridges/modules/xcm-bridge-hub/src/mock.rs
@@ -19,11 +19,10 @@
use crate as pallet_xcm_bridge_hub;
use bp_messages::{
- source_chain::LaneMessageVerifier,
target_chain::{DispatchMessage, MessageDispatch},
- LaneId, OutboundLaneData, VerificationError,
+ LaneId,
};
-use bp_runtime::{messages::MessageDispatchResult, Chain, UnderlyingChainProvider};
+use bp_runtime::{messages::MessageDispatchResult, Chain, ChainId, UnderlyingChainProvider};
use bridge_runtime_common::{
messages::{
source::TargetHeaderChainAdapter, target::SourceHeaderChainAdapter,
@@ -78,20 +77,6 @@ impl pallet_balances::Config for TestRuntime {
type AccountStore = System;
}
-/// Lane message verifier that is used in tests.
-#[derive(Debug, Default)]
-pub struct TestLaneMessageVerifier;
-
-impl LaneMessageVerifier<Vec<u8>> for TestLaneMessageVerifier {
- fn verify_message(
- _lane: &LaneId,
- _lane_outbound_data: &OutboundLaneData,
- _payload: &Vec<u8>,
- ) -> Result<(), VerificationError> {
- Ok(())
- }
-}
-
parameter_types! {
pub const ActiveOutboundLanes: &'static [LaneId] = &[TEST_LANE_ID];
}
@@ -110,7 +95,6 @@ impl pallet_bridge_messages::Config for TestRuntime {
type InboundRelayer = ();
type DeliveryPayments = ();
type TargetHeaderChain = TargetHeaderChainAdapter<OnThisChainBridge>;
- type LaneMessageVerifier = TestLaneMessageVerifier;
type DeliveryConfirmationPayments = ();
type OnMessagesDelivered = ();
type SourceHeaderChain = SourceHeaderChainAdapter<OnThisChainBridge>;
@@ -220,6 +204,7 @@ impl XcmBlobHauler for TestXcmBlobHauler {
pub struct ThisChain;
impl Chain for ThisChain {
+ const ID: ChainId = *b"tuch";
type BlockNumber = u64;
type Hash = H256;
type Hasher = BlakeTwo256;
@@ -243,6 +228,7 @@ pub type BridgedHeaderHash = H256;
pub type BridgedChainHeader = SubstrateHeader;
impl Chain for BridgedChain {
+ const ID: ChainId = *b"tuch";
type BlockNumber = u64;
type Hash = BridgedHeaderHash;
type Hasher = BlakeTwo256;
diff --git a/bridges/primitives/messages/src/source_chain.rs b/bridges/primitives/messages/src/source_chain.rs
index 56bcf0bb4bc..f4aefd97355 100644
--- a/bridges/primitives/messages/src/source_chain.rs
+++ b/bridges/primitives/messages/src/source_chain.rs
@@ -125,22 +125,22 @@ pub trait MessagesBridge<Payload> {
/// Error type.
type Error: Debug;
- /// Send message over the bridge.
+ /// Intermediary structure returned by `validate_message()`.
///
- /// Returns unique message nonce or error if send has failed.
- fn send_message(lane: LaneId, message: Payload) -> Result<SendMessageArtifacts, Self::Error>;
-}
-
-/// Bridge that does nothing when message is being sent.
-#[derive(Eq, RuntimeDebug, PartialEq)]
-pub struct NoopMessagesBridge;
+ /// It can than be passed to `send_message()` in order to actually send the message
+ /// on the bridge.
+ type SendMessageArgs;
-impl<Payload> MessagesBridge<Payload> for NoopMessagesBridge {
- type Error = &'static str;
+ /// Check if the message can be sent over the bridge.
+ fn validate_message(
+ lane: LaneId,
+ message: &Payload,
+ ) -> Result<Self::SendMessageArgs, Self::Error>;
- fn send_message(_lane: LaneId, _message: Payload) -> Result<SendMessageArtifacts, Self::Error> {
- Ok(SendMessageArtifacts { nonce: 0, enqueued_messages: 0 })
- }
+ /// Send message over the bridge.
+ ///
+ /// Returns unique message nonce or error if send has failed.
+ fn send_message(message: Self::SendMessageArgs) -> SendMessageArtifacts;
}
/// Structure that may be used in place of `TargetHeaderChain` and
diff --git a/bridges/primitives/runtime/src/lib.rs b/bridges/primitives/runtime/src/lib.rs
index e3bdc53c515..850318923dc 100644
--- a/bridges/primitives/runtime/src/lib.rs
+++ b/bridges/primitives/runtime/src/lib.rs
@@ -313,7 +313,7 @@ pub trait StorageDoubleMapKeyProvider {
}
/// Error generated by the `OwnedBridgeModule` trait.
-#[derive(Encode, Decode, TypeInfo, PalletError)]
+#[derive(Encode, Decode, PartialEq, Eq, TypeInfo, PalletError)]
pub enum OwnedBridgeModuleError {
/// All pallet operations are halted.
Halted,
--
GitLab