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

Feat/stdlib diff #1

Open
wants to merge 117 commits into
base: master
Choose a base branch
from
Open

Feat/stdlib diff #1

wants to merge 117 commits into from

Conversation

Villaquiranm
Copy link
Owner

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

hthieu1110 and others added 30 commits September 21, 2024 07:14
jeronimoalbi and others added 10 commits December 11, 2024 11:42
The new package is a generic implementation for datasources. It aims to
be one possible solution to integrate/aggregate data from different
realms.
…lang#3121)

If we want to guard the MemStore by checking the active DAO realm,
m.daoPkgPath must first be assigned a realm package path; otherwise, the
isCallerDAORealm() method may return a false positive, failing to
protect the MemStore.

<!-- 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
</details>

---------

Co-authored-by: Miloš Živković <[email protected]>
fix gnolang#2107 

--------------------------

## Problem

> From [gnolang#875
(review)](gnolang#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 gnolang#1475 gnolang#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:

```go
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
}
```

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

- [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
- [x] 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
- [x] Provided any useful hints for running manual tests
</details>

---------

Co-authored-by: Morgan Bazalgette <[email protected]>
Co-authored-by: Leon Hudak <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.