Platform: Code4rena
Start Date: 02/08/2023
Pot Size: $42,000 USDC
Total HM: 13
Participants: 45
Period: 5 days
Judge: hickuphh3
Total Solo HM: 5
Id: 271
League: ETH
Rank: 43/45
Findings: 1
Award: $30.61
š Selected for report: 0
š Solo Findings: 0
š Selected for report: Rolezn
Also found by: 0xhex, 0xta, JCK, K42, Rageur, Raihan, ReyAdmirado, SAQ, SY_S, dharma09, hunter_w3b, petrichor, shamsulhaq123, wahedtalash77
30.6063 USDC - $30.61
| Number | Issue | Instances |
|---|---|---|
| [G-01] | Structs can be packed into fewer storage slots by editing variables | 2 |
| [G-02] | Functions guaranteed to revert when called by normal users can be marked payable | 4 |
| [G-03] | UsingĀ deleteĀ statement can save gas | 3 |
| [G-04] | Missing zero address check in constructor | 4 |
| [G-05] | >=Ā costs less gas thanĀ > | 8 |
| [G-06] | UseĀ do while loops instead ofĀ forĀ loops | 2 |
| [G-07] | Do not calculate constant | 1 |
| [G-08] | use mappings instead of arrays | 1 |
| [G-09] | UseĀ assemblyĀ to writeĀ address storage values | 5 |
| [G-10] | Avoid contract existence checks by using low level calls | 6 |
| [G-11] | Declare the State Variabel outside the loop to save gas | 1 |
| [G-12] | Caching global variables is more expensive than using the actual variable (use block.timestamp instead of caching it) | 1 |
| [G-13] | Ternary operation is cheaper than if-else statement | 2 |
| [G-14] | Expensive operation inside a for loop | 2 |
| [G-15] | Use hardcode address instead address(this) | 6 |
| [G-16] | Public Functions To External | 2 |
| [G-17] | Using bools for storage incurs overhead | 1 |
| [G-18] | Use assembly to perform efficient back-to-back calls | 1 |
| [G-19] | Internal functions not called by the contract should be removed to save deployment gas | 1 |
| [G-20] | Forgo internal function to save 1 STATICCALL | 1 |
| [G-21] | With assembly, .call (bool success) transfer can be done gas-optimized | 1 |
| [G-22] | Amounts should be checked for 0 before calling a transfer | 2 |
| [G-23] | For loops in public or external functions should be avoided due to high gas costs and possible DOS | 1 |
File: src/RngAuction.sol 29 struct RngAuctionResult { address recipient; UD2x18 rewardFraction; uint32 sequenceId; RNGInterface rng; uint32 rngRequestId; }
File: src/VaultBooster.sol 29 struct Boost { address liquidationPair; UD2x18 multiplierOfTotalSupplyPerSecond; uint96 tokensPerSecond; uint144 available; uint48 lastAccruedAt; }
If a function modifier or require such as onlyOwner/onlyX 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. The extra opcodes avoided are CALLVALUE(2), DUP1(3), ISZERO(3), PUSH2(3), JUMPI(10), PUSH1(3), DUP1(3), REVERT(0), JUMPDEST(1), POP(2) which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost.
file: src/RngAuction.sol 347 function setNextRngService(RNGInterface _rngService) external onlyOwner {
file: src/VaultBooster.sol 142 function setBoost(IERC20 _token, address _liquidationPair, UD2x18 _multiplierOfTotalSupplyPerSecond, uint96 _tokensPerSecond, uint144 _initialAvailable) external onlyOwner { 188 function withdraw(IERC20 _token, uint256 _amount) external onlyOwner { 242 function targetOf(address _tokenIn) external view override onlyPrizeToken(_tokenIn) returns (address) {
File: src/LiquidationPair.sol 279 amount = 0; 337 _amountInForPeriod = 0; 338 _amountOutForPeriod = 0;
Missing checks for zero-addresses may lead to infunctional protocol, if the variable addresses are updated incorrectly. It also wast gas as it requires the redeployment of the contract.
File: src/LiquidationPair.sol constructor( ILiquidationSource _source, address _tokenIn, // <= FOUND address _tokenOut, // <= FOUND uint32 _periodLength, uint32 _periodOffset, uint32 _targetFirstSaleTime, SD59x18 _decayConstant, uint112 _initialAmountIn, uint112 _initialAmountOut, uint256 _minimumAuctionAmount ) { source = _source; tokenIn = _tokenIn; tokenOut = _tokenOut; decayConstant = _decayConstant; periodLength = _periodLength; periodOffset = _periodOffset; targetFirstSaleTime = _targetFirstSaleTime; SD59x18 period59 = convert(int256(uint256(_periodLength))); if (_decayConstant.mul(period59).unwrap() > uEXP_MAX_INPUT) { revert DecayConstantTooLarge(wrap(uEXP_MAX_INPUT).div(period59), _decayConstant); } if (targetFirstSaleTime >= periodLength) { revert TargetFirstSaleTimeLtPeriodLength(targetFirstSaleTime, periodLength); }
File: src/RngAuction.sol constructor( RNGInterface rng_, address owner_, // <= FOUND uint64 sequencePeriod_, uint64 sequenceOffset_, uint64 auctionDurationSeconds_, uint64 auctionTargetTime_ ) Ownable(owner_) { if (sequencePeriod_ == 0) revert SequencePeriodZero(); if (auctionTargetTime_ > auctionDurationSeconds_) revert AuctionTargetTimeExceedsDuration(uint64(auctionTargetTime_), uint64(auctionDurationSeconds_)); sequencePeriod = sequencePeriod_; sequenceOffset = sequenceOffset_; auctionDuration = auctionDurationSeconds_; auctionTargetTime = auctionTargetTime_; _auctionTargetTimeFraction = intoUD2x18(convert(uint(auctionTargetTime_)).div(convert(uint(auctionDurationSeconds_)))); _setNextRngService(rng_); }
File: src/VaultBooster.sol constructor( PrizePool _prizePool, // <= FOUND address _vault, // <= FOUND address _owner // <= FOUND ) Ownable(_owner) { prizePool = _prizePool; // <= FOUND twabController = prizePool.twabController(); vault = _vault; }
File: src/RemoteOwner.sol constructor( uint256 originChainId_, address executor_, // <= FOUND address __originChainOwner // <= FOUND ) ExecutorAware(executor_) { if (originChainId_ == 0) revert OriginChainIdZero(); _originChainId = originChainId_; _setOriginChainOwner(__originChainOwner); }
>=Ā costs less gas thanĀ >File: src/RngRelayAuction.sol 113 if (auctionTargetTime_ > auctionDurationSeconds_) { 140 if (_auctionElapsedSeconds > (_auctionDurationSeconds-1)) revert AuctionExpired();
File: src/libraries/ContinuousGDA.sol 38 if (_emissionRate.unwrap() > 1e18) {
File: src/LiquidationPair.sol 114 if (_decayConstant.mul(period59).unwrap() > uEXP_MAX_INPUT) { 218 if (swapAmountIn > _amountInMax) { 302 if (_amountOut > maxOut) {
File: src/RngAuction.sol 149 if (auctionTargetTime_ > auctionDurationSeconds_) revert AuctionTargetTimeExceedsDuration(uint64(auctionTargetTime_), uint64(auctionDurationSeconds_)); 176 if (_auctionElapsedTimeSeconds > auctionDuration) revert AuctionExpired();
File: libraries/RewardLib.sol 65 for (uint256 i; i < _auctionResultsLength; i++) { _rewards[i] = reward(_auctionResults[i], remainingReserve); remainingReserve = remainingReserve - _rewards[i]; }
File: src/RngRelayAuction.sol 167 for (uint8 i = 0; i < _rewards.length; i++) { uint104 _reward = uint104(_rewards[i]); if (_reward > 0) { prizePool.withdrawReserve(auctionResults[i].recipient, _reward); emit AuctionRewardDistributed(_sequenceId, auctionResults[i].recipient, i, _reward); }
File:src/libraries/ContinuousGDA.sol 14 SD59x18 internal constant ONE = SD59x18.wrap(1e18);
File: src/LiquidationPairFactory.sol 43 LiquidationPair[] public allPairs;
File: src/LiquidationPair.sol 106 tokenIn = _tokenIn; 107 tokenOut = _tokenOut;
File: src/RngRelayAuction.sol 116 rngAuctionRelayer = _rngAuctionRelayer;
File: src/VaultBooster.sol 125 vault = _vault;
File: src/RemoteOwner.sol 106 _originChainOwner = _newOriginChainOwner;
Prior to 0.8.10 the compiler inserted extra code, including EXTCODESIZE (100 gas), to check for contract existence for external function calls. In more recent solidity versions, the compiler will not insert these checks if the external call has a return value. Similar behavior can be achieved in earlier versions by using low-level calls, since low level calls never check for contract existence.
File: src/LiquidationPair.sol 141 return source.targetOf(tokenIn);
File: src/LiquidationRouter.sol 69 IERC20(_liquidationPair.tokenIn()).safeTransferFrom(
File: src/RngAuction.sol 180 if (IERC20(_feeToken).balanceOf(address(this)) < _requestFee) { 182 IERC20(_feeToken).transferFrom(msg.sender, address(this), _requestFee); 185 IERC20(_feeToken).safeIncreaseAllowance(address(rng), _requestFee);
File: src/VaultBooster.sol 226 IERC20(_tokenOut).safeTransfer(_account, _amountOut);
File: src/RngRelayAuction.sol 167 for (uint8 i = 0; i < _rewards.length; i++) { uint104 _reward = uint104(_rewards[i]); if (_reward > 0) { prizePool.withdrawReserve(auctionResults[i].recipient, _reward); emit AuctionRewardDistributed(_sequenceId, auctionResults[i].recipient, i, _reward); } }
File: src/LiquidationPair.sol 378 uint256 _timestamp = block.timestamp;
File: src/abstract/AddressRemapper.sol 33 if (_destinationAddress[_addr] == address(0)) { return _addr; } else { return _destinationAddress[_addr]; }
File: src/libraries/ContinuousGDA.sol 23 function purchasePrice( SD59x18 _amount, SD59x18 _emissionRate, SD59x18 _k, SD59x18 _decayConstant, SD59x18 _timeSinceLastAuctionStart ) internal pure returns (SD59x18) { if (_amount.unwrap() == 0) { return SD59x18.wrap(0); } SD59x18 topE = _decayConstant.mul(_amount).div(_emissionRate); topE = topE.exp().sub(ONE); SD59x18 bottomE = _decayConstant.mul(_timeSinceLastAuctionStart); bottomE = bottomE.exp(); SD59x18 result; if (_emissionRate.unwrap() > 1e18) { result = _k.div(_emissionRate).mul(topE).div(bottomE); } else { result = _k.mul(topE.div(_emissionRate.mul(bottomE))); } return result; }
file: RngRelayAuction.sol 167 for (uint8 i = 0; i < _rewards.length; i++) { uint104 _reward = uint104(_rewards[i]); if (_reward > 0) { prizePool.withdrawReserve(auctionResults[i].recipient, _reward); emit AuctionRewardDistributed(_sequenceId, auctionResults[i].recipient, i, _reward); } }
file: src/libraries/RewardLib.sol 65 for (uint256 i; i < _auctionResultsLength; i++) { _rewards[i] = reward(_auctionResults[i], remainingReserve); remainingReserve = remainingReserve - _rewards[i]; }
Instead of using address(this), it is more gas-efficient to pre-calculate and use the hardcoded address. Foundryās script.sol and solmateās LibRlp.sol contracts can help achieve this.
References:
https://book.getfoundry.sh/reference/forge-std/compute-create-address https://twitter.com/transmissions11/status/1518507047943245824
file: src/RngAuction.sol 180 if (IERC20(_feeToken).balanceOf(address(this)) < _requestFee) { 182 IERC20(_feeToken).transferFrom(msg.sender, address(this), _requestFee);
file: src/VaultBooster.sol 144 uint256 balance = _token.balanceOf(address(this)); 173 _token.safeTransferFrom(msg.sender, address(this), _amount); 190 uint256 availableBalance = _token.balanceOf(address(this)); 276 uint256 availableBalance = _tokenOut.balanceOf(address(this));
file: src/abstract/AddressRemapper.sol 32 function remappingOf(address _addr) public view returns (address) {
file: src/LiquidationRouter.sol 84 modifier onlyTrustedLiquidationPair(LiquidationPair _liquidationPair) {
Both state variables can potentially be set back and forth from true and false. This would result in a Gsset (20000 gas) everytime the values are set to true from false. We can instead use uint(1) in place of true and uint(2) in place of false and pay the Gsset (20000 gas) once during deployment (to set the slot to uint(1). This would save two Gsset (20000 gas). However, a more efficient mitigation would be to pack the variables into a slot with other variables that would inevitably be written to. Please see this finding for a more efficient solution.
file: src/LiquidationPairFactory.sol 51 mapping(LiquidationPair => bool) public deployedPairs;
If similar external calls are performed back-to-back, we can use assembly to reuse any function signatures and function parameters that stay the same. In addition, we can also reuse the same memory space for each function call (scratch space + free memory pointer), which can potentially allow us to avoid memory expansion costs. In this case we are also able to efficiently store both function signatures together in memory as one word, saving one MLOAD in the process.
Note: In order to do this optimization safely we will cache and restore the free memory pointer after we are done with our function calls
file: src/libraries/RewardLib.sol 32 UD60x18 t = UD60x18.wrap(_targetTimeFraction.unwrap()); 33 UD60x18 r = UD60x18.wrap(_targetRewardFraction.unwrap());
File: src/abstract/RngAuctionRelayer.sol 31 function encodeCalldata(address rewardRecipient) internal returns (bytes memory) { if (!rngAuction.isRngComplete()) revert RngNotCompleted(); (uint256 randomNumber, uint64 rngCompletedAt) = rngAuction.getRngResults(); AuctionResult memory results = rngAuction.getLastAuctionResult(); uint32 sequenceId = rngAuction.openSequenceId(); results.recipient = remappingOf(results.recipient); return abi.encodeWithSelector(IRngAuctionRelayListener.rngComplete.selector, randomNumber, rngCompletedAt, rewardRecipient, sequenceId, results); }
The _checkUpdateAuction() internal function performs 14 external calls and returns 14 of the return values from those calls. Certain functions invoke _checkUpdateAuction() but only use the return value from the first external call, thus performing an unnecessary extra external call. We can forgo using the internal function and instead only perform our desired external call. to save (100 gas ) for one external call
file: src/LiquidationPair.sol 145 function maxAmountOut() external returns (uint256) { _checkUpdateAuction();
return data (bool success,) has to be stored due to EVM architecture, but in a usage like below, āoutā and āoutsizeā values are given (0,0), this storage disappears and gas optimization is provided.
file: src/RngAuctionRelayerDirect.sol 39 (bool success, bytes memory returnData) = address(_rngAuctionRelayListener).call(data);
file: src/VaultBooster.sol 173 _token.safeTransferFrom(msg.sender, address(this), _amount); 193 _token.transfer(msg.sender, _amount);
file: src/RngRelayAuction.sol 167 for (uint8 i = 0; i < _rewards.length; i++) { uint104 _reward = uint104(_rewards[i]); if (_reward > 0) { prizePool.withdrawReserve(auctionResults[i].recipient, _reward); emit AuctionRewardDistributed(_sequenceId, auctionResults[i].recipient, i, _reward); } }
#0 - c4-judge
2023-08-14T11:18:11Z
HickupHH3 marked the issue as grade-b