diff --git a/contracts/src/BorrowerOperations.sol b/contracts/src/BorrowerOperations.sol index 96257f24..a98218f4 100644 --- a/contracts/src/BorrowerOperations.sol +++ b/contracts/src/BorrowerOperations.sol @@ -272,6 +272,7 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe _requireValidAnnualInterestRate(_newAnnualInterestRate); ITroveManager troveManagerCached = troveManager; _requireTroveisActive(troveManagerCached, _troveId); + _requireIsOwner(_troveId); // TODO: apply individual and aggregate pending interest, and take snapshots of current timestamp. // TODO: determine how applying pending interest should interact / be sequenced with applying pending rewards from redistributions. @@ -422,8 +423,7 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe * Claim remaining collateral from a redemption or from a liquidation with ICR > MCR in Recovery Mode */ function claimCollateral(uint256 _troveId) external override { - address owner = troveManager.ownerOf(_troveId); - require(owner == msg.sender, "BO: Only owner can claim trove collateral"); + _requireIsOwner(_troveId); // send ETH from CollSurplus Pool to owner collSurplusPool.claimColl(msg.sender, _troveId); @@ -534,6 +534,10 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe require(_collChange != 0 || _boldChange != 0, "BorrowerOps: There must be either a collateral change or a debt change"); } + function _requireIsOwner(uint256 _troveId) internal view { + require(troveManager.ownerOf(_troveId) == msg.sender, "BO: Only owner"); + } + function _requireTroveisActive(ITroveManager _troveManager, uint256 _troveId) internal view { uint status = _troveManager.getTroveStatus(_troveId); require(status == 1, "BorrowerOps: Trove does not exist or is closed"); diff --git a/contracts/src/TroveManager.sol b/contracts/src/TroveManager.sol index 1850c86d..b64d71ea 100644 --- a/contracts/src/TroveManager.sol +++ b/contracts/src/TroveManager.sol @@ -889,8 +889,11 @@ contract TroveManager is ERC721, LiquityBase, Ownable, CheckContract, ITroveMana _maxIterations--; // Save the uint256 of the Trove preceding the current one, before potentially modifying the list uint256 nextUserToCheck = contractsCache.sortedTroves.getPrev(currentTroveId); - // TODO: check ICR? - //getCurrentICR(currentTroveId, _price) < MCR + // Skip if ICR < 100%, to make sure that redemptions always improve the CR of hit Troves + if (getCurrentICR(currentTroveId, totals.price) < _100pct) { + currentTroveId = nextUserToCheck; + continue; + } _applyPendingRewards(contractsCache.activePool, contractsCache.defaultPool, currentTroveId); @@ -1427,7 +1430,6 @@ contract TroveManager is ERC721, LiquityBase, Ownable, CheckContract, ITroveMana _updateTroveRewardSnapshots(_troveId); // mint ERC721 - // TODO: Should we use safeMint? I guess not _mint(_owner, _troveId); return _updateStakeAndTotalStakes(_troveId); diff --git a/contracts/src/test/TestContracts/BaseTest.sol b/contracts/src/test/TestContracts/BaseTest.sol index 38ed5b69..635581ea 100644 --- a/contracts/src/test/TestContracts/BaseTest.sol +++ b/contracts/src/test/TestContracts/BaseTest.sol @@ -32,6 +32,7 @@ contract BaseTest is Test { uint256 public constant MAX_UINT256 = type(uint256).max; uint256 public constant SECONDS_IN_1_YEAR = 31536000; // 60*60*24*365 + uint256 _100pct = 100e16; uint256 MCR = 110e16; uint256 CCR = 150e16; address public constant ZERO_ADDRESS = address(0); diff --git a/contracts/src/test/interestRateBasic.t.sol b/contracts/src/test/interestRateBasic.t.sol index fe79b271..3647170d 100644 --- a/contracts/src/test/interestRateBasic.t.sol +++ b/contracts/src/test/interestRateBasic.t.sol @@ -71,6 +71,20 @@ contract InterestRateBasic is DevTestSetup { borrowerOperations.openTrove(A, 1, 1e18, 2e18, 2000e18, 0, 0, 42e18); } + function testRevertWhenAdjustInterestRateFromNonOwner() public { + priceFeed.setPrice(2000e18); + + // A opens Trove + uint256 A_Id = openTroveNoHints100pctMaxFee(A, 2 ether, 2000e18, 37e16); + assertEq(troveManager.getTroveAnnualInterestRate(A_Id), 37e16); + + // B (who is not delegate) tries to adjust it + vm.startPrank(B); + vm.expectRevert("BO: Only owner"); + borrowerOperations.adjustTroveInterestRate(A_Id, 40e16, 0, 0); + vm.stopPrank(); + } + function testRevertWhenAdjustInterestRateGreaterThanMax() public { priceFeed.setPrice(2000e18); diff --git a/contracts/src/test/troveManager.t.sol b/contracts/src/test/troveManager.t.sol new file mode 100644 index 00000000..035add25 --- /dev/null +++ b/contracts/src/test/troveManager.t.sol @@ -0,0 +1,54 @@ +pragma solidity 0.8.18; + +import "./TestContracts/DevTestSetup.sol"; + + +contract TroveManagerTest is DevTestSetup { + + function testRedeemSkipTrovesUnder100pct() public { + priceFeed.setPrice(2000e18); + uint256 ATroveId = openTroveNoHints100pctMaxFee(A, 2 ether, 2001e18, 1e17); + uint256 BTroveId = openTroveNoHints100pctMaxFee(B, 5 ether, 2000e18, 2e17); + uint256 CTroveId = openTroveNoHints100pctMaxFee(C, 5 ether, 2000e18, 3e17); + + uint256 debtA1 = troveManager.getTroveDebt(ATroveId); + assertGt(debtA1, 0); + uint256 collA1 = troveManager.getTroveColl(ATroveId); + assertGt(collA1, 0); + uint256 debtB1 = troveManager.getTroveDebt(BTroveId); + assertGt(debtB1, 0); + uint256 collB1 = troveManager.getTroveColl(BTroveId); + assertGt(collB1, 0); + + // fast-forward until redemptions are enabled + vm.warp(block.timestamp + troveManager.BOOTSTRAP_PERIOD() + 1); + // Reduce ETH price so A’s ICR goes below 100% + uint256 newPrice = 1000e18; + priceFeed.setPrice(newPrice); + assertLt(troveManager.getCurrentICR(ATroveId, newPrice), _100pct); + assertGt(troveManager.getCurrentICR(BTroveId, newPrice), _100pct); + + uint256 redemptionAmount = 1000e18; // 1k BOLD + + // C redeems 1k BOLD + vm.startPrank(C); + troveManager.redeemCollateral( + redemptionAmount, + 10, + 1e18 + ); + vm.stopPrank(); + + // Check A's coll and debt are the same + uint256 debtA2 = troveManager.getTroveDebt(ATroveId); + assertEq(debtA2, debtA1, "A debt mismatch"); + uint256 collA2 = troveManager.getTroveColl(ATroveId); + assertEq(collA2, collA1, "A coll mismatch"); + + // Check B's coll and debt reduced + uint256 debtB2 = troveManager.getTroveDebt(BTroveId); + assertLt(debtB2, debtB1, "B debt mismatch"); + uint256 collB2 = troveManager.getTroveColl(BTroveId); + assertLt(collB2, collB1, "B coll mismatch"); + } +}