diff --git a/contracts/credit/CreditConfiguratorV3.sol b/contracts/credit/CreditConfiguratorV3.sol index cee31154..12fa6df6 100644 --- a/contracts/credit/CreditConfiguratorV3.sol +++ b/contracts/credit/CreditConfiguratorV3.sol @@ -256,7 +256,7 @@ contract CreditConfiguratorV3 is ICreditConfiguratorV3, ACLNonReentrantTrait { /// @param adapter Adapter to allow /// @dev Reverts if `adapter` is incompatible with the credit manager /// @dev Reverts if `adapter`'s target contract is not a contract - /// @dev Reverts if `adapter` or its target contract is credit manager or credit facade + /// @dev Reverts if `adapter` or its target contract is credit facade function allowAdapter(address adapter) external override @@ -268,10 +268,10 @@ contract CreditConfiguratorV3 is ICreditConfiguratorV3, ACLNonReentrantTrait { revert AddressIsNotContractException(targetContract); // I:[CC-10A] } - if ( - targetContract == creditManager || targetContract == creditFacade() || adapter == creditManager - || adapter == creditFacade() - ) revert TargetContractNotAllowedException(); // I:[CC-10C] + address cf = creditFacade(); + if (targetContract == cf || adapter == cf) { + revert TargetContractNotAllowedException(); // I:[CC-10C] + } address currentAdapter = CreditManagerV3(creditManager).contractToAdapter(targetContract); if (currentAdapter != address(0)) { @@ -420,6 +420,7 @@ contract CreditConfiguratorV3 is ICreditConfiguratorV3, ACLNonReentrantTrait { /// @notice Sets the new price oracle contract in the credit manager /// @param newPriceOracle New price oracle + /// @dev Reverts if `newPriceOracle` does not have price feeds for all collateral tokens function setPriceOracle(address newPriceOracle) external override @@ -427,6 +428,16 @@ contract CreditConfiguratorV3 is ICreditConfiguratorV3, ACLNonReentrantTrait { { if (newPriceOracle == CreditManagerV3(creditManager).priceOracle()) return; + uint256 num = CreditManagerV3(creditManager).collateralTokensCount(); + unchecked { + for (uint256 i; i < num; ++i) { + address token = CreditManagerV3(creditManager).getTokenByMask(1 << i); + if (IPriceOracleV3(newPriceOracle).priceFeeds(token) == address(0)) { + revert PriceFeedDoesNotExistException(); // I:[CC-21] + } + } + } + CreditManagerV3(creditManager).setPriceOracle(newPriceOracle); // I:[CC-21] emit SetPriceOracle(newPriceOracle); // I:[CC-21] } @@ -453,6 +464,7 @@ contract CreditConfiguratorV3 is ICreditConfiguratorV3, ACLNonReentrantTrait { /// @param newCreditFacade New credit facade /// @param migrateParams Whether to migrate old credit facade params /// @dev Reverts if `newCreditFacade` is incompatible with credit manager + /// @dev Reverts if `newCreditFacade` is one of allowed adapters or their target contracts function setCreditFacade(address newCreditFacade, bool migrateParams) external override @@ -462,6 +474,10 @@ contract CreditConfiguratorV3 is ICreditConfiguratorV3, ACLNonReentrantTrait { if (newCreditFacade == address(prevCreditFacade)) return; _revertIfContractIncompatible(newCreditFacade); // I:[CC-20] + if ( + CreditManagerV3(creditManager).adapterToContract(newCreditFacade) != address(0) + || CreditManagerV3(creditManager).contractToAdapter(newCreditFacade) != address(0) + ) revert TargetContractNotAllowedException(); // I:[CC-22B] CreditManagerV3(creditManager).setCreditFacade(newCreditFacade); // I:[CC-22] @@ -472,7 +488,7 @@ contract CreditConfiguratorV3 is ICreditConfiguratorV3, ACLNonReentrantTrait { _setLimits({minDebt: minDebt, maxDebt: maxDebt}); // I:[CC-22] (, uint128 maxCumulativeLoss) = prevCreditFacade.lossParams(); - _setMaxCumulativeLoss(maxCumulativeLoss); // [CC-22] + _setMaxCumulativeLoss(maxCumulativeLoss); // I:[CC-22] _migrateEmergencyLiquidators(prevCreditFacade); // I:[CC-22C] @@ -515,6 +531,7 @@ contract CreditConfiguratorV3 is ICreditConfiguratorV3, ACLNonReentrantTrait { /// @notice Upgrades credit manager's configurator contract /// @param newCreditConfigurator New credit configurator /// @dev Reverts if `newCreditConfigurator` is incompatible with credit manager + /// @dev Reverts if sets of allowed adapters of this contract and new configurator are inconsistent function upgradeCreditConfigurator(address newCreditConfigurator) external override @@ -523,6 +540,16 @@ contract CreditConfiguratorV3 is ICreditConfiguratorV3, ACLNonReentrantTrait { if (newCreditConfigurator == address(this)) return; _revertIfContractIncompatible(newCreditConfigurator); // I:[CC-20] + + address[] memory newAllowedAdapters = CreditConfiguratorV3(newCreditConfigurator).allowedAdapters(); + uint256 num = newAllowedAdapters.length; + if (num != allowedAdaptersSet.length()) revert IncorrectAdaptersSetException(); // I:[CC-23] + unchecked { + for (uint256 i; i < num; ++i) { + if (!allowedAdaptersSet.contains(newAllowedAdapters[i])) revert IncorrectAdaptersSetException(); // I:[CC-23] + } + } + CreditManagerV3(creditManager).setCreditConfigurator(newCreditConfigurator); // I:[CC-23] emit CreditConfiguratorUpgraded(newCreditConfigurator); // I:[CC-23] } diff --git a/contracts/credit/CreditFacadeV3.sol b/contracts/credit/CreditFacadeV3.sol index f642a0f3..7958d085 100644 --- a/contracts/credit/CreditFacadeV3.sol +++ b/contracts/credit/CreditFacadeV3.sol @@ -155,7 +155,8 @@ contract CreditFacadeV3 is ICreditFacadeV3, ACLNonReentrantTrait { /// @param _botList Bot list address /// @param _weth WETH token address /// @param _degenNFT Degen NFT address or `address(0)` - /// @param _expirable Whether this facade should be expirable + /// @param _expirable Whether this facade should be expirable. If `true`, the expiration date remains unset, + /// and facade never expires, unless the date is set via `setExpirationDate` in the configurator. constructor( address _acl, address _creditManager, diff --git a/contracts/credit/CreditManagerV3.sol b/contracts/credit/CreditManagerV3.sol index af7e40ff..39ed57f2 100644 --- a/contracts/credit/CreditManagerV3.sol +++ b/contracts/credit/CreditManagerV3.sol @@ -152,7 +152,10 @@ contract CreditManagerV3 is ICreditManagerV3, SanityCheckTrait, ReentrancyGuardT /// @param _feeInterest Percentage of accrued interest in bps to take by the protocol as profit /// @param _name Credit manager name /// @dev Adds pool's underlying as collateral token with LT = 0 - /// @dev Sets `msg.sender` as credit configurator + /// @dev Reverts if `_priceOracle` does not have a price feed for underlying + /// @dev Sets `msg.sender` as credit configurator, which MUST then pass the role to `CreditConfiguratorV3` contract + /// once it's deployed via `setCreditConfigurator`. The latter performs crucial sanity checks, and configuring + /// the credit manager without them might have dramatic consequences. /// @dev Reverts if `_maxEnabledTokens` is zero constructor( address _pool, @@ -172,6 +175,7 @@ contract CreditManagerV3 is ICreditManagerV3, SanityCheckTrait, ReentrancyGuardT name = _name; // U:[CM-1] underlying = IPoolV3(_pool).underlyingToken(); // U:[CM-1] + if (IPriceOracleV3(_priceOracle).priceFeeds(underlying) == address(0)) revert PriceFeedDoesNotExistException(); // U:[CM-1] _addToken(underlying); // U:[CM-1] creditConfigurator = msg.sender; // U:[CM-1] diff --git a/contracts/interfaces/IExceptions.sol b/contracts/interfaces/IExceptions.sol index 63610d71..01a2234e 100644 --- a/contracts/interfaces/IExceptions.sol +++ b/contracts/interfaces/IExceptions.sol @@ -150,6 +150,9 @@ error IncompatibleContractException(); /// @notice Thrown if attempting to forbid an adapter that is not registered in the credit manager error AdapterIsNotRegisteredException(); +/// @notice Thrown if new credit configurator's set of allowed adapters differs from the current one +error IncorrectAdaptersSetException(); + // ------------- // // CREDIT FACADE // // ------------- // diff --git a/contracts/interfaces/base/IAdapter.sol b/contracts/interfaces/base/IAdapter.sol index 0f10be66..b9777028 100644 --- a/contracts/interfaces/base/IAdapter.sol +++ b/contracts/interfaces/base/IAdapter.sol @@ -16,8 +16,10 @@ interface IAdapter { function _gearboxAdapterVersion() external view returns (uint16); /// @notice Credit manager this adapter is connected to + /// @dev Assumed to be an immutable state variable function creditManager() external view returns (address); /// @notice Target contract adapter helps to interact with + /// @dev Assumed to be an immutable state variable function targetContract() external view returns (address); } diff --git a/contracts/test/integration/credit/CreditConfigurator.int.t.sol b/contracts/test/integration/credit/CreditConfigurator.int.t.sol index d75f1cd4..328752ba 100644 --- a/contracts/test/integration/credit/CreditConfigurator.int.t.sol +++ b/contracts/test/integration/credit/CreditConfigurator.int.t.sol @@ -7,6 +7,7 @@ import "../../interfaces/IAddressProviderV3.sol"; import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; import {CreditFacadeV3} from "../../../credit/CreditFacadeV3.sol"; import {CreditManagerV3} from "../../../credit/CreditManagerV3.sol"; +import {PriceOracleV3} from "../../../core/PriceOracleV3.sol"; import {CreditConfiguratorV3, AllowanceAction} from "../../../credit/CreditConfiguratorV3.sol"; import {ICreditManagerV3} from "../../../interfaces/ICreditManagerV3.sol"; @@ -759,14 +760,25 @@ contract CreditConfiguratorIntegrationTest is IntegrationTestHelper, ICreditConf /// @dev I:[CC-21]: setPriceOracle upgrades priceOracle correctly function test_I_CC_21_setPriceOracle_upgrades_priceOracle_correctly() public creditTest { + PriceOracleV3 newPriceOracle = new PriceOracleV3(address(acl)); + vm.startPrank(CONFIGURATOR); - vm.expectEmit(true, false, false, false); - emit SetPriceOracle(DUMB_ADDRESS); + vm.expectRevert(PriceFeedDoesNotExistException.selector); + creditConfigurator.setPriceOracle(address(newPriceOracle)); + + uint256 num = creditManager.collateralTokensCount(); + for (uint256 i; i < num; ++i) { + address token = creditManager.getTokenByMask(1 << i); + newPriceOracle.setPriceFeed(token, priceOracle.priceFeeds(token), 1); + } + + vm.expectEmit(true, true, true, true); + emit SetPriceOracle(address(newPriceOracle)); - creditConfigurator.setPriceOracle(DUMB_ADDRESS); + creditConfigurator.setPriceOracle(address(newPriceOracle)); - assertEq(address(creditManager.priceOracle()), DUMB_ADDRESS); + assertEq(address(creditManager.priceOracle()), address(newPriceOracle)); vm.stopPrank(); } @@ -850,6 +862,30 @@ contract CreditConfiguratorIntegrationTest is IntegrationTestHelper, ICreditConf } } + /// @dev I:[CC-22B]: setCreditFacade reverts if new facade is adapter or target contract + function test_I_CC_22B_setCreditFacade_reverts_if_new_facade_is_adapter() public creditTest { + vm.startPrank(CONFIGURATOR); + + CreditFacadeV3 cf = + new CreditFacadeV3(address(acl), address(creditManager), address(0), address(0), address(0), false); + AdapterMock adapter = new AdapterMock(address(creditManager), address(cf)); + TargetContractMock target = new TargetContractMock(); + + vm.mockCall(address(cf), abi.encodeCall(IAdapter.targetContract, ()), abi.encode(address(target))); + creditConfigurator.allowAdapter(address(cf)); + + vm.expectRevert(TargetContractNotAllowedException.selector); + creditConfigurator.setCreditFacade(address(cf), false); + + creditConfigurator.forbidAdapter(address(cf)); + creditConfigurator.allowAdapter(address(adapter)); + + vm.expectRevert(TargetContractNotAllowedException.selector); + creditConfigurator.setCreditFacade(address(cf), false); + + vm.stopPrank(); + } + /// @dev I:[CC-22C]: setCreditFacade correctly migrates array parameters function test_I_CC_22C_setCreditFacade_correctly_migrates_array_parameters() public creditTest { for (uint256 ms = 0; ms < 2; ms++) { @@ -907,14 +943,33 @@ contract CreditConfiguratorIntegrationTest is IntegrationTestHelper, ICreditConf } /// @dev I:[CC-23]: uupgradeCreditConfigurator upgrades creditConfigurator - function test_I_CC_23_upgradeCreditConfigurator_upgrades_creditConfigurator() public withAdapterMock creditTest { - vm.expectEmit(true, false, false, false); - emit CreditConfiguratorUpgraded(address(adapterMock)); + function test_I_CC_23_upgradeCreditConfigurator_upgrades_creditConfigurator() public creditTest { + TargetContractMock target1 = new TargetContractMock(); + TargetContractMock target2 = new TargetContractMock(); + AdapterMock adapter1 = new AdapterMock(address(creditManager), address(target1)); + AdapterMock adapter2 = new AdapterMock(address(creditManager), address(target2)); + + vm.prank(CONFIGURATOR); + creditConfigurator.allowAdapter(address(adapter1)); + + CreditConfiguratorV3 cc1 = new CreditConfiguratorV3(address(acl), address(creditManager)); + + vm.prank(CONFIGURATOR); + creditConfigurator.allowAdapter(address(adapter2)); + + CreditConfiguratorV3 cc2 = new CreditConfiguratorV3(address(acl), address(creditManager)); + + vm.expectRevert(IncorrectAdaptersSetException.selector); + vm.prank(CONFIGURATOR); + creditConfigurator.upgradeCreditConfigurator(address(cc1)); + + vm.expectEmit(true, true, true, true); + emit CreditConfiguratorUpgraded(address(cc2)); vm.prank(CONFIGURATOR); - creditConfigurator.upgradeCreditConfigurator(address(adapterMock)); + creditConfigurator.upgradeCreditConfigurator(address(cc2)); - assertEq(address(creditManager.creditConfigurator()), address(adapterMock)); + assertEq(address(creditManager.creditConfigurator()), address(cc2)); } /// @dev I:[CC-24]: setMaxDebtPerBlockMultiplier and forbidBorrowing work correctly diff --git a/contracts/test/unit/credit/CreditManagerV3.unit.t.sol b/contracts/test/unit/credit/CreditManagerV3.unit.t.sol index bcffbbf4..a87b8211 100644 --- a/contracts/test/unit/credit/CreditManagerV3.unit.t.sol +++ b/contracts/test/unit/credit/CreditManagerV3.unit.t.sol @@ -132,6 +132,8 @@ contract CreditManagerV3UnitTest is TestHelper, ICreditManagerV3Events, BalanceH poolQuotaKeeperMock = new PoolQuotaKeeperMock(address(poolMock), underlying); poolMock.setPoolQuotaKeeper(address(poolQuotaKeeperMock)); + priceOracleMock.addPriceFeed(underlying, makeAddr("UNDERLYING_PRICE_FEED")); + creditManager = new CreditManagerV3Harness( address(poolMock), address(accountFactory), @@ -310,6 +312,18 @@ contract CreditManagerV3UnitTest is TestHelper, ICreditManagerV3Events, BalanceH isFeeToken ); + PriceOracleMock priceOracleMock2 = new PriceOracleMock(); + vm.expectRevert(PriceFeedDoesNotExistException.selector); + new CreditManagerV3Harness( + address(poolMock), + address(accountFactory), + address(priceOracleMock2), + DEFAULT_MAX_ENABLED_TOKENS, + DEFAULT_FEE_INTEREST, + name, + isFeeToken + ); + assertEq(address(creditManager.pool()), address(poolMock), _testCaseErr("Incorrect pool")); assertEq(creditManager.underlying(), tokenTestSuite.addressOf(Tokens.DAI), _testCaseErr("Incorrect underlying"));