From 2baf190306b5aeba3d81256e8a2d2ef14cfcb96c Mon Sep 17 00:00:00 2001 From: samliok Date: Tue, 13 Oct 2026 16:59:50 -0400 Subject: [PATCH 1/5] ensure non-ancestors are pruned when finalized --- simplex/epoch.go | 48 +++++- simplex/epoch_test.go | 330 +++++++++++++++++++++++++++++++++++++++ simplex/recovery_test.go | 1 + 3 files changed, 372 insertions(+), 7 deletions(-) diff --git a/simplex/epoch.go b/simplex/epoch.go index 2afda697..77da3efa 100644 --- a/simplex/epoch.go +++ b/simplex/epoch.go @@ -550,6 +550,7 @@ func (e *Epoch) loadFinalizationRecord(r []byte) error { if err != nil { return err } + e.pruneToFinalizedChain(finalization) e.Logger.Info("Finalization Recovered From WAL", zap.Uint64("Round", finalization.Finalization.Round)) return nil } @@ -1342,7 +1343,7 @@ func (e *Epoch) persistFinalization(finalization common.Finalization) error { e.blockVerificationScheduler.ExecuteBlockDependents(finalization.Finalization.Digest) e.blockVerificationScheduler.RemoveOldTasks(finalization.Finalization.Seq) e.replicationState.clearBlockDependencyTasks(finalization.Finalization.Digest, finalization.Finalization.Seq, true) - e.deleteOldEmptyVotes(finalization.Finalization.Round) + e.pruneToFinalizedChain(finalization) if finalization.Finalization.Seq == nextSeqToCommit { if err := e.indexFinalizations(finalization.Finalization.Round); err != nil { @@ -1772,7 +1773,7 @@ func (e *Epoch) handleEmptyNotarizationMessage(emptyNotarization *common.EmptyNo e.Logger.Verbo("Received empty notarization message", zap.Uint64("round", vote.Round)) - if e.isVoteForFinalizedRound(vote.Round) { + if e.isBelowHighestFinalizedRound(vote.Round) { e.Logger.Debug("Received an empty notarization for a too low round", zap.Uint64("round", vote.Round), zap.Uint64("our round", e.round)) @@ -1808,7 +1809,7 @@ func (e *Epoch) handleEmptyNotarizationMessage(emptyNotarization *common.EmptyNo } // we do not care to process votes for rounds where we have finalized -func (e *Epoch) isVoteForFinalizedRound(round uint64) bool { +func (e *Epoch) isBelowHighestFinalizedRound(round uint64) bool { max := uint64(0) for _, round := range e.rounds { @@ -1834,7 +1835,7 @@ func (e *Epoch) handleNotarizationMessage(message *common.Notarization, from com e.Logger.Verbo("Received notarization message", zap.Stringer("from", from), zap.Uint64("round", vote.Round)) - if e.isVoteForFinalizedRound(vote.Round) { + if e.isBelowHighestFinalizedRound(vote.Round) { return nil } @@ -2232,6 +2233,12 @@ func (e *Epoch) createBlockVerificationTask(block common.Block, from common.Node return md.Digest } + // A higher round was finalized while we were verifying, so this block can no longer be finalized. + if e.isBelowHighestFinalizedRound(md.Round) { + e.Logger.Debug("Dropping proposal for a round below a finalized round", zap.Uint64("round", md.Round)) + return md.Digest + } + if !e.storeProposal(verifiedBlock) { e.Logger.Debug("Unable to store proposed block for the round", zap.Stringer("NodeID", from), zap.Uint64("round", md.Round)) return md.Digest @@ -2376,6 +2383,12 @@ func (e *Epoch) createNotarizedBlockVerificationTask(block common.Block, notariz e.lock.Lock() defer e.lock.Unlock() + // A higher round was finalized while we were verifying, so this block can no longer be finalized. + if e.isBelowHighestFinalizedRound(md.Round) { + e.Logger.Debug("Dropping notarized block for a round below a finalized round", zap.Uint64("round", md.Round)) + return md.Digest + } + // we started verifying the block when we didn't have a notarization, however its // possible we received a notarization or empty notarization for this block in the meantime. round, ok := e.rounds[md.Round] @@ -3070,9 +3083,30 @@ func (e *Epoch) deleteRounds(round uint64) { } } -func (e *Epoch) deleteOldEmptyVotes(finalizedRound uint64) { +// pruneToFinalizedChain deletes the blocks and empty votes below the finalized round that are not +// part of the finalized block's chain. Rounds strictly decrease along a chain, so a single descending +// pass from the finalized block finds all of its ancestors. +func (e *Epoch) pruneToFinalizedChain(finalization common.Finalization) { + prev := finalization.Finalization.Prev + minRound := e.minRoundInRoundsMap() + for r := finalization.Finalization.Round; r > minRound; r-- { + round, exists := e.rounds[r-1] + if !exists { + continue + } + + // don't delete this round, since the block is an ancestor of finalization + if header := round.block.BlockHeader(); header.Digest == prev { + prev = header.Prev + continue + } + + e.Logger.Debug("Pruning block not on the finalized chain", zap.Uint64("round", r-1), zap.Uint64("finalized round", finalization.Finalization.Round)) + delete(e.rounds, r-1) + } + for r := range e.emptyVotes { - if r < finalizedRound { + if r < finalization.Finalization.Round { delete(e.emptyVotes, r) } } @@ -3594,7 +3628,7 @@ func (e *Epoch) processQuorumRound(round *common.QuorumRound, from common.NodeID return nil } - if round.Finalization == nil && e.isVoteForFinalizedRound(round.GetRound()) { + if round.Finalization == nil && e.isBelowHighestFinalizedRound(round.GetRound()) { return fmt.Errorf("received a quorum round for a round that has been finalized. round: %d; seq: %d", round.GetRound(), round.GetSequence()) } diff --git a/simplex/epoch_test.go b/simplex/epoch_test.go index 40b5c9c9..256c95f8 100644 --- a/simplex/epoch_test.go +++ b/simplex/epoch_test.go @@ -1918,6 +1918,336 @@ func TestEpochRequestsEmptyRoundDependency(t *testing.T) { require.Equal(t, uint64(3), e.Metadata().Round) } +// TestEpochFinalizeVoteRebroadcastForTimedOutRound ensures that after finalizing a future sequence, +// we rebroadcast a finalize vote for a round we timed out on only if its block is an ancestor of the +// finalized block. Otherwise the vote could finalize a second block at the finalized sequence. +func TestEpochFinalizeVoteRebroadcastForTimedOutRound(t *testing.T) { + t.Run("fork", func(t *testing.T) { + testEpochFinalizeVoteRebroadcastForTimedOutRound(t, false) + }) + t.Run("ancestor", func(t *testing.T) { + testEpochFinalizeVoteRebroadcastForTimedOutRound(t, true) + }) +} + +func testEpochFinalizeVoteRebroadcastForTimedOutRound(t *testing.T, buildOnTimedOutRound bool) { + bb := testutil.NewTestBlockBuilder() + nodes := []NodeID{{1}, {2}, {3}, {4}} + ctx := context.Background() + broadcastMessages := make(chan *Message, 100) + sentMessages := make(chan *Message, 100) + comm := &recordingComm{Communication: testutil.NewNoopComm(nodes), BroadcastMessages: broadcastMessages, SentMessages: sentMessages} + conf, wal, _ := testutil.DefaultTestNodeEpochConfig(t, nodes[3], comm, bb) + conf.ReplicationEnabled = true + e, err := NewEpoch(conf) + require.NoError(t, err) + t.Cleanup(e.Stop) + + require.NoError(t, e.Start()) + + // round 0 is notarized but not finalized, so seq 0 is never committed + block0, _ := advanceRoundFromNotarization(t, e, bb) + testutil.WaitToEnterRound(t, e, 1) + + // we time out on round 1, then learn that block1 was notarized anyway + bb.BlockShouldBeBuilt <- struct{}{} + testutil.WaitForBlockProposerTimeout(t, e, &e.StartTime, 1) + + _, built := bb.BuildBlock(ctx, e.Metadata(), emptyBlacklist) + require.True(t, built) + block1 := bb.GetBuiltBlock() + + vote1, err := testutil.NewTestVote(block1, nodes[1]) + require.NoError(t, err) + require.NoError(t, e.HandleMessage(&Message{ + BlockMessage: &BlockMessage{ + Block: block1, + Vote: *vote1, + }, + }, nodes[1])) + notarization1, err := testutil.NewNotarization(e.Logger, e.SignatureAggregatorCreator(e.Comm.Validators()), block1, nodes[0:3]) + require.NoError(t, err) + testutil.InjectTestNotarization(t, e, notarization1, nodes[1]) + + require.Equal(t, NotarizationRecordType, wal.AssertNotarization(1)) + testutil.WaitToEnterRound(t, e, 2) + + // block2 either extends block1, or builds off block0 + md2 := e.Metadata() + if !buildOnTimedOutRound { + md2.Seq = 1 + md2.Prev = block0.BlockHeader().Digest + } + + _, built = bb.BuildBlock(ctx, md2, emptyBlacklist) + require.True(t, built) + block2 := bb.GetBuiltBlock() + + vote2, err := testutil.NewTestVote(block2, nodes[2]) + require.NoError(t, err) + require.NoError(t, e.HandleMessage(&Message{ + BlockMessage: &BlockMessage{ + Block: block2, + Vote: *vote2, + }, + }, nodes[2])) + + // block2 skips round 1, so we need to send an empty notarization before block2 can be verified + if !buildOnTimedOutRound { + for msg := range sentMessages { + if msg.ReplicationRequest != nil { + require.Equal(t, uint64(1), msg.ReplicationRequest.Rounds[0]) + break + } + } + + require.NoError(t, e.HandleMessage(&Message{ + ReplicationResponse: &ReplicationResponse{ + Data: []QuorumRound{{EmptyNotarization: testutil.NewEmptyNotarization(nodes, 1)}}, + }, + }, nodes[2])) + + for !wal.ContainsEmptyNotarization(1) { + time.Sleep(20 * time.Millisecond) + } + } + + testutil.InjectTestVote(t, e, block2, nodes[0]) + testutil.InjectTestVote(t, e, block2, nodes[1]) + wal.AssertNotarization(2) + + // finalizing block2 is a future sequence since seq 0 is not committed + testutil.InjectTestFinalizeVote(t, e, block2, nodes[0]) + testutil.InjectTestFinalizeVote(t, e, block2, nodes[1]) + + var sentFinalizeVoteForBlock1 bool + for msg := range broadcastMessages { + if msg.FinalizeVote != nil && msg.FinalizeVote.Finalization.Round == 1 { + sentFinalizeVoteForBlock1 = true + } + if msg.Finalization != nil && msg.Finalization.Finalization.Round == 2 { + break + } + } + + // If block2 skips round 1, we must not finalize vote block1, since both blocks would be finalized at seq 1. + // If block2 builds on block1, we must finalize vote block1, since block2 cannot be committed until block1 is. + require.Equal(t, buildOnTimedOutRound, sentFinalizeVoteForBlock1) +} + +// TestEpochFinalizeVoteRebroadcastFollowsFinalizedChain ensures we rebroadcast & create +// finalized votes only for the ancestors of the future finalizations. +// Otherwise, we may end up signing two finalize vote messages for different blocks for the same sequence. +func TestEpochFinalizeVoteRebroadcastFollowsFinalizedChain(t *testing.T) { + bb := testutil.NewTestBlockBuilder() + nodes := []NodeID{{1}, {2}, {3}, {4}} + broadcastMessages := make(chan *Message, 100) + comm := &recordingComm{Communication: testutil.NewNoopComm(nodes), BroadcastMessages: broadcastMessages} + conf, wal, _ := testutil.DefaultTestNodeEpochConfig(t, nodes[3], comm, bb) + conf.ReplicationEnabled = true + e, err := NewEpoch(conf) + require.NoError(t, err) + t.Cleanup(e.Stop) + + require.NoError(t, e.Start()) + + // build two blocks that are notarized(block0, and block1) + block0, _ := advanceRoundFromNotarization(t, e, bb) + advanceRoundFromNotarization(t, e, bb) + + // block2 skips round 1, so it needs round 1's empty notarization before it can be verified + require.NoError(t, e.HandleMessage(&Message{ + ReplicationResponse: &ReplicationResponse{ + Data: []QuorumRound{{EmptyNotarization: testutil.NewEmptyNotarization(nodes, 1)}}, + }, + }, nodes[2])) + require.True(t, wal.ContainsEmptyNotarization(1)) + + md2 := e.Metadata() + md2.Seq = 1 + md2.Prev = block0.BlockHeader().Digest + block2, _ := advanceRound(t, e, bb, true, false, &md2) + + // we lead round 3, so we build block3 on top of block2 + block3, _ := advanceRoundFromNotarization(t, e, bb) + require.Equal(t, block2.BlockHeader().Digest, block3.BlockHeader().Prev) + + for len(broadcastMessages) > 0 { + <-broadcastMessages + } + + // This gives block3 a finalization. block3 points to block2 -> block0. + // Our node also has block1 + its notarization but it is not part of the finalizations chain. + testutil.InjectTestFinalizeVote(t, e, block3, nodes[0]) + testutil.InjectTestFinalizeVote(t, e, block3, nodes[1]) + + var rebroadcastRounds []uint64 + for msg := range broadcastMessages { + if msg.FinalizeVote != nil { + rebroadcastRounds = append(rebroadcastRounds, msg.FinalizeVote.Finalization.Round) + } + if msg.Finalization != nil && msg.Finalization.Finalization.Round == 3 { + break + } + } + // block3 is finalized at a future sequence since seq 0 is not committed, which triggers the rebroadcast + require.Equal(t, uint64(0), e.Storage.NumBlocks()) + + // ensure that we don't send a finalization for round 1 + require.Equal(t, []uint64{0, 2}, rebroadcastRounds) +} + +// TestEpochPruneRestoredForkFromWal ensures a fork block restored from the WAL is never finalized. +// The WAL holds our empty vote and and a block in round 1. We then have a finalization for for block2 which +// was finalized on top of the empty round 1. This makes block1 a fork so after restarting, +// finalizing at a future sequence must not rebroadcast a finalize vote for block1. +func TestEpochPruneRestoredForkFromWal(t *testing.T) { + bb := testutil.NewTestBlockBuilder() + nodes := []NodeID{{1}, {2}, {3}, {4}} + ctx := context.Background() + quorum := Quorum(len(nodes)) + broadcastMessages := make(chan *Message, 100) + comm := &recordingComm{Communication: testutil.NewNoopComm(nodes), BroadcastMessages: broadcastMessages} + conf, wal, _ := testutil.DefaultTestNodeEpochConfig(t, nodes[0], comm, bb) + e, err := NewEpoch(conf) + require.NoError(t, err) + t.Cleanup(e.Stop) + + sigAggr := e.SignatureAggregatorCreator(conf.Comm.Validators()) + appendBlock := func(md ProtocolMetadata) VerifiedBlock { + _, built := bb.BuildBlock(ctx, md, emptyBlacklist) + require.True(t, built) + block := bb.GetBuiltBlock() + record, err := BlockRecord(block.BlockHeader(), block.Bytes()) + require.NoError(t, err) + require.NoError(t, wal.Append(record)) + notarizationRecord, err := testutil.NewNotarizationRecord(conf.Logger, sigAggr, block, nodes[0:quorum]) + require.NoError(t, err) + require.NoError(t, wal.Append(notarizationRecord)) + return block + } + + md := e.Metadata() + block0 := appendBlock(md) + + emptyVote := createEmptyVote(EmptyVoteMetadata{Round: 1, Epoch: md.Epoch}, e.ID).Vote + require.NoError(t, wal.Append(NewEmptyVoteRecord(emptyVote))) + + md.Round, md.Seq, md.Prev = 1, 1, block0.BlockHeader().Digest + appendBlock(md) + require.NoError(t, wal.Append(NewEmptyNotarizationRecord(testutil.NewEmptyNotarization(nodes[0:quorum], 1)))) + + md.Round = 2 + block2 := appendBlock(md) + _, finalizationRecord := testutil.NewFinalizationRecord(t, sigAggr, block2, nodes[0:quorum]) + require.NoError(t, wal.Append(finalizationRecord)) + + require.NoError(t, e.Start()) + require.Equal(t, uint64(3), e.Metadata().Round) + + block3, _ := advanceRoundFromNotarization(t, e, bb) + require.Equal(t, block2.BlockHeader().Digest, block3.BlockHeader().Prev) + + for len(broadcastMessages) > 0 { + <-broadcastMessages + } + + testutil.InjectTestFinalizeVote(t, e, block3, nodes[1]) + testutil.InjectTestFinalizeVote(t, e, block3, nodes[2]) + + var rebroadcastRounds []uint64 + for msg := range broadcastMessages { + if msg.FinalizeVote != nil { + rebroadcastRounds = append(rebroadcastRounds, msg.FinalizeVote.Finalization.Round) + } + if msg.Finalization != nil && msg.Finalization.Finalization.Round == 3 { + break + } + } + require.Equal(t, uint64(0), e.Storage.NumBlocks()) + + // only send a finalize vote for block0. Block2 & 3 are finalized, and block1 is a fork. + require.Equal(t, []uint64{0}, rebroadcastRounds) +} + +// TestEpochReplicatedNotarizationVerifiedAfterFinalizationIsNotFinalized ensures a replicated notarized block +// is dropped if a higher round is finalized while it is being verified. block1's verification is held while +// block2 is finalized. Because block2 is built on top of the empty round 1 at block1's sequence, we should +// not add block1 or sign a finalize vote for it once its verification completes. +func TestEpochReplicatedNotarizationVerifiedAfterFinalizationIsNotFinalized(t *testing.T) { + bb := testutil.NewTestBlockBuilder() + nodes := []NodeID{{1}, {2}, {3}, {4}} + ctx := context.Background() + broadcastMessages := make(chan *Message, 100) + comm := &recordingComm{Communication: testutil.NewNoopComm(nodes), BroadcastMessages: broadcastMessages} + conf, wal, _ := testutil.DefaultTestNodeEpochConfig(t, nodes[2], comm, bb) + conf.ReplicationEnabled = true + e, err := NewEpoch(conf) + require.NoError(t, err) + t.Cleanup(e.Stop) + + require.NoError(t, e.Start()) + + advanceRoundFromNotarization(t, e, bb) + + // we time out on round 1, which is then empty notarized + bb.BlockShouldBeBuilt <- struct{}{} + testutil.WaitForBlockProposerTimeout(t, e, &e.StartTime, 1) + require.NoError(t, e.HandleMessage(&Message{EmptyNotarization: testutil.NewEmptyNotarization(nodes, 1)}, nodes[1])) + + // we lead round 2, so we build block2 on top of block0 and the empty round 1 + block2, _ := advanceRoundFromNotarization(t, e, bb) + + // block1 is a fork of block2 in round 1 at the same sequence + md1 := block2.BlockHeader().ProtocolMetadata + md1.Round = 1 + _, built := bb.BuildBlock(ctx, md1, emptyBlacklist) + require.True(t, built) + block1 := bb.GetBuiltBlock() + block1.VerificationDelay = make(chan struct{}) + + notarization1, err := testutil.NewNotarization(e.Logger, e.SignatureAggregatorCreator(e.Comm.Validators()), block1, []NodeID{nodes[0], nodes[1], nodes[3]}) + require.NoError(t, err) + require.NoError(t, e.HandleMessage(&Message{ + ReplicationResponse: &ReplicationResponse{ + Data: []QuorumRound{{Block: block1, Notarization: ¬arization1}}, + }, + }, nodes[0])) + + testutil.InjectTestFinalizeVote(t, e, block2, nodes[0]) + testutil.InjectTestFinalizeVote(t, e, block2, nodes[1]) + for msg := range broadcastMessages { + if msg.Finalization != nil && msg.Finalization.Finalization.Round == 2 { + break + } + } + // block2 is finalized at a future sequence since seq 0 is not committed + require.Equal(t, uint64(0), e.Storage.NumBlocks()) + + close(block1.VerificationDelay) + + // To ensure block1 has finished verifying, we send our node block3. Because the verification scheduler only verifies + // one block at a time, we know that if block3 is verified, block1's verification task must have completed(it was enqueued before block3). + blacklist2 := block2.Blacklist() + _, built = bb.BuildBlock(ctx, e.Metadata(), blacklist2.ApplyUpdates(nil, 3)) + require.True(t, built) + block3 := bb.GetBuiltBlock() + vote3, err := testutil.NewTestVote(block3, nodes[3]) + require.NoError(t, err) + require.NoError(t, e.HandleMessage(&Message{ + BlockMessage: &BlockMessage{ + Block: block3, + Vote: *vote3, + }, + }, nodes[3])) + wal.AssertBlockProposal(3) + + for len(broadcastMessages) > 0 { + msg := <-broadcastMessages + require.Nil(t, msg.FinalizeVote, "signed a finalize vote for a block verified after a higher round was finalized") + } +} + func TestReplicatedNotarizationRestart(t *testing.T) { // A test that makes a notarization be replicated to a node that doesn't have the block, // and then restarts the node. The node must recover from the WAL and start successfully. diff --git a/simplex/recovery_test.go b/simplex/recovery_test.go index 26dab64c..5dfb609a 100644 --- a/simplex/recovery_test.go +++ b/simplex/recovery_test.go @@ -441,6 +441,7 @@ func TestRecoverFromMultipleNotarizations(t *testing.T) { protocolMetadata.Round = 1 protocolMetadata.Seq = 1 + protocolMetadata.Prev = firstBlock.BlockHeader().Digest secondBlock, ok := bb.BuildBlock(ctx, protocolMetadata, emptyBlacklist) require.True(t, ok) sBytes := secondBlock.Bytes() From f49e6c2694591a93629c855450e85980d55ae70b Mon Sep 17 00:00:00 2001 From: samliok Date: Wed, 14 Oct 2026 10:13:03 -0400 Subject: [PATCH 2/5] clearer comment --- simplex/epoch.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/simplex/epoch.go b/simplex/epoch.go index 77da3efa..f6ee1206 100644 --- a/simplex/epoch.go +++ b/simplex/epoch.go @@ -550,7 +550,7 @@ func (e *Epoch) loadFinalizationRecord(r []byte) error { if err != nil { return err } - e.pruneToFinalizedChain(finalization) + e.pruneNonFinalizedChain(finalization) e.Logger.Info("Finalization Recovered From WAL", zap.Uint64("Round", finalization.Finalization.Round)) return nil } @@ -1343,7 +1343,7 @@ func (e *Epoch) persistFinalization(finalization common.Finalization) error { e.blockVerificationScheduler.ExecuteBlockDependents(finalization.Finalization.Digest) e.blockVerificationScheduler.RemoveOldTasks(finalization.Finalization.Seq) e.replicationState.clearBlockDependencyTasks(finalization.Finalization.Digest, finalization.Finalization.Seq, true) - e.pruneToFinalizedChain(finalization) + e.pruneNonFinalizedChain(finalization) if finalization.Finalization.Seq == nextSeqToCommit { if err := e.indexFinalizations(finalization.Finalization.Round); err != nil { @@ -3083,10 +3083,10 @@ func (e *Epoch) deleteRounds(round uint64) { } } -// pruneToFinalizedChain deletes the blocks and empty votes below the finalized round that are not -// part of the finalized block's chain. Rounds strictly decrease along a chain, so a single descending -// pass from the finalized block finds all of its ancestors. -func (e *Epoch) pruneToFinalizedChain(finalization common.Finalization) { +// pruneNonFinalizedChain deletes the blocks and empty votes below the finalized round that are not +// part of the finalized block's chain. Ex. if we have blockchains b1 <- b3 <- b5, and b1 <- b2 <- b4, +// a call to pruneNonFinalizedChain(finalization b5), will prune blocks b2 & b4 from the rounds map. +func (e *Epoch) pruneNonFinalizedChain(finalization common.Finalization) { prev := finalization.Finalization.Prev minRound := e.minRoundInRoundsMap() for r := finalization.Finalization.Round; r > minRound; r-- { From 3c95cf5d7fe7797ab92dfc04250eebd2062fa90f Mon Sep 17 00:00:00 2001 From: samliok Date: Wed, 21 Oct 2026 15:32:59 -0400 Subject: [PATCH 3/5] add asci art --- simplex/epoch.go | 16 ++++++++-------- simplex/epoch_test.go | 12 ++++++++++++ 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/simplex/epoch.go b/simplex/epoch.go index dfb26763..870c7199 100644 --- a/simplex/epoch.go +++ b/simplex/epoch.go @@ -550,7 +550,7 @@ func (e *Epoch) loadFinalizationRecord(r []byte) error { if err != nil { return err } - e.pruneNonFinalizedChain(finalization) + e.pruneCompetingRounds(finalization) e.Logger.Info("Finalization Recovered From WAL", zap.Uint64("Round", finalization.Finalization.Round)) return nil } @@ -1343,7 +1343,7 @@ func (e *Epoch) persistFinalization(finalization common.Finalization) error { e.blockVerificationScheduler.ExecuteBlockDependents(finalization.Finalization.Digest) e.blockVerificationScheduler.RemoveOldTasks(finalization.Finalization.Seq) e.replicationState.clearBlockDependencyTasks(finalization.Finalization.Digest, finalization.Finalization.Seq, true) - e.pruneNonFinalizedChain(finalization) + e.pruneCompetingRounds(finalization) if finalization.Finalization.Seq == nextSeqToCommit { if err := e.indexFinalizations(finalization.Finalization.Round); err != nil { @@ -2228,7 +2228,7 @@ func (e *Epoch) createBlockVerificationTask(block common.Block, from common.Node // A higher round was finalized while we were verifying, so this block can no longer be finalized. if e.isBelowHighestFinalizedRound(md.Round) { e.Logger.Debug("Dropping proposal for a round below a finalized round", zap.Uint64("round", md.Round)) - return md.Digest + return common.Digest{} } if !e.storeProposal(verifiedBlock) { @@ -2387,7 +2387,7 @@ func (e *Epoch) createNotarizedBlockVerificationTask(block common.Block, notariz // A higher round was finalized while we were verifying, so this block can no longer be finalized. if e.isBelowHighestFinalizedRound(md.Round) { e.Logger.Debug("Dropping notarized block for a round below a finalized round", zap.Uint64("round", md.Round)) - return md.Digest + return common.Digest{} } // we started verifying the block when we didn't have a notarization, however its @@ -3085,10 +3085,10 @@ func (e *Epoch) deleteRounds(round uint64) { } } -// pruneNonFinalizedChain deletes the blocks and empty votes below the finalized round that are not +// pruneCompetingRounds deletes the blocks and empty votes below the finalized round that are not // part of the finalized block's chain. Ex. if we have blockchains b1 <- b3 <- b5, and b1 <- b2 <- b4, -// a call to pruneNonFinalizedChain(finalization b5), will prune blocks b2 & b4 from the rounds map. -func (e *Epoch) pruneNonFinalizedChain(finalization common.Finalization) { +// a call to pruneCompetingRounds(finalization b5), will prune blocks b2 & b4 from the rounds map. +func (e *Epoch) pruneCompetingRounds(finalization common.Finalization) { prev := finalization.Finalization.Prev minRound := e.minRoundInRoundsMap() for r := finalization.Finalization.Round; r > minRound; r-- { @@ -3097,7 +3097,7 @@ func (e *Epoch) pruneNonFinalizedChain(finalization common.Finalization) { continue } - // don't delete this round, since the block is an ancestor of finalization + // don't delete this round, since the block is an ancestor of finalization in our chain if header := round.block.BlockHeader(); header.Digest == prev { prev = header.Prev continue diff --git a/simplex/epoch_test.go b/simplex/epoch_test.go index deb19345..5b11bd9c 100644 --- a/simplex/epoch_test.go +++ b/simplex/epoch_test.go @@ -1921,6 +1921,18 @@ func TestEpochRequestsEmptyRoundDependency(t *testing.T) { // TestEpochFinalizeVoteRebroadcastForTimedOutRound ensures that after finalizing a future sequence, // we rebroadcast a finalize vote for a round we timed out on only if its block is an ancestor of the // finalized block. Otherwise the vote could finalize a second block at the finalized sequence. +// +// fork: +// +// block0 (r0,s0) <-- block1 (r1,s1) timed out, notarized +// \ +// -- block2 (r2,s1) finalized +// +// ancestor: +// +// block0 (r0,s0) <-- block1 (r1,s1) <-- block2 (r2,s2) +// timed out, finalized +// notarized func TestEpochFinalizeVoteRebroadcastForTimedOutRound(t *testing.T) { t.Run("fork", func(t *testing.T) { testEpochFinalizeVoteRebroadcastForTimedOutRound(t, false) From 53042e672b0def2469fbc0cb1333091c2cab8ba1 Mon Sep 17 00:00:00 2001 From: samliok Date: Wed, 21 Oct 2026 15:35:35 -0400 Subject: [PATCH 4/5] add assertion --- simplex/epoch_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/simplex/epoch_test.go b/simplex/epoch_test.go index 5b11bd9c..a859d015 100644 --- a/simplex/epoch_test.go +++ b/simplex/epoch_test.go @@ -1968,6 +1968,8 @@ func testEpochFinalizeVoteRebroadcastForTimedOutRound(t *testing.T, buildOnTimed _, built := bb.BuildBlock(ctx, e.Metadata(), emptyBlacklist) require.True(t, built) block1 := bb.GetBuiltBlock() + require.Equal(t, block0.BlockHeader().Digest, block1.BlockHeader().Prev) + require.Equal(t, uint64(1), block1.BlockHeader().Seq) vote1, err := testutil.NewTestVote(block1, nodes[1]) require.NoError(t, err) @@ -1994,6 +1996,13 @@ func testEpochFinalizeVoteRebroadcastForTimedOutRound(t *testing.T, buildOnTimed _, built = bb.BuildBlock(ctx, md2, emptyBlacklist) require.True(t, built) block2 := bb.GetBuiltBlock() + if buildOnTimedOutRound { + require.Equal(t, block1.BlockHeader().Digest, block2.BlockHeader().Prev) + require.Equal(t, uint64(2), block2.BlockHeader().Seq) + } else { + require.Equal(t, block0.BlockHeader().Digest, block2.BlockHeader().Prev) + require.Equal(t, uint64(1), block2.BlockHeader().Seq) + } vote2, err := testutil.NewTestVote(block2, nodes[2]) require.NoError(t, err) From 0e9960918b0b9d5622002a45d7785df315463a82 Mon Sep 17 00:00:00 2001 From: samliok Date: Wed, 21 Oct 2026 15:46:55 -0400 Subject: [PATCH 5/5] create 2 seperate tests --- simplex/epoch_test.go | 157 +++++++++++++++++++++++++++++------------- 1 file changed, 109 insertions(+), 48 deletions(-) diff --git a/simplex/epoch_test.go b/simplex/epoch_test.go index a859d015..2dac0856 100644 --- a/simplex/epoch_test.go +++ b/simplex/epoch_test.go @@ -1918,31 +1918,14 @@ func TestEpochRequestsEmptyRoundDependency(t *testing.T) { require.Equal(t, uint64(3), e.Metadata().Round) } -// TestEpochFinalizeVoteRebroadcastForTimedOutRound ensures that after finalizing a future sequence, -// we rebroadcast a finalize vote for a round we timed out on only if its block is an ancestor of the +// TestEpochFinalizeVoteRebroadcastSkipsTimedOutFork ensures that after finalizing a future sequence, +// we do not rebroadcast a finalize vote for a timed out round whose block is not an ancestor of the // finalized block. Otherwise the vote could finalize a second block at the finalized sequence. // -// fork: -// // block0 (r0,s0) <-- block1 (r1,s1) timed out, notarized // \ // -- block2 (r2,s1) finalized -// -// ancestor: -// -// block0 (r0,s0) <-- block1 (r1,s1) <-- block2 (r2,s2) -// timed out, finalized -// notarized -func TestEpochFinalizeVoteRebroadcastForTimedOutRound(t *testing.T) { - t.Run("fork", func(t *testing.T) { - testEpochFinalizeVoteRebroadcastForTimedOutRound(t, false) - }) - t.Run("ancestor", func(t *testing.T) { - testEpochFinalizeVoteRebroadcastForTimedOutRound(t, true) - }) -} - -func testEpochFinalizeVoteRebroadcastForTimedOutRound(t *testing.T, buildOnTimedOutRound bool) { +func TestEpochFinalizeVoteRebroadcastSkipsTimedOutFork(t *testing.T) { bb := testutil.NewTestBlockBuilder() nodes := []NodeID{{1}, {2}, {3}, {4}} ctx := context.Background() @@ -1986,23 +1969,16 @@ func testEpochFinalizeVoteRebroadcastForTimedOutRound(t *testing.T, buildOnTimed require.Equal(t, NotarizationRecordType, wal.AssertNotarization(1)) testutil.WaitToEnterRound(t, e, 2) - // block2 either extends block1, or builds off block0 + // block2 builds off block0, skipping round 1 md2 := e.Metadata() - if !buildOnTimedOutRound { - md2.Seq = 1 - md2.Prev = block0.BlockHeader().Digest - } + md2.Seq = 1 + md2.Prev = block0.BlockHeader().Digest _, built = bb.BuildBlock(ctx, md2, emptyBlacklist) require.True(t, built) block2 := bb.GetBuiltBlock() - if buildOnTimedOutRound { - require.Equal(t, block1.BlockHeader().Digest, block2.BlockHeader().Prev) - require.Equal(t, uint64(2), block2.BlockHeader().Seq) - } else { - require.Equal(t, block0.BlockHeader().Digest, block2.BlockHeader().Prev) - require.Equal(t, uint64(1), block2.BlockHeader().Seq) - } + require.Equal(t, block0.BlockHeader().Digest, block2.BlockHeader().Prev) + require.Equal(t, uint64(1), block2.BlockHeader().Seq) vote2, err := testutil.NewTestVote(block2, nodes[2]) require.NoError(t, err) @@ -2014,25 +1990,111 @@ func testEpochFinalizeVoteRebroadcastForTimedOutRound(t *testing.T, buildOnTimed }, nodes[2])) // block2 skips round 1, so we need to send an empty notarization before block2 can be verified - if !buildOnTimedOutRound { - for msg := range sentMessages { - if msg.ReplicationRequest != nil { - require.Equal(t, uint64(1), msg.ReplicationRequest.Rounds[0]) - break - } + for msg := range sentMessages { + if msg.ReplicationRequest != nil { + require.Equal(t, uint64(1), msg.ReplicationRequest.Rounds[0]) + break } + } - require.NoError(t, e.HandleMessage(&Message{ - ReplicationResponse: &ReplicationResponse{ - Data: []QuorumRound{{EmptyNotarization: testutil.NewEmptyNotarization(nodes, 1)}}, - }, - }, nodes[2])) + require.NoError(t, e.HandleMessage(&Message{ + ReplicationResponse: &ReplicationResponse{ + Data: []QuorumRound{{EmptyNotarization: testutil.NewEmptyNotarization(nodes, 1)}}, + }, + }, nodes[2])) - for !wal.ContainsEmptyNotarization(1) { - time.Sleep(20 * time.Millisecond) + for !wal.ContainsEmptyNotarization(1) { + time.Sleep(20 * time.Millisecond) + } + + testutil.InjectTestVote(t, e, block2, nodes[0]) + testutil.InjectTestVote(t, e, block2, nodes[1]) + wal.AssertNotarization(2) + + // finalizing block2 is a future sequence since seq 0 is not committed + testutil.InjectTestFinalizeVote(t, e, block2, nodes[0]) + testutil.InjectTestFinalizeVote(t, e, block2, nodes[1]) + + var sentFinalizeVoteForBlock1 bool + for msg := range broadcastMessages { + if msg.FinalizeVote != nil && msg.FinalizeVote.Finalization.Round == 1 { + sentFinalizeVoteForBlock1 = true + } + if msg.Finalization != nil && msg.Finalization.Finalization.Round == 2 { + break } } + // block1 and block2 are both at seq 1, so we must not finalize vote block1 + require.False(t, sentFinalizeVoteForBlock1) +} + +// TestEpochFinalizeVoteRebroadcastForTimedOutAncestor ensures that after finalizing a future sequence, +// we rebroadcast a finalize vote for a timed out round whose block is an ancestor of the finalized block, +// since the finalized block cannot be committed until its ancestor is. +// +// block0 (r0,s0) <-- block1 (r1,s1) <-- block2 (r2,s2) +// timed out, finalized +// notarized +func TestEpochFinalizeVoteRebroadcastForTimedOutAncestor(t *testing.T) { + bb := testutil.NewTestBlockBuilder() + nodes := []NodeID{{1}, {2}, {3}, {4}} + ctx := context.Background() + broadcastMessages := make(chan *Message, 100) + comm := &recordingComm{Communication: testutil.NewNoopComm(nodes), BroadcastMessages: broadcastMessages} + conf, wal, _ := testutil.DefaultTestNodeEpochConfig(t, nodes[3], comm, bb) + conf.ReplicationEnabled = true + e, err := NewEpoch(conf) + require.NoError(t, err) + t.Cleanup(e.Stop) + + require.NoError(t, e.Start()) + + // round 0 is notarized but not finalized, so seq 0 is never committed + block0, _ := advanceRoundFromNotarization(t, e, bb) + testutil.WaitToEnterRound(t, e, 1) + + // we time out on round 1, then learn that block1 was notarized anyway + bb.BlockShouldBeBuilt <- struct{}{} + testutil.WaitForBlockProposerTimeout(t, e, &e.StartTime, 1) + + _, built := bb.BuildBlock(ctx, e.Metadata(), emptyBlacklist) + require.True(t, built) + block1 := bb.GetBuiltBlock() + require.Equal(t, block0.BlockHeader().Digest, block1.BlockHeader().Prev) + require.Equal(t, uint64(1), block1.BlockHeader().Seq) + + vote1, err := testutil.NewTestVote(block1, nodes[1]) + require.NoError(t, err) + require.NoError(t, e.HandleMessage(&Message{ + BlockMessage: &BlockMessage{ + Block: block1, + Vote: *vote1, + }, + }, nodes[1])) + notarization1, err := testutil.NewNotarization(e.Logger, e.SignatureAggregatorCreator(e.Comm.Validators()), block1, nodes[0:3]) + require.NoError(t, err) + testutil.InjectTestNotarization(t, e, notarization1, nodes[1]) + + require.Equal(t, NotarizationRecordType, wal.AssertNotarization(1)) + testutil.WaitToEnterRound(t, e, 2) + + // block2 extends block1 + _, built = bb.BuildBlock(ctx, e.Metadata(), emptyBlacklist) + require.True(t, built) + block2 := bb.GetBuiltBlock() + require.Equal(t, block1.BlockHeader().Digest, block2.BlockHeader().Prev) + require.Equal(t, uint64(2), block2.BlockHeader().Seq) + + vote2, err := testutil.NewTestVote(block2, nodes[2]) + require.NoError(t, err) + require.NoError(t, e.HandleMessage(&Message{ + BlockMessage: &BlockMessage{ + Block: block2, + Vote: *vote2, + }, + }, nodes[2])) + testutil.InjectTestVote(t, e, block2, nodes[0]) testutil.InjectTestVote(t, e, block2, nodes[1]) wal.AssertNotarization(2) @@ -2051,9 +2113,8 @@ func testEpochFinalizeVoteRebroadcastForTimedOutRound(t *testing.T, buildOnTimed } } - // If block2 skips round 1, we must not finalize vote block1, since both blocks would be finalized at seq 1. - // If block2 builds on block1, we must finalize vote block1, since block2 cannot be committed until block1 is. - require.Equal(t, buildOnTimedOutRound, sentFinalizeVoteForBlock1) + // block2 cannot be committed until block1 is, so we must finalize vote block1 + require.True(t, sentFinalizeVoteForBlock1) } // TestEpochFinalizeVoteRebroadcastFollowsFinalizedChain ensures we rebroadcast & create