abci++: Vote extension cleanup (#8402)

* Split vote verification/validation based on vote extensions Some parts of the code need vote extensions to be verified and validated (mostly in consensus), and other parts of the code don't because its possible that, in some cases (as per RFC 017), we won't have vote extensions. This explicitly facilitates that split. Signed-off-by: Thane Thomson <connect@thanethomson.com> * Only sign extensions in precommits, not prevotes Signed-off-by: Thane Thomson <connect@thanethomson.com> * Update privval/file.go Co-authored-by: M. J. Fromberger <fromberger@interchain.io> * Apply suggestions from code review Co-authored-by: M. J. Fromberger <fromberger@interchain.io> * Temporarily disable extension requirement again for E2E testing Signed-off-by: Thane Thomson <connect@thanethomson.com> * Reorganize comment for clarity Signed-off-by: Thane Thomson <connect@thanethomson.com> * Leave vote validation and pre-call nil check up to caller of VoteToProto Signed-off-by: Thane Thomson <connect@thanethomson.com> * Split complex vote validation test into multiple tests Signed-off-by: Thane Thomson <connect@thanethomson.com> * Universally enforce no vote extensions on any vote type but precommits Signed-off-by: Thane Thomson <connect@thanethomson.com> * Make error messages more generic Signed-off-by: Thane Thomson <connect@thanethomson.com> * Verify with vote extensions when constructing a VoteSet Signed-off-by: Thane Thomson <connect@thanethomson.com> * Expand comment for clarity Signed-off-by: Thane Thomson <connect@thanethomson.com> * Add extension check for prevotes prior to signing votes Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix supporting test code to only inject extensions into precommits Signed-off-by: Thane Thomson <connect@thanethomson.com> * Separate vote malleation from signing in vote tests for clarity Signed-off-by: Thane Thomson <connect@thanethomson.com> * Add extension signature length check and corresponding test Signed-off-by: Thane Thomson <connect@thanethomson.com> * Perform basic vote validation in CommitToVoteSet Signed-off-by: Thane Thomson <connect@thanethomson.com> Co-authored-by: M. J. Fromberger <fromberger@interchain.io>
2026-01-09 22:47:24 +00:00 · 2022-04-28 13:53:44 -04:00
parent 47d52fc78d
commit d69bd64702
10 changed files with 278 additions and 100 deletions
--- a/internal/consensus/common_test.go
+++ b/internal/consensus/common_test.go
@@ -159,7 +159,11 @@ func signVote(
 	chainID string,
 	blockID types.BlockID) *types.Vote {

-	v, err := vs.signVote(ctx, voteType, chainID, blockID, []byte("extension"))
+	var ext []byte
+	if voteType == tmproto.PrecommitType {
+		ext = []byte("extension")
+	}
+	v, err := vs.signVote(ctx, voteType, chainID, blockID, ext)
 	require.NoError(t, err, "failed to sign vote")

 	vs.lastVote = v
--- a/internal/consensus/msgs.go
+++ b/internal/consensus/msgs.go
@@ -220,9 +220,13 @@ type VoteMessage struct {

 func (*VoteMessage) TypeTag() string { return "tendermint/Vote" }

-// ValidateBasic performs basic validation.
+// ValidateBasic checks whether the vote within the message is well-formed.
 func (m *VoteMessage) ValidateBasic() error {
-	return m.Vote.ValidateBasic()
+	// Here we validate votes with vote extensions, since we require vote
+	// extensions to be sent in precommit messages during consensus. Prevote
+	// messages should never have vote extensions, and this is also validated
+	// here.
+	return m.Vote.ValidateWithExtension()
 }

 // String returns a string representation.
@@ -534,6 +538,8 @@ func MsgFromProto(msg *tmcons.Message) (Message, error) {
 			Part:   parts,
 		}
 	case *tmcons.Message_Vote:
+		// Vote validation will be handled in the vote message ValidateBasic
+		// call below.
 		vote, err := types.VoteFromProto(msg.Vote.Vote)
 		if err != nil {
 			return nil, fmt.Errorf("vote msg to proto error: %w", err)
--- a/internal/test/factory/vote.go
+++ b/internal/test/factory/vote.go
@@ -40,5 +40,6 @@ func MakeVote(
 	}

 	v.Signature = vpb.Signature
+	v.ExtensionSignature = vpb.ExtensionSignature
 	return v, nil
 }
--- a/privval/file.go
+++ b/privval/file.go
@@ -372,11 +372,20 @@ func (pv *FilePV) signVote(chainID string, vote *tmproto.Vote) error {
 	}

 	signBytes := types.VoteSignBytes(chainID, vote)
-	extSignBytes := types.VoteExtensionSignBytes(chainID, vote)
-	// We always sign the vote extension. See below for details.
-	extSig, err := pv.Key.PrivKey.Sign(extSignBytes)
-	if err != nil {
-		return err
+
+	// Vote extensions are non-deterministic, so it is possible that an
+	// application may have created a different extension. We therefore always
+	// re-sign the vote extensions of precommits. For prevotes, the extension
+	// signature will always be empty.
+	var extSig []byte
+	if vote.Type == tmproto.PrecommitType {
+		extSignBytes := types.VoteExtensionSignBytes(chainID, vote)
+		extSig, err = pv.Key.PrivKey.Sign(extSignBytes)
+		if err != nil {
+			return err
+		}
+	} else if len(vote.Extension) > 0 {
+		return errors.New("unexpected vote extension - extensions are only allowed in precommits")
 	}

 	// We might crash before writing to the wal,
@@ -402,9 +411,6 @@ func (pv *FilePV) signVote(chainID string, vote *tmproto.Vote) error {
 			vote.Signature = lss.Signature
 		}

-		// Vote extensions are non-deterministic, so it's possible that an
-		// application may have created a different extension. We therefore
-		// always re-sign the vote extension.
 		vote.ExtensionSignature = extSig

 		return nil
--- a/types/block.go
+++ b/types/block.go
@@ -780,7 +780,11 @@ func CommitToVoteSet(chainID string, commit *Commit, vals *ValidatorSet) *VoteSe
 		if commitSig.Absent() {
 			continue // OK, some precommits can be missing.
 		}
-		added, err := voteSet.AddVote(commit.GetVote(int32(idx)))
+		vote := commit.GetVote(int32(idx))
+		if err := vote.ValidateBasic(); err != nil {
+			panic(fmt.Errorf("failed to validate vote reconstructed from LastCommit: %w", err))
+		}
+		added, err := voteSet.AddVote(vote)
 		if !added || err != nil {
 			panic(fmt.Errorf("failed to reconstruct LastCommit: %w", err))
 		}
--- a/types/evidence.go
+++ b/types/evidence.go
@@ -211,14 +211,28 @@ func DuplicateVoteEvidenceFromProto(pb *tmproto.DuplicateVoteEvidence) (*Duplica
 		return nil, errors.New("nil duplicate vote evidence")
 	}

-	vA, err := VoteFromProto(pb.VoteA)
-	if err != nil {
-		return nil, err
+	var vA *Vote
+	if pb.VoteA != nil {
+		var err error
+		vA, err = VoteFromProto(pb.VoteA)
+		if err != nil {
+			return nil, err
+		}
+		if err = vA.ValidateBasic(); err != nil {
+			return nil, err
+		}
 	}

-	vB, err := VoteFromProto(pb.VoteB)
-	if err != nil {
-		return nil, err
+	var vB *Vote
+	if pb.VoteB != nil {
+		var err error
+		vB, err = VoteFromProto(pb.VoteB)
+		if err != nil {
+			return nil, err
+		}
+		if err = vB.ValidateBasic(); err != nil {
+			return nil, err
+		}
 	}

 	dve := &DuplicateVoteEvidence{
--- a/types/priv_validator.go
+++ b/types/priv_validator.go
@@ -96,9 +96,16 @@ func (pv MockPV) SignVote(ctx context.Context, chainID string, vote *tmproto.Vot
 		return err
 	}
 	vote.Signature = sig
-	extSig, err := pv.PrivKey.Sign(extSignBytes)
-	if err != nil {
-		return err
+
+	var extSig []byte
+	// We only sign vote extensions for precommits
+	if vote.Type == tmproto.PrecommitType {
+		extSig, err = pv.PrivKey.Sign(extSignBytes)
+		if err != nil {
+			return err
+		}
+	} else if len(vote.Extension) > 0 {
+		return errors.New("unexpected vote extension - vote extensions are only allowed in precommits")
 	}
 	vote.ExtensionSignature = extSig
 	return nil
--- a/types/vote.go
+++ b/types/vote.go
@@ -61,6 +61,30 @@ type Vote struct {
 	ExtensionSignature []byte                `json:"extension_signature"`
 }

+// VoteFromProto attempts to convert the given serialization (Protobuf) type to
+// our Vote domain type. No validation is performed on the resulting vote -
+// this is left up to the caller to decide whether to call ValidateBasic or
+// ValidateWithExtension.
+func VoteFromProto(pv *tmproto.Vote) (*Vote, error) {
+	blockID, err := BlockIDFromProto(&pv.BlockID)
+	if err != nil {
+		return nil, err
+	}
+
+	return &Vote{
+		Type:               pv.Type,
+		Height:             pv.Height,
+		Round:              pv.Round,
+		BlockID:            *blockID,
+		Timestamp:          pv.Timestamp,
+		ValidatorAddress:   pv.ValidatorAddress,
+		ValidatorIndex:     pv.ValidatorIndex,
+		Signature:          pv.Signature,
+		Extension:          pv.Extension,
+		ExtensionSignature: pv.ExtensionSignature,
+	}, nil
+}
+
 // CommitSig converts the Vote to a CommitSig.
 func (vote *Vote) CommitSig() CommitSig {
 	if vote == nil {
@@ -164,24 +188,49 @@ func (vote *Vote) String() string {
 	)
 }

-func (vote *Vote) Verify(chainID string, pubKey crypto.PubKey) error {
+func (vote *Vote) verifyAndReturnProto(chainID string, pubKey crypto.PubKey) (*tmproto.Vote, error) {
 	if !bytes.Equal(pubKey.Address(), vote.ValidatorAddress) {
-		return ErrVoteInvalidValidatorAddress
+		return nil, ErrVoteInvalidValidatorAddress
 	}
 	v := vote.ToProto()
 	if !pubKey.VerifySignature(VoteSignBytes(chainID, v), vote.Signature) {
-		return ErrVoteInvalidSignature
+		return nil, ErrVoteInvalidSignature
 	}
-	extSignBytes := VoteExtensionSignBytes(chainID, v)
-	// TODO: Remove extension signature nil check to enforce vote extension
-	//       signing once we resolve https://github.com/tendermint/tendermint/issues/8272
-	if vote.ExtensionSignature != nil && !pubKey.VerifySignature(extSignBytes, vote.ExtensionSignature) {
-		return ErrVoteInvalidSignature
+	return v, nil
+}
+
+// Verify checks whether the signature associated with this vote corresponds to
+// the given chain ID and public key. This function does not validate vote
+// extension signatures - to do so, use VerifyWithExtension instead.
+func (vote *Vote) Verify(chainID string, pubKey crypto.PubKey) error {
+	_, err := vote.verifyAndReturnProto(chainID, pubKey)
+	return err
+}
+
+// VerifyWithExtension 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(chainID string, pubKey crypto.PubKey) error {
+	v, err := vote.verifyAndReturnProto(chainID, pubKey)
+	if err != nil {
+		return err
+	}
+	// We only verify vote extension signatures for precommits.
+	if vote.Type == tmproto.PrecommitType {
+		extSignBytes := VoteExtensionSignBytes(chainID, v)
+		// TODO: Remove extension signature nil check to enforce vote extension
+		//       signing once we resolve https://github.com/tendermint/tendermint/issues/8272
+		if vote.ExtensionSignature != nil && !pubKey.VerifySignature(extSignBytes, vote.ExtensionSignature) {
+			return ErrVoteInvalidSignature
+		}
 	}
 	return nil
 }

-// ValidateBasic performs basic validation.
+// ValidateBasic checks whether the vote is well-formed. It does not, however,
+// check vote extensions - for vote validation with vote extension validation,
+// use ValidateWithExtension.
 func (vote *Vote) ValidateBasic() error {
 	if !IsVoteTypeValid(vote.Type) {
 		return errors.New("invalid Type")
@@ -224,10 +273,38 @@ func (vote *Vote) ValidateBasic() error {
 		return fmt.Errorf("signature is too big (max: %d)", MaxSignatureSize)
 	}

-	// TODO: Remove the extension length check such that we always require
-	//       extension signatures to be present.
-	if len(vote.Extension) > 0 && len(vote.ExtensionSignature) == 0 {
-		return errors.New("vote extension signature is missing")
+	// We should only ever see vote extensions in precommits.
+	if vote.Type != tmproto.PrecommitType {
+		if len(vote.Extension) > 0 {
+			return errors.New("unexpected vote extension")
+		}
+		if len(vote.ExtensionSignature) > 0 {
+			return errors.New("unexpected vote extension signature")
+		}
+	}
+
+	return nil
+}
+
+// ValidateWithExtension performs the same validations as ValidateBasic, but
+// additionally checks whether a vote extension signature is present. This
+// function is used in places where vote extension signatures are expected.
+func (vote *Vote) ValidateWithExtension() error {
+	if err := vote.ValidateBasic(); err != nil {
+		return err
+	}
+
+	// We should always see vote extension signatures in precommits
+	if vote.Type == tmproto.PrecommitType {
+		// TODO(thane): Remove extension length check once
+		//              https://github.com/tendermint/tendermint/issues/8272 is
+		//              resolved.
+		if len(vote.Extension) > 0 && len(vote.ExtensionSignature) == 0 {
+			return errors.New("vote extension signature is missing")
+		}
+		if len(vote.ExtensionSignature) > MaxSignatureSize {
+			return fmt.Errorf("vote extension signature is too big (max: %d)", MaxSignatureSize)
+		}
 	}

 	return nil
@@ -269,30 +346,3 @@ func VotesToProto(votes []*Vote) []*tmproto.Vote {
 	}
 	return res
 }
-
-// FromProto converts a proto generetad type to a handwritten type
-// return type, nil if everything converts safely, otherwise nil, error
-func VoteFromProto(pv *tmproto.Vote) (*Vote, error) {
-	if pv == nil {
-		return nil, errors.New("nil vote")
-	}
-
-	blockID, err := BlockIDFromProto(&pv.BlockID)
-	if err != nil {
-		return nil, err
-	}
-
-	vote := new(Vote)
-	vote.Type = pv.Type
-	vote.Height = pv.Height
-	vote.Round = pv.Round
-	vote.BlockID = *blockID
-	vote.Timestamp = pv.Timestamp
-	vote.ValidatorAddress = pv.ValidatorAddress
-	vote.ValidatorIndex = pv.ValidatorIndex
-	vote.Signature = pv.Signature
-	vote.Extension = pv.Extension
-	vote.ExtensionSignature = pv.ExtensionSignature
-
-	return vote, vote.ValidateBasic()
-}
--- a/types/vote_set.go
+++ b/types/vote_set.go
@@ -194,7 +194,7 @@ func (voteSet *VoteSet) addVote(vote *Vote) (added bool, err error) {
 	}

 	// Check signature.
-	if err := vote.Verify(voteSet.chainID, val.PubKey); err != nil {
+	if err := vote.VerifyWithExtension(voteSet.chainID, val.PubKey); err != nil {
 		return false, fmt.Errorf("failed to verify vote with ChainID %s and PubKey %s: %w", voteSet.chainID, val.PubKey, err)
 	}

--- a/types/vote_test.go
+++ b/types/vote_test.go
@@ -24,7 +24,9 @@ func examplePrevote(t *testing.T) *Vote {

 func examplePrecommit(t testing.TB) *Vote {
 	t.Helper()
-	return exampleVote(t, byte(tmproto.PrecommitType))
+	vote := exampleVote(t, byte(tmproto.PrecommitType))
+	vote.ExtensionSignature = []byte("signature")
+	return vote
 }

 func exampleVote(tb testing.TB, t byte) *Vote {
@@ -48,6 +50,7 @@ func exampleVote(tb testing.TB, t byte) *Vote {
 		ValidatorIndex:   56789,
 	}
 }
+
 func TestVoteSignable(t *testing.T) {
 	vote := examplePrecommit(t)
 	v := vote.ToProto()
@@ -221,8 +224,8 @@ func TestVoteExtension(t *testing.T) {
 			includeSignature: true,
 			expectError:      false,
 		},
-		// TODO: Re-enable once
-		// https://github.com/tendermint/tendermint/issues/8272 is resolved.
+		// TODO(thane): Re-enable once
+		// https://github.com/tendermint/tendermint/issues/8272 is resolved
 		//{
 		//	name:             "no extension signature",
 		//	extension:        []byte("extension"),
@@ -269,7 +272,7 @@ func TestVoteExtension(t *testing.T) {
 			if tc.includeSignature {
 				vote.ExtensionSignature = v.ExtensionSignature
 			}
-			err = vote.Verify("test_chain_id", pk)
+			err = vote.VerifyWithExtension("test_chain_id", pk)
 			if tc.expectError {
 				require.Error(t, err)
 			} else {
@@ -336,39 +339,115 @@ func TestVoteString(t *testing.T) {
 	}
 }

-func TestVoteValidateBasic(t *testing.T) {
+func signVote(ctx context.Context, t *testing.T, pv PrivValidator, chainID string, vote *Vote) {
+	t.Helper()
+
+	v := vote.ToProto()
+	require.NoError(t, pv.SignVote(ctx, chainID, v))
+	vote.Signature = v.Signature
+	vote.ExtensionSignature = v.ExtensionSignature
+}
+
+func TestValidVotes(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
 	privVal := NewMockPV()

 	testCases := []struct {
-		testName     string
+		name         string
+		vote         *Vote
 		malleateVote func(*Vote)
-		expectErr    bool
 	}{
-		{"Good Vote", func(v *Vote) {}, false},
-		{"Negative Height", func(v *Vote) { v.Height = -1 }, true},
-		{"Negative Round", func(v *Vote) { v.Round = -1 }, true},
-		{"Invalid BlockID", func(v *Vote) {
-			v.BlockID = BlockID{[]byte{1, 2, 3}, PartSetHeader{111, []byte("blockparts")}}
-		}, true},
-		{"Invalid Address", func(v *Vote) { v.ValidatorAddress = make([]byte, 1) }, true},
-		{"Invalid ValidatorIndex", func(v *Vote) { v.ValidatorIndex = -1 }, true},
-		{"Invalid Signature", func(v *Vote) { v.Signature = nil }, true},
-		{"Too big Signature", func(v *Vote) { v.Signature = make([]byte, MaxSignatureSize+1) }, true},
+		{"good prevote", examplePrevote(t), func(v *Vote) {}},
+		{"good precommit without vote extension", examplePrecommit(t), func(v *Vote) { v.Extension = nil }},
+		{"good precommit with vote extension", examplePrecommit(t), func(v *Vote) { v.Extension = []byte("extension") }},
 	}
 	for _, tc := range testCases {
-		tc := tc
-		t.Run(tc.testName, func(t *testing.T) {
-			ctx, cancel := context.WithCancel(context.Background())
-			defer cancel()
+		signVote(ctx, t, privVal, "test_chain_id", tc.vote)
+		tc.malleateVote(tc.vote)
+		require.NoError(t, tc.vote.ValidateBasic(), "ValidateBasic for %s", tc.name)
+		require.NoError(t, tc.vote.ValidateWithExtension(), "ValidateWithExtension for %s", tc.name)
+	}
+}

-			vote := examplePrecommit(t)
-			v := vote.ToProto()
-			err := privVal.SignVote(ctx, "test_chain_id", v)
-			vote.Signature = v.Signature
-			require.NoError(t, err)
-			tc.malleateVote(vote)
-			assert.Equal(t, tc.expectErr, vote.ValidateBasic() != nil, "Validate Basic had an unexpected result")
-		})
+func TestInvalidVotes(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	privVal := NewMockPV()
+
+	testCases := []struct {
+		name         string
+		malleateVote func(*Vote)
+	}{
+		{"negative height", func(v *Vote) { v.Height = -1 }},
+		{"negative round", func(v *Vote) { v.Round = -1 }},
+		{"invalid block ID", func(v *Vote) { v.BlockID = BlockID{[]byte{1, 2, 3}, PartSetHeader{111, []byte("blockparts")}} }},
+		{"invalid address", func(v *Vote) { v.ValidatorAddress = make([]byte, 1) }},
+		{"invalid validator index", func(v *Vote) { v.ValidatorIndex = -1 }},
+		{"invalid signature", func(v *Vote) { v.Signature = nil }},
+		{"oversized signature", func(v *Vote) { v.Signature = make([]byte, MaxSignatureSize+1) }},
+	}
+	for _, tc := range testCases {
+		prevote := examplePrevote(t)
+		signVote(ctx, t, privVal, "test_chain_id", prevote)
+		tc.malleateVote(prevote)
+		require.Error(t, prevote.ValidateBasic(), "ValidateBasic for %s in invalid prevote", tc.name)
+		require.Error(t, prevote.ValidateWithExtension(), "ValidateWithExtension for %s in invalid prevote", tc.name)
+
+		precommit := examplePrecommit(t)
+		signVote(ctx, t, privVal, "test_chain_id", precommit)
+		tc.malleateVote(precommit)
+		require.Error(t, precommit.ValidateBasic(), "ValidateBasic for %s in invalid precommit", tc.name)
+		require.Error(t, precommit.ValidateWithExtension(), "ValidateWithExtension for %s in invalid precommit", tc.name)
+	}
+}
+
+func TestInvalidPrevotes(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	privVal := NewMockPV()
+
+	testCases := []struct {
+		name         string
+		malleateVote func(*Vote)
+	}{
+		{"vote extension present", func(v *Vote) { v.Extension = []byte("extension") }},
+		{"vote extension signature present", func(v *Vote) { v.ExtensionSignature = []byte("signature") }},
+	}
+	for _, tc := range testCases {
+		prevote := examplePrevote(t)
+		signVote(ctx, t, privVal, "test_chain_id", prevote)
+		tc.malleateVote(prevote)
+		require.Error(t, prevote.ValidateBasic(), "ValidateBasic for %s", tc.name)
+		require.Error(t, prevote.ValidateWithExtension(), "ValidateWithExtension for %s", tc.name)
+	}
+}
+
+func TestInvalidPrecommitExtensions(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	privVal := NewMockPV()
+
+	testCases := []struct {
+		name         string
+		malleateVote func(*Vote)
+	}{
+		{"vote extension present without signature", func(v *Vote) {
+			v.Extension = []byte("extension")
+			v.ExtensionSignature = nil
+		}},
+		// TODO(thane): Re-enable once https://github.com/tendermint/tendermint/issues/8272 is resolved
+		//{"missing vote extension signature", func(v *Vote) { v.ExtensionSignature = nil }},
+		{"oversized vote extension signature", func(v *Vote) { v.ExtensionSignature = make([]byte, MaxSignatureSize+1) }},
+	}
+	for _, tc := range testCases {
+		precommit := examplePrecommit(t)
+		signVote(ctx, t, privVal, "test_chain_id", precommit)
+		tc.malleateVote(precommit)
+		// We don't expect an error from ValidateBasic, because it doesn't
+		// handle vote extensions.
+		require.NoError(t, precommit.ValidateBasic(), "ValidateBasic for %s", tc.name)
+		require.Error(t, precommit.ValidateWithExtension(), "ValidateWithExtension for %s", tc.name)
 	}
 }

@@ -384,21 +463,28 @@ func TestVoteProtobuf(t *testing.T) {
 	require.NoError(t, err)

 	testCases := []struct {
-		msg     string
-		v1      *Vote
-		expPass bool
+		msg                 string
+		vote                *Vote
+		convertsOk          bool
+		passesValidateBasic bool
 	}{
-		{"success", vote, true},
-		{"fail vote validate basic", &Vote{}, false},
-		{"failure nil", nil, false},
+		{"success", vote, true, true},
+		{"fail vote validate basic", &Vote{}, true, false},
 	}
 	for _, tc := range testCases {
-		protoProposal := tc.v1.ToProto()
+		protoProposal := tc.vote.ToProto()

 		v, err := VoteFromProto(protoProposal)
-		if tc.expPass {
+		if tc.convertsOk {
 			require.NoError(t, err)
-			require.Equal(t, tc.v1, v, tc.msg)
+		} else {
+			require.Error(t, err)
+		}
+
+		err = v.ValidateBasic()
+		if tc.passesValidateBasic {
+			require.NoError(t, err)
+			require.Equal(t, tc.vote, v, tc.msg)
 		} else {
 			require.Error(t, err)
 		}