core: refine VerifyAgreementResult (#553)

* Pass notary set directly to VerifyAgreementResult

* Fix core.Consensus

* Fix syncer

4 files changed, 31 insertions, 18 deletions

diff --git a/core/consensus.go b/core/consensus.go
index ca7d10f..6c300c9 100644
--- a/core/consensus.go
+++ b/core/consensus.go
@@ -1338,7 +1338,11 @@ func (con *Consensus) ProcessAgreementResult(
 		return nil
 	}
 	// Sanity Check.
-	if err := VerifyAgreementResult(rand, con.nodeSetCache); err != nil {
+	notarySet, err := con.nodeSetCache.GetNotarySet(rand.Position.Round)
+	if err != nil {
+		return err
+	}
+	if err := VerifyAgreementResult(rand, notarySet); err != nil {
 		con.baMgr.untouchAgreementResult(rand)
 		return err
 	}
diff --git a/core/syncer/agreement.go b/core/syncer/agreement.go
index d39c246..b414e11 100644
--- a/core/syncer/agreement.go
+++ b/core/syncer/agreement.go
@@ -176,7 +176,12 @@ func (a *agreement) processAgreementResult(r *types.AgreementResult) {
 		a.logger.Trace("Agreement result cached", "result", r)
 		return
 	}
-	if err := core.VerifyAgreementResult(r, a.cache); err != nil {
+	notarySet, err := a.cache.GetNotarySet(r.Position.Round)
+	if err != nil {
+		a.logger.Error("unable to get notary set", "result", r, "error", err)
+		return
+	}
+	if err := core.VerifyAgreementResult(r, notarySet); err != nil {
 		a.logger.Error("Agreement result verification failed",
 			"result", r,
 			"error", err)
@@ -252,13 +257,18 @@ func (a *agreement) processNewCRS(round uint64) {
 	a.latestCRSRound = round
 	// Verify all pending results.
 	for r := prevRound; r <= a.latestCRSRound; r++ {
+		notarySet, err := a.cache.GetNotarySet(r)
+		if err != nil {
+			a.logger.Error("Unable to get notary set", "round", r, "error", err)
+			continue
+		}
 		pendingsForRound := a.pendingAgrs[r]
 		if pendingsForRound == nil {
 			continue
 		}
 		delete(a.pendingAgrs, r)
 		for _, res := range pendingsForRound {
-			if err := core.VerifyAgreementResult(res, a.cache); err != nil {
+			if err := core.VerifyAgreementResult(res, notarySet); err != nil {
 				a.logger.Error("Invalid agreement result",
 					"result", res,
 					"error", err)
diff --git a/core/utils.go b/core/utils.go
index 0250cf2..c9d5f84 100644
--- a/core/utils.go
+++ b/core/utils.go
@@ -158,17 +158,13 @@ func HashConfigurationBlock(
 // VerifyAgreementResult perform sanity check against a types.AgreementResult
 // instance.
 func VerifyAgreementResult(
-	res *types.AgreementResult, cache *utils.NodeSetCache) error {
+	res *types.AgreementResult, notarySet map[types.NodeID]struct{}) error {
 	if res.Position.Round >= DKGDelayRound {
 		if len(res.Randomness) == 0 {
 			return ErrMissingRandomness
 		}
 		return nil
 	}
-	notarySet, err := cache.GetNotarySet(res.Position.Round)
-	if err != nil {
-		return err
-	}
 	if len(res.Votes) < len(notarySet)*2/3+1 {
 		return ErrNotEnoughVotes
 	}
diff --git a/core/utils_test.go b/core/utils_test.go
index 560e923..e1bbb1e 100644
--- a/core/utils_test.go
+++ b/core/utils_test.go
@@ -71,52 +71,55 @@ func (s *UtilsTestSuite) TestVerifyAgreementResult() {
 		s.Require().NoError(signer.SignVote(vote))
 		baResult.Votes = append(baResult.Votes, *vote)
 	}
-	s.Require().NoError(VerifyAgreementResult(baResult, cache))
+	nSet, err := cache.GetNotarySet(pos.Round)
+	s.Require().NoError(err)
+	s.Require().NotEmpty(nSet)
+	s.Require().NoError(VerifyAgreementResult(baResult, nSet))
 
 	// Test negative case.
 	// All period should be the same.
 	baResult.Votes[1].Period++
-	s.Equal(ErrIncorrectVotePeriod, VerifyAgreementResult(baResult, cache))
+	s.Equal(ErrIncorrectVotePeriod, VerifyAgreementResult(baResult, nSet))
 	baResult.Votes[1].Period--
 
 	// Blockhash should match the one in votes.
 	baResult.BlockHash = common.NewRandomHash()
-	s.Equal(ErrIncorrectVoteBlockHash, VerifyAgreementResult(baResult, cache))
+	s.Equal(ErrIncorrectVoteBlockHash, VerifyAgreementResult(baResult, nSet))
 	baResult.BlockHash = hash
 
 	// Position should match.
 	baResult.Position.Height++
-	s.Equal(ErrIncorrectVotePosition, VerifyAgreementResult(baResult, cache))
+	s.Equal(ErrIncorrectVotePosition, VerifyAgreementResult(baResult, nSet))
 	baResult.Position = pos
 
 	// types.VotePreCom is not accepted in agreement result.
 	baResult.Votes[0].Type = types.VotePreCom
-	s.Equal(ErrIncorrectVoteType, VerifyAgreementResult(baResult, cache))
+	s.Equal(ErrIncorrectVoteType, VerifyAgreementResult(baResult, nSet))
 	baResult.Votes[0].Type = types.VoteCom
 
 	// Vote type should be the same.
 	baResult.Votes[1].Type = types.VoteFastCom
-	s.Equal(ErrIncorrectVoteType, VerifyAgreementResult(baResult, cache))
+	s.Equal(ErrIncorrectVoteType, VerifyAgreementResult(baResult, nSet))
 	baResult.Votes[1].Type = types.VoteCom
 
 	// Only vote proposed by notarySet is valid.
 	baResult.Votes[0].ProposerID = types.NodeID{Hash: common.NewRandomHash()}
-	s.Equal(ErrIncorrectVoteProposer, VerifyAgreementResult(baResult, cache))
+	s.Equal(ErrIncorrectVoteProposer, VerifyAgreementResult(baResult, nSet))
 	baResult.Votes[0].ProposerID = types.NewNodeID(pubKeys[0])
 
 	// Vote shuold have valid signature.
 	baResult.Votes[0].Signature, err = prvKeys[0].Sign(common.NewRandomHash())
 	s.Require().NoError(err)
-	s.Equal(ErrIncorrectVoteSignature, VerifyAgreementResult(baResult, cache))
+	s.Equal(ErrIncorrectVoteSignature, VerifyAgreementResult(baResult, nSet))
 	s.Require().NoError(signers[0].SignVote(&baResult.Votes[0]))
 
 	// Unique votes shuold be more than threshold.
 	baResult.Votes = baResult.Votes[:1]
-	s.Equal(ErrNotEnoughVotes, VerifyAgreementResult(baResult, cache))
+	s.Equal(ErrNotEnoughVotes, VerifyAgreementResult(baResult, nSet))
 	for range signers {
 		baResult.Votes = append(baResult.Votes, baResult.Votes[0])
 	}
-	s.Equal(ErrNotEnoughVotes, VerifyAgreementResult(baResult, cache))
+	s.Equal(ErrNotEnoughVotes, VerifyAgreementResult(baResult, nSet))
 }
 
 func TestUtils(t *testing.T) {