From cbd9b18a99c4e1f92bf9404e88fceb8ebc36d55f Mon Sep 17 00:00:00 2001 From: Debugger022 Date: Fri, 15 Sep 2023 16:58:20 +0530 Subject: [PATCH] fix: pr comments --- contracts/Comptroller.sol | 15 +- contracts/ComptrollerStorage.sol | 6 +- .../Comptroller/liquidateAccountTest.ts | 132 ++++++++---------- 3 files changed, 60 insertions(+), 93 deletions(-) diff --git a/contracts/Comptroller.sol b/contracts/Comptroller.sol index 75d88f7db..33a4fe45c 100644 --- a/contracts/Comptroller.sol +++ b/contracts/Comptroller.sol @@ -478,7 +478,7 @@ contract Comptroller is uint256 borrowBalance = VToken(vTokenBorrowed).borrowBalanceStored(borrower); /* Allow accounts to be liquidated if the market is deprecated or it is a forced liquidation */ - if (skipLiquidityCheck || isDeprecated(VToken(vTokenBorrowed)) || isForcedLiquidationEnabled[vTokenBorrowed]) { + if (skipLiquidityCheck || isForcedLiquidationEnabled[vTokenBorrowed]) { if (repayAmount > borrowBalance) { revert TooMuchRepay(); } @@ -1244,19 +1244,6 @@ contract Comptroller is return _actionPaused[market][action]; } - /** - * @notice Check if a vToken market has been deprecated - * @dev All borrows in a deprecated vToken market can be immediately liquidated - * @param vToken The market to check if deprecated - * @return deprecated True if the given vToken market has been deprecated - */ - function isDeprecated(VToken vToken) public view returns (bool) { - return - markets[address(vToken)].collateralFactorMantissa == 0 && - actionPaused(address(vToken), Action.BORROW) && - vToken.reserveFactorMantissa() == MANTISSA_ONE; - } - /** * @notice Add the market to the borrower's "assets in" for liquidity calculations * @param vToken The market to enter diff --git a/contracts/ComptrollerStorage.sol b/contracts/ComptrollerStorage.sol index af90d8d23..6732a6349 100644 --- a/contracts/ComptrollerStorage.sol +++ b/contracts/ComptrollerStorage.sol @@ -107,6 +107,9 @@ contract ComptrollerStorage { // Used to check if rewards distributor is added mapping(address => bool) internal rewardsDistributorExists; + /// @notice Flag indicating whether forced liquidation enabled for a market + mapping(address => bool) public isForcedLiquidationEnabled; + uint256 internal constant NO_ERROR = 0; // closeFactorMantissa must be strictly greater than this value @@ -118,9 +121,6 @@ contract ComptrollerStorage { // No collateralFactorMantissa may exceed this value uint256 internal constant MAX_COLLATERAL_FACTOR_MANTISSA = 0.9e18; // 0.9 - /// @notice Flag indicating whether forced liquidation enabled for a market - mapping(address => bool) public isForcedLiquidationEnabled; - /** * @dev This empty reserved space is put in place to allow future versions to add new * variables without shifting down storage in the inheritance chain. diff --git a/tests/hardhat/Comptroller/liquidateAccountTest.ts b/tests/hardhat/Comptroller/liquidateAccountTest.ts index e5b006e14..75fbd4f06 100644 --- a/tests/hardhat/Comptroller/liquidateAccountTest.ts +++ b/tests/hardhat/Comptroller/liquidateAccountTest.ts @@ -382,90 +382,70 @@ describe("liquidateAccount", () => { describe("preLiquidateHook", async () => { let accounts: SignerWithAddress[]; - before(async () => { + beforeEach(async () => { accounts = await ethers.getSigners(); + await comptroller.setForcedLiquidation(OMG.address, true); }); - const generalTests = () => { - it("reverts if borrowed market is not listed", async () => { - const someVToken = await smock.fake("VToken"); - await expect( - comptroller.preLiquidateHook( - someVToken.address, - OMG.address, - accounts[0].address, - parseUnits("1", 18), - false, - ), - ).to.be.revertedWithCustomError(comptroller, "MarketNotListed"); - }); - it("reverts if collateral market is not listed", async () => { - const someVToken = await smock.fake("VToken"); - await expect( - comptroller.preLiquidateHook( - OMG.address, - someVToken.address, - accounts[0].address, - parseUnits("1", 18), - false, - ), - ).to.be.revertedWithCustomError(comptroller, "MarketNotListed"); - }); - }; + it("reverts if borrowed market is not listed", async () => { + const someVToken = await smock.fake("VToken"); + await expect( + comptroller.preLiquidateHook(someVToken.address, OMG.address, accounts[0].address, parseUnits("1", 18), false), + ).to.be.revertedWithCustomError(comptroller, "MarketNotListed"); + }); - describe("isForcedLiquidationEnabled == true", async () => { - beforeEach(async () => { - await comptroller.setForcedLiquidation(OMG.address, true); - }); + it("reverts if collateral market is not listed", async () => { + const someVToken = await smock.fake("VToken"); + await expect( + comptroller.preLiquidateHook(OMG.address, someVToken.address, accounts[0].address, parseUnits("1", 18), false), + ).to.be.revertedWithCustomError(comptroller, "MarketNotListed"); + }); - generalTests(); - - it("allows liquidations without shortfall", async () => { - OMG.borrowBalanceStored.returns(parseUnits("100", 18)); - await comptroller.callStatic.preLiquidateHook( - OMG.address, - OMG.address, - accounts[0].address, - parseUnits("1", 18), - true, - ); - }); + it("allows liquidations without shortfall", async () => { + OMG.borrowBalanceStored.returns(parseUnits("100", 18)); + await comptroller.callStatic.preLiquidateHook( + OMG.address, + OMG.address, + accounts[0].address, + parseUnits("1", 18), + true, + ); + }); - it("allows to repay 100% of the borrow", async () => { - OMG.borrowBalanceStored.returns(parseUnits("1", 18)); - await comptroller.callStatic.preLiquidateHook( - OMG.address, - OMG.address, - accounts[0].address, - parseUnits("1", 18), - false, - ); - }); + it("allows to repay 100% of the borrow", async () => { + OMG.borrowBalanceStored.returns(parseUnits("1", 18)); + await comptroller.callStatic.preLiquidateHook( + OMG.address, + OMG.address, + accounts[0].address, + parseUnits("1", 18), + false, + ); + }); - it("fails with TOO_MUCH_REPAY if trying to repay > borrowed amount", async () => { - OMG.borrowBalanceStored.returns(parseUnits("0.99", 18)); - const tx = comptroller.callStatic.preLiquidateHook( - OMG.address, - OMG.address, - accounts[0].address, - parseUnits("1", 18), - false, - ); - await expect(tx).to.be.revertedWithCustomError(comptroller, "TooMuchRepay"); - }); + it("fails with TOO_MUCH_REPAY if trying to repay > borrowed amount", async () => { + OMG.borrowBalanceStored.returns(parseUnits("0.99", 18)); + const tx = comptroller.callStatic.preLiquidateHook( + OMG.address, + OMG.address, + accounts[0].address, + parseUnits("1", 18), + false, + ); + await expect(tx).to.be.revertedWithCustomError(comptroller, "TooMuchRepay"); + }); - it("checks the shortfall if isForcedLiquidationEnabled is set back to false", async () => { - await comptroller.setForcedLiquidation(OMG.address, false); - OMG.borrowBalanceStored.returns(parseUnits("100", 18)); - const tx = comptroller.callStatic.preLiquidateHook( - OMG.address, - OMG.address, - accounts[0].address, - parseUnits("1", 18), - false, - ); - await expect(tx).to.be.revertedWithCustomError(comptroller, "MinimalCollateralViolated"); - }); + it("checks the shortfall if isForcedLiquidationEnabled is set back to false", async () => { + await comptroller.setForcedLiquidation(OMG.address, false); + OMG.borrowBalanceStored.returns(parseUnits("100", 18)); + const tx = comptroller.callStatic.preLiquidateHook( + OMG.address, + OMG.address, + accounts[0].address, + parseUnits("1", 18), + false, + ); + await expect(tx).to.be.revertedWithCustomError(comptroller, "MinimalCollateralViolated"); }); }); });