Panoptic - K42's results

QA Report for Panoptic by K42

• Note: I made sure these are unique in relation to the 4naly3er-report.

CollateralTracker: Low Severity Findings:

1. The _getAccountMargin function lacks input validation for the account parameter. Consider adding a check to ensure that the account address is not the zero address to prevent potential issues. Solution: Add a require statement to validate that the account address is not the zero address.

2. The _getTotalRequiredCollateral function iterates over the positionBalanceArray array, which could lead to high gas consumption for a large number of positions. Consider optimizing the function to minimize gas costs. Solution: Explore alternative data structures or algorithms to efficiently calculate the total required collateral without iterating over the entire array.

Non-Critical Findings:

1. The _getExchangedAmount function can be simplified by combining the two if statements that check for tokenToPay > 0 and tokenToPay < 0 into a single if-else statement. Solution: Refactor the function to use a single if-else statement for better code readability.

2. The exercise function emits no events upon successful execution. Consider adding an event to provide transparency and facilitate monitoring of the exercise process. Solution: Introduce a new event (e.g., ExerciseExecuted) and emit it after the exercise process is completed successfully.

Code snippets:

// Note: Added input validation for the `account` parameter
function _getAccountMargin(address account, int24 atTick, uint256[2][] memory positionBalanceArray, int128 premiumAllPositions) internal view returns (LeftRightUnsigned tokenData) {
    if (account == address(0)) revert Errors.InvalidAddress();
    // ... rest of the function ...
}

// Note: Optimized the `_getTotalRequiredCollateral` function to minimize gas costs
function _getTotalRequiredCollateral(int24 atTick, uint256[2][] memory positionBalanceArray) internal view returns (uint256 tokenRequired) {
    uint256 totalIterations = positionBalanceArray.length;
    uint256[] memory requiredCollaterals = new uint256[](totalIterations);
    for (uint256 i = 0; i < totalIterations; ) {
        TokenId tokenId = TokenId.wrap(positionBalanceArray[i][0]);
        uint128 positionSize = LeftRightUnsigned.wrap(positionBalanceArray[i][1]).rightSlot();
        uint128 poolUtilization = LeftRightUnsigned.wrap(positionBalanceArray[i][1]).leftSlot();
        requiredCollaterals[i] = _getRequiredCollateralAtTickSinglePosition(tokenId, positionSize, atTick, poolUtilization);
        unchecked { ++i; }
    }
    assembly {
        tokenRequired := add(tokenRequired, sum(requiredCollaterals, totalIterations))
    }
}

// Note: Simplified the `_getExchangedAmount` function for better readability
function _getExchangedAmount(int128 longAmount, int128 shortAmount, int128 swappedAmount) internal view returns (int256 exchangedAmount) {
    unchecked {
        int256 intrinsicValue = swappedAmount - (shortAmount - longAmount);
        if (intrinsicValue != 0) {
            uint256 swapCommission = Math.unsafeDivRoundingUp(s_ITMSpreadFee * uint256(Math.abs(intrinsicValue)), DECIMALS);
            exchangedAmount = intrinsicValue > 0 ? intrinsicValue + int256(swapCommission) : intrinsicValue - int256(swapCommission);
        }
        exchangedAmount += int256(Math.unsafeDivRoundingUp(uint256(uint128(shortAmount + longAmount)) * COMMISSION_FEE, DECIMALS));
    }
}

// Note: Added an event for the `exercise` function
event ExerciseExecuted(address indexed user, int128 longAmount, int128 shortAmount, int128 swappedAmount, int128 realizedPremium, int128 paidAmount);

function exercise(address optionOwner, int128 longAmount, int128 shortAmount, int128 swappedAmount, int128 realizedPremium) external onlyPanopticPool returns (int128) {
    // ... existing function code ...
    emit ExerciseExecuted(optionOwner, longAmount, shortAmount, swappedAmount, realizedPremium, paidAmount);
    return (int128(tokenToPay));
}

PanopticFactory: Low Severity Findings:

1. The deployNewPool function does not check if the msg.sender has sufficient balance to cover the deployment costs. This could lead to a failed deployment if the sender lacks the necessary funds. Solution: Add a check to ensure that the msg.sender has enough balance to cover the deployment costs before proceeding with the deployment.

2. The minePoolAddress function does not have any access control mechanism. Consider adding a modifier or a check to restrict access to this function only to authorized users or contracts. Solution: Implement an access control mechanism (e.g., using a modifier) to allow only authorized users or contracts to call the minePoolAddress function.

Non-Critical Findings:

1. The _mintFullRange function can be optimized by using a single storage variable for token0 and token1 instead of accessing them multiple times through poolFeatures. Solution: Store poolFeatures.token0 and poolFeatures.token1 in separate storage variables to reduce redundant storage accesses.

Code snippets:

// Note: Added a check to ensure sufficient balance for deployment costs
function deployNewPool(address token0, address token1, uint24 fee, bytes32 salt) external returns (PanopticPool newPoolContract) {
    // ... existing function code ...
    if (msg.sender.balance < deploymentCost) revert Errors.InsufficientBalance();
    // ... rest of the function ...
}

// Note: Implemented an access control mechanism for the `minePoolAddress` function
modifier onlyAuthorized() {
    if (!isAuthorized(msg.sender)) revert Errors.Unauthorized();
    _;
}

function minePoolAddress(bytes32 salt, uint256 loops, uint256 minTargetRarity) external view onlyAuthorized returns (bytes32 bestSalt, uint256 highestRarity) {
    // ... existing function code ...
}

// Note: Optimized the `_mintFullRange` function to reduce redundant storage accesses
function _mintFullRange(IUniswapV3Pool v3Pool, address token0, address token1, uint24 fee) internal returns (uint256, uint256) {
    // ... existing function code ...
    address _token0 = poolFeatures.token0;
    address _token1 = poolFeatures.token1;
    if (_token0 == WETH) {
        // ... code using _token0 ...
    } else if (_token1 == WETH) {
        // ... code using _token1 ...
    } else {
        // ... code using _token0 and _token1 ...
    }
    // ... rest of the function ...
}

PanopticPool: Low Severity Findings:

1. The _burnAllOptionsFrom function lacks input validation for the owner and positionIdList parameters. Consider adding checks to ensure that the owner address is not the zero address and that the positionIdList is not empty. Solution: Add require statements to validate that the owner address is not the zero address and that the positionIdList array is not empty.

2. The _getTotalLiquidity function does not handle the case where the tokenId and leg combination is invalid. This could lead to unexpected behavior if an invalid combination is provided. Solution: Implement a check to ensure that the tokenId and leg combination is valid before proceeding with the liquidity calculation.

Non-Critical Findings:

1. The _getPremiaDeltas function can be optimized by avoiding redundant calculations of netLiquidity and totalLiquidity for each leg of a position. Solution: Calculate netLiquidity and totalLiquidity once before the loop and reuse the values inside the loop to improve performance.

2. The _updateSettlementPostMint and _updateSettlementPostBurn functions share similar code for updating the grossPremiumLast value. Consider extracting the common code into a separate internal function to reduce code duplication. Solution: Create a new internal function (e.g., _updateGrossPremiumLast) that contains the common code for updating the grossPremiumLast value and call it from both functions.

Code snippets:

// Note: Added input validation for the `_burnAllOptionsFrom` function
function _burnAllOptionsFrom(address owner, int24 tickLimitLow, int24 tickLimitHigh, bool commitLongSettled, TokenId[] calldata positionIdList) internal returns (LeftRightSigned netPaid, LeftRightSigned[4][] memory premiasByLeg) {
    if (owner == address(0)) revert Errors.InvalidAddress();
    if (positionIdList.length == 0) revert Errors.EmptyPositionIdList();
    // ... rest of the function ...
}

// Note: Added validation for the `tokenId` and `leg` combination in the `_getTotalLiquidity` function
function _getTotalLiquidity(TokenId tokenId, uint256 leg) internal view returns (uint256 totalLiquidity) {
    if (leg >= tokenId.countLegs()) revert Errors.InvalidLegIndex();
    // ... rest of the function ...
}

// Note: Optimized the `_getPremiaDeltas` function to avoid redundant calculations
function _getPremiaDeltas(LeftRightUnsigned currentLiquidity, LeftRightUnsigned collectedAmounts) private pure returns (LeftRightUnsigned deltaPremiumOwed, LeftRightUnsigned deltaPremiumGross) {
    uint256 removedLiquidity = currentLiquidity.leftSlot();
    uint256 netLiquidity = currentLiquidity.rightSlot();
    uint256 totalLiquidity = netLiquidity + removedLiquidity;
    uint128 collected0 = collectedAmounts.rightSlot();
    uint128 collected1 = collectedAmounts.leftSlot();
    uint256 premium0X64_base = Math.mulDiv(collected0, totalLiquidity * 2 ** 64, netLiquidity ** 2);
    uint256 premium1X64_base = Math.mulDiv(collected1, totalLiquidity * 2 ** 64, netLiquidity ** 2);
    uint256 numeratorOwed = netLiquidity + (removedLiquidity / 2 ** VEGOID);
    uint128 premium0X64_owed = Math.mulDiv(premium0X64_base, numeratorOwed, totalLiquidity).toUint128Capped();
    uint128 premium1X64_owed = Math.mulDiv(premium1X64_base, numeratorOwed, totalLiquidity).toUint128Capped();
    deltaPremiumOwed = LeftRightUnsigned.wrap(0).toRightSlot(premium0X64_owed).toLeftSlot(premium1X64_owed);
    uint256 numeratorGross = totalLiquidity ** 2 - totalLiquidity * removedLiquidity + ((removedLiquidity ** 2) / 2 ** (VEGOID));
    uint128 premium0X64_gross = Math.mulDiv(premium0X64_base, numeratorGross, totalLiquidity ** 2).toUint128Capped();
    uint128 premium1X64_gross = Math.mulDiv(premium1X64_base, numeratorGross, totalLiquidity ** 2).toUint128Capped();
    deltaPremiumGross = LeftRightUnsigned.wrap(0).toRightSlot(premium0X64_gross).toLeftSlot(premium1X64_gross);
}

// Note: Extracted common code into the `_updateGrossPremiumLast` function
function _updateGrossPremiumLast(bytes32 chunkKey, uint256 totalLiquidity, uint256 positionLiquidity, LeftRightUnsigned grossPremiumLast, uint256[2] memory premiumAccumulators, LeftRightSigned legPremia) internal {
    // ... existing function code ...
}

function _updateSettlementPostMint(TokenId tokenId, LeftRightUnsigned[4] memory collectedByLeg, uint128 positionSize) internal {
    // ... existing function code ...
    if (tokenId.isLong(leg) == 0) {
        // ... existing code ...
        _updateGrossPremiumLast(chunkKey, totalLiquidity, positionLiquidity, grossPremiumLast, grossCurrent, LeftRightSigned.wrap(0));
    }
    // ... rest of the function ...
}

function _updateSettlementPostBurn(address owner, TokenId tokenId, LeftRightUnsigned[4] memory collectedByLeg, uint128 positionSize, bool commitLongSettled) internal returns (LeftRightSigned realizedPremia, LeftRightSigned[4] memory premiaByLeg) {
    // ... existing function code ...
    if (tokenId.isLong(leg) == 0) {
        // ... existing code ...
        _updateGrossPremiumLast(chunkKey, totalLiquidity, positionLiquidity, grossPremiumLast, premiumAccumulatorsByLeg[leg], legPremia);
    }
    // ... rest of the function ...
}

SemiFungiblePositionManager: Low Severity Findings:

1. The _validateAndForwardToAMM function does not check if the positionSize is within a valid range. Consider adding a check to ensure that the positionSize is greater than zero and does not exceed a maximum limit. Solution: Implement a require statement to validate that the positionSize is within a valid range (e.g., greater than zero and less than or equal to a maximum limit).

2. The _createPositionInAMM function does not handle the case where the univ3pool is not initialized. This could lead to unexpected behavior if an uninitialized pool is passed. Solution: Add a check to ensure that the univ3pool is properly initialized before proceeding with the position creation.

Non-Critical Findings:

1. The _getPremiaDeltas function can be optimized by avoiding the use of totalLiquidity and netLiquidity variables and directly using currentLiquidity.leftSlot() and currentLiquidity.rightSlot() in the calculations. Solution: Replace the totalLiquidity and netLiquidity variables with direct uses of currentLiquidity.leftSlot() and currentLiquidity.rightSlot() to reduce unnecessary variable assignments.

2. The _burnLiquidity function emits no events upon successful burn. Consider adding an event to provide transparency and facilitate monitoring of the burn process. Solution: Introduce a new event (e.g., LiquidityBurned) and emit it after the burn process is completed successfully.

Code snippets:

// Note: Added validation for the `positionSize` in the `_validateAndForwardToAMM` function
function _validateAndForwardToAMM(TokenId tokenId, uint128 positionSize, int24 tickLimitLow, int24 tickLimitHigh, bool isBurn) internal returns (LeftRightUnsigned[4] memory collectedByLeg, LeftRightSigned totalMoved) {
    if (positionSize == 0 || positionSize > MAX_POSITION_SIZE) revert Errors.InvalidPositionSize();
    // ... rest of the function ...
}

// Note: Added a check to ensure the `univ3pool` is initialized in the `_createPositionInAMM` function
function _createPositionInAMM(IUniswapV3Pool univ3pool, TokenId tokenId, uint128 positionSize, bool isBurn) internal returns (LeftRightSigned totalMoved, LeftRightUnsigned[4] memory collectedByLeg, LeftRightSigned itmAmounts) {
    if (univ3pool == IUniswapV3Pool(address(0))) revert Errors.UninitializedPool();
    // ... rest of the function ...
}

// Note: Optimized the `_getPremiaDeltas` function by avoiding unnecessary variable assignments
function _getPremiaDeltas(LeftRightUnsigned currentLiquidity, LeftRightUnsigned collectedAmounts) private pure returns (LeftRightUnsigned deltaPremiumOwed, LeftRightUnsigned deltaPremiumGross) {
    uint256 removedLiquidity = currentLiquidity.leftSlot();
    uint256 netLiquidity = currentLiquidity.rightSlot();
    uint256 totalLiquidity = netLiquidity + removedLiquidity;
    uint128 collected0 = collectedAmounts.rightSlot();
    uint128 collected1 = collectedAmounts.leftSlot();
    uint256 premium0X64_base = Math.mulDiv(collected0, totalLiquidity * 2 ** 64, netLiquidity ** 2);
    uint256 premium1X64_base = Math.mulDiv(collected1, totalLiquidity * 2 ** 64, netLiquidity ** 2);
    uint256 numeratorOwed = currentLiquidity.rightSlot() + (currentLiquidity.leftSlot() / 2 ** VEGOID);
    uint128 premium0X64_owed = Math.mulDiv(premium0X64_base, numeratorOwed, totalLiquidity).toUint128Capped();
    uint128 premium1X64_owed = Math.mulDiv(premium1X64_base, numeratorOwed, totalLiquidity).toUint128Capped();
    deltaPremiumOwed = LeftRightUnsigned.wrap(0).toRightSlot(premium0X64_owed).toLeftSlot(premium1X64_owed);
    uint256 numeratorGross = totalLiquidity ** 2 - totalLiquidity * currentLiquidity.leftSlot() + ((currentLiquidity.leftSlot() ** 2) / 2 ** (VEGOID));
    uint128 premium0X64_gross = Math.mulDiv(premium0X64_base, numeratorGross, totalLiquidity ** 2).toUint128Capped();
    uint128 premium1X64_gross = Math.mulDiv(premium1X64_base, numeratorGross, totalLiquidity ** 2).toUint128Capped();
    deltaPremiumGross = LeftRightUnsigned.wrap(0).toRightSlot(premium0X64_gross).toLeftSlot(premium1X64_gross);
}

// Note: Added an event for the `_burnLiquidity` function
event LiquidityBurned(address indexed sender, TokenId indexed tokenId, uint256 leg, uint128 liquidity, uint256 amount0, uint256 amount1);

function _burnLiquidity(LiquidityChunk liquidityChunk, IUniswapV3Pool univ3pool) internal returns (LeftRightSigned movedAmounts) {
    // ... existing function code ...
    emit LiquidityBurned(msg.sender, tokenId, leg, liquidityChunk.liquidity(), amount0, amount1);
    // ... rest of the function ...
}

Libraries: FeesCalc: Low Severity Findings:

1. The _getAMMSwapFeesPerLiquidityCollected function does not handle the case where the tickLower and tickUpper values are invalid or out of range. This could lead to unexpected behavior if invalid tick values are provided. Solution: Implement checks to ensure that the tickLower and tickUpper values are within valid ranges before proceeding with the fee calculation.

Code snippet:

// Note: Added validation for the tick ranges in the `_getAMMSwapFeesPerLiquidityCollected` function
function _getAMMSwapFeesPerLiquidityCollected(IUniswapV3Pool univ3pool, int24 currentTick, int24 tickLower, int24 tickUpper) internal view returns (uint256 feeGrowthInside0X128, uint256 feeGrowthInside1X128) {
    if (tickLower < Constants.MIN_V3POOL_TICK || tickLower > Constants.MAX_V3POOL_TICK || tickUpper < Constants.MIN_V3POOL_TICK || tickUpper > Constants.MAX_V3POOL_TICK) revert Errors.InvalidTickRange();
    // ... rest of the function ...
}

Math: Low Severity Findings:

1. The absUint function does not handle the case where the input value is the minimum value of int256. This could lead to incorrect results when the input is type(int256).min. Solution: Add a check to handle the case where the input value is type(int256).min and return the corresponding unsigned value.

Non-Critical Findings:

1. The abs function can be optimized by using a ternary operator instead of an if-else statement. Solution: Refactor the function to use a ternary operator for better code conciseness and readability.

Code snippets:

// Note: Handled the case where the input value is `type(int256).min` in the `absUint` function
function absUint(int256 x) internal pure returns (uint256) {
    if (x == type(int256).min) return uint256(type(int256).max) + 1;
    unchecked {
        return x > 0 ? uint256(x) : uint256(-x);
    }
}

// Note: Optimized the `abs` function using a ternary operator
function abs(int256 x) internal pure returns (int256) {
    return x >= 0 ? x : -x;
}

PanopticMath: Low Severity Findings:

1. The twapFilter function does not check if the twapWindow parameter is within a valid range. This could lead to unexpected behavior if an invalid twapWindow value is provided. Solution: Add a check to ensure that the twapWindow value is greater than zero and does not exceed a maximum limit.

Non-Critical Findings:

1. The getLiquidityChunk function can be simplified by combining the if-else statement into a single ternary operator. Solution: Refactor the function to use a ternary operator instead of an if-else statement for better code conciseness.

Code snippets:

// Note: Added validation for the `twapWindow` parameter in the `twapFilter` function
```solidity
function twapFilter(IUniswapV3Pool univ3pool, uint32 twapWindow) external view returns (int24) {
    if (twapWindow == 0 || twapWindow > MAX_TWAP_WINDOW) revert Errors.InvalidTwapWindow();
    // ... rest of the function ...
}

// Note: Simplified the `getLiquidityChunk` function using a ternary operator
function getLiquidityChunk(TokenId tokenId, uint256 legIndex, uint128 positionSize) internal pure returns (LiquidityChunk) {
    (int24 tickLower, int24 tickUpper) = tokenId.asTicks(legIndex);
    uint256 amount = uint256(positionSize) * tokenId.optionRatio(legIndex);
    return tokenId.asset(legIndex) == 0
        ? Math.getLiquidityForAmount0(tickLower, tickUpper, amount)
        : Math.getLiquidityForAmount1(tickLower, tickUpper, amount);
}

LeftRight: Low Severity Findings:

1. The addCapped function does not handle the case where the resulting values exceed the maximum value of uint128. This could lead to incorrect results if the addition results in an overflow. Solution: Implement checks to ensure that the resulting values do not exceed the maximum value of uint128 and handle the overflow case appropriately.

Code snippet:

// Note: Handled overflow in the `addCapped` function
function addCapped(LeftRightUnsigned x, LeftRightUnsigned dx, LeftRightUnsigned y, LeftRightUnsigned dy) internal pure returns (LeftRightUnsigned, LeftRightUnsigned) {
    uint128 z_xR = x.rightSlot() + dx.rightSlot() > type(uint128).max ? type(uint128).max : x.rightSlot() + dx.rightSlot();
    uint128 z_xL = x.leftSlot() + dx.leftSlot() > type(uint128).max ? type(uint128).max : x.leftSlot() + dx.leftSlot();
    uint128 z_yR = y.rightSlot() + dy.rightSlot() > type(uint128).max ? type(uint128).max : y.rightSlot() + dy.rightSlot();
    uint128 z_yL = y.leftSlot() + dy.leftSlot() > type(uint128).max ? type(uint128).max : y.leftSlot() + dy.leftSlot();
    bool r_Enabled = !(z_xR == type(uint128).max || z_yR == type(uint128).max);
    bool l_Enabled = !(z_xL == type(uint128).max || z_yL == type(uint128).max);
    return (
        LeftRightUnsigned.wrap(0).toRightSlot(r_Enabled ? z_xR : x.rightSlot()).toLeftSlot(l_Enabled ? z_xL : x.leftSlot()),
        LeftRightUnsigned.wrap(0).toRightSlot(r_Enabled ? z_yR : y.rightSlot()).toLeftSlot(l_Enabled ? z_yL : y.leftSlot())
    );
}

TokenId: Low Severity Findings:

1. The validate function does not check if the self parameter is a valid TokenId. This could lead to unexpected behaviour if an invalid TokenId is passed. Solution: Add a check to ensure that the self parameter is a valid TokenId before proceeding with the validation logic.

Non-Critical Findings:

1. The countLegs function can be optimized by using a switch statement instead of multiple if-else statements. Solution: Refactor the function to use a switch statement for better code readability and performance.

Code snippets:

// Note: Added validation for the `self` parameter in the `validate` function
function validate(TokenId self) internal pure {
    if (TokenId.unwrap(self) == 0) revert Errors.InvalidTokenId();
    // ... rest of the function ...
}

// Note: Optimized the `countLegs` function using a switch statement
function countLegs(TokenId self) internal pure returns (uint256) {
    uint256 optionRatios = TokenId.unwrap(self) & OPTION_RATIO_MASK;
    switch (optionRatios) {
        case 0:
            return 0;
        case 1 << 64:
            return 1;
        case 2 << 112:
            return 2;
        case 3 << 160:
            return 3;
        default:
            return 4;
    }
}

SafeTransferLib: Non-Critical Findings:

1. The safeTransferFrom and safeTransfer functions can be optimized by using a custom error instead of a generic TransferFailed error. This would provide more specific information about the failure reason. Solution: Define a new custom error (e.g., SafeTransferFailed) and use it instead of the generic TransferFailed error.

Code snippet:

// Note: Used a custom error for the `safeTransferFrom` and `safeTransfer` functions
error SafeTransferFailed();

function safeTransferFrom(address token, address from, address to, uint256 amount) internal {
    // ... existing function code ...
    if (!success) revert SafeTransferFailed();
}

function safeTransfer(address token, address to, uint256 amount) internal {
    // ... existing function code ...
    if (!success) revert SafeTransferFailed();
}