From 13790c26055d275a658a5918167d129660140fea Mon Sep 17 00:00:00 2001 From: Lasse Herskind <16536249+LHerskind@users.noreply.github.com> Date: Wed, 9 Oct 2024 19:14:53 +0100 Subject: [PATCH] test: More tests + cleanup --- l1-contracts/.solhint.json | 10 +- l1-contracts/src/governance/Apella.sol | 93 +++-- .../src/governance/interfaces/IApella.sol | 7 +- .../governance/libraries/ConfigurationLib.sol | 49 ++- .../governance/libraries/DataStructures.sol | 2 +- .../src/governance/libraries/Errors.sol | 15 +- .../src/governance/libraries/ProposalLib.sol | 102 +++-- .../src/governance/libraries/UserLib.sol | 6 +- l1-contracts/test/base/Base.sol | 12 +- .../test/governance/apella/TestPayloads.sol | 79 ++++ .../test/governance/apella/base.t.sol | 228 ++++++++++- .../test/governance/apella/dropProposal.t.sol | 136 +++++++ .../test/governance/apella/dropProposal.tree | 25 ++ .../test/governance/apella/execute.t.sol | 183 +++++++-- .../test/governance/apella/execute.tree | 15 +- .../governance/apella/getProposalState.t.sol | 364 ++++++++++------- .../governance/apella/getProposalState.tree | 53 ++- .../governance/apella/initiateWithdraw.t.sol | 2 +- .../apella/proposallib/isRejected.tree | 32 -- .../apella/proposallib/isStableState.t.sol | 24 +- .../apella/proposallib/isStableState.tree | 4 - .../apella/proposallib/static.t.sol | 16 +- ...{isRejected.t.sol => voteTabulation.t.sol} | 137 ++++--- .../apella/proposallib/voteTabulation.tree | 32 ++ .../test/governance/apella/propose.t.sol | 59 ++- .../apella/scenarios/noVoteAndExit.t.sol | 76 ++++ .../apella/updateConfiguration.t.sol | 367 +++++++++++++++++- .../apella/updateConfiguration.tree | 38 +- .../governance/apella/updateGerousia.t.sol | 33 ++ .../governance/apella/updateGerousia.tree | 6 + .../governance/apella/userlib/powerAt.t.sol | 2 +- .../test/governance/apella/userlib/sub.t.sol | 4 +- .../test/governance/apella/vote.t.sol | 194 +++++++-- l1-contracts/test/governance/apella/vote.tree | 29 +- 34 files changed, 1950 insertions(+), 484 deletions(-) create mode 100644 l1-contracts/test/governance/apella/TestPayloads.sol create mode 100644 l1-contracts/test/governance/apella/dropProposal.t.sol create mode 100644 l1-contracts/test/governance/apella/dropProposal.tree delete mode 100644 l1-contracts/test/governance/apella/proposallib/isRejected.tree rename l1-contracts/test/governance/apella/proposallib/{isRejected.t.sol => voteTabulation.t.sol} (60%) create mode 100644 l1-contracts/test/governance/apella/proposallib/voteTabulation.tree create mode 100644 l1-contracts/test/governance/apella/scenarios/noVoteAndExit.t.sol create mode 100644 l1-contracts/test/governance/apella/updateGerousia.t.sol create mode 100644 l1-contracts/test/governance/apella/updateGerousia.tree diff --git a/l1-contracts/.solhint.json b/l1-contracts/.solhint.json index 970f17dbcc50..3c14ae3cf163 100644 --- a/l1-contracts/.solhint.json +++ b/l1-contracts/.solhint.json @@ -20,10 +20,10 @@ "error" ], "not-rely-on-time": "off", - "const-name-snakecase": [ - "error", + "immutable-vars-naming": [ + "warn", { - "treatImmutableVarAsConstant": true + "immutablesAsConstants": true } ], "var-name-mixedcase": [ @@ -47,6 +47,7 @@ "func-param-name-leading-underscore": [ "error" ], + "interface-starts-with-i": "warn", "func-param-name-mixedcase": [ "error" ], @@ -62,6 +63,7 @@ "comprehensive-interface": [ "error" ], - "custom-error-over-require": "off" + "custom-error-over-require": "off", + "no-unused-import": "error" } } \ No newline at end of file diff --git a/l1-contracts/src/governance/Apella.sol b/l1-contracts/src/governance/Apella.sol index bd3f2ca3ce35..da05aa10e3c5 100644 --- a/l1-contracts/src/governance/Apella.sol +++ b/l1-contracts/src/governance/Apella.sol @@ -7,7 +7,7 @@ import {IApella} from "@aztec/governance/interfaces/IApella.sol"; import {DataStructures} from "@aztec/governance/libraries/DataStructures.sol"; import {ConfigurationLib} from "@aztec/governance/libraries/ConfigurationLib.sol"; import {Errors} from "@aztec/governance/libraries/Errors.sol"; -import {ProposalLib} from "@aztec/governance/libraries/ProposalLib.sol"; +import {ProposalLib, VoteTabulationReturn} from "@aztec/governance/libraries/ProposalLib.sol"; import {UserLib} from "@aztec/governance/libraries/UserLib.sol"; import {Timestamp} from "@aztec/core/libraries/TimeMath.sol"; @@ -25,16 +25,15 @@ contract Apella is IApella { address public gerousia; - DataStructures.Configuration internal configuration; - - uint256 public proposalCount; mapping(uint256 proposalId => DataStructures.Proposal) internal proposals; mapping(uint256 proposalId => mapping(address user => DataStructures.Ballot)) public ballots; mapping(address => DataStructures.User) internal users; - DataStructures.User internal total; + mapping(uint256 withdrawalId => DataStructures.Withdrawal) internal withdrawals; + DataStructures.Configuration internal configuration; + DataStructures.User internal total; + uint256 public proposalCount; uint256 public withdrawalCount; - mapping(uint256 withdrawalId => DataStructures.Withdrawal) internal withdrawals; constructor(IERC20 _asset, address _gerousia) { ASSET = _asset; @@ -52,16 +51,10 @@ contract Apella is IApella { configuration.assertValid(); } - function getConfiguration() external view returns (DataStructures.Configuration memory) { - return configuration; - } - - function getWithdrawal(uint256 _withdrawalId) - external - view - returns (DataStructures.Withdrawal memory) - { - return withdrawals[_withdrawalId]; + function updateGerousia(address _gerousia) external override(IApella) { + require(msg.sender == address(this), Errors.Apella__CallerNotSelf(msg.sender, address(this))); + gerousia = _gerousia; + emit GerousiaUpdated(_gerousia); } function updateConfiguration(DataStructures.Configuration memory _configuration) @@ -69,8 +62,13 @@ contract Apella is IApella { override(IApella) { require(msg.sender == address(this), Errors.Apella__CallerNotSelf(msg.sender, address(this))); - require(_configuration.assertValid(), Errors.Apella__InvalidConfiguration()); + + // This following MUST revert if the configuration is invalid + _configuration.assertValid(); + configuration = _configuration; + + emit ConfigurationUpdated(Timestamp.wrap(block.timestamp)); } function deposit(address _onBehalfOf, uint256 _amount) external override(IApella) { @@ -81,7 +79,11 @@ contract Apella is IApella { emit Deposit(msg.sender, _onBehalfOf, _amount); } - function initiateWithdraw(address _to, uint256 _amount) external override(IApella) { + function initiateWithdraw(address _to, uint256 _amount) + external + override(IApella) + returns (uint256) + { users[msg.sender].sub(_amount); total.sub(_amount); @@ -95,6 +97,8 @@ contract Apella is IApella { }); emit WithdrawInitiated(withdrawalId, _to, _amount); + + return withdrawalId; } function finaliseWithdraw(uint256 _withdrawalId) external override(IApella) { @@ -139,12 +143,15 @@ contract Apella is IApella { require(state == DataStructures.ProposalState.Active, Errors.Apella__ProposalNotActive()); // Compute the power at the time where we became active - uint256 userPower = users[msg.sender].powerAt(proposals[_proposalId].pendingUntil()); + uint256 userPower = users[msg.sender].powerAt(proposals[_proposalId].pendingThrough()); DataStructures.Ballot storage userBallot = ballots[_proposalId][msg.sender]; uint256 availablePower = userPower - (userBallot.nea + userBallot.yea); - require(_amount <= availablePower, Errors.Apella__InsufficientPower(availablePower, _amount)); + require( + _amount <= availablePower, + Errors.Apella__InsufficientPower(msg.sender, availablePower, _amount) + ); DataStructures.Ballot storage summedBallot = proposals[_proposalId].summedBallot; if (_support) { @@ -178,6 +185,8 @@ contract Apella is IApella { require(success, Errors.Apella__CallFailed(actions[i].target)); } + emit ProposalExecuted(_proposalId); + return true; } @@ -195,6 +204,36 @@ contract Apella is IApella { return total.powerAt(_ts); } + function getConfiguration() external view returns (DataStructures.Configuration memory) { + return configuration; + } + + function getProposal(uint256 _proposalId) external view returns (DataStructures.Proposal memory) { + return proposals[_proposalId]; + } + + function getWithdrawal(uint256 _withdrawalId) + external + view + returns (DataStructures.Withdrawal memory) + { + return withdrawals[_withdrawalId]; + } + + function dropProposal(uint256 _proposalId) external returns (bool) { + DataStructures.Proposal storage self = proposals[_proposalId]; + require( + self.state != DataStructures.ProposalState.Dropped, Errors.Apella__ProposalAlreadyDropped() + ); + require( + getProposalState(_proposalId) == DataStructures.ProposalState.Dropped, + Errors.Apella__ProposalCannotBeDropped() + ); + + self.state = DataStructures.ProposalState.Dropped; + return true; + } + /** * @notice Get the state of the proposal * @@ -215,30 +254,32 @@ contract Apella is IApella { return self.state; } - // If the gerousia have changed drop the old proposals no matter what + // If the gerousia have changed we mark is as dropped if (gerousia != self.creator) { return DataStructures.ProposalState.Dropped; } Timestamp currentTime = Timestamp.wrap(block.timestamp); - if (currentTime < self.pendingUntil()) { + if (currentTime <= self.pendingThrough()) { return DataStructures.ProposalState.Pending; } - if (currentTime < self.activeUntil()) { + if (currentTime <= self.activeThrough()) { return DataStructures.ProposalState.Active; } - if (self.isRejected(total.powerAt(self.pendingUntil()))) { + uint256 totalPower = total.powerAt(self.pendingThrough()); + (VoteTabulationReturn vtr,) = self.voteTabulation(totalPower); + if (vtr != VoteTabulationReturn.Accepted) { return DataStructures.ProposalState.Rejected; } - if (currentTime < self.queuedUntil()) { + if (currentTime <= self.queuedThrough()) { return DataStructures.ProposalState.Queued; } - if (currentTime < self.executableUntil()) { + if (currentTime <= self.executableThrough()) { return DataStructures.ProposalState.Executable; } diff --git a/l1-contracts/src/governance/interfaces/IApella.sol b/l1-contracts/src/governance/interfaces/IApella.sol index c89dd45ad9d7..52a84c2e1b02 100644 --- a/l1-contracts/src/governance/interfaces/IApella.sol +++ b/l1-contracts/src/governance/interfaces/IApella.sol @@ -8,13 +8,18 @@ import {Timestamp} from "@aztec/core/libraries/TimeMath.sol"; interface IApella { event Proposed(uint256 indexed proposalId, address indexed proposal); event VoteCast(uint256 indexed proposalId, address indexed voter, bool support, uint256 amount); + event ProposalExecuted(uint256 indexed proposalId); + event GerousiaUpdated(address indexed gerousia); + event ConfigurationUpdated(Timestamp indexed time); + event Deposit(address indexed depositor, address indexed onBehalfOf, uint256 amount); event WithdrawInitiated(uint256 indexed withdrawalId, address indexed recipient, uint256 amount); event WithdrawFinalised(uint256 indexed withdrawalId); + function updateGerousia(address _gerousia) external; function updateConfiguration(DataStructures.Configuration memory _configuration) external; function deposit(address _onBehalfOf, uint256 _amount) external; - function initiateWithdraw(address _to, uint256 _amount) external; + function initiateWithdraw(address _to, uint256 _amount) external returns (uint256); function finaliseWithdraw(uint256 _withdrawalId) external; function propose(IPayload _proposal) external returns (bool); function vote(uint256 _proposalId, uint256 _amount, bool _support) external returns (bool); diff --git a/l1-contracts/src/governance/libraries/ConfigurationLib.sol b/l1-contracts/src/governance/libraries/ConfigurationLib.sol index 97c2fd11ac44..266e9c83ac75 100644 --- a/l1-contracts/src/governance/libraries/ConfigurationLib.sol +++ b/l1-contracts/src/governance/libraries/ConfigurationLib.sol @@ -11,13 +11,16 @@ library ConfigurationLib { uint256 internal constant QUORUM_LOWER = 1; uint256 internal constant QUORUM_UPPER = 1e18; - uint256 internal constant DIFFERENTIAL_LOWER = 0; uint256 internal constant DIFFERENTIAL_UPPER = 1e18; uint256 internal constant VOTES_LOWER = 1; + Timestamp internal constant TIME_LOWER = Timestamp.wrap(3600); + Timestamp internal constant TIME_UPPER = Timestamp.wrap(30 * 24 * 3600); + function lockDelay(DataStructures.Configuration storage self) internal view returns (Timestamp) { - return self.votingDuration + self.executionDelay; + return Timestamp.wrap(Timestamp.unwrap(self.votingDelay) / 5) + self.votingDuration + + self.executionDelay; } /** @@ -26,23 +29,49 @@ library ConfigurationLib { * before writing it to state. */ function assertValid(DataStructures.Configuration memory self) internal pure returns (bool) { - require(self.quorum >= QUORUM_LOWER, Errors.Apella__ConfigurationLib__ConfigInvalidQuorum()); - require(self.quorum <= QUORUM_UPPER, Errors.Apella__ConfigurationLib__ConfigInvalidQuorum()); + require(self.quorum >= QUORUM_LOWER, Errors.Apella__ConfigurationLib__QuorumTooSmall()); + require(self.quorum <= QUORUM_UPPER, Errors.Apella__ConfigurationLib__QuorumTooBig()); - require( - self.voteDifferential >= DIFFERENTIAL_LOWER, - Errors.Apella__ConfigurationLib__ConfigInvalidDifferential() - ); require( self.voteDifferential <= DIFFERENTIAL_UPPER, - Errors.Apella__ConfigurationLib__ConfigInvalidDifferential() + Errors.Apella__ConfigurationLib__DifferentialTooBig() ); require( self.minimumVotes >= VOTES_LOWER, Errors.Apella__ConfigurationLib__InvalidMinimumVotes() ); - // Some restrictions on the individual sections. + require( + self.votingDelay >= TIME_LOWER, Errors.Apella__ConfigurationLib__TimeTooSmall("VotingDelay") + ); + require( + self.votingDelay <= TIME_UPPER, Errors.Apella__ConfigurationLib__TimeTooBig("VotingDelay") + ); + + require( + self.votingDuration >= TIME_LOWER, + Errors.Apella__ConfigurationLib__TimeTooSmall("VotingDuration") + ); + require( + self.votingDuration <= TIME_UPPER, + Errors.Apella__ConfigurationLib__TimeTooBig("VotingDuration") + ); + + require( + self.executionDelay >= TIME_LOWER, + Errors.Apella__ConfigurationLib__TimeTooSmall("ExecutionDelay") + ); + require( + self.executionDelay <= TIME_UPPER, + Errors.Apella__ConfigurationLib__TimeTooBig("ExecutionDelay") + ); + + require( + self.gracePeriod >= TIME_LOWER, Errors.Apella__ConfigurationLib__TimeTooSmall("GracePeriod") + ); + require( + self.gracePeriod <= TIME_UPPER, Errors.Apella__ConfigurationLib__TimeTooBig("GracePeriod") + ); return true; } diff --git a/l1-contracts/src/governance/libraries/DataStructures.sol b/l1-contracts/src/governance/libraries/DataStructures.sol index 4ab2d24ed62f..821fadc4e524 100644 --- a/l1-contracts/src/governance/libraries/DataStructures.sol +++ b/l1-contracts/src/governance/libraries/DataStructures.sol @@ -50,7 +50,7 @@ library DataStructures { } struct Proposal { - DataStructures.Configuration config; + Configuration config; ProposalState state; IPayload payload; address creator; diff --git a/l1-contracts/src/governance/libraries/Errors.sol b/l1-contracts/src/governance/libraries/Errors.sol index 47c7221a6828..cb06f44342b8 100644 --- a/l1-contracts/src/governance/libraries/Errors.sol +++ b/l1-contracts/src/governance/libraries/Errors.sol @@ -16,8 +16,7 @@ library Errors { error Apella__CallerNotGerousia(address caller, address gerousia); error Apella__CallerNotSelf(address caller, address self); error Apella__NoCheckpointsFound(); - error Apella__InsufficientPower(uint256 have, uint256 required); - error Apella__NotInPast(); + error Apella__InsufficientPower(address voter, uint256 have, uint256 required); error Apella__InvalidConfiguration(); error Apella__WithdrawalAlreadyclaimed(); error Apella__WithdrawalNotUnlockedYet(Timestamp currentTime, Timestamp unlocksAt); @@ -26,10 +25,18 @@ library Errors { error Apella__CannotCallAsset(); error Apella__CallFailed(address target); error Apella__ProposalDoesNotExists(uint256 proposalId); + error Apella__ProposalAlreadyDropped(); + error Apella__ProposalCannotBeDropped(); + + error Apella__UserLib__NotInPast(); error Apella__ConfigurationLib__InvalidMinimumVotes(); - error Apella__ConfigurationLib__ConfigInvalidQuorum(); - error Apella__ConfigurationLib__ConfigInvalidDifferential(); + error Apella__ConfigurationLib__QuorumTooSmall(); + error Apella__ConfigurationLib__QuorumTooBig(); + error Apella__ConfigurationLib__DifferentialTooSmall(); + error Apella__ConfigurationLib__DifferentialTooBig(); + error Apella__ConfigurationLib__TimeTooSmall(string name); + error Apella__ConfigurationLib__TimeTooBig(string name); error Apella__ProposalLib__ZeroMinimum(); error Apella__ProposalLib__ZeroVotesNeeded(); diff --git a/l1-contracts/src/governance/libraries/ProposalLib.sol b/l1-contracts/src/governance/libraries/ProposalLib.sol index a2bc4eefa155..4a72201a887e 100644 --- a/l1-contracts/src/governance/libraries/ProposalLib.sol +++ b/l1-contracts/src/governance/libraries/ProposalLib.sol @@ -4,7 +4,26 @@ pragma solidity >=0.8.27; import {DataStructures} from "@aztec/governance/libraries/DataStructures.sol"; import {Timestamp} from "@aztec/core/libraries/TimeMath.sol"; import {Math} from "@oz/utils/math/Math.sol"; -import {Errors} from "@aztec/governance/libraries/Errors.sol"; + +enum VoteTabulationReturn { + Accepted, + Rejected, + Invalid +} + +enum VoteTabulationInfo { + MinimumEqZero, + TotalPowerLtMinimum, + VotesNeededEqZero, + VotesNeededGtTotalPower, + VotesCastLtVotesNeeded, + YeaLimitEqZero, + YeaLimitGtVotesCast, + YeaLimitEqVotesCast, + YeaVotesEqVotesCast, + YeaVotesLeYeaLimit, + YeaVotesGtYeaLimit +} /** * @notice Library for dealing with proposal math @@ -23,78 +42,87 @@ import {Errors} from "@aztec/governance/libraries/Errors.sol"; * for example ending at 0, having a case where no votes are needed */ library ProposalLib { - /** - * @notice Compute if we reject the proposal or not - * Can - * - * Should only ever revert, if "bad" values are setup for the config. - */ - function isRejected(DataStructures.Proposal storage self, uint256 _totalPower) + function voteTabulation(DataStructures.Proposal storage self, uint256 _totalPower) internal view - returns (bool) + returns (VoteTabulationReturn, VoteTabulationInfo) { - require(self.config.minimumVotes > 0, Errors.Apella__ProposalLib__ZeroMinimum()); + if (self.config.minimumVotes == 0) { + return (VoteTabulationReturn.Invalid, VoteTabulationInfo.MinimumEqZero); + } if (_totalPower < self.config.minimumVotes) { - return true; + return (VoteTabulationReturn.Rejected, VoteTabulationInfo.TotalPowerLtMinimum); } uint256 votesNeeded = Math.mulDiv(_totalPower, self.config.quorum, 1e18, Math.Rounding.Ceil); - require(votesNeeded > 0, Errors.Apella__ProposalLib__ZeroVotesNeeded()); - require(votesNeeded <= _totalPower, Errors.Apella__ProposalLib__MoreVoteThanExistNeeded()); + if (votesNeeded == 0) { + return (VoteTabulationReturn.Invalid, VoteTabulationInfo.VotesNeededEqZero); + } + if (votesNeeded > _totalPower) { + return (VoteTabulationReturn.Invalid, VoteTabulationInfo.VotesNeededGtTotalPower); + } uint256 votesCast = self.summedBallot.nea + self.summedBallot.yea; if (votesCast < votesNeeded) { - // Insufficient votes cast, better luck next time - return true; + return (VoteTabulationReturn.Rejected, VoteTabulationInfo.VotesCastLtVotesNeeded); } // Edge case where all the votes are yea, no need to compute differential // Assumes a "sane" value for differential, e.g., you cannot require more votes // to be yes than total votes. if (self.summedBallot.yea == votesCast) { - return false; + return (VoteTabulationReturn.Accepted, VoteTabulationInfo.YeaVotesEqVotesCast); } - uint256 yeaNeededFraction = Math.ceilDiv(1e18 + self.config.voteDifferential, 2); - uint256 yeaNeeded = Math.mulDiv(votesCast, yeaNeededFraction, 1e18, Math.Rounding.Ceil); - require(yeaNeeded > 0, Errors.Apella__ProposalLib__ZeroYeaVotesNeeded()); - require(yeaNeeded <= votesCast, Errors.Apella__ProposalLib__MoreYeaVoteThanExistNeeded()); + uint256 yeaLimitFraction = Math.ceilDiv(1e18 + self.config.voteDifferential, 2); + uint256 yeaLimit = Math.mulDiv(votesCast, yeaLimitFraction, 1e18, Math.Rounding.Ceil); - // If we need as many yea as there are votes, we know it is impossible already. - // due to the check earlier. - if (yeaNeeded == votesCast) { - return true; + /*if (yeaLimit == 0) { + // It should be impossible to hit this case as `yeaLimitFraction` cannot be 0, + // and due to rounding, only way to hit this would be if `votesCast = 0`, + // which is already handled as `votesCast >= votesNeeded` and `votesNeeded > 0`. + return (VoteTabulationReturn.Invalid, VoteTabulationInfo.YeaLimitEqZero); + }*/ + if (yeaLimit > votesCast) { + return (VoteTabulationReturn.Invalid, VoteTabulationInfo.YeaLimitGtVotesCast); } - // In all other cases, we want to see that there are MORE votes on yea than needed + // We want to see that there are MORE votes on yea than needed // We explictly need MORE to ensure we don't "tie". - return self.summedBallot.yea <= yeaNeeded; + // If we need as many yea as there are votes, we know it is impossible already. + // due to the check earlier, that summedBallot.yea == votesCast. + if (self.summedBallot.yea <= yeaLimit) { + return (VoteTabulationReturn.Rejected, VoteTabulationInfo.YeaVotesLeYeaLimit); + } + + return (VoteTabulationReturn.Accepted, VoteTabulationInfo.YeaVotesGtYeaLimit); } /** * @notice A stable state is one which cannoted be moved away from */ function isStable(DataStructures.Proposal storage self) internal view returns (bool) { - return self.state == DataStructures.ProposalState.Executed - || self.state == DataStructures.ProposalState.Dropped - || self.state == DataStructures.ProposalState.Expired - || self.state == DataStructures.ProposalState.Rejected; + DataStructures.ProposalState s = self.state; // cache + return s == DataStructures.ProposalState.Executed || s == DataStructures.ProposalState.Dropped; } - function pendingUntil(DataStructures.Proposal storage self) internal view returns (Timestamp) { + function pendingThrough(DataStructures.Proposal storage self) internal view returns (Timestamp) { return self.creation + self.config.votingDelay; } - function activeUntil(DataStructures.Proposal storage self) internal view returns (Timestamp) { - return ProposalLib.pendingUntil(self) + self.config.votingDuration; + function activeThrough(DataStructures.Proposal storage self) internal view returns (Timestamp) { + return ProposalLib.pendingThrough(self) + self.config.votingDuration; } - function queuedUntil(DataStructures.Proposal storage self) internal view returns (Timestamp) { - return ProposalLib.activeUntil(self) + self.config.executionDelay; + function queuedThrough(DataStructures.Proposal storage self) internal view returns (Timestamp) { + return ProposalLib.activeThrough(self) + self.config.executionDelay; } - function executableUntil(DataStructures.Proposal storage self) internal view returns (Timestamp) { - return ProposalLib.queuedUntil(self) + self.config.gracePeriod; + function executableThrough(DataStructures.Proposal storage self) + internal + view + returns (Timestamp) + { + return ProposalLib.queuedThrough(self) + self.config.gracePeriod; } } diff --git a/l1-contracts/src/governance/libraries/UserLib.sol b/l1-contracts/src/governance/libraries/UserLib.sol index 2b519a2151db..bc3ca1f7d341 100644 --- a/l1-contracts/src/governance/libraries/UserLib.sol +++ b/l1-contracts/src/governance/libraries/UserLib.sol @@ -34,7 +34,9 @@ library UserLib { } require(self.numCheckPoints > 0, Errors.Apella__NoCheckpointsFound()); DataStructures.CheckPoint storage last = self.checkpoints[self.numCheckPoints - 1]; - require(last.power >= _amount, Errors.Apella__InsufficientPower(last.power, _amount)); + require( + last.power >= _amount, Errors.Apella__InsufficientPower(msg.sender, last.power, _amount) + ); if (last.time == Timestamp.wrap(block.timestamp)) { last.power -= _amount; } else { @@ -61,7 +63,7 @@ library UserLib { { // If not in the past, the values are not stable. // We disallow using it to avoid potential misuse. - require(_time < Timestamp.wrap(block.timestamp), Errors.Apella__NotInPast()); + require(_time < Timestamp.wrap(block.timestamp), Errors.Apella__UserLib__NotInPast()); uint256 numCheckPoints = self.numCheckPoints; if (numCheckPoints == 0) { diff --git a/l1-contracts/test/base/Base.sol b/l1-contracts/test/base/Base.sol index 047cc5b9ccff..f05cef62d5e7 100644 --- a/l1-contracts/test/base/Base.sol +++ b/l1-contracts/test/base/Base.sol @@ -29,14 +29,14 @@ contract TestBase is Test { function assertGt(Timestamp a, Timestamp b, string memory err) internal { if (a <= b) { emit log_named_string("Error", err); - assertEq(a, b); + assertGt(a, b); } } function assertGt(Timestamp a, uint256 b, string memory err) internal { if (a <= Timestamp.wrap(b)) { emit log_named_string("Error", err); - assertEq(a, b); + assertGt(a, b); } } @@ -61,14 +61,14 @@ contract TestBase is Test { function assertLe(Timestamp a, Timestamp b, string memory err) internal { if (a > b) { emit log_named_string("Error", err); - assertEq(a, b); + assertLe(a, b); } } function assertLe(Timestamp a, uint256 b, string memory err) internal { if (a > Timestamp.wrap(b)) { emit log_named_string("Error", err); - assertEq(a, b); + assertLe(a, b); } } @@ -93,14 +93,14 @@ contract TestBase is Test { function assertLt(Timestamp a, Timestamp b, string memory err) internal { if (a >= b) { emit log_named_string("Error", err); - assertEq(a, b); + assertLt(a, b); } } function assertLt(Timestamp a, uint256 b, string memory err) internal { if (a >= Timestamp.wrap(b)) { emit log_named_string("Error", err); - assertEq(a, b); + assertLt(a, b); } } diff --git a/l1-contracts/test/governance/apella/TestPayloads.sol b/l1-contracts/test/governance/apella/TestPayloads.sol new file mode 100644 index 000000000000..1d541015ab02 --- /dev/null +++ b/l1-contracts/test/governance/apella/TestPayloads.sol @@ -0,0 +1,79 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.27; + +import {IPayload} from "@aztec/governance/interfaces/IPayload.sol"; + +import {IMintableERC20} from "@aztec/governance/interfaces/IMintableERC20.sol"; +import {IRegistry} from "@aztec/governance/interfaces/IRegistry.sol"; + +contract EmptyPayload is IPayload { + function getActions() external view override(IPayload) returns (IPayload.Action[] memory) {} +} + +contract CallAssetPayload is IPayload { + IMintableERC20 internal immutable ASSET; + address internal immutable OWNER; + address internal immutable APELLA; + + constructor(IMintableERC20 _asset, address _apella) { + ASSET = _asset; + OWNER = msg.sender; + APELLA = _apella; + } + + function getActions() external view override(IPayload) returns (IPayload.Action[] memory) { + IPayload.Action[] memory res = new IPayload.Action[](1); + uint256 balance = ASSET.balanceOf(APELLA); + + res[0] = Action({ + target: address(ASSET), + data: abi.encodeWithSelector(ASSET.transfer.selector, OWNER, balance) + }); + + return res; + } +} + +contract UpgradePayload is IPayload { + IRegistry public immutable REGISTRY; + address public constant NEW_ROLLUP = + address(uint160(uint256(keccak256(bytes("a new fancy rollup built with magic"))))); + + constructor(IRegistry _registry) { + REGISTRY = _registry; + } + + function getActions() external view override(IPayload) returns (IPayload.Action[] memory) { + IPayload.Action[] memory res = new IPayload.Action[](1); + + res[0] = Action({ + target: address(REGISTRY), + data: abi.encodeWithSelector(REGISTRY.upgrade.selector, NEW_ROLLUP) + }); + + return res; + } +} + +contract CallRevertingPayload is IPayload { + RevertingCall public immutable TARGET = new RevertingCall(); + + function getActions() external view override(IPayload) returns (IPayload.Action[] memory) { + IPayload.Action[] memory res = new IPayload.Action[](1); + + res[0] = Action({ + target: address(TARGET), + data: abi.encodeWithSelector(TARGET.skibBobFlipFlop.selector) + }); + + return res; + } +} + +contract RevertingCall { + error TrapCardActivated(); + + function skibBobFlipFlop() external pure { + revert TrapCardActivated(); + } +} diff --git a/l1-contracts/test/governance/apella/base.t.sol b/l1-contracts/test/governance/apella/base.t.sol index 3ac56fc2928b..fbb367735089 100644 --- a/l1-contracts/test/governance/apella/base.t.sol +++ b/l1-contracts/test/governance/apella/base.t.sol @@ -5,28 +5,248 @@ import {TestBase} from "@test/base/Base.sol"; import {Apella} from "@aztec/governance/Apella.sol"; import {Gerousia} from "@aztec/governance/Gerousia.sol"; import {Registry} from "@aztec/governance/Registry.sol"; - +import {DataStructures} from "@aztec/governance/libraries/DataStructures.sol"; import {IMintableERC20} from "@aztec/governance/interfaces/IMintableERC20.sol"; import {TestERC20} from "@aztec/mock/TestERC20.sol"; +import {Timestamp} from "@aztec/core/libraries/TimeMath.sol"; +import {Math} from "@oz/utils/math/Math.sol"; + +import { + ProposalLib, + VoteTabulationReturn, + VoteTabulationInfo +} from "@aztec/governance/libraries/ProposalLib.sol"; + +import { + CallAssetPayload, UpgradePayload, CallRevertingPayload, EmptyPayload +} from "./TestPayloads.sol"; contract ApellaBase is TestBase { + using ProposalLib for DataStructures.Proposal; + IMintableERC20 internal token; Registry internal registry; Apella internal apella; Gerousia internal gerousia; + mapping(bytes32 => DataStructures.Proposal) internal proposals; + mapping(bytes32 => uint256) internal proposalIds; + DataStructures.Proposal internal proposal; + uint256 proposalId; + function setUp() public virtual { token = IMintableERC20(address(new TestERC20())); - registry = new Registry(address(this)); // @todo We should be using a bit of create2 magic to do this nicely apella = new Apella(token, address(0)); + registry = new Registry(address(apella)); gerousia = new Gerousia(apella, registry, 677, 1000); + // cheeky breeky vm.store(address(apella), 0, bytes32(uint256(uint160(address(gerousia))))); - registry.transferOwnership(address(apella)); - assertEq(apella.gerousia(), address(gerousia)); + + { + CallAssetPayload payload = new CallAssetPayload(token, address(apella)); + vm.prank(address(gerousia)); + assertTrue(apella.propose(payload)); + + proposalIds["call_asset"] = apella.proposalCount() - 1; + proposals["call_asset"] = apella.getProposal(proposalIds["call_asset"]); + } + + { + UpgradePayload payload = new UpgradePayload(registry); + vm.prank(address(gerousia)); + assertTrue(apella.propose(payload)); + + proposalIds["upgrade"] = apella.proposalCount() - 1; + proposals["upgrade"] = apella.getProposal(proposalIds["upgrade"]); + } + + { + CallRevertingPayload payload = new CallRevertingPayload(); + vm.prank(address(gerousia)); + assertTrue(apella.propose(payload)); + + proposalIds["revert"] = apella.proposalCount() - 1; + proposals["revert"] = apella.getProposal(proposalIds["revert"]); + } + + { + EmptyPayload payload = new EmptyPayload(); + vm.prank(address(gerousia)); + assertTrue(apella.propose(payload)); + + proposalIds["empty"] = apella.proposalCount() - 1; + proposals["empty"] = apella.getProposal(proposalIds["empty"]); + } + } + + function _statePending(bytes32 _proposalName) internal { + proposal = proposals[_proposalName]; + proposalId = proposalIds[_proposalName]; + } + + function _stateActive(bytes32 _proposalName) internal { + proposal = proposals[_proposalName]; + proposalId = proposalIds[_proposalName]; + + // @note We jump to the point where it becomes active + vm.warp(Timestamp.unwrap(proposal.pendingThrough()) + 1); + + assertTrue(apella.getProposalState(proposalId) == DataStructures.ProposalState.Active); + } + + function _stateDropped(bytes32 _proposalName, address _gerousia) internal { + proposal = proposals[_proposalName]; + proposalId = proposalIds[_proposalName]; + + vm.assume(_gerousia != proposal.creator); + + vm.prank(address(apella)); + apella.updateGerousia(_gerousia); + } + + function _stateRejected(bytes32 _proposalName) internal { + // We just take a really simple case here. As the cases area covered separately in `voteTabulation.t.sol` + // We simple throw no votes at all. + proposal = proposals[_proposalName]; + proposalId = proposalIds[_proposalName]; + + vm.warp(Timestamp.unwrap(proposal.activeThrough()) + 1); + + assertTrue(apella.getProposalState(proposalId) == DataStructures.ProposalState.Rejected); + } + + function _stateQueued( + bytes32 _proposalName, + address _voter, + uint256 _totalPower, + uint256 _votesCast, + uint256 _yeas + ) internal { + vm.assume(_voter != address(0)); + proposal = proposals[_proposalName]; + proposalId = proposalIds[_proposalName]; + + uint256 totalPower = bound(_totalPower, proposal.config.minimumVotes, type(uint128).max); + uint256 votesNeeded = Math.mulDiv(totalPower, proposal.config.quorum, 1e18, Math.Rounding.Ceil); + uint256 votesCast = bound(_votesCast, votesNeeded, totalPower); + + uint256 yeaLimitFraction = Math.ceilDiv(1e18 + proposal.config.voteDifferential, 2); + uint256 yeaLimit = Math.mulDiv(votesCast, yeaLimitFraction, 1e18, Math.Rounding.Ceil); + + uint256 yeas = yeaLimit == votesCast ? votesCast : bound(_yeas, yeaLimit + 1, votesCast); + + token.mint(_voter, totalPower); + vm.startPrank(_voter); + token.approve(address(apella), totalPower); + apella.deposit(_voter, totalPower); + vm.stopPrank(); + + _stateActive(_proposalName); + + vm.startPrank(_voter); + apella.vote(proposalId, yeas, true); + apella.vote(proposalId, votesCast - yeas, false); + vm.stopPrank(); + + vm.warp(Timestamp.unwrap(proposal.activeThrough()) + 1); + + assertEq( + apella.getProposalState(proposalId), DataStructures.ProposalState.Queued, "invalid state" + ); + } + + function _stateExecutable( + bytes32 _proposalName, + address _voter, + uint256 _totalPower, + uint256 _votesCast, + uint256 _yeas + ) internal { + proposal = proposals[_proposalName]; + proposalId = proposalIds[_proposalName]; + + _stateQueued(_proposalName, _voter, _totalPower, _votesCast, _yeas); + + vm.warp(Timestamp.unwrap(proposal.queuedThrough()) + 1); + + assertEq( + apella.getProposalState(proposalId), DataStructures.ProposalState.Executable, "invalid state" + ); + } + + function _stateExpired( + bytes32 _proposalName, + address _voter, + uint256 _totalPower, + uint256 _votesCast, + uint256 _yeas + ) internal { + proposal = proposals[_proposalName]; + proposalId = proposalIds[_proposalName]; + + _stateExecutable(_proposalName, _voter, _totalPower, _votesCast, _yeas); + + vm.warp(Timestamp.unwrap(proposal.executableThrough()) + 1); + + assertEq( + apella.getProposalState(proposalId), DataStructures.ProposalState.Expired, "invalid state" + ); + } + + function assertEq(VoteTabulationReturn a, VoteTabulationReturn b) internal { + if (a != b) { + emit log("Error: a == b not satisfied [VoteTabulationReturn]"); + emit log_named_uint(" Left", uint256(a)); + emit log_named_uint(" Right", uint256(b)); + fail(); + } + } + + function assertEq(VoteTabulationReturn a, VoteTabulationReturn b, string memory err) internal { + if (a != b) { + emit log_named_string("Error", err); + assertEq(a, b); + } + } + + function assertEq(VoteTabulationInfo a, VoteTabulationInfo b) internal { + if (a != b) { + emit log("Error: a == b not satisfied [VoteTabulationInfo]"); + emit log_named_uint(" Left", uint256(a)); + emit log_named_uint(" Right", uint256(b)); + fail(); + } + } + + function assertEq(VoteTabulationInfo a, VoteTabulationInfo b, string memory err) internal { + if (a != b) { + emit log_named_string("Error", err); + assertEq(a, b); + } + } + + function assertEq(DataStructures.ProposalState a, DataStructures.ProposalState b) internal { + if (a != b) { + emit log("Error: a == b not satisfied [DataStructures.ProposalState]"); + emit log_named_uint(" Left", uint256(a)); + emit log_named_uint(" Right", uint256(b)); + fail(); + } + } + + function assertEq( + DataStructures.ProposalState a, + DataStructures.ProposalState b, + string memory err + ) internal { + if (a != b) { + emit log_named_string("Error", err); + assertEq(a, b); + } } } diff --git a/l1-contracts/test/governance/apella/dropProposal.t.sol b/l1-contracts/test/governance/apella/dropProposal.t.sol new file mode 100644 index 000000000000..4cb9794547e0 --- /dev/null +++ b/l1-contracts/test/governance/apella/dropProposal.t.sol @@ -0,0 +1,136 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.27; + +import {ApellaBase} from "./base.t.sol"; +import {DataStructures} from "@aztec/governance/libraries/DataStructures.sol"; +import {Errors} from "@aztec/governance/libraries/Errors.sol"; + +contract DropProposalTest is ApellaBase { + modifier givenProposalIsStable() { + _; + } + + function test_GivenProposalIsDropped(address _gerousia) external givenProposalIsStable { + // it revert + _stateDropped("empty", _gerousia); + assertEq(apella.getProposal(proposalId).state, DataStructures.ProposalState.Pending); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Dropped); + assertTrue(apella.dropProposal(proposalId)); + assertEq(apella.getProposal(proposalId).state, DataStructures.ProposalState.Dropped); + + vm.expectRevert(abi.encodeWithSelector(Errors.Apella__ProposalAlreadyDropped.selector)); + apella.dropProposal(proposalId); + } + + function test_GivenProposalIsExecuted( + address _voter, + uint256 _totalPower, + uint256 _votesCast, + uint256 _yeas + ) external givenProposalIsStable { + // it revert + _stateExecutable("empty", _voter, _totalPower, _votesCast, _yeas); + assertTrue(apella.execute(proposalId)); + assertEq(apella.getProposal(proposalId).state, DataStructures.ProposalState.Executed); + + vm.expectRevert(abi.encodeWithSelector(Errors.Apella__ProposalCannotBeDropped.selector)); + apella.dropProposal(proposalId); + } + + modifier givenProposalIsUnstable() { + _; + } + + modifier whenGetProposalStateIsNotDropped() { + _; + vm.expectRevert(abi.encodeWithSelector(Errors.Apella__ProposalCannotBeDropped.selector)); + apella.dropProposal(proposalId); + } + + function test_WhenGetProposalStateIsPending() + external + givenProposalIsUnstable + whenGetProposalStateIsNotDropped + { + // it revert + _statePending("empty"); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Pending); + } + + function test_WhenGetProposalStateIsActive() + external + givenProposalIsUnstable + whenGetProposalStateIsNotDropped + { + // it revert + _stateActive("empty"); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Active); + } + + function test_WhenGetProposalStateIsQueued( + address _voter, + uint256 _totalPower, + uint256 _votesCast, + uint256 _yeas + ) external givenProposalIsUnstable whenGetProposalStateIsNotDropped { + // it revert + _stateQueued("empty", _voter, _totalPower, _votesCast, _yeas); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Queued); + } + + function test_WhenGetProposalStateIsExecutable( + address _voter, + uint256 _totalPower, + uint256 _votesCast, + uint256 _yeas + ) external givenProposalIsUnstable whenGetProposalStateIsNotDropped { + // it revert + _stateExecutable("empty", _voter, _totalPower, _votesCast, _yeas); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Executable); + } + + function test_WhenGetProposalStateIsRejected() + external + givenProposalIsUnstable + whenGetProposalStateIsNotDropped + { + // it revert + _stateRejected("empty"); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Rejected); + } + + function test_WhenGetProposalStateIsExecuted( + address _voter, + uint256 _totalPower, + uint256 _votesCast, + uint256 _yeas + ) external givenProposalIsUnstable whenGetProposalStateIsNotDropped { + // it revert + _stateExecutable("empty", _voter, _totalPower, _votesCast, _yeas); + assertTrue(apella.execute(proposalId)); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Executed); + } + + function test_WhenGetProposalStateIsExpired( + address _voter, + uint256 _totalPower, + uint256 _votesCast, + uint256 _yeas + ) external givenProposalIsUnstable whenGetProposalStateIsNotDropped { + // it revert + _stateExpired("empty", _voter, _totalPower, _votesCast, _yeas); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Expired); + } + + function test_WhenGetProposalStateIsDropped(address _gerousia) external givenProposalIsUnstable { + // it updates state to Dropped + // it return true + + _stateDropped("empty", _gerousia); + assertEq(apella.getProposal(proposalId).state, DataStructures.ProposalState.Pending); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Dropped); + assertTrue(apella.dropProposal(proposalId)); + assertEq(apella.getProposal(proposalId).state, DataStructures.ProposalState.Dropped); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Dropped); + } +} diff --git a/l1-contracts/test/governance/apella/dropProposal.tree b/l1-contracts/test/governance/apella/dropProposal.tree new file mode 100644 index 000000000000..156db4a948ab --- /dev/null +++ b/l1-contracts/test/governance/apella/dropProposal.tree @@ -0,0 +1,25 @@ +DropProposalTest +├── given proposal is stable +│ ├── given proposal is Dropped +│ │ └── it revert +│ └── given proposal is Executed +│ └── it revert +└── given proposal is unstable + ├── when getProposalState is not Dropped + │ ├── when getProposalState is Pending + │ │ └── it revert + │ ├── when getProposalState is Active + │ │ └── it revert + │ ├── when getProposalState is Queued + │ │ └── it revert + │ ├── when getProposalState is Executable + │ │ └── it revert + │ ├── when getProposalState is Rejected + │ │ └── it revert + │ ├── when getProposalState is Executed + │ │ └── it revert + │ └── when getProposalState is Expired + │ └── it revert + └── when getProposalState is Dropped + ├── it updates state to Dropped + └── it return true \ No newline at end of file diff --git a/l1-contracts/test/governance/apella/execute.t.sol b/l1-contracts/test/governance/apella/execute.t.sol index b907e90ec230..10fe58a4e74f 100644 --- a/l1-contracts/test/governance/apella/execute.t.sol +++ b/l1-contracts/test/governance/apella/execute.t.sol @@ -1,31 +1,160 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity >=0.8.27; -contract ExecuteTest { - function test_GivenStateIsNotExecutable() external { - // it revert - } - - modifier givenStateIsExecutable() { - _; - } - - function test_GivenPayloadCallAsset() external givenStateIsExecutable { - // it revert - } - - modifier givenPayloadDontCallAsset() { - _; - } - - function test_GivenAPayloadCallFails() external givenStateIsExecutable givenPayloadDontCallAsset { - // it revert - } - - function test_GivenAllPayloadCallSucceeds() external givenStateIsExecutable givenPayloadDontCallAsset { - // it updates state to Executed - // it executes the calls - // it emits {ProposalExecuted} event - // it return true - } +import {ApellaBase} from "./base.t.sol"; +import {Errors} from "@aztec/governance/libraries/Errors.sol"; +import {DataStructures} from "@aztec/governance/libraries/DataStructures.sol"; +import {IApella} from "@aztec/governance/interfaces/IApella.sol"; +import { + ProposalLib, + VoteTabulationReturn, + VoteTabulationInfo +} from "@aztec/governance/libraries/ProposalLib.sol"; + +import {CallAssetPayload, UpgradePayload, CallRevertingPayload} from "./TestPayloads.sol"; + +contract ExecuteTest is ApellaBase { + using ProposalLib for DataStructures.Proposal; + + uint256 internal depositPower; + + modifier givenStateIsNotExecutable() { + _; + vm.expectRevert(abi.encodeWithSelector(Errors.Apella__ProposalNotExecutable.selector)); + apella.execute(proposalId); + } + + function test_GivenStateIsPending() external givenStateIsNotExecutable { + // it revert + _statePending("empty"); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Pending); + } + + function test_GivenStateIsActive() external givenStateIsNotExecutable { + // it revert + _stateActive("empty"); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Active); + } + + function test_GivenStateIsQueued( + address _voter, + uint256 _totalPower, + uint256 _votesCast, + uint256 _yeas + ) external givenStateIsNotExecutable { + // it revert + _stateQueued("empty", _voter, _totalPower, _votesCast, _yeas); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Queued); + } + + function test_GivenStateIsRejected() external givenStateIsNotExecutable { + // it revert + _stateRejected("empty"); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Rejected); + } + + function test_GivenStateIsDropped(address _gerousia) external givenStateIsNotExecutable { + // it revert + _stateDropped("empty", _gerousia); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Dropped); + } + + function test_GivenStateIsExecuted( + address _voter, + uint256 _totalPower, + uint256 _votesCast, + uint256 _yeas + ) external givenStateIsNotExecutable { + // it revert + _stateExecutable("empty", _voter, _totalPower, _votesCast, _yeas); + assertTrue(apella.execute(proposalId)); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Executed); + } + + function test_GivenStateIsExpired( + address _voter, + uint256 _totalPower, + uint256 _votesCast, + uint256 _yeas + ) external givenStateIsNotExecutable { + // it revert + _stateExpired("empty", _voter, _totalPower, _votesCast, _yeas); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Expired); + } + + modifier givenStateIsExecutable( + address _voter, + uint256 _totalPower, + uint256 _votesCast, + uint256 _yeas, + bytes32 _proposalName + ) { + _stateExecutable(_proposalName, _voter, _totalPower, _votesCast, _yeas); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Executable); + _; + } + + function test_GivenPayloadCallAsset( + address _voter, + uint256 _totalPower, + uint256 _votesCast, + uint256 _yeas + ) external givenStateIsExecutable(_voter, _totalPower, _votesCast, _yeas, "call_asset") { + // it revert + + vm.expectRevert(abi.encodeWithSelector(Errors.Apella__CannotCallAsset.selector)); + apella.execute(proposalId); + } + + modifier givenPayloadDontCallAsset() { + _; + } + + function test_GivenAPayloadCallFails( + address _voter, + uint256 _totalPower, + uint256 _votesCast, + uint256 _yeas + ) + external + givenStateIsExecutable(_voter, _totalPower, _votesCast, _yeas, "revert") + givenPayloadDontCallAsset + { + // it revert + + vm.expectRevert( + abi.encodeWithSelector( + Errors.Apella__CallFailed.selector, + address(CallRevertingPayload(address(proposal.payload)).TARGET()) + ) + ); + apella.execute(proposalId); + } + + function test_GivenAllPayloadCallSucceeds( + address _voter, + uint256 _totalPower, + uint256 _votesCast, + uint256 _yeas + ) + external + givenStateIsExecutable(_voter, _totalPower, _votesCast, _yeas, "upgrade") + givenPayloadDontCallAsset + { + // it updates state to Executed + // it executes the calls + // it emits {ProposalExecuted} event + // it return true + + vm.expectEmit(true, true, true, true, address(apella)); + emit IApella.ProposalExecuted(proposalId); + assertTrue(apella.execute(proposalId)); + + proposal = apella.getProposal(proposalId); + + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Executed); + assertEq(proposal.state, DataStructures.ProposalState.Executed); + address rollup = registry.getRollup(); + assertEq(rollup, UpgradePayload(address(proposal.payload)).NEW_ROLLUP()); + } } diff --git a/l1-contracts/test/governance/apella/execute.tree b/l1-contracts/test/governance/apella/execute.tree index fad6755d511e..696d3c9fc051 100644 --- a/l1-contracts/test/governance/apella/execute.tree +++ b/l1-contracts/test/governance/apella/execute.tree @@ -1,6 +1,19 @@ ExecuteTest ├── given state is not executable -│ └── it revert +│ ├── given state is pending +│ │ └── it revert +│ ├── given state is active +│ │ └── it revert +│ ├── given state is queued +│ │ └── it revert +│ ├── given state is rejected +│ │ └── it revert +│ ├── given state is dropped +│ │ └── it revert +│ ├── given state is executed +│ │ └── it revert +│ └── given state is expired +│ └── it revert └── given state is executable ├── given payload call asset │ └── it revert diff --git a/l1-contracts/test/governance/apella/getProposalState.t.sol b/l1-contracts/test/governance/apella/getProposalState.t.sol index 452e07561f81..7c4b2bd83b73 100644 --- a/l1-contracts/test/governance/apella/getProposalState.t.sol +++ b/l1-contracts/test/governance/apella/getProposalState.t.sol @@ -1,139 +1,233 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity >=0.8.27; -contract GetProposalStateTest { - function test_WhenProposalIsOutOfBounds() external { - // it revert - } - - modifier whenValidProposalId() { - _; - } - - function test_GivenStateIsExecuted() external whenValidProposalId { - // it return Executed - } - - function test_GivenStateIsDropped() external whenValidProposalId { - // it return Dropped - } - - function test_GivenStateIsExpired() external whenValidProposalId { - // it return Expired - } - - function test_GivenStateIsRejected() external whenValidProposalId { - // it return Rejected - } - - modifier givenStateIsUnstable() { - _; - } - - modifier givenGerousiaHaveChanged() { - _; - } - - function test_GivenGerousiaHaveChanged() - external - whenValidProposalId - givenStateIsUnstable - givenGerousiaHaveChanged - { - // it return Dropped - } - - modifier givenGerousiaIsUnchanged() { - _; - } - - function test_WhenVotingDelayHaveNotPassed() - external - whenValidProposalId - givenStateIsUnstable - givenGerousiaHaveChanged - givenGerousiaIsUnchanged - { - // it return Pending - } - - modifier whenVotingDelayHavePassed() { - _; - } - - function test_WhenVotingDurationHaveNotPassed() - external - whenValidProposalId - givenStateIsUnstable - givenGerousiaHaveChanged - givenGerousiaIsUnchanged - whenVotingDelayHavePassed - { - // it return Active - } - - modifier whenVotingDurationHavePassed() { - _; - } - - function test_GivenLackOfQourumOrLackOfDifferential() - external - whenValidProposalId - givenStateIsUnstable - givenGerousiaHaveChanged - givenGerousiaIsUnchanged - whenVotingDelayHavePassed - whenVotingDurationHavePassed - { - // it return Rejected - } - - modifier givenQuorumAndDifferential() { - _; - } - - function test_GivenExecutionDelayHaveNotPassed() - external - whenValidProposalId - givenStateIsUnstable - givenGerousiaHaveChanged - givenGerousiaIsUnchanged - whenVotingDelayHavePassed - whenVotingDurationHavePassed - givenQuorumAndDifferential - { - // it return Queued - } - - modifier givenExecutionDelayHavePassed() { - _; - } - - function test_GivenGracePeriodHaveNotPassed() - external - whenValidProposalId - givenStateIsUnstable - givenGerousiaHaveChanged - givenGerousiaIsUnchanged - whenVotingDelayHavePassed - whenVotingDurationHavePassed - givenQuorumAndDifferential - givenExecutionDelayHavePassed - { - // it return Executable - } - - function test_GivenGracePeriodHavePassed() - external - whenValidProposalId - givenStateIsUnstable - givenGerousiaHaveChanged - givenGerousiaIsUnchanged - whenVotingDelayHavePassed - whenVotingDurationHavePassed - givenQuorumAndDifferential - givenExecutionDelayHavePassed - { - // it return Expired - } +import {ApellaBase} from "./base.t.sol"; +import {DataStructures} from "@aztec/governance/libraries/DataStructures.sol"; +import {Errors} from "@aztec/governance/libraries/Errors.sol"; +import {Timestamp} from "@aztec/core/libraries/TimeMath.sol"; +import {ProposalLib, VoteTabulationReturn} from "@aztec/governance/libraries/ProposalLib.sol"; + +contract GetProposalStateTest is ApellaBase { + using ProposalLib for DataStructures.Proposal; + + function test_WhenProposalIsOutOfBounds(uint256 _index) external { + // it revert + uint256 index = bound(_index, apella.proposalCount(), type(uint256).max); + vm.expectRevert(abi.encodeWithSelector(Errors.Apella__ProposalDoesNotExists.selector, index)); + apella.getProposalState(index); + } + + modifier whenValidProposalId() { + _; + } + + modifier givenStateIsStable() { + _; + } + + function test_GivenStateIsExecuted( + address _voter, + uint256 _totalPower, + uint256 _votesCast, + uint256 _yeas + ) external whenValidProposalId givenStateIsStable { + // it return Executed + _stateExecutable("empty", _voter, _totalPower, _votesCast, _yeas); + apella.execute(proposalId); + + assertEq(proposal.state, DataStructures.ProposalState.Pending); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Executed); + } + + function test_GivenStateIsDropped(address _gerousia) + external + whenValidProposalId + givenStateIsStable + { + // it return Dropped + _stateDropped("empty", _gerousia); + + assertEq(proposal.state, DataStructures.ProposalState.Pending); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Dropped); + + apella.dropProposal(proposalId); + + DataStructures.Proposal memory fresh = apella.getProposal(proposalId); + assertEq(fresh.state, DataStructures.ProposalState.Dropped); + } + + modifier givenStateIsUnstable() { + _; + + DataStructures.Proposal memory fresh = apella.getProposal(proposalId); + assertEq(fresh.state, DataStructures.ProposalState.Pending); + } + + function test_GivenGerousiaHaveChanged(address _gerousia) + external + whenValidProposalId + givenStateIsUnstable + { + // it return Dropped + _stateDropped("empty", _gerousia); + + assertEq(proposal.state, DataStructures.ProposalState.Pending); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Dropped); + } + + modifier givenGerousiaIsUnchanged() { + _; + } + + function test_WhenVotingDelayHaveNotPassed() + external + whenValidProposalId + givenStateIsUnstable + givenGerousiaIsUnchanged + { + // it return Pending + _statePending("empty"); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Pending); + } + + modifier whenVotingDelayHavePassed() { + _; + } + + function test_WhenVotingDurationHaveNotPassed() + external + whenValidProposalId + givenStateIsUnstable + givenGerousiaIsUnchanged + whenVotingDelayHavePassed + { + // it return Active + _stateActive("empty"); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Active); + } + + modifier whenVotingDurationHavePassed() { + _; + } + + function test_GivenVoteTabulationIsRejected() + external + whenValidProposalId + givenStateIsUnstable + givenGerousiaIsUnchanged + whenVotingDelayHavePassed + whenVotingDurationHavePassed + { + // it return Rejected + _stateRejected("empty"); + + uint256 totalPower = apella.totalPowerAt(Timestamp.wrap(block.timestamp)); + (VoteTabulationReturn vtr,) = proposal.voteTabulation(totalPower); + assertEq(vtr, VoteTabulationReturn.Rejected, "invalid return value"); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Rejected); + } + + function test_GivenVoteTabulationIsInvalid( + address _voter, + uint256 _totalPower, + uint256 _votesCast, + uint256 _yeas + ) + external + whenValidProposalId + givenStateIsUnstable + givenGerousiaIsUnchanged + whenVotingDelayHavePassed + whenVotingDurationHavePassed + { + // it return Rejected + + _stateQueued("empty", _voter, _totalPower, _votesCast, _yeas); + + // We can overwrite the quorum to be 0 to hit an invalid case + assertGt(apella.getProposal(proposalId).config.quorum, 0); + bytes32 slot = + bytes32(uint256(keccak256(abi.encodePacked(uint256(proposalId), uint256(1)))) + 4); + vm.store(address(apella), slot, 0); + assertEq(apella.getProposal(proposalId).config.quorum, 0); + + uint256 totalPower = apella.totalPowerAt(Timestamp.wrap(block.timestamp)); + + proposal = apella.getProposal(proposalId); + (VoteTabulationReturn vtr,) = proposal.voteTabulation(totalPower); + assertEq(vtr, VoteTabulationReturn.Invalid, "invalid return value"); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Rejected); + } + + modifier givenVoteTabulationIsAccepted( + address _voter, + uint256 _totalPower, + uint256 _votesCast, + uint256 _yeas + ) { + _stateQueued("empty", _voter, _totalPower, _votesCast, _yeas); + _; + } + + function test_GivenExecutionDelayHaveNotPassed( + address _voter, + uint256 _totalPower, + uint256 _votesCast, + uint256 _yeas + ) + external + whenValidProposalId + givenStateIsUnstable + givenGerousiaIsUnchanged + whenVotingDelayHavePassed + whenVotingDurationHavePassed + givenVoteTabulationIsAccepted(_voter, _totalPower, _votesCast, _yeas) + { + // it return Queued + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Queued); + } + + modifier givenExecutionDelayHavePassed() { + vm.warp(Timestamp.unwrap(proposal.queuedThrough()) + 1); + _; + } + + function test_GivenGracePeriodHaveNotPassed( + address _voter, + uint256 _totalPower, + uint256 _votesCast, + uint256 _yeas + ) + external + whenValidProposalId + givenStateIsUnstable + givenGerousiaIsUnchanged + whenVotingDelayHavePassed + whenVotingDurationHavePassed + givenVoteTabulationIsAccepted(_voter, _totalPower, _votesCast, _yeas) + givenExecutionDelayHavePassed + { + // it return Executable + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Executable); + } + + function test_GivenGracePeriodHavePassed( + address _voter, + uint256 _totalPower, + uint256 _votesCast, + uint256 _yeas + ) + external + whenValidProposalId + givenStateIsUnstable + givenGerousiaIsUnchanged + whenVotingDelayHavePassed + whenVotingDurationHavePassed + givenVoteTabulationIsAccepted(_voter, _totalPower, _votesCast, _yeas) + givenExecutionDelayHavePassed + { + // it return Expired + vm.warp(Timestamp.unwrap(proposal.executableThrough()) + 1); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Expired); + } } diff --git a/l1-contracts/test/governance/apella/getProposalState.tree b/l1-contracts/test/governance/apella/getProposalState.tree index 1cc29605e0b2..2a80ccd79d8b 100644 --- a/l1-contracts/test/governance/apella/getProposalState.tree +++ b/l1-contracts/test/governance/apella/getProposalState.tree @@ -2,31 +2,30 @@ GetProposalStateTest ├── when proposal is out of bounds │ └── it revert └── when valid proposal id - ├── given state is Executed - │ └── it return Executed - ├── given state is Dropped - │ └── it return Dropped - ├── given state is Expired - │ └── it return Expired - ├── given state is Rejected - │ └── it return Rejected + ├── given state is stable + │ ├── given state is Executed + │ │ └── it return Executed + │ └── given state is Dropped + │ └── it return Dropped └── given state is unstable - └── given gerousia have changed - ├── it return Dropped - └── given gerousia is unchanged - ├── when voting delay have not passed - │ └── it return Pending - └── when voting delay have passed - ├── when voting duration have not passed - │ └── it return Active - └── when voting duration have passed - ├── given lack of qourum or lack of differential - │ └── it return Rejected - └── given quorum and differential - ├── given execution delay have not passed - │ └── it return Queued - └── given execution delay have passed - ├── given grace period have not passed - │ └── it return Executable - └── given grace period have passed - └── it return Expired \ No newline at end of file + ├── given gerousia have changed + │ └── it return Dropped + └── given gerousia is unchanged + ├── when voting delay have not passed + │ └── it return Pending + └── when voting delay have passed + ├── when voting duration have not passed + │ └── it return Active + └── when voting duration have passed + ├── given vote tabulation is Rejected + │ └── it return Rejected + ├── given vote tabulation is Invalid + │ └── it return Rejected + └── given vote tabulation is Accepted + ├── given execution delay have not passed + │ └── it return Queued + └── given execution delay have passed + ├── given grace period have not passed + │ └── it return Executable + └── given grace period have passed + └── it return Expired \ No newline at end of file diff --git a/l1-contracts/test/governance/apella/initiateWithdraw.t.sol b/l1-contracts/test/governance/apella/initiateWithdraw.t.sol index 6e15451de5a7..853f70d0eff0 100644 --- a/l1-contracts/test/governance/apella/initiateWithdraw.t.sol +++ b/l1-contracts/test/governance/apella/initiateWithdraw.t.sol @@ -40,7 +40,7 @@ contract InitiateWithdrawTest is ApellaBase { vm.expectRevert( abi.encodeWithSelector( - Errors.Apella__InsufficientPower.selector, depositAmount, withdrawalAmount + Errors.Apella__InsufficientPower.selector, address(this), depositAmount, withdrawalAmount ) ); apella.initiateWithdraw(address(this), withdrawalAmount); diff --git a/l1-contracts/test/governance/apella/proposallib/isRejected.tree b/l1-contracts/test/governance/apella/proposallib/isRejected.tree deleted file mode 100644 index 936f80c9fdb2..000000000000 --- a/l1-contracts/test/governance/apella/proposallib/isRejected.tree +++ /dev/null @@ -1,32 +0,0 @@ -IsRejectedTest -├── when minimum config eq 0 -│ └── it revert -└── when minimum gt 0 - ├── when total power lt minimum - │ └── it return true - └── when total power gte minimum - ├── when quorum config invalid - │ ├── when votes needed eq 0 - │ │ └── it revert - │ ├── when votes needed gt total - │ │ └── it revert - │ └── when votes needed gt uint256 max - │ └── it revert - └── when quorum config valid - ├── when votes cast lt votes needed - │ └── it return true - └── when votes cast gte votes needed - ├── when differential config invalid - │ ├── when yea votes needed eq 0 - │ │ └── it revert - │ ├── when yea votes needed gt uint256 max - │ │ └── it revert - │ └── when yea votes needed gt votes cast - │ └── it revert - └── when differential config valid - ├── when yea needed eq votes cast - │ └── it return true - ├── when yea votes lte yea needed - │ └── it return true - └── when yea votes gt yea needed - └── it return false \ No newline at end of file diff --git a/l1-contracts/test/governance/apella/proposallib/isStableState.t.sol b/l1-contracts/test/governance/apella/proposallib/isStableState.t.sol index 0d8b33a1b15c..6f68e0e43efa 100644 --- a/l1-contracts/test/governance/apella/proposallib/isStableState.t.sol +++ b/l1-contracts/test/governance/apella/proposallib/isStableState.t.sol @@ -22,32 +22,16 @@ contract IsStableStateTest is Test { assertTrue(proposal.isStable()); } - function test_GivenStateIsExpired() external { - // it return true - proposal.state = DataStructures.ProposalState.Expired; - assertTrue(proposal.isStable()); - } - - function test_GivenStateIsRejected() external { - // it return true - proposal.state = DataStructures.ProposalState.Rejected; - assertTrue(proposal.isStable()); - } - function test_GivenStateNotInAbove(uint8 _state) external { // it return false - proposal.state = DataStructures.ProposalState(bound(_state, 0, 7)); + DataStructures.ProposalState s = DataStructures.ProposalState(bound(_state, 0, 7)); - // I know, might not be the best way, but really easy to read vm.assume( - !( - proposal.state == DataStructures.ProposalState.Executed - || proposal.state == DataStructures.ProposalState.Dropped - || proposal.state == DataStructures.ProposalState.Expired - || proposal.state == DataStructures.ProposalState.Rejected - ) + !(s == DataStructures.ProposalState.Executed || s == DataStructures.ProposalState.Dropped) ); + proposal.state = s; + assertFalse(proposal.isStable()); } } diff --git a/l1-contracts/test/governance/apella/proposallib/isStableState.tree b/l1-contracts/test/governance/apella/proposallib/isStableState.tree index b409697c3199..60e73f9ee765 100644 --- a/l1-contracts/test/governance/apella/proposallib/isStableState.tree +++ b/l1-contracts/test/governance/apella/proposallib/isStableState.tree @@ -3,9 +3,5 @@ IsStableStateTest │ └── it return true ├── given state is Dropped │ └── it return true -├── given state is Expired -│ └── it return true -├── given state is Rejected -│ └── it return true └── given state not in above └── it return false \ No newline at end of file diff --git a/l1-contracts/test/governance/apella/proposallib/static.t.sol b/l1-contracts/test/governance/apella/proposallib/static.t.sol index 637b4cde7d27..314db1519e1d 100644 --- a/l1-contracts/test/governance/apella/proposallib/static.t.sol +++ b/l1-contracts/test/governance/apella/proposallib/static.t.sol @@ -24,44 +24,44 @@ contract Static is TestBase { _; } - function test_PendingUntil(DataStructures.Configuration memory _config, uint256 _creation) + function test_pendingThrough(DataStructures.Configuration memory _config, uint256 _creation) external limitConfig(_config) { proposal.creation = Timestamp.wrap(bound(_creation, 0, type(uint32).max)); - assertEq(proposal.pendingUntil(), proposal.creation + proposal.config.votingDelay); + assertEq(proposal.pendingThrough(), proposal.creation + proposal.config.votingDelay); } - function test_ActiveUntil(DataStructures.Configuration memory _config, uint256 _creation) + function test_activeThrough(DataStructures.Configuration memory _config, uint256 _creation) external limitConfig(_config) { proposal.creation = Timestamp.wrap(bound(_creation, 0, type(uint32).max)); assertEq( - proposal.activeUntil(), + proposal.activeThrough(), proposal.creation + proposal.config.votingDelay + proposal.config.votingDuration ); } - function test_QueuedUntil(DataStructures.Configuration memory _config, uint256 _creation) + function test_queuedThrough(DataStructures.Configuration memory _config, uint256 _creation) external limitConfig(_config) { proposal.creation = Timestamp.wrap(bound(_creation, 0, type(uint32).max)); assertEq( - proposal.queuedUntil(), + proposal.queuedThrough(), proposal.creation + proposal.config.votingDelay + proposal.config.votingDuration + proposal.config.executionDelay ); } - function test_ExecutableUntil(DataStructures.Configuration memory _config, uint256 _creation) + function test_executableThrough(DataStructures.Configuration memory _config, uint256 _creation) external limitConfig(_config) { proposal.creation = Timestamp.wrap(bound(_creation, 0, type(uint32).max)); assertEq( - proposal.executableUntil(), + proposal.executableThrough(), proposal.creation + proposal.config.votingDelay + proposal.config.votingDuration + proposal.config.executionDelay + proposal.config.gracePeriod ); diff --git a/l1-contracts/test/governance/apella/proposallib/isRejected.t.sol b/l1-contracts/test/governance/apella/proposallib/voteTabulation.t.sol similarity index 60% rename from l1-contracts/test/governance/apella/proposallib/isRejected.t.sol rename to l1-contracts/test/governance/apella/proposallib/voteTabulation.t.sol index 3e905c3ca3d2..0543ac6747e3 100644 --- a/l1-contracts/test/governance/apella/proposallib/isRejected.t.sol +++ b/l1-contracts/test/governance/apella/proposallib/voteTabulation.t.sol @@ -1,28 +1,31 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity >=0.8.27; -import {Test} from "forge-std/Test.sol"; +import {ApellaBase} from "../base.t.sol"; import {DataStructures} from "@aztec/governance/libraries/DataStructures.sol"; -import {ProposalLib} from "@aztec/governance/libraries/ProposalLib.sol"; +import { + ProposalLib, + VoteTabulationReturn, + VoteTabulationInfo +} from "@aztec/governance/libraries/ProposalLib.sol"; import {ConfigurationLib} from "@aztec/governance/libraries/ConfigurationLib.sol"; -import {Errors} from "@aztec/governance/libraries/Errors.sol"; import {Math} from "@oz/utils/math/Math.sol"; -contract IsRejectedTest is Test { +contract VoteTabulationTest is ApellaBase { using ProposalLib for DataStructures.Proposal; using ConfigurationLib for DataStructures.Configuration; - DataStructures.Proposal internal proposal; uint256 internal totalPower; uint256 internal votes; uint256 internal votesNeeded; uint256 internal yeaLimit; function test_WhenMinimumConfigEq0() external { - // it revert - vm.expectRevert(abi.encodeWithSelector(Errors.Apella__ProposalLib__ZeroMinimum.selector)); - proposal.isRejected(0); + // it return (Invalid, MinimumEqZero) + (VoteTabulationReturn vtr, VoteTabulationInfo vti) = proposal.voteTabulation(0); + assertEq(vtr, VoteTabulationReturn.Invalid, "invalid return value"); + assertEq(vti, VoteTabulationInfo.MinimumEqZero, "invalid info value"); } modifier whenMinimumGt0(DataStructures.Configuration memory _config) { @@ -31,17 +34,19 @@ contract IsRejectedTest is Test { _; } - function test_WhenTotalPowerLtMinimum(DataStructures.Configuration memory _config) - external - whenMinimumGt0(_config) - { - // it return true - assertTrue(proposal.isRejected(0)); + function test_WhenTotalPowerLtMinimum( + DataStructures.Configuration memory _config, + uint256 _totalPower + ) external whenMinimumGt0(_config) { + // it return (Rejected, TotalPowerLtMinimum) + totalPower = bound(_totalPower, 0, proposal.config.minimumVotes - 1); + (VoteTabulationReturn vtr, VoteTabulationInfo vti) = proposal.voteTabulation(totalPower); + assertEq(vtr, VoteTabulationReturn.Rejected, "invalid return value"); + assertEq(vti, VoteTabulationInfo.TotalPowerLtMinimum, "invalid info value"); } modifier whenTotalPowerGteMinimum(uint256 _totalPower) { totalPower = bound(_totalPower, proposal.config.minimumVotes, type(uint256).max); - _; } @@ -55,16 +60,18 @@ contract IsRejectedTest is Test { whenTotalPowerGteMinimum(_totalPower) whenQuorumConfigInvalid { - // it revert - vm.expectRevert(abi.encodeWithSelector(Errors.Apella__ProposalLib__ZeroVotesNeeded.selector)); - proposal.isRejected(totalPower); + // it return (Invalid, VotesNeededEqZero) + (VoteTabulationReturn vtr, VoteTabulationInfo vti) = proposal.voteTabulation(totalPower); + assertEq(vtr, VoteTabulationReturn.Invalid, "invalid return value"); + assertEq(vti, VoteTabulationInfo.VotesNeededEqZero, "invalid info value"); } function test_WhenVotesNeededGtTotal( DataStructures.Configuration memory _config, uint256 _totalPower ) external whenMinimumGt0(_config) whenTotalPowerGteMinimum(_totalPower) whenQuorumConfigInvalid { - // it revert + // it return (Invalid, VotesNeededGtTotalPower) + proposal.config.quorum = 1e18 + 1; // Overwriting some limits such that we do not overflow @@ -73,10 +80,9 @@ contract IsRejectedTest is Test { bound(_config.minimumVotes, ConfigurationLib.VOTES_LOWER, upperLimit); totalPower = bound(_totalPower, proposal.config.minimumVotes, upperLimit); - vm.expectRevert( - abi.encodeWithSelector(Errors.Apella__ProposalLib__MoreVoteThanExistNeeded.selector) - ); - proposal.isRejected(totalPower); + (VoteTabulationReturn vtr, VoteTabulationInfo vti) = proposal.voteTabulation(totalPower); + assertEq(vtr, VoteTabulationReturn.Invalid, "invalid return value"); + assertEq(vti, VoteTabulationInfo.VotesNeededGtTotalPower, "invalid info value"); } function test_WhenVotesNeededGtUint256Max( @@ -87,7 +93,7 @@ contract IsRejectedTest is Test { totalPower = type(uint256).max; proposal.config.quorum = 1e18 + 1; vm.expectRevert(abi.encodeWithSelector(Math.MathOverflowedMulDiv.selector)); - proposal.isRejected(totalPower); + proposal.voteTabulation(totalPower); } modifier whenQuorumConfigValid(DataStructures.Configuration memory _config) { @@ -109,7 +115,7 @@ contract IsRejectedTest is Test { whenTotalPowerGteMinimum(_totalPower) whenQuorumConfigValid(_config) { - // it return true + // it return (Rejected, VotesCastLtVotesNeeded) uint256 maxVotes = votesNeeded > 0 ? votesNeeded - 1 : votesNeeded; @@ -121,7 +127,9 @@ contract IsRejectedTest is Test { proposal.summedBallot.yea = yea; proposal.summedBallot.nea = nea; - assertTrue(proposal.isRejected(totalPower)); + (VoteTabulationReturn vtr, VoteTabulationInfo vti) = proposal.voteTabulation(totalPower); + assertEq(vtr, VoteTabulationReturn.Rejected, "invalid return value"); + assertEq(vti, VoteTabulationInfo.VotesCastLtVotesNeeded, "invalid info value"); } modifier whenVotesCastGteVotesNeeded(uint256 _votes) { @@ -133,7 +141,7 @@ contract IsRejectedTest is Test { _; } - function test_WhenYeaVotesNeededEq0( + function test_WhenYeaLimitEq0( DataStructures.Configuration memory _config, uint256 _totalPower, uint256 _votes @@ -145,14 +153,13 @@ contract IsRejectedTest is Test { whenVotesCastGteVotesNeeded(_votes) whenDifferentialConfigInvalid { - // it revert - // Not sure if we can actually even hit this case, think we could only hit it - // if votesCast is 0 because we are rounding up - // @todo - assertFalse(true, "TODO"); + // it return (Invalid, YeaLimitEqZero) + // It should be impossible to hit this case as `yeaLimitFraction` cannot be 0, + // and due to rounding, only way to hit this would be if `votesCast = 0`, + // which is already handled as `votesCast >= votesNeeded` and `votesNeeded > 0`. } - function test_WhenYeaVotesNeededGtUint256Max( + function test_WhenYeaLimitGtUint256Max( DataStructures.Configuration memory _config, uint256 _totalPower, uint256 _votes @@ -165,11 +172,15 @@ contract IsRejectedTest is Test { whenDifferentialConfigInvalid { // it revert - // @todo - assertFalse(true, "TODO"); + proposal.config.voteDifferential = 1e18 + 1; + totalPower = type(uint256).max; + proposal.summedBallot.nea = totalPower; + + vm.expectRevert(abi.encodeWithSelector(Math.MathOverflowedMulDiv.selector)); + proposal.voteTabulation(totalPower); } - function _test_WhenYeaVotesNeededGtVotesCast( + function test_WhenYeaLimitGtVotesCast( DataStructures.Configuration memory _config, uint256 _totalPower, uint256 _votes @@ -181,7 +192,7 @@ contract IsRejectedTest is Test { whenVotesCastGteVotesNeeded(_votes) whenDifferentialConfigInvalid { - // it revert + // it return (Invalid, YeaLimitGtVotesCast) proposal.config.voteDifferential = 1e18 + 1; // Overwriting some limits such that we do not overflow @@ -193,25 +204,21 @@ contract IsRejectedTest is Test { votes = bound(_votes, votesNeeded, totalPower); proposal.summedBallot.nea = votes; - vm.expectRevert( - abi.encodeWithSelector(Errors.Apella__ProposalLib__MoreYeaVoteThanExistNeeded.selector) - ); - proposal.isRejected(totalPower); + (VoteTabulationReturn vtr, VoteTabulationInfo vti) = proposal.voteTabulation(totalPower); + assertEq(vtr, VoteTabulationReturn.Invalid, "invalid return value"); + assertEq(vti, VoteTabulationInfo.YeaLimitGtVotesCast, "invalid info value"); } modifier whenDifferentialConfigValid(DataStructures.Configuration memory _config) { - proposal.config.voteDifferential = bound( - _config.voteDifferential, - ConfigurationLib.DIFFERENTIAL_LOWER, - ConfigurationLib.DIFFERENTIAL_UPPER - ); + proposal.config.voteDifferential = + bound(_config.voteDifferential, 0, ConfigurationLib.DIFFERENTIAL_UPPER); uint256 yeaFraction = Math.ceilDiv(1e18 + proposal.config.voteDifferential, 2); yeaLimit = Math.mulDiv(votes, yeaFraction, 1e18, Math.Rounding.Ceil); _; } - function test_WhenYeaNeededEqVotesCast( + function test_WhenYeaVotesEqVotesCast( DataStructures.Configuration memory _config, uint256 _totalPower, uint256 _votes @@ -223,16 +230,15 @@ contract IsRejectedTest is Test { whenVotesCastGteVotesNeeded(_votes) whenDifferentialConfigValid(_config) { - // it return true - // overwriting whenDifferentialConfigValid - proposal.config.voteDifferential = 1e18; - uint256 yeaFraction = Math.ceilDiv(1e18 + proposal.config.voteDifferential, 2); - yeaLimit = Math.mulDiv(votes, yeaFraction, 1e18, Math.Rounding.Ceil); + // it return (Accepted, YeaVotesEqVotesCast) + proposal.summedBallot.yea = votes; - assertTrue(proposal.isRejected(totalPower)); + (VoteTabulationReturn vtr, VoteTabulationInfo vti) = proposal.voteTabulation(totalPower); + assertEq(vtr, VoteTabulationReturn.Accepted, "invalid return value"); + assertEq(vti, VoteTabulationInfo.YeaVotesEqVotesCast, "invalid info value"); } - function test_WhenYeaVotesLteYeaNeeded( + function test_WhenYeaVotesLteYeaLimit( DataStructures.Configuration memory _config, uint256 _totalPower, uint256 _votes, @@ -245,7 +251,10 @@ contract IsRejectedTest is Test { whenVotesCastGteVotesNeeded(_votes) whenDifferentialConfigValid(_config) { - // it return true + // it return (Rejected, YeaVotesLeYeaLimit) + + // Likely not the best way to do it, but we just need to avoid that one case. + vm.assume(yeaLimit != votes); // Avoid the edge case where all votes YEA, which should pass uint256 maxYea = yeaLimit == votes ? yeaLimit - 1 : yeaLimit; @@ -256,10 +265,12 @@ contract IsRejectedTest is Test { proposal.summedBallot.yea = yea; proposal.summedBallot.nea = nea; - assertTrue(proposal.isRejected(totalPower)); + (VoteTabulationReturn vtr, VoteTabulationInfo vti) = proposal.voteTabulation(totalPower); + assertEq(vtr, VoteTabulationReturn.Rejected, "invalid return value"); + assertEq(vti, VoteTabulationInfo.YeaVotesLeYeaLimit, "invalid info value"); } - function test_WhenYeaVotesGtYeaNeeded( + function test_WhenYeaVotesGtYeaLimit( DataStructures.Configuration memory _config, uint256 _totalPower, uint256 _votes, @@ -272,19 +283,25 @@ contract IsRejectedTest is Test { whenVotesCastGteVotesNeeded(_votes) whenDifferentialConfigValid(_config) { - // it return false + // it return (Accepted, YeaVotesGtYeaLimit) // If we are not in the case where all votes are YEA, we should add 1 to ensure // that we actually have sufficient YEA votes. uint256 minYea = yeaLimit < votes ? yeaLimit + 1 : yeaLimit; - uint256 yea = bound(_yea, minYea, votes); + + // Likely not the best way to do it, but we just need to avoid that one case. + vm.assume(yea != votes); + uint256 nea = votes - yea; proposal.summedBallot.yea = yea; proposal.summedBallot.nea = nea; assertGt(yea, nea, "yea <= nea"); - assertFalse(proposal.isRejected(totalPower), "Was rejected"); + + (VoteTabulationReturn vtr, VoteTabulationInfo vti) = proposal.voteTabulation(totalPower); + assertEq(vtr, VoteTabulationReturn.Accepted, "invalid return value"); + assertEq(vti, VoteTabulationInfo.YeaVotesGtYeaLimit, "invalid info value"); } } diff --git a/l1-contracts/test/governance/apella/proposallib/voteTabulation.tree b/l1-contracts/test/governance/apella/proposallib/voteTabulation.tree new file mode 100644 index 000000000000..bfd4a5ce91ac --- /dev/null +++ b/l1-contracts/test/governance/apella/proposallib/voteTabulation.tree @@ -0,0 +1,32 @@ +VoteTabulationTest +├── when minimum config eq 0 +│ └── it return (Invalid, MinimumEqZero) +└── when minimum gt 0 + ├── when total power lt minimum + │ └── it return (Rejected, TotalPowerLtMinimum) + └── when total power gte minimum + ├── when quorum config invalid + │ ├── when votes needed eq 0 + │ │ └── it return (Invalid, VotesNeededEqZero) + │ ├── when votes needed gt total + │ │ └── it return (Invalid, VotesNeededGtTotalPower) + │ └── when votes needed gt uint256 max + │ └── it revert + └── when quorum config valid + ├── when votes cast lt votes needed + │ └── it return (Rejected, VotesCastLtVotesNeeded) + └── when votes cast gte votes needed + ├── when differential config invalid + │ ├── when yea limit eq 0 + │ │ └── it return (Invalid, YeaLimitEqZero) + │ ├── when yea limit gt uint256 max + │ │ └── it revert + │ └── when yea limit gt votes cast + │ └── it return (Invalid, YeaLimitGtVotesCast) + └── when differential config valid + ├── when yea votes eq votes cast + │ └── it return (Accepted, YeaVotesEqVotesCast) + ├── when yea votes lte yea limit + │ └── it return (Rejected, YeaVotesLeYeaLimit) + └── when yea votes gt yea limit + └── it return (Accepted, YeaVotesGtYeaLimit) \ No newline at end of file diff --git a/l1-contracts/test/governance/apella/propose.t.sol b/l1-contracts/test/governance/apella/propose.t.sol index 4e307a73dbd5..2b1eaf56958e 100644 --- a/l1-contracts/test/governance/apella/propose.t.sol +++ b/l1-contracts/test/governance/apella/propose.t.sol @@ -1,14 +1,53 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity >=0.8.27; -contract ProposeTest { - function test_WhenCallerIsNotGerousia() external { - // it revert - } - - function test_WhenCallerIsGerousia() external { - // it creates a new proposal with current config - // it emits a {ProposalCreated} event - // it returns true - } +import {IPayload} from "@aztec/governance/interfaces/IPayload.sol"; +import {ApellaBase} from "./base.t.sol"; +import {IApella} from "@aztec/governance/interfaces/IApella.sol"; +import {IERC20Errors} from "@oz/interfaces/draft-IERC6093.sol"; +import {Timestamp} from "@aztec/core/libraries/TimeMath.sol"; +import {Errors} from "@aztec/governance/libraries/Errors.sol"; +import {DataStructures} from "@aztec/governance/libraries/DataStructures.sol"; +import {ConfigurationLib} from "@aztec/governance/libraries/ConfigurationLib.sol"; + +contract ProposeTest is ApellaBase { + function test_WhenCallerIsNotGerousia() external { + // it revert + vm.expectRevert( + abi.encodeWithSelector( + Errors.Apella__CallerNotGerousia.selector, address(this), address(gerousia) + ) + ); + apella.propose(IPayload(address(0))); + } + + function test_WhenCallerIsGerousia(address _proposal) external { + // it creates a new proposal with current config + // it emits a {ProposalCreated} event + // it returns true + + DataStructures.Configuration memory config = apella.getConfiguration(); + + proposalId = apella.proposalCount(); + + vm.expectEmit(true, true, true, true, address(apella)); + emit IApella.Proposed(proposalId, _proposal); + + vm.prank(address(gerousia)); + assertTrue(apella.propose(IPayload(_proposal))); + + DataStructures.Proposal memory proposal = apella.getProposal(proposalId); + assertEq(proposal.config.executionDelay, config.executionDelay); + assertEq(proposal.config.gracePeriod, config.gracePeriod); + assertEq(proposal.config.minimumVotes, config.minimumVotes); + assertEq(proposal.config.quorum, config.quorum); + assertEq(proposal.config.voteDifferential, config.voteDifferential); + assertEq(proposal.config.votingDelay, config.votingDelay); + assertEq(proposal.config.votingDuration, config.votingDuration); + assertEq(proposal.creation, Timestamp.wrap(block.timestamp)); + assertEq(proposal.creator, address(gerousia)); + assertEq(proposal.summedBallot.nea, 0); + assertEq(proposal.summedBallot.yea, 0); + assertTrue(proposal.state == DataStructures.ProposalState.Pending); + } } diff --git a/l1-contracts/test/governance/apella/scenarios/noVoteAndExit.t.sol b/l1-contracts/test/governance/apella/scenarios/noVoteAndExit.t.sol new file mode 100644 index 000000000000..ecdbdb6f34c3 --- /dev/null +++ b/l1-contracts/test/governance/apella/scenarios/noVoteAndExit.t.sol @@ -0,0 +1,76 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.27; + +import {ApellaBase} from "../base.t.sol"; +import {DataStructures} from "@aztec/governance/libraries/DataStructures.sol"; +import {Errors} from "@aztec/governance/libraries/Errors.sol"; +import {Math} from "@oz/utils/math/Math.sol"; +import {Timestamp} from "@aztec/core/libraries/TimeMath.sol"; +import {ProposalLib} from "@aztec/governance/libraries/ProposalLib.sol"; + +contract NoVoteAndExitTest is ApellaBase { + using ProposalLib for DataStructures.Proposal; + // Ensure that it is not possible to BOTH vote on proposal AND withdraw the funds before + // it can be executed + + function test_CannotVoteAndExit( + address _voter, + uint256 _totalPower, + uint256 _votesCast, + uint256 _yeas + ) external { + bytes32 _proposalName = "empty"; + + vm.assume(_voter != address(0)); + proposal = proposals[_proposalName]; + proposalId = proposalIds[_proposalName]; + + uint256 totalPower = bound(_totalPower, proposal.config.minimumVotes, type(uint128).max); + uint256 votesNeeded = Math.mulDiv(totalPower, proposal.config.quorum, 1e18, Math.Rounding.Ceil); + uint256 votesCast = bound(_votesCast, votesNeeded, totalPower); + + uint256 yeaLimitFraction = Math.ceilDiv(1e18 + proposal.config.voteDifferential, 2); + uint256 yeaLimit = Math.mulDiv(votesCast, yeaLimitFraction, 1e18, Math.Rounding.Ceil); + + uint256 yeas = yeaLimit == votesCast ? votesCast : bound(_yeas, yeaLimit + 1, votesCast); + + token.mint(_voter, totalPower); + vm.startPrank(_voter); + token.approve(address(apella), totalPower); + apella.deposit(_voter, totalPower); + vm.stopPrank(); + + // Jump up to the point where the proposal becomes active + vm.warp(Timestamp.unwrap(proposal.pendingThrough()) + 1); + + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Active); + + vm.prank(_voter); + apella.vote(proposalId, yeas, true); + vm.prank(_voter); + apella.vote(proposalId, votesCast - yeas, false); + + vm.prank(_voter); + uint256 withdrawalId = apella.initiateWithdraw(_voter, totalPower); + + // Jump to the block just before we become executable. + vm.warp(Timestamp.unwrap(proposal.queuedThrough())); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Queued); + + vm.warp(Timestamp.unwrap(proposal.queuedThrough()) + 1); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Executable); + + DataStructures.Withdrawal memory withdrawal = apella.getWithdrawal(withdrawalId); + + vm.expectRevert( + abi.encodeWithSelector( + Errors.Apella__WithdrawalNotUnlockedYet.selector, + Timestamp.wrap(block.timestamp), + withdrawal.unlocksAt + ) + ); + apella.finaliseWithdraw(withdrawalId); + + apella.execute(proposalId); + } +} diff --git a/l1-contracts/test/governance/apella/updateConfiguration.t.sol b/l1-contracts/test/governance/apella/updateConfiguration.t.sol index 40eb7472e5b7..b45a5e916dc9 100644 --- a/l1-contracts/test/governance/apella/updateConfiguration.t.sol +++ b/l1-contracts/test/governance/apella/updateConfiguration.t.sol @@ -1,21 +1,354 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity >=0.8.27; -contract UpdateConfigurationTest { - function test_WhenCallerIsNotSelf() external { - // it revert - } - - modifier whenCallerIsSelf() { - _; - } - - function test_WhenConfigurationIsInvalid() external whenCallerIsSelf { - // it revert - } - - function test_WhenConfigurationIsValid() external whenCallerIsSelf { - // it updates the configuration - // it emits {ConfigurationUpdates} event - } +import {ApellaBase} from "./base.t.sol"; +import {IApella} from "@aztec/governance/interfaces/IApella.sol"; +import {Timestamp} from "@aztec/core/libraries/TimeMath.sol"; +import {Errors} from "@aztec/governance/libraries/Errors.sol"; +import {DataStructures} from "@aztec/governance/libraries/DataStructures.sol"; +import {ConfigurationLib} from "@aztec/governance/libraries/ConfigurationLib.sol"; +import {Timestamp} from "@aztec/core/libraries/TimeMath.sol"; + +contract UpdateConfigurationTest is ApellaBase { + using ConfigurationLib for DataStructures.Configuration; + + DataStructures.Configuration internal config; + + // Doing this as we are using a lib that works on storage + DataStructures.Configuration internal fresh; + + function setUp() public override(ApellaBase) { + super.setUp(); + config = apella.getConfiguration(); + } + + function test_WhenCallerIsNotSelf() external { + // it revert + vm.expectRevert( + abi.encodeWithSelector(Errors.Apella__CallerNotSelf.selector, address(this), address(apella)) + ); + apella.updateConfiguration(config); + } + + modifier whenCallerIsSelf() { + _; + } + + modifier whenConfigurationIsInvalid() { + _; + } + + function test_WhenQuorumLtMinOrGtMax(uint256 _val) + external + whenCallerIsSelf + whenConfigurationIsInvalid + { + // it revert + config.quorum = bound(_val, 0, ConfigurationLib.QUORUM_LOWER - 1); + vm.expectRevert( + abi.encodeWithSelector(Errors.Apella__ConfigurationLib__QuorumTooSmall.selector) + ); + + vm.prank(address(apella)); + apella.updateConfiguration(config); + + config.quorum = bound(_val, ConfigurationLib.QUORUM_UPPER + 1, type(uint256).max); + vm.expectRevert(abi.encodeWithSelector(Errors.Apella__ConfigurationLib__QuorumTooBig.selector)); + + vm.prank(address(apella)); + apella.updateConfiguration(config); + } + + function test_WhenDifferentialLtMinOrGtMax(uint256 _val) + external + whenCallerIsSelf + whenConfigurationIsInvalid + { + // it revert + config.voteDifferential = + bound(_val, ConfigurationLib.DIFFERENTIAL_UPPER + 1, type(uint256).max); + vm.expectRevert( + abi.encodeWithSelector(Errors.Apella__ConfigurationLib__DifferentialTooBig.selector) + ); + + vm.prank(address(apella)); + apella.updateConfiguration(config); + } + + function test_WhenMinimumVotesLtMin(uint256 _val) + external + whenCallerIsSelf + whenConfigurationIsInvalid + { + // it revert + config.minimumVotes = bound(_val, 0, ConfigurationLib.VOTES_LOWER - 1); + vm.expectRevert( + abi.encodeWithSelector(Errors.Apella__ConfigurationLib__InvalidMinimumVotes.selector) + ); + + vm.prank(address(apella)); + apella.updateConfiguration(config); + } + + function test_WhenVotingDelayLtMinOrGtMax(uint256 _val) + external + whenCallerIsSelf + whenConfigurationIsInvalid + { + // it revert + + config.votingDelay = + Timestamp.wrap(bound(_val, 0, Timestamp.unwrap(ConfigurationLib.TIME_LOWER) - 1)); + vm.expectRevert( + abi.encodeWithSelector(Errors.Apella__ConfigurationLib__TimeTooSmall.selector, "VotingDelay") + ); + vm.prank(address(apella)); + apella.updateConfiguration(config); + + config.votingDelay = Timestamp.wrap( + bound(_val, Timestamp.unwrap(ConfigurationLib.TIME_UPPER) + 1, type(uint256).max) + ); + vm.expectRevert( + abi.encodeWithSelector(Errors.Apella__ConfigurationLib__TimeTooBig.selector, "VotingDelay") + ); + vm.prank(address(apella)); + apella.updateConfiguration(config); + } + + function test_WhenVotingDurationLtMinOrGtMax(uint256 _val) + external + whenCallerIsSelf + whenConfigurationIsInvalid + { + // it revert + + config.votingDuration = + Timestamp.wrap(bound(_val, 0, Timestamp.unwrap(ConfigurationLib.TIME_LOWER) - 1)); + vm.expectRevert( + abi.encodeWithSelector( + Errors.Apella__ConfigurationLib__TimeTooSmall.selector, "VotingDuration" + ) + ); + vm.prank(address(apella)); + apella.updateConfiguration(config); + + config.votingDuration = Timestamp.wrap( + bound(_val, Timestamp.unwrap(ConfigurationLib.TIME_UPPER) + 1, type(uint256).max) + ); + vm.expectRevert( + abi.encodeWithSelector(Errors.Apella__ConfigurationLib__TimeTooBig.selector, "VotingDuration") + ); + vm.prank(address(apella)); + apella.updateConfiguration(config); + } + + function test_WhenExecutionDelayLtMinOrGtMax(uint256 _val) + external + whenCallerIsSelf + whenConfigurationIsInvalid + { + // it revert + + config.executionDelay = + Timestamp.wrap(bound(_val, 0, Timestamp.unwrap(ConfigurationLib.TIME_LOWER) - 1)); + vm.expectRevert( + abi.encodeWithSelector( + Errors.Apella__ConfigurationLib__TimeTooSmall.selector, "ExecutionDelay" + ) + ); + vm.prank(address(apella)); + apella.updateConfiguration(config); + + config.executionDelay = Timestamp.wrap( + bound(_val, Timestamp.unwrap(ConfigurationLib.TIME_UPPER) + 1, type(uint256).max) + ); + vm.expectRevert( + abi.encodeWithSelector(Errors.Apella__ConfigurationLib__TimeTooBig.selector, "ExecutionDelay") + ); + vm.prank(address(apella)); + apella.updateConfiguration(config); + } + + function test_WhenGracePeriodLtMinOrGtMax(uint256 _val) + external + whenCallerIsSelf + whenConfigurationIsInvalid + { + // it revert + + config.gracePeriod = + Timestamp.wrap(bound(_val, 0, Timestamp.unwrap(ConfigurationLib.TIME_LOWER) - 1)); + vm.expectRevert( + abi.encodeWithSelector(Errors.Apella__ConfigurationLib__TimeTooSmall.selector, "GracePeriod") + ); + vm.prank(address(apella)); + apella.updateConfiguration(config); + + config.gracePeriod = Timestamp.wrap( + bound(_val, Timestamp.unwrap(ConfigurationLib.TIME_UPPER) + 1, type(uint256).max) + ); + vm.expectRevert( + abi.encodeWithSelector(Errors.Apella__ConfigurationLib__TimeTooBig.selector, "GracePeriod") + ); + vm.prank(address(apella)); + apella.updateConfiguration(config); + } + + modifier whenConfigurationIsValid() { + // the local `config` will be modified throughout the execution + // We check that it matches the what is seen on chain afterwards + DataStructures.Configuration memory old = apella.getConfiguration(); + + _; + + vm.expectEmit(true, true, true, true, address(apella)); + emit IApella.ConfigurationUpdated(Timestamp.wrap(block.timestamp)); + vm.prank(address(apella)); + apella.updateConfiguration(config); + + fresh = apella.getConfiguration(); + + assertEq(config.executionDelay, fresh.executionDelay); + assertEq(config.gracePeriod, fresh.gracePeriod); + assertEq(config.minimumVotes, fresh.minimumVotes); + assertEq(config.quorum, fresh.quorum); + assertEq(config.voteDifferential, fresh.voteDifferential); + assertEq(config.votingDelay, fresh.votingDelay); + assertEq(config.votingDuration, fresh.votingDuration); + + assertEq(config.lockDelay(), fresh.lockDelay()); + assertEq( + config.lockDelay(), + Timestamp.wrap(Timestamp.unwrap(fresh.votingDelay) / 5) + fresh.votingDuration + + fresh.executionDelay + ); + + // Ensure that there is a difference between the two + assertFalse( + old.executionDelay == fresh.executionDelay && old.gracePeriod == fresh.gracePeriod + && old.minimumVotes == fresh.minimumVotes && old.quorum == fresh.quorum + && old.voteDifferential == fresh.voteDifferential && old.votingDelay == fresh.votingDelay + && old.votingDuration == fresh.votingDuration + ); + } + + function test_WhenQuorumGeMinAndLeMax(uint256 _val) + external + whenCallerIsSelf + whenConfigurationIsValid + { + // it updates the configuration + // it emits {ConfigurationUpdated} event + + uint256 val = bound(_val, ConfigurationLib.QUORUM_LOWER, ConfigurationLib.QUORUM_UPPER); + + vm.assume(val != config.quorum); + config.quorum = val; + } + + function test_WhenDifferentialGeMinAndLeMax(uint256 _val) + external + whenCallerIsSelf + whenConfigurationIsValid + { + // it updates the configuration + // it emits {ConfigurationUpdated} event + + uint256 val = bound(_val, 0, ConfigurationLib.DIFFERENTIAL_UPPER); + + vm.assume(val != config.voteDifferential); + config.voteDifferential = val; + } + + function test_WhenMinimumVotesGeMin(uint256 _val) + external + whenCallerIsSelf + whenConfigurationIsValid + { + // it updates the configuration + // it emits {ConfigurationUpdated} event + + uint256 val = bound(_val, ConfigurationLib.VOTES_LOWER, type(uint256).max); + + vm.assume(val != config.minimumVotes); + config.minimumVotes = val; + } + + function test_WhenVotingDelayGeMinAndLeMax(uint256 _val) + external + whenCallerIsSelf + whenConfigurationIsValid + { + // it updates the configuration + // it emits {ConfigurationUpdated} event + Timestamp val = Timestamp.wrap( + bound( + _val, + Timestamp.unwrap(ConfigurationLib.TIME_LOWER), + Timestamp.unwrap(ConfigurationLib.TIME_UPPER) + ) + ); + + vm.assume(val != config.votingDelay); + config.votingDelay = val; + } + + function test_WhenVotingDurationGeMinAndLeMax(uint256 _val) + external + whenCallerIsSelf + whenConfigurationIsValid + { + // it updates the configuration + // it emits {ConfigurationUpdated} event + Timestamp val = Timestamp.wrap( + bound( + _val, + Timestamp.unwrap(ConfigurationLib.TIME_LOWER), + Timestamp.unwrap(ConfigurationLib.TIME_UPPER) + ) + ); + + vm.assume(val != config.votingDuration); + config.votingDuration = val; + } + + function test_WhenExecutionDelayGeMinAndLeMax(uint256 _val) + external + whenCallerIsSelf + whenConfigurationIsValid + { + // it updates the configuration + // it emits {ConfigurationUpdated} event + + Timestamp val = Timestamp.wrap( + bound( + _val, + Timestamp.unwrap(ConfigurationLib.TIME_LOWER), + Timestamp.unwrap(ConfigurationLib.TIME_UPPER) + ) + ); + + vm.assume(val != config.executionDelay); + config.executionDelay = val; + } + + function test_WhenGracePeriodGeMinAndLeMax(uint256 _val) + external + whenCallerIsSelf + whenConfigurationIsValid + { + // it updates the configuration + // it emits {ConfigurationUpdated} event + + Timestamp val = Timestamp.wrap( + bound( + _val, + Timestamp.unwrap(ConfigurationLib.TIME_LOWER), + Timestamp.unwrap(ConfigurationLib.TIME_UPPER) + ) + ); + + vm.assume(val != config.gracePeriod); + config.gracePeriod = val; + } } diff --git a/l1-contracts/test/governance/apella/updateConfiguration.tree b/l1-contracts/test/governance/apella/updateConfiguration.tree index ff9c5c0d582d..c453d22712ce 100644 --- a/l1-contracts/test/governance/apella/updateConfiguration.tree +++ b/l1-contracts/test/governance/apella/updateConfiguration.tree @@ -3,7 +3,39 @@ UpdateConfigurationTest │ └── it revert └── when caller is self ├── when configuration is invalid - │ └── it revert + │ ├── when quorum lt min or gt max + │ │ └── it revert + │ ├── when differential lt min or gt max + │ │ └── it revert + │ ├── when minimumVotes lt min + │ │ └── it revert + │ ├── when votingDelay lt min or gt max + │ │ └── it revert + │ ├── when votingDuration lt min or gt max + │ │ └── it revert + │ ├── when executionDelay lt min or gt max + │ │ └── it revert + │ └── when gracePeriod lt min or gt max + │ └── it revert └── when configuration is valid - ├── it updates the configuration - └── it emits {ConfigurationUpdates} event + ├── when quorum ge min and le max + │ ├── it updates the configuration + │ └── it emits {ConfigurationUpdated} event + ├── when differential ge min and le max + │ ├── it updates the configuration + │ └── it emits {ConfigurationUpdated} event + ├── when minimumVotes ge min + │ ├── it updates the configuration + │ └── it emits {ConfigurationUpdated} event + ├── when votingDelay ge min and le max + │ ├── it updates the configuration + │ └── it emits {ConfigurationUpdated} event + ├── when votingDuration ge min and le max + │ ├── it updates the configuration + │ └── it emits {ConfigurationUpdated} event + ├── when executionDelay ge min and le max + │ ├── it updates the configuration + │ └── it emits {ConfigurationUpdated} event + └── when gracePeriod ge min and le max + ├── it updates the configuration + └── it emits {ConfigurationUpdated} event \ No newline at end of file diff --git a/l1-contracts/test/governance/apella/updateGerousia.t.sol b/l1-contracts/test/governance/apella/updateGerousia.t.sol new file mode 100644 index 000000000000..1322eb15fbd7 --- /dev/null +++ b/l1-contracts/test/governance/apella/updateGerousia.t.sol @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.27; + +import {ApellaBase} from "./base.t.sol"; +import {Errors} from "@aztec/governance/libraries/Errors.sol"; +import {IApella} from "@aztec/governance/interfaces/IApella.sol"; + +contract UpdateGerousiaTest is ApellaBase { + function test_WhenCallerIsNotApella(address _caller, address _gerousia) external { + // it revert + vm.assume(_caller != address(apella)); + vm.expectRevert( + abi.encodeWithSelector(Errors.Apella__CallerNotSelf.selector, _caller, address(apella)) + ); + vm.prank(_caller); + apella.updateGerousia(_gerousia); + } + + function test_WhenCallerIsApella(address _gerousia) external { + // it updates the gerousia + // it emit the {GerousiaUpdated} event + + vm.assume(_gerousia != address(apella.gerousia())); + + vm.expectEmit(true, true, true, true, address(apella)); + emit IApella.GerousiaUpdated(_gerousia); + + vm.prank(address(apella)); + apella.updateGerousia(_gerousia); + + assertEq(_gerousia, apella.gerousia()); + } +} diff --git a/l1-contracts/test/governance/apella/updateGerousia.tree b/l1-contracts/test/governance/apella/updateGerousia.tree new file mode 100644 index 000000000000..d2fd3dac6229 --- /dev/null +++ b/l1-contracts/test/governance/apella/updateGerousia.tree @@ -0,0 +1,6 @@ +UpdateGerousiaTest +├── when caller is not apella +│ └── it revert +└── when caller is apella + ├── it updates the gerousia + └── it emit the {GerousiaUpdated} event \ No newline at end of file diff --git a/l1-contracts/test/governance/apella/userlib/powerAt.t.sol b/l1-contracts/test/governance/apella/userlib/powerAt.t.sol index 63c3dd20afb9..64eacf8cfd30 100644 --- a/l1-contracts/test/governance/apella/userlib/powerAt.t.sol +++ b/l1-contracts/test/governance/apella/userlib/powerAt.t.sol @@ -14,7 +14,7 @@ contract PowerAtTest is UserLibBase { function test_WhenTimeNotInPast() external { // it revert - vm.expectRevert(abi.encodeWithSelector(Errors.Apella__NotInPast.selector)); + vm.expectRevert(abi.encodeWithSelector(Errors.Apella__UserLib__NotInPast.selector)); user.powerAt(Timestamp.wrap(block.timestamp)); } diff --git a/l1-contracts/test/governance/apella/userlib/sub.t.sol b/l1-contracts/test/governance/apella/userlib/sub.t.sol index 278c4d206bde..438edf91da96 100644 --- a/l1-contracts/test/governance/apella/userlib/sub.t.sol +++ b/l1-contracts/test/governance/apella/userlib/sub.t.sol @@ -41,7 +41,9 @@ contract SubTest is UserLibBase { amount = bound(amount, sumBefore + 1, type(uint256).max); vm.expectRevert( - abi.encodeWithSelector(Errors.Apella__InsufficientPower.selector, sumBefore, amount) + abi.encodeWithSelector( + Errors.Apella__InsufficientPower.selector, msg.sender, sumBefore, amount + ) ); user.sub(amount); } diff --git a/l1-contracts/test/governance/apella/vote.t.sol b/l1-contracts/test/governance/apella/vote.t.sol index 96ef2a542c1c..fbc24cd53db6 100644 --- a/l1-contracts/test/governance/apella/vote.t.sol +++ b/l1-contracts/test/governance/apella/vote.t.sol @@ -1,34 +1,168 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity >=0.8.27; -contract VoteTest { - function test_GivenStateIsNotActive() external { - // it revert - } - - modifier givenStatIsActive() { - _; - } - - function test_GivenAmountLargerThanAvailablePower() external givenStatIsActive { - // it revert - } - - modifier givenAmountSmallerOrEqAvailablePower() { - _; - } - - function test_WhenSupportIsYea() external givenStatIsActive givenAmountSmallerOrEqAvailablePower { - // it increase yea on user ballot by amount - // it increase yea on total by amount - // it emitts {VoteCast} event - // it returns true - } - - function test_WhenSupportIsNea() external givenStatIsActive givenAmountSmallerOrEqAvailablePower { - // it increase nea on user ballot by amount - // it increase nea on total by amount - // it emitts {VoteCast} event - // it returns true - } +import {IPayload} from "@aztec/governance/interfaces/IPayload.sol"; +import {ApellaBase} from "./base.t.sol"; +import {Errors} from "@aztec/governance/libraries/Errors.sol"; +import {DataStructures} from "@aztec/governance/libraries/DataStructures.sol"; +import {ProposalLib} from "@aztec/governance/libraries/ProposalLib.sol"; +import {Timestamp} from "@aztec/core/libraries/TimeMath.sol"; +import {IApella} from "@aztec/governance/interfaces/IApella.sol"; + +contract VoteTest is ApellaBase { + using ProposalLib for DataStructures.Proposal; + + uint256 internal depositPower; + + function setUp() public override(ApellaBase) { + super.setUp(); + + proposal = proposals["empty"]; + proposalId = proposalIds["empty"]; + } + + modifier givenStateIsNotActive(address _voter, uint256 _amount, bool _support) { + _; + vm.prank(_voter); + vm.expectRevert(abi.encodeWithSelector(Errors.Apella__ProposalNotActive.selector)); + apella.vote(proposalId, _amount, _support); + } + + function test_GivenStateIsPending(address _voter, uint256 _amount, bool _support) + external + givenStateIsNotActive(_voter, _amount, _support) + { + // it revert + _statePending("empty"); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Pending); + } + + function test_GivenStateIsQueued( + address _voter, + uint256 _amount, + bool _support, + uint256 _totalPower, + uint256 _votesCast, + uint256 _yeas + ) external givenStateIsNotActive(_voter, _amount, _support) { + // it revert + _stateQueued("empty", _voter, _totalPower, _votesCast, _yeas); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Queued); + } + + function test_GivenStateIsExecutable( + address _voter, + uint256 _amount, + bool _support, + uint256 _totalPower, + uint256 _votesCast, + uint256 _yeas + ) external givenStateIsNotActive(_voter, _amount, _support) { + // it revert + _stateExecutable("empty", _voter, _totalPower, _votesCast, _yeas); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Executable); + } + + function test_GivenStateIsRejected(address _voter, uint256 _amount, bool _support) + external + givenStateIsNotActive(_voter, _amount, _support) + { + // it revert + _stateRejected("empty"); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Rejected); + } + + function test_GivenStateIsDropped( + address _voter, + uint256 _amount, + bool _support, + address _gerousia + ) external givenStateIsNotActive(_voter, _amount, _support) { + // it revert + _stateDropped("empty", _gerousia); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Dropped); + } + + function test_GivenStateIsExpired( + address _voter, + uint256 _amount, + bool _support, + uint256 _totalPower, + uint256 _votesCast, + uint256 _yeas + ) external givenStateIsNotActive(_voter, _amount, _support) { + // it revert + _stateExpired("empty", _voter, _totalPower, _votesCast, _yeas); + assertEq(apella.getProposalState(proposalId), DataStructures.ProposalState.Expired); + } + + modifier givenStateIsActive(address _voter, uint256 _depositPower) { + vm.assume(_voter != address(0)); + depositPower = bound(_depositPower, 1, type(uint128).max); + + token.mint(_voter, depositPower); + vm.startPrank(_voter); + token.approve(address(apella), depositPower); + apella.deposit(_voter, depositPower); + vm.stopPrank(); + + assertEq(token.balanceOf(address(apella)), depositPower); + assertEq(apella.powerAt(_voter, Timestamp.wrap(block.timestamp)), depositPower); + + _stateActive("empty"); + + _; + } + + function test_GivenAmountLargerThanAvailablePower( + address _voter, + uint256 _depositPower, + uint256 _votePower, + bool _support + ) external givenStateIsActive(_voter, _depositPower) { + // it revert + + uint256 power = bound(_votePower, depositPower + 1, type(uint256).max); + + vm.expectRevert( + abi.encodeWithSelector(Errors.Apella__InsufficientPower.selector, _voter, depositPower, power) + ); + vm.prank(_voter); + apella.vote(proposalId, power, _support); + } + + function test_GivenAmountSmallerOrEqAvailablePower( + address _voter, + uint256 _depositPower, + uint256 _votePower, + bool _support + ) external givenStateIsActive(_voter, _depositPower) { + // it increase yea or nea on user ballot by amount + // it increase yea or nea on total by amount + // it emits {VoteCast} event + // it returns true + + uint256 power = bound(_votePower, 1, depositPower); + + vm.expectEmit(true, true, true, true, address(apella)); + emit IApella.VoteCast(proposalId, _voter, _support, power); + vm.prank(_voter); + apella.vote(proposalId, power, _support); + + DataStructures.Proposal memory fresh = apella.getProposal(proposalId); + + assertEq(proposal.config.executionDelay, fresh.config.executionDelay, "executionDelay"); + assertEq(proposal.config.gracePeriod, fresh.config.gracePeriod, "gracePeriod"); + assertEq(proposal.config.minimumVotes, fresh.config.minimumVotes, "minimumVotes"); + assertEq(proposal.config.quorum, fresh.config.quorum, "quorum"); + assertEq(proposal.config.voteDifferential, fresh.config.voteDifferential, "voteDifferential"); + assertEq(proposal.config.votingDelay, fresh.config.votingDelay, "votingDelay"); + assertEq(proposal.config.votingDuration, fresh.config.votingDuration, "votingDuration"); + assertEq(proposal.creation, fresh.creation, "creation"); + assertEq(proposal.creator, fresh.creator, "creator"); + assertEq(proposal.summedBallot.nea + (_support ? 0 : power), fresh.summedBallot.nea, "nea"); + assertEq(proposal.summedBallot.yea + (_support ? power : 0), fresh.summedBallot.yea, "yea"); + // The "written" state is still the same. + assertTrue(proposal.state == fresh.state, "state"); + } } diff --git a/l1-contracts/test/governance/apella/vote.tree b/l1-contracts/test/governance/apella/vote.tree index b3eebbaba6ce..deecfed732d3 100644 --- a/l1-contracts/test/governance/apella/vote.tree +++ b/l1-contracts/test/governance/apella/vote.tree @@ -1,17 +1,22 @@ VoteTest ├── given state is not active -│ └── it revert -└── given stat is active +│ ├── given state is pending +│ │ └── it revert +│ ├── given state is queued +│ │ └── it revert +│ ├── given state is executable +│ │ └── it revert +│ ├── given state is rejected +│ │ └── it revert +│ ├── given state is dropped +│ │ └── it revert +│ └── given state is expired +│ └── it revert +└── given state is active ├── given amount larger than available power │ └── it revert └── given amount smaller or eq available power - ├── when support is yea - │ ├── it increase yea on user ballot by amount - │ ├── it increase yea on total by amount - │ ├── it emitts {VoteCast} event - │ └── it returns true - └── when support is nea - ├── it increase nea on user ballot by amount - ├── it increase nea on total by amount - ├── it emitts {VoteCast} event - └── it returns true \ No newline at end of file + ├── it increase yea or nea on user ballot by amount + ├── it increase yea or nea on total by amount + ├── it emits {VoteCast} event + └── it returns true \ No newline at end of file