Extract Checkpointing functionality from ERC20Votes

[cygnusv]

🧐 Motivation Currently Governor assumes the underlying voting source is an ERC20 token, which is fine in most cases. However, it would useful that other types of voting sources are allowed.

For example, current NuCypher DAO (which uses Aragon), only works for stakers (not liquid holders), so all the checkpointing logic occurs in the staking contract, and not in the ERC20 token. We're currently working on a new DAO for the Threshold Network (which comprises both NuCypher and KEEP networks) and we plan to use Governor Bravo. Our idea is that governance will not be based solely on ERC20 tokens, but also on staked tokens, which means the same checkpointing logic will be used both in the token and staking contracts.

Furthermore, we plan to combine both approaches (i.e., token-based and staking-based voting sources), so an aggregation layer would be ideal (we have experience with Aragon's VotingAggregator in current NuCypher DAO, and it has been working great).

I'm already working on this, and I personally believe it would be great for these improvements to go upstream.

📝 Details

• Extract checkpointing logic from ERC20Votes.sol, and place it in a library/abstract contract.
• Create an aggregation layer that's capable to collect votes from several sources, and aggregate them according to some logic (e.g., weighted average).
• Define new interfaces that improve composability of Governor Bravo (particularly wrt to integration of voting sources)

[Amxx]

Hello @cygnusv

Currently, the governor does NOT assume the underlying voting source is an ERC20 token. This logic is implemented by the "Votes" modules, which currently comes in 3 different flavors:

• GovernorVotes: for when your voting source is an ERC20Votes
• GovernorVotesQuorumFraction: for when your voting source is an ERC20Votes and you want your quorum to be a fraction of the total supply
• GovernorVotesComp: for when your voting source is a Comp fork

If you want to vote using something else (NFTs, sum of the balances of 2 tokens, proof of humanity, ...) you "just" have to write your own "Votes" module for the governor.

To do so:

• create a new contract that inherits "@openzeppelin/contracts/governance/Governor.sol"
• add other modules as you need them (timelock, counting, ...)
• add a function getVotes(address account, uint256 blockNumber) public view virtual override returns (uint256) function with your custom logic

And you should be good to go!

[ivanminutillo]

Hi @Amxx I'd like to figure out a basic governance system using an ERC721 token. From your summary (but I am way naive in the solidity realm) to have a duplicate of the actual governor that use NFTs, it is mostly about duplicating existing contracts and replacing erc20 with erc721 ?

Basically:

• Create a GovernorERC721Votes.sol that will be added to the existing flavors

  
  // SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import "../Governor.sol"; import "../../token/ERC721/extensions/ERC721Votes.sol"; import "../../utils/math/Math.sol";

/**

• @dev Extension of {Governor} for voting weight extraction from an {ERC721Votes} token.

• Available since v4.3. */ abstract contract GovernorERC721Votes is Governor { ERC721Votes public immutable token;

  constructor(ERC721Votes tokenAddress) { token = tokenAddress; }

  /**

  • Read the voting weight from the token's built in snapshot mechanism (see {IGovernor-getVotes}). */ function getVotes(address account, uint256 blockNumber) public view virtual override returns (uint256) { return token.getPastVotes(account, blockNumber); } }

    
    - Clone `ERC20Votes.sol` as `ERC721Votes.sol` extension (that inherit its own `draft-ERC721Permit.sol` contract to enable signatures for delegating votes)

  Does it make sense at all? ps. Do you think it's better to move it on a dedicate issue? Thasks for your work :)

[Amxx]

This sounds like a good plan of action !

[ivanminutillo]

Great, I see here https://eips.ethereum.org/EIPS/eip-2612 though, that the Permit implementation is bound to erc20 tokens... Do you think it's fair enough to use the same implementation for ERC721 tokens?

EDIT: I just discovered your implementation of ERC721Permit :1st_place_medal: XD

[cygnusv]

Thanks for the clarification, @Amxx! I have a clearer picture now. Going back to the original issue, I see that there's a lot of reuse potential if the checkpointing functionality is extracted from ERC20Votes; same logic could be plugged in to another type of contract (e.g, staking, liquidity pools, ERC721, etc) without copy-pasting or re-implementing again.

I have a proof of concept of such extracted logic here. Do you think this is something that could be interesting to include upstream?

[frangio]

Fixed in https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2944. There is now a Checkpoints library and a Votes contract.