onflow / nft-storefront

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

Add `nftID` [and `nftType`] to `ListingCompleted` event #22

Closed nvdtf closed 2 years ago

nvdtf commented 2 years ago

Most marketplaces need to handle stale/ghost listings to provide a good UX (see discussion https://github.com/onflow/nft-storefront/issues/21). They need to remove the corresponding listings when an NFT is sold/removed through other apps. One solution for the marketplace app is to listen for ListingCompleted events and remove the listings for the corresponding NFTs in the DB/UI. The current ListingCompleted event has minimal data while ListingAvailable provides the needed nftID [and nftType] fields, so the app has to track/store both events and match the storefront/listing IDs. If we store and emit nftID [and nftType] on the contract, the marketplace's logic would be simplified.

judezhu commented 2 years ago

Totally agree. Especially in the case that there are more than one type of NFTs in storefront, it makes very challenging to handle ListingCompleted event. We prefer to add both nftID and nftType, if possible.

iliawx commented 2 years ago

Yes, this is a must have for us. Otherwise, we need to implement workarounds in order to filter out events for our NFTs types.