PoolInitializer Manual Review Findings

PIR-01M: Incorrect Pool Creation Fee Acquisition

Type	Severity	Location
Logical Fault		PoolInitializer.sol:L25-L31, L38-L43

Description:

The PoolInitializer::createAndInitializePoolIfNecessary function will mandate that the call contains the poolCreateFeeInTinybars even if no pool will be created due to pool evaluating to a non-zero address.

Impact:

Any PoolInitializer::createAndInitializePoolIfNecessary function invocation with a pool that has already been created will lead to loss of funds.

Example:

Copy
14function createAndInitializePoolIfNecessary(
15    address token0,
16    address token1,
17    uint24 fee,
18    uint160 sqrtPriceX96
19) external payable override returns (address pool) {
20    require(token0 < token1);
21    
22    uint256 poolCreateFee = IUniswapV3Factory(factory).poolCreateFee();
23    uint256 poolCreateFeeInTinybars;
24
25    if(poolCreateFee > 0) {
26        poolCreateFeeInTinybars = HbarConversion.tinycentsToTinybars(poolCreateFee);
27        
28        // Slop for conversion rounding
29        poolCreateFeeInTinybars += 1;
30        require(msg.value >= poolCreateFeeInTinybars, 'PCF');
31    }
32
33    pool = IUniswapV3Factory(factory).getPool(token0, token1, fee);
34
35    if (pool == address(0)) {
36        pool = IUniswapV3Factory(factory).createPool{value: poolCreateFeeInTinybars}(token0, token1, fee);
37        IUniswapV3Pool(pool).initialize(sqrtPriceX96);
38    } else {
39        (uint160 sqrtPriceX96Existing, , , , , , ) = IUniswapV3Pool(pool).slot0();
40        if (sqrtPriceX96Existing == 0) {
41            IUniswapV3Pool(pool).initialize(sqrtPriceX96);
42        }
43    }
44}

Recommendation:

We advise the fee to either be refunded if a pool already exists or its mandation to be omitted entirely as the IUniswapV3Factory of SaucerSwap already mandates that the fee is paid, meaning that the call can simply relay msg.value to the factory.

Alleviation (d8d187efd1):

The msg.value validation logic was relocated within the code branch that ensures a pool does not already exists, meaning that users are able to not send any funds if the pool has already been created.

We would like to note that this is a partial alleviation as it is still possible to lock funds within the contract by sending a non-zero msg.value with a pool that has already been created. To amend this, a require check should be introduced in the else branch of the code ensuring that the msg.value is 0 or refunding it as we originally advised.

Alleviation (92f8d9f89f):

Annotation was introduced to the PoolInitializer::createAndInitializePoolIfNecessary function, instructing users to make use of the Multicall contract to ensure a refund ETH transaction follows the creation and initialization of a pool, ensuring any superfluous funds sent to the contract are refunded to its caller.

Given that an execution path exists to properly acquire a refund in case more funds than required were transmitted, we consider this exhibit alleviated.