Skip to content

Commit 9bc1c28

Browse files
0xChin0xOneTony
authored andcommitted
fix: check hash after proposal (#447)
* fix: add proposal id validation on submit upgrade proposal returned proposalId * fix: pre-pr
1 parent d4f7b8a commit 9bc1c28

File tree

3 files changed

+46
-3
lines changed

3 files changed

+46
-3
lines changed

packages/contracts-bedrock/snapshots/semver-lock.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,8 +172,8 @@
172172
"sourceCodeHash": "0x9baa0f9e744cc0ecc61d0fade8bffc18321b228833ea0904dc645f3975be9ed1"
173173
},
174174
"src/governance/ProposalValidator.sol:ProposalValidator": {
175-
"initCodeHash": "0xe7a93826772bf108a21923f7e45b1f46cdadb75e48b0c796e43d64f0c1d81504",
176-
"sourceCodeHash": "0xadd8e049bf3c652af123b6c64a1d504c92be13b4797ab10b978df907b05dcf7f"
175+
"initCodeHash": "0x06a2b713907a4a4c961061edeb8f9417f9fdf63fc5512e6794af67fcc548935d",
176+
"sourceCodeHash": "0x98cb001a29058d9d4bdd017c73e3520af1e22e9dee15e153799196b3390923df"
177177
},
178178
"src/legacy/DeployerWhitelist.sol:DeployerWhitelist": {
179179
"initCodeHash": "0x53099379ed48b87f027d55712dbdd1da7d7099925426eb0531da9c0012e02c29",

packages/contracts-bedrock/src/governance/ProposalValidator.sol

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -369,10 +369,15 @@ contract ProposalValidator is OwnableUpgradeable, ReinitializableBase, ISemver {
369369
if (_proposalType == ProposalType.MaintenanceUpgrade) {
370370
proposal.movedToVote = true;
371371

372-
GOVERNOR.proposeWithModule(
372+
uint256 proposalId = GOVERNOR.proposeWithModule(
373373
votingModule, proposalVotingModuleData, _proposalDescription, uint8(_proposalType)
374374
);
375375

376+
// Make sure the proposalId is the same as the proposalHash
377+
if (proposalId != uint256(proposalHash_)) {
378+
revert ProposalValidator_ProposalIdMismatch();
379+
}
380+
376381
emit ProposalMovedToVote(proposalHash_, msg.sender);
377382
}
378383
}

packages/contracts-bedrock/test/governance/ProposalValidator.t.sol

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1077,6 +1077,44 @@ contract ProposalValidator_SubmitUpgradeProposal_TestFail is ProposalValidator_I
10771077
againstThreshold, proposalDescription, attestationUid, proposalType, CYCLE_NUMBER
10781078
);
10791079
}
1080+
1081+
function test_submitUpgradeProposal_proposalIdMismatch_reverts(uint256 proposalId) public {
1082+
uint248 againstThreshold = 5000;
1083+
ProposalValidator.ProposalType proposalType = ProposalValidator.ProposalType.MaintenanceUpgrade;
1084+
bytes32 attestationUid = _createApprovedProposerAttestation(topDelegate_A, proposalType);
1085+
1086+
// Calculate expected proposal hash
1087+
bytes memory votingModuleData = _constructOptimisticVotingModuleData(againstThreshold);
1088+
bytes32 expectedHash = validator.hashProposalWithModule(
1089+
optimisticVotingModule, votingModuleData, keccak256(bytes(proposalDescription))
1090+
);
1091+
1092+
vm.assume(proposalId != uint256(expectedHash)); // Ensure proposalId is different from expectedHash
1093+
1094+
_mockProposalTypesConfiguratorCall(OPTIMISTIC_VOTING_MODULE_ID);
1095+
1096+
_mockAndExpect(
1097+
address(governor),
1098+
abi.encodeCall(IOptimismGovernor.proposalSnapshot, (uint256(expectedHash))),
1099+
abi.encode(0)
1100+
);
1101+
1102+
// Mock the proposeWithModule call to return a different proposalId
1103+
_mockAndExpect(
1104+
address(governor),
1105+
abi.encodeCall(
1106+
IOptimismGovernor.proposeWithModule,
1107+
(optimisticVotingModule, votingModuleData, proposalDescription, uint8(proposalType))
1108+
),
1109+
abi.encode(proposalId)
1110+
);
1111+
1112+
vm.expectRevert(ProposalValidator.ProposalValidator_ProposalIdMismatch.selector);
1113+
vm.prank(topDelegate_A);
1114+
validator.submitUpgradeProposal(
1115+
againstThreshold, proposalDescription, attestationUid, proposalType, CYCLE_NUMBER
1116+
);
1117+
}
10801118
}
10811119

10821120
/// @title ProposalValidator_SubmitCouncilMemberElectionsProposal_Test

0 commit comments

Comments
 (0)