Choose better header timestamp, if none supplied

Actually look at the clock, and compare it to the parent. Related:

- Fix doctests: set timestamp manually for reproducible results (new
MiningChain API)
- clique should verify timestamp in UTC (bug uncovered)
- Only guess timestamp on non-genesis headers (when parent & VM are
unavailable), so manually set in a test where headers are built manually

Author: carver

docs/guides/understanding_the_mining_process.rst

@@ -341,6 +341,7 @@ zero value transfer transaction.
   ...     ))
 
   >>> chain = klass.from_genesis(AtomicDB(), GENESIS_PARAMS)
+  >>> genesis = chain.get_canonical_block_header_by_number(0)
   >>> vm = chain.get_vm()
 
   >>> nonce = vm.state.get_nonce(SENDER)
@@ -358,6 +359,12 @@ zero value transfer transaction.
 
   >>> chain.apply_transaction(signed_tx)
   (<ByzantiumBlock(#Block #1...)
+
+  >>> # Normally, we can let the timestamp be chosen automatically, but
+  >>> # for the sake of reproducing exactly the same block every time,
+  >>> # we will set it manually here:
+  >>> chain.set_header_timestamp(genesis.timestamp + 1)
+
   >>> # We have to finalize the block first in order to be able read the
   >>> # attributes that are important for the PoW algorithm
   >>> block_result = chain.get_vm().finalize_block(chain.get_block())

eth/_utils/headers.py

@@ -1,3 +1,4 @@
+import datetime
 from typing import (
     Dict,
     Tuple,
@@ -25,6 +26,31 @@
 )
 
 
+def eth_now() -> int:
+    """
+    The timestamp is in UTC.
+    """
+    return int(datetime.datetime.utcnow().timestamp())
+
+
+def new_timestamp_from_parent(parent: BlockHeaderAPI) -> int:
+    """
+    Generate a timestamp to use on a new header.
+
+    Generally, attempt to use the current time. If timestamp is too old (equal
+    or less than parent), return `parent.timestamp + 1`. If parent is None,
+    then consider this a genesis block.
+    """
+    if parent is None:
+        return eth_now()
+    else:
+        # header timestamps must increment
+        return max(
+            parent.timestamp + 1,
+            eth_now(),
+        )
+
+
 def fill_header_params_from_parent(
         parent: BlockHeaderAPI,
         gas_limit: int,

eth/abc.py

@@ -4070,6 +4070,30 @@ def __init__(self, base_db: AtomicDatabaseAPI, header: BlockHeaderAPI = None) ->
         """
         ...
 
+    @abstractmethod
+    def set_header_timestamp(self, timestamp: int) -> None:
+        """
+        Set the timestamp of the pending header to mine.
+
+        This is mostly useful for testing, as the timestamp will be chosen
+        automatically if this method is not called.
+        """
+        ...
+
+    @abstractmethod
+    def set_coinbase(self, coinbase: Address) -> None:
+        """
+        Set the coinbase for all new headers.
+        """
+        ...
+
+    @abstractmethod
+    def set_extra_data(self, extra_data: bytes) -> None:
+        """
+        Set the extra_data (sometimes called graffiti) for all new headers.
+        """
+        ...
+
     @abstractmethod
     def mine_all(
             self,

eth/chains/base.py

@@ -60,7 +60,9 @@
 )
 from eth.constants import (
     EMPTY_UNCLE_HASH,
+    GENESIS_EXTRA_DATA,
     MAX_UNCLE_DEPTH,
+    ZERO_ADDRESS,
 )
 
 from eth.db.chain import (
@@ -646,6 +648,8 @@ class MiningChain(Chain, MiningChainAPI):
 
     def __init__(self, base_db: AtomicDatabaseAPI, header: BlockHeaderAPI = None) -> None:
         super().__init__(base_db)
+        self._header_extra_data = GENESIS_EXTRA_DATA
+        self._header_coinbase = ZERO_ADDRESS
         self.header = self.ensure_header(header)
 
     def apply_transaction(self,
@@ -680,6 +684,17 @@ def import_block(self,
         self.header = self.ensure_header()
         return result
 
+    def set_header_timestamp(self, timestamp: int) -> None:
+        self.header = self.header.copy(timestamp=timestamp)
+
+    def set_coinbase(self, coinbase: Address) -> None:
+        self.header = self.header.copy(coinbase=coinbase)
+        self._header_coinbase = coinbase
+
+    def set_extra_data(self, extra_data: bytes) -> None:
+        self.header = self.header.copy(extra_data=extra_data)
+        self._header_extra_data = extra_data
+
     def mine_all(
             self,
             transactions: Sequence[SignedTransactionAPI],

eth/consensus/clique/_utils.py

@@ -1,4 +1,3 @@
-import time
 from typing import (
     Iterable,
 )
@@ -15,6 +14,7 @@
     ValidationError,
 )
 
+from eth._utils.headers import eth_now
 from eth.abc import (
     BlockHeaderAPI,
 )
@@ -127,7 +127,7 @@ def is_checkpoint(block_number: int, epoch_length: int) -> bool:
 
 def validate_header_integrity(header: BlockHeaderAPI, epoch_length: int) -> None:
 
-    if header.timestamp > int(time.time()):
+    if header.timestamp > eth_now():
         raise ValidationError(f"Invalid future timestamp: {header.timestamp}")
 
     at_checkpoint = is_checkpoint(header.block_number, epoch_length)

eth/rlp/headers.py

@@ -1,4 +1,3 @@
-import time
 from typing import (
     Optional,
     cast,
@@ -24,6 +23,9 @@
     encode_hex,
 )
 
+from eth._utils.headers import (
+    new_timestamp_from_parent,
+)
 from eth.abc import (
     BlockHeaderAPI,
     MiningHeaderAPI,
@@ -123,7 +125,12 @@ def __init__(self,              # type: ignore  # noqa: F811
                  mix_hash: Hash32 = ZERO_HASH32,
                  nonce: bytes = GENESIS_NONCE) -> None:
         if timestamp is None:
-            timestamp = int(time.time())
+            if parent_hash == ZERO_HASH32:
+                timestamp = new_timestamp_from_parent(None)
+            else:
+                # without access to the parent header, we cannot select a new timestamp correctly
+                raise ValueError("Must set timestamp explicitly if this is not a genesis header")
+
         super().__init__(
             parent_hash=parent_hash,
             uncles_hash=uncles_hash,

eth/vm/forks/byzantium/headers.py

@@ -20,6 +20,9 @@
 from eth._utils.db import (
     get_parent_header,
 )
+from eth._utils.headers import (
+    new_timestamp_from_parent,
+)
 from eth.validation import (
     validate_gt,
     validate_header_params_for_configuration,
@@ -77,9 +80,10 @@ def create_header_from_parent(difficulty_fn: Callable[[BlockHeaderAPI, int], int
                               parent_header: BlockHeaderAPI,
                               **header_params: Any) -> BlockHeaderAPI:
 
-    if 'difficulty' not in header_params:
-        header_params.setdefault('timestamp', parent_header.timestamp + 1)
+    if 'timestamp' not in header_params:
+        header_params['timestamp'] = new_timestamp_from_parent(parent_header)
 
+    if 'difficulty' not in header_params:
         header_params['difficulty'] = difficulty_fn(
             parent_header,
             header_params['timestamp'],

eth/vm/forks/frontier/headers.py

@@ -22,6 +22,7 @@
 from eth._utils.headers import (
     compute_gas_limit,
     fill_header_params_from_parent,
+    new_timestamp_from_parent,
 )
 from eth.rlp.headers import BlockHeader
 
@@ -75,10 +76,12 @@ def compute_frontier_difficulty(parent_header: BlockHeaderAPI, timestamp: int) -
 
 def create_frontier_header_from_parent(parent_header: BlockHeaderAPI,
                                        **header_params: Any) -> BlockHeader:
+    if 'timestamp' not in header_params:
+        header_params['timestamp'] = new_timestamp_from_parent(parent_header)
+
     if 'difficulty' not in header_params:
         # Use setdefault to ensure the new header has the same timestamp we use to calculate its
         # difficulty.
-        header_params.setdefault('timestamp', parent_header.timestamp + 1)
         header_params['difficulty'] = compute_frontier_difficulty(
             parent_header,
             header_params['timestamp'],

eth/vm/forks/london/headers.py

@@ -10,6 +10,7 @@
 from eth._utils.headers import (
     compute_gas_limit,
     fill_header_params_from_parent,
+    new_timestamp_from_parent,
 )
 from eth.abc import (
     BlockHeaderAPI,
@@ -79,11 +80,7 @@ def create_header_from_parent(difficulty_fn: Callable[[BlockHeaderAPI, int], int
 
     # byzantium
     if 'timestamp' not in header_params:
-        if parent_header is None:
-            timestamp = 0
-        else:
-            timestamp = parent_header.timestamp + 1
-        header_params['timestamp'] = timestamp
+        header_params['timestamp'] = new_timestamp_from_parent(parent_header)
 
     if 'difficulty' not in header_params:
         if parent_header is None:

tests/database/test_eth1_chaindb.py

@@ -275,7 +275,13 @@ def test_chaindb_get_score(chaindb):
     assert genesis_score == 1
     assert chaindb.get_score(genesis.hash) == 1
 
-    block1 = BlockHeader(difficulty=10, block_number=1, gas_limit=0, parent_hash=genesis.hash)
+    block1 = BlockHeader(
+        difficulty=10,
+        block_number=1,
+        gas_limit=0,
+        parent_hash=genesis.hash,
+        timestamp=genesis.timestamp + 1,
+    )
     chaindb.persist_header(block1)
 
     block1_score_key = SchemaV1.make_block_hash_to_score_lookup_key(block1.hash)