From 3b968a2aba86b64988e565889ab57cf7961fd742 Mon Sep 17 00:00:00 2001
From: Serban Iorga <serban@parity.io>
Date: Mon, 10 Apr 2023 10:39:24 +0300
Subject: [PATCH] Reintroduce msg dispatch status reporting (#2027)
* Use an actual Result inside MessageDispatchResult
We need this in order to distinguish between Ok and Err
* Revert #1660
* Fixes + simplifications
* Implement review suggestions
---
bridges/bin/millau/runtime/src/xcm_config.rs | 10 +-
.../bin/rialto-parachain/runtime/src/lib.rs | 6 +-
bridges/bin/rialto/runtime/src/xcm_config.rs | 6 +-
bridges/bin/runtime-common/src/integrity.rs | 6 +-
.../src/messages_xcm_extension.rs | 18 +--
bridges/modules/messages/Cargo.toml | 1 +
bridges/modules/messages/src/benchmarking.rs | 12 +-
bridges/modules/messages/src/inbound_lane.rs | 27 +++--
bridges/modules/messages/src/lib.rs | 111 ++++++++++-------
bridges/modules/messages/src/mock.rs | 24 ++--
bridges/modules/messages/src/outbound_lane.rs | 88 +++++++++++---
bridges/primitives/messages/Cargo.toml | 2 +
bridges/primitives/messages/src/lib.rs | 113 +++++++++++++++---
.../primitives/messages/src/target_chain.rs | 10 +-
bridges/primitives/runtime/src/messages.rs | 4 +-
15 files changed, 307 insertions(+), 131 deletions(-)
diff --git a/bridges/bin/millau/runtime/src/xcm_config.rs b/bridges/bin/millau/runtime/src/xcm_config.rs
index 4aaec83771b..56546f29992 100644
--- a/bridges/bin/millau/runtime/src/xcm_config.rs
+++ b/bridges/bin/millau/runtime/src/xcm_config.rs
@@ -242,7 +242,7 @@ mod tests {
target_chain::{DispatchMessage, DispatchMessageData, MessageDispatch},
LaneId, MessageKey,
};
- use bridge_runtime_common::messages_xcm_extension::XcmBlobMessageDispatchResult;
+ use bridge_runtime_common::messages_xcm_extension::XcmBlobMessageDispatchError;
use codec::Encode;
use pallet_bridge_messages::OutboundLanes;
use xcm_executor::XcmExecutor;
@@ -352,8 +352,8 @@ mod tests {
let dispatch_result =
FromRialtoMessageDispatch::dispatch(&AccountId::from([0u8; 32]), incoming_message);
assert!(matches!(
- dispatch_result.dispatch_level_result,
- XcmBlobMessageDispatchResult::NotDispatched(_),
+ dispatch_result.dispatch_result,
+ Err(XcmBlobMessageDispatchError::NotDispatched(_)),
));
}
@@ -366,8 +366,8 @@ mod tests {
let dispatch_result =
FromRialtoMessageDispatch::dispatch(&AccountId::from([0u8; 32]), incoming_message);
assert!(matches!(
- dispatch_result.dispatch_level_result,
- XcmBlobMessageDispatchResult::NotDispatched(_),
+ dispatch_result.dispatch_result,
+ Err(XcmBlobMessageDispatchError::NotDispatched(_)),
));
}
}
diff --git a/bridges/bin/rialto-parachain/runtime/src/lib.rs b/bridges/bin/rialto-parachain/runtime/src/lib.rs
index cd4e256f420..b03f7965d7b 100644
--- a/bridges/bin/rialto-parachain/runtime/src/lib.rs
+++ b/bridges/bin/rialto-parachain/runtime/src/lib.rs
@@ -853,7 +853,7 @@ mod tests {
LaneId, MessageKey,
};
use bridge_runtime_common::{
- integrity::check_additional_signed, messages_xcm_extension::XcmBlobMessageDispatchResult,
+ integrity::check_additional_signed, messages_xcm_extension::XcmBlobMessageDispatchError,
};
use codec::Encode;
use pallet_bridge_messages::OutboundLanes;
@@ -928,8 +928,8 @@ mod tests {
let dispatch_result =
FromMillauMessageDispatch::dispatch(&AccountId::from([0u8; 32]), incoming_message);
assert!(matches!(
- dispatch_result.dispatch_level_result,
- XcmBlobMessageDispatchResult::NotDispatched(_),
+ dispatch_result.dispatch_result,
+ Err(XcmBlobMessageDispatchError::NotDispatched(_)),
));
});
}
diff --git a/bridges/bin/rialto/runtime/src/xcm_config.rs b/bridges/bin/rialto/runtime/src/xcm_config.rs
index 9f6488b4c4d..265f435645e 100644
--- a/bridges/bin/rialto/runtime/src/xcm_config.rs
+++ b/bridges/bin/rialto/runtime/src/xcm_config.rs
@@ -197,7 +197,7 @@ mod tests {
target_chain::{DispatchMessage, DispatchMessageData, MessageDispatch},
LaneId, MessageKey,
};
- use bridge_runtime_common::messages_xcm_extension::XcmBlobMessageDispatchResult;
+ use bridge_runtime_common::messages_xcm_extension::XcmBlobMessageDispatchError;
use codec::Encode;
use pallet_bridge_messages::OutboundLanes;
use xcm_executor::XcmExecutor;
@@ -269,8 +269,8 @@ mod tests {
let dispatch_result =
FromMillauMessageDispatch::dispatch(&AccountId::from([0u8; 32]), incoming_message);
assert!(matches!(
- dispatch_result.dispatch_level_result,
- XcmBlobMessageDispatchResult::NotDispatched(_),
+ dispatch_result.dispatch_result,
+ Err(XcmBlobMessageDispatchError::NotDispatched(_)),
));
}
}
diff --git a/bridges/bin/runtime-common/src/integrity.rs b/bridges/bin/runtime-common/src/integrity.rs
index 5820dd99b91..3a677ce43ce 100644
--- a/bridges/bin/runtime-common/src/integrity.rs
+++ b/bridges/bin/runtime-common/src/integrity.rs
@@ -307,8 +307,10 @@ pub fn check_message_lane_weights<C: Chain, T: frame_system::Config>(
messages::target::maximal_incoming_message_dispatch_weight(C::max_extrinsic_weight()),
);
- let max_incoming_inbound_lane_data_proof_size =
- InboundLaneData::<()>::encoded_size_hint_u32(this_chain_max_unrewarded_relayers as _);
+ let max_incoming_inbound_lane_data_proof_size = InboundLaneData::<()>::encoded_size_hint_u32(
+ this_chain_max_unrewarded_relayers as _,
+ this_chain_max_unconfirmed_messages as _,
+ );
pallet_bridge_messages::ensure_able_to_receive_confirmation::<Weights<T>>(
C::max_extrinsic_size(),
C::max_extrinsic_weight(),
diff --git a/bridges/bin/runtime-common/src/messages_xcm_extension.rs b/bridges/bin/runtime-common/src/messages_xcm_extension.rs
index 4ccdd7a4b4d..d0fdc458e95 100644
--- a/bridges/bin/runtime-common/src/messages_xcm_extension.rs
+++ b/bridges/bin/runtime-common/src/messages_xcm_extension.rs
@@ -39,9 +39,8 @@ pub type XcmAsPlainPayload = sp_std::prelude::Vec<u8>;
/// Message dispatch result type for single message
#[derive(CloneNoBound, EqNoBound, PartialEqNoBound, Encode, Decode, Debug, TypeInfo)]
-pub enum XcmBlobMessageDispatchResult {
+pub enum XcmBlobMessageDispatchError {
InvalidPayload,
- Dispatched,
NotDispatched(#[codec(skip)] Option<DispatchBlobError>),
}
@@ -65,7 +64,7 @@ impl<
for XcmBlobMessageDispatch<SourceBridgeHubChain, TargetBridgeHubChain, BlobDispatcher, Weights>
{
type DispatchPayload = XcmAsPlainPayload;
- type DispatchLevelResult = XcmBlobMessageDispatchResult;
+ type DispatchError = XcmBlobMessageDispatchError;
fn dispatch_weight(message: &mut DispatchMessage<Self::DispatchPayload>) -> Weight {
match message.data.payload {
@@ -80,7 +79,7 @@ impl<
fn dispatch(
_relayer_account: &AccountIdOf<SourceBridgeHubChain>,
message: DispatchMessage<Self::DispatchPayload>,
- ) -> MessageDispatchResult<Self::DispatchLevelResult> {
+ ) -> MessageDispatchResult<Self::DispatchError> {
let payload = match message.data.payload {
Ok(payload) => payload,
Err(e) => {
@@ -92,7 +91,7 @@ impl<
);
return MessageDispatchResult {
unspent_weight: Weight::zero(),
- dispatch_level_result: XcmBlobMessageDispatchResult::InvalidPayload,
+ dispatch_result: Err(XcmBlobMessageDispatchError::InvalidPayload),
}
},
};
@@ -103,7 +102,7 @@ impl<
"[XcmBlobMessageDispatch] DispatchBlob::dispatch_blob was ok - message_nonce: {:?}",
message.key.nonce
);
- XcmBlobMessageDispatchResult::Dispatched
+ Ok(())
},
Err(e) => {
log::error!(
@@ -111,10 +110,13 @@ impl<
"[XcmBlobMessageDispatch] DispatchBlob::dispatch_blob failed, error: {:?} - message_nonce: {:?}",
e, message.key.nonce
);
- XcmBlobMessageDispatchResult::NotDispatched(Some(e))
+ Err(XcmBlobMessageDispatchError::NotDispatched(Some(e)))
},
};
- MessageDispatchResult { unspent_weight: Weight::zero(), dispatch_level_result }
+ MessageDispatchResult {
+ unspent_weight: Weight::zero(),
+ dispatch_result: dispatch_level_result,
+ }
}
}
diff --git a/bridges/modules/messages/Cargo.toml b/bridges/modules/messages/Cargo.toml
index f733d62bf64..7dbe6e15ba3 100644
--- a/bridges/modules/messages/Cargo.toml
+++ b/bridges/modules/messages/Cargo.toml
@@ -7,6 +7,7 @@ edition = "2021"
license = "GPL-3.0-or-later WITH Classpath-exception-2.0"
[dependencies]
+bitvec = { version = "1", default-features = false, features = ["alloc"] }
codec = { package = "parity-scale-codec", version = "3.1.5", default-features = false }
log = { version = "0.4.17", default-features = false }
num-traits = { version = "0.2", default-features = false }
diff --git a/bridges/modules/messages/src/benchmarking.rs b/bridges/modules/messages/src/benchmarking.rs
index aab8855a729..f7a2769715f 100644
--- a/bridges/modules/messages/src/benchmarking.rs
+++ b/bridges/modules/messages/src/benchmarking.rs
@@ -301,7 +301,7 @@ benchmarks_instance_pallet! {
inbound_lane_data: InboundLaneData {
relayers: vec![UnrewardedRelayer {
relayer: relayer_id.clone(),
- messages: DeliveredMessages::new(1),
+ messages: DeliveredMessages::new(1, true),
}].into_iter().collect(),
last_confirmed_nonce: 0,
},
@@ -333,8 +333,8 @@ benchmarks_instance_pallet! {
total_messages: 2,
last_delivered_nonce: 2,
};
- let mut delivered_messages = DeliveredMessages::new(1);
- delivered_messages.note_dispatched_message();
+ 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 {
@@ -379,11 +379,11 @@ benchmarks_instance_pallet! {
relayers: vec![
UnrewardedRelayer {
relayer: relayer1_id.clone(),
- messages: DeliveredMessages::new(1),
+ messages: DeliveredMessages::new(1, true),
},
UnrewardedRelayer {
relayer: relayer2_id.clone(),
- messages: DeliveredMessages::new(2),
+ messages: DeliveredMessages::new(2, true),
},
].into_iter().collect(),
last_confirmed_nonce: 0,
@@ -451,7 +451,7 @@ fn receive_messages<T: Config<I>, I: 'static>(nonce: MessageNonce) {
inbound_lane_storage.set_data(InboundLaneData {
relayers: vec![UnrewardedRelayer {
relayer: T::bridged_relayer_id(),
- messages: DeliveredMessages::new(nonce),
+ messages: DeliveredMessages::new(nonce, true),
}]
.into_iter()
.collect(),
diff --git a/bridges/modules/messages/src/inbound_lane.rs b/bridges/modules/messages/src/inbound_lane.rs
index 3f64ab765b5..21d835d31c7 100644
--- a/bridges/modules/messages/src/inbound_lane.rs
+++ b/bridges/modules/messages/src/inbound_lane.rs
@@ -101,6 +101,7 @@ impl<T: Config<I>, I: 'static> MaxEncodedLen for StoredInboundLaneData<T, I> {
fn max_encoded_len() -> usize {
InboundLaneData::<T::InboundRelayer>::encoded_size_hint(
T::MaxUnrewardedRelayerEntriesAtInboundLane::get() as usize,
+ T::MaxUnconfirmedMessagesAtInboundLane::get() as usize,
)
.unwrap_or(usize::MAX)
}
@@ -154,6 +155,9 @@ impl<S: InboundLaneStorage> InboundLane<S> {
// overlap.
match data.relayers.front_mut() {
Some(entry) if entry.messages.begin < new_confirmed_nonce => {
+ entry.messages.dispatch_results = entry.messages.dispatch_results
+ [(new_confirmed_nonce + 1 - entry.messages.begin) as usize..]
+ .to_bitvec();
entry.messages.begin = new_confirmed_nonce + 1;
},
_ => {},
@@ -170,7 +174,7 @@ impl<S: InboundLaneStorage> InboundLane<S> {
relayer_at_this_chain: &AccountId,
nonce: MessageNonce,
message_data: DispatchMessageData<Dispatch::DispatchPayload>,
- ) -> ReceivalResult<Dispatch::DispatchLevelResult> {
+ ) -> ReceivalResult<Dispatch::DispatchError> {
let mut data = self.storage.data();
let is_correct_message = nonce == data.last_delivered_nonce() + 1;
if !is_correct_message {
@@ -198,19 +202,20 @@ impl<S: InboundLaneStorage> InboundLane<S> {
);
// now let's update inbound lane storage
- let push_new = match data.relayers.back_mut() {
+ match data.relayers.back_mut() {
Some(entry) if entry.relayer == *relayer_at_bridged_chain => {
- entry.messages.note_dispatched_message();
- false
+ entry.messages.note_dispatched_message(dispatch_result.dispatch_result.is_ok());
+ },
+ _ => {
+ data.relayers.push_back(UnrewardedRelayer {
+ relayer: relayer_at_bridged_chain.clone(),
+ messages: DeliveredMessages::new(
+ nonce,
+ dispatch_result.dispatch_result.is_ok(),
+ ),
+ });
},
- _ => true,
};
- if push_new {
- data.relayers.push_back(UnrewardedRelayer {
- relayer: (*relayer_at_bridged_chain).clone(),
- messages: DeliveredMessages::new(nonce),
- });
- }
self.storage.set_data(data);
ReceivalResult::Dispatched(dispatch_result)
diff --git a/bridges/modules/messages/src/lib.rs b/bridges/modules/messages/src/lib.rs
index f7bc7d8a109..f425c824b6d 100644
--- a/bridges/modules/messages/src/lib.rs
+++ b/bridges/modules/messages/src/lib.rs
@@ -66,7 +66,7 @@ use bp_messages::{
use bp_runtime::{BasicOperatingMode, ChainId, OwnedBridgeModule, Size};
use codec::{Decode, Encode, MaxEncodedLen};
use frame_support::{dispatch::PostDispatchInfo, ensure, fail, traits::Get};
-use sp_runtime::traits::UniqueSaturatedFrom;
+use sp_runtime::{traits::UniqueSaturatedFrom, SaturatedConversion};
use sp_std::{cell::RefCell, marker::PhantomData, prelude::*};
mod inbound_lane;
@@ -527,7 +527,7 @@ pub mod pallet {
MessagesReceived(
Vec<
ReceivedMessages<
- <T::MessageDispatch as MessageDispatch<T::AccountId>>::DispatchLevelResult,
+ <T::MessageDispatch as MessageDispatch<T::AccountId>>::DispatchError,
>,
>,
),
@@ -807,15 +807,19 @@ impl<T: Config<I>, I: 'static> RuntimeInboundLaneStorage<T, I> {
/// `receive_messages_proof` call, because the actual inbound lane state is smaller than the
/// maximal configured.
///
- /// Maximal inbound lane state set size is configured by the
- /// `MaxUnrewardedRelayerEntriesAtInboundLane` constant from the pallet configuration. The PoV
- /// of the call includes the maximal size of inbound lane state. If the actual size is smaller,
- /// we may subtract extra bytes from this component.
+ /// Maximal inbound lane state size is computed using the
+ /// `MaxUnrewardedRelayerEntriesAtInboundLane` and `MaxUnconfirmedMessagesAtInboundLane`
+ /// constants from the pallet configuration. The PoV of the call includes the maximal size
+ /// of the inbound lane state. If the actual size is smaller, we may subtract extra bytes
+ /// from this component.
pub fn extra_proof_size_bytes(&self) -> u64 {
let max_encoded_len = StoredInboundLaneData::<T, I>::max_encoded_len();
let relayers_count = self.data().relayers.len();
+ let messages_count = self.data().relayers.iter().fold(0usize, |sum, relayer| {
+ sum.saturating_add(relayer.messages.total_messages().saturated_into::<usize>())
+ });
let actual_encoded_len =
- InboundLaneData::<T::InboundRelayer>::encoded_size_hint(relayers_count)
+ InboundLaneData::<T::InboundRelayer>::encoded_size_hint(relayers_count, messages_count)
.unwrap_or(usize::MAX);
max_encoded_len.saturating_sub(actual_encoded_len) as _
}
@@ -946,6 +950,7 @@ mod tests {
};
use frame_system::{EventRecord, Pallet as System, Phase};
use sp_runtime::DispatchError;
+ use std::collections::VecDeque;
fn get_ready_for_events() {
System::<TestRuntime>::set_block_number(1);
@@ -1003,7 +1008,7 @@ mod tests {
last_confirmed_nonce: 1,
relayers: vec![UnrewardedRelayer {
relayer: 0,
- messages: DeliveredMessages::new(1),
+ messages: DeliveredMessages::new(1, true),
}]
.into_iter()
.collect(),
@@ -1023,13 +1028,44 @@ mod tests {
phase: Phase::Initialization,
event: TestEvent::Messages(Event::MessagesDelivered {
lane_id: TEST_LANE_ID,
- messages: DeliveredMessages::new(1),
+ messages: DeliveredMessages::new(1, true),
}),
topics: vec![],
}],
);
}
+ fn unrewarded_relayer_entry(msg_count: usize) -> UnrewardedRelayer<TestRelayer> {
+ UnrewardedRelayer {
+ relayer: 42u64,
+ messages: DeliveredMessages {
+ begin: 0,
+ end: msg_count as MessageNonce - 1,
+ dispatch_results: FromIterator::from_iter(vec![true; msg_count]),
+ },
+ }
+ }
+
+ fn unrewarded_relayers_vec(
+ entry_count: usize,
+ msg_count: usize,
+ ) -> VecDeque<UnrewardedRelayer<TestRelayer>> {
+ if entry_count > msg_count {
+ panic!("unrewarded_relayers_vec(): expecting msg_count to be >= entry_count");
+ }
+
+ let mut unrewarded_relayers = vec![];
+ let mut available_msg_count = msg_count;
+ for _ in 0..entry_count - 1 {
+ unrewarded_relayers
+ .push(unrewarded_relayer_entry(std::cmp::min(1, available_msg_count)));
+ available_msg_count -= 1
+ }
+ unrewarded_relayers.push(unrewarded_relayer_entry(available_msg_count));
+
+ unrewarded_relayers.into_iter().collect()
+ }
+
#[test]
fn pallet_rejects_transactions_if_halted() {
run_test(|| {
@@ -1634,6 +1670,7 @@ mod tests {
fn proof_size_refund_from_receive_messages_proof_works() {
run_test(|| {
let max_entries = crate::mock::MaxUnrewardedRelayerEntriesAtInboundLane::get() as usize;
+ let max_msgs = crate::mock::MaxUnconfirmedMessagesAtInboundLane::get() as usize;
// if there's maximal number of unrewarded relayer entries at the inbound lane, then
// `proof_size` is unchanged in post-dispatch weight
@@ -1648,15 +1685,7 @@ mod tests {
InboundLanes::<TestRuntime>::insert(
TEST_LANE_ID,
StoredInboundLaneData(InboundLaneData {
- relayers: vec![
- UnrewardedRelayer {
- relayer: 42,
- messages: DeliveredMessages { begin: 0, end: 100 }
- };
- max_entries
- ]
- .into_iter()
- .collect(),
+ relayers: unrewarded_relayers_vec(max_entries, max_msgs),
last_confirmed_nonce: 0,
}),
);
@@ -1677,15 +1706,7 @@ mod tests {
InboundLanes::<TestRuntime>::insert(
TEST_LANE_ID,
StoredInboundLaneData(InboundLaneData {
- relayers: vec![
- UnrewardedRelayer {
- relayer: 42,
- messages: DeliveredMessages { begin: 0, end: 100 }
- };
- max_entries - 1
- ]
- .into_iter()
- .collect(),
+ relayers: unrewarded_relayers_vec(max_entries - 1, max_msgs),
last_confirmed_nonce: 0,
}),
);
@@ -1701,7 +1722,7 @@ mod tests {
.unwrap();
assert!(
post_dispatch_weight.proof_size() < pre_dispatch_weight.proof_size(),
- "Expected post-dispatch PoV {} to be less than pre-dispatch PoV {}",
+ "Expected post-dispatch PoV {} to be < than pre-dispatch PoV {}",
post_dispatch_weight.proof_size(),
pre_dispatch_weight.proof_size(),
);
@@ -1717,8 +1738,8 @@ mod tests {
// 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);
- delivered_messages_1_and_2.note_dispatched_message();
+ let mut delivered_messages_1_and_2 = DeliveredMessages::new(1, true);
+ delivered_messages_1_and_2.note_dispatched_message(true);
let messages_1_and_2_proof = Ok((
TEST_LANE_ID,
InboundLaneData {
@@ -1731,7 +1752,7 @@ mod tests {
.collect(),
},
));
- let delivered_message_3 = DeliveredMessages::new(3);
+ let delivered_message_3 = DeliveredMessages::new(3, true);
let messages_3_proof = Ok((
TEST_LANE_ID,
InboundLaneData {
@@ -2022,7 +2043,7 @@ mod tests {
last_confirmed_nonce: 1,
relayers: vec![UnrewardedRelayer {
relayer: 0,
- messages: DeliveredMessages::new(1),
+ messages: DeliveredMessages::new(1, true),
}]
.into_iter()
.collect(),
@@ -2082,39 +2103,39 @@ mod tests {
#[test]
fn inbound_storage_extra_proof_size_bytes_works() {
- fn relayer_entry() -> UnrewardedRelayer<TestRelayer> {
- UnrewardedRelayer { relayer: 42u64, messages: DeliveredMessages { begin: 0, end: 100 } }
- }
+ let max_entries = crate::mock::MaxUnrewardedRelayerEntriesAtInboundLane::get() as usize;
+ let max_msgs = crate::mock::MaxUnconfirmedMessagesAtInboundLane::get() as usize;
- fn storage(relayer_entries: usize) -> RuntimeInboundLaneStorage<TestRuntime, ()> {
+ fn storage(
+ entry_count: usize,
+ msg_count: usize,
+ ) -> RuntimeInboundLaneStorage<TestRuntime, ()> {
RuntimeInboundLaneStorage {
lane_id: Default::default(),
cached_data: RefCell::new(Some(InboundLaneData {
- relayers: vec![relayer_entry(); relayer_entries].into_iter().collect(),
+ relayers: unrewarded_relayers_vec(entry_count, msg_count),
last_confirmed_nonce: 0,
})),
_phantom: Default::default(),
}
}
- let max_entries = crate::mock::MaxUnrewardedRelayerEntriesAtInboundLane::get() as usize;
-
// when we have exactly `MaxUnrewardedRelayerEntriesAtInboundLane` unrewarded relayers
- assert_eq!(storage(max_entries).extra_proof_size_bytes(), 0);
+ assert_eq!(storage(max_entries, max_msgs).extra_proof_size_bytes(), 0);
// when we have less than `MaxUnrewardedRelayerEntriesAtInboundLane` unrewarded relayers
assert_eq!(
- storage(max_entries - 1).extra_proof_size_bytes(),
- relayer_entry().encode().len() as u64
+ storage(max_entries - 1, max_msgs).extra_proof_size_bytes(),
+ unrewarded_relayer_entry(1).encoded_size() as u64
);
assert_eq!(
- storage(max_entries - 2).extra_proof_size_bytes(),
- 2 * relayer_entry().encode().len() as u64
+ storage(max_entries - 2, max_msgs).extra_proof_size_bytes(),
+ 2 * unrewarded_relayer_entry(1).encoded_size() as u64
);
// when we have more than `MaxUnrewardedRelayerEntriesAtInboundLane` unrewarded relayers
// (shall not happen in practice)
- assert_eq!(storage(max_entries + 1).extra_proof_size_bytes(), 0);
+ assert_eq!(storage(max_entries + 1, max_msgs).extra_proof_size_bytes(), 0);
}
#[test]
diff --git a/bridges/modules/messages/src/mock.rs b/bridges/modules/messages/src/mock.rs
index 02940f369e2..be4072f9278 100644
--- a/bridges/modules/messages/src/mock.rs
+++ b/bridges/modules/messages/src/mock.rs
@@ -19,6 +19,7 @@
use crate::Config;
+use bitvec::prelude::*;
use bp_messages::{
calc_relayers_rewards,
source_chain::{DeliveryConfirmationPayments, LaneMessageVerifier, TargetHeaderChain},
@@ -62,13 +63,13 @@ pub struct TestPayload {
///
/// Note: in correct code `dispatch_result.unspent_weight` will always be <= `declared_weight`,
/// but for test purposes we'll be making it larger than `declared_weight` sometimes.
- pub dispatch_result: MessageDispatchResult<TestDispatchLevelResult>,
+ pub dispatch_result: MessageDispatchResult<TestDispatchError>,
/// Extra bytes that affect payload size.
pub extra: Vec<u8>,
}
pub type TestMessageFee = u64;
pub type TestRelayer = u64;
-pub type TestDispatchLevelResult = ();
+pub type TestDispatchError = ();
type Block = frame_system::mocking::MockBlock<TestRuntime>;
type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic<TestRuntime>;
@@ -412,7 +413,7 @@ pub struct TestMessageDispatch;
impl MessageDispatch<AccountId> for TestMessageDispatch {
type DispatchPayload = TestPayload;
- type DispatchLevelResult = TestDispatchLevelResult;
+ type DispatchError = TestDispatchError;
fn dispatch_weight(message: &mut DispatchMessage<TestPayload>) -> Weight {
match message.data.payload.as_ref() {
@@ -424,7 +425,7 @@ impl MessageDispatch<AccountId> for TestMessageDispatch {
fn dispatch(
_relayer_account: &AccountId,
message: DispatchMessage<TestPayload>,
- ) -> MessageDispatchResult<TestDispatchLevelResult> {
+ ) -> MessageDispatchResult<TestDispatchError> {
match message.data.payload.as_ref() {
Ok(payload) => payload.dispatch_result.clone(),
Err(_) => dispatch_result(0),
@@ -459,12 +460,10 @@ pub const fn message_payload(id: u64, declared_weight: u64) -> TestPayload {
}
/// Returns message dispatch result with given unspent weight.
-pub const fn dispatch_result(
- unspent_weight: u64,
-) -> MessageDispatchResult<TestDispatchLevelResult> {
+pub const fn dispatch_result(unspent_weight: u64) -> MessageDispatchResult<TestDispatchError> {
MessageDispatchResult {
unspent_weight: Weight::from_parts(unspent_weight, 0),
- dispatch_level_result: (),
+ dispatch_result: Ok(()),
}
}
@@ -474,7 +473,14 @@ pub fn unrewarded_relayer(
end: MessageNonce,
relayer: TestRelayer,
) -> UnrewardedRelayer<TestRelayer> {
- UnrewardedRelayer { relayer, messages: DeliveredMessages { begin, end } }
+ UnrewardedRelayer {
+ relayer,
+ messages: DeliveredMessages {
+ begin,
+ end,
+ dispatch_results: bitvec![u8, Msb0; 1; (end + 1).saturating_sub(begin) as _],
+ },
+ }
}
/// Return test externalities to use in tests.
diff --git a/bridges/modules/messages/src/outbound_lane.rs b/bridges/modules/messages/src/outbound_lane.rs
index 33a58a40400..5b3281b5b16 100644
--- a/bridges/modules/messages/src/outbound_lane.rs
+++ b/bridges/modules/messages/src/outbound_lane.rs
@@ -18,8 +18,10 @@
use crate::Config;
+use bitvec::prelude::*;
use bp_messages::{
- DeliveredMessages, LaneId, MessageNonce, MessagePayload, OutboundLaneData, UnrewardedRelayer,
+ DeliveredMessages, DispatchResultsBitVec, LaneId, MessageNonce, MessagePayload,
+ OutboundLaneData, UnrewardedRelayer,
};
use frame_support::{
weights::{RuntimeDbWeight, Weight},
@@ -65,6 +67,9 @@ pub enum ReceivalConfirmationResult {
/// 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,
/// The chain has more messages that need to be confirmed than there is in the proof.
TryingToConfirmMoreMessagesThanExpected(MessageNonce),
}
@@ -124,9 +129,14 @@ impl<S: OutboundLaneStorage> OutboundLane<S> {
)
}
- if let Err(e) = ensure_unrewarded_relayers_are_correct(latest_delivered_nonce, relayers) {
- return e
- }
+ let dispatch_results = match extract_dispatch_results(
+ data.latest_received_nonce,
+ latest_delivered_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_delivered_nonce;
@@ -135,6 +145,7 @@ impl<S: OutboundLaneStorage> OutboundLane<S> {
ReceivalConfirmationResult::ConfirmedMessages(DeliveredMessages {
begin: prev_latest_received_nonce + 1,
end: latest_delivered_nonce,
+ dispatch_results,
})
}
@@ -169,30 +180,34 @@ impl<S: OutboundLaneStorage> OutboundLane<S> {
}
}
-/// Verifies unrewarded relayers vec.
+/// 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 ensure_unrewarded_relayers_are_correct<RelayerId>(
+fn extract_dispatch_results<RelayerId>(
+ prev_latest_received_nonce: MessageNonce,
latest_received_nonce: MessageNonce,
relayers: &VecDeque<UnrewardedRelayer<RelayerId>>,
-) -> Result<(), ReceivalConfirmationResult> {
- let mut last_entry_end: Option<MessageNonce> = None;
+) -> Result<DispatchResultsBitVec, ReceivalConfirmationResult> {
+ // 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 expected_entry_begin = relayers.front().map(|entry| entry.messages.begin);
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 {
+ if entry.messages.total_messages() == 0 {
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)
- }
+ if expected_entry_begin != Some(entry.messages.begin) {
+ return Err(ReceivalConfirmationResult::NonConsecutiveUnrewardedRelayerEntries)
}
- last_entry_end = Some(entry.messages.end);
+ expected_entry_begin = entry.messages.end.checked_add(1);
// 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 {
@@ -201,9 +216,30 @@ fn ensure_unrewarded_relayers_are_correct<RelayerId>(
// 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.total_messages()
+ {
+ 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);
+ if entry.messages.end < new_messages_begin {
+ 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(())
+ Ok(received_dispatch_result)
}
#[cfg(test)]
@@ -228,7 +264,11 @@ mod tests {
}
fn delivered_messages(nonces: RangeInclusive<MessageNonce>) -> DeliveredMessages {
- DeliveredMessages { begin: *nonces.start(), end: *nonces.end() }
+ DeliveredMessages {
+ begin: *nonces.start(),
+ end: *nonces.end(),
+ dispatch_results: bitvec![u8, Msb0; 1; (nonces.end() - nonces.start() + 1) as _],
+ }
}
fn assert_3_messages_confirmation_fails(
@@ -361,6 +401,20 @@ mod tests {
);
}
+ #[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]
fn prune_messages_works() {
run_test(|| {
diff --git a/bridges/primitives/messages/Cargo.toml b/bridges/primitives/messages/Cargo.toml
index 32d7c65ebcb..bc1b92c6b88 100644
--- a/bridges/primitives/messages/Cargo.toml
+++ b/bridges/primitives/messages/Cargo.toml
@@ -7,6 +7,7 @@ edition = "2021"
license = "GPL-3.0-or-later WITH Classpath-exception-2.0"
[dependencies]
+bitvec = { version = "1", default-features = false, features = ["alloc"] }
codec = { package = "parity-scale-codec", version = "3.1.5", default-features = false, features = ["derive", "bit-vec"] }
scale-info = { version = "2.5.0", default-features = false, features = ["bit-vec", "derive"] }
serde = { version = "1.0", optional = true, features = ["derive"] }
@@ -28,6 +29,7 @@ hex-literal = "0.4"
[features]
default = ["std"]
std = [
+ "bitvec/std",
"bp-runtime/std",
"codec/std",
"frame-support/std",
diff --git a/bridges/primitives/messages/src/lib.rs b/bridges/primitives/messages/src/lib.rs
index 8c10989d620..c877c9cdf2c 100644
--- a/bridges/primitives/messages/src/lib.rs
+++ b/bridges/primitives/messages/src/lib.rs
@@ -20,6 +20,7 @@
// RuntimeApi generated functions
#![allow(clippy::too_many_arguments)]
+use bitvec::prelude::*;
use bp_runtime::{BasicOperatingMode, OperatingMode};
use codec::{Decode, Encode, MaxEncodedLen};
use frame_support::RuntimeDebug;
@@ -156,18 +157,68 @@ impl<RelayerId> Default for InboundLaneData<RelayerId> {
}
impl<RelayerId> InboundLaneData<RelayerId> {
+ fn dispatch_results_encoded_size_hint(
+ relayers_entries: usize,
+ message_count: usize,
+ ) -> Option<usize>
+ where
+ RelayerId: MaxEncodedLen,
+ {
+ // The worst-case scenario for the bitvecs size is the one in which we have as many relayer
+ // entries as possible taking an extra 1 byte slot with just 1 bit of actual information.
+ // For example:
+ // 11111111 1-------
+ // 11111111 1-------
+ // 1-------
+ // 1-------
+
+ // If there are less msgs than relayer entries, in the worst case, each dispatch result
+ // belongs to a different relayer slot. This means 1 byte for the len prefix and 1 byte
+ // for the actual data.
+ if relayers_entries >= message_count {
+ return relayers_entries.checked_add(message_count)
+ }
+
+ let msgs_per_byte = 8;
+ // At the begining each relayer slot has 1 message, using 1 byte
+ let mut num_result_bytes = relayers_entries;
+ // Then we add batches of 8 messages to some relayer slot until there are no more messages.
+ // Each batch takes up 1 more byte.
+ num_result_bytes =
+ num_result_bytes.checked_add((message_count - relayers_entries) / msgs_per_byte)?;
+
+ // The len is stored in a `Compact<u32>`. `Compact<u32>` can store a max value of
+ // 63 on 1 byte, 16383 on 2 bytes, etc.
+ let max_len_per_first_byte = 0b0011_1111;
+ // At the begining each relayer slot uses 1 byte for the len prefix
+ // (each relayer slot contains 1 message)
+ let mut num_len_bytes = relayers_entries;
+ // Then we add batches of 63 messages to as many relayer slots as possible, requiring 2
+ // bytes for the `len` prefix. It's hard to believe that we'll need more than 2 bytes
+ // (more than 16383 messages in 1 relayer slot).
+ num_len_bytes = num_len_bytes.checked_add(sp_std::cmp::min(
+ (message_count - relayers_entries) / max_len_per_first_byte,
+ relayers_entries,
+ ))?;
+
+ num_result_bytes.checked_add(num_len_bytes)
+ }
+
/// Returns approximate size of the struct, given a number of entries in the `relayers` set and
/// size of each entry.
///
/// Returns `None` if size overflows `usize` limits.
- pub fn encoded_size_hint(relayers_entries: usize) -> Option<usize>
+ pub fn encoded_size_hint(relayers_entries: usize, message_count: usize) -> Option<usize>
where
RelayerId: MaxEncodedLen,
{
let message_nonce_size = MessageNonce::max_encoded_len();
let relayer_id_encoded_size = RelayerId::max_encoded_len();
let relayers_entry_size = relayer_id_encoded_size.checked_add(2 * message_nonce_size)?;
- let relayers_size = relayers_entries.checked_mul(relayers_entry_size)?;
+ let relayers_size = relayers_entries.checked_mul(relayers_entry_size)?.checked_add(
+ Self::dispatch_results_encoded_size_hint(relayers_entries, message_count)?,
+ )?;
+
relayers_size.checked_add(message_nonce_size)
}
@@ -175,11 +226,11 @@ impl<RelayerId> InboundLaneData<RelayerId> {
/// `relayers` set and the size of each entry.
///
/// Returns `u32::MAX` if size overflows `u32` limits.
- pub fn encoded_size_hint_u32(relayers_entries: usize) -> u32
+ pub fn encoded_size_hint_u32(relayers_entries: usize, messages_count: usize) -> u32
where
RelayerId: MaxEncodedLen,
{
- Self::encoded_size_hint(relayers_entries)
+ Self::encoded_size_hint(relayers_entries, messages_count)
.and_then(|x| u32::try_from(x).ok())
.unwrap_or(u32::MAX)
}
@@ -219,6 +270,9 @@ pub struct InboundMessageDetails {
pub dispatch_weight: Weight,
}
+/// Bit vector of message dispatch results.
+pub type DispatchResultsBitVec = BitVec<u8, Msb0>;
+
/// Unrewarded relayer entry stored in the inbound lane data.
///
/// This struct represents a continuous range of messages that have been delivered by the same
@@ -276,13 +330,19 @@ pub struct DeliveredMessages {
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) -> Self {
- DeliveredMessages { begin: nonce, end: nonce }
+ pub fn new(nonce: MessageNonce, dispatch_result: bool) -> Self {
+ let mut dispatch_results = BitVec::with_capacity(1);
+ dispatch_results.push(dispatch_result);
+ DeliveredMessages { begin: nonce, end: nonce, dispatch_results }
}
/// Return total count of delivered messages.
@@ -295,14 +355,25 @@ impl DeliveredMessages {
}
/// Note new dispatched message.
- pub fn note_dispatched_message(&mut self) {
+ 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.
+ pub fn message_dispatch_result(&self, nonce: MessageNonce) -> Option<bool> {
+ let index = nonce.checked_sub(self.begin)? as usize;
+ self.dispatch_results.get(index).map(|bit| *bit)
+ }
}
/// Gist of `InboundLaneData::relayers` field used by runtime APIs.
@@ -414,10 +485,10 @@ mod tests {
assert_eq!(
total_unrewarded_messages(
&vec![
- UnrewardedRelayer { relayer: 1, messages: DeliveredMessages::new(0) },
+ UnrewardedRelayer { relayer: 1, messages: DeliveredMessages::new(0, true) },
UnrewardedRelayer {
relayer: 2,
- messages: DeliveredMessages::new(MessageNonce::MAX)
+ messages: DeliveredMessages::new(MessageNonce::MAX, true)
},
]
.into_iter()
@@ -438,12 +509,21 @@ mod tests {
(13u8, 128u8),
];
for (relayer_entries, messages_count) in test_cases {
- let expected_size = InboundLaneData::<u8>::encoded_size_hint(relayer_entries as _);
+ let expected_size =
+ InboundLaneData::<u8>::encoded_size_hint(relayer_entries as _, messages_count as _);
let actual_size = InboundLaneData {
relayers: (1u8..=relayer_entries)
- .map(|i| UnrewardedRelayer {
- relayer: i,
- messages: DeliveredMessages::new(i as _),
+ .map(|i| {
+ let mut entry = UnrewardedRelayer {
+ relayer: i,
+ messages: DeliveredMessages::new(i as _, true),
+ };
+ entry.messages.dispatch_results = bitvec![
+ u8, Msb0;
+ 1;
+ (messages_count / relayer_entries) as _
+ ];
+ entry
})
.collect(),
last_confirmed_nonce: messages_count as _,
@@ -459,13 +539,16 @@ mod tests {
}
#[test]
- fn contains_result_works() {
- let delivered_messages = DeliveredMessages { begin: 100, end: 150 };
+ fn message_dispatch_result_works() {
+ let delivered_messages =
+ DeliveredMessages { begin: 100, end: 150, dispatch_results: bitvec![u8, Msb0; 1; 151] };
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), Some(true));
}
#[test]
diff --git a/bridges/primitives/messages/src/target_chain.rs b/bridges/primitives/messages/src/target_chain.rs
index 8496b90214c..da96c4e59cd 100644
--- a/bridges/primitives/messages/src/target_chain.rs
+++ b/bridges/primitives/messages/src/target_chain.rs
@@ -90,7 +90,7 @@ pub trait MessageDispatch<AccountId> {
type DispatchPayload: Decode;
/// Fine-grained result of single message dispatch (for better diagnostic purposes)
- type DispatchLevelResult: Clone + sp_std::fmt::Debug + Eq;
+ type DispatchError: Clone + sp_std::fmt::Debug + Eq;
/// Estimate dispatch weight.
///
@@ -109,7 +109,7 @@ pub trait MessageDispatch<AccountId> {
fn dispatch(
relayer_account: &AccountId,
message: DispatchMessage<Self::DispatchPayload>,
- ) -> MessageDispatchResult<Self::DispatchLevelResult>;
+ ) -> MessageDispatchResult<Self::DispatchError>;
}
/// Manages payments that are happening at the target chain during message delivery transaction.
@@ -190,7 +190,7 @@ impl<MessagesProof, DispatchPayload: Decode, AccountId> MessageDispatch<AccountI
for ForbidInboundMessages<MessagesProof, DispatchPayload>
{
type DispatchPayload = DispatchPayload;
- type DispatchLevelResult = ();
+ type DispatchError = ();
fn dispatch_weight(_message: &mut DispatchMessage<Self::DispatchPayload>) -> Weight {
Weight::MAX
@@ -199,7 +199,7 @@ impl<MessagesProof, DispatchPayload: Decode, AccountId> MessageDispatch<AccountI
fn dispatch(
_: &AccountId,
_: DispatchMessage<Self::DispatchPayload>,
- ) -> MessageDispatchResult<Self::DispatchLevelResult> {
- MessageDispatchResult { unspent_weight: Weight::zero(), dispatch_level_result: () }
+ ) -> MessageDispatchResult<Self::DispatchError> {
+ MessageDispatchResult { unspent_weight: Weight::zero(), dispatch_result: Err(()) }
}
}
diff --git a/bridges/primitives/runtime/src/messages.rs b/bridges/primitives/runtime/src/messages.rs
index 9f7c8ab5ca4..efbd7518167 100644
--- a/bridges/primitives/runtime/src/messages.rs
+++ b/bridges/primitives/runtime/src/messages.rs
@@ -22,7 +22,7 @@ use scale_info::TypeInfo;
/// Message dispatch result.
#[derive(Encode, Decode, RuntimeDebug, Clone, PartialEq, Eq, TypeInfo)]
-pub struct MessageDispatchResult<DispatchLevelResult> {
+pub struct MessageDispatchResult<DispatchError> {
/// 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:
///
@@ -31,5 +31,5 @@ pub struct MessageDispatchResult<DispatchLevelResult> {
/// 2) if message has not been dispatched at all.
pub unspent_weight: Weight,
/// Fine-grained result of single message dispatch (for better diagnostic purposes)
- pub dispatch_level_result: DispatchLevelResult,
+ pub dispatch_result: Result<(), DispatchError>,
}
--
GitLab