code-423n4 / 2022-06-nested-findings

0 stars 1 forks source link

Gas Optimizations #58

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

1 Use require instead &&

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/NestedFactory.sol#L66

use require instead of && for efficient gas cost. change it from

          require(
        address(_nestedAsset) != address(0) &&
            address(_nestedRecords) != address(0) &&
            address(_reserve) != address(0) &&
            address(_feeSplitter) != address(0) &&
            address(_weth) != address(0) &&
            _operatorResolver != address(0) &&
            address(_withdrawer) != address(0),
        "NF: INVALID_ADDRESS"
    );

to

          require(address(_nestedAsset) != address(0),"NF: INVALID_ADDRESS");
          require(address(_nestedRecords) != address(0),"NF: INVALID_ADDRESS");
          require(address(_reserve) != address(0),"NF: INVALID_ADDRESS");
          require(address(_feeSplitter) != address(0),"NF: INVALID_ADDRESS");
          require(address(_weth) != address(0),"NF: INVALID_ADDRESS");
          require( _operatorResolver != address(0),"NF: INVALID_ADDRESS");
          require(address(_withdrawer) != address(0),"NF: INVALID_ADDRESS");

2 Change to storage

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/NestedFactory.sol#L123

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/NestedFactory.sol#L248

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/NestedFactory.sol#L466

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/abstracts/MixinOperatorResolver.sol#L52

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/abstracts/MixinOperatorResolver.sol#L101

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/abstracts/MixinOperatorResolver.sol#L33

Use storage instead of memory to reduce the gas fee.

          bytes32[] memory operatorsCache = operators;

to

          bytes32[] storage operatorsCache = operators;

apply to others.

3 Looping

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/NestedFactory.sol#L124

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/NestedFactory.sol#L651

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/abstracts/MixinOperatorResolver.sol#L56

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/abstracts/MixinOperatorResolver.sol#L37

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/governance/TimelockControllerEmergency.sol#L234

default uint is 0 so remove unnecassary explicit can reduce gas. caching the array length can reduce gas it caused access to a local variable is more cheap than query storage / calldata / memory in solidity. pre increment ++i more cheaper gas than post increment i++. i suggest to use pre increment.

4 Default value and increment

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/NestedFactory.sol#L136

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/NestedFactory.sol#L196

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/NestedFactory.sol#L315

default uint is 0 so remove unnecassary explicit can reduce gas pre increment ++i more cheaper gas than post increment i++. i suggest to use pre increment.

5 Inequality

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/NestedFactory.sol#L286

non strict inequality are cheaper than strict one. i suggest to use >= or <= instead of > and < if possible.

6 Caching names.length

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/OperatorResolver.sol#L57

caching the names.length can reduce gas it caused access to a local variable is more cheap than query storage / calldata / memory in solidity and it use twice.

7 Cache the cacheTmp.implementation

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/abstracts/MixinOperatorResolver.sol#L62-L64

cache the cacheTmp.implementation to the memory for reduce the gas fee because it use multiple times.

8 Use calldata

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/abstracts/MixinOperatorResolver.sol#L93

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/operators/Beefy/BeefyVaultOperator.sol#L13

In the external functions where the function argument is read-only, the function() has an inputed parameter that using memory, if this function didnt change the parameter, its cheaper to use calldata then memory. so we suggest to change it.

9 Division

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol#L275

A division by 2 can be calculated by shifting one to the right. The div opcode used 5 gas and SHR opcode used 3 gas. Solidity's division operation also includes a division-by-0 prevention by pass using shifting. so i suggest to use >>1.

10 Sort struct

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/operators/Yearn/YearnVaultStorage.sol#L6-L9

shorting the struct can reduce gas cost, so change it from

struct CurvePool {
address poolAddress;
uint96 poolCoinAmount;
address lpToken;

to

struct CurvePool {
uint96 poolCoinAmount;
address poolAddress;
address lpToken;

11 Short the string

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/governance/TimelockControllerEmergency.sol#L243-L244

reduce size of string error message to bytes32 for cheap gas if possible.

12 Caching the targets.length

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/governance/TimelockControllerEmergency.sol#L319-L320

caching the targets.length can reduce gas it caused access to a local variable is more cheap than query storage / calldata / memory in solidity and it use twice.

    require(targets.length == values.length, "TimelockController: length mismatch");
    require(targets.length == datas.length, "TimelockController: length mismatch");

i suggest to add uint256 _targets = targets.length;

    uint256 _targets = targets.length;
    require(_targets == values.length, "TimelockController: length mismatch");
    require(_targets == datas.length, "TimelockController: length mismatch");
Yashiru commented 2 years ago

6 Caching names.length (Confirmed)

Gas optimization confirmed

obatirou commented 2 years ago

8 Use calldata (disputed)

Examples are not relevant:

Yashiru commented 2 years ago

5 Inequality (Confirmed)

Gas optimization confirmed

Yashiru commented 2 years ago

12 Caching the targets.length (Duplicated)

Duplicated of #49 at TimelockControllerEmergency.sol

obatirou commented 2 years ago

11 Short the string (duplicated)

Duplicate of point 2 from issue #62

Yashiru commented 2 years ago

9 Division (Duplicated)

Duplicated of #89 at Use Shift Right/Left instead of Division/Multiplication

Yashiru commented 2 years ago

3 Looping (Disputed)

Duplicated of #2 at For loop optimizaion

4 Default value and increment (Disputed)

Duplicated of #2 at For loop optimizaion

maximebrugel commented 2 years ago

10 Sort struct (Disputed)

Two addresses can't be packed (20 bytes and 20 bytes).

obatirou commented 2 years ago

1 Use require instead && (duplicate)

https://github.com/code-423n4/2022-06-nested-findings/issues/29#issuecomment-1165702145

maximebrugel commented 2 years ago

7 Cache the cacheTmp.implementation (Acknowledged)

Yashiru commented 2 years ago

2 Change to storage (Confirmed)