diff --git a/epoch.go b/epoch.go index 8488b2dc..1c2aede0 100644 --- a/epoch.go +++ b/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 } diff --git a/epoch_test.go b/epoch_test.go index 44aa8975..76eb32ec 100644 --- a/epoch_test.go +++ b/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: ¬arization, + }, 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 diff --git a/testutil/wal.go b/testutil/wal.go index 7adb636f..219f52af 100644 --- a/testutil/wal.go +++ b/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) + } + } +}