Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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? | Royalty Datum Metadata #551
CIP-0102? | Royalty Datum Metadata #551
Changes from all commits
1b8654f
461e207
a05fa4a
860a77a
1fb1f14
f198e2a
693adac
1709808
6409e4c
304dcb0
6893503
fce05fc
0b9a955
34089a1
357202f
713aad5
0041a70
c51bc4a
506447d
7725aaf
6d897eb
4ecab23
2e7f21f
d77752c
ecf6b28
1a177b4
f28f8c2
38a3146
9480534
75eb324
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Is an address truly needed here or could a payment credential (i.e. a vk or script hash) suffices? The address also contains a network discriminant and, a possible delegation payload. If there's a desire to handle delegation payload then, how will pointer addresses be handled?
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.
Yes there's a delegation consideration, as you figured. I'm not sure I understand the concern with pointer addresses, can you elaborate?
Another consideration is consistency with the existing implementations. I want to break things as little as possible here. At the moment no changes are required to Nebula, at least. I'm less sure of closed source implementations, for obvious reasons.
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.
Are these the same pointer addresses that were determined to be obsolescent here? #374 (comment)
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.
Suggestion, bundle the version separately, so that the format can be adjusted if needed without prior knowledge of the version. For example:
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.
This is a very interesting idea. If I understand you correctly, this would be helpful for parsing the datum as a third party once multiple versions are introduced. One could parse the version number first, then know what type to expect for the royalty info itself. Is that what you're saying?
I like the idea, but I'm not sure how much of a difference it would actually make. Do you have a good idea of how much more efficient a change like this would be?
My only real concern is compatibility with existing implementations. I'm not sure this provides enough value to warrant making Nebula & other nebula-based implementations incompatible.
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.
What about using
#6.121[] / #6.122[]
(False / True) instead ?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.
This was my initial idea, and it would certainly work for v1. I have an idea for v2 to make use of this field to allow for multiple royalty datums to be attached to a policy, with the correct datum to be used for each individual NFT specified in this field.
With that plan in mind, I decided to make the type
big_int
instead, so the type expected by third parties doesn't have to change in future versions. Since computational complexity of Plutus primitives scale with size, performance should be effectively identical.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.
👍