Skip to content

Commit

Permalink
Merge pull request #169 from lidofinance/fix/functions-visibility
Browse files Browse the repository at this point in the history
Audit fix: Function Visibility Overly Permissive
  • Loading branch information
bulbozaur authored Nov 11, 2024
2 parents d8f759b + b5f735b commit a428d7c
Show file tree
Hide file tree
Showing 9 changed files with 44 additions and 44 deletions.
6 changes: 3 additions & 3 deletions contracts/EmergencyProtectedTimelock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -232,19 +232,19 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock {

/// @notice Returns whether the emergency protection is enabled.
/// @return isEmergencyProtectionEnabled A boolean indicating whether the emergency protection is enabled.
function isEmergencyProtectionEnabled() public view returns (bool) {
function isEmergencyProtectionEnabled() external view returns (bool) {
return _emergencyProtection.isEmergencyProtectionEnabled();
}

/// @notice Returns whether the emergency mode is active.
/// @return isEmergencyModeActive A boolean indicating whether the emergency protection is enabled.
function isEmergencyModeActive() public view returns (bool) {
function isEmergencyModeActive() external view returns (bool) {
return _emergencyProtection.isEmergencyModeActive();
}

/// @notice Returns the details of the emergency protection.
/// @return details A struct containing the emergency mode duration, emergency mode ends after, and emergency protection ends after.
function getEmergencyProtectionDetails() public view returns (EmergencyProtectionDetails memory details) {
function getEmergencyProtectionDetails() external view returns (EmergencyProtectionDetails memory details) {
return _emergencyProtection.getEmergencyProtectionDetails();
}

Expand Down
4 changes: 2 additions & 2 deletions contracts/ResealManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ contract ResealManager is IResealManager {
/// - The DAO governance is blocked by DualGovernance;
/// - Function is called by the governance contract.
/// @param sealable The address of the sealable contract.
function reseal(address sealable) public {
function reseal(address sealable) external {
_checkCallerIsGovernance();

uint256 sealableResumeSinceTimestamp = ISealable(sealable).getResumeSinceTimestamp();
Expand All @@ -62,7 +62,7 @@ contract ResealManager is IResealManager {
/// - Contract are paused until timestamp after current timestamp;
/// - Function is called by the governance contract.
/// @param sealable The address of the sealable contract.
function resume(address sealable) public {
function resume(address sealable) external {
_checkCallerIsGovernance();

uint256 sealableResumeSinceTimestamp = ISealable(sealable).getResumeSinceTimestamp();
Expand Down
14 changes: 7 additions & 7 deletions contracts/committees/HashConsensus.sol
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ abstract contract HashConsensus is Ownable {
/// 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 {
function addMembers(address[] memory newMembers, uint256 executionQuorum) external {
_checkOwner();

_addMembers(newMembers, executionQuorum);
Expand All @@ -126,7 +126,7 @@ abstract contract HashConsensus is Ownable {
/// the new total number of members.
/// @param membersToRemove The array of addresses to be removed from the members list.
/// @param executionQuorum The updated minimum number of members required for executing certain operations.
function removeMembers(address[] memory membersToRemove, uint256 executionQuorum) public {
function removeMembers(address[] memory membersToRemove, uint256 executionQuorum) external {
_checkOwner();

_removeMembers(membersToRemove, executionQuorum);
Expand All @@ -135,22 +135,22 @@ abstract contract HashConsensus is Ownable {
/// @notice Gets the list of committee members
/// @dev Public function to return the list of members
/// @return An array of addresses representing the committee members
function getMembers() public view returns (address[] memory) {
function getMembers() external view returns (address[] memory) {
return _members.values();
}

/// @notice Checks if an address is a member of the committee
/// @dev Public function to check membership status
/// @param member The address to check
/// @return A boolean indicating whether the address is a member
function isMember(address member) public view returns (bool) {
function isMember(address member) external view returns (bool) {
return _members.contains(member);
}

/// @notice Sets the timelock duration
/// @dev Only callable by the owner
/// @param timelock The new timelock duration in seconds
function setTimelockDuration(Duration timelock) public {
function setTimelockDuration(Duration timelock) external {
_checkOwner();
if (timelock == timelockDuration) {
revert InvalidTimelockDuration(timelock);
Expand All @@ -162,7 +162,7 @@ abstract contract HashConsensus is Ownable {
/// @notice Sets the quorum value
/// @dev Only callable by the owner
/// @param newQuorum The new quorum value
function setQuorum(uint256 newQuorum) public {
function setQuorum(uint256 newQuorum) external {
_checkOwner();
if (newQuorum == quorum) {
revert InvalidQuorum();
Expand All @@ -175,7 +175,7 @@ abstract contract HashConsensus is Ownable {
/// the proposal has not been scheduled yet. Could happen when execution quorum was set to the same value as
/// current support of the proposal.
/// @param hash The hash of the proposal to be scheduled
function schedule(bytes32 hash) public {
function schedule(bytes32 hash) external {
if (_hashStates[hash].scheduledAt.isNotZero()) {
revert HashAlreadyScheduled(hash);
}
Expand Down
10 changes: 5 additions & 5 deletions contracts/committees/ProposalsList.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ contract ProposalsList {
/// @param offset The starting index for the list of proposals
/// @param limit The maximum number of proposals to return
/// @return proposals An array of Proposal structs
function getProposals(uint256 offset, uint256 limit) public view returns (Proposal[] memory proposals) {
function getProposals(uint256 offset, uint256 limit) external view returns (Proposal[] memory proposals) {
bytes32[] memory keys = _proposals.getOrderedKeys(offset, limit);

uint256 length = keys.length;
Expand All @@ -31,22 +31,22 @@ contract ProposalsList {
/// @dev Fetches the proposal located at the specified index in the map
/// @param index The index of the proposal to retrieve
/// @return The Proposal struct at the given index
function getProposalAt(uint256 index) public view returns (Proposal memory) {
function getProposalAt(uint256 index) external view returns (Proposal memory) {
return _proposals.at(index);
}

/// @notice Retrieves a proposal by its key
/// @dev Fetches the proposal associated with the given key
/// @param key The key of the proposal to retrieve
/// @return The Proposal struct associated with the given key
function getProposal(bytes32 key) public view returns (Proposal memory) {
function getProposal(bytes32 key) external view returns (Proposal memory) {
return _proposals.get(key);
}

/// @notice Retrieves the total number of proposals
/// @dev Fetches the length of the proposals map
/// @return The total number of proposals
function getProposalsLength() public view returns (uint256) {
function getProposalsLength() external view returns (uint256) {
return _proposals.length();
}

Expand All @@ -55,7 +55,7 @@ contract ProposalsList {
/// @param offset The starting index for the list of keys
/// @param limit The maximum number of keys to return
/// @return An array of proposal keys
function getOrderedKeys(uint256 offset, uint256 limit) public view returns (bytes32[] memory) {
function getOrderedKeys(uint256 offset, uint256 limit) external view returns (bytes32[] memory) {
return _proposals.getOrderedKeys(offset, limit);
}

Expand Down
4 changes: 2 additions & 2 deletions contracts/committees/ResealCommittee.sol
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ contract ResealCommittee is HashConsensus, ProposalsList {
/// @dev Allows committee members to vote on resealing a sealed address
/// @param sealable The address to reseal
/// @param support Indicates whether the member supports the proposal
function voteReseal(address sealable, bool support) public {
function voteReseal(address sealable, bool support) external {
_checkCallerIsMember();

if (sealable == address(0)) {
Expand All @@ -56,7 +56,7 @@ contract ResealCommittee is HashConsensus, ProposalsList {
/// @return executionQuorum The required number of votes for execution
/// @return quorumAt The timestamp when the quorum was reached
function getResealState(address sealable)
public
external
view
returns (uint256 support, uint256 executionQuorum, Timestamp quorumAt)
{
Expand Down
12 changes: 6 additions & 6 deletions contracts/committees/TiebreakerCoreCommittee.sol
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ contract TiebreakerCoreCommittee is ITiebreakerCoreCommittee, HashConsensus, Pro
/// @notice Votes on a proposal to schedule
/// @dev Allows committee members to vote on scheduling a proposal
/// @param proposalId The ID of the proposal to schedule
function scheduleProposal(uint256 proposalId) public {
function scheduleProposal(uint256 proposalId) external {
_checkCallerIsMember();
checkProposalExists(proposalId);
(bytes memory proposalData, bytes32 key) = _encodeScheduleProposal(proposalId);
Expand All @@ -58,7 +58,7 @@ contract TiebreakerCoreCommittee is ITiebreakerCoreCommittee, HashConsensus, Pro
/// @return quorumAt The timestamp when the quorum was reached
/// @return isExecuted Whether the proposal has been executed
function getScheduleProposalState(uint256 proposalId)
public
external
view
returns (uint256 support, uint256 executionQuorum, Timestamp quorumAt, bool isExecuted)
{
Expand All @@ -69,7 +69,7 @@ contract TiebreakerCoreCommittee is ITiebreakerCoreCommittee, HashConsensus, Pro
/// @notice Executes an approved schedule proposal
/// @dev Executes the schedule proposal by calling the tiebreakerScheduleProposal function on the Dual Governance contract
/// @param proposalId The ID of the proposal to schedule
function executeScheduleProposal(uint256 proposalId) public {
function executeScheduleProposal(uint256 proposalId) external {
(, bytes32 key) = _encodeScheduleProposal(proposalId);
_markUsed(key);
Address.functionCall(
Expand Down Expand Up @@ -105,7 +105,7 @@ contract TiebreakerCoreCommittee is ITiebreakerCoreCommittee, HashConsensus, Pro
/// @dev Retrieves the resume nonce for the given sealable address
/// @param sealable The address of the sealable to get the nonce for
/// @return The current resume nonce for the sealable address
function getSealableResumeNonce(address sealable) public view returns (uint256) {
function getSealableResumeNonce(address sealable) external view returns (uint256) {
return _sealableResumeNonces[sealable];
}

Expand All @@ -114,7 +114,7 @@ contract TiebreakerCoreCommittee is ITiebreakerCoreCommittee, HashConsensus, Pro
/// reverts if the sealable address is the zero address
/// @param sealable The address to resume
/// @param nonce The nonce for the resume proposal
function sealableResume(address sealable, uint256 nonce) public {
function sealableResume(address sealable, uint256 nonce) external {
_checkCallerIsMember();

if (sealable == address(0)) {
Expand All @@ -140,7 +140,7 @@ contract TiebreakerCoreCommittee is ITiebreakerCoreCommittee, HashConsensus, Pro
function getSealableResumeState(
address sealable,
uint256 nonce
) public view returns (uint256 support, uint256 executionQuorum, Timestamp quorumAt, bool isExecuted) {
) external view returns (uint256 support, uint256 executionQuorum, Timestamp quorumAt, bool isExecuted) {
(, bytes32 key) = _encodeSealableResume(sealable, nonce);
return _getHashState(key);
}
Expand Down
12 changes: 6 additions & 6 deletions contracts/committees/TiebreakerSubCommittee.sol
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ contract TiebreakerSubCommittee is HashConsensus, ProposalsList {
/// @notice Votes on a proposal to schedule
/// @dev Allows committee members to vote on scheduling a proposal
/// @param proposalId The ID of the proposal to schedule
function scheduleProposal(uint256 proposalId) public {
function scheduleProposal(uint256 proposalId) external {
_checkCallerIsMember();
ITiebreakerCoreCommittee(TIEBREAKER_CORE_COMMITTEE).checkProposalExists(proposalId);
(bytes memory proposalData, bytes32 key) = _encodeApproveProposal(proposalId);
Expand All @@ -58,7 +58,7 @@ contract TiebreakerSubCommittee is HashConsensus, ProposalsList {
/// @return quorumAt The number of votes required to reach quorum
/// @return isExecuted Whether the proposal has been executed
function getScheduleProposalState(uint256 proposalId)
public
external
view
returns (uint256 support, uint256 executionQuorum, Timestamp quorumAt, bool isExecuted)
{
Expand All @@ -69,7 +69,7 @@ contract TiebreakerSubCommittee is HashConsensus, ProposalsList {
/// @notice Executes an approved schedule proposal
/// @dev Executes the schedule proposal by calling the scheduleProposal function on the Tiebreaker Core contract
/// @param proposalId The ID of the proposal to schedule
function executeScheduleProposal(uint256 proposalId) public {
function executeScheduleProposal(uint256 proposalId) external {
(, bytes32 key) = _encodeApproveProposal(proposalId);
_markUsed(key);
Address.functionCall(
Expand All @@ -96,7 +96,7 @@ contract TiebreakerSubCommittee is HashConsensus, ProposalsList {
/// @dev Allows committee members to vote on resuming a sealable address
/// reverts if the sealable address is the zero address
/// @param sealable The address to resume
function sealableResume(address sealable) public {
function sealableResume(address sealable) external {
_checkCallerIsMember();

if (sealable == address(0)) {
Expand All @@ -115,7 +115,7 @@ contract TiebreakerSubCommittee is HashConsensus, ProposalsList {
/// @return quorumAt The timestamp when the quorum was reached
/// @return isExecuted Whether the proposal has been executed
function getSealableResumeState(address sealable)
public
external
view
returns (uint256 support, uint256 executionQuorum, Timestamp quorumAt, bool isExecuted)
{
Expand All @@ -126,7 +126,7 @@ contract TiebreakerSubCommittee is HashConsensus, ProposalsList {
/// @notice Executes an approved resume sealable proposal
/// @dev Executes the resume sealable proposal by calling the sealableResume function on the Tiebreaker Core contract
/// @param sealable The address to resume
function executeSealableResume(address sealable) public {
function executeSealableResume(address sealable) external {
(, bytes32 key, uint256 nonce) = _encodeSealableResume(sealable);
_markUsed(key);
Address.functionCall(
Expand Down
22 changes: 11 additions & 11 deletions docs/plan-b.md
Original file line number Diff line number Diff line change
Expand Up @@ -238,31 +238,31 @@ The result of the call.

### Function: `ProposalsList.getProposals`
```solidity
function getProposals(uint256 offset, uint256 limit) public view returns (Proposal[] memory proposals)
function getProposals(uint256 offset, uint256 limit) external view returns (Proposal[] memory proposals)
```
Returns a list of `Proposal` structs, starting from the specified `offset` and bounded to the specified `limit`.

### Function: `ProposalsList.getProposalAt`
```solidity
function getProposalAt(uint256 index) public view returns (Proposal memory)
function getProposalAt(uint256 index) external view returns (Proposal memory)
```
Returns the `Proposal` at the specified index.

### Function: `ProposalsList.getProposal`
```solidity
function getProposal(bytes32 key) public view returns (Proposal memory)
function getProposal(bytes32 key) external view returns (Proposal memory)
```
Returns the `Proposal` with the given key.

### Function: `ProposalsList.getProposalsLength`
```solidity
function getProposalsLength() public view returns (uint256)
function getProposalsLength() external view returns (uint256)
```
Returns the total number of created `Proposal`s.

### Function: `ProposalsList.getOrderedKeys`
```solidity
function getOrderedKeys(uint256 offset, uint256 limit) public view returns (bytes32[] memory)
function getOrderedKeys(uint256 offset, uint256 limit) external view returns (bytes32[] memory)
```
Returns a list of `Proposal` keys, starting from the specified `offset` and bounded by the specified `limit`.

Expand All @@ -272,7 +272,7 @@ Returns a list of `Proposal` keys, starting from the specified `offset` and boun

### Function: `HashConsensus.addMember`
```solidity
function addMember(address newMember, uint256 newQuorum) public onlyOwner
function addMember(address newMember, uint256 newQuorum) external onlyOwner
```
Adds a new member and updates the quorum.

Expand All @@ -282,7 +282,7 @@ Adds a new member and updates the quorum.

### Function: `HashConsensus.removeMember`
```solidity
function removeMember(address memberToRemove, uint256 newQuorum) public onlyOwner
function removeMember(address memberToRemove, uint256 newQuorum) external onlyOwner
```
Removes a member and updates the quorum.

Expand All @@ -293,19 +293,19 @@ Removes a member and updates the quorum.

### Function: `HashConsensus.getMembers`
```solidity
function getMembers() public view returns (address[] memory)
function getMembers() external view returns (address[] memory)
```
Returns the list of current members.

### Function: `HashConsensus.isMember`
```solidity
function isMember(address member) public view returns (bool)
function isMember(address member) external view returns (bool)
```
Returns whether an account is listed as a member.

### Function: `HashConsensus.setTimelockDuration`
```solidity
function setTimelockDuration(uint256 timelock) public onlyOwner
function setTimelockDuration(uint256 timelock) external onlyOwner
```
Sets the duration of the timelock.

Expand All @@ -314,7 +314,7 @@ Sets the duration of the timelock.

### Function: `HashConsensus.setQuorum`
```solidity
function setQuorum(uint256 newQuorum) public onlyOwner
function setQuorum(uint256 newQuorum) external onlyOwner
```
Sets the quorum required for decision execution.

Expand Down
4 changes: 2 additions & 2 deletions docs/specification.md
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ To function properly, the **ResealManager** must be granted the `PAUSE_ROLE` and
### Function ResealManager.reseal

```solidity
function reseal(address sealable) public
function reseal(address sealable) external
```

Extends the pause of the specified `sealable` contract indefinitely.
Expand Down Expand Up @@ -1372,7 +1372,7 @@ Returns the state of a sealable resume request including support count, quorum,
### Function: TiebreakerSubCommittee.executeSealableResume

```solidity
function executeSealableResume(address sealable) public
function executeSealableResume(address sealable) external
```

Executes a sealable resume request by calling the sealableResume function on the `TiebreakerCoreCommittee` contract and increments the nonce.
Expand Down

0 comments on commit a428d7c

Please sign in to comment.