Skip to content

Commit

Permalink
Reintroduce msg dispatch status reporting (paritytech#2027)
Browse files Browse the repository at this point in the history
* Use an actual Result inside MessageDispatchResult

We need this in order to distinguish between Ok and Err

* Revert paritytech#1660

* Fixes + simplifications

* Implement review suggestions
  • Loading branch information
serban300 authored Apr 10, 2023
1 parent d1e852c commit 68ba699
Show file tree
Hide file tree
Showing 16 changed files with 309 additions and 131 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions bin/millau/runtime/src/xcm_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(_)),
));
}

Expand All @@ -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(_)),
));
}
}
6 changes: 3 additions & 3 deletions bin/rialto-parachain/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(_)),
));
});
}
Expand Down
6 changes: 3 additions & 3 deletions bin/rialto/runtime/src/xcm_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(_)),
));
}
}
6 changes: 4 additions & 2 deletions bin/runtime-common/src/integrity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
18 changes: 10 additions & 8 deletions bin/runtime-common/src/messages_xcm_extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>),
}

Expand All @@ -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 {
Expand All @@ -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) => {
Expand All @@ -92,7 +91,7 @@ impl<
);
return MessageDispatchResult {
unspent_weight: Weight::zero(),
dispatch_level_result: XcmBlobMessageDispatchResult::InvalidPayload,
dispatch_result: Err(XcmBlobMessageDispatchError::InvalidPayload),
}
},
};
Expand All @@ -103,18 +102,21 @@ impl<
"[XcmBlobMessageDispatch] DispatchBlob::dispatch_blob was ok - message_nonce: {:?}",
message.key.nonce
);
XcmBlobMessageDispatchResult::Dispatched
Ok(())
},
Err(e) => {
log::error!(
target: crate::LOG_TARGET_BRIDGE_DISPATCH,
"[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,
}
}
}

Expand Down
1 change: 1 addition & 0 deletions modules/messages/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
12 changes: 6 additions & 6 deletions modules/messages/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
Expand Down
27 changes: 16 additions & 11 deletions modules/messages/src/inbound_lane.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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;
},
_ => {},
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down
Loading

0 comments on commit 68ba699

Please sign in to comment.