search

code-423n4 / 2021-05-nftx-findings

1 stars 0 forks source link

uint256[25] ___gap argument in NFTXVaultUpgradeable Increases Deployment Costs For createVault() #96

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

jvaqa

Vulnerability details

uint256[25] ___gap variables in NFTXVaultUpgradeable Increases Deployment Costs For createVault()

Impact

The ___gap variable in NFTXVaultUpgradeable unnecessarily pads the contract with extra zeroes at the end, while also making the contract pay extra gas to declare an extra unused variable.

Seemingly, this was done to try to optimize variable packing. However, since these variables occur at the end of the contract, and since they are all of size uint256, they only bloat the contract by making the contract declaring an unnecessary variable.

This unnecessary variable declaration will slightly increase the gas costs of contract deployment in NFTXVaultFactory.createVault()

Proof of Concept

Here are the unused variables:

https://github.com/code-423n4/2021-05-nftx/blob/f6d793c136d110774de259d9f3b25d003c4f8098/nftx-protocol-v2/contracts/solidity/NFTXVaultUpgradeable.sol#L438

Recommended Mitigation Steps

Delete the following line: uint256[25] ___gap;

0xKiwi commented 3 years ago

This was not done for packing, it was done to keep more storage slot room for future updates in case its needed.

cemozerr commented 3 years ago

This seems like a non-issue.