PoolSharkBaseLiquidityManager Manual Review Findings

PSB-01M: Incorrect Usage of Checked Arithmetic (TWAP Formula)

Type	Severity	Location
Mathematical Operations		PoolSharkBaseLiquidityManager.sol:L560

Description:

The referenced statement will perform a safe subtraction of the cumulative tick values which is incorrect per the original implementation of the OracleLibrary::consult library which will deliberately permit the cumulative values to underflow.

Impact:

The PoolSharkBaseLiquidityManager::_checkVolatility function may fail to execute under certain cumulative value conditions as the delta between ticks may result in an underflow that should be permitted.

Example:

Copy
542/// @dev revert if volatility is above acceptable levels
543///      (mainly used to prevent flashloan attacks)
544/// @param currentTick Current pool tick
545function _checkVolatility(int24 currentTick) internal view {
546    // SLOADS for efficiency
547    uint32 _twapInterval = twapInterval;
548
549    int24 _maxTickChange = maxTickChange;
550
551    // Get TWAP tick
552    uint32[] memory secondsAgos = new uint32[](2);
553    secondsAgos[0] = _twapInterval;
554
555    // tickCumulatives is basically where the tick was as of twapInterval seconds ago
556    (int56[] memory tickCumulatives, , , , ) = pool.sample(secondsAgos);
557
558    // tickCumulatives[1] will always be greater than tickCumulatives[0]
559    int24 twapTick = int24(
560        (tickCumulatives[1] - tickCumulatives[0]) / int32(twapInterval)
561    );
562
563    // Make sure currentTick is not more than maxTickChange ticks away from twapTick
564    // No SafeMath here--even if a compromised governance contract set _maxTickChange to a very high value,
565    // it would only wrap around and cause this check to fail.
566    require(
567        currentTick <= twapTick + _maxTickChange &&
568            currentTick >= twapTick - _maxTickChange,
569        "V"
570    );
571}

Recommendation:

We advise the referenced arithmetic statements to be wrapped in an unchecked code block, ensuring functional equivalence of the PoolSharkBaseLiquidityManager::_checkVolatility function with its Uniswap V3 counterpart.

Alleviation (6513a21a002d422e298719b22f73a4559dfd4663):

The relevant code has been relocated to the Helper::poolsharkCheckVolatility function, and the referenced subtraction does not need to be wrapped in an unchecked code block as it should never overflow per Uniswap V3's own 0.8.X compatible oracle implementation.

As such, we consider this exhibit inapplicable.

PSB-02M: Inexecutable Emergency Operation

Type	Severity	Location
Logical Fault		PoolSharkBaseLiquidityManager.sol:L290-L295

Description:

The PoolSharkBaseLiquidityManager::emergencyBurn function will be unable to properly execute, as the code will attempt to burn 100% of the position's liquidity and then an additional 1e-36 percent which will fail as the Poolshark system will burn the position ID if its full liquidity is removed.

Impact:

An emergency burn operation is presently inexecutable and even if it managed to execute, it would lead to a downstream error in the PoolSharkMultiPositionLiquidityManager contract that would affect the overall liquidity manager's operation.

Example:

Copy
279/// @notice Removes liquidity in case of emergency.
280function emergencyBurn(
281    uint32 pId
282) external onlyRole(STEER_ROLE) returns (int256 amount0, int256 amount1) {
283    PoolsharkStructs.BurnRangeParams memory burnParams = PoolsharkStructs
284        .BurnRangeParams({
285            to: address(this),
286            positionId: pId,
287            burnPercent: 1e38
288        }); //1e38 100% of the liquidity in given position id
289    (amount0, amount1) = pool.burnRange(burnParams);
290    burnParams = PoolsharkStructs.BurnRangeParams({
291        to: address(this),
292        positionId: pId,
293        burnPercent: 1
294    }); //1 only collects the accumulated fees
295    (amount0, amount1) = pool.burnRange(burnParams);
296}

Recommendation:

We advise the code to perform a burn of 1e38 - 1, ensuring that a negligible portion of the liquidity remains in the position and the ID is not burned which may result in downstream failures in the PoolSharkMultiPositionLiquidityManager::getTotalAmounts function.

Alternatively, we advise the code to properly remove the NFT ID from the positionIds array ensuring that the data integrity of the PoolSharkMultiPositionLiquidityManager is not affected.

Alleviation (6513a21a002d422e298719b22f73a4559dfd4663):

The PoolSharkBaseLiquidityManager::emergencyBurn function has been relocated to the PoolSharkMultiPositionLiquidityManager contract, permitting the emergency burn operation to remove the position IDs burned from the positionIds entry thus ensuring the operation is executed safely.

PSB-03M: Inexistent Acceptance of Poolshark Positions

Type	Severity	Location
Logical Fault		PoolSharkBaseLiquidityManager.sol:L40-L47

Description:

The PoolSharkBaseLiquidityManager contract does not accept EIP-1155 transfers and thus will not be able to accept EIP-1155 positions minted via the Poolshark system.

Impact:

Neither the PoolSharkBaseLiquidityManager nor the PoolSharkMultiPositionLiquidityManager contract that derives from it are able to accept EIP-1155 transfers and thus will fail when minting range positions in the Poolshark AMM.

Example:

Copy
40abstract contract PoolSharkBaseLiquidityManager is
41    Initializable,
42    ERC20Upgradeable,
43    PausableUpgradeable,
44    AccessControlUpgradeable,
45    ILimitPoolMintRangeCallback,
46    ILimitPoolSwapCallback
47{

Recommendation:

We advise a proper handler to be introduced, ensuring that the contract is able to mint positions properly and retain them in custody.

Alleviation (6513a21a002d422e298719b22f73a4559dfd4663):

The Steer Protocol team reached out to the Poolshark team and validated that the contract does not need to actively accept EIP-1155 transfers; it merely needs to signal support for them via the IERC165::supportsInterface function.

As the integration point we identified as being incorrect has been assessed as correct by the integrated party, we consider this exhibit no longer applicable.