Skip to content

Commit

Permalink
treasury: Delta Governor v1 (#615)
Browse files Browse the repository at this point in the history
* 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]>
  • Loading branch information
victorges and 0xcadams authored Aug 25, 2023
1 parent 4d720f0 commit ad48642
Show file tree
Hide file tree
Showing 15 changed files with 1,760 additions and 6 deletions.
51 changes: 51 additions & 0 deletions contracts/test/mocks/GovenorInterfacesFixture.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.9;

import "@openzeppelin/contracts-upgradeable/governance/IGovernorUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/governance/extensions/IGovernorTimelockUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/token/ERC1155/IERC1155ReceiverUpgradeable.sol";

/**
* @dev This is a helper contract to return the expected interface values that the LivepeerGovenor interface should
* support. This only exists in Solidity since generating these interfaces in JS is kinda of a pain.
*/
contract GovernorInterfacesFixture {
function TimelockUpgradeableInterface() external pure returns (bytes4) {
return type(IGovernorTimelockUpgradeable).interfaceId;
}

/**
* @dev ID calculation logic copied from {GovernorUpgradeable-supportsInterface}.
*/
function GovernorInterfaces() external pure returns (bytes4[] memory) {
IGovernorUpgradeable governor;
// <begin of copy, replacing `this` with `governor`>
bytes4 governorCancelId = governor.cancel.selector ^ governor.proposalProposer.selector;

bytes4 governorParamsId = governor.castVoteWithReasonAndParams.selector ^
governor.castVoteWithReasonAndParamsBySig.selector ^
governor.getVotesWithParams.selector;

// The original interface id in v4.3.
bytes4 governor43Id = type(IGovernorUpgradeable).interfaceId ^
type(IERC6372Upgradeable).interfaceId ^
governorCancelId ^
governorParamsId;

// An updated interface id in v4.6, with params added.
bytes4 governor46Id = type(IGovernorUpgradeable).interfaceId ^
type(IERC6372Upgradeable).interfaceId ^
governorCancelId;

// For the updated interface id in v4.9, we use governorCancelId directly.
// </end of copy>

// replace the interface checks with return the expected interface ids
bytes4[] memory ids = new bytes4[](4);
ids[0] = governor43Id;
ids[1] = governor46Id;
ids[2] = governorCancelId;
ids[3] = type(IERC1155ReceiverUpgradeable).interfaceId;
return ids;
}
}
74 changes: 74 additions & 0 deletions contracts/test/mocks/GovernorCountingOverridableHarness.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.9;

import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import "@openzeppelin/contracts-upgradeable/governance/GovernorUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/governance/extensions/GovernorVotesUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/governance/extensions/GovernorVotesQuorumFractionUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/governance/extensions/GovernorSettingsUpgradeable.sol";

import "../../treasury/GovernorCountingOverridable.sol";

/**
* @dev This is a concrete contract to test the GovernorCountingOverridable extension. It implements the minimum
* necessary to get a working Governor to test the extension.
*/
contract GovernorCountingOverridableHarness is
Initializable,
GovernorUpgradeable,
GovernorSettingsUpgradeable,
GovernorVotesUpgradeable,
GovernorCountingOverridable
{
// use non-standard values for these to test if it's really used
uint256 constant QUOTA = 420000; // 42%
uint256 constant QUORUM = 370000; // 37%

IVotes internal iVotes; // 🍎

function initialize(IVotes _votes) public initializer {
iVotes = _votes;

__Governor_init("GovernorCountingOverridableConcrete");
__GovernorSettings_init(
0, /* no voting delay */
100, /* 100 blocks voting period */
0 /* no minimum proposal threshold */
);

__GovernorVotes_init(iVotes);
__GovernorCountingOverridable_init(QUOTA);
}

function votes() public view override returns (IVotes) {
return iVotes;
}

function quorum(uint256 timepoint) public view virtual override returns (uint256) {
uint256 totalSupply = iVotes.getPastTotalSupply(timepoint);
return MathUtils.percOf(totalSupply, QUORUM);
}

/**
* @dev Expose internal _quorumReached function for testing.
*/
function quorumReached(uint256 proposalId) public view returns (bool) {
return super._quorumReached(proposalId);
}

/**
* @dev Expose internal _voteSucceeded function for testing.
*/
function voteSucceeded(uint256 proposalId) public view returns (bool) {
return super._voteSucceeded(proposalId);
}

function proposalThreshold()
public
view
override(GovernorUpgradeable, GovernorSettingsUpgradeable)
returns (uint256)
{
return super.proposalThreshold();
}
}
14 changes: 14 additions & 0 deletions contracts/test/mocks/LivepeerGovernorUpgradeMock.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.9;

import "../../treasury/LivepeerGovernor.sol";

contract LivepeerGovernorUpgradeMock is LivepeerGovernor {
uint256 public customField;

constructor(address _controller) LivepeerGovernor(_controller) {}

function setCustomField(uint256 _customField) external {
customField = _customField;
}
}
112 changes: 112 additions & 0 deletions contracts/test/mocks/VotesMock.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.9;

import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";
import "@openzeppelin/contracts-upgradeable/token/ERC20/extensions/ERC20BurnableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/token/ERC20/extensions/ERC20SnapshotUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/token/ERC20/extensions/ERC20VotesUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";

import { IVotes } from "../../treasury/GovernorCountingOverridable.sol";
import "../../bonding/libraries/SortedArrays.sol";

/**
* @dev Minimum implementation of an IVotes interface to test the GovernorCountingOverridable extension. It inherits
* from the default ERC20VotesUpgradeable implementation but overrides the voting power functions to provide power to
* delegators as well (to be made overridable by the GovernorCountingOverridable extension).
*/
contract VotesMock is
Initializable,
ERC20Upgradeable,
ERC20BurnableUpgradeable,
OwnableUpgradeable,
ERC20VotesUpgradeable,
IVotes
{
function initialize() public initializer {
__ERC20_init("VotesMock", "VTCK");
__ERC20Burnable_init();
__Ownable_init();
__ERC20Votes_init();
}

function delegatedAt(address _account, uint256 _timepoint) external view returns (address) {
_timepoint; // unused
// Blatant simplification that only works in our tests where we never change participants balance during
// proposal voting period. We check and return delegators current state instead of tracking historical values.
return delegates(_account);
}

/**
* @dev Simulates the behavior of our actual voting power, where the delegator also has voting power which can
* override their transcoder's vote. This is not the case in the OpenZeppelin implementation.
*/
function getPastVotes(address account, uint256 blockNumber)
public
view
override(IVotesUpgradeable, ERC20VotesUpgradeable)
returns (uint256)
{
// Blatant simplification that only works in our tests where we never change participants balance during
// proposal voting period. We check and return delegators current state instead of tracking historical values.
if (delegates(account) != account) {
return balanceOf(account);
}
return super.getPastVotes(account, blockNumber);
}

/**
* @dev Same as above. Still don't understand why the OZ implementation for these 2 is incompatible, with getPast*
* reverting if you query it with the current round.
*/
function getVotes(address account)
public
view
override(IVotesUpgradeable, ERC20VotesUpgradeable)
returns (uint256)
{
if (delegates(account) != account) {
return balanceOf(account);
}
return super.getVotes(account);
}

function mint(address to, uint256 amount) public onlyOwner {
_mint(to, amount);
}

// The following functions are overrides required by Solidity.

function _afterTokenTransfer(
address from,
address to,
uint256 amount
) internal override(ERC20Upgradeable, ERC20VotesUpgradeable) {
super._afterTokenTransfer(from, to, amount);
}

function _mint(address to, uint256 amount) internal override(ERC20Upgradeable, ERC20VotesUpgradeable) {
super._mint(to, amount);
}

function _burn(address account, uint256 amount) internal override(ERC20Upgradeable, ERC20VotesUpgradeable) {
super._burn(account, amount);
}

function name() public view override(IVotes, ERC20Upgradeable) returns (string memory) {
return super.name();
}

function symbol() public view override(IVotes, ERC20Upgradeable) returns (string memory) {
return super.symbol();
}

function decimals() public view override(IVotes, ERC20Upgradeable) returns (uint8) {
return super.decimals();
}

function totalSupply() public view override(IVotes, ERC20Upgradeable) returns (uint256) {
return super.totalSupply();
}
}
Loading

0 comments on commit ad48642

Please sign in to comment.