refactor: general cleanup, variables/events/functions renaming

ensure functions/events/variables/test descriptions reflect the latest code changes
optimize contracts and remove unnecessary logic

cli/ts/commands/genLocalState.ts

@@ -95,7 +95,7 @@ export const genLocalState = async ({
       .queryFilter(pollContract.filters.MergeMessageAq(messageRoot), fromBlock)
       .then((events) => events[events.length - 1]?.blockNumber),
     pollContract
-      .queryFilter(pollContract.filters.MergeMaciStateAq(stateRoot, numSignups), fromBlock)
+      .queryFilter(pollContract.filters.MergeMaciState(stateRoot, numSignups), fromBlock)
       .then((events) => events[events.length - 1]?.blockNumber),
   ]).then((blocks) => Math.max(...blocks));
 

cli/ts/commands/genProofs.ts

@@ -156,7 +156,7 @@ export const genProofs = async ({
   const messageAqContract = AccQueueFactory.connect(messageAqContractAddr, signer);
 
   // Check that the state and message trees have been merged
-  if (!(await pollContract.stateAqMerged())) {
+  if (!(await pollContract.stateMerged())) {
     logError("The state tree has not been merged yet. Please use the mergeSignups subcommmand to do so.");
   }
 
@@ -201,7 +201,7 @@ export const genProofs = async ({
         .queryFilter(pollContract.filters.MergeMessageAq(messageRoot), fromBlock)
         .then((events) => events[events.length - 1]?.blockNumber),
       pollContract
-        .queryFilter(pollContract.filters.MergeMaciStateAq(stateRoot, numSignups), fromBlock)
+        .queryFilter(pollContract.filters.MergeMaciState(stateRoot, numSignups), fromBlock)
         .then((events) => events[events.length - 1]?.blockNumber),
     ]).then((blocks) => Math.max(...blocks));
 

cli/ts/commands/mergeMessages.ts

@@ -58,14 +58,6 @@ export const mergeMessages = async ({
 
   const accQueueContract = AccQueueFactory.connect(messageAqContractAddr, signer);
 
-  // we need to ensure that the signer is the owner of the poll contract
-  // this is because only the owner can merge the message AQ
-  const pollOwner = await pollContract.owner();
-  const signerAddress = await signer.getAddress();
-  if (pollOwner.toLowerCase() !== signerAddress.toLowerCase()) {
-    logError("The signer is not the owner of this Poll contract");
-  }
-
   // check if it's time to merge the message AQ
   const dd = await pollContract.getDeployTimeAndDuration();
   const deadline = Number(dd[0]) + Number(dd[1]);

cli/ts/commands/mergeSignups.ts

@@ -54,10 +54,10 @@ export const mergeSignups = async ({ pollId, maciAddress, signer, quiet = true }
     logError("Voting period is not over");
   }
 
-  if (!(await pollContract.stateAqMerged())) {
+  if (!(await pollContract.stateMerged())) {
     // go and merge the state tree
-    logYellow(quiet, info("Merging subroots to a main state root..."));
-    const tx = await pollContract.mergeMaciStateAq();
+    logYellow(quiet, info("Calculating root and storing on Poll..."));
+    const tx = await pollContract.mergeMaciState();
     const receipt = await tx.wait();
 
     if (receipt?.status !== 1) {

cli/ts/commands/poll.ts

@@ -42,7 +42,7 @@ export const getPoll = async ({
 
   const [[deployTime, duration], isStateAqMerged] = await Promise.all([
     pollContract.getDeployTimeAndDuration(),
-    pollContract.stateAqMerged(),
+    pollContract.stateMerged(),
   ]);
 
   const numSignups = await (isStateAqMerged ? pollContract.numSignups() : maciContract.numSignUps());

contracts/contracts/MACI.sol

@@ -12,21 +12,19 @@ import { TopupCredit } from "./TopupCredit.sol";
 import { Utilities } from "./utilities/Utilities.sol";
 import { DomainObjs } from "./utilities/DomainObjs.sol";
 import { CurveBabyJubJub } from "./crypto/BabyJubJub.sol";
-import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
 import { InternalLazyIMT, LazyIMTData } from "./trees/LazyIMT.sol";
 
 /// @title MACI - Minimum Anti-Collusion Infrastructure Version 1
 /// @notice A contract which allows users to sign up, and deploy new polls
-contract MACI is IMACI, DomainObjs, Params, Utilities, Ownable(msg.sender) {
+contract MACI is IMACI, DomainObjs, Params, Utilities {
   /// @notice The state tree depth is fixed. As such it should be as large as feasible
-  /// so that there can be as many users as possible.  i.e. 5 ** 10 = 9765625
+  /// so that there can be as many users as possible.  i.e. 2 ** 23 = 8388608
   /// this should also match the parameter of the circom circuits.
-  uint8 public immutable stateTreeDepth;
-
   /// @notice IMPORTANT: remember to change the ballot tree depth
   /// in contracts/ts/genEmptyBallotRootsContract.ts file
   /// if we change the state tree depth!
-  uint8 internal constant STATE_TREE_SUBDEPTH = 2;
+  uint8 public immutable stateTreeDepth;
+
   uint8 internal constant TREE_ARITY = 2;
   uint8 internal constant MESSAGE_TREE_ARITY = 5;
 
@@ -86,20 +84,11 @@ contract MACI is IMACI, DomainObjs, Params, Utilities, Ownable(msg.sender) {
     PollContracts pollAddr
   );
 
-  /// @notice Only allow a Poll contract to call the modified function.
-  modifier onlyPoll(uint256 _pollId) {
-    if (msg.sender != address(polls[_pollId])) revert CallerMustBePoll(msg.sender);
-    _;
-  }
-
   /// @notice custom errors
-  error CallerMustBePoll(address _caller);
   error PoseidonHashLibrariesNotLinked();
   error TooManySignups();
   error InvalidPubKey();
-  error PreviousPollNotCompleted(uint256 pollId);
   error PollDoesNotExist(uint256 pollId);
-  error SignupTemporaryBlocked();
 
   /// @notice Create a new instance of the MACI contract.
   /// @param _pollFactory The PollFactory contract
@@ -190,7 +179,7 @@ contract MACI is IMACI, DomainObjs, Params, Utilities, Ownable(msg.sender) {
     address _verifier,
     address _vkRegistry,
     Mode _mode
-  ) public virtual onlyOwner returns (PollContracts memory pollAddr) {
+  ) public virtual returns (PollContracts memory pollAddr) {
     // cache the poll to a local variable so we can increment it
     uint256 pollId = nextPollId;
 
@@ -210,20 +199,13 @@ contract MACI is IMACI, DomainObjs, Params, Utilities, Ownable(msg.sender) {
       maxVoteOptions: uint256(MESSAGE_TREE_ARITY) ** _treeDepths.voteOptionTreeDepth
     });
 
-    address _owner = owner();
+    // the owner of the message processor and tally contract will be the msg.sender
+    address _msgSender = msg.sender;
 
-    address p = pollFactory.deploy(
-      _duration,
-      maxValues,
-      _treeDepths,
-      _coordinatorPubKey,
-      address(this),
-      topupCredit,
-      _owner
-    );
+    address p = pollFactory.deploy(_duration, maxValues, _treeDepths, _coordinatorPubKey, address(this), topupCredit);
 
-    address mp = messageProcessorFactory.deploy(_verifier, _vkRegistry, p, _owner, _mode);
-    address tally = tallyFactory.deploy(_verifier, _vkRegistry, p, mp, _owner, _mode);
+    address mp = messageProcessorFactory.deploy(_verifier, _vkRegistry, p, _msgSender, _mode);
+    address tally = tallyFactory.deploy(_verifier, _vkRegistry, p, mp, _msgSender, _mode);
 
     polls[pollId] = p;
 

contracts/contracts/MessageProcessor.sol

@@ -17,13 +17,12 @@ import { DomainObjs } from "./utilities/DomainObjs.sol";
 /// @dev MessageProcessor is used to process messages published by signup users.
 /// It will process message by batch due to large size of messages.
 /// After it finishes processing, the sbCommitment will be used for Tally and Subsidy contracts.
-contract MessageProcessor is Ownable(msg.sender), SnarkCommon, Hasher, CommonUtilities, IMessageProcessor, DomainObjs {
+contract MessageProcessor is Ownable, SnarkCommon, Hasher, CommonUtilities, IMessageProcessor, DomainObjs {
   /// @notice custom errors
   error NoMoreMessages();
-  error StateAqNotMerged();
+  error StateNotMerged();
   error MessageAqNotMerged();
   error InvalidProcessMessageProof();
-  error VkNotSet();
   error MaxVoteOptionsTooLarge();
   error NumSignUpsTooLarge();
   error CurrentMessageBatchIndexTooLarge();
@@ -55,8 +54,15 @@ contract MessageProcessor is Ownable(msg.sender), SnarkCommon, Hasher, CommonUti
   /// @param _verifier The Verifier contract address
   /// @param _vkRegistry The VkRegistry contract address
   /// @param _poll The Poll contract address
+  /// @param _mpOwner The owner of the MessageProcessor contract
   /// @param _mode Voting mode
-  constructor(address _verifier, address _vkRegistry, address _poll, Mode _mode) payable {
+  constructor(
+    address _verifier,
+    address _vkRegistry,
+    address _poll,
+    address _mpOwner,
+    Mode _mode
+  ) payable Ownable(_mpOwner) {
     verifier = IVerifier(_verifier);
     vkRegistry = IVkRegistry(_vkRegistry);
     poll = IPoll(_poll);
@@ -77,8 +83,8 @@ contract MessageProcessor is Ownable(msg.sender), SnarkCommon, Hasher, CommonUti
     }
 
     // The state AccQueue must be merged
-    if (!poll.stateAqMerged()) {
-      revert StateAqNotMerged();
+    if (!poll.stateMerged()) {
+      revert StateNotMerged();
     }
 
     // Retrieve stored vals

contracts/contracts/MessageProcessorFactory.sol

@@ -18,8 +18,7 @@ contract MessageProcessorFactory is Params, DomainObjs, IMessageProcessorFactory
     Mode _mode
   ) public returns (address messageProcessorAddr) {
     // deploy MessageProcessor for this Poll
-    MessageProcessor messageProcessor = new MessageProcessor(_verifier, _vkRegistry, _poll, _mode);
-    messageProcessor.transferOwnership(_owner);
+    MessageProcessor messageProcessor = new MessageProcessor(_verifier, _vkRegistry, _poll, _owner, _mode);
     messageProcessorAddr = address(messageProcessor);
   }
 }

contracts/contracts/Poll.sol

@@ -1,11 +1,11 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.20;
 
-import { Params } from "./utilities/Params.sol";
-import { SnarkCommon } from "./crypto/SnarkCommon.sol";
-import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
 import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
 import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
+
+import { Params } from "./utilities/Params.sol";
+import { SnarkCommon } from "./crypto/SnarkCommon.sol";
 import { EmptyBallotRoots } from "./trees/EmptyBallotRoots.sol";
 import { IPoll } from "./interfaces/IPoll.sol";
 import { Utilities } from "./utilities/Utilities.sol";
@@ -16,7 +16,7 @@ import { CurveBabyJubJub } from "./crypto/BabyJubJub.sol";
 /// which can be either votes, key change messages or topup messages.
 /// @dev Do not deploy this directly. Use PollFactory.deploy() which performs some
 /// checks on the Poll constructor arguments.
-contract Poll is Params, Utilities, SnarkCommon, Ownable(msg.sender), EmptyBallotRoots, IPoll {
+contract Poll is Params, Utilities, SnarkCommon, EmptyBallotRoots, IPoll {
   using SafeERC20 for ERC20;
 
   /// @notice Whether the Poll has been initialized
@@ -38,7 +38,7 @@ contract Poll is Params, Utilities, SnarkCommon, Ownable(msg.sender), EmptyBallo
   uint256 internal immutable duration;
 
   /// @notice Whether the MACI contract's stateAq has been merged by this contract
-  bool public stateAqMerged;
+  bool public stateMerged;
 
   /// @notice Get the commitment to the state leaves and the ballots. This is
   /// hash3(stateRoot, ballotRoot, salt).
@@ -80,8 +80,7 @@ contract Poll is Params, Utilities, SnarkCommon, Ownable(msg.sender), EmptyBallo
 
   event PublishMessage(Message _message, PubKey _encPubKey);
   event TopupMessage(Message _message);
-  event MergeMaciStateAqSubRoots(uint256 indexed _numSrQueueOps);
-  event MergeMaciStateAq(uint256 indexed _stateRoot, uint256 indexed _numSignups);
+  event MergeMaciState(uint256 indexed _stateRoot, uint256 indexed _numSignups);
   event MergeMessageAqSubRoots(uint256 indexed _numSrQueueOps);
   event MergeMessageAq(uint256 indexed _messageRoot);
 
@@ -229,13 +228,13 @@ contract Poll is Params, Utilities, SnarkCommon, Ownable(msg.sender), EmptyBallo
   }
 
   /// @inheritdoc IPoll
-  function mergeMaciStateAq() public onlyOwner isAfterVotingDeadline {
+  function mergeMaciState() public isAfterVotingDeadline {
     // This function can only be called once per Poll after the voting
     // deadline
-    if (stateAqMerged) revert StateAqAlreadyMerged();
+    if (stateMerged) revert StateAqAlreadyMerged();
 
     // set merged to true so it cannot be called again
-    stateAqMerged = true;
+    stateMerged = true;
 
     mergedStateRoot = extContracts.maci.getStateTreeRoot();
 
@@ -259,17 +258,17 @@ contract Poll is Params, Utilities, SnarkCommon, Ownable(msg.sender), EmptyBallo
 
     actualStateTreeDepth = depth;
 
-    emit MergeMaciStateAq(mergedStateRoot, numSignups);
+    emit MergeMaciState(mergedStateRoot, numSignups);
   }
 
   /// @inheritdoc IPoll
-  function mergeMessageAqSubRoots(uint256 _numSrQueueOps) public onlyOwner isAfterVotingDeadline {
+  function mergeMessageAqSubRoots(uint256 _numSrQueueOps) public isAfterVotingDeadline {
     extContracts.messageAq.mergeSubRoots(_numSrQueueOps);
     emit MergeMessageAqSubRoots(_numSrQueueOps);
   }
 
   /// @inheritdoc IPoll
-  function mergeMessageAq() public onlyOwner isAfterVotingDeadline {
+  function mergeMessageAq() public isAfterVotingDeadline {
     uint256 root = extContracts.messageAq.merge(treeDepths.messageTreeDepth);
     emit MergeMessageAq(root);
   }

contracts/contracts/PollFactory.sol

@@ -31,8 +31,7 @@ contract PollFactory is Params, DomainObjs, IPollFactory {
     TreeDepths calldata _treeDepths,
     PubKey calldata _coordinatorPubKey,
     address _maci,
-    TopupCredit _topupCredit,
-    address _pollOwner
+    TopupCredit _topupCredit
   ) public virtual returns (address pollAddr) {
     /// @notice Validate _maxValues
     /// maxVoteOptions must be less than 2 ** 50 due to circuit limitations;
@@ -62,8 +61,6 @@ contract PollFactory is Params, DomainObjs, IPollFactory {
     // init Poll
     poll.init();
 
-    poll.transferOwnership(_pollOwner);
-
     pollAddr = address(poll);
   }
 }

contracts/contracts/Tally.sol

@@ -15,7 +15,7 @@ import { DomainObjs } from "./utilities/DomainObjs.sol";
 /// @title Tally
 /// @notice The Tally contract is used during votes tallying
 /// and by users to verify the tally results.
-contract Tally is Ownable(msg.sender), SnarkCommon, CommonUtilities, Hasher, DomainObjs {
+contract Tally is Ownable, SnarkCommon, CommonUtilities, Hasher, DomainObjs {
   uint256 internal constant TREE_ARITY = 2;
   uint256 internal constant VOTE_OPTION_TREE_ARITY = 5;
 
@@ -65,7 +65,16 @@ contract Tally is Ownable(msg.sender), SnarkCommon, CommonUtilities, Hasher, Dom
   /// @param _vkRegistry The VkRegistry contract
   /// @param _poll The Poll contract
   /// @param _mp The MessageProcessor contract
-  constructor(address _verifier, address _vkRegistry, address _poll, address _mp, Mode _mode) payable {
+  /// @param _tallyOwner The owner of the Tally contract
+  /// @param _mode The mode of the poll
+  constructor(
+    address _verifier,
+    address _vkRegistry,
+    address _poll,
+    address _mp,
+    address _tallyOwner,
+    Mode _mode
+  ) payable Ownable(_tallyOwner) {
     verifier = IVerifier(_verifier);
     vkRegistry = IVkRegistry(_vkRegistry);
     poll = IPoll(_poll);

contracts/contracts/TallyFactory.sol

@@ -18,8 +18,7 @@ contract TallyFactory is ITallyFactory, DomainObjs {
     Mode _mode
   ) public virtual returns (address tallyAddr) {
     // deploy Tally for this Poll
-    Tally tally = new Tally(_verifier, _vkRegistry, _poll, _messageProcessor, _mode);
-    tally.transferOwnership(_owner);
+    Tally tally = new Tally(_verifier, _vkRegistry, _poll, _messageProcessor, _owner, _mode);
     tallyAddr = address(tally);
   }
 }

contracts/contracts/interfaces/IPoll.sol

@@ -30,7 +30,7 @@ interface IPoll {
   /// @notice The second step of merging the MACI state AccQueue. This allows the
   /// ProcessMessages circuit to access the latest state tree and ballots via
   /// currentSbCommitment.
-  function mergeMaciStateAq() external;
+  function mergeMaciState() external;
 
   /// @notice The first step in merging the message AccQueue so that the
   /// ProcessMessages circuit can access the message root.
@@ -48,7 +48,7 @@ interface IPoll {
 
   /// @notice Get the result of whether the MACI contract's stateAq has been merged by this contract
   /// @return Whether the MACI contract's stateAq has been merged by this contract
-  function stateAqMerged() external view returns (bool);
+  function stateMerged() external view returns (bool);
 
   /// @notice Get the depths of the merkle trees
   /// @return intStateTreeDepth The depth of the state tree

contracts/contracts/interfaces/IPollFactory.sol

@@ -15,15 +15,13 @@ interface IPollFactory {
   /// @param _coordinatorPubKey The coordinator's public key
   /// @param _maci The MACI contract interface reference
   /// @param _topupCredit The TopupCredit contract
-  /// @param _pollOwner The owner of the poll
   /// @return The deployed Poll contract
   function deploy(
     uint256 _duration,
     Params.MaxValues memory _maxValues,
     Params.TreeDepths memory _treeDepths,
     DomainObjs.PubKey memory _coordinatorPubKey,
     address _maci,
-    TopupCredit _topupCredit,
-    address _pollOwner
+    TopupCredit _topupCredit
   ) external returns (address);
 }

contracts/tasks/helpers/ProofGenerator.ts

@@ -113,7 +113,7 @@ export class ProofGenerator {
         .queryFilter(pollContract.filters.MergeMessageAq(messageRoot), fromBlock)
         .then((events) => events[events.length - 1]?.blockNumber),
       pollContract
-        .queryFilter(pollContract.filters.MergeMaciStateAq(stateRoot, numSignups), fromBlock)
+        .queryFilter(pollContract.filters.MergeMaciState(stateRoot, numSignups), fromBlock)
         .then((events) => events[events.length - 1]?.blockNumber),
     ]).then((blocks) => Math.max(...blocks));