code423n4 commented 2 years ago

1. REQUIRE INSTEAD OF &&

Require statements including conditions with the && operator can be broken down in multiple require statements to save gas. instances include : NestedFactory.sol:69 https://github.com/code-423n4/2022-06-nested/blob/b253ed80f67d1bb2a04e1702f5796fd96a7c521e/contracts/operators/Beefy/BeefyVaultOperator.sol#L54 https://github.com/code-423n4/2022-06-nested/blob/b253ed80f67d1bb2a04e1702f5796fd96a7c521e/contracts/operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol#L61-L62 https://github.com/code-423n4/2022-06-nested/blob/b253ed80f67d1bb2a04e1702f5796fd96a7c521e/contracts/operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol#L63-L65 https://github.com/code-423n4/2022-06-nested/blob/b253ed80f67d1bb2a04e1702f5796fd96a7c521e/contracts/operators/Paraswap/ParaswapOperator.sol#L16

2. save gas by using msg.sender instead of msgSender() function

msg.sender is global variable and msgSender() function needs th get that variable making it cost more gas. instances: NestedFactory.sol:104,200,207

3. Mark functions as payable when users can't mistakenly send ETH

impact

Functions marked as payable are 24 gas cheaper than their counterpart (in non-payable functions, Solidity adds an extra check to ensure msg.value is zero). When users can't mistakenly send ETH to a function (as an example, when there's an onlyOwner modifier or alike), it is safe to mark it as payable

proof of concept

Functions with onlyOwner modifier that aren't payable yet: instances include: NestedFactory.sol 126: function addOperator(bytes32 operator) external override onlyOwner { 139: function removeOperator(bytes32 operator) external override onlyOwner { 158: function setFeeSplitter(FeeSplitter _feeSplitter) external override onlyOwner { 165: function setEntryFees(uint256 _entryFees) external override onlyOwner { 173: function setExitFees(uint256 _exitFees) external override onlyOwner { 181: function unlockTokens(IERC20 _token) external override onlyOwner { OpertorResolver.sol 56: ) external override onlyOwner { 74: function rebuildCaches(MixinOperatorResolver[] calldata destinations) public onlyOwner { https://github.com/code-423n4/2022-06-nested/blob/b253ed80f67d1bb2a04e1702f5796fd96a7c521e/contracts/operators/Beefy/BeefyVaultStorage.sol#L34 https://github.com/code-423n4/2022-06-nested/blob/b253ed80f67d1bb2a04e1702f5796fd96a7c521e/contracts/operators/Beefy/BeefyVaultStorage.sol#L24

recommended mitigation steps

Mark as payable the functions that have a "safe for users" modifier

4. No need to explicitly initialize variables with default values

If a variable is not set/initialized, it is assumed to have the default value (0 for uint, false for bool, address(0) for address...). Explicitly initializing it with its default value is an anti-pattern and wastes gas. instances include: NestedFactory.sol : 130: for (uint256 i = 0; i < operatorsCache.length; i++) { 142: for (uint256 i = 0; i < operatorsLength; i++) { 202: for (uint256 i = 0; i < batchedOrdersLength; i++) { 262: for (uint256 i = 0; i < tokensLength; i++) { 321: for (uint256 i = 0; i < batchedOrdersLength; i++) { 339: for (uint256 i = 0; i < batchedOrdersLength; i++) { 375: for (uint256 i = 0; i < batchLength; i++) { 418: for (uint256 i = 0; i < batchLength; i++) { OpertorResolver.sol: 40: for (uint256 i = 0; i < namesLength; i++) { 60: for (uint256 i = 0; i < names.length; i++) { 75: for (uint256 i = 0; i < destinations.length; i++) { MixinOperatorResolver.sol: 37: for (uint256 i = 0; i < requiredOperators.length; i++) { 56: for (uint256 i = 0; i < requiredOperators.length; i++) { TimelockControllerEmergency.sol: 84,89,234,324