Repository navigation
Do not restore a proposal that a sync already delivered - #753
Merged
pfi79 merged 1 commit intoNov 4, 2026
Merged
Conversation
recoverProposed restored the trailing pre-prepare as in flight without comparing its sequence with the sequence the view starts at. A sync delivers decisions without writing to the WAL, so a node that persisted a pre-prepare and then caught up by syncing could restart with a proposal it had already delivered. The restored view re-broadcast a prepare for it and rejected every prepare of the next sequence as a wrong digest, so the node missed that sequence and caught up only through a later sync. Skip the record when the view already starts after it, as recoverPrepared and the view-record path do. TestRestartKeepsViewAndInFlightProposal restarted with a sync, which delivered the very proposal the test expects to find in flight. Restart it without a sync, as its description intends. Fixes hyperledger#752 Written by Claude (AI assistant) on behalf of Hagar Meir. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Hagar Meir <hagar.meir@ibm.com>
pfi79
approved these changes
Nov 4, 2026
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.
Fixes #752.
Problem
recoverProposedrestored the trailing pre-prepare as in flight without checking its sequence. A sync delivers decisions without writing to the WAL, so a node that persisted a pre-prepare and then caught up by syncing can restart with a WAL that ends with a proposal it has already delivered.The restored view then re-broadcasts a prepare for that proposal and rejects every prepare of the next sequence as a wrong digest. The node misses that sequence and catches up only through a later sync. No wrong value is delivered.
recoverPreparedand the view-record path from #748 already skip a proposal below the sequence the view starts at;recoverProposedwas the only restore path without that check.Change
recoverProposedskips the record when the view already starts after it, with the same check and log line as the other two paths.TestRestartKeepsViewAndInFlightProposalrestarted with a sync, which delivered the very proposal the test expects to find in flight. It now restarts without a sync, as its description intends. As Restored in-flight proposal below the checkpoint causes wrong-digest rejections after a restart #752 points out, its other assertions are unaffected.Tests
TestStateRestore: new case "proposed and already committed".TestRestartDoesNotRestoreSyncedProposal, based on the reproduction in Restored in-flight proposal below the checkpoint causes wrong-digest rejections after a restart #752 (thanks @s-xoxo): a node persists the pre-prepare of sequence 1, syncs to sequence 2, restarts, and must decide sequence 3 by consensus without rejecting prepares.Both fail without the change to
recoverProposed.Run locally: golangci-lint (pinned version, 0 issues),
go vet ./...,go test -race -count=1 ./...including the integration suite, and 10 runs each of the new integration test (with-race),TestTryCommittedSequenceTwiceandTestRestartKeepsViewAndInFlightProposal.Written by Claude (AI assistant) on behalf of Hagar Meir.
🤖 Generated with Claude Code