test: More tests + cleanup

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"
   }
 }

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,25 +51,24 @@ 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)
     external
     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;
     }
 

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);

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;
   }

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;

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();