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

Expect full denoms in Banker.IssueCoin and Banker.RemoveCoin #2107

Open
thehowl opened this issue May 14, 2024 · 1 comment · May be fixed by #3239
Open

Expect full denoms in Banker.IssueCoin and Banker.RemoveCoin #2107

thehowl opened this issue May 14, 2024 · 1 comment · May be fixed by #3239
Assignees

Comments

@thehowl
Copy link
Member

thehowl commented May 14, 2024

From #875 (review):

That said, soon after this is merged, I think we'll need to change this API again. This current implementation creates an inconsistency within the Banker API. All other banker methods now require you to pass in the full realm path to the token you're referring to, but IssueCoin and RemoveCoin do not.

Thus, I think a few more changes are in order:

  1. There should be a RealmDenom(pkgpath, denom string) function in std, which creates a realm denomination (ie. /gno.land/r/morgan:bitcoin). There can be a helper method Realm.Denom(denom string) (so you can do std.CurrentRealm().Denom("bitcoin")
  2. Instead of modifying denom's value in the native function, we should check it matches what we expect. ie. strings.HasPrefix(denom, RealmDenom(std.CurrentRealm().PkgPath()), then check the last part of the denom to see that it matches the Gno regex. (This can all be done in gno, without needing to put it in native code)

Related with #1475 #1576

@MikaelVallenet
Copy link
Member

I will start to work on it in few days after finishing time pkg PR

@MikaelVallenet MikaelVallenet linked a pull request Nov 28, 2024 that will close this issue
6 tasks
@Kouteki Kouteki moved this from Todo to In Progress in 🧙‍♂️gno.land core team Nov 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Teritori (confirmed)
Status: In Progress
Development

Successfully merging a pull request may close this issue.

6 participants