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

feat(voting): first implementation #12

Open
wants to merge 25 commits into
base: dev
Choose a base branch
from
Open

feat(voting): first implementation #12

wants to merge 25 commits into from

Conversation

valentinpollart
Copy link
Collaborator

Implement a voting features embedding :

  • Voting and delegating vote to other voters
  • Tag management for votes

Each ballot has its own contract deployed following clone factory pattern (see EIP-1167 and open-zeppelin proposition of minimal proxy implementation.

@codecov
Copy link

codecov bot commented Apr 22, 2022

Codecov Report

❗ No coverage uploaded for pull request base (dev@b2affbf). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 55044a5 differs from pull request most recent head c3064ca. Consider uploading reports for the commit c3064ca to get more accurate results

@@          Coverage Diff           @@
##             dev      #12   +/-   ##
======================================
  Coverage       ?   97.35%           
======================================
  Files          ?        8           
  Lines          ?      151           
  Branches       ?       32           
======================================
  Hits           ?      147           
  Misses         ?        2           
  Partials       ?        2           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b2affbf...c3064ca. Read the comment docs.

Copy link
Collaborator

@matbour matbour left a comment

Choose a reason for hiding this comment

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

First of all, thank you so much for your work. Regarding the PR, I think that we should take more time to comment our contracts (at the moment, none of the voting contracts have comments).
Please use the @dev and @notice and comment all the contracts and the functions.

Regarding the Ballot contract, we need to get more info on a few topics.

  • What happens if someone has more thatn 25,000 DPS, submit a vote and the, uses the DPS/SQUARE bridge?
  • Is the "manual" ballot close by the a owner a good solution? It would let the controls when the ballot ends and might allow to close the ballot at an arbritrary timestamp. We could use the block.timestamp value in order to be transparent and decentralized. We could even discuss of removing the Ownable interface.

Regarding the BallogTags, which definitely need to be renamed, I think that using standard keccak digests, like the in the OpenZeppelin AccessControl. The would allow to save gas by using 32 bytes alignments + using arbitrary long strings as tags.

contracts/VotingProxy.sol Outdated Show resolved Hide resolved
contracts/VotingProxy.sol Outdated Show resolved Hide resolved
contracts/Ballot.sol Outdated Show resolved Hide resolved
contracts/Ballot.sol Outdated Show resolved Hide resolved
@matbour
Copy link
Collaborator

matbour commented Apr 25, 2022

I also suggest to drop the BallotTagManager contract and use the keccak of the topics instead. Users can then delegate to any other user keccack(TOPIC).

contracts/VotingProxy.sol Outdated Show resolved Hide resolved
@matbour matbour changed the title feat: voting feat(voting): first implementation Apr 26, 2022
@matbour matbour changed the base branch from main to dev April 26, 2022 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

3 participants