Skip to content

Commit

Permalink
feat: Remove contract manager roles (#372)
Browse files Browse the repository at this point in the history
  • Loading branch information
dgusakov authored Dec 9, 2024
1 parent 82dd433 commit 7ec80dd
Show file tree
Hide file tree
Showing 20 changed files with 31 additions and 80 deletions.
4 changes: 2 additions & 2 deletions script/DeployMainnet.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ contract DeployMainnet is DeployBase {
config.oracleMembers[4] = 0x007DE4a5F7bc37E2F26c0cb2E8A95006EE9B89b5; // P2P
config.oracleMembers[5] = 0xc79F702202E3A6B0B6310B537E786B9ACAA19BAf; // Chainlayer
config.oracleMembers[6] = 0x61c91ECd902EB56e314bB2D5c5C07785444Ea1c8; // bloXroute
config.oracleMembers[7] = 0x1Ca0fEC59b86F549e1F1184d97cb47794C8Af58d; // Instadapp
config.oracleMembers[8] = 0xe57B3792aDCc5da47EF4fF588883F0ee0c9835C9; // MatrixedLink
config.oracleMembers[7] = 0xe57B3792aDCc5da47EF4fF588883F0ee0c9835C9; // MatrixedLink
config.oracleMembers[8] = 0x73181107c8D9ED4ce0bbeF7A0b4ccf3320C41d12; // Instadapp
config.hashConsensusQuorum = 5;
// Verifier
// NOTE: Deneb fork gIndexes. Should be updated according to `config.verifierSupportedEpoch` fork epoch if needed
Expand Down
1 change: 0 additions & 1 deletion script/fork-helpers/PauseResume.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ contract PauseResume is Script, DeploymentFixtures, ForkHelpersCommon {
}

function publicRelease() external broadcastCSMAdmin {
csm.grantRole(csm.MODULE_MANAGER_ROLE(), csmAdmin);
csm.activatePublicRelease();

assertTrue(csm.publicRelease());
Expand Down
14 changes: 6 additions & 8 deletions script/fork-helpers/SimulateVote.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,8 @@ contract SimulateVote is Script, DeploymentFixtures, ForkHelpersCommon {
vm.stopBroadcast();

vm.startBroadcast(agent);
// 1. Grant manager role
csm.grantRole(csm.MODULE_MANAGER_ROLE(), agent);

// 2. Add CommunityStaking module
// 1. Add CommunityStaking module
stakingRouter.addStakingModule({
_name: "community-staking-v1",
_stakingModuleAddress: address(csm),
Expand All @@ -51,18 +49,18 @@ contract SimulateVote is Script, DeploymentFixtures, ForkHelpersCommon {
_maxDepositsPerBlock: 30,
_minDepositBlockDistance: 25
});
// 3. burner role
// 2. burner role
burner.grantRole(
burner.REQUEST_BURN_SHARES_ROLE(),
address(accounting)
);
// 4. Grant resume to agent
// 3. Grant resume to agent
csm.grantRole(csm.RESUME_ROLE(), agent);
// 5. Resume CSM
// 4. Resume CSM
csm.resume();
// 6. Revoke resume
// 5. Revoke resume
csm.revokeRole(csm.RESUME_ROLE(), agent);
// 7. Update initial epoch
// 6. Update initial epoch
hashConsensus.updateInitialEpoch(47480);
}

Expand Down
8 changes: 3 additions & 5 deletions src/CSAccounting.sol
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ contract CSAccounting is
{
bytes32 public constant PAUSE_ROLE = keccak256("PAUSE_ROLE");
bytes32 public constant RESUME_ROLE = keccak256("RESUME_ROLE");
bytes32 public constant ACCOUNTING_MANAGER_ROLE =
keccak256("ACCOUNTING_MANAGER_ROLE");
bytes32 public constant MANAGE_BOND_CURVES_ROLE =
keccak256("MANAGE_BOND_CURVES_ROLE");
bytes32 public constant SET_BOND_CURVE_ROLE =
Expand Down Expand Up @@ -123,21 +121,21 @@ contract CSAccounting is
/// @inheritdoc ICSAccounting
function setChargePenaltyRecipient(
address _chargePenaltyRecipient
) external onlyRole(ACCOUNTING_MANAGER_ROLE) {
) external onlyRole(DEFAULT_ADMIN_ROLE) {
_setChargePenaltyRecipient(_chargePenaltyRecipient);
}

/// @dev DEPRECATED. Use `setLockedBondPeriod` instead
function setLockedBondRetentionPeriod(
uint256 retention
) external onlyRole(ACCOUNTING_MANAGER_ROLE) {
) external onlyRole(DEFAULT_ADMIN_ROLE) {
CSBondLock._setBondLockPeriod(retention);
}

/// @inheritdoc ICSAccounting
function setLockedBondPeriod(
uint256 period
) external onlyRole(ACCOUNTING_MANAGER_ROLE) {
) external onlyRole(DEFAULT_ADMIN_ROLE) {
CSBondLock._setBondLockPeriod(period);
}

Expand Down
8 changes: 2 additions & 6 deletions src/CSFeeOracle.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,6 @@ contract CSFeeOracle is
{
/// @notice No assets are stored in the contract

/// @notice An ACL role granting the permission to manage the contract (update variables).
bytes32 public constant CONTRACT_MANAGER_ROLE =
keccak256("CONTRACT_MANAGER_ROLE");

/// @notice An ACL role granting the permission to submit the data for a committee report.
bytes32 public constant SUBMIT_DATA_ROLE = keccak256("SUBMIT_DATA_ROLE");

Expand Down Expand Up @@ -68,14 +64,14 @@ contract CSFeeOracle is
/// @inheritdoc ICSFeeOracle
function setFeeDistributorContract(
address feeDistributorContract
) external onlyRole(CONTRACT_MANAGER_ROLE) {
) external onlyRole(DEFAULT_ADMIN_ROLE) {
_setFeeDistributorContract(feeDistributorContract);
}

/// @inheritdoc ICSFeeOracle
function setPerformanceLeeway(
uint256 valueBP
) external onlyRole(CONTRACT_MANAGER_ROLE) {
) external onlyRole(DEFAULT_ADMIN_ROLE) {
_setPerformanceLeeway(valueBP);
}

Expand Down
6 changes: 2 additions & 4 deletions src/CSModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ contract CSModule is

bytes32 public constant PAUSE_ROLE = keccak256("PAUSE_ROLE");
bytes32 public constant RESUME_ROLE = keccak256("RESUME_ROLE");
bytes32 public constant MODULE_MANAGER_ROLE =
keccak256("MODULE_MANAGER_ROLE");
bytes32 public constant STAKING_ROUTER_ROLE =
keccak256("STAKING_ROUTER_ROLE");
bytes32 public constant REPORT_EL_REWARDS_STEALING_PENALTY_ROLE =
Expand Down Expand Up @@ -137,7 +135,7 @@ contract CSModule is
}

/// @inheritdoc ICSModule
function activatePublicRelease() external onlyRole(MODULE_MANAGER_ROLE) {
function activatePublicRelease() external onlyRole(DEFAULT_ADMIN_ROLE) {
if (publicRelease) revert AlreadyActivated();
publicRelease = true;
emit PublicRelease();
Expand All @@ -146,7 +144,7 @@ contract CSModule is
/// @inheritdoc ICSModule
function setKeyRemovalCharge(
uint256 amount
) external onlyRole(MODULE_MANAGER_ROLE) {
) external onlyRole(DEFAULT_ADMIN_ROLE) {
_setKeyRemovalCharge(amount);
}

Expand Down
2 changes: 0 additions & 2 deletions src/interfaces/ICSAccounting.sol
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ interface ICSAccounting is

function RESUME_ROLE() external view returns (bytes32);

function ACCOUNTING_MANAGER_ROLE() external view returns (bytes32);

function MANAGE_BOND_CURVES_ROLE() external view returns (bytes32);

function SET_BOND_CURVE_ROLE() external view returns (bytes32);
Expand Down
2 changes: 0 additions & 2 deletions src/interfaces/ICSFeeOracle.sol
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@ interface ICSFeeOracle is IAssetRecovererLib {
error InvalidPerfLeeway();
error SenderNotAllowed();

function CONTRACT_MANAGER_ROLE() external view returns (bytes32);

function SUBMIT_DATA_ROLE() external view returns (bytes32);

function PAUSE_ROLE() external view returns (bytes32);
Expand Down
2 changes: 0 additions & 2 deletions src/interfaces/ICSModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,6 @@ interface ICSModule is IQueueLib, INOAddresses, IAssetRecovererLib {
view
returns (uint256);

function MODULE_MANAGER_ROLE() external view returns (bytes32);

function PAUSE_ROLE() external view returns (bytes32);

function RECOVERER_ROLE() external view returns (bytes32);
Expand Down
5 changes: 2 additions & 3 deletions test/CSAccounting.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,6 @@ contract CSAccountingBaseTest is CSAccountingFixtures {

vm.startPrank(admin);

accounting.grantRole(accounting.ACCOUNTING_MANAGER_ROLE(), admin);
accounting.grantRole(accounting.PAUSE_ROLE(), admin);
accounting.grantRole(accounting.RESUME_ROLE(), admin);
accounting.grantRole(accounting.MANAGE_BOND_CURVES_ROLE(), admin);
Expand Down Expand Up @@ -3613,7 +3612,7 @@ contract CSAccountingLockBondETHTest is CSAccountingBaseTest {
}

function setLockedBondPeriod_RevertWhen_DoesNotHaveRole() public {
expectRoleRevert(stranger, accounting.ACCOUNTING_MANAGER_ROLE());
expectRoleRevert(stranger, accounting.DEFAULT_ADMIN_ROLE());
vm.prank(stranger);
accounting.setLockedBondPeriod(200 days);
}
Expand Down Expand Up @@ -3907,7 +3906,7 @@ contract CSAccountingMiscTest is CSAccountingBaseTest {
function test_setChargePenaltyRecipient_RevertWhen_DoesNotHaveRole()
public
{
expectRoleRevert(stranger, accounting.ACCOUNTING_MANAGER_ROLE());
expectRoleRevert(stranger, accounting.DEFAULT_ADMIN_ROLE());
vm.prank(stranger);
accounting.setChargePenaltyRecipient(address(1337));
}
Expand Down
1 change: 0 additions & 1 deletion test/CSFeeOracle.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,6 @@ contract CSFeeOracleTest is Test, Utilities {
oracle.grantRole(oracle.MANAGE_CONSENSUS_VERSION_ROLE(), ORACLE_ADMIN);
oracle.grantRole(oracle.PAUSE_ROLE(), ORACLE_ADMIN);
oracle.grantRole(oracle.RESUME_ROLE(), ORACLE_ADMIN);
oracle.grantRole(oracle.CONTRACT_MANAGER_ROLE(), ORACLE_ADMIN);
}
vm.stopPrank();
}
Expand Down
21 changes: 9 additions & 12 deletions test/CSModule.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ contract CSMCommonNoPublicRelease is CSMFixtures {
vm.startPrank(admin);
csm.grantRole(csm.PAUSE_ROLE(), address(this));
csm.grantRole(csm.RESUME_ROLE(), address(this));
csm.grantRole(csm.MODULE_MANAGER_ROLE(), address(this));
csm.grantRole(csm.DEFAULT_ADMIN_ROLE(), address(this));
csm.grantRole(csm.STAKING_ROUTER_ROLE(), address(this));
csm.grantRole(
csm.SETTLE_EL_REWARDS_STEALING_PENALTY_ROLE(),
Expand Down Expand Up @@ -418,7 +418,7 @@ contract CSMCommonNoPublicReleaseNoEA is CSMFixtures {
vm.startPrank(admin);
csm.grantRole(csm.PAUSE_ROLE(), address(this));
csm.grantRole(csm.RESUME_ROLE(), address(this));
csm.grantRole(csm.MODULE_MANAGER_ROLE(), address(this));
csm.grantRole(csm.DEFAULT_ADMIN_ROLE(), address(this));
csm.grantRole(csm.STAKING_ROUTER_ROLE(), address(this));
csm.grantRole(
csm.SETTLE_EL_REWARDS_STEALING_PENALTY_ROLE(),
Expand Down Expand Up @@ -516,7 +516,7 @@ contract CSMCommonNoRoles is CSMFixtures {
});

vm.startPrank(admin);
csm.grantRole(csm.MODULE_MANAGER_ROLE(), address(this));
csm.grantRole(csm.DEFAULT_ADMIN_ROLE(), address(this));
csm.grantRole(csm.RESUME_ROLE(), admin);
csm.grantRole(csm.VERIFIER_ROLE(), address(this));
csm.resume();
Expand Down Expand Up @@ -4163,9 +4163,6 @@ contract CSMRemoveKeysChargeFee is CSMCommon {
}

function test_removeKeys_withNoFee() public assertInvariants {
bytes32 role = csm.MODULE_MANAGER_ROLE();
vm.prank(admin);
csm.grantRole(role, admin);
vm.prank(admin);
csm.setKeyRemovalCharge(0);

Expand Down Expand Up @@ -6446,7 +6443,7 @@ contract CSMAccessControl is CSMCommonNoRoles {
_enableInitializers(address(csm));
csm.initialize(address(154), address(42), 0.05 ether, actor);

bytes32 role = csm.MODULE_MANAGER_ROLE();
bytes32 role = csm.DEFAULT_ADMIN_ROLE();
vm.prank(actor);
csm.grantRole(role, stranger);
assertTrue(csm.hasRole(role, stranger));
Expand All @@ -6465,7 +6462,7 @@ contract CSMAccessControl is CSMCommonNoRoles {
maxKeyRemovalCharge: 0.1 ether,
lidoLocator: address(locator)
});
bytes32 role = csm.MODULE_MANAGER_ROLE();
bytes32 role = csm.DEFAULT_ADMIN_ROLE();
bytes32 adminRole = csm.DEFAULT_ADMIN_ROLE();

vm.startPrank(stranger);
Expand All @@ -6474,7 +6471,7 @@ contract CSMAccessControl is CSMCommonNoRoles {
}

function test_moduleManagerRole_setRemovalCharge() public {
bytes32 role = csm.MODULE_MANAGER_ROLE();
bytes32 role = csm.DEFAULT_ADMIN_ROLE();
vm.prank(admin);
csm.grantRole(role, actor);

Expand All @@ -6483,15 +6480,15 @@ contract CSMAccessControl is CSMCommonNoRoles {
}

function test_moduleManagerRole_setRemovalCharge_revert() public {
bytes32 role = csm.MODULE_MANAGER_ROLE();
bytes32 role = csm.DEFAULT_ADMIN_ROLE();

vm.prank(stranger);
expectRoleRevert(stranger, role);
csm.setKeyRemovalCharge(0.1 ether);
}

function test_moduleManagerRole_activatePublicRelease() public {
bytes32 role = csm.MODULE_MANAGER_ROLE();
bytes32 role = csm.DEFAULT_ADMIN_ROLE();
vm.prank(admin);
csm.grantRole(role, actor);

Expand All @@ -6500,7 +6497,7 @@ contract CSMAccessControl is CSMCommonNoRoles {
}

function test_moduleManagerRole_activatePublicRelease_revert() public {
bytes32 role = csm.MODULE_MANAGER_ROLE();
bytes32 role = csm.DEFAULT_ADMIN_ROLE();

vm.prank(stranger);
expectRoleRevert(stranger, role);
Expand Down
6 changes: 0 additions & 6 deletions test/fork/deployment/PostDeployment.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ contract CSModuleDeploymentTest is Test, Utilities, DeploymentFixtures {
);
assertTrue(csm.hasRole(csm.VERIFIER_ROLE(), address(verifier)));
assertEq(csm.getRoleMemberCount(csm.VERIFIER_ROLE()), 1);
assertEq(csm.getRoleMemberCount(csm.MODULE_MANAGER_ROLE()), 0);
assertEq(csm.getRoleMemberCount(csm.RECOVERER_ROLE()), 0);
}

Expand Down Expand Up @@ -228,10 +227,6 @@ contract CSAccountingDeploymentTest is Test, Utilities, DeploymentFixtures {
2
);
assertEq(accounting.getRoleMemberCount(accounting.RESUME_ROLE()), 0);
assertEq(
accounting.getRoleMemberCount(accounting.ACCOUNTING_MANAGER_ROLE()),
0
);
assertEq(
accounting.getRoleMemberCount(accounting.MANAGE_BOND_CURVES_ROLE()),
0
Expand Down Expand Up @@ -349,7 +344,6 @@ contract CSFeeOracleDeploymentTest is Test, Utilities, DeploymentFixtures {
assertTrue(oracle.hasRole(oracle.PAUSE_ROLE(), address(gateSeal)));
assertEq(oracle.getRoleMemberCount(oracle.PAUSE_ROLE()), 1);
assertEq(oracle.getRoleMemberCount(oracle.RESUME_ROLE()), 0);
assertEq(oracle.getRoleMemberCount(oracle.CONTRACT_MANAGER_ROLE()), 0);
assertEq(oracle.getRoleMemberCount(oracle.SUBMIT_DATA_ROLE()), 0);
assertEq(oracle.getRoleMemberCount(oracle.RECOVERER_ROLE()), 0);
assertEq(
Expand Down
2 changes: 1 addition & 1 deletion test/fork/integration/ClaimInTokens.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ contract ClaimIntegrationTest is

vm.startPrank(csm.getRoleMember(csm.DEFAULT_ADMIN_ROLE(), 0));
csm.grantRole(csm.RESUME_ROLE(), address(this));
csm.grantRole(csm.MODULE_MANAGER_ROLE(), address(this));
csm.grantRole(csm.DEFAULT_ADMIN_ROLE(), address(this));
csm.grantRole(csm.STAKING_ROUTER_ROLE(), address(stakingRouter));
vm.stopPrank();
if (csm.isPaused()) csm.resume();
Expand Down
2 changes: 1 addition & 1 deletion test/fork/integration/DepositInTokens.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ contract DepositIntegrationTest is

vm.startPrank(csm.getRoleMember(csm.DEFAULT_ADMIN_ROLE(), 0));
csm.grantRole(csm.RESUME_ROLE(), address(this));
csm.grantRole(csm.MODULE_MANAGER_ROLE(), address(this));
csm.grantRole(csm.DEFAULT_ADMIN_ROLE(), address(this));
csm.grantRole(csm.STAKING_ROUTER_ROLE(), address(stakingRouter));
vm.stopPrank();
if (csm.isPaused()) csm.resume();
Expand Down
2 changes: 1 addition & 1 deletion test/fork/integration/GateSeal.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ contract GateSealTest is Test, Utilities, DeploymentFixtures {

vm.startPrank(csm.getRoleMember(csm.DEFAULT_ADMIN_ROLE(), 0));
csm.grantRole(csm.RESUME_ROLE(), address(this));
csm.grantRole(csm.MODULE_MANAGER_ROLE(), address(this));
csm.grantRole(csm.DEFAULT_ADMIN_ROLE(), address(this));
vm.stopPrank();
if (csm.isPaused()) csm.resume();
if (!csm.publicRelease()) csm.activatePublicRelease();
Expand Down
2 changes: 1 addition & 1 deletion test/fork/integration/Penalty.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ contract PenaltyIntegrationTest is

vm.startPrank(csm.getRoleMember(csm.DEFAULT_ADMIN_ROLE(), 0));
csm.grantRole(csm.RESUME_ROLE(), address(this));
csm.grantRole(csm.MODULE_MANAGER_ROLE(), address(this));
csm.grantRole(csm.DEFAULT_ADMIN_ROLE(), address(this));
csm.grantRole(
csm.REPORT_EL_REWARDS_STEALING_PENALTY_ROLE(),
address(this)
Expand Down
15 changes: 0 additions & 15 deletions test/fork/invariant/Invariants.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,6 @@ contract CSModuleInvariants is InvariantsBase {
"pause address"
);
assertEq(csm.getRoleMemberCount(csm.RESUME_ROLE()), 0, "resume");
assertEq(
csm.getRoleMemberCount(csm.MODULE_MANAGER_ROLE()),
0,
"module manager"
);
assertEq(
csm.getRoleMemberCount(csm.STAKING_ROUTER_ROLE()),
1,
Expand Down Expand Up @@ -126,11 +121,6 @@ contract CSAccountingInvariants is InvariantsBase {
0,
"resume"
);
assertEq(
accounting.getRoleMemberCount(accounting.ACCOUNTING_MANAGER_ROLE()),
0,
"accounting manager"
);
assertEq(
accounting.getRoleMemberCount(accounting.MANAGE_BOND_CURVES_ROLE()),
0,
Expand Down Expand Up @@ -197,11 +187,6 @@ contract CSFeeOracleInvariant is InvariantsBase {
adminsCount,
"default admin"
);
assertEq(
oracle.getRoleMemberCount(oracle.CONTRACT_MANAGER_ROLE()),
0,
"contract manager"
);
assertEq(
oracle.getRoleMemberCount(oracle.SUBMIT_DATA_ROLE()),
0,
Expand Down
Loading

0 comments on commit 7ec80dd

Please sign in to comment.