Merge pull request #1943 from carver/raise-block-not-found-with-trie-v2

Raise BlockNotFound on missing txns or uncles

Author: carver

eth/abc.py

@@ -3197,13 +3197,18 @@ def get_block(self) -> BlockAPI:
     def get_block_by_hash(self, block_hash: Hash32) -> BlockAPI:
         """
         Return the requested block as specified by ``block_hash``.
+
+        :raise eth.exceptions.HeaderNotFound: if the header is missing
+        :raise eth.exceptions.BlockNotFound: if any part of the block body is missing
         """
         ...
 
     @abstractmethod
     def get_block_by_header(self, block_header: BlockHeaderAPI) -> BlockAPI:
         """
         Return the requested block as specified by the ``block_header``.
+
+        :raise eth.exceptions.BlockNotFound: if any part of the block body is missing
         """
         ...
 

eth/db/chain.py

@@ -84,10 +84,10 @@ def get_block_uncles(self, uncles_hash: Hash32) -> Tuple[BlockHeaderAPI, ...]:
             return ()
         try:
             encoded_uncles = self.db[uncles_hash]
-        except KeyError:
+        except KeyError as exc:
             raise HeaderNotFound(
                 f"No uncles found for hash {uncles_hash!r}"
-            )
+            ) from exc
         else:
             return tuple(rlp.decode(encoded_uncles, sedes=rlp.sedes.CountableList(BlockHeader)))
 
@@ -377,7 +377,8 @@ def get(self, key: bytes) -> bytes:
         return self.db[key]
 
     def persist_trie_data_dict(self, trie_data_dict: Dict[Hash32, bytes]) -> None:
-        self._persist_trie_data_dict(self.db, trie_data_dict)
+        with self.db.atomic_batch() as db:
+            self._persist_trie_data_dict(db, trie_data_dict)
 
     @classmethod
     def _persist_trie_data_dict(cls, db: DatabaseAPI, trie_data_dict: Dict[Hash32, bytes]) -> None:

eth/exceptions.py

@@ -33,6 +33,8 @@ class HeaderNotFound(PyEVMError):
 class BlockNotFound(PyEVMError):
     """
     Raised when the block with the given number/hash does not exist.
+    This will happen, for example, if the transactions or uncles are not
+    saved in the database.
     """
     pass
 

eth/vm/forks/frontier/blocks.py

@@ -19,6 +19,9 @@
 )
 
 from eth_hash.auto import keccak
+from trie.exceptions import (
+    MissingTrieNode,
+)
 
 from eth.abc import (
     BlockHeaderAPI,
@@ -29,6 +32,10 @@
 from eth.constants import (
     EMPTY_UNCLE_HASH,
 )
+from eth.exceptions import (
+    BlockNotFound,
+    HeaderNotFound,
+)
 from eth.rlp.blocks import (
     BaseBlock,
 )
@@ -103,13 +110,21 @@ def get_receipts(self, chaindb: ChainDatabaseAPI) -> Tuple[ReceiptAPI, ...]:
     def from_header(cls, header: BlockHeaderAPI, chaindb: ChainDatabaseAPI) -> "FrontierBlock":
         """
         Returns the block denoted by the given block header.
+
+        :raise eth.exceptions.BlockNotFound: if transactions or uncle headers are missing
         """
         if header.uncles_hash == EMPTY_UNCLE_HASH:
             uncles: Tuple[BlockHeader, ...] = ()
         else:
-            uncles = chaindb.get_block_uncles(header.uncles_hash)
-
-        transactions = chaindb.get_block_transactions(header, cls.get_transaction_class())
+            try:
+                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
+
+        try:
+            transactions = chaindb.get_block_transactions(header, cls.get_transaction_class())
+        except MissingTrieNode as exc:
+            raise BlockNotFound(f"Transactions not found in database for {header}: {exc}") from exc
 
         return cls(
             header=header,

newsfragments/1943.feature.rst

@@ -0,0 +1,8 @@
+Methods now raise :class:`~eth.exceptions.BlockNotFound` when retrieving a block, and some part
+of the block is missing. These methods used to raise a KeyError if transactions were missing, or a
+``HeaderNotFound`` if uncles were missing:
+
+  - :meth:`eth.db.chain.ChainDB.get_block_by_header`
+  - :meth:`eth.db.chain.ChainDB.get_block_by_hash` (it still raises a HeaderNotFound if there is no
+    header matching the given hash)
+  - :meth:`~eth.abc.BlockHeaderAPI`

tests/database/test_eth1_chaindb.py

@@ -21,6 +21,7 @@
 )
 from eth.db.schema import SchemaV1
 from eth.exceptions import (
+    BlockNotFound,
     HeaderNotFound,
     ParentNotFound,
     ReceiptNotFound,
@@ -291,3 +292,19 @@ def test_chaindb_persist_unexecuted_block(chain,
             NUMBER_BLOCKS_IN_CHAIN,
             TRANSACTIONS_IN_BLOCK + 1,
         )
+
+
+def test_chaindb_raises_blocknotfound_on_missing_uncles(VM, chaindb, header):
+    bad_header = header.copy(uncles_hash=b'unicorns' * 4)
+    chaindb.persist_header(bad_header)
+
+    with pytest.raises(BlockNotFound):
+        VM.get_block_class().from_header(bad_header, chaindb)
+
+
+def test_chaindb_raises_blocknotfound_on_missing_transactions(VM, chaindb, header):
+    bad_header = header.copy(transaction_root=b'unicorns' * 4)
+    chaindb.persist_header(bad_header)
+
+    with pytest.raises(BlockNotFound):
+        VM.get_block_class().from_header(bad_header, chaindb)