diff --git a/contracts/src/CollateralRegistry.sol b/contracts/src/CollateralRegistry.sol index c879b89b7..8e8fa4a90 100644 --- a/contracts/src/CollateralRegistry.sol +++ b/contracts/src/CollateralRegistry.sol @@ -10,7 +10,7 @@ import "./Dependencies/LiquityBase.sol"; import "./Interfaces/ICollateralRegistry.sol"; -//import "forge-std/console.sol"; +// import "forge-std/console.sol"; contract CollateralRegistry is LiquityBase, ICollateralRegistry { // mapping from Collateral token address to the corresponding TroveManagers @@ -170,8 +170,11 @@ contract CollateralRegistry is LiquityBase, ICollateralRegistry { internal view { + uint256 boldBalance = _boldToken.balanceOf(_redeemer); + // Confirm redeemer's balance is less than total Bold supply + assert(boldBalance <= _boldToken.totalSupply()); require( - _boldToken.balanceOf(_redeemer) >= _amount, + boldBalance >= _amount, "TroveManager: Requested redemption amount must be <= user's Bold token balance" ); } diff --git a/contracts/src/Interfaces/ITroveManager.sol b/contracts/src/Interfaces/ITroveManager.sol index a7a135d9e..00018f94f 100644 --- a/contracts/src/Interfaces/ITroveManager.sol +++ b/contracts/src/Interfaces/ITroveManager.sol @@ -82,6 +82,7 @@ interface ITroveManager is IERC721, ILiquityBase { function getRedemptionRateWithDecay() external view returns (uint256); function getRedemptionFeeWithDecay(uint256 _ETHDrawn) external view returns (uint256); + function getEffectiveRedemptionFee(uint256 _redeemAmount, uint256 _price) external view returns (uint256); function getTroveStatus(uint256 _troveId) external view returns (uint256); diff --git a/contracts/src/TroveManager.sol b/contracts/src/TroveManager.sol index 989b10f21..7b1efcd13 100644 --- a/contracts/src/TroveManager.sol +++ b/contracts/src/TroveManager.sol @@ -963,10 +963,7 @@ contract TroveManager is ERC721, LiquityBase, Ownable, CheckContract, ITroveMana // TODO: remove. Return instead? _requireTCRoverMCR(totals.price); - // TODO: Is this enough? Or should we add all branches `getEntireSystemDebt()`? - totals.totalBoldSupplyAtStart = contractsCache.boldToken.totalSupply(); - // Confirm redeemer's balance is less than total Bold supply - assert(contractsCache.boldToken.balanceOf(_sender) <= totals.totalBoldSupplyAtStart); + totals.totalBoldSupplyAtStart = getEntireSystemDebt(); totals.remainingBold = _boldamount; uint256 currentTroveId; @@ -1381,23 +1378,34 @@ contract TroveManager is ERC721, LiquityBase, Ownable, CheckContract, ITroveMana // --- Redemption fee functions --- /* - * This function has two impacts on the baseRate state variable: - * 1) decays the baseRate based on time passed since last redemption or Bold borrowing operation. - * then, - * 2) increases the baseRate based on the amount redeemed, as a proportion of total supply - */ - function _updateBaseRateFromRedemption(uint256 _ETHDrawn, uint256 _price, uint256 _totalBoldSupply) - internal + * This function has two impacts on the baseRate state variable: + * 1) decays the baseRate based on time passed since last redemption or Bold borrowing operation. + * then, + * 2) increases the baseRate based on the amount redeemed, as a proportion of total supply + */ + function _getUpdatedBaseRateFromRedemption(uint256 _redeemAmount, uint256 _totalBoldSupply) + internal view returns (uint256) { uint256 decayedBaseRate = _calcDecayedBaseRate(); /* Convert the drawn ETH back to Bold at face value rate (1 Bold:1 USD), in order to get - * the fraction of total supply that was redeemed at face value. */ - uint256 redeemedBoldFraction = _ETHDrawn * _price / _totalBoldSupply; + * the fraction of total supply that was redeemed at face value. */ + uint256 redeemedBoldFraction = _redeemAmount * DECIMAL_PRECISION / _totalBoldSupply; uint256 newBaseRate = decayedBaseRate + redeemedBoldFraction / BETA; newBaseRate = LiquityMath._min(newBaseRate, DECIMAL_PRECISION); // cap baseRate at a maximum of 100% + + return newBaseRate; + } + + // Updates the `baseRate` state with math from previous function + function _updateBaseRateFromRedemption(uint256 _ETHDrawn, uint256 _price, uint256 _totalBoldSupply) + internal + returns (uint256) + { + uint256 newBaseRate = _getUpdatedBaseRateFromRedemption(_ETHDrawn * _price / DECIMAL_PRECISION, _totalBoldSupply); + //assert(newBaseRate <= DECIMAL_PRECISION); // This is already enforced in the line above assert(newBaseRate > 0); // Base rate is always non-zero after redemption @@ -1433,6 +1441,12 @@ contract TroveManager is ERC721, LiquityBase, Ownable, CheckContract, ITroveMana return _calcRedemptionFee(getRedemptionRateWithDecay(), _ETHDrawn); } + function getEffectiveRedemptionFee(uint256 _redeemAmount, uint256 _price) external view override returns (uint256) { + uint256 totalBoldSupply = getEntireSystemDebt(); + uint256 newBaseRate = _getUpdatedBaseRateFromRedemption(_redeemAmount, totalBoldSupply); + return _calcRedemptionFee(_calcRedemptionRate(newBaseRate), _redeemAmount * DECIMAL_PRECISION / _price); + } + function _calcRedemptionFee(uint256 _redemptionRate, uint256 _ETHDrawn) internal pure returns (uint256) { uint256 redemptionFee = _redemptionRate * _ETHDrawn / DECIMAL_PRECISION; require(redemptionFee < _ETHDrawn, "TroveManager: Fee would eat up all returned collateral"); diff --git a/contracts/src/test/TestContracts/BaseTest.sol b/contracts/src/test/TestContracts/BaseTest.sol index 5bb774171..fb8d1b793 100644 --- a/contracts/src/test/TestContracts/BaseTest.sol +++ b/contracts/src/test/TestContracts/BaseTest.sol @@ -31,6 +31,7 @@ contract BaseTest is Test { address public F; address public G; + uint256 public constant DECIMAL_PRECISION = 1e18; uint256 public constant MAX_UINT256 = type(uint256).max; uint256 public constant SECONDS_IN_1_YEAR = 31536000; // 60*60*24*365 uint256 _100pct = 100e16; diff --git a/contracts/src/test/multicollateral.t.sol b/contracts/src/test/multicollateral.t.sol index b234df1cd..efeeeeabf 100644 --- a/contracts/src/test/multicollateral.t.sol +++ b/contracts/src/test/multicollateral.t.sol @@ -89,6 +89,9 @@ contract MulticollateralTest is DevTestSetup { } function testMultiCollateralRedemption() public { + // All collaterals have the same price for this test + uint256 price = contractsArray[0].priceFeed.getPrice(); + // First collateral unbacked Bold: 10k (SP empty) openMulticollateralTroveNoHints100pctMaxFeeWithIndex(0, A, 0, 10e18, 10000e18, 5e16); @@ -115,6 +118,11 @@ contract MulticollateralTest is DevTestSetup { uint256 coll3InitialBalance = contractsArray[2].WETH.balanceOf(A); uint256 coll4InitialBalance = contractsArray[3].WETH.balanceOf(A); + // fees + uint256 fee1 = contractsArray[0].troveManager.getEffectiveRedemptionFee(1000e18, price); + uint256 fee2 = contractsArray[1].troveManager.getEffectiveRedemptionFee(500e18, price); + uint256 fee3 = contractsArray[2].troveManager.getEffectiveRedemptionFee(100e18, price); + // A redeems 1.6k vm.startPrank(A); collateralRegistry.redeemCollateral(1600e18, 0, 1e18); @@ -132,16 +140,19 @@ contract MulticollateralTest is DevTestSetup { uint256 coll3FinalBalance = contractsArray[2].WETH.balanceOf(A); uint256 coll4FinalBalance = contractsArray[3].WETH.balanceOf(A); - assertEq(coll1FinalBalance - coll1InitialBalance, 5e17, "Wrong Collateral 1 balance"); - assertEq(coll2FinalBalance - coll2InitialBalance, 25e16, "Wrong Collateral 2 balance"); - assertEq(coll3FinalBalance - coll3InitialBalance, 5e16, "Wrong Collateral 3 balance"); + assertEq(coll1FinalBalance - coll1InitialBalance, 5e17 - fee1, "Wrong Collateral 1 balance"); + assertEq(coll2FinalBalance - coll2InitialBalance, 25e16 - fee2, "Wrong Collateral 2 balance"); + assertEq(coll3FinalBalance - coll3InitialBalance, 5e16 - fee3, "Wrong Collateral 3 balance"); assertEq(coll4FinalBalance - coll4InitialBalance, 0, "Wrong Collateral 4 balance"); } struct TestValues { + uint256 price; + uint256 unbackedPortion; + uint256 redeemAmount; + uint256 fee; uint256 collInitialBalance; uint256 collFinalBalance; - uint256 unbackedPortion; } function testMultiCollateralRedemptionFuzz( @@ -156,6 +167,11 @@ contract MulticollateralTest is DevTestSetup { TestValues memory testValues3; TestValues memory testValues4; + testValues1.price = contractsArray[0].priceFeed.getPrice(); + testValues2.price = contractsArray[1].priceFeed.getPrice(); + testValues3.price = contractsArray[2].priceFeed.getPrice(); + testValues4.price = contractsArray[3].priceFeed.getPrice(); + uint256 boldAmount = 10000e18; // TODO: remove gas compensation _spBoldAmount1 = bound(_spBoldAmount1, 0, boldAmount - 200e18); @@ -163,7 +179,7 @@ contract MulticollateralTest is DevTestSetup { _spBoldAmount3 = bound(_spBoldAmount3, 0, boldAmount - 200e18); _spBoldAmount4 = bound(_spBoldAmount4, 0, boldAmount - 200e18); // With too low redemption fractions, it reverts due to `newBaseRate` rounding down to zero, so we put a min of 0.01% - _redemptionFraction = bound(_redemptionFraction, 1e14, 1e18); + _redemptionFraction = bound(_redemptionFraction, 1e14, DECIMAL_PRECISION); // First collateral openMulticollateralTroveNoHints100pctMaxFeeWithIndex(0, A, 0, 10e18, boldAmount, 5e16); @@ -181,25 +197,45 @@ contract MulticollateralTest is DevTestSetup { openMulticollateralTroveNoHints100pctMaxFeeWithIndex(3, A, 0, 10e18, boldAmount, 5e16); if (_spBoldAmount4 > 0) makeMulticollateralSPDeposit(3, A, _spBoldAmount4); - // Check A’s final bal uint256 boldBalance = boldToken.balanceOf(A); + // Check A’s final bal // TODO: change when we switch to new gas compensation //assertEq(boldToken.balanceOf(A), boldAmount * 4 - _spBoldAmount1 - _spBoldAmount2 - _spBoldAmount3 - _spBoldAmount4, "Wrong Bold balance before redemption"); - console.log(boldBalance, "boldBalance"); - console.log(boldAmount, "boldAmount"); - console.log(boldAmount * 4 - 800e18, "boldAmount * 4 - 800e18"); - //assertApproximatelyEqual(boldBalance, boldAmount * 4 - 800e18, 10000, "Wrong Bold balance before redemption"); + // Stack too deep //assertEq(boldBalance, boldAmount * 4 - _spBoldAmount1 - _spBoldAmount2 - _spBoldAmount3 - _spBoldAmount4 - 800e18, "Wrong Bold balance before redemption"); + uint256 redeemAmount = boldBalance * _redemptionFraction / DECIMAL_PRECISION; + // initial balances testValues1.collInitialBalance = contractsArray[0].WETH.balanceOf(A); testValues2.collInitialBalance = contractsArray[1].WETH.balanceOf(A); testValues3.collInitialBalance = contractsArray[2].WETH.balanceOf(A); testValues4.collInitialBalance = contractsArray[3].WETH.balanceOf(A); + testValues1.unbackedPortion = boldAmount - _spBoldAmount1; + testValues2.unbackedPortion = boldAmount - _spBoldAmount2; + testValues3.unbackedPortion = boldAmount - _spBoldAmount3; + testValues4.unbackedPortion = boldAmount - _spBoldAmount4; + uint256 totalUnbacked = testValues1.unbackedPortion + testValues2.unbackedPortion + testValues3.unbackedPortion + + testValues4.unbackedPortion; + + testValues1.redeemAmount = redeemAmount * testValues1.unbackedPortion / totalUnbacked; + testValues2.redeemAmount = redeemAmount * testValues2.unbackedPortion / totalUnbacked; + testValues3.redeemAmount = redeemAmount * testValues3.unbackedPortion / totalUnbacked; + testValues4.redeemAmount = redeemAmount * testValues4.unbackedPortion / totalUnbacked; + + // fees + testValues1.fee = contractsArray[0].troveManager.getEffectiveRedemptionFee(testValues1.redeemAmount, testValues1.price); + testValues2.fee = contractsArray[1].troveManager.getEffectiveRedemptionFee(testValues2.redeemAmount, testValues2.price); + testValues3.fee = contractsArray[2].troveManager.getEffectiveRedemptionFee(testValues3.redeemAmount, testValues3.price); + testValues4.fee = contractsArray[3].troveManager.getEffectiveRedemptionFee(testValues4.redeemAmount, testValues4.price); + console.log(testValues1.fee, "fee1"); + console.log(testValues2.fee, "fee2"); + console.log(testValues3.fee, "fee3"); + console.log(testValues4.fee, "fee4"); + // A redeems 1.6k vm.startPrank(A); - uint256 redeemAmount = boldBalance * _redemptionFraction / 1e18; collateralRegistry.redeemCollateral(redeemAmount, 0, 1e18); vm.stopPrank(); @@ -215,24 +251,29 @@ contract MulticollateralTest is DevTestSetup { testValues3.collFinalBalance = contractsArray[2].WETH.balanceOf(A); testValues4.collFinalBalance = contractsArray[3].WETH.balanceOf(A); - testValues1.unbackedPortion = boldAmount - _spBoldAmount1; - testValues2.unbackedPortion = boldAmount - _spBoldAmount2; - testValues3.unbackedPortion = boldAmount - _spBoldAmount3; - testValues4.unbackedPortion = boldAmount - _spBoldAmount4; - uint256 totalUnbacked = testValues1.unbackedPortion + testValues2.unbackedPortion + testValues3.unbackedPortion - + testValues4.unbackedPortion; - console.log(redeemAmount, "redeemAmount"); console.log(testValues1.unbackedPortion, "testValues1.unbackedPortion"); console.log(totalUnbacked, "totalUnbacked"); - console.log(redeemAmount * testValues1.unbackedPortion / totalUnbacked, "var"); + console.log(testValues1.redeemAmount, "partial redeem amount 1"); assertEq( testValues1.collFinalBalance - testValues1.collInitialBalance, - redeemAmount * testValues1.unbackedPortion / totalUnbacked / 2000, + testValues1.redeemAmount * DECIMAL_PRECISION / testValues1.price - testValues1.fee, "Wrong Collateral 1 balance" ); - //assertEq(coll2FinalBalance - coll2InitialBalance, 25e16, "Wrong Collateral 2 balance"); - //assertEq(coll3FinalBalance - coll3InitialBalance, 5e16, "Wrong Collateral 3 balance"); - //assertEq(coll4FinalBalance - coll4InitialBalance, 0, "Wrong Collateral 4 balance"); + assertEq( + testValues2.collFinalBalance - testValues2.collInitialBalance, + testValues2.redeemAmount * DECIMAL_PRECISION / testValues2.price - testValues2.fee, + "Wrong Collateral 2 balance" + ); + assertEq( + testValues3.collFinalBalance - testValues3.collInitialBalance, + testValues3.redeemAmount * DECIMAL_PRECISION / testValues3.price - testValues3.fee, + "Wrong Collateral 3 balance" + ); + assertEq( + testValues4.collFinalBalance - testValues4.collInitialBalance, + testValues4.redeemAmount * DECIMAL_PRECISION / testValues4.price - testValues4.fee, + "Wrong Collateral 4 balance" + ); } }