diff --git a/lib/grandpa/errors.go b/lib/grandpa/errors.go index 705316dc8f..1908f785cf 100644 --- a/lib/grandpa/errors.go +++ b/lib/grandpa/errors.go @@ -96,7 +96,7 @@ var ( ErrAuthorityNotInSet = errors.New("authority is not in set") errVoteToSignatureMismatch = errors.New("votes and authority count mismatch") - errInvalidVoteBlock = errors.New("block in vote is not descendant of previously finalised block") + errVoteBlockMismatch = 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") ) diff --git a/lib/grandpa/message_handler.go b/lib/grandpa/message_handler.go index f0a7662326..6a5241694c 100644 --- a/lib/grandpa/message_handler.go +++ b/lib/grandpa/message_handler.go @@ -302,11 +302,33 @@ func getEquivocatoryVoters(votes []AuthData) (map[ed25519.PublicKeyBytes]struct{ return eqvVoters, nil } +func isDescendantOfHighestFinalisedBlock(blockState BlockState, hash common.Hash) (bool, error) { + highestHeader, err := blockState.GetHighestFinalisedHeader() + if err != nil { + return false, fmt.Errorf("could not get highest finalised header: %w", err) + } + + return blockState.IsDescendantOf(highestHeader.Hash(), hash) +} + func (h *MessageHandler) verifyCommitMessageJustification(fm *CommitMessage) error { if len(fm.Precommits) != len(fm.AuthData) { return ErrPrecommitSignatureMismatch } + if fm.SetID != h.grandpa.state.setID { + return fmt.Errorf("%w: grandpa state set id %d, set id in the commit message %d", + ErrSetIDMismatch, h.grandpa.state.setID, fm.SetID) + } + + isDescendant, err := isDescendantOfHighestFinalisedBlock(h.blockState, fm.Vote.Hash) + if err != nil { + return fmt.Errorf("cannot verify ancestry of highest finalised block: %w", err) + } + if !isDescendant { + return errVoteBlockMismatch + } + eqvVoters, err := getEquivocatoryVoters(fm.AuthData) if err != nil { return fmt.Errorf("could not get valid equivocatory voters: %w", err) @@ -314,11 +336,6 @@ func (h *MessageHandler) verifyCommitMessageJustification(fm *CommitMessage) err var count int for i, pc := range fm.Precommits { - _, ok := eqvVoters[fm.AuthData[i].AuthorityID] - if ok { - continue - } - just := &SignedVote{ Vote: pc, Signature: fm.AuthData[i].Signature, @@ -327,7 +344,8 @@ func (h *MessageHandler) verifyCommitMessageJustification(fm *CommitMessage) err err := h.verifyJustification(just, fm.Round, h.grandpa.state.setID, precommit) if err != nil { - logger.Errorf("could not verify justification: %s", err) + logger.Errorf("failed to verify justification for vote from authority id %s, for block hash %s: %s", + just.AuthorityID.String(), just.Vote.Hash, err) continue } @@ -337,6 +355,15 @@ func (h *MessageHandler) verifyCommitMessageJustification(fm *CommitMessage) err continue } + err = verifyBlockHashAgainstBlockNumber(h.blockState, pc.Hash, uint(pc.Number)) + if err != nil { + return err + } + + if _, ok := eqvVoters[fm.AuthData[i].AuthorityID]; ok { + continue + } + if isDescendant { count++ } @@ -425,23 +452,19 @@ func (h *MessageHandler) verifyPreVoteJustification(msg *CatchUpResponse) (commo } func (h *MessageHandler) verifyPreCommitJustification(msg *CatchUpResponse) error { - for _, pcj := range msg.PreCommitJustification { - err := verifyBlockHashAgainstBlockNumber(h.blockState, pcj.Vote.Hash, uint(pcj.Vote.Number)) - if err != nil { - if errors.Is(err, chaindb.ErrKeyNotFound) { - h.grandpa.tracker.addCatchUpResponse(msg) - logger.Infof("we might not have synced to the given block %s yet: %s", pcj.Vote.Hash, err) - continue - } - return err - } - } - auths := make([]AuthData, len(msg.PreCommitJustification)) for i, pcj := range msg.PreCommitJustification { auths[i] = AuthData{AuthorityID: pcj.AuthorityID} } + isDescendant, err := isDescendantOfHighestFinalisedBlock(h.blockState, msg.Hash) + if err != nil { + return err + } + if !isDescendant { + return errVoteBlockMismatch + } + eqvVoters, err := getEquivocatoryVoters(auths) if err != nil { return fmt.Errorf("could not get valid equivocatory voters: %w", err) @@ -452,12 +475,24 @@ func (h *MessageHandler) verifyPreCommitJustification(msg *CatchUpResponse) erro for idx := range msg.PreCommitJustification { just := &msg.PreCommitJustification[idx] - if _, ok := eqvVoters[just.AuthorityID]; ok { - continue + err = verifyBlockHashAgainstBlockNumber(h.blockState, just.Vote.Hash, uint(just.Vote.Number)) + if err != nil { + if errors.Is(err, chaindb.ErrKeyNotFound) { + h.grandpa.tracker.addCatchUpResponse(msg) + logger.Infof("we might not have synced to the given block %s yet: %s", just.Vote.Hash, err) + continue + } + return err } err := h.verifyJustification(just, msg.Round, msg.SetID, precommit) if err != nil { + logger.Errorf("could not verify precommit justification for block %s from authority %s: %s", + just.Vote.Hash.String(), just.AuthorityID.String(), err) + continue + } + + if _, ok := eqvVoters[just.AuthorityID]; ok { continue } @@ -540,6 +575,15 @@ func (s *Service) VerifyBlockJustification(hash common.Hash, justification []byt return fmt.Errorf("already have finalised block with setID=%d and round=%d", setID, fj.Round) } + isDescendant, err := isDescendantOfHighestFinalisedBlock(s.blockState, fj.Commit.Hash) + if err != nil { + return err + } + + if !isDescendant { + return errVoteBlockMismatch + } + auths, err := s.grandpaState.GetAuthorities(setID) if err != nil { return fmt.Errorf("cannot get authorities for set ID: %w", err) @@ -570,10 +614,6 @@ func (s *Service) VerifyBlockJustification(hash common.Hash, justification []byt setID, fj.Round, fj.Commit.Hash, fj.Commit.Number, len(fj.Commit.Precommits)) for _, just := range fj.Commit.Precommits { - if _, ok := equivocatoryVoters[just.AuthorityID]; ok { - continue - } - // check if vote was for descendant of committed block isDescendant, err := s.blockState.IsDescendantOf(hash, just.Vote.Hash) if err != nil { @@ -613,6 +653,10 @@ func (s *Service) VerifyBlockJustification(hash common.Hash, justification []byt return ErrInvalidSignature } + if _, ok := equivocatoryVoters[just.AuthorityID]; ok { + continue + } + count++ } diff --git a/lib/grandpa/state.go b/lib/grandpa/state.go index 63217856aa..c6b546316b 100644 --- a/lib/grandpa/state.go +++ b/lib/grandpa/state.go @@ -26,6 +26,7 @@ type BlockState interface { BestBlockHeader() (*types.Header, error) BestBlockHash() common.Hash Leaves() []common.Hash + GetHighestFinalisedHeader() (*types.Header, error) BlocktreeAsString() string GetImportedBlockNotifierChannel() chan *types.Block FreeImportedBlockNotifierChannel(ch chan *types.Block) diff --git a/lib/grandpa/vote_message.go b/lib/grandpa/vote_message.go index b72291cd64..f2849ab09c 100644 --- a/lib/grandpa/vote_message.go +++ b/lib/grandpa/vote_message.go @@ -283,7 +283,7 @@ func (s *Service) validateVote(v *Vote) error { } if !isDescendant { - return errInvalidVoteBlock + return errVoteBlockMismatch } return nil diff --git a/lib/grandpa/vote_message_test.go b/lib/grandpa/vote_message_test.go index a6f9df79db..e1773613d7 100644 --- a/lib/grandpa/vote_message_test.go +++ b/lib/grandpa/vote_message_test.go @@ -370,5 +370,5 @@ func TestValidateMessage_IsNotDescendant(t *testing.T) { gs.keypair = kr.Bob().(*ed25519.Keypair) _, err = gs.validateVoteMessage("", msg) - require.Equal(t, errInvalidVoteBlock, err, gs.prevotes) + require.Equal(t, errVoteBlockMismatch, err, gs.prevotes) }