ethereum-optimism / op-geth

GNU Lesser General Public License v3.0
294 stars 762 forks source link

Incorrect gasUsed in callTracer #298

Open quickchase opened 7 months ago

quickchase commented 7 months ago

System information

Geth version: Geth/v1.101311.0-stable-e9a306ba/linux-amd64/go1.21.9 CL client & version: op-node v1.7.3 OS & Version:Linux

Issue

Given the following call:

{"id":"synth","jsonrpc":"2.0","method":"debug_traceTransaction","params":["0xc93e9c095619a713a1e29557d8a51a7f020844b3b97c549f6c3e1da721fba115",{"tracer":"callTracer"}]}

op-geth is returning:

{
  "jsonrpc": "2.0",
  "id": "synth",
  "result": {
    "from": "0xdeaddeaddeaddeaddeaddeaddeaddeaddead0001",
    "gas": "0xf4240",
    "gasUsed": "0xf4240",
    "to": "0x4200000000000000000000000000000000000015",
    "input": "0x440a5e2000000558000c5fc500000000000000030000000066197f4b00000000012bb4050000000000000000000000000000000000000000000000000000002e4cb91cea000000000000000000000000000000000000000000000000000000000000000145a826de7841ea76c056554f1c351464a964420e8283b88a1762549f987e688c0000000000000000000000006887246668a3b87f54deb3b94ba47a6f63f32985",
    "calls": [
      {
        "from": "0x4200000000000000000000000000000000000015",
        "gas": "0xe9b69",
        "gasUsed": "0x3fac",
        "to": "0x07dbe8500fc591d1852b76fee44d5a05e13097ff",
        "input": "0x440a5e2000000558000c5fc500000000000000030000000066197f4b00000000012bb4050000000000000000000000000000000000000000000000000000002e4cb91cea000000000000000000000000000000000000000000000000000000000000000145a826de7841ea76c056554f1c351464a964420e8283b88a1762549f987e688c0000000000000000000000006887246668a3b87f54deb3b94ba47a6f63f32985",
        "value": "0x0",
        "type": "DELEGATECALL"
      }
    ],
    "value": "0x0",
    "type": "CALL"
  }
}

However, the gasUsed is incorrect, if you look at the receipt:

{
  "jsonrpc": "2.0",
  "id": 1,
  "result": {
    "blockHash": "0x5cf34d3f6ec2dae3c78a5dd9e89a8dad5d139f91983ce91aa0f75ece0ada1bb9",
    "blockNumber": "0x712d12d",
    "contractAddress": null,
    "cumulativeGasUsed": "0xab4b",
    "depositNonce": "0xcd0eb6",
    "depositReceiptVersion": "0x1",
    "effectiveGasPrice": "0x0",
    "from": "0xdeaddeaddeaddeaddeaddeaddeaddeaddead0001",
    "gasUsed": "0xab4b",
    "logs": [],
    "logsBloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
    "status": "0x1",
    "to": "0x4200000000000000000000000000000000000015",
    "transactionHash": "0xd8fca681d83ef12ebf44a31dafad3431b5cfcb8f7490851c8d4337085a99a38d",
    "transactionIndex": "0x0",
    "type": "0x7e"
  }
}

This is how much gas is used: "cumulativeGasUsed": "0xab4b", NOT 0xf4240

Both op-erigon and the explorer say that the output of the debug_traceTransaction should be "gasUsed": "0xab4b",

protolambda commented 7 months ago

Found the issue: see https://github.com/ethereum-optimism/op-geth/blob/9e50b7db766762138a0db913fe85e7a2cb82c832/core/state_transition.go#L442

When the tracer reports gasUsed, it reports it based on the gasUsed that was determined based on remaining gas in CaptureTxEnd. For deposits we don't have refunds, hence 0 remaining gas. But that doesn't mean all gas was used. Hence the incorrect tracer value.

In the implementation the gas-price is 0, and so a 0 refund is given to deposits, without skipping the refund code-path.

I think we can reduce the diff, and just report the actual remainingGas to the tracer. Would like some feedback here from @trianglesphere and/or @axelKingsley.

trianglesphere commented 7 months ago

@smartcontracts actually added the CaptureTxEnd(0) pretty recently to handle the case of failed deposit transactions.

quickchase commented 6 months ago

Bump 🙇‍♂️