WIP changes chain.import_block to return new blocks and blocks evicte… (#1151)

* WIP changes chain.import_block to return new blocks and blocks evicted issue #772

* added test for import_block

Author: eddybenkhin

eth/chains/base.py

@@ -8,12 +8,13 @@
 import random
 from typing import (  # noqa: F401
     Any,
-    Optional,
     Callable,
     cast,
     Dict,
     Generator,
     Iterator,
+    List,
+    Optional,
     Tuple,
     Type,
     TYPE_CHECKING,
@@ -265,7 +266,10 @@ def estimate_gas(
         raise NotImplementedError("Chain classes must implement this method")
 
     @abstractmethod
-    def import_block(self, block: BaseBlock, perform_validation: bool=True) -> BaseBlock:
+    def import_block(self,
+                     block: BaseBlock,
+                     perform_validation: bool=True,
+                     ) -> Tuple[Tuple[BaseBlock, ...], Tuple[BaseBlock, ...]]:
         raise NotImplementedError("Chain classes must implement this method")
 
     #
@@ -584,10 +588,14 @@ def estimate_gas(
         with self.get_vm(at_header).state_in_temp_block() as state:
             return self.gas_estimator(state, transaction)
 
-    def import_block(self, block: BaseBlock, perform_validation: bool=True) -> BaseBlock:
+    def import_block(self,
+                     block: BaseBlock,
+                     perform_validation: bool=True
+                     ) -> Tuple[Tuple[BaseBlock, ...], Tuple[BaseBlock, ...]]:
         """
-        Imports a complete block.
+        Imports a complete block and returns new_blocks and orphaned_canonical_blocks.
         """
+
         try:
             parent_header = self.get_block_header_by_hash(block.header.parent_hash)
         except HeaderNotFound:
@@ -608,13 +616,26 @@ def import_block(self, block: BaseBlock, perform_validation: bool=True) -> BaseB
             ensure_imported_block_unchanged(imported_block, block)
             self.validate_block(imported_block)
 
-        self.chaindb.persist_block(imported_block)
+        (
+            new_canonical_header_hashes,
+            orphaned_header_hashes,
+        ) = self.chaindb.persist_block(imported_block)
+
         self.logger.debug(
             'IMPORTED_BLOCK: number %s | hash %s',
             imported_block.number,
             encode_hex(imported_block.hash),
         )
-        return imported_block
+
+        new_blocks = tuple(
+            self.get_block_by_hash(header_hash) for header_hash in new_canonical_header_hashes)
+        orphaned_canonical_blocks = tuple(
+            self.get_block_by_hash(header_hash) for header_hash in orphaned_header_hashes)
+        # add imported block to new block when no chain re-org
+        if not new_blocks:
+            new_blocks = (imported_block, )
+
+        return new_blocks, orphaned_canonical_blocks
 
     #
     # Validation API
@@ -780,10 +801,15 @@ def apply_transaction(self, transaction):
 
         return new_block, receipt, computation
 
-    def import_block(self, block: BaseBlock, perform_validation: bool=True) -> BaseBlock:
-        result_block = super().import_block(block, perform_validation)
+    def import_block(self,
+                     block: BaseBlock,
+                     perform_validation: bool=True
+                     ) -> Tuple[Tuple[BaseBlock, ...], Tuple[BaseBlock, ...]]:
+        new_canonical_blocks, orphaned_canonical_blocks = super().import_block(
+            block, perform_validation)
+
         self.header = self.ensure_header()
-        return result_block
+        return new_canonical_blocks, orphaned_canonical_blocks
 
     def mine_block(self, *args: Any, **kwargs: Any) -> BaseBlock:
         """

eth/db/chain.py

@@ -94,7 +94,9 @@ def get_block_uncles(self, uncles_hash: Hash32) -> List[BlockHeader]:
     # Block API
     #
     @abstractmethod
-    def persist_block(self, block: 'BaseBlock') -> None:
+    def persist_block(self,
+                      block: 'BaseBlock'
+                      ) -> Tuple[Tuple[bytes, ...], Tuple[bytes, ...]]:
         raise NotImplementedError("ChainDB classes must implement this method")
 
     @abstractmethod
@@ -186,7 +188,9 @@ def get_block_uncles(self, uncles_hash: Hash32) -> List[BlockHeader]:
 
     # TODO: This method should take a chain of headers as that's the most common use case
     # and it'd be much faster than inserting each header individually.
-    def persist_header(self, header: BlockHeader) -> Tuple[BlockHeader, ...]:
+    def persist_header(self,
+                       header: BlockHeader
+                       ) -> Tuple[Tuple[BlockHeader, ...], Tuple[BlockHeader, ...]]:
         """
         Returns iterable of headers newly on the canonical chain
         """
@@ -214,17 +218,26 @@ def persist_header(self, header: BlockHeader) -> Tuple[BlockHeader, ...]:
         try:
             head_score = self.get_score(self.get_canonical_head().hash)
         except CanonicalHeadNotFound:
-            new_headers = self._set_as_canonical_chain_head(header)
+            (
+                new_canonical_headers,
+                orphaned_canonical_headers
+            ) = self._set_as_canonical_chain_head(header)
         else:
             if score > head_score:
-                new_headers = self._set_as_canonical_chain_head(header)
+                (
+                    new_canonical_headers,
+                    orphaned_canonical_headers
+                ) = self._set_as_canonical_chain_head(header)
             else:
-                new_headers = tuple()
+                new_canonical_headers = tuple()
+                orphaned_canonical_headers = tuple()
 
-        return new_headers
+        return new_canonical_headers, orphaned_canonical_headers
 
     # TODO: update this to take a `hash` rather than a full header object.
-    def _set_as_canonical_chain_head(self, header: BlockHeader) -> Tuple[BlockHeader, ...]:
+    def _set_as_canonical_chain_head(self,
+                                     header: BlockHeader
+                                     ) -> Tuple[Tuple[BlockHeader, ...], Tuple[BlockHeader, ...]]:
         """
         Returns iterable of headers newly on the canonical head
         """
@@ -235,7 +248,7 @@ def _set_as_canonical_chain_head(self, header: BlockHeader) -> Tuple[BlockHeader
                 header.hash))
 
         new_canonical_headers = tuple(reversed(self._find_new_ancestors(header)))
-
+        orphaned_canonical_headers = []
         # remove transaction lookups for blocks that are no longer canonical
         for h in new_canonical_headers:
             try:
@@ -244,29 +257,30 @@ def _set_as_canonical_chain_head(self, header: BlockHeader) -> Tuple[BlockHeader
                 # no old block, and no more possible
                 break
             else:
-                old_header = self.get_block_header_by_hash(old_hash)
-                for transaction_hash in self.get_block_transaction_hashes(old_header):
+                orphaned_header = self.get_block_header_by_hash(old_hash)
+                orphaned_canonical_headers.append(orphaned_header)
+                for transaction_hash in self.get_block_transaction_hashes(orphaned_header):
                     self._remove_transaction_from_canonical_chain(transaction_hash)
-                    # TODO re-add txn to internal pending pool (only if local sender)
-                    pass
 
         for h in new_canonical_headers:
             self._add_block_number_to_hash_lookup(h)
 
         self.db.set(SchemaV1.make_canonical_head_hash_lookup_key(), header.hash)
 
-        return new_canonical_headers
+        return new_canonical_headers, tuple(orphaned_canonical_headers)
 
     #
     # Block API
     #
-    def persist_block(self, block: 'BaseBlock') -> None:
+    def persist_block(self,
+                      block: 'BaseBlock'
+                      ) -> Tuple[Tuple[bytes, ...], Tuple[bytes, ...]]:
         '''
         Persist the given block's header and uncles.
 
         Assumes all block transactions have been persisted already.
         '''
-        new_canonical_headers = self.persist_header(block.header)
+        new_canonical_headers, orphaned_canonical_headers = self.persist_header(block.header)
 
         for header in new_canonical_headers:
             if header.hash == block.hash:
@@ -288,6 +302,11 @@ def persist_block(self, block: 'BaseBlock') -> None:
             raise ValidationError(
                 "Block's uncles_hash (%s) does not match actual uncles' hash (%s)",
                 block.header.uncles_hash, uncles_hash)
+        new_canonical_header_hashes = tuple(header.hash for header in new_canonical_headers)
+        orphaned_canonical_header_hashes = tuple(
+            header.hash for header in orphaned_canonical_headers)
+
+        return new_canonical_header_hashes, orphaned_canonical_header_hashes
 
     def persist_uncles(self, uncles: Tuple[BlockHeader]) -> Hash32:
         """

eth/db/header.py

@@ -68,7 +68,9 @@ def header_exists(self, block_hash: Hash32) -> bool:
         raise NotImplementedError("ChainDB classes must implement this method")
 
     @abstractmethod
-    def persist_header(self, header: BlockHeader) -> Tuple[BlockHeader, ...]:
+    def persist_header(self,
+                       header: BlockHeader
+                       ) -> Tuple[Tuple[BlockHeader, ...], Tuple[BlockHeader, ...]]:
         raise NotImplementedError("ChainDB classes must implement this method")
 
 
@@ -144,7 +146,9 @@ def header_exists(self, block_hash: Hash32) -> bool:
         validate_word(block_hash, title="Block Hash")
         return block_hash in self.db
 
-    def persist_header(self, header: BlockHeader) -> Tuple[BlockHeader, ...]:
+    def persist_header(self,
+                       header: BlockHeader
+                       ) -> Tuple[Tuple[BlockHeader, ...], Tuple[BlockHeader, ...]]:
         """
         :returns: iterable of headers newly on the canonical chain
         """
@@ -174,16 +178,18 @@ def persist_header(self, header: BlockHeader) -> Tuple[BlockHeader, ...]:
         try:
             head_score = self.get_score(self.get_canonical_head().hash)
         except CanonicalHeadNotFound:
-            new_canonical_headers = self._set_as_canonical_chain_head(header.hash)
+            new_canonical_headers, old_headers = self._set_as_canonical_chain_head(header.hash)
         else:
             if score > head_score:
-                new_canonical_headers = self._set_as_canonical_chain_head(header.hash)
+                new_canonical_headers, old_headers = self._set_as_canonical_chain_head(header.hash)
             else:
                 new_canonical_headers = tuple()
+                old_headers = tuple()
 
-        return new_canonical_headers
+        return new_canonical_headers, old_headers
 
-    def _set_as_canonical_chain_head(self, block_hash: Hash32) -> Tuple[BlockHeader, ...]:
+    def _set_as_canonical_chain_head(self, block_hash: Hash32
+                                     ) -> Tuple[Tuple[BlockHeader, ...], Tuple[BlockHeader, ...]]:
         """
         Sets the canonical chain HEAD to the block header as specified by the
         given block hash.
@@ -198,13 +204,24 @@ def _set_as_canonical_chain_head(self, block_hash: Hash32) -> Tuple[BlockHeader,
             )
 
         new_canonical_headers = tuple(reversed(self._find_new_ancestors(header)))
+        orphaned_headers = []
+
+        for h in new_canonical_headers:
+            try:
+                orphaned_hash = self.get_canonical_block_hash(h.block_number)
+            except HeaderNotFound:
+                # no orphaned block, and no more possible
+                break
+            else:
+                orphaned_header = self.get_block_header_by_hash(orphaned_hash)
+                orphaned_headers.append(orphaned_header)
 
         for h in new_canonical_headers:
             self._add_block_number_to_hash_lookup(h)
 
         self.db.set(SchemaV1.make_canonical_head_hash_lookup_key(), header.hash)
 
-        return new_canonical_headers
+        return new_canonical_headers, tuple(orphaned_headers)
 
     @to_tuple
     def _find_new_ancestors(self, header: BlockHeader) -> Iterable[BlockHeader]:

eth/tools/fixture_tests.py

@@ -641,7 +641,8 @@ def apply_fixture_block_to_chain(block_fixture, chain):
 
     block = rlp.decode(block_fixture['rlp'], sedes=block_class)
 
-    mined_block = chain.import_block(block)
+    imported_blocks, _ = chain.import_block(block)
+    mined_block = imported_blocks[-1]
 
     rlp_encoded_mined_block = rlp.encode(mined_block, sedes=block_class)
 

scripts/benchmark/checks/import_empty_blocks.py

@@ -47,7 +47,9 @@ def import_empty_blocks(self, chain: Chain, number_blocks: int) -> int:
 
         total_gas_used = 0
         for _ in range(1, number_blocks + 1):
-            block = chain.import_block(chain.get_vm().block, False)
+            imported_blocks, _ = chain.import_block(chain.get_vm().block, False)
+            block = imported_blocks[-1]
+
             total_gas_used = total_gas_used + block.header.gas_used
             logging.debug(format_block(block))
 

tests/core/chain-object/test_chain.py

@@ -41,7 +41,9 @@ def tx(chain, funded_address, funded_address_private_key):
 @pytest.mark.xfail(reason="modification to initial allocation made the block fixture invalid")
 def test_import_block_validation(valid_chain, funded_address, funded_address_initial_balance):
     block = rlp.decode(valid_block_rlp, sedes=FrontierBlock)
-    imported_block = valid_chain.import_block(block)
+    imported_blocks, _ = valid_chain.import_block(block)
+    imported_block = imported_blocks[-1]
+
     assert len(imported_block.transactions) == 1
     tx = imported_block.transactions[0]
     assert tx.value == 10
@@ -64,7 +66,9 @@ def test_import_block(chain, tx):
         new_block, receipts, computations = chain.build_block_with_transactions([tx])
         assert computations[0].is_success
 
-    block = chain.import_block(new_block)
+    imported_blocks, _ = chain.import_block(new_block)
+    block = imported_blocks[-1]
+
     assert block.transactions == (tx,)
     assert chain.get_block_by_hash(block.hash) == block
     assert chain.get_canonical_block_by_number(block.number) == block

tests/core/chain-object/test_header_chain.py

@@ -106,17 +106,17 @@ def test_header_chain_import_block(header_chain, genesis_header):
     chain_c = mk_header_chain(genesis_header, 5)
 
     for header in chain_a:
-        res = header_chain.import_header(header)
+        res, _ = header_chain.import_header(header)
         assert res == (header,)
         assert_headers_eq(header_chain.header, header)
 
     for header in chain_b:
-        res = header_chain.import_header(header)
+        res, _ = header_chain.import_header(header)
         assert res == tuple()
         assert_headers_eq(header_chain.header, chain_a[-1])
 
     for idx, header in enumerate(chain_c, 1):
-        res = header_chain.import_header(header)
+        res, _ = header_chain.import_header(header)
         if idx <= 3:
             # prior to passing up `chain_a` each import should not return new
             # canonical headers.