Pieces Protocol

First Flight #32
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Submission Details
Severity: low
Invalid

Users will experience high transaction cost due to Code redundancy

Summary

The TokenDivider contract contains instances of redundant code where identical checks and modifiers are repeated unnecessarily. These redundancies not only make the code less efficient but also increase gas costs for users interacting with the contract.

Vulnerability Details

There are two significant instances of redundant code in the contract:

  1. In the divideNft function, the onlyNftOwner modifier is applied twice with identical parameters:

function divideNft(address nftAddress, uint256 tokenId, uint256 amount)
onlyNftOwner(nftAddress, tokenId)
onlyNftOwner(nftAddress ,tokenId) // Redundant modifier
external {
// Function implementation
}
  1. In the transferErcTokens function, the zero address check for the recipient is performed twice:

function transferErcTokens(address nftAddress, address to, uint256 amount) external {
if(nftAddress == address(0)) {
revert TokenDivider__NftAddressIsZero();
}
if(to == address(0)) {
revert TokenDivider__CantTransferToAddressZero();
}
// ... other code ...
if(to == address(0)) { // Redundant check
revert TokenDivider__CantTransferToAddressZero();
}
}

Impact

Each redundant check and modifier adds unnecessary gas costs to transactions. The duplicate modifier in divideNft means users pay twice for the same ownership verification, while the repeated zero address check-in transferErcTokens wastes gas in an already-verified condition.

Recommendation

For the divideNft function:

function divideNft(address nftAddress, uint256 tokenId, uint256 amount)
onlyNftOwner(nftAddress, tokenId) // Single modifier application
external {
}

For the transferErcTokens function:

function transferErcTokens(address nftAddress, address to, uint256 amount) external {
if(nftAddress == address(0)) {
revert TokenDivider__NftAddressIsZero();
}
if(to == address(0)) {
revert TokenDivider__CantTransferToAddressZero();
}
// Rest of the function without the second zero address check
Updates

Lead Judging Commences

fishy Lead Judge 5 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.