fail with InsufficientDispatchWeight if dispatch_weight doesn't cover…

… weight of all bundled messages (#2018)

bridges/modules/grandpa/src/lib.rs

@@ -160,6 +160,16 @@ pub mod pallet {
 		///
 		/// If successful in verification, it will write the target header to the underlying storage
 		/// pallet.
+		///
+		/// The call fails if:
+		///
+		/// - the pallet is halted;
+		///
+		/// - the pallet knows better header than the `finality_target`;
+		///
+		/// - verification is not optimized or invalid;
+		///
+		/// - header contains forced authorities set change or change with non-zero delay.
 		#[pallet::call_index(0)]
 		#[pallet::weight(<T::WeightInfo as WeightInfo>::submit_finality_proof(
 			justification.commit.precommits.len().saturated_into(),

bridges/modules/messages/src/lib.rs

@@ -249,6 +249,22 @@ pub mod pallet {
 		/// The weight of the call assumes that the transaction always brings outbound lane
 		/// state update. Because of that, the submitter (relayer) has no benefit of not including
 		/// this data in the transaction, so reward confirmations lags should be minimal.
+		///
+		/// The call fails if:
+		///
+		/// - the pallet is halted;
+		///
+		/// - the call origin is not `Signed(_)`;
+		///
+		/// - there are too many messages in the proof;
+		///
+		/// - the proof verification procedure returns an error - e.g. because header used to craft
+		///   proof is not imported by the associated finality pallet;
+		///
+		/// - the `dispatch_weight` argument is not sufficient to dispatch all bundled messages.
+		///
+		/// The call may succeed, but some messages may not be delivered e.g. if they are not fit
+		/// into the unrewarded relayers vector.
 		#[pallet::call_index(2)]
 		#[pallet::weight(T::WeightInfo::receive_messages_proof_weight(proof, *messages_count, *dispatch_weight))]
 		pub fn receive_messages_proof(
@@ -324,18 +340,10 @@ pub mod pallet {
 
 				let mut lane_messages_received_status =
 					ReceivedMessages::new(lane_id, Vec::with_capacity(lane_data.messages.len()));
-				let mut is_lane_processing_stopped_no_weight_left = false;
-
 				for mut message in lane_data.messages {
 					debug_assert_eq!(message.key.lane_id, lane_id);
 					total_messages += 1;
 
-					if is_lane_processing_stopped_no_weight_left {
-						lane_messages_received_status
-							.push_skipped_for_not_enough_weight(message.key.nonce);
-						continue
-					}
-
 					// ensure that relayer has declared enough weight for dispatching next message
 					// on this lane. We can't dispatch lane messages out-of-order, so if declared
 					// weight is not enough, let's move to next lane
@@ -348,10 +356,8 @@ pub mod pallet {
 							message_dispatch_weight,
 							dispatch_weight_left,
 						);
-						lane_messages_received_status
-							.push_skipped_for_not_enough_weight(message.key.nonce);
-						is_lane_processing_stopped_no_weight_left = true;
-						continue
+
+						fail!(Error::<T, I>::InsufficientDispatchWeight);
 					}
 
 					let receival_result = lane.receive_message::<T::MessageDispatch, T::AccountId>(
@@ -554,8 +560,9 @@ pub mod pallet {
 		/// The relayer has declared invalid unrewarded relayers state in the
 		/// `receive_messages_delivery_proof` call.
 		InvalidUnrewardedRelayersState,
-		/// The message someone is trying to work with (i.e. increase fee) is already-delivered.
-		MessageIsAlreadyDelivered,
+		/// The cumulative dispatch weight, passed by relayer is not enough to cover dispatch
+		/// of all bundled messages.
+		InsufficientDispatchWeight,
 		/// The message someone is trying to work with (i.e. increase fee) is not yet sent.
 		MessageIsNotYetSent,
 		/// The number of actually confirmed messages is going to be larger than the number of
@@ -1277,13 +1284,16 @@ mod tests {
 		run_test(|| {
 			let mut declared_weight = REGULAR_PAYLOAD.declared_weight;
 			*declared_weight.ref_time_mut() -= 1;
-			assert_ok!(Pallet::<TestRuntime>::receive_messages_proof(
-				RuntimeOrigin::signed(1),
-				TEST_RELAYER_A,
-				Ok(vec![message(1, REGULAR_PAYLOAD)]).into(),
-				1,
-				declared_weight,
-			));
+			assert_noop!(
+				Pallet::<TestRuntime>::receive_messages_proof(
+					RuntimeOrigin::signed(1),
+					TEST_RELAYER_A,
+					Ok(vec![message(1, REGULAR_PAYLOAD)]).into(),
+					1,
+					declared_weight,
+				),
+				Error::<TestRuntime, ()>::InsufficientDispatchWeight
+			);
 			assert_eq!(InboundLanes::<TestRuntime>::get(TEST_LANE_ID).last_delivered_nonce(), 0);
 		});
 	}
@@ -1541,15 +1551,18 @@ mod tests {
 			let message2 = message(2, message_payload(0, u64::MAX / 2));
 			let message3 = message(3, message_payload(0, u64::MAX / 2));
 
-			assert_ok!(Pallet::<TestRuntime, ()>::receive_messages_proof(
-				RuntimeOrigin::signed(1),
-				TEST_RELAYER_A,
-				// this may cause overflow if source chain storage is invalid
-				Ok(vec![message1, message2, message3]).into(),
-				3,
-				Weight::MAX,
-			));
-			assert_eq!(InboundLanes::<TestRuntime>::get(TEST_LANE_ID).last_delivered_nonce(), 2);
+			assert_noop!(
+				Pallet::<TestRuntime, ()>::receive_messages_proof(
+					RuntimeOrigin::signed(1),
+					TEST_RELAYER_A,
+					// this may cause overflow if source chain storage is invalid
+					Ok(vec![message1, message2, message3]).into(),
+					3,
+					Weight::MAX,
+				),
+				Error::<TestRuntime, ()>::InsufficientDispatchWeight
+			);
+			assert_eq!(InboundLanes::<TestRuntime>::get(TEST_LANE_ID).last_delivered_nonce(), 0);
 		});
 	}
 

bridges/modules/parachains/src/lib.rs

@@ -294,6 +294,16 @@ pub mod pallet {
 		/// `polkadot-runtime-parachains::paras` pallet instance, deployed at the bridged chain.
 		/// The proof is supposed to be crafted at the `relay_header_hash` that must already be
 		/// imported by corresponding GRANDPA pallet at this chain.
+		///
+		/// The call fails if:
+		///
+		/// - the pallet is halted;
+		///
+		/// - the relay chain block `at_relay_block` is not imported by the associated bridge
+		///   GRANDPA pallet.
+		///
+		/// The call may succeed, but some heads may not be updated e.g. because pallet knows
+		/// better head or it isn't tracked by the pallet.
 		#[pallet::call_index(0)]
 		#[pallet::weight(WeightInfoOf::<T, I>::submit_parachain_heads_weight(
 			T::DbWeight::get(),

bridges/primitives/messages/src/lib.rs

@@ -238,25 +238,19 @@ pub struct ReceivedMessages<DispatchLevelResult> {
 	pub lane: LaneId,
 	/// Result of messages which we tried to dispatch
 	pub receive_results: Vec<(MessageNonce, ReceivalResult<DispatchLevelResult>)>,
-	/// Messages which were skipped and never dispatched
-	pub skipped_for_not_enough_weight: Vec<MessageNonce>,
 }
 
 impl<DispatchLevelResult> ReceivedMessages<DispatchLevelResult> {
 	pub fn new(
 		lane: LaneId,
 		receive_results: Vec<(MessageNonce, ReceivalResult<DispatchLevelResult>)>,
 	) -> Self {
-		ReceivedMessages { lane, receive_results, skipped_for_not_enough_weight: Vec::new() }
+		ReceivedMessages { lane, receive_results }
 	}
 
 	pub fn push(&mut self, message: MessageNonce, result: ReceivalResult<DispatchLevelResult>) {
 		self.receive_results.push((message, result));
 	}
-
-	pub fn push_skipped_for_not_enough_weight(&mut self, message: MessageNonce) {
-		self.skipped_for_not_enough_weight.push(message);
-	}
 }
 
 /// Result of single message receival.

bridges/relays/client-rialto-parachain/src/codegen_runtime.rs

@@ -6043,7 +6043,6 @@ pub mod api {
 					::core::primitive::u64,
 					runtime_types::bp_messages::ReceivalResult<_0>,
 				)>,
-				pub skipped_for_not_enough_weight: ::std::vec::Vec<::core::primitive::u64>,
 			}
 			#[derive(
 				:: subxt :: ext :: codec :: Decode, :: subxt :: ext :: codec :: Encode, Clone, Debug,
@@ -7508,8 +7507,9 @@ pub mod api {
 					#[doc = "`receive_messages_delivery_proof` call."]
 					InvalidUnrewardedRelayersState,
 					#[codec(index = 11)]
-					#[doc = "The message someone is trying to work with (i.e. increase fee) is already-delivered."]
-					MessageIsAlreadyDelivered,
+					#[doc = "The cumulative dispatch weight, passed by relayer is not enough to cover dispatch"]
+					#[doc = "of all bundled messages."]
+					InsufficientDispatchWeight,
 					#[codec(index = 12)]
 					#[doc = "The message someone is trying to work with (i.e. increase fee) is not yet sent."]
 					MessageIsNotYetSent,