From 99569d6ced53bff3f3e98143a6cb717d8e365c88 Mon Sep 17 00:00:00 2001 From: deelawn Date: Wed, 20 Mar 2024 18:38:40 -0700 Subject: [PATCH 1/7] guard against infinite cycles where provisioning a machine --- gnovm/pkg/gnolang/machine.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/gnovm/pkg/gnolang/machine.go b/gnovm/pkg/gnolang/machine.go index e9e8eba8adc..6c230969caf 100644 --- a/gnovm/pkg/gnolang/machine.go +++ b/gnovm/pkg/gnolang/machine.go @@ -99,6 +99,10 @@ func NewMachineWithOptions(opts MachineOptions) *Machine { checkTypes := opts.CheckTypes readOnly := opts.ReadOnly maxCycles := opts.MaxCycles + if maxCycles == 0 { + // Never allow an infinite number of cycles. + maxCycles = 10_000_000 + } output := opts.Output if output == nil { output = os.Stdout From da743676ffd3020aee50b31d43a45772a3538dc7 Mon Sep 17 00:00:00 2001 From: deelawn Date: Wed, 20 Mar 2024 19:04:05 -0700 Subject: [PATCH 2/7] avoid printing massive machine state --- gnovm/pkg/gnolang/machine.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/gnovm/pkg/gnolang/machine.go b/gnovm/pkg/gnolang/machine.go index 6c230969caf..fdf6a2b3107 100644 --- a/gnovm/pkg/gnolang/machine.go +++ b/gnovm/pkg/gnolang/machine.go @@ -658,6 +658,13 @@ func (m *Machine) RunFunc(fn Name) { func (m *Machine) RunMain() { defer func() { if r := recover(); r != nil { + // This is likely due to an infinite loop or recursion. + // Don't tryp to print out the machine state as it may + // take a very long time or never finish. + if v, ok := r.(cpuCyclePanic); ok { + panic(v.String()) + } + fmt.Printf("Machine.RunMain() panic: %v\n%s\n", r, m.String()) panic(r) @@ -953,10 +960,16 @@ const ( //---------------------------------------- // "CPU" steps. +type cpuCyclePanic struct{} + +func (p cpuCyclePanic) String() string { + return "CPU cycle overrun" +} + func (m *Machine) incrCPU(cycles int64) { m.Cycles += cycles if m.MaxCycles != 0 && m.Cycles > m.MaxCycles { - panic("CPU cycle overrun") + panic(cpuCyclePanic{}) } } From f6ffe76f30859a79a68d9b8fb10ab66663b7c74e Mon Sep 17 00:00:00 2001 From: deelawn Date: Wed, 20 Mar 2024 19:04:30 -0700 Subject: [PATCH 3/7] added cpu cycle error test --- gnovm/tests/files/cycles.gno | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 gnovm/tests/files/cycles.gno diff --git a/gnovm/tests/files/cycles.gno b/gnovm/tests/files/cycles.gno new file mode 100644 index 00000000000..95c7f35c02d --- /dev/null +++ b/gnovm/tests/files/cycles.gno @@ -0,0 +1,12 @@ +package main + +func main() { + recurse() +} + +func recurse() int { + return recurse() +} + +// Error: +// CPU cycle overrun From 50666c6c5d0cd5f1a42e929e2e596018c09ce4a8 Mon Sep 17 00:00:00 2001 From: deelawn Date: Wed, 20 Mar 2024 19:35:02 -0700 Subject: [PATCH 4/7] don't limit cycles for testing for convenience --- gnovm/pkg/gnolang/machine.go | 3 ++- gnovm/tests/file.go | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/gnovm/pkg/gnolang/machine.go b/gnovm/pkg/gnolang/machine.go index fdf6a2b3107..d6b1fcfb546 100644 --- a/gnovm/pkg/gnolang/machine.go +++ b/gnovm/pkg/gnolang/machine.go @@ -75,6 +75,7 @@ type MachineOptions struct { Alloc *Allocator // or see MaxAllocBytes. MaxAllocBytes int64 // or 0 for no limit. MaxCycles int64 // or 0 for no limit. + IsTest bool } // the machine constructor gets spammed @@ -99,7 +100,7 @@ func NewMachineWithOptions(opts MachineOptions) *Machine { checkTypes := opts.CheckTypes readOnly := opts.ReadOnly maxCycles := opts.MaxCycles - if maxCycles == 0 { + if !opts.IsTest && maxCycles == 0 { // Never allow an infinite number of cycles. maxCycles = 10_000_000 } diff --git a/gnovm/tests/file.go b/gnovm/tests/file.go index 70bed4eda50..e183fcedef1 100644 --- a/gnovm/tests/file.go +++ b/gnovm/tests/file.go @@ -57,6 +57,7 @@ func testMachineCustom(store gno.Store, pkgPath string, stdout io.Writer, maxAll Store: store, Context: ctx, MaxAllocBytes: maxAlloc, + IsTest: true, }) return m } From bac5deca32545661b0c3b474cc528f9a6cee7aa4 Mon Sep 17 00:00:00 2001 From: deelawn Date: Wed, 20 Mar 2024 19:43:00 -0700 Subject: [PATCH 5/7] added machine test flag --- gnovm/tests/machine_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/gnovm/tests/machine_test.go b/gnovm/tests/machine_test.go index a67d67f1ff2..c85fb33565e 100644 --- a/gnovm/tests/machine_test.go +++ b/gnovm/tests/machine_test.go @@ -47,6 +47,7 @@ func TestMachineTestMemPackage(t *testing.T) { Output: os.Stdout, Store: store, Context: nil, + IsTest: true, }) memPkg := gno.ReadMemPackage(tt.path, "test") From b940ee8c3094addbf84e4287ca2b3f2928c366a9 Mon Sep 17 00:00:00 2001 From: deelawn Date: Tue, 26 Mar 2024 15:31:32 -0700 Subject: [PATCH 6/7] reworked to restrict max cycles to the keeper --- gno.land/pkg/sdk/vm/keeper.go | 12 +++++++++++- gnovm/pkg/gnolang/machine.go | 20 +------------------- gnovm/tests/file.go | 1 - gnovm/tests/files/cycles.gno | 12 ------------ gnovm/tests/machine_test.go | 1 - 5 files changed, 12 insertions(+), 34 deletions(-) delete mode 100644 gnovm/tests/files/cycles.gno diff --git a/gno.land/pkg/sdk/vm/keeper.go b/gno.land/pkg/sdk/vm/keeper.go index 54f424ee058..cc9a23bbff1 100644 --- a/gno.land/pkg/sdk/vm/keeper.go +++ b/gno.land/pkg/sdk/vm/keeper.go @@ -22,6 +22,11 @@ import ( const ( maxAllocTx = 500 * 1000 * 1000 maxAllocQuery = 1500 * 1000 * 1000 // higher limit for queries + + // maxVMCycles is the maximum number of cycles allowed while executing a single VM + // message. Ideally this should not be needed, as execution should halt when out of + // gas. The worst case scenario is that this value is used as a fallback. + maxVMCycles = 10_000_000 ) // vm.VMKeeperI defines a module interface that supports Gno @@ -57,6 +62,11 @@ func NewVMKeeper( stdlibsDir string, maxCycles int64, ) *VMKeeper { + cycles := maxCycles + if cycles <= 0 || cycles > maxVMCycles { + cycles = maxVMCycles + } + // TODO: create an Options struct to avoid too many constructor parameters vmk := &VMKeeper{ baseKey: baseKey, @@ -64,7 +74,7 @@ func NewVMKeeper( acck: acck, bank: bank, stdlibsDir: stdlibsDir, - maxCycles: maxCycles, + maxCycles: cycles, } return vmk } diff --git a/gnovm/pkg/gnolang/machine.go b/gnovm/pkg/gnolang/machine.go index d6b1fcfb546..e9e8eba8adc 100644 --- a/gnovm/pkg/gnolang/machine.go +++ b/gnovm/pkg/gnolang/machine.go @@ -75,7 +75,6 @@ type MachineOptions struct { Alloc *Allocator // or see MaxAllocBytes. MaxAllocBytes int64 // or 0 for no limit. MaxCycles int64 // or 0 for no limit. - IsTest bool } // the machine constructor gets spammed @@ -100,10 +99,6 @@ func NewMachineWithOptions(opts MachineOptions) *Machine { checkTypes := opts.CheckTypes readOnly := opts.ReadOnly maxCycles := opts.MaxCycles - if !opts.IsTest && maxCycles == 0 { - // Never allow an infinite number of cycles. - maxCycles = 10_000_000 - } output := opts.Output if output == nil { output = os.Stdout @@ -659,13 +654,6 @@ func (m *Machine) RunFunc(fn Name) { func (m *Machine) RunMain() { defer func() { if r := recover(); r != nil { - // This is likely due to an infinite loop or recursion. - // Don't tryp to print out the machine state as it may - // take a very long time or never finish. - if v, ok := r.(cpuCyclePanic); ok { - panic(v.String()) - } - fmt.Printf("Machine.RunMain() panic: %v\n%s\n", r, m.String()) panic(r) @@ -961,16 +949,10 @@ const ( //---------------------------------------- // "CPU" steps. -type cpuCyclePanic struct{} - -func (p cpuCyclePanic) String() string { - return "CPU cycle overrun" -} - func (m *Machine) incrCPU(cycles int64) { m.Cycles += cycles if m.MaxCycles != 0 && m.Cycles > m.MaxCycles { - panic(cpuCyclePanic{}) + panic("CPU cycle overrun") } } diff --git a/gnovm/tests/file.go b/gnovm/tests/file.go index e183fcedef1..70bed4eda50 100644 --- a/gnovm/tests/file.go +++ b/gnovm/tests/file.go @@ -57,7 +57,6 @@ func testMachineCustom(store gno.Store, pkgPath string, stdout io.Writer, maxAll Store: store, Context: ctx, MaxAllocBytes: maxAlloc, - IsTest: true, }) return m } diff --git a/gnovm/tests/files/cycles.gno b/gnovm/tests/files/cycles.gno deleted file mode 100644 index 95c7f35c02d..00000000000 --- a/gnovm/tests/files/cycles.gno +++ /dev/null @@ -1,12 +0,0 @@ -package main - -func main() { - recurse() -} - -func recurse() int { - return recurse() -} - -// Error: -// CPU cycle overrun diff --git a/gnovm/tests/machine_test.go b/gnovm/tests/machine_test.go index c85fb33565e..a67d67f1ff2 100644 --- a/gnovm/tests/machine_test.go +++ b/gnovm/tests/machine_test.go @@ -47,7 +47,6 @@ func TestMachineTestMemPackage(t *testing.T) { Output: os.Stdout, Store: store, Context: nil, - IsTest: true, }) memPkg := gno.ReadMemPackage(tt.path, "test") From e03cf4495a0d2140b4ab9c79d74f9ad85ac99e33 Mon Sep 17 00:00:00 2001 From: deelawn Date: Wed, 27 Mar 2024 16:24:24 -0700 Subject: [PATCH 7/7] do not allow max cycles to be configurable --- contribs/gnodev/pkg/dev/node.go | 7 +++---- gno.land/cmd/gnoland/start.go | 10 +--------- gno.land/pkg/gnoland/app.go | 5 ++--- gno.land/pkg/gnoland/node_inmemory.go | 9 +++------ gno.land/pkg/sdk/vm/common_test.go | 2 +- gno.land/pkg/sdk/vm/keeper.go | 8 +------- 6 files changed, 11 insertions(+), 30 deletions(-) diff --git a/contribs/gnodev/pkg/dev/node.go b/contribs/gnodev/pkg/dev/node.go index 23ac66a5f9a..5d5a4c5c5e6 100644 --- a/contribs/gnodev/pkg/dev/node.go +++ b/contribs/gnodev/pkg/dev/node.go @@ -410,9 +410,8 @@ func newNodeConfig(tmc *tmcfg.Config, chainid string, appstate gnoland.GnoGenesi } return &gnoland.InMemoryNodeConfig{ - PrivValidator: pv, - TMConfig: tmc, - Genesis: genesis, - GenesisMaxVMCycles: 10_000_000, + PrivValidator: pv, + TMConfig: tmc, + Genesis: genesis, } } diff --git a/gno.land/cmd/gnoland/start.go b/gno.land/cmd/gnoland/start.go index 2b1757706f8..7fa7de32e5c 100644 --- a/gno.land/cmd/gnoland/start.go +++ b/gno.land/cmd/gnoland/start.go @@ -36,7 +36,6 @@ type startCfg struct { chainID string genesisRemote string dataDir string - genesisMaxVMCycles int64 config string txEventStoreType string @@ -125,13 +124,6 @@ func (c *startCfg) RegisterFlags(fs *flag.FlagSet) { "replacement for '%%REMOTE%%' in genesis", ) - fs.Int64Var( - &c.genesisMaxVMCycles, - "genesis-max-vm-cycles", - 10_000_000, - "set maximum allowed vm cycles per operation. Zero means no limit.", - ) - fs.StringVar( &c.config, flagConfigFlag, @@ -254,7 +246,7 @@ func execStart(c *startCfg, io commands.IO) error { cfg.TxEventStore = txEventStoreCfg // Create application and node. - gnoApp, err := gnoland.NewApp(dataDir, c.skipFailingGenesisTxs, logger, c.genesisMaxVMCycles) + gnoApp, err := gnoland.NewApp(dataDir, c.skipFailingGenesisTxs, logger) if err != nil { return fmt.Errorf("error in creating new app: %w", err) } diff --git a/gno.land/pkg/gnoland/app.go b/gno.land/pkg/gnoland/app.go index cc15f74134e..86fb6321386 100644 --- a/gno.land/pkg/gnoland/app.go +++ b/gno.land/pkg/gnoland/app.go @@ -31,7 +31,6 @@ type AppOptions struct { GnoRootDir string SkipFailingGenesisTxs bool Logger *slog.Logger - MaxCycles int64 } func NewAppOptions() *AppOptions { @@ -78,7 +77,7 @@ func NewAppWithOptions(cfg *AppOptions) (abci.Application, error) { // XXX: Embed this ? stdlibsDir := filepath.Join(cfg.GnoRootDir, "gnovm", "stdlibs") - vmKpr := vm.NewVMKeeper(baseKey, mainKey, acctKpr, bankKpr, stdlibsDir, cfg.MaxCycles) + vmKpr := vm.NewVMKeeper(baseKey, mainKey, acctKpr, bankKpr, stdlibsDir) // Set InitChainer baseApp.SetInitChainer(InitChainer(baseApp, acctKpr, bankKpr, cfg.SkipFailingGenesisTxs)) @@ -123,7 +122,7 @@ func NewAppWithOptions(cfg *AppOptions) (abci.Application, error) { } // NewApp creates the GnoLand application. -func NewApp(dataRootDir string, skipFailingGenesisTxs bool, logger *slog.Logger, maxCycles int64) (abci.Application, error) { +func NewApp(dataRootDir string, skipFailingGenesisTxs bool, logger *slog.Logger) (abci.Application, error) { var err error cfg := NewAppOptions() diff --git a/gno.land/pkg/gnoland/node_inmemory.go b/gno.land/pkg/gnoland/node_inmemory.go index 89f222738d0..d8dc3caa485 100644 --- a/gno.land/pkg/gnoland/node_inmemory.go +++ b/gno.land/pkg/gnoland/node_inmemory.go @@ -25,7 +25,6 @@ type InMemoryNodeConfig struct { Genesis *bft.GenesisDoc TMConfig *tmcfg.Config SkipFailingGenesisTxs bool - GenesisMaxVMCycles int64 } // NewMockedPrivValidator generate a new key @@ -79,10 +78,9 @@ func NewDefaultInMemoryNodeConfig(rootdir string) *InMemoryNodeConfig { } return &InMemoryNodeConfig{ - PrivValidator: pv, - TMConfig: tm, - Genesis: genesis, - GenesisMaxVMCycles: 10_000_000, + PrivValidator: pv, + TMConfig: tm, + Genesis: genesis, } } @@ -115,7 +113,6 @@ func NewInMemoryNode(logger *slog.Logger, cfg *InMemoryNodeConfig) (*node.Node, Logger: logger, GnoRootDir: cfg.TMConfig.RootDir, SkipFailingGenesisTxs: cfg.SkipFailingGenesisTxs, - MaxCycles: cfg.GenesisMaxVMCycles, DB: memdb.NewMemDB(), }) if err != nil { diff --git a/gno.land/pkg/sdk/vm/common_test.go b/gno.land/pkg/sdk/vm/common_test.go index b65757da403..ec14ae8515b 100644 --- a/gno.land/pkg/sdk/vm/common_test.go +++ b/gno.land/pkg/sdk/vm/common_test.go @@ -39,7 +39,7 @@ func setupTestEnv() testEnv { acck := authm.NewAccountKeeper(iavlCapKey, std.ProtoBaseAccount) bank := bankm.NewBankKeeper(acck) stdlibsDir := filepath.Join("..", "..", "..", "..", "gnovm", "stdlibs") - vmk := NewVMKeeper(baseCapKey, iavlCapKey, acck, bank, stdlibsDir, 10_000_000) + vmk := NewVMKeeper(baseCapKey, iavlCapKey, acck, bank, stdlibsDir) vmk.Initialize(ms.MultiCacheWrap()) diff --git a/gno.land/pkg/sdk/vm/keeper.go b/gno.land/pkg/sdk/vm/keeper.go index cc9a23bbff1..a6df5853b8b 100644 --- a/gno.land/pkg/sdk/vm/keeper.go +++ b/gno.land/pkg/sdk/vm/keeper.go @@ -60,13 +60,7 @@ func NewVMKeeper( acck auth.AccountKeeper, bank bank.BankKeeper, stdlibsDir string, - maxCycles int64, ) *VMKeeper { - cycles := maxCycles - if cycles <= 0 || cycles > maxVMCycles { - cycles = maxVMCycles - } - // TODO: create an Options struct to avoid too many constructor parameters vmk := &VMKeeper{ baseKey: baseKey, @@ -74,7 +68,7 @@ func NewVMKeeper( acck: acck, bank: bank, stdlibsDir: stdlibsDir, - maxCycles: cycles, + maxCycles: maxVMCycles, } return vmk }