From d2afb91e9954781dc1c68526a0f004cb03137674 Mon Sep 17 00:00:00 2001 From: mconcat Date: Thu, 23 Sep 2021 00:19:09 +0900 Subject: [PATCH] ABCI Vote Extension 2 (#6885) * add proto, add boilerplates * add canonical * fix tests * add vote signing test * Update internal/consensus/msgs_test.go * modify state execution in progress * add extension signing * add extension signing * VoteExtension -> ExtendVote * modify state execution in progress * add extension signing * verify in progress * modify CommitSig * fix test * apply review * update data structures * Apply suggestions from code review * Add comments * fix test * VoteExtensionSigned => VoteExtensionToSigned * Apply suggestions from code review Co-authored-by: Aleksandr Bezobchuk * *Signed -> *ToSign * add Vote to RequestExtendVote * add example VoteExtension * apply reviews * fix vote * Apply suggestions from code review Co-authored-by: Dev Ojha Co-authored-by: Aleksandr Bezobchuk * fix typo, modify proto * add abcipp_kvstore.go * add extension test * fix test * fix test * fix test * fit lint * uncomment test * refactor test in progress * gofmt * apply review * fix lint Co-authored-by: Aleksandr Bezobchuk Co-authored-by: Dev Ojha --- abci/example/kvstore/persistent_kvstore.go | 68 +++++++++++++++++++--- abci/types/application.go | 20 ++++--- abci/types/messages.go | 4 +- abci/types/result.go | 34 +++++++++++ internal/consensus/common_test.go | 5 ++ internal/consensus/mempool_test.go | 2 +- internal/consensus/state.go | 20 ++++--- internal/state/execution.go | 36 +++++++++--- internal/state/execution_test.go | 8 +-- privval/msgs_test.go | 4 +- rpc/client/helpers.go | 2 +- test/e2e/app/app.go | 2 +- types/block.go | 2 + types/vote.go | 38 +++++++++--- types/vote_set.go | 1 + 15 files changed, 196 insertions(+), 50 deletions(-) diff --git a/abci/example/kvstore/persistent_kvstore.go b/abci/example/kvstore/persistent_kvstore.go index 7bf158327..83922eb80 100644 --- a/abci/example/kvstore/persistent_kvstore.go +++ b/abci/example/kvstore/persistent_kvstore.go @@ -14,6 +14,7 @@ import ( "github.com/tendermint/tendermint/crypto/encoding" "github.com/tendermint/tendermint/libs/log" cryptoproto "github.com/tendermint/tendermint/proto/tendermint/crypto" + ptypes "github.com/tendermint/tendermint/proto/tendermint/types" ) const ( @@ -72,6 +73,10 @@ func (app *PersistentKVStoreApplication) DeliverTx(req types.RequestDeliverTx) t return app.execValidatorTx(req.Tx) } + if isPrepareTx(req.Tx) { + return app.execPrepareTx(req.Tx) + } + // otherwise, update the key-value store return app.app.DeliverTx(req) } @@ -168,21 +173,20 @@ func (app *PersistentKVStoreApplication) ApplySnapshotChunk( func (app *PersistentKVStoreApplication) ExtendVote( req types.RequestExtendVote) types.ResponseExtendVote { - return types.ResponseExtendVote{} + return types.ResponseExtendVote{ + VoteExtension: ConstructVoteExtension(req.Vote.ValidatorAddress), + } } func (app *PersistentKVStoreApplication) VerifyVoteExtension( req types.RequestVerifyVoteExtension) types.ResponseVerifyVoteExtension { - return types.ResponseVerifyVoteExtension{} + return types.RespondVerifyVoteExtension( + app.verifyExtension(req.Vote.ValidatorAddress, req.Vote.VoteExtension)) } func (app *PersistentKVStoreApplication) PrepareProposal( req types.RequestPrepareProposal) types.ResponsePrepareProposal { - if len(req.BlockData) >= 1 { - req.BlockData[1] = []byte("modified tx") - } - - return types.ResponsePrepareProposal{BlockData: req.BlockData} + return types.ResponsePrepareProposal{BlockData: app.substPrepareTx(req.BlockData)} } //--------------------------------------------- @@ -299,3 +303,53 @@ func (app *PersistentKVStoreApplication) updateValidator(v types.ValidatorUpdate return types.ResponseDeliverTx{Code: code.CodeTypeOK} } + +// ----------------------------- + +const PreparePrefix = "prepare" + +func isPrepareTx(tx []byte) bool { + return strings.HasPrefix(string(tx), PreparePrefix) +} + +// execPrepareTx is noop. tx data is considered as placeholder +// and is substitute at the PrepareProposal. +func (app *PersistentKVStoreApplication) execPrepareTx(tx []byte) types.ResponseDeliverTx { + // noop + return types.ResponseDeliverTx{} +} + +// substPrepareTx subst all the preparetx in the blockdata +// to null string(could be any arbitrary string). +func (app *PersistentKVStoreApplication) substPrepareTx(blockData [][]byte) [][]byte { + // TODO: this mechanism will change with the current spec of PrepareProposal + // We now have a special type for marking a tx as changed + for i, tx := range blockData { + if isPrepareTx(tx) { + blockData[i] = make([]byte, len(tx)) + } + } + + return blockData +} + +func ConstructVoteExtension(valAddr []byte) *ptypes.VoteExtension { + return &ptypes.VoteExtension{ + AppDataToSign: valAddr, + AppDataSelfAuthenticating: valAddr, + } +} + +func (app *PersistentKVStoreApplication) verifyExtension(valAddr []byte, ext *ptypes.VoteExtension) bool { + if ext == nil { + return false + } + canonical := ConstructVoteExtension(valAddr) + if !bytes.Equal(canonical.AppDataToSign, ext.AppDataToSign) { + return false + } + if !bytes.Equal(canonical.AppDataSelfAuthenticating, ext.AppDataSelfAuthenticating) { + return false + } + return true +} diff --git a/abci/types/application.go b/abci/types/application.go index ca1b47b83..dfbe1bf6e 100644 --- a/abci/types/application.go +++ b/abci/types/application.go @@ -19,12 +19,18 @@ type Application interface { // Consensus Connection InitChain(RequestInitChain) ResponseInitChain // Initialize blockchain w validators/other info from TendermintCore PrepareProposal(RequestPrepareProposal) ResponsePrepareProposal - BeginBlock(RequestBeginBlock) ResponseBeginBlock // Signals the beginning of a block - DeliverTx(RequestDeliverTx) ResponseDeliverTx // Deliver a tx for full processing - EndBlock(RequestEndBlock) ResponseEndBlock // Signals the end of a block, returns changes to the validator set - Commit() ResponseCommit // Commit the state and return the application Merkle root hash - ExtendVote(RequestExtendVote) ResponseExtendVote // Create application specific vote extension - VerifyVoteExtension(RequestVerifyVoteExtension) ResponseVerifyVoteExtension // Verify application's vote extension data + // Signals the beginning of a block + BeginBlock(RequestBeginBlock) ResponseBeginBlock + // Deliver a tx for full processing + DeliverTx(RequestDeliverTx) ResponseDeliverTx + // Signals the end of a block, returns changes to the validator set + EndBlock(RequestEndBlock) ResponseEndBlock + // Commit the state and return the application Merkle root hash + Commit() ResponseCommit + // Create application specific vote extension + ExtendVote(RequestExtendVote) ResponseExtendVote + // Verify application's vote extension data + VerifyVoteExtension(RequestVerifyVoteExtension) ResponseVerifyVoteExtension // State Sync Connection ListSnapshots(RequestListSnapshots) ResponseListSnapshots // List available snapshots @@ -197,7 +203,7 @@ func (app *GRPCApplication) ExtendVote( func (app *GRPCApplication) VerifyVoteExtension( ctx context.Context, req *RequestVerifyVoteExtension) (*ResponseVerifyVoteExtension, error) { res := app.app.VerifyVoteExtension(*req) - return &res, nil + return &res, nil } func (app *GRPCApplication) PrepareProposal( diff --git a/abci/types/messages.go b/abci/types/messages.go index f82632982..ec2a2d28d 100644 --- a/abci/types/messages.go +++ b/abci/types/messages.go @@ -119,7 +119,7 @@ func ToRequestExtendVote(req RequestExtendVote) *Request { func ToRequestVerifyVoteExtension(req RequestVerifyVoteExtension) *Request { return &Request{ Value: &Request_VerifyVoteExtension{&req}, - } + } } func ToRequestPrepareProposal(req RequestPrepareProposal) *Request { @@ -228,7 +228,7 @@ func ToResponseExtendVote(res ResponseExtendVote) *Response { func ToResponseVerifyVoteExtension(res ResponseVerifyVoteExtension) *Response { return &Response{ Value: &Response_VerifyVoteExtension{&res}, - } + } } func ToResponsePrepareProposal(res ResponsePrepareProposal) *Response { diff --git a/abci/types/result.go b/abci/types/result.go index dba6bfd15..a08c3fda5 100644 --- a/abci/types/result.go +++ b/abci/types/result.go @@ -5,6 +5,8 @@ import ( "encoding/json" "github.com/gogo/protobuf/jsonpb" + + types "github.com/tendermint/tendermint/proto/tendermint/types" ) const ( @@ -41,6 +43,16 @@ func (r ResponseQuery) IsErr() bool { return r.Code != CodeTypeOK } +// 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 +} + //--------------------------------------------------------------------------- // override JSON marshaling so we emit defaults (ie. disable omitempty) @@ -118,3 +130,25 @@ var _ jsonRoundTripper = (*ResponseDeliverTx)(nil) var _ jsonRoundTripper = (*ResponseCheckTx)(nil) var _ jsonRoundTripper = (*EventAttribute)(nil) + +// ----------------------------------------------- +// 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/internal/consensus/common_test.go b/internal/consensus/common_test.go index e2f11b566..72a6bb1a5 100644 --- a/internal/consensus/common_test.go +++ b/internal/consensus/common_test.go @@ -125,6 +125,7 @@ func (vs *validatorStub) signVote( Timestamp: vs.clock.Now(), Type: voteType, BlockID: blockID, + VoteExtension: types.VoteExtensionFromProto(kvstore.ConstructVoteExtension(pubKey.Address())), } v := vote.ToProto() if err := vs.PrivValidator.SignVote(ctx, chainID, v); err != nil { @@ -155,6 +156,10 @@ func signVote( v, err := vs.signVote(ctx, voteType, chainID, blockID) require.NoError(t, err, "failed to sign vote") + // 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 diff --git a/internal/consensus/mempool_test.go b/internal/consensus/mempool_test.go index 7c6a769be..0cbee4fcd 100644 --- a/internal/consensus/mempool_test.go +++ b/internal/consensus/mempool_test.go @@ -304,5 +304,5 @@ func (app *CounterApplication) Commit() abci.ResponseCommit { func (app *CounterApplication) PrepareProposal( req abci.RequestPrepareProposal) abci.ResponsePrepareProposal { - return abci.ResponsePrepareProposal{BlockData: req.BlockData} //nolint:gosimple + return abci.ResponsePrepareProposal{BlockData: req.BlockData} } diff --git a/internal/consensus/state.go b/internal/consensus/state.go index ef4cc3bba..e6633d114 100644 --- a/internal/consensus/state.go +++ b/internal/consensus/state.go @@ -2233,6 +2233,13 @@ func (cs *State) addVote( return } + // Verify VoteExtension if precommit + if vote.Type == tmproto.PrecommitType { + if err = cs.blockExec.VerifyVoteExtension(vote); err != nil { + return false, err + } + } + height := cs.Height added, err = cs.Votes.AddVote(vote, peerID) if !added { @@ -2370,8 +2377,6 @@ func (cs *State) signVote( BlockID: types.BlockID{Hash: hash, PartSetHeader: header}, } - v := vote.ToProto() - // If the signedMessageType is for precommit, // use our local precommit Timeout as the max wait time for getting a singed commit. The same goes for prevote. var timeout time.Duration @@ -2391,6 +2396,8 @@ func (cs *State) signVote( timeout = time.Second } + v := vote.ToProto() + ctxto, cancel := context.WithTimeout(ctx, timeout) defer cancel() @@ -2398,16 +2405,15 @@ func (cs *State) signVote( vote.Signature = v.Signature vote.Timestamp = v.Timestamp - return vote, err } // sign the vote and publish on internalMsgQueue func (cs *State) signAddVote( - ctx context.Context, - msgType tmproto.SignedMsgType, - hash []byte, - header types.PartSetHeader, + ctx context.Context, + msgType tmproto.SignedMsgType, + hash []byte, + header types.PartSetHeader, ) *types.Vote { if cs.privValidator == nil { // the node does not have a key return nil diff --git a/internal/state/execution.go b/internal/state/execution.go index 1465c33ed..01922266c 100644 --- a/internal/state/execution.go +++ b/internal/state/execution.go @@ -261,17 +261,35 @@ func (blockExec *BlockExecutor) ApplyBlock( } func (blockExec *BlockExecutor) ExtendVote(vote *types.Vote) (types.VoteExtension, error) { - ctx := context.TODO() - req := abci.RequestExtendVote{ - Vote: vote.ToProto(), - } + ctx := context.Background() + req := abci.RequestExtendVote{ + Vote: vote.ToProto(), + } + + resp, err := blockExec.proxyApp.ExtendVote(ctx, req) + if err != nil { + return types.VoteExtension{}, err + } + + return types.VoteExtensionFromProto(resp.VoteExtension), nil +} + +func (blockExec *BlockExecutor) VerifyVoteExtension(vote *types.Vote) error { + ctx := context.Background() + req := abci.RequestVerifyVoteExtension{ + Vote: vote.ToProto(), + } - resp, err := blockExec.proxyApp.ExtendVote(ctx, req) - if err != nil { - return types.VoteExtension{}, err - } + resp, err := blockExec.proxyApp.VerifyVoteExtension(ctx, req) + if err != nil { + return err + } - return types.VoteExtensionFromProto(resp.VoteExtension), nil + if resp.IsErr() { + return types.ErrVoteInvalidExtension + } + + return nil } // Commit locks the mempool, runs the ABCI Commit message, and updates the diff --git a/internal/state/execution_test.go b/internal/state/execution_test.go index 06f12d640..96b92d9bd 100644 --- a/internal/state/execution_test.go +++ b/internal/state/execution_test.go @@ -91,14 +91,14 @@ func TestBeginBlockValidators(t *testing.T) { []byte("Signature1"), state.Validators.Validators[0].Address, now, - types.VoteExtensionToSign{}, - ) + types.VoteExtensionToSign{}, + ) commitSig1 = types.NewCommitSigForBlock( []byte("Signature2"), state.Validators.Validators[1].Address, now, - types.VoteExtensionToSign{}, - ) + types.VoteExtensionToSign{}, + ) absentSig = types.NewCommitSigAbsent() ) diff --git a/privval/msgs_test.go b/privval/msgs_test.go index 3c15f1089..bbd3f6319 100644 --- a/privval/msgs_test.go +++ b/privval/msgs_test.go @@ -35,8 +35,8 @@ func exampleVote() *types.Vote { }, ValidatorAddress: crypto.AddressHash([]byte("validator_address")), ValidatorIndex: 56789, - VoteExtension: types.VoteExtension { - AppDataToSign: []byte("app_data_signed"), + VoteExtension: types.VoteExtension{ + AppDataToSign: []byte("app_data_signed"), AppDataSelfAuthenticating: []byte("app_data_self_authenticating"), }, } diff --git a/rpc/client/helpers.go b/rpc/client/helpers.go index 2919345e0..529537082 100644 --- a/rpc/client/helpers.go +++ b/rpc/client/helpers.go @@ -73,7 +73,7 @@ func WaitForOneEvent(c EventsClient, eventValue string, timeout time.Duration) ( // make sure to un-register after the test is over defer func() { if deferErr := c.UnsubscribeAll(ctx, subscriber); deferErr != nil { - panic(err) + panic(deferErr) } }() diff --git a/test/e2e/app/app.go b/test/e2e/app/app.go index 2ee57cd86..ad840b6a1 100644 --- a/test/e2e/app/app.go +++ b/test/e2e/app/app.go @@ -269,7 +269,7 @@ func (app *Application) ApplySnapshotChunk(req abci.RequestApplySnapshotChunk) a func (app *Application) PrepareProposal( req abci.RequestPrepareProposal) abci.ResponsePrepareProposal { - return abci.ResponsePrepareProposal{BlockData: req.BlockData} //nolint:gosimple + return abci.ResponsePrepareProposal{BlockData: req.BlockData} } func (app *Application) Rollback() error { diff --git a/types/block.go b/types/block.go index 0caa80195..aef752847 100644 --- a/types/block.go +++ b/types/block.go @@ -728,6 +728,7 @@ func (cs *CommitSig) ToProto() *tmproto.CommitSig { ValidatorAddress: cs.ValidatorAddress, Timestamp: cs.Timestamp, Signature: cs.Signature, + VoteExtension: cs.VoteExtension.ToProto(), } } @@ -739,6 +740,7 @@ 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() } diff --git a/types/vote.go b/types/vote.go index 3dd8ad525..0a3ff2bef 100644 --- a/types/vote.go +++ b/types/vote.go @@ -52,12 +52,34 @@ 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 := make([]byte, len(ext.AppDataToSign)) - copy(res, ext.AppDataToSign) + res := []byte{} + res = append(res, ext.AppDataToSign...) return res } @@ -86,9 +108,9 @@ func (ext VoteExtension) ToSign() VoteExtensionToSign { // debugging and human identification. This function should // not be used for any logical operations. func (ext VoteExtension) BytesPacked() []byte { - res := make([]byte, len(ext.AppDataToSign)+len(ext.AppDataSelfAuthenticating)) - copy(res[:len(ext.AppDataToSign)], ext.AppDataToSign) - copy(res[len(ext.AppDataToSign):], ext.AppDataSelfAuthenticating) + res := []byte{} + res = append(res, ext.AppDataToSign...) + res = append(res, ext.AppDataSelfAuthenticating...) return res } @@ -257,11 +279,9 @@ func (vote *Vote) ValidateBasic() error { func (ext VoteExtension) Copy() VoteExtension { res := VoteExtension{ - AppDataToSign: make([]byte, len(ext.AppDataToSign)), - AppDataSelfAuthenticating: make([]byte, len(ext.AppDataSelfAuthenticating)), + AppDataToSign: ext.AppDataToSign, + AppDataSelfAuthenticating: ext.AppDataSelfAuthenticating, } - copy(res.AppDataToSign, ext.AppDataToSign) - copy(res.AppDataSelfAuthenticating, ext.AppDataSelfAuthenticating) return res } diff --git a/types/vote_set.go b/types/vote_set.go index 4aae267ad..03c287666 100644 --- a/types/vote_set.go +++ b/types/vote_set.go @@ -630,6 +630,7 @@ func (voteSet *VoteSet) MakeCommit() *Commit { if commitSig.ForBlock() && !v.BlockID.Equals(*voteSet.maj23) { commitSig = NewCommitSigAbsent() } + commitSigs[i] = commitSig }