LoopFi #3
Re: [WP-N14] Unused function _updatePosition()
The commit 55d82969a435ab599b47e9b874951e57e5dc77f6 "Merge pull request #5 from LoopFi/feat/quotas" unexpectedly reintroduced _updatePosition().
The main branch at b98c2fb9db96ea0959a5b978b56aab70133126c7 still contains the unused _updatePosition() function.
function _updatePosition(address position) internal view returns (Position memory updatedPos) {
Position memory pos = positions[position];
// pos.cumulativeIndexLastUpdate =
uint256 accruedInterest = CreditLogic.calcAccruedInterest(
pos.debt,
pos.cumulativeIndexLastUpdate,
pool.baseInterestIndex()
);
uint256 currentDebt = pos.debt + accruedInterest;
uint256 spotPrice_ = spotPrice();
uint256 collateralValue = wmul(pos.collateral, spotPrice_);
if (spotPrice_ == 0 || _isCollateralized(currentDebt, collateralValue, vaultConfig.liquidationRatio))
revert CDPVault__modifyCollateralAndDebt_notSafe();
return pos;
}
function _updatePosition(address position) internal view returns (Position memory updatedPos) {
Position memory pos = positions[position];
// pos.cumulativeIndexLastUpdate =
uint256 accruedInterest = CreditLogic.calcAccruedInterest(
pos.debt,
pos.cumulativeIndexLastUpdate,
pool.baseInterestIndex()
);
uint256 currentDebt = pos.debt + accruedInterest;
uint256 spotPrice_ = spotPrice();
uint256 collateralValue = wmul(pos.collateral, spotPrice_);
if (spotPrice_ == 0 || _isCollateralized(currentDebt, collateralValue, vaultConfig.liquidationRatio))
revert CDPVault__modifyCollateralAndDebt_notSafe();
return pos;
}
Re: [WP-C2] Attackers can still initiate Flashloans to unauthorized users with forged calldata and a fake Vault.
Flashlender should not allow others to call flashLoan on behalf of the user (receiver).
Otherwise, if the account has a balance for any reason, the attack vector we described in [WP-C2] is still possible with fake Vault in the flashLoan data.
function flashLoan(
IERC3156FlashBorrower receiver,
address token,
uint256 amount,
bytes calldata data
) external override nonReentrant returns (bool) {
if (token != address(underlyingToken)) revert Flash__flashLoan_unsupportedToken();
uint256 fee = wmul(amount, protocolFee);
uint256 total = amount + fee;
pool.lendCreditAccount(amount, address(receiver));
emit FlashLoan(address(receiver), token, amount, fee);
if (receiver.onFlashLoan(msg.sender, token, amount, fee, data) != CALLBACK_SUCCESS)
revert Flash__flashLoan_callbackFailed();
// reverts if not enough Stablecoin have been send back
underlyingToken.transferFrom(address(receiver), address(pool), total);
pool.repayCreditAccount(total - fee, fee, 0);
return true;
}
function onFlashLoan(
address /*initiator*/,
address /*token*/,
uint256 /*amount*/,
uint256 /*fee*/,
bytes calldata data
) external returns (bytes32) {
if (msg.sender != address(flashlender)) revert PositionAction__onFlashLoan__invalidSender();
(LeverParams memory leverParams, address upFrontToken, uint256 upFrontAmount) = abi.decode(
data,
(LeverParams, address, uint256)
);
@@ 391,408 @@
// perform a pre swap from arbitrary token to collateral token if necessary
if (leverParams.auxSwap.assetIn != address(0)) {
bytes memory auxSwapData = _delegateCall(
address(swapAction),
abi.encodeWithSelector(swapAction.swap.selector, leverParams.auxSwap)
);
upFrontAmount = abi.decode(auxSwapData, (uint256));
}
// swap stablecoin to collateral
bytes memory swapData = _delegateCall(
address(swapAction),
abi.encodeWithSelector(swapAction.swap.selector, leverParams.primarySwap)
);
uint256 swapAmountOut = abi.decode(swapData, (uint256));
// deposit collateral and handle any CDP specific actions
uint256 collateral = _onIncreaseLever(leverParams, upFrontToken, upFrontAmount, swapAmountOut);
// derive the amount of normal debt from the amount of Stablecoin swapped
uint256 addDebt = leverParams.primarySwap.amount;
// add collateral and debt
ICDPVault(leverParams.vault).modifyCollateralAndDebt(
leverParams.position,
address(this),
address(this),
toInt256(collateral),
toInt256(addDebt)
);
return CALLBACK_SUCCESS;
}
Recommendation
There are two ways to prevent initiating flashloans from addresses other than the user's own address:
- In
PositionAction.sol#onFlashLoan, strictly check theinitiatorandamountaccording to the parameters provided byflashLoan:
function onFlashLoan(
address initiator,
address /*token*/,
uint256 amount,
uint256 fee,
bytes calldata data
) external returns (bytes32) {
if (msg.sender != address(flashlender)) revert PositionAction__onFlashLoan__invalidSender();
if (initiator != address(this)) revert();
(LeverParams memory leverParams, address upFrontToken, uint256 upFrontAmount) = abi.decode(
data,
(LeverParams, address, uint256)
);
@@ 391,408 @@
// perform a pre swap from arbitrary token to collateral token if necessary
if (leverParams.auxSwap.assetIn != address(0)) {
bytes memory auxSwapData = _delegateCall(
address(swapAction),
abi.encodeWithSelector(swapAction.swap.selector, leverParams.auxSwap)
);
upFrontAmount = abi.decode(auxSwapData, (uint256));
}
// swap stablecoin to collateral
bytes memory swapData = _delegateCall(
address(swapAction),
abi.encodeWithSelector(swapAction.swap.selector, leverParams.primarySwap)
);
uint256 swapAmountOut = abi.decode(swapData, (uint256));
// deposit collateral and handle any CDP specific actions
uint256 collateral = _onIncreaseLever(leverParams, upFrontToken, upFrontAmount, swapAmountOut);
// derive the amount of normal debt from the amount of Stablecoin swapped
// uint256 addDebt = leverParams.primarySwap.amount;
// add collateral and debt
ICDPVault(leverParams.vault).modifyCollateralAndDebt(
leverParams.position,
address(this),
address(this),
toInt256(collateral),
amount + fee
);
return CALLBACK_SUCCESS;
}
- Only allow
receiver == msg.senderinFlashlender.sol#flashLoan():
function flashLoan(
IERC3156FlashBorrower receiver,
address token,
uint256 amount,
bytes calldata data
) external override nonReentrant returns (bool) {
if (token != address(underlyingToken)) revert Flash__flashLoan_unsupportedToken();
if(msg.sender != address(receiver)) revert();
uint256 fee = wmul(amount, protocolFee);
uint256 total = amount + fee;
pool.lendCreditAccount(amount, address(receiver));
emit FlashLoan(address(receiver), token, amount, fee);
if (receiver.onFlashLoan(msg.sender, token, amount, fee, data) != CALLBACK_SUCCESS)
revert Flash__flashLoan_callbackFailed();
// reverts if not enough Stablecoin have been send back
underlyingToken.transferFrom(address(receiver), address(pool), total);
pool.repayCreditAccount(total - fee, fee, 0);
return true;
}
The fix can be either of the two.
function flashLoan(
IERC3156FlashBorrower receiver,
address token,
uint256 amount,
bytes calldata data
) external override nonReentrant returns (bool) {
if (token != address(underlyingToken)) revert Flash__flashLoan_unsupportedToken();
uint256 fee = wmul(amount, protocolFee);
uint256 total = amount + fee;
pool.lendCreditAccount(amount, address(receiver));
emit FlashLoan(address(receiver), token, amount, fee);
if (receiver.onFlashLoan(msg.sender, token, amount, fee, data) != CALLBACK_SUCCESS)
revert Flash__flashLoan_callbackFailed();
// reverts if not enough Stablecoin have been send back
underlyingToken.transferFrom(address(receiver), address(pool), total);
pool.repayCreditAccount(total - fee, fee, 0);
return true;
}
function onFlashLoan(
address /*initiator*/,
address /*token*/,
uint256 /*amount*/,
uint256 /*fee*/,
bytes calldata data
) external returns (bytes32) {
if (msg.sender != address(flashlender)) revert PositionAction__onFlashLoan__invalidSender();
(LeverParams memory leverParams, address upFrontToken, uint256 upFrontAmount) = abi.decode(
data,
(LeverParams, address, uint256)
);
@@ 391,408 @@
// perform a pre swap from arbitrary token to collateral token if necessary
if (leverParams.auxSwap.assetIn != address(0)) {
bytes memory auxSwapData = _delegateCall(
address(swapAction),
abi.encodeWithSelector(swapAction.swap.selector, leverParams.auxSwap)
);
upFrontAmount = abi.decode(auxSwapData, (uint256));
}
// swap stablecoin to collateral
bytes memory swapData = _delegateCall(
address(swapAction),
abi.encodeWithSelector(swapAction.swap.selector, leverParams.primarySwap)
);
uint256 swapAmountOut = abi.decode(swapData, (uint256));
// deposit collateral and handle any CDP specific actions
uint256 collateral = _onIncreaseLever(leverParams, upFrontToken, upFrontAmount, swapAmountOut);
// derive the amount of normal debt from the amount of Stablecoin swapped
uint256 addDebt = leverParams.primarySwap.amount;
// add collateral and debt
ICDPVault(leverParams.vault).modifyCollateralAndDebt(
leverParams.position,
address(this),
address(this),
toInt256(collateral),
toInt256(addDebt)
);
return CALLBACK_SUCCESS;
}
Re: [WP-H3] Health Factor Calculation Should Take Interest and Fees Into Account
newDebt returned from calcDecrease() CDPVault.sol#L414 doesn't include unpaid interest (if any). See also [WP-M6].
newDebt at CDPVault.sol#L428 also doesn't include any unpaid interest.
newDebt at CDPVault.sol#L388 is the same.
Therefore, CDPVault.sol#L448 _isCollateralized() didn't take the interest into account.
function modifyCollateralAndDebt(
address owner,
address collateralizer,
address creditor,
int256 deltaCollateral,
int256 deltaDebt
) public {
if (
// position is either more safe than before or msg.sender has the permission from the owner
((deltaDebt > 0 || deltaCollateral < 0) && !hasPermission(owner, msg.sender)) ||
// msg.sender has the permission of the collateralizer to collateralize the position using their cash
(deltaCollateral > 0 && !hasPermission(collateralizer, msg.sender)) ||
// msg.sender has the permission of the creditor to use their credit to repay the debt
(deltaDebt < 0 && !hasPermission(creditor, msg.sender))
) revert CDPVault__modifyCollateralAndDebt_noPermission();
Position memory position = positions[owner];
DebtData memory debtData = _calcDebt(position);
uint256 newDebt;
uint256 newCumulativeIndex;
uint256 profit;
int256 quotaRevenueChange;
if (deltaDebt > 0) {
(newDebt, newCumulativeIndex) = CreditLogic.calcIncrease(
uint256(deltaDebt), // delta debt
position.debt,
debtData.cumulativeIndexNow, // current cumulative base interest index in Ray
position.cumulativeIndexLastUpdate
); // U:[CM-10]
position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow;
quotaRevenueChange = _calcQuotaRevenueChange(deltaDebt);
pool.lendCreditAccount(uint256(deltaDebt), creditor); // F:[CM-20]
} else if (deltaDebt < 0) {
uint256 maxRepayment = calcTotalDebt(debtData);
uint256 amount = abs(deltaDebt);
if (amount >= maxRepayment) {
amount = maxRepayment; // U:[CM-11]
deltaDebt = -toInt256(maxRepayment);
}
poolUnderlying.safeTransferFrom(creditor, address(pool), amount);
uint128 newCumulativeQuotaInterest;
if (amount == maxRepayment) {
newDebt = 0;
newCumulativeIndex = debtData.cumulativeIndexNow;
profit = debtData.accruedInterest;
newCumulativeQuotaInterest = 0;
} else {
(newDebt, newCumulativeIndex, profit, newCumulativeQuotaInterest) = calcDecrease(
amount, // delta debt
position.debt,
debtData.cumulativeIndexNow, // current cumulative base interest index in Ray
position.cumulativeIndexLastUpdate,
debtData.cumulativeQuotaInterest
);
}
quotaRevenueChange = _calcQuotaRevenueChange(-int(debtData.debt - newDebt));
pool.repayCreditAccount(debtData.debt - newDebt, profit, 0); // U:[CM-11]
position.cumulativeQuotaInterest = newCumulativeQuotaInterest;
position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow;
} else {
newDebt = position.debt;
newCumulativeIndex = debtData.cumulativeIndexNow;
}
if (deltaCollateral > 0) {
uint256 amount = wmul(deltaCollateral.toUint256(), tokenScale);
token.safeTransferFrom(collateralizer, address(this), amount);
} else if (deltaCollateral < 0) {
uint256 amount = wmul(abs(deltaCollateral), tokenScale);
token.safeTransfer(collateralizer, amount);
}
position = _modifyPosition(owner, position, newDebt, newCumulativeIndex, deltaCollateral, totalDebt);
VaultConfig memory config = vaultConfig;
uint256 spotPrice_ = spotPrice();
uint256 collateralValue = wmul(position.collateral, spotPrice_);
if (
(deltaDebt > 0 || deltaCollateral < 0) &&
!_isCollateralized(newDebt, collateralValue, config.liquidationRatio)
) revert CDPVault__modifyCollateralAndDebt_notSafe();
if (quotaRevenueChange != 0) {
IPoolV3(pool).updateQuotaRevenue(quotaRevenueChange); // U:[PQK-15]
}
emit ModifyCollateralAndDebt(owner, collateralizer, creditor, deltaCollateral, deltaDebt);
}
function modifyCollateralAndDebt(
address owner,
address collateralizer,
address creditor,
int256 deltaCollateral,
int256 deltaDebt
) public {
if (
// position is either more safe than before or msg.sender has the permission from the owner
((deltaDebt > 0 || deltaCollateral < 0) && !hasPermission(owner, msg.sender)) ||
// msg.sender has the permission of the collateralizer to collateralize the position using their cash
(deltaCollateral > 0 && !hasPermission(collateralizer, msg.sender)) ||
// msg.sender has the permission of the creditor to use their credit to repay the debt
(deltaDebt < 0 && !hasPermission(creditor, msg.sender))
) revert CDPVault__modifyCollateralAndDebt_noPermission();
Position memory position = positions[owner];
DebtData memory debtData = _calcDebt(position);
uint256 newDebt;
uint256 newCumulativeIndex;
uint256 profit;
int256 quotaRevenueChange;
if (deltaDebt > 0) {
(newDebt, newCumulativeIndex) = CreditLogic.calcIncrease(
uint256(deltaDebt), // delta debt
position.debt,
debtData.cumulativeIndexNow, // current cumulative base interest index in Ray
position.cumulativeIndexLastUpdate
); // U:[CM-10]
position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow;
quotaRevenueChange = _calcQuotaRevenueChange(deltaDebt);
pool.lendCreditAccount(uint256(deltaDebt), creditor); // F:[CM-20]
} else if (deltaDebt < 0) {
uint256 maxRepayment = calcTotalDebt(debtData);
uint256 amount = abs(deltaDebt);
if (amount >= maxRepayment) {
amount = maxRepayment; // U:[CM-11]
deltaDebt = -toInt256(maxRepayment);
}
poolUnderlying.safeTransferFrom(creditor, address(pool), amount);
uint128 newCumulativeQuotaInterest;
if (amount == maxRepayment) {
newDebt = 0;
newCumulativeIndex = debtData.cumulativeIndexNow;
profit = debtData.accruedInterest;
newCumulativeQuotaInterest = 0;
} else {
(newDebt, newCumulativeIndex, profit, newCumulativeQuotaInterest) = calcDecrease(
amount, // delta debt
position.debt,
debtData.cumulativeIndexNow, // current cumulative base interest index in Ray
position.cumulativeIndexLastUpdate,
debtData.cumulativeQuotaInterest
);
}
quotaRevenueChange = _calcQuotaRevenueChange(-int(debtData.debt - newDebt));
pool.repayCreditAccount(debtData.debt - newDebt, profit, 0); // U:[CM-11]
position.cumulativeQuotaInterest = newCumulativeQuotaInterest;
position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow;
} else {
newDebt = position.debt;
newCumulativeIndex = debtData.cumulativeIndexNow;
}
if (deltaCollateral > 0) {
uint256 amount = wmul(deltaCollateral.toUint256(), tokenScale);
token.safeTransferFrom(collateralizer, address(this), amount);
} else if (deltaCollateral < 0) {
uint256 amount = wmul(abs(deltaCollateral), tokenScale);
token.safeTransfer(collateralizer, amount);
}
position = _modifyPosition(owner, position, newDebt, newCumulativeIndex, deltaCollateral, totalDebt);
VaultConfig memory config = vaultConfig;
uint256 spotPrice_ = spotPrice();
uint256 collateralValue = wmul(position.collateral, spotPrice_);
if (
(deltaDebt > 0 || deltaCollateral < 0) &&
!_isCollateralized(newDebt, collateralValue, config.liquidationRatio)
) revert CDPVault__modifyCollateralAndDebt_notSafe();
if (quotaRevenueChange != 0) {
IPoolV3(pool).updateQuotaRevenue(quotaRevenueChange); // U:[PQK-15]
}
emit ModifyCollateralAndDebt(owner, collateralizer, creditor, deltaCollateral, deltaDebt);
}
[WP-M4] The whenNotLocked modifier causes the function body of any function it's applied to to execute twice
The _ appeared twice in both branches of whenNotLocked() modifier.
modifier whenNotLocked() {
if (_allowed[msg.sender]) {
_;
} else {
_revertIfLocked();
_;
}
_;
}
/// @notice Redeems given number of pool shares for underlying tokens
/// @param shares Number of pool shares to redeem
/// @param receiver Account to send underlying to
/// @param owner Account to burn pool shares from
/// @return assets Amount of underlying withdrawn
function redeem(
uint256 shares,
address receiver,
address owner
)
public
override(ERC4626, IERC4626)
whenNotPaused // U:[LP-2A]
whenNotLocked
nonReentrant // U:[LP-2B]
nonZeroAddress(receiver) // U:[LP-5]
returns (uint256 assets)
{
uint256 assetsSent = _convertToAssets(shares); // U:[LP-9]
uint256 assetsToUser = _amountMinusWithdrawalFee(assetsSent);
assets = _amountMinusFee(assetsToUser); // U:[LP-9]
_withdraw(receiver, owner, assetsSent, assets, assetsToUser, shares); // U:[LP-9]
}
/// @notice Withdraws pool shares for given amount of underlying tokens
/// @param assets Amount of underlying to withdraw
/// @param receiver Account to send underlying to
/// @param owner Account to burn pool shares from
/// @return shares Number of pool shares burned
function withdraw(
uint256 assets,
address receiver,
address owner
)
public
override(ERC4626, IERC4626)
whenNotPaused // U:[LP-2A]
whenNotLocked
nonReentrant // U:[LP-2B]
nonZeroAddress(receiver) // U:[LP-5]
returns (uint256 shares)
{
uint256 assetsToUser = _amountWithFee(assets);
uint256 assetsSent = _amountWithWithdrawalFee(assetsToUser); // U:[LP-8]
shares = _convertToShares(assetsSent); // U:[LP-8]
_withdraw(receiver, owner, assetsSent, assets, assetsToUser, shares); // U:[LP-8]
}
In the redeem function, when a user wants to withdraw shares from the contract using
redeem, after passing authentication, the actual withdrawal will be shares.
This leads to:
-
When
, users can withdraw normally, receiving
shares -
When
, users cannot withdraw normally, and the transaction will
revert
The same logic applies to the withdraw function.
Recommendation
Remove the extra _ at line 127.
modifier whenNotLocked() {
if (_allowed[msg.sender]) {
_;
} else {
_revertIfLocked();
_;
}
_;
}
/// @notice Redeems given number of pool shares for underlying tokens
/// @param shares Number of pool shares to redeem
/// @param receiver Account to send underlying to
/// @param owner Account to burn pool shares from
/// @return assets Amount of underlying withdrawn
function redeem(
uint256 shares,
address receiver,
address owner
)
public
override(ERC4626, IERC4626)
whenNotPaused // U:[LP-2A]
whenNotLocked
nonReentrant // U:[LP-2B]
nonZeroAddress(receiver) // U:[LP-5]
returns (uint256 assets)
{
uint256 assetsSent = _convertToAssets(shares); // U:[LP-9]
uint256 assetsToUser = _amountMinusWithdrawalFee(assetsSent);
assets = _amountMinusFee(assetsToUser); // U:[LP-9]
_withdraw(receiver, owner, assetsSent, assets, assetsToUser, shares); // U:[LP-9]
}
/// @notice Withdraws pool shares for given amount of underlying tokens
/// @param assets Amount of underlying to withdraw
/// @param receiver Account to send underlying to
/// @param owner Account to burn pool shares from
/// @return shares Number of pool shares burned
function withdraw(
uint256 assets,
address receiver,
address owner
)
public
override(ERC4626, IERC4626)
whenNotPaused // U:[LP-2A]
whenNotLocked
nonReentrant // U:[LP-2B]
nonZeroAddress(receiver) // U:[LP-5]
returns (uint256 shares)
{
uint256 assetsToUser = _amountWithFee(assets);
uint256 assetsSent = _amountWithWithdrawalFee(assetsToUser); // U:[LP-8]
shares = _convertToShares(assetsSent); // U:[LP-8]
_withdraw(receiver, owner, assetsSent, assets, assetsToUser, shares); // U:[LP-8]
}
[WP-I5] modifyCollateralAndDebt(..., deltaDebt) might forget to convert the amount passed to pool.lendCreditAccount(uint256(deltaDebt), ...) to the correct scale
In the current implementation, deltaDebt is not converted to the appropriate unit.
This implicitly requires the pool's underlyingToken to have 18 decimals, otherwise the system will not function correctly.
deltaDebt is in [wad] scale as mentioned in CDPVault.sol#L362, since the deltaDebt used in CDPVault.sol#L396 is not converted to the scale expected by
PoolV3.lendCreditAccount(), it will transfer the wrong token amount in PoolV3.sol#L509 when underlyingToken.decimals() != 18.
@@ 349,360 @@
/// @notice Modifies a Position's collateral and debt balances
/// @dev Checks that the global debt ceiling and the vault's debt ceiling have not been exceeded via the CDM,
/// - that the Position is still safe after the modification,
/// - that the msg.sender has the permission of the owner to decrease the collateral-to-debt ratio,
/// - that the msg.sender has the permission of the collateralizer to put up new collateral,
/// - that the msg.sender has the permission of the creditor to settle debt with their credit,
/// - that that the vault debt floor is exceeded
/// - that the vault minimum collateralization ratio is met
/// @param owner Address of the owner of the position
/// @param collateralizer Address of who puts up or receives the collateral delta
/// @param creditor Address of who provides or receives the credit delta for the debt delta
/// @param deltaCollateral Amount of collateral to put up (+) or to remove (-) from the position [wad]
/// @param deltaDebt Amount of normalized debt (gross, before rate is applied) to generate (+) or
/// to settle (-) on this position [wad]
function modifyCollateralAndDebt(
address owner,
address collateralizer,
address creditor,
int256 deltaCollateral,
int256 deltaDebt
) public {
@@ 370,386 @@
if (
// position is either more safe than before or msg.sender has the permission from the owner
((deltaDebt > 0 || deltaCollateral < 0) && !hasPermission(owner, msg.sender)) ||
// msg.sender has the permission of the collateralizer to collateralize the position using their cash
(deltaCollateral > 0 && !hasPermission(collateralizer, msg.sender)) ||
// msg.sender has the permission of the creditor to use their credit to repay the debt
(deltaDebt < 0 && !hasPermission(creditor, msg.sender))
) revert CDPVault__modifyCollateralAndDebt_noPermission();
Position memory position = positions[owner];
DebtData memory debtData = _calcDebt(position);
uint256 newDebt;
uint256 newCumulativeIndex;
uint256 profit;
int256 quotaRevenueChange;
if (deltaDebt > 0) {
(newDebt, newCumulativeIndex) = CreditLogic.calcIncrease(
uint256(deltaDebt), // delta debt
position.debt,
debtData.cumulativeIndexNow, // current cumulative base interest index in Ray
position.cumulativeIndexLastUpdate
); // U:[CM-10]
position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow;
quotaRevenueChange = _calcQuotaRevenueChange(deltaDebt);
pool.lendCreditAccount(uint256(deltaDebt), creditor); // F:[CM-20]
} else if (deltaDebt < 0) {
@@ 398,426 @@
uint256 maxRepayment = calcTotalDebt(debtData);
uint256 amount = abs(deltaDebt);
if (amount >= maxRepayment) {
amount = maxRepayment; // U:[CM-11]
deltaDebt = -toInt256(maxRepayment);
}
poolUnderlying.safeTransferFrom(creditor, address(pool), amount);
uint128 newCumulativeQuotaInterest;
if (amount == maxRepayment) {
newDebt = 0;
newCumulativeIndex = debtData.cumulativeIndexNow;
profit = debtData.accruedInterest;
newCumulativeQuotaInterest = 0;
} else {
(newDebt, newCumulativeIndex, profit, newCumulativeQuotaInterest) = calcDecrease(
amount, // delta debt
position.debt,
debtData.cumulativeIndexNow, // current cumulative base interest index in Ray
position.cumulativeIndexLastUpdate,
debtData.cumulativeQuotaInterest
);
}
quotaRevenueChange = _calcQuotaRevenueChange(-int(debtData.debt - newDebt));
pool.repayCreditAccount(debtData.debt - newDebt, profit, 0); // U:[CM-11]
position.cumulativeQuotaInterest = newCumulativeQuotaInterest;
position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow;
} else {
@@ 428,429 @@
newDebt = position.debt;
newCumulativeIndex = debtData.cumulativeIndexNow;
}
@@ 432,454 @@
if (deltaCollateral > 0) {
uint256 amount = wmul(deltaCollateral.toUint256(), tokenScale);
token.safeTransferFrom(collateralizer, address(this), amount);
} else if (deltaCollateral < 0) {
uint256 amount = wmul(abs(deltaCollateral), tokenScale);
token.safeTransfer(collateralizer, amount);
}
position = _modifyPosition(owner, position, newDebt, newCumulativeIndex, deltaCollateral, totalDebt);
VaultConfig memory config = vaultConfig;
uint256 spotPrice_ = spotPrice();
uint256 collateralValue = wmul(position.collateral, spotPrice_);
if (
(deltaDebt > 0 || deltaCollateral < 0) &&
!_isCollateralized(newDebt, collateralValue, config.liquidationRatio)
) revert CDPVault__modifyCollateralAndDebt_notSafe();
if (quotaRevenueChange != 0) {
IPoolV3(pool).updateQuotaRevenue(quotaRevenueChange); // U:[PQK-15]
}
emit ModifyCollateralAndDebt(owner, collateralizer, creditor, deltaCollateral, deltaDebt);
}
/// @notice Lends funds to a credit account, can only be called by credit managers
/// @param borrowedAmount Amount to borrow
/// @param creditAccount Credit account to send the funds to
function lendCreditAccount(
uint256 borrowedAmount,
address creditAccount
)
external
override
creditManagerOnly // U:[LP-2C]
whenNotPaused // U:[LP-2A]
nonReentrant // U:[LP-2B]
{
@@ 491,507 @@
uint128 borrowedAmountU128 = borrowedAmount.toUint128();
DebtParams storage cmDebt = _creditManagerDebt[msg.sender];
uint128 totalBorrowed_ = _totalDebt.borrowed + borrowedAmountU128;
uint128 cmBorrowed_ = cmDebt.borrowed + borrowedAmountU128;
if (borrowedAmount == 0 || cmBorrowed_ > cmDebt.limit || totalBorrowed_ > _totalDebt.limit) {
revert CreditManagerCantBorrowException(); // U:[LP-2C,13A]
}
_updateBaseInterest({
expectedLiquidityDelta: 0,
availableLiquidityDelta: -borrowedAmount.toInt256(),
checkOptimalBorrowing: true
}); // U:[LP-13B]
cmDebt.borrowed = cmBorrowed_; // U:[LP-13B]
_totalDebt.borrowed = totalBorrowed_; // U:[LP-13B]
IERC20(underlyingToken).safeTransfer({to: creditAccount, value: borrowedAmount}); // U:[LP-13B]
emit Borrow(msg.sender, creditAccount, borrowedAmount); // U:[LP-13B]
}
Recommendation
Consider either:
- Add conversion for
deltaDebtas well; - Assert
underlyingToken's decimals to be18when settingunderlyingTokeninPoolV3.
@@ 349,360 @@
/// @notice Modifies a Position's collateral and debt balances
/// @dev Checks that the global debt ceiling and the vault's debt ceiling have not been exceeded via the CDM,
/// - that the Position is still safe after the modification,
/// - that the msg.sender has the permission of the owner to decrease the collateral-to-debt ratio,
/// - that the msg.sender has the permission of the collateralizer to put up new collateral,
/// - that the msg.sender has the permission of the creditor to settle debt with their credit,
/// - that that the vault debt floor is exceeded
/// - that the vault minimum collateralization ratio is met
/// @param owner Address of the owner of the position
/// @param collateralizer Address of who puts up or receives the collateral delta
/// @param creditor Address of who provides or receives the credit delta for the debt delta
/// @param deltaCollateral Amount of collateral to put up (+) or to remove (-) from the position [wad]
/// @param deltaDebt Amount of normalized debt (gross, before rate is applied) to generate (+) or
/// to settle (-) on this position [wad]
function modifyCollateralAndDebt(
address owner,
address collateralizer,
address creditor,
int256 deltaCollateral,
int256 deltaDebt
) public {
@@ 370,386 @@
if (
// position is either more safe than before or msg.sender has the permission from the owner
((deltaDebt > 0 || deltaCollateral < 0) && !hasPermission(owner, msg.sender)) ||
// msg.sender has the permission of the collateralizer to collateralize the position using their cash
(deltaCollateral > 0 && !hasPermission(collateralizer, msg.sender)) ||
// msg.sender has the permission of the creditor to use their credit to repay the debt
(deltaDebt < 0 && !hasPermission(creditor, msg.sender))
) revert CDPVault__modifyCollateralAndDebt_noPermission();
Position memory position = positions[owner];
DebtData memory debtData = _calcDebt(position);
uint256 newDebt;
uint256 newCumulativeIndex;
uint256 profit;
int256 quotaRevenueChange;
if (deltaDebt > 0) {
(newDebt, newCumulativeIndex) = CreditLogic.calcIncrease(
uint256(deltaDebt), // delta debt
position.debt,
debtData.cumulativeIndexNow, // current cumulative base interest index in Ray
position.cumulativeIndexLastUpdate
); // U:[CM-10]
position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow;
quotaRevenueChange = _calcQuotaRevenueChange(deltaDebt);
pool.lendCreditAccount(uint256(deltaDebt), creditor); // F:[CM-20]
} else if (deltaDebt < 0) {
@@ 398,426 @@
uint256 maxRepayment = calcTotalDebt(debtData);
uint256 amount = abs(deltaDebt);
if (amount >= maxRepayment) {
amount = maxRepayment; // U:[CM-11]
deltaDebt = -toInt256(maxRepayment);
}
poolUnderlying.safeTransferFrom(creditor, address(pool), amount);
uint128 newCumulativeQuotaInterest;
if (amount == maxRepayment) {
newDebt = 0;
newCumulativeIndex = debtData.cumulativeIndexNow;
profit = debtData.accruedInterest;
newCumulativeQuotaInterest = 0;
} else {
(newDebt, newCumulativeIndex, profit, newCumulativeQuotaInterest) = calcDecrease(
amount, // delta debt
position.debt,
debtData.cumulativeIndexNow, // current cumulative base interest index in Ray
position.cumulativeIndexLastUpdate,
debtData.cumulativeQuotaInterest
);
}
quotaRevenueChange = _calcQuotaRevenueChange(-int(debtData.debt - newDebt));
pool.repayCreditAccount(debtData.debt - newDebt, profit, 0); // U:[CM-11]
position.cumulativeQuotaInterest = newCumulativeQuotaInterest;
position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow;
} else {
@@ 428,429 @@
newDebt = position.debt;
newCumulativeIndex = debtData.cumulativeIndexNow;
}
@@ 432,454 @@
if (deltaCollateral > 0) {
uint256 amount = wmul(deltaCollateral.toUint256(), tokenScale);
token.safeTransferFrom(collateralizer, address(this), amount);
} else if (deltaCollateral < 0) {
uint256 amount = wmul(abs(deltaCollateral), tokenScale);
token.safeTransfer(collateralizer, amount);
}
position = _modifyPosition(owner, position, newDebt, newCumulativeIndex, deltaCollateral, totalDebt);
VaultConfig memory config = vaultConfig;
uint256 spotPrice_ = spotPrice();
uint256 collateralValue = wmul(position.collateral, spotPrice_);
if (
(deltaDebt > 0 || deltaCollateral < 0) &&
!_isCollateralized(newDebt, collateralValue, config.liquidationRatio)
) revert CDPVault__modifyCollateralAndDebt_notSafe();
if (quotaRevenueChange != 0) {
IPoolV3(pool).updateQuotaRevenue(quotaRevenueChange); // U:[PQK-15]
}
emit ModifyCollateralAndDebt(owner, collateralizer, creditor, deltaCollateral, deltaDebt);
}
/// @notice Lends funds to a credit account, can only be called by credit managers
/// @param borrowedAmount Amount to borrow
/// @param creditAccount Credit account to send the funds to
function lendCreditAccount(
uint256 borrowedAmount,
address creditAccount
)
external
override
creditManagerOnly // U:[LP-2C]
whenNotPaused // U:[LP-2A]
nonReentrant // U:[LP-2B]
{
@@ 491,507 @@
uint128 borrowedAmountU128 = borrowedAmount.toUint128();
DebtParams storage cmDebt = _creditManagerDebt[msg.sender];
uint128 totalBorrowed_ = _totalDebt.borrowed + borrowedAmountU128;
uint128 cmBorrowed_ = cmDebt.borrowed + borrowedAmountU128;
if (borrowedAmount == 0 || cmBorrowed_ > cmDebt.limit || totalBorrowed_ > _totalDebt.limit) {
revert CreditManagerCantBorrowException(); // U:[LP-2C,13A]
}
_updateBaseInterest({
expectedLiquidityDelta: 0,
availableLiquidityDelta: -borrowedAmount.toInt256(),
checkOptimalBorrowing: true
}); // U:[LP-13B]
cmDebt.borrowed = cmBorrowed_; // U:[LP-13B]
_totalDebt.borrowed = totalBorrowed_; // U:[LP-13B]
IERC20(underlyingToken).safeTransfer({to: creditAccount, value: borrowedAmount}); // U:[LP-13B]
emit Borrow(msg.sender, creditAccount, borrowedAmount); // U:[LP-13B]
}
[WP-M6] Incorrect Implementation of Interest Repayment in calcDecrease(): Wrong Usage of interestAccrued and Failure to Recalculate newIndex
Missing corresponding code for CDPVault.sol#L611
/// @dev Computes new debt principal and interest index (and other values) after decreasing debt
/// - Debt comprises of multiple components which are repaid in the following order:
/// quota update fees => quota interest => base interest => debt principal.
/// New values for all these components depend on what portion of each was repaid.
/// - Debt principal, for example, only decreases if all previous components were fully repaid
/// - The new credit account's interest index stays the same if base interest was not repaid at all,
/// is set to the current interest index if base interest was repaid fully, and is a solution to
/// the equation `debt * (indexNow / indexLastUpdate - 1) - delta = debt * (indexNow / indexNew - 1)`
/// when only `delta` of accrued interest was repaid
@@ 613,621 @@
/// @param amount Amount of debt to repay
/// @param debt Debt principal before repayment
/// @param cumulativeIndexNow The current interest index
/// @param cumulativeIndexLastUpdate Credit account's interest index as of last update
/// @return newDebt Debt principal after repayment
/// @return newCumulativeIndex Credit account's quota interest after repayment
/// @return profit Amount of underlying tokens received as fees by the DAO
/// @return newCumulativeQuotaInterest Credit account's accrued quota interest after repayment
// @return newQuotaFees Amount of unpaid quota fees left after repayment
function calcDecrease(
uint256 amount,
uint256 debt,
uint256 cumulativeIndexNow,
uint256 cumulativeIndexLastUpdate,
uint128 cumulativeQuotaInterest
)
internal
view
returns (uint256 newDebt, uint256 newCumulativeIndex, uint256 profit, uint128 newCumulativeQuotaInterest)
{
uint256 amountToRepay = amount;
if (cumulativeQuotaInterest != 0 && amountToRepay != 0) {
// All interest accrued on the quota interest is taken by the DAO to be distributed to LP stakers, dLP stakers and the DAO
if (amountToRepay >= cumulativeQuotaInterest) {
amountToRepay -= cumulativeQuotaInterest; // U:[CL-3]
profit += cumulativeQuotaInterest; // U:[CL-3]
newCumulativeQuotaInterest = 0; // U:[CL-3]
} else {
// If amount is not enough to repay quota interest + DAO fee, then send all to the stakers
uint256 quotaInterestPaid = amountToRepay; // U:[CL-3]
profit += amountToRepay; // U:[CL-3]
amountToRepay = 0; // U:[CL-3]
newCumulativeQuotaInterest = uint128(cumulativeQuotaInterest - quotaInterestPaid); // U:[CL-3]
}
} else {
newCumulativeQuotaInterest = cumulativeQuotaInterest;
}
if (amountToRepay != 0) {
uint256 interestAccrued = CreditLogic.calcAccruedInterest({
amount: debt,
cumulativeIndexLastUpdate: cumulativeIndexLastUpdate,
cumulativeIndexNow: cumulativeIndexNow
}); // U:[CL-3]
// All interest accrued on the base interest is taken by the DAO to be distributed to LP stakers, dLP stakers and the DAO
if (amountToRepay >= interestAccrued) {
amountToRepay -= interestAccrued;
profit += interestAccrued; // U:[CL-3]
newCumulativeIndex = cumulativeIndexNow; // U:[CL-3]
} else {
// If amount is not enough to repay quota interest + DAO fee, then send all to the stakers
profit += interestAccrued; // U:[CL-3]
amountToRepay = 0; // U:[CL-3]
newCumulativeIndex = cumulativeIndexNow;
}
} else {
newCumulativeIndex = cumulativeIndexLastUpdate;
}
newDebt = debt - amountToRepay;
}
/// @dev Computes new debt principal and interest index (and other values) after decreasing debt
/// - Debt comprises of multiple components which are repaid in the following order:
/// quota update fees => quota interest => base interest => debt principal.
/// New values for all these components depend on what portion of each was repaid.
/// - Debt principal, for example, only decreases if all previous components were fully repaid
/// - The new credit account's interest index stays the same if base interest was not repaid at all,
/// is set to the current interest index if base interest was repaid fully, and is a solution to
/// the equation `debt * (indexNow / indexLastUpdate - 1) - delta = debt * (indexNow / indexNew - 1)`
/// when only `delta` of accrued interest was repaid
@@ 613,621 @@
/// @param amount Amount of debt to repay
/// @param debt Debt principal before repayment
/// @param cumulativeIndexNow The current interest index
/// @param cumulativeIndexLastUpdate Credit account's interest index as of last update
/// @return newDebt Debt principal after repayment
/// @return newCumulativeIndex Credit account's quota interest after repayment
/// @return profit Amount of underlying tokens received as fees by the DAO
/// @return newCumulativeQuotaInterest Credit account's accrued quota interest after repayment
// @return newQuotaFees Amount of unpaid quota fees left after repayment
function calcDecrease(
uint256 amount,
uint256 debt,
uint256 cumulativeIndexNow,
uint256 cumulativeIndexLastUpdate,
uint128 cumulativeQuotaInterest
)
internal
view
returns (uint256 newDebt, uint256 newCumulativeIndex, uint256 profit, uint128 newCumulativeQuotaInterest)
{
uint256 amountToRepay = amount;
if (cumulativeQuotaInterest != 0 && amountToRepay != 0) {
// All interest accrued on the quota interest is taken by the DAO to be distributed to LP stakers, dLP stakers and the DAO
if (amountToRepay >= cumulativeQuotaInterest) {
amountToRepay -= cumulativeQuotaInterest; // U:[CL-3]
profit += cumulativeQuotaInterest; // U:[CL-3]
newCumulativeQuotaInterest = 0; // U:[CL-3]
} else {
// If amount is not enough to repay quota interest + DAO fee, then send all to the stakers
uint256 quotaInterestPaid = amountToRepay; // U:[CL-3]
profit += amountToRepay; // U:[CL-3]
amountToRepay = 0; // U:[CL-3]
newCumulativeQuotaInterest = uint128(cumulativeQuotaInterest - quotaInterestPaid); // U:[CL-3]
}
} else {
newCumulativeQuotaInterest = cumulativeQuotaInterest;
}
if (amountToRepay != 0) {
uint256 interestAccrued = CreditLogic.calcAccruedInterest({
amount: debt,
cumulativeIndexLastUpdate: cumulativeIndexLastUpdate,
cumulativeIndexNow: cumulativeIndexNow
}); // U:[CL-3]
// All interest accrued on the base interest is taken by the DAO to be distributed to LP stakers, dLP stakers and the DAO
if (amountToRepay >= interestAccrued) {
amountToRepay -= interestAccrued;
profit += interestAccrued; // U:[CL-3]
newCumulativeIndex = cumulativeIndexNow; // U:[CL-3]
} else {
// If amount is not enough to repay quota interest + DAO fee, then send all to the stakers
profit += interestAccrued; // U:[CL-3]
amountToRepay = 0; // U:[CL-3]
newCumulativeIndex = cumulativeIndexNow;
}
} else {
newCumulativeIndex = cumulativeIndexLastUpdate;
}
newDebt = debt - amountToRepay;
}
[WP-M7] PoolV3._updateBaseInterest() Double Counts InterestAccrued and QuotaRevenueAccrued When Calculating expectedLiquidity_
The expectedLiquidity() function at PoolV3.sol#L644 already includes all accrued interest (_calcBaseInterestAccrued()), which incorporates the profit from the current period (see PoolV3.sol#L564).
Adding expectedLiquidityDelta to expectedLiquidity() at PoolV3.sol#L644 results in double counting of the profit.
@@ 349,362 @@
/// @notice Modifies a Position's collateral and debt balances
/// @dev Checks that the global debt ceiling and the vault's debt ceiling have not been exceeded via the CDM,
/// - that the Position is still safe after the modification,
/// - that the msg.sender has the permission of the owner to decrease the collateral-to-debt ratio,
/// - that the msg.sender has the permission of the collateralizer to put up new collateral,
/// - that the msg.sender has the permission of the creditor to settle debt with their credit,
/// - that that the vault debt floor is exceeded
/// - that the vault minimum collateralization ratio is met
/// @param owner Address of the owner of the position
/// @param collateralizer Address of who puts up or receives the collateral delta
/// @param creditor Address of who provides or receives the credit delta for the debt delta
/// @param deltaCollateral Amount of collateral to put up (+) or to remove (-) from the position [wad]
/// @param deltaDebt Amount of normalized debt (gross, before rate is applied) to generate (+) or
/// to settle (-) on this position [wad]
function modifyCollateralAndDebt(
address owner,
address collateralizer,
address creditor,
int256 deltaCollateral,
int256 deltaDebt
) public {
if (
// position is either more safe than before or msg.sender has the permission from the owner
((deltaDebt > 0 || deltaCollateral < 0) && !hasPermission(owner, msg.sender)) ||
// msg.sender has the permission of the collateralizer to collateralize the position using their cash
(deltaCollateral > 0 && !hasPermission(collateralizer, msg.sender)) ||
// msg.sender has the permission of the creditor to use their credit to repay the debt
(deltaDebt < 0 && !hasPermission(creditor, msg.sender))
) revert CDPVault__modifyCollateralAndDebt_noPermission();
Position memory position = positions[owner];
DebtData memory debtData = _calcDebt(position);
uint256 newDebt;
uint256 newCumulativeIndex;
uint256 profit;
int256 quotaRevenueChange;
if (deltaDebt > 0) {
(newDebt, newCumulativeIndex) = CreditLogic.calcIncrease(
uint256(deltaDebt), // delta debt
position.debt,
debtData.cumulativeIndexNow, // current cumulative base interest index in Ray
position.cumulativeIndexLastUpdate
); // U:[CM-10]
position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow;
quotaRevenueChange = _calcQuotaRevenueChange(deltaDebt);
pool.lendCreditAccount(uint256(deltaDebt), creditor); // F:[CM-20]
} else if (deltaDebt < 0) {
uint256 maxRepayment = calcTotalDebt(debtData);
uint256 amount = abs(deltaDebt);
if (amount >= maxRepayment) {
amount = maxRepayment; // U:[CM-11]
deltaDebt = -toInt256(maxRepayment);
}
poolUnderlying.safeTransferFrom(creditor, address(pool), amount);
uint128 newCumulativeQuotaInterest;
if (amount == maxRepayment) {
newDebt = 0;
newCumulativeIndex = debtData.cumulativeIndexNow;
profit = debtData.accruedInterest;
newCumulativeQuotaInterest = 0;
} else {
(newDebt, newCumulativeIndex, profit, newCumulativeQuotaInterest) = calcDecrease(
amount, // delta debt
position.debt,
debtData.cumulativeIndexNow, // current cumulative base interest index in Ray
position.cumulativeIndexLastUpdate,
debtData.cumulativeQuotaInterest
);
}
quotaRevenueChange = _calcQuotaRevenueChange(-int(debtData.debt - newDebt));
pool.repayCreditAccount(debtData.debt - newDebt, profit, 0); // U:[CM-11]
position.cumulativeQuotaInterest = newCumulativeQuotaInterest;
position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow;
} else {
newDebt = position.debt;
newCumulativeIndex = debtData.cumulativeIndexNow;
}
if (deltaCollateral > 0) {
uint256 amount = wmul(deltaCollateral.toUint256(), tokenScale);
token.safeTransferFrom(collateralizer, address(this), amount);
} else if (deltaCollateral < 0) {
uint256 amount = wmul(abs(deltaCollateral), tokenScale);
token.safeTransfer(collateralizer, amount);
}
position = _modifyPosition(owner, position, newDebt, newCumulativeIndex, deltaCollateral, totalDebt);
VaultConfig memory config = vaultConfig;
uint256 spotPrice_ = spotPrice();
uint256 collateralValue = wmul(position.collateral, spotPrice_);
if (
(deltaDebt > 0 || deltaCollateral < 0) &&
!_isCollateralized(newDebt, collateralValue, config.liquidationRatio)
) revert CDPVault__modifyCollateralAndDebt_notSafe();
if (quotaRevenueChange != 0) {
IPoolV3(pool).updateQuotaRevenue(quotaRevenueChange); // U:[PQK-15]
}
emit ModifyCollateralAndDebt(owner, collateralizer, creditor, deltaCollateral, deltaDebt);
}
@@ 633,638 @@
/// @dev Updates base interest rate based on expected and available liquidity deltas
/// - Adds expected liquidity delta to stored expected liquidity
/// - If time has passed since the last base interest update, adds accrued interest
/// to stored expected liquidity, updates interest index and last update timestamp
/// - If time has passed since the last quota revenue update, adds accrued revenue
/// to stored expected liquidity and updates last update timestamp
function _updateBaseInterest(
int256 expectedLiquidityDelta,
int256 availableLiquidityDelta,
bool checkOptimalBorrowing
) internal {
uint256 expectedLiquidity_ = (expectedLiquidity().toInt256() + expectedLiquidityDelta).toUint256();
@@ 645,655 @@
uint256 availableLiquidity_ = (availableLiquidity().toInt256() + availableLiquidityDelta).toUint256();
uint256 lastBaseInterestUpdate_ = lastBaseInterestUpdate;
if (block.timestamp != lastBaseInterestUpdate_) {
_baseInterestIndexLU = _calcBaseInterestIndex(lastBaseInterestUpdate_).toUint128(); // U:[LP-18]
lastBaseInterestUpdate = uint40(block.timestamp);
}
if (block.timestamp != lastQuotaRevenueUpdate) {
lastQuotaRevenueUpdate = uint40(block.timestamp); // U:[LP-18]
}
_expectedLiquidityLU = expectedLiquidity_.toUint128(); // U:[LP-18]
_baseInterestRate = ILinearInterestRateModelV3(interestRateModel)
.calcBorrowRate({
expectedLiquidity: expectedLiquidity_,
availableLiquidity: availableLiquidity_,
checkOptimalBorrowing: checkOptimalBorrowing
})
.toUint128(); // U:[LP-18]
}
@@ 513,525 @@
/// @notice Updates pool state to indicate debt repayment, can only be called by credit managers
/// after transferring underlying from a credit account to the pool.
/// - If transferred amount exceeds debt principal + base interest + quota interest,
/// the difference is deemed protocol's profit and the respective number of shares
/// is minted to the treasury.
/// - If, however, transferred amount is insufficient to repay debt and interest,
/// which may only happen during liquidation, treasury's shares are burned to
/// cover as much of the loss as possible.
/// @param repaidAmount Amount of debt principal repaid
/// @param profit Pool's profit in underlying after repaying
/// @param loss Pool's loss in underlying after repaying
/// @custom:expects Credit manager transfers underlying from a credit account to the pool before calling this function
/// @custom:expects Profit/loss computed in the credit manager are cosistent with pool's implicit calculations
function repayCreditAccount(
uint256 repaidAmount,
uint256 profit,
uint256 loss
)
@@ 531,535 @@
external
override
creditManagerOnly // U:[LP-2C]
whenNotPaused // U:[LP-2A]
nonReentrant // U:[LP-2B]
{
@@ 537,562 @@
uint128 repaidAmountU128 = repaidAmount.toUint128();
DebtParams storage cmDebt = _creditManagerDebt[msg.sender];
uint128 cmBorrowed = cmDebt.borrowed;
if (cmBorrowed == 0) {
revert CallerNotCreditManagerException(); // U:[LP-2C,14A]
}
if (profit > 0) {
_mint(treasury, convertToShares(profit)); // U:[LP-14B]
} else if (loss > 0) {
address treasury_ = treasury;
uint256 sharesInTreasury = balanceOf(treasury_);
uint256 sharesToBurn = convertToShares(loss);
if (sharesToBurn > sharesInTreasury) {
unchecked {
emit IncurUncoveredLoss({
creditManager: msg.sender,
loss: convertToAssets(sharesToBurn - sharesInTreasury)
}); // U:[LP-14D]
}
sharesToBurn = sharesInTreasury;
}
_burn(treasury_, sharesToBurn); // U:[LP-14C,14D]
}
_updateBaseInterest({
expectedLiquidityDelta: profit.toInt256() - loss.toInt256(),
availableLiquidityDelta: 0,
checkOptimalBorrowing: false
}); // U:[LP-14B,14C,14D]
_totalDebt.borrowed -= repaidAmountU128; // U:[LP-14B,14C,14D]
cmDebt.borrowed = cmBorrowed - repaidAmountU128; // U:[LP-14B,14C,14D]
emit Repay(msg.sender, repaidAmount, profit, loss); // U:[LP-14B,14C,14D]
}
/// @notice Amount of underlying that would be in the pool if debt principal, base interest
/// and quota revenue were fully repaid
function expectedLiquidity() public view override returns (uint256) {
return _expectedLiquidityLU + _calcBaseInterestAccrued() + _calcQuotaRevenueAccrued(); // U:[LP-4]
}
@@ 633,638 @@
/// @dev Updates base interest rate based on expected and available liquidity deltas
/// - Adds expected liquidity delta to stored expected liquidity
/// - If time has passed since the last base interest update, adds accrued interest
/// to stored expected liquidity, updates interest index and last update timestamp
/// - If time has passed since the last quota revenue update, adds accrued revenue
/// to stored expected liquidity and updates last update timestamp
function _updateBaseInterest(
int256 expectedLiquidityDelta,
int256 availableLiquidityDelta,
bool checkOptimalBorrowing
) internal {
uint256 expectedLiquidity_ = (expectedLiquidity().toInt256() + expectedLiquidityDelta).toUint256();
@@ 645,655 @@
uint256 availableLiquidity_ = (availableLiquidity().toInt256() + availableLiquidityDelta).toUint256();
uint256 lastBaseInterestUpdate_ = lastBaseInterestUpdate;
if (block.timestamp != lastBaseInterestUpdate_) {
_baseInterestIndexLU = _calcBaseInterestIndex(lastBaseInterestUpdate_).toUint128(); // U:[LP-18]
lastBaseInterestUpdate = uint40(block.timestamp);
}
if (block.timestamp != lastQuotaRevenueUpdate) {
lastQuotaRevenueUpdate = uint40(block.timestamp); // U:[LP-18]
}
_expectedLiquidityLU = expectedLiquidity_.toUint128(); // U:[LP-18]
_baseInterestRate = ILinearInterestRateModelV3(interestRateModel)
.calcBorrowRate({
expectedLiquidity: expectedLiquidity_,
availableLiquidity: availableLiquidity_,
checkOptimalBorrowing: checkOptimalBorrowing
})
.toUint128(); // U:[LP-18]
}
@@ 513,525 @@
/// @notice Updates pool state to indicate debt repayment, can only be called by credit managers
/// after transferring underlying from a credit account to the pool.
/// - If transferred amount exceeds debt principal + base interest + quota interest,
/// the difference is deemed protocol's profit and the respective number of shares
/// is minted to the treasury.
/// - If, however, transferred amount is insufficient to repay debt and interest,
/// which may only happen during liquidation, treasury's shares are burned to
/// cover as much of the loss as possible.
/// @param repaidAmount Amount of debt principal repaid
/// @param profit Pool's profit in underlying after repaying
/// @param loss Pool's loss in underlying after repaying
/// @custom:expects Credit manager transfers underlying from a credit account to the pool before calling this function
/// @custom:expects Profit/loss computed in the credit manager are cosistent with pool's implicit calculations
function repayCreditAccount(
uint256 repaidAmount,
uint256 profit,
uint256 loss
)
@@ 531,535 @@
external
override
creditManagerOnly // U:[LP-2C]
whenNotPaused // U:[LP-2A]
nonReentrant // U:[LP-2B]
{
@@ 537,562 @@
uint128 repaidAmountU128 = repaidAmount.toUint128();
DebtParams storage cmDebt = _creditManagerDebt[msg.sender];
uint128 cmBorrowed = cmDebt.borrowed;
if (cmBorrowed == 0) {
revert CallerNotCreditManagerException(); // U:[LP-2C,14A]
}
if (profit > 0) {
_mint(treasury, convertToShares(profit)); // U:[LP-14B]
} else if (loss > 0) {
address treasury_ = treasury;
uint256 sharesInTreasury = balanceOf(treasury_);
uint256 sharesToBurn = convertToShares(loss);
if (sharesToBurn > sharesInTreasury) {
unchecked {
emit IncurUncoveredLoss({
creditManager: msg.sender,
loss: convertToAssets(sharesToBurn - sharesInTreasury)
}); // U:[LP-14D]
}
sharesToBurn = sharesInTreasury;
}
_burn(treasury_, sharesToBurn); // U:[LP-14C,14D]
}
_updateBaseInterest({
expectedLiquidityDelta: profit.toInt256() - loss.toInt256(),
availableLiquidityDelta: 0,
checkOptimalBorrowing: false
}); // U:[LP-14B,14C,14D]
_totalDebt.borrowed -= repaidAmountU128; // U:[LP-14B,14C,14D]
cmDebt.borrowed = cmBorrowed - repaidAmountU128; // U:[LP-14B,14C,14D]
emit Repay(msg.sender, repaidAmount, profit, loss); // U:[LP-14B,14C,14D]
}
/// @notice Amount of underlying that would be in the pool if debt principal, base interest
/// and quota revenue were fully repaid
function expectedLiquidity() public view override returns (uint256) {
return _expectedLiquidityLU + _calcBaseInterestAccrued() + _calcQuotaRevenueAccrued(); // U:[LP-4]
}
[WP-G8] Directly test discountedPrice instead of doing another duplicate external call
@@ 507,518 @@
/*//////////////////////////////////////////////////////////////
LIQUIDATION
//////////////////////////////////////////////////////////////*/
/// @notice Liquidates a single unsafe position by selling collateral at a discounted (`liquidationDiscount`)
/// oracle price. The liquidator has to provide the amount he wants to repay or sell (`repayAmounts`) for
/// the position. From that repay amount a penalty (`liquidationPenalty`) is subtracted to mitigate against
/// profitable self liquidations. If the available collateral of a position is not sufficient to cover the debt
/// the vault accumulates 'bad debt'.
/// @dev The liquidator has to approve the vault to transfer the sum of `repayAmounts`.
/// @param owner Owner of the position to liquidate
/// @param repayAmount Amount the liquidator wants to repay [wad]
function liquidatePosition(address owner, uint256 repayAmount) external whenNotPaused {
@@ 520,530 @@
// validate params
if (owner == address(0) || repayAmount == 0) revert CDPVault__liquidatePosition_invalidParameters();
// load configs
VaultConfig memory config = vaultConfig;
LiquidationConfig memory liqConfig_ = liquidationConfig;
// load liquidated position
Position memory position = positions[owner];
DebtData memory debtData = _calcDebt(position);
// load price and calculate discounted price
uint256 discountedPrice = wmul(spotPrice(), liqConfig_.liquidationDiscount);
if (spotPrice() == 0) revert CDPVault__liquidatePosition_invalidSpotPrice();
@@ 535,593 @@
// compute collateral to take, debt to repay and penalty to pay
uint256 takeCollateral = wdiv(repayAmount, discountedPrice);
uint256 deltaDebt = wmul(repayAmount, liqConfig_.liquidationPenalty);
uint256 penalty = wmul(repayAmount, WAD - liqConfig_.liquidationPenalty);
// verify that the position is indeed unsafe
if (_isCollateralized(calcTotalDebt(debtData), wmul(position.collateral, spotPrice()), config.liquidationRatio))
revert CDPVault__liquidatePosition_notUnsafe();
// account for bad debt
// TODO: review this
uint256 loss;
if (takeCollateral > position.collateral) {
takeCollateral = position.collateral;
repayAmount = wmul(takeCollateral, discountedPrice);
penalty = wmul(repayAmount, WAD - liqConfig_.liquidationPenalty);
deltaDebt = debtData.debt;
loss = calcTotalDebt(debtData) - deltaDebt;
}
// transfer the repay amount from the liquidator to the vault
poolUnderlying.safeTransferFrom(msg.sender, address(pool), repayAmount);
uint256 newDebt;
uint256 profit;
uint256 maxRepayment = calcTotalDebt(debtData);
uint256 newCumulativeIndex;
//uint128 newCumulativeQuotaInterest;
// uint128 quotaFees;
if (deltaDebt == maxRepayment) {
newDebt = 0;
newCumulativeIndex = debtData.cumulativeIndexNow;
profit = debtData.accruedInterest;
position.cumulativeQuotaInterest = 0;
} else {
(newDebt, newCumulativeIndex, profit, position.cumulativeQuotaInterest) = calcDecrease(
deltaDebt, // delta debt
debtData.debt,
debtData.cumulativeIndexNow, // current cumulative base interest index in Ray
debtData.cumulativeIndexLastUpdate,
debtData.cumulativeQuotaInterest
);
}
// position.cumulativeQuotaInterest = newCumulativeQuotaInterest;
//position.quotaFees = quotaFees;
position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow;
// update liquidated position
position = _modifyPosition(owner, position, newDebt, newCumulativeIndex, -toInt256(takeCollateral), totalDebt);
pool.repayCreditAccount(debtData.debt - newDebt, profit, loss); // U:[CM-11]
// transfer the collateral amount from the vault to the liquidator
// cash[msg.sender] += takeCollateral;
token.safeTransfer(msg.sender, takeCollateral);
// Mint the penalty from the vault to the treasury
IPoolV3Loop(address(pool)).mintProfit(penalty);
}
Recommendation
// load price and calculate discounted price
uint256 discountedPrice = wmul(spotPrice(), liqConfig_.liquidationDiscount);
if (discountedPrice == 0) revert CDPVault__liquidatePosition_invalidSpotPrice();
[WP-G9] CDPVault.modifyCollateralAndDebt() when deltaDebt <= 0 && deltaCollateral >= 0, there is no need to calculate collateralValue and SLOAD config.
CDPVault.sol#L423-425 can be moved to after L428 to avoid unnecessary code execution.
@@ 343,356 @@
/// @notice Modifies a Position's collateral and debt balances
/// @dev Checks that the global debt ceiling and the vault's debt ceiling have not been exceeded via the CDM,
/// - that the Position is still safe after the modification,
/// - that the msg.sender has the permission of the owner to decrease the collateral-to-debt ratio,
/// - that the msg.sender has the permission of the collateralizer to put up new collateral,
/// - that the msg.sender has the permission of the creditor to settle debt with their credit,
/// - that that the vault debt floor is exceeded
/// - that the vault minimum collateralization ratio is met
/// @param owner Address of the owner of the position
/// @param collateralizer Address of who puts up or receives the collateral delta
/// @param creditor Address of who provides or receives the credit delta for the debt delta
/// @param deltaCollateral Amount of collateral to put up (+) or to remove (-) from the position [wad]
/// @param deltaDebt Amount of normalized debt (gross, before rate is applied) to generate (+) or
/// to settle (-) on this position [wad]
function modifyCollateralAndDebt(
address owner,
address collateralizer,
address creditor,
int256 deltaCollateral,
int256 deltaDebt
) public {
if (
// position is either more safe than before or msg.sender has the permission from the owner
((deltaDebt > 0 || deltaCollateral < 0) && !hasPermission(owner, msg.sender)) ||
// msg.sender has the permission of the collateralizer to collateralize the position using their cash
(deltaCollateral > 0 && !hasPermission(collateralizer, msg.sender)) ||
// msg.sender has the permission of the creditor to use their credit to repay the debt
(deltaDebt < 0 && !hasPermission(creditor, msg.sender))
) revert CDPVault__modifyCollateralAndDebt_noPermission();
Position memory position = positions[owner];
DebtData memory debtData = _calcDebt(position);
uint256 newDebt;
uint256 newCumulativeIndex;
uint256 profit;
if (deltaDebt > 0) {
@@ 380,387 @@
(newDebt, newCumulativeIndex) = calcIncrease(
uint256(deltaDebt), // delta debt
position.debt,
debtData.cumulativeIndexNow, // current cumulative base interest index in Ray
position.cumulativeIndexLastUpdate
); // U:[CM-10]
pool.lendCreditAccount(uint256(deltaDebt), creditor); // F:[CM-20]
} else if (deltaDebt < 0) {
uint256 maxRepayment = calcTotalDebt(debtData);
uint256 amount = abs(deltaDebt);
if (amount >= maxRepayment) {
amount = maxRepayment; // U:[CM-11]
}
poolUnderlying.safeTransferFrom(creditor, address(pool), amount);
if (amount == maxRepayment) {
newDebt = 0;
newCumulativeIndex = debtData.cumulativeIndexNow;
profit = debtData.accruedFees;
} else {
(newDebt, newCumulativeIndex, profit) = calcDecrease(
amount, // delta debt
position.debt,
debtData.cumulativeIndexNow, // current cumulative base interest index in Ray
position.cumulativeIndexLastUpdate
);
}
pool.repayCreditAccount(debtData.debt - newDebt, profit, 0); // U:[CM-11]
}
if (deltaCollateral > 0) {
uint256 amount = deltaCollateral.toUint256();
token.safeTransferFrom(collateralizer, address(this), amount);
} else if (deltaCollateral < 0) {
uint256 amount = abs(deltaCollateral);
token.safeTransfer(collateralizer, amount);
}
position = _modifyPosition(owner, position, newDebt, newCumulativeIndex, deltaCollateral, totalDebt);
VaultConfig memory config = vaultConfig;
uint256 spotPrice_ = spotPrice();
uint256 collateralValue = wmul(position.collateral, spotPrice_);
if (
(deltaDebt > 0 || deltaCollateral < 0) &&
!_isCollateralized(newDebt, collateralValue, config.liquidationRatio)
) revert CDPVault__modifyCollateralAndDebt_notSafe();
emit ModifyCollateralAndDebt(owner, collateralizer, creditor, deltaCollateral, deltaDebt);
}
@@ 343,356 @@
/// @notice Modifies a Position's collateral and debt balances
/// @dev Checks that the global debt ceiling and the vault's debt ceiling have not been exceeded via the CDM,
/// - that the Position is still safe after the modification,
/// - that the msg.sender has the permission of the owner to decrease the collateral-to-debt ratio,
/// - that the msg.sender has the permission of the collateralizer to put up new collateral,
/// - that the msg.sender has the permission of the creditor to settle debt with their credit,
/// - that that the vault debt floor is exceeded
/// - that the vault minimum collateralization ratio is met
/// @param owner Address of the owner of the position
/// @param collateralizer Address of who puts up or receives the collateral delta
/// @param creditor Address of who provides or receives the credit delta for the debt delta
/// @param deltaCollateral Amount of collateral to put up (+) or to remove (-) from the position [wad]
/// @param deltaDebt Amount of normalized debt (gross, before rate is applied) to generate (+) or
/// to settle (-) on this position [wad]
function modifyCollateralAndDebt(
address owner,
address collateralizer,
address creditor,
int256 deltaCollateral,
int256 deltaDebt
) public {
if (
// position is either more safe than before or msg.sender has the permission from the owner
((deltaDebt > 0 || deltaCollateral < 0) && !hasPermission(owner, msg.sender)) ||
// msg.sender has the permission of the collateralizer to collateralize the position using their cash
(deltaCollateral > 0 && !hasPermission(collateralizer, msg.sender)) ||
// msg.sender has the permission of the creditor to use their credit to repay the debt
(deltaDebt < 0 && !hasPermission(creditor, msg.sender))
) revert CDPVault__modifyCollateralAndDebt_noPermission();
Position memory position = positions[owner];
DebtData memory debtData = _calcDebt(position);
uint256 newDebt;
uint256 newCumulativeIndex;
uint256 profit;
if (deltaDebt > 0) {
@@ 380,387 @@
(newDebt, newCumulativeIndex) = calcIncrease(
uint256(deltaDebt), // delta debt
position.debt,
debtData.cumulativeIndexNow, // current cumulative base interest index in Ray
position.cumulativeIndexLastUpdate
); // U:[CM-10]
pool.lendCreditAccount(uint256(deltaDebt), creditor); // F:[CM-20]
} else if (deltaDebt < 0) {
uint256 maxRepayment = calcTotalDebt(debtData);
uint256 amount = abs(deltaDebt);
if (amount >= maxRepayment) {
amount = maxRepayment; // U:[CM-11]
}
poolUnderlying.safeTransferFrom(creditor, address(pool), amount);
if (amount == maxRepayment) {
newDebt = 0;
newCumulativeIndex = debtData.cumulativeIndexNow;
profit = debtData.accruedFees;
} else {
(newDebt, newCumulativeIndex, profit) = calcDecrease(
amount, // delta debt
position.debt,
debtData.cumulativeIndexNow, // current cumulative base interest index in Ray
position.cumulativeIndexLastUpdate
);
}
pool.repayCreditAccount(debtData.debt - newDebt, profit, 0); // U:[CM-11]
}
if (deltaCollateral > 0) {
uint256 amount = deltaCollateral.toUint256();
token.safeTransferFrom(collateralizer, address(this), amount);
} else if (deltaCollateral < 0) {
uint256 amount = abs(deltaCollateral);
token.safeTransfer(collateralizer, amount);
}
position = _modifyPosition(owner, position, newDebt, newCumulativeIndex, deltaCollateral, totalDebt);
VaultConfig memory config = vaultConfig;
uint256 spotPrice_ = spotPrice();
uint256 collateralValue = wmul(position.collateral, spotPrice_);
if (
(deltaDebt > 0 || deltaCollateral < 0) &&
!_isCollateralized(newDebt, collateralValue, config.liquidationRatio)
) revert CDPVault__modifyCollateralAndDebt_notSafe();
emit ModifyCollateralAndDebt(owner, collateralizer, creditor, deltaCollateral, deltaDebt);
}