Unify CheckTx and DeliverTx (#286)

Unify CheckTx and DeliverTx

.gitignore

@@ -1,4 +1,5 @@
 *.swp
+*.swo
 vendor
 .vagrant
 merkleeyes.db

app/base.go

@@ -30,14 +30,14 @@ type BaseApp struct {
 	// CheckTx state
 	msCheck CacheMultiStore
 
-	// Cached validator changes from DeliverTx
-	pending []*abci.Validator
-
-	// Parser for the tx.
-	txParser sdk.TxParser
+	// Current block header
+	header *abci.Header
 
 	// Handler for CheckTx and DeliverTx.
 	handler sdk.Handler
+
+	// Cached validator changes from DeliverTx
+	valSetDiff []abci.Validator
 }
 
 var _ abci.Application = &BaseApp{}
@@ -80,20 +80,17 @@ func NewBaseApp(name string, ms MultiStore) (*BaseApp, error) {
 	}
 
 	return &BaseApp{
-		logger:  logger,
-		name:    name,
-		ms:      ms,
-		msCheck: msCheck,
-		pending: nil,
-		header:  header,
+		logger:     logger,
+		name:       name,
+		ms:         ms,
+		msCheck:    msCheck,
+		header:     header,
+		hander:     nil, // set w/ .WithHandler()
+		valSetDiff: nil,
 	}
 }
 
-func (app *BaseApp) SetTxParser(parser TxParser) {
-	app.txParser = parser
-}
-
-func (app *BaseApp) SetHandler(handler sdk.Handler) {
+func (app *BaseApp) WithHandler(handler sdk.Handler) *BaseApp {
 	app.handler = handler
 }
 
@@ -102,60 +99,36 @@ func (app *BaseApp) SetHandler(handler sdk.Handler) {
 // DeliverTx - ABCI - dispatches to the handler
 func (app *BaseApp) DeliverTx(txBytes []byte) abci.ResponseDeliverTx {
 
-	// TODO: use real context on refactor
-	ctx := util.MockContext(
-		app.GetChainID(),
-		app.WorkingHeight(),
-	)
-
-	// Parse the transaction
-	tx, err := app.parseTxFn(ctx, txBytes)
-	if err != nil {
-		err := sdk.TxParseError("").WithCause(err)
-		return sdk.ResponseDeliverTxFromErr(err)
+	ctx := sdk.NewContext(app.header, false, txBytes)
+	// NOTE: Tx is nil until a decorator parses it.
+	result := app.handler(ctx, nil)
+	if result.Code == abci.CodeType_OK {
+		app.ValSetDiff = append(app.ValSetDiff, result.ValSetDiff)
+	} else {
+		// Even though the Code is not OK, there will be some side effects,
+		// like those caused by fee deductions or sequence incrementations.
 	}
-
-	// Make handler deal with it
-	data, err := app.handler.DeliverTx(ctx, app.ms, tx)
-	if err != nil {
-		return sdk.ResponseDeliverTxFromErr(err)
-	}
-
-	app.AddValChange(res.Diff)
-
 	return abci.ResponseDeliverTx{
-		Code: abci.CodeType_OK,
-		Data: data,
-		Log:  "", // TODO add log from ctx.logger
+		Code: result.Code,
+		Data: result.Data,
+		Log:  result.Log,
+		Tags: result.Tags,
 	}
 }
 
 // CheckTx - ABCI - dispatches to the handler
 func (app *BaseApp) CheckTx(txBytes []byte) abci.ResponseCheckTx {
 
-	// TODO: use real context on refactor
-	ctx := util.MockContext(
-		app.GetChainID(),
-		app.WorkingHeight(),
-	)
-
-	// Parse the transaction
-	tx, err := app.parseTxFn(ctx, txBytes)
-	if err != nil {
-		err := sdk.TxParseError("").WithCause(err)
-		return sdk.ResponseCheckTxFromErr(err)
-	}
-
-	// Make handler deal with it
-	data, err := app.handler.CheckTx(ctx, app.ms, tx)
-	if err != nil {
-		return sdk.ResponseCheckTx(err)
-	}
-
+	ctx := sdk.NewContext(app.header, true, txBytes)
+	// NOTE: Tx is nil until a decorator parses it.
+	result := app.handler(ctx, nil)
 	return abci.ResponseCheckTx{
-		Code: abci.CodeType_OK,
-		Data: data,
-		Log:  "", // TODO add log from ctx.logger
+		Code:      result.Code,
+		Data:      result.Data,
+		Log:       result.Log,
+		Gas:       result.Gas,
+		FeeDenom:  result.FeeDenom,
+		FeeAmount: result.FeeAmount,
 	}
 }
 
@@ -172,12 +145,12 @@ func (app *BaseApp) Info(req abci.RequestInfo) abci.ResponseInfo {
 }
 
 // SetOption - ABCI
-func (app *StoreApp) SetOption(key string, value string) string {
+func (app *BaseApp) SetOption(key string, value string) string {
 	return "Not Implemented"
 }
 
 // Query - ABCI
-func (app *StoreApp) Query(reqQuery abci.RequestQuery) (resQuery abci.ResponseQuery) {
+func (app *BaseApp) Query(reqQuery abci.RequestQuery) (resQuery abci.ResponseQuery) {
 	/* TODO
 
 	if len(reqQuery.Data) == 0 {
@@ -231,7 +204,7 @@ func (app *StoreApp) Query(reqQuery abci.RequestQuery) (resQuery abci.ResponseQu
 }
 
 // Commit implements abci.Application
-func (app *StoreApp) Commit() (res abci.Result) {
+func (app *BaseApp) Commit() (res abci.Result) {
 	/*
 		hash, err := app.state.Commit(app.height)
 		if err != nil {
@@ -251,37 +224,23 @@ func (app *StoreApp) Commit() (res abci.Result) {
 }
 
 // InitChain - ABCI
-func (app *StoreApp) InitChain(req abci.RequestInitChain) {}
+func (app *BaseApp) InitChain(req abci.RequestInitChain) {}
 
 // BeginBlock - ABCI
-func (app *StoreApp) BeginBlock(req abci.RequestBeginBlock) {
-	// TODO
+func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) {
+	app.header = req.Header
 }
 
 // EndBlock - ABCI
 // Returns a list of all validator changes made in this block
-func (app *StoreApp) EndBlock(height uint64) (res abci.ResponseEndBlock) {
-	// TODO: cleanup in case a validator exists multiple times in the list
-	res.Diffs = app.pending
-	app.pending = nil
+func (app *BaseApp) EndBlock(height uint64) (res abci.ResponseEndBlock) {
+	// TODO: Compress duplicates
+	res.Diffs = app.valSetDiff
+	app.valSetDiff = nil
 	return
 }
 
-// AddValChange is meant to be called by apps on DeliverTx
-// results, this is added to the cache for the endblock
-// changeset
-func (app *StoreApp) AddValChange(diffs []*abci.Validator) {
-	for _, d := range diffs {
-		idx := pubKeyIndex(d, app.pending)
-		if idx >= 0 {
-			app.pending[idx] = d
-		} else {
-			app.pending = append(app.pending, d)
-		}
-	}
-}
-
-// return index of list with validator of same PubKey, or -1 if no match
+// Return index of list with validator of same PubKey, or -1 if no match
 func pubKeyIndex(val *abci.Validator, list []*abci.Validator) int {
 	for i, v := range list {
 		if bytes.Equal(val.PubKey, v.PubKey) {

exports.go

@@ -1,10 +1,18 @@
 package sdk
 
 import (
+	"github.com/cosmos/cosmos-sdk/store"
 	types "github.com/cosmos/cosmos-sdk/types"
 )
 
-// XXX type aliases for the types/* directory
 type (
-	Handler = types.Handler
+	// Type aliases for the cosmos-sdk/types module.  We keep all of them in
+	// types/* but they are all meant to be imported as
+	// "github.com/cosmos/cosmos-sdk".  So, add all of them.
+	Handler   = types.Handler
+	Context   = types.Context
+	Decorator = types.Decorator
+
+	// Type aliases for other modules.
+	MultiStore = store.MultiStore modules.
 )

types/context.go

@@ -2,46 +2,39 @@ package types
 
 import (
 	"context"
-	tm "github.com/tendermint/tendermint/types"
+	abci "github.com/tendermint/abci/types"
 )
 
-/*
-
-NOTE: Golang's Context is embedded and relied on
-for compatibility w/ tools like monkit.
-(https://github.com/spacemonkeygo/monkit)
-
-Usage:
-
-defer mon.Task()(&ctx.Context)(&err)
-
-*/
-type SDKContext struct {
+type Context struct {
 	context.Context
-	// NOTE: adding fields here will break monkit compatibility
-	// use context.Context instead if possible.
+	// Don't add any other fields here,
+	// it's probably not what you want to do.
 }
 
-func NewSDKContext(header tm.Header) SDKContext {
-	c := SDKContext{
+func NewContext(header tm.Header, isCheckTx bool, txBytes []byte) Context {
+	c := Context{
 		Context: context.Background(),
 	}
 	c = c.setBlockHeader(header)
 	c = c.setBlockHeight(int64(header.Height))
 	c = c.setChainID(header.ChainID)
+	c = c.setIsCheckTx(isCheckTx)
+	c = c.setTxBytes(txBytes)
 	return c
 }
 
-func (c SDKContext) WithValueSDK(key interface{}, value interface{}) SDKContext {
-	return SDKContext{
+// The original context.Context API.
+func (c Context) WithValue(key interface{}, value interface{}) context.Context {
+	return context.WithValue(c.Context, key, value)
+}
+
+// Like WithValue() but retains this API.
+func (c Context) WithValueSDK(key interface{}, value interface{}) Context {
+	return Context{
 		Context: context.WithValue(c.Context, key, value),
 	}
 }
 
-func (c SDKContext) WithValue(key interface{}, value interface{}) Context {
-	return c
-}
-
 //----------------------------------------
 // Our extensions
 
@@ -51,31 +44,51 @@ const (
 	contextKeyBlockHeader contextKey = iota
 	contextKeyBlockHeight
 	contextKeyChainID
+	contextKeyIsCheckTx
+	contextKeyTxBytes
 )
 
-func (c SDKContext) BlockHeader() tm.Header {
+func (c Context) BlockHeader() tm.Header {
 	return c.Value(contextKeyBlockHeader).(tm.Header)
 }
 
-func (c SDKContext) BlockHeight() int64 {
+func (c Context) BlockHeight() int64 {
 	return c.Value(contextKeyBlockHeight).(int64)
 }
 
-func (c SDKContext) ChainID() string {
+func (c Context) ChainID() string {
 	return c.Value(contextKeyChainID).(string)
 }
 
+func (c Context) IsCheckTx() bool {
+	return c.Value(contextKeyIsCheckTx).(bool)
+}
+
+func (c Context) TxBytes() []byte {
+	return c.Value(contextKeyTxBytes).([]byte)
+}
+
 // Unexposed to prevent overriding.
-func (c SDKContext) setBlockHeader(header tm.Header) SDKContext {
+func (c Context) setBlockHeader(header tm.Header) Context {
 	return c.WithValueSDK(contextKeyBlockHeader, header)
 }
 
 // Unexposed to prevent overriding.
-func (c SDKContext) setBlockHeight(height int64) SDKContext {
+func (c Context) setBlockHeight(height int64) Context {
 	return c.WithValueSDK(contextKeyBlockHeight, header)
 }
 
 // Unexposed to prevent overriding.
-func (c SDKContext) setChainID(chainID string) SDKContext {
+func (c Context) setChainID(chainID string) Context {
 	return c.WithValueSDK(contextKeyChainID, header)
 }
+
+// Unexposed to prevent overriding.
+func (c Context) setIsCheckTx(isCheckTx bool) Context {
+	return c.WithValueSDK(contextKeyIsCheckTx, isCheckTx)
+}
+
+// Unexposed to prevent overriding.
+func (c Context) setTxBytes(txBytes []byte) Context {
+	return c.WithValueSDK(contextKeyTxBytes, txBytes)
+}

types/decorators.go

@@ -1,22 +1,12 @@
 package types
 
 // A Decorator executes before/during/after a handler to enhance functionality.
-type Decorator interface {
+type Decorator func(ctx Context, ms MultiStore, tx Tx, next Handler) Result
 
-	// Decorate Handler.CheckTx
-	CheckTx(ctx Context, ms MultiStore, tx Tx,
-		next CheckTxFunc) CheckResult
-
-	// Decorate Handler.DeliverTx
-	DeliverTx(ctx Context, ms MultiStore, tx Tx,
-		next DeliverTxFunc) DeliverResult
-}
-
-// A Decorator tied to its base handler "next" is itself a handler.
+// Return a decorated handler
 func Decorate(dec Decorator, next Handler) Handler {
-	return &decHandler{
-		decorator: dec,
-		next:      next,
+	return func(ctx Context, ms MultiStore, tx Tx) Result {
+		return dec(ctx, ms, tx, next)
 	}
 }
 
@@ -62,25 +52,5 @@ func build(stack []Decorator, end Handler) Handler {
 	if len(stack) == 0 {
 		return end
 	}
-	return decHandler{
-		decorator: stack[0],
-		next:      build(stack[1:], end),
-	}
-}
-
-//----------------------------------------
-
-type decHandler struct {
-	decorator Decorator
-	next      Handler
-}
-
-var _ Handler = &decHandler{}
-
-func (dh *decHandler) CheckTx(ctx Context, ms MultiStore, tx Tx) CheckResult {
-	return dh.decorator.CheckTx(ctx, ms, tx, dh.next)
-}
-
-func (dh *decHandler) DeliverTx(ctx Context, ms MultiStore, tx Tx) DeliverResult {
-	return dh.decorator.DeliverTx(ctx, ms, tx, dh.next)
+	return Decorate(stack[0], build(stack[1:], end))
 }

types/handler.go

@@ -1,24 +1,9 @@
 package types
 
 import (
-	abci "github.com/tendermint/abci/types"
-	"github.com/tendermint/tmlibs/log"
+	"github.com/cosmos/cosmos-sdk"
 )
 
-// Handler is something that processes a transaction.
-type Handler interface {
-
-	// Checker verifies there are valid fees and estimates work.
-	CheckTx(ctx Context, ms MultiStore, tx Tx) CheckResult
-
-	// Deliverer performs the tx once it makes it in the block.
-	DeliverTx(ctx Context, ms MultiStore, tx Tx) DeliverResult
-}
-
-// Checker verifies there are valid fees and estimates work.
-// NOTE: Keep in sync with Handler.CheckTx
-type CheckTxFunc func(ctx Context, ms MultiStore, tx Tx) CheckResult
-
-// Deliverer performs the tx once it makes it in the block.
-// NOTE: Keep in sync with Handler.DeliverTx
-type DeliverTxFunc func(ctx Context, ms MultiStore, tx Tx) DeliverResult
+// Handler handles both ABCI DeliverTx and CheckTx requests.
+// Iff ABCI.CheckTx, ctx.IsCheckTx() returns true.
+type Handler func(ctx Context, ms MultiStore, tx Tx)

types/result.go

@@ -0,0 +1,39 @@
+package types
+
+import (
+	abci "github.com/tendermint/abci/types"
+)
+
+type KVPair struct {
+	Key   []byte
+	Value []byte
+}
+
+// Result is the union of ResponseDeliverTx and ResponseCheckTx.
+type Result struct {
+
+	// Code is the response code, is stored back on the chain.
+	Code uint32
+
+	// Data is any data returned from the app.
+	Data []byte
+
+	// Log is just debug information. NOTE: nondeterministic.
+	Log string
+
+	// GasAllocated is the maximum units of work we allow this tx to perform.
+	GasAllocated int64
+
+	// GasUsed is the amount of gas actually consumed. NOTE: not used.
+	GasUsed int64
+
+	// Tx fee amount and denom.
+	FeeAmount int64
+	FeeDenom  string
+
+	// Changes to the validator set.
+	ValSetDiff []abci.Validator
+
+	// Tags are used for transaction indexing and pubsub.
+	Tags []KVPair
+}

types/results.go

@@ -1,29 +0,0 @@
-package types
-
-import (
-	abci "github.com/tendermint/abci/types"
-)
-
-// CheckResult captures any non-error ABCI  result
-// to make sure people use error for error cases.
-type CheckResult struct {
-	abci.Result
-
-	// GasAllocated is the maximum units of work we allow this tx to perform
-	GasAllocated uint64
-
-	// GasPayment is the total fees for this tx (or other source of payment)
-	GasPayment uint64
-}
-
-// DeliverResult captures any non-error abci result
-// to make sure people use error for error cases
-type DeliverResult struct {
-	abci.Result
-
-	// TODO comment
-	Diff []*abci.Validator
-
-	// TODO comment
-	GasUsed uint64
-}

types/tx_msg.go

@@ -34,5 +34,3 @@ type Tx interface {
 	// .Empty().
 	Signatures() []Signature
 }
-
-type TxParser func(txBytes []byte) (Tx, error)