From 2009b009865daf3aaf4b6142d0354c5fa2ec065c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Fingen?= Date: Sun, 3 Nov 2024 11:21:59 +0000 Subject: [PATCH 1/3] chore: Update constants and params in invariant tests --- contracts/src/Dependencies/Constants.sol | 20 ++++++++++---------- contracts/src/test/Invariants.t.sol | 6 +++--- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/contracts/src/Dependencies/Constants.sol b/contracts/src/Dependencies/Constants.sol index 26223925..4cc0c8c2 100644 --- a/contracts/src/Dependencies/Constants.sol +++ b/contracts/src/Dependencies/Constants.sol @@ -23,8 +23,8 @@ uint256 constant MIN_ANNUAL_INTEREST_RATE = _1pct / 2; // 0.5% uint256 constant MAX_ANNUAL_INTEREST_RATE = _100pct; // Batch management params -uint128 constant MAX_ANNUAL_BATCH_MANAGEMENT_FEE = uint128(_100pct); -uint128 constant MIN_INTEREST_RATE_CHANGE_PERIOD = 1 seconds; // prevents more than one adjustment per block +uint128 constant MAX_ANNUAL_BATCH_MANAGEMENT_FEE = uint128(_100pct / 10); // 10% +uint128 constant MIN_INTEREST_RATE_CHANGE_PERIOD = 120 seconds; // prevents more than one adjustment per ~10 blocks uint256 constant REDEMPTION_FEE_FLOOR = _1pct / 2; // 0.5% @@ -35,26 +35,26 @@ uint256 constant REDEMPTION_FEE_FLOOR = _1pct / 2; // 0.5% // but precisely the fact that we have this max value now prevents the attack uint256 constant MAX_BATCH_SHARES_RATIO = 1e9; -// Half-life of 12h. 12h = 720 min -// (1/2) = d^720 => d = (1/2)^(1/720) -uint256 constant REDEMPTION_MINUTE_DECAY_FACTOR = 999037758833783000; +// Half-life of 6h. 6h = 3600 min +// (1/2) = d^360 => d = (1/2)^(1/360) +uint256 constant REDEMPTION_MINUTE_DECAY_FACTOR = 998076443575628800; // BETA: 18 digit decimal. Parameter by which to divide the redeemed fraction, in order to calc the new base rate from a redemption. // Corresponds to (1 / ALPHA) in the white paper. -uint256 constant REDEMPTION_BETA = 2; +uint256 constant REDEMPTION_BETA = 1; // To prevent redemptions unless Bold depegs below 0.95 and allow the system to take off -uint256 constant INITIAL_BASE_RATE = 5 * _1pct - REDEMPTION_FEE_FLOOR; // 5% initial redemption rate +uint256 constant INITIAL_BASE_RATE = _100pct; // 100% initial redemption rate // Discount to be used once the shutdown thas been triggered -uint256 constant URGENT_REDEMPTION_BONUS = 1e16; // 1% +uint256 constant URGENT_REDEMPTION_BONUS = 2e16; // 2% uint256 constant ONE_MINUTE = 1 minutes; uint256 constant ONE_YEAR = 365 days; uint256 constant UPFRONT_INTEREST_PERIOD = 7 days; -uint256 constant INTEREST_RATE_ADJ_COOLDOWN = 3 days; +uint256 constant INTEREST_RATE_ADJ_COOLDOWN = 7 days; -uint256 constant SP_YIELD_SPLIT = 72 * _1pct; // 72% +uint256 constant SP_YIELD_SPLIT = 75 * _1pct; // 75% // Dummy contract that lets legacy Hardhat tests query some of the constants contract Constants { diff --git a/contracts/src/test/Invariants.t.sol b/contracts/src/test/Invariants.t.sol index f78e0e79..de0324df 100644 --- a/contracts/src/test/Invariants.t.sol +++ b/contracts/src/test/Invariants.t.sol @@ -71,9 +71,9 @@ contract InvariantsTest is Assertions, Logging, BaseInvariantTest, BaseMultiColl // TODO: randomize params? How to do it with Foundry invariant testing? TestDeployer.TroveManagerParams[] memory p = new TestDeployer.TroveManagerParams[](n); - if (n > 0) p[0] = TestDeployer.TroveManagerParams(1.5 ether, 1.1 ether, 1.01 ether, 0.05 ether, 0.1 ether); - if (n > 1) p[1] = TestDeployer.TroveManagerParams(1.6 ether, 1.2 ether, 1.01 ether, 0.05 ether, 0.1 ether); - if (n > 2) p[2] = TestDeployer.TroveManagerParams(1.6 ether, 1.2 ether, 1.01 ether, 0.05 ether, 0.1 ether); + if (n > 0) p[0] = TestDeployer.TroveManagerParams(1.5 ether, 1.1 ether, 1.1 ether, 0.05 ether, 0.1 ether); + if (n > 1) p[1] = TestDeployer.TroveManagerParams(1.6 ether, 1.2 ether, 1.2 ether, 0.05 ether, 0.2 ether); + if (n > 2) p[2] = TestDeployer.TroveManagerParams(1.6 ether, 1.2 ether, 1.2 ether, 0.05 ether, 0.2 ether); if (n > 3) p[3] = TestDeployer.TroveManagerParams(1.6 ether, 1.25 ether, 1.01 ether, 0.05 ether, 0.1 ether); TestDeployer deployer = new TestDeployer(); From 16addf67013a7b98ad54ff990dc5b7758d7cf0e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Fingen?= Date: Sun, 3 Nov 2024 16:53:47 +0000 Subject: [PATCH 2/3] fix: Avoid hardcoded values for min and max liquidation penalties --- contracts/src/AddressesRegistry.sol | 5 +++-- contracts/src/Dependencies/Constants.sol | 6 +++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/contracts/src/AddressesRegistry.sol b/contracts/src/AddressesRegistry.sol index 65306bc7..606c8833 100644 --- a/contracts/src/AddressesRegistry.sol +++ b/contracts/src/AddressesRegistry.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.24; import "./Dependencies/Ownable.sol"; +import {MIN_LIQUIDATION_PENALTY_SP, MAX_LIQUIDATION_PENALTY_REDISTRIBUTION} from "./Dependencies/Constants.sol"; import "./Interfaces/IAddressesRegistry.sol"; contract AddressesRegistry is Ownable, IAddressesRegistry { @@ -76,9 +77,9 @@ contract AddressesRegistry is Ownable, IAddressesRegistry { if (_ccr <= 1e18 || _ccr >= 2e18) revert InvalidCCR(); if (_mcr <= 1e18 || _mcr >= 2e18) revert InvalidMCR(); if (_scr <= 1e18 || _scr >= 2e18) revert InvalidSCR(); - if (_liquidationPenaltySP < 5e16) revert SPPenaltyTooLow(); + if (_liquidationPenaltySP < MIN_LIQUIDATION_PENALTY_SP) revert SPPenaltyTooLow(); if (_liquidationPenaltySP > _liquidationPenaltyRedistribution) revert SPPenaltyGtRedist(); - if (_liquidationPenaltyRedistribution > 10e16) revert RedistPenaltyTooHigh(); + if (_liquidationPenaltyRedistribution > MAX_LIQUIDATION_PENALTY_REDISTRIBUTION) revert RedistPenaltyTooHigh(); CCR = _ccr; SCR = _scr; diff --git a/contracts/src/Dependencies/Constants.sol b/contracts/src/Dependencies/Constants.sol index 4cc0c8c2..94f3719d 100644 --- a/contracts/src/Dependencies/Constants.sol +++ b/contracts/src/Dependencies/Constants.sol @@ -12,6 +12,10 @@ uint256 constant _1pct = DECIMAL_PRECISION / 100; // Amount of ETH to be locked in gas pool on opening troves uint256 constant ETH_GAS_COMPENSATION = 0.0375 ether; +// Liquidation +uint256 constant MIN_LIQUIDATION_PENALTY_SP = 5e16; // 5% +uint256 constant MAX_LIQUIDATION_PENALTY_REDISTRIBUTION = 20e16; // 20% + // Fraction of collateral awarded to liquidator uint256 constant COLL_GAS_COMPENSATION_DIVISOR = 200; // dividing by 200 yields 0.5% uint256 constant COLL_GAS_COMPENSATION_CAP = 2 ether; // Max coll gas compensation capped at 2 ETH @@ -35,7 +39,7 @@ uint256 constant REDEMPTION_FEE_FLOOR = _1pct / 2; // 0.5% // but precisely the fact that we have this max value now prevents the attack uint256 constant MAX_BATCH_SHARES_RATIO = 1e9; -// Half-life of 6h. 6h = 3600 min +// Half-life of 6h. 6h = 360 min // (1/2) = d^360 => d = (1/2)^(1/360) uint256 constant REDEMPTION_MINUTE_DECAY_FACTOR = 998076443575628800; From a3651b7cdf5cbdab51833f1e67c94d358729b88f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Fingen?= Date: Thu, 7 Nov 2024 15:59:08 +0000 Subject: [PATCH 3/3] test: Fix tests after parameters update --- .../src/test/TestContracts/DevTestSetup.sol | 3 +++ contracts/src/test/basicOps.t.sol | 11 +++++++---- contracts/src/test/rebasingBatchShares.t.sol | 2 +- contracts/src/test/redemptions.t.sol | 6 +----- contracts/src/test/shutdown.t.sol | 18 +++++++++--------- contracts/src/test/troveManager.t.sol | 11 +++++++---- 6 files changed, 28 insertions(+), 23 deletions(-) diff --git a/contracts/src/test/TestContracts/DevTestSetup.sol b/contracts/src/test/TestContracts/DevTestSetup.sol index 0800e7b4..5aec497c 100644 --- a/contracts/src/test/TestContracts/DevTestSetup.sol +++ b/contracts/src/test/TestContracts/DevTestSetup.sol @@ -245,6 +245,9 @@ contract DevTestSetup is BaseTest { } function _redeemAndCreateZombieTrovesAAndB(ABCDEF memory _troveIDs) internal { + // Wait some time to make sure redemption rate low + vm.warp(block.timestamp + 30 days); + // Redeem enough to leave A with 0 debt and B with debt < MIN_DEBT uint256 redeemFromA = troveManager.getTroveEntireDebt(_troveIDs.A); uint256 redeemFromB = troveManager.getTroveEntireDebt(_troveIDs.B) - MIN_DEBT / 2; diff --git a/contracts/src/test/basicOps.t.sol b/contracts/src/test/basicOps.t.sol index bf1575fd..bcc8ec04 100644 --- a/contracts/src/test/basicOps.t.sol +++ b/contracts/src/test/basicOps.t.sol @@ -106,9 +106,9 @@ contract BasicOps is DevTestSetup { B, 0, 5e18, 4_000e18, 0, 0, MIN_ANNUAL_INTEREST_RATE, 1000e18, address(0), address(0), address(0) ); uint256 debt_1 = troveManager.getTroveDebt(B_Id); - assertGt(debt_1, 0); + assertGt(debt_1, 0, "Debt cannot be zero"); uint256 coll_1 = troveManager.getTroveColl(B_Id); - assertGt(coll_1, 0); + assertGt(coll_1, 0, "Coll cannot be zero"); vm.stopPrank(); // B is now first in line to get redeemed, as they both have the same interest rate, @@ -116,15 +116,18 @@ contract BasicOps is DevTestSetup { uint256 redemptionAmount = 1000e18; // 1k BOLD + // Wait some time so that redemption rate is not 100% + vm.warp(block.timestamp + 7 days); + // A redeems 1k BOLD vm.startPrank(A); collateralRegistry.redeemCollateral(redemptionAmount, 10, 1e18); // Check B's coll and debt reduced uint256 debt_2 = troveManager.getTroveDebt(B_Id); - assertLt(debt_2, debt_1); + assertLt(debt_2, debt_1, "Debt mismatch after"); uint256 coll_2 = troveManager.getTroveColl(B_Id); - assertLt(coll_2, coll_1); + assertLt(coll_2, coll_1, "Coll mismatch after"); } function testLiquidation() public { diff --git a/contracts/src/test/rebasingBatchShares.t.sol b/contracts/src/test/rebasingBatchShares.t.sol index 92c52799..6bddff49 100644 --- a/contracts/src/test/rebasingBatchShares.t.sol +++ b/contracts/src/test/rebasingBatchShares.t.sol @@ -127,7 +127,7 @@ contract RebasingBatchShares is DevTestSetup { // Trigger interest fee by changing the fee to it -1 function _triggerInterestRateFee() internal { // Add Fee? - vm.warp(block.timestamp + 1); + vm.warp(block.timestamp + MIN_INTEREST_RATE_CHANGE_PERIOD); vm.startPrank(B); borrowerOperations.setBatchManagerAnnualInterestRate(1e18 - subTractor++, 0, 0, type(uint256).max); vm.stopPrank(); diff --git a/contracts/src/test/redemptions.t.sol b/contracts/src/test/redemptions.t.sol index fe1e62a2..0d7c1898 100644 --- a/contracts/src/test/redemptions.t.sol +++ b/contracts/src/test/redemptions.t.sol @@ -471,11 +471,7 @@ contract Redemptions is DevTestSetup { assertEq(troveManager.lastZombieTroveId(), troveIDs.B, "Wrong last zombie trove pointer before"); // Liquidate B - console2.log( - troveManager.getCurrentICR(troveIDs.B, priceFeed.getPrice()), - "troveManager.getCurrentICR(troveIDs.E, price)" - ); - priceFeed.setPrice(priceFeed.getPrice() / 25); + priceFeed.setPrice(priceFeed.getPrice() / 30); liquidate(A, troveIDs.B); // Check B is liquidated diff --git a/contracts/src/test/shutdown.t.sol b/contracts/src/test/shutdown.t.sol index ff7f2a5f..d2d6cb95 100644 --- a/contracts/src/test/shutdown.t.sol +++ b/contracts/src/test/shutdown.t.sol @@ -373,13 +373,13 @@ contract ShutdownTest is DevTestSetup { // Min not reached vm.startPrank(A); - vm.expectRevert(abi.encodeWithSelector(TroveManager.MinCollNotReached.selector, 101e16)); - troveManager.urgentRedemption(1000e18, uintToArray(troveId), 102e16); + vm.expectRevert(abi.encodeWithSelector(TroveManager.MinCollNotReached.selector, 102e16)); + troveManager.urgentRedemption(1000e18, uintToArray(troveId), 103e16); vm.stopPrank(); // Min just reached vm.startPrank(A); - troveManager.urgentRedemption(1000e18, uintToArray(troveId), 101e16); + troveManager.urgentRedemption(1000e18, uintToArray(troveId), 102e16); vm.stopPrank(); } @@ -406,7 +406,7 @@ contract ShutdownTest is DevTestSetup { assertEq(boldToken.balanceOf(A), boldBalanceBefore - redemptionAmount, "Bold balance mismatch"); assertEq( contractsArray[0].collToken.balanceOf(A), - collBalanceBefore + redemptionAmount * DECIMAL_PRECISION / price * 101 / 100, + collBalanceBefore + redemptionAmount * DECIMAL_PRECISION / price * (DECIMAL_PRECISION + URGENT_REDEMPTION_BONUS) / DECIMAL_PRECISION, "Coll balance mismatch" ); } @@ -433,7 +433,7 @@ contract ShutdownTest is DevTestSetup { assertEq( boldToken.balanceOf(A), - boldBalanceBefore - 11e18 * price / DECIMAL_PRECISION * 100 / 101, + boldBalanceBefore - 11e18 * price / DECIMAL_PRECISION * DECIMAL_PRECISION / (DECIMAL_PRECISION + URGENT_REDEMPTION_BONUS), "Bold balance mismatch" ); assertEq(contractsArray[0].collToken.balanceOf(A), collBalanceBefore + 11e18, "Coll balance mismatch"); @@ -463,7 +463,7 @@ contract ShutdownTest is DevTestSetup { assertEq(boldToken.balanceOf(A), boldBalanceBefore - redemptionAmount, "Bold balance mismatch"); assertEq( contractsArray[0].collToken.balanceOf(A), - collBalanceBefore + redemptionAmount * DECIMAL_PRECISION / price * 101 / 100, + collBalanceBefore + redemptionAmount * DECIMAL_PRECISION / price * (DECIMAL_PRECISION + URGENT_REDEMPTION_BONUS) / DECIMAL_PRECISION, "Coll balance mismatch" ); } @@ -493,7 +493,7 @@ contract ShutdownTest is DevTestSetup { // TODO: determine why this is off by 1 wei - it should be exact assertApproximatelyEqual( contractsArray[0].collToken.balanceOf(A), - collBalanceBefore + redemptionAmount * DECIMAL_PRECISION / price * 101 / 100, + collBalanceBefore + redemptionAmount * DECIMAL_PRECISION / price * (DECIMAL_PRECISION + URGENT_REDEMPTION_BONUS) / DECIMAL_PRECISION, 1, // 1 wei tolerance "Coll balance mismatch" ); @@ -522,7 +522,7 @@ contract ShutdownTest is DevTestSetup { assertEq(boldToken.balanceOf(A), boldBalanceBefore - redemptionAmount, "Bold balance mismatch"); assertEq( contractsArray[0].collToken.balanceOf(A), - collBalanceBefore + redemptionAmount * DECIMAL_PRECISION / price * 101 / 100, + collBalanceBefore + redemptionAmount * DECIMAL_PRECISION / price * (DECIMAL_PRECISION + URGENT_REDEMPTION_BONUS) / DECIMAL_PRECISION, "Coll balance mismatch" ); } @@ -551,7 +551,7 @@ contract ShutdownTest is DevTestSetup { // TODO: determine why this is off by 1 wei - it should be exact assertApproximatelyEqual( contractsArray[0].collToken.balanceOf(A), - collBalanceBefore + redemptionAmount * DECIMAL_PRECISION / price * 101 / 100, + collBalanceBefore + redemptionAmount * DECIMAL_PRECISION / price * (DECIMAL_PRECISION + URGENT_REDEMPTION_BONUS) / DECIMAL_PRECISION, 1, // 1 wei tolerance "Coll balance mismatch" ); diff --git a/contracts/src/test/troveManager.t.sol b/contracts/src/test/troveManager.t.sol index 0e12c54c..d0c07ffd 100644 --- a/contracts/src/test/troveManager.t.sol +++ b/contracts/src/test/troveManager.t.sol @@ -39,6 +39,9 @@ contract TroveManagerTest is DevTestSetup { uint256 redemptionAmount = 1000e18; // 1k BOLD + // Wait some time so that redemption rate is not 100% + vm.warp(block.timestamp + 7 days); + // C redeems 1k BOLD vm.startPrank(C); collateralRegistry.redeemCollateral(redemptionAmount, 10, 1e18); @@ -69,13 +72,13 @@ contract TroveManagerTest is DevTestSetup { priceFeed.setPrice(2000e18); openTroveNoHints100pct(A, 200 ether, 200000e18, 1e17); - // A redeems 0.01 BOLD, base rate goes down to almost zero (it’s updated on redemption) + // A redeems 1 wei of BOLD, base rate goes down to almost zero (it’s updated on redemption) vm.startPrank(A); - collateralRegistry.redeemCollateral(1e16, 10, 1e18); + collateralRegistry.redeemCollateral(1, 10, 1e18); vm.stopPrank(); - console.log(collateralRegistry.baseRate(), "baseRate"); - assertLt(collateralRegistry.baseRate(), 3e10); // Goes down below 3e-8, i.e., below 0.000003% + //console.log(collateralRegistry.baseRate(), "baseRate"); + assertLt(collateralRegistry.baseRate(), 20); // Goes down below 2e-16 } function testLiquidationSucceedsEvenWhenEncounteringInactiveTroves() public {