Omniscia Steer Protocol Audit

AlgebraMultiPositionLiquidityManager Code Style Findings

AlgebraMultiPositionLiquidityManager Code Style Findings

AMP-01C: Generic Typographic Mistake

TypeSeverityLocation
Code StyleAlgebraMultiPositionLiquidityManager.sol:L409

Description:

The referenced line contains a typographical mistake (i.e. private variable without an underscore prefix) or generic documentational error (i.e. copy-paste) that should be corrected.

Example:

contracts/vault-types/AlgebraLiquidityManagers/AlgebraMultiPositionLiquidityManager.sol
409// Shares is always >= totalShares so no risk of uint128 overflow here.

Recommendation:

We advise this to be done so to enhance the legibility of the codebase.

Alleviation (0c3f85c7c11805ac412fe291f5681bef26da7244):

The referenced comment has been omitted, addressing this exhibit.

AMP-02C: Inefficient Usage of Storage Space

TypeSeverityLocation
Gas OptimizationAlgebraMultiPositionLiquidityManager.sol:L12, L32-L36

Description:

The LiquidityPositions structure utilized in AlgebraMultiPositionLiquidityManager is inefficient as it will store multiple dynamic arrays of sub-256-bit elements.

Example:

contracts/vault-types/AlgebraLiquidityManagers/AlgebraMultiPositionLiquidityManager.sol
12LiquidityPositions internal positions;
13
14// Types
15
16/// @dev The vault's position data. At any given moment this represents
17/// all active positions inside the pool.
18/// Each lowerTick is the lower bound of the position at that index.
19/// Each upperTick is the upper bound of the position at that index.
20/// Each relativeWeight is the relative weight of the position at that index,
21/// relative to the other positions.
22/// So for example if LiquidityPositions is
23/// {
24/// lowerTicks: [0, 20, 40],
25/// upperTicks: [10, 30, 50],
26/// relativeWeights: [1, 2, 3]
27/// }
28/// then that means the vault has 3 positions:
29/// 1. 0-10 with relative weight 1
30/// 2. 20-30 with relative weight 2
31/// 3. 40-50 with relative weight 3
32struct LiquidityPositions {
33 int24[] lowerTick;
34 int24[] upperTick;
35 uint16[] relativeWeight;
36}

Recommendation:

We advise the code to introduce a LiquidityPosition struct that contains the int24, int24, and uint16 entries and the positions member of the contract to be made a LiquidityPosition[] dynamic array.

In this case, a single SSTORE would be required per position rather than three in the current iteration, greatly reducing the gas cost of the contract.

Alleviation (0c3f85c7c11805ac412fe291f5681bef26da7244):

The Steer Protocol team evaluated this exhibit but opted not to apply it in the current iteration of the codebase as it would require significant changes throughout the contract's logic.

AMP-03C: Loop Iterator Optimizations

TypeSeverityLocation
Gas OptimizationAlgebraMultiPositionLiquidityManager.sol:L155, L195, L266, L302, L393

Description:

The linked for loops increment / decrement their iterator "safely" due to Solidity's built - in safe arithmetics (post-0.8.X).

Example:

contracts/vault-types/AlgebraLiquidityManagers/AlgebraMultiPositionLiquidityManager.sol
155for (uint256 i; i != positionCount; ++i) {

Recommendation:

We advise the increment / decrement operations to be performed in an unchecked code block as the last statement within each for loop to optimize their execution cost.

Alleviation (0c3f85c7c11805ac412fe291f5681bef26da7244):

The pragma version of the contract has been regressed to 0.7.6 no longer containing built-in safe arithmetics and thus indirectly addressing this exhibit.

AMP-04C: Redundant Convoluted Logic Structure

TypeSeverityLocation
Gas OptimizationAlgebraMultiPositionLiquidityManager.sol:L269-L277

Description:

The referenced code block will evaluate an if condition, revert if it evaluates to true, and evaluate a require check in its else clause.

Example:

contracts/vault-types/AlgebraLiquidityManagers/AlgebraMultiPositionLiquidityManager.sol
269if (i >= 1) {
270 if (_positions.lowerTick[i - 1] > _positions.lowerTick[i]) {
271 revert();
272 } else {
273 require(
274 _positions.upperTick[i - 1] < _positions.upperTick[i]
275 );
276 }
277}

Recommendation:

We advise the logic structure to be replaced by a single require check, increasing the code's legibility while decreasing its gas cost.

Alleviation (0c3f85c7c11805ac412fe291f5681bef26da7244):

The referenced structure has been optimized to a single require check per our recommendation.

AMP-05C: Repetitive Value Literals

TypeSeverityLocation
Code StyleAlgebraMultiPositionLiquidityManager.sol:L139, L141, L367, L371

Description:

The linked value literals are repeated across the codebase multiple times.

Example:

contracts/vault-types/AlgebraLiquidityManagers/AlgebraMultiPositionLiquidityManager.sol
139FullMath.mulDiv(balance0, totalWeight, 1e4),

Recommendation:

We advise each to be set to its dedicated constant variable instead optimizing the legibility of the codebase.

Alleviation (0c3f85c7c1):

The Steer Protocol team has stated that they do not wish to introduce constant declarations to the contract in fear of increasing its bytecode size.

All variables declared as constant are compilation artefacts and do not affect the contract's bytecode size. As such, we consider this exhibit addressed but advise the Steer Protocol team to reconsider applying it.

Alleviation (b1b5eabd4d):

All repetitive value literals have been properly defined as constant variables in the AlgebraBaseLiquidityManager dependency of the contract, alleviating this exhibit in full.