fix(std): add full denom in banker issue & remove coin

[MikaelVallenet]

fix #2107

---

Problem

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

---

Solution

BREAKING CHANGE: All previous realm calling IssueCoin or RemoveCoin are now expected to append the prefix "/" + realmPkgPath + ":" before the denom, it should be done by using std.CurrentRealm().CoinDenom(denom string) or by using std.CoinDenom(pkgPath, denom string)

For now to avoid to mix coins and fix security issues like being able to issue coins from other realm, when a realm issue a coin, the pkg path of the realm is added as a prefix to the coin.

the thing is some function expect only the base denom bitcoin (issue & remove) but the others like get require the qualified denom gno.land/r/demo/banktest:bitcoin. it can be confusing

I also answer the requirements of the comment @thehowl made:

• Two functions are now available std.CoinDenom(pkgpath, demon string) && the method std.Realm.CoinDenom(denom string)
• the denom's value is changed in the .gno file and not the native.

Here is an example of how it looks like:

func IssueNewCoin(denom string, amount int64) string {
	std.AssertOriginCall()
	banker := std.GetBanker(std.BankerTypeRealmIssue)
	addr := std.PrevRealm().Addr()
	banker.IssueCoin(addr, std.CurrentRealm().CoinDenom(denom), amount)
	return std.CurrentRealm().Denom(denom)
}

func RemoveCoin(denom string, amount int64) {
	std.AssertOriginCall()
	banker := std.GetBanker(std.BankerTypeRealmIssue)
	addr := std.PrevRealm().Addr()
	banker.RemoveCoin(addr, std.CurrentRealm().CoinDenom(denom), amount)
}

func GetCoins(denom string) uint64 {
	banker := std.GetBanker(std.BankerTypeReadonly)
	addr := std.PrevRealm().Addr()
	coins := banker.GetCoins(addr)
	for _, coin := range coins {
		if coin.Denom == std.CurrentRealm().CoinDenom(denom) {
			return uint64(coin.Amount)
		}
	}
	return 0
}

Contributors' checklist...

• 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

[Gno2D2]

🛠 PR Checks Summary

🔴 Changes to 'docs' folder must be reviewed/authored by at least one devrel and one tech-staff

Manual Checks (for Reviewers):

• SKIP: Do not block the CI for this PR (checked by @leohhhn)

Read More

🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers.

✅ Automated Checks (for Contributors):

🟢 Maintainers must be able to edit this pull request (more info)
🟢 The pull request head branch must be up-to-date with its base (more info)
🔴 Changes to 'docs' folder must be reviewed/authored by at least one devrel and one tech-staff

☑️ Contributor Actions:

1. Fix any issues flagged by automated checks.
2. Follow the Contributor Checklist to ensure your PR is ready for review.
   • Add new tests, or document why they are unnecessary.
   • Provide clear examples/screenshots, if necessary.
   • Update documentation, if required.
   • Ensure no breaking changes, or include BREAKING CHANGE notes.
   • Link related issues/PRs, where applicable.

☑️ Reviewer Actions:

1. Complete manual checks for the PR, including the guidelines and additional checks if applicable.

📚 Resources:

• Report a bug with the bot.
• View the bot’s configuration file.

Debug

Automated Checks

Maintainers must be able to edit this pull request (more info)

If

🟢 Condition met
└── 🟢 The pull request was created from a fork (head branch repo: MikaelVallenet/gno)

Then

🟢 Requirement satisfied
└── 🟢 Maintainer can modify this pull request

The pull request head branch must be up-to-date with its base (more info)

If

🟢 Condition met
└── 🟢 On every pull request

Then

🟢 Requirement satisfied
└── 🟢 Head branch (MikaelVallenet:dev/mikaelvallenet/feat/full-denom-banker-issue-and-remove-coin) is up to date with base (master): behind by 0 / ahead by 45

Changes to 'docs' folder must be reviewed/authored by at least one devrel and one tech-staff

If

🟢 Condition met
└── 🟢 A changed file matches this pattern: ^docs/ (filename: docs/reference/stdlibs/std/banker.md)

Then

🔴 Requirement not satisfied
└── 🔴 Or
    ├── 🔴 And
    │   ├── 🔴 Pull request author is a member of the team: devrels
    │   └── 🟢 At least 1 user(s) of the team tech-staff approved pull request
    └── 🔴 And
        ├── 🔴 Pull request author is a member of the team: tech-staff
        └── 🟢 At least 1 user(s) of the team devrels approved pull request

Manual Checks

**SKIP**: Do not block the CI for this PR

If

🟢 Condition met
└── 🟢 On every pull request

Can be checked by

• Any user with comment edit permission

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
gnovm/stdlibs/std/banker.go	0.00%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

[jefft0]

Hello @MikaelVallenet . Can you fix the failed CI test for TestFiles/zrealm_natbind0.gno ?

[MikaelVallenet]

Hello @MikaelVallenet . Can you fix the failed CI test for TestFiles/zrealm_natbind0.gno ?

done

[jefft0]

Hi @MikaelVallenet . Part of my job on the review team is the make sure that possible breaking changes are documented.

• Could this be a breaking change for other realms which use the banker? For example, will std.IssueCoin panic in cases where it does not currently panic?
• I see that you changed many tests to use more gas-wanted. Was the realm changed to require more gas? Could this break existing use of the realm?

[MikaelVallenet]

Hi @MikaelVallenet . Part of my job on the review team is the make sure that possible breaking changes are documented.

• Could this be a breaking change for other realms which use the banker? For example, will std.IssueCoin panic in cases where it does not currently panic?
• I see that you changed many tests to use more gas-wanted. Was the realm changed to require more gas? Could this break existing use of the realm?

I don't really know how gas is managed. My intuition is that with the addition of new functions within the “std” lib, it becomes more gas-expensive when imported.

In terms of breaking changes, this modification may break the current realms as the Banker API now expects a full denomination e.g. /gno.land/r/demo/banktest:bitcoin instead of before where it expected just the base denom bitcoin in IssueCoin & RemoveCoin.

[leohhhn]

@MikaelVallenet

Just following up on some internal discussions we had during the retreat, mainly about the naming of the functions. Please take a look at the comments below 🙏

[thehowl]

sorry for the back and forths Mikael, naming is always hard :)

[leohhhn]

lgtm for the docs 👍

Thanks @MikaelVallenet!

[leohhhn]

lgtm for the docs 👍

Thanks @MikaelVallenet!