Skip to content

Commit

Permalink
feat: error cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
@rkolpakov
rkolpakov committed Aug 8, 2024
1 parent 578b3b9 commit 2f2a0b6
Show file tree
Hide file tree
Showing 24 changed files with 124 additions and 126 deletions.
34 changes: 17 additions & 17 deletions contracts/DualGovernance.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ contract DualGovernance is IDualGovernance {
// Errors
// ---

error CallerIsNotResealCommittee(address caller);
error CallerIsNotAdminExecutor(address caller);
error InvalidConfigProvider(IDualGovernanceConfigProvider configProvider);
error NotResealCommittee(address account);
error ProposalSubmissionBlocked();
error InvalidAdminExecutor(address value);
error ProposalSchedulingBlocked(uint256 proposalId);
error ResealIsNotAllowedInNormalState();

Expand Down 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.checkSenderIsProposer();
_proposers.checkCallerIsProposer();
if (!_stateMachine.canSubmitProposal()) {
revert ProposalSubmissionBlocked();
}
Expand All @@ -123,7 +123,7 @@ contract DualGovernance is IDualGovernance {
}

function cancelAllPendingProposals() external {
_proposers.checkSenderIsAdminProposer(TIMELOCK.getAdminExecutor());
_proposers.checkCallerIsAdminProposer(TIMELOCK.getAdminExecutor());
TIMELOCK.cancelAllNonExecutedProposals();
}

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

function setConfigProvider(IDualGovernanceConfigProvider newConfigProvider) external {
_checkSenderIsAdminExecutor();
_checkCallerIsAdminExecutor();
_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 {
_checkSenderIsAdminExecutor();
_checkCallerIsAdminExecutor();
_proposers.register(proposer, executor);
}

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

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

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

function removeTiebreakerSealableWithdrawalBlocker(address sealableWithdrawalBlocker) external {
_checkSenderIsAdminExecutor();
_checkCallerIsAdminExecutor();
_tiebreaker.removeSealableWithdrawalBlocker(sealableWithdrawalBlocker);
}

function setTiebreakerCommittee(address tiebreakerCommittee) external {
_checkSenderIsAdminExecutor();
_checkCallerIsAdminExecutor();
_tiebreaker.setTiebreakerCommittee(tiebreakerCommittee);
}

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

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

function tiebreakerScheduleProposal(uint256 proposalId) external {
_tiebreaker.checkSenderIsTiebreakerCommittee();
_tiebreaker.checkCallerIsTiebreakerCommittee();
_tiebreaker.checkTie(_stateMachine.getCurrentState(), _stateMachine.getNormalOrVetoCooldownStateExitedAt());
TIMELOCK.schedule(proposalId);
}
Expand All @@ -268,7 +268,7 @@ contract DualGovernance is IDualGovernance {

function resealSealable(address sealable) external {
if (msg.sender != _resealCommittee) {
revert NotResealCommittee(msg.sender);
revert CallerIsNotResealCommittee(msg.sender);
}
if (_stateMachine.getCurrentState() == State.Normal) {
revert ResealIsNotAllowedInNormalState();
Expand All @@ -277,7 +277,7 @@ contract DualGovernance is IDualGovernance {
}

function setResealCommittee(address resealCommittee) external {
_checkSenderIsAdminExecutor();
_checkCallerIsAdminExecutor();
_resealCommittee = resealCommittee;
}

Expand All @@ -298,9 +298,9 @@ contract DualGovernance is IDualGovernance {
emit ConfigProviderSet(newConfigProvider);
}

function _checkSenderIsAdminExecutor() internal view {
function _checkCallerIsAdminExecutor() internal view {
if (TIMELOCK.getAdminExecutor() != msg.sender) {
revert InvalidAdminExecutor(msg.sender);
revert CallerIsNotAdminExecutor(msg.sender);
}
}
}
28 changes: 14 additions & 14 deletions contracts/EmergencyProtectedTimelock.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ contract EmergencyProtectedTimelock is ITimelock {
using ExecutableProposals for ExecutableProposals.State;
using EmergencyProtection for EmergencyProtection.Context;

error InvalidAdminExecutor(address value);
error CallerIsNotAdminExecutor(address value);

// ---
// Sanity Check Params Immutables
Expand Down 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.checkSenderIsGovernance();
_timelockState.checkCallerIsGovernance();
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.checkSenderIsGovernance();
_timelockState.checkCallerIsGovernance();
_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.checkSenderIsGovernance();
_timelockState.checkCallerIsGovernance();
_proposals.cancelAll();
}

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

function setGovernance(address newGovernance) external {
_checkSenderIsAdminExecutor();
_checkCallerIsAdminExecutor();
_timelockState.setGovernance(newGovernance);
}

function setDelays(Duration afterSubmitDelay, Duration afterScheduleDelay) external {
_checkSenderIsAdminExecutor();
_checkCallerIsAdminExecutor();
_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 {
_checkSenderIsAdminExecutor();
_checkCallerIsAdminExecutor();
IOwnable(executor).transferOwnership(owner);
}

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

_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.checkSenderIsEmergencyActivationCommittee();
_emergencyProtection.checkCallerIsEmergencyActivationCommittee();
_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.checkSenderIsEmergencyExecutionCommittee();
_emergencyProtection.checkCallerIsEmergencyExecutionCommittee();
_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()) {
_checkSenderIsAdminExecutor();
_checkCallerIsAdminExecutor();
}
_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.checkSenderIsEmergencyExecutionCommittee();
_emergencyProtection.checkCallerIsEmergencyExecutionCommittee();
_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 _checkSenderIsAdminExecutor() internal view {
function _checkCallerIsAdminExecutor() internal view {
if (msg.sender != _ADMIN_EXECUTOR) {
revert InvalidAdminExecutor(msg.sender);
revert CallerIsNotAdminExecutor(msg.sender);
}
}
}
12 changes: 6 additions & 6 deletions contracts/Escrow.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ contract Escrow is IEscrow {
error UnexpectedUnstETHId();
error NonProxyCallsForbidden();
error InvalidBatchSize(uint256 size);
error InvalidDualGovernance(address value);
error CallerIsNotGovernance(address caller);
error InvalidHintsLength(uint256 actual, uint256 expected);
error InvalidETHSender(address actual, address expected);

Expand Down Expand Up @@ -124,7 +124,7 @@ contract Escrow is IEscrow {
if (address(this) == _SELF) {
revert NonProxyCallsForbidden();
}
_checkSenderIsDualGovernance();
_checkCallerIsGovernance();

_escrowState.initialize(minAssetsLockDuration);

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

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

function setMinAssetsLockDuration(Duration newMinAssetsLockDuration) external {
_checkSenderIsDualGovernance();
_checkCallerIsGovernance();
_escrowState.setMinAssetsLockDuration(newMinAssetsLockDuration);
}

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

function _checkSenderIsDualGovernance() internal view {
function _checkCallerIsGovernance() internal view {
if (msg.sender != address(DUAL_GOVERNANCE)) {
revert InvalidDualGovernance(msg.sender);
revert CallerIsNotGovernance(msg.sender);
}
}
}
11 changes: 5 additions & 6 deletions contracts/ResealManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ import {IResealManager} from "./interfaces/IResealManager.sol";
/// @dev Allows to extend pause of temporarily paused contracts to permanent pause or resume it.
contract ResealManager is IResealManager {
error SealableWrongPauseState();
error SenderIsNotGovernance();
error NotAllowed();
error CallerIsNotGovernance(address caller);

uint256 public constant PAUSE_INFINITELY = type(uint256).max;
ITimelock public immutable EMERGENCY_PROTECTED_TIMELOCK;
Expand All @@ -31,7 +30,7 @@ contract ResealManager is IResealManager {
/// - Function is called by the governance contract.
/// @param sealable The address of the sealable contract.
function reseal(address sealable) public {
_checkSenderIsGovernance();
_checkCallerIsGovernance();

uint256 sealableResumeSinceTimestamp = ISealable(sealable).getResumeSinceTimestamp();
if (sealableResumeSinceTimestamp < block.timestamp || sealableResumeSinceTimestamp == PAUSE_INFINITELY) {
Expand All @@ -48,7 +47,7 @@ contract ResealManager is IResealManager {
/// - Function is called by the governance contract.
/// @param sealable The address of the sealable contract.
function resume(address sealable) public {
_checkSenderIsGovernance();
_checkCallerIsGovernance();

uint256 sealableResumeSinceTimestamp = ISealable(sealable).getResumeSinceTimestamp();
if (sealableResumeSinceTimestamp < block.timestamp) {
Expand All @@ -59,10 +58,10 @@ contract ResealManager is IResealManager {

/// @notice Ensures that the function can only be called by the governance address.
/// @dev Reverts if the sender is not the governance address.
function _checkSenderIsGovernance() internal view {
function _checkCallerIsGovernance() internal view {
address governance = EMERGENCY_PROTECTED_TIMELOCK.getGovernance();
if (msg.sender != governance) {
revert SenderIsNotGovernance();
revert CallerIsNotGovernance(msg.sender);
}
}
}
10 changes: 5 additions & 5 deletions contracts/TimelockedGovernance.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {ExternalCall} from "./libraries/ExternalCalls.sol";
/// @title TimelockedGovernance
/// @dev A contract that serves as the interface for submitting and scheduling the execution of governance proposals.
contract TimelockedGovernance is IGovernance {
error NotGovernance(address account);
error CallerIsNotGovernance(address caller);

address public immutable GOVERNANCE;
ITimelock public immutable TIMELOCK;
Expand All @@ -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) {
_checkSenderIsGovernance();
_checkCallerIsGovernance();
return TIMELOCK.submit(TIMELOCK.getAdminExecutor(), calls);
}

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

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

/// @dev Checks if the msg.sender is the governance address.
function _checkSenderIsGovernance() internal view {
function _checkCallerIsGovernance() internal view {
if (msg.sender != GOVERNANCE) {
revert NotGovernance(msg.sender);
revert CallerIsNotGovernance(msg.sender);
}
}
}
2 changes: 1 addition & 1 deletion contracts/committees/EmergencyActivationCommittee.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ contract EmergencyActivationCommittee is HashConsensus {
/// @notice Approves the emergency activation by casting a vote
/// @dev Only callable by committee members
function approveActivateEmergencyMode() public {
_checkSenderIsMember();
_checkCallerIsMember();
_vote(EMERGENCY_ACTIVATION_HASH, true);
}

Expand Down
4 changes: 2 additions & 2 deletions contracts/committees/EmergencyExecutionCommittee.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ contract EmergencyExecutionCommittee is HashConsensus, ProposalsList {
/// @param proposalId The ID of the proposal to vote on
/// @param _supports Indicates whether the member supports the proposal execution
function voteEmergencyExecute(uint256 proposalId, bool _supports) public {
_checkSenderIsMember();
_checkCallerIsMember();
(bytes memory proposalData, bytes32 key) = _encodeEmergencyExecute(proposalId);
_vote(key, _supports);
_pushProposal(key, uint256(ProposalType.EmergencyExecute), proposalData);
Expand Down Expand Up @@ -85,7 +85,7 @@ contract EmergencyExecutionCommittee is HashConsensus, ProposalsList {
/// @notice Approves an emergency reset proposal
/// @dev Only callable by committee members
function approveEmergencyReset() public {
_checkSenderIsMember();
_checkCallerIsMember();
bytes32 proposalKey = _encodeEmergencyResetProposalKey();
_vote(proposalKey, true);
_pushProposal(proposalKey, uint256(ProposalType.EmergencyReset), bytes(""));
Expand Down
Loading

0 comments on commit 2f2a0b6

Please sign in to comment.