Throw away equivocated round when processing finalizations

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
 }
 

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 {

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 {

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.

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