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(gnovm): charge gas for type checking the Go AST statically #3974

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open
2 changes: 1 addition & 1 deletion gno.land/pkg/gnoclient/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ func TestRunMultiple_Integration(t *testing.T) {
// Make Tx config
baseCfg := BaseTxCfg{
GasFee: ugnot.ValueString(2300000),
GasWanted: 23000000,
GasWanted: 33000000, // To account for typechecking gas
AccountNumber: 0,
SequenceNumber: 0,
Memo: "",
Expand Down
2 changes: 1 addition & 1 deletion gno.land/pkg/integration/testdata/append.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ loadpkg gno.land/p/demo/ufmt
# start a new node
gnoland start

gnokey maketx addpkg -pkgdir $WORK -pkgpath gno.land/r/append -gas-fee 1000000ugnot -gas-wanted 9000000 -broadcast -chainid=tendermint_test test1
gnokey maketx addpkg -pkgdir $WORK -pkgpath gno.land/r/append -gas-fee 1000000ugnot -gas-wanted 9990000 -broadcast -chainid=tendermint_test test1
stdout OK!

# Call Append 1
Expand Down
20 changes: 10 additions & 10 deletions gno.land/pkg/integration/testdata/assertorigincall.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -72,39 +72,39 @@ stderr 'invalid non-origin call'
## stderr 'invalid non-origin call'

## 10. MsgRun -> run.main -> myrlm.A: PANIC
! gnokey maketx run -gas-fee 100000ugnot -gas-wanted 10_000_000 -broadcast -chainid tendermint_test test1 $WORK/run/myrlmA.gno
! gnokey maketx run -gas-fee 100000ugnot -gas-wanted 11_000_000 -broadcast -chainid tendermint_test test1 $WORK/run/myrlmA.gno
stderr 'invalid non-origin call'

## 11. MsgRun -> run.main -> myrlm.B: PASS
gnokey maketx run -gas-fee 100000ugnot -gas-wanted 10_000_000 -broadcast -chainid tendermint_test test1 $WORK/run/myrlmB.gno
gnokey maketx run -gas-fee 100000ugnot -gas-wanted 11_000_000 -broadcast -chainid tendermint_test test1 $WORK/run/myrlmB.gno
stdout 'OK!'

## 12. MsgRun -> run.main -> myrlm.C: PANIC
! gnokey maketx run -gas-fee 100000ugnot -gas-wanted 10_000_000 -broadcast -chainid tendermint_test test1 $WORK/run/myrlmC.gno
! gnokey maketx run -gas-fee 100000ugnot -gas-wanted 11_000_000 -broadcast -chainid tendermint_test test1 $WORK/run/myrlmC.gno
stderr 'invalid non-origin call'

## 13. MsgRun -> run.main -> foo.A: PANIC
! gnokey maketx run -gas-fee 100000ugnot -gas-wanted 10_000_000 -broadcast -chainid tendermint_test test1 $WORK/run/fooA.gno
! gnokey maketx run -gas-fee 100000ugnot -gas-wanted 11_000_000 -broadcast -chainid tendermint_test test1 $WORK/run/fooA.gno
stderr 'invalid non-origin call'

## 14. MsgRun -> run.main -> foo.B: PASS
gnokey maketx run -gas-fee 100000ugnot -gas-wanted 10_000_000 -broadcast -chainid tendermint_test test1 $WORK/run/fooB.gno
gnokey maketx run -gas-fee 100000ugnot -gas-wanted 11_000_000 -broadcast -chainid tendermint_test test1 $WORK/run/fooB.gno
stdout 'OK!'

## 15. MsgRun -> run.main -> foo.C: PANIC
! gnokey maketx run -gas-fee 100000ugnot -gas-wanted 10_000_000 -broadcast -chainid tendermint_test test1 $WORK/run/fooC.gno
! gnokey maketx run -gas-fee 100000ugnot -gas-wanted 11_000_000 -broadcast -chainid tendermint_test test1 $WORK/run/fooC.gno
stderr 'invalid non-origin call'

## 16. MsgRun -> run.main -> bar.A: PANIC
! gnokey maketx run -gas-fee 100000ugnot -gas-wanted 10_000_000 -broadcast -chainid tendermint_test test1 $WORK/run/barA.gno
! gnokey maketx run -gas-fee 100000ugnot -gas-wanted 11_000_000 -broadcast -chainid tendermint_test test1 $WORK/run/barA.gno
stderr 'invalid non-origin call'

## 17. MsgRun -> run.main -> bar.B: PASS
gnokey maketx run -gas-fee 100000ugnot -gas-wanted 10000000 -broadcast -chainid tendermint_test test1 $WORK/run/barB.gno
gnokey maketx run -gas-fee 100000ugnot -gas-wanted 11000000 -broadcast -chainid tendermint_test test1 $WORK/run/barB.gno
stdout 'OK!'

## 18. MsgRun -> run.main -> bar.C: PANIC
! gnokey maketx run -gas-fee 100000ugnot -gas-wanted 10_000_000 -broadcast -chainid tendermint_test test1 $WORK/run/barC.gno
! gnokey maketx run -gas-fee 100000ugnot -gas-wanted 11_000_000 -broadcast -chainid tendermint_test test1 $WORK/run/barC.gno
stderr 'invalid non-origin call'

## remove testcase 19 due to maketx call forced to call a realm
Expand All @@ -113,7 +113,7 @@ stderr 'invalid non-origin call'
## stdout 'OK!'

## 20. MsgRun -> std.AssertOriginCall: PANIC
! gnokey maketx run -gas-fee 100000ugnot -gas-wanted 10_000_000 -broadcast -chainid tendermint_test test1 $WORK/run/baz.gno
! gnokey maketx run -gas-fee 100000ugnot -gas-wanted 11_000_000 -broadcast -chainid tendermint_test test1 $WORK/run/baz.gno
stderr 'invalid non-origin call'


Expand Down
4 changes: 2 additions & 2 deletions gno.land/pkg/integration/testdata/grc20_registry.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ gnokey maketx call -pkgpath gno.land/r/registry -func TransferByName -args 'foo2
stdout 'not found'

# add foo20, and foo20wrapper
gnokey maketx addpkg -pkgdir $WORK/foo20 -pkgpath gno.land/r/foo20 -gas-fee 1000000ugnot -gas-wanted 10000000 -broadcast -chainid=tendermint_test test1
gnokey maketx addpkg -pkgdir $WORK/foo20wrapper -pkgpath gno.land/r/foo20wrapper -gas-fee 1000000ugnot -gas-wanted 10000000 -broadcast -chainid=tendermint_test test1
gnokey maketx addpkg -pkgdir $WORK/foo20 -pkgpath gno.land/r/foo20 -gas-fee 1000000ugnot -gas-wanted 12000000 -broadcast -chainid=tendermint_test test1
gnokey maketx addpkg -pkgdir $WORK/foo20wrapper -pkgpath gno.land/r/foo20wrapper -gas-fee 1000000ugnot -gas-wanted 12000000 -broadcast -chainid=tendermint_test test1

# we call Transfer with foo20, after it's registered
gnokey maketx call -pkgpath gno.land/r/registry -func TransferByName -args 'foo20' -args 'g123456789' -args '42' -gas-fee 1000000ugnot -gas-wanted 8000000 -broadcast -chainid=tendermint_test test1
Expand Down
6 changes: 3 additions & 3 deletions gno.land/pkg/integration/testdata/transfer_lock.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@ gnoland start -lock-transfer

## test1 is the DefaultAccount in the integration test. To ensure that the unrestricted account can send tokens even when token transfers are locked,
## we included it in the unrestricted account list in the genesis state. By default, the unrestricted account list is empty.
gnokey maketx send -send "9999999ugnot" -to $regular1_user_addr -gas-fee 1ugnot -gas-wanted 10000000 -broadcast -chainid=tendermint_test test1
gnokey maketx send -send "9999999ugnot" -to $regular1_user_addr -gas-fee 1ugnot -gas-wanted 12000000 -broadcast -chainid=tendermint_test test1

stdout 'OK!'

## Restricted simple token transfer
! gnokey maketx send -send "9999999ugnot" -to g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5 -gas-fee 1ugnot -gas-wanted 10000000 -broadcast -chainid=tendermint_test regular1
! gnokey maketx send -send "9999999ugnot" -to g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5 -gas-fee 1ugnot -gas-wanted 12000000 -broadcast -chainid=tendermint_test regular1
stderr 'restricted token transfer error'

## Restricted token transfer by calling a realm deposit function.
Expand All @@ -29,7 +29,7 @@ stderr 'restricted token transfer error'


## Restricted token transfer with depositing to a realm package while adding a package.
! gnokey maketx addpkg -pkgdir $WORK -pkgpath gno.land/r/bank -deposit "1000ugnot" -gas-fee 1000000ugnot -gas-wanted 10000000 -broadcast -chainid=tendermint_test regular1
! gnokey maketx addpkg -pkgdir $WORK -pkgpath gno.land/r/bank -deposit "1000ugnot" -gas-fee 1000000ugnot -gas-wanted 12000000 -broadcast -chainid=tendermint_test regular1
stderr 'restricted token transfer error'

## paying gas fees to add a package is acceptable.
Expand Down
6 changes: 3 additions & 3 deletions gno.land/pkg/sdk/vm/gas_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func TestAddPkgDeliverTxInsuffGas(t *testing.T) {
assert.True(t, abort)
assert.False(t, res.IsOK())
gasCheck := gctx.GasMeter().GasConsumed()
assert.Equal(t, int64(3462), gasCheck)
assert.Equal(t, int64(3641), gasCheck)
} else {
t.Errorf("should panic")
}
Expand All @@ -71,7 +71,7 @@ func TestAddPkgDeliverTx(t *testing.T) {
assert.True(t, res.IsOK())

// NOTE: let's try to keep this bellow 150_000 :)
assert.Equal(t, int64(143845), gasDeliver)
assert.Equal(t, int64(144024), gasDeliver)
}

// Enough gas for a failed transaction.
Expand All @@ -88,7 +88,7 @@ func TestAddPkgDeliverTxFailed(t *testing.T) {
gasDeliver := gctx.GasMeter().GasConsumed()

assert.False(t, res.IsOK())
assert.Equal(t, int64(1231), gasDeliver)
assert.Equal(t, int64(1410), gasDeliver)
}

// Not enough gas for a failed transaction.
Expand Down
4 changes: 2 additions & 2 deletions gno.land/pkg/sdk/vm/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ func (vm *VMKeeper) AddPackage(ctx sdk.Context, msg MsgAddPackage) (err error) {

// Validate Gno syntax and type check.
format := true
if err := gno.TypeCheckMemPackage(memPkg, gnostore, format); err != nil {
if err := gno.TypeCheckMemPackageWithGasMeter(memPkg, gnostore, format, ctx.GasMeter()); err != nil {
return ErrTypeCheck(err)
}

Expand Down Expand Up @@ -587,7 +587,7 @@ func (vm *VMKeeper) Run(ctx sdk.Context, msg MsgRun) (res string, err error) {

// Validate Gno syntax and type check.
format := false
if err = gno.TypeCheckMemPackage(memPkg, gnostore, format); err != nil {
if err = gno.TypeCheckMemPackageWithGasMeter(memPkg, gnostore, format, ctx.GasMeter()); err != nil {
return "", ErrTypeCheck(err)
}

Expand Down
51 changes: 51 additions & 0 deletions gno.land/pkg/sdk/vm/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package vm
// TODO: move most of the logic in ROOT/gno.land/...

import (
"bytes"
"errors"
"fmt"
"strings"
Expand Down Expand Up @@ -68,6 +69,56 @@ func Echo() string { return "hello world" }
assert.Equal(t, expected, memFile.Body)
}

func TestVMKeeperAddPackage_runOutOfGasInTypecheck(t *testing.T) {
env := setupTestEnv()
ctx := env.vmk.MakeGnoTransactionStore(env.ctx)
ctx = ctx.WithGasMeter(types.NewGasMeter(100_000))

// Give "addr1" some gnots.
addr := crypto.AddressFromPreimage([]byte("addr1"))
acc := env.acck.NewAccountWithAddress(ctx, addr)
env.acck.SetAccount(ctx, acc)
env.bankk.SetCoins(ctx, addr, std.MustParseCoins(coinsString))
assert.True(t, env.bankk.GetCoins(ctx, addr).IsEqual(std.MustParseCoins(coinsString)))

// Create the package.
src := func() string {
buf := new(bytes.Buffer)
N := 1000
buf.WriteString("package main\nfunc main() {\n\tconst x = 1")
for i := 0; i < 3; i++ {
for j := 0; j < N; j++ {
buf.WriteString("+\n(1")
}
for j := 0; j < N; j++ {
buf.WriteByte(')')
}
}
buf.WriteString("\nprintln(x)\n}")
return buf.String()
}()
files := []*gnovm.MemFile{
{
Name: "test.gno",
Body: src,
},
}
pkgPath := "gno.land/r/main"
msg1 := NewMsgAddPackage(addr, pkgPath, files)
assert.Nil(t, env.vmk.getGnoTransactionStore(ctx).GetPackage(pkgPath, false))

defer func() {
r := recover()
require.NotNil(t, r, "expected a panic")
rs := fmt.Sprintf("%s", r)
require.Contains(t, rs, "out of gas in location: typeCheck")
_, ok := r.(types.OutOfGasError)
require.True(t, ok)
}()

_ = env.vmk.AddPackage(ctx, msg1)
}

func TestVMKeeperAddPackage_InvalidDomain(t *testing.T) {
env := setupTestEnv()
ctx := env.vmk.MakeGnoTransactionStore(env.ctx)
Expand Down
108 changes: 105 additions & 3 deletions gnovm/pkg/gnolang/gotypecheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
"strings"

"github.com/gnolang/gno/gnovm"
storetypes "github.com/gnolang/gno/tm2/pkg/store/types"
"go.uber.org/multierr"
)

Expand All @@ -23,26 +24,36 @@
GetMemPackage(path string) *gnovm.MemPackage
}

const DEFAULT_MAX_GAS_UGNOT = 1_000_000 // 1Gnot aka 1e6 ugnots

// TypeCheckMemPackage performs type validation and checking on the given
// mempkg. To retrieve dependencies, it uses getter.
//
// The syntax checking is performed entirely using Go's go/types package.
//
// If format is true, the code will be automatically updated with the
// formatted source code.
//
// By default it uses a gas meter with `DEFAULT_MAX_GAS_UGNOT`.
func TypeCheckMemPackage(mempkg *gnovm.MemPackage, getter MemPackageGetter, format bool) error {
return typeCheckMemPackage(mempkg, getter, false, format)
return typeCheckMemPackage(mempkg, getter, false, format, storetypes.NewGasMeter(DEFAULT_MAX_GAS_UGNOT))
}

// TypeCheckMemPackageWithGasMeter is like TypeCheckMemPackage, except
// that it allows passing in the gas meter to use.
func TypeCheckMemPackageWithGasMeter(mempkg *gnovm.MemPackage, getter MemPackageGetter, format bool, gasMeter storetypes.GasMeter) error {
return typeCheckMemPackage(mempkg, getter, false, format, gasMeter)
}

// TypeCheckMemPackageTest performs the same type checks as [TypeCheckMemPackage],
// but allows re-declarations.
//
// Note: like TypeCheckMemPackage, this function ignores tests and filetests.
func TypeCheckMemPackageTest(mempkg *gnovm.MemPackage, getter MemPackageGetter) error {
return typeCheckMemPackage(mempkg, getter, true, false)
return typeCheckMemPackage(mempkg, getter, true, false, storetypes.NewInfiniteGasMeter())
}

func typeCheckMemPackage(mempkg *gnovm.MemPackage, getter MemPackageGetter, testing, format bool) error {
func typeCheckMemPackage(mempkg *gnovm.MemPackage, getter MemPackageGetter, testing, format bool, gasMeter storetypes.GasMeter) error {
var errs error
imp := &gnoImporter{
getter: getter,
Expand All @@ -53,6 +64,7 @@
},
},
allowRedefinitions: testing,
gasMeter: gasMeter,
}
imp.cfg.Importer = imp

Expand All @@ -77,6 +89,7 @@

// allow symbol redefinitions? (test standard libraries)
allowRedefinitions bool
gasMeter storetypes.GasMeter
}

// Unused, but satisfies the Importer interface.
Expand Down Expand Up @@ -136,6 +149,8 @@
continue
}

chargeGasForTypecheck(g.gasMeter, f)

if delFunc != nil {
deleteOldIdents(delFunc, f)
}
Expand Down Expand Up @@ -177,3 +192,90 @@
}
}
}

func chargeGasForTypecheck(gasMeter storetypes.GasMeter, f *ast.File) {
ast.Walk(&astTraversingGasCharger{gasMeter}, f)
}

// astTraversingGasCharger is an ast.Visitor helper that statically traverses an AST
// charging gas for the respective typechecking operations so as to bear a cost
// and not let typechecking be abused
type astTraversingGasCharger struct {
m storetypes.GasMeter
}

var _ ast.Visitor = (*astTraversingGasCharger)(nil)

func (atgc *astTraversingGasCharger) consumeGas(amount storetypes.Gas) {
atgc.m.ConsumeGas(amount, "typeCheck")
}

const _BASIC_TYPECHECK_GAS_CHARGE = 5 // Arbitrary value, needs more research and derivation.

func (atgc *astTraversingGasCharger) Visit(n ast.Node) ast.Visitor {
switch n.(type) {
case *ast.ImportSpec:
// No need to charge gas for imports.
return nil

case *ast.UnaryExpr:
atgc.consumeGas(_BASIC_TYPECHECK_GAS_CHARGE * 2)
Comment on lines +221 to +222
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I attached the comment to the base gas charge, please take a look.


case *ast.BinaryExpr:
atgc.consumeGas(_BASIC_TYPECHECK_GAS_CHARGE * 3)

case *ast.BasicLit:
atgc.consumeGas(_BASIC_TYPECHECK_GAS_CHARGE * 2)

case *ast.CompositeLit:
atgc.consumeGas(_BASIC_TYPECHECK_GAS_CHARGE * 3)

case *ast.CallExpr:
atgc.consumeGas(_BASIC_TYPECHECK_GAS_CHARGE * 4)

case *ast.ForStmt:
atgc.consumeGas(_BASIC_TYPECHECK_GAS_CHARGE * 5)

case *ast.RangeStmt:
atgc.consumeGas(_BASIC_TYPECHECK_GAS_CHARGE * 6)
// TODO: Alternate on the different type of range statements.

case *ast.FuncDecl:
atgc.consumeGas(_BASIC_TYPECHECK_GAS_CHARGE * 6)

case *ast.SwitchStmt:
atgc.consumeGas(_BASIC_TYPECHECK_GAS_CHARGE * 4)

Check warning on line 247 in gnovm/pkg/gnolang/gotypecheck.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/gotypecheck.go#L246-L247

Added lines #L246 - L247 were not covered by tests

case *ast.IfStmt:
atgc.consumeGas(_BASIC_TYPECHECK_GAS_CHARGE * 5)

case *ast.CaseClause:
atgc.consumeGas(_BASIC_TYPECHECK_GAS_CHARGE * 3)

Check warning on line 253 in gnovm/pkg/gnolang/gotypecheck.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/gotypecheck.go#L252-L253

Added lines #L252 - L253 were not covered by tests

case *ast.BranchStmt:
atgc.consumeGas(_BASIC_TYPECHECK_GAS_CHARGE * 3)

Check warning on line 256 in gnovm/pkg/gnolang/gotypecheck.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/gotypecheck.go#L255-L256

Added lines #L255 - L256 were not covered by tests

case *ast.AssignStmt:
atgc.consumeGas(_BASIC_TYPECHECK_GAS_CHARGE * 2)

case *ast.Ident:
atgc.consumeGas(_BASIC_TYPECHECK_GAS_CHARGE * 1)

case *ast.SelectorExpr:
atgc.consumeGas(_BASIC_TYPECHECK_GAS_CHARGE * 5)

case *ast.ParenExpr:
atgc.consumeGas(_BASIC_TYPECHECK_GAS_CHARGE * 3)

case *ast.ReturnStmt, *ast.DeferStmt:
atgc.consumeGas(_BASIC_TYPECHECK_GAS_CHARGE * 2)

case nil:
atgc.consumeGas(_BASIC_TYPECHECK_GAS_CHARGE / 2)

default: // IndexExpr, StarExpr et al, all fall under defaults here.
atgc.consumeGas(_BASIC_TYPECHECK_GAS_CHARGE * 3)
}

return atgc
}
Loading
Loading