Platform: Code4rena
Start Date: 04/03/2024
Pot Size: $88,500 USDC
Total HM: 31
Participants: 105
Period: 11 days
Judge: ronnyx2017
Total Solo HM: 7
Id: 342
League: ETH
Rank: 43/105
Findings: 1
Award: $269.18
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: 0x11singh99
Also found by: 0xAnah, 0xhacksmithh, InAllHonesty, SAQ, SM3_SS, dharma09
269.1754 USDC - $269.18
Highlighted below are optimizations exclusively targeting state-mutating functions and view/pure functions invoked by state-mutating functions. In the discussion that follows, only runtime gas is emphasized, given its inevitable dominance over deployment gas costs throughout the protocol's lifetime.
Please be aware that some code snippets may be shortened to conserve space, and certain code snippets may include @audit tags in comments to facilitate issue explanations.
execute
function to save gas
Note: The issues addressed here were not reported by the bot, for packing variables, notes explaining the how and why are included
We can re-arrange the order of V30racle to save 1 storage slot (~2000 Gas) pack MIN_RANGE_DIFFRENCE with factory address
File: src/V3Oracle.sol 25: uint16 public constant MIN_PRICE_DIFFERENCE = 200; //2% /@audit pack uint256 private constant Q96 = 2 ** 96; uint256 private constant Q128 = 2 ** 128;
diff --git a/src/V3Oracle.sol b/src/V3Oracle.sol index 1ad544a..6e173a6 100644 --- a/src/V3Oracle.sol +++ b/src/V3Oracle.sol @@ -22,7 +22,6 @@ import "./interfaces/IErrors.sol"; /// @title V3Oracle to be used in V3Vault to calculate position values /// @notice It uses both chainlink and uniswap v3 TWAP and provides emergency fallback mode contract V3Oracle is IV3Oracle, Ownable, IErrors { - uint16 public constant MIN_PRICE_DIFFERENCE = 200; //2% uint256 private constant Q96 = 2 ** 96; uint256 private constant Q128 = 2 ** 128; @@ -54,7 +53,7 @@ contract V3Oracle is IV3Oracle, Ownable, IErrors { // token => config mapping mapping(address => TokenConfig) public feedConfigs; - + uint16 public constant MIN_PRICE_DIFFERENCE = 200; address public immutable factory; INonfungiblePositionManager public immutable nonfungiblePositionManager;
We can re-arrange the order of V3vault.sol to save 1 storage slot (~2000 Gas) pack emergencyAdmin address with MIN_RESERVE_PROTECTION_FACTOR_X32address
File: src/V3Vault.sol 161: uint256 private transformedTokenId = 0; // transient storage (when available in dencun) mapping(address => bool) public transformerAllowList; // contracts allowed to transform positions (selected audited contracts e.g. V3Utils) mapping(address => mapping(uint256 => mapping(address => bool))) public transformApprovals; // owners permissions for other addresses to call transform on owners behalf (e.g. AutoRange contract) // address which can call special emergency actions without timelock @> address public emergencyAdmin;
diff --git a/src/V3Vault.sol b/src/V3Vault.sol index 64141ec..556ee1d 100644 --- a/src/V3Vault.sol +++ b/src/V3Vault.sol @@ -120,6 +120,9 @@ contract V3Vault is ERC20, Multicall, Ownable, IVault, IERC721Receiver, IErrors // percentage of lend amount which needs to be in reserves before withdrawn uint32 public reserveProtectionFactorX32 = MIN_RESERVE_PROTECTION_FACTOR_X32; + // address which can call special emergency actions without timelock + address public emergencyAdmin; + // total of debt shares - increases when borrow - decreases when repay uint256 public debtSharesTotal = 0; @@ -163,8 +166,7 @@ contract V3Vault is ERC20, Multicall, Ownable, IVault, IERC721Receiver, IErrors mapping(address => bool) public transformerAllowList; // contracts allowed to transform positions (selected audited contracts e.g. V3Utils) mapping(address => mapping(uint256 => mapping(address => bool))) public transformApprovals; // owners permissions for other addresses to call transform on owners behalf (e.g. AutoRange contract) - // address which can call special emergency actions without timelock - address public emergencyAdmin; +
collect fee amount (if type(uint128).max - ALL) So we can pac this variable in small data type by doing this we can save i slots. Which save around 2k gas.
In AutoRange.sol
feeAmount1 and feeAmount1 can be packed together into 1 slot saves 1 Storage Slot.(~2000 Gas)
File: src/transformers/AutoRange.sol 65: struct ExecuteState { address owner; address realOwner; IUniswapV3Pool pool; address token0; address token1; uint24 fee; int24 tickLower; int24 tickUpper; int24 currentTick; uint256 amount0; uint256 amount1; uint256 feeAmount0; //@audit we can pack uint128 uint256 feeAmount1; //this also uint256 maxAddAmount0; uint256 maxAddAmount1; uint256 amountAdded0; uint256 amountAdded1; uint128 liquidity; //@audit look for struct pack uint256 protocolReward0; uint256 protocolReward1; uint256 amountOutMin; uint256 amountInDelta; uint256 amountOutDelta; uint256 newTokenId; }
diff --git a/src/transformers/AutoRange.sol b/src/transformers/AutoRange.sol index 05e63d6..e5f9285 100644 --- a/src/transformers/AutoRange.sol +++ b/src/transformers/AutoRange.sol @@ -1,4 +1,4 @@ // SPDX-License-Identifier: BUSL-1.1 pragma solidity ^0.8.0; import "./Automator.sol"; @@ -79,8 +79,8 @@ contract AutoExit is Automator { uint128 liquidity; uint256 amount0; uint256 amount1; - uint256 feeAmount0; - uint256 feeAmount1; + uint128 feeAmount0; //@audit uint128 + uint128 feeAmount1; uint256 amountOutMin; uint256 amountInDelta; uint256 amountOutDelta;
collect fee amount (if type(uint128).max - ALL) So we can pac this variable in small data type by doing this we can save i slots. Which save around 2k gas.
In AutoExit.sol
feeAmount1 and feeAmount1 can be packed together into 1 slot saves 1 Storage Slot.(~2000 Gas)
File: src/automators/AutoExit.sol 73: struct ExecuteState { address token0; address token1; uint24 fee; int24 tickLower; int24 tickUpper; uint128 liquidity; uint256 amount0; uint256 amount1; uint128 feeAmount0; //@audit uint128 uint128 feeAmount1; //@audit uint128 uint256 amountOutMin; uint256 amountInDelta; uint256 amountOutDelta; IUniswapV3Pool pool; uint256 swapAmount; int24 tick; bool isSwap; bool isAbove; address owner; }
diff --git a/src/automators/AutoExit.sol b/src/automators/AutoExit.sol index 05e63d6..e5f9285 100644 --- a/src/automators/AutoExit.sol +++ b/src/automators/AutoExit.sol @@ -1,4 +1,4 @@ // SPDX-License-Identifier: BUSL-1.1 pragma solidity ^0.8.0; import "./Automator.sol"; @@ -79,8 +79,8 @@ contract AutoExit is Automator { uint128 liquidity; uint256 amount0; uint256 amount1; - uint256 feeAmount0; - uint256 feeAmount1; + uint128 feeAmount0; //@audit uint128 + uint128 feeAmount1; uint256 amountOutMin; uint256 amountInDelta; uint256 amountOutDelta;
execute
function to save gasIn execute()
, multiple conditions are checked after reading the state, which generally consumes more gas; if one of the if conditions reverts, gas is wasted. As a result, whenever a state variable is required, it is best to create one.
In below code access state varibale in line after 119.
File: src/automators/AutoExit.sol 100: function execute(ExecuteParams calldata params) external { if (!operators[msg.sender]) { revert Unauthorized(); } ExecuteState memory state; //@audit cache when its necessary move before state access PositionConfig memory config = positionConfigs[params.tokenId]; if (!config.isActive) { revert NotConfigured(); }
diff --git a/src/automators/AutoExit.sol b/src/automators/AutoExit.sol index 05e63d6..20833bb 100644 --- a/src/automators/AutoExit.sol +++ b/src/automators/AutoExit.sol @@ -102,7 +102,6 @@ contract AutoExit is Automator { revert Unauthorized(); } - ExecuteState memory state; PositionConfig memory config = positionConfigs[params.tokenId]; if (!config.isActive) { @@ -115,7 +114,7 @@ contract AutoExit is Automator { ) { revert ExceedsMaxReward(); } - + ExecuteState memory state; // get position info (,, state.token0, state.token1, state.fee, state.tickLower, state.tickUpper, state.liquidity,,,,) = nonfungiblePositionManager.positions(params.tokenId);
In execute()
, multiple conditions are checked after reading the state, which generally consumes more gas; if one of the if conditions reverts, gas is wasted. As a result, whenever a state variable is required, it is best to create one.
In below code access state varibale in line after 129.
File: src/transformers/AutoRange.sol 111: function execute(ExecuteParams calldata params) external { if (!operators[msg.sender] && !vaults[msg.sender]) { revert Unauthorized(); } ExecuteState memory state; //@audit make state variable when its necessary PositionConfig memory config = positionConfigs[params.tokenId]; if (config.lowerTickDelta == config.upperTickDelta) { revert NotConfigured(); }
diff --git a/src/transformers/AutoRange.sol b/src/transformers/AutoRange.sol index 8c29e6a..f5e83d0 100644 --- a/src/transformers/AutoRange.sol +++ b/src/transformers/AutoRange.sol @@ -112,7 +112,6 @@ contract AutoRange is Automator { if (!operators[msg.sender] && !vaults[msg.sender]) { revert Unauthorized(); } - ExecuteState memory state; PositionConfig memory config = positionConfigs[params.tokenId]; if (config.lowerTickDelta == config.upperTickDelta) { @@ -125,7 +124,7 @@ contract AutoRange is Automator { ) { revert ExceedsMaxReward(); } - + ExecuteState memory state; // get position info (,, state.token0, state.token1, state.fee, state.tickLower, state.tickUpper, state.liquidity,,,,) = nonfungiblePositionManager.positions(params.tokenId);
In _calculateGlobalInterest()
calculate block.timestamp - lastRateUpdate
in if condition which is redundant. So we can avoid that multiple computation by caching this logic in local variable once then use multiple times.This way we can save gas.
File: src/V3Vault.sol 1167: function _calculateGlobalInterest() internal view returns (uint256 newDebtExchangeRateX96, uint256 newLendExchangeRateX96) { uint256 oldDebtExchangeRateX96 = lastDebtExchangeRateX96; uint256 oldLendExchangeRateX96 = lastLendExchangeRateX96; (, uint256 available,) = _getAvailableBalance(oldDebtExchangeRateX96, oldLendExchangeRateX96); uint256 debt = _convertToAssets(debtSharesTotal, oldDebtExchangeRateX96, Math.Rounding.Up); (uint256 borrowRateX96, uint256 supplyRateX96) = interestRateModel.getRatesPerSecondX96(available, debt); supplyRateX96 = supplyRateX96.mulDiv(Q32 - reserveFactorX32, Q32); // always growing or equal uint256 lastRateUpdate = lastExchangeRateUpdate; if (lastRateUpdate > 0) { newDebtExchangeRateX96 = oldDebtExchangeRateX96 + oldDebtExchangeRateX96 * (block.timestamp - lastRateUpdate) * borrowRateX96 / Q96; //@audit cache calculation newLendExchangeRateX96 = oldLendExchangeRateX96 + oldLendExchangeRateX96 * (block.timestamp - lastRateUpdate) * supplyRateX96 / Q96; } else { newDebtExchangeRateX96 = oldDebtExchangeRateX96; newLendExchangeRateX96 = oldLendExchangeRateX96; } }
diff --git a/src/V3Vault.sol b/src/V3Vault.sol index 64141ec..69791fa 100644 --- a/src/V3Vault.sol +++ b/src/V3Vault.sol @@ -1182,12 +1182,13 @@ contract V3Vault is ERC20, Multicall, Ownable, IVault, IERC721Receiver, IErrors // always growing or equal uint256 lastRateUpdate = lastExchangeRateUpdate; + uint256 timeSinceLastUpdate = block.timestamp - lastRateUpdate; if (lastRateUpdate > 0) { newDebtExchangeRateX96 = oldDebtExchangeRateX96 - + oldDebtExchangeRateX96 * (block.timestamp - lastRateUpdate) * borrowRateX96 / Q96; + + oldDebtExchangeRateX96 * timeSinceLastUpdate * borrowRateX96 / Q96; newLendExchangeRateX96 = oldLendExchangeRateX96 - + oldLendExchangeRateX96 * (block.timestamp - lastRateUpdate) * supplyRateX96 / Q96; + + oldLendExchangeRateX96 * timeSinceLastUpdate * supplyRateX96 / Q96; } else { newDebtExchangeRateX96 = oldDebtExchangeRateX96; newLendExchangeRateX96 = oldLendExchangeRateX96;
In Automator.sol:withdrawBalances()
: tokens[i]
we can cache once and use multiple times. Instead of repeatedly calling _newFees[i] in the loop, you can calculate it once and use the result.
File: src/automators/Automator.sol 104: function withdrawBalances(address[] calldata tokens, address to) external virtual { if (msg.sender != withdrawer) { revert Unauthorized(); } uint256 i; uint256 count = tokens.length; for (; i < count; ++i) { uint256 balance = IERC20(tokens[i]).balanceOf(address(this)); //@audit cache token[i] if (balance > 0) { _transferToken(to, IERC20(tokens[i]), balance, true); } } }
diff --git a/src/automators/Automator.sol b/src/automators/Automator.sol index 1d20e99..8a9eaee 100644 --- a/src/automators/Automator.sol +++ b/src/automators/Automator.sol @@ -109,9 +109,10 @@ abstract contract Automator is Swapper, Ownable { uint256 i; uint256 count = tokens.length; for (; i < count; ++i) { - uint256 balance = IERC20(tokens[i]).balanceOf(address(this)); + address token = tokens[i]; + uint256 balance = IERC20(token).balanceOf(address(this)); if (balance > 0) { - _transferToken(to, IERC20(tokens[i]), balance, true); + _transferToken(to, IERC20(token), balance, true); } } }
In AutoCompund.sol:withdrawBalances()
: tokens[i]
we can cache once and use multiple times. Instead of repeatedly calling _newFees[i] in the loop, you can calculate it once and use the result.
diff --git a/src/transformers/AutoCompound.sol b/src/transformers/AutoCompound.sol index a948bae..30eb110 100644 --- a/src/transformers/AutoCompound.sol +++ b/src/transformers/AutoCompound.sol @@ -231,8 +231,9 @@ contract AutoCompound is Automator, Multicall, ReentrancyGuard { uint256 i; uint256 count = tokens.length; for (; i < count; ++i) { - uint256 balance = positionBalances[0][tokens[i]]; - _withdrawBalanceInternal(0, tokens[i], to, balance, balance); + address token = tokens[i]; + uint256 balance = positionBalances[0][token]; + _withdrawBalanceInternal(0, token, to, balance, balance); } }
selfbalance() function from yul is the best gas-effective alternative for The solidity code address(this).balance Across the whole WiseLending codebase, there are only two instances of it that can be optimized to save lot of gas as they are being inherited by lot of functions
File: src/automators/Automator.sol 123: function withdrawETH(address to) external { if (msg.sender != withdrawer) { revert Unauthorized(); } uint256 balance = address(this).balance; //@audit if (balance > 0) { (bool sent,) = to.call{value: balance}(""); if (!sent) { revert EtherSendFailed(); } } }
diff --git a/src/automators/Automator.sol b/src/automators/Automator.sol index 1d20e99..7c61eb7 100644 --- a/src/automators/Automator.sol +++ b/src/automators/Automator.sol @@ -124,10 +124,12 @@ abstract contract Automator is Swapper, Ownable { if (msg.sender != withdrawer) { revert Unauthorized(); } - - uint256 balance = address(this).balance; - if (balance > 0) { - (bool sent,) = to.call{value: balance}(""); + uint256 Balance; + assembly { + Balance := selfbalance() + } + if (Balance > 0) { + (bool sent,) = to.call{value: Balance}(""); if (!sent) { revert EtherSendFailed(); }
The balance parameter is already the current balance, so you can subtract amount from it and then update the mapping directly.Directly update the balance in storage.No need to read from storage again since we already have the 'balance' variable
File: src/transformers/AutoCompound.sol 267: function _withdrawBalanceInternal(uint256 tokenId, address token, address to, uint256 balance, uint256 amount) internal { require(amount <= balance, "amount>balance"); positionBalances[tokenId][token] = positionBalances[tokenId][token] - amount; emit BalanceRemoved(tokenId, token, amount); SafeERC20.safeTransfer(IERC20(token), to, amount); emit BalanceWithdrawn(tokenId, token, to, amount); }
diff --git a/src/transformers/AutoCompound.sol b/src/transformers/AutoCompound.sol index a948bae..c9bd822 100644 --- a/src/transformers/AutoCompound.sol +++ b/src/transformers/AutoCompound.sol @@ -267,7 +268,8 @@ contract AutoCompound is Automator, Multicall, ReentrancyGuard { internal { require(amount <= balance, "amount>balance"); - positionBalances[tokenId][token] = positionBalances[tokenId][token] - amount; + balance -= amount; + positionBalances[tokenId][token] = balance; emit BalanceRemoved(tokenId, token, amount); SafeERC20.safeTransfer(IERC20(token), to, amount); emit BalanceWithdrawn(tokenId, token, to, amount);
In line 959 cached uint256 currentShares = loan.debtShares;
. Use this local variable for all subsequent operations within that function.Especially on line 992
.
File: 956: function _repay(uint256 tokenId, uint256 amount, bool isShare, bytes memory permitData) internal { (uint256 newDebtExchangeRateX96, uint256 newLendExchangeRateX96) = _updateGlobalInterest(); Loan storage loan = loans[tokenId]; @> uint256 currentShares = loan.debtShares; 992: uint256 loanDebtShares = currentShares - shares; //@audit Utilize local variables instead of repeatedly accessing storage variables. loan.debtShares = loanDebtShares; debtSharesTotal -= shares; // when amounts are repayed - they maybe borrowed again dailyDebtIncreaseLimitLeft += assets;
diff --git a/src/V3Vault.sol b/src/V3Vault.sol index 64141ec..57fe5ef 100644 --- a/src/V3Vault.sol +++ b/src/V3Vault.sol @@ -987,14 +989,14 @@ contract V3Vault is ERC20, Multicall, Ownable, IVault, IERC721Receiver, IErrors } } - uint256 loanDebtShares = loan.debtShares - shares; + uint256 loanDebtShares = currentShares - shares; loan.debtShares = loanDebtShares; debtSharesTotal -= shares; // when amounts are repayed - they maybe borrowed again dailyDebtIncreaseLimitLeft += assets; _updateAndCheckCollateral( tokenId, newDebtExchangeRateX96, newLendExchangeRateX96, loanDebtShares + shares, loanDebtShares );
#0 - c4-pre-sort
2024-03-25T00:32:11Z
0xEVom marked the issue as sufficient quality report
#1 - c4-judge
2024-04-01T15:28:30Z
jhsagd76 marked the issue as grade-b
#2 - c4-judge
2024-04-01T15:29:18Z
jhsagd76 marked the issue as grade-a
#3 - c4-sponsor
2024-04-01T20:36:42Z
kalinbas (sponsor) confirmed