diff --git a/consensus/state.go b/consensus/state.go index 9c0f96415d..d2ae99aef8 100644 --- a/consensus/state.go +++ b/consensus/state.go @@ -2395,7 +2395,7 @@ func (cs *State) signVote( recoverable, err := types.SignAndCheckVote(vote, cs.privValidator, cs.state.ChainID, extEnabled && (msgType == cmtproto.PrecommitType)) if err != nil && !recoverable { - panic(fmt.Sprintf("non-recoverable error when signing vote (%d/%d)", vote.Height, vote.Round)) + panic(fmt.Sprintf("non-recoverable error when signing vote %v: %v", vote, err)) } return vote, err diff --git a/types/vote.go b/types/vote.go index 660b538d17..9d7b157546 100644 --- a/types/vote.go +++ b/types/vote.go @@ -47,6 +47,15 @@ func NewConflictingVoteError(vote1, vote2 *Vote) *ErrVoteConflictingVotes { } } +// The vote extension is only valid for non-nil precommits. +type ErrVoteExtensionInvalid struct { + ExtSignature []byte +} + +func (err *ErrVoteExtensionInvalid) Error() string { + return fmt.Sprintf("extensions must be present IFF vote is a non-nil Precommit; extension signature: %X", err.ExtSignature) +} + // Address is hex bytes. type Address = crypto.Address @@ -396,6 +405,9 @@ func VotesToProto(votes []*Vote) []*cmtproto.Vote { return res } +// SignAndCheckVote signs the vote with the given privVal and checks the vote. +// It returns an error if the vote is invalid and a boolean indicating if the +// error is recoverable or not. func SignAndCheckVote( vote *Vote, privVal PrivValidator, @@ -404,33 +416,30 @@ func SignAndCheckVote( ) (bool, error) { v := vote.ToProto() if err := privVal.SignVote(chainID, v); err != nil { - // Failing to sign a vote has always been a recoverable error, this function keeps it that way - return true, err // true = recoverable + // Failing to sign a vote has always been a recoverable error, this + // function keeps it that way. + return true, err } vote.Signature = v.Signature isPrecommit := vote.Type == cmtproto.PrecommitType if !isPrecommit && extensionsEnabled { // Non-recoverable because the caller passed parameters that don't make sense - return false, fmt.Errorf("only Precommit votes may have extensions enabled; vote type: %d", vote.Type) + return false, &ErrVoteExtensionInvalid{ExtSignature: v.ExtensionSignature} } isNil := vote.BlockID.IsZero() extSignature := (len(v.ExtensionSignature) > 0) if extSignature == (!isPrecommit || isNil) { // Non-recoverable because the vote is malformed - return false, fmt.Errorf( - "extensions must be present IFF vote is a non-nil Precommit; present %t, vote type %d, is nil %t", - extSignature, - vote.Type, - isNil, - ) + return false, &ErrVoteExtensionInvalid{ExtSignature: v.ExtensionSignature} } vote.ExtensionSignature = nil if extensionsEnabled { vote.ExtensionSignature = v.ExtensionSignature } + vote.Timestamp = v.Timestamp return true, nil diff --git a/types/vote_test.go b/types/vote_test.go index 9c7e8777f7..f49368ca8d 100644 --- a/types/vote_test.go +++ b/types/vote_test.go @@ -1,6 +1,7 @@ package types import ( + "fmt" "testing" "time" @@ -22,12 +23,13 @@ func examplePrevote() *Vote { func examplePrecommit() *Vote { vote := exampleVote(byte(cmtproto.PrecommitType)) + vote.Extension = []byte("extension") vote.ExtensionSignature = []byte("signature") return vote } func exampleVote(t byte) *Vote { - var stamp, err = time.Parse(TimeFormat, "2017-12-25T03:00:01.234Z") + stamp, err := time.Parse(TimeFormat, "2017-12-25T03:00:01.234Z") if err != nil { panic(err) } @@ -61,7 +63,6 @@ func TestVoteSignable(t *testing.T) { } func TestVoteSignBytesTestVectors(t *testing.T) { - tests := []struct { chainID string vote *Vote @@ -85,7 +86,8 @@ func TestVoteSignBytesTestVectors(t *testing.T) { 0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, // round 0x2a, // (field_number << 3) | wire_type // remaining fields (timestamp): - 0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1}, + 0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1, + }, }, // with proper (fixed size) height and round (PreVote): 2: { @@ -100,7 +102,8 @@ func TestVoteSignBytesTestVectors(t *testing.T) { 0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, // round 0x2a, // (field_number << 3) | wire_type // remaining fields (timestamp): - 0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1}, + 0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1, + }, }, 3: { "", &Vote{Height: 1, Round: 1}, @@ -112,7 +115,8 @@ func TestVoteSignBytesTestVectors(t *testing.T) { 0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, // round // remaining fields (timestamp): 0x2a, - 0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1}, + 0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1, + }, }, // containing non-empty chain_id: 4: { @@ -128,7 +132,8 @@ func TestVoteSignBytesTestVectors(t *testing.T) { 0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1, // timestamp // (field_number << 3) | wire_type 0x32, - 0xd, 0x74, 0x65, 0x73, 0x74, 0x5f, 0x63, 0x68, 0x61, 0x69, 0x6e, 0x5f, 0x69, 0x64}, // chainID + 0xd, 0x74, 0x65, 0x73, 0x74, 0x5f, 0x63, 0x68, 0x61, 0x69, 0x6e, 0x5f, 0x69, 0x64, + }, // chainID }, // containing vote extension 5: { @@ -309,7 +314,7 @@ func TestVoteVerify(t *testing.T) { func TestVoteString(t *testing.T) { str := examplePrecommit().String() - expected := `Vote{56789:6AF1F4111082 12345/02/SIGNED_MSG_TYPE_PRECOMMIT(Precommit) 8B01023386C3 000000000000 000000000000 @ 2017-12-25T03:00:01.234Z}` //nolint:lll //ignore line length for tests + expected := `Vote{56789:6AF1F4111082 12345/02/SIGNED_MSG_TYPE_PRECOMMIT(Precommit) 8B01023386C3 000000000000 657874656E73 @ 2017-12-25T03:00:01.234Z}` //nolint:lll //ignore line length for tests if str != expected { t.Errorf("got unexpected string for Vote. Expected:\n%v\nGot:\n%v", expected, str) } @@ -486,3 +491,110 @@ func TestVoteProtobuf(t *testing.T) { } } } + +func TestSignAndCheckVote(t *testing.T) { + privVal := NewMockPV() + + testCases := []struct { + name string + extensionsEnabled bool + vote *Vote + expectError bool + }{ + { + name: "precommit with extension signature", + extensionsEnabled: true, + vote: examplePrecommit(), + expectError: false, + }, + { + name: "precommit with extension signature", + extensionsEnabled: false, + vote: examplePrecommit(), + expectError: false, + }, + { + name: "precommit with extension signature for a nil block", + extensionsEnabled: true, + vote: func() *Vote { + v := examplePrecommit() + v.BlockID = BlockID{make([]byte, 0), PartSetHeader{0, make([]byte, 0)}} + return v + }(), + expectError: true, + }, + { + name: "precommit with extension signature for a nil block", + extensionsEnabled: false, + vote: func() *Vote { + v := examplePrecommit() + v.BlockID = BlockID{make([]byte, 0), PartSetHeader{0, make([]byte, 0)}} + return v + }(), + expectError: true, + }, + { + name: "precommit without extension", + extensionsEnabled: true, + vote: func() *Vote { + v := examplePrecommit() + v.Extension = make([]byte, 0) + return v + }(), + expectError: false, + }, + { + name: "precommit without extension", + extensionsEnabled: false, + vote: func() *Vote { + v := examplePrecommit() + v.Extension = make([]byte, 0) + return v + }(), + expectError: false, + }, + { + name: "prevote", + extensionsEnabled: true, + vote: examplePrevote(), + expectError: true, + }, + { + name: "prevote", + extensionsEnabled: false, + vote: examplePrevote(), + expectError: false, + }, + { + name: "prevote with extension", + extensionsEnabled: true, + vote: func() *Vote { + v := examplePrevote() + v.Extension = []byte("extension") + return v + }(), + expectError: true, + }, + { + name: "prevote with extension", + extensionsEnabled: false, + vote: func() *Vote { + v := examplePrevote() + v.Extension = []byte("extension") + return v + }(), + expectError: true, + }, + } + + for _, tc := range testCases { + t.Run(fmt.Sprintf("%s (extensionsEnabled: %t) ", tc.name, tc.extensionsEnabled), func(t *testing.T) { + _, err := SignAndCheckVote(tc.vote, privVal, "test_chain_id", tc.extensionsEnabled) + if tc.expectError { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +}