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

Change ERC721 samples to use auto-indexing mint by default #109

Merged
merged 3 commits into from Jan 12, 2023

Conversation

awrichar
Copy link
Contributor

@awrichar awrichar commented Jan 9, 2023

With this change, the default "out of the box" sample for ERC721 will not require (or allow) you to pass a token index when minting. It will choose an increasing token index starting from 0.

Via ERC165 support, the connector can now check for 3 different versions of the ERC721WithData interface:

  • the current version (V2), which uses auto-indexing on mint and also supports all withData/withURI methods
  • the previous version (V1b), which requires explicit indexes, but supports all withData/withURI methods
  • the original version (V1a), which requires explicit indexes, and supports withData but not withURI

For the "no data" variant, we have no ERC165 support, so I have destructively replaced the ERC721NoData sample with a new one that uses auto-indexing. Note that it was just replaced in #104 and released as v1.2.0 - I don't really see any need to preserve all the versions of this sample, so I'm just preserving this new one and the original (not the interim one introduced in v1.2.0). This new PR will probably need to become release v1.2.1, and we'll just want to note that the sample in v1.2.0 is not supported.

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
@awrichar awrichar merged commit cc41532 into hyperledger:main Jan 12, 2023
@awrichar awrichar deleted the autoindex branch January 12, 2023 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants