feat: clean GRC721 implementation #2117

leohhhn · 2024-05-15T20:45:57Z

Description

Ported from here for visibility.

This is an experiment involving a refactor of the GRC721 package. The idea with this is that we might want a fully "by-spec" implementation, that people who know the standard can use out of the box.

The process for writing this was the following:

Wrote the implementation in Go
Ported the implementation to Gno
Wrote an exampleNFT app using the package

This PR replaces the old GRC721 implementation with the new one, and updates the foo721 example realm, and removes the nft example realm to only have a single example implementation.

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.

irreverentsimplicity · 2024-05-21T14:37:13Z

LGTM to. As far as I'm concerned, there are no breaking changes. It's nice to have an implementation following the standard ERCs. I will update Flippando to use this realm once the PR is merged.

irreverentsimplicity

The only major thing is the addition of the external Ownable realm, but it's not a breaking change. LGTM.

linhpn99 · 2024-06-06T02:13:25Z

after this PR is merged, i will update NFT's extensions realms @leohhhn

## Description This PR removes the outdated (and full of mistakes) guide from the docs. After [this PR](#2117) is merged, we can add a create a new how-to guide on how to create a GRC721 token. I've added [an issue](#2288) about this. <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 - [ ] Provided any useful hints for running manual tests - [ ] Added new benchmarks to [generated graphs](https://gnoland.github.io/benchmarks), if any. More info [here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md). </details>

moul · 2024-06-07T11:47:36Z

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

+	owners            *avl.Tree // tokenID > std.Address
+	balances          *avl.Tree // std.Address > # of owned tokens
+	tokenApprovals    *avl.Tree // tokenID > std.Address
+	operatorApprovals *avl.Tree // "OwnerAddress:OperatorAddress" -> bool
+	tokenURIs         *avl.Tree // tokenID > URI


since you've at least 3 elements sharing the same key, I suggest having a single avl.Tree but returning an object.

moul · 2024-06-07T11:49:05Z

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

+		nft.getApproved(tokenId) == owner
+}
+
+func (nft Token) update(to std.Address, tokenId string, auth std.Address) std.Address {


Suggested change

func (nft Token) update(to std.Address, tokenId string, auth std.Address) std.Address {

func (nft *Token) update(to std.Address, tokenId string, auth std.Address) std.Address {

was the code currently working?
because it shouldn't.

It seems that this was working mainly because I use pointers to the AVL storage. Tests were also passing. I'll see to either use non-pointers for AVL trees or just add pointer receivers.

Ended up just making the function take receiver pointers, as avl.NewTree returns a pointer.

moul · 2024-06-07T11:49:55Z

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

+		}
+	}
+
+	std.Emit("Transfer", "from", from.String(), "to", to.String(), "tokenID", tokenId)


Suggested change

std.Emit("Transfer", "from", from.String(), "to", to.String(), "tokenID", tokenId)

std.Emit("Transfer",

"from", from.String(),

"to", to.String(),

"tokenID", tokenId,

)

I suggest this format for emitted events, until we can have typesafe ones.

# Conflicts: # examples/gno.land/r/demo/nft/README.md # examples/gno.land/r/demo/nft/z_0_filetest.gno # examples/gno.land/r/demo/nft/z_1_filetest.gno # examples/gno.land/r/demo/nft/z_2_filetest.gno # examples/gno.land/r/demo/nft/z_3_filetest.gno # examples/gno.land/r/demo/nft/z_4_filetest.gno

dongwon8247 · 2024-10-25T04:15:29Z

It this PR still relevant? @jinoosss is working on supporting GRC721s in Adena & GnoScan. Let us know the status of the PR, thanks!

leohhhn · 2024-11-05T10:10:49Z

Hey @dongwon8247, sorry for the late reply.

Any package we make for the GRC721 standard will have the same API. I think going forward with this in mind should be the way, and it will especially show us if we're maintaining this standard properly.

I think this PR will be closed, as we're discovering some new ways of working with GRCs (specifically grc20, ie here)

copy over from hackerspace

375d1cf

leohhhn requested review from a team as code owners May 15, 2024 20:45

leohhhn requested review from jaekwon and piux2 and removed request for a team May 15, 2024 20:45

github-actions bot assigned leohhhn May 15, 2024

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

leohhhn requested review from irreverentsimplicity and n0izn0iz May 15, 2024 20:46

leohhhn mentioned this pull request May 16, 2024

x: grc721 full refactor gnolang/hackerspace#63

Closed

remove unused test

7d87461

irreverentsimplicity reviewed May 21, 2024

View reviewed changes

This was referenced Jun 5, 2024

[docs] Add GRC721 how-to guide #2288

Open

docs(GRC721): remove outdated guide #2289

Merged

leohhhn and others added 2 commits June 5, 2024 17:26

Merge branch 'master' into feat/grc7211-by-spec

7454c42

remove old nft, add new

ba52830

leohhhn changed the title ~~x: clean GRC721 implementation~~ feat: clean GRC721 implementation Jun 5, 2024

Merge branch 'master' into feat/grc7211-by-spec

5d364be

leohhhn requested a review from moul June 5, 2024 16:46

moul reviewed Jun 7, 2024

View reviewed changes

leohhhn added 4 commits June 10, 2024 18:36

typo

0da8c99

format events

a3c33fa

collection

742d6d5

leohhhn and others added 3 commits June 25, 2024 12:25

unify AVL tree

1525fbb

add basetokenURI

15b7b71

Merge branch 'master' into feat/grc7211-by-spec

f02f3de

leohhhn marked this pull request as draft November 15, 2024 02:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: clean GRC721 implementation #2117

feat: clean GRC721 implementation #2117

leohhhn commented May 15, 2024 •

edited

Loading

irreverentsimplicity commented May 21, 2024 •

edited

Loading

irreverentsimplicity left a comment

linhpn99 commented Jun 6, 2024

moul Jun 7, 2024

moul Jun 7, 2024

leohhhn Jun 12, 2024

leohhhn Jun 17, 2024 •

edited

Loading

moul Jun 7, 2024

leohhhn Jun 25, 2024

dongwon8247 commented Oct 25, 2024

leohhhn commented Nov 5, 2024 •

edited

Loading

	func (nft Token) update(to std.Address, tokenId string, auth std.Address) std.Address {
	func (nft *Token) update(to std.Address, tokenId string, auth std.Address) std.Address {

feat: clean GRC721 implementation #2117

Are you sure you want to change the base?

feat: clean GRC721 implementation #2117

Conversation

leohhhn commented May 15, 2024 • edited Loading

Description

irreverentsimplicity commented May 21, 2024 • edited Loading

irreverentsimplicity left a comment

Choose a reason for hiding this comment

linhpn99 commented Jun 6, 2024

moul Jun 7, 2024

Choose a reason for hiding this comment

moul Jun 7, 2024

Choose a reason for hiding this comment

leohhhn Jun 12, 2024

Choose a reason for hiding this comment

leohhhn Jun 17, 2024 • edited Loading

Choose a reason for hiding this comment

moul Jun 7, 2024

Choose a reason for hiding this comment

leohhhn Jun 25, 2024

Choose a reason for hiding this comment

dongwon8247 commented Oct 25, 2024

leohhhn commented Nov 5, 2024 • edited Loading

leohhhn commented May 15, 2024 •

edited

Loading

irreverentsimplicity commented May 21, 2024 •

edited

Loading

leohhhn Jun 17, 2024 •

edited

Loading

leohhhn commented Nov 5, 2024 •

edited

Loading