Move receipt validation to Chain and VM methods (#1256)

* move receipt validation to Chain and VM method

* fix light chain

* PR feedback

Author: pipermerriam

eth/chains/base.py

@@ -78,6 +78,9 @@
     BlockHeader,
     HeaderParams,
 )
+from eth.rlp.receipts import (
+    Receipt,
+)
 from eth.rlp.transactions import (
     BaseTransaction,
     BaseUnsignedTransaction,
@@ -277,6 +280,10 @@ def import_block(self,
     #
     # Validation API
     #
+    @abstractmethod
+    def validate_receipt(self, receipt: Receipt, at_header: BlockHeader) -> None:
+        raise NotImplementedError("Chain classes must implement this method")
+
     @abstractmethod
     def validate_block(self, block: BaseBlock) -> None:
         raise NotImplementedError("Chain classes must implement this method")
@@ -665,6 +672,10 @@ def import_block(self,
     #
     # Validation API
     #
+    def validate_receipt(self, receipt: Receipt, at_header: BlockHeader) -> None:
+        VM = self.get_vm_class(at_header)
+        VM.validate_receipt(receipt)
+
     def validate_block(self, block: BaseBlock) -> None:
         """
         Performs validation on a block that is either being mined or imported.
@@ -876,3 +887,8 @@ async def coro_validate_chain(
             chain: Tuple[BlockHeader, ...],
             seal_check_random_sample_rate: int = 1) -> None:
         raise NotImplementedError()
+
+    async def coro_validate_receipt(self,
+                                    receipt: Receipt,
+                                    at_header: BlockHeader) -> None:
+        raise NotImplementedError()

eth/rlp/receipts.py

@@ -6,15 +6,11 @@
     CountableList,
     binary,
 )
-from eth_utils import (
-    ValidationError,
-)
 
 from eth_bloom import BloomFilter
 
 from .sedes import (
     int256,
-    int32,
 )
 
 from .logs import Log
@@ -48,20 +44,6 @@ def __init__(self,
             logs=logs,
         )
 
-        for log_idx, log in enumerate(self.logs):
-            if log.address not in self.bloom_filter:
-                raise ValidationError(
-                    "The address from the log entry at position {0} is not "
-                    "present in the provided bloom filter.".format(log_idx)
-                )
-            for topic_idx, topic in enumerate(log.topics):
-                if int32.serialize(topic) not in self.bloom_filter:
-                    raise ValidationError(
-                        "The topic at position {0} from the log entry at "
-                        "position {1} is not present in the provided bloom "
-                        "filter.".format(topic_idx, log_idx)
-                    )
-
     @property
     def bloom_filter(self) -> BloomFilter:
         return BloomFilter(self.bloom)

eth/vm/base.py

@@ -6,8 +6,7 @@
 import contextlib
 import functools
 import logging
-from typing import (  # noqa: F401
-    List,
+from typing import (
     Type,
 )
 
@@ -36,13 +35,16 @@
 from eth.exceptions import (
     HeaderNotFound,
 )
-from eth.rlp.blocks import (  # noqa: F401
+from eth.rlp.blocks import (
     BaseBlock,
 )
 from eth.rlp.headers import (
     BlockHeader,
 )
-from eth.rlp.receipts import Receipt  # noqa: F401
+from eth.rlp.receipts import Receipt
+from eth.rlp.sedes import (
+    int32,
+)
 from eth.utils.datatypes import (
     Configurable,
 )
@@ -189,7 +191,7 @@ def generate_block_from_parent_header_and_coinbase(cls, parent_header, coinbase)
 
     @classmethod
     @abstractmethod
-    def get_block_class(cls) -> Type['BaseBlock']:
+    def get_block_class(cls) -> Type[BaseBlock]:
         raise NotImplementedError("VM classes must implement this method")
 
     @staticmethod
@@ -251,6 +253,11 @@ def get_transaction_class(cls):
     #
     # Validate
     #
+    @classmethod
+    @abstractmethod
+    def validate_receipt(self, receipt: Receipt) -> None:
+        raise NotImplementedError("VM classes must implement this method")
+
     @abstractmethod
     def validate_block(self, block):
         raise NotImplementedError("VM classes must implement this method")
@@ -348,6 +355,7 @@ def apply_transaction(self, header, transaction):
         self.validate_transaction_against_header(header, transaction)
         state_root, computation = self.state.apply_transaction(transaction)
         receipt = self.make_receipt(header, transaction, computation, self.state)
+        self.validate_receipt(receipt)
 
         new_header = header.copy(
             bloom=int(BloomFilter(header.bloom) | receipt.bloom),
@@ -606,7 +614,7 @@ def generate_block_from_parent_header_and_coinbase(cls, parent_header, coinbase)
         return block
 
     @classmethod
-    def get_block_class(cls) -> Type['BaseBlock']:
+    def get_block_class(cls) -> Type[BaseBlock]:
         """
         Return the :class:`~eth.rlp.blocks.Block` class that this VM uses for blocks.
         """
@@ -664,6 +672,22 @@ def get_transaction_class(cls):
     #
     # Validate
     #
+    @classmethod
+    def validate_receipt(cls, receipt: Receipt) -> None:
+        for log_idx, log in enumerate(receipt.logs):
+            if log.address not in receipt.bloom_filter:
+                raise ValidationError(
+                    "The address from the log entry at position {0} is not "
+                    "present in the provided bloom filter.".format(log_idx)
+                )
+            for topic_idx, topic in enumerate(log.topics):
+                if int32.serialize(topic) not in receipt.bloom_filter:
+                    raise ValidationError(
+                        "The topic at position {0} from the log entry at "
+                        "position {1} is not present in the provided bloom "
+                        "filter.".format(topic_idx, log_idx)
+                    )
+
     def validate_block(self, block):
         """
         Validate the the given block.

eth/vm/forks/byzantium/__init__.py

@@ -2,10 +2,16 @@
     Type,
 )
 
+from eth_utils import (
+    encode_hex,
+    ValidationError,
+)
+
 from eth.constants import (
     MAX_UNCLE_DEPTH,
 )
 from eth.rlp.blocks import BaseBlock  # noqa: F401
+from eth.rlp.receipts import Receipt
 from eth.validation import (
     validate_lte,
 )
@@ -38,6 +44,12 @@ def make_byzantium_receipt(base_header, transaction, computation, state):
     return frontier_receipt.copy(state_root=status_code)
 
 
+EIP658_STATUS_CODES = {
+    EIP658_TRANSACTION_STATUS_CODE_SUCCESS,
+    EIP658_TRANSACTION_STATUS_CODE_FAILURE,
+}
+
+
 class ByzantiumVM(SpuriousDragonVM):
     # fork name
     fork = 'byzantium'
@@ -52,6 +64,19 @@ class ByzantiumVM(SpuriousDragonVM):
     configure_header = configure_byzantium_header
     make_receipt = staticmethod(make_byzantium_receipt)
 
+    @classmethod
+    def validate_receipt(cls, receipt: Receipt) -> None:
+        super().validate_receipt(receipt)
+        if receipt.state_root not in EIP658_STATUS_CODES:
+            raise ValidationError(
+                "The receipt's `state_root` must be one of [{0}, {1}].  Got: "
+                "{2}".format(
+                    encode_hex(EIP658_TRANSACTION_STATUS_CODE_SUCCESS),
+                    encode_hex(EIP658_TRANSACTION_STATUS_CODE_FAILURE),
+                    encode_hex(receipt.state_root),
+                )
+            )
+
     @staticmethod
     def get_block_reward():
         return EIP649_BLOCK_REWARD

tests/trinity/core/integration_test_helpers.py

@@ -76,14 +76,17 @@ class FakeAsyncRopstenChain(RopstenChain):
     chaindb_class = FakeAsyncChainDB
     coro_import_block = coro_import_block
     coro_validate_chain = async_passthrough('validate_chain')
+    coro_validate_receipt = async_passthrough('validate_receipt')
 
 
 class FakeAsyncMainnetChain(MainnetChain):
     chaindb_class = FakeAsyncChainDB
     coro_import_block = coro_import_block
     coro_validate_chain = async_passthrough('validate_chain')
+    coro_validate_receipt = async_passthrough('validate_receipt')
 
 
 class FakeAsyncChain(MiningChain):
     coro_import_block = coro_import_block
     coro_validate_chain = async_passthrough('validate_chain')
+    coro_validate_receipt = async_passthrough('validate_receipt')

trinity/chains/__init__.py

@@ -260,6 +260,7 @@ class DBManager(BaseManager):
 class ChainProxy(BaseProxy):
     coro_import_block = async_method('import_block')
     coro_validate_chain = async_method('validate_chain')
+    coro_validate_receipt = async_method('validate_receipt')
     get_vm_configuration = sync_method('get_vm_configuration')
     get_vm_class = sync_method('get_vm_class')
     get_vm_class_for_block_number = sync_method('get_vm_class_for_block_number')

trinity/chains/light.py

@@ -213,6 +213,9 @@ def mine_block(self, *args: Any, **kwargs: Any) -> BaseBlock:
     #
     # Validation API
     #
+    def validate_receipt(self, receipt: Receipt, at_header: BlockHeader) -> None:
+        raise NotImplementedError("Chain classes must implement " + inspect.stack()[0][3])
+
     def validate_block(self, block: BaseBlock) -> None:
         raise NotImplementedError("Chain classes must implement " + inspect.stack()[0][3])
 

trinity/sync/full/chain.py

@@ -23,13 +23,16 @@
 
 from eth_typing import Hash32
 
+from eth_utils import ValidationError
+
 from eth.constants import (
     BLANK_ROOT_HASH, EMPTY_UNCLE_HASH, GENESIS_PARENT_HASH)
 from eth.rlp.headers import BlockHeader
 from eth.rlp.receipts import Receipt
 from eth.rlp.transactions import BaseTransaction
 
 from p2p import protocol
+from p2p.p2p_proto import DisconnectReason
 from p2p.exceptions import NoEligiblePeers, PeerConnectionLost
 from p2p.protocol import Command
 
@@ -354,7 +357,32 @@ async def _get_receipts(self,
         if not receipt_bundles:
             return tuple(), batch
 
-        receipts, trie_roots_and_data_dicts = zip(*receipt_bundles)
+        # First validate the receipts.
+        header_by_root = {
+            header.receipt_root: header
+            for header in batch
+            if not _is_receipts_empty(header)
+        }
+        receipts_by_root = {
+            receipt_root: receipts
+            for (receipts, (receipt_root, _))
+            in receipt_bundles
+            if receipt_root != BLANK_ROOT_HASH
+        }
+        for receipt_root, header in header_by_root.items():
+            for receipt in receipts_by_root[receipt_root]:
+                try:
+                    await self.chain.coro_validate_receipt(receipt, header)
+                except ValidationError as err:
+                    self.logger.info(
+                        "Disconnecting from %s: sent invalid receipt: %s",
+                        peer,
+                        err,
+                    )
+                    await peer.disconnect(DisconnectReason.bad_protocol)
+                    return tuple(), batch
+
+        _, trie_roots_and_data_dicts = zip(*receipt_bundles)
         receipt_roots, trie_data_dicts = zip(*trie_roots_and_data_dicts)
         receipt_roots_set = set(receipt_roots)
         missing = tuple(