diff --git a/packages/contracts-bedrock/snapshots/semver-lock.json b/packages/contracts-bedrock/snapshots/semver-lock.json index 823c10c7b40d7..e769c0254363b 100644 --- a/packages/contracts-bedrock/snapshots/semver-lock.json +++ b/packages/contracts-bedrock/snapshots/semver-lock.json @@ -176,8 +176,8 @@ "sourceCodeHash": "0x18f43b227decd0f2a895b8b55e23fa6a47706697c272bbf2482b3f912be446e1" }, "src/governance/ProposalValidator.sol:ProposalValidator": { - "initCodeHash": "0x44d71e84d08aeb7abc82993a0293f646952d83e439e95427d2c432568f95524e", - "sourceCodeHash": "0xddf3e6506f0155d0120e467cb1437c49b1eff28d362b8e7de55101cd91e43427" + "initCodeHash": "0x553e9a5abda992f985f23d02f07c169be7ee39063d6dbd00742b4298089d3602", + "sourceCodeHash": "0xe3649d1d6a51572d2f6a0f82683d54eb3717dae3373f9a0c2a3648c392e66433" }, "src/legacy/DeployerWhitelist.sol:DeployerWhitelist": { "initCodeHash": "0x53099379ed48b87f027d55712dbdd1da7d7099925426eb0531da9c0012e02c29", diff --git a/packages/contracts-bedrock/src/governance/ProposalValidator.sol b/packages/contracts-bedrock/src/governance/ProposalValidator.sol index dcb729480be77..18ef431638907 100644 --- a/packages/contracts-bedrock/src/governance/ProposalValidator.sol +++ b/packages/contracts-bedrock/src/governance/ProposalValidator.sol @@ -267,7 +267,7 @@ contract ProposalValidator is OwnableUpgradeable, ReinitializableBase, ISemver { { _validateProposal(_targets, _values, _calldatas, _proposalType, _attestationUid); - proposalHash_ = _hashProposal(_targets, _values, _calldatas, _description); + proposalHash_ = bytes32(0); // TODO: Implement hashProposalWithModule ProposalData storage proposal = _proposals[proposalHash_]; if (proposal.proposer != address(0)) { @@ -327,7 +327,7 @@ contract ProposalValidator is OwnableUpgradeable, ReinitializableBase, ISemver { returns (uint256 governorProposalId_) { // Verify that the provided data matches the proposalHash - bytes32 _proposalHash = _hashProposal(_targets, _values, _calldatas, _description); + bytes32 _proposalHash = bytes32(0); // TODO: Implement hashProposalWithModule ProposalData storage proposal = _proposals[_proposalHash]; @@ -454,17 +454,21 @@ contract ProposalValidator is OwnableUpgradeable, ReinitializableBase, ISemver { isValid_ = approvedDelegate == msg.sender && proposalType == uint8(_expectedProposalType); } - function _hashProposal( - address[] memory _targets, - uint256[] memory _values, - bytes[] memory _calldatas, - string memory _description + /// @notice Calculate `proposalId` hashing similarly to `hashProposal` but based on `module` and `proposalData`. + /// @param _module The address of the voting module to use for this proposal. + /// @param _proposalData The proposal data to pass to the voting module. + /// @param _descriptionHash The hash of the proposal description. + /// @return The hash of the proposal. + function _hashProposalWithModule( + address _module, + bytes memory _proposalData, + bytes32 _descriptionHash ) internal - pure - returns (bytes32 proposalHash_) + view + returns (bytes32) { - return keccak256(abi.encode(_targets, _values, _calldatas, _description)); + return keccak256(abi.encode(address(this), _module, _proposalData, _descriptionHash)); } /// @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 1c6745438adcf..3b6c8f3697387 100644 --- a/packages/contracts-bedrock/test/governance/ProposalValidator.t.sol +++ b/packages/contracts-bedrock/test/governance/ProposalValidator.t.sol @@ -30,17 +30,16 @@ contract ProposalValidatorForTest is ProposalValidator { ProposalValidator(_attestationSchemaUid, _governor, _governanceToken) { } - function hashProposal( - address[] memory _targets, - uint256[] memory _values, - bytes[] memory _calldatas, - string memory _description + function hashProposalWithModule( + address _module, + bytes memory _proposalData, + bytes32 _descriptionHash ) public - pure + view returns (bytes32) { - return _hashProposal(_targets, _values, _calldatas, _description); + return _hashProposalWithModule(_module, _proposalData, _descriptionHash); } } @@ -254,7 +253,7 @@ contract ProposalValidator_SubmitProposal_Test is ProposalValidator_Init { ProposalValidator.ProposalType proposalType = ProposalValidator.ProposalType.ProtocolOrGovernorUpgrade; uint8 proposalVotingModule = 0; bytes32 attestationUid = _createAttestation(topDelegate_A, proposalType); - bytes32 expectedProposalHash = validator.hashProposal(_targets, _values, _calldatas, _description); + bytes32 expectedProposalHash = bytes32(0); // TODO: Implement hashProposalWithModule // Expect event to be emitted vm.expectEmit(address(validator)); @@ -678,6 +677,42 @@ contract ProposalValidator_Integration_Test is ProposalValidator_Init { } } +/// @title ProposalValidator_HashProposalWithModule_Test +/// @notice Tests for the hashProposalWithModule function +contract ProposalValidator_HashProposalWithModule_Test is ProposalValidator_Init { + function test_hashProposalWithModule_succeeds() public { + address testModule = makeAddr("testModule"); + bytes memory testProposalData = abi.encode("test", "proposal", "data"); + bytes32 testDescriptionHash = keccak256("test description"); + + bytes32 hash = validator.hashProposalWithModule(testModule, testProposalData, testDescriptionHash); + assertTrue(hash != bytes32(0)); + } + + function test_hashProposalWithModule_consistentHash_succeeds() public { + address testModule = makeAddr("testModule"); + bytes memory testProposalData = abi.encode("test data"); + bytes32 testDescriptionHash = keccak256("description"); + + bytes32 hash1 = validator.hashProposalWithModule(testModule, testProposalData, testDescriptionHash); + bytes32 hash2 = validator.hashProposalWithModule(testModule, testProposalData, testDescriptionHash); + + assertEq(hash1, hash2); + } + + function test_hashProposalWithModule_differentInputs_succeeds() public { + address module1 = makeAddr("module1"); + address module2 = makeAddr("module2"); + bytes memory data = abi.encode("data"); + bytes32 descHash = keccak256("desc"); + + bytes32 hash1 = validator.hashProposalWithModule(module1, data, descHash); + bytes32 hash2 = validator.hashProposalWithModule(module2, data, descHash); + + assertTrue(hash1 != hash2); + } +} + /// @title ProposalValidator_Initialize_Test /// @notice Tests for the initialize function contract ProposalValidator_Initialize_Test is ProposalValidator_Init {