-
Notifications
You must be signed in to change notification settings - Fork 3.6k
op-service,op-supervisor: Correctly detect ethereum.NotFound in RPCs #16555
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #16555 +/- ##
============================================
- Coverage 77.18% 0 -77.19%
============================================
Files 166 0 -166
Lines 9507 0 -9507
============================================
- Hits 7338 0 -7338
+ Misses 2019 0 -2019
+ Partials 150 0 -150
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
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.
The EthClient.payloadCall
, EthClient.headerCall
, EthClient.blockCall
inner methods should be getting this error check. The methods that were changed here just wrap those 3 inner methods. If the inner methods handle the error, then we don't have to repeat it in every usage. Also see other comment about the existing nil checks (the standard way of communicating not-found in RPC for blocks). But these 3 methods miss the error inspection when the error is non-nil.
c95ff94
to
d8f9519
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.
Very nice
Description
The
ethereum.NotFound
error can be lost across RPC boundaries. This PR fixes our L2 source and Interop RPC clients to heuristically detect this error and forward it internally accordingly. This is important because this error is often treated in a special way. The heuristic is isolated into a new functioneth.MaybeAsNotFoundErr(err)
for reuse.Tests
Unit tests added for the new
MaybeAsNotFoundErr
function.Additional context
Without this, Interop resets don't work because the bisection checks for this error but the RPC client didn't produce it correctly.