From d69b5529e938f89b05b449fcd0a0bdcf7bcdc09e Mon Sep 17 00:00:00 2001 From: Morgan Date: Wed, 18 Dec 2024 18:12:37 +0100 Subject: [PATCH 1/4] feat(cmd/gno): re-use teststore in `lint` (#3315) continuing the work of #3119. this PR makes `gno lint` re-use the same test.Store, speeding up execution (especially on the CI, which lints all packages) by not needing to load all packages for each package that is linted. --- .../pkg/integration/testing_integration.go | 6 +- gnovm/cmd/gno/download_deps.go | 4 +- gnovm/cmd/gno/lint.go | 55 ++++++++++++------- gnovm/cmd/gno/mod.go | 5 +- gnovm/cmd/gno/testdata/lint/bad_import.txtar | 1 - gnovm/pkg/doc/dirs.go | 10 +++- gnovm/pkg/gnomod/pkg.go | 18 +++--- gnovm/pkg/packages/imports.go | 47 +++++++++------- gnovm/pkg/packages/imports_test.go | 8 ++- gnovm/pkg/test/filetest.go | 11 +++- gnovm/pkg/test/imports.go | 14 ++--- gnovm/pkg/test/test.go | 10 ++-- 12 files changed, 111 insertions(+), 78 deletions(-) diff --git a/gno.land/pkg/integration/testing_integration.go b/gno.land/pkg/integration/testing_integration.go index ce1413134e3..0a181950bb3 100644 --- a/gno.land/pkg/integration/testing_integration.go +++ b/gno.land/pkg/integration/testing_integration.go @@ -753,15 +753,15 @@ func (pl *pkgsLoader) LoadPackage(modroot string, path, name string) error { if err != nil { return fmt.Errorf("unable to read package at %q: %w", currentPkg.Dir, err) } - imports, err := packages.Imports(pkg) + imports, err := packages.Imports(pkg, nil) if err != nil { return fmt.Errorf("unable to load package imports in %q: %w", currentPkg.Dir, err) } for _, imp := range imports { - if imp == currentPkg.Name || gnolang.IsStdlib(imp) { + if imp.PkgPath == currentPkg.Name || gnolang.IsStdlib(imp.PkgPath) { continue } - currentPkg.Imports = append(currentPkg.Imports, imp) + currentPkg.Imports = append(currentPkg.Imports, imp.PkgPath) } } diff --git a/gnovm/cmd/gno/download_deps.go b/gnovm/cmd/gno/download_deps.go index d19de9dd338..5a8c50be20b 100644 --- a/gnovm/cmd/gno/download_deps.go +++ b/gnovm/cmd/gno/download_deps.go @@ -25,13 +25,13 @@ func downloadDeps(io commands.IO, pkgDir string, gnoMod *gnomod.File, fetcher pk if err != nil { return fmt.Errorf("read package at %q: %w", pkgDir, err) } - imports, err := packages.Imports(pkg) + imports, err := packages.Imports(pkg, nil) if err != nil { return fmt.Errorf("read imports at %q: %w", pkgDir, err) } for _, pkgPath := range imports { - resolved := gnoMod.Resolve(module.Version{Path: pkgPath}) + resolved := gnoMod.Resolve(module.Version{Path: pkgPath.PkgPath}) resolvedPkgPath := resolved.Path if !isRemotePkgPath(resolvedPkgPath) { diff --git a/gnovm/cmd/gno/lint.go b/gnovm/cmd/gno/lint.go index a3e7f5310e1..8a1256772df 100644 --- a/gnovm/cmd/gno/lint.go +++ b/gnovm/cmd/gno/lint.go @@ -7,7 +7,7 @@ import ( "fmt" "go/scanner" "go/types" - "io" + goio "io" "os" "path/filepath" "regexp" @@ -97,6 +97,11 @@ func execLint(cfg *lintCfg, args []string, io commands.IO) error { hasError := false + bs, ts := test.Store( + rootDir, false, + nopReader{}, goio.Discard, goio.Discard, + ) + for _, pkgPath := range pkgPaths { if verbose { io.ErrPrintln(pkgPath) @@ -120,12 +125,6 @@ func execLint(cfg *lintCfg, args []string, io commands.IO) error { hasError = true } - stdout, stdin, stderr := io.Out(), io.In(), io.Err() - _, testStore := test.Store( - rootDir, false, - stdin, stdout, stderr, - ) - memPkg, err := gno.ReadMemPackage(pkgPath, pkgPath) if err != nil { io.ErrPrintln(issueFromError(pkgPath, err).String()) @@ -133,22 +132,34 @@ func execLint(cfg *lintCfg, args []string, io commands.IO) error { continue } - // Run type checking - if gmFile == nil || !gmFile.Draft { - foundErr, err := lintTypeCheck(io, memPkg, testStore) - if err != nil { - io.ErrPrintln(err) - hasError = true - } else if foundErr { - hasError = true - } - } else if verbose { - io.ErrPrintfln("%s: module is draft, skipping type check", pkgPath) + // Perform imports using the parent store. + if err := test.LoadImports(ts, memPkg); err != nil { + io.ErrPrintln(issueFromError(pkgPath, err).String()) + hasError = true + continue } // Handle runtime errors hasRuntimeErr := catchRuntimeError(pkgPath, io.Err(), func() { - tm := test.Machine(testStore, stdout, memPkg.Path) + // Wrap in cache wrap so execution of the linter doesn't impact + // other packages. + cw := bs.CacheWrap() + gs := ts.BeginTransaction(cw, cw) + + // Run type checking + if gmFile == nil || !gmFile.Draft { + foundErr, err := lintTypeCheck(io, memPkg, gs) + if err != nil { + io.ErrPrintln(err) + hasError = true + } else if foundErr { + hasError = true + } + } else if verbose { + io.ErrPrintfln("%s: module is draft, skipping type check", pkgPath) + } + + tm := test.Machine(gs, goio.Discard, memPkg.Path) defer tm.Release() // Check package @@ -253,7 +264,7 @@ func guessSourcePath(pkg, source string) string { // XXX: Ideally, error handling should encapsulate location details within a dedicated error type. var reParseRecover = regexp.MustCompile(`^([^:]+)((?::(?:\d+)){1,2}):? *(.*)$`) -func catchRuntimeError(pkgPath string, stderr io.WriteCloser, action func()) (hasError bool) { +func catchRuntimeError(pkgPath string, stderr goio.WriteCloser, action func()) (hasError bool) { defer func() { // Errors catched here mostly come from: gnovm/pkg/gnolang/preprocess.go r := recover() @@ -307,3 +318,7 @@ func issueFromError(pkgPath string, err error) lintIssue { } return issue } + +type nopReader struct{} + +func (nopReader) Read(p []byte) (int, error) { return 0, goio.EOF } diff --git a/gnovm/cmd/gno/mod.go b/gnovm/cmd/gno/mod.go index f762b070fe4..5479d934ce6 100644 --- a/gnovm/cmd/gno/mod.go +++ b/gnovm/cmd/gno/mod.go @@ -362,15 +362,12 @@ func getImportToFilesMap(pkgPath string) (map[string][]string, error) { if err != nil { return nil, err } - imports, _, err := packages.FileImports(filename, string(data)) + imports, err := packages.FileImports(filename, string(data), nil) if err != nil { return nil, err } for _, imp := range imports { - if imp.Error != nil { - return nil, err - } m[imp.PkgPath] = append(m[imp.PkgPath], filename) } } diff --git a/gnovm/cmd/gno/testdata/lint/bad_import.txtar b/gnovm/cmd/gno/testdata/lint/bad_import.txtar index b5edbdd0223..e2c0431443c 100644 --- a/gnovm/cmd/gno/testdata/lint/bad_import.txtar +++ b/gnovm/cmd/gno/testdata/lint/bad_import.txtar @@ -19,5 +19,4 @@ module gno.land/p/test -- stdout.golden -- -- stderr.golden -- -bad_file.gno:3:8: could not import python (import not found: python) (code=4) bad_file.gno:3:8: unknown import path python (code=2) diff --git a/gnovm/pkg/doc/dirs.go b/gnovm/pkg/doc/dirs.go index eadbec7d464..b287fd20708 100644 --- a/gnovm/pkg/doc/dirs.go +++ b/gnovm/pkg/doc/dirs.go @@ -105,10 +105,14 @@ func packageImportsRecursive(root string, pkgPath string) []string { pkg = &gnovm.MemPackage{} } - res, err := packages.Imports(pkg) + resRaw, err := packages.Imports(pkg, nil) if err != nil { // ignore packages with invalid imports - res = nil + resRaw = nil + } + res := make([]string, len(resRaw)) + for idx, imp := range resRaw { + res[idx] = imp.PkgPath } entries, err := os.ReadDir(root) @@ -127,7 +131,7 @@ func packageImportsRecursive(root string, pkgPath string) []string { for _, imp := range sub { if !slices.Contains(res, imp) { - res = append(res, imp) + res = append(res, imp) //nolint:makezero } } } diff --git a/gnovm/pkg/gnomod/pkg.go b/gnovm/pkg/gnomod/pkg.go index 35f52e3dded..a0831d494b0 100644 --- a/gnovm/pkg/gnomod/pkg.go +++ b/gnovm/pkg/gnomod/pkg.go @@ -5,7 +5,6 @@ import ( "io/fs" "os" "path/filepath" - "slices" "strings" "github.com/gnolang/gno/gnovm" @@ -122,16 +121,19 @@ func ListPkgs(root string) (PkgList, error) { pkg = &gnovm.MemPackage{} } - imports, err := packages.Imports(pkg) + importsRaw, err := packages.Imports(pkg, nil) if err != nil { // ignore imports on error - imports = []string{} + importsRaw = nil + } + imports := make([]string, 0, len(importsRaw)) + for _, imp := range importsRaw { + // remove self and standard libraries from imports + if imp.PkgPath != gnoMod.Module.Mod.Path && + !gnolang.IsStdlib(imp.PkgPath) { + imports = append(imports, imp.PkgPath) + } } - - // remove self and standard libraries from imports - imports = slices.DeleteFunc(imports, func(imp string) bool { - return imp == gnoMod.Module.Mod.Path || gnolang.IsStdlib(imp) - }) pkgs = append(pkgs, Pkg{ Dir: path, diff --git a/gnovm/pkg/packages/imports.go b/gnovm/pkg/packages/imports.go index e72f37276db..201965bc588 100644 --- a/gnovm/pkg/packages/imports.go +++ b/gnovm/pkg/packages/imports.go @@ -13,9 +13,10 @@ import ( ) // Imports returns the list of gno imports from a [gnovm.MemPackage]. -func Imports(pkg *gnovm.MemPackage) ([]string, error) { - allImports := make([]string, 0) - seen := make(map[string]struct{}) +// fset is optional. +func Imports(pkg *gnovm.MemPackage, fset *token.FileSet) ([]FileImport, error) { + allImports := make([]FileImport, 0, 16) + seen := make(map[string]struct{}, 16) for _, file := range pkg.Files { if !strings.HasSuffix(file.Name, ".gno") { continue @@ -23,50 +24,54 @@ func Imports(pkg *gnovm.MemPackage) ([]string, error) { if strings.HasSuffix(file.Name, "_filetest.gno") { continue } - imports, _, err := FileImports(file.Name, file.Body) + imports, err := FileImports(file.Name, file.Body, fset) if err != nil { return nil, err } for _, im := range imports { - if im.Error != nil { - return nil, err - } if _, ok := seen[im.PkgPath]; ok { continue } - allImports = append(allImports, im.PkgPath) + allImports = append(allImports, im) seen[im.PkgPath] = struct{}{} } } - sort.Strings(allImports) + sort.Slice(allImports, func(i, j int) bool { + return allImports[i].PkgPath < allImports[j].PkgPath + }) return allImports, nil } +// FileImport represents an import type FileImport struct { PkgPath string Spec *ast.ImportSpec - Error error } // FileImports returns the list of gno imports in the given file src. // The given filename is only used when recording position information. -func FileImports(filename string, src string) ([]*FileImport, *token.FileSet, error) { - fs := token.NewFileSet() - f, err := parser.ParseFile(fs, filename, src, parser.ImportsOnly) +func FileImports(filename string, src string, fset *token.FileSet) ([]FileImport, error) { + if fset == nil { + fset = token.NewFileSet() + } + f, err := parser.ParseFile(fset, filename, src, parser.ImportsOnly) if err != nil { - return nil, nil, err + return nil, err } - res := make([]*FileImport, len(f.Imports)) + res := make([]FileImport, len(f.Imports)) for i, im := range f.Imports { - fi := FileImport{Spec: im} importPath, err := strconv.Unquote(im.Path.Value) if err != nil { - fi.Error = fmt.Errorf("%v: unexpected invalid import path: %v", fs.Position(im.Pos()).String(), im.Path.Value) - } else { - fi.PkgPath = importPath + // should not happen - parser.ParseFile should already ensure we get + // a valid string literal here. + panic(fmt.Errorf("%v: unexpected invalid import path: %v", fset.Position(im.Pos()).String(), im.Path.Value)) + } + + res[i] = FileImport{ + PkgPath: importPath, + Spec: im, } - res[i] = &fi } - return res, fs, nil + return res, nil } diff --git a/gnovm/pkg/packages/imports_test.go b/gnovm/pkg/packages/imports_test.go index 14808dcbd6f..3750aa9108c 100644 --- a/gnovm/pkg/packages/imports_test.go +++ b/gnovm/pkg/packages/imports_test.go @@ -120,8 +120,12 @@ func TestImports(t *testing.T) { pkg, err := gnolang.ReadMemPackage(tmpDir, "test") require.NoError(t, err) - imports, err := Imports(pkg) + imports, err := Imports(pkg, nil) require.NoError(t, err) + importsStrings := make([]string, len(imports)) + for idx, imp := range imports { + importsStrings[idx] = imp.PkgPath + } - require.Equal(t, expected, imports) + require.Equal(t, expected, importsStrings) } diff --git a/gnovm/pkg/test/filetest.go b/gnovm/pkg/test/filetest.go index da2c7a55797..1934f429568 100644 --- a/gnovm/pkg/test/filetest.go +++ b/gnovm/pkg/test/filetest.go @@ -175,12 +175,20 @@ type runResult struct { } func (opts *TestOptions) runTest(m *gno.Machine, pkgPath, filename string, content []byte) (rr runResult) { + pkgName := gno.Name(pkgPath[strings.LastIndexByte(pkgPath, '/')+1:]) + // Eagerly load imports. // This is executed using opts.Store, rather than the transaction store; // it allows us to only have to load the imports once (and re-use the cached // versions). Running the tests in separate "transactions" means that they // don't get the parent store dirty. - if err := LoadImports(opts.TestStore, filename, content); err != nil { + if err := LoadImports(opts.TestStore, &gnovm.MemPackage{ + Name: string(pkgName), + Path: pkgPath, + Files: []*gnovm.MemFile{ + {Name: filename, Body: string(content)}, + }, + }); err != nil { // NOTE: we perform this here, so we can capture the runResult. return runResult{Error: err.Error()} } @@ -210,7 +218,6 @@ func (opts *TestOptions) runTest(m *gno.Machine, pkgPath, filename string, conte }() // Use last element after / (works also if slash is missing). - pkgName := gno.Name(pkgPath[strings.LastIndexByte(pkgPath, '/')+1:]) if !gno.IsRealmPath(pkgPath) { // Simple case - pure package. pn := gno.NewPackageNode(pkgName, pkgPath, &gno.FileSet{}) diff --git a/gnovm/pkg/test/imports.go b/gnovm/pkg/test/imports.go index 731bf9756dd..8b24fdeaa77 100644 --- a/gnovm/pkg/test/imports.go +++ b/gnovm/pkg/test/imports.go @@ -4,6 +4,7 @@ import ( "encoding/json" "errors" "fmt" + "go/token" "io" "math/big" "os" @@ -12,6 +13,7 @@ import ( "strings" "time" + "github.com/gnolang/gno/gnovm" gno "github.com/gnolang/gno/gnovm/pkg/gnolang" "github.com/gnolang/gno/gnovm/pkg/packages" teststdlibs "github.com/gnolang/gno/gnovm/tests/stdlibs" @@ -214,13 +216,13 @@ func (e *stackWrappedError) String() string { return fmt.Sprintf("%v\nstack:\n%v", e.err, string(e.stack)) } -// LoadImports parses the given file and attempts to retrieve all pure packages +// LoadImports parses the given MemPackage and attempts to retrieve all pure packages // from the store. This is mostly useful for "eager import loading", whereby all // imports are pre-loaded in a permanent store, so that the tests can use // ephemeral transaction stores. -func LoadImports(store gno.Store, filename string, content []byte) (err error) { +func LoadImports(store gno.Store, memPkg *gnovm.MemPackage) (err error) { defer func() { - // This is slightly different from the handling below; we do not have a + // This is slightly different from other similar error handling; we do not have a // machine to work with, as this comes from an import; so we need // "machine-less" alternatives. (like v.String instead of v.Sprint) if r := recover(); r != nil { @@ -239,14 +241,12 @@ func LoadImports(store gno.Store, filename string, content []byte) (err error) { } }() - imports, fset, err := packages.FileImports(filename, string(content)) + fset := token.NewFileSet() + imports, err := packages.Imports(memPkg, fset) if err != nil { return err } for _, imp := range imports { - if imp.Error != nil { - return imp.Error - } if gno.IsRealmPath(imp.PkgPath) { // Don't eagerly load realms. // Realms persist state and can change the state of other realms in initialization. diff --git a/gnovm/pkg/test/test.go b/gnovm/pkg/test/test.go index 077abbe8984..80f56e66d2e 100644 --- a/gnovm/pkg/test/test.go +++ b/gnovm/pkg/test/test.go @@ -9,7 +9,6 @@ import ( "io" "math" "os" - "path" "path/filepath" "runtime/debug" "strconv" @@ -182,6 +181,11 @@ func Test(memPkg *gnovm.MemPackage, fsDir string, opts *TestOptions) error { var errs error + // Eagerly load imports. + if err := LoadImports(opts.TestStore, memPkg); err != nil { + return err + } + // Stands for "test", "integration test", and "filetest". // "integration test" are the test files with `package xxx_test` (they are // not necessarily integration tests, it's just for our internal reference.) @@ -424,10 +428,6 @@ func parseMemPackageTests(store gno.Store, memPkg *gnovm.MemPackage) (tset, itse continue // skip this file. } - if err := LoadImports(store, path.Join(memPkg.Path, mfile.Name), []byte(mfile.Body)); err != nil { - errs = multierr.Append(errs, err) - } - n, err := gno.ParseFile(mfile.Name, mfile.Body) if err != nil { errs = multierr.Append(errs, err) From c8cd8f4b6ccbe9f4ee5622032228553496186d51 Mon Sep 17 00:00:00 2001 From: David Manpearl Date: Thu, 19 Dec 2024 01:56:58 -0800 Subject: [PATCH 2/4] =?UTF-8?q?fix(lint):=20fix=20'make=20install'=20failu?= =?UTF-8?q?re=20due=20to=20bad=20'BeginTransaction'=20c=E2=80=A6=20(#3369)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses #3368 …all in lint.io lint.go: add missing 'gasMeter' param to 'BeginTransaction' call Impact: caused 'make install' to fail on branch 'master' with the following error: cmd/gno/lint.go:147:34: not enough arguments in call to ts.BeginTransaction have ("github.com/gnolang/gno/tm2/pkg/store/types".Store, "github.com/gnolang/gno/tm2/pkg/store/types".Store) want ("github.com/gnolang/gno/tm2/pkg/store/types".Store, "github.com/gnolang/gno/tm2/pkg/store/types".Store, "github.com/gnolang/gno/tm2/pkg/store/types".GasMeter) make[1]: *** [install] Error 1 Testing: This fix resolves test failure commanded by `make test`: FAIL github.com/gnolang/gno/gnovm/cmd/gno [build failed] --- gnovm/cmd/gno/lint.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gnovm/cmd/gno/lint.go b/gnovm/cmd/gno/lint.go index 8a1256772df..ce3465b484e 100644 --- a/gnovm/cmd/gno/lint.go +++ b/gnovm/cmd/gno/lint.go @@ -144,7 +144,7 @@ func execLint(cfg *lintCfg, args []string, io commands.IO) error { // Wrap in cache wrap so execution of the linter doesn't impact // other packages. cw := bs.CacheWrap() - gs := ts.BeginTransaction(cw, cw) + gs := ts.BeginTransaction(cw, cw, nil) // Run type checking if gmFile == nil || !gmFile.Draft { From 018ef433f6c48b256575eb8f83d856e32d916989 Mon Sep 17 00:00:00 2001 From: Miguel Victoria Villaquiran Date: Thu, 19 Dec 2024 07:45:01 -0500 Subject: [PATCH 3/4] fix: stdlib_diff published on pages (#3367) --- .github/workflows/gh-pages.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/gh-pages.yml b/.github/workflows/gh-pages.yml index 993c11b5941..3baf8e1ee8a 100644 --- a/.github/workflows/gh-pages.yml +++ b/.github/workflows/gh-pages.yml @@ -28,10 +28,10 @@ jobs: - run: echo $GOROOT - run: "cd misc/stdlib_diff && make gen" - run: "cd misc/gendocs && make install gen" - - run: "mkdir -p pages_ouput/stdlib_diff" + - run: "mkdir -p pages_output/stdlib_diff" - run: | - mv misc/gendocs/godoc pages_output - mv misc/stdlib_diff/stdlib_diff pages_ouput/stdlib_diff + cp -r misc/gendocs/godoc/* pages_output/ + cp -r misc/stdlib_diff/stdlib_diff/* pages_output/stdlib_diff/ - uses: actions/configure-pages@v5 id: pages - uses: actions/upload-pages-artifact@v3 From 87a503514c52fe3ec5543a1aca71a1ab4eca82cc Mon Sep 17 00:00:00 2001 From: Morgan Date: Thu, 19 Dec 2024 14:49:54 +0100 Subject: [PATCH 4/4] refactor(gno.land): use errors.As to detect out of gas exceptions (#3320) --- .../gnoland/testdata/addpkg_outofgas.txtar | 9 +- gno.land/pkg/sdk/vm/gas_test.go | 4 +- gno.land/pkg/sdk/vm/keeper.go | 151 +++++++----------- gnovm/pkg/gnolang/preprocess.go | 9 -- tm2/pkg/sdk/auth/ante.go | 2 +- tm2/pkg/sdk/baseapp.go | 2 +- tm2/pkg/store/exports.go | 4 +- tm2/pkg/store/types/gas.go | 22 ++- 8 files changed, 83 insertions(+), 120 deletions(-) diff --git a/gno.land/cmd/gnoland/testdata/addpkg_outofgas.txtar b/gno.land/cmd/gnoland/testdata/addpkg_outofgas.txtar index 56050f4733b..fc536b705c6 100644 --- a/gno.land/cmd/gnoland/testdata/addpkg_outofgas.txtar +++ b/gno.land/cmd/gnoland/testdata/addpkg_outofgas.txtar @@ -7,17 +7,12 @@ gnoland start gnokey maketx addpkg -pkgdir $WORK/foo -pkgpath gno.land/r/foo -gas-fee 1000000ugnot -gas-wanted 220000 -broadcast -chainid=tendermint_test test1 -# add bar package -# out of gas at store.GetPackage() with gas 60000 +# add bar package - out of gas at store.GetPackage() with gas 60000 ! gnokey maketx addpkg -pkgdir $WORK/bar -pkgpath gno.land/r/bar -gas-fee 1000000ugnot -gas-wanted 60000 -broadcast -chainid=tendermint_test test1 -# Out of gas error - stderr '--= Error =--' stderr 'Data: out of gas error' -stderr 'Msg Traces:' -stderr 'out of gas.*?in preprocess' stderr '--= /Error =--' @@ -28,8 +23,6 @@ stderr '--= /Error =--' stderr '--= Error =--' stderr 'Data: out of gas error' -stderr 'Msg Traces:' -stderr 'out of gas.*?in preprocess' stderr '--= /Error =--' diff --git a/gno.land/pkg/sdk/vm/gas_test.go b/gno.land/pkg/sdk/vm/gas_test.go index 0001e3acf7c..276aa9db0b0 100644 --- a/gno.land/pkg/sdk/vm/gas_test.go +++ b/gno.land/pkg/sdk/vm/gas_test.go @@ -38,7 +38,7 @@ func TestAddPkgDeliverTxInsuffGas(t *testing.T) { defer func() { if r := recover(); r != nil { switch r.(type) { - case store.OutOfGasException: + case store.OutOfGasError: res.Error = sdk.ABCIError(std.ErrOutOfGas("")) abort = true default: @@ -117,7 +117,7 @@ func TestAddPkgDeliverTxFailedNoGas(t *testing.T) { defer func() { if r := recover(); r != nil { switch r.(type) { - case store.OutOfGasException: + case store.OutOfGasError: res.Error = sdk.ABCIError(std.ErrOutOfGas("")) abort = true default: diff --git a/gno.land/pkg/sdk/vm/keeper.go b/gno.land/pkg/sdk/vm/keeper.go index f158524b3df..bf16cd44243 100644 --- a/gno.land/pkg/sdk/vm/keeper.go +++ b/gno.land/pkg/sdk/vm/keeper.go @@ -5,6 +5,7 @@ package vm import ( "bytes" "context" + goerrors "errors" "fmt" "io" "log/slog" @@ -392,18 +393,7 @@ func (vm *VMKeeper) AddPackage(ctx sdk.Context, msg MsgAddPackage) (err error) { GasMeter: ctx.GasMeter(), }) defer m2.Release() - defer func() { - if r := recover(); r != nil { - switch r.(type) { - case store.OutOfGasException: // panic in consumeGas() - panic(r) - default: - err = errors.Wrapf(fmt.Errorf("%v", r), "VM addpkg panic: %v\n%s\n", - r, m2.String()) - return - } - } - }() + defer doRecover(m2, &err) m2.RunMemPackage(memPkg, true) // Log the telemetry @@ -495,21 +485,7 @@ func (vm *VMKeeper) Call(ctx sdk.Context, msg MsgCall) (res string, err error) { }) defer m.Release() m.SetActivePackage(mpv) - defer func() { - if r := recover(); r != nil { - switch r := r.(type) { - case store.OutOfGasException: // panic in consumeGas() - panic(r) - case gno.UnhandledPanicError: - err = errors.Wrapf(fmt.Errorf("%v", r.Error()), "VM call panic: %s\nStacktrace: %s\n", - r.Error(), m.ExceptionsStacktrace()) - default: - err = errors.Wrapf(fmt.Errorf("%v", r), "VM call panic: %v\nMachine State:%s\nStacktrace: %s\n", - r, m.String(), m.Stacktrace().String()) - return - } - } - }() + defer doRecover(m, &err) rtvs := m.Eval(xn) for i, rtv := range rtvs { res = res + rtv.String() @@ -534,6 +510,35 @@ func (vm *VMKeeper) Call(ctx sdk.Context, msg MsgCall) (res string, err error) { // TODO pay for gas? TODO see context? } +func doRecover(m *gno.Machine, e *error) { + r := recover() + if r == nil { + return + } + if err, ok := r.(error); ok { + var oog types.OutOfGasError + if goerrors.As(err, &oog) { + // Re-panic and don't wrap. + panic(oog) + } + var up gno.UnhandledPanicError + if goerrors.As(err, &up) { + // Common unhandled panic error, skip machine state. + *e = errors.Wrapf( + errors.New(up.Descriptor), + "VM panic: %s\nStacktrace: %s\n", + up.Descriptor, m.ExceptionsStacktrace(), + ) + return + } + } + *e = errors.Wrapf( + fmt.Errorf("%v", r), + "VM panic: %v\nMachine State:%s\nStacktrace: %s\n", + r, m.String(), m.Stacktrace().String(), + ) +} + // Run executes arbitrary Gno code in the context of the caller's realm. func (vm *VMKeeper) Run(ctx sdk.Context, msg MsgRun) (res string, err error) { caller := msg.Caller @@ -583,37 +588,36 @@ func (vm *VMKeeper) Run(ctx sdk.Context, msg MsgRun) (res string, err error) { Params: NewSDKParams(vm, ctx), EventLogger: ctx.EventLogger(), } - // Parse and run the files, construct *PV. + buf := new(bytes.Buffer) output := io.Writer(buf) - if vm.Output != nil { - output = io.MultiWriter(buf, vm.Output) - } - m := gno.NewMachineWithOptions( - gno.MachineOptions{ - PkgPath: "", - Output: output, - Store: gnostore, - Alloc: gnostore.GetAllocator(), - Context: msgCtx, - GasMeter: ctx.GasMeter(), - }) - // XXX MsgRun does not have pkgPath. How do we find it on chain? - defer m.Release() - defer func() { - if r := recover(); r != nil { - switch r.(type) { - case store.OutOfGasException: // panic in consumeGas() - panic(r) - default: - err = errors.Wrapf(fmt.Errorf("%v", r), "VM run main addpkg panic: %v\n%s\n", - r, m.String()) - return - } + + // Run as self-executing closure to have own function for doRecover / m.Release defers. + pv := func() *gno.PackageValue { + // Parse and run the files, construct *PV. + if vm.Output != nil { + output = io.MultiWriter(buf, vm.Output) } - }() + m := gno.NewMachineWithOptions( + gno.MachineOptions{ + PkgPath: "", + Output: output, + Store: gnostore, + Alloc: gnostore.GetAllocator(), + Context: msgCtx, + GasMeter: ctx.GasMeter(), + }) + // XXX MsgRun does not have pkgPath. How do we find it on chain? + defer m.Release() + defer doRecover(m, &err) - _, pv := m.RunMemPackage(memPkg, false) + _, pv := m.RunMemPackage(memPkg, false) + return pv + }() + if err != nil { + // handle any errors happened within pv generation. + return + } m2 := gno.NewMachineWithOptions( gno.MachineOptions{ @@ -626,18 +630,7 @@ func (vm *VMKeeper) Run(ctx sdk.Context, msg MsgRun) (res string, err error) { }) defer m2.Release() m2.SetActivePackage(pv) - defer func() { - if r := recover(); r != nil { - switch r.(type) { - case store.OutOfGasException: // panic in consumeGas() - panic(r) - default: - err = errors.Wrapf(fmt.Errorf("%v", r), "VM run main call panic: %v\n%s\n", - r, m2.String()) - return - } - } - }() + defer doRecover(m2, &err) m2.RunMain() res = buf.String() @@ -758,18 +751,7 @@ func (vm *VMKeeper) QueryEval(ctx sdk.Context, pkgPath string, expr string) (res GasMeter: ctx.GasMeter(), }) defer m.Release() - defer func() { - if r := recover(); r != nil { - switch r.(type) { - case store.OutOfGasException: // panic in consumeGas() - panic(r) - default: - err = errors.Wrapf(fmt.Errorf("%v", r), "VM query eval panic: %v\n%s\n", - r, m.String()) - return - } - } - }() + defer doRecover(m, &err) rtvs := m.Eval(xx) res = "" for i, rtv := range rtvs { @@ -826,18 +808,7 @@ func (vm *VMKeeper) QueryEvalString(ctx sdk.Context, pkgPath string, expr string GasMeter: ctx.GasMeter(), }) defer m.Release() - defer func() { - if r := recover(); r != nil { - switch r.(type) { - case store.OutOfGasException: // panic in consumeGas() - panic(r) - default: - err = errors.Wrapf(fmt.Errorf("%v", r), "VM query eval string panic: %v\n%s\n", - r, m.String()) - return - } - } - }() + defer doRecover(m, &err) rtvs := m.Eval(xx) if len(rtvs) != 1 { return "", errors.New("expected 1 string result, got %d", len(rtvs)) diff --git a/gnovm/pkg/gnolang/preprocess.go b/gnovm/pkg/gnolang/preprocess.go index d47067854ca..79695d8888a 100644 --- a/gnovm/pkg/gnolang/preprocess.go +++ b/gnovm/pkg/gnolang/preprocess.go @@ -10,7 +10,6 @@ import ( "sync/atomic" "github.com/gnolang/gno/tm2/pkg/errors" - tmstore "github.com/gnolang/gno/tm2/pkg/store" ) const ( @@ -366,12 +365,6 @@ func initStaticBlocks(store Store, ctx BlockNode, bn BlockNode) { func doRecover(stack []BlockNode, n Node) { if r := recover(); r != nil { - // Catch the out-of-gas exception and throw it - if exp, ok := r.(tmstore.OutOfGasException); ok { - exp.Descriptor = fmt.Sprintf("in preprocess: %v", r) - panic(exp) - } - if _, ok := r.(*PreprocessError); ok { // re-panic directly if this is a PreprocessError already. panic(r) @@ -388,10 +381,8 @@ func doRecover(stack []BlockNode, n Node) { var err error rerr, ok := r.(error) if ok { - // NOTE: gotuna/gorilla expects error exceptions. err = errors.Wrap(rerr, loc.String()) } else { - // NOTE: gotuna/gorilla expects error exceptions. err = fmt.Errorf("%s: %v", loc.String(), r) } diff --git a/tm2/pkg/sdk/auth/ante.go b/tm2/pkg/sdk/auth/ante.go index 4495a1729ad..997478fe4b5 100644 --- a/tm2/pkg/sdk/auth/ante.go +++ b/tm2/pkg/sdk/auth/ante.go @@ -76,7 +76,7 @@ func NewAnteHandler(ak AccountKeeper, bank BankKeeperI, sigGasConsumer Signature defer func() { if r := recover(); r != nil { switch ex := r.(type) { - case store.OutOfGasException: + case store.OutOfGasError: log := fmt.Sprintf( "out of gas in location: %v; gasWanted: %d, gasUsed: %d", ex.Descriptor, tx.Fee.GasWanted, newCtx.GasMeter().GasConsumed(), diff --git a/tm2/pkg/sdk/baseapp.go b/tm2/pkg/sdk/baseapp.go index ea729abd6ae..746dd618800 100644 --- a/tm2/pkg/sdk/baseapp.go +++ b/tm2/pkg/sdk/baseapp.go @@ -759,7 +759,7 @@ func (app *BaseApp) runTx(ctx Context, tx Tx) (result Result) { defer func() { if r := recover(); r != nil { switch ex := r.(type) { - case store.OutOfGasException: + case store.OutOfGasError: log := fmt.Sprintf( "out of gas, gasWanted: %d, gasUsed: %d location: %v", gasWanted, diff --git a/tm2/pkg/store/exports.go b/tm2/pkg/store/exports.go index a1b4aba3655..d6e3b32bc25 100644 --- a/tm2/pkg/store/exports.go +++ b/tm2/pkg/store/exports.go @@ -22,8 +22,8 @@ type ( Gas = types.Gas GasMeter = types.GasMeter GasConfig = types.GasConfig - OutOfGasException = types.OutOfGasException - GasOverflowException = types.GasOverflowException + OutOfGasError = types.OutOfGasError + GasOverflowError = types.GasOverflowError ) var ( diff --git a/tm2/pkg/store/types/gas.go b/tm2/pkg/store/types/gas.go index 9d1f3d70c28..a86cff17d1a 100644 --- a/tm2/pkg/store/types/gas.go +++ b/tm2/pkg/store/types/gas.go @@ -21,17 +21,25 @@ const ( // Gas measured by the SDK type Gas = int64 -// OutOfGasException defines an error thrown when an action results in out of gas. -type OutOfGasException struct { +// OutOfGasError defines an error thrown when an action results in out of gas. +type OutOfGasError struct { Descriptor string } -// GasOverflowException defines an error thrown when an action results gas consumption +func (oog OutOfGasError) Error() string { + return "out of gas in location: " + oog.Descriptor +} + +// GasOverflowError defines an error thrown when an action results gas consumption // unsigned integer overflow. -type GasOverflowException struct { +type GasOverflowError struct { Descriptor string } +func (oog GasOverflowError) Error() string { + return "gas overflow in location: " + oog.Descriptor +} + // GasMeter interface to track gas consumption type GasMeter interface { GasConsumed() Gas @@ -88,13 +96,13 @@ func (g *basicGasMeter) ConsumeGas(amount Gas, descriptor string) { } consumed, ok := overflow.Add64(g.consumed, amount) if !ok { - panic(GasOverflowException{descriptor}) + panic(GasOverflowError{descriptor}) } // consume gas even if out of gas. // corollary, call (Did)ConsumeGas after consumption. g.consumed = consumed if consumed > g.limit { - panic(OutOfGasException{descriptor}) + panic(OutOfGasError{descriptor}) } } @@ -139,7 +147,7 @@ func (g *infiniteGasMeter) Remaining() Gas { func (g *infiniteGasMeter) ConsumeGas(amount Gas, descriptor string) { consumed, ok := overflow.Add64(g.consumed, amount) if !ok { - panic(GasOverflowException{descriptor}) + panic(GasOverflowError{descriptor}) } g.consumed = consumed }