[move] NFT marketplace #921
Conversation
| /// Remove listing and get an NFT back. Only owner can do that. | ||
| public fun delist<T: store, C>( | ||
| market: &mut Marketplace, | ||
| listing: Listing<T, C>, |
There was a problem hiding this comment.
I am trying to understand how we are able to authenticate listing here. It is owned by Bag, but Bag is wrapped in Marketplace instead of being owned by Marketplace.
Let's assume these have the following object ID's
Marketplace: 0xA
Bag: 0xB
Listing: 0xC
Nft: 0xD
, I would expect the tx ownership chain to be
[Shared] -> 0xA -> 0xB -> 0xC -> 0xD
But the 0xA -> 0xB link can't exist if the Bag is wrapped in the marketplace instead of owned by it.
I actually don't know why this works in the tests--there might be a bug in TestScenario::remove_nested_object.
There was a problem hiding this comment.
Correct, in the implementation of remove_nested_object, there is no check on the ownership of the parent object, it just checks whether the child is owned by the parent.
I think this is probably the way to go, but I also want to understand why the approach of giving Bag |
| @@ -164,7 +164,9 @@ fn get_inventory_for( | |||
| // TODO: We should also be able to include objects indirectly owned by the | |||
| // requested address through owning other objects. | |||
| // https://github.com/MystenLabs/sui/issues/673 | |||
| if (obj.owner == Owner::AddressOwner(sui_addr) || obj.owner.is_shared()) | |||
| if (obj.owner == Owner::AddressOwner(sui_addr) | |||
| || obj.owner == Owner::ObjectOwner(sui_addr) | |||
There was a problem hiding this comment.
Thanks! This is a great first step, but it's not sufficient.
To properly remove a child object, we still need to know the address of the signer, and check that the parent object is actually owned by the signer.
There was a problem hiding this comment.
What would be the best approach to this? Passing TxContext to the get_inventory() method to determine signer and check privileges? I'm new to the backend part of it.
There was a problem hiding this comment.
Yeah I think we likely need to have another version of remove_unique_object that supports two signer addresses, one being the object ID of the parent object, one being the actual sender's address, for this to authenticate properly.
| /// Remove listing and get an NFT back. Only owner can do that. | ||
| public fun delist<T: store, C>( | ||
| market: &mut Marketplace, | ||
| listing: Listing<T, C>, |
There was a problem hiding this comment.
Correct, in the implementation of remove_nested_object, there is no check on the ownership of the parent object, it just checks whether the child is owned by the parent.
- made KITTY a ticker-like struct - arbitrary T with key+store instead of NFT - use GAS as a coin
Drafting a rough template of an NFT marketplace which makes use of shared objects and a Bag.
Two changes in Sui which I'm not sure about:
remove_nested_object, @lxfind if you have a chance, please check my solution to TestScenario::get_inventory() should support retrieving objects owned by objects #673.storeability to theBag.Crossroads
To continue with the topic raised on our last Sui/Move call, we could keep
transfer_..._unsafea friend-only thing if we allowed to store Bag, so that people would have built-in approach to storing large (or large enough) amounts of data.Alternatively, if we expose this API to public, then I can remove
storecapability from a Bag and implement bag-like functionality in Marketplace.I was also looking into storing
ChildRefto a bag in a Marketplace object (could possibly lead to Solidity-like discovery-objects) but haven't found any API to make use of ChildRef, so got nowhere.