Skip to content

Commit

Permalink
Merge pull request #136 from lidofinance/feature/change-variables-dg-ept
Browse files Browse the repository at this point in the history 
Change variables DG EPT
  • Loading branch information
@Psirex
Psirex authored Oct 23, 2024
2 parents f0312a1 + ec68145 commit b9b0ae6
Show file tree
Hide file tree
Showing 7 changed files with 242 additions and 20 deletions.
41 changes: 33 additions & 8 deletions contracts/DualGovernance.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ contract DualGovernance is IDualGovernance {
error ProposalSubmissionBlocked();
error ProposalSchedulingBlocked(uint256 proposalId);
error ResealIsNotAllowedInNormalState();
error InvalidResealManager(address resealManager);
error InvalidResealCommittee(address resealCommittee);

// ---
// Events
Expand All @@ -52,6 +54,7 @@ contract DualGovernance is IDualGovernance {
event CancelAllPendingProposalsExecuted();
event EscrowMasterCopyDeployed(IEscrow escrowMasterCopy);
event ResealCommitteeSet(address resealCommittee);
event ResealManagerSet(address resealManager);

// ---
// Sanity Check Parameters & Immutables
Expand Down Expand Up @@ -110,9 +113,6 @@ contract DualGovernance is IDualGovernance {
/// @notice The address of the Timelock contract.
ITimelock public immutable TIMELOCK;

/// @notice The address of the Reseal Manager.
IResealManager public immutable RESEAL_MANAGER;

/// @notice The address of the Escrow contract used as the implementation for the Signalling and Rage Quit
/// instances of the Escrows managed by the DualGovernance contract.
IEscrow public immutable ESCROW_MASTER_COPY;
Expand All @@ -138,13 +138,15 @@ contract DualGovernance is IDualGovernance {
/// period of time when the Dual Governance proposal adoption is blocked.
address internal _resealCommittee;

/// @dev The address of the Reseal Manager.
IResealManager internal _resealManager;

// ---
// Constructor
// ---

constructor(ExternalDependencies memory dependencies, SanityCheckParams memory sanityCheckParams) {
TIMELOCK = dependencies.timelock;
RESEAL_MANAGER = dependencies.resealManager;

MIN_TIEBREAKER_ACTIVATION_TIMEOUT = sanityCheckParams.minTiebreakerActivationTimeout;
MAX_TIEBREAKER_ACTIVATION_TIMEOUT = sanityCheckParams.maxTiebreakerActivationTimeout;
Expand All @@ -157,10 +159,10 @@ contract DualGovernance is IDualGovernance {
withdrawalQueue: dependencies.withdrawalQueue,
minWithdrawalsBatchSize: sanityCheckParams.minWithdrawalsBatchSize
});

emit EscrowMasterCopyDeployed(ESCROW_MASTER_COPY);

_stateMachine.initialize(dependencies.configProvider, ESCROW_MASTER_COPY);
_setResealManager(address(dependencies.resealManager));
}

// ---
Expand Down Expand Up @@ -444,7 +446,7 @@ contract DualGovernance is IDualGovernance {
_tiebreaker.checkCallerIsTiebreakerCommittee();
_stateMachine.activateNextState(ESCROW_MASTER_COPY);
_tiebreaker.checkTie(_stateMachine.getPersistedState(), _stateMachine.normalOrVetoCooldownExitedAt);
RESEAL_MANAGER.resume(sealable);
_resealManager.resume(sealable);
}

/// @notice Allows the tiebreaker committee to schedule for execution a submitted proposal when
Expand Down Expand Up @@ -484,22 +486,45 @@ contract DualGovernance is IDualGovernance {
if (_stateMachine.getPersistedState() == State.Normal) {
revert ResealIsNotAllowedInNormalState();
}
RESEAL_MANAGER.reseal(sealable);
_resealManager.reseal(sealable);
}

/// @notice Sets the address of the reseal committee.
/// @param resealCommittee The address of the new reseal committee.
function setResealCommittee(address resealCommittee) external {
_checkCallerIsAdminExecutor();
if (resealCommittee == _resealCommittee) {
revert InvalidResealCommittee(resealCommittee);
}
_resealCommittee = resealCommittee;

emit ResealCommitteeSet(resealCommittee);
}

/// @notice Sets the address of the Reseal Manager.
/// @param resealManager The address of the new Reseal Manager.
function setResealManager(address resealManager) external {
_checkCallerIsAdminExecutor();
_setResealManager(resealManager);
}

/// @notice Gets the address of the Reseal Manager.
/// @return resealManager The address of the Reseal Manager.
function getResealManager() external view returns (IResealManager) {
return _resealManager;
}

// ---
// Internal methods
// ---

function _setResealManager(address resealManager) internal {
if (resealManager == address(_resealManager) || resealManager == address(0)) {
revert InvalidResealManager(resealManager);
}
_resealManager = IResealManager(resealManager);
emit ResealManagerSet(resealManager);
}

function _checkCallerIsAdminExecutor() internal view {
if (TIMELOCK.getAdminExecutor() != msg.sender) {
revert CallerIsNotAdminExecutor(msg.sender);
Expand Down
26 changes: 15 additions & 11 deletions contracts/EmergencyProtectedTimelock.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,6 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock {
/// @notice The upper bound for the time the emergency protection mechanism can be activated.
Duration public immutable MAX_EMERGENCY_PROTECTION_DURATION;

// ---
// Admin Executor Immutables
// ---

/// @dev The address of the admin executor, authorized to manage the EmergencyProtectedTimelock instance.
address private immutable _ADMIN_EXECUTOR;

// ---
// Aspects
// ---
Expand All @@ -80,12 +73,12 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock {
// ---

constructor(SanityCheckParams memory sanityCheckParams, address adminExecutor) {
_ADMIN_EXECUTOR = adminExecutor;

MAX_AFTER_SUBMIT_DELAY = sanityCheckParams.maxAfterSubmitDelay;
MAX_AFTER_SCHEDULE_DELAY = sanityCheckParams.maxAfterScheduleDelay;
MAX_EMERGENCY_MODE_DURATION = sanityCheckParams.maxEmergencyModeDuration;
MAX_EMERGENCY_PROTECTION_DURATION = sanityCheckParams.maxEmergencyProtectionDuration;

_timelockState.setAdminExecutor(adminExecutor);
}

// ---
Expand Down Expand Up @@ -279,7 +272,7 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock {
/// @notice Returns the address of the admin executor.
/// @return adminExecutor The address of the admin executor.
function getAdminExecutor() external view returns (address) {
return _ADMIN_EXECUTOR;
return _timelockState.adminExecutor;
}

/// @notice Returns the configured delay duration required before a submitted proposal can be scheduled.
Expand Down Expand Up @@ -354,12 +347,23 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock {
return _proposals.canSchedule(proposalId, _timelockState.getAfterSubmitDelay());
}

// ---
// Admin Executor Methods
// ---

/// @notice Sets the address of the admin executor.
/// @param newAdminExecutor The address of the new admin executor.
function setAdminExecutor(address newAdminExecutor) external {
_checkCallerIsAdminExecutor();
_timelockState.setAdminExecutor(newAdminExecutor);
}

// ---
// Internal Methods
// ---

function _checkCallerIsAdminExecutor() internal view {
if (msg.sender != _ADMIN_EXECUTOR) {
if (msg.sender != _timelockState.adminExecutor) {
revert CallerIsNotAdminExecutor(msg.sender);
}
}
Expand Down
16 changes: 16 additions & 0 deletions contracts/libraries/TimelockState.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@ library TimelockState {
error InvalidGovernance(address value);
error InvalidAfterSubmitDelay(Duration value);
error InvalidAfterScheduleDelay(Duration value);
error InvalidAdminExecutor(address adminExecutor);

event GovernanceSet(address newGovernance);
event AfterSubmitDelaySet(Duration newAfterSubmitDelay);
event AfterScheduleDelaySet(Duration newAfterScheduleDelay);
event AdminExecutorSet(address newAdminExecutor);

struct Context {
/// @dev slot0 [0..159]
Expand All @@ -22,6 +24,8 @@ library TimelockState {
Duration afterSubmitDelay;
/// @dev slot0 [192..224]
Duration afterScheduleDelay;
/// @dev slot1 [0..159]
address adminExecutor;
}

/// @notice Sets the governance address.
Expand Down Expand Up @@ -70,6 +74,18 @@ library TimelockState {
emit AfterScheduleDelaySet(newAfterScheduleDelay);
}

/// @notice Sets the admin executor address.
/// @dev Reverts if the new admin executor address is zero or the same as the current one.
/// @param self The context of the timelock state.
/// @param newAdminExecutor The new admin executor address.
function setAdminExecutor(Context storage self, address newAdminExecutor) internal {
if (newAdminExecutor == address(0) || newAdminExecutor == self.adminExecutor) {
revert InvalidAdminExecutor(newAdminExecutor);
}
self.adminExecutor = newAdminExecutor;
emit AdminExecutorSet(newAdminExecutor);
}

/// @notice Gets the after submit delay.
/// @param self The context of the timelock state.
/// @return The current after submit delay.
Expand Down
118 changes: 118 additions & 0 deletions test/unit/DualGovernance.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,14 @@ import {IWithdrawalQueue} from "contracts/interfaces/IWithdrawalQueue.sol";
import {ITimelock} from "contracts/interfaces/ITimelock.sol";
import {ISealable} from "contracts/interfaces/ISealable.sol";
import {ITiebreaker} from "contracts/interfaces/ITiebreaker.sol";
import {IEscrow} from "contracts/interfaces/IEscrow.sol";

import {UnitTest} from "test/utils/unit-test.sol";
import {StETHMock} from "test/mocks/StETHMock.sol";
import {TimelockMock} from "test/mocks/TimelockMock.sol";
import {WithdrawalQueueMock} from "test/mocks/WithdrawalQueueMock.sol";
import {SealableMock} from "test/mocks/SealableMock.sol";
import {computeAddress} from "test/utils/addresses.sol";

contract DualGovernanceUnitTests is UnitTest {
Executor private _executor = new Executor(address(this));
Expand Down Expand Up @@ -99,6 +101,50 @@ contract DualGovernanceUnitTests is UnitTest {
_STETH_MOCK.approve(address(_escrow), 10 ether);
}

// ---
// constructor()
// ---

function test_constructor_HappyPath() external {
address testDeployerAddress = address(this);
uint256 testDeployerNonce = vm.getNonce(testDeployerAddress);
address predictedDualGovernanceAddress = computeAddress(testDeployerAddress, testDeployerNonce);

address predictedEscrowCopyAddress = computeAddress(predictedDualGovernanceAddress, 1);

vm.expectEmit();
emit DualGovernance.EscrowMasterCopyDeployed(IEscrow(predictedEscrowCopyAddress));
vm.expectEmit();
emit DualGovernance.ResealManagerSet(address(_RESEAL_MANAGER_STUB));

Duration minTiebreakerActivationTimeout = Durations.from(30 days);
Duration maxTiebreakerActivationTimeout = Durations.from(180 days);
uint256 maxSealableWithdrawalBlockersCount = 128;

DualGovernance dualGovernanceLocal = new DualGovernance({
dependencies: DualGovernance.ExternalDependencies({
stETH: _STETH_MOCK,
wstETH: _WSTETH_STUB,
withdrawalQueue: _WITHDRAWAL_QUEUE_MOCK,
timelock: _timelock,
resealManager: _RESEAL_MANAGER_STUB,
configProvider: _configProvider
}),
sanityCheckParams: DualGovernance.SanityCheckParams({
minWithdrawalsBatchSize: 4,
minTiebreakerActivationTimeout: minTiebreakerActivationTimeout,
maxTiebreakerActivationTimeout: maxTiebreakerActivationTimeout,
maxSealableWithdrawalBlockersCount: maxSealableWithdrawalBlockersCount
})
});

assertEq(address(dualGovernanceLocal.TIMELOCK()), address(_timelock));
assertEq(dualGovernanceLocal.MIN_TIEBREAKER_ACTIVATION_TIMEOUT(), minTiebreakerActivationTimeout);
assertEq(dualGovernanceLocal.MAX_TIEBREAKER_ACTIVATION_TIMEOUT(), maxTiebreakerActivationTimeout);
assertEq(dualGovernanceLocal.MAX_SEALABLE_WITHDRAWAL_BLOCKERS_COUNT(), maxSealableWithdrawalBlockersCount);
assertEq(address(dualGovernanceLocal.ESCROW_MASTER_COPY()), predictedEscrowCopyAddress);
}

// ---
// submitProposal()
// ---
Expand Down Expand Up @@ -2062,6 +2108,78 @@ contract DualGovernanceUnitTests is UnitTest {
_dualGovernance.setResealCommittee(makeAddr("NEW_RESEAL_COMMITTEE"));
}

function testFuzz_setResealCommittee_RevertOn_InvalidResealCommittee(address newResealCommittee) external {
_executor.execute(
address(_dualGovernance),
0,
abi.encodeWithSelector(DualGovernance.setResealCommittee.selector, newResealCommittee)
);

vm.expectRevert(abi.encodeWithSelector(DualGovernance.InvalidResealCommittee.selector, newResealCommittee));
_executor.execute(
address(_dualGovernance),
0,
abi.encodeWithSelector(DualGovernance.setResealCommittee.selector, newResealCommittee)
);
}

// ---
// setResealManager()
// ---

function testFuzz_setResealManager_HappyPath(address newResealManager) external {
vm.assume(newResealManager != address(0) && newResealManager != address(_RESEAL_MANAGER_STUB));

vm.expectEmit();
emit DualGovernance.ResealManagerSet(newResealManager);

_executor.execute(
address(_dualGovernance),
0,
abi.encodeWithSelector(DualGovernance.setResealManager.selector, newResealManager)
);
}

function test_setResealManager_RevertOn_InvalidAddress() external {
vm.expectRevert(
abi.encodeWithSelector(DualGovernance.InvalidResealManager.selector, address(_RESEAL_MANAGER_STUB))
);
_executor.execute(
address(_dualGovernance),
0,
abi.encodeWithSelector(DualGovernance.setResealManager.selector, address(_RESEAL_MANAGER_STUB))
);

vm.expectRevert(abi.encodeWithSelector(DualGovernance.InvalidResealManager.selector, address(0)));
_executor.execute(
address(_dualGovernance), 0, abi.encodeWithSelector(DualGovernance.setResealManager.selector, address(0))
);
}

function test_setResealManger_RevertOn_CallerIsNotAdminExecutor(address stranger) external {
vm.assume(stranger != address(_executor));

vm.prank(stranger);
vm.expectRevert(abi.encodeWithSelector(DualGovernance.CallerIsNotAdminExecutor.selector, stranger));
_dualGovernance.setResealManager(address(0x123));
}

// ---
// getResealManager()
// ---

function testFuzz_getResealManager_HappyPath(address newResealManager) external {
vm.assume(newResealManager != address(_RESEAL_MANAGER_STUB));

_executor.execute(
address(_dualGovernance),
0,
abi.encodeWithSelector(DualGovernance.setResealManager.selector, newResealManager)
);

assertEq(newResealManager, address(_dualGovernance.getResealManager()));
}

// ---
// Helper methods
// ---
Expand Down
18 changes: 17 additions & 1 deletion test/unit/EmergencyProtectedTimelock.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest {
EmergencyProtectedTimelock.SanityCheckParams memory sanityCheckParams,
address adminExecutor
) external {
vm.assume(adminExecutor != address(0));
EmergencyProtectedTimelock timelock = new EmergencyProtectedTimelock(sanityCheckParams, adminExecutor);

assertEq(timelock.getAdminExecutor(), adminExecutor);

assertEq(timelock.MAX_AFTER_SUBMIT_DELAY(), sanityCheckParams.maxAfterSubmitDelay);
assertEq(timelock.MAX_AFTER_SCHEDULE_DELAY(), sanityCheckParams.maxAfterScheduleDelay);
assertEq(timelock.MAX_EMERGENCY_MODE_DURATION(), sanityCheckParams.maxEmergencyModeDuration);
Expand Down Expand Up @@ -990,6 +990,22 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest {
assertEq(timelock.getAdminExecutor(), executor);
}

function testFuzz_setAdminExecutor_HappyPath(address adminExecutor) external {
vm.assume(adminExecutor != _adminExecutor && adminExecutor != address(0));
vm.prank(_adminExecutor);
_timelock.setAdminExecutor(adminExecutor);

assertEq(_timelock.getAdminExecutor(), adminExecutor);
}

function test_setAdminExecutor_RevertOn_NotAdminExecutor(address stranger) external {
vm.assume(stranger != _adminExecutor);

vm.prank(stranger);
vm.expectRevert(abi.encodeWithSelector(EmergencyProtectedTimelock.CallerIsNotAdminExecutor.selector, stranger));
_timelock.setAdminExecutor(address(0x123));
}

// Utils

function _submitProposal() internal {
Expand Down
Loading

0 comments on commit b9b0ae6

Please sign in to comment.