NFT Dealers
First Flight #58
Beginner FriendlyFoundry
100 EXP
View all submissions
Previous Next
Submission Details
Impact: low
Likelihood: high

NFTDealers::collectUsdcFromSelling transfers fees to itself wasting gas

Author Revealed upon completion

NFTDealers::collectUsdcFromSelling transfers fees to itself wasting gas

Description

  • collectUsdcFromSelling is responsible for distributing sale proceeds to the seller after a successful NFT sale, deducting protocol fees before the transfer.

  • The function calls usdc.safeTransfer(address(this), fees) which transfers USDC from the contract back to itself. Since the fees are already held in the contract, this transfer is a complete no-op that wastes gas on every call.

function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
...
totalFeesCollected += fees;
amountToSeller += collateralToReturn;
//@> transfers fees from contract to itself — does nothing
usdc.safeTransfer(address(this), fees);
usdc.safeTransfer(msg.sender, amountToSeller);
}

Risk

Likelihood: High

  • Every seller who calls collectUsdcFromSelling triggers this unnecessary transfer

  • No special conditions required — executes on every single collection

Impact:

  • Wastes gas on every collectUsdcFromSelling call

  • Emits a misleading Transfer event from contract to itself, polluting event logs and confusing off-chain indexers

Proof of Concept

Self transfer changes nothing — fees stay in contract, but gas was wasted and a misleading Transfer event was emitted

function test_SelfTransferNoOp() public revealed whitelisted() {
uint256 tokenId = 1;
uint256 nftPrice = 1000e6;
​
mintAndListNFTForTesting(tokenId, nftPrice);
​
vm.startPrank(userWithEvenMoreCash);
usdc.approve(address(nftDealers), nftPrice);
nftDealers.buy(tokenId);
vm.stopPrank();
​
uint256 contractBalanceBefore = usdc.balanceOf(address(nftDealers));
​
vm.prank(userWithCash);
nftDealers.collectUsdcFromSelling(tokenId);
​
// self transfer
console.log("contract before:", contractBalanceBefore);
console.log("contract after :", usdc.balanceOf(address(nftDealers)));
}

Recommended Mitigation

Consider removing the following line in order to save gas

function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
Listing memory listing = s_listings[_listingId];
require(!listing.isActive, "Listing must be inactive to collect USDC");
​
uint256 fees = _calculateFees(listing.price);
uint256 amountToSeller = listing.price - fees;
uint256 collateralToReturn = collateralForMinting[listing.tokenId];
​
totalFeesCollected += fees;
+ collateralForMinting[listing.tokenId] = 0;
amountToSeller += collateralToReturn;
- usdc.safeTransfer(address(this), fees);
usdc.safeTransfer(msg.sender, amountToSeller);
}

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!