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

Negative voting (v1) #283

Closed
wants to merge 23 commits into from
Closed

Negative voting (v1) #283

wants to merge 23 commits into from

Conversation

deomaius
Copy link

A feature previously prototyped but not entirely integrated in #112, the discussions here concluded that:

  • Positive and negative votes should be packed into a singular value, to avoid rewiring and optimise circuit logic
  • Positive and negative votes should be tallied separately to identify how contentious a vote may be

Upon building on the structure previously developed, the following aspects of the stack have been altered to support this feature

Circuits

VoteLeaf

  • Packed leaf now represents 50 bits, with each vote value (positive and negative) allocated 25 bits
  • Fixed range check in template UnpackVoteLeaf as it would fail for zero values with circomlib's LessEqThan
  • Introduced unit tests to validate the unpacking of values and computing the square root of the total weight cast

MessageValidator

  • Introduction of two CalculateVoteLeafSquared components; one for the messages vote leaf, the other for the current vote option vote leaf. Allowing the correct computation of the squared sum of positive and negative values for each leaf and deeming whether there is a sufficient voice credit balance

ProcessMessages

  • Addition of two CalculateVoteLeafSquared components; one for the current vote leaf, the other being the command's vote leaf to compute whether there are enough voice credits to process the message as valid

TallyVotes

  • Changed resultCalc component CalculateTotal to 2-dimensional array with elements [n][0] assigned positive tallies and [n][1] one with negative
  • Refactor result calculation to unpack positive and negative values via an array of UnpackVoteLeaf components, to feed each as an input to the associated index for summation
  • Replaced squared computation with the addition of two CalculateVoteLeafSquared template arrays, one for the total voice credits spent per vote option tally and the other for the total voice credits spent
  • Modified the root composition of the new results and current results Merkle trees, each leaf now translates to the sum of positive and negative votes at a specific depth for any tally

Domain Objects

  • VoteLeaf class refactored to suit a 50 bit value and to the new BigInt type

Core

  • Refactored to accommodate vote leaf objects rather than vote weight values

Cli

  • Negative weight flag is optional declared by --wv, if not flagged negative votes are set to zero

Integration tests

  • Accommodated to suit vote leaf objects rather than vote weight values
  • Refactored test suites to handle tally array elements as a 2-dimensional array

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.

I see some changes to cli/test2.sh. May I suggest creating a new testN.sh file instead? Also, please try the other testX.sh files; I'm not sure but they may also require updates.

@deomaius
Copy link
Author

deomaius commented Sep 24, 2021

After citing feedback from @weijiekoh on the initial iteration of this pull request, the change brought upon the tallying circuits result tree's root composition gave rise to a collision vulnerability. As the sum of the positive and negative tallies at a specific depth can be replicated using other integer combinations.

Therefore to inhibit this and retain the level of viability that is present in v1, leaves within the results trees now translate to packed vote leafs for positive and negative tallies at a specific depth with the introduction of the PackVoteLeaf circuit. |

Furthermore, integration tests have been refined and expanded, as it was not successfully conditioning the presence of the changeUsersKeys object.

circuits/circom/tallyVotes.circom Outdated Show resolved Hide resolved
circuits/circom/tallyVotes.circom Outdated Show resolved Hide resolved
circuits/ts/__tests__/TallyVotes.test.ts Outdated Show resolved Hide resolved
circuits/ts/__tests__/VoteLeaf.test.ts Outdated Show resolved Hide resolved
circuits/circom/voteLeaf.circom Outdated Show resolved Hide resolved
circuits/circom/voteLeaf.circom Outdated Show resolved Hide resolved
circuits/circom/voteLeaf.circom Outdated Show resolved Hide resolved
circuits/circom/voteLeaf.circom Outdated Show resolved Hide resolved
@weijiekoh weijiekoh marked this pull request as ready for review September 28, 2021 03:11
circuits/circom/voteLeaf.circom Outdated Show resolved Hide resolved
circuits/circom/voteLeaf.circom Outdated Show resolved Hide resolved
@deomaius
Copy link
Author

deomaius commented Sep 29, 2021

The constraints limiting vote leaf field sizes in both circumstances have been redacted, in place is the mux selector that @weijiekoh recommended to no-op invalid vote leaves when processing messages and PackVoteLeaf has been resized to 100 bits to accommodate positive and negative tallies. So, that an instance of MACI can operate without disruption if:

  • it fulfills the maximum possible amount of signups 2 ** 5 (tree arity) * 2 ** 10 (state tree depth)
  • if each user is configured a maximum voice credit balance 2 ** 32
  • if every user in a poll casts all credits (assuming 2 ** 32) towards the same vote option (whether negative or positive)
  • if a user submits an invalid vote leaf not fulfilling the range of 2 ** 50

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.

All good once this final suggestion is tested and committed.

circuits/circom/voteLeaf.circom Show resolved Hide resolved
Co-authored-by: Koh Wei Jie <weijiekoh@users.noreply.github.com>
@weijiekoh weijiekoh mentioned this pull request Oct 6, 2021
@daodesigner daodesigner self-requested a review June 7, 2022 21:23
This pull request was closed.
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