Skip to content

Add transaction hash for debug trace responses#8205

Merged
matkt merged 9 commits intobesu-eth:mainfrom
matkt:feature/fix-debug-trace-api
Feb 4, 2025
Merged

Add transaction hash for debug trace responses#8205
matkt merged 9 commits intobesu-eth:mainfrom
matkt:feature/fix-debug-trace-api

Conversation

@matkt
Copy link
Copy Markdown
Contributor

@matkt matkt commented Jan 30, 2025

PR description

This PR adds the transaction hash to the response of a debug trace API. This aligns the behavior with other clients and makes debugging easier.

for debug_traceBlock* the trx hash will be displayed and for debugTraceTransaction the tx hash will not be displayed

{
      "txHash":"0x49f505ed122c398287e1558275ccfcaa2d22df98df5b55e6c70635e6aa3a6ecc",
      "result":{
        "gas":21164,
        "failed":false,
        "returnValue":"",
        "structLogs":[
          {
            "pc":0,
            "op":"STOP",
            "gas":17592186023252,
            "gasCost":0,
            "depth":1
          }
        ]
      }

Fixed Issue(s)

fixes #8195

Thanks for sending a pull request! Have you done the following?

  • Checked out our contribution guidelines?
  • Considered documentation and added the doc-change-required label to this PR if updates are required.
  • Considered the changelog and included an update if required.
  • For database changes (e.g. KeyValueSegmentIdentifier) considered compatibility and performed forwards and backwards compatibility tests

Locally, you can run these tests to catch failures early:

  • spotless: ./gradlew spotlessApply
  • unit tests: ./gradlew build
  • acceptance tests: ./gradlew acceptanceTest
  • integration tests: ./gradlew integrationTest
  • reference tests: ./gradlew ethereum:referenceTests:referenceTests

matkt added 2 commits January 30, 2025 16:10
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
@matkt matkt requested a review from ahamlat January 30, 2025 15:48
Copy link
Copy Markdown
Contributor

@ahamlat ahamlat left a comment

Choose a reason for hiding this comment

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

LGTM, just a small comment related to formatting json files in the tests.
I think it makes sens to keep existing format, unless you have a strong motivation around it.

"id":1,
"result":{
"txHash":"0x2d8f594c7a98a9cb3d3a21d551f68e916e7ecd767971b061f2994b9a7426fba8",
"result":{
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe review the format to keep is as it is and in the differences only what changes. I see that almost the whole json file changed because of the change in the format, e.g

}, {

is replaced with

},
 {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think I will not be able to avoid that because it will change with the indentation

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

fair enough

matkt added 2 commits January 30, 2025 17:11
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
@matkt matkt requested a review from ahamlat January 31, 2025 14:41
matkt added 2 commits January 31, 2025 15:41
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
@matkt matkt enabled auto-merge (squash) February 4, 2025 09:03
@macfarla
Copy link
Copy Markdown
Contributor

macfarla commented Feb 4, 2025

@matkt think these might be legit failures (not flaky)

DebugTraceJsonRpcHttpBySpecTest > jsonRPCCallWithSpecFile(String, URL) > 5: debug_traceCall_noGasPrice.json FAILED
    org.opentest4j.AssertionFailedError at AbstractJsonRpcHttpBySpecTest.java:250

DebugTraceJsonRpcHttpBySpecTest > jsonRPCCallWithSpecFile(String, URL) > 6: debug_traceCall_withFutureNonce.json FAILED
    org.opentest4j.AssertionFailedError at AbstractJsonRpcHttpBySpecTest.java:250

Signed-off-by: Karim Taam <karim.t2am@gmail.com>
@matkt
Copy link
Copy Markdown
Contributor Author

matkt commented Feb 4, 2025

debug_traceCall_noGasPrice

should be good now

@matkt matkt merged commit 214673c into besu-eth:main Feb 4, 2025
pullurib pushed a commit to pullurib/besu that referenced this pull request Feb 6, 2025
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: Bhanu Pulluri <bhanu.pulluri@kaleido.io>
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_traceBlockByX response is missing txHash for trace entries

3 participants