Skip to content

debug_traceBlock use existing block header instead of hash #8184

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

Merged
merged 4 commits into from
Jan 29, 2025

Conversation

siladu
Copy link
Contributor

@siladu siladu commented Jan 28, 2025

By using the other Tracing.processTracing overloaded method, this avoids this lookup

blockchainQueries, blockchainQueries.getBlockHeaderByHash(blockHash), mapper);

and means you don't need to have the block already in the chain when you call debug_traceBlock with a block's RLP in the request.

Since this is an Abstract class, I'm not 100% sure how this might impact the other subclasses (think it's just DebugTraceBlockByHash).

Would like some clarification from someone who knows the API better.

Tested output is the same between debug_traceBlock vs debug_traceBlockByNumber vs debug_traceBlockByHash.

@siladu siladu changed the title Use existing block header rather than looking up again by hash debug_traceBlock use existing block header instead of hash Jan 28, 2025
Copy link
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.

LGFM, just need to fix unit tests.

@@ -102,7 +102,7 @@ protected Collection<DebugTraceTransactionResult> getTraces(
block ->
Tracer.processTracing(
getBlockchainQueries(),
block.getHash(),
Optional.of(block.getHeader()),
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch.
I remember having issues with unit tests related to this, but I agree, it is better to use header instead of the hash.

Signed-off-by: Simon Dudley <[email protected]>
@siladu siladu marked this pull request as ready for review January 29, 2025 01:06
@siladu siladu enabled auto-merge (squash) January 29, 2025 02:12
@siladu siladu merged commit 4b8d935 into hyperledger:main Jan 29, 2025
43 checks passed
daniellehrner pushed a commit to daniellehrner/besu that referenced this pull request Jan 31, 2025
…er#8184)

This means you don't need to have the block already in the chain when you call debug_traceBlock with a block's RLP (you just need the parent).

Signed-off-by: Simon Dudley <[email protected]>
Signed-off-by: Daniel Lehrner <[email protected]>
github-merge-queue bot pushed a commit to Consensys/protocols-release-sandbox that referenced this pull request Feb 4, 2025
* fixes syntax errors (hyperledger#8164)

Signed-off-by: jflo <[email protected]>

* Pectra testnet fork slots (hyperledger#8163)



* adds Prague slot times

Signed-off-by: jflo <[email protected]>

---------

Signed-off-by: jflo <[email protected]>

* Check if discovery service is running before `admin_addPeer` (hyperledger#8160)

Signed-off-by: Gabriel-Trintinalia <[email protected]>

* update eth_call error message (hyperledger#8129)

Signed-off-by: Karim Taam <[email protected]>

* Refactor the methods for getting mutable states in the WorldState provider (hyperledger#8113)

Refactor the methods for retrieving mutable states in the WorldState provider and make additional improvements to clean up the code.

---------

Signed-off-by: Karim Taam <[email protected]>

* Extend simulate transaction on pending block plugin API (hyperledger#8174)

Signed-off-by: Fabio Di Fabio <[email protected]>

* debug_traceBlock use existing block header instead of hash (hyperledger#8184)

This means you don't need to have the block already in the chain when you call debug_traceBlock with a block's RLP (you just need the parent).

Signed-off-by: Simon Dudley <[email protected]>

* fixes warning on sonar analysis (hyperledger#8193)

Signed-off-by: jflo <[email protected]>

* add spotless to PR checklist (hyperledger#8185)

Signed-off-by: Sally MacFarlane <[email protected]>

* Reuse constant INVALID_PARAMS_ERROR_CODE for RpcErrorType.java (hyperledger#8154)

Magic number "-32602" has been defined in the super interface, so reusing that constant value.

Signed-off-by: Andrew Cheng <[email protected]>
Co-authored-by: Sally MacFarlane <[email protected]>

* Improve BLOCKHASH Short Circuit (hyperledger#8171)

* Improve BLOCKHASH Short Circuit
Signed-off-by: Danno Ferrin <[email protected]>
Co-authored-by: Justin Florentine <[email protected]>

* TransactionResult: add v for blob and pending tx (hyperledger#8196)

* add v for blob and pending tx

Signed-off-by: Sally MacFarlane <[email protected]>

* add v for blob and pending tx in tests

Signed-off-by: Sally MacFarlane <[email protected]>

* add v for blob tx in test

Signed-off-by: Sally MacFarlane <[email protected]>

---------

Signed-off-by: Sally MacFarlane <[email protected]>

* Prague t8n and referenceTests fixes for EEST (hyperledger#8167)

Fix referenceTests bug with fallback EMPTY Address.
T8n support for excessBlobGas for both state tests and blockchain test. State tests should use the given excessBlobGas rather than computing from the parent header.

Signed-off-by: Simon Dudley <[email protected]>

* disable fast sync actions test

Signed-off-by: Sally MacFarlane <[email protected]>

---------

Signed-off-by: jflo <[email protected]>
Signed-off-by: Gabriel-Trintinalia <[email protected]>
Signed-off-by: Karim Taam <[email protected]>
Signed-off-by: Fabio Di Fabio <[email protected]>
Signed-off-by: Simon Dudley <[email protected]>
Signed-off-by: Sally MacFarlane <[email protected]>
Signed-off-by: Andrew Cheng <[email protected]>
Co-authored-by: Justin Florentine <[email protected]>
Co-authored-by: Gabriel-Trintinalia <[email protected]>
Co-authored-by: Karim Taam <[email protected]>
Co-authored-by: Fabio Di Fabio <[email protected]>
Co-authored-by: Simon Dudley <[email protected]>
Co-authored-by: Andrew Cheng <[email protected]>
Co-authored-by: Danno Ferrin <[email protected]>
pullurib pushed a commit to pullurib/besu that referenced this pull request Feb 6, 2025
…er#8184)

This means you don't need to have the block already in the chain when you call debug_traceBlock with a block's RLP (you just need the parent).

Signed-off-by: Simon Dudley <[email protected]>
Signed-off-by: Bhanu Pulluri <[email protected]>
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.

3 participants