Platform: Code4rena
Start Date: 26/09/2022
Pot Size: $50,000 USDC
Total HM: 13
Participants: 113
Period: 5 days
Judge: 0xean
Total Solo HM: 6
Id: 166
League: ETH
Rank: 16/113
Findings: 2
Award: $442.91
š Selected for report: 1
š Solo Findings: 0
š Selected for report: 0xNazgul
Also found by: 0x1f8b, 0x52, 0xDecorativePineapple, 0xSmartContract, 0xmatt, Aeros, Aymen0909, Bnke0x0, Chom, CodingNameKiki, Deivitto, DimitarDimitrov, IllIllI, JC, Jeiwan, Lambda, Matin, Migue, Mukund, Ocean_Sky, Olivierdem, RaymondFam, RockingMiles, Rolezn, Ruhum, Satyam_Sharma, Shinchan, Tomo, Trabajo_de_mates, V_B, Waze, __141345__, a12jmx, ajtra, asutorufos, aysha, brgltd, bulej93, carrotsmuggler, catchup, cccz, chrisdior4, cryptonue, cryptphi, d3e4, defsec, delfin454000, durianSausage, erictee, fatherOfBlocks, gogo, kaden, karanctf, ladboy233, lukris02, mahdikarimi, martin, mics, natzuu, oyc_109, p_crypt0, pedr02b2, rbserver, reassor, rotcivegaf, rvierdiiev, sikorico, slowmoses, sorrynotsorry, tnevler, trustindistrust
207.8103 USDC - $207.81
| Issue | Instances | |
|---|---|---|
| [Lā01] | There will be no liquidity cooldowns for the first few blocks in ~84 years | 1 |
| [Lā02] | TWAP answers will be wrong in ~84 years | 1 |
| [Lā03] | require() should be used instead of assert() | 1 |
| [Lā04] | Missing checks for address(0x0) when assigning values to address state variables | 7 |
Total: 10 instances over 4 issues
| Issue | Instances | |
|---|---|---|
| [Nā01] | require()/revert() statements should have descriptive reason strings | 27 |
| [Nā02] | 2**<n> - 1 should be re-written as type(uint<n>).max | 1 |
| [Nā03] | constants should be defined rather than using magic numbers | 41 |
| [Nā04] | Large multiples of ten should use scientific notation (e.g. 1e6) rather than decimal literals (e.g. 1000000), for readability | 1 |
| [Nā05] | Use bit shifts in an imutable variable rather than long bit masks of a single bit, for readability | 2 |
| [Nā06] | Use a more recent version of solidity | 5 |
| [Nā07] | Use a more recent version of solidity | 1 |
| [Nā08] | Lines are too long | 1 |
| [Nā09] | Variable names that consist of all capital letters should be reserved for constant/immutable variables | 2 |
| [Nā10] | File is missing NatSpec | 4 |
| [Nā11] | NatSpec is incomplete | 7 |
| [Nā12] | Not using the named return variables anywhere in the function is confusing | 33 |
Total: 125 instances over 12 issues
Since _blockTimestamp() truncates block.timestamp to uint32, once the current timestamp passes type(uint32).max, the subtraction below will underflow, causing the require() statement to pass, regardless of the time since the last liquidity addition
There is 1 instance of this issue:
File: /src/core/contracts/AlgebraPool.sol 229: require((_blockTimestamp() - lastLiquidityAddTimestamp) >= _liquidityCooldown);
Like the issue above, the timestamp in the calculation below will wrap, leading to answers about the TWAP value to be wrong
There is 1 instance of this issue:
File: /src/core/contracts/libraries/DataStorage.sol 133: avgTick = (lastTimestamp == oldestTimestamp) ? tick : (lastTickCumulative - oldestTickCumulative) / (lastTimestamp - oldestTimestamp);
require() should be used instead of assert()Prior to solidity version 0.8.0, hitting an assert consumes the remainder of the transaction's available gas rather than returning it, as require()/revert() do. assert() should be avoided even past solidity version 0.8.0 as its documentation states that "The assert function creates an error of type Panic(uint256). ... Properly functioning code should never create a Panic, not even on invalid external input. If this happens, then there is a bug in your contract which you should fix".
There is 1 instance of this issue:
File: src/core/contracts/libraries/DataStorage.sol 190: assert(false);
address(0x0) when assigning values to address state variablesThere are 7 instances of this issue:
File: src/core/contracts/AlgebraFactory.sol 54: poolDeployer = _poolDeployer; 55: vaultAddress = _vaultAddress; 80: owner = _owner; 87: farmingAddress = _farmingAddress; 94: vaultAddress = _vaultAddress;
File: src/core/contracts/AlgebraPool.sol 961: activeIncentive = virtualPoolAddress;
File: src/core/contracts/DataStorageOperator.sol 33: pool = _pool;
require()/revert() statements should have descriptive reason stringsThere are 27 instances of this issue:
File: src/core/contracts/AlgebraFactory.sol 43: require(msg.sender == owner); 60: require(tokenA != tokenB); 62: require(token0 != address(0)); 63: require(poolByPair[token0][token1] == address(0)); 78: require(owner != _owner); 85: require(farmingAddress != _farmingAddress); 92: require(vaultAddress != _vaultAddress);
File: src/core/contracts/AlgebraPoolDeployer.sol 22: require(msg.sender == factory); 27: require(msg.sender == owner); 37: require(_factory != address(0)); 38: require(factory == address(0));
File: src/core/contracts/AlgebraPool.sol 55: require(msg.sender == IAlgebraFactory(factory).owner()); 122: require(_lower.initialized); 134: require(_upper.initialized); 229: require((_blockTimestamp() - lastLiquidityAddTimestamp) >= _liquidityCooldown); 953: require((communityFee0 <= Constants.MAX_COMMUNITY_FEE) && (communityFee1 <= Constants.MAX_COMMUNITY_FEE)); 960: require(msg.sender == IAlgebraFactory(factory).farmingAddress()); 968: require(newLiquidityCooldown <= Constants.MAX_LIQUIDITY_COOLDOWN && liquidityCooldown != newLiquidityCooldown);
File: src/core/contracts/DataStorageOperator.sol 43: require(msg.sender == factory || msg.sender == IAlgebraFactory(factory).owner());
File: src/core/contracts/libraries/DataStorage.sol 369: require(!self[0].initialized);
File: src/core/contracts/libraries/PriceMovementMath.sol 52: require(price > 0); 53: require(liquidity > 0); 70: require((product = amount * price) / amount == price); // if the product overflows, we know the denominator underflows 71: require(liquidityShifted > product); // in addition, we must check that the denominator does not underflow 87: require(price > quotient);
File: src/core/contracts/libraries/TokenDeltaMath.sol 30: require(priceDelta < priceUpper); // forbids underflow and 0 priceLower 51: require(priceUpper >= priceLower);
2**<n> - 1 should be re-written as type(uint<n>).maxEarlier 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)
There is 1 instance of this issue:
File: src/core/contracts/DataStorageOperator.sol 138: if (volume >= 2**192) volumeShifted = (type(uint256).max) / (liquidity > 0 ? liquidity : 1);
constants should be defined rather than using magic numbersEven assembly can benefit from using readable constants instead of hex/numeric literals
There are 41 instances of this issue:
File: src/core/contracts/AlgebraPool.sol /// @audit 0xFFFFFF /// @audit 0xFFFFFF 410: key := or(shl(24, or(shl(24, owner), and(bottomTick, 0xFFFFFF))), and(topTick, 0xFFFFFF)) /// @audit 1e6 905: fee0 = FullMath.mulDivRoundingUp(amount0, _fee, 1e6); /// @audit 1e6 912: fee1 = FullMath.mulDivRoundingUp(amount1, _fee, 1e6);
File: src/core/contracts/DataStorageOperator.sol /// @audit 192 138: if (volume >= 2**192) volumeShifted = (type(uint256).max) / (liquidity > 0 ? liquidity : 1); /// @audit 64 139: else volumeShifted = (volume << 64) / (liquidity > 0 ? liquidity : 1); /// @audit 15 158: return AdaptiveFee.getFee(volatilityAverage / 15, volumePerLiqAverage, feeConfig);
File: src/core/contracts/libraries/AdaptiveFee.sol /// @audit 6 52: if (x >= 6 * uint256(g)) return alpha; // so x < 19 bits /// @audit 8 53: uint256 g8 = uint256(g)**8; // < 128 bits (8*16) /// @audit 6 59: if (x >= 6 * uint256(g)) return 0; // so x < 19 bits /// @audit 8 60: uint256 g8 = uint256(g)**8; // < 128 bits (8*16) /// @audit 6 92: res += (xLowestDegree * gHighestDegree) / 6; /// @audit 24 96: res += (xLowestDegree * gHighestDegree) / 24; /// @audit 120 100: res += (xLowestDegree * gHighestDegree) / 120; /// @audit 720 104: res += (xLowestDegree * gHighestDegree) / 720; /// @audit 5040 107: res += (xLowestDegree * g) / 5040 + (xLowestDegree * x) / (40320);
File: src/core/contracts/libraries/DataStorage.sol /// @audit 6 /// @audit 6 /// @audit 6 53: volatility = uint256((K**2 * sumOfSquares + 6 * B * K * sumOfSequence + 6 * dt * B**2) / (6 * dt**2)); /// @audit 128 80: last.secondsPerLiquidityCumulative += ((uint160(delta) << 128) / (liquidity > 0 ? liquidity : 1)); // just timedelta if liquidity == 0 /// @audit 57 348: uint256(endOfWindow.volumePerLiquidityCumulative - startOfWindow.volumePerLiquidityCumulative) >> 57 /// @audit 57 355: uint256(endOfWindow.volumePerLiquidityCumulative - _oldestVolumePerLiquidityCumulative) >> 57
File: src/core/contracts/libraries/PriceMovementMath.sol /// @audit 1e6 /// @audit 1e6 157: uint256 amountAvailableAfterFee = FullMath.mulDiv(uint256(amountAvailable), 1e6 - fee, 1e6); /// @audit 1e6 161: feeAmount = FullMath.mulDivRoundingUp(input, fee, 1e6 - fee); /// @audit 1e6 170: feeAmount = FullMath.mulDivRoundingUp(input, fee, 1e6 - fee); /// @audit 1e6 195: feeAmount = FullMath.mulDivRoundingUp(input, fee, 1e6 - fee);
File: src/core/contracts/libraries/TickTable.sol /// @audit 0xFF 21: bitNumber := and(tick, 0xFF) /// @audit 0x5555555555555555555555555555555555555555555555555555555555555555 32: singleBitPos := iszero(and(word, 0x5555555555555555555555555555555555555555555555555555555555555555)) /// @audit 0x00000000000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF 33: singleBitPos := or(singleBitPos, shl(7, iszero(and(word, 0x00000000000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF)))) /// @audit 0x0000000000000000FFFFFFFFFFFFFFFF0000000000000000FFFFFFFFFFFFFFFF 34: singleBitPos := or(singleBitPos, shl(6, iszero(and(word, 0x0000000000000000FFFFFFFFFFFFFFFF0000000000000000FFFFFFFFFFFFFFFF)))) /// @audit 0x00000000FFFFFFFF00000000FFFFFFFF00000000FFFFFFFF00000000FFFFFFFF 35: singleBitPos := or(singleBitPos, shl(5, iszero(and(word, 0x00000000FFFFFFFF00000000FFFFFFFF00000000FFFFFFFF00000000FFFFFFFF)))) /// @audit 0x0000FFFF0000FFFF0000FFFF0000FFFF0000FFFF0000FFFF0000FFFF0000FFFF 36: singleBitPos := or(singleBitPos, shl(4, iszero(and(word, 0x0000FFFF0000FFFF0000FFFF0000FFFF0000FFFF0000FFFF0000FFFF0000FFFF)))) /// @audit 0x00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF 37: singleBitPos := or(singleBitPos, shl(3, iszero(and(word, 0x00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF)))) /// @audit 0x0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F 38: singleBitPos := or(singleBitPos, shl(2, iszero(and(word, 0x0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F)))) /// @audit 0x3333333333333333333333333333333333333333333333333333333333333333 39: singleBitPos := or(singleBitPos, shl(1, iszero(and(word, 0x3333333333333333333333333333333333333333333333333333333333333333)))) /// @audit 0xFF 86: bitNumber := and(tick, 0xFF) /// @audit 255 89: uint256 _row = self[rowNumber] << (255 - bitNumber); // all the 1s at or to the right of the current bitNumber /// @audit 255 92: tick -= int24(255 - getMostSignificantBit(_row)); /// @audit 0xFF 104: bitNumber := and(tick, 0xFF) /// @audit 255 115: tick += int24(255 - bitNumber);
1e6) rather than decimal literals (e.g. 1000000), for readabilityThere is 1 instance of this issue:
File: src/core/contracts/DataStorageOperator.sol 16: uint128 constant MAX_VOLUME_PER_LIQUIDITY = 100000 << 64; // maximum meaningful ratio of volume to liquidity
There are 2 instances of this issue:
File: src/core/contracts/libraries/Constants.sol 6: uint256 internal constant Q96 = 0x1000000000000000000000000; 7: uint256 internal constant Q128 = 0x100000000000000000000000000000000;
Use a solidity version of at least 0.8.13 to get the ability to use using for with a list of free functions
There are 5 instances of this issue:
File: src/core/contracts/AlgebraPool.sol 2: pragma solidity =0.7.6;
File: src/core/contracts/DataStorageOperator.sol 2: pragma solidity =0.7.6;
File: src/core/contracts/libraries/PriceMovementMath.sol 2: pragma solidity =0.7.6;
File: src/core/contracts/libraries/TickManager.sol 2: pragma solidity =0.7.6;
File: src/core/contracts/libraries/TokenDeltaMath.sol 2: pragma solidity =0.7.6;
Use a solidity version of at least 0.8.4 to get bytes.concat() instead of abi.encodePacked(<bytes>,<bytes>)
Use a solidity version of at least 0.8.12 to get string.concat() instead of abi.encodePacked(<str>,<str>)
There is 1 instance of this issue:
File: src/core/contracts/AlgebraFactory.sol 2: pragma solidity =0.7.6;
Usually lines in source code are limited to 80 characters. Today's screens are much larger so it's reasonable to stretch this in some cases. Since the files will most likely reside in GitHub, and GitHub starts using a scroll bar in all cases when the length is over 164 characters, the lines below should be split when they reach that length
There is 1 instance of this issue:
File: src/core/contracts/libraries/AdaptiveFee.sol 38: return uint16(config.baseFee + sigmoid(volumePerLiquidity, config.volumeGamma, uint16(sumOfSigmoids), config.volumeBeta)); // safe since alpha1 + alpha2 + baseFee _must_ be <= type(uint16).max
constant/immutable variablesIf the variable needs to be different based on which class it comes from, a view/pure function should be used instead (e.g. like this).
There are 2 instances of this issue:
File: src/core/contracts/libraries/DataStorage.sol 49: int256 K = (tick1 - tick0) - (avgTick1 - avgTick0); // (k - p)*dt 50: int256 B = (tick0 - avgTick0) * dt; // (b - q)*dt
There are 4 instances of this issue:
File: src/core/contracts/AlgebraPoolDeployer.sol
File: src/core/contracts/base/PoolImmutables.sol
File: src/core/contracts/DataStorageOperator.sol
File: src/core/contracts/libraries/Constants.sol
There are 7 instances of this issue:
File: src/core/contracts/AlgebraPool.sol /// @audit Missing: '@param owner' /// @audit Missing: '@param bottomTick' /// @audit Missing: '@param topTick' /// @audit Missing: '@param liquidityDelta' 268 /** 269 * @dev Updates position's ticks and its fees 270 * @return position The Position object to operate with 271 * @return amount0 The amount of token0 the caller needs to send, negative if the pool needs to send it 272 * @return amount1 The amount of token1 the caller needs to send, negative if the pool needs to send it 273 */ 274 function _updatePositionTicksAndFees( 275 address owner, 276 int24 bottomTick, 277 int24 topTick, 278 int128 liquidityDelta 279 ) 280 private 281 returns ( 282 Position storage position, 283 int256 amount0, 284: int256 amount1
File: src/core/contracts/libraries/PriceMovementMath.sol /// @audit Missing: '@param zeroToOne' 125 /// @notice Computes the result of swapping some amount in, or amount out, given the parameters of the swap 126 /// @dev The fee, plus the amount in, will never exceed the amount remaining if the swap's `amountSpecified` is positive 127 /// @param currentPrice The current Q64.96 sqrt price of the pool 128 /// @param targetPrice The Q64.96 sqrt price that cannot be exceeded, from which the direction of the swap is inferred 129 /// @param liquidity The usable liquidity 130 /// @param amountAvailable How much input or output amount is remaining to be swapped in/out 131 /// @param fee The fee taken from the input amount, expressed in hundredths of a bip 132 /// @return resultPrice The Q64.96 sqrt price after swapping the amount in/out, not to exceed the price target 133 /// @return input The amount to be swapped in, of either token0 or token1, based on the direction of the swap 134 /// @return output The amount to be received, of either token0 or token1, based on the direction of the swap 135 /// @return feeAmount The amount of input that will be taken as a fee 136 function movePriceTowardsTarget( 137 bool zeroToOne, 138 uint160 currentPrice, 139 uint160 targetPrice, 140 uint128 liquidity, 141 int256 amountAvailable, 142 uint16 fee 143 ) 144 internal 145 pure 146 returns ( 147 uint160 resultPrice, 148 uint256 input, 149 uint256 output, 150: uint256 feeAmount
File: src/core/contracts/libraries/TickTable.sol /// @audit Missing: '@return' 28 /// @dev it is assumed that word contains exactly one 1-bit, otherwise the result will be incorrect 29 /// @param word The word containing only one 1-bit 30: function getSingleSignificantBit(uint256 word) internal pure returns (uint8 singleBitPos) { /// @audit Missing: '@return' 44 /// @dev it is assumed that before the call, a check will be made that the argument (word) is not equal to zero 45 /// @param word The word containing at least one 1-bit 46: function getMostSignificantBit(uint256 word) internal pure returns (uint8 mostBitPos) {
Consider changing the variable to be an unnamed one
There are 33 instances of this issue:
File: src/core/contracts/AlgebraPool.sol /// @audit initialized /// @audit blockTimestamp /// @audit tickCumulative /// @audit secondsPerLiquidityCumulative /// @audit volatilityCumulative /// @audit averageTick /// @audit volumePerLiquidityCumulative 79 function timepoints(uint256 index) 80 external 81 view 82 override 83 returns ( 84 bool initialized, 85 uint32 blockTimestamp, 86 int56 tickCumulative, 87 uint160 secondsPerLiquidityCumulative, 88 uint88 volatilityCumulative, 89 int24 averageTick, 90: uint144 volumePerLiquidityCumulative /// @audit innerTickCumulative /// @audit innerSecondsSpentPerLiquidity /// @audit innerSecondsSpent 103 function getInnerCumulatives(int24 bottomTick, int24 topTick) 104 external 105 view 106 override 107 onlyValidTicks(bottomTick, topTick) 108 returns ( 109 int56 innerTickCumulative, 110 uint160 innerSecondsSpentPerLiquidity, 111: uint32 innerSecondsSpent /// @audit tickCumulatives /// @audit secondsPerLiquidityCumulatives /// @audit volatilityCumulatives /// @audit volumePerAvgLiquiditys 171 function getTimepoints(uint32[] calldata secondsAgos) 172 external 173 view 174 override 175 returns ( 176 int56[] memory tickCumulatives, 177 uint160[] memory secondsPerLiquidityCumulatives, 178 uint112[] memory volatilityCumulatives, 179: uint256[] memory volumePerAvgLiquiditys /// @audit newTimepointIndex 551 function _writeTimepoint( 552 uint16 timepointIndex, 553 uint32 blockTimestamp, 554 int24 tick, 555 uint128 liquidity, 556 uint128 volumePerLiquidityInBlock 557: ) private returns (uint16 newTimepointIndex) { /// @audit tickCumulative /// @audit secondsPerLiquidityCumulative /// @audit volatilityCumulative /// @audit volumePerAvgLiquidity 561 function _getSingleTimepoint( 562 uint32 blockTimestamp, 563 uint32 secondsAgo, 564 int24 startTick, 565 uint16 timepointIndex, 566 uint128 liquidityStart 567 ) 568 private 569 view 570 returns ( 571 int56 tickCumulative, 572 uint160 secondsPerLiquidityCumulative, 573 uint112 volatilityCumulative, 574: uint256 volumePerAvgLiquidity
File: src/core/contracts/DataStorageOperator.sol /// @audit tickCumulatives /// @audit secondsPerLiquidityCumulatives /// @audit volatilityCumulatives /// @audit volumePerAvgLiquiditys 88 function getTimepoints( 89 uint32 time, 90 uint32[] memory secondsAgos, 91 int24 tick, 92 uint16 index, 93 uint128 liquidity 94 ) 95 external 96 view 97 override 98 onlyPool 99 returns ( 100 int56[] memory tickCumulatives, 101 uint160[] memory secondsPerLiquidityCumulatives, 102 uint112[] memory volatilityCumulatives, 103: uint256[] memory volumePerAvgLiquiditys /// @audit TWVolatilityAverage /// @audit TWVolumePerLiqAverage 110 function getAverages( 111 uint32 time, 112 int24 tick, 113 uint16 index, 114 uint128 liquidity 115: ) external view override onlyPool returns (uint112 TWVolatilityAverage, uint256 TWVolumePerLiqAverage) { /// @audit indexUpdated 120 function write( 121 uint16 index, 122 uint32 blockTimestamp, 123 int24 tick, 124 uint128 liquidity, 125 uint128 volumePerLiquidity 126: ) external override onlyPool returns (uint16 indexUpdated) { /// @audit fee 150 function getFee( 151 uint32 _time, 152 int24 _tick, 153 uint16 _index, 154 uint128 _liquidity 155: ) external view override onlyPool returns (uint16 fee) {
File: src/core/contracts/libraries/AdaptiveFee.sol /// @audit fee 25 function getFee( 26 uint88 volatility, 27 uint256 volumePerLiquidity, 28 Configuration memory config 29: ) internal pure returns (uint16 fee) {
File: src/core/contracts/libraries/DataStorage.sol /// @audit targetTimepoint 205 function getSingleTimepoint( 206 Timepoint[UINT16_MODULO] storage self, 207 uint32 time, 208 uint32 secondsAgo, 209 int24 tick, 210 uint16 index, 211 uint16 oldestIndex, 212 uint128 liquidity 213: ) internal view returns (Timepoint memory targetTimepoint) {
File: src/core/contracts/libraries/PriceMovementMath.sol /// @audit resultPrice 20 function getNewPriceAfterInput( 21 uint160 price, 22 uint128 liquidity, 23 uint256 input, 24 bool zeroToOne 25: ) internal pure returns (uint160 resultPrice) { /// @audit resultPrice 36 function getNewPriceAfterOutput( 37 uint160 price, 38 uint128 liquidity, 39 uint256 output, 40 bool zeroToOne 41: ) internal pure returns (uint160 resultPrice) {
File: src/core/contracts/libraries/TickManager.sol /// @audit liquidityDelta 129 function cross( 130 mapping(int24 => Tick) storage self, 131 int24 tick, 132 uint256 totalFeeGrowth0Token, 133 uint256 totalFeeGrowth1Token, 134 uint160 secondsPerLiquidityCumulative, 135 int56 tickCumulative, 136 uint32 time 137: ) internal returns (int128 liquidityDelta) {
File: src/core/contracts/libraries/TickTable.sol /// @audit mostBitPos 46: function getMostSignificantBit(uint256 word) internal pure returns (uint8 mostBitPos) {
š Selected for report: IllIllI
Also found by: 0x1f8b, 0x5rings, 0xNazgul, 0xRoxas, 0xSmartContract, 0xbepresent, 0xmatt, Aeros, Amithuddar, Awesome, Aymen0909, B2, Bnke0x0, ChristianKuri, CodingNameKiki, Deivitto, Diraco, Fitraldys, HardlyCodeMan, JC, Mukund, Noah3o6, Olivierdem, RaymondFam, ReyAdmirado, RockingMiles, Rolezn, Ruhum, Saintcode_, Shinchan, SnowMan, TomJ, Tomio, Tomo, V_B, Waze, __141345__, ajtra, asutorufos, aysha, beardofginger, bobirichman, brgltd, bulej93, c3phas, ch0bu, cryptonue, defsec, delfin454000, dharma09, durianSausage, emrekocak, erictee, fatherOfBlocks, francoHacker, gianganhnguyen, gogo, imare, kaden, karanctf, ladboy233, lukris02, m_Rassska, martin, medikko, mics, natzuu, oyc_109, peiw, rbserver, ret2basic, rotcivegaf, saian, shark, slowmoses, tnevler, trustindistrust, zeesaw, zishansami
235.0963 USDC - $235.10
| Issue | Instances | Total Gas Saved | |
|---|---|---|---|
| [Gā01] | State variables only set in the constructor should be declared immutable | 2 | 4194 |
| [Gā02] | Using calldata instead of memory for read-only arguments in external functions saves gas | 1 | 120 |
| [Gā03] | Using storage instead of memory for structs/arrays saves gas | 1 | 4200 |
| [Gā04] | Avoid contract existence checks by using low level calls | 24 | 2400 |
| [Gā05] | State variables should be cached in stack variables rather than re-reading them from storage | 7 | 679 |
| [Gā06] | internal functions only called once can be inlined to save gas | 2 | 40 |
| [Gā07] | <array>.length should not be looked up in every loop of a for-loop | 1 | 3 |
| [Gā08] | Use a more recent version of solidity | 13 | - |
| [Gā09] | Using > 0 costs more gas than != 0 when used on a uint in a require() statement | 6 | 36 |
| [Gā10] | >= costs less gas than > | 2 | 6 |
| [Gā11] | ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too) | 1 | 5 |
| [Gā12] | Splitting require() statements that use && saves gas | 6 | 18 |
| [Gā13] | Using private rather than public for constants, saves gas | 1 | - |
| [Gā14] | Division by two should use bit shifting | 1 | 20 |
| [Gā15] | Use custom errors rather than revert()/require() strings to save gas | 32 | - |
| [Gā16] | Functions guaranteed to revert when called by normal users can be marked payable | 17 | 357 |
Total: 117 instances over 16 issues with 12078 gas saved
Gas totals use lower bounds of ranges and count two iterations of each for-loop. All values above are runtime, not deployment, values; deployment values are listed in the individual issue descriptions
immutableAvoids a Gsset (20000 gas) in the constructor, and replaces the first access in each transaction (Gcoldsload - 2100 gas) and each access thereafter (Gwarmacces - 100 gas) with a PUSH32 (3 gas).
While strings are not value types, and therefore cannot be immutable/constant if not hard-coded outside of the constructor, the same behavior can be achieved by making the current contract abstract with virtual functions for the string accessors, and having a child contract override the functions with the hard-coded implementation-specific values.
There are 2 instances of this issue:
File: src/core/contracts/AlgebraPoolDeployer.sol /// @audit owner (access) 27: require(msg.sender == owner); /// @audit owner (constructor) 32: owner = msg.sender;
calldata instead of memory for read-only arguments in external functions saves gasWhen a function with a memory array is called externally, the abi.decode() step has to use a for-loop to copy each index of the calldata to the memory index. Each iteration of this for-loop costs at least 60 gas (i.e. 60 * <mem_array>.length). Using calldata directly, obliviates the need for such a loop in the contract code and runtime execution. Note that even if an interface defines a function as having memory arguments, it's still valid for implementation contracs to use calldata arguments instead.
If the array is passed to an internal function which passes the array to another internal function where the array is modified and therefore memory is used in the external call, it's still more gass-efficient to use calldata when the external function uses modifiers, since the modifiers may prevent the internal functions from being called. Structs have the same overhead as an array of length one
Note that I've also flagged instances where the function is public but can be marked as external since it's not called by the contract, and cases where a constructor is involved
There is 1 instance of this issue:
File: src/core/contracts/DataStorageOperator.sol /// @audit secondsAgos 88 function getTimepoints( 89 uint32 time, 90 uint32[] memory secondsAgos, 91 int24 tick, 92 uint16 index, 93 uint128 liquidity 94 ) 95 external 96 view 97 override 98 onlyPool 99 returns ( 100 int56[] memory tickCumulatives, 101 uint160[] memory secondsPerLiquidityCumulatives, 102 uint112[] memory volatilityCumulatives, 103: uint256[] memory volumePerAvgLiquiditys
storage instead of memory for structs/arrays saves gasWhen fetching data from a storage location, assigning the data to a memory variable causes all fields of the struct/array to be read from storage, which incurs a Gcoldsload (2100 gas) for each field of the struct/array. If the fields are read from the new memory variable, they incur an additional MLOAD rather than a cheap stack read. Instead of declearing the variable with the memory keyword, declaring the variable with the storage keyword and caching any fields that need to be re-read in stack variables, will be much cheaper, only incuring the Gcoldsload for the fields actually read. The only time it makes sense to read the whole struct/array into a memory variable, is if the full struct/array is being returned by the function, is being passed to a function that requires memory, or if the array/struct is being read from another memory array/struct
There is 1 instance of this issue:
File: src/core/contracts/libraries/DataStorage.sol 397: Timepoint memory last = _last;
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
There are 24 instances of this issue:
File: src/core/contracts/AlgebraFactory.sol /// @audit deploy() 69: pool = IAlgebraPoolDeployer(poolDeployer).deploy(address(dataStorage), address(this), token0, token1);
File: src/core/contracts/AlgebraPool.sol /// @audit owner() 55: require(msg.sender == IAlgebraFactory(factory).owner()); /// @audit balanceOf() 71: return IERC20Minimal(token0).balanceOf(address(this)); /// @audit balanceOf() 75: return IERC20Minimal(token1).balanceOf(address(this)); /// @audit timepoints() 93: return IDataStorageOperator(dataStorageOperator).timepoints(index); /// @audit getTimepoints() 183: IDataStorageOperator(dataStorageOperator).getTimepoints( /// @audit algebraMintCallback() 453: IAlgebraMintCallback(msg.sender).algebraMintCallback(amount0, amount1, data); /// @audit getFee() 542: newFee = IDataStorageOperator(dataStorageOperator).getFee(_time, _tick, _index, _liquidity); /// @audit vaultAddress() 547: address vault = IAlgebraFactory(factory).vaultAddress(); /// @audit write() 558: return IDataStorageOperator(dataStorageOperator).write(timepointIndex, blockTimestamp, tick, liquidity, volumePerLiquidityInBlock); /// @audit getSingleTimepoint() 577: return IDataStorageOperator(dataStorageOperator).getSingleTimepoint(blockTimestamp, secondsAgo, startTick, timepointIndex, liquidityStart); /// @audit algebraSwapCallback() 585: IAlgebraSwapCallback(msg.sender).algebraSwapCallback(amount0, amount1, data); /// @audit increaseCumulative() 753: IAlgebraVirtualPool.Status _status = IAlgebraVirtualPool(activeIncentive).increaseCumulative(blockTimestamp); /// @audit cross() 833: IAlgebraVirtualPool(activeIncentive).cross(step.nextTick, zeroToOne); /// @audit calculateVolumePerLiquidity() 880: cache.volumePerLiquidityInBlock + IDataStorageOperator(dataStorageOperator).calculateVolumePerLiquidity(currentLiquidity, amount0, amount1) /// @audit algebraFlashCallback() 916: IAlgebraFlashCallback(msg.sender).algebraFlashCallback(fee0, fee1, data); /// @audit vaultAddress() 918: address vault = IAlgebraFactory(factory).vaultAddress(); /// @audit farmingAddress() 960: require(msg.sender == IAlgebraFactory(factory).farmingAddress());
File: src/core/contracts/base/PoolImmutables.sol /// @audit parameters() 30: (dataStorageOperator, factory, token0, token1) = IAlgebraPoolDeployer(deployer).parameters();
File: src/core/contracts/DataStorageOperator.sol /// @audit owner() 43: require(msg.sender == factory || msg.sender == IAlgebraFactory(factory).owner());
File: src/core/contracts/libraries/TokenDeltaMath.sol /// @audit toInt256() 67: ? getToken0Delta(priceLower, priceUpper, uint128(liquidity), true).toInt256() /// @audit toInt256() 68: : -getToken0Delta(priceLower, priceUpper, uint128(-liquidity), false).toInt256(); /// @audit toInt256() 82: ? getToken1Delta(priceLower, priceUpper, uint128(liquidity), true).toInt256() /// @audit toInt256() 83: : -getToken1Delta(priceLower, priceUpper, uint128(-liquidity), false).toInt256();
The instances below point to the second+ access of a state variable within a function. Caching of a state variable replaces each Gwarmaccess (100 gas) with a much cheaper stack read. Other less obvious fixes/optimizations include having local memory caches of state variable structs, or having local caches of state variable contracts/addresses.
There are 7 instances of this issue:
File: src/core/contracts/AlgebraPool.sol /// @audit totalFeeGrowth0Token on line 740 /// @audit totalFeeGrowth1Token on line 744 829: cache.totalFeeGrowthB = zeroToOne ? totalFeeGrowth1Token : totalFeeGrowth0Token; /// @audit liquidity on line 297 354: uint128 liquidityBefore = liquidity; /// @audit liquidity on line 736 /// @audit volumePerLiquidityInBlock on line 736 878: (liquidity, volumePerLiquidityInBlock) = ( /// @audit activeIncentive on line 752 753: IAlgebraVirtualPool.Status _status = IAlgebraVirtualPool(activeIncentive).increaseCumulative(blockTimestamp); /// @audit activeIncentive on line 755 833: IAlgebraVirtualPool(activeIncentive).cross(step.nextTick, zeroToOne);
internal functions only called once can be inlined to save gasNot inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.
There are 2 instances of this issue:
File: src/core/contracts/AlgebraFactory.sol 122: function computeAddress(address token0, address token1) internal view returns (address pool) {
File: src/core/contracts/AlgebraPool.sol 215 function _recalculatePosition( 216 Position storage _position, 217 int128 liquidityDelta, 218 uint256 innerFeeGrowth0Token, 219: uint256 innerFeeGrowth1Token
<array>.length should not be looked up in every loop of a for-loopThe overheads outlined below are PER LOOP, excluding the first loop
MLOAD (3 gas)CALLDATALOAD (3 gas)Caching the length changes each of these to a DUP<N> (3 gas), and gets rid of the extra DUP<N> needed to store the stack offset
There is 1 instance of this issue:
File: src/core/contracts/libraries/DataStorage.sol 307: for (uint256 i = 0; i < secondsAgos.length; i++) {
Use a solidity version of at least 0.8.0 to get overflow protection without SafeMath
Use a solidity version of at least 0.8.2 to get simple compiler automatic inlining
Use a solidity version of at least 0.8.3 to get better struct packing and cheaper multiple storage reads
Use a solidity version of at least 0.8.4 to get custom errors, which are cheaper at deployment than revert()/require() strings
Use a solidity version of at least 0.8.10 to have external calls skip contract existence checks if the external call has a return value
There are 13 instances of this issue:
File: src/core/contracts/AlgebraFactory.sol 2: pragma solidity =0.7.6;
File: src/core/contracts/AlgebraPoolDeployer.sol 2: pragma solidity =0.7.6;
File: src/core/contracts/AlgebraPool.sol 2: pragma solidity =0.7.6;
File: src/core/contracts/base/PoolImmutables.sol 2: pragma solidity =0.7.6;
File: src/core/contracts/base/PoolState.sol 2: pragma solidity =0.7.6;
File: src/core/contracts/DataStorageOperator.sol 2: pragma solidity =0.7.6;
File: src/core/contracts/libraries/AdaptiveFee.sol 2: pragma solidity =0.7.6;
File: src/core/contracts/libraries/Constants.sol 2: pragma solidity =0.7.6;
File: src/core/contracts/libraries/DataStorage.sol 2: pragma solidity =0.7.6;
File: src/core/contracts/libraries/PriceMovementMath.sol 2: pragma solidity =0.7.6;
File: src/core/contracts/libraries/TickManager.sol 2: pragma solidity =0.7.6;
File: src/core/contracts/libraries/TickTable.sol 2: pragma solidity =0.7.6;
File: src/core/contracts/libraries/TokenDeltaMath.sol 2: pragma solidity =0.7.6;
> 0 costs more gas than != 0 when used on a uint in a require() statementThis change saves 6 gas per instance. The optimization works until solidity version 0.8.13 where there is a regression in gas costs.
There are 6 instances of this issue:
File: src/core/contracts/AlgebraPool.sol 224: require(currentLiquidity > 0, 'NP'); // Do not recalculate the empty ranges 434: require(liquidityDesired > 0, 'IL'); 469: require(liquidityActual > 0, 'IIL2'); 898: require(_liquidity > 0, 'L');
File: src/core/contracts/libraries/PriceMovementMath.sol 52: require(price > 0); 53: require(liquidity > 0);
>= costs less gas than >The compiler uses opcodes GT and ISZERO for solidity code that uses >, but only requires LT for >=, which saves 3 gas
There are 2 instances of this issue:
File: src/core/contracts/AlgebraPool.sol 498: amount0 = amount0Requested > positionFees0 ? positionFees0 : amount0Requested; 499: amount1 = amount1Requested > positionFees1 ? positionFees1 : amount1Requested;
++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too)Saves 5 gas per loop
There is 1 instance of this issue:
File: src/core/contracts/libraries/DataStorage.sol 307: for (uint256 i = 0; i < secondsAgos.length; i++) {
require() statements that use && saves gasSee this issue which describes the fact that there is a larger deployment gas cost, but with enough runtime calls, the change ends up being cheaper by 3 gas
There are 6 instances of this issue:
File: src/core/contracts/AlgebraFactory.sol 110: require(gamma1 != 0 && gamma2 != 0 && volumeGamma != 0, 'Gammas must be > 0');
File: src/core/contracts/AlgebraPool.sol 739: require(limitSqrtPrice < currentPrice && limitSqrtPrice > TickMath.MIN_SQRT_RATIO, 'SPL'); 743: require(limitSqrtPrice > currentPrice && limitSqrtPrice < TickMath.MAX_SQRT_RATIO, 'SPL'); 953: require((communityFee0 <= Constants.MAX_COMMUNITY_FEE) && (communityFee1 <= Constants.MAX_COMMUNITY_FEE)); 968: require(newLiquidityCooldown <= Constants.MAX_LIQUIDITY_COOLDOWN && liquidityCooldown != newLiquidityCooldown);
File: src/core/contracts/DataStorageOperator.sol 46: require(_feeConfig.gamma1 != 0 && _feeConfig.gamma2 != 0 && _feeConfig.volumeGamma != 0, 'Gammas must be > 0');
private rather than public for constants, saves gasIf 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
There is 1 instance of this issue:
File: src/core/contracts/libraries/DataStorage.sol 12: uint32 public constant WINDOW = 1 days;
<x> / 2 is the same as <x> >> 1. While the compiler uses the SHR opcode to accomplish both, the version that uses division incurs an overhead of 20 gas due to JUMPs to and from a compiler utility function that introduces checks which can be avoided by using unchecked {} around the division by two
There is 1 instance of this issue:
File: src/core/contracts/libraries/AdaptiveFee.sol 88: res += (xLowestDegree * gHighestDegree) / 2;
revert()/require() strings to save gasCustom errors are available from solidity version 0.8.4. Custom errors save ~50 gas each time they're hit by avoiding having to allocate and store the revert string. Not defining the strings also save deployment gas
There are 32 instances of this issue:
File: src/core/contracts/AlgebraFactory.sol 109: require(uint256(alpha1) + uint256(alpha2) + uint256(baseFee) <= type(uint16).max, 'Max fee exceeded'); 110: require(gamma1 != 0 && gamma2 != 0 && volumeGamma != 0, 'Gammas must be > 0');
File: src/core/contracts/AlgebraPool.sol 60: require(topTick < TickMath.MAX_TICK + 1, 'TUM'); 61: require(topTick > bottomTick, 'TLU'); 62: require(bottomTick > TickMath.MIN_TICK - 1, 'TLM'); 194: require(globalState.price == 0, 'AI'); 224: require(currentLiquidity > 0, 'NP'); // Do not recalculate the empty ranges 434: require(liquidityDesired > 0, 'IL'); 454: if (amount0 > 0) require((receivedAmount0 = balanceToken0() - receivedAmount0) > 0, 'IIAM'); 455: if (amount1 > 0) require((receivedAmount1 = balanceToken1() - receivedAmount1) > 0, 'IIAM'); 469: require(liquidityActual > 0, 'IIL2'); 474: require((amount0 = uint256(amount0Int)) <= receivedAmount0, 'IIAM2'); 475: require((amount1 = uint256(amount1Int)) <= receivedAmount1, 'IIAM2'); 608: require(balance0Before.add(uint256(amount0)) <= balanceToken0(), 'IIA'); 614: require(balance1Before.add(uint256(amount1)) <= balanceToken1(), 'IIA'); 636: require(globalState.unlocked, 'LOK'); 641: require((amountRequired = int256(balanceToken0().sub(balance0Before))) > 0, 'IIA'); 645: require((amountRequired = int256(balanceToken1().sub(balance1Before))) > 0, 'IIA'); 731: require(unlocked, 'LOK'); 733: require(amountRequired != 0, 'AS'); 739: require(limitSqrtPrice < currentPrice && limitSqrtPrice > TickMath.MIN_SQRT_RATIO, 'SPL'); 743: require(limitSqrtPrice > currentPrice && limitSqrtPrice < TickMath.MAX_SQRT_RATIO, 'SPL'); 898: require(_liquidity > 0, 'L'); 921: require(balance0Before.add(fee0) <= paid0, 'F0'); 935: require(balance1Before.add(fee1) <= paid1, 'F1');
File: src/core/contracts/base/PoolState.sol 41: require(globalState.unlocked, 'LOK');
File: src/core/contracts/DataStorageOperator.sol 27: require(msg.sender == pool, 'only pool can call this'); 45: require(uint256(_feeConfig.alpha1) + uint256(_feeConfig.alpha2) + uint256(_feeConfig.baseFee) <= type(uint16).max, 'Max fee exceeded'); 46: require(_feeConfig.gamma1 != 0 && _feeConfig.gamma2 != 0 && _feeConfig.volumeGamma != 0, 'Gammas must be > 0');
File: src/core/contracts/libraries/DataStorage.sol 238: require(lteConsideringOverflow(self[oldestIndex].blockTimestamp, target, time), 'OLD');
File: src/core/contracts/libraries/TickManager.sol 96: require(liquidityTotalAfter < Constants.MAX_LIQUIDITY_PER_TICK + 1, 'LO');
File: src/core/contracts/libraries/TickTable.sol 15: require(tick % Constants.TICK_SPACING == 0, 'tick is not spaced'); // ensure that the tick is spaced
payableIf a function modifier such as onlyOwner 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
There are 17 instances of this issue:
File: src/core/contracts/AlgebraFactory.sol 77: function setOwner(address _owner) external override onlyOwner { 84: function setFarmingAddress(address _farmingAddress) external override onlyOwner { 91: function setVaultAddress(address _vaultAddress) external override onlyOwner { 98 function setBaseFeeConfiguration( 99 uint16 alpha1, 100 uint16 alpha2, 101 uint32 beta1, 102 uint32 beta2, 103 uint16 gamma1, 104 uint16 gamma2, 105 uint32 volumeBeta, 106 uint16 volumeGamma, 107 uint16 baseFee 108: ) external override onlyOwner {
File: src/core/contracts/AlgebraPoolDeployer.sol 36: function setFactory(address _factory) external override onlyOwner { 44 function deploy( 45 address dataStorage, 46 address _factory, 47 address token0, 48 address token1 49: ) external override onlyFactory returns (address pool) {
File: src/core/contracts/AlgebraPool.sol 103 function getInnerCumulatives(int24 bottomTick, int24 topTick) 104 external 105 view 106 override 107 onlyValidTicks(bottomTick, topTick) 108 returns ( 109 int56 innerTickCumulative, 110 uint160 innerSecondsSpentPerLiquidity, 111: uint32 innerSecondsSpent 416 function mint( 417 address sender, 418 address recipient, 419 int24 bottomTick, 420 int24 topTick, 421 uint128 liquidityDesired, 422 bytes calldata data 423 ) 424 external 425 override 426 lock 427 onlyValidTicks(bottomTick, topTick) 428 returns ( 429 uint256 amount0, 430 uint256 amount1, 431: uint128 liquidityActual 513 function burn( 514 int24 bottomTick, 515 int24 topTick, 516 uint128 amount 517: ) external override lock onlyValidTicks(bottomTick, topTick) returns (uint256 amount0, uint256 amount1) { 952: function setCommunityFee(uint8 communityFee0, uint8 communityFee1) external override lock onlyFactoryOwner { 967: function setLiquidityCooldown(uint32 newLiquidityCooldown) external override onlyFactoryOwner {
File: src/core/contracts/DataStorageOperator.sol 37: function initialize(uint32 time, int24 tick) external override onlyPool { 53 function getSingleTimepoint( 54 uint32 time, 55 uint32 secondsAgo, 56 int24 tick, 57 uint16 index, 58 uint128 liquidity 59 ) 60 external 61 view 62 override 63 onlyPool 64 returns ( 65 int56 tickCumulative, 66 uint160 secondsPerLiquidityCumulative, 67 uint112 volatilityCumulative, 68: uint256 volumePerAvgLiquidity 88 function getTimepoints( 89 uint32 time, 90 uint32[] memory secondsAgos, 91 int24 tick, 92 uint16 index, 93 uint128 liquidity 94 ) 95 external 96 view 97 override 98 onlyPool 99 returns ( 100 int56[] memory tickCumulatives, 101 uint160[] memory secondsPerLiquidityCumulatives, 102 uint112[] memory volatilityCumulatives, 103: uint256[] memory volumePerAvgLiquiditys 110 function getAverages( 111 uint32 time, 112 int24 tick, 113 uint16 index, 114 uint128 liquidity 115: ) external view override onlyPool returns (uint112 TWVolatilityAverage, uint256 TWVolumePerLiqAverage) { 120 function write( 121 uint16 index, 122 uint32 blockTimestamp, 123 int24 tick, 124 uint128 liquidity, 125 uint128 volumePerLiquidity 126: ) external override onlyPool returns (uint16 indexUpdated) { 150 function getFee( 151 uint32 _time, 152 int24 _tick, 153 uint16 _index, 154 uint128 _liquidity 155: ) external view override onlyPool returns (uint16 fee) {