From ff0ce70169ca556c978fbcf013528b7a3daf12e9 Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Thu, 5 Apr 2018 16:11:21 +0800 Subject: [PATCH] Decouple timestamp open-block-assignment/verification to Engine (#8305) --- ethcore/src/block.rs | 2 +- ethcore/src/client/test_client.rs | 2 +- ethcore/src/engines/instant_seal.rs | 11 +++++++++++ ethcore/src/engines/mod.rs | 13 +++++++++++++ ethcore/src/header.rs | 8 -------- ethcore/src/verification/verification.rs | 12 +++++++----- 6 files changed, 33 insertions(+), 15 deletions(-) diff --git a/ethcore/src/block.rs b/ethcore/src/block.rs index f8ccb9196..2280b40de 100644 --- a/ethcore/src/block.rs +++ b/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); diff --git a/ethcore/src/client/test_client.rs b/ethcore/src/client/test_client.rs index 31d935766..6d61035af 100644 --- a/ethcore/src/client/test_client.rs +++ b/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 } diff --git a/ethcore/src/engines/instant_seal.rs b/ethcore/src/engines/instant_seal.rs index 9c2600212..af997c022 100644 --- a/ethcore/src/engines/instant_seal.rs +++ b/ethcore/src/engines/instant_seal.rs @@ -50,6 +50,17 @@ impl Engine for InstantSeal 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)] diff --git a/ethcore/src/engines/mod.rs b/ethcore/src/engines/mod.rs index 5b2b9abb0..41b6fe7b2 100644 --- a/ethcore/src/engines/mod.rs +++ b/ethcore/src/engines/mod.rs @@ -325,6 +325,19 @@ pub trait Engine: 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. diff --git a/ethcore/src/header.rs b/ethcore/src/header.rs index 80f1646ad..50529c720 100644 --- a/ethcore/src/header.rs +++ b/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); } } - diff --git a/ethcore/src/verification/verification.rs b/ethcore/src/verification/verification.rs index 87ed04afd..b670851f0 100644 --- a/ethcore/src/verification/verification.rs +++ b/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(header: &Header, parent: &Header, engine: &EthEngine, do_full: Option>) -> 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 {