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.

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

Add errors and update quorum calculation #81

Merged
merged 3 commits into from
Mar 4, 2025
Merged

Add errors and update quorum calculation #81

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d083948
Fix vote succeeded for approval voting module
corydickson Jan 31, 2025
b843113
Add error messages into the governor and implement quorum reached cal…
corydickson Jan 31, 2025
d864191
Fix tests and remove quorum calculation error
corydickson Feb 6, 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
27 changes: 15 additions & 12 deletions src/AgoraGovernor.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ contract AgoraGovernor is

error GovernorUnauthorizedCancel();
error HookAddressNotValid();
error InvalidProposalIdHook();
error InvalidVoteWeightHook();
error InvalidVoteSucceededHook();

/*//////////////////////////////////////////////////////////////
STORAGE
Expand Down Expand Up @@ -113,7 +116,7 @@ contract AgoraGovernor is

uint256 afterProposalId = hooks.afterPropose(proposalId, targets, values, calldatas, description);

// TODO: check that before and after proposal IDs are the same
if (beforeProposalId != afterProposalId) revert InvalidProposalIdHook();

// TODO: replace this with a flag on hook address
return beforeProposalId != 0 ? beforeProposalId : proposalId;
Expand All @@ -131,7 +134,7 @@ contract AgoraGovernor is

uint256 afterProposalId = hooks.afterQueue(proposalId, targets, values, calldatas, descriptionHash);

// TODO: check that before and after proposal IDs are the same
if (beforeProposalId != afterProposalId) revert InvalidProposalIdHook();

// TODO: replace this with a flag on hook address
return beforeProposalId != 0 ? beforeProposalId : proposalId;
Expand All @@ -146,9 +149,10 @@ contract AgoraGovernor is
uint256 beforeProposalId = hooks.beforeExecute(targets, values, calldatas, descriptionHash);

uint256 proposalId = super.execute(targets, values, calldatas, descriptionHash);

uint256 afterProposalId = hooks.afterExecute(proposalId, targets, values, calldatas, descriptionHash);

// TODO: check that before and after proposal IDs are the same
if (beforeProposalId != afterProposalId) revert InvalidProposalIdHook();

// TODO: replace this with a flag on hook address
return beforeProposalId != 0 ? beforeProposalId : proposalId;
Expand Down Expand Up @@ -178,7 +182,7 @@ contract AgoraGovernor is

uint256 afterProposalId = hooks.afterCancel(proposalId, targets, values, calldatas, descriptionHash);

// TODO: check that before and after proposal IDs are the same
if (beforeProposalId != afterProposalId) revert InvalidProposalIdHook();

// TODO: replace this with a flag on hook address
return beforeProposalId != 0 ? beforeProposalId : proposalId;
Expand All @@ -204,8 +208,6 @@ contract AgoraGovernor is
// Call the hook after quorum calculation
uint256 afterQuorum = hooks.afterQuorumCalculation(proposalId, _quorum);

// TODO: check that before and after quorum are the same

// TODO: replace this with a flag on hook address
return beforeQuorum != 0 ? beforeQuorum : _quorum;
}
Expand Down Expand Up @@ -256,7 +258,7 @@ contract AgoraGovernor is

uint256 afterWeight = hooks.afterVote(votedWeight, proposalId, account, support, reason, params);

// TODO: check that before and after weights are the same
if (beforeWeight != afterWeight) revert InvalidVoteWeightHook();

// TODO: replace this with a flag on hook address
return beforeWeight != 0 ? beforeWeight : votedWeight;
Expand Down Expand Up @@ -299,10 +301,11 @@ contract AgoraGovernor is
{
(uint256 againstVotes, uint256 forVotes, uint256 abstainVotes) = proposalVotes(proposalId);

// uint256 defaultQuorum = this.quorum()
// uint256 x = afterQuorum(against, for, abstain, defaultQuorum);
// return x == 0 ? defaultQuorum :
return quorum(proposalId) <= againstVotes + forVotes + abstainVotes;
uint256 defaultQuorum = quorum(proposalId);
uint256 afterQuorum = hooks.afterQuorumCalculation(proposalId, defaultQuorum);
return afterQuorum == 0
? (defaultQuorum <= againstVotes + forVotes + abstainVotes)
: (afterQuorum <= againstVotes + forVotes + abstainVotes);
}

/**
Expand All @@ -321,7 +324,7 @@ contract AgoraGovernor is

uint8 afterVoteSucceeded = hooks.afterVoteSucceeded(proposalId, voteSucceeded);

// TODO: check that before and after vote succeeded are the same
if (beforeVoteSucceeded != afterVoteSucceeded) revert InvalidVoteSucceededHook();

// TODO: replace this with a flag on hook address because it returns false by default!
return beforeVoteSucceeded != 0 ? beforeVoteSucceeded == 2 : voteSucceeded;
Expand Down
10 changes: 4 additions & 6 deletions src/Middleware.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {Bytes} from "@openzeppelin/contracts/utils/Bytes.sol";
import {Strings} from "@openzeppelin/contracts/utils/Strings.sol";
import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol";
import {Validator} from "src/libraries/Validator.sol";
import {ApprovalVotingModule} from "src/modules/ApprovalVoting.sol";

contract Middleware is IMiddleware, BaseHook {
using Hooks for IHooks;
Expand Down Expand Up @@ -167,11 +168,8 @@ contract Middleware is IMiddleware, BaseHook {
uint8 proposalTypeId = _proposalTypeId[proposalId];
address votingModule = _proposalTypes[proposalTypeId].module;
if (votingModule != address(0)) {
/*
if (!VotingModule(votingModule)._voteSucceeded(proposalId)) {
return false;
}
*/
(bytes4 selector, bool success) = ApprovalVotingModule(votingModule).beforeVoteSucceeded(sender, proposalId);
return (selector, success);
}

uint256 approvalThreshold = _proposalTypes[proposalTypeId].approvalThreshold;
Expand All @@ -185,7 +183,7 @@ contract Middleware is IMiddleware, BaseHook {
voteSucceeded = (forVotes * PERCENT_DIVISOR) / totalVotes >= approvalThreshold;
}

return (this.afterPropose.selector, voteSucceeded);
return (this.beforeVoteSucceeded.selector, voteSucceeded);
}

/*//////////////////////////////////////////////////////////////
Expand Down
33 changes: 20 additions & 13 deletions test/mocks/BaseHookMock.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ contract BaseHookMock is BaseHook {
return (this.afterVoteSucceeded.selector, true);
}

function beforeQuorumCalculation(address, uint256) external view virtual override returns (bytes4, uint256) {
function beforeQuorumCalculation(address, uint256 quorum) external view virtual override returns (bytes4, uint256) {
return (this.beforeQuorumCalculation.selector, 100);
}

Expand All @@ -53,35 +53,38 @@ contract BaseHookMock is BaseHook {
return (this.afterQuorumCalculation.selector, afterQuorum);
}

function beforeVote(address, uint256, address, uint8, string memory, bytes memory)
function beforeVote(address, uint256, address, uint8 support, string memory, bytes memory)
external
virtual
override
returns (bytes4, uint256)
{
emit BeforeVote();
return (this.beforeVote.selector, 299);
return (this.beforeVote.selector, uint256(support));
}

function afterVote(address, uint256, uint256, address, uint8, string memory, bytes memory)
function afterVote(address, uint256, uint256, address, uint8 support, string memory, bytes memory)
external
virtual
override
returns (bytes4, uint256)
{
emit AfterVote();
return (this.afterVote.selector, 0);
return (this.afterVote.selector, uint256(support));
}

function beforePropose(address, address[] memory, uint256[] memory, bytes[] memory, string memory)
function beforePropose(address, address[] memory targets, uint256[] memory values, bytes[] memory calldatas, string
memory description)
external
virtual
override
returns (bytes4, uint256)
{
emit BeforePropose();
uint256 proposalId = governor.hashProposal(targets, values, calldatas,
keccak256(abi.encodePacked(description)));

return (this.beforePropose.selector, 0);
return (this.beforePropose.selector, proposalId);
}

function afterPropose(
Expand All @@ -96,24 +99,26 @@ contract BaseHookMock is BaseHook {
return (this.afterPropose.selector, proposalId);
}

function beforeCancel(address, address[] memory, uint256[] memory, bytes[] memory, bytes32)
function beforeCancel(address, address[] memory targets, uint256[] memory values, bytes[] memory calldatas, bytes32
descriptionHash)
external
virtual
override
returns (bytes4, uint256)
{
emit BeforeCancel();
return (this.beforeCancel.selector, 0);
uint256 proposalId = governor.hashProposal(targets, values, calldatas, descriptionHash);
return (this.beforeCancel.selector, proposalId);
}

function afterCancel(address, uint256, address[] memory, uint256[] memory, bytes[] memory, bytes32)
function afterCancel(address, uint256 proposalId, address[] memory, uint256[] memory, bytes[] memory, bytes32)
external
virtual
override
returns (bytes4, uint256)
{
emit AfterCancel();
return (this.afterCancel.selector, 0);
return (this.afterCancel.selector, proposalId);
}

function beforeQueue(address, address[] memory, uint256[] memory, bytes[] memory, bytes32)
Expand All @@ -136,14 +141,16 @@ contract BaseHookMock is BaseHook {
return (this.afterQueue.selector, 0);
}

function beforeExecute(address, address[] memory, uint256[] memory, bytes[] memory, bytes32)
function beforeExecute(address, address[] memory targets, uint256[] memory values, bytes[] memory calldatas, bytes32
descriptionHash)
external
virtual
override
returns (bytes4, uint256)
{
emit BeforeExecute();
return (this.beforeExecute.selector, 0);
uint256 proposalId = governor.hashProposal(targets, values, calldatas, descriptionHash);
return (this.beforeExecute.selector, proposalId);
}

function afterExecute(address, uint256 proposalId, address[] memory, uint256[] memory, bytes[] memory, bytes32)
Expand Down
Loading