Create EmptyVoteMetadata Struct (#215)

* add empty vote metadata

* remove canoto

* rebase fix

* fix check

* update if

---------

Co-authored-by: yacovm <yacovm@users.noreply.github.com>

Author: samliok

epoch.go

@@ -664,13 +664,6 @@ func (e *Epoch) handleEmptyVoteMessage(message *EmptyVote, from NodeID) error {
 		return nil
 	}
 
-	if vote.HasParent() {
-		e.Logger.Debug("Empty vote has a parent but should not have one",
-			zap.Uint64("round", vote.Round),
-			zap.Stringer("Prev", vote.Prev), zap.Uint64("Seq", vote.Seq))
-		return nil
-	}
-
 	if e.round > vote.Round {
 		e.Logger.Debug("Got empty vote from a past round",
 			zap.Uint64("round", vote.Round), zap.Uint64("my round", e.round), zap.Stringer("from", from))
@@ -1311,13 +1304,6 @@ func (e *Epoch) handleEmptyNotarizationMessage(emptyNotarization *EmptyNotarizat
 }
 
 func (e *Epoch) verifyEmptyNotarization(emptyNotarization *EmptyNotarization) bool {
-	if emptyNotarization.Vote.HasParent() {
-		e.Logger.Debug("Empty notarization vote has a parent but should not have one",
-			zap.Uint64("round", emptyNotarization.Vote.Round),
-			zap.Stringer("Prev", emptyNotarization.Vote.Prev),
-			zap.Uint64("Seq", emptyNotarization.Vote.Seq))
-		return false
-	}
 	// Check empty notarization was signed by only eligible nodes
 	if emptyNotarization.QC == nil {
 		e.Logger.Debug("Empty notarization quorum certificate is nil")
@@ -2086,11 +2072,10 @@ func (e *Epoch) metadata() ProtocolMetadata {
 }
 
 func (e *Epoch) triggerEmptyBlockNotarization(round uint64) {
-	md := e.metadata()
-	md.Seq = 0            // Empty votes have no sequence number because they don't have a parent block.
-	md.Prev = emptyDigest // Empty votes are orphans and do not have parent blocks.
-
-	emptyVote := ToBeSignedEmptyVote{ProtocolMetadata: md}
+	emptyVote := ToBeSignedEmptyVote{EmptyVoteMetadata: EmptyVoteMetadata{
+		Round: round,
+		Epoch: e.Epoch,
+	}}
 	rawSig, err := emptyVote.Sign(e.Signer)
 	if err != nil {
 		e.Logger.Error("Failed signing message", zap.Error(err))
@@ -2540,7 +2525,7 @@ func (e *Epoch) handleReplicationResponse(resp *ReplicationResponse, from NodeID
 			continue
 		}
 
-		if nextSeqToCommit > data.GetSequence() {
+		if data.EmptyNotarization == nil && nextSeqToCommit > data.GetSequence() {
 			e.Logger.Debug("Received quorum round for a seq that is too far behind", zap.Uint64("seq", data.GetSequence()))
 			continue
 		}

epoch_failover_test.go

@@ -94,7 +94,7 @@ func TestEpochLeaderFailoverWithEmptyNotarization(t *testing.T) {
 		bb.in <- block.(*testBlock)
 	}
 
-	emptyNotarization := newEmptyNotarization(nodes[:3], 2, 1)
+	emptyNotarization := newEmptyNotarization(nodes[:3], 2)
 
 	e.HandleMessage(&Message{
 		EmptyNotarization: emptyNotarization,
@@ -116,7 +116,7 @@ func TestEpochLeaderFailoverWithEmptyNotarization(t *testing.T) {
 }
 
 // newEmptyNotarization creates a new empty notarization
-func newEmptyNotarization(nodes []NodeID, round uint64, seq uint64) *EmptyNotarization {
+func newEmptyNotarization(nodes []NodeID, round uint64) *EmptyNotarization {
 	var qc testQC
 
 	for i, node := range nodes {
@@ -125,7 +125,7 @@ func newEmptyNotarization(nodes []NodeID, round uint64, seq uint64) *EmptyNotari
 
 	return &EmptyNotarization{
 		QC: qc,
-		Vote: ToBeSignedEmptyVote{ProtocolMetadata: ProtocolMetadata{
+		Vote: ToBeSignedEmptyVote{EmptyVoteMetadata: EmptyVoteMetadata{
 			Round: round,
 		}},
 	}
@@ -212,7 +212,6 @@ func TestEpochLeaderFailoverReceivesEmptyVotesEarly(t *testing.T) {
 		require.NoError(t, err)
 		require.Equal(t, emptyVoteFrom1.Vote, emptyNotarization.Vote)
 		require.Equal(t, uint64(3), emptyNotarization.Vote.Round)
-		require.Equal(t, uint64(0), emptyNotarization.Vote.Seq)
 		require.Equal(t, uint64(3), storage.Height())
 
 		header, _, err := ParseBlockRecord(rawProposal)
@@ -266,7 +265,7 @@ func TestReceiveEmptyNotarizationWithNoQC(t *testing.T) {
 
 	require.NoError(t, e.Start())
 
-	emptyNotarization := newEmptyNotarization(nodes[:3], 0, 0)
+	emptyNotarization := newEmptyNotarization(nodes[:3], 0)
 
 	e.HandleMessage(&Message{
 		EmptyNotarization: &EmptyNotarization{Vote: emptyNotarization.Vote},
@@ -353,7 +352,6 @@ func TestEpochLeaderFailover(t *testing.T) {
 		require.NoError(t, err)
 		require.Equal(t, emptyVoteFrom1.Vote, emptyNotarization.Vote)
 		require.Equal(t, uint64(3), emptyNotarization.Vote.Round)
-		require.Equal(t, uint64(0), emptyNotarization.Vote.Seq)
 		require.Equal(t, uint64(3), storage.Height())
 
 		nextBlockSeqToCommit := uint64(3)
@@ -571,7 +569,6 @@ func TestEpochLeaderFailoverAfterProposal(t *testing.T) {
 		require.NoError(t, err)
 		require.Equal(t, emptyVoteFrom1.Vote, emptyNotarization.Vote)
 		require.Equal(t, uint64(3), emptyNotarization.Vote.Round)
-		require.Equal(t, uint64(0), emptyNotarization.Vote.Seq)
 		require.Equal(t, uint64(4), storage.Height())
 	})
 }
@@ -687,7 +684,6 @@ func TestEpochLeaderFailoverTwice(t *testing.T) {
 			require.NoError(t, err)
 			require.Equal(t, emptyVoteFrom1.Vote, emptyNotarization.Vote)
 			require.Equal(t, uint64(3), emptyNotarization.Vote.Round)
-			require.Equal(t, uint64(0), emptyNotarization.Vote.Seq)
 			require.Equal(t, uint64(3), storage.Height())
 		})
 	})
@@ -762,14 +758,15 @@ func TestEpochLeaderFailoverBecauseOfBadBlock(t *testing.T) {
 }
 
 func createEmptyVote(md ProtocolMetadata, signer NodeID) *EmptyVote {
-	md.Prev = Digest{}
-	md.Seq = 0
 	emptyVoteFrom2 := &EmptyVote{
 		Signature: Signature{
 			Signer: signer,
 		},
 		Vote: ToBeSignedEmptyVote{
-			ProtocolMetadata: md,
+			EmptyVoteMetadata: EmptyVoteMetadata{
+				Round: md.Round,
+				Epoch: 0,
+			},
 		},
 	}
 	return emptyVoteFrom2
@@ -951,7 +948,7 @@ func TestEpochRebroadcastsEmptyVote(t *testing.T) {
 		wal.assertWALSize(1)
 	}
 
-	emptyNotarization := newEmptyNotarization(nodes, 0, 0)
+	emptyNotarization := newEmptyNotarization(nodes, 0)
 	e.HandleMessage(&simplex.Message{
 		EmptyNotarization: emptyNotarization,
 	}, nodes[2])

epoch_test.go

@@ -327,7 +327,7 @@ func TestEpochNotarizeTwiceThenFinalize(t *testing.T) {
 	wal.assertNotarization(0)
 
 	// Round 1
-	emptyNote := newEmptyNotarization(nodes, 1, 0)
+	emptyNote := newEmptyNotarization(nodes, 1)
 	err = e.HandleMessage(&Message{
 		EmptyNotarization: emptyNote,
 	}, nodes[1])
@@ -542,7 +542,7 @@ func advanceRoundFromEmpty(t *testing.T, e *Epoch) {
 	leader := LeaderForRound(e.Comm.Nodes(), e.Metadata().Round)
 	require.False(t, e.ID.Equals(leader), "epoch cannot be the leader for the empty round")
 
-	emptyNote := newEmptyNotarization(e.Comm.Nodes(), e.Metadata().Round, e.Metadata().Seq)
+	emptyNote := newEmptyNotarization(e.Comm.Nodes(), e.Metadata().Round)
 	err := e.HandleMessage(&Message{
 		EmptyNotarization: emptyNote,
 	}, leader)
@@ -943,9 +943,9 @@ func TestEpochQCSignedByNonExistentNodes(t *testing.T) {
 
 		err = e.HandleMessage(&Message{
 			EmptyNotarization: &EmptyNotarization{
-				Vote: ToBeSignedEmptyVote{ProtocolMetadata: ProtocolMetadata{
+				Vote: ToBeSignedEmptyVote{EmptyVoteMetadata: EmptyVoteMetadata{
 					Round: 0,
-					Seq:   0,
+					Epoch: 0,
 				}},
 				QC: qc,
 			},
@@ -963,9 +963,9 @@ func TestEpochQCSignedByNonExistentNodes(t *testing.T) {
 
 		err = e.HandleMessage(&Message{
 			EmptyNotarization: &EmptyNotarization{
-				Vote: ToBeSignedEmptyVote{ProtocolMetadata: ProtocolMetadata{
+				Vote: ToBeSignedEmptyVote{EmptyVoteMetadata: EmptyVoteMetadata{
 					Round: 0,
-					Seq:   0,
+					Epoch: 0,
 				}},
 				QC: qc,
 			},
@@ -998,86 +998,6 @@ func TestEpochQCSignedByNonExistentNodes(t *testing.T) {
 	})
 }
 
-func TestEpochDiscardEmptyVotesWithParent(t *testing.T) {
-	loggedMessages := make(chan string, 100)
-	l := testutil.MakeLogger(t, 1)
-	l.Intercept(func(entry zapcore.Entry) error {
-		loggedMessages <- entry.Message
-		return nil
-	})
-	hasLogged := func(msg string) bool {
-		for len(loggedMessages) > 0 {
-			loggedMsg := <-loggedMessages
-			if loggedMsg == msg {
-				return true
-			}
-		}
-		return false
-	}
-
-	bb := &testBlockBuilder{out: make(chan *testBlock, 1)}
-	storage := newInMemStorage()
-
-	nodes := []NodeID{{1}, {2}, {3}, {4}}
-	conf := EpochConfig{
-		MaxProposalWait:     DefaultMaxProposalWaitTime,
-		Logger:              l,
-		ID:                  nodes[0],
-		Signer:              &testSigner{},
-		WAL:                 newTestWAL(t),
-		Verifier:            &testVerifier{},
-		Storage:             storage,
-		Comm:                noopComm(nodes),
-		BlockBuilder:        bb,
-		SignatureAggregator: &testSignatureAggregator{},
-	}
-
-	e, err := NewEpoch(conf)
-	require.NoError(t, err)
-
-	require.NoError(t, e.Start())
-
-	vb, _ := notarizeAndFinalizeRound(t, e, bb)
-	require.NotNil(t, vb)
-
-	emptyBlockMd := e.Metadata()
-	emptyBlockMd.Seq--
-
-	// Receive empty votes from all nodes, but with a parent set to the last block.
-	// While this is not a real scenario but an artificial one, we are testing it to ensure that we do not
-	// collect an empty notarization.
-	for i := 1; i <= 3; i++ {
-		emptyVote := createEmptyVote(emptyBlockMd, nodes[i])
-		emptyVote.Vote.Prev = vb.BlockHeader().Digest
-
-		require.False(t, hasLogged("Empty vote has a parent but should not have one"))
-
-		e.HandleMessage(&Message{
-			EmptyVoteMessage: emptyVote,
-		}, nodes[i])
-
-		require.True(t, hasLogged("Empty vote has a parent but should not have one"))
-	}
-
-	emptyNotarization := newEmptyNotarization(nodes, emptyBlockMd.Round, emptyBlockMd.Seq)
-	emptyNotarization.Vote.Prev = vb.BlockHeader().Digest
-
-	require.False(t, hasLogged("Empty notarization vote has a parent but should not have one"))
-
-	e.HandleMessage(&Message{
-		EmptyNotarization: emptyNotarization,
-	}, nodes[1])
-
-	require.True(t, hasLogged("Empty notarization vote has a parent but should not have one"))
-
-	vb, _ = notarizeAndFinalizeRound(t, e, bb)
-	require.NotNil(t, vb)
-
-	// Ensure the previous empty votes and empty notarization did not have any effect.
-	require.Equal(t, uint64(1), vb.BlockHeader().Round)
-
-}
-
 func TestEpochBlockSentFromNonLeader(t *testing.T) {
 	l := testutil.MakeLogger(t, 1)
 	nonLeaderMessage := false

msg.go

@@ -6,6 +6,7 @@ package simplex
 import (
 	"bytes"
 	"encoding/asn1"
+	"encoding/binary"
 	"fmt"
 )
 
@@ -23,30 +24,34 @@ type Message struct {
 	ReplicationRequest          *ReplicationRequest
 }
 
-type ToBeSignedEmptyVote struct {
-	ProtocolMetadata
+type EmptyVoteMetadata struct {
+	Round uint64
+	Epoch uint64
 }
 
-func (v *ToBeSignedEmptyVote) HasParent() bool {
-	return v.ProtocolMetadata.Prev != emptyDigest || v.ProtocolMetadata.Seq != 0
+type ToBeSignedEmptyVote struct {
+	EmptyVoteMetadata
 }
 
 func (v *ToBeSignedEmptyVote) Bytes() []byte {
-	return v.ProtocolMetadata.Bytes()
+	bytes := make([]byte, 8+8) // Round + Epoch
+	binary.BigEndian.PutUint64(bytes[0:8], v.EmptyVoteMetadata.Round)
+	binary.BigEndian.PutUint64(bytes[8:16], v.EmptyVoteMetadata.Epoch)
+	return bytes
 }
 
 func (v *ToBeSignedEmptyVote) FromBytes(buff []byte) error {
-	if len(buff) != ProtocolMetadataLen {
-		return fmt.Errorf("invalid buffer length %d, expected %d", len(buff), ProtocolMetadataLen)
-	}
-
-	md, err := ProtocolMetadataFromBytes(buff[:ProtocolMetadataLen])
-	if err != nil {
-		return fmt.Errorf("failed to parse ProtocolMetadata: %w", err)
+	if len(buff) != 16 {
+		return fmt.Errorf("invalid buffer length")
 	}
 
-	v.ProtocolMetadata = *md
+	round := binary.BigEndian.Uint64(buff[0:8])
+	epoch := binary.BigEndian.Uint64(buff[8:16])
 
+	v.EmptyVoteMetadata = EmptyVoteMetadata{
+		Round: round,
+		Epoch: epoch,
+	}
 	return nil
 }
 
@@ -255,10 +260,6 @@ func (q *QuorumRound) GetRound() uint64 {
 }
 
 func (q *QuorumRound) GetSequence() uint64 {
-	if q.EmptyNotarization != nil {
-		return q.EmptyNotarization.Vote.Seq
-	}
-
 	if q.Block != nil {
 		return q.Block.BlockHeader().Seq
 	}

replication.go

@@ -27,12 +27,11 @@ func newSignedSequenceFromRound(round QuorumRound) (*signedSequence, error) {
 	case round.Finalization != nil:
 		ss.signers = round.Finalization.QC.Signers()
 		ss.seq = round.Finalization.Finalization.Seq
-	case round.EmptyNotarization != nil:
-		ss.signers = round.EmptyNotarization.QC.Signers()
-		ss.seq = round.EmptyNotarization.Vote.Seq
 	case round.Notarization != nil:
 		ss.signers = round.Notarization.QC.Signers()
 		ss.seq = round.Notarization.Vote.Seq
+	case round.EmptyNotarization != nil:
+		return nil, fmt.Errorf("should not create signed sequence from empty notarization")
 	default:
 		return nil, fmt.Errorf("round does not contain a finalization, empty notarization, or notarization")
 	}
@@ -278,7 +277,7 @@ func (r *ReplicationState) StoreQuorumRound(round QuorumRound) {
 		return
 	}
 
-	if round.GetSequence() > r.highestSequenceObserved.seq {
+	if round.EmptyNotarization == nil && round.GetSequence() > r.highestSequenceObserved.seq {
 		signedSeq, err := newSignedSequenceFromRound(round)
 		if err != nil {
 			// should never be here since we already checked the QuorumRound was valid

replication_request_test.go

@@ -138,7 +138,7 @@ func TestReplicationRequestMixed(t *testing.T) {
 		leaderForRound := bytes.Equal(simplex.LeaderForRound(nodes, uint64(i)), e.ID)
 		emptyBlock := !leaderForRound
 		if emptyBlock {
-			emptyNotarization := newEmptyNotarization(nodes, uint64(i), uint64(i))
+			emptyNotarization := newEmptyNotarization(nodes, uint64(i))
 			e.HandleMessage(&simplex.Message{
 				EmptyNotarization: emptyNotarization,
 			}, nodes[1])

util.go

@@ -113,7 +113,7 @@ func GetLatestVerifiedQuorumRound(round *Round, emptyNotarization *EmptyNotariza
 			verifiedQuorumRound = &VerifiedQuorumRound{
 				EmptyNotarization: emptyNotarization,
 			}
-			highestRound = emptyNotarization.Vote.ProtocolMetadata.Round
+			highestRound = emptyNotarization.Vote.Round
 			exists = true
 		}
 	}