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: accounting for inflation update in tokenomics #200

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions contracts/Tokenomics.sol
Original file line number Diff line number Diff line change
Expand Up @@ -1296,6 +1296,46 @@ contract Tokenomics is TokenomicsConstants {
return true;
}

/// @dev Updates inflation per second if .
function updateInflationPerSecond() external {
// Check for the contract ownership
if (msg.sender != owner) {
revert OwnerOnly(msg.sender, owner);
}
Comment on lines +1303 to +1305
Copy link
Collaborator Author

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.


// Get current epoch length
uint256 curEpochLen = epochLen;
// Find if the year changes within the epoch
uint256 numYears = (block.timestamp + curEpochLen - timeLaunch) / ONE_YEAR;
// Revert if the year changes within the next epoch as it requires more complicated set of calculations
if (numYears > currentYear) {
revert Overflow(numYears, currentYear);
}
Comment on lines +1311 to +1314
Copy link
Contributor

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?

Copy link
Collaborator Author

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK

Copy link
Contributor

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;
Copy link
Contributor

Choose a reason for hiding this comment

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

what's maxBond?

Copy link
Collaborator Author

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.

if (curMaxBond > curEffectiveBond) {
revert Overflow(curMaxBond, curEffectiveBond);
}

// Adjust effective bond by reducing it with the on-going epoch maxBond value
curEffectiveBond -= curMaxBond;
Copy link
Contributor

Choose a reason for hiding this comment

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

don't follow

Copy link
Collaborator Author

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.


// Recalculate inflation per second based on the updated current year inflation
Copy link
Contributor

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Correct

Copy link
Contributor

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

uint256 curInflationPerSecond = getInflationForYear(currentYear) / ONE_YEAR;
Copy link
Collaborator

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.

Copy link
Collaborator Author

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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Needing new implementation getInflationForYear()


// maxBond has to be recalculated
curMaxBond = (curEpochLen * curInflationPerSecond * mapEpochTokenomics[epochCounter].epochPoint.maxBondFraction) / 100;
// Adjust effective bond with a new maxBond value
curEffectiveBond += curMaxBond;

// Update state variables
maxBond = uint96(curMaxBond);
effectiveBond = uint96(curEffectiveBond);
inflationPerSecond = uint96(curInflationPerSecond);
Comment on lines +1348 to +1350
Copy link
Collaborator Author

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.

Copy link
Contributor

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?

Copy link
Collaborator Author

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.

}

/// @dev Gets component / agent owner incentives and clears the balances.
/// @notice `account` must be the owner of components / agents Ids, otherwise the function will revert.
/// @notice If not all `unitIds` belonging to `account` were provided, they will be untouched and keep accumulating.
Expand Down
2 changes: 1 addition & 1 deletion hardhat.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ module.exports = {
settings: {
optimizer: {
enabled: true,
runs: 1500,
runs: 750,
Copy link
Collaborator Author

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

},
evmVersion: "cancun"
},
Expand Down
Loading