refactor: compression of config in gov to reduce costs. (#16603)

Spearbit suggested altering the `configuration` storage to use a map and
just have an id in the proposal. However, we can heavily compress the
values of the configuration that needs to actually go into the proposal.
So I believe a better option is to do that (we can get it down to 2
slots), as we then reduced the number of reads on more important
functions such as `vote` with only a minor added cost over the id for
the proposal itself.

- Ballots can be stored in a single `uint256` as they will never
meaningfully be size > `uint128`.
- All the timings in the configuration can be compressed to `uint32`
(the `CompressedTimestamp` that we already have).
- Altering the ordering in the `Proposal` allow us to more tightly pack
it for storage.

It is a bit more tricky to update some of the storage in tests, but we
can utilise a testonly wrapper that allow direct manipulation to address
that.

```solidity
struct CompressedConfiguration {
  // Slot 1: Timing and percentages - 32*4 + 64*2 = 256 bits
  CompressedTimestamp votingDelay;
  CompressedTimestamp votingDuration;
  CompressedTimestamp executionDelay;
  CompressedTimestamp gracePeriod;
  uint64 quorum;
  uint64 requiredYeaMargin;
  // Slot 2: Amounts and proposeConfig - 96 + 96 + 32 = 224 bits (32 bits unused)
  uint96 minimumVotes;
  uint96 lockAmount;
  CompressedTimestamp lockDelay;
}

struct CompressedProposal {
  // Slot 1: Core Identity (256 bits)
  address proposer; // 160 bits
  uint96 minimumVotes; // 96 bits - from config
  // Slot 2: Timing (232 bits used, 24 bits padding)
  ProposalState cachedState; // 8 bits
  CompressedTimestamp creation; // 32 bits
  CompressedTimestamp votingDelay; // 32 bits - from config
  CompressedTimestamp votingDuration; // 32 bits - from config
  CompressedTimestamp executionDelay; // 32 bits - from config
  CompressedTimestamp gracePeriod; // 32 bits - from config
  uint64 quorum; // 64 bits - from config
  // Slot 3: Votes (256 bits)
  CompressedBallot summedBallot; // 256 bits (128 yea + 128 nay)
  // Slot 4: References (224 bits used, 32 bits padding)
  IPayload payload; // 160 bits
  uint64 requiredYeaMargin; // 64 bits - from config
}
```

Author: LHerskind

l1-contracts/foundry.toml

@@ -16,6 +16,7 @@ gas_reports = [
   "CoinIssuer",
   "FeeJuicePortal",
   "Governance",
+  "TestGov",
   "GovernanceProposer",
   "GSE",
   "Inbox",
@@ -37,38 +38,37 @@ remappings = [
   "@aztec/=src",
   "@test/=test",
   "@zkpassport/=lib/circuits/src/solidity/src/",
-  "@zkpassport-test/=lib/circuits/src/solidity/test"
+  "@zkpassport-test/=lib/circuits/src/solidity/test",
 ]
 
 # See more config options https://github.com/foundry-rs/foundry/tree/master/config
 
 fs_permissions = [
-  {access = "read", path = "./test/fixtures/bn254_constants.json"},
-  {access = "read", path = "./test/fixtures/mixed_block_1.json"},
-  {access = "read", path = "./test/fixtures/mixed_block_2.json"},
-  {access = "read", path = "./test/fixtures/single_tx_block_1.json"},
-  {access = "read", path = "./test/fixtures/single_tx_block_2.json"},
-  {access = "read", path = "./test/fixtures/empty_block_1.json"},
-  {access = "read", path = "./test/fixtures/empty_block_2.json"},
-  {access = "read", path = "./test/fixtures/fee_data_points.json"},
-  {access = "read", path = "./test/staking_asset_handler/zkpassport/fixtures/valid_proof.hex"},
-  {access = "read", path = "./test/staking_asset_handler/zkpassport/fixtures/valid_committed_inputs.hex"},
-  {access = "read", path = "./test/staking_asset_handler/zkpassport/fixtures/valid_public_inputs.json"},
-  {access = "read", path = "./test/fixtures/boosted_rewards/shares.json"},
-  {access = "read", path = "./test/fixtures/boosted_rewards/activity_scores.json"},
-  {access = "read", path = "./script/registration_data.json"},
+  { access = "read", path = "./test/fixtures/bn254_constants.json" },
+  { access = "read", path = "./test/fixtures/mixed_block_1.json" },
+  { access = "read", path = "./test/fixtures/mixed_block_2.json" },
+  { access = "read", path = "./test/fixtures/single_tx_block_1.json" },
+  { access = "read", path = "./test/fixtures/single_tx_block_2.json" },
+  { access = "read", path = "./test/fixtures/empty_block_1.json" },
+  { access = "read", path = "./test/fixtures/empty_block_2.json" },
+  { access = "read", path = "./test/fixtures/fee_data_points.json" },
+  { access = "read", path = "./test/staking_asset_handler/zkpassport/fixtures/valid_proof.hex" },
+  { access = "read", path = "./test/staking_asset_handler/zkpassport/fixtures/valid_committed_inputs.hex" },
+  { access = "read", path = "./test/staking_asset_handler/zkpassport/fixtures/valid_public_inputs.json" },
+  { access = "read", path = "./test/fixtures/boosted_rewards/shares.json" },
+  { access = "read", path = "./test/fixtures/boosted_rewards/activity_scores.json" },
+  { access = "read", path = "./script/registration_data.json" },
 ]
 
-no_match_contract="(ScreamAndShoutTest|UniswapPortalTest|MerkleCheck)"
+no_match_contract = "(ScreamAndShoutTest|UniswapPortalTest|MerkleCheck)"
 
 [fmt]
 line_length = 120
 tab_width = 2
-variable_override_spacing=false
+variable_override_spacing = false
 wrap_comments = true
 number_underscore = "thousands"
 override_spacing = false
 
 [rpc_endpoints]
-mainnet_fork="https://mainnet.infura.io/v3/9928b52099854248b3a096be07a6b23c"
-
+mainnet_fork = "https://mainnet.infura.io/v3/9928b52099854248b3a096be07a6b23c"

l1-contracts/script/InternalGov.s.sol

@@ -13,7 +13,6 @@ import {IRegistry} from "@aztec/governance/interfaces/IRegistry.sol";
 import {IValidatorSelection} from "@aztec/core/interfaces/IValidatorSelection.sol";
 import {IPayload} from "@aztec/governance/interfaces/IPayload.sol";
 import {RegisterNewRollupVersionPayload} from "../test/governance/scenario/RegisterNewRollupVersionPayload.sol";
-import {ProposalLib} from "@aztec/governance/libraries/ProposalLib.sol";
 import {Ownable} from "@oz/access/Ownable.sol";
 import {Fakerollup} from "../test/governance/governance-proposer/mocks/Fakerollup.sol";
 import {StakingAssetHandler} from "../src/mock/StakingAssetHandler.sol";
@@ -23,9 +22,10 @@ import {IStaking} from "@aztec/core/interfaces/IStaking.sol";
 import {RewardDistributor} from "@aztec/governance/RewardDistributor.sol";
 import {IInstance} from "@aztec/core/interfaces/IInstance.sol";
 import {RoundAccounting} from "@aztec/governance/proposer/EmpireBase.sol";
+import {UncompressedProposalWrapper} from "@test/governance/helpers/UncompressedProposalTestLib.sol";
 
 contract GovScript is Test {
-  using ProposalLib for Proposal;
+  UncompressedProposalWrapper internal upw = new UncompressedProposalWrapper();
 
   address internal constant ME = address(0xf8d7d601759CBcfB78044bA7cA9B0c0D6301A54f);
 
@@ -138,14 +138,14 @@ contract GovScript is Test {
     proposal = governance.getProposal(_proposalId);
     ProposalState state = governance.getProposalState(_proposalId);
 
-    Timestamp pendingThrough = proposal.pendingThrough();
+    Timestamp pendingThrough = upw.pendingThrough(proposal);
 
     emit log_named_string("Proposal state", stateNames[uint256(state)]);
     emit log_named_address("Proposal payload", address(proposal.payload));
     emit log_named_uint("pendingThrough   ", Timestamp.unwrap(pendingThrough));
-    emit log_named_uint("activeThrough    ", Timestamp.unwrap(proposal.activeThrough()));
-    emit log_named_uint("queuedThrough    ", Timestamp.unwrap(proposal.queuedThrough()));
-    emit log_named_uint("executableThrough", Timestamp.unwrap(proposal.executableThrough()));
+    emit log_named_uint("activeThrough    ", Timestamp.unwrap(upw.activeThrough(proposal)));
+    emit log_named_uint("queuedThrough    ", Timestamp.unwrap(upw.queuedThrough(proposal)));
+    emit log_named_uint("executableThrough", Timestamp.unwrap(upw.executableThrough(proposal)));
     emit log_named_uint("creation         ", Timestamp.unwrap(proposal.creation));
     emit log_named_decimal_uint("yeaCount         ", proposal.summedBallot.yea, 18);
     emit log_named_decimal_uint("nayCount         ", proposal.summedBallot.nay, 18);
@@ -317,7 +317,7 @@ contract GovScript is Test {
 
     proposal = governance.getProposal(_proposalId);
 
-    uint256 power = governance.powerAt(ME, proposal.pendingThrough());
+    uint256 power = governance.powerAt(ME, upw.pendingThrough(proposal));
     emit log_named_decimal_uint("power", power, 18);
 
     vm.startBroadcast(ME);

l1-contracts/src/core/libraries/rollup/StakingLib.sol

@@ -147,7 +147,7 @@ library StakingLib {
     Proposal memory proposal = gov.getProposal(_proposalId);
     require(proposal.proposer == address(govProposer), Errors.Staking__IncorrectGovProposer(_proposalId));
 
-    Timestamp ts = proposal.pendingThroughMemory();
+    Timestamp ts = proposal.creation + proposal.config.votingDelay;
 
     // Cast votes with all our power
     uint256 vp = store.gse.getVotingPowerAt(address(this), ts);

l1-contracts/src/governance/GSE.sol

@@ -10,7 +10,6 @@ import {
   DepositDelegationLib, DepositAndDelegationAccounting
 } from "@aztec/governance/libraries/DepositDelegationLib.sol";
 import {Errors} from "@aztec/governance/libraries/Errors.sol";
-import {ProposalLib} from "@aztec/governance/libraries/ProposalLib.sol";
 import {BN254Lib, G1Point, G2Point} from "@aztec/shared/libraries/BN254Lib.sol";
 import {Timestamp} from "@aztec/shared/libraries/TimeMath.sol";
 import {Ownable} from "@oz/access/Ownable.sol";
@@ -122,7 +121,6 @@ contract GSECore is IGSECore, Ownable {
   using SafeCast for uint224;
   using Checkpoints for Checkpoints.Trace224;
   using DepositDelegationLib for DepositAndDelegationAccounting;
-  using ProposalLib for Proposal;
 
   /**
    * Create a special "bonus" address for use by the latest rollup.
@@ -619,7 +617,9 @@ contract GSECore is IGSECore, Ownable {
   }
 
   function _pendingThrough(uint256 _proposalId) internal view returns (Timestamp) {
-    return getGovernance().getProposal(_proposalId).pendingThroughMemory();
+    // Directly compute pendingThrough for memory proposal
+    Proposal memory proposal = getGovernance().getProposal(_proposalId);
+    return proposal.creation + proposal.config.votingDelay;
   }
 }
 

l1-contracts/src/governance/Governance.sol

@@ -7,11 +7,17 @@ import {
   Proposal,
   ProposalState,
   Configuration,
-  Ballot,
+  ProposeConfiguration,
   Withdrawal
 } from "@aztec/governance/interfaces/IGovernance.sol";
 import {IPayload} from "@aztec/governance/interfaces/IPayload.sol";
 import {Checkpoints, CheckpointedUintLib} from "@aztec/governance/libraries/CheckpointedUintLib.sol";
+import {Ballot, CompressedBallot, BallotLib} from "@aztec/governance/libraries/compressed-data/Ballot.sol";
+import {
+  CompressedConfiguration,
+  CompressedConfigurationLib
+} from "@aztec/governance/libraries/compressed-data/Configuration.sol";
+import {CompressedProposal, CompressedProposalLib} from "@aztec/governance/libraries/compressed-data/Proposal.sol";
 import {ConfigurationLib} from "@aztec/governance/libraries/ConfigurationLib.sol";
 import {Errors} from "@aztec/governance/libraries/Errors.sol";
 import {ProposalLib, VoteTabulationReturn} from "@aztec/governance/libraries/ProposalLib.sol";
@@ -129,9 +135,13 @@ struct DepositControl {
  */
 contract Governance is IGovernance {
   using SafeERC20 for IERC20;
-  using ProposalLib for Proposal;
+  using ProposalLib for CompressedProposal;
   using CheckpointedUintLib for Checkpoints.Trace224;
   using ConfigurationLib for Configuration;
+  using ConfigurationLib for CompressedConfiguration;
+  using CompressedConfigurationLib for CompressedConfiguration;
+  using CompressedProposalLib for CompressedProposal;
+  using BallotLib for CompressedBallot;
 
   IERC20 public immutable ASSET;
 
@@ -155,15 +165,16 @@ contract Governance is IGovernance {
    * New proposals are created by calling `_propose`, via `propose` or `proposeWithLock`.
    * The storage of a proposal may be modified by calling `vote`, `execute`, or `dropProposal`.
    */
-  mapping(uint256 proposalId => Proposal proposal) internal proposals;
+  mapping(uint256 proposalId => CompressedProposal proposal) internal proposals;
 
   /**
    * @dev The ballots that have been cast for each proposal.
    *
-   * `Ballot`s contain a `yea` and `nay` count, which are the number of votes for and against the proposal.
+   * `CompressedBallot`s contain a compressed `yea` and `nay` count (uint128 each packed into uint256),
+   * which are the number of votes for and against the proposal.
    * `ballots` is only updated during `vote`.
    */
-  mapping(uint256 proposalId => mapping(address user => Ballot ballot)) public ballots;
+  mapping(uint256 proposalId => mapping(address user => CompressedBallot ballot)) internal ballots;
 
   /**
    * @dev Checkpointed deposit amounts for an address.
@@ -185,7 +196,7 @@ contract Governance is IGovernance {
    * `configuration` is set in the constructor, and is only updated during `updateConfiguration`,
    * which must be done via a proposal.
    */
-  Configuration internal configuration;
+  CompressedConfiguration internal configuration;
 
   /**
    * @dev The total power of the governance contract.
@@ -237,8 +248,8 @@ contract Governance is IGovernance {
     ASSET = _asset;
     governanceProposer = _governanceProposer;
 
-    configuration = _configuration;
-    configuration.assertValid();
+    _configuration.assertValid();
+    configuration = CompressedConfigurationLib.compress(_configuration);
 
     // Unnecessary to set, but better clarity.
     depositControl.allBeneficiariesAllowed = false;
@@ -296,7 +307,7 @@ contract Governance is IGovernance {
     // This following MUST revert if the configuration is invalid
     _configuration.assertValid();
 
-    configuration = _configuration;
+    configuration = CompressedConfigurationLib.compress(_configuration);
 
     emit ConfigurationUpdated(Timestamp.wrap(block.timestamp));
   }
@@ -332,14 +343,14 @@ contract Governance is IGovernance {
    * @notice Initiate a withdrawal of funds from the governance contract,
    * decreasing the power of the beneficiary within the governance contract.
    *
-   * @dev the withdraw may be finalized by anyone after configuration.withdrawalDelay() has passed.
+   * @dev the withdraw may be finalized by anyone after configuration.getWithdrawalDelay() has passed.
    *
    * @param _to The address that will receive the funds when the withdrawal is finalized.
    * @param _amount The amount of power to reduce, and thus funds to withdraw.
    * @return The id of the withdrawal, passed to `finalizeWithdraw`.
    */
   function initiateWithdraw(address _to, uint256 _amount) external override(IGovernance) returns (uint256) {
-    return _initiateWithdraw(msg.sender, _to, _amount, configuration.withdrawalDelay());
+    return _initiateWithdraw(msg.sender, _to, _amount, configuration.getWithdrawalDelay());
   }
 
   /**
@@ -405,7 +416,8 @@ contract Governance is IGovernance {
    * @return The id of the proposal
    */
   function proposeWithLock(IPayload _proposal, address _to) external override(IGovernance) returns (uint256) {
-    _initiateWithdraw(msg.sender, _to, configuration.proposeConfig.lockAmount, configuration.proposeConfig.lockDelay);
+    ProposeConfiguration memory proposeConfig = configuration.getProposeConfig();
+    _initiateWithdraw(msg.sender, _to, proposeConfig.lockAmount, proposeConfig.lockDelay);
     return _propose(_proposal, address(this));
   }
 
@@ -435,18 +447,18 @@ contract Governance is IGovernance {
     // alter the power while the proposal is active since all txs in a block have the same timestamp.
     uint256 userPower = users[msg.sender].valueAt(proposals[_proposalId].pendingThrough());
 
-    Ballot storage userBallot = ballots[_proposalId][msg.sender];
+    CompressedBallot userBallot = ballots[_proposalId][msg.sender];
 
-    uint256 availablePower = userPower - (userBallot.nay + userBallot.yea);
+    uint256 availablePower = userPower - (userBallot.getNay() + userBallot.getYea());
     require(_amount <= availablePower, Errors.Governance__InsufficientPower(msg.sender, availablePower, _amount));
 
-    Ballot storage summedBallot = proposals[_proposalId].summedBallot;
+    CompressedProposal storage proposal = proposals[_proposalId];
     if (_support) {
-      userBallot.yea += _amount;
-      summedBallot.yea += _amount;
+      ballots[_proposalId][msg.sender] = userBallot.addYea(_amount);
+      proposal.addYea(_amount);
     } else {
-      userBallot.nay += _amount;
-      summedBallot.nay += _amount;
+      ballots[_proposalId][msg.sender] = userBallot.addNay(_amount);
+      proposal.addNay(_amount);
     }
 
     emit VoteCast(_proposalId, msg.sender, _support, _amount);
@@ -470,7 +482,7 @@ contract Governance is IGovernance {
     ProposalState state = getProposalState(_proposalId);
     require(state == ProposalState.Executable, Errors.Governance__ProposalNotExecutable());
 
-    Proposal storage proposal = proposals[_proposalId];
+    CompressedProposal storage proposal = proposals[_proposalId];
     proposal.cachedState = ProposalState.Executed;
 
     IPayload.Action[] memory actions = proposal.payload.getActions();
@@ -496,7 +508,7 @@ contract Governance is IGovernance {
    * @param _proposalId The id of the proposal to mark as `Dropped`.
    */
   function dropProposal(uint256 _proposalId) external override(IGovernance) returns (bool) {
-    Proposal storage self = proposals[_proposalId];
+    CompressedProposal storage self = proposals[_proposalId];
     require(self.cachedState != ProposalState.Dropped, Errors.Governance__ProposalAlreadyDropped());
     require(getProposalState(_proposalId) == ProposalState.Droppable, Errors.Governance__ProposalCannotBeDropped());
 
@@ -577,7 +589,7 @@ contract Governance is IGovernance {
   }
 
   function getConfiguration() external view override(IGovernance) returns (Configuration memory) {
-    return configuration;
+    return configuration.decompress();
   }
 
   /**
@@ -589,7 +601,7 @@ contract Governance is IGovernance {
    * @return The proposal.
    */
   function getProposal(uint256 _proposalId) external view override(IGovernance) returns (Proposal memory) {
-    return proposals[_proposalId];
+    return proposals[_proposalId].decompress();
   }
 
   /**
@@ -604,6 +616,19 @@ contract Governance is IGovernance {
     return withdrawals[_withdrawalId];
   }
 
+  /**
+   * @notice Get a user's ballot for a specific proposal.
+   *
+   * @dev Returns the uncompressed Ballot struct for external callers.
+   *
+   * @param _proposalId The id of the proposal.
+   * @param _user The address of the user.
+   * @return The user's ballot with yea and nay votes.
+   */
+  function getBallot(uint256 _proposalId, address _user) external view override(IGovernance) returns (Ballot memory) {
+    return ballots[_proposalId][_user].decompress();
+  }
+
   /**
    * @notice Get the state of a proposal in the governance system
    *
@@ -648,7 +673,7 @@ contract Governance is IGovernance {
   function getProposalState(uint256 _proposalId) public view override(IGovernance) returns (ProposalState) {
     require(_proposalId < proposalCount, Errors.Governance__ProposalDoesNotExists(_proposalId));
 
-    Proposal storage self = proposals[_proposalId];
+    CompressedProposal storage self = proposals[_proposalId];
 
     // A proposal's state is "stable" after `execute` or `dropProposal` has been called on it.
     // In this case, the state of the proposal as returned by `getProposalState` is the same as the cached state,
@@ -735,14 +760,8 @@ contract Governance is IGovernance {
   function _propose(IPayload _proposal, address _proposer) internal returns (uint256) {
     uint256 proposalId = proposalCount++;
 
-    proposals[proposalId] = Proposal({
-      config: configuration,
-      cachedState: ProposalState.Pending,
-      payload: _proposal,
-      proposer: _proposer,
-      creation: Timestamp.wrap(block.timestamp),
-      summedBallot: Ballot({yea: 0, nay: 0})
-    });
+    proposals[proposalId] =
+      CompressedProposalLib.create(_proposer, _proposal, Timestamp.wrap(block.timestamp), configuration);
 
     emit Proposed(proposalId, address(_proposal));