From 5a2da490029e3a14b116d3ad488e1b9a928d2fa1 Mon Sep 17 00:00:00 2001 From: blockgroot <170620375+blockgroot@users.noreply.github.com> Date: Tue, 27 Aug 2024 12:23:44 +0000 Subject: [PATCH 1/4] feat: create new role to update pool weights --- contracts/PoolSelector.sol | 13 +++++++++++-- test/foundry_tests/PoolSelector.t.sol | 5 ++++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/contracts/PoolSelector.sol b/contracts/PoolSelector.sol index d2054637..25e01d59 100644 --- a/contracts/PoolSelector.sol +++ b/contracts/PoolSelector.sol @@ -22,6 +22,9 @@ contract PoolSelector is IPoolSelector, AccessControlUpgradeable { mapping(uint8 => uint256) public poolWeights; + bytes32 public constant POOL_WEIGHTS_OPERATOR = keccak256("POOL_WEIGHTS_OPERATOR"); + error CallerNotPoolWeightsOperator(); + /// @custom:oz-upgrades-unsafe-allow constructor constructor() { _disableInitializers(); @@ -108,12 +111,14 @@ contract PoolSelector is IPoolSelector, AccessControlUpgradeable { /** * @notice update the target weights of existing pools - * @dev only `Manager` can call, + * @dev only `POOL_WEIGHTS_OPERATOR's` can call, * @param _poolTargets new target weights of pools * `_poolTargets` array provide pool target in the same order of poolIDs that are stored in poolIdArray of poolUtils */ function updatePoolWeights(uint256[] calldata _poolTargets) external { - UtilLib.onlyManagerRole(msg.sender, staderConfig); + if (!onlyPoolWeightsOperatorRole(msg.sender)) { + revert CallerNotPoolWeightsOperator(); + } uint8[] memory poolIdArray = IPoolUtils(staderConfig.getPoolUtils()).getPoolIdArray(); uint256 poolCount = poolIdArray.length; uint256 poolTargetLength = _poolTargets.length; @@ -145,4 +150,8 @@ contract PoolSelector is IPoolSelector, AccessControlUpgradeable { staderConfig = IStaderConfig(_staderConfig); emit UpdatedStaderConfig(_staderConfig); } + + function onlyPoolWeightsOperatorRole(address account) private view returns (bool) { + return hasRole(POOL_WEIGHTS_OPERATOR, account); + } } diff --git a/test/foundry_tests/PoolSelector.t.sol b/test/foundry_tests/PoolSelector.t.sol index baf7ef39..537968b2 100644 --- a/test/foundry_tests/PoolSelector.t.sol +++ b/test/foundry_tests/PoolSelector.t.sol @@ -23,6 +23,8 @@ contract PoolSelectorTest is Test { PoolSelector poolSelector; PoolUtilsMockForDepositFlow poolUtils; + error CallerNotPoolWeightsOperator(); + function setUp() public { vm.clearMockedCalls(); staderAdmin = vm.addr(100); @@ -55,6 +57,7 @@ contract PoolSelectorTest is Test { poolUtils = new PoolUtilsMockForDepositFlow(address(0), address(staderConfig)); vm.startPrank(staderAdmin); + poolSelector.grantRole(poolSelector.POOL_WEIGHTS_OPERATOR(), staderManager); staderConfig.updatePoolUtils(address(poolUtils)); staderConfig.updateStakePoolManager(staderStakePoolManager); staderConfig.grantRole(staderConfig.MANAGER(), staderManager); @@ -95,7 +98,7 @@ contract PoolSelectorTest is Test { invalidSizePoolWeight[1] = 4000; invalidSizePoolWeight[2] = 4000; - vm.expectRevert(UtilLib.CallerNotManager.selector); + vm.expectRevert(CallerNotPoolWeightsOperator.selector); poolSelector.updatePoolWeights(poolWeight); vm.startPrank(staderManager); From 77907d82c38b5ef2c4cf2a3289c854ec1d995b66 Mon Sep 17 00:00:00 2001 From: blockgroot <170620375+blockgroot@users.noreply.github.com> Date: Fri, 13 Sep 2024 11:01:14 +0000 Subject: [PATCH 2/4] feat: generic check for configurator --- contracts/PoolSelector.sol | 13 ++----------- contracts/PoolUtils.sol | 2 +- contracts/StaderConfig.sol | 7 +++++++ contracts/interfaces/IStaderConfig.sol | 4 ++++ contracts/library/UtilLib.sol | 7 +++++++ test/foundry_tests/PoolSelector.t.sol | 12 ++++++------ test/foundry_tests/PoolUtils.t.sol | 7 +++++-- 7 files changed, 32 insertions(+), 20 deletions(-) diff --git a/contracts/PoolSelector.sol b/contracts/PoolSelector.sol index 25e01d59..6ada8aa4 100644 --- a/contracts/PoolSelector.sol +++ b/contracts/PoolSelector.sol @@ -22,9 +22,6 @@ contract PoolSelector is IPoolSelector, AccessControlUpgradeable { mapping(uint8 => uint256) public poolWeights; - bytes32 public constant POOL_WEIGHTS_OPERATOR = keccak256("POOL_WEIGHTS_OPERATOR"); - error CallerNotPoolWeightsOperator(); - /// @custom:oz-upgrades-unsafe-allow constructor constructor() { _disableInitializers(); @@ -111,14 +108,12 @@ contract PoolSelector is IPoolSelector, AccessControlUpgradeable { /** * @notice update the target weights of existing pools - * @dev only `POOL_WEIGHTS_OPERATOR's` can call, + * @dev only `CONFIGURATOR's` can call, * @param _poolTargets new target weights of pools * `_poolTargets` array provide pool target in the same order of poolIDs that are stored in poolIdArray of poolUtils */ function updatePoolWeights(uint256[] calldata _poolTargets) external { - if (!onlyPoolWeightsOperatorRole(msg.sender)) { - revert CallerNotPoolWeightsOperator(); - } + UtilLib.onlyConfiguratorRole(msg.sender, staderConfig); uint8[] memory poolIdArray = IPoolUtils(staderConfig.getPoolUtils()).getPoolIdArray(); uint256 poolCount = poolIdArray.length; uint256 poolTargetLength = _poolTargets.length; @@ -150,8 +145,4 @@ contract PoolSelector is IPoolSelector, AccessControlUpgradeable { staderConfig = IStaderConfig(_staderConfig); emit UpdatedStaderConfig(_staderConfig); } - - function onlyPoolWeightsOperatorRole(address account) private view returns (bool) { - return hasRole(POOL_WEIGHTS_OPERATOR, account); - } } diff --git a/contracts/PoolUtils.sol b/contracts/PoolUtils.sol index 38a928e4..2ad95cb0 100644 --- a/contracts/PoolUtils.sol +++ b/contracts/PoolUtils.sol @@ -70,7 +70,7 @@ contract PoolUtils is IPoolUtils, AccessControlUpgradeable { * @dev emit an event containing validator pubkey for offchain to exit the validator */ function processValidatorExitList(bytes[] calldata _pubkeys) external override { - UtilLib.onlyOperatorRole(msg.sender, staderConfig); + UtilLib.onlyConfiguratorRole(msg.sender, staderConfig); uint256 exitValidatorCount = _pubkeys.length; for (uint256 i; i < exitValidatorCount; ) { emit ExitValidator(_pubkeys[i]); diff --git a/contracts/StaderConfig.sol b/contracts/StaderConfig.sol index 24af1f35..6b6005dd 100644 --- a/contracts/StaderConfig.sol +++ b/contracts/StaderConfig.sol @@ -80,6 +80,9 @@ contract StaderConfig is IStaderConfig, AccessControlUpgradeable { bytes32 public constant override SD_UTILITY_POOL = keccak256("SD_UTILITY_POOL"); bytes32 public constant override SD_INCENTIVE_CONTROLLER = keccak256("SD_INCENTIVE_CONTROLLER"); + // Role define to manage pools config + bytes32 public constant override CONFIGURATOR = keccak256("CONFIGURATOR"); + /// @custom:oz-upgrades-unsafe-allow constructor constructor() { _disableInitializers(); @@ -537,6 +540,10 @@ contract StaderConfig is IStaderConfig, AccessControlUpgradeable { return hasRole(OPERATOR, account); } + function onlyConfiguratorRole(address account) external view override returns (bool) { + return hasRole(CONFIGURATOR, account); + } + function verifyDepositAndWithdrawLimits() internal view { if ( !(variablesMap[MIN_DEPOSIT_AMOUNT] != 0 && diff --git a/contracts/interfaces/IStaderConfig.sol b/contracts/interfaces/IStaderConfig.sol index f8f9854a..bd662d25 100644 --- a/contracts/interfaces/IStaderConfig.sol +++ b/contracts/interfaces/IStaderConfig.sol @@ -74,6 +74,8 @@ interface IStaderConfig { function OPERATOR() external view returns (bytes32); + function CONFIGURATOR() external view returns (bytes32); + // Constants function getStakedEthPerNode() external view returns (uint256); @@ -171,4 +173,6 @@ interface IStaderConfig { function onlyManagerRole(address account) external view returns (bool); function onlyOperatorRole(address account) external view returns (bool); + + function onlyConfiguratorRole(address account) external view returns (bool); } diff --git a/contracts/library/UtilLib.sol b/contracts/library/UtilLib.sol index c75fe06c..5ffb72b5 100644 --- a/contracts/library/UtilLib.sol +++ b/contracts/library/UtilLib.sol @@ -14,6 +14,7 @@ library UtilLib { error CallerNotStaderContract(); error CallerNotWithdrawVault(); error TransferFailed(); + error CallerNotConfigurator(); uint64 private constant VALIDATOR_PUBKEY_LENGTH = 48; @@ -35,6 +36,12 @@ library UtilLib { } } + function onlyConfiguratorRole(address _addr, IStaderConfig _staderConfig) internal view { + if (!_staderConfig.onlyConfiguratorRole(_addr)) { + revert CallerNotConfigurator(); + } + } + //checks if caller is a stader contract address function onlyStaderContract(address _addr, IStaderConfig _staderConfig, bytes32 _contractName) internal view { if (!_staderConfig.onlyStaderContract(_addr, _contractName)) { diff --git a/test/foundry_tests/PoolSelector.t.sol b/test/foundry_tests/PoolSelector.t.sol index 537968b2..63a8d3de 100644 --- a/test/foundry_tests/PoolSelector.t.sol +++ b/test/foundry_tests/PoolSelector.t.sol @@ -16,6 +16,7 @@ contract PoolSelectorTest is Test { address staderAdmin; address staderManager; address operator; + address configurator; address staderStakePoolManager; @@ -23,13 +24,12 @@ contract PoolSelectorTest is Test { PoolSelector poolSelector; PoolUtilsMockForDepositFlow poolUtils; - error CallerNotPoolWeightsOperator(); - function setUp() public { vm.clearMockedCalls(); staderAdmin = vm.addr(100); staderManager = vm.addr(101); operator = vm.addr(102); + configurator = vm.addr(116); staderStakePoolManager = vm.addr(110); address ethDepositAddr = vm.addr(103); @@ -57,11 +57,11 @@ contract PoolSelectorTest is Test { poolUtils = new PoolUtilsMockForDepositFlow(address(0), address(staderConfig)); vm.startPrank(staderAdmin); - poolSelector.grantRole(poolSelector.POOL_WEIGHTS_OPERATOR(), staderManager); staderConfig.updatePoolUtils(address(poolUtils)); staderConfig.updateStakePoolManager(staderStakePoolManager); staderConfig.grantRole(staderConfig.MANAGER(), staderManager); staderConfig.grantRole(staderConfig.OPERATOR(), operator); + staderConfig.grantRole(staderConfig.CONFIGURATOR(), configurator); vm.stopPrank(); } @@ -98,10 +98,10 @@ contract PoolSelectorTest is Test { invalidSizePoolWeight[1] = 4000; invalidSizePoolWeight[2] = 4000; - vm.expectRevert(CallerNotPoolWeightsOperator.selector); + vm.expectRevert(UtilLib.CallerNotConfigurator.selector); poolSelector.updatePoolWeights(poolWeight); - vm.startPrank(staderManager); + vm.startPrank(configurator); vm.expectRevert(IPoolSelector.InvalidNewTargetInput.selector); poolSelector.updatePoolWeights(invalidSizePoolWeight); @@ -115,7 +115,7 @@ contract PoolSelectorTest is Test { uint256[] memory poolWeight = new uint256[](2); poolWeight[0] = 7000; poolWeight[1] = 3000; - vm.prank(staderManager); + vm.prank(configurator); poolSelector.updatePoolWeights(poolWeight); vm.prank(operator); poolSelector.updatePoolAllocationMaxSize(1000); diff --git a/test/foundry_tests/PoolUtils.t.sol b/test/foundry_tests/PoolUtils.t.sol index e7f10ddc..9d4d9c42 100644 --- a/test/foundry_tests/PoolUtils.t.sol +++ b/test/foundry_tests/PoolUtils.t.sol @@ -17,6 +17,7 @@ contract PoolUtilsTest is Test { address staderAdmin; address staderManager; address operator; + address configurator; PoolUtils poolUtils; StaderConfig staderConfig; @@ -31,6 +32,7 @@ contract PoolUtilsTest is Test { staderAdmin = vm.addr(100); staderManager = vm.addr(101); operator = vm.addr(102); + configurator = vm.addr(114); address ethDepositAddr = vm.addr(103); nodeRegistry = new NodeRegistryMock(operator); @@ -59,6 +61,7 @@ contract PoolUtilsTest is Test { vm.startPrank(staderAdmin); staderConfig.grantRole(staderConfig.MANAGER(), staderManager); staderConfig.grantRole(staderConfig.OPERATOR(), operator); + staderConfig.grantRole(staderConfig.CONFIGURATOR(), configurator); vm.stopPrank(); } @@ -134,10 +137,10 @@ contract PoolUtilsTest is Test { pubkey[0] = "0x8faa339ba46c649885ea0fc9c34d32f9d99c5bde336750"; pubkey[1] = "0x8faa339ba46c649885ea0fc9c34d32f9d99c5bde336750"; - vm.expectRevert(UtilLib.CallerNotOperator.selector); + vm.expectRevert(UtilLib.CallerNotConfigurator.selector); poolUtils.processValidatorExitList(pubkey); - vm.prank(operator); + vm.prank(configurator); poolUtils.processValidatorExitList(pubkey); } From ac8e162ffcefc4693ec9ba37b60d0372ee48ade8 Mon Sep 17 00:00:00 2001 From: blockgroot <170620375+blockgroot@users.noreply.github.com> Date: Mon, 30 Sep 2024 12:28:05 +0000 Subject: [PATCH 3/4] chore: update permission mechanism --- contracts/PoolSelector.sol | 7 +++++-- contracts/PoolUtils.sol | 5 ++++- contracts/StaderConfig.sol | 29 +++++++++++++++++++++----- contracts/interfaces/IPoolSelector.sol | 1 + contracts/interfaces/IPoolUtils.sol | 1 + contracts/interfaces/IStaderConfig.sol | 14 ++++++++++--- contracts/library/UtilLib.sol | 7 ------- test/foundry_tests/PoolSelector.t.sol | 7 +++++-- test/foundry_tests/PoolUtils.t.sol | 7 +++++-- 9 files changed, 56 insertions(+), 22 deletions(-) diff --git a/contracts/PoolSelector.sol b/contracts/PoolSelector.sol index 6ada8aa4..ecbb4879 100644 --- a/contracts/PoolSelector.sol +++ b/contracts/PoolSelector.sol @@ -108,12 +108,15 @@ contract PoolSelector is IPoolSelector, AccessControlUpgradeable { /** * @notice update the target weights of existing pools - * @dev only `CONFIGURATOR's` can call, + * @dev only authorised callers can call, * @param _poolTargets new target weights of pools * `_poolTargets` array provide pool target in the same order of poolIDs that are stored in poolIdArray of poolUtils */ function updatePoolWeights(uint256[] calldata _poolTargets) external { - UtilLib.onlyConfiguratorRole(msg.sender, staderConfig); + if (!staderConfig.isAllowedToCall(msg.sender, "updatePoolWeights(uint256[])")) { + revert AccessDenied(msg.sender); + } + uint8[] memory poolIdArray = IPoolUtils(staderConfig.getPoolUtils()).getPoolIdArray(); uint256 poolCount = poolIdArray.length; uint256 poolTargetLength = _poolTargets.length; diff --git a/contracts/PoolUtils.sol b/contracts/PoolUtils.sol index 2ad95cb0..f5ede32d 100644 --- a/contracts/PoolUtils.sol +++ b/contracts/PoolUtils.sol @@ -70,7 +70,10 @@ contract PoolUtils is IPoolUtils, AccessControlUpgradeable { * @dev emit an event containing validator pubkey for offchain to exit the validator */ function processValidatorExitList(bytes[] calldata _pubkeys) external override { - UtilLib.onlyConfiguratorRole(msg.sender, staderConfig); + if (!staderConfig.isAllowedToCall(msg.sender, "processValidatorExitList(bytes[])")) { + revert AccessDenied(msg.sender); + } + uint256 exitValidatorCount = _pubkeys.length; for (uint256 i; i < exitValidatorCount; ) { emit ExitValidator(_pubkeys[i]); diff --git a/contracts/StaderConfig.sol b/contracts/StaderConfig.sol index 6b6005dd..eaa28149 100644 --- a/contracts/StaderConfig.sol +++ b/contracts/StaderConfig.sol @@ -80,9 +80,6 @@ contract StaderConfig is IStaderConfig, AccessControlUpgradeable { bytes32 public constant override SD_UTILITY_POOL = keccak256("SD_UTILITY_POOL"); bytes32 public constant override SD_INCENTIVE_CONTROLLER = keccak256("SD_INCENTIVE_CONTROLLER"); - // Role define to manage pools config - bytes32 public constant override CONFIGURATOR = keccak256("CONFIGURATOR"); - /// @custom:oz-upgrades-unsafe-allow constructor constructor() { _disableInitializers(); @@ -300,6 +297,27 @@ contract StaderConfig is IStaderConfig, AccessControlUpgradeable { setContract(SD_INCENTIVE_CONTROLLER, _sdIncentiveController); } + // Access Control + function giveCallPermission( + address contractAddress, + string calldata functionSig, + address accountToPermit + ) external override onlyRole(DEFAULT_ADMIN_ROLE) { + bytes32 role = keccak256(abi.encodePacked(contractAddress, functionSig)); + grantRole(role, accountToPermit); + emit PermissionGranted(accountToPermit, contractAddress, functionSig); + } + + function revokeCallPermission( + address contractAddress, + string calldata functionSig, + address accountToRevoke + ) external override onlyRole(DEFAULT_ADMIN_ROLE) { + bytes32 role = keccak256(abi.encodePacked(contractAddress, functionSig)); + revokeRole(role, accountToRevoke); + emit PermissionRevoked(accountToRevoke, contractAddress, functionSig); + } + //Constants Getters function getStakedEthPerNode() external view override returns (uint256) { @@ -540,8 +558,9 @@ contract StaderConfig is IStaderConfig, AccessControlUpgradeable { return hasRole(OPERATOR, account); } - function onlyConfiguratorRole(address account) external view override returns (bool) { - return hasRole(CONFIGURATOR, account); + function isAllowedToCall(address account, string calldata functionSig) external view override returns (bool) { + bytes32 role = keccak256(abi.encodePacked(msg.sender, functionSig)); + return hasRole(role, account); } function verifyDepositAndWithdrawLimits() internal view { diff --git a/contracts/interfaces/IPoolSelector.sol b/contracts/interfaces/IPoolSelector.sol index aea1b49e..ab93b860 100644 --- a/contracts/interfaces/IPoolSelector.sol +++ b/contracts/interfaces/IPoolSelector.sol @@ -6,6 +6,7 @@ interface IPoolSelector { error InvalidTargetWeight(); error InvalidNewTargetInput(); error InvalidSumOfPoolWeights(); + error AccessDenied(address account); // Events diff --git a/contracts/interfaces/IPoolUtils.sol b/contracts/interfaces/IPoolUtils.sol index 35d73bd1..99ff4fcd 100644 --- a/contracts/interfaces/IPoolUtils.sol +++ b/contracts/interfaces/IPoolUtils.sol @@ -14,6 +14,7 @@ interface IPoolUtils { error OperatorIsNotOnboarded(); error InvalidLengthOfSignature(); error ExistingOrMismatchingPoolId(); + error AccessDenied(address account); // Events event PoolAdded(uint8 indexed poolId, address poolAddress); diff --git a/contracts/interfaces/IStaderConfig.sol b/contracts/interfaces/IStaderConfig.sol index bd662d25..cf49f219 100644 --- a/contracts/interfaces/IStaderConfig.sol +++ b/contracts/interfaces/IStaderConfig.sol @@ -16,6 +16,8 @@ interface IStaderConfig { event SetAccount(bytes32 key, address newAddress); event SetContract(bytes32 key, address newAddress); event SetToken(bytes32 key, address newAddress); + event PermissionGranted(address indexed accountToPermit, address indexed contractAddress, string functionSig); + event PermissionRevoked(address indexed accountToRevoke, address indexed contractAddress, string functionSig); //Contracts function POOL_UTILS() external view returns (bytes32); @@ -74,8 +76,6 @@ interface IStaderConfig { function OPERATOR() external view returns (bytes32); - function CONFIGURATOR() external view returns (bytes32); - // Constants function getStakedEthPerNode() external view returns (uint256); @@ -174,5 +174,13 @@ interface IStaderConfig { function onlyOperatorRole(address account) external view returns (bool); - function onlyConfiguratorRole(address account) external view returns (bool); + function isAllowedToCall(address account, string calldata functionSig) external view returns (bool); + + function giveCallPermission(address contractAddress, string calldata functionSig, address accountToPermit) external; + + function revokeCallPermission( + address contractAddress, + string calldata functionSig, + address accountToRevoke + ) external; } diff --git a/contracts/library/UtilLib.sol b/contracts/library/UtilLib.sol index 5ffb72b5..c75fe06c 100644 --- a/contracts/library/UtilLib.sol +++ b/contracts/library/UtilLib.sol @@ -14,7 +14,6 @@ library UtilLib { error CallerNotStaderContract(); error CallerNotWithdrawVault(); error TransferFailed(); - error CallerNotConfigurator(); uint64 private constant VALIDATOR_PUBKEY_LENGTH = 48; @@ -36,12 +35,6 @@ library UtilLib { } } - function onlyConfiguratorRole(address _addr, IStaderConfig _staderConfig) internal view { - if (!_staderConfig.onlyConfiguratorRole(_addr)) { - revert CallerNotConfigurator(); - } - } - //checks if caller is a stader contract address function onlyStaderContract(address _addr, IStaderConfig _staderConfig, bytes32 _contractName) internal view { if (!_staderConfig.onlyStaderContract(_addr, _contractName)) { diff --git a/test/foundry_tests/PoolSelector.t.sol b/test/foundry_tests/PoolSelector.t.sol index 63a8d3de..e395e7e7 100644 --- a/test/foundry_tests/PoolSelector.t.sol +++ b/test/foundry_tests/PoolSelector.t.sol @@ -17,6 +17,7 @@ contract PoolSelectorTest is Test { address staderManager; address operator; address configurator; + address naiveAddress; address staderStakePoolManager; @@ -30,6 +31,7 @@ contract PoolSelectorTest is Test { staderManager = vm.addr(101); operator = vm.addr(102); configurator = vm.addr(116); + naiveAddress = vm.addr(117); staderStakePoolManager = vm.addr(110); address ethDepositAddr = vm.addr(103); @@ -61,7 +63,7 @@ contract PoolSelectorTest is Test { staderConfig.updateStakePoolManager(staderStakePoolManager); staderConfig.grantRole(staderConfig.MANAGER(), staderManager); staderConfig.grantRole(staderConfig.OPERATOR(), operator); - staderConfig.grantRole(staderConfig.CONFIGURATOR(), configurator); + staderConfig.giveCallPermission(address(poolSelector), "updatePoolWeights(uint256[])", configurator); vm.stopPrank(); } @@ -98,7 +100,8 @@ contract PoolSelectorTest is Test { invalidSizePoolWeight[1] = 4000; invalidSizePoolWeight[2] = 4000; - vm.expectRevert(UtilLib.CallerNotConfigurator.selector); + vm.prank(naiveAddress); + vm.expectRevert(abi.encodeWithSignature("AccessDenied(address)", naiveAddress)); poolSelector.updatePoolWeights(poolWeight); vm.startPrank(configurator); diff --git a/test/foundry_tests/PoolUtils.t.sol b/test/foundry_tests/PoolUtils.t.sol index 9d4d9c42..df59a1c8 100644 --- a/test/foundry_tests/PoolUtils.t.sol +++ b/test/foundry_tests/PoolUtils.t.sol @@ -18,6 +18,7 @@ contract PoolUtilsTest is Test { address staderManager; address operator; address configurator; + address naiveAddress; PoolUtils poolUtils; StaderConfig staderConfig; @@ -33,6 +34,7 @@ contract PoolUtilsTest is Test { staderManager = vm.addr(101); operator = vm.addr(102); configurator = vm.addr(114); + naiveAddress = vm.addr(117); address ethDepositAddr = vm.addr(103); nodeRegistry = new NodeRegistryMock(operator); @@ -61,7 +63,7 @@ contract PoolUtilsTest is Test { vm.startPrank(staderAdmin); staderConfig.grantRole(staderConfig.MANAGER(), staderManager); staderConfig.grantRole(staderConfig.OPERATOR(), operator); - staderConfig.grantRole(staderConfig.CONFIGURATOR(), configurator); + staderConfig.giveCallPermission(address(poolUtils), "processValidatorExitList(bytes[])", configurator); vm.stopPrank(); } @@ -137,7 +139,8 @@ contract PoolUtilsTest is Test { pubkey[0] = "0x8faa339ba46c649885ea0fc9c34d32f9d99c5bde336750"; pubkey[1] = "0x8faa339ba46c649885ea0fc9c34d32f9d99c5bde336750"; - vm.expectRevert(UtilLib.CallerNotConfigurator.selector); + vm.prank(naiveAddress); + vm.expectRevert(abi.encodeWithSignature("AccessDenied(address)", naiveAddress)); poolUtils.processValidatorExitList(pubkey); vm.prank(configurator); From 76100fabbab7e52c6bc5a3f270685787a6512f2d Mon Sep 17 00:00:00 2001 From: blockgroot <170620375+blockgroot@users.noreply.github.com> Date: Thu, 3 Oct 2024 15:16:54 +0000 Subject: [PATCH 4/4] =?UTF-8?q?fix:=20call=20internal=20function=20and=20a?= =?UTF-8?q?llow=20manager=20to=20grant=20and=20revoke=20per=C3=A2=C2=80mis?= =?UTF-8?q?sions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- contracts/StaderConfig.sol | 8 ++++---- test/foundry_tests/PoolSelector.t.sol | 4 +++- test/foundry_tests/PoolUtils.t.sol | 4 +++- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/contracts/StaderConfig.sol b/contracts/StaderConfig.sol index eaa28149..6bd19121 100644 --- a/contracts/StaderConfig.sol +++ b/contracts/StaderConfig.sol @@ -302,9 +302,9 @@ contract StaderConfig is IStaderConfig, AccessControlUpgradeable { address contractAddress, string calldata functionSig, address accountToPermit - ) external override onlyRole(DEFAULT_ADMIN_ROLE) { + ) external override onlyRole(MANAGER) { bytes32 role = keccak256(abi.encodePacked(contractAddress, functionSig)); - grantRole(role, accountToPermit); + _grantRole(role, accountToPermit); emit PermissionGranted(accountToPermit, contractAddress, functionSig); } @@ -312,9 +312,9 @@ contract StaderConfig is IStaderConfig, AccessControlUpgradeable { address contractAddress, string calldata functionSig, address accountToRevoke - ) external override onlyRole(DEFAULT_ADMIN_ROLE) { + ) external override onlyRole(MANAGER) { bytes32 role = keccak256(abi.encodePacked(contractAddress, functionSig)); - revokeRole(role, accountToRevoke); + _revokeRole(role, accountToRevoke); emit PermissionRevoked(accountToRevoke, contractAddress, functionSig); } diff --git a/test/foundry_tests/PoolSelector.t.sol b/test/foundry_tests/PoolSelector.t.sol index e395e7e7..101600e8 100644 --- a/test/foundry_tests/PoolSelector.t.sol +++ b/test/foundry_tests/PoolSelector.t.sol @@ -63,8 +63,10 @@ contract PoolSelectorTest is Test { staderConfig.updateStakePoolManager(staderStakePoolManager); staderConfig.grantRole(staderConfig.MANAGER(), staderManager); staderConfig.grantRole(staderConfig.OPERATOR(), operator); - staderConfig.giveCallPermission(address(poolSelector), "updatePoolWeights(uint256[])", configurator); vm.stopPrank(); + + vm.prank(staderManager); + staderConfig.giveCallPermission(address(poolSelector), "updatePoolWeights(uint256[])", configurator); } function test_JustToIncreaseCoverage() public { diff --git a/test/foundry_tests/PoolUtils.t.sol b/test/foundry_tests/PoolUtils.t.sol index df59a1c8..cf444381 100644 --- a/test/foundry_tests/PoolUtils.t.sol +++ b/test/foundry_tests/PoolUtils.t.sol @@ -63,8 +63,10 @@ contract PoolUtilsTest is Test { vm.startPrank(staderAdmin); staderConfig.grantRole(staderConfig.MANAGER(), staderManager); staderConfig.grantRole(staderConfig.OPERATOR(), operator); - staderConfig.giveCallPermission(address(poolUtils), "processValidatorExitList(bytes[])", configurator); vm.stopPrank(); + + vm.prank(staderManager); + staderConfig.giveCallPermission(address(poolUtils), "processValidatorExitList(bytes[])", configurator); } function test_JustToIncreaseCoverage() public {