Merge pull request #755 from clrfund/fix/incorrect-max-contributor

Fix incorrect max contributor causing frontend to prematurely closing a round

common/src/utils.ts

@@ -12,7 +12,9 @@ import { Keypair } from './keypair'
 import { Tally } from './tally'
 import { bnSqrt } from './math'
 
-const LEAVES_PER_NODE = 5
+// This has to match the MACI TREE_ARITY at:
+// github.com/privacy-scaling-explorations/maci/blob/0c18913d4c84bfa9fbfd66dc017e338df9fdda96/contracts/contracts/MACI.sol#L31
+export const MACI_TREE_ARITY = 5
 
 export function createMessage(
   userStateIndex: number,
@@ -65,7 +67,7 @@ export function getRecipientClaimData(
   const spentTree = new IncrementalQuinTree(
     recipientTreeDepth,
     BigInt(0),
-    LEAVES_PER_NODE,
+    MACI_TREE_ARITY,
     hash5
   )
   for (const leaf of tally.perVOSpentVoiceCredits.tally) {
@@ -94,6 +96,15 @@ export function getRecipientClaimData(
   ]
 }
 
+/**
+ * Returns the maximum MACI users allowed by the state tree
+ * @param stateTreeDepth MACI state tree depth
+ * @returns the maximum number of contributors allowed by MACI circuit
+ */
+export function getMaxContributors(stateTreeDepth: number): number {
+  return MACI_TREE_ARITY ** stateTreeDepth - 1
+}
+
 export {
   genTallyResultCommitment,
   Message,
@@ -103,5 +114,4 @@ export {
   hash2,
   hash3,
   hashLeftRight,
-  LEAVES_PER_NODE,
 }

contracts/contracts/ClrFund.sol

@@ -60,8 +60,14 @@ contract ClrFund is OwnableUpgradeable, DomainObjs, Params {
   error InvalidFundingRoundFactory();
   error InvalidMaciFactory();
   error RecipientRegistryNotSet();
+  error MaxRecipientsNotSet();
   error NotInitialized();
-  error VoteOptionTreeDepthNotSet();
+
+  modifier maciFactoryInitialized() {
+    if (address(maciFactory) == address(0)) revert InvalidMaciFactory();
+    if (maciFactory.maxRecipients() == 0) revert MaxRecipientsNotSet();
+    _;
+  }
 
 
   /**
@@ -128,20 +134,6 @@ contract ClrFund is OwnableUpgradeable, DomainObjs, Params {
     _setFundingRoundFactory(_roundFactory);
   }
 
-  /**
-   * @dev Get the maximum recipients allowed in the recipient registry
-   */
-  function getMaxRecipients() public view returns (uint256 _maxRecipients) {
-    TreeDepths memory treeDepths = maciFactory.treeDepths();
-    if (treeDepths.voteOptionTreeDepth == 0) revert VoteOptionTreeDepthNotSet();
-
-    uint256 maxVoteOption = maciFactory.TREE_ARITY() ** treeDepths.voteOptionTreeDepth;
-
-    // -1 because the first slot of the recipients array is not used
-    // and maxRecipients is used to generate 0 based index to the array
-    _maxRecipients = maxVoteOption - 1;
-  }
-
   /**
     * @dev Set registry of verified users.
     * @param _userRegistry Address of a user registry.
@@ -162,10 +154,13 @@ contract ClrFund is OwnableUpgradeable, DomainObjs, Params {
   function setRecipientRegistry(IRecipientRegistry _recipientRegistry)
     external
     onlyOwner
+    maciFactoryInitialized
   {
+
     recipientRegistry = _recipientRegistry;
-    uint256 maxRecipients = getMaxRecipients();
-    recipientRegistry.setMaxRecipients(maxRecipients);
+
+    // Make sure that the max number of recipients is set correctly
+    recipientRegistry.setMaxRecipients(maciFactory.maxRecipients());
 
     emit RecipientRegistryChanged(address(_recipientRegistry));
   }
@@ -220,6 +215,7 @@ contract ClrFund is OwnableUpgradeable, DomainObjs, Params {
   )
     external
     onlyOwner
+    maciFactoryInitialized
   {
     IFundingRound currentRound = getCurrentRound();
     if (address(currentRound) != address(0) && !currentRound.isFinalized()) {
@@ -229,8 +225,7 @@ contract ClrFund is OwnableUpgradeable, DomainObjs, Params {
     if (address(recipientRegistry) == address(0)) revert RecipientRegistryNotSet();
 
     // Make sure that the max number of recipients is set correctly
-    uint256 maxRecipients = getMaxRecipients();
-    recipientRegistry.setMaxRecipients(maxRecipients);
+    recipientRegistry.setMaxRecipients(maciFactory.maxRecipients());
 
     // Deploy funding round and MACI contracts
     address newRound = roundFactory.deploy(duration, address(this));

contracts/contracts/FundingRound.sol

@@ -66,6 +66,7 @@ contract FundingRound is
   error TallyHashNotPublished();
   error IncompleteTallyResults(uint256 total, uint256 actual);
   error NoVotes();
+  error NoSignUps();
   error MaciNotSet();
   error PollNotSet();
   error InvalidMaci();
@@ -175,19 +176,6 @@ contract FundingRound is
     return (addressValue == address(0));
   }
 
-  /**
-   * @dev Have the votes been tallied
-   */
-  function isTallied() private view returns (bool) {
-    (uint256 numSignUps, ) = poll.numSignUpsAndMessages();
-    (uint8 intStateTreeDepth, , , ) = poll.treeDepths();
-    uint256 tallyBatchSize = TREE_ARITY ** uint256(intStateTreeDepth);
-    uint256 tallyBatchNum = tally.tallyBatchNum();
-    uint256 totalTallied = tallyBatchNum * tallyBatchSize;
-
-    return numSignUps > 0 && totalTallied >= numSignUps;
-  }
-
   /**
   * @dev Set the tally contract
   * @param _tally The tally contract address
@@ -470,18 +458,18 @@ contract FundingRound is
 
     _votingPeriodOver(poll);
 
-    if (!isTallied()) {
+    if (!tally.isTallied()) {
       revert VotesNotTallied();
     }
+
     if (bytes(tallyHash).length == 0) {
       revert TallyHashNotPublished();
     }
 
     // make sure we have received all the tally results
-    (,,, uint8 voteOptionTreeDepth) = poll.treeDepths();
-    uint256 totalResults = uint256(TREE_ARITY) ** uint256(voteOptionTreeDepth);
-    if ( totalTallyResults != totalResults ) {
-      revert IncompleteTallyResults(totalResults, totalTallyResults);
+    (, uint256 maxVoteOptions) = poll.maxValues();
+    if (totalTallyResults != maxVoteOptions) {
+      revert IncompleteTallyResults(maxVoteOptions, totalTallyResults);
     }
 
     // If nobody voted, the round should be cancelled to avoid locking of matching funds
@@ -494,7 +482,6 @@ contract FundingRound is
       revert IncorrectSpentVoiceCredits();
     }
 
-
     totalSpent = _totalSpent;
     // Total amount of spent voice credits is the size of the pool of direct rewards.
     // Everything else, including unspent voice credits and downscaling error,
@@ -675,9 +662,15 @@ contract FundingRound is
   {
     if (isAddressZero(address(maci))) revert MaciNotSet();
 
-    if (!isTallied()) {
+    if (maci.numSignUps() == 0) {
+      // no sign ups, so no tally results
+      revert NoSignUps();
+    }
+
+    if (!tally.isTallied()) {
       revert VotesNotTallied();
     }
+
     if (isFinalized) {
       revert RoundAlreadyFinalized();
     }

contracts/contracts/MACICommon.sol

@@ -6,9 +6,6 @@ pragma solidity 0.8.20;
  * @dev a contract that holds common MACI structures
  */
 contract MACICommon {
-  // MACI tree arity
-  uint256 public constant TREE_ARITY = 5;
-
   /**
    * @dev These are contract factories used to deploy MACI poll processing contracts
    * when creating a new ClrFund funding round.

contracts/contracts/MACIFactory.sol

@@ -29,6 +29,8 @@ contract MACIFactory is Ownable(msg.sender), Params, SnarkCommon, DomainObjs, MA
   // circuit parameters
   uint8 public stateTreeDepth;
   TreeDepths public treeDepths;
+  uint256 public messageBatchSize;
+  uint256 public maxRecipients;
 
   // Events
   event MaciParametersChanged();
@@ -38,11 +40,15 @@ contract MACIFactory is Ownable(msg.sender), Params, SnarkCommon, DomainObjs, MA
   error NotInitialized();
   error ProcessVkNotSet();
   error TallyVkNotSet();
+  error VoteOptionTreeDepthNotSet();
   error InvalidVkRegistry();
   error InvalidPollFactory();
   error InvalidTallyFactory();
   error InvalidMessageProcessorFactory();
   error InvalidVerifier();
+  error InvalidMaxRecipients();
+  error InvalidMessageBatchSize();
+  error InvalidVoteOptionTreeDepth();
 
   constructor(
     address _vkRegistry,
@@ -60,14 +66,6 @@ contract MACIFactory is Ownable(msg.sender), Params, SnarkCommon, DomainObjs, MA
     verifier = Verifier(_verifier);
   }
 
-  /**
-   * @dev calculate the message batch size
-   */
-  function getMessageBatchSize(uint8 messageTreeSubDepth) public pure
-    returns(uint256 _messageBatchSize) {
-      _messageBatchSize = TREE_ARITY ** messageTreeSubDepth;
-  }
-
   /**
    * @dev set vk registry
    */
@@ -122,13 +120,19 @@ contract MACIFactory is Ownable(msg.sender), Params, SnarkCommon, DomainObjs, MA
    */
   function setMaciParameters(
     uint8 _stateTreeDepth,
+    uint256 _messageBatchSize,
+    uint256 _maxRecipients,
     TreeDepths calldata _treeDepths
   )
     public
     onlyOwner
   {
+    if (_treeDepths.voteOptionTreeDepth == 0) revert InvalidVoteOptionTreeDepth();
+    if (_maxRecipients == 0) revert InvalidMaxRecipients();
+    if (_messageBatchSize == 0) revert InvalidMessageBatchSize();
 
-    uint256 messageBatchSize = getMessageBatchSize(_treeDepths.messageTreeSubDepth);
+    messageBatchSize = _messageBatchSize;
+    maxRecipients = _maxRecipients;
 
     if (!vkRegistry.hasProcessVk(
       _stateTreeDepth,
@@ -155,6 +159,7 @@ contract MACIFactory is Ownable(msg.sender), Params, SnarkCommon, DomainObjs, MA
     emit MaciParametersChanged();
   }
 
+
   /**
     * @dev Deploy new MACI instance.
     */
@@ -170,8 +175,6 @@ contract MACIFactory is Ownable(msg.sender), Params, SnarkCommon, DomainObjs, MA
     external
     returns (MACI _maci, MACI.PollContracts memory _pollContracts)
   {
-    uint256 messageBatchSize = getMessageBatchSize(treeDepths.messageTreeSubDepth);
-
     if (!vkRegistry.hasProcessVk(
       stateTreeDepth,
       treeDepths.messageTreeDepth,

contracts/contracts/interfaces/IMACIFactory.sol

@@ -28,10 +28,7 @@ interface IMACIFactory {
   function stateTreeDepth() external view returns (uint8);
   function treeDepths() external view returns (Params.TreeDepths memory);
 
-  function getMessageBatchSize(uint8 _messageTreeSubDepth) external pure
-    returns(uint256 _messageBatchSize);
-
-  function TREE_ARITY() external pure returns (uint256);
+  function maxRecipients() external view returns (uint256);
 
   function deployMaci(
     SignUpGatekeeper signUpGatekeeper,

contracts/contracts/recipientRegistry/IRecipientRegistry.sol

@@ -17,6 +17,7 @@ pragma solidity 0.8.20;
  */
 interface IRecipientRegistry {
 
+  function maxRecipients() external returns (uint256);
   function setMaxRecipients(uint256 _maxRecipients) external returns (bool);
 
   function getRecipientAddress(uint256 _index, uint256 _startBlock, uint256 _endBlock) external view returns (address);

contracts/tasks/index.ts

@@ -29,3 +29,4 @@ import './runners/proveOnChain'
 import './runners/publishTallyResults'
 import './runners/resetTally'
 import './runners/maciPubkey'
+import './runners/simulateContribute'

contracts/tasks/runners/exportRound.ts

@@ -227,7 +227,6 @@ async function getRoundInfo(
         await pollContract.getDeployTimeAndDuration()
       startTime = getNumber(roundStartTime)
       signUpDuration = roundDuration
-      votingDuration = roundDuration
       endTime = startTime + getNumber(roundDuration)
 
       pollId = await roundContract.pollId()