Omniscia Steer Protocol Audit

BlackholePositionManagerFeeManager Code Style Findings

BlackholePositionManagerFeeManager Code Style Findings

BPM-01C: Code Repetition

Description:

The referenced code blocks are repetitive with minor adjustments.

Example:

src/BlackholePositionManagerFeeManager.sol
590if (!_getFlag(6)) { // if skipExtra == false
591    extraFees = extraRewardToken.balanceOf(address(this)) - startingBalanceExtra;
592    if (extraFees > 0) {
593        if (_getFlag(2)) { // rewardExtraIsPoolToken
594            if (_getFlag(3)) { // rewardExtraIsToken0
595                rewardBal0 += extraFees;
596            } else {
597                rewardBal1 += extraFees;
598            }
599        }
600        totalCutExtra = FullMath.mulDiv(extraFees, totalFees, DIVISOR);
601        uint256 netGainExtra = extraFees - totalCutExtra;
602        reservedExtraFee += totalCutExtra;
603        accumulatedExtraRewardsPerShareP += ((netGainExtra * PRECISION)/
604            totalSupply);
605        if (totalCutExtra > 0) {
606            for (uint256 j; j != feesInfo.length; ++j) {
607                accruedFees1[feesInfo[j].feeIdentifier] = accruedFees1[
608                    feesInfo[j].feeIdentifier
609                ] + (FullMath.mulDiv(totalCutExtra, feesInfo[j].feeValue, DIVISOR));
610            }
611        }
612    }
613}
614

615uint256 fees = baseRewardToken.balanceOf(address(this)) - startingBalance;
616if (fees > 0) {
617    if (_getFlag(0)) { // rewardIsPoolToken
618        if (_getFlag(1)) { // rewardIsToken0
619            rewardBal0 += fees;
620        } else {
621            rewardBal1 += fees;
622        }
623    }
624    totalCutBase = FullMath.mulDiv(fees, totalFees, DIVISOR);
625    uint256 netGain = fees - totalCutBase;
626    accumulatedRewardsPerShareP += ((netGain * PRECISION) /
627        totalSupply);
628    reservedBaseFee += totalCutBase;
629    if (totalCutBase > 0 ) {
630        for (uint256 j; j != feesInfo.length; ++j) {
631            accruedFees0[feesInfo[j].feeIdentifier] = accruedFees0[
632                feesInfo[j].feeIdentifier
633            ] + (FullMath.mulDiv(totalCutBase, feesInfo[j].feeValue, DIVISOR));
634        }
635    }
636}

Recommendation:

We advise them to be refactored into internal functions, minimizing the code's bytecode size whilst increasing its maintainability.

Alleviation (aafd869b2be09d232f2bbb4bdcbb3479d829f6ec):

The Steer Protocol team evaluated this exhibit but opted to not apply it due to bytecode size constraints.

BPM-02C: Generic Typographic Mistakes

TypeSeverityLocation
Code StyleBlackholePositionManagerFeeManager.sol:
• I-1: L43
• I-2: L191
• I-3: L539
• I-4: L551

Description:

The referenced lines contain typographical mistakes (i.e. private variable without an underscore prefix) or generic documentational errors (i.e. copy-paste) that should be corrected.

Example:

src/BlackholePositionManagerFeeManager.sol
43IBlackholePositionManager

Recommendation:

We advise them to be corrected enhancing the legibility of the codebase.

Alleviation (aafd869b2b):

While some typographic mistakes have been corrected, styling issues continue to exist in the code.

We advise a proper linter to be applied so as to avoid these types of issues entirely.

Alleviation (bf4fc8ec2d):

A linter has been applied to the codebase per our recommendation, greatly increasing the code's legibility and addressing this exhibit fully.

BPM-03C: Illegible Usage of Special-Purpose Flag Concept

TypeSeverityLocation
Code StyleBlackholePositionManagerFeeManager.sol:
• I-1: L117
• I-2: L120-L122
• I-3: L124-L130
• I-4: L134
• I-5: L138
• I-6: L142
• I-7: L146
• I-8: L224
• I-9: L266
• I-10: L281
• I-11: L289
• I-12: L290
• I-13: L303
• I-14: L304
• I-15: L377
• I-16: L380
• I-17: L381
• I-18: L383
• I-19: L387
• I-20: L389
• I-21: L390
• I-22: L392
• I-23: L554
• I-24: L569
• I-25: L590
• I-26: L593
• I-27: L594
• I-28: L617
• I-29: L618
• I-30: L777
• I-31: L946
• I-32: L1136
• I-33: L1137
• I-34: L1148
• I-35: L1149
• I-36: L1165
• I-37: L1197
• I-38: L1198
• I-39: L1207
• I-40: L1212
• I-41: L1213

Description:

The referenced flag concept is significantly illegible as it utilizes value literals to distinguish between various flags.

Example:

src/BlackholePositionManagerFeeManager.sol
590if (!_getFlag(6)) { // if skipExtra == false
591    extraFees = extraRewardToken.balanceOf(address(this)) - startingBalanceExtra;
592    if (extraFees > 0) {
593        if (_getFlag(2)) { // rewardExtraIsPoolToken
594            if (_getFlag(3)) { // rewardExtraIsToken0

Recommendation:

We advise a custom data type to be utilized instead, permitting function overloading to increase its legibility.

Additionally, we advise all states to be signified via enum declarations, ensuring that the library can be consistently re-used across the Steer Protocol codebase legibly.

Alleviation (aafd869b2be09d232f2bbb4bdcbb3479d829f6ec):

The Steer Protocol team evaluated this exhibit but opted to not apply it due to bytecode size constraints.

BPM-04C: Ineffectual Usage of Safe Arithmetics

Description:

The linked mathematical operation is guaranteed to be performed safely by logical inference, such as surrounding conditionals evaluated in require checks or if-else constructs.

Example:

src/BlackholePositionManagerFeeManager.sol
199uint256 originalUserShares = balanceOf(msg.sender);
200

201WithdrawCache memory cache;
202

203updateRewards();
204

205(rewards, extraRewards) = pullRewards(msg.sender);
206

207cache.preBurnSupply = totalSupply();
208

209(amount0, amount1) = _calculateWithdrawAmounts(
210    shares,
211    cache.preBurnSupply
212);
213

214if (amount0 < amount0Min) revert WithdrawTooLittleToken0();
215if (amount1 < amount1Min) revert WithdrawTooLittleToken1();
216

217_burn(msg.sender, shares);
218

219uint256 remainingShares = originalUserShares - shares;

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 (aafd869b2be09d232f2bbb4bdcbb3479d829f6ec):

The referenced operation has been properly wrapped in an unchecked code block, optimizing the operation's gas cost.

BPM-05C: Inefficient Disbursement of Fees

Description:

The referenced loops will iterate the same feesInfo data point twice redundantly.

Example:

src/BlackholePositionManagerFeeManager.sol
590if (!_getFlag(6)) { // if skipExtra == false
591    extraFees = extraRewardToken.balanceOf(address(this)) - startingBalanceExtra;
592    if (extraFees > 0) {
593        if (_getFlag(2)) { // rewardExtraIsPoolToken
594            if (_getFlag(3)) { // rewardExtraIsToken0
595                rewardBal0 += extraFees;
596            } else {
597                rewardBal1 += extraFees;
598            }
599        }
600        totalCutExtra = FullMath.mulDiv(extraFees, totalFees, DIVISOR);
601        uint256 netGainExtra = extraFees - totalCutExtra;
602        reservedExtraFee += totalCutExtra;
603        accumulatedExtraRewardsPerShareP += ((netGainExtra * PRECISION)/
604            totalSupply);
605        if (totalCutExtra > 0) {
606            for (uint256 j; j != feesInfo.length; ++j) {
607                accruedFees1[feesInfo[j].feeIdentifier] = accruedFees1[
608                    feesInfo[j].feeIdentifier
609                ] + (FullMath.mulDiv(totalCutExtra, feesInfo[j].feeValue, DIVISOR));
610            }
611        }
612    }
613}
614

615uint256 fees = baseRewardToken.balanceOf(address(this)) - startingBalance;
616if (fees > 0) {
617    if (_getFlag(0)) { // rewardIsPoolToken
618        if (_getFlag(1)) { // rewardIsToken0
619            rewardBal0 += fees;
620        } else {
621            rewardBal1 += fees;
622        }
623    }
624    totalCutBase = FullMath.mulDiv(fees, totalFees, DIVISOR);
625    uint256 netGain = fees - totalCutBase;
626    accumulatedRewardsPerShareP += ((netGain * PRECISION) /
627        totalSupply);
628    reservedBaseFee += totalCutBase;
629    if (totalCutBase > 0 ) {
630        for (uint256 j; j != feesInfo.length; ++j) {
631            accruedFees0[feesInfo[j].feeIdentifier] = accruedFees0[
632                feesInfo[j].feeIdentifier
633            ] + (FullMath.mulDiv(totalCutBase, feesInfo[j].feeValue, DIVISOR));
634        }
635    }
636}

Recommendation:

We advise a single loop to be performed that disburses the relevant fees at the end of the function, optimizing its gas cost.

Alleviation (aafd869b2be09d232f2bbb4bdcbb3479d829f6ec):

Fee disbursement has been bundled into a single for loop as recommended greatly reducing the code's gas cost.

BPM-06C: Inexistent Error Messages

TypeSeverityLocation
Code StyleBlackholePositionManagerFeeManager.sol:
• I-1: L1135
• I-2: L1136
• I-3: L1147
• I-4: L1148
• I-5: L1187

Description:

The linked require checks have no error messages explicitly defined.

Example:

src/BlackholePositionManagerFeeManager.sol
1135require(msg.sender == address(pool));

Recommendation:

We advise each to be set so to increase the legibility of the codebase and aid in validating the require checks' conditions.

Alleviation (aafd869b2be09d232f2bbb4bdcbb3479d829f6ec):

All referenced require checks have been updated to if-revert checks, addressing this exhibit.

BPM-07C: Redundant Data Point Propagation

Description:

The swapAmount data point propagated from the BlackholePositionManager::tend function to the BlackholePositionManager::_mintPositions function is redundant as it remains unused.

Example:

src/BlackholePositionManagerFeeManager.sol
850function _setBins(
851    uint160 sqrtPriceX96,
852    uint256 t0ToDeposit,
853    uint256 t1ToDeposit,
854    int256 swapAmount
855) internal {
856    // Pack parameters into a struct to reduce stack variables
857    MintParams memory params = MintParams({
858        sqrtPriceX96: sqrtPriceX96,
859        t0ToDeposit: t0ToDeposit,
860        t1ToDeposit: t1ToDeposit,
861        swapAmount: swapAmount
862    });
863    // Process positions and calculate requested amounts
864    PositionData memory posData = _calculatePositionRequests(
865        params.sqrtPriceX96
866    );
867

868    // Now add liquidity to those bins based on their weights vs total token weights
869    _mintPositions(posData, params);
870}

Recommendation:

We advise it to be omitted, optimizing the code's gas cost.

Alleviation (aafd869b2be09d232f2bbb4bdcbb3479d829f6ec):

The redundant data point has been omitted from the overall call flow, addressing this exhibit.

BPM-08C: Redundant Flag Re-Evaluation

Description:

The referenced flag validation is redundant as a non-zero extraRewards value guarantees that the 6th bit of the flag has been set to true.

Example:

src/BlackholePositionManagerFeeManager.sol
333if (!_getFlag(6) && extraRewards > 0) {

Recommendation:

We consider the flag's validation to be omitted, optimizing the code's gas cost.

Alleviation (bf4fc8ec2d63040ec62fb9ee839950cbf29a8b19):

The flag evaluation has been omitted per our recommendation, optimizing the code's gas cost.

BPM-09C: Redundant Parenthesis Statements

TypeSeverityLocation
Code StyleBlackholePositionManagerFeeManager.sol:
• I-1: L603-L604
• I-2: L656
• I-3: L661
• I-4: L776
• I-5: L1195
• I-6: L1210

Description:

The referenced statements are redundantly wrapped in parenthesis' (()).

Example:

src/BlackholePositionManagerFeeManager.sol
603accumulatedExtraRewardsPerShareP += ((netGainExtra * PRECISION)/
604    totalSupply);

Recommendation:

We advise them to be safely omitted, increasing the legibility of the codebase.

Alleviation (aafd869b2be09d232f2bbb4bdcbb3479d829f6ec):

The redundant parenthesis in the referenced statements have been safely omitted.

BPM-10C: Repetitive Value Literals

TypeSeverityLocation
Code StyleBlackholePositionManagerFeeManager.sol:
• I-1: L981
• I-2: L987
• I-3: L1239

Description:

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

Example:

src/BlackholePositionManagerFeeManager.sol
981if (_getBalance1() - expectedLeftOver1 >= (t1ToDeposit * 5) / 100) revert Imbalance1();

Recommendation:

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

In case some of the constant declarations have already been introduced, we advise them to be properly re-used across the code.

Alleviation (aafd869b2be09d232f2bbb4bdcbb3479d829f6ec):

The referenced value literals 5, 100, and 1500 have all been relocated to contract-level constant declarations labelled FIVE`, `HUNDRED, and TOTAL_FEE respectively, optimizing the code's legibility.

BPM-11C: Simplification of Condition

Description:

The referenced condition can be satisfied solely through the totalWeight == 0 condition.

Example:

src/BlackholePositionManagerFeeManager.sol
727// There are 4 conditions of position and weight inputs:
728// -    defined positions,   totalWeight > 0    = Normal functionality
729// -    undefined positions, totalWeight > 0    = Previous positions are used
730// -    defined positions,   totalWeight = 0    = Empty positions, no liquidity deployed
731// -    undefined positions, totalWeight = 0    = Empty positions, no liquidity deployed
732

733// 5. Handle position migration and deletion based on conditions
734if (newPositions.lowerTick.length > 0 && totalWeight > 0) {
735    // Only migrate if new positions are provided AND will be used
736    migratePositions(newPositions);
737} else if (newPositions.lowerTick.length > 0 || totalWeight == 0) {
738    // Delete positions if new positions provided (but not used) OR totalWeight is 0
739    delete opPositions;
740}

Recommendation:

We advise it to be done so, adhering to the condition cases that precede it more accurately.

Alleviation (aafd869b2be09d232f2bbb4bdcbb3479d829f6ec):

The Steer Protocol team evaluated this exhibit but opted to not apply it due to bytecode size constraints.

BPM-12C: Unused Function Implementation

Description:

The referenced function remains unused in the codebase.

Example:

src/BlackholePositionManagerFeeManager.sol
353function pullRewards(address user) internal returns (uint256 rewards, uint256 extraRewards) {

Recommendation:

We advise it to be omitted.

Alleviation (bf4fc8ec2d63040ec62fb9ee839950cbf29a8b19):

The unused function implementation has been safely removed from the codebase as advised.

BPM-13C: Unutilized Constant Declaration

Description:

The referenced constant declaration remains unused within the codebase.

Example:

src/BlackholePositionManagerFeeManager.sol
73uint256 public constant TOTAL_FEE = 15_00;

Recommendation:

We advise it to be omitted, optimizing the code's style.

Alleviation (aafd869b2be09d232f2bbb4bdcbb3479d829f6ec):

The referenced constant is now properly utilized to address exhibit BPM-09C, addressing this exhibit indirectly.

Prev

BlackholePositionManagerFeeManager.sol (BPM-M)

Next

Finding Types

On this page