Magic Values are used in the function BidBeastsNFTMarket::placeBid and BidBeastsNFTMarket::_executeSale which is not a recommended practice as decreases the readablilty of the code.
Description: In the BidBeastsNFTMarket::placeBid and BidBeastsNFTMarket::_executeSale function ,the codebase have magic values like 100 which is not a good practice ,hence causing confusion and descreases the readability of the code.
Impact: This descreases the readability of the code and also causes confusion and is not a good practice .
Proof of Concept: These magic values are found in BidBeastsNFTMarket::placeBid and BidBeastsNFTMarket::_executeSale, can be seen below :
Found : BidBeastsNFTMarket::placeBid
function placeBid(uint256 tokenId) external payable isListed(tokenId) {
...
if (previousBidAmount == 0) {
requiredAmount = listing.minPrice;
require(
msg.value > requiredAmount,
"First bid must be > min price"
);
listing.auctionEnd = block.timestamp + S_AUCTION_EXTENSION_DURATION;
emit AuctionExtended(tokenId, listing.auctionEnd);
} else {
requiredAmount =
@> (previousBidAmount / 100) *
@> (100 + S_MIN_BID_INCREMENT_PERCENTAGE);
require(msg.value >= requiredAmount, "Bid not high enough");
}
...
}
Found : BidBeastsNFTMarket::_executeSale
function _executeSale(uint256 tokenId) internal {
Listing storage listing = listings[tokenId];
Bid memory bid = bids[tokenId];
listing.listed = false;
delete bids[tokenId];
BBERC721.transferFrom(address(this), bid.bidder, tokenId);
@> uint256 fee = (bid.amount * S_FEE_PERCENTAGE) / 100;
s_totalFee += fee;
uint256 sellerProceeds = bid.amount - fee;
_payout(listing.seller, sellerProceeds);
emit AuctionSettled(tokenId, bid.bidder, listing.seller, bid.amount);
}
Recommended Mitigation: It is recommended to use proper constants for these magic value as these makes the code unreadable and unnecessarily confusing .
uint256 public constant S_AUCTION_EXTENSION_DURATION = 15 minutes;
uint256 public constant S_MIN_NFT_PRICE = 0.01 ether;
uint256 public constant S_FEE_PERCENTAGE = 5;
uint256 public constant S_MIN_BID_INCREMENT_PERCENTAGE = 5;
+ uint256 public constant S_AUCTION_EXTENSION_DURATION=100;
+ uint256 public constant S_PERCENTAGE_FEE_PRECISION_FACTOR=100;
function placeBid(uint256 tokenId) external payable isListed(tokenId) {
...
if (previousBidAmount == 0) {
requiredAmount = listing.minPrice;
require(
msg.value > requiredAmount,
"First bid must be > min price"
);
listing.auctionEnd = block.timestamp + S_AUCTION_EXTENSION_DURATION;
emit AuctionExtended(tokenId, listing.auctionEnd);
} else {
requiredAmount =
- (previousBidAmount / 100) *
- (100 + S_MIN_BID_INCREMENT_PERCENTAGE);
+ (previousBidAmount / MIN_BID_PRECISION_FACTOR) *
+ (MIN_BID_PRECISION_FACTOR + S_MIN_BID_INCREMENT_PERCENTAGE);
require(msg.value >= requiredAmount, "Bid not high enough");
}
...
}
...
function _executeSale(uint256 tokenId) internal {
Listing storage listing = listings[tokenId];
Bid memory bid = bids[tokenId];
listing.listed = false;
delete bids[tokenId];
BBERC721.transferFrom(address(this), bid.bidder, tokenId);
- uint256 fee = (bid.amount * S_FEE_PERCENTAGE) / 100;
+ uint256 fee = (bid.amount * S_FEE_PERCENTAGE) / S_PERCENTAGE_FEE_PRECISION_FACTOR;
s_totalFee += fee;
uint256 sellerProceeds = bid.amount - fee;
_payout(listing.seller, sellerProceeds);
emit AuctionSettled(tokenId, bid.bidder, listing.seller, bid.amount);
}
}
...
