Review fixes (#16)

* feat: feed registry contract

* fix: @hoytech's review comments

Author: totomanov

src/EulerRouter.sol

@@ -4,11 +4,12 @@ pragma solidity 0.8.23;
 import {ERC4626} from "@solady/tokens/ERC4626.sol";
 import {IPriceOracle} from "src/interfaces/IPriceOracle.sol";
 import {Errors} from "src/lib/Errors.sol";
+import {Governable} from "src/lib/Governable.sol";
 
 /// @title EulerRouter
 /// @author Euler Labs (https://www.eulerlabs.com/)
 /// @notice Default Oracle resolver for Euler lending products.
-contract EulerRouter is IPriceOracle {
+contract EulerRouter is Governable, IPriceOracle {
     /// @notice The PriceOracle to call if this router is not configured for base/quote.
     /// @dev If `address(0)` then there is no fallback.
     address public fallbackOracle;
@@ -18,16 +19,14 @@ contract EulerRouter is IPriceOracle {
     /// @dev During resolution the vault is substituted with its asset.
     /// The `inAmount` is augmented by the vault's `convert*` function.
     mapping(address vault => address asset) public resolvedVaults;
-    /// @notice The active governor address. If `address(0)` then the role is renounced.
-    address public governor;
 
-    /// @notice Configure an PriceOracle to resolve base/quote.
+    /// @notice Configure a PriceOracle to resolve base/quote.
     /// @param base The address of the base token.
     /// @param quote The address of the quote token.
     /// @param oracle The address of the PriceOracle that resolves base/quote.
     /// @dev If `oracle` is `address(0)` then the base/quote configuration was removed.
     event ConfigSet(address indexed base, address indexed quote, address indexed oracle);
-    /// @notice Set an PriceOracle as a fallback resolver.
+    /// @notice Set a PriceOracle as a fallback resolver.
     /// @param fallbackOracle The address of the PriceOracle that is called when base/quote is not configured.
     /// @dev If `fallbackOracle` is `address(0)` then there is no fallback resolver.
     event FallbackOracleSet(address indexed fallbackOracle);
@@ -36,18 +35,12 @@ contract EulerRouter is IPriceOracle {
     /// @param asset The address of the vault's asset.
     /// @dev If `asset` is `address(0)` then the configuration was removed.
     event ResolvedVaultSet(address indexed vault, address indexed asset);
-    /// @notice Set the governor of the contract.
-    /// @param oldGovernor The address of the previous governor.
-    /// @param newGovernor The address of the newly appointed governor.
-    event GovernorSet(address indexed oldGovernor, address indexed newGovernor);
 
     /// @notice Deploy EulerRouter.
     /// @param _governor The address of the governor.
-    constructor(address _governor) {
-        _setGovernor(_governor);
-    }
+    constructor(address _governor) Governable(_governor) {}
 
-    /// @notice Configure an PriceOracle to resolve base/quote.
+    /// @notice Configure a PriceOracle to resolve base/quote.
     /// @param base The address of the base token.
     /// @param quote The address of the quote token.
     /// @param oracle The address of the PriceOracle that resolves base/quote.
@@ -57,55 +50,26 @@ contract EulerRouter is IPriceOracle {
         emit ConfigSet(base, quote, oracle);
     }
 
-    /// @notice Clear the configuration for base/quote.
-    /// @param base The address of the base token.
-    /// @param quote The address of the quote token.
-    /// @dev Callable only by the governor.
-    function govClearConfig(address base, address quote) external onlyGovernor {
-        delete oracles[base][quote];
-        emit ConfigSet(base, quote, address(0));
-    }
-
     /// @notice Configure an ERC4626 vault to use internal pricing via `convert*` methods.
     /// @param vault The address of the ERC4626 vault.
+    /// @param set True to configure the vault, false to clear the record.
     /// @dev Callable only by the governor. Vault must be ERC4626.
     /// Only configure internal pricing after verifying that the implementation of
     /// `convertToAssets` and `convertToShares` cannot be manipulated.
-    function govSetResolvedVault(address vault) external onlyGovernor {
-        address asset = ERC4626(vault).asset();
+    function govSetResolvedVault(address vault, bool set) external onlyGovernor {
+        address asset = set ? ERC4626(vault).asset() : address(0);
         resolvedVaults[vault] = asset;
         emit ResolvedVaultSet(vault, asset);
     }
 
-    /// @notice Clear the configuration for internal pricing resolution for a vault.
-    /// @param vault The address of the ERC4626 vault.
-    /// @dev Callable only by the governor.
-    function govClearResolvedVault(address vault) external onlyGovernor {
-        delete resolvedVaults[vault];
-        emit ResolvedVaultSet(vault, address(0));
-    }
-
-    /// @notice Set an PriceOracle as a fallback resolver.
+    /// @notice Set a PriceOracle as a fallback resolver.
     /// @param _fallbackOracle The address of the PriceOracle that is called when base/quote is not configured.
     /// @dev `address(0)` removes the fallback.
     function govSetFallbackOracle(address _fallbackOracle) external onlyGovernor {
         fallbackOracle = _fallbackOracle;
         emit FallbackOracleSet(_fallbackOracle);
     }
 
-    /// @notice Transfer the governor role to another address.
-    /// @param newGovernor The address of the next governor.
-    /// @dev Can only be called by the current governor.
-    function transferGovernance(address newGovernor) external onlyGovernor {
-        _setGovernor(newGovernor);
-    }
-
-    /// @notice Renounce the governor role.
-    /// @dev Sets governor to address(0), effectively removing governance.
-    function renounceGovernance() external onlyGovernor {
-        _setGovernor(address(0));
-    }
-
     /// @inheritdoc IPriceOracle
     function getQuote(uint256 inAmount, address base, address quote) external view returns (uint256) {
         address oracle;
@@ -128,7 +92,7 @@ contract EulerRouter is IPriceOracle {
     /// @param quote The token that is the unit of account.
     /// @dev Implements the following recursive resolution logic:
     /// 1. Check the base case: `base == quote` and terminate if true.
-    /// 2. If an PriceOracle is configured for base/quote in the `oracles` mapping,
+    /// 2. If a PriceOracle is configured for base/quote in the `oracles` mapping,
     ///    return it without transforming the other variables.
     /// 3. If `base` is configured as an ERC4626 vault with internal pricing,
     ///    transform inAmount by calling `convertToAssets` and recurse by substituting `asset` for `base`.
@@ -137,7 +101,7 @@ contract EulerRouter is IPriceOracle {
     /// 5. If there is a fallback oracle, return it without transforming the other variables, else revert.
     /// @return The resolved inAmount.
     /// @return The resolved base.
-    /// @return The resolved base.
+    /// @return The resolved quote.
     /// @return The resolved PriceOracle to call.
     function _resolveOracle(uint256 inAmount, address base, address quote)
         internal
@@ -166,20 +130,4 @@ contract EulerRouter is IPriceOracle {
         if (oracle == address(0)) revert Errors.PriceOracle_NotSupported(base, quote);
         return (inAmount, base, quote, oracle);
     }
-
-    /// @notice Set the governor address.
-    /// @param newGovernor The address of the new governor.
-    function _setGovernor(address newGovernor) internal {
-        address oldGovernor = governor;
-        governor = newGovernor;
-        emit GovernorSet(oldGovernor, newGovernor);
-    }
-
-    /// @notice Restrict access to the governor.
-    modifier onlyGovernor() {
-        if (msg.sender != governor) {
-            revert Errors.Governance_CallerNotGovernor();
-        }
-        _;
-    }
 }

src/FeedRegistry.sol

@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity 0.8.23;
+
+import {Errors} from "src/lib/Errors.sol";
+import {Governable} from "src/lib/Governable.sol";
+
+contract FeedRegistry is Governable {
+    address public immutable quote;
+    mapping(bytes32 feedId => address base) public feeds;
+
+    event FeedSet(bytes32 indexed feedId, address indexed base);
+
+    constructor(address _governor, address _quote) Governable(_governor) {
+        quote = _quote;
+    }
+
+    function setFeeds(bytes32[] calldata feedIds, address[] calldata bases) external onlyGovernor {
+        _setFeeds(feedIds, bases);
+    }
+
+    function _setFeeds(bytes32[] calldata _feedIds, address[] calldata _bases) internal {
+        if (_feedIds.length != _bases.length) revert Errors.PriceOracle_InvalidConfiguration();
+        for (uint256 i = 0; i < _feedIds.length; ++i) {
+            _setFeed(_feedIds[i], _bases[i]);
+        }
+    }
+
+    function _setFeed(bytes32 _feedId, address _base) internal {
+        if (feeds[_feedId] != address(0)) revert Errors.PriceOracle_InvalidConfiguration();
+        feeds[_feedId] = _base;
+        emit FeedSet(_feedId, _base);
+    }
+}

src/adapter/chainlink/ChainlinkOracle.sol

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: UNLICENSED
 pragma solidity 0.8.23;
 
-import {ERC20} from "@solady/tokens/ERC20.sol";
+import {IERC20} from "forge-std/interfaces/IERC20.sol";
 import {BaseAdapter} from "src/adapter/BaseAdapter.sol";
 import {AggregatorV3Interface} from "src/adapter/chainlink/AggregatorV3Interface.sol";
 import {Errors} from "src/lib/Errors.sol";
@@ -43,8 +43,8 @@ contract ChainlinkOracle is BaseAdapter {
         inverse = _inverse;
 
         // The scale factor is used to correctly convert decimals.
-        int8 baseDecimals = int8(ERC20(base).decimals());
-        int8 quoteDecimals = int8(ERC20(quote).decimals());
+        int8 baseDecimals = int8(IERC20(base).decimals());
+        int8 quoteDecimals = int8(IERC20(quote).decimals());
         int8 feedDecimals = int8(AggregatorV3Interface(feed).decimals());
         int8 scaleDecimals;
         if (inverse) {

src/adapter/pyth/PythOracle.sol

@@ -1,9 +1,9 @@
 // SPDX-License-Identifier: UNLICENSED
 pragma solidity 0.8.23;
 
+import {IERC20} from "forge-std/interfaces/IERC20.sol";
 import {IPyth} from "@pyth/IPyth.sol";
 import {PythStructs} from "@pyth/PythStructs.sol";
-import {ERC20} from "@solady/tokens/ERC20.sol";
 import {BaseAdapter} from "src/adapter/BaseAdapter.sol";
 import {Errors} from "src/lib/Errors.sol";
 
@@ -44,8 +44,8 @@ contract PythOracle is BaseAdapter {
         feedId = _feedId;
         maxStaleness = _maxStaleness;
         inverse = _inverse;
-        uint8 baseDecimals = ERC20(_base).decimals();
-        uint8 quoteDecimals = ERC20(_quote).decimals();
+        uint8 baseDecimals = IERC20(_base).decimals();
+        uint8 quoteDecimals = IERC20(_quote).decimals();
         scaleExponent = inverse ? int8(baseDecimals) - int8(quoteDecimals) : int8(quoteDecimals) - int8(baseDecimals);
     }
 

src/adapter/redstone/RedstoneCoreOracle.sol

@@ -1,10 +1,10 @@
 // SPDX-License-Identifier: UNLICENSED
 pragma solidity 0.8.23;
 
+import {IERC20} from "forge-std/interfaces/IERC20.sol";
 import {RedstoneDefaultsLib} from "@redstone/evm-connector/core/RedstoneDefaultsLib.sol";
 import {PrimaryProdDataServiceConsumerBase} from
     "@redstone/evm-connector/data-services/PrimaryProdDataServiceConsumerBase.sol";
-import {ERC20} from "@solady/tokens/ERC20.sol";
 import {BaseAdapter} from "src/adapter/BaseAdapter.sol";
 import {Errors} from "src/lib/Errors.sol";
 
@@ -29,10 +29,10 @@ contract RedstoneCoreOracle is PrimaryProdDataServiceConsumerBase, BaseAdapter {
     uint256 internal immutable scaleFactor;
     /// @notice The last updated price.
     /// @dev This gets updated after calling `updatePrice`.
-    uint224 public lastPrice;
+    uint208 public lastPrice;
     /// @notice The timestamp of the last update.
     /// @dev Gets updated ot `block.timestamp` after calling `updatePrice`.
-    uint32 public lastUpdatedAt;
+    uint48 public lastUpdatedAt;
 
     /// @notice Deploy a RedstoneCoreOracle.
     /// @param _base The address of the base asset corresponding to the feed.
@@ -53,7 +53,7 @@ contract RedstoneCoreOracle is PrimaryProdDataServiceConsumerBase, BaseAdapter {
         maxStaleness = _maxStaleness;
         inverse = _inverse;
 
-        uint8 decimals = ERC20(inverse ? _quote : _base).decimals();
+        uint8 decimals = IERC20(inverse ? _quote : _base).decimals();
         scaleFactor = 10 ** decimals;
     }
 
@@ -64,9 +64,9 @@ contract RedstoneCoreOracle is PrimaryProdDataServiceConsumerBase, BaseAdapter {
         if (block.timestamp < lastUpdatedAt + maxStaleness) return;
 
         uint256 price = getOracleNumericValueFromTxMsg(feedId);
-        if (price > type(uint224).max) revert Errors.PriceOracle_Overflow();
-        lastPrice = uint224(price);
-        lastUpdatedAt = uint32(block.timestamp);
+        if (price > type(uint208).max) revert Errors.PriceOracle_Overflow();
+        lastPrice = uint208(price);
+        lastUpdatedAt = uint48(block.timestamp);
     }
 
     /// @notice Get the quote from the Redstone feed.

src/adapter/uniswap/UniswapV3Oracle.sol

@@ -43,7 +43,6 @@ contract UniswapV3Oracle is BaseAdapter {
     }
 
     /// @notice Get a quote by calling the pool's TWAP oracle.
-    /// @dev Supports spot pricing if twapWindow=0.
     /// @param inAmount The amount of `base` to convert.
     /// @param base The token that is being priced. Either `tokenA` or `tokenB`.
     /// @param quote The token that is the unit of account. Either `tokenB` or `tokenA`.

src/interfaces/IFactoryInitializable.sol

@@ -15,9 +15,6 @@ interface IFactoryInitializable {
     /// @param newGovernor The address of the next governor.
     /// @dev Can only be called by the current governor.
     function transferGovernance(address newGovernor) external;
-    /// @notice Remove the governor.
-    /// @dev Sets governor to address(0), effectively removing governance.
-    function renounceGovernance() external;
     /// @notice Check whether the contract has been initialized.
     /// @return Whether `initialize` has been called.
     function initialized() external view returns (bool);

src/lib/Governable.sol

@@ -0,0 +1,44 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity 0.8.23;
+
+import {Errors} from "src/lib/Errors.sol";
+
+/// @title Governable
+/// @author Euler Labs (https://www.eulerlabs.com/)
+/// @notice Contract mixin for governance.
+abstract contract Governable {
+    /// @notice The active governor address. If `address(0)` then the role is renounced.
+    address public governor;
+
+    /// @notice Set the governor of the contract.
+    /// @param oldGovernor The address of the previous governor.
+    /// @param newGovernor The address of the newly appointed governor.
+    event GovernorSet(address indexed oldGovernor, address indexed newGovernor);
+
+    constructor(address _governor) {
+        _setGovernor(_governor);
+    }
+
+    /// @notice Transfer the governor role to another address.
+    /// @param newGovernor The address of the next governor.
+    /// @dev Can only be called by the current governor.
+    function transferGovernance(address newGovernor) external onlyGovernor {
+        _setGovernor(newGovernor);
+    }
+
+    /// @notice Restrict access to the governor.
+    modifier onlyGovernor() {
+        if (msg.sender != governor) {
+            revert Errors.Governance_CallerNotGovernor();
+        }
+        _;
+    }
+
+    /// @notice Set the governor address.
+    /// @param newGovernor The address of the new governor.
+    function _setGovernor(address newGovernor) internal {
+        address oldGovernor = governor;
+        governor = newGovernor;
+        emit GovernorSet(oldGovernor, newGovernor);
+    }
+}

test/fork/adapter/lido/LidoOracle.fork.sol

@@ -14,15 +14,15 @@ contract LidoOracleForkTest is ForkTest {
         oracle = new LidoOracle(STETH, WSTETH);
     }
 
-    function test_GetQuote_Integrity() public {
+    function test_GetQuote_Integrity() public view {
         uint256 stEthWstEth = oracle.getQuote(1e18, STETH, WSTETH);
         assertApproxEqRel(stEthWstEth, 0.85e18, 0.1e18);
 
         uint256 wstEthStEth = oracle.getQuote(1e18, WSTETH, STETH);
         assertApproxEqRel(wstEthStEth, 1.15e18, 0.1e18);
     }
 
-    function test_GetQuotes_Integrity() public {
+    function test_GetQuotes_Integrity() public view {
         (uint256 stEthWstEthBid, uint256 stEthWstEthAsk) = oracle.getQuotes(1e18, STETH, WSTETH);
         assertApproxEqRel(stEthWstEthBid, 0.85e18, 0.1e18);
         assertApproxEqRel(stEthWstEthAsk, 0.85e18, 0.1e18);

test/fork/adapter/rocketpool/RethOracle.fork.t.sol

@@ -14,15 +14,15 @@ contract RethOracleForkTest is ForkTest {
         oracle = new RethOracle(WETH, RETH);
     }
 
-    function test_GetQuote_Integrity() public {
+    function test_GetQuote_Integrity() public view {
         uint256 wethReth = oracle.getQuote(1e18, WETH, RETH);
         assertApproxEqRel(wethReth, 0.9e18, 0.1e18);
 
         uint256 rethWeth = oracle.getQuote(1e18, RETH, WETH);
         assertApproxEqRel(rethWeth, 1.1e18, 0.1e18);
     }
 
-    function test_GetQuotes_Integrity() public {
+    function test_GetQuotes_Integrity() public view {
         (uint256 wethRethBid, uint256 wethRethAsk) = oracle.getQuotes(1e18, WETH, RETH);
         assertApproxEqRel(wethRethBid, 0.9e18, 0.1e18);
         assertApproxEqRel(wethRethAsk, 0.9e18, 0.1e18);