Skip to content

Commit

Permalink
fix(test3): RunMemPackage on p/leon/avl to restore types (#2217)
Browse files Browse the repository at this point in the history
```
		// XXX(morgan): test3 specific, there is a bug with the transactions and
		// the way test3 runs.
		// The theory is as follows:
		//     Take a look at defaultStore.SetType
		//     When p/leon/avl was called, a type with the same TypeID was found in the cache, but it was different than the ones passed, so the function returned
		//     But the type was never in the store
		//     The reason this can happen is that up until this recent commit
		//     8afb1a4
		//     The store caches were shared with all child stores, even the ones that are eventually rolled back (due to, like, a failing transaction)
		//     So, in the running node, in a first addpkg, the type was saved in the cache, but then there was a bug in the code so the transaction failed
		//     The type is now in cacheTypes, but not in the database
		//     Afterwards, it was re-uploaded. SetType is called, it detects an existing type in cacheType, but the types are different; so SetType returns without storing it in the database
```
  • Loading branch information
thehowl authored May 27, 2024
1 parent ca7f448 commit b515cd0
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 32 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
strategy:
fail-fast: false
matrix:
goversion: ["1.18.x", "1.19.x"]
goversion: ["1.21.x", "1.22.x"]
goarch: ["amd64"]
goos: ["linux"]
program: ["genproto", "gnofaucet", "gnokey", "gnoland", "gnotxport", "goscan", "gnodev", "gnoweb"]
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/db-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
strategy:
fail-fast: false
matrix:
goversion: ["1.18.x", "1.19.x"]
goversion: ["1.21.x", "1.22.x"]
tags:
- cleveldb
- memdb
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/examples.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
strategy:
fail-fast: false
matrix:
goversion: ["1.18.x", "1.19.x"]
goversion: ["1.21.x", "1.22.x"]
runs-on: ubuntu-latest
timeout-minutes: 30
steps:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/unit-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
strategy:
fail-fast: false
matrix:
goversion: ["1.18.x", "1.19.x"]
goversion: ["1.21.x", "1.22.x"]
args:
- test.go1
- test.go2
Expand Down
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# build
FROM golang:1.19 AS build
FROM golang:1.21 AS build
RUN mkdir -p /opt/gno/src /opt/build
WORKDIR /opt/build
ADD go.mod go.sum /opt/build/
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/gnolang/gno

go 1.18
go 1.21

require (
github.com/btcsuite/btcd v0.22.0-beta.0.20220111032746-97732e52810c
Expand Down
60 changes: 40 additions & 20 deletions pkgs/gnolang/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,28 +138,48 @@ func (m *Machine) SetActivePackage(pv *PackageValue) {
func (m *Machine) PreprocessAllFilesAndSaveBlockNodes() {
ch := m.Store.IterMemPackage()
for memPkg := range ch {
fset := ParseMemPackage(memPkg)
pn := NewPackageNode(Name(memPkg.Name), memPkg.Path, fset)
m.Store.SetBlockNode(pn)
PredefineFileSet(m.Store, pn, fset)
for _, fn := range fset.Files {
// Save Types to m.Store (while preprocessing).
fn = Preprocess(m.Store, pn, fn).(*FileNode)
// Save BlockNodes to m.Store.
SaveBlockNodes(m.Store, fn)
}
// Normally, the fileset would be added onto the
// package node only after runFiles(), but we cannot
// run files upon restart (only preprocess them).
// So, add them here instead.
// TODO: is this right?
if pn.FileSet == nil {
pn.FileSet = fset
} else {
// This happens for non-realm file tests.
// TODO ensure the files are the same.
// XXX(morgan): test3 specific, there is a bug with the transactions and
// the way test3 runs.
// The theory is as follows:
// Take a look at defaultStore.SetType
// When p/leon/avl was called, a type with the same TypeID was found in the cache, but it was different than the ones passed, so the function returned
// But the type was never in the store
// The reason this can happen is that up until this recent commit
// https://github.com/gnolang/gno/commit/8afb1a42e138014a3025ef4817aff67b6f4f7ded
// The store caches were shared with all child stores, even the ones that are eventually rolled back (due to, like, a failing transaction)
// So, in the running node, in a first addpkg, the type was saved in the cache, but then there was a bug in the code so the transaction failed
// The type is now in cacheTypes, but not in the database
// Afterwards, it was re-uploaded. SetType is called, it detects an existing type in cacheType, but the types are different; so SetType returns without storing it in the database
switch memPkg.Path {
case "gno.land/p/leon/avl":
m.RunMemPackage(memPkg, true)
continue
}
m.preprocessMemPackageSaveBlockNodes(memPkg)
}
}

func (m *Machine) preprocessMemPackageSaveBlockNodes(memPkg *std.MemPackage) {
fset := ParseMemPackage(memPkg)
pn := NewPackageNode(Name(memPkg.Name), memPkg.Path, fset)
m.Store.SetBlockNode(pn)
PredefineFileSet(m.Store, pn, fset)
for _, fn := range fset.Files {
// Save Types to m.Store (while preprocessing).
fn = Preprocess(m.Store, pn, fn).(*FileNode)
// Save BlockNodes to m.Store.
SaveBlockNodes(m.Store, fn)
}
// Normally, the fileset would be added onto the
// package node only after runFiles(), but we cannot
// run files upon restart (only preprocess them).
// So, add them here instead.
// TODO: is this right?
if pn.FileSet == nil {
pn.FileSet = fset
} else {
// This happens for non-realm file tests.
// TODO ensure the files are the same.
}
}

Expand Down
19 changes: 13 additions & 6 deletions pkgs/gnolang/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package gnolang

import (
"fmt"
"maps"
"reflect"
"strconv"
"strings"
Expand Down Expand Up @@ -370,7 +371,7 @@ func (ds *defaultStore) DelObject(oo Object) {
func (ds *defaultStore) GetType(tid TypeID) Type {
tt := ds.GetTypeSafe(tid)
if tt == nil {
ds.Print()
// ds.Print()
panic(fmt.Sprintf("unexpected type with id %s", tid.String()))
}
return tt
Expand Down Expand Up @@ -594,12 +595,18 @@ func (ds *defaultStore) ClearObjectCache() {
// This function is used to handle queries and checktx transactions.
func (ds *defaultStore) Fork() Store {
ds2 := &defaultStore{
alloc: ds.alloc.Fork().Reset(),
pkgGetter: ds.pkgGetter,
cacheObjects: make(map[ObjectID]Object), // new cache.
cacheTypes: ds.cacheTypes,
alloc: ds.alloc.Fork().Reset(),
pkgGetter: ds.pkgGetter,
// Re-initialize caches. Some are cloned for speed.
cacheObjects: make(map[ObjectID]Object),
cacheTypes: maps.Clone(ds.cacheTypes),
// XXX: This is bad to say the least (ds.cacheNodes is shared with a
// child Store); however, cacheNodes is _not_ a cache, but a proper
// data store instead. SetBlockNode does not write anything to
// the underlying baseStore, and cloning this map makes everything run
// 4x slower, so here we are, copying the reference.
cacheNodes: ds.cacheNodes,
cacheNativeTypes: ds.cacheNativeTypes,
cacheNativeTypes: maps.Clone(ds.cacheNativeTypes),
baseStore: ds.baseStore,
iavlStore: ds.iavlStore,
pkgInjector: ds.pkgInjector,
Expand Down

0 comments on commit b515cd0

Please sign in to comment.