Correctly load uncles across fork boundary

- Add test to validate uncles across chain VMs
- Use a universal header sedes, since we can't load the VM before
decoding the header
- Bonus bugfix: must double the header's gas limit at the VM boundary
- Bonus cleanup: remove unused all_header_sedes

Author: carver

eth/abc.py

@@ -1071,7 +1071,7 @@ class ChainDatabaseAPI(HeaderDatabaseAPI):
     # Header API
     #
     @abstractmethod
-    def get_block_uncles(self, niece_header: BlockHeaderAPI) -> Tuple[BlockHeaderAPI, ...]:
+    def get_block_uncles(self, uncles_hash: Hash32) -> Tuple[BlockHeaderAPI, ...]:
         """
         Return an iterable of uncle headers, specified by the header's ``uncles_hash``.
 
@@ -1128,8 +1128,7 @@ def persist_unexecuted_block(self,
     @abstractmethod
     def persist_uncles(
             self,
-            uncles: Tuple[BlockHeaderAPI],
-            header_sedes: BlockHeaderSedesAPI) -> Hash32:
+            uncles: Tuple[BlockHeaderAPI]) -> Hash32:
         """
         Persist the list of uncles to the database.
 
@@ -3080,7 +3079,6 @@ class VirtualMachineAPI(ConfigurableAPI):
     extra_data_max_bytes: ClassVar[int]
     consensus_class: Type[ConsensusAPI]
     consensus_context: ConsensusContextAPI
-    all_header_sedes: ClassVar[Type[BlockHeaderSedesAPI]]
 
     @abstractmethod
     def __init__(self,

eth/db/chain.py

@@ -26,7 +26,6 @@
 from eth.abc import (
     BlockAPI,
     BlockHeaderAPI,
-    BlockHeaderSedesAPI,
     ChainDatabaseAPI,
     DatabaseAPI,
     AtomicDatabaseAPI,
@@ -60,6 +59,7 @@
 from eth.validation import (
     validate_word,
 )
+from eth.vm.header import HeaderSedes
 from eth._warnings import catch_and_ignore_import_warning
 with catch_and_ignore_import_warning():
     import rlp
@@ -153,8 +153,7 @@ def _update_header_chain_gaps(
     #
     # Header API
     #
-    def get_block_uncles(self, niece_header: BlockHeaderAPI) -> Tuple[BlockHeaderAPI, ...]:
-        uncles_hash = niece_header.uncles_hash
+    def get_block_uncles(self, uncles_hash: Hash32) -> Tuple[BlockHeaderAPI, ...]:
         validate_word(uncles_hash, title="Uncles Hash")
         if uncles_hash == EMPTY_UNCLE_HASH:
             return ()
@@ -165,8 +164,7 @@ def get_block_uncles(self, niece_header: BlockHeaderAPI) -> Tuple[BlockHeaderAPI
                 f"No uncles found for hash {uncles_hash!r}"
             ) from exc
         else:
-            header_sedes = niece_header
-            return tuple(rlp.decode(encoded_uncles, sedes=rlp.sedes.CountableList(header_sedes)))
+            return tuple(rlp.decode(encoded_uncles, sedes=rlp.sedes.CountableList(HeaderSedes)))
 
     @classmethod
     def _decanonicalize_old_headers(
@@ -262,7 +260,7 @@ def _persist_block(
                 cls._add_transaction_to_canonical_chain(db, transaction_hash, header, index)
 
         if block.uncles:
-            uncles_hash = cls._persist_uncles(db, block.uncles, block.header)
+            uncles_hash = cls._persist_uncles(db, block.uncles)
         else:
             uncles_hash = EMPTY_UNCLE_HASH
         if uncles_hash != block.header.uncles_hash:
@@ -278,20 +276,18 @@ def _persist_block(
 
     def persist_uncles(
             self,
-            uncles: Tuple[BlockHeaderAPI],
-            header_sedes: BlockHeaderSedesAPI) -> Hash32:
-        return self._persist_uncles(self.db, uncles, header_sedes)
+            uncles: Tuple[BlockHeaderAPI]) -> Hash32:
+        return self._persist_uncles(self.db, uncles)
 
     @staticmethod
     def _persist_uncles(
             db: DatabaseAPI,
-            uncles: Tuple[BlockHeaderAPI, ...],
-            header_sedes: BlockHeaderSedesAPI) -> Hash32:
+            uncles: Tuple[BlockHeaderAPI, ...]) -> Hash32:
 
         uncles_hash = keccak(rlp.encode(uncles))
         db.set(
             uncles_hash,
-            rlp.encode(uncles, sedes=rlp.sedes.CountableList(header_sedes)))
+            rlp.encode(uncles, sedes=rlp.sedes.CountableList(HeaderSedes)))
         return cast(Hash32, uncles_hash)
 
     #

eth/db/header.py

@@ -55,9 +55,7 @@
     validate_block_number,
     validate_word,
 )
-from eth.vm.forks.london.headers import (
-    LondonBackwardsHeader,
-)
+from eth.vm.header import HeaderSedes
 
 
 class HeaderDB(HeaderDatabaseAPI):
@@ -624,7 +622,6 @@ def _decode_block_header(header_rlp: bytes) -> BlockHeaderAPI:
     # Use a deserialization class that can handle any type of header.
     # This feels a little hack-y, but we don't know the shape of the header
     # at this point. It could be a pre-London header, or a post-London
-    # header, which includes the base fee. So we use a class that simply
-    # checks both. Ideally, it will try London first, since all new headers will
-    # be valid for London (until the next fork that breaks it).
-    return rlp.decode(header_rlp, sedes=LondonBackwardsHeader)
+    # header, which includes the base fee. So we use a class that knows how to
+    # decode both.
+    return rlp.decode(header_rlp, sedes=HeaderSedes)

eth/vm/forks/frontier/__init__.py

@@ -1,5 +1,4 @@
 from typing import (
-    ClassVar,
     Type,
 )
 
@@ -10,7 +9,6 @@
 from eth.abc import (
     BlockAPI,
     BlockHeaderAPI,
-    BlockHeaderSedesAPI,
     ReceiptAPI,
     StateAPI,
     SignedTransactionAPI,
@@ -21,7 +19,6 @@
     UNCLE_DEPTH_PENALTY_FACTOR,
     ZERO_HASH32,
 )
-from eth.rlp.headers import BlockHeader
 from eth.rlp.logs import Log
 from eth.rlp.receipts import Receipt
 
@@ -76,7 +73,6 @@ class FrontierVM(VM):
     # classes
     block_class: Type[BlockAPI] = FrontierBlock
     _state_class: Type[StateAPI] = FrontierState
-    all_header_sedes: ClassVar[Type[BlockHeaderSedesAPI]] = BlockHeader
 
     # methods
     create_header_from_parent = staticmethod(create_frontier_header_from_parent)    # type: ignore

eth/vm/forks/frontier/blocks.py

@@ -122,7 +122,7 @@ def from_header(cls, header: BlockHeaderAPI, chaindb: ChainDatabaseAPI) -> "Fron
             uncles: Tuple[BlockHeaderAPI, ...] = ()
         else:
             try:
-                uncles = chaindb.get_block_uncles(header)
+                uncles = chaindb.get_block_uncles(header.uncles_hash)
             except HeaderNotFound as exc:
                 raise BlockNotFound(f"Uncles not found in database for {header}: {exc}") from exc
 

eth/vm/forks/london/__init__.py

@@ -15,7 +15,6 @@
     ELASTICITY_MULTIPLIER,
 )
 from .headers import (
-    LondonBackwardsHeader,
     calculate_expected_base_fee_per_gas,
     compute_london_difficulty,
     configure_london_header,
@@ -31,7 +30,6 @@ class LondonVM(BerlinVM):
     # classes
     block_class: Type[BaseBlock] = LondonBlock
     _state_class: Type[BaseState] = LondonState
-    all_header_sedes = LondonBackwardsHeader
 
     # Methods
     create_header_from_parent = staticmethod(create_london_header_from_parent)  # type: ignore

eth/vm/forks/london/headers.py

@@ -28,6 +28,7 @@
 from .blocks import LondonBlockHeader
 from .constants import (
     BASE_FEE_MAX_CHANGE_DENOMINATOR,
+    ELASTICITY_MULTIPLIER,
     INITIAL_BASE_FEE,
 )
 
@@ -72,12 +73,17 @@ def create_header_from_parent(difficulty_fn: Callable[[BlockHeaderAPI, int], int
                               parent_header: Optional[BlockHeaderAPI],
                               **header_params: Any) -> BlockHeaderAPI:
 
-    # frontier
     if 'gas_limit' not in header_params:
-        header_params['gas_limit'] = compute_gas_limit(
-            parent_header,
-            genesis_gas_limit=GENESIS_GAS_LIMIT,
-        )
+        if parent_header is not None and parent_header.base_fee_per_gas is None:
+            # If the previous block was not a London block,
+            #   double the gas limit, so the new target is the old gas limit
+            header_params['gas_limit'] = parent_header.gas_limit * ELASTICITY_MULTIPLIER
+        else:
+            # frontier rules
+            header_params['gas_limit'] = compute_gas_limit(
+                parent_header,
+                genesis_gas_limit=GENESIS_GAS_LIMIT,
+            )
 
     # byzantium
     if 'timestamp' not in header_params:
@@ -125,10 +131,7 @@ class LondonBackwardsHeader(BlockHeaderSedesAPI):
 
     @classmethod
     def serialize(cls, obj: BlockHeaderAPI) -> List[bytes]:
-        if isinstance(obj, LondonBlockHeader):
-            return LondonBlockHeader.serialize(obj)
-        else:
-            return BlockHeader.serialize(obj)
+        return obj.serialize(obj)
 
     @classmethod
     def deserialize(cls, encoded: List[bytes]) -> BlockHeaderAPI:

eth/vm/header.py

@@ -0,0 +1,12 @@
+from eth.vm.forks.london.headers import LondonBackwardsHeader
+
+HeaderSedes = LondonBackwardsHeader
+"""
+An RLP codec that can decode *all* known header types.
+
+Unfortunately, we often cannot look up the VM to determine the valid codec. For
+example, when looking up a header by hash, we don't have the block number yet,
+and so can't load the VM configuration to find out which VM's rules to use to
+decode the header. Also, it's useful to have this universal sedes class when
+decoding multiple uncles that span a fork block.
+"""

tests/core/chain-object/test_chain.py

@@ -2,11 +2,17 @@
 import rlp
 
 from eth_utils import decode_hex
+from eth_utils.toolz import sliding_window
 
 from eth import constants
 from eth.abc import MiningChainAPI
-from eth.chains.mainnet import MAINNET_GENESIS_HEADER
+from eth.chains.base import MiningChain
+from eth.chains.mainnet import (
+    MAINNET_GENESIS_HEADER,
+    MAINNET_VMS,
+)
 from eth.chains.ropsten import ROPSTEN_GENESIS_HEADER
+from eth.consensus.noproof import NoProofConsensus
 from eth.exceptions import (
     TransactionNotFound,
 )
@@ -25,6 +31,30 @@ def chain(chain_without_block_validation):
     return chain_without_block_validation
 
 
+VM_PAIRS = sliding_window(2, MAINNET_VMS)
+
+
+@pytest.fixture(params=VM_PAIRS)
+def vm_crossover_chain(request, base_db, genesis_state):
+    start_vm, end_vm = request.param
+    klass = MiningChain.configure(
+        __name__='CrossoverTestChain',
+        vm_configuration=(
+            (
+                constants.GENESIS_BLOCK_NUMBER,
+                start_vm.configure(consensus_class=NoProofConsensus),
+            ),
+            # Can mine one block of the first VM, then the next block with be the next VM
+            (
+                constants.GENESIS_BLOCK_NUMBER + 2,
+                end_vm.configure(consensus_class=NoProofConsensus),
+            ),
+        ),
+        chain_id=1337,
+    )
+    return klass.from_genesis(base_db, dict(difficulty=1), genesis_state)
+
+
 @pytest.fixture
 def valid_chain(chain_with_block_validation):
     return chain_with_block_validation
@@ -200,3 +230,37 @@ def test_get_transaction_receipt(chain, tx):
     assert chain.get_canonical_transaction_index(tx.hash) == (1, 0)
     assert chain.get_transaction_receipt_by_index(1, 0) == expected_receipt
     assert chain.get_transaction_receipt(tx.hash) == expected_receipt
+
+
+def _mine_result_to_header(mine_all_result):
+    block_import_result, _, _ = mine_all_result
+    return block_import_result.imported_block.header
+
+
+def test_uncles_across_VMs(vm_crossover_chain):
+    chain = vm_crossover_chain
+
+    genesis = chain.get_canonical_block_header_by_number(0)
+
+    # Mine in 1st VM
+    uncle_header1 = chain.mine_block(extra_data=b'uncle1').header
+    canon_header1 = _mine_result_to_header(
+        chain.mine_all([], parent_header=genesis)
+    )
+
+    # Mine in 2nd VM
+    uncle_header2 = chain.mine_block(extra_data=b'uncle2').header
+    canon_header2 = _mine_result_to_header(
+        chain.mine_all([], parent_header=canon_header1)
+    )
+
+    # Mine block with uncles from both VMs
+    canon_block3 = chain.mine_block(uncles=[uncle_header1, uncle_header2])
+
+    assert canon_header2.hash == canon_block3.header.parent_hash
+
+    assert canon_block3.uncles == (uncle_header1, uncle_header2)
+
+    deserialized_block3 = chain.get_canonical_block_by_number(3)
+
+    assert deserialized_block3.uncles == (uncle_header1, uncle_header2)