search

code-423n4 / 2022-04-backd-findings

6 stars 4 forks source link

Gas Optimizations #168

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Gas Report

Table of Contents

Caching storage variables in memory to save gas

PROBLEM

Anytime you are reading from storage more than once, it is cheaper in gas cost to cache the variable in memory: a SLOAD cost 100gas, while MLOAD and MSTORE cost 3 gas.

In particular, in for loops, when using the length of a storage array as the condition being checked after each loop, caching the array length in memory can yield significant gas savings if the array length is high.

PROOF OF CONCEPT

Instances include:

AaveHandler.sol

scope: topUp()

AaveHandler.sol:44
AaveHandler.sol:45
AaveHandler.sol:50
AaveHandler.sol:53
AaveHandler.sol:60
AaveHandler.sol:65

CompoundHandler.sol

scope: _getAccountBorrowsAndSupply()

CompoundHandler.sol:132
CompoundHandler.sol:134
CompoundHandler.sol:142

CTokenRegistry.sol

scope: _isCTokenUsable()

CTokenRegistry.sol:77
CTokenRegistry.sol:79
CTokenRegistry.sol:80

TopUpAction.sol

scope: resetPosition()

TopUpAction.sol:284
TopUpAction.sol:295

scope: execute()

TopUpAction.sol:562
TopUpAction.sol:604
TopUpAction.sol:632

BkdEthCvx.sol

scope: _withdraw()

BkdEthCvx.sol:77
BkdEthCvx.sol:93

BkdTriHopCvx.sol

scope: _withdraw()

BkdTriHopCvx.sol:175
BkdTriHopCvx.sol:201

ConvexStrategyBase.sol

scope: addRewardToken()

ConvexStrategyBase.sol:279
ConvexStrategyBase.sol:280

scope: harvestable()

ConvexStrategyBase.sol:307
ConvexStrategyBase.sol:311
ConvexStrategyBase.sol:313

scope: _harvest()

ConvexStrategyBase.sol:380

scope: _sendCommunityReserveShare()

ConvexStrategyBase.sol:398
ConvexStrategyBase.sol:409

Vault.sol

scope: _handleExcessDebt()

Vault.sol:645
Vault.sol:648
Vault.sol:649

scope: _handleExcessDebt()

Vault.sol:657
Vault.sol:658

Vault.sol

scope: stakeFor()

Vault.sol:324
Vault.sol:331
Vault.sol:338
Vault.sol:339

scope: unStakeFor()

Vault.sol:365
Vault.sol:376
Vault.sol:382
Vault.sol:384

TOOLS USED

Manual Analysis

MITIGATION

cache these storage variables in memory

Calldata instead of memory for RO function parameters

PROBLEM

If a reference type function parameter is read-only, it is cheaper in gas to use calldata instead of memory. Calldata is a non-modifiable, non-persistent area where function arguments are stored, and behaves mostly like memory.

Try to use calldata as a data location because it will avoid copies and also makes sure that the data cannot be modified.

PROOF OF CONCEPT

Instances include:

RoleManager.sol

scope: hasAnyRole()

RoleManager.sol:73: bytes32[] memory roles

AaveHandler.sol

scope: topUp()

AaveHandler.sol:40: bytes memory extra

CompoundHandler.sol

scope: topUp()

CompoundHandler.sol:54: bytes memory extra

TopUpAction.sol

scope: getHealthFactor()

TopUpAction.sol:760: bytes memory extra

TopUpKeeperHelper.sol

scope: canExecute()

TopUpKeeperHelper.sol:108: ITopUpAction.RecordKey memory key

scope: _canExecute()

TopUpKeeperHelper.sol:131: ITopUpAction.RecordWithMeta memory position

scope: _positionToTopup()

TopUpKeeperHelper.sol:145: ITopUpAction.RecordWithMeta memory position

scope: _shortenTopups()

TopUpKeeperHelper.sol:159: TopupData[] memory topups

Erc20Pool.sol

scope: initialize()

Erc20Pool.sol:15: string memory name_

EthPool.sol

scope: initialize()

EthPool.sol:13: string memory name_

LiquidityPool.sol

scope: initialize()

LiquidityPool.sol:702: string memory name_

LpToken.sol

scope: initialize()

LpToken.sol:29: string memory name_
LpToken.sol:30: string memory symbol_

TOOLS USED

Manual Analysis

MITIGATION

Replace memory with calldata

Comparisons with zero for unsigned integers

PROBLEM

>0 is less gas efficient than !0 if you enable the optimizer at 10k AND you’re in a require statement. Detailed explanation with the opcodes here

PROOF OF CONCEPT

Instances include:

TopUpAction.sol

scope: register()

TopUpAction.sol:210

scope: execute()

TopUpAction.sol:554

TopUpActionFeeHandler.sol

scope: claimKeeperFeesForPool()

TopUpActionFeeHandler.sol:123

LiquidityPool.sol

scope: updateDepositCap()

LiquidityPool.sol:401

scope: calcRedeem()

LiquidityPool.sol:471
LiquidityPool.sol:473

scope: redeem()

LiquidityPool.sol:549

Vault.sol

scope: withdrawFromReserve()

Vault.sol:164

BkdLocker.sol

scope: depositFees()

BkdLocker.sol:90
BkdLocker.sol:91
BkdLocker.sol:136

TOOLS USED

Manual Analysis

MITIGATION

Replace > 0 with !0

Comparison Operators

PROBLEM

In the EVM, there is no opcode for >= or <=. When using greater than or equal, two operations are performed: > and =.

Using strict comparison operators hence saves gas

PROOF OF CONCEPT

Instances include:

TopUpAction.sol

TopUpAction.sol:61
TopUpAction.sol:212
TopUpAction.sol:224
TopUpAction.sol:328
TopUpAction.sol:360
TopUpAction.sol:361
TopUpAction.sol:500
TopUpAction.sol:501
TopUpAction.sol:576
TopUpAction.sol:584
TopUpAction.sol:724

TopUpActionFeeHandler.sol

TopUpActionFeeHandler.sol:54
TopUpActionFeeHandler.sol:151
TopUpActionFeeHandler.sol:163
TopUpActionFeeHandler.sol:196
TopUpActionFeeHandler.sol:208

ChainLinkOracleProvider.sol

ChainLinkOracleProvider.sol:41
ChainLinkOracleProvider.sol:57

EthPool.sol

EthPool.sol:442
EthPool.sol:208
EthPool.sol:518
EthPool.sol:525
EthPool.sol:551
EthPool.sol:562
EthPool.sol:690
EthPool.sol:811
EthPool.sol:812

BkdEthCvx.sol

BkdEthCvx.sol:76

BkdTriHopCvx.sol

BkdTriHopCvx.sol:174

ConvexStrategyBase.sol

ConvexStrategyBase.sol:197
ConvexStrategyBase.sol:214

StrategySwapper.sol

StrategySwapper.sol:110

CvxMintAmount.sol

CvxMintAmount.sol:21

Preparable.sol

Preparable.sol:29
Preparable.sol:110

Vault.sol

Vault.sol:88
Vault.sol:89
Vault.sol:90
Vault.sol:167
Vault.sol:264
Vault.sol:323
Vault.sol:392
Vault.sol:437
Vault.sol:482
Vault.sol:712
Vault.sol:763

VaultReserve.sol

VaultReserve.sol:59
VaultReserve.sol:75
VaultReserve.sol:103

BkdLocker.sol

BkdLocker.sol:119
BkdLocker.sol:140
BkdLocker.sol:281

Controller.sol

Controller:98

CvxCrvRewardsLocker.sol

CvxCrvRewardsLocker:84

GasBank.sol

GasBank:68
GasBank:76

StakerVault.sol

StakerVault:107
StakerVault:150
StakerVault:153
StakerVault:324
StakerVault:368
StakerVault:371

TOOLS USED

Manual Analysis

MITIGATION

Replace <= with <, and >= with >. Do not forget to increment/decrement the compared variable

example:

-require(maxFee >= minFee, Error.INVALID_AMOUNT);
+require(maxFee > minFee - 1, Error.INVALID_AMOUNT);

When the comparison is with a constant storage variable, you can also do the increment in the storage variable declaration

example:

-require(maxFee <= _MAX_WITHDRAWAL_FEE, Error.INVALID_AMOUNT)
+require(maxFee < _MAX_WITHDRAWAL_FEE_PLUS_ONE, Error.INVALID_AMOUNT)

However, when 1 is negligible compared to the variable (with is the case here as the variable is in the order of 10**16), it is not necessary to increment.

Custom Errors

PROBLEM

Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met) while providing the same amount of information, as explained here

Custom errors are defined using the error statement

PROOF OF CONCEPT

Instances include:

RoleManager.sol

RoleManager.sol:44
RoleManager.sol:110
RoleManager.sol:111

AaveHandler.sol

AaveHandler.sol:51

CompoundHandler.sol

CompoundHandler.sol:74
CompoundHandler.sol:80
CompoundHandler.sol:123
CompoundHandler.sol:141
CompoundHandler.sol:148

TopUpAction.sol

TopUpAction.sol:67
TopUpAction.sol:98
TopUpAction.sol:185
TopUpAction.sol:209
TopUpAction.sol:210
TopUpAction.sol:211
TopUpAction.sol:212
TopUpAction.sol:213
TopUpAction.sol:217
TopUpAction.sol:218
TopUpAction.sol:224
TopUpAction.sol:282
TopUpAction.sol:328
TopUpAction.sol:359
TopUpAction.sol:546
TopUpAction.sol:553
TopUpAction.sol:554
TopUpAction.sol:560
TopUpAction.sol:575
TopUpAction.sol:583
TopUpAction.sol:676
TopUpAction.sol:723
TopUpAction.sol:928

TopUpActionFeeHandler.sol

TopUpActionFeeHandler.sol:67
TopUpActionFeeHandler.sol:68
TopUpActionFeeHandler.sol:87
TopUpActionFeeHandler.sol:123
TopUpActionFeeHandler.sol:151
TopUpActionFeeHandler.sol:161
TopUpActionFeeHandler.sol:196
TopUpActionFeeHandler.sol:206

ChainLinkOracleProvider.sol

ChainLinkOracleProvider.sol:31
ChainLinkOracleProvider.sol:41
ChainLinkOracleProvider.sol:53
ChainLinkOracleProvider.sol:57
ChainLinkOracleProvider.sol:58

Erc20Pool.sol

Erc20Pool.sol:20
Erc20Pool.sol:30

EthPool.sol

EthPool.sol:25
EthPool.sol:26

LiquidityPool.sol

LiquidityPool.sol:76
LiquidityPool.sol:136
LiquidityPool.sol:137
LiquidityPool.sol:155
LiquidityPool.sol:179
LiquidityPool.sol:208
LiquidityPool.sol:331
LiquidityPool.sol:333
LiquidityPool.sol:387
LiquidityPool.sol:399
LiquidityPool.sol:400
LiquidityPool.sol:401
LiquidityPool.sol:441
LiquidityPool.sol:471
LiquidityPool.sol:473
LiquidityPool.sol:517
LiquidityPool.sol:525
LiquidityPool.sol:549
LiquidityPool.sol:551
LiquidityPool.sol:562
LiquidityPool.sol:811
LiquidityPool.sol:812

PoolFactory.sol

PoolFactory.sol:159
PoolFactory.sol:162
PoolFactory.sol:165
PoolFactory.sol:170
PoolFactory.sol:180
PoolFactory.sol:184

BkdTriHopCvx.sol

BkdTriHopCvx.sol:133
BkdTriHopCvx.sol:147

ConvexStrategyBase.sol

ConvexStrategyBase.sol:117
ConvexStrategyBase.sol:144
ConvexStrategyBase.sol:197
ConvexStrategyBase.sol:198
ConvexStrategyBase.sol:214
ConvexStrategyBase.sol:215
ConvexStrategyBase.sol:260
ConvexStrategyBase.sol:273

StrategySwapper.sol

StrategySwapper.sol:69
StrategySwapper.sol:110
StrategySwapper.sol:111
StrategySwapper.sol:123
StrategySwapper.sol:124
StrategySwapper.sol:139

Preparable.sol

Preparable.sol:28
Preparable.sol:29
Preparable.sol:86
Preparable.sol:98
Preparable.sol:110
Preparable.sol:111

Erc20Vault.sol

Erc20Vault.sol:20

Vault.sol

Vault.sol:88
Vault.sol:89
Vault.sol:90
Vault.sol:164
Vault.sol:165
Vault.sol:167
Vault.sol:194
Vault.sol:195
Vault.sol:198
Vault.sol:264
Vault.sol:392
Vault.sol:429
Vault.sol:762

VaultReserve.sol

VaultReserve.sol:51
VaultReserve.sol:59
VaultReserve.sol:73
VaultReserve.sol:75

AddressProvider.sol

AddressProvider.sol:64
AddressProvider.sol:70
AddressProvider.sol:96
AddressProvider.sol:100
AddressProvider.sol:170
AddressProvider.sol:179
AddressProvider.sol:188
AddressProvider.sol:230
AddressProvider.sol:231
AddressProvider.sol:249
AddressProvider.sol:259
AddressProvider.sol:284
AddressProvider.sol:285
AddressProvider.sol:314
AddressProvider.sol:417
AddressProvider.sol:423

BkdLocker.sol

BkdLocker.sol:58
BkdLocker.sol:90
BkdLocker.sol:91
BkdLocker.sol:118
BkdLocker.sol:136
BkdLocker.sol:207

Controller.sol

Controller:32
Controller:33
Controller:80

CvxCrvRewardsLocker.sol

CvxCrvRewardsLocker:83
CvxCrvRewardsLocker:135

GasBank.sol

GasBank:42
GasBank:68
GasBank:69
GasBank:76
GasBank:91

LpToken.sol

LpToken:34

StakerVault.sol

StakerVault:70
StakerVault:93
StakerVault:106
StakerVault:107
StakerVault:139
StakerVault:150
StakerVault:153
StakerVault:203
StakerVault:224
StakerVault:324
StakerVault:340
StakerVault:367
StakerVault:371

SwapperRegistry.sol

SwapperRegistry:35

TOOLS USED

Manual Analysis

MITIGATION

Replace require statements with custom errors.

Default value initialization

PROBLEM

If a variable is not set/initialized, it is assumed to have the default value (0, false, 0x0 etc depending on the data type). Explicitly initializing it with its default value is an anti-pattern and wastes gas.

PROOF OF CONCEPT

Instances include:

RoleManager.sol

RoleManager.sol:80
RoleManager.sol:110
RoleManager.sol:111

CompoundHandler.sol

CompoundHandler.sol:135

CTokenRegistry.sol

CTokenRegistry.sol:61

TopUpAction.sol

TopUpAction.sol:188
TopUpAction.sol:452
TopUpAction.sol:456
TopUpAction.sol:479
TopUpAction.sol:506
TopUpAction.sol:891

TopUpActionKeeperHandler.sol

TopUpActionKeeperHandler.sol:43
TopUpActionKeeperHandler.sol:46
TopUpActionKeeperHandler.sol:72
TopUpActionKeeperHandler.sol:93
TopUpActionKeeperHandler.sol:165

LiquidityPool.sol

LiquidityPool.sol:483

ConvexStrategyBase.sol

ConvexStrategyBase.sol:313
ConvexStrategyBase.sol:380

Vault.sol

Vault.sol:42
Vault.sol:135
Vault.sol:583

BkdLocker.sol

BkdLocker.sol:133
BkdLocker.sol:310

Controller.sol

Controller:114
Controller:117

CvxCrvRewardsLocker.sol

CvxCrvRewardsLocker:43

StakerVault.sol

StakerVault:144
StakerVault:260

TOOLS USED

Manual Analysis

MITIGATION

Remove explicit initialization for default values.

Prefix increments

PROBLEM

Prefix increments are cheaper than postfix increments.

PROOF OF CONCEPT

Instances include:

RoleManager.sol

RoleManager.sol:80

CompoundHandler.sol

CompoundHandler.sol:135

CTokenRegistry.sol

CTokenRegistry.sol:61

TopUpAction.sol

TopUpAction.sol:188
TopUpAction.sol:456
TopUpAction.sol:479
TopUpAction.sol:506
TopUpAction.sol:891

TopUpActionKeeperHandler.sol

TopUpActionKeeperHandler.sol:43
TopUpActionKeeperHandler.sol:46
TopUpActionKeeperHandler.sol:50
TopUpActionKeeperHandler.sol:72
TopUpActionKeeperHandler.sol:93
TopUpActionKeeperHandler.sol:165

ConvexStrategyBase.sol

ConvexStrategyBase.sol:313
ConvexStrategyBase.sol:380

BkdLocker.sol

BkdLocker.sol:310

Controller.sol

Controller:117

StakerVault.sol

StakerVault:260

TOOLS USED

Manual Analysis

MITIGATION

change variable++ to ++variable

Redundant code

IMPACT

Redundant code should be avoided as it costs unnecessary gas

PROOF OF CONCEPT

Instances include:

Preparable.sol

Preparable.sol:140:
address oldValue = currentAddresses[key];
currentAddresses[key] = value;
pendingAddresses[key] = address(0);
deadlines[key] = 0;
emit ConfigUpdatedAddress(key, oldValue, value);
return value;

We can update currentAddresses[key] after emitting the event to save the gas of the declaration of oldValue:

+emit ConfigUpdatedAddress(key, currentAddresses[key], value);
pendingAddresses[key] = address(0);
deadlines[key] = 0;
currentAddresses[key] = value;
return value;

TOOLS USED

Manual Analysis

MITIGATION

see Proof of Concept for mitigation steps.

Require instead of &&

IMPACT

Require statements including conditions with the && operator can be broken down in multiple require statements to save gas.

PROOF OF CONCEPT

Instances include:

TopUpAction.sol

TopUpAction:360:
require(
            newSwapperSlippage >= _MIN_SWAPPER_SLIPPAGE &&
                newSwapperSlippage <= _MAX_SWAPPER_SLIPPAGE,
            Error.INVALID_AMOUNT
        );

ConvexStrategyBase.sol

ConvexStrategyBase:274:
require(
            token_ != address(_CVX) && token_ != address(underlying) && token_ != address(_CRV),
            Error.INVALID_TOKEN_TO_ADD
        );

SwapperRegistry.sol

SwapperRegistry:35:
require(
            fromToken != toToken &&
                fromToken != address(0) &&
                toToken != address(0) &&
                newSwapper != address(0),
            Error.INVALID_TOKEN_PAIR
        );

TOOLS USED

Manual Analysis

MITIGATION

Breakdown each condition in a separate require statement (though require statements should be replaced with custom errors)

require( newSwapperSlippage >=_MIN_SWAPPER_SLIPPAGE);
require(newSwapperSlippage <= _MAX_SWAPPER_SLIPPAGE,
            Error.INVALID_AMOUNT)

Tight Variable Packing

PROBLEM

Solidity contracts have contiguous 32 bytes (256 bits) slots used in storage. By arranging the variables, it is possible to minimize the number of slots used within a contract's storage and therefore reduce deployment costs.

address type variables are each of 20 bytes size (way less than 32 bytes). However, they here take up a whole 32 bytes slot (they are contiguous).

As bool type variables are of size 1 byte, there's a slot here that can get saved by moving them closer to an address

PROOF OF CONCEPT

Instances include:

VaultStorage.sol

VaultStorage.sol:11:
address public pool;
uint256 public totalDebt;
bool public strategyActive;

TOOLS USED

Manual Analysis

MITIGATION

Place strategyActive after pool to save one storage slot

address public pool;
+bool public strategyActive;
uint256 public totalDebt;

Unchecked arithmetic

PROBLEM

The default "checked" behavior costs more gas when adding/diving/multiplying, because under-the-hood those checks are implemented as a series of opcodes that, prior to performing the actual arithmetic, check for under/overflow and revert if it is detected.

if it can statically be determined there is no possible way for your arithmetic to under/overflow (such as a condition in an if statement), surrounding the arithmetic in an unchecked block will save gas

PROOF OF CONCEPT

Instances include:

LiquidityPool.sol

LiquidityPool.sol:751: underlyingBalance - underlyingToWithdraw; //because of the condition line 745, the underflow check is unnecessary

BkdEthCvx.sol

BkdEthCvx.sol:83: uint256 requiredUnderlyingAmount = amount - underlyingBalance; //because of the condition line 76, the underflow check is unnecessary

BkdTriHopCvx.sol

BkdTriHopCvx.sol:181: uint256 requiredUnderlyingAmount = amount - underlyingBalance; //because of the condition line 174, the underflow check is unnecessary

CvxMintAmount.sol

CvxMintAmount.sol:24: uint256 remaining = _CLIFF_COUNT - currentCliff; //because of the condition line 21, the underflow check is unnecessary

Vault.sol

Vault.sol:24: uint256 remaining = _CLIFF_COUNT - currentCliff; //because of the condition line 21, the underflow check is unnecessary

Vault.sol:125: uint256 requiredWithdrawal = amount - availableUnderlying_; //because of the condition line 122, the underflow check is unnecessary

Vault.sol:130: uint256 newTarget = (allocated - requiredWithdrawal) //because of the condition line 127, the underflow check is unnecessary

Vault.sol:141: uint256 totalUnderlyingAfterWithdraw = totalUnderlying - amount; //because of the condition line 122, the underflow check is unnecessary

Vault.sol:440: waitingForRemovalAllocated = _waitingForRemovalAllocated - withdrawn; //because of the condition line 437, the underflow check is unnecessary

Vault.sol:444: uint256 profit = withdrawn - allocated; //because of the condition line 443, the underflow check is unnecessary

Vault.sol:452: allocated -= withdrawn; //because of the condition line 443, the underflow check is unnecessary

Vault.sol:591: uint256 profit = allocatedUnderlying - amountAllocated; //because of the condition line 589, the underflow check is unnecessary

Vault.sol:595: profit -= currentDebt; //because of the condition line 593, the underflow check is unnecessary

Vault.sol:600: currentDebt -= profit; //because of the condition line 593, the underflow check is unnecessary

Vault.sol:605: uint256 loss = amountAllocated - allocatedUnderlying; //because of the condition line 603, the underflow check is unnecessary

Vault.sol:784: uint256 withdrawAmount = allocatedUnderlying - target; //because of the condition line 782, the underflow check is unnecessary

Vault.sol:790: uint256 depositAmount = target - allocatedUnderlying; //because of the condition line 788, the underflow check is unnecessary

StakerVault.sol

StakerVault.sol:164: uint256 srcTokensNew = srcTokens - amount; //because of the condition line 153, the underflow check is unnecessary

TOOLS USED

Manual Analysis

MITIGATION

Place the arithmetic operations in an unchecked block

chase-manning commented 2 years ago

I consider this report to be of particularly high quality