fix two twice delivery on sync

.github/workflows/build.yml

@@ -2,7 +2,7 @@ name: Build
 
 on:
   pull_request:
-    
+
 jobs:
 
   build:

internal/bft/controller.go

@@ -623,6 +623,10 @@ func (c *Controller) sync() (viewNum uint64, seq uint64, decisions uint64) {
 
 	latestSequence := c.latestSeq()
 
+	c.Logger.Debugf("Node %d is setting the checkpoint after sync to view %d and seq %d", c.ID, md.ViewId, md.LatestSequence)
+	c.Checkpoint.Set(decision.Proposal, decision.Signatures)
+	c.verificationSequence = uint64(decision.Proposal.VerificationSequence)
+
 	if md.ViewId < c.currViewNumber {
 		c.Logger.Infof("Synchronizer returned with view number %d but the controller is at view number %d", md.ViewId, c.currViewNumber)
 		return 0, 0, 0
@@ -657,9 +661,6 @@ func (c *Controller) sync() (viewNum uint64, seq uint64, decisions uint64) {
 		}
 	}
 
-	c.Logger.Debugf("Node %d is setting the checkpoint after sync to view %d and seq %d", c.ID, md.ViewId, md.LatestSequence)
-	c.Checkpoint.Set(decision.Proposal, decision.Signatures)
-	c.verificationSequence = uint64(decision.Proposal.VerificationSequence)
 	c.Logger.Debugf("Node %d is informing the view changer after sync of view %d and seq %d", c.ID, md.ViewId, md.LatestSequence)
 	c.ViewChanger.InformNewView(view)
 	if md.LatestSequence == 0 || newView {

internal/bft/controller_test.go

@@ -545,6 +545,168 @@ func TestSyncPrevView(t *testing.T) {
 	wal.Close()
 }
 
+func TestSyncPrevViewAnd2Twice(t *testing.T) {
+	basicLog, err := zap.NewDevelopment()
+	assert.NoError(t, err)
+	log := basicLog.Sugar()
+	app := &mocks.ApplicationMock{}
+	appWG := sync.WaitGroup{}
+	app.On("Deliver", mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
+		appWG.Done()
+	}).Return(types.Reconfig{InLatestDecision: false})
+	batcher := &mocks.Batcher{}
+	batcher.On("Close")
+	pool := &mocks.RequestPool{}
+	pool.On("Close")
+	pool.On("Prune", mock.Anything)
+	leaderMon := &mocks.LeaderMonitor{}
+	leaderMon.On("InjectArtificialHeartbeat", mock.Anything, mock.Anything)
+	leaderMonWG := sync.WaitGroup{}
+	leaderMon.On("ChangeRole", mock.Anything, mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
+		leaderMonWG.Done()
+	})
+	leaderMon.On("Close")
+	comm := &mocks.CommMock{}
+	comm.On("SendConsensus", mock.Anything, mock.Anything)
+	verifier := &mocks.VerifierMock{}
+	verifier.On("VerifyProposal", mock.Anything, mock.Anything).Return(nil, nil)
+	verifier.On("VerificationSequence").Return(uint64(1))
+	verifier.On("VerifyConsenterSig", mock.Anything, mock.Anything, mock.Anything).Return(nil, nil)
+	signer := &mocks.SignerMock{}
+	signer.On("SignProposal", mock.Anything, mock.Anything).Return(&types.Signature{
+		ID:    4,
+		Value: []byte{4},
+	})
+	fd := &mocks.FailureDetector{}
+	fd.On("Complain", mock.Anything, mock.Anything)
+	var numCall int
+	synchronizer := &mocks.SynchronizerMock{}
+	synchronizerWG := sync.WaitGroup{}
+	synchronizer.On("Sync").Run(func(args mock.Arguments) {
+		numCall++
+		if numCall == 1 {
+			synchronizerWG.Done()
+		}
+		if numCall == 2 {
+			appWG.Done()
+		}
+	}).Return(types.SyncResponse{Latest: types.Decision{
+		Proposal: types.Proposal{
+			Metadata: bft.MarshalOrPanic(&protos.ViewMetadata{
+				LatestSequence: 1,
+				ViewId:         0, // previous view number
+			}),
+			VerificationSequence: 1,
+		},
+		Signatures: nil,
+	}, Reconfig: types.ReconfigSync{InReplicatedDecisions: false}})
+
+	startedWG := sync.WaitGroup{}
+	startedWG.Add(1)
+
+	testDir, err := os.MkdirTemp("", "controller-unittest")
+	assert.NoErrorf(t, err, "generate temporary test dir")
+	defer os.RemoveAll(testDir)
+	wal, err := wal.Create(log, testDir, nil)
+	assert.NoError(t, err)
+
+	collector := bft.StateCollector{
+		SelfID:         0,
+		N:              4,
+		Logger:         log,
+		CollectTimeout: 100 * time.Millisecond,
+	}
+	collector.Start()
+	defer collector.Stop()
+
+	controller := &bft.Controller{
+		Collector:       &collector,
+		InFlight:        &bft.InFlightData{},
+		Batcher:         batcher,
+		RequestPool:     pool,
+		LeaderMonitor:   leaderMon,
+		ID:              4, // not the leader
+		N:               4,
+		NodesList:       []uint64{1, 2, 3, 4},
+		Logger:          log,
+		Application:     app,
+		Comm:            comm,
+		ViewChanger:     &bft.ViewChanger{},
+		Checkpoint:      &types.Checkpoint{},
+		FailureDetector: fd,
+		Synchronizer:    synchronizer,
+		Verifier:        verifier,
+		Signer:          signer,
+		WAL:             wal,
+		StartedWG:       &startedWG,
+		MetricsView:     api.NewMetricsView(&disabled.Provider{}),
+	}
+	controller.Deliver = &bft.MutuallyExclusiveDeliver{C: controller}
+
+	vs := configureProposerBuilder(controller)
+	controller.ViewSequences = vs
+
+	leaderMonWG.Add(1)
+	controller.Start(1, 1, 0, false)
+	leaderMonWG.Wait()
+
+	synchronizerWG.Add(1)
+	leaderMonWG.Add(1)
+	// send a message with view 2 to trigger sync
+	wrongViewMsg := proto.Clone(prePrepare).(*protos.Message)
+	wrongViewMsgGet := wrongViewMsg.GetPrePrepare()
+	wrongViewMsgGet.View = 2
+	controller.ProcessMessages(2, wrongViewMsg)
+	// waiting for a synchronization that returned blocks with LatestSequence: 1,
+	synchronizerWG.Wait()
+	leaderMonWG.Wait() // wait for view to start before sending messages
+
+	prePrepareNext := proto.Clone(prePrepare).(*protos.Message)
+	prePrepareNextGet := prePrepareNext.GetPrePrepare()
+	prePrepareNextGet.Seq = 1
+	prePrepareNextGet.GetProposal().Metadata = bft.MarshalOrPanic(&protos.ViewMetadata{
+		DecisionsInView: 0,
+		LatestSequence:  1,
+		ViewId:          1,
+	})
+	controller.ProcessMessages(2, prePrepareNext)
+
+	nextProp := types.Proposal{
+		Header:               prePrepareNextGet.Proposal.Header,
+		Payload:              prePrepareNextGet.Proposal.Payload,
+		Metadata:             prePrepareNextGet.Proposal.Metadata,
+		VerificationSequence: 1,
+	}
+	prepareNext := proto.Clone(prepare).(*protos.Message)
+	prepareNextGet := prepareNext.GetPrepare()
+	prepareNextGet.Seq = 1
+	prepareNextGet.Digest = nextProp.Digest()
+	controller.ProcessMessages(2, prepareNext)
+	controller.ProcessMessages(3, prepareNext)
+
+	commit2Next := proto.Clone(commit2).(*protos.Message)
+	commit2NextGet := commit2Next.GetCommit()
+	commit2NextGet.Seq = 1
+	commit2NextGet.Digest = nextProp.Digest()
+
+	commit3Next := proto.Clone(commit3).(*protos.Message)
+	commit3NextGet := commit3Next.GetCommit()
+	commit3NextGet.Seq = 1
+	commit3NextGet.Digest = nextProp.Digest()
+
+	appWG.Add(1)
+	controller.ProcessMessages(2, commit2Next)
+	controller.ProcessMessages(3, commit3Next)
+
+	// send a message with seq 1, but have already received a block with seq 1
+	// therefore no delivery will occur
+	appWG.Wait()
+	app.AssertNumberOfCalls(t, "Deliver", 0)
+
+	controller.Stop()
+	wal.Close()
+}
+
 func TestControllerLeaderRequestHandling(t *testing.T) {
 	for _, testCase := range []struct {
 		description      string