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

Refactor events #231

Merged
merged 4 commits into from
Dec 6, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
4 changes: 3 additions & 1 deletion contracts/DualGovernance.sol
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,9 @@ contract DualGovernance is IDualGovernance {
revert ProposalSubmissionBlocked();
}
Proposers.Proposer memory proposer = _proposers.getProposer(msg.sender);
proposalId = TIMELOCK.submit(proposer.account, proposer.executor, calls, metadata);
proposalId = TIMELOCK.submit(proposer.executor, calls);

emit ProposalReported(proposer.account, proposalId, metadata);
}

/// @notice Schedules a previously submitted proposal for execution in the Dual Governance system.
Expand Down
11 changes: 2 additions & 9 deletions contracts/EmergencyProtectedTimelock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -102,19 +102,12 @@ 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) {
function submit(address executor, ExternalCall[] calldata calls) external returns (uint256 newProposalId) {
_timelockState.checkCallerIsGovernance();
newProposalId = _proposals.submit(proposer, executor, calls, metadata);
newProposalId = _proposals.submit(executor, calls);
}

/// @notice Schedules a proposal for execution after a specified delay.
Expand Down
11 changes: 7 additions & 4 deletions contracts/Executor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ contract Executor is IExternalExecutor, Ownable {
// Events
// ---

event Execute(address indexed sender, address indexed target, uint256 ethValue, bytes data);
event ETHReceived(address sender, uint256 value);
event Executed(address indexed target, uint256 ethValue, bytes data, bytes returndata);

// ---
// Constructor
Expand All @@ -32,10 +33,12 @@ contract Executor is IExternalExecutor, Ownable {
/// @param payload The calldata for the function call.
function execute(address target, uint256 value, bytes calldata payload) external payable {
_checkOwner();
Address.functionCallWithValue(target, payload, value);
emit Execute(msg.sender, target, value, payload);
bytes memory returndata = Address.functionCallWithValue(target, payload, value);
emit Executed(target, value, payload, returndata);
}

/// @notice Allows the contract to receive ether.
receive() external payable {}
receive() external payable {
emit ETHReceived(msg.sender, msg.value);
}
}
3 changes: 2 additions & 1 deletion contracts/TimelockedGovernance.sol
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ contract TimelockedGovernance is IGovernance {
string calldata metadata
) external returns (uint256 proposalId) {
_checkCallerIsGovernance();
return TIMELOCK.submit(msg.sender, TIMELOCK.getAdminExecutor(), calls, metadata);
proposalId = TIMELOCK.submit(TIMELOCK.getAdminExecutor(), calls);
emit ProposalReported(msg.sender, proposalId, metadata);
}

/// @notice Schedules a submitted proposal.
Expand Down
2 changes: 2 additions & 0 deletions contracts/interfaces/IGovernance.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import {ITimelock} from "./ITimelock.sol";
import {ExternalCall} from "../libraries/ExternalCalls.sol";

interface IGovernance {
event ProposalReported(address indexed proposerAccount, uint256 indexed proposalId, string metadata);
Psirex marked this conversation as resolved.
Show resolved Hide resolved

function TIMELOCK() external view returns (ITimelock);
function submitProposal(
ExternalCall[] calldata calls,
Expand Down
7 changes: 1 addition & 6 deletions contracts/interfaces/ITimelock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,7 @@ interface ITimelock {
ProposalStatus status;
}

function submit(
address proposer,
address executor,
ExternalCall[] calldata calls,
string calldata metadata
) external returns (uint256 newProposalId);
function submit(address executor, ExternalCall[] calldata calls) external returns (uint256 newProposalId);
function schedule(uint256 proposalId) external;
function execute(uint256 proposalId) external;
function cancelAllNonExecutedProposals() external;
Expand Down
12 changes: 3 additions & 9 deletions contracts/libraries/ExecutableProposals.sol
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,7 @@ library ExecutableProposals {
// Events
// ---

event ProposalSubmitted(
uint256 indexed id, address indexed proposer, address indexed executor, ExternalCall[] calls, string metadata
);
event ProposalSubmitted(uint256 indexed id, address indexed executor, ExternalCall[] calls);
event ProposalScheduled(uint256 indexed id);
event ProposalExecuted(uint256 indexed id);
event ProposalsCancelledTill(uint256 proposalId);
Expand All @@ -102,17 +100,13 @@ 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.
/// @return newProposalId The id of the newly submitted proposal.
function submit(
Context storage self,
address proposer,
address executor,
ExternalCall[] memory calls,
string memory metadata
ExternalCall[] memory calls
) internal returns (uint256 newProposalId) {
if (calls.length == 0) {
revert EmptyCalls();
Expand All @@ -131,7 +125,7 @@ library ExecutableProposals {
newProposal.calls.push(calls[i]);
}

emit ProposalSubmitted(newProposalId, proposer, executor, calls, metadata);
emit ProposalSubmitted(newProposalId, executor, calls);
}

/// @notice Marks a previously submitted proposal as scheduled for execution if the required delay period
Expand Down
7 changes: 1 addition & 6 deletions test/mocks/TimelockMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,7 @@ contract TimelockMock is ITimelock {

address internal governance;

function submit(
address,
address,
ExternalCall[] calldata,
string calldata
) external returns (uint256 newProposalId) {
function submit(address, ExternalCall[] calldata) external returns (uint256 newProposalId) {
newProposalId = submittedProposals.length + OFFSET;
submittedProposals.push(newProposalId);
canScheduleProposal[newProposalId] = false;
Expand Down
26 changes: 15 additions & 11 deletions test/unit/DualGovernance.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ import {Tiebreaker} from "contracts/libraries/Tiebreaker.sol";
import {Resealer} from "contracts/libraries/Resealer.sol";
import {Status as ProposalStatus} from "contracts/libraries/ExecutableProposals.sol";
import {Proposers} from "contracts/libraries/Proposers.sol";

import {IGovernance} from "contracts/interfaces/IGovernance.sol";
import {IResealManager} from "contracts/interfaces/IResealManager.sol";

import {
DualGovernanceConfig,
IDualGovernanceConfigProvider,
Expand Down Expand Up @@ -191,13 +194,15 @@ contract DualGovernanceUnitTests is UnitTest {
function test_submitProposal_HappyPath() external {
ExternalCall[] memory calls = _generateExternalCalls();
Proposers.Proposer memory proposer = _dualGovernance.getProposer(address(this));
vm.expectCall(
address(_timelock),
0,
abi.encodeWithSelector(TimelockMock.submit.selector, address(this), proposer.executor, calls, "")
);
vm.expectCall(address(_timelock), 0, abi.encodeCall(TimelockMock.submit, (proposer.executor, calls)));

uint256 expectedProposalId = 1;
string memory metadata = "New proposal description";

uint256 proposalId = _dualGovernance.submitProposal(calls, "");
vm.expectEmit();
emit IGovernance.ProposalReported(proposer.account, expectedProposalId, metadata);

uint256 proposalId = _dualGovernance.submitProposal(calls, metadata);
uint256[] memory submittedProposals = _timelock.getSubmittedProposals();

assertEq(submittedProposals.length, 1);
Expand Down Expand Up @@ -1073,11 +1078,11 @@ contract DualGovernanceUnitTests is UnitTest {
vm.expectEmit();
emit DualGovernanceStateMachine.ConfigProviderSet(IDualGovernanceConfigProvider(address(newConfigProvider)));
vm.expectEmit();
emit Executor.Execute(
address(this),
emit Executor.Executed(
address(_dualGovernance),
0,
abi.encodeWithSelector(DualGovernance.setConfigProvider.selector, address(newConfigProvider))
abi.encodeWithSelector(DualGovernance.setConfigProvider.selector, address(newConfigProvider)),
new bytes(0)
);

vm.expectCall(
Expand Down Expand Up @@ -2384,8 +2389,7 @@ contract DualGovernanceUnitTests is UnitTest {
// ---

function _submitMockProposal() internal {
// mock timelock doesn't uses proposal data
_timelock.submit(msg.sender, address(0), new ExternalCall[](0), "");
_timelock.submit(address(0), new ExternalCall[](0));
}

function _scheduleProposal(uint256 proposalId, Timestamp submittedAt) internal {
Expand Down
20 changes: 9 additions & 11 deletions test/unit/EmergencyProtectedTimelock.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -186,21 +186,19 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest {

vm.prank(stranger);
vm.expectRevert(abi.encodeWithSelector(TimelockState.CallerIsNotGovernance.selector, [stranger]));
_timelock.submit(stranger, _adminExecutor, new ExternalCall[](0), "");
_timelock.submit(_adminExecutor, new ExternalCall[](0));
assertEq(_timelock.getProposalsCount(), 0);
}

function test_submit_HappyPath() external {
string memory testMetadata = "testMetadata";

vm.expectEmit(true, true, true, true);
vm.expectEmit();
emit ExecutableProposals.ProposalSubmitted(
1, _dualGovernance, _adminExecutor, _getMockTargetRegularStaffCalls(address(_targetMock)), testMetadata
1, _adminExecutor, _getMockTargetRegularStaffCalls(address(_targetMock))
);
vm.prank(_dualGovernance);
_timelock.submit(
_dualGovernance, _adminExecutor, _getMockTargetRegularStaffCalls(address(_targetMock)), testMetadata
);
_timelock.submit(_adminExecutor, _getMockTargetRegularStaffCalls(address(_targetMock)));

assertEq(_timelock.getProposalsCount(), 1);

Expand Down Expand Up @@ -593,7 +591,7 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest {

function test_emergencyExecute_RevertOn_ModeNotActive() external {
vm.startPrank(_dualGovernance);
_timelock.submit(_dualGovernance, _adminExecutor, _getMockTargetRegularStaffCalls(address(_targetMock)), "");
_timelock.submit(_adminExecutor, _getMockTargetRegularStaffCalls(address(_targetMock)));

assertEq(_timelock.getProposalsCount(), 1);

Expand Down Expand Up @@ -1045,8 +1043,8 @@ 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, anotherExecutorCalls, "");
_timelock.submit(_adminExecutor, executorCalls);
_timelock.submit(_adminExecutor, anotherExecutorCalls);

(ITimelock.ProposalDetails memory submittedProposal, ExternalCall[] memory calls) = _timelock.getProposal(1);

Expand Down Expand Up @@ -1203,7 +1201,7 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest {
function test_getProposalCalls() external {
ExternalCall[] memory executorCalls = _getMockTargetRegularStaffCalls(address(_targetMock));
vm.prank(_dualGovernance);
_timelock.submit(_dualGovernance, _adminExecutor, executorCalls, "");
_timelock.submit(_adminExecutor, executorCalls);

ExternalCall[] memory calls = _timelock.getProposalCalls(1);

Expand Down Expand Up @@ -1254,7 +1252,7 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest {

function _submitProposal() internal {
vm.prank(_dualGovernance);
_timelock.submit(_dualGovernance, _adminExecutor, _getMockTargetRegularStaffCalls(address(_targetMock)), "");
_timelock.submit(_adminExecutor, _getMockTargetRegularStaffCalls(address(_targetMock)));
}

function _scheduleProposal(uint256 proposalId) internal {
Expand Down
Loading
Loading