test(consensus): fix vote extension tests

internal/consensus/common_test.go

@@ -158,11 +158,6 @@ func signVote(
 	quorumType btcjson.LLMQType,
 	quorumHash crypto.QuorumHash) *types.Vote {
 	exts := make(types.VoteExtensions, 0)
-	if voteType == tmproto.PrecommitType && !blockID.IsNil() {
-		exts.Add(tmproto.VoteExtension{
-			Type:      tmproto.VoteExtensionType_THRESHOLD_RECOVER,
-			Extension: []byte("extension")})
-	}
 	v, err := vs.signVote(ctx, voteType, chainID, blockID, quorumType, quorumHash, exts)
 	require.NoError(t, err, "failed to sign vote")
 

internal/consensus/state_test.go

@@ -3,6 +3,7 @@ package consensus
 import (
 	"bytes"
 	"context"
+	"fmt"
 	"reflect"
 	"strconv"
 	"testing"
@@ -2141,12 +2142,10 @@ func TestExtendVote(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
-	voteExtensions := []*abci.ExtendVoteExtension{
-		{
-			Type:      tmproto.VoteExtensionType_THRESHOLD_RECOVER,
-			Extension: []byte("extension"),
-		},
-	}
+	voteExtensions := types.VoteExtensionsFromProto(&tmproto.VoteExtension{
+		Type:      tmproto.VoteExtensionType_THRESHOLD_RECOVER,
+		Extension: []byte("extension"),
+	})
 
 	m := abcimocks.NewApplication(t)
 	m.On("ProcessProposal", mock.Anything, mock.Anything).Return(&abci.ResponseProcessProposal{
@@ -2200,14 +2199,14 @@ func TestExtendVote(t *testing.T) {
 			assert.Equal(t, req.Round, round)
 	})
 	m.On("ExtendVote", mock.Anything, reqExtendVoteFunc).Return(&abci.ResponseExtendVote{
-		VoteExtensions: voteExtensions,
+		VoteExtensions: voteExtensions.ToExtendProto(),
 	}, nil)
 	reqVerifyVoteExtFunc := mock.MatchedBy(func(req *abci.RequestVerifyVoteExtension) bool {
 		_, ok := proTxHashMap[types.ProTxHash(req.ValidatorProTxHash).String()]
-		return assert.Equal(t, req.Hash, blockID.Hash.Bytes()) &&
-			assert.Equal(t, req.Height, height) &&
-			assert.Equal(t, req.Round, round) &&
-			assert.Equal(t, req.VoteExtensions, voteExtensions) &&
+		return assert.Equal(t, blockID.Hash.Bytes(), req.Hash) &&
+			assert.Equal(t, height, req.Height) &&
+			assert.Equal(t, round, req.Round) &&
+			assert.Equal(t, voteExtensions.ToExtendProto(), req.VoteExtensions) &&
 			assert.True(t, ok)
 	})
 	m.On("VerifyVoteExtension", mock.Anything, reqVerifyVoteExtFunc).
@@ -2218,7 +2217,7 @@ func TestExtendVote(t *testing.T) {
 	ensurePrevoteMatch(t, voteCh, height, round, blockID.Hash)
 
 	ensurePrecommit(t, voteCh, height, round)
-	signAddVotes(ctx, t, cs1, tmproto.PrecommitType, config.ChainID(), blockID, vss[1:]...)
+	signAddPrecommitsWithExtension(ctx, t, cs1, config.ChainID(), blockID, voteExtensions, vss[1:]...)
 	ensureNewRound(t, newRoundCh, height+1, 0)
 	m.AssertExpectations(t)
 	mock.AssertExpectationsForObjects(t, m)
@@ -2302,12 +2301,11 @@ func TestVerifyVoteExtensionNotCalledOnAbsentPrecommit(t *testing.T) {
 	config := configSetup(t)
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
-	voteExtensions := []*abci.ExtendVoteExtension{
-		{
-			Type:      tmproto.VoteExtensionType_THRESHOLD_RECOVER,
-			Extension: []byte("extension"),
-		},
-	}
+	voteExtensions := types.VoteExtensionsFromProto(&tmproto.VoteExtension{
+		Type:      tmproto.VoteExtensionType_THRESHOLD_RECOVER,
+		Extension: []byte("extension"),
+	})
+
 	m := abcimocks.NewApplication(t)
 	m.On("ProcessProposal", mock.Anything, mock.Anything).Return(&abci.ResponseProcessProposal{
 		AppHash: make([]byte, crypto.DefaultAppHashSize),
@@ -2317,7 +2315,7 @@ func TestVerifyVoteExtensionNotCalledOnAbsentPrecommit(t *testing.T) {
 		AppHash: make([]byte, crypto.DefaultAppHashSize),
 	}, nil)
 	m.On("ExtendVote", mock.Anything, mock.Anything).Return(&abci.ResponseExtendVote{
-		VoteExtensions: voteExtensions,
+		VoteExtensions: voteExtensions.ToExtendProto(),
 	}, nil)
 	m.On("FinalizeBlock", mock.Anything, mock.Anything).Return(&abci.ResponseFinalizeBlock{}, nil).Maybe()
 	cs1, vss := makeState(ctx, t, makeStateArgs{config: config, application: m})
@@ -2355,18 +2353,18 @@ func TestVerifyVoteExtensionNotCalledOnAbsentPrecommit(t *testing.T) {
 	})
 	reqVerifyVoteExtFunc := mock.MatchedBy(func(req *abci.RequestVerifyVoteExtension) bool {
 		_, ok := proTxHashMap[types.ProTxHash(req.ValidatorProTxHash).String()]
-		return assert.Equal(t, req.Hash, blockID.Hash.Bytes()) &&
-			assert.Equal(t, req.Height, height) &&
-			assert.Equal(t, req.Round, round) &&
-			assert.Equal(t, req.VoteExtensions, voteExtensions) &&
+		return assert.Equal(t, blockID.Hash.Bytes(), req.Hash) &&
+			assert.Equal(t, height, req.Height) &&
+			assert.Equal(t, round, req.Round) &&
+			assert.Equal(t, voteExtensions.ToExtendProto(), req.VoteExtensions) &&
 			assert.True(t, ok)
 	})
 	m.On("VerifyVoteExtension", mock.Anything, reqVerifyVoteExtFunc).
 		Return(&abci.ResponseVerifyVoteExtension{
 			Status: abci.ResponseVerifyVoteExtension_ACCEPT,
 		}, nil)
 
-	signAddVotes(ctx, t, cs1, tmproto.PrecommitType, config.ChainID(), blockID, vss[2:]...)
+	signAddPrecommitsWithExtension(ctx, t, cs1, config.ChainID(), blockID, voteExtensions, vss[2:]...)
 	ensureNewRound(t, newRoundCh, height+1, 0)
 	m.AssertExpectations(t)
 
@@ -2392,7 +2390,7 @@ func TestVerifyVoteExtensionNotCalledOnAbsentPrecommit(t *testing.T) {
 // TestPrepareProposalReceivesVoteExtensions tests that the PrepareProposal method
 // is called with the vote extensions from the previous height. The test functions
 // be completing a consensus height with a mock application as the proposer. The
-// test then proceeds to fail sever rounds of consensus until the mock application
+// test then proceeds to fail several rounds of consensus until the mock application
 // is the proposer again and ensures that the mock application receives the set of
 // vote extensions from the previous consensus instance.
 func TestPrepareProposalReceivesVoteExtensions(t *testing.T) {
@@ -2401,53 +2399,56 @@ func TestPrepareProposalReceivesVoteExtensions(t *testing.T) {
 
 	config := configSetup(t)
 
+	voteExtensions := types.VoteExtensionsFromProto(
+		&tmproto.VoteExtension{
+			Type:      tmproto.VoteExtensionType_THRESHOLD_RECOVER_RAW,
+			Extension: crypto.Checksum([]byte("extension-raw")),
+		}, &tmproto.VoteExtension{
+			Type:      tmproto.VoteExtensionType_THRESHOLD_RECOVER,
+			Extension: []byte("deterministic"),
+		},
+	)
+
 	m := &abcimocks.Application{}
 	m.On("ExtendVote", mock.Anything, mock.Anything).Return(&abci.ResponseExtendVote{
-		VoteExtensions: []*abci.ExtendVoteExtension{
-			{
-				Type:      tmproto.VoteExtensionType_THRESHOLD_RECOVER_RAW,
-				Extension: crypto.Checksum([]byte("extension-raw")),
-			},
-			{
-				Type:      tmproto.VoteExtensionType_THRESHOLD_RECOVER,
-				Extension: []byte("deterministic"),
-			},
-		},
+		VoteExtensions: voteExtensions.ToExtendProto(),
 	}, nil)
 	m.On("ProcessProposal", mock.Anything, mock.Anything).Return(&abci.ResponseProcessProposal{
 		AppHash: make([]byte, crypto.DefaultAppHashSize),
 		Status:  abci.ResponseProcessProposal_ACCEPT,
 	}, nil)
 
-	// capture the prepare proposal request.
-	rpp := &abci.RequestPrepareProposal{}
+	// matcher for prepare proposal request
 	m.On("PrepareProposal", mock.Anything, mock.MatchedBy(func(r *abci.RequestPrepareProposal) bool {
-		rpp = r
-		return true
+		if r.Height == 1 {
+			return assert.Empty(t, r.GetLocalLastCommit().ThresholdVoteExtensions, "no vote extensions should be present on the first height")
+		}
+
+		// at height 2, we expect the vote extensions from the previous height to be present.
+		extensions := make([][]byte, 0)
+		for _, ext := range r.GetLocalLastCommit().ThresholdVoteExtensions {
+			extensions = append(extensions, ext.Extension)
+		}
+		return assert.EqualValues(t, 2, r.Height) &&
+			assert.EqualValues(t, 3, r.Round) &&
+			assert.Len(t, r.GetLocalLastCommit().ThresholdVoteExtensions, 2, "expected 2 vote extensions at height %d", r.Height) &&
+			assert.EqualValues(t, voteExtensions.GetExtensions(), extensions)
+
 	})).Return(&abci.ResponsePrepareProposal{
 		AppHash: make([]byte, crypto.DefaultAppHashSize),
 	}, nil)
 
-	m.On("PrepareProposal", mock.Anything, mock.Anything).Return(&abci.ResponsePrepareProposal{AppHash: make([]byte, crypto.DefaultAppHashSize)}, nil).Once()
 	m.On("VerifyVoteExtension", mock.Anything, mock.Anything).Return(&abci.ResponseVerifyVoteExtension{Status: abci.ResponseVerifyVoteExtension_ACCEPT}, nil)
 
-	// no vote extensions are present
-	m.On("FinalizeBlock", mock.Anything, mock.MatchedBy(func(r *abci.RequestFinalizeBlock) bool {
-		return len(r.Commit.ThresholdVoteExtensions) == 0
-	})).Return(&abci.ResponseFinalizeBlock{}, nil)
-
-	// we expect 2 threshold-recovered vote extensions
+	// We expect 2 threshold-recovered vote extensions in current Commit
 	m.On("FinalizeBlock", mock.Anything, mock.MatchedBy(func(r *abci.RequestFinalizeBlock) bool {
-		if len(r.Commit.ThresholdVoteExtensions) != 0 {
-			return false
-		}
-		t.Logf("FinalizeBlock with %d vote extensions", len(r.Commit.ThresholdVoteExtensions))
-
+		assert.Len(t, r.Commit.ThresholdVoteExtensions, 2)
 		vexts := r.Commit.ThresholdVoteExtensions
 
-		return bytes.Equal(vexts[0].Extension, []byte("deterministic")) &&
-			bytes.Equal(vexts[1].Extension, crypto.Checksum([]byte("raw")))
-	})).Return(&abci.ResponseFinalizeBlock{}, nil).Once()
+		return bytes.Equal(vexts[0].Extension, voteExtensions[0].GetExtension()) &&
+			bytes.Equal(vexts[1].Extension, voteExtensions[1].GetExtension())
+
+	})).Return(&abci.ResponseFinalizeBlock{}, nil)
 
 	cs1, vss := makeState(ctx, t, makeStateArgs{config: config, application: m})
 	stateData := cs1.GetStateData()
@@ -2469,24 +2470,7 @@ func TestPrepareProposalReceivesVoteExtensions(t *testing.T) {
 	signAddVotes(ctx, t, cs1, tmproto.PrevoteType, config.ChainID(), blockID, vss[1:]...)
 
 	// create a precommit for each validator with the associated vote extension.
-	for _, vs := range vss[1:] {
-		voteExtensions := tmproto.VoteExtensions{
-			{
-				Type:      tmproto.VoteExtensionType_THRESHOLD_RECOVER,
-				Extension: []byte("extension"),
-			},
-			{
-				Type:      tmproto.VoteExtensionType_THRESHOLD_RECOVER,
-				Extension: []byte("deterministic"),
-			},
-			{
-				Type:      tmproto.VoteExtensionType_THRESHOLD_RECOVER_RAW,
-				Extension: crypto.Checksum([]byte("raw")),
-			},
-		}
-
-		signAddPrecommitWithExtension(ctx, t, cs1, config.ChainID(), blockID, types.VoteExtensionsFromProto(voteExtensions...), vs)
-	}
+	signAddPrecommitsWithExtension(ctx, t, cs1, config.ChainID(), blockID, voteExtensions, vss[1:]...)
 
 	ensurePrevote(t, voteCh, height, round)
 
@@ -2506,9 +2490,6 @@ func TestPrepareProposalReceivesVoteExtensions(t *testing.T) {
 	ensureNewRound(t, newRoundCh, height, round)
 	ensureNewProposal(t, proposalCh, height, round)
 
-	// ensure that the proposer received the list of vote extensions from the
-	// previous height.
-	require.Len(t, rpp.LocalLastCommit.ThresholdVoteExtensions, 1)
 	m.AssertExpectations(t)
 }
 
@@ -3307,18 +3288,28 @@ func subscribe(
 	return ch
 }
 
-func signAddPrecommitWithExtension(ctx context.Context,
+func signAddPrecommitsWithExtension(ctx context.Context,
 	t *testing.T,
 	cs *State,
 	chainID string,
 	blockID types.BlockID,
 	extensions types.VoteExtensions,
-	stub *validatorStub) {
+	vss ...*validatorStub) {
 	_, valSet := cs.GetValidatorSet()
-	v, err := stub.signVote(ctx, tmproto.PrecommitType, chainID, blockID, valSet.QuorumType,
-		valSet.QuorumHash, extensions)
-	require.NoError(t, err, "failed to sign vote")
-	addVotes(cs, v)
+	votes := make([]*types.Vote, 0, len(vss))
+
+	for _, vs := range vss {
+		v, err := vs.signVote(ctx, tmproto.PrecommitType, chainID, blockID, valSet.QuorumType,
+			valSet.QuorumHash, extensions.Copy())
+		require.NoError(t, err, "failed to sign vote")
+		vs.lastVote = v
+		votes = append(votes, v)
+
+		protx, _ := vs.GetProTxHash(ctx)
+		q, _ := vs.GetPubKey(ctx, valSet.QuorumHash)
+		fmt.Printf("signAddPrecommitsWithExtension: pubkey: %X, sigs %X, val protxhash(%d): %X\n", q.Bytes(), v.VoteExtensions.GetSignatures(), vs.Index, protx)
+	}
+	addVotes(cs, votes...)
 }
 
 // mockProposerApplicationCalls configures mock Application `m` to support calls executed for each round on the proposer.

internal/consensus/vote_signer_test.go

@@ -87,25 +87,25 @@ func TestVoteSigner_signAddVote(t *testing.T) {
 		{
 			msgType:       tmproto.PrevoteType,
 			blockID:       blockID,
-			wantBlockSign: "8400E1599EB8E954C7DBE069B14941B71E010D92F4999B4A64752AF1969EE9AF140F953A40F212F0E63C4EF92C5F9BDA1930D753B1522183C7EA46EC847C9053FE4AFAEA17B60C263F015380497B64F4CE2480D01CB4DE9A64F9C8E048472CF2",
+			wantBlockSign: "8B52677D4D455125808EDEE715D2A999695A6701E477C1F44CEDCCE3FC62FB88698D0B6B3CA0429E17EDA9DBCEA932720C189E21F5A6FB31B2C244152F0CD7988598AD572E5D605164554C80880BDC130E23C9DBEF20CF315D05F8C13B6C92CC",
 		},
 		{
 			msgType:       tmproto.PrecommitType,
-			wantBlockSign: "88AB6D08FC3E9D258A1CB78288EF00E49F245A4E9AF5CAFF0DBEBDB31CB0098D7B4A93424681F1747686163938CE6647023424C47E7ED5656F33693D112853D6483CE795F788ED3A657F74B58C2215CA056324EC33DF6C44608DA65B13563224",
+			wantBlockSign: "97CCF337D8FCA05E600EAAF769D73BE9A0D1466CAE85374E9E0EF4C3DD1759131E1D2C8B9E8D8D28EBEF27074669D46C0820DF4DA337DFFA6B3EB5BEEA4B78CA8EA131ED584609D227025DB96990C732C2D04A693BC0402B8A19229ED32A51B8",
 		},
 		{
 			msgType:        tmproto.PrecommitType,
 			blockID:        blockID,
 			voteExtensions: nil,
 			mockFn:         mockFn,
-			wantBlockSign:  "B89D8AB4B59B4285A05C59C7C2641EF64DBB1A68C99F55FB2716EE25CDA264A0F87D02767EEBA7124E926325CE36D96D157674633C229D8BFCD8CB039889700D87A2041CF9D44A3D0BC2F231E64EB3815199DCB70184BCDC8CAC593AF3C3FE5F",
+			wantBlockSign:  "9755FA9803D98C344CB16A43B782D2A93ED9A7E7E1C8437482F42781D5EF802EC82442C14C44429737A7355B1F9D87CB139EB2CF193A1CF7C812E38B99221ADF4DAA60CE16550ED6509A9C467A3D4492D77038505235796968465337A1E14B3E",
 		},
 		{
 			msgType:        tmproto.PrecommitType,
 			blockID:        blockID,
 			voteExtensions: types.VoteExtensionsFromProto(voteExtensions...),
 			mockFn:         mockFn,
-			wantBlockSign:  "B89D8AB4B59B4285A05C59C7C2641EF64DBB1A68C99F55FB2716EE25CDA264A0F87D02767EEBA7124E926325CE36D96D157674633C229D8BFCD8CB039889700D87A2041CF9D44A3D0BC2F231E64EB3815199DCB70184BCDC8CAC593AF3C3FE5F",
+			wantBlockSign:  "9755FA9803D98C344CB16A43B782D2A93ED9A7E7E1C8437482F42781D5EF802EC82442C14C44429737A7355B1F9D87CB139EB2CF193A1CF7C812E38B99221ADF4DAA60CE16550ED6509A9C467A3D4492D77038505235796968465337A1E14B3E",
 		},
 	}
 	for i, tc := range testCases {

types/evidence_test.go

@@ -302,7 +302,7 @@ func TestEvidenceVectors(t *testing.T) {
 	}{
 		{"duplicateVoteEvidence",
 			EvidenceList{&DuplicateVoteEvidence{VoteA: v2, VoteB: v}},
-			"c031e2c89a40678572c2353a302d7823b968e0470384471481e0aadd95e4f8c0",
+			"87904f3525bfdb8474a18bc44fcadf76f63f0e7cabc3063f5eae8dcf0eb11d79",
 		},
 	}
 

types/quorum.go

@@ -150,8 +150,13 @@ func (q *QuorumSingsVerifier) verifyVoteExtensions(
 
 	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", i,
-				signItems[i].Msg, 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

types/signs_recoverer.go

@@ -116,7 +116,8 @@ func (v *SignsRecoverer) addVoteExtensionSigs(vote *Vote) {
 	}
 
 	if len(vote.VoteExtensions) != len(v.voteExtensions) {
-		panic(fmt.Sprintf("received vote extensions with different length: current %d, new %d", len(v.voteExtensions), len(v.voteExtensions)))
+		panic(fmt.Sprintf("received vote extensions with different length: current %d, received %d",
+			len(v.voteExtensions), len(vote.VoteExtensions)))
 	}
 
 	// append signatures from this vote to each extension
@@ -125,6 +126,7 @@ func (v *SignsRecoverer) addVoteExtensionSigs(vote *Vote) {
 			if err := recoverable.AddThresholdSignature(vote.ValidatorProTxHash, ext.GetSignature()); err != nil {
 				panic(fmt.Errorf("failed to add vote %s to recover vote extension threshold sig: %w", vote.String(), err))
 			}
+			v.voteExtensions[i] = recoverable
 		}
 	}
 }