test(contracts): add avs admin tests (#494)

Add avs admin tests. - move admin tests out of allowlist tests - remove some checks on admin params task: https://app.asana.com/0/1206208509925075/1206741146429449
omni-network · Mar 4, 2024 · 76ad9d7 · 76ad9d7
1 parent 5bd05ae
commit 76ad9d7
Show file tree

Hide file tree

Showing 5 changed files with 114 additions and 82 deletions.
diff --git a/contracts/.gas-snapshot b/contracts/.gas-snapshot
@@ -2,36 +2,40 @@ FeeOracleV1_Test:test_feeFor_succeeds() (gas: 38619)
 FeeOracleV1_Test:test_setFee_nonOwner_reverts() (gas: 20220)
 FeeOracleV1_Test:test_setFee_succeeds() (gas: 30824)
 FeeOracleV1_Test:test_setFee_zero_reverts() (gas: 20212)
-OmniAVS_admin_Test:test_avsDirectory_succeeds() (gas: 14980)
-OmniAVS_admin_Test:test_deregisterOperator_byOwner_succeeds() (gas: 334360)
-OmniAVS_admin_Test:test_deregisterOperator_whenPaused_reverts() (gas: 69783)
+OmniAVS_admin_Test:test_avsDirectory_succeeds() (gas: 14958)
+OmniAVS_admin_Test:test_deregisterOperator_byOwner_succeeds() (gas: 334463)
+OmniAVS_admin_Test:test_deregisterOperator_whenPaused_reverts() (gas: 69739)
 OmniAVS_admin_Test:test_getOPeratorRestakedStrategies_notOperator_succeeds() (gas: 42527)
-OmniAVS_admin_Test:test_getOperatorRestakedStrategies_noStrategies_succeeds() (gas: 308794)
-OmniAVS_admin_Test:test_getOperatorRestakedStrategies_succeeds() (gas: 369484)
-OmniAVS_admin_Test:test_getRestakeableStrategies_noStrategies_succeeds() (gas: 26237)
-OmniAVS_admin_Test:test_getRestakeableStrategies_succeeds() (gas: 84514)
-OmniAVS_admin_Test:test_pause_byOwner_succeeds() (gas: 44775)
-OmniAVS_admin_Test:test_registerOperator_whenPaused_reverts() (gas: 70972)
-OmniAVS_admin_Test:test_setMetadataURI_notOwner_reverts() (gas: 18630)
-OmniAVS_admin_Test:test_setMetadataURI_succeeds() (gas: 38488)
-OmniAVS_admin_Test:test_setStrategyParams_duplicateStrategy_reverts() (gas: 37780)
-OmniAVS_admin_Test:test_setStrategyParams_notOwner_reverts() (gas: 18312)
-OmniAVS_admin_Test:test_setStrategyParams_succeds() (gas: 70145)
-OmniAVS_admin_Test:test_setStrategyParams_twice_succeeds() (gas: 48824)
-OmniAVS_admin_Test:test_setStrategyParams_zeroAddress_reverts() (gas: 36856)
-OmniAVS_admin_Test:test_syncWithOmni_whenPaused_reverts() (gas: 45254)
-OmniAVS_admin_Test:test_unpause_byOwner_succeeds() (gas: 35560)
-OmniAVS_allowlist_Test:test_addToAllowlist_notOwner_reverts() (gas: 42465)
+OmniAVS_admin_Test:test_getOperatorRestakedStrategies_noStrategies_succeeds() (gas: 308750)
+OmniAVS_admin_Test:test_getOperatorRestakedStrategies_succeeds() (gas: 369440)
+OmniAVS_admin_Test:test_getRestakeableStrategies_noStrategies_succeeds() (gas: 26202)
+OmniAVS_admin_Test:test_getRestakeableStrategies_succeeds() (gas: 84470)
+OmniAVS_admin_Test:test_pause_byOwner_succeeds() (gas: 44820)
+OmniAVS_admin_Test:test_registerOperator_whenPaused_reverts() (gas: 70928)
+OmniAVS_admin_Test:test_setEthStakeInbox_notOwner_reverts() (gas: 17955)
+OmniAVS_admin_Test:test_setEthStakeInbox_succeeds() (gas: 26693)
+OmniAVS_admin_Test:test_setMetadataURI_notOwner_reverts() (gas: 18722)
+OmniAVS_admin_Test:test_setMetadataURI_succeeds() (gas: 38560)
+OmniAVS_admin_Test:test_setOmniChainId_notOwner_reverts() (gas: 17973)
+OmniAVS_admin_Test:test_setOmniChainId_succeeds() (gas: 26660)
+OmniAVS_admin_Test:test_setPortal_notOwner_reverts() (gas: 17912)
+OmniAVS_admin_Test:test_setPortal_succeeds() (gas: 26695)
+OmniAVS_admin_Test:test_setStrategyParams_duplicateStrategy_reverts() (gas: 37824)
+OmniAVS_admin_Test:test_setStrategyParams_notOwner_reverts() (gas: 18350)
+OmniAVS_admin_Test:test_setStrategyParams_succeds() (gas: 70124)
+OmniAVS_admin_Test:test_setStrategyParams_twice_succeeds() (gas: 48781)
+OmniAVS_admin_Test:test_setStrategyParams_zeroAddress_reverts() (gas: 36835)
+OmniAVS_admin_Test:test_setXCallGasLimits_notOwner_reverts() (gas: 18056)
+OmniAVS_admin_Test:test_setXCallGasLimits_succeeds() (gas: 31912)
+OmniAVS_admin_Test:test_syncWithOmni_whenPaused_reverts() (gas: 45232)
+OmniAVS_admin_Test:test_unpause_byOwner_succeeds() (gas: 35650)
+OmniAVS_allowlist_Test:test_addToAllowlist_notOwner_reverts() (gas: 42417)
 OmniAVS_allowlist_Test:test_addToAllowlist_succeeds() (gas: 69742)
-OmniAVS_allowlist_Test:test_registerOperator_notAllowed_reverts() (gas: 46150)
-OmniAVS_allowlist_Test:test_registerOperator_succeeds() (gas: 664961)
-OmniAVS_allowlist_Test:test_removeFromAllowlist_notOwner_reverts() (gas: 42401)
-OmniAVS_allowlist_Test:test_removeFromAllowlist_succeeds() (gas: 109388)
-OmniAVS_allowlist_Test:test_setEthStakeInbox_notOwner_reverts() (gas: 18010)
-OmniAVS_allowlist_Test:test_setEthStakeInbox_succeeds() (gas: 26728)
-OmniAVS_allowlist_Test:test_setEthStakeInbox_zeroAddress_reverts() (gas: 20449)
-OmniAVS_initialize_Test:test_initialize_defaultParams_succeeds() (gas: 2804599)
-OmniAVS_syncWithOmni_Test:test_depositBeaconEth_succeeds() (gas: 475174)
+OmniAVS_allowlist_Test:test_registerOperator_succeeds() (gas: 664896)
+OmniAVS_allowlist_Test:test_removeFromAllowlist_notOwner_reverts() (gas: 42342)
+OmniAVS_allowlist_Test:test_removeFromAllowlist_succeeds() (gas: 109355)
+OmniAVS_initialize_Test:test_initialize_defaultParams_succeeds() (gas: 2789031)
+OmniAVS_syncWithOmni_Test:test_depositBeaconEth_succeeds() (gas: 475197)
 OmniPortal_admin_Test:test_setFeeOracle_nonOwner_reverts() (gas: 20322)
 OmniPortal_admin_Test:test_setFeeOracle_succeeds() (gas: 31835)
 OmniPortal_admin_Test:test_setFeeOracle_zero_reverts() (gas: 20375)

diff --git a/contracts/src/interfaces/IOmniAVSAdmin.sol b/contracts/src/interfaces/IOmniAVSAdmin.sol
@@ -66,7 +66,7 @@ interface IOmniAVSAdmin {
      * @param base          The base xcall gas limit
      * @param perValidator  The per-validator additional xcall gas limit
      */
-    function setXcallGasLimits(uint64 base, uint64 perValidator) external;
+    function setXCallGasLimits(uint64 base, uint64 perValidator) external;
 
     /**
      * @notice Returns true if the operator is in the allowlist.

diff --git a/contracts/src/protocol/OmniAVS.sol b/contracts/src/protocol/OmniAVS.sol
@@ -233,7 +233,6 @@ contract OmniAVS is
      * @param inbox The EthStakeInbox contract address
      */
     function setEthStakeInbox(address inbox) external onlyOwner {
-        require(inbox != address(0), "OmniAVS: zero address");
         ethStakeInbox = inbox;
     }
 
@@ -250,7 +249,7 @@ contract OmniAVS is
      * @param base          The base xcall gas limit
      * @param perOperator   The per-operator additional xcall gas limit
      */
-    function setXcallGasLimits(uint64 base, uint64 perOperator) external onlyOwner {
+    function setXCallGasLimits(uint64 base, uint64 perOperator) external onlyOwner {
         xcallBaseGasLimit = base;
         xcallGasLimitPerOperator = perOperator;
     }
@@ -318,21 +317,6 @@ contract OmniAVS is
         }
     }
 
-    /**
-     * @notice Returns true if the operator is in the list of operators
-     */
-    function _isOperator(address operator) private view returns (bool) {
-        for (uint256 i = 0; i < _operators.length;) {
-            if (_operators[i] == operator) {
-                return true;
-            }
-            unchecked {
-                i++;
-            }
-        }
-        return false;
-    }
-
     /**
      * @notice Set the strategy parameters.
      * @param params The strategy parameters
@@ -369,6 +353,21 @@ contract OmniAVS is
         return uint64(numOperators) * xcallGasLimitPerOperator + xcallBaseGasLimit;
     }
 
+    /**
+     * @notice Returns true if the operator is in the list of operators
+     */
+    function _isOperator(address operator) private view returns (bool) {
+        for (uint256 i = 0; i < _operators.length;) {
+            if (_operators[i] == operator) {
+                return true;
+            }
+            unchecked {
+                i++;
+            }
+        }
+        return false;
+    }
+
     /**
      * @notice Return current list of Operators, including their personal stake and delegated stake
      */

diff --git a/contracts/test/avs/OmniAVS_admin.t.sol b/contracts/test/avs/OmniAVS_admin.t.sol
@@ -3,6 +3,8 @@ pragma solidity =0.8.12;
 
 import { IStrategy } from "eigenlayer-contracts/src/contracts/interfaces/IStrategy.sol";
 
+import { OmniPortalConstants } from "src/protocol/OmniPortalConstants.sol";
+import { IOmniPortal } from "src/interfaces/IOmniPortal.sol";
 import { IOmniAVS } from "src/interfaces/IOmniAVS.sol";
 import { Base } from "./common/Base.sol";
 
@@ -125,8 +127,70 @@ contract OmniAVS_admin_Test is Base {
     /// @dev Test that only the owner can set the metadata URI
     function test_setMetadataURI_notOwner_reverts() public {
         string memory uri = "https://example.com/avs/";
-
         vm.expectRevert("Ownable: caller is not the owner");
         omniAVS.setMetadataURI(uri);
     }
+
+    /// @dev Test that the owner can set omniChainId
+    function test_setOmniChainId_succeeds() public {
+        uint64 chainId = 1;
+        vm.prank(omniAVSOwner);
+        omniAVS.setOmniChainId(chainId);
+        assertEq(omniAVS.omniChainId(), chainId);
+    }
+
+    /// @dev Test that only the owner can set omniChainId
+    function test_setOmniChainId_notOwner_reverts() public {
+        uint64 chainId = 1;
+        vm.expectRevert("Ownable: caller is not the owner");
+        omniAVS.setOmniChainId(chainId);
+    }
+
+    /// @dev Test that the owner can set ethStakeInbox
+    function test_setEthStakeInbox_succeeds() public {
+        address ethStakeInbox = address(1);
+        vm.prank(omniAVSOwner);
+        omniAVS.setEthStakeInbox(ethStakeInbox);
+        assertEq(omniAVS.ethStakeInbox(), ethStakeInbox);
+    }
+
+    /// @dev Test that only the owner can set ethStakeInbox
+    function test_setEthStakeInbox_notOwner_reverts() public {
+        address ethStakeInbox = address(1);
+        vm.expectRevert("Ownable: caller is not the owner");
+        omniAVS.setEthStakeInbox(ethStakeInbox);
+    }
+
+    /// @dev Test thath the owner can set the portal address
+    function test_setPortal_succeeds() public {
+        address portal = address(1);
+        vm.prank(omniAVSOwner);
+        omniAVS.setOmniPortal(IOmniPortal(portal));
+        assertEq(address(omniAVS.omni()), portal);
+    }
+
+    /// @dev Test that only the owner can set the portal address
+    function test_setPortal_notOwner_reverts() public {
+        address portal = address(1);
+        vm.expectRevert("Ownable: caller is not the owner");
+        omniAVS.setOmniPortal(IOmniPortal(portal));
+    }
+
+    /// @dev Test that the owner can set the xcall gas limit params
+    function test_setXCallGasLimits_succeeds() public {
+        uint64 base = omniAVS.xcallBaseGasLimit() + 10_000;
+        uint64 perOperator = omniAVS.xcallGasLimitPerOperator() + 20_000;
+
+        vm.prank(omniAVSOwner);
+        omniAVS.setXCallGasLimits(base, perOperator);
+
+        assertEq(omniAVS.xcallBaseGasLimit(), base);
+        assertEq(omniAVS.xcallGasLimitPerOperator(), perOperator);
+    }
+
+    /// @dev Test that only the owner can set the xcall gas limit params
+    function test_setXCallGasLimits_notOwner_reverts() public {
+        vm.expectRevert("Ownable: caller is not the owner");
+        omniAVS.setXCallGasLimits(0, 0);
+    }
 }
diff --git a/contracts/test/avs/OmniAVS_allowlist.t.sol b/contracts/test/avs/OmniAVS_allowlist.t.sol
@@ -62,39 +62,4 @@ contract OmniAVS_allowlist_Test is Base {
         assertEq(operators.length, 1);
         assertEq(operators[0].addr, operator);
     }
-
-    /// @dev Test that an operator cannot register if not in allow list
-    function test_registerOperator_notAllowed_reverts() public {
-        address operator = _operator(0);
-
-        ISignatureUtils.SignatureWithSaltAndExpiry memory emptySig;
-
-        vm.expectRevert("OmniAVS: not allowed");
-        vm.prank(operator);
-        omniAVS.registerOperatorToAVS(operator, emptySig);
-    }
-
-    /// @dev Test that the owner can set the ethStakeInbox
-    function test_setEthStakeInbox_succeeds() public {
-        address newInbox = 0x1234567890123456789012345678901234567890;
-
-        vm.prank(omniAVSOwner);
-        omniAVS.setEthStakeInbox(newInbox);
-        assertEq(omniAVS.ethStakeInbox(), newInbox);
-    }
-
-    /// @dev Test that only the owner can set the ethStakeInbox
-    function test_setEthStakeInbox_notOwner_reverts() public {
-        address newInbox = 0x1234567890123456789012345678901234567890;
-
-        vm.expectRevert("Ownable: caller is not the owner");
-        omniAVS.setEthStakeInbox(newInbox);
-    }
-
-    /// @dev Test the the ethStakeInbox cannot be set to the zero address
-    function test_setEthStakeInbox_zeroAddress_reverts() public {
-        vm.prank(omniAVSOwner);
-        vm.expectRevert("OmniAVS: zero address");
-        omniAVS.setEthStakeInbox(address(0));
-    }
 }