diff --git a/packages/contracts/contracts/BorrowerOperations.sol b/packages/contracts/contracts/BorrowerOperations.sol index 589392ec4..d6db169c5 100644 --- a/packages/contracts/contracts/BorrowerOperations.sol +++ b/packages/contracts/contracts/BorrowerOperations.sol @@ -170,7 +170,7 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe vars.LUSDFee = _triggerBorrowingFee(contractsCache.troveManager, contractsCache.lusdToken, _LUSDAmount, _maxFeePercentage); vars.netDebt = vars.netDebt.add(vars.LUSDFee); } - _requireAtLeastMinNetDebt(vars.netDebt); + _requireAtLeastMinNetDebt(vars.netDebt, 0); // ICR is based on the composite debt, i.e. the requested LUSD amount + LUSD borrowing fee + LUSD gas comp. vars.compositeDebt = _getCompositeDebt(vars.netDebt); @@ -279,21 +279,20 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe vars.debt = contractsCache.troveManager.getTroveDebt(_borrower); vars.coll = contractsCache.troveManager.getTroveColl(_borrower); - + + // When the adjustment is a debt repayment, check it's a valid amount and that the caller has enough LUSD + if (!_isDebtIncrease && _LUSDChange > 0) { + _requireAtLeastMinNetDebt(_getNetDebt(vars.debt), vars.netDebtChange); + _requireSufficientLUSDBalance(contractsCache.lusdToken, _borrower, vars.netDebtChange); + } + // Get the trove's old ICR before the adjustment, and what its new ICR will be after the adjustment vars.oldICR = LiquityMath._computeCR(vars.coll, vars.debt, vars.price); vars.newICR = _getNewICRFromTroveChange(vars.coll, vars.debt, vars.collChange, vars.isCollIncrease, vars.netDebtChange, _isDebtIncrease, vars.price); - assert(_collWithdrawal <= vars.coll); + assert(_collWithdrawal <= vars.coll); // Check the adjustment satisfies all conditions for the current system mode _requireValidAdjustmentInCurrentMode(isRecoveryMode, _collWithdrawal, _isDebtIncrease, vars); - - // When the adjustment is a debt repayment, check it's a valid amount and that the caller has enough LUSD - if (!_isDebtIncrease && _LUSDChange > 0) { - _requireAtLeastMinNetDebt(_getNetDebt(vars.debt).sub(vars.netDebtChange)); - _requireValidLUSDRepayment(vars.debt, vars.netDebtChange); - _requireSufficientLUSDBalance(contractsCache.lusdToken, _borrower, vars.netDebtChange); - } (vars.newColl, vars.newDebt) = _updateTroveFromAdjustment(contractsCache.troveManager, _borrower, vars.collChange, vars.isCollIncrease, vars.netDebtChange, _isDebtIncrease); vars.stake = contractsCache.troveManager.updateStakeAndTotalStakes(_borrower); @@ -466,10 +465,6 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe require(msg.value == 0 || _collWithdrawal == 0, "BorrowerOperations: Cannot withdraw and add coll"); } - function _requireCallerIsBorrower(address _borrower) internal view { - require(msg.sender == _borrower, "BorrowerOps: Caller must be the borrower for a withdrawal"); - } - function _requireNonZeroAdjustment(uint _collWithdrawal, uint _LUSDChange) internal view { require(msg.value != 0 || _collWithdrawal != 0 || _LUSDChange != 0, "BorrowerOps: There must be either a collateral change or a debt change"); } @@ -548,12 +543,8 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe require(_newTCR >= CCR, "BorrowerOps: An operation that would result in TCR < CCR is not permitted"); } - function _requireAtLeastMinNetDebt(uint _netDebt) internal pure { - require (_netDebt >= MIN_NET_DEBT, "BorrowerOps: Trove's net debt must be greater than minimum"); - } - - function _requireValidLUSDRepayment(uint _currentDebt, uint _debtRepayment) internal pure { - require(_debtRepayment <= _currentDebt.sub(LUSD_GAS_COMPENSATION), "BorrowerOps: Amount repaid must not be larger than the Trove's debt"); + function _requireAtLeastMinNetDebt(uint _currentNetDebt, uint _debtRepayment) internal pure { + require (_currentNetDebt >= MIN_NET_DEBT.add(_debtRepayment), "BorrowerOps: Trove's net debt must be greater than minimum"); } function _requireCallerIsStabilityPool() internal view { @@ -565,13 +556,11 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe } function _requireValidMaxFeePercentage(uint _maxFeePercentage, bool _isRecoveryMode) internal pure { - if (_isRecoveryMode) { - require(_maxFeePercentage <= DECIMAL_PRECISION, - "Max fee percentage must less than or equal to 100%"); - } else { - require(_maxFeePercentage >= BORROWING_FEE_FLOOR && _maxFeePercentage <= DECIMAL_PRECISION, - "Max fee percentage must be between 0.5% and 100%"); - } + // In recovery mode we are not charging borrowing fee, so we can ignore this param + if (_isRecoveryMode) { return; } + + require(_maxFeePercentage >= BORROWING_FEE_FLOOR && _maxFeePercentage <= DECIMAL_PRECISION, + "Max fee percentage must be between 0.5% and 100%"); } // --- ICR and TCR getters --- diff --git a/packages/contracts/contracts/TroveManager.sol b/packages/contracts/contracts/TroveManager.sol index 147e43353..37a58c281 100644 --- a/packages/contracts/contracts/TroveManager.sol +++ b/packages/contracts/contracts/TroveManager.sol @@ -788,10 +788,16 @@ contract TroveManager is LiquityBase, Ownable, CheckContract, ITroveManager { } function _sendGasCompensation(IActivePool _activePool, address _liquidator, uint _LUSD, uint _ETH) internal { + // Before calling this function, we always check that something was liquidated, otherwise revert. + // LUSD gas compensation could then only be zero if we set to zero that constant, but it’s ok to have this here as a sanity check if (_LUSD > 0) { lusdToken.returnFromPool(gasPoolAddress, _liquidator, _LUSD); } + // ETH gas compensation could only be zero if all liquidated troves in the sequence had collateral lower than 200 Wei + // (see _getCollGasCompensation function in LiquityBase) + // With the current values of min debt this seems quite unlikely, unless ETH price was in the order of magnitude of $10^19 or more, + // but it’s ok to have this here as a sanity check if (_ETH > 0) { _activePool.sendETH(_liquidator, _ETH); } diff --git a/packages/contracts/test/BorrowerOperationsTest.js b/packages/contracts/test/BorrowerOperationsTest.js index b98ee259a..3017bcbc8 100644 --- a/packages/contracts/test/BorrowerOperationsTest.js +++ b/packages/contracts/test/BorrowerOperationsTest.js @@ -1365,6 +1365,17 @@ contract('BorrowerOperations', async accounts => { await assertRevert(repayTxAPromise, "BorrowerOps: Trove's net debt must be greater than minimum") }) + it("adjustTrove(): Reverts if repaid amount is greater than current debt", async () => { + const { totalDebt } = await openTrove({ extraLUSDAmount: toBN(dec(10000, 18)), ICR: toBN(dec(10, 18)), extraParams: { from: alice } }) + const repayAmount = totalDebt.add(toBN(1)) + await openTrove({ extraLUSDAmount: repayAmount, ICR: toBN(dec(150, 16)), extraParams: { from: bob } }) + + await lusdToken.transfer(alice, repayAmount, { from: bob }) + + await assertRevert(borrowerOperations.adjustTrove(th._100pct, 0, repayAmount, false, alice, alice, { from: alice }), + "BorrowerOps: Trove's net debt must be greater than minimum") + }) + it("repayLUSD(): reverts when calling address does not have active trove", async () => { await openTrove({ extraLUSDAmount: toBN(dec(10000, 18)), ICR: toBN(dec(2, 18)), extraParams: { from: alice } }) await openTrove({ extraLUSDAmount: toBN(dec(10000, 18)), ICR: toBN(dec(2, 18)), extraParams: { from: bob } })