-
Notifications
You must be signed in to change notification settings - Fork 831
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
feat(rpc): implement debug_executionWitness API #397
feat(rpc): implement debug_executionWitness API #397
Conversation
9d71866
to
93214fb
Compare
93214fb
to
6ae6ba3
Compare
Adding myself as reviewer so I don't forget this after the morning meeting run but no need to block on me if someone else gets to this first. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good generally but would be worth adding a test for it. @refcell or @clabby does the returned format match what you'd expect?
There's also a comment saying the format might be simplified in the future - should be consider doing that now so we don't have to break compatibility? I'm not too sure what's actually likely to get standardised here.
regarding format, I believe so, I compared returns from reth & geth, and also the queries from op-program, they match. Currently trying to figure out return mismatch between reth & geth, working with reth teams on that |
@ajsutton, also regarding standardization, currently the conversation is around two things:
Added tests and ready for another review |
612e482
to
442b152
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Given the benefit to fault proofs I think this is worth having in op-geth now, but would be good to submit this to upstream as well since it's a generically useful API.
Thanks @ajsutton, haven't done it now because there's still unanswered questions regarding return format and I sense resistance from geth side, but will try it to see how geth team reacts |
Yeah, I'd say it's worth creating the PR to upstream to push the conversation forward. |
Description
implement debug_executionWitness API
Tests
Tested offline with reth and op-program
Additional context
Metadata