From b793c11294898aa64c946007479b96bdaf6d1de7 Mon Sep 17 00:00:00 2001 From: Jason Carver Date: Wed, 2 May 2018 14:02:16 -0700 Subject: [PATCH 1/3] move get_block_by_header from vm to chain --- evm/chains/base.py | 6 +----- evm/vm/base.py | 7 ------- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/evm/chains/base.py b/evm/chains/base.py index f1a9f3a4a7..f2852b9a45 100644 --- a/evm/chains/base.py +++ b/evm/chains/base.py @@ -145,7 +145,7 @@ def get_block_by_hash(self, block_hash): def get_block_by_header(self, block_header): vm = self.get_vm(block_header) - return vm.get_block_by_header(block_header, self.chaindb) + return vm.get_block_class().from_header(block_header, self.chaindb) @to_tuple def get_ancestors(self, limit): @@ -422,10 +422,6 @@ def get_block_by_hash(self, block_hash): block_header = self.get_block_header_by_hash(block_hash) return self.get_block_by_header(block_header) - def get_block_by_header(self, block_header): - vm = self.get_vm(block_header) - return vm.get_block_by_header(block_header, self.chaindb) - # # Chain Initialization # diff --git a/evm/vm/base.py b/evm/vm/base.py index 3a3a66b26d..e918c6a45c 100644 --- a/evm/vm/base.py +++ b/evm/vm/base.py @@ -435,13 +435,6 @@ def get_block_class(cls): """ return cls.get_state_class().get_block_class() - @classmethod - def get_block_by_header(cls, block_header, db): - """ - Lookup and return the block for the given header. - """ - return cls.get_block_class().from_header(block_header, db) - @classmethod @functools.lru_cache(maxsize=32) @to_tuple From 0b257bfd611cfabfc79b9280fdf89a67267c7c4d Mon Sep 17 00:00:00 2001 From: Jason Carver Date: Wed, 2 May 2018 14:18:18 -0700 Subject: [PATCH 2/3] vm should only validate its own blocks --- evm/vm/base.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/evm/vm/base.py b/evm/vm/base.py index e918c6a45c..684fdfbb73 100644 --- a/evm/vm/base.py +++ b/evm/vm/base.py @@ -315,6 +315,13 @@ def validate_block(self, block): """ Validate the the given block. """ + if not isinstance(block, self.get_block_class()): + raise ValidationError( + "This vm ({0!r}) is not equipped to validate a block of type {1!r}".format( + self, + block, + ) + ) if not block.is_genesis: parent_header = get_parent_header(block.header, self.chaindb) From 9356fca188ea53ec0d3f883e9b95faacd9097a9f Mon Sep 17 00:00:00 2001 From: Jason Carver Date: Wed, 2 May 2018 17:03:28 -0700 Subject: [PATCH 3/3] Remove gas_used from state, and related cleanup --- evm/chains/base.py | 2 +- evm/vm/base.py | 67 ++++++++++++++++++---------- evm/vm/forks/byzantium/__init__.py | 18 +++----- evm/vm/forks/frontier/__init__.py | 8 ++-- evm/vm/forks/frontier/state.py | 2 +- evm/vm/forks/frontier/validation.py | 20 ++++++--- evm/vm/forks/homestead/state.py | 2 +- evm/vm/forks/homestead/validation.py | 4 +- evm/vm/state.py | 4 +- tests/core/helpers.py | 6 +-- tests/core/vm/test_vm.py | 2 +- tests/json-fixtures/test_state.py | 2 +- 12 files changed, 79 insertions(+), 58 deletions(-) diff --git a/evm/chains/base.py b/evm/chains/base.py index f2852b9a45..e92e186035 100644 --- a/evm/chains/base.py +++ b/evm/chains/base.py @@ -484,7 +484,7 @@ def apply_transaction(self, transaction): vm = self.get_vm() base_block = vm.block - new_header, receipt, computation = vm.apply_transaction(transaction) + new_header, receipt, computation = vm.apply_transaction(base_block.header, transaction) transactions = base_block.transactions + (transaction, ) receipts = base_block.get_receipts(self.chaindb) + (receipt, ) diff --git a/evm/vm/base.py b/evm/vm/base.py index 684fdfbb73..8e838f8624 100644 --- a/evm/vm/base.py +++ b/evm/vm/base.py @@ -75,7 +75,6 @@ def __init__(self, header, chaindb): db=self.chaindb.db, execution_context=self.block.header.create_execution_context(self.previous_hashes), state_root=self.block.header.state_root, - gas_used=self.block.header.gas_used, ) # @@ -131,30 +130,64 @@ def execute_bytecode(self, ) @abstractmethod - def make_receipt(self, transaction, computation, state): + def make_receipt(self, base_header, transaction, computation, state): """ - Make receipt. + Generate the receipt resulting from applying the transaction. + + :param base_header: the header of the block before the transaction was applied. + :param transaction: the transaction used to generate the receipt + :param computation: the result of running the transaction computation + :param state: the resulting state, after executing the computation + + :return: receipt """ raise NotImplementedError("Must be implemented by subclasses") - def apply_transaction(self, transaction): + @abstractmethod + def validate_transaction_against_header(self, base_header, transaction): + """ + Validate that the given transaction is valid to apply to the given header. + + :param base_header: header before applying the transaction + :param transaction: the transaction to validate + + :raises: ValidationError if the transaction is not valid to apply + """ + raise NotImplementedError("Must be implemented by subclasses") + + def apply_transaction(self, header, transaction): """ Apply the transaction to the current block. This is a wrapper around :func:`~evm.vm.state.State.apply_transaction` with some extra orchestration logic. + + :param header: header of the block before application + :param transaction: to apply """ + self.validate_transaction_against_header(header, transaction) state_root, computation = self.state.apply_transaction(transaction) - receipt = self.make_receipt(transaction, computation, self.state) - # TODO: remove this mutation. - self.state.gas_used = receipt.gas_used + receipt = self.make_receipt(header, transaction, computation, self.state) - new_header = self.block.header.copy( - bloom=int(BloomFilter(self.block.header.bloom) | receipt.bloom), + new_header = header.copy( + bloom=int(BloomFilter(header.bloom) | receipt.bloom), gas_used=receipt.gas_used, state_root=state_root, ) return new_header, receipt, computation + def _apply_all_transactions(self, transactions, base_header): + receipts = [] + previous_header = base_header + result_header = base_header + + for transaction in transactions: + result_header, receipt, _ = self.apply_transaction(previous_header, transaction) + + previous_header = result_header + receipts.append(receipt) + + return result_header, receipts + # # Mining # @@ -179,25 +212,14 @@ def import_block(self, block): db=self.chaindb.db, execution_context=self.block.header.create_execution_context(self.previous_hashes), state_root=self.block.header.state_root, - gas_used=self.block.header.gas_used, ) # run all of the transactions. - execution_data = [ - self.apply_transaction(transaction) - for transaction - in block.transactions - ] - if execution_data: - headers, receipts, _ = zip(*execution_data) - header_with_txns = headers[-1] - else: - receipts = tuple() - header_with_txns = self.block.header + last_header, receipts = self._apply_all_transactions(block.transactions, self.block.header) self.block = self.set_block_transactions( self.block, - header_with_txns, + last_header, block.transactions, receipts, ) @@ -283,7 +305,6 @@ def state_in_temp_block(self): db=self.chaindb.db, execution_context=temp_block.header.create_execution_context(prev_hashes), state_root=temp_block.header.state_root, - gas_used=0, ) snapshot = state.snapshot() diff --git a/evm/vm/forks/byzantium/__init__.py b/evm/vm/forks/byzantium/__init__.py index 367ec24d88..c77e55f17f 100644 --- a/evm/vm/forks/byzantium/__init__.py +++ b/evm/vm/forks/byzantium/__init__.py @@ -1,6 +1,3 @@ -from evm.rlp.receipts import ( - Receipt, -) from evm.vm.forks.spurious_dragon import SpuriousDragonVM from evm.vm.forks.frontier import make_frontier_receipt @@ -16,20 +13,15 @@ from .state import ByzantiumState -def make_byzantium_receipt(transaction, computation, state): - old_receipt = make_frontier_receipt(transaction, computation, state) +def make_byzantium_receipt(base_header, transaction, computation, state): + frontier_receipt = make_frontier_receipt(base_header, transaction, computation, state) if computation.is_error: - state_root = EIP658_TRANSACTION_STATUS_CODE_FAILURE + status_code = EIP658_TRANSACTION_STATUS_CODE_FAILURE else: - state_root = EIP658_TRANSACTION_STATUS_CODE_SUCCESS + status_code = EIP658_TRANSACTION_STATUS_CODE_SUCCESS - receipt = Receipt( - state_root=state_root, - gas_used=old_receipt.gas_used, - logs=old_receipt.logs, - ) - return receipt + return frontier_receipt.copy(state_root=status_code) ByzantiumVM = SpuriousDragonVM.configure( diff --git a/evm/vm/forks/frontier/__init__.py b/evm/vm/forks/frontier/__init__.py index 9665c1833d..ed862db0b1 100644 --- a/evm/vm/forks/frontier/__init__.py +++ b/evm/vm/forks/frontier/__init__.py @@ -14,9 +14,10 @@ compute_frontier_difficulty, configure_frontier_header, ) +from .validation import validate_frontier_transaction_against_header -def make_frontier_receipt(transaction, computation, state): +def make_frontier_receipt(base_header, transaction, computation, state): # Reusable for other forks logs = [ @@ -33,7 +34,7 @@ def make_frontier_receipt(transaction, computation, state): gas_refund, (transaction.gas - gas_remaining) // 2, ) - gas_used = state.gas_used + tx_gas_used + gas_used = base_header.gas_used + tx_gas_used receipt = Receipt( state_root=state.state_root, @@ -55,5 +56,6 @@ def make_frontier_receipt(transaction, computation, state): create_header_from_parent=staticmethod(create_frontier_header_from_parent), compute_difficulty=staticmethod(compute_frontier_difficulty), configure_header=configure_frontier_header, - make_receipt=staticmethod(make_frontier_receipt) + make_receipt=staticmethod(make_frontier_receipt), + validate_transaction_against_header=validate_frontier_transaction_against_header, ) diff --git a/evm/vm/forks/frontier/state.py b/evm/vm/forks/frontier/state.py index fd0c55a6b1..1d92967183 100644 --- a/evm/vm/forks/frontier/state.py +++ b/evm/vm/forks/frontier/state.py @@ -181,7 +181,7 @@ class FrontierState(BaseState, FrontierTransactionExecutor): account_db_class = AccountDB # Type[BaseAccountDB] def validate_transaction(self, transaction): - validate_frontier_transaction(self, transaction) + validate_frontier_transaction(self.account_db, transaction) @staticmethod def get_block_reward(): diff --git a/evm/vm/forks/frontier/validation.py b/evm/vm/forks/frontier/validation.py index 0e0c4e82e9..86c39aee14 100644 --- a/evm/vm/forks/frontier/validation.py +++ b/evm/vm/forks/frontier/validation.py @@ -3,9 +3,9 @@ ) -def validate_frontier_transaction(state, transaction): +def validate_frontier_transaction(account_db, transaction): gas_cost = transaction.gas * transaction.gas_price - sender_balance = state.account_db.get_balance(transaction.sender) + sender_balance = account_db.get_balance(transaction.sender) if sender_balance < gas_cost: raise ValidationError( @@ -17,8 +17,16 @@ def validate_frontier_transaction(state, transaction): if sender_balance < total_cost: raise ValidationError("Sender account balance cannot afford txn") - if state.gas_used + transaction.gas > state.gas_limit: - raise ValidationError("Transaction exceeds gas limit") - - if state.account_db.get_nonce(transaction.sender) != transaction.nonce: + if account_db.get_nonce(transaction.sender) != transaction.nonce: raise ValidationError("Invalid transaction nonce") + + +def validate_frontier_transaction_against_header(_vm, base_header, transaction): + if base_header.gas_used + transaction.gas > base_header.gas_limit: + raise ValidationError( + "Transaction exceeds gas limit: using {}, bringing total to {}, but limit is {}".format( + transaction.gas, + base_header.gas_used + transaction.gas, + base_header.gas_limit, + ) + ) diff --git a/evm/vm/forks/homestead/state.py b/evm/vm/forks/homestead/state.py index 8cc43b6313..6c96344edd 100644 --- a/evm/vm/forks/homestead/state.py +++ b/evm/vm/forks/homestead/state.py @@ -10,4 +10,4 @@ class HomesteadState(FrontierState): computation_class = HomesteadComputation def validate_transaction(self, transaction): - validate_homestead_transaction(self, transaction) + validate_homestead_transaction(self.account_db, transaction) diff --git a/evm/vm/forks/homestead/validation.py b/evm/vm/forks/homestead/validation.py index d8f74c23ff..ad75705554 100644 --- a/evm/vm/forks/homestead/validation.py +++ b/evm/vm/forks/homestead/validation.py @@ -10,8 +10,8 @@ ) -def validate_homestead_transaction(evm, transaction): +def validate_homestead_transaction(account_db, transaction): if transaction.s > SECPK1_N // 2 or transaction.s == 0: raise ValidationError("Invalid signature S value") - validate_frontier_transaction(evm, transaction) + validate_frontier_transaction(account_db, transaction) diff --git a/evm/vm/state.py b/evm/vm/state.py index 51158459f1..66e7e8973a 100644 --- a/evm/vm/state.py +++ b/evm/vm/state.py @@ -63,18 +63,16 @@ class for vm execution. _chaindb = None execution_context = None state_root = None - gas_used = None block_class = None # type: Type[BaseBlock] computation_class = None # type: Type[BaseComputation] transaction_context_class = None # type: Type[BaseTransactionContext] account_db_class = None # type: Type[BaseAccountDB] - def __init__(self, db, execution_context, state_root, gas_used): + def __init__(self, db, execution_context, state_root): self._db = db self.execution_context = execution_context self.account_db = self.get_account_db_class()(self._db, state_root) - self.gas_used = gas_used # # Logging diff --git a/tests/core/helpers.py b/tests/core/helpers.py index b40e1f3318..bf6e04f0ca 100644 --- a/tests/core/helpers.py +++ b/tests/core/helpers.py @@ -36,16 +36,16 @@ def fill_block(chain, from_, key, gas, data): amount = 100 vm = chain.get_vm() - assert vm.state.gas_used == 0 + assert vm.block.header.gas_used == 0 while True: tx = new_transaction(chain.get_vm(), from_, recipient, amount, key, gas=gas, data=data) try: chain.apply_transaction(tx) except ValidationError as exc: - if "Transaction exceeds gas limit" == str(exc): + if str(exc).startswith("Transaction exceeds gas limit"): break else: raise exc - assert chain.get_vm().state.gas_used > 0 + assert chain.get_vm().block.header.gas_used > 0 diff --git a/tests/core/vm/test_vm.py b/tests/core/vm/test_vm.py index a6decb1147..f9942e6a16 100644 --- a/tests/core/vm/test_vm.py +++ b/tests/core/vm/test_vm.py @@ -24,7 +24,7 @@ def test_apply_transaction( amount = 100 from_ = funded_address tx = new_transaction(vm, from_, recipient, amount, funded_address_private_key) - new_header, _, computation = vm.apply_transaction(tx) + new_header, _, computation = vm.apply_transaction(vm.block.header, tx) assert not computation.is_error tx_gas = tx.gas_price * constants.GAS_TX diff --git a/tests/json-fixtures/test_state.py b/tests/json-fixtures/test_state.py index bfc5803258..f923a94c46 100644 --- a/tests/json-fixtures/test_state.py +++ b/tests/json-fixtures/test_state.py @@ -307,7 +307,7 @@ def test_state_fixtures(fixture, fixture_vm_class): ) try: - header, receipt, computation = vm.apply_transaction(transaction) + header, receipt, computation = vm.apply_transaction(vm.block.header, transaction) transactions = vm.block.transactions + (transaction, ) receipts = vm.block.get_receipts(chaindb) + (receipt, ) block = vm.set_block_transactions(vm.block, header, transactions, receipts)