abci: Refactor ABCI CheckTx and DeliverTx signatures (#3735)

* Refactor signature of Application.CheckTx

* Refactor signature of Application.DeliverTx

* Refactor example variable names for clarity and consistency

* Rename method variables for consistency

* Rename method variables for consistency

* add a changelog entry

* update docs

CHANGELOG_PENDING.md

@@ -23,6 +23,7 @@
     If you have `db_backend` set to `leveldb` in your config file, please
     change it to `goleveldb` or `cleveldb`.
   - [p2p] \#3521 Remove NewNetAddressStringWithOptionalID
+  - [abci] \#3193 Use RequestDeliverTx and RequestCheckTx in the ABCI interface
 
 * Blockchain Protocol
 

abci/client/local_client.go

@@ -85,7 +85,7 @@ func (app *localClient) DeliverTxAsync(tx []byte) *ReqRes {
 	app.mtx.Lock()
 	defer app.mtx.Unlock()
 
-	res := app.Application.DeliverTx(tx)
+	res := app.Application.DeliverTx(types.RequestDeliverTx{Tx: tx})
 	return app.callback(
 		types.ToRequestDeliverTx(tx),
 		types.ToResponseDeliverTx(res),
@@ -96,7 +96,7 @@ func (app *localClient) CheckTxAsync(tx []byte) *ReqRes {
 	app.mtx.Lock()
 	defer app.mtx.Unlock()
 
-	res := app.Application.CheckTx(tx)
+	res := app.Application.CheckTx(types.RequestCheckTx{Tx: tx})
 	return app.callback(
 		types.ToRequestCheckTx(tx),
 		types.ToResponseCheckTx(res),
@@ -188,7 +188,7 @@ func (app *localClient) DeliverTxSync(tx []byte) (*types.ResponseDeliverTx, erro
 	app.mtx.Lock()
 	defer app.mtx.Unlock()
 
-	res := app.Application.DeliverTx(tx)
+	res := app.Application.DeliverTx(types.RequestDeliverTx{Tx: tx})
 	return &res, nil
 }
 
@@ -196,7 +196,7 @@ func (app *localClient) CheckTxSync(tx []byte) (*types.ResponseCheckTx, error) {
 	app.mtx.Lock()
 	defer app.mtx.Unlock()
 
-	res := app.Application.CheckTx(tx)
+	res := app.Application.CheckTx(types.RequestCheckTx{Tx: tx})
 	return &res, nil
 }
 

abci/example/counter/counter.go

@@ -42,15 +42,15 @@ func (app *CounterApplication) SetOption(req types.RequestSetOption) types.Respo
 	return types.ResponseSetOption{}
 }
 
-func (app *CounterApplication) DeliverTx(tx []byte) types.ResponseDeliverTx {
+func (app *CounterApplication) DeliverTx(req types.RequestDeliverTx) types.ResponseDeliverTx {
 	if app.serial {
-		if len(tx) > 8 {
+		if len(req.Tx) > 8 {
 			return types.ResponseDeliverTx{
 				Code: code.CodeTypeEncodingError,
-				Log:  fmt.Sprintf("Max tx size is 8 bytes, got %d", len(tx))}
+				Log:  fmt.Sprintf("Max tx size is 8 bytes, got %d", len(req.Tx))}
 		}
 		tx8 := make([]byte, 8)
-		copy(tx8[len(tx8)-len(tx):], tx)
+		copy(tx8[len(tx8)-len(req.Tx):], req.Tx)
 		txValue := binary.BigEndian.Uint64(tx8)
 		if txValue != uint64(app.txCount) {
 			return types.ResponseDeliverTx{
@@ -62,15 +62,15 @@ func (app *CounterApplication) DeliverTx(tx []byte) types.ResponseDeliverTx {
 	return types.ResponseDeliverTx{Code: code.CodeTypeOK}
 }
 
-func (app *CounterApplication) CheckTx(tx []byte) types.ResponseCheckTx {
+func (app *CounterApplication) CheckTx(req types.RequestCheckTx) types.ResponseCheckTx {
 	if app.serial {
-		if len(tx) > 8 {
+		if len(req.Tx) > 8 {
 			return types.ResponseCheckTx{
 				Code: code.CodeTypeEncodingError,
-				Log:  fmt.Sprintf("Max tx size is 8 bytes, got %d", len(tx))}
+				Log:  fmt.Sprintf("Max tx size is 8 bytes, got %d", len(req.Tx))}
 		}
 		tx8 := make([]byte, 8)
-		copy(tx8[len(tx8)-len(tx):], tx)
+		copy(tx8[len(tx8)-len(req.Tx):], req.Tx)
 		txValue := binary.BigEndian.Uint64(tx8)
 		if txValue < uint64(app.txCount) {
 			return types.ResponseCheckTx{

abci/example/kvstore/kvstore.go

@@ -76,13 +76,13 @@ func (app *KVStoreApplication) Info(req types.RequestInfo) (resInfo types.Respon
 }
 
 // tx is either "key=value" or just arbitrary bytes
-func (app *KVStoreApplication) DeliverTx(tx []byte) types.ResponseDeliverTx {
+func (app *KVStoreApplication) DeliverTx(req types.RequestDeliverTx) types.ResponseDeliverTx {
 	var key, value []byte
-	parts := bytes.Split(tx, []byte("="))
+	parts := bytes.Split(req.Tx, []byte("="))
 	if len(parts) == 2 {
 		key, value = parts[0], parts[1]
 	} else {
-		key, value = tx, tx
+		key, value = req.Tx, req.Tx
 	}
 
 	app.state.db.Set(prefixKey(key), value)
@@ -101,7 +101,7 @@ func (app *KVStoreApplication) DeliverTx(tx []byte) types.ResponseDeliverTx {
 	return types.ResponseDeliverTx{Code: code.CodeTypeOK, Events: events}
 }
 
-func (app *KVStoreApplication) CheckTx(tx []byte) types.ResponseCheckTx {
+func (app *KVStoreApplication) CheckTx(req types.RequestCheckTx) types.ResponseCheckTx {
 	return types.ResponseCheckTx{Code: code.CodeTypeOK, GasWanted: 1}
 }
 

abci/example/kvstore/kvstore_test.go

@@ -19,10 +19,11 @@ import (
 )
 
 func testKVStore(t *testing.T, app types.Application, tx []byte, key, value string) {
-	ar := app.DeliverTx(tx)
+	req := types.RequestDeliverTx{Tx: tx}
+	ar := app.DeliverTx(req)
 	require.False(t, ar.IsErr(), ar)
 	// repeating tx doesn't raise error
-	ar = app.DeliverTx(tx)
+	ar = app.DeliverTx(req)
 	require.False(t, ar.IsErr(), ar)
 
 	// make sure query is fine
@@ -179,7 +180,7 @@ func makeApplyBlock(t *testing.T, kvstore types.Application, heightInt int, diff
 
 	kvstore.BeginBlock(types.RequestBeginBlock{Hash: hash, Header: header})
 	for _, tx := range txs {
-		if r := kvstore.DeliverTx(tx); r.IsErr() {
+		if r := kvstore.DeliverTx(types.RequestDeliverTx{Tx: tx}); r.IsErr() {
 			t.Fatal(r)
 		}
 	}

abci/example/kvstore/persistent_kvstore.go

@@ -61,21 +61,21 @@ func (app *PersistentKVStoreApplication) SetOption(req types.RequestSetOption) t
 }
 
 // tx is either "val:pubkey/power" or "key=value" or just arbitrary bytes
-func (app *PersistentKVStoreApplication) DeliverTx(tx []byte) types.ResponseDeliverTx {
+func (app *PersistentKVStoreApplication) DeliverTx(req types.RequestDeliverTx) types.ResponseDeliverTx {
 	// if it starts with "val:", update the validator set
 	// format is "val:pubkey/power"
-	if isValidatorTx(tx) {
+	if isValidatorTx(req.Tx) {
 		// update validators in the merkle tree
 		// and in app.ValUpdates
-		return app.execValidatorTx(tx)
+		return app.execValidatorTx(req.Tx)
 	}
 
 	// otherwise, update the key-value store
-	return app.app.DeliverTx(tx)
+	return app.app.DeliverTx(req)
 }
 
-func (app *PersistentKVStoreApplication) CheckTx(tx []byte) types.ResponseCheckTx {
+func (app *PersistentKVStoreApplication) CheckTx(req types.RequestCheckTx) types.ResponseCheckTx {
-	return app.app.CheckTx(tx)
+	return app.app.CheckTx(req)
 }
 
 // Commit will panic if InitChain was not called

abci/server/socket_server.go

@@ -178,10 +178,10 @@ func (s *SocketServer) handleRequest(req *types.Request, responses chan<- *types
 		res := s.app.SetOption(*r.SetOption)
 		responses <- types.ToResponseSetOption(res)
 	case *types.Request_DeliverTx:
-		res := s.app.DeliverTx(r.DeliverTx.Tx)
+		res := s.app.DeliverTx(*r.DeliverTx)
 		responses <- types.ToResponseDeliverTx(res)
 	case *types.Request_CheckTx:
-		res := s.app.CheckTx(r.CheckTx.Tx)
+		res := s.app.CheckTx(*r.CheckTx)
 		responses <- types.ToResponseCheckTx(res)
 	case *types.Request_Commit:
 		res := s.app.Commit()

abci/types/application.go

@@ -15,12 +15,12 @@ type Application interface {
 	Query(RequestQuery) ResponseQuery             // Query for state
 
 	// Mempool Connection
-	CheckTx(tx []byte) ResponseCheckTx // Validate a tx for the mempool
+	CheckTx(RequestCheckTx) ResponseCheckTx // Validate a tx for the mempool
 
 	// Consensus Connection
 	InitChain(RequestInitChain) ResponseInitChain    // Initialize blockchain with validators and other info from TendermintCore
 	BeginBlock(RequestBeginBlock) ResponseBeginBlock // Signals the beginning of a block
-	DeliverTx(tx []byte) ResponseDeliverTx           // Deliver a tx for full processing
+	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
 }
@@ -45,11 +45,11 @@ func (BaseApplication) SetOption(req RequestSetOption) ResponseSetOption {
 	return ResponseSetOption{}
 }
 
-func (BaseApplication) DeliverTx(tx []byte) ResponseDeliverTx {
+func (BaseApplication) DeliverTx(req RequestDeliverTx) ResponseDeliverTx {
 	return ResponseDeliverTx{Code: CodeTypeOK}
 }
 
-func (BaseApplication) CheckTx(tx []byte) ResponseCheckTx {
+func (BaseApplication) CheckTx(req RequestCheckTx) ResponseCheckTx {
 	return ResponseCheckTx{Code: CodeTypeOK}
 }
 
@@ -103,12 +103,12 @@ func (app *GRPCApplication) SetOption(ctx context.Context, req *RequestSetOption
 }
 
 func (app *GRPCApplication) DeliverTx(ctx context.Context, req *RequestDeliverTx) (*ResponseDeliverTx, error) {
-	res := app.app.DeliverTx(req.Tx)
+	res := app.app.DeliverTx(*req)
 	return &res, nil
 }
 
 func (app *GRPCApplication) CheckTx(ctx context.Context, req *RequestCheckTx) (*ResponseCheckTx, error) {
-	res := app.app.CheckTx(req.Tx)
+	res := app.app.CheckTx(*req)
 	return &res, nil
 }
 

blockchain/reactor_test.go

@@ -291,11 +291,11 @@ func (app *testApp) EndBlock(req abci.RequestEndBlock) abci.ResponseEndBlock {
 	return abci.ResponseEndBlock{}
 }
 
-func (app *testApp) DeliverTx(tx []byte) abci.ResponseDeliverTx {
+func (app *testApp) DeliverTx(req abci.RequestDeliverTx) abci.ResponseDeliverTx {
 	return abci.ResponseDeliverTx{Events: []abci.Event{}}
 }
 
-func (app *testApp) CheckTx(tx []byte) abci.ResponseCheckTx {
+func (app *testApp) CheckTx(req abci.RequestCheckTx) abci.ResponseCheckTx {
 	return abci.ResponseCheckTx{}
 }
 

consensus/mempool_test.go

@@ -141,7 +141,7 @@ func TestMempoolRmBadTx(t *testing.T) {
 	txBytes := make([]byte, 8)
 	binary.BigEndian.PutUint64(txBytes, uint64(0))
 
-	resDeliver := app.DeliverTx(txBytes)
+	resDeliver := app.DeliverTx(abci.RequestDeliverTx{Tx: txBytes})
 	assert.False(t, resDeliver.IsErr(), fmt.Sprintf("expected no error. got %v", resDeliver))
 
 	resCommit := app.Commit()
@@ -209,8 +209,8 @@ func (app *CounterApplication) Info(req abci.RequestInfo) abci.ResponseInfo {
 	return abci.ResponseInfo{Data: fmt.Sprintf("txs:%v", app.txCount)}
 }
 
-func (app *CounterApplication) DeliverTx(tx []byte) abci.ResponseDeliverTx {
+func (app *CounterApplication) DeliverTx(req abci.RequestDeliverTx) abci.ResponseDeliverTx {
-	txValue := txAsUint64(tx)
+	txValue := txAsUint64(req.Tx)
 	if txValue != uint64(app.txCount) {
 		return abci.ResponseDeliverTx{
 			Code: code.CodeTypeBadNonce,
@@ -220,8 +220,8 @@ func (app *CounterApplication) DeliverTx(tx []byte) abci.ResponseDeliverTx {
 	return abci.ResponseDeliverTx{Code: code.CodeTypeOK}
 }
 
-func (app *CounterApplication) CheckTx(tx []byte) abci.ResponseCheckTx {
+func (app *CounterApplication) CheckTx(req abci.RequestCheckTx) abci.ResponseCheckTx {
-	txValue := txAsUint64(tx)
+	txValue := txAsUint64(req.Tx)
 	if txValue != uint64(app.mempoolTxCount) {
 		return abci.ResponseCheckTx{
 			Code: code.CodeTypeBadNonce,

consensus/replay.go

@@ -515,7 +515,7 @@ type mockProxyApp struct {
 	abciResponses *sm.ABCIResponses
 }
 
-func (mock *mockProxyApp) DeliverTx(tx []byte) abci.ResponseDeliverTx {
+func (mock *mockProxyApp) DeliverTx(req abci.RequestDeliverTx) abci.ResponseDeliverTx {
 	r := mock.abciResponses.DeliverTx[mock.txCount]
 	mock.txCount++
 	if r == nil { //it could be nil because of amino unMarshall, it will cause an empty ResponseDeliverTx to become nil

docs/app-dev/app-development.md

@@ -101,8 +101,8 @@ mempool state (this behaviour can be turned off with
 In go:
 
 ```
-func (app *KVStoreApplication) CheckTx(tx []byte) types.Result {
+func (app *KVStoreApplication) CheckTx(req types.RequestCheckTx) types.ResponseCheckTx {
-  return types.OK
+	return types.ResponseCheckTx{Code: code.CodeTypeOK, GasWanted: 1}
 }
 ```
 
@@ -168,14 +168,29 @@ In go:
 
 ```
 // tx is either "key=value" or just arbitrary bytes
-func (app *KVStoreApplication) DeliverTx(tx []byte) types.Result {
+func (app *KVStoreApplication) DeliverTx(req types.RequestDeliverTx) types.ResponseDeliverTx {
-  parts := strings.Split(string(tx), "=")
+	var key, value []byte
+	parts := bytes.Split(req.Tx, []byte("="))
 	if len(parts) == 2 {
-    app.state.Set([]byte(parts[0]), []byte(parts[1]))
+		key, value = parts[0], parts[1]
 	} else {
-    app.state.Set(tx, tx)
+		key, value = req.Tx, req.Tx
 	}
-  return types.OK
+
+	app.state.db.Set(prefixKey(key), value)
+	app.state.Size += 1
+
+	events := []types.Event{
+		{
+			Type: "app",
+			Attributes: []cmn.KVPair{
+				{Key: []byte("creator"), Value: []byte("Cosmoshi Netowoko")},
+				{Key: []byte("key"), Value: key},
+			},
+		},
+	}
+
+	return types.ResponseDeliverTx{Code: code.CodeTypeOK, Events: events}
 }
 ```
 
@@ -223,9 +238,14 @@ job of the [Handshake](#handshake).
 In go:
 
 ```
-func (app *KVStoreApplication) Commit() types.Result {
+func (app *KVStoreApplication) Commit() types.ResponseCommit {
-  hash := app.state.Hash()
+	// Using a memdb - just return the big endian size of the db
-  return types.NewResultOK(hash, "")
+	appHash := make([]byte, 8)
+	binary.PutVarint(appHash, app.state.Size)
+	app.state.AppHash = appHash
+	app.state.Height += 1
+	saveState(app.state)
+	return types.ResponseCommit{Data: appHash}
 }
 ```
 
@@ -256,12 +276,10 @@ In go:
 
 ```
 // Track the block hash and header information
-func (app *PersistentKVStoreApplication) BeginBlock(params types.RequestBeginBlock) {
+func (app *PersistentKVStoreApplication) BeginBlock(req types.RequestBeginBlock) types.ResponseBeginBlock {
-  // update latest block info
-  app.blockHeader = params.Header
-
 	// reset valset changes
-  app.changes = make([]*types.Validator, 0)
+	app.ValUpdates = make([]types.ValidatorUpdate, 0)
+	return types.ResponseBeginBlock{}
 }
 ```
 
@@ -347,37 +365,23 @@ Note: these query formats are subject to change!
 In go:
 
 ```
-    func (app *KVStoreApplication) Query(reqQuery types.RequestQuery) (resQuery types.ResponseQuery) {
+func (app *KVStoreApplication) Query(reqQuery types.RequestQuery) (resQuery types.ResponseQuery) {
 	if reqQuery.Prove {
-        value, proof, exists := app.state.GetWithProof(reqQuery.Data)
+		value := app.state.db.Get(prefixKey(reqQuery.Data))
 		resQuery.Index = -1 // TODO make Proof return index
 		resQuery.Key = reqQuery.Data
 		resQuery.Value = value
-        resQuery.Proof = proof
+		if value != nil {
-        if exists {
 			resQuery.Log = "exists"
 		} else {
 			resQuery.Log = "does not exist"
 		}
 		return
 	} else {
-        index, value, exists := app.state.Get(reqQuery.Data)
+		resQuery.Key = reqQuery.Data
-        resQuery.Index = int64(index)
+		value := app.state.db.Get(prefixKey(reqQuery.Data))
 		resQuery.Value = value
-        if exists {
+		if value != nil {
-          resQuery.Log = "exists"
-        } else {
-          resQuery.Log = "does not exist"
-        }
-        return
-      }
-    }
-    return
-  } else {
-    index, value, exists := app.state.Get(reqQuery.Data)
-    resQuery.Index = int64(index)
-    resQuery.Value = value
-    if exists {
 			resQuery.Log = "exists"
 		} else {
 			resQuery.Log = "does not exist"
@@ -439,7 +443,11 @@ In go:
 
 ```
 func (app *KVStoreApplication) Info(req types.RequestInfo) (resInfo types.ResponseInfo) {
-  return types.ResponseInfo{Data: fmt.Sprintf("{\"size\":%v}", app.state.Size())}
+	return types.ResponseInfo{
+		Data:       fmt.Sprintf("{\"size\":%v}", app.state.Size),
+		Version:    version.ABCIVersion,
+		AppVersion: ProtocolVersion.Uint64(),
+	}
 }
 ```
 
@@ -463,13 +471,14 @@ In go:
 
 ```
 // Save the validators in the merkle tree
-func (app *PersistentKVStoreApplication) InitChain(params types.RequestInitChain) {
+func (app *PersistentKVStoreApplication) InitChain(req types.RequestInitChain) types.ResponseInitChain {
-  for _, v := range params.Validators {
+	for _, v := range req.Validators {
 		r := app.updateValidator(v)
 		if r.IsErr() {
 			app.logger.Error("Error updating validators", "r", r)
 		}
 	}
+	return types.ResponseInitChain{}
 }
 ```
 

docs/app-dev/indexing-transactions.md

@@ -47,7 +47,7 @@ pairs of UTF-8 encoded strings (e.g. "account.owner": "Bob", "balance":
 Example:
 
 ```
-func (app *KVStoreApplication) DeliverTx(tx []byte) types.Result {
+func (app *KVStoreApplication) DeliverTx(req types.RequestDeliverTx) types.Result {
     ...
     tags := []cmn.KVPair{
       {[]byte("account.name"), []byte("igor")},

mempool/clist_mempool_test.go

@@ -99,7 +99,7 @@ func TestReapMaxBytesMaxGas(t *testing.T) {
 	checkTxs(t, mempool, 1, UnknownPeerID)
 	tx0 := mempool.TxsFront().Value.(*mempoolTx)
 	// assert that kv store has gas wanted = 1.
-	require.Equal(t, app.CheckTx(tx0.tx).GasWanted, int64(1), "KVStore had a gas value neq to 1")
+	require.Equal(t, app.CheckTx(abci.RequestCheckTx{Tx: tx0.tx}).GasWanted, int64(1), "KVStore had a gas value neq to 1")
 	require.Equal(t, tx0.gasWanted, int64(1), "transactions gas was set incorrectly")
 	// ensure each tx is 20 bytes long
 	require.Equal(t, len(tx0.tx), 20, "Tx is longer than 20 bytes")

rpc/client/mock/abci.go

@@ -45,29 +45,29 @@ func (a ABCIApp) ABCIQueryWithOptions(path string, data cmn.HexBytes, opts clien
 // TODO: Make it wait for a commit and set res.Height appropriately.
 func (a ABCIApp) BroadcastTxCommit(tx types.Tx) (*ctypes.ResultBroadcastTxCommit, error) {
 	res := ctypes.ResultBroadcastTxCommit{}
-	res.CheckTx = a.App.CheckTx(tx)
+	res.CheckTx = a.App.CheckTx(abci.RequestCheckTx{Tx: tx})
 	if res.CheckTx.IsErr() {
 		return &res, nil
 	}
-	res.DeliverTx = a.App.DeliverTx(tx)
+	res.DeliverTx = a.App.DeliverTx(abci.RequestDeliverTx{Tx: tx})
 	res.Height = -1 // TODO
 	return &res, nil
 }
 
 func (a ABCIApp) BroadcastTxAsync(tx types.Tx) (*ctypes.ResultBroadcastTx, error) {
-	c := a.App.CheckTx(tx)
+	c := a.App.CheckTx(abci.RequestCheckTx{Tx: tx})
 	// and this gets written in a background thread...
 	if !c.IsErr() {
-		go func() { a.App.DeliverTx(tx) }() // nolint: errcheck
+		go func() { a.App.DeliverTx(abci.RequestDeliverTx{Tx: tx}) }() // nolint: errcheck
 	}
 	return &ctypes.ResultBroadcastTx{Code: c.Code, Data: c.Data, Log: c.Log, Hash: tx.Hash()}, nil
 }
 
 func (a ABCIApp) BroadcastTxSync(tx types.Tx) (*ctypes.ResultBroadcastTx, error) {
-	c := a.App.CheckTx(tx)
+	c := a.App.CheckTx(abci.RequestCheckTx{Tx: tx})
 	// and this gets written in a background thread...
 	if !c.IsErr() {
-		go func() { a.App.DeliverTx(tx) }() // nolint: errcheck
+		go func() { a.App.DeliverTx(abci.RequestDeliverTx{Tx: tx}) }() // nolint: errcheck
 	}
 	return &ctypes.ResultBroadcastTx{Code: c.Code, Data: c.Data, Log: c.Log, Hash: tx.Hash()}, nil
 }

state/execution_test.go

@@ -453,11 +453,11 @@ func (app *testApp) EndBlock(req abci.RequestEndBlock) abci.ResponseEndBlock {
 	return abci.ResponseEndBlock{ValidatorUpdates: app.ValidatorUpdates}
 }
 
-func (app *testApp) DeliverTx(tx []byte) abci.ResponseDeliverTx {
+func (app *testApp) DeliverTx(req abci.RequestDeliverTx) abci.ResponseDeliverTx {
 	return abci.ResponseDeliverTx{Events: []abci.Event{}}
 }
 
-func (app *testApp) CheckTx(tx []byte) abci.ResponseCheckTx {
+func (app *testApp) CheckTx(req abci.RequestCheckTx) abci.ResponseCheckTx {
 	return abci.ResponseCheckTx{}
 }