refactor(contracts): smart contracts optimizations

Unify usage of TREE_ARITY
Use calldata vs memory for function params which are immutable
Ensure we do not accept max messages + 1 in top up
Use payable for constructors to reduce deployment costs

contracts/contracts/MACI.sol

@@ -18,7 +18,7 @@ import { Ownable } from "@openzeppelin/contracts/access/Ownable.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 {
+contract MACI is IMACI, Params, Utilities, Ownable {
   /// @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
   /// this should also match the parameter of the circom circuits.
@@ -28,8 +28,7 @@ contract MACI is IMACI, DomainObjs, Params, Utilities, Ownable {
   /// in contracts/ts/genEmptyBallotRootsContract.ts file
   /// if we change the state tree depth!
   uint8 internal constant STATE_TREE_SUBDEPTH = 2;
-  uint8 internal constant STATE_TREE_ARITY = 5;
-  uint8 internal constant MESSAGE_TREE_ARITY = 5;
+  uint8 internal constant TREE_ARITY = 5;
 
   /// @notice The hash of a blank state leaf
   uint256 internal constant BLANK_STATE_LEAF_HASH =
@@ -78,6 +77,7 @@ contract MACI is IMACI, DomainObjs, Params, Utilities, Ownable {
     _;
   }
 
+  /// @notice custom errors
   error CallerMustBePoll(address _caller);
   error AlreadyInitialized();
   error PoseidonHashLibrariesNotLinked();
@@ -98,7 +98,7 @@ contract MACI is IMACI, DomainObjs, Params, Utilities, Ownable {
     InitialVoiceCreditProxy _initialVoiceCreditProxy,
     TopupCredit _topupCredit,
     uint8 _stateTreeDepth
-  ) {
+  ) payable {
     // Deploy the state AccQueue
     stateAq = new AccQueueQuinaryBlankSl(STATE_TREE_SUBDEPTH);
     stateAq.enqueue(BLANK_STATE_LEAF_HASH);
@@ -133,14 +133,15 @@ contract MACI is IMACI, DomainObjs, Params, Utilities, Ownable {
     bytes memory _initialVoiceCreditProxyData
   ) public {
     // ensure we do not have more signups than what the circuits support
-    if (numSignUps == uint256(STATE_TREE_ARITY) ** uint256(stateTreeDepth)) revert TooManySignups();
+    if (numSignUps == uint256(TREE_ARITY) ** uint256(stateTreeDepth)) revert TooManySignups();
 
     if (_pubKey.x >= SNARK_SCALAR_FIELD || _pubKey.y >= SNARK_SCALAR_FIELD) {
       revert MaciPubKeyLargerThanSnarkFieldSize();
     }
 
     // Increment the number of signups
-    // cannot overflow as numSignUps < 5 ** 10 -1
+    // cannot overflow with realistic stateTreeDepth
+    // values as numSignUps < 5 ** stateTreeDepth -1
     unchecked {
       numSignUps++;
     }
@@ -172,6 +173,7 @@ contract MACI is IMACI, DomainObjs, Params, Utilities, Ownable {
     TreeDepths memory _treeDepths,
     PubKey memory _coordinatorPubKey
   ) public onlyOwner returns (address pollAddr) {
+    // cache the poll to a local variable so we can increment it
     uint256 pollId = nextPollId;
 
     // Increment the poll ID for the next poll
@@ -186,9 +188,9 @@ contract MACI is IMACI, DomainObjs, Params, Utilities, Ownable {
 
     // The message batch size and the tally batch size
     BatchSizes memory batchSizes = BatchSizes(
-      uint24(MESSAGE_TREE_ARITY) ** _treeDepths.messageTreeSubDepth,
-      uint24(STATE_TREE_ARITY) ** _treeDepths.intStateTreeDepth,
-      uint24(STATE_TREE_ARITY) ** _treeDepths.intStateTreeDepth
+      uint24(TREE_ARITY) ** _treeDepths.messageTreeSubDepth,
+      uint24(TREE_ARITY) ** _treeDepths.intStateTreeDepth,
+      uint24(TREE_ARITY) ** _treeDepths.intStateTreeDepth
     );
 
     Poll p = pollFactory.deploy(

contracts/contracts/MessageProcessor.sol

@@ -44,7 +44,7 @@ contract MessageProcessor is Ownable, SnarkCommon, CommonUtilities, Hasher {
   /// @notice Create a new instance
   /// @param _verifier The Verifier contract address
   /// @param _vkRegistry The VkRegistry contract address
-  constructor(Verifier _verifier, VkRegistry _vkRegistry) {
+  constructor(Verifier _verifier, VkRegistry _vkRegistry) payable {
     verifier = _verifier;
     vkRegistry = _vkRegistry;
   }

contracts/contracts/Poll.sol

@@ -49,20 +49,11 @@ contract Poll is Params, Utilities, SnarkCommon, Ownable, EmptyBallotRoots {
 
   uint256 internal numMessages;
 
-  /// @notice The number of messages which have been processed and the number of
-  /// signups
-  /// @return numSignups The number of signups
-  /// @return numMsgs The number of messages sent by voters
-  function numSignUpsAndMessages() public view returns (uint256 numSignups, uint256 numMsgs) {
-    numSignups = extContracts.maci.numSignUps();
-    numMsgs = numMessages;
-  }
-
   MaxValues public maxValues;
   TreeDepths public treeDepths;
   BatchSizes public batchSizes;
 
-  // errors
+  /// @notice custom errors
   error VotingPeriodOver();
   error VotingPeriodNotOver();
   error PollAlreadyInit();
@@ -95,7 +86,7 @@ contract Poll is Params, Utilities, SnarkCommon, Ownable, EmptyBallotRoots {
     BatchSizes memory _batchSizes,
     PubKey memory _coordinatorPubKey,
     ExtContracts memory _extContracts
-  ) {
+  ) payable {
     extContracts = _extContracts;
 
     coordinatorPubKey = _coordinatorPubKey;
@@ -151,7 +142,8 @@ contract Poll is Params, Utilities, SnarkCommon, Ownable, EmptyBallotRoots {
   /// @param stateIndex The index of user in the state queue
   /// @param amount The amount of credits to topup
   function topup(uint256 stateIndex, uint256 amount) public isWithinVotingDeadline {
-    if (numMessages > maxValues.maxMessages) revert TooManyMessages();
+    // we check that we do not exceed the max number of messages
+    if (numMessages == maxValues.maxMessages) revert TooManyMessages();
 
     unchecked {
       numMessages++;
@@ -173,9 +165,11 @@ contract Poll is Params, Utilities, SnarkCommon, Ownable, EmptyBallotRoots {
   /// @param _encPubKey An epheremal public key which can be combined with the
   /// coordinator's private key to generate an ECDH shared key with which
   /// to encrypt the message.
-  function publishMessage(Message memory _message, PubKey memory _encPubKey) public isWithinVotingDeadline {
+  function publishMessage(Message memory _message, PubKey calldata _encPubKey) public isWithinVotingDeadline {
+    // we check that we do not exceed the max number of messages
     if (numMessages == maxValues.maxMessages) revert TooManyMessages();
 
+    // validate that the public key is valid
     if (_encPubKey.x >= SNARK_SCALAR_FIELD || _encPubKey.y >= SNARK_SCALAR_FIELD) {
       revert MaciPubKeyLargerThanSnarkFieldSize();
     }
@@ -184,6 +178,7 @@ contract Poll is Params, Utilities, SnarkCommon, Ownable, EmptyBallotRoots {
       numMessages++;
     }
 
+    // force the message to have type 1
     _message.msgType = 1;
     uint256 messageLeaf = hashMessageAndEncPubKey(_message, _encPubKey);
     extContracts.messageAq.enqueue(messageLeaf);
@@ -252,4 +247,13 @@ contract Poll is Params, Utilities, SnarkCommon, Ownable, EmptyBallotRoots {
     _deployTime = deployTime;
     _duration = duration;
   }
+
+  /// @notice The number of messages which have been processed and the number of
+  /// signups
+  /// @return numSignups The number of signups
+  /// @return numMsgs The number of messages sent by voters
+  function numSignUpsAndMessages() public view returns (uint256 numSignups, uint256 numMsgs) {
+    numSignups = extContracts.maci.numSignUps();
+    numMsgs = numMessages;
+  }
 }

contracts/contracts/PollFactory.sol

@@ -14,10 +14,14 @@ import { Poll } from "./Poll.sol";
 /// size to stay within the limit set by EIP-170.
 contract PollFactory is Params, DomainObjs {
   // The number of children each node in the message tree has
-  uint256 public constant TREE_ARITY = 5;
+  uint256 internal constant TREE_ARITY = 5;
+
   // custom error
   error InvalidMaxValues();
 
+  /// @notice The PollFactory constructor
+  constructor() payable {}
+
   /// @notice Deploy a new Poll contract and AccQueue contract for messages.
   /// @param _duration The duration of the poll
   /// @param _maxValues The max values for the poll
@@ -30,10 +34,10 @@ contract PollFactory is Params, DomainObjs {
   /// @return poll The deployed Poll contract
   function deploy(
     uint256 _duration,
-    MaxValues memory _maxValues,
-    TreeDepths memory _treeDepths,
-    BatchSizes memory _batchSizes,
-    PubKey memory _coordinatorPubKey,
+    MaxValues calldata _maxValues,
+    TreeDepths calldata _treeDepths,
+    BatchSizes calldata _batchSizes,
+    PubKey calldata _coordinatorPubKey,
     IMACI _maci,
     TopupCredit _topupCredit,
     address _pollOwner

contracts/contracts/SignUpToken.sol

@@ -9,7 +9,7 @@ import "@openzeppelin/contracts/access/Ownable.sol";
 /// can be used to allow users to sign up for a poll.
 contract SignUpToken is ERC721, Ownable {
   /// @notice The constructor which calls the ERC721 constructor
-  constructor() ERC721("SignUpToken", "SignUpToken") {}
+  constructor() payable ERC721("SignUpToken", "SignUpToken") {}
 
   /// @notice Gives an ERC721 token to an address
   /// @param to The address to give the token to

contracts/contracts/Subsidy.sol

@@ -16,13 +16,16 @@ import { VkRegistry } from "./VkRegistry.sol";
 /// are correct. It is also used to update the subsidy commitment if the
 /// proof is valid.
 contract Subsidy is Ownable, CommonUtilities, Hasher, SnarkCommon {
-  uint256 public rbi; // row batch index
-  uint256 public cbi; // column batch index
+  // row batch index
+  uint256 public rbi;
+  // column batch index
+  uint256 public cbi;
+
   // The final commitment to the state and ballot roots
   uint256 public sbCommitment;
   uint256 public subsidyCommitment;
 
-  uint8 public constant treeArity = 5;
+  uint8 internal constant TREE_ARITY = 5;
 
   // Error codes
   error ProcessingNotComplete();
@@ -39,7 +42,7 @@ contract Subsidy is Ownable, CommonUtilities, Hasher, SnarkCommon {
   /// @notice Create a new Subsidy contract
   /// @param _verifier The Verifier contract
   /// @param _vkRegistry The VkRegistry contract
-  constructor(Verifier _verifier, VkRegistry _vkRegistry) {
+  constructor(Verifier _verifier, VkRegistry _vkRegistry) payable {
     verifier = _verifier;
     vkRegistry = _vkRegistry;
   }
@@ -93,14 +96,14 @@ contract Subsidy is Ownable, CommonUtilities, Hasher, SnarkCommon {
     Poll _poll,
     MessageProcessor _mp,
     uint256 _newSubsidyCommitment,
-    uint256[8] memory _proof
+    uint256[8] calldata _proof
   ) external onlyOwner {
     _votingPeriodOver(_poll);
     updateSbCommitment(_mp);
 
     (uint8 intStateTreeDepth, , , ) = _poll.treeDepths();
 
-    uint256 subsidyBatchSize = uint256(treeArity) ** intStateTreeDepth;
+    uint256 subsidyBatchSize = uint256(TREE_ARITY) ** intStateTreeDepth;
 
     (uint256 numSignUps, ) = _poll.numSignUpsAndMessages();
     uint256 numLeaves = numSignUps + 1;
@@ -141,7 +144,7 @@ contract Subsidy is Ownable, CommonUtilities, Hasher, SnarkCommon {
   /// @return isValid True if the proof is valid
   function verifySubsidyProof(
     Poll _poll,
-    uint256[8] memory _proof,
+    uint256[8] calldata _proof,
     uint256 _numSignUps,
     uint256 _newSubsidyCommitment
   ) public view returns (bool isValid) {

contracts/contracts/Tally.sol

@@ -24,7 +24,7 @@ contract Tally is Ownable, SnarkCommon, CommonUtilities, Hasher {
   error BatchStartIndexTooLarge();
   error TallyBatchSizeTooLarge();
 
-  uint8 private constant LEAVES_PER_NODE = 5;
+  uint8 private constant TREE_ARITY = 5;
 
   /// @notice The commitment to the tally results. Its initial value is 0, but after
   /// the tally of each batch is proven on-chain via a zk-SNARK, it should be
@@ -51,7 +51,7 @@ contract Tally is Ownable, SnarkCommon, CommonUtilities, Hasher {
   /// @notice Create a new Tally contract
   /// @param _verifier The Verifier contract
   /// @param _vkRegistry The VkRegistry contract
-  constructor(Verifier _verifier, VkRegistry _vkRegistry) {
+  constructor(Verifier _verifier, VkRegistry _vkRegistry) payable {
     verifier = _verifier;
     vkRegistry = _vkRegistry;
   }
@@ -115,7 +115,7 @@ contract Tally is Ownable, SnarkCommon, CommonUtilities, Hasher {
     Poll _poll,
     MessageProcessor _mp,
     uint256 _newTallyCommitment,
-    uint256[8] memory _proof
+    uint256[8] calldata _proof
   ) public onlyOwner {
     _votingPeriodOver(_poll);
     updateSbCommitment(_mp);
@@ -149,7 +149,7 @@ contract Tally is Ownable, SnarkCommon, CommonUtilities, Hasher {
   /// @return isValid whether the proof is valid
   function verifyTallyProof(
     Poll _poll,
-    uint256[8] memory _proof,
+    uint256[8] calldata _proof,
     uint256 _numSignUps,
     uint256 _batchStartIndex,
     uint256 _tallyBatchSize,
@@ -185,16 +185,16 @@ contract Tally is Ownable, SnarkCommon, CommonUtilities, Hasher {
     uint8 _depth,
     uint256 _index,
     uint256 _leaf,
-    uint256[][] memory _pathElements
+    uint256[][] calldata _pathElements
   ) internal pure returns (uint256 current) {
-    uint256 pos = _index % LEAVES_PER_NODE;
+    uint256 pos = _index % TREE_ARITY;
     current = _leaf;
     uint8 k;
 
-    uint256[LEAVES_PER_NODE] memory level;
+    uint256[TREE_ARITY] memory level;
 
     for (uint8 i = 0; i < _depth; ++i) {
-      for (uint8 j = 0; j < LEAVES_PER_NODE; ++j) {
+      for (uint8 j = 0; j < TREE_ARITY; ++j) {
         if (j == pos) {
           level[j] = current;
         } else {
@@ -207,8 +207,8 @@ contract Tally is Ownable, SnarkCommon, CommonUtilities, Hasher {
         }
       }
 
-      _index /= LEAVES_PER_NODE;
-      pos = _index % LEAVES_PER_NODE;
+      _index /= TREE_ARITY;
+      pos = _index % TREE_ARITY;
       current = hash5(level);
     }
   }
@@ -246,7 +246,7 @@ contract Tally is Ownable, SnarkCommon, CommonUtilities, Hasher {
   function verifyPerVOSpentVoiceCredits(
     uint256 _voteOptionIndex,
     uint256 _spent,
-    uint256[][] memory _spentProof,
+    uint256[][] calldata _spentProof,
     uint256 _spentSalt,
     uint8 _voteOptionTreeDepth,
     uint256 _spentVoiceCreditsHash,
@@ -275,7 +275,7 @@ contract Tally is Ownable, SnarkCommon, CommonUtilities, Hasher {
   function verifyTallyResult(
     uint256 _voteOptionIndex,
     uint256 _tallyResult,
-    uint256[][] memory _tallyResultProof,
+    uint256[][] calldata _tallyResultProof,
     uint256 _tallyResultSalt,
     uint8 _voteOptionTreeDepth,
     uint256 _spentVoiceCreditsHash,

contracts/contracts/TopupCredit.sol

@@ -12,7 +12,7 @@ contract TopupCredit is ERC20, Ownable {
   uint256 public constant MAXIMUM_AIRDROP_AMOUNT = 100000 * 10 ** _decimals;
 
   /// @notice create  a new TopupCredit token
-  constructor() ERC20("TopupCredit", "TopupCredit") {}
+  constructor() payable ERC20("TopupCredit", "TopupCredit") {}
 
   /// @notice mint tokens to an account
   /// @param account the account to mint tokens to

contracts/contracts/VkRegistry.sol

@@ -30,6 +30,9 @@ contract VkRegistry is Ownable, SnarkCommon {
   error TallyVkNotSet();
   error SubsidyVkNotSet();
 
+  /// @notice Create a new instance of the VkRegistry contract
+  constructor() payable {}
+
   /// @notice Check if the process verifying key is set
   /// @param _sig The signature
   /// @return isSet whether the verifying key is set
@@ -106,8 +109,8 @@ contract VkRegistry is Ownable, SnarkCommon {
     uint256 _messageTreeDepth,
     uint256 _voteOptionTreeDepth,
     uint256 _messageBatchSize,
-    VerifyingKey memory _processVk,
-    VerifyingKey memory _tallyVk
+    VerifyingKey calldata _processVk,
+    VerifyingKey calldata _tallyVk
   ) public onlyOwner {
     uint256 processVkSig = genProcessVkSig(_stateTreeDepth, _messageTreeDepth, _voteOptionTreeDepth, _messageBatchSize);
 
@@ -152,7 +155,7 @@ contract VkRegistry is Ownable, SnarkCommon {
     uint256 _stateTreeDepth,
     uint256 _intStateTreeDepth,
     uint256 _voteOptionTreeDepth,
-    VerifyingKey memory _subsidyVk
+    VerifyingKey calldata _subsidyVk
   ) public onlyOwner {
     uint256 subsidyVkSig = genSubsidyVkSig(_stateTreeDepth, _intStateTreeDepth, _voteOptionTreeDepth);
 

contracts/contracts/gatekeepers/FreeForAllSignUpGatekeeper.sol

@@ -7,6 +7,9 @@ import { MACI } from "../MACI.sol";
 /// @title FreeForAllGatekeeper
 /// @notice A SignUpGatekeeper which allows anyone to sign up.
 contract FreeForAllGatekeeper is SignUpGatekeeper {
+  /// @notice Create a new instance of FreeForAllGatekeeper
+  constructor() payable {}
+
   /// @notice setMaciInstance does nothing in this gatekeeper
   /// @param _maci The MACI contract
   function setMaciInstance(MACI _maci) public override {}

contracts/contracts/gatekeepers/SignUpTokenGatekeeper.sol

@@ -27,7 +27,7 @@ contract SignUpTokenGatekeeper is SignUpGatekeeper, Ownable {
 
   /// @notice creates a new SignUpTokenGatekeeper
   /// @param _token the address of the SignUpToken contract
-  constructor(SignUpToken _token) Ownable() {
+  constructor(SignUpToken _token) payable Ownable() {
     token = _token;
   }