From 826a7150b754ed7e138f17983a67183060c9aaf8 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Thu, 7 May 2020 13:49:51 +0400 Subject: [PATCH] types: remove extra validation in VerifyCommit plus make sure LastCommit is always non-nil --- state/validation.go | 9 +++------ types/block.go | 6 +++--- types/validator_set.go | 15 ++++++++++----- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/state/validation.go b/state/validation.go index 221927af3..6d2a7eb4d 100644 --- a/state/validation.go +++ b/state/validation.go @@ -86,12 +86,9 @@ func validateBlock(evidencePool EvidencePool, stateDB dbm.DB, state State, block return errors.New("block at height 1 can't have LastCommit signatures") } } else { - if len(block.LastCommit.Signatures) != state.LastValidators.Size() { - return types.NewErrInvalidCommitSignatures(state.LastValidators.Size(), len(block.LastCommit.Signatures)) - } - err := state.LastValidators.VerifyCommit( - state.ChainID, state.LastBlockID, block.Height-1, block.LastCommit) - if err != nil { + // LastCommit.Signatures length is checked in VerifyCommit. + if err := state.LastValidators.VerifyCommit( + state.ChainID, state.LastBlockID, block.Height-1, block.LastCommit); err != nil { return err } } diff --git a/types/block.go b/types/block.go index 8fa02e298..7e112e5e5 100644 --- a/types/block.go +++ b/types/block.go @@ -60,10 +60,10 @@ func (b *Block) ValidateBasic() error { } // Validate the last commit and its hash. + if b.LastCommit == nil { + return errors.New("nil LastCommit") + } if b.Header.Height > 1 { - if b.LastCommit == nil { - return errors.New("nil LastCommit") - } if err := b.LastCommit.ValidateBasic(); err != nil { return fmt.Errorf("wrong LastCommit: %v", err) } diff --git a/types/validator_set.go b/types/validator_set.go index 63573e90f..3e4be2c97 100644 --- a/types/validator_set.go +++ b/types/validator_set.go @@ -642,8 +642,13 @@ func (vals *ValidatorSet) VerifyCommit(chainID string, blockID BlockID, return NewErrInvalidCommitSignatures(vals.Size(), len(commit.Signatures)) } - if err := commit.ValidateBasic(); err != nil { - return err + // Validate Height and BlockID. + if height != commit.Height { + return NewErrInvalidCommitHeight(height, commit.Height) + } + if !blockID.Equals(commit.BlockID) { + return fmt.Errorf("invalid commit -- wrong block ID: want %v, got %v", + blockID, commit.BlockID) } if height != commit.Height { return NewErrInvalidCommitHeight(height, commit.Height) @@ -670,12 +675,12 @@ func (vals *ValidatorSet) VerifyCommit(chainID string, blockID BlockID, return fmt.Errorf("wrong signature (#%d): %X", idx, commitSig.Signature) } // Good! - if blockID.Equals(commitSig.BlockID(commit.BlockID)) { + if commitSig.ForBlock() { talliedVotingPower += val.VotingPower } // else { - // It's OK that the BlockID doesn't match. We include stray - // signatures (~votes for nil) to measure validator availability. + // It's OK. We include stray signatures (~votes for nil) to measure + // validator availability. // } // return as soon as +2/3 of the signatures are verified