Fixed Supply

The MarketMaking contract verifies if the protocol can bump by simulating an increase in the blvTick, and later validating some conditions, like bumpedCapacity > circulating.

The bumpedAnchorCapacity is calculated based on the _getAnchorReserves. However, the bumpedFloorCapacity is mistakenly uses the remainingReserves as follows:

int256 remainingReserves = _getVirtualReserves() + reserve.balanceOf(address(BPOOL));

However, the balance of the BPOOL contains all reserves, as they were all removed from the ranges, so remainingReserves is actually equal to totalReserves.

Subtract the reserves used to calculate the ANCHOR range to correctly determine how many reserves remain.

Baseline Team: The issue was fixed in line MarketMaking.sol#L354.

Proof of concept: PoC

Deploying with vm.startBroadcast() will lead to each call happening as a separate transaction. This gives a user who was distributed bTokens the opportunity to interact with the Uniswap pool prior to the first rebalance occurring.

A malicious user can deploy liquidity below the desired BLV. Then, they can swap to their new deployed liquidity range.

This will mean the active tick is below the BLV tick. When rebalance() is invoked, it will attempt to set the lower tick of the anchor range to the BLV tick and the upper tick will be calculated using the current active tick.

Since the current active tick is below the BLV tick, setTicks() will revert due to InvalidTickRange. This will DoS the deployment after the tokens are distributed and the contract has been deployed.

Deploy inside a smart contract function, so that the deployment happens atomically.

Baseline Team: Acknowledged.

The Interest accrued from the extend function sits in the CreditFacility contract until the fee recipient removes it with their approval.

However this poses an issue because the _swapExactOut function transfers the entire contract balance of the CreditFacility to the BPOOL contract.

As a result these fee amounts which are sitting in the CreditFacility contract will be deployed into the protocol liquidity instead of collectable.

In the extend function, instead of transferring the reserve amount from the user to the CreditFacility contract, transfer the reserve amount to the fee recipient directly. Furthermore, be sure there are no other instances where reserves are left in the CreditFacility contract.

Baseline Team: The issue was fixed in line CreditFacility.sol#L437.

The CreditFacility _sendReserves function used to keep the interest reserves amount in the CreditFacility contract so that this amount could be removed by the fee receiver which is approved for the CreditFacility.

However now, because the removeAllFrom function leaves all removed tokens in the BPOOL contract, the interest amount is not collected by the protocol and will instead be deployed back into the liquidity structure.

Use BPOOL.transferToken(reserve, feeRecipient, _interest); in the _sendReserves function. Additionally, remove the feeRecipient approval logic as it is no longer necessary.

Baseline Team: The issue was fixed in line CreditFacility.sol#L548.

The _tradingInFloor functions in Policies verify if the active tick is at or below the floor's upper tick. However, in Brouter, this function only contains <, so the check will succeed when activeTick == tickU.

Update the operator to <= so the call reverts when active tick is exactly at the BLV.

Baseline Team: Acknowledged.

The canBump function uses an outdated anchorTick which has not been updated to reflect the current price which the protocol is rebalancing for.

As a result the capacity calculations for the Anchor range are not accurate to what the capacity will actually be after the rebalance.

This will often result in bumping when bumps should not occur, which will often prevent a rebalance from occurring since the final capacity invariant cannot be held. Or, more rarely, not allowing bumps to occur when they ought to be.

Consider updating the anchorTick to the latest that will be used in the rebalance.

Baseline Team: Resolved.

The MarketMaking policy is mainly in charged of rebalancing the liquidity positions, when canRebalance is true.

Normally, a rebalance will be triggered when price moved outside of the rebalance ticks and its within a certain range:

bool isWithinRange = activeTick > blvTick & activeTick < anchorTick + DISCOVERY_LENGTH;

However, the DISCOVERY liquidity will range from the anchorTick to the MAX_TICK. This suggests that rebalance is not possible when price is inside the DISCOVERY but above anchorTick + DISCOVERY_LENGTH.

Update the withinRange to include all the DISCOVERY range so rebalance is possible when price is at that range:

bool isWithinRange = activeTick > blvTick & activeTick < MAX_TICK;

Baseline Team: The issue was fixed in line MarketMaking.sol#L201.

The isEth validation checks the balance of the contract instead of the msg.value. A malicious user can send ether to the Brouter, in order to trigger a refund to LoopFacility when openPosition() is called.

Since LoopFacility does not have a receive() function, this will cause a call to openPosition() to revert. The malicious user can then pull their ether out through a swap in the following transaction.

Change the validation for isEth from checking the contract balance to checking the msg.value.

Baseline Team: The issue was fixed in line Brouter.sol#L242.

The CreditFacility allows users to borrow reserves with bAssets as collateral. These reserves will be retrieved from the Liquidity Ranges.

If there are not enough reserves in the FLOOR, the excess amount will be taken from the ANCHOR. However, when users repay reserves, these will be added to the FLOOR only.

Due to the fact that there is no interest charged, users can deliberately borrow enough reserves to remove some from the ANCHOR range and immediately repay.

This will increase the capacity of the system as the reserves that were in the ANCHOR had lower capacity than if they are valued at the blv. Additionally, this may open arbitrage opportunities, as well as DoS user actions, as liquidity in the trading range is moved down.

Set interest greater than zero, to disincentivize whales from manipulating the reserves and capacity. Additionally, consider triggering a rebalance, if possible, to re distribute the reserves where they belong.

Baseline Team: Acknowledged.

When trading in the floor, liquidity is removed from the floor during a buy in order to improve price movement.

However, an external user can provide liquidity in the floor range, making the trade still revert. This revert will prevent swaps that are bringing the price closer to the BLV from executing.

Instead, validate that the price has moved close to the BLV and check that the price is not in the floor for closePosition().

Baseline Team: The issue was fixed in line Brouter.sol#L272.

In the _getACU function when the active pool price is just above the blvTick there will be an overflow when attempting to cast the result of FullMath.mulDiv(amount1, FixedPoint96.Q96, sqrtRatioBX96 - sqrtRatioAX96) to a uint128 variable inside of the getAmount0ForLiquidity function.

As a result rebalances are DoS'd when the pool price is in this edge case range.

Be aware of this DoS and consider refactoring the leverage calculations to avoid calculating the ACU when the price is close to the blvTick and instead returning a default asymptotic value.

Baseline Team: Resolved.

In the closePosition function the exactInputSingle function on the BRouter contract is used which rebalances before and after the swap.

This does not match the previous behavior of the closePosition function and can prevent users from repaying their debts because the rebalance function may revert with an BackingInsolvent error.

Use the exactInputSingleVanilla function instead.

Baseline Team: The issue was fixed in line LoopFacility.sol#L175.

In the removeAllFrom the donated liquidity that was not deployed by the protocol is intended to be transferred to the fee recipient.

However in the case where the ranges are updated and third party liquidity is found in the new ranges, the following early return is used in the removeAllFrom function:

liquidityToRemove = currentLiquidity; if (liquidityToRemove = 0) return (0,
bAssetFees_, 0, reserveFees_);

In this case the transfers at the end of the removeAllFrom function are not made.

reserve.safeTransfer(feeRecipient, reserveFees_); bAsset.transfer(feeRecipient,
bAssetFees_);

Inside the early return case, be sure to make the same transfers to the feeRecipient.

Baseline Team: The issue was fixed in line BPOOL.v1.sol#L218.

During the liquidity deployment phase of rebalancing, DISCOVERY liquidity is added first, followed by ANCHOR liquidity. The threshold liquidity added to DISCOVERY is the minimum of multiple calculations: uint256(threshold_).min(uint256(bTokenLiquidityMax)).

When the final threshold liquidity is bTokenLiquidityMax, the entire bAsset balance of the BPOOL will be deployed to the DISCOVERY range during deployLiquidityTo(DISCOVERY), as bTokenLiquidityMax is calculated using balanceOf(BPOOL).

However, adding liquidity to ANCHOR in the next step also requires some bAssets when activeTick < anchorTick. Since the entire bAsset balance has already been deployed to the DISCOVERY range, addReservesTo(ANCHOR) fails during uniswapV3MintCallback due to insufficient balance.

Consider leaving a buffer amount when calculating bTokenLiquidityMax instead of using the entire balance. This ensures that the contract retains bAssets to add to ANCHOR, even when bTokenLiquidityMax is deployed to DISCOVERY.

Baseline Team: The issue was fixed in line MarketMaking.sol#L472.

The following code is not used in the current implementation:

• LoopFacility._tradingInFloor()
• BlastClaimer, IUniswapV3Pool, FixedPoint96 import in LOOPS
• debug code (console2.sol, PoolViewerLib.sol)

Remove the unused code or add an implementation for it.

Baseline Team: Resolved.

_swap() sends ether to the user via call(), which will hand over the execution flow to the receive() function if it is a smart contract. It is best practice to use reentrancy modifiers when this occurs.

Add reentrancy guard modifiers to the swap functions.

Baseline Team: Acknowledged.

_swap() does not validate the fee tier that is passed for a swap. This allows users to perform swaps with pools that have been created with the same tokens but set to different fee tiers.

Validate the fee of the trade in the _swap() function.

Baseline Team: The issue was fixed in line Brouter.sol#L234.

The _removeLiquidity contains a call to bAsset.balanceOf(address(BPOOL)); whose return value is not used.

Remove the balanceOf call.

Baseline Team: Resolved.

The BPOOL contract will always own the reserves and bAssets used to add liquidity to ranges, so in the uniswapV3MintCallback these tokens are transferred directly to the pool.

Therefore, there is no need to encode the payer or msg.sender during pool.mint as this encoded data is not longer used.

Send empty data in the last param in pool.mint

Baseline Team: Resolved.

The rebalance action can now be executed when more than 8 hours have passed since the last rebalance, bypassing the other price and range checks.

This allows the ANCHOR range to disappear when the following scenarios are met:

• activeTick = blvTick
• (activeTick < blvTick + 199) & (liquidityA > _getThresholdLiquidity())

Although the first scenario might be expected, the second one might not, as it will create a DISCOVERY position with reserves.

Consider if this is the expected behavior and prevent rebalances to occur.

Baseline Team: Resolved.

The onlyKernel modifier prevents external calls to certain functions when Modules are installed or Policies are activated.

However, this modifier is only used in certain cases, leaving some unprotected functions, like the configureDependencies in Policies, that can lead to unexpected scenarios.

Consider adding the onlyKernel modifier to the all Module and Policies functions that should only be called by the Kernel contract.

Baseline Team: Resolved.

The CREDT module still contains a _burnDefaultedCollateral, but the bAssets are not burned anymore. Instead, they are transferred to the BPOOL module to be used for the next liquidity rebalance. Although the function does not actually burn, it can be misleading, as well as its natspec.

Update the _burnDefaultedCollateral function name as well as the comments, and avoid suggesting a burn

Baseline Team: Acknowledged.

The rebalance operation, after removing liquidity, will try to check if the current liquidity structure accepts a bump, which relies on certain conditions being met at the same time.

One of these conditions is tickDelta > BUMPABLE_PREMIUM which prevents bumps if the tick premium (difference between the activeTick and the blvTick) is greater than 1500 (default value).

Therefore, to save gas and avoid more calculations, the function should early return with false if this condition is not met.

Early return false if tickDelta = BUMPABLE_PREMIUM

Baseline Team: The issue was fixed in line MarketMaking.sol#L335.

Both BPOOL.getBaselineValue, LoopFacility.getBaselineValue() and MarketMaking.getBLV() calculate the current baseline value price based on the upper tick of the floor.

Although they all return the same value, this can lead to issues in the future if one is updated but not the others.

Consider having one single source of truth for the blv calculation.

Baseline Team: Acknowledged.

The Brouter performs swaps in the UniswapV3Pool, used by some Policies. The issue arises when using block.timestamp as the deadline parameter for these swaps.

A malicious block builder will be able to execute this at any time, when such transaction is useful for manipulating the price.

Add an optional deadline parameter to functions that routes swaps through the Brouter and use this instead of block.timestamp for all the swaps.

Baseline Team: Acknowledged.

The getBaselineValue calculates the BToken price at upper tick of the FLOOR range. However, the documentation mentions Returns the price at the lower tick of the floor position, which is misleading.

Update the documentation to: Returns the price at the upper tick of the floor position

Baseline Team: The issue was fixed in line BPOOL.v1.sol#L271.

The DISCOVERY_LENGTH value is assignable by the owner address and is used in canBump but actually has nothing to do with the length of the discovery range as it is hardcoded to the max tick.

Consider either removing the DISCOVERY_LENGTH variable or making the Discovery range configurable by it.

Baseline Team: Resolved.

In the _canBump function there is validation to check if the circulating supply can be absorbed by the total reserves immediately after increasing the blvTick.

if (totalReserves < circulating.mulWad(getBLV())) {blvTick = T_S; return false;}

This validation multiplies the circulating supply by the BLV price using mulWad, which rounds down. This does not round in the protocol's favor as it is rounding down the circulating value that the reserves must cover.

This is in contrast to the same validation which is performed differently in the _removeLiquidity function.

     uint256 maxCapacity = totalReserves.divWad(getBLV());
if (maxCapacity < circulating) {revert BackingInsolvent();}

In the _removeLiquidity function the maxCapacity is exposed to rounding down because it is the totalReserves divided by the BLV price with divWad which rounds down.

This is the correct way to perform this validation which rounds in favor of being more conservative about the capacity invariant.

Use the same validation as is performed in the _removeLiquidity which rounds conservatively.

Baseline Team: Resolved.

Since the Discovery liquidity is now deployed to the max tick and the circulating supply is now limited, the liquidity achievable in the Discovery range will be somewhat limited.

In many cases this is will result in the capping of the threshold liquidity, which may give resistance to deploying the liquidity structure that is desired.

Be aware of this constraint and be prepared to adjust the discovery range as needed if outcomes are not as desired.

Baseline Team: Acknowledged.

The exactInputSingleVanilla function does not have the payable keyword, unlike the other three functions, and therefore cannot perform swaps with the native token.

Add payable to this function as well.

Baseline Team: The issue was fixed in line Brouter.sol#L126.

The MarketMaking contract grants approval to BPOOL to spend reserve tokens. However, this approval is unnecessary, as BPOOL no longer invokes transferFrom after the updates.

Remove floating allowances.

Baseline Team: The issue was fixed in line MarketMaking.sol#L115.

In the _getLeverage function the leverage is multiplied by 0.9999 to avoid any rounding up edge cases. However this allows the leverage to be lower than 1x, which may be unexpected.

Consider enforcing a minimum value of 1e18 for the leverage result.

Baseline Team: The issue was fixed in line MarketMaking.sol#L509.

The comment related to spot supply invariant check at lines 410–411 of the MarketMaking contract still persists, even though the line uint256 spotSupply = getCirculatingSupply() - LOOPS.totalCollateral() - CREDT.totalCollateralized() has been removed.

Remove the unnecessary comment.

Baseline Team: The issue was fixed in line MarketMaking.sol#L407.

The BToken controller is initialized with the BPOOL address during deployment, which is also set as the controller address.

The BToken.setController has an access control validation so only the current controller address (BPOOL) can update it.

However, the only function in BPOOL that can update the controller is the migrateBToken, which is permissioned and there is no Policy that has the function permission.

Consider adding migrateBToken permission to the policy that should be allowed to call this function.

Baseline Team: Acknowledged.