Fix Double Start Round Bug

epoch.go

@@ -901,8 +901,8 @@ func (e *Epoch) handleVoteMessage(message *Vote, from NodeID) error {
 		return nil
 	}
 
-	if round.notarization != nil {
-		e.Logger.Debug("Round already notarized", zap.Uint64("round", vote.Round))
+	if round.notarization != nil || round.finalization != nil {
+		e.Logger.Debug("Round already notarized or finalized", zap.Uint64("round", vote.Round))
 		return nil
 	}
 
@@ -1279,11 +1279,11 @@ func (e *Epoch) persistEmptyNotarization(emptyVotes *EmptyVoteSet, shouldBroadca
 	emptyNotarization := emptyVotes.emptyNotarization
 	emptyNotarizationRecord := NewEmptyNotarizationRecord(emptyNotarization)
 	if err := e.WAL.Append(emptyNotarizationRecord); err != nil {
-		e.Logger.Error("Failed to append empty block record to WAL", zap.Error(err))
+		e.Logger.Error("Failed to append empty notarization record to WAL", zap.Error(err))
 		return err
 	}
 
-	e.Logger.Debug("Persisted empty block to WAL",
+	e.Logger.Debug("Persisted empty notarization to WAL",
 		zap.Int("size", len(emptyNotarizationRecord)),
 		zap.Uint64("round", emptyNotarization.Vote.Round))
 
@@ -1514,8 +1514,8 @@ func (e *Epoch) handleNotarizationMessage(message *Notarization, from NodeID) er
 	// Can we handle this notarization right away or should we handle it later?
 	round, exists := e.rounds[vote.Round]
 	// If we have already notarized the round, no need to continue
-	if exists && round.notarization != nil {
-		e.Logger.Debug("Received a notarization for an already notarized round")
+	if exists && (round.notarization != nil || round.finalization != nil) {
+		e.Logger.Debug("Received a notarization for an already notarized or finalized round")
 		return nil
 	}
 

epoch_test.go

@@ -1512,6 +1512,85 @@ func (b *listenerComm) Send(msg *Message, id NodeID) {
 	b.in <- msg
 }
 
+func TestRejectsOldNotarizationAndVotes(t *testing.T) {
+	bb := testutil.NewTestBlockBuilder()
+	ctx := context.Background()
+	nodes := []NodeID{{1}, {2}, {3}, {4}}
+	initialBlock := createBlocks(t, nodes, 1)[0]
+	conf, wal, storage := testutil.DefaultTestNodeEpochConfig(t, nodes[3], testutil.NewNoopComm(nodes), bb)
+	storage.Index(ctx, initialBlock.VerifiedBlock, initialBlock.Finalization)
+
+	e, err := NewEpoch(conf)
+	require.NoError(t, err)
+
+	require.NoError(t, e.Start())
+	require.Equal(t, uint64(1), e.Metadata().Seq)
+
+	// send a block for round 1. then finalization then notarization for round 1
+	md := e.Metadata()
+	_, ok := bb.BuildBlock(context.Background(), md, emptyBlacklist)
+	require.True(t, ok)
+
+	block := bb.GetBuiltBlock()
+
+	vote, err := testutil.NewTestVote(block, nodes[1])
+	require.NoError(t, err)
+	err = e.HandleMessage(&Message{
+		BlockMessage: &BlockMessage{
+			Vote:  *vote,
+			Block: block,
+		},
+	}, nodes[1])
+	require.NoError(t, err)
+	wal.AssertBlockProposal(1)
+
+	for i := 0; i < len(nodes); i++ {
+		if nodes[i].Equals(e.ID) {
+			continue
+		}
+		testutil.InjectTestFinalizeVote(t, e, block, nodes[i])
+	}
+	testutil.WaitToEnterRound(t, e, 2)
+
+	increment := 1
+	// wait for the empty vote
+	for !wal.ContainsEmptyVote(2) {
+		if len(bb.BlockShouldBeBuilt) == 0 {
+			bb.BlockShouldBeBuilt <- struct{}{}
+		}
+		e.AdvanceTime(e.StartTime.Add(conf.MaxProposalWait * time.Duration(increment)))
+		time.Sleep(100 * time.Millisecond)
+		increment++
+	}
+
+	// send notarization for round 1, after the finalization was sent
+	notarization, err := testutil.NewNotarization(conf.Logger, conf.SignatureAggregator, block, nodes)
+	require.NoError(t, err)
+
+	err = e.HandleMessage(&Message{
+		Notarization: &notarization,
+	}, nodes[0])
+	require.NoError(t, err)
+
+	timer := time.NewTimer(3 * time.Second)
+	defer timer.Stop()
+	for {
+		select {
+		case <-timer.C:
+			require.False(t, wal.ContainsNotarization(1), "notarization for old round should not be recorded")
+			return
+		default:
+			if len(bb.BlockShouldBeBuilt) == 0 {
+				bb.BlockShouldBeBuilt <- struct{}{}
+			}
+			wal.AssertHealthy(e.BlockDeserializer, e.QCDeserializer)
+			e.AdvanceTime(e.StartTime.Add(conf.MaxProposalWait * time.Duration(increment)))
+			time.Sleep(100 * time.Millisecond)
+			increment++
+		}
+	}
+}
+
 func TestBlockDeserializer(t *testing.T) {
 	var blockDeserializer testutil.BlockDeserializer
 

testutil/wal.go

@@ -4,6 +4,7 @@
 package testutil
 
 import (
+	"context"
 	"encoding/binary"
 	"sync"
 	"testing"
@@ -216,3 +217,75 @@ func (tw *TestWAL) ContainsEmptyNotarization(round uint64) bool {
 
 	return false
 }
+
+type walRound struct {
+	round                   uint64
+	blockRecord             bool
+	notarizationRecord      bool
+	finalizationRecord      bool
+	emptyNotarizationRecord bool
+	emptyVoteRecord         bool
+}
+
+// AssertHealthy checks that the WAL has at most one of each record type per round.
+func (tw *TestWAL) AssertHealthy(bd simplex.BlockDeserializer, qcd simplex.QCDeserializer) {
+	ctx := context.Background()
+	records, err := tw.WriteAheadLog.ReadAll()
+	require.NoError(tw.t, err)
+
+	rounds := make(map[uint64]*walRound)
+
+	for _, r := range records {
+		recordType := binary.BigEndian.Uint16(r)
+
+		switch recordType {
+		case record.BlockRecordType:
+			block, err := simplex.BlockFromRecord(ctx, bd, r)
+			require.NoError(tw.t, err)
+			round := block.BlockHeader().Round
+			if _, exists := rounds[round]; !exists {
+				rounds[round] = &walRound{round: round}
+			}
+			require.False(tw.t, rounds[round].blockRecord, "duplicate block record for round %d", round)
+			rounds[round].blockRecord = true
+		case record.NotarizationRecordType:
+			_, vote, err := simplex.ParseNotarizationRecord(r)
+			require.NoError(tw.t, err)
+			round := vote.Round
+			if _, exists := rounds[round]; !exists {
+				rounds[round] = &walRound{round: round}
+			}
+			require.False(tw.t, rounds[round].notarizationRecord, "duplicate notarization record for round %d", round)
+			rounds[round].notarizationRecord = true
+		case record.EmptyNotarizationRecordType:
+			_, vote, err := simplex.ParseEmptyNotarizationRecord(r)
+			require.NoError(tw.t, err)
+			round := vote.Round
+			if _, exists := rounds[round]; !exists {
+				rounds[round] = &walRound{round: round}
+			}
+			require.False(tw.t, rounds[round].emptyNotarizationRecord, "duplicate empty notarization record for round %d", round)
+			rounds[round].emptyNotarizationRecord = true
+		case record.EmptyVoteRecordType:
+			vote, err := simplex.ParseEmptyVoteRecord(r)
+			require.NoError(tw.t, err)
+			round := vote.Round
+			if _, exists := rounds[round]; !exists {
+				rounds[round] = &walRound{round: round}
+			}
+			require.False(tw.t, rounds[round].emptyVoteRecord, "duplicate empty vote record for round %d", round)
+			rounds[round].emptyVoteRecord = true
+		case record.FinalizationRecordType:
+			finalization, err := simplex.FinalizationFromRecord(r, qcd)
+			require.NoError(tw.t, err)
+			round := finalization.Finalization.Round
+			if _, exists := rounds[round]; !exists {
+				rounds[round] = &walRound{round: round}
+			}
+			require.False(tw.t, rounds[round].finalizationRecord, "duplicate finalization record for round %d", round)
+			rounds[round].finalizationRecord = true
+		default:
+			tw.t.Fatalf("undefined record type: %d", recordType)
+		}
+	}
+}