onflow / nft-storefront

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

ListingAvailable event needs some 'filter-friendly' properties #27

Open ph0ph0 opened 2 years ago

ph0ph0 commented 2 years ago

Currently, ListingAvailable event takes the following params:

We listen for events, and these are used to populate the marketplace. NFTStorefront contract was designed to be a general marketplace contract, where people could use it to list their NFTs on multiple markets.

However, I think that the ListingAvailable event doesn't contain any information that would allow event listeners, and by extension, market makers, to target specific Listings only (and therefore specific NFTs). Maybe I don't want loads of irrelevant NFTs listed via NFTStorefront contract in my marketplace?

For example, let's say that I want to create a collection of NFTs, called FlowCats, and I want my buyers from the primary market (the minting of the NFTs) to be able to sell their FlowCats on the secondary market (the marketplace). I create a 'marketplace' page on my web app, and I want to allow my users to go on this page and list their NFTs for sale, and I use the NFTStorefront contract to achieve this.

The problem with the NFTStorefront contract is that there is no way to effectively filter the ListingAvailable events, so that I only listen for those associated with the listing of FlowCats. You could deploy your own custom FlowCatNFT contract, where the type of the NFT would be FlowCatNFT.NFT, which would allow you to then filter on the nftType property of the ListingAvailable event. However, I don't think that this fully solves the problem, because there are generic NFT contracts deployed to mainnet that allow you to save an NFTMinter to your account, and mint NFTs using that resource (see TatumMultiNFT contract as an example, they provide the nftMinter resource via an API). Now if I use this for my FlowCat NFTs, the type will be TatumMultiNFT.NFT, and I can no longer extract only the FlowCat ListingAvailable events from the blockchain.

I came across this issue because I wanted to create a 'no-deploy' NFT project where I used the TatumMultiNFT contract and NFTStorefront, which are both already deployed to mainnet.

I have come up with some (rubbish) ways of solving this, I've only included them to get people's cogs turning:

Food for thought. Looking forward to hearing people's thoughts on this. Thanks!

justjoolz commented 2 years ago

Perhaps not the greatest solution, but could you simply store the ids of the NFTs as you mint via Tatum and filter for those ids specifically?

Could be cumbersome with larger numbers of nfts but in that case you may be better off deploying your own contract?

ph0ph0 commented 2 years ago

That's definitely a solution that I hadn't thought of, storing the ids of the NFTs as you mint them, thanks for the suggestions!

Yes, Bjartek suggested deploying my own NFTStorefront contract because then the event listening could be targeted to the account holding that contract. I wanted a 'no-deploy' solution as we had a tight deadline and so I didn't want to go through the mainnet audit process. However, auditing would likely be quicker if I copy/paste the NFTStorefront contract as it has already been audited, so it is a potential solution for my use-case.

Maybe I'm just being a bit nit-picky, but I do feel as though some way of allowing more effective event filtering would be beneficial for the NFTStorefront contract and its intended general applicability, but I appreciate that it isn't easy to implement effectively.