Platform: Code4rena
Start Date: 07/07/2023
Pot Size: $121,650 USDC
Total HM: 36
Participants: 111
Period: 7 days
Judge: Picodes
Total Solo HM: 13
Id: 258
League: ETH
Rank: 47/111
Findings: 1
Award: $247.48
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: c3phas
Also found by: 0x11singh99, 0xAnah, 0xn006e7, LeoS, Rageur, Raihan, ReyAdmirado, Rolezn, SAAJ, SAQ, SM3_SS, Udsen, alymurtazamemon, hunter_w3b, koxuan, naman1778, petrichor, ybansal2403
247.4795 USDC - $247.48
no | Issue | Instances | |
---|---|---|---|
[G-01] | Cache state variables outside of loop to avoid reading storage on every iteration | 1 | - |
[G-02] | Using private rather than public for constants, saves gas | 1 | - |
[G-03] | Use constants instead of type(uintx).max | 11 | - |
[G-04] | Use hardcode address instead address(this) | 19 | - |
[G-05] | Don’t apply the same value to state variables | 5 | - |
[G-06] | Avoid emitting storage values | 7 | - |
[G-07] | Use assembly to write address storage values | 1 | - |
[G-08] | Amounts should be checked for 0 before calling a transfer | 2 | - |
[G-09] | Multiple accesses of a mapping/array should use a storage pointer | 1 | - |
[G-10] | 2**<N> should be re-written as type(uint<N>).max | 7 | - |
[G-11] | Using XOR (^) and OR ( | ) bitwise equivalents | 2 |
[G-12] | IF’s/require() statements that check input arguments should be at the top of the function | 4 | - |
[G-13] | Use assembly to perform efficient back-to-back calls | 2 | - |
[G-14] | Using a positive conditional flow to save a NOT opcode | 14 | - |
Reading from storage should always try to be avoided within loops. In the following instances, we are able to cache state variables outside of the loop to save a Gwarmaccess (100 gas) per loop iteration.
file: src/abstract/TieredLiquidityDistributor.sol 361 for (uint8 i = start; i < end; i++) { _tiers[i] = Tier({ drawId: closedDrawId, prizeTokenPerShare: prizeTokenPerShare, prizeSize: uint96( _computePrizeSize(i, _nextNumberOfTiers, _prizeTokenPerShare, newPrizeTokenPerShare) ) });
If needed, the values can be read from the verified contract source code, or if there are multiple values there can be a single getter function that returns a tuple of the values of all currently-public constants. Saves 3406-3606 gas in deployment gas due to the compiler not having to create non-payable getter functions for deployment calldata, not having to store the bytes of the value outside of where it’s used, and not adding another entry to the method ID table.
file: src/TwabController.sol 24 address public constant SPONSORSHIP_ADDRESS = address(1);
Certainly! When working with maximum values in Solidity, it's more gas-efficient to use constants instead of the expression type(uintX).max, where X represents the number of bits in the unsigned integer type. This optimization can help reduce the gas costs associated with storing and manipulating large numbers.
file: src/libraries/LinearVRGDALib.sol 56 targetTimeUnwrapped > 0 && decayConstantUnwrapped > type(int256).max / targetTimeUnwrapped 58 return type(uint256).max; 70 return type(uint256).max; 82 if (targetPriceInt > type(int256).max / expResult) 83 return type(uint256).max; 89 if (targetPriceInt > type(int256).max / extraPrecisionExpResult) 90 return type(uint256).max;
file: src/Vault.sol 282 asset_.safeApprove(address(yieldVault_), type(uint256).max); 376 return _isVaultCollateralized() ? type(uint96).max : 0; 384 return _isVaultCollateralized() ? type(uint96).max : 0; 677 _asset.safeApprove(address(liquidationPair_), type(uint256).max);
To save gas, you can hardcode the contract's address as a constant variable within the contract. By doing so, you eliminate the need for a function call to retrieve the contract's address, resulting in gas savings.
file: src/Vault.sol 336 return _twabController.balanceOf(address(this), _account); 335 _permit(IERC20Permit(asset()), msg.sender, address(this), _assets, _deadline, _v, _r, _s); 468 _permit(IERC20Permit(asset()), msg.sender, address(this), _assets, _deadline, _v, _r, _s); 502 _permit(IERC20Permit(asset()), msg.sender, address(this), _assets, _deadline, _v, _r, _s); 562 if (_tokenOut != address(this)) 563 revert LiquidationTokenOutNotVaultShare(_tokenOut, address(this)); 570 _prizePool.contributePrizeTokens(address(this), _amountIn); 578 uint256 _vaultAssets = IERC20(asset()).balanceOf(address(this)); 581 _yieldVault.deposit(_vaultAssets, address(this)); 801 return _yieldVault.maxWithdraw(address(this)) + super.totalAssets(); 809 return _twabController.totalSupply(address(this)); 830 if (_token != address(this)) revert LiquidationTokenOutNotVaultShare(_token, address(this)); 932 uint256 _vaultAssets = _asset.balanceOf(address(this)); 954 address(this), 959 _yieldVault.deposit(_assets, address(this)); 986 _twabController.delegateOf(address(this), _receiver) != _twabController.SPONSORSHIP_ADDRESS() 1026 _yieldVault.withdraw(_assets, address(this), address(this)); 1176 uint256 _withdrawableAssets = _yieldVault.maxWithdraw(address(this));
file: src/VaultFactory.sol 83 emit NewFactoryVault(_vault, VaultFactory(address(this)));
If claimCount is already 0 , it’ll save 2100 gas to not set it to that value again
file: src/PrizePool.sol 369 claimCount = 0; canaryClaimCount = 0; largestTierClaimed = 0;
Caching of a state variable replaces each Gwarmaccess (130 gas) with a much cheaper stack read. We can avoid unecessary SLOADs by caching storage values that were previously accessed and emitting those cached values.
file: src/PrizePool.sol 375 emit DrawClosed( lastClosedDrawId, winningRandomNumber_, _numTiers, _nextNumberOfTiers, _reserve, prizeTokenPerShare, _lastClosedDrawStartedAt );
file: src/TwabController.sol 692 emit ObservationRecorded( _vault, _user, _account.details.balance, _account.details.delegateBalance, _isNewObservation, _observation ); 735 emit ObservationRecorded( _vault, _user, _account.details.balance, _account.details.delegateBalance, _isNewObservation, _observation ); 777 emit TotalSupplyObservationRecorded( _vault, _account.details.balance, _account.details.delegateBalance, _isNewObservation, _observation ); 811 emit TotalSupplyObservationRecorded( _vault, _account.details.balance, _account.details.delegateBalance, _isNewObservation, _observation );
file: src/Vault.sol 1111 emit RecordedExchangeRate(_lastRecordedExchangeRate);
Using assembly in Solidity allows you to directly access and manipulate the low-level operations of the Ethereum Virtual Machine (EVM). By leveraging assembly, you can optimize gas usage in certain scenarios, such as when reading or writing values to storage slots of an address.
file: src/PrizePool.sol 299 function setDrawManager(address _drawManager) external { if (drawManager != address(0)) { revert DrawManagerAlreadySet(); } drawManager = _drawManager;
Checking non-zero transfer values can avoid an expensive external call and save gas. While this is done at some places, it’s not consistently done in the solution. I suggest adding a non-zero-value check here:
file: src/PrizePool.sol 832 prizeToken.safeTransfer(_to, _amount);
file: src/Vault.sol 1126 emit Transfer(address(0), _receiver, _shares);
file: src/PrizePool.sol 483 function withdrawClaimRewards(address _to, uint256 _amount) external { uint256 _available = claimerRewards[msg.sender]; if (_amount > _available) { revert InsufficientRewardsError(_amount, _available); } claimerRewards[msg.sender] -= _amount; (_to, _amount)
Earlier versions of solidity can use uint<n>(-1) instead. Expressions not including the - 1 can often be re-written to accomodate the change (e.g. by using a > rather than a >=, which will also save some gas)
file: src/libraries/TierCalculationLib.sol 40 uint256 _numberOfPrizes = 4 ** _tier;
file: src/libraries/OverflowSafeComparatorLib.sol 19 uint256 aAdjusted = _a > _timestamp ? _a : _a + 2 ** 32; 20 uint256 bAdjusted = _b > _timestamp ? _b : _b + 2 ** 32; 35 uint256 aAdjusted = _a > _timestamp ? _a : _a + 2 ** 32; 36 uint256 bAdjusted = _b > _timestamp ? _b : _b + 2 ** 32; 52 uint256 aAdjusted = _a > _timestamp ? _a : _a + 2 ** 32; 53 uint256 bAdjusted = _b > _timestamp ? _b : _b + 2 ** 32;
Estimated savings: 73 gas Max savings according to yarn profile: 282 gas
On Remix, given only uint256 types, the following are logical equivalents, but don’t cost the same amount of gas:
(a != b || c != d || e != f) costs 571 ((a ^ b) | (c ^ d) | (e ^ f)) != 0 costs 498 (saving 73 gas)
file: src/TwabController.sol 545 (_to == address(0) && _fromDelegate != SPONSORSHIP_ADDRESS) || 575 (_from == address(0) && _toDelegate != SPONSORSHIP_ADDRESS) ||
Checks that involve constants should come before checks that involve state variables, function calls, and calculations. By doing these checks first, the function is able to revert before wasting a Gcoldsload (2100 gas) in a function that may ultimately revert in the unhappy case.
file: src/libraries/TwabLib.sol 473 function _getPreviousOrAtObservation( uint32 PERIOD_OFFSET, ObservationLib.Observation[MAX_CARDINALITY] storage _observations, AccountDetails memory _accountDetails, uint32 _targetTime ) private view returns (ObservationLib.Observation memory prevOrAtObservation) { uint32 currentTime = uint32(block.timestamp); uint16 oldestTwabIndex; uint16 newestTwabIndex; // If there are no observations, return a zeroed observation if (_accountDetails.cardinality == 0) { return ObservationLib.Observation({ cumulativeBalance: 0, balance: 0, timestamp: PERIOD_OFFSET }); }
file: src/abstract/TieredLiquidityDistributor.sol 612 function _getTierLiquidityToReclaim( uint8 _numberOfTiers, uint8 _nextNumberOfTiers, UD60x18 _prizeTokenPerShare ) internal view returns (uint256) { UD60x18 reclaimedLiquidity; // need to redistribute to the canary tier and any new tiers (if expanding) uint8 start; uint8 end; // if we are expanding, need to reset the canary tier and all of the new tiers if (_nextNumberOfTiers < _numberOfTiers) { start = _nextNumberOfTiers - 1; end = _numberOfTiers; } else { // just reset the canary tier start = _numberOfTiers - 1; end = _numberOfTiers; } for (uint8 i = start; i < end; i++) { Tier memory tierLiquidity = _tiers[i]; uint8 shares = _computeShares(i, _numberOfTiers); UD60x18 liq = _getTierRemainingLiquidity( shares, fromUD34x4toUD60x18(tierLiquidity.prizeTokenPerShare), _prizeTokenPerShare ); reclaimedLiquidity = reclaimedLiquidity.add(liq); } return fromUD60x18(reclaimedLiquidity); }
file: src/libraries/TwabLib.sol 561 function _getNextOrNewestObservation( uint32 PERIOD_OFFSET, ObservationLib.Observation[MAX_CARDINALITY] storage _observations, AccountDetails memory _accountDetails, uint32 _targetTime ) private view returns (ObservationLib.Observation memory nextOrNewestObservation) { uint32 currentTime = uint32(block.timestamp); uint16 oldestTwabIndex; // If there are no observations, return a zeroed observation if (_accountDetails.cardinality == 0) { return ObservationLib.Observation({ cumulativeBalance: 0, balance: 0, timestamp: PERIOD_OFFSET }); }
file: src/Vault.sol 550 function liquidate( address _account, address _tokenIn, uint256 _amountIn, address _tokenOut, uint256 _amountOut ) public virtual override returns (bool) { _requireVaultCollateralized(); if (msg.sender != address(_liquidationPair)) revert LiquidationCallerNotLP(msg.sender, address(_liquidationPair)); if (_tokenIn != address(_prizePool.prizeToken())) revert LiquidationTokenInNotPrizeToken(_tokenIn, address(_prizePool.prizeToken())); if (_tokenOut != address(this)) revert LiquidationTokenOutNotVaultShare(_tokenOut, address(this)); if (_amountOut == 0) revert LiquidationAmountOutZero(); uint256 _liquidableYield = _liquidatableBalanceOf(_tokenOut); if (_amountOut > _liquidableYield) revert LiquidationAmountOutGTYield(_amountOut, _liquidableYield); _prizePool.contributePrizeTokens(address(this), _amountIn); if (_yieldFeePercentage != 0) { _increaseYieldFeeBalance( (_amountOut * FEE_PRECISION) / (FEE_PRECISION - _yieldFeePercentage) - _amountOut ); } uint256 _vaultAssets = IERC20(asset()).balanceOf(address(this)); if (_vaultAssets != 0 && _amountOut >= _vaultAssets) { _yieldVault.deposit(_vaultAssets, address(this)); } _mint(_account, _amountOut); return true; }
file: src/Vault.sol 665 function setLiquidationPair( LiquidationPair liquidationPair_ ) external onlyOwner returns (address) { if (address(liquidationPair_) == address(0)) revert LPZeroAddress(); IERC20 _asset = IERC20(asset()); address _previousLiquidationPair = address(_liquidationPair); if (_previousLiquidationPair != address(0)) { _asset.safeApprove(_previousLiquidationPair, 0); } _asset.safeApprove(address(liquidationPair_), type(uint256).max); _liquidationPair = liquidationPair_;
file: src/Claimer.sol 169 function _computeMaxFee(uint8 _tier, uint8 _numTiers) internal view returns (uint256) { uint8 _canaryTier = _numTiers - 1; if (_tier != _canaryTier) { // canary tier return _computeMaxFee(prizePool.getTierPrizeSize(_canaryTier - 1)); } else { return _computeMaxFee(prizePool.getTierPrizeSize(_canaryTier)); }
Estimated savings: 3 gas Max savings according to yarn profile: 150 gas
The following function either revert or returns some value. To save some gas (NOT opcode costing 3 gas), switch to a positive statement:
file: src/PrizePool.sol 428 if ( !_isWinner(msg.sender, _winner, _tier, _prizeIndex, _vaultPortion, _tierOdds, _drawDuration)
file: src/libraries/DrawAccumulatorLib.sol 459 if (!targetAtOrAfter)
file: src/TwabController.sol 530 if (!_isFromDelegate && _fromDelegate != SPONSORSHIP_ADDRESS) 561 if (!_isToDelegate && _toDelegate != SPONSORSHIP_ADDRESS)
file: src/libraries/ObservationLib.sol 93 if (!targetAfterOrAt)
file: src/Vault.sol 1200 if (!_isVaultCollateralized()) revert VaultUnderCollateralized();
#0 - c4-judge
2023-07-18T18:54:48Z
Picodes marked the issue as grade-a
#1 - c4-sponsor
2023-07-20T23:12:12Z
asselstine marked the issue as sponsor confirmed
#2 - PierrickGT
2023-09-08T23:57:48Z
G-1, 6, 12: has been fixed
G-2: constants can't be private
G-3, 4: does not save any gas
G-5, 7, 11, 13, 14: we would lose in code legibility
G-8: is handled by safeTransfer
G-9: values are assigned, not accessed
G-10: we won't implement