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

Calculate Reward Earnings For Stakers #10

Merged
merged 3 commits into from
Jan 19, 2024
Merged

Calculate Reward Earnings For Stakers #10

merged 3 commits into from
Jan 19, 2024

Conversation

apbendi
Copy link
Collaborator

@apbendi apbendi commented Dec 30, 2023

This PR implements the calculation of reward earnings and adds some tests for these calculations as well. Note that reward earning calculations are a port of the battle tested Synthetix Staking Rewards contract.

This PR is not as exhaustive as it could be, to prevent it from becoming too large. Purposely omitted from this PR are:

  • Making the notify rewards method permissioned EDIT: with the merger of Add and test privileged rewards notifier role #12 this functionality is now included
  • Tests for various additional scenarios, such as when the user withdraws some of their stake
  • Tests for each of the public view methods

Each of these will be tackled in future PRs. While these tasks are already documented in the spec issues, several "TODO" statements have also been left in the code for now as reminders. There are also comments left intentionally as reminders to revisit possible areas for better naming, refactoring and gas savings.

@apbendi apbendi marked this pull request as draft December 30, 2023 11:45
@apbendi apbendi marked this pull request as ready for review January 4, 2024 17:39
Copy link
Collaborator

@alexkeating alexkeating left a comment

Choose a reason for hiding this comment

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

Finished a first pass, I plan to take another look after our meeting Monday

src/UniStaker.sol Show resolved Hide resolved
src/UniStaker.sol Show resolved Hide resolved
src/UniStaker.sol Show resolved Hide resolved
src/UniStaker.sol Show resolved Hide resolved
test/UniStaker.t.sol Outdated Show resolved Hide resolved
Copy link
Collaborator

@alexkeating alexkeating left a 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, as long as the remaining TODOs are turned into tickets if they are not already

src/UniStaker.sol Outdated Show resolved Hide resolved
src/UniStaker.sol Outdated Show resolved Hide resolved
test/UniStaker.t.sol Outdated Show resolved Hide resolved
test/UniStaker.t.sol Outdated Show resolved Hide resolved
src/UniStaker.sol Outdated Show resolved Hide resolved
test/UniStaker.t.sol Show resolved Hide resolved
test/UniStaker.t.sol Outdated Show resolved Hide resolved
This commit implements the basic reward calculation mechanics associated
with staking. The mechanics are largely modeled off the Synthetix StakingRewards
contract. This commit also adds numerous tests, and related testing
infrastructure for assesing whether the calucations for earned rewards is being
done correctly.
Since we have (intentionally) left a full test suite of the reward earning
functionality out of this PR for now, we lower the coverage threshold a bit.
In the near future we will implement comprehensive tests and turn it back to
100.
Copy link

Coverage after merging stake-earn into main will be

100.00%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
src
   UniStaker.sol97.40%94.44%90.91%100%132
   V3FactoryOwner.sol100%100%100%100%

@apbendi apbendi merged commit 9fa7193 into main Jan 19, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants