Repository navigation
Conversation
DAClient.GetInput panicked when the proxy's 418 body had a StatusCode missing or outside 1-4, crash-looping every op-node deriving that cert. It is now an ordinary error, which derivation retries instead of dropping.
GetInput returned on 404 and other error statuses without closing the response body, which kept the connection to the DA server open.
A fetch that keeps failing stalls derivation with only a log line, so DA.GetInput now records it in the storage_errors metric, which nothing used. It also matches ErrNotFound when the storage wraps it.
The proxy's message for a cert it fails to parse carries the cert in hex, so a body over the 5000-byte cap was cut off, failed to decode, and was retried forever instead of dropping the commitment.
A drop's Msg can hold a whole cert in hex and is logged twice per dropped commitment, so the error text shows its first 1000 bytes only.
palango
changed the title
palango
changed the base branch from
palango/celo-rebase-19
to
celo-rebase-19
October 13, 2026 12:34
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
op-node mishandles two kinds of 418 response from the EigenDA proxy. A body whose
StatusCodeis missing or outside 1-4 makes it panic, which crash-loops every node deriving that commitment. A body over 5000 bytes gets cut off, fails to decode, and is retried forever when the commitment should be dropped. With this change op-node retries the first kind as a temporary error and reads bodies up to 2 MiB, so the second kind drops as intended. The 418 handling is Celo's EigenDA patch; upstream has none.The long body isn't exotic. For a cert it can't parse, the proxy puts the whole cert in hex into the message (
NewCertParsingFailedErrorin Layr-Labs/eigenda), so any unparseable commitment over roughly 2.4 KB hit the old cap. I read that in the current eigenda source and haven't checked it against the proxy version we run. A body over 2 MiB still isn't read and still retries, now with an error that says so. That needs a commitment over about 1 MiB, which only the batcher can post.For operators:
failed to get preimageon each retry. Before, it restarted in a loop.storage_errorsnow counts failed fetches. The metric was registered in op-node but nothing recorded to it, so a fetch that keeps failing was visible only in logs. Drops and not-found don't count.Two smaller fixes ride along, both in code that's the same upstream:
GetInputnow closes the response body on 404 and other error statuses, andDA.GetInputmatchesErrNotFoundwhen a storage wraps it (theerrors.Isarguments were swapped). No storage in the repo wraps it today, so that one changes nothing in practice.Codex flagged the panic on #541 (#541 (comment)); the rest came out of reviewing that fix. There's no issue. The base is
palango/celo-rebase-19becausecelo-rebase-19isn't published yet.The go-code-reviewer pass ran twice and found nothing blocking. Dismissed from it: a
nullbody test case, which takes the same branch as{}; skippingcontext.Canceledin the metric, since a cancelled fetch on shutdown adds one to a counter; and reading onlyStatusCodefrom the body so the message length stops mattering, which I'd rather not hand-parse in the path that decides a drop. I ran theop-alt-daandop-node/rollup/derivetests locally but didn't build the op-node binary, so that's left to CI. That a malformed 418 ends in a retry (op-node/rollup/derive/altda_data_source.go:105-113) I checked by reading the code; no test drives it through the pipeline.TestFakeDAServer_OutOfOrderResponseshas a data race under-racethat this PR doesn't touch.