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

Duplicate Zero Address Check in transferErcTokens() Increases Gas Cost

mukulkolpe

Summary

Duplicate Zero Address Check in transferErcTokens() Increases Gas Cost

Vulnerability Details

In the TokenDivider::transferErcTokens function, there is a redundant check for the zero address. The same condition if(to == address(0)) appears twice in the function, with both checks reverting with the same error TokenDivider__CantTransferToAddressZero(). This duplicate validation unnecessarily increases gas costs without providing any additional security benefits.

Proof of Concept

To verify this gas optimization, paste the following test into your TokenDiverTest.t.sol contract and run it:

function testTransferErcTokens() public nftDivided {
ERC20Mock erc20Mock = ERC20Mock(
tokenDivider.getErc20InfoFromNft(address(erc721Mock)).erc20Address
);
vm.startPrank(USER);
erc20Mock.approve(address(tokenDivider), AMOUNT);
tokenDivider.transferErcTokens(address(erc721Mock), USER2, AMOUNT);
vm.stopPrank();
assertEq(tokenDivider.getBalanceOf(USER2, address(erc20Mock)), AMOUNT);
assertEq(tokenDivider.getBalanceOf(USER, address(erc20Mock)), 0);
}

Running forge test --mt testTransferErcTokens shows:

  • With duplicate check: 1,147,593 gas units

  • After removing duplicate check: 1,147,559 gas units

Impact

The duplicate check results in:

  • Higher gas costs for users when calling the transferErcTokens function

  • While relatively small, this cost multiplies across all transfer transactions in the system

Tools Used

Foundry

Recommendations

Remove the second zero address check in the transferErcTokens function while keeping the first one. Here's the diff of the changes:

function transferErcTokens(address nftAddress, address to, uint256 amount) external {
if(nftAddress == address(0)) {
revert TokenDivider__NftAddressIsZero();
}
​
if(to == address(0)) {
revert TokenDivider__CantTransferToAddressZero();
}
​
if(amount == 0) {
revert TokenDivider__AmountCantBeZero();
}
​
ERC20Info memory tokenInfo = nftToErc20Info[nftAddress];
​
- if(to == address(0)) {
- revert TokenDivider__CantTransferToAddressZero();
- }
if(balances[msg.sender][tokenInfo.erc20Address] < amount) {
revert TokenDivider__NotEnoughErc20Balance();
}
​
balances[msg.sender][tokenInfo.erc20Address] -= amount;
balances[to][tokenInfo.erc20Address] += amount;
​
emit TokensTransfered(amount, tokenInfo.erc20Address);
​
IERC20(tokenInfo.erc20Address).transferFrom(msg.sender, to, amount);
}
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.