Nested Finance contest - Meera's results

Overview

Table of Contents

• Low Risk Issues
  • 1. Known vulnerabilities exist in currently used @openzeppelin/contracts version
  • 2. Missing address(0) checks
  • 3. OwnableProxyDelegation.initialize() is front-runnable in the solution
  • 4. Use a constant instead of duplicating the same string
  • 5. Funds can be locked
  • 6. A magic number should be documented and explained. Use a constant instead
• Non-Critical Issues
  • 1. It's better to emit after all processing is done
  • 2. Typos
  • 3. Adding a return statement when the function defines a named return variable, is redundant
  • 4. public functions not called by the contract should be declared external instead

Low Risk Issues

1. Known vulnerabilities exist in currently used @openzeppelin/contracts version

As some known vulnerabilities exist in the current @openzeppelin/contracts version, consider updating package.json with at least @openzeppelin/contracts@4.4.2 here:

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/package.json#L65

        "@openzeppelin/contracts": "^4.3.2",

While vulnerabilities are known, the current scope isn't affected (this might not hold true for the whole solution)

2. Missing address(0) checks

Consider adding an address(0) check for immutable variables:

operators/Yearn/YearnCurveVaultOperator.sol:23:    address public immutable eth;
operators/Yearn/YearnCurveVaultOperator.sol:26:    IWETH private immutable weth;
operators/Yearn/YearnCurveVaultOperator.sol:29:    Withdrawer private immutable withdrawer;
utils/NestedAssetBatcher.sol:19:    INestedAsset public immutable nestedAsset;
utils/NestedAssetBatcher.sol:20:    INestedRecords public immutable nestedRecords;
Withdrawer.sol:14:    IWETH public immutable weth;

3. OwnableProxyDelegation.initialize() is front-runnable in the solution

I suggest adding some access control or atomically initializing the contract:

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/abstracts/OwnableProxyDelegation.sol#L24-L32

File: OwnableProxyDelegation.sol
24:     function initialize(address ownerAddr) external {
25:         require(ownerAddr != address(0), "OPD: INVALID_ADDRESS");
26:         require(!initialized, "OPD: INITIALIZED");
27:         require(StorageSlot.getAddressSlot(_ADMIN_SLOT).value == msg.sender, "OPD: FORBIDDEN");
28: 
29:         _setOwner(ownerAddr);
30: 
31:         initialized = true;
32:     }

4. Use a constant instead of duplicating the same string

abstracts/OwnableProxyDelegation.sol:25:        require(ownerAddr != address(0), "OPD: INVALID_ADDRESS");
abstracts/OwnableProxyDelegation.sol:57:        require(newOwner != address(0), "OPD: INVALID_ADDRESS");

governance/TimelockControllerEmergency.sol:229:        require(targets.length == values.length, "TimelockController: length mismatch");
governance/TimelockControllerEmergency.sol:230:        require(targets.length == datas.length, "TimelockController: length mismatch");
governance/TimelockControllerEmergency.sol:319:        require(targets.length == values.length, "TimelockController: length mismatch");
governance/TimelockControllerEmergency.sol:320:        require(targets.length == datas.length, "TimelockController: length mismatch");

governance/TimelockControllerEmergency.sol:334:        require(isOperationReady(id), "TimelockController: operation is not ready");
governance/TimelockControllerEmergency.sol:342:        require(isOperationReady(id), "TimelockController: operation is not ready");

libraries/CurveHelpers/CurveHelpers.sol:28:        revert("CH: INVALID_INPUT_TOKEN");
libraries/CurveHelpers/CurveHelpers.sol:48:        revert("CH: INVALID_INPUT_TOKEN");
libraries/CurveHelpers/CurveHelpers.sol:68:        revert("CH: INVALID_INPUT_TOKEN");

libraries/StakingLPVaultHelpers.sol:108:        require(success, "SDCSO: CURVE_RM_LIQUIDITY_FAILED");
libraries/StakingLPVaultHelpers.sol:138:        require(success, "SDCSO: CURVE_RM_LIQUIDITY_FAILED");

operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:52:        require(amountToDeposit != 0, "BLVO: INVALID_AMOUNT");
operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:97:        require(amount != 0, "BLVO: INVALID_AMOUNT");

operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:54:        require(router != address(0), "BLVO: INVALID_VAULT");
operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:99:        require(router != address(0), "BLVO: INVALID_VAULT");
operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:187:        require(pair.factory() == biswapRouter.factory(), "BLVO: INVALID_VAULT");

operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:97:        require(amount != 0, "BLVO: INVALID_AMOUNT");
operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:52:        require(amountToDeposit != 0, "BLVO: INVALID_AMOUNT");

operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:271:        require(reserveA > 1000, "BLVO: PAIR_RESERVE_TOO_LOW");
operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:272:        require(reserveB > 1000, "BLVO: PAIR_RESERVE_TOO_LOW");

operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:54:        require(router != address(0), "BLVO: INVALID_VAULT");
operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:99:        require(router != address(0), "BLVO: INVALID_VAULT");

operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:269:        require(reserveA > 1000, "BLVO: PAIR_RESERVE_TOO_LOW");
operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:270:        require(reserveB > 1000, "BLVO: PAIR_RESERVE_TOO_LOW");

operators/Beefy/BeefyVaultOperator.sol:41:        require(amount != 0, "BVO: INVALID_AMOUNT");
operators/Beefy/BeefyVaultOperator.sol:83:        require(amount != 0, "BVO: INVALID_AMOUNT");
operators/Beefy/BeefyVaultOperator.sol:96:        require(tokenAmount != 0, "BVO: INVALID_AMOUNT");

operators/Beefy/BeefyVaultOperator.sol:43:        require(address(token) != address(0), "BVO: INVALID_VAULT");
operators/Beefy/BeefyVaultOperator.sol:85:        require(address(token) != address(0), "BVO: INVALID_VAULT");

operators/Yearn/YearnCurveVaultOperator.sol:70:        require(amount != 0, "YCVO: INVALID_AMOUNT");
operators/Yearn/YearnCurveVaultOperator.sol:121:        require(amount != 0, "YCVO: INVALID_AMOUNT");
operators/Yearn/YearnCurveVaultOperator.sol:164:        require(amount != 0, "YCVO: INVALID_AMOUNT");
operators/Yearn/YearnCurveVaultOperator.sol:212:        require(amount != 0, "YCVO: INVALID_AMOUNT");
operators/Yearn/YearnCurveVaultOperator.sol:260:        require(amount != 0, "YCVO: INVALID_AMOUNT");

operators/Yearn/YearnCurveVaultOperator.sol:73:        require(pool != address(0), "YCVO: INVALID_VAULT");
operators/Yearn/YearnCurveVaultOperator.sol:123:        require(pool != address(0), "YCVO: INVALID_VAULT");
operators/Yearn/YearnCurveVaultOperator.sol:167:        require(pool != address(0), "YCVO: INVALID_VAULT");
operators/Yearn/YearnCurveVaultOperator.sol:215:        require(pool != address(0), "YCVO: INVALID_VAULT");
operators/Yearn/YearnCurveVaultOperator.sol:263:        require(pool != address(0), "YCVO: INVALID_VAULT");

NestedFactory.sol:160:        require(_entryFees != 0, "NF: ZERO_FEES");
NestedFactory.sol:168:        require(_exitFees != 0, "NF: ZERO_FEES");

NestedFactory.sol:161:        require(_entryFees <= 10000, "NF: FEES_OVERFLOW");
NestedFactory.sol:169:        require(_exitFees <= 10000, "NF: FEES_OVERFLOW");

NestedFactory.sol:191:        require(batchedOrdersLength != 0, "NF: INVALID_MULTI_ORDERS");
NestedFactory.sol:312:        require(batchedOrdersLength != 0, "NF: INVALID_MULTI_ORDERS");
NestedFactory.sol:330:        require(batchedOrdersLength != 0, "NF: INVALID_MULTI_ORDERS");

NestedFactory.sol:250:        require(_orders.length != 0, "NF: INVALID_ORDERS");
NestedFactory.sol:359:        require(batchLength != 0, "NF: INVALID_ORDERS");
NestedFactory.sol:406:        require(batchLength != 0, "NF: INVALID_ORDERS");

NestedFactory.sol:251:        require(tokensLength == _orders.length, "NF: INPUTS_LENGTH_MUST_MATCH");
NestedFactory.sol:407:        require(_batchedOrders.amounts.length == batchLength, "NF: INPUTS_LENGTH_MUST_MATCH");

NestedFactory.sol:252:        require(nestedRecords.getAssetReserve(_nftId) == address(reserve), "NF: RESERVE_MISMATCH");
NestedFactory.sol:289:        require(nestedRecords.getAssetReserve(_nftId) == address(reserve), "NF: RESERVE_MISMATCH");
NestedFactory.sol:313:        require(nestedRecords.getAssetReserve(_nftId) == address(reserve), "NF: RESERVE_MISMATCH");
NestedFactory.sol:331:        require(nestedRecords.getAssetReserve(_nftId) == address(reserve), "NF: RESERVE_MISMATCH");

NestedFactory.sol:379:        require(amountSpent <= _inputTokenAmount - feesAmount, "NF: OVERSPENT");
NestedFactory.sol:428:            require(amountSpent <= _inputTokenAmount, "NF: OVERSPENT");
NestedFactory.sol:495:            require(amounts[1] <= _amountToSpend, "NF: OVERSPENT");

OperatorResolver.sol:39:        require(namesLength == destinations.length, "OR: INPUTS_LENGTH_MUST_MATCH");
OperatorResolver.sol:57:        require(names.length == operatorsToImport.length, "OR: INPUTS_LENGTH_MUST_MATCH");

5. Funds can be locked

There isn't a withdraw mechanism and several payable methods are implemented:

• BeefyZapBiswapLPVaultOperator.sol:

operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:51:    ) external payable returns (uint256[] memory amounts, address[] memory tokens) {

• YearnCurveVaultOperator.sol:

operators/Yearn/YearnCurveVaultOperator.sol:69:    ) external payable returns (uint256[] memory amounts, address[] memory tokens) {
operators/Yearn/YearnCurveVaultOperator.sol:120:    ) external payable returns (uint256[] memory amounts, address[] memory tokens) {
operators/Yearn/YearnCurveVaultOperator.sol:163:    ) external payable returns (uint256[] memory amounts, address[] memory tokens) {
operators/Yearn/YearnCurveVaultOperator.sol:211:    ) external payable returns (uint256[] memory amounts, address[] memory tokens) {
operators/Yearn/YearnCurveVaultOperator.sol:259:    ) external payable returns (uint256[] memory amounts, address[] memory tokens) {

6. A magic number should be documented and explained. Use a constant instead

Similar issue in the past: here

• 1:

operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:240:            1,
operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:251:            1,
operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:252:            1,
operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:240:            1,
operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:251:            1,
operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:252:            1,

• 10000:

NestedFactory.sol:378:        feesAmount = (amountSpent * entryFees) / 10000; // Entry Fees
NestedFactory.sol:443:            feesAmount = (amountBought * (_toReserve ? entryFees : exitFees)) / 10000;

I suggest using constant variables as this would make the code more maintainable and readable while costing nothing gas-wise (constants are replaced by their value at compile-time).

Non-Critical Issues

1. It's better to emit after all processing is done

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

contracts/governance/TimelockControllerEmergency.sol:
  374      function updateDelay(uint256 newDelay) external virtual {
  375          require(msg.sender == address(this), "TimelockController: caller must be timelock");
  376:         emit MinDelayChange(_minDelay, newDelay);
  377          _minDelay = newDelay;
  378      }

2. Typos

• datas vs data

abstracts/MixinOperatorResolver.sol:81:    /// @dev Build the calldata (with safe datas) and call the Operator

• setted vs set

- abstracts/OwnableProxyDelegation.sol:17:    /// @dev True if the owner is setted
+ abstracts/OwnableProxyDelegation.sol:17:    /// @dev True if the owner is set

• liquitiy vs liquidity

libraries/StakingLPVaultHelpers.sol:21:    /// @param pool The Curve pool to add liquitiy in
libraries/StakingLPVaultHelpers.sol:52:    /// @param pool The Curve pool to add liquitiy in
libraries/StakingLPVaultHelpers.sol:85:    /// @param pool The Curve pool to remove liquitiy from
libraries/StakingLPVaultHelpers.sol:115:    /// @param pool The Curve pool to remove liquitiy from

• WITHDRAWED vs WITHDREW or WITHDRAWN

operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:108:        require(vaultAmount == amount, "BLVO: INVALID_AMOUNT_WITHDRAWED");
operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:108:        require(vaultAmount == amount, "BLVO: INVALID_AMOUNT_WITHDRAWED");
operators/Beefy/BeefyVaultOperator.sol:95:        require(vaultAmount == amount, "BVO: INVALID_AMOUNT_WITHDRAWED");
NestedFactory.sol:51:    /// @dev Fees when funds are withdrawed
NestedFactory.sol:639:    /// @return The withdrawed amount from the reserve

• dont vs don't

NestedFactory.sol:477:    /// @dev Call the operator to submit the order but dont stop if the call to the operator fail.

• transfered vs transferred

NestedFactory.sol:534:    /// @return Token transfered (in case of ETH)

3. Adding a return statement when the function defines a named return variable, is redundant

While not consuming more gas with the Optimizer enabled: using both named returns and a return statement isn't necessary. Removing one of those can improve code clarity.

Affected code:

contracts/governance/TimelockControllerEmergency.sol:
  119:     function isOperation(bytes32 id) public view virtual returns (bool pending) {
  126:     function isOperationPending(bytes32 id) public view virtual returns (bool pending) {
  133:     function isOperationReady(bytes32 id) public view virtual returns (bool ready) {
  141:     function isOperationDone(bytes32 id) public view virtual returns (bool done) {
  149:     function getTimestamp(bytes32 id) public view virtual returns (uint256 timestamp) {
  158:     function getMinDelay() public view virtual returns (uint256 duration) {

contracts/libraries/CurveHelpers/CurveHelpers.sol:
  21:     ) internal view returns (uint256[2] memory amounts) {
  41:     ) internal view returns (uint256[3] memory amounts) {
  61:     ) internal view returns (uint256[4] memory amounts) {
  85:     ) internal returns (bool success) {

4. public functions not called by the contract should be declared external instead

governance/OwnerProxy.sol:16:    function execute(address _target, bytes memory _data) public payable onlyOwner returns (bytes memory response) {

TimelockControllerEmergency.sol, hashOperation
TimelockControllerEmergency.sol, hashOperationBatch

Table of Contents:

• 1. Cheap Contract Deployment Through Clones
• 2. Reduce the size of error messages (Long revert Strings)
• 3. Splitting require() statements that use && saves gas
• 4. Using private rather than public for constants saves gas
• 5. Use shift right/left instead of division/multiplication if possible
• 6. <array>.length should not be looked up in every loop of a for-loop
• 7. ++i costs less gas compared to i++ or i += 1 (same for --i vs i-- or i -= 1)
• 8. Increments/decrements can be unchecked in for-loops
• 9. It costs more gas to initialize variables with their default value than letting the default value be applied
• 10. Use Custom Errors instead of Revert Strings to save Gas
• 11. Functions guaranteed to revert when called by normal users can be marked payable

1. Cheap Contract Deployment Through Clones

operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:25:        operatorStorage = new BeefyVaultStorage();
operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:25:        operatorStorage = new BeefyVaultStorage();
operators/Yearn/YearnCurveVaultOperator.sol:40:        operatorStorage = new YearnVaultStorage();

There's a way to save a significant amount of gas on deployment using Clones: https://www.youtube.com/watch?v=3Mw-pMmJ7TA .

This is a solution that was adopted, as an example, by Porter Finance. They realized that deploying using clones was 10x cheaper:

• https://github.com/porter-finance/v1-core/issues/15#issuecomment-1035639516
• https://github.com/porter-finance/v1-core/pull/34

I suggest applying a similar pattern.

2. Reduce the size of error messages (Long revert Strings)

Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition is met.

Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc.

Revert strings > 32 bytes:

governance/TimelockControllerEmergency.sol:229:        require(targets.length == values.length, "TimelockController: length mismatch");
governance/TimelockControllerEmergency.sol:230:        require(targets.length == datas.length, "TimelockController: length mismatch");
governance/TimelockControllerEmergency.sol:243:        require(!isOperation(id), "TimelockController: operation already scheduled");
governance/TimelockControllerEmergency.sol:244:        require(delay >= getMinDelay(), "TimelockController: insufficient delay");
governance/TimelockControllerEmergency.sol:256:        require(isOperationPending(id), "TimelockController: operation cannot be cancelled");
governance/TimelockControllerEmergency.sol:319:        require(targets.length == values.length, "TimelockController: length mismatch");
governance/TimelockControllerEmergency.sol:320:        require(targets.length == datas.length, "TimelockController: length mismatch");
governance/TimelockControllerEmergency.sol:334:        require(isOperationReady(id), "TimelockController: operation is not ready");
governance/TimelockControllerEmergency.sol:335:        require(predecessor == bytes32(0) || isOperationDone(predecessor), "TimelockController: missing dependency");
governance/TimelockControllerEmergency.sol:342:        require(isOperationReady(id), "TimelockController: operation is not ready");
governance/TimelockControllerEmergency.sol:359:        require(success, "TimelockController: underlying transaction reverted");
governance/TimelockControllerEmergency.sol:375:        require(msg.sender == address(this), "TimelockController: caller must be timelock");

I suggest shortening the revert strings to fit in 32 bytes.

3. Splitting require() statements that use && saves gas

If you're using the Optimizer at 200, instead of using the && operator in a single require statement to check multiple conditions, I suggest using multiple require statements with 1 condition per require statement:

operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:64:        require(vaultAmount != 0 && vaultAmount >= minVaultAmount, "BLVO: INVALID_AMOUNT_RECEIVED");
operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:65:        require(depositedAmount != 0 && amountToDeposit >= depositedAmount, "BLVO: INVALID_AMOUNT_DEPOSITED");
operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:64:        require(vaultAmount != 0 && vaultAmount >= minVaultAmount, "BLVO: INVALID_AMOUNT_RECEIVED");
operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:65:        require(depositedAmount != 0 && amountToDeposit >= depositedAmount, "BLVO: INVALID_AMOUNT_DEPOSITED");
operators/Beefy/BeefyVaultOperator.sol:54:        require(vaultAmount != 0 && vaultAmount >= minVaultAmount, "BVO: INVALID_AMOUNT_RECEIVED");
operators/Paraswap/ParaswapOperator.sol:16:        require(_tokenTransferProxy != address(0) && _augustusSwapper != address(0), "PSO: INVALID_ADDRESS");
NestedFactory.sol:67:            address(_nestedAsset) != address(0) &&
NestedFactory.sol:68:                address(_nestedRecords) != address(0) &&
NestedFactory.sol:69:                address(_reserve) != address(0) &&
NestedFactory.sol:70:                address(_feeSplitter) != address(0) &&
NestedFactory.sol:71:                address(_weth) != address(0) &&
NestedFactory.sol:72:                _operatorResolver != address(0) &&

Please, note that this might not hold true at a higher number of runs for the Optimizer (10k). However, it indeed is true at 200.

4. Using private rather than public for constants saves gas

If needed, the value can be read from the verified contract source code. Savings are due to the compiler not having to create non-payable getter functions for deployment calldata, and not adding another entry to the method ID table

governance/TimelockControllerEmergency.sol:25:    bytes32 public constant TIMELOCK_ADMIN_ROLE = keccak256("TIMELOCK_ADMIN_ROLE");
governance/TimelockControllerEmergency.sol:26:    bytes32 public constant PROPOSER_ROLE = keccak256("PROPOSER_ROLE");
governance/TimelockControllerEmergency.sol:27:    bytes32 public constant EXECUTOR_ROLE = keccak256("EXECUTOR_ROLE");
governance/TimelockControllerEmergency.sol:28:    bytes32 public constant EMERGENCY_ROLE = keccak256("EMERGENCY_ROLE"); 

5. Use shift right/left instead of division/multiplication if possible

While the DIV / MUL opcode uses 5 gas, the SHR / SHL opcode only uses 3 gas. Furthermore, beware that Solidity's division operation also includes a division-by-0 prevention which is bypassed using shifting. Eventually, overflow checks are never performed for shift operations as they are done for arithmetic operations. Instead, the result is always truncated.

• Use >> 1 instead of / 2
• Use >> 2 instead of / 4
• Use << 3 instead of * 8

Affected code:

operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:275:        uint256 halfInvestment = investmentA / 2;
operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:273:        uint256 halfInvestment = investmentA / 2;

6. <array>.length should not be looked up in every loop of a for-loop

Reading array length at each iteration of the loop consumes more gas than necessary.

In the best case scenario (length read on a memory variable), caching the array length in the stack saves around 3 gas per iteration. In the worst case scenario (external calls at each iteration), the amount of gas wasted can be massive.

Here, I suggest storing the array's length in a variable before the for-loop, and use this new variable instead:

abstracts/MixinOperatorResolver.sol:37:        for (uint256 i = 0; i < requiredOperators.length; i++) {
abstracts/MixinOperatorResolver.sol:56:        for (uint256 i = 0; i < requiredOperators.length; i++) {
governance/TimelockControllerEmergency.sol:84:        for (uint256 i = 0; i < proposers.length; ++i) {
governance/TimelockControllerEmergency.sol:89:        for (uint256 i = 0; i < executors.length; ++i) {
governance/TimelockControllerEmergency.sol:234:        for (uint256 i = 0; i < targets.length; ++i) {
governance/TimelockControllerEmergency.sol:324:        for (uint256 i = 0; i < targets.length; ++i) {
NestedFactory.sol:124:        for (uint256 i = 0; i < operatorsCache.length; i++) {
NestedFactory.sol:651:        for (uint256 i = 0; i < _batchedOrders.length; i++) {
OperatorResolver.sol:60:        for (uint256 i = 0; i < names.length; i++) {
OperatorResolver.sol:75:        for (uint256 i = 0; i < destinations.length; i++) {

7. ++i costs less gas compared to i++ or i += 1 (same for --i vs i-- or i -= 1)

Pre-increments and pre-decrements are cheaper.

For a uint256 i variable, the following is true with the Optimizer enabled at 10k:

Increment:

• i += 1 is the most expensive form
• i++ costs 6 gas less than i += 1
• ++i costs 5 gas less than i++ (11 gas less than i += 1)

Decrement:

• i -= 1 is the most expensive form
• i-- costs 11 gas less than i -= 1
• --i costs 5 gas less than i-- (16 gas less than i -= 1)

Note that post-increments (or post-decrements) return the old value before incrementing or decrementing, hence the name post-increment:

uint i = 1;  
uint j = 2;
require(j == i++, "This will be false as i is incremented after the comparison");

However, pre-increments (or pre-decrements) return the new value:

uint i = 1;  
uint j = 2;
require(j == ++i, "This will be true as i is incremented before the comparison");

In the pre-increment case, the compiler has to create a temporary variable (when used) for returning 1 instead of 2.

Affected code:

abstracts/MixinOperatorResolver.sol:37:        for (uint256 i = 0; i < requiredOperators.length; i++) {
abstracts/MixinOperatorResolver.sol:56:        for (uint256 i = 0; i < requiredOperators.length; i++) {
governance/scripts/OperatorScripts.sol:67:        for (uint256 i; i < operatorLength; i++) {
governance/scripts/OperatorScripts.sol:80:        for (uint256 i; i < operatorLength; i++) {
libraries/CurveHelpers/CurveHelpers.sol:22:        for (uint256 i; i < 2; i++) {
libraries/CurveHelpers/CurveHelpers.sol:42:        for (uint256 i; i < 3; i++) {
libraries/CurveHelpers/CurveHelpers.sol:62:        for (uint256 i; i < 4; i++) {
libraries/CurveHelpers/CurveHelpers.sol:86:        for (uint256 i; i < poolCoinAmount; i++) {
operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:27:        for (uint256 i; i < vaultsLength; i++) {
operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:27:        for (uint256 i; i < vaultsLength; i++) {
operators/Beefy/BeefyVaultOperator.sol:18:        for (uint256 i; i < vaultsLength; i++) {
operators/Yearn/YearnCurveVaultOperator.sol:42:        for (uint256 i; i < vaultsLength; i++) {
utils/NestedAssetBatcher.sol:45:            for (uint256 i; i < numTokens; i++) {
utils/NestedAssetBatcher.sol:60:            for (uint256 i; i < numTokens; i++) {
utils/NestedAssetBatcher.sol:74:            for (uint256 i; i < numTokens; i++) {
utils/NestedAssetBatcher.sol:79:                for (uint256 j; j < tokenLength; j++) {
NestedFactory.sol:124:        for (uint256 i = 0; i < operatorsCache.length; i++) {
NestedFactory.sol:136:        for (uint256 i = 0; i < operatorsLength; i++) {
NestedFactory.sol:196:        for (uint256 i = 0; i < batchedOrdersLength; i++) {
NestedFactory.sol:256:        for (uint256 i = 0; i < tokensLength; i++) {
NestedFactory.sol:315:        for (uint256 i = 0; i < batchedOrdersLength; i++) {
NestedFactory.sol:333:        for (uint256 i = 0; i < batchedOrdersLength; i++) {
NestedFactory.sol:369:        for (uint256 i = 0; i < batchLength; i++) {
NestedFactory.sol:412:        for (uint256 i = 0; i < batchLength; i++) {
NestedFactory.sol:651:        for (uint256 i = 0; i < _batchedOrders.length; i++) {
OperatorResolver.sol:40:        for (uint256 i = 0; i < namesLength; i++) {
OperatorResolver.sol:60:        for (uint256 i = 0; i < names.length; i++) {
OperatorResolver.sol:75:        for (uint256 i = 0; i < destinations.length; i++) {

Consider using pre-increments and pre-decrements where they are relevant (meaning: not where post-increments/decrements logic are relevant).

8. Increments/decrements can be unchecked in for-loops

In Solidity 0.8+, there's a default overflow check on unsigned integers. It's possible to uncheck this in for-loops and save some gas at each iteration, but at the cost of some code readability, as this uncheck cannot be made inline.

ethereum/solidity#10695

Affected code:

abstracts/MixinOperatorResolver.sol:37:        for (uint256 i = 0; i < requiredOperators.length; i++) {
abstracts/MixinOperatorResolver.sol:56:        for (uint256 i = 0; i < requiredOperators.length; i++) {
governance/scripts/OperatorScripts.sol:67:        for (uint256 i; i < operatorLength; i++) {
governance/scripts/OperatorScripts.sol:80:        for (uint256 i; i < operatorLength; i++) {
governance/TimelockControllerEmergency.sol:84:        for (uint256 i = 0; i < proposers.length; ++i) {
governance/TimelockControllerEmergency.sol:89:        for (uint256 i = 0; i < executors.length; ++i) {
governance/TimelockControllerEmergency.sol:234:        for (uint256 i = 0; i < targets.length; ++i) {
governance/TimelockControllerEmergency.sol:324:        for (uint256 i = 0; i < targets.length; ++i) {
libraries/CurveHelpers/CurveHelpers.sol:22:        for (uint256 i; i < 2; i++) {
libraries/CurveHelpers/CurveHelpers.sol:42:        for (uint256 i; i < 3; i++) {
libraries/CurveHelpers/CurveHelpers.sol:62:        for (uint256 i; i < 4; i++) {
libraries/CurveHelpers/CurveHelpers.sol:86:        for (uint256 i; i < poolCoinAmount; i++) {
operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:27:        for (uint256 i; i < vaultsLength; i++) {
operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:27:        for (uint256 i; i < vaultsLength; i++) {
operators/Beefy/BeefyVaultOperator.sol:18:        for (uint256 i; i < vaultsLength; i++) {
operators/Yearn/YearnCurveVaultOperator.sol:42:        for (uint256 i; i < vaultsLength; i++) {
utils/NestedAssetBatcher.sol:45:            for (uint256 i; i < numTokens; i++) {
utils/NestedAssetBatcher.sol:60:            for (uint256 i; i < numTokens; i++) {
utils/NestedAssetBatcher.sol:74:            for (uint256 i; i < numTokens; i++) {
utils/NestedAssetBatcher.sol:79:                for (uint256 j; j < tokenLength; j++) {
NestedFactory.sol:124:        for (uint256 i = 0; i < operatorsCache.length; i++) {
NestedFactory.sol:136:        for (uint256 i = 0; i < operatorsLength; i++) {
NestedFactory.sol:196:        for (uint256 i = 0; i < batchedOrdersLength; i++) {
NestedFactory.sol:256:        for (uint256 i = 0; i < tokensLength; i++) {
NestedFactory.sol:315:        for (uint256 i = 0; i < batchedOrdersLength; i++) {
NestedFactory.sol:333:        for (uint256 i = 0; i < batchedOrdersLength; i++) {
NestedFactory.sol:369:        for (uint256 i = 0; i < batchLength; i++) {
NestedFactory.sol:412:        for (uint256 i = 0; i < batchLength; i++) {
NestedFactory.sol:651:        for (uint256 i = 0; i < _batchedOrders.length; i++) {
OperatorResolver.sol:40:        for (uint256 i = 0; i < namesLength; i++) {
OperatorResolver.sol:60:        for (uint256 i = 0; i < names.length; i++) {
OperatorResolver.sol:75:        for (uint256 i = 0; i < destinations.length; i++) {

The change would be:

- for (uint256 i; i < numIterations; i++) {
+ for (uint256 i; i < numIterations;) {
 // ...  
+   unchecked { ++i; }
}  

The same can be applied with decrements (which should use break when i == 0).

The risk of overflow is non-existant for uint256 here.

9. It costs more gas to initialize variables with their default value than letting the default value be applied

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.

As an example: for (uint256 i = 0; i < numIterations; ++i) { should be replaced with for (uint256 i; i < numIterations; ++i) {

Affected code:

abstracts/MixinOperatorResolver.sol:37:        for (uint256 i = 0; i < requiredOperators.length; i++) {
abstracts/MixinOperatorResolver.sol:56:        for (uint256 i = 0; i < requiredOperators.length; i++) {
governance/TimelockControllerEmergency.sol:84:        for (uint256 i = 0; i < proposers.length; ++i) {
governance/TimelockControllerEmergency.sol:89:        for (uint256 i = 0; i < executors.length; ++i) {
governance/TimelockControllerEmergency.sol:234:        for (uint256 i = 0; i < targets.length; ++i) {
governance/TimelockControllerEmergency.sol:324:        for (uint256 i = 0; i < targets.length; ++i) {
NestedFactory.sol:124:        for (uint256 i = 0; i < operatorsCache.length; i++) {
NestedFactory.sol:136:        for (uint256 i = 0; i < operatorsLength; i++) {
NestedFactory.sol:196:        for (uint256 i = 0; i < batchedOrdersLength; i++) {
NestedFactory.sol:256:        for (uint256 i = 0; i < tokensLength; i++) {
NestedFactory.sol:315:        for (uint256 i = 0; i < batchedOrdersLength; i++) {
NestedFactory.sol:333:        for (uint256 i = 0; i < batchedOrdersLength; i++) {
NestedFactory.sol:369:        for (uint256 i = 0; i < batchLength; i++) {
NestedFactory.sol:412:        for (uint256 i = 0; i < batchLength; i++) {
NestedFactory.sol:651:        for (uint256 i = 0; i < _batchedOrders.length; i++) {
OperatorResolver.sol:40:        for (uint256 i = 0; i < namesLength; i++) {
OperatorResolver.sol:60:        for (uint256 i = 0; i < names.length; i++) {
OperatorResolver.sol:75:        for (uint256 i = 0; i < destinations.length; i++) {

I suggest removing explicit initializations for default values.

10. Use Custom Errors instead of Revert Strings to save Gas

Solidity 0.8.4 introduced custom errors. They are more gas efficient than revert strings, when it comes to deploy cost as well as runtime cost when the revert condition is met. Use custom errors instead of revert strings for gas savings.

Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met)

Source: https://blog.soliditylang.org/2021/04/21/custom-errors/:

Starting from Solidity v0.8.4, there is a convenient and gas-efficient way to explain to users why an operation failed through the use of custom errors. Until now, you could already use strings to give more information about failures (e.g., revert("Insufficient funds.");), but they are rather expensive, especially when it comes to deploy cost, and it is difficult to use dynamic information in them.

Custom errors are defined using the error statement, which can be used inside and outside of contracts (including interfaces and libraries).

Consider replacing all revert strings with custom errors in the solution.

abstracts/MixinOperatorResolver.sol:23:        require(_resolver != address(0), "MOR: INVALID_ADDRESS");
abstracts/MixinOperatorResolver.sol:77:        require(_foundAddress.implementation != address(0), string(abi.encodePacked("MOR: MISSING_OPERATOR: ", name)));
abstracts/MixinOperatorResolver.sol:103:            require(tokens[0] == _outputToken, "MOR: INVALID_OUTPUT_TOKEN");
abstracts/MixinOperatorResolver.sol:104:            require(tokens[1] == _inputToken, "MOR: INVALID_INPUT_TOKEN");
abstracts/OwnableProxyDelegation.sol:25:        require(ownerAddr != address(0), "OPD: INVALID_ADDRESS");
abstracts/OwnableProxyDelegation.sol:26:        require(!initialized, "OPD: INITIALIZED");
abstracts/OwnableProxyDelegation.sol:27:        require(StorageSlot.getAddressSlot(_ADMIN_SLOT).value == msg.sender, "OPD: FORBIDDEN");
abstracts/OwnableProxyDelegation.sol:41:        require(owner() == _msgSender(), "OPD: NOT_OWNER");
abstracts/OwnableProxyDelegation.sol:57:        require(newOwner != address(0), "OPD: INVALID_ADDRESS");
governance/scripts/OperatorScripts.sol:19:        require(_nestedFactory != address(0), "AO-SCRIPT: INVALID_FACTORY_ADDR");
governance/scripts/OperatorScripts.sol:20:        require(_resolver != address(0), "AO-SCRIPT: INVALID_RESOLVER_ADDR");
governance/scripts/OperatorScripts.sol:29:        require(operator.implementation != address(0), "AO-SCRIPT: INVALID_IMPL_ADDRESS");
governance/scripts/OperatorScripts.sol:54:        require(operatorLength != 0, "DAO-SCRIPT: INVALID_OPERATOR_LEN");
governance/scripts/OperatorScripts.sol:55:        require(bytecode.length != 0, "DAO-SCRIPT: BYTECODE_ZERO");
governance/scripts/OperatorScripts.sol:61:        require(deployedAddress != address(0), "DAO-SCRIPT: FAILED_DEPLOY");
governance/OwnerProxy.sol:17:        require(_target != address(0), "OP: INVALID_TARGET");
governance/TimelockControllerEmergency.sol:229:        require(targets.length == values.length, "TimelockController: length mismatch");
governance/TimelockControllerEmergency.sol:230:        require(targets.length == datas.length, "TimelockController: length mismatch");
governance/TimelockControllerEmergency.sol:243:        require(!isOperation(id), "TimelockController: operation already scheduled");
governance/TimelockControllerEmergency.sol:244:        require(delay >= getMinDelay(), "TimelockController: insufficient delay");
governance/TimelockControllerEmergency.sol:256:        require(isOperationPending(id), "TimelockController: operation cannot be cancelled");
governance/TimelockControllerEmergency.sol:319:        require(targets.length == values.length, "TimelockController: length mismatch");
governance/TimelockControllerEmergency.sol:320:        require(targets.length == datas.length, "TimelockController: length mismatch");
governance/TimelockControllerEmergency.sol:334:        require(isOperationReady(id), "TimelockController: operation is not ready");
governance/TimelockControllerEmergency.sol:335:        require(predecessor == bytes32(0) || isOperationDone(predecessor), "TimelockController: missing dependency");
governance/TimelockControllerEmergency.sol:342:        require(isOperationReady(id), "TimelockController: operation is not ready");
governance/TimelockControllerEmergency.sol:359:        require(success, "TimelockController: underlying transaction reverted");
governance/TimelockControllerEmergency.sol:375:        require(msg.sender == address(this), "TimelockController: caller must be timelock");
libraries/StakingLPVaultHelpers.sol:108:        require(success, "SDCSO: CURVE_RM_LIQUIDITY_FAILED");
libraries/StakingLPVaultHelpers.sol:138:        require(success, "SDCSO: CURVE_RM_LIQUIDITY_FAILED");
operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:23:        require(vaultsLength == routers.length, "BLVO: INVALID_VAULTS_LENGTH");
operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:52:        require(amountToDeposit != 0, "BLVO: INVALID_AMOUNT");
operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:54:        require(router != address(0), "BLVO: INVALID_VAULT");
operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:64:        require(vaultAmount != 0 && vaultAmount >= minVaultAmount, "BLVO: INVALID_AMOUNT_RECEIVED");
operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:65:        require(depositedAmount != 0 && amountToDeposit >= depositedAmount, "BLVO: INVALID_AMOUNT_DEPOSITED");
operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:97:        require(amount != 0, "BLVO: INVALID_AMOUNT");
operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:99:        require(router != address(0), "BLVO: INVALID_VAULT");
operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:108:        require(vaultAmount == amount, "BLVO: INVALID_AMOUNT_WITHDRAWED");
operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:109:        require(tokenAmount >= minTokenAmount, "BLVO: INVALID_OUTPUT_AMOUNT");
operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:142:        require(token0 == token || token1 == token, "BLVO: INVALID_TOKEN");
operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:187:        require(pair.factory() == biswapRouter.factory(), "BLVO: INVALID_VAULT");
operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:198:        require(isInput0 || cachedToken1 == token, "BLVO: INVALID_INPUT_TOKEN");
operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:271:        require(reserveA > 1000, "BLVO: PAIR_RESERVE_TOO_LOW");
operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol:272:        require(reserveB > 1000, "BLVO: PAIR_RESERVE_TOO_LOW");
operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:23:        require(vaultsLength == routers.length, "BLVO: INVALID_VAULTS_LENGTH");
operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:52:        require(amountToDeposit != 0, "BLVO: INVALID_AMOUNT");
operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:54:        require(router != address(0), "BLVO: INVALID_VAULT");
operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:64:        require(vaultAmount != 0 && vaultAmount >= minVaultAmount, "BLVO: INVALID_AMOUNT_RECEIVED");
operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:65:        require(depositedAmount != 0 && amountToDeposit >= depositedAmount, "BLVO: INVALID_AMOUNT_DEPOSITED");
operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:97:        require(amount != 0, "BLVO: INVALID_AMOUNT");
operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:99:        require(router != address(0), "BLVO: INVALID_VAULT");
operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:108:        require(vaultAmount == amount, "BLVO: INVALID_AMOUNT_WITHDRAWED");
operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:109:        require(tokenAmount >= minTokenAmount, "BLVO: INVALID_OUTPUT_AMOUNT");
operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:142:        require(token0 == token || token1 == token, "BLVO: INVALID_TOKEN");
operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:187:        require(pair.factory() == uniswapRouter.factory(), "BLVO: INVALID_VAULT");
operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:198:        require(isInput0 || cachedToken1 == token, "BLVO: INVALID_INPUT_TOKEN");
operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:269:        require(reserveA > 1000, "BLVO: PAIR_RESERVE_TOO_LOW");
operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol:270:        require(reserveB > 1000, "BLVO: PAIR_RESERVE_TOO_LOW");
operators/Beefy/BeefyVaultOperator.sol:15:        require(vaultsLength == tokens.length, "BVO: INVALID_VAULTS_LENGTH");
operators/Beefy/BeefyVaultOperator.sol:41:        require(amount != 0, "BVO: INVALID_AMOUNT");
operators/Beefy/BeefyVaultOperator.sol:43:        require(address(token) != address(0), "BVO: INVALID_VAULT");
operators/Beefy/BeefyVaultOperator.sol:50:        require(success, "BVO: DEPOSIT_CALL_FAILED");
operators/Beefy/BeefyVaultOperator.sol:54:        require(vaultAmount != 0 && vaultAmount >= minVaultAmount, "BVO: INVALID_AMOUNT_RECEIVED");
operators/Beefy/BeefyVaultOperator.sol:55:        require(amount == tokenAmount, "BVO: INVALID_AMOUNT_DEPOSITED");
operators/Beefy/BeefyVaultOperator.sol:83:        require(amount != 0, "BVO: INVALID_AMOUNT");
operators/Beefy/BeefyVaultOperator.sol:85:        require(address(token) != address(0), "BVO: INVALID_VAULT");
operators/Beefy/BeefyVaultOperator.sol:91:        require(success, "BVO: WITHDRAW_CALL_FAILED");
operators/Beefy/BeefyVaultOperator.sol:95:        require(vaultAmount == amount, "BVO: INVALID_AMOUNT_WITHDRAWED");
operators/Beefy/BeefyVaultOperator.sol:96:        require(tokenAmount != 0, "BVO: INVALID_AMOUNT");
operators/Beefy/BeefyVaultStorage.sol:25:        require(vault != address(0), "BVS: INVALID_VAULT_ADDRESS");
operators/Beefy/BeefyVaultStorage.sol:26:        require(tokenOrZapper != address(0), "BVS: INVALID_UNDERLYING_ADDRESS");
operators/Beefy/BeefyVaultStorage.sol:27:        require(vaults[vault] == address(0), "BVS: ALREADY_EXISTENT_VAULT");
operators/Beefy/BeefyVaultStorage.sol:35:        require(vaults[vault] != address(0), "BVS: NON_EXISTENT_VAULT");
operators/Paraswap/ParaswapOperator.sol:16:        require(_tokenTransferProxy != address(0) && _augustusSwapper != address(0), "PSO: INVALID_ADDRESS");
operators/Paraswap/ParaswapOperator.sol:27:        require(sellToken != buyToken, "PSO: SAME_INPUT_OUTPUT");
operators/Paraswap/ParaswapOperator.sol:35:        require(success, "PSO: SWAP_FAILED");
operators/Paraswap/ParaswapOperator.sol:39:        require(amountBought != 0, "PSO: INVALID_AMOUNT_BOUGHT");
operators/Paraswap/ParaswapOperator.sol:40:        require(amountSold != 0, "PSO: INVALID_AMOUNT_SOLD");
operators/Yearn/YearnCurveVaultOperator.sol:39:        require(vaultsLength == pools.length, "YCVO: INVALID_VAULTS_LENGTH");
operators/Yearn/YearnCurveVaultOperator.sol:70:        require(amount != 0, "YCVO: INVALID_AMOUNT");
operators/Yearn/YearnCurveVaultOperator.sol:73:        require(pool != address(0), "YCVO: INVALID_VAULT");
operators/Yearn/YearnCurveVaultOperator.sol:121:        require(amount != 0, "YCVO: INVALID_AMOUNT");
operators/Yearn/YearnCurveVaultOperator.sol:123:        require(pool != address(0), "YCVO: INVALID_VAULT");
operators/Yearn/YearnCurveVaultOperator.sol:164:        require(amount != 0, "YCVO: INVALID_AMOUNT");
operators/Yearn/YearnCurveVaultOperator.sol:167:        require(pool != address(0), "YCVO: INVALID_VAULT");
operators/Yearn/YearnCurveVaultOperator.sol:212:        require(amount != 0, "YCVO: INVALID_AMOUNT");
operators/Yearn/YearnCurveVaultOperator.sol:215:        require(pool != address(0), "YCVO: INVALID_VAULT");
operators/Yearn/YearnCurveVaultOperator.sol:260:        require(amount != 0, "YCVO: INVALID_AMOUNT");
operators/Yearn/YearnCurveVaultOperator.sol:263:        require(pool != address(0), "YCVO: INVALID_VAULT");
operators/Yearn/YearnVaultStorage.sol:30:        require(vault != address(0), "YVS: INVALID_VAULT_ADDRESS");
operators/Yearn/YearnVaultStorage.sol:31:        require(curvePool.poolAddress != address(0), "YVS: INVALID_POOL_ADDRESS");
operators/Yearn/YearnVaultStorage.sol:32:        require(curvePool.lpToken != address(0), "YVS: INVALID_TOKEN_ADDRESS");
operators/Yearn/YearnVaultStorage.sol:33:        require(vaults[vault].poolAddress == address(0), "YVS: VAULT_ALREADY_HAS_POOL");
operators/Yearn/YearnVaultStorage.sol:34:        require(vaults[vault].lpToken == address(0), "YVS: VAULT_ALREADY_HAS_LP");
operators/Yearn/YearnVaultStorage.sol:42:        require(vaults[vault].poolAddress != address(0), "YVS: NON_EXISTENT_VAULT");
utils/NestedAssetBatcher.sol:94:            require(nestedAsset.lastOwnerBeforeBurn(_nftId) != address(0), "NAB: NEVER_CREATED");
NestedFactory.sol:66:        require(
NestedFactory.sol:99:        require(nestedAsset.ownerOf(_nftId) == _msgSender(), "NF: CALLER_NOT_OWNER");
NestedFactory.sol:107:        require(block.timestamp > nestedRecords.getLockTimestamp(_nftId), "NF: LOCKED_NFT");
NestedFactory.sol:122:        require(operator != bytes32(""), "NF: INVALID_OPERATOR_NAME");
NestedFactory.sol:125:            require(operatorsCache[i] != operator, "NF: EXISTENT_OPERATOR");
NestedFactory.sol:153:        require(address(_feeSplitter) != address(0), "NF: INVALID_FEE_SPLITTER_ADDRESS");
NestedFactory.sol:160:        require(_entryFees != 0, "NF: ZERO_FEES");
NestedFactory.sol:161:        require(_entryFees <= 10000, "NF: FEES_OVERFLOW");
NestedFactory.sol:168:        require(_exitFees != 0, "NF: ZERO_FEES");
NestedFactory.sol:169:        require(_exitFees <= 10000, "NF: FEES_OVERFLOW");
NestedFactory.sol:191:        require(batchedOrdersLength != 0, "NF: INVALID_MULTI_ORDERS");
NestedFactory.sol:250:        require(_orders.length != 0, "NF: INVALID_ORDERS");
NestedFactory.sol:251:        require(tokensLength == _orders.length, "NF: INPUTS_LENGTH_MUST_MATCH");
NestedFactory.sol:252:        require(nestedRecords.getAssetReserve(_nftId) == address(reserve), "NF: RESERVE_MISMATCH");
NestedFactory.sol:286:        require(assetTokensLength > _tokenIndex, "NF: INVALID_TOKEN_INDEX");
NestedFactory.sol:288:        require(assetTokensLength > 1, "NF: UNALLOWED_EMPTY_PORTFOLIO");
NestedFactory.sol:289:        require(nestedRecords.getAssetReserve(_nftId) == address(reserve), "NF: RESERVE_MISMATCH");
NestedFactory.sol:312:        require(batchedOrdersLength != 0, "NF: INVALID_MULTI_ORDERS");
NestedFactory.sol:313:        require(nestedRecords.getAssetReserve(_nftId) == address(reserve), "NF: RESERVE_MISMATCH");
NestedFactory.sol:330:        require(batchedOrdersLength != 0, "NF: INVALID_MULTI_ORDERS");
NestedFactory.sol:331:        require(nestedRecords.getAssetReserve(_nftId) == address(reserve), "NF: RESERVE_MISMATCH");
NestedFactory.sol:359:        require(batchLength != 0, "NF: INVALID_ORDERS");
NestedFactory.sol:379:        require(amountSpent <= _inputTokenAmount - feesAmount, "NF: OVERSPENT");
NestedFactory.sol:406:        require(batchLength != 0, "NF: INVALID_ORDERS");
NestedFactory.sol:407:        require(_batchedOrders.amounts.length == batchLength, "NF: INPUTS_LENGTH_MUST_MATCH");
NestedFactory.sol:428:            require(amountSpent <= _inputTokenAmount, "NF: OVERSPENT");
NestedFactory.sol:469:        require(success, "NF: OPERATOR_CALL_FAILED");
NestedFactory.sol:495:            require(amounts[1] <= _amountToSpend, "NF: OVERSPENT");
NestedFactory.sol:543:            require(!_fromReserve, "NF: NO_ETH_FROM_RESERVE");
NestedFactory.sol:544:            require(address(this).balance >= _inputTokenAmount, "NF: INVALID_AMOUNT_IN");
NestedFactory.sol:551:            require(
NestedFactory.sol:612:            require(success, "NF: ETH_TRANSFER_ERROR");
NestedFactory.sol:656:        require(msg.value == ethNeeded, "NF: WRONG_MSG_VALUE");
OperatorResolver.sol:27:        require(_foundOperator.implementation != address(0), reason);
OperatorResolver.sol:39:        require(namesLength == destinations.length, "OR: INPUTS_LENGTH_MUST_MATCH");
OperatorResolver.sol:57:        require(names.length == operatorsToImport.length, "OR: INPUTS_LENGTH_MUST_MATCH");
Withdrawer.sol:21:        require(msg.sender == address(weth), "WD: ETH_SENDER_NOT_WETH");

11. Functions guaranteed to revert when called by normal users can be marked payable

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided.

operators/Beefy/BeefyVaultStorage.sol:24:    function addVault(address vault, address tokenOrZapper) external onlyOwner {
operators/Beefy/BeefyVaultStorage.sol:34:    function removeVault(address vault) external onlyOwner {
operators/Yearn/YearnVaultStorage.sol:29:    function addVault(address vault, CurvePool calldata curvePool) external onlyOwner {
operators/Yearn/YearnVaultStorage.sol:41:    function removeVault(address vault) external onlyOwner {
NestedFactory.sol:121:    function addOperator(bytes32 operator) external override onlyOwner {
NestedFactory.sol:133:    function removeOperator(bytes32 operator) external override onlyOwner {
NestedFactory.sol:152:    function setFeeSplitter(FeeSplitter _feeSplitter) external override onlyOwner {
NestedFactory.sol:159:    function setEntryFees(uint256 _entryFees) external override onlyOwner {
NestedFactory.sol:167:    function setExitFees(uint256 _exitFees) external override onlyOwner {
NestedFactory.sol:175:    function unlockTokens(IERC20 _token) external override onlyOwner {
OperatorResolver.sol:56:    ) external override onlyOwner {
OperatorResolver.sol:74:    function rebuildCaches(MixinOperatorResolver[] calldata destinations) public onlyOwner {