Skip to content

Commit

Permalink
refactor: reduce comptroller size
Browse files Browse the repository at this point in the history
  • Loading branch information
Debugger022 committed Dec 30, 2024
1 parent a8bebee commit 673612a
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 22 deletions.
63 changes: 53 additions & 10 deletions contracts/Comptroller.sol
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,24 @@ contract Comptroller is
/// @notice Thrown when collateral factor is not zero
error CollateralFactorIsNotZero();

/// @notice Thrown when the close factor is invalid
error InvalidCloseFactor();

/// @notice Thrown when the liquidation incentive is invalid
error InvalidLiquidationIncentive();

/// @notice Thrown when the VToken is invalid
error InvalidVToken();

/// @notice Thrown when the input is invalid
error InvalidInput();

/// @notice Thrown when the rewards distributor already exists
error RewardsDistributorAlreadyExists();

/// @notice Thrown when the market does not exist
error MarketNotExist();

/**
* @notice Thrown during the liquidation if user's total collateral amount is lower than
* a predefined threshold. In this case only batch liquidations (either liquidateAccount
Expand Down Expand Up @@ -1062,7 +1080,10 @@ contract Comptroller is

for (uint256 i; i < marketsCount; ++i) {
(, uint256 borrowBalance, ) = _safeGetAccountSnapshot(borrowMarkets[i], borrower);
require(borrowBalance == 0, "Nonzero borrow balance after liquidation");
// require(borrowBalance == 0, "Nonzero borrow balance after liquidation");
if (borrowBalance > 0) {
revert NonzeroBorrowBalance();
}
}
}

Expand All @@ -1074,8 +1095,11 @@ contract Comptroller is
*/
function setCloseFactor(uint256 newCloseFactorMantissa) external {
_checkAccessAllowed("setCloseFactor(uint256)");
require(MAX_CLOSE_FACTOR_MANTISSA >= newCloseFactorMantissa, "Close factor greater than maximum close factor");
require(MIN_CLOSE_FACTOR_MANTISSA <= newCloseFactorMantissa, "Close factor smaller than minimum close factor");
if (MAX_CLOSE_FACTOR_MANTISSA < newCloseFactorMantissa || MIN_CLOSE_FACTOR_MANTISSA > newCloseFactorMantissa) {
revert InvalidCloseFactor();
}
// require(MAX_CLOSE_FACTOR_MANTISSA >= newCloseFactorMantissa, "Close factor greater than maximum close factor");
// require(MIN_CLOSE_FACTOR_MANTISSA <= newCloseFactorMantissa, "Close factor smaller than minimum close factor");

uint256 oldCloseFactorMantissa = closeFactorMantissa;
closeFactorMantissa = newCloseFactorMantissa;
Expand Down Expand Up @@ -1150,7 +1174,10 @@ contract Comptroller is
* @custom:access Controlled by AccessControlManager
*/
function setLiquidationIncentive(uint256 newLiquidationIncentiveMantissa) external {
require(newLiquidationIncentiveMantissa >= MANTISSA_ONE, "liquidation incentive should be greater than 1e18");
if (newLiquidationIncentiveMantissa < MANTISSA_ONE) {
revert InvalidLiquidationIncentive();
}
// require(newLiquidationIncentiveMantissa >= MANTISSA_ONE, "liquidation incentive should be greater than 1e18");

_checkAccessAllowed("setLiquidationIncentive(uint256)");

Expand Down Expand Up @@ -1178,7 +1205,11 @@ contract Comptroller is
revert MarketAlreadyListed(address(vToken));
}

require(vToken.isVToken(), "Comptroller: Invalid vToken"); // Sanity check to make sure its really a VToken
if (!vToken.isVToken()) {
revert InvalidVToken();
}

// require(vToken.isVToken(), "Comptroller: Invalid vToken"); // Sanity check to make sure its really a VToken

Market storage newMarket = markets[address(vToken)];
newMarket.isListed = true;
Expand Down Expand Up @@ -1212,7 +1243,10 @@ contract Comptroller is
uint256 numMarkets = vTokens.length;
uint256 numBorrowCaps = newBorrowCaps.length;

require(numMarkets != 0 && numMarkets == numBorrowCaps, "invalid input");
if (numMarkets == 0 || numMarkets != numBorrowCaps) {
revert InvalidInput();
}
// require(numMarkets != 0 && numMarkets == numBorrowCaps, "invalid input");

_ensureMaxLoops(numMarkets);

Expand All @@ -1236,8 +1270,11 @@ contract Comptroller is
_checkAccessAllowed("setMarketSupplyCaps(address[],uint256[])");
uint256 vTokensCount = vTokens.length;

require(vTokensCount != 0, "invalid number of markets");
require(vTokensCount == newSupplyCaps.length, "invalid number of markets");
if (vTokensCount == 0 || vTokensCount != newSupplyCaps.length) {
revert InvalidInput();
}
// require(vTokensCount != 0, "invalid number of markets");
// require(vTokensCount == newSupplyCaps.length, "invalid number of markets");

_ensureMaxLoops(vTokensCount);

Expand Down Expand Up @@ -1295,7 +1332,10 @@ contract Comptroller is
* @custom:event Emits NewRewardsDistributor with distributor address
*/
function addRewardsDistributor(RewardsDistributor _rewardsDistributor) external onlyOwner {
require(!rewardsDistributorExists[address(_rewardsDistributor)], "already exists");
if (rewardsDistributorExists[address(_rewardsDistributor)]) {
revert RewardsDistributorAlreadyExists();
}
// require(!rewardsDistributorExists[address(_rewardsDistributor)], "already exists");

uint256 rewardsDistributorsLen = rewardsDistributors.length;
_ensureMaxLoops(rewardsDistributorsLen + 1);
Expand Down Expand Up @@ -1633,7 +1673,10 @@ contract Comptroller is
* @param paused The new paused state (true=paused, false=unpaused)
*/
function _setActionPaused(address market, Action action, bool paused) internal {
require(markets[market].isListed, "cannot pause a market that is not listed");
if (!markets[market].isListed) {
revert MarketNotExist();
}
// require(markets[market].isListed, "cannot pause a market that is not listed");
_actionPaused[market][action] = paused;
emit ActionPausedMarket(VToken(market), action, paused);
}
Expand Down
5 changes: 3 additions & 2 deletions tests/hardhat/Comptroller/liquidateAccountTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -338,8 +338,9 @@ describe("liquidateAccount", () => {
beforeLiquidation: { supply: 0, borrows: parseUnits("1", 18) },
afterLiquidation: { supply: 0, borrows: 0 },
});
await expect(comptroller.connect(liquidator).liquidateAccount(user.address, [])).to.be.revertedWith(
"Nonzero borrow balance after liquidation",
await expect(comptroller.connect(liquidator).liquidateAccount(user.address, [])).to.be.revertedWithCustomError(
comptroller,
"NonzeroBorrowBalance",
);
});
});
Expand Down
5 changes: 3 additions & 2 deletions tests/hardhat/Comptroller/pauseTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,9 @@ describe("Comptroller", () => {
});

it("reverts if the market is not listed", async () => {
await expect(comptroller.setActionsPaused([SKT.address], [1], true)).to.be.revertedWith(
"cannot pause a market that is not listed",
await expect(comptroller.setActionsPaused([SKT.address], [1], true)).to.be.revertedWithCustomError(
comptroller,
"MarketNotExist",
);
});

Expand Down
20 changes: 14 additions & 6 deletions tests/hardhat/Comptroller/setters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,9 @@ describe("setters", async () => {

it("reverts if re-adding same rewardDistributor", async () => {
await comptroller.addRewardsDistributor(newRewardsDistributor.address);
await expect(comptroller.addRewardsDistributor(newRewardsDistributor.address)).to.be.revertedWith(
"already exists",
await expect(comptroller.addRewardsDistributor(newRewardsDistributor.address)).to.be.revertedWithCustomError(
comptroller,
"RewardsDistributorAlreadyExists",
);
});
});
Expand Down Expand Up @@ -147,17 +148,24 @@ describe("setters", async () => {

describe("SupplyAndBorrowCaps", async () => {
it("reverts if token data is invalid", async () => {
await expect(comptroller.setMarketSupplyCaps([], [1, 2])).to.be.revertedWith("invalid number of markets");
await expect(comptroller.setMarketSupplyCaps([], [1, 2])).to.be.revertedWithCustomError(
comptroller,
"InvalidInput",
);
});

it("reverts if supply and token data is invalid", async () => {
await expect(comptroller.setMarketSupplyCaps([OMG.address], [1, 2])).to.be.revertedWith(
"invalid number of markets",
await expect(comptroller.setMarketSupplyCaps([OMG.address], [1, 2])).to.be.revertedWithCustomError(
comptroller,
"InvalidInput",
);
});

it("reverts if borrow and token data is invalid", async () => {
await expect(comptroller.setMarketBorrowCaps([OMG.address], [1, 2])).to.be.revertedWith("invalid input");
await expect(comptroller.setMarketBorrowCaps([OMG.address], [1, 2])).to.be.revertedWithCustomError(
comptroller,
"InvalidInput",
);
});
});

Expand Down
5 changes: 3 additions & 2 deletions tests/integration/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -484,8 +484,9 @@ describe("Straight Cases For Single User Liquidation and healing", function () {
dummyPriceOracle.getUnderlyingPrice.whenCalledWith(vBNX.address).returns(convertToUnit("100", 12));
dummyPriceOracle.getUnderlyingPrice.whenCalledWith(vBTCB.address).returns(convertToUnit("100", 12));
await Comptroller.setPriceOracle(dummyPriceOracle.address);
await expect(Comptroller.connect(acc1Signer).liquidateAccount(acc2, [param])).to.be.revertedWith(
"Nonzero borrow balance after liquidation",
await expect(Comptroller.connect(acc1Signer).liquidateAccount(acc2, [param])).to.be.revertedWithCustomError(
Comptroller,
"NonzeroBorrowBalance",
);
});

Expand Down

0 comments on commit 673612a

Please sign in to comment.