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

Fix debug_traceCall to handle underpriced transactions #7510

Merged
merged 6 commits into from
Sep 13, 2024

Conversation

7suyash7
Copy link
Contributor

Fixes: #7503 where debug_traceCall was failing due to gas price checks, while eth_call was succeeding.

Output from debug_traceCall

curl --data '{               
  "jsonrpc": "2.0",
  "id": 0,
  "method": "debug_traceCall",
  "params": [ {
    "to": "0x0aae40965e6800cd9b1f4b05ff21581047e3f91e",
    "data": "0x000000000000000000000000000000000000000000000000000000000001A00E"
  }, "latest"]
}' -H "Content-Type: application/json" -X POST http://127.0.0.1:8545
{"jsonrpc":"2.0","id":0,"result":{"gas":21164,"failed":false,"returnValue":"","structLogs":[{"pc":0,"op":"STOP","gas":7978836,"gasCost":0,"depth":1,"stack":[],"memory":[],"storage":{},"reason":null}]}}

Signed-off-by: 7suyash7 <suyashnyn1@gmail.com>
@@ -69,6 +70,44 @@ protected TraceOptions getTraceOptions(final JsonRpcRequestContext requestContex
}
}

protected Object resultByBlockHeader(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method doesn't appear to be used anywhere in DebugTraceCall

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, yeah, I just removed it, I also removed the errorResponse method. Weirdly, the compiler caught that errorResponse was not being used anywhere when I tried compiling and building after removing resultByBlockHeader right now but didn't catch resultByBlockHeader...

Signed-off-by: 7suyash7 <suyashnyn1@gmail.com>
Copy link
Contributor

@fab-10 fab-10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fix the issue with debug_traceCall, but since it part of a group of RPC methods extending the AbstractTraceByBlock class, it is better to review if the issue affects other methods in the group, and compare the behavior with other clients to be sure there are not other differences.

@7suyash7
Copy link
Contributor Author

7suyash7 commented Sep 7, 2024

This fix the issue with debug_traceCall, but since it part of a group of RPC methods extending the AbstractTraceByBlock class, it is better to review if the issue affects other methods in the group, and compare the behavior with other clients to be sure there are not other differences.

@fab-10 What do you expect the next steps to be here?

@fab-10
Copy link
Contributor

fab-10 commented Sep 9, 2024

@7suyash7 first I suggest to review if trace_call, trace_callMany and trace_rawTransaction that extend AbstractTraceByBlock like debug_traceCall have a similar issue and depending on that uniform the fix across all of them.

@7suyash7
Copy link
Contributor Author

@7suyash7 first I suggest to review if trace_call, trace_callMany and trace_rawTransaction that extend AbstractTraceByBlock like debug_traceCall have a similar issue and depending on that uniform the fix across all of them.

Hi, @fab-10,
As you suggested, I've checked if trace_call, trace_callMany and trace_rawTransaction have a similar issue to what, debug_traceCall had, I just checked them and they look like they are working fine after testing.

Here is the output from each one of them:

  1. trace_call:
 curl -X POST --data '{
  "jsonrpc":"2.0",
  "method":"trace_call",
  "params":[
    {
      "to": "0x0aae40965e6800cd9b1f4b05ff21581047e3f91e",
      "data": "0x000000000000000000000000000000000000000000000000000000000001A00E"
    },
    ["trace"],
    "latest"
  ],
  "id":1
}' http://localhost:8545
{"jsonrpc":"2.0","id":1,"result":{"output":"0x","stateDiff":null,"trace":[{"action":{"callType":"call","from":"0x0000000000000000000000000000000000000000","gas":"0x9fad54","input":"0x000000000000000000000000000000000000000000000000000000000001a00e","to":"0x0aae40965e6800cd9b1f4b05ff21581047e3f91e","value":"0x0"},"result":{"gasUsed":"0x0","output":"0x"},"subtraces":0,"traceAddress":[],"type":"call"}],"vmTrace":null}}
  1. trace_callMany
 curl -X POST --data '{
  "jsonrpc":"2.0",
  "method":"trace_callMany",
  "params":[
    [
      [
        {
          "to": "0x0aae40965e6800cd9b1f4b05ff21581047e3f91e",
          "data": "0x000000000000000000000000000000000000000000000000000000000001A00E"
        },
        ["trace"]
      ]
    ],
    "latest"
  ],
  "id":1
}' http://localhost:8545
{"jsonrpc":"2.0","id":1,"result":[{"output":"0x","stateDiff":null,"trace":[{"action":{"callType":"call","from":"0x0000000000000000000000000000000000000000","gas":"0x9fad54","input":"0x000000000000000000000000000000000000000000000000000000000001a00e","to":"0x0aae40965e6800cd9b1f4b05ff21581047e3f91e","value":"0x0"},"result":{"gasUsed":"0x0","output":"0x"},"subtraces":0,"traceAddress":[],"type":"call"}],"vmTrace":null}]}
  1. trace_rawTransaction
curl -X POST --data '{
  "jsonrpc":"2.0",
  "method":"trace_rawTransaction",
  "params":["0x01f86f8205390184ee6b2800825208940aae40965e6800cd9b1f4b05ff21581047e3f91e88016345785d8a000080c001a05c91601be0a97bcf8bf8dee799ed5fb785f9bea38bd1fa899d047cefb553b4c2a06b417fb534a24c1bcb63301ea462b982b2454dedf624b6195e6805315429f130", ["trace"]],
  "id":1
}' http://localhost:8545
{"jsonrpc":"2.0","id":1,"result":{"output":"0x","stateDiff":null,"trace":[{"action":{"callType":"call","from":"0x99ac17ce1163d1de38f6fa64d5aa96cbb19c0c55","gas":"0x0","input":"0x","to":"0x0aae40965e6800cd9b1f4b05ff21581047e3f91e","value":"0x16345785d8a0000"},"result":{"gasUsed":"0x0","output":"0x"},"subtraces":0,"traceAddress":[],"type":"call"}],"vmTrace":null}}

Let me know if you need further information or want me to conduct additional tests.

@fab-10
Copy link
Contributor

fab-10 commented Sep 11, 2024

Thanks @7suyash7 for checking, I think that is enough and the change can be limited to this call, could you just add a test case and a changelog entry to complete the fix?

Signed-off-by: 7suyash7 <suyashnyn1@gmail.com>
@7suyash7
Copy link
Contributor Author

Thanks @7suyash7 for checking, I think that is enough and the change can be limited to this call, could you just add a test case and a changelog entry to complete the fix?

@fab-10 I just did, can you please check?

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
@fab-10 fab-10 enabled auto-merge (squash) September 12, 2024 11:40
@fab-10
Copy link
Contributor

fab-10 commented Sep 12, 2024

LGTM

@fab-10 fab-10 merged commit 5df2a71 into hyperledger:main Sep 13, 2024
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

debug_traceCall fails with "gasPrice is less than the current BaseFee" while eth_call works
3 participants