From 16c36162ae81516c5d92ceb3e4585cb37ce881ea Mon Sep 17 00:00:00 2001 From: OneTony Date: Fri, 18 Jul 2025 16:00:25 +0300 Subject: [PATCH] fix: approved proposer schema --- .../governance/IProposalValidator.sol | 1 + .../src/governance/ProposalValidator.sol | 14 ++++-- .../test/governance/ProposalValidator.t.sol | 44 ++++++++++++++----- 3 files changed, 46 insertions(+), 13 deletions(-) diff --git a/packages/contracts-bedrock/interfaces/governance/IProposalValidator.sol b/packages/contracts-bedrock/interfaces/governance/IProposalValidator.sol index 5c0c48a7be456..2aec2aba09ac6 100644 --- a/packages/contracts-bedrock/interfaces/governance/IProposalValidator.sol +++ b/packages/contracts-bedrock/interfaces/governance/IProposalValidator.sol @@ -22,6 +22,7 @@ interface IProposalValidator is ISemver { error ProposalValidator_ExceedsDistributionThreshold(); error ProposalValidator_InvalidOptionsLength(); error ProposalValidator_AttestationRevoked(); + error ProposalValidator_AttestationExpired(); error ProposalValidator_InvalidAttestationSchema(); error ProposalValidator_InvalidCriteriaValue(); error ProposalValidator_InvalidAgainstThreshold(); diff --git a/packages/contracts-bedrock/src/governance/ProposalValidator.sol b/packages/contracts-bedrock/src/governance/ProposalValidator.sol index f37a40d1a7f0f..432fc79ba5175 100644 --- a/packages/contracts-bedrock/src/governance/ProposalValidator.sol +++ b/packages/contracts-bedrock/src/governance/ProposalValidator.sol @@ -62,6 +62,9 @@ contract ProposalValidator is OwnableUpgradeable, ReinitializableBase, ISemver { /// @notice Thrown when an attestation is revoked. error ProposalValidator_AttestationRevoked(); + /// @notice Thrown when the attestation is expired. + error ProposalValidator_AttestationExpired(); + /// @notice Thrown when an attestation schema is invalid. error ProposalValidator_InvalidAttestationSchema(); @@ -207,7 +210,7 @@ contract ProposalValidator is OwnableUpgradeable, ReinitializableBase, ISemver { /// @notice The schema UID for attestations in the Ethereum Attestation Service for checking if the caller /// is an approved proposer. - /// @dev Schema format: { approvedProposer: address, proposalType: uint8 } + /// @dev Schema format: { proposalType: uint8, date: string } bytes32 public immutable APPROVED_PROPOSER_ATTESTATION_SCHEMA_UID; /// @notice The schema UID for attestations in the Ethereum Attestation Service for checking if the caller @@ -887,11 +890,16 @@ contract ProposalValidator is OwnableUpgradeable, ReinitializableBase, ISemver { revert ProposalValidator_AttestationRevoked(); } - (address approvedDelegate, uint8 proposalType) = abi.decode(attestation.data, (address, uint8)); + // check if the attestation is expired + if (attestation.expirationTime != 0 && attestation.expirationTime < block.timestamp) { + revert ProposalValidator_AttestationExpired(); + } + + (uint8 proposalType, ) = abi.decode(attestation.data, (uint8, string)); if ( attestation.attester != owner() || attestation.schema != APPROVED_PROPOSER_ATTESTATION_SCHEMA_UID - || approvedDelegate != msg.sender || proposalType != uint8(_expectedProposalType) + || attestation.recipient != _msgSender() || proposalType != uint8(_expectedProposalType) ) { revert ProposalValidator_InvalidAttestation(); } diff --git a/packages/contracts-bedrock/test/governance/ProposalValidator.t.sol b/packages/contracts-bedrock/test/governance/ProposalValidator.t.sol index b538055e2c23d..9b70a5928e1b8 100644 --- a/packages/contracts-bedrock/test/governance/ProposalValidator.t.sol +++ b/packages/contracts-bedrock/test/governance/ProposalValidator.t.sol @@ -115,6 +115,7 @@ contract ProposalValidator_Init is CommonTest { uint256 public constant OPTIMISTIC_MODULE_PERCENT_DIVISOR = 10_000; uint8 public constant APPROVAL_VOTING_MODULE_ID = 1; uint8 public constant OPTIMISTIC_VOTING_MODULE_ID = 2; + uint64 public constant ATT_EXPIRATION_TIME = 10 days; address owner; address user; @@ -526,7 +527,7 @@ contract ProposalValidator_Init is CommonTest { // Create schemas vm.prank(owner); APPROVED_PROPOSER_ATTESTATION_SCHEMA_UID = ISchemaRegistry(Predeploys.SCHEMA_REGISTRY).register( - "address approvedAddress,uint8 proposalType", ISchemaResolver(address(0)), true + "uint8 proposalType,string date", ISchemaResolver(address(0)), true ); vm.prank(owner); @@ -553,11 +554,11 @@ contract ProposalValidator_Init is CommonTest { AttestationRequest({ schema: APPROVED_PROPOSER_ATTESTATION_SCHEMA_UID, data: AttestationRequestData({ - recipient: address(0), - expirationTime: 0, + recipient: _delegate, + expirationTime: uint64(block.timestamp + ATT_EXPIRATION_TIME), revocable: true, refUID: bytes32(0), - data: abi.encode(_delegate, _proposalType), + data: abi.encode(_proposalType, "2000-01-01"), value: 0 }) }) @@ -915,6 +916,19 @@ 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 + vm.warp(block.timestamp + ATT_EXPIRATION_TIME + 1); + vm.expectRevert(ProposalValidator.ProposalValidator_AttestationExpired.selector); + vm.prank(topDelegate_A); + validator.submitUpgradeProposal(againstThreshold, proposalDescription, attestationUid, proposalType, CYCLE_NUMBER); + } + function test_submitUpgradeProposal_zeroAgainstThreshold_reverts() public { uint248 zeroThreshold = 0; ProposalValidator.ProposalType proposalType = ProposalValidator.ProposalType.ProtocolOrGovernorUpgrade; @@ -1035,11 +1049,11 @@ contract ProposalValidator_SubmitUpgradeProposal_TestFail is ProposalValidator_I AttestationRequest({ schema: APPROVED_PROPOSER_ATTESTATION_SCHEMA_UID, data: AttestationRequestData({ - recipient: address(0), - expirationTime: 0, + recipient: topDelegate_A, + expirationTime: uint64(block.timestamp + ATT_EXPIRATION_TIME), revocable: false, refUID: bytes32(0), - data: abi.encode(topDelegate_A, proposalType), + data: abi.encode(proposalType, "2000-01-01"), value: 0 }) }) @@ -1244,6 +1258,16 @@ contract ProposalValidator_SubmitCouncilMemberElectionsProposal_TestFail is Prop ); } + function testFuzz_submitCouncilMemberElectionsProposal_attestationExpired_reverts() public { + // warp the time to after the attestation expiration time + vm.warp(block.timestamp + ATT_EXPIRATION_TIME + 1); + vm.expectRevert(ProposalValidator.ProposalValidator_AttestationExpired.selector); + vm.prank(topDelegate_A); + validator.submitCouncilMemberElectionsProposal( + criteriaValue, optionDescriptions, proposalDescription, attestationUid, CYCLE_NUMBER + ); + } + function test_submitCouncilMemberElectionsProposal_zeroOptions_reverts() public { string[] memory emptyOptions = new string[](0); @@ -1322,11 +1346,11 @@ contract ProposalValidator_SubmitCouncilMemberElectionsProposal_TestFail is Prop AttestationRequest({ schema: APPROVED_PROPOSER_ATTESTATION_SCHEMA_UID, data: AttestationRequestData({ - recipient: address(0), - expirationTime: 0, + recipient: topDelegate_A, + expirationTime: uint64(block.timestamp + ATT_EXPIRATION_TIME), revocable: false, refUID: bytes32(0), - data: abi.encode(topDelegate_A, ProposalValidator.ProposalType.CouncilMemberElections), + data: abi.encode(ProposalValidator.ProposalType.CouncilMemberElections, "2000-01-01"), value: 0 }) })