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

ERC721 Standard #151

Closed
dekz opened this issue Mar 2, 2018 · 5 comments
Closed

ERC721 Standard #151

dekz opened this issue Mar 2, 2018 · 5 comments

Comments

@dekz
Copy link

dekz commented Mar 2, 2018

Hey @enlight

These lessons are absolutely awesome!

I'd just like to talk about lesson number 5 or the one on NFT (aka ethereum/EIPs#721). The implementation in cryptozombies is influenced by CryptoKitties for obvious reasons, but it is not reflective of the 721 current draft going through the EIP process. I am more than happy to work with you or someone on your team to get lesson 5 to be compliant with the proposed 721 draft. IChanges include things like no takeOwnership and the addition of transferFrom and approvals to be more ERC20 like.

I see your Note that this is a proposal and you're using what is in OpenZeppelin currently. OpenZeppelin are also about to change that interface to be the new proposal.

@dekz dekz changed the title ERC721 Standard ERC721 Standard (proposal) Mar 2, 2018
@jamesmartinduffy
Copy link
Contributor

Hi @dekz, thanks for the kind words!

I looked at the EIP when writing the lesson, but it seemed like things were still being changed on a daily basis and it wasn't close enough to being a standard to base the lesson on. (E.g. A week ago the draft had tokens being referred to as "deeds", which I felt was a confusing word choice and would likely change by the final release anyway). Thus I chose to base the implementation on OpenZeppelin's contract.

I'm totally in favor of updating the lesson to be compliant with the standard, but would prefer to wait until it's an accepted standard rather than in draft form. That way we don't need to update the lesson content multiple times if it changes again before being finalized, if that makes sense.

Please feel free to keep us updated on the progress, and when it's far enough along we can update the lesson. Awesome work you guys are doing!

@dekz
Copy link
Author

dekz commented Mar 3, 2018

Sounds good to me, you're definitely correct, it was moving quite quickly! I'll ping you back when its moved forward with an OpenZeppelin example!

@dekz dekz changed the title ERC721 Standard (proposal) ERC721 Standard Mar 14, 2018
@dekz
Copy link
Author

dekz commented May 9, 2018

hey @jamesmartinduffy Just checking in on this again, any updates on changing the lesson to match the interface of the ERC721? I think having it right now as the very old draft is quite misleading to new developers.

OpenZeppelin have also merged their PR as can be seen here

@andreipope
Copy link
Collaborator

hey @dekz We are updating the lessons to match the ERC721 interface. We'll keep you updated on the progress. Thanks!

@jlubeck
Copy link

jlubeck commented Aug 11, 2021

Has this been updated? Just want to make sure since I'm new to this and don't want to start off the wrong foot... thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants