Bid Beasts

First Flight #49
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Submission Details
Impact: low
Likelihood: high
Invalid

Redundant Code Checks Leading to Gas Inefficiencies

Root + Impact

The contract contains redundant validation checks in multiple functions, which result in unnecessary gas consumption.
Users pay higher gas fees than necessary for certain operations, particularly when using the Buy Now feature or settling auctions.

1. Redundant Overpay Check Before Payout

Description

  • The normal behavior of well-optimized contracts is to avoid redundant checks to minimize gas costs.

  • The specific issue is that in the placeBid function's Buy Now logic, there's a check for overpay > 0 before calling _payout, but the _payout function already has its own check for zero amounts.

// In placeBid function:
if (overpay > 0) { @> // Redundant check
_payout(msg.sender, overpay);
}
// In _payout function:
function _payout(address recipient, uint256 amount) internal {
if (amount == 0) return; @> // Already checks for zero amount
// ... rest of the function
}

Risk

Likelihood: High

  • This redundant check is executed every time a user uses the Buy Now feature and has some overpayment.

Impact: Low

  • Causes a slight increase in gas costs for users.

  • No security implications, purely a gas optimization issue.

Proof of Concept

The redundant check can be observed in the code:

  1. Line 134-136: if (overpay > 0) { _payout(msg.sender, overpay); }

  2. Line 227: if (amount == 0) return; in the _payout function

If overpay is 0, even if the external check is removed and _payout is called directly, the function would immediately return due to its internal check, resulting in the same behavior but with gas savings.

Recommended Mitigation

// In placeBid function:
- if (overpay > 0) {
_payout(msg.sender, overpay);
- }

This simple change:

  1. Reduces Gas Costs: Eliminates an unnecessary condition check

  2. Maintains Functionality: The behavior remains identical as _payout already handles zero amounts

  3. Improves Code Cleanliness: Removes redundant validation

2. Redundant Minimum Price Check in settleAuction

Description

  • The normal behavior of efficient smart contracts is to avoid redundant validation when a condition is guaranteed by previous constraints.

  • The specific issue is that settleAuction checks if the highest bid meets the minimum price, but this is already guaranteed by the bidding logic.

function settleAuction(uint256 tokenId) external isListed(tokenId) {
Listing storage listing = listings[tokenId];
require(listing.auctionEnd > 0, "Auction has not started (no bids)");
require(block.timestamp >= listing.auctionEnd, "Auction has not ended");
require(bids[tokenId].amount >= listing.minPrice, "Highest bid did not meet min price"); @> // Redundant check
_executeSale(tokenId);
}

Risk

Likelihood: High

  • his redundant check is executed every time an auction is settled.

Impact: Low

  • Causes a slight increase in gas costs for auction settlement.

  • No security implications, purely a gas optimization issue.

Proof of Concept

This check is redundant because:

  1. Line 183 confirms there's at least one bid (listing.auctionEnd > 0)

  2. First bids must be greater than the minimum price per line 150: require(msg.value > requiredAmount, "First bid must be > min price")

  3. Subsequent bids must be greater than previous bids (line 156): require(msg.value >= requiredAmount, "Bid not high enough")

Given these constraints, it's impossible for the highest bid to be below the minimum price if the auction has received any bids. Therefore, the check on line 185 is redundant and wastes gas.

Recommended Mitigation

function settleAuction(uint256 tokenId) external isListed(tokenId) {
Listing storage listing = listings[tokenId];
require(listing.auctionEnd > 0, "Auction has not started (no bids)");
require(block.timestamp >= listing.auctionEnd, "Auction has not ended");
- require(bids[tokenId].amount >= listing.minPrice, "Highest bid did not meet min price");
_executeSale(tokenId);
}

This change:

  1. Reduces Gas Costs: Eliminates an unnecessary validation check

  2. Maintains Security: The contract's logic already ensures this condition is true

  3. Improves Efficiency: Simplifies the auction settlement process

Overall Recommendation

These findings highlight opportunities for gas optimization by removing redundant checks. While they don't represent security vulnerabilities, addressing them would improve the contract's efficiency and reduce transaction costs for users.

The recommended approach is to:

  1. Remove the redundant overpay > 0 check in the Buy Now logic

  2. Remove the redundant minimum price check in the settleAuction function

These changes would maintain identical functionality while improving gas efficiency.

Updates

Lead Judging Commences

cryptoghost Lead Judge 2 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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

Give us feedback!