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

[L-3] Potential Overuse of Fractional Amount in `divideNft` Function

koinerdegen

Summary

The divideNft function currently allows an unlimited value for the amount parameter, which is defined as uint256. This can result in an excessive number of fractional NFT units, potentially leading to operational inefficiencies and higher gas costs.

Issue Details

Function: divideNft

Code Snippet:

function divideNft(address nftAddress, uint256 tokenId, uint256 amount) onlyNftOwner(nftAddress, tokenId) onlyNftOwner(nftAddress ,tokenId) external {
if(nftAddress == address(0)) { revert TokenDivider__NftAddressIsZero(); }
if(amount == 0) { revert TokenDivider__AmountCantBeZero(); }
​
ERC20ToGenerateNftFraccion erc20Contract = new ERC20ToGenerateNftFraccion(
string(abi.encodePacked(ERC721(nftAddress).name(), "Fraccion")),
string(abi.encodePacked("F", ERC721(nftAddress).symbol())));
​
erc20Contract.mint(address(this), amount);
address erc20 = address(erc20Contract);
​
IERC721(nftAddress).safeTransferFrom(msg.sender, address(this), tokenId, "");
if(IERC721(nftAddress).ownerOf(tokenId) == msg.sender) { revert TokenDivider__NftTransferFailed(); }
​
balances[msg.sender][erc20] = amount;
nftToErc20Info[nftAddress] = ERC20Info({erc20Address: erc20, tokenId: tokenId});
erc20ToMintedAmount[erc20] = amount;
erc20ToNft[erc20] = nftAddress;
​
emit NftDivided(nftAddress, amount, erc20);
​
bool transferSuccess = IERC20(erc20).transfer(msg.sender, amount);
if(!transferSuccess) {
revert TokenDivider__TransferFailed();
}
}

Impact

  • Gas Efficiency: Large values for amount can increase gas costs due to higher storage operations.

  • Operational Complexity: Excessive fractionalization can lead to challenges in managing and tracking smaller units.

  • User Experience: Users may find it cumbersome to handle an unnecessarily high number of fractional tokens.

Recommendation

Consider limiting the amount parameter to a smaller data type such as uint8 or uint16 to control the number of fractional units and improve gas efficiency.

function divideNft(address nftAddress, uint256 tokenId, uint16 amount) onlyNftOwner(nftAddress, tokenId) external {
// Function logic remains unchanged
}

This change would ensure a practical upper limit on fractional tokens while optimizing storage and gas consumption.

Severity

Low – The current implementation is functional but may lead to inefficiencies if excessively large values are used.

Status

Pending review and implementation.

Updates

Lead Judging Commences

fishy Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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