centrifuge-chain
centrifuge-chain copied to clipboard
Consider dropping `nft-sales` in favour of `pallet_uniques`
Motivation
Since this PR, the pallet_uniques
offers a set_price
and a buy_item
extrinsics to allow for NFT owners to set their NFTs up for sale for a given asking price and for potential buyers to buy said NFTs for said price. This functionality is exactly what we handcrafted with nft-sales
.
Proposal / Consideration
Since nft-sales
is essentially just duplicated logic, we can choose to adopt the official pallet_uniques
API instead.
Pros
- Less code to maintain
- One less pallet in our runtime
- Following community standards
- Simpler solution since users don't need to give up ownership of their NFTs to the
nft-sales
pallet to sell their NFTs
Migration requirements
- The frontend would need to switch to pulling NFTs for sale from the
pallet_uniques
- migration: active NFTs for sale on
nft-sales
would need to be removed and their ownership restored to the original owner
Questions
- How many active NFTs for sale on
nft-sales
do we have in Kusama?
@onnovisser it would be cool getting your thoughts on this regarding potential work that would need to happen in the frontend that we may not be considering now.
The surface area of NFT sales is pretty small, so shouldn't need much rework. Is there an extrinsic to remove an nft from being listed?
The surface area of NFT sales is pretty small, so shouldn't need much rework. Is there an extrinsic to remove an nft from being listed?
Well, we have a remove
extrinsic but that would require every user with NFTs up for sale to call that extrinsic. An alternative would be for us to write a migration script that would basically do that automatically on_runtime_upgrade
.
So we could have that and have the UI switch to adopt the pallet_uniques
API. Users who had NFTs open for sale would need to call basically putting them up for sale again, which would then happen through pallet_uniques
.
Wdyt?
That sounds good to me. With my question, I meant to ask if the new pallet_uniques
has an extrinsic to remove an item from sale, as users will need to still be able to do that in the new version, and it only mentions set_price
and buy_item
. Looking at the code though, it looks like I can call set_price
with None
to remove an item from sale.
Yes, you can remove the price by setting it to None
.
Btw, feel free to consider waiting a bit and switching to the new "uniques-v2" pallet that would bring much more features and control over nfts
Thanks for chipping in, @jsidorenko. @onnovisser let's wait for Uniques v2 then 👍