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

feat(rpc-types-trace): always serialize result if no error #1258

Merged
merged 4 commits into from
Sep 9, 2024

Conversation

jsvisa
Copy link
Contributor

@jsvisa jsvisa commented Sep 9, 2024

Motivation

I'm checking with reth's trace results with erigon's, seems erigon will always return result:null if no error occurred:

image

And found the OG OpenEthereum's serialized logic as below:

https://github.com/openethereum/openethereum/blob/main/crates/rpc/src/v1/types/trace.rs#L532

  match self.result {
        Res::Call(ref call) => struc.serialize_field("result", call)?,
        Res::Create(ref create) => struc.serialize_field("result", create)?,
        Res::FailedCall(ref error) => struc.serialize_field("error", &error.to_string())?,
        Res::FailedCreate(ref error) => struc.serialize_field("error", &error.to_string())?,
        Res::None => struc.serialize_field("result", &None as &Option<u8>)?,
    }

Solution

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

Signed-off-by: jsvisa <delweng@gmail.com>
Signed-off-by: jsvisa <delweng@gmail.com>
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.

is this causing issues?

do you know when this can be null?

@jsvisa
Copy link
Contributor Author

jsvisa commented Sep 9, 2024

is this causing issues?

Not really, I'm comparing the trace results between reth and erigon, and then found this issue(see the diff image)

do you know when this can be null?

suicide's results were null

after thinking twice, I think we can keep the same format as erigon's, result always present, error omitted if none https://github.com/erigontech/erigon/blob/13b4b7768485736e54ff5ca3270ebeec5c023ba8/turbo/jsonrpc/trace_types.go#L62-L63

type ParityTrace struct {
...
	Error               string       `json:"error,omitempty"`
	Result              interface{}  `json:"result"`
...
}

@mattsse WDYT

Signed-off-by: jsvisa <delweng@gmail.com>
@mattsse
Copy link
Member

mattsse commented Sep 9, 2024

I see, then this makes sense

@mattsse mattsse merged commit 7b487e4 into alloy-rs:main Sep 9, 2024
30 checks passed
@jsvisa jsvisa deleted the parity-result-as-null branch September 10, 2024 03:48
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.

2 participants