diff --git a/types/block_test.go b/types/block_test.go index ff56b09f2..b37e087d2 100644 --- a/types/block_test.go +++ b/types/block_test.go @@ -660,7 +660,7 @@ func TestExtendedCommitToVoteSet(t *testing.T) { } chainID := voteSet.ChainID() - voteSet2 := extCommit.ToVoteSet(chainID, valSet, true) + voteSet2 := extCommit.ToVoteSet(chainID, valSet, testCase.includeExtension) for i := int32(0); int(i) < len(vals); i++ { vote1 := voteSet.GetByIndex(i) diff --git a/types/vote.go b/types/vote.go index 5a3598387..e81b631d1 100644 --- a/types/vote.go +++ b/types/vote.go @@ -302,6 +302,15 @@ func (vote *Vote) ValidateBasic() error { } } + if vote.Type == tmproto.PrecommitType && !vote.BlockID.IsNil() { + if len(vote.ExtensionSignature) > MaxSignatureSize { + return fmt.Errorf("vote extension signature is too big (max: %d)", MaxSignatureSize) + } + if len(vote.ExtensionSignature) == 0 && len(vote.Extension) != 0 { + return fmt.Errorf("vote extension signature absent on vote with extension") + } + } + return nil } diff --git a/types/vote_test.go b/types/vote_test.go index 6188131e0..66cb8eed2 100644 --- a/types/vote_test.go +++ b/types/vote_test.go @@ -387,13 +387,13 @@ func TestInvalidVotes(t *testing.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.EnsureExtension(), "EnsureExtension for %s in invalid prevote", tc.name) + require.NoError(t, prevote.EnsureExtension(), "EnsureExtension 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.EnsureExtension(), "EnsureExtension for %s in invalid precommit", tc.name) + require.NoError(t, precommit.EnsureExtension(), "EnsureExtension for %s in invalid precommit", tc.name) } } @@ -414,7 +414,7 @@ func TestInvalidPrevotes(t *testing.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.EnsureExtension(), "EnsureExtension for %s", tc.name) + require.NoError(t, prevote.EnsureExtension(), "EnsureExtension for %s", tc.name) } } @@ -431,18 +431,44 @@ func TestInvalidPrecommitExtensions(t *testing.T) { 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.EnsureExtension(), "EnsureExtension for %s", tc.name) + // ValidateBasic ensures that vote extensions, if present, are well formed + require.Error(t, precommit.ValidateBasic(), "ValidateBasic for %s", tc.name) + } +} + +func TestEnsureVoteExtension(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + privVal := NewMockPV() + + testCases := []struct { + name string + malleateVote func(*Vote) + expectError bool + }{ + {"vote extension signature absent", func(v *Vote) { + v.Extension = nil + v.ExtensionSignature = nil + }, true}, + {"vote extension signature present", func(v *Vote) { + v.ExtensionSignature = []byte("extension signature") + }, false}, + } + for _, tc := range testCases { + precommit := examplePrecommit(t) + signVote(ctx, t, privVal, "test_chain_id", precommit) + tc.malleateVote(precommit) + if tc.expectError { + require.Error(t, precommit.EnsureExtension(), "EnsureExtension for %s", tc.name) + } else { + require.NoError(t, precommit.EnsureExtension(), "EnsureExtension for %s", tc.name) + } } }