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

CHIP-0008: Splitter Puzzle #30

Merged
merged 15 commits into from
Jan 30, 2023
Merged

CHIP-0008: Splitter Puzzle #30

merged 15 commits into from
Jan 30, 2023

Conversation

greimela
Copy link
Contributor

@greimela greimela commented Sep 5, 2022

This proposal aims to define a standard for splitting royalties for NFT1-compliant NFTs, using a standard Chialisp puzzle to prevent marketplace lock-in.

The implementation of the Chialisp is already included, but it has not been audited by any third party yet.

Feedback is very welcome!

Copy link
Contributor

@BrandtH22 BrandtH22 left a comment

Choose a reason for hiding this comment

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

I might be missing these components but I am not seeing where:

  1. Excess mojo are allocated (if the split leaves change what happens to the remainder?)
  2. CATs are being handled (likely needs to be done in a driver program and handled per CAT or via a connected API)

Here is another version of this royalty split that takes both of the above into account.
This version is undergoing final testing but is close to production ready:
https://github.com/trgarrett/chialisp/tree/main/royalty_share/clsp/p2_royalty_share_allocated_percentages_feeless

@greimela
Copy link
Contributor Author

greimela commented Sep 5, 2022

Thank you for your review!

Regarding 1:
It is left to the farmer. The proposal states

If the created coins do not use up the full coin amount, the rest is left for the farmer of the block. This difference is most likely very small and not worth the effort to construct a more complex puzzle to collect this rest.

Regarding 2:
That is a good point, I will provide a unit test to show how the CAT spend would look like.

Also, thanks for pointing out the other royalty puzzle!
I knew there was one out there, but I couldn't find it again...

@trgarrett, I would love to incorporate your learnings into this proposal!

@L4bb3rs
Copy link

L4bb3rs commented Sep 5, 2022

Been playing with Tgarretts and discussing. Some very good work has been done!

@trgarrett
Copy link

CATs were the biggest challenge I ran into with my puzzle. Unlike XCH which smoothly flowed to the farmer (other than one potential opportunistic attack due to not using a 52 RESERVE_FEE condition), excess CAT mojos would always break the CAT spend accounting and make the whole coin held by the royalty puzzle unspendable.

Also, because the puzzle address changes with CATs, I had to code in the set of CATs I was willing to support. An API integration or CSV import would work just as well, but would progressively slow the spending down if you had a significant number of them you weren't actually needing to spend. I went this way rather than relying on address hints because, although I was told they were a best practice, they didn't seem to be used consistently in all wallets and reference documentation and I wanted to avoid unspendable (unfindable) coins.

@trgarrett
Copy link

One other point of feedback that I saw on Twitter, that I had also wondered about was whether this was fully in keeping with the spirit of CHIPs. I do think it would be a great feature to add to one or more Chia clients, but I wouldn't want all the clients/wallets to have an enormous set of features they all had to support in order to be "real" Chia wallets.

From a user interface perspective in the Chia wallet, it would be fairly straightforward to have the mint take parameters like -ra{n}:{address n} -rp{n}basis points and have the royalty puzzle built on the fly. I think the very first feature request you would see if Chia went down that road would be to have a singleton fronting the royalty puzzle to allow the minter to redefine the allocations over time. You would likely end up with all sorts of weird business requirements for that around multisigs, change of ownership, voting on membership allocations, etc., which is where it really starts to sound like something the Chia standards might not want to concern themselves with.

@greimela
Copy link
Contributor Author

greimela commented Sep 7, 2022

CATs were the biggest challenge I ran into with my puzzle. Unlike XCH which smoothly flowed to the farmer (other than one potential opportunistic attack due to not using a 52 RESERVE_FEE condition), excess CAT mojos would always break the CAT spend accounting and make the whole coin held by the royalty puzzle unspendable.

Good point, I didn't consider that CATs can not be left for the farmer.
So I guess we have to determine the amount left and send it to someone.
I saw your solution with the tip jar, but I think I'd prefer it to go to some address in the list, to simplify things.
The tip jar is a nice idea, but the value will most likely be very small, so I don't think it's work the effort.

Also, because the puzzle address changes with CATs, I had to code in the set of CATs I was willing to support. An API integration or CSV import would work just as well, but would progressively slow the spending down if you had a significant number of them you weren't actually needing to spend. I went this way rather than relying on address hints because, although I was told they were a best practice, they didn't seem to be used consistently in all wallets and reference documentation and I wanted to avoid unspendable (unfindable) coins.

I only considered the case of NFT royalties, since it seems to be the most popular use case for split puzzles.
NFT generally only pay out royalties when using offers.
In this case, it is actually pretty easy to determine the CAT TAILs.
You just uncurry all the relevant coins of an offer and it will give you all the TAILS involved.

This would not detect the case where somebody wants to send funds to the split puzzle as a transfer, without an offer being involved.
But I would consider that a different use case, which happens to use the same puzzle.

One other point of feedback that I saw on Twitter, that I had also wondered about was whether this was fully in keeping with the spirit of CHIPs. I do think it would be a great feature to add to one or more Chia clients, but I wouldn't want all the clients/wallets to have an enormous set of features they all had to support in order to be "real" Chia wallets.

I can see that point.
As an alternative to the wallet, the split could also be executed by a minting tool provider or some other player in the ecosystem.
And since the puzzle is open source, it can also be done manually by using an open-source driver like you have provided.

But I don't think the manual split is a reasonable approach for a typical end user.
Some user of the Goby wallet will not know how to set up a Python routine on a server to split her royalties.

@trgarrett
Copy link

I saw your solution with the tip jar, but I think I'd prefer it to go to some address in the list, to simplify things.

That’s reasonable. I thought of that. I couldn’t think of a fair way to do that which also minimized computation (fake/derived randomness) so I landed on the tip jar. You could just as well always give it to the highest or lowest earner, I suppose.

CHIPs/chip-XXXX.md Outdated Show resolved Hide resolved
CHIPs/chip-XXXX.md Outdated Show resolved Hide resolved
CHIPs/chip-XXXX.md Outdated Show resolved Hide resolved
Copy link

@CommanderMoto CommanderMoto left a comment

Choose a reason for hiding this comment

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

Why do all the shares need to add up to 100? Why not just add up the total number of "shares" in the royalty scheme, then apportion payouts based on that denominator?

CHIPs/chip-XXXX.md Outdated Show resolved Hide resolved
CHIPs/chip-XXXX.md Outdated Show resolved Hide resolved
@greimela
Copy link
Contributor Author

Thanks for the feedback @CommanderMoto @Rigidity!
I have updated the puzzle to derive the number of shares by summing them up.
This should reduce the error potential quite a bit and didn't add a lot of complexity to the puzzle.

@danieljperry danieljperry changed the title NFT Royalty Splitting CHIP-0008: NFT Royalty Splitting Sep 19, 2022
Signed-off-by: danieljperry <[email protected]>
CHIPs/chip-0008.md Outdated Show resolved Hide resolved
CHIPs/chip-0008.md Outdated Show resolved Hide resolved
@trgarrett
Copy link

Am I right to think this puzzle doesn't include memos/hints for the wallet? I was experimenting with the puzzle, and I think I got a send to go through. The wallet didn't recognize it in my transaction history though.

@trgarrett
Copy link

I did some further testing and would recommend including the memo for best results. Shown here: https://github.com/trgarrett/chialisp/pull/3/files/a38dbe55e892203acd8001df974939e74d77f225#diff-6e7343f80d63a39868837db9e25ac65bd69f332b56b24cfef7e18f34c19d9613R39

@greimela greimela changed the title CHIP-0008: NFT Royalty Splitting CHIP-0008: Splitter Puzzle Dec 6, 2022
@greimela
Copy link
Contributor Author

greimela commented Dec 6, 2022

@trgarrett I have update the Chialisp in this proposal to represent our latest changes, including the memo.

@greimela
Copy link
Contributor Author

greimela commented Dec 6, 2022

@danieljperry From my perspective, we are at a point where the proposal can be considered complete.

hoffmang9
hoffmang9 previously approved these changes Jan 10, 2023
Copy link
Member

@hoffmang9 hoffmang9 left a comment

Choose a reason for hiding this comment

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

lgtm

@danieljperry
Copy link
Contributor

This CHIP has been reviewed internally at CNI and is now being moved to Last Call. This is your last chance to request any changes. If no changes need to be made to the CHIP over the next two weeks, it will be set to Final.

Copy link
Contributor

@danieljperry danieljperry left a comment

Choose a reason for hiding this comment

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

This CHIP is now final. No further changes are allowed, other than errata. Thanks for submitting it, @greimela!

@wallentx wallentx merged commit f5bd8ff into Chia-Network:main Jan 30, 2023
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

Successfully merging this pull request may close these issues.

10 participants