feat: small improvements for gnolang#1702 (gnolang#2276)

Also includes improved coloredbytes and debug printing, and more
comments.
At first I thought gnolang#1702's passing of the store was not ideal, but upon
much confusion with cache invalidation,
it became clear that passing in a store to getPackage() makes sense.

This means that any store operations that occur through the loading of
dependencies will incur gas charges for the transaction, e.g. for
AddPkg() with dependencies like "time" or "strconv". Rather than clear
cache-misses from the cacheStore, which is confusing, we would be better
off passing in a mutated store go getPackage (if we need to).

Or, just load the standard packages upon genesis.

Also added improvements to ColoredBytes; this is now faster since not
every character needs to be escaped, but rather escaping happens in
chunks. As part of this refactor, the key & values are also clipped. I
suppose we could maybe 1. improve ColoredBytesN() to clip exactly to N,
but implementing this is non-trivial, and also 2. make the key/value
limits perhps depend on a configuration or environment variable.

Poll... would you be sad if the Print() output for databases clipped the
values? I think it makes it much better for dev experience; and if you
need the full value you can tinker with the source where appropriate.
The downside is, we might lose information from logs. But I'm not sure
we even use the Print() feature for any logs as of now.

.gitignore

@@ -30,3 +30,7 @@ pbbindings.go
 # Test coverage leftovers
 cover.out
 coverage.out
+
+*.swp
+*.swo
+*.bak

gno.land/pkg/gnoland/app.go

@@ -120,7 +120,9 @@ func NewAppWithOptions(cfg *AppOptions) (abci.Application, error) {
 	}
 
 	// Initialize the VMKeeper.
-	vmKpr.Initialize(baseApp.GetCacheMultiStore())
+	ms := baseApp.GetCacheMultiStore()
+	vmKpr.Initialize(ms)
+	ms.MultiWrite() // XXX why was't this needed?
 
 	return baseApp, nil
 }

gno.land/pkg/sdk/vm/builtins.go

@@ -5,42 +5,41 @@ import (
 	"path/filepath"
 
 	gno "github.com/gnolang/gno/gnovm/pkg/gnolang"
-	"github.com/gnolang/gno/gnovm/stdlibs"
 	"github.com/gnolang/gno/tm2/pkg/crypto"
 	osm "github.com/gnolang/gno/tm2/pkg/os"
 	"github.com/gnolang/gno/tm2/pkg/sdk"
 	"github.com/gnolang/gno/tm2/pkg/std"
 )
 
-func (vm *VMKeeper) initBuiltinPackagesAndTypes(store gno.Store) {
-	// NOTE: native functions/methods added here must be quick operations,
-	// 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) {
-		// otherwise, built-in package value.
-		// first, load from filepath.
-		stdlibPath := filepath.Join(vm.stdlibsDir, pkgPath)
-		if !osm.DirExists(stdlibPath) {
-			// does not exist.
-			return nil, nil
-		}
-		memPkg := gno.ReadMemPackage(stdlibPath, pkgPath)
-		if memPkg.IsEmpty() {
-			// no gno files are present, skip this package
-			return nil, nil
-		}
-
-		m2 := gno.NewMachineWithOptions(gno.MachineOptions{
-			PkgPath: "gno.land/r/stdlibs/" + pkgPath,
-			// PkgPath: pkgPath,
-			Output: os.Stdout,
-			Store:  newStore,
-		})
-		defer m2.Release()
-		return m2.RunMemPackage(memPkg, true)
+// NOTE: this function may add loaded dependencies to store if they don't
+// already exist, including mem packages. If this happens during a transaction
+// with the tx context store, the transaction caller will pay for operations.
+// NOTE: native functions/methods added here must be quick operations, or
+// account for gas before operation.
+// TODO: define criteria for inclusion, and solve gas calculations(???).
+func (vm *VMKeeper) getPackage(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)
+	if !osm.DirExists(stdlibPath) {
+		// does not exist.
+		return nil, nil
+	}
+	memPkg := gno.ReadMemPackage(stdlibPath, pkgPath)
+	if memPkg.IsEmpty() {
+		// no gno files are present, skip this package
+		return nil, nil
 	}
-	store.SetPackageGetter(getPackage)
-	store.SetNativeStore(stdlibs.NativeStore)
+
+	m2 := gno.NewMachineWithOptions(gno.MachineOptions{
+		PkgPath: "gno.land/r/stdlibs/" + pkgPath,
+		// PkgPath: pkgPath,
+		Output: os.Stdout,
+		Store:  store,
+	})
+	defer m2.Release()
+	pn, pv = m2.RunMemPackage(memPkg, true)
+	return
 }
 
 // ----------------------------------------

gno.land/pkg/sdk/vm/keeper.go

@@ -81,7 +81,8 @@ func (vm *VMKeeper) Initialize(ms store.MultiStore) {
 	baseSDKStore := ms.GetStore(vm.baseKey)
 	iavlSDKStore := ms.GetStore(vm.iavlKey)
 	vm.gnoStore = gno.NewStore(alloc, baseSDKStore, iavlSDKStore)
-	vm.initBuiltinPackagesAndTypes(vm.gnoStore)
+	vm.gnoStore.SetPackageGetter(vm.getPackage)
+	vm.gnoStore.SetNativeStore(stdlibs.NativeStore)
 	if vm.gnoStore.NumMemPackages() > 0 {
 		// for now, all mem packages must be re-run after reboot.
 		// TODO remove this, and generally solve for in-mem garbage collection
@@ -157,7 +158,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)
 	}

gnovm/pkg/gnolang/go2gno.go

@@ -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.

gnovm/pkg/gnolang/store.go

@@ -9,16 +9,21 @@ import (
 	"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)
@@ -68,6 +73,8 @@ type Store interface {
 	LogSwitchRealm(rlmpath string) // to mark change of realm boundaries
 	ClearCache()
 	Print()
+	Write()
+	Flush()
 }
 
 // Used to keep track of in-mem objects during tx.
@@ -86,7 +93,7 @@ type defaultStore struct {
 
 	// transient
 	opslog  []StoreOp // for debugging and testing.
-	current []string
+	current []string  // for detecting import cycles.
 }
 
 func NewStore(alloc *Allocator, baseStore, iavlStore store.Store) *defaultStore {
@@ -115,6 +122,7 @@ func (ds *defaultStore) SetPackageGetter(pg PackageGetter) {
 
 // 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))
@@ -672,8 +680,16 @@ func (ds *defaultStore) GetNative(pkgPath string, name Name) func(m *Machine) {
 	return nil
 }
 
+// Writes one level of cache to store.
+func (ds *defaultStore) Write() {
+	ds.baseStore.(types.Writer).Write()
+	ds.iavlStore.(types.Writer).Write()
+}
+
+// Flush cached writes to disk.
 func (ds *defaultStore) Flush() {
-	// XXX
+	ds.baseStore.(types.Flusher).Flush()
+	ds.iavlStore.(types.Flusher).Flush()
 }
 
 // ----------------------------------------
@@ -755,22 +771,25 @@ func (ds *defaultStore) ClearCache() {
 
 // 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..."))
 	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))
 	}
-	fmt.Println("//----------------------------------------")
-	fmt.Println("defaultStore:cacheNodes...")
+	fmt.Println(colors.Yellow("//----------------------------------------"))
+	fmt.Println(colors.Green("defaultStore:cacheNodes..."))
 	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))
 	}
+	fmt.Println(colors.Red("//----------------------------------------"))
 }
 
 // ----------------------------------------

gnovm/tests/imports.go

@@ -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"))
 		}
@@ -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)
@@ -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
 			}
@@ -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
 			}
@@ -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)
 	return
 }
 

tm2/pkg/colors/colors.go

@@ -96,14 +96,80 @@ func Gray(args ...interface{}) string {
 	return treatAll(ANSIFgGray, args...)
 }
 
-func ColoredBytes(data []byte, textColor, bytesColor func(...interface{}) string) string {
+// result may be 4 ASNSII chars longer than they should be to denote the
+// elipses (...), and one for a trailing hex nibble in case the last byte is
+// non-ascii.
+// NOTE: it is annoying to try make this perfect and always fit within n, so we
+// don't do this yet, but left as an exercise. :)
+func ColoredBytesN(data []byte, n int, textColor, bytesColor func(...interface{}) string) string {
+	_n := 0
 	s := ""
-	for _, b := range data {
+	buf := ""         // buffer
+	bufIsText := true // is buf text or hex
+	for i, b := range data {
+	RESTART:
 		if 0x21 <= b && b < 0x7F {
-			s += textColor(string(b))
+			if !bufIsText {
+				s += bytesColor(buf)
+				buf = ""
+				bufIsText = true
+				goto RESTART
+			}
+			buf += string(b)
+			_n += 1
+			if n != 0 && _n >= n {
+				if i == len(data)-1 {
+					// done
+					s += textColor(buf)
+					buf = ""
+				} else {
+					s += textColor(buf) + "..."
+					buf = ""
+				}
+				break
+			}
+		} else {
+			if bufIsText {
+				s += textColor(buf)
+				buf = ""
+				bufIsText = false
+				goto RESTART
+			}
+			buf += fmt.Sprintf("%02X", b)
+			_n += 2
+			if n != 0 && _n >= n {
+				if i == len(data)-1 {
+					// done
+					s += bytesColor(buf)
+					buf = ""
+				} else {
+					s += bytesColor(buf) + "..."
+					buf = ""
+				}
+				break
+			}
+		}
+	}
+	if buf != "" {
+		if bufIsText {
+			s += textColor(buf)
+			buf = ""
 		} else {
-			s += bytesColor(fmt.Sprintf("%02X", b))
+			s += bytesColor(buf)
+			buf = ""
 		}
 	}
 	return s
 }
+
+func DefaultColoredBytesN(data []byte, n int) string {
+	return ColoredBytesN(data, n, Blue, Green)
+}
+
+func ColoredBytes(data []byte, textColor, bytesColor func(...interface{}) string) string {
+	return ColoredBytesN(data, 0, textColor, bytesColor)
+}
+
+func DefaultColoredBytes(data []byte) string {
+	return ColoredBytes(data, Blue, Green)
+}

tm2/pkg/db/goleveldb/go_level_db.go

@@ -6,6 +6,7 @@ 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/syndtr/goleveldb/leveldb"
@@ -115,9 +116,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.DefaultColoredBytesN(itr.Key(), 50)
+		value := colors.DefaultColoredBytesN(itr.Value(), 100)
+		fmt.Printf("%v: %v\n", key, value)
 	}
 }