search

onflow / nft-storefront

A general-purpose Cadence contract for trading NFTs on Flow
The Unlicense
102 stars 53 forks source link

Improve Storefront Cleanup #33

Closed rheaplex closed 2 years ago

rheaplex commented 2 years ago

Where a storefront contains more than one listing for the same NFT, all listings should be removed if:

This should be a public ability, as the token owner may not be the storefront owner.

It is also important to make sure that all listings can be removed, even if the successfully completed (purchased) one is removed. If a listing exists in a storefront that the token owner does not control, and the owner of that storefront removes its public capabilities, they will not be able to do so.
OPEN QUESTION: does this justify providing access(contract) functions on storefronts to prevent this? Discuss with Dete...

Failure to remove all listings from all storefronts opens the token owner who created them to various attacks on the order of - https://finance.yahoo.com/news/cancel-old-opensea-listings-risk-163829354.html

I recommend two public functions in the StorefrontPublic interface. The first is an optimization. The second ensures that the first cannot be used to block removal of other listings by removing the completed listing.

pub fun cleanupIfSameToken(purchasedResourceID: UInt64, otherResourceIDs: [UInt64])
pub fun cleanupIfTokenMoved(listingResourceID: UInt64)
pub fun cleanupIfSameToken(purchasedResourceID: UInt64, otherResourceIDs: [UInt64])

This checks that the purchasedResourceID listing exists, and that it is purchased.

If not, it fails the precondition.

If so, it gets the token Type and id from it, then gets each listing in the otherResourceIDs list and removes it if it has the same Type and id. Finally, it removes the listing with purchasedResourceID.

Note that we need not specify all listing resource IDs within the storefront instance in otherResourceIDs. We therefore need a less efficient function to ensure that any listings that refer to an absent token can be removed.

pub fun cleanupIfTokenMoved(listingResourceID: UInt64)

This gets the listing and if its capability cannot be borrowed, removes it. If the capability resolves to something of the wrong type, it removes it. Finally, if the token of the specified Type and id cannot be borrowed via the capability, it removes the listing.

Note: an ideal solution to this would be link farms of Collection capabilities in each user's private storage, one per listing. I recommend revisiting this when Path wrangling supports it.

bjartek commented 2 years ago

I wanted to do an experiment the other day and try to see what happend when i listed the same item in rarible and at blocto bay. I was quite surprised when it turned out that i cannot. If i list something at rarible it is shown at blocto and I can then delist it at blocto.