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 <alexanderbez@users.noreply.github.com>

* *Signed -> *ToSign

* add Vote to RequestExtendVote

* add example VoteExtension

* apply reviews

* fix vote

* Apply suggestions from code review

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>

* 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 <alexanderbez@users.noreply.github.com>
Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>

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
+}

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(

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 {

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,
+	}
+}

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

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}
 }

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

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

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()
 	)
 

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"),
 		},
 	}

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)
 		}
 	}()
 

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 {

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()
 }

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
 }
 

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
 	}