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

fix: consolidate vm gas consumption #1430

Merged
merged 16 commits into from
Apr 25, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
5 changes: 4 additions & 1 deletion gno.land/pkg/gnoland/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
baseKey := store.NewStoreKey("base")

// Create BaseApp.
// TODO: Add a flag to set min gas prices for the node, by default it does not check.

Check warning on line 63 in gno.land/pkg/gnoland/app.go

View check run for this annotation

Codecov / codecov/patch

gno.land/pkg/gnoland/app.go#L63

Added line #L63 was not covered by tests
piux2 marked this conversation as resolved.
Show resolved Hide resolved
baseApp := sdk.NewBaseApp("gnoland", cfg.Logger, cfg.DB, baseKey, mainKey)
baseApp.SetAppVersion("dev")

Expand All @@ -79,8 +80,10 @@
baseApp.SetInitChainer(InitChainer(baseApp, acctKpr, bankKpr, cfg.SkipFailingGenesisTxs))

// Set AnteHandler
vmah := vm.NewAnteHandler(vmKpr)

Check warning on line 83 in gno.land/pkg/gnoland/app.go

View check run for this annotation

Codecov / codecov/patch

gno.land/pkg/gnoland/app.go#L83

Added line #L83 was not covered by tests
authOptions := auth.AnteOptions{
VerifyGenesisSignatures: false, // for development
AnteHandlerChain: []sdk.AnteHandler{vmah},

Check warning on line 86 in gno.land/pkg/gnoland/app.go

View check run for this annotation

Codecov / codecov/patch

gno.land/pkg/gnoland/app.go#L86

Added line #L86 was not covered by tests
}
authAnteHandler := auth.NewAnteHandler(
acctKpr, bankKpr, auth.DefaultSigVerificationGasConsumer, authOptions)
Expand Down Expand Up @@ -131,7 +134,7 @@
}

cfg.Logger = logger

cfg.SkipFailingGenesisTxs = skipFailingGenesisTxs

Check warning on line 137 in gno.land/pkg/gnoland/app.go

View check run for this annotation

Codecov / codecov/patch

gno.land/pkg/gnoland/app.go#L137

Added line #L137 was not covered by tests
piux2 marked this conversation as resolved.
Show resolved Hide resolved
return NewAppWithOptions(cfg)
}

Expand Down
44 changes: 44 additions & 0 deletions gno.land/pkg/sdk/vm/ante.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package vm

import (
"github.com/gnolang/gno/tm2/pkg/sdk"
"github.com/gnolang/gno/tm2/pkg/std"
)

// It returns a AnteHandler function that can be chained togateher in app to check pre condition before put it the mempool and propergate to other nodes.
piux2 marked this conversation as resolved.
Show resolved Hide resolved
// It checks there is enough gas to execute the transaction in TxCheck and Simulation mode.
piux2 marked this conversation as resolved.
Show resolved Hide resolved
piux2 marked this conversation as resolved.
Show resolved Hide resolved
// XXX: We only abort the tx due to the insufficient gas. Should we even allow it pass ante handler to prevent censorship? In other word, should we only keep the min gas price as the check to drop a transaction?
piux2 marked this conversation as resolved.
Show resolved Hide resolved
func NewAnteHandler(vmKpr *VMKeeper) sdk.AnteHandler {
return func(
ctx sdk.Context, tx std.Tx, simulate bool,
) (newCtx sdk.Context, res sdk.Result, abort bool) {
// skip the check for Deliver Mode Gas and Msg Executions
if ctx.Mode() == sdk.RunTxModeDeliver {
return ctx, res, false
}
// XXXX: check vm gas here for CheckTx and Simulation node.
piux2 marked this conversation as resolved.
Show resolved Hide resolved

vmh := NewHandler(vmKpr)
msgs := tx.GetMsgs()

for _, msg := range msgs {
// match message route
msgRoute := msg.Route()
if msgRoute == RouterKey {
// XXX: When there is no enough gas left in gas meter, it will stop the transaction in CheckTx() before passing the tx to mempool and broadcasting to other nodes.
// Same message will be processed sencond time in DeliverTx(). It should be ok for now, since CheckTx and DeliverTx execution are in different contexts that are not linked to each other.
piux2 marked this conversation as resolved.
Show resolved Hide resolved

res = vmh.Process(ctx, msg)
}

// we don't abort the transaction when there is a message execution failer. the failed message should be allowed to propergate to other nodes.
// We dont not want to censor the tx for VM execution failures just by one node.
piux2 marked this conversation as resolved.
Show resolved Hide resolved
// XXX: Do not uncomment this. Do not remvove this either to prevent someone accidentally add this check.
piux2 marked this conversation as resolved.
Show resolved Hide resolved
// if !res.IsOK() {
// return ctx, res, true
// }
}

return ctx, res, false
}
}
211 changes: 211 additions & 0 deletions gno.land/pkg/sdk/vm/ante_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,211 @@
package vm

import (
"testing"

bft "github.com/gnolang/gno/tm2/pkg/bft/types"
"github.com/gnolang/gno/tm2/pkg/crypto"
"github.com/gnolang/gno/tm2/pkg/sdk"
"github.com/gnolang/gno/tm2/pkg/sdk/auth"
"github.com/gnolang/gno/tm2/pkg/std"
"github.com/gnolang/gno/tm2/pkg/store"
"github.com/jaekwon/testify/assert"
)

// The gas consumed is counted only towards the execution of gno messages
// We only abort the tx due to the insufficient gas.

func TestAddPkgSimulateGas(t *testing.T) {
// setup
success := true
ctx, tx, anteHandler := setup(success)
// simulation should not fail even if gas wanted is low

ctx = ctx.WithMode(sdk.RunTxModeSimulate)
simulate := true

tx.Fee.GasWanted = 1
gctx := auth.SetGasMeter(simulate, ctx, tx.Fee.GasWanted)
gctx, res, abort := anteHandler(gctx, tx, simulate)
gasSimulate := gctx.GasMeter().GasConsumed()

assert.False(t, abort)
assert.True(t, res.IsOK())
assert.Equal(t, gasSimulate, int64(94055))
}

// failed tx will not aborted
piux2 marked this conversation as resolved.
Show resolved Hide resolved
func TestAddPkgSimulateFailedGas(t *testing.T) {
// setup
success := false
ctx, tx, anteHandler := setup(success)
// simulation should not fail even if gas wanted is low

ctx = ctx.WithMode(sdk.RunTxModeSimulate)
simulate := true

tx.Fee.GasWanted = 1
gctx := auth.SetGasMeter(simulate, ctx, tx.Fee.GasWanted)
gctx, res, abort := anteHandler(gctx, tx, simulate)
gasSimulate := gctx.GasMeter().GasConsumed()

assert.False(t, abort)
assert.True(t, res.IsOK())
assert.Equal(t, gasSimulate, int64(18989))
}

func TestAddPkgCheckTxGas(t *testing.T) {
success := true
ctx, tx, anteHandler := setup(success)
// Testing case with enough gas and succcful message execution
piux2 marked this conversation as resolved.
Show resolved Hide resolved

ctx = ctx.WithMode(sdk.RunTxModeCheck)
simulate := false
tx.Fee.GasWanted = 500000
gctx := auth.SetGasMeter(simulate, ctx, tx.Fee.GasWanted)
gctx, res, abort := anteHandler(gctx, tx, simulate)
gasCheck := gctx.GasMeter().GasConsumed()

assert.False(t, abort)
assert.True(t, res.IsOK())
assert.Equal(t, gasCheck, int64(94055))
}

// CheckTx only abort when there is no enough gas meter.
piux2 marked this conversation as resolved.
Show resolved Hide resolved
func TestAddPkgCheckTxNoGas(t *testing.T) {
success := true
ctx, tx, anteHandler := setup(success)
// Testing case with enough gas and succcful message execution
ctx = ctx.WithMode(sdk.RunTxModeCheck)
simulate := false
tx.Fee.GasWanted = 3000
gctx := auth.SetGasMeter(simulate, ctx, tx.Fee.GasWanted)

var res sdk.Result
abort := false

defer func() {
if r := recover(); r != nil {
switch r.(type) {
case store.OutOfGasException:
res.Error = sdk.ABCIError(std.ErrOutOfGas(""))
abort = true
default:
t.Errorf("should panic on OutOfGasException only")
}
assert.True(t, abort)
assert.False(t, res.IsOK())
gasCheck := gctx.GasMeter().GasConsumed()
assert.Equal(t, gasCheck, int64(3231))
} else {
t.Errorf("should panic")
}
}()
gctx, res, abort = anteHandler(gctx, tx, simulate)
}

// failed tx execution should pass the vm.AnteHandler
func TestAddPkgCheckTxFailedGas(t *testing.T) {
success := false
ctx, tx, anteHandler := setup(success)

ctx = ctx.WithMode(sdk.RunTxModeCheck)
simulate := false
tx.Fee.GasWanted = 500000
gctx := auth.SetGasMeter(simulate, ctx, tx.Fee.GasWanted)
gctx, res, abort := anteHandler(gctx, tx, simulate)
gasCheck := gctx.GasMeter().GasConsumed()

assert.False(t, abort)
assert.True(t, res.IsOK())
assert.Equal(t, gasCheck, int64(18989))
}

// For deliver Tx ante handler does not check gas consumption and does not consume gas
piux2 marked this conversation as resolved.
Show resolved Hide resolved
func TestAddPkgDeliverTxGas(t *testing.T) {
success := true
ctx, tx, anteHandler := setup(success)

var simulate bool

ctx = ctx.WithMode(sdk.RunTxModeDeliver)
simulate = false
tx.Fee.GasWanted = 1
gctx := auth.SetGasMeter(simulate, ctx, tx.Fee.GasWanted)
gasDeliver := gctx.GasMeter().GasConsumed()
gctx, res, abort := anteHandler(gctx, tx, simulate)
assert.False(t, abort)
assert.True(t, res.IsOK())
assert.Equal(t, gasDeliver, int64(0))
}

// // For deliver Tx, ante handler does not check gas consumption and does not consume gas
piux2 marked this conversation as resolved.
Show resolved Hide resolved
func TestAddPkgDeliverTxFailGas(t *testing.T) {
success := true
ctx, tx, anteHandler := setup(success)

var simulate bool

ctx = ctx.WithMode(sdk.RunTxModeDeliver)
simulate = false
tx.Fee.GasWanted = 1
gctx := auth.SetGasMeter(simulate, ctx, tx.Fee.GasWanted)
gasDeliver := gctx.GasMeter().GasConsumed()
gctx, res, abort := anteHandler(gctx, tx, simulate)
assert.False(t, abort)
assert.True(t, res.IsOK())
assert.Equal(t, gasDeliver, int64(0))
}

func setup(success bool) (sdk.Context, sdk.Tx, sdk.AnteHandler) {
// setup
env := setupTestEnv()
ctx := env.ctx
// conduct base gas meter tests from a non-genesis block since genesis block use infinite gas meter instead.
ctx = ctx.WithBlockHeader(&bft.Header{Height: int64(1)})
anteHandler := NewAnteHandler(env.vmk)
// Createa an account with 10M gnot (10gnot)
piux2 marked this conversation as resolved.
Show resolved Hide resolved
addr := crypto.AddressFromPreimage([]byte("test1"))
acc := env.acck.NewAccountWithAddress(ctx, addr)
env.acck.SetAccount(ctx, acc)
env.bank.SetCoins(ctx, addr, std.MustParseCoins("10000000ugnot"))
// success message
var files []*std.MemFile
if success {
files = []*std.MemFile{
{
Name: "hello.gno",
Body: `package hello

import "std"

func Echo() string {
return "hello world"
}`,
},
}
} else {
// falied message
piux2 marked this conversation as resolved.
Show resolved Hide resolved
files = []*std.MemFile{
{
Name: "hello.gno",
Body: `package hello

import "std"

func Echo() UnknowType {
return "hello world"
}`,
},
}
}

pkgPath := "gno.land/r/hello"
// creat messages and a transaction
piux2 marked this conversation as resolved.
Show resolved Hide resolved
msg := NewMsgAddPackage(addr, pkgPath, files)
msgs := []std.Msg{msg}
fee := std.NewFee(500000, std.MustParseCoin("1ugnot"))
tx := std.NewTx(msgs, fee, []std.Signature{}, "")

return ctx, tx, anteHandler
}
2 changes: 1 addition & 1 deletion gno.land/pkg/sdk/vm/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func setupTestEnv() testEnv {
ms.MountStoreWithDB(iavlCapKey, iavl.StoreConstructor, db)
ms.LoadLatestVersion()

ctx := sdk.NewContext(sdk.RunTxModeDeliver, ms, &bft.Header{ChainID: "test-chain-id"}, log.NewNopLogger())
ctx := sdk.NewContext(sdk.RunTxModeDeliver, ms, &bft.Header{ChainID: "test-chain-id"}, log.TestingLogger())
acck := authm.NewAccountKeeper(iavlCapKey, std.ProtoBaseAccount)
bank := bankm.NewBankKeeper(acck)
stdlibsDir := filepath.Join("..", "..", "..", "..", "gnovm", "stdlibs")
Expand Down
48 changes: 48 additions & 0 deletions gno.land/pkg/sdk/vm/gas.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package vm

import (
"fmt"

gno "github.com/gnolang/gno/gnovm/pkg/gnolang"

"github.com/gnolang/gno/tm2/pkg/sdk"
"github.com/gnolang/overflow"
)

const (

// We will use gasMemFactor and gasCpuFactor to multiply the vm memory allocation and cpu cycles to get the gas number.
// We can use these two factoctors to keep the gas for storage access, CPU and Mem in reasonable proportion.
piux2 marked this conversation as resolved.
Show resolved Hide resolved
piux2 marked this conversation as resolved.
Show resolved Hide resolved
gasFactorMem int64 = 1 // change this value based on gas profiling
gasFactorCpu int64 = 1 // change this value based on gas profiling

logPrefixAddPkg = "gas.vm.addpkg"
logPrefixCall = "gas.vm.call"
logPrefixRun = "gas.vm.run"
logPrefixQeval = "gas.vm.qeval"
logPrefixQevalStr = "gas.vm.qevalstr"
)

// consume gas and log vm gas usage
func consumeGas(ctx sdk.Context, m *gno.Machine, prefix string, pkgPath string, expr string) {
_, mem := m.Alloc.Status()

gasCpu := overflow.Mul64p(m.Cycles, gasFactorCpu)
gasMem := overflow.Mul64p(mem, gasFactorMem)

// we simplify the log here, the storage gas log included tx size and sigature verification gas.
piux2 marked this conversation as resolved.
Show resolved Hide resolved
storeLog := fmt.Sprintf("%s.storage, %s %s, %d", prefix, pkgPath, expr, ctx.GasMeter().GasConsumed())
piux2 marked this conversation as resolved.
Show resolved Hide resolved
ctx.Logger().Info(storeLog)

memLog := fmt.Sprintf("%s.memalloc, %s %s, %d", prefix, pkgPath, expr, gasMem)
ctx.Logger().Info(memLog)

cpuLog := fmt.Sprintf("%s.cpucycles, %s %s, %d", prefix, pkgPath, expr, gasCpu)
ctx.Logger().Info(cpuLog)

ctx.GasMeter().ConsumeGas(gasMem, prefix+".MemAlloc")
ctx.GasMeter().ConsumeGas(gasCpu, prefix+".CpuCycles")

gasTotal := fmt.Sprintf("%s.total, %s %s, %d", prefix, pkgPath, expr, ctx.GasMeter().GasConsumed())
ctx.Logger().Info(gasTotal)
}
Loading
Loading