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: 21/45
Findings: 2
Award: $129.80
š 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 | Gas Optimization Details | Context |
---|---|---|
[G-01] | Avoid contract existence checks by using low level calls | 6 |
[G-02] | Functions Guaranteed to revert when called by normal users can be markd payable | 5 |
[G-03] | Stack variable used as a cheaper cache for a state variable is only used once | 1 |
[G-04] | RngAuction::sequenceOffset should be cached in stack variables rather than re-reading them from storage | 11 |
[G-05] | Make the State Variabel outside the loop to save gas | 2 |
[G-06] | Using calldata instead of memory for read-only arguments | 1 |
[G-07] | Internal functions not called by the contract should be removed to save deployment gas | 1 |
[G-08] | Structs can be packed into fewer storage slots by editing variables | 2 |
[G-09] | Use nested if and, avoid multiple check combinations && | 5 |
[G-10] | UseĀ assemblyĀ to writeĀ address storage values | 6 |
[G-11] | Caching global variables is more expensive than using the actual variable | 1 |
[G-12] | A modifier used only once and not being inherited should be inlined to save gas | 3 |
[G-13] | Do not calculate constant | 1 |
[G-14] | >= Ā costs less gas thanĀ > | 15 |
[G-15] | UseĀ do while loops instead ofĀ forĀ loops | 2 |
[G-16] | Missing zero address check in constructor | 8 |
[G-17] | Ternary operation is cheaper than if-else statement | 2 |
[G-18] | Use assembly to hash instead of solidity | 2 |
[G-19] | Use hardcode address instead address(this) | 6 |
[G-20] | Use mappings instead of arrays for possible gas save | 1 |
[G-21] | UsingĀ deleteĀ statement can save gas | 4 |
[G-22] | Remove importĀ forge-std | 1 |
[G-23] | Don't use _msgSender() if not supporting EIP-2771 | 1 |
[G-24] | Make 3 event parameters indexed when possible | 26 |
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);
The onlyTrustedLiquidationPair
modifierĀ makes a function revert if not called by normal users
File: src/LiquidationRouter.sol 68 ) external onlyTrustedLiquidationPair(_liquidationPair) returns (uint256) {
The onlyOwner
modifierĀ makes a function revert if not called by the address registered as the owner
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 { 217 ) external override onlyPrizeToken(_tokenIn) onlyLiquidationPair(_tokenOut) returns (bool) { 242 function targetOf(address _tokenIn) external view override onlyPrizeToken(_tokenIn) returns (address) {
File: src/libraries/ContinuousGDA.sol 14 SD59x18 internal constant ONE = SD59x18.wrap(1e18);
RngAuction::sequenceOffset
should be cached in stack variables rather than re-reading them from storageFile: src/RngAuction.sol function _openSequenceId() internal view returns (uint32) { /** * Use integer division to calculate a unique ID based off the current timestamp that will remain the same * throughout the entire sequence. */ uint64 currentTime = _currentTime(); if (currentTime < sequenceOffset) { return 0; } return uint32((currentTime - sequenceOffset) / sequencePeriod); }
diff --git a/RngAuction.org.sol b/RngAuction.sol index 62ebb34..8164f77 100644 --- a/RngAuction.org.sol +++ b/RngAuction.sol @@ -1,11 +1,14 @@ function _openSequenceId() internal view returns (uint32) { + + uint64 _SequenceOffset = sequenceOffset; + /** * Use integer division to calculate a unique ID based off the current timestamp that will remain the same * throughout the entire sequence. */ uint64 currentTime = _currentTime(); - if (currentTime < sequenceOffset) { + if (currentTime < _SequenceOffset) { return 0; } - return uint32((currentTime - sequenceOffset) / sequencePeriod); + return uint32((currentTime - _SequenceOffset) / sequencePeriod); }
File: src/RngAuction.sol function _auctionElapsedTime() internal view returns (uint64) { uint64 currentTime = _currentTime(); if (currentTime < sequenceOffset) { return 0; } return (_currentTime() - sequenceOffset) % sequencePeriod; }
diff --git a/RngAuction.org.sol b/RngAuction.sol index 4fd9320..98de5e2 100644 --- a/RngAuction.org.sol +++ b/RngAuction.sol @@ -1,7 +1,9 @@ function _auctionElapsedTime() internal view returns (uint64) { + uint64 _SequenceOffset = sequenceOffset; + uint64 currentTime = _currentTime(); - if (currentTime < sequenceOffset) { + if (currentTime < _SequenceOffset) { return 0; } - return (_currentTime() - sequenceOffset) % sequencePeriod; + return (_currentTime() - _SequenceOffset) % sequencePeriod; }
RngAuction::_lastAuction
should be cached in stack variables rather than re-reading them from storageFile: src/RngAuction.sol function getLastAuctionResult() external view returns (AuctionResult memory) { address recipient = _lastAuction.recipient; UD2x18 rewardFraction = _lastAuction.rewardFraction; return AuctionResult({ recipient: recipient, rewardFraction: rewardFraction }); }
diff --git a/RngAuction.org.sol b/RngAuction.sol index b69878b..eb6a903 100644 --- a/RngAuction.org.sol +++ b/RngAuction.sol @@ -3,6 +3,10 @@ view returns (AuctionResult memory) { + RngAuctionResult lastAuction = _lastAuction; + address recipient = lastAuction.recipient; + UD2x18 rewardFraction = lastAuction.rewardFraction; - address recipient = _lastAuction.recipient; - UD2x18 rewardFraction = _lastAuction.rewardFraction; return AuctionResult({
File: src/RngAuction.sol function getRngResults() external returns ( uint256 randomNumber, uint64 rngCompletedAt ) { RNGInterface rng = _lastAuction.rng; uint32 requestId = _lastAuction.rngRequestId; return (rng.randomNumber(requestId), rng.completedAt(requestId)); } /// @notice Computes the reward fraction for the given auction elapsed time. function computeRewardFraction(uint64 __auctionElapsedTime) external view returns (UD2x18) { return _computeRewardFraction(__auctionElapsedTime); }
diff --git a/RngAuction.org.sol b/RngAuction.sol index 6eb05b8..f1c5428 100644 --- a/RngAuction.org.sol +++ b/RngAuction.sol @@ -4,8 +4,10 @@ uint256 randomNumber, uint64 rngCompletedAt ) { + RngAuctionResult lastAuction = _lastAuction; + RNGInterface rng = lastAuction.rng; + uint32 requestId = lastAuction.rngRequestId; - RNGInterface rng = _lastAuction.rng; - uint32 requestId = _lastAuction.rngRequestId; return (rng.randomNumber(requestId), rng.completedAt(requestId)); }
File: src/RngAuction.sol function _isRngComplete() internal view returns (bool) { RNGInterface rng = _lastAuction.rng; uint32 requestId = _lastAuction.rngRequestId; return !_canStartNextSequence() && rng.isRequestComplete(requestId); }
diff --git a/RngAuction.org.sol b/RngAuction.sol index 81744e6..6a20cd0 100644 --- a/RngAuction.org.sol +++ b/RngAuction.sol @@ -1,4 +1,6 @@ function _isRngComplete() internal view returns (bool) { + RngAuctionResult lastAuction = _lastAuction; - RNGInterface rng = _lastAuction.rng; - uint32 requestId = _lastAuction.rngRequestId; + RNGInterface rng = lastAuction.rng; + uint32 requestId = lastAuction.rngRequestId; return !_canStartNextSequence() && rng.isRequestComplete(requestId);
RngAuctionRelayer::rngAuction
should be cached in stack variables rather than re-reading them from storageFile: src/abstract/RngAuctionRelayer.sol 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); }
diff --git a/RngAuctionRelayer.org.sol b/RngAuctionRelayer.sol index d196760..c220226 100644 --- a/RngAuctionRelayer.org.sol +++ b/RngAuctionRelayer.sol @@ -1,8 +1,9 @@ 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(); + RngAuction _rngAuction = rngAuction; + 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); }
RngRelayAuction::prizePool
should be cached in stack variables rather than re-reading them from storageFile: src/RngRelayAuction.sol function rngComplete( uint256 _randomNumber, uint256 _rngCompletedAt, address _rewardRecipient, uint32 _sequenceId, AuctionResult calldata _rngAuctionResult ) external returns (bytes32) { if (_sequenceHasCompleted(_sequenceId)) revert SequenceAlreadyCompleted(); uint64 _auctionElapsedSeconds = uint64(block.timestamp < _rngCompletedAt ? 0 : block.timestamp - _rngCompletedAt); if (_auctionElapsedSeconds > (_auctionDurationSeconds-1)) revert AuctionExpired(); // Calculate the reward fraction and set the draw auction results UD2x18 rewardFraction = _fractionalReward(_auctionElapsedSeconds); _auctionResults.rewardFraction = rewardFraction; _auctionResults.recipient = _rewardRecipient; _lastSequenceId = _sequenceId; AuctionResult[] memory auctionResults = new AuctionResult[](2); auctionResults[0] = _rngAuctionResult; auctionResults[1] = AuctionResult({ rewardFraction: rewardFraction, recipient: _rewardRecipient }); uint32 drawId = prizePool.closeDraw(_randomNumber); uint256 futureReserve = prizePool.reserve() + prizePool.reserveForOpenDraw(); uint256[] memory _rewards = RewardLib.rewards(auctionResults, futureReserve); emit RngSequenceCompleted( _sequenceId, drawId, _rewardRecipient, _auctionElapsedSeconds, rewardFraction ); 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); } } return bytes32(uint(drawId)); }
diff --git a/RngRelayAuction.org.sol b/RngRelayAuction.sol index a24c414..b73ad58 100644 --- a/RngRelayAuction.org.sol +++ b/RngRelayAuction.sol @@ -5,6 +5,9 @@ uint32 _sequenceId, AuctionResult calldata _rngAuctionResult ) external returns (bytes32) { + + PrizePool _prizePool = prizePool + if (_sequenceHasCompleted(_sequenceId)) revert SequenceAlreadyCompleted(); uint64 _auctionElapsedSeconds = uint64(block.timestamp < _rngCompletedAt ? 0 : block.timestamp - _rngCompletedAt); if (_auctionElapsedSeconds > (_auctionDurationSeconds-1)) revert AuctionExpired(); @@ -21,9 +24,9 @@ recipient: _rewardRecipient }); - uint32 drawId = prizePool.closeDraw(_randomNumber); + uint32 drawId = _prizePool.closeDraw(_randomNumber); - uint256 futureReserve = prizePool.reserve() + prizePool.reserveForOpenDraw(); + uint256 futureReserve = _prizePool.reserve() + _prizePool.reserveForOpenDraw(); uint256[] memory _rewards = RewardLib.rewards(auctionResults, futureReserve); emit RngSequenceCompleted( @@ -37,7 +40,7 @@ for (uint8 i = 0; i < _rewards.length; i++) { uint104 _reward = uint104(_rewards[i]); if (_reward > 0) { - prizePool.withdrawReserve(auctionResults[i].recipient, _reward); + _prizePool.withdrawReserve(auctionResults[i].recipient, _reward); emit AuctionRewardDistributed(_sequenceId, auctionResults[i].recipient, i, _reward); }
File: src/RngRelayAuction.sol function computeRewards(AuctionResult[] calldata __auctionResults) external returns (uint256[] memory) { uint256 totalReserve = prizePool.reserve() + prizePool.reserveForOpenDraw(); return _computeRewards(__auctionResults, totalReserve); }
diff --git a/RngRelayAuction.org.sol b/RngRelayAuction.sol index 9d63e05..83a0af4 100644 --- a/RngRelayAuction.org.sol +++ b/RngRelayAuction.sol @@ -1,4 +1,6 @@ function computeRewards(AuctionResult[] calldata __auctionResults) external returns (uint256[] memory) { + PrizePool _prizePool = prizePool + + uint256 totalReserve = _prizePool.reserve() + _prizePool.reserveForOpenDraw(); - uint256 totalReserve = prizePool.reserve() + prizePool.reserveForOpenDraw(); return _computeRewards(__auctionResults, totalReserve); }
File: src/RngRelayAuction.sol 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); } }
diff --git a/org.sol b/not.sol index a76849b..5a505c3 100644 --- a/org.sol +++ b/not.sol @@ -1,6 +1,8 @@ + PrizePool _prizePool = prizePool; for (uint8 i = 0; i < _rewards.length; i++) { uint104 _reward = uint104(_rewards[i]); if (_reward > 0) { - prizePool.withdrawReserve(auctionResults[i].recipient, _reward); + _prizePool.withdrawReserve(auctionResults[i].recipient, _reward); emit AuctionRewardDistributed(_sequenceId, auctionResults[i].recipient, i, _reward); }
File: src/libraries/RewardLib.sol 59 AuctionResult[] memory _auctionResults,
File: src/abstract/RngAuctionRelayer.sol 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); }
File: src/RngAuction.sol struct RngAuctionResult { address recipient; UD2x18 rewardFraction; uint32 sequenceId; RNGInterface rng; uint32 rngRequestId; }
Save 1 slot
diff --git a/RngAuction.org.sol b/RngAuction.sol index 6d7a1ba..a2adaa2 100644 --- a/RngAuction.org.sol +++ b/RngAuction.sol @@ -1,7 +1,7 @@ struct RngAuctionResult { address recipient; UD2x18 rewardFraction; + RNGInterface rng; uint32 sequenceId; - RNGInterface rng; uint32 rngRequestId; }
File: src/VaultBooster.sol struct Boost { address liquidationPair; UD2x18 multiplierOfTotalSupplyPerSecond; uint96 tokensPerSecond; uint144 available; uint48 lastAccruedAt; }
The change swaps the positions of the lastAccruedAt and multiplierOfTotalSupplyPerSecond variables. Since uint48 occupies 48 bits and UD2x18 (assuming it's a 2-decimal fixed-point number) occupies 36 bits (18 bits for each decimal) and address occupies 160 bits, these three variables together will take up 244 bits, which fits within a single storage slot (256 bits).
diff --git a/VaultBooster.org.sol b/VaultBooster.sol index dfa2f96..a430c42 100644 --- a/VaultBooster.org.sol +++ b/VaultBooster.sol @@ -1,7 +1,7 @@ struct Boost { address liquidationPair; + uint48 lastAccruedAt; UD2x18 multiplierOfTotalSupplyPerSecond; uint96 tokensPerSecond; uint144 available; - uint48 lastAccruedAt; }
&&
File: src/LiquidationPair.sol 332 if (_amountInForPeriod > 0 && _amountOutForPeriod > 0) {
diff --git a/LiquidationPair.org.sol b/LiquidationPair.sol index f7586f6..85eb4e3 100644 --- a/LiquidationPair.org.sol +++ b/LiquidationPair.sol @@ -1,6 +1,8 @@ function _updateAuction(uint256 __period) internal { - if (_amountInForPeriod > 0 && _amountOutForPeriod > 0) { + if (_amountInForPeriod > 0 ){ + if ( _amountOutForPeriod > 0) { // if we sold something, then update the previous non-zero amount _lastNonZeroAmountIn = _amountInForPeriod; _lastNonZeroAmountOut = _amountOutForPeriod; + } }
File: src/RngAuction.sol 179 if (_feeToken != address(0) && _requestFee > 0) {
diff --git a/RngAuction.org.sol b/RngAuction.sol index 2d7c938..2c65f0f 100644 --- a/RngAuction.org.sol +++ b/RngAuction.sol @@ -1,3 +1,4 @@ (address _feeToken, uint256 _requestFee) = rng.getRequestFee(); - if (_feeToken != address(0) && _requestFee > 0) { + if (_feeToken != address(0)) { + if( _requestFee > 0) { if (IERC20(_feeToken).balanceOf(address(this)) < _requestFee) {
File: src/LiquidationPair.sol 106 tokenIn = _tokenIn; 107 tokenOut = _tokenOut;
diff --git a/LiquidationPair.org.sol b/LiquidationPair.sol index 2a1892d..216c674 100644 --- a/LiquidationPair.org.sol +++ b/LiquidationPair.sol @@ -11,9 +11,11 @@ uint256 _minimumAuctionAmount ) { source = _source; - tokenIn = _tokenIn; - tokenOut = _tokenOut; + assembly { + sstore(tokenIn.slot, _tokenIn) + sstore(tokenOut.slot, _tokenOut) + } decayConstant = _decayConstant; periodLength = _periodLength; periodOffset = _periodOffset;
File: src/RngRelayAuction.sol 116 rngAuctionRelayer = _rngAuctionRelayer;
File: src/VaultBooster.sol 125 vault = _vault;
File: src/RemoteOwner.sol 106 _originChainOwner = _newOriginChainOwner;
File: src/LiquidationPair.sol 378 uint256 _timestamp = block.timestamp;
File: src/LiquidationRouter.sol modifier onlyTrustedLiquidationPair(LiquidationPair _liquidationPair) { if (!_liquidationPairFactory.deployedPairs(_liquidationPair)) { revert UnknownLiquidationPair(_liquidationPair); } _; }
The above Modifier onlyTrustedLiquidationPair
only use in this function It should be inlined to save gas
File: src/LiquidationRouter.sol function swapExactAmountOut( LiquidationPair _liquidationPair, address _receiver, uint256 _amountOut, uint256 _amountInMax ) external onlyTrustedLiquidationPair(_liquidationPair) returns (uint256) { IERC20(_liquidationPair.tokenIn()).safeTransferFrom( msg.sender, _liquidationPair.target(), _liquidationPair.computeExactAmountIn(_amountOut) ); uint256 amountIn = _liquidationPair.swapExactAmountOut(_receiver, _amountOut, _amountInMax); emit SwappedExactAmountOut(_liquidationPair, _receiver, _amountOut, _amountInMax, amountIn); return amountIn; }
constant
File:src/libraries/ContinuousGDA.sol 14 SD59x18 internal constant ONE = SD59x18.wrap(1e18);
>=
Ā costs less gas thanĀ >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) { 332 if (_amountInForPeriod > 0 && _amountOutForPeriod > 0) {
File: src/RngAuction.sol 149 if (auctionTargetTime_ > auctionDurationSeconds_) revert AuctionTargetTimeExceedsDuration(uint64(auctionTargetTime_), uint64(auctionDurationSeconds_)); 176 if (_auctionElapsedTimeSeconds > auctionDuration) revert AuctionExpired(); 179 if (_feeToken != address(0) && _requestFee > 0) {
File: src/RngRelayAuction.sol 113 if (auctionTargetTime_ > auctionDurationSeconds_) { 140 if (_auctionElapsedSeconds > (_auctionDurationSeconds-1)) revert AuctionExpired(); 169 if (_reward > 0) {
File: src/VaultBooster.sol 143 if (_initialAvailable > 0) { 219 if (_amountOut > amountAvailable) { 269 if (boost.tokensPerSecond > 0) { 272 if (boost.multiplierOfTotalSupplyPerSecond.unwrap() > 0) {
while
loops instead ofĀ forĀ loops
https://code4rena.com/reports/2023-05-ajna#g-09-use-do-while-loops-instead-of-for-loops
File: libraries/RewardLib.sol for (uint256 i; i < _auctionResultsLength; i++) { _rewards[i] = reward(_auctionResults[i], remainingReserve); remainingReserve = remainingReserve - _rewards[i]; }
diff --git a/RewardLib.org.sol b/RewardLib.sol index c7237a8..f006fb3 100644 --- a/RewardLib.org.sol +++ b/RewardLib.sol @@ -1,10 +1,11 @@ rewardFraction ); - for (uint8 i = 0; i < _rewards.length; i++) { + uint8 i = 0; + do { uint104 _reward = uint104(_rewards[i]); if (_reward > 0) { prizePool.withdrawReserve(auctionResults[i].recipient, _reward); emit AuctionRewardDistributed(_sequenceId, auctionResults[i].recipient, i, _reward); } - } + } while (i < _rewards.length);
File: src/RngRelayAuction.sol 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); }
constructor
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, address _tokenOut, 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_, 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, address _vault, address _owner ) Ownable(_owner) { prizePool = _prizePool; twabController = prizePool.twabController(); vault = _vault; }
File: src/RemoteOwner.sol constructor( uint256 originChainId_, address executor_, address __originChainOwner ) ExecutorAware(executor_) { if (originChainId_ == 0) revert OriginChainIdZero(); _originChainId = originChainId_; _setOriginChainOwner(__originChainOwner); }
Ternary
operation is cheaper than if-else
statementFile: src/abstract/AddressRemapper.sol if (_destinationAddress[_addr] == address(0)) { return _addr; } else { return _destinationAddress[_addr]; }
diff --git a/AddressRemapper.org.sol b/AddressRemapper.sol index e47b41d..c21492c 100644 --- a/AddressRemapper.org.sol +++ b/AddressRemapper.sol @@ -1,7 +1,3 @@ function remappingOf(address _addr) public view returns (address) { - if (_destinationAddress[_addr] == address(0)) { - return _addr; - } else { - return _destinationAddress[_addr]; - } + return (_destinationAddress[_addr] == address(0)) ? _addr : _destinationAddress[_addr];
File: src/libraries/ContinuousGDA.sol 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; }
diff --git a/ContinuousGDA.org.sol b/ContinuousGDA.sol index 3f1a6b1..fa259e6 100644 --- a/ContinuousGDA.org.sol +++ b/ContinuousGDA.sol @@ -13,10 +13,6 @@ 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; + + return result = (_emissionRate.unwrap() > 1e18) ? _k.div(_emissionRate).mul(topE).div(bottomE) : _k.mul(topE.div(_emissionRate.mul(bottomE))); }
assembly
to hash instead of solidityA good examples to how to use assembly for optimization Resource
File: src/libraries/RemoteOwnerCallEncoder.sol return abi.encodeWithSelector( RemoteOwner.execute.selector, target, value, data );
File: src/abstract/RngAuctionRelayer.sol 37 return abi.encodeWithSelector(IRngAuctionRelayListener.rngComplete.selector, randomNumber, rngCompletedAt, rewardRecipient, sequenceId, results);
address(this)
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/RngAuction.sol 180 if (IERC20(_feeToken).balanceOf(address(this)) < _requestFee) { 182 IERC20(_feeToken).transferFrom(msg.sender, address(this), _requestFee);
File: src/LiquidationPairFactory.sol 43 LiquidationPair[] public allPairs;
File: src/LiquidationPair.sol 279 amount = 0; 337 _amountInForPeriod = 0; 338 _amountOutForPeriod = 0;
It's used to print the values of variables while running tests to help debug and see what's happening inside your contracts but since it's a development tool, it serves no purpose on mainnet. Also, the remember to remove the usage of calls that useĀ forge-stdĀ when removing of the import ofĀ forge-std.
File: src/VaultBooster.sol 4 import "forge-std/console2.sol";
File: src/RemoteOwner.sol 120 if (_msgSender() != address(_originChainOwner)) revert OriginSenderNotOwner(_msgSender());
Events are used to emit information about state changes in a contract. When defining an event, it's important to consider the gas cost of emitting the event, as well as the efficiency of searching for events in the Ethereum blockchain.
Making event parameters indexed can help reduce the gas cost of emitting events and improve the efficiency of searching for events. When an event parameter is marked as indexed, its value is stored in a separate data structure called the event topic, which allows
File: src/LiquidationPairFactory.sol event PairCreated( LiquidationPair indexed pair, ILiquidationSource source, address tokenIn, address tokenOut, uint32 periodLength, uint32 periodOffset, uint32 targetFirstSaleTime, SD59x18 decayConstant, uint112 initialAmountIn, uint112 initialAmountOut, uint256 minimumAuctionAmount );
File: src/LiquidationRouter.sol event SwappedExactAmountOut( LiquidationPair indexed liquidationPair, address indexed receiver, uint256 amountOut, uint256 amountInMax, uint256 amountIn );
File: src/RngRelayAuction.sol event AuctionRewardDistributed( uint32 indexed sequenceId, address indexed recipient, uint32 index, uint256 reward ); event RngSequenceCompleted( uint32 indexed sequenceId, uint32 indexed drawId, address indexed rewardRecipient, uint64 auctionElapsedSeconds, UD2x18 rewardFraction );
File: src/VaultBooster.sol event SetBoost( IERC20 indexed token, address liquidationPair, UD2x18 multiplierOfTotalSupplyPerSecond, uint96 tokensPerSecond, uint144 initialAvailable, uint48 lastAccruedAt ); event Liquidated( IERC20 indexed token, address indexed from, uint256 amountIn, uint256 amountOut, uint256 availableBoostBalance );
File: src/RemoteOwner.sol event OriginChainOwnerSet(address owner);
#0 - c4-judge
2023-08-14T11:16:29Z
HickupHH3 marked the issue as grade-b
š Selected for report: 3agle
Also found by: 0xSmartContract, 0xmystery, DedOhWale, K42, cholakov, hunter_w3b
My primary objective is to enhance the gas efficiency and cost-effectiveness of the protocol. Throughout my analysis, I have diligently pursued opportunities to optimize gas consumption and have prepared a comprehensive report comprising 23
detailed gas optimization recommendations tailored to the specific needs of the protocol.
It is my firm belief that the implementation of these optimization techniques can lead to substantial improvements in the protocol's performance, especially concerning gas fees. By carefully reviewing and implementing these suggestions, the protocol stands to benefit from reduced transaction costs and enhanced overall efficiency.
I am confident that my gas optimization expertise and the comprehensive report submitted will prove instrumental in streamlining the protocol's gas usage and making it more competitive in an environment of ever-changing gas fees.
Accordingly, I analyzed and audited the subject in the following steps;
Core Protocol Contracts Overview: In this step, I will conduct a comprehensive evaluation of the core smart contracts that constitute the PoolTogether V5 protocol. The contracts within the scope of this audit encompass critical components such as LiquidationPair, Draw Auction, Remote Owner, and Vault Boost, among others.The PoolTogether V5 protocol introduces novel auction mechanisms, with specific contracts utilizing Continuous Gradual Dutch Auction (CGDA) and Parabolic Fractional Dutch Auction (PFDA) models. These models enable effective yield liquidation and RNG request auctions while allowing seamless interaction between L1 and L2 Prize Pools.
Documentation Review:
Liquidation Pair:The Liquidation Pair in PoolTogether V5 is a crucial contract responsible for executing Periodic Continuous Gradual Dutch Auctions (CGDA) to convert yield into prize tokens. It ensures that each Vault has its own Liquidation Pair, aligns with draw lengths for regular auctions, and aims for a 95.2% liquidation efficiency. Security concerns include the potential for permanent breakage or deliberate/accidental bricking.
Draw Auction: The Draw Auction is a pivotal component of the PoolTogether V5 protocol, specifically designed to ensure a seamless and permissionless process for conducting daily prize pool draws. In contrast to previous versions, the V5 protocol eliminates the need for permissioned controls, relying instead on a sophisticated incentivization mechanism through timed auctions, the Draw Auction empowers the PoolTogether V5 protocol with a robust and transparent process for conducting prize pool draws. By leveraging timed auctions and PFDA pricing, this incentivization approach fosters an ecosystem of self-sufficiency, enabling seamless daily operations across all deployed prize pools.
Remote Owner: The Remote Owner is a pivotal contract within the PoolTogether V5 protocol, extending its control from one chain to another through ERC-5164 integration. This unique contract empowers a contract deployed on one chain to govern and control a contract situated on a different chain. Its role is critical in achieving cross-chain functionality and interconnectivity within the protocol,By building upon ERC-5164, the Remote Owner facilitates seamless and secure communication between contracts on separate chains, allowing for efficient cross-chain operations. It enables actions initiated on one chain to have corresponding effects on a contract deployed on another, fostering a unified and decentralized ecosystem.
Vault Boost: Vault Boost is a fundamental contract within the PoolTogether V5 protocol, introducing a novel mechanism that empowers anyone to liquidate any token and make contributions to the prize pool on behalf of a vault. This feature enhances the protocol's flexibility and inclusivity, enabling participants to actively participate in the growth and success of the prize pool,With the power to facilitate token liquidation and direct contributions to the prize pool, Vault Boost significantly enhances the overall liquidity and utility of the protocol. By permitting users to take a proactive role in supporting the prize pool, it strengthens the protocol's sustainability and attractiveness.
Graphical Analysis: Solidity-metrics is a popular tool used to analyze and visualize Solidity code. It provides various metrics and visualizations to gain insights into the codebase's complexity and maintainability. solidity-metrics
Utilizing Static Analysis Tools: During the static analysis phase, I have discovered specific vulnerabilities in the smart contract. However, I want to bring to your attention that these vulnerabilities are already known issues and are associated with BotRace
.
Test Coverage Evaluation: I currently encounter challenges with the foundry and, regrettably, am not proficient in resolving these issues. As a result, I am unable to run the tests effectively.
Manuel Code Review
Decentralized RNG Source: While Chainlink VRF V2 is a solid initial choice for the RNG source, consider exploring the integration of multiple decentralized RNG solutions. This approach can provide additional layers of security and decentralization, reducing the risk of a single point of failure and enhancing the overall robustness of the Draw Auction system.
Dynamic Auction Duration: Implement an adaptive auction duration based on the auction's reserve size and expected demand. Shorter auction durations during periods of high activity can increase auction efficiency, while longer durations during periods of low activity can give participants more time to bid accurately. This dynamic approach can optimize the auction process and adapt to varying market conditions.
Incentives Mechanism: The architecture should implement a rewards and incentives mechanism to motivate users to participate in liquidations actively. Users can earn governance tokens, yield farming rewards, or other benefits based on their liquidation contributions.
Multi-Token Reward System: Allow users to deposit different tokens into the RngAuction contract, each representing a separate share in the prize pool. This would provide more flexibility for users and allow the protocol to accept a broader range of assets.
Overview of the Codebase:
LiquidationPair.sol: This contract facilitates Periodic Continuous Gradual Dutch Auctions (CGDA) for yield. It utilizes the prb-math library to implement the CGDA formulas.
LiquidationPairFactory.sol: This contract is responsible for creating new LiquidationPairs.
LiquidationRouter.sol: This contract acts as the user-facing interface for LiquidationPairs. Users interact with this contract to initiate liquidations.
ContinuousGDA.sol: This library provides implementations of the Continuous Gradual Dutch Auction (CGDA) formulas using the prb-math library.
RngAuction.sol: This contract auctions off an RNG (Random Number Generator) request. It uses the prb-math, openzeppelin, owner-manager-contracts, and pt-v5-rng-contracts.
RngAuctionRelayerDirect.sol: This contract relays RNG auction results to a listener directly.
RngAuctionRelayerRemoteOwner.sol: This contract relays RNG auction results over an ERC-5164 bridge to a listener. It uses ERC5164.
RngRelayAuction.sol: This contract auctions off the RNG result relay. It uses ERC5164.
RewardLib.sol: This library implements Parabolic Fractional Dutch Auction math using the prb-math library.
IRngAuctionRelayListener.sol: This interface defines the listener for the RNG auction relay.
IAuction.sol: This interface defines common auction functions.
RngAuctionRelayer.sol: This is the base class for relayers.
AddressRemapper.sol: This contract allows addresses to remap themselves for relayers.
VaultBooster.sol: This contract allows anyone to liquidate tokens to boost the chances of a Vault winning.
VaultBoosterFactory.sol: This contract creates new Vault Booster contracts.
RemoteOwner.sol: This contract enables a contract on one chain to control a contract on another using ERC-5164.
RemoteOwnerCallEncoder.sol: This library handles encoding remote calls for the RemoteOwner contract.
The codebase consists of several contracts and libraries that facilitate different functionalities related to auctions, RNG requests, liquidation, and boosting the chances of a Vault winning. Some contracts are responsible for relaying information between different chains, and others implement auction-related logic using mathematical formulas. The contracts utilize external libraries like prb-math and openzeppelin to enhance their functionality.
Centralization of Vaults in the PoolTogether V5 protocol can indeed pose significant risks to the overall decentralization and security of the system. When a substantial number of Vaults are controlled by a few entities or individuals, it creates a concentration of power and control within the protocol.
Unfair Competition: If a few entities control a large number of Vaults, it may deter new participants from entering the system. This lack of competition can harm the protocol's overall health and growth, limiting the potential benefits of decentralization and open participation.
Price Manipulation: Centralized control of Vaults can lead to market manipulation. Entities with significant control over Vaults may coordinate their actions to influence token prices or the outcome of auctions, impacting the fair operation of the protocol.
Liquidity Risk: Inadequate liquidity in the auction or liquidation processes could lead to inefficient price discovery, potentially causing large price fluctuations or failed liquidations.
RNG Manipulation: The randomness provided by the RNG service is critical for the fairness of the protocol. Any manipulation or compromise of the RNG service could result in predictable outcomes, impacting the integrity of the entire system.
16 hours
#0 - c4-judge
2023-08-14T11:01:54Z
HickupHH3 marked the issue as grade-b