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

Adding a package with insufficient gas causes causes 'should not happen' the next time #2453

Open
grepsuzette opened this issue Jun 27, 2024 · 3 comments
Assignees

Comments

@grepsuzette
Copy link
Contributor

grepsuzette commented Jun 27, 2024

Calling addpkg with insufficient gas seems to corrupt the machine.
Issue became apparent to me while testing #2368, but it also happens without.

Steps to reproduce

On a clean chain:

  • add a package with very limited gas (so it fails),
  • Add another package, again with insufficient gas.

For example:

  • gnodev -minimal
  • gnokey maketx addpkg -pkgpath p/demo/bf -pkgdir examples/p/demo/bf -deposit 10ugnot -gas-fee 10ugnot -gas-wanted 9000000 -broadcast=true -chainid dev -remote localhost:26657 myaccount
  • gnokey maketx addpkg -pkgpath p/demo/cford32 -pkgdir examples/p/demo/cford32 -deposit 10ugnot -gas-fee 10ugnot -gas-wanted 9000000 -broadcast=true -chainid dev -remote localhost:26657 myaccount

This consistently produces should not happen the second time.

Note

It happens on master.
But it is more apparent after applying #2368.

That is because in that case, the first time you add a package, it then shows a clear Out of gas error. Whereas without #2368, you get some random error for the first failing addpkg, meaning you're not as likely to go further.

Expected behaviour

The second addpkg with insufficient gas (same or different package), should fail in the same way as the first. When the tx fails we don't expect state to change in the VM itself.

Actual behaviour

... instead it currently panic with should not happen the second time, line 428:

func (ds *defaultStore) SetCacheType(tt Type) {
tid := tt.TypeID()
if tt2, exists := ds.cacheTypes[tid]; exists {
if tt != tt2 {
// NOTE: not sure why this would happen.
panic("should not happen")
} else {
// already set.
}

VS Code. tt != tt2 (click to show debugger screenshots)

![_](https://github.com/gnolang/gno/assets/350354/0344f91e-f374-48d9-947f-335efe829c2c) See tt and tt2 compared here:

tt4

The caller of this is func (ds *defaultStore) GetPackage(pkgPath string, isImport bool).
More exactly here is where it gets called (L209):

// cache all types. usually preprocess() sets types,
// but packages gotten from the pkgGetter may skip this step,
// so fill in store.CacheTypes here.
for _, tv := range pv.GetBlock(nil).Values {
if tv.T.Kind() == TypeKind {
t := tv.GetType()
ds.SetCacheType(t)
}
}

In the case I was debugging, it was about import "errors"...

Logs

Call stack
```
goroutine 144171 [running]:
runtime/debug.Stack()
        /usr/lib/go/src/runtime/debug/stack.go:24 +0x5e
github.com/gnolang/gno/tm2/pkg/sdk.(*BaseApp).runTx.func1()
        /me/gno/tm2/pkg/sdk/baseapp.go:749 +0x205
panic({0xfda0c0?, 0xc00ed1b300?})
        /usr/lib/go/src/runtime/panic.go:770 +0x132
go/types.(*Checker).handleBailout(0xc000f66200, 0xc00f123d40)
        /usr/lib/go/src/go/types/check.go:367 +0x88
panic({0xfda0c0?, 0xc00ed1b300?})
        /usr/lib/go/src/runtime/panic.go:770 +0x132
github.com/gnolang/gno/gnovm/pkg/gnolang.predefineNow.func1()
        /me/gno/gnovm/pkg/gnolang/preprocess.go:3068 +0x2b3
panic({0xf1e340?, 0xc00f3070e0?})
        /usr/lib/go/src/runtime/panic.go:770 +0x132
github.com/gnolang/gno/gnovm/pkg/gnolang.predefineNow.func1()
        /me/gno/gnovm/pkg/gnolang/preprocess.go:3075 +0x245
panic({0xed45a0?, 0x13bb2a0?})
        /usr/lib/go/src/runtime/panic.go:770 +0x132
github.com/gnolang/gno/gnovm/pkg/gnolang.(*defaultStore).SetCacheType(0xc00e42b720, {0x13d1690, 0xc00f2f3860})
        /me/gno/gnovm/pkg/gnolang/store.go:420 +0xe6
github.com/gnolang/gno/gnovm/pkg/gnolang.(*defaultStore).GetPackage(0xc00e42b720, {0xc00e526031, 0x6}, 0x0?)
        /me/gno/gnovm/pkg/gnolang/store.go:201 +0x777
github.com/gnolang/gno/gnovm/pkg/gnolang.tryPredefine({0x13e48c0, 0xc00e42b720}, {0x13e1720, 0xc00f2fe908}, {0x13d74c8, 0xc0
0f2ba080})
        /me/gno/gnovm/pkg/gnolang/preprocess.go:3231 +0x163
github.com/gnolang/gno/gnovm/pkg/gnolang.predefineNow2({0x13e48c0, 0xc00e42b720}, {0x13e1720, 0xc00f2fe908}, {0x13d74c8, 0xc
00f2ba080}, 0xc00f121ce0)
        /me/gno/gnovm/pkg/gnolang/preprocess.go:3095 +0x130
github.com/gnolang/gno/gnovm/pkg/gnolang.predefineNow({0x13e48c0, 0xc00e42b720}, {0x13e1720, 0xc00f2fe908}, {0x13d74c8, 0xc0
0f2ba080})
        /me/gno/gnovm/pkg/gnolang/preprocess.go:3080 +0x153
github.com/gnolang/gno/gnovm/pkg/gnolang.PredefineFileSet({0x13e48c0, 0xc00e42b720}, 0xc00f2fef08, 0xc00f298168)
        /me/gno/gnovm/pkg/gnolang/preprocess.go:44 +0x6f5
github.com/gnolang/gno/gnovm/pkg/gnolang.(*Machine).runFiles(0xc00653e008, {0xc00f2af140, 0x3, 0x4})
        /me/gno/gnovm/pkg/gnolang/machine.go:530 +0x275
github.com/gnolang/gno/gnovm/pkg/gnolang.(*Machine).RunFiles(...)
        /me/gno/gnovm/pkg/gnolang/machine.go:496
github.com/gnolang/gno/gnovm/pkg/gnolang.(*Machine).runMemPackage(0xc00653e008, 0xc00f1b7400, 0x1, 0x0)
        /me/gno/gnovm/pkg/gnolang/machine.go:287 +0x22d
github.com/gnolang/gno/gnovm/pkg/gnolang.(*Machine).RunMemPackage(...)
        /me/gno/gnovm/pkg/gnolang/machine.go:255
github.com/gnolang/gno/gno.land/pkg/sdk/vm.(*VMKeeper).initBuiltinPackagesAndTypes.func1({0xc00e6e85a1, 0x9}, {0x13e48c0, 0x
c00e42b720})
        /me/gno/gno.land/pkg/sdk/vm/builtins.go:40 +0x245
github.com/gnolang/gno/gnovm/pkg/gnolang.(*defaultStore).GetPackage(0xc00e42b720, {0xc00e6e85a1, 0x9}, 0x0?)
        /me/gno/gnovm/pkg/gnolang/store.go:170 +0x125
github.com/gnolang/gno/gnovm/pkg/gnolang.tryPredefine({0x13e48c0, 0xc00e42b720}, {0x13e1720, 0xc00ea8bb08}, {0x13d74c8, 0xc0
0e955180})
        /me/gno/gnovm/pkg/gnolang/preprocess.go:3231 +0x163
github.com/gnolang/gno/gnovm/pkg/gnolang.predefineNow2({0x13e48c0, 0xc00e42b720}, {0x13e1720, 0xc00ea8bb08}, {0x13d74c8, 0xc
00e955180}, 0xc00f1229e8)
        /me/gno/gnovm/pkg/gnolang/preprocess.go:3095 +0x130
github.com/gnolang/gno/gnovm/pkg/gnolang.predefineNow({0x13e48c0, 0xc00e42b720}, {0x13e1720, 0xc00ea8bb08}, {0x13d74c8, 0xc0
0e955180})
        /me/gno/gnovm/pkg/gnolang/preprocess.go:3080 +0x153
github.com/gnolang/gno/gnovm/pkg/gnolang.PredefineFileSet({0x13e48c0, 0xc00e42b720}, 0xc00f193808, 0xc00f167c98)
        /me/gno/gnovm/pkg/gnolang/preprocess.go:44 +0x6f5
github.com/gnolang/gno/gnovm/pkg/gnolang.(*Machine).runFiles(0xc008692008, {0xc0065cf400, 0x2d, 0x40})
        /me/gno/gnovm/pkg/gnolang/machine.go:530 +0x275
github.com/gnolang/gno/gnovm/pkg/gnolang.(*Machine).RunFiles(...)
        /me/gno/gnovm/pkg/gnolang/machine.go:496
github.com/gnolang/gno/gnovm/pkg/gnolang.(*Machine).runMemPackage(0xc008692008, 0xc00e6a4880, 0x1, 0x0)
        /me/gno/gnovm/pkg/gnolang/machine.go:287 +0x22d
github.com/gnolang/gno/gnovm/pkg/gnolang.(*Machine).RunMemPackage(...)
        /me/gno/gnovm/pkg/gnolang/machine.go:255
github.com/gnolang/gno/gno.land/pkg/sdk/vm.(*VMKeeper).initBuiltinPackagesAndTypes.func1({0xc00e612757, 0x4}, {0x13e48c0, 0x
c00e42b720})
        /me/gno/gno.land/pkg/sdk/vm/builtins.go:40 +0x245
github.com/gnolang/gno/gnovm/pkg/gnolang.(*defaultStore).GetPackage(0xc00e42b720, {0xc00e612757, 0x4}, 0x8?)
        /me/gno/gnovm/pkg/gnolang/store.go:170 +0x125
github.com/gnolang/gno/gnovm/pkg/gnolang.(*defaultStore).getMemPackage(0xc00e42b720, {0xc00e612757, 0x4}, 0x0)
        /me/gno/gnovm/pkg/gnolang/store.go:558 +0xbd
github.com/gnolang/gno/gnovm/pkg/gnolang.(*defaultStore).GetMemPackage(0xc00e77b228?, {0xc00e612757?, 0xc00e612757?})
        /me/gno/gnovm/pkg/gnolang/store.go:545 +0x1a
github.com/gnolang/gno/gnovm/pkg/gnolang.(*gnoImporter).ImportFrom(0xc00e48fc20, {0xc00e612757, 0x4}, {0x76f0ed309108?, 0x22
0040?}, 0x4?)
        /me/gno/gnovm/pkg/gnolang/go2gno.go:540 +0x64
go/types.(*Checker).importPackage(0xc000f66200, {0x13c3980, 0xc00e48fca0}, {0xc00e612757, 0x4}, {0x13b6bf0, 0x1})
        /usr/lib/go/src/go/types/resolver.go:158 +0x53a
go/types.(*Checker).collectObjects.func1({0x13c3b00?, 0xc00e6089c0})
        /usr/lib/go/src/go/types/resolver.go:267 +0x125
go/types.(*Checker).walkDecl(0xc000f66200, {0x13c9f20?, 0xc00e5c6340}, 0xc00f123c38)
        /usr/lib/go/src/go/types/decl.go:412 +0x2e7
go/types.(*Checker).walkDecls(...)
        /usr/lib/go/src/go/types/decl.go:399
go/types.(*Checker).collectObjects(0xc000f66200)
        /usr/lib/go/src/go/types/resolver.go:254 +0xe2f
go/types.(*Checker).checkFiles(0xc000f66200, {0xc00e608990, 0x3, 0x6})
        /usr/lib/go/src/go/types/check.go:405 +0x17c
go/types.(*Checker).Files(...)
        /usr/lib/go/src/go/types/check.go:372
go/types.(*Config).Check(0xc00e4a9560, {0xc00e6eb968?, 0x8?}, 0xc00e5c6300, {0xc00e608990, 0x3, 0x6}, 0x0)
        /usr/lib/go/src/go/types/api.go:458 +0x79
github.com/gnolang/gno/gnovm/pkg/gnolang.(*gnoImporter).parseCheckMemPackage(0xc00e48fc20, 0xc00e5c6200)
        /me/gno/gnovm/pkg/gnolang/go2gno.go:574 +0x366
github.com/gnolang/gno/gnovm/pkg/gnolang.TypeCheckMemPackage(0xc00e5c6200, {0x76f0a6484570, 0xc00e42b720})
        /me/gno/gnovm/pkg/gnolang/go2gno.go:505 +0x14c
github.com/gnolang/gno/gno.land/pkg/sdk/vm.(*VMKeeper).AddPackage(_, {{0x13cb988, 0xc00e5c3b30}, 0x1, {0x13cabe0, 0xc00e5977
60}, {0x13cbcd0, 0xc00e9431e0}, {0xc00e4aa03b, 0x3}, ...}, ...)
        /me/gno/gno.land/pkg/sdk/vm/keeper.go:166 +0x545
github.com/gnolang/gno/gno.land/pkg/sdk/vm.vmHandler.handleMsgAddPackage({_}, {{0x13cb988, 0xc00e5c3b30}, 0x1, {0x13cabe0, 0
xc00e597760}, {0x13cbcd0, 0xc00e9431e0}, {0xc00e4aa03b, 0x3}, ...}, ...)
        /me/gno/gno.land/pkg/sdk/vm/handler.go:44 +0xb8
github.com/gnolang/gno/gno.land/pkg/sdk/vm.vmHandler.Process({_}, {{0x13cb988, 0xc00e5c3b30}, 0x1, {0x13cabe0, 0xc00e597760}
, {0x13cbcd0, 0xc00e9431e0}, {0xc00e4aa03b, 0x3}, ...}, ...)
        /me/gno/gno.land/pkg/sdk/vm/handler.go:31 +0x358
github.com/gnolang/gno/tm2/pkg/sdk.(*BaseApp).runMsgs(_, {{0x13cb988, 0xc00e5c3b30}, 0x1, {0x13cabe0, 0xc00e597760}, {0x13cb
cd0, 0xc00e9431e0}, {0xc00e4aa03b, 0x3}, ...}, ...)
        /me/gno/tm2/pkg/sdk/baseapp.go:650 +0x2ed
github.com/gnolang/gno/tm2/pkg/sdk.(*BaseApp).runTx(0xc00042c600, 0x1, {0xc00ebd5500, 0x54a0, _}, {{0xc00e5969d0, 0x1, 0x1},
 {0x895440, {{0xc00e703bd9, ...}, ...}}, ...})
        /me/gno/tm2/pkg/sdk/baseapp.go:829 +0xae5
github.com/gnolang/gno/tm2/pkg/sdk.(*BaseApp).Simulate(...)
        /me/gno/tm2/pkg/sdk/helpers.go:17
github.com/gnolang/gno/tm2/pkg/sdk.handleQueryApp(0xc00042c600, {0xc00e48f400?, 0x2?, _}, {{}, {0xc00ebd5500, 0x54a0, 0x54a2
}, {0xc00e703af0, 0xd}, ...})
        /me/gno/tm2/pkg/sdk/baseapp.go:418 +0x25d
github.com/gnolang/gno/tm2/pkg/sdk.(*BaseApp).Query(0xc00042c600, {{}, {0xc00ebd5500, 0x54a0, 0x54a2}, {0xc00e703af0, 0xd}, 
```
Reproducibility

First we have determined that some packages require more gas than others to be added:

cheap: p/demo/rat, p/demo/gnode, p/stack
expensive: p/demo/bf, p/demo/merkle, p/demo/cford32

-- produced: 3 times
gnodev -minimal
addpkg p/demo/bf out of gas
addpkg p/demo/rat ok
addpkg p/demo/cford32 should not happen

-- produced: 3 times
gnodev -minimal
addpkg p/demo/bf out of gas
addpkg p/demo/cford32 should not happen
addpkg p/demo/rat ok

-- produced: 1 times
gnodev -minimal
addpkg p/demo/cford32 ok
addpkg p/demo/rat ok
addpkg p/demo/bf out of gas
addpkg p/demo/gnode ok
addpkg p/demo/stack ok
addpkg p/demo/merkle should not happen

-- produced: 1 times
gnodev -minimal
addpkg p/demo/merkle out of gas
addpkg p/demo/gnode ok
addpkg p/demo/cford32 should not happen

-- predicted, and produced: 1 times
gnodev -minimal
addpkg p/demo/merkle out of gas
addpkg p/demo/gnode ok
addpkg p/demo/rat ok
addpkg p/demo/stack ok
addpkg p/demo/cford32 should not happen

-- predicted, and produced: 1 times
gnodev -minimal
addpkg p/demo/merkle out of gas
addpkg p/demo/gnode ok
addpkg p/demo/rat ok
addpkg p/demo/stack ok
addpkg p/demo/bf should not happen

Proposed solution

Spent many hours on this already, mostly reading early commits about the store.

The most promising clue I've seen so far is this message #1702 (comment) by Morgan

@thehowl, you're the hope for humanity here :p
Do you understand what's happening?

@grepsuzette
Copy link
Contributor Author

grepsuzette commented Jul 16, 2024

Linking to #2319
Even though method described here seems not reproducible since #2504 (git bisect), it seems the same family of problem.

@Kouteki
Copy link
Contributor

Kouteki commented Nov 17, 2024

@grepsuzette is the issue still reproducible after merging #2319?

@Kouteki Kouteki moved this from In Progress to Backlog in 🧙‍♂️gno.land core team Nov 17, 2024
@grepsuzette
Copy link
Contributor Author

Will test it this week, @Kouteki.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
Development

No branches or pull requests

2 participants