From e990d59f2b0eb6c0e2fd3932b7325ff9e6c4427c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Fingen?= Date: Tue, 27 Aug 2024 19:09:18 +0100 Subject: [PATCH 01/11] fix: Save 1 SLOAD (Dedaub A10) --- contracts/src/TroveManager.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/src/TroveManager.sol b/contracts/src/TroveManager.sol index f13e4294..f9b35f78 100644 --- a/contracts/src/TroveManager.sol +++ b/contracts/src/TroveManager.sol @@ -423,7 +423,7 @@ contract TroveManager is LiquityBase, ITroveManager, ITroveEvents { revert NothingToLiquidate(); } - activePool.mintAggInterestAndAccountForTroveChange(troveChange, address(0)); + activePoolCached.mintAggInterestAndAccountForTroveChange(troveChange, address(0)); // Move liquidated Coll and Bold to the appropriate pools if (totals.debtToOffset > 0 || totals.collToSendToSP > 0) { From bad0f54b3569cb85f377f5585df38d418ad2529f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Fingen?= Date: Tue, 27 Aug 2024 19:08:27 +0100 Subject: [PATCH 02/11] fix: Dedaub issue A7 --- contracts/src/TroveManager.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/src/TroveManager.sol b/contracts/src/TroveManager.sol index f9b35f78..1e59c2b4 100644 --- a/contracts/src/TroveManager.sol +++ b/contracts/src/TroveManager.sol @@ -982,10 +982,10 @@ contract TroveManager is LiquityBase, ITroveManager, ITroveEvents { uint256 period = _getInterestPeriod(batch.lastDebtUpdateTime); latestBatchData.accruedInterest = _calcInterest(latestBatchData.weightedRecordedDebt, period); latestBatchData.annualManagementFee = batch.annualManagementFee; - latestBatchData.accruedManagementFee = - _calcInterest(latestBatchData.recordedDebt * latestBatchData.annualManagementFee, period); latestBatchData.weightedRecordedBatchManagementFee = latestBatchData.recordedDebt * latestBatchData.annualManagementFee; + latestBatchData.accruedManagementFee = + _calcInterest(latestBatchData.weightedRecordedBatchManagementFee, period); latestBatchData.entireDebtWithoutRedistribution = latestBatchData.recordedDebt + latestBatchData.accruedInterest + latestBatchData.accruedManagementFee; From 459cbd8c769bc3c9c618772ea5d4025e7ab44ce6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Fingen?= Date: Tue, 27 Aug 2024 19:12:40 +0100 Subject: [PATCH 03/11] fix: Minor urgent redemption optimizations (Dedaub A8, A9) --- contracts/src/TroveManager.sol | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/contracts/src/TroveManager.sol b/contracts/src/TroveManager.sol index 1e59c2b4..c55138d2 100644 --- a/contracts/src/TroveManager.sol +++ b/contracts/src/TroveManager.sol @@ -162,6 +162,7 @@ contract TroveManager is LiquityBase, ITroveManager, ITroveEvents { error TroveNotOpen(uint256 _troveId); error OnlyOneTroveLeft(); error NotShutDown(); + error NotEnoughBoldBalance(); error MinCollNotReached(uint256 _coll); // --- Events --- @@ -821,6 +822,7 @@ contract TroveManager is LiquityBase, ITroveManager, ITroveEvents { function urgentRedemption(uint256 _boldAmount, uint256[] calldata _troveIds, uint256 _minCollateral) external { _requireIsShutDown(); + _requireBoldBalanceCoversRedemption(boldToken, msg.sender, _boldAmount); IActivePool activePoolCached = activePool; TroveChange memory totalsTroveChange; @@ -851,6 +853,7 @@ contract TroveManager is LiquityBase, ITroveManager, ITroveEvents { totalsTroveChange.oldWeightedRecordedDebt += singleRedemption.oldWeightedRecordedDebt; remainingBold -= singleRedemption.boldLot; + if (remainingBold == 0) break; } if (totalsTroveChange.collDecrease < _minCollateral) { @@ -1138,6 +1141,16 @@ contract TroveManager is LiquityBase, ITroveManager, ITroveEvents { } } + function _requireBoldBalanceCoversRedemption(IBoldToken _boldToken, address _redeemer, uint256 _amount) + internal + view + { + uint256 boldBalance = _boldToken.balanceOf(_redeemer); + if (boldBalance < _amount) { + revert NotEnoughBoldBalance(); + } + } + // --- Trove property getters --- function getUnbackedPortionPriceAndRedeemability() external returns (uint256, uint256, bool) { From c2fe15c71fa743a60bef41ec6547fb841147b851 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Fingen?= Date: Wed, 28 Aug 2024 09:45:04 +0100 Subject: [PATCH 04/11] fix: Dedaub issues A2, A4, A5, A6 --- contracts/src/BorrowerOperations.sol | 11 +----- contracts/src/CollSurplusPool.sol | 6 ++-- contracts/src/CollateralRegistry.sol | 7 ++-- contracts/src/DefaultPool.sol | 9 +++-- contracts/src/Dependencies/LiquityBase.sol | 16 +++++++-- contracts/src/GasPool.sol | 2 +- contracts/src/SortedTroves.sol | 4 +-- contracts/src/StabilityPool.sol | 21 ++++-------- contracts/src/TroveManager.sol | 11 +----- contracts/src/TroveNFT.sol | 2 +- contracts/src/test/stabilityPool.t.sol | 40 +++++++++++----------- 11 files changed, 57 insertions(+), 72 deletions(-) diff --git a/contracts/src/BorrowerOperations.sol b/contracts/src/BorrowerOperations.sol index f6cbbaa4..ba9872c4 100644 --- a/contracts/src/BorrowerOperations.sol +++ b/contracts/src/BorrowerOperations.sol @@ -151,18 +151,15 @@ contract BorrowerOperations is LiquityBase, AddRemoveManagers, IBorrowerOperatio error MinInterestRateChangePeriodTooLow(); event TroveManagerAddressChanged(address _newTroveManagerAddress); - event ActivePoolAddressChanged(address _activePoolAddress); - event DefaultPoolAddressChanged(address _defaultPoolAddress); event GasPoolAddressChanged(address _gasPoolAddress); event CollSurplusPoolAddressChanged(address _collSurplusPoolAddress); - event PriceFeedAddressChanged(address _newPriceFeedAddress); event SortedTrovesAddressChanged(address _sortedTrovesAddress); event BoldTokenAddressChanged(address _boldTokenAddress); event ShutDown(uint256 _tcr); event ShutDownFromOracleFailure(address _oracleAddress); - constructor(IAddressesRegistry _addressesRegistry) AddRemoveManagers(_addressesRegistry) { + constructor(IAddressesRegistry _addressesRegistry) AddRemoveManagers(_addressesRegistry) LiquityBase(_addressesRegistry) { // This makes impossible to open a trove with zero withdrawn Bold assert(MIN_DEBT > 0); @@ -175,20 +172,14 @@ contract BorrowerOperations is LiquityBase, AddRemoveManagers, IBorrowerOperatio MCR = _addressesRegistry.MCR(); troveManager = _addressesRegistry.troveManager(); - activePool = _addressesRegistry.activePool(); - defaultPool = _addressesRegistry.defaultPool(); gasPoolAddress = _addressesRegistry.gasPoolAddress(); collSurplusPool = _addressesRegistry.collSurplusPool(); - priceFeed = _addressesRegistry.priceFeed(); sortedTroves = _addressesRegistry.sortedTroves(); boldToken = _addressesRegistry.boldToken(); emit TroveManagerAddressChanged(address(troveManager)); - emit ActivePoolAddressChanged(address(activePool)); - emit DefaultPoolAddressChanged(address(defaultPool)); emit GasPoolAddressChanged(gasPoolAddress); emit CollSurplusPoolAddressChanged(address(collSurplusPool)); - emit PriceFeedAddressChanged(address(priceFeed)); emit SortedTrovesAddressChanged(address(sortedTroves)); emit BoldTokenAddressChanged(address(boldToken)); diff --git a/contracts/src/CollSurplusPool.sol b/contracts/src/CollSurplusPool.sol index a53d7ceb..fcad84e1 100644 --- a/contracts/src/CollSurplusPool.sol +++ b/contracts/src/CollSurplusPool.sol @@ -13,9 +13,9 @@ contract CollSurplusPool is ICollSurplusPool { string public constant NAME = "CollSurplusPool"; IERC20 public immutable collToken; - address public borrowerOperationsAddress; - address public troveManagerAddress; - address public activePoolAddress; + address public immutable borrowerOperationsAddress; + address public immutable troveManagerAddress; + address public immutable activePoolAddress; // deposited ether tracker uint256 internal collBalance; diff --git a/contracts/src/CollateralRegistry.sol b/contracts/src/CollateralRegistry.sol index f7bb110f..cf288187 100644 --- a/contracts/src/CollateralRegistry.sol +++ b/contracts/src/CollateralRegistry.sol @@ -6,13 +6,14 @@ import "openzeppelin-contracts/contracts/token/ERC20/IERC20.sol"; import "./Interfaces/ITroveManager.sol"; import "./Interfaces/IBoldToken.sol"; -import "./Dependencies/LiquityBase.sol"; +import "./Dependencies/Constants.sol"; +import "./Dependencies/LiquityMath.sol"; import "./Interfaces/ICollateralRegistry.sol"; // import "forge-std/console2.sol"; -contract CollateralRegistry is LiquityBase, ICollateralRegistry { +contract CollateralRegistry is ICollateralRegistry { // mapping from Collateral token address to the corresponding TroveManagers //mapping(address => address) troveManagers; // See: https://github.com/ethereum/solidity/issues/12587 @@ -258,7 +259,7 @@ contract CollateralRegistry is LiquityBase, ICollateralRegistry { return _calcRedemptionFee(getRedemptionRateWithDecay(), _ETHDrawn); } - function getEffectiveRedemptionFeeInBold(uint256 _redeemAmount) public view override returns (uint256) { + function getEffectiveRedemptionFeeInBold(uint256 _redeemAmount) external view override returns (uint256) { uint256 totalBoldSupply = boldToken.totalSupply(); uint256 newBaseRate = _getUpdatedBaseRateFromRedemption(_redeemAmount, totalBoldSupply); return _calcRedemptionFee(_calcRedemptionRate(newBaseRate), _redeemAmount); diff --git a/contracts/src/DefaultPool.sol b/contracts/src/DefaultPool.sol index 207a09e9..aeb3871e 100644 --- a/contracts/src/DefaultPool.sol +++ b/contracts/src/DefaultPool.sol @@ -23,8 +23,8 @@ contract DefaultPool is IDefaultPool { string public constant NAME = "DefaultPool"; IERC20 public immutable collToken; - address public troveManagerAddress; - address public activePoolAddress; + address public immutable troveManagerAddress; + address public immutable activePoolAddress; uint256 internal collBalance; // deposited Coll tracker uint256 internal BoldDebt; // debt @@ -67,14 +67,13 @@ contract DefaultPool is IDefaultPool { function sendCollToActivePool(uint256 _amount) external override { _requireCallerIsTroveManager(); - address activePool = activePoolAddress; // cache to save an SLOAD uint256 newCollBalance = collBalance - _amount; collBalance = newCollBalance; emit DefaultPoolCollBalanceUpdated(newCollBalance); - emit EtherSent(activePool, _amount); + emit EtherSent(activePoolAddress, _amount); // Send Coll to Active Pool and increase its recorded Coll balance - IActivePool(activePool).receiveColl(_amount); + IActivePool(activePoolAddress).receiveColl(_amount); } function receiveColl(uint256 _amount) external { diff --git a/contracts/src/Dependencies/LiquityBase.sol b/contracts/src/Dependencies/LiquityBase.sol index 1bc7b82f..4fa99921 100644 --- a/contracts/src/Dependencies/LiquityBase.sol +++ b/contracts/src/Dependencies/LiquityBase.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.18; import "./Constants.sol"; import "./LiquityMath.sol"; +import "../Interfaces/IAddressesRegistry.sol"; import "../Interfaces/IActivePool.sol"; import "../Interfaces/IDefaultPool.sol"; import "../Interfaces/IPriceFeed.sol"; @@ -17,11 +18,22 @@ import "../Interfaces/ILiquityBase.sol"; */ contract LiquityBase is ILiquityBase { IActivePool public activePool; - IDefaultPool internal defaultPool; - IPriceFeed internal priceFeed; + event ActivePoolAddressChanged(address _newActivePoolAddress); + event DefaultPoolAddressChanged(address _newDefaultPoolAddress); + event PriceFeedAddressChanged(address _newPriceFeedAddress); + + constructor(IAddressesRegistry _addressesRegistry) { + activePool = _addressesRegistry.activePool(); + defaultPool = _addressesRegistry.defaultPool(); + priceFeed = _addressesRegistry.priceFeed(); + + emit ActivePoolAddressChanged(address(activePool)); + emit DefaultPoolAddressChanged(address(defaultPool)); + emit PriceFeedAddressChanged(address(priceFeed)); + } // --- Gas compensation functions --- function getEntireSystemColl() public view returns (uint256 entireSystemColl) { diff --git a/contracts/src/GasPool.sol b/contracts/src/GasPool.sol index db3bb8a7..8850acd7 100644 --- a/contracts/src/GasPool.sol +++ b/contracts/src/GasPool.sol @@ -9,7 +9,7 @@ import "./Interfaces/ITroveManager.sol"; /** * The purpose of this contract is to hold WETH tokens for gas compensation: - * https://github.com/liquity/dev#gas-compensation + * https://github.com/liquity/bold/?tab=readme-ov-file#liquidation-gas-compensation * When a borrower opens a trove, an additional amount of WETH is pulled, * and sent to this contract. * When a borrower closes their active trove, this gas compensation is refunded diff --git a/contracts/src/SortedTroves.sol b/contracts/src/SortedTroves.sol index d20e0e53..df65cd3f 100644 --- a/contracts/src/SortedTroves.sol +++ b/contracts/src/SortedTroves.sol @@ -45,8 +45,8 @@ contract SortedTroves is ISortedTroves { event TroveManagerAddressChanged(address _troveManagerAddress); event BorrowerOperationsAddressChanged(address _borrowerOperationsAddress); - address public borrowerOperationsAddress; - ITroveManager public troveManager; + address public immutable borrowerOperationsAddress; + ITroveManager public immutable troveManager; // Information for a node in the list struct Node { diff --git a/contracts/src/StabilityPool.sol b/contracts/src/StabilityPool.sol index 47f6a2fd..895ef9a4 100644 --- a/contracts/src/StabilityPool.sol +++ b/contracts/src/StabilityPool.sol @@ -136,11 +136,11 @@ contract StabilityPool is LiquityBase, IStabilityPool, IStabilityPoolEvents { string public constant NAME = "StabilityPool"; IERC20 public immutable collToken; - IBorrowerOperations public borrowerOperations; - ITroveManager public troveManager; - IBoldToken public boldToken; + IBorrowerOperations public immutable borrowerOperations; + ITroveManager public immutable troveManager; + IBoldToken public immutable boldToken; // Needed to check if there are pending liquidations - ISortedTroves public sortedTroves; + ISortedTroves public immutable sortedTroves; uint256 internal collBalance; // deposited ether tracker @@ -208,27 +208,20 @@ contract StabilityPool is LiquityBase, IStabilityPool, IStabilityPoolEvents { event BorrowerOperationsAddressChanged(address _newBorrowerOperationsAddress); event TroveManagerAddressChanged(address _newTroveManagerAddress); - event ActivePoolAddressChanged(address _newActivePoolAddress); - event DefaultPoolAddressChanged(address _newDefaultPoolAddress); event BoldTokenAddressChanged(address _newBoldTokenAddress); event SortedTrovesAddressChanged(address _newSortedTrovesAddress); - event PriceFeedAddressChanged(address _newPriceFeedAddress); - constructor(IAddressesRegistry _addressesRegistry) { + constructor(IAddressesRegistry _addressesRegistry) LiquityBase(_addressesRegistry) { collToken = _addressesRegistry.collToken(); borrowerOperations = _addressesRegistry.borrowerOperations(); troveManager = _addressesRegistry.troveManager(); - activePool = _addressesRegistry.activePool(); boldToken = _addressesRegistry.boldToken(); sortedTroves = _addressesRegistry.sortedTroves(); - priceFeed = _addressesRegistry.priceFeed(); emit BorrowerOperationsAddressChanged(address(borrowerOperations)); emit TroveManagerAddressChanged(address(troveManager)); - emit ActivePoolAddressChanged(address(activePool)); emit BoldTokenAddressChanged(address(boldToken)); emit SortedTrovesAddressChanged(address(sortedTroves)); - emit PriceFeedAddressChanged(address(priceFeed)); // Allow funds movements between Liquity contracts collToken.approve(address(borrowerOperations), type(uint256).max); @@ -539,8 +532,6 @@ contract StabilityPool is LiquityBase, IStabilityPool, IStabilityPoolEvents { } function _moveOffsetCollAndDebt(uint256 _collToAdd, uint256 _debtToOffset) internal { - IActivePool activePoolCached = activePool; - // Cancel the liquidated Bold debt with the Bold in the stability pool _updateTotalBoldDeposits(0, _debtToOffset); @@ -552,7 +543,7 @@ contract StabilityPool is LiquityBase, IStabilityPool, IStabilityPoolEvents { collBalance = newCollBalance; // Pull Coll from Active Pool - activePoolCached.sendColl(address(this), _collToAdd); + activePool.sendColl(address(this), _collToAdd); emit StabilityPoolCollBalanceUpdated(newCollBalance); } diff --git a/contracts/src/TroveManager.sol b/contracts/src/TroveManager.sol index c55138d2..a0f9d9f4 100644 --- a/contracts/src/TroveManager.sol +++ b/contracts/src/TroveManager.sol @@ -169,17 +169,14 @@ contract TroveManager is LiquityBase, ITroveManager, ITroveEvents { event TroveNFTAddressChanged(address _newTroveNFTAddress); event BorrowerOperationsAddressChanged(address _newBorrowerOperationsAddress); - event PriceFeedAddressChanged(address _newPriceFeedAddress); event BoldTokenAddressChanged(address _newBoldTokenAddress); - event ActivePoolAddressChanged(address _activePoolAddress); - event DefaultPoolAddressChanged(address _defaultPoolAddress); event StabilityPoolAddressChanged(address _stabilityPoolAddress); event GasPoolAddressChanged(address _gasPoolAddress); event CollSurplusPoolAddressChanged(address _collSurplusPoolAddress); event SortedTrovesAddressChanged(address _sortedTrovesAddress); event CollateralRegistryAddressChanged(address _collateralRegistryAddress); - constructor(IAddressesRegistry _addressesRegistry) { + constructor(IAddressesRegistry _addressesRegistry) LiquityBase(_addressesRegistry) { CCR = _addressesRegistry.CCR(); MCR = _addressesRegistry.MCR(); SCR = _addressesRegistry.SCR(); @@ -188,12 +185,9 @@ contract TroveManager is LiquityBase, ITroveManager, ITroveEvents { troveNFT = _addressesRegistry.troveNFT(); borrowerOperations = _addressesRegistry.borrowerOperations(); - activePool = _addressesRegistry.activePool(); - defaultPool = _addressesRegistry.defaultPool(); stabilityPool = _addressesRegistry.stabilityPool(); gasPoolAddress = _addressesRegistry.gasPoolAddress(); collSurplusPool = _addressesRegistry.collSurplusPool(); - priceFeed = _addressesRegistry.priceFeed(); boldToken = _addressesRegistry.boldToken(); sortedTroves = _addressesRegistry.sortedTroves(); WETH = _addressesRegistry.WETH(); @@ -201,12 +195,9 @@ contract TroveManager is LiquityBase, ITroveManager, ITroveEvents { emit TroveNFTAddressChanged(address(troveNFT)); emit BorrowerOperationsAddressChanged(address(borrowerOperations)); - emit ActivePoolAddressChanged(address(activePool)); - emit DefaultPoolAddressChanged(address(defaultPool)); emit StabilityPoolAddressChanged(address(stabilityPool)); emit GasPoolAddressChanged(gasPoolAddress); emit CollSurplusPoolAddressChanged(address(collSurplusPool)); - emit PriceFeedAddressChanged(address(priceFeed)); emit BoldTokenAddressChanged(address(boldToken)); emit SortedTrovesAddressChanged(address(sortedTroves)); emit CollateralRegistryAddressChanged(address(collateralRegistry)); diff --git a/contracts/src/TroveNFT.sol b/contracts/src/TroveNFT.sol index e774613d..92b62aa7 100644 --- a/contracts/src/TroveNFT.sol +++ b/contracts/src/TroveNFT.sol @@ -13,7 +13,7 @@ contract TroveNFT is ERC721, ITroveNFT { string internal constant NAME = "TroveNFT"; // TODO string internal constant SYMBOL = "Lv2T"; // TODO - ITroveManager public troveManager; + ITroveManager public immutable troveManager; constructor(IAddressesRegistry _addressesRegistry) ERC721(NAME, SYMBOL) { troveManager = _addressesRegistry.troveManager(); diff --git a/contracts/src/test/stabilityPool.t.sol b/contracts/src/test/stabilityPool.t.sol index d8c870ff..9b177da6 100644 --- a/contracts/src/test/stabilityPool.t.sol +++ b/contracts/src/test/stabilityPool.t.sol @@ -1758,13 +1758,13 @@ contract SPTest is DevTestSetup { // Cheat 1: manipulate contract state to make value of P low vm.store( address(stabilityPool), - bytes32(uint256(13)), // 13th storage slot where P is stored + bytes32(uint256(9)), // 9th storage slot where P is stored bytes32(uint256(_cheatP)) ); - // Confirm that storage slot 13 is set - uint256 storedVal = uint256(vm.load(address(stabilityPool), bytes32(uint256(13)))); - assertEq(storedVal, _cheatP, "value of slot 13 is not set"); + // Confirm that storage slot 9 is set + uint256 storedVal = uint256(vm.load(address(stabilityPool), bytes32(uint256(9)))); + assertEq(storedVal, _cheatP, "value of slot 9 is not set"); // Confirm that P specfically is set assertEq(stabilityPool.P(), _cheatP, "P is not set"); @@ -1803,13 +1803,13 @@ contract SPTest is DevTestSetup { // Cheat 1: manipulate contract state to make value of P low vm.store( address(stabilityPool), - bytes32(uint256(13)), // 13th storage slot where P is stored + bytes32(uint256(9)), // 9th storage slot where P is stored bytes32(uint256(_cheatP)) ); - // Confirm that storage slot 13 is set - uint256 storedVal = uint256(vm.load(address(stabilityPool), bytes32(uint256(13)))); - assertEq(storedVal, _cheatP, "value of slot 13 is not set"); + // Confirm that storage slot 9 is set + uint256 storedVal = uint256(vm.load(address(stabilityPool), bytes32(uint256(9)))); + assertEq(storedVal, _cheatP, "value of slot 9 is not set"); // Confirm that P specfically is set assertEq(stabilityPool.P(), _cheatP, "P is not set"); @@ -1923,13 +1923,13 @@ contract SPTest is DevTestSetup { // Cheat 1: manipulate contract state to make value of P low vm.store( address(stabilityPool), - bytes32(uint256(13)), // 13th storage slot where P is stored + bytes32(uint256(9)), // 9th storage slot where P is stored bytes32(uint256(_cheatP)) ); - // Confirm that storage slot 13 is set - uint256 storedVal = uint256(vm.load(address(stabilityPool), bytes32(uint256(13)))); - assertEq(storedVal, _cheatP, "value of slot 13 is not set"); + // Confirm that storage slot 9 is set + uint256 storedVal = uint256(vm.load(address(stabilityPool), bytes32(uint256(9)))); + assertEq(storedVal, _cheatP, "value of slot 9 is not set"); // Confirm that P specfically is set assertEq(stabilityPool.P(), _cheatP, "P is not set"); @@ -2040,13 +2040,13 @@ contract SPTest is DevTestSetup { // Cheat 1: manipulate contract state to make value of P low vm.store( address(stabilityPool), - bytes32(uint256(13)), // 13th storage slot where P is stored + bytes32(uint256(9)), // 9th storage slot where P is stored bytes32(uint256(_cheatP)) ); - // Confirm that storage slot 13 is set - uint256 storedVal = uint256(vm.load(address(stabilityPool), bytes32(uint256(13)))); - assertEq(storedVal, _cheatP, "value of slot 13 is not set"); + // Confirm that storage slot 9 is set + uint256 storedVal = uint256(vm.load(address(stabilityPool), bytes32(uint256(9)))); + assertEq(storedVal, _cheatP, "value of slot 9 is not set"); // Confirm that P specfically is set console2.log(stabilityPool.P(), "stabilityPool.P()"); console2.log(_cheatP, "_cheatP"); @@ -2080,13 +2080,13 @@ contract SPTest is DevTestSetup { // Cheat 1: manipulate contract state to make value of P low vm.store( address(stabilityPool), - bytes32(uint256(13)), // 13th storage slot where P is stored + bytes32(uint256(9)), // 9th storage slot where P is stored bytes32(uint256(_cheatP)) ); - // Confirm that storage slot 13 is set - uint256 storedVal = uint256(vm.load(address(stabilityPool), bytes32(uint256(13)))); - assertEq(storedVal, _cheatP, "value of slot 13 is not set"); + // Confirm that storage slot 9 is set + uint256 storedVal = uint256(vm.load(address(stabilityPool), bytes32(uint256(9)))); + assertEq(storedVal, _cheatP, "value of slot 9 is not set"); // Confirm that P specfically is set assertEq(stabilityPool.P(), _cheatP, "P is not set"); From 94d90827367d001874f4161e80d00cbab28df38d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Fingen?= Date: Wed, 28 Aug 2024 10:03:00 +0100 Subject: [PATCH 05/11] fix: Remove unused ActivePool from CollSurplusPool --- contracts/src/CollSurplusPool.sol | 4 ---- 1 file changed, 4 deletions(-) diff --git a/contracts/src/CollSurplusPool.sol b/contracts/src/CollSurplusPool.sol index fcad84e1..86508344 100644 --- a/contracts/src/CollSurplusPool.sol +++ b/contracts/src/CollSurplusPool.sol @@ -15,7 +15,6 @@ contract CollSurplusPool is ICollSurplusPool { IERC20 public immutable collToken; address public immutable borrowerOperationsAddress; address public immutable troveManagerAddress; - address public immutable activePoolAddress; // deposited ether tracker uint256 internal collBalance; @@ -26,7 +25,6 @@ contract CollSurplusPool is ICollSurplusPool { event BorrowerOperationsAddressChanged(address _newBorrowerOperationsAddress); event TroveManagerAddressChanged(address _newTroveManagerAddress); - event ActivePoolAddressChanged(address _newActivePoolAddress); event CollBalanceUpdated(address indexed _account, uint256 _newBalance); event CollSent(address _to, uint256 _amount); @@ -35,11 +33,9 @@ contract CollSurplusPool is ICollSurplusPool { collToken = _addressesRegistry.collToken(); borrowerOperationsAddress = address(_addressesRegistry.borrowerOperations()); troveManagerAddress = address(_addressesRegistry.troveManager()); - activePoolAddress = address(_addressesRegistry.activePool()); emit BorrowerOperationsAddressChanged(borrowerOperationsAddress); emit TroveManagerAddressChanged(troveManagerAddress); - emit ActivePoolAddressChanged(activePoolAddress); } /* Returns the collBalance state variable From 5abc4c4e4654c708efc1b9a3495cbaa4adbd0cbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Fingen?= Date: Wed, 28 Aug 2024 10:03:33 +0100 Subject: [PATCH 06/11] fix: Make addresses immutable in ActivePool --- contracts/src/ActivePool.sol | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/contracts/src/ActivePool.sol b/contracts/src/ActivePool.sol index 2d665878..62376420 100644 --- a/contracts/src/ActivePool.sol +++ b/contracts/src/ActivePool.sol @@ -27,9 +27,9 @@ contract ActivePool is IActivePool { string public constant NAME = "ActivePool"; IERC20 public immutable collToken; - address public borrowerOperationsAddress; - address public troveManagerAddress; - address public defaultPoolAddress; + address public immutable borrowerOperationsAddress; + address public immutable troveManagerAddress; + address public immutable defaultPoolAddress; IBoldToken boldToken; @@ -173,10 +173,9 @@ contract ActivePool is IActivePool { function sendCollToDefaultPool(uint256 _amount) external override { _requireCallerIsTroveManager(); - address defaultPoolAddressCached = defaultPoolAddress; - _accountForSendColl(defaultPoolAddressCached, _amount); + _accountForSendColl(defaultPoolAddress, _amount); - IDefaultPool(defaultPoolAddressCached).receiveColl(_amount); + IDefaultPool(defaultPoolAddress).receiveColl(_amount); } function _accountForSendColl(address _account, uint256 _amount) internal { From dd887ee2a27bd2b6a803c225cfcd1f92c504e62d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Fingen?= Date: Wed, 28 Aug 2024 10:04:03 +0100 Subject: [PATCH 07/11] fix: Remove unused BorrowerOperations from StabilityPool --- contracts/src/Interfaces/IStabilityPool.sol | 2 -- contracts/src/Interfaces/ITroveManager.sol | 1 + contracts/src/StabilityPool.sol | 9 --------- contracts/src/test/deployment.t.sol | 6 ------ 4 files changed, 1 insertion(+), 17 deletions(-) diff --git a/contracts/src/Interfaces/IStabilityPool.sol b/contracts/src/Interfaces/IStabilityPool.sol index 370c9b57..d4d7d567 100644 --- a/contracts/src/Interfaces/IStabilityPool.sol +++ b/contracts/src/Interfaces/IStabilityPool.sol @@ -4,7 +4,6 @@ pragma solidity ^0.8.0; import "./IActivePool.sol"; import "./ILiquityBase.sol"; -import "./IBorrowerOperations.sol"; import "./IBoldToken.sol"; import "./ITroveManager.sol"; import "./IBoldRewardsReceiver.sol"; @@ -30,7 +29,6 @@ import "./IBoldRewardsReceiver.sol"; * */ interface IStabilityPool is ILiquityBase, IBoldRewardsReceiver { - function borrowerOperations() external view returns (IBorrowerOperations); function boldToken() external view returns (IBoldToken); function troveManager() external view returns (ITroveManager); diff --git a/contracts/src/Interfaces/ITroveManager.sol b/contracts/src/Interfaces/ITroveManager.sol index 73cac44b..e5a62631 100644 --- a/contracts/src/Interfaces/ITroveManager.sol +++ b/contracts/src/Interfaces/ITroveManager.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.0; import "./ILiquityBase.sol"; import "./ITroveNFT.sol"; +import "./IBorrowerOperations.sol"; import "./IStabilityPool.sol"; import "./IBoldToken.sol"; import "./ISortedTroves.sol"; diff --git a/contracts/src/StabilityPool.sol b/contracts/src/StabilityPool.sol index 895ef9a4..f0c55dd3 100644 --- a/contracts/src/StabilityPool.sol +++ b/contracts/src/StabilityPool.sol @@ -6,9 +6,7 @@ import "openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol"; import "./Interfaces/IStabilityPool.sol"; import "./Interfaces/IAddressesRegistry.sol"; -import "./Interfaces/IBorrowerOperations.sol"; import "./Interfaces/IStabilityPoolEvents.sol"; -import "./Interfaces/IBorrowerOperations.sol"; import "./Interfaces/ITroveManager.sol"; import "./Interfaces/IBoldToken.sol"; import "./Interfaces/ISortedTroves.sol"; @@ -136,7 +134,6 @@ contract StabilityPool is LiquityBase, IStabilityPool, IStabilityPoolEvents { string public constant NAME = "StabilityPool"; IERC20 public immutable collToken; - IBorrowerOperations public immutable borrowerOperations; ITroveManager public immutable troveManager; IBoldToken public immutable boldToken; // Needed to check if there are pending liquidations @@ -206,25 +203,19 @@ contract StabilityPool is LiquityBase, IStabilityPool, IStabilityPoolEvents { // --- Events --- - event BorrowerOperationsAddressChanged(address _newBorrowerOperationsAddress); event TroveManagerAddressChanged(address _newTroveManagerAddress); event BoldTokenAddressChanged(address _newBoldTokenAddress); event SortedTrovesAddressChanged(address _newSortedTrovesAddress); constructor(IAddressesRegistry _addressesRegistry) LiquityBase(_addressesRegistry) { collToken = _addressesRegistry.collToken(); - borrowerOperations = _addressesRegistry.borrowerOperations(); troveManager = _addressesRegistry.troveManager(); boldToken = _addressesRegistry.boldToken(); sortedTroves = _addressesRegistry.sortedTroves(); - emit BorrowerOperationsAddressChanged(address(borrowerOperations)); emit TroveManagerAddressChanged(address(troveManager)); emit BoldTokenAddressChanged(address(boldToken)); emit SortedTrovesAddressChanged(address(sortedTroves)); - - // Allow funds movements between Liquity contracts - collToken.approve(address(borrowerOperations), type(uint256).max); } // --- Getters for public variables. Required by IPool interface --- diff --git a/contracts/src/test/deployment.t.sol b/contracts/src/test/deployment.t.sol index 6d0d64bf..2dbfa495 100644 --- a/contracts/src/test/deployment.t.sol +++ b/contracts/src/test/deployment.t.sol @@ -110,12 +110,6 @@ contract Deployment is DevTestSetup { assertEq(activePoolAddress, recordedActivePoolAddress); } - function testStabilityPoolHasCorrectCorrectBorrowerOpsAddress() public view { - address borrowerOperationsAddress = address(borrowerOperations); - address recordedBorrowerOperationsAddress = address(stabilityPool.borrowerOperations()); - assertEq(borrowerOperationsAddress, recordedBorrowerOperationsAddress); - } - function testStabilityPoolHasCorrectCorrectBoldTokenAddress() public view { address boldTokenAddress = address(boldToken); address recordedBoldTokenAddress = address(stabilityPool.boldToken()); From 8d48abba2a0cdde55a6b586ede401f7a71b372f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Fingen?= Date: Wed, 28 Aug 2024 11:23:41 +0100 Subject: [PATCH 08/11] fix: Turn division into multiplication (Debaub A12) --- contracts/src/BorrowerOperations.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/src/BorrowerOperations.sol b/contracts/src/BorrowerOperations.sol index ba9872c4..7f4ba202 100644 --- a/contracts/src/BorrowerOperations.sol +++ b/contracts/src/BorrowerOperations.sol @@ -1413,7 +1413,7 @@ contract BorrowerOperations is LiquityBase, AddRemoveManagers, IBorrowerOperatio } function _requireDebtRepaymentGeCollWithdrawal(TroveChange memory _troveChange, uint256 _price) internal pure { - if ((_troveChange.debtDecrease < _troveChange.collDecrease * _price / DECIMAL_PRECISION)) { + if ((_troveChange.debtDecrease * DECIMAL_PRECISION < _troveChange.collDecrease * _price)) { revert RepaymentNotMatchingCollWithdrawal(); } } From 9c0c99408bfe65920b050d45ff56de023564ef53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Fingen?= Date: Wed, 28 Aug 2024 11:26:58 +0100 Subject: [PATCH 09/11] fix: Remove unused error TroveNotOpen (Dedaub A14) --- contracts/src/TroveManager.sol | 1 - contracts/src/test/TestContracts/InvariantsTestHandler.t.sol | 4 ---- 2 files changed, 5 deletions(-) diff --git a/contracts/src/TroveManager.sol b/contracts/src/TroveManager.sol index a0f9d9f4..a4c95560 100644 --- a/contracts/src/TroveManager.sol +++ b/contracts/src/TroveManager.sol @@ -159,7 +159,6 @@ contract TroveManager is LiquityBase, ITroveManager, ITroveEvents { error NothingToLiquidate(); error CallerNotBorrowerOperations(); error CallerNotCollateralRegistry(); - error TroveNotOpen(uint256 _troveId); error OnlyOneTroveLeft(); error NotShutDown(); error NotEnoughBoldBalance(); diff --git a/contracts/src/test/TestContracts/InvariantsTestHandler.t.sol b/contracts/src/test/TestContracts/InvariantsTestHandler.t.sol index 4e598ae0..7a2be249 100644 --- a/contracts/src/test/TestContracts/InvariantsTestHandler.t.sol +++ b/contracts/src/test/TestContracts/InvariantsTestHandler.t.sol @@ -2906,10 +2906,6 @@ contract InvariantsTestHandler is BaseHandler, BaseMultiCollateralTest { if (revertData.length == 4 + 32) { bytes32 param = bytes32(revertData.slice(4)); - if (selector == TroveManager.TroveNotOpen.selector) { - return (selector, string.concat("TroveManager.TroveNotOpen(", uint256(param).toString(), ")")); - } - if (selector == TroveManager.MinCollNotReached.selector) { return (selector, string.concat("TroveManager.MinCollNotReached(", uint256(param).decimal(), ")")); } From 5cb0e86383be1040d84662bf3c4920b31fb1b738 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Fingen?= Date: Wed, 28 Aug 2024 11:27:39 +0100 Subject: [PATCH 10/11] fix: Remove unused function from StabilityPool (Dedaub A15) --- contracts/src/StabilityPool.sol | 4 ---- 1 file changed, 4 deletions(-) diff --git a/contracts/src/StabilityPool.sol b/contracts/src/StabilityPool.sol index f0c55dd3..91927602 100644 --- a/contracts/src/StabilityPool.sol +++ b/contracts/src/StabilityPool.sol @@ -792,8 +792,4 @@ contract StabilityPool is LiquityBase, IStabilityPool, IStabilityPoolEvents { function _requireNonZeroAmount(uint256 _amount) internal pure { require(_amount > 0, "StabilityPool: Amount must be non-zero"); } - - function _requireValidKickbackRate(uint256 _kickbackRate) internal pure { - require(_kickbackRate <= DECIMAL_PRECISION, "StabilityPool: Kickback rate must be in range [0,1]"); - } } From b8da9d3c071b6f5fea86a2ace0170e5910acff98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Fingen?= Date: Wed, 28 Aug 2024 11:33:54 +0100 Subject: [PATCH 11/11] fix: Remove return value from closeTrove (Dedaub A16) --- contracts/src/BorrowerOperations.sol | 4 +--- contracts/src/Interfaces/IBorrowerOperations.sol | 2 +- contracts/src/Zappers/GasCompZapper.sol | 4 ++-- contracts/src/Zappers/WETHZapper.sol | 6 +++--- 4 files changed, 7 insertions(+), 9 deletions(-) diff --git a/contracts/src/BorrowerOperations.sol b/contracts/src/BorrowerOperations.sol index 7f4ba202..c1b8d34f 100644 --- a/contracts/src/BorrowerOperations.sol +++ b/contracts/src/BorrowerOperations.sol @@ -672,7 +672,7 @@ contract BorrowerOperations is LiquityBase, AddRemoveManagers, IBorrowerOperatio _moveTokensFromAdjustment(receiver, _troveChange, vars.boldToken, vars.activePool); } - function closeTrove(uint256 _troveId) external override returns (uint256) { + function closeTrove(uint256 _troveId) external override { ITroveManager troveManagerCached = troveManager; IActivePool activePoolCached = activePool; IBoldToken boldTokenCached = boldToken; @@ -738,8 +738,6 @@ contract BorrowerOperations is LiquityBase, AddRemoveManagers, IBorrowerOperatio activePoolCached.sendColl(receiver, trove.entireColl); _wipeTroveMappings(_troveId); - - return trove.entireColl; } function applyPendingDebt(uint256 _troveId, uint256 _lowerHint, uint256 _upperHint) public { diff --git a/contracts/src/Interfaces/IBorrowerOperations.sol b/contracts/src/Interfaces/IBorrowerOperations.sol index 664e750e..d4eb9c61 100644 --- a/contracts/src/Interfaces/IBorrowerOperations.sol +++ b/contracts/src/Interfaces/IBorrowerOperations.sol @@ -56,7 +56,7 @@ interface IBorrowerOperations is ILiquityBase, IAddRemoveManagers { function repayBold(uint256 _troveId, uint256 _amount) external; - function closeTrove(uint256 _troveId) external returns (uint256); + function closeTrove(uint256 _troveId) external; function adjustTrove( uint256 _troveId, diff --git a/contracts/src/Zappers/GasCompZapper.sol b/contracts/src/Zappers/GasCompZapper.sol index f5765bc4..19d11794 100644 --- a/contracts/src/Zappers/GasCompZapper.sol +++ b/contracts/src/Zappers/GasCompZapper.sol @@ -232,10 +232,10 @@ contract GasCompZapper is AddRemoveManagers { LatestTroveData memory trove = troveManager.getLatestTroveData(_troveId); boldToken.transferFrom(msg.sender, address(this), trove.entireDebt); - uint256 collLeft = borrowerOperations.closeTrove(_troveId); + borrowerOperations.closeTrove(_troveId); // Send coll left - collToken.safeTransfer(receiver, collLeft); + collToken.safeTransfer(receiver, trove.entireColl); // Send gas compensation WETH.withdraw(ETH_GAS_COMPENSATION); diff --git a/contracts/src/Zappers/WETHZapper.sol b/contracts/src/Zappers/WETHZapper.sol index d4655297..85c501a1 100644 --- a/contracts/src/Zappers/WETHZapper.sol +++ b/contracts/src/Zappers/WETHZapper.sol @@ -226,10 +226,10 @@ contract WETHZapper is AddRemoveManagers { LatestTroveData memory trove = troveManager.getLatestTroveData(_troveId); boldToken.transferFrom(msg.sender, address(this), trove.entireDebt); - uint256 collLeft = borrowerOperations.closeTrove(_troveId); + borrowerOperations.closeTrove(_troveId); - WETH.withdraw(collLeft + ETH_GAS_COMPENSATION); - (bool success,) = receiver.call{value: collLeft + ETH_GAS_COMPENSATION}(""); + WETH.withdraw(trove.entireColl + ETH_GAS_COMPENSATION); + (bool success,) = receiver.call{value: trove.entireColl + ETH_GAS_COMPENSATION}(""); require(success, "WZ: Sending ETH failed"); }