core: import equivocated aura and babe blocks (#2709)

* core: import equivocated aura and babe blocks

* core: cleanup check_equivocation handling

* fix: use map_err on Aura

* core: slots: remove unneeded Arc and minimize cloning

* core: fix slots equivocation tests

* core: slots: remove unused import

* core: remove unnecessary comments

substrate/Cargo.lock

@@ -4061,6 +4061,7 @@ dependencies = [
  "substrate-consensus-common 2.0.0",
  "substrate-inherents 2.0.0",
  "substrate-primitives 2.0.0",
+ "substrate-test-client 2.0.0",
  "tokio 0.1.19 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 

substrate/core/consensus/aura/src/lib.rs

@@ -395,7 +395,7 @@ fn find_pre_digest<B: Block, P: Pair>(header: &B::Header) -> Result<u64, String>
 //
 // FIXME #1018 needs misbehavior types
 fn check_header<C, B: Block, P: Pair>(
-	client: &Arc<C>,
+	client: &C,
 	slot_now: u64,
 	mut header: B::Header,
 	hash: B::Hash,
@@ -431,26 +431,22 @@ fn check_header<C, B: Block, P: Pair>(
 		let pre_hash = header.hash();
 
 		if P::verify(&sig, pre_hash.as_ref(), expected_author) {
-			match check_equivocation::<_, _, <P as Pair>::Public>(
+			if let Some(equivocation_proof) = check_equivocation(
 				client,
 				slot_now,
 				slot_num,
-				header.clone(),
-				expected_author.clone(),
-			) {
-				Ok(Some(equivocation_proof)) => {
-					let log_str = format!(
-						"Slot author is equivocating at slot {} with headers {:?} and {:?}",
-						slot_num,
-						equivocation_proof.fst_header().hash(),
-						equivocation_proof.snd_header().hash(),
-					);
-					info!("{}", log_str);
-					Err(log_str)
-				},
-				Ok(None) => Ok(CheckedHeader::Checked(header, (slot_num, seal))),
-				Err(e) => Err(e.to_string()),
+				&header,
+				expected_author,
+			).map_err(|e| e.to_string())? {
+				info!(
+					"Slot author is equivocating at slot {} with headers {:?} and {:?}",
+					slot_num,
+					equivocation_proof.fst_header().hash(),
+					equivocation_proof.snd_header().hash(),
+				);
 			}
+
+			Ok(CheckedHeader::Checked(header, (slot_num, seal)))
 		} else {
 			Err(format!("Bad signature on {:?}", hash))
 		}
@@ -766,9 +762,6 @@ mod tests {
 	use primitives::sr25519;
 	use client::{LongestChain, BlockchainEvents};
 	use test_client;
-	use primitives::hash::H256;
-	use runtime_primitives::testing::{Header as HeaderTest, Digest as DigestTest, Block as RawBlock, ExtrinsicWrapper};
-	use slots::{MAX_SLOT_CAPACITY, PRUNING_BOUND};
 
 	type Error = client::error::Error;
 
@@ -873,24 +866,6 @@ mod tests {
 		}
 	}
 
-	fn create_header(slot_num: u64, number: u64, pair: &sr25519::Pair) -> (HeaderTest, H256) {
-		let mut header = HeaderTest {
-			parent_hash: Default::default(),
-			number,
-			state_root: Default::default(),
-			extrinsics_root: Default::default(),
-			digest: DigestTest { logs: vec![], },
-		};
-		let item = <DigestItemFor<TestBlock> as CompatibleDigestItem<sr25519::Pair>>::aura_pre_digest(slot_num);
-		header.digest_mut().push(item);
-		let header_hash: H256 = header.hash();
-		let signature = pair.sign(&header_hash[..]);
-
-		let item = CompatibleDigestItem::<sr25519::Pair>::aura_seal(signature);
-		header.digest_mut().push(item);
-		(header, header_hash)
-	}
-
 	#[test]
 	#[allow(deprecated)]
 	fn authoring_blocks() {
@@ -975,43 +950,4 @@ mod tests {
 			Keyring::Charlie.into()
 		]);
 	}
-
-	#[test]
-	fn check_header_works_with_equivocation() {
-		let client = test_client::new();
-		let pair = sr25519::Pair::generate();
-		let public = pair.public();
-		let authorities = vec![public.clone(), sr25519::Pair::generate().public()];
-
-		let (header1, header1_hash) = create_header(2, 1, &pair);
-		let (header2, header2_hash) = create_header(2, 2, &pair);
-		let (header3, header3_hash) = create_header(4, 2, &pair);
-		let (header4, header4_hash) = create_header(MAX_SLOT_CAPACITY + 4, 3, &pair);
-		let (header5, header5_hash) = create_header(MAX_SLOT_CAPACITY + 4, 4, &pair);
-		let (header6, header6_hash) = create_header(4, 3, &pair);
-
-		type B = RawBlock<ExtrinsicWrapper<u64>>;
-		type P = sr25519::Pair;
-
-		let c = Arc::new(client);
-
-		// It's ok to sign same headers.
-		check_header::<_, B, P>(&c, 2, header1.clone(), header1_hash, &authorities).unwrap();
-		assert!(check_header::<_, B, P>(&c, 3, header1, header1_hash, &authorities).is_ok());
-
-		// But not two different headers at the same slot.
-		assert!(check_header::<_, B, P>(&c, 4, header2, header2_hash, &authorities).is_err());
-
-		// Different slot is ok.
-		assert!(check_header::<_, B, P>(&c, 5, header3, header3_hash, &authorities).is_ok());
-
-		// Here we trigger pruning and save header 4.
-		assert!(check_header::<_, B, P>(&c, PRUNING_BOUND + 2, header4, header4_hash, &authorities).is_ok());
-
-		// This fails because header 5 is an equivocation of header 4.
-		assert!(check_header::<_, B, P>(&c, PRUNING_BOUND + 3, header5, header5_hash, &authorities).is_err());
-
-		// This is ok because we pruned the corresponding header. Shows that we are pruning.
-		assert!(check_header::<_, B, P>(&c, PRUNING_BOUND + 4, header6, header6_hash, &authorities).is_ok());
-	}
 }

substrate/core/consensus/babe/src/lib.rs

@@ -461,7 +461,7 @@ fn find_pre_digest<B: Block>(header: &B::Header) -> Result<BabePreDigest, String
 // FIXME #1018 needs misbehavior types
 #[forbid(warnings)]
 fn check_header<B: Block + Sized, C: AuxStore>(
-	client: &Arc<C>,
+	client: &C,
 	slot_now: u64,
 	mut header: B::Header,
 	hash: B::Hash,
@@ -508,28 +508,27 @@ fn check_header<B: Block + Sized, C: AuxStore>(
 
 			if !check(&inout, threshold) {
 				return Err(babe_err!("VRF verification of block by author {:?} failed: \
-									threshold {} exceeded", author, threshold))
+									  threshold {} exceeded", author, threshold));
 			}
-			match check_equivocation(&client, slot_now, slot_num, header.clone(), author.clone()) {
-				Ok(Some(equivocation_proof)) => {
-					let log_str = format!(
-						"Slot author {:?} is equivocating at slot {} with headers {:?} and {:?}",
-						author,
-						slot_num,
-						equivocation_proof.fst_header().hash(),
-						equivocation_proof.snd_header().hash(),
-					);
-					info!(target: "babe", "{}", log_str);
-					Err(log_str)
-				},
-				Ok(None) => {
-					let pre_digest = CompatibleDigestItem::babe_pre_digest(pre_digest);
-					Ok(CheckedHeader::Checked(header, (pre_digest, seal)))
-				},
-				Err(e) => {
-					Err(e.to_string())
-				},
+
+			if let Some(equivocation_proof) = check_equivocation(
+				client,
+				slot_now,
+				slot_num,
+				&header,
+				author,
+			).map_err(|e| e.to_string())? {
+				info!(
+					"Slot author {:?} is equivocating at slot {} with headers {:?} and {:?}",
+					author,
+					slot_num,
+					equivocation_proof.fst_header().hash(),
+					equivocation_proof.snd_header().hash(),
+				);
 			}
+
+			let pre_digest = CompatibleDigestItem::babe_pre_digest(pre_digest);
+			Ok(CheckedHeader::Checked(header, (pre_digest, seal)))
 		} else {
 			Err(babe_err!("Bad signature on {:?}", hash))
 		}
@@ -823,9 +822,6 @@ mod tests {
 	use std::time::Duration;
 	type Item = generic::DigestItem<Hash, Public, Signature>;
 	use test_client::AuthorityKeyring;
-	use primitives::hash::H256;
-	use runtime_primitives::testing::{Header as HeaderTest, Digest as DigestTest, Block as RawBlock, ExtrinsicWrapper};
-	use slots::{MAX_SLOT_CAPACITY, PRUNING_BOUND};
 
 	type Error = client::error::Error;
 
@@ -934,44 +930,6 @@ mod tests {
 		}
 	}
 
-	fn create_header(slot_num: u64, number: u64, pair: &sr25519::Pair) -> (HeaderTest, H256) {
-		let mut header = HeaderTest {
-			parent_hash: Default::default(),
-			number,
-			state_root: Default::default(),
-			extrinsics_root: Default::default(),
-			digest: DigestTest { logs: vec![], },
-		};
-
-		let transcript = make_transcript(
-			Default::default(),
-			slot_num,
-			Default::default(),
-			0,
-		);
-
-		let (inout, proof, _batchable_proof) = get_keypair(&pair).vrf_sign_n_check(transcript, |inout| check(inout, u64::MAX)).unwrap();
-		let pre_hash: H256 = header.hash();
-		let pre_digest = BabePreDigest {
-			proof,
-			author: pair.public(),
-			slot_num,
-			vrf_output: inout.to_output(),
-		};
-		assert_eq!(
-			Decode::decode(&mut &pre_digest.encode()[..]).as_ref(),
-			Some(&pre_digest),
-			"Pre-digest encoding and decoding did not round-trip",
-		);
-		let item = generic::DigestItem::babe_pre_digest(pre_digest);
-		header.digest_mut().push(item);
-
-		let to_sign = header.hash();
-		let signature = pair.sign(&to_sign[..]);
-		header.digest_mut().push(generic::DigestItem::babe_seal(signature));
-		(header, pre_hash)
-	}
-
 	#[test]
 	fn can_serialize_block() {
 		drop(env_logger::try_init());
@@ -1107,45 +1065,4 @@ mod tests {
 			Keyring::Charlie.into()
 		]);
 	}
-
-	#[test]
-	fn check_header_works_with_equivocation() {
-		drop(env_logger::try_init());
-		let client = test_client::new();
-		let pair = sr25519::Pair::generate();
-		let public = pair.public();
-		let authorities = vec![public.clone(), sr25519::Pair::generate().public()];
-
-		let (header1, header1_hash) = create_header(2, 1, &pair);
-		let (header2, header2_hash) = create_header(2, 2, &pair);
-		let (header3, header3_hash) = create_header(4, 2, &pair);
-		let (header4, header4_hash) = create_header(MAX_SLOT_CAPACITY + 4, 3, &pair);
-		let (header5, header5_hash) = create_header(MAX_SLOT_CAPACITY + 4, 4, &pair);
-		let (header6, header6_hash) = create_header(4, 3, &pair);
-
-		let c = Arc::new(client);
-		let max = u64::MAX;
-
-		type B = RawBlock<ExtrinsicWrapper<u64>>;
-		type P = sr25519::Pair;
-
-		// It's ok to sign same headers.
-		assert!(check_header::<B, _>(&c, 2, header1.clone(), header1_hash, &authorities, max).is_ok());
-		assert!(check_header::<B, _>(&c, 3, header1, header1_hash, &authorities, max).is_ok());
-
-		// But not two different headers at the same slot.
-		assert!(check_header::<B, _>(&c, 4, header2, header2_hash, &authorities, max).is_err());
-
-		// Different slot is ok.
-		assert!(check_header::<B, _>(&c, 5, header3, header3_hash, &authorities, max).is_ok());
-
-		// Here we trigger pruning and save header 4.
-		assert!(check_header::<B, _>(&c, PRUNING_BOUND + 2, header4, header4_hash, &authorities, max).is_ok());
-
-		// This fails because header 5 is an equivocation of header 4.
-		assert!(check_header::<B, _>(&c, PRUNING_BOUND + 3, header5, header5_hash, &authorities, max).is_err());
-
-		// This is ok because we pruned the corresponding header. Shows that we are pruning.
-		assert!(check_header::<B, _>(&c, PRUNING_BOUND + 4, header6, header6_hash, &authorities, max).is_ok());
-	}
 }

substrate/core/consensus/slots/Cargo.toml

@@ -16,3 +16,6 @@ futures = "0.1.17"
 tokio = "0.1.7"
 parking_lot = "0.7.1"
 log = "0.4"
+
+[dev-dependencies]
+test_client = { package = "substrate-test-client", path = "../../test-client" }

substrate/core/consensus/slots/src/aux_schema.rs

@@ -16,7 +16,6 @@
 
 //! Schema for slots in the aux-db.
 
-use std::sync::Arc;
 use codec::{Encode, Decode};
 use client::backend::AuxStore;
 use client::error::{Result as ClientResult, Error as ClientError};
@@ -30,7 +29,7 @@ pub const MAX_SLOT_CAPACITY: u64 = 1000;
 /// We prune slots when they reach this number.
 pub const PRUNING_BOUND: u64 = 2 * MAX_SLOT_CAPACITY;
 
-fn load_decode<C, T>(backend: Arc<C>, key: &[u8]) -> ClientResult<Option<T>>
+fn load_decode<C, T>(backend: &C, key: &[u8]) -> ClientResult<Option<T>>
 	where
 		C: AuxStore,
 		T: Decode,
@@ -74,16 +73,16 @@ impl<H> EquivocationProof<H> {
 ///
 /// Note: it detects equivocations only when slot_now - slot <= MAX_SLOT_CAPACITY.
 pub fn check_equivocation<C, H, P>(
-	backend: &Arc<C>,
+	backend: &C,
 	slot_now: u64,
 	slot: u64,
-	header: H,
-	signer: P,
+	header: &H,
+	signer: &P,
 ) -> ClientResult<Option<EquivocationProof<H>>>
 	where
 		H: Header,
 		C: AuxStore,
-		P: Encode + Decode + PartialEq,
+		P: Clone + Encode + Decode + PartialEq,
 {
 	// We don't check equivocations for old headers out of our capacity.
 	if slot_now - slot > MAX_SLOT_CAPACITY {
@@ -95,18 +94,18 @@ pub fn check_equivocation<C, H, P>(
 	slot.using_encoded(|s| curr_slot_key.extend(s));
 
 	// Get headers of this slot.
-	let mut headers_with_sig = load_decode::<_, Vec<(H, P)>>(backend.clone(), &curr_slot_key[..])?
+	let mut headers_with_sig = load_decode::<_, Vec<(H, P)>>(backend, &curr_slot_key[..])?
 		.unwrap_or_else(Vec::new);
 
 	// Get first slot saved.
 	let slot_header_start = SLOT_HEADER_START.to_vec();
-	let first_saved_slot = load_decode::<_, u64>(backend.clone(), &slot_header_start[..])?
+	let first_saved_slot = load_decode::<_, u64>(backend, &slot_header_start[..])?
 		.unwrap_or(slot);
 
 	for (prev_header, prev_signer) in headers_with_sig.iter() {
 		// A proof of equivocation consists of two headers:
 		// 1) signed by the same voter,
-		if *prev_signer == signer {
+		if prev_signer == signer {
 			// 2) with different hash
 			if header.hash() != prev_header.hash() {
 				return Ok(Some(EquivocationProof {
@@ -137,7 +136,7 @@ pub fn check_equivocation<C, H, P>(
 		}
 	}
 
-	headers_with_sig.push((header, signer));
+	headers_with_sig.push((header.clone(), signer.clone()));
 
 	backend.insert_aux(
 		&[
@@ -149,3 +148,118 @@ pub fn check_equivocation<C, H, P>(
 
 	Ok(None)
 }
+
+#[cfg(test)]
+mod test {
+	use primitives::{sr25519, Pair};
+	use primitives::hash::H256;
+	use runtime_primitives::testing::{Header as HeaderTest, Digest as DigestTest};
+	use test_client;
+
+	use super::{MAX_SLOT_CAPACITY, PRUNING_BOUND, check_equivocation};
+
+	fn create_header(number: u64) -> HeaderTest {
+		// so that different headers for the same number get different hashes
+		let parent_hash = H256::random();
+
+		let header = HeaderTest {
+			parent_hash,
+			number,
+			state_root: Default::default(),
+			extrinsics_root: Default::default(),
+			digest: DigestTest { logs: vec![], },
+		};
+
+		header
+	}
+
+	#[test]
+	fn check_equivocation_works() {
+		let client = test_client::new();
+		let pair = sr25519::Pair::generate();
+		let public = pair.public();
+
+		let header1 = create_header(1); // @ slot 2
+		let header2 = create_header(2); // @ slot 2
+		let header3 = create_header(2); // @ slot 4
+		let header4 = create_header(3); // @ slot MAX_SLOT_CAPACITY + 4
+		let header5 = create_header(4); // @ slot MAX_SLOT_CAPACITY + 4
+		let header6 = create_header(3); // @ slot 4
+
+		// It's ok to sign same headers.
+		assert!(
+			check_equivocation(
+				&client,
+				2,
+				2,
+				&header1,
+				&public,
+			).unwrap().is_none(),
+		);
+
+		assert!(
+			check_equivocation(
+				&client,
+				3,
+				2,
+				&header1,
+				&public,
+			).unwrap().is_none(),
+		);
+
+		// But not two different headers at the same slot.
+		assert!(
+			check_equivocation(
+				&client,
+				4,
+				2,
+				&header2,
+				&public,
+			).unwrap().is_some(),
+		);
+
+		// Different slot is ok.
+		assert!(
+			check_equivocation(
+				&client,
+				5,
+				4,
+				&header3,
+				&public,
+			).unwrap().is_none(),
+		);
+
+		// Here we trigger pruning and save header 4.
+		assert!(
+			check_equivocation(
+				&client,
+				PRUNING_BOUND + 2,
+				MAX_SLOT_CAPACITY + 4,
+				&header4,
+				&public,
+			).unwrap().is_none(),
+		);
+
+		// This fails because header 5 is an equivocation of header 4.
+		assert!(
+			check_equivocation(
+				&client,
+				PRUNING_BOUND + 3,
+				MAX_SLOT_CAPACITY + 4,
+				&header5,
+				&public,
+			).unwrap().is_some(),
+		);
+
+		// This is ok because we pruned the corresponding header. Shows that we are pruning.
+		assert!(
+			check_equivocation(
+				&client,
+				PRUNING_BOUND + 4,
+				4,
+				&header6,
+				&public,
+			).unwrap().is_none(),
+		);
+	}
+}