Relay basic single-bit message dispatch results back to the source ch…

…ain (paritytech#935)

* relay dispatch result flags back to the source chain

* OnMessagesDelivered callback

* add lane id to OnDeliveredMessages callback

* fix benchmarks && upate weights

* clippy

* clippy

* clipy another try

* OnMessagesDelivered -> OnDeliveryConfirmed

* Update primitives/messages/src/source_chain.rs

Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com>

Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com>

bin/millau/runtime/src/lib.rs

@@ -384,6 +384,7 @@ impl pallet_bridge_messages::Config<WithRialtoMessagesInstance> for Runtime {
 		GetDeliveryConfirmationTransactionFee,
 		RootAccountForPayments,
 	>;
+	type OnDeliveryConfirmed = ();
 
 	type SourceHeaderChain = crate::rialto_messages::Rialto;
 	type MessageDispatch = crate::rialto_messages::FromRialtoMessageDispatch;
@@ -702,6 +703,7 @@ mod tests {
 		let max_incoming_inbound_lane_data_proof_size = bp_messages::InboundLaneData::<()>::encoded_size_hint(
 			bp_millau::MAXIMAL_ENCODED_ACCOUNT_ID_SIZE,
 			bp_rialto::MAX_UNREWARDED_RELAYER_ENTRIES_AT_INBOUND_LANE as _,
+			bp_rialto::MAX_UNCONFIRMED_MESSAGES_AT_INBOUND_LANE as _,
 		)
 		.unwrap_or(u32::MAX);
 		pallet_bridge_messages::ensure_able_to_receive_confirmation::<Weights>(

bin/millau/runtime/src/rialto_messages.rs

@@ -113,9 +113,12 @@ impl messages::ThisChainWithMessages for Millau {
 	}
 
 	fn estimate_delivery_confirmation_transaction() -> MessageTransaction<Weight> {
-		let inbound_data_size =
-			InboundLaneData::<bp_millau::AccountId>::encoded_size_hint(bp_millau::MAXIMAL_ENCODED_ACCOUNT_ID_SIZE, 1)
-				.unwrap_or(u32::MAX);
+		let inbound_data_size = InboundLaneData::<bp_millau::AccountId>::encoded_size_hint(
+			bp_millau::MAXIMAL_ENCODED_ACCOUNT_ID_SIZE,
+			1,
+			1,
+		)
+		.unwrap_or(u32::MAX);
 
 		MessageTransaction {
 			dispatch_weight: bp_millau::MAX_SINGLE_MESSAGE_DELIVERY_CONFIRMATION_TX_WEIGHT,

bin/rialto/runtime/src/lib.rs

@@ -491,6 +491,7 @@ impl pallet_bridge_messages::Config<WithMillauMessagesInstance> for Runtime {
 		GetDeliveryConfirmationTransactionFee,
 		RootAccountForPayments,
 	>;
+	type OnDeliveryConfirmed = ();
 
 	type SourceHeaderChain = crate::millau_messages::Millau;
 	type MessageDispatch = crate::millau_messages::FromMillauMessageDispatch;
@@ -1028,7 +1029,7 @@ impl_runtime_apis! {
 						.map(|event_record| event_record.event)
 						.any(|event| matches!(
 							event,
-							Event::pallet_bridge_dispatch(pallet_bridge_dispatch::Event::<Runtime, _>::MessageDispatched(
+							Event::BridgeDispatch(pallet_bridge_dispatch::Event::<Runtime, _>::MessageDispatched(
 								_, ([0, 0, 0, 0], nonce_from_event), _,
 							)) if nonce_from_event == nonce
 						))
@@ -1144,6 +1145,7 @@ mod tests {
 		let max_incoming_inbound_lane_data_proof_size = bp_messages::InboundLaneData::<()>::encoded_size_hint(
 			bp_rialto::MAXIMAL_ENCODED_ACCOUNT_ID_SIZE,
 			bp_millau::MAX_UNREWARDED_RELAYER_ENTRIES_AT_INBOUND_LANE as _,
+			bp_millau::MAX_UNCONFIRMED_MESSAGES_AT_INBOUND_LANE as _,
 		)
 		.unwrap_or(u32::MAX);
 		pallet_bridge_messages::ensure_able_to_receive_confirmation::<Weights>(

bin/rialto/runtime/src/millau_messages.rs

@@ -113,9 +113,12 @@ impl messages::ThisChainWithMessages for Rialto {
 	}
 
 	fn estimate_delivery_confirmation_transaction() -> MessageTransaction<Weight> {
-		let inbound_data_size =
-			InboundLaneData::<bp_rialto::AccountId>::encoded_size_hint(bp_rialto::MAXIMAL_ENCODED_ACCOUNT_ID_SIZE, 1)
-				.unwrap_or(u32::MAX);
+		let inbound_data_size = InboundLaneData::<bp_rialto::AccountId>::encoded_size_hint(
+			bp_rialto::MAXIMAL_ENCODED_ACCOUNT_ID_SIZE,
+			1,
+			1,
+		)
+		.unwrap_or(u32::MAX);
 
 		MessageTransaction {
 			dispatch_weight: bp_rialto::MAX_SINGLE_MESSAGE_DELIVERY_CONFIRMATION_TX_WEIGHT,

modules/dispatch/src/lib.rs

@@ -154,6 +154,7 @@ impl<T: Config<I>, I: Instance> MessageDispatch<T::AccountId, T::MessageId> for
 				);
 				Self::deposit_event(RawEvent::MessageRejected(source_chain, id));
 				return MessageDispatchResult {
+					dispatch_result: false,
 					unspent_weight: 0,
 					dispatch_fee_paid_during_dispatch: false,
 				};
@@ -163,6 +164,7 @@ impl<T: Config<I>, I: Instance> MessageDispatch<T::AccountId, T::MessageId> for
 		// verify spec version
 		// (we want it to be the same, because otherwise we may decode Call improperly)
 		let mut dispatch_result = MessageDispatchResult {
+			dispatch_result: false,
 			unspent_weight: message.weight,
 			dispatch_fee_paid_during_dispatch: false,
 		};
@@ -303,6 +305,7 @@ impl<T: Config<I>, I: Instance> MessageDispatch<T::AccountId, T::MessageId> for
 		log::trace!(target: "runtime::bridge-dispatch", "Message being dispatched is: {:.4096?}", &call);
 		let result = call.dispatch(origin);
 		let actual_call_weight = extract_actual_weight(&result, &dispatch_info);
+		dispatch_result.dispatch_result = result.is_ok();
 		dispatch_result.unspent_weight = message.weight.saturating_sub(actual_call_weight);
 
 		log::trace!(
@@ -573,6 +576,7 @@ mod tests {
 			System::set_block_number(1);
 			let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| unreachable!());
 			assert_eq!(result.unspent_weight, weight);
+			assert!(!result.dispatch_result);
 
 			assert_eq!(
 				System::events(),
@@ -601,6 +605,7 @@ mod tests {
 			System::set_block_number(1);
 			let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| unreachable!());
 			assert_eq!(result.unspent_weight, 7);
+			assert!(!result.dispatch_result);
 
 			assert_eq!(
 				System::events(),
@@ -633,6 +638,7 @@ mod tests {
 			System::set_block_number(1);
 			let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| unreachable!());
 			assert_eq!(result.unspent_weight, weight);
+			assert!(!result.dispatch_result);
 
 			assert_eq!(
 				System::events(),
@@ -683,6 +689,7 @@ mod tests {
 			System::set_block_number(1);
 			let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| unreachable!());
 			assert_eq!(result.unspent_weight, weight);
+			assert!(!result.dispatch_result);
 
 			assert_eq!(
 				System::events(),
@@ -711,6 +718,7 @@ mod tests {
 			System::set_block_number(1);
 			let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| unreachable!());
 			assert_eq!(result.unspent_weight, weight);
+			assert!(!result.dispatch_result);
 
 			assert_eq!(
 				System::events(),
@@ -739,12 +747,13 @@ mod tests {
 			System::set_block_number(1);
 			let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| Err(()));
 			assert_eq!(result.unspent_weight, weight);
+			assert!(!result.dispatch_result);
 
 			assert_eq!(
 				System::events(),
 				vec![EventRecord {
 					phase: Phase::Initialization,
-					event: Event::call_dispatch(call_dispatch::Event::<TestRuntime>::MessageDispatchPaymentFailed(
+					event: Event::Dispatch(call_dispatch::Event::<TestRuntime>::MessageDispatchPaymentFailed(
 						SOURCE_CHAIN_ID,
 						id,
 						AccountIdConverter::convert(derive_account_id::<AccountId>(
@@ -771,12 +780,13 @@ mod tests {
 			System::set_block_number(1);
 			let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| Ok(()));
 			assert!(result.dispatch_fee_paid_during_dispatch);
+			assert!(result.dispatch_result);
 
 			assert_eq!(
 				System::events(),
 				vec![EventRecord {
 					phase: Phase::Initialization,
-					event: Event::call_dispatch(call_dispatch::Event::<TestRuntime>::MessageDispatched(
+					event: Event::Dispatch(call_dispatch::Event::<TestRuntime>::MessageDispatched(
 						SOURCE_CHAIN_ID,
 						id,
 						Ok(())
@@ -787,6 +797,34 @@ mod tests {
 		});
 	}
 
+	#[test]
+	fn should_return_dispatch_failed_flag_if_dispatch_happened_but_failed() {
+		new_test_ext().execute_with(|| {
+			let id = [0; 4];
+
+			let call = Call::System(<frame_system::Call<TestRuntime>>::set_heap_pages(1));
+			let message = prepare_target_message(call);
+
+			System::set_block_number(1);
+			let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| unreachable!());
+			assert!(!result.dispatch_fee_paid_during_dispatch);
+			assert!(!result.dispatch_result);
+
+			assert_eq!(
+				System::events(),
+				vec![EventRecord {
+					phase: Phase::Initialization,
+					event: Event::Dispatch(call_dispatch::Event::<TestRuntime>::MessageDispatched(
+						SOURCE_CHAIN_ID,
+						id,
+						Err(sp_runtime::DispatchError::BadOrigin)
+					)),
+					topics: vec![],
+				}],
+			);
+		})
+	}
+
 	#[test]
 	fn should_dispatch_bridge_message_from_root_origin() {
 		new_test_ext().execute_with(|| {
@@ -796,6 +834,7 @@ mod tests {
 			System::set_block_number(1);
 			let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| unreachable!());
 			assert!(!result.dispatch_fee_paid_during_dispatch);
+			assert!(result.dispatch_result);
 
 			assert_eq!(
 				System::events(),
@@ -823,6 +862,7 @@ mod tests {
 			System::set_block_number(1);
 			let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| unreachable!());
 			assert!(!result.dispatch_fee_paid_during_dispatch);
+			assert!(result.dispatch_result);
 
 			assert_eq!(
 				System::events(),
@@ -850,6 +890,7 @@ mod tests {
 			System::set_block_number(1);
 			let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| unreachable!());
 			assert!(!result.dispatch_fee_paid_during_dispatch);
+			assert!(result.dispatch_result);
 
 			assert_eq!(
 				System::events(),

modules/messages/Cargo.toml

@@ -7,6 +7,7 @@ edition = "2018"
 license = "GPL-3.0-or-later WITH Classpath-exception-2.0"
 
 [dependencies]
+bitvec = { version = "0.20", default-features = false, features = ["alloc"] }
 codec = { package = "parity-scale-codec", version = "2.0.0", default-features = false }
 log = { version = "0.4.14", default-features = false }
 num-traits = { version = "0.2", default-features = false }

modules/messages/README.md

@@ -101,7 +101,14 @@ the `MessageAccepted` event is emitted in the `send_message()` transaction. The
 message lane identifier and nonce that has been assigned to the message. When a message is delivered
 to the target chain, the `MessagesDelivered` event is emitted from the
 `receive_messages_delivery_proof()` transaction. The `MessagesDelivered` contains the message lane
-identifier and inclusive range of delivered message nonces.
+identifier, inclusive range of delivered message nonces and their single-bit dispatch results.
+
+Please note that the meaning of the 'dispatch result' is determined by the message dispatcher at
+the target chain. For example, in case of immediate call dispatcher it will be the `true` if call
+has been successfully dispatched and `false` if it has only been delivered. This simple mechanism
+built into the messages module allows building basic bridge applications, which only care whether
+their messages have been successfully dispatched or not. More sophisticated applications may use
+their own dispatch result delivery mechanism to deliver something larger than single bit.
 
 ### How to plug-in Messages Module to Send Messages to the Bridged Chain?
 

modules/messages/src/benchmarking.rs

@@ -17,17 +17,25 @@
 //! Messages pallet benchmarking.
 
 use crate::weights_ext::EXPECTED_DEFAULT_MESSAGE_LENGTH;
-use crate::{inbound_lane::InboundLaneStorage, inbound_lane_storage, outbound_lane, Call, Instance};
+use crate::{
+	inbound_lane::InboundLaneStorage, inbound_lane_storage, outbound_lane, outbound_lane::ReceivalConfirmationResult,
+	Call, Instance,
+};
 
 use bp_messages::{
-	source_chain::TargetHeaderChain, target_chain::SourceHeaderChain, InboundLaneData, LaneId, MessageData,
-	MessageNonce, OutboundLaneData, UnrewardedRelayersState,
+	source_chain::TargetHeaderChain, target_chain::SourceHeaderChain, DeliveredMessages, InboundLaneData, LaneId,
+	MessageData, MessageNonce, OutboundLaneData, UnrewardedRelayer, UnrewardedRelayersState,
 };
 use bp_runtime::messages::DispatchFeePayment;
 use frame_benchmarking::{account, benchmarks_instance};
 use frame_support::{traits::Get, weights::Weight};
 use frame_system::RawOrigin;
-use sp_std::{collections::btree_map::BTreeMap, convert::TryInto, ops::RangeInclusive, prelude::*};
+use sp_std::{
+	collections::{btree_map::BTreeMap, vec_deque::VecDeque},
+	convert::TryInto,
+	ops::RangeInclusive,
+	prelude::*,
+};
 
 /// Fee paid by submitter for single message delivery.
 pub const MESSAGE_FEE: u64 = 10_000_000_000;
@@ -471,7 +479,10 @@ benchmarks_instance! {
 		let proof = T::prepare_message_delivery_proof(MessageDeliveryProofParams {
 			lane: T::bench_lane_id(),
 			inbound_lane_data: InboundLaneData {
-				relayers: vec![(1, 1, relayer_id.clone())].into_iter().collect(),
+				relayers: vec![UnrewardedRelayer {
+					relayer: relayer_id.clone(),
+					messages: DeliveredMessages::new(1, true),
+				}].into_iter().collect(),
 				last_confirmed_nonce: 0,
 			},
 			size: ProofSize::Minimal(0),
@@ -506,10 +517,15 @@ benchmarks_instance! {
 			messages_in_oldest_entry: 2,
 			total_messages: 2,
 		};
+		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 {
-				relayers: vec![(1, 2, relayer_id.clone())].into_iter().collect(),
+				relayers: vec![UnrewardedRelayer {
+					relayer: relayer_id.clone(),
+					messages: delivered_messages,
+				}].into_iter().collect(),
 				last_confirmed_nonce: 0,
 			},
 			size: ProofSize::Minimal(0),
@@ -547,8 +563,14 @@ benchmarks_instance! {
 			lane: T::bench_lane_id(),
 			inbound_lane_data: InboundLaneData {
 				relayers: vec![
-					(1, 1, relayer1_id.clone()),
-					(2, 2, relayer2_id.clone()),
+					UnrewardedRelayer {
+						relayer: relayer1_id.clone(),
+						messages: DeliveredMessages::new(1, true),
+					},
+					UnrewardedRelayer {
+						relayer: relayer2_id.clone(),
+						messages: DeliveredMessages::new(2, true),
+					},
 				].into_iter().collect(),
 				last_confirmed_nonce: 0,
 			},
@@ -783,10 +805,17 @@ benchmarks_instance! {
 			messages_in_oldest_entry: 1,
 			total_messages: i as MessageNonce,
 		};
+		let mut delivered_messages = DeliveredMessages::new(1, true);
+		for nonce in 2..=i {
+			delivered_messages.note_dispatched_message(true);
+		}
 		let proof = T::prepare_message_delivery_proof(MessageDeliveryProofParams {
 			lane: T::bench_lane_id(),
 			inbound_lane_data: InboundLaneData {
-				relayers: vec![(1, i as MessageNonce, relayer_id.clone())].into_iter().collect(),
+				relayers: vec![UnrewardedRelayer {
+					relayer: relayer_id.clone(),
+					messages: delivered_messages,
+				}].into_iter().collect(),
 				last_confirmed_nonce: 0,
 			},
 			size: ProofSize::Minimal(0),
@@ -831,7 +860,10 @@ benchmarks_instance! {
 				relayers: relayers
 					.keys()
 					.enumerate()
-					.map(|(j, relayer_id)| (j as MessageNonce + 1, j as MessageNonce + 1, relayer_id.clone()))
+					.map(|(j, relayer)| UnrewardedRelayer {
+						relayer: relayer.clone(),
+						messages: DeliveredMessages::new(j as MessageNonce + 1, true),
+					})
 					.collect(),
 				last_confirmed_nonce: 0,
 			},
@@ -863,13 +895,29 @@ fn send_regular_message_with_payload<T: Config<I>, I: Instance>(payload: Vec<u8>
 
 fn confirm_message_delivery<T: Config<I>, I: Instance>(nonce: MessageNonce) {
 	let mut outbound_lane = outbound_lane::<T, I>(T::bench_lane_id());
-	assert!(outbound_lane.confirm_delivery(nonce).is_some());
+	let latest_received_nonce = outbound_lane.data().latest_received_nonce;
+	let mut relayers = VecDeque::with_capacity((nonce - latest_received_nonce) as usize);
+	for nonce in latest_received_nonce + 1..=nonce {
+		relayers.push_back(UnrewardedRelayer {
+			relayer: (),
+			messages: DeliveredMessages::new(nonce, true),
+		});
+	}
+	assert!(matches!(
+		outbound_lane.confirm_delivery(nonce, &relayers),
+		ReceivalConfirmationResult::ConfirmedMessages(_),
+	));
 }
 
 fn receive_messages<T: Config<I>, I: Instance>(nonce: MessageNonce) {
 	let mut inbound_lane_storage = inbound_lane_storage::<T, I>(T::bench_lane_id());
 	inbound_lane_storage.set_data(InboundLaneData {
-		relayers: vec![(1, nonce, T::bridged_relayer_id())].into_iter().collect(),
+		relayers: vec![UnrewardedRelayer {
+			relayer: T::bridged_relayer_id(),
+			messages: DeliveredMessages::new(nonce, true),
+		}]
+		.into_iter()
+		.collect(),
 		last_confirmed_nonce: 0,
 	});
 }