Skip to content

Commit

Permalink
fix: liquidation covered with tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@0xmikko
0xmikko committed May 24, 2023
1 parent 23a1d2f commit bd94bda
Show file tree
Hide file tree
Showing 6 changed files with 117 additions and 138 deletions.
90 changes: 0 additions & 90 deletions .favorites.json
View file

This file was deleted.

3 changes: 2 additions & 1 deletion .gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,5 @@ forge-out/
.mainnet.test.cache
*.log
.eslintcache
.eslint.local.json
.eslint.local.json
.favorites.json
18 changes: 9 additions & 9 deletions contracts/credit/CreditFacadeV3.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -421,13 +421,13 @@ contract CreditFacadeV3 is ICreditFacade, ACLNonReentrantTrait {

if (calls.length != 0) {
FullCheckParams memory fullCheckParams =
_multicall(creditAccount, calls, collateralDebtData.enabledTokensMask, CLOSE_CREDIT_ACCOUNT_FLAGS);
collateralDebtData.enabledTokensMask = fullCheckParams.enabledTokensMaskAfter;
_multicall(creditAccount, calls, collateralDebtData.enabledTokensMask, CLOSE_CREDIT_ACCOUNT_FLAGS); // U:[FA-16]
collateralDebtData.enabledTokensMask = fullCheckParams.enabledTokensMaskAfter; // U:[FA-16]
}

/// Bot permissions are specific to (owner, creditAccount),
/// so they need to be erased on account closure
_eraseAllBotPermissionsAtClosure({creditAccount: creditAccount});
_eraseAllBotPermissionsAtClosure({creditAccount: creditAccount}); // U:[FA-16]

(uint256 remainingFunds, uint256 reportedLoss) = _closeCreditAccount({
creditAccount: creditAccount,
Expand All @@ -437,28 +437,28 @@ contract CreditFacadeV3 is ICreditFacade, ACLNonReentrantTrait {
to: to,
skipTokensMask: skipTokenMask,
convertToETH: convertToETH
});
}); // U:[FA-16]

/// If there is non-zero loss, then borrowing is forbidden in
/// case this is an attack and there is risk of copycats afterwards
/// If cumulative loss exceeds maxCumulativeLoss, the CF is paused,
/// which ensures that the attacker can create at most maxCumulativeLoss + maxBorrowedAmount of bad debt
if (reportedLoss > 0) {
maxDebtPerBlockMultiplier = 0; // F: [FA-15A]
maxDebtPerBlockMultiplier = 0; // U:[FA-17]

/// reportedLoss is always less than uint128, because
/// maxLoss = maxBorrowAmount which is uint128
lossParams.currentCumulativeLoss += uint128(reportedLoss);
lossParams.currentCumulativeLoss += uint128(reportedLoss); // U:[FA-17]
if (lossParams.currentCumulativeLoss > lossParams.maxCumulativeLoss) {
_pause(); // F: [FA-15B]
_pause(); // U:[FA-17]
}
}

if (convertToETH) {
_wethWithdrawTo(to);
_wethWithdrawTo(to); // U:[FA-16]
}

emit LiquidateCreditAccount(creditAccount, borrower, msg.sender, to, closeAction, remainingFunds);
emit LiquidateCreditAccount(creditAccount, borrower, msg.sender, to, closeAction, remainingFunds); // U:[FA-14,16,17]
}

/// @notice Executes a batch of transactions within a Multicall, to manage an existing account
Expand Down
5 changes: 4 additions & 1 deletion contracts/interfaces/ICreditFacadeMulticall.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ uint256 constant DISABLE_TOKEN_PERMISSION = 2 ** 4;
uint256 constant WITHDRAW_PERMISSION = 2 ** 5;
uint256 constant UPDATE_QUOTA_PERMISSION = 2 ** 6;
uint256 constant REVOKE_ALLOWANCES_PERMISSION = 2 ** 7;
uint256 constant PAY_BOT_PERMISSION = 2 ** 8;

uint256 constant EXTERNAL_CALLS_PERMISSION = 2 ** 16;

uint256 constant ALL_CREDIT_FACADE_CALLS_PERMISSION = ADD_COLLATERAL_PERMISSION | INCREASE_DEBT_PERMISSION
Expand All @@ -27,6 +27,9 @@ uint256 constant ALL_PERMISSIONS = ALL_CREDIT_FACADE_CALLS_PERMISSION | EXTERNAL
uint256 constant INCREASE_DEBT_WAS_CALLED = 2 ** 193;
uint256 constant EXTERNAL_CONTRACT_WAS_CALLED = 2 ** 194;

// Pay bot permisson is set there because it's used for bots only
uint256 constant PAY_BOT_PERMISSION = 2 ** 195;

interface ICreditFacadeMulticall {
/// @dev Instructs CreditFacadeV3 to check token balances at the end
/// Used to control slippage after the entire sequence of operations, since tracking slippage
Expand Down
10 changes: 10 additions & 0 deletions contracts/test/mocks/credit/CreditManagerMock.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ contract CreditManagerMock {
/// @notice Maps 3rd party contracts to their respective adapters
mapping(address => address) public contractToAdapter;

uint256 return_remainingFunds;
uint256 return_loss;

constructor(address _addressProvider, address _pool) {
addressProvider = _addressProvider;
weth = IAddressProviderV3(addressProvider).getAddressOrRevert(AP_WETH_TOKEN, NO_VERSION_CONTROL); // U:[CM-1]
Expand Down Expand Up @@ -132,6 +135,11 @@ contract CreditManagerMock {
return nextCreditAccount;
}

function setCloseCreditAccountReturns(uint256 remainingFunds, uint256 loss) external {
return_remainingFunds = remainingFunds;
return_loss = loss;
}

function closeCreditAccount(
address creditAccount,
ClosureAction closureAction,
Expand All @@ -142,6 +150,8 @@ contract CreditManagerMock {
bool convertToETH
) external returns (uint256 remainingFunds, uint256 loss) {
_closeCollateralDebtData = collateralDebtData;
remainingFunds = return_remainingFunds;
loss = return_loss;
}

function fullCollateralCheck(
Expand Down
129 changes: 92 additions & 37 deletions contracts/test/unit/credit/CreditFacadeV3.unit.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -792,22 +792,14 @@ contract CreditFacadeV3UnitTest is TestHelper, BalanceHelper, ICreditFacadeEvent
}
}

// NEXT-]]
//
//
//
//

/// @dev U:[FA-24]: liquidateCreditAccount reverts if account has enough collateral
function test_U_FA_24_liquidateCreditAccount_reverts_if_account_has_enough_collateral(uint256 enabledTokensMask)
public
notExpirableCase
{
/// @dev U:[FA-16]: liquidate wokrs as expected
function test_U_FA_16_liquidate_wokrs_as_expected(uint256 enabledTokensMask) public notExpirableCase {
address creditAccount = DUMB_ADDRESS;

bool hasCalls = (getHash({value: enabledTokensMask, seed: 2}) % 2) == 0;
bool hasBotPermissions = (getHash({value: enabledTokensMask, seed: 3}) % 2) == 0;

uint256 cancelMask = 1 << 7;
uint256 LINK_TOKEN_MASK = 4;

address adapter = address(new AdapterMock(address(creditManagerMock), DUMB_ADDRESS));
Expand All @@ -819,26 +811,18 @@ contract CreditFacadeV3UnitTest is TestHelper, BalanceHelper, ICreditFacadeEvent
creditManagerMock.setBorrower(USER);
creditManagerMock.setFlagFor(creditAccount, BOT_PERMISSIONS_SET_FLAG, hasBotPermissions);

CollateralDebtData memory collateralDebtData = CollateralDebtData({
debt: getHash({value: enabledTokensMask, seed: 2}),
cumulativeIndexNow: getHash({value: enabledTokensMask, seed: 3}),
cumulativeIndexLastUpdate: getHash({value: enabledTokensMask, seed: 4}),
cumulativeQuotaInterest: getHash({value: enabledTokensMask, seed: 5}),
accruedInterest: getHash({value: enabledTokensMask, seed: 6}),
accruedFees: getHash({value: enabledTokensMask, seed: 7}),
totalDebtUSD: 0,
totalValue: 0,
totalValueUSD: 0,
twvUSD: 0,
enabledTokensMask: enabledTokensMask,
quotedTokensMask: 0,
quotedTokens: new address[](0),
quotedLts: new uint16[](0),
quotas: new uint256[](0),
_poolQuotaKeeper: address(0)
});
CollateralDebtData memory collateralDebtData;
collateralDebtData.totalDebtUSD = 101;
collateralDebtData.twvUSD = 100;

creditManagerMock.setDebtAndCollateralData(collateralDebtData);
creditManagerMock.setClaimWithdrawals(cancelMask);

vm.prank(CONFIGURATOR);
creditFacade.setEmergencyLiquidator(LIQUIDATOR, AllowanceAction.ALLOW);

CollateralDebtData memory expectedCollateralDebtData = clone(collateralDebtData);
expectedCollateralDebtData.enabledTokensMask = cancelMask;

if (hasCalls) {
calls = MulticallBuilder.build(
Expand All @@ -850,21 +834,22 @@ contract CreditFacadeV3UnitTest is TestHelper, BalanceHelper, ICreditFacadeEvent
creditManagerMock.setRevertOnActiveAccount(true);
}

creditManagerMock.setDebtAndCollateralData(collateralDebtData);

bool convertToETH = (getHash({value: enabledTokensMask, seed: 1}) % 2) == 1;

caseName =
string.concat(caseName, "convertToETH = ", boolToStr(convertToETH), ", hasCalls = ", boolToStr(hasCalls));

vm.expectCall(
address(creditManagerMock),
abi.encodeCall(ICreditManagerV3.calcDebtAndCollateral, (creditAccount, CollateralCalcTask.DEBT_ONLY))
abi.encodeCall(
ICreditManagerV3.calcDebtAndCollateral,
(creditAccount, CollateralCalcTask.DEBT_COLLATERAL_CANCEL_WITHDRAWALS)
)
);

vm.expectCall(
address(creditManagerMock),
abi.encodeCall(ICreditManagerV3.claimWithdrawals, (creditAccount, FRIEND, ClaimAction.FORCE_CLAIM))
abi.encodeCall(ICreditManagerV3.claimWithdrawals, (creditAccount, USER, ClaimAction.CANCEL))
);

uint256 skipTokenMask = getHash({value: enabledTokensMask, seed: 1});
Expand All @@ -875,9 +860,9 @@ contract CreditFacadeV3UnitTest is TestHelper, BalanceHelper, ICreditFacadeEvent
ICreditManagerV3.closeCreditAccount,
(
creditAccount,
ClosureAction.CLOSE_ACCOUNT,
ClosureAction.LIQUIDATE_ACCOUNT,
expectedCollateralDebtData,
USER,
LIQUIDATOR,
FRIEND,
skipTokenMask,
convertToETH
Expand All @@ -892,11 +877,12 @@ contract CreditFacadeV3UnitTest is TestHelper, BalanceHelper, ICreditFacadeEvent
} else {
botListMock.setRevertOnErase(true);
}
creditManagerMock.setCloseCreditAccountReturns(1_000, 0);

vm.expectEmit(true, true, true, true);
emit CloseCreditAccount(creditAccount, USER, FRIEND);
emit LiquidateCreditAccount(creditAccount, USER, LIQUIDATOR, FRIEND, ClosureAction.LIQUIDATE_ACCOUNT, 1_000);

vm.prank(USER);
vm.prank(LIQUIDATOR);
creditFacade.liquidateCreditAccount({
creditAccount: creditAccount,
to: FRIEND,
Expand All @@ -911,4 +897,73 @@ contract CreditFacadeV3UnitTest is TestHelper, BalanceHelper, ICreditFacadeEvent
_testCaseErr("Incorrect collateralDebtData")
);
}

/// @dev U:[FA-17]: liquidate correctly computes cumulative loss and pause contract if needed
function test_U_FA_17_liquidate_correctly_computes_cumulative_loss_and_pause_contract_if_needed(uint128 maxLoss)
public
notExpirableCase
{
vm.assume(maxLoss > 0 && maxLoss < type(uint120).max);

address creditAccount = DUMB_ADDRESS;

MultiCall[] memory calls;

vm.prank(CONFIGURATOR);
creditFacade.setCumulativeLossParams(maxLoss, true);

vm.prank(CONFIGURATOR);
creditFacade.setDebtLimits(1, 100, 10);

assertEq(creditFacade.maxDebtPerBlockMultiplier(), 10, "SETUP: incorrect maxDebtPerBlockMultiplier");

uint256 step = maxLoss / (getHash(maxLoss, 3) % 5 + 1) + 1;

uint256 expectedCumulativeLoss;

creditManagerMock.setBorrower(USER);

CollateralDebtData memory collateralDebtData;
collateralDebtData.totalDebtUSD = 101;
collateralDebtData.twvUSD = 100;

creditManagerMock.setDebtAndCollateralData(collateralDebtData);
creditManagerMock.setClaimWithdrawals(0);

creditManagerMock.setCloseCreditAccountReturns(1000, step);

do {
vm.expectCall(
address(creditManagerMock),
abi.encodeCall(
ICreditManagerV3.closeCreditAccount,
(creditAccount, ClosureAction.LIQUIDATE_ACCOUNT, collateralDebtData, LIQUIDATOR, FRIEND, 0, false)
)
);

vm.expectEmit(true, true, true, true);
emit LiquidateCreditAccount(creditAccount, USER, LIQUIDATOR, FRIEND, ClosureAction.LIQUIDATE_ACCOUNT, 1_000);

vm.prank(LIQUIDATOR);
creditFacade.liquidateCreditAccount({
creditAccount: creditAccount,
to: FRIEND,
skipTokenMask: 0,
convertToETH: false,
calls: calls
});

assertEq(creditFacade.maxDebtPerBlockMultiplier(), 0, "maxDebtPerBlockMultiplier wasnt set to zero");

(uint128 currentCumulativeLoss,) = creditFacade.lossParams();

expectedCumulativeLoss += step;

assertEq(currentCumulativeLoss, expectedCumulativeLoss, "Incorrect currentCumulativeLoss");

bool shoudBePaused = expectedCumulativeLoss > maxLoss;

assertEq(creditFacade.paused(), shoudBePaused, "Paused wasn't set");
} while (expectedCumulativeLoss < maxLoss);
}
}

0 comments on commit bd94bda

Please sign in to comment.