Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix proof generation for subsequential polls #317

Merged

Conversation

corydickson
Copy link
Contributor

@corydickson corydickson commented Nov 30, 2021

Instead of allowing concurrent polls where users must signup for each poll, this PR aims to allow on chain proof verification for a multiple poll instances with the same set of users.

Testing run cli/testMultiplePoll.sh


node build/index.js timeTravel -s 140 && \
node build/index.js mergeMessages -x 0xf204a4Ef082f5c04bB89F7D5E6568B796096735a -o 1 && \
node build/index.js mergeSignups -x 0xf204a4Ef082f5c04bB89F7D5E6568B796096735a -o 1 && \
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without changing how the sbCommitments are updated, this command is still necessary even though the StateAq subtrees are merged.

@weijiekoh
Copy link
Contributor

Hi! Does this include #311 or should that be merged separately?

@weijiekoh
Copy link
Contributor

PollFactory compiles to 25Kb which exceeds the mainnet limit

weijiekoh
weijiekoh previously approved these changes Nov 30, 2021
@weijiekoh weijiekoh self-requested a review November 30, 2021 23:49
@weijiekoh
Copy link
Contributor

Suggestions to reduce the size of Poll.sol:

  • Comment out batchEnqueueMessage which isn't used - size is now 24.57 Kb
  • Have a separate ResultVerifier contract that implements verifySpentVoiceCredits, verifyPerVOSpentVoiceCredits, verifyTallyResult, and computeMerkleRootFromPath, instead of implementing these functions in Poll.sol.

@weijiekoh weijiekoh dismissed their stale review November 30, 2021 23:54

We should reduce the size of Poll.sol before merging this PR

Copy link
Contributor

@weijiekoh weijiekoh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and the test script works on my end too. We just need to reduce the size of Poll.sol - see the previous comment on how we can do this.

Edit: let's do the size reduction in a different PR.

@weijiekoh weijiekoh merged commit 1ce1cb7 into privacy-scaling-explorations:v1 Dec 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants