From 69a8385476983b4a060f307061ab258027b64eff Mon Sep 17 00:00:00 2001 From: Piper Merriam Date: Wed, 18 Oct 2017 13:23:12 -0600 Subject: [PATCH 1/4] Spurious Dragon fork rules support --- .travis.yml | 2 +- evm/chains/mainnet/__init__.py | 2 + evm/chains/tester/__init__.py | 31 +++++- evm/constants.py | 13 +++ evm/db/state.py | 13 ++- evm/logic/arithmetic.py | 9 +- evm/logic/call.py | 20 +++- evm/logic/system.py | 32 +++++- evm/utils/fixture_tests.py | 4 +- evm/utils/transactions.py | 75 ++++++++++++- evm/vm/base.py | 8 ++ evm/vm/computation.py | 27 ++--- evm/vm/forks/__init__.py | 3 + evm/vm/forks/eip150/opcodes.py | 10 +- evm/vm/forks/frontier/__init__.py | 33 +++--- evm/vm/forks/frontier/opcodes.py | 2 +- evm/vm/forks/homestead/__init__.py | 10 +- evm/vm/forks/homestead/opcodes.py | 16 ++- evm/vm/forks/spurious_dragon/__init__.py | 101 ++++++++++++++++++ evm/vm/forks/spurious_dragon/blocks.py | 21 ++++ evm/vm/forks/spurious_dragon/opcodes.py | 42 ++++++++ evm/vm/forks/spurious_dragon/transactions.py | 65 +++++++++++ evm/vm/forks/spurious_dragon/utils.py | 41 +++++++ .../tester/test_generate_vm_configuration.py | 30 +++++- .../test_transaction_signature_validation.py | 27 +++++ tests/json-fixtures/test_blockchain.py | 5 +- tests/json-fixtures/test_state.py | 13 ++- tests/json-fixtures/test_transactions.py | 2 +- tests/json-fixtures/test_virtual_machine.py | 4 +- 29 files changed, 595 insertions(+), 66 deletions(-) create mode 100644 evm/vm/forks/spurious_dragon/__init__.py create mode 100644 evm/vm/forks/spurious_dragon/blocks.py create mode 100644 evm/vm/forks/spurious_dragon/opcodes.py create mode 100644 evm/vm/forks/spurious_dragon/transactions.py create mode 100644 evm/vm/forks/spurious_dragon/utils.py diff --git a/.travis.yml b/.travis.yml index a696a4123f..18b3abd579 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,7 +17,7 @@ env: - TOX_POSARGS="-e py35-state-frontier" - TOX_POSARGS="-e py35-state-homestead" - TOX_POSARGS="-e py35-state-eip150" - #- TOX_POSARGS="-e py35-state-eip158" + - TOX_POSARGS="-e py35-state-eip158" #- TOX_POSARGS="-e py35-state-byzantium" #- TOX_POSARGS="-e py35-state-constantinople" #- TOX_POSARGS="-e py35-state-metropolis" diff --git a/evm/chains/mainnet/__init__.py b/evm/chains/mainnet/__init__.py index 0e6c2dcd90..68217f4425 100644 --- a/evm/chains/mainnet/__init__.py +++ b/evm/chains/mainnet/__init__.py @@ -5,6 +5,7 @@ EIP150VM, FrontierVM, HomesteadVM, + SpuriousDragonVM, ) @@ -12,6 +13,7 @@ (0, FrontierVM), (constants.HOMESTEAD_MAINNET_BLOCK, HomesteadVM), (constants.EIP150_MAINNET_BLOCK, EIP150VM), + (constants.SPURIOUS_DRAGON_MAINNET_BLOCK, SpuriousDragonVM), ) diff --git a/evm/chains/tester/__init__.py b/evm/chains/tester/__init__.py index 824230c779..fd1cdb4b31 100644 --- a/evm/chains/tester/__init__.py +++ b/evm/chains/tester/__init__.py @@ -12,6 +12,7 @@ FrontierVM as BaseFrontierVM, HomesteadVM as BaseHomesteadVM, EIP150VM as BaseEIP150VM, + SpuriousDragonVM as BaseSpuriousDragonVM, ) from evm.utils.chain import ( @@ -44,6 +45,10 @@ class EIP150TesterVM(MaintainGasLimitMixin, BaseEIP150VM): pass +class SpuriousDragonTesterVM(MaintainGasLimitMixin, BaseSpuriousDragonVM): + pass + + INVALID_FORK_ACTIVATION_MSG = ( "The {0}-fork activation block may not be null if the {1}-fork block " "is non null" @@ -53,11 +58,27 @@ class EIP150TesterVM(MaintainGasLimitMixin, BaseEIP150VM): @reversed_return def _generate_vm_configuration(homestead_start_block=None, dao_start_block=None, - eip150_start_block=None): + eip150_start_block=None, + spurious_dragon_block=None): # If no explicit configuration has been passed, configure the vm to start # with the latest fork rules at block 0 - if eip150_start_block is None and homestead_start_block is None: - yield (0, EIP150TesterVM) + no_declared_blocks = ( + spurious_dragon_block is None and + eip150_start_block is None and + homestead_start_block is None + ) + if no_declared_blocks: + yield (0, SpuriousDragonTesterVM) + + if spurious_dragon_block is not None: + yield (spurious_dragon_block, SpuriousDragonTesterVM) + + remaining_blocks_not_declared = ( + homestead_start_block is None and + eip150_start_block is None + ) + if spurious_dragon_block > 0 and remaining_blocks_not_declared: + yield (0, EIP150TesterVM) if eip150_start_block is not None: yield (eip150_start_block, EIP150TesterVM) @@ -111,7 +132,8 @@ def validate_seal(self, block): def configure_forks(self, homestead_start_block=None, dao_start_block=None, - eip150_start_block=0): + eip150_start_block=None, + spurious_dragon_block=None): """ TODO: add support for state_cleanup """ @@ -119,5 +141,6 @@ def configure_forks(self, homestead_start_block=homestead_start_block, dao_start_block=dao_start_block, eip150_start_block=eip150_start_block, + spurious_dragon_block=spurious_dragon_block, ) self.vms_by_range = generate_vms_by_range(vm_configuration) diff --git a/evm/constants.py b/evm/constants.py index 2f29b7bbb3..58a29b67be 100644 --- a/evm/constants.py +++ b/evm/constants.py @@ -181,3 +181,16 @@ # EIP150 # EIP150_MAINNET_BLOCK = 2463000 + + +# +# Spurious Dragon +# +SPURIOUS_DRAGON_MAINNET_BLOCK = 2675000 + +# https://github.com/ethereum/EIPs/issues/160 +GAS_EXP_EIP160 = 10 +GAS_EXPBYTE_EIP160 = 50 + +# https://github.com/ethereum/EIPs/issues/170 +EIP170_CODE_SIZE_LIMIT = 24577 diff --git a/evm/db/state.py b/evm/db/state.py index 5605e9406d..4f52670e05 100644 --- a/evm/db/state.py +++ b/evm/db/state.py @@ -160,7 +160,6 @@ def get_code_hash(self, address): def delete_code(self, address): validate_canonical_address(address, title="Storage Address") account = self._get_account(address) - del self.db[account.code_hash] account.code_hash = EMPTY_SHA3 self._set_account(address, account) @@ -185,6 +184,18 @@ def account_has_code_or_nonce(self, address): else: return False + def account_is_empty(self, address): + validate_canonical_address(address, title="Storage Address") + account = self._get_account(address) + if account.code_hash != EMPTY_SHA3: + return False + elif account.balance != 0: + return False + elif account.nonce != 0: + return False + else: + return True + def touch_account(self, address): account = self._get_account(address) self._set_account(address, account) diff --git a/evm/logic/arithmetic.py b/evm/logic/arithmetic.py index 4213519ef4..5f7197a1d6 100644 --- a/evm/logic/arithmetic.py +++ b/evm/logic/arithmetic.py @@ -1,3 +1,7 @@ +from cytoolz import ( + curry, +) + from evm import constants from evm.utils.numeric import ( @@ -133,7 +137,8 @@ def sdiv(computation): computation.stack.push(signed_to_unsigned(result)) -def exp(computation): +@curry +def exp(computation, gas_per_byte): """ Exponentiation """ @@ -148,7 +153,7 @@ def exp(computation): result = pow(base, exponent, constants.UINT_256_CEILING) computation.gas_meter.consume_gas( - constants.GAS_EXPBYTE * byte_size, + gas_per_byte * byte_size, reason="EXP: exponent bytes", ) diff --git a/evm/logic/call.py b/evm/logic/call.py index 0c7a3512cb..378627782a 100644 --- a/evm/logic/call.py +++ b/evm/logic/call.py @@ -224,6 +224,9 @@ def get_call_params(self, computation): ) +# +# EIP150 +# class CallEIP150(Call): def compute_msg_gas(self, computation, gas, to, value): extra_gas = self.compute_msg_extra_gas(computation, gas, to, value) @@ -257,7 +260,7 @@ def compute_eip150_msg_gas(computation, gas, extra_gas, value, mnemonic, callsti # It feels wrong to raise an OutOfGas exception outside of GasMeter, # but I don't see an easy way around it. raise OutOfGas("Out of gas: Needed {0} - Remaining {1} - Reason: {2}".format( - gas, + extra_gas, computation.gas_meter.gas_remaining, mnemonic, )) @@ -267,3 +270,18 @@ def compute_eip150_msg_gas(computation, gas, extra_gas, value, mnemonic, callsti total_fee = gas + extra_gas child_msg_gas = gas + (callstipend if value else 0) return child_msg_gas, total_fee + + +# +# EIP161 +# +class CallEIP161(CallEIP150): + def compute_msg_extra_gas(self, computation, gas, to, value): + with computation.vm.state_db(read_only=True) as state_db: + account_is_dead = ( + not state_db.account_exists(to) or + state_db.account_is_empty(to) + ) + transfer_gas_fee = constants.GAS_CALLVALUE if value else 0 + create_gas_fee = constants.GAS_NEWACCOUNT if (account_is_dead and value) else 0 + return transfer_gas_fee + create_gas_fee diff --git a/evm/logic/system.py b/evm/logic/system.py index 2f6f9c5f06..d622719d10 100644 --- a/evm/logic/system.py +++ b/evm/logic/system.py @@ -34,7 +34,24 @@ def suicide_eip150(computation): with computation.vm.state_db(read_only=True) as state_db: if not state_db.account_exists(beneficiary): computation.gas_meter.consume_gas( - constants.GAS_SUICIDE_NEWACCOUNT, reason=mnemonics.SUICIDE) + constants.GAS_SUICIDE_NEWACCOUNT, + reason=mnemonics.SUICIDE, + ) + _suicide(computation, beneficiary) + + +def suicide_eip161(computation): + beneficiary = force_bytes_to_address(computation.stack.pop(type_hint=constants.BYTES)) + with computation.vm.state_db(read_only=True) as state_db: + is_dead = ( + not state_db.account_exists(beneficiary) or + state_db.account_is_empty(beneficiary) + ) + if is_dead and state_db.get_balance(computation.msg.storage_address): + computation.gas_meter.consume_gas( + constants.GAS_SUICIDE_NEWACCOUNT, + reason=mnemonics.SUICIDE, + ) _suicide(computation, beneficiary) @@ -50,8 +67,15 @@ def _suicide(computation, beneficiary): # beneficiary. state_db.set_balance(computation.msg.storage_address, 0) + computation.vm.logger.debug( + "SUICIDE: %s (%s) -> %s", + encode_hex(computation.msg.storage_address), + local_balance, + encode_hex(beneficiary), + ) + # 3rd: Register the account to be deleted - computation.register_account_for_deletion(computation.msg.storage_address) + computation.register_account_for_deletion(beneficiary) class Create(Opcode): @@ -80,7 +104,8 @@ def __call__(self, computation): call_data = computation.memory.read(start_position, size) create_msg_gas = self.max_child_gas_modifier( - computation.gas_meter.gas_remaining) + computation.gas_meter.gas_remaining + ) computation.gas_meter.consume_gas(create_msg_gas, reason="CREATE") with computation.vm.state_db() as state_db: @@ -112,7 +137,6 @@ def __call__(self, computation): ) child_computation = computation.vm.apply_create_message(child_msg) - computation.children.append(child_computation) if child_computation.error: diff --git a/evm/utils/fixture_tests.py b/evm/utils/fixture_tests.py index 363e787a46..ada479ed62 100644 --- a/evm/utils/fixture_tests.py +++ b/evm/utils/fixture_tests.py @@ -21,9 +21,9 @@ pad_left, remove_0x_prefix, to_canonical_address, - to_dict, to_normalized_address, to_tuple, + to_dict, ) from evm.constants import ( @@ -291,6 +291,7 @@ def normalize_unsigned_transaction(transaction, indexes): yield 'gasLimit', to_int(transaction['gasLimit'][indexes['gas']]) yield 'gasPrice', to_int(transaction['gasPrice']) yield 'nonce', to_int(transaction['nonce']) + if 'secretKey' in transaction: yield 'secretKey', decode_hex(transaction['secretKey']) elif 'v' in transaction and 'r' in transaction and 's' in transaction: @@ -301,6 +302,7 @@ def normalize_unsigned_transaction(transaction, indexes): ) else: raise KeyError("transaction missing secret key or vrs values") + yield 'to', normalize_to_address(transaction['to']) yield 'value', to_int(transaction['value'][indexes['value']]) diff --git a/evm/utils/transactions.py b/evm/utils/transactions.py index e0fa9d3cf8..1be6b25e34 100644 --- a/evm/utils/transactions.py +++ b/evm/utils/transactions.py @@ -9,6 +9,14 @@ from evm.exceptions import ( ValidationError, ) +from evm.validation import ( + validate_gt, +) + +from evm.utils.numeric import ( + is_even, + int_to_big_endian, +) def create_transaction_signature(unsigned_txn, private_key): @@ -18,6 +26,23 @@ def create_transaction_signature(unsigned_txn, private_key): return v, r, s +def create_eip155_transaction_signature(unsigned_txn, chain_id, private_key): + transaction_parts = rlp.decode(rlp.encode(unsigned_txn)) + transaction_parts_for_signature = ( + transaction_parts[:-3] + [int_to_big_endian(chain_id), b'', b''] + ) + message = rlp.encode(transaction_parts_for_signature) + + signature = private_key.sign_msg(message) + canonical_v, r, s = signature.vrs + v = canonical_v + 27 + if v == 27: + eip155_v = chain_id * 2 + 36 + else: + eip155_v = chain_id * 2 + 35 + return eip155_v, r, s + + def validate_transaction_signature(transaction): v, r, s = transaction.v, transaction.r, transaction.s canonical_v = v - 27 @@ -33,8 +58,56 @@ def validate_transaction_signature(transaction): raise ValidationError("Invalid Signature") +def is_eip_155_signed_transaction(transaction): + if transaction.v >= 35: + return True + else: + return False + + +def extract_chain_id(v): + if is_even(v): + return (v - 36) // 2 + else: + return (v - 35) // 2 + + +def extract_signature_v(v): + if is_even(v): + return 28 + else: + return 27 + + +def validate_eip155_transaction_signature(transaction): + validate_gt(transaction.v, 34, title="Transaction.v") + + v = extract_signature_v(transaction.v) + + canonical_v = v - 27 + vrs = (canonical_v, transaction.r, transaction.s) + signature = keys.Signature(vrs=vrs) + message = transaction.get_message_for_signing() + + try: + public_key = signature.recover_public_key_from_msg(message) + except BadSignature as e: + raise ValidationError("Bad Signature: {0}".format(str(e))) + + if not signature.verify_msg(message, public_key): + raise ValidationError("Invalid Signature") + + def extract_transaction_sender(transaction): - v, r, s = transaction.v, transaction.r, transaction.s + if is_eip_155_signed_transaction(transaction): + if is_even(transaction.v): + v = 28 + else: + v = 27 + else: + v = transaction.v + + r, s = transaction.r, transaction.s canonical_v = v - 27 vrs = (canonical_v, r, s) diff --git a/evm/vm/base.py b/evm/vm/base.py index 97a6a63fc1..e780bfd744 100644 --- a/evm/vm/base.py +++ b/evm/vm/base.py @@ -31,6 +31,7 @@ class VM(object): """ opcodes = None _block_class = None + _precompiles = None def __init__(self, header, chaindb): if chaindb is None: @@ -76,6 +77,13 @@ def state_db(self, read_only=False): state.db = None state._trie = None + @property + def precompiles(self): + if self._precompiles is None: + return set() + else: + return self._precompiles + # # Logging # diff --git a/evm/vm/computation.py b/evm/vm/computation.py index 4f8525c5b4..eb275823bb 100644 --- a/evm/vm/computation.py +++ b/evm/vm/computation.py @@ -181,15 +181,6 @@ def register_account_for_deletion(self, beneficiary): ) self.accounts_to_delete[self.msg.storage_address] = beneficiary - def get_accounts_for_deletion(self): - if self.error: - return tuple(dict().items()) - else: - return tuple(dict(itertools.chain( - self.accounts_to_delete.items(), - *(child.get_accounts_for_deletion() for child in self.children) - )).items()) - def add_log_entry(self, account, topics, data): validate_canonical_address(account, title="Log entry address") for topic in topics: @@ -200,6 +191,15 @@ def add_log_entry(self, account, topics, data): # # Getters # + def get_accounts_for_deletion(self): + if self.error: + return tuple(dict().items()) + else: + return tuple(dict(itertools.chain( + self.accounts_to_delete.items(), + *(child.get_accounts_for_deletion() for child in self.children) + )).items()) + def get_log_entries(self): if self.error: return tuple() @@ -236,11 +236,12 @@ def get_gas_remaining(self): def __enter__(self): if self.logger is not None: self.logger.debug( - "COMPUTATION STARTING: gas: %s | from: %s | to: %s | value: %s", + "COMPUTATION STARTING: gas: %s | from: %s | to: %s | value: %s | depth %s", self.msg.gas, encode_hex(self.msg.sender), encode_hex(self.msg.to), self.msg.value, + self.msg.depth, ) return self @@ -249,11 +250,12 @@ def __exit__(self, exc_type, exc_value, traceback): if exc_value and isinstance(exc_value, VMError): if self.logger is not None: self.logger.debug( - "COMPUTATION ERROR: gas: %s | from: %s | to: %s | value: %s | error: %s", + "COMPUTATION ERROR: gas: %s | from: %s | to: %s | value: %s | depth: %s | error: %s", # noqa: E501 self.msg.gas, encode_hex(self.msg.sender), encode_hex(self.msg.to), self.msg.value, + self.msg.depth, exc_value, ) self.error = exc_value @@ -271,12 +273,13 @@ def __exit__(self, exc_type, exc_value, traceback): if self.logger is not None: self.logger.debug( ( - "COMPUTATION SUCCESS: from: %s | to: %s | value: %s | " + "COMPUTATION SUCCESS: from: %s | to: %s | value: %s | depth: %s | " "gas-used: %s | gas-remaining: %s" ), encode_hex(self.msg.sender), encode_hex(self.msg.to), self.msg.value, + self.msg.depth, self.msg.gas - self.gas_meter.gas_remaining, self.gas_meter.gas_remaining, ) diff --git a/evm/vm/forks/__init__.py b/evm/vm/forks/__init__.py index 2b360c3f09..ec2e1dd317 100644 --- a/evm/vm/forks/__init__.py +++ b/evm/vm/forks/__init__.py @@ -7,3 +7,6 @@ from .homestead import ( # noqa: F401 HomesteadVM, ) +from .spurious_dragon import ( # noqa: F401 + SpuriousDragonVM, +) diff --git a/evm/vm/forks/eip150/opcodes.py b/evm/vm/forks/eip150/opcodes.py index e8b248fbcc..c836246fdf 100644 --- a/evm/vm/forks/eip150/opcodes.py +++ b/evm/vm/forks/eip150/opcodes.py @@ -1,5 +1,7 @@ import copy +from cytoolz import merge + from evm import constants from evm import opcode_values from evm import mnemonics @@ -65,7 +67,7 @@ } -EIP150_OPCODES = { - **copy.deepcopy(HOMESTEAD_OPCODES), # noqa: E999 - **UPDATED_OPCODES, -} +EIP150_OPCODES = merge( + copy.deepcopy(HOMESTEAD_OPCODES), + UPDATED_OPCODES, +) diff --git a/evm/vm/forks/frontier/__init__.py b/evm/vm/forks/frontier/__init__.py index 27f79f4ab8..54185086a3 100644 --- a/evm/vm/forks/frontier/__init__.py +++ b/evm/vm/forks/frontier/__init__.py @@ -28,6 +28,9 @@ from evm.utils.hexadecimal import ( encode_hex, ) +from evm.utils.keccak import ( + keccak, +) from .opcodes import FRONTIER_OPCODES from .blocks import FrontierBlock @@ -84,7 +87,7 @@ def _execute_frontier_transaction(vm, transaction): vm.logger.info( ( "TRANSACTION: sender: %s | to: %s | value: %s | gas: %s | " - "gas-price: %s | s: %s | r: %s | v: %s | data: %s" + "gas-price: %s | s: %s | r: %s | v: %s | data-hash: %s" ), encode_hex(transaction.sender), encode_hex(transaction.to), @@ -94,7 +97,7 @@ def _execute_frontier_transaction(vm, transaction): transaction.s, transaction.r, transaction.v, - encode_hex(transaction.data), + encode_hex(keccak(transaction.data)), ) message = Message( @@ -188,13 +191,15 @@ def _execute_frontier_transaction(vm, transaction): ) # Suicides - for account, beneficiary in computation.get_accounts_for_deletion(): - # TODO: need to figure out how we prevent multiple suicides from - # the same account and if this is the right place to put this. - if vm.logger is not None: - vm.logger.debug('DELETING ACCOUNT: %s', encode_hex(account)) - - with vm.state_db() as state_db: + with vm.state_db() as state_db: + for account, beneficiary in computation.get_accounts_for_deletion(): + # TODO: need to figure out how we prevent multiple suicides from + # the same account and if this is the right place to put this. + if vm.logger is not None: + vm.logger.debug('DELETING ACCOUNT: %s', encode_hex(account)) + + # TODO: this balance setting is likely superflous and can be + # removed since `delete_account` does this. state_db.set_balance(account, 0) state_db.delete_account(account) @@ -250,8 +255,8 @@ def _apply_frontier_computation(vm, message): with computation: # Early exit on pre-compiles - if computation.msg.code_address in PRECOMPILES: - return PRECOMPILES[computation.msg.code_address](computation) + if computation.msg.code_address in vm.precompiles: + return vm.precompiles[computation.msg.code_address](computation) for opcode in computation.code: opcode_fn = computation.vm.get_opcode_fn(opcode) @@ -291,9 +296,10 @@ def _apply_frontier_create_message(vm, message): else: if vm.logger: vm.logger.debug( - "SETTING CODE: %s -> %s", + "SETTING CODE: %s -> length: %s | hash: %s", encode_hex(message.storage_address), - contract_code, + len(contract_code), + encode_hex(keccak(contract_code)) ) with vm.state_db() as state_db: state_db.set_code(message.storage_address, contract_code) @@ -306,6 +312,7 @@ def _apply_frontier_create_message(vm, message): opcodes=FRONTIER_OPCODES, # classes _block_class=FrontierBlock, + _precompiles=PRECOMPILES, # helpers create_header_from_parent=staticmethod(create_frontier_header_from_parent), configure_header=configure_frontier_header, diff --git a/evm/vm/forks/frontier/opcodes.py b/evm/vm/forks/frontier/opcodes.py index b09e69d7db..90b06d02f0 100644 --- a/evm/vm/forks/frontier/opcodes.py +++ b/evm/vm/forks/frontier/opcodes.py @@ -76,7 +76,7 @@ gas_cost=constants.GAS_MID, ), opcode_values.EXP: as_opcode( - logic_fn=arithmetic.exp, + logic_fn=arithmetic.exp(gas_per_byte=constants.GAS_EXPBYTE), mnemonic=mnemonics.EXP, gas_cost=constants.GAS_EXP, ), diff --git a/evm/vm/forks/homestead/__init__.py b/evm/vm/forks/homestead/__init__.py index 6604b5c8f2..9012fbdfcd 100644 --- a/evm/vm/forks/homestead/__init__.py +++ b/evm/vm/forks/homestead/__init__.py @@ -6,6 +6,9 @@ from evm.utils.hexadecimal import ( encode_hex, ) +from evm.utils.keccak import ( + keccak, +) from ..frontier import FrontierVM @@ -24,7 +27,7 @@ def _apply_homestead_create_message(vm, message): computation = vm.apply_message(message) if computation.error: - vm.commit(snapshot) + vm.revert(snapshot) return computation else: contract_code = computation.output @@ -44,9 +47,10 @@ def _apply_homestead_create_message(vm, message): else: if vm.logger: vm.logger.debug( - "SETTING CODE: %s -> %s", + "SETTING CODE: %s -> length: %s | hash: %s", encode_hex(message.storage_address), - contract_code, + len(contract_code), + encode_hex(keccak(contract_code)) ) with vm.state_db() as state_db: diff --git a/evm/vm/forks/homestead/opcodes.py b/evm/vm/forks/homestead/opcodes.py index b1290f58a6..fda3676f2d 100644 --- a/evm/vm/forks/homestead/opcodes.py +++ b/evm/vm/forks/homestead/opcodes.py @@ -1,5 +1,7 @@ import copy +from cytoolz import merge + from evm import constants from evm import opcode_values from evm import mnemonics @@ -20,13 +22,7 @@ } -UPDATED_OPCODES = { - # TODO: suicide? -} - - -HOMESTEAD_OPCODES = { - **copy.deepcopy(FRONTIER_OPCODES), # noqa: E999 - **UPDATED_OPCODES, - **NEW_OPCODES -} +HOMESTEAD_OPCODES = merge( + copy.deepcopy(FRONTIER_OPCODES), + NEW_OPCODES +) diff --git a/evm/vm/forks/spurious_dragon/__init__.py b/evm/vm/forks/spurious_dragon/__init__.py new file mode 100644 index 0000000000..f571ce526c --- /dev/null +++ b/evm/vm/forks/spurious_dragon/__init__.py @@ -0,0 +1,101 @@ +from evm import constants +from evm.exceptions import ( + OutOfGas, +) +from evm.utils.hexadecimal import ( + encode_hex, +) +from evm.utils.keccak import ( + keccak, +) + +from ..frontier import _execute_frontier_transaction +from ..homestead import HomesteadVM + +from .blocks import SpuriousDragonBlock +from .opcodes import SPURIOUS_DRAGON_OPCODES +from .utils import collect_touched_accounts + + +def _execute_spurious_dragon_transaction(vm, transaction): + computation = _execute_frontier_transaction(vm, transaction) + + # + # EIP161 state clearing + # + touched_accounts = collect_touched_accounts(computation) + + with vm.state_db() as state_db: + for account in touched_accounts: + if state_db.account_exists(account) and state_db.account_is_empty(account): + vm.logger.debug( + "CLEARING EMPTY ACCOUNT: %s", + encode_hex(account), + ) + state_db.delete_account(account) + + return computation + + +def _apply_spurious_dragon_create_message(vm, message): + snapshot = vm.snapshot() + + # EIP161 nonce incrementation + with vm.state_db() as state_db: + state_db.increment_nonce(message.storage_address) + + computation = vm.apply_message(message) + + if computation.error: + vm.revert(snapshot) + return computation + else: + contract_code = computation.output + + if contract_code and len(contract_code) >= constants.EIP170_CODE_SIZE_LIMIT: + computation.error = OutOfGas( + "Contract code size exceeds EIP170 limit of {0}. Got code of " + "size: {1}".format( + constants.EIP170_CODE_SIZE_LIMIT, + len(contract_code), + ) + ) + vm.revert(snapshot) + elif contract_code: + contract_code_gas_cost = len(contract_code) * constants.GAS_CODEDEPOSIT + try: + computation.gas_meter.consume_gas( + contract_code_gas_cost, + reason="Write contract code for CREATE", + ) + except OutOfGas as err: + # Different from Frontier: reverts state on gas failure while + # writing contract code. + computation.error = err + vm.revert(snapshot) + else: + if vm.logger: + vm.logger.debug( + "SETTING CODE: %s -> length: %s | hash: %s", + encode_hex(message.storage_address), + len(contract_code), + encode_hex(keccak(contract_code)) + ) + + with vm.state_db() as state_db: + state_db.set_code(message.storage_address, contract_code) + vm.commit(snapshot) + else: + vm.commit(snapshot) + return computation + + +SpuriousDragonVM = HomesteadVM.configure( + name='SpuriousDragonVM', + # rlp classes + _block_class=SpuriousDragonBlock, + # opcodes + opcodes=SPURIOUS_DRAGON_OPCODES, + apply_create_message=_apply_spurious_dragon_create_message, + execute_transaction=_execute_spurious_dragon_transaction, +) diff --git a/evm/vm/forks/spurious_dragon/blocks.py b/evm/vm/forks/spurious_dragon/blocks.py new file mode 100644 index 0000000000..e8ae306f20 --- /dev/null +++ b/evm/vm/forks/spurious_dragon/blocks.py @@ -0,0 +1,21 @@ +from rlp.sedes import ( + CountableList, +) +from evm.rlp.headers import ( + BlockHeader, +) +from evm.vm.forks.homestead.blocks import ( + HomesteadBlock, +) +from .transactions import ( + SpuriousDragonTransaction, +) + + +class SpuriousDragonBlock(HomesteadBlock): + transaction_class = SpuriousDragonTransaction + fields = [ + ('header', BlockHeader), + ('transactions', CountableList(SpuriousDragonTransaction)), + ('uncles', CountableList(BlockHeader)) + ] diff --git a/evm/vm/forks/spurious_dragon/opcodes.py b/evm/vm/forks/spurious_dragon/opcodes.py new file mode 100644 index 0000000000..fe76ebaf09 --- /dev/null +++ b/evm/vm/forks/spurious_dragon/opcodes.py @@ -0,0 +1,42 @@ +import copy + +from cytoolz import merge + +from evm import constants +from evm import opcode_values +from evm import mnemonics + +from evm.opcode import as_opcode + +from evm.logic import ( + arithmetic, + system, + call, +) + +from evm.vm.forks.eip150.opcodes import EIP150_OPCODES + + +UPDATED_OPCODES = { + opcode_values.EXP: as_opcode( + logic_fn=arithmetic.exp(gas_per_byte=constants.GAS_EXPBYTE_EIP160), + mnemonic=mnemonics.EXP, + gas_cost=constants.GAS_EXP_EIP160, + ), + opcode_values.SUICIDE: as_opcode( + logic_fn=system.suicide_eip161, + mnemonic=mnemonics.SUICIDE, + gas_cost=constants.GAS_SUICIDE_EIP150, + ), + opcode_values.CALL: call.CallEIP161.configure( + name='opcode:CALL', + mnemonic=mnemonics.CALL, + gas_cost=constants.GAS_CALL_EIP150, + )(), +} + + +SPURIOUS_DRAGON_OPCODES = merge( + copy.deepcopy(EIP150_OPCODES), + UPDATED_OPCODES, +) diff --git a/evm/vm/forks/spurious_dragon/transactions.py b/evm/vm/forks/spurious_dragon/transactions.py new file mode 100644 index 0000000000..40d7f805cd --- /dev/null +++ b/evm/vm/forks/spurious_dragon/transactions.py @@ -0,0 +1,65 @@ +import rlp + +from evm.vm.forks.homestead.transactions import ( + HomesteadTransaction, + HomesteadUnsignedTransaction, +) + +from evm.utils.numeric import ( + int_to_big_endian, +) +from evm.utils.transactions import ( + create_transaction_signature, + create_eip155_transaction_signature, + extract_chain_id, + is_eip_155_signed_transaction, + validate_eip155_transaction_signature, + validate_transaction_signature, +) + + +class SpuriousDragonTransaction(HomesteadTransaction): + def get_message_for_signing(self): + if is_eip_155_signed_transaction(self): + chain_id = extract_chain_id(self.v) + txn_parts = rlp.decode(rlp.encode(self)) + txn_parts_for_signing = txn_parts[:-3] + [int_to_big_endian(chain_id), b'', b''] + return rlp.encode(txn_parts_for_signing) + else: + return rlp.encode(SpuriousDragonUnsignedTransaction( + nonce=self.nonce, + gas_price=self.gas_price, + gas=self.gas, + to=self.to, + value=self.value, + data=self.data, + )) + + def check_signature_validity(self): + if is_eip_155_signed_transaction(self): + validate_eip155_transaction_signature(self) + else: + validate_transaction_signature(self) + + @classmethod + def create_unsigned_transaction(cls, nonce, gas_price, gas, to, value, data): + return SpuriousDragonUnsignedTransaction(nonce, gas_price, gas, to, value, data) + + +class SpuriousDragonUnsignedTransaction(HomesteadUnsignedTransaction): + def as_signed_transaction(self, private_key, chain_id=None): + if chain_id is None: + v, r, s = create_transaction_signature(self, private_key) + else: + v, r, s = create_eip155_transaction_signature(self, private_key) + return SpuriousDragonTransaction( + nonce=self.nonce, + gas_price=self.gas_price, + gas=self.gas, + to=self.to, + value=self.value, + data=self.data, + v=v, + r=r, + s=s, + ) diff --git a/evm/vm/forks/spurious_dragon/utils.py b/evm/vm/forks/spurious_dragon/utils.py new file mode 100644 index 0000000000..fa4bd517ab --- /dev/null +++ b/evm/vm/forks/spurious_dragon/utils.py @@ -0,0 +1,41 @@ +from eth_utils import to_set + +from evm import constants +from evm.utils.address import ( + force_bytes_to_address, +) + + +THREE = force_bytes_to_address(b'\x03') + + +@to_set +def collect_touched_accounts(computation): + """ + Collect all of the accounts that *may* need to be deleted based on EIP161: + + https://github.com/ethereum/EIPs/blob/master/EIPS/eip-161.md + """ + if computation.is_origin_computation and computation.msg.gas_price == 0: + yield computation.vm.block.header.coinbase + + for beneficiary in sorted(set(computation.accounts_to_delete.values())): + if computation.error and computation.is_origin_computation: + # Special case to account for geth+parity bug + if beneficiary == THREE: + yield beneficiary + continue + else: + yield beneficiary + + if computation.msg.to != constants.CREATE_CONTRACT_ADDRESS: + if computation.error and computation.is_origin_computation: + # Special case to account for geth+parity bug + if computation.msg.to == THREE: + yield computation.msg.to + else: + yield computation.msg.to + + if not computation.is_origin_computation or not computation.error: + for child in computation.children: + yield from collect_touched_accounts(child) diff --git a/tests/core/tester/test_generate_vm_configuration.py b/tests/core/tester/test_generate_vm_configuration.py index 9e6ef8b711..1dabeee349 100644 --- a/tests/core/tester/test_generate_vm_configuration.py +++ b/tests/core/tester/test_generate_vm_configuration.py @@ -11,6 +11,7 @@ class Forks(enum.Enum): Frontier = 0 Homestead = 1 EIP150 = 2 + SpuriousDragon = 3 @pytest.mark.parametrize( @@ -18,7 +19,32 @@ class Forks(enum.Enum): ( ( dict(), - ((0, Forks.EIP150),), + ((0, Forks.SpuriousDragon),), + ), + ( + dict(spurious_dragon_block=1), + ((0, Forks.EIP150), (1, Forks.SpuriousDragon)), + ), + ( + dict(eip150_start_block=1, spurious_dragon_block=2), + ((0, Forks.Homestead), (1, Forks.EIP150), (2, Forks.SpuriousDragon)), + ), + ( + dict(homestead_start_block=1, eip150_start_block=2, spurious_dragon_block=3), + ( + (0, Forks.Frontier), + (1, Forks.Homestead), + (2, Forks.EIP150), + (3, Forks.SpuriousDragon), + ), + ), + ( + dict(homestead_start_block=1, spurious_dragon_block=3), + ( + (0, Forks.Frontier), + (1, Forks.Homestead), + (3, Forks.SpuriousDragon), + ), ), ( dict(eip150_start_block=1), @@ -67,5 +93,7 @@ def test_generate_vm_configuration(kwargs, expected): assert left_vm.dao_fork_block_number == dao_start_block elif right_vm == Forks.EIP150: assert 'EIP150' in left_vm.__name__ + elif right_vm == Forks.SpuriousDragon: + assert 'SpuriousDragon' in left_vm.__name__ else: assert False, "Invariant" diff --git a/tests/core/transaction-utils/test_transaction_signature_validation.py b/tests/core/transaction-utils/test_transaction_signature_validation.py index d8605e28e4..eda2dd13d3 100644 --- a/tests/core/transaction-utils/test_transaction_signature_validation.py +++ b/tests/core/transaction-utils/test_transaction_signature_validation.py @@ -22,6 +22,16 @@ ) +from evm.vm.forks.spurious_dragon.transactions import ( + SpuriousDragonTransaction, +) + +from evm.utils.transactions import ( + is_eip_155_signed_transaction, + validate_eip155_transaction_signature, +) + + @pytest.mark.parametrize( "txn_class", (FrontierTransaction, HomesteadTransaction), @@ -34,6 +44,15 @@ def test_pre_EIP155_transaction_signature_validation(txn_class, txn_fixture): transaction.check_signature_validity() +def test_EIP155_transaction_signature_validation(txn_fixture): + transaction = rlp.decode(decode_hex(txn_fixture['signed']), sedes=SpuriousDragonTransaction) + if is_eip_155_signed_transaction(transaction): + validate_eip155_transaction_signature(transaction) + else: + validate_transaction_signature(transaction) + transaction.check_signature_validity() + + @pytest.mark.parametrize( "txn_class", (FrontierTransaction, HomesteadTransaction), @@ -45,3 +64,11 @@ def test_pre_EIP155_transaction_sender_extraction(txn_class, txn_fixture): transaction = rlp.decode(decode_hex(txn_fixture['signed']), sedes=txn_class) sender = extract_transaction_sender(transaction) assert is_same_address(sender, key.public_key.to_canonical_address()) + + +def test_EIP155_transaction_sender_extraction(txn_fixture): + key = keys.PrivateKey(decode_hex(txn_fixture['key'])) + transaction = rlp.decode(decode_hex(txn_fixture['signed']), sedes=SpuriousDragonTransaction) + sender = extract_transaction_sender(transaction) + assert is_same_address(sender, transaction.sender) + assert is_same_address(sender, key.public_key.to_canonical_address()) diff --git a/tests/json-fixtures/test_blockchain.py b/tests/json-fixtures/test_blockchain.py index 388a3831a5..894fa23a06 100644 --- a/tests/json-fixtures/test_blockchain.py +++ b/tests/json-fixtures/test_blockchain.py @@ -22,6 +22,7 @@ EIP150VM, FrontierVM, HomesteadVM as BaseHomesteadVM, + SpuriousDragonVM, ) from evm.rlp.headers import ( BlockHeader, @@ -120,7 +121,9 @@ def chain_vm_configuration(fixture_data, fixture): (0, EIP150VM), ) elif 'EIP158' in fixture_key or fixture_key.endswith('EIP158'): - pytest.skip('Spurious Dragon VM rules not yet supported') + return ( + (0, SpuriousDragonVM), + ) elif fixture_key.endswith('Metropolis'): pytest.skip('Metropolis VM rules not yet supported') elif fixture_key.endswith('Byzantium'): diff --git a/tests/json-fixtures/test_state.py b/tests/json-fixtures/test_state.py index 0856703048..6abb3dc968 100644 --- a/tests/json-fixtures/test_state.py +++ b/tests/json-fixtures/test_state.py @@ -21,17 +21,18 @@ EIP150VM, FrontierVM, HomesteadVM, + SpuriousDragonVM, ) from evm.rlp.headers import ( BlockHeader, ) from evm.utils.fixture_tests import ( - load_fixture, - normalize_statetest_fixture, - setup_state_db, filter_fixtures, generate_fixture_tests, hash_log_entries, + load_fixture, + normalize_statetest_fixture, + setup_state_db, ) @@ -111,6 +112,10 @@ def get_block_hash_for_testing(self, block_number): name='EIP150VMForTesting', get_ancestor_hash=get_block_hash_for_testing, ) +SpuriousDragonVMForTesting = SpuriousDragonVM.configure( + name='SpuriousDragonVMForTesting', + get_ancestor_hash=get_block_hash_for_testing, +) @pytest.fixture @@ -123,7 +128,7 @@ def fixture_vm_class(fixture_data): elif fork_name == 'EIP150': return EIP150VMForTesting elif fork_name == 'EIP158': - pytest.skip("EIP158 VM has not been implemented") + return SpuriousDragonVMForTesting elif fork_name == 'Byzantium': pytest.skip("Byzantium VM has not been implemented") elif fork_name == 'Constantinople': diff --git a/tests/json-fixtures/test_transactions.py b/tests/json-fixtures/test_transactions.py index f52e2ca5ea..d722ba6c8a 100644 --- a/tests/json-fixtures/test_transactions.py +++ b/tests/json-fixtures/test_transactions.py @@ -60,7 +60,7 @@ def fixture(fixture_data): def test_transaction_fixtures(fixture): - header = BlockHeader(1, fixture.get('blocknumber', 0), 100) + header = BlockHeader(1, fixture['blocknumber'], 100) chain = MainnetChain(BaseChainDB(get_db_backend()), header=header) vm = chain.get_vm() TransactionClass = vm.get_transaction_class() diff --git a/tests/json-fixtures/test_virtual_machine.py b/tests/json-fixtures/test_virtual_machine.py index 041634c201..42871f6de9 100644 --- a/tests/json-fixtures/test_virtual_machine.py +++ b/tests/json-fixtures/test_virtual_machine.py @@ -115,7 +115,7 @@ def get_block_hash_for_testing(self, block_number): ) -@pytest.fixture(params=['Frontier', 'Homestead', 'EIP150']) +@pytest.fixture(params=['Frontier', 'Homestead', 'EIP150', 'SpuriousDragon']) def vm_class(request): if request.param == 'Frontier': pytest.skip('Only the Homestead VM rules are currently supported') @@ -123,6 +123,8 @@ def vm_class(request): return HomesteadVMForTesting elif request.param == 'EIP150': pytest.skip('Only the Homestead VM rules are currently supported') + elif request.param == 'SpuriousDragon': + pytest.skip('Only the Homestead VM rules are currently supported') else: assert False, "Unsupported VM: {0}".format(request.param) From acd45a07e810741315a2eebbd51f15eb5fa58192 Mon Sep 17 00:00:00 2001 From: Piper Merriam Date: Thu, 19 Oct 2017 09:01:06 -0600 Subject: [PATCH 2/4] PR feedback --- evm/vm/computation.py | 2 +- evm/vm/forks/frontier/blocks.py | 5 ++--- evm/vm/forks/homestead/blocks.py | 2 +- evm/vm/forks/spurious_dragon/blocks.py | 2 +- evm/vm/forks/spurious_dragon/utils.py | 4 ++++ 5 files changed, 9 insertions(+), 6 deletions(-) diff --git a/evm/vm/computation.py b/evm/vm/computation.py index eb275823bb..d4dba82df7 100644 --- a/evm/vm/computation.py +++ b/evm/vm/computation.py @@ -193,7 +193,7 @@ def add_log_entry(self, account, topics, data): # def get_accounts_for_deletion(self): if self.error: - return tuple(dict().items()) + return tuple() else: return tuple(dict(itertools.chain( self.accounts_to_delete.items(), diff --git a/evm/vm/forks/frontier/blocks.py b/evm/vm/forks/frontier/blocks.py index df10508a4d..c032e07b94 100644 --- a/evm/vm/forks/frontier/blocks.py +++ b/evm/vm/forks/frontier/blocks.py @@ -44,9 +44,10 @@ class FrontierBlock(BaseBlock): + transaction_class = FrontierTransaction fields = [ ('header', BlockHeader), - ('transactions', CountableList(FrontierTransaction)), + ('transactions', CountableList(transaction_class)), ('uncles', CountableList(BlockHeader)) ] @@ -185,8 +186,6 @@ def get_parent_header(self): # # Transaction class for this block class # - transaction_class = FrontierTransaction - @classmethod def get_transaction_class(cls): return cls.transaction_class diff --git a/evm/vm/forks/homestead/blocks.py b/evm/vm/forks/homestead/blocks.py index baa87e84a7..50c96c3731 100644 --- a/evm/vm/forks/homestead/blocks.py +++ b/evm/vm/forks/homestead/blocks.py @@ -16,6 +16,6 @@ class HomesteadBlock(FrontierBlock): transaction_class = HomesteadTransaction fields = [ ('header', BlockHeader), - ('transactions', CountableList(HomesteadTransaction)), + ('transactions', CountableList(transaction_class)), ('uncles', CountableList(BlockHeader)) ] diff --git a/evm/vm/forks/spurious_dragon/blocks.py b/evm/vm/forks/spurious_dragon/blocks.py index e8ae306f20..d721c5c0d4 100644 --- a/evm/vm/forks/spurious_dragon/blocks.py +++ b/evm/vm/forks/spurious_dragon/blocks.py @@ -16,6 +16,6 @@ class SpuriousDragonBlock(HomesteadBlock): transaction_class = SpuriousDragonTransaction fields = [ ('header', BlockHeader), - ('transactions', CountableList(SpuriousDragonTransaction)), + ('transactions', CountableList(transaction_class)), ('uncles', CountableList(BlockHeader)) ] diff --git a/evm/vm/forks/spurious_dragon/utils.py b/evm/vm/forks/spurious_dragon/utils.py index fa4bd517ab..95773d3bb3 100644 --- a/evm/vm/forks/spurious_dragon/utils.py +++ b/evm/vm/forks/spurious_dragon/utils.py @@ -15,6 +15,8 @@ def collect_touched_accounts(computation): Collect all of the accounts that *may* need to be deleted based on EIP161: https://github.com/ethereum/EIPs/blob/master/EIPS/eip-161.md + + also see: https://github.com/ethereum/EIPs/issues/716 """ if computation.is_origin_computation and computation.msg.gas_price == 0: yield computation.vm.block.header.coinbase @@ -22,6 +24,7 @@ def collect_touched_accounts(computation): for beneficiary in sorted(set(computation.accounts_to_delete.values())): if computation.error and computation.is_origin_computation: # Special case to account for geth+parity bug + # https://github.com/ethereum/EIPs/issues/716 if beneficiary == THREE: yield beneficiary continue @@ -31,6 +34,7 @@ def collect_touched_accounts(computation): if computation.msg.to != constants.CREATE_CONTRACT_ADDRESS: if computation.error and computation.is_origin_computation: # Special case to account for geth+parity bug + # https://github.com/ethereum/EIPs/issues/716 if computation.msg.to == THREE: yield computation.msg.to else: From 9d124f80db445193dea9bcb82a1ca967e03ac14e Mon Sep 17 00:00:00 2001 From: Piper Merriam Date: Thu, 19 Oct 2017 08:59:55 -0600 Subject: [PATCH 3/4] revamp eip155 transactions signing --- evm/utils/transactions.py | 80 +++++++---------- evm/vm/forks/spurious_dragon/transactions.py | 18 ++-- .../test_transaction_signature_validation.py | 88 ++++++++++++++----- 3 files changed, 101 insertions(+), 85 deletions(-) diff --git a/evm/utils/transactions.py b/evm/utils/transactions.py index 1be6b25e34..1835bbdfb6 100644 --- a/evm/utils/transactions.py +++ b/evm/utils/transactions.py @@ -9,57 +9,18 @@ from evm.exceptions import ( ValidationError, ) -from evm.validation import ( - validate_gt, -) - from evm.utils.numeric import ( is_even, int_to_big_endian, ) -def create_transaction_signature(unsigned_txn, private_key): - signature = private_key.sign_msg(rlp.encode(unsigned_txn)) - canonical_v, r, s = signature.vrs - v = canonical_v + 27 - return v, r, s - - -def create_eip155_transaction_signature(unsigned_txn, chain_id, private_key): - transaction_parts = rlp.decode(rlp.encode(unsigned_txn)) - transaction_parts_for_signature = ( - transaction_parts[:-3] + [int_to_big_endian(chain_id), b'', b''] - ) - message = rlp.encode(transaction_parts_for_signature) - - signature = private_key.sign_msg(message) - canonical_v, r, s = signature.vrs - v = canonical_v + 27 - if v == 27: - eip155_v = chain_id * 2 + 36 - else: - eip155_v = chain_id * 2 + 35 - return eip155_v, r, s - - -def validate_transaction_signature(transaction): - v, r, s = transaction.v, transaction.r, transaction.s - canonical_v = v - 27 - vrs = (canonical_v, r, s) - signature = keys.Signature(vrs=vrs) - message = transaction.get_message_for_signing() - try: - public_key = signature.recover_public_key_from_msg(message) - except BadSignature as e: - raise ValidationError("Bad Signature: {0}".format(str(e))) - - if not signature.verify_msg(message, public_key): - raise ValidationError("Invalid Signature") +EIP155_CHAIN_ID_OFFSET = 35 +V_OFFSET = 27 def is_eip_155_signed_transaction(transaction): - if transaction.v >= 35: + if transaction.v >= EIP155_CHAIN_ID_OFFSET: return True else: return False @@ -67,9 +28,9 @@ def is_eip_155_signed_transaction(transaction): def extract_chain_id(v): if is_even(v): - return (v - 36) // 2 + return (v - EIP155_CHAIN_ID_OFFSET) // 2 else: - return (v - 35) // 2 + return (v - EIP155_CHAIN_ID_OFFSET) // 2 def extract_signature_v(v): @@ -79,16 +40,39 @@ def extract_signature_v(v): return 27 -def validate_eip155_transaction_signature(transaction): - validate_gt(transaction.v, 34, title="Transaction.v") +def create_transaction_signature(unsigned_txn, private_key, chain_id=None): + transaction_parts = rlp.decode(rlp.encode(unsigned_txn)) + + if chain_id: + transaction_parts_for_signature = ( + transaction_parts + [int_to_big_endian(chain_id), b'', b''] + ) + else: + transaction_parts_for_signature = transaction_parts + + message = rlp.encode(transaction_parts_for_signature) + signature = private_key.sign_msg(message) + + canonical_v, r, s = signature.vrs + + if chain_id: + v = canonical_v + chain_id * 2 + EIP155_CHAIN_ID_OFFSET + else: + v = canonical_v + V_OFFSET + + return v, r, s - v = extract_signature_v(transaction.v) + +def validate_transaction_signature(transaction): + if is_eip_155_signed_transaction(transaction): + v = extract_signature_v(transaction.v) + else: + v = transaction.v canonical_v = v - 27 vrs = (canonical_v, transaction.r, transaction.s) signature = keys.Signature(vrs=vrs) message = transaction.get_message_for_signing() - try: public_key = signature.recover_public_key_from_msg(message) except BadSignature as e: diff --git a/evm/vm/forks/spurious_dragon/transactions.py b/evm/vm/forks/spurious_dragon/transactions.py index 40d7f805cd..c79638e4b6 100644 --- a/evm/vm/forks/spurious_dragon/transactions.py +++ b/evm/vm/forks/spurious_dragon/transactions.py @@ -10,11 +10,8 @@ ) from evm.utils.transactions import ( create_transaction_signature, - create_eip155_transaction_signature, extract_chain_id, is_eip_155_signed_transaction, - validate_eip155_transaction_signature, - validate_transaction_signature, ) @@ -35,23 +32,18 @@ def get_message_for_signing(self): data=self.data, )) - def check_signature_validity(self): - if is_eip_155_signed_transaction(self): - validate_eip155_transaction_signature(self) - else: - validate_transaction_signature(self) - @classmethod def create_unsigned_transaction(cls, nonce, gas_price, gas, to, value, data): return SpuriousDragonUnsignedTransaction(nonce, gas_price, gas, to, value, data) + @property + def chain_id(self): + return extract_chain_id(self) + class SpuriousDragonUnsignedTransaction(HomesteadUnsignedTransaction): def as_signed_transaction(self, private_key, chain_id=None): - if chain_id is None: - v, r, s = create_transaction_signature(self, private_key) - else: - v, r, s = create_eip155_transaction_signature(self, private_key) + v, r, s = create_transaction_signature(self, private_key, chain_id=chain_id) return SpuriousDragonTransaction( nonce=self.nonce, gas_price=self.gas_price, diff --git a/tests/core/transaction-utils/test_transaction_signature_validation.py b/tests/core/transaction-utils/test_transaction_signature_validation.py index eda2dd13d3..fad40c2b22 100644 --- a/tests/core/transaction-utils/test_transaction_signature_validation.py +++ b/tests/core/transaction-utils/test_transaction_signature_validation.py @@ -5,6 +5,7 @@ from eth_utils import ( decode_hex, is_same_address, + to_canonical_address, ) from eth_keys import keys @@ -15,6 +16,9 @@ from evm.vm.forks.homestead.transactions import ( HomesteadTransaction, ) +from evm.vm.forks.spurious_dragon.transactions import ( + SpuriousDragonTransaction, +) from evm.utils.transactions import ( extract_transaction_sender, @@ -22,47 +26,40 @@ ) -from evm.vm.forks.spurious_dragon.transactions import ( - SpuriousDragonTransaction, -) - -from evm.utils.transactions import ( - is_eip_155_signed_transaction, - validate_eip155_transaction_signature, -) +@pytest.fixture(params=['Frontier', 'Homestead', 'SpuriousDragon']) +def transaction_class(request): + if request.param == 'Frontier': + return FrontierTransaction + elif request.param == 'Homestead': + return HomesteadTransaction + elif request.param == 'SpuriousDragon': + return SpuriousDragonTransaction + else: + raise AssertionError("Unknown param: {0}".format(request.param)) -@pytest.mark.parametrize( - "txn_class", - (FrontierTransaction, HomesteadTransaction), -) -def test_pre_EIP155_transaction_signature_validation(txn_class, txn_fixture): +def test_pre_EIP155_transaction_signature_validation(transaction_class, txn_fixture): if txn_fixture['chainId'] is not None: pytest.skip("Only testng non-EIP155 transactions") - transaction = rlp.decode(decode_hex(txn_fixture['signed']), sedes=txn_class) + transaction = rlp.decode(decode_hex(txn_fixture['signed']), sedes=transaction_class) validate_transaction_signature(transaction) transaction.check_signature_validity() def test_EIP155_transaction_signature_validation(txn_fixture): transaction = rlp.decode(decode_hex(txn_fixture['signed']), sedes=SpuriousDragonTransaction) - if is_eip_155_signed_transaction(transaction): - validate_eip155_transaction_signature(transaction) - else: - validate_transaction_signature(transaction) + validate_transaction_signature(transaction) transaction.check_signature_validity() -@pytest.mark.parametrize( - "txn_class", - (FrontierTransaction, HomesteadTransaction), -) -def test_pre_EIP155_transaction_sender_extraction(txn_class, txn_fixture): +def test_pre_EIP155_transaction_sender_extraction(transaction_class, txn_fixture): if txn_fixture['chainId'] is not None: pytest.skip("Only testng non-EIP155 transactions") key = keys.PrivateKey(decode_hex(txn_fixture['key'])) - transaction = rlp.decode(decode_hex(txn_fixture['signed']), sedes=txn_class) + transaction = rlp.decode(decode_hex(txn_fixture['signed']), sedes=transaction_class) sender = extract_transaction_sender(transaction) + + assert is_same_address(sender, transaction.sender) assert is_same_address(sender, key.public_key.to_canonical_address()) @@ -72,3 +69,46 @@ def test_EIP155_transaction_sender_extraction(txn_fixture): sender = extract_transaction_sender(transaction) assert is_same_address(sender, transaction.sender) assert is_same_address(sender, key.public_key.to_canonical_address()) + + +def test_unsigned_to_signed_transaction(txn_fixture, transaction_class): + key = keys.PrivateKey(decode_hex(txn_fixture['key'])) + unsigned_txn = transaction_class.create_unsigned_transaction( + nonce=txn_fixture['nonce'], + gas_price=txn_fixture['gasPrice'], + gas=txn_fixture['gas'], + to=( + to_canonical_address(txn_fixture['to']) + if txn_fixture['to'] + else b'' + ), + value=txn_fixture['value'], + data=decode_hex(txn_fixture['data']), + ) + signed_txn = unsigned_txn.as_signed_transaction(key) + + assert is_same_address(signed_txn.sender, key.public_key.to_canonical_address()) + + +def test_unsigned_to_eip155_signed_transaction(txn_fixture, transaction_class): + if txn_fixture['chainId'] is None: + pytest.skip('No chain id for EIP155 signing') + elif not hasattr(transaction_class, 'chain_id'): + pytest.skip('Transaction class is not chain aware') + + key = keys.PrivateKey(decode_hex(txn_fixture['key'])) + unsigned_txn = transaction_class.create_unsigned_transaction( + nonce=txn_fixture['nonce'], + gas_price=txn_fixture['gasPrice'], + gas=txn_fixture['gas'], + to=( + to_canonical_address(txn_fixture['to']) + if txn_fixture['to'] + else b'' + ), + value=txn_fixture['value'], + data=decode_hex(txn_fixture['data']), + ) + signed_txn = unsigned_txn.as_signed_transaction(key, chain_id=txn_fixture['chainId']) + + assert is_same_address(signed_txn.sender, key.public_key.to_canonical_address()) From 948e6b6d5869a87433e0c40b55c1e2ed2723d4e7 Mon Sep 17 00:00:00 2001 From: Piper Merriam Date: Thu, 19 Oct 2017 09:51:26 -0600 Subject: [PATCH 4/4] fix for chain id extraction from EIP155 signed transactions --- evm/utils/transactions.py | 6 +++--- evm/vm/forks/spurious_dragon/transactions.py | 5 ++++- .../test_transaction_signature_validation.py | 12 +++--------- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/evm/utils/transactions.py b/evm/utils/transactions.py index 1835bbdfb6..5581f3e35a 100644 --- a/evm/utils/transactions.py +++ b/evm/utils/transactions.py @@ -28,16 +28,16 @@ def is_eip_155_signed_transaction(transaction): def extract_chain_id(v): if is_even(v): - return (v - EIP155_CHAIN_ID_OFFSET) // 2 + return (v - EIP155_CHAIN_ID_OFFSET - 1) // 2 else: return (v - EIP155_CHAIN_ID_OFFSET) // 2 def extract_signature_v(v): if is_even(v): - return 28 + return V_OFFSET + 1 else: - return 27 + return V_OFFSET def create_transaction_signature(unsigned_txn, private_key, chain_id=None): diff --git a/evm/vm/forks/spurious_dragon/transactions.py b/evm/vm/forks/spurious_dragon/transactions.py index c79638e4b6..926c8480a2 100644 --- a/evm/vm/forks/spurious_dragon/transactions.py +++ b/evm/vm/forks/spurious_dragon/transactions.py @@ -38,7 +38,10 @@ def create_unsigned_transaction(cls, nonce, gas_price, gas, to, value, data): @property def chain_id(self): - return extract_chain_id(self) + if is_eip_155_signed_transaction(self): + return extract_chain_id(self.v) + else: + return None class SpuriousDragonUnsignedTransaction(HomesteadUnsignedTransaction): diff --git a/tests/core/transaction-utils/test_transaction_signature_validation.py b/tests/core/transaction-utils/test_transaction_signature_validation.py index fad40c2b22..f86eedd592 100644 --- a/tests/core/transaction-utils/test_transaction_signature_validation.py +++ b/tests/core/transaction-utils/test_transaction_signature_validation.py @@ -26,16 +26,9 @@ ) -@pytest.fixture(params=['Frontier', 'Homestead', 'SpuriousDragon']) +@pytest.fixture(params=[FrontierTransaction, HomesteadTransaction, SpuriousDragonTransaction]) def transaction_class(request): - if request.param == 'Frontier': - return FrontierTransaction - elif request.param == 'Homestead': - return HomesteadTransaction - elif request.param == 'SpuriousDragon': - return SpuriousDragonTransaction - else: - raise AssertionError("Unknown param: {0}".format(request.param)) + return request.param def test_pre_EIP155_transaction_signature_validation(transaction_class, txn_fixture): @@ -112,3 +105,4 @@ def test_unsigned_to_eip155_signed_transaction(txn_fixture, transaction_class): signed_txn = unsigned_txn.as_signed_transaction(key, chain_id=txn_fixture['chainId']) assert is_same_address(signed_txn.sender, key.public_key.to_canonical_address()) + assert signed_txn.chain_id == txn_fixture['chainId']