feat: general credit configuration fixes (#229)

In this PR:
* `CreditConfiguratorV3.setPriceOracle` now ensures that new price oracle has all required price feeds
* `CreditManagerV3`'s constructor now ensures that price oracle has a price feed for the underlying token
* `CreditConfiguratorV3.upgradeCreditConfigurator` now ensures that set of allowed adapters of the new configurator coincides with the one of the current
* `CreditConfiguratorV3.setCreditFacade` now ensures that new credit facade is not an adapter or some adapter's target contract
* More comments on system behaviour

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,13 +420,24 @@ 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
         configuratorOnly // I:[CC-2]
     {
         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]
     }

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,

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]

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 //
 // ------------- //

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);
 }

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

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"));