Omniscia Kyo Finance Audit

UniswapV3Pool Code Style Findings

UniswapV3Pool Code Style Findings

UVP-01C: Ineffectual Usage of Safe Arithmetics

TypeSeverityLocation
Language SpecificUniswapV3Pool.sol:
• I-1: L289, L290
• I-2: L690
• I-3: L698

Description:

The linked mathematical operations are guaranteed to be performed safely by surrounding conditionals evaluated in either require checks or if-else constructs.

Example:

contracts/univ3/UniswapV3Pool.sol
287if (pendingReward > lastPendingReward) {
288    // this should always be true, if gauge works correctly
289    rewardGrowthGlobalX128 += FullMath.mulDiv(pendingReward - lastPendingReward, FixedPoint128.Q128, liquidityStaked);
290    emit Reward(pendingReward - lastPendingReward);
291    lastPendingReward = pendingReward;
292}

Recommendation:

Given that safe arithmetics are toggled on by default in pragma versions of 0.8.X, we advise the linked statements to be wrapped in unchecked code blocks thereby optimizing their execution cost.

Alleviation (17c8d4e59f398021156f6f9657ff278aae0462ae):

The Kyo Finance team evaluated this exhibit but opted to acknowledge it in the current iteration of the codebase.

UVP-02C: Inefficient Conditional Trigger

Description:

The referenced conditional will trigger inefficiently whenever the liquidityStakedDelta is non-zero as a state mutation solely occurs when the _slot0.tick is also lower than the tickUpper in such a case.

Example:

contracts/univ3/UniswapV3Pool.sol
254if (params.liquidityDelta != 0 || params.liquidityStakedDelta != 0) {
255    if (_slot0.tick < params.tickLower) {
256        // current tick is below the passed range; liquidity can only become in range by crossing from left to
257        // right, when we'll need _more_ token0 (it's becoming more valuable) so user must provide it
258        amount0 = SqrtPriceMath.getAmount0Delta(TickMath.getSqrtRatioAtTick(params.tickLower), TickMath.getSqrtRatioAtTick(params.tickUpper), params.liquidityDelta);
259    } else if (_slot0.tick < params.tickUpper) {
260        // current tick is inside the passed range
261        uint128 liquidityBefore = liquidity; // SLOAD for gas optimization
262        uint128 liquidityStakedBefore = liquidityStaked; // SLOAD for gas optimization
263

264        if (params.liquidityDelta != 0) {
265            // write an oracle entry
266            (slot0.observationIndex, slot0.observationCardinality) = observations.write(_slot0.observationIndex, _slot0.tick, liquidityBefore, _slot0.observationCardinality, _slot0.observationCardinalityNext);
267        }
268

269        amount0 = SqrtPriceMath.getAmount0Delta(_slot0.sqrtPriceX96, TickMath.getSqrtRatioAtTick(params.tickUpper), params.liquidityDelta);
270        amount1 = SqrtPriceMath.getAmount1Delta(TickMath.getSqrtRatioAtTick(params.tickLower), _slot0.sqrtPriceX96, params.liquidityDelta);
271

272        liquidity = params.liquidityDelta < 0 ? liquidityBefore - uint128(-params.liquidityDelta) : liquidityBefore + uint128(params.liquidityDelta);
273

274        liquidityStaked = params.liquidityStakedDelta < 0 ? liquidityStakedBefore - uint128(-params.liquidityStakedDelta) : liquidityStakedBefore + uint128(params.liquidityStakedDelta);
275    } else {
276        // current tick is above the passed range; liquidity can only become in range by crossing from right to
277        // left, when we'll need _more_ token1 (it's becoming more valuable) so user must provide it
278        amount1 = SqrtPriceMath.getAmount1Delta(TickMath.getSqrtRatioAtTick(params.tickLower), TickMath.getSqrtRatioAtTick(params.tickUpper), params.liquidityDelta);
279    }
280}

Recommendation:

We advise the conditional and relevant logic to be relocated to a new if clause that checks whether the liquidityStakedDelta is non-zero and the _slot0.tick is less than the params.tickUpper, optimizing the code's gas cost significantly.

Alleviation (17c8d4e59f398021156f6f9657ff278aae0462ae):

The Kyo Finance team evaluated this exhibit but opted to acknowledge it in the current iteration of the codebase.

UVP-03C: Inefficient Evaluations of Both Input Tokens

Description:

The UniswapV3Pool::mint function will inefficiently evaluate both tokens of the AMM pair even if the user is supplying one-sided liquidity.

Example:

contracts/univ3/UniswapV3Pool.sol
339/// @inheritdoc IUniswapV3PoolActions
340function mint(address recipient, int24 tickLower, int24 tickUpper, uint128 amount, bytes calldata data) external override lock returns (uint256 amount0, uint256 amount1) {
341    require(amount > 0);
342    _updateRewardGrowth();
343    (Position.Info storage position, int256 amount0Int, int256 amount1Int) = _modifyPosition(ModifyPositionParams({owner: recipient, tickLower: tickLower, tickUpper: tickUpper, liquidityDelta: int256(uint256(amount)).toInt128(), liquidityStakedDelta: 0}));
344

345    amount0 = uint256(amount0Int);
346    amount1 = uint256(amount1Int);
347

348    uint256 balance0Before;
349    uint256 balance1Before;
350    balance0Before = balance0();
351    balance1Before = balance1();
352    IUniswapV3MintCallback(msg.sender).uniswapV3MintCallback(amount0, amount1, data);
353    require(balance0Before + amount0 <= balance0(), M0());
354    require(balance1Before + amount1 <= balance1(), M1());
355

356    position.log(msg.sender, tickLower, tickUpper);
357    emit Mint(msg.sender, recipient, tickLower, tickUpper, amount, amount0, amount1);
358}

Recommendation:

We advise the code to optimally confirm balance changes by evaluating the balance before and after checks solely when the relevant amount variable is positive, similarly to the original Uniswap V3 code.

Alleviation (17c8d4e59f398021156f6f9657ff278aae0462ae):

The Kyo Finance team evaluated this exhibit but opted to acknowledge it in the current iteration of the codebase.

UVP-04C: Inefficient Structure Pointer Type

Description:

The referenced local variable is declared as memory yet not all of the structure's data points are read.

Example:

contracts/univ3/UniswapV3Pool.sol
133function positions(bytes32 i) external view override returns (uint128 liquidity, uint256 feeGrowthInside0LastX128, uint256 feeGrowthInside1LastX128, uint128 tokensOwed0, uint128 tokensOwed1) {
134    Position.Info memory pos = _positions[i];
135    return (pos.liquidity, pos.feeGrowthInside0LastX128, pos.feeGrowthInside1LastX128, pos.tokensOwed0, pos.tokensOwed1);
136}

Recommendation:

We advise it to be updated to storage, optimizing the code's gas cost.

Alleviation (17c8d4e59f398021156f6f9657ff278aae0462ae):

The Kyo Finance team evaluated this exhibit but opted to acknowledge it in the current iteration of the codebase.

UVP-05C: Redundant Casting Sequence

Description:

The referenced casting sequence is redundant as the type(int128).max value can be directly cast to the uint128 data type that the amount is represented in.

Example:

contracts/univ3/UniswapV3Pool.sol
420require(amount <= uint256(int256(type(int128).max)));

Recommendation:

We advise this to be done so, optimizing the code's legibility.

Alleviation (17c8d4e59f398021156f6f9657ff278aae0462ae):

The Kyo Finance team evaluated this exhibit but opted to acknowledge it in the current iteration of the codebase.

UVP-06C: Repetitive Value Literal

Description:

The linked value literal is repeated across the codebase multiple times.

Example:

contracts/univ3/UniswapV3Pool.sol
39using Oracle for Oracle.Observation[65535];

Recommendation:

We advise it to be set to a constant variable instead, optimizing the legibility of the codebase.

In case the constant has already been declared, we advise it to be properly re-used across the code.

Alleviation (17c8d4e59f398021156f6f9657ff278aae0462ae):

The Kyo Finance team evaluated this exhibit but opted to acknowledge it in the current iteration of the codebase.

UVP-07C: Unconditional Increment of Fee Growth Global

TypeSeverityLocation
Gas OptimizationUniswapV3Pool.sol:
• I-1: L666
• I-2: L671

Description:

The relevant fee-growth-global variables of the contract are unconditionally incremented in the UniswapV3Pool::flash function yet they are conditionally incremented in the UniswapV3Pool::swap implementation.

Example:

contracts/univ3/UniswapV3Pool.sol
663if (paid0 > 0) {
664    (uint256 lpFee, uint256 protocolFee) = splitFees(paid0, _protocolFeeRate, _liquidity, liquidityStaked);
665    _bribe(protocolFee, 0);
666    feeGrowthGlobal0X128 += FullMath.mulDiv(lpFee, FixedPoint128.Q128, _liquidity - liquidityStaked);
667}

Recommendation:

We advise a uniform approach to be followed by the codebase, the latter of which we advise as it appears to be the most optimal one.

Alleviation (17c8d4e59f398021156f6f9657ff278aae0462ae):

The Kyo Finance team evaluated this exhibit but opted to acknowledge it in the current iteration of the codebase.

Prev

UniswapV3Factory.sol (UVF-C)

Next

Voter.sol (VRE-C)

On this page