Propagate message verification errors (paritytech#2114)

* Propagate message verification errors * Replace parse_finalized_storage_proof() with storage_proof_checker() * small fixes * fix comment
serban300 · Apr 9, 2024 · 378edb0 · 378edb0
1 parent 27ddb6c
commit 378edb0
Show file tree

Hide file tree

Showing 10 changed files with 277 additions and 304 deletions.
diff --git a/bridges/bin/runtime-common/src/messages.rs b/bridges/bin/runtime-common/src/messages.rs
diff --git a/bridges/modules/grandpa/src/lib.rs b/bridges/modules/grandpa/src/lib.rs
@@ -1212,11 +1212,8 @@ mod tests {
 	fn parse_finalized_storage_proof_rejects_proof_on_unknown_header() {
 		run_test(|| {
 			assert_noop!(
-				Pallet::<TestRuntime>::parse_finalized_storage_proof(
-					Default::default(),
-					vec![],
-					|_| (),
-				),
+				Pallet::<TestRuntime>::storage_proof_checker(Default::default(), vec![],)
+					.map(|_| ()),
 				bp_header_chain::HeaderChainError::UnknownHeader,
 			);
 		});
@@ -1235,8 +1232,7 @@ mod tests {
 			<ImportedHeaders<TestRuntime>>::insert(hash, header.build());
 
 			assert_ok!(
-				Pallet::<TestRuntime>::parse_finalized_storage_proof(hash, storage_proof, |_| (),),
-				(),
+				Pallet::<TestRuntime>::storage_proof_checker(hash, storage_proof).map(|_| ())
 			);
 		});
 	}

diff --git a/bridges/modules/messages/src/lib.rs b/bridges/modules/messages/src/lib.rs
@@ -61,7 +61,7 @@ use bp_messages::{
 	},
 	total_unrewarded_messages, DeliveredMessages, InboundLaneData, InboundMessageDetails, LaneId,
 	MessageKey, MessageNonce, MessagePayload, MessagesOperatingMode, OutboundLaneData,
-	OutboundMessageDetails, UnrewardedRelayersState,
+	OutboundMessageDetails, UnrewardedRelayersState, VerificationError,
 };
 use bp_runtime::{BasicOperatingMode, ChainId, OwnedBridgeModule, PreComputedSize, Size};
 use codec::{Decode, Encode, MaxEncodedLen};
@@ -557,9 +557,9 @@ pub mod pallet {
 		/// The message is too large to be sent over the bridge.
 		MessageIsTooLarge,
 		/// Message has been treated as invalid by chain verifier.
-		MessageRejectedByChainVerifier,
+		MessageRejectedByChainVerifier(VerificationError),
 		/// Message has been treated as invalid by lane verifier.
-		MessageRejectedByLaneVerifier,
+		MessageRejectedByLaneVerifier(VerificationError),
 		/// Submitter has failed to pay fee for delivering and dispatching messages.
 		FailedToWithdrawMessageFee,
 		/// The transaction brings too many messages.
@@ -732,7 +732,7 @@ fn send_message<T: Config<I>, I: 'static>(
 			err,
 		);
 
-		Error::<T, I>::MessageRejectedByChainVerifier
+		Error::<T, I>::MessageRejectedByChainVerifier(err)
 	})?;
 
 	// now let's enforce any additional lane rules
@@ -746,7 +746,7 @@ fn send_message<T: Config<I>, I: 'static>(
 				err,
 			);
 
-			Error::<T, I>::MessageRejectedByLaneVerifier
+			Error::<T, I>::MessageRejectedByLaneVerifier(err)
 		},
 	)?;
 
@@ -918,7 +918,7 @@ impl<T: Config<I>, I: 'static> OutboundLaneStorage for RuntimeOutboundLaneStorag
 fn verify_and_decode_messages_proof<Chain: SourceHeaderChain, DispatchPayload: Decode>(
 	proof: Chain::MessagesProof,
 	messages_count: u32,
-) -> Result<ProvedMessages<DispatchMessage<DispatchPayload>>, Chain::Error> {
+) -> Result<ProvedMessages<DispatchMessage<DispatchPayload>>, VerificationError> {
 	// `receive_messages_proof` weight formula and `MaxUnconfirmedMessagesAtInboundLane` check
 	// guarantees that the `message_count` is sane and Vec<Message> may be allocated.
 	// (tx with too many messages will either be rejected from the pool, or will fail earlier)
@@ -1177,7 +1177,9 @@ mod tests {
 					TEST_LANE_ID,
 					PAYLOAD_REJECTED_BY_TARGET_CHAIN,
 				),
-				Error::<TestRuntime, ()>::MessageRejectedByChainVerifier,
+				Error::<TestRuntime, ()>::MessageRejectedByChainVerifier(VerificationError::Other(
+					mock::TEST_ERROR
+				)),
 			);
 		});
 	}
@@ -1190,7 +1192,9 @@ mod tests {
 			message.reject_by_lane_verifier = true;
 			assert_noop!(
 				send_message::<TestRuntime, ()>(RuntimeOrigin::signed(1), TEST_LANE_ID, message,),
-				Error::<TestRuntime, ()>::MessageRejectedByLaneVerifier,
+				Error::<TestRuntime, ()>::MessageRejectedByLaneVerifier(VerificationError::Other(
+					mock::TEST_ERROR
+				)),
 			);
 		});
 	}

diff --git a/bridges/modules/messages/src/mock.rs b/bridges/modules/messages/src/mock.rs
@@ -27,7 +27,7 @@ use bp_messages::{
 		ProvedLaneMessages, ProvedMessages, SourceHeaderChain,
 	},
 	DeliveredMessages, InboundLaneData, LaneId, Message, MessageKey, MessageNonce, MessagePayload,
-	OutboundLaneData, UnrewardedRelayer, UnrewardedRelayersState,
+	OutboundLaneData, UnrewardedRelayer, UnrewardedRelayersState, VerificationError,
 };
 use bp_runtime::{messages::MessageDispatchResult, Size};
 use codec::{Decode, Encode};
@@ -295,22 +295,20 @@ impl Size for TestMessagesDeliveryProof {
 pub struct TestTargetHeaderChain;
 
 impl TargetHeaderChain<TestPayload, TestRelayer> for TestTargetHeaderChain {
-	type Error = &'static str;
-
 	type MessagesDeliveryProof = TestMessagesDeliveryProof;
 
-	fn verify_message(payload: &TestPayload) -> Result<(), Self::Error> {
+	fn verify_message(payload: &TestPayload) -> Result<(), VerificationError> {
 		if *payload == PAYLOAD_REJECTED_BY_TARGET_CHAIN {
-			Err(TEST_ERROR)
+			Err(VerificationError::Other(TEST_ERROR))
 		} else {
 			Ok(())
 		}
 	}
 
 	fn verify_messages_delivery_proof(
 		proof: Self::MessagesDeliveryProof,
-	) -> Result<(LaneId, InboundLaneData<TestRelayer>), Self::Error> {
-		proof.0.map_err(|_| TEST_ERROR)
+	) -> Result<(LaneId, InboundLaneData<TestRelayer>), VerificationError> {
+		proof.0.map_err(|_| VerificationError::Other(TEST_ERROR))
 	}
 }
 
@@ -319,18 +317,16 @@ impl TargetHeaderChain<TestPayload, TestRelayer> for TestTargetHeaderChain {
 pub struct TestLaneMessageVerifier;
 
 impl LaneMessageVerifier<RuntimeOrigin, TestPayload> for TestLaneMessageVerifier {
-	type Error = &'static str;
-
 	fn verify_message(
 		_submitter: &RuntimeOrigin,
 		_lane: &LaneId,
 		_lane_outbound_data: &OutboundLaneData,
 		payload: &TestPayload,
-	) -> Result<(), Self::Error> {
+	) -> Result<(), VerificationError> {
 		if !payload.reject_by_lane_verifier {
 			Ok(())
 		} else {
-			Err(TEST_ERROR)
+			Err(VerificationError::Other(TEST_ERROR))
 		}
 	}
 }
@@ -400,15 +396,16 @@ impl DeliveryConfirmationPayments<AccountId> for TestDeliveryConfirmationPayment
 pub struct TestSourceHeaderChain;
 
 impl SourceHeaderChain for TestSourceHeaderChain {
-	type Error = &'static str;
-
 	type MessagesProof = TestMessagesProof;
 
 	fn verify_messages_proof(
 		proof: Self::MessagesProof,
 		_messages_count: u32,
-	) -> Result<ProvedMessages<Message>, Self::Error> {
-		proof.result.map(|proof| proof.into_iter().collect()).map_err(|_| TEST_ERROR)
+	) -> Result<ProvedMessages<Message>, VerificationError> {
+		proof
+			.result
+			.map(|proof| proof.into_iter().collect())
+			.map_err(|_| VerificationError::Other(TEST_ERROR))
 	}
 }