From 7e591a8f4d116fc39ac7fbc354b1103100c58309 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Wed, 29 May 2019 15:25:02 +0200 Subject: [PATCH] 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 | 1 + substrate/core/consensus/aura/src/lib.rs | 90 ++---------- substrate/core/consensus/babe/src/lib.rs | 123 +++------------- substrate/core/consensus/slots/Cargo.toml | 3 + .../core/consensus/slots/src/aux_schema.rs | 134 ++++++++++++++++-- 5 files changed, 161 insertions(+), 190 deletions(-) diff --git a/substrate/Cargo.lock b/substrate/Cargo.lock index 83a85c4d69..d585910b14 100644 --- a/substrate/Cargo.lock +++ b/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)", ] diff --git a/substrate/core/consensus/aura/src/lib.rs b/substrate/core/consensus/aura/src/lib.rs index e3ab9e31cf..36c51c8cff 100644 --- a/substrate/core/consensus/aura/src/lib.rs +++ b/substrate/core/consensus/aura/src/lib.rs @@ -395,7 +395,7 @@ fn find_pre_digest(header: &B::Header) -> Result // // FIXME #1018 needs misbehavior types fn check_header( - client: &Arc, + client: &C, slot_now: u64, mut header: B::Header, hash: B::Hash, @@ -431,26 +431,22 @@ fn check_header( let pre_hash = header.hash(); if P::verify(&sig, pre_hash.as_ref(), expected_author) { - match check_equivocation::<_, _,

::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 = as CompatibleDigestItem>::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::::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>; - 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()); - } } diff --git a/substrate/core/consensus/babe/src/lib.rs b/substrate/core/consensus/babe/src/lib.rs index 2d1afb0187..81b4be4137 100644 --- a/substrate/core/consensus/babe/src/lib.rs +++ b/substrate/core/consensus/babe/src/lib.rs @@ -461,7 +461,7 @@ fn find_pre_digest(header: &B::Header) -> Result( - client: &Arc, + client: &C, slot_now: u64, mut header: B::Header, hash: B::Hash, @@ -508,28 +508,27 @@ fn check_header( 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; 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>; - type P = sr25519::Pair; - - // It's ok to sign same headers. - assert!(check_header::(&c, 2, header1.clone(), header1_hash, &authorities, max).is_ok()); - assert!(check_header::(&c, 3, header1, header1_hash, &authorities, max).is_ok()); - - // But not two different headers at the same slot. - assert!(check_header::(&c, 4, header2, header2_hash, &authorities, max).is_err()); - - // Different slot is ok. - assert!(check_header::(&c, 5, header3, header3_hash, &authorities, max).is_ok()); - - // Here we trigger pruning and save header 4. - assert!(check_header::(&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::(&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::(&c, PRUNING_BOUND + 4, header6, header6_hash, &authorities, max).is_ok()); - } } diff --git a/substrate/core/consensus/slots/Cargo.toml b/substrate/core/consensus/slots/Cargo.toml index ee3571bcdc..2be9133d54 100644 --- a/substrate/core/consensus/slots/Cargo.toml +++ b/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" } diff --git a/substrate/core/consensus/slots/src/aux_schema.rs b/substrate/core/consensus/slots/src/aux_schema.rs index 3d4a11d487..ed96bf2e22 100644 --- a/substrate/core/consensus/slots/src/aux_schema.rs +++ b/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(backend: Arc, key: &[u8]) -> ClientResult> +fn load_decode(backend: &C, key: &[u8]) -> ClientResult> where C: AuxStore, T: Decode, @@ -74,16 +73,16 @@ impl EquivocationProof { /// /// Note: it detects equivocations only when slot_now - slot <= MAX_SLOT_CAPACITY. pub fn check_equivocation( - backend: &Arc, + backend: &C, slot_now: u64, slot: u64, - header: H, - signer: P, + header: &H, + signer: &P, ) -> ClientResult>> 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( 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( } } - 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( 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(), + ); + } +}