Repository navigation
Fix support for speculative execution response parsing - #172
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Update types.Message to map entity_addr to key.EntityAddr and add coverage for the producer address.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Adds Casper 1.x/2.x speculative execution response parsing with regression fixtures and tests.
Changes:
- Adds Casper 2.x execution-result modeling and conversion.
- Implements version-aware response unmarshalling.
- Adds success and failure fixtures for both protocol versions.
- Remaining issue: message producer addresses still use
hash_addrinstead ofentity_addr.
| File | Summary |
|---|---|
types/execution_result.go |
Defines Casper 2.x execution results and conversion logic. |
tests/rpc/speculative_exec_test.go |
Tests speculative execution parsing. |
tests/data/rpc_response/specexec_transfer_success_v158.json |
Casper 1.x success fixture. |
tests/data/rpc_response/specexec_transfer_failure_v200.json |
Casper 2.x failure fixture. |
tests/data/rpc_response/specexec_transfer_failure_v158.json |
Casper 1.x failure fixture. |
tests/data/rpc_response/specexec_cep18transfer_success_v200.json |
Casper 2.x success fixture. |
rpc/response.go |
Parses Casper 1.x and 2.x response shapes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| ApiVersion string `json:"api_version"` | ||
| BlockHash key.Hash `json:"block_hash"` | ||
| ExecutionResult types.ExecutionResult `json:"execution_result"` | ||
| Messages []types.Message `json:"messages"` |
There was a problem hiding this comment.
Addressed in 4dbd5ed: types.Message now unmarshals entity_addr into key.EntityAddr, and the speculative execution test asserts the parsed producer address.
There was a problem hiding this comment.
Btw, messages is nested inside execution_result, but the public Messages field is declared at the top level. So UnmarshalJSON handles reading correctly, but json.Marshal(SpeculativeExecResult) will produce a different structure: messages will be moved to the top level, is it expected? Should we overwrite MarshalJSON?
There was a problem hiding this comment.
You're right. I guess the same happens with BlockHash. Since SpeculativeExecResult is not a native type, I wouldn't overwrite MarshalJSON.
| TopicNameHash key.Hash `json:"topic_name_hash"` | ||
| // The identity of the entity that produced the message. | ||
| HashAddr key.Hash `json:"hash_addr"` | ||
| EntityAddr key.EntityAddr `json:"entity_addr"` |
There was a problem hiding this comment.
Isn't it a breaking change?
There was a problem hiding this comment.
It's been entity_addr since rc-8
ZhmakAS
left a comment
ZhmakAS
left a comment
There was a problem hiding this comment.
Overall, lgtm, only left couple of questions
| ApiVersion string `json:"api_version"` | ||
| BlockHash key.Hash `json:"block_hash"` | ||
| ExecutionResult types.ExecutionResult `json:"execution_result"` | ||
| Messages []types.Message `json:"messages"` |
There was a problem hiding this comment.
Btw, messages is nested inside execution_result, but the public Messages field is declared at the top level. So UnmarshalJSON handles reading correctly, but json.Marshal(SpeculativeExecResult) will produce a different structure: messages will be moved to the top level, is it expected? Should we overwrite MarshalJSON?
… nodes. Signed-off-by: David Hernando <david.hernando@make.services>
…ease-2.0.0-rc8) Signed-off-by: David Hernando <david.hernando@make.services>
4dbd5ed to
103cd0b
Compare
* Fix speculative exec and message parsing for Casper 2.0 nodes Message read the pre-rc8 hash_addr field, which 2.0 nodes replaced with entity_addr, so every message in a speculative result or a TransactionProcessed event failed to parse. SpeculativeExecResult took apiVersion from the JSON-RPC envelope and missed the 1.5 block hash, which sits beside execution_result rather than inside it. Ports make-software/casper-go-sdk#172, with its node responses as fixtures. Moves the 5.2.0 release date to 2026-10-05. * Move the Message.entityAddr changelog entry under Fixed The rename exists only to make messages from a 2.x node parse, so it reads as a fix. The release intro now points to Fixed for it.
Summary
The SpeculativeExecutionResult class failed to parse responses from Casper 2.0.0 nodes.
The Message class incorrectly contained a HashAddr property that must be EntityAddr property (changed in release-2.0.0-rc8`.
TODO
Checklist