From 9a5e0d28330b8f668353ce0bc20973da736b518d Mon Sep 17 00:00:00 2001 From: Jamie Lokier Date: Mon, 3 May 2021 07:58:45 +0100 Subject: [PATCH] RPC: Change rpcEstimateGas to use the EVM without a fake transaction Change `rpcEstimateGas` to setup and execute a computation directly, in a similar way to `rpcDoCall` and `rpcMakeCall`, instead of constructing a fake transaction and then validating it. This patch does not (or should not) change any behaviour. Although this looks a bit messy as it duplicates parts of `validateTransaction` and `processTransaction`, proc names have been used to hopefully keep the meanings clear, and it's just a stepping stone as those transaction functions will be changed next. Also the behaviour of RPC `estimateGas` may not be correct (although this patch is careful not to change it), so it's good to make it explicit so we can see how it differs from other RPCs. Doing this change exposed some interesting behaviour differences between the `call` RPC and `estimateGas` RPC, which may be bugs, or may be intentional. These differences are now obvious and explicit. The unclear areas are not well documented by any of the clients, even Infura which says a bit more than the others. So to find out if they are intended, we'll have to run tests against other Ethereum services. Guessing, on the face of it, it looks likely that RPC `call` should: - Setup EIP2929 access lists - Account for intrinsic gas (maybe not because zero-gas transactions are ok) And it looks likely that RPC `estimateGas` should: - Not return zero when an account has insufficient balance - Maybe use a different gas cost estimate when one isn't supplied in the RPC Signed-off-by: Jamie Lokier --- nimbus/transaction/call_evm.nim | 78 ++++++++++++++++++++++++++++----- 1 file changed, 66 insertions(+), 12 deletions(-) diff --git a/nimbus/transaction/call_evm.nim b/nimbus/transaction/call_evm.nim index bf9a49187..c1551a242 100644 --- a/nimbus/transaction/call_evm.nim +++ b/nimbus/transaction/call_evm.nim @@ -10,7 +10,7 @@ import eth/common/eth_types, stint, options, stew/byteutils, ".."/[vm_types, vm_types2, vm_state, vm_computation, utils], ".."/[db/db_chain, config, vm_state_transactions, rpc/hexstrings], - ".."/[db/accounts_cache, p2p/executor], eth/trie/db + ".."/[db/accounts_cache, transaction, vm_precompiles, vm_gas_costs], eth/trie/db type RpcCallData* = object @@ -73,6 +73,38 @@ proc rpcMakeCall*(call: RpcCallData, header: BlockHeader, chain: BaseChainDB): ( comp.execComputation() return (comp.output.toHex, gas - comp.gasMeter.gasRemaining, comp.isError) +func rpcIntrinsicGas(call: RpcCallData, fork: Fork): GasInt = + var intrinsicGas = call.data.intrinsicGas(fork) + if call.contractCreation: + intrinsicGas = intrinsicGas + gasFees[fork][GasTXCreate] + return intrinsicGas + +func rpcValidateCall(call: RpcCallData, vmState: BaseVMState, gasLimit: GasInt, + fork: Fork, intrinsicGas: var GasInt, gasCost: var UInt256): bool = + # This behaviour matches `validateTransaction`, used by `processTransaction`. + if vmState.cumulativeGasUsed + gasLimit > vmState.blockHeader.gasLimit: + return false + let balance = vmState.readOnlyStateDB.getBalance(call.source) + gasCost = gasLimit.u256 * call.gasPrice.u256 + if gasCost > balance or call.value > balance - gasCost: + return false + intrinsicGas = rpcIntrinsicGas(call, fork) + if intrinsicGas > gasLimit: + return false + return true + +proc rpcInitialAccessListEIP2929(call: RpcCallData, vmState: BaseVMState, fork: Fork) = + # EIP2929 initial access list. + if fork >= FkBerlin: + vmState.mutateStateDB: + db.accessList(call.source) + # For contract creations the EVM will add the contract address to the + # access list itself, after calculating the new contract address. + if not call.contractCreation: + db.accessList(call.to) + for c in activePrecompiles(): + db.accessList(c) + proc rpcEstimateGas*(call: RpcCallData, header: BlockHeader, chain: BaseChainDB, haveGasLimit: bool): GasInt = # TODO: handle revert and error var @@ -80,17 +112,39 @@ proc rpcEstimateGas*(call: RpcCallData, header: BlockHeader, chain: BaseChainDB, # which use previous block stateRoot vmState = newBaseVMState(header.stateRoot, header, chain) fork = toFork(chain.config, header.blockNumber) - tx = Transaction( - accountNonce: vmState.accountdb.getNonce(call.source), - gasPrice: call.gasPrice, - gasLimit: if haveGasLimit: call.gas else: header.gasLimit - vmState.cumulativeGasUsed, - to : call.to, - value : call.value, - payload : call.data, - isContractCreation: call.contractCreation - ) + gasLimit = if haveGasLimit: call.gas else: header.gasLimit - vmState.cumulativeGasUsed + intrinsicGas: GasInt + gasCost: UInt256 + + # Nimbus `estimateGas` has historically checked against remaining gas in the + # current block, balance in the sender account (even if the sender is default + # account 0x00), and other limits, and returned 0 as the gas estimate if any + # checks failed. This behaviour came from how it used `processTransaction` + # which calls `validateTransaction`. For now, keep this behaviour the same. + # Compare this code with `validateTransaction`. + # + # TODO: This historically differs from `rpcDoCall` and `rpcMakeCall`. There + # are other differences in rpc_utils.nim `callData` too. Are the different + # behaviours intended, and is 0 the correct return value to mean "not enough + # gas to start"? Probably not. + if not rpcValidateCall(call, vmState, gasLimit, fork, intrinsicGas, gasCost): + return 0 var dbTx = chain.db.beginTransaction() defer: dbTx.dispose() - result = processTransaction(tx, call.source, vmState, fork) - dbTx.dispose() + + # TODO: EIP2929 setup also historically differs from `rpcDoCall` and `rpcMakeCall`. + rpcInitialAccessListEIP2929(call, vmState, fork) + + # TODO: Deduction of `intrinsicGas` also differs from `rpcDoCall` and `rpcMakeCall`. + var c = rpcSetupComputation(vmState, call, fork, gasLimit - intrinsicGas) + vmState.mutateStateDB: + db.subBalance(call.source, gasCost) + + execComputation(c) + + if c.shouldBurnGas: + return gasLimit + let maxRefund = (gasLimit - c.gasMeter.gasRemaining) div 2 + let refund = min(c.getGasRefund(), maxRefund) + return gasLimit - c.gasMeter.gasRemaining - refund