-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
continuation of previous discrepancy tests #3782
Milestone
Comments
sslivkoff
added
C-bug
An unexpected or incorrect behavior
S-needs-triage
This issue needs to be labelled
labels
Jul 14, 2023
added almost every trace method to the flood equality test seeing discrepancies in these methods
|
onbjerg
added
A-rpc
Related to the RPC implementation
and removed
S-needs-triage
This issue needs to be labelled
labels
Jul 19, 2023
This was referenced Jul 24, 2024
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
Describe the bug
this is a sequel to #2457
rerunning the flood equality tests I'm seeing discrepancies for 7 methods
with this command
flood all erigon=$IP_ERIGON:8545 reth=$IP_RETH:8545 --equality
I am seeing this:
Steps to reproduce
flood all erigon=$IP_ERIGON:8545 reth=$IP_RETH:8545 --equality
Node logs
No response
Platform(s)
Linux (x86)
What version/commit are you on?
v0.1.0-alpha.3-dbafe23c
What database version are you on?
Current database version: 1
Local database version: 1
If you've built Reth from source, provide the full command you used
No response
Code of Conduct
The text was updated successfully, but these errors were encountered: