From ff54840a18726596ed3f1e44b9063c4f23fc6667 Mon Sep 17 00:00:00 2001 From: nezouse Date: Thu, 25 Apr 2024 08:34:50 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9E=20Update=20redeemers=20whitelist?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- contracts/AdminContract.sol | 18 ++++--- contracts/Interfaces/IAdminContract.sol | 7 +-- .../Interfaces/IVesselManagerOperations.sol | 1 + contracts/VesselManagerOperations.sol | 4 +- test/trinity/AdminContractTest.js | 16 +++---- test/trinity/BorrowerOperationsTest.js | 2 +- test/trinity/CollSurplusPoolTest.js | 1 + test/trinity/VesselManagerTest.js | 47 ++++++++++++------- .../trinity/VesselManager_RecoveryModeTest.js | 4 ++ test/utils/testHelpers.js | 1 - 10 files changed, 64 insertions(+), 37 deletions(-) diff --git a/contracts/AdminContract.sol b/contracts/AdminContract.sol index 9c7ed45..ff53d8c 100644 --- a/contracts/AdminContract.sol +++ b/contracts/AdminContract.sol @@ -38,7 +38,7 @@ contract AdminContract is IAdminContract, UUPSUpgradeable, OwnableUpgradeable, A */ mapping(address => CollateralParams) internal collateralParams; - mapping(address => bool) internal whitelistedRedeemers; + mapping(address => mapping(address => bool)) internal collateralWhitelistedAddresses; // list of all collateral types in collateralParams (active and deprecated) // Addresses for easy access @@ -248,9 +248,14 @@ contract AdminContract is IAdminContract, UUPSUpgradeable, OwnableUpgradeable, A emit RedemptionBlockTimestampChanged(_collateral, _blockTimestamp); } - function setWhitelistedRedeemer(address _redeemer, bool _whitelisted) external onlyTimelock { - whitelistedRedeemers[_redeemer] = _whitelisted; - emit RedeemerWhitelisted(_redeemer, _whitelisted); + + function setAddressCollateralWhitelisted( + address _collateral, + address _address, + bool _whitelisted + ) external onlyTimelock { + collateralWhitelistedAddresses[_collateral][_address] = _whitelisted; + emit AddressCollateralWhitelisted(_collateral, _address, _whitelisted); } function setRedemptionBaseFeeEnabled(address _collateral, bool _enabled) external onlyTimelock { @@ -329,8 +334,9 @@ contract AdminContract is IAdminContract, UUPSUpgradeable, OwnableUpgradeable, A return IActivePool(activePool).getDebtTokenBalance(_asset) + IDefaultPool(defaultPool).getDebtTokenBalance(_asset); } - function getRedeemerIsWhitelisted(address _redeemer) external view returns (bool) { - return whitelistedRedeemers[_redeemer]; + + function getIsAddressCollateralWhitelisted(address _collateral, address _address) external view returns (bool) { + return collateralWhitelistedAddresses[_collateral][_address]; } function getRedemptionBaseFeeEnabled(address _collateral) external view override returns (bool) { diff --git a/contracts/Interfaces/IAdminContract.sol b/contracts/Interfaces/IAdminContract.sol index b15589c..64d9bc8 100644 --- a/contracts/Interfaces/IAdminContract.sol +++ b/contracts/Interfaces/IAdminContract.sol @@ -43,7 +43,7 @@ interface IAdminContract { event RedemptionFeeFloorChanged(uint256 oldRedemptionFeeFloor, uint256 newRedemptionFeeFloor); event MintCapChanged(uint256 oldMintCap, uint256 newMintCap); event RedemptionBlockTimestampChanged(address _collateral, uint256 _blockTimestamp); - event RedeemerWhitelisted(address _redeemer, bool _whitelisted); + event AddressCollateralWhitelisted(address _collateral, address _address, bool _whitelisted); event BaseFeeEnabledChanged(address _collateral, bool _enabled); // Functions -------------------------------------------------------------------------------------------------------- @@ -81,7 +81,7 @@ interface IAdminContract { function setRedemptionBlockTimestamp(address _collateral, uint256 _blockTimestamp) external; - function setWhitelistedRedeemer(address _redeemer, bool _whitelisted) external; + function setAddressCollateralWhitelisted(address _collateral, address _address, bool _whitelisted) external; function setRedemptionBaseFeeEnabled(address _collateral, bool _enabled) external; @@ -111,7 +111,8 @@ interface IAdminContract { function getTotalAssetDebt(address _asset) external view returns (uint256); - function getRedeemerIsWhitelisted(address _redeemer) external view returns (bool); + + function getIsAddressCollateralWhitelisted(address _collateral, address _address) external view returns (bool); function getRedemptionBaseFeeEnabled(address _collateral) external view returns (bool); } diff --git a/contracts/Interfaces/IVesselManagerOperations.sol b/contracts/Interfaces/IVesselManagerOperations.sol index 862c897..a0e6387 100644 --- a/contracts/Interfaces/IVesselManagerOperations.sol +++ b/contracts/Interfaces/IVesselManagerOperations.sol @@ -50,6 +50,7 @@ interface IVesselManagerOperations is ITrinityBase { error VesselManagerOperations__VesselNotActive(); error VesselManagerOperations__InvalidParam(); error VesselManagerOperations__NotTimelock(); + error VesselManagerOperations__AddressNotCollateralWhitelisted(); // Structs ---------------------------------------------------------------------------------------------------------- diff --git a/contracts/VesselManagerOperations.sol b/contracts/VesselManagerOperations.sol index fbda0e4..4a12b7b 100644 --- a/contracts/VesselManagerOperations.sol +++ b/contracts/VesselManagerOperations.sol @@ -870,7 +870,9 @@ contract VesselManagerOperations is IVesselManagerOperations, UUPSUpgradeable, R uint256 _price ) internal view { address redeemer = msg.sender; - require(IAdminContract(adminContract).getRedeemerIsWhitelisted(redeemer), "VesselManagerOperations: Redeemer not whitelisted"); + if (!IAdminContract(adminContract).getIsAddressCollateralWhitelisted(_asset, redeemer)) { + revert VesselManagerOperations__AddressNotCollateralWhitelisted(); + } uint256 redemptionBlockTimestamp = IAdminContract(adminContract).getRedemptionBlockTimestamp(_asset); if (redemptionBlockTimestamp > block.timestamp) { diff --git a/test/trinity/AdminContractTest.js b/test/trinity/AdminContractTest.js index f7b9483..abef539 100644 --- a/test/trinity/AdminContractTest.js +++ b/test/trinity/AdminContractTest.js @@ -310,16 +310,16 @@ contract("AdminContract", async accounts => { assert.isTrue(_USDVFee_Asset.eq(expectedFee_Asset)) }) - it('setRedeemerWhitelisted: Owner change parameter - Valid Owner', async () => { - await adminContract.setWhitelistedRedeemer(ZERO_ADDRESS, true) - assert.isTrue(await adminContract.getRedeemerIsWhitelisted(ZERO_ADDRESS)) - await adminContract.setWhitelistedRedeemer(ZERO_ADDRESS, false) - assert.isFalse(await adminContract.getRedeemerIsWhitelisted(ZERO_ADDRESS)) + it('setAddressCollateralWhitelisted: Owner change parameter - Valid Owner', async () => { + await adminContract.setAddressCollateralWhitelisted(erc20.address, ZERO_ADDRESS, true) + assert.isTrue(await adminContract.getIsAddressCollateralWhitelisted(erc20.address, ZERO_ADDRESS)) + await adminContract.setAddressCollateralWhitelisted(erc20.address, ZERO_ADDRESS, false) + assert.isFalse(await adminContract.getIsAddressCollateralWhitelisted(erc20.address, ZERO_ADDRESS)) }) - it('setRedeemerWhitelisted: Owner change parameter - Invalid Owner', async () => { - await assertRevert(adminContract.setWhitelistedRedeemer(ZERO_ADDRESS, true, {from: user})) - await assertRevert(adminContract.setWhitelistedRedeemer(ZERO_ADDRESS, false, {from: user})) + it('setAddressCollateralWhitelisted: Owner change parameter - Invalid Owner', async () => { + await assertRevert(adminContract.setAddressCollateralWhitelisted(erc20.address, ZERO_ADDRESS, true, {from: user})) + await assertRevert(adminContract.setAddressCollateralWhitelisted(erc20.address, ZERO_ADDRESS, false, {from: user})) }) it('setRedemptionBaseFeeEnabled: Owner change parameter - Valid Owner', async () => { diff --git a/test/trinity/BorrowerOperationsTest.js b/test/trinity/BorrowerOperationsTest.js index f10b181..9e91ecd 100644 --- a/test/trinity/BorrowerOperationsTest.js +++ b/test/trinity/BorrowerOperationsTest.js @@ -60,7 +60,7 @@ contract("BorrowerOperations", async accounts => { describe("BorrowerOperations Mechanisms", async () => { before(async () => { - await deploy(treasury, []) + await deploy(treasury, accounts.slice(0, 20)) TRI_GAS_COMPENSATION_ERC20 = await adminContract.getDebtTokenGasCompensation(erc20.address) MIN_NET_DEBT_ERC20 = await adminContract.getMinNetDebt(erc20.address) diff --git a/test/trinity/CollSurplusPoolTest.js b/test/trinity/CollSurplusPoolTest.js index 6b03770..de99249 100644 --- a/test/trinity/CollSurplusPoolTest.js +++ b/test/trinity/CollSurplusPoolTest.js @@ -82,6 +82,7 @@ contract("CollSurplusPool", async accounts => { await th.fastForwardTime(timeValues.SECONDS_IN_ONE_WEEK * 2, web3.currentProvider) // At ETH:USD = 100, this redemption should leave 1 ether of coll surplus + await adminContract.setAddressCollateralWhitelisted(erc20.address, A, true) await th.redeemCollateralAndGetTxObject(A, contracts.core, B_netDebt, erc20.address) const ETH_2 = await collSurplusPool.getAssetBalance(erc20.address) diff --git a/test/trinity/VesselManagerTest.js b/test/trinity/VesselManagerTest.js index 2211947..b0f825c 100644 --- a/test/trinity/VesselManagerTest.js +++ b/test/trinity/VesselManagerTest.js @@ -45,6 +45,10 @@ const deploy = async (treasury, mintingAccounts) => { // getDepositorGains() expects a sorted collateral array validCollateral = validCollateral.slice(0).sort((a, b) => toBN(a.toLowerCase()).sub(toBN(b.toLowerCase()))) + + for(const account of mintingAccounts) { + await adminContract.setAddressCollateralWhitelisted(erc20.address, account, true) + } } /* NOTE: Some tests involving ETH redemption fees do not test for specific fee values. @@ -3223,7 +3227,6 @@ contract("VesselManager", async accounts => { // skip redemption await time.increase(redemptionWait) - await adminContract.setWhitelistedRedeemer(dennis, true) // this time tx should succeed await vesselManagerOperations.redeemCollateral( erc20.address, @@ -3292,7 +3295,6 @@ contract("VesselManager", async accounts => { // Dennis redeems 20 debt tokens // Don't pay for gas, as it makes it easier to calculate the received collateral - await adminContract.setWhitelistedRedeemer(dennis, true) const redemptionTx = await vesselManagerOperations.redeemCollateral( erc20.address, redemptionAmount, @@ -3393,7 +3395,6 @@ contract("VesselManager", async accounts => { // skip redemption bootstrapping phase await th.fastForwardTime(timeValues.SECONDS_IN_ONE_WEEK * 2, web3.currentProvider) - await adminContract.setWhitelistedRedeemer(dennis, true) // Dennis redeems 20 debt tokens const redemptionTx = await vesselManagerOperations.redeemCollateral( erc20.address, @@ -3494,7 +3495,6 @@ contract("VesselManager", async accounts => { // skip redemption bootstrapping phase await th.fastForwardTime(timeValues.SECONDS_IN_ONE_WEEK * 2, web3.currentProvider) - await adminContract.setWhitelistedRedeemer(dennis, true) // Dennis redeems 20 debt tokens const redemptionTx = await vesselManagerOperations.redeemCollateral( erc20.address, @@ -3607,7 +3607,6 @@ contract("VesselManager", async accounts => { // Dennis redeems 20 debt tokens // Don't pay for gas, as it makes it easier to calculate the received Ether - await adminContract.setWhitelistedRedeemer(dennis, true) const redemptionTx = await vesselManagerOperations.redeemCollateral( erc20.address, redemptionAmount, @@ -3705,7 +3704,6 @@ contract("VesselManager", async accounts => { // skip redemption bootstrapping phase await th.fastForwardTime(timeValues.SECONDS_IN_ONE_WEEK * 2, web3.currentProvider) - await adminContract.setWhitelistedRedeemer(flyn, true) // Flyn redeems collateral await vesselManagerOperations.redeemCollateral( erc20.address, @@ -3802,7 +3800,6 @@ contract("VesselManager", async accounts => { // Flyn redeems collateral with only two iterations - await adminContract.setWhitelistedRedeemer(flyn, true) await vesselManagerOperations.redeemCollateral( erc20.address, attemptedRedemptionAmount_Asset, @@ -3996,7 +3993,6 @@ contract("VesselManager", async accounts => { // skip redemption bootstrapping phase await th.fastForwardTime(timeValues.SECONDS_IN_ONE_WEEK * 2, web3.currentProvider) - await adminContract.setWhitelistedRedeemer(alice, true) // Alice redeems 1 debt token from Carol's Vessel await vesselManagerOperations.redeemCollateral( erc20.address, @@ -4011,7 +4007,6 @@ contract("VesselManager", async accounts => { ) } - await adminContract.setWhitelistedRedeemer(dennis, true) // Dennis tries to redeem 20 debt tokens const redemptionTx = await vesselManagerOperations.redeemCollateral( erc20.address, @@ -4140,7 +4135,6 @@ contract("VesselManager", async accounts => { // skip bootstrapping phase await th.fastForwardTime(timeValues.SECONDS_IN_ONE_WEEK * 2, web3.currentProvider) - await adminContract.setWhitelistedRedeemer(carol, true) await vesselManagerOperations.redeemCollateral( erc20.address, A_debt_Asset, @@ -4219,7 +4213,6 @@ contract("VesselManager", async accounts => { // skip bootstrapping phase await th.fastForwardTime(timeValues.SECONDS_IN_ONE_WEEK * 2, web3.currentProvider) - await adminContract.setWhitelistedRedeemer(dennis, true) const tx_Asset = await vesselManagerOperations.redeemCollateral( erc20.address, redemptionAmount_Asset, @@ -4246,6 +4239,32 @@ contract("VesselManager", async accounts => { th.assertIsApproximatelyEqual(dennis_Debt_After_Asset, D_totalDebt_Asset) }) + it("redeemCollateral(): reverts when not whitelisted", async () => { + await openVessel({ + asset: erc20.address, + ICR: toBN(dec(200, 16)), + extraParams: { from: alice }, + }) + await openVessel({ + asset: erc20.address, + ICR: toBN(dec(200, 16)), + extraParams: { from: bob }, + }) + + // skip bootstrapping phase + await th.fastForwardTime(timeValues.SECONDS_IN_ONE_WEEK * 2, web3.currentProvider) + + await adminContract.setAddressCollateralWhitelisted(erc20.address, alice, false) + + try { + const tx = await th.redeemCollateralAndGetTxObject(alice, contracts.core, dec(10, 18), erc20.address) + assert.isFalse(tx.receipt.status) + } catch (err) { + assert.include(err.message, "revert") + assert.include(err.message, "VesselManagerOperations__AddressNotCollateralWhitelisted()") + } + }) + it("redeemCollateral(): reverts when TCR < MCR", async () => { await openVessel({ asset: erc20.address, @@ -4807,7 +4826,6 @@ contract("VesselManager", async accounts => { erin ) - await adminContract.setWhitelistedRedeemer(erin, true) await vesselManagerOperations.redeemCollateral( erc20.address, dec(400, 18), @@ -4906,7 +4924,6 @@ contract("VesselManager", async accounts => { const { 0: upperPartialRedemptionHint_1_Asset, 1: lowerPartialRedemptionHint_1_Asset } = await sortedVessels.findInsertPosition(erc20.address, partialRedemptionHintNICR_Asset, erin, erin) - await adminContract.setWhitelistedRedeemer(erin, true) const redemptionTx_Asset = await vesselManagerOperations.redeemCollateral( erc20.address, dec(1000, 18), @@ -5059,7 +5076,6 @@ contract("VesselManager", async accounts => { // skip redemption bootstrapping phase await th.fastForwardTime(timeValues.SECONDS_IN_ONE_WEEK * 2, web3.currentProvider) - await adminContract.setWhitelistedRedeemer(erin, true) // Erin redeems 120 debt tokens await ({ 0: firstRedemptionHint, 1: partialRedemptionHintNewICR } = await vesselManagerOperations.getRedemptionHints(erc20.address, _120_, price, 0)) @@ -5094,7 +5110,6 @@ contract("VesselManager", async accounts => { const { 0: upperPartialRedemptionHint_2, 1: lowerPartialRedemptionHint_2 } = await sortedVessels.findInsertPosition(erc20.address, partialRedemptionHintNewICR, flyn, flyn) - await adminContract.setWhitelistedRedeemer(flyn, true) const redemption_2 = await vesselManagerOperations.redeemCollateral( erc20.address, _373_, @@ -5126,7 +5141,6 @@ contract("VesselManager", async accounts => { const { 0: upperPartialRedemptionHint_3, 1: lowerPartialRedemptionHint_3 } = await sortedVessels.findInsertPosition(erc20.address, partialRedemptionHintNewICR, graham, graham) - await adminContract.setWhitelistedRedeemer(graham, true) const redemption_3 = await vesselManagerOperations.redeemCollateral( erc20.address, _950_, @@ -6402,7 +6416,6 @@ contract("VesselManager", async accounts => { const { 0: firstRedemptionHint_Asset, 1: partialRedemptionHintNICR_Asset } = await vesselManagerOperations.getRedemptionHints(erc20.address, TRIAmount_Asset, price, 0) - await adminContract.setWhitelistedRedeemer(alice, true) // Don't pay for gas, as it makes it easier to calculate the received Ether const redemptionTx_Asset = await vesselManagerOperations.redeemCollateral( erc20.address, diff --git a/test/trinity/VesselManager_RecoveryModeTest.js b/test/trinity/VesselManager_RecoveryModeTest.js index 69c8650..b92acbd 100644 --- a/test/trinity/VesselManager_RecoveryModeTest.js +++ b/test/trinity/VesselManager_RecoveryModeTest.js @@ -42,6 +42,10 @@ const deploy = async (treasury, mintingAccounts) => { // getDepositorGains() expects a sorted collateral array validCollateral = validCollateral.slice(0).sort((a, b) => toBN(a.toLowerCase()).sub(toBN(b.toLowerCase()))) + + for(const account of mintingAccounts) { + await adminContract.setAddressCollateralWhitelisted(erc20.address, account, true) + } } contract("VesselManager - in Recovery Mode", async accounts => { diff --git a/test/utils/testHelpers.js b/test/utils/testHelpers.js index 7d1d85c..5b3c875 100644 --- a/test/utils/testHelpers.js +++ b/test/utils/testHelpers.js @@ -1367,7 +1367,6 @@ class TestHelper { approxPartialRedemptionHint ) - await contracts.adminContract.setWhitelistedRedeemer(redeemer, true) const tx = await contracts.vesselManagerOperations.redeemCollateral( asset, TRIAmount,