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 · 0 comments
Open

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

thehowl opened this issue May 14, 2024 · 0 comments

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

@thehowl thehowl self-assigned this May 14, 2024
@Kouteki Kouteki added this to the 🏗4️⃣ test4.gno.land milestone May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

2 participants