From 96791743e08637b119541e5930d193aeac29cda6 Mon Sep 17 00:00:00 2001 From: rick Date: Wed, 1 Apr 2026 16:00:31 +0800 Subject: [PATCH 01/13] feat: add SmartProvider support to BrokerLiquidator Add `redeemSmartCollateral` to allow BOT role to redeem smart collateral LP tokens via whitelisted SmartProviders. Add `batchSetSmartProviders` for MANAGER role to manage the provider whitelist. --- src/liquidator/BrokerLiquidator.sol | 3 +- test/liquidator/BrokerLiquidator.t.sol | 147 ++++++++++++++++++++ test/liquidator/mocks/MockSmartProvider.sol | 31 +++++ 3 files changed, 179 insertions(+), 2 deletions(-) create mode 100644 test/liquidator/BrokerLiquidator.t.sol create mode 100644 test/liquidator/mocks/MockSmartProvider.sol diff --git a/src/liquidator/BrokerLiquidator.sol b/src/liquidator/BrokerLiquidator.sol index ade01ed8..7bb4c192 100644 --- a/src/liquidator/BrokerLiquidator.sol +++ b/src/liquidator/BrokerLiquidator.sol @@ -8,7 +8,7 @@ import { SafeTransferLib } from "solady/utils/SafeTransferLib.sol"; import { MarketParamsLib } from "moolah/libraries/MarketParamsLib.sol"; import { IBroker, IBrokerBase } from "../broker/interfaces/IBroker.sol"; import { Id, MarketParams, IMoolah } from "moolah/interfaces/IMoolah.sol"; -import "./IBrokerLiquidator.sol"; +import { IBrokerLiquidator } from "./IBrokerLiquidator.sol"; import { ISmartProvider } from "../provider/interfaces/IProvider.sol"; contract BrokerLiquidator is UUPSUpgradeable, AccessControlUpgradeable, IBrokerLiquidator { @@ -342,7 +342,6 @@ contract BrokerLiquidator is UUPSUpgradeable, AccessControlUpgradeable, IBrokerL } /// @dev sets the smart collateral providers. - /// @dev allows the contract to receive native BNB, e.g. when redeeming slisBNB/BNB LP. /// @param providers The array of smart collateral providers. /// @param status The status of the providers. function batchSetSmartProviders(address[] calldata providers, bool status) external onlyRole(MANAGER) { diff --git a/test/liquidator/BrokerLiquidator.t.sol b/test/liquidator/BrokerLiquidator.t.sol new file mode 100644 index 00000000..74989a75 --- /dev/null +++ b/test/liquidator/BrokerLiquidator.t.sol @@ -0,0 +1,147 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.34; + +import "../moolah/BaseTest.sol"; + +import { BrokerLiquidator, IBrokerLiquidator } from "liquidator/BrokerLiquidator.sol"; +import { MarketParamsLib, MarketParams, Id } from "moolah/libraries/MarketParamsLib.sol"; +import { MockSmartProvider } from "./mocks/MockSmartProvider.sol"; + +contract BrokerLiquidatorTest is BaseTest { + using MarketParamsLib for MarketParams; + + BrokerLiquidator brokerLiquidator; + address BOT; + address MANAGER_ADDR; + MockSmartProvider smartProvider; + + function setUp() public override { + super.setUp(); + + BOT = makeAddr("Bot"); + MANAGER_ADDR = OWNER; + + BrokerLiquidator impl = new BrokerLiquidator(address(moolah)); + ERC1967Proxy proxy = new ERC1967Proxy( + address(impl), + abi.encodeWithSelector(impl.initialize.selector, OWNER, OWNER, BOT) + ); + brokerLiquidator = BrokerLiquidator(address(proxy)); + + smartProvider = new MockSmartProvider(address(loanToken), address(collateralToken)); + } + + // ==================== batchSetSmartProviders ==================== + + function testBatchSetSmartProviders() public { + address[] memory providers = new address[](2); + providers[0] = address(smartProvider); + providers[1] = makeAddr("Provider2"); + + vm.prank(MANAGER_ADDR); + brokerLiquidator.batchSetSmartProviders(providers, true); + + assertTrue(brokerLiquidator.smartProviders(address(smartProvider))); + assertTrue(brokerLiquidator.smartProviders(providers[1])); + } + + function testBatchSetSmartProvidersRemove() public { + address[] memory providers = new address[](1); + providers[0] = address(smartProvider); + + vm.prank(MANAGER_ADDR); + brokerLiquidator.batchSetSmartProviders(providers, true); + assertTrue(brokerLiquidator.smartProviders(address(smartProvider))); + + vm.prank(MANAGER_ADDR); + brokerLiquidator.batchSetSmartProviders(providers, false); + assertFalse(brokerLiquidator.smartProviders(address(smartProvider))); + } + + function testBatchSetSmartProvidersEmitsEvents() public { + address[] memory providers = new address[](2); + providers[0] = address(smartProvider); + providers[1] = makeAddr("Provider2"); + + vm.expectEmit(true, true, true, true); + emit BrokerLiquidator.SmartProvidersChanged(providers[0], true); + vm.expectEmit(true, true, true, true); + emit BrokerLiquidator.SmartProvidersChanged(providers[1], true); + + vm.prank(MANAGER_ADDR); + brokerLiquidator.batchSetSmartProviders(providers, true); + } + + function testBatchSetSmartProvidersRevertsIfNotManager() public { + address[] memory providers = new address[](1); + providers[0] = address(smartProvider); + + vm.prank(BOT); + vm.expectRevert(); + brokerLiquidator.batchSetSmartProviders(providers, true); + } + + function testBatchSetSmartProvidersEmpty() public { + address[] memory providers = new address[](0); + + vm.prank(MANAGER_ADDR); + brokerLiquidator.batchSetSmartProviders(providers, true); + // no revert, no-op + } + + // ==================== redeemSmartCollateral ==================== + + function testRedeemSmartCollateral() public { + // whitelist provider + address[] memory providers = new address[](1); + providers[0] = address(smartProvider); + vm.prank(MANAGER_ADDR); + brokerLiquidator.batchSetSmartProviders(providers, true); + + uint256 lpAmount = 1e18; + vm.prank(BOT); + (uint256 token0Out, uint256 token1Out) = brokerLiquidator.redeemSmartCollateral( + address(smartProvider), + lpAmount, + 0, + 0 + ); + + assertEq(token0Out, lpAmount / 2); + assertEq(token1Out, lpAmount / 2); + assertEq(loanToken.balanceOf(address(brokerLiquidator)), token0Out); + assertEq(collateralToken.balanceOf(address(brokerLiquidator)), token1Out); + } + + function testRedeemSmartCollateralRevertsIfNotWhitelisted() public { + vm.prank(BOT); + vm.expectRevert(BrokerLiquidator.NotWhitelisted.selector); + brokerLiquidator.redeemSmartCollateral(address(smartProvider), 1e18, 0, 0); + } + + function testRedeemSmartCollateralRevertsIfNotBot() public { + address[] memory providers = new address[](1); + providers[0] = address(smartProvider); + vm.prank(MANAGER_ADDR); + brokerLiquidator.batchSetSmartProviders(providers, true); + + vm.prank(MANAGER_ADDR); + vm.expectRevert(); + brokerLiquidator.redeemSmartCollateral(address(smartProvider), 1e18, 0, 0); + } + + function testRedeemSmartCollateralRevertsAfterProviderRemoved() public { + address[] memory providers = new address[](1); + providers[0] = address(smartProvider); + + vm.prank(MANAGER_ADDR); + brokerLiquidator.batchSetSmartProviders(providers, true); + + vm.prank(MANAGER_ADDR); + brokerLiquidator.batchSetSmartProviders(providers, false); + + vm.prank(BOT); + vm.expectRevert(BrokerLiquidator.NotWhitelisted.selector); + brokerLiquidator.redeemSmartCollateral(address(smartProvider), 1e18, 0, 0); + } +} diff --git a/test/liquidator/mocks/MockSmartProvider.sol b/test/liquidator/mocks/MockSmartProvider.sol new file mode 100644 index 00000000..ed9e6b12 --- /dev/null +++ b/test/liquidator/mocks/MockSmartProvider.sol @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.34; + +import "forge-std/Test.sol"; +import { IERC20 } from "@openzeppelin/contracts/interfaces/IERC20.sol"; + +contract MockSmartProvider is Test { + address public token0; + address public token1; + + constructor(address _token0, address _token1) { + token0 = _token0; + token1 = _token1; + } + + function redeemLpCollateral( + uint256 lpAmount, + uint256 minToken0Out, + uint256 minToken1Out + ) external returns (uint256 token0Out, uint256 token1Out) { + token0Out = lpAmount / 2; + token1Out = lpAmount / 2; + require(token0Out >= minToken0Out, "token0 slippage"); + require(token1Out >= minToken1Out, "token1 slippage"); + + deal(token0, address(this), token0Out); + deal(token1, address(this), token1Out); + IERC20(token0).transfer(msg.sender, token0Out); + IERC20(token1).transfer(msg.sender, token1Out); + } +} From 058bbf44f493fd4a2171c8425244fc7bf1276898 Mon Sep 17 00:00:00 2001 From: Razorback Date: Fri, 3 Apr 2026 13:09:49 +0800 Subject: [PATCH 02/13] feat: BrokerLiquidator support flash liqudate smart collateral --- src/liquidator/BrokerLiquidator.sol | 160 ++++++++++++++++ src/liquidator/IBrokerLiquidator.sol | 21 ++ test/broker/LendingBroker.t.sol | 200 +++++++++++++++++++- test/liquidator/BrokerLiquidator.t.sol | 2 +- test/liquidator/mocks/MockSmartProvider.sol | 29 ++- 5 files changed, 408 insertions(+), 4 deletions(-) diff --git a/src/liquidator/BrokerLiquidator.sol b/src/liquidator/BrokerLiquidator.sol index 7bb4c192..8cf00a93 100644 --- a/src/liquidator/BrokerLiquidator.sol +++ b/src/liquidator/BrokerLiquidator.sol @@ -47,6 +47,16 @@ contract BrokerLiquidator is UUPSUpgradeable, AccessControlUpgradeable, IBrokerL event PairWhitelistChanged(address pair, bool added); event SellToken(address pair, address tokenIn, address tokenOut, uint256 amountIn, uint256 amountOutMin); event SmartProvidersChanged(address provider, bool added); + event SmartLiquidation( + bytes32 indexed id, + address indexed lpToken, + address indexed collateralToken, + uint256 lpAmount, + uint256 minToken0Amt, + uint256 minToken1Amt, + uint256 amount0, + uint256 amount1 + ); /// @custom:oz-upgrades-unsafe-allow constructor /// @param moolah The address of the Moolah contract. @@ -70,6 +80,8 @@ contract BrokerLiquidator is UUPSUpgradeable, AccessControlUpgradeable, IBrokerL _grantRole(BOT, bot); } + receive() external payable {} + /// @dev withdraws ERC20 tokens. /// @param token The address of the token. /// @param amount The amount to withdraw. @@ -300,6 +312,123 @@ contract BrokerLiquidator is UUPSUpgradeable, AccessControlUpgradeable, IBrokerL ); } + /// @dev liquidates a position with smart collateral. + /// @param id The id of the market. + /// @param borrower The address of the borrower. + /// @param smartProvider The address of the smart collateral provider. + /// @param seizedAssets The amount of assets to seize. + /// @param repaidShares The amount of shares to repay. + /// @param payload The payload for the liquidation (min amounts for SmartProvider liquidation). + /// @return The actual seized assets and repaid assets. + function liquidateSmartCollateral( + bytes32 id, + address borrower, + address smartProvider, + uint256 seizedAssets, + uint256 repaidShares, + bytes memory payload + ) external onlyRole(BOT) returns (uint256, uint256) { + address broker = marketIdToBroker[id]; + require(broker != address(0), NotWhitelisted()); + require(_checkBrokerMarketId(broker, id), BrokerMarketIdMismatch()); + require(smartProviders[smartProvider], NotWhitelisted()); + MarketParams memory params = IMoolah(MOOLAH).idToMarketParams(Id.wrap(id)); + require(ISmartProvider(smartProvider).TOKEN() == params.collateralToken, "Invalid smart provider"); + address lpToken = ISmartProvider(smartProvider).dexLP(); + + uint256 collBalanceBefore = IERC20(params.collateralToken).balanceOf(address(this)); + (uint256 minAmount0, uint256 minAmount1) = abi.decode(payload, (uint256, uint256)); + IBrokerBase(broker).liquidate( + params, + borrower, + seizedAssets, + repaidShares, + abi.encode( + MoolahLiquidateData( + params.collateralToken, + params.loanToken, + seizedAssets, + address(0), + "", + false, + // --- below fields are only used for smart collateral liquidation callback --- + false, + address(0), + 0, + 0, + address(0), + address(0), + "", + "" + ) + ) + ); + uint256 collAmount = IERC20(params.collateralToken).balanceOf(address(this)) - collBalanceBefore; + require(collAmount > 0, "No collateral seized"); + + (uint256 amount0, uint256 amount1) = ISmartProvider(smartProvider).redeemLpCollateral( + collAmount, + minAmount0, + minAmount1 + ); + + emit SmartLiquidation(id, lpToken, params.collateralToken, collAmount, minAmount0, minAmount1, amount0, amount1); + return (seizedAssets, 0); + } + + /// @dev flash liquidates a position with smart collateral. + /// @param id The id of the market. + /// @param borrower The address of the borrower. + /// @param smartProvider The address of the smart collateral provider. + /// @param seizedAssets The amount of assets to seize. + /// @param token0Pair The address of the token0 pair. + /// @param token1Pair The address of the token1 pair. + /// @param swapToken0Data The swap data for token0 swapping to loan token. + /// @param swapToken1Data The swap data for token1 swapping to loan token. + /// @param payload The payload for the liquidation (min amounts for SmartProvider liquidation). + /// @return The actual seized assets and repaid assets. + function flashLiquidateSmartCollateral( + bytes32 id, + address borrower, + address smartProvider, + uint256 seizedAssets, + address token0Pair, + address token1Pair, + bytes calldata swapToken0Data, + bytes calldata swapToken1Data, + bytes memory payload + ) external onlyRole(BOT) returns (uint256, uint256) { + address broker = marketIdToBroker[id]; + require(broker != address(0), NotWhitelisted()); + require(_checkBrokerMarketId(broker, id), BrokerMarketIdMismatch()); + require(smartProviders[smartProvider], NotWhitelisted()); + require(pairWhitelist[token0Pair], NotWhitelisted()); + require(pairWhitelist[token1Pair], NotWhitelisted()); + MarketParams memory params = IMoolah(MOOLAH).idToMarketParams(Id.wrap(id)); + require(ISmartProvider(smartProvider).TOKEN() == params.collateralToken, "Invalid smart provider"); + (uint256 minAmount0, uint256 minAmount1) = abi.decode(payload, (uint256, uint256)); + + MoolahLiquidateData memory callback = MoolahLiquidateData( + params.collateralToken, + params.loanToken, + seizedAssets, + address(0), + "", + false, + true, + smartProvider, + minAmount0, + minAmount1, + token0Pair, + token1Pair, + swapToken0Data, + swapToken1Data + ); + + IBrokerBase(broker).liquidate(params, borrower, seizedAssets, 0, abi.encode(callback)); + return (seizedAssets, 0); + } + /// @dev the function will be called by the the Broker, when Broker's onMoolahLiquidate is called by Moolah. /// @param repaidAssets The amount of assets repaid. /// @param data The callback data. @@ -320,6 +449,37 @@ contract BrokerLiquidator is UUPSUpgradeable, AccessControlUpgradeable, IBrokerL if (out < repaidAssets) revert NoProfit(); SafeTransferLib.safeApprove(arb.collateralToken, arb.collateralPair, 0); + } else if (arb.swapSmartCollateral) { + uint256 before = SafeTransferLib.balanceOf(arb.loanToken, address(this)); + // redeem lp + (uint256 amount0, uint256 amount1) = ISmartProvider(arb.smartProvider).redeemLpCollateral( + arb.seized, + arb.minToken0Amt, + arb.minToken1Amt + ); + + address token0 = ISmartProvider(arb.smartProvider).token(0); + address token1 = ISmartProvider(arb.smartProvider).token(1); + + // swap token0 and token1 to loanToken if needed + if (amount0 > 0 && token0 != arb.loanToken) { + if (token0 != BNB_ADDRESS) SafeTransferLib.safeApprove(token0, arb.token0Pair, amount0); + uint256 _value = token0 == BNB_ADDRESS ? arb.minToken0Amt : 0; + (bool success, ) = arb.token0Pair.call{ value: _value }(arb.swapToken0Data); + require(success, SwapFailed()); + } + + if (amount1 > 0 && token1 != arb.loanToken) { + if (token1 != BNB_ADDRESS) SafeTransferLib.safeApprove(token1, arb.token1Pair, amount1); + uint256 _value = token1 == BNB_ADDRESS ? arb.minToken1Amt : 0; + (bool success, ) = arb.token1Pair.call{ value: _value }(arb.swapToken1Data); + require(success, SwapFailed()); + } + uint256 out = SafeTransferLib.balanceOf(arb.loanToken, address(this)) - before; + + if (out < repaidAssets) revert NoProfit(); + if (token0 != BNB_ADDRESS) SafeTransferLib.safeApprove(token0, arb.token0Pair, 0); + if (token1 != BNB_ADDRESS) SafeTransferLib.safeApprove(token1, arb.token1Pair, 0); } SafeTransferLib.safeApprove(arb.loanToken, msg.sender, repaidAssets); diff --git a/src/liquidator/IBrokerLiquidator.sol b/src/liquidator/IBrokerLiquidator.sol index c0175d36..d8a5ede7 100644 --- a/src/liquidator/IBrokerLiquidator.sol +++ b/src/liquidator/IBrokerLiquidator.sol @@ -29,6 +29,27 @@ interface IBrokerLiquidator { function liquidate(bytes32 id, address borrower, uint256 seizedAssets, uint256 repaidShares) external; + function liquidateSmartCollateral( + bytes32 id, + address borrower, + address smartProvider, + uint256 seizedAssets, + uint256 repaidShares, + bytes memory payload + ) external returns (uint256, uint256); + + function flashLiquidateSmartCollateral( + bytes32 id, + address borrower, + address smartProvider, + uint256 seizedAssets, + address token0Pair, + address token1Pair, + bytes calldata swapToken0Data, + bytes calldata swapToken1Data, + bytes memory payload + ) external returns (uint256, uint256); + function sellToken( address pair, address tokenIn, diff --git a/test/broker/LendingBroker.t.sol b/test/broker/LendingBroker.t.sol index daca1a0c..8537761a 100644 --- a/test/broker/LendingBroker.t.sol +++ b/test/broker/LendingBroker.t.sol @@ -31,8 +31,10 @@ import { SharesMathLib } from "moolah/libraries/SharesMathLib.sol"; import { MathLib, WAD } from "moolah/libraries/MathLib.sol"; import { UtilsLib } from "moolah/libraries/UtilsLib.sol"; import { IMoolahLiquidateCallback } from "../../src/moolah/interfaces/IMoolahCallbacks.sol"; -import { BrokerLiquidator } from "../../src/liquidator/BrokerLiquidator.sol"; +import { BrokerLiquidator, IBrokerLiquidator } from "../../src/liquidator/BrokerLiquidator.sol"; import { MockOneInch } from "../liquidator/mocks/MockOneInch.sol"; +import { MockSmartProvider } from "../liquidator/mocks/MockSmartProvider.sol"; +import { ISmartProvider } from "../../src/provider/interfaces/IProvider.sol"; import { ORACLE_PRICE_SCALE, LIQUIDATION_CURSOR, MAX_LIQUIDATION_INCENTIVE_FACTOR } from "moolah/libraries/ConstantsLib.sol"; contract LendingBrokerTest is Test { @@ -2155,6 +2157,202 @@ contract LendingBrokerTest is Test { vm.prank(MANAGER); vault.setLockBuffer(address(newBuffer)); } + + // ============================================= + // Smart LP liquidation tests + // ============================================= + + /// @dev Helper: deploy a MockSmartProvider whose collateralToken == BTCB + /// and whose underlying tokens are LISUSD (token0) and BTCB (token1). + function _setupSmartProvider() internal returns (MockSmartProvider sp, MockSwapPair swapPair) { + sp = new MockSmartProvider(address(LISUSD), address(BTCB)); + sp.setCollateralToken(address(BTCB)); + + // whitelist the smart provider + address[] memory providers = new address[](1); + providers[0] = address(sp); + vm.prank(MANAGER); + liquidator.batchSetSmartProviders(providers, true); + + // deploy a mock swap pair that converts BTCB -> LISUSD at oracle price + swapPair = new MockSwapPair(address(BTCB), address(LISUSD), oracle); + + // whitelist swap pair + vm.prank(MANAGER); + liquidator.setPairWhitelist(address(swapPair), true); + + // whitelist tokens + vm.prank(MANAGER); + liquidator.setTokenWhitelist(address(LISUSD), true); + vm.prank(MANAGER); + liquidator.setTokenWhitelist(address(BTCB), true); + } + + function test_liquidateSmartCollateral_seizes_and_redeems() public { + _prepareLiquidatablePosition(false); + + (MockSmartProvider sp, ) = _setupSmartProvider(); + + Position memory posBefore = moolah.position(marketParams.id(), borrower); + uint256 userRepayShares = BrokerMath.mulDivCeiling(posBefore.borrowShares, 1 * 1e8, 100 * 1e8); // 1% + + bytes memory payload = abi.encode(uint256(0), uint256(0)); // no slippage min + + // Fund liquidator with LISUSD for repayment (non-flash: liquidator pays upfront) + LISUSD.setBalance(address(liquidator), 1_000_000 ether); + + vm.prank(BOT); + liquidator.liquidateSmartCollateral(Id.unwrap(id), borrower, address(sp), 0, userRepayShares, payload); + + // After liquidation + LP redemption, the liquidator should hold redeemed tokens + // The smart provider splits into LISUSD (token0) and BTCB (token1) + uint256 lisusdAfter = LISUSD.balanceOf(address(liquidator)); + uint256 btcbAfter = BTCB.balanceOf(address(liquidator)); + // Liquidator should have some redeemed tokens (LISUSD from redemption + leftover, BTCB from redemption) + assertGt(lisusdAfter + btcbAfter, 0, "liquidator received no redeemed tokens"); + + // Borrower position should have decreased + Position memory posAfter = moolah.position(marketParams.id(), borrower); + assertLt(posAfter.borrowShares, posBefore.borrowShares, "borrow shares did not decrease"); + } + + function test_flashLiquidateSmartCollateral_swaps_and_repays() public { + _prepareLiquidatablePosition(false); + + (MockSmartProvider sp, MockSwapPair swapPair) = _setupSmartProvider(); + // Configure mock to return 0% as token0 (LISUSD), 100% as token1 (BTCB) + // so that the entire LP value is in BTCB and gets swapped to LISUSD at oracle price + sp.setToken0Bps(0); + + Position memory posBefore = moolah.position(marketParams.id(), borrower); + uint256 userRepayShares = BrokerMath.mulDivCeiling(posBefore.borrowShares, 1 * 1e8, 100 * 1e8); // 1% + uint256 seizedAssets = _previewLiquidationRepayment( + marketParams, + moolah.market(marketParams.id()), + 0, + userRepayShares, + moolah._getPrice(marketParams, borrower) + ); + + // token0 = LISUSD, token1 = BTCB + // With 0% token0Bps, all redeemed as BTCB, so only token1 needs swapping + bytes memory swapToken0Data = ""; // no token0 output + bytes memory swapToken1Data = abi.encodeWithSelector(MockSwapPair.swap.selector); + bytes memory payload = abi.encode(uint256(0), uint256(0)); + + vm.prank(BOT); + liquidator.flashLiquidateSmartCollateral( + Id.unwrap(id), + borrower, + address(sp), + seizedAssets, + address(swapPair), // token0Pair (unused since amount0 == 0) + address(swapPair), // token1Pair (BTCB -> LISUSD swap) + swapToken0Data, + swapToken1Data, + payload + ); + + // Position should have reduced debt + Position memory posAfter = moolah.position(marketParams.id(), borrower); + assertLt(posAfter.borrowShares, posBefore.borrowShares, "borrow shares did not decrease"); + } + + function test_flashLiquidateSmartCollateral_reverts_notWhitelisted() public { + _prepareLiquidatablePosition(false); + + MockSmartProvider sp = new MockSmartProvider(address(LISUSD), address(BTCB)); + sp.setCollateralToken(address(BTCB)); + // NOT whitelisted + + bytes memory payload = abi.encode(uint256(0), uint256(0)); + + vm.prank(BOT); + vm.expectRevert(BrokerLiquidator.NotWhitelisted.selector); + liquidator.flashLiquidateSmartCollateral( + Id.unwrap(id), + borrower, + address(sp), + 1e18, + address(1), + address(1), + "", + "", + payload + ); + } + + function test_liquidateSmartCollateral_reverts_invalidProvider() public { + _prepareLiquidatablePosition(false); + + // Create a smart provider whose TOKEN() != collateralToken (BTCB) + MockSmartProvider sp = new MockSmartProvider(address(LISUSD), address(BTCB)); + // collateralToken defaults to address(sp) != BTCB + + address[] memory providers = new address[](1); + providers[0] = address(sp); + vm.prank(MANAGER); + liquidator.batchSetSmartProviders(providers, true); + + bytes memory payload = abi.encode(uint256(0), uint256(0)); + + LISUSD.setBalance(address(liquidator), 1_000_000 ether); + + vm.prank(BOT); + vm.expectRevert(bytes("Invalid smart provider")); + liquidator.liquidateSmartCollateral(Id.unwrap(id), borrower, address(sp), 0, 1, payload); + } + + function test_flashLiquidateSmartCollateral_reverts_pairNotWhitelisted() public { + _prepareLiquidatablePosition(false); + + (MockSmartProvider sp, MockSwapPair swapPair) = _setupSmartProvider(); + + address badPair = makeAddr("badPair"); + bytes memory payload = abi.encode(uint256(0), uint256(0)); + + vm.prank(BOT); + vm.expectRevert(BrokerLiquidator.NotWhitelisted.selector); + liquidator.flashLiquidateSmartCollateral( + Id.unwrap(id), + borrower, + address(sp), + 1e18, + badPair, // not whitelisted + address(swapPair), + "", + "", + payload + ); + } +} + +/// @dev Mock swap pair that converts tokenIn -> tokenOut at oracle price. +/// Pulls the approved amount (not full balance) to match real aggregator behavior. +contract MockSwapPair is Test { + address public tokenIn; + address public tokenOut; + OracleMock public oracle; + + constructor(address _tokenIn, address _tokenOut, OracleMock _oracle) { + tokenIn = _tokenIn; + tokenOut = _tokenOut; + oracle = _oracle; + } + + /// @dev Swap approved tokenIn for tokenOut at oracle price + function swap() external { + uint256 amountIn = IERC20(tokenIn).allowance(msg.sender, address(this)); + if (amountIn > 0) { + IERC20(tokenIn).transferFrom(msg.sender, address(this), amountIn); + // convert at oracle price: amountIn * tokenInPrice / tokenOutPrice + uint256 tokenInPrice = oracle.peek(tokenIn); + uint256 tokenOutPrice = oracle.peek(tokenOut); + uint256 amountOut = (amountIn * tokenInPrice) / tokenOutPrice; + deal(tokenOut, address(this), amountOut); + IERC20(tokenOut).transfer(msg.sender, amountOut); + } + } } contract LiquidationCallbackMock is IMoolahLiquidateCallback { diff --git a/test/liquidator/BrokerLiquidator.t.sol b/test/liquidator/BrokerLiquidator.t.sol index 74989a75..f23e4432 100644 --- a/test/liquidator/BrokerLiquidator.t.sol +++ b/test/liquidator/BrokerLiquidator.t.sol @@ -26,7 +26,7 @@ contract BrokerLiquidatorTest is BaseTest { address(impl), abi.encodeWithSelector(impl.initialize.selector, OWNER, OWNER, BOT) ); - brokerLiquidator = BrokerLiquidator(address(proxy)); + brokerLiquidator = BrokerLiquidator(payable(address(proxy))); smartProvider = new MockSmartProvider(address(loanToken), address(collateralToken)); } diff --git a/test/liquidator/mocks/MockSmartProvider.sol b/test/liquidator/mocks/MockSmartProvider.sol index ed9e6b12..3ede1b7c 100644 --- a/test/liquidator/mocks/MockSmartProvider.sol +++ b/test/liquidator/mocks/MockSmartProvider.sol @@ -7,10 +7,35 @@ import { IERC20 } from "@openzeppelin/contracts/interfaces/IERC20.sol"; contract MockSmartProvider is Test { address public token0; address public token1; + address public collateralToken; + /// @dev Percentage of LP value going to token0 (in basis points, default 5000 = 50%) + uint256 public token0Bps = 5000; constructor(address _token0, address _token1) { token0 = _token0; token1 = _token1; + collateralToken = address(this); // default: LP token == this contract + } + + function setCollateralToken(address _collateralToken) external { + collateralToken = _collateralToken; + } + + function setToken0Bps(uint256 _bps) external { + token0Bps = _bps; + } + + function TOKEN() external view returns (address) { + return collateralToken; + } + + function dexLP() external view returns (address) { + return address(this); + } + + function token(uint256 id) external view returns (address) { + if (id == 0) return token0; + return token1; } function redeemLpCollateral( @@ -18,8 +43,8 @@ contract MockSmartProvider is Test { uint256 minToken0Out, uint256 minToken1Out ) external returns (uint256 token0Out, uint256 token1Out) { - token0Out = lpAmount / 2; - token1Out = lpAmount / 2; + token0Out = (lpAmount * token0Bps) / 10000; + token1Out = lpAmount - token0Out; require(token0Out >= minToken0Out, "token0 slippage"); require(token1Out >= minToken1Out, "token1 slippage"); From 22bb8cf2c271d2271470b102f0e53013dff042f6 Mon Sep 17 00:00:00 2001 From: Razorback Date: Fri, 3 Apr 2026 14:43:28 +0800 Subject: [PATCH 03/13] feat: move LendingBroker RELAYER/ORACLE from immutables to storage for shared impl Move RELAYER and ORACLE from constructor immutables to storage variables (appended at end of layout to preserve upgrade compatibility). This allows all LendingBroker proxies to share a single implementation deployment. - Constructor now only takes moolah address - initialize() accepts relayer and oracle as additional params - Added setRelayer() and setOracle() manager-only setters - Updated deploy scripts and all tests Co-Authored-By: Claude Opus 4.6 (1M context) --- script/broker/deploy_broker.s.sol | 8 +- script/broker/deploy_brokerImpl.s.sol | 18 +--- src/broker/LendingBroker.sol | 49 +++++++--- test/broker/LendingBroker.t.sol | 126 ++++++++++++++++++++++++-- test/moolah/MarketFactoryTest.sol | 6 +- 5 files changed, 166 insertions(+), 41 deletions(-) diff --git a/script/broker/deploy_broker.s.sol b/script/broker/deploy_broker.s.sol index 57bc9b89..7c942bfa 100644 --- a/script/broker/deploy_broker.s.sol +++ b/script/broker/deploy_broker.s.sol @@ -37,8 +37,8 @@ contract DeployLendingBroker is DeployBase { console.log("Deployer: ", deployer); vm.startBroadcast(deployerPrivateKey); - // Deploy LendingBroker implementation - LendingBroker impl = new LendingBroker(moolah, interestRelayer, oracle, wbnb); + // Deploy LendingBroker implementation (single impl shared across all proxies) + LendingBroker impl = new LendingBroker(moolah, wbnb); console.log("LendingBroker implementation: ", address(impl)); // Deploy LendingBroker proxy @@ -51,7 +51,9 @@ contract DeployLendingBroker is DeployBase { bot, pauser, rateCalculator, - maxFixedLoanPositions + maxFixedLoanPositions, + interestRelayer, + oracle ) ); console.log("LendingBroker proxy: ", address(proxy)); diff --git a/script/broker/deploy_brokerImpl.s.sol b/script/broker/deploy_brokerImpl.s.sol index 4ca96922..5ceeb797 100644 --- a/script/broker/deploy_brokerImpl.s.sol +++ b/script/broker/deploy_brokerImpl.s.sol @@ -35,9 +35,11 @@ contract DeployLendingBrokerImpl is DeployBase { 0xc26CaAcb00854c5460030B0aFde60C37D9d39C79, 0x3ade951523e81dD45e5787bb0b95Ce7341Db1287 ]; + address moolah; address wbnb; function setUp() public { + moolah = vm.envAddress("MOOLAH"); wbnb = vm.envOr("WBNB", address(0)); } @@ -48,19 +50,9 @@ contract DeployLendingBrokerImpl is DeployBase { vm.startBroadcast(deployerPrivateKey); - for (uint256 i = 0; i < brokers.length; i++) { - address payable proxy = payable(brokers[i]); - - // Read constructor params from the existing proxy contract - address _moolah = address(LendingBroker(proxy).MOOLAH()); - address _relayer = LendingBroker(proxy).RELAYER(); - address _oracle = address(LendingBroker(proxy).ORACLE()); - - // Deploy LendingBroker implementation - LendingBroker impl = new LendingBroker(_moolah, _relayer, _oracle, wbnb); - console.log("Broker proxy:", proxy); - console.log(" New impl: ", address(impl)); - } + // Deploy LendingBroker implementation + LendingBroker impl = new LendingBroker(moolah, wbnb); + console.log("LendingBroker implementation: ", address(impl)); vm.stopBroadcast(); } diff --git a/src/broker/LendingBroker.sol b/src/broker/LendingBroker.sol index 4488e199..cf9be9c0 100644 --- a/src/broker/LendingBroker.sol +++ b/src/broker/LendingBroker.sol @@ -77,8 +77,6 @@ contract LendingBroker is // ------- Immutables ------- IMoolah public immutable MOOLAH; - address public immutable RELAYER; - IOracle public immutable ORACLE; /// @dev Wrapped native token (e.g. WBNB). address(0) if native borrow/repay is not supported. address public immutable WBNB; uint256 public constant MAX_FIXED_TERM_APR = 13e26; // 1.3 * RATE_SCALE = 30% MAX APR @@ -118,6 +116,10 @@ contract LendingBroker is /// @dev liquidation whitelist EnumerableSet.AddressSet private liquidationWhitelist; + // --- V2 storage (appended to preserve layout) --- + address public RELAYER; + IOracle public ORACLE; + // ------- Modifiers ------- modifier onlyMoolah() { if (msg.sender != address(MOOLAH)) revert NotMoolah(); @@ -137,19 +139,12 @@ contract LendingBroker is /** * @dev Constructor for the LendingBroker contract * @param moolah The address of the Moolah contract - * @param relayer The address of the BrokerInterestRelayer contract - * @param oracle The address of the oracle * @param wbnb The address of the wrapped native token (e.g. WBNB). Pass address(0) to disable native support. */ - constructor(address moolah, address relayer, address oracle, address wbnb) { - // zero address assert - if (moolah == address(0) || relayer == address(0) || oracle == address(0)) revert ZeroAddressProvided(); - // set addresses + constructor(address moolah, address wbnb) { + if (moolah == address(0)) revert ZeroAddressProvided(); MOOLAH = IMoolah(moolah); - RELAYER = relayer; - ORACLE = IOracle(oracle); WBNB = wbnb; - _disableInitializers(); } @@ -161,6 +156,8 @@ contract LendingBroker is * @param _pauser The address of the pauser * @param _rateCalculator The address of the rate calculator * @param _maxFixedLoanPositions The maximum number of fixed loan positions a user can have + * @param _relayer The address of the BrokerInterestRelayer contract + * @param _oracle The address of the oracle */ function initialize( address _admin, @@ -168,7 +165,9 @@ contract LendingBroker is address _bot, address _pauser, address _rateCalculator, - uint256 _maxFixedLoanPositions + uint256 _maxFixedLoanPositions, + address _relayer, + address _oracle ) public initializer { if ( _admin == address(0) || @@ -176,7 +175,9 @@ contract LendingBroker is _bot == address(0) || _pauser == address(0) || _rateCalculator == address(0) || - _maxFixedLoanPositions == 0 + _maxFixedLoanPositions == 0 || + _relayer == address(0) || + _oracle == address(0) ) revert ZeroAddressProvided(); __AccessControlEnumerable_init(); @@ -190,6 +191,8 @@ contract LendingBroker is // init state variables rateCalculator = _rateCalculator; maxFixedLoanPositions = _maxFixedLoanPositions; + RELAYER = _relayer; + ORACLE = IOracle(_oracle); } /////////////////////////////////////// @@ -1118,6 +1121,26 @@ contract LendingBroker is emit BorrowPaused(paused); } + /** + * @dev Set the relayer address + * @param _relayer The address of the BrokerInterestRelayer contract + */ + function setRelayer(address _relayer) external onlyRole(MANAGER) { + require(_relayer != address(0), "broker/zero-address-provided"); + require(RELAYER != _relayer, "broker/same-value-provided"); + RELAYER = _relayer; + } + + /** + * @dev Set the oracle address + * @param _oracle The address of the oracle + */ + function setOracle(address _oracle) external onlyRole(MANAGER) { + require(_oracle != address(0), "broker/zero-address-provided"); + require(address(ORACLE) != _oracle, "broker/same-value-provided"); + ORACLE = IOracle(_oracle); + } + /** * @dev pause contract */ diff --git a/test/broker/LendingBroker.t.sol b/test/broker/LendingBroker.t.sol index 8537761a..c0782f71 100644 --- a/test/broker/LendingBroker.t.sol +++ b/test/broker/LendingBroker.t.sol @@ -182,10 +182,20 @@ contract LendingBrokerTest is Test { rateCalc = RateCalculator(address(rcProxy)); // Deploy LendingBroker proxy first (used as oracle by the market) - LendingBroker bImpl = new LendingBroker(address(moolah), address(relayer), address(oracle), address(0)); + LendingBroker bImpl = new LendingBroker(address(moolah), address(0)); ERC1967Proxy bProxy = new ERC1967Proxy( address(bImpl), - abi.encodeWithSelector(LendingBroker.initialize.selector, ADMIN, MANAGER, BOT, PAUSER, address(rateCalc), 10) + abi.encodeWithSelector( + LendingBroker.initialize.selector, + ADMIN, + MANAGER, + BOT, + PAUSER, + address(rateCalc), + 10, + address(relayer), + address(oracle) + ) ); broker = LendingBroker(payable(address(bProxy))); @@ -1444,10 +1454,20 @@ contract LendingBrokerTest is Test { function test_marketIdSet_guard_reverts() public { // Deploy a second broker without setting market id - LendingBroker bImpl2 = new LendingBroker(address(moolah), address(vault), address(oracle), address(0)); + LendingBroker bImpl2 = new LendingBroker(address(moolah), address(0)); ERC1967Proxy bProxy2 = new ERC1967Proxy( address(bImpl2), - abi.encodeWithSelector(LendingBroker.initialize.selector, ADMIN, MANAGER, BOT, PAUSER, address(rateCalc), 10) + abi.encodeWithSelector( + LendingBroker.initialize.selector, + ADMIN, + MANAGER, + BOT, + PAUSER, + address(rateCalc), + 10, + address(relayer), + address(oracle) + ) ); LendingBroker broker2 = LendingBroker(payable(address(bProxy2))); vm.expectRevert(LendingBroker.MarketNotSet.selector); @@ -1462,10 +1482,20 @@ contract LendingBrokerTest is Test { } function test_liquidatorSetMarketWhitelist_whitelistsNewBroker() public { - LendingBroker bImpl2 = new LendingBroker(address(moolah), address(relayer), address(oracle), address(0)); + LendingBroker bImpl2 = new LendingBroker(address(moolah), address(0)); ERC1967Proxy bProxy2 = new ERC1967Proxy( address(bImpl2), - abi.encodeWithSelector(LendingBroker.initialize.selector, ADMIN, MANAGER, BOT, PAUSER, address(rateCalc), 10) + abi.encodeWithSelector( + LendingBroker.initialize.selector, + ADMIN, + MANAGER, + BOT, + PAUSER, + address(rateCalc), + 10, + address(relayer), + address(oracle) + ) ); LendingBroker newBroker = LendingBroker(payable(address(bProxy2))); @@ -1493,10 +1523,20 @@ contract LendingBrokerTest is Test { } function test_liquidatorBatchSetMarketWhitelist_whitelistsMultipleMarkets() public { - LendingBroker bImplA = new LendingBroker(address(moolah), address(relayer), address(oracle), address(0)); + LendingBroker bImplA = new LendingBroker(address(moolah), address(0)); ERC1967Proxy bProxyA = new ERC1967Proxy( address(bImplA), - abi.encodeWithSelector(LendingBroker.initialize.selector, ADMIN, MANAGER, BOT, PAUSER, address(rateCalc), 10) + abi.encodeWithSelector( + LendingBroker.initialize.selector, + ADMIN, + MANAGER, + BOT, + PAUSER, + address(rateCalc), + 10, + address(relayer), + address(oracle) + ) ); LendingBroker brokerA = LendingBroker(payable(address(bProxyA))); @@ -1512,10 +1552,20 @@ contract LendingBrokerTest is Test { vm.prank(MANAGER); brokerA.setMarketId(idA); - LendingBroker bImplB = new LendingBroker(address(moolah), address(relayer), address(oracle), address(0)); + LendingBroker bImplB = new LendingBroker(address(moolah), address(0)); ERC1967Proxy bProxyB = new ERC1967Proxy( address(bImplB), - abi.encodeWithSelector(LendingBroker.initialize.selector, ADMIN, MANAGER, BOT, PAUSER, address(rateCalc), 10) + abi.encodeWithSelector( + LendingBroker.initialize.selector, + ADMIN, + MANAGER, + BOT, + PAUSER, + address(rateCalc), + 10, + address(relayer), + address(oracle) + ) ); LendingBroker brokerB = LendingBroker(payable(address(bProxyB))); @@ -2158,6 +2208,62 @@ contract LendingBrokerTest is Test { vault.setLockBuffer(address(newBuffer)); } + // ============================================= + // setRelayer / setOracle tests + // ============================================= + + function test_setRelayer_success() public { + address newRelayer = makeAddr("newRelayer"); + vm.prank(MANAGER); + broker.setRelayer(newRelayer); + assertEq(broker.RELAYER(), newRelayer); + } + + function test_setRelayer_reverts_zeroAddress() public { + vm.prank(MANAGER); + vm.expectRevert(bytes("broker/zero-address-provided")); + broker.setRelayer(address(0)); + } + + function test_setRelayer_reverts_sameValue() public { + address currentRelayer = broker.RELAYER(); + vm.prank(MANAGER); + vm.expectRevert(bytes("broker/same-value-provided")); + broker.setRelayer(currentRelayer); + } + + function test_setRelayer_reverts_notManager() public { + vm.prank(borrower); + vm.expectRevert(); + broker.setRelayer(makeAddr("newRelayer")); + } + + function test_setOracle_success() public { + address newOracle = makeAddr("newOracle"); + vm.prank(MANAGER); + broker.setOracle(newOracle); + assertEq(address(broker.ORACLE()), newOracle); + } + + function test_setOracle_reverts_zeroAddress() public { + vm.prank(MANAGER); + vm.expectRevert(bytes("broker/zero-address-provided")); + broker.setOracle(address(0)); + } + + function test_setOracle_reverts_sameValue() public { + address currentOracle = address(broker.ORACLE()); + vm.prank(MANAGER); + vm.expectRevert(bytes("broker/same-value-provided")); + broker.setOracle(currentOracle); + } + + function test_setOracle_reverts_notManager() public { + vm.prank(borrower); + vm.expectRevert(); + broker.setOracle(makeAddr("newOracle")); + } + // ============================================= // Smart LP liquidation tests // ============================================= diff --git a/test/moolah/MarketFactoryTest.sol b/test/moolah/MarketFactoryTest.sol index 5e4d41fb..a43b5b61 100644 --- a/test/moolah/MarketFactoryTest.sol +++ b/test/moolah/MarketFactoryTest.sol @@ -495,7 +495,7 @@ contract MarketFactoryTest is Test { } function newLendingBroker(address replayer) private returns (LendingBroker) { - LendingBroker lendingBrokerImpl = new LendingBroker(address(moolah), replayer, address(oracle), address(0)); + LendingBroker lendingBrokerImpl = new LendingBroker(address(moolah), address(0)); ERC1967Proxy lendingBrokerProxy = new ERC1967Proxy( address(lendingBrokerImpl), abi.encodeWithSelector( @@ -505,7 +505,9 @@ contract MarketFactoryTest is Test { bot, pauser, address(rateCalculator), - 100 + 100, + replayer, + address(oracle) ) ); From 18dc48812f0d33b6d073ed3cdc88b9b57186fb26 Mon Sep 17 00:00:00 2001 From: yq <153907566+qingyang-lista@users.noreply.github.com> Date: Tue, 31 Mar 2026 22:10:43 +0800 Subject: [PATCH 04/13] fix: preserve unpaid interest in deductFixedPositionDebt during partial liquidation When partial liquidation deducts both interest and principal from a fixed position, the old code unconditionally reset interestRepaid=0 and lastRepaidTime=now. This erased any unpaid interest from position tracking, causing getUserTotalDebt() to under-report debt. The fix introduces three branches: 1. All interest covered -> safe to reset (unchanged behavior) 2. Partial interest + formula can represent outstanding -> adjust interestRepaid precisely so outstanding = accruedInterest - paidInterest (exact) 3. Partial interest + formula too small (most principal repaid) -> set interestRepaid=0 without resetting lastRepaidTime to maximize preserved outstanding Co-Authored-By: Claude Opus 4.6 (1M context) --- src/broker/libraries/BrokerMath.sol | 25 +- test/broker/BrokerMathDeductFixed.t.sol | 332 ++++++++++++++++++++++++ 2 files changed, 353 insertions(+), 4 deletions(-) create mode 100644 test/broker/BrokerMathDeductFixed.t.sol diff --git a/src/broker/libraries/BrokerMath.sol b/src/broker/libraries/BrokerMath.sol index c0ae5103..3b3b7b47 100644 --- a/src/broker/libraries/BrokerMath.sol +++ b/src/broker/libraries/BrokerMath.sol @@ -546,10 +546,27 @@ library BrokerMath { // update repaid principal amount principalToDeduct -= repayPrincipalAmt; p.principalRepaid += repayPrincipalAmt; - // reset repaid interest to zero (all accrued interest has been cleared) - p.interestRepaid = 0; - // reset repaid time to now - p.lastRepaidTime = block.timestamp; + + if (repayInterestAmt >= accruedInterest) { + // all accrued interest fully covered -> safe to reset tracking + p.interestRepaid = 0; + p.lastRepaidTime = block.timestamp; + } else { + // partial interest covered -> adjust interestRepaid to preserve outstanding + // After principalRepaid increased, getAccruedInterestForFixedPosition() recalculates + // with smaller (principal - principalRepaid), producing a lower total (newTotalAccrued). + // We set interestRepaid so that: newTotalAccrued - interestRepaid = unpaidInterest + uint256 unpaidInterest = accruedInterest - repayInterestAmt; + uint256 newTotalAccrued = getAccruedInterestForFixedPosition(p); + if (newTotalAccrued >= unpaidInterest) { + // exact: outstanding is perfectly preserved + p.interestRepaid = newTotalAccrued - unpaidInterest; + } else { + // edge case: most principal repaid, formula can't represent full outstanding + // preserve maximum possible: outstanding = newTotalAccrued (don't reset lastRepaidTime) + p.interestRepaid = 0; + } + } } return (interestToDeduct, principalToDeduct, p); diff --git a/test/broker/BrokerMathDeductFixed.t.sol b/test/broker/BrokerMathDeductFixed.t.sol new file mode 100644 index 00000000..72f0d7b6 --- /dev/null +++ b/test/broker/BrokerMathDeductFixed.t.sol @@ -0,0 +1,332 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.34; + +import "forge-std/Test.sol"; +import { BrokerMath, RATE_SCALE } from "../../src/broker/libraries/BrokerMath.sol"; +import { FixedLoanPosition } from "../../src/broker/interfaces/IBroker.sol"; +import { UtilsLib } from "../../src/moolah/libraries/UtilsLib.sol"; + +/// @title Tests for BrokerMath.deductFixedPositionDebt +/// @notice Validates that partial liquidation preserves exact outstanding interest, +/// accounting for the reduced principal effect on the interest formula. +contract BrokerMathDeductFixedTest is Test { + uint256 constant DURATION = 365 days; + // 10% APR -> RATE_SCALE * 1.10 + uint256 constant APR = 110 * 1e25; + + uint256 startTs; + + function setUp() public { + vm.warp(1_000_000); + startTs = block.timestamp; + } + + function _makePosition(uint256 principal) internal view returns (FixedLoanPosition memory) { + return + FixedLoanPosition({ + posId: 1, + principal: principal, + apr: APR, + start: startTs, + end: startTs + DURATION, + lastRepaidTime: startTs, + interestRepaid: 0, + principalRepaid: 0 + }); + } + + /// @dev Helper: compute outstanding interest for a position + function _outstanding(FixedLoanPosition memory p) internal view returns (uint256) { + return BrokerMath.getAccruedInterestForFixedPosition(p) - p.interestRepaid; + } + + // ==================================================================== + // Core fix: partial liquidation preserves EXACT outstanding interest + // ==================================================================== + + /// @notice principal=100e18, interest~10e18, liquidation pays half interest + half principal. + /// Outstanding interest must be exactly (accruedInterest - paidInterest). + function test_partialLiquidation_preservesExactOutstanding() public { + FixedLoanPosition memory pos = _makePosition(100 ether); + skip(DURATION); + + uint256 accruedInterest = _outstanding(pos); + assertApproxEqRel(accruedInterest, 10 ether, 1e15, "accrued ~10 ether"); + + uint256 interestBudget = accruedInterest / 2; + uint256 principalBudget = 50 ether; + + (uint256 interestLeft, uint256 principalLeft, FixedLoanPosition memory updated) = BrokerMath + .deductFixedPositionDebt(interestBudget, principalBudget, pos); + + // Budgets fully consumed + assertEq(interestLeft, 0, "interest budget consumed"); + assertEq(principalLeft, 0, "principal budget consumed"); + assertEq(updated.principalRepaid, principalBudget, "principalRepaid correct"); + + // Core invariant: outstanding = accruedInterest - paidInterest (exact) + uint256 expectedOutstanding = accruedInterest - interestBudget; + uint256 actualOutstanding = _outstanding(updated); + assertEq(actualOutstanding, expectedOutstanding, "outstanding interest must be exact"); + } + + // ==================================================================== + // Full interest payment resets correctly + // ==================================================================== + + function test_fullInterestPayment_resetsTracking() public { + FixedLoanPosition memory pos = _makePosition(100 ether); + skip(DURATION); + + uint256 accruedInterest = _outstanding(pos); + + (, , FixedLoanPosition memory updated) = BrokerMath.deductFixedPositionDebt(accruedInterest, 30 ether, pos); + + assertEq(updated.interestRepaid, 0, "interestRepaid resets when all interest paid"); + assertEq(updated.lastRepaidTime, block.timestamp, "lastRepaidTime resets to now"); + assertEq(updated.principalRepaid, 30 ether, "principalRepaid correct"); + + // Outstanding should be 0 after full interest payment + assertEq(_outstanding(updated), 0, "no outstanding interest after full payment"); + } + + // ==================================================================== + // Interest only, no principal deduction + // ==================================================================== + + function test_interestOnlyPartial_preservesOutstanding() public { + FixedLoanPosition memory pos = _makePosition(100 ether); + skip(DURATION); + + uint256 accruedInterest = _outstanding(pos); + uint256 interestBudget = accruedInterest / 3; + + (uint256 interestLeft, uint256 principalLeft, FixedLoanPosition memory updated) = BrokerMath + .deductFixedPositionDebt(interestBudget, 0, pos); + + assertEq(interestLeft, 0, "interest budget consumed"); + assertEq(principalLeft, 0, "no principal to deduct"); + assertEq(updated.interestRepaid, interestBudget, "partial interest tracked"); + assertEq(updated.lastRepaidTime, startTs, "lastRepaidTime unchanged"); + + uint256 expectedOutstanding = accruedInterest - interestBudget; + assertEq(_outstanding(updated), expectedOutstanding, "outstanding exact for interest-only"); + } + + // ==================================================================== + // Full principal with partial interest -> fallback (position filtered out) + // ==================================================================== + + function test_fullPrincipalPartialInterest_fallback() public { + FixedLoanPosition memory pos = _makePosition(100 ether); + skip(DURATION); + + uint256 accruedInterest = _outstanding(pos); + uint256 interestBudget = accruedInterest / 4; + + (, , FixedLoanPosition memory updated) = BrokerMath.deductFixedPositionDebt(interestBudget, 100 ether, pos); + + assertEq(updated.principalRepaid, 100 ether, "full principal repaid"); + + // When principal is fully repaid, (principal - principalRepaid) = 0, + // so getAccruedInterestForFixedPosition returns 0 -> fallback case. + // outstanding = 0, but position would be filtered out anyway. + bool wouldBeFiltered = !(updated.principal > updated.principalRepaid); + assertTrue(wouldBeFiltered, "fully-repaid position filtered out in sortAndFilter"); + } + + // ==================================================================== + // Zero interest accrued (immediate liquidation) + // ==================================================================== + + function test_zeroInterest_principalOnly() public { + FixedLoanPosition memory pos = _makePosition(100 ether); + // No time skip -> zero interest + + (uint256 interestLeft, uint256 principalLeft, FixedLoanPosition memory updated) = BrokerMath + .deductFixedPositionDebt(10 ether, 50 ether, pos); + + assertEq(interestLeft, 10 ether, "interest budget returned unused"); + assertEq(principalLeft, 0, "principal budget consumed"); + assertEq(updated.principalRepaid, 50 ether, "principal repaid"); + // 0 >= 0 -> reset happens (correct, no interest to lose) + assertEq(updated.interestRepaid, 0, "no interest to track"); + assertEq(updated.lastRepaidTime, block.timestamp, "reset fine when no interest"); + } + + // ==================================================================== + // Sequential partial liquidations preserve cumulative outstanding + // ==================================================================== + + function test_sequentialPartialLiquidations_preserveOutstanding() public { + FixedLoanPosition memory pos = _makePosition(100 ether); + skip(DURATION); + + uint256 originalAccrued = _outstanding(pos); + + // First partial: 1/4 interest + 20 principal + uint256 firstInterest = originalAccrued / 4; + (, , FixedLoanPosition memory after1) = BrokerMath.deductFixedPositionDebt(firstInterest, 20 ether, pos); + + uint256 expectedAfter1 = originalAccrued - firstInterest; + assertEq(_outstanding(after1), expectedAfter1, "first: outstanding exact"); + assertEq(after1.principalRepaid, 20 ether, "first: principal tracked"); + + // Second partial: another chunk of interest + 30 principal + uint256 outstandingAfter1 = _outstanding(after1); + uint256 secondInterest = outstandingAfter1 / 3; + + (, , FixedLoanPosition memory after2) = BrokerMath.deductFixedPositionDebt(secondInterest, 30 ether, after1); + + uint256 expectedAfter2 = outstandingAfter1 - secondInterest; + // allow 1 wei tolerance due to Ceil rounding in interest formula + assertApproxEqAbs(_outstanding(after2), expectedAfter2, 1, "second: outstanding exact"); + assertEq(after2.principalRepaid, 50 ether, "second: cumulative principal"); + + // Outstanding still positive + assertGt(_outstanding(after2), 0, "interest still outstanding"); + } + + // ==================================================================== + // Over-payment: budget exceeds debt + // ==================================================================== + + function test_overPayment_cappedCorrectly() public { + FixedLoanPosition memory pos = _makePosition(100 ether); + skip(DURATION); + + uint256 accruedInterest = _outstanding(pos); + + (uint256 interestLeft, uint256 principalLeft, FixedLoanPosition memory updated) = BrokerMath + .deductFixedPositionDebt(accruedInterest * 10, 500 ether, pos); + + assertApproxEqAbs(interestLeft, accruedInterest * 9, 1e15, "excess interest returned"); + assertEq(principalLeft, 400 ether, "excess principal returned"); + assertEq(updated.principalRepaid, 100 ether, "full principal repaid"); + // All interest paid -> reset + assertEq(updated.interestRepaid, 0, "reset after full interest payment"); + assertEq(updated.lastRepaidTime, block.timestamp, "reset lastRepaidTime"); + } + + // ==================================================================== + // Exact interest match triggers reset + // ==================================================================== + + function test_exactInterestMatch_triggersReset() public { + FixedLoanPosition memory pos = _makePosition(100 ether); + skip(DURATION); + + uint256 accruedInterest = _outstanding(pos); + + (, , FixedLoanPosition memory updated) = BrokerMath.deductFixedPositionDebt(accruedInterest, 40 ether, pos); + + assertEq(updated.interestRepaid, 0, "reset on exact match"); + assertEq(updated.lastRepaidTime, block.timestamp, "reset time on exact match"); + assertEq(updated.principalRepaid, 40 ether, "principal deducted"); + assertEq(_outstanding(updated), 0, "no outstanding after exact match"); + } + + // ==================================================================== + // 1 wei short of full interest -> no reset, exact outstanding + // ==================================================================== + + function test_oneWeiShort_preservesExactOutstanding() public { + FixedLoanPosition memory pos = _makePosition(100 ether); + skip(DURATION); + + uint256 accruedInterest = _outstanding(pos); + require(accruedInterest > 1, "need non-trivial interest"); + + uint256 interestBudget = accruedInterest - 1; + + (, , FixedLoanPosition memory updated) = BrokerMath.deductFixedPositionDebt(interestBudget, 50 ether, pos); + + // 1 wei unpaid -> must preserve exactly + assertEq(_outstanding(updated), 1, "exactly 1 wei outstanding preserved"); + } + + // ==================================================================== + // Zero interest budget with principal deduction -> maximize preserved + // ==================================================================== + + function test_zeroInterestBudget_principalOnly_maximizesOutstanding() public { + FixedLoanPosition memory pos = _makePosition(100 ether); + skip(DURATION); + + uint256 accruedInterest = _outstanding(pos); + assertGt(accruedInterest, 0, "should have accrued interest"); + + (uint256 interestLeft, uint256 principalLeft, FixedLoanPosition memory updated) = BrokerMath + .deductFixedPositionDebt(0, 50 ether, pos); + + assertEq(interestLeft, 0, "no interest budget to return"); + assertEq(principalLeft, 0, "principal consumed"); + assertEq(updated.principalRepaid, 50 ether, "principal repaid"); + + // newTotalAccrued = (100-50)*10%*1year = 5e18, unpaidInterest = 10e18 + // newTotalAccrued < unpaidInterest -> fallback: outstanding = newTotalAccrued + // This is the maximum the formula can represent (better than reset which gives 0) + uint256 newTotalAccrued = BrokerMath.getAccruedInterestForFixedPosition(updated); + assertEq(_outstanding(updated), newTotalAccrued, "fallback preserves maximum possible"); + assertGt(_outstanding(updated), 0, "outstanding > 0 (not reset to zero)"); + // Verify: lastRepaidTime was NOT reset (preserves historical accrual) + assertEq(updated.lastRepaidTime, startTs, "lastRepaidTime not reset in fallback"); + } + + // ==================================================================== + // Small principal + large interest -> fallback maximizes preserved + // ==================================================================== + + function test_smallPrincipal_largeInterest_maximizesOutstanding() public { + FixedLoanPosition memory pos = _makePosition(100 ether); + skip(DURATION); + + uint256 accruedInterest = _outstanding(pos); + + // Tiny interest budget + small principal + uint256 interestBudget = 1; + uint256 principalBudget = 5 ether; + + (, , FixedLoanPosition memory updated) = BrokerMath.deductFixedPositionDebt(interestBudget, principalBudget, pos); + + // newTotalAccrued = (100-5)*10%*1year = 9.5e18, unpaidInterest ~= 10e18 + // newTotalAccrued < unpaidInterest -> fallback: maximize outstanding + uint256 newTotalAccrued = BrokerMath.getAccruedInterestForFixedPosition(updated); + assertEq(_outstanding(updated), newTotalAccrued, "fallback preserves max possible"); + assertGt(_outstanding(updated), 0, "outstanding > 0"); + // Verify: better than old code which would give outstanding = 0 + assertGt(_outstanding(updated), accruedInterest / 2, "preserves majority of interest"); + } + + // ==================================================================== + // After reset, new interest accrues correctly + // ==================================================================== + + function test_resetThenNewAccrual_worksCorrectly() public { + FixedLoanPosition memory pos = _makePosition(100 ether); + skip(DURATION / 2); + + uint256 accrued1 = _outstanding(pos); + + // Pay all interest + 20 principal -> triggers reset + (, , FixedLoanPosition memory after1) = BrokerMath.deductFixedPositionDebt(accrued1, 20 ether, pos); + + assertEq(after1.interestRepaid, 0, "reset after full interest"); + assertEq(after1.lastRepaidTime, block.timestamp, "lastRepaidTime reset"); + assertEq(_outstanding(after1), 0, "no outstanding after reset"); + + // More time passes -> new interest accrues on reduced principal + skip(DURATION / 2); + + uint256 accrued2 = _outstanding(after1); + assertGt(accrued2, 0, "new interest accrued after reset"); + + // Partial interest + 30 principal -> should NOT reset, preserve exact + uint256 partialInterest = accrued2 / 2; + (, , FixedLoanPosition memory after2) = BrokerMath.deductFixedPositionDebt(partialInterest, 30 ether, after1); + + uint256 expectedOutstanding = accrued2 - partialInterest; + assertEq(_outstanding(after2), expectedOutstanding, "exact after second partial"); + assertEq(after2.principalRepaid, 50 ether, "cumulative 50 principal"); + } +} From 955d6a4c0f3743da7381dd24f0089ffc46cfea6a Mon Sep 17 00:00:00 2001 From: Razorback Date: Fri, 3 Apr 2026 16:00:02 +0800 Subject: [PATCH 05/13] fix: setter should be one-time migration --- src/broker/LendingBroker.sol | 12 ++--- test/broker/LendingBroker.t.sol | 92 ++++++++++++++++++++++----------- 2 files changed, 69 insertions(+), 35 deletions(-) diff --git a/src/broker/LendingBroker.sol b/src/broker/LendingBroker.sol index cf9be9c0..d3703a73 100644 --- a/src/broker/LendingBroker.sol +++ b/src/broker/LendingBroker.sol @@ -1122,22 +1122,22 @@ contract LendingBroker is } /** - * @dev Set the relayer address + * @dev Set the relayer address (one-time migration from immutable to storage) * @param _relayer The address of the BrokerInterestRelayer contract */ - function setRelayer(address _relayer) external onlyRole(MANAGER) { + function setRelayer(address _relayer) external onlyRole(DEFAULT_ADMIN_ROLE) { require(_relayer != address(0), "broker/zero-address-provided"); - require(RELAYER != _relayer, "broker/same-value-provided"); + require(RELAYER == address(0), "broker/already-set"); RELAYER = _relayer; } /** - * @dev Set the oracle address + * @dev Set the oracle address (one-time migration from immutable to storage) * @param _oracle The address of the oracle */ - function setOracle(address _oracle) external onlyRole(MANAGER) { + function setOracle(address _oracle) external onlyRole(DEFAULT_ADMIN_ROLE) { require(_oracle != address(0), "broker/zero-address-provided"); - require(address(ORACLE) != _oracle, "broker/same-value-provided"); + require(address(ORACLE) == address(0), "broker/already-set"); ORACLE = IOracle(_oracle); } diff --git a/test/broker/LendingBroker.t.sol b/test/broker/LendingBroker.t.sol index c0782f71..826a029a 100644 --- a/test/broker/LendingBroker.t.sol +++ b/test/broker/LendingBroker.t.sol @@ -2209,59 +2209,93 @@ contract LendingBrokerTest is Test { } // ============================================= - // setRelayer / setOracle tests + // setRelayer / setOracle tests (one-time, admin-only) // ============================================= + /// @dev Deploy a fresh broker proxy with RELAYER/ORACLE unset (simulating V1->V2 upgrade) + function _deployBrokerWithEmptyRelayerOracle() internal returns (LendingBroker) { + LendingBroker bImpl = new LendingBroker(address(moolah)); + // Use 6-param initialize (no relayer/oracle) by encoding only the original params + // and leaving RELAYER/ORACLE as address(0) + ERC1967Proxy bProxy = new ERC1967Proxy( + address(bImpl), + abi.encodeWithSelector( + LendingBroker.initialize.selector, + ADMIN, + MANAGER, + BOT, + PAUSER, + address(rateCalc), + 10, + address(1), // placeholder relayer — will be overwritten below + address(1) // placeholder oracle — will be overwritten below + ) + ); + LendingBroker b = LendingBroker(payable(address(bProxy))); + // Simulate V1->V2 upgrade: storage RELAYER/ORACLE are zeroed out + vm.store(address(b), bytes32(uint256(18)), bytes32(0)); // RELAYER slot + vm.store(address(b), bytes32(uint256(19)), bytes32(0)); // ORACLE slot + return b; + } + function test_setRelayer_success() public { - address newRelayer = makeAddr("newRelayer"); - vm.prank(MANAGER); - broker.setRelayer(newRelayer); - assertEq(broker.RELAYER(), newRelayer); + LendingBroker b = _deployBrokerWithEmptyRelayerOracle(); + assertEq(b.RELAYER(), address(0)); + + vm.prank(ADMIN); + b.setRelayer(address(relayer)); + assertEq(b.RELAYER(), address(relayer)); } function test_setRelayer_reverts_zeroAddress() public { - vm.prank(MANAGER); + LendingBroker b = _deployBrokerWithEmptyRelayerOracle(); + vm.prank(ADMIN); vm.expectRevert(bytes("broker/zero-address-provided")); - broker.setRelayer(address(0)); + b.setRelayer(address(0)); } - function test_setRelayer_reverts_sameValue() public { - address currentRelayer = broker.RELAYER(); - vm.prank(MANAGER); - vm.expectRevert(bytes("broker/same-value-provided")); - broker.setRelayer(currentRelayer); + function test_setRelayer_reverts_alreadySet() public { + // broker from setUp already has RELAYER set via initialize + vm.prank(ADMIN); + vm.expectRevert(bytes("broker/already-set")); + broker.setRelayer(makeAddr("newRelayer")); } - function test_setRelayer_reverts_notManager() public { - vm.prank(borrower); + function test_setRelayer_reverts_notAdmin() public { + LendingBroker b = _deployBrokerWithEmptyRelayerOracle(); + vm.prank(MANAGER); vm.expectRevert(); - broker.setRelayer(makeAddr("newRelayer")); + b.setRelayer(address(relayer)); } function test_setOracle_success() public { - address newOracle = makeAddr("newOracle"); - vm.prank(MANAGER); - broker.setOracle(newOracle); - assertEq(address(broker.ORACLE()), newOracle); + LendingBroker b = _deployBrokerWithEmptyRelayerOracle(); + assertEq(address(b.ORACLE()), address(0)); + + vm.prank(ADMIN); + b.setOracle(address(oracle)); + assertEq(address(b.ORACLE()), address(oracle)); } function test_setOracle_reverts_zeroAddress() public { - vm.prank(MANAGER); + LendingBroker b = _deployBrokerWithEmptyRelayerOracle(); + vm.prank(ADMIN); vm.expectRevert(bytes("broker/zero-address-provided")); - broker.setOracle(address(0)); + b.setOracle(address(0)); } - function test_setOracle_reverts_sameValue() public { - address currentOracle = address(broker.ORACLE()); - vm.prank(MANAGER); - vm.expectRevert(bytes("broker/same-value-provided")); - broker.setOracle(currentOracle); + function test_setOracle_reverts_alreadySet() public { + // broker from setUp already has ORACLE set via initialize + vm.prank(ADMIN); + vm.expectRevert(bytes("broker/already-set")); + broker.setOracle(makeAddr("newOracle")); } - function test_setOracle_reverts_notManager() public { - vm.prank(borrower); + function test_setOracle_reverts_notAdmin() public { + LendingBroker b = _deployBrokerWithEmptyRelayerOracle(); + vm.prank(MANAGER); vm.expectRevert(); - broker.setOracle(makeAddr("newOracle")); + b.setOracle(address(oracle)); } // ============================================= From 89adb0e70f9ae19476d805d5e53466f217d43284 Mon Sep 17 00:00:00 2001 From: Rick Date: Thu, 9 Apr 2026 12:04:51 +0800 Subject: [PATCH 06/13] fix: address audit findings for BrokerLiquidator and LendingBroker - Add RelayerSet/OracleSet event emissions in LendingBroker migration setters (#7) - Rename withdrawERC20 to withdraw with native BNB support in BrokerLiquidator (#1) - Add NatSpec documenting custody requirement for redeemSmartCollateral (#2) --- src/broker/LendingBroker.sol | 2 ++ src/broker/interfaces/IBroker.sol | 2 ++ src/liquidator/BrokerLiquidator.sol | 15 +++++++++++---- src/liquidator/IBrokerLiquidator.sol | 2 +- 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/broker/LendingBroker.sol b/src/broker/LendingBroker.sol index d3703a73..2e4497aa 100644 --- a/src/broker/LendingBroker.sol +++ b/src/broker/LendingBroker.sol @@ -1129,6 +1129,7 @@ contract LendingBroker is require(_relayer != address(0), "broker/zero-address-provided"); require(RELAYER == address(0), "broker/already-set"); RELAYER = _relayer; + emit RelayerSet(_relayer); } /** @@ -1139,6 +1140,7 @@ contract LendingBroker is require(_oracle != address(0), "broker/zero-address-provided"); require(address(ORACLE) == address(0), "broker/already-set"); ORACLE = IOracle(_oracle); + emit OracleSet(_oracle); } /** diff --git a/src/broker/interfaces/IBroker.sol b/src/broker/interfaces/IBroker.sol index 4f586e96..2978a980 100644 --- a/src/broker/interfaces/IBroker.sol +++ b/src/broker/interfaces/IBroker.sol @@ -106,6 +106,8 @@ interface IBroker is IBrokerBase { event Liquidated(address indexed user, uint256 principalCleared, uint256 interestCleared); event MarketIdSet(Id marketId); event BorrowPaused(bool paused); + event RelayerSet(address indexed relayer); + event OracleSet(address indexed oracle); event AddedLiquidationWhitelist(address indexed account); event RemovedLiquidationWhitelist(address indexed account); event EmergencyWithdrawn(address indexed sender, address indexed token, uint256 amount); diff --git a/src/liquidator/BrokerLiquidator.sol b/src/liquidator/BrokerLiquidator.sol index 8cf00a93..a2f4d516 100644 --- a/src/liquidator/BrokerLiquidator.sol +++ b/src/liquidator/BrokerLiquidator.sol @@ -82,11 +82,16 @@ contract BrokerLiquidator is UUPSUpgradeable, AccessControlUpgradeable, IBrokerL receive() external payable {} - /// @dev withdraws ERC20 tokens. - /// @param token The address of the token. + /// @dev withdraws ERC20 tokens or native BNB from the contract. + /// @param token The address of the token. Use BNB_ADDRESS (0xEeee...EEeE) for native BNB. /// @param amount The amount to withdraw. - function withdrawERC20(address token, uint256 amount) external onlyRole(MANAGER) { - SafeTransferLib.safeTransfer(token, msg.sender, amount); + function withdraw(address token, uint256 amount) external onlyRole(MANAGER) { + if (token == BNB_ADDRESS) { + (bool success, ) = msg.sender.call{ value: amount }(""); + require(success, "broker-liquidator/native-transfer-failed"); + } else { + SafeTransferLib.safeTransfer(token, msg.sender, amount); + } } /// @dev sets the token whitelist. @@ -488,6 +493,8 @@ contract BrokerLiquidator is UUPSUpgradeable, AccessControlUpgradeable, IBrokerL /// @dev redeems smart collateral LP tokens. /// @param smartProvider The address of the smart collateral provider. /// @param lpAmount The amount of LP collateral tokens to redeem. + /// @notice Redeems LP collateral that is already held by this contract (seized during a prior liquidation step). + /// The SmartProvider burns LP tokens from msg.sender (this contract), so the LP must already be in custody. /// @param minToken0Amt The minimum amount of token0 to receive. /// @param minToken1Amt The minimum amount of token1 to receive. /// @return The amount of token0 and token1 redeemed. diff --git a/src/liquidator/IBrokerLiquidator.sol b/src/liquidator/IBrokerLiquidator.sol index d8a5ede7..bd1aa42d 100644 --- a/src/liquidator/IBrokerLiquidator.sol +++ b/src/liquidator/IBrokerLiquidator.sol @@ -18,7 +18,7 @@ interface IBrokerLiquidator { bytes swapToken0Data; bytes swapToken1Data; } - function withdrawERC20(address token, uint256 amount) external; + function withdraw(address token, uint256 amount) external; function flashLiquidate( bytes32 id, address borrower, From 21bb9eaefd0ce5e8bdedce02ee434c3153c63dd7 Mon Sep 17 00:00:00 2001 From: Rick Date: Fri, 10 Apr 2026 10:21:07 +0800 Subject: [PATCH 07/13] fix: return actual liquidation values in smart collateral liquidation functions - liquidateSmartCollateral: return actual seized (collAmount) and repaid (loanToken balance diff) instead of placeholder values - flashLiquidateSmartCollateral: capture repaidAssets from onMoolahLiquidate callback via _lastRepaidAssets storage --- src/liquidator/BrokerLiquidator.sol | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/liquidator/BrokerLiquidator.sol b/src/liquidator/BrokerLiquidator.sol index a2f4d516..7f19769f 100644 --- a/src/liquidator/BrokerLiquidator.sol +++ b/src/liquidator/BrokerLiquidator.sol @@ -36,6 +36,8 @@ contract BrokerLiquidator is UUPSUpgradeable, AccessControlUpgradeable, IBrokerL mapping(address => bytes32) public brokerToMarketId; // @dev smart collateral provider whitelist mapping(address => bool) public smartProviders; + /// @dev transient storage for repaidAssets from onMoolahLiquidate callback + uint256 internal _lastRepaidAssets; bytes32 public constant MANAGER = keccak256("MANAGER"); // manager role bytes32 public constant BOT = keccak256("BOT"); // manager role @@ -342,6 +344,7 @@ contract BrokerLiquidator is UUPSUpgradeable, AccessControlUpgradeable, IBrokerL address lpToken = ISmartProvider(smartProvider).dexLP(); uint256 collBalanceBefore = IERC20(params.collateralToken).balanceOf(address(this)); + uint256 loanBalanceBefore = IERC20(params.loanToken).balanceOf(address(this)); (uint256 minAmount0, uint256 minAmount1) = abi.decode(payload, (uint256, uint256)); IBrokerBase(broker).liquidate( params, @@ -369,6 +372,7 @@ contract BrokerLiquidator is UUPSUpgradeable, AccessControlUpgradeable, IBrokerL ) ); uint256 collAmount = IERC20(params.collateralToken).balanceOf(address(this)) - collBalanceBefore; + uint256 repaidAssets = loanBalanceBefore - IERC20(params.loanToken).balanceOf(address(this)); require(collAmount > 0, "No collateral seized"); (uint256 amount0, uint256 amount1) = ISmartProvider(smartProvider).redeemLpCollateral( @@ -378,7 +382,8 @@ contract BrokerLiquidator is UUPSUpgradeable, AccessControlUpgradeable, IBrokerL ); emit SmartLiquidation(id, lpToken, params.collateralToken, collAmount, minAmount0, minAmount1, amount0, amount1); - return (seizedAssets, 0); + _lastRepaidAssets = 0; + return (collAmount, repaidAssets); } /// @dev flash liquidates a position with smart collateral. @@ -431,7 +436,9 @@ contract BrokerLiquidator is UUPSUpgradeable, AccessControlUpgradeable, IBrokerL ); IBrokerBase(broker).liquidate(params, borrower, seizedAssets, 0, abi.encode(callback)); - return (seizedAssets, 0); + uint256 repaidAssets = _lastRepaidAssets; + _lastRepaidAssets = 0; + return (seizedAssets, repaidAssets); } /// @dev the function will be called by the the Broker, when Broker's onMoolahLiquidate is called by Moolah. @@ -487,6 +494,7 @@ contract BrokerLiquidator is UUPSUpgradeable, AccessControlUpgradeable, IBrokerL if (token1 != BNB_ADDRESS) SafeTransferLib.safeApprove(token1, arb.token1Pair, 0); } + _lastRepaidAssets = repaidAssets; SafeTransferLib.safeApprove(arb.loanToken, msg.sender, repaidAssets); } From def0edac39105036bd634989e2060ceb13792a04 Mon Sep 17 00:00:00 2001 From: Rick Date: Fri, 10 Apr 2026 14:10:15 +0800 Subject: [PATCH 08/13] fix: split withdraw into withdrawERC20 and withdrawETH in BrokerLiquidator --- src/liquidator/BrokerLiquidator.sol | 18 +++++++++--------- src/liquidator/IBrokerLiquidator.sol | 3 ++- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/liquidator/BrokerLiquidator.sol b/src/liquidator/BrokerLiquidator.sol index 7f19769f..6cd8926c 100644 --- a/src/liquidator/BrokerLiquidator.sol +++ b/src/liquidator/BrokerLiquidator.sol @@ -84,16 +84,16 @@ contract BrokerLiquidator is UUPSUpgradeable, AccessControlUpgradeable, IBrokerL receive() external payable {} - /// @dev withdraws ERC20 tokens or native BNB from the contract. - /// @param token The address of the token. Use BNB_ADDRESS (0xEeee...EEeE) for native BNB. + /// @dev withdraws ERC20 tokens. + /// @param token The address of the token. /// @param amount The amount to withdraw. - function withdraw(address token, uint256 amount) external onlyRole(MANAGER) { - if (token == BNB_ADDRESS) { - (bool success, ) = msg.sender.call{ value: amount }(""); - require(success, "broker-liquidator/native-transfer-failed"); - } else { - SafeTransferLib.safeTransfer(token, msg.sender, amount); - } + function withdrawERC20(address token, uint256 amount) external onlyRole(MANAGER) { + SafeTransferLib.safeTransfer(token, msg.sender, amount); + } + /// @dev withdraws ETH. + /// @param amount The amount to withdraw. + function withdrawETH(uint256 amount) external onlyRole(MANAGER) { + SafeTransferLib.safeTransferETH(msg.sender, amount); } /// @dev sets the token whitelist. diff --git a/src/liquidator/IBrokerLiquidator.sol b/src/liquidator/IBrokerLiquidator.sol index bd1aa42d..0c41378c 100644 --- a/src/liquidator/IBrokerLiquidator.sol +++ b/src/liquidator/IBrokerLiquidator.sol @@ -18,7 +18,8 @@ interface IBrokerLiquidator { bytes swapToken0Data; bytes swapToken1Data; } - function withdraw(address token, uint256 amount) external; + function withdrawERC20(address token, uint256 amount) external; + function withdrawETH(uint256 amount) external; function flashLiquidate( bytes32 id, address borrower, From 01ab54646509b2fcc0d2f29132281842e4252465 Mon Sep 17 00:00:00 2001 From: Rick Date: Thu, 16 Apr 2026 11:42:02 +0800 Subject: [PATCH 09/13] fix: simplify interest reset in deductFixedPositionDebt during liquidation Remove partial interest tracking logic and always reset interestRepaid and lastRepaidTime after liquidation deduction. --- src/broker/libraries/BrokerMath.sol | 25 ++++--------------------- 1 file changed, 4 insertions(+), 21 deletions(-) diff --git a/src/broker/libraries/BrokerMath.sol b/src/broker/libraries/BrokerMath.sol index 3b3b7b47..c0ae5103 100644 --- a/src/broker/libraries/BrokerMath.sol +++ b/src/broker/libraries/BrokerMath.sol @@ -546,27 +546,10 @@ library BrokerMath { // update repaid principal amount principalToDeduct -= repayPrincipalAmt; p.principalRepaid += repayPrincipalAmt; - - if (repayInterestAmt >= accruedInterest) { - // all accrued interest fully covered -> safe to reset tracking - p.interestRepaid = 0; - p.lastRepaidTime = block.timestamp; - } else { - // partial interest covered -> adjust interestRepaid to preserve outstanding - // After principalRepaid increased, getAccruedInterestForFixedPosition() recalculates - // with smaller (principal - principalRepaid), producing a lower total (newTotalAccrued). - // We set interestRepaid so that: newTotalAccrued - interestRepaid = unpaidInterest - uint256 unpaidInterest = accruedInterest - repayInterestAmt; - uint256 newTotalAccrued = getAccruedInterestForFixedPosition(p); - if (newTotalAccrued >= unpaidInterest) { - // exact: outstanding is perfectly preserved - p.interestRepaid = newTotalAccrued - unpaidInterest; - } else { - // edge case: most principal repaid, formula can't represent full outstanding - // preserve maximum possible: outstanding = newTotalAccrued (don't reset lastRepaidTime) - p.interestRepaid = 0; - } - } + // reset repaid interest to zero (all accrued interest has been cleared) + p.interestRepaid = 0; + // reset repaid time to now + p.lastRepaidTime = block.timestamp; } return (interestToDeduct, principalToDeduct, p); From 7daf76ffb68b846cb1eb1338939da127c01fae2f Mon Sep 17 00:00:00 2001 From: Rick Date: Thu, 16 Apr 2026 12:02:58 +0800 Subject: [PATCH 10/13] fix: convertDynamicToFixed prioritizes interest then principal deduction Amount now clears interest first, then principal. New fixed position principal <= amount. Also fix constructor args after rebase and add exact-full and excess-capped conversion tests. --- script/broker/deploy_broker_20260408.s.sol | 2 +- src/broker/LendingBroker.sol | 15 +-- test/broker/LendingBroker.t.sol | 117 +++++++++++++++++---- test/utils/PositionManager.t.sol | 42 ++++++-- 4 files changed, 139 insertions(+), 37 deletions(-) diff --git a/script/broker/deploy_broker_20260408.s.sol b/script/broker/deploy_broker_20260408.s.sol index 168829dd..789c7c06 100644 --- a/script/broker/deploy_broker_20260408.s.sol +++ b/script/broker/deploy_broker_20260408.s.sol @@ -55,7 +55,7 @@ contract DeployXautBrokers is DeployBase { function _deployBroker(string memory label, address relayer, address oracle, address deployer) internal { // Deploy implementation - LendingBroker impl = new LendingBroker(MOOLAH, relayer, oracle, address(0)); + LendingBroker impl = new LendingBroker(MOOLAH, address(0)); console.log(string.concat("LendingBroker(", label, ") impl: "), address(impl)); // Deploy proxy diff --git a/src/broker/LendingBroker.sol b/src/broker/LendingBroker.sol index 2e4497aa..128eaa3e 100644 --- a/src/broker/LendingBroker.sol +++ b/src/broker/LendingBroker.sol @@ -439,15 +439,16 @@ contract LendingBroker is address user = msg.sender; DynamicLoanPosition storage position = dynamicLoanPositions[user]; if (fixedLoanPositions[user].length >= maxFixedLoanPositions) revert ExceedMaxFixedPositions(); - // cap amount by principal - amount = UtilsLib.min(amount, position.principal); // accrue current rate so normalized debt reflects the latest interest uint256 rate = IRateCalculator(rateCalculator).accrueRate(address(this)); uint256 actualDebt = BrokerMath.denormalizeBorrowAmount(position.normalizedDebt, rate); uint256 totalInterest = actualDebt.zeroFloorSub(position.principal); - // force user to repay interest portion when converting to fixed - uint256 interestToRepay = BrokerMath.mulDivCeiling(amount, totalInterest, position.principal); + // prioritize clearing interest first, then principal + uint256 interestToRepay = UtilsLib.min(amount, totalInterest); + uint256 principalToMove = UtilsLib.min(amount - interestToRepay, position.principal); + amount = interestToRepay + principalToMove; + if (interestToRepay > 0) { // borrow from Moolah to increase user's actual debt at moolah _borrowFromMoolah(user, interestToRepay); @@ -456,9 +457,9 @@ contract LendingBroker is } position.normalizedDebt = position.normalizedDebt.zeroFloorSub( - BrokerMath.normalizeBorrowAmount(amount + interestToRepay, rate, false) + BrokerMath.normalizeBorrowAmount(amount, rate, false) ); - position.principal -= amount; + position.principal -= principalToMove; if (position.principal == 0) { delete dynamicLoanPositions[user]; @@ -473,7 +474,7 @@ contract LendingBroker is fixedLoanPositions[user].push( FixedLoanPosition({ posId: fixedPosUuid, - principal: amount + interestToRepay, + principal: amount, apr: term.apr, start: start, end: end, diff --git a/test/broker/LendingBroker.t.sol b/test/broker/LendingBroker.t.sol index 826a029a..8abab106 100644 --- a/test/broker/LendingBroker.t.sol +++ b/test/broker/LendingBroker.t.sol @@ -199,10 +199,20 @@ contract LendingBrokerTest is Test { ); broker = LendingBroker(payable(address(bProxy))); - LendingBroker bnbImpl = new LendingBroker(address(moolah), address(bnbRelayer), address(oracle), address(WBNB)); + LendingBroker bnbImpl = new LendingBroker(address(moolah), address(WBNB)); ERC1967Proxy bnbProxy = new ERC1967Proxy( address(bnbImpl), - abi.encodeWithSelector(LendingBroker.initialize.selector, ADMIN, MANAGER, BOT, PAUSER, address(rateCalc), 10) + abi.encodeWithSelector( + LendingBroker.initialize.selector, + ADMIN, + MANAGER, + BOT, + PAUSER, + address(rateCalc), + 10, + address(bnbRelayer), + address(oracle) + ) ); bnbBroker = LendingBroker(payable(address(bnbProxy))); @@ -686,15 +696,11 @@ contract LendingBrokerTest is Test { uint256 actualDebt = BrokerMath.denormalizeBorrowAmount(normalizedBefore, rate); uint256 outstandingInterest = actualDebt > principalBefore ? actualDebt - principalBefore : 0; + // amount > interest => interest fully cleared, remainder moves principal uint256 convertAmount = 400 ether; - uint256 expectedInterestShare = outstandingInterest == 0 - ? 0 - : BrokerMath.mulDivCeiling(outstandingInterest, convertAmount, principalBefore); - uint256 expectedNormalizedDelta = BrokerMath.normalizeBorrowAmount( - convertAmount + expectedInterestShare, - rate, - true - ); + uint256 expectedInterest = outstandingInterest < convertAmount ? outstandingInterest : convertAmount; + uint256 expectedPrincipalMove = convertAmount - expectedInterest; + uint256 expectedNormalizedDelta = BrokerMath.normalizeBorrowAmount(convertAmount, rate, true); uint256 expectedNormalizedAfter = normalizedBefore > expectedNormalizedDelta ? normalizedBefore - expectedNormalizedDelta : 0; @@ -703,12 +709,12 @@ contract LendingBrokerTest is Test { broker.convertDynamicToFixed(convertAmount, 51); (uint256 principalAfter, uint256 normalizedAfter) = broker.dynamicLoanPositions(borrower); - assertEq(principalAfter, principalBefore - convertAmount, "dynamic principal not reduced by amount"); + assertEq(principalAfter, principalBefore - expectedPrincipalMove, "dynamic principal not reduced correctly"); assertApproxEqAbs(normalizedAfter, expectedNormalizedAfter, 1, "normalized debt delta mismatch"); FixedLoanPosition[] memory fixedPositions = broker.userFixedPositions(borrower); assertEq(fixedPositions.length, 1, "fixed position not created"); - assertEq(fixedPositions[0].principal, convertAmount + expectedInterestShare, "converted fixed principal incorrect"); + assertEq(fixedPositions[0].principal, convertAmount, "fixed principal should equal amount"); assertEq(fixedPositions[0].interestRepaid, 0); assertEq(fixedPositions[0].principalRepaid, 0); } @@ -732,10 +738,10 @@ contract LendingBrokerTest is Test { uint256 rate = rateCalc.accrueRate(address(broker)); uint256 actualDebt = BrokerMath.denormalizeBorrowAmount(normalizedBefore, rate); uint256 outstandingInterest = actualDebt > principalBefore ? actualDebt - principalBefore : 0; - uint256 expectedNormalizedDelta = BrokerMath.normalizeBorrowAmount(actualDebt, rate, true); + // pass amount > actualDebt => capped to interest + principal vm.prank(borrower); - broker.convertDynamicToFixed(principalBefore, 52); + broker.convertDynamicToFixed(actualDebt + 100 ether, 52); (uint256 principalAfter, uint256 normalizedAfter) = broker.dynamicLoanPositions(borrower); assertApproxEqAbs(principalAfter, 0, 1, "dynamic principal should be cleared"); @@ -743,15 +749,80 @@ contract LendingBrokerTest is Test { FixedLoanPosition[] memory fixedPositions = broker.userFixedPositions(borrower); assertEq(fixedPositions.length, 1); - assertApproxEqAbs( - fixedPositions[0].principal, - principalBefore + outstandingInterest, - 1, - "fixed principal should equal full outstanding debt" - ); + uint256 expectedFixed = outstandingInterest + principalBefore; + assertApproxEqAbs(fixedPositions[0].principal, expectedFixed, 1, "fixed principal should equal full debt"); + } + + function test_convertDynamicToFixed_exactFullAmount() public { + FixedTermAndRate memory term = FixedTermAndRate({ termId: 53, duration: 60 days, apr: 105 * 1e25 }); + vm.prank(BOT); + broker.updateFixedTermAndRate(term, false); + + uint256 borrowAmt = 500 ether; + vm.prank(borrower); + broker.borrow(borrowAmt); + + vm.prank(MANAGER); + rateCalc.setMaxRatePerSecond(address(broker), RATE_SCALE + 5); + vm.prank(BOT); + rateCalc.setRatePerSecond(address(broker), RATE_SCALE + 3); + skip(4 days); - // sanity: normalized delta consumed the whole normalized debt (allowing rounding wiggle) - assertApproxEqAbs(expectedNormalizedDelta, normalizedBefore, 1, "normalized debt delta rounding"); + (uint256 principalBefore, uint256 normalizedBefore) = broker.dynamicLoanPositions(borrower); + uint256 rate = rateCalc.accrueRate(address(broker)); + uint256 actualDebt = BrokerMath.denormalizeBorrowAmount(normalizedBefore, rate); + uint256 outstandingInterest = actualDebt > principalBefore ? actualDebt - principalBefore : 0; + + // amount == interest + principal exactly + uint256 convertAmount = outstandingInterest + principalBefore; + + vm.prank(borrower); + broker.convertDynamicToFixed(convertAmount, 53); + + (uint256 principalAfter, uint256 normalizedAfter) = broker.dynamicLoanPositions(borrower); + assertApproxEqAbs(principalAfter, 0, 1, "dynamic principal should be cleared"); + assertApproxEqAbs(normalizedAfter, 0, 1, "dynamic normalized debt should be cleared"); + + FixedLoanPosition[] memory fixedPositions = broker.userFixedPositions(borrower); + assertEq(fixedPositions.length, 1); + assertEq(fixedPositions[0].principal, convertAmount, "fixed principal should equal amount"); + } + + function test_convertDynamicToFixed_excessAmountCapped() public { + FixedTermAndRate memory term = FixedTermAndRate({ termId: 54, duration: 30 days, apr: 105 * 1e25 }); + vm.prank(BOT); + broker.updateFixedTermAndRate(term, false); + + uint256 borrowAmt = 600 ether; + vm.prank(borrower); + broker.borrow(borrowAmt); + + vm.prank(MANAGER); + rateCalc.setMaxRatePerSecond(address(broker), RATE_SCALE + 5); + vm.prank(BOT); + rateCalc.setRatePerSecond(address(broker), RATE_SCALE + 3); + skip(3 days); + + (uint256 principalBefore, uint256 normalizedBefore) = broker.dynamicLoanPositions(borrower); + uint256 rate = rateCalc.accrueRate(address(broker)); + uint256 actualDebt = BrokerMath.denormalizeBorrowAmount(normalizedBefore, rate); + uint256 outstandingInterest = actualDebt > principalBefore ? actualDebt - principalBefore : 0; + + // amount much larger than actualDebt => should be capped + uint256 convertAmount = actualDebt + 999 ether; + + vm.prank(borrower); + broker.convertDynamicToFixed(convertAmount, 54); + + (uint256 principalAfter, uint256 normalizedAfter) = broker.dynamicLoanPositions(borrower); + assertApproxEqAbs(principalAfter, 0, 1, "dynamic principal should be cleared"); + assertApproxEqAbs(normalizedAfter, 0, 1, "dynamic normalized debt should be cleared"); + + FixedLoanPosition[] memory fixedPositions = broker.userFixedPositions(borrower); + assertEq(fixedPositions.length, 1); + uint256 expectedFixed = outstandingInterest + principalBefore; + assertApproxEqAbs(fixedPositions[0].principal, expectedFixed, 1, "fixed principal capped to actual debt"); + assertLe(fixedPositions[0].principal, convertAmount, "fixed principal must be <= amount"); } // ----------------------------- @@ -2214,7 +2285,7 @@ contract LendingBrokerTest is Test { /// @dev Deploy a fresh broker proxy with RELAYER/ORACLE unset (simulating V1->V2 upgrade) function _deployBrokerWithEmptyRelayerOracle() internal returns (LendingBroker) { - LendingBroker bImpl = new LendingBroker(address(moolah)); + LendingBroker bImpl = new LendingBroker(address(moolah), address(0)); // Use 6-param initialize (no relayer/oracle) by encoding only the original params // and leaving RELAYER/ORACLE as address(0) ERC1967Proxy bProxy = new ERC1967Proxy( diff --git a/test/utils/PositionManager.t.sol b/test/utils/PositionManager.t.sol index 412368de..ec5e1be3 100644 --- a/test/utils/PositionManager.t.sol +++ b/test/utils/PositionManager.t.sol @@ -230,10 +230,20 @@ contract PositionManagerTest is Test { rateCalc = RateCalculator(address(rcProxy)); // ── Deploy inBroker (LendingBroker for the fixed-term market) ──────────── - LendingBroker bImpl = new LendingBroker(address(moolah), address(relayer), address(oracle), address(wbnb)); + LendingBroker bImpl = new LendingBroker(address(moolah), address(wbnb)); ERC1967Proxy bProxy = new ERC1967Proxy( address(bImpl), - abi.encodeWithSelector(LendingBroker.initialize.selector, admin, manager, bot, pauser, address(rateCalc), 100) + abi.encodeWithSelector( + LendingBroker.initialize.selector, + admin, + manager, + bot, + pauser, + address(rateCalc), + 100, + address(relayer), + address(oracle) + ) ); inBroker = LendingBroker(payable(address(bProxy))); @@ -312,10 +322,20 @@ contract PositionManagerTest is Test { // ── Deploy inBrokerSlis ─────────────────────────────────────────────────── { - LendingBroker bSlisImpl = new LendingBroker(address(moolah), address(relayer), address(oracle), address(wbnb)); + LendingBroker bSlisImpl = new LendingBroker(address(moolah), address(wbnb)); ERC1967Proxy bSlisProxy = new ERC1967Proxy( address(bSlisImpl), - abi.encodeWithSelector(LendingBroker.initialize.selector, admin, manager, bot, pauser, address(rateCalc), 100) + abi.encodeWithSelector( + LendingBroker.initialize.selector, + admin, + manager, + bot, + pauser, + address(rateCalc), + 100, + address(relayer), + address(oracle) + ) ); inBrokerSlis = LendingBroker(payable(address(bSlisProxy))); } @@ -399,10 +419,20 @@ contract PositionManagerTest is Test { // ── Deploy inBrokerNative ───────────────────────────────────────────────── { - LendingBroker bNativeImpl = new LendingBroker(address(moolah), address(relayer), address(oracle), address(wbnb)); + LendingBroker bNativeImpl = new LendingBroker(address(moolah), address(wbnb)); ERC1967Proxy bNativeProxy = new ERC1967Proxy( address(bNativeImpl), - abi.encodeWithSelector(LendingBroker.initialize.selector, admin, manager, bot, pauser, address(rateCalc), 10) + abi.encodeWithSelector( + LendingBroker.initialize.selector, + admin, + manager, + bot, + pauser, + address(rateCalc), + 10, + address(relayer), + address(oracle) + ) ); inBrokerNative = LendingBroker(payable(address(bNativeProxy))); } From 6174dd264c604b873e301a6286ca85aa9485a62c Mon Sep 17 00:00:00 2001 From: Rick Date: Thu, 16 Apr 2026 17:33:44 +0800 Subject: [PATCH 11/13] fix: address audit findings for BrokerLiquidator SmartProvider support - [L03] Block smart collateral markets from being liquidated via normal liquidate() by adding _isSmartCollateral() check that detects StableSwapLPCollateral via minter() -> TOKEN() chain - [I01] Add sellBNB() function for selling native BNB received from smart collateral redemptions, mirroring Liquidator.sol - [I05] Add zero address check in batchSetSmartProviders() - [I07] Fix incorrect comment on BOT role constant --- src/liquidator/BrokerLiquidator.sol | 23 ++- test/liquidator/BrokerLiquidator.t.sol | 153 ++++++++++++++++++ .../mocks/MockStableSwapLPCollateral.sol | 17 ++ 3 files changed, 192 insertions(+), 1 deletion(-) create mode 100644 test/liquidator/mocks/MockStableSwapLPCollateral.sol diff --git a/src/liquidator/BrokerLiquidator.sol b/src/liquidator/BrokerLiquidator.sol index 6cd8926c..a8e93039 100644 --- a/src/liquidator/BrokerLiquidator.sol +++ b/src/liquidator/BrokerLiquidator.sol @@ -11,6 +11,10 @@ import { Id, MarketParams, IMoolah } from "moolah/interfaces/IMoolah.sol"; import { IBrokerLiquidator } from "./IBrokerLiquidator.sol"; import { ISmartProvider } from "../provider/interfaces/IProvider.sol"; +interface IHasMinter { + function minter() external view returns (address); +} + contract BrokerLiquidator is UUPSUpgradeable, AccessControlUpgradeable, IBrokerLiquidator { using MarketParamsLib for MarketParams; @@ -23,6 +27,7 @@ contract BrokerLiquidator is UUPSUpgradeable, AccessControlUpgradeable, IBrokerL error NotWhitelisted(); error SwapFailed(); error BrokerMarketIdMismatch(); + error SmartCollateralMustUseDedicatedFunction(); address public immutable MOOLAH; mapping(address => bool) public tokenWhitelist; @@ -40,7 +45,7 @@ contract BrokerLiquidator is UUPSUpgradeable, AccessControlUpgradeable, IBrokerL uint256 internal _lastRepaidAssets; bytes32 public constant MANAGER = keccak256("MANAGER"); // manager role - bytes32 public constant BOT = keccak256("BOT"); // manager role + bytes32 public constant BOT = keccak256("BOT"); // bot role /// @dev sentinel address representing native BNB address public constant BNB_ADDRESS = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE; @@ -292,6 +297,7 @@ contract BrokerLiquidator is UUPSUpgradeable, AccessControlUpgradeable, IBrokerL require(broker != address(0), NotWhitelisted()); require(_checkBrokerMarketId(broker, id), BrokerMarketIdMismatch()); MarketParams memory params = IMoolah(MOOLAH).idToMarketParams(Id.wrap(id)); + require(!_isSmartCollateral(params.collateralToken), SmartCollateralMustUseDedicatedFunction()); IBrokerBase(broker).liquidate( params, borrower, @@ -522,11 +528,26 @@ contract BrokerLiquidator is UUPSUpgradeable, AccessControlUpgradeable, IBrokerL function batchSetSmartProviders(address[] calldata providers, bool status) external onlyRole(MANAGER) { for (uint256 i = 0; i < providers.length; i++) { address provider = providers[i]; + require(provider != address(0), ZERO_ADDRESS); smartProviders[provider] = status; emit SmartProvidersChanged(provider, status); } } + /// @dev Checks if a collateral token is a SmartCollateral (StableSwapLPCollateral). + /// Uses try/catch so it won't revert for normal collateral tokens that lack minter(). + function _isSmartCollateral(address collateralToken) internal view returns (bool) { + try IHasMinter(collateralToken).minter() returns (address minterAddr) { + try ISmartProvider(minterAddr).TOKEN() returns (address token) { + return token == collateralToken; + } catch { + return false; + } + } catch { + return false; + } + } + function _authorizeUpgrade(address newImplementation) internal override onlyRole(DEFAULT_ADMIN_ROLE) {} /// @dev allows the contract to receive native BNB, e.g. when redeeming slisBNB/BNB LP. diff --git a/test/liquidator/BrokerLiquidator.t.sol b/test/liquidator/BrokerLiquidator.t.sol index f23e4432..e2be8863 100644 --- a/test/liquidator/BrokerLiquidator.t.sol +++ b/test/liquidator/BrokerLiquidator.t.sol @@ -6,6 +6,8 @@ import "../moolah/BaseTest.sol"; import { BrokerLiquidator, IBrokerLiquidator } from "liquidator/BrokerLiquidator.sol"; import { MarketParamsLib, MarketParams, Id } from "moolah/libraries/MarketParamsLib.sol"; import { MockSmartProvider } from "./mocks/MockSmartProvider.sol"; +import { MockStableSwapLPCollateral } from "./mocks/MockStableSwapLPCollateral.sol"; +import { MockOneInch } from "./mocks/MockOneInch.sol"; contract BrokerLiquidatorTest is BaseTest { using MarketParamsLib for MarketParams; @@ -144,4 +146,155 @@ contract BrokerLiquidatorTest is BaseTest { vm.expectRevert(BrokerLiquidator.NotWhitelisted.selector); brokerLiquidator.redeemSmartCollateral(address(smartProvider), 1e18, 0, 0); } + + // ==================== batchSetSmartProviders zero address ==================== + + function testBatchSetSmartProvidersRevertsOnZeroAddress() public { + address[] memory providers = new address[](2); + providers[0] = address(smartProvider); + providers[1] = address(0); + + vm.prank(MANAGER_ADDR); + vm.expectRevert("zero address"); + brokerLiquidator.batchSetSmartProviders(providers, true); + } + + // ==================== _isSmartCollateral (via liquidate) ==================== + + function testLiquidateRevertsForSmartCollateral() public { + // Create a MockSmartProvider whose TOKEN() returns a MockStableSwapLPCollateral + // and the collateral's minter() returns the MockSmartProvider + MockStableSwapLPCollateral mockLPCollateral = new MockStableSwapLPCollateral( + "MockLP", + "MLP", + address(smartProvider) + ); + // Configure smartProvider so TOKEN() returns the LP collateral address + smartProvider.setCollateralToken(address(mockLPCollateral)); + + // Create a market with this LP collateral + MarketParams memory smartMarketParams = MarketParams({ + loanToken: address(loanToken), + collateralToken: address(mockLPCollateral), + oracle: address(oracle), + irm: address(irm), + lltv: 0.8e18 + }); + moolah.createMarket(smartMarketParams); + bytes32 smartMarketId = Id.unwrap(smartMarketParams.id()); + + // Deploy a mock broker that returns the correct MARKET_ID + MockBrokerForLiquidation mockBroker = new MockBrokerForLiquidation(smartMarketParams.id()); + + // Mock the brokers call so whitelist validation passes + vm.mockCall( + address(moolah), + abi.encodeWithSelector(moolah.brokers.selector, smartMarketParams.id()), + abi.encode(address(mockBroker)) + ); + + // Whitelist the market + vm.prank(MANAGER_ADDR); + brokerLiquidator.setMarketToBroker(smartMarketId, address(mockBroker), true); + + // Attempt to liquidate should revert with SmartCollateralMustUseDedicatedFunction + vm.prank(BOT); + vm.expectRevert(BrokerLiquidator.SmartCollateralMustUseDedicatedFunction.selector); + brokerLiquidator.liquidate(smartMarketId, address(1), 1e18, 0); + } + + function testLiquidateAllowsNormalCollateral() public { + // Normal collateral (ERC20Mock) does not have minter(), so _isSmartCollateral returns false + bytes32 normalMarketId = Id.unwrap(marketParams.id()); + + MockBrokerForLiquidation mockBroker = new MockBrokerForLiquidation(marketParams.id()); + + vm.mockCall( + address(moolah), + abi.encodeWithSelector(moolah.brokers.selector, marketParams.id()), + abi.encode(address(mockBroker)) + ); + + vm.prank(MANAGER_ADDR); + brokerLiquidator.setMarketToBroker(normalMarketId, address(mockBroker), true); + + // Should NOT revert with SmartCollateralMustUseDedicatedFunction + // It will revert inside broker.liquidate (mock is a no-op), but the smart collateral check passes + vm.prank(BOT); + brokerLiquidator.liquidate(normalMarketId, address(1), 1e18, 0); + // If we get here, the _isSmartCollateral check did not block normal collateral + } + + // ==================== sellBNB ==================== + + function testSellBNB() public { + MockOneInch mockDex = new MockOneInch(); + address BNB_ADDRESS = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE; + + // Whitelist BNB, loanToken, and the pair + vm.startPrank(MANAGER_ADDR); + brokerLiquidator.setTokenWhitelist(BNB_ADDRESS, true); + brokerLiquidator.setTokenWhitelist(address(loanToken), true); + brokerLiquidator.setPairWhitelist(address(mockDex), true); + vm.stopPrank(); + + // Fund the liquidator with BNB + uint256 amountIn = 1 ether; + uint256 amountOutMin = 2000e18; + deal(address(brokerLiquidator), amountIn); + + bytes memory swapData = abi.encodeWithSelector( + mockDex.swap.selector, + BNB_ADDRESS, + address(loanToken), + amountIn, + amountOutMin + ); + + vm.prank(BOT); + brokerLiquidator.sellBNB(address(mockDex), address(loanToken), amountIn, amountOutMin, swapData); + + assertEq(address(brokerLiquidator).balance, 0); + assertEq(loanToken.balanceOf(address(brokerLiquidator)), amountOutMin); + } + + function testSellBNBRevertsIfNotBot() public { + vm.prank(MANAGER_ADDR); + vm.expectRevert(); + brokerLiquidator.sellBNB(address(1), address(loanToken), 1 ether, 0, ""); + } + + function testSellBNBRevertsIfBNBNotWhitelisted() public { + vm.prank(BOT); + vm.expectRevert(BrokerLiquidator.NotWhitelisted.selector); + brokerLiquidator.sellBNB(address(1), address(loanToken), 1 ether, 0, ""); + } + + function testSellBNBRevertsIfInsufficientBalance() public { + address BNB_ADDRESS = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE; + + vm.startPrank(MANAGER_ADDR); + brokerLiquidator.setTokenWhitelist(BNB_ADDRESS, true); + brokerLiquidator.setTokenWhitelist(address(loanToken), true); + brokerLiquidator.setPairWhitelist(address(1), true); + vm.stopPrank(); + + // No BNB in contract + vm.prank(BOT); + vm.expectRevert(BrokerLiquidator.ExceedAmount.selector); + brokerLiquidator.sellBNB(address(1), address(loanToken), 1 ether, 0, ""); + } +} + +/// @dev Minimal mock broker that returns a MARKET_ID for whitelist validation +contract MockBrokerForLiquidation { + Id public immutable MARKET_ID; + + constructor(Id _marketId) { + MARKET_ID = _marketId; + } + + function liquidate(MarketParams memory, address, uint256, uint256, bytes calldata) external { + // no-op for testing + } } diff --git a/test/liquidator/mocks/MockStableSwapLPCollateral.sol b/test/liquidator/mocks/MockStableSwapLPCollateral.sol new file mode 100644 index 00000000..aa8308e5 --- /dev/null +++ b/test/liquidator/mocks/MockStableSwapLPCollateral.sol @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.34; + +import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; + +/// @dev Minimal mock that exposes minter(), like the real StableSwapLPCollateral. +contract MockStableSwapLPCollateral is ERC20 { + address public minter; + + constructor(string memory name, string memory symbol, address _minter) ERC20(name, symbol) { + minter = _minter; + } + + function setMinter(address _minter) external { + minter = _minter; + } +} From 770128016a4a10cbe89f38bfdf7084fc132ca074 Mon Sep 17 00:00:00 2001 From: Rick Date: Mon, 11 May 2026 15:18:27 +0800 Subject: [PATCH 12/13] fix: address Lista Fixed Term audit findings on LendingBroker LendingBroker - Add repayAll(onBehalf) for full position clearing, gated by whenNotPaused. - Replace global checkPositionsMeetsMinLoan with per-position validators (_validateDynamicPosition, _validateFixedPosition); remove the post-cascade global check from onMoolahLiquidate so partial liquidations no longer revert when a single position lands in the (0, minLoan) band. - Reject non-divisible repaidShares in liquidate() to preserve the broker market's totalBorrowAssets:totalBorrowShares = 1:VIRTUAL_SHARES invariant. - Re-check amount == 0 after interest/principal clamping in convertDynamicToFixed to prevent zero-principal fixed positions when the user has no dynamic debt. - Add docstring notes that fixed-repay amount and liquidate caller-whitelist / cascade ordering match the actual behavior. Size optimization - Move repay / repayAll bodies into LendingBrokerOperatorLib (DELEGATECALL'd). - Move liquidation cascade orchestration to BrokerMath.executeLiquidationCascade. - Move repayAll / convertDynamicToFixed pre-compute math to BrokerMath helpers. - Consolidate duplicate errors and dead helpers. LendingBroker shrinks from 27,358 -> 22,462 bytes (back under EIP-170). Addresses audit findings: H1 (loan-token price drift stranding positions), M (positions stranded by minLoanValue change), M (liquidation cascade dead zone revert), M (non-divisible share liquidation drift), Info (zero-principal fixed via convert), Low (_validatePositions stricter than Moolah post-liq check), Info (fixed-repay natspec), Info (liquidate natspec outdated). Generated with Claude Code --- src/broker/LendingBroker.sol | 444 ++++------------- src/broker/interfaces/IBroker.sol | 6 + src/broker/libraries/BrokerMath.sol | 155 ++++-- .../libraries/LendingBrokerOperatorLib.sol | 365 ++++++++++++++ test/broker/BrokerMathDeductFixed.t.sol | 124 ++--- test/broker/LendingBroker.t.sol | 456 +++++++++++++++++- 6 files changed, 1113 insertions(+), 437 deletions(-) create mode 100644 src/broker/libraries/LendingBrokerOperatorLib.sol diff --git a/src/broker/LendingBroker.sol b/src/broker/LendingBroker.sol index 128eaa3e..84de558e 100644 --- a/src/broker/LendingBroker.sol +++ b/src/broker/LendingBroker.sol @@ -13,6 +13,7 @@ import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol"; import { IBroker, FixedLoanPosition, DynamicLoanPosition, FixedTermAndRate, LiquidationContext } from "./interfaces/IBroker.sol"; import { IRateCalculator } from "./interfaces/IRateCalculator.sol"; import { BrokerMath, RATE_SCALE } from "./libraries/BrokerMath.sol"; +import { LendingBrokerOperatorLib } from "./libraries/LendingBrokerOperatorLib.sol"; import { IBrokerInterestRelayer } from "./interfaces/IBrokerInterestRelayer.sol"; import { MarketParamsLib } from "../moolah/libraries/MarketParamsLib.sol"; @@ -43,9 +44,7 @@ contract LendingBroker is using EnumerableSet for EnumerableSet.AddressSet; // ------- Custom Errors ------- - error ZeroAddressProvided(); error ZeroAmount(); - error AmountZero(); error NativeNotSupported(); error ZeroAddress(); error NothingToRepay(); @@ -56,19 +55,20 @@ contract LendingBroker is error UnsupportedToken(); error ZeroPositions(); error InvalidBorrowedAmount(); + error InsufficientAmount(); error NativeTransferFailed(); error InvalidMarket(); error InvalidTermId(); error InvalidDuration(); error InvalidAPR(); error SameValueProvided(); - error TransferFailed(); error TermNotFound(); error PositionNotFound(); error NotAuthorized(); error NotMoolah(); error MarketNotSet(); error BorrowIsPaused(); + error InvalidRepaidShares(); // ------- Roles ------- bytes32 public constant MANAGER = keccak256("MANAGER"); @@ -142,7 +142,7 @@ contract LendingBroker is * @param wbnb The address of the wrapped native token (e.g. WBNB). Pass address(0) to disable native support. */ constructor(address moolah, address wbnb) { - if (moolah == address(0)) revert ZeroAddressProvided(); + if (moolah == address(0)) revert ZeroAddress(); MOOLAH = IMoolah(moolah); WBNB = wbnb; _disableInitializers(); @@ -178,7 +178,7 @@ contract LendingBroker is _maxFixedLoanPositions == 0 || _relayer == address(0) || _oracle == address(0) - ) revert ZeroAddressProvided(); + ) revert ZeroAddress(); __AccessControlEnumerable_init(); __Pausable_init(); @@ -218,8 +218,8 @@ contract LendingBroker is _borrowFromMoolah(user, amount); // transfer loan token to user _transferLoanToken(payable(user), amount); - // validate positions - _validatePositions(user); + // validate the modified dynamic position + _validateDynamicPosition(user); // emit event emit DynamicLoanPositionBorrowed(user, amount, position.principal); } @@ -235,7 +235,7 @@ contract LendingBroker is uint256 amount, uint256 termId ) external override marketIdSet whenNotPaused whenBorrowNotPaused nonReentrant { - if (amount == 0) revert AmountZero(); + if (amount == 0) revert ZeroAmount(); address user = msg.sender; _borrowFixed(user, amount, termId); // transfer loan token to user (unwraps to native BNB if supported) @@ -248,82 +248,13 @@ contract LendingBroker is * @param onBehalf The address of the user whose position to repay */ function repay(uint256 amount, address onBehalf) external payable override marketIdSet whenNotPaused nonReentrant { - bool isNative = msg.value > 0; - address user = msg.sender; - if (isNative) { - if (LOAN_TOKEN != WBNB) revert NativeNotSupported(); - amount = msg.value; - IWBNB(WBNB).deposit{ value: amount }(); - } else { - IERC20(LOAN_TOKEN).safeTransferFrom(user, address(this), amount); - } - if (amount == 0) revert ZeroAmount(); - if (onBehalf == address(0)) revert ZeroAddress(); - // get user's dynamic position - DynamicLoanPosition storage position = dynamicLoanPositions[onBehalf]; - // get updated rate - uint256 rate = IRateCalculator(rateCalculator).accrueRate(address(this)); - // get net accrued interest - uint256 accruedInterest = BrokerMath.denormalizeBorrowAmount(position.normalizedDebt, rate).zeroFloorSub( - position.principal - ); - // calculate the amount we need to repay for interest and principal - uint256 repayInterestAmt = amount < accruedInterest ? amount : accruedInterest; - uint256 amountLeft = amount - repayInterestAmt; - uint256 repayPrincipalAmt = amountLeft > position.principal ? position.principal : amountLeft; - - if (repayInterestAmt + repayPrincipalAmt == 0) revert NothingToRepay(); - - // record the actual repaid amount for event - uint256 totalRepaid = 0; - - // (1) Repay interest first - // update position - position.normalizedDebt = position.normalizedDebt.zeroFloorSub( - BrokerMath.normalizeBorrowAmount(repayInterestAmt, rate, false) - ); - // supply interest to moolah vault - _supplyToMoolahVault(repayInterestAmt); - totalRepaid += repayInterestAmt; - - // has left to repay principal - if (repayPrincipalAmt > 0) { - uint256 principalRepaid; - principalRepaid = _repayToMoolah(onBehalf, repayPrincipalAmt); - if (principalRepaid > 0) { - // update position - position.principal = position.principal.zeroFloorSub(principalRepaid); - position.normalizedDebt = position.normalizedDebt.zeroFloorSub( - BrokerMath.normalizeBorrowAmount(principalRepaid, rate, false) - ); - totalRepaid += principalRepaid; - } - // remove position if fully repaid - if (position.principal == 0) { - delete dynamicLoanPositions[onBehalf]; - } - } - - // refund excess native BNB to sender - uint256 excess = amount - totalRepaid; - if (excess > 0) { - if (isNative) { - _unwrapAndSend(payable(user), excess); - } else { - IERC20(LOAN_TOKEN).safeTransfer(user, excess); - } - } - - // validate positions - _validatePositions(onBehalf); - - emit DynamicLoanPositionRepaid(onBehalf, totalRepaid, position.principal); + LendingBrokerOperatorLib.repayDynamic(dynamicLoanPositions, _operatorCtx(), amount, onBehalf); } /** * @dev Repay a Fixed loan position * @notice repay interest first then principal, repay amount must larger than interest - * @param amount The amount to repay + * @param amount The amount to repay (overridden by msg.value when sending native BNB) * @param posId The ID of the fixed position to repay * @param onBehalf The address of the user whose position to repay */ @@ -332,98 +263,32 @@ contract LendingBroker is uint256 posId, address onBehalf ) external payable override marketIdSet whenNotPaused nonReentrant { - bool isNative = msg.value > 0; - address user = msg.sender; - if (isNative) { - if (LOAN_TOKEN != WBNB) revert NativeNotSupported(); - amount = msg.value; - IWBNB(WBNB).deposit{ value: amount }(); - } else { - IERC20(LOAN_TOKEN).safeTransferFrom(user, address(this), amount); - } - if (amount == 0) revert ZeroAmount(); - if (onBehalf == address(0)) revert ZeroAddress(); - - // fetch position (will revert if not found) - FixedLoanPosition memory position = _getFixedPositionByPosId(onBehalf, posId); - // remaining principal before repayment - uint256 remainingPrincipal = position.principal - position.principalRepaid; - // get outstanding accrued interest - uint256 accruedInterest = _getAccruedInterestForFixedPosition(position) - position.interestRepaid; - - // initialize repay amounts - uint256 repayInterestAmt = amount < accruedInterest ? amount : accruedInterest; - uint256 repayPrincipalAmt = amount - repayInterestAmt; - - // repay interest first, it might be zero if user just repaid before - if (repayInterestAmt > 0) { - // update repaid interest amount - position.interestRepaid += repayInterestAmt; - // supply interest into vault as revenue - _supplyToMoolahVault(repayInterestAmt); - } - - uint256 penalty = 0; - uint256 principalRepaid = 0; - // then repay principal if there is any amount left - if (repayPrincipalAmt > 0) { - // check penalty if user is repaying before expiration - penalty = _getPenaltyForFixedPosition(position, UtilsLib.min(repayPrincipalAmt, remainingPrincipal)); - // supply penalty into vault as revenue - if (penalty > 0) { - repayPrincipalAmt -= penalty; - _supplyToMoolahVault(penalty); - } - - // the rest will be used to repay partially - uint256 repayablePrincipal = UtilsLib.min(repayPrincipalAmt, remainingPrincipal); - if (repayablePrincipal > 0) { - principalRepaid = _repayToMoolah(onBehalf, repayablePrincipal); - position.principalRepaid += principalRepaid; - // reset repaid interest to zero (all accrued interest has been cleared) - position.interestRepaid = 0; - // reset last repay time to now - position.lastRepaidTime = block.timestamp; - } - } - - // post repayment - if (position.principalRepaid >= position.principal) { - // removes it from user's fixed positions - _removeFixedPositionByPosId(onBehalf, posId); - } else { - // update position - _updateFixedPosition(onBehalf, position); - } - - // refund excess native BNB to sender - uint256 used = repayInterestAmt + penalty + principalRepaid; - uint256 excess = amount - used; - if (excess > 0) { - if (isNative) { - _unwrapAndSend(payable(user), excess); - } else { - IERC20(LOAN_TOKEN).safeTransfer(user, excess); - } - } + LendingBrokerOperatorLib.repayFixed(fixedLoanPositions, _operatorCtx(), amount, posId, onBehalf); + } - // validate positions - _validatePositions(onBehalf); + /** + * @dev Emergency: fully repay every position (dynamic + all fixed) of `onBehalf` in one call. + * Charges full early-repay penalty on fixed positions, identical to normal `repay`. + * Skips per-position validation since every position is cleared. + * @notice For native BNB, send `msg.value >= totalDebt`; excess is refunded. + * For ERC20, the contract pulls exactly `totalDebt` from `msg.sender`. + * @param onBehalf The address of the user whose positions to repay + */ + function repayAll(address onBehalf) external payable override marketIdSet whenNotPaused nonReentrant { + LendingBrokerOperatorLib.repayAll(dynamicLoanPositions, fixedLoanPositions, _operatorCtx(), onBehalf); + } - // emit event - emit RepaidFixedLoanPosition( - onBehalf, - posId, - position.principal, - position.start, - position.end, - position.apr, - position.principalRepaid, - principalRepaid, - repayInterestAmt, - penalty, - position.interestRepaid - ); + /// @dev Build the operator-library context. Internal helper, inlined. + function _operatorCtx() private view returns (LendingBrokerOperatorLib.OperatorContext memory) { + return + LendingBrokerOperatorLib.OperatorContext({ + moolah: MOOLAH, + loanToken: LOAN_TOKEN, + wbnb: WBNB, + rateCalculator: rateCalculator, + relayer: RELAYER, + marketId: MARKET_ID + }); } /** @@ -438,16 +303,14 @@ contract LendingBroker is if (amount == 0) revert ZeroAmount(); address user = msg.sender; DynamicLoanPosition storage position = dynamicLoanPositions[user]; - if (fixedLoanPositions[user].length >= maxFixedLoanPositions) revert ExceedMaxFixedPositions(); // accrue current rate so normalized debt reflects the latest interest uint256 rate = IRateCalculator(rateCalculator).accrueRate(address(this)); - uint256 actualDebt = BrokerMath.denormalizeBorrowAmount(position.normalizedDebt, rate); - uint256 totalInterest = actualDebt.zeroFloorSub(position.principal); - - // prioritize clearing interest first, then principal - uint256 interestToRepay = UtilsLib.min(amount, totalInterest); - uint256 principalToMove = UtilsLib.min(amount - interestToRepay, position.principal); - amount = interestToRepay + principalToMove; + (uint256 interestToRepay, uint256 principalToMove, uint256 finalAmount) = BrokerMath.previewConvertDynamicToFixed( + position, + amount, + rate + ); + if (finalAmount == 0) revert ZeroAmount(); if (interestToRepay > 0) { // borrow from Moolah to increase user's actual debt at moolah @@ -457,7 +320,7 @@ contract LendingBroker is } position.normalizedDebt = position.normalizedDebt.zeroFloorSub( - BrokerMath.normalizeBorrowAmount(amount, rate, false) + BrokerMath.normalizeBorrowAmount(finalAmount, rate, false) ); position.principal -= principalToMove; @@ -465,29 +328,8 @@ contract LendingBroker is delete dynamicLoanPositions[user]; } - FixedTermAndRate memory term = _getTermById(termId); - uint256 start = block.timestamp; - uint256 end = start + term.duration; - // pos uuid increment - fixedPosUuid++; - // create new fixed position - fixedLoanPositions[user].push( - FixedLoanPosition({ - posId: fixedPosUuid, - principal: amount, - apr: term.apr, - start: start, - end: end, - lastRepaidTime: start, - interestRepaid: 0, - principalRepaid: 0 - }) - ); - - // validate positions - _validatePositions(user); - - emit FixedLoanPositionCreated(user, fixedPosUuid, amount, start, end, term.apr, termId); + _validateDynamicPosition(user); + _createFixedPosition(user, finalAmount, termId); } /** @@ -505,7 +347,7 @@ contract LendingBroker is address user, address receiver ) external marketIdSet whenNotPaused whenBorrowNotPaused nonReentrant { - if (amount == 0) revert AmountZero(); + if (amount == 0) revert ZeroAmount(); if (receiver == address(0)) revert ZeroAddress(); if (!MOOLAH.isAuthorized(user, msg.sender)) revert NotAuthorized(); _borrowFixed(user, amount, termId); @@ -522,10 +364,10 @@ contract LendingBroker is /////////////////////////////////////// /** * @dev Liquidate a borrower's debt by accruing interest and repaying the dynamic - * position first, then settling fixed-rate positions sorted by APR and - * remaining principal. The last fixed position absorbs any rounding delta. + * position first, then settling fixed-rate positions in order of earliest end + * time first. The last fixed position absorbs any rounding delta. * the parameters are the same as normal liquidator calls moolah.liquidate() - * @notice Only `Liquidator.sol` and `PublicLiquidator.sol` contracts are allowed to call this function + * @notice Only contracts whitelisted via `toggleLiquidationWhitelist` (e.g. `BrokerLiquidator.sol`) can call this function * @param marketParams The market of the position. * @param borrower The owner of the position. * @param seizedAssets The amount of collateral to seize. @@ -543,6 +385,7 @@ contract LendingBroker is if (!_checkLiquidationWhiteList(msg.sender)) revert NotLiquidationWhitelist(); if (Id.unwrap(id) != Id.unwrap(MARKET_ID)) revert InvalidMarketId(); if (!UtilsLib.exactlyOneZero(seizedAssets, repaidShares)) revert InvalidMarketId(); + if (repaidShares > 0 && repaidShares % SharesMathLib.VIRTUAL_SHARES != 0) revert InvalidRepaidShares(); if (borrower == address(0)) revert InvalidUser(); // [1] init liquidation context for onMoolahLiquidate() Callback @@ -616,26 +459,23 @@ contract LendingBroker is delete dynamicLoanPositions[borrower]; delete fixedLoanPositions[borrower]; } else { - // [9] deduct interest and principal from positions - // deduct from dynamic position and returns the leftover assets to deduct - (uint256 interestLeftover, uint256 principalLeftover) = _deductDynamicPositionDebt( - borrower, - dynamicPosition, - interestToBroker, - repaidAssets, - rate - ); - // deduct from fixed positions - if ((principalLeftover > 0 || interestLeftover > 0) && fixedPositions.length > 0) { - // sort fixed positions from earliest end time to latest, filter out fully repaid positions - // positions with earlier end time will be deducted first - FixedLoanPosition[] memory sorted = BrokerMath.sortAndFilterFixedPositions(fixedPositions); - if (sorted.length > 0) { - _deductFixedPositionsDebt(borrower, sorted, interestLeftover, principalLeftover); + // [9] run the full cascade in one library call: deduct from the dynamic + // position, sort the fixed positions, then deduct from them in order + ( + DynamicLoanPosition memory updatedDyn, + FixedLoanPosition[] memory touched, + bool[] memory shouldRemove + ) = BrokerMath.executeLiquidationCascade(dynamicPosition, fixedPositions, interestToBroker, repaidAssets, rate); + // apply dynamic update + dynamicLoanPositions[borrower] = updatedDyn; + // apply fixed-position updates + for (uint256 i = 0; i < touched.length; i++) { + if (shouldRemove[i]) { + _removeFixedPositionByPosId(borrower, touched[i].posId); + } else { + _updateFixedPosition(borrower, touched[i]); } } - // [10] validate every position after deduction meets minLoan - _validatePositions(borrower); } } } @@ -651,68 +491,6 @@ contract LendingBroker is return liquidationWhitelist.contains(liquidator); } - /** - * @dev deducts debt from the dynamic position and returns leftover assets - * @param position The dynamic loan position to modify - * @param interestToDeduct The amount of interest repaid during liquidation, leads to deduct from principal and interest - * @param principalToDeduct The amount of assets repaid during liquidation, leads to deduct from principal and interest - * @param rate The current interest rate - */ - function _deductDynamicPositionDebt( - address user, - DynamicLoanPosition memory position, - uint256 interestToDeduct, - uint256 principalToDeduct, - uint256 rate - ) internal returns (uint256, uint256) { - // call BrokerMath to process deduction - // will return leftover interest/principal to deduct and the updated position - (interestToDeduct, principalToDeduct, position) = BrokerMath.deductDynamicPositionDebt( - position, - interestToDeduct, - principalToDeduct, - rate - ); - // update position - dynamicLoanPositions[user] = position; - return (interestToDeduct, principalToDeduct); - } - - /** - * @dev allocates repayments to fixed positions by APR and remaining principal - * @param user The address of the user - * @param sortedFixedPositions The sorted fixed loan positions - * @param interestToDeduct The amount of interest repaid during liquidation, leads to deduct from principal and interest - * @param principalToDeduct The amount of assets repaid during liquidation, leads to deduct from principal and interest - */ - function _deductFixedPositionsDebt( - address user, - FixedLoanPosition[] memory sortedFixedPositions, - uint256 interestToDeduct, - uint256 principalToDeduct - ) internal { - uint256 len = sortedFixedPositions.length; - for (uint256 i = 0; i < len; i++) { - if (principalToDeduct == 0 && interestToDeduct == 0) break; - FixedLoanPosition memory p = sortedFixedPositions[i]; - // call BrokerMath to process deduction one by one - // will return leftover interest/principal to deduct and the updated position - (interestToDeduct, principalToDeduct, p) = BrokerMath.deductFixedPositionDebt( - interestToDeduct, - principalToDeduct, - p - ); - // post repayment - if (p.principalRepaid >= p.principal) { - // removes it from user's fixed positions - _removeFixedPositionByPosId(user, p.posId); - } else { - // update position - _updateFixedPosition(user, p); - } - } - } - /////////////////////////////////////// ///// View functions ///// /////////////////////////////////////// @@ -853,25 +631,36 @@ contract LendingBroker is * @param termId The fixed-term product ID */ function _borrowFixed(address user, uint256 amount, uint256 termId) private { + _createFixedPosition(user, amount, termId); + _borrowFromMoolah(user, amount); + } + + /** + * @dev Create and push a new fixed-term position for `user`. Used by both `_borrowFixed` + * (fresh fixed borrow) and `convertDynamicToFixed` (moves principal/interest from + * a dynamic position). Validates the new position before returning. + * @param user The user the position belongs to + * @param amount The position's principal + * @param termId The fixed-term product ID + */ + function _createFixedPosition(address user, uint256 amount, uint256 termId) private { if (fixedLoanPositions[user].length >= maxFixedLoanPositions) revert ExceedMaxFixedPositions(); FixedTermAndRate memory term = _getTermById(termId); uint256 start = block.timestamp; - uint256 end = block.timestamp + term.duration; + uint256 end = start + term.duration; fixedPosUuid++; - fixedLoanPositions[user].push( - FixedLoanPosition({ - posId: fixedPosUuid, - principal: amount, - apr: term.apr, - start: start, - end: end, - lastRepaidTime: start, - interestRepaid: 0, - principalRepaid: 0 - }) - ); - _borrowFromMoolah(user, amount); - _validatePositions(user); + FixedLoanPosition memory newPos = FixedLoanPosition({ + posId: fixedPosUuid, + principal: amount, + apr: term.apr, + start: start, + end: end, + lastRepaidTime: start, + interestRepaid: 0, + principalRepaid: 0 + }); + fixedLoanPositions[user].push(newPos); + _validateFixedPosition(newPos); emit FixedLoanPositionCreated(user, fixedPosUuid, amount, start, end, term.apr, termId); } @@ -979,26 +768,6 @@ contract LendingBroker is revert PositionNotFound(); } - /** - * @dev Get the interest for a fixed loan position - * @param position The fixed loan position to get the interest for - */ - function _getAccruedInterestForFixedPosition(FixedLoanPosition memory position) internal view returns (uint256) { - return BrokerMath.getAccruedInterestForFixedPosition(position); - } - - /** - * @dev Get the penalty for a fixed loan position - * @param position The fixed loan position to get the penalty for - * @param repayAmt The actual repay amount (repay amount excluded accrued interest) - */ - function _getPenaltyForFixedPosition( - FixedLoanPosition memory position, - uint256 repayAmt - ) internal view returns (uint256 penalty) { - return BrokerMath.getPenaltyForFixedPosition(position, repayAmt); - } - /** * @dev Transfer loan token to recipient. Unwraps to native BNB when supported. */ @@ -1020,29 +789,26 @@ contract LendingBroker is } /** - * @dev Repay an amount on behalf of a user to Moolah - * @param onBehalf The address of the user to repay on behalf of - * @param amount The amount to repay + * @dev Validate that the user's dynamic position is either zero or meets the minimum loan + * @param user The address of the user */ - function _repayToMoolah(address onBehalf, uint256 amount) internal returns (uint256 assetsRepaid) { - IERC20(LOAN_TOKEN).safeIncreaseAllowance(address(MOOLAH), amount); - Market memory market = MOOLAH.market(MARKET_ID); - // convert amount to shares - uint256 amountShares = amount.toSharesDown(market.totalBorrowAssets, market.totalBorrowShares); - // using `shares` to ensure full repayment - (assetsRepaid /* sharesRepaid */, ) = MOOLAH.repay(_getMarketParams(MARKET_ID), 0, amountShares, onBehalf, ""); - IERC20(LOAN_TOKEN).forceApprove(address(MOOLAH), 0); + function _validateDynamicPosition(address user) internal view { + uint256 principal = dynamicLoanPositions[user].principal; + if (principal == 0) return; + uint256 minLoan = MOOLAH.minLoan(_getMarketParams(MARKET_ID)); + require(principal >= minLoan, "broker/dynamic-below-min-loan"); } /** - * @dev Validate that the user's positions meet the minimum loan requirement - * @param user The address of the user + * @dev Validate that a fixed position is either fully repaid or has remaining principal + * at or above the minimum loan + * @param position The fixed loan position to validate */ - function _validatePositions(address user) internal view { - require( - BrokerMath.checkPositionsMeetsMinLoan(user, address(MOOLAH), rateCalculator), - "broker/positions-below-min-loan" - ); + function _validateFixedPosition(FixedLoanPosition memory position) internal view { + uint256 remaining = position.principal - position.principalRepaid; + if (remaining == 0) return; + uint256 minLoan = MOOLAH.minLoan(_getMarketParams(MARKET_ID)); + require(remaining >= minLoan, "broker/fixed-below-min-loan"); } /////////////////////////////////////// @@ -1183,7 +949,7 @@ contract LendingBroker is if (token == address(0)) { (bool success, ) = msg.sender.call{ value: amount }(""); - if (!success) revert TransferFailed(); + if (!success) revert NativeTransferFailed(); } else { IERC20(token).safeTransfer(msg.sender, amount); } diff --git a/src/broker/interfaces/IBroker.sol b/src/broker/interfaces/IBroker.sol index 2978a980..ff7f593b 100644 --- a/src/broker/interfaces/IBroker.sol +++ b/src/broker/interfaces/IBroker.sol @@ -104,6 +104,7 @@ interface IBroker is IBrokerBase { event MaxFixedLoanPositionsUpdated(uint256 oldMax, uint256 newMax); event FixedTermAndRateUpdated(uint256 termId, uint256 duration, uint256 apr); event Liquidated(address indexed user, uint256 principalCleared, uint256 interestCleared); + event AllPositionsRepaid(address indexed user, uint256 totalRepaid); event MarketIdSet(Id marketId); event BorrowPaused(bool paused); event RelayerSet(address indexed relayer); @@ -157,6 +158,11 @@ interface IBroker is IBrokerBase { /// @param onBehalf The address of the user whose position to repay function repay(uint256 amount, uint256 posIdx, address onBehalf) external payable; + /// @dev emergency: fully repay every position (dynamic + all fixed) of a user in one call. + /// Charges full early-repay penalty on fixed positions. + /// @param onBehalf The address of the user whose positions to repay + function repayAll(address onBehalf) external payable; + /// @dev refinance expired fixed positions to dynamic /// @param user The address of the user to refinance /// @param positionIds The posIds of the fixed positions to refinance diff --git a/src/broker/libraries/BrokerMath.sol b/src/broker/libraries/BrokerMath.sol index c0ae5103..d7797459 100644 --- a/src/broker/libraries/BrokerMath.sol +++ b/src/broker/libraries/BrokerMath.sol @@ -94,43 +94,6 @@ library BrokerMath { } } - /** - * @dev Ensure every position's principal either cleared or larger than Moolah.minLoan - * @param user The address of the user - * @param moolah The address of the Moolah contract - * @param rateCalculator The address of the rate calculator - */ - function checkPositionsMeetsMinLoan( - address user, - address moolah, - address rateCalculator - ) public view returns (bool isValid) { - // get current rate - uint256 currentRate = IRateCalculator(rateCalculator).getRate(address(this)); - // get positions - IBroker broker = IBroker(address(this)); - FixedLoanPosition[] memory fixedPositions = broker.userFixedPositions(user); - DynamicLoanPosition memory dynamicPosition = broker.userDynamicPosition(user); - // assume valid first - isValid = true; - IMoolah _moolah = IMoolah(moolah); - uint256 minLoan = _moolah.minLoan(_moolah.idToMarketParams(IBroker(address(this)).MARKET_ID())); - // ensure each position either zero or larger than minLoan - // check dynamic position - uint256 dynamicDebt = dynamicPosition.principal; - if (dynamicDebt > 0 && dynamicDebt < minLoan) { - isValid = false; - } - // check fixed positions - for (uint256 i = 0; i < fixedPositions.length; i++) { - FixedLoanPosition memory _fixedPos = fixedPositions[i]; - uint256 fixedPosDebt = _fixedPos.principal - _fixedPos.principalRepaid; - if (fixedPosDebt > 0 && fixedPosDebt < minLoan) { - isValid = false; - } - } - } - /** * @dev Get the total debt for a user * @param fixedPositions The fixed loan positions of the user @@ -589,4 +552,122 @@ library BrokerMath { } return filtered; } + + /** + * @dev Compute the interest/principal split for convertDynamicToFixed. + * @param position The dynamic loan position (memory copy) + * @param amount The amount the user wants to convert + * @param rate The current dynamic-loan rate + * @return interestToRepay The interest portion (paid to vault) + * @return principalToMove The principal portion (moved to the new fixed position) + * @return finalAmount The total of interestToRepay + principalToMove + */ + function previewConvertDynamicToFixed( + DynamicLoanPosition memory position, + uint256 amount, + uint256 rate + ) public pure returns (uint256 interestToRepay, uint256 principalToMove, uint256 finalAmount) { + uint256 actualDebt = denormalizeBorrowAmount(position.normalizedDebt, rate); + uint256 totalInterest = UtilsLib.zeroFloorSub(actualDebt, position.principal); + interestToRepay = UtilsLib.min(amount, totalInterest); + principalToMove = UtilsLib.min(amount - interestToRepay, position.principal); + finalAmount = interestToRepay + principalToMove; + } + + /** + * @dev Compute every amount needed by repayAll: dynamic interest, fixed interest + early-repay + * penalty, Moolah-side principal, and the grand total. Lets the broker delegate the + * iteration + math to the library instead of duplicating it inline. + * @param onBehalf The user whose positions are being repaid + * @param dynPos The user's dynamic loan position (memory copy) + * @param fixedPositions The user's full fixed-position array + * @param rate The current dynamic-loan rate + * @return dynamicInterest The dynamic position's accrued interest + * @return fixedInterestAndPenalty The sum of accrued interest and early-repay penalty across fixed positions + * @return debtAtMoolah The principal owed to Moolah for `onBehalf` + * @return totalDebt debtAtMoolah + dynamicInterest + fixedInterestAndPenalty + */ + function previewRepayAllAmounts( + address onBehalf, + DynamicLoanPosition memory dynPos, + FixedLoanPosition[] memory fixedPositions, + uint256 rate + ) + public + view + returns (uint256 dynamicInterest, uint256 fixedInterestAndPenalty, uint256 debtAtMoolah, uint256 totalDebt) + { + dynamicInterest = UtilsLib.zeroFloorSub(denormalizeBorrowAmount(dynPos.normalizedDebt, rate), dynPos.principal); + for (uint256 i = 0; i < fixedPositions.length; i++) { + FixedLoanPosition memory p = fixedPositions[i]; + uint256 remainingPrincipal = p.principal - p.principalRepaid; + uint256 accruedInterest = getAccruedInterestForFixedPosition(p) - p.interestRepaid; + uint256 penalty = getPenaltyForFixedPosition(p, remainingPrincipal); + fixedInterestAndPenalty += accruedInterest + penalty; + } + debtAtMoolah = getDebtAtMoolah(onBehalf); + totalDebt = debtAtMoolah + dynamicInterest + fixedInterestAndPenalty; + } + + /** + * @dev Run the full liquidation cascade in one call: deduct from the dynamic position, + * then sort and filter fixed positions, then deduct from them in order. Returns the + * updated dynamic position plus parallel arrays describing what the caller should do + * with each touched fixed position. The caller is responsible for writing the dynamic + * update back to storage and applying the fixed-position actions. + * @param dynPos The dynamic loan position to deduct from (memory copy) + * @param fixedPositions The user's full fixed-position array (unfiltered) + * @param interestToDeduct The interest budget for the cascade + * @param principalToDeduct The principal budget for the cascade + * @param rate The current dynamic-loan rate + * @return updatedDyn The dynamic position after the cascade + * @return touched Fixed positions that were modified (trimmed to actual count) + * @return shouldRemove Parallel flags indicating which entries should be removed + */ + function executeLiquidationCascade( + DynamicLoanPosition memory dynPos, + FixedLoanPosition[] memory fixedPositions, + uint256 interestToDeduct, + uint256 principalToDeduct, + uint256 rate + ) + public + returns (DynamicLoanPosition memory updatedDyn, FixedLoanPosition[] memory touched, bool[] memory shouldRemove) + { + // deduct from dynamic position first + (interestToDeduct, principalToDeduct, dynPos) = deductDynamicPositionDebt( + dynPos, + interestToDeduct, + principalToDeduct, + rate + ); + updatedDyn = dynPos; + + // nothing left or no fixed positions → return empty action arrays + if ((interestToDeduct == 0 && principalToDeduct == 0) || fixedPositions.length == 0) { + return (updatedDyn, new FixedLoanPosition[](0), new bool[](0)); + } + + // sort by end-time ascending and drop fully-repaid entries + FixedLoanPosition[] memory sorted = sortAndFilterFixedPositions(fixedPositions); + uint256 len = sorted.length; + touched = new FixedLoanPosition[](len); + shouldRemove = new bool[](len); + uint256 count; + + for (uint256 i = 0; i < len; i++) { + if (principalToDeduct == 0 && interestToDeduct == 0) break; + FixedLoanPosition memory p = sorted[i]; + (interestToDeduct, principalToDeduct, p) = deductFixedPositionDebt(interestToDeduct, principalToDeduct, p); + touched[count] = p; + shouldRemove[count] = p.principalRepaid >= p.principal; + count++; + } + + // trim arrays to actual touched count + assembly { + mstore(touched, count) + mstore(shouldRemove, count) + } + } } diff --git a/src/broker/libraries/LendingBrokerOperatorLib.sol b/src/broker/libraries/LendingBrokerOperatorLib.sol new file mode 100644 index 00000000..02682374 --- /dev/null +++ b/src/broker/libraries/LendingBrokerOperatorLib.sol @@ -0,0 +1,365 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.34; + +import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; + +import { IBroker, FixedLoanPosition, DynamicLoanPosition } from "../interfaces/IBroker.sol"; +import { IRateCalculator } from "../interfaces/IRateCalculator.sol"; +import { IBrokerInterestRelayer } from "../interfaces/IBrokerInterestRelayer.sol"; +import { BrokerMath } from "./BrokerMath.sol"; + +import { Id, IMoolah, MarketParams, Market, Position } from "../../moolah/interfaces/IMoolah.sol"; +import { SharesMathLib } from "../../moolah/libraries/SharesMathLib.sol"; +import { UtilsLib } from "../../moolah/libraries/UtilsLib.sol"; +import { IWBNB } from "../../provider/interfaces/IWBNB.sol"; + +/// @title LendingBroker operator library +/// @notice Houses the bytecode for `repay`, `repay(fixed)`, and `repayAll` so the broker +/// itself stays under the EIP-170 size limit. Invoked via DELEGATECALL, so all +/// state mutations apply to LendingBroker's storage and events surface from +/// LendingBroker's address. +library LendingBrokerOperatorLib { + using SafeERC20 for IERC20; + using SharesMathLib for uint256; + using UtilsLib for uint256; + + // ------- Errors (must match LendingBroker's set) ------- + error ZeroAmount(); + error ZeroAddress(); + error NothingToRepay(); + error NativeNotSupported(); + error InsufficientAmount(); + error NativeTransferFailed(); + error PositionNotFound(); + + // ------- Events (must match IBroker signatures so indexers keep working) ------- + event DynamicLoanPositionRepaid(address indexed user, uint256 repaid, uint256 principalLeft); + event RepaidFixedLoanPosition( + address indexed user, + uint256 posId, + uint256 principal, + uint256 start, + uint256 end, + uint256 apr, + uint256 principalRepaid, + uint256 repayPrincipal, + uint256 repayInterest, + uint256 repayPenalty, + uint256 totalInterestRepaid + ); + event FixedLoanPositionRemoved(address indexed user, uint256 posId); + event AllPositionsRepaid(address indexed user, uint256 totalRepaid); + + /// @dev Immutable/state values that the operator paths need. Filled by the broker on each call. + struct OperatorContext { + IMoolah moolah; + address loanToken; + address wbnb; + address rateCalculator; + address relayer; + Id marketId; + } + + // ============================================= + // External entry points + // ============================================= + + /// @dev Implements LendingBroker.repay(amount, onBehalf). Storage refs come from the broker. + function repayDynamic( + mapping(address => DynamicLoanPosition) storage dynamicLoanPositions, + OperatorContext memory ctx, + uint256 amount, + address onBehalf + ) external { + address user = msg.sender; + bool isNative; + (amount, isNative) = _pullPayment(ctx, amount, user); + if (amount == 0) revert ZeroAmount(); + if (onBehalf == address(0)) revert ZeroAddress(); + + DynamicLoanPosition storage position = dynamicLoanPositions[onBehalf]; + uint256 rate = IRateCalculator(ctx.rateCalculator).accrueRate(address(this)); + uint256 accruedInterest = BrokerMath.denormalizeBorrowAmount(position.normalizedDebt, rate).zeroFloorSub( + position.principal + ); + uint256 repayInterestAmt = amount < accruedInterest ? amount : accruedInterest; + uint256 amountLeft = amount - repayInterestAmt; + uint256 repayPrincipalAmt = amountLeft > position.principal ? position.principal : amountLeft; + if (repayInterestAmt + repayPrincipalAmt == 0) revert NothingToRepay(); + + uint256 totalRepaid; + + // (1) Repay interest first + position.normalizedDebt = position.normalizedDebt.zeroFloorSub( + BrokerMath.normalizeBorrowAmount(repayInterestAmt, rate, false) + ); + _supplyToMoolahVault(ctx, repayInterestAmt); + totalRepaid += repayInterestAmt; + + // (2) Repay principal if any + if (repayPrincipalAmt > 0) { + uint256 principalRepaid = _repayToMoolah(ctx, onBehalf, repayPrincipalAmt); + if (principalRepaid > 0) { + position.principal = position.principal.zeroFloorSub(principalRepaid); + position.normalizedDebt = position.normalizedDebt.zeroFloorSub( + BrokerMath.normalizeBorrowAmount(principalRepaid, rate, false) + ); + totalRepaid += principalRepaid; + } + if (position.principal == 0) { + delete dynamicLoanPositions[onBehalf]; + } + } + + _refundExcess(ctx, amount - totalRepaid, user, isNative); + _validateDynamicPosition(dynamicLoanPositions, ctx, onBehalf); + emit DynamicLoanPositionRepaid(onBehalf, totalRepaid, position.principal); + } + + /// @dev Implements LendingBroker.repay(amount, posId, onBehalf). + function repayFixed( + mapping(address => FixedLoanPosition[]) storage fixedLoanPositions, + OperatorContext memory ctx, + uint256 amount, + uint256 posId, + address onBehalf + ) external { + address user = msg.sender; + bool isNative; + (amount, isNative) = _pullPayment(ctx, amount, user); + if (amount == 0) revert ZeroAmount(); + if (onBehalf == address(0)) revert ZeroAddress(); + + FixedLoanPosition memory position = _getFixedPositionByPosId(fixedLoanPositions, onBehalf, posId); + uint256 remainingPrincipal = position.principal - position.principalRepaid; + uint256 accruedInterest = BrokerMath.getAccruedInterestForFixedPosition(position) - position.interestRepaid; + + uint256 repayInterestAmt = amount < accruedInterest ? amount : accruedInterest; + uint256 repayPrincipalAmt = amount - repayInterestAmt; + + if (repayInterestAmt > 0) { + position.interestRepaid += repayInterestAmt; + _supplyToMoolahVault(ctx, repayInterestAmt); + } + + uint256 penalty; + uint256 principalRepaid; + if (repayPrincipalAmt > 0) { + penalty = BrokerMath.getPenaltyForFixedPosition(position, UtilsLib.min(repayPrincipalAmt, remainingPrincipal)); + if (penalty > 0) { + repayPrincipalAmt -= penalty; + _supplyToMoolahVault(ctx, penalty); + } + uint256 repayablePrincipal = UtilsLib.min(repayPrincipalAmt, remainingPrincipal); + if (repayablePrincipal > 0) { + principalRepaid = _repayToMoolah(ctx, onBehalf, repayablePrincipal); + position.principalRepaid += principalRepaid; + position.interestRepaid = 0; + position.lastRepaidTime = block.timestamp; + } + } + + if (position.principalRepaid >= position.principal) { + _removeFixedPositionByPosId(fixedLoanPositions, onBehalf, posId); + } else { + _updateFixedPosition(fixedLoanPositions, onBehalf, position); + } + + _refundExcess(ctx, amount - (repayInterestAmt + penalty + principalRepaid), user, isNative); + _validateFixedPosition(ctx, position); + + emit RepaidFixedLoanPosition( + onBehalf, + posId, + position.principal, + position.start, + position.end, + position.apr, + position.principalRepaid, + principalRepaid, + repayInterestAmt, + penalty, + position.interestRepaid + ); + } + + /// @dev Implements LendingBroker.repayAll(onBehalf). Charges full early-repay penalty + /// on every outstanding fixed position. Caller must send `msg.value >= totalDebt` + /// when paying in native BNB; excess is refunded. + function repayAll( + mapping(address => DynamicLoanPosition) storage dynamicLoanPositions, + mapping(address => FixedLoanPosition[]) storage fixedLoanPositions, + OperatorContext memory ctx, + address onBehalf + ) external { + if (onBehalf == address(0)) revert ZeroAddress(); + address user = msg.sender; + bool isNative = msg.value > 0; + + uint256 rate = IRateCalculator(ctx.rateCalculator).accrueRate(address(this)); + DynamicLoanPosition memory dynPos = dynamicLoanPositions[onBehalf]; + FixedLoanPosition[] memory fixedPositions = fixedLoanPositions[onBehalf]; + + (uint256 dynamicInterest, uint256 fixedInterestAndPenalty, uint256 debtAtMoolah, uint256 totalDebt) = BrokerMath + .previewRepayAllAmounts(onBehalf, dynPos, fixedPositions, rate); + if (totalDebt == 0) revert NothingToRepay(); + + // pull funds + if (isNative) { + if (ctx.loanToken != ctx.wbnb) revert NativeNotSupported(); + if (msg.value < totalDebt) revert InsufficientAmount(); + IWBNB(ctx.wbnb).deposit{ value: msg.value }(); + } else { + IERC20(ctx.loanToken).safeTransferFrom(user, address(this), totalDebt); + } + + // supply broker revenue (interest + penalty) to the vault + _supplyToMoolahVault(ctx, dynamicInterest + fixedInterestAndPenalty); + + // repay every Moolah borrow share at once via shares + Position memory pos = ctx.moolah.position(ctx.marketId, onBehalf); + if (pos.borrowShares > 0) { + _repayMoolahByShares(ctx, onBehalf, pos.borrowShares, debtAtMoolah); + } + + // clear dynamic position + if (dynPos.principal > 0 || dynamicInterest > 0) { + delete dynamicLoanPositions[onBehalf]; + emit DynamicLoanPositionRepaid(onBehalf, dynPos.principal + dynamicInterest, 0); + } + + // emit per-position removal events then wipe the array + for (uint256 i = 0; i < fixedPositions.length; i++) { + FixedLoanPosition memory p = fixedPositions[i]; + if (p.principal > p.principalRepaid) { + emit FixedLoanPositionRemoved(onBehalf, p.posId); + } + } + delete fixedLoanPositions[onBehalf]; + + if (isNative) _refundExcess(ctx, msg.value - totalDebt, user, true); + emit AllPositionsRepaid(onBehalf, totalDebt); + } + + // ============================================= + // Internal helpers + // ============================================= + + function _pullPayment( + OperatorContext memory ctx, + uint256 amount, + address user + ) internal returns (uint256 finalAmount, bool isNative) { + isNative = msg.value > 0; + if (isNative) { + if (ctx.loanToken != ctx.wbnb) revert NativeNotSupported(); + finalAmount = msg.value; + IWBNB(ctx.wbnb).deposit{ value: finalAmount }(); + } else { + IERC20(ctx.loanToken).safeTransferFrom(user, address(this), amount); + finalAmount = amount; + } + } + + function _refundExcess(OperatorContext memory ctx, uint256 excess, address user, bool isNative) internal { + if (excess == 0) return; + if (isNative) { + _unwrapAndSend(ctx, payable(user), excess); + } else { + IERC20(ctx.loanToken).safeTransfer(user, excess); + } + } + + function _unwrapAndSend(OperatorContext memory ctx, address payable recipient, uint256 amount) internal { + IWBNB(ctx.wbnb).withdraw(amount); + (bool ok, ) = recipient.call{ value: amount }(""); + if (!ok) revert NativeTransferFailed(); + } + + function _supplyToMoolahVault(OperatorContext memory ctx, uint256 interest) internal { + if (interest > 0) { + IERC20(ctx.loanToken).safeIncreaseAllowance(ctx.relayer, interest); + IBrokerInterestRelayer(ctx.relayer).supplyToVault(interest); + } + } + + function _repayToMoolah(OperatorContext memory ctx, address onBehalf, uint256 amount) internal returns (uint256) { + Market memory market = ctx.moolah.market(ctx.marketId); + uint256 amountShares = amount.toSharesDown(market.totalBorrowAssets, market.totalBorrowShares); + return _repayMoolahByShares(ctx, onBehalf, amountShares, amount); + } + + function _repayMoolahByShares( + OperatorContext memory ctx, + address onBehalf, + uint256 shares, + uint256 allowance + ) internal returns (uint256 assetsRepaid) { + IERC20(ctx.loanToken).safeIncreaseAllowance(address(ctx.moolah), allowance); + (assetsRepaid, ) = ctx.moolah.repay(ctx.moolah.idToMarketParams(ctx.marketId), 0, shares, onBehalf, ""); + IERC20(ctx.loanToken).forceApprove(address(ctx.moolah), 0); + } + + function _getFixedPositionByPosId( + mapping(address => FixedLoanPosition[]) storage fixedLoanPositions, + address user, + uint256 posId + ) internal view returns (FixedLoanPosition memory) { + FixedLoanPosition[] memory positions = fixedLoanPositions[user]; + for (uint256 i = 0; i < positions.length; i++) { + if (positions[i].posId == posId) return positions[i]; + } + revert PositionNotFound(); + } + + function _removeFixedPositionByPosId( + mapping(address => FixedLoanPosition[]) storage fixedLoanPositions, + address user, + uint256 posId + ) internal { + FixedLoanPosition[] storage positions = fixedLoanPositions[user]; + for (uint256 i = 0; i < positions.length; i++) { + if (positions[i].posId == posId) { + positions[i] = positions[positions.length - 1]; + positions.pop(); + emit FixedLoanPositionRemoved(user, posId); + return; + } + } + revert PositionNotFound(); + } + + function _updateFixedPosition( + mapping(address => FixedLoanPosition[]) storage fixedLoanPositions, + address user, + FixedLoanPosition memory position + ) internal { + FixedLoanPosition[] storage positions = fixedLoanPositions[user]; + for (uint256 i = 0; i < positions.length; i++) { + if (positions[i].posId == position.posId) { + positions[i] = position; + return; + } + } + revert PositionNotFound(); + } + + function _validateDynamicPosition( + mapping(address => DynamicLoanPosition) storage dynamicLoanPositions, + OperatorContext memory ctx, + address user + ) internal view { + uint256 principal = dynamicLoanPositions[user].principal; + if (principal == 0) return; + uint256 minLoan = ctx.moolah.minLoan(ctx.moolah.idToMarketParams(ctx.marketId)); + require(principal >= minLoan, "broker/dynamic-below-min-loan"); + } + + function _validateFixedPosition(OperatorContext memory ctx, FixedLoanPosition memory position) internal view { + uint256 remaining = position.principal - position.principalRepaid; + if (remaining == 0) return; + uint256 minLoan = ctx.moolah.minLoan(ctx.moolah.idToMarketParams(ctx.marketId)); + require(remaining >= minLoan, "broker/fixed-below-min-loan"); + } +} diff --git a/test/broker/BrokerMathDeductFixed.t.sol b/test/broker/BrokerMathDeductFixed.t.sol index 72f0d7b6..b59a3fe4 100644 --- a/test/broker/BrokerMathDeductFixed.t.sol +++ b/test/broker/BrokerMathDeductFixed.t.sol @@ -7,8 +7,11 @@ import { FixedLoanPosition } from "../../src/broker/interfaces/IBroker.sol"; import { UtilsLib } from "../../src/moolah/libraries/UtilsLib.sol"; /// @title Tests for BrokerMath.deductFixedPositionDebt -/// @notice Validates that partial liquidation preserves exact outstanding interest, -/// accounting for the reduced principal effect on the interest formula. +/// @notice The current implementation deliberately resets `interestRepaid` and +/// `lastRepaidTime` on any positive principal payment — the audit-acknowledged +/// simplified semantic. Outstanding interest is only preserved when the call +/// is interest-only. These tests validate that reset behaviour and the +/// interest-only preservation path. contract BrokerMathDeductFixedTest is Test { uint256 constant DURATION = 365 days; // 10% APR -> RATE_SCALE * 1.10 @@ -41,12 +44,13 @@ contract BrokerMathDeductFixedTest is Test { } // ==================================================================== - // Core fix: partial liquidation preserves EXACT outstanding interest + // Any principal payment resets interest tracking // ==================================================================== - /// @notice principal=100e18, interest~10e18, liquidation pays half interest + half principal. - /// Outstanding interest must be exactly (accruedInterest - paidInterest). - function test_partialLiquidation_preservesExactOutstanding() public { + /// @notice principal=100e18, interest~10e18, partial-interest + partial-principal. + /// After the call: interestRepaid = 0, lastRepaidTime = now, outstanding = 0 + /// (no time has elapsed since the reset). + function test_partialLiquidation_resetsInterestOnPrincipalPayment() public { FixedLoanPosition memory pos = _makePosition(100 ether); skip(DURATION); @@ -64,14 +68,14 @@ contract BrokerMathDeductFixedTest is Test { assertEq(principalLeft, 0, "principal budget consumed"); assertEq(updated.principalRepaid, principalBudget, "principalRepaid correct"); - // Core invariant: outstanding = accruedInterest - paidInterest (exact) - uint256 expectedOutstanding = accruedInterest - interestBudget; - uint256 actualOutstanding = _outstanding(updated); - assertEq(actualOutstanding, expectedOutstanding, "outstanding interest must be exact"); + // Simplified reset semantic: any positive principal payment wipes interest tracking + assertEq(updated.interestRepaid, 0, "interestRepaid reset to 0"); + assertEq(updated.lastRepaidTime, block.timestamp, "lastRepaidTime reset to now"); + assertEq(_outstanding(updated), 0, "no outstanding immediately after reset"); } // ==================================================================== - // Full interest payment resets correctly + // Full interest + principal: reset (same as core path) // ==================================================================== function test_fullInterestPayment_resetsTracking() public { @@ -91,7 +95,7 @@ contract BrokerMathDeductFixedTest is Test { } // ==================================================================== - // Interest only, no principal deduction + // Interest-only path: preserves exact outstanding (no principal => no reset) // ==================================================================== function test_interestOnlyPartial_preservesOutstanding() public { @@ -114,7 +118,7 @@ contract BrokerMathDeductFixedTest is Test { } // ==================================================================== - // Full principal with partial interest -> fallback (position filtered out) + // Full principal with partial interest -> reset + filtered out // ==================================================================== function test_fullPrincipalPartialInterest_fallback() public { @@ -155,36 +159,41 @@ contract BrokerMathDeductFixedTest is Test { } // ==================================================================== - // Sequential partial liquidations preserve cumulative outstanding + // Sequential partial liquidations: each principal payment resets, + // new interest accrues between calls based on elapsed time + reduced principal. // ==================================================================== - function test_sequentialPartialLiquidations_preserveOutstanding() public { + function test_sequentialPartialLiquidations_resetEachTime() public { FixedLoanPosition memory pos = _makePosition(100 ether); - skip(DURATION); + // Move part-way through the term so interest can still accrue between calls. + skip(DURATION / 3); uint256 originalAccrued = _outstanding(pos); + assertGt(originalAccrued, 0, "precondition: interest accrued"); - // First partial: 1/4 interest + 20 principal + // First partial: 1/4 interest + 20 principal -> reset triggered uint256 firstInterest = originalAccrued / 4; (, , FixedLoanPosition memory after1) = BrokerMath.deductFixedPositionDebt(firstInterest, 20 ether, pos); - uint256 expectedAfter1 = originalAccrued - firstInterest; - assertEq(_outstanding(after1), expectedAfter1, "first: outstanding exact"); + // Reset semantic: interestRepaid wiped, lastRepaidTime = now, outstanding = 0 in this block + assertEq(after1.interestRepaid, 0, "first: interest tracking reset"); + assertEq(after1.lastRepaidTime, block.timestamp, "first: lastRepaidTime = now"); assertEq(after1.principalRepaid, 20 ether, "first: principal tracked"); + assertEq(_outstanding(after1), 0, "first: no outstanding immediately after reset"); - // Second partial: another chunk of interest + 30 principal + // Let time elapse — still within the term — so new interest accrues on the reduced principal + skip(DURATION / 3); uint256 outstandingAfter1 = _outstanding(after1); - uint256 secondInterest = outstandingAfter1 / 3; + assertGt(outstandingAfter1, 0, "new interest accrued after reset"); + // Second partial: another chunk of interest + 30 principal -> reset again + uint256 secondInterest = outstandingAfter1 / 3; (, , FixedLoanPosition memory after2) = BrokerMath.deductFixedPositionDebt(secondInterest, 30 ether, after1); - uint256 expectedAfter2 = outstandingAfter1 - secondInterest; - // allow 1 wei tolerance due to Ceil rounding in interest formula - assertApproxEqAbs(_outstanding(after2), expectedAfter2, 1, "second: outstanding exact"); + assertEq(after2.interestRepaid, 0, "second: interest tracking reset"); + assertEq(after2.lastRepaidTime, block.timestamp, "second: lastRepaidTime = now"); assertEq(after2.principalRepaid, 50 ether, "second: cumulative principal"); - - // Outstanding still positive - assertGt(_outstanding(after2), 0, "interest still outstanding"); + assertEq(_outstanding(after2), 0, "second: no outstanding immediately after reset"); } // ==================================================================== @@ -227,10 +236,11 @@ contract BrokerMathDeductFixedTest is Test { } // ==================================================================== - // 1 wei short of full interest -> no reset, exact outstanding + // 1-wei-short interest + principal: principal still triggers reset + // (the 1 wei of unpaid interest is forgiven — the simplified semantic). // ==================================================================== - function test_oneWeiShort_preservesExactOutstanding() public { + function test_oneWeiShort_stillResetsOnPrincipalPayment() public { FixedLoanPosition memory pos = _makePosition(100 ether); skip(DURATION); @@ -241,15 +251,18 @@ contract BrokerMathDeductFixedTest is Test { (, , FixedLoanPosition memory updated) = BrokerMath.deductFixedPositionDebt(interestBudget, 50 ether, pos); - // 1 wei unpaid -> must preserve exactly - assertEq(_outstanding(updated), 1, "exactly 1 wei outstanding preserved"); + // Simplified semantic: the principal payment resets tracking regardless of the 1-wei gap. + assertEq(updated.interestRepaid, 0, "interest tracking reset"); + assertEq(updated.lastRepaidTime, block.timestamp, "lastRepaidTime reset"); + assertEq(_outstanding(updated), 0, "outstanding = 0 right after the reset"); } // ==================================================================== - // Zero interest budget with principal deduction -> maximize preserved + // Zero interest budget + positive principal -> principal triggers reset + // (the entire accrued interest is forgiven — the simplified semantic). // ==================================================================== - function test_zeroInterestBudget_principalOnly_maximizesOutstanding() public { + function test_zeroInterestBudget_principalOnly_resetsInterest() public { FixedLoanPosition memory pos = _makePosition(100 ether); skip(DURATION); @@ -263,46 +276,40 @@ contract BrokerMathDeductFixedTest is Test { assertEq(principalLeft, 0, "principal consumed"); assertEq(updated.principalRepaid, 50 ether, "principal repaid"); - // newTotalAccrued = (100-50)*10%*1year = 5e18, unpaidInterest = 10e18 - // newTotalAccrued < unpaidInterest -> fallback: outstanding = newTotalAccrued - // This is the maximum the formula can represent (better than reset which gives 0) - uint256 newTotalAccrued = BrokerMath.getAccruedInterestForFixedPosition(updated); - assertEq(_outstanding(updated), newTotalAccrued, "fallback preserves maximum possible"); - assertGt(_outstanding(updated), 0, "outstanding > 0 (not reset to zero)"); - // Verify: lastRepaidTime was NOT reset (preserves historical accrual) - assertEq(updated.lastRepaidTime, startTs, "lastRepaidTime not reset in fallback"); + // Simplified semantic: the principal payment resets all interest tracking, + // even though the budget did not include any interest. + assertEq(updated.interestRepaid, 0, "interest tracking reset"); + assertEq(updated.lastRepaidTime, block.timestamp, "lastRepaidTime reset"); + assertEq(_outstanding(updated), 0, "outstanding = 0 right after the reset"); } // ==================================================================== - // Small principal + large interest -> fallback maximizes preserved + // Tiny interest budget + small principal: same reset behaviour. // ==================================================================== - function test_smallPrincipal_largeInterest_maximizesOutstanding() public { + function test_smallPrincipal_largeInterest_resetsOnPrincipalPayment() public { FixedLoanPosition memory pos = _makePosition(100 ether); skip(DURATION); - uint256 accruedInterest = _outstanding(pos); - // Tiny interest budget + small principal uint256 interestBudget = 1; uint256 principalBudget = 5 ether; (, , FixedLoanPosition memory updated) = BrokerMath.deductFixedPositionDebt(interestBudget, principalBudget, pos); - // newTotalAccrued = (100-5)*10%*1year = 9.5e18, unpaidInterest ~= 10e18 - // newTotalAccrued < unpaidInterest -> fallback: maximize outstanding - uint256 newTotalAccrued = BrokerMath.getAccruedInterestForFixedPosition(updated); - assertEq(_outstanding(updated), newTotalAccrued, "fallback preserves max possible"); - assertGt(_outstanding(updated), 0, "outstanding > 0"); - // Verify: better than old code which would give outstanding = 0 - assertGt(_outstanding(updated), accruedInterest / 2, "preserves majority of interest"); + // Simplified semantic: principal payment resets tracking regardless of how much interest was left. + assertEq(updated.interestRepaid, 0, "interest tracking reset"); + assertEq(updated.lastRepaidTime, block.timestamp, "lastRepaidTime reset"); + assertEq(updated.principalRepaid, principalBudget, "principal repaid"); + assertEq(_outstanding(updated), 0, "outstanding = 0 right after the reset"); } // ==================================================================== - // After reset, new interest accrues correctly + // After reset, new interest accrues correctly; a subsequent + // partial-with-principal call resets again. // ==================================================================== - function test_resetThenNewAccrual_worksCorrectly() public { + function test_resetThenNewAccrual_resetsAgainOnSecondPrincipalPayment() public { FixedLoanPosition memory pos = _makePosition(100 ether); skip(DURATION / 2); @@ -315,18 +322,19 @@ contract BrokerMathDeductFixedTest is Test { assertEq(after1.lastRepaidTime, block.timestamp, "lastRepaidTime reset"); assertEq(_outstanding(after1), 0, "no outstanding after reset"); - // More time passes -> new interest accrues on reduced principal + // More time passes -> new interest accrues on the reduced (80e18) principal skip(DURATION / 2); uint256 accrued2 = _outstanding(after1); assertGt(accrued2, 0, "new interest accrued after reset"); - // Partial interest + 30 principal -> should NOT reset, preserve exact + // Partial interest + 30 principal -> resets again per the simplified semantic uint256 partialInterest = accrued2 / 2; (, , FixedLoanPosition memory after2) = BrokerMath.deductFixedPositionDebt(partialInterest, 30 ether, after1); - uint256 expectedOutstanding = accrued2 - partialInterest; - assertEq(_outstanding(after2), expectedOutstanding, "exact after second partial"); + assertEq(after2.interestRepaid, 0, "second: interest tracking reset"); + assertEq(after2.lastRepaidTime, block.timestamp, "second: lastRepaidTime = now"); assertEq(after2.principalRepaid, 50 ether, "cumulative 50 principal"); + assertEq(_outstanding(after2), 0, "no outstanding immediately after reset"); } } diff --git a/test/broker/LendingBroker.t.sol b/test/broker/LendingBroker.t.sol index 8abab106..0160e4e2 100644 --- a/test/broker/LendingBroker.t.sol +++ b/test/broker/LendingBroker.t.sol @@ -825,6 +825,28 @@ contract LendingBrokerTest is Test { assertLe(fixedPositions[0].principal, convertAmount, "fixed principal must be <= amount"); } + /// @notice A user with no dynamic position (or with both principal and interest at zero) must + /// not be able to spawn zero-principal fixed positions via convertDynamicToFixed. + /// Without the post-clamp guard, repeated 1-wei calls would let an attacker fill + /// maxFixedLoanPositions slots with empty entries to grief liquidation gas later. + function test_convertDynamicToFixed_revertsWhenNoDynamicDebt() public { + FixedTermAndRate memory term = FixedTermAndRate({ termId: 55, duration: 30 days, apr: 105 * 1e25 }); + vm.prank(BOT); + broker.updateFixedTermAndRate(term, false); + + // user has never borrowed → no dynamic position + (uint256 principal, uint256 normalizedDebt) = broker.dynamicLoanPositions(borrower); + assertEq(principal, 0, "precondition: no dynamic principal"); + assertEq(normalizedDebt, 0, "precondition: no dynamic normalizedDebt"); + + vm.prank(borrower); + vm.expectRevert(LendingBroker.ZeroAmount.selector); + broker.convertDynamicToFixed(1, 55); + + // confirm: no fixed position was ever created + assertEq(broker.userFixedPositions(borrower).length, 0, "no fixed position should be created"); + } + // ----------------------------- // Fixed borrow and repay (partial and full) // ----------------------------- @@ -1802,7 +1824,7 @@ contract LendingBrokerTest is Test { vm.stopPrank(); vm.prank(borrower); - vm.expectRevert("broker/positions-below-min-loan"); + vm.expectRevert("broker/dynamic-below-min-loan"); broker.repay(minLoan / 2, borrower); } @@ -1868,7 +1890,7 @@ contract LendingBrokerTest is Test { broker.emergencyWithdraw(address(LISUSD), amount); } - function test_checkPositionsMeetsMinLoan_allowsFullRepay() public { + function test_validateDynamicPosition_allowsFullRepay() public { vm.prank(MANAGER); moolah.setMinLoanValue(1e8); uint256 minLoan = moolah.minLoan(marketParams); @@ -1876,7 +1898,7 @@ contract LendingBrokerTest is Test { vm.prank(borrower); broker.borrow(minLoan); - // full repayment leaves zero debt, which BrokerMath.checkPositionsMeetsMinLoan accepts + // full repayment leaves zero principal, which _validateDynamicPosition accepts vm.prank(borrower); broker.repay(minLoan, borrower); @@ -2536,6 +2558,434 @@ contract LendingBrokerTest is Test { payload ); } + + // ============================================= + // repayAll tests + // ============================================= + + /// @notice repayAll clears a dynamic-only position and supplies accrued interest as revenue. + function test_repayAll_dynamicOnly() public { + uint256 borrowAmt = 1000 ether; + vm.prank(borrower); + broker.borrow(borrowAmt); + + // bump rate so meaningful interest accrues + vm.prank(MANAGER); + rateCalc.setMaxRatePerSecond(address(broker), RATE_SCALE + 1e20); + vm.prank(BOT); + rateCalc.setRatePerSecond(address(broker), RATE_SCALE + 1e20); + skip(7 days); + + uint256 rate = rateCalc.accrueRate(address(broker)); + (, uint256 normalizedDebt) = broker.dynamicLoanPositions(borrower); + uint256 actualDebt = BrokerMath.denormalizeBorrowAmount(normalizedDebt, rate); + uint256 interestPortion = actualDebt - borrowAmt; + assertGt(interestPortion, 0, "interest did not accrue"); + + LISUSD.setBalance(borrower, actualDebt); + uint256 relayerBalBefore = LISUSD.balanceOf(address(relayer)); + uint256 vaultSharesBefore = moolah.position(id, address(vault)).supplyShares; + + vm.prank(borrower); + broker.repayAll(borrower); + + // dynamic position cleared + (uint256 principalAfter, uint256 normalizedAfter) = broker.dynamicLoanPositions(borrower); + assertEq(principalAfter, 0, "dynamic principal not cleared"); + assertEq(normalizedAfter, 0, "dynamic normalized debt not cleared"); + + // borrow shares cleared at Moolah + Position memory posAfter = moolah.position(id, borrower); + assertEq(posAfter.borrowShares, 0, "borrow shares not cleared"); + + // borrower spent the entire budget + assertEq(LISUSD.balanceOf(borrower), 0, "borrower retained funds"); + + // revenue (interest) was supplied to relayer / vault + uint256 vaultSharesAfter = moolah.position(id, address(vault)).supplyShares; + bool revenueSupplied = vaultSharesAfter > vaultSharesBefore || + LISUSD.balanceOf(address(relayer)) > relayerBalBefore; + assertTrue(revenueSupplied, "interest not supplied to vault/relayer"); + } + + /// @notice repayAll clears every fixed position and charges full early-repay penalty. + function test_repayAll_fixedOnly_chargesPenalty() public { + vm.startPrank(BOT); + broker.updateFixedTermAndRate(FixedTermAndRate({ termId: 1, duration: 30 days, apr: 110 * 1e25 }), false); + broker.updateFixedTermAndRate(FixedTermAndRate({ termId: 2, duration: 60 days, apr: 115 * 1e25 }), false); + vm.stopPrank(); + + vm.startPrank(borrower); + broker.borrow(500 ether, 1); + broker.borrow(700 ether, 2); + vm.stopPrank(); + + skip(5 days); + moolah.accrueInterest(marketParams); + + // every position should have a non-zero penalty (early repay) + FixedLoanPosition[] memory positions = broker.userFixedPositions(borrower); + assertEq(positions.length, 2); + for (uint256 i = 0; i < positions.length; i++) { + uint256 remaining = positions[i].principal - positions[i].principalRepaid; + assertGt(BrokerMath.getPenaltyForFixedPosition(positions[i], remaining), 0, "penalty not charged"); + } + + LISUSD.setBalance(borrower, 5_000 ether); + uint256 relayerBefore = LISUSD.balanceOf(address(relayer)); + uint256 vaultSharesBefore = moolah.position(id, address(vault)).supplyShares; + + vm.prank(borrower); + broker.repayAll(borrower); + + // every fixed position cleared + assertEq(broker.userFixedPositions(borrower).length, 0, "fixed positions not cleared"); + Position memory posAfter = moolah.position(id, borrower); + assertEq(posAfter.borrowShares, 0, "borrow shares not cleared"); + + // revenue (interest + penalty) reached relayer/vault + uint256 vaultSharesAfter = moolah.position(id, address(vault)).supplyShares; + bool revenueSupplied = vaultSharesAfter > vaultSharesBefore || LISUSD.balanceOf(address(relayer)) > relayerBefore; + assertTrue(revenueSupplied, "no revenue supplied"); + } + + /// @notice repayAll clears dynamic and fixed positions in a single call. + function test_repayAll_dynamicAndFixed() public { + vm.prank(BOT); + broker.updateFixedTermAndRate(FixedTermAndRate({ termId: 1, duration: 30 days, apr: 105 * 1e25 }), false); + + vm.startPrank(borrower); + broker.borrow(800 ether); + broker.borrow(400 ether, 1); + vm.stopPrank(); + + vm.prank(MANAGER); + rateCalc.setMaxRatePerSecond(address(broker), RATE_SCALE + 1e20); + vm.prank(BOT); + rateCalc.setRatePerSecond(address(broker), RATE_SCALE + 1e20); + skip(3 days); + + LISUSD.setBalance(borrower, 10_000 ether); + + vm.prank(borrower); + broker.repayAll(borrower); + + (uint256 dynPrincipal, uint256 dynNormalized) = broker.dynamicLoanPositions(borrower); + assertEq(dynPrincipal, 0, "dynamic not cleared"); + assertEq(dynNormalized, 0, "dynamic normalized not cleared"); + assertEq(broker.userFixedPositions(borrower).length, 0, "fixed not cleared"); + + Position memory posAfter = moolah.position(id, borrower); + assertEq(posAfter.borrowShares, 0, "borrow shares not cleared"); + } + + /// @notice repayAll emits AllPositionsRepaid with the total amount charged. + function test_repayAll_emitsEvent() public { + vm.prank(borrower); + broker.borrow(500 ether); + + skip(1 hours); + + LISUSD.setBalance(borrower, 1_000 ether); + + // we don't pin the totalRepaid amount precisely (interest tiny but non-deterministic); + // assert event topic + emitter, then check positions cleared after + vm.expectEmit(true, false, false, false, address(broker)); + emit IBroker.AllPositionsRepaid(borrower, 0); // value not checked + vm.prank(borrower); + broker.repayAll(borrower); + } + + /// @notice repayAll emits FixedLoanPositionRemoved for every cleared fixed position + /// and DynamicLoanPositionRepaid for the cleared dynamic position. + function test_repayAll_emitsPerPositionEvents() public { + vm.startPrank(BOT); + broker.updateFixedTermAndRate(FixedTermAndRate({ termId: 1, duration: 30 days, apr: 105 * 1e25 }), false); + broker.updateFixedTermAndRate(FixedTermAndRate({ termId: 2, duration: 60 days, apr: 110 * 1e25 }), false); + vm.stopPrank(); + + vm.startPrank(borrower); + broker.borrow(300 ether); + broker.borrow(200 ether, 1); + broker.borrow(150 ether, 2); + vm.stopPrank(); + + FixedLoanPosition[] memory positions = broker.userFixedPositions(borrower); + assertEq(positions.length, 2); + + LISUSD.setBalance(borrower, 5_000 ether); + + // Expect: dynamic repaid event + a removed event for each fixed pos + the all-repaid event. + // Don't pin amount in the dynamic repaid event (interest is tiny but non-deterministic). + vm.expectEmit(true, false, false, false, address(broker)); + emit IBroker.DynamicLoanPositionRepaid(borrower, 0, 0); + vm.expectEmit(true, false, false, true, address(broker)); + emit IBroker.FixedLoanPositionRemoved(borrower, positions[0].posId); + vm.expectEmit(true, false, false, true, address(broker)); + emit IBroker.FixedLoanPositionRemoved(borrower, positions[1].posId); + vm.expectEmit(true, false, false, false, address(broker)); + emit IBroker.AllPositionsRepaid(borrower, 0); + + vm.prank(borrower); + broker.repayAll(borrower); + } + + /// @notice A third party may repayAll on behalf of any borrower. + function test_repayAll_byThirdParty() public { + vm.prank(borrower); + broker.borrow(300 ether); + + address helper = address(0x707); + LISUSD.setBalance(helper, 1_000 ether); + vm.startPrank(helper); + IERC20(address(LISUSD)).approve(address(broker), type(uint256).max); + broker.repayAll(borrower); + vm.stopPrank(); + + (uint256 p, ) = broker.dynamicLoanPositions(borrower); + assertEq(p, 0, "dynamic not cleared"); + Position memory posAfter = moolah.position(id, borrower); + assertEq(posAfter.borrowShares, 0, "borrow shares not cleared"); + assertLt(LISUSD.balanceOf(helper), 1_000 ether, "helper balance unchanged"); + } + + /// @notice repayAll with native BNB clears the position and refunds excess. + function test_repayAll_native_refundsExcess() public { + // bootstrap WBNB totalSupply so the burn-then-mint sequence in withdraw/deposit + // does not underflow→overflow (WBNBMock uses OZ ERC20; deal() does not adjust totalSupply) + vm.deal(address(this), 10_000 ether); + WBNB.deposit{ value: 10_000 ether }(); + + vm.deal(borrower, 1_000 ether); + vm.deal(address(WBNB), 10_000 ether); + + vm.startPrank(borrower); + bnbBroker.borrow(500 ether); + vm.stopPrank(); + + uint256 budget = 600 ether; // overpay + vm.deal(borrower, borrower.balance + budget); + uint256 balBefore = borrower.balance; + + vm.prank(borrower); + bnbBroker.repayAll{ value: budget }(borrower); + + (uint256 p, ) = bnbBroker.dynamicLoanPositions(borrower); + assertEq(p, 0, "dynamic not cleared"); + Position memory posAfter = moolah.position(bnbId, borrower); + assertEq(posAfter.borrowShares, 0, "borrow shares not cleared"); + + uint256 balAfter = borrower.balance; + assertGt(balAfter, balBefore - budget, "no native refund issued"); + } + + /// @notice repayAll reverts when called with native value on a non-WBNB market. + function test_repayAll_revertsNativeOnNonWBNB() public { + vm.prank(borrower); + broker.borrow(100 ether); + + vm.deal(borrower, 1 ether); + vm.expectRevert(LendingBroker.NativeNotSupported.selector); + vm.prank(borrower); + broker.repayAll{ value: 1 ether }(borrower); + } + + /// @notice repayAll reverts when msg.value < totalDebt on the WBNB broker. + function test_repayAll_revertsInsufficientNativeValue() public { + vm.deal(borrower, 1_000 ether); + vm.deal(address(WBNB), 10_000 ether); + + vm.startPrank(borrower); + bnbBroker.borrow(500 ether); + vm.stopPrank(); + + // send a tiny fraction — must revert with InsufficientAmount + vm.expectRevert(LendingBroker.InsufficientAmount.selector); + vm.prank(borrower); + bnbBroker.repayAll{ value: 1 wei }(borrower); + } + + /// @notice repayAll reverts on zero onBehalf address. + function test_repayAll_revertsZeroAddress() public { + vm.expectRevert(LendingBroker.ZeroAddress.selector); + vm.prank(borrower); + broker.repayAll(address(0)); + } + + /// @notice repayAll reverts when the user has no debt. + function test_repayAll_revertsNothingToRepay() public { + vm.expectRevert(LendingBroker.NothingToRepay.selector); + vm.prank(borrower); + broker.repayAll(borrower); + } + + // ============================================= + // Per-position validation tests + // ============================================= + + /// @notice partial repay of a fixed position to below minLoan reverts with the per-fixed error. + /// A second (large) position keeps Moolah's own debt above minLoan so the broker-level + /// validation is the one that fires. + function test_repayFixed_belowMin_reverts() public { + vm.prank(MANAGER); + moolah.setMinLoanValue(100 * 1e8); // minLoan = 100 ether + uint256 minLoan = moolah.minLoan(marketParams); + assertEq(minLoan, 100 ether); + + FixedTermAndRate memory term = FixedTermAndRate({ termId: 100, duration: 30 days, apr: 105 * 1e25 }); + vm.prank(BOT); + broker.updateFixedTermAndRate(term, false); + + vm.startPrank(borrower); + broker.borrow(500 ether); // dynamic — keeps total Moolah debt well above minLoan + broker.borrow(minLoan, term.termId); + vm.stopPrank(); + + FixedLoanPosition[] memory positions = broker.userFixedPositions(borrower); + uint256 posId = positions[0].posId; + + LISUSD.setBalance(borrower, 1_000 ether); + vm.expectRevert("broker/fixed-below-min-loan"); + vm.prank(borrower); + broker.repay(minLoan / 2, posId, borrower); + } + + /// @notice "validate current" isolation: an unrelated below-min fixed position does not block + /// a dynamic borrow on the same user. Old "validate all" would have reverted here. + function test_validation_isolation_borrowDynamicWhenFixedBelowMin() public { + // initial minLoan = 15 ether (set in setUp via Moolah.initialize(_minLoanValue=15e8)) + assertEq(moolah.minLoan(marketParams), 15 ether); + + FixedTermAndRate memory term = FixedTermAndRate({ termId: 200, duration: 30 days, apr: 105 * 1e25 }); + vm.prank(BOT); + broker.updateFixedTermAndRate(term, false); + + // borrow 50 dynamic + 50 fixed (both above the 15-ether minLoan) + vm.startPrank(borrower); + broker.borrow(50 ether); + broker.borrow(50 ether, term.termId); + vm.stopPrank(); + + // raise minLoan so the existing fixed position (50) is now below the new floor (100) + vm.prank(MANAGER); + moolah.setMinLoanValue(100 * 1e8); + assertEq(moolah.minLoan(marketParams), 100 ether); + + // borrow more dynamic — only the dynamic position (which becomes 150) is validated + vm.prank(borrower); + broker.borrow(100 ether); + + (uint256 dynPrincipal, ) = broker.dynamicLoanPositions(borrower); + assertEq(dynPrincipal, 150 ether); + // unrelated below-min fixed position remains untouched + FixedLoanPosition[] memory positions = broker.userFixedPositions(borrower); + assertEq(positions.length, 1); + assertEq(positions[0].principal, 50 ether); + } + + // ============================================= + // Liquidation runs even when leaving below-min + // ============================================= + + /// @notice After my change, liquidation no longer validates positions, so it succeeds even + /// when the resulting principal falls below minLoan. + function test_liquidation_noRevertWhenLeavesPositionBelowMin() public { + _prepareLiquidatablePosition(false); + + // raise minLoan above what each position will end up with after a 50% liquidation. + // setUp creates 40000 dyn + 40000 fixed; ~50% liquidation leaves ~20000 each. + vm.prank(MANAGER); + moolah.setMinLoanValue(25_000 * 1e8); // minLoan = 25,000 ether + + Position memory posBefore = moolah.position(marketParams.id(), borrower); + uint256 userRepayShares = BrokerMath.mulDivCeiling(posBefore.borrowShares, 50 * 1e8, 100 * 1e8); + + LISUSD.setBalance(address(liquidator), 1_000_000 ether); + + // old code would have reverted with broker/positions-below-min-loan; the new code must succeed + vm.prank(BOT); + liquidator.liquidate(Id.unwrap(id), borrower, 0, userRepayShares); + + Position memory posAfter = moolah.position(marketParams.id(), borrower); + assertLt(posAfter.borrowShares, posBefore.borrowShares, "shares should decrease"); + } + + /// @notice Regression: after a partial liquidation, a position can be left in the + /// "interest-only residual" state — `principal == 0` while `normalizedDebt > 0` + /// (because liquidation paid the principal but only a fraction of the broker's + /// accrued interest). This test exercises that path through repayAll: + /// (1) liquidation completes (no broker validation) + /// (2) repayAll's `dynamicInterest > 0` guard fires and clears the residual + /// (3) DynamicLoanPositionRepaid event still emits for the residual + /// (4) no leftover broker tracking, no Moolah debt + function test_liquidation_postResidual_repayAllClearsCleanly() public { + _prepareLiquidatablePosition(false); + + // 50% liquidation of 40k dyn + 40k fixed: dynamic absorbs all 40k principal first, + // leaving dynamic with only an interest residual (principal == 0, normalizedDebt > 0). + Position memory posBefore = moolah.position(marketParams.id(), borrower); + uint256 userRepayShares = BrokerMath.mulDivCeiling(posBefore.borrowShares, 50 * 1e8, 100 * 1e8); + LISUSD.setBalance(address(liquidator), 1_000_000 ether); + + vm.prank(BOT); + liquidator.liquidate(Id.unwrap(id), borrower, 0, userRepayShares); + + // confirm the residual: principal cleared but interest still tracked + (uint256 dynPrincipal, uint256 dynNormDebt) = broker.dynamicLoanPositions(borrower); + assertEq(dynPrincipal, 0, "dynamic principal should be cleared"); + assertGt(dynNormDebt, 0, "dynamic should retain interest residual"); + + FixedLoanPosition[] memory positions = broker.userFixedPositions(borrower); + assertEq(positions.length, 1, "expected one fixed position remaining"); + uint256 fixedPosId = positions[0].posId; + + // repayAll should: charge the residual interest, fire DynamicLoanPositionRepaid (via the + // `dynamicInterest > 0` guard), remove the fixed position, and clear all state. + LISUSD.setBalance(borrower, 1_000_000 ether); + + vm.expectEmit(true, false, false, false, address(broker)); + emit IBroker.DynamicLoanPositionRepaid(borrower, 0, 0); + vm.expectEmit(true, false, false, true, address(broker)); + emit IBroker.FixedLoanPositionRemoved(borrower, fixedPosId); + vm.expectEmit(true, false, false, false, address(broker)); + emit IBroker.AllPositionsRepaid(borrower, 0); + + vm.prank(borrower); + broker.repayAll(borrower); + + (uint256 dynAfter, uint256 normAfter) = broker.dynamicLoanPositions(borrower); + assertEq(dynAfter, 0, "dynamic principal not cleared"); + assertEq(normAfter, 0, "dynamic normalized debt not cleared (interest residual leaked)"); + assertEq(broker.userFixedPositions(borrower).length, 0, "fixed not cleared"); + Position memory moolahAfter = moolah.position(id, borrower); + assertEq(moolahAfter.borrowShares, 0, "moolah debt not cleared"); + } + + /// @notice Broker market runs 0% Moolah-side IRM, so totalBorrowAssets/totalBorrowShares stays + /// locked at 1:VIRTUAL_SHARES across the market's lifetime. A liquidation that passes a + /// repaidShares value not divisible by VIRTUAL_SHARES would have toAssetsUp ceiling round + /// the asset deduction up by 1 wei, drifting the ratio and corrupting later repayments + /// for every borrower in the market. The guard rejects this at the broker layer. + function test_liquidation_revertsOnNonDivisibleRepaidShares() public { + _prepareLiquidatablePosition(false); + + Position memory posBefore = moolah.position(marketParams.id(), borrower); + // 50% clamped to a clean multiple, then add 1 to force non-divisible + uint256 cleanShares = (posBefore.borrowShares / 2 / SharesMathLib.VIRTUAL_SHARES) * SharesMathLib.VIRTUAL_SHARES; + uint256 dirtyShares = cleanShares + 1; + + LISUSD.setBalance(address(liquidator), 1_000_000 ether); + + vm.prank(BOT); + vm.expectRevert(LendingBroker.InvalidRepaidShares.selector); + liquidator.liquidate(Id.unwrap(id), borrower, 0, dirtyShares); + + // clean multiple still succeeds + vm.prank(BOT); + liquidator.liquidate(Id.unwrap(id), borrower, 0, cleanShares); + Position memory posAfter = moolah.position(marketParams.id(), borrower); + assertLt(posAfter.borrowShares, posBefore.borrowShares, "clean shares should liquidate"); + } } /// @dev Mock swap pair that converts tokenIn -> tokenOut at oracle price. From 3d3d63ca05b6bc1ccf94dae906abfad852e7ee3b Mon Sep 17 00:00:00 2001 From: Rick Date: Tue, 16 Jun 2026 17:22:51 +0800 Subject: [PATCH 13/13] fix: resolve post-rebase compilation breaks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - BrokerLiquidator: drop duplicate receive() — master's d309fdc and the branch each declared an identical receive(); Solidity allows only one. The documented receive() at the end of the contract is kept, so the liquidator can still receive native BNB. - BatchManagementUtils.t.sol: update _deployBroker() to the new LendingBroker 2-arg constructor and 8-arg initialize (RELAYER/ORACLE moved from immutables to storage in 22bb8cf). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/liquidator/BrokerLiquidator.sol | 2 -- test/utils/BatchManagementUtils.t.sol | 14 ++++++++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/liquidator/BrokerLiquidator.sol b/src/liquidator/BrokerLiquidator.sol index a8e93039..e759f278 100644 --- a/src/liquidator/BrokerLiquidator.sol +++ b/src/liquidator/BrokerLiquidator.sol @@ -87,8 +87,6 @@ contract BrokerLiquidator is UUPSUpgradeable, AccessControlUpgradeable, IBrokerL _grantRole(BOT, bot); } - receive() external payable {} - /// @dev withdraws ERC20 tokens. /// @param token The address of the token. /// @param amount The amount to withdraw. diff --git a/test/utils/BatchManagementUtils.t.sol b/test/utils/BatchManagementUtils.t.sol index 63530bcf..85420af0 100644 --- a/test/utils/BatchManagementUtils.t.sol +++ b/test/utils/BatchManagementUtils.t.sol @@ -160,10 +160,20 @@ contract BatchManagementUtilsTest is Test { } function _deployBroker() internal returns (LendingBroker b) { - LendingBroker impl = new LendingBroker(address(moolah), RELAYER_STUB, address(oracle), address(0)); + LendingBroker impl = new LendingBroker(address(moolah), address(0)); ERC1967Proxy proxy = new ERC1967Proxy( address(impl), - abi.encodeWithSelector(LendingBroker.initialize.selector, ADMIN, MANAGER, BOT, PAUSER, address(rateCalc), 10) + abi.encodeWithSelector( + LendingBroker.initialize.selector, + ADMIN, + MANAGER, + BOT, + PAUSER, + address(rateCalc), + 10, + RELAYER_STUB, + address(oracle) + ) ); b = LendingBroker(payable(address(proxy))); }