refactor: vote extensions refactoring

internal/consensus/reactor.go

@@ -687,6 +687,10 @@ func (r *Reactor) handleVoteMessage(ctx context.Context, envelope *p2p.Envelope,
 		if isValidator { // ignore votes on non-validator nodes; TODO don't even send it
 			vMsg := msgI.(*VoteMessage)
 
+			if err := vMsg.Vote.ValidateBasic(); err != nil {
+				return fmt.Errorf("invalid vote received from %s: %w", envelope.From, err)
+			}
+
 			ps.EnsureVoteBitArrays(height, valSize)
 			ps.EnsureVoteBitArrays(height-1, lastCommitSize)
 			if err := ps.SetHasVote(vMsg.Vote); err != nil {

internal/consensus/vote_signer_test.go

@@ -138,6 +138,10 @@ func TestVoteSigner_signAddVote(t *testing.T) {
 			key, err := privVal.GetPubKey(ctx, valSet.QuorumHash)
 			assert.NoError(t, err)
 
+			for _, ext := range vote.VoteExtensions {
+				assert.NotEmpty(t, ext.GetSignature())
+			}
+
 			key1, err := bls.G1ElementFromBytes(key.Bytes())
 			assert.NoError(t, err)
 

internal/state/execution.go

@@ -615,7 +615,7 @@ func buildLastCommitInfo(block *types.Block, initialHeight int64) abci.CommitInf
 		Round:                   block.LastCommit.Round,
 		QuorumHash:              block.LastCommit.QuorumHash,
 		BlockSignature:          block.LastCommit.ThresholdBlockSignature,
-		ThresholdVoteExtensions: block.LastCommit.ThresholdVoteExtensions.ToProto(),
+		ThresholdVoteExtensions: block.LastCommit.ThresholdVoteExtensions,
 	}
 }
 

privval/dash_core_signer_client.go

@@ -275,7 +275,7 @@ func (sc *DashCoreSignerClient) SignVote(
 		"voteType", protoVote.Type,
 		"quorumType", quorumType,
 		"quorumHash", quorumHash,
-		"signature", qs.sign,
+		"signature", hex.EncodeToString(qs.sign),
 		"proTxHash", proTxHash,
 		"coreBlockRequestId", qs.ID,
 		"coreSignId", tmbytes.Reverse(qs.signHash),

proto/tendermint/types/dash_test.go

@@ -0,0 +1,31 @@
+package types_test
+
+import (
+	"testing"
+
+	"github.com/dashpay/tenderdash/internal/libs/protoio"
+	"github.com/dashpay/tenderdash/proto/tendermint/types"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestMarshalVoteExtension(t *testing.T) {
+
+	ve := &types.VoteExtension{
+		Type:      types.VoteExtensionType_THRESHOLD_RECOVER_RAW,
+		Extension: []byte("threshold"),
+		XSignRequestId: &types.VoteExtension_SignRequestId{
+			SignRequestId: []byte("sign-request-id"),
+		},
+	}
+
+	v := types.Vote{
+		Type:           types.PrecommitType,
+		VoteExtensions: []*types.VoteExtension{ve},
+	}
+	v.Marshal()
+
+	ve.Marshal()
+	marshaled, err := protoio.MarshalDelimited(ve)
+	assert.NoError(t, err)
+	assert.NotEmpty(t, marshaled)
+}

test/e2e/app/app.go

@@ -6,7 +6,7 @@ import (
 	"encoding/binary"
 	"errors"
 	"fmt"
-	"math/rand"
+	"math/big"
 	"strconv"
 	"strings"
 	"time"
@@ -17,6 +17,7 @@ import (
 	"github.com/dashpay/tenderdash/abci/example/code"
 	"github.com/dashpay/tenderdash/abci/example/kvstore"
 	abci "github.com/dashpay/tenderdash/abci/types"
+	"github.com/dashpay/tenderdash/crypto"
 	"github.com/dashpay/tenderdash/libs/log"
 	types1 "github.com/dashpay/tenderdash/proto/tendermint/types"
 	"github.com/dashpay/tenderdash/types"
@@ -85,22 +86,18 @@ func (app *Application) ExtendVote(_ context.Context, req *abci.RequestExtendVot
 		)
 		return &abci.ResponseExtendVote{}, nil
 	}
-	ext := make([]byte, binary.MaxVarintLen64)
-	// We don't care that these values are generated by a weak random number
-	// generator. It's just for test purposes.
-	//nolint:gosec // G404: Use of weak random number generator
-	num := rand.Int63n(voteExtensionMaxVal)
-	extLen := binary.PutVarint(ext, num)
+	ext := make([]byte, crypto.DefaultHashSize)
+	copy(ext, big.NewInt(lastHeight+1).Bytes())
+
 	app.logger.Info("generated vote extension",
-		"num", num,
-		"ext", fmt.Sprintf("%x", ext[:extLen]),
-		"state.Height", lastHeight,
+		"ext", fmt.Sprintf("%x", ext),
+		"state.Height", lastHeight+1,
 	)
 	return &abci.ResponseExtendVote{
 		VoteExtensions: []*abci.ExtendVoteExtension{
 			{
-				Type:      types1.VoteExtensionType_DEFAULT,
-				Extension: ext[:extLen],
+				Type:      types1.VoteExtensionType_THRESHOLD_RECOVER_RAW,
+				Extension: ext,
 			},
 			{
 				Type:      types1.VoteExtensionType_THRESHOLD_RECOVER,

types/block.go

@@ -737,7 +737,7 @@ type Commit struct {
 	QuorumHash              crypto.QuorumHash `json:"quorum_hash"`
 	ThresholdBlockSignature []byte            `json:"threshold_block_signature"`
 	// ThresholdVoteExtensions keeps the list of recovered threshold signatures for vote-extensions
-	ThresholdVoteExtensions VoteExtensions `json:"threshold_vote_extensions"`
+	ThresholdVoteExtensions tmproto.VoteExtensions `json:"threshold_vote_extensions"`
 
 	// Memoized in first call to corresponding method.
 	// NOTE: can't memoize in constructor because constructor isn't used for
@@ -764,7 +764,7 @@ func (commit *Commit) ToCommitInfo() types.CommitInfo {
 		Round:                   commit.Round,
 		QuorumHash:              commit.QuorumHash,
 		BlockSignature:          commit.ThresholdBlockSignature,
-		ThresholdVoteExtensions: commit.ThresholdVoteExtensions.ToProto(),
+		ThresholdVoteExtensions: commit.ThresholdVoteExtensions,
 	}
 }
 
@@ -943,7 +943,7 @@ func (commit *Commit) ToProto() *tmproto.Commit {
 	c.BlockID = commit.BlockID.ToProto()
 
 	c.ThresholdBlockSignature = commit.ThresholdBlockSignature
-	c.ThresholdVoteExtensions = commit.ThresholdVoteExtensions.ToProto()
+	c.ThresholdVoteExtensions = commit.ThresholdVoteExtensions
 	c.QuorumHash = commit.QuorumHash
 
 	return c
@@ -967,7 +967,7 @@ func CommitFromProto(cp *tmproto.Commit) (*Commit, error) {
 
 	commit.QuorumHash = cp.QuorumHash
 	commit.ThresholdBlockSignature = cp.ThresholdBlockSignature
-	commit.ThresholdVoteExtensions = VoteExtensionsFromProto(cp.ThresholdVoteExtensions...)
+	commit.ThresholdVoteExtensions = cp.ThresholdVoteExtensions
 
 	commit.Height = cp.Height
 	commit.Round = cp.Round

types/canonical.go

@@ -1,10 +1,9 @@
 package types
 
 import (
-	"errors"
+	"fmt"
 	"time"
 
-	"github.com/dashpay/tenderdash/internal/libs/protoio"
 	tmtime "github.com/dashpay/tenderdash/libs/time"
 	tmproto "github.com/dashpay/tenderdash/proto/tendermint/types"
 )
@@ -33,7 +32,7 @@ func CanonicalizeProposal(chainID string, proposal *tmproto.Proposal) tmproto.Ca
 // CanonicalizeVoteExtension extracts the vote extension from the given vote
 // and constructs a CanonicalizeVoteExtension struct, whose representation in
 // bytes is what is signed in order to produce the vote extension's signature.
-func CanonicalizeVoteExtension(chainID string, ext *tmproto.VoteExtension, height int64, round int32) ([]byte, error) {
+func CanonicalizeVoteExtension(chainID string, ext *tmproto.VoteExtension, height int64, round int32) (tmproto.CanonicalVoteExtension, error) {
 	switch ext.Type {
 	case tmproto.VoteExtensionType_DEFAULT, tmproto.VoteExtensionType_THRESHOLD_RECOVER:
 		{
@@ -44,12 +43,10 @@ func CanonicalizeVoteExtension(chainID string, ext *tmproto.VoteExtension, heigh
 				Round:     int64(round),
 				ChainId:   chainID,
 			}
-			return protoio.MarshalDelimited(&canonical)
+			return canonical, nil
 		}
-	case tmproto.VoteExtensionType_THRESHOLD_RECOVER_RAW:
-		return ext.Extension, nil
 	}
-	return nil, errors.New("provided vote extension type does not have canonical form for signing")
+	return tmproto.CanonicalVoteExtension{}, fmt.Errorf("provided vote extension type %s does not have canonical form for signing", ext.Type)
 }
 
 // CanonicalTime can be used to stringify time in a canonical way.

types/quorum.go

@@ -17,7 +17,7 @@ type CommitSigns struct {
 func (c *CommitSigns) CopyToCommit(commit *Commit) {
 	commit.QuorumHash = c.QuorumHash
 	commit.ThresholdBlockSignature = c.BlockSign
-	commit.ThresholdVoteExtensions = c.ThresholdVoteExtensions
+	commit.ThresholdVoteExtensions = c.ThresholdVoteExtensions.ToProto()
 }
 
 // QuorumSigns holds all created signatures, block, state and for each recovered vote-extensions
@@ -33,7 +33,7 @@ type QuorumSigns struct {
 func NewQuorumSignsFromCommit(commit *Commit) QuorumSigns {
 	return QuorumSigns{
 		BlockSign:               commit.ThresholdBlockSignature,
-		ThresholdVoteExtensions: commit.ThresholdVoteExtensions,
+		ThresholdVoteExtensions: VoteExtensionsFromProto(commit.ThresholdVoteExtensions...),
 	}
 }
 
@@ -135,8 +135,8 @@ func (q *QuorumSingsVerifier) verifyVoteExtensions(
 
 	for i, sig := range thresholdSigs {
 		if !pubKey.VerifySignatureDigest(signItems[i].ID, sig) {
-			return fmt.Errorf("vote-extension %d signature is invalid: %X", i,
-				signItems[i].Raw)
+			return fmt.Errorf("vote-extension %d signature is invalid: raw %X, signatrure %X", i,
+				signItems[i].Raw, sig)
 		}
 	}
 	return nil

types/signs_recoverer.go

@@ -96,7 +96,6 @@ func (v *SignsRecoverer) addVoteExtensionSigs(voteExtensions VoteExtensions) {
 	for i, ext := range thresholdExtensions {
 		v.voteThresholdExtensionSigs[i] = append(v.voteThresholdExtensionSigs[i], ext.GetSignature())
 	}
-
 }
 
 func (v *SignsRecoverer) recoverBlockSig(thresholdSigns *QuorumSigns) error {
@@ -128,7 +127,17 @@ func (v *SignsRecoverer) recoverVoteExtensionSigs(quorumSigs *QuorumSigns) error
 			thresholdExtIndex++
 
 			if len(extensionSignatures) > 0 {
-				thresholdSignature, err := bls12381.RecoverThresholdSignatureFromShares(extensionSignatures, v.validatorProTxHashes)
+				// as some nodes might have voted nil, we will have empty sigs from them;
+				// we need to filter them out before threshold-recovering
+				sigs := make([][]byte, 0, len(extensionSignatures))
+				proTxHashes := make([][]byte, 0, len(extensionSignatures))
+				for i, sig := range extensionSignatures {
+					if len(sig) > 0 {
+						sigs = append(sigs, sig)
+						proTxHashes = append(proTxHashes, v.validatorProTxHashes[i])
+					}
+				}
+				thresholdSignature, err := bls12381.RecoverThresholdSignatureFromShares(sigs, proTxHashes)
 				if err != nil {
 					return fmt.Errorf("error recovering vote-extension %d threshold signature: %w", extIndex, err)
 				}

types/validation_test.go

@@ -140,7 +140,7 @@ func TestValidatorSet_VerifyCommit_CheckThresholdSignatures(t *testing.T) {
 	thresholdSigns, err := recoverer.Recover()
 	require.NoError(t, err)
 	commit.ThresholdBlockSignature = thresholdSigns.BlockSign
-	commit.ThresholdVoteExtensions = thresholdSigns.ThresholdVoteExtensions
+	commit.ThresholdVoteExtensions = thresholdSigns.ThresholdVoteExtensions.ToProto()
 	err = valSet.VerifyCommit(chainID, blockID, h, commit)
 	require.NoError(t, err)
 }

types/vote.go

@@ -380,6 +380,7 @@ func (vote *Vote) MarshalZerologObject(e *zerolog.Event) {
 	e.Str("val_proTxHash", vote.ValidatorProTxHash.ShortString())
 	e.Int32("val_index", vote.ValidatorIndex)
 	e.Bool("nil", vote.BlockID.IsNil())
+	e.Array("extensions", vote.VoteExtensions)
 }
 
 func (vote *Vote) HasVoteMessage() *tmcons.HasVote {

types/vote_extension.go

@@ -10,9 +10,11 @@ import (
 	"github.com/dashpay/dashd-go/btcjson"
 	abci "github.com/dashpay/tenderdash/abci/types"
 	"github.com/dashpay/tenderdash/crypto"
+	"github.com/dashpay/tenderdash/internal/libs/protoio"
 	tmbytes "github.com/dashpay/tenderdash/libs/bytes"
 	tmproto "github.com/dashpay/tenderdash/proto/tendermint/types"
 	"github.com/hashicorp/go-multierror"
+	"github.com/rs/zerolog"
 )
 
 var (
@@ -191,6 +193,10 @@ func VoteExtensionsFromProto(pve ...*tmproto.VoteExtension) VoteExtensions {
 
 // Copy creates a deep copy of VoteExtensions
 func (e VoteExtensions) Copy() VoteExtensions {
+	if e.IsEmpty() {
+		return nil
+	}
+
 	copied := make(VoteExtensions, 0, len(e))
 	for _, ext := range e {
 		copied = append(copied, ext.Copy())
@@ -237,6 +243,13 @@ func (e VoteExtensions) CopySignsToProto(dest tmproto.VoteExtensions) error {
 	return nil
 }
 
+// Marshal VoteExtensions as zerolog array
+func (e VoteExtensions) MarshalZerologArray(a *zerolog.Array) {
+	for _, ext := range e {
+		a.Object(ext)
+	}
+}
+
 type VoteExtensionIf interface {
 	// Return type of this vote extension
 	GetType() tmproto.VoteExtensionType
@@ -255,6 +268,8 @@ type VoteExtensionIf interface {
 	Validate() error
 
 	SetSignature(sig []byte)
+
+	zerolog.LogObjectMarshaler
 }
 
 func VoteExtensionFromProto(ve tmproto.VoteExtension) VoteExtensionIf {
@@ -282,7 +297,7 @@ func (e GenericVoteExtension) Copy() VoteExtensionIf {
 }
 
 func (e GenericVoteExtension) ToProto() tmproto.VoteExtension {
-	return e.VoteExtension
+	return e.VoteExtension.Clone()
 }
 
 func (e GenericVoteExtension) SignItem(chainID string, height int64, round int32, quorumType btcjson.LLMQType, quorumHash []byte) (crypto.SignItem, error) {
@@ -297,10 +312,29 @@ func (e *GenericVoteExtension) SetSignature(sig []byte) {
 	e.Signature = sig
 }
 
+func (e GenericVoteExtension) MarshalZerologObject(o *zerolog.Event) {
+	o.Str("type", e.GetType().String())
+	o.Hex("extension", e.GetExtension())
+	o.Hex("signature", e.GetSignature())
+	o.Hex("sign_request_id", e.GetSignRequestId())
+}
+
+//nolint:revive,stylecheck // name is the same as in protobuf-generated code
+func (e GenericVoteExtension) GetSignRequestId() []byte {
+	if e.XSignRequestId == nil {
+		return nil
+	}
+	id, ok := e.XSignRequestId.(*tmproto.VoteExtension_SignRequestId)
+	if !ok || id == nil {
+		return nil
+	}
+
+	return id.SignRequestId
+}
+
 // ThresholdVoteExtension is a threshold type of VoteExtension
 type ThresholdVoteExtension struct {
 	GenericVoteExtension
-	ThresholdSignature []byte
 }
 
 func (e ThresholdVoteExtension) Copy() VoteExtensionIf {
@@ -369,10 +403,15 @@ func voteExtensionRequestID(height int64, round int32) ([]byte, error) {
 // Similar to VoteSignBytes, the encoded Protobuf message is varint
 // length-prefixed for backwards-compatibility with the Amino encoding.
 func voteExtensionSignBytes(chainID string, height int64, round int32, ext *tmproto.VoteExtension) []byte {
-	bz, err := CanonicalizeVoteExtension(chainID, ext, height, round)
+	canonical, err := CanonicalizeVoteExtension(chainID, ext, height, round)
 	if err != nil {
 		panic(err)
 	}
+	bz, err := protoio.MarshalDelimited(&canonical)
+	if err != nil {
+		panic(err)
+	}
+
 	return bz
 }
 

types/vote_extension_test.go

@@ -0,0 +1,24 @@
+package types
+
+import (
+	"testing"
+
+	tmproto "github.com/dashpay/tenderdash/proto/tendermint/types"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestVoteExtensionCopySignsFromProto(t *testing.T) {
+	src := tmproto.VoteExtensions{
+		&tmproto.VoteExtension{
+			Type:      tmproto.VoteExtensionType_THRESHOLD_RECOVER,
+			Extension: []byte("threshold"),
+			Signature: []byte("signature"),
+		},
+	}
+
+	dst := VoteExtensions{&ThresholdVoteExtension{}}
+	err := dst.CopySignsFromProto(src)
+	require.NoError(t, err)
+	assert.EqualValues(t, src[0].GetSignature(), dst[0].GetSignature())
+}