Skip to content
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.

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

Fix H-01: Allow whitelisted accounts below threshold to propose #89

Merged
merged 8 commits into from
Dec 16, 2024
80 changes: 61 additions & 19 deletions contracts/CompoundGovernor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -135,32 +135,73 @@ contract CompoundGovernor is
return GovernorSequentialProposalIdUpgradeable.hashProposal(_targets, _values, _calldatas, _descriptionHash);
}

/// @notice Creates a new proposal. Skips proposal threshold check for whitelisted accounts.
/// @param _targets An array of addresses that will be called if the proposal is executed.
/// @param _values An array of ETH values to be sent to each address when the proposal is executed.
/// @param _calldatas An array of calldata to be sent to each address when the proposal is executed.
/// @param _description A human-readable description of the proposal.
/// @return uint256 The ID of the newly created proposal.
function propose(
address[] memory _targets,
uint256[] memory _values,
bytes[] memory _calldatas,
string memory _description
) public override(GovernorUpgradeable) returns (uint256) {
address _proposer = _msgSender();

// check description restriction
if (!_isValidDescriptionForProposer(_proposer, _description)) {
revert GovernorRestrictedProposer(_proposer);
}

if (!isWhitelisted(_proposer)) {
// check proposal threshold
uint256 _votesThreshold = proposalThreshold();
if (_votesThreshold > 0) {
uint256 _proposerVotes = getVotes(_proposer, clock() - 1);
if (_proposerVotes < _votesThreshold) {
revert GovernorInsufficientProposerVotes(_proposer, _proposerVotes, _votesThreshold);
}
}
}

return _propose(_targets, _values, _calldatas, _description, _proposer);
}

/// @notice Internal function used to create a new proposal.
/// @dev This is an override that supports sequential proposal IDs. Called by the public `propose` function.
/// @param _targets An array of addresses that will be called if the proposal is executed.
/// @param _values An array of ETH values to be sent to each address when the proposal is executed.
/// @param _calldatas An array of calldata to be sent to each address when the proposal is executed.
/// @param _description A human-readable description of the proposal.
/// @param _proposer The address of the account creating the proposal.
/// @return uint256 The ID of the newly created proposal.
function _propose(
address[] memory targets,
uint256[] memory values,
bytes[] memory calldatas,
string memory description,
address proposer
address[] memory _targets,
uint256[] memory _values,
bytes[] memory _calldatas,
string memory _description,
address _proposer
) internal override(GovernorUpgradeable, GovernorSequentialProposalIdUpgradeable) returns (uint256) {
return GovernorSequentialProposalIdUpgradeable._propose(targets, values, calldatas, description, proposer);
return GovernorSequentialProposalIdUpgradeable._propose(_targets, _values, _calldatas, _description, _proposer);
}

/// @notice Cancels an active proposal.
/// @notice This function can be called by the proposer, the proposal guardian, or anyone if the proposer's voting
/// power has dropped below the proposal threshold. For whitelisted proposers, only special actors (proposer,
/// proposal guardian, whitelist guardian) can cancel if the proposer is below the threshold.
/// @param targets An array of addresses that will be called if the proposal is executed.
/// @param values An array of ETH values to be sent to each address when the proposal is executed.
/// @param calldatas An array of calldata to be sent to each address when the proposal is executed.
/// @param descriptionHash The hash of the proposal's description string.
/// @param _targets An array of addresses that will be called if the proposal is executed.
/// @param _values An array of ETH values to be sent to each address when the proposal is executed.
/// @param _calldatas An array of calldata to be sent to each address when the proposal is executed.
/// @param _descriptionHash The hash of the proposal's description string.
/// @return uint256 The ID of the canceled proposal.
function cancel(
address[] memory targets,
uint256[] memory values,
bytes[] memory calldatas,
bytes32 descriptionHash
address[] memory _targets,
uint256[] memory _values,
bytes[] memory _calldatas,
bytes32 _descriptionHash
) public override returns (uint256) {
uint256 _proposalId = hashProposal(targets, values, calldatas, descriptionHash);
uint256 _proposalId = hashProposal(_targets, _values, _calldatas, _descriptionHash);
address _proposer = proposalProposer(_proposalId);

if (msg.sender != _proposer && msg.sender != proposalGuardian.account) {
Expand All @@ -173,7 +214,7 @@ contract CompoundGovernor is
}
}

return _cancel(targets, values, calldatas, descriptionHash);
return _cancel(_targets, _values, _calldatas, _descriptionHash);
}

/// @notice Cancels a proposal given its ID.
Expand All @@ -189,9 +230,10 @@ contract CompoundGovernor is
}

/// @notice Sets or updates the whitelist expiration for a specific account.
/// @notice A whitelisted account's proposals cannot be canceled by anyone except the `whitelistGuardian` when its
/// voting weight falls below the `proposalThreshold`.
/// @notice The whitelist account and `proposalGuardian` can still cancel its proposals regardless of voting weight.
/// A whitelisted account can create proposals without meeting the 'proposalThreshold'.
/// A whitelisted account's proposals cannot be canceled by anyone except the `whitelistGuardian` and only when its
/// voting weight is below the `proposalThreshold`.
/// A whitelisted account and `proposalGuardian` can still cancel its proposals regardless of voting weight.
/// @dev Only the executor (timelock) or the `whitelistGuardian` can call this function.
/// @param _account The address of the account to be whitelisted.
/// @param _expiration The timestamp until which the account will be whitelisted.
Expand Down
50 changes: 45 additions & 5 deletions contracts/test/CompoundGovernor.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,15 @@ contract Propose is CompoundGovernorTest {
vm.assertEq(uint8(governor.state(_proposalId)), uint8(IGovernor.ProposalState.Active));
}

function testFuzz_WhitelistedAccountCanProposeBelowThreshold(address _proposer) public {
Proposal memory _proposal = _buildAnEmptyProposal();
_setWhitelistedProposer(_proposer);
uint256 _proposalId = _getProposalId(_proposal);

_submitProposal(_proposer, _proposal);
vm.assertEq(uint8(governor.state(_proposalId)), uint8(IGovernor.ProposalState.Active));
}

function test_EmitsProposalCreatedEvent() public {
Proposal memory _proposal = _buildAnEmptyProposal();
address _proposer = _getRandomProposer();
Expand All @@ -93,6 +102,42 @@ contract Propose is CompoundGovernorTest {
);
_submitProposal(_proposer, _proposal);
}

function testFuzz_RevertIf_NonWhitelistedProposerIsBelowThreshold(address _proposer) public {
vm.assume(governor.getVotes(_proposer, vm.getBlockNumber() - 1) < governor.proposalThreshold());
Proposal memory _proposal = _buildAnEmptyProposal();

vm.expectRevert(
abi.encodeWithSelector(
IGovernor.GovernorInsufficientProposerVotes.selector,
_proposer,
governor.getVotes(_proposer, vm.getBlockNumber() - 1),
governor.proposalThreshold()
)
);
_submitProposal(_proposer, _proposal);
}

function testFuzz_RevertIf_ExpiredWhitelistedAccountIsBelowThreshold(
address _proposer,
uint256 _timeElapsedAfterAccountExpiry
) public {
_timeElapsedAfterAccountExpiry = bound(_timeElapsedAfterAccountExpiry, 0, type(uint96).max);
vm.assume(governor.getVotes(_proposer, vm.getBlockNumber() - 1) < governor.proposalThreshold());
_setWhitelistedProposer(_proposer);
vm.warp(governor.whitelistAccountExpirations(_proposer) + _timeElapsedAfterAccountExpiry);
Proposal memory _proposal = _buildAnEmptyProposal();

vm.expectRevert(
abi.encodeWithSelector(
IGovernor.GovernorInsufficientProposerVotes.selector,
_proposer,
governor.getVotes(_proposer, vm.getBlockNumber() - 1),
governor.proposalThreshold()
)
);
_submitProposal(_proposer, _proposal);
}
}

abstract contract Queue is CompoundGovernorTest {
Expand Down Expand Up @@ -340,11 +385,6 @@ abstract contract Cancel is CompoundGovernorTest {

function _cancelWithProposalDetailsOrId(Proposal memory _proposal, uint256 _proposalId) internal virtual;

function _setWhitelistedProposer(address _proposer) private {
vm.prank(whitelistGuardian);
governor.setWhitelistAccountExpiration(_proposer, block.timestamp + 2_000_000);
}

function _removeDelegateeVotingWeight(address _proposer) private {
vm.mockCall(
address(token),
Expand Down
5 changes: 5 additions & 0 deletions contracts/test/helpers/CompoundGovernorTest.sol
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,11 @@ contract CompoundGovernorTest is Test, CompoundGovernorConstants {
return _majorDelegates[vm.randomUint(0, _majorDelegates.length - 1)];
}

function _setWhitelistedProposer(address _proposer) public {
vm.prank(whitelistGuardian);
governor.setWhitelistAccountExpiration(_proposer, block.timestamp + 2_000_000);
}

function _submitProposal(Proposal memory _proposal) public returns (uint256 _proposalId) {
vm.prank(_getRandomProposer());
_proposalId = governor.propose(_proposal.targets, _proposal.values, _proposal.calldatas, _proposal.description);
Expand Down
Loading