Add Ghost listing functionality

[satyamakgec]

• Add functionality to allow to remove of the ghost listing from storefront cleanupGhostListings.
• Add transaction to execute cleanupGhostListings.
• Add script to get the unique ghost listing for a given storefront.
• Add test cases.

[chriswayoub]

Should the same functionality being added to NFTStorefront V1 be a part of this PR as well?

[bluesign]

I don’t understand this, @satyamakgec can you explain a bit the use case?

[chriswayoub]

@bluesign I don't want to speak for @satyamakgec, but I would use this functionality to clean up listings that have become invalid due to purchase on another marketplace or some other transfer of the NFT away from the account that created the listing. There are currently a lot of "ghost" listings out there that will become valid again if the original account ever regains custody of the NFT. I think being able to "clean up" another user's listing that cannot currently be fulfilled is a necessary evil to prevent the worse situation of a person unknowingly selling an item for a price they maybe created years in the past.

This is also an issue with V1 of the storefront contract, so I would think to either make it part of this PR (or a separate PR if that's preferred) which I'm happy to do if it's not already in-progress.

[satyamakgec]

I don’t understand this, @satyamakgec can you explain a bit the use case?

cc @bluesign

Yes sure, as @chriswayoub explained. This PR tries to introduce a helper function to remove the ghost listings by anyone (In an Ideal world, those listings would be removed by the marketplaces).

Now the question arises how to find the ghost listing programmatically? So as per the definition of ghost listing, if the NFT doesn't exist in the provided Provider capability, then that listing becomes a ghost listing. The very same thing the cleanupGhostListings function is doing. It finds out whether the NFT exists or not if yes, then it removes the listing.

This functionality would be helpful for the marketplaces to clean up the garbage listings, and sellers would also not get shocked when NFT get removed from their account unknowingly because of ghost listing.

Let me know if anything doesn't make sense.

[satyamakgec]

Should the same functionality being added to NFTStorefront V1 be a part of this PR as well?

Thanks @chriswayoub for raising this, I am not sure whether we are going to introduce the same thing in the NFTStorefrontV1 or not because we are trying our best to move the ecosystem to the NFTStorefrontV2 contract. If you are willing to add that functionality in the NFTStorefront then I am happy to review the PR and upgrade the NFTStorefront contract on mainnet.

[bluesign]

I don't understand the use case for ghost listing removal. Isn't NFTStorefrontV2 based on user has the storefront and marketplace manage via events ? I understand that listing can stay in user account, but when marketplace will go to clean up?

[austinkline]

I don't understand the use case for ghost listing removal. Isn't NFTStorefrontV2 based on user has the storefront and marketplace manage via events ? I understand that listing can stay in user account, but when marketplace will go to clean up?

The use case is that a user might list their NFT for sale on two marketplaces at once to get more coverage on potential buyers. But what happens if they:

1. Sell on one of them
2. Transfer their nft elsewhere

In both cases, you have listings on marketplaces that are valid, and, should you ever find yourself in possession of that nft again, you would find yourself at risk of losing the NFT for a price that is (presumably) much lower than it is now worth. This happened a lot to users on OpenSea about a year ago, they had made listings that never expired and then in an effort to save gas on cancellations, moved their nfts to another wallet instead of cancelling. Once the NFT came back to their original wallet, it could get sniped for much lower than the floor price

[bluesign]

@austinkline I know that case, but what I dont get here, when will marketplace call user’s cleanlisting function? It will go periodically to every user?

[satyamakgec]

@austinkline I know that case, but what I don't get here, is when will the marketplace call the user’s cleanlisting function? It will go periodically to every user?

There can be multiple ways of doing so.

1- Before showing any listing on the listing dashboard marketplaces can check whether the listing is a ghost listing, If yes then either delete it or raise a notification to the listing creator account whenever it visits they can delete it by themselves.

2- Marketplaces can check periodically whether there is a periodic ghost listing or not if yes then delete it.

[satyamakgec]

Glad that this is being thought about. I'd stress that ghost listings aren't the only issue, and that you actually should just be thinking about how listings can be made invalid in general and let all listings that fall under this category be cleaned up.

For example, what if my payment receivers are broken?

Ghost listing can be fatal for the listing creator while other scenarios wouldn't be as fatal as ghost listing so we tried to resolve it first , Other scenarios maybe taken care of afterwards.

[bluesign]

@austinkline I know that case, but what I don't get here, is when will the marketplace call the user’s cleanlisting function? It will go periodically to every user?

There can be multiple ways of doing so.

1- Before showing any listing on the listing dashboard marketplaces can check whether the listing is a ghost listing, If yes then either delete it or raise a notification to the listing creator account whenever it visits they can delete it by themselves.

2- Marketplaces can check periodically whether there is a periodic ghost listing or not if yes then delete it.

@satyamakgec

for this, I think we need a 'hasGhostListings' or 'hasGhostlisting(listingid)' function, so marketplace can check with script and then run transaction if there is ghost

but more importantly problem is not ghost listing usually, it is ghost listing staying active.

why we don't add a field, like 'ghosted'. and set to true, and generate listingcompleted event if purchase fails for not having nft? and later clean ghosted can also clean those. this way it doesnt depend on marketplaces, it is crowdsourced.

if i have nft moved and listed cheap, as @austinkline said, it will be for sure tried to be purchased before i put it back, then listing would be already deactivated.

easier for marketplaces, easier and safer for users imo.

[alliu930410]

Should the same functionality being added to NFTStorefront V1 be a part of this PR as well?

Thanks @chriswayoub for raising this, I am not sure whether we are going to introduce the same thing in the NFTStorefrontV1 or not because we are trying our best to move the ecosystem to the NFTStorefrontV2 contract. If you are willing to add that functionality in the NFTStorefront then I am happy to review the PR and upgrade the NFTStorefront contract on mainnet.

My statement is not tech-related.

The old NFTStorefront needs to be updated to avoid potential exploit of the ecosystem when the same NFT is listed in multiple marketplaces, as this is an official storefront contract repo, they need to be non-exploitable for the broader ecosystem to use directly at least 🙏

The users are not aware their assets are in jeopardy when using multiple marketplaces right now.

Could we either prepare for discontinuing of the V1 contract for the ecosystem or make projects able to remove the listings on that contract? (e.g. I can't remove Gaia listings to protect our users as they are on the V1 contract)

[satyamakgec]

@austinkline I know that case, but what I don't get here, is when will the marketplace call the user’s cleanlisting function? It will go periodically to every user?

There can be multiple ways of doing so. 1- Before showing any listing on the listing dashboard marketplaces can check whether the listing is a ghost listing, If yes then either delete it or raise a notification to the listing creator account whenever it visits they can delete it by themselves.

2- Marketplaces can check periodically whether there is a periodic ghost listing or not if yes then delete it.

@satyamakgec

for this, I think we need a 'hasGhostListings' or 'hasGhostlisting(listingid)' function, so marketplace can check with script and then run transaction if there is ghost

but more importantly problem is not ghost listing usually, it is ghost listing staying active.

why we don't add a field, like 'ghosted'. and set to true, and generate listingcompleted event if purchase fails for not having nft? and later clean ghosted can also clean those. this way it doesnt depend on marketplaces, it is crowdsourced.

if i have nft moved and listed cheap, as @austinkline said, it will be for sure tried to be purchased before i put it back, then listing would be already deactivated.

easier for marketplaces, easier and safer for users imo.

Thanks @bluesign for the feedback, We do have the function hasNFTPresentInListingProvider to query whether the listing is ghost listing or not. I think I may need to change the name to make it clearer. And I don't understand why adding a flag is better then just removing the listing altogether. As you said anyone can set the flag, In a similar manner anyone can delete the ghost listing.

Anyway, even for setting the flag, someone has to query and find out the ghost listing and instead of deleting at the same time why do they prefer to just set the flag and delete the listing in another call? Or did I misunderstood what you are trying to say here.

[satyamakgec]

Should the same functionality being added to NFTStorefront V1 be a part of this PR as well?

Thanks @chriswayoub for raising this, I am not sure whether we are going to introduce the same thing in the NFTStorefrontV1 or not because we are trying our best to move the ecosystem to the NFTStorefrontV2 contract. If you are willing to add that functionality in the NFTStorefront then I am happy to review the PR and upgrade the NFTStorefront contract on mainnet.

My statement is not tech-related.

The old NFTStorefront needs to be updated to avoid potential exploit of the ecosystem when the same NFT is listed in multiple marketplaces, as this is an official storefront contract repo, they need to be non-exploitable for the broader ecosystem to use directly at least 🙏

The users are not aware their assets are in jeopardy when using multiple marketplaces right now.

Could we either prepare for discontinuing of the V1 contract for the ecosystem or make projects able to remove the listings on that contract? (e.g. I can't remove Gaia listings to protect our users as they are on the V1 contract)

My high hopes are to discontinue the V1 contract and we want flow community to adopt the V2 contract at its fullest. We are soon going to publish migration transactions as well that can help the community to move from v1 contract to v2 contract. Although I am open to adding this change in the v1 contract if you or someone from the community proposes a PR or maybe once we merge this PR, I will create the same change for V1 contract as it is a low priority for us.

[chriswayoub]

If the plan is to discontinue the V1 contract, then I can agree with not patching it depending on the timeline for that happening. But if it's going to be around for any significant amount of time, then I think it's a liability to leave it as-is, and it's only a matter of time before it becomes a problem that gets visibility.

[bymi15]

Hi @satyamakgec, Is there an ETA on when the changes in this PR will get deployed to testnet/mainnet NFTStorefrontV2 contract?

[satyamakgec]

Hi @satyamakgec, Is there an ETA on when the changes in this PR will get deployed to testnet/mainnet NFTStorefrontV2 contract?

Most probably these changes would be live next week.