Skip to content

Commit

Permalink
Revert "fix: don't verify evidence signatures on non-validators"
Browse files Browse the repository at this point in the history
This reverts commit fe41a12.

We cannot just skip verifying evidence signatures for non-validators, as this
would allow non-validators to add invalid evidence to the pool.
  • Loading branch information
lklimek committed Aug 12, 2024
1 parent af39480 commit 87ed796
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 19 deletions.
29 changes: 12 additions & 17 deletions internal/evidence/verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"context"
"fmt"

"github.com/dashpay/tenderdash/libs/log"
"github.com/dashpay/tenderdash/types"
)

Expand Down Expand Up @@ -65,7 +64,7 @@ func (evpool *Pool) verify(ctx context.Context, evidence types.Evidence) error {
return err
}

if err := VerifyDuplicateVote(ev, state.ChainID, valSet, evpool.logger); err != nil {
if err := VerifyDuplicateVote(ev, state.ChainID, valSet); err != nil {
return types.NewErrInvalidEvidence(evidence, err)
}

Expand All @@ -91,15 +90,15 @@ func (evpool *Pool) verify(ctx context.Context, evidence types.Evidence) error {
// - the validator is in the validator set at the height of the evidence
// - the height, round, type and validator address of the votes must be the same
// - the block ID's must be different
// - The signatures must both be valid (only if we have public keys of the validator set)
func VerifyDuplicateVote(e *types.DuplicateVoteEvidence, chainID string, valSet *types.ValidatorSet, logger log.Logger) error {
// - The signatures must both be valid
func VerifyDuplicateVote(e *types.DuplicateVoteEvidence, chainID string, valSet *types.ValidatorSet) error {
_, val := valSet.GetByProTxHash(e.VoteA.ValidatorProTxHash)
if val == nil {
return fmt.Errorf("protxhash %X was not a validator at height %d", e.VoteA.ValidatorProTxHash, e.Height())
}
proTxHash := val.ProTxHash
pubKey := val.PubKey
if valSet.HasPublicKeys && pubKey == nil {
if pubKey == nil {
return fmt.Errorf("we don't have a public key of validator %X at height %d", proTxHash, e.Height())
}

Expand Down Expand Up @@ -135,18 +134,14 @@ func VerifyDuplicateVote(e *types.DuplicateVoteEvidence, chainID string, valSet
voteProTxHash, proTxHash)
}

if pubKey != nil {
va := e.VoteA.ToProto()
vb := e.VoteB.ToProto()
// Signatures must be valid
if !pubKey.VerifySignatureDigest(types.VoteBlockSignID(chainID, va, valSet.QuorumType, valSet.QuorumHash), e.VoteA.BlockSignature) {
return fmt.Errorf("verifying VoteA: %w", types.ErrVoteInvalidBlockSignature)
}
if !pubKey.VerifySignatureDigest(types.VoteBlockSignID(chainID, vb, valSet.QuorumType, valSet.QuorumHash), e.VoteB.BlockSignature) {
return fmt.Errorf("verifying VoteB: %w", types.ErrVoteInvalidBlockSignature)
}
} else {
logger.Debug("skipping verification of duplicate vote evidence signatures - no access public key", "evidence", e)
va := e.VoteA.ToProto()
vb := e.VoteB.ToProto()
// Signatures must be valid
if !pubKey.VerifySignatureDigest(types.VoteBlockSignID(chainID, va, valSet.QuorumType, valSet.QuorumHash), e.VoteA.BlockSignature) {
return fmt.Errorf("verifying VoteA: %w", types.ErrVoteInvalidBlockSignature)
}
if !pubKey.VerifySignatureDigest(types.VoteBlockSignID(chainID, vb, valSet.QuorumType, valSet.QuorumHash), e.VoteB.BlockSignature) {
return fmt.Errorf("verifying VoteB: %w", types.ErrVoteInvalidBlockSignature)
}

return nil
Expand Down
4 changes: 2 additions & 2 deletions internal/evidence/verify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ func TestVerifyDuplicateVoteEvidence(t *testing.T) {
Timestamp: defaultEvidenceTime,
}
if c.valid {
assert.Nil(t, evidence.VerifyDuplicateVote(ev, chainID, valSet, logger), "evidence should be valid")
assert.Nil(t, evidence.VerifyDuplicateVote(ev, chainID, valSet), "evidence should be valid")
} else {
assert.NotNil(t, evidence.VerifyDuplicateVote(ev, chainID, valSet, logger), "evidence should be invalid")
assert.NotNil(t, evidence.VerifyDuplicateVote(ev, chainID, valSet), "evidence should be invalid")
}
}

Expand Down

0 comments on commit 87ed796

Please sign in to comment.