UniswapInteractionHelper Code Style Findings

UIH-01C: Implied Order of Operations

Type	Severity	Location
Code Style		UniswapInteractionHelper.sol:L153

Description:

The referenced statement contains a mathematical equation without strict definition of each operand's order.

Example:

Copy
153shares = amount1Max + (amount0Max * price) / PRECISION_FACTOR;

Recommendation:

We advise an additional parenthesis (()) to be introduced for the division statement of the equation as the order of arithmetic operations is not guaranteed to be the same in future Solidity versions.

Alleviation:

The order of operations is now distinctly depicted via the introduction of parenthesis as advised.

UIH-02C: Improper Negation of Values

Type	Severity	Location
Mathematical Operations		UniswapInteractionHelper.sol:L196-L198

Description:

The abs function is meant to calculate the absolute value of a signed data type (namely int24), however, the negation performed is incorrect.

Impact:

As there are no implications in the current mechanism due to Solidity's built-in safe arithmetics, this exhibit is inconsequential and should solely be adopted for a more legible error message.

Example:

Copy
196function abs(int24 x) internal pure returns (int24) {
197    return x >= 0 ? x : -x;
198}

Recommendation:

As signed data types in Solidity do not cover the full range and contain an extra negative value, a negation of type(int24).min will fail to execute in abs. We advise the x value to be sanitized as not equal to the type(int24).min value, ensuring that the abs function operations on a valid numerical range.

Alleviation:

The Metavisor team evaluated this exhibit and opted not to apply a remediation for it in the current iteration of the codebase as it is inconsequential.

UIH-03C: Ineffectual Usage of Safe Arithmetics

Type	Severity	Location
Language Specific		UniswapInteractionHelper.sol:L53

Description:

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

Example:

Copy
38(amount0, amount1) = pool.burn(
39    ticks.tickLower,
40    ticks.tickUpper,
41    uint128(liquidityToRemove)
42);
43
44// Always collects all fee.
45(uint256 collect0, uint256 collect1) = pool.collect(
46    address(this),
47    ticks.tickLower,
48    ticks.tickUpper,
49    type(uint128).max,
50    type(uint128).max
51);
52
53(fees0, fees1) = (collect0 - amount0, collect1 - amount1);

Recommendation:

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

Alleviation:

The Metavisor team evaluated this exhibit and opted not to apply a remediation for it in the current iteration of the codebase as it is inconsequential.

UIH-04C: Inefficient Boolean Argument

Type	Severity	Location
Gas Optimization		UniswapInteractionHelper.sol:L29

Description:

The burnLiquidity function includes an everything boolean argument meant to signify whether all liquidity available should be withdrawn.

Example:

Copy
24function burnLiquidity(
25    IUniswapV3Pool pool,
26    TicksData memory ticks,
27    uint256 shares,
28    uint256 totalSupply,
29    bool everything
30) internal returns (uint256 amount0, uint256 amount1, uint256 fees0, uint256 fees1) {
31    (uint128 liquidity, , ) = getLiquidityInPosition(pool, ticks);
32
33    uint256 liquidityToRemove = everything
34        ? liquidity
35        : FullMath.mulDiv(liquidity, shares, totalSupply);

Recommendation:

We advise the bool variable to be omitted and the shares value to be utilized for this purpose by evaluating whether it is equal to type(uint256).max, as is standard with most vault-based systems.

Alleviation:

The value of shares as type(uint256).max is now properly consumed as a "flag" signalling that the full liquidity should be removed as advised.

UIH-05C: Literal Value Presence

Type	Severity	Location
Gas Optimization		UniswapInteractionHelper.sol:L150

Description:

The code contains the 2 ** (96 * 2) value literal defined which will perform a multiplication prior to performing a power operation and is meant to represent the accuracy of sqrtPriceX96 to the power of 2 which is expected to be normalized in the referenced calculation.

Example:

Copy
147uint256 price = FullMath.mulDiv(
148    uint256(sqrtPriceX96) ** 2,
149    PRECISION_FACTOR,
150    2 ** (96 * 2)
151);

Recommendation:

We advise the literal to be relocated to a file-level constant representing the double accuracy of X96, optimizing the code's legibility and gas cost.

Alleviation:

The literal value has been relocated to a file-level constant declaration labelled X96, optimizing the codebase's legibility.

UIH-06C: Suboptimal Struct Declaration Style

Type	Severity	Location
Code Style		UniswapInteractionHelper.sol:L213

Description:

The linked declaration style of a struct is using index-based argument initialization.

Example:

Copy
213return TicksData(tickFloor - baseTicks, tickFloor + baseTicks);

Recommendation:

We advise the key-value declaration format to be utilized instead, greatly increasing the legibility of the codebase.

Alleviation:

The struct declaration now properly employs the key-value declaration format, increasing its legibility significantly.