AuRa fix for 1.7.x series (#7666)

* Fix Temporarily Invalid blocks handling (#7613)

* Handle temporarily invalid blocks in sync.

* Fix tests.

* Bump rustc-serialize

* Bump version.

* Update .gitlab-ci.yml

fix lint

* Remove slash from gitlab ci script to fix builds

* Start build.

.gitlab-ci.yml

@@ -11,7 +11,7 @@ variables:
   CARGOFLAGS: ""
   CI_SERVER_NAME: "GitLab CI"
 cache:
-  key: "$CI_BUILD_STAGE/$CI_BUILD_REF_NAME"
+  key: "$CI_BUILD_STAGE-$CI_BUILD_REF_NAME"
   untracked: true
 linux-stable:
   stage: build
@@ -95,7 +95,7 @@ linux-snap:
     paths:
     - scripts/parity_*_amd64.snap
     name: "stable-x86_64-unknown-snap-gnu_parity"
-  allow_failure: true  
+  allow_failure: true
 linux-stable-debian:
   stage: build
   image: parity/rust-debian:gitlab-ci
@@ -246,7 +246,7 @@ linux-i686:
     - strip target/$PLATFORM/release/parity
     - strip target/$PLATFORM/release/parity-evm
     - strip target/$PLATFORM/release/ethstore
-    - strip target/$PLATFORM/release/ethkey   
+    - strip target/$PLATFORM/release/ethkey
     - strip target/$PLATFORM/release/parity
     - md5sum target/$PLATFORM/release/parity > parity.md5
     - export SHA3=$(target/$PLATFORM/release/parity tools hash target/$PLATFORM/release/parity)
@@ -488,7 +488,7 @@ darwin:
     name: "x86_64-apple-darwin_parity"
 windows:
   cache:
-    key: "%CI_BUILD_STAGE%/%CI_BUILD_REF_NAME%"
+    key: "%CI_BUILD_STAGE%-%CI_BUILD_REF_NAME%"
     untracked: true
   stage: build
   only:

Cargo.lock

@@ -273,7 +273,7 @@ dependencies = [
 name = "common-types"
 version = "0.1.0"
 dependencies = [
- "ethcore-util 1.7.12",
+ "ethcore-util 1.7.13",
  "ethjson 0.1.0",
  "rlp 0.2.0",
  "rustc-hex 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -472,7 +472,7 @@ dependencies = [
  "ethcore-ipc-nano 1.7.0",
  "ethcore-logger 1.7.0",
  "ethcore-stratum 1.7.0",
- "ethcore-util 1.7.12",
+ "ethcore-util 1.7.13",
  "ethjson 0.1.0",
  "ethkey 0.2.0",
  "ethstore 0.1.0",
@@ -541,7 +541,7 @@ name = "ethcore-ipc"
 version = "1.7.0"
 dependencies = [
  "ethcore-devtools 1.7.0",
- "ethcore-util 1.7.12",
+ "ethcore-util 1.7.13",
  "nanomsg 0.5.1 (git+https://github.com/paritytech/nanomsg.rs.git?branch=parity-1.7)",
  "semver 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
@@ -590,7 +590,7 @@ dependencies = [
  "ethcore-ipc 1.7.0",
  "ethcore-ipc-codegen 1.7.0",
  "ethcore-ipc-nano 1.7.0",
- "ethcore-util 1.7.12",
+ "ethcore-util 1.7.13",
  "log 0.3.7 (registry+https://github.com/rust-lang/crates.io-index)",
  "nanomsg 0.5.1 (git+https://github.com/paritytech/nanomsg.rs.git?branch=parity-1.7)",
  "semver 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -607,7 +607,7 @@ dependencies = [
  "ethcore-ipc 1.7.0",
  "ethcore-ipc-codegen 1.7.0",
  "ethcore-network 1.7.0",
- "ethcore-util 1.7.12",
+ "ethcore-util 1.7.13",
  "evm 0.1.0",
  "futures 0.1.11 (registry+https://github.com/rust-lang/crates.io-index)",
  "itertools 0.5.9 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -646,7 +646,7 @@ dependencies = [
  "ethcore-devtools 1.7.0",
  "ethcore-io 1.7.0",
  "ethcore-logger 1.7.0",
- "ethcore-util 1.7.12",
+ "ethcore-util 1.7.13",
  "ethcrypto 0.1.0",
  "ethkey 0.2.0",
  "igd 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -677,7 +677,7 @@ dependencies = [
  "ethcore-ipc-codegen 1.7.0",
  "ethcore-ipc-nano 1.7.0",
  "ethcore-logger 1.7.0",
- "ethcore-util 1.7.12",
+ "ethcore-util 1.7.13",
  "ethcrypto 0.1.0",
  "ethkey 0.2.0",
  "futures 0.1.11 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -707,7 +707,7 @@ dependencies = [
  "ethcore-ipc-codegen 1.7.0",
  "ethcore-ipc-nano 1.7.0",
  "ethcore-logger 1.7.0",
- "ethcore-util 1.7.12",
+ "ethcore-util 1.7.13",
  "futures 0.1.11 (registry+https://github.com/rust-lang/crates.io-index)",
  "jsonrpc-core 7.0.0 (git+https://github.com/paritytech/jsonrpc.git?branch=parity-1.7)",
  "jsonrpc-macros 7.0.0 (git+https://github.com/paritytech/jsonrpc.git?branch=parity-1.7)",
@@ -720,7 +720,7 @@ dependencies = [
 
 [[package]]
 name = "ethcore-util"
-version = "1.7.12"
+version = "1.7.13"
 dependencies = [
  "ansi_term 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "clippy 0.0.103 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -770,7 +770,7 @@ name = "ethjson"
 version = "0.1.0"
 dependencies = [
  "clippy 0.0.103 (registry+https://github.com/rust-lang/crates.io-index)",
- "ethcore-util 1.7.12",
+ "ethcore-util 1.7.13",
  "rustc-hex 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "serde 1.0.9 (registry+https://github.com/rust-lang/crates.io-index)",
  "serde_derive 1.0.9 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -851,7 +851,7 @@ dependencies = [
  "ethcore-ipc-nano 1.7.0",
  "ethcore-light 1.7.0",
  "ethcore-network 1.7.0",
- "ethcore-util 1.7.12",
+ "ethcore-util 1.7.13",
  "ethkey 0.2.0",
  "heapsize 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.3.7 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -870,7 +870,7 @@ dependencies = [
  "bit-set 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "byteorder 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "common-types 0.1.0",
- "ethcore-util 1.7.12",
+ "ethcore-util 1.7.13",
  "ethjson 0.1.0",
  "evmjit 1.7.0",
  "lazy_static 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -887,7 +887,7 @@ version = "0.1.0"
 dependencies = [
  "docopt 0.8.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "ethcore 1.7.0",
- "ethcore-util 1.7.12",
+ "ethcore-util 1.7.13",
  "ethjson 0.1.0",
  "evm 0.1.0",
  "rustc-hex 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -1132,7 +1132,7 @@ version = "1.7.0"
 dependencies = [
  "ethcore-ipc 1.7.0",
  "ethcore-ipc-codegen 1.7.0",
- "ethcore-util 1.7.12",
+ "ethcore-util 1.7.13",
  "semver 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
@@ -1526,7 +1526,7 @@ version = "0.1.0"
 dependencies = [
  "byteorder 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "ethabi 2.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
- "ethcore-util 1.7.12",
+ "ethcore-util 1.7.13",
  "futures 0.1.11 (registry+https://github.com/rust-lang/crates.io-index)",
  "native-contract-generator 0.1.0",
 ]
@@ -1731,7 +1731,7 @@ dependencies = [
 
 [[package]]
 name = "parity"
-version = "1.7.12"
+version = "1.7.13"
 dependencies = [
  "ansi_term 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "app_dirs 1.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -1751,7 +1751,7 @@ dependencies = [
  "ethcore-logger 1.7.0",
  "ethcore-secretstore 1.0.0",
  "ethcore-stratum 1.7.0",
- "ethcore-util 1.7.12",
+ "ethcore-util 1.7.13",
  "ethkey 0.2.0",
  "ethsync 1.7.0",
  "fdlimit 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -1797,7 +1797,7 @@ dependencies = [
  "clippy 0.0.103 (registry+https://github.com/rust-lang/crates.io-index)",
  "env_logger 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "ethcore-devtools 1.7.0",
- "ethcore-util 1.7.12",
+ "ethcore-util 1.7.13",
  "fetch 0.1.0",
  "futures 0.1.11 (registry+https://github.com/rust-lang/crates.io-index)",
  "jsonrpc-core 7.0.0 (git+https://github.com/paritytech/jsonrpc.git?branch=parity-1.7)",
@@ -1841,7 +1841,7 @@ name = "parity-hash-fetch"
 version = "1.7.0"
 dependencies = [
  "ethabi 2.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
- "ethcore-util 1.7.12",
+ "ethcore-util 1.7.13",
  "fetch 0.1.0",
  "futures 0.1.11 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.3.7 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -1859,7 +1859,7 @@ version = "1.7.0"
 dependencies = [
  "cid 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "ethcore 1.7.0",
- "ethcore-util 1.7.12",
+ "ethcore-util 1.7.13",
  "jsonrpc-http-server 7.0.0 (git+https://github.com/paritytech/jsonrpc.git?branch=parity-1.7)",
  "mime 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "multihash 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -1872,7 +1872,7 @@ version = "0.1.0"
 dependencies = [
  "ethcore 1.7.0",
  "ethcore-io 1.7.0",
- "ethcore-util 1.7.12",
+ "ethcore-util 1.7.13",
  "ethkey 0.2.0",
  "log 0.3.7 (registry+https://github.com/rust-lang/crates.io-index)",
  "rlp 0.2.0",
@@ -1902,7 +1902,7 @@ dependencies = [
  "ethcore-ipc 1.7.0",
  "ethcore-light 1.7.0",
  "ethcore-logger 1.7.0",
- "ethcore-util 1.7.12",
+ "ethcore-util 1.7.13",
  "ethcrypto 0.1.0",
  "ethjson 0.1.0",
  "ethkey 0.2.0",
@@ -1944,7 +1944,7 @@ dependencies = [
 name = "parity-rpc-client"
 version = "1.4.0"
 dependencies = [
- "ethcore-util 1.7.12",
+ "ethcore-util 1.7.13",
  "futures 0.1.11 (registry+https://github.com/rust-lang/crates.io-index)",
  "jsonrpc-core 7.0.0 (git+https://github.com/paritytech/jsonrpc.git?branch=parity-1.7)",
  "jsonrpc-ws-server 7.0.0 (git+https://github.com/paritytech/jsonrpc.git?branch=parity-1.7)",
@@ -2007,7 +2007,7 @@ dependencies = [
  "ethcore 1.7.0",
  "ethcore-ipc 1.7.0",
  "ethcore-ipc-codegen 1.7.0",
- "ethcore-util 1.7.12",
+ "ethcore-util 1.7.13",
  "ethsync 1.7.0",
  "futures 0.1.11 (registry+https://github.com/rust-lang/crates.io-index)",
  "ipc-common-types 1.7.0",
@@ -2363,7 +2363,7 @@ name = "rpc-cli"
 version = "1.4.0"
 dependencies = [
  "bigint 3.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
- "ethcore-util 1.7.12",
+ "ethcore-util 1.7.13",
  "futures 0.1.11 (registry+https://github.com/rust-lang/crates.io-index)",
  "parity-rpc 1.7.0",
  "parity-rpc-client 1.4.0",

Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 description = "Parity Ethereum client"
 name = "parity"
-version = "1.7.12"
+version = "1.7.13"
 license = "GPL-3.0"
 authors = ["Parity Technologies <admin@parity.io>"]
 build = "build.rs"

ethcore/src/engines/authority_round/mod.rs

@@ -301,9 +301,11 @@ struct EpochVerifier {
 
 impl super::EpochVerifier for EpochVerifier {
 	fn verify_light(&self, header: &Header) -> Result<(), Error> {
+		// Validate the timestamp
+		verify_timestamp(&*self.step, header_step(header)?)?;
 		// always check the seal since it's fast.
 		// nothing heavier to do.
-		verify_external(header, &self.subchain_validators, &*self.step, |_| {})
+		verify_external(header, &self.subchain_validators)
 	}
 
 	fn check_finality_proof(&self, proof: &[u8]) -> Option<Vec<H256>> {
@@ -328,7 +330,7 @@ impl super::EpochVerifier for EpochVerifier {
 			//
 			// `verify_external` checks that signature is correct and author == signer.
 			if header.seal().len() != 2 { return None }
-			otry!(verify_external(header, &self.subchain_validators, &*self.step, |_| {}).ok());
+			otry!(verify_external(header, &self.subchain_validators).ok());
 
 			let newly_finalized = otry!(finality_checker.push_hash(header.hash(), header.author().clone()).ok());
 			finalized.extend(newly_finalized);
@@ -338,16 +340,6 @@ impl super::EpochVerifier for EpochVerifier {
 	}
 }
 
-// Report misbehavior
-#[derive(Debug)]
-#[allow(dead_code)]
-enum Report {
-	// Malicious behavior
-	Malicious(Address, BlockNumber, Bytes),
-	// benign misbehavior
-	Benign(Address, BlockNumber),
-}
-
 fn header_step(header: &Header) -> Result<usize, ::rlp::DecoderError> {
 	UntrustedRlp::new(&header.seal().get(0).expect("was either checked with verify_block_basic or is genesis; has 2 fields; qed (Make sure the spec file has a correct genesis seal)")).as_val()
 }
@@ -366,34 +358,35 @@ fn is_step_proposer(validators: &ValidatorSet, bh: &H256, step: usize, address:
 	step_proposer(validators, bh, step) == *address
 }
 
-fn verify_external<F: Fn(Report)>(header: &Header, validators: &ValidatorSet, step: &Step, report: F)
+fn verify_timestamp(step: &Step, header_step: usize) -> Result<(), BlockError> {
-	-> Result<(), Error>
-{
-	let header_step = header_step(header)?;
-
 	match step.check_future(header_step) {
 		Err(None) => {
-			trace!(target: "engine", "verify_block_external: block from the future");
+			trace!(target: "engine", "verify_timestamp: block from the future");
-			report(Report::Benign(*header.author(), header.number()));
+			Err(BlockError::InvalidSeal.into())
-			return Err(BlockError::InvalidSeal.into())
 		},
 		Err(Some(oob)) => {
-			trace!(target: "engine", "verify_block_external: block too early");
+			// NOTE This error might be returned only in early stage of verification (Stage 1).
-			return Err(BlockError::TemporarilyInvalid(oob).into())
+			// Returning it further won't recover the sync process.
+			trace!(target: "engine", "verify_timestamp: block too early");
+			Err(BlockError::TemporarilyInvalid(oob).into())
 		},
-		Ok(_) => {
+		Ok(_) => Ok(()),
-			let proposer_signature = header_signature(header)?;
+	}
-			let correct_proposer = validators.get(header.parent_hash(), header_step);
+}
-			let is_invalid_proposer = *header.author() != correct_proposer ||
-				!verify_address(&correct_proposer, &proposer_signature, &header.bare_hash())?;
 
-			if is_invalid_proposer {
+fn verify_external(header: &Header, validators: &ValidatorSet) -> Result<(), Error> {
-				trace!(target: "engine", "verify_block_external: bad proposer for step: {}", header_step);
+	let header_step = header_step(header)?;
-				Err(EngineError::NotProposer(Mismatch { expected: correct_proposer, found: header.author().clone() }))?
+
-			} else {
+	let proposer_signature = header_signature(header)?;
-				Ok(())
+	let correct_proposer = validators.get(header.parent_hash(), header_step);
-			}
+	let is_invalid_proposer = *header.author() != correct_proposer ||
-		}
+		!verify_address(&correct_proposer, &proposer_signature, &header.bare_hash())?;
+
+	if is_invalid_proposer {
+		trace!(target: "engine", "verify_block_external: bad proposer for step: {}", header_step);
+		Err(EngineError::NotProposer(Mismatch { expected: correct_proposer, found: header.author().clone() }))?
+	} else {
+		Ok(())
 	}
 }
 
@@ -694,15 +687,28 @@ impl Engine for AuthorityRound {
 	fn verify_block_basic(&self, header: &Header, _block: Option<&[u8]>) -> Result<(), Error> {
 		if header.seal().len() != self.seal_fields() {
 			trace!(target: "engine", "verify_block_basic: wrong number of seal fields");
-			Err(From::from(BlockError::InvalidSealArity(
+			return Err(From::from(BlockError::InvalidSealArity(
 				Mismatch { expected: self.seal_fields(), found: header.seal().len() }
-			)))
+			)));
-		} else if header.number() >= self.validate_score_transition && *header.difficulty() >= U256::from(U128::max_value()) {
+		}
-			Err(From::from(BlockError::DifficultyOutOfBounds(
+
+		if header.number() >= self.validate_score_transition && *header.difficulty() >= U256::from(U128::max_value()) {
+			return Err(From::from(BlockError::DifficultyOutOfBounds(
 				OutOfBounds { min: None, max: Some(U256::from(U128::max_value())), found: *header.difficulty() }
-			)))
+			)));
-		} else {
+		}
-			Ok(())
+
+		// TODO [ToDr] Should this go from epoch manager?
+		// If yes then probably benign reporting needs to be moved further in the verification.
+		let set_number = header.number();
+
+		match verify_timestamp(&*self.step, header_step(header)?) {
+			Err(BlockError::InvalidSeal) => {
+				self.validators.report_benign(header.author(), set_number, header.number());
+				Err(BlockError::InvalidSeal.into())
+			}
+			Err(e) => Err(e.into()),
+			Ok(()) => Ok(()),
 		}
 	}
 
@@ -720,13 +726,15 @@ impl Engine for AuthorityRound {
 		}
 
 		let parent_step = header_step(parent)?;
+		// TODO [ToDr] Should this go from epoch manager?
+		let set_number = header.number();
 
 		// 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(), header.number(), Default::default());
+			self.validators.report_malicious(header.author(), set_number, header.number(), Default::default());
 			Err(EngineError::DoubleVote(header.author().clone()))?;
 		}
 		// Report skipped primaries.
@@ -738,7 +746,7 @@ impl Engine for AuthorityRound {
 				let skipped_primary = step_proposer(&*self.validators, &parent.hash(), s);
 				// Do not report this signer.
 				if skipped_primary != me {
-					self.validators.report_benign(&skipped_primary, header.number(), header.number());
+					self.validators.report_benign(&skipped_primary, set_number, header.number());
 				}
 				// Stop reporting once validators start repeating.
 				if !reported.insert(skipped_primary) { break; }
@@ -759,9 +767,8 @@ impl Engine for AuthorityRound {
 		// fetch correct validator set for current epoch, taking into account
 		// finality of previous transitions.
 		let active_set;
-
+		let validators = if self.immediate_transitions {
-		let (validators, set_number) = if self.immediate_transitions {
+			&*self.validators
-			(&*self.validators, header.number())
 		} else {
 			// get correct validator set for epoch.
 			let client = match self.client.read().as_ref().and_then(|weak| weak.upgrade()) {
@@ -779,19 +786,12 @@ impl Engine for AuthorityRound {
 			}
 
 			active_set = epoch_manager.validators().clone();
-			(&active_set as &_, epoch_manager.epoch_transition_number)
+			&active_set as &_
-		};
-
-		let report = |report| match report {
-			Report::Benign(address, block_number) =>
-				self.validators.report_benign(&address, set_number, block_number),
-			Report::Malicious(address, block_number, proof) =>
-				self.validators.report_malicious(&address, set_number, block_number, proof),
 		};
 
 		// verify signature against fixed list, but reports should go to the
 		// contract itself.
-		verify_external(header, validators, &*self.step, report)
+		verify_external(header, validators)
 	}
 
 	fn genesis_epoch_data(&self, header: &Header, call: &Call) -> Result<Vec<u8>, String> {
@@ -1133,8 +1133,7 @@ mod tests {
 		assert!(engine.verify_block_family(&header, &parent_header, None).is_ok());
 		assert!(engine.verify_block_external(&header, None).is_ok());
 		header.set_seal(vec![encode(&5usize).into_vec(), encode(&(&*signature as &[u8])).into_vec()]);
-		assert!(engine.verify_block_family(&header, &parent_header, None).is_ok());
+		assert!(engine.verify_block_basic(&header, None).is_err());
-		assert!(engine.verify_block_external(&header, None).is_err());
 	}
 
 	#[test]
@@ -1270,3 +1269,4 @@ mod tests {
 		AuthorityRound::new(Default::default(), params, Default::default()).unwrap();
 	}
 }
+

ethcore/src/engines/validator_set/contract.rs

@@ -180,7 +180,7 @@ mod tests {
 		header.set_number(2);
 		header.set_parent_hash(client.chain_info().best_block_hash);
 		// `reportBenign` when the designated proposer releases block from the future (bad clock).
-		assert!(client.engine().verify_block_external(&header, None).is_err());
+		assert!(client.engine().verify_block_basic(&header, None).is_err());
 		// Seal a block.
 		client.engine().step();
 		assert_eq!(client.chain_info().best_block_number, 1);

parity/run.rs

@@ -1,6 +1,7 @@
 // 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

sync/src/block_sync.rs

@@ -519,6 +519,10 @@ impl BlockDownloader {
 					trace!(target: "sync", "Unknown new block parent, restarting sync");
 					break;
 				},
+				Err(BlockImportError::Block(BlockError::TemporarilyInvalid(_))) => {
+					debug!(target: "sync", "Block temporarily invalid, restarting sync");
+					break;
+				},
 				Err(e) => {
 					debug!(target: "sync", "Bad block {:?} : {:?}", h, e);
 					bad = true;

util/Cargo.toml

@@ -3,7 +3,7 @@ description = "Ethcore utility library"
 homepage = "http://parity.io"
 license = "GPL-3.0"
 name = "ethcore-util"
-version = "1.7.12"
+version = "1.7.13"
 authors = ["Parity Technologies <admin@parity.io>"]
 build = "build.rs"