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

feat: small improvements for #1702 #2276

Merged
merged 15 commits into from
Jun 24, 2024
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,7 @@ pbbindings.go
# Test coverage leftovers
cover.out
coverage.out

*.swp
*.swo
*.bak
4 changes: 3 additions & 1 deletion gno.land/pkg/gnoland/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,9 @@
}

// Initialize the VMKeeper.
vmKpr.Initialize(baseApp.GetCacheMultiStore())
ms := baseApp.GetCacheMultiStore()
vmKpr.Initialize(ms)
ms.MultiWrite() // XXX why was't this needed?

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#L123-L125

Added lines #L123 - L125 were not covered by tests
Copy link
Contributor Author

@jaekwon jaekwon Jun 4, 2024

Choose a reason for hiding this comment

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

I think it was always requried, but its absence (of MultiWrite) being masked by the fact that later transactions would commit the same side effect, e.g. populate the mem package for dependencies like "time" or "strconv". But i'm not sure. If that's true, then loadpkg in genesis would have no effect for our integration tests?

Can somebody look into it?


return baseApp, nil
}
Expand Down
7 changes: 4 additions & 3 deletions gno.land/pkg/sdk/vm/builtins.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ func (vm *VMKeeper) initBuiltinPackagesAndTypes(store gno.Store) {
// NOTE: native functions/methods added here must be quick operations,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's no real need to have this function at all... getPackage below might as well be a method on the store.

// or account for gas before operation.
// TODO: define criteria for inclusion, and solve gas calculations.
getPackage := func(pkgPath string, newStore gno.Store) (pn *gno.PackageNode, pv *gno.PackageValue) {
getPackage := func(pkgPath string, store gno.Store) (pn *gno.PackageNode, pv *gno.PackageValue) {
// otherwise, built-in package value.
// first, load from filepath.
stdlibPath := filepath.Join(vm.stdlibsDir, pkgPath)
Expand All @@ -34,10 +34,11 @@ func (vm *VMKeeper) initBuiltinPackagesAndTypes(store gno.Store) {
PkgPath: "gno.land/r/stdlibs/" + pkgPath,
// PkgPath: pkgPath,
Output: os.Stdout,
Store: newStore,
Store: store,
})
defer m2.Release()
return m2.RunMemPackage(memPkg, true)
pn, pv = m2.RunMemPackage(memPkg, true)
return
}
store.SetPackageGetter(getPackage)
store.SetNativeStore(stdlibs.NativeStore)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

diff artifact, not actually removed from the original code block.

Expand Down
1 change: 0 additions & 1 deletion gno.land/pkg/sdk/vm/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,6 @@ func (vm *VMKeeper) AddPackage(ctx sdk.Context, msg MsgAddPackage) (err error) {
if pv := gnostore.GetPackage(pkgPath, false); pv != nil {
return ErrInvalidPkgPath("package already exists: " + pkgPath)
}

if gno.ReGnoRunPath.MatchString(pkgPath) {
return ErrInvalidPkgPath("reserved package name: " + pkgPath)
}
Expand Down
3 changes: 2 additions & 1 deletion gnovm/pkg/gnolang/debugger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,8 @@ func TestRemoteDebug(t *testing.T) {
func TestRemoteError(t *testing.T) {
_, err := evalTest(":xxx", "", debugTarget)
t.Log("err:", err)
if !strings.Contains(err, "tcp/xxx: unknown port") {
if !strings.Contains(err, "tcp/xxx: unknown port") &&
!strings.Contains(err, "tcp/xxx: nodename nor servname provided, or not known") {
t.Error(err)
}
}
2 changes: 2 additions & 0 deletions gnovm/pkg/gnolang/go2gno.go
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,7 @@ func Go2Gno(fs *token.FileSet, gon ast.Node) (n Node) {

//----------------------------------------
// type checking (using go/types)
// XXX move to gotypecheck.go.

// MemPackageGetter implements the GetMemPackage() method. It is a subset of
// [Store], separated for ease of testing.
Expand Down Expand Up @@ -539,6 +540,7 @@ func (g *gnoImporter) ImportFrom(path, _ string, _ types.ImportMode) (*types.Pac
}
mpkg := g.getter.GetMemPackage(path)
if mpkg == nil {
// fmt.Println("gnoImporter.ImportFrom/was nil", path)
err := importNotFoundError(path)
g.cache[path] = gnoImporterResult{err: err}
return nil, err
Expand Down
50 changes: 35 additions & 15 deletions gnovm/pkg/gnolang/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,21 @@
"strings"

"github.com/gnolang/gno/tm2/pkg/amino"
"github.com/gnolang/gno/tm2/pkg/colors"
"github.com/gnolang/gno/tm2/pkg/std"
"github.com/gnolang/gno/tm2/pkg/store"
"github.com/gnolang/gno/tm2/pkg/store/types"
"github.com/gnolang/gno/tm2/pkg/store/utils"
stringz "github.com/gnolang/gno/tm2/pkg/strings"
)

// PackageGetter specifies how the store may retrieve packages which are not
// already in its cache. PackageGetter should return nil when the requested
// package does not exist. store should be used to run the machine, or otherwise
// call any methods which may call store.GetPackage; avoid using any "global"
// store as the one passed to the PackageGetter may be a fork of that (ie.
// the original is not meant to be written to).
// the original is not meant to be written to). Loading dependencies may
// cause writes to happen to the store, such as MemPackages to iavlstore.
type PackageGetter func(pkgPath string, store Store) (*PackageNode, *PackageValue)

// inject natives into a new or loaded package (value and node)
Expand Down Expand Up @@ -68,6 +73,8 @@
LogSwitchRealm(rlmpath string) // to mark change of realm boundaries
ClearCache()
Print()
Write()
Flush()
}

// Used to keep track of in-mem objects during tx.
Expand All @@ -86,7 +93,7 @@

// transient
opslog []StoreOp // for debugging and testing.
current []string
current []string // for detecting import cycles.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's still detecting import cycles.

}

func NewStore(alloc *Allocator, baseStore, iavlStore store.Store) *defaultStore {
Expand Down Expand Up @@ -115,6 +122,7 @@

// Gets package from cache, or loads it from baseStore, or gets it from package getter.
func (ds *defaultStore) GetPackage(pkgPath string, isImport bool) *PackageValue {
// helper to detect circular imports
if isImport {
if slices.Contains(ds.current, pkgPath) {
panic(fmt.Sprintf("import cycle detected: %q (through %v)", pkgPath, ds.current))
Expand Down Expand Up @@ -613,6 +621,7 @@
ds.alloc.Reset()
ds.cacheObjects = make(map[ObjectID]Object) // new cache.
ds.opslog = nil // new ops log.
//ds.current = nil

Check failure on line 624 in gnovm/pkg/gnolang/store.go

View workflow job for this annotation

GitHub Actions / Run Main / Go Linter / lint

File is not `gofumpt`-ed (gofumpt)
ds.SetCachePackage(Uverse())
}

Expand Down Expand Up @@ -672,8 +681,16 @@
return nil
}

// Writes one level of cache to store.
func (ds *defaultStore) Write() {
ds.baseStore.(types.Writer).Write()
ds.iavlStore.(types.Writer).Write()

Check warning on line 687 in gnovm/pkg/gnolang/store.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/store.go#L685-L687

Added lines #L685 - L687 were not covered by tests
}

// Flush cached writes to disk.
func (ds *defaultStore) Flush() {
// XXX
ds.baseStore.(types.Flusher).Flush()
ds.iavlStore.(types.Flusher).Flush()

Check warning on line 693 in gnovm/pkg/gnolang/store.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/store.go#L692-L693

Added lines #L692 - L693 were not covered by tests
}

// ----------------------------------------
Expand Down Expand Up @@ -755,22 +772,25 @@

// for debugging
func (ds *defaultStore) Print() {
fmt.Println("//----------------------------------------")
fmt.Println("defaultStore:baseStore...")
store.Print(ds.baseStore)
fmt.Println("//----------------------------------------")
fmt.Println("defaultStore:iavlStore...")
store.Print(ds.iavlStore)
fmt.Println("//----------------------------------------")
fmt.Println("defaultStore:cacheTypes...")
fmt.Println(colors.Yellow("//----------------------------------------"))
fmt.Println(colors.Green("defaultStore:baseStore..."))
utils.Print(ds.baseStore)
fmt.Println(colors.Yellow("//----------------------------------------"))
fmt.Println(colors.Green("defaultStore:iavlStore..."))
utils.Print(ds.iavlStore)
fmt.Println(colors.Yellow("//----------------------------------------"))
fmt.Println(colors.Green("defaultStore:cacheTypes..."))

Check warning on line 782 in gnovm/pkg/gnolang/store.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/store.go#L775-L782

Added lines #L775 - L782 were not covered by tests
for tid, typ := range ds.cacheTypes {
fmt.Printf("- %v: %v\n", tid, typ)
fmt.Printf("- %v: %v\n", tid,
stringz.TrimN(fmt.Sprintf("%v", typ), 50))

Check warning on line 785 in gnovm/pkg/gnolang/store.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/store.go#L784-L785

Added lines #L784 - L785 were not covered by tests
}
fmt.Println("//----------------------------------------")
fmt.Println("defaultStore:cacheNodes...")
fmt.Println(colors.Yellow("//----------------------------------------"))
fmt.Println(colors.Green("defaultStore:cacheNodes..."))

Check warning on line 788 in gnovm/pkg/gnolang/store.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/store.go#L787-L788

Added lines #L787 - L788 were not covered by tests
for loc, bn := range ds.cacheNodes {
fmt.Printf("- %v: %v\n", loc, bn)
fmt.Printf("- %v: %v\n", loc,
stringz.TrimN(fmt.Sprintf("%v", bn), 50))

Check warning on line 791 in gnovm/pkg/gnolang/store.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/store.go#L790-L791

Added lines #L790 - L791 were not covered by tests
}
fmt.Println(colors.Red("//----------------------------------------"))

Check warning on line 793 in gnovm/pkg/gnolang/store.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/store.go#L793

Added line #L793 was not covered by tests
}

// ----------------------------------------
Expand Down
24 changes: 12 additions & 12 deletions gnovm/tests/imports.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ const (
)

// NOTE: this isn't safe, should only be used for testing.
func TestStore(rootDir, filesPath string, stdin io.Reader, stdout, stderr io.Writer, mode importMode) (store gno.Store) {
getPackage := func(pkgPath string, newStore gno.Store) (pn *gno.PackageNode, pv *gno.PackageValue) {
func TestStore(rootDir, filesPath string, stdin io.Reader, stdout, stderr io.Writer, mode importMode) (resStore gno.Store) {
getPackage := func(pkgPath string, store gno.Store) (pn *gno.PackageNode, pv *gno.PackageValue) {
if pkgPath == "" {
panic(fmt.Sprintf("invalid zero package path in testStore().pkgGetter"))
}
Expand All @@ -83,7 +83,7 @@ func TestStore(rootDir, filesPath string, stdin io.Reader, stdout, stderr io.Wri
m2 := gno.NewMachineWithOptions(gno.MachineOptions{
PkgPath: "test",
Output: stdout,
Store: newStore,
Store: store,
Context: ctx,
})
// pkg := gno.NewPackageNode(gno.Name(memPkg.Name), memPkg.Path, nil)
Expand All @@ -96,7 +96,7 @@ func TestStore(rootDir, filesPath string, stdin io.Reader, stdout, stderr io.Wri
// if stdlibs package is preferred , try to load it first.
if mode == ImportModeStdlibsOnly ||
mode == ImportModeStdlibsPreferred {
pn, pv = loadStdlib(rootDir, pkgPath, newStore, stdout)
pn, pv = loadStdlib(rootDir, pkgPath, store, stdout)
if pn != nil {
return
}
Expand Down Expand Up @@ -375,7 +375,7 @@ func TestStore(rootDir, filesPath string, stdin io.Reader, stdout, stderr io.Wri

// if native package is preferred, try to load stdlibs/* as backup.
if mode == ImportModeNativePreferred {
pn, pv = loadStdlib(rootDir, pkgPath, newStore, stdout)
pn, pv = loadStdlib(rootDir, pkgPath, store, stdout)
if pn != nil {
return
}
Expand All @@ -394,23 +394,23 @@ func TestStore(rootDir, filesPath string, stdin io.Reader, stdout, stderr io.Wri
m2 := gno.NewMachineWithOptions(gno.MachineOptions{
PkgPath: "test",
Output: stdout,
Store: newStore,
Store: store,
Context: ctx,
})
pn, pv = m2.RunMemPackage(memPkg, true)
return
}
return nil, nil
}
// NOTE: store is also used in closure above.
db := memdb.NewMemDB()
baseStore := dbadapter.StoreConstructor(db, stypes.StoreOptions{})
iavlStore := iavl.StoreConstructor(db, stypes.StoreOptions{})
store = gno.NewStore(nil, baseStore, iavlStore)
store.SetPackageGetter(getPackage)
store.SetNativeStore(teststdlibs.NativeStore)
store.SetPackageInjector(testPackageInjector)
store.SetStrictGo2GnoMapping(false)
// make a new store
resStore = gno.NewStore(nil, baseStore, iavlStore)
resStore.SetPackageGetter(getPackage)
resStore.SetNativeStore(teststdlibs.NativeStore)
resStore.SetPackageInjector(testPackageInjector)
resStore.SetStrictGo2GnoMapping(false)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

they're both new stores, so name neither "newStore" :)

return
}

Expand Down
8 changes: 5 additions & 3 deletions tm2/pkg/db/goleveldb/go_level_db.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ import (
"fmt"
"path/filepath"

"github.com/gnolang/gno/tm2/pkg/colors"
"github.com/gnolang/gno/tm2/pkg/db"
"github.com/gnolang/gno/tm2/pkg/db/internal"
"github.com/gnolang/gno/tm2/pkg/strings"
"github.com/syndtr/goleveldb/leveldb"
"github.com/syndtr/goleveldb/leveldb/errors"
"github.com/syndtr/goleveldb/leveldb/iterator"
Expand Down Expand Up @@ -115,9 +117,9 @@ func (db *GoLevelDB) Print() {

itr := db.db.NewIterator(nil, nil)
for itr.Next() {
key := itr.Key()
value := itr.Value()
fmt.Printf("[%X]:\t[%X]\n", key, value)
key := colors.ColoredBytes([]byte(strings.TrimN(string(itr.Key()), 50)), colors.Blue, colors.Green)
value := colors.ColoredBytes([]byte(strings.TrimN(string(itr.Value()), 50)), colors.Blue, colors.Green)
fmt.Printf("%v:\t%v\n", key, value)
}
}

Expand Down
13 changes: 3 additions & 10 deletions tm2/pkg/db/memdb/mem_db.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"sort"
"sync"

"github.com/gnolang/gno/tm2/pkg/colors"
dbm "github.com/gnolang/gno/tm2/pkg/db"
"github.com/gnolang/gno/tm2/pkg/db/internal"
"github.com/gnolang/gno/tm2/pkg/strings"
Expand Down Expand Up @@ -128,16 +129,8 @@

for key, value := range db.db {
var keystr, valstr string
if strings.IsASCIIText(key) {
keystr = key
} else {
keystr = fmt.Sprintf("0x%X", []byte(key))
}
if strings.IsASCIIText(string(value)) {
valstr = string(value)
} else {
valstr = fmt.Sprintf("0x%X", value)
}
keystr = colors.ColoredBytes([]byte(strings.TrimN(string(key), 50)), colors.Green, colors.Blue)

Check failure on line 132 in tm2/pkg/db/memdb/mem_db.go

View workflow job for this annotation

GitHub Actions / Run Main / Go Linter / lint

unnecessary conversion (unconvert)
valstr = colors.ColoredBytes([]byte(strings.TrimN(string(value), 50)), colors.Green, colors.Blue)
fmt.Printf("%s:\t%s\n", keystr, valstr)
}
}
Expand Down
15 changes: 6 additions & 9 deletions tm2/pkg/db/prefix_db.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"bytes"
"fmt"
"sync"

"github.com/gnolang/gno/tm2/pkg/colors"
)

// IteratePrefix is a convenience function for iterating over a key domain
Expand Down Expand Up @@ -188,15 +190,10 @@ func (pdb *PrefixDB) Close() {

// Implements DB.
func (pdb *PrefixDB) Print() {
fmt.Printf("prefix: %X\n", pdb.prefix)

itr := pdb.Iterator(nil, nil)
defer itr.Close()
for ; itr.Valid(); itr.Next() {
key := itr.Key()
value := itr.Value()
fmt.Printf("[%X]:\t[%X]\n", key, value)
}
fmt.Println(colors.Blue("prefix ---------------------"))
fmt.Printf("prefix: %v\n", colors.ColoredBytes(pdb.prefix, colors.Green, colors.Blue))
pdb.db.Print()
fmt.Println(colors.Blue("prefix --------------------- end"))
}

// Implements DB.
Expand Down
1 change: 0 additions & 1 deletion tm2/pkg/sdk/auth/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,6 @@ func NewAnteHandler(ak AccountKeeper, bank BankKeeperI, sigGasConsumer Signature
if err != nil {
return newCtx, res, true
}

signerAccs[i], res = processSig(newCtx, sacc, stdSigs[i], signBytes, simulate, params, sigGasConsumer)
if !res.IsOK() {
return newCtx, res, true
Expand Down
7 changes: 7 additions & 0 deletions tm2/pkg/sdk/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,10 @@ func (app *BaseApp) GetCacheMultiStore() store.MultiStore {
return app.cms.MultiCacheWrap()
}

func (app *BaseApp) GetCMS() store.CommitMultiStore {
return app.cms
}

// Router returns the router of the BaseApp.
func (app *BaseApp) Router() Router {
if app.sealed {
Expand Down Expand Up @@ -615,6 +619,9 @@ func (app *BaseApp) getContextForTx(mode RunTxMode, txBytes []byte) (ctx Context
WithVoteInfos(app.voteInfos).
WithConsensusParams(app.consensusParams)

// NOTE: This is especially required to simulate transactions because
// otherwise baseapp writes the antehandler mods (sequence and balance)
// to the underlying store for deliver and checktx.
if mode == RunTxModeSimulate {
ctx, _ = ctx.CacheContext()
}
Expand Down
Loading
Loading