Repository navigation
Prune the pool after a reconfig when the config is installed late - #729
Merged
Merged
Conversation
pfi79
previously approved these changes
Oct 22, 2026
pfi79
force-pushed
the
prune-after-async-reconfig
branch
from
October 22, 2026 13:34
dd76d7d to
a725f87
Compare
HagarMeir
force-pushed
the
prune-after-async-reconfig
branch
from
October 22, 2026 15:01
a725f87 to
50130f9
Compare
Consensus.reconfig prunes the request pool and then starts the new controller, and Controller.Start stored the verification sequence the Verifier reported at that moment. If the application installs the new config asynchronously and the switch lands during the prune or before Controller.Start, the prune used the old config while the controller stored the new sequence. MaybePruneRevokedRequests then never saw a change, and requests that are invalid under the new config stayed in the pool. Read the verification sequence before the prune in reconfig and start the controller with it, so a switch that lands later is detected and pruned on the next decision, sync or view change. The initial Start passes the Verifier's current sequence, as before. The test app can now delay revoking clients until the prune during reconfig, and TestPoolPrunedAfterAsyncReconfig checks that the next decision removes the revoked request. Fixes hyperledger#728 Written by Claude (AI assistant) on behalf of Hagar Meir. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Hagar Meir <hagar.meir@ibm.com>
HagarMeir
force-pushed
the
prune-after-async-reconfig
branch
from
October 22, 2026 15:10
50130f9 to
a87102a
Compare
pfi79
approved these changes
Oct 22, 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 #728.
Problem
Consensus.reconfigprunes the request pool, then starts the new controller.Controller.Startstored the verification sequence theVerifierreported at that moment. If the application installs the new config asynchronously and the switch lands during the prune or beforeController.Start, the prune ran against the old config while the controller stored the new sequence.MaybePruneRevokedRequeststhen never saw a change, so requests that are invalid under the new config stayed in the pool.Change
Consensus.reconfigreadsVerificationSequence()beforePool.Pruneand passes it throughstartComponentstoController.Start.Controller.Starttakes the sequence as a parameter instead of reading theVerifier. A switch that lands during or after the prune is then detected, and the pool is pruned on the next decision, sync or view change.Consensus.Startpasses theVerifier's current sequence, so its behaviour is unchanged.Controlleris ininternal/, so the new parameter changes no public API.Test
revokeDuringPruneoption. It delays revoking clients after a reconfig is delivered until the nextVerifyRequest, which is the prune inreconfig.TestPoolPrunedAfterAsyncReconfig: a follower holds a request from a client that the reconfig revokes. The test checks that the request is still in the pool after the reconfig, and that the next decision removes it.Controller.Start, the new test fails because the request is never removed.Run locally: golangci-lint (pinned version, 0 issues),
go vet ./..., andgo test -race -count=1 ./..., including the integration suite.Written by Claude (AI assistant) on behalf of Hagar Meir.
🤖 Generated with Claude Code