From cb4277f4b71e9bc7d35c53e9eedc9fc4abdbac3e Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sat, 29 Jul 2017 20:22:45 -0400 Subject: [PATCH 01/13] Change Handler interface --- handler.go | 44 +++++++++++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/handler.go b/handler.go index 7b028716a..49262fbf4 100644 --- a/handler.go +++ b/handler.go @@ -25,24 +25,24 @@ type Named interface { } type Checker interface { - CheckTx(ctx Context, store state.SimpleDB, tx Tx) (Result, error) + CheckTx(ctx Context, store state.SimpleDB, tx Tx) (CheckResult, error) } // CheckerFunc (like http.HandlerFunc) is a shortcut for making wrapers -type CheckerFunc func(Context, state.SimpleDB, Tx) (Result, error) +type CheckerFunc func(Context, state.SimpleDB, Tx) (CheckResult, error) -func (c CheckerFunc) CheckTx(ctx Context, store state.SimpleDB, tx Tx) (Result, error) { +func (c CheckerFunc) CheckTx(ctx Context, store state.SimpleDB, tx Tx) (CheckResult, error) { return c(ctx, store, tx) } type Deliver interface { - DeliverTx(ctx Context, store state.SimpleDB, tx Tx) (Result, error) + DeliverTx(ctx Context, store state.SimpleDB, tx Tx) (DeliverResult, error) } // DeliverFunc (like http.HandlerFunc) is a shortcut for making wrapers -type DeliverFunc func(Context, state.SimpleDB, Tx) (Result, error) +type DeliverFunc func(Context, state.SimpleDB, Tx) (DeliverResult, error) -func (c DeliverFunc) DeliverTx(ctx Context, store state.SimpleDB, tx Tx) (Result, error) { +func (c DeliverFunc) DeliverTx(ctx Context, store state.SimpleDB, tx Tx) (DeliverResult, error) { return c(ctx, store, tx) } @@ -57,14 +57,32 @@ func (c SetOptionFunc) SetOption(l log.Logger, store state.SimpleDB, module, key return c(l, store, module, key, value) } -// Result captures any non-error abci result +// CheckResult captures any non-error abci result // to make sure people use error for error cases -type Result struct { - Data data.Bytes - Log string +type CheckResult struct { + Data data.Bytes + Log string + GasAllocated uint + GasPrice uint } -func (r Result) ToABCI() abci.Result { +func (r CheckResult) ToABCI() abci.Result { + return abci.Result{ + Data: r.Data, + Log: r.Log, + } +} + +// DeliverResult captures any non-error abci result +// to make sure people use error for error cases +type DeliverResult struct { + Data data.Bytes + Log string + Diff []*abci.Validator + GasUsed uint +} + +func (r DeliverResult) ToABCI() abci.Result { return abci.Result{ Data: r.Data, Log: r.Log, @@ -75,11 +93,11 @@ func (r Result) ToABCI() abci.Result { // holders type NopCheck struct{} -func (_ NopCheck) CheckTx(Context, state.SimpleDB, Tx) (r Result, e error) { return } +func (_ NopCheck) CheckTx(Context, state.SimpleDB, Tx) (r CheckResult, e error) { return } type NopDeliver struct{} -func (_ NopDeliver) DeliverTx(Context, state.SimpleDB, Tx) (r Result, e error) { return } +func (_ NopDeliver) DeliverTx(Context, state.SimpleDB, Tx) (r DeliverResult, e error) { return } type NopOption struct{} From cbfd2cd6114d57035eda45f41504288f2b746704 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sun, 30 Jul 2017 11:45:08 -0400 Subject: [PATCH 02/13] Make it all compile with new Deliver/CheckTx --- docs/guide/counter/plugins/counter/counter.go | 4 +- modules/auth/signature.go | 4 +- modules/base/chain.go | 4 +- modules/base/logger.go | 4 +- modules/base/multiplexer.go | 122 +++++++++--------- modules/base/tx.go | 4 +- modules/coin/handler.go | 38 +++--- modules/fee/handler.go | 52 +++++--- modules/ibc/handler.go | 21 +-- modules/ibc/middleware.go | 4 +- modules/ibc/test_helpers.go | 2 +- modules/nonce/replaycheck.go | 4 +- modules/roles/handler.go | 4 +- modules/roles/middleware.go | 4 +- stack/checkpoint.go | 4 +- stack/context.go | 4 +- stack/dispatcher.go | 4 +- stack/helpers.go | 41 +++--- stack/helperware.go | 8 +- stack/interface.go | 20 +-- stack/middleware.go | 4 +- stack/recovery.go | 4 +- 22 files changed, 193 insertions(+), 167 deletions(-) diff --git a/docs/guide/counter/plugins/counter/counter.go b/docs/guide/counter/plugins/counter/counter.go index e9c2b503d..6a17653d3 100644 --- a/docs/guide/counter/plugins/counter/counter.go +++ b/docs/guide/counter/plugins/counter/counter.go @@ -128,13 +128,13 @@ func (Handler) Name() string { func (Handler) AssertDispatcher() {} // CheckTx checks if the tx is properly structured -func (h Handler) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, _ basecoin.Checker) (res basecoin.Result, err error) { +func (h Handler) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, _ basecoin.Checker) (res basecoin.CheckResult, err error) { _, err = checkTx(ctx, tx) return } // DeliverTx executes the tx if valid -func (h Handler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, dispatch basecoin.Deliver) (res basecoin.Result, err error) { +func (h Handler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, dispatch basecoin.Deliver) (res basecoin.DeliverResult, err error) { ctr, err := checkTx(ctx, tx) if err != nil { return res, err diff --git a/modules/auth/signature.go b/modules/auth/signature.go index c80dc6998..a10ebd5b9 100644 --- a/modules/auth/signature.go +++ b/modules/auth/signature.go @@ -39,7 +39,7 @@ type Signable interface { } // CheckTx verifies the signatures are correct - fulfills Middlware interface -func (Signatures) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Checker) (res basecoin.Result, err error) { +func (Signatures) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Checker) (res basecoin.CheckResult, err error) { sigs, tnext, err := getSigners(tx) if err != nil { return res, err @@ -49,7 +49,7 @@ func (Signatures) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoi } // DeliverTx verifies the signatures are correct - fulfills Middlware interface -func (Signatures) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.Result, err error) { +func (Signatures) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.DeliverResult, err error) { sigs, tnext, err := getSigners(tx) if err != nil { return res, err diff --git a/modules/base/chain.go b/modules/base/chain.go index 1068fba57..98391e14b 100644 --- a/modules/base/chain.go +++ b/modules/base/chain.go @@ -24,7 +24,7 @@ func (Chain) Name() string { var _ stack.Middleware = Chain{} // CheckTx makes sure we are on the proper chain - fulfills Middlware interface -func (c Chain) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Checker) (res basecoin.Result, err error) { +func (c Chain) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Checker) (res basecoin.CheckResult, err error) { stx, err := c.checkChainTx(ctx.ChainID(), ctx.BlockHeight(), tx) if err != nil { return res, err @@ -33,7 +33,7 @@ func (c Chain) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.T } // DeliverTx makes sure we are on the proper chain - fulfills Middlware interface -func (c Chain) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.Result, err error) { +func (c Chain) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.DeliverResult, err error) { stx, err := c.checkChainTx(ctx.ChainID(), ctx.BlockHeight(), tx) if err != nil { return res, err diff --git a/modules/base/logger.go b/modules/base/logger.go index 2cd888386..126e3d056 100644 --- a/modules/base/logger.go +++ b/modules/base/logger.go @@ -26,7 +26,7 @@ func (Logger) Name() string { var _ stack.Middleware = Logger{} // CheckTx logs time and result - fulfills Middlware interface -func (Logger) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Checker) (res basecoin.Result, err error) { +func (Logger) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Checker) (res basecoin.CheckResult, err error) { start := time.Now() res, err = next.CheckTx(ctx, store, tx) delta := time.Now().Sub(start) @@ -41,7 +41,7 @@ func (Logger) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx } // DeliverTx logs time and result - fulfills Middlware interface -func (Logger) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.Result, err error) { +func (Logger) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.DeliverResult, err error) { start := time.Now() res, err = next.DeliverTx(ctx, store, tx) delta := time.Now().Sub(start) diff --git a/modules/base/multiplexer.go b/modules/base/multiplexer.go index 92f3e9c18..4f2c4713a 100644 --- a/modules/base/multiplexer.go +++ b/modules/base/multiplexer.go @@ -1,73 +1,73 @@ package base -import ( - "strings" +// import ( +// "strings" - wire "github.com/tendermint/go-wire" - "github.com/tendermint/go-wire/data" +// wire "github.com/tendermint/go-wire" +// "github.com/tendermint/go-wire/data" - "github.com/tendermint/basecoin" - "github.com/tendermint/basecoin/stack" - "github.com/tendermint/basecoin/state" -) +// "github.com/tendermint/basecoin" +// "github.com/tendermint/basecoin/stack" +// "github.com/tendermint/basecoin/state" +// ) -//nolint -const ( - NameMultiplexer = "mplx" -) +// //nolint +// const ( +// NameMultiplexer = "mplx" +// ) -// Multiplexer grabs a MultiTx and sends them sequentially down the line -type Multiplexer struct { - stack.PassOption -} +// // Multiplexer grabs a MultiTx and sends them sequentially down the line +// type Multiplexer struct { +// stack.PassOption +// } -// Name of the module - fulfills Middleware interface -func (Multiplexer) Name() string { - return NameMultiplexer -} +// // Name of the module - fulfills Middleware interface +// func (Multiplexer) Name() string { +// return NameMultiplexer +// } -var _ stack.Middleware = Multiplexer{} +// var _ stack.Middleware = Multiplexer{} -// CheckTx splits the input tx and checks them all - fulfills Middlware interface -func (Multiplexer) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Checker) (res basecoin.Result, err error) { - if mtx, ok := tx.Unwrap().(*MultiTx); ok { - return runAll(ctx, store, mtx.Txs, next.CheckTx) - } - return next.CheckTx(ctx, store, tx) -} +// // CheckTx splits the input tx and checks them all - fulfills Middlware interface +// func (Multiplexer) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Checker) (res basecoin.CheckResult, err error) { +// if mtx, ok := tx.Unwrap().(*MultiTx); ok { +// return runAll(ctx, store, mtx.Txs, next.CheckTx) +// } +// return next.CheckTx(ctx, store, tx) +// } -// DeliverTx splits the input tx and checks them all - fulfills Middlware interface -func (Multiplexer) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.Result, err error) { - if mtx, ok := tx.Unwrap().(*MultiTx); ok { - return runAll(ctx, store, mtx.Txs, next.DeliverTx) - } - return next.DeliverTx(ctx, store, tx) -} +// // DeliverTx splits the input tx and checks them all - fulfills Middlware interface +// func (Multiplexer) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.DeliverResult, err error) { +// if mtx, ok := tx.Unwrap().(*MultiTx); ok { +// return runAll(ctx, store, mtx.Txs, next.DeliverTx) +// } +// return next.DeliverTx(ctx, store, tx) +// } -func runAll(ctx basecoin.Context, store state.SimpleDB, txs []basecoin.Tx, next basecoin.CheckerFunc) (res basecoin.Result, err error) { - // store all results, unless anything errors - rs := make([]basecoin.Result, len(txs)) - for i, stx := range txs { - rs[i], err = next(ctx, store, stx) - if err != nil { - return - } - } - // now combine the results into one... - return combine(rs), nil -} +// func runAll(ctx basecoin.Context, store state.SimpleDB, txs []basecoin.Tx, next basecoin.CheckerFunc) (res basecoin.Result, err error) { +// // store all results, unless anything errors +// rs := make([]basecoin.Result, len(txs)) +// for i, stx := range txs { +// rs[i], err = next(ctx, store, stx) +// if err != nil { +// return +// } +// } +// // now combine the results into one... +// return combine(rs), nil +// } -// combines all data bytes as a go-wire array. -// joins all log messages with \n -func combine(all []basecoin.Result) basecoin.Result { - datas := make([]data.Bytes, len(all)) - logs := make([]string, len(all)) - for i, r := range all { - datas[i] = r.Data - logs[i] = r.Log - } - return basecoin.Result{ - Data: wire.BinaryBytes(datas), - Log: strings.Join(logs, "\n"), - } -} +// // combines all data bytes as a go-wire array. +// // joins all log messages with \n +// func combine(all []basecoin.Result) basecoin.Result { +// datas := make([]data.Bytes, len(all)) +// logs := make([]string, len(all)) +// for i, r := range all { +// datas[i] = r.Data +// logs[i] = r.Log +// } +// return basecoin.Result{ +// Data: wire.BinaryBytes(datas), +// Log: strings.Join(logs, "\n"), +// } +// } diff --git a/modules/base/tx.go b/modules/base/tx.go index d365c1b7b..b2f23db2a 100644 --- a/modules/base/tx.go +++ b/modules/base/tx.go @@ -16,13 +16,13 @@ const ( //nolint const ( - TypeMultiTx = NameMultiplexer + "/tx" + // TypeMultiTx = NameMultiplexer + "/tx" TypeChainTx = NameChain + "/tx" ) func init() { basecoin.TxMapper. - RegisterImplementation(MultiTx{}, TypeMultiTx, ByteMultiTx). + // RegisterImplementation(MultiTx{}, TypeMultiTx, ByteMultiTx). RegisterImplementation(ChainTx{}, TypeChainTx, ByteChainTx) } diff --git a/modules/coin/handler.go b/modules/coin/handler.go index e22dacd26..cb1f4093d 100644 --- a/modules/coin/handler.go +++ b/modules/coin/handler.go @@ -35,7 +35,7 @@ func (Handler) AssertDispatcher() {} // CheckTx checks if there is enough money in the account func (h Handler) CheckTx(ctx basecoin.Context, store state.SimpleDB, - tx basecoin.Tx, _ basecoin.Checker) (res basecoin.Result, err error) { + tx basecoin.Tx, _ basecoin.Checker) (res basecoin.CheckResult, err error) { err = tx.ValidateBasic() if err != nil { @@ -46,14 +46,14 @@ func (h Handler) CheckTx(ctx basecoin.Context, store state.SimpleDB, case SendTx: return res, h.checkSendTx(ctx, store, t) case CreditTx: - return h.creditTx(ctx, store, t) + return res, h.creditTx(ctx, store, t) } return res, errors.ErrUnknownTxType(tx.Unwrap()) } // DeliverTx moves the money func (h Handler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, - tx basecoin.Tx, cb basecoin.Deliver) (res basecoin.Result, err error) { + tx basecoin.Tx, cb basecoin.Deliver) (res basecoin.DeliverResult, err error) { err = tx.ValidateBasic() if err != nil { @@ -62,9 +62,9 @@ func (h Handler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, switch t := tx.Unwrap().(type) { case SendTx: - return h.sendTx(ctx, store, t, cb) + return res, h.sendTx(ctx, store, t, cb) case CreditTx: - return h.creditTx(ctx, store, t) + return res, h.creditTx(ctx, store, t) } return res, errors.ErrUnknownTxType(tx.Unwrap()) } @@ -85,11 +85,11 @@ func (h Handler) SetOption(l log.Logger, store state.SimpleDB, } func (h Handler) sendTx(ctx basecoin.Context, store state.SimpleDB, - send SendTx, cb basecoin.Deliver) (res basecoin.Result, err error) { + send SendTx, cb basecoin.Deliver) error { - err = checkTx(ctx, send) + err := checkTx(ctx, send) if err != nil { - return res, err + return err } // deduct from all input accounts @@ -97,7 +97,7 @@ func (h Handler) sendTx(ctx basecoin.Context, store state.SimpleDB, for _, in := range send.Inputs { _, err = ChangeCoins(store, in.Address, in.Coins.Negative()) if err != nil { - return res, err + return err } senders = append(senders, in.Address) } @@ -112,7 +112,7 @@ func (h Handler) sendTx(ctx basecoin.Context, store state.SimpleDB, _, err = ChangeCoins(store, out.Address, out.Coins) if err != nil { - return res, err + return err } // now send ibc packet if needed... if out.Address.ChainID != "" { @@ -133,46 +133,46 @@ func (h Handler) sendTx(ctx basecoin.Context, store state.SimpleDB, ibcCtx := ctx.WithPermissions(ibc.AllowIBC(NameCoin)) _, err := cb.DeliverTx(ibcCtx, store, packet.Wrap()) if err != nil { - return res, err + return err } } } // a-ok! - return res, nil + return nil } func (h Handler) creditTx(ctx basecoin.Context, store state.SimpleDB, - credit CreditTx) (res basecoin.Result, err error) { + credit CreditTx) error { // first check permissions!! info, err := loadHandlerInfo(store) if err != nil { - return res, err + return err } if info.Issuer.Empty() || !ctx.HasPermission(info.Issuer) { - return res, errors.ErrUnauthorized() + return errors.ErrUnauthorized() } // load up the account addr := ChainAddr(credit.Debitor) acct, err := GetAccount(store, addr) if err != nil { - return res, err + return err } // make and check changes acct.Coins = acct.Coins.Plus(credit.Credit) if !acct.Coins.IsNonnegative() { - return res, ErrInsufficientFunds() + return ErrInsufficientFunds() } acct.Credit = acct.Credit.Plus(credit.Credit) if !acct.Credit.IsNonnegative() { - return res, ErrInsufficientCredit() + return ErrInsufficientCredit() } err = storeAccount(store, addr.Bytes(), acct) - return res, err + return err } func checkTx(ctx basecoin.Context, send SendTx) error { diff --git a/modules/fee/handler.go b/modules/fee/handler.go index 19e9f9f8f..1c597c7f4 100644 --- a/modules/fee/handler.go +++ b/modules/fee/handler.go @@ -45,40 +45,56 @@ 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.Result, err error) { - return h.doTx(ctx, store, tx, next.CheckTx) +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 err != nil { + return res, err + } + + if !fee.Fee.IsZero() { // now, try to make a IPC call to coins... + send := coin.NewSendOneTx(fee.Payer, h.Collector, coin.Coins{fee.Fee}) + _, err = next.CheckTx(ctx, store, send) + if err != nil { + 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.Result, err error) { - return h.doTx(ctx, store, tx, next.DeliverTx) +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 err != nil { + return res, err + } + + if !fee.Fee.IsZero() { // now, try to make a IPC call to coins... + send := coin.NewSendOneTx(fee.Payer, h.Collector, coin.Coins{fee.Fee}) + _, err = next.DeliverTx(ctx, store, send) + if err != nil { + return res, err + } + } + return next.DeliverTx(ctx, store, fee.Tx) } -func (h SimpleFeeMiddleware) doTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.CheckerFunc) (res basecoin.Result, err error) { +func (h SimpleFeeMiddleware) verifyFee(ctx basecoin.Context, tx basecoin.Tx) (Fee, error) { feeTx, ok := tx.Unwrap().(Fee) if !ok { // the fee wrapper is not required if there is no minimum if h.MinFee.IsZero() { - return next(ctx, store, tx) + return feeTx, nil } - return res, errors.ErrInvalidFormat(TypeFees, tx) + return feeTx, errors.ErrInvalidFormat(TypeFees, tx) } // see if it is the proper denom and big enough fee := feeTx.Fee if fee.Denom != h.MinFee.Denom { - return res, ErrWrongFeeDenom(h.MinFee.Denom) + return feeTx, ErrWrongFeeDenom(h.MinFee.Denom) } if !fee.IsGTE(h.MinFee) { - return res, ErrInsufficientFees() + return feeTx, ErrInsufficientFees() } - - // now, try to make a IPC call to coins... - send := coin.NewSendOneTx(feeTx.Payer, h.Collector, coin.Coins{fee}) - _, err = next(ctx, store, send) - if err != nil { - return res, err - } - - return next(ctx, store, feeTx.Tx) + return feeTx, nil } diff --git a/modules/ibc/handler.go b/modules/ibc/handler.go index 4665f5f32..297957af9 100644 --- a/modules/ibc/handler.go +++ b/modules/ibc/handler.go @@ -70,26 +70,27 @@ func (h Handler) SetOption(l log.Logger, store state.SimpleDB, module, key, valu // CheckTx verifies the packet is formated correctly, and has the proper sequence // for a registered chain -func (h Handler) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.Result, err error) { +func (h Handler) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.CheckResult, err error) { err = tx.ValidateBasic() if err != nil { return res, err } - switch t := tx.Unwrap().(type) { + // TODO: better fee calculation (don't do complex crypto) + switch tx.Unwrap().(type) { case RegisterChainTx: - return h.initSeed(ctx, store, t) + return res, nil case UpdateChainTx: - return h.updateSeed(ctx, store, t) + return res, nil case CreatePacketTx: - return h.createPacket(ctx, store, t) + return res, nil } return res, errors.ErrUnknownTxType(tx.Unwrap()) } // DeliverTx verifies all signatures on the tx and updates the chain state // apropriately -func (h Handler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.Result, err error) { +func (h Handler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.DeliverResult, err error) { err = tx.ValidateBasic() if err != nil { return res, err @@ -111,7 +112,7 @@ func (h Handler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx baseco // // only the registrar, if set, is allowed to do this func (h Handler) initSeed(ctx basecoin.Context, store state.SimpleDB, - t RegisterChainTx) (res basecoin.Result, err error) { + t RegisterChainTx) (res basecoin.DeliverResult, err error) { info := LoadInfo(store) if !info.Registrar.Empty() && !ctx.HasPermission(info.Registrar) { @@ -135,7 +136,7 @@ func (h Handler) initSeed(ctx basecoin.Context, store state.SimpleDB, // updateSeed checks the seed against the existing chain data and rejects it if it // doesn't fit (or no chain data) func (h Handler) updateSeed(ctx basecoin.Context, store state.SimpleDB, - t UpdateChainTx) (res basecoin.Result, err error) { + t UpdateChainTx) (res basecoin.DeliverResult, err error) { chainID := t.ChainID() s := NewChainSet(store) @@ -165,7 +166,7 @@ func (h Handler) updateSeed(ctx basecoin.Context, store state.SimpleDB, // createPacket makes sure all permissions are good and the destination // chain is registed. If so, it appends it to the outgoing queue func (h Handler) createPacket(ctx basecoin.Context, store state.SimpleDB, - t CreatePacketTx) (res basecoin.Result, err error) { + t CreatePacketTx) (res basecoin.DeliverResult, err error) { // make sure the chain is registed dest := t.DestChain @@ -203,6 +204,6 @@ func (h Handler) createPacket(ctx basecoin.Context, store state.SimpleDB, packet.Sequence = q.Tail() q.Push(packet.Bytes()) - res = basecoin.Result{Log: fmt.Sprintf("Packet %s %d", dest, packet.Sequence)} + res = basecoin.DeliverResult{Log: fmt.Sprintf("Packet %s %d", dest, packet.Sequence)} return } diff --git a/modules/ibc/middleware.go b/modules/ibc/middleware.go index fe6176964..191a53313 100644 --- a/modules/ibc/middleware.go +++ b/modules/ibc/middleware.go @@ -26,7 +26,7 @@ func (Middleware) Name() string { // CheckTx verifies the named chain and height is present, and verifies // the merkle proof in the packet -func (m Middleware) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Checker) (res basecoin.Result, err error) { +func (m Middleware) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Checker) (res basecoin.CheckResult, err error) { // if it is not a PostPacket, just let it go through post, ok := tx.Unwrap().(PostPacketTx) if !ok { @@ -43,7 +43,7 @@ func (m Middleware) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basec // DeliverTx verifies the named chain and height is present, and verifies // the merkle proof in the packet -func (m Middleware) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.Result, err error) { +func (m Middleware) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.DeliverResult, err error) { // if it is not a PostPacket, just let it go through post, ok := tx.Unwrap().(PostPacketTx) if !ok { diff --git a/modules/ibc/test_helpers.go b/modules/ibc/test_helpers.go index 9d118141e..089bc5d9f 100644 --- a/modules/ibc/test_helpers.go +++ b/modules/ibc/test_helpers.go @@ -100,7 +100,7 @@ func (a *AppChain) IncrementHeight(delta int) int { // DeliverTx runs the tx and commits the new tree, incrementing height // by one. -func (a *AppChain) DeliverTx(tx basecoin.Tx, perms ...basecoin.Actor) (basecoin.Result, error) { +func (a *AppChain) DeliverTx(tx basecoin.Tx, perms ...basecoin.Actor) (basecoin.DeliverResult, error) { ctx := stack.MockContext(a.chainID, uint64(a.height)).WithPermissions(perms...) store := a.store.Checkpoint() res, err := a.app.DeliverTx(ctx, store, tx) diff --git a/modules/nonce/replaycheck.go b/modules/nonce/replaycheck.go index bc57ded96..1a125dea9 100644 --- a/modules/nonce/replaycheck.go +++ b/modules/nonce/replaycheck.go @@ -25,7 +25,7 @@ var _ stack.Middleware = ReplayCheck{} // CheckTx verifies tx is not being replayed - fulfills Middlware interface func (r ReplayCheck) CheckTx(ctx basecoin.Context, store state.SimpleDB, - tx basecoin.Tx, next basecoin.Checker) (res basecoin.Result, err error) { + tx basecoin.Tx, next basecoin.Checker) (res basecoin.CheckResult, err error) { stx, err := r.checkIncrementNonceTx(ctx, store, tx) if err != nil { @@ -39,7 +39,7 @@ func (r ReplayCheck) CheckTx(ctx basecoin.Context, store state.SimpleDB, // NOTE It is okay to modify the sequence before running the wrapped TX because if the // wrapped Tx fails, the state changes are not applied func (r ReplayCheck) DeliverTx(ctx basecoin.Context, store state.SimpleDB, - tx basecoin.Tx, next basecoin.Deliver) (res basecoin.Result, err error) { + tx basecoin.Tx, next basecoin.Deliver) (res basecoin.DeliverResult, err error) { stx, err := r.checkIncrementNonceTx(ctx, store, tx) if err != nil { diff --git a/modules/roles/handler.go b/modules/roles/handler.go index c6f785c14..155ba056b 100644 --- a/modules/roles/handler.go +++ b/modules/roles/handler.go @@ -27,7 +27,7 @@ func (Handler) Name() string { } // CheckTx verifies if the transaction is properly formated -func (h Handler) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.Result, err error) { +func (h Handler) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.CheckResult, err error) { var cr CreateRoleTx cr, err = checkTx(ctx, tx) if err != nil { @@ -40,7 +40,7 @@ func (h Handler) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin // DeliverTx tries to create a new role. // // Returns an error if the role already exists -func (h Handler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.Result, err error) { +func (h Handler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.DeliverResult, err error) { create, err := checkTx(ctx, tx) if err != nil { return res, err diff --git a/modules/roles/middleware.go b/modules/roles/middleware.go index 296f4fa1a..c25382a62 100644 --- a/modules/roles/middleware.go +++ b/modules/roles/middleware.go @@ -27,7 +27,7 @@ func (Middleware) Name() string { // CheckTx tries to assume the named role if requested. // If no role is requested, do nothing. // If insufficient authority to assume the role, return error. -func (m Middleware) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Checker) (res basecoin.Result, err error) { +func (m Middleware) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Checker) (res basecoin.CheckResult, err error) { // if this is not an AssumeRoleTx, then continue assume, ok := tx.Unwrap().(AssumeRoleTx) if !ok { // this also breaks the recursion below @@ -46,7 +46,7 @@ func (m Middleware) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basec // DeliverTx tries to assume the named role if requested. // If no role is requested, do nothing. // If insufficient authority to assume the role, return error. -func (m Middleware) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.Result, err error) { +func (m Middleware) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.DeliverResult, err error) { // if this is not an AssumeRoleTx, then continue assume, ok := tx.Unwrap().(AssumeRoleTx) if !ok { // this also breaks the recursion below diff --git a/stack/checkpoint.go b/stack/checkpoint.go index 00fbe61a1..66524ae54 100644 --- a/stack/checkpoint.go +++ b/stack/checkpoint.go @@ -25,7 +25,7 @@ func (Checkpoint) Name() string { var _ Middleware = Checkpoint{} // CheckTx reverts all data changes if there was an error -func (c Checkpoint) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Checker) (res basecoin.Result, err error) { +func (c Checkpoint) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Checker) (res basecoin.CheckResult, err error) { if !c.OnCheck { return next.CheckTx(ctx, store, tx) } @@ -38,7 +38,7 @@ func (c Checkpoint) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basec } // DeliverTx reverts all data changes if there was an error -func (c Checkpoint) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.Result, err error) { +func (c Checkpoint) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.DeliverResult, err error) { if !c.OnDeliver { return next.DeliverTx(ctx, store, tx) } diff --git a/stack/context.go b/stack/context.go index 7359adbe1..6fe31f244 100644 --- a/stack/context.go +++ b/stack/context.go @@ -104,7 +104,7 @@ func withIBC(ctx basecoin.Context) basecoin.Context { } func secureCheck(h basecoin.Checker, parent basecoin.Context) basecoin.Checker { - next := func(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.Result, err error) { + next := func(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.CheckResult, err error) { if !parent.IsParent(ctx) { return res, errors.New("Passing in non-child Context") } @@ -114,7 +114,7 @@ func secureCheck(h basecoin.Checker, parent basecoin.Context) basecoin.Checker { } func secureDeliver(h basecoin.Deliver, parent basecoin.Context) basecoin.Deliver { - next := func(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.Result, err error) { + next := func(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.DeliverResult, err error) { if !parent.IsParent(ctx) { return res, errors.New("Passing in non-child Context") } diff --git a/stack/dispatcher.go b/stack/dispatcher.go index a8ff96c48..0efd7d339 100644 --- a/stack/dispatcher.go +++ b/stack/dispatcher.go @@ -64,7 +64,7 @@ func (d *Dispatcher) Name() string { // Tries to find a registered module (Dispatchable) based on the name of the tx. // The tx name (as registered with go-data) should be in the form `/XXXX`, // where `module name` must match the name of a dispatchable and XXX can be any string. -func (d *Dispatcher) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.Result, err error) { +func (d *Dispatcher) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.CheckResult, err error) { r, err := d.lookupTx(tx) if err != nil { return res, err @@ -85,7 +85,7 @@ func (d *Dispatcher) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx base // Tries to find a registered module (Dispatchable) based on the name of the tx. // The tx name (as registered with go-data) should be in the form `/XXXX`, // where `module name` must match the name of a dispatchable and XXX can be any string. -func (d *Dispatcher) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.Result, err error) { +func (d *Dispatcher) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.DeliverResult, err error) { r, err := d.lookupTx(tx) if err != nil { return res, err diff --git a/stack/helpers.go b/stack/helpers.go index b38e86bc6..e91086d35 100644 --- a/stack/helpers.go +++ b/stack/helpers.go @@ -105,13 +105,13 @@ func (OKHandler) Name() string { } // CheckTx always returns an empty success tx -func (ok OKHandler) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.Result, err error) { - return basecoin.Result{Log: ok.Log}, nil +func (ok OKHandler) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.CheckResult, err error) { + return basecoin.CheckResult{Log: ok.Log}, nil } // DeliverTx always returns an empty success tx -func (ok OKHandler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.Result, err error) { - return basecoin.Result{Log: ok.Log}, nil +func (ok OKHandler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.DeliverResult, err error) { + return basecoin.DeliverResult{Log: ok.Log}, nil } // EchoHandler returns success, echoing res.Data = tx bytes @@ -127,15 +127,15 @@ func (EchoHandler) Name() string { } // CheckTx always returns an empty success tx -func (EchoHandler) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.Result, err error) { +func (EchoHandler) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.CheckResult, err error) { data, err := data.ToWire(tx) - return basecoin.Result{Data: data}, err + return basecoin.CheckResult{Data: data}, err } // DeliverTx always returns an empty success tx -func (EchoHandler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.Result, err error) { +func (EchoHandler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.DeliverResult, err error) { data, err := data.ToWire(tx) - return basecoin.Result{Data: data}, err + return basecoin.DeliverResult{Data: data}, err } // FailHandler always returns an error @@ -152,12 +152,12 @@ func (FailHandler) Name() string { } // CheckTx always returns the given error -func (f FailHandler) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.Result, err error) { +func (f FailHandler) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.CheckResult, err error) { return res, errors.Wrap(f.Err) } // DeliverTx always returns the given error -func (f FailHandler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.Result, err error) { +func (f FailHandler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.DeliverResult, err error) { return res, errors.Wrap(f.Err) } @@ -176,7 +176,7 @@ func (PanicHandler) Name() string { } // CheckTx always panics -func (p PanicHandler) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.Result, err error) { +func (p PanicHandler) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.CheckResult, err error) { if p.Err != nil { panic(p.Err) } @@ -184,7 +184,7 @@ func (p PanicHandler) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx bas } // DeliverTx always panics -func (p PanicHandler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.Result, err error) { +func (p PanicHandler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.DeliverResult, err error) { if p.Err != nil { panic(p.Err) } @@ -204,7 +204,7 @@ func (CheckHandler) Name() string { } // CheckTx verifies the permissions -func (c CheckHandler) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.Result, err error) { +func (c CheckHandler) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.CheckResult, err error) { check, ok := tx.Unwrap().(CheckTx) if !ok { return res, errors.ErrUnknownTxType(tx) @@ -219,7 +219,16 @@ func (c CheckHandler) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx bas } // DeliverTx verifies the permissions -func (c CheckHandler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.Result, err error) { - // until something changes, just do the same as check - return c.CheckTx(ctx, store, tx) +func (c CheckHandler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.DeliverResult, err error) { + check, ok := tx.Unwrap().(CheckTx) + if !ok { + return res, errors.ErrUnknownTxType(tx) + } + + for _, perm := range check.Required { + if !ctx.HasPermission(perm) { + return res, errors.ErrUnauthorized() + } + } + return res, nil } diff --git a/stack/helperware.go b/stack/helperware.go index 353f90907..6fd2078a3 100644 --- a/stack/helperware.go +++ b/stack/helperware.go @@ -25,14 +25,14 @@ func (_ CheckMiddleware) Name() string { return NameCheck } -func (p CheckMiddleware) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Checker) (res basecoin.Result, err error) { +func (p CheckMiddleware) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Checker) (res basecoin.CheckResult, err error) { if !ctx.HasPermission(p.Required) { return res, errors.ErrUnauthorized() } return next.CheckTx(ctx, store, tx) } -func (p CheckMiddleware) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.Result, err error) { +func (p CheckMiddleware) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.DeliverResult, err error) { if !ctx.HasPermission(p.Required) { return res, errors.ErrUnauthorized() } @@ -51,12 +51,12 @@ func (_ GrantMiddleware) Name() string { return NameGrant } -func (g GrantMiddleware) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Checker) (res basecoin.Result, err error) { +func (g GrantMiddleware) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Checker) (res basecoin.CheckResult, err error) { ctx = ctx.WithPermissions(g.Auth) return next.CheckTx(ctx, store, tx) } -func (g GrantMiddleware) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.Result, err error) { +func (g GrantMiddleware) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.DeliverResult, err error) { ctx = ctx.WithPermissions(g.Auth) return next.DeliverTx(ctx, store, tx) } diff --git a/stack/interface.go b/stack/interface.go index fedf70401..81cdbffad 100644 --- a/stack/interface.go +++ b/stack/interface.go @@ -21,27 +21,27 @@ type Middleware interface { type CheckerMiddle interface { 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) } type CheckerMiddleFunc func(basecoin.Context, state.SimpleDB, - basecoin.Tx, basecoin.Checker) (basecoin.Result, error) + basecoin.Tx, basecoin.Checker) (basecoin.CheckResult, error) func (c CheckerMiddleFunc) 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) { return c(ctx, store, tx, next) } type DeliverMiddle interface { DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, - next basecoin.Deliver) (basecoin.Result, error) + next basecoin.Deliver) (basecoin.DeliverResult, error) } type DeliverMiddleFunc func(basecoin.Context, state.SimpleDB, - basecoin.Tx, basecoin.Deliver) (basecoin.Result, error) + basecoin.Tx, basecoin.Deliver) (basecoin.DeliverResult, error) func (d DeliverMiddleFunc) 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) { return d(ctx, store, tx, next) } @@ -62,14 +62,14 @@ func (c SetOptionMiddleFunc) SetOption(l log.Logger, store state.SimpleDB, type PassCheck struct{} func (_ PassCheck) 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) { return next.CheckTx(ctx, store, tx) } type PassDeliver struct{} func (_ PassDeliver) 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) { return next.DeliverTx(ctx, store, tx) } @@ -113,12 +113,12 @@ func (w wrapped) Name() string { } func (w wrapped) CheckTx(ctx basecoin.Context, store state.SimpleDB, - tx basecoin.Tx, _ basecoin.Checker) (basecoin.Result, error) { + tx basecoin.Tx, _ basecoin.Checker) (basecoin.CheckResult, error) { return w.h.CheckTx(ctx, store, tx) } func (w wrapped) DeliverTx(ctx basecoin.Context, store state.SimpleDB, - tx basecoin.Tx, _ basecoin.Deliver) (basecoin.Result, error) { + tx basecoin.Tx, _ basecoin.Deliver) (basecoin.DeliverResult, error) { return w.h.DeliverTx(ctx, store, tx) } diff --git a/stack/middleware.go b/stack/middleware.go index c5f6a9b71..e24e86bde 100644 --- a/stack/middleware.go +++ b/stack/middleware.go @@ -31,7 +31,7 @@ func (m *middleware) wrapCtx(ctx basecoin.Context) basecoin.Context { } // CheckTx always returns an empty success tx -func (m *middleware) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (basecoin.Result, error) { +func (m *middleware) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (basecoin.CheckResult, error) { // make sure we pass in proper context to child next := secureCheck(m.next, ctx) // set the permissions for this app @@ -42,7 +42,7 @@ func (m *middleware) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx base } // DeliverTx always returns an empty success tx -func (m *middleware) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.Result, err error) { +func (m *middleware) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx) (res basecoin.DeliverResult, err error) { // make sure we pass in proper context to child next := secureDeliver(m.next, ctx) // set the permissions for this app diff --git a/stack/recovery.go b/stack/recovery.go index 77ff09ef4..eae976ad0 100644 --- a/stack/recovery.go +++ b/stack/recovery.go @@ -26,7 +26,7 @@ func (Recovery) Name() string { var _ Middleware = Recovery{} // CheckTx catches any panic and converts to error - fulfills Middlware interface -func (Recovery) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Checker) (res basecoin.Result, err error) { +func (Recovery) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Checker) (res basecoin.CheckResult, err error) { defer func() { if r := recover(); r != nil { err = normalizePanic(r) @@ -36,7 +36,7 @@ func (Recovery) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin. } // DeliverTx catches any panic and converts to error - fulfills Middlware interface -func (Recovery) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.Result, err error) { +func (Recovery) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.DeliverResult, err error) { defer func() { if r := recover(); r != nil { err = normalizePanic(r) From eb08136104f17b7fcd845b07d8927595d11379a9 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sun, 30 Jul 2017 12:41:41 -0400 Subject: [PATCH 03/13] 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, From 4b69f1d5bad35dab3107e54f3c3b98e9a4627c46 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sun, 30 Jul 2017 12:57:48 -0400 Subject: [PATCH 04/13] Rename SetOption to InitState --- TODO.md | 8 +++++ app/app.go | 12 +++++-- app/app_test.go | 20 ++++++------ app/genesis.go | 6 ++-- benchmarks/app_test.go | 4 +-- .../counter/plugins/counter/counter_test.go | 4 +-- handler.go | 22 +++++++------ modules/base/logger.go | 10 +++--- modules/coin/bench_test.go | 2 +- modules/coin/handler.go | 6 ++-- modules/coin/handler_test.go | 10 +++--- modules/coin/helper.go | 2 +- modules/coin/ibc_test.go | 2 +- modules/fee/handler_test.go | 4 +-- modules/ibc/handler.go | 6 ++-- modules/ibc/ibc_test.go | 2 +- modules/ibc/test_helpers.go | 6 ++-- stack/dispatcher.go | 8 ++--- stack/interface.go | 32 +++++++++---------- stack/middleware.go | 4 +-- stack/recovery.go | 6 ++-- stack/state_space_test.go | 8 ++--- 22 files changed, 100 insertions(+), 84 deletions(-) create mode 100644 TODO.md diff --git a/TODO.md b/TODO.md new file mode 100644 index 000000000..2eb6af870 --- /dev/null +++ b/TODO.md @@ -0,0 +1,8 @@ +# TODO for rewrite + +* Reimplement MultiTx in base + +* FeeTx and CheckTx changes logic to estimate, not validate +* Add tests for new CheckTx +* Handle ValidatorSet responses from DeliverTx calls in EndBlock +* Add InitValidator call to all modules diff --git a/app/app.go b/app/app.go index ca62ea80f..d05459297 100644 --- a/app/app.go +++ b/app/app.go @@ -65,8 +65,9 @@ func (app *Basecoin) Info() abci.ResponseInfo { } } -// SetOption - ABCI -func (app *Basecoin) SetOption(key string, value string) string { +// InitState - used to setup state (was SetOption) +// to be used by InitChain later +func (app *Basecoin) InitState(key string, value string) string { module, key := splitKey(key) state := app.state.Append() @@ -79,13 +80,18 @@ func (app *Basecoin) SetOption(key string, value string) string { return fmt.Sprintf("Error: unknown base option: %s", key) } - log, err := app.handler.SetOption(app.logger, state, module, key, value) + log, err := app.handler.InitState(app.logger, state, module, key, value) if err == nil { return log } return "Error: " + err.Error() } +// SetOption - ABCI +func (app *Basecoin) SetOption(key string, value string) string { + return "Not Implemented" +} + // DeliverTx - ABCI func (app *Basecoin) DeliverTx(txBytes []byte) abci.Result { tx, err := basecoin.LoadTx(txBytes) diff --git a/app/app_test.go b/app/app_test.go index 4bef8443c..0da5b31c1 100644 --- a/app/app_test.go +++ b/app/app_test.go @@ -98,9 +98,9 @@ func (at *appTest) feeTx(coins coin.Coins, toll coin.Coin, sequence uint32) base return at.signTx(tx) } -// set the account on the app through SetOption +// set the account on the app through InitState func (at *appTest) initAccount(acct *coin.AccountWithKey) { - res := at.app.SetOption("coin/account", acct.MakeOption()) + res := at.app.InitState("coin/account", acct.MakeOption()) require.EqualValues(at.t, res, "Success") } @@ -121,7 +121,7 @@ func (at *appTest) reset() { logger.With("module", "app"), ) - res := at.app.SetOption("base/chain_id", at.chainID) + res := at.app.InitState("base/chain_id", at.chainID) require.EqualValues(at.t, res, "Success") at.initAccount(at.acctIn) @@ -167,7 +167,7 @@ func (at *appTest) exec(t *testing.T, tx basecoin.Tx, checkTx bool) (res abci.Re //-------------------------------------------------------- -func TestSetOption(t *testing.T) { +func TestInitState(t *testing.T) { assert := assert.New(t) require := require.New(t) @@ -183,14 +183,14 @@ func TestSetOption(t *testing.T) { //testing ChainID chainID := "testChain" - res := app.SetOption("base/chain_id", chainID) + res := app.InitState("base/chain_id", chainID) assert.EqualValues(app.GetChainID(), chainID) assert.EqualValues(res, "Success") // make a nice account... bal := coin.Coins{{"atom", 77}, {"eth", 12}} acct := coin.NewAccountWithKey(bal) - res = app.SetOption("coin/account", acct.MakeOption()) + res = app.InitState("coin/account", acct.MakeOption()) require.EqualValues(res, "Success") // make sure it is set correctly, with some balance @@ -218,7 +218,7 @@ func TestSetOption(t *testing.T) { } ] }` - res = app.SetOption("coin/account", unsortAcc) + res = app.InitState("coin/account", unsortAcc) require.EqualValues(res, "Success") coins, err = getAddr(unsortAddr, app.GetState()) @@ -226,13 +226,13 @@ func TestSetOption(t *testing.T) { assert.True(coins.IsValid()) assert.Equal(unsortCoins, coins) - res = app.SetOption("base/dslfkgjdas", "") + res = app.InitState("base/dslfkgjdas", "") assert.NotEqual(res, "Success") - res = app.SetOption("dslfkgjdas", "") + res = app.InitState("dslfkgjdas", "") assert.NotEqual(res, "Success") - res = app.SetOption("dslfkgjdas/szfdjzs", "") + res = app.InitState("dslfkgjdas/szfdjzs", "") assert.NotEqual(res, "Success") } diff --git a/app/genesis.go b/app/genesis.go index 87c60227f..81d99fead 100644 --- a/app/genesis.go +++ b/app/genesis.go @@ -16,16 +16,16 @@ func (app *Basecoin) LoadGenesis(path string) error { } // set chain_id - app.SetOption("base/chain_id", genDoc.ChainID) + app.InitState("base/chain_id", genDoc.ChainID) // set accounts for _, acct := range genDoc.AppOptions.Accounts { - _ = app.SetOption("coin/account", string(acct)) + _ = app.InitState("coin/account", string(acct)) } // set plugin options for _, kv := range genDoc.AppOptions.pluginOptions { - _ = app.SetOption(kv.Key, kv.Value) + _ = app.InitState(kv.Key, kv.Value) } return nil diff --git a/benchmarks/app_test.go b/benchmarks/app_test.go index 96487afe0..b25ffce0c 100644 --- a/benchmarks/app_test.go +++ b/benchmarks/app_test.go @@ -72,7 +72,7 @@ func NewBenchApp(h basecoin.Handler, chainID string, n int, store, logger.With("module", "app"), ) - res := app.SetOption("base/chain_id", chainID) + res := app.InitState("base/chain_id", chainID) if res != "Success" { panic("cannot set chain") } @@ -82,7 +82,7 @@ func NewBenchApp(h basecoin.Handler, chainID string, n int, accts := make([]*coin.AccountWithKey, n) for i := 0; i < n; i++ { accts[i] = coin.NewAccountWithKey(money) - res := app.SetOption("coin/account", accts[i].MakeOption()) + res := app.InitState("coin/account", accts[i].MakeOption()) if res != "Success" { panic("can't set account") } diff --git a/docs/guide/counter/plugins/counter/counter_test.go b/docs/guide/counter/plugins/counter/counter_test.go index b9fb9c2d5..968760492 100644 --- a/docs/guide/counter/plugins/counter/counter_test.go +++ b/docs/guide/counter/plugins/counter/counter_test.go @@ -36,12 +36,12 @@ func TestCounterPlugin(t *testing.T) { store, logger.With("module", "app"), ) - bcApp.SetOption("base/chain_id", chainID) + bcApp.InitState("base/chain_id", chainID) // Account initialization bal := coin.Coins{{"", 1000}, {"gold", 1000}} acct := coin.NewAccountWithKey(bal) - log := bcApp.SetOption("coin/account", acct.MakeOption()) + log := bcApp.InitState("coin/account", acct.MakeOption()) require.Equal("Success", log) // Deliver a CounterTx diff --git a/handler.go b/handler.go index d2858ea94..a7e2b7334 100644 --- a/handler.go +++ b/handler.go @@ -12,12 +12,14 @@ import ( type Handler interface { Checker Deliver - SetOptioner + // This is for app options + InitStater Named - // TODO: flesh these out as well - // InitChain(store state.SimpleDB, vals []*abci.Validator) + // TODO: for staker + // InitChain(log log.Logger, store state.SimpleDB, vals []*abci.Validator) + + // TODO???? // BeginBlock(store state.SimpleDB, hash []byte, header *abci.Header) - // EndBlock(store state.SimpleDB, height uint64) abci.ResponseEndBlock } type Named interface { @@ -46,14 +48,14 @@ func (c DeliverFunc) DeliverTx(ctx Context, store state.SimpleDB, tx Tx) (Delive return c(ctx, store, tx) } -type SetOptioner interface { - SetOption(l log.Logger, store state.SimpleDB, module, key, value string) (string, error) +type InitStater interface { + InitState(l log.Logger, store state.SimpleDB, module, key, value string) (string, error) } -// SetOptionFunc (like http.HandlerFunc) is a shortcut for making wrapers -type SetOptionFunc func(log.Logger, state.SimpleDB, string, string, string) (string, error) +// InitStateFunc (like http.HandlerFunc) is a shortcut for making wrapers +type InitStateFunc func(log.Logger, state.SimpleDB, string, string, string) (string, error) -func (c SetOptionFunc) SetOption(l log.Logger, store state.SimpleDB, module, key, value string) (string, error) { +func (c InitStateFunc) InitState(l log.Logger, store state.SimpleDB, module, key, value string) (string, error) { return c(l, store, module, key, value) } @@ -119,6 +121,6 @@ func (_ NopDeliver) DeliverTx(Context, state.SimpleDB, Tx) (r DeliverResult, e e type NopOption struct{} -func (_ NopOption) SetOption(log.Logger, state.SimpleDB, string, string, string) (string, error) { +func (_ NopOption) InitState(log.Logger, state.SimpleDB, string, string, string) (string, error) { return "", nil } diff --git a/modules/base/logger.go b/modules/base/logger.go index 126e3d056..581fcfb42 100644 --- a/modules/base/logger.go +++ b/modules/base/logger.go @@ -55,17 +55,17 @@ func (Logger) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin. return } -// SetOption logs time and result - fulfills Middlware interface -func (Logger) SetOption(l log.Logger, store state.SimpleDB, module, key, value string, next basecoin.SetOptioner) (string, error) { +// InitState logs time and result - fulfills Middlware interface +func (Logger) InitState(l log.Logger, store state.SimpleDB, module, key, value string, next basecoin.InitStater) (string, error) { start := time.Now() - res, err := next.SetOption(l, store, module, key, value) + res, err := next.InitState(l, store, module, key, value) delta := time.Now().Sub(start) // TODO: log the value being set also? l = l.With("duration", micros(delta)).With("mod", module).With("key", key) if err == nil { - l.Info("SetOption", "log", res) + l.Info("InitState", "log", res) } else { - l.Error("SetOption", "err", err) + l.Error("InitState", "err", err) } return res, err } diff --git a/modules/coin/bench_test.go b/modules/coin/bench_test.go index bc83ea90c..cfa7b708b 100644 --- a/modules/coin/bench_test.go +++ b/modules/coin/bench_test.go @@ -28,7 +28,7 @@ func BenchmarkSimpleTransfer(b *testing.B) { // set the initial account acct := NewAccountWithKey(Coins{{"mycoin", 1234567890}}) - h.SetOption(logger, store, NameCoin, "account", acct.MakeOption(), nil) + h.InitState(logger, store, NameCoin, "account", acct.MakeOption(), nil) sender := acct.Actor() receiver := basecoin.Actor{App: "foo", Address: cmn.RandBytes(20)} diff --git a/modules/coin/handler.go b/modules/coin/handler.go index cb1f4093d..dfaa2fa17 100644 --- a/modules/coin/handler.go +++ b/modules/coin/handler.go @@ -69,9 +69,9 @@ func (h Handler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, return res, errors.ErrUnknownTxType(tx.Unwrap()) } -// SetOption - sets the genesis account balance -func (h Handler) SetOption(l log.Logger, store state.SimpleDB, - module, key, value string, cb basecoin.SetOptioner) (log string, err error) { +// InitState - sets the genesis account balance +func (h Handler) InitState(l log.Logger, store state.SimpleDB, + module, key, value string, cb basecoin.InitStater) (log string, err error) { if module != NameCoin { return "", errors.ErrUnknownModule(module) } diff --git a/modules/coin/handler_test.go b/modules/coin/handler_test.go index b0f20c42f..d43e20fa4 100644 --- a/modules/coin/handler_test.go +++ b/modules/coin/handler_test.go @@ -167,7 +167,7 @@ func TestDeliverSendTx(t *testing.T) { } } -func TestSetOption(t *testing.T) { +func TestInitState(t *testing.T) { assert := assert.New(t) require := require.New(t) @@ -205,7 +205,7 @@ func TestSetOption(t *testing.T) { for j, gen := range tc.init { value, err := json.Marshal(gen) require.Nil(err, "%d,%d: %+v", i, j, err) - _, err = h.SetOption(l, store, NameCoin, key, string(value), nil) + _, err = h.InitState(l, store, NameCoin, key, string(value), nil) require.Nil(err) } @@ -239,7 +239,7 @@ func TestSetIssuer(t *testing.T) { value, err := json.Marshal(tc.issuer) require.Nil(err, "%d,%d: %+v", i, err) - _, err = h.SetOption(l, store, NameCoin, key, string(value), nil) + _, err = h.InitState(l, store, NameCoin, key, string(value), nil) require.Nil(err, "%+v", err) // check state is proper @@ -274,11 +274,11 @@ func TestDeliverCreditTx(t *testing.T) { // set the owner who can issue credit js, err := json.Marshal(owner) require.Nil(err, "%+v", err) - _, err = h.SetOption(log.NewNopLogger(), store, "coin", "issuer", string(js), nil) + _, err = h.InitState(log.NewNopLogger(), store, "coin", "issuer", string(js), nil) require.Nil(err, "%+v", err) // give addr2 some coins to start - _, err = h.SetOption(log.NewNopLogger(), store, "coin", "account", key.MakeOption(), nil) + _, err = h.InitState(log.NewNopLogger(), store, "coin", "account", key.MakeOption(), nil) require.Nil(err, "%+v", err) cases := []struct { diff --git a/modules/coin/helper.go b/modules/coin/helper.go index 4bf98cddc..c4af2f080 100644 --- a/modules/coin/helper.go +++ b/modules/coin/helper.go @@ -41,7 +41,7 @@ func (a *AccountWithKey) NextSequence() uint32 { return a.Sequence } -// MakeOption returns a string to use with SetOption to initialize this account +// MakeOption returns a string to use with InitState to initialize this account // // This is intended for use in test cases func (a *AccountWithKey) MakeOption() string { diff --git a/modules/coin/ibc_test.go b/modules/coin/ibc_test.go index 7b4014a32..81aeb8365 100644 --- a/modules/coin/ibc_test.go +++ b/modules/coin/ibc_test.go @@ -44,7 +44,7 @@ func TestIBCPostPacket(t *testing.T) { // set up a rich guy on this chain wealth := Coins{{"btc", 300}, {"eth", 2000}, {"ltc", 5000}} rich := NewAccountWithKey(wealth) - _, err = ourChain.SetOption("coin", "account", rich.MakeOption()) + _, err = ourChain.InitState("coin", "account", rich.MakeOption()) require.Nil(err, "%+v", err) // sends money to another guy on a different chain, now other chain has credit diff --git a/modules/fee/handler_test.go b/modules/fee/handler_test.go index 000d99f0b..3fb72abaa 100644 --- a/modules/fee/handler_test.go +++ b/modules/fee/handler_test.go @@ -50,9 +50,9 @@ func TestFeeChecks(t *testing.T) { // set up the store and init the accounts store := state.NewMemKVStore() l := log.NewNopLogger() - _, err := app1.SetOption(l, store, "coin", "account", key1.MakeOption()) + _, err := app1.InitState(l, store, "coin", "account", key1.MakeOption()) require.Nil(err, "%+v", err) - _, err = app2.SetOption(l, store, "coin", "account", key2.MakeOption()) + _, err = app2.InitState(l, store, "coin", "account", key2.MakeOption()) require.Nil(err, "%+v", err) cases := []struct { diff --git a/modules/ibc/handler.go b/modules/ibc/handler.go index 297957af9..298f62780 100644 --- a/modules/ibc/handler.go +++ b/modules/ibc/handler.go @@ -39,7 +39,7 @@ type Handler struct{} var _ basecoin.Handler = Handler{} // NewHandler returns a Handler that allows all chains to connect via IBC. -// Set a Registrar via SetOption to restrict it. +// Set a Registrar via InitState to restrict it. func NewHandler() Handler { return Handler{} } @@ -49,8 +49,8 @@ func (Handler) Name() string { return NameIBC } -// SetOption sets the registrar for IBC -func (h Handler) SetOption(l log.Logger, store state.SimpleDB, module, key, value string) (log string, err error) { +// InitState sets the registrar for IBC +func (h Handler) InitState(l log.Logger, store state.SimpleDB, module, key, value string) (log string, err error) { if module != NameIBC { return "", errors.ErrUnknownModule(module) } diff --git a/modules/ibc/ibc_test.go b/modules/ibc/ibc_test.go index c31aeae53..1fb039419 100644 --- a/modules/ibc/ibc_test.go +++ b/modules/ibc/ibc_test.go @@ -130,7 +130,7 @@ func TestIBCRegisterPermissions(t *testing.T) { // set option specifies the registrar msg, err := json.Marshal(tc.registrar) require.Nil(err, "%+v", err) - _, err = app.SetOption(log.NewNopLogger(), store, + _, err = app.InitState(log.NewNopLogger(), store, NameIBC, OptionRegistrar, string(msg)) require.Nil(err, "%+v", err) diff --git a/modules/ibc/test_helpers.go b/modules/ibc/test_helpers.go index 089bc5d9f..d2272a0e0 100644 --- a/modules/ibc/test_helpers.go +++ b/modules/ibc/test_helpers.go @@ -118,9 +118,9 @@ func (a *AppChain) Update(tx UpdateChainTx) error { return err } -// SetOption sets the option on our app -func (a *AppChain) SetOption(mod, key, value string) (string, error) { - return a.app.SetOption(log.NewNopLogger(), a.store, mod, key, value) +// InitState sets the option on our app +func (a *AppChain) InitState(mod, key, value string) (string, error) { + return a.app.InitState(log.NewNopLogger(), a.store, mod, key, value) } // GetStore is used to get the app-specific sub-store diff --git a/stack/dispatcher.go b/stack/dispatcher.go index 0efd7d339..e4a34c0b7 100644 --- a/stack/dispatcher.go +++ b/stack/dispatcher.go @@ -101,11 +101,11 @@ func (d *Dispatcher) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx ba return r.DeliverTx(ctx, store, tx, cb) } -// SetOption - implements Handler interface +// InitState - implements Handler interface // // Tries to find a registered module (Dispatchable) based on the -// module name from SetOption of the tx. -func (d *Dispatcher) SetOption(l log.Logger, store state.SimpleDB, module, key, value string) (string, error) { +// module name from InitState of the tx. +func (d *Dispatcher) InitState(l log.Logger, store state.SimpleDB, module, key, value string) (string, error) { r, err := d.lookupModule(module) if err != nil { return "", err @@ -116,7 +116,7 @@ func (d *Dispatcher) SetOption(l log.Logger, store state.SimpleDB, module, key, // but isolate data space store = stateSpace(store, r.Name()) - return r.SetOption(l, store, module, key, value, cb) + return r.InitState(l, store, module, key, value, cb) } func (d *Dispatcher) lookupTx(tx basecoin.Tx) (Dispatchable, error) { diff --git a/stack/interface.go b/stack/interface.go index 81cdbffad..173e61d14 100644 --- a/stack/interface.go +++ b/stack/interface.go @@ -15,7 +15,7 @@ import ( type Middleware interface { CheckerMiddle DeliverMiddle - SetOptionMiddle + InitStateMiddle basecoin.Named } @@ -45,16 +45,16 @@ func (d DeliverMiddleFunc) DeliverTx(ctx basecoin.Context, store state.SimpleDB, return d(ctx, store, tx, next) } -type SetOptionMiddle interface { - SetOption(l log.Logger, store state.SimpleDB, module, - key, value string, next basecoin.SetOptioner) (string, error) +type InitStateMiddle interface { + InitState(l log.Logger, store state.SimpleDB, module, + key, value string, next basecoin.InitStater) (string, error) } -type SetOptionMiddleFunc func(log.Logger, state.SimpleDB, - string, string, string, basecoin.SetOptioner) (string, error) +type InitStateMiddleFunc func(log.Logger, state.SimpleDB, + string, string, string, basecoin.InitStater) (string, error) -func (c SetOptionMiddleFunc) SetOption(l log.Logger, store state.SimpleDB, - module, key, value string, next basecoin.SetOptioner) (string, error) { +func (c InitStateMiddleFunc) InitState(l log.Logger, store state.SimpleDB, + module, key, value string, next basecoin.InitStater) (string, error) { return c(l, store, module, key, value, next) } @@ -75,15 +75,15 @@ func (_ PassDeliver) DeliverTx(ctx basecoin.Context, store state.SimpleDB, type PassOption struct{} -func (_ PassOption) SetOption(l log.Logger, store state.SimpleDB, module, - key, value string, next basecoin.SetOptioner) (string, error) { - return next.SetOption(l, store, module, key, value) +func (_ PassOption) InitState(l log.Logger, store state.SimpleDB, module, + key, value string, next basecoin.InitStater) (string, error) { + return next.InitState(l, store, module, key, value) } type NopOption struct{} -func (_ NopOption) SetOption(l log.Logger, store state.SimpleDB, module, - key, value string, next basecoin.SetOptioner) (string, error) { +func (_ NopOption) InitState(l log.Logger, store state.SimpleDB, module, + key, value string, next basecoin.InitStater) (string, error) { return "", nil } @@ -122,7 +122,7 @@ func (w wrapped) DeliverTx(ctx basecoin.Context, store state.SimpleDB, return w.h.DeliverTx(ctx, store, tx) } -func (w wrapped) SetOption(l log.Logger, store state.SimpleDB, - module, key, value string, _ basecoin.SetOptioner) (string, error) { - return w.h.SetOption(l, store, module, key, value) +func (w wrapped) InitState(l log.Logger, store state.SimpleDB, + module, key, value string, _ basecoin.InitStater) (string, error) { + return w.h.InitState(l, store, module, key, value) } diff --git a/stack/middleware.go b/stack/middleware.go index e24e86bde..4ba29a202 100644 --- a/stack/middleware.go +++ b/stack/middleware.go @@ -52,11 +52,11 @@ func (m *middleware) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx ba return m.middleware.DeliverTx(ctx, store, tx, next) } -func (m *middleware) SetOption(l log.Logger, store state.SimpleDB, module, key, value string) (string, error) { +func (m *middleware) InitState(l log.Logger, store state.SimpleDB, module, key, value string) (string, error) { // set the namespace for the app store = stateSpace(store, m.space) - return m.middleware.SetOption(l, store, module, key, value, m.next) + return m.middleware.InitState(l, store, module, key, value, m.next) } // builder is used to associate info with the middleware, so we can build diff --git a/stack/recovery.go b/stack/recovery.go index eae976ad0..e5b986ef2 100644 --- a/stack/recovery.go +++ b/stack/recovery.go @@ -45,14 +45,14 @@ func (Recovery) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoi return next.DeliverTx(ctx, store, tx) } -// SetOption catches any panic and converts to error - fulfills Middlware interface -func (Recovery) SetOption(l log.Logger, store state.SimpleDB, module, key, value string, next basecoin.SetOptioner) (log string, err error) { +// InitState catches any panic and converts to error - fulfills Middlware interface +func (Recovery) InitState(l log.Logger, store state.SimpleDB, module, key, value string, next basecoin.InitStater) (log string, err error) { defer func() { if r := recover(); r != nil { err = normalizePanic(r) } }() - return next.SetOption(l, store, module, key, value) + return next.InitState(l, store, module, key, value) } // normalizePanic makes sure we can get a nice TMError (with stack) out of it diff --git a/stack/state_space_test.go b/stack/state_space_test.go index 1b3a6f368..b18671106 100644 --- a/stack/state_space_test.go +++ b/stack/state_space_test.go @@ -35,10 +35,10 @@ func (w writerMid) DeliverTx(ctx basecoin.Context, store state.SimpleDB, return next.DeliverTx(ctx, store, tx) } -func (w writerMid) SetOption(l log.Logger, store state.SimpleDB, module, - key, value string, next basecoin.SetOptioner) (string, error) { +func (w writerMid) InitState(l log.Logger, store state.SimpleDB, module, + key, value string, next basecoin.InitStater) (string, error) { store.Set([]byte(key), []byte(value)) - return next.SetOption(l, store, module, key, value) + return next.InitState(l, store, module, key, value) } // writerHand is a middleware that writes the given bytes on CheckTx and DeliverTx @@ -63,7 +63,7 @@ func (w writerHand) DeliverTx(ctx basecoin.Context, store state.SimpleDB, return basecoin.DeliverResult{}, nil } -func (w writerHand) SetOption(l log.Logger, store state.SimpleDB, module, +func (w writerHand) InitState(l log.Logger, store state.SimpleDB, module, key, value string) (string, error) { store.Set([]byte(key), []byte(value)) return "Success", nil From 37550ca91d24a16b7f0750f55d4d5d92e40f6b0c Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sun, 30 Jul 2017 17:26:25 -0400 Subject: [PATCH 05/13] Add InitValidate method for setup --- docs/guide/counter/plugins/counter/counter.go | 3 +- handler.go | 33 +++++++++++++--- modules/auth/signature.go | 3 +- modules/base/chain.go | 3 +- modules/base/logger.go | 10 +++++ modules/base/multiplexer.go | 2 +- modules/coin/handler.go | 4 +- modules/fee/handler.go | 3 +- modules/ibc/handler.go | 4 +- modules/ibc/middleware.go | 3 +- modules/nonce/replaycheck.go | 3 +- modules/roles/handler.go | 3 +- modules/roles/middleware.go | 3 +- stack/checkpoint.go | 3 +- stack/checkpoint_test.go | 4 +- stack/dispatcher.go | 28 +++++++++++++ stack/helpers.go | 15 ++++--- stack/helperware.go | 6 ++- stack/interface.go | 39 ++++++++++++++----- stack/middleware.go | 7 ++++ stack/recovery.go | 16 ++++++++ stack/state_space_test.go | 8 ++-- 22 files changed, 164 insertions(+), 39 deletions(-) diff --git a/docs/guide/counter/plugins/counter/counter.go b/docs/guide/counter/plugins/counter/counter.go index 6a17653d3..53ae4f9f5 100644 --- a/docs/guide/counter/plugins/counter/counter.go +++ b/docs/guide/counter/plugins/counter/counter.go @@ -114,7 +114,8 @@ func NewHandler(feeDenom string) basecoin.Handler { // Handler the counter transaction processing handler type Handler struct { - stack.NopOption + stack.PassInitState + stack.PassInitValidate } var _ stack.Dispatchable = Handler{} diff --git a/handler.go b/handler.go index a7e2b7334..c15d479fa 100644 --- a/handler.go +++ b/handler.go @@ -10,22 +10,27 @@ import ( // Handler is anything that processes a transaction type Handler interface { + // Checker verifies there are valid fees and estimates work Checker + // Deliver performs the tx once it makes it in the block Deliver - // This is for app options + // InitStater sets state from the genesis file InitStater + // InitValidater sets the initial validator set + InitValidater + // Named ensures there is a name for the item Named - // TODO: for staker - // InitChain(log log.Logger, store state.SimpleDB, vals []*abci.Validator) // TODO???? // BeginBlock(store state.SimpleDB, hash []byte, header *abci.Header) } +// Named ensures there is a name for the item type Named interface { Name() string } +// Checker verifies there are valid fees and estimates work type Checker interface { CheckTx(ctx Context, store state.SimpleDB, tx Tx) (CheckResult, error) } @@ -37,6 +42,7 @@ func (c CheckerFunc) CheckTx(ctx Context, store state.SimpleDB, tx Tx) (CheckRes return c(ctx, store, tx) } +// Deliver performs the tx once it makes it in the block type Deliver interface { DeliverTx(ctx Context, store state.SimpleDB, tx Tx) (DeliverResult, error) } @@ -48,6 +54,7 @@ func (c DeliverFunc) DeliverTx(ctx Context, store state.SimpleDB, tx Tx) (Delive return c(ctx, store, tx) } +// InitStater sets state from the genesis file type InitStater interface { InitState(l log.Logger, store state.SimpleDB, module, key, value string) (string, error) } @@ -59,6 +66,18 @@ func (c InitStateFunc) InitState(l log.Logger, store state.SimpleDB, module, key return c(l, store, module, key, value) } +// InitValidater sets the initial validator set +type InitValidater interface { + InitValidate(log log.Logger, store state.SimpleDB, vals []*abci.Validator) +} + +// InitValidateFunc (like http.HandlerFunc) is a shortcut for making wrapers +type InitValidateFunc func(log.Logger, state.SimpleDB, []*abci.Validator) + +func (c InitValidateFunc) InitValidate(l log.Logger, store state.SimpleDB, vals []*abci.Validator) { + c(l, store, vals) +} + //---------- results and some wrappers -------- type Dataer interface { @@ -119,8 +138,12 @@ type NopDeliver struct{} func (_ NopDeliver) DeliverTx(Context, state.SimpleDB, Tx) (r DeliverResult, e error) { return } -type NopOption struct{} +type NopInitState struct{} -func (_ NopOption) InitState(log.Logger, state.SimpleDB, string, string, string) (string, error) { +func (_ NopInitState) InitState(log.Logger, state.SimpleDB, string, string, string) (string, error) { return "", nil } + +type NopInitValidate struct{} + +func (_ NopInitValidate) InitValidate(log log.Logger, store state.SimpleDB, vals []*abci.Validator) {} diff --git a/modules/auth/signature.go b/modules/auth/signature.go index a10ebd5b9..01ee72bd4 100644 --- a/modules/auth/signature.go +++ b/modules/auth/signature.go @@ -17,7 +17,8 @@ const ( // Signatures parses out go-crypto signatures and adds permissions to the // context for use inside the application type Signatures struct { - stack.PassOption + stack.PassInitState + stack.PassInitValidate } // Name of the module - fulfills Middleware interface diff --git a/modules/base/chain.go b/modules/base/chain.go index 98391e14b..b2fa845c4 100644 --- a/modules/base/chain.go +++ b/modules/base/chain.go @@ -13,7 +13,8 @@ const ( // Chain enforces that this tx was bound to the named chain type Chain struct { - stack.PassOption + stack.PassInitState + stack.PassInitValidate } // Name of the module - fulfills Middleware interface diff --git a/modules/base/logger.go b/modules/base/logger.go index 581fcfb42..4d8d29135 100644 --- a/modules/base/logger.go +++ b/modules/base/logger.go @@ -3,6 +3,7 @@ package base import ( "time" + abci "github.com/tendermint/abci/types" "github.com/tendermint/tmlibs/log" "github.com/tendermint/basecoin" @@ -70,6 +71,15 @@ func (Logger) InitState(l log.Logger, store state.SimpleDB, module, key, value s return res, err } +// InitValidate logs time and result - fulfills Middlware interface +func (Logger) InitValidate(l log.Logger, store state.SimpleDB, vals []*abci.Validator, next basecoin.InitValidater) { + start := time.Now() + next.InitValidate(l, store, vals) + delta := time.Now().Sub(start) + l = l.With("duration", micros(delta)) + l.Info("InitValidate") +} + // micros returns how many microseconds passed in a call func micros(d time.Duration) int { return int(d.Seconds() * 1000000) diff --git a/modules/base/multiplexer.go b/modules/base/multiplexer.go index 4f2c4713a..f9eac8207 100644 --- a/modules/base/multiplexer.go +++ b/modules/base/multiplexer.go @@ -18,7 +18,7 @@ package base // // Multiplexer grabs a MultiTx and sends them sequentially down the line // type Multiplexer struct { -// stack.PassOption +// stack.PassInitState // } // // Name of the module - fulfills Middleware interface diff --git a/modules/coin/handler.go b/modules/coin/handler.go index dfaa2fa17..52fa820b0 100644 --- a/modules/coin/handler.go +++ b/modules/coin/handler.go @@ -16,7 +16,9 @@ import ( const NameCoin = "coin" // Handler includes an accountant -type Handler struct{} +type Handler struct { + stack.PassInitValidate +} var _ stack.Dispatchable = Handler{} diff --git a/modules/fee/handler.go b/modules/fee/handler.go index ab0e6fbb7..536131648 100644 --- a/modules/fee/handler.go +++ b/modules/fee/handler.go @@ -24,7 +24,8 @@ type SimpleFeeMiddleware struct { // all fees go here, which could be a dump (Bank) or something reachable // by other app logic Collector basecoin.Actor - stack.PassOption + stack.PassInitState + stack.PassInitValidate } var _ stack.Middleware = SimpleFeeMiddleware{} diff --git a/modules/ibc/handler.go b/modules/ibc/handler.go index 298f62780..b4df69c22 100644 --- a/modules/ibc/handler.go +++ b/modules/ibc/handler.go @@ -34,7 +34,9 @@ func AllowIBC(app string) basecoin.Actor { } // Handler updates the chain state or creates an ibc packet -type Handler struct{} +type Handler struct { + basecoin.NopInitValidate +} var _ basecoin.Handler = Handler{} diff --git a/modules/ibc/middleware.go b/modules/ibc/middleware.go index 191a53313..c81bcec48 100644 --- a/modules/ibc/middleware.go +++ b/modules/ibc/middleware.go @@ -9,7 +9,8 @@ import ( // Middleware allows us to verify the IBC proof on a packet and // and if valid, attach this permission to the wrapped packet type Middleware struct { - stack.PassOption + stack.PassInitState + stack.PassInitValidate } var _ stack.Middleware = Middleware{} diff --git a/modules/nonce/replaycheck.go b/modules/nonce/replaycheck.go index 1a125dea9..742cd348f 100644 --- a/modules/nonce/replaycheck.go +++ b/modules/nonce/replaycheck.go @@ -13,7 +13,8 @@ const ( // ReplayCheck uses the sequence to check for replay attacks type ReplayCheck struct { - stack.PassOption + stack.PassInitState + stack.PassInitValidate } // Name of the module - fulfills Middleware interface diff --git a/modules/roles/handler.go b/modules/roles/handler.go index 155ba056b..a3d4161bb 100644 --- a/modules/roles/handler.go +++ b/modules/roles/handler.go @@ -11,7 +11,8 @@ const NameRole = "role" // Handler allows us to create new roles type Handler struct { - basecoin.NopOption + basecoin.NopInitState + basecoin.NopInitValidate } var _ basecoin.Handler = Handler{} diff --git a/modules/roles/middleware.go b/modules/roles/middleware.go index c25382a62..229eb0a55 100644 --- a/modules/roles/middleware.go +++ b/modules/roles/middleware.go @@ -9,7 +9,8 @@ import ( // Middleware allows us to add a requested role as a permission // if the tx requests it and has sufficient authority type Middleware struct { - stack.PassOption + stack.PassInitState + stack.PassInitValidate } var _ stack.Middleware = Middleware{} diff --git a/stack/checkpoint.go b/stack/checkpoint.go index 66524ae54..e5426be8a 100644 --- a/stack/checkpoint.go +++ b/stack/checkpoint.go @@ -14,7 +14,8 @@ const ( type Checkpoint struct { OnCheck bool OnDeliver bool - PassOption + PassInitState + PassInitValidate } // Name of the module - fulfills Middleware interface diff --git a/stack/checkpoint_test.go b/stack/checkpoint_test.go index dd356e3f9..013990649 100644 --- a/stack/checkpoint_test.go +++ b/stack/checkpoint_test.go @@ -32,12 +32,12 @@ func makeState() state.SimpleDB { func TestCheckpointer(t *testing.T) { assert, require := assert.New(t), require.New(t) - good := writerHand{"foo", []byte{1, 2}, []byte("bar")} + good := writerHand{name: "foo", key: []byte{1, 2}, value: []byte("bar")} bad := FailHandler{Err: errors.New("no go")} app := New( Checkpoint{OnCheck: true}, - writerMid{"bing", []byte{1, 2}, []byte("bang")}, + writerMid{name: "bing", key: []byte{1, 2}, value: []byte("bang")}, Checkpoint{OnDeliver: true}, ).Use( NewDispatcher( diff --git a/stack/dispatcher.go b/stack/dispatcher.go index e4a34c0b7..54d67f11c 100644 --- a/stack/dispatcher.go +++ b/stack/dispatcher.go @@ -2,8 +2,10 @@ package stack import ( "fmt" + "sort" "strings" + abci "github.com/tendermint/abci/types" "github.com/tendermint/tmlibs/log" "github.com/tendermint/basecoin" @@ -119,6 +121,16 @@ func (d *Dispatcher) InitState(l log.Logger, store state.SimpleDB, module, key, return r.InitState(l, store, module, key, value, cb) } +// InitValidate makes sure all modules are informed +func (d *Dispatcher) InitValidate(log log.Logger, store state.SimpleDB, vals []*abci.Validator) { + for _, mod := range d.sortedModules() { + // no ctx, so secureCheck not needed + cb := d + space := stateSpace(store, mod.Name()) + mod.InitValidate(log, space, vals, cb) + } +} + func (d *Dispatcher) lookupTx(tx basecoin.Tx) (Dispatchable, error) { kind, err := tx.GetKind() if err != nil { @@ -140,3 +152,19 @@ func (d *Dispatcher) lookupModule(name string) (Dispatchable, error) { } return r, nil } + +func (d *Dispatcher) sortedModules() []Dispatchable { + // order all routes names + size := len(d.routes) + names := make([]string, 0, size) + for k := range d.routes { + names = append(names, k) + } + sort.Strings(names) + + res := make([]Dispatchable, size) + for i, k := range names { + res[i] = d.routes[k] + } + return res +} diff --git a/stack/helpers.go b/stack/helpers.go index e91086d35..ca3c7f586 100644 --- a/stack/helpers.go +++ b/stack/helpers.go @@ -94,7 +94,8 @@ func (r FailTx) ValidateBasic() error { // OKHandler just used to return okay to everything type OKHandler struct { Log string - basecoin.NopOption + basecoin.NopInitState + basecoin.NopInitValidate } var _ basecoin.Handler = OKHandler{} @@ -116,7 +117,8 @@ func (ok OKHandler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx bas // EchoHandler returns success, echoing res.Data = tx bytes type EchoHandler struct { - basecoin.NopOption + basecoin.NopInitState + basecoin.NopInitValidate } var _ basecoin.Handler = EchoHandler{} @@ -141,7 +143,8 @@ func (EchoHandler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx base // FailHandler always returns an error type FailHandler struct { Err error - basecoin.NopOption + basecoin.NopInitState + basecoin.NopInitValidate } var _ basecoin.Handler = FailHandler{} @@ -165,7 +168,8 @@ func (f FailHandler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx ba type PanicHandler struct { Msg string Err error - basecoin.NopOption + basecoin.NopInitState + basecoin.NopInitValidate } var _ basecoin.Handler = PanicHandler{} @@ -193,7 +197,8 @@ func (p PanicHandler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx b // CheckHandler accepts CheckTx and verifies the permissions type CheckHandler struct { - basecoin.NopOption + basecoin.NopInitState + basecoin.NopInitValidate } var _ basecoin.Handler = CheckHandler{} diff --git a/stack/helperware.go b/stack/helperware.go index 6fd2078a3..003b1156b 100644 --- a/stack/helperware.go +++ b/stack/helperware.go @@ -16,7 +16,8 @@ const ( // Required Actor, otherwise passes along the call untouched type CheckMiddleware struct { Required basecoin.Actor - PassOption + PassInitState + PassInitValidate } var _ Middleware = CheckMiddleware{} @@ -42,7 +43,8 @@ func (p CheckMiddleware) DeliverTx(ctx basecoin.Context, store state.SimpleDB, t // GrantMiddleware tries to set the permission to this Actor, which may be prohibited type GrantMiddleware struct { Auth basecoin.Actor - PassOption + PassInitState + PassInitValidate } var _ Middleware = GrantMiddleware{} diff --git a/stack/interface.go b/stack/interface.go index 173e61d14..35fe35e8d 100644 --- a/stack/interface.go +++ b/stack/interface.go @@ -2,6 +2,7 @@ package stack import ( + abci "github.com/tendermint/abci/types" "github.com/tendermint/tmlibs/log" "github.com/tendermint/basecoin" @@ -15,7 +16,8 @@ import ( type Middleware interface { CheckerMiddle DeliverMiddle - InitStateMiddle + InitStaterMiddle + InitValidaterMiddle basecoin.Named } @@ -45,19 +47,31 @@ func (d DeliverMiddleFunc) DeliverTx(ctx basecoin.Context, store state.SimpleDB, return d(ctx, store, tx, next) } -type InitStateMiddle interface { +type InitStaterMiddle interface { InitState(l log.Logger, store state.SimpleDB, module, key, value string, next basecoin.InitStater) (string, error) } -type InitStateMiddleFunc func(log.Logger, state.SimpleDB, +type InitStaterMiddleFunc func(log.Logger, state.SimpleDB, string, string, string, basecoin.InitStater) (string, error) -func (c InitStateMiddleFunc) InitState(l log.Logger, store state.SimpleDB, +func (c InitStaterMiddleFunc) InitState(l log.Logger, store state.SimpleDB, module, key, value string, next basecoin.InitStater) (string, error) { return c(l, store, module, key, value, next) } +type InitValidaterMiddle interface { + InitValidate(l log.Logger, store state.SimpleDB, vals []*abci.Validator, next basecoin.InitValidater) +} + +type InitValidaterMiddleFunc func(log.Logger, state.SimpleDB, + []*abci.Validator, basecoin.InitValidater) + +func (c InitValidaterMiddleFunc) InitValidate(l log.Logger, store state.SimpleDB, + vals []*abci.Validator, next basecoin.InitValidater) { + c(l, store, vals, next) +} + // holders type PassCheck struct{} @@ -73,18 +87,18 @@ func (_ PassDeliver) DeliverTx(ctx basecoin.Context, store state.SimpleDB, return next.DeliverTx(ctx, store, tx) } -type PassOption struct{} +type PassInitState struct{} -func (_ PassOption) InitState(l log.Logger, store state.SimpleDB, module, +func (_ PassInitState) InitState(l log.Logger, store state.SimpleDB, module, key, value string, next basecoin.InitStater) (string, error) { return next.InitState(l, store, module, key, value) } -type NopOption struct{} +type PassInitValidate struct{} -func (_ NopOption) InitState(l log.Logger, store state.SimpleDB, module, - key, value string, next basecoin.InitStater) (string, error) { - return "", nil +func (_ PassInitValidate) InitValidate(l log.Logger, store state.SimpleDB, + vals []*abci.Validator, next basecoin.InitValidater) { + next.InitValidate(l, store, vals) } // Dispatchable is like middleware, except the meaning of "next" is different. @@ -126,3 +140,8 @@ func (w wrapped) InitState(l log.Logger, store state.SimpleDB, module, key, value string, _ basecoin.InitStater) (string, error) { return w.h.InitState(l, store, module, key, value) } + +func (w wrapped) InitValidate(l log.Logger, store state.SimpleDB, + vals []*abci.Validator, next basecoin.InitValidater) { + w.h.InitValidate(l, store, vals) +} diff --git a/stack/middleware.go b/stack/middleware.go index 4ba29a202..ebd2a4bba 100644 --- a/stack/middleware.go +++ b/stack/middleware.go @@ -1,6 +1,7 @@ package stack import ( + abci "github.com/tendermint/abci/types" "github.com/tendermint/tmlibs/log" "github.com/tendermint/basecoin" @@ -59,6 +60,12 @@ func (m *middleware) InitState(l log.Logger, store state.SimpleDB, module, key, return m.middleware.InitState(l, store, module, key, value, m.next) } +func (m *middleware) InitValidate(l log.Logger, store state.SimpleDB, vals []*abci.Validator) { + // set the namespace for the app + store = stateSpace(store, m.space) + m.middleware.InitValidate(l, store, vals, m.next) +} + // builder is used to associate info with the middleware, so we can build // it properly type builder struct { diff --git a/stack/recovery.go b/stack/recovery.go index e5b986ef2..cbf9d2d49 100644 --- a/stack/recovery.go +++ b/stack/recovery.go @@ -3,6 +3,7 @@ package stack import ( "fmt" + abci "github.com/tendermint/abci/types" "github.com/tendermint/tmlibs/log" "github.com/tendermint/basecoin" @@ -55,6 +56,21 @@ func (Recovery) InitState(l log.Logger, store state.SimpleDB, module, key, value return next.InitState(l, store, module, key, value) } +// InitValidate catches any panic and logs it +// TODO: return an error??? +func (Recovery) InitValidate(l log.Logger, store state.SimpleDB, + vals []*abci.Validator, next basecoin.InitValidater) { + + defer func() { + if r := recover(); r != nil { + // TODO: return an error??? + err := normalizePanic(r) + l.With("err", err).Error(err.Error()) + } + }() + next.InitValidate(l, store, vals) +} + // normalizePanic makes sure we can get a nice TMError (with stack) out of it func normalizePanic(p interface{}) error { if err, isErr := p.(error); isErr { diff --git a/stack/state_space_test.go b/stack/state_space_test.go index b18671106..8f9d302e3 100644 --- a/stack/state_space_test.go +++ b/stack/state_space_test.go @@ -17,6 +17,7 @@ import ( type writerMid struct { name string key, value []byte + PassInitValidate } var _ Middleware = writerMid{} @@ -41,10 +42,11 @@ func (w writerMid) InitState(l log.Logger, store state.SimpleDB, module, return next.InitState(l, store, module, key, value) } -// writerHand is a middleware that writes the given bytes on CheckTx and DeliverTx +// writerHand is a handler that writes the given bytes on CheckTx and DeliverTx type writerHand struct { name string key, value []byte + basecoin.NopInitValidate } var _ basecoin.Handler = writerHand{} @@ -76,9 +78,9 @@ func TestStateSpace(t *testing.T) { expected []data.Bytes }{ { - writerHand{"foo", []byte{1, 2}, []byte("bar")}, + writerHand{name: "foo", key: []byte{1, 2}, value: []byte("bar")}, []Middleware{ - writerMid{"bing", []byte{1, 2}, []byte("bang")}, + writerMid{name: "bing", key: []byte{1, 2}, value: []byte("bang")}, }, []data.Bytes{ {'f', 'o', 'o', 0, 1, 2}, From a46cb62272e661b52345a0972d8deeb1748cb705 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sun, 30 Jul 2017 17:34:33 -0400 Subject: [PATCH 06/13] Returns all validator changes from DeliverTx in EndBlock --- TODO.md | 3 +-- app/app.go | 16 ++++++++++------ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/TODO.md b/TODO.md index 2eb6af870..e9d2f61ea 100644 --- a/TODO.md +++ b/TODO.md @@ -4,5 +4,4 @@ * FeeTx and CheckTx changes logic to estimate, not validate * Add tests for new CheckTx -* Handle ValidatorSet responses from DeliverTx calls in EndBlock -* Add InitValidator call to all modules +* Test EndBlock validator set changes diff --git a/app/app.go b/app/app.go index d05459297..f0852b49b 100644 --- a/app/app.go +++ b/app/app.go @@ -23,11 +23,12 @@ const ( // Basecoin - The ABCI application type Basecoin struct { - info *sm.ChainState - + info *sm.ChainState state *Store handler basecoin.Handler + + pending []*abci.Validator height uint64 logger log.Logger } @@ -109,6 +110,9 @@ func (app *Basecoin) DeliverTx(txBytes []byte) abci.Result { if err != nil { return errors.Result(err) } + if len(res.Diff) > 0 { + app.pending = append(app.pending, res.Diff...) + } return res.ToABCI() } @@ -169,11 +173,11 @@ func (app *Basecoin) BeginBlock(hash []byte, header *abci.Header) { } // EndBlock - ABCI +// Returns a list of all validator changes made in this block func (app *Basecoin) EndBlock(height uint64) (res abci.ResponseEndBlock) { - // for _, plugin := range app.plugins.GetList() { - // pluginRes := plugin.EndBlock(app.state, height) - // res.Diffs = append(res.Diffs, pluginRes.Diffs...) - // } + // TODO: cleanup in case a validator exists multiple times in the list + res.Diffs = app.pending + app.pending = nil return } From ff658f032691316d6cf2b574e01eb7bd5472b3d9 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Sun, 30 Jul 2017 17:51:57 -0400 Subject: [PATCH 07/13] Reenable multiplexer --- TODO.md | 3 +- handler.go | 8 +- modules/base/multiplexer.go | 165 +++++++++++++++++++++++------------- 3 files changed, 110 insertions(+), 66 deletions(-) diff --git a/TODO.md b/TODO.md index e9d2f61ea..bff7c68fd 100644 --- a/TODO.md +++ b/TODO.md @@ -1,7 +1,6 @@ # TODO for rewrite -* Reimplement MultiTx in base - * FeeTx and CheckTx changes logic to estimate, not validate * Add tests for new CheckTx * Test EndBlock validator set changes +* Test Multiplexer diff --git a/handler.go b/handler.go index c15d479fa..0e4e16c8a 100644 --- a/handler.go +++ b/handler.go @@ -87,10 +87,12 @@ type Dataer interface { // CheckResult captures any non-error abci result // to make sure people use error for error cases type CheckResult struct { - Data data.Bytes - Log string + Data data.Bytes + Log string + // GasAllocated is the maximum units of work we allow this tx to perform GasAllocated uint - GasPrice uint + // GasPayment is the total fees for this tx (or other source of payment) + GasPayment uint } var _ Dataer = CheckResult{} diff --git a/modules/base/multiplexer.go b/modules/base/multiplexer.go index f9eac8207..408812901 100644 --- a/modules/base/multiplexer.go +++ b/modules/base/multiplexer.go @@ -1,73 +1,116 @@ package base -// import ( -// "strings" +import ( + "strings" -// wire "github.com/tendermint/go-wire" -// "github.com/tendermint/go-wire/data" + abci "github.com/tendermint/abci/types" + wire "github.com/tendermint/go-wire" + "github.com/tendermint/go-wire/data" -// "github.com/tendermint/basecoin" -// "github.com/tendermint/basecoin/stack" -// "github.com/tendermint/basecoin/state" -// ) + "github.com/tendermint/basecoin" + "github.com/tendermint/basecoin/stack" + "github.com/tendermint/basecoin/state" +) -// //nolint -// const ( -// NameMultiplexer = "mplx" -// ) +//nolint +const ( + NameMultiplexer = "mplx" +) -// // Multiplexer grabs a MultiTx and sends them sequentially down the line -// type Multiplexer struct { -// stack.PassInitState -// } +// Multiplexer grabs a MultiTx and sends them sequentially down the line +type Multiplexer struct { + stack.PassInitState + stack.PassInitValidate +} -// // Name of the module - fulfills Middleware interface -// func (Multiplexer) Name() string { -// return NameMultiplexer -// } +// Name of the module - fulfills Middleware interface +func (Multiplexer) Name() string { + return NameMultiplexer +} -// var _ stack.Middleware = Multiplexer{} +var _ stack.Middleware = Multiplexer{} -// // CheckTx splits the input tx and checks them all - fulfills Middlware interface -// func (Multiplexer) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Checker) (res basecoin.CheckResult, err error) { -// if mtx, ok := tx.Unwrap().(*MultiTx); ok { -// return runAll(ctx, store, mtx.Txs, next.CheckTx) -// } -// return next.CheckTx(ctx, store, tx) -// } +// CheckTx splits the input tx and checks them all - fulfills Middlware interface +func (Multiplexer) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Checker) (res basecoin.CheckResult, err error) { + if mtx, ok := tx.Unwrap().(*MultiTx); ok { + return runAllChecks(ctx, store, mtx.Txs, next) + } + return next.CheckTx(ctx, store, tx) +} -// // DeliverTx splits the input tx and checks them all - fulfills Middlware interface -// func (Multiplexer) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.DeliverResult, err error) { -// if mtx, ok := tx.Unwrap().(*MultiTx); ok { -// return runAll(ctx, store, mtx.Txs, next.DeliverTx) -// } -// return next.DeliverTx(ctx, store, tx) -// } +// DeliverTx splits the input tx and checks them all - fulfills Middlware interface +func (Multiplexer) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.DeliverResult, err error) { + if mtx, ok := tx.Unwrap().(*MultiTx); ok { + return runAllDelivers(ctx, store, mtx.Txs, next) + } + return next.DeliverTx(ctx, store, tx) +} -// func runAll(ctx basecoin.Context, store state.SimpleDB, txs []basecoin.Tx, next basecoin.CheckerFunc) (res basecoin.Result, err error) { -// // store all results, unless anything errors -// rs := make([]basecoin.Result, len(txs)) -// for i, stx := range txs { -// rs[i], err = next(ctx, store, stx) -// if err != nil { -// return -// } -// } -// // now combine the results into one... -// return combine(rs), nil -// } +func runAllChecks(ctx basecoin.Context, store state.SimpleDB, txs []basecoin.Tx, next basecoin.Checker) (res basecoin.CheckResult, err error) { + // store all results, unless anything errors + rs := make([]basecoin.CheckResult, len(txs)) + for i, stx := range txs { + rs[i], err = next.CheckTx(ctx, store, stx) + if err != nil { + return + } + } + // now combine the results into one... + return combineChecks(rs), nil +} -// // combines all data bytes as a go-wire array. -// // joins all log messages with \n -// func combine(all []basecoin.Result) basecoin.Result { -// datas := make([]data.Bytes, len(all)) -// logs := make([]string, len(all)) -// for i, r := range all { -// datas[i] = r.Data -// logs[i] = r.Log -// } -// return basecoin.Result{ -// Data: wire.BinaryBytes(datas), -// Log: strings.Join(logs, "\n"), -// } -// } +func runAllDelivers(ctx basecoin.Context, store state.SimpleDB, txs []basecoin.Tx, next basecoin.Deliver) (res basecoin.DeliverResult, err error) { + // store all results, unless anything errors + rs := make([]basecoin.DeliverResult, len(txs)) + for i, stx := range txs { + rs[i], err = next.DeliverTx(ctx, store, stx) + if err != nil { + return + } + } + // now combine the results into one... + return combineDelivers(rs), nil +} + +// combines all data bytes as a go-wire array. +// joins all log messages with \n +func combineChecks(all []basecoin.CheckResult) basecoin.CheckResult { + datas := make([]data.Bytes, len(all)) + logs := make([]string, len(all)) + var allocated, payments uint + for i, r := range all { + datas[i] = r.Data + logs[i] = r.Log + allocated += r.GasAllocated + payments += r.GasPayment + } + return basecoin.CheckResult{ + Data: wire.BinaryBytes(datas), + Log: strings.Join(logs, "\n"), + GasAllocated: allocated, + GasPayment: payments, + } +} + +// combines all data bytes as a go-wire array. +// joins all log messages with \n +func combineDelivers(all []basecoin.DeliverResult) basecoin.DeliverResult { + datas := make([]data.Bytes, len(all)) + logs := make([]string, len(all)) + var used uint + var diffs []*abci.Validator + for i, r := range all { + datas[i] = r.Data + logs[i] = r.Log + used += r.GasUsed + if len(r.Diff) > 0 { + diffs = append(diffs, r.Diff...) + } + } + return basecoin.DeliverResult{ + Data: wire.BinaryBytes(datas), + Log: strings.Join(logs, "\n"), + GasUsed: used, + Diff: diffs, + } +} From 70fe2444ab1c3aab77e7de977e3a191c70d1fe27 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Thu, 3 Aug 2017 19:00:03 +0200 Subject: [PATCH 08/13] Deduplicate validator changes in EndBlock and test this --- app/app.go | 26 ++++++++- app/app_val_test.go | 137 ++++++++++++++++++++++++++++++++++++++++++++ app/store.go | 10 ++++ 3 files changed, 170 insertions(+), 3 deletions(-) create mode 100644 app/app_val_test.go diff --git a/app/app.go b/app/app.go index f0852b49b..117b155e8 100644 --- a/app/app.go +++ b/app/app.go @@ -1,6 +1,7 @@ package app import ( + "bytes" "fmt" "strings" @@ -110,9 +111,7 @@ func (app *Basecoin) DeliverTx(txBytes []byte) abci.Result { if err != nil { return errors.Result(err) } - if len(res.Diff) > 0 { - app.pending = append(app.pending, res.Diff...) - } + app.addValChange(res.Diff) return res.ToABCI() } @@ -181,6 +180,27 @@ func (app *Basecoin) EndBlock(height uint64) (res abci.ResponseEndBlock) { return } +func (app *Basecoin) addValChange(diffs []*abci.Validator) { + for _, d := range diffs { + idx := findVal(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 +func findVal(val *abci.Validator, list []*abci.Validator) int { + for i, v := range list { + if bytes.Equal(val.PubKey, v.PubKey) { + return i + } + } + return -1 +} + //TODO move split key to tmlibs? // Splits the string at the first '/'. diff --git a/app/app_val_test.go b/app/app_val_test.go new file mode 100644 index 000000000..cf2cdbfe0 --- /dev/null +++ b/app/app_val_test.go @@ -0,0 +1,137 @@ +package app + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + abci "github.com/tendermint/abci/types" + wire "github.com/tendermint/go-wire" + cmn "github.com/tendermint/tmlibs/common" + "github.com/tendermint/tmlibs/log" + + "github.com/tendermint/basecoin" + "github.com/tendermint/basecoin/errors" + "github.com/tendermint/basecoin/state" +) + +//-------------------------------- +// Setup tx and handler for validation test cases + +const ( + ValName = "val" + TypeValChange = ValName + "/change" + ByteValChange = 0xfe +) + +func init() { + basecoin.TxMapper.RegisterImplementation(ValChangeTx{}, TypeValChange, ByteValChange) +} + +type ValSetHandler struct { + basecoin.NopCheck + basecoin.NopInitState + basecoin.NopInitValidate +} + +var _ basecoin.Handler = ValSetHandler{} + +func (ValSetHandler) Name() string { + return ValName +} + +func (ValSetHandler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, + tx basecoin.Tx) (res basecoin.DeliverResult, err error) { + change, ok := tx.Unwrap().(ValChangeTx) + if !ok { + return res, errors.ErrUnknownTxType(tx) + } + res.Diff = change.Diff + return +} + +type ValChangeTx struct { + Diff []*abci.Validator +} + +func (v ValChangeTx) Wrap() basecoin.Tx { + return basecoin.Tx{v} +} + +func (v ValChangeTx) ValidateBasic() error { return nil } + +//----------------------------------- +// Test cases start here + +func power() uint64 { + // % can return negative numbers, so this ensures result is positive + return uint64(cmn.RandInt()%50 + 60) +} + +func makeVal() *abci.Validator { + return &abci.Validator{ + PubKey: cmn.RandBytes(10), + Power: power(), + } +} + +// newPower returns a copy of the validator with a different power +func newPower(val *abci.Validator) *abci.Validator { + res := *val + res.Power = power() + if res.Power == val.Power { + panic("no no") + } + return &res +} + +func TestEndBlock(t *testing.T) { + assert, require := assert.New(t), require.New(t) + + logger := log.NewNopLogger() + store := MockStore() + handler := ValSetHandler{} + app := NewBasecoin(handler, store, logger) + + val1 := makeVal() + val2 := makeVal() + val3 := makeVal() + val1a := newPower(val1) + val2a := newPower(val2) + + cases := [...]struct { + changes [][]*abci.Validator + expected []*abci.Validator + }{ + // Nothing in, nothing out, no crash + 0: {}, + // One in, one out, no problem + 1: { + changes: [][]*abci.Validator{{val1}}, + expected: []*abci.Validator{val1}, + }, + // Combine a few ones + 2: { + changes: [][]*abci.Validator{{val1}, {val2, val3}}, + expected: []*abci.Validator{val1, val2, val3}, + }, + // Make sure changes all to one validators are squished into one diff + 3: { + changes: [][]*abci.Validator{{val1}, {val2, val1a}, {val2a, val3}}, + expected: []*abci.Validator{val1a, val2a, val3}, + }, + } + + for i, tc := range cases { + app.BeginBlock(nil, nil) + for _, c := range tc.changes { + tx := ValChangeTx{c}.Wrap() + txBytes := wire.BinaryBytes(tx) + res := app.DeliverTx(txBytes) + require.True(res.IsOK(), "%#v", res) + } + diff := app.EndBlock(app.height) + // TODO: don't care about order here... + assert.Equal(tc.expected, diff.Diffs, "%d", i) + } +} diff --git a/app/store.go b/app/store.go index adf512806..329b63516 100644 --- a/app/store.go +++ b/app/store.go @@ -36,6 +36,16 @@ type ChainState struct { Height uint64 } +// MockStore returns an in-memory store only intended for testing +func MockStore() *Store { + res, err := NewStore("", 0, log.NewNopLogger()) + if err != nil { + // should never happen, abort test if it does + panic(err) + } + return res +} + // NewStore initializes an in-memory IAVLTree, or attempts to load a persistant // tree from disk func NewStore(dbName string, cacheSize int, logger log.Logger) (*Store, error) { From 1715c0aeba09d156714cc583fd7502fcad914833 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Thu, 3 Aug 2017 19:27:06 +0200 Subject: [PATCH 09/13] Handlers and middlewares add gas prices --- TODO.md | 26 ++++++++++++++++++++++++-- handler.go | 9 +++++++++ modules/coin/handler.go | 17 +++++++++++++---- modules/fee/handler.go | 13 +++++++++++-- modules/nonce/replaycheck.go | 5 ++++- modules/roles/handler.go | 11 +++++++++-- modules/roles/middleware.go | 5 ++++- 7 files changed, 74 insertions(+), 12 deletions(-) diff --git a/TODO.md b/TODO.md index bff7c68fd..eb4a4ab39 100644 --- a/TODO.md +++ b/TODO.md @@ -1,6 +1,28 @@ # TODO for rewrite -* FeeTx and CheckTx changes logic to estimate, not validate * Add tests for new CheckTx -* Test EndBlock validator set changes * Test Multiplexer + + +Alexis: + +* merkle - proof (non-existence - maybe range) +* intro to light-client and proofs +light-client proofs: +* make this sensible -> very tied to merkle proofs and API +* support new proof types + +* abci add range suppprt + + + +* merkle - api cleanup (also Bonsai) +* later: C bindings (to Bonsai?) + + +* crypto-ledger (while ethan gone) + +light-client provider: +* caching checkpoint on Verify +* cleanup (trim old node) + diff --git a/handler.go b/handler.go index 0e4e16c8a..32689b3a4 100644 --- a/handler.go +++ b/handler.go @@ -95,6 +95,15 @@ type CheckResult struct { GasPayment uint } +// NewCheck sets the gas used and the response data but no more info +// these are the most common info needed to be set by the Handler +func NewCheck(gasAllocated uint, log string) CheckResult { + return CheckResult{ + GasAllocated: gasAllocated, + Log: log, + } +} + var _ Dataer = CheckResult{} func (r CheckResult) ToABCI() abci.Result { diff --git a/modules/coin/handler.go b/modules/coin/handler.go index 52fa820b0..c02460cbd 100644 --- a/modules/coin/handler.go +++ b/modules/coin/handler.go @@ -12,8 +12,14 @@ import ( "github.com/tendermint/basecoin/state" ) -//NameCoin - name space of the coin module -const NameCoin = "coin" +const ( + //NameCoin - name space of the coin module + NameCoin = "coin" + // CostSend is GasAllocation per input/output + CostSend = 10 + // CostCredit is GasAllocation of a credit allocation + CostCredit = 20 +) // Handler includes an accountant type Handler struct { @@ -46,9 +52,12 @@ func (h Handler) CheckTx(ctx basecoin.Context, store state.SimpleDB, switch t := tx.Unwrap().(type) { case SendTx: - return res, h.checkSendTx(ctx, store, t) + // price based on inputs and outputs + used := uint(len(t.Inputs) + len(t.Outputs)) + return basecoin.NewCheck(used*CostSend, ""), h.checkSendTx(ctx, store, t) case CreditTx: - return res, h.creditTx(ctx, store, t) + // default price of 20, constant work + return basecoin.NewCheck(CostCredit, ""), h.creditTx(ctx, store, t) } return res, errors.ErrUnknownTxType(tx.Unwrap()) } diff --git a/modules/fee/handler.go b/modules/fee/handler.go index 536131648..d1eca64bb 100644 --- a/modules/fee/handler.go +++ b/modules/fee/handler.go @@ -55,15 +55,24 @@ func (h SimpleFeeMiddleware) CheckTx(ctx basecoin.Context, store state.SimpleDB, return res, err } + var paid, used uint if !fee.Fee.IsZero() { // now, try to make a IPC call to coins... send := coin.NewSendOneTx(fee.Payer, h.Collector, coin.Coins{fee.Fee}) - _, err = next.CheckTx(ctx, store, send) + sendRes, err := next.CheckTx(ctx, store, send) if err != nil { return res, err } + paid = uint(fee.Fee.Amount) + used = sendRes.GasAllocated } - return next.CheckTx(ctx, store, fee.Tx) + res, err = next.CheckTx(ctx, store, fee.Tx) + // add the given fee to the price for gas, plus one query + if err == nil { + res.GasPayment += paid + res.GasAllocated += used + } + return res, err } // DeliverTx - send the fee handler transaction diff --git a/modules/nonce/replaycheck.go b/modules/nonce/replaycheck.go index 742cd348f..2d532a1c2 100644 --- a/modules/nonce/replaycheck.go +++ b/modules/nonce/replaycheck.go @@ -9,6 +9,7 @@ import ( //nolint const ( NameNonce = "nonce" + CostNonce = 10 ) // ReplayCheck uses the sequence to check for replay attacks @@ -33,7 +34,9 @@ func (r ReplayCheck) CheckTx(ctx basecoin.Context, store state.SimpleDB, return res, err } - return next.CheckTx(ctx, store, stx) + res, err = next.CheckTx(ctx, store, stx) + res.GasAllocated += CostNonce + return res, err } // DeliverTx verifies tx is not being replayed - fulfills Middlware interface diff --git a/modules/roles/handler.go b/modules/roles/handler.go index a3d4161bb..e7073d034 100644 --- a/modules/roles/handler.go +++ b/modules/roles/handler.go @@ -6,8 +6,14 @@ import ( "github.com/tendermint/basecoin/state" ) -//NameRole - name space of the roles module -const NameRole = "role" +const ( + //NameRole - name space of the roles module + NameRole = "role" + // CostCreate is the cost to create a new role + CostCreate = 40 + // CostAssume is the cost to assume a role as part of a tx + CostAssume = 5 +) // Handler allows us to create new roles type Handler struct { @@ -34,6 +40,7 @@ func (h Handler) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin if err != nil { return } + res = basecoin.NewCheck(CostCreate, "") err = checkNoRole(store, cr.Role) return } diff --git a/modules/roles/middleware.go b/modules/roles/middleware.go index 229eb0a55..6266db8e4 100644 --- a/modules/roles/middleware.go +++ b/modules/roles/middleware.go @@ -41,7 +41,10 @@ func (m Middleware) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basec } // one could add multiple role statements, repeat as needed - return m.CheckTx(ctx, store, assume.Tx, next) + // charging for each level + res, err = m.CheckTx(ctx, store, assume.Tx, next) + res.GasAllocated += CostAssume + return } // DeliverTx tries to assume the named role if requested. From 74070f1cacb95ccbc96d47249a0be025b6b038f3 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Thu, 3 Aug 2017 19:43:37 +0200 Subject: [PATCH 10/13] Add module tests for checktx return values --- modules/coin/handler.go | 4 ++-- modules/coin/handler_test.go | 31 ++++++++++++++++++++++++++++--- modules/fee/handler_test.go | 32 +++++++++++++++++++------------- modules/roles/handler.go | 4 ++-- modules/roles/handler_test.go | 4 +++- modules/roles/middleware_test.go | 5 ++++- 6 files changed, 58 insertions(+), 22 deletions(-) diff --git a/modules/coin/handler.go b/modules/coin/handler.go index c02460cbd..0d1010803 100644 --- a/modules/coin/handler.go +++ b/modules/coin/handler.go @@ -16,9 +16,9 @@ const ( //NameCoin - name space of the coin module NameCoin = "coin" // CostSend is GasAllocation per input/output - CostSend = 10 + CostSend = uint(10) // CostCredit is GasAllocation of a credit allocation - CostCredit = 20 + CostCredit = uint(20) ) // Handler includes an accountant diff --git a/modules/coin/handler_test.go b/modules/coin/handler_test.go index d43e20fa4..502f1d71a 100644 --- a/modules/coin/handler_test.go +++ b/modules/coin/handler_test.go @@ -85,7 +85,7 @@ func TestHandlerValidation(t *testing.T) { } } -func TestDeliverSendTx(t *testing.T) { +func TestCheckDeliverSendTx(t *testing.T) { assert := assert.New(t) require := require.New(t) @@ -110,6 +110,7 @@ func TestDeliverSendTx(t *testing.T) { tx basecoin.Tx perms []basecoin.Actor final []money // nil for error + cost uint // gas allocated (if not error) }{ { []money{{addr1, moreCoins}}, @@ -118,6 +119,7 @@ func TestDeliverSendTx(t *testing.T) { []TxOutput{NewTxOutput(addr2, someCoins)}), []basecoin.Actor{addr1}, []money{{addr1, diffCoins}, {addr2, someCoins}}, + 20, }, // simple multi-sig 2 accounts to 1 { @@ -127,6 +129,7 @@ func TestDeliverSendTx(t *testing.T) { []TxOutput{NewTxOutput(addr3, mixedCoins)}), []basecoin.Actor{addr1, addr2}, []money{{addr1, someCoins}, {addr2, diffCoins}, {addr3, mixedCoins}}, + 30, }, // multi-sig with one account sending many times { @@ -136,6 +139,17 @@ func TestDeliverSendTx(t *testing.T) { []TxOutput{NewTxOutput(addr2, mixedCoins)}), []basecoin.Actor{addr1}, []money{{addr1, diffCoins}, {addr2, mixedCoins}}, + 30, + }, + // invalid send (not enough money ) + { + []money{{addr1, moreCoins}, {addr2, someCoins}}, + NewSendTx( + []TxInput{NewTxInput(addr2, moreCoins)}, + []TxOutput{NewTxOutput(addr1, moreCoins)}), + []basecoin.Actor{addr1, addr2}, + nil, + 0, }, } @@ -150,9 +164,19 @@ func TestDeliverSendTx(t *testing.T) { } ctx := stack.MockContext("base-chain", 100).WithPermissions(tc.perms...) - _, err := h.DeliverTx(ctx, store, tc.tx, nil) + + // throw-away state for checktx + cache := store.Checkpoint() + cres, err := h.CheckTx(ctx, cache, tc.tx, nil) + // real store for delivertx + _, err2 := h.DeliverTx(ctx, store, tc.tx, nil) + if len(tc.final) > 0 { // valid assert.Nil(err, "%d: %+v", i, err) + assert.Nil(err2, "%d: %+v", i, err2) + // make sure proper gas is set + assert.Equal(uint(0), cres.GasPayment, "%d", i) + assert.Equal(tc.cost, cres.GasAllocated, "%d", i) // make sure the final balances are correct for _, f := range tc.final { acct, err := loadAccount(store, f.addr.Bytes()) @@ -160,8 +184,9 @@ func TestDeliverSendTx(t *testing.T) { assert.Equal(f.coins, acct.Coins) } } else { + // both check and deliver should fail assert.NotNil(err, "%d", i) - // TODO: make sure balances unchanged! + assert.NotNil(err2, "%d", i) } } diff --git a/modules/fee/handler_test.go b/modules/fee/handler_test.go index 3fb72abaa..546ebba2f 100644 --- a/modules/fee/handler_test.go +++ b/modules/fee/handler_test.go @@ -55,6 +55,8 @@ func TestFeeChecks(t *testing.T) { _, err = app2.InitState(l, store, "coin", "account", key2.MakeOption()) require.Nil(err, "%+v", err) + // feeCost is what we expect if the fee is properly paid + feeCost := coin.CostSend * 2 cases := []struct { valid bool // this is the middleware stack to test @@ -68,30 +70,32 @@ func TestFeeChecks(t *testing.T) { // expected balance after the tx left coin.Coins collected coin.Coins + // expected gas allocated + expectedCost uint }{ // make sure it works with no fee (control group) - {true, app1, act1, false, act1, zero, mixed, nil}, - {true, app1, act2, false, act2, zero, pure, nil}, + {true, app1, act1, false, act1, zero, mixed, nil, 0}, + {true, app1, act2, false, act2, zero, pure, nil, 0}, // no fee or too low is rejected - {false, app2, act1, false, act1, zero, mixed, nil}, - {false, app2, act2, false, act2, zero, pure, nil}, - {false, app2, act1, true, act1, zero, mixed, nil}, - {false, app2, act2, true, act2, atom(1), pure, nil}, + {false, app2, act1, false, act1, zero, mixed, nil, 0}, + {false, app2, act2, false, act2, zero, pure, nil, 0}, + {false, app2, act1, true, act1, zero, mixed, nil, 0}, + {false, app2, act2, true, act2, atom(1), pure, nil, 0}, // proper fees are transfered in both cases - {true, app1, act1, true, act1, atom(1), wallet(1199, 55), atoms(1)}, - {true, app2, act2, true, act2, atom(57), atoms(46600), atoms(58)}, + {true, app1, act1, true, act1, atom(1), wallet(1199, 55), atoms(1), feeCost}, + {true, app2, act2, true, act2, atom(57), atoms(46600), atoms(58), feeCost}, // // fee must be the proper type... - {false, app1, act1, true, act1, eth(5), wallet(1199, 55), atoms(58)}, + {false, app1, act1, true, act1, eth(5), wallet(1199, 55), atoms(58), 0}, // signature must match - {false, app2, act1, true, act2, atom(5), atoms(46600), atoms(58)}, + {false, app2, act1, true, act2, atom(5), atoms(46600), atoms(58), 0}, // send only works within limits - {true, app2, act1, true, act1, atom(1100), wallet(99, 55), atoms(1158)}, - {false, app2, act1, true, act1, atom(500), wallet(99, 55), atoms(1158)}, + {true, app2, act1, true, act1, atom(1100), wallet(99, 55), atoms(1158), feeCost}, + {false, app2, act1, true, act1, atom(500), wallet(99, 55), atoms(1158), 0}, } for i, tc := range cases { @@ -105,9 +109,11 @@ func TestFeeChecks(t *testing.T) { ctx := stack.MockContext("x", 1).WithPermissions(tc.signer) // call checktx... - _, err := tc.app.CheckTx(ctx, store, tx) + cres, err := tc.app.CheckTx(ctx, store, tx) if tc.valid { assert.Nil(err, "%d: %+v", i, err) + assert.EqualValues(tc.fee.Amount, cres.GasPayment) + assert.EqualValues(tc.expectedCost, cres.GasAllocated) } else { assert.NotNil(err, "%d", i) } diff --git a/modules/roles/handler.go b/modules/roles/handler.go index e7073d034..bca582fba 100644 --- a/modules/roles/handler.go +++ b/modules/roles/handler.go @@ -10,9 +10,9 @@ const ( //NameRole - name space of the roles module NameRole = "role" // CostCreate is the cost to create a new role - CostCreate = 40 + CostCreate = uint(40) // CostAssume is the cost to assume a role as part of a tx - CostAssume = 5 + CostAssume = uint(5) ) // Handler allows us to create new roles diff --git a/modules/roles/handler_test.go b/modules/roles/handler_test.go index 6d1c5dfa6..1d6859259 100644 --- a/modules/roles/handler_test.go +++ b/modules/roles/handler_test.go @@ -38,11 +38,13 @@ func TestCreateRole(t *testing.T) { store := state.NewMemKVStore() for i, tc := range cases { tx := roles.NewCreateRoleTx([]byte(tc.role), tc.min, tc.sigs) - _, err := h.CheckTx(ctx, store, tx) + cres, err := h.CheckTx(ctx, store, tx) _, err2 := h.DeliverTx(ctx, store, tx) if tc.valid { assert.Nil(err, "%d/%s: %+v", i, tc.role, err) assert.Nil(err2, "%d/%s: %+v", i, tc.role, err2) + assert.Equal(roles.CostCreate, cres.GasAllocated) + assert.Equal(uint(0), cres.GasPayment) } else { assert.NotNil(err, "%d/%s", i, tc.role) assert.NotNil(err2, "%d/%s", i, tc.role) diff --git a/modules/roles/middleware_test.go b/modules/roles/middleware_test.go index 14e792b1a..77b8bbd73 100644 --- a/modules/roles/middleware_test.go +++ b/modules/roles/middleware_test.go @@ -93,11 +93,14 @@ func TestAssumeRole(t *testing.T) { } // try CheckTx and DeliverTx and make sure they both assert permissions - _, err := app.CheckTx(myCtx, store, tx) + cres, err := app.CheckTx(myCtx, store, tx) _, err2 := app.DeliverTx(myCtx, store, tx) if tc.valid { assert.Nil(err, "%d: %+v", i, err) assert.Nil(err2, "%d: %+v", i, err2) + // make sure we charge for each role + assert.Equal(roles.CostAssume*uint(len(tc.roles)), cres.GasAllocated) + assert.Equal(uint(0), cres.GasPayment) } else { assert.NotNil(err, "%d", i) assert.NotNil(err2, "%d", i) From 21e2399fc4526067b6f9bf7eca60e223237aec61 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Thu, 3 Aug 2017 21:40:45 +0200 Subject: [PATCH 11/13] Tested and fixed multiplexer, add more helpers --- TODO.md | 9 +-- app/app_val_test.go | 54 +------------- modules/base/helpers.go | 118 +++++++++++++++++++++++++++++++ modules/base/multiplexer.go | 4 +- modules/base/multiplexer_test.go | 87 +++++++++++++++++++++++ modules/base/tx.go | 4 +- 6 files changed, 213 insertions(+), 63 deletions(-) create mode 100644 modules/base/helpers.go create mode 100644 modules/base/multiplexer_test.go diff --git a/TODO.md b/TODO.md index eb4a4ab39..9a215a078 100644 --- a/TODO.md +++ b/TODO.md @@ -1,9 +1,3 @@ -# TODO for rewrite - -* Add tests for new CheckTx -* Test Multiplexer - - Alexis: * merkle - proof (non-existence - maybe range) @@ -12,8 +6,7 @@ light-client proofs: * make this sensible -> very tied to merkle proofs and API * support new proof types -* abci add range suppprt - +* expose more proof types in basecoin.Query * merkle - api cleanup (also Bonsai) diff --git a/app/app_val_test.go b/app/app_val_test.go index cf2cdbfe0..d1697168d 100644 --- a/app/app_val_test.go +++ b/app/app_val_test.go @@ -6,60 +6,12 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" abci "github.com/tendermint/abci/types" + "github.com/tendermint/basecoin/modules/base" wire "github.com/tendermint/go-wire" cmn "github.com/tendermint/tmlibs/common" "github.com/tendermint/tmlibs/log" - - "github.com/tendermint/basecoin" - "github.com/tendermint/basecoin/errors" - "github.com/tendermint/basecoin/state" ) -//-------------------------------- -// Setup tx and handler for validation test cases - -const ( - ValName = "val" - TypeValChange = ValName + "/change" - ByteValChange = 0xfe -) - -func init() { - basecoin.TxMapper.RegisterImplementation(ValChangeTx{}, TypeValChange, ByteValChange) -} - -type ValSetHandler struct { - basecoin.NopCheck - basecoin.NopInitState - basecoin.NopInitValidate -} - -var _ basecoin.Handler = ValSetHandler{} - -func (ValSetHandler) Name() string { - return ValName -} - -func (ValSetHandler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, - tx basecoin.Tx) (res basecoin.DeliverResult, err error) { - change, ok := tx.Unwrap().(ValChangeTx) - if !ok { - return res, errors.ErrUnknownTxType(tx) - } - res.Diff = change.Diff - return -} - -type ValChangeTx struct { - Diff []*abci.Validator -} - -func (v ValChangeTx) Wrap() basecoin.Tx { - return basecoin.Tx{v} -} - -func (v ValChangeTx) ValidateBasic() error { return nil } - //----------------------------------- // Test cases start here @@ -90,7 +42,7 @@ func TestEndBlock(t *testing.T) { logger := log.NewNopLogger() store := MockStore() - handler := ValSetHandler{} + handler := base.ValSetHandler{} app := NewBasecoin(handler, store, logger) val1 := makeVal() @@ -125,7 +77,7 @@ func TestEndBlock(t *testing.T) { for i, tc := range cases { app.BeginBlock(nil, nil) for _, c := range tc.changes { - tx := ValChangeTx{c}.Wrap() + tx := base.ValChangeTx{c}.Wrap() txBytes := wire.BinaryBytes(tx) res := app.DeliverTx(txBytes) require.True(res.IsOK(), "%#v", res) diff --git a/modules/base/helpers.go b/modules/base/helpers.go new file mode 100644 index 000000000..953e903ff --- /dev/null +++ b/modules/base/helpers.go @@ -0,0 +1,118 @@ +package base + +import ( + abci "github.com/tendermint/abci/types" + + "github.com/tendermint/basecoin" + "github.com/tendermint/basecoin/errors" + "github.com/tendermint/basecoin/state" +) + +//nolint +const ( + NameVal = "val" + NamePrice = "price" + + TypeValChange = NameVal + "/change" + ByteValChange = 0xfe + + TypePriceShow = NamePrice + "/show" + BytePriceShow = 0xfd +) + +func init() { + basecoin.TxMapper. + RegisterImplementation(ValChangeTx{}, TypeValChange, ByteValChange). + RegisterImplementation(PriceShowTx{}, TypePriceShow, BytePriceShow) +} + +//-------------------------------- +// Setup tx and handler for validation test cases + +type ValSetHandler struct { + basecoin.NopCheck + basecoin.NopInitState + basecoin.NopInitValidate +} + +var _ basecoin.Handler = ValSetHandler{} + +func (ValSetHandler) Name() string { + return NameVal +} + +func (ValSetHandler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, + tx basecoin.Tx) (res basecoin.DeliverResult, err error) { + change, ok := tx.Unwrap().(ValChangeTx) + if !ok { + return res, errors.ErrUnknownTxType(tx) + } + res.Diff = change.Diff + return +} + +type ValChangeTx struct { + Diff []*abci.Validator +} + +func (v ValChangeTx) Wrap() basecoin.Tx { + return basecoin.Tx{v} +} + +func (v ValChangeTx) ValidateBasic() error { return nil } + +//-------------------------------- +// Setup tx and handler for testing checktx fees/gas + +// PriceData is the data we ping back +var PriceData = []byte{0xCA, 0xFE} + +// PriceHandler returns checktx results based on the input +type PriceHandler struct { + basecoin.NopInitState + basecoin.NopInitValidate +} + +var _ basecoin.Handler = PriceHandler{} + +func (PriceHandler) Name() string { + return NamePrice +} + +func (PriceHandler) CheckTx(ctx basecoin.Context, store state.SimpleDB, + tx basecoin.Tx) (res basecoin.CheckResult, err error) { + price, ok := tx.Unwrap().(PriceShowTx) + if !ok { + return res, errors.ErrUnknownTxType(tx) + } + res.GasAllocated = price.GasAllocated + res.GasPayment = price.GasPayment + res.Data = PriceData + return +} + +func (PriceHandler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, + tx basecoin.Tx) (res basecoin.DeliverResult, err error) { + _, ok := tx.Unwrap().(PriceShowTx) + if !ok { + return res, errors.ErrUnknownTxType(tx) + } + res.Data = PriceData + return +} + +// PriceShowTx lets us bounce back a given fee/gas on CheckTx +type PriceShowTx struct { + GasAllocated uint + GasPayment uint +} + +func NewPriceShowTx(gasAllocated, gasPayment uint) basecoin.Tx { + return PriceShowTx{GasAllocated: gasAllocated, GasPayment: gasPayment}.Wrap() +} + +func (p PriceShowTx) Wrap() basecoin.Tx { + return basecoin.Tx{p} +} + +func (v PriceShowTx) ValidateBasic() error { return nil } diff --git a/modules/base/multiplexer.go b/modules/base/multiplexer.go index 408812901..f71b37cf9 100644 --- a/modules/base/multiplexer.go +++ b/modules/base/multiplexer.go @@ -32,7 +32,7 @@ var _ stack.Middleware = Multiplexer{} // CheckTx splits the input tx and checks them all - fulfills Middlware interface func (Multiplexer) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Checker) (res basecoin.CheckResult, err error) { - if mtx, ok := tx.Unwrap().(*MultiTx); ok { + if mtx, ok := tx.Unwrap().(MultiTx); ok { return runAllChecks(ctx, store, mtx.Txs, next) } return next.CheckTx(ctx, store, tx) @@ -40,7 +40,7 @@ func (Multiplexer) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx baseco // DeliverTx splits the input tx and checks them all - fulfills Middlware interface func (Multiplexer) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.DeliverResult, err error) { - if mtx, ok := tx.Unwrap().(*MultiTx); ok { + if mtx, ok := tx.Unwrap().(MultiTx); ok { return runAllDelivers(ctx, store, mtx.Txs, next) } return next.DeliverTx(ctx, store, tx) diff --git a/modules/base/multiplexer_test.go b/modules/base/multiplexer_test.go new file mode 100644 index 000000000..a3862bb68 --- /dev/null +++ b/modules/base/multiplexer_test.go @@ -0,0 +1,87 @@ +package base + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/tendermint/basecoin" + "github.com/tendermint/basecoin/stack" + "github.com/tendermint/basecoin/state" + wire "github.com/tendermint/go-wire" + "github.com/tendermint/go-wire/data" + "github.com/tendermint/tmlibs/log" +) + +func TestMultiplexer(t *testing.T) { + assert := assert.New(t) + msg := "diddly" + chainID := "multi-verse" + height := uint64(100) + + // Generic args here... + store := state.NewMemKVStore() + ctx := stack.NewContext(chainID, height, log.NewNopLogger()) + + // Build the stack + app := stack. + New(Multiplexer{}). + Dispatch( + stack.WrapHandler(stack.OKHandler{Log: msg}), + stack.WrapHandler(PriceHandler{}), + ) + + raw := stack.NewRawTx([]byte{1, 2, 3, 4}) + fail := stack.NewFailTx() + price1 := NewPriceShowTx(123, 456) + price2 := NewPriceShowTx(1000, 2000) + price3 := NewPriceShowTx(11, 0) + + join := func(data ...[]byte) []byte { + return wire.BinaryBytes(data) + } + + cases := [...]struct { + tx basecoin.Tx + valid bool + gasAllocated uint + gasPayment uint + log string + data data.Bytes + }{ + // test the components without multiplexer (no effect) + 0: {raw, true, 0, 0, msg, nil}, + 1: {price1, true, 123, 456, "", PriceData}, + 2: {fail, false, 0, 0, "", nil}, + // test multiplexer on error + 3: {NewMultiTx(raw, fail, price1), false, 0, 0, "", nil}, + // test combining info on multiplexer + 4: {NewMultiTx(price1, raw), true, 123, 456, "\n" + msg, join(PriceData, nil)}, + // add lots of prices + 5: {NewMultiTx(price1, price2, price3), true, 1134, 2456, "\n\n", join(PriceData, PriceData, PriceData)}, + // combine multiple logs + 6: {NewMultiTx(raw, price3, raw), true, 11, 0, msg + "\n\n" + msg, join(nil, PriceData, nil)}, + } + + for i, tc := range cases { + cres, err := app.CheckTx(ctx, store, tc.tx) + if tc.valid { + assert.Nil(err, "%d: %+v", i, err) + assert.Equal(tc.log, cres.Log, "%d", i) + assert.Equal(tc.data, cres.Data, "%d", i) + assert.Equal(tc.gasAllocated, cres.GasAllocated, "%d", i) + assert.Equal(tc.gasPayment, cres.GasPayment, "%d", i) + } else { + assert.NotNil(err, "%d", i) + } + + // make sure deliver returns error, not a panic crash + dres, err := app.DeliverTx(ctx, store, tc.tx) + if tc.valid { + assert.Nil(err, "%d: %+v", i, err) + assert.Equal(tc.log, dres.Log, "%d", i) + assert.Equal(tc.data, dres.Data, "%d", i) + } else { + assert.NotNil(err, "%d", i) + } + } +} diff --git a/modules/base/tx.go b/modules/base/tx.go index b2f23db2a..d365c1b7b 100644 --- a/modules/base/tx.go +++ b/modules/base/tx.go @@ -16,13 +16,13 @@ const ( //nolint const ( - // TypeMultiTx = NameMultiplexer + "/tx" + TypeMultiTx = NameMultiplexer + "/tx" TypeChainTx = NameChain + "/tx" ) func init() { basecoin.TxMapper. - // RegisterImplementation(MultiTx{}, TypeMultiTx, ByteMultiTx). + RegisterImplementation(MultiTx{}, TypeMultiTx, ByteMultiTx). RegisterImplementation(ChainTx{}, TypeChainTx, ByteChainTx) } From 2f4f875dd4449b81a369a177ee3c3dba387f191d Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Fri, 4 Aug 2017 13:50:55 +0200 Subject: [PATCH 12/13] 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 117b155e8..f31425f36 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 d1697168d..aabb83443 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 32689b3a4..6c45b4c69 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 d1eca64bb..53de71d5b 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 4db113f25..b7fa9231a 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 { From 640f06998afbe479fab7a5bf48edb3257fba4355 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Fri, 4 Aug 2017 14:11:01 +0200 Subject: [PATCH 13/13] Moved all gas and payment values to uint64 to make sure we are safe here --- handler.go | 8 ++++---- modules/base/helpers.go | 6 +++--- modules/base/multiplexer.go | 4 ++-- modules/base/multiplexer_test.go | 4 ++-- modules/coin/handler.go | 6 +++--- modules/coin/handler_test.go | 4 ++-- modules/fee/handler.go | 4 ++-- modules/fee/handler_test.go | 2 +- modules/roles/handler.go | 4 ++-- modules/roles/handler_test.go | 2 +- modules/roles/middleware_test.go | 4 ++-- 11 files changed, 24 insertions(+), 24 deletions(-) diff --git a/handler.go b/handler.go index 6c45b4c69..82c636ddd 100644 --- a/handler.go +++ b/handler.go @@ -99,14 +99,14 @@ type CheckResult struct { Data data.Bytes Log string // GasAllocated is the maximum units of work we allow this tx to perform - GasAllocated uint + GasAllocated uint64 // GasPayment is the total fees for this tx (or other source of payment) - GasPayment uint + GasPayment uint64 } // NewCheck sets the gas used and the response data but no more info // these are the most common info needed to be set by the Handler -func NewCheck(gasAllocated uint, log string) CheckResult { +func NewCheck(gasAllocated uint64, log string) CheckResult { return CheckResult{ GasAllocated: gasAllocated, Log: log, @@ -129,7 +129,7 @@ type DeliverResult struct { Data data.Bytes Log string Diff []*abci.Validator - GasUsed uint + GasUsed uint64 } var _ Result = DeliverResult{} diff --git a/modules/base/helpers.go b/modules/base/helpers.go index 953e903ff..e99d5af7e 100644 --- a/modules/base/helpers.go +++ b/modules/base/helpers.go @@ -103,11 +103,11 @@ func (PriceHandler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, // PriceShowTx lets us bounce back a given fee/gas on CheckTx type PriceShowTx struct { - GasAllocated uint - GasPayment uint + GasAllocated uint64 + GasPayment uint64 } -func NewPriceShowTx(gasAllocated, gasPayment uint) basecoin.Tx { +func NewPriceShowTx(gasAllocated, gasPayment uint64) basecoin.Tx { return PriceShowTx{GasAllocated: gasAllocated, GasPayment: gasPayment}.Wrap() } diff --git a/modules/base/multiplexer.go b/modules/base/multiplexer.go index f71b37cf9..38008376c 100644 --- a/modules/base/multiplexer.go +++ b/modules/base/multiplexer.go @@ -77,7 +77,7 @@ func runAllDelivers(ctx basecoin.Context, store state.SimpleDB, txs []basecoin.T func combineChecks(all []basecoin.CheckResult) basecoin.CheckResult { datas := make([]data.Bytes, len(all)) logs := make([]string, len(all)) - var allocated, payments uint + var allocated, payments uint64 for i, r := range all { datas[i] = r.Data logs[i] = r.Log @@ -97,7 +97,7 @@ func combineChecks(all []basecoin.CheckResult) basecoin.CheckResult { func combineDelivers(all []basecoin.DeliverResult) basecoin.DeliverResult { datas := make([]data.Bytes, len(all)) logs := make([]string, len(all)) - var used uint + var used uint64 var diffs []*abci.Validator for i, r := range all { datas[i] = r.Data diff --git a/modules/base/multiplexer_test.go b/modules/base/multiplexer_test.go index a3862bb68..1378d8dbd 100644 --- a/modules/base/multiplexer_test.go +++ b/modules/base/multiplexer_test.go @@ -43,8 +43,8 @@ func TestMultiplexer(t *testing.T) { cases := [...]struct { tx basecoin.Tx valid bool - gasAllocated uint - gasPayment uint + gasAllocated uint64 + gasPayment uint64 log string data data.Bytes }{ diff --git a/modules/coin/handler.go b/modules/coin/handler.go index 0d1010803..c083ab7a3 100644 --- a/modules/coin/handler.go +++ b/modules/coin/handler.go @@ -16,9 +16,9 @@ const ( //NameCoin - name space of the coin module NameCoin = "coin" // CostSend is GasAllocation per input/output - CostSend = uint(10) + CostSend = uint64(10) // CostCredit is GasAllocation of a credit allocation - CostCredit = uint(20) + CostCredit = uint64(20) ) // Handler includes an accountant @@ -53,7 +53,7 @@ func (h Handler) CheckTx(ctx basecoin.Context, store state.SimpleDB, switch t := tx.Unwrap().(type) { case SendTx: // price based on inputs and outputs - used := uint(len(t.Inputs) + len(t.Outputs)) + used := uint64(len(t.Inputs) + len(t.Outputs)) return basecoin.NewCheck(used*CostSend, ""), h.checkSendTx(ctx, store, t) case CreditTx: // default price of 20, constant work diff --git a/modules/coin/handler_test.go b/modules/coin/handler_test.go index 502f1d71a..4c6a4b784 100644 --- a/modules/coin/handler_test.go +++ b/modules/coin/handler_test.go @@ -110,7 +110,7 @@ func TestCheckDeliverSendTx(t *testing.T) { tx basecoin.Tx perms []basecoin.Actor final []money // nil for error - cost uint // gas allocated (if not error) + cost uint64 // gas allocated (if not error) }{ { []money{{addr1, moreCoins}}, @@ -175,7 +175,7 @@ func TestCheckDeliverSendTx(t *testing.T) { assert.Nil(err, "%d: %+v", i, err) assert.Nil(err2, "%d: %+v", i, err2) // make sure proper gas is set - assert.Equal(uint(0), cres.GasPayment, "%d", i) + assert.Equal(uint64(0), cres.GasPayment, "%d", i) assert.Equal(tc.cost, cres.GasAllocated, "%d", i) // make sure the final balances are correct for _, f := range tc.final { diff --git a/modules/fee/handler.go b/modules/fee/handler.go index 53de71d5b..564eab1ce 100644 --- a/modules/fee/handler.go +++ b/modules/fee/handler.go @@ -55,14 +55,14 @@ func (h SimpleFeeMiddleware) CheckTx(ctx basecoin.Context, store state.SimpleDB, return res, err } - var paid, used uint + var paid, used uint64 if !fee.Fee.IsZero() { // now, try to make a IPC call to coins... send := coin.NewSendOneTx(fee.Payer, h.Collector, coin.Coins{fee.Fee}) sendRes, err := next.CheckTx(ctx, store, send) if err != nil { return res, err } - paid = uint(fee.Fee.Amount) + paid = uint64(fee.Fee.Amount) used = sendRes.GasAllocated } diff --git a/modules/fee/handler_test.go b/modules/fee/handler_test.go index 546ebba2f..dcaff0b09 100644 --- a/modules/fee/handler_test.go +++ b/modules/fee/handler_test.go @@ -71,7 +71,7 @@ func TestFeeChecks(t *testing.T) { left coin.Coins collected coin.Coins // expected gas allocated - expectedCost uint + expectedCost uint64 }{ // make sure it works with no fee (control group) {true, app1, act1, false, act1, zero, mixed, nil, 0}, diff --git a/modules/roles/handler.go b/modules/roles/handler.go index bca582fba..d12ba0764 100644 --- a/modules/roles/handler.go +++ b/modules/roles/handler.go @@ -10,9 +10,9 @@ const ( //NameRole - name space of the roles module NameRole = "role" // CostCreate is the cost to create a new role - CostCreate = uint(40) + CostCreate = uint64(40) // CostAssume is the cost to assume a role as part of a tx - CostAssume = uint(5) + CostAssume = uint64(5) ) // Handler allows us to create new roles diff --git a/modules/roles/handler_test.go b/modules/roles/handler_test.go index 1d6859259..ca177bf22 100644 --- a/modules/roles/handler_test.go +++ b/modules/roles/handler_test.go @@ -44,7 +44,7 @@ func TestCreateRole(t *testing.T) { assert.Nil(err, "%d/%s: %+v", i, tc.role, err) assert.Nil(err2, "%d/%s: %+v", i, tc.role, err2) assert.Equal(roles.CostCreate, cres.GasAllocated) - assert.Equal(uint(0), cres.GasPayment) + assert.Equal(uint64(0), cres.GasPayment) } else { assert.NotNil(err, "%d/%s", i, tc.role) assert.NotNil(err2, "%d/%s", i, tc.role) diff --git a/modules/roles/middleware_test.go b/modules/roles/middleware_test.go index 77b8bbd73..032a9106e 100644 --- a/modules/roles/middleware_test.go +++ b/modules/roles/middleware_test.go @@ -99,8 +99,8 @@ func TestAssumeRole(t *testing.T) { assert.Nil(err, "%d: %+v", i, err) assert.Nil(err2, "%d: %+v", i, err2) // make sure we charge for each role - assert.Equal(roles.CostAssume*uint(len(tc.roles)), cres.GasAllocated) - assert.Equal(uint(0), cres.GasPayment) + assert.Equal(roles.CostAssume*uint64(len(tc.roles)), cres.GasAllocated) + assert.Equal(uint64(0), cres.GasPayment) } else { assert.NotNil(err, "%d", i) assert.NotNil(err2, "%d", i)