fix: carefully handle optional calls to external contracts (#297)

contracts/credit/CreditConfiguratorV3.sol

@@ -10,6 +10,7 @@ import {Address} from "@openzeppelin/contracts/utils/Address.sol";
 
 // LIBRARIES & CONSTANTS
 import {BitMask} from "../libraries/BitMask.sol";
+import {OptionalCall} from "../libraries/OptionalCall.sol";
 import {PERCENTAGE_FACTOR, UNDERLYING_TOKEN_MASK, WAD} from "../libraries/Constants.sol";
 import {MarketHelper} from "../libraries/MarketHelper.sol";
 
@@ -122,9 +123,18 @@ contract CreditConfiguratorV3 is ICreditConfiguratorV3, ACLTrait, SanityCheckTra
             revert IncorrectTokenContractException(); // I:[CC-3]
         }
 
-        try IPhantomToken(token).getPhantomTokenInfo() returns (address, address depositedToken) {
+        /// NOTE: Some external tokens without getPhantomTokenInfo may have a fallback function that changes state, which can cause a THROW
+        ///      that burns all gas, or does not change state and instead returns empty data. To handle these cases,
+        ///      we use a special call construction with a strict gas limit.
+        (bool success, bytes memory returnData) = OptionalCall.staticCallOptionalSafe({
+            target: token,
+            data: abi.encodeWithSelector(IPhantomToken.getPhantomTokenInfo.selector),
+            gasAllowance: 30_000
+        });
+        if (success) {
+            (, address depositedToken) = abi.decode(returnData, (address, address));
             _getTokenMaskOrRevert(depositedToken); // I:[CC-3]
-        } catch {}
+        }
 
         if (IPriceOracleV3(CreditManagerV3(creditManager).priceOracle()).priceFeeds(token) == address(0)) {
             revert PriceFeedDoesNotExistException(); // I:[CC-3]

contracts/libraries/OptionalCall.sol

@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: BUSL-1.1
+// Gearbox Protocol. Generalized leverage for DeFi protocols
+// (c) Gearbox Foundation, 2024.
+pragma solidity ^0.8.17;
+
+/// @title Optional call library
+/// @notice Implements a function that calls a contract that may not have
+///         an expected selector. Handles the case where the contract has a fallback function that may or may
+///         not change state.
+library OptionalCall {
+    function staticCallOptionalSafe(address target, bytes memory data, uint256 gasAllowance)
+        internal
+        view
+        returns (bool, bytes memory)
+    {
+        (bool success, bytes memory returnData) = target.staticcall{gas: gasAllowance}(data);
+        return returnData.length > 0 ? (success, returnData) : (false, returnData);
+    }
+}

contracts/test/integration/credit/CreditConfigurator.int.t.sol

@@ -13,6 +13,7 @@ import {CreditConfiguratorV3, AllowanceAction} from "../../../credit/CreditConfi
 import {ICreditManagerV3} from "../../../interfaces/ICreditManagerV3.sol";
 import {ICreditConfiguratorV3Events} from "../../../interfaces/ICreditConfiguratorV3.sol";
 import {IAdapter} from "../../../interfaces/base/IAdapter.sol";
+import {IPriceOracleV3} from "../../../interfaces/IPriceOracleV3.sol";
 
 //
 import "../../../libraries/Constants.sol";
@@ -33,6 +34,8 @@ import {CreditFacadeV3Harness} from "../../unit/credit/CreditFacadeV3Harness.sol
 import {IntegrationTestHelper} from "../../helpers/IntegrationTestHelper.sol";
 import {FlagState, PriceFeedMock} from "../../mocks/oracles/PriceFeedMock.sol";
 import {PhantomTokenMock} from "../../mocks/token/PhantomTokenMock.sol";
+import {WETHFallbackMock} from "../../mocks/token/WETHFallbackMock.sol";
+import {PriceFeedMock} from "../../mocks/oracles/PriceFeedMock.sol";
 
 // SUITES
 import {TokensTestSuite} from "../../suites/TokensTestSuite.sol";
@@ -1080,4 +1083,28 @@ contract CreditConfiguratorIntegrationTest is IntegrationTestHelper, ICreditConf
         vm.prank(CONFIGURATOR);
         creditConfigurator.rampLiquidationThreshold(usdc, 9000, uint40(block.timestamp - 1), 2 days);
     }
+
+    function test_I_CC_31_addCollateralToken_works_with_fallback() public creditTest {
+        PriceFeedMock pf = new PriceFeedMock(10 ** 8, 8);
+
+        WETHFallbackMock wethFallback = new WETHFallbackMock();
+
+        for (uint256 i = 0; i < 2; i++) {
+            uint256 snapshot = vm.snapshot();
+
+            wethFallback.setDepositOnFallback(i == 0);
+
+            vm.prank(CONFIGURATOR);
+            IPriceOracleV3(address(priceOracle)).setPriceFeed(address(wethFallback), address(pf), 1 hours);
+
+            makeTokenQuoted(address(wethFallback), 1, uint96(type(int96).max));
+
+            vm.prank(CONFIGURATOR);
+            creditConfigurator.addCollateralToken(address(wethFallback), 8800);
+
+            assertTrue(creditManager.getTokenMaskOrRevert(address(wethFallback)) > 0, "Token wasn't added");
+
+            vm.revertTo(snapshot);
+        }
+    }
 }

contracts/test/mocks/oracles/PriceFeedFallbackMock.sol

@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: UNLICENSED
+// Gearbox Protocol. Generalized leverage for DeFi protocols
+// (c) Gearbox Foundation, 2023.
+pragma solidity ^0.8.10;
+
+contract PriceFeedFallbackMock {
+    uint256 public constant version = 3_10;
+    bytes32 public constant contractType = "PRICE_FEED_FALLBACK_MOCK";
+
+    int256 public price;
+    uint8 public immutable decimals;
+
+    bool public immutable changeStateInFallback;
+    bool public fallbackStateFlag;
+
+    uint80 internal roundId;
+    uint256 startedAt;
+    uint256 updatedAt;
+    uint80 internal answerInRound;
+
+    bool internal revertOnLatestRound;
+
+    constructor(int256 _price, uint8 _decimals, bool _changeStateInFallback) {
+        price = _price;
+        decimals = _decimals;
+        changeStateInFallback = _changeStateInFallback;
+        roundId = 80;
+        answerInRound = 80;
+        // set to quite far in the future
+        startedAt = block.timestamp + 36500 days;
+        updatedAt = block.timestamp + 36500 days;
+    }
+
+    function setParams(uint80 _roundId, uint256 _startedAt, uint256 _updatedAt, uint80 _answerInRound) external {
+        roundId = _roundId;
+        startedAt = _startedAt;
+        updatedAt = _updatedAt;
+        answerInRound = _answerInRound;
+    }
+
+    function description() external pure returns (string memory) {
+        return "price oracle";
+    }
+
+    function setPrice(int256 newPrice) external {
+        price = newPrice;
+    }
+
+    function latestRoundData()
+        external
+        view
+        returns (
+            uint80, // roundId,
+            int256, // answer,
+            uint256, // startedAt,
+            uint256, // updatedAt,
+            uint80 //answeredInRound
+        )
+    {
+        if (revertOnLatestRound) revert();
+
+        return (roundId, price, startedAt, updatedAt, answerInRound);
+    }
+
+    fallback() external {
+        if (changeStateInFallback) {
+            fallbackStateFlag = true;
+        }
+    }
+}

contracts/test/mocks/token/WETHFallbackMock.sol

@@ -0,0 +1,76 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity ^0.8.10;
+
+import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
+
+contract WETHFallbackMock is IERC20 {
+    string public name = "Wrapped Ether";
+    string public symbol = "WETH";
+    uint8 public decimals = 18;
+
+    bool public depositOnFallback = true;
+
+    // event Approval(address indexed src, address indexed guy, uint256 wad);
+    // event Transfer(address indexed src, address indexed dst, uint256 wad);
+    event Deposit(address indexed dst, uint256 wad);
+    event Withdrawal(address indexed src, uint256 wad);
+
+    mapping(address => uint256) public balanceOf;
+    mapping(address => mapping(address => uint256)) public allowance;
+
+    function mint(address to, uint256 amount) external {
+        balanceOf[to] += amount;
+    }
+
+    function setDepositOnFallback(bool _depositOnFallback) external {
+        depositOnFallback = _depositOnFallback;
+    }
+
+    fallback() external payable {
+        if (depositOnFallback) {
+            deposit(); // T:[WM-1]
+        }
+    }
+
+    function deposit() public payable {
+        balanceOf[msg.sender] += msg.value; // T:[WM-1]
+        emit Deposit(msg.sender, msg.value); // T:[WM-1]
+    }
+
+    function withdraw(uint256 wad) public {
+        require(balanceOf[msg.sender] >= wad); // T:[WM-2]
+        balanceOf[msg.sender] -= wad; // T:[WM-2]
+        payable(msg.sender).transfer(wad); // T:[WM-3]
+        emit Withdrawal(msg.sender, wad); // T:[WM-4]
+    }
+
+    function totalSupply() public view returns (uint256) {
+        return address(this).balance; // T:[WM-1, 2]
+    }
+
+    function approve(address guy, uint256 wad) public returns (bool) {
+        allowance[msg.sender][guy] = wad; // T:[WM-3]
+        emit Approval(msg.sender, guy, wad); // T:[WM-3]
+        return true;
+    }
+
+    function transfer(address dst, uint256 wad) public returns (bool) {
+        return transferFrom(msg.sender, dst, wad); // T:[WM-4,5,6]
+    }
+
+    function transferFrom(address src, address dst, uint256 wad) public returns (bool) {
+        require(balanceOf[src] >= wad); // T:[WM-4]
+
+        if (src != msg.sender && allowance[src][msg.sender] != type(uint256).max) {
+            require(allowance[src][msg.sender] >= wad); // T:[WM-4]
+            allowance[src][msg.sender] -= wad; // T:[WM-7]
+        }
+
+        balanceOf[src] -= wad; // T:[WM-5]
+        balanceOf[dst] += wad; // T:[WM-5]
+
+        emit Transfer(src, dst, wad); // T:[WM-6]
+
+        return true;
+    }
+}

contracts/test/unit/traits/PriceFeedValidationTrait.t.sol

@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: UNLICENSED
+// Gearbox Protocol. Generalized leverage for DeFi protocols
+// (c) Gearbox Foundation, 2023.
+pragma solidity ^0.8.17;
+
+import {PriceFeedValidationTrait} from "../../../traits/PriceFeedValidationTrait.sol";
+import {TestHelper} from "../../lib/helper.sol";
+import {PriceFeedFallbackMock} from "../../mocks/oracles/PriceFeedFallbackMock.sol";
+
+contract PriceFeedValidationTraitUnitTest is PriceFeedValidationTrait, TestHelper {
+    function test_U_PFVT_01_validatePriceFeed_works_correctly_for_PF_with_fallback() public {
+        address priceFeed = address(new PriceFeedFallbackMock(1e8, 8, false));
+
+        _validatePriceFeed(priceFeed, 1000);
+
+        priceFeed = address(new PriceFeedFallbackMock(1e8, 8, true));
+        _validatePriceFeed(priceFeed, 1000);
+    }
+}

contracts/traits/PriceFeedValidationTrait.sol

@@ -4,6 +4,7 @@
 pragma solidity ^0.8.17;
 
 import {Address} from "@openzeppelin/contracts/utils/Address.sol";
+import {OptionalCall} from "../libraries/OptionalCall.sol";
 
 import {
     AddressIsNotContractException,
@@ -37,9 +38,17 @@ abstract contract PriceFeedValidationTrait {
             revert IncorrectPriceFeedException();
         }
 
-        try IPriceFeed(priceFeed).skipPriceCheck() returns (bool _skipCheck) {
-            skipCheck = _skipCheck;
-        } catch {}
+        /// NOTE: Some external price feeds without skipPriceCheck may have a fallback function that changes state, which can cause a THROW
+        ///      that burns all gas, or does not change state and instead returns empty data. To handle these cases,
+        ///      we use a special call construction with a strict gas limit.
+        (bool success, bytes memory returnData) = OptionalCall.staticCallOptionalSafe({
+            target: priceFeed,
+            data: abi.encodeWithSelector(IPriceFeed.skipPriceCheck.selector),
+            gasAllowance: 10_000
+        });
+        if (success) {
+            skipCheck = abi.decode(returnData, (bool));
+        }
 
         try IPriceFeed(priceFeed).latestRoundData() returns (uint80, int256 answer, uint256, uint256 updatedAt, uint80)
         {