syncer: fix syncer panic (#456)

* Fix syncer panic

We can't verify correctness for randomness result from rounds that
corresponding configurations are not ready yet.

* Fix blocks are not confirmed while they should be

3 files changed, 127 insertions, 3 deletions

diff --git a/core/syncer/agreement.go b/core/syncer/agreement.go
index acc4f1c..9f1abca 100644
--- a/core/syncer/agreement.go
+++ b/core/syncer/agreement.go
@@ -160,8 +160,10 @@ func (a *agreement) processNewCRS(round uint64) {
 	if round <= a.latestCRSRound {
 		return
 	}
+	prevRound := a.latestCRSRound + 1
+	a.latestCRSRound = round
 	// Verify all pending results.
-	for r := a.latestCRSRound + 1; r <= round; r++ {
+	for r := prevRound; r <= a.latestCRSRound; r++ {
 		pendingsForRound := a.pendings[r]
 		if pendingsForRound == nil {
 			continue
@@ -169,7 +171,9 @@ func (a *agreement) processNewCRS(round uint64) {
 		delete(a.pendings, r)
 		for _, res := range pendingsForRound {
 			if err := core.VerifyAgreementResult(res, a.cache); err != nil {
-				a.logger.Error("invalid agreement result", "result", res)
+				a.logger.Error("invalid agreement result",
+					"result", res,
+					"error", err)
 				continue
 			}
 			a.logger.Error("flush agreement result", "result", res)
@@ -177,7 +181,6 @@ func (a *agreement) processNewCRS(round uint64) {
 			break
 		}
 	}
-	a.latestCRSRound = round
 }
 
 // confirm notifies consensus the confirmation of a block in BA.
diff --git a/core/syncer/agreement_test.go b/core/syncer/agreement_test.go
new file mode 100644
index 0000000..4e66f8b
--- /dev/null
+++ b/core/syncer/agreement_test.go
@@ -0,0 +1,110 @@
+// Copyright 2019 The dexon-consensus Authors
+// This file is part of the dexon-consensus library.
+//
+// The dexon-consensus library is free software: you can redistribute it
+// and/or modify it under the terms of the GNU Lesser General Public License as
+// published by the Free Software Foundation, either version 3 of the License,
+// or (at your option) any later version.  //
+// The dexon-consensus library is distributed in the hope that it will be
+// useful, but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser
+// General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public License
+// along with the dexon-consensus library. If not, see
+// <http://www.gnu.org/licenses/>.
+
+package syncer
+
+import (
+	"testing"
+	"time"
+
+	"github.com/dexon-foundation/dexon-consensus/common"
+	"github.com/dexon-foundation/dexon-consensus/core"
+	"github.com/dexon-foundation/dexon-consensus/core/crypto"
+	"github.com/dexon-foundation/dexon-consensus/core/test"
+	"github.com/dexon-foundation/dexon-consensus/core/types"
+	"github.com/dexon-foundation/dexon-consensus/core/utils"
+	"github.com/stretchr/testify/suite"
+)
+
+type AgreementTestSuite struct {
+	suite.Suite
+
+	signers []*utils.Signer
+	pubKeys []crypto.PublicKey
+	prvKeys []crypto.PrivateKey
+}
+
+func (s *AgreementTestSuite) SetupSuite() {
+	var err error
+	s.prvKeys, s.pubKeys, err = test.NewKeys(4)
+	s.Require().NoError(err)
+	for _, k := range s.prvKeys {
+		s.signers = append(s.signers, utils.NewSigner(k))
+	}
+}
+
+func (s *AgreementTestSuite) prepareAgreementResult(pos types.Position,
+	hash common.Hash) *types.AgreementResult {
+	votes := []types.Vote{}
+	for _, signer := range s.signers {
+		v := types.NewVote(types.VoteCom, hash, 0)
+		v.Position = pos
+		s.Require().NoError(signer.SignVote(v))
+		votes = append(votes, *v)
+	}
+	return &types.AgreementResult{
+		BlockHash: hash,
+		Position:  pos,
+		Votes:     votes,
+	}
+}
+
+func (s *AgreementTestSuite) prepareBlock(pos types.Position) *types.Block {
+	b := &types.Block{
+		Position: pos,
+	}
+	s.Require().NoError(s.signers[0].SignBlock(b))
+	return b
+}
+
+func (s *AgreementTestSuite) TestFutureAgreementResult() {
+	// Make sure future types.AgreementResult could be processed correctly
+	// when corresponding CRS is ready.
+	var (
+		futureRound = uint64(7)
+		pos         = types.Position{Round: 7, Height: 1000}
+	)
+	gov, err := test.NewGovernance(
+		test.NewState(s.pubKeys, time.Second, &common.NullLogger{}, true),
+		core.ConfigRoundShift,
+	)
+	s.Require().NoError(err)
+	// Make sure goverance is ready for some future round, including CRS.
+	gov.CatchUpWithRound(futureRound)
+	for i := uint64(1); i <= futureRound; i++ {
+		gov.ProposeCRS(i, common.NewRandomHash().Bytes())
+	}
+	s.Require().NoError(err)
+	blockChan := make(chan *types.Block, 10)
+	agr := newAgreement(blockChan, make(chan common.Hash, 100),
+		utils.NewNodeSetCache(gov), &common.SimpleLogger{})
+	go agr.run()
+	block := s.prepareBlock(pos)
+	result := s.prepareAgreementResult(pos, block.Hash)
+	agr.inputChan <- result
+	agr.inputChan <- block
+	agr.inputChan <- futureRound
+	select {
+	case confirmedBlock := <-blockChan:
+		s.Require().Equal(block.Hash, confirmedBlock.Hash)
+	case <-time.After(2 * time.Second):
+		s.Require().True(false)
+	}
+}
+
+func TestAgreement(t *testing.T) {
+	suite.Run(t, new(AgreementTestSuite))
+}
diff --git a/core/syncer/consensus.go b/core/syncer/consensus.go
index 566b3f4..7ba659f 100644
--- a/core/syncer/consensus.go
+++ b/core/syncer/consensus.go
@@ -73,6 +73,7 @@ type Consensus struct {
 	// lock for accessing all fields.
 	lock               sync.RWMutex
 	duringBuffering    bool
+	latestCRSRound     uint64
 	moduleWaitGroup    sync.WaitGroup
 	agreementWaitGroup sync.WaitGroup
 	pullChan           chan common.Hash
@@ -416,6 +417,11 @@ func (con *Consensus) cacheRandomnessResult(r *types.BlockRandomnessResult) {
 		if len(con.blocks) > 0 && r.Position.Older(con.blocks[0].Position) {
 			return true
 		}
+		if r.Position.Round > con.latestCRSRound {
+			// We can't process randomness from rounds that its CRS is still
+			// unknown.
+			return true
+		}
 		_, exists := con.randomnessResults[r.BlockHash]
 		return exists
 	}() {
@@ -486,6 +492,11 @@ func (con *Consensus) startCRSMonitor() {
 		}
 		con.logger.Debug("CRS is ready", "round", round)
 		lastNotifiedRound = round
+		func() {
+			con.lock.Lock()
+			defer con.lock.Unlock()
+			con.latestCRSRound = round
+		}()
 		for func() bool {
 			con.lock.RLock()
 			defer con.lock.RUnlock()