Market Maker Looping, Round 2

Proof of concept: PoC

The removeAllFrom function in the BPOOLv1 contract is intended to handle any unexpected liquidity (i.e., donations) by removing it from the position and then accounting for both the liquidity amount and its fees in the total bAssetFees_, which is subsequently sent to the fee recipient.

However, the current implementation does not behave as intended. Instead, the donated amount is burned in the internal removeLiquidity function while that same burned amount is also counted in bAssetFees.

This discrepancy causes the MarketMaking contract’s bAsset balance to be lower than the total bAssetFees_, leading to a revert when fees are transferred.

Because of this revert, the rebalance function fails to execute, preventing the protocol from adjusting its liquidity to changing market conditions. Additionally, a malicious actor could intentionally exploit this flaw by donating, causing a DOS to the rebalance function.

Modify the implementation so that the donated liquidity is not burned and is correctly accounted for and transferred to the fee recipient.

Baseline Team: Resolved.

Proof of concept: PoC

The rebalance function removes the protocol-owned liquidity, updates ticks, and redeploys liquidity using new tick ranges. The _updateTicks logic determines the anchorTick based on whether the anchor liquidity is higher or lower than the discovery liquidity.

There are immediate arbitrage opportunities by using the rebalance functionality. When the anchor liquidity is higher than the discovery liquidity:

• The upper anchor tick is below the active tick, and the current price is within the discovery range.
• User can buy a large amount of bToken, pushing the price even higher.
• Call rebalance, which updates ticks and range liquidities.
• Sell the same amount of bToken at a higher average price due to higher anchor liquidity.

A similar arbitrage opportunity can occur in the opposite direction as well. When anchor liquidity is lower than discovery liquidity, the user can sell, rebalance, and buy back. This time, the user sells in a low-liquidity environment and buys back in a high-liquidity environment.

Consider rate limiting the amount of ticks that can be dropped at a time to limit the scale of this arbitrage vector.

Baseline Team: Partially Resolved by rate limiting the arbitrage.

After the updates, the blv is calculated based on the upper floor tick in the MarketMaking, CreditFacility, and LoopFacility contracts. However, in the BPOOL contract, it is still calculated using the lower floor tick.

The BaselineInit.launch function uses the baseline value from the BPOOL contract when calculating capacity, which creates a discrepancy.

Update the getBaselineValue function in BPOOL contract.

Baseline Team: Acknowledged.

In the _updateTicks function of the MarketMaking policy there is logic to alter the upper tick of the anchor position based upon the liquidity of the Discovery position relative to the Anchor position liquidity.

This is done to ideally prevent bAsset supply being minted in excess of the Discovery position liquidity in the range above the price in the Anchor position.

This reduces the magnitude of an arbitrage opportunity that would arise from selling through the Discovery range into the Anchor range and benefitting from the increased liquidity due to a higher leverage of the Anchor.

However in such an arbitrage scenario, the liquidity that the Anchor position should be compared against is the liquidity of the previous Discovery position rather than the liquidity of the new Discovery position.

This is because the old Discovery position is the one which is sold through to reach the new Anchor range and thus trigger the rebalance and therefore is the liquidity which the Arbitrage economics are based upon.

As a result the Anchor range upper tick handling should consider the liquidity of the old Discovery position rather than the current result of the _getThresholdLiquidity function, which will be the new Discovery position liquidity.

Consider comparing the predicted anchor liquidity against the minimum of both the result of the _getThresholdLiquidity and the old Discovery position liquidity to be the most conservative in limiting the arbitrage opportunities from selling through the Discovery position.

Baseline Team: Acknowledged.

The DISCOVERY_LENGTH variable in the MarketMaking contract is intended to represent 30 tick spacings, as indicated by the comment.

However, its current implementation as 30 x 200 only aligns with 1% fee pools. This means the calculation will be incorrect if applied to pools with different fee tiers.

If the protocol intends to only use 1% fee pools, no changes are needed. Otherwise, it should obtain the correct tick spacing from the BPOOLv1 contract instead.

Baseline Team: Acknowledged.

The MarketMaking contract requests permission for BPOOL.mint function. However, this function is not called from MarketMaking after updates, and the permission request can be removed.

Consider removing unnecessary permission request.

Baseline Team: Resolved.

NotOwner error in the MarketMaking contract is defined but never used.

Remove unused error.

Baseline Team: Resolved.

The canRebalance function retrieves the anchor range bounds using the _getTSBounds function. However, these ticks are not utilized in canRebalance, as rebalanceTicks are used to determine price movement.

Consider removing unused ticks and the _getTSBounds call.

Baseline Team: Resolved.

In the previous version of the contract, the active tick was required to be within the anchor range during configuration.

Currently, the MarketMaking.configureDependencies function still retrieves the lower and upper ticks of the anchor range.

However, unlike before, these ticks are no longer used for comparison against the active tick and remain unutilized.

If the active tick must be within the anchor range, compare it against the anchor range ticks. Otherwise, remove the BPOOL.getTicks(Range.ANCHOR) call from the function.

Baseline Team: Resolved.

BUMPABLE_PREMIUM is set to 1500, with a comment stating "1500 tick spacings". However, this value represents only 1500 ticks, not 1500 tick spacings. Based on the current setup, it corresponds to 7.5 tick spacings.

Update to comment to 1500 ticks.

Baseline Team: Resolved.

SafeCastLib library is imported in MarketMaking contract but never used.

Consider removing unused imports.

Baseline Team: Resolved.

The rebalance process removes liquidity from all existing ranges, checks for donations during this step, and accounts for them as fees before updating the ranges and re-adding liquidity.

However, when liquidity is added back to the new ranges, previously donated amounts in these new ranges are not accounted for.

As a result, in _deployLiquidity, if the discovery range had prior donations, the buffer amount and consequently the anchorReserves will differ from the values in _updateTicks, which were used to adjust the next rebalance ticks and anchorTick.

This discrepancy can lead to imbalances in the rebalancing logic, introducing inefficiencies in the system. For example, _updateTicks ensures that the anchor does not include the current price when anchor liquidity exceeds discovery liquidity.

However, a prior donation could cause this condition to be met, resulting in an extra minted supply and potentially creating an arbitrage opportunity for users, though its profitability may be limited.

Remove any unexpected extra liquidity as fees before adding liquidity to the new ranges to ensure consistency in rebalancing calculations.

Baseline Team: Acknowledged.

In very extreme ticks such as -557658 an attacker would only need 300 ETH to fill all available liquidity (liquidityGross). This would make it impossible to rebalance a position in this range.

Therefore, if the initial active tick is around this tick, an attacker could prevent BVL from ever increasing. The same issue can occur in higher ticks for bAsset, where a user can buy many bAssets when they have a low price and DOS a liquidity range in high ticks.

Making a barrie on how high discovery range can rise before hitting the maxed out liquidity. We would need approximately 1 billion bAssets to DOS the liquidity range between tick 317273 and 317273 - 200 for example.

We recommend the team to be aware that having a very low INITIAL_ACTIVE_TICK (such as

• 557658. would make it possible to either DOS a nearby liquidity position using ETH reserves or

make it possible for someone to gather enough bAssets to DOS a liquidity range in the higher tick ranges such as 317273, preventing the price to go further up.

Ultimately, putting a hardcoded limitation on the minimum INITIAL_ACTIVE_TICK would prevent this issue.

Baseline Team: Acknowledged.

The if statement on line 414 already implies that anchorReserves_ > totalReserves - _getVirtualReserves(). Therefore the use of min() is redundant.

We recommend refactoring line 415 by removing min() and setting anchorReserves_ to the difference:

anchorReserves_ = totalReserves - _getVirtualReserves());

Baseline Team: Resolved.

When swapping zeroToOne an edge scenario makes it so active tick and sqrtPrice can be misaligned, with tick being one less than what is should.

This could lead to rebalancing positions based on the wrong tick value. Specifically, using one tick lower than what it should (in relation to sqrtPrice). This ultimately makes it so the positions would be assigned to a lower range than what it should.

Although we could not find a scenario where this is harmful for the protocol, we believe it has potential to do so in unforeseen circumstances or with further development.

We recommend the Baseline team to be mindful of this scenario as they implement new features or change the current ones.

Baseline Team: Acknowledged.

When resetting the ticks for the ranges, it is possible for the anchor range to have the same value in the upper tick and lower tick. addReservesTo() will prevent a revert from happening by returning early, but this will lead to no liquidity deployed for the anchor range.

Consider shifting the upper and lower ticks if they are equal, so that the anchor range can be deployed.

Baseline Team: Acknowledged.

BPOOL.v1.sol and MarketMaking imports the console2 library. Console logging can be removed for production since it is unnecessary.

Remove the imports of the console2 library.

Baseline Team: Resolved.

The getCirculatingSupply view function in the MarketMaking contract does not account for collateral in the Loops facility which is burned with time passing.

However the corresponding getTotalCapacity function will account for the capacity change with respect to time resulting from the LOOPS.totalDebt().

This may mislead users and integrators and increases the risk of this function causing bugs in the future due to this undocumented behavior.

Consider adjusting the getCirculatingSupply function to account for the collateral in the Loops facility which is yet to be burnt.

Baseline Team: Acknowledged.