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

Erroring related to top-level variables in p/ #1538

Open
leohhhn opened this issue Jan 15, 2024 · 1 comment
Open

Erroring related to top-level variables in p/ #1538

leohhhn opened this issue Jan 15, 2024 · 1 comment

Comments

@leohhhn
Copy link
Contributor

leohhhn commented Jan 15, 2024

Description

Packages under p/ are not supposed to have top-level variables - they are supposed to be stateless. Packages can have top-level variables and they can be initialized, but cannot be later modified. Erroring related to this is non-existent - modifying these variables will not work as expected. Replicate the following example locally:

In root of Gno repo, run

mkdir examples/gno.land/p/demo/tt

Add following files:

p/demo/tt/gno.mod:

module gno.land/p/demo/tt

p/demo/tt/tt.gno:

package tt

var toplevelvar string

func init() {
	toplevelvar = "initialized"
}

func GetVar() string {
	return toplevelvar
}

func SetVar(arg string) {
	toplevelvar = arg
}
  • Deployment of this will pass normally when running the chain.
  • Calling GetVar with gnokey will successfully access the variable
> gnokey maketx call -pkgpath gno.land/p/demo/tt --func GetVar --gas-fee 10000000ugnot --gas-wanted 800000 --broadcast dev  

Enter password.

("initialized" string)
OK!
GAS WANTED: 800000
GAS USED:   67444
  • Calling SetVar will not error in any way, but toplevelvar will not be updated:
> gnokey maketx call -pkgpath gno.land/p/demo/tt --func SetVar --args "changed" --gas-fee 10000000ugnot --gas-wanted 800000 --broadcast dev

Enter password.

OK!
GAS WANTED: 800000
GAS USED:   70271

> gnokey maketx call -pkgpath gno.land/p/demo/tt --func GetVar --gas-fee 10000000ugnot --gas-wanted 800000 --broadcast dev                           

Enter password.
("initialized" string)
OK!
GAS WANTED: 800000
GAS USED:   70081

Not sure if this is a feature or a bug, but we should be clear on erroring related to this, since we are pushing the "packages are stateless" narrative.

Also, should we allow for init() in p/?

EDIT: Top-level variables in packages is indeed a feature. Packages can define top-level variables and have them initialized in init(). However, they are not to be modified, and we do not have erroring related to that, as you can see in the previous CLI block.

@thehowl
Copy link
Member

thehowl commented Jan 26, 2024

I'll tell you something even funnier: you can actually maketx call stdlibs! 😂

$ gnokey maketx call -broadcast -pkgpath strconv -gas-wanted 10000000 -gas-fee 1000000ugnot -func Itoa -args 11 gentest
Enter password.
("11" string)
OK!
GAS WANTED: 10000000
GAS USED:   77386

I think the action here is to restrict maketx call to work exclusively on gno.land/r. wdyt @moul @jaekwon?

thehowl added a commit that referenced this issue May 31, 2024
<!-- please provide a detailed description of the changes made in this
pull request. -->

<details><summary>Contributors' checklist...</summary>

- [ ] Added new tests, or not needed, or not feasible
- [ ] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [ ] Updated the official documentation or not needed
- [ ] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [ ] Added references to related issues and PRs
- [ ] Provided any useful hints for running manual tests
- [ ] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>

Follow discussion in #2192 and [TheHowl's
comment](#1538 (comment)),
this PR aims to prohibit the use of `maketx call` with a `p/` entrypoint
and force `maketx call` to call to a `gno.land/` pkgpath

Behavior:
```
$ gnokey maketx call -broadcast -pkgpath gno.land/p/ -gas-wanted 10000000 -gas-fee 1000000ugnot -func Demo -remote localhost:26657 testKey

--= Error =--
Data: forbidden/bad package called
Msg Traces:
    0  ....... deliver transaction failed: log:
--= /Error =--
```

```
$ gnokey maketx call -broadcast -pkgpath gno.land/p -gas-wanted 10000000 -gas-fee 1000000ugnot -func Demo -remote localhost: 26657 testKey

--= Error =--
Data: forbidden/bad package called
Msg Traces:
    0  ....... deliver transaction failed: log:
--= /Error =--
```

For `stdlibs`, force the pkgpath to begins with `gno.land/`
```
$ gnokey maketx call -broadcast -pkgpath strconv -gas-wanted 10000000 -gas-fee 1000000ugnot -func Itoa -args 11 testKey

--= Error =--
Data: forbidden/bad package called
Msg Traces:
    0  ....... deliver transaction failed: log:
--= /Error =--
```

---------

Co-authored-by: Morgan Bazalgette <morgan@morganbaz.com>
DIGIX666 pushed a commit to DIGIX666/gno that referenced this issue Jun 2, 2024
<!-- please provide a detailed description of the changes made in this
pull request. -->

<details><summary>Contributors' checklist...</summary>

- [ ] Added new tests, or not needed, or not feasible
- [ ] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [ ] Updated the official documentation or not needed
- [ ] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [ ] Added references to related issues and PRs
- [ ] Provided any useful hints for running manual tests
- [ ] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>

Follow discussion in gnolang#2192 and [TheHowl's
comment](gnolang#1538 (comment)),
this PR aims to prohibit the use of `maketx call` with a `p/` entrypoint
and force `maketx call` to call to a `gno.land/` pkgpath

Behavior:
```
$ gnokey maketx call -broadcast -pkgpath gno.land/p/ -gas-wanted 10000000 -gas-fee 1000000ugnot -func Demo -remote localhost:26657 testKey

--= Error =--
Data: forbidden/bad package called
Msg Traces:
    0  ....... deliver transaction failed: log:
--= /Error =--
```

```
$ gnokey maketx call -broadcast -pkgpath gno.land/p -gas-wanted 10000000 -gas-fee 1000000ugnot -func Demo -remote localhost: 26657 testKey

--= Error =--
Data: forbidden/bad package called
Msg Traces:
    0  ....... deliver transaction failed: log:
--= /Error =--
```

For `stdlibs`, force the pkgpath to begins with `gno.land/`
```
$ gnokey maketx call -broadcast -pkgpath strconv -gas-wanted 10000000 -gas-fee 1000000ugnot -func Itoa -args 11 testKey

--= Error =--
Data: forbidden/bad package called
Msg Traces:
    0  ....... deliver transaction failed: log:
--= /Error =--
```

---------

Co-authored-by: Morgan Bazalgette <morgan@morganbaz.com>
omarsy pushed a commit to TERITORI/gno that referenced this issue Jun 3, 2024
<!-- please provide a detailed description of the changes made in this
pull request. -->

<details><summary>Contributors' checklist...</summary>

- [ ] Added new tests, or not needed, or not feasible
- [ ] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [ ] Updated the official documentation or not needed
- [ ] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [ ] Added references to related issues and PRs
- [ ] Provided any useful hints for running manual tests
- [ ] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>

Follow discussion in gnolang#2192 and [TheHowl's
comment](gnolang#1538 (comment)),
this PR aims to prohibit the use of `maketx call` with a `p/` entrypoint
and force `maketx call` to call to a `gno.land/` pkgpath

Behavior:
```
$ gnokey maketx call -broadcast -pkgpath gno.land/p/ -gas-wanted 10000000 -gas-fee 1000000ugnot -func Demo -remote localhost:26657 testKey

--= Error =--
Data: forbidden/bad package called
Msg Traces:
    0  ....... deliver transaction failed: log:
--= /Error =--
```

```
$ gnokey maketx call -broadcast -pkgpath gno.land/p -gas-wanted 10000000 -gas-fee 1000000ugnot -func Demo -remote localhost: 26657 testKey

--= Error =--
Data: forbidden/bad package called
Msg Traces:
    0  ....... deliver transaction failed: log:
--= /Error =--
```

For `stdlibs`, force the pkgpath to begins with `gno.land/`
```
$ gnokey maketx call -broadcast -pkgpath strconv -gas-wanted 10000000 -gas-fee 1000000ugnot -func Itoa -args 11 testKey

--= Error =--
Data: forbidden/bad package called
Msg Traces:
    0  ....... deliver transaction failed: log:
--= /Error =--
```

---------

Co-authored-by: Morgan Bazalgette <morgan@morganbaz.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Triage
Development

No branches or pull requests

2 participants