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.

Sign up for GitHub

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

Jump to bottom

fix: hardcode max vm cycles in keeper #1807

Merged
merged 7 commits into from
Mar 30, 2024
Merged

fix: hardcode max vm cycles in keeper #1807

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
99569d6
guard against infinite cycles where provisioning a machine
deelawn Mar 21, 2024
da74367
avoid printing massive machine state
deelawn Mar 21, 2024
f6ffe76
added cpu cycle error test
deelawn Mar 21, 2024
50666c6
don't limit cycles for testing for convenience
deelawn Mar 21, 2024
bac5dec
added machine test flag
deelawn Mar 21, 2024
b940ee8
reworked to restrict max cycles to the keeper
deelawn Mar 26, 2024
e03cf44
do not allow max cycles to be configurable
deelawn Mar 27, 2024
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
7 changes: 3 additions & 4 deletions contribs/gnodev/pkg/dev/node.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
10 changes: 1 addition & 9 deletions gno.land/cmd/gnoland/start.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ type startCfg struct {
chainID string
genesisRemote string
dataDir string
genesisMaxVMCycles int64
config string

txEventStoreType string
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
}
Expand Down
5 changes: 2 additions & 3 deletions gno.land/pkg/gnoland/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
GnoRootDir string
SkipFailingGenesisTxs bool
Logger *slog.Logger
MaxCycles int64
}

func NewAppOptions() *AppOptions {
Expand Down Expand Up @@ -78,7 +77,7 @@

// 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)

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

View check run for this annotation

Codecov / codecov/patch

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

Added line #L80 was not covered by tests

// Set InitChainer
baseApp.SetInitChainer(InitChainer(baseApp, acctKpr, bankKpr, cfg.SkipFailingGenesisTxs))
Expand Down Expand Up @@ -123,7 +122,7 @@
}

// 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) {

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

View check run for this annotation

Codecov / codecov/patch

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

Added line #L125 was not covered by tests
var err error

cfg := NewAppOptions()
Expand Down
9 changes: 3 additions & 6 deletions gno.land/pkg/gnoland/node_inmemory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
Genesis *bft.GenesisDoc
TMConfig *tmcfg.Config
SkipFailingGenesisTxs bool
GenesisMaxVMCycles int64
}

// NewMockedPrivValidator generate a new key
Expand Down Expand Up @@ -79,10 +78,9 @@
}

return &InMemoryNodeConfig{
PrivValidator: pv,
TMConfig: tm,
Genesis: genesis,
GenesisMaxVMCycles: 10_000_000,
PrivValidator: pv,
TMConfig: tm,
Genesis: genesis,

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

View check run for this annotation

Codecov / codecov/patch

gno.land/pkg/gnoland/node_inmemory.go#L81-L83 

Added lines #L81 - L83 were not covered by tests
}
}

Expand Down Expand Up @@ -115,7 +113,6 @@
Logger: logger,
GnoRootDir: cfg.TMConfig.RootDir,
SkipFailingGenesisTxs: cfg.SkipFailingGenesisTxs,
MaxCycles: cfg.GenesisMaxVMCycles,
DB: memdb.NewMemDB(),
})
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion gno.land/pkg/sdk/vm/common_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())

Expand Down
8 changes: 6 additions & 2 deletions gno.land/pkg/sdk/vm/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -55,7 +60,6 @@ func NewVMKeeper(
acck auth.AccountKeeper,
bank bank.BankKeeper,
stdlibsDir string,
maxCycles int64,
) *VMKeeper {
// TODO: create an Options struct to avoid too many constructor parameters
vmk := &VMKeeper{
Expand All @@ -64,7 +68,7 @@ func NewVMKeeper(
acck: acck,
bank: bank,
stdlibsDir: stdlibsDir,
maxCycles: maxCycles,
maxCycles: maxVMCycles,
}
return vmk
}
Expand Down
Loading