TempleGold
TempleDAO
Foundry
25,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Inefficient Placement of AuctionEnded Check in removeAuctionConfig

tpiliposian

Summary

The removeAuctionConfig() function in the SpiceAuction.sol contract contains a check to determine if an auction has ended. This check is currently placed within a nested conditional block, which can lead to unnecessary computational steps before reverting the transaction. Moving this check earlier in the function would improve efficiency and readability.

Vulnerability Details

The removeAuctionConfig() function checks if an auction has ended using if (info.hasEnded()) { revert AuctionEnded(); }. This check is performed within a nested conditional block, potentially after several other checks and computations have been executed. This placement can result in unnecessary processing if the auction has already ended, as the function will ultimately revert anyway.

function removeAuctionConfig() external override onlyDAOExecutor {
/// only delete latest epoch if auction is not started
uint256 id = _currentEpochId;
EpochInfo storage info = epochs[id];
// _currentEpochId = 0
if (info.startTime == 0) { revert InvalidConfigOperation(); }
// Cannot reset an ongoing auction
if (info.isActive()) { revert InvalidConfigOperation(); }
/// @dev could be that `auctionStart` is triggered but there's cooldown, which is not reached (so can delete epochInfo for _currentEpochId)
// or `auctionStart` is not triggered but `auctionConfig` is set (where _currentEpochId is not updated yet)
bool configSetButAuctionStartNotCalled = auctionConfigs[id+1].duration > 0;
if (!configSetButAuctionStartNotCalled) {
/// @dev unlikely because this is a DAO execution, but avoid deleting old ended auctions
if (info.hasEnded()) { revert AuctionEnded(); }
/// auction was started but cooldown has not passed yet
delete auctionConfigs[id];
delete epochs[id];
_currentEpochId = id - 1;
emit AuctionConfigRemoved(id, id);
} else {
// `auctionStart` is not triggered but `auctionConfig` is set
id += 1;
delete auctionConfigs[id];
emit AuctionConfigRemoved(id, 0);
}
}

Impact

The current placement of the AuctionEnded check can lead to inefficiencies in the execution of the removeAuctionConfig() function. It does result in unnecessary computational steps, which can increase gas costs for transactions that invoke this function.

Tools Used

Manual review.

Recommendations

Move the if (info.hasEnded()) { revert AuctionEnded(); } check to the beginning of the removeAuctionConfig() function, e.g. after if (info.isActive()) { revert InvalidConfigOperation(); } check.

Updates

Lead Judging Commences

inallhonesty Lead Judge 12 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.