-
Notifications
You must be signed in to change notification settings - Fork 341
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: implement grc721 #414
Conversation
Add package `p/demo/grc/grc721` implementing igrc721.gno Signed-off-by: Hariom Verma <hariom.verma@tendermint.com>
Add tests for `p/demo/grc/grc721` methods. Signed-off-by: Hariom Verma <hariom.verma@tendermint.com>
6d052a7
to
dea211e
Compare
Signed-off-by: Hariom Verma <hariom.verma@tendermint.com>
Signed-off-by: Hariom Verma <hariom.verma@tendermint.com>
a78ff4b
to
2346689
Compare
Signed-off-by: Manfred Touron <94029+moul@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've left a few comments that should be addressed before we proceed with adding the GRC721 contract.
The main concern I have is that this contract is not modeled after ERC721 (NFT) contracts, but is more similar to ERC20. There is no way to specify a content URI, which is what NFTs exclusively are for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've resolved the remaining convos - for this demo implementation I think it's fine if the contract doesn't have URI support, as you've mentioned it's optional
Please just update this PR with the latest master changes, and we're good to go 🚀
Hey, this PR is ready, but since I merged #450, it will break. Please, can you update your code so it compiles with latest |
Hello @harry-hov, what an nice code!! Can it be replaced with |
Hey @r3v4-onbloc. It's not intentional. Just a blunder on my side. Since it's not used. It can be replaced with |
p/demo/grc/grc721
andr/demo/foo721