fix(consensus): return Result from IncrementalProofState::take instead of panicking#726
Open
keanji-x wants to merge 1 commit into
Open
fix(consensus): return Result from IncrementalProofState::take instead of panicking#726keanji-x wants to merge 1 commit into
keanji-x wants to merge 1 commit into
Conversation
…d of panicking Closes #67 (gravity-audit). When BLS signature aggregation fails (e.g. after a validator-set rotation that invalidates some collected per-signer keys), the old code hit `unreachable!()` and crashed the `ProofCoordinator` tokio task, stalling consensus on the affected node. This is a minimal surgical fix that mirrors the relevant slice of upstream aptos-core PR #14644: - Change `IncrementalProofState::take` to return `Result<ProofOfStore, SignedBatchInfoError>`. - On aggregation failure, log via `error!` and return `SignedBatchInfoError::UnableToAggregate` (new variant) instead of panicking. Leave `completed = false` so a future attempt can retry. - Propagate `?` at the single caller in `aggregate_and_verify`. Co-Authored-By: Claude Opus 4.7 (1M context) <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.
Summary
unreachable!()inIncrementalProofState::takewith a realResultreturn so theProofCoordinatortask survives a BLS aggregation failure (e.g. after validator-set rotation) instead of crashing and halting consensus.SignedBatchInfoError::UnableToAggregateand propagates?at the single caller.Why minimal (not a full backport of upstream #14644)
Upstream aptos-core PR #14644 fixes the same panic but as part of a much larger refactor (~466 lines of conflicts against our vendored consensus). This PR ports only the panic→Result slice; we can decide later whether to take the rest of the refactor.
Test plan
cargo check -p aptos-consensus --testspasses.IncrementalProofState::take(there is only one, inaggregate_and_verify).🤖 Generated with Claude Code