ethereum / py-evm

A Python implementation of the Ethereum Virtual Machine
https://py-evm.readthedocs.io/en/latest/
MIT License
2.26k stars 650 forks source link

estimate_gas should work with unsigned transactions #453

Open dylanjw opened 6 years ago

dylanjw commented 6 years ago

What is wrong?

Gas estimation should not require a signed transaction. An example use case is getting a gas price estimate for transactions before local signing, where the private key is not available to the node.

How can it be fixed

The instance of SpoofTransaction can be extended to provide the rest of the missing attributes from an unsigned transaction object. The following stack trace + exceptions show each dependent attribute that would need to be added to SpoofTransaction when passing an unsigned transaction object. Note, Im passing in a SpoofTransaction object to chain.Chain.estimate_gas(), adding override attributes one at a time to get all required attributes:

instrinsic_gas

eth_tester/main.py:445: in estimate_gas
    raw_gas_estimate = self.backend.estimate_gas(raw_transaction)
eth_tester/backends/pyevm/main.py:444: in estimate_gas
    return self.chain.estimate_gas(spoof_evm_transaction)
../py-evm/evm/chains/base.py:478: in estimate_gas
    return self.gas_estimator(state, transaction)
cytoolz/functoolz.pyx:232: in cytoolz.functoolz.curry.__call__
    ???
../py-evm/evm/estimators/gas.py:54: in binary_gas_search
    minimum_transaction = SpoofTransaction(transaction, gas=transaction.intrinsic_gas)
[...]
E           AttributeError: 'SpuriousDragonUnsignedTransaction' object has no attribute 'intrinsic_gas'

get_sender()

    def __init__(self, transaction, **overrides):
        if 'get_sender' not in overrides:
>           current_sender = transaction.get_sender()
E           AttributeError: 'SpuriousDragonUnsignedTransaction' object has no attribute 'get_sender'

s, r, & v in validate_transaction

eth_tester/main.py:445: in estimate_gas
    raw_gas_estimate = self.backend.estimate_gas(raw_transaction)
eth_tester/backends/pyevm/main.py:444: in estimate_gas
    return self.chain.estimate_gas(spoof_evm_transaction)
../py-evm/evm/chains/base.py:478: in estimate_gas
    return self.gas_estimator(state, transaction)
cytoolz/functoolz.pyx:232: in cytoolz.functoolz.curry.__call__
    ???
../py-evm/evm/estimators/gas.py:55: in binary_gas_search
    if _get_computation_error(state, minimum_transaction) is None:
../py-evm/evm/estimators/gas.py:29: in _get_computation_error
    computation = state.execute_transaction(transaction)
../py-evm/evm/vm/forks/spurious_dragon/vm_state.py:22: in execute_transaction
    computation = _execute_frontier_transaction(self, transaction)
../py-evm/evm/vm/forks/frontier/vm_state.py:56: in _execute_frontier_transaction
    vm_state.validate_transaction(transaction)
../py-evm/evm/vm/forks/homestead/vm_state.py:13: in validate_transaction
    validate_homestead_transaction(self, transaction)
../py-evm/evm/vm/forks/homestead/validation.py:14: in validate_homestead_transaction
    if transaction.s > SECPK1_N // 2 or transaction.s == 0:
../py-evm/evm/utils/spoof.py:10: in __getattr__
    return getattr(self.spoof_target, attr)

sender

eth_tester/main.py:445: in estimate_gas
    raw_gas_estimate = self.backend.estimate_gas(raw_transaction)
eth_tester/backends/pyevm/main.py:444: in estimate_gas
    return self.chain.estimate_gas(spoof_evm_transaction)
../py-evm/evm/chains/base.py:478: in estimate_gas
    return self.gas_estimator(state, transaction)
cytoolz/functoolz.pyx:232: in cytoolz.functoolz.curry.__call__
    ???
../py-evm/evm/estimators/gas.py:55: in binary_gas_search
    if _get_computation_error(state, minimum_transaction) is None:
../py-evm/evm/estimators/gas.py:29: in _get_computation_error
    computation = state.execute_transaction(transaction)
../py-evm/evm/vm/forks/spurious_dragon/vm_state.py:22: in execute_transaction
    computation = _execute_frontier_transaction(self, transaction)
../py-evm/evm/vm/forks/frontier/vm_state.py:56: in _execute_frontier_transaction
    vm_state.validate_transaction(transaction)
../py-evm/evm/vm/forks/homestead/vm_state.py:13: in validate_transaction
    validate_homestead_transaction(self, transaction)
../py-evm/evm/vm/forks/homestead/validation.py:17: in validate_homestead_transaction
    validate_frontier_transaction(evm, transaction)
../py-evm/evm/vm/forks/frontier/validation.py:8: in validate_frontier_transaction
    sender_balance = vm_state.read_only_state_db.get_balance(transaction.sender)
../py-evm/evm/utils/spoof.py:10: in __getattr__
    return getattr(self.spoof_target, attr)
AttributeError: 'SpuriousDragonUnsignedTransaction' object has no attribute 'sender'

Im unsure if this approach will break anything. A few questions I have:

pipermerriam commented 6 years ago

what is the effect of inserting dummy values for v, s and r in the context of gas estimation? Im hoping none.

Should be minimal depending on what else you spoof. Looking into what happens when transactions get validated is probably the right place to start here.

Is this overdoing it

As long as our mechanism for gas estimation appropriately handles things like gas refunds, base transaction gas costs, etc, I'm fine with whatever approach we choose.

dylanjw commented 6 years ago

I think if the transaction execution refactor in: https://github.com/ethereum/py-evm/pull/383 could include an entrypoint to pass a message rather than a transaction, it would be pretty easy to implement a call method to use in the gas estimation.

pipermerriam commented 6 years ago

Can that be done by just manually constructing the message instance and then manually calling executor.run_computation -> executor.run_post_computation with the message instance?