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: make Parity TraceResults output optional #1102

Merged
merged 2 commits into from
Jul 26, 2024

Conversation

m1stoyanov
Copy link
Contributor

That will resolve the issue when the RPC server returns "output": null, as Nethermind does.

Motivation

That will resolve the issue when the RPC server returns "output": null, as Nethermind does.

Solution

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

That will resolve the issue when the RPC server returns "output": null, as Nethermind does.
@DaniPopes DaniPopes changed the title Update parity.rs Bytes to Option<Bytes> fix: make Parity TraceResults output optional Jul 25, 2024
},
"trace": [],
"vmTrace": null,
"transactionHash": "0xe56a5e7455c45b1842b35dbcab9d024b21870ee59820525091e183b573b4f9eb"
Copy link
Member

Choose a reason for hiding this comment

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

is this a real transaction?
wasn't able to find it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines +826 to +828
fn test_nethermind_trace_result_null_output_value() {
let reference_data = r#"{
"output": null,
Copy link
Member

Choose a reason for hiding this comment

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

is there a nethermind issue for this, I want to understand when this can be null to rule out something we may have missed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be honest, I didn't dig very deeply, but it seems that vm_trace can also be null.

Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

looking at the og docs
https://openethereum.github.io/JSONRPC-trace-module#trace_transaction
it looks like output should always be there

so treating null as empty makes sense

@DaniPopes DaniPopes merged commit fa01c2a into alloy-rs:main Jul 26, 2024
22 checks passed
ben186 pushed a commit to ben186/alloy that referenced this pull request Jul 27, 2024
* Update parity.rs Bytes to Option<Bytes>

That will resolve the issue when the RPC server returns "output": null, as Nethermind does.

* Use null_as_default for handling null output and added test
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