Decouple timestamp open-block-assignment/verification to Engine (#8305)

ethcore/src/block.rs

@@ -267,7 +267,7 @@ impl<'x> OpenBlock<'x> {
 		r.block.header.set_parent_hash(parent.hash());
 		r.block.header.set_number(number);
 		r.block.header.set_author(author);
-		r.block.header.set_timestamp_now(parent.timestamp());
+		r.block.header.set_timestamp(engine.open_block_header_timestamp(parent.timestamp()));
 		r.block.header.set_extra_data(extra_data);
 
 		let gas_floor_target = cmp::max(gas_range_target.0, engine.params().min_gas_limit);

ethcore/src/client/test_client.rs

@@ -397,7 +397,7 @@ impl PrepareOpenBlock for TestBlockChainClient {
 			extra_data,
 			false,
 		).expect("Opening block for tests will not fail.");
-		// TODO [todr] Override timestamp for predictability (set_timestamp_now kind of sucks)
+		// TODO [todr] Override timestamp for predictability
 		open_block.set_timestamp(*self.latest_block_timestamp.read());
 		open_block
 	}

ethcore/src/engines/instant_seal.rs

@@ -50,6 +50,17 @@ impl<M: Machine> Engine<M> for InstantSeal<M>
 	fn verify_local_seal(&self, _header: &M::Header) -> Result<(), M::Error> {
 		Ok(())
 	}
+
+	fn open_block_header_timestamp(&self, parent_timestamp: u64) -> u64 {
+		use std::{time, cmp};
+
+		let now = time::SystemTime::now().duration_since(time::UNIX_EPOCH).unwrap_or_default();
+		cmp::max(now.as_secs(), parent_timestamp)
+	}
+
+	fn is_timestamp_valid(&self, header_timestamp: u64, parent_timestamp: u64) -> bool {
+		header_timestamp >= parent_timestamp
+	}
 }
 
 #[cfg(test)]

ethcore/src/engines/mod.rs

@@ -325,6 +325,19 @@ pub trait Engine<M: Machine>: Sync + Send {
 	fn supports_warp(&self) -> bool {
 		self.snapshot_components().is_some()
 	}
+
+	/// Return a new open block header timestamp based on the parent timestamp.
+	fn open_block_header_timestamp(&self, parent_timestamp: u64) -> u64 {
+		use std::{time, cmp};
+
+		let now = time::SystemTime::now().duration_since(time::UNIX_EPOCH).unwrap_or_default();
+		cmp::max(now.as_secs() as u64, parent_timestamp + 1)
+	}
+
+	/// Check whether the parent timestamp is valid.
+	fn is_timestamp_valid(&self, header_timestamp: u64, parent_timestamp: u64) -> bool {
+		header_timestamp > parent_timestamp
+	}
 }
 
 /// Common type alias for an engine coupled with an Ethereum-like state machine.

ethcore/src/header.rs

@@ -17,7 +17,6 @@
 //! Block header.
 
 use std::cmp;
-use std::time::{SystemTime, UNIX_EPOCH};
 use hash::{KECCAK_NULL_RLP, KECCAK_EMPTY_LIST_RLP, keccak};
 use heapsize::HeapSizeOf;
 use ethereum_types::{H256, U256, Address, Bloom};
@@ -224,12 +223,6 @@ impl Header {
 		change_field(&mut self.hash, &mut self.timestamp, a);
 	}
 
-	/// Set the timestamp field of the header to the current time.
-	pub fn set_timestamp_now(&mut self, but_later_than: u64) {
-		let now = SystemTime::now().duration_since(UNIX_EPOCH).unwrap_or_default();
-		self.set_timestamp(cmp::max(now.as_secs() as u64, but_later_than + 1));
-	}
-
 	/// Set the number field of the header.
 	pub fn set_number(&mut self, a: BlockNumber) {
 		change_field(&mut self.hash, &mut self.number, a);
@@ -428,4 +421,3 @@ mod tests {
 		assert_eq!(header_rlp, encoded_header);
 	}
 }
-

ethcore/src/verification/verification.rs

@@ -25,7 +25,7 @@ use std::collections::HashSet;
 use std::time::{SystemTime, UNIX_EPOCH};
 
 use bytes::Bytes;
-use ethereum_types::{H256, U256};
+use ethereum_types::H256;
 use hash::keccak;
 use heapsize::HeapSizeOf;
 use rlp::UntrustedRlp;
@@ -127,7 +127,7 @@ pub struct FullFamilyParams<'a, C: BlockInfo + CallContract + 'a> {
 /// Phase 3 verification. Check block information against parent and uncles.
 pub fn verify_block_family<C: BlockInfo + CallContract>(header: &Header, parent: &Header, engine: &EthEngine, do_full: Option<FullFamilyParams<C>>) -> Result<(), Error> {
 	// TODO: verify timestamp
-	verify_parent(&header, &parent, engine.params().gas_limit_bound_divisor)?;
+	verify_parent(&header, &parent, engine)?;
 	engine.verify_block_family(&header, &parent)?;
 
 	let params = match do_full {
@@ -225,7 +225,7 @@ fn verify_uncles(header: &Header, bytes: &[u8], bc: &BlockProvider, engine: &Eth
 			}
 
 			let uncle_parent = uncle_parent.decode();
-			verify_parent(&uncle, &uncle_parent, engine.params().gas_limit_bound_divisor)?;
+			verify_parent(&uncle, &uncle_parent, engine)?;
 			engine.verify_block_family(&uncle, &uncle_parent)?;
 			verified.insert(uncle.hash());
 		}
@@ -303,11 +303,13 @@ pub fn verify_header_params(header: &Header, engine: &EthEngine, is_full: bool)
 }
 
 /// Check header parameters agains parent header.
-fn verify_parent(header: &Header, parent: &Header, gas_limit_divisor: U256) -> Result<(), Error> {
+fn verify_parent(header: &Header, parent: &Header, engine: &EthEngine) -> Result<(), Error> {
+	let gas_limit_divisor = engine.params().gas_limit_bound_divisor;
+
 	if !header.parent_hash().is_zero() && &parent.hash() != header.parent_hash() {
 		return Err(From::from(BlockError::InvalidParentHash(Mismatch { expected: parent.hash(), found: header.parent_hash().clone() })))
 	}
-	if header.timestamp() <= parent.timestamp() {
+	if !engine.is_timestamp_valid(header.timestamp(), parent.timestamp()) {
 		return Err(From::from(BlockError::InvalidTimestamp(OutOfBounds { max: None, min: Some(parent.timestamp() + 1), found: header.timestamp() })))
 	}
 	if header.number() != parent.number() + 1 {