NFT Dealers

[I-1] mintNft() and buy() are marked payable but do not handle ETH

Description: Both mintNft() and buy() are declared payable, but neither uses msg.value. Any ETH sent to these functions will be permanently locked in the contract with no recovery mechanism.

Impact: User ETH may be accidentally locked in the contract.

Recommended Mitigation: Remove the payable keyword from both functions.

[I-2] msg.sender == address(0) check in mintNft() is dead code

Description: The EVM guarantees that msg.sender is never address(0). The check on line 115 can never be triggered and wastes gas.

Recommended Mitigation: Remove the check.

[I-3] list() contains a redundant _price > 0 check

Description: Line 128 already requires _price >= MIN_PRICE where MIN_PRICE = 1e6 > 0. The subsequent require(_price > 0) on line 131 is unreachable.

Recommended Mitigation: Remove the redundant check.

[I-4] collectUsdcFromSelling() calls safeTransfer(address(this), fees) which is a no-op

Description: usdc.safeTransfer(address(this), fees) transfers USDC from the contract to itself. The buyer's full payment (including fees) is already held in the contract after buy(). This line has no effect and wastes gas.

Recommended Mitigation: Remove the usdc.safeTransfer(address(this), fees) line.

[I-5] metadataFrozen state variable is declared but never used

Description: metadataFrozen is declared as a public boolean but no function sets it to true and no modifier checks it. The intended freeze mechanism was never implemented.

Recommended Mitigation: Either implement the freeze mechanism (a setter function + modifier on _baseURI) or remove the unused variable.

[I-6] _listingId parameter is actually _tokenId, causing naming confusion

Description: In buy(), cancelListing(), and collectUsdcFromSelling(), the parameter is named _listingId, but s_listings is keyed by tokenId. The listingsCounter increments independently and is only used in events, making the naming misleading and inconsistent.

Recommended Mitigation: Rename _listingId to _tokenId in the affected functions, or rearchitect to use listingsCounter as the actual mapping key.