Update locking formula #375
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
Update locking formula
🚨 Report Summary
For more details view the full report in OpenZeppelin Code Inspector |
chapati23
approved these changes
Feb 21, 2024
There was a problem hiding this comment.
great work (esp given deadline pressure!)
- i took some time to understand the new requirements and read through the rarible docs again as well
- reviewed the code changes
- reviewed the tests (although my review here wasn't as deep, so 2nd review should dig a bit deeper here)
- ran the tests locally
think @bowd should be final reviewer on this
nvtaveras
approved these changes
Feb 22, 2024
bowd
commented
Feb 23, 2024
|
|
||
| uint256 amountMultiplied = uint256(amount) * uint256(multiplier); | ||
| lockAmount = uint96(amountMultiplied / (ST_FORMULA_DIVIDER)); | ||
| lockAmount = uint96(amountMultiplied / (ST_FORMULA_BASIS)); | ||
| require(lockAmount > 0, "voting power is 0"); |
There was a problem hiding this comment.
Not sure if this require is really needed anymore unless both minCliffPeriod and minSlopePeriod are configured to 0, but it's probably good to have anyway
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.
Description
Currently the voting power formula looks like this (slightly simplified):
We want to bring the system closer to the classical veCurve formula, by changing it to:
More details can be seen here #373
Since this formula sits in the core of the voting power calculations, reviews should be on a broader level. Please go through code changes, test calculations and more importantly how this may affect the whole locking mechanism.
Known issues:
Other changes
Updated the tests to use the exact amounts and not approximations
Removed unused imports
Tested
Fixed the tests related to change. Actual calculations are added as comments.
Some extra fuzz tests are WIP and will be added with a separate PR
Related issues