Skip to content

Commit

Permalink
Merge pull request #81 from lidofinance/feature/checks
Browse files Browse the repository at this point in the history
Rename checks checkSenderIs
  • Loading branch information
Psirex authored Aug 7, 2024
2 parents bec810f + 186e368 commit 0fcc95d
Show file tree
Hide file tree
Showing 9 changed files with 61 additions and 124 deletions.
28 changes: 14 additions & 14 deletions contracts/DualGovernance.sol
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ contract DualGovernance is IDualGovernance {

function submitProposal(ExternalCall[] calldata calls) external returns (uint256 proposalId) {
_stateMachine.activateNextState(_configProvider.getDualGovernanceConfig(), ESCROW_MASTER_COPY);
_proposers.checkProposer(msg.sender);
_proposers.checkSenderIsProposer();
if (!_stateMachine.canSubmitProposal()) {
revert ProposalSubmissionBlocked();
}
Expand All @@ -123,7 +123,7 @@ contract DualGovernance is IDualGovernance {
}

function cancelAllPendingProposals() external {
_proposers.checkAdminProposer(TIMELOCK.getAdminExecutor(), msg.sender);
_proposers.checkSenderIsAdminProposer(TIMELOCK.getAdminExecutor());
TIMELOCK.cancelAllNonExecutedProposals();
}

Expand All @@ -146,7 +146,7 @@ contract DualGovernance is IDualGovernance {
}

function setConfigProvider(IDualGovernanceConfigProvider newConfigProvider) external {
_checkAdminExecutor(msg.sender);
_checkSenderIsAdminExecutor();
_setConfigProvider(newConfigProvider);

/// @dev the minAssetsLockDuration is kept as a storage variable in the signalling Escrow instance
Expand Down Expand Up @@ -185,12 +185,12 @@ contract DualGovernance is IDualGovernance {
// ---

function registerProposer(address proposer, address executor) external {
_checkAdminExecutor(msg.sender);
_checkSenderIsAdminExecutor();
_proposers.register(proposer, executor);
}

function unregisterProposer(address proposer) external {
_checkAdminExecutor(msg.sender);
_checkSenderIsAdminExecutor();
_proposers.unregister(TIMELOCK.getAdminExecutor(), proposer);
}

Expand All @@ -215,35 +215,35 @@ contract DualGovernance is IDualGovernance {
// ---

function addTiebreakerSealableWithdrawalBlocker(address sealableWithdrawalBlocker) external {
_checkAdminExecutor(msg.sender);
_checkSenderIsAdminExecutor();
_tiebreaker.addSealableWithdrawalBlocker(sealableWithdrawalBlocker, MAX_SEALABLE_WITHDRAWAL_BLOCKERS_COUNT);
}

function removeTiebreakerSealableWithdrawalBlocker(address sealableWithdrawalBlocker) external {
_checkAdminExecutor(msg.sender);
_checkSenderIsAdminExecutor();
_tiebreaker.removeSealableWithdrawalBlocker(sealableWithdrawalBlocker);
}

function setTiebreakerCommittee(address tiebreakerCommittee) external {
_checkAdminExecutor(msg.sender);
_checkSenderIsAdminExecutor();
_tiebreaker.setTiebreakerCommittee(tiebreakerCommittee);
}

function setTiebreakerActivationTimeout(Duration tiebreakerActivationTimeout) external {
_checkAdminExecutor(msg.sender);
_checkSenderIsAdminExecutor();
_tiebreaker.setTiebreakerActivationTimeout(
MIN_TIEBREAKER_ACTIVATION_TIMEOUT, tiebreakerActivationTimeout, MAX_TIEBREAKER_ACTIVATION_TIMEOUT
);
}

function tiebreakerResumeSealable(address sealable) external {
_tiebreaker.checkTiebreakerCommittee(msg.sender);
_tiebreaker.checkSenderIsTiebreakerCommittee();
_tiebreaker.checkTie(_stateMachine.getCurrentState(), _stateMachine.getNormalOrVetoCooldownStateExitedAt());
RESEAL_MANAGER.resume(sealable);
}

function tiebreakerScheduleProposal(uint256 proposalId) external {
_tiebreaker.checkTiebreakerCommittee(msg.sender);
_tiebreaker.checkSenderIsTiebreakerCommittee();
_tiebreaker.checkTie(_stateMachine.getCurrentState(), _stateMachine.getNormalOrVetoCooldownStateExitedAt());
TIMELOCK.schedule(proposalId);
}
Expand Down Expand Up @@ -293,9 +293,9 @@ contract DualGovernance is IDualGovernance {
emit ConfigProviderSet(newConfigProvider);
}

function _checkAdminExecutor(address account) internal view {
if (TIMELOCK.getAdminExecutor() != account) {
revert InvalidAdminExecutor(account);
function _checkSenderIsAdminExecutor() internal view {
if (TIMELOCK.getAdminExecutor() != msg.sender) {
revert InvalidAdminExecutor(msg.sender);
}
}
}
28 changes: 14 additions & 14 deletions contracts/EmergencyProtectedTimelock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,15 @@ contract EmergencyProtectedTimelock is ITimelock {
/// @param calls An array of `ExternalCall` structs representing the calls to be executed.
/// @return newProposalId The ID of the newly created proposal.
function submit(address executor, ExternalCall[] calldata calls) external returns (uint256 newProposalId) {
_timelockState.checkGovernance(msg.sender);
_timelockState.checkSenderIsGovernance();
newProposalId = _proposals.submit(executor, calls);
}

/// @dev Schedules a proposal for execution after a specified delay.
/// Only the governance contract can call this function.
/// @param proposalId The ID of the proposal to be scheduled.
function schedule(uint256 proposalId) external {
_timelockState.checkGovernance(msg.sender);
_timelockState.checkSenderIsGovernance();
_proposals.schedule(proposalId, _timelockState.getAfterSubmitDelay());
}

Expand All @@ -96,7 +96,7 @@ contract EmergencyProtectedTimelock is ITimelock {
/// @dev Cancels all non-executed proposals.
/// Only the governance contract can call this function.
function cancelAllNonExecutedProposals() external {
_timelockState.checkGovernance(msg.sender);
_timelockState.checkSenderIsGovernance();
_proposals.cancelAll();
}

Expand All @@ -105,12 +105,12 @@ contract EmergencyProtectedTimelock is ITimelock {
// ---

function setGovernance(address newGovernance) external {
_checkAdminExecutor(msg.sender);
_checkSenderIsAdminExecutor();
_timelockState.setGovernance(newGovernance);
}

function setDelays(Duration afterSubmitDelay, Duration afterScheduleDelay) external {
_checkAdminExecutor(msg.sender);
_checkSenderIsAdminExecutor();
_timelockState.setAfterSubmitDelay(afterSubmitDelay, MAX_AFTER_SUBMIT_DELAY);
_timelockState.setAfterScheduleDelay(afterScheduleDelay, MAX_AFTER_SCHEDULE_DELAY);
}
Expand All @@ -120,7 +120,7 @@ contract EmergencyProtectedTimelock is ITimelock {
/// @param executor The address of the executor contract.
/// @param owner The address of the new owner.
function transferExecutorOwnership(address executor, address owner) external {
_checkAdminExecutor(msg.sender);
_checkSenderIsAdminExecutor();
IOwnable(executor).transferOwnership(owner);
}

Expand All @@ -135,7 +135,7 @@ contract EmergencyProtectedTimelock is ITimelock {
Timestamp emergencyProtectionEndDate,
Duration emergencyModeDuration
) external {
_checkAdminExecutor(msg.sender);
_checkSenderIsAdminExecutor();

_emergencyProtection.setEmergencyGovernance(emergencyGovernance);
_emergencyProtection.setEmergencyActivationCommittee(emergencyActivationCommittee);
Expand All @@ -149,7 +149,7 @@ contract EmergencyProtectedTimelock is ITimelock {
/// @dev Activates the emergency mode.
/// Only the activation committee can call this function.
function activateEmergencyMode() external {
_emergencyProtection.checkEmergencyActivationCommittee(msg.sender);
_emergencyProtection.checkSenderIsEmergencyActivationCommittee();
_emergencyProtection.checkEmergencyMode({isActive: false});
_emergencyProtection.activateEmergencyMode();
}
Expand All @@ -159,7 +159,7 @@ contract EmergencyProtectedTimelock is ITimelock {
/// @param proposalId The ID of the proposal to be executed.
function emergencyExecute(uint256 proposalId) external {
_emergencyProtection.checkEmergencyMode({isActive: true});
_emergencyProtection.checkEmergencyExecutionCommittee(msg.sender);
_emergencyProtection.checkSenderIsEmergencyExecutionCommittee();
_proposals.execute({proposalId: proposalId, afterScheduleDelay: Duration.wrap(0)});
}

Expand All @@ -168,7 +168,7 @@ contract EmergencyProtectedTimelock is ITimelock {
function deactivateEmergencyMode() external {
_emergencyProtection.checkEmergencyMode({isActive: true});
if (!_emergencyProtection.isEmergencyModeDurationPassed()) {
_checkAdminExecutor(msg.sender);
_checkSenderIsAdminExecutor();
}
_emergencyProtection.deactivateEmergencyMode();
_proposals.cancelAll();
Expand All @@ -177,7 +177,7 @@ contract EmergencyProtectedTimelock is ITimelock {
/// @dev Resets the system after entering the emergency mode.
/// Only the execution committee can call this function.
function emergencyReset() external {
_emergencyProtection.checkEmergencyExecutionCommittee(msg.sender);
_emergencyProtection.checkSenderIsEmergencyExecutionCommittee();
_emergencyProtection.checkEmergencyMode({isActive: true});
_emergencyProtection.deactivateEmergencyMode();

Expand Down Expand Up @@ -278,9 +278,9 @@ contract EmergencyProtectedTimelock is ITimelock {
return _proposals.canSchedule(proposalId, _timelockState.getAfterSubmitDelay());
}

function _checkAdminExecutor(address account) internal view {
if (account != _ADMIN_EXECUTOR) {
revert InvalidAdminExecutor(account);
function _checkSenderIsAdminExecutor() internal view {
if (msg.sender != _ADMIN_EXECUTOR) {
revert InvalidAdminExecutor(msg.sender);
}
}
}
12 changes: 6 additions & 6 deletions contracts/Escrow.sol
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ contract Escrow is IEscrow {
if (address(this) == _SELF) {
revert NonProxyCallsForbidden();
}
_checkDualGovernance(msg.sender);
_checkSenderIsDualGovernance();

_escrowState.initialize(minAssetsLockDuration);

Expand Down Expand Up @@ -243,7 +243,7 @@ contract Escrow is IEscrow {
// ---

function startRageQuit(Duration rageQuitExtensionDelay, Duration rageQuitWithdrawalsTimelock) external {
_checkDualGovernance(msg.sender);
_checkSenderIsDualGovernance();
_escrowState.startRageQuit(rageQuitExtensionDelay, rageQuitWithdrawalsTimelock);
_batchesQueue.open();
}
Expand Down Expand Up @@ -316,7 +316,7 @@ contract Escrow is IEscrow {
// ---

function setMinAssetsLockDuration(Duration newMinAssetsLockDuration) external {
_checkDualGovernance(msg.sender);
_checkSenderIsDualGovernance();
_escrowState.setMinAssetsLockDuration(newMinAssetsLockDuration);
}

Expand Down Expand Up @@ -422,9 +422,9 @@ contract Escrow is IEscrow {
}
}

function _checkDualGovernance(address account) internal view {
if (account != address(DUAL_GOVERNANCE)) {
revert InvalidDualGovernance(account);
function _checkSenderIsDualGovernance() internal view {
if (msg.sender != address(DUAL_GOVERNANCE)) {
revert InvalidDualGovernance(msg.sender);
}
}
}
13 changes: 6 additions & 7 deletions contracts/TimelockedGovernance.sol
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ contract TimelockedGovernance is IGovernance {
/// @param calls An array of ExternalCall structs representing the calls to be executed in the proposal.
/// @return proposalId The ID of the submitted proposal.
function submitProposal(ExternalCall[] calldata calls) external returns (uint256 proposalId) {
_checkGovernance(msg.sender);
_checkSenderIsGovernance();
return TIMELOCK.submit(TIMELOCK.getAdminExecutor(), calls);
}

Expand All @@ -51,15 +51,14 @@ contract TimelockedGovernance is IGovernance {

/// @dev Cancels all pending proposals that have not been executed.
function cancelAllPendingProposals() external {
_checkGovernance(msg.sender);
_checkSenderIsGovernance();
TIMELOCK.cancelAllNonExecutedProposals();
}

/// @dev Checks if the given account is the governance address.
/// @param account The address to check.
function _checkGovernance(address account) internal view {
if (account != GOVERNANCE) {
revert NotGovernance(account);
/// @dev Checks if the msg.sender is the governance address.
function _checkSenderIsGovernance() internal view {
if (msg.sender != GOVERNANCE) {
revert NotGovernance(msg.sender);
}
}
}
14 changes: 6 additions & 8 deletions contracts/libraries/EmergencyProtection.sol
Original file line number Diff line number Diff line change
Expand Up @@ -133,19 +133,17 @@ library EmergencyProtection {

/// @dev Checks if the caller is the emergency activator and reverts if not.
/// @param self The storage reference to the Context struct.
/// @param account The account to check.
function checkEmergencyActivationCommittee(Context storage self, address account) internal view {
if (self.emergencyActivationCommittee != account) {
revert InvalidEmergencyActivationCommittee(account);
function checkSenderIsEmergencyActivationCommittee(Context storage self) internal view {
if (self.emergencyActivationCommittee != msg.sender) {
revert InvalidEmergencyActivationCommittee(msg.sender);
}
}

/// @dev Checks if the caller is the emergency enactor and reverts if not.
/// @param self The storage reference to the Context struct.
/// @param account The account to check.
function checkEmergencyExecutionCommittee(Context storage self, address account) internal view {
if (self.emergencyExecutionCommittee != account) {
revert InvalidEmergencyExecutionCommittee(account);
function checkSenderIsEmergencyExecutionCommittee(Context storage self) internal view {
if (self.emergencyExecutionCommittee != msg.sender) {
revert InvalidEmergencyExecutionCommittee(msg.sender);
}
}

Expand Down
32 changes: 9 additions & 23 deletions contracts/libraries/Proposers.sol
Original file line number Diff line number Diff line change
Expand Up @@ -131,35 +131,21 @@ library Proposers {
return self.executorRefsCounts[account] > 0;
}

/// @dev Checks if an account is a registered proposer and reverts if not.
/// @dev Checks if msg.sender is a registered proposer and reverts if not.
/// @param self The storage state of the Proposers library.
/// @param account The address to check.
function checkProposer(State storage self, address account) internal view {
if (!isProposer(self, account)) {
revert NotProposer(account);
}
}

/// @dev Checks if an account is a registered proposer assigned to the expected executor and reverts if not.
/// @param self The storage state of the Proposers library.
/// @param account The address of the proposer.
/// @param executor The address of the expected executor.
function checkExecutor(State storage self, address account, address executor) internal view {
checkProposer(self, account);
ExecutorData memory executorData = self.executors[account];
if (executor != executorData.executor) {
revert NotAssignedExecutor(account, executor, executorData.executor);
function checkSenderIsProposer(State storage self) internal view {
if (!isProposer(self, msg.sender)) {
revert NotProposer(msg.sender);
}
}

/// @dev Checks if an account is an admin proposer and reverts if not.
/// @dev Checks if msg.sender is an admin proposer and reverts if not.
/// @param self The storage state of the Proposers library.
/// @param adminExecutor The address of the admin executor.
/// @param account The address to check.
function checkAdminProposer(State storage self, address adminExecutor, address account) internal view {
checkProposer(self, account);
if (!isAdminProposer(self, adminExecutor, account)) {
revert NotAdminProposer(account);
function checkSenderIsAdminProposer(State storage self, address adminExecutor) internal view {
checkSenderIsProposer(self);
if (!isAdminProposer(self, adminExecutor, msg.sender)) {
revert NotAdminProposer(msg.sender);
}
}
}
6 changes: 3 additions & 3 deletions contracts/libraries/Tiebreaker.sol
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,9 @@ library Tiebreaker {
// Checks
// ---

function checkTiebreakerCommittee(Context storage self, address account) internal view {
if (account != self.tiebreakerCommittee) {
revert InvalidTiebreakerCommittee(account);
function checkSenderIsTiebreakerCommittee(Context storage self) internal view {
if (msg.sender != self.tiebreakerCommittee) {
revert InvalidTiebreakerCommittee(msg.sender);
}
}

Expand Down
6 changes: 3 additions & 3 deletions contracts/libraries/TimelockState.sol
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,9 @@ library TimelockState {
emit AfterScheduleDelaySet(newAfterScheduleDelay);
}

function checkGovernance(Context storage self, address account) internal view {
if (self.governance != account) {
revert InvalidGovernance(account);
function checkSenderIsGovernance(Context storage self) internal view {
if (self.governance != msg.sender) {
revert InvalidGovernance(msg.sender);
}
}
}
Loading

0 comments on commit 0fcc95d

Please sign in to comment.