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

Calling injected function in same package files (returns undefined error) #876

Closed
r3v4s opened this issue Jun 7, 2023 · 2 comments
Closed
Labels
❓ question Questions about Gno

Comments

@r3v4s
Copy link
Contributor

r3v4s commented Jun 7, 2023

Description

To fix bug #875, I had to use injected function std.GetOrigCaller in banker.gno which located in std

GetOrigCaller() is being injected from here into std package

pn.DefineNative("GetOrigCaller",
gno.Flds( // params
),
gno.Flds( // results
"", "Address",
),
func(m *gno.Machine) {
ctx := m.Context.(ExecContext)
res0 := gno.Go2GnoValue(
m.Alloc,
m.Store,
reflect.ValueOf(ctx.OrigCaller),
)
addrT := store.GetType(gno.DeclaredTypeID("std", "Address"))
res0.T = addrT
m.PushValue(res0)
},
)

So I thought the package std declared by banker.gno

is the same package with std inject by stdlibs.go

case "std":

But calling GetOrigCaller in banker.gno returned undefined error
Is is intended structure??

@ajnavarro ajnavarro added the ❓ question Questions about Gno label Jun 7, 2023
@r3v4s r3v4s changed the title Calling injected function in same package files Calling injected function in same package files (returns undefined error) Jun 7, 2023
@thehowl
Copy link
Member

thehowl commented Jun 9, 2023

@r3v4s the way that native imports are implemented are that when you import a package, ie. you write import "std", the gnovm loads the package's symbols, and on top of them some extra native bindings.

Notice that this means that native functions are only available to external users of the package; not from the package itself. This is one of the things I aim to improve/fix with #814.

For now, if you need to fix the bug, a good idea might be to extend stdlibs.go to go from case "std" to case "std", "internal/stdnative"; so that after that, calling GetOrigCaller becomes possible from banker.gno, by importing internal/stdnative.

@r3v4s
Copy link
Contributor Author

r3v4s commented Jun 12, 2023

@r3v4s the way that native imports are implemented are that when you import a package, ie. you write import "std", the gnovm loads the package's symbols, and on top of them some extra native bindings.

Notice that this means that native functions are only available to external users of the package; not from the package itself. This is one of the things I aim to improve/fix with #814.

For now, if you need to fix the bug, a good idea might be to extend stdlibs.go to go from case "std" to case "std", "internal/stdnative"; so that after that, calling GetOrigCaller becomes possible from banker.gno, by importing internal/stdnative.

thx @thehowl, perhaps I have to stick with internal/stdnative for now.
I'll keep my eye open for your #814 :D

@r3v4s r3v4s closed this as completed Jun 13, 2023
zivkovicmilos pushed a commit that referenced this issue May 14, 2024
<!-- Please provide a brief summary of your changes in the Title above
-->
# Previous & Related PR
#393 

# Description
## BREAKING CHANGE: banker will not use raw(input) denom for minting
Currently, there is a bug that additional coins are mintable using
banker.
Lots of discussing were made in #393, this PR include following.

1. It aims to use `pkg_path` that called (banker or Mint) as prefix for
denom(Similar to IBC Spec)
```
ibc_denom := 'ibc/' + hash('path' + 'base_denom')
denom_in_this_pr := '{pkg_path}:denom'
```

~2. As @piux2 mentioned
[here](#393 (comment))
currently gno has very inflexible format regexp for denom
-- Changed to same as Cosmos's regex, but without capitals~


~### is some issue about using `std.GetOrigCaller` or `std.PrevRealm`
Some of you might be wonder why I made some packaged called `istd`, I
made a issue: #876~
Update: After @thehowl's native binding PR, doesn't need this kind of
work around


## Contributors Checklist

- [x] Added new tests, or not needed, or not feasible
- [x] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [ ] Updated the official documentation or not needed
- [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [x] Added references to related issues and PRs
- [ ] Provided any useful hints for running manual tests

## Maintainers Checklist

- [ ] Checked that the author followed the guidelines in
`CONTRIBUTING.md`
- [ ] Checked the conventional-commit (especially PR title and verb,
presence of `BREAKING CHANGE:` in the body)
- [ ] Ensured that this PR is not a significant change or confirmed that
the review/consideration process was appropriate for the change

---------

Co-authored-by: Morgan <[email protected]>
jefft0 pushed a commit to jefft0/gno that referenced this issue May 15, 2024
<!-- Please provide a brief summary of your changes in the Title above
-->
# Previous & Related PR
gnolang#393 

# Description
## BREAKING CHANGE: banker will not use raw(input) denom for minting
Currently, there is a bug that additional coins are mintable using
banker.
Lots of discussing were made in gnolang#393, this PR include following.

1. It aims to use `pkg_path` that called (banker or Mint) as prefix for
denom(Similar to IBC Spec)
```
ibc_denom := 'ibc/' + hash('path' + 'base_denom')
denom_in_this_pr := '{pkg_path}:denom'
```

~2. As @piux2 mentioned
[here](gnolang#393 (comment))
currently gno has very inflexible format regexp for denom
-- Changed to same as Cosmos's regex, but without capitals~


~### is some issue about using `std.GetOrigCaller` or `std.PrevRealm`
Some of you might be wonder why I made some packaged called `istd`, I
made a issue: gnolang#876~
Update: After @thehowl's native binding PR, doesn't need this kind of
work around


## Contributors Checklist

- [x] Added new tests, or not needed, or not feasible
- [x] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [ ] Updated the official documentation or not needed
- [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [x] Added references to related issues and PRs
- [ ] Provided any useful hints for running manual tests

## Maintainers Checklist

- [ ] Checked that the author followed the guidelines in
`CONTRIBUTING.md`
- [ ] Checked the conventional-commit (especially PR title and verb,
presence of `BREAKING CHANGE:` in the body)
- [ ] Ensured that this PR is not a significant change or confirmed that
the review/consideration process was appropriate for the change

---------

Co-authored-by: Morgan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
❓ question Questions about Gno
Projects
Development

No branches or pull requests

3 participants