From 79947f8a88747b874da8373a3281e3e57f01a5c1 Mon Sep 17 00:00:00 2001 From: Yacov Manevich Date: Thu, 23 Jul 2026 18:36:20 +0200 Subject: [PATCH] Throw away equivocated round when processing finalizations When receiving a finalization, check if the block received beforehand corresponds to the received finalization, and if not, delete the round. Signed-off-by: Yacov Manevich --- common/msg.go | 8 +++ common/msg_test.go | 25 ++++++++ simplex/epoch.go | 65 ++++++++++++++++----- simplex/epoch_failover_test.go | 18 +----- simplex/epoch_test.go | 103 +++++++++++++++++++++++++++++++++ 5 files changed, 189 insertions(+), 30 deletions(-) diff --git a/common/msg.go b/common/msg.go index 19e2ba6e..9a37d6d0 100644 --- a/common/msg.go +++ b/common/msg.go @@ -285,6 +285,14 @@ func (q *QuorumRound) IsWellFormed() error { return fmt.Errorf("malformed QuorumRound, block but no notarization or finalization") } + if q.Finalization != nil && q.Block == nil { + return fmt.Errorf("malformed QuorumRound, finalization but no block") + } + + if q.Notarization != nil && q.Block == nil { + return fmt.Errorf("malformed QuorumRound, notarization but no block") + } + return nil } diff --git a/common/msg_test.go b/common/msg_test.go index 32567e68..13d0dc72 100644 --- a/common/msg_test.go +++ b/common/msg_test.go @@ -193,6 +193,31 @@ func TestQuorumRoundMalformed(t *testing.T) { }, expectedErr: false, }, + { + name: "empty notarization and finalization, no block", + qr: common.QuorumRound{ + EmptyNotarization: &common.EmptyNotarization{}, + Finalization: &common.Finalization{}, + }, + expectedErr: true, + }, + { + name: "empty notarization and notarization, no block", + qr: common.QuorumRound{ + EmptyNotarization: &common.EmptyNotarization{}, + Notarization: &common.Notarization{}, + }, + expectedErr: true, + }, + { + name: "empty notarization and notarization and finalization, no block", + qr: common.QuorumRound{ + EmptyNotarization: &common.EmptyNotarization{}, + Notarization: &common.Notarization{}, + Finalization: &common.Finalization{}, + }, + expectedErr: true, + }, } for _, test := range tests { diff --git a/simplex/epoch.go b/simplex/epoch.go index ab58f7f4..529d30ab 100644 --- a/simplex/epoch.go +++ b/simplex/epoch.go @@ -479,13 +479,11 @@ func (e *Epoch) loadFinalizationRecord(r []byte) error { e.Logger.Debug("Finalization already indexed, skipping restoration", zap.Uint64("Sequence", finalization.Finalization.Seq)) return nil } - - round, ok := e.rounds[finalization.Finalization.Round] - if !ok { - return fmt.Errorf("round not found for finalization") + err = e.storeFinalization(&finalization) + if err != nil { + return err } e.Logger.Info("Finalization Recovered From WAL", zap.Uint64("Round", finalization.Finalization.Round)) - round.finalization = &finalization return nil } @@ -758,19 +756,16 @@ func (e *Epoch) handleFinalizationMessage(message *common.Finalization, from com return nil } - round, exists := e.rounds[message.Finalization.Round] + _, exists := e.rounds[message.Finalization.Round] if !exists { e.handleFinalizationForPendingOrFutureRound(message, message.Finalization.Round, nextSeqToCommit) return nil } - if round.finalization != nil { - e.Logger.Debug("Received finalization for an already finalized round", zap.Uint64("round", message.Finalization.Round)) + if err := e.storeFinalization(message); err != nil { return nil } - round.finalization = message - return e.persistFinalization(*message) } @@ -1200,10 +1195,10 @@ func (e *Epoch) maybeCollectFinalization(round *Round) error { return nil } - return e.assembleFinalization(round, finalizations) + return e.assembleFinalization(finalizations) } -func (e *Epoch) assembleFinalization(round *Round, finalizationVotes []*common.FinalizeVote) error { +func (e *Epoch) assembleFinalization(finalizationVotes []*common.FinalizeVote) error { for _, vote := range finalizationVotes { e.Logger.Debug("Collected a finalize vote from node", zap.Stringer("NodeID", vote.Signature.Signer), zap.Uint64("round", vote.Finalization.Round), zap.Uint64("seq", vote.Finalization.Seq)) } @@ -1213,7 +1208,10 @@ func (e *Epoch) assembleFinalization(round *Round, finalizationVotes []*common.F return err } - round.finalization = &finalization + if err := e.storeFinalization(&finalization); err != nil { + return nil + } + return e.persistFinalization(finalization) } @@ -1950,7 +1948,10 @@ func (e *Epoch) processFinalizedBlock(block common.Block, finalization *common.F delete(e.rounds, round.num) return e.processFinalizedBlock(block, finalization) } - round.finalization = finalization + if err := e.storeFinalization(finalization); err != nil { + e.Logger.Error("Failed storing finalization", zap.Error(err)) + return err + } prevEpochRound := e.round if err := e.indexFinalizations(round.num); err != nil { e.Logger.Error("Failed to index finalization", zap.Error(err)) @@ -2176,8 +2177,10 @@ func (e *Epoch) createFinalizedBlockVerificationTask(block common.Block, finaliz // Store the verified block in rounds map so subsequent blocks can find it as a dependency roundEntry := NewRound(verifiedBlock) - roundEntry.finalization = finalization e.rounds[md.Round] = roundEntry + if err := e.storeFinalization(finalization); err != nil { + return md.Digest + } e.Logger.Debug("Stored finalized replicated block in rounds map", zap.Uint64("round", md.Round), zap.Uint64("seq", md.Seq), @@ -2819,6 +2822,38 @@ func (e *Epoch) retrieveLastPersistedBlacklist() (common.Blacklist, bool) { return blacklist, true } +func (e *Epoch) storeFinalization(finalization *common.Finalization) error { + if finalization == nil { + return errors.New("finalization is nil") + } + roundNum := finalization.Finalization.BlockHeader.Round + round, exists := e.rounds[roundNum] + if !exists { + return fmt.Errorf("round %d not found", roundNum) + } + if round.finalization != nil { + e.Logger.Debug("Received finalization for an already finalized round", zap.Uint64("round", finalization.Finalization.Round)) + return fmt.Errorf("round %d already has a finalization", roundNum) + } + if round.block == nil { + return fmt.Errorf("round %d has no block to associate finalization with", roundNum) + } + + expectedBlockHeader := round.block.BlockHeader() + if !expectedBlockHeader.Equals(&finalization.Finalization.BlockHeader) { + e.Logger.Debug("Equivocation detected: finalization block header does not match round block header", + zap.Uint64("round", roundNum), + zap.Stringer("block header", &expectedBlockHeader), + zap.Stringer("finalized block header", &finalization.Finalization.BlockHeader)) + delete(e.rounds, roundNum) + return fmt.Errorf("finalization block header does not match round %d block header", roundNum) + } + + round.finalization = finalization + + return nil +} + func (e *Epoch) startRound() error { // before starting the round, load any future messages we might have received if err := e.maybeLoadFutureMessages(); err != nil { diff --git a/simplex/epoch_failover_test.go b/simplex/epoch_failover_test.go index 480002f1..3d33da2f 100644 --- a/simplex/epoch_failover_test.go +++ b/simplex/epoch_failover_test.go @@ -1078,6 +1078,7 @@ func TestEpochBlacklist(t *testing.T) { wal.AssertNotarization(11) // Now it's our turn to propose a new block. + // We make sure it is built how we expect it to be built below. bb.BlockShouldBeBuilt <- struct{}{} block = bb.GetBuiltBlock() @@ -1096,21 +1097,8 @@ func TestEpochBlacklist(t *testing.T) { Updates: []BlacklistUpdate{{Type: BlacklistOpType_NodeRedeemed, NodeIndex: 3}}, }, block.Blacklist(), "Node should vote to redeem the previously failed node") - blacklist = Blacklist{ - NodeCount: 4, - SuspectedNodes: SuspectedNodes{ - { - NodeIndex: 3, - SuspectingCount: 2, - OrbitSuspected: 1, - RedeemingCount: 2, - OrbitToRedeem: Orbit(12, 3, 4), - }, - }, - Updates: []BlacklistUpdate{{Type: BlacklistOpType_NodeRedeemed, NodeIndex: 3}}, - } - - block, _ = bb.BuildBlock(context.Background(), e.Metadata(), blacklist) + require.Equal(t, conf.ID, LeaderForRound(nodes, block.BlockHeader().Round)) + bb.SetBuiltBlock(block.(*testutil.TestBlock)) // Insert the block built by the block builder back into block builder so it will re-propose it block, _ = notarizeAndFinalizeRound(t, e, bb) // The next blacklist garbage collects node 3 from the blacklist. diff --git a/simplex/epoch_test.go b/simplex/epoch_test.go index 1e000d03..ef7a8ecc 100644 --- a/simplex/epoch_test.go +++ b/simplex/epoch_test.go @@ -451,6 +451,109 @@ func TestEpochIndexFinalization(t *testing.T) { storage.WaitForBlockCommit(2) } +func TestEquivocatedBlockFinalized(t *testing.T) { + bb := testutil.NewTestBlockBuilder() + nodes := []NodeID{{1}, {2}, {3}, {4}} + + recordingComm := &recordingComm{ + Communication: testutil.NewNoopComm(nodes), + BroadcastMessages: make(chan *Message, 100), + SentMessages: make(chan *Message, 100), + } + conf, wal, storage := testutil.DefaultTestNodeEpochConfig(t, nodes[1], recordingComm, bb) + conf.ReplicationEnabled = true + + leader := LeaderForRound(nodes, 0) + require.NotEqual(t, conf.ID, leader) // Ensure that the node is not the leader for the first round. + + e, err := NewEpoch(conf) + require.NoError(t, err) + t.Cleanup(e.Stop) + require.NoError(t, e.Start()) + + // (1) Byzantine leader equivocates and sends a block to the node. + md := e.Metadata() + _, ok := bb.BuildBlock(context.Background(), md, emptyBlacklist) + require.True(t, ok) + blockA := bb.GetBuiltBlock() + + voteA, err := testutil.NewTestVote(blockA, leader) + require.NoError(t, err) + require.NoError(t, e.HandleMessage(&Message{ + BlockMessage: &BlockMessage{ + Vote: *voteA, + Block: blockA, + }, + }, leader)) + + // (2) Ensure the node has written the block to the WAL. + wal.AssertWALSize(1) + + // (3) The honest majority finalizes a *different* block B + blockB := testutil.NewTestBlock(blockA.BlockHeader().ProtocolMetadata, emptyBlacklist) + blockB.Data = []byte("equivocated-block-B") + blockB.ComputeDigest() + // Ensure that the two blocks are different. + require.NotEqual(t, blockA.BlockHeader().Digest, blockB.BlockHeader().Digest) + + validators := e.Comm.Validators() + quorum := Quorum(len(validators)) + sigAggr := e.SignatureAggregatorCreator(validators) + finalizationB, _ := testutil.NewFinalizationRecord(t, sigAggr, blockB, validators.NodeIDs()[:quorum]) + require.Equal(t, blockB.BlockHeader().Digest, finalizationB.Finalization.BlockHeader.Digest) + + // (4) Send the finalization for block B to the node. + testutil.InjectTestFinalization(t, e, &finalizationB, nodes[2]) + + // Block 0 should not commit the block because it has never received block B. + storage.EnsureNoBlockCommit(t, 0) + + // (5) The honest majority continues past the equivocated round: round 1 is empty + // notarized (a timeout), and block C is finalized at round 2 building directly on + // top of block B (seq 1). Receiving the finalization for round 2 makes the node + // realize it is behind and triggers replication. + + blockC := testutil.NewTestBlock(ProtocolMetadata{ + Round: 2, + Seq: 1, + Prev: blockB.BlockHeader().Digest, + }, emptyBlacklist) + finalizationC, _ := testutil.NewFinalizationRecord(t, sigAggr, blockC, validators.NodeIDs()[:quorum]) + + // (6) The future finalization for round 2 triggers a replication request. + testutil.InjectTestFinalization(t, e, &finalizationC, nodes[2]) + + for msg := range recordingComm.SentMessages { + if msg.ReplicationRequest != nil { + break + } + } + + // (7) Feed the node the replication response containing the correctly finalized + // block B and block C built on block B. + replicationResponse := &ReplicationResponse{ + Data: []QuorumRound{ + { + Block: blockB, + Finalization: &finalizationB, + }, + { + Block: blockC, + Finalization: &finalizationC, + }, + }, + } + require.NoError(t, e.HandleMessage(&Message{ + ReplicationResponse: replicationResponse, + }, nodes[2])) + + // (8) The node recovers by committing the block the network actually finalized + // (block B) at seq 0, not the equivocated block A, followed by block C at seq 1. + require.Equal(t, blockB, storage.WaitForBlockCommit(0)) + require.Equal(t, blockC, storage.WaitForBlockCommit(1)) + require.Equal(t, uint64(2), storage.NumBlocks()) +} + func TestEpochConsecutiveProposalsDoNotGetVerified(t *testing.T) { for _, test := range []struct { name string