Merge pull request #87 from yieldnest/test/role-updates-review

Review of test/role-updates: Updates to Oracle and RewardsDistro, test updates
yieldnest · Apr 12, 2024 · 12cdf0f · 12cdf0f
2 parents 45777d7 + d99c60c
commit 12cdf0f
Show file tree

Hide file tree

Showing 6 changed files with 123 additions and 46 deletions.
diff --git a/script/Actors.sol b/script/Actors.sol
@@ -8,6 +8,7 @@ contract ActorAddresses {
         address ADMIN;
         address STAKING_ADMIN;
         address STAKING_NODES_ADMIN;
+        address REWARDS_ADMIN;
         address VALIDATOR_MANAGER;
         address FEE_RECEIVER;
         address PAUSE_ADMIN;
@@ -25,18 +26,19 @@ contract ActorAddresses {
             DEFAULT_SIGNER: 0x72fdBD51085bDa5eEEd3b55D1a46E2e92f0837a5,
             DEPOSIT_BOOTSTRAPER: 0x72fdBD51085bDa5eEEd3b55D1a46E2e92f0837a5,
             // protocol fee receiver
-            FEE_RECEIVER: 0xa9295EA69DaF30A8CB81610c23966240BdDA2876,
+            FEE_RECEIVER: 0x743b91CDB1C694D4F51bCDA3a4A59DcC0d02b913,
             // admin multisig roles
-            ADMIN: 0xa9295EA69DaF30A8CB81610c23966240BdDA2876,
-            STAKING_ADMIN: 0xa9295EA69DaF30A8CB81610c23966240BdDA2876,
-            PROXY_ADMIN_OWNER: 0xa9295EA69DaF30A8CB81610c23966240BdDA2876,
-            STAKING_NODES_ADMIN: 0xa9295EA69DaF30A8CB81610c23966240BdDA2876,
-            PAUSE_ADMIN: 0xa9295EA69DaF30A8CB81610c23966240BdDA2876,
+            ADMIN: 0x743b91CDB1C694D4F51bCDA3a4A59DcC0d02b913,
+            STAKING_ADMIN: 0x743b91CDB1C694D4F51bCDA3a4A59DcC0d02b913,
+            PROXY_ADMIN_OWNER: 0x743b91CDB1C694D4F51bCDA3a4A59DcC0d02b913,
+            STAKING_NODES_ADMIN: 0x743b91CDB1C694D4F51bCDA3a4A59DcC0d02b913,
+            PAUSE_ADMIN: 0x743b91CDB1C694D4F51bCDA3a4A59DcC0d02b913,
+            REWARDS_ADMIN: 0x743b91CDB1C694D4F51bCDA3a4A59DcC0d02b913,
             // operational multisig roles
-            VALIDATOR_MANAGER: 0xa9295EA69DaF30A8CB81610c23966240BdDA2876,
-            LSD_RESTAKING_MANAGER: 0xa9295EA69DaF30A8CB81610c23966240BdDA2876,
-            STAKING_NODE_CREATOR: 0xa9295EA69DaF30A8CB81610c23966240BdDA2876,
-            ORACLE_MANAGER: 0xa9295EA69DaF30A8CB81610c23966240BdDA2876
+            VALIDATOR_MANAGER: 0x9Dd8F69b62ddFd990241530F47dcEd0Dad7f7d39,
+            LSD_RESTAKING_MANAGER: 0x9Dd8F69b62ddFd990241530F47dcEd0Dad7f7d39,
+            STAKING_NODE_CREATOR: 0x9Dd8F69b62ddFd990241530F47dcEd0Dad7f7d39,
+            ORACLE_MANAGER: 0x9Dd8F69b62ddFd990241530F47dcEd0Dad7f7d39
         });
 
         actors[1] = Actors({
@@ -51,7 +53,8 @@ contract ActorAddresses {
             LSD_RESTAKING_MANAGER: 0xa0Ee7A142d267C1f36714E4a8F75612F20a79720,
             STAKING_NODE_CREATOR: 0xBcd4042DE499D14e55001CcbB24a551F3b954096,
             ORACLE_MANAGER: 0x71bE63f3384f5fb98995898A86B02Fb2426c5788,
-            DEPOSIT_BOOTSTRAPER: 0xFABB0ac9d68B0B445fB7357272Ff202C5651694a
+            DEPOSIT_BOOTSTRAPER: 0xFABB0ac9d68B0B445fB7357272Ff202C5651694a,
+            REWARDS_ADMIN: 0x743b91CDB1C694D4F51bCDA3a4A59DcC0d02b913
         });
     }
 

diff --git a/script/DeployYieldNest.s.sol b/script/DeployYieldNest.s.sol
@@ -147,6 +147,7 @@ contract DeployYieldNest is BaseScript {
 
         RewardsDistributor.Init memory rewardsDistributorInit = RewardsDistributor.Init({
             admin: actors.ADMIN,
+            rewardsAdmin: actors.REWARDS_ADMIN,
             executionLayerReceiver: executionLayerReceiver,
             consensusLayerReceiver: consensusLayerReceiver, // Adding consensusLayerReceiver to the initialization
             feesReceiver: payable(actors.FEE_RECEIVER), // should be the FEE_RECEIVER role

diff --git a/src/RewardsDistributor.sol b/src/RewardsDistributor.sol
@@ -2,7 +2,6 @@
 pragma solidity ^0.8.24;
 
 import {Math} from "lib/openzeppelin-contracts/contracts/utils/math/Math.sol";
-import {Initializable} from "lib/openzeppelin-contracts-upgradeable/contracts/proxy/utils/Initializable.sol";
 import {AccessControlUpgradeable} from "lib/openzeppelin-contracts-upgradeable/contracts/access/AccessControlUpgradeable.sol";
 import {RewardsReceiver} from "src/RewardsReceiver.sol";
 import {IynETH} from "src/interfaces/IynETH.sol";
@@ -15,7 +14,7 @@ interface RewardsDistributorEvents {
 }
 
 
-contract RewardsDistributor is Initializable, AccessControlUpgradeable, RewardsDistributorEvents {
+contract RewardsDistributor is AccessControlUpgradeable, RewardsDistributorEvents {
 
     //--------------------------------------------------------------------------------------
     //----------------------------------  ERRORS  ------------------------------------------
@@ -49,6 +48,12 @@ contract RewardsDistributor is Initializable, AccessControlUpgradeable, RewardsD
     /// @notice The protocol fees in basis points (1/10000).
     uint16 public feesBasisPoints;
 
+    //--------------------------------------------------------------------------------------
+    //----------------------------------  ROLES  -------------------------------------------
+    //--------------------------------------------------------------------------------------
+
+    bytes32 public constant REWARDS_ADMIN_ROLE = keccak256("REWARDS_ADMIN_ROLE");    
+
     //--------------------------------------------------------------------------------------
     //----------------------------------  INITIALIZATION  ----------------------------------
     //--------------------------------------------------------------------------------------
@@ -60,6 +65,7 @@ contract RewardsDistributor is Initializable, AccessControlUpgradeable, RewardsD
     /// @notice Configuration for contract initialization.
     struct Init {
         address admin;
+        address rewardsAdmin;
         RewardsReceiver executionLayerReceiver;
         RewardsReceiver consensusLayerReceiver;
         address payable feesReceiver;
@@ -77,6 +83,7 @@ contract RewardsDistributor is Initializable, AccessControlUpgradeable, RewardsD
         __AccessControl_init();
 
         _grantRole(DEFAULT_ADMIN_ROLE, init.admin);
+        _grantRole(REWARDS_ADMIN_ROLE, init.rewardsAdmin);
         executionLayerReceiver = init.executionLayerReceiver;
         consensusLayerReceiver = init.consensusLayerReceiver;
         feesReceiver = init.feesReceiver;
@@ -140,7 +147,7 @@ contract RewardsDistributor is Initializable, AccessControlUpgradeable, RewardsD
     /// @param newReceiver The new fees receiver wallet.
     function setFeesReceiver(address payable newReceiver)
         external
-        onlyRole(DEFAULT_ADMIN_ROLE)
+        onlyRole(REWARDS_ADMIN_ROLE)
         notZeroAddress(newReceiver)
     {
         feesReceiver = newReceiver;
@@ -151,7 +158,7 @@ contract RewardsDistributor is Initializable, AccessControlUpgradeable, RewardsD
     /// @param newFeesBasisPoints The new fees basis points.
     function setFeesBasisPoints(uint16 newFeesBasisPoints)
         external
-        onlyRole(DEFAULT_ADMIN_ROLE)
+        onlyRole(REWARDS_ADMIN_ROLE)
     {
         if (newFeesBasisPoints > _BASIS_POINTS_DENOMINATOR) revert InvalidBasisPoints();
         feesBasisPoints = newFeesBasisPoints;

diff --git a/src/YieldNestOracle.sol b/src/YieldNestOracle.sol
@@ -9,20 +9,39 @@ interface IYieldNestOracleEvents {
 }
 
 contract YieldNestOracle is AccessControlUpgradeable, IYieldNestOracleEvents {
-    struct AssetPriceFeed {
-        AggregatorV3Interface priceFeed;
-        uint256 maxAge; // in seconds
-    }
 
+    //--------------------------------------------------------------------------------------
+    //----------------------------------  ERRORS  -------------------------------------------
+    //--------------------------------------------------------------------------------------
+
     error PriceFeedTooStale(uint256 age, uint256 maxAge);
     error ZeroAddress();
     error ZeroAge();
     error ArraysLengthMismatch(uint256 assetsLength, uint256 priceFeedAddressesLength, uint256 maxAgesLength);
     error PriceFeedNotSet();
-    error InvalidPriceValue(int256 price);
+    error InvalidPriceValue(int256 price); 
+
+    //--------------------------------------------------------------------------------------
+    //----------------------------------  VARIABLES  ---------------------------------------
+    //--------------------------------------------------------------------------------------
+
+    struct AssetPriceFeed {
+        AggregatorV3Interface priceFeed;
+        uint256 maxAge; // in seconds
+    }
 
     mapping(address => AssetPriceFeed) public assetPriceFeeds;
 
+    //--------------------------------------------------------------------------------------
+    //----------------------------------  ROLES  -------------------------------------------
+    //--------------------------------------------------------------------------------------
+
+    bytes32 public constant ORACLE_MANAGER_ROLE = keccak256("ORACLE_MANAGER_ROLE");
+
+    //--------------------------------------------------------------------------------------
+    //----------------------------------  INITIALIZATION  ----------------------------------
+    //--------------------------------------------------------------------------------------
+
     struct Init {
         address[] assets;
         address[] priceFeedAddresses;
@@ -31,9 +50,6 @@ contract YieldNestOracle is AccessControlUpgradeable, IYieldNestOracleEvents {
         address oracleManager;
     }
 
-    bytes32 public constant ADMIN_ROLE = keccak256("ADMIN_ROLE");
-    bytes32 public constant ORACLE_MANAGER_ROLE = keccak256("ORACLE_MANAGER_ROLE");
-
     constructor() {
        _disableInitializers();
     }
@@ -44,7 +60,7 @@ contract YieldNestOracle is AccessControlUpgradeable, IYieldNestOracleEvents {
         notZeroAddress(init.oracleManager)
         initializer {
          __AccessControl_init();
-        _grantRole(ADMIN_ROLE, init.admin);
+        _grantRole(DEFAULT_ADMIN_ROLE, init.admin);
         _grantRole(ORACLE_MANAGER_ROLE, init.oracleManager);
 
         if (init.assets.length != init.priceFeedAddresses.length || init.assets.length != init.maxAges.length) {
@@ -55,6 +71,9 @@ contract YieldNestOracle is AccessControlUpgradeable, IYieldNestOracleEvents {
         }
     }
 
+    //--------------------------------------------------------------------------------------
+    //----------------------------------  FUNCTIONS  ---------------------------------------
+    //--------------------------------------------------------------------------------------
 
     /**
      * @notice Sets the price feed for a given asset.

diff --git a/test/integration/IntegrationBaseTest.sol b/test/integration/IntegrationBaseTest.sol
@@ -191,6 +191,7 @@ contract IntegrationBaseTest is Test, Utils {
 
         RewardsDistributor.Init memory rewardsDistributorInit = RewardsDistributor.Init({
             admin: actors.ADMIN,
+            rewardsAdmin: actors.REWARDS_ADMIN,
             executionLayerReceiver: executionLayerReceiver,
             consensusLayerReceiver: consensusLayerReceiver,
             feesReceiver: payable(actors.FEE_RECEIVER),
@@ -222,7 +223,7 @@ contract IntegrationBaseTest is Test, Utils {
         vm.prank(actors.STAKING_ADMIN); // StakingNodesManager is the only contract that can register a staking node implementation contract
         stakingNodesManager.registerStakingNodeImplementationContract(address(stakingNodeImplementation));
     }
-
+    
     function setupYieldNestOracleAndYnLSD() public {
         IERC20[] memory assets = new IERC20[](2);
         address[] memory assetsAddresses = new address[](2);

diff --git a/test/integration/Roles.t.sol b/test/integration/Roles.t.sol
@@ -1,30 +1,15 @@
 // SPDX-License-Identifier: BSD 3-Clause License
 pragma solidity ^0.8.24;
-import {IntegrationBaseTest} from "./IntegrationBaseTest.sol";
-import {StakingNodesManager} from "src/StakingNodesManager.sol";
+import {IntegrationBaseTest} from "test/integration/IntegrationBaseTest.sol";
 import {ynETH} from "src/ynETH.sol";
-import {ynLSD} from "src/ynLSD.sol";
-import {YieldNestOracle} from "src/YieldNestOracle.sol";
-import {MockYnETHERC4626} from "test/mocks/MockYnETHERC4626.sol";
-import {MockERC20} from "test/mocks/MockERC20.sol";
 import {RewardsDistributor} from "src/RewardsDistributor.sol";
-import {ProxyAdmin} from "lib/openzeppelin-contracts/contracts/proxy/transparent/ProxyAdmin.sol";
-import {IRewardsDistributor} from "src/interfaces/IRewardsDistributor.sol";
-import {IStakingNodesManager} from "src/interfaces/IStakingNodesManager.sol";
-import {IStrategy} from "src/external/eigenlayer/v0.1.0/interfaces/IStrategy.sol";
-import {TransparentUpgradeableProxy} from "lib/openzeppelin-contracts/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
-import {ITransparentUpgradeableProxy} from "lib/openzeppelin-contracts/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
-import {IERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/IERC20.sol";
-import {TestStakingNodesManagerV2} from "test/mocks/TestStakingNodesManagerV2.sol";
+import {IAccessControl} from "lib/openzeppelin-contracts/contracts/access/IAccessControl.sol";
 
 contract RolesTest is IntegrationBaseTest {
 
     function testRoleChangeYnETH() public {
         address newOperator = address(0x123);
-        bytes32 ADMIN_ROLE = keccak256("ADMIN_ROLE");
         bytes32 PAUSER_ROLE = keccak256("PAUSER_ROLE");
-        bytes32 STAKING_NODES_MANAGER_ROLE = keccak256("STAKING_NODES_MANAGER_ROLE");
-        bytes32 REWARDS_DISTRIBUTOR_ROLE = keccak256("REWARDS_DISTRIBUTOR_ROLE");
 
         assertTrue(stakingNodesManager.hasRole(stakingNodesManager.DEFAULT_ADMIN_ROLE(), actors.ADMIN));
         assertTrue(stakingNodesManager.hasRole(PAUSER_ROLE, actors.PAUSE_ADMIN));
@@ -83,9 +68,70 @@ contract RolesTest is IntegrationBaseTest {
     }
 
     function testRoleChangeRewardsDistributor() public {
-        // TODO: add after fixing roles in RewardsDistributor.sol
+        address newRewardsAdmin = address(0x789);
+        bytes32 REWARDS_ADMIN_ROLE = keccak256("REWARDS_ADMIN_ROLE");
+
+        assertTrue(rewardsDistributor.hasRole(rewardsDistributor.DEFAULT_ADMIN_ROLE(), actors.ADMIN));
+        assertTrue(rewardsDistributor.hasRole(REWARDS_ADMIN_ROLE, actors.REWARDS_ADMIN));
+
+        vm.startPrank(actors.ADMIN);
+        rewardsDistributor.grantRole(REWARDS_ADMIN_ROLE, newRewardsAdmin);
+        rewardsDistributor.revokeRole(REWARDS_ADMIN_ROLE, actors.REWARDS_ADMIN);
+        vm.stopPrank();
+
+        assertTrue(rewardsDistributor.hasRole(REWARDS_ADMIN_ROLE, newRewardsAdmin));
+        assertFalse(rewardsDistributor.hasRole(REWARDS_ADMIN_ROLE, actors.REWARDS_ADMIN));
+    }
+
+    function testRewardsDistributorFeeAdminRoles() public {
+        address payable newFeeReceiver = payable(address(0x789));
+        bytes32 REWARDS_ADMIN_ROLE = keccak256("REWARDS_ADMIN_ROLE");
+
+        vm.prank(actors.REWARDS_ADMIN);
+        rewardsDistributor.setFeesReceiver(newFeeReceiver);
+
+        vm.expectRevert(abi.encodeWithSelector(IAccessControl.AccessControlUnauthorizedAccount.selector, address(this), REWARDS_ADMIN_ROLE));
+        rewardsDistributor.setFeesReceiver(newFeeReceiver);
+
+        vm.prank(actors.REWARDS_ADMIN);
+        rewardsDistributor.setFeesBasisPoints(1000);
+
+        vm.expectRevert(abi.encodeWithSelector(IAccessControl.AccessControlUnauthorizedAccount.selector, address(this), REWARDS_ADMIN_ROLE));
+        rewardsDistributor.setFeesBasisPoints(1000);
+    }
+
+    function testConsensusLayerRewardsReceiverRoles() public {
+        address newRewardsDistributor = address(0x789);
+        bytes32 WITHDRAWER_ROLE = keccak256("WITHDRAWER_ROLE");
+
+        assertTrue(consensusLayerReceiver.hasRole(consensusLayerReceiver.DEFAULT_ADMIN_ROLE(), actors.ADMIN));
+        assertTrue(consensusLayerReceiver.hasRole(WITHDRAWER_ROLE, address(rewardsDistributor)));
+
+        vm.startPrank(actors.ADMIN);
+        consensusLayerReceiver.grantRole(WITHDRAWER_ROLE, newRewardsDistributor);
+        consensusLayerReceiver.revokeRole(WITHDRAWER_ROLE, address(rewardsDistributor));
+        vm.stopPrank();
+
+        assertTrue(consensusLayerReceiver.hasRole(WITHDRAWER_ROLE, newRewardsDistributor));
+        assertFalse(consensusLayerReceiver.hasRole(WITHDRAWER_ROLE, address(rewardsDistributor)));
     }
 
+    function testExecutionLayerRewardsReceiverRoles() public {
+        address newRewardsDistributor = address(0x789);
+        bytes32 WITHDRAWER_ROLE = keccak256("WITHDRAWER_ROLE");
+
+        assertTrue(executionLayerReceiver.hasRole(executionLayerReceiver.DEFAULT_ADMIN_ROLE(), actors.ADMIN));
+        assertTrue(executionLayerReceiver.hasRole(WITHDRAWER_ROLE, address(rewardsDistributor)));
+
+        vm.startPrank(actors.ADMIN);
+        executionLayerReceiver.grantRole(WITHDRAWER_ROLE, newRewardsDistributor);
+        executionLayerReceiver.revokeRole(WITHDRAWER_ROLE, address(rewardsDistributor));
+        vm.stopPrank();
+
+        assertTrue(executionLayerReceiver.hasRole(WITHDRAWER_ROLE, newRewardsDistributor));
+        assertFalse(executionLayerReceiver.hasRole(WITHDRAWER_ROLE, address(rewardsDistributor)));
+    }    
+
     function testRoleChangeYnLSD() public {
         address newStakingAdmin = address(0xABC);
         bytes32 STAKING_ADMIN_ROLE = keccak256("STAKING_ADMIN_ROLE");
@@ -115,18 +161,18 @@ contract RolesTest is IntegrationBaseTest {
     }
 
     function testRoleChangeYieldNestOracle() public {
-        address newOracleAdmin = address(0xDEF);
+        address newOracleManager = address(0xDEF);
         bytes32 ORACLE_MANAGER_ROLE = keccak256("ORACLE_MANAGER_ROLE");
 
         assertTrue(yieldNestOracle.hasRole(yieldNestOracle.DEFAULT_ADMIN_ROLE(), actors.ADMIN));
         assertTrue(yieldNestOracle.hasRole(ORACLE_MANAGER_ROLE, actors.ORACLE_MANAGER));
 
         vm.startPrank(actors.ADMIN);
-        yieldNestOracle.grantRole(ORACLE_MANAGER_ROLE, newOracleAdmin);
+        yieldNestOracle.grantRole(ORACLE_MANAGER_ROLE, newOracleManager);
         yieldNestOracle.revokeRole(ORACLE_MANAGER_ROLE, actors.ORACLE_MANAGER);
         vm.stopPrank();
 
-        assertTrue(yieldNestOracle.hasRole(ORACLE_MANAGER_ROLE, newOracleAdmin));
+        assertTrue(yieldNestOracle.hasRole(ORACLE_MANAGER_ROLE, newOracleManager));
         assertFalse(yieldNestOracle.hasRole(ORACLE_MANAGER_ROLE, actors.ORACLE_MANAGER));
     }
 }