Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gas_used/gas for the root trace should use tx's #3678

Closed
1 task done
jsvisa opened this issue Jul 9, 2023 · 17 comments
Closed
1 task done

gas_used/gas for the root trace should use tx's #3678

jsvisa opened this issue Jul 9, 2023 · 17 comments
Labels
A-rpc Related to the RPC implementation C-bug An unexpected or incorrect behavior M-prevent-stale Prevents old inactive issues/PRs from being closed due to inactivity S-needs-investigation This issue requires detective work to figure out what's going wrong

Comments

@jsvisa
Copy link
Contributor

jsvisa commented Jul 9, 2023

Describe the bug

For this tx 0x00f139f7deb8b18d27bbed972720ba4e8a3b7d9806b186bdb29cdbfff3b95a9e,

we request the result as:

{
  "jsonrpc": "2.0",
  "method": "trace_transaction",
  "id": 67,
  "params": [
    "0x00f139f7deb8b18d27bbed972720ba4e8a3b7d9806b186bdb29cdbfff3b95a9e"
  ]
}

the response is:

{
  "id": 67,
  "jsonrpc": "2.0",
  "result": [
    {
      "action": {
        "callType": "call",
        "from": "0x93f3ef9fa5e668b0ccf5ad93e53f991aaea6131c",
        "gas": "0x1068",
        "input": "0x",
        "to": "0x28c6c06298d514db089934071355e5743bf21d60",
        "value": "0x12134e80a17c78e"
      },
      "blockHash": "0x92486dfdb758642482f94049048abbef3d6c6aeaeaac30df0e005473d6a83f05",
      "blockNumber": 17626652,
      "result": {
        "gasUsed": "0x0",
        "output": "0x"
      },
      "subtraces": 0,
      "traceAddress": [],
      "transactionHash": "0x00f139f7deb8b18d27bbed972720ba4e8a3b7d9806b186bdb29cdbfff3b95a9e",
      "transactionPosition": 349,
      "type": "call"
    }
  ]
}

The gas and gas_used are incorrect, these two values should be taken out of the transaction structure, see more in this pr ethereum/go-ethereum#27029

Steps to reproduce

Node logs

No response

Platform(s)

Linux (x86)

What version/commit are you on?

1330fc1

What database version are you on?

No response

If you've built Reth from source, provide the full command you used

docker build

Code of Conduct

  • I agree to follow the Code of Conduct
@jsvisa jsvisa added C-bug An unexpected or incorrect behavior S-needs-triage This issue needs to be labelled labels Jul 9, 2023
@mattsse
Copy link
Collaborator

mattsse commented Jul 9, 2023

hmm,
I thought I added this

#3594

need to double-check what I'm missing

not sure why this is even 0, perhaps some price snafu

EDIT:
comparing this to geth isn't really helpful, because geth doesn't have this endpoint, how does erigon behave her @jsvisa ?

@jsvisa
Copy link
Contributor Author

jsvisa commented Jul 9, 2023

Erigon’s response is also incorrect. You can use debug_traceTransaction api?

@mattsse
Copy link
Collaborator

mattsse commented Jul 9, 2023

You can use debug_traceTransaction api?

can you elaborate?

@jsvisa
Copy link
Contributor Author

jsvisa commented Jul 9, 2023

You can use debug_traceTransaction api?

can you elaborate?

this API?

{
  "jsonrpc": "2.0",
  "method": "debug_traceTransaction",
  "id": 67,
  "params": [
    "0x00f139f7deb8b18d27bbed972720ba4e8a3b7d9806b186bdb29cdbfff3b95a9e",
    {
      "timeout": "120s",
      "tracer": "callTracer"
    }
  ]
}

by the way, I think the refund's logical is too complicated. IMO, for the root trace, the gas and gas_used use the transaction's is a simple implementation.

@mattsse
Copy link
Collaborator

mattsse commented Jul 9, 2023

there's no real spec for any of this, I can see the argument for using the tx's values for the root call trace, but imo that should represent the root call, but I'm also not entirely familiar with evm internals here

need to think about this

@jsvisa
Copy link
Contributor Author

jsvisa commented Jul 9, 2023

@mattsse thanks for it.

BTW, May I ask if Reth generates and stores traces to the database during synchronization? If so, this means the RPC requests directly read traces from the database instead of replaying to generate them like Geth does.
If that is the case, then for traces with flawed generation logic - even if the bug is fixed, because the data will not regenerate, the RPC will still get incorrect data since it is reading the old faulty data from the database. So for this case #3427 (comment), I did need to resync the reth node

@mattsse
Copy link
Collaborator

mattsse commented Jul 9, 2023

if Reth generates and stores traces to the database during synchronization?

it does not, traces are generated entirely by replaying the transaction on the fly

@jsvisa
Copy link
Contributor Author

jsvisa commented Jul 9, 2023

if Reth generates and stores traces to the database during synchronization?

it does not, traces are generated entirely by replaying the transaction on the fly

thanks, but for the Genesis accounts, I did need to resync?(because the bug I raised was not fixed)

@mattsse
Copy link
Collaborator

mattsse commented Jul 9, 2023

thanks, but for the Genesis accounts, I did need to resync?

yes

because the bug I raised was not fixed

which one?

@jsvisa
Copy link
Contributor Author

jsvisa commented Jul 9, 2023

which one?

#3427 (comment)

@mattsse mattsse added S-needs-investigation This issue requires detective work to figure out what's going wrong A-rpc Related to the RPC implementation and removed S-needs-triage This issue needs to be labelled labels Jul 10, 2023
@mattsse
Copy link
Collaborator

mattsse commented Jul 13, 2023

the initial gas limit now uses the tx's gas limit

is gas_used still incorrect? if so I need to have a closer look at how this is tracked during tracing, perhaps we're missing the init costs

@jsvisa
Copy link
Contributor Author

jsvisa commented Jul 13, 2023

I'll retest this with the latest commit in one or more days, please wait for a moment. :)

@jsvisa
Copy link
Contributor Author

jsvisa commented Jul 13, 2023

@mattsse for this tx, the gas_used still is zero, I'm running reth with commit: 8bfc3d093ed27ff00b3f03ebd898da1b782e0321

request

{
  "jsonrpc": "2.0",
  "method": "trace_transaction",
  "id": 67,
  "params": [
    "0x00f139f7deb8b18d27bbed972720ba4e8a3b7d9806b186bdb29cdbfff3b95a9e"
  ]
}

response

{
  "id": 67,
  "jsonrpc": "2.0",
  "result": [
    {
      "action": {
        "callType": "call",
        "from": "0x93f3ef9fa5e668b0ccf5ad93e53f991aaea6131c",
        "gas": "0x6270",
        "input": "0x",
        "to": "0x28c6c06298d514db089934071355e5743bf21d60",
        "value": "0x12134e80a17c78e"
      },
      "blockHash": "0x92486dfdb758642482f94049048abbef3d6c6aeaeaac30df0e005473d6a83f05",
      "blockNumber": 17626652,
      "result": {
        "gasUsed": "0x0",
        "output": "0x"
      },
      "subtraces": 0,
      "traceAddress": [],
      "transactionHash": "0x00f139f7deb8b18d27bbed972720ba4e8a3b7d9806b186bdb29cdbfff3b95a9e",
      "transactionPosition": 349,
      "type": "call"
    }
  ]
}

@mattsse
Copy link
Collaborator

mattsse commented Jul 13, 2023

thanks, I also found #3760

will investigate once merged

appreciate your effort on this

@jsvisa
Copy link
Contributor Author

jsvisa commented Jul 18, 2023

@mattsse I just found another case of gas_used incorrect, this tx 0x05b92ce4dd554193718d918e4c01150014547f6a9a2762c425f23f7623ca30d4 is failed of OOG, but the gas_used in reth's response is smaller than gas_limit. Not sure if it helps, but hopefully this can be an easy entry point to the problem.

geth's response

{
  "id": 67,
  "jsonrpc": "2.0",
  "result": {
    "calls": [
      {
        "calls": [
          {
            "error": "out of gas",
            "from": "0x6e2a43be0b1d33b726f0ca3b8de60b3482b8b050",
            "gas": "0x75de",
            "gasUsed": "0x75de",
            "input": "0xa9059cbb00000000000000000000000000354f2b395ec8fd6957de64aa602ec1fb09ff9f00000000000000000000000000000000000000000000000ab3e25398a24d6af0",
            "to": "0x2291323cf23d1553c6f79dc30b4a8865c03a90cf",
            "type": "DELEGATECALL",
            "value": "0x0"
          }
        ],
        "error": "execution reverted",
        "from": "0x08c7676680f187a31241e83e6d44c03a98adab05",
        "gas": "0x8ab8",
        "gasUsed": "0x8901",
        "input": "0xa9059cbb00000000000000000000000000354f2b395ec8fd6957de64aa602ec1fb09ff9f00000000000000000000000000000000000000000000000ab3e25398a24d6af0",
        "to": "0x6e2a43be0b1d33b726f0ca3b8de60b3482b8b050",
        "type": "CALL",
        "value": "0x0"
      }
    ],
    "error": "execution reverted",
    "from": "0x00354f2b395ec8fd6957de64aa602ec1fb09ff9f",
    "gas": "0x1ab2c",
    "gasUsed": "0x1a766",
    "input": "0x3d13f87400000000000000000000000000354f2b395ec8fd6957de64aa602ec1fb09ff9f00000000000000000000000000000000000000000000000ab3e25398a24d6af000000000000000000000000000000000000000000000000000000000000000600000000000000000000000000000000000000000000000000000000000000011f6c443cf87f6b7ee2691446a33e858afb83c77fdfd04d0fd2fcb706cd71c6283e410e56dd67216f608e515cfb3084d315a29964e055c6e0e9f040ab2221680a481980dbc5f10d08738024f92c8f9b99bb7e3af4ee32ce31cf896a4b9c4e763a1c0160bf40bd44b3faf0dac89f379938ecf541dd4750ccbd2eadebd8ec8b42066d276b58f73bc2b8738431c35322ee38af1d2d7fdd3a2acaf0c975a83a19da46ef0d5fed61f8addef1df164976ac69f044c09c607546a04bb98c3471d9d3858887c14efc89ff341c184098c00b9af3d9df7efe9a4f699c192dfee6b0e71fe72eeacd48c95456cde33666cf718191cda5c9656f09ab6882086a342e7ef88edfd16811868329c9d3d90a6ddca20b871c8272c64cc8844917e80474847de77bac905cdc58a8cba8bd7575b3a8864480085cb6e523653329fccba7406d07f07908c33aa947c830c696aec9b92fcc1e76e60958c1c7aee9c754da40f0d4fd74534af63b853497fb9095e14de7f956a946c8ad17d438eb04efa20b1e05c989c2ae278027b484629f1620b6d875e941dd5a1f4a94527ef5912b27b8e11a339deb3b357b3795c469bbc09dca5fcebb9c3f972ccb474c4f3321fd49e11d8f5d1b2a511ffbdd805e48e5f11a74803cefedf1794161e3164e9ee8eb1336b4478256a1e33624640977ab2908d920d1db6f1beaf8d19b4407251dcf036e5f48bcfe53ce514af6f0ec16713ef301bdf97a87aa0549710597199a5a25c2a88802035c85fb295b7db",
    "to": "0x08c7676680f187a31241e83e6d44c03a98adab05",
    "type": "CALL",
    "value": "0x0"
  }
}

reth's response

{
  "id": 67,
  "jsonrpc": "2.0",
  "result": [
    {
      "action": {
        "callType": "call",
        "from": "0x00354f2b395ec8fd6957de64aa602ec1fb09ff9f",
        "gas": "0x1ab2c",
        "input": "0x3d13f87400000000000000000000000000354f2b395ec8fd6957de64aa602ec1fb09ff9f00000000000000000000000000000000000000000000000ab3e25398a24d6af000000000000000000000000000000000000000000000000000000000000000600000000000000000000000000000000000000000000000000000000000000011f6c443cf87f6b7ee2691446a33e858afb83c77fdfd04d0fd2fcb706cd71c6283e410e56dd67216f608e515cfb3084d315a29964e055c6e0e9f040ab2221680a481980dbc5f10d08738024f92c8f9b99bb7e3af4ee32ce31cf896a4b9c4e763a1c0160bf40bd44b3faf0dac89f379938ecf541dd4750ccbd2eadebd8ec8b42066d276b58f73bc2b8738431c35322ee38af1d2d7fdd3a2acaf0c975a83a19da46ef0d5fed61f8addef1df164976ac69f044c09c607546a04bb98c3471d9d3858887c14efc89ff341c184098c00b9af3d9df7efe9a4f699c192dfee6b0e71fe72eeacd48c95456cde33666cf718191cda5c9656f09ab6882086a342e7ef88edfd16811868329c9d3d90a6ddca20b871c8272c64cc8844917e80474847de77bac905cdc58a8cba8bd7575b3a8864480085cb6e523653329fccba7406d07f07908c33aa947c830c696aec9b92fcc1e76e60958c1c7aee9c754da40f0d4fd74534af63b853497fb9095e14de7f956a946c8ad17d438eb04efa20b1e05c989c2ae278027b484629f1620b6d875e941dd5a1f4a94527ef5912b27b8e11a339deb3b357b3795c469bbc09dca5fcebb9c3f972ccb474c4f3321fd49e11d8f5d1b2a511ffbdd805e48e5f11a74803cefedf1794161e3164e9ee8eb1336b4478256a1e33624640977ab2908d920d1db6f1beaf8d19b4407251dcf036e5f48bcfe53ce514af6f0ec16713ef301bdf97a87aa0549710597199a5a25c2a88802035c85fb295b7db",
        "to": "0x08c7676680f187a31241e83e6d44c03a98adab05",
        "value": "0x0"
      },
      "blockHash": "0xed1e05fbbb0b343f137e9519147ecf624b952d126fce46509a7d3db5e1c67004",
      "blockNumber": 17718787,
      "error": "Reverted",
      "result": {
        "gasUsed": "0x12fce",
        "output": "0x"
      },
      "subtraces": 1,
      "traceAddress": [],
      "transactionHash": "0x05b92ce4dd554193718d918e4c01150014547f6a9a2762c425f23f7623ca30d4",
      "transactionPosition": 83,
      "type": "call"
    },
    {
      "action": {
        "callType": "call",
        "from": "0x08c7676680f187a31241e83e6d44c03a98adab05",
        "gas": "0x8ab8",
        "input": "0xa9059cbb00000000000000000000000000354f2b395ec8fd6957de64aa602ec1fb09ff9f00000000000000000000000000000000000000000000000ab3e25398a24d6af0",
        "to": "0x6e2a43be0b1d33b726f0ca3b8de60b3482b8b050",
        "value": "0x0"
      },
      "blockHash": "0xed1e05fbbb0b343f137e9519147ecf624b952d126fce46509a7d3db5e1c67004",
      "blockNumber": 17718787,
      "error": "Reverted",
      "result": {
        "gasUsed": "0x8901",
        "output": "0x"
      },
      "subtraces": 1,
      "traceAddress": [
        0
      ],
      "transactionHash": "0x05b92ce4dd554193718d918e4c01150014547f6a9a2762c425f23f7623ca30d4",
      "transactionPosition": 83,
      "type": "call"
    },
    {
      "action": {
        "callType": "delegatecall",
        "from": "0x6e2a43be0b1d33b726f0ca3b8de60b3482b8b050",
        "gas": "0x75de",
        "input": "0xa9059cbb00000000000000000000000000354f2b395ec8fd6957de64aa602ec1fb09ff9f00000000000000000000000000000000000000000000000ab3e25398a24d6af0",
        "to": "0x2291323cf23d1553c6f79dc30b4a8865c03a90cf",
        "value": "0x0"
      },
      "blockHash": "0xed1e05fbbb0b343f137e9519147ecf624b952d126fce46509a7d3db5e1c67004",
      "blockNumber": 17718787,
      "error": "OutOfGas",
      "result": {
        "gasUsed": "0x296a",
        "output": "0x"
      },
      "subtraces": 0,
      "traceAddress": [
        0,
        0
      ],
      "transactionHash": "0x05b92ce4dd554193718d918e4c01150014547f6a9a2762c425f23f7623ca30d4",
      "transactionPosition": 83,
      "type": "call"
    }
  ]
}
image

@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2023

This issue is stale because it has been open for 14 days with no activity.

@github-actions github-actions bot added the S-stale This issue/PR is stale and will close with no further activity label Aug 2, 2023
@mattsse mattsse removed the S-stale This issue/PR is stale and will close with no further activity label Aug 2, 2023
@mattsse mattsse added the M-prevent-stale Prevents old inactive issues/PRs from being closed due to inactivity label Aug 2, 2023
@mattsse
Copy link
Collaborator

mattsse commented Sep 25, 2023

this is now the case

@mattsse mattsse closed this as completed Sep 25, 2023
@github-project-automation github-project-automation bot moved this from Todo to Done in Reth Tracker Sep 25, 2023
mattsse pushed a commit to paradigmxyz/revm-inspectors that referenced this issue Jul 25, 2024
## Description

This pull request is Part 1/2 of fixing the bug where the `gas` and
`gasUsed` fields in Parity Trace root are incorrect.

Part 2/2 paradigmxyz/reth#9761

## Related Issues and Pull Requests

- Follow: ethereum/go-ethereum#27029
- Improve: paradigmxyz/reth#3678 and paradigmxyz/reth#3719
- Fix: paradigmxyz/reth#9142 with #170 
- Update: paradigmxyz/reth#3782

## Problem

The `gas` and `gasUsed` fields in Geth Debug Trace root should be the
gas limit and gas used for the entire transaction.

However, two fields in Parity Trace root should be the original ones.

### Reproducible Example

With the latest version Reth v1.0.3, using `trace_transaction()` to
trace
the transaction
`0x03128677ee3a9623d20f3c677f423ccc592d126374bf32e331343dd1bdf38b61`

```
curl http://localhost:8545 \
  -X POST \
  -H "Content-Type: application/json" \
  --data '{"method":"trace_transaction","params":["0x03128677ee3a9623d20f3c677f423ccc592d126374bf32e331343dd1bdf38b61"],"id":1,"jsonrpc":"2.0"}'  
```

**From Reth**

```
gas: 0x55493 (349331)
gasUsed: 0x32d16 (208150)
```

**From
[Etherscan](https://etherscan.io/vmtrace?txhash=0x03128677ee3a9623d20f3c677f423ccc592d126374bf32e331343dd1bdf38b61&type=parity#raw)
and QuickNode**

```
gas: 0x4f227 (324135)
gasUsed: 0x36622 (222754)
```

## Solution for `revm-inspectors`

1. Not modify `gas_limit` and `gas_used` in the trace root

    ```diff
    - gas_limit = context.env.tx.gas_limit;
    - trace.set_root_trace_gas_used(gas_used);
- trace.gas_used = gas_used(context.spec_id(), gas.spent(),
gas.refunded() as u64);
    ```

2. The modification in Step 1 will cause another problem

The `gas` field for Geth Debug Trace root will also be reset (not the
gas limitation for the entire transaction).

therefore, can define `set_transaction_gas_limit()` and
`with_transaction_gas_limit()` for Geth Debug,

which is similar to current `set_transaction_gas_used()` and
`with_transaction_gas_used()` for Parity.

3. Then, modify the Reth Part: 

`crates/rpc/rpc/src/trace.rs` and `crates/rpc/rpc/src/debug.rs` to
completely fix the bug.

## Miscellaneous

- Actually, I love the current design, but the results are inconsistent
with those of others.

- When I used `make pr` to test the Reth Part, the issue
paradigmxyz/reth#9381 still exists for me.

    I should only skip tests for `lockfile` and test them seperately.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rpc Related to the RPC implementation C-bug An unexpected or incorrect behavior M-prevent-stale Prevents old inactive issues/PRs from being closed due to inactivity S-needs-investigation This issue requires detective work to figure out what's going wrong
Projects
Archived in project
Development

No branches or pull requests

2 participants