Fira - Lending Market

In LiquidityInjector.injectLiquidity, the return value of lendingMarket.supply is checked to ensure assetsSupplied == amountToMintAndInject:

(uint256 assetsSupplied,) = lendingMarket.supply(params, amountToMintAndInject, 0, address(this), "");
require(assetsSupplied == amountToMintAndInject, "LI: partial inject");

This check is unnecessary. When LendingMarket.supply is called with a non-zero assets argument and shares == 0, the function computes shares from the given assets but never modifies the assets value itself. The returned assets is always equal to the input assets parameter:

if (assets > 0) {
    shares = assets.toSharesDown(market[id].totalSupplyAssets, market[id].totalSupplyShares);
} else {
    assets = shares.toAssetsUp(market[id].totalSupplyAssets, market[id].totalSupplyShares);
}
// ...
return (assets, shares);

Since injectLiquidity always passes amountToMintAndInject as the assets argument and 0 as shares, the assets > 0 branch is taken and assets is returned unchanged. The require statement can therefore never fail.

A similar pattern exists in LiquidityInjector.withdrawLiquidity, where the return value of lendingMarket.withdraw is checked in the same manner. The same reasoning applies: when LendingMarket.withdraw is called with a non-zero assets argument, the returned value is identical to the input.

Low. The check wastes gas but has no functional consequence.

Remove the redundant checks in both injectLiquidity and withdrawLiquidity:

- (uint256 assetsSupplied,) = lendingMarket.supply(params, amountToMintAndInject, 0, address(this), "");
- require(assetsSupplied == amountToMintAndInject, "LI: partial inject");
+ lendingMarket.supply(params, amountToMintAndInject, 0, address(this), "");

- (uint256 assetsWithdrawn,) =
-     lendingMarket.withdraw(params, amountToWithdrawAndBurn, 0, address(this), address(this));
- require(assetsWithdrawn == amountToWithdrawAndBurn, "LI: partial withdraw");
+ (uint256 assetsWithdrawn,) =
+     lendingMarket.withdraw(params, amountToWithdrawAndBurn, 0, address(this), address(this));

Note that in withdrawLiquidity, the assetsWithdrawn variable is still needed for the subsequent burnByLI call, so it should be retained but the require can be removed.

Acknowledged.

FWBase.previewDeposit() and FWBase.previewRedeem() ignore depositsPaused and redemptionsPaused. Unlike deposit() and redeem(), they still return quotes while the corresponding action is paused.

Informational. This is a low-severity UX/integration issue. Frontends or off-chain systems may show valid previews for actions that will revert when executed.

Make previews respect the same pause checks as execution, or clearly document that preview functions do not reflect pause state.

Acknowledged.

The CouponToken contract defines a notExpired modifier that reverts with Errors.YCExpired() when the contract has passed its expiry timestamp:

However, this modifier is not applied to any function in the CouponToken contract. By contrast, the analogous notExpired modifier in FiraMarket is actively used to guard swapExactBTForFw, swapFwForExactBT, and addLiquidity.

Informational. The unused modifier represents dead code.

Remove the unused notExpired modifier to reduce code size and avoid confusion.

Acknowledged, not worth redeploying for now.

The supplyAndBorrowSingleToken and borrowSingleToken functions accept a LimitOrderData calldata limitOrderData parameter that is documented as disabled in the current version:

"LimitOrderData is kept for interface compatibility but orderbook is not used in this version."

However, neither function validates that the provided limitOrderData is actually empty. If a caller supplies non-empty limitOrderData, the code will route the borrowed BT through _fillLimit, which calls fill() on the user-supplied limitRouter address and grants it token approvals via _safeApproveInf.

This contradicts the documented invariant that limit orders are disabled and creates a discrepancy between intended and actual behavior.

Informational.

Add a guard at the top of both functions to enforce the documented invariant, reusing the existing _isEmptyLimit helper:

 function supplyAndBorrowSingleToken(
     MarketParams memory marketParams,
     ILendingMarket lendingMarket,
     IPMarketV3 firaMarket,
     uint256 collateralAmount,
     uint256 tokensToBorrow,
     TokenOutput calldata output,
     LimitOrderData calldata limitOrderData,
     address receiver
 ) external returns (uint256 btBorrowed, uint256 tokenBorrowed) {
+    require(_isEmptyLimit(limitOrderData), "AB: limit orders disabled");
     require(collateralAmount > 0, "AB: zero collateral");

Apply the same guard to borrowSingleToken.

Acknowledged, we have no order book so shall be good.

Both ActionBorrow.supplyAndBorrowSingleToken and ActionBorrow.borrowSingleToken redeem FW tokens to the router contract itself via _redeemFwToToken, then perform a separate _transferOut to send the output tokens to the receiver:

tokenBorrowed = _redeemFwToToken(address(this), address(FW), fwOut, output, false);
_transferOut(output.tokenOut, receiver, tokenBorrowed);

The _redeemFwToToken function in ActionBase already supports sending tokens directly to an arbitrary receiver. When SwapType.NONE is used, it calls __redeemFw(receiver, ...) which redeems directly to the specified address. When SwapType.ETH_WETH or an aggregator swap is used, it performs the conversion and then calls _transferOut(out.tokenOut, receiver, netTokenOut) internally.

By passing receiver directly instead of address(this), the final _transferOut call in both borrow functions becomes unnecessary. This eliminates one external ERC20 transfer call per borrow operation.

Gas optimization.

Pass receiver directly to _redeemFwToToken and remove the subsequent _transferOut call in both functions:

-   tokenBorrowed = _redeemFwToToken(address(this), address(FW), fwOut, output, false);
-   // 5. Transfer the borrowed token to receiver
-   _transferOut(output.tokenOut, receiver, tokenBorrowed);
+   tokenBorrowed = _redeemFwToToken(receiver, address(FW), fwOut, output, false);

Acknowledged.