Pieces Protocol

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

Low findings

##L-1. Using ERC20 token is not needed

Summary

The contract TokenDivider creates ERC20ToGenerateNftFraccion token for every divided NFT. However, the token ERC20ToGenerateNftFraccion is used only for the contract mappings keys and for transfers. The token does not make any sense, because all logic in the contract is based on the contract mapping balances and does not use the ERC20ToGenerateNftFraccion balances. Transfers of ERC20ToGenerateNftFraccion outside the contract (for example, in trading) also does not make any since since the balances mapping in the contract is not updated and the ERC20ToGenerateNftFraccion is not used. Keys for the mapping in the contract where the ERC20ToGenerateNftFraccion is used can be changed to keccak256 of some other values, and this will save much gas (especially when the new contract will not be deployed for every divided NFT).

Recommendations

Remove the ERC20ToGenerateNftFraccion contract and use combination of nftAddress and tkoenId for the contract mappings where it is used.

##L-2. Anyone can mint any amount of ERC20ToGenerateNftFraccion

####Summary
ERC20ToGenerateNftFraccion::mint has no access controls in place and anyone can mint any amount. The issue is considered as low, because the token balance is not used anywhere (see the L-1 issue) and does not affect the contract logic.

Recommendations

Add owner functionality to the ERC20ToGenerateNftFraccion contract and check that only owner can mint ERC20ToGenerateNftFraccion.

##L-3. onlyNftOwner modifier is applied twice for TokenDivider::divideNft function

####Summary
onlyNftOwner modifier is applied twice for TokenDivider::divideNft function:

function divideNft(address nftAddress, uint256 tokenId, uint256 amount) onlyNftOwner(nftAddress, tokenId) onlyNftOwner(nftAddress ,tokenId)

Recommendations

Remove duplicated modifier:

-function divideNft(address nftAddress, uint256 tokenId, uint256 amount) onlyNftOwner(nftAddress, tokenId) onlyNftOwner(nftAddress ,tokenId)
+function divideNft(address nftAddress, uint256 tokenId, uint256 amount) onlyNftOwner(nftAddress, tokenId)
Updates

Lead Judging Commences

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

Support

FAQs

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