-
Notifications
You must be signed in to change notification settings - Fork 0
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
Adding rewards per block with premint #50
Conversation
This relates to the debate on total supply and some implementation details of the desirable UX. I wanted to show that
|
rewardIndex += (newRewards * SCALE_FACTOR) / totalStaked; | ||
accountedRewards += newRewards; | ||
if (block.number > rewardEndBlock) { | ||
applicableBlocks = rewardEndBlock - lastRewardBlock; |
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.
I think this is problematic because rewardEndBlock
is not set during construction. Meaning, even when setRewardsPerBlock
is called the first time, rewardEndBlock
will be 0
if I'm not mistaken.
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.
Good point, gotta admit I kind of neglected the initiation and edge cases 😅
// Update the rewards per block and reward end block | ||
rewardsPerBlock = _rewardsPerBlock; | ||
rewardEndBlock = block.number + _durationInBlocks; | ||
} |
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.
Okay, so here we're saying, we have an admin that is able to set some reward per block for some duration in blocks.
This allows for issuance of rewards "in the future" because there's a fixed amount of rewards being available for claim per block.
This is opposed to prior this change, where, at any point in time one would update the reward index with whatever rewards are in regardless of how many blocks have passed.
To achieve the same result, one would have to send REWARD_TOKEN to the staking contract on a per block basis, so that rewardIndex
would be updatable on a per block basis with the newest rewards as well.
I think your proposed change is not bad
What I don't like that much is that the duration unit is blocks. Blocks time may very, so for the same segment of time, there could be different rewards because a different number of blocks has been processed.
I think it'd be more predictable to have a solution that uses actual time as duration instead.
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.
Okay, so here we're saying, we have an admin that is able to set some reward per block for some duration in blocks.
Or you can have epochs and rewards that arrived in the past one determine the current rate of rewards - and you do not need an admin.
What I don't like that much is that the duration unit is blocks.
Just wanted to show a minimal example and this was the easiest way to do it. In terms of UX, it does make sense to use time.
If setting |
} | ||
|
||
uint256 newRewards = applicableBlocks * rewardsPerBlock; |
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 one wants to eliminate the REWARD_TOKEN premint, this is where a mint call for newRewards
can be instead. Then, rewards are minted as state is updated and not in advance.
Thanks once again @mart1n-xyz for you contribution here. I'm closing this in favor of #74 which adds a similar functionality but it's more complete. |
Description
Main changes:
currentRewardIndex()
now usesrewardsPerBlock
andblock.number
to determine the currentrewardIndex
but does not update state. Hence, balances update in real time without a need for a transaction.updateRewardIndex()
updates state with the same logic.setRewardsPerBlock()
is an ownable function that changesrewardsPerBlock
untilrewardEndBlock
, it callsREWARD_TOKEN.mint()
for the amount need for this period given the two parameters. So, there should be always sufficient rewards ready in the contract.Checklist
Ensure you completed all of the steps below before submitting your pull request:
pnpm adorno
?pnpm verify
?