search

PatrickAlphaC / hardhat-nft-marketplace-fcc

114 stars 95 forks source link

Handling case where user sends token after listing? #2

Closed nishilshah17 closed 2 years ago

nishilshah17 commented 2 years ago

Hey thanks for writing this detailed article and publishing the NFTMarketplace.sol code. Trying to understand everything and had a question about what happens if a user sends a token to a different address after they have listed the token already.

Am I correct that the buyItem function will fail and revert any changes to s_proceeds? Also curious how you would handle this from the client-side. With this current logic it seems possible to get listings which are no longer "valid" (the owner has changed since the listing was created). Thanks!

PatrickAlphaC commented 2 years ago

That was my friend @zeuslawyer actually!

You are correct! buyItem would fail. On the client-side, you'd just check to see that the NFT is still approved by the NFT marketplace contract, and only show the NFT if it is.

That's something we didn't add, but would be easy enough to add. There were a few things we didnt add... but that lesson was already 5 hours heh.

PatrickAlphaC commented 2 years ago

Closing, but feel free to follow up.

ade-hardhat commented 1 year ago

That was my friend @zeuslawyer actually!

You are correct! buyItem would fail. On the client-side, you'd just check to see that the NFT is still approved by the NFT marketplace contract, and only show the NFT if it is.

That's something we didn't add, but would be easy enough to add. There were a few things we didnt add... but that lesson was already 5 hours heh.

Hi @PatrickAlphaC , is there a way to check if approval has been renounced from the Marketplace? During testing I noticed that if a user deletes the NFT Listing from the marketplace and tries to buy the same NFT it reverts with 'ERC721: transfer from incorrect owner' as opposed to my custom error to check if NFT is approved for listing NftMarketplace__NftNotApprovedForListing(). So I guess that means the NFT is still approved after it's been deleted from the Marketplace, right?

However, this wouldn't be an issue on the client side since we're only querying the active items . But in the case where the user sends the NFT to a different address after they have listed the NFT already, how would we check to see that the NFT is still approved by the NFT marketplace contract?

Thank you for your time!

PatrickAlphaC commented 1 year ago

@ade-hardhat good question.

At the moment, I didn't code one (I mentioned it somewhere in the video that we aren't accounting for that), there are ways we could code this to do it, but it would add a lot of extra code and take away from the simplicity.

You make a great point though :)

We could call the getApproved function on our ERC721 to see who has approved access.