diff --git a/abci/example/kvstore/verify.go b/abci/example/kvstore/verify.go index e52116b5eb..33408712fd 100644 --- a/abci/example/kvstore/verify.go +++ b/abci/example/kvstore/verify.go @@ -17,7 +17,6 @@ func (app *Application) verifyBlockCommit(qsd types.QuorumSignData, commit abci. if !bytes.Equal(commit.QuorumHash, vsu.QuorumHash) { return fmt.Errorf("mismatch quorum hashes got %X, want %X", commit.QuorumHash, vsu.QuorumHash) } - verifier := types.NewQuorumSignsVerifier(qsd) pubKey, err := encoding.PubKeyFromProto(vsu.ThresholdPublicKey) if err != nil { return err @@ -28,7 +27,7 @@ func (app *Application) verifyBlockCommit(qsd types.QuorumSignData, commit abci. extSigs = append(extSigs, ext.Signature) } - return verifier.Verify(pubKey, types.QuorumSigns{ + return qsd.Verify(pubKey, types.QuorumSigns{ BlockSign: commit.BlockSignature, VoteExtensionSignatures: extSigs, }) diff --git a/internal/consensus/state_test.go b/internal/consensus/state_test.go index cb90c58b7f..8a5e5a5e9d 100644 --- a/internal/consensus/state_test.go +++ b/internal/consensus/state_test.go @@ -3334,12 +3334,7 @@ func mockProposerApplicationCalls(t *testing.T, m *abcimocks.Application, round if final { m.On("ExtendVote", mock.Anything, roundMatcher). - Return(&abci.ResponseExtendVote{ - VoteExtensions: []*abci.ExtendVoteExtension{{ - Type: tmproto.VoteExtensionType_THRESHOLD_RECOVER, - Extension: []byte("extension"), - }}, - }, nil).Once() + Return(&abci.ResponseExtendVote{}, nil).Once() m.On("VerifyVoteExtension", mock.Anything, roundMatcher). Return(&abci.ResponseVerifyVoteExtension{ diff --git a/types/block.go b/types/block.go index 434c1a3fae..f6a1554cbf 100644 --- a/types/block.go +++ b/types/block.go @@ -790,10 +790,11 @@ func (commit *Commit) ToCommitInfo() types.CommitInfo { // GetCanonicalVote returns the message that is being voted on in the form of a vote without signatures. func (commit *Commit) GetCanonicalVote() *Vote { return &Vote{ - Type: tmproto.PrecommitType, - Height: commit.Height, - Round: commit.Round, - BlockID: commit.BlockID, + Type: tmproto.PrecommitType, + Height: commit.Height, + Round: commit.Round, + BlockID: commit.BlockID, + VoteExtensions: VoteExtensionsFromProto(commit.ThresholdVoteExtensions...), } } diff --git a/types/quorum.go b/types/quorum.go index 367e28f9e2..527c19fb34 100644 --- a/types/quorum.go +++ b/types/quorum.go @@ -3,9 +3,6 @@ package types import ( "bytes" "fmt" - - "github.com/dashpay/tenderdash/crypto" - "github.com/dashpay/tenderdash/libs/log" ) // CommitSigns is used to combine threshold signatures and quorum-hash that were used @@ -51,113 +48,3 @@ func NewQuorumSignsFromCommit(commit *Commit) QuorumSigns { VoteExtensionSignatures: sigs, } } - -// QuorumSingsVerifier ... -type QuorumSingsVerifier struct { - QuorumSignData - shouldVerifyBlock bool - shouldVerifyVoteExtensions bool - logger log.Logger -} - -// WithVerifyExtensions sets a flag that tells QuorumSingsVerifier to verify vote-extension signatures or not -func WithVerifyExtensions(shouldVerify bool) func(*QuorumSingsVerifier) { - return func(verifier *QuorumSingsVerifier) { - verifier.shouldVerifyVoteExtensions = shouldVerify - } -} - -// WithVerifyBlock sets a flag that tells QuorumSingsVerifier to verify block signature or not -func WithVerifyBlock(shouldVerify bool) func(*QuorumSingsVerifier) { - return func(verifier *QuorumSingsVerifier) { - verifier.shouldVerifyBlock = shouldVerify - } -} - -// WithVerifyReachedQuorum sets a flag that tells QuorumSingsVerifier to verify -// vote-extension and stateID signatures or not -func WithVerifyReachedQuorum(quorumReached bool) func(*QuorumSingsVerifier) { - return func(verifier *QuorumSingsVerifier) { - verifier.shouldVerifyVoteExtensions = quorumReached - } -} - -// WithLogger sets a logger -func WithLogger(logger log.Logger) func(*QuorumSingsVerifier) { - return func(verifier *QuorumSingsVerifier) { - verifier.logger = logger - } -} - -// NewQuorumSignsVerifier creates and returns an instance of QuorumSingsVerifier that is used for verification -// quorum signatures -func NewQuorumSignsVerifier(quorumData QuorumSignData, opts ...func(*QuorumSingsVerifier)) *QuorumSingsVerifier { - verifier := &QuorumSingsVerifier{ - QuorumSignData: quorumData, - shouldVerifyBlock: true, - shouldVerifyVoteExtensions: true, - logger: log.NewNopLogger(), - } - for _, opt := range opts { - opt(verifier) - } - return verifier -} - -// Verify verifies quorum data using public key and passed signatures -func (q *QuorumSingsVerifier) Verify(pubKey crypto.PubKey, signs QuorumSigns) error { - err := q.verifyBlock(pubKey, signs) - if err != nil { - return err - } - return q.verifyVoteExtensions(pubKey, signs) -} - -func (q *QuorumSingsVerifier) verifyBlock(pubKey crypto.PubKey, signs QuorumSigns) error { - if !q.shouldVerifyBlock { - return nil - } - if !pubKey.VerifySignatureDigest(q.Block.SignHash, signs.BlockSign) { - return fmt.Errorf( - "threshold block signature is invalid: (%X) signID=%X: %w", - q.Block.Msg, - q.Block.SignHash, - ErrVoteInvalidBlockSignature, - ) - } - return nil -} - -// verify threshold-recoverable vote extensions signatures -func (q *QuorumSingsVerifier) verifyVoteExtensions( - pubKey crypto.PubKey, - signs QuorumSigns, -) error { - if !q.shouldVerifyVoteExtensions { - return nil - } - - thresholdSigs := signs.VoteExtensionSignatures - signItems := q.VoteExtensionSignItems - if len(signItems) == 0 { - return nil - } - if len(signItems) != len(thresholdSigs) { - return fmt.Errorf("count of threshold vote extension signatures (%d) doesn't match recoverable vote extensions (%d)", - len(thresholdSigs), len(signItems), - ) - } - - for i, sig := range thresholdSigs { - if !pubKey.VerifySignatureDigest(signItems[i].SignHash, sig) { - return fmt.Errorf("vote-extension %d signature is invalid: raw %X, signature %X, pubkey %X, sigHash: %X", - i, - signItems[i].Msg, - sig, - pubKey.Bytes(), - signItems[i].SignHash, - ) - } - } - return nil -} diff --git a/types/quorum_sign_data.go b/types/quorum_sign_data.go index af7db79477..1bd51c6224 100644 --- a/types/quorum_sign_data.go +++ b/types/quorum_sign_data.go @@ -6,10 +6,12 @@ import ( bls "github.com/dashpay/bls-signatures/go-bindings" "github.com/dashpay/dashd-go/btcjson" + "github.com/hashicorp/go-multierror" + "github.com/rs/zerolog" + "github.com/dashpay/tenderdash/crypto" tmbytes "github.com/dashpay/tenderdash/libs/bytes" "github.com/dashpay/tenderdash/proto/tendermint/types" - "github.com/rs/zerolog" ) // QuorumSignData holds data which is necessary for signing and verification block, state, and each vote-extension in a list @@ -40,9 +42,52 @@ func (q QuorumSignData) SignWithPrivkey(key crypto.PrivKey) (QuorumSigns, error) return signs, nil } -// Verify verifies a quorum signatures: block, state and vote-extensions -func (q QuorumSignData) Verify(pubKey crypto.PubKey, signs QuorumSigns) error { - return NewQuorumSignsVerifier(q).Verify(pubKey, signs) +// Verify verifies a block and threshold vote extensions quorum signatures. +// It needs quorum to be reached so that we have enough signatures to verify. +func (q QuorumSignData) Verify(pubKey crypto.PubKey, signatures QuorumSigns) error { + var err error + if err1 := q.VerifyBlock(pubKey, signatures); err1 != nil { + err = multierror.Append(err, err1) + } + + if err1 := q.VerifyVoteExtensions(pubKey, signatures); err1 != nil { + err = multierror.Append(err, err1) + } + + return err +} + +// VerifyBlock verifies block signature +func (q QuorumSignData) VerifyBlock(pubKey crypto.PubKey, signatures QuorumSigns) error { + if !q.Block.VerifySignature(pubKey, signatures.BlockSign) { + return ErrVoteInvalidBlockSignature + } + + return nil +} + +// VerifyVoteExtensions verifies threshold vote extensions signatures +func (q QuorumSignData) VerifyVoteExtensions(pubKey crypto.PubKey, signatures QuorumSigns) error { + if len(q.VoteExtensionSignItems) != len(signatures.VoteExtensionSignatures) { + return fmt.Errorf("count of vote extension signatures (%d) doesn't match recoverable vote extensions (%d)", + len(signatures.VoteExtensionSignatures), len(q.VoteExtensionSignItems), + ) + } + + var err error + for i, signItem := range q.VoteExtensionSignItems { + if !signItem.VerifySignature(pubKey, signatures.VoteExtensionSignatures[i]) { + err = multierror.Append(err, fmt.Errorf("vote-extension %d signature is invalid: pubkey %X, raw msg: %X, sigHash: %X, signature %X", + i, + pubKey.Bytes(), + signItem.Msg, + signItem.MsgHash, + signatures.VoteExtensionSignatures[i], + )) + } + } + + return err } // MakeQuorumSignsWithVoteSet creates and returns QuorumSignData struct built with a vote-set and an added vote @@ -209,3 +254,8 @@ func (i *SignItem) UpdateSignHash(reverse bool) { i.SignHash = signHash } + +// VerifySignature verifies signature for a sign item +func (i *SignItem) VerifySignature(pubkey crypto.PubKey, sig []byte) bool { + return pubkey.VerifySignatureDigest(i.SignHash, sig) +} diff --git a/types/validation_test.go b/types/validation_test.go index 9d1224c0e9..6c5c7b98ce 100644 --- a/types/validation_test.go +++ b/types/validation_test.go @@ -68,8 +68,7 @@ func TestValidatorSet_VerifyCommit_All(t *testing.T) { expErr bool }{ {"good", chainID, vote.BlockID, vote.Height, commit, false}, - - {"threshold block signature is invalid", "EpsilonEridani", vote.BlockID, vote.Height, commit, true}, + {"invalid block signature", "EpsilonEridani", vote.BlockID, vote.Height, commit, true}, {"wrong block ID", chainID, makeBlockIDRandom(), vote.Height, commit, true}, { description: "invalid commit -- wrong block ID", @@ -84,8 +83,8 @@ func TestValidatorSet_VerifyCommit_All(t *testing.T) { expErr: true, }, {"wrong height", chainID, vote.BlockID, vote.Height - 1, commit, true}, - - {"threshold block signature is invalid", chainID, vote.BlockID, vote.Height, + // block sign malformed + {"invalid block signature", chainID, vote.BlockID, vote.Height, NewCommit(vote.Height, vote.Round, vote.BlockID, vote.VoteExtensions, &CommitSigns{ QuorumHash: quorumHash, @@ -93,8 +92,8 @@ func TestValidatorSet_VerifyCommit_All(t *testing.T) { BlockSign: []byte("invalid block signature"), VoteExtensionSignatures: sig.VoteExtensionSignatures, }}), true}, - - {"threshold block signature is invalid", chainID, vote.BlockID, vote.Height, + // quorum signs are replaced with vote2 non-threshold signature + {"invalid block signature", chainID, vote.BlockID, vote.Height, NewCommit(vote.Height, vote.Round, vote.BlockID, vote.VoteExtensions, &CommitSigns{ @@ -143,7 +142,7 @@ func TestValidatorSet_VerifyCommit_CheckThresholdSignatures(t *testing.T) { commit.ThresholdBlockSignature = v.BlockSignature err = valSet.VerifyCommit(chainID, blockID, h, commit) if assert.Error(t, err) { - assert.Contains(t, err.Error(), "threshold block signature is invalid") + assert.Contains(t, err.Error(), "invalid block signature") } goodVote := voteSet.GetByIndex(0) diff --git a/types/vote.go b/types/vote.go index 4f49f95f53..82c6f11866 100644 --- a/types/vote.go +++ b/types/vote.go @@ -172,11 +172,9 @@ func (vote *Vote) String() string { ) } -// VerifyVoteAndExtension performs the same verification as Verify, but -// additionally checks whether the vote extension signature corresponds to the -// given chain ID and public key. We only verify vote extension signatures for -// precommits. -func (vote *Vote) VerifyWithExtension( +// Verify performs vote signature verification. It checks whether the block signature +// and vote extensions signatures correspond to the given chain ID and public key. +func (vote *Vote) Verify( chainID string, quorumType btcjson.LLMQType, quorumHash crypto.QuorumHash, @@ -191,28 +189,8 @@ func (vote *Vote) VerifyWithExtension( if err != nil { return err } - return vote.verifySign(pubKey, quorumSignData, WithVerifyExtensions(vote.Type == tmproto.PrecommitType)) -} - -func (vote *Vote) Verify( - chainID string, - quorumType btcjson.LLMQType, - quorumHash []byte, - pubKey crypto.PubKey, - proTxHash crypto.ProTxHash, - _stateID tmproto.StateID, -) error { - err := vote.verifyBasic(proTxHash, pubKey) - if err != nil { - return err - } - quorumSignData, err := MakeQuorumSigns(chainID, quorumType, quorumHash, vote.ToProto()) - if err != nil { - return err - } - // TODO check why we don't verify extensions here - return vote.verifySign(pubKey, quorumSignData, WithVerifyExtensions(false)) + return quorumSignData.Verify(pubKey, vote.makeQuorumSigns()) } func (vote *Vote) verifyBasic(proTxHash ProTxHash, pubKey crypto.PubKey) error { @@ -223,6 +201,10 @@ func (vote *Vote) verifyBasic(proTxHash ProTxHash, pubKey crypto.PubKey) error { return ErrVoteInvalidValidatorPubKeySize } + if vote.Type != tmproto.PrecommitType && vote.VoteExtensions.Len() > 0 { + return ErrVoteInvalidExtension + } + return nil } @@ -236,23 +218,8 @@ func (vote *Vote) VerifyExtensionSign(chainID string, pubKey crypto.PubKey, quor if err != nil { return err } - verifier := NewQuorumSignsVerifier( - quorumSignData, - WithVerifyBlock(false), - ) - return verifier.Verify(pubKey, vote.makeQuorumSigns()) -} -func (vote *Vote) verifySign( - pubKey crypto.PubKey, - quorumSignData QuorumSignData, - opts ...func(verifier *QuorumSingsVerifier), -) error { - verifier := NewQuorumSignsVerifier( - quorumSignData, - opts..., - ) - return verifier.Verify(pubKey, vote.makeQuorumSigns()) + return quorumSignData.VerifyVoteExtensions(pubKey, vote.makeQuorumSigns()) } func (vote *Vote) makeQuorumSigns() QuorumSigns { diff --git a/types/vote_set.go b/types/vote_set.go index b80ce203aa..bf4520cb0d 100644 --- a/types/vote_set.go +++ b/types/vote_set.go @@ -216,7 +216,7 @@ func (voteSet *VoteSet) addVote(vote *Vote) (added bool, err error) { } // Check signature. - err = vote.VerifyWithExtension( + err = vote.Verify( voteSet.chainID, voteSet.valSet.QuorumType, voteSet.valSet.QuorumHash, @@ -341,6 +341,8 @@ func (voteSet *VoteSet) addVerifiedVote( return true, conflicting } +// recoverThresholdSignsAndVerify recovers threshold signatures and verifies them. +// precondition: quorum reached func (voteSet *VoteSet) recoverThresholdSignsAndVerify(blockVotes *blockVotes, quorumDataSigns QuorumSignData) error { if len(blockVotes.votes) == 0 { return nil @@ -359,11 +361,10 @@ func (voteSet *VoteSet) recoverThresholdSignsAndVerify(blockVotes *blockVotes, q if err != nil { return err } - verifier := NewQuorumSignsVerifier( - quorumDataSigns, - WithVerifyReachedQuorum(voteSet.IsQuorumReached()), - ) - return verifier.Verify(voteSet.valSet.ThresholdPublicKey, voteSet.makeQuorumSigns()) + + sigs := voteSet.makeQuorumSigns() + // we assume quorum is reached + return quorumDataSigns.Verify(voteSet.valSet.ThresholdPublicKey, sigs) } func (voteSet *VoteSet) recoverThresholdSigns(blockVotes *blockVotes) error { diff --git a/types/vote_test.go b/types/vote_test.go index 1dd1847440..b7d413a373 100644 --- a/types/vote_test.go +++ b/types/vote_test.go @@ -407,7 +407,7 @@ func TestVoteExtension(t *testing.T) { } } - err = vote.VerifyWithExtension("test_chain_id", btcjson.LLMQType_5_60, quorumHash, pk, proTxHash) + err = vote.Verify("test_chain_id", btcjson.LLMQType_5_60, quorumHash, pk, proTxHash) if tc.expectError { require.Error(t, err) } else { @@ -458,13 +458,13 @@ func TestVoteVerify(t *testing.T) { stateID := RandStateID() stateID.Height = uint64(vote.Height - 1) pubKey := bls12381.GenPrivKey().PubKey() - err = vote.Verify("test_chain_id", quorumType, quorumHash, pubKey, crypto.RandProTxHash(), stateID) + err = vote.Verify("test_chain_id", quorumType, quorumHash, pubKey, crypto.RandProTxHash()) if assert.Error(t, err) { assert.Equal(t, ErrVoteInvalidValidatorProTxHash, err) } - err = vote.Verify("test_chain_id", quorumType, quorumHash, pubkey, proTxHash, stateID) + err = vote.Verify("test_chain_id", quorumType, quorumHash, pubkey, proTxHash) if assert.Error(t, err) { assert.ErrorIs(t, err, ErrVoteInvalidBlockSignature) // since block signatures are verified first }