Merge pull request #565 from gsalgado/block-import-trie-generation-re…

…design

Redesign receipt/tx trie generation on block imports

evm/chains/base.py

@@ -209,13 +209,6 @@ def get_vm(self, header=None):
     #
     # Execution API
     #
-    @abstractmethod
-    def apply_transaction(self, transaction):
-        """
-        Applies the transaction to the current head block of the Chain.
-        """
-        raise NotImplementedError("Chain classes must implement this method")
-
     @abstractmethod
     def estimate_gas(self, transaction, at_header=None):
         """
@@ -487,18 +480,6 @@ def from_genesis_header(cls, chaindb, genesis_header):
     #
     # Mining and Execution API
     #
-    def apply_transaction(self, transaction):
-        """
-        Applies the transaction to the current head block of the Chain.
-        """
-        vm = self.get_vm()
-        computation, block = vm.apply_transaction(transaction)
-
-        # Update header
-        self.header = block.header
-
-        return computation
-
     def estimate_gas(self, transaction, at_header=None):
         if at_header is None:
             at_header = self.get_canonical_head()

evm/vm/base.py

@@ -3,12 +3,12 @@
     ABCMeta,
     abstractmethod
 )
-
 import contextlib
+import functools
 import logging
-import rlp
+from typing import List  # noqa: F401
 
-import functools
+import rlp
 
 from eth_utils import (
     keccak,
@@ -28,6 +28,7 @@
 from evm.rlp.headers import (
     BlockHeader,
 )
+from evm.rlp.receipts import Receipt  # noqa: F401
 from evm.utils.datatypes import (
     Configurable,
 )
@@ -62,6 +63,7 @@ def __init__(self, header, chaindb):
         self.chaindb = chaindb
         block_class = self.get_block_class()
         self.block = block_class.from_header(header=header, chaindb=self.chaindb)
+        self.receipts = []  # type: List[Receipt]
 
     #
     # Logging
@@ -77,14 +79,12 @@ def apply_transaction(self, transaction):
         """
         Apply the transaction to the vm in the current block.
         """
-        computation, block, trie_data_dict = self.state.apply_transaction(
+        computation, block, receipt = self.state.apply_transaction(
             transaction,
             self.block,
         )
         self.block = block
-
-        # Persist changed transaction and receipt key-values to self.chaindb.
-        self.chaindb.persist_trie_data_dict(trie_data_dict)
+        self.receipts.append(receipt)
 
         self.clear_journal()
 
@@ -132,6 +132,7 @@ def execute_bytecode(self,
     # Mining
     #
     def import_block(self, block):
+        self.receipts = []
         self.configure_header(
             coinbase=block.header.coinbase,
             gas_limit=block.header.gas_limit,
@@ -156,6 +157,13 @@ def mine_block(self, *args, **kwargs):
         Mine the current block. Proxies to self.pack_block method.
         """
         block = self.block
+        tx_root_hash, tx_kv_nodes = make_trie_root_and_nodes(block.transactions)
+        receipt_root_hash, receipt_kv_nodes = make_trie_root_and_nodes(self.receipts)
+        self.chaindb.persist_trie_data_dict(tx_kv_nodes)
+        self.chaindb.persist_trie_data_dict(receipt_kv_nodes)
+        self.block.header.transaction_root = tx_root_hash
+        self.block.header.receipt_root = receipt_root_hash
+
         self.pack_block(block, *args, **kwargs)
 
         if block.number == 0:
@@ -211,7 +219,10 @@ def state_in_temp_block(self):
         header = self.block.header
         temp_block = self.generate_block_from_parent_header_and_coinbase(header, header.coinbase)
         prev_hashes = (header.hash, ) + self.previous_hashes
-        state = self.get_state(self.chaindb, temp_block, prev_hashes)
+        gas_used = 0
+        if self.receipts:
+            gas_used = self.receipts[-1].gas_used
+        state = self.get_state(self.chaindb, temp_block, prev_hashes, gas_used)
         assert state.gas_used == 0, "There must not be any gas used in a fresh temporary block"
 
         snapshot = state.snapshot()
@@ -266,6 +277,8 @@ def validate_block(self, block):
                     )
                 )
 
+        # FIXME: make_trie_root_and_nodes() is rather expensive, and we already run that once in
+        # import_block(), so should refactor some of this code to avoid re-generating it here.
         tx_root_hash, _ = make_trie_root_and_nodes(block.transactions)
         if tx_root_hash != block.header.transaction_root:
             raise ValidationError(
@@ -435,7 +448,7 @@ def get_state_class(cls):
         return cls._state_class
 
     @classmethod
-    def get_state(cls, chaindb, block, prev_hashes):
+    def get_state(cls, chaindb, block, prev_hashes, gas_used):
         """
         Return state object
 
@@ -445,21 +458,24 @@ def get_state(cls, chaindb, block, prev_hashes):
         :return: state with root defined in block
         """
         execution_context = block.header.create_execution_context(prev_hashes)
-        receipts = block.get_receipts(chaindb)
 
         return cls.get_state_class()(
             chaindb,
             execution_context=execution_context,
             state_root=block.header.state_root,
-            receipts=receipts,
+            gas_used=gas_used,
         )
 
     @property
     def state(self):
         """Return current state property
         """
+        gas_used = 0
+        if self.receipts:
+            gas_used = self.receipts[-1].gas_used
         return self.get_state(
             chaindb=self.chaindb,
             block=self.block,
-            prev_hashes=self.previous_hashes
+            prev_hashes=self.previous_hashes,
+            gas_used=gas_used,
         )

evm/vm/state.py

@@ -9,10 +9,6 @@
     TYPE_CHECKING
 )
 
-from cytoolz import (
-    merge,
-)
-
 from eth_utils import (
     to_bytes
 )
@@ -25,9 +21,6 @@
     MAX_PREV_HEADER_DEPTH,
     UINT_256_MAX,
 )
-from evm.db.trie import (
-    make_trie_root_and_nodes,
-)
 from evm.utils.datatypes import (
     Configurable,
 )
@@ -51,19 +44,18 @@ class BaseState(Configurable, metaclass=ABCMeta):
     _chaindb = None
     execution_context = None
     state_root = None
-    receipts = None
-    access_logs = None
+    gas_used = None
 
     block_class = None  # type: Type[BaseBlock]
     computation_class = None  # type: Type[BaseComputation]
     trie_class = None
     transaction_context_class = None  # type: Type[BaseTransactionContext]
 
-    def __init__(self, chaindb, execution_context, state_root, receipts=[]):
+    def __init__(self, chaindb, execution_context, state_root, gas_used):
         self._chaindb = chaindb
         self.execution_context = execution_context
         self.state_root = state_root
-        self.receipts = receipts
+        self.gas_used = gas_used
 
     #
     # Logging
@@ -96,16 +88,6 @@ def difficulty(self):
     def gas_limit(self):
         return self.execution_context.gas_limit
 
-    #
-    # Helpers
-    #
-    @property
-    def gas_used(self):
-        if self.receipts:
-            return self.receipts[-1].gas_used
-        else:
-            return 0
-
     #
     # read only state_db
     #
@@ -299,9 +281,9 @@ def apply_transaction(
         computation = self.execute_transaction(transaction)
 
         # Set block.
-        block, trie_data_dict = self.add_transaction(transaction, computation, block)
+        block, receipt = self.add_transaction(transaction, computation, block)
         block.header.state_root = self.state_root
-        return computation, block, trie_data_dict
+        return computation, block, receipt
 
     def add_transaction(self, transaction, computation, block):
         """
@@ -322,7 +304,7 @@ def add_transaction(self, transaction, computation, block):
         :rtype: (Block, dict[bytes, bytes])
         """
         receipt = self.make_receipt(transaction, computation)
-        self.add_receipt(receipt)
+        self.gas_used = receipt.gas_used
 
         # Create a new Block object
         block_header = block.header.clone()
@@ -331,29 +313,12 @@ def add_transaction(self, transaction, computation, block):
 
         block.transactions.append(transaction)
 
-        # Get trie roots and changed key-values.
-        tx_root_hash, tx_kv_nodes = make_trie_root_and_nodes(
-            block.transactions,
-            self.trie_class,
-        )
-        receipt_root_hash, receipt_kv_nodes = make_trie_root_and_nodes(
-            self.receipts,
-            self.trie_class,
-        )
-
-        trie_data = merge(tx_kv_nodes, receipt_kv_nodes)
-
         block.bloom_filter |= receipt.bloom
 
-        block.header.transaction_root = tx_root_hash
-        block.header.receipt_root = receipt_root_hash
         block.header.bloom = int(block.bloom_filter)
         block.header.gas_used = receipt.gas_used
 
-        return block, trie_data
-
-    def add_receipt(self, receipt):
-        self.receipts.append(receipt)
+        return block, receipt
 
     @abstractmethod
     def make_receipt(self, transaction, computation):

tests/core/chain-object/test_chain.py

@@ -34,17 +34,6 @@ def tx(chain, funded_address, funded_address_private_key):
     return new_transaction(vm, from_, recipient, amount, funded_address_private_key)
 
 
-def test_apply_transaction(chain, tx):
-    vm = chain.get_vm()
-
-    computation = chain.apply_transaction(tx)
-
-    # Check if the state is updated.
-    vm = chain.get_vm()
-    assert vm.state.state_root == computation.state.state_root
-    assert vm.state.read_only_state_db.get_balance(tx.to) == tx.value
-
-
 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)

tests/core/helpers.py

@@ -35,17 +35,17 @@ def fill_block(chain, from_, key, gas, data):
     recipient = decode_hex('0xa94f5374fce5edbc8e2a8697c15331677e6ebf0c')
     amount = 100
 
-    assert chain.get_vm().state.gas_used == 0
+    vm = chain.get_vm()
+    assert vm.state.gas_used == 0
 
     while True:
-        vm = chain.get_vm()
         tx = new_transaction(vm, from_, recipient, amount, key, gas=gas, data=data)
         try:
-            chain.apply_transaction(tx)
+            vm.apply_transaction(tx)
         except ValidationError as exc:
             if "Transaction exceeds gas limit" == str(exc):
                 break
             else:
                 raise exc
 
-    assert chain.get_vm().state.gas_used > 0
+    assert vm.state.gas_used > 0

tests/core/vm/test_vm_state.py

@@ -106,7 +106,7 @@ def test_apply_transaction(  # noqa: F811
         chaindb=chaindb1,
         execution_context=execution_context,
         state_root=block1.header.state_root,
-        receipts=[],
+        gas_used=0,
     )
     parent_hash = copy.deepcopy(prev_hashes[0])
 
@@ -124,7 +124,7 @@ def test_apply_transaction(  # noqa: F811
         chaindb=chaindb1,
         execution_context=execution_context,
         state_root=block.header.state_root,
-        receipts=computation.state.receipts,
+        gas_used=computation.state.gas_used,
     )
     computation, block, _ = state1.apply_transaction(
         tx2,