Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename checks checkSenderIs #81

Merged
merged 2 commits into from
Aug 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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