-
Notifications
You must be signed in to change notification settings - Fork 5
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
refactor: use minime and require limit when evaluating votes #72
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
0x-r4bbit
force-pushed
the
refactor/limit-and-minime
branch
from
September 28, 2023 07:11
8d6e330
to
fd14a19
Compare
0x-r4bbit
force-pushed
the
refactor/remove-safemath
branch
from
September 28, 2023 11:47
ba1c956
to
08c6a09
Compare
0x-r4bbit
force-pushed
the
refactor/limit-and-minime
branch
from
September 28, 2023 11:48
fd14a19
to
5c2f8c7
Compare
0x-r4bbit
force-pushed
the
refactor/remove-safemath
branch
from
September 28, 2023 13:41
08c6a09
to
fcb33ca
Compare
0x-r4bbit
force-pushed
the
refactor/limit-and-minime
branch
from
September 28, 2023 13:43
5c2f8c7
to
41fe3f3
Compare
Closed
0x-r4bbit
force-pushed
the
refactor/remove-safemath
branch
from
September 29, 2023 10:16
fcb33ca
to
6380995
Compare
0x-r4bbit
force-pushed
the
refactor/limit-and-minime
branch
from
September 29, 2023 10:24
41fe3f3
to
01e207a
Compare
osmaczko
approved these changes
Oct 2, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 👍
0x-r4bbit
force-pushed
the
refactor/remove-safemath
branch
from
October 4, 2023 08:21
6380995
to
763f3ac
Compare
0x-r4bbit
force-pushed
the
refactor/limit-and-minime
branch
from
October 4, 2023 08:21
01e207a
to
8a9428b
Compare
0x-r4bbit
force-pushed
the
refactor/remove-safemath
branch
2 times, most recently
from
October 4, 2023 09:06
f172af7
to
9999b49
Compare
This commit introduces a change that has been largely discussed in #65 Generally the idea is the following: Prior to this commit, the amount of votes to be evaluated is unbounded, meaning we can potentially run out of gas when too many votes have to be evaluated in a single transaction. To account for that, vote processing has to be batched based on a limit that is provided when the contract is called. As a result of batching, finalization of votes can happen over multiple transactions which makes checking balances of voters tricky because evaluation happens at different blocks where balances might have changed. That's why we're also introducing the use of MiniMeToken over standard ERC20 because it allows us to check balances of accounts at certain blocks. This commit introduces the following changes in both VotingContract and FeaturedVotingContract: Evaluation of votes now always happens in three places: When a voting is initialized, when votes are casted and when a voting is being finalized. When initializing a voting or casting votes, the votes are evaluated against the current block, front-ends can therefore show a good estimation of the current vote state. Only the "new" votes are evaluated, which should equal to the amount of votes being sent. When finalizaing a vote, all votes are (re)evaluated against the block at which the finalization has started. This means with finalization, all votes are evaluated again to ensure they represent the latest state. In terms of code this means: _evaluateVotes() requires and additional limit, as a result of that finializeVoting() and finalizeVotingRoom() require a limit as well. castVotes() derives the limit from the vote amount that's being sent.
0x-r4bbit
force-pushed
the
refactor/limit-and-minime
branch
from
October 4, 2023 09:12
8a9428b
to
e8ecac4
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit introduces a change that has been largely discussed in #65
Generally the idea is the following:
limit
that is provided when the contract is called.MiniMeToken
over standardERC20
because it allows us to check balances of accounts at certain blocks.This commit introduces the following changes in both
VotingContract
andFeaturedVotingContract
:In terms of code this means:
_evaluateVotes()
requires and additionallimit
, as a result of thatfinializeVoting()
andfinalizeVotingRoom()
require alimit
as well.castVotes()
derives thelimit
from the vote amount that's being sent.