-
Notifications
You must be signed in to change notification settings - Fork 9
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: accounting for inflation update in tokenomics #200
base: main
Are you sure you want to change the base?
Conversation
kupermind
commented
Jan 7, 2025
- Accounting for inflation update in tokenomics.
if (msg.sender != owner) { | ||
revert OwnerOnly(msg.sender, owner); | ||
} |
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.
This can be called by owner at any time, as the logic below accounts for it.
maxBond = uint96(curMaxBond); | ||
effectiveBond = uint96(curEffectiveBond); | ||
inflationPerSecond = uint96(curInflationPerSecond); |
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.
These are critical values that need to be updated.
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.
So we only need to do this for bonding?
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.
Correct, bonding is the only one that is issued as a credit for epoch, the rest is end-of-epoch-computed.
@@ -251,7 +251,7 @@ module.exports = { | |||
settings: { | |||
optimizer: { | |||
enabled: true, | |||
runs: 1500, | |||
runs: 750, |
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.
Need to reduce optimization iterations in order to fit into deployment size
// Revert if the year changes within the next epoch as it requires more complicated set of calculations | ||
if (numYears > currentYear) { | ||
revert Overflow(numYears, currentYear); | ||
} |
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.
hm - can you be more explicit what happens then?
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.
When the year changes, the complex re-calculation of inflation before the exact second year end and the rest of the epoch happens, so it's better to avoid to perform another change during that time.
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.
OK
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.
Please add as much detail in the contract comment for future
|
||
// Ensure effectiveBond is bigger than maxBond | ||
uint256 curEffectiveBond = effectiveBond; | ||
uint256 curMaxBond = maxBond; |
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.
what's maxBond?
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.
max amount that can go for bonding per epoch.
// Adjust effective bond by reducing it with the on-going epoch maxBond value | ||
curEffectiveBond -= curMaxBond; | ||
|
||
// Recalculate inflation per second based on the updated current year inflation |
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.
If the curve hasn't changed, the value will be the same as before right? We should have a test
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.
Correct
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.
Let's add this as a comment to the contract and also write a test for it
maxBond = uint96(curMaxBond); | ||
effectiveBond = uint96(curEffectiveBond); | ||
inflationPerSecond = uint96(curInflationPerSecond); |
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.
So we only need to do this for bonding?
} | ||
|
||
// Adjust effective bond by reducing it with the on-going epoch maxBond value | ||
curEffectiveBond -= curMaxBond; |
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.
don't follow
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.
maxBond is given as a credit for epoch, so we need to subtract what is already given in the current epoch based on previous inflation, and add it again later based on a new inflation.
contracts/Tokenomics.sol
Outdated
curEffectiveBond -= curMaxBond; | ||
|
||
// Recalculate inflation per second based on the updated current year inflation | ||
uint256 curInflationPerSecond = getInflationForYear(currentYear) / ONE_YEAR; |
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.
@kupermind @mariapiamo
Is there some unaccounted/side effect between the current implementation, when it changes strictly once a year and in the constants there is a mutually agreed table of inflation per year with the fact that now inflation can change an arbitrary number of times during the year.
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.
We just need to give an extra buffer accounting for the current inflation and for the time when it's going to change to another one.
contracts/Tokenomics.sol
Outdated
curEffectiveBond -= curMaxBond; | ||
|
||
// Recalculate inflation per second based on the updated current year inflation | ||
uint256 curInflationPerSecond = getInflationForYear(currentYear) / ONE_YEAR; |
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.
Needing new implementation getInflationForYear()