fix(lib/grandpa): check equivocatory votes count (#2497)

A valid justification can't have more than two equivocatory votes per
voter. Check the number of equivocatory votes per voter and throw an
error if more than two.

Fixes #2401

lib/grandpa/errors.go

@@ -98,4 +98,5 @@ var (
 	errVoteToSignatureMismatch = errors.New("votes and authority count mismatch")
 	errInvalidVoteBlock        = errors.New("block in vote is not descendant of previously finalised block")
 	errVoteFromSelf            = errors.New("got vote from ourselves")
+	errInvalidMultiplicity     = errors.New("more than two equivocatory votes for a voter")
 )

lib/grandpa/message_handler.go

@@ -284,27 +284,33 @@ func (h *MessageHandler) verifyCatchUpResponseCompletability(prevote, precommit
 	return nil
 }
 
-func getEquivocatoryVoters(votes []AuthData) map[ed25519.PublicKeyBytes]struct{} {
+func getEquivocatoryVoters(votes []AuthData) (map[ed25519.PublicKeyBytes]struct{}, error) {
 	eqvVoters := make(map[ed25519.PublicKeyBytes]struct{})
 	voters := make(map[ed25519.PublicKeyBytes]int, len(votes))
 
 	for _, v := range votes {
 		voters[v.AuthorityID]++
-
-		if voters[v.AuthorityID] > 1 {
+		switch voters[v.AuthorityID] {
+		case 1:
+		case 2:
 			eqvVoters[v.AuthorityID] = struct{}{}
+		default:
+			return nil, fmt.Errorf("%w: authority id %x has %d votes",
+				errInvalidMultiplicity, v.AuthorityID, voters[v.AuthorityID])
 		}
 	}
-
-	return eqvVoters
+	return eqvVoters, nil
 }
 
 func (h *MessageHandler) verifyCommitMessageJustification(fm *CommitMessage) error {
 	if len(fm.Precommits) != len(fm.AuthData) {
 		return ErrPrecommitSignatureMismatch
 	}
 
-	eqvVoters := getEquivocatoryVoters(fm.AuthData)
+	eqvVoters, err := getEquivocatoryVoters(fm.AuthData)
+	if err != nil {
+		return fmt.Errorf("could not get valid equivocatory voters: %w", err)
+	}
 
 	var count int
 	for i, pc := range fm.Precommits {
@@ -436,7 +442,10 @@ func (h *MessageHandler) verifyPreCommitJustification(msg *CatchUpResponse) erro
 		auths[i] = AuthData{AuthorityID: pcj.AuthorityID}
 	}
 
-	eqvVoters := getEquivocatoryVoters(auths)
+	eqvVoters, err := getEquivocatoryVoters(auths)
+	if err != nil {
+		return fmt.Errorf("could not get valid equivocatory voters: %w", err)
+	}
 
 	// verify pre-commit justification
 	var count uint64
@@ -549,7 +558,11 @@ func (s *Service) VerifyBlockJustification(hash common.Hash, justification []byt
 		authPubKeys[i] = AuthData{AuthorityID: pcj.AuthorityID}
 	}
 
-	equivocatoryVoters := getEquivocatoryVoters(authPubKeys)
+	equivocatoryVoters, err := getEquivocatoryVoters(authPubKeys)
+	if err != nil {
+		return fmt.Errorf("could not get valid equivocatory voters: %w", err)
+	}
+
 	var count int
 
 	logger.Debugf(

lib/grandpa/message_handler_test.go

@@ -649,7 +649,7 @@ func TestMessageHandler_VerifyBlockJustification_WithEquivocatoryVotes(t *testin
 
 	round := uint64(1)
 	number := uint32(1)
-	precommits := buildTestJustification(t, 20, round, setID, kr, precommit)
+	precommits := buildTestJustification(t, 18, round, setID, kr, precommit)
 	just := newJustification(round, testHash, number, precommits)
 	data, err := scale.Marshal(*just)
 	require.NoError(t, err)
@@ -788,13 +788,11 @@ func Test_getEquivocatoryVoters(t *testing.T) {
 	ed25519Keyring, err := keystore.NewEd25519Keyring()
 	require.NoError(t, err)
 	fakeAuthorities := []*ed25519.Keypair{
-		ed25519Keyring.Alice().(*ed25519.Keypair),
 		ed25519Keyring.Alice().(*ed25519.Keypair),
 		ed25519Keyring.Alice().(*ed25519.Keypair),
 		ed25519Keyring.Bob().(*ed25519.Keypair),
 		ed25519Keyring.Charlie().(*ed25519.Keypair),
 		ed25519Keyring.Charlie().(*ed25519.Keypair),
-		ed25519Keyring.Charlie().(*ed25519.Keypair),
 		ed25519Keyring.Dave().(*ed25519.Keypair),
 		ed25519Keyring.Dave().(*ed25519.Keypair),
 		ed25519Keyring.Eve().(*ed25519.Keypair),
@@ -813,8 +811,17 @@ func Test_getEquivocatoryVoters(t *testing.T) {
 		}
 	}
 
-	eqv := getEquivocatoryVoters(authData)
+	eqv, err := getEquivocatoryVoters(authData)
+	require.NoError(t, err)
 	require.Len(t, eqv, 5)
+
+	// test that getEquivocatoryVoters returns an error if a voter has more than two equivocatory votes
+	authData = append(authData, AuthData{
+		AuthorityID: ed25519Keyring.Alice().Public().(*ed25519.PublicKey).AsBytes(),
+	})
+
+	_, err = getEquivocatoryVoters(authData)
+	require.ErrorIs(t, err, errInvalidMultiplicity)
 }
 
 func Test_VerifyCommitMessageJustification_ShouldRemoveEquivocatoryVotes(t *testing.T) {
@@ -838,13 +845,11 @@ func Test_VerifyCommitMessageJustification_ShouldRemoveEquivocatoryVotes(t *test
 	ed25519Keyring, err := keystore.NewEd25519Keyring()
 	require.NoError(t, err)
 	fakeAuthorities := []*ed25519.Keypair{
-		ed25519Keyring.Alice().(*ed25519.Keypair),
 		ed25519Keyring.Alice().(*ed25519.Keypair),
 		ed25519Keyring.Alice().(*ed25519.Keypair),
 		ed25519Keyring.Bob().(*ed25519.Keypair),
 		ed25519Keyring.Charlie().(*ed25519.Keypair),
 		ed25519Keyring.Charlie().(*ed25519.Keypair),
-		ed25519Keyring.Charlie().(*ed25519.Keypair),
 		ed25519Keyring.Dave().(*ed25519.Keypair),
 		ed25519Keyring.Dave().(*ed25519.Keypair),
 		ed25519Keyring.Eve().(*ed25519.Keypair),
@@ -989,13 +994,11 @@ func Test_VerifyPreCommitJustification(t *testing.T) {
 	// total of votes 4 legit + 3 equivocatory
 	// the threshold for testing is 9, so 2/3 of 9 = 6
 	fakeAuthorities := []*ed25519.Keypair{
-		ed25519Keyring.Alice().(*ed25519.Keypair),
 		ed25519Keyring.Alice().(*ed25519.Keypair),
 		ed25519Keyring.Alice().(*ed25519.Keypair),
 		ed25519Keyring.Bob().(*ed25519.Keypair),
 		ed25519Keyring.Charlie().(*ed25519.Keypair),
 		ed25519Keyring.Charlie().(*ed25519.Keypair),
-		ed25519Keyring.Charlie().(*ed25519.Keypair),
 		ed25519Keyring.Dave().(*ed25519.Keypair),
 		ed25519Keyring.Dave().(*ed25519.Keypair),
 		ed25519Keyring.Eve().(*ed25519.Keypair),