From e7870263aba64113967589a16280382f7c339b24 Mon Sep 17 00:00:00 2001 From: Aleksandr Tarelkin Date: Wed, 20 Nov 2024 14:04:21 +0300 Subject: [PATCH 1/2] Additional submitted proposal eventdata --- contracts/DualGovernance.sol | 2 +- contracts/EmergencyProtectedTimelock.sol | 4 +- contracts/TimelockedGovernance.sol | 2 +- contracts/interfaces/ITimelock.sol | 1 + contracts/libraries/ExecutableProposals.sol | 7 ++- docs/specification.md | 2 +- test/mocks/TimelockMock.sol | 7 ++- test/unit/DualGovernance.t.sol | 6 +- test/unit/EmergencyProtectedTimelock.t.sol | 14 ++--- test/unit/libraries/ExecutableProposals.t.sol | 55 ++++++++++--------- 10 files changed, 57 insertions(+), 43 deletions(-) diff --git a/contracts/DualGovernance.sol b/contracts/DualGovernance.sol index 66175aaf..c7d9f123 100644 --- a/contracts/DualGovernance.sol +++ b/contracts/DualGovernance.sol @@ -187,7 +187,7 @@ contract DualGovernance is IDualGovernance { revert ProposalSubmissionBlocked(); } Proposers.Proposer memory proposer = _proposers.getProposer(msg.sender); - proposalId = TIMELOCK.submit(proposer.executor, calls, metadata); + proposalId = TIMELOCK.submit(proposer.account, proposer.executor, calls, metadata); } /// @notice Schedules a previously submitted proposal for execution in the Dual Governance system. diff --git a/contracts/EmergencyProtectedTimelock.sol b/contracts/EmergencyProtectedTimelock.sol index 645eba5d..a9b454c4 100644 --- a/contracts/EmergencyProtectedTimelock.sol +++ b/contracts/EmergencyProtectedTimelock.sol @@ -93,17 +93,19 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock { // --- /// @notice Submits a new proposal to execute a series of calls through an executor. + /// @param proposer The address of the proposer submitting the proposal. /// @param executor The address of the executor contract that will execute the calls. /// @param calls An array of `ExternalCall` structs representing the calls to be executed. /// @param metadata A string containing additional information about the proposal. /// @return newProposalId The id of the newly created proposal. function submit( + address proposer, address executor, ExternalCall[] calldata calls, string calldata metadata ) external returns (uint256 newProposalId) { _timelockState.checkCallerIsGovernance(); - newProposalId = _proposals.submit(executor, calls, metadata); + newProposalId = _proposals.submit(proposer, executor, calls, metadata); } /// @notice Schedules a proposal for execution after a specified delay. diff --git a/contracts/TimelockedGovernance.sol b/contracts/TimelockedGovernance.sol index 6cd4ff68..69abbfe0 100644 --- a/contracts/TimelockedGovernance.sol +++ b/contracts/TimelockedGovernance.sol @@ -30,7 +30,7 @@ contract TimelockedGovernance is IGovernance { string calldata metadata ) external returns (uint256 proposalId) { _checkCallerIsGovernance(); - return TIMELOCK.submit(TIMELOCK.getAdminExecutor(), calls, metadata); + return TIMELOCK.submit(msg.sender, TIMELOCK.getAdminExecutor(), calls, metadata); } /// @dev Schedules a submitted proposal. diff --git a/contracts/interfaces/ITimelock.sol b/contracts/interfaces/ITimelock.sol index 017c2236..99566a01 100644 --- a/contracts/interfaces/ITimelock.sol +++ b/contracts/interfaces/ITimelock.sol @@ -16,6 +16,7 @@ interface ITimelock { } function submit( + address proposer, address executor, ExternalCall[] calldata calls, string calldata metadata diff --git a/contracts/libraries/ExecutableProposals.sol b/contracts/libraries/ExecutableProposals.sol index a1ad977d..8f88900a 100644 --- a/contracts/libraries/ExecutableProposals.sol +++ b/contracts/libraries/ExecutableProposals.sol @@ -67,7 +67,9 @@ library ExecutableProposals { error AfterSubmitDelayNotPassed(uint256 proposalId); error AfterScheduleDelayNotPassed(uint256 proposalId); - event ProposalSubmitted(uint256 indexed id, address indexed executor, ExternalCall[] calls, string metadata); + event ProposalSubmitted( + uint256 indexed id, address indexed proposer, address indexed executor, ExternalCall[] calls, string metadata + ); event ProposalScheduled(uint256 indexed id); event ProposalExecuted(uint256 indexed id, bytes[] callResults); event ProposalsCancelledTill(uint256 proposalId); @@ -84,6 +86,7 @@ library ExecutableProposals { function submit( Context storage self, + address proposer, address executor, ExternalCall[] memory calls, string memory metadata @@ -105,7 +108,7 @@ library ExecutableProposals { newProposal.calls.push(calls[i]); } - emit ProposalSubmitted(newProposalId, executor, calls, metadata); + emit ProposalSubmitted(newProposalId, proposer, executor, calls, metadata); } function schedule(Context storage self, uint256 proposalId, Duration afterSubmitDelay) internal { diff --git a/docs/specification.md b/docs/specification.md index 47a55d05..255b0443 100644 --- a/docs/specification.md +++ b/docs/specification.md @@ -950,7 +950,7 @@ The governance reset entails the following steps: ### Function: EmergencyProtectedTimelock.submit ```solidity -function submit(address executor, ExecutorCall[] calls, string calldata metadata) +function submit(address proposer, address executor, ExecutorCall[] calls, string calldata metadata) returns (uint256 proposalId) ``` diff --git a/test/mocks/TimelockMock.sol b/test/mocks/TimelockMock.sol index 543fd6e8..32dd2c30 100644 --- a/test/mocks/TimelockMock.sol +++ b/test/mocks/TimelockMock.sol @@ -24,7 +24,12 @@ contract TimelockMock is ITimelock { address internal governance; - function submit(address, ExternalCall[] calldata, string calldata) external returns (uint256 newProposalId) { + function submit( + address, + address, + ExternalCall[] calldata, + string calldata + ) external returns (uint256 newProposalId) { newProposalId = submittedProposals.length + OFFSET; submittedProposals.push(newProposalId); canScheduleProposal[newProposalId] = false; diff --git a/test/unit/DualGovernance.t.sol b/test/unit/DualGovernance.t.sol index 36a6601c..a980f760 100644 --- a/test/unit/DualGovernance.t.sol +++ b/test/unit/DualGovernance.t.sol @@ -107,7 +107,9 @@ contract DualGovernanceUnitTests is UnitTest { ExternalCall[] memory calls = _generateExternalCalls(); Proposers.Proposer memory proposer = _dualGovernance.getProposer(address(this)); vm.expectCall( - address(_timelock), 0, abi.encodeWithSelector(TimelockMock.submit.selector, proposer.executor, calls, "") + address(_timelock), + 0, + abi.encodeWithSelector(TimelockMock.submit.selector, address(this), proposer.executor, calls, "") ); uint256 proposalId = _dualGovernance.submitProposal(calls, ""); @@ -2068,7 +2070,7 @@ contract DualGovernanceUnitTests is UnitTest { function _submitMockProposal() internal { // mock timelock doesn't uses proposal data - _timelock.submit(address(0), new ExternalCall[](0), ""); + _timelock.submit(msg.sender, address(0), new ExternalCall[](0), ""); } function _generateExternalCalls() internal pure returns (ExternalCall[] memory calls) { diff --git a/test/unit/EmergencyProtectedTimelock.t.sol b/test/unit/EmergencyProtectedTimelock.t.sol index d5827462..2d789990 100644 --- a/test/unit/EmergencyProtectedTimelock.t.sol +++ b/test/unit/EmergencyProtectedTimelock.t.sol @@ -77,13 +77,13 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest { vm.prank(stranger); vm.expectRevert(abi.encodeWithSelector(TimelockState.CallerIsNotGovernance.selector, [stranger])); - _timelock.submit(_adminExecutor, new ExternalCall[](0), ""); + _timelock.submit(stranger, _adminExecutor, new ExternalCall[](0), ""); assertEq(_timelock.getProposalsCount(), 0); } function test_submit_HappyPath() external { vm.prank(_dualGovernance); - _timelock.submit(_adminExecutor, _getMockTargetRegularStaffCalls(address(_targetMock)), ""); + _timelock.submit(_dualGovernance, _adminExecutor, _getMockTargetRegularStaffCalls(address(_targetMock)), ""); assertEq(_timelock.getProposalsCount(), 1); @@ -357,7 +357,7 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest { function test_emergencyExecute_RevertOn_ModeNotActive() external { vm.startPrank(_dualGovernance); - _timelock.submit(_adminExecutor, _getMockTargetRegularStaffCalls(address(_targetMock)), ""); + _timelock.submit(_dualGovernance, _adminExecutor, _getMockTargetRegularStaffCalls(address(_targetMock)), ""); assertEq(_timelock.getProposalsCount(), 1); @@ -808,8 +808,8 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest { vm.startPrank(_dualGovernance); ExternalCall[] memory executorCalls = _getMockTargetRegularStaffCalls(address(_targetMock)); - _timelock.submit(_adminExecutor, executorCalls, ""); - _timelock.submit(_adminExecutor, executorCalls, ""); + _timelock.submit(_dualGovernance, _adminExecutor, executorCalls, ""); + _timelock.submit(_dualGovernance, _adminExecutor, executorCalls, ""); (ITimelock.ProposalDetails memory submittedProposal, ExternalCall[] memory calls) = _timelock.getProposal(1); @@ -966,7 +966,7 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest { function test_getProposalCalls() external { ExternalCall[] memory executorCalls = _getMockTargetRegularStaffCalls(address(_targetMock)); vm.prank(_dualGovernance); - _timelock.submit(_adminExecutor, executorCalls, ""); + _timelock.submit(_dualGovernance, _adminExecutor, executorCalls, ""); ExternalCall[] memory calls = _timelock.getProposalCalls(1); @@ -994,7 +994,7 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest { function _submitProposal() internal { vm.prank(_dualGovernance); - _timelock.submit(_adminExecutor, _getMockTargetRegularStaffCalls(address(_targetMock)), ""); + _timelock.submit(_dualGovernance, _adminExecutor, _getMockTargetRegularStaffCalls(address(_targetMock)), ""); } function _scheduleProposal(uint256 proposalId) internal { diff --git a/test/unit/libraries/ExecutableProposals.t.sol b/test/unit/libraries/ExecutableProposals.t.sol index 30dc25fd..05836f3a 100644 --- a/test/unit/libraries/ExecutableProposals.t.sol +++ b/test/unit/libraries/ExecutableProposals.t.sol @@ -21,6 +21,7 @@ contract ExecutableProposalsUnitTests is UnitTest { Executor private _executor; TargetMock private _targetMock; ExecutableProposals.Context internal _proposals; + address proposer = makeAddr("PROPOSER"); uint256 private constant PROPOSAL_ID_OFFSET = 1; @@ -31,7 +32,7 @@ contract ExecutableProposalsUnitTests is UnitTest { function test_submit_reverts_if_empty_proposals() external { vm.expectRevert(ExecutableProposals.EmptyCalls.selector); - _proposals.submit(address(0), new ExternalCall[](0), "Empty calls"); + _proposals.submit(proposer, address(0), new ExternalCall[](0), "Empty calls"); } function test_submit_proposal() external { @@ -43,11 +44,11 @@ contract ExecutableProposalsUnitTests is UnitTest { string memory description = "Regular staff calls"; vm.expectEmit(); - emit ExecutableProposals.ProposalSubmitted(expectedProposalId, address(_executor), calls, description); + emit ExecutableProposals.ProposalSubmitted(expectedProposalId, proposer, address(_executor), calls, description); vm.recordLogs(); - _proposals.submit(address(_executor), calls, description); + _proposals.submit(proposer, address(_executor), calls, description); Vm.Log[] memory entries = vm.getRecordedLogs(); assertEq(entries.length, 1); @@ -73,7 +74,7 @@ contract ExecutableProposalsUnitTests is UnitTest { function testFuzz_schedule_proposal(Duration delay) external { vm.assume(delay > Durations.ZERO && delay <= Durations.MAX); - _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); + _proposals.submit(proposer, address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); uint256 expectedProposalId = 1; ExecutableProposals.Proposal memory proposal = _proposals.proposals[expectedProposalId]; @@ -105,7 +106,7 @@ contract ExecutableProposalsUnitTests is UnitTest { } function test_cannot_schedule_proposal_twice() external { - _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); + _proposals.submit(proposer, address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); uint256 proposalId = 1; _proposals.schedule(proposalId, Durations.ZERO); @@ -116,7 +117,7 @@ contract ExecutableProposalsUnitTests is UnitTest { function testFuzz_cannot_schedule_proposal_before_delay_passed(Duration delay) external { vm.assume(delay > Durations.ZERO && delay <= Durations.MAX); - _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); + _proposals.submit(proposer, address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); _wait(delay.minusSeconds(1 seconds)); @@ -127,7 +128,7 @@ contract ExecutableProposalsUnitTests is UnitTest { } function test_cannot_schedule_cancelled_proposal() external { - _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); + _proposals.submit(proposer, address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); _proposals.cancelAll(); uint256 proposalId = _proposals.getProposalsCount(); @@ -139,7 +140,7 @@ contract ExecutableProposalsUnitTests is UnitTest { function testFuzz_execute_proposal(Duration delay) external { vm.assume(delay > Durations.ZERO && delay <= Durations.MAX); - _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); + _proposals.submit(proposer, address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); uint256 proposalId = _proposals.getProposalsCount(); _proposals.schedule(proposalId, Durations.ZERO); @@ -175,7 +176,7 @@ contract ExecutableProposalsUnitTests is UnitTest { } function test_cannot_execute_unscheduled_proposal() external { - _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); + _proposals.submit(proposer, address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); uint256 proposalId = _proposals.getProposalsCount(); vm.expectRevert(abi.encodeWithSelector(ExecutableProposals.ProposalNotScheduled.selector, proposalId)); @@ -183,7 +184,7 @@ contract ExecutableProposalsUnitTests is UnitTest { } function test_cannot_execute_twice() external { - _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); + _proposals.submit(proposer, address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); uint256 proposalId = _proposals.getProposalsCount(); _proposals.schedule(proposalId, Durations.ZERO); _proposals.execute(proposalId, Durations.ZERO); @@ -193,7 +194,7 @@ contract ExecutableProposalsUnitTests is UnitTest { } function test_cannot_execute_cancelled_proposal() external { - _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); + _proposals.submit(proposer, address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); uint256 proposalId = _proposals.getProposalsCount(); _proposals.schedule(proposalId, Durations.ZERO); _proposals.cancelAll(); @@ -204,7 +205,7 @@ contract ExecutableProposalsUnitTests is UnitTest { function testFuzz_cannot_execute_before_delay_passed(Duration delay) external { vm.assume(delay > Durations.ZERO && delay <= Durations.MAX); - _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); + _proposals.submit(proposer, address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); uint256 proposalId = _proposals.getProposalsCount(); _proposals.schedule(proposalId, Durations.ZERO); @@ -215,8 +216,8 @@ contract ExecutableProposalsUnitTests is UnitTest { } function test_cancel_all_proposals() external { - _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); - _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); + _proposals.submit(proposer, address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); + _proposals.submit(proposer, address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); uint256 proposalsCount = _proposals.getProposalsCount(); @@ -232,7 +233,7 @@ contract ExecutableProposalsUnitTests is UnitTest { // TODO: change this test completely to use getters function test_get_proposal_info_and_external_calls() external { ExternalCall[] memory expectedCalls = _getMockTargetRegularStaffCalls(address(_targetMock)); - _proposals.submit(address(_executor), expectedCalls, ""); + _proposals.submit(proposer, address(_executor), expectedCalls, ""); uint256 proposalId = _proposals.getProposalsCount(); ITimelock.ProposalDetails memory proposalDetails = _proposals.getProposalDetails(proposalId); @@ -294,7 +295,7 @@ contract ExecutableProposalsUnitTests is UnitTest { function test_get_cancelled_proposal() external { ExternalCall[] memory expectedCalls = _getMockTargetRegularStaffCalls(address(_targetMock)); - _proposals.submit(address(_executor), expectedCalls, ""); + _proposals.submit(proposer, address(_executor), expectedCalls, ""); uint256 proposalId = _proposals.getProposalsCount(); ITimelock.ProposalDetails memory proposalDetails = _proposals.getProposalDetails(proposalId); @@ -345,16 +346,16 @@ contract ExecutableProposalsUnitTests is UnitTest { function test_count_proposals() external { assertEq(_proposals.getProposalsCount(), 0); - _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); + _proposals.submit(proposer, address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); assertEq(_proposals.getProposalsCount(), 1); - _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); + _proposals.submit(proposer, address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); assertEq(_proposals.getProposalsCount(), 2); _proposals.schedule(1, Durations.ZERO); assertEq(_proposals.getProposalsCount(), 2); - _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); + _proposals.submit(proposer, address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); assertEq(_proposals.getProposalsCount(), 3); _proposals.schedule(2, Durations.ZERO); @@ -363,7 +364,7 @@ contract ExecutableProposalsUnitTests is UnitTest { _proposals.execute(1, Durations.ZERO); assertEq(_proposals.getProposalsCount(), 3); - _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); + _proposals.submit(proposer, address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); assertEq(_proposals.getProposalsCount(), 4); _proposals.cancelAll(); @@ -372,7 +373,7 @@ contract ExecutableProposalsUnitTests is UnitTest { function test_can_execute_proposal() external { Duration delay = Durations.from(100 seconds); - _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); + _proposals.submit(proposer, address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); uint256 proposalId = _proposals.getProposalsCount(); assert(!_proposals.canExecute(proposalId, Durations.ZERO)); @@ -391,7 +392,7 @@ contract ExecutableProposalsUnitTests is UnitTest { } function test_can_not_execute_cancelled_proposal() external { - _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); + _proposals.submit(proposer, address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); uint256 proposalId = _proposals.getProposalsCount(); _proposals.schedule(proposalId, Durations.ZERO); @@ -402,18 +403,18 @@ contract ExecutableProposalsUnitTests is UnitTest { } function test_cancelAll_DoesNotModifyStateOfExecutedProposals() external { - _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); + _proposals.submit(proposer, address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); assertEq(_proposals.getProposalsCount(), 1); uint256 executedProposalId = 1; _proposals.schedule(executedProposalId, Durations.ZERO); _proposals.execute(executedProposalId, Durations.ZERO); - _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); + _proposals.submit(proposer, address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); assertEq(_proposals.getProposalsCount(), 2); uint256 scheduledProposalId = 2; _proposals.schedule(scheduledProposalId, Durations.ZERO); - _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); + _proposals.submit(proposer, address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); assertEq(_proposals.getProposalsCount(), 3); uint256 submittedProposalId = 3; @@ -435,7 +436,7 @@ contract ExecutableProposalsUnitTests is UnitTest { function test_can_schedule_proposal() external { Duration delay = Durations.from(100 seconds); - _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); + _proposals.submit(proposer, address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); uint256 proposalId = _proposals.getProposalsCount(); assert(!_proposals.canSchedule(proposalId, delay)); @@ -450,7 +451,7 @@ contract ExecutableProposalsUnitTests is UnitTest { } function test_can_not_schedule_cancelled_proposal() external { - _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); + _proposals.submit(proposer, address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); uint256 proposalId = _proposals.getProposalsCount(); assert(_proposals.canSchedule(proposalId, Durations.ZERO)); From dd7f80ce93c07e9f6217febbf6672a0f4d0885df Mon Sep 17 00:00:00 2001 From: Aleksandr Tarelkin Date: Wed, 27 Nov 2024 13:15:50 +0300 Subject: [PATCH 2/2] review fixes --- contracts/libraries/ExecutableProposals.sol | 1 + docs/plan-b.md | 2 +- test/unit/EmergencyProtectedTimelock.t.sol | 22 ++++++++++++++----- test/unit/libraries/ExecutableProposals.t.sol | 2 -- 4 files changed, 19 insertions(+), 8 deletions(-) diff --git a/contracts/libraries/ExecutableProposals.sol b/contracts/libraries/ExecutableProposals.sol index 68767cde..5d0c47f9 100644 --- a/contracts/libraries/ExecutableProposals.sol +++ b/contracts/libraries/ExecutableProposals.sol @@ -104,6 +104,7 @@ library ExecutableProposals { /// @notice Submits a new proposal with the specified executor and external calls. /// @param self The context of the Executable Proposal library. + /// @param proposer The address of the proposer submitting the proposal. /// @param executor The address authorized to execute the proposal. /// @param calls The list of external calls to include in the proposal. /// @param metadata Metadata describing the proposal. diff --git a/docs/plan-b.md b/docs/plan-b.md index ca81d4bf..09e01a5f 100644 --- a/docs/plan-b.md +++ b/docs/plan-b.md @@ -138,7 +138,7 @@ If the Emergency Committees are set up and active, the governance proposal under While active, the Emergency Activation Committee can enable Emergency Mode. This mode prohibits anyone but the Emergency Execution Committee from executing proposals. Once the **Emergency Duration** has ended, the Emergency Execution Committee or anyone else may disable the emergency mode, canceling all pending proposals. After the emergency mode is deactivated or the Emergency Period has elapsed, the Emergency Committees lose their power. ### Function: `EmergencyProtectedTimelock.submit` ```solidity -function submit(address executor, ExecutorCall[] calls, string metadata) +function submit(address proposer, address executor, ExecutorCall[] calls, string metadata) returns (uint256 proposalId) ``` Registers a new governance proposal composed of one or more external `calls` to be made by the `executor` contract. Initiates the `AfterSubmitDelay`. diff --git a/test/unit/EmergencyProtectedTimelock.t.sol b/test/unit/EmergencyProtectedTimelock.t.sol index 2d789990..6340fe42 100644 --- a/test/unit/EmergencyProtectedTimelock.t.sol +++ b/test/unit/EmergencyProtectedTimelock.t.sol @@ -14,6 +14,7 @@ import {EmergencyProtection} from "contracts/libraries/EmergencyProtection.sol"; import {Executor} from "contracts/Executor.sol"; import {EmergencyProtectedTimelock, TimelockState} from "contracts/EmergencyProtectedTimelock.sol"; +import {ExecutableProposals} from "contracts/libraries/ExecutableProposals.sol"; import {UnitTest} from "test/utils/unit-test.sol"; import {TargetMock} from "test/utils/target-mock.sol"; @@ -22,6 +23,7 @@ import {ExternalCall} from "test/utils/executor-calls.sol"; contract EmergencyProtectedTimelockUnitTests is UnitTest { EmergencyProtectedTimelock private _timelock; TargetMock private _targetMock; + TargetMock private _anotherTargetMock; Executor private _executor; address private _emergencyActivator = makeAddr("EMERGENCY_ACTIVATION_COMMITTEE"); @@ -40,6 +42,7 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest { _timelock = _deployEmergencyProtectedTimelock(); _targetMock = new TargetMock(); + _anotherTargetMock = new TargetMock(); _executor.transferOwnership(address(_timelock)); @@ -82,8 +85,16 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest { } function test_submit_HappyPath() external { + string memory testMetadata = "testMetadata"; + + vm.expectEmit(true, true, true, true); + emit ExecutableProposals.ProposalSubmitted( + 1, _dualGovernance, _adminExecutor, _getMockTargetRegularStaffCalls(address(_targetMock)), testMetadata + ); vm.prank(_dualGovernance); - _timelock.submit(_dualGovernance, _adminExecutor, _getMockTargetRegularStaffCalls(address(_targetMock)), ""); + _timelock.submit( + _dualGovernance, _adminExecutor, _getMockTargetRegularStaffCalls(address(_targetMock)), testMetadata + ); assertEq(_timelock.getProposalsCount(), 1); @@ -808,8 +819,9 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest { vm.startPrank(_dualGovernance); ExternalCall[] memory executorCalls = _getMockTargetRegularStaffCalls(address(_targetMock)); + ExternalCall[] memory anotherExecutorCalls = _getMockTargetRegularStaffCalls(address(_anotherTargetMock)); _timelock.submit(_dualGovernance, _adminExecutor, executorCalls, ""); - _timelock.submit(_dualGovernance, _adminExecutor, executorCalls, ""); + _timelock.submit(_dualGovernance, _adminExecutor, anotherExecutorCalls, ""); (ITimelock.ProposalDetails memory submittedProposal, ExternalCall[] memory calls) = _timelock.getProposal(1); @@ -876,9 +888,9 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest { // assertEq(cancelledProposal.executedAt, Timestamps.ZERO); // assertEq doesn't support comparing enumerables so far assertEq(cancelledCalls.length, 1); - assertEq(cancelledCalls[0].value, executorCalls[0].value); - assertEq(cancelledCalls[0].target, executorCalls[0].target); - assertEq(cancelledCalls[0].payload, executorCalls[0].payload); + assertEq(cancelledCalls[0].value, anotherExecutorCalls[0].value); + assertEq(cancelledCalls[0].target, anotherExecutorCalls[0].target); + assertEq(cancelledCalls[0].payload, anotherExecutorCalls[0].payload); } function test_get_not_existing_proposal() external { diff --git a/test/unit/libraries/ExecutableProposals.t.sol b/test/unit/libraries/ExecutableProposals.t.sol index 929cb365..d95c328a 100644 --- a/test/unit/libraries/ExecutableProposals.t.sol +++ b/test/unit/libraries/ExecutableProposals.t.sol @@ -204,8 +204,6 @@ contract ExecutableProposalsUnitTests is UnitTest { } function testFuzz_cannot_execute_before_delay_passed(Duration delay) external { - vm.assume(delay > Durations.ZERO && delay <= Durations.MAX); - _proposals.submit(proposer, address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); vm.assume(delay > Durations.ZERO && delay.toSeconds() <= MAX_DURATION_VALUE); _proposals.submit(proposer, address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); uint256 proposalId = _proposals.getProposalsCount();