Merge pull request #91 from lidofinance/feature/committee-setMembers

Committees refactoring: add/remove list of members

contracts/committees/EmergencyActivationCommittee.sol

@@ -19,8 +19,10 @@ contract EmergencyActivationCommittee is HashConsensus {
         address[] memory committeeMembers,
         uint256 executionQuorum,
         address emergencyProtectedTimelock
-    ) HashConsensus(owner, committeeMembers, executionQuorum, 0) {
+    ) HashConsensus(owner, 0) {
         EMERGENCY_PROTECTED_TIMELOCK = emergencyProtectedTimelock;
+
+        _addMembers(committeeMembers, executionQuorum);
     }
 
     /// @notice Approves the emergency activation by casting a vote

contracts/committees/EmergencyExecutionCommittee.sol

@@ -22,8 +22,10 @@ contract EmergencyExecutionCommittee is HashConsensus, ProposalsList {
         address[] memory committeeMembers,
         uint256 executionQuorum,
         address emergencyProtectedTimelock
-    ) HashConsensus(owner, committeeMembers, executionQuorum, 0) {
+    ) HashConsensus(owner, 0) {
         EMERGENCY_PROTECTED_TIMELOCK = emergencyProtectedTimelock;
+
+        _addMembers(committeeMembers, executionQuorum);
     }
 
     // ---

contracts/committees/HashConsensus.sol

@@ -37,19 +37,9 @@ abstract contract HashConsensus is Ownable {
     EnumerableSet.AddressSet private _members;
     mapping(address signer => mapping(bytes32 => bool)) public approves;
 
-    constructor(address owner, address[] memory newMembers, uint256 executionQuorum, uint256 timelock) Ownable(owner) {
-        if (executionQuorum == 0) {
-            revert InvalidQuorum();
-        }
-        quorum = executionQuorum;
-        emit QuorumSet(executionQuorum);
-
+    constructor(address owner, uint256 timelock) Ownable(owner) {
         timelockDuration = timelock;
         emit TimelockDurationSet(timelock);
-
-        for (uint256 i = 0; i < newMembers.length; ++i) {
-            _addMember(newMembers[i]);
-        }
     }
 
     /// @notice Casts a vote on a given hash if hash has not been used
@@ -81,7 +71,10 @@ abstract contract HashConsensus is Ownable {
         if (_hashStates[hash].usedAt > 0) {
             revert HashAlreadyUsed(hash);
         }
-        if (_getSupport(hash) < quorum) {
+
+        uint256 support = _getSupport(hash);
+
+        if (support == 0 || support < quorum) {
             revert QuorumIsNotReached();
         }
         if (block.timestamp < _hashStates[hash].quorumAt + timelockDuration) {
@@ -109,39 +102,37 @@ abstract contract HashConsensus is Ownable {
         isUsed = _hashStates[hash].usedAt > 0;
     }
 
-    /// @notice Adds a new member to the committee and updates the quorum
-    /// @dev Only callable by the owner
-    /// @param newMember The address of the new member
-    /// @param newQuorum The new quorum value
-    function addMember(address newMember, uint256 newQuorum) public {
+    /// @notice Adds new members to the contract and sets the execution quorum.
+    /// @dev This function allows the contract owner to add multiple new members and set the execution quorum.
+    ///      The function reverts if the caller is not the owner, if the execution quorum is set to zero,
+    ///      or if it exceeds the total number of members.
+    /// @param newMembers The array of addresses to be added as new members
+    /// @param executionQuorum The minimum number of members required for executing certain operations
+    function addMembers(address[] memory newMembers, uint256 executionQuorum) public {
         _checkOwner();
-        _addMember(newMember);
 
-        if (newQuorum == 0 || newQuorum > _members.length()) {
-            revert InvalidQuorum();
-        }
-        quorum = newQuorum;
-        emit QuorumSet(newQuorum);
+        _addMembers(newMembers, executionQuorum);
     }
 
-    /// @notice Removes a member from the committee and updates the quorum
-    /// @dev Only callable by the owner
-    /// @param memberToRemove The address of the member to remove
-    /// @param newQuorum The new quorum value
-    function removeMember(address memberToRemove, uint256 newQuorum) public {
+    /// @notice Removes specified members from the contract and updates the execution quorum.
+    /// @dev This function can only be called by the contract owner. It removes multiple members from
+    ///      the contract. If any of the specified members are not found in the members list, the
+    ///      function will revert. The quorum is also updated and must not be zero or greater than
+    ///      the new total number of members.
+    /// @param membersToRemove The array of addresses to be removed from the members list.
+    /// @param newQuorum The updated minimum number of members required for executing certain operations.
+    function removeMembers(address[] memory membersToRemove, uint256 newQuorum) public {
         _checkOwner();
 
-        if (!_members.contains(memberToRemove)) {
-            revert AccountIsNotMember(memberToRemove);
+        for (uint256 i = 0; i < membersToRemove.length; ++i) {
+            if (!_members.contains(membersToRemove[i])) {
+                revert AccountIsNotMember(membersToRemove[i]);
+            }
+            _members.remove(membersToRemove[i]);
+            emit MemberRemoved(membersToRemove[i]);
         }
-        _members.remove(memberToRemove);
-        emit MemberRemoved(memberToRemove);
 
-        if (newQuorum == 0 || newQuorum > _members.length()) {
-            revert InvalidQuorum();
-        }
-        quorum = newQuorum;
-        emit QuorumSet(newQuorum);
+        _setQuorum(newQuorum);
     }
 
     /// @notice Gets the list of committee members
@@ -173,23 +164,35 @@ abstract contract HashConsensus is Ownable {
     /// @param newQuorum The new quorum value
     function setQuorum(uint256 newQuorum) public {
         _checkOwner();
-        if (newQuorum == 0 || newQuorum > _members.length()) {
+        _setQuorum(newQuorum);
+    }
+
+    /// @notice Sets the execution quorum required for certain operations.
+    /// @dev The quorum value must be greater than zero and not exceed the current number of members.
+    /// @param executionQuorum The new quorum value to be set.
+    function _setQuorum(uint256 executionQuorum) internal {
+        if (executionQuorum == 0 || executionQuorum > _members.length()) {
             revert InvalidQuorum();
         }
-
-        quorum = newQuorum;
-        emit QuorumSet(newQuorum);
+        quorum = executionQuorum;
+        emit QuorumSet(executionQuorum);
     }
 
-    /// @notice Adds a new member to the committee
-    /// @dev Internal function to add a new member
-    /// @param newMember The address of the new member
-    function _addMember(address newMember) internal {
-        if (_members.contains(newMember)) {
-            revert DuplicatedMember(newMember);
+    /// @notice Adds new members to the contract and sets the execution quorum.
+    /// @dev This internal function adds multiple new members and sets the execution quorum.
+    ///      The function reverts if the execution quorum is set to zero or exceeds the total number of members.
+    /// @param newMembers The array of addresses to be added as new members.
+    /// @param executionQuorum The minimum number of members required for executing certain operations.
+    function _addMembers(address[] memory newMembers, uint256 executionQuorum) internal {
+        for (uint256 i = 0; i < newMembers.length; ++i) {
+            if (_members.contains(newMembers[i])) {
+                revert DuplicatedMember(newMembers[i]);
+            }
+            _members.add(newMembers[i]);
+            emit MemberAdded(newMembers[i]);
         }
-        _members.add(newMember);
-        emit MemberAdded(newMember);
+
+        _setQuorum(executionQuorum);
     }
 
     /// @notice Gets the number of votes in support of a given hash

contracts/committees/ResealCommittee.sol

@@ -21,8 +21,10 @@ contract ResealCommittee is HashConsensus, ProposalsList {
         uint256 executionQuorum,
         address dualGovernance,
         uint256 timelock
-    ) HashConsensus(owner, committeeMembers, executionQuorum, timelock) {
+    ) HashConsensus(owner, timelock) {
         DUAL_GOVERNANCE = dualGovernance;
+
+        _addMembers(committeeMembers, executionQuorum);
     }
 
     /// @notice Votes on a reseal proposal

contracts/committees/TiebreakerCore.sol

@@ -23,13 +23,7 @@ contract TiebreakerCore is ITiebreakerCore, HashConsensus, ProposalsList {
 
     mapping(address => uint256) private _sealableResumeNonces;
 
-    constructor(
-        address owner,
-        address[] memory committeeMembers,
-        uint256 executionQuorum,
-        address dualGovernance,
-        uint256 timelock
-    ) HashConsensus(owner, committeeMembers, executionQuorum, timelock) {
+    constructor(address owner, address dualGovernance, uint256 timelock) HashConsensus(owner, timelock) {
         DUAL_GOVERNANCE = dualGovernance;
     }
 

contracts/committees/TiebreakerSubCommittee.sol

@@ -23,8 +23,10 @@ contract TiebreakerSubCommittee is HashConsensus, ProposalsList {
         address[] memory committeeMembers,
         uint256 executionQuorum,
         address tiebreakerCore
-    ) HashConsensus(owner, committeeMembers, executionQuorum, 0) {
+    ) HashConsensus(owner, 0) {
         TIEBREAKER_CORE = tiebreakerCore;
+
+        _addMembers(committeeMembers, executionQuorum);
     }
 
     // ---
@@ -73,7 +75,7 @@ contract TiebreakerSubCommittee is HashConsensus, ProposalsList {
     /// @return data The encoded proposal data
     /// @return key The generated proposal key
     function _encodeAproveProposal(uint256 proposalId) internal pure returns (bytes memory data, bytes32 key) {
-        data = abi.encode(ProposalType.ScheduleProposal, data);
+        data = abi.encode(ProposalType.ScheduleProposal, proposalId);
         key = keccak256(data);
     }
 

docs/specification.md

@@ -1062,31 +1062,32 @@ Returns total list of `Proposal`s keys with offset and limit.
 
 `HashConsensus` is an abstract contract that allows for consensus-based decision-making among a set of members. The consensus is achieved by members voting on a specific hash, and decisions can only be executed if a quorum is reached and a timelock period has elapsed.
 
-### Function: HashConsensus.addMember
+### Function: HashConsensus.addMembers
 
 ```solidity
-function addMember(address newMember, uint256 newQuorum) public onlyOwner
+function addMembers(address[] memory newMembers, uint256 newQuorum) public onlyOwner
 ```
 
-Adds a new member and updates the quorum.
+Adds new members and updates the quorum.
 
 #### Preconditions
 
 * Only the owner can call this function.
+* Members MUST NOT be part of the set.
 * `newQuorum` MUST be greater than 0 and less than or equal to the number of members.
 
 ### Function: HashConsensus.removeMember
 
 ```solidity
-function removeMember(address memberToRemove, uint256 newQuorum) public onlyOwner
+function removeMembers(address[] memory membersToRemove, uint256 newQuorum) public onlyOwner
 ```
 
-Removes a member and updates the quorum.
+Removes members and updates the quorum.
 
 #### Preconditions
 
 * Only the owner can call this function.
-* Member MUST be part of the set.
+* Members MUST be part of the set.
 * `newQuorum` MUST be greater than 0 and less than or equal to the number of remaining members.
 
 ### Function: HashConsensus.getMembers