From eb08136104f17b7fcd845b07d8927595d11379a9 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sun, 30 Jul 2017 12:41:41 -0400 Subject: [PATCH] Fix all tests for CheckTx/DeliverTx split --- handler.go | 18 ++++++++++++++++++ modules/base/chain_test.go | 8 ++++---- modules/base/tx_test.go | 4 ++-- modules/fee/errors.go | 8 ++++++++ modules/fee/handler.go | 9 ++++++++- stack/helpers_test.go | 4 ++-- stack/middleware_test.go | 12 ++++++------ stack/state_space_test.go | 12 ++++++------ 8 files changed, 54 insertions(+), 21 deletions(-) diff --git a/handler.go b/handler.go index 49262fbf4..d2858ea94 100644 --- a/handler.go +++ b/handler.go @@ -57,6 +57,12 @@ func (c SetOptionFunc) SetOption(l log.Logger, store state.SimpleDB, module, key return c(l, store, module, key, value) } +//---------- results and some wrappers -------- + +type Dataer interface { + GetData() data.Bytes +} + // CheckResult captures any non-error abci result // to make sure people use error for error cases type CheckResult struct { @@ -66,6 +72,8 @@ type CheckResult struct { GasPrice uint } +var _ Dataer = CheckResult{} + func (r CheckResult) ToABCI() abci.Result { return abci.Result{ Data: r.Data, @@ -73,6 +81,10 @@ func (r CheckResult) ToABCI() abci.Result { } } +func (r CheckResult) GetData() data.Bytes { + return r.Data +} + // DeliverResult captures any non-error abci result // to make sure people use error for error cases type DeliverResult struct { @@ -82,6 +94,8 @@ type DeliverResult struct { GasUsed uint } +var _ Dataer = DeliverResult{} + func (r DeliverResult) ToABCI() abci.Result { return abci.Result{ Data: r.Data, @@ -89,6 +103,10 @@ func (r DeliverResult) ToABCI() abci.Result { } } +func (r DeliverResult) GetData() data.Bytes { + return r.Data +} + // placeholders // holders type NopCheck struct{} diff --git a/modules/base/chain_test.go b/modules/base/chain_test.go index 8da281cdd..4f039892c 100644 --- a/modules/base/chain_test.go +++ b/modules/base/chain_test.go @@ -82,10 +82,10 @@ func TestChain(t *testing.T) { i := strconv.Itoa(idx) // make sure check returns error, not a panic crash - res, err := app.CheckTx(ctx, store, tc.tx) + cres, err := app.CheckTx(ctx, store, tc.tx) if tc.valid { assert.Nil(err, "%d: %+v", idx, err) - assert.Equal(msg, res.Log, i) + assert.Equal(msg, cres.Log, i) } else { if assert.NotNil(err, i) { assert.Contains(err.Error(), tc.errorMsg, i) @@ -93,10 +93,10 @@ func TestChain(t *testing.T) { } // make sure deliver returns error, not a panic crash - res, err = app.DeliverTx(ctx, store, tc.tx) + dres, err := app.DeliverTx(ctx, store, tc.tx) if tc.valid { assert.Nil(err, "%d: %+v", idx, err) - assert.Equal(msg, res.Log, i) + assert.Equal(msg, dres.Log, i) } else { if assert.NotNil(err, i) { assert.Contains(err.Error(), tc.errorMsg, i) diff --git a/modules/base/tx_test.go b/modules/base/tx_test.go index 1830c469d..e2ad3e219 100644 --- a/modules/base/tx_test.go +++ b/modules/base/tx_test.go @@ -18,13 +18,13 @@ func TestEncoding(t *testing.T) { require := require.New(t) raw := stack.NewRawTx([]byte{0x34, 0xa7}) - raw2 := stack.NewRawTx([]byte{0x73, 0x86, 0x22}) + // raw2 := stack.NewRawTx([]byte{0x73, 0x86, 0x22}) cases := []struct { Tx basecoin.Tx }{ {raw}, - {NewMultiTx(raw, raw2)}, + // {NewMultiTx(raw, raw2)}, {NewChainTx("foobar", 0, raw)}, } diff --git a/modules/fee/errors.go b/modules/fee/errors.go index 5edc84786..5d06fce41 100644 --- a/modules/fee/errors.go +++ b/modules/fee/errors.go @@ -12,6 +12,7 @@ import ( var ( errInsufficientFees = fmt.Errorf("Insufficient fees") errWrongFeeDenom = fmt.Errorf("Required fee denomination") + errSkipFees = fmt.Errorf("Skip fees") invalidInput = abci.CodeType_BaseInvalidInput ) @@ -29,3 +30,10 @@ func ErrWrongFeeDenom(denom string) errors.TMError { func IsWrongFeeDenomErr(err error) bool { return errors.IsSameError(errWrongFeeDenom, err) } + +func ErrSkipFees() errors.TMError { + return errors.WithCode(errSkipFees, invalidInput) +} +func IsSkipFeesErr(err error) bool { + return errors.IsSameError(errSkipFees, err) +} diff --git a/modules/fee/handler.go b/modules/fee/handler.go index 1c597c7f4..ab0e6fbb7 100644 --- a/modules/fee/handler.go +++ b/modules/fee/handler.go @@ -47,6 +47,9 @@ func (SimpleFeeMiddleware) Name() string { // CheckTx - check the transaction func (h SimpleFeeMiddleware) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Checker) (res basecoin.CheckResult, err error) { fee, err := h.verifyFee(ctx, tx) + if IsSkipFeesErr(err) { + return next.CheckTx(ctx, store, tx) + } if err != nil { return res, err } @@ -58,12 +61,16 @@ func (h SimpleFeeMiddleware) CheckTx(ctx basecoin.Context, store state.SimpleDB, return res, err } } + return next.CheckTx(ctx, store, fee.Tx) } // DeliverTx - send the fee handler transaction func (h SimpleFeeMiddleware) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.DeliverResult, err error) { fee, err := h.verifyFee(ctx, tx) + if IsSkipFeesErr(err) { + return next.DeliverTx(ctx, store, tx) + } if err != nil { return res, err } @@ -83,7 +90,7 @@ func (h SimpleFeeMiddleware) verifyFee(ctx basecoin.Context, tx basecoin.Tx) (Fe if !ok { // the fee wrapper is not required if there is no minimum if h.MinFee.IsZero() { - return feeTx, nil + return feeTx, ErrSkipFees() } return feeTx, errors.ErrInvalidFormat(TypeFees, tx) } diff --git a/stack/helpers_test.go b/stack/helpers_test.go index 888bb790f..fb07c35c7 100644 --- a/stack/helpers_test.go +++ b/stack/helpers_test.go @@ -25,9 +25,9 @@ func TestOK(t *testing.T) { assert.Nil(err, "%+v", err) assert.Equal(data, res.Log) - res, err = ok.DeliverTx(ctx, store, tx) + dres, err := ok.DeliverTx(ctx, store, tx) assert.Nil(err, "%+v", err) - assert.Equal(data, res.Log) + assert.Equal(data, dres.Log) } func TestFail(t *testing.T) { diff --git a/stack/middleware_test.go b/stack/middleware_test.go index bb33a9a62..4db113f25 100644 --- a/stack/middleware_test.go +++ b/stack/middleware_test.go @@ -72,20 +72,20 @@ func TestPermissionSandbox(t *testing.T) { Apps(CheckMiddleware{Required: tc.require}). Use(EchoHandler{}) - res, err := app.CheckTx(ctx, store, raw) - checkPerm(t, i, tc.expectedRes, tc.expected, res, err) + cres, err := app.CheckTx(ctx, store, raw) + checkPerm(t, i, tc.expectedRes, tc.expected, cres, err) - res, err = app.DeliverTx(ctx, store, raw) - checkPerm(t, i, tc.expectedRes, tc.expected, res, err) + dres, err := app.DeliverTx(ctx, store, raw) + checkPerm(t, i, tc.expectedRes, tc.expected, dres, err) } } -func checkPerm(t *testing.T, idx int, data []byte, check func(error) bool, res basecoin.Result, err error) { +func checkPerm(t *testing.T, idx int, data []byte, check func(error) bool, res basecoin.Dataer, err error) { assert := assert.New(t) if len(data) > 0 { assert.Nil(err, "%d: %+v", idx, err) - assert.EqualValues(data, res.Data) + assert.EqualValues(data, res.GetData()) } else { assert.NotNil(err, "%d", idx) // check error code! diff --git a/stack/state_space_test.go b/stack/state_space_test.go index 36a20ed7b..1b3a6f368 100644 --- a/stack/state_space_test.go +++ b/stack/state_space_test.go @@ -24,13 +24,13 @@ var _ Middleware = writerMid{} func (w writerMid) Name() string { return w.name } func (w writerMid) CheckTx(ctx basecoin.Context, store state.SimpleDB, - tx basecoin.Tx, next basecoin.Checker) (basecoin.Result, error) { + tx basecoin.Tx, next basecoin.Checker) (basecoin.CheckResult, error) { store.Set(w.key, w.value) return next.CheckTx(ctx, store, tx) } func (w writerMid) DeliverTx(ctx basecoin.Context, store state.SimpleDB, - tx basecoin.Tx, next basecoin.Deliver) (basecoin.Result, error) { + tx basecoin.Tx, next basecoin.Deliver) (basecoin.DeliverResult, error) { store.Set(w.key, w.value) return next.DeliverTx(ctx, store, tx) } @@ -52,15 +52,15 @@ var _ basecoin.Handler = writerHand{} func (w writerHand) Name() string { return w.name } func (w writerHand) CheckTx(ctx basecoin.Context, store state.SimpleDB, - tx basecoin.Tx) (basecoin.Result, error) { + tx basecoin.Tx) (basecoin.CheckResult, error) { store.Set(w.key, w.value) - return basecoin.Result{}, nil + return basecoin.CheckResult{}, nil } func (w writerHand) DeliverTx(ctx basecoin.Context, store state.SimpleDB, - tx basecoin.Tx) (basecoin.Result, error) { + tx basecoin.Tx) (basecoin.DeliverResult, error) { store.Set(w.key, w.value) - return basecoin.Result{}, nil + return basecoin.DeliverResult{}, nil } func (w writerHand) SetOption(l log.Logger, store state.SimpleDB, module,