treasury: Delta Governor v1 #615
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Will be required for new checkpoints code and later governor implementation.
Used for checkpointing logic in bonding state checkpoints
Handles historic checkpointing ("snapshotting") and lookup
of the bonding state.
- unit tests - integration tests - gas-report - fork test for upgrade
This will provide a cleaner experience using governance tools, since users that don't have any stake won't get errors when querying their voting power. For users that do have stake, we will make sure to checkpoint them on first deploy.
victorges
changed the base branch from
confluence
to
vg/feat/bonding-checkpoints
victorges
force-pushed
the
vg/feat/onchain-governor
branch
5 times, most recently
from
80cf668
to
109d0b1
Compare
victorges
force-pushed
the
vg/feat/onchain-governor
branch
from
109d0b1
to
6c7b733
Compare
victorges
force-pushed
the
vg/feat/onchain-governor
branch
from
6c7b733
to
2fd7abb
Compare
100% coverage 😎
"Harness" seems to make more sense, I could only think of that now.
victorges
commented
Jul 25, 2023
victorges
commented
Jul 26, 2023
0xcadams
reviewed
Jul 26, 2023
Co-authored-by: Chase Adams <[email protected]>
yondonfu
requested changes
Aug 10, 2023
2 tasks
Meaning the start of next round instead of the current round. This is more compatible with the way OZ expects the timepoints to work in the clock and snapshots on the Governor framework.
This changes the BondingVotes implementation to stop having so many reverts on supposedly invalid states. Instead, redefine the voting power (and checkpointed stake) as being zero before the first round to be checkpointed. This had other implications in the code like removing changes in BondingManager to make the state complete (always having an earnings pool on lastClaimRound). Instead, the BondingVotes implementaiton is resilient to that as well and just assumes reward() had never been called. This also fixed the redundant reverts between BondingVotes and SortedArrays, as now there are almost no reverts.
2 tasks
Just for consistency with other contracts. Some docs as a bonus
|
@yondonfu Also ready for another round! 😁 Notice that I made a "merge" with the parent branch just to avoid rebasing and losing all history. Let me know if you'd prefer a clean rebase. When merging, my plan is to either do the full linearization of the branches or squash&merge them instead not to keep the non-linear git history. |
This is to increase compatibility with some tools out there like Tally, which require only these functions from the ERC20 spec, not the full implementation. So we can have these from the get-go to make things easier if we want to make something with them.
victorges
added a commit
that referenced
this pull request
Oct 11, 2023
* package.json: Update @OpenZeppelin libraries Will be required for new checkpoints code and later governor implementation. * bonding: Create SortedArrays library Used for checkpointing logic in bonding state checkpoints * bonding: Create BondingCheckpoints contract Handles historic checkpointing ("snapshotting") and lookup of the bonding state. * bonding: Checkpoint bonding state on changes * test/bonding: Test BondingManager and Checkpoints - unit tests - integration tests - gas-report - fork test for upgrade * bonding: Migrate to custom error types * bonding: Allow querying unbonded+uncheckpointed accounts This will provide a cleaner experience using governance tools, since users that don't have any stake won't get errors when querying their voting power. For users that do have stake, we will make sure to checkpoint them on first deploy. * treasury: Create treasury governance contracts * test/treasury: Add unit test fro BondingCheckpointsVotes * test/treasury: Test GovernorCountingOverridable * test/treasury: Test LivepeerGovernor * test/treasury: A couple additional Governor tests 100% coverage 😎 * test/treasury: Rename Counting unit test mock "Harness" seems to make more sense, I could only think of that now. * Apply suggestions from code review Co-authored-by: Chase Adams <[email protected]> * treasury: Fix storage layout situation * treasury: Move governor initial params to configs * bonding: Make sure we checkpoint up to once per op * bonding: Make bonding checkpoints implement IVotes * bonding: Read votes from the end of the round Meaning the start of next round instead of the current round. This is more compatible with the way OZ expects the timepoints to work in the clock and snapshots on the Governor framework. * bonding: Make checkpoint reading revert-free This changes the BondingVotes implementation to stop having so many reverts on supposedly invalid states. Instead, redefine the voting power (and checkpointed stake) as being zero before the first round to be checkpointed. This had other implications in the code like removing changes in BondingManager to make the state complete (always having an earnings pool on lastClaimRound). Instead, the BondingVotes implementaiton is resilient to that as well and just assumes reward() had never been called. This also fixed the redundant reverts between BondingVotes and SortedArrays, as now there are almost no reverts. * bonding: Address minor code review comments * treasury: Migrate to the new BondingVotes contract * treasury: Address PR comments * bonding: Move constructor to after modifiers Just for consistency with other contracts. Some docs as a bonus * test/mocks: Remove mock functions that moved to other mock * bonding: Implement ERC20 metadata on votes This is to increase compatibility with some tools out there like Tally, which require only these functions from the ERC20 spec, not the full implementation. So we can have these from the get-go to make things easier if we want to make something with them. * bonding: Address PR comments * bonding: Address BondingVotes review comments * treasury: Merge BondingCheckpoints and nits --------- Co-authored-by: Chase Adams <[email protected]>
victorges
added a commit
that referenced
this pull request
Oct 11, 2023
* package.json: Update @OpenZeppelin libraries Will be required for new checkpoints code and later governor implementation. * bonding: Create SortedArrays library Used for checkpointing logic in bonding state checkpoints * bonding: Create BondingCheckpoints contract Handles historic checkpointing ("snapshotting") and lookup of the bonding state. * bonding: Checkpoint bonding state on changes * test/bonding: Test BondingManager and Checkpoints - unit tests - integration tests - gas-report - fork test for upgrade * bonding: Migrate to custom error types * bonding: Allow querying unbonded+uncheckpointed accounts This will provide a cleaner experience using governance tools, since users that don't have any stake won't get errors when querying their voting power. For users that do have stake, we will make sure to checkpoint them on first deploy. * treasury: Create treasury governance contracts * test/treasury: Add unit test fro BondingCheckpointsVotes * test/treasury: Test GovernorCountingOverridable * test/treasury: Test LivepeerGovernor * test/treasury: A couple additional Governor tests 100% coverage 😎 * test/treasury: Rename Counting unit test mock "Harness" seems to make more sense, I could only think of that now. * Apply suggestions from code review Co-authored-by: Chase Adams <[email protected]> * treasury: Fix storage layout situation * treasury: Move governor initial params to configs * bonding: Make sure we checkpoint up to once per op * bonding: Make bonding checkpoints implement IVotes * bonding: Read votes from the end of the round Meaning the start of next round instead of the current round. This is more compatible with the way OZ expects the timepoints to work in the clock and snapshots on the Governor framework. * bonding: Make checkpoint reading revert-free This changes the BondingVotes implementation to stop having so many reverts on supposedly invalid states. Instead, redefine the voting power (and checkpointed stake) as being zero before the first round to be checkpointed. This had other implications in the code like removing changes in BondingManager to make the state complete (always having an earnings pool on lastClaimRound). Instead, the BondingVotes implementaiton is resilient to that as well and just assumes reward() had never been called. This also fixed the redundant reverts between BondingVotes and SortedArrays, as now there are almost no reverts. * bonding: Address minor code review comments * treasury: Migrate to the new BondingVotes contract * treasury: Address PR comments * bonding: Move constructor to after modifiers Just for consistency with other contracts. Some docs as a bonus * test/mocks: Remove mock functions that moved to other mock * bonding: Implement ERC20 metadata on votes This is to increase compatibility with some tools out there like Tally, which require only these functions from the ERC20 spec, not the full implementation. So we can have these from the get-go to make things easier if we want to make something with them. * bonding: Address PR comments * bonding: Address BondingVotes review comments * treasury: Merge BondingCheckpoints and nits --------- Co-authored-by: Chase Adams <[email protected]>
victorges
added a commit
that referenced
this pull request
Oct 11, 2023
* package.json: Update @OpenZeppelin libraries Will be required for new checkpoints code and later governor implementation. * bonding: Create SortedArrays library Used for checkpointing logic in bonding state checkpoints * bonding: Create BondingCheckpoints contract Handles historic checkpointing ("snapshotting") and lookup of the bonding state. * bonding: Checkpoint bonding state on changes * test/bonding: Test BondingManager and Checkpoints - unit tests - integration tests - gas-report - fork test for upgrade * bonding: Migrate to custom error types * bonding: Allow querying unbonded+uncheckpointed accounts This will provide a cleaner experience using governance tools, since users that don't have any stake won't get errors when querying their voting power. For users that do have stake, we will make sure to checkpoint them on first deploy. * treasury: Create treasury governance contracts * test/treasury: Add unit test fro BondingCheckpointsVotes * test/treasury: Test GovernorCountingOverridable * test/treasury: Test LivepeerGovernor * test/treasury: A couple additional Governor tests 100% coverage 😎 * test/treasury: Rename Counting unit test mock "Harness" seems to make more sense, I could only think of that now. * Apply suggestions from code review Co-authored-by: Chase Adams <[email protected]> * treasury: Fix storage layout situation * treasury: Move governor initial params to configs * bonding: Make sure we checkpoint up to once per op * bonding: Make bonding checkpoints implement IVotes * bonding: Read votes from the end of the round Meaning the start of next round instead of the current round. This is more compatible with the way OZ expects the timepoints to work in the clock and snapshots on the Governor framework. * bonding: Make checkpoint reading revert-free This changes the BondingVotes implementation to stop having so many reverts on supposedly invalid states. Instead, redefine the voting power (and checkpointed stake) as being zero before the first round to be checkpointed. This had other implications in the code like removing changes in BondingManager to make the state complete (always having an earnings pool on lastClaimRound). Instead, the BondingVotes implementaiton is resilient to that as well and just assumes reward() had never been called. This also fixed the redundant reverts between BondingVotes and SortedArrays, as now there are almost no reverts. * bonding: Address minor code review comments * treasury: Migrate to the new BondingVotes contract * treasury: Address PR comments * bonding: Move constructor to after modifiers Just for consistency with other contracts. Some docs as a bonus * test/mocks: Remove mock functions that moved to other mock * bonding: Implement ERC20 metadata on votes This is to increase compatibility with some tools out there like Tally, which require only these functions from the ERC20 spec, not the full implementation. So we can have these from the get-go to make things easier if we want to make something with them. * bonding: Address PR comments * bonding: Address BondingVotes review comments * treasury: Merge BondingCheckpoints and nits --------- Co-authored-by: Chase Adams <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this pull request do? Explain your changes. (required)
This is the implementation for the LivepeerGovernor specified in LIP-89.
It depends on the
BondingVotescontract implementation from #614.The higher level output of this PR is a
LivepeerGovernorcontract compatible withthe OZ Governor framework, whose participants bonding power is derived from their
staked LPT in the same way as LIP-16).
Specific updates (required)
BondingVotes.GovernorCountingOverridableLivepeerGovernorincl deployment scriptsHow did you test each of these updates (required)
yarn testall the old and new testsDoes this pull request close any open issues?
Implements PRO-31
Checklist:
yarn testpass