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

Per deposit accounting #14

Merged
merged 1 commit into from
Sep 25, 2024
Merged

Per deposit accounting #14

merged 1 commit into from
Sep 25, 2024

Conversation

apbendi
Copy link
Collaborator

@apbendi apbendi commented Sep 24, 2024

closes #5

@apbendi apbendi force-pushed the per-deposit-accounting branch from 06d913a to aae0398 Compare September 24, 2024 21:53
@apbendi apbendi requested a review from alexkeating September 24, 2024 21:53
@apbendi apbendi marked this pull request as ready for review September 24, 2024 21:54
@@ -130,13 +130,23 @@ contract GovernanceStakerTest is Test, PercentAssertions {
view
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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.

Nits

src/GovernanceStaker.sol Outdated Show resolved Hide resolved
src/GovernanceStaker.sol Outdated Show resolved Hide resolved
src/GovernanceStaker.sol Show resolved Hide resolved
src/GovernanceStaker.sol Show resolved Hide resolved
@apbendi apbendi changed the base branch from convert-uint to main September 25, 2024 19:28
@apbendi apbendi force-pushed the per-deposit-accounting branch from aae0398 to 1b75abc Compare September 25, 2024 19:30
UniStaker tracks rewards earned by individual addresses which are specified as
the beneficiary of deposits. The same address may earn rewards from tokens
which have been staked in multiple deposits.

This commit converts the system to track rewards on a per-deposit basis instead.
Each deposit earns rewards according to the tokens staked in it. The test suite has
been updated to reflect this change.

The meaning of beneficiary changed as well. It is no longer the address that earns
the rewards, but instead is the address that has permissions to withdrawal the
rewards earned by a given deposit.

Co-Author: Keating <[email protected]>
@apbendi apbendi force-pushed the per-deposit-accounting branch from 8ea6f48 to f26956a Compare September 25, 2024 19:56
Copy link

Coverage after merging per-deposit-accounting into main will be

100.00%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
src
   DelegationSurrogate.sol100%100%100%100%
   GovernanceStaker.sol100%100%100%100%

@apbendi apbendi merged commit 61d1040 into main Sep 25, 2024
4 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.

Move values tracked on a per-beneficiary basis to be tracked on a per deposit basis
2 participants