refactor: GRC20 #2314

leohhhn · 2024-06-10T12:09:31Z

Description

Closes: #2294

This PR refactors the GRC20 folder to follow the ERC20 spec as close as possibly.

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
Added new benchmarks to generated graphs, if any. More info here.

leohhhn · 2024-06-10T13:03:18Z

Interesting discussion point: #2316

leohhhn · 2024-06-12T12:24:47Z

Opening up for reviews - grc20 package, foo20, wugnot & grc20 factory realms have been refactored, need to fix up some other stuff that is failing due to the changed API, awaiting a reply from @moul

tbruyelle

I really like the simplicity of this version.

With grc20factory that acts as a grc20 multiplexer, I wonder if we still need to have the IGRC20 interface. This interface was initially designed to make *grc20.Token implementation shareable between realms, but now these realms can just use the grc20factory right ?

leohhhn · 2024-06-17T10:17:23Z

@tbruyelle I think both have valid use cases. People can choose to use the factory, or simply write their own realm importing the GRC20 package.

moul · 2024-06-18T16:48:18Z

examples/gno.land/p/demo/grc/grc20/grc20.gno

+	ApprovalEvent = "Approval"
+)
+
+func NewGRC20Token(name, symbol string, decimals uint8) *Token {


Suggested change

func NewGRC20Token(name, symbol string, decimals uint8) *Token {

func NewToken(name, symbol string, decimals uint8) *Token {

grc20 is the package name.

i think we should not have a NewToken helper, but only a NewBanker one.

moul · 2024-06-18T17:27:57Z

examples/gno.land/p/demo/grc/grc20/grc20.gno

+	"gno.land/p/demo/ufmt"
+)
+
+type Token struct {


This looks good, but you need to have a type Banker interface { Token /* embedded */, Mint() } which is what you initializes first.

the banker should also have all the "admin" helpers, such as ways to call Transfer() and specifying a custom from.

moul · 2024-06-18T17:28:57Z

examples/gno.land/p/demo/grc/grc20/grc20.gno

+}
+
+func (t *Token) BalanceOf(owner std.Address) (balance uint64) {
+	mustBeValid(owner)


we can remove this one.

moul · 2024-06-18T17:29:40Z

examples/gno.land/p/demo/grc/grc20/grc20.gno

+	return rawAllowance.(uint64)
+}
+
+func (t *Token) Mint(account std.Address, amount uint64) {


Suggested change

func (t *Token) Mint(account std.Address, amount uint64) {

func (t *Banker) Mint(account std.Address, amount uint64) {

moul · 2024-06-18T17:31:46Z

examples/gno.land/p/demo/grc/grc20/grc20.gno

+			panic(err)
+		}
+
+		t.totalSupply = sum


shouldn't you also mint the tokens to the to address?

moul · 2024-06-18T17:42:30Z

examples/gno.land/r/demo/foo20/foo20.gno

+	o = ownable.New()
+	o.TransferOwnership("g125em6arxsnj49vx35f0n0z34putv5ty3376fg5") // @admin


Let's remove the token owner completely and expose a "Faucet" function for the demonstration.

See #2388 for an example.

moul · 2024-06-18T17:44:40Z

examples/gno.land/r/demo/wugnot/z0_filetest.gno

I prefer if you keep this file as a test, or else, please implement it completely using standard unit tests.

moul · 2024-06-18T17:46:23Z

examples/gno.land/r/demo/wugnot/wugnot.gno

-	if err != nil {
-		panic(err)
-	}
+	wugnot.Approve(users.Resolve(spender), amount)


why removing the panic?

moul · 2024-06-18T17:48:01Z

examples/gno.land/r/demo/wugnot/wugnot.gno

-		return ufmt.Sprintf("%d\n", balance)
+	case c == 2 && parts[0] == "balance": // pkgpath:balance/address_or_username
+		owner := pusers.AddressOrName(parts[1])
+		return ufmt.Sprintf("%d\n", wugnot.BalanceOf(users.Resolve(owner)))


Suggested change

return ufmt.Sprintf("%d\n", wugnot.BalanceOf(users.Resolve(owner)))

balance := wugnot.BalanceOf(users.Resolve(owner))

return ufmt.Sprintf("%d\n", balance)

moul · 2024-06-18T17:52:55Z

examples/gno.land/p/demo/grc/grc20/grc20.gno

+func (t *Token) spendAllowance(owner, spender std.Address, value uint64) {
+	currentAllowance := t.Allowance(owner, spender)
+
+	if currentAllowance != math.MaxUint64 {


This exception should be documented. If I'm not mistaken, you consider a MaxUint64 allowance to be equivalent to an infinite allowance, correct?

moul · 2024-06-18T18:00:16Z

examples/gno.land/r/demo/grc20factory/grc20factory_test.gno

-	unknown := std.Address("g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5") // valid but never used.
+
+	admin := testutils.TestAddress("admin")     // g1v9kxjcm9ta047h6lta047h6lta047h6lzd40gh
+	manfred := testutils.TestAddress("manfred") // g1wymu47drhr0kuq2098m792lytgtj2nyx77yrsm


Suggested change

manfred := testutils.TestAddress("manfred") // g1wymu47drhr0kuq2098m792lytgtj2nyx77yrsm

alice := testutils.TestAddress("alice") // xxx

let's keep "manfred" when it's really my address.

moul · 2024-06-18T18:03:16Z

examples/gno.land/r/demo/grc20factory/grc20factory.gno

+	t := tokenWithAdmin{
+		token:    newToken,
+		admin:    admin,
+		fauceted: false, // faucet disabled by default


If disabled by default, please register one or two tokens in init() that will be fauceted as true by default, so this package can be easily tested.

```go // Package bar20 is similar to foo20 but exposes a safe-object that can be used // by `maketx run`, another contract importing foo20, and in the future when // we'll support `maketx call Token.XXX`. ``` This package currently has limited functionality, but it should become more useful in the future. I'm using it to demonstrate the rationale for having two implementations in the `grc20` package - one with a banker, and one with a safe object. Related with #2314 --------- Signed-off-by: moul <94029+moul@users.noreply.github.com>

moul · 2024-07-07T01:09:38Z

#2529

@leohhhn

Main changes: - rename `AdminToken` -> `Banker` - rename `GRC20` -> `Token` - remove unused helpers - remove vault (temporarily, will be reimplemented) - remove the returner ˋerror` when unnecessary - use `std.Emit` - use uassert for testing - better file naming and organization for improved readability Fixes #2294 Replaces #2314 (h/t @leohhhn) ~Depends on #2534~ BREAKING CHANGE --------- Signed-off-by: moul <94029+moul@users.noreply.github.com> Co-authored-by: Leon Hudak <33522493+leohhhn@users.noreply.github.com>

```go // Package bar20 is similar to foo20 but exposes a safe-object that can be used // by `maketx run`, another contract importing foo20, and in the future when // we'll support `maketx call Token.XXX`. ``` This package currently has limited functionality, but it should become more useful in the future. I'm using it to demonstrate the rationale for having two implementations in the `grc20` package - one with a banker, and one with a safe object. Related with gnolang#2314 --------- Signed-off-by: moul <94029+moul@users.noreply.github.com>

@leohhhn

Main changes: - rename `AdminToken` -> `Banker` - rename `GRC20` -> `Token` - remove unused helpers - remove vault (temporarily, will be reimplemented) - remove the returner ˋerror` when unnecessary - use `std.Emit` - use uassert for testing - better file naming and organization for improved readability Fixes gnolang#2294 Replaces gnolang#2314 (h/t @leohhhn) ~Depends on gnolang#2534~ BREAKING CHANGE --------- Signed-off-by: moul <94029+moul@users.noreply.github.com> Co-authored-by: Leon Hudak <33522493+leohhhn@users.noreply.github.com>

leohhhn added 2 commits June 10, 2024 13:02

rm grc20 folder

68589d3

add interface

0e3bcf7

github-actions bot assigned leohhhn Jun 10, 2024

github-actions bot added the 🧾 package/realm Tag used for new Realms or Packages. label Jun 10, 2024

add initial implementation

6513052

leohhhn added 17 commits June 10, 2024 15:07

add update helper

cde3839

implement functions

eefe8a1

comments & transfer

9f5830c

mint & tests

283b82c

rename to .gno, fix valid transfer

3cb8085

add more tests

9ca19cc

remove leftover

2344a39

play around with pointer tests

17d2978

modify tests

6cc7ee2

gno mod tidy, fix imports

d653cd2

add docs to grc20, modify foo20

b98a9ce

fix errors, add faucet management

9780137

fix rendering

60d5197

fix errors & rendering for grc20factory

d17c1fc

fixing up wugnot

ae91731

tests for wugnot

1960279

add deposit & withdraw tests

f58010a

leohhhn marked this pull request as ready for review June 12, 2024 12:24

leohhhn requested review from a team as code owners June 12, 2024 12:24

leohhhn requested review from deelawn and mvertes and removed request for a team June 12, 2024 12:24

leohhhn and others added 2 commits June 13, 2024 15:00

Merge branch 'master' into refac/grc20

3e707c8

update eff gno

93ddcf8

tbruyelle approved these changes Jun 13, 2024

View reviewed changes

leohhhn and others added 2 commits June 13, 2024 19:49

update guide & playground link

5204628

Merge branch 'master' into refac/grc20

6437691

moul reviewed Jun 18, 2024

View reviewed changes

moul mentioned this pull request Jun 18, 2024

feat: add r/demo/bar20 example #2388

Merged

moul reviewed Jun 18, 2024

View reviewed changes

examples/gno.land/p/demo/grc/grc20/grc20.gno

panic(err)

}

t.totalSupply = sum

Copy link

Member

moul Jun 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't you also mint the tokens to the to address?

moul reviewed Jun 18, 2024

View reviewed changes

thinhnx-var mentioned this pull request Jun 20, 2024

feat(gnovm/tm2): add ZeroAddress constant #2401

Draft

7 tasks

Merge branch 'master' into refac/grc20

c48a2ba

michelleellen mentioned this pull request Jun 26, 2024

Minutes: Public Dev Calls [every Wednesday] gnolang/meetings#37

Open

moul mentioned this pull request Jul 7, 2024

feat(examples): refactor grc20 #2529

Merged

moul mentioned this pull request Jul 7, 2024

chore: update docs/' Makefile and CI #2534

Merged

moul marked this pull request as draft July 8, 2024 11:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: GRC20 #2314

refactor: GRC20 #2314

leohhhn commented Jun 10, 2024 •

edited

Loading

leohhhn commented Jun 10, 2024

leohhhn commented Jun 12, 2024 •

edited

Loading

tbruyelle left a comment

leohhhn commented Jun 17, 2024

moul Jun 18, 2024

moul Jun 18, 2024

moul Jun 18, 2024

moul Jun 18, 2024

moul Jun 18, 2024

moul Jun 18, 2024

moul Jun 18, 2024

moul Jun 18, 2024

moul Jun 18, 2024

moul Jun 18, 2024

moul Jun 18, 2024

moul Jun 18, 2024

moul Jun 18, 2024

moul Jun 18, 2024

moul commented Jul 7, 2024

	func NewGRC20Token(name, symbol string, decimals uint8) *Token {
	func NewToken(name, symbol string, decimals uint8) *Token {

	func (t *Token) Mint(account std.Address, amount uint64) {
	func (t *Banker) Mint(account std.Address, amount uint64) {

		o = ownable.New()
		o.TransferOwnership("g125em6arxsnj49vx35f0n0z34putv5ty3376fg5") // @admin

	return ufmt.Sprintf("%d\n", wugnot.BalanceOf(users.Resolve(owner)))
	balance := wugnot.BalanceOf(users.Resolve(owner))
	return ufmt.Sprintf("%d\n", balance)

	manfred := testutils.TestAddress("manfred") // g1wymu47drhr0kuq2098m792lytgtj2nyx77yrsm
	alice := testutils.TestAddress("alice") // xxx

refactor: GRC20 #2314

Are you sure you want to change the base?

refactor: GRC20 #2314

Conversation

leohhhn commented Jun 10, 2024 • edited Loading

Description

leohhhn commented Jun 10, 2024

leohhhn commented Jun 12, 2024 • edited Loading

tbruyelle left a comment

Choose a reason for hiding this comment

leohhhn commented Jun 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moul commented Jul 7, 2024

leohhhn commented Jun 10, 2024 •

edited

Loading

leohhhn commented Jun 12, 2024 •

edited

Loading