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

[GnoVM] bug: inconsistent map key #2060

Open
deelawn opened this issue May 8, 2024 · 4 comments · May be fixed by #3127
Open

[GnoVM] bug: inconsistent map key #2060

deelawn opened this issue May 8, 2024 · 4 comments · May be fixed by #3127
Assignees
Labels
🐞 bug Something isn't working in focus Core team is prioritizing this work 📦 🤖 gnovm Issues or PRs gnovm related

Comments

@deelawn
Copy link
Contributor

deelawn commented May 8, 2024

Description

When a pointer to a persisted realm variable is used as a key for a map type, the key value is not consistent across multiple transactions. This is because it computes the key by using the value's actual TV at runtime. This address will change each time the value is loaded into memory for each transaction.

This seems like an issue but it is hard to tell. I guess it depends on how we define the lifetime of a pointer. Should persisted realm values be considered to have a lifetime of start of execution to end of execution, saved and loaded each time? Or should we define the lifetime as being infinite since we've abstracted away the saving and loading of the data? If there is a pointer type variable persisted to realm storage, what should we consider to be persisted -- the address (not the case) or the reference (this is the case)? I think it's more intuitive that both realm pointer values and pointers to realm values produce a consistent result when used as keys for maps but could see it argued either way, especially when viewing it from the point of view where values are loaded into memory for each transaction, so it should be expected that the pointer values are no longer the same.

Here is an example:

loadpkg gno.land/r/ptrmap $WORK

gnoland start

gnokey maketx call -pkgpath gno.land/r/ptrmap -func AddToMap -args 5 -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1
stdout OK!

gnokey maketx call -pkgpath gno.land/r/ptrmap -func GetFromMap -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1
stdout '(5 int)'
stdout OK!

-- realm.gno --
package ptrmap

var (
	m = map[*int]int{}
	i = new(int)
)

func AddToMap(value int) {
	m[i] = value
}

func GetFromMap() int {
	return m[i]
}
@deelawn deelawn added the 📦 🤖 gnovm Issues or PRs gnovm related label May 8, 2024
@Kouteki Kouteki added the 🐞 bug Something isn't working label May 10, 2024
@Kouteki Kouteki moved this from Triage to Backlog in 🧙‍♂️gno.land core team May 10, 2024
@omarsy
Copy link
Member

omarsy commented May 12, 2024

We also have it in this scenario.Maybe In all scenarios, we need to work with pointers.

# load the package from $WORK directory
loadpkg gno.land/r/ptrmap $WORK
gnoland start


gnokey maketx call -pkgpath gno.land/r/ptrmap -func Add -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1
stdout OK!

gnokey maketx call -pkgpath gno.land/r/ptrmap -func Test -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1
stdout '(true bool)'
stdout OK!

-- realm.gno --
package ptrmap

var (
	li = []*int{}
	i = new(int)
)
func Add() {
	li = append(li, i)
}

func Test() bool {
	return li[0] == i
}

I think the problem lies in the way we save and load a pointer, where the pointer is loaded without taking into account its previous dependencies.

I think it's more intuitive that both realm pointer values and pointers to realm values produce a consistent result when used as keys for maps but could

I think too

A possible solution might be to review the process of saving and loading Object in a Realm. When the object is saved, each object of type pointer would be considered as a distinct object. Consequently, when the State is fully loaded, each pointer object would only be loaded once.

@thehowl
Copy link
Member

thehowl commented May 22, 2024

Just commenting that this is indeed a bug

Making sure that pointers point to the same thing, even in serialisation, is a feature we want. Persisting pointers properly allows us to use them as the "foreign keys" from one data structure to the other. So yes, this should be fixed; as for the implementation, I think one could be creating a counter for objects (ObjectID) regardless of whether they're stored or not; but it's just an idea.

@petar-dambovaliev
Copy link
Contributor

Lifetimes shoud be infinite IMO

@thehowl
Copy link
Member

thehowl commented Oct 21, 2024

possibly related to #2429

@ltzmaxwell ltzmaxwell added the in focus Core team is prioritizing this work label Nov 1, 2024
@ltzmaxwell ltzmaxwell linked a pull request Nov 14, 2024 that will close this issue
6 tasks
@Kouteki Kouteki moved this from Todo to In Progress in 🧙‍♂️gno.land core team Nov 15, 2024
@Kouteki Kouteki moved this to Core in 🍜 Seoul triage Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working in focus Core team is prioritizing this work 📦 🤖 gnovm Issues or PRs gnovm related
Projects
Status: Core
Status: In Progress
Development

Successfully merging a pull request may close this issue.

8 participants