ERC20Votes, Liquidity Pools, and dead address

Hello everyone!

This is my first post, and I hope this message finds you well. :heart: :partying_face:

I've recently been exploring the dynamics of DAOs and the ERC20Votes module and I have a few questions and concerns that I'd like to clarification for.

  1. Delegation of Voting Rights During Token Transfers: From what I understand, when ERC20 tokens, that have ERC20Votes module are transferred to another address, the associated delegation of voting rights ceases to apply. However, I'm curious about the implications of transferring tokens to a liquidity pool. Does providing liquidity to a liquidity pool effectively delegate the associated voting rights to the liquidity pool contract itself? In other words, if I provide liquidity to a pool, am I relinquishing my ability to actively participate in governance votes?
  2. Maximizing Active Votes: My goal is to ensure that the maximum amount of "active" votes is available to participate in governance decisions. Given this objective, what strategies can be employed to achieve this while also utilizing liquidity pools effectively?
  3. Dead Address: Similarly to liquidity pools, I'm wondering about the impact of tokens that are "burnt" instead of really, really burnt (tokens that cease to exist, disappear from max supply)... Tokens held in dead addresses (specifically 0x000000000000000000000000000000000000dEaD) on governance votes. Is there a way to make them not count for the total supply?

I would greatly appreciate any insights, experiences, or best practices that the community can offer on these topics. My aim is to better understand how ERC20Votes tokens interact with liquidity pools and to explore ways to optimize active participation in governance processes.

Thank you in advance for your assistance and contributions to this discussion.

Best regards,
Glow Worm

Yes.

It really depends who asks the question. If you are a token holder, and you want to put your assets in a Defi protocole, there is really not much you can do. If you are a protocole developper, you could design a system where the liquidity pool delegates its voting power to a third party contract of you design, that would aggregate the intent of the liquidity provider and submit a vote accordingly.

By default, accounts (addresses) do not delegate their assets. When delegation is not set, no-one can use them to vote. Not even the owner. If you want to vote with your token, you first have to delegate to yourself.

Ideally we would have a totalDelegatedSupply(), that would provide details about the amount of tokens that are able to vote at a given moment. For some reason I cannot remember, we decided to not implement that, and to provide the getPastTotalSupply function instead, which does no account for locked tokens.

You may want to implement totalDelegatedSupply() yourself. It should not be too difficult, and we can provide quidance. It would add a cost to the users however.

1 Like

@ernestognw, If someone wanted to add totalDelegatedSupply(uint256 timepoint), I believe the best way to do that would be to override Votes._moveDelegateVotes and do

Checkpoints.Trace208 private _totalDelegatedCheckpoints;

function getPastTotalDelegatedSupply(uint256 timepoint) public view virtual returns (uint256) {
    uint48 currentTimepoint = clock();
    if (timepoint >= currentTimepoint) {
        revert ERC5805FutureLookup(timepoint, currentTimepoint);
    }
    return _totalDelegatedCheckpoints.upperLookupRecent(SafeCast.toUint48(timepoint));
}

function _moveDelegateVotes(address from, address to, uint256 amount) internal virtual override {
    if (from == address(0) && to != address(0)) {
        _push(_totalDelegatedCheckpoints, _add, SafeCast.toUint208(amount));
    } else if (from != address(0) && to == address(0)) {
        _push(_totalDelegatedCheckpoints, _substract, SafeCast.toUint208(amount));
    }
    super._moveDelegateVotes(from, to, amount);
}

However, this is not possible because _moveDelegateVotes is private. We would need to make it internal virtual.

2 Likes

EDIT: :warning: The following code does not track delegation created indirectly by transferring tokens to an address who has previously delegated to another address

This is one of those cases where making sure of calling super is critical :sweat_smile:

Seems like _moveDelegateVotes is only used in _delegate and _transferVotingUnits and such separation makes sense depending on what you want to track in my opinion. Here, it's possible to track only with _delegate:

Checkpoints.Trace208 private _totalDelegatedCheckpoints;

function getPastTotalDelegatedSupply(uint256 timepoint) public view virtual returns (uint256) {
    uint48 currentTimepoint = clock();
    if (timepoint >= currentTimepoint) {
        revert ERC5805FutureLookup(timepoint, currentTimepoint);
    }
    return _totalDelegatedCheckpoints.upperLookupRecent(SafeCast.toUint48(timepoint));
}

function _delegate(address account, address delegatee) internal virtual override {
    address oldDelegate = delegates(account);
    if (oldDelegate == address(0) && to != address(0)) {
        _push(_totalDelegatedCheckpoints, _add, SafeCast.toUint208(amount));
    } else if (oldDelegate != address(0) && to == address(0)) {
        _push(_totalDelegatedCheckpoints, _substract, SafeCast.toUint208(amount));
    }
    super._delegate(account, delegatee);
}

Though, it requires an extra sload. I would say that's fine if delegations are not a very frequent operation but I'm not sure how to measure that.

2 Likes

Hello friends! :star2:

Thank you both for putting so much effort into the replies, you are being very kind and helpful, and I really appreciate it! :heart:

I am indeed working on my own liquidity pools to delegate the votes instead of locking them, as I want to maximize liquidity and vote participation!

I'm very new to Solidity, may I ask what would be considered best practices to implement your code snippet? I'm tempted in directly modifying the OpenZeppelin templates, but I have a feeling it would be considered a sacrilege by auditors :innocent:

Since this functionality doesn't yet exist in the standard, would it be better to override / extend Votes.sol , or is it better to just put it in the file directly?


I believe I understand the intent here: in the end, we want to have a higher and more effective participation ratio by dividing the number of delegated votes by the total delegated supply, instead of the total supply (which includes dead addresses, etc)... is that correct?

Assuming I'm on the right path, I would have to search for references of the function getPastTotalSupply and replace it to getPastTotalDelegatedSupply.

I believe it changes depending on each DAO, but I'd basically have to modify this line in GovernorVotesQuorumFraction.sol, right?

    /**
     * @dev Returns the quorum for a timepoint, in terms of number of votes: `supply * numerator / denominator`.
     */
    function quorum(uint256 timepoint) public view virtual override returns (uint256) {
        return (token().getPastTotalSupply(timepoint) * quorumNumerator(timepoint)) / quorumDenominator();
    }

I believe I see the light at the end of the tunnel!
Thank you! :heart:

You're very welcome!

In regards to our recommendation to implement the code we're suggesting, I would you to first take a look to our "Extending Contracts" section in the documentation. You're not expected to copy, paste and modify the contracts as it may have huge security implications! Contracts should always be inherited.

would it be better to override / extend Votes.sol , or is it better to just put it in the file directly?

Given my previous comments, please always override / extend.

The snippet from @Amxx requires modifying the contracts directly because you can't override the _moveDelegateVotes function since it's private. This is why we're discussing if it would make sense to allow overriding by making the function internal instead.

However, you can use the alternative I shared although it's a bit more expensive in comparison, just note that it's not audited code and we suggest making an audit of such changes before going to production.

I believe it changes depending on each DAO, but I'd basically have to modify this line in GovernorVotesQuorumFraction.sol , right?

Yes, you got it right! You're encouraged to override the quorum function, just make sure you won't end up in a locked state where quorum is unreachable :sweat_smile:

1 Like

Thank you very much for your in depth explanations. I no longer have any concerns regarding the implementation of your provided code, and you've just taught me how to write better solidity! :partying_face:

Thank you again!
Kind regards :heart: :star2:

No. You would also have to override transferVotingUnits for cases where either

  • from as a delegate, but to doesn't have one
  • from does not have a delegate, and to does have one.

That is actually the whole point of the issue here: when moving token from a user that has delegation set, to a liquidity pool that doesn't have delegation, you want the value to be updated.

That override is possible , but as you proved its easy to miss, and implementing it would add 2 sloads .

    function _transferVotingUnits(address from, address to, uint256 amount) internal virtual override {
        super._transferVotingUnits(from, to, amount);
        address fromDelegate = delegates(from);
        address toDelegate = delegates(to);
        if (fromDelegate == address(0) && toDelegate != address(0))
            _push(_totalDelegatedCheckpoints, _add, SafeCast.toUint208(amount));
        else if (fromDelegate != address(0) && toDelegate == address(0))
            _push(_totalDelegatedCheckpoints, _substract, SafeCast.toUint208(amount));
    }

So more complexity, more cost, more chances of doing something wrong ...

1 Like

You're totally right. I updated my previous comment with a warning.

So to summarize, considering Alice, Bob and Charlie, if Alice has previously delegated to Bob, then a transfer of X from Charlie to Bob will indirectly add X tokens to the total delegated amount without going through the _delegate function.

Let's make it internal then, I agree this is an honest use case.

EDIT: Here's the PR

1 Like