Omniscia Steer Protocol Audit

PluginHelper Manual Review Findings

PluginHelper Manual Review Findings

PHR-01M: Incorrect Evaluations of Algebra Vault Balances

Description:

The Algebra V4 positions can have fees that have yet to be captured which are tracked through their fee growth parameters, however, the PluginHelper::getAlgebraVaultBalances function completely ignores them resulting in them being able to be captured via new depositors immediately.

Impact:

Although fees are not accounted for in the vault's total asset state, the positions are properly poked before any deposits in PluginV1 rendering this issue to solely manifest for evaluating whether a re-balance is required which is less sensitive.

Example:

src/libraries/PluginHelper.sol
205for (uint256 i; i != data.positionCount; ++i) {
206 (data.liquidity,,, data.fees0, data.fees1) = IAlgebraPool(pool).positions(
207 PositionKey.compute(msg.sender, positions[i].lowerTick, positions[i].upperTick)
208 );
209
210 (data.amt0, data.amt1) = LiquidityAmounts.getAmountsForLiquidity(
211 sqrtPriceX96,
212 TickMath.getSqrtRatioAtTick(positions[i].lowerTick),
213 TickMath.getSqrtRatioAtTick(positions[i].upperTick),
214 uint128(data.liquidity)
215 );
216
217 total0 += data.amt0 + FullMath.mulDiv(data.fees0, data.feeSubtract, FEE_DIVISOR);
218 total1 += data.amt1 + FullMath.mulDiv(data.fees1, data.feeSubtract, FEE_DIVISOR);
219}

Recommendation:

We advise the code to properly accommodate for any pending fees to ensure the before-swap hook of the PluginV1 implementation functions as expected.

Alleviation:

The Steer Protocol team evaluated this exhibit and opted to acknowledge this behaviour as they anticipate the discrepancy to be negligible when it comes to a rebalance operation's sensitivity.

PHR-02M: Underestimation of Vault Balance

Description:

The PluginHelper::getAlgebraVaultBalances function will underestimate the vault's balance by at most 1 unit due to rounding down the fee due for the vault.

Specifically, the PluginV1 implementation will impose a fee on the acquired fees and subtract the percentage-based proportion from the total fee amount. This infers that the protocol fees are rounded down and the user fees are rounded upward whereas the PluginHelper does the opposite.

Impact:

A maximum error of 1 will be observed when evaluating the vault's balance for each asset due to the way protocol fees are deducted.

Example:

src/libraries/PluginHelper.sol
200AlgebraBalanceCalculationData memory data;
201data.totalFees = IFeeManager(feeManager).vaultTotalFees(msg.sender);
202data.feeSubtract = FEE_DIVISOR - data.totalFees;
203data.positionCount = positions.length;
204(uint160 sqrtPriceX96,,,,,) = IAlgebraPool(pool).globalState();
205for (uint256 i; i != data.positionCount; ++i) {
206 (data.liquidity,,, data.fees0, data.fees1) = IAlgebraPool(pool).positions(
207 PositionKey.compute(msg.sender, positions[i].lowerTick, positions[i].upperTick)
208 );
209
210 (data.amt0, data.amt1) = LiquidityAmounts.getAmountsForLiquidity(
211 sqrtPriceX96,
212 TickMath.getSqrtRatioAtTick(positions[i].lowerTick),
213 TickMath.getSqrtRatioAtTick(positions[i].upperTick),
214 uint128(data.liquidity)
215 );
216
217 total0 += data.amt0 + FullMath.mulDiv(data.fees0, data.feeSubtract, FEE_DIVISOR);
218 total1 += data.amt1 + FullMath.mulDiv(data.fees1, data.feeSubtract, FEE_DIVISOR);
219}

Recommendation:

We advise the code to properly calculate the fees due for the system (i.e. data.fees0 * data.totalFees / FEE_DIVISOR) and to subtract this value from each respective fee ensuring the vault's balance is correctly calculated.

Alleviation:

All referenced multiplication and divisions were replaced where needed with subtractions to ensure an upward rounding operation is observed.

PHR-03M: Inexistent Rounding of TWAP

Description:

The TWAP observation implemented through the Algebra volatility oracle plugin implemented in PluginV1 is meant to mimic the Algebra V4 approach and should thus round toward negative infinity as well.

Impact:

The current TWAP tick calculation mechanism will round toward zero which contradicts the expected behaviour of the system for negative values.

Example:

src/libraries/PluginHelper.sol
490function volatilityCheck(int24 currentTick, uint32 _twapInterval, int56 _maxTickChange) external view {
491 // Get TWAP tick
492 uint32[] memory secondsAgos = new uint32[](2);
493 secondsAgos[0] = _twapInterval;
494
495 // tickCumulatives is basically where the tick was as of twapInterval seconds ago
496 (int56[] memory tickCumulatives,) = IVolatilityOracle(msg.sender).getTimepoints(secondsAgos);
497
498 // tickCumulatives[1] will always be greater than [0]
499 // so no need to check for underflow or division overflow here.
500 int24 twapTick = int24((tickCumulatives[1] - tickCumulatives[0]) / int56(uint56(_twapInterval)));
501
502 // Make sure currentTick is not more than maxTickChange ticks away from twapTick
503 // No SafeMath here--even if a compromised governance contract set _maxTickChange to a very high value,
504 // it would only wrap around and cause this check to fail.
505 require(currentTick <= twapTick + _maxTickChange && currentTick >= twapTick - _maxTickChange, "V");
506}

Recommendation:

We advise the same behaviour to be implemented hear as well, ensuring that the observed tick is correct for negative values.

Alleviation:

The code was revised to properly round toward negative infinity for negative tick values, alleviating this exhibit.