Add cancel logic and tests #36
Conversation
| } | ||
|
|
||
| function _removeDelegateeVotingWeight() private { | ||
| vm.prank(COMPOUND_COMPTROLLER); |
There was a problem hiding this comment.
what's going on here?
There was a problem hiding this comment.
Reimplemented #36 (comment)
| _randomIndex = bound(_randomIndex, 0, _majorDelegates.length - 1); | ||
| address _proposer = _majorDelegates[_randomIndex]; | ||
| Proposal memory _proposal = _buildAnEmptyProposal(); | ||
| uint256 _proposalId = governor.hashProposal( |
There was a problem hiding this comment.
here and elsewhere, can you refactor calls to governor.hashProposal so that it reads from an internal method here _getProposalId? in that method, we'll call governor.hashProposal for now, but when @jferas's enumerable proposalIds comes down, we can make a oneline change to support
There was a problem hiding this comment.
makes sense, made the changes in e01c34e
| ); | ||
| vm.prank(whitelistGuardian); | ||
| governor.setWhitelistAccountExpiration(delegatee, block.timestamp + 2_000_000); | ||
| vm.assertTrue(governor.isWhitelisted(delegatee)); |
There was a problem hiding this comment.
this isn't a test for whether setWhitelist... works, so this assertion can be removed
| _proposal.targets, _proposal.values, _proposal.calldatas, keccak256(bytes(_proposal.description)) | ||
| ); | ||
| vm.prank(whitelistGuardian); | ||
| governor.setWhitelistAccountExpiration(delegatee, block.timestamp + 2_000_000); |
There was a problem hiding this comment.
can we refactor lines 76-77 to internal helper _setWhitelistedProposer(_proposer)?
can we fuzz proposer here? i see the problem with fuzzing proposer -- later, we have to remove the delegatee voting weight. ideally we could do that for any address, not just delegatee. is it difficult?
There was a problem hiding this comment.
Was not difficult using mockCall cheatcode. Now fuzzing the proposer in 9bd64df
| ); | ||
| } | ||
|
|
||
| function testFuzz_RevertIf_AnyoneCancelsWhitelistedProposalAboveThreshold(address _caller, uint256 _randomIndex) |
There was a problem hiding this comment.
| function testFuzz_RevertIf_AnyoneCancelsWhitelistedProposalAboveThreshold(address _caller, uint256 _randomIndex) | |
| function testFuzz_RevertIf_NonProposerOrGuardianCancelsWhitelistedProposalAboveThreshold(address _caller, uint256 _randomIndex) |
contracts/CompoundGovernor.sol
Outdated
| @@ -116,6 +116,37 @@ contract CompoundGovernor is | |||
| _setProposalGuardian(_proposalGuardian); | |||
| } | |||
|
|
|||
| /// @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 the whitelist guardian can | |||
There was a problem hiding this comment.
| /// power has dropped below the proposal threshold. For whitelisted proposers, only the whitelist guardian can | |
| /// power has dropped below the proposal threshold. For whitelisted proposers, only special actors (proposer, proposer guardian, whitelist guardian) can |
| /// @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( |
wildmolasses
changed the base branch from
feat/add-whitelists-and-guardians
to
main
garyghayrat
force-pushed
the
feat/add-custom-cancel-logic
branch
from
bafcd0f to
cc14c23
Compare
Pull Request Test Coverage Report for Build 11483898126Details
💛 - Coveralls |
* Add cancel logic and tests * Use `_getProposalId` instead of `hashProposal` to prep for upcoming change * Rename and cleanup test * Use helpers to set whitelist accounts and remove voting weight * Clean up test * Update natspec * docstring nit --------- Co-authored-by: Ed Mazurek <[email protected]>
closes #28