Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Coverage post mainnet contracts #515

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Coverage post mainnet contracts #515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5d87d2f
contracts: Add comments to TroveManager
bingen Apr 12, 2021
bf8bc1f
contracts: Remove unused _requireCallerIsBorrower
bingen Apr 12, 2021
691f19f
contracts: Remove unnecessary _requireValidLUSDRepayment
bingen Apr 12, 2021
f4a21a4
contracts: Don’t check max fee percentage in Recovery Mode
bingen Apr 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 16 additions & 27 deletions packages/contracts/contracts/BorrowerOperations.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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");
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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 ---
Expand Down
6 changes: 6 additions & 0 deletions packages/contracts/contracts/TroveManager.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
11 changes: 11 additions & 0 deletions packages/contracts/test/BorrowerOperationsTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 } })
Expand Down