[VEN-3321]: Liquidation Threshold and repay Improvements in Core Pool on BNB by Debugger022 · Pull Request #604 · VenusProtocol/venus-protocol

Debugger022 · 2025-07-09T13:02:22Z

Description

This PR implements the following features:-

[VEN-3321] - Add the concept of Liquidation Threshold to the core Pool on BNB.
[VEN-3322] - Dynamic Liquidation Incentive and Close Factor in core Pool on BNB.
[VEN-3323] - Define maximum Liquidation Incentive per seized asset in the core pool on BNB.
[VEN-233] - Repay logic improvisation

This pull request introduces significant enhancements and refactoring to the contracts, primarily to support dynamic liquidation incentives, improved health factor calculations, and integration with a new LiquidationManager contract. The changes include new interfaces and storage structures, updated function signatures, and expanded account health and liquidation logic.

Liquidation and Account Health Enhancements

Added new functions and updated existing ones in ComptrollerInterface.sol to support dynamic liquidation incentives and close factor, health factor calculations, and detailed account snapshots, including overloaded methods for seize token calculations.

LiquidationManager Integration

Added references to the new LiquidationManager contract in both storage and interface files, and exposed its address via the Comptroller contract.

Storage and Contract Refactoring

Deprecated old close factor and liquidation incentive variables, replacing them with new ones (__oldCloseFactorMantissaSlot, maxLiquidationIncentiveMantissa) and moved to ComptrollerV19Storage. Updated all relevant contract inheritance to use the new storage version. Also, removed the old setCloseFactor and setLiquidationIncentive functions as they will now be calculated dynamically using the liquidationManager.

Diamond Facet and Functionality Updates

Updated FacetBase and MarketFacet to use new health snapshot logic and ComptrollerLens integration, including new internal and external functions for health factor calculations and liquidation seize token calculations.

Repay logic Improvisation

Updated the repayAmount calculation in the VToken from this to this:
vars.repayAmount = repayAmount >= vars.accountBorrows ? vars.accountBorrows : repayAmount;

…ruct

contracts/Comptroller/Diamond/facets/SetterFacet.sol

contracts/Comptroller/Diamond/facets/PolicyFacet.sol

GitGuru7 · 2025-12-04T06:17:51Z

contracts/Comptroller/ComptrollerStorage.sol

+     * @notice Multiplier used to calculate the maximum repayAmount when liquidating a borrow (deprecated)
     */
-    uint256 public closeFactorMantissa;
+    uint256 public __oldCloseFactorMantissaSlot;


we could make it private then

GitGuru7 · 2025-12-04T06:22:03Z

contracts/Comptroller/Diamond/facets/FacetBase.sol

+    function getHypotheticalHealthSnapshot(
+        address account,
+        VToken vTokenModify,
+        uint256 redeemTokens,
+        uint256 borrowAmount,
+        WeightFunction weightingStrategy
+    ) external view returns (uint256 err, ComptrollerLensInterface.AccountSnapshot memory snapshot) {


We should move this external function to the policy facets to reduce the contract size across the other facets. The internal version can remain here in the FacetBase so it's still accessible to all facets that need it.

GitGuru7 · 2025-12-04T06:28:13Z

contracts/Lens/ComptrollerLens.sol

-                vars.sumBorrowPlusEffects = mul_ScalarTruncateAddUInt(
-                    vars.oraclePrice,
+                // borrow effect: oraclePrice * borrowAmount
+                snapshot.borrows = mul_ScalarTruncateAddUInt(


Not required, but we could call this totalBorrows to keep the naming consistent with totalCollateral.

GitGuru7 · 2025-12-04T06:31:34Z

contracts/Lens/ComptrollerLens.sol

+     */
+    function _finalizeSnapshot(AccountSnapshot memory snapshot) internal pure {
+        if (snapshot.totalCollateral > 0) {
+            snapshot.liquidationThresholdAvg = div_(snapshot.weightedCollateral * 1e18, snapshot.totalCollateral);


This value could represent either the LT or the CF, depending on the WeightFunction being applied right?

GitGuru7 · 2025-12-04T07:09:52Z

contracts/Comptroller/Diamond/facets/MarketFacet.sol

+     * @param liquidationIncentiveMantissa The liquidation incentive to apply
     * @return (errorCode, number of vTokenCollateral tokens to be seized in a liquidation)
     */
    function liquidateCalculateSeizeTokens(
-        address borrower,
        address vTokenBorrowed,
        address vTokenCollateral,
-        uint256 actualRepayAmount
+        uint256 actualRepayAmount,
+        uint256 liquidationIncentiveMantissa
    ) external view returns (uint256, uint256) {


This liquidateCalculateSeizeTokens variant (without the borrower parameter) is intentionally kept for vBNB, since vBNB is non-upgradeable and cannot adopt the updated signature. I missed adding the explanatory comment earlier, please avoid making changes to this version and add a note similar to the one in the corresponding ComptrollerLens function.

GitGuru7 · 2025-12-04T07:40:23Z

contracts/Comptroller/Diamond/facets/SetterFacet.sol

+     * @param newMaxLiquidationIncentive The new max liquidation incentive, scaled by 1e18
+     * @return uint256 0=success, otherwise reverted
+     */
+    function __setMarketMaxLiquidationIncentive(


Please move the internal functions below the external ones to keep the function

GitGuru7 · 2025-12-04T07:43:22Z

contracts/Utils/ErrorReporter.sol

        SET_COLLATERAL_FACTOR_NO_EXISTS,
        SET_COLLATERAL_FACTOR_VALIDATION,
        SET_COLLATERAL_FACTOR_WITHOUT_PRICE,
+        SET_COLLATERAL_FACTOR_VALIDATION_LIQUIDATION_THRESHOLD,


We could add the new error code at the end of the enum to avoid breaking existing error-code ordering.

GitGuru7 · 2025-12-04T07:45:13Z

contracts/Tokens/VTokens/VBep20.sol

    function liquidateBorrow(
        address borrower,
        uint repayAmount,
-        VTokenInterface vTokenCollateral
+        VTokenInterface vTokenCollateral,
+        ComptrollerLensInterface.AccountSnapshot memory snapshot


Why should the liquidator provide the snapshot? They could pass an invalid or manipulated snapshot.

The liquidateBorrow function can only be called from the liquidator contract by the liquidator. In this function, we calculate the snapshot internally, preventing the liquidator from passing an invalid snapshot.

GitGuru7 · 2025-12-04T07:46:44Z

contracts/Tokens/VTokens/VToken.sol

+     * @param snapshot The account snapshot of the borrower
+     * @return (uint, uint) An error code (0=success, otherwise a failure, see ErrorReporter.sol), and the actual repayment amount.
+     */
+    function liquidateBorrowInternal(


why do we need two variants of this function ?

One variant keeps vBNB compatibility, while the other implements the dynamic liquidation logic.

GitGuru7 · 2025-12-04T07:46:53Z

contracts/Tokens/VTokens/VToken.sol

+        }
+
+        // liquidateBorrowFresh emits borrow-specific logs on errors, so we don't need to
+        return liquidateBorrowFresh(msg.sender, borrower, repayAmount, vTokenCollateral, snapshot);


why do we need two variants of this function ?

One variant keeps vBNB compatibility, while the other implements the dynamic liquidation logic.

GitGuru7 · 2025-12-04T08:18:38Z

contracts/Comptroller/Diamond/facets/MarketFacet.sol

+     * @param actualRepayAmount The amount of vTokenBorrowed underlying to convert into vTokenCollateral tokens
+     * @return (errorCode, number of vTokenCollateral tokens to be seized in a liquidation)
+     */
+    function liquidateVAICalculateSeizeTokens(


Since this calculation is only needed by the VAIController, why do we maintain two variants of liquidateVAICalculateSeizeTokens?

GitGuru7 · 2025-12-04T08:23:28Z

contracts/Comptroller/Diamond/facets/MarketFacet.sol

+     * @param healthFactor The health factor of the borrower
+     * @return incentive The dynamic liquidation incentive for the borrower, scaled by 1e18
+     */
+    function getDynamicLiquidationIncentive(


This variant of getDynamicLiquidationIncentive (the one that doesn’t take the borrower address) is currently used only in the Liquidator contract, but we need to account for eMode groups there as well. The only cases where eMode can safely be ignored are vBNB and VAI.

GitGuru7 · 2025-12-04T08:28:41Z

contracts/Lens/ComptrollerLens.sol


            // Read the balances and exchange rate from the vToken
-            (oErr, vars.vTokenBalance, vars.borrowBalance, vars.exchangeRateMantissa) = asset.getAccountSnapshot(
+            (vars.err, vars.vTokenBalance, vars.borrowBalance, vars.exchangeRateMantissa) = asset.getAccountSnapshot(


We can use the returned errorCode directly instead of storing it in vars. In that case, we can remove err from AccountSnapshotLocal entirely.

GitGuru7 · 2025-12-04T08:29:11Z

contracts/Tokens/VTokens/legacy/VTokenR1.sol


        /* We calculate the number of collateral tokens that will be seized */
        (uint amountSeizeError, uint seizeTokens) = comptroller.liquidateCalculateSeizeTokens(
+            borrower,


Please consider avoiding changes to legacy contracts so we can preserve their ABI.

github-actions · 2025-12-16T10:39:09Z

Package	Line Rate	Branch Rate	Health
contracts	73%	59%	➖
contracts.Admin	88%	41%	✔
contracts.Comptroller	100%	90%	✔
contracts.Comptroller.Diamond	95%	61%	✔
contracts.Comptroller.Diamond.facets	81%	68%	✔
contracts.Comptroller.Diamond.interfaces	100%	100%	✔
contracts.Comptroller.Types	100%	100%	✔
contracts.Comptroller.legacy	100%	100%	✔
contracts.Comptroller.legacy.Diamond	0%	0%	❌
contracts.Comptroller.legacy.Diamond.facets	0%	0%	❌
contracts.Comptroller.legacy.Diamond.interfaces	100%	100%	✔
contracts.DelegateBorrowers	100%	89%	✔
contracts.FlashLoan.interfaces	100%	100%	✔
contracts.Governance	68%	45%	➖
contracts.InterestRateModels	74%	59%	➖
contracts.Lens	43%	38%	❌
contracts.Liquidator	84%	59%	✔
contracts.Oracle	100%	100%	✔
contracts.PegStability	88%	84%	✔
contracts.Swap	88%	57%	✔
contracts.Swap.interfaces	100%	100%	✔
contracts.Swap.lib	81%	53%	✔
contracts.Tokens	100%	100%	✔
contracts.Tokens.Prime	96%	72%	✔
contracts.Tokens.Prime.Interfaces	100%	100%	✔
contracts.Tokens.Prime.libs	90%	76%	✔
contracts.Tokens.VAI	82%	53%	✔
contracts.Tokens.VRT	20%	9%	❌
contracts.Tokens.VTokens	69%	51%	➖
contracts.Tokens.VTokens.legacy	0%	0%	❌
contracts.Tokens.VTokens.legacy.Utils	0%	0%	❌
contracts.Tokens.XVS	19%	8%	❌
contracts.Tokens.test	100%	100%	✔
contracts.Utils	52%	31%	➖
contracts.VAIVault	50%	45%	➖
contracts.VRTVault	49%	36%	❌
contracts.XVSVault	63%	50%	➖
contracts.external	100%	100%	✔
contracts.lib	89%	71%	✔
Summary	56% (3725 / 6596)	42% (1407 / 3322)	➖

web3rover · 2025-12-30T17:57:21Z

contracts/Comptroller/Diamond/facets/MarketFacet.sol

+        address borrower,
+        address vToken
+    ) external view returns (uint256 incentive) {
+        uint256 effectiveLiquidationIncentive = this.getEffectiveLiquidationIncentive(borrower, vToken);


We can make getEffectiveLiquidationIncentive an public function so that we can avoid the use of this. We don't follow this pattern in our contracts. Use of this is more expensive.

web3rover · 2025-12-30T17:58:01Z

contracts/Comptroller/Diamond/facets/PolicyFacet.sol


        /* The borrower must have shortfall in order to be liquidatable */
-        (Error err, , uint256 shortfall) = getHypotheticalAccountLiquidityInternal(
+        (uint256 err, ComptrollerLensInterface.AccountSnapshot memory snapshot) = this.getHypotheticalHealthSnapshot(


Same here we can avoid the use of this

web3rover · 2025-12-30T18:06:55Z

contracts/Comptroller/Diamond/facets/MarketFacet.sol

     * @dev Calculates incentive based on provided health factor and liquidation threshold average,
     *      without reading borrower's account state from storage
     * @param vToken The address of the vToken to be seized
+     * @param borrower The address of the borrower


We can expand the comment to include why the borrower parameter is needed

feat: add Liquidation Threshold in Core Pool on BNB

f608f86

Debugger022 self-assigned this Jul 9, 2025

Debugger022 and others added 10 commits July 10, 2025 12:19

fix: minor fix

294f79e

fix: fix test

fae7c2a

fix: fix test for liquidation threshold setter

e139b03

fix: spelling correction

7ba7bac

test: add tests for borrow and user liquidation state

624c961

test: add liquidation functionality test for Liquidator contract

10d7785

test: add test for XVS rewards claiming functionality

b04a893

feat: add market liquidation incentive and update liquidity calculations

cb5a5db

refactor: add liquidation threshold and incentive params to Market st…

a6d1926

…ruct

feat!: migrate markets and Comptroller to Solidity 0.8.25

f16a9b5

chechu reviewed Jul 28, 2025

View reviewed changes

contracts/Comptroller/Diamond/facets/SetterFacet.sol Show resolved Hide resolved

chechu reviewed Jul 28, 2025

View reviewed changes

contracts/Comptroller/Diamond/facets/SetterFacet.sol Outdated Show resolved Hide resolved

Debugger022 added 8 commits July 28, 2025 15:01

refactor: update references to Market with new params

caeea51

feat: add function to calculate health factor of an account

cf0354b

refactor: getter function for account health factors

da9ef57

feat: add Toxic liquidation check in liquidate hook

8a38506

feat: add dynamic liquidation incentive per asset

ff22d54

feat: update Liquidator contract to use dynamic liquidation incentive

c4786ce

refactor: update calculate SeizeTokens params

1e8f708

fix: minor fix

f7e3f53

chechu reviewed Jul 28, 2025

View reviewed changes

contracts/Comptroller/Diamond/facets/PolicyFacet.sol Outdated Show resolved Hide resolved

Debugger022 added 7 commits July 28, 2025 18:24

fix: lint

91316bb

feat: add external Liquidation Manager contract

dc16c67

feat: setter for Liquidation Manager

b526832

feat: using Liquidation Manager functions

348fbb3

fix: fix comptroller unit tests

7eeeaa2

refactor: update markets return params

69f820e

fix: fix VAI tests

4ea3457

test: update TokenRedeemer test

9ce8a5a

Debugger022 changed the title ~~[VEN-3321]: Liquidation Threshold and Improvements in Core Pool on BNB~~ [VEN-3321]: Liquidation Threshold and repay Improvements in Core Pool on BNB Nov 27, 2025

Debugger022 and others added 8 commits December 1, 2025 17:49

test: add tests for liquidation scenario

1560cc2

Merge branch 'develop' into feat/VEN-3321

5ad06f6

fix: lint

1659da4

feat: updating deployment files

8b972e1

fix: minor fixes

da82294

feat: update bsctestnet deployment files

f9d6dc0

feat: update liquidationManager deployment

b29d11a

feat: updating deployment files

44b4d88

GitGuru7 reviewed Dec 4, 2025

View reviewed changes

fred-venus force-pushed the develop branch 6 times, most recently from f8786ac to d1378e1 Compare December 12, 2025 09:03

Debugger022 and others added 7 commits December 15, 2025 13:51

refactor: rename borrows to totalBorrows and update related logic

2361f5e

refactor: update dynamic liquidation mechanism with respect to e-mode

fff566c

refactor: remove liquidateVAICalculateSeizeTokens unused variant

9072738

Merge branch 'develop' into feat/VEN-3321

545cd00

fix: lint and prettier

b008927

feat: updating deployment files

f83afff

refactor: update tests for new liquidation changes

7dd2804

refactor: update fork tests for dynamic liquidation mechanism

6fa0386

web3rover reviewed Dec 30, 2025

View reviewed changes

Conversation

Debugger022 commented Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Dec 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Debugger022 commented Jul 9, 2025 •

edited

Loading