Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gas management, estimation, limitation #963

Merged
merged 32 commits into from
May 16, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
46f9445
Add gas limit / gas consumed to context
cwgoes May 7, 2018
2699180
GasMeter & context updates
cwgoes May 7, 2018
efc7843
Changes to bank keeper for gas
cwgoes May 7, 2018
f0e4d24
Basic gas impl, quick testcase
cwgoes May 7, 2018
ddb3b36
Pass gas consumed back in result struct
cwgoes May 7, 2018
af379b6
Linter fixes
cwgoes May 7, 2018
1f8ef62
Swap to panic/recover version
cwgoes May 8, 2018
0951705
Catch out-of-gas panics
cwgoes May 8, 2018
ca4ef9a
Add baseapp test for out-of-gas handling
cwgoes May 8, 2018
c410ceb
GasKVStore WIP
cwgoes May 8, 2018
ef1923f
Add GasIterator
cwgoes May 8, 2018
9dfccb1
Update iterator gas pricing model
cwgoes May 8, 2018
1c4ed7b
Gas-wrap ctx.KVStore
cwgoes May 8, 2018
da5fe2e
Add baseapp.CheckFull
cwgoes May 9, 2018
097646e
Correct semantics for simulateDeliver
cwgoes May 9, 2018
a240554
SimulateTx through Query
cwgoes May 9, 2018
8c1c40b
New store query prefixes (ref #979)
cwgoes May 10, 2018
2147203
Update changelog
cwgoes May 10, 2018
702ffaf
Rebase
cwgoes May 11, 2018
147cf9f
Move GasKVStore to /store
cwgoes May 11, 2018
ce38d8f
Minor fix, testcases
cwgoes May 11, 2018
a801874
PR comment: codec => cdc
cwgoes May 11, 2018
38716d5
ConsumeGas for pubkey.VerifyBytes
cwgoes May 15, 2018
4cfa99e
Move to new version in changelog
cwgoes May 15, 2018
d55ba2c
Add p2p filter functions & tests
cwgoes May 15, 2018
396bf74
Update changelog
cwgoes May 15, 2018
4775437
Unexport verifyCost
cwgoes May 15, 2018
0cc1c52
Rebase changelog
cwgoes May 15, 2018
4134bf9
Address PR comments
cwgoes May 16, 2018
03e2207
Unexport RunTxMode (fix linter)
cwgoes May 16, 2018
3d5b048
Remove txGasLimit, update tests
cwgoes May 16, 2018
4bdcad5
remove gasLimit from NewContext
ebuchman May 16, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 38 additions & 30 deletions baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,17 @@ import (
// and to avoid affecting the Merkle root.
var dbHeaderKey = []byte("header")

type RunTxMode uint8

const (
// Check a transaction
RunTxModeCheck RunTxMode = iota
// Simulate a transaction
RunTxModeSimulate RunTxMode = iota
// Deliver a transaction
RunTxModeDeliver RunTxMode = iota
)

// The ABCI application
type BaseApp struct {
// initialized on creation
Expand Down Expand Up @@ -309,13 +320,17 @@ func (app *BaseApp) FilterPeerByPubKey(info string) abci.ResponseQuery {
// Implements ABCI.
// Delegates to CommitMultiStore if it implements Queryable
func (app *BaseApp) Query(req abci.RequestQuery) (res abci.ResponseQuery) {
path := req.Path
path := strings.Split(req.Path, "/")
// first element is empty string
if len(path) > 0 && path[0] == "" {
path = path[1:]
}
fmt.Sprintf("Path: %v\n", path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove

// "/app" prefix for special application queries
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we ran strings.Split on / and switched on it ? Would be nice not to slice strings with magic numbers everywhere

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, updated.

if strings.HasPrefix(path, "/app") {
query := path[4:]
if len(path) >= 2 && path[0] == "app" {
var result sdk.Result
switch query {
case "/simulate":
switch path[1] {
case "simulate":
txBytes := req.Data
tx, err := app.txDecoder(txBytes)
if err != nil {
Expand All @@ -333,27 +348,23 @@ func (app *BaseApp) Query(req abci.RequestQuery) (res abci.ResponseQuery) {
}
}
// "/store" prefix for store queries
if strings.HasPrefix(path, "/store") {
if len(path) >= 1 && path[0] == "store" {
queryable, ok := app.cms.(sdk.Queryable)
if !ok {
msg := "multistore doesn't support queries"
return sdk.ErrUnknownRequest(msg).QueryResult()
}
req.Path = req.Path[6:] // slice off "/store"
req.Path = "/" + strings.Join(path[1:], "/")
return queryable.Query(req)
}
// "/p2p" prefix for p2p queries
if strings.HasPrefix(path, "/p2p") {
path = path[4:]
if strings.HasPrefix(path, "/filter") {
path = path[7:]
if strings.HasPrefix(path, "/addr") {
path = path[6:]
return app.FilterPeerByAddrPort(path)
if len(path) >= 4 && path[0] == "p2p" {
if path[1] == "filter" {
if path[2] == "addr" {
return app.FilterPeerByAddrPort(path[3])
}
if strings.HasPrefix(path, "/pubkey") {
path = path[8:]
return app.FilterPeerByPubKey(path)
if path[2] == "pubkey" {
return app.FilterPeerByPubKey(path[3])
}
}
}
Expand Down Expand Up @@ -385,7 +396,7 @@ func (app *BaseApp) CheckTx(txBytes []byte) (res abci.ResponseCheckTx) {
if err != nil {
result = err.Result()
} else {
result = app.runTx(true, false, txBytes, tx)
result = app.runTx(RunTxModeCheck, txBytes, tx)
}

return abci.ResponseCheckTx{
Expand All @@ -410,7 +421,7 @@ func (app *BaseApp) DeliverTx(txBytes []byte) (res abci.ResponseDeliverTx) {
if err != nil {
result = err.Result()
} else {
result = app.runTx(false, false, txBytes, tx)
result = app.runTx(RunTxModeDeliver, txBytes, tx)
}

// After-handler hooks.
Expand All @@ -434,22 +445,22 @@ func (app *BaseApp) DeliverTx(txBytes []byte) (res abci.ResponseDeliverTx) {

// nolint - Mostly for testing
func (app *BaseApp) Check(tx sdk.Tx) (result sdk.Result) {
return app.runTx(true, false, nil, tx)
return app.runTx(RunTxModeCheck, nil, tx)
}

// nolint - full tx execution
func (app *BaseApp) Simulate(tx sdk.Tx) (result sdk.Result) {
return app.runTx(true, true, nil, tx)
return app.runTx(RunTxModeSimulate, nil, tx)
}

// nolint
func (app *BaseApp) Deliver(tx sdk.Tx) (result sdk.Result) {
return app.runTx(false, false, nil, tx)
return app.runTx(RunTxModeDeliver, nil, tx)
}

// txBytes may be nil in some cases, eg. in tests.
// Also, in the future we may support "internal" transactions.
func (app *BaseApp) runTx(isCheckTx bool, simulateDeliver bool, txBytes []byte, tx sdk.Tx) (result sdk.Result) {
func (app *BaseApp) runTx(mode RunTxMode, txBytes []byte, tx sdk.Tx) (result sdk.Result) {
// Handle any panics.
defer func() {
if r := recover(); r != nil {
Expand Down Expand Up @@ -479,17 +490,14 @@ func (app *BaseApp) runTx(isCheckTx bool, simulateDeliver bool, txBytes []byte,

// Get the context
var ctx sdk.Context
if isCheckTx {
if mode == RunTxModeCheck || mode == RunTxModeSimulate {
ctx = app.checkState.ctx.WithTxBytes(txBytes)
} else {
ctx = app.deliverState.ctx.WithTxBytes(txBytes)
}

// Create a new zeroed gas meter
ctx = ctx.WithGasMeter(sdk.NewGasMeter(app.txGasLimit))

// Simulate a DeliverTx for gas calculation
if isCheckTx && simulateDeliver {
if mode == RunTxModeSimulate {
ctx = ctx.WithIsCheckTx(false)
}

Expand All @@ -513,7 +521,7 @@ func (app *BaseApp) runTx(isCheckTx bool, simulateDeliver bool, txBytes []byte,

// Get the correct cache
var msCache sdk.CacheMultiStore
if isCheckTx {
if mode == RunTxModeCheck || mode == RunTxModeSimulate {
// CacheWrap app.checkState.ms in case it fails.
msCache = app.checkState.CacheMultiStore()
ctx = ctx.WithMultiStore(msCache)
Expand All @@ -529,7 +537,7 @@ func (app *BaseApp) runTx(isCheckTx bool, simulateDeliver bool, txBytes []byte,
result.GasUsed = ctx.GasMeter().GasConsumed()

// If not a simulated run and result was successful, write to app.checkState.ms or app.deliverState.ms
if !simulateDeliver && result.IsOK() {
if mode != RunTxModeSimulate && result.IsOK() {
msCache.Write()
}

Expand Down
6 changes: 3 additions & 3 deletions baseapp/baseapp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ func TestSimulateTx(t *testing.T) {
app.BeginBlock(abci.RequestBeginBlock{Header: header})
result := app.Simulate(tx)
require.Equal(t, result.Code, sdk.ABCICodeOK)
require.Equal(t, result.GasUsed, int64(80))
require.Equal(t, int64(80), result.GasUsed)
counter--
encoded, err := json.Marshal(tx)
require.Nil(t, err)
Expand All @@ -317,8 +317,8 @@ func TestSimulateTx(t *testing.T) {
require.Equal(t, queryResult.Code, uint32(sdk.ABCICodeOK))
var res sdk.Result
app.cdc.MustUnmarshalBinary(queryResult.Value, &res)
require.Equal(t, res.Code, sdk.ABCICodeOK)
require.Equal(t, res.GasUsed, int64(80))
require.Equal(t, sdk.ABCICodeOK, res.Code)
require.Equal(t, int64(160), res.GasUsed)
app.EndBlock(abci.RequestEndBlock{})
app.Commit()
}
Expand Down
1 change: 1 addition & 0 deletions client/context/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ func (ctx CoreContext) SignAndBuild(name, passphrase string, msg sdk.Msg, cdc *w
ChainID: chainID,
Sequences: []int64{sequence},
Msg: msg,
Fee: sdk.NewStdFee(10000, sdk.Coin{}), // TODO run simulate to estimate gas?
}

keybase, err := keys.GetKeyBase()
Expand Down
2 changes: 1 addition & 1 deletion cmd/gaia/app/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ var (
manyCoins = sdk.Coins{{"foocoin", 1}, {"barcoin", 1}}
fee = sdk.StdFee{
sdk.Coins{{"foocoin", 0}},
0,
100000,
}

sendMsg1 = bank.MsgSend{
Expand Down
2 changes: 1 addition & 1 deletion examples/basecoin/app/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ var (
manyCoins = sdk.Coins{{"foocoin", 1}, {"barcoin", 1}}
fee = sdk.StdFee{
sdk.Coins{{"foocoin", 0}},
0,
100000,
}

sendMsg1 = bank.MsgSend{
Expand Down
2 changes: 1 addition & 1 deletion examples/democoin/app/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ var (
coins = sdk.Coins{{"foocoin", 10}}
fee = sdk.StdFee{
sdk.Coins{{"foocoin", 0}},
0,
1000000,
}

sendMsg = bank.MsgSend{
Expand Down
3 changes: 3 additions & 0 deletions x/auth/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ func NewAnteHandler(am sdk.AccountMapper, feeHandler sdk.FeeHandler) sdk.AnteHan
// cache the signer accounts in the context
ctx = WithSigners(ctx, signerAccs)

// set the gas meter
ctx = ctx.WithGasMeter(sdk.NewGasMeter(stdTx.Fee.Gas))

// TODO: tx tags (?)

return ctx, sdk.Result{}, false // continue...
Expand Down
18 changes: 13 additions & 5 deletions x/bank/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
)

const (
costGetCoins sdk.Gas = 10
costHasCoins sdk.Gas = 10
costSetCoins sdk.Gas = 100
costSubtractCoins sdk.Gas = 10
costAddCoins sdk.Gas = 10
)

// Keeper manages transfers between accounts
type Keeper struct {
am sdk.AccountMapper
Expand Down Expand Up @@ -108,7 +116,7 @@ func (keeper ViewKeeper) HasCoins(ctx sdk.Context, addr sdk.Address, amt sdk.Coi
//______________________________________________________________________________________________

func getCoins(ctx sdk.Context, am sdk.AccountMapper, addr sdk.Address) sdk.Coins {
ctx.GasMeter().ConsumeGas(10, "getCoins")
ctx.GasMeter().ConsumeGas(costGetCoins, "getCoins")
acc := am.GetAccount(ctx, addr)
if acc == nil {
return sdk.Coins{}
Expand All @@ -117,7 +125,7 @@ func getCoins(ctx sdk.Context, am sdk.AccountMapper, addr sdk.Address) sdk.Coins
}

func setCoins(ctx sdk.Context, am sdk.AccountMapper, addr sdk.Address, amt sdk.Coins) sdk.Error {
ctx.GasMeter().ConsumeGas(100, "setCoins")
ctx.GasMeter().ConsumeGas(costSetCoins, "setCoins")
acc := am.GetAccount(ctx, addr)
if acc == nil {
acc = am.NewAccountWithAddress(ctx, addr)
Expand All @@ -129,13 +137,13 @@ func setCoins(ctx sdk.Context, am sdk.AccountMapper, addr sdk.Address, amt sdk.C

// HasCoins returns whether or not an account has at least amt coins.
func hasCoins(ctx sdk.Context, am sdk.AccountMapper, addr sdk.Address, amt sdk.Coins) bool {
ctx.GasMeter().ConsumeGas(10, "hasCoins")
ctx.GasMeter().ConsumeGas(costHasCoins, "hasCoins")
return getCoins(ctx, am, addr).IsGTE(amt)
}

// SubtractCoins subtracts amt from the coins at the addr.
func subtractCoins(ctx sdk.Context, am sdk.AccountMapper, addr sdk.Address, amt sdk.Coins) (sdk.Coins, sdk.Tags, sdk.Error) {
ctx.GasMeter().ConsumeGas(10, "subtractCoins")
ctx.GasMeter().ConsumeGas(costSubtractCoins, "subtractCoins")
oldCoins := getCoins(ctx, am, addr)
newCoins := oldCoins.Minus(amt)
if !newCoins.IsNotNegative() {
Expand All @@ -148,7 +156,7 @@ func subtractCoins(ctx sdk.Context, am sdk.AccountMapper, addr sdk.Address, amt

// AddCoins adds amt to the coins at the addr.
func addCoins(ctx sdk.Context, am sdk.AccountMapper, addr sdk.Address, amt sdk.Coins) (sdk.Coins, sdk.Tags, sdk.Error) {
ctx.GasMeter().ConsumeGas(10, "addCoins")
ctx.GasMeter().ConsumeGas(costAddCoins, "addCoins")
oldCoins := getCoins(ctx, am, addr)
newCoins := oldCoins.Plus(amt)
if !newCoins.IsNotNegative() {
Expand Down