diff --git a/packages/contracts-bedrock/interfaces/governance/IProposalValidator.sol b/packages/contracts-bedrock/interfaces/governance/IProposalValidator.sol index caaf75aee424b..4dc0cd45ad123 100644 --- a/packages/contracts-bedrock/interfaces/governance/IProposalValidator.sol +++ b/packages/contracts-bedrock/interfaces/governance/IProposalValidator.sol @@ -9,17 +9,15 @@ import {IOptimismGovernor} from "./IOptimismGovernor.sol"; interface IProposalValidator { error ProposalValidator_InsufficientApprovals(); error ProposalValidator_ProposalAlreadyApproved(); - error ProposalValidator_ProposalAlreadyInVoting(); + error ProposalValidator_ProposalAlreadySubmitted(); error ProposalValidator_InsufficientVotingPower(); error ProposalValidator_InvalidAttestation(); + error ProposalValidator_ProposalDoesNotExist(); struct ProposalData { address proposer; - address[] targets; - uint256[] values; - bytes[] calldatas; - string description; ProposalType proposalType; + uint8 proposalTypeConfigurator; bool inVoting; mapping(address => bool) delegateApprovals; uint256 remainingApprovalsRequired; @@ -40,22 +38,23 @@ interface IProposalValidator { } event ProposalSubmitted( - uint256 indexed proposalId, + bytes32 indexed proposalHash, address indexed proposer, address[] targets, uint256[] values, bytes[] calldatas, string description, - ProposalType proposalType + ProposalType proposalType, + uint8 proposalTypeConfigurator ); event ProposalApproved( - uint256 indexed proposalId, + bytes32 indexed proposalHash, address indexed approver ); event ProposalMovedToVote( - uint256 indexed proposalId, + bytes32 indexed proposalHash, address indexed executor ); @@ -75,12 +74,18 @@ interface IProposalValidator { bytes[] memory _calldatas, string memory _description, ProposalType _proposalType, + uint8 _proposalTypeConfigurator, bytes32 _attestationUid - ) external returns (uint256 proposalId_); + ) external returns (bytes32 proposalHash_); - function approveProposal(uint256 _proposalId) external; + function approveProposal(bytes32 _proposalHash) external; - function moveToVote(uint256 _proposalId) external returns (uint256 governorProposalId_); + function moveToVote( + address[] memory _targets, + uint256[] memory _values, + bytes[] memory _calldatas, + string memory _description + ) external returns (uint256 governorProposalId_); function setMinimumVotingPower(uint256 _minimumVotingPower) external; diff --git a/packages/contracts-bedrock/snapshots/abi/ProposalValidator.json b/packages/contracts-bedrock/snapshots/abi/ProposalValidator.json index 8044503e6fc49..bdd01c8d1d936 100644 --- a/packages/contracts-bedrock/snapshots/abi/ProposalValidator.json +++ b/packages/contracts-bedrock/snapshots/abi/ProposalValidator.json @@ -114,9 +114,9 @@ { "inputs": [ { - "internalType": "uint256", - "name": "_proposalId", - "type": "uint256" + "internalType": "bytes32", + "name": "_proposalHash", + "type": "bytes32" } ], "name": "approveProposal", @@ -172,9 +172,24 @@ { "inputs": [ { - "internalType": "uint256", - "name": "_proposalId", - "type": "uint256" + "internalType": "address[]", + "name": "_targets", + "type": "address[]" + }, + { + "internalType": "uint256[]", + "name": "_values", + "type": "uint256[]" + }, + { + "internalType": "bytes[]", + "name": "_calldatas", + "type": "bytes[]" + }, + { + "internalType": "string", + "name": "_description", + "type": "string" } ], "name": "moveToVote", @@ -292,6 +307,11 @@ "name": "_proposalType", "type": "uint8" }, + { + "internalType": "uint8", + "name": "_proposalTypeConfigurator", + "type": "uint8" + }, { "internalType": "bytes32", "name": "_attestationUid", @@ -301,9 +321,9 @@ "name": "submitProposal", "outputs": [ { - "internalType": "uint256", - "name": "proposalId_", - "type": "uint256" + "internalType": "bytes32", + "name": "proposalHash_", + "type": "bytes32" } ], "stateMutability": "nonpayable", @@ -404,9 +424,9 @@ "inputs": [ { "indexed": true, - "internalType": "uint256", - "name": "proposalId", - "type": "uint256" + "internalType": "bytes32", + "name": "proposalHash", + "type": "bytes32" }, { "indexed": true, @@ -423,9 +443,9 @@ "inputs": [ { "indexed": true, - "internalType": "uint256", - "name": "proposalId", - "type": "uint256" + "internalType": "bytes32", + "name": "proposalHash", + "type": "bytes32" }, { "indexed": true, @@ -442,9 +462,9 @@ "inputs": [ { "indexed": true, - "internalType": "uint256", - "name": "proposalId", - "type": "uint256" + "internalType": "bytes32", + "name": "proposalHash", + "type": "bytes32" }, { "indexed": true, @@ -481,6 +501,12 @@ "internalType": "enum ProposalValidator.ProposalType", "name": "proposalType", "type": "uint8" + }, + { + "indexed": false, + "internalType": "uint8", + "name": "proposalTypeConfigurator", + "type": "uint8" } ], "name": "ProposalSubmitted", @@ -521,7 +547,12 @@ }, { "inputs": [], - "name": "ProposalValidator_ProposalAlreadyInVoting", + "name": "ProposalValidator_ProposalAlreadySubmitted", + "type": "error" + }, + { + "inputs": [], + "name": "ProposalValidator_ProposalDoesNotExist", "type": "error" } ] \ No newline at end of file diff --git a/packages/contracts-bedrock/snapshots/storageLayout/ProposalValidator.json b/packages/contracts-bedrock/snapshots/storageLayout/ProposalValidator.json index 4050cff3575aa..f7fdeefae8ced 100644 --- a/packages/contracts-bedrock/snapshots/storageLayout/ProposalValidator.json +++ b/packages/contracts-bedrock/snapshots/storageLayout/ProposalValidator.json @@ -46,13 +46,6 @@ "label": "_proposals", "offset": 0, "slot": "6", - "type": "mapping(uint256 => struct ProposalValidator.ProposalData)" - }, - { - "bytes": "32", - "label": "_proposalCounter", - "offset": 0, - "slot": "7", - "type": "uint256" + "type": "mapping(bytes32 => struct ProposalValidator.ProposalData)" } ] \ No newline at end of file diff --git a/packages/contracts-bedrock/src/governance/ProposalValidator.sol b/packages/contracts-bedrock/src/governance/ProposalValidator.sol index 0be3d7dbe1113..6661bc17030f8 100644 --- a/packages/contracts-bedrock/src/governance/ProposalValidator.sol +++ b/packages/contracts-bedrock/src/governance/ProposalValidator.sol @@ -22,11 +22,12 @@ contract ProposalValidator is Ownable { /// @notice Thrown when a proposal doesn't have enough delegate approvals to move to vote. error ProposalValidator_InsufficientApprovals(); + /// @notice Thrown when a delegate attempts to approve a proposal they've already approved. error ProposalValidator_ProposalAlreadyApproved(); /// @notice Thrown when attempting to move a proposal to vote that is already in voting. - error ProposalValidator_ProposalAlreadyInVoting(); + error ProposalValidator_ProposalAlreadySubmitted(); /// @notice Thrown when a delegate has insufficient voting power to approve a proposal. error ProposalValidator_InsufficientVotingPower(); @@ -34,27 +35,24 @@ contract ProposalValidator is Ownable { /// @notice Thrown when an invalid attestation is provided for a proposal. error ProposalValidator_InvalidAttestation(); + /// @notice Thrown when a proposal does not exist. + error ProposalValidator_ProposalDoesNotExist(); + /*////////////////////////////////////////////////////////////// STRUCTS //////////////////////////////////////////////////////////////*/ /// @notice Data structure for storing proposal information. /// @param proposer The address that submitted the proposal. - /// @param targets Target addresses for proposal calls. - /// @param values ETH values for proposal calls. - /// @param calldatas Function data for proposal calls. - /// @param description Description of the proposal. /// @param proposalType Type of the proposal from the ProposalType enum. + /// @param proposalTypeConfigurator Configuration value specific to the proposal type. /// @param inVoting Whether the proposal has been moved to the voting phase. /// @param delegateApprovals Mapping of delegate addresses to their approval status. - /// @param remainingApprovalsRequired Number of approvals still needed before being able to move for voting. + /// @param remainingApprovalsRequired Number of approvals still needed before voting. struct ProposalData { address proposer; - address[] targets; - uint256[] values; - bytes[] calldatas; - string description; ProposalType proposalType; + uint8 proposalTypeConfigurator; bool inVoting; mapping(address => bool) delegateApprovals; uint256 remainingApprovalsRequired; @@ -93,32 +91,34 @@ contract ProposalValidator is Ownable { //////////////////////////////////////////////////////////////*/ /// @notice Emitted when a new proposal is submitted to the validator contract. - /// @param proposalId The ID of the submitted proposal. + /// @param proposalHash The hash of the submitted proposal. /// @param proposer The address that submitted the proposal. /// @param targets Target addresses for proposal calls. /// @param values ETH values for proposal calls. /// @param calldatas Function data for proposal calls. /// @param description Description of the proposal. /// @param proposalType Type of the proposal. + /// @param proposalTypeConfigurator Configuration value specific to the proposal type. event ProposalSubmitted( - uint256 indexed proposalId, + bytes32 indexed proposalHash, address indexed proposer, address[] targets, uint256[] values, bytes[] calldatas, string description, - ProposalType proposalType + ProposalType proposalType, + uint8 proposalTypeConfigurator ); /// @notice Emitted when a delegate approves a proposal. - /// @param proposalId The ID of the approved proposal. + /// @param proposalHash The hash of the approved proposal. /// @param approver The address of the delegate who approved the proposal. - event ProposalApproved(uint256 indexed proposalId, address indexed approver); + event ProposalApproved(bytes32 indexed proposalHash, address indexed approver); /// @notice Emitted when a proposal is moved to the voting phase in the governor contract. - /// @param proposalId The ID of the proposal moved to vote. + /// @param proposalHash The hash of the proposal moved to vote. /// @param executor The address that executed the move to vote. - event ProposalMovedToVote(uint256 indexed proposalId, address indexed executor); + event ProposalMovedToVote(bytes32 indexed proposalHash, address indexed executor); /// @notice Emitted when the minimum voting power is set. /// @param newMinimumVotingPower The new minimum voting power. @@ -162,11 +162,8 @@ contract ProposalValidator is Ownable { /// @notice The immutable data for each proposal type. mapping(ProposalType => ImmutableProposalTypeData) private _proposalTypeData; - /// @notice Mapping of proposal IDs to their corresponding proposal data. - mapping(uint256 => ProposalData) private _proposals; - - /// @notice Counter for generating unique proposal IDs. - uint256 private _proposalCounter; + /// @notice Mapping of proposal hash to their corresponding proposal data. + mapping(bytes32 => ProposalData) private _proposals; /// @notice Initializes the ProposalValidator contract. /// @param _owner The address that will own the contract. @@ -206,52 +203,60 @@ contract ProposalValidator is Ownable { } } - /// @notice Submit a proposal for delegate approval. - /// @param _targets Target addresses for proposal calls. - /// @param _values ETH values for proposal calls. - /// @param _calldatas Function data for proposal calls. - /// @param _description Description of the proposal. - /// @param _proposalType Type of the proposal. - /// @param _attestationUid The UID of the attestation proving eligibility. - /// @return proposalId_ The ID of the submitted proposal. + /// @notice Submit a proposal for delegate approval + /// @param _targets Target addresses for proposal calls + /// @param _values ETH values for proposal calls + /// @param _calldatas Function data for proposal calls + /// @param _description Description of the proposal + /// @param _proposalType Type of the proposal + /// @return proposalHash_ The hash of the submitted proposal function submitProposal( address[] memory _targets, uint256[] memory _values, bytes[] memory _calldatas, string memory _description, ProposalType _proposalType, + uint8 _proposalTypeConfigurator, bytes32 _attestationUid ) external - returns (uint256 proposalId_) + returns (bytes32 proposalHash_) { _validateProposal(_targets, _values, _calldatas, _proposalType, _attestationUid); - proposalId_ = ++_proposalCounter; + proposalHash_ = _hashProposal(_targets, _values, _calldatas, _description); + ProposalData storage proposal = _proposals[proposalHash_]; + + if (proposal.proposer != address(0)) { + revert ProposalValidator_ProposalAlreadySubmitted(); + } - ProposalData storage proposal = _proposals[proposalId_]; proposal.proposer = msg.sender; - proposal.targets = _targets; - proposal.values = _values; - proposal.calldatas = _calldatas; - proposal.description = _description; proposal.proposalType = _proposalType; + proposal.proposalTypeConfigurator = _proposalTypeConfigurator; proposal.inVoting = false; proposal.remainingApprovalsRequired = 4; // Hardcoded for now, will change with proposalTypes - emit ProposalSubmitted(proposalId_, msg.sender, _targets, _values, _calldatas, _description, _proposalType); - - return proposalId_; + emit ProposalSubmitted( + proposalHash_, + msg.sender, + _targets, + _values, + _calldatas, + _description, + _proposalType, + _proposalTypeConfigurator + ); } - /// @notice Approve a proposal (only callable by delegates with sufficient voting power). - /// @param _proposalId The ID of the proposal to approve. - function approveProposal(uint256 _proposalId) external { + /// @notice Approve a proposal (only callable by delegates with sufficient voting power) + /// @param _proposalHash The hash of the proposal to approve + function approveProposal(bytes32 _proposalHash) external { if (!canSignOff(msg.sender)) { revert ProposalValidator_InsufficientVotingPower(); } - ProposalData storage proposal = _proposals[_proposalId]; + ProposalData storage proposal = _proposals[_proposalHash]; if (proposal.delegateApprovals[msg.sender]) { revert ProposalValidator_ProposalAlreadyApproved(); @@ -260,40 +265,54 @@ contract ProposalValidator is Ownable { proposal.delegateApprovals[msg.sender] = true; proposal.remainingApprovalsRequired--; // Expected overflow when all approvals are granted - emit ProposalApproved(_proposalId, msg.sender); + emit ProposalApproved(_proposalHash, msg.sender); } - /// @notice Move a proposal to voting phase after sufficient delegate approvals. - /// @dev After passing all checks, the proposal is submitted with a external call to the governor contract. - /// @param _proposalId The ID of the proposal to move to vote. - /// @return governorProposalId_ The proposal ID in the governor contract. - function moveToVote(uint256 _proposalId) external returns (uint256 governorProposalId_) { - ProposalData storage proposal = _proposals[_proposalId]; + /// @notice Move a proposal to voting phase after sufficient delegate approvals + /// @param _targets Target addresses for proposal calls + /// @param _values ETH values for proposal calls + /// @param _calldatas Function data for proposal calls + /// @param _description Description of the proposal + /// @return governorProposalId_ The proposal ID in the governor contract + function moveToVote( + address[] memory _targets, + uint256[] memory _values, + bytes[] memory _calldatas, + string memory _description + ) + external + returns (uint256 governorProposalId_) + { + // Verify that the provided data matches the proposalHash + bytes32 _proposalHash = _hashProposal(_targets, _values, _calldatas, _description); + + ProposalData storage proposal = _proposals[_proposalHash]; + + if (proposal.proposer == address(0)) { + revert ProposalValidator_ProposalDoesNotExist(); + } if (proposal.remainingApprovalsRequired > 0) { revert ProposalValidator_InsufficientApprovals(); } if (proposal.inVoting) { - revert ProposalValidator_ProposalAlreadyInVoting(); + revert ProposalValidator_ProposalAlreadySubmitted(); } proposal.inVoting = true; - governorProposalId_ = GOVERNOR.propose( - proposal.targets, proposal.values, proposal.calldatas, proposal.description, uint8(proposal.proposalType) - ); - - emit ProposalMovedToVote(_proposalId, msg.sender); + governorProposalId_ = + GOVERNOR.propose(_targets, _values, _calldatas, _description, proposal.proposalTypeConfigurator); - return governorProposalId_; + emit ProposalMovedToVote(_proposalHash, msg.sender); } /// @notice Returns whether a delegate has enough voting power to approve a proposal. /// @param _delegate The address of the delegate to check. /// @return canSignOff_ True if the delegate has sufficient voting power, false otherwise. function canSignOff(address _delegate) public view returns (bool canSignOff_) { - return VOTING_TOKEN.balanceOf(_delegate) >= minimumVotingPower; + canSignOff_ = VOTING_TOKEN.balanceOf(_delegate) >= minimumVotingPower; } /// @notice Sets the minimum voting power required for a delegate to approve proposals. @@ -371,7 +390,20 @@ contract ProposalValidator is Ownable { returns (bool isValid_) { (address approvedDelegate, uint8 proposalType) = abi.decode(_data, (address, uint8)); - return approvedDelegate == msg.sender && proposalType == uint8(_expectedProposalType); + isValid_ = approvedDelegate == msg.sender && proposalType == uint8(_expectedProposalType); + } + + function _hashProposal( + address[] memory _targets, + uint256[] memory _values, + bytes[] memory _calldatas, + string memory _description + ) + internal + pure + returns (bytes32 proposalHash_) + { + return keccak256(abi.encode(_targets, _values, _calldatas, _description)); } /// @notice Private function to set the minimum voting power and emit event. diff --git a/packages/contracts-bedrock/test/governance/ProposalValidator.t.sol b/packages/contracts-bedrock/test/governance/ProposalValidator.t.sol index 7e9c85a18f046..b58c5c7995be4 100644 --- a/packages/contracts-bedrock/test/governance/ProposalValidator.t.sol +++ b/packages/contracts-bedrock/test/governance/ProposalValidator.t.sol @@ -35,6 +35,7 @@ contract ProposalValidator_Init is CommonTest { ProposalValidator public validator; IOptimismGovernor public governor; bytes32 public ATTESTATION_SCHEMA_UID; + bytes32 public proposalHash; /// @notice Helper function to setup a mock and expect a call to it. function _mockAndExpect(address _receiver, bytes memory _calldata, bytes memory _returned) internal { @@ -52,9 +53,9 @@ contract ProposalValidator_Init is CommonTest { } /// @notice Helper function to make a (top) delegate approve a proposal. - function _approveProposal(address _delegate, uint256 _proposalId) internal { + function _approveProposal(address _delegate, bytes32 _proposalHash) internal { vm.prank(_delegate); - validator.approveProposal(_proposalId); + validator.approveProposal(_proposalHash); } function _getProposalTypesRequiredApprovalsAndImmutableData() @@ -181,13 +182,16 @@ contract ProposalValidator_SubmitProposal_Test is ProposalValidator_Init { _createProposalSetup(); ProposalValidator.ProposalType proposalType = ProposalValidator.ProposalType.ProtocolOrGovernorUpgrade; + uint8 proposalTypeConfigurator = 0; bytes32 attestationUid = _createAttestation(topDelegate_A, proposalType); + // Submit the proposal vm.prank(topDelegate_A); - uint256 proposalId = - validator.submitProposal(_targets, _values, _calldatas, _description, proposalType, attestationUid); + bytes32 proposalHash = validator.submitProposal( + _targets, _values, _calldatas, _description, proposalType, proposalTypeConfigurator, attestationUid + ); - assertEq(proposalId, 1); + assertEq(proposalHash, keccak256(abi.encode(_targets, _values, _calldatas, _description))); } } @@ -199,11 +203,14 @@ contract ProposalValidator_SubmitProposal_TestFail is ProposalValidator_Init { _createProposalSetup(); ProposalValidator.ProposalType proposalType = ProposalValidator.ProposalType.ProtocolOrGovernorUpgrade; + uint8 proposalTypeConfigurator = 0; bytes32 invalidAttestationUid = bytes32(uint256(1)); // Invalid attestation UID vm.prank(topDelegate_A); vm.expectRevert(IProposalValidator.ProposalValidator_InvalidAttestation.selector); - validator.submitProposal(targets, values, calldatas, description, proposalType, invalidAttestationUid); + validator.submitProposal( + targets, values, calldatas, description, proposalType, proposalTypeConfigurator, invalidAttestationUid + ); } function test_submitProposal_wrongAttester_reverts() public { @@ -211,21 +218,22 @@ contract ProposalValidator_SubmitProposal_TestFail is ProposalValidator_Init { _createProposalSetup(); ProposalValidator.ProposalType proposalType = ProposalValidator.ProposalType.ProtocolOrGovernorUpgrade; + uint8 proposalTypeConfigurator = 0; // Create attestation with wrong delegate bytes32 attestationUid = _createAttestation(topDelegate_B, proposalType); vm.prank(topDelegate_A); vm.expectRevert(IProposalValidator.ProposalValidator_InvalidAttestation.selector); - validator.submitProposal(targets, values, calldatas, description, proposalType, attestationUid); + validator.submitProposal( + targets, values, calldatas, description, proposalType, proposalTypeConfigurator, attestationUid + ); } } /// @title ProposalValidator_ApproveProposal_Test /// @notice Happy path tests for approveProposal function contract ProposalValidator_ApproveProposal_Test is ProposalValidator_Init { - uint256 proposalId; - function setUp() public override { super.setUp(); @@ -233,25 +241,26 @@ contract ProposalValidator_ApproveProposal_Test is ProposalValidator_Init { _createProposalSetup(); ProposalValidator.ProposalType proposalType = ProposalValidator.ProposalType.ProtocolOrGovernorUpgrade; + uint8 proposalTypeConfigurator = 0; bytes32 attestationUid = _createAttestation(topDelegate_A, proposalType); vm.prank(topDelegate_A); - proposalId = validator.submitProposal(targets, values, calldatas, description, proposalType, attestationUid); + proposalHash = validator.submitProposal( + targets, values, calldatas, description, proposalType, proposalTypeConfigurator, attestationUid + ); } function test_approveProposal_succeeds() public { - _approveProposal(topDelegate_A, proposalId); - _approveProposal(topDelegate_B, proposalId); - _approveProposal(topDelegate_C, proposalId); - _approveProposal(topDelegate_D, proposalId); + _approveProposal(topDelegate_A, proposalHash); + _approveProposal(topDelegate_B, proposalHash); + _approveProposal(topDelegate_C, proposalHash); + _approveProposal(topDelegate_D, proposalHash); } } /// @title ProposalValidator_ApproveProposal_TestFail /// @notice Sad path tests for approveProposal function contract ProposalValidator_ApproveProposal_TestFail is ProposalValidator_Init { - uint256 proposalId; - function setUp() public override { super.setUp(); @@ -259,34 +268,37 @@ contract ProposalValidator_ApproveProposal_TestFail is ProposalValidator_Init { _createProposalSetup(); ProposalValidator.ProposalType proposalType = ProposalValidator.ProposalType.ProtocolOrGovernorUpgrade; + uint8 proposalTypeConfigurator = 0; bytes32 attestationUid = _createAttestation(topDelegate_A, proposalType); vm.prank(topDelegate_A); - proposalId = validator.submitProposal(targets, values, calldatas, description, proposalType, attestationUid); + proposalHash = validator.submitProposal( + targets, values, calldatas, description, proposalType, proposalTypeConfigurator, attestationUid + ); } function test_approveProposal_insufficientVotingPower_reverts() public { vm.expectRevert(IProposalValidator.ProposalValidator_InsufficientVotingPower.selector); - _approveProposal(rando, proposalId); + _approveProposal(rando, proposalHash); } function test_approveProposal_alreadyApproved_reverts() public { - _approveProposal(topDelegate_A, proposalId); + _approveProposal(topDelegate_A, proposalHash); vm.expectRevert(IProposalValidator.ProposalValidator_ProposalAlreadyApproved.selector); - _approveProposal(topDelegate_A, proposalId); + _approveProposal(topDelegate_A, proposalHash); } } /// @title ProposalValidator_MoveToVote_Test /// @notice Happy path tests for moveToVote function contract ProposalValidator_MoveToVote_Test is ProposalValidator_Init { - uint256 proposalId; address[] targets; uint256[] values; bytes[] calldatas; string description; ProposalValidator.ProposalType proposalType; + uint8 proposalTypeConfigurator; function setUp() public override { super.setUp(); @@ -294,26 +306,31 @@ contract ProposalValidator_MoveToVote_Test is ProposalValidator_Init { (targets, values, calldatas, description) = _createProposalSetup(); proposalType = ProposalValidator.ProposalType.ProtocolOrGovernorUpgrade; + proposalTypeConfigurator = 0; bytes32 attestationUid = _createAttestation(topDelegate_A, proposalType); vm.prank(topDelegate_A); - proposalId = validator.submitProposal(targets, values, calldatas, description, proposalType, attestationUid); + proposalHash = validator.submitProposal( + targets, values, calldatas, description, proposalType, proposalTypeConfigurator, attestationUid + ); - _approveProposal(topDelegate_A, proposalId); - _approveProposal(topDelegate_B, proposalId); - _approveProposal(topDelegate_C, proposalId); - _approveProposal(topDelegate_D, proposalId); + _approveProposal(topDelegate_A, proposalHash); + _approveProposal(topDelegate_B, proposalHash); + _approveProposal(topDelegate_C, proposalHash); + _approveProposal(topDelegate_D, proposalHash); } function test_moveToVote_succeeds() public { _mockAndExpect( address(governor), - abi.encodeCall(IOptimismGovernor.propose, (targets, values, calldatas, description, uint8(proposalType))), + abi.encodeCall( + IOptimismGovernor.propose, (targets, values, calldatas, description, proposalTypeConfigurator) + ), abi.encode(1) ); vm.prank(owner); - uint256 governorProposalId = validator.moveToVote(proposalId); + uint256 governorProposalId = validator.moveToVote(targets, values, calldatas, description); assertEq(governorProposalId, 1); } @@ -322,12 +339,12 @@ contract ProposalValidator_MoveToVote_Test is ProposalValidator_Init { /// @title ProposalValidator_MoveToVote_TestFail /// @notice Sad path tests for moveToVote function contract ProposalValidator_MoveToVote_TestFail is ProposalValidator_Init { - uint256 proposalId; address[] targets; uint256[] values; bytes[] calldatas; string description; ProposalValidator.ProposalType proposalType; + uint8 proposalTypeConfigurator; function setUp() public override { super.setUp(); @@ -335,42 +352,47 @@ contract ProposalValidator_MoveToVote_TestFail is ProposalValidator_Init { (targets, values, calldatas, description) = _createProposalSetup(); proposalType = ProposalValidator.ProposalType.ProtocolOrGovernorUpgrade; + proposalTypeConfigurator = 0; bytes32 attestationUid = _createAttestation(topDelegate_A, proposalType); vm.prank(topDelegate_A); - proposalId = validator.submitProposal(targets, values, calldatas, description, proposalType, attestationUid); + proposalHash = validator.submitProposal( + targets, values, calldatas, description, proposalType, proposalTypeConfigurator, attestationUid + ); } function test_moveToVote_insufficientApprovals_reverts() public { // Only approve with 3 delegates (need 4) - _approveProposal(topDelegate_A, proposalId); - _approveProposal(topDelegate_B, proposalId); - _approveProposal(topDelegate_C, proposalId); + _approveProposal(topDelegate_A, proposalHash); + _approveProposal(topDelegate_B, proposalHash); + _approveProposal(topDelegate_C, proposalHash); vm.expectRevert(IProposalValidator.ProposalValidator_InsufficientApprovals.selector); vm.prank(owner); - validator.moveToVote(proposalId); + validator.moveToVote(targets, values, calldatas, description); } function test_moveToVote_alreadyProposed_reverts() public { // Approve with all 4 delegates - _approveProposal(topDelegate_A, proposalId); - _approveProposal(topDelegate_B, proposalId); - _approveProposal(topDelegate_C, proposalId); - _approveProposal(topDelegate_D, proposalId); + _approveProposal(topDelegate_A, proposalHash); + _approveProposal(topDelegate_B, proposalHash); + _approveProposal(topDelegate_C, proposalHash); + _approveProposal(topDelegate_D, proposalHash); _mockAndExpect( address(governor), - abi.encodeCall(IOptimismGovernor.propose, (targets, values, calldatas, description, uint8(proposalType))), + abi.encodeCall( + IOptimismGovernor.propose, (targets, values, calldatas, description, proposalTypeConfigurator) + ), abi.encode(1) ); vm.prank(owner); - validator.moveToVote(proposalId); + validator.moveToVote(targets, values, calldatas, description); - vm.expectRevert(IProposalValidator.ProposalValidator_ProposalAlreadyInVoting.selector); + vm.expectRevert(IProposalValidator.ProposalValidator_ProposalAlreadySubmitted.selector); vm.prank(owner); - validator.moveToVote(proposalId); + validator.moveToVote(targets, values, calldatas, description); } } @@ -401,41 +423,36 @@ contract ProposalValidator_Integration_Test is ProposalValidator_Init { _createProposalSetup(); ProposalValidator.ProposalType proposalType = ProposalValidator.ProposalType.ProtocolOrGovernorUpgrade; + uint8 proposalTypeConfigurator = 0; bytes32 attestationUid = _createAttestation(topDelegate_A, proposalType); vm.prank(topDelegate_A); - uint256 proposalId = - validator.submitProposal(targets, values, calldatas, description, proposalType, attestationUid); - - assertEq(proposalId, 1); - - // It reverts when caller is not a top delegate - vm.expectRevert(IProposalValidator.ProposalValidator_InsufficientVotingPower.selector); - _approveProposal(rando, proposalId); - - _approveProposal(topDelegate_A, proposalId); - _approveProposal(topDelegate_B, proposalId); - _approveProposal(topDelegate_C, proposalId); - - // It reverts when proposal hasn't reached the required approvals - vm.expectRevert(IProposalValidator.ProposalValidator_InsufficientApprovals.selector); - vm.prank(owner); - validator.moveToVote(proposalId); + bytes32 proposalHash = validator.submitProposal( + targets, values, calldatas, description, proposalType, proposalTypeConfigurator, attestationUid + ); - _approveProposal(topDelegate_D, proposalId); + // Collect all required approvals + _approveProposal(topDelegate_A, proposalHash); + _approveProposal(topDelegate_B, proposalHash); + _approveProposal(topDelegate_C, proposalHash); + _approveProposal(topDelegate_D, proposalHash); + // Mock the governor call _mockAndExpect( address(governor), - abi.encodeCall(IOptimismGovernor.propose, (targets, values, calldatas, description, uint8(proposalType))), + abi.encodeCall( + IOptimismGovernor.propose, (targets, values, calldatas, description, proposalTypeConfigurator) + ), abi.encode(1) ); + // Move to vote phase vm.prank(owner); - validator.moveToVote(proposalId); + uint256 governorProposalId = validator.moveToVote(targets, values, calldatas, description); // It reverts when proposal is already in voting phase - vm.expectRevert(IProposalValidator.ProposalValidator_ProposalAlreadyInVoting.selector); + vm.expectRevert(IProposalValidator.ProposalValidator_ProposalAlreadySubmitted.selector); vm.prank(owner); - validator.moveToVote(proposalId); + validator.moveToVote(targets, values, calldatas, description); } }