Add unit tests for the equivocation detection loop (#2571)

* Add unit tests for the equivocation detection loop

* clippy

* use std::future::pending()

primitives/header-chain/src/justification/verification/mod.rs

@@ -143,6 +143,7 @@ pub enum PrecommitError {
 }
 
 /// The context needed for validating GRANDPA finality proofs.
+#[derive(RuntimeDebug)]
 pub struct JustificationVerificationContext {
 	/// The authority set used to verify the justification.
 	pub voter_set: VoterSet<AuthorityId>,

relays/equivocation/Cargo.toml

@@ -7,7 +7,7 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0"
 description = "Equivocation detector"
 
 [dependencies]
-async-std = "1.6.5"
+async-std = { version = "1.6.5", features = ["attributes"] }
 async-trait = "0.1"
 bp-header-chain = { path = "../../primitives/header-chain" }
 finality-relay = { path = "../finality" }

relays/equivocation/src/block_checker.rs

@@ -28,6 +28,7 @@ use num_traits::Saturating;
 ///
 /// Getting the finality info associated to the source headers synced with the target chain
 /// at the specified block.
+#[cfg_attr(test, derive(Debug, PartialEq))]
 pub struct ReadSyncedHeaders<P: EquivocationDetectionPipeline> {
 	pub target_block_num: P::TargetNumber,
 }
@@ -61,6 +62,7 @@ impl<P: EquivocationDetectionPipeline> ReadSyncedHeaders<P> {
 /// Second step in the block checking state machine.
 ///
 /// Reading the equivocation reporting context from the target chain.
+#[cfg_attr(test, derive(Debug))]
 pub struct ReadContext<P: EquivocationDetectionPipeline> {
 	target_block_num: P::TargetNumber,
 	synced_headers: Vec<HeaderFinalityInfo<P>>,
@@ -104,6 +106,7 @@ impl<P: EquivocationDetectionPipeline> ReadContext<P> {
 /// Third step in the block checking state machine.
 ///
 /// Searching for equivocations in the source headers synced with the target chain.
+#[cfg_attr(test, derive(Debug))]
 pub struct FindEquivocations<P: EquivocationDetectionPipeline> {
 	target_block_num: P::TargetNumber,
 	synced_headers: Vec<HeaderFinalityInfo<P>>,
@@ -122,10 +125,13 @@ impl<P: EquivocationDetectionPipeline> FindEquivocations<P> {
 				&synced_header.finality_proof,
 				finality_proofs_buf.buf().as_slice(),
 			) {
-				Ok(equivocations) => result.push(ReportEquivocations {
-					source_block_hash: self.context.synced_header_hash,
-					equivocations,
-				}),
+				Ok(equivocations) =>
+					if !equivocations.is_empty() {
+						result.push(ReportEquivocations {
+							source_block_hash: self.context.synced_header_hash,
+							equivocations,
+						})
+					},
 				Err(e) => {
 					log::error!(
 						target: "bridge",
@@ -148,6 +154,7 @@ impl<P: EquivocationDetectionPipeline> FindEquivocations<P> {
 /// Fourth step in the block checking state machine.
 ///
 /// Reporting the detected equivocations (if any).
+#[cfg_attr(test, derive(Debug))]
 pub struct ReportEquivocations<P: EquivocationDetectionPipeline> {
 	source_block_hash: P::Hash,
 	equivocations: Vec<P::EquivocationProof>,
@@ -157,7 +164,7 @@ impl<P: EquivocationDetectionPipeline> ReportEquivocations<P> {
 	pub async fn next<SC: SourceClient<P>>(
 		mut self,
 		source_client: &mut SC,
-		reporter: &mut EquivocationsReporter<P, SC>,
+		reporter: &mut EquivocationsReporter<'_, P, SC>,
 	) -> Result<(), Self> {
 		let mut unprocessed_equivocations = vec![];
 		for equivocation in self.equivocations {
@@ -191,6 +198,7 @@ impl<P: EquivocationDetectionPipeline> ReportEquivocations<P> {
 }
 
 /// Block checking state machine.
+#[cfg_attr(test, derive(Debug))]
 pub enum BlockChecker<P: EquivocationDetectionPipeline> {
 	ReadSyncedHeaders(ReadSyncedHeaders<P>),
 	ReadContext(ReadContext<P>),
@@ -250,3 +258,214 @@ impl<P: EquivocationDetectionPipeline> BlockChecker<P> {
 		.boxed()
 	}
 }
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+	use crate::mock::*;
+	use std::collections::HashMap;
+
+	impl PartialEq for ReadContext<TestEquivocationDetectionPipeline> {
+		fn eq(&self, other: &Self) -> bool {
+			self.target_block_num == other.target_block_num &&
+				self.synced_headers == other.synced_headers
+		}
+	}
+
+	impl PartialEq for FindEquivocations<TestEquivocationDetectionPipeline> {
+		fn eq(&self, other: &Self) -> bool {
+			self.target_block_num == other.target_block_num &&
+				self.synced_headers == other.synced_headers &&
+				self.context == other.context
+		}
+	}
+
+	impl PartialEq for ReportEquivocations<TestEquivocationDetectionPipeline> {
+		fn eq(&self, other: &Self) -> bool {
+			self.source_block_hash == other.source_block_hash &&
+				self.equivocations == other.equivocations
+		}
+	}
+
+	impl PartialEq for BlockChecker<TestEquivocationDetectionPipeline> {
+		fn eq(&self, _other: &Self) -> bool {
+			matches!(self, _other)
+		}
+	}
+
+	#[async_std::test]
+	async fn block_checker_works() {
+		let mut source_client = TestSourceClient { ..Default::default() };
+		let mut target_client = TestTargetClient {
+			best_synced_header_hash: HashMap::from([(9, Ok(Some(5)))]),
+			finality_verification_context: HashMap::from([(
+				9,
+				Ok(TestFinalityVerificationContext { check_equivocations: true }),
+			)]),
+			synced_headers_finality_info: HashMap::from([(
+				10,
+				Ok(vec![
+					new_header_finality_info(6, None),
+					new_header_finality_info(7, Some(false)),
+					new_header_finality_info(8, None),
+					new_header_finality_info(9, Some(true)),
+					new_header_finality_info(10, None),
+					new_header_finality_info(11, None),
+					new_header_finality_info(12, None),
+				]),
+			)]),
+			..Default::default()
+		};
+		let mut reporter =
+			EquivocationsReporter::<TestEquivocationDetectionPipeline, TestSourceClient>::new();
+
+		let block_checker = BlockChecker::new(10);
+		assert!(block_checker
+			.run(
+				&mut source_client,
+				&mut target_client,
+				&mut FinalityProofsBuf::new(vec![
+					TestFinalityProof(6, vec!["6-1"]),
+					TestFinalityProof(7, vec![]),
+					TestFinalityProof(8, vec!["8-1"]),
+					TestFinalityProof(9, vec!["9-1"]),
+					TestFinalityProof(10, vec![]),
+					TestFinalityProof(11, vec!["11-1", "11-2"]),
+					TestFinalityProof(12, vec!["12-1"])
+				]),
+				&mut reporter
+			)
+			.await
+			.is_ok());
+		assert_eq!(
+			*source_client.reported_equivocations.lock().unwrap(),
+			HashMap::from([(5, vec!["6-1"]), (9, vec!["11-1", "11-2", "12-1"])])
+		);
+	}
+
+	#[async_std::test]
+	async fn block_checker_works_with_empty_context() {
+		let mut target_client = TestTargetClient {
+			best_synced_header_hash: HashMap::from([(9, Ok(None))]),
+			finality_verification_context: HashMap::from([(
+				9,
+				Ok(TestFinalityVerificationContext { check_equivocations: true }),
+			)]),
+			synced_headers_finality_info: HashMap::from([(
+				10,
+				Ok(vec![new_header_finality_info(6, None)]),
+			)]),
+			..Default::default()
+		};
+		let mut source_client = TestSourceClient { ..Default::default() };
+		let mut reporter =
+			EquivocationsReporter::<TestEquivocationDetectionPipeline, TestSourceClient>::new();
+
+		let block_checker = BlockChecker::new(10);
+		assert!(block_checker
+			.run(
+				&mut source_client,
+				&mut target_client,
+				&mut FinalityProofsBuf::new(vec![TestFinalityProof(6, vec!["6-1"])]),
+				&mut reporter
+			)
+			.await
+			.is_ok());
+		assert_eq!(*source_client.reported_equivocations.lock().unwrap(), HashMap::default());
+	}
+
+	#[async_std::test]
+	async fn read_synced_headers_handles_errors() {
+		let mut target_client = TestTargetClient {
+			synced_headers_finality_info: HashMap::from([
+				(10, Err(TestClientError::NonConnection)),
+				(11, Err(TestClientError::Connection)),
+			]),
+			..Default::default()
+		};
+		let mut source_client = TestSourceClient { ..Default::default() };
+		let mut reporter =
+			EquivocationsReporter::<TestEquivocationDetectionPipeline, TestSourceClient>::new();
+
+		// NonConnection error
+		let block_checker = BlockChecker::new(10);
+		assert_eq!(
+			block_checker
+				.run(
+					&mut source_client,
+					&mut target_client,
+					&mut FinalityProofsBuf::new(vec![]),
+					&mut reporter
+				)
+				.await,
+			Err(BlockChecker::ReadSyncedHeaders(ReadSyncedHeaders { target_block_num: 10 }))
+		);
+		assert_eq!(target_client.num_reconnects, 0);
+
+		// Connection error
+		let block_checker = BlockChecker::new(11);
+		assert_eq!(
+			block_checker
+				.run(
+					&mut source_client,
+					&mut target_client,
+					&mut FinalityProofsBuf::new(vec![]),
+					&mut reporter
+				)
+				.await,
+			Err(BlockChecker::ReadSyncedHeaders(ReadSyncedHeaders { target_block_num: 11 }))
+		);
+		assert_eq!(target_client.num_reconnects, 1);
+	}
+
+	#[async_std::test]
+	async fn read_context_handles_errors() {
+		let mut target_client = TestTargetClient {
+			synced_headers_finality_info: HashMap::from([(10, Ok(vec![])), (11, Ok(vec![]))]),
+			best_synced_header_hash: HashMap::from([
+				(9, Err(TestClientError::NonConnection)),
+				(10, Err(TestClientError::Connection)),
+			]),
+			..Default::default()
+		};
+		let mut source_client = TestSourceClient { ..Default::default() };
+		let mut reporter =
+			EquivocationsReporter::<TestEquivocationDetectionPipeline, TestSourceClient>::new();
+
+		// NonConnection error
+		let block_checker = BlockChecker::new(10);
+		assert_eq!(
+			block_checker
+				.run(
+					&mut source_client,
+					&mut target_client,
+					&mut FinalityProofsBuf::new(vec![]),
+					&mut reporter
+				)
+				.await,
+			Err(BlockChecker::ReadContext(ReadContext {
+				target_block_num: 10,
+				synced_headers: vec![]
+			}))
+		);
+		assert_eq!(target_client.num_reconnects, 0);
+
+		// Connection error
+		let block_checker = BlockChecker::new(11);
+		assert_eq!(
+			block_checker
+				.run(
+					&mut source_client,
+					&mut target_client,
+					&mut FinalityProofsBuf::new(vec![]),
+					&mut reporter
+				)
+				.await,
+			Err(BlockChecker::ReadContext(ReadContext {
+				target_block_num: 11,
+				synced_headers: vec![]
+			}))
+		);
+		assert_eq!(target_client.num_reconnects, 1);
+	}
+}