From 2f4f875dd4449b81a369a177ee3c3dba387f191d Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Fri, 4 Aug 2017 13:50:55 +0200 Subject: [PATCH] Cleanup from PR comments --- app/app.go | 8 +++---- app/app_val_test.go | 15 +++++++------- handler.go | 45 +++++++++++++++++++++------------------- modules/fee/handler.go | 6 +++--- stack/middleware_test.go | 2 +- 5 files changed, 39 insertions(+), 37 deletions(-) diff --git a/app/app.go b/app/app.go index 117b155e88..f31425f366 100644 --- a/app/app.go +++ b/app/app.go @@ -112,7 +112,7 @@ func (app *Basecoin) DeliverTx(txBytes []byte) abci.Result { return errors.Result(err) } app.addValChange(res.Diff) - return res.ToABCI() + return basecoin.ToABCI(res) } // CheckTx - ABCI @@ -132,7 +132,7 @@ func (app *Basecoin) CheckTx(txBytes []byte) abci.Result { if err != nil { return errors.Result(err) } - return res.ToABCI() + return basecoin.ToABCI(res) } // Query - ABCI @@ -182,7 +182,7 @@ func (app *Basecoin) EndBlock(height uint64) (res abci.ResponseEndBlock) { func (app *Basecoin) addValChange(diffs []*abci.Validator) { for _, d := range diffs { - idx := findVal(d, app.pending) + idx := pubKeyIndex(d, app.pending) if idx >= 0 { app.pending[idx] = d } else { @@ -192,7 +192,7 @@ func (app *Basecoin) addValChange(diffs []*abci.Validator) { } // return index of list with validator of same PubKey, or -1 if no match -func findVal(val *abci.Validator, list []*abci.Validator) int { +func pubKeyIndex(val *abci.Validator, list []*abci.Validator) int { for i, v := range list { if bytes.Equal(val.PubKey, v.PubKey) { return i diff --git a/app/app_val_test.go b/app/app_val_test.go index d1697168dd..aabb834434 100644 --- a/app/app_val_test.go +++ b/app/app_val_test.go @@ -15,22 +15,21 @@ import ( //----------------------------------- // Test cases start here -func power() uint64 { - // % can return negative numbers, so this ensures result is positive +func randPower() uint64 { return uint64(cmn.RandInt()%50 + 60) } func makeVal() *abci.Validator { return &abci.Validator{ PubKey: cmn.RandBytes(10), - Power: power(), + Power: randPower(), } } -// newPower returns a copy of the validator with a different power -func newPower(val *abci.Validator) *abci.Validator { +// withNewPower returns a copy of the validator with a different power +func withNewPower(val *abci.Validator) *abci.Validator { res := *val - res.Power = power() + res.Power = randPower() if res.Power == val.Power { panic("no no") } @@ -48,8 +47,8 @@ func TestEndBlock(t *testing.T) { val1 := makeVal() val2 := makeVal() val3 := makeVal() - val1a := newPower(val1) - val2a := newPower(val2) + val1a := withNewPower(val1) + val2a := withNewPower(val2) cases := [...]struct { changes [][]*abci.Validator diff --git a/handler.go b/handler.go index 32689b3a42..6c45b4c69c 100644 --- a/handler.go +++ b/handler.go @@ -35,7 +35,7 @@ type Checker interface { CheckTx(ctx Context, store state.SimpleDB, tx Tx) (CheckResult, error) } -// CheckerFunc (like http.HandlerFunc) is a shortcut for making wrapers +// CheckerFunc (like http.HandlerFunc) is a shortcut for making wrappers type CheckerFunc func(Context, state.SimpleDB, Tx) (CheckResult, error) func (c CheckerFunc) CheckTx(ctx Context, store state.SimpleDB, tx Tx) (CheckResult, error) { @@ -47,7 +47,7 @@ type Deliver interface { DeliverTx(ctx Context, store state.SimpleDB, tx Tx) (DeliverResult, error) } -// DeliverFunc (like http.HandlerFunc) is a shortcut for making wrapers +// DeliverFunc (like http.HandlerFunc) is a shortcut for making wrappers type DeliverFunc func(Context, state.SimpleDB, Tx) (DeliverResult, error) func (c DeliverFunc) DeliverTx(ctx Context, store state.SimpleDB, tx Tx) (DeliverResult, error) { @@ -59,7 +59,7 @@ type InitStater interface { InitState(l log.Logger, store state.SimpleDB, module, key, value string) (string, error) } -// InitStateFunc (like http.HandlerFunc) is a shortcut for making wrapers +// InitStateFunc (like http.HandlerFunc) is a shortcut for making wrappers type InitStateFunc func(log.Logger, state.SimpleDB, string, string, string) (string, error) func (c InitStateFunc) InitState(l log.Logger, store state.SimpleDB, module, key, value string) (string, error) { @@ -71,7 +71,7 @@ type InitValidater interface { InitValidate(log log.Logger, store state.SimpleDB, vals []*abci.Validator) } -// InitValidateFunc (like http.HandlerFunc) is a shortcut for making wrapers +// InitValidateFunc (like http.HandlerFunc) is a shortcut for making wrappers type InitValidateFunc func(log.Logger, state.SimpleDB, []*abci.Validator) func (c InitValidateFunc) InitValidate(l log.Logger, store state.SimpleDB, vals []*abci.Validator) { @@ -80,8 +80,17 @@ func (c InitValidateFunc) InitValidate(l log.Logger, store state.SimpleDB, vals //---------- results and some wrappers -------- -type Dataer interface { +// Result is a common interface of CheckResult and GetResult +type Result interface { GetData() data.Bytes + GetLog() string +} + +func ToABCI(r Result) abci.Result { + return abci.Result{ + Data: r.GetData(), + Log: r.GetLog(), + } } // CheckResult captures any non-error abci result @@ -104,19 +113,16 @@ func NewCheck(gasAllocated uint, log string) CheckResult { } } -var _ Dataer = CheckResult{} - -func (r CheckResult) ToABCI() abci.Result { - return abci.Result{ - Data: r.Data, - Log: r.Log, - } -} +var _ Result = CheckResult{} func (r CheckResult) GetData() data.Bytes { return r.Data } +func (r CheckResult) GetLog() string { + return r.Log +} + // DeliverResult captures any non-error abci result // to make sure people use error for error cases type DeliverResult struct { @@ -126,19 +132,16 @@ type DeliverResult struct { GasUsed uint } -var _ Dataer = DeliverResult{} - -func (r DeliverResult) ToABCI() abci.Result { - return abci.Result{ - Data: r.Data, - Log: r.Log, - } -} +var _ Result = DeliverResult{} func (r DeliverResult) GetData() data.Bytes { return r.Data } +func (r DeliverResult) GetLog() string { + return r.Log +} + // placeholders // holders type NopCheck struct{} diff --git a/modules/fee/handler.go b/modules/fee/handler.go index d1eca64bb2..53de71d5b2 100644 --- a/modules/fee/handler.go +++ b/modules/fee/handler.go @@ -48,10 +48,10 @@ 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 { + if IsSkipFeesErr(err) { + return next.CheckTx(ctx, store, tx) + } return res, err } diff --git a/stack/middleware_test.go b/stack/middleware_test.go index 4db113f258..b7fa9231a4 100644 --- a/stack/middleware_test.go +++ b/stack/middleware_test.go @@ -80,7 +80,7 @@ func TestPermissionSandbox(t *testing.T) { } } -func checkPerm(t *testing.T, idx int, data []byte, check func(error) bool, res basecoin.Dataer, err error) { +func checkPerm(t *testing.T, idx int, data []byte, check func(error) bool, res basecoin.Result, err error) { assert := assert.New(t) if len(data) > 0 {