Merge pull request #769 from clrfund/merge/fix-blank-leaderboard

Fix blank leaderboard view for canceled rounds

.github/workflows/create-version.yml

@@ -31,18 +31,14 @@ jobs:
         node-version: ${{ env.NODE_VERSION }}
     - name: Checkout source code
       uses: actions/checkout@v3
-    - name: Install dependencies
+    - name: Create new version
       run: |
         # use https to avoid error: unable to connect to github.com
         git config --global url."https://".insteadOf git://
-        yarn && yarn build
-    - name: setup git config
-      run: |
         # setup the username and email. I tend to use 'GitHub Actions Bot' with no email by default
         git config user.name "GitHub Actions Bot"
         git config user.email "<>"
-    - name: Create new version
-      run: |
+        yarn && yarn build
         echo "Version: ${{ github.event.inputs.version }}" 
         cd contracts
         npm version ${{ github.event.inputs.version }}

common/package.json

@@ -22,9 +22,9 @@
   },
   "dependencies": {
     "@openzeppelin/merkle-tree": "^1.0.5",
-    "ethers": "^6.11.1",
-    "maci-crypto": "^1.2.0",
-    "maci-domainobjs": "^1.2.0"
+    "ethers": "^6.12.1",
+    "maci-crypto": "1.2.2",
+    "maci-domainobjs": "1.2.2"
   },
   "repository": {
     "type": "git",

common/src/__tests__/keypair.spec.ts

@@ -0,0 +1,21 @@
+import { expect } from 'chai'
+import { Keypair, PubKey } from '../keypair'
+import { Wallet, sha256, randomBytes } from 'ethers'
+
+describe('keypair', function () {
+  for (let i = 0; i < 10; i++) {
+    it(`should generate key ${i} from seed successfully`, function () {
+      const wallet = Wallet.createRandom()
+      const signature = wallet.signMessageSync(randomBytes(32).toString())
+      const seed = sha256(signature)
+      const keypair = Keypair.createFromSeed(seed)
+      expect(keypair.pubKey.serialize()).to.match(/^macipk./)
+    })
+  }
+
+  it('should throw if pubKey is invalid', () => {
+    expect(() => {
+      new PubKey([1n, 1n])
+    }).to.throw('PubKey not on curve')
+  })
+})

common/src/keypair.ts

@@ -1,37 +1,31 @@
 import { keccak256, isBytesLike, concat, toBeArray } from 'ethers'
 import { Keypair as MaciKeypair, PrivKey, PubKey } from 'maci-domainobjs'
 
-const SNARK_FIELD_SIZE = BigInt(
-  '21888242871839275222246405745257275088548364400416034343698204186575808495617'
-)
-
 /**
- * Returns a BabyJub-compatible value. This function is modified from
- * the MACI's genRandomBabyJubValue(). Instead of returning random value
- * for the private key, it derives the private key from the users
- * signature hash
+ * Derives the MACI private key from the users signature hash
  * @param hash - user's signature hash
+ * @return The MACI private key
  */
 function genPrivKey(hash: string): PrivKey {
-  // Prevent modulo bias
-  //const lim = BigInt('0x10000000000000000000000000000000000000000000000000000000000000000')
-  //const min = (lim - SNARK_FIELD_SIZE) % SNARK_FIELD_SIZE
-  const min = BigInt(
-    '6350874878119819312338956282401532410528162663560392320966563075034087161851'
-  )
-
   if (!isBytesLike(hash)) {
-    throw new Error(`Hash must be a hex string: ${hash}`)
+    throw new Error(`genPrivKey() error. Hash must be a hex string: ${hash}`)
   }
 
-  let hashBN = BigInt(hash)
-  // don't think we'll enter the for loop below, but, just in case
-  for (let counter = 1; hashBN < min; counter++) {
-    const data = concat([toBeArray(hashBN), toBeArray(counter)])
-    hashBN = BigInt(keccak256(data))
+  let rawPrivKey = BigInt(hash)
+  let pubKey: PubKey | null = null
+
+  for (let counter = 1; pubKey === null; counter++) {
+    try {
+      const privKey = new PrivKey(rawPrivKey)
+      // this will throw 'Invalid public key' if key is not on the Baby Jubjub elliptic curve
+      const keypair = new Keypair(privKey)
+      pubKey = keypair.pubKey
+    } catch {
+      const data = concat([toBeArray(rawPrivKey), toBeArray(counter)])
+      rawPrivKey = BigInt(keccak256(data))
+    }
   }
 
-  const rawPrivKey = hashBN % SNARK_FIELD_SIZE
   return new PrivKey(rawPrivKey)
 }
 

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/AnyOldERC20Token.sol

@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-3.0
 
-pragma solidity ^0.8.10;
+pragma solidity 0.8.20;
 
 import '@openzeppelin/contracts/token/ERC20/ERC20.sol';
 

contracts/contracts/CloneFactory.sol

@@ -21,7 +21,7 @@ TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
 SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 */
 
-pragma solidity 0.8.10;
+pragma solidity 0.8.20;
 
 contract CloneFactory { // implementation of eip-1167 - see https://eips.ethereum.org/EIPS/eip-1167
     function createClone(address target) internal returns (address result) {

contracts/contracts/ClrFund.sol

@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-3.0
 
-pragma solidity 0.8.10;
+pragma solidity 0.8.20;
 
 import '@openzeppelin/contracts/token/ERC20/ERC20.sol';
 import '@openzeppelin/contracts/token/ERC20/utils/SafeERC20.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/ClrFundDeployer.sol

@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-3.0
 
-pragma solidity 0.8.10;
+pragma solidity 0.8.20;
 
 import {MACIFactory} from './MACIFactory.sol';
 import {ClrFund} from './ClrFund.sol';
@@ -9,7 +9,7 @@ import {SignUpGatekeeper} from "maci-contracts/contracts/gatekeepers/SignUpGatek
 import {InitialVoiceCreditProxy} from "maci-contracts/contracts/initialVoiceCreditProxy/InitialVoiceCreditProxy.sol";
 import {Ownable} from '@openzeppelin/contracts/access/Ownable.sol';
 
-contract ClrFundDeployer is CloneFactory, Ownable {
+contract ClrFundDeployer is CloneFactory, Ownable(msg.sender) {
   address public clrfundTemplate;
   address public maciFactory;
   address public roundFactory;

contracts/contracts/ExternalContacts.sol

@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-3.0
 
-pragma solidity ^0.8.10;
+pragma solidity 0.8.20;
 
 /*
  * These imports are just for hardhat to find the contracts for deployment
@@ -9,5 +9,4 @@ pragma solidity ^0.8.10;
 import {Poll} from 'maci-contracts/contracts/Poll.sol';
 import {PollFactory} from 'maci-contracts/contracts/PollFactory.sol';
 import {TallyFactory} from 'maci-contracts/contracts/TallyFactory.sol';
-import {SubsidyFactory} from 'maci-contracts/contracts/SubsidyFactory.sol';
 import {MessageProcessorFactory} from 'maci-contracts/contracts/MessageProcessorFactory.sol';

contracts/contracts/FundingRound.sol

@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-3.0
 
-pragma solidity 0.8.10;
+pragma solidity 0.8.20;
 
 import '@openzeppelin/contracts/access/Ownable.sol';
 import '@openzeppelin/contracts/token/ERC20/ERC20.sol';
@@ -15,7 +15,7 @@ import {SignUpGatekeeper} from 'maci-contracts/contracts/gatekeepers/SignUpGatek
 import {InitialVoiceCreditProxy} from 'maci-contracts/contracts/initialVoiceCreditProxy/InitialVoiceCreditProxy.sol';
 import {CommonUtilities} from 'maci-contracts/contracts/utilities/CommonUtilities.sol';
 import {SnarkCommon} from 'maci-contracts/contracts/crypto/SnarkCommon.sol';
-import {ITallySubsidyFactory} from 'maci-contracts/contracts/interfaces/ITallySubsidyFactory.sol';
+import {ITallyFactory} from 'maci-contracts/contracts/interfaces/ITallyFactory.sol';
 import {IMessageProcessorFactory} from 'maci-contracts/contracts/interfaces/IMPFactory.sol';
 import {IClrFund} from './interfaces/IClrFund.sol';
 import {IMACIFactory} from './interfaces/IMACIFactory.sol';
@@ -25,7 +25,7 @@ import './userRegistry/IUserRegistry.sol';
 import './recipientRegistry/IRecipientRegistry.sol';
 
 contract FundingRound is
-  Ownable,
+  Ownable(msg.sender),
   SignUpGatekeeper,
   InitialVoiceCreditProxy,
   DomainObjs,
@@ -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
@@ -221,10 +209,10 @@ contract FundingRound is
     address vkRegistry = address(tally.vkRegistry());
 
     IMessageProcessorFactory messageProcessorFactory = maci.messageProcessorFactory();
-    ITallySubsidyFactory tallyFactory = maci.tallyFactory();
+    ITallyFactory tallyFactory = maci.tallyFactory();
 
-    address mp = messageProcessorFactory.deploy(verifier, vkRegistry, address(poll), coordinator);
-    address newTally = tallyFactory.deploy(verifier, vkRegistry, address(poll), mp, coordinator);
+    address mp = messageProcessorFactory.deploy(verifier, vkRegistry, address(poll), coordinator, Mode.QV);
+    address newTally = tallyFactory.deploy(verifier, vkRegistry, address(poll), mp, coordinator, Mode.QV);
     _setTally(newTally);
   }
 
@@ -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();
     }