Relay basic single-bit message dispatch results back to the source chain (#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 <[email protected]>

Co-authored-by: Tomasz Drwięga <[email protected]>

Author: svyatonik

Cargo.lock

@@ -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/lib.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/millau/runtime/src/rialto_messages.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/lib.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,

bin/rialto/runtime/src/millau_messages.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/dispatch/src/lib.rs

@@ -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/Cargo.toml

@@ -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?