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

CIP-0102 | Reference Implementation #848

Merged
merged 25 commits into from
Jul 9, 2024

Conversation

SamDelaney
Copy link
Contributor

A reference implementation for CIP-0102 royalties, written in Lucid (offchain) and Aiken (onchain).

In this reference implementation you'll find examples of:

  • Minting a CIP 102 compliant NFT with royalties
  • Validators for storing royalty metadata
    • Always Fails (royalty cannot be modified)
    • Reducible (royalty cannot be increased, but can be decreased)
  • Reading a CIP 102 NFT’s royalties off chain
  • Reading and validating against CIP 102 NFT royalties on chain

This code has not been audited and is NOT INTENDED FOR PRODUCTION, but to provide code examples of expected behavior.

@SamDelaney
Copy link
Contributor Author

I still have some cleanup for clarity and consistency to be done here, and may dabble in some optimizations; however, it's now fully featured & ready for review by interested parties who don't mind digging through some sub-optimal code. Feedback is very welcome :)

I will mark as ready for merge when I'm satisfied with my sanitization.

@rphair rphair added the Category: Tokens Proposals belonging to the 'Tokens' category. label Jun 27, 2024
Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

thanks @SamDelaney for keeping this standard moving along & bringing your progress into the CIP process. 🙏

We have a long held standard of not including representative code in CIPs: this was an editorial decision made about 3 years ago & upheld in several instances since then. Beyond maintaining a proper focus on CIPs as documentation, mainly this is because any changes to the code should not be made subject to editorial approval or even involvement.

This matter has always been resolved by keeping CIP reference implementations in a different repository. I think that would work well for this case, and therefore that this PR should be redrafted to simply characterise your reference implementation (and of course updating your Path to Active) with links to that code repository as necessary.

If you would like to me to find links GitHub discussions where this process was mandated by consensus, please let me know and I'll try to dig them up. As with all of the CIP process this is always subject to review so @Ryun1 @Crypto2099 @KtorZ would also be welcome to confirm or contradict this recommendation.

@SamDelaney
Copy link
Contributor Author

@rphair I was not aware of that decision, thanks for letting me know! No links necessary, I trust you 😄

I'll move my code to another repo & update this PR accordingly.

@SamDelaney
Copy link
Contributor Author

Moved the code I had here to a new repo as requested. I can include my disclaimers about production readiness & general lack of polish in that repo, so I see no reason to delay this PR. Marking as ready for review & merge.

@SamDelaney SamDelaney marked this pull request as ready for review June 28, 2024 08:48
@rphair
Copy link
Collaborator

rphair commented Jun 28, 2024

@SamDelaney #848 (comment): @rphair I was not aware of that decision, thanks for letting me know! No links necessary, I trust you 😄

Thanks for your trust but I thought [recalling incorrectly] that one of the outcomes of that past deliberation was to update CIP-0001 (and now that I think about it, CIP-9999) with this reservation. I'll submit a change request in the near future which would have the additional advantage of the 3 currently active editors unanimously confirming both this policy & the language used to express it. cc @Crypto2099 @Ryun1

Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

Will be happy to approve this because it just updates the Path to Active along with the link to the completed reference implementation. It looks like it's getting very close to Active status now (but not there yet; please anyone correct me if this appears incorrect).

Will sign off on this once the empty file & directory are removed under ref_impl.

@SamDelaney
Copy link
Contributor Author

@rphair I think you may have misread the files changed. TODO.md was already included in the last PR so I'm just removing it here.

Here's the current state of the CIP-102 subdirectory, and here's what I'm updating it to with this PR.

Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

thanks @SamDelaney, I didn't misread the files changed... I just temporarily forgot how git works 🤪

@rphair rphair requested a review from Crypto2099 June 28, 2024 18:53
Copy link
Collaborator

@Ryun1 Ryun1 left a comment

Choose a reason for hiding this comment

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

LGTM!

thanks for the update @SamDelaney

@rphair rphair added the State: Last Check Review favourable with disputes resolved; staged for merging. label Jul 7, 2024
@rphair
Copy link
Collaborator

rphair commented Jul 7, 2024

Marked Last Check so we can merge at Tuesday's meeting (or before then) once @Crypto2099 can also give it a look: https://hackmd.io/@cip-editors/92

Copy link
Collaborator

@Crypto2099 Crypto2099 left a comment

Choose a reason for hiding this comment

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

These changes make sense and it's good to see the external links to the reference implementation so that can be iterated on asynchronously from the CIP Editors workflow.

@rphair
Copy link
Collaborator

rphair commented Jul 9, 2024

taking off agenda & merging then, since unanimous 🚀

@rphair rphair merged commit c41c2d6 into cardano-foundation:master Jul 9, 2024
@rphair rphair removed the State: Last Check Review favourable with disputes resolved; staged for merging. label Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Tokens Proposals belonging to the 'Tokens' category.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants