Skip to content

fix: voting cycle validity on submit #454

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jul 22, 2025
Merged

fix: voting cycle validity on submit #454

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b983417
fix: voting cycle validity on submit
0xOneTony Jul 21, 2025
1a67513
fix: edge case on upgrade proposals
0xOneTony Jul 22, 2025
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: 2 additions & 2 deletions packages/contracts-bedrock/snapshots/semver-lock.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,8 @@
"sourceCodeHash": "0x18f43b227decd0f2a895b8b55e23fa6a47706697c272bbf2482b3f912be446e1"
},
"src/governance/ProposalValidator.sol:ProposalValidator": {
"initCodeHash": "0xbac284f6ec21a5d65d5b86d7e6406e0805d77e15dc4bd66397f0111701110e0e",
"sourceCodeHash": "0x58048692d1da18d17958b2dc950055ae2a58ab645385b0aed3528b289dfee21d"
"initCodeHash": "0xd91db9afff20ea39d50d59f9c29ddfdd70fa5d438bd789eebd568be3dd5e9331",
"sourceCodeHash": "0xcbd5116adad22793fa6c5a8321ae035f976619a8f5d2963e36a1bff2c93941e2"
},
"src/legacy/DeployerWhitelist.sol:DeployerWhitelist": {
"initCodeHash": "0x53099379ed48b87f027d55712dbdd1da7d7099925426eb0531da9c0012e02c29",
Expand Down
18 changes: 18 additions & 0 deletions packages/contracts-bedrock/src/governance/ProposalValidator.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,12 @@ contract ProposalValidator is OwnableUpgradeable, ReinitializableBase, ISemver {
revert ProposalValidator_InvalidUpgradeProposalType();
}

// Validate voting cycle exists and is not in the past
VotingCycleData memory votingCycleData = votingCycles[_votingCycle];
if (votingCycleData.startingTimestamp == 0 || votingCycleData.startingTimestamp < block.timestamp) {
revert ProposalValidator_InvalidVotingCycle();
}

// Validate EAS attestation - must be called by owner-approved address
_validateApprovedProposerAttestation(_attestationUid, _proposalType);

Expand Down Expand Up @@ -407,6 +413,12 @@ contract ProposalValidator is OwnableUpgradeable, ReinitializableBase, ISemver {
external
returns (bytes32 proposalHash_)
{
// Validate voting cycle exists and is not in the past
VotingCycleData memory votingCycleData = votingCycles[_votingCycle];
if (votingCycleData.startingTimestamp == 0 || votingCycleData.startingTimestamp < block.timestamp) {
revert ProposalValidator_InvalidVotingCycle();
}

// Validate EAS attestation - must be called by owner-approved address
_validateApprovedProposerAttestation(_attestationUid, ProposalType.CouncilMemberElections);

Expand Down Expand Up @@ -503,6 +515,12 @@ contract ProposalValidator is OwnableUpgradeable, ReinitializableBase, ISemver {
revert ProposalValidator_InvalidFundingProposalType();
}

// Validate voting cycle exists and is not in the past
VotingCycleData memory votingCycleData = votingCycles[_votingCycle];
if (votingCycleData.startingTimestamp == 0 || votingCycleData.startingTimestamp < block.timestamp) {
revert ProposalValidator_InvalidVotingCycle();
}

// Validate input arrays have matching lengths
uint256 optionsLength = _optionsDescriptions.length;
if (optionsLength != _optionsRecipients.length || optionsLength != _optionsAmounts.length) {
Expand Down
59 changes: 48 additions & 11 deletions packages/contracts-bedrock/test/governance/ProposalValidator.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -895,6 +895,7 @@ contract ProposalValidator_SubmitUpgradeProposal_Test is ProposalValidator_Init
/// @notice Sad path tests for submitUpgradeProposal function
contract ProposalValidator_SubmitUpgradeProposal_TestFail is ProposalValidator_Init {
string proposalDescription;
uint248 againstThreshold = 5000; // 50%

function setUp() public override {
super.setUp();
Expand All @@ -910,7 +911,6 @@ contract ProposalValidator_SubmitUpgradeProposal_TestFail is ProposalValidator_I
proposalTypeValue = uint8(bound(proposalTypeValue, 2, 4));
ProposalValidator.ProposalType proposalType = ProposalValidator.ProposalType(proposalTypeValue);

uint248 againstThreshold = 5000; // 50%
bytes32 attestationUid = _createApprovedProposerAttestation(topDelegate_A, proposalType);

vm.expectRevert(ProposalValidator.ProposalValidator_InvalidUpgradeProposalType.selector);
Expand All @@ -920,8 +920,25 @@ contract ProposalValidator_SubmitUpgradeProposal_TestFail is ProposalValidator_I
);
}

function testFuzz_submitUpgradeProposal_invalidVotingCycle_reverts(
uint8 proposalTypeValue,
uint256 votingCycle
)
public
{
proposalTypeValue = uint8(bound(proposalTypeValue, 0, 1));
vm.assume(votingCycle != CYCLE_NUMBER);
ProposalValidator.ProposalType proposalType = ProposalValidator.ProposalType(proposalTypeValue);
bytes32 attestationUid = _createApprovedProposerAttestation(topDelegate_A, proposalType);

vm.expectRevert(ProposalValidator.ProposalValidator_InvalidVotingCycle.selector);
vm.prank(topDelegate_A);
validator.submitUpgradeProposal(
againstThreshold, proposalDescription, attestationUid, proposalType, votingCycle
);
}

function testFuzz_submitUpgradeProposal_invalidAttestation_reverts(bytes32 fuzzedAttestationUid) public {
uint248 againstThreshold = 5000;
ProposalValidator.ProposalType proposalType = ProposalValidator.ProposalType.ProtocolOrGovernorUpgrade;
bytes32 validAttestationUid = _createApprovedProposerAttestation(topDelegate_A, proposalType);

Expand All @@ -937,7 +954,6 @@ contract ProposalValidator_SubmitUpgradeProposal_TestFail is ProposalValidator_I
function testFuzz_submitUpgradeProposal_unattestedProposer_reverts(address fuzzedProposer) public {
vm.assume(fuzzedProposer != topDelegate_A); // Ensure it's different from attested proposer

uint248 againstThreshold = 5000;
ProposalValidator.ProposalType proposalType = ProposalValidator.ProposalType.ProtocolOrGovernorUpgrade;
bytes32 attestationUid = _createApprovedProposerAttestation(topDelegate_A, proposalType);

Expand All @@ -952,7 +968,6 @@ contract ProposalValidator_SubmitUpgradeProposal_TestFail is ProposalValidator_I
function testFuzz_submitUpgradeProposal_attestationExpired_reverts(uint8 proposalTypeValue) public {
proposalTypeValue = uint8(bound(proposalTypeValue, 0, 1));
ProposalValidator.ProposalType proposalType = ProposalValidator.ProposalType(proposalTypeValue);
uint248 againstThreshold = 5000;
bytes32 attestationUid = _createApprovedProposerAttestation(topDelegate_A, proposalType);

// warp the time to after the attestation expiration time
Expand Down Expand Up @@ -990,7 +1005,6 @@ contract ProposalValidator_SubmitUpgradeProposal_TestFail is ProposalValidator_I
}

function test_submitUpgradeProposal_invalidVotingModule_reverts() public {
uint248 againstThreshold = 5000; // 50%
ProposalValidator.ProposalType proposalType = ProposalValidator.ProposalType.ProtocolOrGovernorUpgrade;
bytes32 attestationUid = _createApprovedProposerAttestation(topDelegate_A, proposalType);

Expand All @@ -1009,7 +1023,6 @@ contract ProposalValidator_SubmitUpgradeProposal_TestFail is ProposalValidator_I
proposalTypeValue = uint8(bound(proposalTypeValue, 0, 1));
ProposalValidator.ProposalType proposalType = ProposalValidator.ProposalType(proposalTypeValue);

uint248 againstThreshold = 5000;
bytes32 attestationUid = _createApprovedProposerAttestation(topDelegate_A, proposalType);

// Calculate expected proposal hash
Expand Down Expand Up @@ -1061,7 +1074,6 @@ contract ProposalValidator_SubmitUpgradeProposal_TestFail is ProposalValidator_I
proposalTypeValue = uint8(bound(proposalTypeValue, 0, 1));
ProposalValidator.ProposalType proposalType = ProposalValidator.ProposalType(proposalTypeValue);

uint248 againstThreshold = 5000;
bytes32 attestationUid = _createApprovedProposerAttestation(topDelegate_A, proposalType);

// Calculate expected proposal hash
Expand Down Expand Up @@ -1090,7 +1102,6 @@ contract ProposalValidator_SubmitUpgradeProposal_TestFail is ProposalValidator_I
function testFuzz_submitUpgradeProposal_attestationNotFromOwner_reverts(address fuzzedAttester) public {
vm.assume(fuzzedAttester != owner); // Ensure it's not the approved owner

uint248 againstThreshold = 5000;
ProposalValidator.ProposalType proposalType = ProposalValidator.ProposalType.ProtocolOrGovernorUpgrade;

// Create attestation but don't use proper owner as attester
Expand Down Expand Up @@ -1121,8 +1132,6 @@ contract ProposalValidator_SubmitUpgradeProposal_TestFail is ProposalValidator_I
proposalTypeValue = uint8(bound(proposalTypeValue, 0, 1));
ProposalValidator.ProposalType proposalType = ProposalValidator.ProposalType(proposalTypeValue);

uint248 againstThreshold = 5000;

// Create valid attestation first (make it revocable)
bytes32 attestationUid = _createApprovedProposerAttestation(topDelegate_A, proposalType);

Expand All @@ -1143,7 +1152,6 @@ contract ProposalValidator_SubmitUpgradeProposal_TestFail is ProposalValidator_I
}

function test_submitUpgradeProposal_proposalIdMismatch_reverts(uint256 proposalId) public {
uint248 againstThreshold = 5000;
ProposalValidator.ProposalType proposalType = ProposalValidator.ProposalType.MaintenanceUpgrade;
bytes32 attestationUid = _createApprovedProposerAttestation(topDelegate_A, proposalType);

Expand Down Expand Up @@ -1285,6 +1293,15 @@ contract ProposalValidator_SubmitCouncilMemberElectionsProposal_TestFail is Prop
_createApprovedProposerAttestation(topDelegate_A, ProposalValidator.ProposalType.CouncilMemberElections);
}

function testFuzz_submitCouncilMemberElectionsProposal_invalidVotingCycle_reverts(uint256 votingCycle) public {
vm.assume(votingCycle != CYCLE_NUMBER);
vm.expectRevert(ProposalValidator.ProposalValidator_InvalidVotingCycle.selector);
vm.prank(topDelegate_A);
validator.submitCouncilMemberElectionsProposal(
criteriaValue, optionDescriptions, proposalDescription, attestationUid, votingCycle
);
}

function testFuzz_submitCouncilMemberElectionsProposal_invalidAttestation_reverts(bytes32 fuzzedAttestationUid)
public
{
Expand Down Expand Up @@ -1581,6 +1598,26 @@ contract ProposalValidator_SubmitFundingProposal_TestFail is ProposalValidator_I
);
}

function testFuzz_submitFundingProposal_invalidVotingCycle_reverts(
uint8 proposalTypeValue,
uint256 votingCycle
)
public
{
proposalTypeValue = uint8(bound(proposalTypeValue, 3, 4));
ProposalValidator.ProposalType proposalType = ProposalValidator.ProposalType(proposalTypeValue);
vm.assume(votingCycle != CYCLE_NUMBER);

(string[] memory descriptions, address[] memory recipients, uint256[] memory amounts) =
_createMinimalFundingArrays(1);

vm.expectRevert(ProposalValidator.ProposalValidator_InvalidVotingCycle.selector);
vm.prank(user);
validator.submitFundingProposal(
FUNDING_CRITERIA_VALUE, descriptions, recipients, amounts, description, proposalType, votingCycle
);
}

function testFuzz_submitFundingProposal_mismatchedDescriptionsLength_reverts(
uint8 matchingLength,
uint8 mismatchedLength,
Expand Down