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.

Sign up for GitHub

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

Jump to bottom

feat(multiple-polls): allow concurrent polls #1417

Merged
merged 1 commit into from
May 2, 2024
Merged

feat(multiple-polls): allow concurrent polls #1417

merged 1 commit into from
May 2, 2024

Conversation

ctrlc03
Copy link
Collaborator

@ctrlc03 ctrlc03 commented May 2, 2024 •
edited
Loading

Description

  • use lazyIMT for signups to allow for concurrent polls
  • add tests
  • amend maci components to support the change

Benefits:

  • cheaper signups overall
  • less code
  • no merge required
  • concurrent polls

Drawbacks:

  • new trusted setup needed
  • slightly more constraints in the circuits

Related issue(s)

fix #1418

Confirmation

@vercel Vercel
Copy link

vercel bot commented May 2, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
maci-website ✅ Ready (Inspect) Visit Preview May 2, 2024 8:28pm

@ctrlc03 ctrlc03 self-assigned this May 2, 2024
github-advanced-security[bot]
github-advanced-security bot found potential problems May 2, 2024
View reviewed changes
contracts/contracts/Tally.sol
current = _leaf;
uint8 k;

uint256[TREE_ARITY] memory level;
uint256[VOTE_OPTION_TREE_ARITY] memory level;

Check warning

Code scanning / Slither

Uninitialized local variables Medium

Tally.computeMerkleRootFromPath(uint8,uint256,uint256,uint256[][]).level is a local variable never initialized
contracts/contracts/trees/LazyIMT.sol Fixed Show fixed Hide fixed
contracts/contracts/trees/LazyIMT.sol Fixed Show fixed Hide fixed
@ctrlc03 ctrlc03 force-pushed the feat/multiple-polls branch from ab67fe0 to b4afd25 Compare May 2, 2024 10:36
@ctrlc03 ctrlc03 added the enhancement New feature or request label May 2, 2024
@ctrlc03 ctrlc03 force-pushed the feat/multiple-polls branch from b4afd25 to 1c3f4d3 Compare May 2, 2024 10:47
0xmad
0xmad requested changes May 2, 2024
View reviewed changes
Copy link
Collaborator

@0xmad 0xmad left a comment

Choose a reason for hiding this comment

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

@ctrlc03 thanks, just added some comments and questions

circuits/circom/trees/incrementalMerkleTree.circom Show resolved Hide resolved
circuits/circom/trees/incrementalMerkleTree.circom Show resolved Hide resolved
circuits/circom/trees/incrementalMerkleTree.circom Outdated Show resolved Hide resolved
contracts/contracts/Poll.sol Outdated Show resolved Hide resolved
contracts/contracts/MACI.sol Show resolved Hide resolved
contracts/contracts/trees/LazyIMT.sol Outdated Show resolved Hide resolved
contracts/tasks/helpers/TreeMerger.ts Outdated Show resolved Hide resolved
contracts/tasks/runner/merge.ts Show resolved Hide resolved
contracts/tests/MACI.test.ts Outdated Show resolved Hide resolved
cli/ts/commands/genProofs.ts Show resolved Hide resolved
@ctrlc03 ctrlc03 force-pushed the feat/multiple-polls branch from 1c3f4d3 to 55fa6f1 Compare May 2, 2024 15:29
0xmad
0xmad reviewed May 2, 2024
View reviewed changes
cli/tests/ceremony-params/ceremonyParams.test.ts Outdated Show resolved Hide resolved
contracts/contracts/trees/LazyIMT.sol Outdated Show resolved Hide resolved
@ctrlc03 ctrlc03 force-pushed the feat/multiple-polls branch from 55fa6f1 to 4b7ae2f Compare May 2, 2024 19:56
@ctrlc03 ctrlc03 requested a review from 0xmad May 2, 2024 19:56
0xmad
0xmad approved these changes May 2, 2024
View reviewed changes
Copy link
Collaborator

@0xmad 0xmad left a comment

Choose a reason for hiding this comment

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

@ctrlc03 thanks!
I'll test it when new zkeys are available, for local env looks good to me.

@ctrlc03
Copy link
Collaborator Author

ctrlc03 commented May 2, 2024

@ctrlc03 thanks! I'll test it when new zkeys are available, for local env looks good to me.

gonna build them and push to s3 :)

@ctrlc03
feat(multiple-polls): allow concurrent polls
14e89ba 
use lazyIMT for signups to allow for concurrent polls
@ctrlc03 ctrlc03 force-pushed the feat/multiple-polls branch from 4b7ae2f to 14e89ba Compare May 2, 2024 20:26
@ctrlc03 ctrlc03 enabled auto-merge May 2, 2024 20:50
@ctrlc03 ctrlc03 merged commit 85b4815 into dev May 2, 2024
20 checks passed
@ctrlc03 ctrlc03 deleted the feat/multiple-polls branch May 2, 2024 20:55
@ctrlc03 ctrlc03 mentioned this pull request May 6, 2024
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xmad 0xmad 0xmad approved these changes

@kittybest kittybest Awaiting requested review from kittybest kittybest is a code owner

@samajammin samajammin Awaiting requested review from samajammin

@crisgarner crisgarner Awaiting requested review from crisgarner crisgarner is a code owner

Assignees

@ctrlc03 ctrlc03

Labels
enhancement New feature or request
Projects
MACI
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow multiple (and concurrent) Polls
2 participants
@ctrlc03 @0xmad