substrate
substrate copied to clipboard
paritytech
[Uniques V2] Atomic NFTs swap
This PR adds a way to swap NFTs in an atomic way. Previously it was not possible to swap items without trusting some 3rd-party, this PR solves that. Additionally, it's possible to set the price for the desired NFT.
nit: I would probably name this functionality atomic_swap as opposed to swap. Simply because it's an established term and very self-explanatory. Not a blocker though.
@jsidorenko https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1863260 was started for your command "$PIPELINE_SCRIPTS_DIR/bench-bot.sh" pallet dev pallet_nfts. Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.
Comment /cmd cancel 38-c7c8a2bc-03ed-45ff-b075-4017759a0ab9 to cancel this command or /cmd cancel to cancel all commands in this pull request.
![command-bot[bot] avatar](https://avatars.githubusercontent.com/in/117604?v=4 "Published by a pub.dev verified publisher"){kind=small}
@jsidorenko Command "$PIPELINE_SCRIPTS_DIR/bench-bot.sh" pallet dev pallet_nfts has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1863260 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1863260/artifacts/download.
Could it otherwise not lead to a situation where both parties called
create_swapand the "matching" offers just sit in storage?
@ggwpez It could be a UI/UX concern though.
https://github.com/paritytech/substrate/pull/12285#discussion_r980016508 @muharem not sure what this refers to. But, I have just introduced a deposit for fast-unstake pallet, but it mostly has to do with security. Please feel free to share a link
I think this has a lots of potential. The only issue I have with the swap design is; how does a user know if he has to call
create_swaporclaim_swapto sell an item? It would be easier if there was just one function:swap.Could it otherwise not lead to a situation where both parties called
create_swapand the "matching" offers just sit in storage?
Hmm.. I was following the pattern we already implemented inside the atomic-swaps pallet. Additionally, as I see this is a pretty common pattern for NFT swaps on other chains. There is always room to improve things, I personally like the addition of the off-chain signatures to this process. Then you could start receiving different offers to swap your NFT and pick the best one. I would like to start from the classic implementation here and receive feedback from the ecosystem, otherwise, we would keep working on V2 features for ages :)
@ruseinov @ggwpez @muharem I've added a new price_direction param and addressed all the comments you put. Could you review this PR again pls?
@jsidorenko https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1902436 was started for your command "$PIPELINE_SCRIPTS_DIR/bench-bot.sh" pallet dev pallet_nfts. Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.
Comment /cmd cancel 106-0ebf84b6-fe87-46cb-811b-1af66386b630 to cancel this command or /cmd cancel to cancel all commands in this pull request.
@jsidorenko Command "$PIPELINE_SCRIPTS_DIR/bench-bot.sh" pallet dev pallet_nfts has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1902436 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1902436/artifacts/download.
Looks good.
I think the point about storage deposit and maximum duration still stands, since otherwise the user can just litter the chain with swaps. The storage deposits are meant to pay for used storage and protect the chain from being attacked by state bloat.
Yeah, I'll apply the max duration changes soon. Working on it now ;)