Report missing author in Aura (#5583)

* report missing block

* add a test validator set

* add a skip test

* clean up test

* report all skipped

ethcore/src/engines/authority_round.rs

@@ -39,7 +39,6 @@ use super::signer::EngineSigner;
 use super::validator_set::{ValidatorSet, SimpleList, new_validator_set};
 
 /// `AuthorityRound` params.
-#[derive(Debug, PartialEq)]
 pub struct AuthorityRoundParams {
 	/// Gas limit divisor.
 	pub gas_limit_bound_divisor: U256,
@@ -52,7 +51,7 @@ pub struct AuthorityRoundParams {
 	/// Starting step,
 	pub start_step: Option<u64>,
 	/// Valid validators.
-	pub validators: ethjson::spec::ValidatorSet,
+	pub validators: Box<ValidatorSet>,
 	/// Chain score validation transition block.
 	pub validate_score_transition: u64,
 	/// Number of first block where EIP-155 rules are validated.
@@ -66,7 +65,7 @@ impl From<ethjson::spec::AuthorityRoundParams> for AuthorityRoundParams {
 		AuthorityRoundParams {
 			gas_limit_bound_divisor: p.gas_limit_bound_divisor.into(),
 			step_duration: Duration::from_secs(p.step_duration.into()),
-			validators: p.validators,
+			validators: new_validator_set(p.validators),
 			block_reward: p.block_reward.map_or_else(U256::zero, Into::into),
 			registrar: p.registrar.map_or_else(Address::new, Into::into),
 			start_step: p.start_step.map(Into::into),
@@ -209,7 +208,7 @@ impl AuthorityRound {
 				proposed: AtomicBool::new(false),
 				client: RwLock::new(None),
 				signer: Default::default(),
-				validators: new_validator_set(our_params.validators),
+				validators: our_params.validators,
 				validate_score_transition: our_params.validate_score_transition,
 				eip155_transition: our_params.eip155_transition,
 				validate_step_transition: our_params.validate_step_transition,
@@ -382,14 +381,22 @@ impl Engine for AuthorityRound {
 			return Err(From::from(BlockError::RidiculousNumber(OutOfBounds { min: Some(1), max: None, found: header.number() })));
 		}
 
-		// Ensure header is from the step after parent.
 		let parent_step = header_step(parent)?;
+		// Ensure header is from the step after parent.
 		if step == parent_step
 			|| (header.number() >= self.validate_step_transition && step <= parent_step) {
 			trace!(target: "engine", "Multiple blocks proposed for step {}.", parent_step);
 			self.validators.report_malicious(header.author(), header.number(), Default::default());
 			Err(EngineError::DoubleVote(header.author().clone()))?;
 		}
+		// Report skipped primaries.
+		if step > parent_step + 1 {
+			for s in parent_step + 1..step {
+				let skipped_primary = self.step_proposer(&parent.hash(), s);
+				trace!(target: "engine", "Author {} did not build his block on top of the intermediate designated primary {}.", header.author(), skipped_primary);
+				self.validators.report_benign(&skipped_primary, header.number());
+			}
+		}
 
 		let gas_limit_divisor = self.gas_limit_bound_divisor;
 		let min_gas = parent.gas_limit().clone() - parent.gas_limit().clone() / gas_limit_divisor;
@@ -460,6 +467,7 @@ impl Engine for AuthorityRound {
 
 #[cfg(test)]
 mod tests {
+	use std::sync::atomic::{AtomicUsize, Ordering as AtomicOrdering};
 	use util::*;
 	use header::Header;
 	use error::{Error, BlockError};
@@ -468,7 +476,9 @@ mod tests {
 	use tests::helpers::*;
 	use account_provider::AccountProvider;
 	use spec::Spec;
-	use engines::Seal;
+	use engines::{Seal, Engine};
+	use engines::validator_set::TestSet;
+	use super::{AuthorityRoundParams, AuthorityRound};
 
 	#[test]
 	fn has_valid_metadata() {
@@ -615,4 +625,32 @@ mod tests {
 		header.set_seal(vec![encode(&3usize).to_vec(), encode(&(&*signature as &[u8])).to_vec()]);
 		assert!(engine.verify_block_family(&header, &parent_header, None).is_err());
 	}
+
+	#[test]
+	fn reports_skipped() {
+		let last_benign = Arc::new(AtomicUsize::new(0));
+		let params = AuthorityRoundParams {
+			gas_limit_bound_divisor: U256::from_str("400").unwrap(),
+			step_duration: Default::default(),
+			block_reward: Default::default(),
+			registrar: Default::default(),
+			start_step: Some(1),
+			validators: Box::new(TestSet::new(Default::default(), last_benign.clone())),
+			validate_score_transition: 0,
+			validate_step_transition: 0,
+			eip155_transition: 0,
+		};
+		let aura = AuthorityRound::new(Default::default(), params, Default::default()).unwrap();
+
+		let mut parent_header: Header = Header::default();
+		parent_header.set_seal(vec![encode(&1usize).to_vec()]);
+		parent_header.set_gas_limit(U256::from_str("222222").unwrap());
+		let mut header: Header = Header::default();
+		header.set_number(1);
+		header.set_gas_limit(U256::from_str("222222").unwrap());
+		header.set_seal(vec![encode(&3usize).to_vec()]);
+
+		assert!(aura.verify_block_family(&header, &parent_header, None).is_ok());
+		assert_eq!(last_benign.load(AtomicOrdering::SeqCst), 1);
+	}
 }

ethcore/src/engines/tendermint/mod.rs

@@ -42,7 +42,7 @@ use evm::Schedule;
 use state::CleanupMode;
 use io::IoService;
 use super::signer::EngineSigner;
-use super::validator_set::{ValidatorSet, new_validator_set};
+use super::validator_set::ValidatorSet;
 use super::transition::TransitionHandler;
 use super::vote_collector::VoteCollector;
 use self::message::*;
@@ -124,7 +124,7 @@ impl Tendermint {
 				proposal: RwLock::new(None),
 				proposal_parent: Default::default(),
 				last_proposed: Default::default(),
-				validators: new_validator_set(our_params.validators),
+				validators: our_params.validators,
 			});
 		let handler = TransitionHandler::new(Arc::downgrade(&engine) as Weak<Engine>, Box::new(our_params.timeouts));
 		engine.step_service.register_handler(Arc::new(handler))?;

ethcore/src/engines/tendermint/params.rs

@@ -19,16 +19,16 @@
 use ethjson;
 use util::{U256, Uint, Address};
 use time::Duration;
+use super::super::validator_set::{ValidatorSet, new_validator_set};
 use super::super::transition::Timeouts;
 use super::Step;
 
 /// `Tendermint` params.
-#[derive(Debug)]
 pub struct TendermintParams {
 	/// Gas limit divisor.
 	pub gas_limit_bound_divisor: U256,
 	/// List of validators.
-	pub validators: ethjson::spec::ValidatorSet,
+	pub validators: Box<ValidatorSet>,
 	/// Timeout durations for different steps.
 	pub timeouts: TendermintTimeouts,
 	/// Block reward.
@@ -82,7 +82,7 @@ impl From<ethjson::spec::TendermintParams> for TendermintParams {
 		let dt = TendermintTimeouts::default();
 		TendermintParams {
 			gas_limit_bound_divisor: p.gas_limit_bound_divisor.into(),
-			validators: p.validators,
+			validators: new_validator_set(p.validators),
 			timeouts: TendermintTimeouts {
 				propose: p.timeout_propose.map_or(dt.propose, to_duration),
 				prevote: p.timeout_prevote.map_or(dt.prevote, to_duration),

ethcore/src/engines/validator_set/mod.rs

@@ -16,6 +16,8 @@
 
 /// Validator lists.
 
+#[cfg(test)]
+mod test;
 mod simple_list;
 mod safe_contract;
 mod contract;
@@ -28,6 +30,8 @@ use ethjson::spec::ValidatorSet as ValidatorSpec;
 use client::Client;
 use header::{Header, BlockNumber};
 
+#[cfg(test)]
+pub use self::test::TestSet;
 pub use self::simple_list::SimpleList;
 use self::contract::ValidatorContract;
 use self::safe_contract::ValidatorSafeContract;

ethcore/src/engines/validator_set/test.rs

@@ -0,0 +1,88 @@
+// Copyright 2015-2017 Parity Technologies (UK) Ltd.
+// This file is part of Parity.
+
+// Parity is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Parity is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Parity.  If not, see <http://www.gnu.org/licenses/>.
+
+/// Used for Engine testing.
+
+use std::str::FromStr;
+use std::sync::atomic::{AtomicUsize, Ordering as AtomicOrdering};
+use util::{Arc, Bytes, H256, Address, HeapSizeOf};
+
+use engines::Call;
+use header::{Header, BlockNumber};
+use super::{ValidatorSet, SimpleList};
+
+/// Set used for testing with a single validator.
+pub struct TestSet {
+	validator: SimpleList,
+	last_malicious: Arc<AtomicUsize>,
+	last_benign: Arc<AtomicUsize>,
+}
+
+impl TestSet {
+	pub fn new(last_malicious: Arc<AtomicUsize>, last_benign: Arc<AtomicUsize>) -> Self {
+		TestSet {
+			validator: SimpleList::new(vec![Address::from_str("7d577a597b2742b498cb5cf0c26cdcd726d39e6e").unwrap()]),
+			last_malicious: last_malicious,
+			last_benign: last_benign,
+		}
+	}
+}
+
+impl HeapSizeOf for TestSet {
+	fn heap_size_of_children(&self) -> usize {
+		self.validator.heap_size_of_children()
+	}
+}
+
+impl ValidatorSet for TestSet {
+	fn default_caller(&self, _block_id: ::ids::BlockId) -> Box<Call> {
+		Box::new(|_, _| Err("Test set doesn't require calls.".into()))
+	}
+
+	fn is_epoch_end(&self, _header: &Header, _block: Option<&[u8]>, _receipts: Option<&[::receipt::Receipt]>)
+		-> ::engines::EpochChange
+	{
+		::engines::EpochChange::No
+	}
+
+	fn epoch_proof(&self, _header: &Header, _caller: &Call) -> Result<Vec<u8>, String> {
+		Ok(Vec::new())
+	}
+
+	fn epoch_set(&self, _header: &Header, _: &[u8]) -> Result<(u64, SimpleList), ::error::Error> {
+		Ok((0, self.validator.clone()))
+	}
+
+	fn contains_with_caller(&self, bh: &H256, address: &Address, _: &Call) -> bool {
+		self.validator.contains(bh, address)
+	}
+
+	fn get_with_caller(&self, bh: &H256, nonce: usize, _: &Call) -> Address {
+		self.validator.get(bh, nonce)
+	}
+
+	fn count_with_caller(&self, _bh: &H256, _: &Call) -> usize {
+		1
+	}
+
+	fn report_malicious(&self, _validator: &Address, block: BlockNumber, _proof: Bytes) {
+		self.last_malicious.store(block as usize, AtomicOrdering::SeqCst)
+	}
+
+	fn report_benign(&self, _validator: &Address, block: BlockNumber) {
+		self.last_benign.store(block as usize, AtomicOrdering::SeqCst)
+	}
+}