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

map state is not persisted correctly, resulting in nil pointer dereference #817

Closed
r3v4s opened this issue May 10, 2023 · 10 comments · Fixed by #932
Closed

map state is not persisted correctly, resulting in nil pointer dereference #817

r3v4s opened this issue May 10, 2023 · 10 comments · Fixed by #932
Labels
🐞 bug Something isn't working 📦 🤖 gnovm Issues or PRs gnovm related

Comments

@r3v4s
Copy link
Contributor

r3v4s commented May 10, 2023

runtime error: invalid memory address or nil pointer dereference

Description

map disappears after first call(not sure this is exact explanation)

Your environment

  • OS and version: macSO Ventura 13.3
  • version of gno: master branch
  • branch that causes this issue (with the commit hash): master branch

Steps to reproduce

Deploy below package

package maptest

var (
	mymap map[int64]int64 = make(map[int64]int64)
)


func UP(key, value int64) {
	println("=== CURRENT MAP === ")
	println(mymap)

	v := mymap[key]

	if v == 0 {
		println("insert key:", key)
		println("insert value:", value)
		mymap[key] = value
	} else {
		println("update key:", key)
		println("update value:", value)
		mymap[key] = value
	}

	println("=== AFTER UPSERT === ")
	println(mymap)
	println()
}

run gno test works fine just like below

# test.gno
package maptest

import (
	"testing"
)

func TestUP(t *testing.T) {
	// gno test ok
	// gnokey maketx ok
	UP(int64(1), int64(100))

	// gno test ok
	// gnokey maketx fail
	UP(int64(2), int64(200))
}

which prints

=== RUN   TestUP
=== CURRENT MAP ===
map{}
insert key: 1
insert value: 100
=== AFTER UPSERT ===
map{(1 int64):(100 int64)}

=== CURRENT MAP ===
map{(1 int64):(100 int64)}
insert key: 2
insert value: 200
=== AFTER UPSERT ===
map{(1 int64):(100 int64),(2 int64):(200 int64)}

--- PASS: TestUP (0.00s)
ok      ./. 	0.47s

after addpkg, first maketx call works fine

but second maketx call fail

### THIS IS FROM TEST3
{
  "jsonrpc": "2.0",
  "id": "",
  "result": {
    "height": "256492",
    "results": {
      "deliver_tx": [
        {
          "ResponseBase": {
            "Error": {
              "@type": "/abci.StringError",
              "value": "runtime error: invalid memory address or nil pointer dereference"
            },
            "Data": null,
            "Events": null,
            "Log": "msg:0,success:false,log:--= Error =--\nData: \u0026errors.errorString{s:\"runtime error: invalid memory address or nil pointer dereference\"}\nMsg Traces:\n    0  /opt/build/pkgs/sdk/vm/keeper.go:239 - VM call panic: runtime error: invalid memory address or nil pointer dereference\nMachine:\n    CheckTypes: false\n\tOp: [OpHalt OpExec OpBody OpPopResults OpPrecall]\n\tValues: (len: 2)\n          #1 (println func(xs ...interface{})())\n          #0 (UP func(key int64,value int64)())\n\tExprs:\n          #0 (const (println func(xs ...interface{})()))(mymap\u003cVPBlock(3,1)\u003e)\n\tStmts:\n          #1 bodyStmt[0/0/2]=v\u003cVPBlock(1,2)\u003e := mymap\u003cVPBlock(3,1)\u003e[key\u003cVPBlock(1,0)\u003e]\n          #0 return\n\tBlocks:\n          @(1) Block(ID:0000000000000000000000000000000000000000:0,Addr:0xc006489860,Source:func UP(key (const-type int64), ...,Parent:0xc006488f00)\n            key: (2 int64)\n            value: (2000 int64)\n            v: (undefined)\n (s vals) @(1) Block(ID:0000000000000000000000000000000000000000:0,Addr:0xc00ab97020,Source:func UP(key (const-type int64), ...,Parent:0xc00c3382e0)\n            key: (0 int64)\n            value: (0 int64)\n            v: (0 int64)\n (s typs) @(1) [int64 int64 int64]\n          @(2) Block(ID:8bbe7bdf853fee9f58c5dc608169c0fd10bd8fec:3,Addr:0xc006488f00,Source:ref(gno.land/r/demo/maptest/ff.g...,Parent:0xc006488d20)\n            (RefNode names not shown)\n (s vals) @(2) Block(ID:0000000000000000000000000000000000000000:0,Addr:0xc00c3382e0,Source:file{ package maptest; var mymap...,Parent:0xc00c338b20)\n (s typs) @(2) []\n          @(3) gno.land/r/demo/maptest\n\tBlocks (other):\n\n\tFrames:\n          #0 [FRAME FUNC:UP RECV:(undefined) (2 args) 1/0/0/0/1 LASTPKG:main LASTRLM:Realm(nil)]\n\tRealm:\n\t  gno.land/r/demo/maptest\n\tException:\n\t  \u003cnil\u003e\n\t  undefined\n\nStack Trace:\n    0  /opt/build/pkgs/errors/errors.go:20\n    1  /opt/build/pkgs/sdk/vm/keeper.go:239\n    2  /usr/local/go/src/runtime/panic.go:884\n    3  /usr/local/go/src/runtime/panic.go:260\n    4  /usr/local/go/src/runtime/panic.go:259\n    5  /opt/build/pkgs/gnolang/values.go:668\n    6  /usr/local/go/src/reflect/value.go:587\n    7  /usr/local/go/src/reflect/value.go:368\n    8  /opt/build/pkgs/amino/binary_decode.go:81\n    9  /opt/build/pkgs/amino/binary_decode.go:1018\n   10  /opt/build/pkgs/amino/binary_decode.go:123\n   11  /opt/build/pkgs/amino/binary_decode.go:482\n   12  /opt/build/pkgs/amino/binary_decode.go:402\n   13  /opt/build/pkgs/amino/binary_decode.go:96\n   14  /opt/build/pkgs/amino/amino.go:659\n   15  /opt/build/pkgs/amino/amino.go:604\n   16  /opt/build/pkgs/amino/amino.go:709\n   17  /opt/build/pkgs/amino/amino.go:100\n   18  /opt/build/pkgs/amino/amino.go:100\n   19  /opt/build/pkgs/gnolang/store.go:258\n   20  /opt/build/pkgs/gnolang/store.go:245\n   21  /opt/build/pkgs/gnolang/values.go:2497\n   22  /opt/build/pkgs/gnolang/values.go:2338\n   23  /opt/build/pkgs/gnolang/values.go:2338\n   24  /opt/build/pkgs/gnolang/op_eval.go:33\n   25  /opt/build/pkgs/gnolang/machine.go:1108\n   26  /opt/build/pkgs/gnolang/machine.go:572\n   27  /opt/build/pkgs/sdk/vm/keeper.go:246\n   28  /opt/build/pkgs/sdk/vm/handler.go:65\n   29  /opt/build/pkgs/sdk/vm/handler.go:29\n   30  /opt/build/pkgs/sdk/baseapp.go:644\n   31  /opt/build/pkgs/sdk/baseapp.go:823\n--= /Error =--\n,events:[]",
            "Info": ""
          },
          "GasWanted": "9000000",
          "GasUsed": "71854"
        }
      ],
      "end_block": {
        "ResponseBase": {
          "Error": null,
          "Data": null,
          "Events": null,
          "Log": "",
          "Info": ""
        },
        "ValidatorUpdates": null,
        "ConsensusParams": null,
        "Events": null
      },
      "begin_block": {
        "ResponseBase": {
          "Error": null,
          "Data": null,
          "Events": null,
          "Log": "",
          "Info": ""
        }
      }
    }
  }
}

Expected behaviour

update map value

Actual behaviour

map disappears

Stack Trace

### THIS IS FROM LOCAL NODE, WITH MASTER

.level 0 .msg Invalid tx [error runtime error: invalid memory address or nil pointer dereference log msg:0,success:false,log:-
-= Error =--
Data: &errors.errorString{s:"runtime error: invalid memory address or nil pointer dereference"}
Msg Traces:
    0  /Users/d3v/gno/tm2/pkg/sdk/vm/keeper.go:256 - VM call panic: runtime error: invalid memory address or nil pointer de
reference
Machine:
    CheckTypes: false
        Op: [OpHalt OpExec OpBody OpPopResults OpPrecall]
        Values: (len: 2)
          #1 (println func(xs ...interface{})())
          #0 (UP func(key int64,value int64)())
        Exprs:
          #0 (const (println func(xs ...interface{})()))(mymap<VPBlock(3,2)>)
        Stmts:
          #1 bodyStmt[0/0/2]=v<VPBlock(1,2)> := mymap<VPBlock(3,2)>[key<VPBlock(1,0)>]
          #0 return
        Blocks:
          @(1) Block(ID:0000000000000000000000000000000000000000:0,Addr:0x1400407b680,Source:func UP(key (const-type int64), .
..,Parent:0x1400407ab40)
            key: (2 int64)
            value: (2000 int64)
            v: (undefined)
 (s vals) @(1) Block(ID:0000000000000000000000000000000000000000:0,Addr:0x14008dbf820,Source:func UP(key (const-type int64), .
..,Parent:0x140090bf0a0)
            key: (0 int64)
            value: (0 int64)
            v: (0 int64)
 (s typs) @(1) [int64 int64 int64]
          @(2) Block(ID:83c0a21c577bd92ec839e25e9b692f6a451cebe3:3,Addr:0x1400407ab40,Source:ref(gno.land/r/demo/mmap/ff.gno:.
..,Parent:0x1400407a960)
            (RefNode names not shown)
 (s vals) @(2) Block(ID:0000000000000000000000000000000000000000:0,Addr:0x140090bf0a0,Source:file{ package maptest; var mymap.
..,Parent:0x140090bfba0)
 (s typs) @(2) []
          @(3) gno.land/r/demo/mmap
        Blocks (other):

        Frames:
          #0 [FRAME FUNC:UP RECV:(undefined) (2 args) 1/0/0/0/1 LASTPKG:main LASTRLM:Realm(nil)]
        Realm:
          gno.land/r/demo/mmap
        Exception:
          <nil>
          undefined

Stack Trace:
    0  /Users/d3v/gno/tm2/pkg/errors/errors.go:20
    1  /Users/d3v/gno/tm2/pkg/sdk/vm/keeper.go:256
    2  /Users/d3v/.go/src/runtime/panic.go:885
    3  /Users/d3v/.go/src/runtime/panic.go:260
    4  /Users/d3v/.go/src/runtime/signal_unix.go:838
    5  /Users/d3v/gno/gnovm/pkg/gnolang/values.go:664
    6  /Users/d3v/.go/src/reflect/value.go:587
    7  /Users/d3v/.go/src/reflect/value.go:368
    8  /Users/d3v/gno/tm2/pkg/amino/binary_decode.go:82
    9  /Users/d3v/gno/tm2/pkg/amino/binary_decode.go:1015
   10  /Users/d3v/gno/tm2/pkg/amino/binary_decode.go:122
   11  /Users/d3v/gno/tm2/pkg/amino/binary_decode.go:481
   12  /Users/d3v/gno/tm2/pkg/amino/binary_decode.go:401
   13  /Users/d3v/gno/tm2/pkg/amino/binary_decode.go:95
   14  /Users/d3v/gno/tm2/pkg/amino/amino.go:659
   15  /Users/d3v/gno/tm2/pkg/amino/amino.go:604
   16  /Users/d3v/gno/tm2/pkg/amino/amino.go:710
   17  /Users/d3v/gno/tm2/pkg/amino/amino.go:100
   18  /Users/d3v/gno/gnovm/pkg/gnolang/store.go:280
   19  /Users/d3v/gno/gnovm/pkg/gnolang/store.go:258
   20  /Users/d3v/gno/gnovm/pkg/gnolang/store.go:245
   21  /Users/d3v/gno/gnovm/pkg/gnolang/values.go:2476
   22  /Users/d3v/gno/gnovm/pkg/gnolang/values.go:2319
   23  /Users/d3v/gno/gnovm/pkg/gnolang/values.go:2319
   24  /Users/d3v/gno/gnovm/pkg/gnolang/op_eval.go:34
   25  /Users/d3v/gno/gnovm/pkg/gnolang/machine.go:1152
   26  /Users/d3v/gno/gnovm/pkg/gnolang/machine.go:616
   27  /Users/d3v/gno/tm2/pkg/sdk/vm/keeper.go:264
   28  /Users/d3v/gno/tm2/pkg/sdk/vm/handler.go:65
   29  /Users/d3v/gno/tm2/pkg/sdk/vm/handler.go:29
   30  /Users/d3v/gno/tm2/pkg/sdk/baseapp.go:644
   31  /Users/d3v/gno/tm2/pkg/sdk/baseapp.go:823
--= /Error =--
,events:[] [module state]]
@ajnavarro
Copy link
Contributor

The error here is coming from the virtual machine interpreting your code, not the Gno code itself.

I'll need more information about the Gno version you are executing. Can you provide a specific commit from where you built the binaries? Thanks!

@ajnavarro ajnavarro added the bug label May 10, 2023
@r3v4s
Copy link
Contributor Author

r3v4s commented May 10, 2023

The error here is coming from the virtual machine interpreting your code, not the Gno code itself.

I'll need more information about the Gno version you are executing. Can you provide a specific commit from where you built the binaries? Thanks!

Thx for checking!, I've built binary with current master branch, so latest commit will be 9041d47

@tbruyelle
Copy link
Contributor

It is important to note that the transactions were executed on test3 which is not master.

Do you reproduce the issue with a local node built from master ?

@r3v4s
Copy link
Contributor Author

r3v4s commented May 10, 2023

It is important to note that the transactions were executed on test3 which is not master.

Do you reproduce the issue with a local node built from master ?

sorry to give you confusion.

stacktrace is from my local node with master,
json error msg came from actual test3

so it is happening both test3 and master

@r3v4s r3v4s changed the title [disappearing map] runtime error: invalid memory address or nil pointer dereference runtime error: invalid memory address or nil pointer dereference May 10, 2023
@anarcher
Copy link
Contributor

#311

There seems to be a problem with persisting maps because they are non-deterministic. For now, it looks like you're better off using avl.Tree.

@r3v4s
Copy link
Contributor Author

r3v4s commented May 12, 2023

UPDATE: looks like map variable isn't being saved (doesn't keep state)

Below example code, there are 2 data types

  • map[int64][int64]
  • int64

In init function, I'm initializing each value

After init, even mymap and couter are both global variable, only counter keep its state while mymap doesn't.

package maps

var (
	mymap map[int64]int64 = make(map[int64]int64)
	counter int64
)

func init() {
	mymap[4] = 4444
	counter = 12345
}


func GetData() (map[int64]int64, int64) {

	// gno test returns => map{(4 int64):(4444 int64)}
	// gnokey addpkg & maketx call returns => (map{} map[int64]int64) // XXX where did it go?
	println(mymap) 

	// gno test returns => 12345
	// gnokey addpkg & maketx call returns => (12345 int64)
	println(counter)

	return mymap, counter
}

@thehowl thehowl changed the title runtime error: invalid memory address or nil pointer dereference map state is not persisted correctly, resulting in nil pointer dereference May 12, 2023
@ajnavarro ajnavarro added 🐞 bug Something isn't working 📦 🤖 gnovm Issues or PRs gnovm related and removed bug labels May 15, 2023
@moul
Copy link
Member

moul commented May 25, 2023

Added a challenge: https://github.com/gnolang/gno/pull/833/files#diff-9be2aeb8ae9c1b2c3eaeeb8b1826d1c5406e18c00b8eded45cdae2fe2c899a88

@jaekwon
Copy link
Contributor

jaekwon commented Jun 2, 2023

Added a challenge: https://github.com/gnolang/gno/pull/833/files#diff-9be2aeb8ae9c1b2c3eaeeb8b1826d1c5406e18c00b8eded45cdae2fe2c899a88

nice we're using the challenges folder! thank you manfred

@jaekwon jaekwon removed their assignment Jun 2, 2023
@jaekwon
Copy link
Contributor

jaekwon commented Jun 2, 2023

Anyone can try this; please add me as reviewer.

tbruyelle added a commit to tbruyelle/gno that referenced this issue Jun 26, 2023
Fix gnolang#817

There was actually 2 bugs in the code responsible for loading a map from
the state:
- `UnmarshalAmino` doesn't check that `ml.Tail` is nil before accessing
  it. It is correctly done for instance in `MapList.Append` for
  instance. The fix just ensures that `ml.Tail` is not nil.
- `fillTypesOfValue` which is called just after the store data is
  unmarshalSized, resets the map data by calling `MapValue.MakeMap()`.
  So at the end the map is always empty, whatever was stored before the
  transaction. The fix replaces the call of `MakeMap` to a simple map
  creation.

This has to be confirmed, but this change may also fix gnolang#311, because I'm
not convinced that the indetermenistic behavior of the golang map can be
a problem when saved to the store. Indeed, a gno map is serialized with
the help of a slice, which should remove any indetermenistic problem.
tbruyelle added a commit to tbruyelle/gno that referenced this issue Jun 26, 2023
Fix gnolang#817

There was actually 2 bugs in the code responsible for loading a map from
the state:
- `UnmarshalAmino` doesn't check that `ml.Tail` is nil before accessing
  it. It is correctly done for instance in `MapList.Append` for
  instance. The fix just ensures that `ml.Tail` is not nil.
- `fillTypesOfValue` which is called just after the store data is
  unmarshalSized, resets the map data by calling `MapValue.MakeMap()`.
  So at the end the map is always empty, whatever was stored before the
  transaction. The fix replaces the call of `MakeMap` to a simple map
  creation.

This has to be confirmed, but this change may also fix gnolang#311, because I'm
not convinced that the indetermenistic behavior of the golang map can be
a problem when saved to the store. Indeed, a gno map is serialized with
the help of a slice, which should remove any indetermenistic problem.
@tbruyelle
Copy link
Contributor

Anyone can try this; please add me as reviewer.

@jaekwon @moul See #932 for the fixes

@moul moul closed this as completed in #932 Jun 27, 2023
@moul moul moved this to 🚀 Needed for Launch in 🚀 The Launch [DEPRECATED] Oct 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working 📦 🤖 gnovm Issues or PRs gnovm related
Projects
Status: 🚀 Needed for Launch
Development

Successfully merging a pull request may close this issue.

6 participants