fix: stop verifying a batch once a proof is rejected - #2297
Open
0xrlawrence wants to merge 1 commit into
Open
Conversation
ProcessNewBatchLogV3 returns as soon as a proof fails, but the worker goroutines kept verifying the rest of the batch, which is the expensive part. Close an abandon channel on the failure path so workers stop picking up further proofs. Two related cleanups in the same function: - results is sized from the batch length, so a zero-length batch made it unbuffered. Reject empty batches up front instead. - The DisabledVerifiers error path pushed a value into results and then returned, so nothing could ever read it. That send was both dead and, with an unbuffered results, the one place that could have blocked forever. Drop it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Description
ProcessNewBatchLogV3returns as soon as any proof fails, but the worker goroutines keep verifying the rest of the batch — which is the expensive part, and on a large batch can occupy every worker for a long time after the outcome is already decided. This closes anabandonchannel on the failure path so workers stop picking up further proofs.Two related cleanups in the same function:
resultsis sized from the batch length, so a zero-length batch leaves it unbuffered. Empty batches are rejected up front instead. This is currently unreachable — the merkle check rejects an empty batch before this point — but the sizing is fragile enough to be worth making explicit.DisabledVerifierserror path didresults <- falseand then returned. Nothing can ever read that value, since the only reader is the loop below which is never reached. It was both dead and, with an unbufferedresults, the one statement that could have blocked forever. Removed.Worth noting for review:
jobsis buffered to the full batch length, so a worker returning early cannot block the producer.Type of change
Checklist
testnet, everything else tostagingVerified with
go vet ./operator/pkg/andgofmtonstaging.