From 9311b0819843eba068a9b5990c5273c6b4e71c4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Fingen?= Date: Wed, 1 May 2024 13:24:59 +0100 Subject: [PATCH 01/13] Remove checkContract from CollSurplusPool --- contracts/src/CollSurplusPool.sol | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/contracts/src/CollSurplusPool.sol b/contracts/src/CollSurplusPool.sol index 76599a86..9c9b246c 100644 --- a/contracts/src/CollSurplusPool.sol +++ b/contracts/src/CollSurplusPool.sol @@ -6,9 +6,8 @@ import "openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol"; import "./Interfaces/ICollSurplusPool.sol"; import "./Dependencies/Ownable.sol"; -import "./Dependencies/CheckContract.sol"; -contract CollSurplusPool is Ownable, CheckContract, ICollSurplusPool { +contract CollSurplusPool is Ownable, ICollSurplusPool { using SafeERC20 for IERC20; string public constant NAME = "CollSurplusPool"; @@ -33,7 +32,6 @@ contract CollSurplusPool is Ownable, CheckContract, ICollSurplusPool { event EtherSent(address _to, uint256 _amount); constructor(address _ETHAddress) { - checkContract(_ETHAddress); ETH = IERC20(_ETHAddress); } @@ -44,10 +42,6 @@ contract CollSurplusPool is Ownable, CheckContract, ICollSurplusPool { override onlyOwner { - checkContract(_borrowerOperationsAddress); - checkContract(_troveManagerAddress); - checkContract(_activePoolAddress); - borrowerOperationsAddress = _borrowerOperationsAddress; troveManagerAddress = _troveManagerAddress; activePoolAddress = _activePoolAddress; From 3fee998e273a13f439cec4292b0c52ecb3860e1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Fingen?= Date: Sun, 28 Apr 2024 18:08:57 +0100 Subject: [PATCH 02/13] feat: Reduce liquidation penalty --- contracts/src/BorrowerOperations.sol | 24 +- contracts/src/CollateralRegistry.sol | 11 +- contracts/src/Dependencies/LiquityBase.sol | 3 - contracts/src/Dependencies/LiquityMath.sol | 4 + .../src/Interfaces/IBorrowerOperations.sol | 1 - contracts/src/Interfaces/ILiquityBase.sol | 1 - contracts/src/Interfaces/ITroveManager.sol | 2 + contracts/src/TroveManager.sol | 139 ++++++--- contracts/src/deployment.sol | 5 +- contracts/src/test/TestContracts/BaseTest.sol | 2 +- .../BorrowerOperationsTester.sol | 2 +- .../CollateralRegistryTester.sol | 4 +- .../src/test/TestContracts/DevTestSetup.sol | 2 + .../test/TestContracts/TroveManagerTester.sol | 4 + contracts/src/test/liquidations.t.sol | 284 ++++++++++++++++++ contracts/src/test/liquidationsLST.t.sol | 250 +++++++++++++++ contracts/src/test/redemptions.t.sol | 24 +- 17 files changed, 691 insertions(+), 71 deletions(-) create mode 100644 contracts/src/test/liquidations.t.sol create mode 100644 contracts/src/test/liquidationsLST.t.sol diff --git a/contracts/src/BorrowerOperations.sol b/contracts/src/BorrowerOperations.sol index 54c9afdd..ba5f462d 100644 --- a/contracts/src/BorrowerOperations.sol +++ b/contracts/src/BorrowerOperations.sol @@ -23,7 +23,7 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe // --- Connected contract declarations --- IERC20 public immutable ETH; - ITroveManager public troveManager; + ITroveManager public immutable troveManager; address stabilityPoolAddress; address gasPoolAddress; ICollSurplusPool collSurplusPool; @@ -31,6 +31,9 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe // A doubly linked list of Troves, sorted by their collateral ratios ISortedTroves public sortedTroves; + // Minimum collateral ratio for individual troves + uint256 public immutable MCR; + /* --- Variable container structs --- Used to hold, return and assign variables inside a function, in order to avoid the error: @@ -101,15 +104,21 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe ); event BoldBorrowingFeePaid(uint256 indexed _troveId, uint256 _boldFee); - constructor(address _ETHAddress) { - checkContract(_ETHAddress); - ETH = IERC20(_ETHAddress); + constructor(IERC20 _ETH, ITroveManager _troveManager) { + checkContract(address(_ETH)); + checkContract(address(_troveManager)); + + ETH = _ETH; + troveManager = _troveManager; + + MCR = _troveManager.MCR(); + + emit TroveManagerAddressChanged(address(_troveManager)); } // --- Dependency setters --- function setAddresses( - address _troveManagerAddress, address _activePoolAddress, address _defaultPoolAddress, address _stabilityPoolAddress, @@ -122,7 +131,6 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe // This makes impossible to open a trove with zero withdrawn Bold assert(MIN_NET_DEBT > 0); - checkContract(_troveManagerAddress); checkContract(_activePoolAddress); checkContract(_defaultPoolAddress); checkContract(_stabilityPoolAddress); @@ -132,7 +140,6 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe checkContract(_sortedTrovesAddress); checkContract(_boldTokenAddress); - troveManager = ITroveManager(_troveManagerAddress); activePool = IActivePool(_activePoolAddress); defaultPool = IDefaultPool(_defaultPoolAddress); stabilityPoolAddress = _stabilityPoolAddress; @@ -142,7 +149,6 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe sortedTroves = ISortedTroves(_sortedTrovesAddress); boldToken = IBoldToken(_boldTokenAddress); - emit TroveManagerAddressChanged(_troveManagerAddress); emit ActivePoolAddressChanged(_activePoolAddress); emit DefaultPoolAddressChanged(_defaultPoolAddress); emit StabilityPoolAddressChanged(_stabilityPoolAddress); @@ -770,7 +776,7 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe } } - function _requireICRisAboveMCR(uint256 _newICR) internal pure { + function _requireICRisAboveMCR(uint256 _newICR) internal view { require(_newICR >= MCR, "BorrowerOps: An operation that would result in ICR < MCR is not permitted"); } diff --git a/contracts/src/CollateralRegistry.sol b/contracts/src/CollateralRegistry.sol index 13504e18..6dcc49aa 100644 --- a/contracts/src/CollateralRegistry.sol +++ b/contracts/src/CollateralRegistry.sol @@ -118,7 +118,9 @@ contract CollateralRegistry is LiquityBase, ICollateralRegistry { uint256 redeemedAmount; } - function redeemCollateral(uint256 _boldAmount, uint256 _maxIterationsPerCollateral, uint256 _maxFeePercentage) external { + function redeemCollateral(uint256 _boldAmount, uint256 _maxIterationsPerCollateral, uint256 _maxFeePercentage) + external + { _requireValidMaxFeePercentage(_maxFeePercentage); _requireAmountGreaterThanZero(_boldAmount); _requireBoldBalanceCoversRedemption(boldToken, msg.sender, _boldAmount); @@ -135,7 +137,8 @@ contract CollateralRegistry is LiquityBase, ICollateralRegistry { // We only compute it here, and update it at the end, // because the final redeemed amount may be less than the requested amount // Redeemers should take this into account in order to request the optimal amount to not overpay - uint256 redemptionRate = _calcRedemptionRate(_getUpdatedBaseRateFromRedemption(_boldAmount, totals.boldSupplyAtStart)); + uint256 redemptionRate = + _calcRedemptionRate(_getUpdatedBaseRateFromRedemption(_boldAmount, totals.boldSupplyAtStart)); require(redemptionRate <= _maxFeePercentage, "CR: Fee exceeded provided maximum"); // Implicit by the above and the _requireValidMaxFeePercentage checks //require(newBaseRate < DECIMAL_PRECISION, "CR: Fee would eat up all collateral"); @@ -195,9 +198,7 @@ contract CollateralRegistry is LiquityBase, ICollateralRegistry { } // Updates the `baseRate` state with math from `_getUpdatedBaseRateFromRedemption` - function _updateBaseRateAndGetRedemptionRate(uint256 _boldAmount, uint256 _totalBoldSupplyAtStart) - internal - { + function _updateBaseRateAndGetRedemptionRate(uint256 _boldAmount, uint256 _totalBoldSupplyAtStart) internal { uint256 newBaseRate = _getUpdatedBaseRateFromRedemption(_boldAmount, _totalBoldSupplyAtStart); //assert(newBaseRate <= DECIMAL_PRECISION); // This is already enforced in `_getUpdatedBaseRateFromRedemption` diff --git a/contracts/src/Dependencies/LiquityBase.sol b/contracts/src/Dependencies/LiquityBase.sol index 83c64f2e..0c7cba76 100644 --- a/contracts/src/Dependencies/LiquityBase.sol +++ b/contracts/src/Dependencies/LiquityBase.sol @@ -20,9 +20,6 @@ contract LiquityBase is BaseMath, ILiquityBase { uint256 public constant _100pct = 1000000000000000000; // 1e18 == 100% - // Minimum collateral ratio for individual troves - uint256 public constant MCR = 1100000000000000000; // 110% - // Critical system collateral ratio. If the system's total collateral ratio (TCR) falls below the CCR, Recovery Mode is triggered. uint256 public constant CCR = 1500000000000000000; // 150% diff --git a/contracts/src/Dependencies/LiquityMath.sol b/contracts/src/Dependencies/LiquityMath.sol index a7c0718f..d96782eb 100644 --- a/contracts/src/Dependencies/LiquityMath.sol +++ b/contracts/src/Dependencies/LiquityMath.sol @@ -13,6 +13,10 @@ library LiquityMath { return (_a >= _b) ? _a : _b; } + function _sub_min_0(uint256 _a, uint256 _b) internal pure returns (uint256) { + return (_a > _b) ? _a - _b : 0; + } + /* * Multiply two decimal numbers and use normal rounding rules: * -round product up if 19'th mantissa digit >= 5 diff --git a/contracts/src/Interfaces/IBorrowerOperations.sol b/contracts/src/Interfaces/IBorrowerOperations.sol index 7609d7ae..e1929725 100644 --- a/contracts/src/Interfaces/IBorrowerOperations.sol +++ b/contracts/src/Interfaces/IBorrowerOperations.sol @@ -13,7 +13,6 @@ interface IBorrowerOperations is ILiquityBase { function sortedTroves() external view returns (ISortedTroves); function setAddresses( - address _troveManagerAddress, address _activePoolAddress, address _defaultPoolAddress, address _stabilityPoolAddress, diff --git a/contracts/src/Interfaces/ILiquityBase.sol b/contracts/src/Interfaces/ILiquityBase.sol index 81504c2d..086a1f73 100644 --- a/contracts/src/Interfaces/ILiquityBase.sol +++ b/contracts/src/Interfaces/ILiquityBase.sol @@ -12,6 +12,5 @@ interface ILiquityBase { function priceFeed() external view returns (IPriceFeed); function BOLD_GAS_COMPENSATION() external view returns (uint256); function MIN_NET_DEBT() external view returns (uint256); - function MCR() external view returns (uint256); function getEntireSystemDebt() external view returns (uint256); } diff --git a/contracts/src/Interfaces/ITroveManager.sol b/contracts/src/Interfaces/ITroveManager.sol index 22f777cc..bdfa989c 100644 --- a/contracts/src/Interfaces/ITroveManager.sol +++ b/contracts/src/Interfaces/ITroveManager.sol @@ -11,6 +11,8 @@ import "./ISortedTroves.sol"; // Common interface for the Trove Manager. interface ITroveManager is IERC721, ILiquityBase { + function MCR() external view returns (uint256); + function setAddresses( address _borrowerOperationsAddress, address _activePoolAddress, diff --git a/contracts/src/TroveManager.sol b/contracts/src/TroveManager.sol index fd62d3ee..d277f67a 100644 --- a/contracts/src/TroveManager.sol +++ b/contracts/src/TroveManager.sol @@ -31,6 +31,13 @@ contract TroveManager is ERC721, LiquityBase, Ownable, ITroveManager { // --- Data structures --- + // Minimum collateral ratio for individual troves + uint256 public immutable MCR; + // Liquidation penalty for troves offset to the SP + uint256 public immutable LIQUIDATION_PENALTY_SP; + // Liquidation penalty for troves redistributed + uint256 public immutable LIQUIDATION_PENALTY_REDISTRIBUTION; + uint256 public constant SECONDS_IN_ONE_YEAR = 31536000; // 60 * 60 * 24 * 365, uint256 public constant STALE_TROVE_DURATION = 7776000; // 90 days: 60*60*24*90 = 7776000 @@ -242,7 +249,18 @@ contract TroveManager is ERC721, LiquityBase, Ownable, ITroveManager { redeemCollateral } - constructor() ERC721(NAME, SYMBOL) {} + constructor(uint256 _mcr, uint256 _liquidationPenaltySP, uint256 _liquidationPenaltyRedistribution) + ERC721(NAME, SYMBOL) + { + require(_mcr > 1e18 && _mcr < 2e18, "Invalid MCR"); + require(_liquidationPenaltySP >= 5e16, "SP penalty too low"); + require(_liquidationPenaltySP <= _liquidationPenaltyRedistribution, "SP penalty cannot be > redist"); + require(_liquidationPenaltyRedistribution <= 10e16, "Redistribution penalty too high"); + + MCR = _mcr; + LIQUIDATION_PENALTY_SP = _liquidationPenaltySP; + LIQUIDATION_PENALTY_REDISTRIBUTION = _liquidationPenaltyRedistribution; + } // --- Dependency setter --- @@ -313,7 +331,8 @@ contract TroveManager is ERC721, LiquityBase, Ownable, ITroveManager { IActivePool _activePool, IDefaultPool _defaultPool, uint256 _troveId, - uint256 _boldInStabPool + uint256 _boldInStabPool, + uint256 _price ) internal returns (LiquidationValues memory singleLiquidation) { LocalVariables_InnerSingleLiquidateFunction memory vars; ( @@ -342,10 +361,17 @@ contract TroveManager is ERC721, LiquityBase, Ownable, ITroveManager { singleLiquidation.debtToOffset, singleLiquidation.collToSendToSP, singleLiquidation.debtToRedistribute, - singleLiquidation.collToRedistribute - ) = _getOffsetAndRedistributionVals(singleLiquidation.entireTroveDebt, collToLiquidate, _boldInStabPool); + singleLiquidation.collToRedistribute, + singleLiquidation.collSurplus + ) = _getOffsetAndRedistributionVals(singleLiquidation.entireTroveDebt, collToLiquidate, _boldInStabPool, _price); _closeTrove(_troveId, Status.closedByLiquidation); + + // Differencen between liquidation penalty and liquidation threshold + if (singleLiquidation.collSurplus > 0) { + collSurplusPool.accountSurplus(_troveId, singleLiquidation.collSurplus); + } + emit TroveLiquidated( _troveId, singleLiquidation.entireTroveDebt, @@ -416,12 +442,19 @@ contract TroveManager is ERC721, LiquityBase, Ownable, ITroveManager { singleLiquidation.debtToOffset, singleLiquidation.collToSendToSP, singleLiquidation.debtToRedistribute, - singleLiquidation.collToRedistribute + singleLiquidation.collToRedistribute, + singleLiquidation.collSurplus ) = _getOffsetAndRedistributionVals( - singleLiquidation.entireTroveDebt, vars.collToLiquidate, _boldInStabPool + singleLiquidation.entireTroveDebt, vars.collToLiquidate, _boldInStabPool, _price ); _closeTrove(_troveId, Status.closedByLiquidation); + + // Differencen between liquidation penalty and liquidation threshold + if (singleLiquidation.collSurplus > 0) { + collSurplusPool.accountSurplus(_troveId, singleLiquidation.collSurplus); + } + emit TroveLiquidated( _troveId, singleLiquidation.entireTroveDebt, @@ -475,33 +508,67 @@ contract TroveManager is ERC721, LiquityBase, Ownable, ITroveManager { */ function _getOffsetAndRedistributionVals( uint256 _entireTroveDebt, - uint256 _collToLiquidate, - uint256 _boldInStabPool + uint256 _collToLiquidate, // gas compensation is already subtracted + uint256 _boldInStabPool, + uint256 _price ) internal - pure - returns (uint256 debtToOffset, uint256 collToSendToSP, uint256 debtToRedistribute, uint256 collToRedistribute) + view + returns ( + uint256 debtToOffset, + uint256 collToSendToSP, + uint256 debtToRedistribute, + uint256 collToRedistribute, + uint256 collSurplus + ) { + uint256 collSPPortion; + /* + * Offset as much debt & collateral as possible against the Stability Pool, and redistribute the remainder + * between all active troves. + * + * If the trove's debt is larger than the deposited Bold in the Stability Pool: + * + * - Offset an amount of the trove's debt equal to the Bold in the Stability Pool + * - Send a fraction of the trove's collateral to the Stability Pool, equal to the fraction of its offset debt + * + */ if (_boldInStabPool > 0) { - /* - * Offset as much debt & collateral as possible against the Stability Pool, and redistribute the remainder - * between all active troves. - * - * If the trove's debt is larger than the deposited Bold in the Stability Pool: - * - * - Offset an amount of the trove's debt equal to the Bold in the Stability Pool - * - Send a fraction of the trove's collateral to the Stability Pool, equal to the fraction of its offset debt - * - */ debtToOffset = LiquityMath._min(_entireTroveDebt, _boldInStabPool); - collToSendToSP = _collToLiquidate * debtToOffset / _entireTroveDebt; - debtToRedistribute = _entireTroveDebt - debtToOffset; - collToRedistribute = _collToLiquidate - collToSendToSP; + collSPPortion = _collToLiquidate * debtToOffset / _entireTroveDebt; + (collToSendToSP, collSurplus) = + _getCollPenaltyAndSurplus(collSPPortion, debtToOffset, LIQUIDATION_PENALTY_SP, _price); + } + // TODO: this fails if debt in gwei is less than price (rounding coll to zero) + //assert(debtToOffset == 0 || collToSendToSP > 0); + + // Redistribution + debtToRedistribute = _entireTroveDebt - debtToOffset; + if (debtToRedistribute > 0) { + uint256 collRedistributionPortion = _collToLiquidate - collSPPortion; + if (collRedistributionPortion > 0) { + (collToRedistribute, collSurplus) = _getCollPenaltyAndSurplus( + collRedistributionPortion + collSurplus, // Coll surplus from offset can be eaten up by red. penalty + debtToRedistribute, LIQUIDATION_PENALTY_REDISTRIBUTION, _price + ); + } + } + assert(_collToLiquidate == collToSendToSP + collToRedistribute + collSurplus); + } + + function _getCollPenaltyAndSurplus( + uint256 _collToLiquidate, + uint256 _debtToLiquidate, + uint256 _penaltyRatio, + uint256 _price + ) internal pure returns (uint256 collPenalty, uint256 collSurplus) { + uint256 maxCollWithPenalty = _debtToLiquidate * (DECIMAL_PRECISION + _penaltyRatio) / _price; + if (_collToLiquidate > maxCollWithPenalty) { + collPenalty = maxCollWithPenalty; + collSurplus = _collToLiquidate - maxCollWithPenalty; } else { - debtToOffset = 0; - collToSendToSP = 0; - debtToRedistribute = _entireTroveDebt; - collToRedistribute = _collToLiquidate; + collPenalty = _collToLiquidate; + collSurplus = 0; } } @@ -514,11 +581,12 @@ contract TroveManager is ERC721, LiquityBase, Ownable, ITroveManager { uint256 _recordedTroveDebt, uint256 _weightedRecordedTroveDebt, uint256 _price - ) internal pure returns (LiquidationValues memory singleLiquidation) { + ) internal view returns (LiquidationValues memory singleLiquidation) { singleLiquidation.entireTroveDebt = _entireTroveDebt; singleLiquidation.entireTroveColl = _entireTroveColl; singleLiquidation.recordedTroveDebt = _recordedTroveDebt; singleLiquidation.weightedRecordedTroveDebt = _weightedRecordedTroveDebt; + // TODO: We don’t bother updating this because we are removing RM uint256 cappedCollPortion = _entireTroveDebt * MCR / _price; singleLiquidation.collGasCompensation = _getCollGasCompensation(cappedCollPortion); @@ -575,7 +643,10 @@ contract TroveManager is ERC721, LiquityBase, Ownable, ITroveManager { ); // Move liquidated ETH and Bold to the appropriate pools - stabilityPoolCached.offset(totals.totalDebtToOffset, totals.totalCollToSendToSP); + if (totals.totalDebtToOffset > 0 || totals.totalCollToSendToSP > 0) { + stabilityPoolCached.offset(totals.totalDebtToOffset, totals.totalCollToSendToSP); + } + // we check amount is not zero inside _redistributeDebtAndColl( activePoolCached, defaultPoolCached, totals.totalDebtToRedistribute, totals.totalCollToRedistribute ); @@ -646,7 +717,7 @@ contract TroveManager is ERC721, LiquityBase, Ownable, ITroveManager { !_checkPotentialRecoveryMode(vars.entireSystemColl, vars.entireSystemDebt, _price); } else if (vars.backToNormalMode && vars.ICR < MCR) { singleLiquidation = - _liquidateNormalMode(_activePool, _defaultPool, vars.troveId, vars.remainingBoldInStabPool); + _liquidateNormalMode(_activePool, _defaultPool, vars.troveId, vars.remainingBoldInStabPool, _price); vars.remainingBoldInStabPool = vars.remainingBoldInStabPool - singleLiquidation.debtToOffset; // Add liquidation values to their respective running totals @@ -675,7 +746,7 @@ contract TroveManager is ERC721, LiquityBase, Ownable, ITroveManager { if (vars.ICR < MCR) { singleLiquidation = - _liquidateNormalMode(_activePool, _defaultPool, vars.troveId, vars.remainingBoldInStabPool); + _liquidateNormalMode(_activePool, _defaultPool, vars.troveId, vars.remainingBoldInStabPool, _price); vars.remainingBoldInStabPool = vars.remainingBoldInStabPool - singleLiquidation.debtToOffset; // Add liquidation values to their respective running totals @@ -772,8 +843,8 @@ contract TroveManager is ERC721, LiquityBase, Ownable, ITroveManager { if (_getNetDebt(singleRedemption.newRecordedTroveDebt) < MIN_NET_DEBT) { Troves[_troveId].status = Status.unredeemable; sortedTroves.remove(_troveId); - // TODO: should we also remove from the Troves array? Seems unneccessary as it's only used for off-chain hints. - // We save borrowers gas by not removing + // TODO: should we also remove from the Troves array? Seems unneccessary as it's only used for off-chain hints. + // We save borrowers gas by not removing } Troves[_troveId].debt = singleRedemption.newRecordedTroveDebt; Troves[_troveId].coll = newColl; @@ -1233,7 +1304,7 @@ contract TroveManager is ERC721, LiquityBase, Ownable, ITroveManager { return TCR < CCR; } - + function checkTroveIsOpen(uint256 _troveId) public view returns (bool) { Status status = Troves[_troveId].status; return status == Status.active || status == Status.unredeemable; diff --git a/contracts/src/deployment.sol b/contracts/src/deployment.sol index df7a6515..354c2230 100644 --- a/contracts/src/deployment.sol +++ b/contracts/src/deployment.sol @@ -94,14 +94,14 @@ function _deployAndConnectCollateralContracts(IERC20 _collateralToken, IBoldToke // Deploy all contracts contracts.activePool = new ActivePool(address(_collateralToken)); - contracts.borrowerOperations = new BorrowerOperations(address(_collateralToken)); + contracts.troveManager = new TroveManager(110e16, 5e16, 10e16); + contracts.borrowerOperations = new BorrowerOperations(_collateralToken, contracts.troveManager); contracts.collSurplusPool = new CollSurplusPool(address(_collateralToken)); contracts.defaultPool = new DefaultPool(address(_collateralToken)); contracts.gasPool = new GasPool(); contracts.priceFeed = new PriceFeedTestnet(); contracts.sortedTroves = new SortedTroves(); contracts.stabilityPool = new StabilityPool(address(_collateralToken)); - contracts.troveManager = new TroveManager(); contracts.interestRouter = new MockInterestRouter(); _boldToken.setBranchAddresses( @@ -129,7 +129,6 @@ function _deployAndConnectCollateralContracts(IERC20 _collateralToken, IBoldToke // set contracts in BorrowerOperations contracts.borrowerOperations.setAddresses( - address(contracts.troveManager), address(contracts.activePool), address(contracts.defaultPool), address(contracts.stabilityPool), diff --git a/contracts/src/test/TestContracts/BaseTest.sol b/contracts/src/test/TestContracts/BaseTest.sol index 26ba8a76..55cf27b7 100644 --- a/contracts/src/test/TestContracts/BaseTest.sol +++ b/contracts/src/test/TestContracts/BaseTest.sol @@ -35,7 +35,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 MCR; uint256 CCR = 150e16; address public constant ZERO_ADDRESS = address(0); diff --git a/contracts/src/test/TestContracts/BorrowerOperationsTester.sol b/contracts/src/test/TestContracts/BorrowerOperationsTester.sol index b3e02581..a9b8b01e 100644 --- a/contracts/src/test/TestContracts/BorrowerOperationsTester.sol +++ b/contracts/src/test/TestContracts/BorrowerOperationsTester.sol @@ -7,7 +7,7 @@ import "../../BorrowerOperations.sol"; /* Tester contract inherits from BorrowerOperations, and provides external functions for testing the parent's internal functions. */ contract BorrowerOperationsTester is BorrowerOperations { - constructor(address _ETHAddress) BorrowerOperations(_ETHAddress) {} + constructor(IERC20 _ETH, ITroveManager _troveManager) BorrowerOperations(_ETH, _troveManager) {} function getNewICRFromTroveChange( uint256 _coll, diff --git a/contracts/src/test/TestContracts/CollateralRegistryTester.sol b/contracts/src/test/TestContracts/CollateralRegistryTester.sol index a69242e5..9568b7c6 100644 --- a/contracts/src/test/TestContracts/CollateralRegistryTester.sol +++ b/contracts/src/test/TestContracts/CollateralRegistryTester.sol @@ -8,7 +8,9 @@ import "../../CollateralRegistry.sol"; for testing the parent's internal functions. */ contract CollateralRegistryTester is CollateralRegistry { - constructor(IBoldToken _boldToken, IERC20[] memory _tokens, ITroveManager[] memory _troveManagers) CollateralRegistry(_boldToken, _tokens, _troveManagers) {} + constructor(IBoldToken _boldToken, IERC20[] memory _tokens, ITroveManager[] memory _troveManagers) + CollateralRegistry(_boldToken, _tokens, _troveManagers) + {} function unprotectedDecayBaseRateFromBorrowing() external returns (uint256) { baseRate = _calcDecayedBaseRate(); diff --git a/contracts/src/test/TestContracts/DevTestSetup.sol b/contracts/src/test/TestContracts/DevTestSetup.sol index 29b3522f..e73a2481 100644 --- a/contracts/src/test/TestContracts/DevTestSetup.sol +++ b/contracts/src/test/TestContracts/DevTestSetup.sol @@ -66,6 +66,8 @@ contract DevTestSetup is BaseTest { troveManager = contracts.troveManager; mockInterestRouter = contracts.interestRouter; + MCR = troveManager.MCR(); + // Give some ETH to test accounts, and approve it to BorrowerOperations uint256 initialETHAmount = 1000_000e18; for (uint256 i = 0; i < 6; i++) { diff --git a/contracts/src/test/TestContracts/TroveManagerTester.sol b/contracts/src/test/TestContracts/TroveManagerTester.sol index ec4388d0..a12ebc28 100644 --- a/contracts/src/test/TestContracts/TroveManagerTester.sol +++ b/contracts/src/test/TestContracts/TroveManagerTester.sol @@ -8,6 +8,10 @@ import "../../TroveManager.sol"; for testing the parent's internal functions. */ contract TroveManagerTester is TroveManager { + constructor(uint256 _mcr, uint256 _liquidationPenaltySP, uint256 _liquidationPenaltyRedistribution) + TroveManager(_mcr, _liquidationPenaltySP, _liquidationPenaltyRedistribution) + {} + function computeICR(uint256 _coll, uint256 _debt, uint256 _price) external pure returns (uint256) { return LiquityMath._computeCR(_coll, _debt, _price); } diff --git a/contracts/src/test/liquidations.t.sol b/contracts/src/test/liquidations.t.sol new file mode 100644 index 00000000..9d607dec --- /dev/null +++ b/contracts/src/test/liquidations.t.sol @@ -0,0 +1,284 @@ +pragma solidity ^0.8.18; + +import "./TestContracts/DevTestSetup.sol"; + +contract LiquidationsTest is DevTestSetup { + function testLiquidationOffsetWithSurplus() public { + uint256 liquidationAmount = 2000e18; + uint256 collAmount = 2e18; + + priceFeed.setPrice(2000e18); + vm.startPrank(A); + uint256 ATroveId = borrowerOperations.openTrove( + A, 0, 1e18, collAmount, liquidationAmount - troveManager.BOLD_GAS_COMPENSATION(), 0, 0, 0 + ); + vm.stopPrank(); + + vm.startPrank(B); + borrowerOperations.openTrove(B, 0, 1e18, 2 * collAmount, liquidationAmount, 0, 0, 0); + // B deposits to SP + stabilityPool.provideToSP(liquidationAmount); + + // Price drops + priceFeed.setPrice(1100e18 - 1); + uint256 price = priceFeed.fetchPrice(); + + uint256 initialSPBoldBalance = stabilityPool.getTotalBoldDeposits(); + uint256 initialSPETHBalance = stabilityPool.getETHBalance(); + uint256 AInitialETHBalance = WETH.balanceOf(A); + + // Check not RM + assertEq(troveManager.checkRecoveryMode(price), false, "System should not be in Recovery Mode"); + + // Check CR_A < MCR and TCR > CCR + assertLt(troveManager.getCurrentICR(ATroveId, price), MCR); + assertGt(troveManager.getTCR(price), CCR); + + uint256 trovesCount = troveManager.getTroveIdsCount(); + assertEq(trovesCount, 2); + + troveManager.liquidate(ATroveId); + + // Check Troves count reduced by 1 + trovesCount = troveManager.getTroveIdsCount(); + assertEq(trovesCount, 1); + + // Check SP Bold has decreased + uint256 finalSPBoldBalance = stabilityPool.getTotalBoldDeposits(); + assertEq(initialSPBoldBalance - finalSPBoldBalance, liquidationAmount, "SP Bold balance mismatch"); + // Check SP ETH has increased + uint256 finalSPETHBalance = stabilityPool.getETHBalance(); + // liquidationAmount to ETH + 5% + assertApproxEqAbs( + finalSPETHBalance - initialSPETHBalance, + liquidationAmount * DECIMAL_PRECISION / price * 105 / 100, + 10, + "SP ETH balance mismatch" + ); + + // Check A retains ~4.5% of the collateral (after claiming from CollSurplus) + // collAmount - 0.5% - (liquidationAmount to ETH + 5%) + uint256 collSurplusAmount = collAmount * 995 / 1000 - liquidationAmount * DECIMAL_PRECISION / price * 105 / 100; + assertEq( + WETH.balanceOf(address(collSurplusPool)), + collSurplusAmount, + "CollSurplusPoll should have received collateral" + ); + vm.startPrank(A); + borrowerOperations.claimCollateral(ATroveId); + vm.stopPrank(); + assertEq(WETH.balanceOf(A) - AInitialETHBalance, collSurplusAmount, "A collateral balance mismatch"); + } + + function testLiquidationOffsetNoSurplus() public { + uint256 liquidationAmount = 10000e18; + uint256 collAmount = 10e18; + + priceFeed.setPrice(2000e18); + vm.startPrank(A); + uint256 ATroveId = borrowerOperations.openTrove( + A, 0, 1e18, collAmount, liquidationAmount - troveManager.BOLD_GAS_COMPENSATION(), 0, 0, 0 + ); + vm.stopPrank(); + + vm.startPrank(B); + borrowerOperations.openTrove(B, 0, 1e18, 3 * collAmount, liquidationAmount, 0, 0, 0); + // B deposits to SP + stabilityPool.provideToSP(liquidationAmount); + + // Price drops + priceFeed.setPrice(1030e18); + uint256 price = priceFeed.fetchPrice(); + + uint256 initialSPBoldBalance = stabilityPool.getTotalBoldDeposits(); + uint256 initialSPETHBalance = stabilityPool.getETHBalance(); + + // Check not RM + assertEq(troveManager.checkRecoveryMode(price), false, "System should not be in Recovery Mode"); + + // Check CR_A < MCR and TCR > CCR + assertLt(troveManager.getCurrentICR(ATroveId, price), MCR, "ICR too high"); + assertGe(troveManager.getTCR(price), CCR, "TCR too low"); + + uint256 trovesCount = troveManager.getTroveIdsCount(); + assertEq(trovesCount, 2); + + troveManager.liquidate(ATroveId); + + // Check Troves count reduced by 1 + trovesCount = troveManager.getTroveIdsCount(); + assertEq(trovesCount, 1); + + // Check SP Bold has decreased + uint256 finalSPBoldBalance = stabilityPool.getTotalBoldDeposits(); + assertEq(initialSPBoldBalance - finalSPBoldBalance, liquidationAmount, "SP Bold balance mismatch"); + // Check SP ETH has increased by coll minus coll gas comp + uint256 finalSPETHBalance = stabilityPool.getETHBalance(); + // liquidationAmount to ETH + 5% + assertApproxEqAbs( + finalSPETHBalance - initialSPETHBalance, collAmount * 995 / 1000, 10, "SP ETH balance mismatch" + ); + + // Check there’s no surplus + assertEq(WETH.balanceOf(address(collSurplusPool)), 0, "CollSurplusPoll should be empty"); + + vm.startPrank(A); + vm.expectRevert("CollSurplusPool: No collateral available to claim"); + borrowerOperations.claimCollateral(); + vm.stopPrank(); + } + + function testLiquidationRedistributionNoSurplus() public { + uint256 liquidationAmount = 2000e18; + uint256 collAmount = 2e18; + + priceFeed.setPrice(2000e18); + vm.startPrank(A); + uint256 ATroveId = borrowerOperations.openTrove( + A, 0, 1e18, collAmount, liquidationAmount - troveManager.BOLD_GAS_COMPENSATION(), 0, 0, 0 + ); + vm.stopPrank(); + + vm.startPrank(B); + uint256 BTroveId = borrowerOperations.openTrove(B, 0, 1e18, 2 * collAmount, liquidationAmount, 0, 0, 0); + + // Price drops + priceFeed.setPrice(1100e18 - 1); + uint256 price = priceFeed.fetchPrice(); + + uint256 BInitialDebt = troveManager.getTroveEntireDebt(BTroveId); + uint256 BInitialColl = troveManager.getTroveEntireColl(BTroveId); + + // Check not RM + assertEq(troveManager.checkRecoveryMode(price), false, "System should not be in Recovery Mode"); + + // Check CR_A < MCR and TCR > CCR + assertLt(troveManager.getCurrentICR(ATroveId, price), MCR); + assertGt(troveManager.getTCR(price), CCR); + + // Check empty SP + assertEq(stabilityPool.getTotalBoldDeposits(), 0, "SP should be empty"); + + uint256 trovesCount = troveManager.getTroveIdsCount(); + assertEq(trovesCount, 2); + + troveManager.liquidate(ATroveId); + + // Check Troves count reduced by 1 + trovesCount = troveManager.getTroveIdsCount(); + assertEq(trovesCount, 1); + + // Check SP stays the same + assertEq(stabilityPool.getTotalBoldDeposits(), 0, "SP should be empty"); + assertEq(stabilityPool.getETHBalance(), 0, "SP should not have ETH rewards"); + + // Check B has received debt + assertEq(troveManager.getTroveEntireDebt(BTroveId) - BInitialDebt, liquidationAmount, "B debt mismatch"); + // Check B has received all coll minus coll gas comp + assertApproxEqAbs( + troveManager.getTroveEntireColl(BTroveId) - BInitialColl, + collAmount * 995 / 1000, // Collateral - coll gas comp + 10, + "B trove coll mismatch" + ); + + assertEq(WETH.balanceOf(address(collSurplusPool)), 0, "CollSurplusPoll should be empty"); + } + + struct InitialValues { + uint256 spBoldBalance; + uint256 spETHBalance; + uint256 AETHBalance; + uint256 BDebt; + uint256 BColl; + } + + // Offset and Redistribution + function testLiquidationMix() public { + uint256 liquidationAmount = 2000e18; + uint256 collAmount = 2e18; + + priceFeed.setPrice(2000e18); + vm.startPrank(A); + uint256 ATroveId = borrowerOperations.openTrove( + A, 0, 1e18, collAmount, liquidationAmount - troveManager.BOLD_GAS_COMPENSATION(), 0, 0, 0 + ); + vm.stopPrank(); + + vm.startPrank(B); + uint256 BTroveId = borrowerOperations.openTrove(B, 0, 1e18, 2 * collAmount, liquidationAmount, 0, 0, 0); + // B deposits to SP + stabilityPool.provideToSP(liquidationAmount / 2); + + // Price drops + priceFeed.setPrice(1100e18 - 1); + uint256 price = priceFeed.fetchPrice(); + + InitialValues memory initialValues; + initialValues.spBoldBalance = stabilityPool.getTotalBoldDeposits(); + initialValues.spETHBalance = stabilityPool.getETHBalance(); + initialValues.AETHBalance = WETH.balanceOf(A); + initialValues.BDebt = troveManager.getTroveEntireDebt(BTroveId); + initialValues.BColl = troveManager.getTroveEntireColl(BTroveId); + + // Check not RM + assertEq(troveManager.checkRecoveryMode(price), false, "System should not be in Recovery Mode"); + + // Check CR_A < MCR and TCR > CCR + assertLt(troveManager.getCurrentICR(ATroveId, price), MCR); + assertGt(troveManager.getTCR(price), CCR); + + uint256 trovesCount = troveManager.getTroveIdsCount(); + assertEq(trovesCount, 2); + + troveManager.liquidate(ATroveId); + + // Check Troves count reduced by 1 + trovesCount = troveManager.getTroveIdsCount(); + assertEq(trovesCount, 1); + + // Check SP Bold has decreased + uint256 finalSPBoldBalance = stabilityPool.getTotalBoldDeposits(); + assertEq(initialValues.spBoldBalance - finalSPBoldBalance, liquidationAmount / 2, "SP Bold balance mismatch"); + // Check SP ETH has increased + uint256 finalSPETHBalance = stabilityPool.getETHBalance(); + // liquidationAmount to ETH + 5% + assertApproxEqAbs( + finalSPETHBalance - initialValues.spETHBalance, + liquidationAmount / 2 * DECIMAL_PRECISION / price * 105 / 100, + 10, + "SP ETH balance mismatch" + ); + + // Check B has received debt + assertEq( + troveManager.getTroveEntireDebt(BTroveId) - initialValues.BDebt, liquidationAmount / 2, "B debt mismatch" + ); + // Check B has received coll + assertApproxEqAbs( + troveManager.getTroveEntireColl(BTroveId) - initialValues.BColl, + //collAmount * 995 / 1000 - liquidationAmount / 2 * DECIMAL_PRECISION / price * 105 / 100, + liquidationAmount / 2 * DECIMAL_PRECISION / price * 110 / 100, + 10, + "B trove coll mismatch" + ); + + // Check A retains ~4.5% of the collateral (after claiming from CollSurplus) + // collAmount - 0.5% - (liquidationAmount to ETH + 5%) + uint256 collSurplusAmount = collAmount * 995 / 1000 + - liquidationAmount / 2 * DECIMAL_PRECISION / price * 105 / 100 + - liquidationAmount / 2 * DECIMAL_PRECISION / price * 110 / 100; + assertApproxEqAbs( + WETH.balanceOf(address(collSurplusPool)), + collSurplusAmount, + 10, + "CollSurplusPoll should have received collateral" + ); + vm.startPrank(A); + borrowerOperations.claimCollateral(); + vm.stopPrank(); + assertApproxEqAbs( + WETH.balanceOf(A) - initialValues.AETHBalance, collSurplusAmount, 10, "A collateral balance mismatch" + ); + } +} diff --git a/contracts/src/test/liquidationsLST.t.sol b/contracts/src/test/liquidationsLST.t.sol new file mode 100644 index 00000000..5e95d0a9 --- /dev/null +++ b/contracts/src/test/liquidationsLST.t.sol @@ -0,0 +1,250 @@ +pragma solidity ^0.8.18; + +import "./TestContracts/DevTestSetup.sol"; + +contract LiquidationsLSTTest is DevTestSetup { + function setUp() public override { + // Start tests at a non-zero timestamp + vm.warp(block.timestamp + 600); + + accounts = new Accounts(); + createAccounts(); + + (A, B, C, D, E, F, G) = ( + accountsList[0], + accountsList[1], + accountsList[2], + accountsList[3], + accountsList[4], + accountsList[5], + accountsList[6] + ); + + LiquityContracts memory contracts; + (contracts, collateralRegistry, boldToken) = _deployAndConnectContracts(TroveManagerParams(120e16, 5e16, 10e16)); + WETH = contracts.WETH; + activePool = contracts.activePool; + borrowerOperations = contracts.borrowerOperations; + collSurplusPool = contracts.collSurplusPool; + defaultPool = contracts.defaultPool; + gasPool = contracts.gasPool; + priceFeed = contracts.priceFeed; + sortedTroves = contracts.sortedTroves; + stabilityPool = contracts.stabilityPool; + troveManager = contracts.troveManager; + mockInterestRouter = contracts.interestRouter; + + MCR = troveManager.MCR(); + + // Give some ETH to test accounts, and approve it to BorrowerOperations + uint256 initialETHAmount = 10_000e18; + for (uint256 i = 0; i < 6; i++) { + // A to F + giveAndApproveETH(accountsList[i], initialETHAmount); + } + } + + function testLiquidationRedistributionWithSurplus() public { + uint256 liquidationAmount = 2000e18; + uint256 collAmount = 2e18; + + priceFeed.setPrice(2000e18); + vm.startPrank(A); + uint256 ATroveId = borrowerOperations.openTrove( + A, 0, 1e18, collAmount, liquidationAmount - troveManager.BOLD_GAS_COMPENSATION(), 0, 0, 0 + ); + vm.stopPrank(); + + vm.startPrank(B); + uint256 BTroveId = borrowerOperations.openTrove(B, 0, 1e18, 2 * collAmount, liquidationAmount, 0, 0, 0); + + // Price drops + priceFeed.setPrice(1200e18 - 1); + uint256 price = priceFeed.fetchPrice(); + + uint256 BInitialDebt = troveManager.getTroveEntireDebt(BTroveId); + uint256 BInitialColl = troveManager.getTroveEntireColl(BTroveId); + uint256 AInitialETHBalance = WETH.balanceOf(A); + + // Check not RM + assertEq(troveManager.checkRecoveryMode(price), false, "System should not be in Recovery Mode"); + + // Check CR_A < MCR and TCR > CCR + assertLt(troveManager.getCurrentICR(ATroveId, price), MCR); + assertGt(troveManager.getTCR(price), CCR); + + uint256 trovesCount = troveManager.getTroveIdsCount(); + assertEq(trovesCount, 2); + + troveManager.liquidate(ATroveId); + + // Check Troves count reduced by 1 + trovesCount = troveManager.getTroveIdsCount(); + assertEq(trovesCount, 1); + + // Check SP stays the same + assertEq(stabilityPool.getTotalBoldDeposits(), 0, "SP should be empty"); + assertEq(stabilityPool.getETHBalance(), 0, "SP should not have ETH rewards"); + + // Check B has received debt + assertEq(troveManager.getTroveEntireDebt(BTroveId) - BInitialDebt, liquidationAmount, "B debt mismatch"); + // Check B has received all coll minus coll gas comp + assertApproxEqAbs( + troveManager.getTroveEntireColl(BTroveId) - BInitialColl, + LiquityMath._min( + collAmount * 995 / 1000, // Collateral - coll gas comp + liquidationAmount * DECIMAL_PRECISION / price * 110 / 100 // debt with penalty + ), + 10, + "B trove coll mismatch" + ); + + // Check A retains ~9.5% of the collateral (after claiming from CollSurplus) + // collAmount - 0.5% - (liquidationAmount to ETH + 10%) + uint256 collSurplusAmount = collAmount * 995 / 1000 - liquidationAmount * DECIMAL_PRECISION / price * 110 / 100; + assertApproxEqAbs( + WETH.balanceOf(address(collSurplusPool)), + collSurplusAmount, + 10, + "CollSurplusPoll should have received collateral" + ); + vm.startPrank(A); + borrowerOperations.claimCollateral(); + vm.stopPrank(); + assertApproxEqAbs( + WETH.balanceOf(A) - AInitialETHBalance, collSurplusAmount, 10, "A collateral balance mismatch" + ); + } + + struct InitialValues { + uint256 spBoldBalance; + uint256 spETHBalance; + uint256 AETHBalance; + uint256 BDebt; + uint256 BColl; + } + + struct FinalValues { + uint256 spBoldBalance; + uint256 spETHBalance; + uint256 collToLiquidate; + uint256 collSPPortion; + uint256 collPenaltySP; + uint256 collToSendToSP; + uint256 collRedistributionPortion; + uint256 collPenaltyRedistribution; + } + + function testLiquidationFuzz(uint256 _finalPrice, uint256 _spAmount) public { + uint256 liquidationAmount = 2000e18; + uint256 collAmount = 2e18; + uint256 initialPrice = 2000e18; + // A initial CR: 200% + + _finalPrice = bound(_finalPrice, 1000e18, 1200e18 - 1); // A final CR in [100%, 120%[ + _spAmount = bound(_spAmount, 0, liquidationAmount); + + priceFeed.setPrice(initialPrice); + vm.startPrank(A); + uint256 ATroveId = borrowerOperations.openTrove( + A, 0, 1e18, collAmount, liquidationAmount - troveManager.BOLD_GAS_COMPENSATION(), 0, 0, 0 + ); + vm.stopPrank(); + + vm.startPrank(B); + uint256 BTroveId = borrowerOperations.openTrove(B, 0, 1e18, 3 * collAmount, liquidationAmount, 0, 0, 0); + // B deposits to SP + if (_spAmount > 0) { + stabilityPool.provideToSP(_spAmount); + } + + // Price drops + priceFeed.setPrice(_finalPrice); + console2.log(_finalPrice, "_finalPrice"); + + InitialValues memory initialValues; + initialValues.spBoldBalance = stabilityPool.getTotalBoldDeposits(); + initialValues.spETHBalance = stabilityPool.getETHBalance(); + initialValues.AETHBalance = WETH.balanceOf(A); + initialValues.BDebt = troveManager.getTroveEntireDebt(BTroveId); + initialValues.BColl = troveManager.getTroveEntireColl(BTroveId); + + // Check not RM + assertEq(troveManager.checkRecoveryMode(_finalPrice), false, "System should not be in Recovery Mode"); + + // Check CR_A < MCR and TCR > CCR + assertLt(troveManager.getCurrentICR(ATroveId, _finalPrice), MCR); + assertGt(troveManager.getTCR(_finalPrice), CCR); + + uint256 trovesCount = troveManager.getTroveIdsCount(); + assertEq(trovesCount, 2); + + troveManager.liquidate(ATroveId); + + // Check Troves count reduced by 1 + trovesCount = troveManager.getTroveIdsCount(); + assertEq(trovesCount, 1); + + // Offset part + FinalValues memory finalValues; + finalValues.collToLiquidate = collAmount * 995 / 1000; + // Check SP Bold has decreased + finalValues.spBoldBalance = stabilityPool.getTotalBoldDeposits(); + assertEq(initialValues.spBoldBalance - finalValues.spBoldBalance, _spAmount, "SP Bold balance mismatch"); + // Check SP ETH has increased + finalValues.spETHBalance = stabilityPool.getETHBalance(); + finalValues.collSPPortion = finalValues.collToLiquidate * _spAmount / liquidationAmount; + finalValues.collPenaltySP = _spAmount * DECIMAL_PRECISION / _finalPrice * 105 / 100; + finalValues.collToSendToSP = LiquityMath._min(finalValues.collPenaltySP, finalValues.collSPPortion); + // liquidationAmount to ETH + 5% + assertApproxEqAbs( + finalValues.spETHBalance - initialValues.spETHBalance, + finalValues.collToSendToSP, + 10, + "SP ETH balance mismatch" + ); + + // Redistribution part + finalValues.collRedistributionPortion = finalValues.collToLiquidate - finalValues.collSPPortion; + finalValues.collPenaltyRedistribution = + (liquidationAmount - _spAmount) * DECIMAL_PRECISION / _finalPrice * 110 / 100; + // Check B has received debt + assertApproxEqAbs( + troveManager.getTroveEntireDebt(BTroveId) - initialValues.BDebt, + liquidationAmount - _spAmount, + 10, + "B debt mismatch" + ); + // Check B has received coll + assertApproxEqAbs( + troveManager.getTroveEntireColl(BTroveId) - initialValues.BColl, + LiquityMath._min( + finalValues.collPenaltyRedistribution, + finalValues.collRedistributionPortion + finalValues.collSPPortion - finalValues.collToSendToSP + ), + 10, + "B trove coll mismatch" + ); + + // Surplus + // Check A retains part of the collateral (after claiming from CollSurplus) + // collAmount - 0.5% - (liquidationAmount to ETH + penalty) + uint256 collPenalty = finalValues.collPenaltySP + finalValues.collPenaltyRedistribution; + console2.log(finalValues.collPenaltySP, "finalValues.collPenaltySP"); + console2.log(finalValues.collPenaltyRedistribution, "finalValues.collPenaltyRedistribution"); + console2.log(collPenalty, "collPenalty"); + uint256 collSurplusAmount; + if (collPenalty < finalValues.collToLiquidate) { + collSurplusAmount = finalValues.collToLiquidate - collPenalty; + } + assertApproxEqAbs(WETH.balanceOf(address(collSurplusPool)), collSurplusAmount, 1e9, "CollSurplusPoll mismatch"); + if (collSurplusAmount > 0) { + vm.startPrank(A); + borrowerOperations.claimCollateral(); + vm.stopPrank(); + assertApproxEqAbs( + WETH.balanceOf(A) - initialValues.AETHBalance, collSurplusAmount, 1e9, "A collateral balance mismatch" + ); + } + } +} diff --git a/contracts/src/test/redemptions.t.sol b/contracts/src/test/redemptions.t.sol index bc8e5732..6acdb972 100644 --- a/contracts/src/test/redemptions.t.sol +++ b/contracts/src/test/redemptions.t.sol @@ -189,13 +189,13 @@ contract Redemptions is DevTestSetup { redeem(E, redeemAmount); // Check B's debt unchanged from redeemAmount < debt_B; - assertEq(debt_B, troveManager.getTroveEntireDebt(troveIDs.B)); + assertEq(debt_B, troveManager.getTroveEntireDebt(troveIDs.B)); redeemAmount = debt_B + 1; redeem(E, redeemAmount); // Check B's debt unchanged from redeemAmount > debt_B; - assertEq(debt_B, troveManager.getTroveEntireDebt(troveIDs.B)); + assertEq(debt_B, troveManager.getTroveEntireDebt(troveIDs.B)); } function testZombieTrovesCanReceiveRedistGains() public { @@ -268,13 +268,13 @@ contract Redemptions is DevTestSetup { _redeemAndCreateZombieTrovesAAndB(troveIDs); - assertLt(troveManager.getTroveEntireDebt(troveIDs.A), troveManager.MIN_NET_DEBT()); + assertLt(troveManager.getTroveEntireDebt(troveIDs.A), troveManager.MIN_NET_DEBT()); assertLt(troveManager.getTroveEntireDebt(troveIDs.B), troveManager.MIN_NET_DEBT()); // 100 years passes vm.warp(block.timestamp + 36500 days); - assertGt(troveManager.getTroveEntireDebt(troveIDs.A), troveManager.MIN_NET_DEBT()); + assertGt(troveManager.getTroveEntireDebt(troveIDs.A), troveManager.MIN_NET_DEBT()); assertGt(troveManager.getTroveEntireDebt(troveIDs.B), troveManager.MIN_NET_DEBT()); } @@ -301,7 +301,7 @@ contract Redemptions is DevTestSetup { // assertFalse(troveManager.checkRecoveryMode(price)); assertLt(troveManager.getCurrentICR(troveID_E, price), troveManager.MCR()); - assertLt(troveManager.getTroveEntireDebt(troveIDs.A), troveManager.MIN_NET_DEBT()); + assertLt(troveManager.getTroveEntireDebt(troveIDs.A), troveManager.MIN_NET_DEBT()); assertLt(troveManager.getTroveEntireDebt(troveIDs.B), troveManager.MIN_NET_DEBT()); // A liquidates E @@ -329,8 +329,8 @@ contract Redemptions is DevTestSetup { assertEq(troveManager.getTroveStatus(troveIDs.A), 5); // Status 5 - 'unredeemable' assertEq(troveManager.getTroveStatus(troveIDs.B), 5); // Status 5 - 'unredeemable' - closeTrove(A, troveIDs.A); - closeTrove(B, troveIDs.B); + closeTrove(A, troveIDs.A); + closeTrove(B, troveIDs.B); assertEq(troveManager.getTroveStatus(troveIDs.A), 2); // Status 2 - 'closed by owner' assertEq(troveManager.getTroveStatus(troveIDs.B), 2); // Status 2 - 'closed by owner' @@ -702,7 +702,7 @@ contract Redemptions is DevTestSetup { vm.expectRevert("BorrowerOps: Trove does not have active status"); borrowerOperations.adjustTroveInterestRate(troveIDs.B, newInterestRate, troveIDs.B, troveIDs.B); vm.stopPrank(); - } + } function testZombieTroveAccruedInterestCanBePermissionlesslyApplied() public { ( , , TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); @@ -723,7 +723,7 @@ contract Redemptions is DevTestSetup { assertEq(troveManager.calcTroveAccruedInterest(troveIDs.A), 0); assertEq(troveManager.calcTroveAccruedInterest(troveIDs.B), 0); - } + } function testZombieTroveCanBeLiquidated() public { ( , , TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); @@ -745,13 +745,13 @@ contract Redemptions is DevTestSetup { // assertFalse(troveManager.checkRecoveryMode(price)); assertLt(troveManager.getCurrentICR(troveIDs.B, price), troveManager.MCR()); - assertEq(troveManager.getTroveStatus(troveIDs.B), 5); + assertEq(troveManager.getTroveStatus(troveIDs.B), 5); // E liquidates B liquidate(E, troveIDs.B); assertEq(troveManager.getTroveStatus(troveIDs.B), 3); // Status 3 - closed by liquidation - } + } // TODO: tests borrower for combined adjustments - debt changes and coll add/withdrawals. - // Borrower should only be able to close OR leave Trove at >= min net debt. + // Borrower should only be able to close OR leave Trove at >= min net debt. } From 32c376691b66c46f4e4c171766d6c328286c8504 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Fingen?= Date: Wed, 1 May 2024 13:26:22 +0100 Subject: [PATCH 03/13] fix: Switch CollSurplusPool to be address based again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before it was impossible to claim, as NFT wouldn’t exist, and therefore we couldn’t check its owner. --- contracts/src/BorrowerOperations.sol | 6 ++--- contracts/src/CollSurplusPool.sol | 24 +++++++++---------- .../src/Interfaces/IBorrowerOperations.sol | 6 ++--- contracts/src/Interfaces/ICollSurplusPool.sol | 6 ++--- contracts/src/TroveManager.sol | 12 +++++++--- contracts/src/test/liquidations.t.sol | 2 +- 6 files changed, 30 insertions(+), 26 deletions(-) diff --git a/contracts/src/BorrowerOperations.sol b/contracts/src/BorrowerOperations.sol index ba5f462d..7c5fac10 100644 --- a/contracts/src/BorrowerOperations.sol +++ b/contracts/src/BorrowerOperations.sol @@ -551,11 +551,9 @@ 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 { - _requireIsOwner(_troveId); - + function claimCollateral() external override { // send ETH from CollSurplus Pool to owner - collSurplusPool.claimColl(msg.sender, _troveId); + collSurplusPool.claimColl(msg.sender); } // --- Helper functions --- diff --git a/contracts/src/CollSurplusPool.sol b/contracts/src/CollSurplusPool.sol index 9c9b246c..8d3c763f 100644 --- a/contracts/src/CollSurplusPool.sol +++ b/contracts/src/CollSurplusPool.sol @@ -20,7 +20,7 @@ contract CollSurplusPool is Ownable, ICollSurplusPool { // deposited ether tracker uint256 internal ETHBalance; // Collateral surplus claimable by trove owners - mapping(uint256 => uint256) internal balances; + mapping(address => uint256) internal balances; // --- Events --- @@ -28,7 +28,7 @@ contract CollSurplusPool is Ownable, ICollSurplusPool { event TroveManagerAddressChanged(address _newTroveManagerAddress); event ActivePoolAddressChanged(address _newActivePoolAddress); - event CollBalanceUpdated(uint256 indexed _troveId, uint256 _newBalance); + event CollBalanceUpdated(address indexed _account, uint256 _newBalance); event EtherSent(address _to, uint256 _amount); constructor(address _ETHAddress) { @@ -59,29 +59,29 @@ contract CollSurplusPool is Ownable, ICollSurplusPool { return ETHBalance; } - function getCollateral(uint256 _troveId) external view override returns (uint256) { - return balances[_troveId]; + function getCollateral(address _account) external view override returns (uint256) { + return balances[_account]; } // --- Pool functionality --- - function accountSurplus(uint256 _troveId, uint256 _amount) external override { + function accountSurplus(address _account, uint256 _amount) external override { _requireCallerIsTroveManager(); - uint256 newAmount = balances[_troveId] + _amount; - balances[_troveId] = newAmount; + uint256 newAmount = balances[_account] + _amount; + balances[_account] = newAmount; ETHBalance = ETHBalance + _amount; - emit CollBalanceUpdated(_troveId, newAmount); + emit CollBalanceUpdated(_account, newAmount); } - function claimColl(address _account, uint256 _troveId) external override { + function claimColl(address _account) external override { _requireCallerIsBorrowerOperations(); - uint256 claimableColl = balances[_troveId]; + uint256 claimableColl = balances[_account]; require(claimableColl > 0, "CollSurplusPool: No collateral available to claim"); - balances[_troveId] = 0; - emit CollBalanceUpdated(_troveId, 0); + balances[_account] = 0; + emit CollBalanceUpdated(_account, 0); ETHBalance = ETHBalance - claimableColl; emit EtherSent(_account, claimableColl); diff --git a/contracts/src/Interfaces/IBorrowerOperations.sol b/contracts/src/Interfaces/IBorrowerOperations.sol index e1929725..75f9ee51 100644 --- a/contracts/src/Interfaces/IBorrowerOperations.sol +++ b/contracts/src/Interfaces/IBorrowerOperations.sol @@ -56,7 +56,7 @@ interface IBorrowerOperations is ILiquityBase { ) external; function adjustUnredeemableTrove( - uint256 _troveId, + uint256 _troveId, uint256 _maxFeePercentage, uint256 _collChange, bool _isCollIncrease, @@ -65,8 +65,8 @@ interface IBorrowerOperations is ILiquityBase { uint256 _upperHint, uint256 _lowerHint ) external; - - function claimCollateral(uint256 _troveId) external; + + function claimCollateral() external; function setAddManager(uint256 _troveId, address _manager) external; function setRemoveManager(uint256 _troveId, address _manager) external; diff --git a/contracts/src/Interfaces/ICollSurplusPool.sol b/contracts/src/Interfaces/ICollSurplusPool.sol index bfb2866c..43a0d058 100644 --- a/contracts/src/Interfaces/ICollSurplusPool.sol +++ b/contracts/src/Interfaces/ICollSurplusPool.sol @@ -8,9 +8,9 @@ interface ICollSurplusPool { function getETHBalance() external view returns (uint256); - function getCollateral(uint256 _troveId) external view returns (uint256); + function getCollateral(address _account) external view returns (uint256); - function accountSurplus(uint256 _troveId, uint256 _amount) external; + function accountSurplus(address _account, uint256 _amount) external; - function claimColl(address _account, uint256 _troveId) external; + function claimColl(address _account) external; } diff --git a/contracts/src/TroveManager.sol b/contracts/src/TroveManager.sol index d277f67a..01d1f620 100644 --- a/contracts/src/TroveManager.sol +++ b/contracts/src/TroveManager.sol @@ -334,6 +334,8 @@ contract TroveManager is ERC721, LiquityBase, Ownable, ITroveManager { uint256 _boldInStabPool, uint256 _price ) internal returns (LiquidationValues memory singleLiquidation) { + address owner = ownerOf(_troveId); + LocalVariables_InnerSingleLiquidateFunction memory vars; ( singleLiquidation.entireTroveDebt, @@ -369,7 +371,7 @@ contract TroveManager is ERC721, LiquityBase, Ownable, ITroveManager { // Differencen between liquidation penalty and liquidation threshold if (singleLiquidation.collSurplus > 0) { - collSurplusPool.accountSurplus(_troveId, singleLiquidation.collSurplus); + collSurplusPool.accountSurplus(owner, singleLiquidation.collSurplus); } emit TroveLiquidated( @@ -433,6 +435,8 @@ contract TroveManager is ERC721, LiquityBase, Ownable, ITroveManager { // If 100% < ICR < MCR, offset as much as possible, and redistribute the remainder } else if ((_ICR > _100pct) && (_ICR < MCR)) { + address owner = ownerOf(_troveId); + _movePendingTroveRewardsToActivePool( _activePool, _defaultPool, singleLiquidation.pendingDebtReward, vars.pendingCollReward ); @@ -452,7 +456,7 @@ contract TroveManager is ERC721, LiquityBase, Ownable, ITroveManager { // Differencen between liquidation penalty and liquidation threshold if (singleLiquidation.collSurplus > 0) { - collSurplusPool.accountSurplus(_troveId, singleLiquidation.collSurplus); + collSurplusPool.accountSurplus(owner, singleLiquidation.collSurplus); } emit TroveLiquidated( @@ -469,6 +473,8 @@ contract TroveManager is ERC721, LiquityBase, Ownable, ITroveManager { * The remainder due to the capped rate will be claimable as collateral surplus. */ } else if ((_ICR >= MCR) && (_ICR < _TCR) && (singleLiquidation.entireTroveDebt <= _boldInStabPool)) { + address owner = ownerOf(_troveId); + _movePendingTroveRewardsToActivePool( _activePool, _defaultPool, singleLiquidation.pendingDebtReward, vars.pendingCollReward ); @@ -485,7 +491,7 @@ contract TroveManager is ERC721, LiquityBase, Ownable, ITroveManager { _closeTrove(_troveId, Status.closedByLiquidation); if (singleLiquidation.collSurplus > 0) { - collSurplusPool.accountSurplus(_troveId, singleLiquidation.collSurplus); + collSurplusPool.accountSurplus(owner, singleLiquidation.collSurplus); } emit TroveLiquidated( diff --git a/contracts/src/test/liquidations.t.sol b/contracts/src/test/liquidations.t.sol index 9d607dec..3f69815c 100644 --- a/contracts/src/test/liquidations.t.sol +++ b/contracts/src/test/liquidations.t.sol @@ -65,7 +65,7 @@ contract LiquidationsTest is DevTestSetup { "CollSurplusPoll should have received collateral" ); vm.startPrank(A); - borrowerOperations.claimCollateral(ATroveId); + borrowerOperations.claimCollateral(); vm.stopPrank(); assertEq(WETH.balanceOf(A) - AInitialETHBalance, collSurplusAmount, "A collateral balance mismatch"); } From c315fdc04c91334a02472e4ce8a4d63c8ed656d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Fingen?= Date: Thu, 2 May 2024 22:02:40 +0100 Subject: [PATCH 04/13] feat: Allow to specify TM params on deployment --- contracts/src/deployment.sol | 48 +++++++++++++++++------- contracts/src/test/multicollateral.t.sol | 10 ++++- 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/contracts/src/deployment.sol b/contracts/src/deployment.sol index 354c2230..74e18453 100644 --- a/contracts/src/deployment.sol +++ b/contracts/src/deployment.sol @@ -34,22 +34,38 @@ struct LiquityContracts { IERC20 WETH; } +struct TroveManagerParams { + uint256 MCR; + uint256 LIQUIDATION_PENALTY_SP; + uint256 LIQUIDATION_PENALTY_REDISTRIBUTION; +} + function _deployAndConnectContracts() returns (LiquityContracts memory contracts, ICollateralRegistry collateralRegistry, IBoldToken boldToken) +{ + return _deployAndConnectContracts(TroveManagerParams(110e16, 5e16, 10e16)); +} + +function _deployAndConnectContracts(TroveManagerParams memory troveManagerParams) + returns (LiquityContracts memory contracts, ICollateralRegistry collateralRegistry, IBoldToken boldToken) { LiquityContracts[] memory contractsArray; - (contractsArray, collateralRegistry, boldToken) = _deployAndConnectContracts(1); + TroveManagerParams[] memory troveManagerParamsArray = new TroveManagerParams[](1); + + troveManagerParamsArray[0] = troveManagerParams; + (contractsArray, collateralRegistry, boldToken) = _deployAndConnectContracts(troveManagerParamsArray); contracts = contractsArray[0]; } -function _deployAndConnectContracts(uint256 _numCollaterals) +function _deployAndConnectContracts(TroveManagerParams[] memory troveManagerParamsArray) returns (LiquityContracts[] memory contractsArray, ICollateralRegistry collateralRegistry, IBoldToken boldToken) { + uint256 numCollaterals = troveManagerParamsArray.length; boldToken = new BoldToken(); - contractsArray = new LiquityContracts[](_numCollaterals); - IERC20[] memory collaterals = new IERC20[](_numCollaterals); - ITroveManager[] memory troveManagers = new ITroveManager[](_numCollaterals); + contractsArray = new LiquityContracts[](numCollaterals); + IERC20[] memory collaterals = new IERC20[](numCollaterals); + ITroveManager[] memory troveManagers = new ITroveManager[](numCollaterals); LiquityContracts memory contracts; IERC20 WETH = new ERC20Faucet( @@ -58,20 +74,20 @@ function _deployAndConnectContracts(uint256 _numCollaterals) 100 ether, // _tapAmount 1 days // _tapPeriod ); - contracts = _deployAndConnectCollateralContracts(WETH, boldToken); + contracts = _deployAndConnectCollateralContracts(WETH, boldToken, troveManagerParamsArray[0]); contractsArray[0] = contracts; collaterals[0] = contracts.WETH; troveManagers[0] = contracts.troveManager; // Multicollateral registry - for (uint256 i = 1; i < _numCollaterals; i++) { + for (uint256 i = 1; i < numCollaterals; i++) { IERC20 stETH = new ERC20Faucet( string.concat("Staked ETH", string(abi.encode(i))), // _name string.concat("stETH", string(abi.encode(i))), // _symbol 100 ether, // _tapAmount 1 days // _tapPeriod ); - contracts = _deployAndConnectCollateralContracts(stETH, boldToken); + contracts = _deployAndConnectCollateralContracts(stETH, boldToken, troveManagerParamsArray[i]); collaterals[i] = contracts.WETH; troveManagers[i] = contracts.troveManager; contractsArray[i] = contracts; @@ -80,21 +96,27 @@ function _deployAndConnectContracts(uint256 _numCollaterals) collateralRegistry = new CollateralRegistry(boldToken, collaterals, troveManagers); boldToken.setCollateralRegistry(address(collateralRegistry)); // Set registry in TroveManagers - for (uint256 i = 0; i < _numCollaterals; i++) { + for (uint256 i = 0; i < numCollaterals; i++) { contractsArray[i].troveManager.setCollateralRegistry(address(collateralRegistry)); } } -function _deployAndConnectCollateralContracts(IERC20 _collateralToken, IBoldToken _boldToken) - returns (LiquityContracts memory contracts) -{ +function _deployAndConnectCollateralContracts( + IERC20 _collateralToken, + IBoldToken _boldToken, + TroveManagerParams memory troveManagerParams +) returns (LiquityContracts memory contracts) { // TODO: optimize deployment order & constructor args & connector functions contracts.WETH = _collateralToken; // Deploy all contracts contracts.activePool = new ActivePool(address(_collateralToken)); - contracts.troveManager = new TroveManager(110e16, 5e16, 10e16); + contracts.troveManager = new TroveManager( + troveManagerParams.MCR, + troveManagerParams.LIQUIDATION_PENALTY_SP, + troveManagerParams.LIQUIDATION_PENALTY_REDISTRIBUTION + ); contracts.borrowerOperations = new BorrowerOperations(_collateralToken, contracts.troveManager); contracts.collSurplusPool = new CollSurplusPool(address(_collateralToken)); contracts.defaultPool = new DefaultPool(address(_collateralToken)); diff --git a/contracts/src/test/multicollateral.t.sol b/contracts/src/test/multicollateral.t.sol index e9f129c7..ab208d8f 100644 --- a/contracts/src/test/multicollateral.t.sol +++ b/contracts/src/test/multicollateral.t.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.18; import "./TestContracts/DevTestSetup.sol"; contract MulticollateralTest is DevTestSetup { - uint256 constant NUM_COLLATERALS = 4; + uint256 NUM_COLLATERALS = 4; LiquityContracts[] public contractsArray; function openMulticollateralTroveNoHints100pctMaxFeeWithIndex( @@ -48,8 +48,14 @@ contract MulticollateralTest is DevTestSetup { accountsList[6] ); + TroveManagerParams[] memory troveManagerParams = new TroveManagerParams[](NUM_COLLATERALS); + troveManagerParams[0] = TroveManagerParams(110e16, 5e16, 10e16); + troveManagerParams[1] = TroveManagerParams(120e16, 5e16, 10e16); + troveManagerParams[2] = TroveManagerParams(120e16, 5e16, 10e16); + troveManagerParams[3] = TroveManagerParams(125e16, 5e16, 10e16); + LiquityContracts[] memory _contractsArray; - (_contractsArray, collateralRegistry, boldToken) = _deployAndConnectContracts(NUM_COLLATERALS); + (_contractsArray, collateralRegistry, boldToken) = _deployAndConnectContracts(troveManagerParams); // Unimplemented feature (...):Copying of type struct LiquityContracts memory[] memory to storage not yet supported. for (uint256 c = 0; c < NUM_COLLATERALS; c++) { contractsArray.push(_contractsArray[c]); From 3aedd9fc0ad9b5cbf1f24e2670e17a6397c8debb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Fingen?= Date: Fri, 3 May 2024 16:20:59 +0100 Subject: [PATCH 05/13] chore: forge fmt --- contracts/src/BorrowerOperations.sol | 46 +++-- contracts/src/HintHelpers.sol | 2 +- contracts/src/TroveManager.sol | 12 +- contracts/src/test/SortedTroves.t.sol | 2 +- .../src/test/TestContracts/DevTestSetup.sol | 26 ++- .../src/test/interestRateAggregate.t.sol | 2 +- contracts/src/test/multicollateral.t.sol | 11 +- contracts/src/test/redemptions.t.sol | 178 +++++------------- 8 files changed, 112 insertions(+), 167 deletions(-) diff --git a/contracts/src/BorrowerOperations.sol b/contracts/src/BorrowerOperations.sol index 7c5fac10..3063f82f 100644 --- a/contracts/src/BorrowerOperations.sol +++ b/contracts/src/BorrowerOperations.sol @@ -284,7 +284,7 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe // TODO: Use oldDebt and assert in fuzzing, remove before deployment uint256 oldDebt = troveManager.getTroveEntireDebt(_troveId); _adjustTrove(msg.sender, _troveId, 0, false, _boldAmount, false, 0, contractsCache); - assert(troveManager.getTroveEntireDebt(_troveId) < oldDebt); + assert(troveManager.getTroveEntireDebt(_troveId) < oldDebt); } function adjustTrove( @@ -298,12 +298,19 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe ContractsCacheTMAPBT memory contractsCache = ContractsCacheTMAPBT(troveManager, activePool, boldToken); _requireTroveIsActive(contractsCache.troveManager, _troveId); _adjustTrove( - msg.sender, _troveId, _collChange, _isCollIncrease, _boldChange, _isDebtIncrease, _maxFeePercentage, contractsCache + msg.sender, + _troveId, + _collChange, + _isCollIncrease, + _boldChange, + _isDebtIncrease, + _maxFeePercentage, + contractsCache ); } function adjustUnredeemableTrove( - uint256 _troveId, + uint256 _troveId, uint256 _maxFeePercentage, uint256 _collChange, bool _isCollIncrease, @@ -314,12 +321,21 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe ) external override { ContractsCacheTMAPBT memory contractsCache = ContractsCacheTMAPBT(troveManager, activePool, boldToken); _requireTroveIsUnredeemable(contractsCache.troveManager, _troveId); - // TODO: Gas - pass the cached TM down here, since we fetch it again inside _adjustTrove? + // TODO: Gas - pass the cached TM down here, since we fetch it again inside _adjustTrove? _adjustTrove( - msg.sender, _troveId, _collChange, _isCollIncrease, _boldChange, _isDebtIncrease, _maxFeePercentage, contractsCache + msg.sender, + _troveId, + _collChange, + _isCollIncrease, + _boldChange, + _isDebtIncrease, + _maxFeePercentage, + contractsCache ); troveManager.setTroveStatusToActive(_troveId); - sortedTroves.insert(_troveId, contractsCache.troveManager.getTroveAnnualInterestRate(_troveId), _upperHint, _lowerHint); + sortedTroves.insert( + _troveId, contractsCache.troveManager.getTroveAnnualInterestRate(_troveId), _upperHint, _lowerHint + ); } function adjustTroveInterestRate( @@ -636,17 +652,19 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe uint256 _boldChange, bool _isDebtIncrease ) internal { - if (_isDebtIncrease) { // implies _boldChange > 0 + if (_isDebtIncrease) { + // implies _boldChange > 0 address borrower = _troveManager.ownerOf(_troveId); _boldToken.mint(borrower, _boldChange); - } else if (_boldChange > 0 ){ + } else if (_boldChange > 0) { _boldToken.burn(msg.sender, _boldChange); } - if (_isCollIncrease) { // implies _collChange > 0 + if (_isCollIncrease) { + // implies _collChange > 0 // Pull ETH tokens from sender and move them to the Active Pool _pullETHAndSendToActivePool(_activePool, _collChange); - } else if (_collChange > 0 ){ + } else if (_collChange > 0) { address borrower = _troveManager.ownerOf(_troveId); // Pull ETH from Active Pool and decrease its recorded ETH balance _activePool.sendETH(borrower, _collChange); @@ -713,11 +731,13 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe } function _requireTroveIsUnredeemable(ITroveManager _troveManager, uint256 _troveId) internal view { - require(_troveManager.checkTroveIsUnredeemable(_troveId), "BorrowerOps: Trove does not have unredeemable status"); + require( + _troveManager.checkTroveIsUnredeemable(_troveId), "BorrowerOps: Trove does not have unredeemable status" + ); } - function _requireTroveIsNotOpen(ITroveManager _troveManager, uint256 _troveId) internal view { - require(!_troveManager.checkTroveIsOpen(_troveId),"BorrowerOps: Trove is open"); + function _requireTroveIsNotOpen(ITroveManager _troveManager, uint256 _troveId) internal view { + require(!_troveManager.checkTroveIsOpen(_troveId), "BorrowerOps: Trove is open"); } function _requireNonZeroCollChange(uint256 _collChange) internal pure { diff --git a/contracts/src/HintHelpers.sol b/contracts/src/HintHelpers.sol index 89540cca..97baf19a 100644 --- a/contracts/src/HintHelpers.sol +++ b/contracts/src/HintHelpers.sol @@ -69,7 +69,7 @@ contract HintHelpers is LiquityBase, Ownable, CheckContract { uint256 currentId = troveManager.getTroveFromTroveIdsArray(arrayIndex); // Skip this Trove if it's unredeeamable and not in the sorted list - if (!sortedTroves.contains(currentId)) { continue; } + if (!sortedTroves.contains(currentId)) continue; uint256 currentInterestRate = troveManager.getTroveAnnualInterestRate(currentId); diff --git a/contracts/src/TroveManager.sol b/contracts/src/TroveManager.sol index 01d1f620..b07ce69f 100644 --- a/contracts/src/TroveManager.sol +++ b/contracts/src/TroveManager.sol @@ -555,7 +555,9 @@ contract TroveManager is ERC721, LiquityBase, Ownable, ITroveManager { if (collRedistributionPortion > 0) { (collToRedistribute, collSurplus) = _getCollPenaltyAndSurplus( collRedistributionPortion + collSurplus, // Coll surplus from offset can be eaten up by red. penalty - debtToRedistribute, LIQUIDATION_PENALTY_REDISTRIBUTION, _price + debtToRedistribute, + LIQUIDATION_PENALTY_REDISTRIBUTION, + _price ); } } @@ -1045,7 +1047,7 @@ contract TroveManager is ERC721, LiquityBase, Ownable, ITroveManager { uint256 snapshotETH = rewardSnapshots[_troveId].ETH; uint256 rewardPerUnitStaked = L_ETH - snapshotETH; - if (rewardPerUnitStaked == 0 || !checkTroveIsOpen(_troveId)) {return 0;} + if (rewardPerUnitStaked == 0 || !checkTroveIsOpen(_troveId)) return 0; uint256 stake = Troves[_troveId].stake; @@ -1059,7 +1061,7 @@ contract TroveManager is ERC721, LiquityBase, Ownable, ITroveManager { uint256 snapshotBoldDebt = rewardSnapshots[_troveId].boldDebt; uint256 rewardPerUnitStaked = L_boldDebt - snapshotBoldDebt; - if (rewardPerUnitStaked == 0 || !checkTroveIsOpen(_troveId)) {return 0;} + if (rewardPerUnitStaked == 0 || !checkTroveIsOpen(_troveId)) return 0; uint256 stake = Troves[_troveId].stake; @@ -1074,7 +1076,7 @@ contract TroveManager is ERC721, LiquityBase, Ownable, ITroveManager { * this indicates that rewards have occured since the snapshot was made, and the user therefore has * redistribution gains */ - if (!checkTroveIsOpen(_troveId)) {return false;} + if (!checkTroveIsOpen(_troveId)) return false; return (rewardSnapshots[_troveId].ETH < L_ETH); } @@ -1224,7 +1226,7 @@ contract TroveManager is ERC721, LiquityBase, Ownable, ITroveManager { rewardSnapshots[_troveId].boldDebt = 0; _removeTroveId(_troveId, TroveIdsArrayLength); - if (prevStatus == Status.active) {sortedTroves.remove(_troveId);} + if (prevStatus == Status.active) sortedTroves.remove(_troveId); // burn ERC721 // TODO: Should we do it? diff --git a/contracts/src/test/SortedTroves.t.sol b/contracts/src/test/SortedTroves.t.sol index 7f19ef76..258545ab 100644 --- a/contracts/src/test/SortedTroves.t.sol +++ b/contracts/src/test/SortedTroves.t.sol @@ -199,7 +199,7 @@ contract MockTroveManager { function _sortedTroves_removeFromBatch(TroveId id) external { _sortedTroves.removeFromBatch(TroveId.unwrap(id)); } - + function _sortedTroves_findInsertPosition(uint256 annualInterestRate, Hints memory hints) external view diff --git a/contracts/src/test/TestContracts/DevTestSetup.sol b/contracts/src/test/TestContracts/DevTestSetup.sol index e73a2481..7097e71e 100644 --- a/contracts/src/test/TestContracts/DevTestSetup.sol +++ b/contracts/src/test/TestContracts/DevTestSetup.sol @@ -168,7 +168,10 @@ contract DevTestSetup is BaseTest { return (ATroveId, BTroveId, CTroveId, DTroveId); } - function _setupForRedemption(TroveInterestRates memory _troveInterestRates) internal returns (uint256, uint256, TroveIDs memory) { + function _setupForRedemption(TroveInterestRates memory _troveInterestRates) + internal + returns (uint256, uint256, TroveIDs memory) + { TroveIDs memory troveIDs; priceFeed.setPrice(2000e18); @@ -202,11 +205,14 @@ contract DevTestSetup is BaseTest { return _setupForRedemption(troveInterestRates); } - - function _redeemAndCreateZombieTrovesAAndB(TroveIDs memory _troveIDs) internal returns (uint256, uint256, TroveIDs memory) { + function _redeemAndCreateZombieTrovesAAndB(TroveIDs memory _troveIDs) + internal + returns (uint256, uint256, TroveIDs memory) + { // Redeem enough to leave to leave A with 0 debt and B with debt < MIN_NET_DEBT uint256 redeemFromA = troveManager.getTroveEntireDebt(_troveIDs.A) - troveManager.BOLD_GAS_COMPENSATION(); - uint256 redeemFromB = troveManager.getTroveEntireDebt(_troveIDs.B) - troveManager.BOLD_GAS_COMPENSATION() - troveManager.MIN_NET_DEBT() / 2; + uint256 redeemFromB = troveManager.getTroveEntireDebt(_troveIDs.B) - troveManager.BOLD_GAS_COMPENSATION() + - troveManager.MIN_NET_DEBT() / 2; uint256 redeemAmount = redeemFromA + redeemFromB; // Fully redeem A and leave B with debt < MIN_NET_DEBT @@ -216,17 +222,21 @@ contract DevTestSetup is BaseTest { assertEq(troveManager.getTroveEntireDebt(_troveIDs.A), BOLD_GAS_COMP); assertLt(troveManager.getTroveEntireDebt(_troveIDs.B) - BOLD_GAS_COMP, troveManager.MIN_NET_DEBT()); - // Check A and B tagged as Zombie troves + // Check A and B tagged as Zombie troves assertEq(troveManager.getTroveStatus(_troveIDs.A), 5); // status 'unredeemable' assertEq(troveManager.getTroveStatus(_troveIDs.A), 5); // status 'unredeemable' } - function _redeemAndCreateZombieTroveAAndHitB(TroveIDs memory _troveIDs) internal returns (uint256, uint256, TroveIDs memory) { + function _redeemAndCreateZombieTroveAAndHitB(TroveIDs memory _troveIDs) + internal + returns (uint256, uint256, TroveIDs memory) + { // Redeem enough to leave to leave A with 0 debt and B with debt < MIN_NET_DEBT uint256 redeemFromA = troveManager.getTroveEntireDebt(_troveIDs.A) - troveManager.BOLD_GAS_COMPENSATION(); // Leave B with net_debt > min_net_debt - uint256 redeemFromB = troveManager.getTroveEntireDebt(_troveIDs.B) - troveManager.BOLD_GAS_COMPENSATION() - troveManager.MIN_NET_DEBT() - 37; - + uint256 redeemFromB = troveManager.getTroveEntireDebt(_troveIDs.B) - troveManager.BOLD_GAS_COMPENSATION() + - troveManager.MIN_NET_DEBT() - 37; + uint256 redeemAmount = redeemFromA + redeemFromB; // Fully redeem A and leave B with debt > MIN_NET_DEBT diff --git a/contracts/src/test/interestRateAggregate.t.sol b/contracts/src/test/interestRateAggregate.t.sol index b3a613e9..a9e198fe 100644 --- a/contracts/src/test/interestRateAggregate.t.sol +++ b/contracts/src/test/interestRateAggregate.t.sol @@ -2570,7 +2570,7 @@ contract InterestRateAggregate is DevTestSetup { // Check recorded debt sum has changed correctly assertEq(activePool.aggWeightedDebtSum(), expectedAggWeightedRecordedDebt); } - + // TODO: mixed collateral & debt adjustment opps // TODO: tests with pending debt redist. gain >0 // TODO: tests that show total debt change under user ops diff --git a/contracts/src/test/multicollateral.t.sol b/contracts/src/test/multicollateral.t.sol index ab208d8f..2017a3e1 100644 --- a/contracts/src/test/multicollateral.t.sol +++ b/contracts/src/test/multicollateral.t.sol @@ -176,13 +176,11 @@ contract MulticollateralTest is DevTestSetup { _spBoldAmount2 = bound(_spBoldAmount2, 0, boldAmount - 200e18); _spBoldAmount3 = bound(_spBoldAmount3, 0, boldAmount - 200e18); _spBoldAmount4 = bound(_spBoldAmount4, 0, boldAmount - 200e18 - minBoldBalance); - _redemptionFraction = bound( - _redemptionFraction, - DECIMAL_PRECISION / minBoldBalance, - DECIMAL_PRECISION - ); + _redemptionFraction = bound(_redemptionFraction, DECIMAL_PRECISION / minBoldBalance, DECIMAL_PRECISION); - _testMultiCollateralRedemption(boldAmount, _spBoldAmount1, _spBoldAmount2, _spBoldAmount3, _spBoldAmount4, _redemptionFraction); + _testMultiCollateralRedemption( + boldAmount, _spBoldAmount1, _spBoldAmount2, _spBoldAmount3, _spBoldAmount4, _redemptionFraction + ); } function testMultiCollateralRedemptionMaxSPAmount() public { @@ -225,7 +223,6 @@ contract MulticollateralTest is DevTestSetup { testValues3.price = contractsArray[2].priceFeed.getPrice(); testValues4.price = contractsArray[3].priceFeed.getPrice(); - // First collateral openMulticollateralTroveNoHints100pctMaxFeeWithIndex(0, A, 0, 10e18, _boldAmount, 5e16); if (_spBoldAmount1 > 0) makeMulticollateralSPDeposit(0, A, _spBoldAmount1); diff --git a/contracts/src/test/redemptions.t.sol b/contracts/src/test/redemptions.t.sol index 6acdb972..2cbeed4b 100644 --- a/contracts/src/test/redemptions.t.sol +++ b/contracts/src/test/redemptions.t.sol @@ -4,7 +4,7 @@ import "./TestContracts/DevTestSetup.sol"; contract Redemptions is DevTestSetup { function testRedemptionIsInOrderOfInterestRate() public { - (uint256 coll, , TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); + (uint256 coll,, TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); uint256 debt_A = troveManager.getTroveEntireDebt(troveIDs.A); uint256 debt_B = troveManager.getTroveEntireDebt(troveIDs.B); @@ -43,7 +43,7 @@ contract Redemptions is DevTestSetup { // - Troves can be redeemed down to gas comp function testFullRedemptionDoesntCloseTroves() public { - ( , , TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); + (,, TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); uint256 debt_A = troveManager.getTroveEntireDebt(troveIDs.A); uint256 debt_B = troveManager.getTroveEntireDebt(troveIDs.B); @@ -58,7 +58,7 @@ contract Redemptions is DevTestSetup { } function testFullRedemptionLeavesTrovesWithDebtEqualToGasComp() public { - ( , , TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); + (,, TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); uint256 debt_A = troveManager.getTroveEntireDebt(troveIDs.A); uint256 debt_B = troveManager.getTroveEntireDebt(troveIDs.B); @@ -73,7 +73,7 @@ contract Redemptions is DevTestSetup { } function testFullRedemptionSkipsTrovesAtGasCompDebt() public { - (uint256 coll, , TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); + (uint256 coll,, TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); uint256 debt_A = troveManager.getTroveEntireDebt(troveIDs.A); uint256 debt_B = troveManager.getTroveEntireDebt(troveIDs.B); @@ -105,7 +105,7 @@ contract Redemptions is DevTestSetup { // - Accrued Trove interest contributes to redee into debt of a redeemed trove function testRedemptionIncludesAccruedTroveInterest() public { - ( , , TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); + (,, TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); // Fast-forward to generate interest vm.warp(block.timestamp + 1 days); @@ -132,7 +132,7 @@ contract Redemptions is DevTestSetup { // --- Zombie Troves --- function testFullyRedeemedTroveBecomesZombieTrove() public { - ( , , TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); + (,, TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); _redeemAndCreateZombieTrovesAAndB(troveIDs); @@ -140,7 +140,7 @@ contract Redemptions is DevTestSetup { } function testTroveRedeemedToBelowMIN_NET_DEBTBecomesZombieTrove() public { - ( , , TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); + (,, TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); _redeemAndCreateZombieTrovesAAndB(troveIDs); @@ -148,7 +148,7 @@ contract Redemptions is DevTestSetup { } function testTroveRedeemedToAboveMIN_NET_DEBTDoesNotBecomesZombieTrove() public { - ( , , TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); + (,, TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); _redeemAndCreateZombieTroveAAndHitB(troveIDs); @@ -156,7 +156,7 @@ contract Redemptions is DevTestSetup { } function testZombieTrovesRemovedFromSortedList() public { - ( , , TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); + (,, TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); // Check A,B,C,D in sorted list assertTrue(sortedTroves.contains(troveIDs.A)); @@ -177,7 +177,7 @@ contract Redemptions is DevTestSetup { } function testZombieTroveCantBeRedeemedFrom() public { - ( , , TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); + (,, TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); _redeemAndCreateZombieTrovesAAndB(troveIDs); @@ -201,11 +201,11 @@ contract Redemptions is DevTestSetup { function testZombieTrovesCanReceiveRedistGains() public { uint256 interestRate_E = 5e16; // 5% uint256 troveDebtRequest_E = 2250e18; - uint256 troveColl_E = 25e17; + uint256 troveColl_E = 25e17; uint256 price = 2000e18; priceFeed.setPrice(price); - ( , , TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); + (,, TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); _redeemAndCreateZombieTrovesAAndB(troveIDs); @@ -236,7 +236,7 @@ contract Redemptions is DevTestSetup { troveInterestRates.C = 3e17; // 30% troveInterestRates.D = 4e17; // 40% - ( , , TroveIDs memory troveIDs) = _setupForRedemption(troveInterestRates); + (,, TroveIDs memory troveIDs) = _setupForRedemption(troveInterestRates); _redeemAndCreateZombieTrovesAAndB(troveIDs); @@ -254,7 +254,7 @@ contract Redemptions is DevTestSetup { uint256 expectedDebt_B = debt_B * (1e18 + troveInterestRates.B) / 1e18; // 20% increase assertEq(troveManager.getTroveEntireDebt(troveIDs.A), expectedDebt_A); // 10% increase - assertEq(troveManager.getTroveEntireDebt(troveIDs.B), expectedDebt_B); // 20% increase + assertEq(troveManager.getTroveEntireDebt(troveIDs.B), expectedDebt_B); // 20% increase } function testZombieTrovesCanAccrueInterestThatBringThemAboveMIN_NET_DEBT() public { @@ -264,18 +264,18 @@ contract Redemptions is DevTestSetup { troveInterestRates.C = 3e17; // 30% troveInterestRates.D = 4e17; // 40% - ( , , TroveIDs memory troveIDs) = _setupForRedemption(troveInterestRates); + (,, TroveIDs memory troveIDs) = _setupForRedemption(troveInterestRates); _redeemAndCreateZombieTrovesAAndB(troveIDs); assertLt(troveManager.getTroveEntireDebt(troveIDs.A), troveManager.MIN_NET_DEBT()); - assertLt(troveManager.getTroveEntireDebt(troveIDs.B), troveManager.MIN_NET_DEBT()); + assertLt(troveManager.getTroveEntireDebt(troveIDs.B), troveManager.MIN_NET_DEBT()); // 100 years passes vm.warp(block.timestamp + 36500 days); assertGt(troveManager.getTroveEntireDebt(troveIDs.A), troveManager.MIN_NET_DEBT()); - assertGt(troveManager.getTroveEntireDebt(troveIDs.B), troveManager.MIN_NET_DEBT()); + assertGt(troveManager.getTroveEntireDebt(troveIDs.B), troveManager.MIN_NET_DEBT()); } function testZombieTrovesCanReceiveRedistGainsThatBringThemAboveMIN_NET_DEBT() public { @@ -286,7 +286,7 @@ contract Redemptions is DevTestSetup { uint256 price = 2000e18; priceFeed.setPrice(price); - ( , , TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); + (,, TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); _redeemAndCreateZombieTrovesAAndB(troveIDs); @@ -302,9 +302,9 @@ contract Redemptions is DevTestSetup { assertLt(troveManager.getCurrentICR(troveID_E, price), troveManager.MCR()); assertLt(troveManager.getTroveEntireDebt(troveIDs.A), troveManager.MIN_NET_DEBT()); - assertLt(troveManager.getTroveEntireDebt(troveIDs.B), troveManager.MIN_NET_DEBT()); + assertLt(troveManager.getTroveEntireDebt(troveIDs.B), troveManager.MIN_NET_DEBT()); - // A liquidates E + // A liquidates E liquidate(A, troveID_E); assertEq(troveManager.getTroveStatus(troveID_E), 3); // Status 3 - closed by liquidation @@ -312,13 +312,13 @@ contract Redemptions is DevTestSetup { assertTrue(troveManager.hasRedistributionGains(troveIDs.B)); assertGt(troveManager.getTroveEntireDebt(troveIDs.A), troveManager.MIN_NET_DEBT()); - assertGt(troveManager.getTroveEntireDebt(troveIDs.B), troveManager.MIN_NET_DEBT()); + assertGt(troveManager.getTroveEntireDebt(troveIDs.B), troveManager.MIN_NET_DEBT()); } // --- Borrower ops on zombie troves --- function testZombieBorrowerCanCloseZombieTrove() public { - ( , , TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); + (,, TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); _redeemAndCreateZombieTrovesAAndB(troveIDs); @@ -337,7 +337,7 @@ contract Redemptions is DevTestSetup { } function testZombieBorrowerCanDrawFreshDebtToAboveMIN_NET_DEBT() public { - ( , , TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); + (,, TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); _redeemAndCreateZombieTrovesAAndB(troveIDs); @@ -355,27 +355,13 @@ contract Redemptions is DevTestSetup { // A and B withdraw Bold from their zombie Trove vm.startPrank(A); borrowerOperations.adjustUnredeemableTrove( - troveIDs.A, - 1e18, - 0, - false, - debtDelta_A + surplusDebt, - true, - troveIDs.A, - troveIDs.A + troveIDs.A, 1e18, 0, false, debtDelta_A + surplusDebt, true, troveIDs.A, troveIDs.A ); vm.stopPrank(); vm.startPrank(B); borrowerOperations.adjustUnredeemableTrove( - troveIDs.B, - 1e18, - 0, - false, - debtDelta_A + surplusDebt, - true, - troveIDs.B, - troveIDs.B + troveIDs.B, 1e18, 0, false, debtDelta_A + surplusDebt, true, troveIDs.B, troveIDs.B ); vm.stopPrank(); @@ -385,7 +371,7 @@ contract Redemptions is DevTestSetup { } function testZombieTroveDrawingFreshDebtToAboveMIN_NET_DEBTChangesStatusToActive() public { - ( , , TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); + (,, TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); _redeemAndCreateZombieTrovesAAndB(troveIDs); @@ -403,27 +389,13 @@ contract Redemptions is DevTestSetup { // A and B withdraw Bold from their zombie Trove vm.startPrank(A); borrowerOperations.adjustUnredeemableTrove( - troveIDs.A, - 1e18, - 0, - false, - debtDelta_A + surplusDebt, - true, - troveIDs.A, - troveIDs.A + troveIDs.A, 1e18, 0, false, debtDelta_A + surplusDebt, true, troveIDs.A, troveIDs.A ); vm.stopPrank(); vm.startPrank(B); borrowerOperations.adjustUnredeemableTrove( - troveIDs.B, - 1e18, - 0, - false, - debtDelta_A + surplusDebt, - true, - troveIDs.B, - troveIDs.B + troveIDs.B, 1e18, 0, false, debtDelta_A + surplusDebt, true, troveIDs.B, troveIDs.B ); vm.stopPrank(); @@ -437,7 +409,7 @@ contract Redemptions is DevTestSetup { } function testZombieTroveDrawingFreshDebtToAboveMIN_NET_DEBTInsertsItToSortedList() public { - ( , , TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); + (,, TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); _redeemAndCreateZombieTrovesAAndB(troveIDs); @@ -459,27 +431,13 @@ contract Redemptions is DevTestSetup { // A and B withdraw Bold from their zombie Trove vm.startPrank(A); borrowerOperations.adjustUnredeemableTrove( - troveIDs.A, - 1e18, - 0, - false, - debtDelta_A + surplusDebt, - true, - troveIDs.A, - troveIDs.A + troveIDs.A, 1e18, 0, false, debtDelta_A + surplusDebt, true, troveIDs.A, troveIDs.A ); vm.stopPrank(); vm.startPrank(B); borrowerOperations.adjustUnredeemableTrove( - troveIDs.B, - 1e18, - 0, - false, - debtDelta_A + surplusDebt, - true, - troveIDs.B, - troveIDs.B + troveIDs.B, 1e18, 0, false, debtDelta_A + surplusDebt, true, troveIDs.B, troveIDs.B ); vm.stopPrank(); @@ -493,7 +451,7 @@ contract Redemptions is DevTestSetup { } function testZombieBorrowerDrawsFreshDebtToAboveMIN_NET_DEBTReducesPendingInterestTo0() public { - ( , , TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); + (,, TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); _redeemAndCreateZombieTrovesAAndB(troveIDs); @@ -514,27 +472,13 @@ contract Redemptions is DevTestSetup { // A and B withdraw Bold from their zombie Trove vm.startPrank(A); borrowerOperations.adjustUnredeemableTrove( - troveIDs.A, - 1e18, - 0, - false, - debtDelta_A + surplusDebt, - true, - troveIDs.A, - troveIDs.A + troveIDs.A, 1e18, 0, false, debtDelta_A + surplusDebt, true, troveIDs.A, troveIDs.A ); vm.stopPrank(); vm.startPrank(B); borrowerOperations.adjustUnredeemableTrove( - troveIDs.B, - 1e18, - 0, - false, - debtDelta_A + surplusDebt, - true, - troveIDs.B, - troveIDs.B + troveIDs.B, 1e18, 0, false, debtDelta_A + surplusDebt, true, troveIDs.B, troveIDs.B ); vm.stopPrank(); @@ -548,7 +492,7 @@ contract Redemptions is DevTestSetup { } function testZombieTroveBorrowerCanNotDrawFreshDebtToBelowMIN_NET_DEBT() public { - ( , , TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); + (,, TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); _redeemAndCreateZombieTrovesAAndB(troveIDs); @@ -567,34 +511,20 @@ contract Redemptions is DevTestSetup { vm.startPrank(A); vm.expectRevert("BorrowerOps: Trove's net debt must be greater than minimum"); borrowerOperations.adjustUnredeemableTrove( - troveIDs.A, - 1e18, - 0, - false, - debtDelta_A - debtDeficiency, - true, - troveIDs.A, - troveIDs.A + troveIDs.A, 1e18, 0, false, debtDelta_A - debtDeficiency, true, troveIDs.A, troveIDs.A ); vm.stopPrank(); vm.startPrank(B); vm.expectRevert("BorrowerOps: Trove's net debt must be greater than minimum"); borrowerOperations.adjustUnredeemableTrove( - troveIDs.B, - 1e18, - 0, - false, - debtDelta_B - debtDeficiency, - true, - troveIDs.B, - troveIDs.B + troveIDs.B, 1e18, 0, false, debtDelta_B - debtDeficiency, true, troveIDs.B, troveIDs.B ); vm.stopPrank(); } function testZombieTroveBorrowerCanNotRepayDebt() public { - ( , , TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); + (,, TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); _redeemAndCreateZombieTrovesAAndB(troveIDs); @@ -616,7 +546,7 @@ contract Redemptions is DevTestSetup { } function testZombieTroveBorrowerCanNotUseNormalWithdrawBoldFunction() public { - ( , , TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); + (,, TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); _redeemAndCreateZombieTrovesAAndB(troveIDs); @@ -634,7 +564,7 @@ contract Redemptions is DevTestSetup { } function testZombieTroveBorrowerCanNotUseNormalAdjustTroveFunction() public { - ( , , TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); + (,, TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); _redeemAndCreateZombieTrovesAAndB(troveIDs); @@ -642,31 +572,17 @@ contract Redemptions is DevTestSetup { vm.startPrank(A); vm.expectRevert("BorrowerOps: Trove does not have active status"); - borrowerOperations.adjustTrove( - troveIDs.A, - 1e18, - 0, - false, - debtWithdrawal, - true - ); + borrowerOperations.adjustTrove(troveIDs.A, 1e18, 0, false, debtWithdrawal, true); vm.stopPrank(); vm.startPrank(B); vm.expectRevert("BorrowerOps: Trove does not have active status"); - borrowerOperations.adjustTrove( - troveIDs.B, - 1e18, - 0, - false, - debtWithdrawal, - true - ); + borrowerOperations.adjustTrove(troveIDs.B, 1e18, 0, false, debtWithdrawal, true); vm.stopPrank(); } function testZombieTroveBorrowerCanNotAddColl() public { - ( , , TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); + (,, TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); _redeemAndCreateZombieTrovesAAndB(troveIDs); @@ -681,12 +597,12 @@ contract Redemptions is DevTestSetup { // B attempts to repay and can't (since would leave Trove at debt < MIN_NET_DEBT) vm.startPrank(B); vm.expectRevert("BorrowerOps: Trove does not have active status"); - borrowerOperations.addColl(troveIDs.B, collTopUp); + borrowerOperations.addColl(troveIDs.B, collTopUp); vm.stopPrank(); } function testZombieTroveBorrowerCanNotChangeInterestRate() public { - ( , , TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); + (,, TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); _redeemAndCreateZombieTrovesAAndB(troveIDs); @@ -705,7 +621,7 @@ contract Redemptions is DevTestSetup { } function testZombieTroveAccruedInterestCanBePermissionlesslyApplied() public { - ( , , TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); + (,, TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); _redeemAndCreateZombieTrovesAAndB(troveIDs); @@ -726,7 +642,7 @@ contract Redemptions is DevTestSetup { } function testZombieTroveCanBeLiquidated() public { - ( , , TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); + (,, TroveIDs memory troveIDs) = _setupForRedemptionAscendingInterest(); _redeemAndCreateZombieTrovesAAndB(troveIDs); From b89f18e432306dd204273564b0580ead92b7ec5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Fingen?= Date: Fri, 3 May 2024 16:31:58 +0100 Subject: [PATCH 06/13] chore: Remove warnings --- contracts/src/test/TestContracts/DevTestSetup.sol | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/contracts/src/test/TestContracts/DevTestSetup.sol b/contracts/src/test/TestContracts/DevTestSetup.sol index 7097e71e..e1992799 100644 --- a/contracts/src/test/TestContracts/DevTestSetup.sol +++ b/contracts/src/test/TestContracts/DevTestSetup.sol @@ -205,10 +205,7 @@ contract DevTestSetup is BaseTest { return _setupForRedemption(troveInterestRates); } - function _redeemAndCreateZombieTrovesAAndB(TroveIDs memory _troveIDs) - internal - returns (uint256, uint256, TroveIDs memory) - { + function _redeemAndCreateZombieTrovesAAndB(TroveIDs memory _troveIDs) internal { // Redeem enough to leave to leave A with 0 debt and B with debt < MIN_NET_DEBT uint256 redeemFromA = troveManager.getTroveEntireDebt(_troveIDs.A) - troveManager.BOLD_GAS_COMPENSATION(); uint256 redeemFromB = troveManager.getTroveEntireDebt(_troveIDs.B) - troveManager.BOLD_GAS_COMPENSATION() @@ -227,10 +224,7 @@ contract DevTestSetup is BaseTest { assertEq(troveManager.getTroveStatus(_troveIDs.A), 5); // status 'unredeemable' } - function _redeemAndCreateZombieTroveAAndHitB(TroveIDs memory _troveIDs) - internal - returns (uint256, uint256, TroveIDs memory) - { + function _redeemAndCreateZombieTroveAAndHitB(TroveIDs memory _troveIDs) internal { // Redeem enough to leave to leave A with 0 debt and B with debt < MIN_NET_DEBT uint256 redeemFromA = troveManager.getTroveEntireDebt(_troveIDs.A) - troveManager.BOLD_GAS_COMPENSATION(); // Leave B with net_debt > min_net_debt From 57446c9fc6bdd247beec16924a475fc2f7b600f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Fingen?= Date: Fri, 3 May 2024 16:57:15 +0100 Subject: [PATCH 07/13] test: Fix hardhat tests after reduce liquidation penalty --- contracts/test/CollSurplusPool.js | 4 ++-- contracts/test/GasCompensationTest.js | 4 ++-- contracts/test/OwnershipTest.js | 4 ++-- contracts/utils/deploymentHelpers.js | 5 ++--- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/contracts/test/CollSurplusPool.js b/contracts/test/CollSurplusPool.js index a431eb11..a3bf5952 100644 --- a/contracts/test/CollSurplusPool.js +++ b/contracts/test/CollSurplusPool.js @@ -40,14 +40,14 @@ contract("CollSurplusPool", async (accounts) => { it("CollSurplusPool: claimColl(): Reverts if caller is not Borrower Operations", async () => { await th.assertRevert( - collSurplusPool.claimColl(A, th.addressToTroveId(A), { from: A }), + collSurplusPool.claimColl(A, { from: A }), "CollSurplusPool: Caller is not Borrower Operations", ); }); it("CollSurplusPool: claimColl(): Reverts if nothing to claim", async () => { await th.assertRevert( - borrowerOperations.claimCollateral(th.addressToTroveId(A), { from: A }), + borrowerOperations.claimCollateral({ from: A }), "CollSurplusPool: No collateral available to claim", ); }); diff --git a/contracts/test/GasCompensationTest.js b/contracts/test/GasCompensationTest.js index d956912a..458988bf 100644 --- a/contracts/test/GasCompensationTest.js +++ b/contracts/test/GasCompensationTest.js @@ -60,9 +60,9 @@ contract("Gas compensation tests", async (accounts) => { }); before(async () => { - troveManagerTester = await TroveManagerTester.new(); + troveManagerTester = await TroveManagerTester.new(toBN(dec(110, 16)), toBN(dec(10, 16)), toBN(dec(10, 16))); const WETH = await ERC20.new("WETH", "WETH"); - borrowerOperationsTester = await BorrowerOperationsTester.new(WETH.address); + borrowerOperationsTester = await BorrowerOperationsTester.new(WETH.address, troveManagerTester.address); TroveManagerTester.setAsDeployed(troveManagerTester); BorrowerOperationsTester.setAsDeployed(borrowerOperationsTester); diff --git a/contracts/test/OwnershipTest.js b/contracts/test/OwnershipTest.js index 0f5bc12d..306f5d6b 100644 --- a/contracts/test/OwnershipTest.js +++ b/contracts/test/OwnershipTest.js @@ -20,7 +20,7 @@ contract("All Liquity functions with onlyOwner modifier", async (accounts) => { before(async () => { contracts = await deploymentHelper.deployLiquityCore(); - contracts.borrowerOperations = await BorrowerOperationsTester.new(contracts.WETH.address); + contracts.borrowerOperations = await BorrowerOperationsTester.new(contracts.WETH.address, contracts.troveManager.address); contracts = await deploymentHelper.deployBoldToken(contracts); boldToken = contracts.boldToken; @@ -89,7 +89,7 @@ contract("All Liquity functions with onlyOwner modifier", async (accounts) => { describe("BorrowerOperations", async (accounts) => { it("setAddresses(): reverts when called by non-owner, with wrong addresses, or twice", async () => { - await testDeploymentSetter(borrowerOperations, 9); + await testDeploymentSetter(borrowerOperations, 8); }); }); diff --git a/contracts/utils/deploymentHelpers.js b/contracts/utils/deploymentHelpers.js index 42959066..bd4b0772 100644 --- a/contracts/utils/deploymentHelpers.js +++ b/contracts/utils/deploymentHelpers.js @@ -59,7 +59,8 @@ class DeploymentHelper { // Borrowing contracts const activePool = await Contracts.ActivePool.new(WETH.address); - const borrowerOperations = await Contracts.BorrowerOperations.new(WETH.address); + const troveManager = await Contracts.TroveManager.new(web3.utils.toBN("1100000000000000000"), web3.utils.toBN("100000000000000000"), web3.utils.toBN("100000000000000000")); + const borrowerOperations = await Contracts.BorrowerOperations.new(WETH.address, troveManager.address); const collSurplusPool = await Contracts.CollSurplusPool.new(WETH.address); const defaultPool = await Contracts.DefaultPool.new(WETH.address); const gasPool = await Contracts.GasPool.new(); @@ -67,7 +68,6 @@ class DeploymentHelper { const priceFeed = await Contracts.PriceFeedMock.new(); const sortedTroves = await Contracts.SortedTroves.new(); const stabilityPool = await Contracts.StabilityPool.new(WETH.address); - const troveManager = await Contracts.TroveManager.new(); const { boldToken } = await this.deployBoldToken({ troveManager, @@ -167,7 +167,6 @@ class DeploymentHelper { // set contracts in BorrowerOperations await contracts.borrowerOperations.setAddresses( - contracts.troveManager.address, contracts.activePool.address, contracts.defaultPool.address, contracts.stabilityPool.address, From 78d631f4c04718e4762c480e834adf711f8e5ebe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Fingen?= Date: Mon, 6 May 2024 10:34:14 +0100 Subject: [PATCH 08/13] Add comments and rename vars for reduce liquidation penalty PR Addressing PR #157 comments. --- contracts/src/BorrowerOperations.sol | 2 +- contracts/src/TroveManager.sol | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/contracts/src/BorrowerOperations.sol b/contracts/src/BorrowerOperations.sol index 3063f82f..437c65b2 100644 --- a/contracts/src/BorrowerOperations.sol +++ b/contracts/src/BorrowerOperations.sol @@ -565,7 +565,7 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe } /** - * Claim remaining collateral from a redemption or from a liquidation with ICR > MCR in Recovery Mode + * Claim remaining collateral from a liquidation with ICR exceeding the liquidation penalty */ function claimCollateral() external override { // send ETH from CollSurplus Pool to owner diff --git a/contracts/src/TroveManager.sol b/contracts/src/TroveManager.sol index b07ce69f..49e758ef 100644 --- a/contracts/src/TroveManager.sol +++ b/contracts/src/TroveManager.sol @@ -556,7 +556,7 @@ contract TroveManager is ERC721, LiquityBase, Ownable, ITroveManager { (collToRedistribute, collSurplus) = _getCollPenaltyAndSurplus( collRedistributionPortion + collSurplus, // Coll surplus from offset can be eaten up by red. penalty debtToRedistribute, - LIQUIDATION_PENALTY_REDISTRIBUTION, + LIQUIDATION_PENALTY_REDISTRIBUTION, // _penaltyRatio _price ); } @@ -569,13 +569,13 @@ contract TroveManager is ERC721, LiquityBase, Ownable, ITroveManager { uint256 _debtToLiquidate, uint256 _penaltyRatio, uint256 _price - ) internal pure returns (uint256 collPenalty, uint256 collSurplus) { - uint256 maxCollWithPenalty = _debtToLiquidate * (DECIMAL_PRECISION + _penaltyRatio) / _price; - if (_collToLiquidate > maxCollWithPenalty) { - collPenalty = maxCollWithPenalty; - collSurplus = _collToLiquidate - maxCollWithPenalty; + ) internal pure returns (uint256 seizedColl, uint256 collSurplus) { + uint256 maxSeizedColl = _debtToLiquidate * (DECIMAL_PRECISION + _penaltyRatio) / _price; + if (_collToLiquidate > maxSeizedColl) { + seizedColl = maxSeizedColl; + collSurplus = _collToLiquidate - maxSeizedColl; } else { - collPenalty = _collToLiquidate; + seizedColl = _collToLiquidate; collSurplus = 0; } } From 81577dc736273445c96398cf6efe26fd23830920 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Fingen?= Date: Thu, 9 May 2024 17:21:59 +0100 Subject: [PATCH 09/13] fix: Fixes after merge of main --- contracts/src/test/liquidations.t.sol | 25 +++++++++++++----------- contracts/src/test/liquidationsLST.t.sol | 11 ++++++----- contracts/src/test/stabilityPool.t.sol | 12 ++++++------ 3 files changed, 26 insertions(+), 22 deletions(-) diff --git a/contracts/src/test/liquidations.t.sol b/contracts/src/test/liquidations.t.sol index 3f69815c..13367fd7 100644 --- a/contracts/src/test/liquidations.t.sol +++ b/contracts/src/test/liquidations.t.sol @@ -10,14 +10,15 @@ contract LiquidationsTest is DevTestSetup { priceFeed.setPrice(2000e18); vm.startPrank(A); uint256 ATroveId = borrowerOperations.openTrove( - A, 0, 1e18, collAmount, liquidationAmount - troveManager.BOLD_GAS_COMPENSATION(), 0, 0, 0 + A, 0, collAmount, liquidationAmount - troveManager.BOLD_GAS_COMPENSATION(), 0, 0, 0 ); vm.stopPrank(); vm.startPrank(B); - borrowerOperations.openTrove(B, 0, 1e18, 2 * collAmount, liquidationAmount, 0, 0, 0); + borrowerOperations.openTrove(B, 0, 2 * collAmount, liquidationAmount, 0, 0, 0); + vm.stopPrank(); // B deposits to SP - stabilityPool.provideToSP(liquidationAmount); + makeSPDepositAndClaim(B, liquidationAmount); // Price drops priceFeed.setPrice(1100e18 - 1); @@ -77,14 +78,15 @@ contract LiquidationsTest is DevTestSetup { priceFeed.setPrice(2000e18); vm.startPrank(A); uint256 ATroveId = borrowerOperations.openTrove( - A, 0, 1e18, collAmount, liquidationAmount - troveManager.BOLD_GAS_COMPENSATION(), 0, 0, 0 + A, 0, collAmount, liquidationAmount - troveManager.BOLD_GAS_COMPENSATION(), 0, 0, 0 ); vm.stopPrank(); vm.startPrank(B); - borrowerOperations.openTrove(B, 0, 1e18, 3 * collAmount, liquidationAmount, 0, 0, 0); + borrowerOperations.openTrove(B, 0, 3 * collAmount, liquidationAmount, 0, 0, 0); + vm.stopPrank(); // B deposits to SP - stabilityPool.provideToSP(liquidationAmount); + makeSPDepositAndClaim(B, liquidationAmount); // Price drops priceFeed.setPrice(1030e18); @@ -135,12 +137,12 @@ contract LiquidationsTest is DevTestSetup { priceFeed.setPrice(2000e18); vm.startPrank(A); uint256 ATroveId = borrowerOperations.openTrove( - A, 0, 1e18, collAmount, liquidationAmount - troveManager.BOLD_GAS_COMPENSATION(), 0, 0, 0 + A, 0, collAmount, liquidationAmount - troveManager.BOLD_GAS_COMPENSATION(), 0, 0, 0 ); vm.stopPrank(); vm.startPrank(B); - uint256 BTroveId = borrowerOperations.openTrove(B, 0, 1e18, 2 * collAmount, liquidationAmount, 0, 0, 0); + uint256 BTroveId = borrowerOperations.openTrove(B, 0, 2 * collAmount, liquidationAmount, 0, 0, 0); // Price drops priceFeed.setPrice(1100e18 - 1); @@ -201,14 +203,15 @@ contract LiquidationsTest is DevTestSetup { priceFeed.setPrice(2000e18); vm.startPrank(A); uint256 ATroveId = borrowerOperations.openTrove( - A, 0, 1e18, collAmount, liquidationAmount - troveManager.BOLD_GAS_COMPENSATION(), 0, 0, 0 + A, 0, collAmount, liquidationAmount - troveManager.BOLD_GAS_COMPENSATION(), 0, 0, 0 ); vm.stopPrank(); vm.startPrank(B); - uint256 BTroveId = borrowerOperations.openTrove(B, 0, 1e18, 2 * collAmount, liquidationAmount, 0, 0, 0); + uint256 BTroveId = borrowerOperations.openTrove(B, 0, 2 * collAmount, liquidationAmount, 0, 0, 0); + vm.stopPrank(); // B deposits to SP - stabilityPool.provideToSP(liquidationAmount / 2); + makeSPDepositAndClaim(B, liquidationAmount / 2); // Price drops priceFeed.setPrice(1100e18 - 1); diff --git a/contracts/src/test/liquidationsLST.t.sol b/contracts/src/test/liquidationsLST.t.sol index 5e95d0a9..bdf743e8 100644 --- a/contracts/src/test/liquidationsLST.t.sol +++ b/contracts/src/test/liquidationsLST.t.sol @@ -51,12 +51,12 @@ contract LiquidationsLSTTest is DevTestSetup { priceFeed.setPrice(2000e18); vm.startPrank(A); uint256 ATroveId = borrowerOperations.openTrove( - A, 0, 1e18, collAmount, liquidationAmount - troveManager.BOLD_GAS_COMPENSATION(), 0, 0, 0 + A, 0, collAmount, liquidationAmount - troveManager.BOLD_GAS_COMPENSATION(), 0, 0, 0 ); vm.stopPrank(); vm.startPrank(B); - uint256 BTroveId = borrowerOperations.openTrove(B, 0, 1e18, 2 * collAmount, liquidationAmount, 0, 0, 0); + uint256 BTroveId = borrowerOperations.openTrove(B, 0, 2 * collAmount, liquidationAmount, 0, 0, 0); // Price drops priceFeed.setPrice(1200e18 - 1); @@ -147,15 +147,16 @@ contract LiquidationsLSTTest is DevTestSetup { priceFeed.setPrice(initialPrice); vm.startPrank(A); uint256 ATroveId = borrowerOperations.openTrove( - A, 0, 1e18, collAmount, liquidationAmount - troveManager.BOLD_GAS_COMPENSATION(), 0, 0, 0 + A, 0, collAmount, liquidationAmount - troveManager.BOLD_GAS_COMPENSATION(), 0, 0, 0 ); vm.stopPrank(); vm.startPrank(B); - uint256 BTroveId = borrowerOperations.openTrove(B, 0, 1e18, 3 * collAmount, liquidationAmount, 0, 0, 0); + uint256 BTroveId = borrowerOperations.openTrove(B, 0, 3 * collAmount, liquidationAmount, 0, 0, 0); + vm.stopPrank(); // B deposits to SP if (_spAmount > 0) { - stabilityPool.provideToSP(_spAmount); + makeSPDepositAndClaim(B, _spAmount); } // Price drops diff --git a/contracts/src/test/stabilityPool.t.sol b/contracts/src/test/stabilityPool.t.sol index 03cce139..4c4193f1 100644 --- a/contracts/src/test/stabilityPool.t.sol +++ b/contracts/src/test/stabilityPool.t.sol @@ -81,7 +81,7 @@ contract SPTest is DevTestSetup { _setupStashedAndCurrentETHGains(); // Check A has both stashed and current gains - uint256 stashedETHGain = stabilityPool.stashedETH(A); + stabilityPool.stashedETH(A); makeSPDepositAndClaim(A, 1e18); @@ -274,8 +274,8 @@ contract SPTest is DevTestSetup { _setupStashedAndCurrentETHGains(); // Check A has both stashed and current gains - uint256 stashedETHGain = stabilityPool.stashedETH(A); - uint256 currentETHGain = stabilityPool.getDepositorETHGain(A); + stabilityPool.stashedETH(A); + stabilityPool.getDepositorETHGain(A); makeSPWithdrawalAndClaim(A, 1e18); @@ -358,8 +358,8 @@ contract SPTest is DevTestSetup { _setupStashedAndCurrentETHGains(); // Check A has both stashed and current gains - uint256 stashedETHGain = stabilityPool.stashedETH(A); - uint256 currentETHGain = stabilityPool.getDepositorETHGain(A); + stabilityPool.stashedETH(A); + stabilityPool.getDepositorETHGain(A); uint256 ETHBal_A = WETH.balanceOf(A); assertGt(ETHBal_A, 0); @@ -586,4 +586,4 @@ contract SPTest is DevTestSetup { // // 2) tests for claimAllETHGains (requires deposit data & getter refactor): // - updates recorded deposit value -// - updates deposit snapshots \ No newline at end of file +// - updates deposit snapshots From bbab92a43d18e1ef31855cc4ce4d25c09230caa6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Fingen?= Date: Thu, 9 May 2024 17:22:20 +0100 Subject: [PATCH 10/13] chore: forge fmt --- contracts/src/StabilityPool.sol | 15 ++--- contracts/src/TroveManager.sol | 10 ++-- contracts/src/test/TestContracts/BaseTest.sol | 3 +- .../src/test/TestContracts/DevTestSetup.sol | 2 +- .../src/test/interestRateAggregate.t.sol | 2 +- contracts/src/test/stabilityPool.t.sol | 60 +++++++++---------- 6 files changed, 44 insertions(+), 48 deletions(-) diff --git a/contracts/src/StabilityPool.sol b/contracts/src/StabilityPool.sol index a55ae57f..0f189448 100644 --- a/contracts/src/StabilityPool.sol +++ b/contracts/src/StabilityPool.sol @@ -333,14 +333,15 @@ contract StabilityPool is LiquityBase, Ownable, CheckContract, IStabilityPool { assert(getDepositorETHGain(msg.sender) == 0); } - function _stashOrSendETHGains(address _depositor, uint256 _currentETHGain, uint256 _boldLoss, bool _doClaim) internal { + function _stashOrSendETHGains(address _depositor, uint256 _currentETHGain, uint256 _boldLoss, bool _doClaim) + internal + { if (_doClaim) { // Get the total gain (stashed + current), zero the stashed balance, send total gain to depositor - uint ETHToSend = _getTotalETHGainAndZeroStash(_depositor, _currentETHGain); + uint256 ETHToSend = _getTotalETHGainAndZeroStash(_depositor, _currentETHGain); emit ETHGainWithdrawn(msg.sender, ETHToSend, _boldLoss); // Bold Loss required for event log _sendETHGainToDepositor(ETHToSend); - } else { // Just stash the current gain stashedETH[_depositor] += _currentETHGain; @@ -348,11 +349,11 @@ contract StabilityPool is LiquityBase, Ownable, CheckContract, IStabilityPool { } function _getTotalETHGainAndZeroStash(address _depositor, uint256 _currentETHGain) internal returns (uint256) { - uint256 stashedETHGain = stashedETH[_depositor]; + uint256 stashedETHGain = stashedETH[_depositor]; uint256 totalETHGain = stashedETHGain + _currentETHGain; // TODO: Gas - saves gas when stashedETHGain == 0? - if (stashedETHGain > 0) {stashedETH[_depositor] = 0;} + if (stashedETHGain > 0) stashedETH[_depositor] = 0; return totalETHGain; } @@ -368,7 +369,7 @@ contract StabilityPool is LiquityBase, Ownable, CheckContract, IStabilityPool { // If they have a deposit, update it and update its snapshots if (initialDeposit > 0) { - currentETHGain = getDepositorETHGain(msg.sender); // Only active deposits can have a current ETH gain + currentETHGain = getDepositorETHGain(msg.sender); // Only active deposits can have a current ETH gain uint256 compoundedBoldDeposit = getCompoundedBoldDeposit(msg.sender); boldLoss = initialDeposit - compoundedBoldDeposit; // Needed only for event log @@ -376,7 +377,7 @@ contract StabilityPool is LiquityBase, Ownable, CheckContract, IStabilityPool { _updateDepositAndSnapshots(msg.sender, compoundedBoldDeposit); } - uint256 ETHToSend = _getTotalETHGainAndZeroStash(msg.sender, currentETHGain); + uint256 ETHToSend = _getTotalETHGainAndZeroStash(msg.sender, currentETHGain); _sendETHGainToDepositor(ETHToSend); assert(getDepositorETHGain(msg.sender) == 0); diff --git a/contracts/src/TroveManager.sol b/contracts/src/TroveManager.sol index e6c12a62..98b75b17 100644 --- a/contracts/src/TroveManager.sol +++ b/contracts/src/TroveManager.sol @@ -323,12 +323,10 @@ contract TroveManager is ERC721, LiquityBase, Ownable, ITroveManager { // --- Inner single liquidation functions --- // Liquidate one trove, in Normal Mode. - function _liquidateNormalMode( - IDefaultPool _defaultPool, - uint256 _troveId, - uint256 _boldInStabPool, - uint256 _price - ) internal returns (LiquidationValues memory singleLiquidation) { + function _liquidateNormalMode(IDefaultPool _defaultPool, uint256 _troveId, uint256 _boldInStabPool, uint256 _price) + internal + returns (LiquidationValues memory singleLiquidation) + { address owner = ownerOf(_troveId); LocalVariables_InnerSingleLiquidateFunction memory vars; diff --git a/contracts/src/test/TestContracts/BaseTest.sol b/contracts/src/test/TestContracts/BaseTest.sol index d0cade9c..9aa35668 100644 --- a/contracts/src/test/TestContracts/BaseTest.sol +++ b/contracts/src/test/TestContracts/BaseTest.sol @@ -184,13 +184,12 @@ contract BaseTest is Test { vm.stopPrank(); } - function claimAllETHGains(address _account) public { + function claimAllETHGains(address _account) public { vm.startPrank(_account); stabilityPool.claimAllETHGains(); vm.stopPrank(); } - function closeTrove(address _account, uint256 _troveId) public { vm.startPrank(_account); borrowerOperations.closeTrove(_troveId); diff --git a/contracts/src/test/TestContracts/DevTestSetup.sol b/contracts/src/test/TestContracts/DevTestSetup.sol index f16e2c6c..aaf0bd6b 100644 --- a/contracts/src/test/TestContracts/DevTestSetup.sol +++ b/contracts/src/test/TestContracts/DevTestSetup.sol @@ -146,7 +146,7 @@ contract DevTestSetup is BaseTest { function _setupForSPDepositAdjustments() internal returns (TroveIDs memory) { TroveIDs memory troveIDs; - (troveIDs.A, troveIDs.B, troveIDs.C, troveIDs.D) = _setupForBatchLiquidateTrovesPureOffset(); + (troveIDs.A, troveIDs.B, troveIDs.C, troveIDs.D) = _setupForBatchLiquidateTrovesPureOffset(); // A liquidates C liquidate(A, troveIDs.C); diff --git a/contracts/src/test/interestRateAggregate.t.sol b/contracts/src/test/interestRateAggregate.t.sol index 373bced2..b124cd72 100644 --- a/contracts/src/test/interestRateAggregate.t.sol +++ b/contracts/src/test/interestRateAggregate.t.sol @@ -2173,7 +2173,7 @@ contract InterestRateAggregate is DevTestSetup { } function testClaimAllETHGainsReducesPendingAggInterestTo0() public { - _setupForSPDepositAdjustments(); + _setupForSPDepositAdjustments(); // A stashes first gain makeSPDepositNoClaim(A, 1e18); diff --git a/contracts/src/test/stabilityPool.t.sol b/contracts/src/test/stabilityPool.t.sol index 4c4193f1..ffd24af3 100644 --- a/contracts/src/test/stabilityPool.t.sol +++ b/contracts/src/test/stabilityPool.t.sol @@ -3,8 +3,7 @@ pragma solidity 0.8.18; import "./TestContracts/DevTestSetup.sol"; contract SPTest is DevTestSetup { - - function _setupStashedAndCurrentETHGains() internal { + function _setupStashedAndCurrentETHGains() internal { TroveIDs memory troveIDs = _setupForSPDepositAdjustments(); // A stashes first gain @@ -20,7 +19,6 @@ contract SPTest is DevTestSetup { assertGt(stashedETHGain_A, 0); assertGt(currentETHGain_A, 0); - // Check B has only current gains, no stashed uint256 stashedETHGain_B = stabilityPool.stashedETH(B); uint256 currentETHGain_B = stabilityPool.getDepositorETHGain(B); @@ -46,21 +44,21 @@ contract SPTest is DevTestSetup { assertEq(WETH.balanceOf(A), ETHBal_A + currentETHGain); } - function testProvideToSPWithClaim_WithOnlyCurrentGainsDoesntChangeStashedGain() public { + function testProvideToSPWithClaim_WithOnlyCurrentGainsDoesntChangeStashedGain() public { _setupForSPDepositAdjustments(); uint256 currentETHGain = stabilityPool.getDepositorETHGain(A); assertGt(currentETHGain, 0); - assertEq( stabilityPool.stashedETH(A), 0); + assertEq(stabilityPool.stashedETH(A), 0); makeSPDepositAndClaim(A, 1e18); - assertEq( stabilityPool.stashedETH(A), 0); + assertEq(stabilityPool.stashedETH(A), 0); } function testProvideToSPWithClaim_WithCurrentAndStashedGainsSendsTotalETHGainToDepositor() public { - // A has stashed & current gains, B has only current + // A has stashed & current gains, B has only current _setupStashedAndCurrentETHGains(); // Check A has both stashed and current gains @@ -86,7 +84,7 @@ contract SPTest is DevTestSetup { makeSPDepositAndClaim(A, 1e18); // Check A's stashed balance reduced to 0 - assertEq(stabilityPool.stashedETH(A),0); + assertEq(stabilityPool.stashedETH(A), 0); } function testProvideToSPWithClaim_WithOnlyStashedGainsSendsStashedETHGainToDepositor() public { @@ -125,13 +123,13 @@ contract SPTest is DevTestSetup { makeSPDepositAndClaim(A, 1e18); // Check A's stashed balance reduced to 0 - assertEq(stabilityPool.stashedETH(A),0); + assertEq(stabilityPool.stashedETH(A), 0); } - // --- provideToSP, doClaim == false --- + // --- provideToSP, doClaim == false --- function testProvideToSPNoClaim_WithOnlyCurrentGainsDoesntChangeDepositorETHBalance() public { - _setupForSPDepositAdjustments(); + _setupForSPDepositAdjustments(); uint256 currentETHGain = stabilityPool.getDepositorETHGain(A); assertGt(currentETHGain, 0); @@ -151,7 +149,7 @@ contract SPTest is DevTestSetup { assertGt(currentETHGain, 0); // Check A has no stashed gains - assertEq( stabilityPool.stashedETH(A), 0); + assertEq(stabilityPool.stashedETH(A), 0); makeSPDepositNoClaim(A, 1e18); @@ -171,7 +169,7 @@ contract SPTest is DevTestSetup { } function testProvideToSPNoClaim_WithCurrentAndStashedGainsIncreasedStashedGainByCurrentGain() public { - // A has stashed & current gains, B has only current + // A has stashed & current gains, B has only current _setupStashedAndCurrentETHGains(); // Check A has both stashed and current gains @@ -222,7 +220,7 @@ contract SPTest is DevTestSetup { assertEq(stabilityPool.stashedETH(A), stashedETHGain); } - // --- withdrawFromSP, doClaim == true --- + // --- withdrawFromSP, doClaim == true --- function testWithdrawFromSPWithClaim_WithOnlyCurrentGainsSendsTotalETHGainToDepositor() public { _setupForSPDepositAdjustments(); @@ -240,17 +238,17 @@ contract SPTest is DevTestSetup { assertEq(WETH.balanceOf(A), ETHBal_A + currentETHGain); } - function testWithdrawFromSPWithClaim_WithOnlyCurrentGainsDoesntChangeStashedGain() public { + function testWithdrawFromSPWithClaim_WithOnlyCurrentGainsDoesntChangeStashedGain() public { _setupForSPDepositAdjustments(); uint256 currentETHGain = stabilityPool.getDepositorETHGain(A); assertGt(currentETHGain, 0); - assertEq( stabilityPool.stashedETH(A), 0); + assertEq(stabilityPool.stashedETH(A), 0); makeSPWithdrawalAndClaim(A, 1e18); - assertEq( stabilityPool.stashedETH(A), 0); + assertEq(stabilityPool.stashedETH(A), 0); } function testWithdrawFromSPWithClaim_WithCurrentAndStashedGainsSendsTotalETHGainToDepositor() public { @@ -280,7 +278,7 @@ contract SPTest is DevTestSetup { makeSPWithdrawalAndClaim(A, 1e18); // Check A's stashed balance reduced to 0 - assertEq(stabilityPool.stashedETH(A),0); + assertEq(stabilityPool.stashedETH(A), 0); } function testWithdrawFromSPPWithClaim_WithOnlyStashedGainsSendsStashedETHGainToDepositor() public { @@ -319,10 +317,10 @@ contract SPTest is DevTestSetup { makeSPWithdrawalAndClaim(A, 1e18); // Check A's stashed balance reduced to 0 - assertEq(stabilityPool.stashedETH(A),0); + assertEq(stabilityPool.stashedETH(A), 0); } - // --- withdrawFromSP, doClaim == false --- + // --- withdrawFromSP, doClaim == false --- function testWithdrawFromSPNoClaim_WithOnlyCurrentGainsDoesntChangeDepositorETHBalance() public { _setupForSPDepositAdjustments(); @@ -345,7 +343,7 @@ contract SPTest is DevTestSetup { assertGt(currentETHGain, 0); // Check A has no stashed gains - assertEq( stabilityPool.stashedETH(A), 0); + assertEq(stabilityPool.stashedETH(A), 0); makeSPWithdrawalNoClaim(A, 1e18); @@ -360,7 +358,7 @@ contract SPTest is DevTestSetup { // Check A has both stashed and current gains stabilityPool.stashedETH(A); stabilityPool.getDepositorETHGain(A); - + uint256 ETHBal_A = WETH.balanceOf(A); assertGt(ETHBal_A, 0); @@ -370,7 +368,7 @@ contract SPTest is DevTestSetup { } function testWithdrawFromSPNoClaim_WithCurrentAndStashedGainsIncreasedStashedGainByCurrentGain() public { - // A has stashed & current gains, B has only current + // A has stashed & current gains, B has only current _setupStashedAndCurrentETHGains(); uint256 stashedETHGain = stabilityPool.stashedETH(A); @@ -467,12 +465,12 @@ contract SPTest is DevTestSetup { assertEq(stabilityPool.stashedETH(B), 0); } - function testClaimAllETHGainsForOnlyCurrentETHGainIncreasesUserBalanceByCurrentETHGain() public { + function testClaimAllETHGainsForOnlyCurrentETHGainIncreasesUserBalanceByCurrentETHGain() public { // A has stashed & current gains, B has only current _setupStashedAndCurrentETHGains(); uint256 currentETHGain_B = stabilityPool.getDepositorETHGain(B); - + uint256 ETHBal_B = WETH.balanceOf(B); assertGt(ETHBal_B, 0); @@ -525,9 +523,9 @@ contract SPTest is DevTestSetup { assertEq(WETH.balanceOf(A), ETHBal_A + stashedGain_A + currentGain_A); } - function testClaimAllETHGainsForOnlyStashedETHGainDoesntChangeETHGain() public { + function testClaimAllETHGainsForOnlyStashedETHGainDoesntChangeETHGain() public { _setupForSPDepositAdjustments(); - + // A stashes first gain makeSPDepositNoClaim(A, 1e18); @@ -544,7 +542,7 @@ contract SPTest is DevTestSetup { function testClaimAllETHGainsForOnlyStashedETHGainZerosStashedETHGain() public { _setupForSPDepositAdjustments(); - + // A stashes first gain makeSPDepositNoClaim(A, 1e18); @@ -560,8 +558,8 @@ contract SPTest is DevTestSetup { } function testClaimAllETHGainsForOnlyStashedETHGainIncreasesUserBalanceByStashedETHGain() public { - _setupForSPDepositAdjustments(); - + _setupForSPDepositAdjustments(); + // A stashes first gain makeSPDepositNoClaim(A, 1e18); @@ -583,7 +581,7 @@ contract SPTest is DevTestSetup { // TODO: // 1) claim tests for withdrawETHGainToTrove (if we don't remove it) -// +// // 2) tests for claimAllETHGains (requires deposit data & getter refactor): // - updates recorded deposit value // - updates deposit snapshots From 1d848b73b8eb6e2e7b9a2b8394d98c76add4c996 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Fingen?= Date: Thu, 9 May 2024 18:02:51 +0100 Subject: [PATCH 11/13] test: Fix hardhat test --- contracts/test/OwnershipTest.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/test/OwnershipTest.js b/contracts/test/OwnershipTest.js index 306f5d6b..434f655a 100644 --- a/contracts/test/OwnershipTest.js +++ b/contracts/test/OwnershipTest.js @@ -89,7 +89,7 @@ contract("All Liquity functions with onlyOwner modifier", async (accounts) => { describe("BorrowerOperations", async (accounts) => { it("setAddresses(): reverts when called by non-owner, with wrong addresses, or twice", async () => { - await testDeploymentSetter(borrowerOperations, 8); + await testDeploymentSetter(borrowerOperations, 7); }); }); From a49a34e7ec67d0d61bba11c9a24968e3867bb8a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Fingen?= Date: Wed, 15 May 2024 12:59:31 +0100 Subject: [PATCH 12/13] fix: Fixes after main merge --- contracts/src/test/liquidations.t.sol | 8 ++++---- contracts/src/test/liquidationsLST.t.sol | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/contracts/src/test/liquidations.t.sol b/contracts/src/test/liquidations.t.sol index 13367fd7..259f6356 100644 --- a/contracts/src/test/liquidations.t.sol +++ b/contracts/src/test/liquidations.t.sol @@ -29,7 +29,7 @@ contract LiquidationsTest is DevTestSetup { uint256 AInitialETHBalance = WETH.balanceOf(A); // Check not RM - assertEq(troveManager.checkRecoveryMode(price), false, "System should not be in Recovery Mode"); + assertEq(troveManager.checkBelowCriticalThreshold(price), false, "System should not be below CT"); // Check CR_A < MCR and TCR > CCR assertLt(troveManager.getCurrentICR(ATroveId, price), MCR); @@ -96,7 +96,7 @@ contract LiquidationsTest is DevTestSetup { uint256 initialSPETHBalance = stabilityPool.getETHBalance(); // Check not RM - assertEq(troveManager.checkRecoveryMode(price), false, "System should not be in Recovery Mode"); + assertEq(troveManager.checkBelowCriticalThreshold(price), false, "System should not be below CT"); // Check CR_A < MCR and TCR > CCR assertLt(troveManager.getCurrentICR(ATroveId, price), MCR, "ICR too high"); @@ -152,7 +152,7 @@ contract LiquidationsTest is DevTestSetup { uint256 BInitialColl = troveManager.getTroveEntireColl(BTroveId); // Check not RM - assertEq(troveManager.checkRecoveryMode(price), false, "System should not be in Recovery Mode"); + assertEq(troveManager.checkBelowCriticalThreshold(price), false, "System should not be below CT"); // Check CR_A < MCR and TCR > CCR assertLt(troveManager.getCurrentICR(ATroveId, price), MCR); @@ -225,7 +225,7 @@ contract LiquidationsTest is DevTestSetup { initialValues.BColl = troveManager.getTroveEntireColl(BTroveId); // Check not RM - assertEq(troveManager.checkRecoveryMode(price), false, "System should not be in Recovery Mode"); + assertEq(troveManager.checkBelowCriticalThreshold(price), false, "System should not be below CT"); // Check CR_A < MCR and TCR > CCR assertLt(troveManager.getCurrentICR(ATroveId, price), MCR); diff --git a/contracts/src/test/liquidationsLST.t.sol b/contracts/src/test/liquidationsLST.t.sol index bdf743e8..e3900515 100644 --- a/contracts/src/test/liquidationsLST.t.sol +++ b/contracts/src/test/liquidationsLST.t.sol @@ -67,7 +67,7 @@ contract LiquidationsLSTTest is DevTestSetup { uint256 AInitialETHBalance = WETH.balanceOf(A); // Check not RM - assertEq(troveManager.checkRecoveryMode(price), false, "System should not be in Recovery Mode"); + assertEq(troveManager.checkBelowCriticalThreshold(price), false, "System should not be below CT"); // Check CR_A < MCR and TCR > CCR assertLt(troveManager.getCurrentICR(ATroveId, price), MCR); @@ -171,7 +171,7 @@ contract LiquidationsLSTTest is DevTestSetup { initialValues.BColl = troveManager.getTroveEntireColl(BTroveId); // Check not RM - assertEq(troveManager.checkRecoveryMode(_finalPrice), false, "System should not be in Recovery Mode"); + assertEq(troveManager.checkBelowCriticalThreshold(_finalPrice), false, "System should not be below CT"); // Check CR_A < MCR and TCR > CCR assertLt(troveManager.getCurrentICR(ATroveId, _finalPrice), MCR); From 2de444ee164a6e569d98187ada8c67081e9c9d14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Fingen?= Date: Wed, 15 May 2024 13:00:00 +0100 Subject: [PATCH 13/13] chore: forge fmt --- contracts/src/TroveManager.sol | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/contracts/src/TroveManager.sol b/contracts/src/TroveManager.sol index 51b3b039..79fe5a16 100644 --- a/contracts/src/TroveManager.sol +++ b/contracts/src/TroveManager.sol @@ -549,8 +549,7 @@ contract TroveManager is ERC721, LiquityBase, Ownable, ITroveManager { vars.ICR = getCurrentICR(vars.troveId, _price); if (vars.ICR < MCR) { - singleLiquidation = - _liquidate(_defaultPool, vars.troveId, vars.remainingBoldInStabPool, _price); + singleLiquidation = _liquidate(_defaultPool, vars.troveId, vars.remainingBoldInStabPool, _price); vars.remainingBoldInStabPool = vars.remainingBoldInStabPool - singleLiquidation.debtToOffset; // Add liquidation values to their respective running totals