diff --git a/abci/types/types.go b/abci/types/types.go index 458e8e130..b84c0639b 100644 --- a/abci/types/types.go +++ b/abci/types/types.go @@ -5,8 +5,6 @@ import ( "encoding/json" "github.com/cosmos/gogoproto/jsonpb" - - types "github.com/tendermint/tendermint/proto/tendermint/types" ) const ( @@ -53,14 +51,8 @@ func (r ResponseProcessProposal) IsStatusUnknown() bool { return r.Status == ResponseProcessProposal_UNKNOWN } -// IsOK returns true if Code is OK -func (r ResponseVerifyVoteExtension) IsOK() bool { - return r.Result <= ResponseVerifyVoteExtension_ACCEPT -} - -// IsErr returns true if Code is something other than OK. -func (r ResponseVerifyVoteExtension) IsErr() bool { - return r.Result > ResponseVerifyVoteExtension_ACCEPT +func (r ResponseVerifyVoteExtension) IsAccepted() bool { + return r.Status == ResponseVerifyVoteExtension_ACCEPT } //--------------------------------------------------------------------------- @@ -171,22 +163,3 @@ func MarshalTxResults(r []*ExecTxResult) ([][]byte, error) { // ----------------------------------------------- // construct Result data - -func RespondExtendVote(appDataToSign, appDataSelfAuthenticating []byte) ResponseExtendVote { - return ResponseExtendVote{ - VoteExtension: &types.VoteExtension{ - AppDataToSign: appDataToSign, - AppDataSelfAuthenticating: appDataSelfAuthenticating, - }, - } -} - -func RespondVerifyVoteExtension(ok bool) ResponseVerifyVoteExtension { - result := ResponseVerifyVoteExtension_REJECT - if ok { - result = ResponseVerifyVoteExtension_ACCEPT - } - return ResponseVerifyVoteExtension{ - Result: result, - } -} diff --git a/consensus/common_test.go b/consensus/common_test.go index c9746c1fe..36d9e08b1 100644 --- a/consensus/common_test.go +++ b/consensus/common_test.go @@ -93,7 +93,8 @@ func newValidatorStub(privValidator types.PrivValidator, valIndex int32) *valida func (vs *validatorStub) signVote( voteType tmproto.SignedMsgType, hash []byte, - header types.PartSetHeader) (*types.Vote, error) { + header types.PartSetHeader, + voteExtension []byte) (*types.Vote, error) { pubKey, err := vs.PrivValidator.GetPubKey() if err != nil { @@ -101,43 +102,41 @@ func (vs *validatorStub) signVote( } vote := &types.Vote{ - ValidatorIndex: vs.Index, - ValidatorAddress: pubKey.Address(), + Type: voteType, Height: vs.Height, Round: vs.Round, - Timestamp: tmtime.Now(), - Type: voteType, BlockID: types.BlockID{Hash: hash, PartSetHeader: header}, - VoteExtension: types.VoteExtensionFromProto(kvstore.ConstructVoteExtension(pubKey.Address())), + Timestamp: tmtime.Now(), + ValidatorAddress: pubKey.Address(), + ValidatorIndex: vs.Index, + Extension: voteExtension, } v := vote.ToProto() - if err := vs.PrivValidator.SignVote(test.DefaultTestChainID, v); err != nil { + if err = vs.PrivValidator.SignVote(test.DefaultTestChainID, v); err != nil { return nil, fmt.Errorf("sign vote failed: %w", err) } - // ref: signVote in FilePV, the vote should use the privious vote info when the sign data is the same. + // ref: signVote in FilePV, the vote should use the previous vote info when the sign data is the same. if signDataIsEqual(vs.lastVote, v) { v.Signature = vs.lastVote.Signature v.Timestamp = vs.lastVote.Timestamp + v.ExtensionSignature = vs.lastVote.ExtensionSignature } vote.Signature = v.Signature vote.Timestamp = v.Timestamp + vote.ExtensionSignature = v.ExtensionSignature return vote, err } // Sign vote for type/hash/header func signVote(vs *validatorStub, voteType tmproto.SignedMsgType, hash []byte, header types.PartSetHeader) *types.Vote { - v, err := vs.signVote(voteType, hash, header) + v, err := vs.signVote(voteType, hash, header, []byte("extension")) if err != nil { panic(fmt.Errorf("failed to sign vote: %v", err)) } - // TODO: remove hardcoded vote extension. - // currently set for abci/examples/kvstore/persistent_kvstore.go - v.VoteExtension = types.VoteExtensionFromProto(kvstore.ConstructVoteExtension(v.ValidatorAddress)) - vs.lastVote = v return v @@ -317,21 +316,22 @@ func validatePrecommit( } } + rs := cs.GetRoundState() if lockedBlockHash == nil { - if cs.LockedRound != lockRound || cs.LockedBlock != nil { + if rs.LockedRound != lockRound || rs.LockedBlock != nil { panic(fmt.Sprintf( "Expected to be locked on nil at round %d. Got locked at round %d with block %v", lockRound, - cs.LockedRound, - cs.LockedBlock)) + rs.LockedRound, + rs.LockedBlock)) } } else { - if cs.LockedRound != lockRound || !bytes.Equal(cs.LockedBlock.Hash(), lockedBlockHash) { + if rs.LockedRound != lockRound || !bytes.Equal(rs.LockedBlock.Hash(), lockedBlockHash) { panic(fmt.Sprintf( "Expected block to be locked on round %d, got %d. Got locked block %X, expected %X", lockRound, - cs.LockedRound, - cs.LockedBlock.Hash(), + rs.LockedRound, + rs.LockedBlock.Hash(), lockedBlockHash)) } } @@ -965,5 +965,6 @@ func signDataIsEqual(v1 *types.Vote, v2 *tmproto.Vote) bool { v1.Height == v2.GetHeight() && v1.Round == v2.Round && bytes.Equal(v1.ValidatorAddress.Bytes(), v2.GetValidatorAddress()) && - v1.ValidatorIndex == v2.GetValidatorIndex() + v1.ValidatorIndex == v2.GetValidatorIndex() && + bytes.Equal(v1.Extension, v2.Extension) } diff --git a/consensus/msgs_test.go b/consensus/msgs_test.go index fd7584af8..4b4990cd8 100644 --- a/consensus/msgs_test.go +++ b/consensus/msgs_test.go @@ -339,11 +339,6 @@ func TestConsMsgsVectors(t *testing.T) { } pbProposal := proposal.ToProto() - ext := types.VoteExtension{ - AppDataToSign: []byte("signed"), - AppDataSelfAuthenticating: []byte("auth"), - } - v := &types.Vote{ ValidatorAddress: []byte("add_more_exclamation"), ValidatorIndex: 1, @@ -352,7 +347,7 @@ func TestConsMsgsVectors(t *testing.T) { Timestamp: date, Type: tmproto.PrecommitType, BlockID: bi, - VoteExtension: ext, + Extension: []byte("extension"), } vpb := v.ToProto() @@ -389,7 +384,7 @@ func TestConsMsgsVectors(t *testing.T) { "2a36080110011a3008011204746573741a26080110011a206164645f6d6f72655f6578636c616d6174696f6e5f6d61726b735f636f64652d"}, {"Vote", &tmcons.Message{Sum: &tmcons.Message_Vote{ Vote: &tmcons.Vote{Vote: vpb}}}, - "3280010a7e0802100122480a206164645f6d6f72655f6578636c616d6174696f6e5f6d61726b735f636f64652d1224080112206164645f6d6f72655f6578636c616d6174696f6e5f6d61726b735f636f64652d2a0608c0b89fdc0532146164645f6d6f72655f6578636c616d6174696f6e38014a0e0a067369676e6564120461757468"}, + "327b0a790802100122480a206164645f6d6f72655f6578636c616d6174696f6e5f6d61726b735f636f64652d1224080112206164645f6d6f72655f6578636c616d6174696f6e5f6d61726b735f636f64652d2a0608c0b89fdc0532146164645f6d6f72655f6578636c616d6174696f6e38014a09657874656e73696f6e"}, {"HasVote", &tmcons.Message{Sum: &tmcons.Message_HasVote{ HasVote: &tmcons.HasVote{Height: 1, Round: 1, Type: tmproto.PrevoteType, Index: 1}}}, "3a080801100118012001"}, diff --git a/consensus/state.go b/consensus/state.go index 3148efc1e..5d665716c 100644 --- a/consensus/state.go +++ b/consensus/state.go @@ -765,7 +765,6 @@ func (cs *State) receiveRoutine(maxSteps int) { if err := cs.wal.Write(mi); err != nil { cs.Logger.Error("failed writing to WAL", "err", err) } - // handles proposals, block parts, votes // may generate internal events (votes, complete proposals, 2/3 majorities) cs.handleMsg(mi) @@ -2244,11 +2243,12 @@ func (cs *State) signVote( if err != nil { return nil, err } - vote.VoteExtension = ext + vote.Extension = ext } v := vote.ToProto() err := cs.privValidator.SignVote(cs.state.ChainID, v) vote.Signature = v.Signature + vote.ExtensionSignature = v.ExtensionSignature vote.Timestamp = v.Timestamp return vote, err diff --git a/consensus/state_test.go b/consensus/state_test.go index 648359845..4f0c92ba2 100644 --- a/consensus/state_test.go +++ b/consensus/state_test.go @@ -1421,6 +1421,235 @@ func TestProcessProposalAccept(t *testing.T) { } } +// TestExtendVoteCalled tests that the vote extension methods are called at the +// correct point in the consensus algorithm. +func TestExtendVoteCalled(t *testing.T) { + config := configSetup(t) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + m := abcimocks.NewBaseMock() + m.On("ProcessProposal", mock.Anything).Return(abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_ACCEPT}) + m.On("ExtendVote", mock.Anything).Return(abci.ResponseExtendVote{ + VoteExtension: []byte("extension"), + }) + m.On("VerifyVoteExtension", mock.Anything).Return(abci.ResponseVerifyVoteExtension{ + Status: abci.ResponseVerifyVoteExtension_ACCEPT, + }) + m.On("FinalizeBlock", mock.Anything).Return(abci.ResponseFinalizeBlock{}).Maybe() + cs1, vss := makeState(ctx, t, makeStateArgs{config: config, application: m}) + height, round := cs1.Height, cs1.Round + + proposalCh := subscribe(ctx, t, cs1.eventBus, types.EventQueryCompleteProposal) + newRoundCh := subscribe(ctx, t, cs1.eventBus, types.EventQueryNewRound) + pv1, err := cs1.privValidator.GetPubKey(ctx) + require.NoError(t, err) + addr := pv1.Address() + voteCh := subscribeToVoter(ctx, t, cs1, addr) + + startTestRound(ctx, cs1, cs1.Height, round) + ensureNewRound(t, newRoundCh, height, round) + ensureNewProposal(t, proposalCh, height, round) + + m.AssertNotCalled(t, "ExtendVote", mock.Anything) + + rs := cs1.GetRoundState() + + blockID := types.BlockID{ + Hash: rs.ProposalBlock.Hash(), + PartSetHeader: rs.ProposalBlockParts.Header(), + } + signAddVotes(ctx, t, cs1, tmproto.PrevoteType, config.ChainID(), blockID, vss[1:]...) + ensurePrevoteMatch(t, voteCh, height, round, blockID.Hash) + + ensurePrecommit(t, voteCh, height, round) + + m.AssertCalled(t, "ExtendVote", abci.RequestExtendVote{ + Height: height, + Hash: blockID.Hash, + }) + + m.AssertCalled(t, "VerifyVoteExtension", abci.RequestVerifyVoteExtension{ + Hash: blockID.Hash, + ValidatorAddress: addr, + Height: height, + VoteExtension: []byte("extension"), + }) + + signAddVotes(ctx, t, cs1, tmproto.PrecommitType, config.ChainID(), blockID, vss[1:]...) + ensureNewRound(t, newRoundCh, height+1, 0) + m.AssertExpectations(t) + + // Only 3 of the vote extensions are seen, as consensus proceeds as soon as the +2/3 threshold + // is observed by the consensus engine. + for _, pv := range vss[:3] { + pv, err := pv.GetPubKey(ctx) + require.NoError(t, err) + addr := pv.Address() + m.AssertCalled(t, "VerifyVoteExtension", abci.RequestVerifyVoteExtension{ + Hash: blockID.Hash, + ValidatorAddress: addr, + Height: height, + VoteExtension: []byte("extension"), + }) + } + +} + +// TestVerifyVoteExtensionNotCalledOnAbsentPrecommit tests that the VerifyVoteExtension +// method is not called for a validator's vote that is never delivered. +func TestVerifyVoteExtensionNotCalledOnAbsentPrecommit(t *testing.T) { + config := configSetup(t) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + m := abcimocks.NewBaseMock() + m.On("ProcessProposal", mock.Anything).Return(abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_ACCEPT}) + m.On("ExtendVote", mock.Anything).Return(abci.ResponseExtendVote{ + VoteExtension: []byte("extension"), + }) + m.On("VerifyVoteExtension", mock.Anything).Return(abci.ResponseVerifyVoteExtension{ + Status: abci.ResponseVerifyVoteExtension_ACCEPT, + }) + m.On("FinalizeBlock", mock.Anything).Return(abci.ResponseFinalizeBlock{}).Maybe() + cs1, vss := makeState(ctx, t, makeStateArgs{config: config, application: m}) + height, round := cs1.Height, cs1.Round + + proposalCh := subscribe(ctx, t, cs1.eventBus, types.EventQueryCompleteProposal) + newRoundCh := subscribe(ctx, t, cs1.eventBus, types.EventQueryNewRound) + pv1, err := cs1.privValidator.GetPubKey(ctx) + require.NoError(t, err) + addr := pv1.Address() + voteCh := subscribeToVoter(ctx, t, cs1, addr) + + startTestRound(ctx, cs1, cs1.Height, round) + ensureNewRound(t, newRoundCh, height, round) + ensureNewProposal(t, proposalCh, height, round) + rs := cs1.GetRoundState() + + blockID := types.BlockID{ + Hash: rs.ProposalBlock.Hash(), + PartSetHeader: rs.ProposalBlockParts.Header(), + } + signAddVotes(ctx, t, cs1, tmproto.PrevoteType, config.ChainID(), blockID, vss[2:]...) + ensurePrevoteMatch(t, voteCh, height, round, blockID.Hash) + + ensurePrecommit(t, voteCh, height, round) + + m.AssertCalled(t, "ExtendVote", abci.RequestExtendVote{ + Height: height, + Hash: blockID.Hash, + }) + + m.AssertCalled(t, "VerifyVoteExtension", abci.RequestVerifyVoteExtension{ + Hash: blockID.Hash, + ValidatorAddress: addr, + Height: height, + VoteExtension: []byte("extension"), + }) + + signAddVotes(ctx, t, cs1, tmproto.PrecommitType, config.ChainID(), blockID, vss[2:]...) + ensureNewRound(t, newRoundCh, height+1, 0) + m.AssertExpectations(t) + + // vss[1] did not issue a precommit for the block, ensure that a vote extension + // for its address was not sent to the application. + pv, err := vss[1].GetPubKey(ctx) + require.NoError(t, err) + addr = pv.Address() + + m.AssertNotCalled(t, "VerifyVoteExtension", abci.RequestVerifyVoteExtension{ + Hash: blockID.Hash, + ValidatorAddress: addr, + Height: height, + VoteExtension: []byte("extension"), + }) + +} + +// 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 +// 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) { + config := configSetup(t) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // create a list of vote extensions, one for each validator. + voteExtensions := [][]byte{ + []byte("extension 0"), + []byte("extension 1"), + []byte("extension 2"), + []byte("extension 3"), + } + + m := abcimocks.NewBaseMock() + m.On("ExtendVote", mock.Anything).Return(abci.ResponseExtendVote{ + VoteExtension: voteExtensions[0], + }) + m.On("PrepareProposal", mock.Anything).Return(abci.ResponsePrepareProposal{}).Once() + + cs1, vss := makeState(ctx, t, makeStateArgs{config: config, application: m}) + height, round := cs1.Height, cs1.Round + + newRoundCh := subscribe(ctx, t, cs1.eventBus, types.EventQueryNewRound) + proposalCh := subscribe(ctx, t, cs1.eventBus, types.EventQueryCompleteProposal) + pv1, err := cs1.privValidator.GetPubKey(ctx) + require.NoError(t, err) + addr := pv1.Address() + voteCh := subscribeToVoter(ctx, t, cs1, addr) + + startTestRound(ctx, cs1, height, round) + ensureNewRound(t, newRoundCh, height, round) + ensureNewProposal(t, proposalCh, height, round) + + rs := cs1.GetRoundState() + blockID := types.BlockID{ + Hash: rs.ProposalBlock.Hash(), + PartSetHeader: rs.ProposalBlockParts.Header(), + } + signAddVotes(ctx, t, cs1, tmproto.PrevoteType, config.ChainID(), blockID, vss[1:]...) + + // create a precommit for each validator with the associated vote extension. + for i, vs := range vss[1:] { + signAddPrecommitWithExtension(ctx, t, cs1, config.ChainID(), blockID, voteExtensions[i+1], vs) + } + + ensurePrevote(t, voteCh, height, round) + + // ensure that the height is committed. + ensurePrecommitMatch(t, voteCh, height, round, blockID.Hash) + incrementHeight(vss[1:]...) + + height++ + round = 0 + ensureNewRound(t, newRoundCh, height, round) + incrementRound(vss[1:]...) + incrementRound(vss[1:]...) + incrementRound(vss[1:]...) + round = 3 + + // capture the prepare proposal request. + rpp := abci.RequestPrepareProposal{} + m.On("PrepareProposal", mock.MatchedBy(func(r abci.RequestPrepareProposal) bool { + rpp = r + return true + })).Return(abci.ResponsePrepareProposal{}) + + signAddVotes(ctx, t, cs1, tmproto.PrecommitType, config.ChainID(), types.BlockID{}, vss[1:]...) + ensureNewRound(t, newRoundCh, height, round) + ensureNewProposal(t, proposalCh, height, round) + + // ensure that the proposer received the list of vote extensions from the + // previous height. + for i := range vss { + require.Equal(t, rpp.LocalLastCommit.Votes[i].VoteExtension, voteExtensions[i]) + } +} + func TestFinalizeBlockCalled(t *testing.T) { for _, testCase := range []struct { name string @@ -2090,3 +2319,15 @@ func subscribeUnBuffered(eventBus *types.EventBus, q tmpubsub.Query) <-chan tmpu } return sub.Out() } + +func signAddPrecommitWithExtension(ctx context.Context, + t *testing.T, + cs *State, + chainID string, + blockID types.BlockID, + extension []byte, + stub *validatorStub) { + v, err := stub.signVote(ctx, tmproto.PrecommitType, chainID, blockID, extension) + require.NoError(t, err, "failed to sign vote") + addVotes(cs, v) +} diff --git a/consensus/types/height_vote_set_test.go b/consensus/types/height_vote_set_test.go index ccbdbed9d..6e07c4c27 100644 --- a/consensus/types/height_vote_set_test.go +++ b/consensus/types/height_vote_set_test.go @@ -82,6 +82,7 @@ func makeVoteHR(t *testing.T, height int64, valIndex, round int32, privVals []ty } vote.Signature = v.Signature + vote.ExtensionSignature = v.ExtensionSignature return vote } diff --git a/internal/test/commit.go b/internal/test/commit.go index fd3a8ac5a..de346fef1 100644 --- a/internal/test/commit.go +++ b/internal/test/commit.go @@ -32,6 +32,7 @@ func MakeCommitFromVoteSet(blockID types.BlockID, voteSet *types.VoteSet, valida return nil, err } vote.Signature = v.Signature + vote.ExtensionSignature = v.ExtensionSignature if _, err := voteSet.AddVote(vote); err != nil { return nil, err } @@ -79,10 +80,11 @@ func MakeCommit(blockID types.BlockID, height int64, round int32, valSet *types. } sigs[idx] = types.CommitSig{ - BlockIDFlag: types.BlockIDFlagCommit, - ValidatorAddress: addr, - Timestamp: now, - Signature: v.Signature, + BlockIDFlag: types.BlockIDFlagCommit, + ValidatorAddress: addr, + Timestamp: now, + Signature: v.Signature, + ExtensionSignature: v.ExtensionSignature, } } diff --git a/internal/test/vote.go b/internal/test/vote.go index 180cb943d..b6c563633 100644 --- a/internal/test/vote.go +++ b/internal/test/vote.go @@ -38,5 +38,6 @@ func MakeVote( } v.Signature = vpb.Signature + v.ExtensionSignature = vpb.ExtensionSignature return v, nil } diff --git a/privval/file.go b/privval/file.go index ad5623915..31fa234a3 100644 --- a/privval/file.go +++ b/privval/file.go @@ -82,6 +82,14 @@ type FilePVLastSignState struct { filePath string } +func (lss *FilePVLastSignState) reset() { + lss.Height = 0 + lss.Round = 0 + lss.Step = 0 + lss.Signature = nil + lss.SignBytes = nil +} + // CheckHRS checks the given height, round, step (HRS) against that of the // FilePVLastSignState. It returns an error if the arguments constitute a regression, // or if they match but the SignBytes are empty. @@ -276,12 +284,7 @@ func (pv *FilePV) Save() { // Reset resets all fields in the FilePV. // NOTE: Unsafe! func (pv *FilePV) Reset() { - var sig []byte - pv.LastSignState.Height = 0 - pv.LastSignState.Round = 0 - pv.LastSignState.Step = 0 - pv.LastSignState.Signature = sig - pv.LastSignState.SignBytes = nil + pv.LastSignState.reset() pv.Save() } @@ -312,6 +315,12 @@ 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 + } // We might crash before writing to the wal, // causing us to try to re-sign for the same HRS. @@ -322,11 +331,19 @@ func (pv *FilePV) signVote(chainID string, vote *tmproto.Vote) error { if bytes.Equal(signBytes, lss.SignBytes) { vote.Signature = lss.Signature } else if timestamp, ok := checkVotesOnlyDifferByTimestamp(lss.SignBytes, signBytes); ok { + // Compares the canonicalized votes (i.e. without vote extensions + // or vote extension signatures). vote.Timestamp = timestamp vote.Signature = lss.Signature } else { err = fmt.Errorf("conflicting data") } + + // 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 err } @@ -337,6 +354,8 @@ func (pv *FilePV) signVote(chainID string, vote *tmproto.Vote) error { } pv.saveSigned(height, round, step, signBytes, sig) vote.Signature = sig + vote.ExtensionSignature = extSig + return nil } @@ -396,8 +415,10 @@ func (pv *FilePV) saveSigned(height int64, round int32, step int8, //----------------------------------------------------------------------------------------- -// returns the timestamp from the lastSignBytes. -// returns true if the only difference in the votes is their timestamp. +// Returns the timestamp from the lastSignBytes. +// Returns true if the only difference in the votes is their timestamp. +// Performs these checks on the canonical votes (excluding the vote extension +// and vote extension signatures). func checkVotesOnlyDifferByTimestamp(lastSignBytes, newSignBytes []byte) (time.Time, bool) { var lastVote, newVote tmproto.CanonicalVote if err := protoio.UnmarshalDelimited(lastSignBytes, &lastVote); err != nil { diff --git a/privval/file_test.go b/privval/file_test.go index c847af2fd..33d75044e 100644 --- a/privval/file_test.go +++ b/privval/file_test.go @@ -1,6 +1,7 @@ package privval import ( + "context" "encoding/base64" "fmt" "os" @@ -20,21 +21,14 @@ import ( ) func TestGenLoadValidator(t *testing.T) { - assert := assert.New(t) - - tempKeyFile, err := os.CreateTemp("", "priv_validator_key_") - require.Nil(t, err) - tempStateFile, err := os.CreateTemp("", "priv_validator_state_") - require.Nil(t, err) - - privVal := GenFilePV(tempKeyFile.Name(), tempStateFile.Name()) + privVal, tempKeyFileName, tempStateFileName := newTestFilePV(t) height := int64(100) privVal.LastSignState.Height = height privVal.Save() addr := privVal.GetAddress() - privVal = LoadFilePV(tempKeyFile.Name(), tempStateFile.Name()) + privVal = LoadFilePV(tempKeyFileName, tempStateFileName) assert.Equal(addr, privVal.GetAddress(), "expected privval addr to be the same") assert.Equal(height, privVal.LastSignState.Height, "expected privval.LastHeight to have been saved") } @@ -45,8 +39,8 @@ func TestResetValidator(t *testing.T) { tempStateFile, err := os.CreateTemp("", "priv_validator_state_") require.Nil(t, err) - privVal := GenFilePV(tempKeyFile.Name(), tempStateFile.Name()) - emptyState := FilePVLastSignState{filePath: tempStateFile.Name()} + privVal, _, tempStateFileName := newTestFilePV(t) + emptyState := FilePVLastSignState{filePath: tempStateFileName} // new priv val has empty state assert.Equal(t, privVal.LastSignState, emptyState) @@ -56,7 +50,7 @@ func TestResetValidator(t *testing.T) { voteType := tmproto.PrevoteType randBytes := tmrand.Bytes(tmhash.Size) blockID := types.BlockID{Hash: randBytes, PartSetHeader: types.PartSetHeader{}} - vote := newVote(privVal.Key.Address, 0, height, round, voteType, blockID) + vote := newVote(privVal.Key.Address, 0, height, round, voteType, blockID, nil) err = privVal.SignVote("mychainid", vote.ToProto()) assert.NoError(t, err, "expected no error signing vote") @@ -158,12 +152,7 @@ func TestUnmarshalValidatorKey(t *testing.T) { func TestSignVote(t *testing.T) { assert := assert.New(t) - tempKeyFile, err := os.CreateTemp("", "priv_validator_key_") - require.Nil(t, err) - tempStateFile, err := os.CreateTemp("", "priv_validator_state_") - require.Nil(t, err) - - privVal := GenFilePV(tempKeyFile.Name(), tempStateFile.Name()) + privVal, _, _ := newTestFilePV(t) randbytes := tmrand.Bytes(tmhash.Size) randbytes2 := tmrand.Bytes(tmhash.Size) @@ -177,9 +166,9 @@ func TestSignVote(t *testing.T) { voteType := tmproto.PrevoteType // sign a vote for first time - vote := newVote(privVal.Key.Address, 0, height, round, voteType, block1) + vote := newVote(privVal.Key.Address, 0, height, round, voteType, block1, nil) v := vote.ToProto() - err = privVal.SignVote("mychainid", v) + err := privVal.SignVote("mychainid", v) assert.NoError(err, "expected no error signing vote") // try to sign the same vote again; should be fine @@ -188,10 +177,10 @@ func TestSignVote(t *testing.T) { // now try some bad votes cases := []*types.Vote{ - newVote(privVal.Key.Address, 0, height, round-1, voteType, block1), // round regression - newVote(privVal.Key.Address, 0, height-1, round, voteType, block1), // height regression - newVote(privVal.Key.Address, 0, height-2, round+4, voteType, block1), // height regression and different round - newVote(privVal.Key.Address, 0, height, round, voteType, block2), // different block + newVote(privVal.Key.Address, 0, height, round-1, voteType, block1, nil), // round regression + newVote(privVal.Key.Address, 0, height-1, round, voteType, block1, nil), // height regression + newVote(privVal.Key.Address, 0, height-2, round+4, voteType, block1, nil), // height regression and different round + newVote(privVal.Key.Address, 0, height, round, voteType, block2, nil), // different block } for _, c := range cases { @@ -211,12 +200,7 @@ func TestSignVote(t *testing.T) { func TestSignProposal(t *testing.T) { assert := assert.New(t) - tempKeyFile, err := os.CreateTemp("", "priv_validator_key_") - require.Nil(t, err) - tempStateFile, err := os.CreateTemp("", "priv_validator_state_") - require.Nil(t, err) - - privVal := GenFilePV(tempKeyFile.Name(), tempStateFile.Name()) + privVal, _, _ := newTestFilePV(t) randbytes := tmrand.Bytes(tmhash.Size) randbytes2 := tmrand.Bytes(tmhash.Size) @@ -230,7 +214,7 @@ func TestSignProposal(t *testing.T) { // sign a proposal for first time proposal := newProposal(height, round, block1) pbp := proposal.ToProto() - err = privVal.SignProposal("mychainid", pbp) + err := privVal.SignProposal("mychainid", pbp) assert.NoError(err, "expected no error signing proposal") // try to sign the same proposal again; should be fine @@ -297,30 +281,94 @@ func TestDifferByTimestamp(t *testing.T) { { voteType := tmproto.PrevoteType blockID := types.BlockID{Hash: randbytes, PartSetHeader: types.PartSetHeader{}} - vote := newVote(privVal.Key.Address, 0, height, round, voteType, blockID) + vote := newVote(privVal.Key.Address, 0, height, round, voteType, blockID, nil) v := vote.ToProto() err := privVal.SignVote("mychainid", v) assert.NoError(t, err, "expected no error signing vote") signBytes := types.VoteSignBytes(chainID, v) sig := v.Signature + extSig := v.ExtensionSignature timeStamp := vote.Timestamp // manipulate the timestamp. should get changed back v.Timestamp = v.Timestamp.Add(time.Millisecond) var emptySig []byte v.Signature = emptySig + v.ExtensionSignature = emptySig err = privVal.SignVote("mychainid", v) assert.NoError(t, err, "expected no error on signing same vote") assert.Equal(t, timeStamp, v.Timestamp) assert.Equal(t, signBytes, types.VoteSignBytes(chainID, v)) assert.Equal(t, sig, v.Signature) + assert.Equal(t, extSig, v.ExtensionSignature) } } +func TestVoteExtensionsAreAlwaysSigned(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + privVal, _, _ := newTestFilePV(t) + pubKey, err := privVal.GetPubKey(ctx) + assert.NoError(t, err) + + block := types.BlockID{ + Hash: tmrand.Bytes(tmhash.Size), + PartSetHeader: types.PartSetHeader{Total: 5, Hash: tmrand.Bytes(tmhash.Size)}, + } + + height, round := int64(10), int32(1) + voteType := tmproto.PrecommitType + + // We initially sign this vote without an extension + vote1 := newVote(privVal.Key.Address, 0, height, round, voteType, block, nil) + vpb1 := vote1.ToProto() + + err = privVal.SignVote(ctx, "mychainid", vpb1) + assert.NoError(t, err, "expected no error signing vote") + assert.NotNil(t, vpb1.ExtensionSignature) + + vesb1 := types.VoteExtensionSignBytes("mychainid", vpb1) + assert.True(t, pubKey.VerifySignature(vesb1, vpb1.ExtensionSignature)) + + // We duplicate this vote precisely, including its timestamp, but change + // its extension + vote2 := vote1.Copy() + vote2.Extension = []byte("new extension") + vpb2 := vote2.ToProto() + + err = privVal.SignVote(ctx, "mychainid", vpb2) + assert.NoError(t, err, "expected no error signing same vote with manipulated vote extension") + + // We need to ensure that a valid new extension signature has been created + // that validates against the vote extension sign bytes with the new + // extension, and does not validate against the vote extension sign bytes + // with the old extension. + vesb2 := types.VoteExtensionSignBytes("mychainid", vpb2) + assert.True(t, pubKey.VerifySignature(vesb2, vpb2.ExtensionSignature)) + assert.False(t, pubKey.VerifySignature(vesb1, vpb2.ExtensionSignature)) + + // We now manipulate the timestamp of the vote with the extension, as per + // TestDifferByTimestamp + expectedTimestamp := vpb2.Timestamp + + vpb2.Timestamp = vpb2.Timestamp.Add(time.Millisecond) + vpb2.Signature = nil + vpb2.ExtensionSignature = nil + + err = privVal.SignVote(ctx, "mychainid", vpb2) + assert.NoError(t, err, "expected no error signing same vote with manipulated timestamp and vote extension") + assert.Equal(t, expectedTimestamp, vpb2.Timestamp) + + vesb3 := types.VoteExtensionSignBytes("mychainid", vpb2) + assert.True(t, pubKey.VerifySignature(vesb3, vpb2.ExtensionSignature)) + assert.False(t, pubKey.VerifySignature(vesb1, vpb2.ExtensionSignature)) +} + func newVote(addr types.Address, idx int32, height int64, round int32, - typ tmproto.SignedMsgType, blockID types.BlockID) *types.Vote { + typ tmproto.SignedMsgType, blockID types.BlockID, extension []byte) *types.Vote { return &types.Vote{ ValidatorAddress: addr, ValidatorIndex: idx, @@ -329,6 +377,7 @@ func newVote(addr types.Address, idx int32, height int64, round int32, Type: typ, Timestamp: tmtime.Now(), BlockID: blockID, + Extension: extension, } } @@ -340,3 +389,15 @@ func newProposal(height int64, round int32, blockID types.BlockID) *types.Propos Timestamp: tmtime.Now(), } } + +func newTestFilePV(t *testing.T) (*FilePV, string, string) { + tempKeyFile, err := os.CreateTemp(t.TempDir(), "priv_validator_key_") + require.NoError(t, err) + tempStateFile, err := os.CreateTemp(t.TempDir(), "priv_validator_state_") + require.NoError(t, err) + + privVal, err := GenFilePV(tempKeyFile.Name(), tempStateFile.Name(), "") + require.NoError(t, err) + + return privVal, tempKeyFile.Name(), tempStateFile.Name() +} diff --git a/privval/msgs_test.go b/privval/msgs_test.go index 23a5d86a5..7bb713c33 100644 --- a/privval/msgs_test.go +++ b/privval/msgs_test.go @@ -22,23 +22,14 @@ var stamp = time.Date(2019, 10, 13, 16, 14, 44, 0, time.UTC) func exampleVote() *types.Vote { return &types.Vote{ - Type: tmproto.SignedMsgType(1), - Height: 3, - Round: 2, - Timestamp: stamp, - BlockID: types.BlockID{ - Hash: tmhash.Sum([]byte("blockID_hash")), - PartSetHeader: types.PartSetHeader{ - Total: 1000000, - Hash: tmhash.Sum([]byte("blockID_part_set_header_hash")), - }, - }, + Type: tmproto.PrecommitType, + Height: 3, + Round: 2, + BlockID: types.BlockID{Hash: tmhash.Sum([]byte("blockID_hash")), PartSetHeader: types.PartSetHeader{Total: 1000000, Hash: tmhash.Sum([]byte("blockID_part_set_header_hash"))}}, + Timestamp: stamp, ValidatorAddress: crypto.AddressHash([]byte("validator_address")), ValidatorIndex: 56789, - VoteExtension: types.VoteExtension { - AppDataToSign: []byte("app_data_to_sign"), - AppDataSelfAuthenticating: []byte("app_data_self_authenticating"), - }, + Extension: []byte("extension"), } } diff --git a/proto/tendermint/abci/types.proto b/proto/tendermint/abci/types.proto index 1345c87e2..ea00a8654 100644 --- a/proto/tendermint/abci/types.proto +++ b/proto/tendermint/abci/types.proto @@ -324,7 +324,13 @@ message ResponseExtendVote { } message ResponseVerifyVoteExtension { - bool accept = 1; + VerifyStatus status = 1; + + enum VerifyStatus { + UNKNOWN = 0; + ACCEPT = 1; + REJECT = 2; + } } message ResponseFinalizeBlock { @@ -351,6 +357,9 @@ message CommitInfo { repeated VoteInfo votes = 2 [(gogoproto.nullable) = false]; } +// ExtendedCommitInfo is similar to CommitInfo except that it is only used in +// the PrepareProposal request such that Tendermint can provide vote extensions +// to the application. message ExtendedCommitInfo { // The round at which the block proposer decided in the previous height. int32 round = 1; @@ -421,19 +430,13 @@ message VoteInfo { bool signed_last_block = 2; } -// TODO: move this to core Tendermint data structures -message CanonicalVoteExtension { - bytes extension = 1; - int64 height = 2; - int32 round = 3; - string chain_id = 4; - bytes address = 5; -} - message ExtendedVoteInfo { - Validator validator = 1 [(gogoproto.nullable) = false]; - bool signed_last_block = 2; - bytes vote_extension = 3; + // The validator that sent the vote. + Validator validator = 1 [(gogoproto.nullable) = false]; + // Indicates whether the validator signed the last block, allowing for rewards based on validator availability. + bool signed_last_block = 2; + // Non-deterministic extension provided by the sending validator's application. + bytes vote_extension = 3; } enum MisbehaviorType { diff --git a/proto/tendermint/types/canonical.proto b/proto/tendermint/types/canonical.proto index b7a66d4d2..705561490 100644 --- a/proto/tendermint/types/canonical.proto +++ b/proto/tendermint/types/canonical.proto @@ -36,3 +36,12 @@ message CanonicalVote { string chain_id = 6 [(gogoproto.customname) = "ChainID"]; VoteExtensionToSign vote_extension = 7; } + +// CanonicalVoteExtension provides us a way to serialize a vote extension from +// a particular validator such that we can sign over those serialized bytes. +message CanonicalVoteExtension { + bytes extension = 1; + sfixed64 height = 2; + sfixed64 round = 3; + string chain_id = 4; +} diff --git a/proto/tendermint/types/types.proto b/proto/tendermint/types/types.proto index c2ec36e5d..0595049a3 100644 --- a/proto/tendermint/types/types.proto +++ b/proto/tendermint/types/types.proto @@ -101,8 +101,15 @@ message Vote { [(gogoproto.nullable) = false, (gogoproto.stdtime) = true]; bytes validator_address = 6; int32 validator_index = 7; - bytes signature = 8; - VoteExtension vote_extension = 9; + // Vote signature by the validator if they participated in consensus for the + // associated block. + bytes signature = 8; + // Vote extension provided by the application. Only valid for precommit + // messages. + bytes extension = 9; + // Vote extension signature by the validator if they participated in + // consensus for the associated block. + bytes extension_signature = 10; } // VoteExtension is app-defined additional information to the validator votes. diff --git a/state/execution.go b/state/execution.go index 167a3aa16..d6d8b3ff6 100644 --- a/state/execution.go +++ b/state/execution.go @@ -6,6 +6,7 @@ import ( "time" abci "github.com/tendermint/tendermint/abci/types" + "github.com/tendermint/tendermint/crypto" cryptoenc "github.com/tendermint/tendermint/crypto/encoding" "github.com/tendermint/tendermint/libs/fail" "github.com/tendermint/tendermint/libs/log" @@ -293,29 +294,33 @@ func (blockExec *BlockExecutor) ApplyBlock( return state, nil } -func (blockExec *BlockExecutor) ExtendVote(vote *types.Vote) (types.VoteExtension, error) { +func (blockExec *BlockExecutor) ExtendVote(vote *types.Vote) ([]byte, error) { req := abci.RequestExtendVote{ - Vote: vote.ToProto(), + Hash: vote.BlockID.Hash, + Height: vote.Height, } resp, err := blockExec.proxyApp.ExtendVoteSync(req) if err != nil { - return types.VoteExtension{}, err + panic(fmt.Errorf("ExtendVote call failed: %w", err)) } - return types.VoteExtensionFromProto(resp.VoteExtension), nil + return resp.VoteExtension, nil } func (blockExec *BlockExecutor) VerifyVoteExtension(vote *types.Vote) error { req := abci.RequestVerifyVoteExtension{ - Vote: vote.ToProto(), + Hash: vote.BlockID.Hash, + ValidatorAddress: vote.ValidatorAddress, + Height: vote.Height, + VoteExtension: vote.Extension, } resp, err := blockExec.proxyApp.VerifyVoteExtensionSync(req) if err != nil { - return err + panic(fmt.Errorf("VerifyVoteExtension call failed: %w", err)) } - if resp.IsErr() { + if !resp.IsAccepted() { return types.ErrVoteInvalidExtension } @@ -413,16 +418,39 @@ func buildLastCommitInfo(block *types.Block, store Store, initialHeight int64) a } } +// extendedCommitInfo expects a CommitInfo struct along with all of the +// original votes relating to that commit, including their vote extensions. The +// order of votes does not matter. func extendedCommitInfo(c abci.CommitInfo, votes []*types.Vote) abci.ExtendedCommitInfo { + if len(c.Votes) != len(votes) { + panic(fmt.Sprintf("extendedCommitInfo: number of votes from commit differ from the number of votes supplied (%d != %d)", len(c.Votes), len(votes))) + } + votesByVal := make(map[string]*types.Vote) + for _, vote := range votes { + if vote != nil { + valAddr := vote.ValidatorAddress.String() + if _, ok := votesByVal[valAddr]; ok { + panic(fmt.Sprintf("extendedCommitInfo: found duplicate vote for validator with address %s", valAddr)) + } + votesByVal[valAddr] = vote + } + } vs := make([]abci.ExtendedVoteInfo, len(c.Votes)) for i := range vs { + var ext []byte + // votes[i] will be nil if c.Votes[i].SignedLastBlock is false + if c.Votes[i].SignedLastBlock { + valAddr := crypto.Address(c.Votes[i].Validator.Address).String() + vote, ok := votesByVal[valAddr] + if !ok || vote == nil { + panic(fmt.Sprintf("extendedCommitInfo: validator with address %s signed last block, but could not find vote for it", valAddr)) + } + ext = vote.Extension + } vs[i] = abci.ExtendedVoteInfo{ Validator: c.Votes[i].Validator, SignedLastBlock: c.Votes[i].SignedLastBlock, - /* - TODO: Include vote extensions information when implementing vote extensions. - VoteExtension: []byte{}, - */ + VoteExtension: ext, } } return abci.ExtendedCommitInfo{ diff --git a/state/execution_test.go b/state/execution_test.go index 0d6b27e57..bc9224878 100644 --- a/state/execution_test.go +++ b/state/execution_test.go @@ -715,9 +715,9 @@ func TestEmptyPrepareProposal(t *testing.T) { blockStore, ) pa, _ := state.Validators.GetByIndex(0) - commit, err := makeValidCommit(height, types.BlockID{}, state.Validators, privVals) + commit, votes, err := makeValidCommit(height, types.BlockID{}, state.Validators, privVals) require.NoError(t, err) - _, err = blockExec.CreateProposalBlock(height, state, commit, pa, nil) + _, err = blockExec.CreateProposalBlock(height, state, commit, pa, votes) require.NoError(t, err) } @@ -758,9 +758,9 @@ func TestPrepareProposalTxsAllIncluded(t *testing.T) { blockStore, ) pa, _ := state.Validators.GetByIndex(0) - commit, err := makeValidCommit(height, types.BlockID{}, state.Validators, privVals) + commit, votes, err := makeValidCommit(height, types.BlockID{}, state.Validators, privVals) require.NoError(t, err) - block, err := blockExec.CreateProposalBlock(height, state, commit, pa, nil) + block, err := blockExec.CreateProposalBlock(height, state, commit, pa, votes) require.NoError(t, err) for i, tx := range block.Data.Txs { @@ -811,9 +811,9 @@ func TestPrepareProposalReorderTxs(t *testing.T) { blockStore, ) pa, _ := state.Validators.GetByIndex(0) - commit, err := makeValidCommit(height, types.BlockID{}, state.Validators, privVals) + commit, votes, err := makeValidCommit(height, types.BlockID{}, state.Validators, privVals) require.NoError(t, err) - block, err := blockExec.CreateProposalBlock(height, state, commit, pa, nil) + block, err := blockExec.CreateProposalBlock(height, state, commit, pa, votes) require.NoError(t, err) for i, tx := range block.Data.Txs { require.Equal(t, txs[i], tx) @@ -866,10 +866,10 @@ func TestPrepareProposalErrorOnTooManyTxs(t *testing.T) { blockStore, ) pa, _ := state.Validators.GetByIndex(0) - commit, err := makeValidCommit(height, types.BlockID{}, state.Validators, privVals) + commit, votes, err := makeValidCommit(height, types.BlockID{}, state.Validators, privVals) require.NoError(t, err) - block, err := blockExec.CreateProposalBlock(height, state, commit, pa, nil) + block, err := blockExec.CreateProposalBlock(height, state, commit, pa, votes) require.Nil(t, block) require.ErrorContains(t, err, "transaction data size exceeds maximum") @@ -916,10 +916,10 @@ func TestPrepareProposalErrorOnPrepareProposalError(t *testing.T) { blockStore, ) pa, _ := state.Validators.GetByIndex(0) - commit, err := makeValidCommit(height, types.BlockID{}, state.Validators, privVals) + commit, votes, err := makeValidCommit(height, types.BlockID{}, state.Validators, privVals) require.NoError(t, err) - block, err := blockExec.CreateProposalBlock(height, state, commit, pa, nil) + block, err := blockExec.CreateProposalBlock(height, state, commit, pa, votes) require.Nil(t, block) require.ErrorContains(t, err, "an injected error") diff --git a/state/helpers_test.go b/state/helpers_test.go index f1d5c971e..51152f4e6 100644 --- a/state/helpers_test.go +++ b/state/helpers_test.go @@ -46,7 +46,7 @@ func makeAndCommitGoodBlock( } // Simulate a lastCommit for this block from all validators for the next height - commit, err := makeValidCommit(height, blockID, state.Validators, privVals) + commit, _, err := makeValidCommit(height, blockID, state.Validators, privVals) if err != nil { return state, types.BlockID{}, nil, err } @@ -88,17 +88,20 @@ func makeValidCommit( blockID types.BlockID, vals *types.ValidatorSet, privVals map[string]types.PrivValidator, -) (*types.Commit, error) { - sigs := make([]types.CommitSig, 0) +) (*types.Commit, []*types.Vote, error) { + t.Helper() + sigs := make([]types.CommitSig, vals.Size()) + votes := make([]*types.Vote, vals.Size()) for i := 0; i < vals.Size(); i++ { _, val := vals.GetByIndex(int32(i)) vote, err := types.MakeVote(height, blockID, vals, privVals[val.Address.String()], chainID, time.Now()) if err != nil { return nil, err } - sigs = append(sigs, vote.CommitSig()) + sigs[i] = vote.CommitSig() + votes[i] = vote } - return types.NewCommit(height, 0, blockID, sigs), nil + return types.NewCommit(height, 0, blockID, sigs), votes, nil } func makeState(nVals, height int) (sm.State, dbm.DB, map[string]types.PrivValidator) { diff --git a/test/e2e/app/app.go b/test/e2e/app/app.go index a2f393977..822e549a9 100644 --- a/test/e2e/app/app.go +++ b/test/e2e/app/app.go @@ -4,20 +4,28 @@ import ( "bytes" "context" "encoding/base64" + "encoding/binary" "errors" "fmt" + "math/rand" "os" "path/filepath" "strconv" + "strings" "time" "github.com/tendermint/tendermint/abci/example/kvstore" abci "github.com/tendermint/tendermint/abci/types" + "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/libs/log" "github.com/tendermint/tendermint/version" ) -const appVersion = 1 +const ( + appVersion = 1 + voteExtensionKey string = "extensionSum" + voteExtensionMaxVal int64 = 128 +) // Application is an ABCI application for use by end-to-end tests. It is a // simple key/value store for strings, storing data in memory and persisting @@ -282,8 +290,69 @@ func (app *Application) ApplySnapshotChunk(_ context.Context, req *abci.RequestA return &abci.ResponseApplySnapshotChunk{Result: abci.ResponseApplySnapshotChunk_ACCEPT}, nil } +// PrepareProposal will take the given transactions and attempt to prepare a +// proposal from them when it's our turn to do so. In the process, vote +// extensions from the previous round of consensus, if present, will be used to +// construct a special transaction whose value is the sum of all of the vote +// extensions from the previous round. +// +// NB: Assumes that the supplied transactions do not exceed `req.MaxTxBytes`. +// If adding a special vote extension-generated transaction would cause the +// total number of transaction bytes to exceed `req.MaxTxBytes`, we will not +// append our special vote extension transaction. func (app *Application) PrepareProposal( _ context.Context, req *abci.RequestPrepareProposal) (*abci.ResponsePrepareProposal, error) { + var sum int64 + var extCount int + for _, vote := range req.LocalLastCommit.Votes { + if !vote.SignedLastBlock || len(vote.VoteExtension) == 0 { + continue + } + extValue, err := parseVoteExtension(vote.VoteExtension) + // This should have been verified in VerifyVoteExtension + if err != nil { + panic(fmt.Errorf("failed to parse vote extension in PrepareProposal: %w", err)) + } + valAddr := crypto.Address(vote.Validator.Address) + app.logger.Info("got vote extension value in PrepareProposal", "valAddr", valAddr, "value", extValue) + sum += extValue + extCount++ + } + + if app.cfg.PrepareProposalDelay != 0 { + time.Sleep(app.cfg.PrepareProposalDelay) + } + + // We only generate our special transaction if we have vote extensions + if extCount > 0 { + var totalBytes int64 + extTxPrefix := fmt.Sprintf("%s=", voteExtensionKey) + extTx := []byte(fmt.Sprintf("%s%d", extTxPrefix, sum)) + app.logger.Info("preparing proposal with custom transaction from vote extensions", "tx", extTx) + // Our generated transaction takes precedence over any supplied + // transaction that attempts to modify the "extensionSum" value. + txs := make([][]byte, len(req.Txs)+1) + for i, tx := range req.Txs { + if strings.HasPrefix(string(tx), extTxPrefix) { + continue + } + txs[i] = tx + totalBytes += int64(len(tx)) + } + if totalBytes+int64(len(extTx)) < req.MaxTxBytes { + txs[len(req.Txs)] = extTx + } else { + app.logger.Info( + "too many txs to include special vote extension-generated tx", + "totalBytes", totalBytes, + "MaxTxBytes", req.MaxTxBytes, + "extTx", extTx, + "extTxLen", len(extTx), + ) + } + return abci.ResponsePrepareProposal{Txs: txs} + } + // None of the transactions are modified by this application. txs := make([][]byte, 0, len(req.Txs)) var totalBytes int64 for _, tx := range req.Txs { @@ -294,10 +363,6 @@ func (app *Application) PrepareProposal( txs = append(txs, tx) } - if app.cfg.PrepareProposalDelay != 0 { - time.Sleep(app.cfg.PrepareProposalDelay) - } - return &abci.ResponsePrepareProposal{Txs: txs}, nil } @@ -305,10 +370,19 @@ func (app *Application) PrepareProposal( // It accepts any proposal that does not contain a malformed transaction. func (app *Application) ProcessProposal(_ context.Context, req *abci.RequestProcessProposal) (*abci.ResponseProcessProposal, error) { for _, tx := range req.Txs { - _, _, err := parseTx(tx) + k, v, err := parseTx(tx) if err != nil { + app.logger.Error("malformed transaction in ProcessProposal", "tx", tx, "err", err) return &abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT}, nil } + // Additional check for vote extension-related txs + if k == voteExtensionKey { + _, err := strconv.Atoi(v) + if err != nil { + app.logger.Error("malformed vote extension transaction", k, v, "err", err) + return abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT} + } + } } if app.cfg.ProcessProposalDelay != 0 { @@ -318,6 +392,70 @@ func (app *Application) ProcessProposal(_ context.Context, req *abci.RequestProc return &abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_ACCEPT}, nil } +// ExtendVote will produce vote extensions in the form of random numbers to +// demonstrate vote extension nondeterminism. +// +// In the next block, if there are any vote extensions from the previous block, +// a new transaction will be proposed that updates a special value in the +// key/value store ("extensionSum") with the sum of all of the numbers collected +// from the vote extensions. +func (app *Application) ExtendVote(req abci.RequestExtendVote) abci.ResponseExtendVote { + // We ignore any requests for vote extensions that don't match our expected + // next height. + if req.Height != int64(app.state.Height)+1 { + app.logger.Error( + "got unexpected height in ExtendVote request", + "expectedHeight", app.state.Height+1, + "requestHeight", req.Height, + ) + return abci.ResponseExtendVote{} + } + 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) + app.logger.Info("generated vote extension", "num", num, "ext", fmt.Sprintf("%x", ext[:extLen]), "state.Height", app.state.Height) + return abci.ResponseExtendVote{ + VoteExtension: ext[:extLen], + } +} + +// VerifyVoteExtension simply validates vote extensions from other validators +// without doing anything about them. In this case, it just makes sure that the +// vote extension is a well-formed integer value. +func (app *Application) VerifyVoteExtension(req abci.RequestVerifyVoteExtension) abci.ResponseVerifyVoteExtension { + // We allow vote extensions to be optional + if len(req.VoteExtension) == 0 { + return abci.ResponseVerifyVoteExtension{ + Status: abci.ResponseVerifyVoteExtension_ACCEPT, + } + } + if req.Height != int64(app.state.Height)+1 { + app.logger.Error( + "got unexpected height in VerifyVoteExtension request", + "expectedHeight", app.state.Height, + "requestHeight", req.Height, + ) + return abci.ResponseVerifyVoteExtension{ + Status: abci.ResponseVerifyVoteExtension_REJECT, + } + } + + num, err := parseVoteExtension(req.VoteExtension) + if err != nil { + app.logger.Error("failed to verify vote extension", "req", req, "err", err) + return abci.ResponseVerifyVoteExtension{ + Status: abci.ResponseVerifyVoteExtension_REJECT, + } + } + app.logger.Info("verified vote extension value", "req", req, "num", num) + return abci.ResponseVerifyVoteExtension{ + Status: abci.ResponseVerifyVoteExtension_ACCEPT, + } +} + func (app *Application) Rollback() error { return app.state.Rollback() } @@ -352,3 +490,19 @@ func parseTx(tx []byte) (string, string, error) { } return string(parts[0]), string(parts[1]), nil } + +// parseVoteExtension attempts to parse the given extension data into a positive +// integer value. +func parseVoteExtension(ext []byte) (int64, error) { + num, errVal := binary.Varint(ext) + if errVal == 0 { + return 0, errors.New("vote extension is too small to parse") + } + if errVal < 0 { + return 0, errors.New("vote extension value is too large") + } + if num >= voteExtensionMaxVal { + return 0, fmt.Errorf("vote extension value must be smaller than %d (was %d)", voteExtensionMaxVal, num) + } + return num, nil +} diff --git a/test/e2e/tests/app_test.go b/test/e2e/tests/app_test.go index 801b5a5c7..717d1a530 100644 --- a/test/e2e/tests/app_test.go +++ b/test/e2e/tests/app_test.go @@ -2,8 +2,10 @@ package e2e_test import ( "bytes" + "context" "fmt" "math/rand" + "strconv" "testing" "time" @@ -38,6 +40,7 @@ func TestApp_Hash(t *testing.T) { testNode(t, func(t *testing.T, node e2e.Node) { client, err := node.Client() require.NoError(t, err) + info, err := client.ABCIInfo(ctx) require.NoError(t, err) require.NotEmpty(t, info.Response.LastBlockAppHash, "expected app to return app hash") @@ -102,3 +105,18 @@ func TestApp_Tx(t *testing.T) { }) } + +func TestApp_VoteExtensions(t *testing.T) { + testNode(t, func(ctx context.Context, t *testing.T, node e2e.Node) { + client, err := node.Client() + require.NoError(t, err) + + // This special value should have been created by way of vote extensions + resp, err := client.ABCIQuery(ctx, "", []byte("extensionSum")) + require.NoError(t, err) + + extSum, err := strconv.Atoi(string(resp.Response.Value)) + require.NoError(t, err) + require.GreaterOrEqual(t, extSum, 0) + }) +} diff --git a/tools/tm-signer-harness/internal/test_harness.go b/tools/tm-signer-harness/internal/test_harness.go index 33ae537fe..62004e157 100644 --- a/tools/tm-signer-harness/internal/test_harness.go +++ b/tools/tm-signer-harness/internal/test_harness.go @@ -286,6 +286,7 @@ func (th *TestHarness) TestSignVote() error { return newTestHarnessError(ErrTestSignVoteFailed, err, fmt.Sprintf("voteType=%d", voteType)) } vote.Signature = v.Signature + vote.ExtensionSignature = v.ExtensionSignature th.logger.Debug("Signed vote", "vote", vote) // validate the contents of the vote if err := vote.ValidateBasic(); err != nil { diff --git a/types/block.go b/types/block.go index dbbd4fac3..3677be8af 100644 --- a/types/block.go +++ b/types/block.go @@ -594,21 +594,19 @@ const ( // CommitSig is a part of the Vote included in a Commit. type CommitSig struct { - BlockIDFlag BlockIDFlag `json:"block_id_flag"` - ValidatorAddress Address `json:"validator_address"` - Timestamp time.Time `json:"timestamp"` - Signature []byte `json:"signature"` - VoteExtension VoteExtensionToSign `json:"vote_extension"` + BlockIDFlag BlockIDFlag `json:"block_id_flag"` + ValidatorAddress Address `json:"validator_address"` + Timestamp time.Time `json:"timestamp"` + Signature []byte `json:"signature"` } // NewCommitSigForBlock returns new CommitSig with BlockIDFlagCommit. -func NewCommitSigForBlock(signature []byte, valAddr Address, ts time.Time, ext VoteExtensionToSign) CommitSig { +func NewCommitSigForBlock(signature []byte, valAddr Address, ts time.Time) CommitSig { return CommitSig{ BlockIDFlag: BlockIDFlagCommit, ValidatorAddress: valAddr, Timestamp: ts, Signature: signature, - VoteExtension: ext, } } @@ -641,14 +639,12 @@ func (cs CommitSig) Absent() bool { // 1. first 6 bytes of signature // 2. first 6 bytes of validator address // 3. block ID flag -// 4. first 6 bytes of the vote extension -// 5. timestamp +// 4. timestamp func (cs CommitSig) String() string { - return fmt.Sprintf("CommitSig{%X by %X on %v with %X @ %s}", + return fmt.Sprintf("CommitSig{%X by %X on %v @ %s}", tmbytes.Fingerprint(cs.Signature), tmbytes.Fingerprint(cs.ValidatorAddress), cs.BlockIDFlag, - tmbytes.Fingerprint(cs.VoteExtension.BytesPacked()), CanonicalTime(cs.Timestamp)) } @@ -720,7 +716,6 @@ func (cs *CommitSig) ToProto() *tmproto.CommitSig { ValidatorAddress: cs.ValidatorAddress, Timestamp: cs.Timestamp, Signature: cs.Signature, - VoteExtension: cs.VoteExtension.ToProto(), } } @@ -732,7 +727,6 @@ func (cs *CommitSig) FromProto(csp tmproto.CommitSig) error { cs.ValidatorAddress = csp.ValidatorAddress cs.Timestamp = csp.Timestamp cs.Signature = csp.Signature - cs.VoteExtension = VoteExtensionToSignFromProto(csp.VoteExtension) return cs.ValidateBasic() } @@ -799,7 +793,6 @@ func (commit *Commit) GetVote(valIdx int32) *Vote { ValidatorAddress: commitSig.ValidatorAddress, ValidatorIndex: valIdx, Signature: commitSig.Signature, - VoteExtension: commitSig.VoteExtension.ToVoteExtension(), } } diff --git a/types/block_test.go b/types/block_test.go index cbc4cf65a..041398dc4 100644 --- a/types/block_test.go +++ b/types/block_test.go @@ -243,7 +243,7 @@ func TestCommit(t *testing.T) { require.NotNil(t, commit.BitArray()) assert.Equal(t, bits.NewBitArray(10).Size(), commit.BitArray().Size()) - assert.Equal(t, voteSet.GetByIndex(0), commit.GetByIndex(0)) + assert.Equal(t, voteWithoutExtension(voteSet.GetByIndex(0)), commit.GetByIndex(0)) assert.True(t, commit.IsCommit()) } @@ -526,7 +526,7 @@ func TestCommitToVoteSet(t *testing.T) { voteSet2 := CommitToVoteSet(chainID, commit, valSet) for i := int32(0); int(i) < len(vals); i++ { - vote1 := voteSet.GetByIndex(i) + vote1 := voteWithoutExtension(voteSet.GetByIndex(i)) vote2 := voteSet2.GetByIndex(i) vote3 := commit.GetVote(i) diff --git a/types/canonical.go b/types/canonical.go index 04fd78628..a9e0aede7 100644 --- a/types/canonical.go +++ b/types/canonical.go @@ -51,26 +51,29 @@ func CanonicalizeProposal(chainID string, proposal *tmproto.Proposal) tmproto.Ca } } -func GetVoteExtensionToSign(ext *tmproto.VoteExtension) *tmproto.VoteExtensionToSign { - if ext == nil { - return nil - } - return &tmproto.VoteExtensionToSign{ - AppDataToSign: ext.AppDataToSign, +// CanonicalizeVote transforms the given Vote to a CanonicalVote, which does +// not contain ValidatorIndex and ValidatorAddress fields, or any fields +// relating to vote extensions. +func CanonicalizeVote(chainID string, vote *tmproto.Vote) tmproto.CanonicalVote { + return tmproto.CanonicalVote{ + Type: vote.Type, + Height: vote.Height, // encoded as sfixed64 + Round: int64(vote.Round), // encoded as sfixed64 + BlockID: CanonicalizeBlockID(vote.BlockID), + Timestamp: vote.Timestamp, + ChainID: chainID, } } -// CanonicalizeVote transforms the given Vote to a CanonicalVote, which does -// not contain ValidatorIndex and ValidatorAddress fields. -func CanonicalizeVote(chainID string, vote *tmproto.Vote) tmproto.CanonicalVote { - return tmproto.CanonicalVote{ - Type: vote.Type, - Height: vote.Height, // encoded as sfixed64 - Round: int64(vote.Round), // encoded as sfixed64 - BlockID: CanonicalizeBlockID(vote.BlockID), - Timestamp: vote.Timestamp, - ChainID: chainID, - VoteExtension: GetVoteExtensionToSign(vote.VoteExtension), +// 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, vote *tmproto.Vote) tmproto.CanonicalVoteExtension { + return tmproto.CanonicalVoteExtension{ + Extension: vote.Extension, + Height: vote.Height, + Round: int64(vote.Round), + ChainId: chainID, } } diff --git a/types/evidence_test.go b/types/evidence_test.go index feb0b0195..e4d253531 100644 --- a/types/evidence_test.go +++ b/types/evidence_test.go @@ -251,6 +251,7 @@ func makeVote( require.NoError(t, err) v.Signature = vpb.Signature + v.ExtensionSignature = vpb.ExtensionSignature return v } diff --git a/types/priv_validator.go b/types/priv_validator.go index 49211773a..e947dd27c 100644 --- a/types/priv_validator.go +++ b/types/priv_validator.go @@ -77,11 +77,17 @@ func (pv MockPV) SignVote(chainID string, vote *tmproto.Vote) error { } signBytes := VoteSignBytes(useChainID, vote) + extSignBytes := VoteExtensionSignBytes(useChainID, vote) sig, err := pv.PrivKey.Sign(signBytes) if err != nil { return err } vote.Signature = sig + extSig, err := pv.PrivKey.Sign(extSignBytes) + if err != nil { + return err + } + vote.ExtensionSignature = extSig return nil } diff --git a/types/test_util.go b/types/test_util.go index 0481c1388..ecb4d75d6 100644 --- a/types/test_util.go +++ b/types/test_util.go @@ -44,6 +44,7 @@ func signAddVote(privVal PrivValidator, vote *Vote, voteSet *VoteSet) (signed bo return false, err } vote.Signature = v.Signature + vote.ExtensionSignature = v.ExtensionSignature return voteSet.AddVote(vote) } @@ -77,6 +78,7 @@ func MakeVote( } vote.Signature = v.Signature + vote.ExtensionSignature = v.ExtensionSignature return vote, nil } @@ -99,3 +101,13 @@ func MakeBlock(height int64, txs []Tx, lastCommit *Commit, evidence []Evidence) block.fillHeader() return block } + +// Votes constructed from commits don't have extensions, because we don't store +// the extensions themselves in the commit. This method is used to construct a +// copy of a vote, but nil its extension and signature. +func voteWithoutExtension(v *Vote) *Vote { + vc := v.Copy() + vc.Extension = nil + vc.ExtensionSignature = nil + return vc +} diff --git a/types/validation_test.go b/types/validation_test.go index d194d680e..ccb9f1845 100644 --- a/types/validation_test.go +++ b/types/validation_test.go @@ -148,6 +148,7 @@ func TestValidatorSet_VerifyCommit_CheckAllSignatures(t *testing.T) { err = vals[3].SignVote("CentaurusA", v) require.NoError(t, err) vote.Signature = v.Signature + vote.ExtendedSignature = v.ExtendedSignature commit.Signatures[3] = vote.CommitSig() err = valSet.VerifyCommit(chainID, blockID, h, commit) @@ -174,6 +175,7 @@ func TestValidatorSet_VerifyCommitLight_ReturnsAsSoonAsMajorityOfVotingPowerSign err = vals[3].SignVote("CentaurusA", v) require.NoError(t, err) vote.Signature = v.Signature + vote.ExtendedSignature = v.ExtendedSignature commit.Signatures[3] = vote.CommitSig() err = valSet.VerifyCommitLight(chainID, blockID, h, commit) @@ -198,6 +200,7 @@ func TestValidatorSet_VerifyCommitLightTrusting_ReturnsAsSoonAsTrustLevelOfVotin err = vals[2].SignVote("CentaurusA", v) require.NoError(t, err) vote.Signature = v.Signature + vote.ExtendedSignature = v.ExtendedSignature commit.Signatures[2] = vote.CommitSig() err = valSet.VerifyCommitLightTrusting(chainID, commit, tmmath.Fraction{Numerator: 1, Denominator: 3}) diff --git a/types/vote.go b/types/vote.go index ad2190a7f..37eccbed0 100644 --- a/types/vote.go +++ b/types/vote.go @@ -46,86 +46,19 @@ func NewConflictingVoteError(vote1, vote2 *Vote) *ErrVoteConflictingVotes { // Address is hex bytes. type Address = crypto.Address -// VoteExtensionToSign is a subset of VoteExtension -// that is signed by the validators private key -type VoteExtensionToSign struct { - AppDataToSign []byte `json:"app_data_to_sign"` -} - -func (ext VoteExtensionToSign) ToProto() *tmproto.VoteExtensionToSign { - if ext.IsEmpty() { - return nil - } - return &tmproto.VoteExtensionToSign{ - AppDataToSign: ext.AppDataToSign, - } -} - -func VoteExtensionToSignFromProto(pext *tmproto.VoteExtensionToSign) VoteExtensionToSign { - if pext == nil { - return VoteExtensionToSign{} - } - return VoteExtensionToSign{ - AppDataToSign: pext.AppDataToSign, - } -} - -func (ext VoteExtensionToSign) IsEmpty() bool { - return len(ext.AppDataToSign) == 0 -} - -// BytesPacked returns a bytes-packed representation for -// debugging and human identification. This function should -// not be used for any logical operations. -func (ext VoteExtensionToSign) BytesPacked() []byte { - res := []byte{} - res = append(res, ext.AppDataToSign...) - return res -} - -// ToVoteExtension constructs a VoteExtension from a VoteExtensionToSign -func (ext VoteExtensionToSign) ToVoteExtension() VoteExtension { - return VoteExtension{ - AppDataToSign: ext.AppDataToSign, - } -} - -// VoteExtension is a set of data provided by the application -// that is additionally included in the vote -type VoteExtension struct { - AppDataToSign []byte `json:"app_data_to_sign"` - AppDataSelfAuthenticating []byte `json:"app_data_self_authenticating"` -} - -// ToSign constructs a VoteExtensionToSign from a VoteExtenstion -func (ext VoteExtension) ToSign() VoteExtensionToSign { - return VoteExtensionToSign{ - AppDataToSign: ext.AppDataToSign, - } -} - -// BytesPacked returns a bytes-packed representation for -// debugging and human identification. This function should -// not be used for any logical operations. -func (ext VoteExtension) BytesPacked() []byte { - res := []byte{} - res = append(res, ext.AppDataToSign...) - res = append(res, ext.AppDataSelfAuthenticating...) - return res -} - // Vote represents a prevote, precommit, or commit vote from validators for // consensus. type Vote struct { - Type tmproto.SignedMsgType `json:"type"` - Height int64 `json:"height"` - Round int32 `json:"round"` // assume there will not be greater than 2_147_483_647 rounds - BlockID BlockID `json:"block_id"` // zero if vote is nil. - Timestamp time.Time `json:"timestamp"` - ValidatorAddress Address `json:"validator_address"` - ValidatorIndex int32 `json:"validator_index"` - Signature []byte `json:"signature"` - VoteExtension VoteExtension `json:"vote_extension"` + Type tmproto.SignedMsgType `json:"type"` + Height int64 `json:"height"` + Round int32 `json:"round"` // assume there will not be greater than 2_147_483_647 rounds + BlockID BlockID `json:"block_id"` // zero if vote is nil. + Timestamp time.Time `json:"timestamp"` + ValidatorAddress Address `json:"validator_address"` + ValidatorIndex int32 `json:"validator_index"` + Signature []byte `json:"signature"` + Extension []byte `json:"extension"` + ExtensionSignature []byte `json:"extension_signature"` } // CommitSig converts the Vote to a CommitSig. @@ -149,12 +82,11 @@ func (vote *Vote) CommitSig() CommitSig { ValidatorAddress: vote.ValidatorAddress, Timestamp: vote.Timestamp, Signature: vote.Signature, - VoteExtension: vote.VoteExtension.ToSign(), } } // VoteSignBytes returns the proto-encoding of the canonicalized Vote, for -// signing. Panics is the marshaling fails. +// signing. Panics if the marshaling fails. // // The encoded Protobuf message is varint length-prefixed (using MarshalDelimited) // for backwards-compatibility with the Amino encoding, due to e.g. hardware @@ -171,9 +103,23 @@ func VoteSignBytes(chainID string, vote *tmproto.Vote) []byte { return bz } +// VoteExtensionSignBytes returns the proto-encoding of the canonicalized vote +// extension for signing. Panics if the marshaling fails. +// +// Similar to VoteSignBytes, the encoded Protobuf message is varint +// length-prefixed for backwards-compatibility with the Amino encoding. +func VoteExtensionSignBytes(chainID string, vote *tmproto.Vote) []byte { + pb := CanonicalizeVoteExtension(chainID, vote) + bz, err := protoio.MarshalDelimited(&pb) + if err != nil { + panic(err) + } + + return bz +} + func (vote *Vote) Copy() *Vote { voteCopy := *vote - voteCopy.VoteExtension = vote.VoteExtension.Copy() return &voteCopy } @@ -213,7 +159,7 @@ func (vote *Vote) String() string { typeString, tmbytes.Fingerprint(vote.BlockID.Hash), tmbytes.Fingerprint(vote.Signature), - tmbytes.Fingerprint(vote.VoteExtension.BytesPacked()), + tmbytes.Fingerprint(vote.Extension), CanonicalTime(vote.Timestamp), ) } @@ -226,6 +172,12 @@ func (vote *Vote) Verify(chainID string, pubKey crypto.PubKey) error { if !pubKey.VerifySignature(VoteSignBytes(chainID, v), vote.Signature) { return 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 nil } @@ -272,40 +224,15 @@ func (vote *Vote) ValidateBasic() error { return fmt.Errorf("signature is too big (max: %d)", MaxSignatureSize) } - // XXX: add length verification for vote extension? + // 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") + } return nil } -func (ext VoteExtension) Copy() VoteExtension { - res := VoteExtension{ - AppDataToSign: ext.AppDataToSign, - AppDataSelfAuthenticating: ext.AppDataSelfAuthenticating, - } - return res -} - -func (ext VoteExtension) IsEmpty() bool { - if len(ext.AppDataToSign) != 0 { - return false - } - if len(ext.AppDataSelfAuthenticating) != 0 { - return false - } - return true -} - -func (ext VoteExtension) ToProto() *tmproto.VoteExtension { - if ext.IsEmpty() { - return nil - } - - return &tmproto.VoteExtension{ - AppDataToSign: ext.AppDataToSign, - AppDataSelfAuthenticating: ext.AppDataSelfAuthenticating, - } -} - // ToProto converts the handwritten type to proto generated type // return type, nil if everything converts safely, otherwise nil, error func (vote *Vote) ToProto() *tmproto.Vote { @@ -314,15 +241,16 @@ func (vote *Vote) ToProto() *tmproto.Vote { } return &tmproto.Vote{ - Type: vote.Type, - Height: vote.Height, - Round: vote.Round, - BlockID: vote.BlockID.ToProto(), - Timestamp: vote.Timestamp, - ValidatorAddress: vote.ValidatorAddress, - ValidatorIndex: vote.ValidatorIndex, - Signature: vote.Signature, - VoteExtension: vote.VoteExtension.ToProto(), + Type: vote.Type, + Height: vote.Height, + Round: vote.Round, + BlockID: vote.BlockID.ToProto(), + Timestamp: vote.Timestamp, + ValidatorAddress: vote.ValidatorAddress, + ValidatorIndex: vote.ValidatorIndex, + Signature: vote.Signature, + Extension: vote.Extension, + ExtensionSignature: vote.ExtensionSignature, } } @@ -342,15 +270,6 @@ func VotesToProto(votes []*Vote) []*tmproto.Vote { return res } -func VoteExtensionFromProto(pext *tmproto.VoteExtension) VoteExtension { - ext := VoteExtension{} - if pext != nil { - ext.AppDataToSign = pext.AppDataToSign - ext.AppDataSelfAuthenticating = pext.AppDataSelfAuthenticating - } - return ext -} - // 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) { @@ -372,7 +291,8 @@ func VoteFromProto(pv *tmproto.Vote) (*Vote, error) { vote.ValidatorAddress = pv.ValidatorAddress vote.ValidatorIndex = pv.ValidatorIndex vote.Signature = pv.Signature - vote.VoteExtension = VoteExtensionFromProto(pv.VoteExtension) + vote.Extension = pv.Extension + vote.ExtensionSignature = pv.ExtensionSignature return vote, vote.ValidateBasic() } diff --git a/types/vote_set_test.go b/types/vote_set_test.go index 4899b04b2..5d70ed3bd 100644 --- a/types/vote_set_test.go +++ b/types/vote_set_test.go @@ -118,6 +118,7 @@ func TestVoteSet_AddVote_Bad(t *testing.T) { t.Errorf("expected VoteSet.Add to fail, wrong type") } } + } func TestVoteSet_2_3Majority(t *testing.T) { diff --git a/types/vote_test.go b/types/vote_test.go index ae7dc518a..78f069e1b 100644 --- a/types/vote_test.go +++ b/types/vote_test.go @@ -1,6 +1,7 @@ package types import ( + "context" "testing" "time" @@ -12,6 +13,7 @@ import ( "github.com/tendermint/tendermint/crypto/ed25519" "github.com/tendermint/tendermint/crypto/tmhash" "github.com/tendermint/tendermint/libs/protoio" + tmtime "github.com/tendermint/tendermint/libs/time" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" ) @@ -129,12 +131,13 @@ func TestVoteSignBytesTestVectors(t *testing.T) { }, // containing vote extension 5: { - "test_chain_id", &Vote{Height: 1, Round: 1, VoteExtension: VoteExtension{ - AppDataToSign: []byte("signed"), - AppDataSelfAuthenticating: []byte("auth"), - }}, + "test_chain_id", &Vote{ + Height: 1, + Round: 1, + Extension: []byte("extension"), + }, []byte{ - 0x38, // length + 0x2e, // length 0x11, // (field_number << 3) | wire_type 0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, // height 0x19, // (field_number << 3) | wire_type @@ -145,13 +148,6 @@ func TestVoteSignBytesTestVectors(t *testing.T) { // (field_number << 3) | wire_type 0x32, 0xd, 0x74, 0x65, 0x73, 0x74, 0x5f, 0x63, 0x68, 0x61, 0x69, 0x6e, 0x5f, 0x69, 0x64, // chainID - // (field_number << 3) | wire_type - 0x3a, - 0x8, // length - 0xa, // (field_number << 3) | wire_type - 0x6, // length - 0x73, 0x69, 0x67, 0x6e, 0x65, 0x64, // AppDataSigned - // SelfAuthenticating data is excluded on signing }, // chainID }, } @@ -204,6 +200,82 @@ func TestVoteVerifySignature(t *testing.T) { require.True(t, valid) } +// TestVoteExtension tests that the vote verification behaves correctly in each case +// of vote extension being set on the vote. +func TestVoteExtension(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + testCases := []struct { + name string + extension []byte + includeSignature bool + expectError bool + }{ + { + name: "all fields present", + extension: []byte("extension"), + includeSignature: true, + expectError: false, + }, + // TODO: Re-enable once + // https://github.com/tendermint/tendermint/issues/8272 is resolved. + //{ + // name: "no extension signature", + // extension: []byte("extension"), + // includeSignature: false, + // expectError: true, + //}, + { + name: "empty extension", + includeSignature: true, + expectError: false, + }, + // TODO: Re-enable once + // https://github.com/tendermint/tendermint/issues/8272 is resolved. + //{ + // name: "no extension and no signature", + // includeSignature: false, + // expectError: true, + //}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + height, round := int64(1), int32(0) + privVal := NewMockPV() + pk, err := privVal.GetPubKey(ctx) + require.NoError(t, err) + blk := Block{} + ps, err := blk.MakePartSet(BlockPartSizeBytes) + require.NoError(t, err) + vote := &Vote{ + ValidatorAddress: pk.Address(), + ValidatorIndex: 0, + Height: height, + Round: round, + Timestamp: tmtime.Now(), + Type: tmproto.PrecommitType, + BlockID: BlockID{blk.Hash(), ps.Header()}, + } + + v := vote.ToProto() + err = privVal.SignVote(ctx, "test_chain_id", v) + require.NoError(t, err) + vote.Signature = v.Signature + if tc.includeSignature { + vote.ExtensionSignature = v.ExtensionSignature + } + err = vote.Verify("test_chain_id", pk) + if tc.expectError { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +} + func TestIsVoteTypeValid(t *testing.T) { tc := []struct { name string