fix: add more explicit account existence checks #158
Conversation
| /// @dev Reverts if remaining token balances increase during the multicall | ||
| function liquidateCreditAccount(address creditAccount, address to, MultiCall[] calldata calls) | ||
| external | ||
| override | ||
| whenNotPausedOrEmergency // U:[FA-2,12] | ||
| nonReentrant // U:[FA-4] | ||
| { | ||
| address borrower = _getBorrowerOrRevert(creditAccount); // U:[FA-5] |
There was a problem hiding this comment.
borrower is removed from LiquidateCreditAccount event, and the check is now performed in calcDebtAndCollateral (additional safe-guard is require(debt != 0) below).
|
|
||
| (uint256 remainingFunds, uint256 reportedLoss) = ICreditManagerV3(creditManager).liquidateCreditAccount({ | ||
| creditAccount: creditAccount, | ||
| collateralDebtData: collateralDebtData, | ||
| to: to, | ||
| isExpired: isExpired | ||
| }); // U:[FA-16] | ||
| isExpired: !isUnhealthy |
There was a problem hiding this comment.
At this point, since it didn't revert earlier, the account is either unhealthy, expired, or both. Expired liquidation is only performed when it's not unhealthy (this is equivalent to what we had earlier).
| bool isUnhealthy = collateralDebtData.twvUSD < collateralDebtData.totalDebtUSD; | ||
| if (collateralDebtData.debt == 0 || !isUnhealthy && !_isExpired()) { | ||
| revert CreditAccountNotLiquidatableException(); // U:[FA-13] | ||
| } |
There was a problem hiding this comment.
Looks much cleaner this way. Besides, collateralDebtData.debt == 0 is now checked in all cases, not only in healthy-expired one.
| @@ -394,6 +388,8 @@ contract CreditFacadeV3 is ICreditFacadeV3, ACLNonReentrantTrait { | |||
| whenNotExpired // U:[FA-3] | |||
| nonReentrant // U:[FA-4] | |||
| { | |||
| _getBorrowerOrRevert(creditAccount); // U:[FA-5] | |||
There was a problem hiding this comment.
Absence of this could have been a problem with addition of special bots (for normal bots, permissions != 0 implies account existence).
| function isLiquidatable(address creditAccount, uint16 minHealthFactor) external view override returns (bool) { | ||
| getBorrowerOrRevert(creditAccount); // U:[CM-17] |
There was a problem hiding this comment.
Explicitly reverting is better than relying on some underflow later.
| if (task == CollateralCalcTask.DEBT_COLLATERAL_SAFE_PRICES) { | ||
| task = CollateralCalcTask.DEBT_COLLATERAL; | ||
| useSafePrices = true; | ||
| } | ||
|
|
||
| getBorrowerOrRevert(creditAccount); // U:[CM-17] |
There was a problem hiding this comment.
Explicitly reverting is better than relying on some underflow later.
|
🎉 This PR is included in version 1.45.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
No description provided.