versioned interface implementation with strict ERC-165 validation

foundry.toml

@@ -23,12 +23,13 @@ verbosity = 3
 # The "size" script in package.json replicates these overrides, as "forge build --sizes" does not use the profiles defined here
 additional_compiler_profiles = [
     { name = "ten-runs", optimizer_runs = 10 },
+    { name = "four-thousand-runs", optimizer_runs = 4000 },
     { name = "five-thousand-runs", optimizer_runs = 5000 },
 ]
 compilation_restrictions = [
     { paths = "src/policies/Operator.sol", profile = "ten-runs" },
     { paths = "src/policies/cooler/MonoCooler.sol", profile = "five-thousand-runs" },
-    { paths = "src/modules/PRICE/OlympusPrice.v1_2.sol", version = "0.8.24", profile = "five-thousand-runs" },
+    { paths = "src/modules/PRICE/OlympusPrice.v1_2.sol", version = "0.8.24", profile = "four-thousand-runs" },
     { paths = "src/policies/bridge/CCIPBurnMintTokenPool.sol", version = "0.8.24" },
     { paths = "**", version = "0.8.15" },
 ]

package.json

@@ -30,7 +30,7 @@
         "test:proposal": "forge test --match-path 'src/test/proposals/*.t.sol' --fork-url mainnet -vvv",
         "test:crosschainfork": "forge test --match-contract 'CrossChainBridgeFork|TokenPoolFork' -vvv",
         "test:coverage": "./shell/test_coverage.sh",
-        "size": "forge clean && forge build --sizes --optimizer-runs 10 --contracts src/policies/Operator.sol && forge build --sizes --optimizer-runs 5000 --contracts src/policies/cooler/MonoCooler.sol && forge build --sizes --optimizer-runs 5000 --use 0.8.24 --contracts src/modules/PRICE/OlympusPrice.v1_2.sol && forge build --sizes --skip 'src/test/*' --skip 'Operator.sol' --skip 'MonoCooler.sol' --skip 'OlympusPrice.v1_2.sol'",
+        "size": "forge clean && forge build --sizes --optimizer-runs 10 --contracts src/policies/Operator.sol && forge build --sizes --optimizer-runs 5000 --contracts src/policies/cooler/MonoCooler.sol && forge build --sizes --optimizer-runs 4000 --use 0.8.24 --contracts src/modules/PRICE/OlympusPrice.v1_2.sol && forge build --sizes --skip 'src/test/*' --skip 'Operator.sol' --skip 'MonoCooler.sol' --skip 'OlympusPrice.v1_2.sol'",
         "metrics": "node shell/metrics.js"
     },
     "devDependencies": {

snapshots/DepositRedemptionVaultFinishRedemptionTest.json

@@ -1,3 +1,3 @@
 {
-    "redeem": "201080"
+    "redeem": "203580"
 }

src/Submodules.sol

@@ -4,6 +4,9 @@
 pragma solidity >=0.8.15;
 
 import {Keycode, Module, fromKeycode, ensureContract} from "src/Kernel.sol";
+import {IERC165} from "@openzeppelin-4.8.0/interfaces/IERC165.sol";
+import {IVersioned} from "src/interfaces/IVersioned.sol";
+import {ISubmodule} from "src/interfaces/ISubmodule.sol";
 
 //============================================================================================//
 //                                        GLOBAL TYPES                                        //
@@ -76,6 +79,7 @@ abstract contract ModuleWithSubmodules is Module {
     error Module_SubmoduleAlreadyInstalled(SubKeycode subKeycode_);
     error Module_SubmoduleNotInstalled(SubKeycode subKeycode_);
     error Module_SubmoduleExecutionReverted(bytes error_);
+    error Module_SubmoduleInterfaceNotImplemented(address submodule_);
 
     // ========= SUBMODULE MANAGEMENT ========= //
 
@@ -183,14 +187,26 @@ abstract contract ModuleWithSubmodules is Module {
         SubKeycode subKeycode = newSubmodule_.SUBKEYCODE();
         ensureValidSubKeycode(subKeycode, keycode);
 
+        // Validate that the submodule implements ISubmodule
+        // We check this by calling supportsInterface with the ISubmodule interface ID
+        (bool success, bytes memory data) = address(newSubmodule_).staticcall(
+            abi.encodeWithSelector(IERC165.supportsInterface.selector, type(ISubmodule).interfaceId)
+        );
+        // The call must succeed and return exactly 32 bytes (bool) for ISubmodule
+        // If success is false, data.length != 32, or decoded bool is false, revert
+        if (!success || data.length != 32)
+            revert Module_SubmoduleInterfaceNotImplemented(address(newSubmodule_));
+        if (!abi.decode(data, (bool)))
+            revert Module_SubmoduleInterfaceNotImplemented(address(newSubmodule_));
+
         return subKeycode;
     }
 }
 
 /// @notice Submodules are isolated components of a module that can be upgraded independently.
 /// @dev    Submodules are installed and uninstalled directly on the module.
 /// @dev    If a module is going to hold state that should be persisted across upgrades, then a submodule pattern may be a good fit.
-abstract contract Submodule {
+abstract contract Submodule is IVersioned, ISubmodule {
     error Submodule_OnlyParent(address caller_);
     error Submodule_ModuleDoesNotExist(Keycode keycode_);
     error Submodule_InvalidParent();
@@ -224,7 +240,18 @@ abstract contract Submodule {
     /// @return major - Major version upgrade indicates breaking change to the interface.
     /// @dev    A major (breaking) change may require the parent module to be updated as well.
     /// @return minor - Minor version change retains backward-compatible interface.
-    function VERSION() external pure virtual returns (uint8 major, uint8 minor) {}
+    /// @inheritdoc IVersioned
+    function VERSION() external pure virtual override returns (uint8 major, uint8 minor) {}
+
+    /// @notice Query if a contract implements an interface
+    /// @param interfaceId The interface identifier, as specified in ERC-165
+    /// @return bool True if the contract supports interfaceId_
+    function supportsInterface(bytes4 interfaceId) public pure virtual returns (bool) {
+        return
+            interfaceId == type(IERC165).interfaceId ||
+            interfaceId == type(IVersioned).interfaceId ||
+            interfaceId == type(ISubmodule).interfaceId;
+    }
 
     /// @notice Initialization function for the submodule
     /// @dev    This function is called when the submodule is installed or upgraded by the module.

src/interfaces/ISubmodule.sol

@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: MIT
+/// forge-lint: disable-start(mixed-case-function)
+pragma solidity >=0.8.0;
+
+import {Keycode} from "src/Kernel.sol";
+import {SubKeycode} from "src/Submodules.sol";
+import {IVersioned} from "src/interfaces/IVersioned.sol";
+
+/// @title ISubmodule
+/// @notice Interface for Bophades submodules
+/// @dev    Submodules are isolated components of a module that can be upgraded independently
+interface ISubmodule is IVersioned {
+    /// @notice 5 byte identifier for the parent module
+    /// @return The keycode of the parent module
+    function PARENT() external pure returns (Keycode);
+
+    /// @notice 20 byte identifier for the submodule. First 5 bytes must match PARENT()
+    /// @return The subkeycode of this submodule
+    function SUBKEYCODE() external pure returns (SubKeycode);
+
+    /// @notice Initialization function for the submodule
+    /// @dev    This function is called when the submodule is installed or upgraded by the module
+    /// @dev    MUST BE GATED BY onlyParent. Used to encompass any initialization or upgrade logic
+    function INIT() external;
+}
+/// forge-lint: disable-end(mixed-case-function)

src/modules/PRICE/PRICE.v2.sol

@@ -40,6 +40,11 @@ abstract contract PRICEv2 is ModuleWithSubmodules, IPRICEv2 {
         return _decimals;
     }
 
+    /// @notice Returns the version of the PRICEv2 module
+    function VERSION() external pure virtual override returns (uint8 major, uint8 minor) {
+        return (2, 0);
+    }
+
     function supportsInterface(bytes4 interfaceId) external view virtual returns (bool) {
         return
             interfaceId == type(IERC165).interfaceId || interfaceId == type(IPRICEv2).interfaceId;

src/modules/PRICE/submodules/feeds/BalancerPoolTokenPrice.sol

@@ -5,22 +5,22 @@ pragma solidity >=0.8.15;
 
 // Interfaces
 import {IPRICEv2} from "src/modules/PRICE/IPRICE.v2.sol";
+import {IStablePool} from "src/libraries/Balancer/interfaces/IStablePool.sol";
 import {IVault} from "src/libraries/Balancer/interfaces/IVault.sol";
 import {IWeightedPool} from "src/libraries/Balancer/interfaces/IWeightedPool.sol";
-import {IStablePool} from "src/libraries/Balancer/interfaces/IStablePool.sol";
 
 // Libraries
 import {ERC20} from "@solmate-6.2.0/tokens/ERC20.sol";
+import {FixedPoint} from "src/libraries/Balancer/math/FixedPoint.sol";
 import {FullMath} from "src/libraries/FullMath.sol";
+import {LogExpMath} from "src/libraries/Balancer/math/LogExpMath.sol";
 import {StableMath} from "src/libraries/Balancer/math/StableMath.sol";
 import {VaultReentrancyLib} from "src/libraries/Balancer/contracts/VaultReentrancyLib.sol";
-import {LogExpMath} from "src/libraries/Balancer/math/LogExpMath.sol";
-import {FixedPoint} from "src/libraries/Balancer/math/FixedPoint.sol";
 
 // Bophades
 import {Module} from "src/Kernel.sol";
-import {Submodule, SubKeycode, toSubKeycode} from "src/Submodules.sol";
 import {PriceSubmodule} from "modules/PRICE/PRICE.v2.sol";
+import {Submodule, SubKeycode, toSubKeycode} from "src/Submodules.sol";
 
 /// @title      BalancerPoolTokenPrice
 /// @author     0xJem
@@ -203,8 +203,7 @@ contract BalancerPoolTokenPrice is PriceSubmodule {
 
     /// @inheritdoc Submodule
     function VERSION() public pure override returns (uint8 major, uint8 minor) {
-        major = 1;
-        minor = 0;
+        return (1, 0);
     }
 
     // ========== HELPER FUNCTIONS ========== //

src/modules/PRICE/submodules/feeds/ChainlinkPriceFeeds.sol

@@ -6,12 +6,12 @@ pragma solidity >=0.8.15;
 import {AggregatorV2V3Interface} from "interfaces/AggregatorV2V3Interface.sol";
 
 // Libraries
-import {FullMath} from "libraries/FullMath.sol";
+import {FullMath} from "src/libraries/FullMath.sol";
 
 // Bophades
 import {Module} from "src/Kernel.sol";
-import {Submodule, SubKeycode, toSubKeycode} from "src/Submodules.sol";
 import {PriceSubmodule} from "modules/PRICE/PRICE.v2.sol";
+import {Submodule, SubKeycode, toSubKeycode} from "src/Submodules.sol";
 
 /// @title      ChainlinkPriceFeeds
 /// @author     0xJem
@@ -121,8 +121,7 @@ contract ChainlinkPriceFeeds is PriceSubmodule {
 
     /// @inheritdoc      Submodule
     function VERSION() public pure override returns (uint8 major, uint8 minor) {
-        major = 1;
-        minor = 0;
+        return (1, 0);
     }
 
     // ========== PRICE FEED FUNCTIONS ========== //

src/modules/PRICE/submodules/feeds/ERC4626Price.sol

@@ -6,14 +6,14 @@ pragma solidity >=0.8.15;
 import {IPRICEv2} from "src/modules/PRICE/IPRICE.v2.sol";
 
 // Libraries
-import {FullMath} from "src/libraries/FullMath.sol";
 import {ERC20} from "@solmate-6.2.0/tokens/ERC20.sol";
 import {ERC4626} from "@solmate-6.2.0/mixins/ERC4626.sol";
+import {FullMath} from "src/libraries/FullMath.sol";
 
 // Bophades
 import {Module} from "src/Kernel.sol";
+import {PRICEv2, PriceSubmodule} from "modules/PRICE/PRICE.v2.sol";
 import {Submodule, SubKeycode, toSubKeycode} from "src/Submodules.sol";
-import {PriceSubmodule, PRICEv2} from "modules/PRICE/PRICE.v2.sol";
 
 /// @title      ERC4626Price
 /// @author     0xJem
@@ -68,8 +68,7 @@ contract ERC4626Price is PriceSubmodule {
 
     /// @inheritdoc      Submodule
     function VERSION() public pure override returns (uint8 major, uint8 minor) {
-        major = 1;
-        minor = 0;
+        return (1, 0);
     }
 
     // ========== PRICE FUNCTIONS ========== //

src/modules/PRICE/submodules/feeds/PythPriceFeeds.sol

@@ -4,17 +4,15 @@ pragma solidity >=0.8.15;
 
 // Interfaces
 import {IPyth} from "src/interfaces/IPyth.sol";
-import {IERC165} from "@openzeppelin-4.8.0/interfaces/IERC165.sol";
-import {IVersioned} from "src/interfaces/IVersioned.sol";
 
 // Libraries
 import {FullMath} from "src/libraries/FullMath.sol";
 import {SafeCast} from "src/libraries/SafeCast.sol";
 
 // Bophades
 import {Module} from "src/Kernel.sol";
+import {PriceSubmodule} from "modules/PRICE/PRICE.v2.sol";
 import {Submodule, SubKeycode, toSubKeycode} from "src/Submodules.sol";
-import {PriceSubmodule} from "src/modules/PRICE/PRICE.v2.sol";
 
 /// @title      PythPriceFeeds
 /// @author     0xJem
@@ -152,8 +150,7 @@ contract PythPriceFeeds is PriceSubmodule {
 
     /// @inheritdoc      Submodule
     function VERSION() public pure override returns (uint8 major, uint8 minor) {
-        major = 1;
-        minor = 0;
+        return (1, 0);
     }
 
     // ========== PRICE FEED FUNCTIONS ========== //
@@ -431,16 +428,5 @@ contract PythPriceFeeds is PriceSubmodule {
 
         return priceResult;
     }
-
-    // ========== IERC165 FUNCTIONS ========== //
-
-    /// @notice     Returns whether the contract supports the given interface
-    ///
-    /// @param interfaceId The interface identifier to check
-    /// @return bool Whether the contract supports the interface
-    function supportsInterface(bytes4 interfaceId) external view virtual returns (bool) {
-        return
-            interfaceId == type(IERC165).interfaceId || interfaceId == type(IVersioned).interfaceId;
-    }
 }
 /// forge-lint: disable-end(mixed-case-function)

src/modules/PRICE/submodules/feeds/UniswapV2PoolTokenPrice.sol

@@ -13,8 +13,8 @@ import {FullMath} from "src/libraries/FullMath.sol";
 
 // Bophades
 import {Module} from "src/Kernel.sol";
+import {PRICEv2, PriceSubmodule} from "modules/PRICE/PRICE.v2.sol";
 import {Submodule, SubKeycode, toSubKeycode} from "src/Submodules.sol";
-import {PriceSubmodule, PRICEv2} from "modules/PRICE/PRICE.v2.sol";
 
 /// @title      UniswapV2PoolTokenPrice
 /// @author     0xJem
@@ -119,8 +119,7 @@ contract UniswapV2PoolTokenPrice is PriceSubmodule {
 
     /// @inheritdoc      Submodule
     function VERSION() public pure override returns (uint8 major, uint8 minor) {
-        major = 1;
-        minor = 0;
+        return (1, 0);
     }
 
     // ========== HELPER FUNCTIONS ========== //

src/modules/PRICE/submodules/feeds/UniswapV3Price.sol

@@ -8,14 +8,14 @@ import {IUniswapV3Pool} from "@uniswap-v3-core-1.0.1/interfaces/IUniswapV3Pool.s
 
 // Libraries
 import {ERC20} from "@solmate-6.2.0/tokens/ERC20.sol";
-import {UniswapV3OracleHelper as OracleHelper} from "libraries/UniswapV3/Oracle.sol";
-import {FullMath} from "libraries/FullMath.sol";
+import {FullMath} from "src/libraries/FullMath.sol";
+import {UniswapV3OracleHelper as OracleHelper} from "src/libraries/UniswapV3/Oracle.sol";
 import {OracleLibrary} from "@uniswap-v3-periphery-1.4.2/libraries/OracleLibrary.sol";
 
 // Bophades
 import {Module} from "src/Kernel.sol";
-import {Submodule, SubKeycode, toSubKeycode} from "src/Submodules.sol";
 import {PriceSubmodule} from "modules/PRICE/PRICE.v2.sol";
+import {Submodule, SubKeycode, toSubKeycode} from "src/Submodules.sol";
 
 /// @title      UniswapV3Price
 /// @author     0xJem
@@ -101,8 +101,7 @@ contract UniswapV3Price is PriceSubmodule {
 
     /// @inheritdoc      Submodule
     function VERSION() public pure override returns (uint8 major, uint8 minor) {
-        major = 1;
-        minor = 0;
+        return (1, 0);
     }
 
     // ========== TOKEN PRICE FUNCTIONS ========== //

src/modules/PRICE/submodules/strategies/SimplePriceFeedStrategy.sol

@@ -3,13 +3,13 @@
 pragma solidity >=0.8.15;
 
 // Libraries
-import {QuickSort} from "libraries/QuickSort.sol";
-import {Deviation} from "libraries/Deviation.sol";
+import {Deviation} from "src/libraries/Deviation.sol";
+import {QuickSort} from "src/libraries/QuickSort.sol";
 
 // Bophades
 import {Module} from "src/Kernel.sol";
-import {Submodule, SubKeycode, toSubKeycode} from "src/Submodules.sol";
 import {PriceSubmodule} from "modules/PRICE/PRICE.v2.sol";
+import {Submodule, SubKeycode, toSubKeycode} from "src/Submodules.sol";
 
 /// @title      SimplePriceFeedStrategy
 /// @author     0xJem
@@ -53,8 +53,7 @@ contract SimplePriceFeedStrategy is PriceSubmodule {
 
     /// @inheritdoc      Submodule
     function VERSION() public pure override returns (uint8 major, uint8 minor) {
-        major = 1;
-        minor = 0;
+        return (1, 0);
     }
 
     // ========== HELPER FUNCTIONS ========== //

src/test/mocks/MockInvalidSubmodule.sol

@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: AGPL-3.0
+/// forge-lint: disable-start(mixed-case-function)
+pragma solidity >=0.8.15;
+
+import {Keycode, Module, toKeycode} from "src/Kernel.sol";
+import {ISubmodule} from "src/interfaces/ISubmodule.sol";
+import {Submodule, SubKeycode, toSubKeycode} from "src/Submodules.sol";
+
+/// @notice Mock submodule that does not implement ISubmodule correctly
+/// @dev    This contract does NOT implement supportsInterface properly, so it should fail validation
+contract MockInvalidSubmodule is Submodule {
+    constructor(Module parent_) Submodule(parent_) {}
+
+    function SUBKEYCODE() public pure override returns (SubKeycode) {
+        return toSubKeycode("PRICE.INVALID");
+    }
+
+    function PARENT() public pure override returns (Keycode) {
+        return toKeycode("PRICE");
+    }
+
+    /// @notice This implementation returns false for ISubmodule interface ID
+    function supportsInterface(bytes4 interfaceId) public pure override returns (bool) {
+        // Always return false for ISubmodule interface ID, making this contract fail validation
+        return interfaceId != type(ISubmodule).interfaceId;
+    }
+}
+/// forge-lint: disable-end(mixed-case-function)