feat(coordinator): add debug_executionWitness JSON-RPC client#3248
feat(coordinator): add debug_executionWitness JSON-RPC client#3248gauravahuja wants to merge 4 commits into
Conversation
Introduce ExecutionWitnessClient in jvm-libs with a coordinator implementation that calls Besu's debug_executionWitness RPC. Extend BlockParameter with BlockHash for hash-based block references aligned with Besu's string param parsing.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3248 +/- ##
=============================================
+ Coverage 55.41% 76.75% +21.33%
- Complexity 5264 7010 +1746
=============================================
Files 1126 1130 +4
Lines 44644 44745 +101
Branches 5356 5372 +16
=============================================
+ Hits 24741 34344 +9603
+ Misses 19182 9023 -10159
- Partials 721 1378 +657
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
| require(hash.size == 32) { "block hash must be 32 bytes, got ${hash.size}" } | ||
| } | ||
|
|
||
| fun getHash(): ByteArray = hash |
There was a problem hiding this comment.
BlockHash.getHash() exposes mutable internal array
Medium Severity
BlockHash.getHash() returns the internal ByteArray reference directly without a defensive copy. Since equals and hashCode depend on the array contents, any caller mutating the returned array silently corrupts the BlockHash instance — breaking map lookups, set membership, and equality checks. This is especially risky because BlockHash is used as a Map key in FakeExecutionWitnessClient.
Triggered by project rule: Hardhat signer UI (browser signing)
Reviewed by Cursor Bugbot for commit ab8aa98. Configure here.
There was a problem hiding this comment.
val a = BlockParameter.fromHash(ByteArray(32) { 7 })
val b = a.getHash()
println("BEFORE a: ${a.getHash().encodeHex()}")
println("BEFORE b: ${b.encodeHex()}")
for (i in b.indices) {
b[i] = (b[i] + 1).toByte()
}
println("AFTER a: ${a.getHash().encodeHex()}")
println("AFTER b: ${b.encodeHex()}")
Output:
BEFORE a: 0x0707070707070707070707070707070707070707070707070707070707070707
BEFORE b: 0x0707070707070707070707070707070707070707070707070707070707070707
AFTER a: 0x0808080808080808080808080808080808080808080808080808080808080808
AFTER b: 0x0808080808080808080808080808080808080808080808080808080808080808
| val hexBody = normalized.drop(2) | ||
| if (hexBody.length == BLOCK_HASH_HEX_LENGTH) { | ||
| return BlockHash("0x$hexBody".decodeHex()) | ||
| } |
There was a problem hiding this comment.
Parse ambiguity: valid hex number misidentified as hash
Medium Severity
BlockParameter.parse treats any 64-character hex string as a BlockHash, but a 64-hex-char string is also a valid 256-bit hex number. For instance, "0x0000000000000000000000000000000000000000000000000000000000000001" parses as a BlockHash rather than BlockNumber(1). Callers using parse for block numbers from sources that zero-pad to 32 bytes will silently get the wrong BlockParameter variant, leading to UnsupportedOperationException when getNumber() is called.
Reviewed by Cursor Bugbot for commit ab8aa98. Configure here.
There was a problem hiding this comment.
This comment seems relevant and the implementation can be simplified as well IMHO
There was a problem hiding this comment.
The comment while theoretically true is not practical. We don't pad numbers to 32 bytes. If we happen to use padded numbers, it will not be possible to differentiate if it is a block number or block hash.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0464554. Configure here.
| private fun byteArrayListHashCode(list: List<ByteArray>): Int { | ||
| return list.fold(0) { acc, bytes -> 31 * acc + bytes.contentHashCode() } | ||
| } | ||
| } |
There was a problem hiding this comment.
ExecutionWitness data class missing toString() override
Low Severity
ExecutionWitness is a data class with List<ByteArray> fields but does not override toString(). The auto-generated data class toString() will produce unreadable output like ExecutionWitness(state=[[B@1a2b, [B@3c4d], ...) since ByteArray uses identity-based toString(). This violates the Kotlin/Java checklist item requiring toString() overrides on data classes with ByteArray fields.
Triggered by project rule: Bugbot Review Instructions
Reviewed by Cursor Bugbot for commit 0464554. Configure here.
| @@ -0,0 +1,41 @@ | |||
| package linea.executionwitness | |||
There was a problem hiding this comment.
| return result | ||
| } | ||
|
|
||
| private fun byteArrayListsEqual(a: List<ByteArray>, b: List<ByteArray>): Boolean { |
There was a problem hiding this comment.
IIRC, I have already seen something with same intent elsewhere before. Maybe this shall be in ByteArrayExtensions.kt ?
There was a problem hiding this comment.
FYI byteArrayListEquals in jvm-libs/generic/extensions/kotlin/src/main/kotlin/linea/kotlin/ListExtensions.kt
| interface ExecutionWitnessClient { | ||
| fun getExecutionWitness( | ||
| block: BlockParameter, | ||
| ): SafeFuture<Result<ExecutionWitness, ErrorResponse<ExecutionWitnessError>>> |
There was a problem hiding this comment.
I suggest we return the value directly instead of the a Result monad. For errors, you can use the JsonRpcErrorResponseException
| ): SafeFuture<Result<ExecutionWitness, ErrorResponse<ExecutionWitnessError>>> | |
| ): SafeFuture<ExecutionWitness> |
|
|
||
| class FakeExecutionWitnessClient( | ||
| private val witnessesByBlock: Map<BlockParameter, ExecutionWitness> = emptyMap(), | ||
| ) : ExecutionWitnessClient { |
There was a problem hiding this comment.
What use cases do you envision for this isolated class? Why not part of FakeEthApiClient with auto-generation of fake execution witness ?
| val hexBody = normalized.drop(2) | ||
| if (hexBody.length == BLOCK_HASH_HEX_LENGTH) { | ||
| return BlockHash("0x$hexBody".decodeHex()) | ||
| } |
There was a problem hiding this comment.
This comment seems relevant and the implementation can be simplified as well IMHO
There was a problem hiding this comment.
|
|
||
| constructor( | ||
| vertx: Vertx, | ||
| rpcClient: JsonRpcClient, |
There was a problem hiding this comment.
use JsonRpcV2Client instead. V1 is meant to be deprecated/removed once Shomei and Traces API get removed.
| } | ||
|
|
||
| enum class ExecutionWitnessError { | ||
| NULL_RESULT, |
There was a problem hiding this comment.
@gauravahuja how does "NULL_RESULT" occurs? like a server side failure where the response somehow was null? what scenario from Besu could that happen? e.g. the target block number is in the future?
There was a problem hiding this comment.
There are cases where the server returns null https://github.com/Consensys/besu-zkevm-plugin/blob/main/src/main/java/net/consensys/stateless/rpc/methods/DebugExecutionWitnessServer.java#L104
| val b = BlockParameter.fromHash(hashBytes.copyOf()) | ||
| assertThat(a).isEqualTo(b) | ||
| assertThat(a.hashCode()).isEqualTo(b.hashCode()) | ||
| assertThat(BlockParameter.fromHash(ByteArray(32) { 8 })).isNotEqualTo(a) |
There was a problem hiding this comment.
I don't see how the above assertion related to "content-based equality"
|
|
||
| private val sampleWitnessJson = """ | ||
| { | ||
| "state": ["f902"], |
There was a problem hiding this comment.
Is it without the 0x prefix?
Not according to the WIP source code https://github.com/besu-eth/besu/pull/10571/changes#diff-ba66fa4b06a2fc76aedd7b2e413873adf2ba9cd9ad99050c4cef5eaea75f3001R29
| } | ||
|
|
||
| @Test | ||
| fun `getExecutionWitness returns NULL_RESULT when result is null`() { |
There was a problem hiding this comment.
Is it speculative or an actually possible response in case a block is unavailable?


Introduce ExecutionWitnessClient in jvm-libs with a coordinator implementation that calls Besu's debug_executionWitness RPC. Extend BlockParameter with BlockHash for hash-based block references aligned with Besu's string param parsing.
This PR implements issue(s) #3085
Checklist
PR.
Note
Medium Risk
Shared BlockParameter gains a new variant with explicit UnsupportedOperationException on some code paths; otherwise additive client and RPC parsing with good test coverage.
Overview
Adds
ExecutionWitnessClientand anExecutionWitnessmodel injvm-libs, plus a newcoordinator:clients:execution-witness-clientmodule that calls Besu’sdebug_executionWitnessJSON-RPC (with retries, hex parsing, and typed errors for null results, RPC failures, and parse errors).Extends
BlockParameterwithBlockHash(fromHash,parsefor 64-hex0x…strings) and wires hash-aware behavior where needed: RPC param encoding for the witness client, in-process state recovery header lookup by hash, and fake ETH/witness test clients.getBlockParameterNumberandtoWeb3j()now fail fast on block hash because those stacks don’t resolve hashes the same way.Includes WireMock tests for the JSON-RPC client and expanded
BlockParameter/ web3j mapping tests; registers the new Gradle subproject.Reviewed by Cursor Bugbot for commit 0464554. Bugbot is set up for automated code reviews on this repo. Configure here.