From a90bcf838a1236f212e9e22a725e3e605dbc707e Mon Sep 17 00:00:00 2001 From: ctrlc03 <93448202+ctrlc03@users.noreply.github.com> Date: Wed, 31 Jan 2024 10:59:52 +0000 Subject: [PATCH] fix(contracts): fix wrong tally processing check Ensure we stop tallying ballots correctly, and that an event is emitted even if batchStartIndex == number of ballots fix #1137 --- cli/tests/e2e/e2e.test.ts | 157 ++++++++--------- contracts/contracts/Tally.sol | 4 +- contracts/tests/Tally.test.ts | 307 +++++++++++++++++++++++++++++++++- 3 files changed, 385 insertions(+), 83 deletions(-) diff --git a/cli/tests/e2e/e2e.test.ts b/cli/tests/e2e/e2e.test.ts index 3b58e5f9b0..ebca9af533 100644 --- a/cli/tests/e2e/e2e.test.ts +++ b/cli/tests/e2e/e2e.test.ts @@ -190,84 +190,6 @@ describe("e2e tests", function test() { }); }); - describe("4 signups, 4 messages", () => { - after(() => { - cleanVanilla(); - }); - - const users = [new Keypair(), new Keypair(), new Keypair(), new Keypair()]; - - before(async () => { - // deploy the smart contracts - maciAddresses = await deploy(deployArgs); - // deploy a poll contract - pollAddresses = await deployPoll(deployPollArgs); - }); - - it("should signup four users", async () => { - // eslint-disable-next-line @typescript-eslint/prefer-for-of - for (let i = 0; i < users.length; i += 1) { - // eslint-disable-next-line no-await-in-loop - await signup({ maciPubKey: users[i].pubKey.serialize() }); - } - }); - - it("should publish four messages", async () => { - await publish({ - pubkey: users[0].pubKey.serialize(), - stateIndex: 1n, - voteOptionIndex: 0n, - nonce: 1n, - pollId: 0n, - newVoteWeight: 9n, - salt: genRandomSalt(), - privateKey: users[0].privKey.serialize(), - }); - - await publish({ - pubkey: users[1].pubKey.serialize(), - stateIndex: 2n, - voteOptionIndex: 1n, - nonce: 1n, - pollId: 0n, - newVoteWeight: 9n, - salt: genRandomSalt(), - privateKey: users[1].privKey.serialize(), - }); - - await publish({ - pubkey: users[2].pubKey.serialize(), - stateIndex: 3n, - voteOptionIndex: 2n, - nonce: 1n, - pollId: 0n, - newVoteWeight: 9n, - salt: genRandomSalt(), - privateKey: users[2].privKey.serialize(), - }); - - await publish({ - pubkey: users[3].pubKey.serialize(), - stateIndex: 4n, - voteOptionIndex: 3n, - nonce: 1n, - pollId: 0n, - newVoteWeight: 9n, - salt: genRandomSalt(), - privateKey: users[3].privKey.serialize(), - }); - }); - - it("should generate zk-SNARK proofs and verify them", async () => { - await timeTravel(timeTravelArgs); - await mergeMessages(mergeMessagesArgs); - await mergeSignups(mergeSignupsArgs); - const tallyFileData = await genProofs(genProofsArgs); - await proveOnChain(proveOnChainArgs); - await verify({ ...verifyArgs, tallyData: tallyFileData }); - }); - }); - describe("4 signups, 6 messages", () => { after(() => { cleanVanilla(); @@ -473,6 +395,85 @@ describe("e2e tests", function test() { }); }); + describe("30 signups (31 ballots), 4 messages", () => { + after(() => { + cleanVanilla(); + }); + + const users = Array.from({ length: 30 }, () => new Keypair()); + + before(async () => { + // deploy the smart contracts + maciAddresses = await deploy(deployArgs); + // deploy a poll contract + pollAddresses = await deployPoll(deployPollArgs); + }); + + it("should signup thirty users", async () => { + // eslint-disable-next-line @typescript-eslint/prefer-for-of + for (let i = 0; i < users.length; i += 1) { + // eslint-disable-next-line no-await-in-loop + await signup({ maciPubKey: users[i].pubKey.serialize() }); + } + }); + + it("should publish 4 messages", async () => { + // publish four different messages + await publish({ + pubkey: users[0].pubKey.serialize(), + stateIndex: 1n, + voteOptionIndex: 0n, + nonce: 1n, + pollId: 0n, + newVoteWeight: 9n, + salt: genRandomSalt(), + privateKey: users[0].privKey.serialize(), + }); + + await publish({ + pubkey: users[1].pubKey.serialize(), + stateIndex: 2n, + voteOptionIndex: 1n, + nonce: 1n, + pollId: 0n, + newVoteWeight: 9n, + salt: genRandomSalt(), + privateKey: users[1].privKey.serialize(), + }); + + await publish({ + pubkey: users[2].pubKey.serialize(), + stateIndex: 3n, + voteOptionIndex: 2n, + nonce: 1n, + pollId: 0n, + newVoteWeight: 9n, + salt: genRandomSalt(), + privateKey: users[2].privKey.serialize(), + }); + + await publish({ + pubkey: users[3].pubKey.serialize(), + stateIndex: 4n, + voteOptionIndex: 3n, + nonce: 1n, + pollId: 0n, + newVoteWeight: 9n, + salt: genRandomSalt(), + privateKey: users[3].privKey.serialize(), + }); + }); + + it("should generate zk-SNARK proofs and verify them", async () => { + await timeTravel(timeTravelArgs); + await mergeMessages(mergeMessagesArgs); + await mergeSignups(mergeSignupsArgs); + const tallyFileData = await genProofs(genProofsArgs); + await proveOnChain(proveOnChainArgs); + await verify({ ...verifyArgs, tallyData: tallyFileData }); + }); + }); + describe("checkKeys", () => { before(async () => { // deploy maci as we need the address diff --git a/contracts/contracts/Tally.sol b/contracts/contracts/Tally.sol index ff0c9493f6..e0dfa86359 100644 --- a/contracts/contracts/Tally.sol +++ b/contracts/contracts/Tally.sol @@ -135,7 +135,7 @@ contract Tally is Ownable, SnarkCommon, CommonUtilities, Hasher { (uint256 numSignUps, ) = poll.numSignUpsAndMessages(); // Require that there are untalied ballots left - if (batchStartIndex > numSignUps) { + if (batchStartIndex >= numSignUps) { revert AllBallotsTallied(); } @@ -148,7 +148,7 @@ contract Tally is Ownable, SnarkCommon, CommonUtilities, Hasher { // Update the tally commitment and the tally batch num tallyCommitment = _newTallyCommitment; - if ((cachedBatchNum + 1) * tallyBatchSize > numSignUps) { + if ((cachedBatchNum + 1) * tallyBatchSize >= numSignUps) { emit BallotsTallied(address(poll)); } } diff --git a/contracts/tests/Tally.test.ts b/contracts/tests/Tally.test.ts index 5dc9918b42..8acc905f9a 100644 --- a/contracts/tests/Tally.test.ts +++ b/contracts/tests/Tally.test.ts @@ -1,6 +1,6 @@ /* eslint-disable no-underscore-dangle */ import { expect } from "chai"; -import { BaseContract, Signer } from "ethers"; +import { AbiCoder, BaseContract, Signer } from "ethers"; import { EthereumProvider } from "hardhat/types"; import { MaciState, @@ -20,6 +20,7 @@ import { Tally, MACI, Poll as PollContract, MessageProcessor, Verifier, VkRegist import { STATE_TREE_DEPTH, duration, + initialVoiceCreditBalance, maxValues, messageBatchSize, tallyBatchSize, @@ -39,8 +40,8 @@ describe("TallyVotes", () => { let vkRegistryContract: VkRegistry; const coordinator = new Keypair(); - const users = [new Keypair(), new Keypair()]; - const maciState = new MaciState(STATE_TREE_DEPTH); + let users: Keypair[]; + let maciState: MaciState; const [pollAbi] = parseArtifact("Poll"); const [mpAbi] = parseArtifact("MessageProcessor"); @@ -52,6 +53,10 @@ describe("TallyVotes", () => { let generatedInputs: IProcessMessagesCircuitInputs; before(async () => { + maciState = new MaciState(STATE_TREE_DEPTH); + + users = [new Keypair(), new Keypair()]; + signer = await getDefaultSigner(); const r = await deployTestContracts(100, STATE_TREE_DEPTH, signer, true); @@ -197,4 +202,300 @@ describe("TallyVotes", () => { ).to.be.revertedWithCustomError(tallyContract, "AllBallotsTallied"); }); }); + + describe("ballots === tallyBatchSize", () => { + before(async () => { + // create 24 users (total 25 - 24 + 1 nothing up my sleeve) + users = Array.from({ length: 24 }, () => new Keypair()); + maciState = new MaciState(STATE_TREE_DEPTH); + + const updatedDuration = 5000000; + + const intStateTreeDepth = 2; + + const r = await deployTestContracts(100, STATE_TREE_DEPTH, signer, true); + maciContract = r.maciContract; + verifierContract = r.mockVerifierContract as Verifier; + vkRegistryContract = r.vkRegistryContract; + + // signup all users + // eslint-disable-next-line @typescript-eslint/prefer-for-of + for (let i = 0; i < users.length; i += 1) { + const timestamp = Math.floor(Date.now() / 1000); + // signup locally + maciState.signUp(users[i].pubKey, BigInt(initialVoiceCreditBalance), BigInt(timestamp)); + // signup on chain + + // eslint-disable-next-line no-await-in-loop + await maciContract.signUp( + users[i].pubKey.asContractParam(), + AbiCoder.defaultAbiCoder().encode(["uint256"], [1]), + AbiCoder.defaultAbiCoder().encode(["uint256"], [0]), + ); + } + + // deploy a poll + // deploy on chain poll + const tx = await maciContract.deployPoll( + updatedDuration, + { + ...treeDepths, + intStateTreeDepth, + }, + coordinator.pubKey.asContractParam(), + verifierContract, + vkRegistryContract, + false, + { + gasLimit: 10000000, + }, + ); + const receipt = await tx.wait(); + + const block = await signer.provider!.getBlock(receipt!.blockHash); + const deployTime = block!.timestamp; + + expect(receipt?.status).to.eq(1); + const iface = maciContract.interface; + const logs = receipt!.logs[receipt!.logs.length - 1]; + const event = iface.parseLog(logs as unknown as { topics: string[]; data: string }) as unknown as { + args: { + _pollId: bigint; + pollAddr: { + poll: string; + messageProcessor: string; + tally: string; + }; + }; + name: string; + }; + expect(event.name).to.eq("DeployPoll"); + + pollId = event.args._pollId; + + const pollContractAddress = await maciContract.getPoll(pollId); + pollContract = new BaseContract(pollContractAddress, pollAbi, signer) as PollContract; + mpContract = new BaseContract(event.args.pollAddr.messageProcessor, mpAbi, signer) as MessageProcessor; + tallyContract = new BaseContract(event.args.pollAddr.tally, tallyAbi, signer) as Tally; + + // deploy local poll + const p = maciState.deployPoll( + BigInt(deployTime + updatedDuration), + maxValues, + { + ...treeDepths, + intStateTreeDepth, + }, + messageBatchSize, + coordinator, + ); + expect(p.toString()).to.eq(pollId.toString()); + // publish the NOTHING_UP_MY_SLEEVE message + const messageData = [NOTHING_UP_MY_SLEEVE]; + for (let i = 1; i < 10; i += 1) { + messageData.push(BigInt(0)); + } + const message = new Message(BigInt(1), messageData); + const padKey = new PubKey([ + BigInt("10457101036533406547632367118273992217979173478358440826365724437999023779287"), + BigInt("19824078218392094440610104313265183977899662750282163392862422243483260492317"), + ]); + + // save the poll + poll = maciState.polls.get(pollId)!; + + poll.publishMessage(message, padKey); + + // update the poll state + poll.updatePoll(BigInt(maciState.stateLeaves.length)); + + // set the verification keys on the vk smart contract + await vkRegistryContract.setVerifyingKeys( + STATE_TREE_DEPTH, + intStateTreeDepth, + treeDepths.messageTreeDepth, + treeDepths.voteOptionTreeDepth, + messageBatchSize, + testProcessVk.asContractParam() as IVerifyingKeyStruct, + testTallyVk.asContractParam() as IVerifyingKeyStruct, + { gasLimit: 1000000 }, + ); + + await timeTravel(signer.provider! as unknown as EthereumProvider, updatedDuration); + await pollContract.mergeMaciStateAqSubRoots(0, pollId); + await pollContract.mergeMaciStateAq(0); + + await pollContract.mergeMessageAqSubRoots(0); + await pollContract.mergeMessageAq(); + + const processMessagesInputs = poll.processMessages(pollId); + await mpContract.processMessages(processMessagesInputs.newSbCommitment, [0, 0, 0, 0, 0, 0, 0, 0]); + }); + + it("should tally votes correctly", async () => { + const tallyGeneratedInputs = poll.tallyVotes(); + await expect(tallyContract.tallyVotes(tallyGeneratedInputs.newTallyCommitment, [0, 0, 0, 0, 0, 0, 0, 0])) + .to.emit(tallyContract, "BallotsTallied") + .withArgs(await pollContract.getAddress()); + + const onChainNewTallyCommitment = await tallyContract.tallyCommitment(); + expect(tallyGeneratedInputs.newTallyCommitment).to.eq(onChainNewTallyCommitment.toString()); + await expect( + tallyContract.tallyVotes(tallyGeneratedInputs.newTallyCommitment, [0, 0, 0, 0, 0, 0, 0, 0]), + ).to.be.revertedWithCustomError(tallyContract, "AllBallotsTallied"); + }); + }); + + describe("ballots > tallyBatchSize", () => { + before(async () => { + // create 25 users (and thus 26 ballots) (total 26 - 25 + 1 nothing up my sleeve) + users = Array.from({ length: 25 }, () => new Keypair()); + maciState = new MaciState(STATE_TREE_DEPTH); + + const updatedDuration = 5000000; + + const intStateTreeDepth = 2; + + const r = await deployTestContracts(100, STATE_TREE_DEPTH, signer, true); + maciContract = r.maciContract; + verifierContract = r.mockVerifierContract as Verifier; + vkRegistryContract = r.vkRegistryContract; + + // signup all users + // eslint-disable-next-line @typescript-eslint/prefer-for-of + for (let i = 0; i < users.length; i += 1) { + const timestamp = Math.floor(Date.now() / 1000); + // signup locally + maciState.signUp(users[i].pubKey, BigInt(initialVoiceCreditBalance), BigInt(timestamp)); + // signup on chain + + // eslint-disable-next-line no-await-in-loop + await maciContract.signUp( + users[i].pubKey.asContractParam(), + AbiCoder.defaultAbiCoder().encode(["uint256"], [1]), + AbiCoder.defaultAbiCoder().encode(["uint256"], [0]), + ); + } + + // deploy a poll + // deploy on chain poll + const tx = await maciContract.deployPoll( + updatedDuration, + { + ...treeDepths, + intStateTreeDepth, + }, + coordinator.pubKey.asContractParam(), + verifierContract, + vkRegistryContract, + false, + { + gasLimit: 10000000, + }, + ); + const receipt = await tx.wait(); + + const block = await signer.provider!.getBlock(receipt!.blockHash); + const deployTime = block!.timestamp; + + expect(receipt?.status).to.eq(1); + const iface = maciContract.interface; + const logs = receipt!.logs[receipt!.logs.length - 1]; + const event = iface.parseLog(logs as unknown as { topics: string[]; data: string }) as unknown as { + args: { + _pollId: bigint; + pollAddr: { + poll: string; + messageProcessor: string; + tally: string; + }; + }; + name: string; + }; + expect(event.name).to.eq("DeployPoll"); + + pollId = event.args._pollId; + + const pollContractAddress = await maciContract.getPoll(pollId); + pollContract = new BaseContract(pollContractAddress, pollAbi, signer) as PollContract; + mpContract = new BaseContract(event.args.pollAddr.messageProcessor, mpAbi, signer) as MessageProcessor; + tallyContract = new BaseContract(event.args.pollAddr.tally, tallyAbi, signer) as Tally; + + // deploy local poll + const p = maciState.deployPoll( + BigInt(deployTime + updatedDuration), + maxValues, + { + ...treeDepths, + intStateTreeDepth, + }, + messageBatchSize, + coordinator, + ); + expect(p.toString()).to.eq(pollId.toString()); + // publish the NOTHING_UP_MY_SLEEVE message + const messageData = [NOTHING_UP_MY_SLEEVE]; + for (let i = 1; i < 10; i += 1) { + messageData.push(BigInt(0)); + } + const message = new Message(BigInt(1), messageData); + const padKey = new PubKey([ + BigInt("10457101036533406547632367118273992217979173478358440826365724437999023779287"), + BigInt("19824078218392094440610104313265183977899662750282163392862422243483260492317"), + ]); + + // save the poll + poll = maciState.polls.get(pollId)!; + + poll.publishMessage(message, padKey); + + // update the poll state + poll.updatePoll(BigInt(maciState.stateLeaves.length)); + + // set the verification keys on the vk smart contract + await vkRegistryContract.setVerifyingKeys( + STATE_TREE_DEPTH, + intStateTreeDepth, + treeDepths.messageTreeDepth, + treeDepths.voteOptionTreeDepth, + messageBatchSize, + testProcessVk.asContractParam() as IVerifyingKeyStruct, + testTallyVk.asContractParam() as IVerifyingKeyStruct, + { gasLimit: 1000000 }, + ); + + await timeTravel(signer.provider! as unknown as EthereumProvider, updatedDuration); + await pollContract.mergeMaciStateAqSubRoots(0, pollId); + await pollContract.mergeMaciStateAq(0); + + await pollContract.mergeMessageAqSubRoots(0); + await pollContract.mergeMessageAq(); + + const processMessagesInputs = poll.processMessages(pollId); + await mpContract.processMessages(processMessagesInputs.newSbCommitment, [0, 0, 0, 0, 0, 0, 0, 0]); + }); + + it("should tally votes correctly", async () => { + // tally first batch + let tallyGeneratedInputs = poll.tallyVotes(); + + await tallyContract.tallyVotes(tallyGeneratedInputs.newTallyCommitment, [0, 0, 0, 0, 0, 0, 0, 0]); + + // check commitment + const onChainNewTallyCommitment = await tallyContract.tallyCommitment(); + expect(tallyGeneratedInputs.newTallyCommitment).to.eq(onChainNewTallyCommitment.toString()); + + // tally second batch + tallyGeneratedInputs = poll.tallyVotes(); + + await expect(tallyContract.tallyVotes(tallyGeneratedInputs.newTallyCommitment, [0, 0, 0, 0, 0, 0, 0, 0])) + .to.emit(tallyContract, "BallotsTallied") + .withArgs(await pollContract.getAddress()); + + // check that it fails to tally again + await expect( + tallyContract.tallyVotes(tallyGeneratedInputs.newTallyCommitment, [0, 0, 0, 0, 0, 0, 0, 0]), + ).to.be.revertedWithCustomError(tallyContract, "AllBallotsTallied"); + }); + }); });