Skip to content

Commit 014629d

Browse files
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
1 parent fa3a2e3 commit 014629d

File tree

3 files changed

+33
-16
lines changed

3 files changed

+33
-16
lines changed

lib/grandpa/errors.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,4 +98,5 @@ var (
9898
errVoteToSignatureMismatch = errors.New("votes and authority count mismatch")
9999
errInvalidVoteBlock = errors.New("block in vote is not descendant of previously finalised block")
100100
errVoteFromSelf = errors.New("got vote from ourselves")
101+
errInvalidMultiplicity = errors.New("more than two equivocatory votes for a voter")
101102
)

lib/grandpa/message_handler.go

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -284,27 +284,33 @@ func (h *MessageHandler) verifyCatchUpResponseCompletability(prevote, precommit
284284
return nil
285285
}
286286

287-
func getEquivocatoryVoters(votes []AuthData) map[ed25519.PublicKeyBytes]struct{} {
287+
func getEquivocatoryVoters(votes []AuthData) (map[ed25519.PublicKeyBytes]struct{}, error) {
288288
eqvVoters := make(map[ed25519.PublicKeyBytes]struct{})
289289
voters := make(map[ed25519.PublicKeyBytes]int, len(votes))
290290

291291
for _, v := range votes {
292292
voters[v.AuthorityID]++
293-
294-
if voters[v.AuthorityID] > 1 {
293+
switch voters[v.AuthorityID] {
294+
case 1:
295+
case 2:
295296
eqvVoters[v.AuthorityID] = struct{}{}
297+
default:
298+
return nil, fmt.Errorf("%w: authority id %x has %d votes",
299+
errInvalidMultiplicity, v.AuthorityID, voters[v.AuthorityID])
296300
}
297301
}
298-
299-
return eqvVoters
302+
return eqvVoters, nil
300303
}
301304

302305
func (h *MessageHandler) verifyCommitMessageJustification(fm *CommitMessage) error {
303306
if len(fm.Precommits) != len(fm.AuthData) {
304307
return ErrPrecommitSignatureMismatch
305308
}
306309

307-
eqvVoters := getEquivocatoryVoters(fm.AuthData)
310+
eqvVoters, err := getEquivocatoryVoters(fm.AuthData)
311+
if err != nil {
312+
return fmt.Errorf("could not get valid equivocatory voters: %w", err)
313+
}
308314

309315
var count int
310316
for i, pc := range fm.Precommits {
@@ -436,7 +442,10 @@ func (h *MessageHandler) verifyPreCommitJustification(msg *CatchUpResponse) erro
436442
auths[i] = AuthData{AuthorityID: pcj.AuthorityID}
437443
}
438444

439-
eqvVoters := getEquivocatoryVoters(auths)
445+
eqvVoters, err := getEquivocatoryVoters(auths)
446+
if err != nil {
447+
return fmt.Errorf("could not get valid equivocatory voters: %w", err)
448+
}
440449

441450
// verify pre-commit justification
442451
var count uint64
@@ -549,7 +558,11 @@ func (s *Service) VerifyBlockJustification(hash common.Hash, justification []byt
549558
authPubKeys[i] = AuthData{AuthorityID: pcj.AuthorityID}
550559
}
551560

552-
equivocatoryVoters := getEquivocatoryVoters(authPubKeys)
561+
equivocatoryVoters, err := getEquivocatoryVoters(authPubKeys)
562+
if err != nil {
563+
return fmt.Errorf("could not get valid equivocatory voters: %w", err)
564+
}
565+
553566
var count int
554567

555568
logger.Debugf(

lib/grandpa/message_handler_test.go

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -649,7 +649,7 @@ func TestMessageHandler_VerifyBlockJustification_WithEquivocatoryVotes(t *testin
649649

650650
round := uint64(1)
651651
number := uint32(1)
652-
precommits := buildTestJustification(t, 20, round, setID, kr, precommit)
652+
precommits := buildTestJustification(t, 18, round, setID, kr, precommit)
653653
just := newJustification(round, testHash, number, precommits)
654654
data, err := scale.Marshal(*just)
655655
require.NoError(t, err)
@@ -788,13 +788,11 @@ func Test_getEquivocatoryVoters(t *testing.T) {
788788
ed25519Keyring, err := keystore.NewEd25519Keyring()
789789
require.NoError(t, err)
790790
fakeAuthorities := []*ed25519.Keypair{
791-
ed25519Keyring.Alice().(*ed25519.Keypair),
792791
ed25519Keyring.Alice().(*ed25519.Keypair),
793792
ed25519Keyring.Alice().(*ed25519.Keypair),
794793
ed25519Keyring.Bob().(*ed25519.Keypair),
795794
ed25519Keyring.Charlie().(*ed25519.Keypair),
796795
ed25519Keyring.Charlie().(*ed25519.Keypair),
797-
ed25519Keyring.Charlie().(*ed25519.Keypair),
798796
ed25519Keyring.Dave().(*ed25519.Keypair),
799797
ed25519Keyring.Dave().(*ed25519.Keypair),
800798
ed25519Keyring.Eve().(*ed25519.Keypair),
@@ -813,8 +811,17 @@ func Test_getEquivocatoryVoters(t *testing.T) {
813811
}
814812
}
815813

816-
eqv := getEquivocatoryVoters(authData)
814+
eqv, err := getEquivocatoryVoters(authData)
815+
require.NoError(t, err)
817816
require.Len(t, eqv, 5)
817+
818+
// test that getEquivocatoryVoters returns an error if a voter has more than two equivocatory votes
819+
authData = append(authData, AuthData{
820+
AuthorityID: ed25519Keyring.Alice().Public().(*ed25519.PublicKey).AsBytes(),
821+
})
822+
823+
_, err = getEquivocatoryVoters(authData)
824+
require.ErrorIs(t, err, errInvalidMultiplicity)
818825
}
819826

820827
func Test_VerifyCommitMessageJustification_ShouldRemoveEquivocatoryVotes(t *testing.T) {
@@ -838,13 +845,11 @@ func Test_VerifyCommitMessageJustification_ShouldRemoveEquivocatoryVotes(t *test
838845
ed25519Keyring, err := keystore.NewEd25519Keyring()
839846
require.NoError(t, err)
840847
fakeAuthorities := []*ed25519.Keypair{
841-
ed25519Keyring.Alice().(*ed25519.Keypair),
842848
ed25519Keyring.Alice().(*ed25519.Keypair),
843849
ed25519Keyring.Alice().(*ed25519.Keypair),
844850
ed25519Keyring.Bob().(*ed25519.Keypair),
845851
ed25519Keyring.Charlie().(*ed25519.Keypair),
846852
ed25519Keyring.Charlie().(*ed25519.Keypair),
847-
ed25519Keyring.Charlie().(*ed25519.Keypair),
848853
ed25519Keyring.Dave().(*ed25519.Keypair),
849854
ed25519Keyring.Dave().(*ed25519.Keypair),
850855
ed25519Keyring.Eve().(*ed25519.Keypair),
@@ -989,13 +994,11 @@ func Test_VerifyPreCommitJustification(t *testing.T) {
989994
// total of votes 4 legit + 3 equivocatory
990995
// the threshold for testing is 9, so 2/3 of 9 = 6
991996
fakeAuthorities := []*ed25519.Keypair{
992-
ed25519Keyring.Alice().(*ed25519.Keypair),
993997
ed25519Keyring.Alice().(*ed25519.Keypair),
994998
ed25519Keyring.Alice().(*ed25519.Keypair),
995999
ed25519Keyring.Bob().(*ed25519.Keypair),
9961000
ed25519Keyring.Charlie().(*ed25519.Keypair),
9971001
ed25519Keyring.Charlie().(*ed25519.Keypair),
998-
ed25519Keyring.Charlie().(*ed25519.Keypair),
9991002
ed25519Keyring.Dave().(*ed25519.Keypair),
10001003
ed25519Keyring.Dave().(*ed25519.Keypair),
10011004
ed25519Keyring.Eve().(*ed25519.Keypair),

0 commit comments

Comments
 (0)