From 2f73cf4193faab13fead96614405947d7e3e037c Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Mon, 12 Nov 2018 23:12:09 -0500 Subject: [PATCH 01/27] block gas meter working --- baseapp/baseapp.go | 25 +++++++++++++++++++++---- types/context.go | 7 +++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 827536d21..419f88eb6 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -68,8 +68,10 @@ type BaseApp struct { deliverState *state // for DeliverTx voteInfos []abci.VoteInfo // absent validators from begin block - // minimum fees for spam prevention - minimumFees sdk.Coins + // spam prevention + minimumFees sdk.Coins + maximumBlockGas int64 + deliverGas // flag for sealing sealed bool @@ -194,6 +196,9 @@ func (app *BaseApp) initFromStore(mainKey sdk.StoreKey) error { // SetMinimumFees sets the minimum fees. func (app *BaseApp) SetMinimumFees(fees sdk.Coins) { app.minimumFees = fees } +// SetMaximumBlockGas sets the maximum gas allowable per block. +func (app *BaseApp) SetMaximumBlockGas(gas int64) { app.maximumBlockGas = gas } + // NewContext returns a new Context with the correct store, the given header, and nil txBytes. func (app *BaseApp) NewContext(isCheckTx bool, header abci.Header) sdk.Context { if isCheckTx { @@ -422,12 +427,19 @@ func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeg // Initialize the DeliverTx state. If this is the first block, it should // already be initialized in InitChain. Otherwise app.deliverState will be // nil, since it is reset on Commit. + blockGasMeter := sdk.NewGasMeter(app.maximumBlockGas) if app.deliverState == nil { app.setDeliverState(req.Header) + app.deliverState.ctx = app.deliverState.ctx. + WithBlockGasMeter(blockGasMeter) + } else { // In the first block, app.deliverState.ctx will already be initialized // by InitChain. Context is now updated with Header information. - app.deliverState.ctx = app.deliverState.ctx.WithBlockHeader(req.Header).WithBlockHeight(req.Header.Height) + app.deliverState.ctx = app.deliverState.ctx. + WithBlockHeader(req.Header). + WithBlockHeight(req.Header.Height). + WithBlockGasMeter(blockGasMeter) } if app.beginBlocker != nil { @@ -467,9 +479,10 @@ func (app *BaseApp) CheckTx(txBytes []byte) (res abci.ResponseCheckTx) { // Implements ABCI func (app *BaseApp) DeliverTx(txBytes []byte) (res abci.ResponseDeliverTx) { + // Decode the Tx. - var result sdk.Result var tx, err = app.txDecoder(txBytes) + var result sdk.Result if err != nil { result = err.Result() } else { @@ -655,6 +668,10 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk result = app.runMsgs(ctx, msgs, mode) result.GasWanted = gasWanted + // consume block gas + ctx.BlockGasMeter.ConsumeGas( + ctx.GasMeter().GasConsumed(), "block gas meter") + // only update state if all messages pass if result.IsOK() { msCache.Write() diff --git a/types/context.go b/types/context.go index bfb4c58fe..ed65e57d9 100644 --- a/types/context.go +++ b/types/context.go @@ -140,6 +140,7 @@ const ( contextKeyLogger contextKeyVoteInfos contextKeyGasMeter + contextKeyBlockGasMeter contextKeyMinimumFees ) @@ -170,6 +171,8 @@ func (c Context) VoteInfos() []abci.VoteInfo { func (c Context) GasMeter() GasMeter { return c.Value(contextKeyGasMeter).(GasMeter) } +func (c Context) BlockGasMeter() GasMeter { return c.Value(contextKeyBlockGasMeter).(GasMeter) } + func (c Context) IsCheckTx() bool { return c.Value(contextKeyIsCheckTx).(bool) } func (c Context) MinimumFees() Coins { return c.Value(contextKeyMinimumFees).(Coins) } @@ -219,6 +222,10 @@ func (c Context) WithVoteInfos(VoteInfos []abci.VoteInfo) Context { func (c Context) WithGasMeter(meter GasMeter) Context { return c.withValue(contextKeyGasMeter, meter) } +func (c Context) WithBlockGasMeter(meter GasMeter) Context { + return c.withValue(contextKeyBlockGasMeter, meter) +} + func (c Context) WithIsCheckTx(isCheckTx bool) Context { return c.withValue(contextKeyIsCheckTx, isCheckTx) } From 956d351f68705e073e6ba1bd412eb5e6ff8d2f7d Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Tue, 13 Nov 2018 11:30:06 -0500 Subject: [PATCH 02/27] basic structure in place --- baseapp/baseapp.go | 16 ++++++++++------ types/gas.go | 9 +++++++++ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 419f88eb6..9f9d98f7e 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -427,20 +427,17 @@ func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeg // Initialize the DeliverTx state. If this is the first block, it should // already be initialized in InitChain. Otherwise app.deliverState will be // nil, since it is reset on Commit. - blockGasMeter := sdk.NewGasMeter(app.maximumBlockGas) if app.deliverState == nil { app.setDeliverState(req.Header) - app.deliverState.ctx = app.deliverState.ctx. - WithBlockGasMeter(blockGasMeter) - } else { // In the first block, app.deliverState.ctx will already be initialized // by InitChain. Context is now updated with Header information. app.deliverState.ctx = app.deliverState.ctx. WithBlockHeader(req.Header). - WithBlockHeight(req.Header.Height). - WithBlockGasMeter(blockGasMeter) + WithBlockHeight(req.Header.Height) } + app.deliverState.ctx = app.deliverState.ctx. + WithBlockGasMeter(sdk.NewGasMeter(app.maximumBlockGas)) if app.beginBlocker != nil { res = app.beginBlocker(app.deliverState.ctx, req) @@ -607,6 +604,13 @@ func (app *BaseApp) initializeContext(ctx sdk.Context, mode runTxMode) sdk.Conte // anteHandler. txBytes may be nil in some cases, eg. in tests. Also, in the // future we may support "internal" transactions. func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk.Result) { + + // only run the tx if there is block gas remaining + if ctx.BlockGasMeter.PastLimit() { + result = sdk.ErrOutOfGas("no block gas left to run tx").Result() + return + } + // NOTE: GasWanted should be returned by the AnteHandler. GasUsed is // determined by the GasMeter. We need access to the context to get the gas // meter so we initialize upfront. diff --git a/types/gas.go b/types/gas.go index 7b0347467..6ec29dd13 100644 --- a/types/gas.go +++ b/types/gas.go @@ -29,6 +29,7 @@ type ErrorOutOfGas struct { type GasMeter interface { GasConsumed() Gas ConsumeGas(amount Gas, descriptor string) + PastLimit() bool } type basicGasMeter struct { @@ -55,6 +56,10 @@ func (g *basicGasMeter) ConsumeGas(amount Gas, descriptor string) { } } +func (g *basicGasMeter) PastLimit() bool { + return g.consumed > g.limit +} + type infiniteGasMeter struct { consumed Gas } @@ -74,6 +79,10 @@ func (g *infiniteGasMeter) ConsumeGas(amount Gas, descriptor string) { g.consumed += amount } +func (g *infiniteGasMeter) PastLimit() bool { + return false +} + // GasConfig defines gas cost for each operation on KVStores type GasConfig struct { HasCost Gas From ebaa39468ac491063959b1d9884bd3c899ab333a Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Tue, 13 Nov 2018 13:01:18 -0500 Subject: [PATCH 03/27] modified app provider to pass genesis --- baseapp/options.go | 5 +++++ cmd/gaia/app/genesis.go | 8 +++++--- cmd/gaia/cmd/gaiad/main.go | 18 +++++++++++++++--- server/constructors.go | 4 +++- server/start.go | 9 ++++++--- 5 files changed, 34 insertions(+), 10 deletions(-) diff --git a/baseapp/options.go b/baseapp/options.go index a6460248d..8d6131314 100644 --- a/baseapp/options.go +++ b/baseapp/options.go @@ -39,6 +39,11 @@ func SetMinimumFees(minFees string) func(*BaseApp) { return func(bap *BaseApp) { bap.SetMinimumFees(fees) } } +// SetMinimumFees returns an option that sets the minimum fees on the app. +func SetMaximumBlockGas(gas int64) func(*BaseApp) { + return func(bap *BaseApp) { bap.SetMaximumBlockGas(gas) } +} + func (app *BaseApp) SetName(name string) { if app.sealed { panic("SetName() on sealed BaseApp") diff --git a/cmd/gaia/app/genesis.go b/cmd/gaia/app/genesis.go index e3c869ada..c0929205b 100644 --- a/cmd/gaia/app/genesis.go +++ b/cmd/gaia/app/genesis.go @@ -42,8 +42,10 @@ type GenesisState struct { GenTxs []json.RawMessage `json:"gentxs"` } -func NewGenesisState(accounts []GenesisAccount, authData auth.GenesisState, stakeData stake.GenesisState, mintData mint.GenesisState, - distrData distr.GenesisState, govData gov.GenesisState, slashingData slashing.GenesisState) GenesisState { +func NewGenesisState(accounts []GenesisAccount, authData auth.GenesisState, + stakeData stake.GenesisState, mintData mint.GenesisState, + distrData distr.GenesisState, govData gov.GenesisState, + slashingData slashing.GenesisState) GenesisState { return GenesisState{ Accounts: accounts, @@ -287,7 +289,7 @@ func CollectStdTxs(cdc *codec.Codec, moniker string, genTxsDir string, genDoc tm func NewDefaultGenesisAccount(addr sdk.AccAddress) GenesisAccount { accAuth := auth.NewBaseAccountWithAddress(addr) - coins :=sdk.Coins{ + coins := sdk.Coins{ {"fooToken", sdk.NewInt(1000)}, {bondDenom, freeFermionsAcc}, } diff --git a/cmd/gaia/cmd/gaiad/main.go b/cmd/gaia/cmd/gaiad/main.go index ef7b39d11..4154b85be 100644 --- a/cmd/gaia/cmd/gaiad/main.go +++ b/cmd/gaia/cmd/gaiad/main.go @@ -13,6 +13,7 @@ import ( "github.com/tendermint/tendermint/libs/cli" dbm "github.com/tendermint/tendermint/libs/db" "github.com/tendermint/tendermint/libs/log" + "github.com/tendermint/tendermint/node" tmtypes "github.com/tendermint/tendermint/types" "github.com/cosmos/cosmos-sdk/cmd/gaia/app" @@ -56,16 +57,27 @@ func main() { } } -func newApp(logger log.Logger, db dbm.DB, traceStore io.Writer) abci.Application { +func newApp(logger log.Logger, db dbm.DB, + traceStore io.Writer, genDocProvider node.GenesisDocProvider) abci.Application { + + // get the maximum gas from tendermint genesis parameters + genDoc, err := genDocProvider() + if err != nil { + panic(err) + } + maxBlockGas := genDoc.ConsensusParams.BlockSize.MaxGas + return app.NewGaiaApp(logger, db, traceStore, baseapp.SetPruning(viper.GetString("pruning")), baseapp.SetMinimumFees(viper.GetString("minimum_fees")), + baseapp.SetMaximumBlockGas(maxBlockGas), ) } func exportAppStateAndTMValidators( - logger log.Logger, db dbm.DB, traceStore io.Writer, -) (json.RawMessage, []tmtypes.GenesisValidator, error) { + logger log.Logger, db dbm.DB, traceStore io.Writer) ( + json.RawMessage, []tmtypes.GenesisValidator, error) { + gApp := app.NewGaiaApp(logger, db, traceStore) return gApp.ExportAppStateAndValidators() } diff --git a/server/constructors.go b/server/constructors.go index 57282e43b..63cb5f62b 100644 --- a/server/constructors.go +++ b/server/constructors.go @@ -9,13 +9,15 @@ import ( abci "github.com/tendermint/tendermint/abci/types" dbm "github.com/tendermint/tendermint/libs/db" "github.com/tendermint/tendermint/libs/log" + "github.com/tendermint/tendermint/node" tmtypes "github.com/tendermint/tendermint/types" ) type ( // AppCreator is a function that allows us to lazily initialize an // application using various configurations. - AppCreator func(log.Logger, dbm.DB, io.Writer) abci.Application + AppCreator func(log.Logger, dbm.DB, + io.Writer, node.GenesisDocProvider) abci.Application // AppExporter is a function that dumps all app state to // JSON-serializable structure and returns the current validator set. diff --git a/server/start.go b/server/start.go index cf39ff71b..d84169ab4 100644 --- a/server/start.go +++ b/server/start.go @@ -68,7 +68,9 @@ func startStandAlone(ctx *Context, appCreator AppCreator) error { return err } - app := appCreator(ctx.Logger, db, traceWriter) + cfg := ctx.Config + genDocProvider := node.DefaultGenesisDocProviderFunc(cfg) + app := appCreator(ctx.Logger, db, traceWriter, genDocProvider) svr, err := server.NewServer(addr, "socket", app) if err != nil { @@ -107,7 +109,8 @@ func startInProcess(ctx *Context, appCreator AppCreator) (*node.Node, error) { return nil, err } - app := appCreator(ctx.Logger, db, traceWriter) + genDocProvider := node.DefaultGenesisDocProviderFunc(cfg) + app := appCreator(ctx.Logger, db, traceWriter, genDocProvider) nodeKey, err := p2p.LoadOrGenNodeKey(cfg.NodeKeyFile()) if err != nil { @@ -120,7 +123,7 @@ func startInProcess(ctx *Context, appCreator AppCreator) (*node.Node, error) { pvm.LoadOrGenFilePV(cfg.PrivValidatorFile()), nodeKey, proxy.NewLocalClientCreator(app), - node.DefaultGenesisDocProviderFunc(cfg), + genDocProvider, node.DefaultDBProvider, node.DefaultMetricsProvider(cfg.Instrumentation), ctx.Logger.With("module", "node"), From 3bf67b63e11560d67ad87e376df44b21a7c4e538 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Tue, 13 Nov 2018 14:27:15 -0500 Subject: [PATCH 04/27] compiling --- baseapp/baseapp.go | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 9f9d98f7e..be7177fb2 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -71,7 +71,6 @@ type BaseApp struct { // spam prevention minimumFees sdk.Coins maximumBlockGas int64 - deliverGas // flag for sealing sealed bool @@ -604,13 +603,6 @@ func (app *BaseApp) initializeContext(ctx sdk.Context, mode runTxMode) sdk.Conte // anteHandler. txBytes may be nil in some cases, eg. in tests. Also, in the // future we may support "internal" transactions. func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk.Result) { - - // only run the tx if there is block gas remaining - if ctx.BlockGasMeter.PastLimit() { - result = sdk.ErrOutOfGas("no block gas left to run tx").Result() - return - } - // NOTE: GasWanted should be returned by the AnteHandler. GasUsed is // determined by the GasMeter. We need access to the context to get the gas // meter so we initialize upfront. @@ -619,6 +611,12 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk ctx := app.getContextForAnte(mode, txBytes) ctx = app.initializeContext(ctx, mode) + // only run the tx if there is block gas remaining + if ctx.BlockGasMeter().PastLimit() { + result = sdk.ErrOutOfGas("no block gas left to run tx").Result() + return + } + defer func() { if r := recover(); r != nil { switch rType := r.(type) { @@ -673,7 +671,7 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk result.GasWanted = gasWanted // consume block gas - ctx.BlockGasMeter.ConsumeGas( + ctx.BlockGasMeter().ConsumeGas( ctx.GasMeter().GasConsumed(), "block gas meter") // only update state if all messages pass From 8069b2b7e6a61c6f9519b943e123e3fae35d0bb8 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Tue, 13 Nov 2018 14:30:24 -0500 Subject: [PATCH 05/27] default infinite block gas meter --- baseapp/baseapp.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index be7177fb2..e661c904f 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -435,8 +435,15 @@ func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeg WithBlockHeader(req.Header). WithBlockHeight(req.Header.Height) } - app.deliverState.ctx = app.deliverState.ctx. - WithBlockGasMeter(sdk.NewGasMeter(app.maximumBlockGas)) + + // add block gas meter + if app.maximumBlockGas > 0 { + app.deliverState.ctx = app.deliverState.ctx. + WithBlockGasMeter(sdk.NewGasMeter(app.maximumBlockGas)) + } else { + app.deliverState.ctx = app.deliverState.ctx. + WithBlockGasMeter(sdk.NewInfiniteGasMeter()) + } if app.beginBlocker != nil { res = app.beginBlocker(app.deliverState.ctx, req) From 24468306b48eb91631c040f262322bab49aba397 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Tue, 13 Nov 2018 14:35:53 -0500 Subject: [PATCH 06/27] examples installing --- examples/basecoin/cmd/basecoind/main.go | 3 ++- examples/democoin/cmd/democoind/main.go | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/examples/basecoin/cmd/basecoind/main.go b/examples/basecoin/cmd/basecoind/main.go index f07fbd3ff..e56654f51 100644 --- a/examples/basecoin/cmd/basecoind/main.go +++ b/examples/basecoin/cmd/basecoind/main.go @@ -6,6 +6,7 @@ import ( "io" "os" + "github.com/tendermint/tendermint/node" "github.com/tendermint/tendermint/p2p" "github.com/cosmos/cosmos-sdk/baseapp" @@ -122,7 +123,7 @@ func InitCmd(ctx *server.Context, cdc *codec.Codec, appInit server.AppInit) *cob return cmd } -func newApp(logger log.Logger, db dbm.DB, storeTracer io.Writer) abci.Application { +func newApp(logger log.Logger, db dbm.DB, storeTracer io.Writer, _ node.GenesisDocProvider) abci.Application { return app.NewBasecoinApp(logger, db, baseapp.SetPruning(viper.GetString("pruning"))) } diff --git a/examples/democoin/cmd/democoind/main.go b/examples/democoin/cmd/democoind/main.go index d095b4c79..5acb459db 100644 --- a/examples/democoin/cmd/democoind/main.go +++ b/examples/democoin/cmd/democoind/main.go @@ -9,6 +9,7 @@ import ( "github.com/cosmos/cosmos-sdk/client" "github.com/spf13/viper" "github.com/tendermint/tendermint/libs/common" + "github.com/tendermint/tendermint/node" "github.com/tendermint/tendermint/p2p" "github.com/spf13/cobra" @@ -129,7 +130,9 @@ func InitCmd(ctx *server.Context, cdc *codec.Codec, appInit server.AppInit) *cob return cmd } -func newApp(logger log.Logger, db dbm.DB, _ io.Writer) abci.Application { +func newApp(logger log.Logger, db dbm.DB, _ io.Writer, + _ node.GenesisDocProvider) abci.Application { + return app.NewDemocoinApp(logger, db) } From eead27872fd577175fab9625be0539e61b3c7d0c Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Tue, 13 Nov 2018 15:12:04 -0500 Subject: [PATCH 07/27] initial test case --- baseapp/baseapp_test.go | 93 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 9f6414214..53c34334d 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -823,3 +823,96 @@ func TestTxGasLimits(t *testing.T) { } } } + +// Test that transactions exceeding gas limits fail +func TestMaxBlockGasLimits(t *testing.T) { + gasGranted := int64(10) + anteOpt := func(bapp *BaseApp) { + bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, res sdk.Result, abort bool) { + newCtx = ctx.WithGasMeter(sdk.NewGasMeter(gasGranted)) + + // NOTE/TODO/XXX: + // AnteHandlers must have their own defer/recover in order + // for the BaseApp to know how much gas was used used! + // This is because the GasMeter is created in the AnteHandler, + // but if it panics the context won't be set properly in runTx's recover ... + defer func() { + if r := recover(); r != nil { + switch rType := r.(type) { + case sdk.ErrorOutOfGas: + log := fmt.Sprintf("out of gas in location: %v", rType.Descriptor) + res = sdk.ErrOutOfGas(log).Result() + res.GasWanted = gasGranted + res.GasUsed = newCtx.GasMeter().GasConsumed() + default: + panic(r) + } + } + }() + + count := tx.(*txTest).Counter + newCtx.GasMeter().ConsumeGas(count, "counter-ante") + res = sdk.Result{ + GasWanted: gasGranted, + } + return + }) + + } + + routerOpt := func(bapp *BaseApp) { + bapp.Router().AddRoute(routeMsgCounter, func(ctx sdk.Context, msg sdk.Msg) sdk.Result { + count := msg.(msgCounter).Counter + ctx.GasMeter().ConsumeGas(count, "counter-handler") + return sdk.Result{} + }) + } + + app := setupBaseApp(t, anteOpt, routerOpt) + app.SetMaximumBlockGas(100) + + testCases := []struct { + tx *txTest + numDelivers int + blockGasUsed int64 + fail bool + }{ + {newTxCounter(0, 0), 0, 0, false}, + {newTxCounter(9, 1), 2, 20, false}, + {newTxCounter(10, 0), 3, 30, false}, + {newTxCounter(10, 0), 10, 100, false}, + {newTxCounter(2, 7), 11, 99, false}, + + {newTxCounter(2, 7), 12, 108, true}, + {newTxCounter(10, 0), 11, 110, true}, + } + + for i, tc := range testCases { + tx := tc.tx + + // reset the block gas + app.BeginBlock(abci.RequestBeginBlock{}) + + // execute the transaction multiple times + for j := 0; j < numDelivers; j++ { + res := app.Deliver(tx) + } + + ctx := app.getContextForAnte(runTxModeDeliver, tx) + ctx = app.initializeContext(ctx, runTxModeDeliver) + blockGasUsed := ctx.BlockGasMeter().ConsumedGas() + + // check gas used and wanted + require.Equal(t, tc.blockGasUsed, blockGasUsed, + fmt.Sprintf("%d: %v, %v, %v", i, tc, blockGasUsed, res)) + + // check for out of gas + if !tc.fail { + require.True(t, res.IsOK(), fmt.Sprintf("%d: %v, %v", i, tc, res)) + require.False(t, ctx.BlockGasMeter().PastLimit()) + } else { + require.Equal(t, res.Code, sdk.ToABCICode(sdk.CodespaceRoot, sdk.CodeOutOfGas), fmt.Sprintf("%d: %v, %v", i, tc, res)) + require.True(t, ctx.BlockGasMeter().PastLimit()) + } + } +} From 7e6fcc0161a549752ff9501480a9a5b01dbfc283 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Tue, 13 Nov 2018 16:01:00 -0500 Subject: [PATCH 08/27] passing test --- baseapp/baseapp_test.go | 55 ++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 53c34334d..56349ebaa 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -872,19 +872,20 @@ func TestMaxBlockGasLimits(t *testing.T) { app.SetMaximumBlockGas(100) testCases := []struct { - tx *txTest - numDelivers int - blockGasUsed int64 - fail bool + tx *txTest + numDelivers int + gasUsedPerDeliver int64 + fail bool + failAfterDeliver int }{ - {newTxCounter(0, 0), 0, 0, false}, - {newTxCounter(9, 1), 2, 20, false}, - {newTxCounter(10, 0), 3, 30, false}, - {newTxCounter(10, 0), 10, 100, false}, - {newTxCounter(2, 7), 11, 99, false}, + {newTxCounter(0, 0), 0, 0, false, 0}, + {newTxCounter(9, 1), 2, 10, false, 0}, + {newTxCounter(10, 0), 3, 10, false, 0}, + {newTxCounter(10, 0), 10, 10, false, 0}, + {newTxCounter(2, 7), 11, 9, false, 0}, - {newTxCounter(2, 7), 12, 108, true}, - {newTxCounter(10, 0), 11, 110, true}, + {newTxCounter(10, 0), 11, 10, true, 10}, + {newTxCounter(10, 0), 15, 10, true, 10}, } for i, tc := range testCases { @@ -894,25 +895,27 @@ func TestMaxBlockGasLimits(t *testing.T) { app.BeginBlock(abci.RequestBeginBlock{}) // execute the transaction multiple times - for j := 0; j < numDelivers; j++ { + for j := 0; j < tc.numDelivers; j++ { res := app.Deliver(tx) - } - ctx := app.getContextForAnte(runTxModeDeliver, tx) - ctx = app.initializeContext(ctx, runTxModeDeliver) - blockGasUsed := ctx.BlockGasMeter().ConsumedGas() + ctx := app.getContextForAnte(runTxModeDeliver, nil) + ctx = app.initializeContext(ctx, runTxModeDeliver) + blockGasUsed := ctx.BlockGasMeter().GasConsumed() - // check gas used and wanted - require.Equal(t, tc.blockGasUsed, blockGasUsed, - fmt.Sprintf("%d: %v, %v, %v", i, tc, blockGasUsed, res)) + // check for failed transactions + if tc.fail && (j+1) > tc.failAfterDeliver { + require.Equal(t, res.Code, sdk.ToABCICode(sdk.CodespaceRoot, sdk.CodeOutOfGas), fmt.Sprintf("%d: %v, %v", i, tc, res)) + require.True(t, ctx.BlockGasMeter().PastLimit()) + } else { - // check for out of gas - if !tc.fail { - require.True(t, res.IsOK(), fmt.Sprintf("%d: %v, %v", i, tc, res)) - require.False(t, ctx.BlockGasMeter().PastLimit()) - } else { - require.Equal(t, res.Code, sdk.ToABCICode(sdk.CodespaceRoot, sdk.CodeOutOfGas), fmt.Sprintf("%d: %v, %v", i, tc, res)) - require.True(t, ctx.BlockGasMeter().PastLimit()) + // check gas used and wanted + expBlockGasUsed := tc.gasUsedPerDeliver * int64(j+1) + require.Equal(t, expBlockGasUsed, blockGasUsed, + fmt.Sprintf("%d,%d: %v, %v, %v, %v", i, j, tc, expBlockGasUsed, blockGasUsed, res)) + + require.True(t, res.IsOK(), fmt.Sprintf("%d,%d: %v, %v", i, j, tc, res)) + require.False(t, ctx.BlockGasMeter().PastLimit()) + } } } } From 68e3b9a55908a4ee9e5b3ad4e7d6dbaf89afe6aa Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Wed, 14 Nov 2018 00:57:27 -0500 Subject: [PATCH 09/27] only use block gas for deliver --- baseapp/baseapp.go | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index e661c904f..17ed869c4 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -264,6 +264,15 @@ func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitC } res = app.initChainer(app.deliverState.ctx, req) + // add block gas meter + if app.maximumBlockGas > 0 { + app.deliverState.ctx = app.deliverState.ctx. + WithBlockGasMeter(sdk.NewGasMeter(app.maximumBlockGas)) + } else { + app.deliverState.ctx = app.deliverState.ctx. + WithBlockGasMeter(sdk.NewInfiniteGasMeter()) + } + // NOTE: we don't commit, but BeginBlock for block 1 // starts from this deliverState return @@ -434,15 +443,15 @@ func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeg app.deliverState.ctx = app.deliverState.ctx. WithBlockHeader(req.Header). WithBlockHeight(req.Header.Height) - } - // add block gas meter - if app.maximumBlockGas > 0 { - app.deliverState.ctx = app.deliverState.ctx. - WithBlockGasMeter(sdk.NewGasMeter(app.maximumBlockGas)) - } else { - app.deliverState.ctx = app.deliverState.ctx. - WithBlockGasMeter(sdk.NewInfiniteGasMeter()) + // add block gas meter + if app.maximumBlockGas > 0 { + app.deliverState.ctx = app.deliverState.ctx. + WithBlockGasMeter(sdk.NewGasMeter(app.maximumBlockGas)) + } else { + app.deliverState.ctx = app.deliverState.ctx. + WithBlockGasMeter(sdk.NewInfiniteGasMeter()) + } } if app.beginBlocker != nil { @@ -619,7 +628,7 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk ctx = app.initializeContext(ctx, mode) // only run the tx if there is block gas remaining - if ctx.BlockGasMeter().PastLimit() { + if mode == runTxModeDeliver && ctx.BlockGasMeter().PastLimit() { result = sdk.ErrOutOfGas("no block gas left to run tx").Result() return } @@ -678,8 +687,10 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk result.GasWanted = gasWanted // consume block gas - ctx.BlockGasMeter().ConsumeGas( - ctx.GasMeter().GasConsumed(), "block gas meter") + if mode == runTxModeDeliver { + ctx.BlockGasMeter().ConsumeGas( + ctx.GasMeter().GasConsumed(), "block gas meter") + } // only update state if all messages pass if result.IsOK() { From 0d4dd8762bb8bba044aaec99684b38263d8f3cb9 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Wed, 14 Nov 2018 14:07:46 -0500 Subject: [PATCH 10/27] fix baseapp tests --- baseapp/baseapp.go | 25 ++++++++----------------- baseapp/baseapp_test.go | 1 + 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 17ed869c4..5e3624bb7 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -264,15 +264,6 @@ func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitC } res = app.initChainer(app.deliverState.ctx, req) - // add block gas meter - if app.maximumBlockGas > 0 { - app.deliverState.ctx = app.deliverState.ctx. - WithBlockGasMeter(sdk.NewGasMeter(app.maximumBlockGas)) - } else { - app.deliverState.ctx = app.deliverState.ctx. - WithBlockGasMeter(sdk.NewInfiniteGasMeter()) - } - // NOTE: we don't commit, but BeginBlock for block 1 // starts from this deliverState return @@ -443,15 +434,15 @@ func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeg app.deliverState.ctx = app.deliverState.ctx. WithBlockHeader(req.Header). WithBlockHeight(req.Header.Height) + } - // add block gas meter - if app.maximumBlockGas > 0 { - app.deliverState.ctx = app.deliverState.ctx. - WithBlockGasMeter(sdk.NewGasMeter(app.maximumBlockGas)) - } else { - app.deliverState.ctx = app.deliverState.ctx. - WithBlockGasMeter(sdk.NewInfiniteGasMeter()) - } + // add block gas meter + if app.maximumBlockGas > 0 { + app.deliverState.ctx = app.deliverState.ctx. + WithBlockGasMeter(sdk.NewGasMeter(app.maximumBlockGas)) + } else { + app.deliverState.ctx = app.deliverState.ctx. + WithBlockGasMeter(sdk.NewInfiniteGasMeter()) } if app.beginBlocker != nil { diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 56349ebaa..6a4bfefcf 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -491,6 +491,7 @@ func TestDeliverTx(t *testing.T) { } app := setupBaseApp(t, anteOpt, routerOpt) + app.InitChain(abci.RequestInitChain{}) // Create same codec used in txDecoder codec := codec.New() From 524906478ab5f906b772218a4b3954cb35f551ff Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Wed, 14 Nov 2018 14:16:52 -0500 Subject: [PATCH 11/27] add init chain block gas for gen-txs (all unit tests fixed) --- baseapp/baseapp.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 5e3624bb7..7e35fb5f6 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -262,6 +262,11 @@ func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitC if app.initChainer == nil { return } + + // add block gas meter for any genesis transactions (allow infinite gas) + app.deliverState.ctx = app.deliverState.ctx. + WithBlockGasMeter(sdk.NewInfiniteGasMeter()) + res = app.initChainer(app.deliverState.ctx, req) // NOTE: we don't commit, but BeginBlock for block 1 From 90217e23134f588ed0bd7c5308ff209346a05d2b Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Wed, 14 Nov 2018 15:14:34 -0500 Subject: [PATCH 12/27] docs and pending --- PENDING.md | 3 +- docs/spec/baseapp/WIP_abci_application.md | 46 +++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 docs/spec/baseapp/WIP_abci_application.md diff --git a/PENDING.md b/PENDING.md index 52bf8a7ee..49a66c79d 100644 --- a/PENDING.md +++ b/PENDING.md @@ -33,7 +33,8 @@ FEATURES for getting governance parameters. * SDK - * [simulator] \#2682 MsgEditValidator now looks at the validator's max rate, thus it now succeeds a significant portion of the time + * [simulator] \#2682 MsgEditValidator now looks at the validator's max rate, thus it now succeeds a significant portion of the time + * [core] \#2775 Add deliverTx maximum block gas limit * Tendermint diff --git a/docs/spec/baseapp/WIP_abci_application.md b/docs/spec/baseapp/WIP_abci_application.md new file mode 100644 index 000000000..52f0ab18d --- /dev/null +++ b/docs/spec/baseapp/WIP_abci_application.md @@ -0,0 +1,46 @@ +# ABCI application + +The `BaseApp` struct fulfills the tendermint-abci `Application` interface. + +## Info + +## SetOption + +## Query + +## CheckTx + +## InitChain +TODO pseudo code + +During chain initialization InitChain runs the initialization logic directly on +the CommitMultiStore and commits it. The deliver and check states are +initialized with the ChainID. Additionally the Block gas meter is initialized +with an infinite amount of gas to run any genesis transactions. + +Note that we do not `Commit` during `InitChain` however BeginBlock for block 1 +starts from this deliverState. + + +## BeginBlock +TODO complete description & pseudo code + +The block gas meter is reset within BeginBlock for the deliver state. +If no maximum block gas is set within baseapp then an infinite +gas meter is set, otherwise a gas meter with the baseapp `maximumBlockGas` +is initialized + +## DeliverTx +TODO complete description & pseudo code + +Before transaction logic is run, the `BlockGasMeter` is first checked for +remaining gas. If no gas remains, then `DeliverTx` immediately returns an error. + +After the transaction has been processed the used gas is deducted from the +BlockGasMeter. If the remaining gas exceeds the meter's limits, then DeliverTx +returns an error and the transaction is not committed. + +## EndBlock + +## Commit + From 2a594fe3381b3b990c08da91fa2848f7f71a5845 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Thu, 15 Nov 2018 11:13:18 -0500 Subject: [PATCH 13/27] basic cwgoes comments --- baseapp/baseapp.go | 10 +++++----- docs/spec/baseapp/WIP_abci_application.md | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 7e35fb5f6..2cd7f562b 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -253,7 +253,7 @@ func (app *BaseApp) SetOption(req abci.RequestSetOption) (res abci.ResponseSetOp } // Implements ABCI -// InitChain runs the initialization logic directly on the CommitMultiStore and commits it. +// InitChain runs the initialization logic directly on the CommitMultiStore. func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitChain) { // Initialize the deliver state and check state with ChainID and run initChain app.setDeliverState(abci.Header{ChainID: req.ChainId}) @@ -442,13 +442,13 @@ func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeg } // add block gas meter + var gasMeter sdk.GasMeter if app.maximumBlockGas > 0 { - app.deliverState.ctx = app.deliverState.ctx. - WithBlockGasMeter(sdk.NewGasMeter(app.maximumBlockGas)) + gasMeter = sdk.NewGasMeter(app.maximumBlockGas) } else { - app.deliverState.ctx = app.deliverState.ctx. - WithBlockGasMeter(sdk.NewInfiniteGasMeter()) + gasMeter = sdk.NewInfiniteGasMeter() } + app.deliverState.ctx = app.deliverState.ctx.WithBlockGasMeter(gasMeter) if app.beginBlocker != nil { res = app.beginBlocker(app.deliverState.ctx, req) diff --git a/docs/spec/baseapp/WIP_abci_application.md b/docs/spec/baseapp/WIP_abci_application.md index 52f0ab18d..7baadccee 100644 --- a/docs/spec/baseapp/WIP_abci_application.md +++ b/docs/spec/baseapp/WIP_abci_application.md @@ -14,9 +14,9 @@ The `BaseApp` struct fulfills the tendermint-abci `Application` interface. TODO pseudo code During chain initialization InitChain runs the initialization logic directly on -the CommitMultiStore and commits it. The deliver and check states are -initialized with the ChainID. Additionally the Block gas meter is initialized -with an infinite amount of gas to run any genesis transactions. +the CommitMultiStore. The deliver and check states are initialized with the +ChainID. Additionally the block gas meter is initialized with an infinite +amount of gas to run any genesis transactions. Note that we do not `Commit` during `InitChain` however BeginBlock for block 1 starts from this deliverState. From d911565d0bb89a5996c0adf0dda21e74dca3e7fc Mon Sep 17 00:00:00 2001 From: Jae Kwon Date: Tue, 20 Nov 2018 13:16:44 -0800 Subject: [PATCH 14/27] Fix compile --- baseapp/baseapp.go | 4 ++-- baseapp/baseapp_test.go | 13 +++++++------ baseapp/options.go | 2 +- cmd/gaia/cmd/gaiad/main.go | 2 +- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 7cd4d4857..34780cfbb 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -68,7 +68,7 @@ type BaseApp struct { // spam prevention minimumFees sdk.Coins - maximumBlockGas int64 + maximumBlockGas uint64 // flag for sealing sealed bool @@ -185,7 +185,7 @@ func (app *BaseApp) initFromStore(mainKey sdk.StoreKey) error { func (app *BaseApp) SetMinimumFees(fees sdk.Coins) { app.minimumFees = fees } // SetMaximumBlockGas sets the maximum gas allowable per block. -func (app *BaseApp) SetMaximumBlockGas(gas int64) { app.maximumBlockGas = gas } +func (app *BaseApp) SetMaximumBlockGas(gas uint64) { app.maximumBlockGas = gas } // NewContext returns a new Context with the correct store, the given header, and nil txBytes. func (app *BaseApp) NewContext(isCheckTx bool, header abci.Header) sdk.Context { diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 27e5f8280..8ea5cbb89 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -855,7 +855,7 @@ func TestTxGasLimits(t *testing.T) { // Test that transactions exceeding gas limits fail func TestMaxBlockGasLimits(t *testing.T) { - gasGranted := int64(10) + gasGranted := uint64(10) anteOpt := func(bapp *BaseApp) { bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, res sdk.Result, abort bool) { newCtx = ctx.WithGasMeter(sdk.NewGasMeter(gasGranted)) @@ -880,7 +880,7 @@ func TestMaxBlockGasLimits(t *testing.T) { }() count := tx.(*txTest).Counter - newCtx.GasMeter().ConsumeGas(count, "counter-ante") + newCtx.GasMeter().ConsumeGas(uint64(count), "counter-ante") res = sdk.Result{ GasWanted: gasGranted, } @@ -892,7 +892,7 @@ func TestMaxBlockGasLimits(t *testing.T) { routerOpt := func(bapp *BaseApp) { bapp.Router().AddRoute(routeMsgCounter, func(ctx sdk.Context, msg sdk.Msg) sdk.Result { count := msg.(msgCounter).Counter - ctx.GasMeter().ConsumeGas(count, "counter-handler") + ctx.GasMeter().ConsumeGas(uint64(count), "counter-handler") return sdk.Result{} }) } @@ -903,7 +903,7 @@ func TestMaxBlockGasLimits(t *testing.T) { testCases := []struct { tx *txTest numDelivers int - gasUsedPerDeliver int64 + gasUsedPerDeliver uint64 fail bool failAfterDeliver int }{ @@ -933,12 +933,13 @@ func TestMaxBlockGasLimits(t *testing.T) { // check for failed transactions if tc.fail && (j+1) > tc.failAfterDeliver { - require.Equal(t, res.Code, sdk.ToABCICode(sdk.CodespaceRoot, sdk.CodeOutOfGas), fmt.Sprintf("%d: %v, %v", i, tc, res)) + require.Equal(t, res.Code, sdk.CodeOutOfGas, fmt.Sprintf("%d: %v, %v", i, tc, res)) + require.Equal(t, res.Codespace, sdk.CodespaceRoot, fmt.Sprintf("%d: %v, %v", i, tc, res)) require.True(t, ctx.BlockGasMeter().PastLimit()) } else { // check gas used and wanted - expBlockGasUsed := tc.gasUsedPerDeliver * int64(j+1) + expBlockGasUsed := tc.gasUsedPerDeliver * uint64(j+1) require.Equal(t, expBlockGasUsed, blockGasUsed, fmt.Sprintf("%d,%d: %v, %v, %v, %v", i, j, tc, expBlockGasUsed, blockGasUsed, res)) diff --git a/baseapp/options.go b/baseapp/options.go index 8d6131314..7fd95aec8 100644 --- a/baseapp/options.go +++ b/baseapp/options.go @@ -40,7 +40,7 @@ func SetMinimumFees(minFees string) func(*BaseApp) { } // SetMinimumFees returns an option that sets the minimum fees on the app. -func SetMaximumBlockGas(gas int64) func(*BaseApp) { +func SetMaximumBlockGas(gas uint64) func(*BaseApp) { return func(bap *BaseApp) { bap.SetMaximumBlockGas(gas) } } diff --git a/cmd/gaia/cmd/gaiad/main.go b/cmd/gaia/cmd/gaiad/main.go index 4ae6f57d5..ecb906bc7 100644 --- a/cmd/gaia/cmd/gaiad/main.go +++ b/cmd/gaia/cmd/gaiad/main.go @@ -63,7 +63,7 @@ func newApp(logger log.Logger, db dbm.DB, if err != nil { panic(err) } - maxBlockGas := genDoc.ConsensusParams.BlockSize.MaxGas + maxBlockGas := uint64(genDoc.ConsensusParams.BlockSize.MaxGas) return app.NewGaiaApp(logger, db, traceStore, baseapp.SetPruning(viper.GetString("pruning")), From 10bdf8fa037439006189f1ea917c1f8f754197a4 Mon Sep 17 00:00:00 2001 From: Jae Kwon Date: Tue, 20 Nov 2018 16:44:49 -0800 Subject: [PATCH 15/27] Store ConsensusParams to main store --- baseapp/baseapp.go | 107 +++++++++++++------ baseapp/baseapp_test.go | 12 ++- baseapp/options.go | 5 - cmd/gaia/cmd/gaiad/main.go | 13 +-- docs/examples/basecoin/cmd/basecoind/main.go | 3 +- docs/examples/democoin/cmd/democoind/main.go | 5 +- server/constructors.go | 4 +- server/start.go | 9 +- store/rootmultistore.go | 8 +- types/store.go | 1 + 10 files changed, 98 insertions(+), 69 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 34780cfbb..19a5d6dab 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -6,6 +6,7 @@ import ( "runtime/debug" "strings" + "github.com/gogo/protobuf/proto" "github.com/pkg/errors" abci "github.com/tendermint/tendermint/abci/types" @@ -19,11 +20,8 @@ import ( "github.com/cosmos/cosmos-sdk/version" ) -// Key to store the header in the DB itself. -// Use the db directly instead of a store to avoid -// conflicts with handlers writing to the store -// and to avoid affecting the Merkle root. -var dbHeaderKey = []byte("header") +// Key to store the consensus params in the main store. +var mainConsensusParamsKey = []byte("consensus_params") // Enum mode for app.runTx type runTxMode uint8 @@ -48,9 +46,11 @@ type BaseApp struct { queryRouter QueryRouter // router for redirecting query calls txDecoder sdk.TxDecoder // unmarshal []byte into sdk.Tx - anteHandler sdk.AnteHandler // ante handler for fee and auth + // set upon LoadVersion or LoadLatestVersion. + mainKey *sdk.KVStoreKey // Main KVStore in cms // may be nil + anteHandler sdk.AnteHandler // ante handler for fee and auth initChainer sdk.InitChainer // initialize state with validators and state blob beginBlocker sdk.BeginBlocker // logic to run before any txs endBlocker sdk.EndBlocker // logic to run after all txs, and to determine valset changes @@ -66,9 +66,12 @@ type BaseApp struct { deliverState *state // for DeliverTx voteInfos []abci.VoteInfo // absent validators from begin block + // consensus params + // TODO move this in the future to baseapp param store on main store. + consensusParams *abci.ConsensusParams + // spam prevention - minimumFees sdk.Coins - maximumBlockGas uint64 + minimumFees sdk.Coins // flag for sealing sealed bool @@ -78,10 +81,6 @@ var _ abci.Application = (*BaseApp)(nil) // NewBaseApp returns a reference to an initialized BaseApp. // -// TODO: Determine how to use a flexible and robust configuration paradigm that -// allows for sensible defaults while being highly configurable -// (e.g. functional options). -// // NOTE: The db is used to store the version number for now. // Accepts a user-defined txDecoder // Accepts variable number of option functions, which act on the BaseApp to set configuration choices @@ -95,7 +94,6 @@ func NewBaseApp(name string, logger log.Logger, db dbm.DB, txDecoder sdk.TxDecod queryRouter: NewQueryRouter(), txDecoder: txDecoder, } - for _, option := range options { option(app) } @@ -138,21 +136,21 @@ func (app *BaseApp) MountStore(key sdk.StoreKey, typ sdk.StoreType) { } // load latest application version -func (app *BaseApp) LoadLatestVersion(mainKey sdk.StoreKey) error { +func (app *BaseApp) LoadLatestVersion(mainKey *sdk.KVStoreKey) error { err := app.cms.LoadLatestVersion() if err != nil { return err } - return app.initFromStore(mainKey) + return app.initFromMainStore(mainKey) } // load application version -func (app *BaseApp) LoadVersion(version int64, mainKey sdk.StoreKey) error { +func (app *BaseApp) LoadVersion(version int64, mainKey *sdk.KVStoreKey) error { err := app.cms.LoadVersion(version) if err != nil { return err } - return app.initFromStore(mainKey) + return app.initFromMainStore(mainKey) } // the last CommitID of the multistore @@ -166,13 +164,34 @@ func (app *BaseApp) LastBlockHeight() int64 { } // initializes the remaining logic from app.cms -func (app *BaseApp) initFromStore(mainKey sdk.StoreKey) error { +func (app *BaseApp) initFromMainStore(mainKey *sdk.KVStoreKey) error { + // main store should exist. - // TODO: we don't actually need the main store here - main := app.cms.GetKVStore(mainKey) - if main == nil { + mainStore := app.cms.GetKVStore(mainKey) + if mainStore == nil { return errors.New("baseapp expects MultiStore with 'main' KVStore") } + + // memoize mainKey. + if app.mainKey != nil { + panic("app.mainKey expected to be nil; duplicate init?") + } + app.mainKey = mainKey + + // load consensus param from the main store + consensusParamsBz := mainStore.Get(mainConsensusParamsKey) + if consensusParamsBz != nil { + var consensusParams = &abci.ConsensusParams{} + err := proto.Unmarshal(consensusParamsBz, consensusParams) + if err != nil { + panic(err) + } + app.setConsensusParams(consensusParams) + } else { + // It will get saved later during InitChain. + // TODO assert that InitChain hasn't yet been called. + } + // Needed for `gaiad export`, which inits from store but never calls initchain app.setCheckState(abci.Header{}) @@ -184,9 +203,6 @@ func (app *BaseApp) initFromStore(mainKey sdk.StoreKey) error { // SetMinimumFees sets the minimum fees. func (app *BaseApp) SetMinimumFees(fees sdk.Coins) { app.minimumFees = fees } -// SetMaximumBlockGas sets the maximum gas allowable per block. -func (app *BaseApp) SetMaximumBlockGas(gas uint64) { app.maximumBlockGas = gas } - // NewContext returns a new Context with the correct store, the given header, and nil txBytes. func (app *BaseApp) NewContext(isCheckTx bool, header abci.Header) sdk.Context { if isCheckTx { @@ -220,6 +236,30 @@ func (app *BaseApp) setDeliverState(header abci.Header) { } } +// setConsensusParams memoizes the consensus params. +func (app *BaseApp) setConsensusParams(consensusParams *abci.ConsensusParams) { + app.consensusParams = consensusParams +} + +// setConsensusParams stores the consensus params to the main store. +func (app *BaseApp) storeConsensusParams(consensusParams *abci.ConsensusParams) { + consensusParamsBz, err := proto.Marshal(consensusParams) + if err != nil { + panic(err) + } + mainStore := app.cms.GetKVStore(app.mainKey) + mainStore.Set(mainConsensusParamsKey, consensusParamsBz) +} + +// getMaximumBlockGas gets the maximum gas from the consensus params. +func (app *BaseApp) getMaximumBlockGas() (maxGas uint64) { + if app.consensusParams == nil || app.consensusParams.BlockSize == nil { + return 0 + } else { + return uint64(app.consensusParams.BlockSize.MaxGas) + } +} + //______________________________________________________________________________ // ABCI @@ -244,6 +284,13 @@ func (app *BaseApp) SetOption(req abci.RequestSetOption) (res abci.ResponseSetOp // Implements ABCI // InitChain runs the initialization logic directly on the CommitMultiStore. func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitChain) { + + // Stash the consensus params in the cms main store and memoize. + if req.ConsensusParams != nil { + app.setConsensusParams(req.ConsensusParams) + app.storeConsensusParams(req.ConsensusParams) + } + // Initialize the deliver state and check state with ChainID and run initChain app.setDeliverState(abci.Header{ChainID: req.ChainId}) app.setCheckState(abci.Header{ChainID: req.ChainId}) @@ -435,8 +482,8 @@ func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeg // add block gas meter var gasMeter sdk.GasMeter - if app.maximumBlockGas > 0 { - gasMeter = sdk.NewGasMeter(app.maximumBlockGas) + if maxGas := app.getMaximumBlockGas(); maxGas > 0 { + gasMeter = sdk.NewGasMeter(maxGas) } else { gasMeter = sdk.NewInfiniteGasMeter() } @@ -733,14 +780,6 @@ func (app *BaseApp) EndBlock(req abci.RequestEndBlock) (res abci.ResponseEndBloc // Implements ABCI func (app *BaseApp) Commit() (res abci.ResponseCommit) { header := app.deliverState.ctx.BlockHeader() - /* - // Write the latest Header to the store - headerBytes, err := proto.Marshal(&header) - if err != nil { - panic(err) - } - app.db.SetSync(dbHeaderKey, headerBytes) - */ // Write the Deliver state and commit the MultiStore app.deliverState.ms.Write() diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 8ea5cbb89..83cecee08 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -56,7 +56,9 @@ func setupBaseApp(t *testing.T, options ...func(*BaseApp)) *BaseApp { require.Equal(t, t.Name(), app.Name()) // no stores are mounted - require.Panics(t, func() { app.LoadLatestVersion(capKey1) }) + require.Panics(t, func() { + app.LoadLatestVersion(capKey1) + }) app.MountStoresIAVL(capKey1, capKey2) @@ -898,7 +900,13 @@ func TestMaxBlockGasLimits(t *testing.T) { } app := setupBaseApp(t, anteOpt, routerOpt) - app.SetMaximumBlockGas(100) + app.InitChain(abci.RequestInitChain{ + ConsensusParams: &abci.ConsensusParams{ + BlockSize: &abci.BlockSizeParams{ + MaxGas: 100, + }, + }, + }) testCases := []struct { tx *txTest diff --git a/baseapp/options.go b/baseapp/options.go index 7fd95aec8..a6460248d 100644 --- a/baseapp/options.go +++ b/baseapp/options.go @@ -39,11 +39,6 @@ func SetMinimumFees(minFees string) func(*BaseApp) { return func(bap *BaseApp) { bap.SetMinimumFees(fees) } } -// SetMinimumFees returns an option that sets the minimum fees on the app. -func SetMaximumBlockGas(gas uint64) func(*BaseApp) { - return func(bap *BaseApp) { bap.SetMaximumBlockGas(gas) } -} - func (app *BaseApp) SetName(name string) { if app.sealed { panic("SetName() on sealed BaseApp") diff --git a/cmd/gaia/cmd/gaiad/main.go b/cmd/gaia/cmd/gaiad/main.go index ecb906bc7..f3a8309e0 100644 --- a/cmd/gaia/cmd/gaiad/main.go +++ b/cmd/gaia/cmd/gaiad/main.go @@ -13,7 +13,6 @@ import ( "github.com/tendermint/tendermint/libs/cli" dbm "github.com/tendermint/tendermint/libs/db" "github.com/tendermint/tendermint/libs/log" - "github.com/tendermint/tendermint/node" tmtypes "github.com/tendermint/tendermint/types" "github.com/cosmos/cosmos-sdk/cmd/gaia/app" @@ -55,20 +54,10 @@ func main() { } } -func newApp(logger log.Logger, db dbm.DB, - traceStore io.Writer, genDocProvider node.GenesisDocProvider) abci.Application { - - // get the maximum gas from tendermint genesis parameters - genDoc, err := genDocProvider() - if err != nil { - panic(err) - } - maxBlockGas := uint64(genDoc.ConsensusParams.BlockSize.MaxGas) - +func newApp(logger log.Logger, db dbm.DB, traceStore io.Writer) abci.Application { return app.NewGaiaApp(logger, db, traceStore, baseapp.SetPruning(viper.GetString("pruning")), baseapp.SetMinimumFees(viper.GetString("minimum_fees")), - baseapp.SetMaximumBlockGas(maxBlockGas), ) } diff --git a/docs/examples/basecoin/cmd/basecoind/main.go b/docs/examples/basecoin/cmd/basecoind/main.go index 3f257c495..318b36a8f 100644 --- a/docs/examples/basecoin/cmd/basecoind/main.go +++ b/docs/examples/basecoin/cmd/basecoind/main.go @@ -6,7 +6,6 @@ import ( "io" "os" - "github.com/tendermint/tendermint/node" "github.com/tendermint/tendermint/p2p" "github.com/cosmos/cosmos-sdk/baseapp" @@ -121,7 +120,7 @@ func InitCmd(ctx *server.Context, cdc *codec.Codec) *cobra.Command { return cmd } -func newApp(logger log.Logger, db dbm.DB, storeTracer io.Writer, _ node.GenesisDocProvider) abci.Application { +func newApp(logger log.Logger, db dbm.DB, storeTracer io.Writer) abci.Application { return app.NewBasecoinApp(logger, db, baseapp.SetPruning(viper.GetString("pruning"))) } diff --git a/docs/examples/democoin/cmd/democoind/main.go b/docs/examples/democoin/cmd/democoind/main.go index a4feef77c..730109798 100644 --- a/docs/examples/democoin/cmd/democoind/main.go +++ b/docs/examples/democoin/cmd/democoind/main.go @@ -9,7 +9,6 @@ import ( "github.com/cosmos/cosmos-sdk/client" "github.com/spf13/viper" "github.com/tendermint/tendermint/libs/common" - "github.com/tendermint/tendermint/node" "github.com/tendermint/tendermint/p2p" "github.com/spf13/cobra" @@ -125,9 +124,7 @@ func InitCmd(ctx *server.Context, cdc *codec.Codec) *cobra.Command { return cmd } -func newApp(logger log.Logger, db dbm.DB, _ io.Writer, - _ node.GenesisDocProvider) abci.Application { - +func newApp(logger log.Logger, db dbm.DB, _ io.Writer) abci.Application { return app.NewDemocoinApp(logger, db) } diff --git a/server/constructors.go b/server/constructors.go index 4e62239b6..9039d8a81 100644 --- a/server/constructors.go +++ b/server/constructors.go @@ -9,15 +9,13 @@ import ( abci "github.com/tendermint/tendermint/abci/types" dbm "github.com/tendermint/tendermint/libs/db" "github.com/tendermint/tendermint/libs/log" - "github.com/tendermint/tendermint/node" tmtypes "github.com/tendermint/tendermint/types" ) type ( // AppCreator is a function that allows us to lazily initialize an // application using various configurations. - AppCreator func(log.Logger, dbm.DB, - io.Writer, node.GenesisDocProvider) abci.Application + AppCreator func(log.Logger, dbm.DB, io.Writer) abci.Application // AppExporter is a function that dumps all app state to // JSON-serializable structure and returns the current validator set. diff --git a/server/start.go b/server/start.go index d84169ab4..cf39ff71b 100644 --- a/server/start.go +++ b/server/start.go @@ -68,9 +68,7 @@ func startStandAlone(ctx *Context, appCreator AppCreator) error { return err } - cfg := ctx.Config - genDocProvider := node.DefaultGenesisDocProviderFunc(cfg) - app := appCreator(ctx.Logger, db, traceWriter, genDocProvider) + app := appCreator(ctx.Logger, db, traceWriter) svr, err := server.NewServer(addr, "socket", app) if err != nil { @@ -109,8 +107,7 @@ func startInProcess(ctx *Context, appCreator AppCreator) (*node.Node, error) { return nil, err } - genDocProvider := node.DefaultGenesisDocProviderFunc(cfg) - app := appCreator(ctx.Logger, db, traceWriter, genDocProvider) + app := appCreator(ctx.Logger, db, traceWriter) nodeKey, err := p2p.LoadOrGenNodeKey(cfg.NodeKeyFile()) if err != nil { @@ -123,7 +120,7 @@ func startInProcess(ctx *Context, appCreator AppCreator) (*node.Node, error) { pvm.LoadOrGenFilePV(cfg.PrivValidatorFile()), nodeKey, proxy.NewLocalClientCreator(app), - genDocProvider, + node.DefaultGenesisDocProviderFunc(cfg), node.DefaultDBProvider, node.DefaultMetricsProvider(cfg.Instrumentation), ctx.Logger.With("module", "node"), diff --git a/store/rootmultistore.go b/store/rootmultistore.go index cd2d0135f..3faf67a5e 100644 --- a/store/rootmultistore.go +++ b/store/rootmultistore.go @@ -230,13 +230,19 @@ func (rs *rootMultiStore) CacheMultiStore() CacheMultiStore { } // Implements MultiStore. +// If the store does not exist, panics. func (rs *rootMultiStore) GetStore(key StoreKey) Store { - return rs.stores[key] + store := rs.stores[key] + if store == nil { + panic("Could not load store " + key.String()) + } + return store } // GetKVStore implements the MultiStore interface. If tracing is enabled on the // rootMultiStore, a wrapped TraceKVStore will be returned with the given // tracer, otherwise, the original KVStore will be returned. +// If the store does not exist, panics. func (rs *rootMultiStore) GetKVStore(key StoreKey) KVStore { store := rs.stores[key].(KVStore) diff --git a/types/store.go b/types/store.go index 8fe0321f5..c2e57a342 100644 --- a/types/store.go +++ b/types/store.go @@ -64,6 +64,7 @@ type MultiStore interface { //nolint CacheMultiStore() CacheMultiStore // Convenience for fetching substores. + // If the store does not exist, panics. GetStore(StoreKey) Store GetKVStore(StoreKey) KVStore From 4afd53d81b515f4c3b0ac4faf6a2590fe1fcf3e6 Mon Sep 17 00:00:00 2001 From: Jae Kwon Date: Tue, 20 Nov 2018 20:07:30 -0800 Subject: [PATCH 16/27] Consume block gas to tx gas limit even upon overconsumption --- baseapp/baseapp.go | 14 +++++++------- baseapp/baseapp_test.go | 6 +++--- types/gas.go | 37 ++++++++++++++++++++++++++++++++++--- types/gas_test.go | 9 +++++++++ 4 files changed, 53 insertions(+), 13 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 19a5d6dab..136099d0d 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -688,7 +688,7 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk ctx = app.initializeContext(ctx, mode) // only run the tx if there is block gas remaining - if mode == runTxModeDeliver && ctx.BlockGasMeter().PastLimit() { + if mode == runTxModeDeliver && ctx.BlockGasMeter().IsOutOfGas() { result = sdk.ErrOutOfGas("no block gas left to run tx").Result() return } @@ -705,6 +705,12 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk } } + // consume block gas whether panic or not. + if mode == runTxModeDeliver { + ctx.BlockGasMeter().ConsumeGas( + ctx.GasMeter().GasConsumedToLimit(), "block gas meter") + } + result.GasWanted = gasWanted result.GasUsed = ctx.GasMeter().GasConsumed() }() @@ -750,12 +756,6 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk result = app.runMsgs(runMsgCtx, msgs, mode) result.GasWanted = gasWanted - // consume block gas - if mode == runTxModeDeliver { - ctx.BlockGasMeter().ConsumeGas( - ctx.GasMeter().GasConsumed(), "block gas meter") - } - // only update state if all messages pass if result.IsOK() { msCache.Write() diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 83cecee08..3dd997150 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -943,16 +943,16 @@ func TestMaxBlockGasLimits(t *testing.T) { if tc.fail && (j+1) > tc.failAfterDeliver { require.Equal(t, res.Code, sdk.CodeOutOfGas, fmt.Sprintf("%d: %v, %v", i, tc, res)) require.Equal(t, res.Codespace, sdk.CodespaceRoot, fmt.Sprintf("%d: %v, %v", i, tc, res)) - require.True(t, ctx.BlockGasMeter().PastLimit()) + //require.True(t, ctx.BlockGasMeter().IsPastLimit()) NOTE: not necessarily true. + require.True(t, ctx.BlockGasMeter().IsOutOfGas()) } else { - // check gas used and wanted expBlockGasUsed := tc.gasUsedPerDeliver * uint64(j+1) require.Equal(t, expBlockGasUsed, blockGasUsed, fmt.Sprintf("%d,%d: %v, %v, %v, %v", i, j, tc, expBlockGasUsed, blockGasUsed, res)) require.True(t, res.IsOK(), fmt.Sprintf("%d,%d: %v, %v", i, j, tc, res)) - require.False(t, ctx.BlockGasMeter().PastLimit()) + require.False(t, ctx.BlockGasMeter().IsPastLimit()) } } } diff --git a/types/gas.go b/types/gas.go index d9bd7de40..be9a55b77 100644 --- a/types/gas.go +++ b/types/gas.go @@ -34,8 +34,11 @@ type ErrorGasOverflow struct { // GasMeter interface to track gas consumption type GasMeter interface { GasConsumed() Gas + GasConsumedToLimit() Gas + Limit() Gas ConsumeGas(amount Gas, descriptor string) - PastLimit() bool + IsPastLimit() bool + IsOutOfGas() bool } type basicGasMeter struct { @@ -55,6 +58,18 @@ func (g *basicGasMeter) GasConsumed() Gas { return g.consumed } +func (g *basicGasMeter) Limit() Gas { + return g.limit +} + +func (g *basicGasMeter) GasConsumedToLimit() Gas { + if g.consumed > g.limit { + return g.limit + } else { + return g.consumed + } +} + func (g *basicGasMeter) ConsumeGas(amount Gas, descriptor string) { var overflow bool @@ -69,10 +84,14 @@ func (g *basicGasMeter) ConsumeGas(amount Gas, descriptor string) { } } -func (g *basicGasMeter) PastLimit() bool { +func (g *basicGasMeter) IsPastLimit() bool { return g.consumed > g.limit } +func (g *basicGasMeter) IsOutOfGas() bool { + return g.consumed >= g.limit +} + type infiniteGasMeter struct { consumed Gas } @@ -88,6 +107,14 @@ func (g *infiniteGasMeter) GasConsumed() Gas { return g.consumed } +func (g *infiniteGasMeter) GasConsumedToLimit() Gas { + return g.consumed +} + +func (g *infiniteGasMeter) Limit() Gas { + return 0 +} + func (g *infiniteGasMeter) ConsumeGas(amount Gas, descriptor string) { var overflow bool @@ -98,7 +125,11 @@ func (g *infiniteGasMeter) ConsumeGas(amount Gas, descriptor string) { } } -func (g *infiniteGasMeter) PastLimit() bool { +func (g *infiniteGasMeter) IsPastLimit() bool { + return false +} + +func (g *infiniteGasMeter) IsOutOfGas() bool { return false } diff --git a/types/gas_test.go b/types/gas_test.go index f4452053f..5f862dccd 100644 --- a/types/gas_test.go +++ b/types/gas_test.go @@ -27,9 +27,18 @@ func TestGasMeter(t *testing.T) { used += usage require.NotPanics(t, func() { meter.ConsumeGas(usage, "") }, "Not exceeded limit but panicked. tc #%d, usage #%d", tcnum, unum) require.Equal(t, used, meter.GasConsumed(), "Gas consumption not match. tc #%d, usage #%d", tcnum, unum) + require.Equal(t, used, meter.GasConsumedToLimit(), "Gas consumption (to limit) not match. tc #%d, usage #%d", tcnum, unum) + require.False(t, meter.IsPastLimit(), "Not exceeded limit but got IsPastLimit() true") + if unum < len(tc.usage)-1 { + require.False(t, meter.IsOutOfGas(), "Not yet at limit but got IsOutOfGas() true") + } else { + require.True(t, meter.IsOutOfGas(), "At limit but got IsOutOfGas() false") + } } require.Panics(t, func() { meter.ConsumeGas(1, "") }, "Exceeded but not panicked. tc #%d", tcnum) + require.Equal(t, meter.GasConsumedToLimit(), meter.Limit(), "Gas consumption (to limit) not match limit") + require.Equal(t, meter.GasConsumed(), meter.Limit()+1, "Gas consumption not match limit+1") break } From bd982b1423aca2c82aee542a777856809c7d841f Mon Sep 17 00:00:00 2001 From: Jae Kwon Date: Tue, 20 Nov 2018 20:23:09 -0800 Subject: [PATCH 17/27] Merge reference/baseapp and spec/baseapp/WIP_abci_application.md --- docs/reference/baseapp.md | 61 +++++++++++++++++++++++ docs/spec/baseapp/WIP_abci_application.md | 46 ----------------- 2 files changed, 61 insertions(+), 46 deletions(-) delete mode 100644 docs/spec/baseapp/WIP_abci_application.md diff --git a/docs/reference/baseapp.md b/docs/reference/baseapp.md index e1a80e293..ad3b56716 100644 --- a/docs/reference/baseapp.md +++ b/docs/reference/baseapp.md @@ -61,3 +61,64 @@ persisted even when the following Handler processing logic fails. It is possible that a malicious proposer may include a transaction in a block that fails the AnteHandler. In this case, all state transitions for the offending transaction are discarded. + + +## Other ABCI Messages + +Besides `CheckTx` and `DeliverTx`, BaseApp handles the following ABCI messages. + +### Info +TODO complete description + +### SetOption +TODO complete description + +### Query +TODO complete description + +### InitChain +TODO complete description + +During chain initialization InitChain runs the initialization logic directly on +the CommitMultiStore. The deliver and check states are initialized with the +ChainID. + +Note that we do not commit after InitChain, so BeginBlock for block 1 starts +from the deliver state as initialized by InitChain. + +### BeginBlock +TODO complete description + +### EndBlock +TODO complete description + +### Commit +TODO complete description + + +## Gas Management + +### Gas: InitChain + +During InitChain, the block gas meter is initialized with an infinite amount of +gas to run any genesis transactions. + +Additionally, the InitChain request message includes ConsensusParams as +declared in the genesis.json file. + +### Gas: BeginBlock + +The block gas meter is reset during BeginBlock for the deliver state. If no +maximum block gas is set within baseapp then an infinite gas meter is set, +otherwise a gas meter with `ConsensusParam.BlockSize.MaxGas` is initialized. + +### Gas: DeliverTx + +Before the transaction logic is run, the `BlockGasMeter` is first checked to +see if any gas remains. If no gas remains, then `DeliverTx` immediately returns +an error. + +After the transaction has been processed, the used gas (up to the transaction +gas limit) is deducted from the BlockGasMeter. If the remaining gas exceeds the +meter's limits, then DeliverTx returns an error and the transaction is not +committed. diff --git a/docs/spec/baseapp/WIP_abci_application.md b/docs/spec/baseapp/WIP_abci_application.md deleted file mode 100644 index 7baadccee..000000000 --- a/docs/spec/baseapp/WIP_abci_application.md +++ /dev/null @@ -1,46 +0,0 @@ -# ABCI application - -The `BaseApp` struct fulfills the tendermint-abci `Application` interface. - -## Info - -## SetOption - -## Query - -## CheckTx - -## InitChain -TODO pseudo code - -During chain initialization InitChain runs the initialization logic directly on -the CommitMultiStore. The deliver and check states are initialized with the -ChainID. Additionally the block gas meter is initialized with an infinite -amount of gas to run any genesis transactions. - -Note that we do not `Commit` during `InitChain` however BeginBlock for block 1 -starts from this deliverState. - - -## BeginBlock -TODO complete description & pseudo code - -The block gas meter is reset within BeginBlock for the deliver state. -If no maximum block gas is set within baseapp then an infinite -gas meter is set, otherwise a gas meter with the baseapp `maximumBlockGas` -is initialized - -## DeliverTx -TODO complete description & pseudo code - -Before transaction logic is run, the `BlockGasMeter` is first checked for -remaining gas. If no gas remains, then `DeliverTx` immediately returns an error. - -After the transaction has been processed the used gas is deducted from the -BlockGasMeter. If the remaining gas exceeds the meter's limits, then DeliverTx -returns an error and the transaction is not committed. - -## EndBlock - -## Commit - From 6fd3132e7122e9db0727fde771653a4c0aa26e0c Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Wed, 21 Nov 2018 02:02:15 -0500 Subject: [PATCH 18/27] lint fix, merge fix --- baseapp/baseapp.go | 3 +-- baseapp/baseapp_test.go | 3 +-- cmd/gaia/app/genesis.go | 2 +- types/gas.go | 3 +-- 4 files changed, 4 insertions(+), 7 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 95f7fbc4b..de7e36d72 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -259,9 +259,8 @@ func (app *BaseApp) storeConsensusParams(consensusParams *abci.ConsensusParams) func (app *BaseApp) getMaximumBlockGas() (maxGas uint64) { if app.consensusParams == nil || app.consensusParams.BlockSize == nil { return 0 - } else { - return uint64(app.consensusParams.BlockSize.MaxGas) } + return uint64(app.consensusParams.BlockSize.MaxGas) } //______________________________________________________________________________ diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 3dd997150..0994dd361 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -935,8 +935,7 @@ func TestMaxBlockGasLimits(t *testing.T) { for j := 0; j < tc.numDelivers; j++ { res := app.Deliver(tx) - ctx := app.getContextForAnte(runTxModeDeliver, nil) - ctx = app.initializeContext(ctx, runTxModeDeliver) + ctx := app.getState(runTxModeDeliver).ctx blockGasUsed := ctx.BlockGasMeter().GasConsumed() // check for failed transactions diff --git a/cmd/gaia/app/genesis.go b/cmd/gaia/app/genesis.go index 5bc9b577b..15010af98 100644 --- a/cmd/gaia/app/genesis.go +++ b/cmd/gaia/app/genesis.go @@ -262,7 +262,7 @@ func CollectStdTxs(cdc *codec.Codec, moniker string, genTxsDir string, genDoc tm "account %v not in genesis.json: %+v", addr, addrMap) } if acc.Coins.AmountOf(msg.Delegation.Denom).LT(msg.Delegation.Amount) { - err = fmt.Errorf("insufficient fund for the delegation: %s < %s", + err = fmt.Errorf("insufficient fund for the delegation: %v < %v", acc.Coins.AmountOf(msg.Delegation.Denom), msg.Delegation.Amount) } diff --git a/types/gas.go b/types/gas.go index be9a55b77..90c9da3ec 100644 --- a/types/gas.go +++ b/types/gas.go @@ -65,9 +65,8 @@ func (g *basicGasMeter) Limit() Gas { func (g *basicGasMeter) GasConsumedToLimit() Gas { if g.consumed > g.limit { return g.limit - } else { - return g.consumed } + return g.consumed } func (g *basicGasMeter) ConsumeGas(amount Gas, descriptor string) { From 972377c2874148881cf0fecfe984d4619362d4f6 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Wed, 21 Nov 2018 23:49:05 -0500 Subject: [PATCH 19/27] lint --- x/auth/ante.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/auth/ante.go b/x/auth/ante.go index 9a7a15e3e..7ac245cf0 100644 --- a/x/auth/ante.go +++ b/x/auth/ante.go @@ -310,7 +310,7 @@ func ensureSufficientMempoolFees(ctx sdk.Context, stdTx StdTx) sdk.Result { if stdTx.Fee.Gas <= 0 { return sdk.ErrInternal(fmt.Sprintf("invalid gas supplied: %d", stdTx.Fee.Gas)).Result() } - requiredFees := adjustFeesByGas(ctx.MinimumFees(), uint64(stdTx.Fee.Gas)) + requiredFees := adjustFeesByGas(ctx.MinimumFees(), stdTx.Fee.Gas) // NOTE: !A.IsAllGTE(B) is not the same as A.IsAllLT(B). if !ctx.MinimumFees().IsZero() && !stdTx.Fee.Amount.IsAllGTE(requiredFees) { From b4b61b890c4f031562a3830351c270561c64cab9 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Thu, 22 Nov 2018 00:30:04 -0500 Subject: [PATCH 20/27] address some comments while reviewing Jaes work --- baseapp/baseapp.go | 6 +++++- types/gas.go | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index de7e36d72..b0e7645d4 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -136,6 +136,7 @@ func (app *BaseApp) MountStore(key sdk.StoreKey, typ sdk.StoreType) { } // load latest application version +// panics if called more than once on a running baseapp func (app *BaseApp) LoadLatestVersion(mainKey *sdk.KVStoreKey) error { err := app.cms.LoadLatestVersion() if err != nil { @@ -145,6 +146,7 @@ func (app *BaseApp) LoadLatestVersion(mainKey *sdk.KVStoreKey) error { } // load application version +// panics if called more than once on a running baseapp func (app *BaseApp) LoadVersion(version int64, mainKey *sdk.KVStoreKey) error { err := app.cms.LoadVersion(version) if err != nil { @@ -702,7 +704,9 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk } } - // consume block gas whether panic or not. + // If BlockGasMeter() panics it will be caught by the above recover and + // return an error - in any case BlockGasMeter will consume gas past + // the limit. if mode == runTxModeDeliver { ctx.BlockGasMeter().ConsumeGas( ctx.GasMeter().GasConsumedToLimit(), "block gas meter") diff --git a/types/gas.go b/types/gas.go index 90c9da3ec..100b00430 100644 --- a/types/gas.go +++ b/types/gas.go @@ -63,7 +63,7 @@ func (g *basicGasMeter) Limit() Gas { } func (g *basicGasMeter) GasConsumedToLimit() Gas { - if g.consumed > g.limit { + if g.IsPastLimit() { return g.limit } return g.consumed From abed373d5c1525e5f1d882c2c885b83dbad464c2 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Thu, 22 Nov 2018 00:36:12 -0500 Subject: [PATCH 21/27] extra max block gas test at limit --- baseapp/baseapp_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 0994dd361..f4b3b1286 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -920,6 +920,7 @@ func TestMaxBlockGasLimits(t *testing.T) { {newTxCounter(10, 0), 3, 10, false, 0}, {newTxCounter(10, 0), 10, 10, false, 0}, {newTxCounter(2, 7), 11, 9, false, 0}, + {newTxCounter(10, 0), 10, 10, false, 0}, // hit the limit but pass {newTxCounter(10, 0), 11, 10, true, 10}, {newTxCounter(10, 0), 15, 10, true, 10}, From 2d3e1afea8dce759f9a0e0cf0a83943563840fde Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Thu, 22 Nov 2018 11:21:11 +0100 Subject: [PATCH 22/27] Add demonstrative failing testcase --- baseapp/baseapp_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index f4b3b1286..c2346499d 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -922,6 +922,7 @@ func TestMaxBlockGasLimits(t *testing.T) { {newTxCounter(2, 7), 11, 9, false, 0}, {newTxCounter(10, 0), 10, 10, false, 0}, // hit the limit but pass + {newTxCounter(9, 0), 12, 9, true, 11}, // fail after 11 {newTxCounter(10, 0), 11, 10, true, 10}, {newTxCounter(10, 0), 15, 10, true, 10}, } From 56fa7dc4ef7aa07757925875e193624f00ce33f1 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Thu, 22 Nov 2018 12:34:13 -0500 Subject: [PATCH 23/27] fix BlockGasRecovery --- baseapp/baseapp.go | 16 ++++++++++------ baseapp/baseapp_test.go | 4 ++-- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index b0e7645d4..02f482415 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -704,16 +704,20 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk } } - // If BlockGasMeter() panics it will be caught by the above recover and - // return an error - in any case BlockGasMeter will consume gas past - // the limit. + result.GasWanted = gasWanted + result.GasUsed = ctx.GasMeter().GasConsumed() + }() + + // If BlockGasMeter() panics it will be caught by the above recover and + // return an error - in any case BlockGasMeter will consume gas past + // the limit. + // NOTE: this must exist in a separate defer function for the + // above recovery to recover from this one + defer func() { if mode == runTxModeDeliver { ctx.BlockGasMeter().ConsumeGas( ctx.GasMeter().GasConsumedToLimit(), "block gas meter") } - - result.GasWanted = gasWanted - result.GasUsed = ctx.GasMeter().GasConsumed() }() var msgs = tx.GetMsgs() diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index c2346499d..d629ec580 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -922,12 +922,13 @@ func TestMaxBlockGasLimits(t *testing.T) { {newTxCounter(2, 7), 11, 9, false, 0}, {newTxCounter(10, 0), 10, 10, false, 0}, // hit the limit but pass - {newTxCounter(9, 0), 12, 9, true, 11}, // fail after 11 {newTxCounter(10, 0), 11, 10, true, 10}, {newTxCounter(10, 0), 15, 10, true, 10}, + {newTxCounter(9, 0), 12, 9, true, 11}, // fly past the limit } for i, tc := range testCases { + fmt.Printf("debug i: %v\n", i) tx := tc.tx // reset the block gas @@ -944,7 +945,6 @@ func TestMaxBlockGasLimits(t *testing.T) { if tc.fail && (j+1) > tc.failAfterDeliver { require.Equal(t, res.Code, sdk.CodeOutOfGas, fmt.Sprintf("%d: %v, %v", i, tc, res)) require.Equal(t, res.Codespace, sdk.CodespaceRoot, fmt.Sprintf("%d: %v, %v", i, tc, res)) - //require.True(t, ctx.BlockGasMeter().IsPastLimit()) NOTE: not necessarily true. require.True(t, ctx.BlockGasMeter().IsOutOfGas()) } else { // check gas used and wanted From ce10ef2b274faf2bf4b4bbf292238f7a3b2c7911 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Thu, 22 Nov 2018 12:41:20 -0500 Subject: [PATCH 24/27] replaced proto with codec in baseapp --- baseapp/baseapp.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 02f482415..f4c20ca90 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -6,7 +6,6 @@ import ( "runtime/debug" "strings" - "github.com/gogo/protobuf/proto" "github.com/pkg/errors" abci "github.com/tendermint/tendermint/abci/types" @@ -184,7 +183,7 @@ func (app *BaseApp) initFromMainStore(mainKey *sdk.KVStoreKey) error { consensusParamsBz := mainStore.Get(mainConsensusParamsKey) if consensusParamsBz != nil { var consensusParams = &abci.ConsensusParams{} - err := proto.Unmarshal(consensusParamsBz, consensusParams) + err := codec.Cdc.UnmarshalBinaryLengthPrefixed(consensusParamsBz, consensusParams) if err != nil { panic(err) } @@ -249,7 +248,7 @@ func (app *BaseApp) setConsensusParams(consensusParams *abci.ConsensusParams) { // setConsensusParams stores the consensus params to the main store. func (app *BaseApp) storeConsensusParams(consensusParams *abci.ConsensusParams) { - consensusParamsBz, err := proto.Marshal(consensusParams) + consensusParamsBz, err := codec.Cdc.MarshalBinaryLengthPrefixed(consensusParams) if err != nil { panic(err) } From 5792e1d5c44a01e9b02347225d980d93b15baf3d Mon Sep 17 00:00:00 2001 From: Alexander Bezobchuk Date: Sat, 24 Nov 2018 18:10:39 -0800 Subject: [PATCH 25/27] Apply suggestions from code review Co-Authored-By: jaekwon --- baseapp/baseapp.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index f4c20ca90..b2e07c588 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -173,13 +173,13 @@ func (app *BaseApp) initFromMainStore(mainKey *sdk.KVStoreKey) error { return errors.New("baseapp expects MultiStore with 'main' KVStore") } - // memoize mainKey. + // memoize mainKey if app.mainKey != nil { panic("app.mainKey expected to be nil; duplicate init?") } app.mainKey = mainKey - // load consensus param from the main store + // load consensus params from the main store consensusParamsBz := mainStore.Get(mainConsensusParamsKey) if consensusParamsBz != nil { var consensusParams = &abci.ConsensusParams{} From 819af35962ac9990b89ca92b20104116eaaf0d8d Mon Sep 17 00:00:00 2001 From: Jae Kwon Date: Sat, 24 Nov 2018 18:10:59 -0800 Subject: [PATCH 26/27] Final fixes from review --- baseapp/baseapp.go | 5 +++-- types/context.go | 15 --------------- 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index b2e07c588..a83713d42 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -6,6 +6,7 @@ import ( "runtime/debug" "strings" + "github.com/gogo/protobuf/proto" "github.com/pkg/errors" abci "github.com/tendermint/tendermint/abci/types" @@ -183,7 +184,7 @@ func (app *BaseApp) initFromMainStore(mainKey *sdk.KVStoreKey) error { consensusParamsBz := mainStore.Get(mainConsensusParamsKey) if consensusParamsBz != nil { var consensusParams = &abci.ConsensusParams{} - err := codec.Cdc.UnmarshalBinaryLengthPrefixed(consensusParamsBz, consensusParams) + err := proto.Unmarshal(consensusParamsBz, consensusParams) if err != nil { panic(err) } @@ -248,7 +249,7 @@ func (app *BaseApp) setConsensusParams(consensusParams *abci.ConsensusParams) { // setConsensusParams stores the consensus params to the main store. func (app *BaseApp) storeConsensusParams(consensusParams *abci.ConsensusParams) { - consensusParamsBz, err := codec.Cdc.MarshalBinaryLengthPrefixed(consensusParams) + consensusParamsBz, err := proto.Marshal(consensusParams) if err != nil { panic(err) } diff --git a/types/context.go b/types/context.go index 905748d51..add88bfc3 100644 --- a/types/context.go +++ b/types/context.go @@ -133,7 +133,6 @@ const ( contextKeyMultiStore contextKey = iota contextKeyBlockHeader contextKeyBlockHeight - contextKeyConsensusParams contextKeyChainID contextKeyIsCheckTx contextKeyTxBytes @@ -152,10 +151,6 @@ func (c Context) BlockHeader() abci.Header { return c.Value(contextKeyBlockHeade func (c Context) BlockHeight() int64 { return c.Value(contextKeyBlockHeight).(int64) } -func (c Context) ConsensusParams() abci.ConsensusParams { - return c.Value(contextKeyConsensusParams).(abci.ConsensusParams) -} - func (c Context) ChainID() string { return c.Value(contextKeyChainID).(string) } func (c Context) TxBytes() []byte { return c.Value(contextKeyTxBytes).([]byte) } @@ -201,16 +196,6 @@ func (c Context) WithBlockHeight(height int64) Context { return c.withValue(contextKeyBlockHeight, height).withValue(contextKeyBlockHeader, newHeader) } -func (c Context) WithConsensusParams(params *abci.ConsensusParams) Context { - if params == nil { - return c - } - - // TODO: Do we need to handle invalid MaxGas values? - return c.withValue(contextKeyConsensusParams, params). - WithGasMeter(NewGasMeter(uint64(params.BlockSize.MaxGas))) -} - func (c Context) WithChainID(chainID string) Context { return c.withValue(contextKeyChainID, chainID) } func (c Context) WithTxBytes(txBytes []byte) Context { return c.withValue(contextKeyTxBytes, txBytes) } From f12ac439f7c62e398da516ffa2bb9572278cfc55 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Sun, 25 Nov 2018 23:44:51 -0500 Subject: [PATCH 27/27] dep --- Gopkg.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 40192b2af..b166713ee 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -165,13 +165,12 @@ version = "v1.2.0" [[projects]] - digest = "1:c0d19ab64b32ce9fe5cf4ddceba78d5bc9807f0016db6b1183599da3dcc24d10" + digest = "1:ea40c24cdbacd054a6ae9de03e62c5f252479b96c716375aace5c120d68647c8" name = "github.com/hashicorp/hcl" packages = [ ".", "hcl/ast", "hcl/parser", - "hcl/printer", "hcl/scanner", "hcl/strconv", "hcl/token", @@ -644,6 +643,7 @@ "github.com/bgentry/speakeasy", "github.com/btcsuite/btcd/btcec", "github.com/cosmos/go-bip39", + "github.com/gogo/protobuf/proto", "github.com/golang/protobuf/proto", "github.com/gorilla/mux", "github.com/mattn/go-isatty",