search

1Hive / dandelion-voting-app

Contains the Dandelion Voting app which implements an ACLOracle
GNU General Public License v3.0
5 stars 8 forks source link

Added staggered vote starting with tests. #10

Closed willjgriff closed 5 years ago

willjgriff commented 5 years ago

PR against remove-early-execution branch for easier review. Will merge both at once.

Requires thought into how to determine vote snapshot block, which is the block number when the vote starts and is used to determine the vote weight of individual addresses. The only solution I can think of is to stagger votes by blocks instead of seconds so we can set the snapshot block when the vote is created. I'm open to suggestions though.

EDIT: Renamed from stagger to buffer and have updated the vote length to be determined by blocks instead of time so we know the snapshot block at vote creation. A check that the vote token has a supply > 0 when the vote is created has been removed as we can only check this when the vote starts. Presumably the UI will display pending votes, in which case it will have to check if the vote isOpen (which is true if the token has a supply > 0 and it's within the vote window) after it's start block has passed to determine whether to show the vote or not. If isOpen is still false at this point no one can vote anyway and the vote should be hidden. The check that the supply > 0 could be extracted into a separate function if we wish. Easier to tell once the UI is being built though.

rperez89 commented 5 years ago

@willjgriff following your idea what can be done in order to have a best ui/ux is to show the numbers of blocks to the user and the current average time by block like in an info section

willjgriff commented 5 years ago

@willjgriff following your idea what can be done in order to have a best ui/ux is to show the numbers of blocks to the user and the current average time by block like in an info section

Great idea, maybe it isn't such a problem if we make sure the user knows the vote end time is an estimation.

lkngtn commented 5 years ago

LGTM, let's get this merged?