[GCP-009] - Upgrading Gitcoin’s Governance Contracts

:grinning: New Governor contract (out of the box Open Zeppelin probably good enough)
New ERC20Votes contract with minting for GTC functionality (out of the box OZ with minor modifications probably good enough)

Thanks for sharing. I understand that a new token contract can be deployed using these steps, but that will not recover non-native tokens from the treasury (stables/eth etc…). I’d suggest moving non-native tokens sufficient to cover these costs and some buffer into a separate multi-sig

1 Like

Say more here? Why would we need to move these from the treasury?

The downside of this upgrade has been flagged as a bottleneck by me and a few others such as @ale.k @QuadraticLander

Since there is no way to install an ‘escape hatch’ incase of

and the suggested solution

Afaik, this cannot be done for nonGTC tokens. If this means that 35% of the treasury value will be inaccessible/permanently lost, then funds on a backup address will help reducing the damage.

I think your proposal is worth a discussion @jengajojo, but something to remember is that moving the tokens is not a risk free proposition either. Where do they get moved to? Who has the ability to send them back. And all of that has to be done without error as well.

I agree with your comments. Maybe a multi-sig with one member each from CSDO, Steward Council, foundation and yourself could be a starting point before diving deeper into governance around the backup address. As long as members on that multi-sig can be trusted, those backup funds are relatively safer in case the main treasury becomes inaccessible.

hmm… this seems like a slippery slope. Why not just empty the treasury to that gnosis safe, then move it back after the upgrade? Im not sure where we draw the line.

1 Like

If the DAO is interested in moving some or all of the non-GTC funds out of the treasury before the update, we can facilitate this as part of the upgrade itself.

Onchain proposals can contain multiple actions that execute in sequence. We can add actions to the upgrade proposal which transfer any amount of any tokens to the desired address before upgrading the Governor.

In the very unlikely event the upgrade breaks governance, the tokens will still be available. This change can be made in the proposal script and we can add simulations to test it as well.

If the DAO would like to go this route, there are two questions which will have to be answered:

  1. How much of the USDC and RAD tokens held by the timelock should be moved.
  2. Where should the tokens be moved to, e.g. a multisig held by DAO leaders, etc…

If the community would be more comfortable with the upgrade given these changes we are happy to facilitate them. I’m curious to hear what folks like @kyle, @shawn16400, @jengajojo, and others in the community think about this option. Thanks!

1 Like

@bendi
In a past life, we would do a risk assessment with each project that would list out realistic risks associated to a project. This would be risks to the project, and risk because of the project (unintended consequences).
Each risk is then weighted according to the vectors of “likelihood” and “consequences”. In this example the likelihood might be considered rare, unlikely, or even unknown. However the consequence of bricking the treasury would rank up there with major/catastrophic.

If Gitcoin followed this methodology, we have to put in a risk mitigation plan in place to protect against the occurrence. Typically I would have done this via phasing in the changes to lower risk components (move change to a sub-treasury), building emergency inventory (moving tokens to a back up wallet), or building a back-up site (new & functional copy of our existing treasury) that is ready for execution.

Net - we really should put a plan in place.

3 Likes

Hey Shawn, I love this framework for thinking about the risks and agree with where you’ve benchmarked it. Very unlikely/rare, but very negative consequences were it to happen.

I’m curious what concrete plan you think ought to be in place beyond moving some of the tokens out of the treasury before the upgrade? I.e. can you flesh out what you mean here, and how what’s suggested is different than simply moving the tokens?

Typically I would have done this via phasing in the changes to lower risk components (move change to a sub-treasury), building emergency inventory (moving tokens to a back up wallet), or building a back-up site (new & functional copy of our existing treasury) that is ready for execution

As I mentioned, I think moving the tokens is a reasonable step to consider, and it can be done (without additional risk) as part of the upgrade proposal itself. We’re happy to execute on this, we just need to decide how much of the tokens to move and where to send them. If you have concrete additional suggestions for risk mitigation we’re also happy to discuss!

@bendi thanks for all you and @ScopeLift have done to upgrade governance for the network. During Tally Delegation Week I heard Scopelift mentioned in two different twitterspaces so these changes are indeed anticipated. And to note, that once one or two protocols successfully perform this move without incident, I think migration becomes easier.

In the old world (if I did not have a QA environment), for a change with this potential impact I would have taken one of two approaches:

  1. Upgrade a stand alone (back-up) system which has the same configuration as the target system. This means move the changes to a mirror copy and (depending on interfaces), run a suite of business cases against the back up to ensure nothing broke. If nothing breaks after testing, move the changes into the target system.
  2. Move the entire upgrade to a new environment and migrate “business” over to the new environment. Basically do step 1, but instead of moving the changes over to the old system after testing the backup, bring the treasury over to the new system in series of increasing stages based on demonstrated use.

Now part of the reason DAOs run like crap are sometimes slow is because people with little knowledge get to weigh in on things they may not know all that much about. Sometimes the outside perspective helps, and sometimes it just gums things up. Not sure which case this is, but if it looks like help to you, I am happy to jump on a call and work through it. :slight_smile:

2 Likes

Thanks @bendi

kyle brings a good point here

if this is indeed a

risk, then @kyle 's line of thinking doesn’t sound so bad. I would like to hear what the other members of the stewards council have to say in this matter.

Hey Shawn, no worries at all. These are good things to talk through to make sure they’re clear. If I understand what you’re saying correctly, it basically sounds like what you’re recommending is that the upgrade be done in incremental steps instead of all at once. Is that basically right?

Unfortunately, because of the way the existing contracts are designed, you can’t put one system in place and slowly migrate over to it. Here’s why, in (somewhat) more technical detail:

The treasury is held by a contract called a Timelock, which executes the onchain proposals after a delay. The Timelock has an administrator that can queue the transactions to execute through it. The Governor contract is the administrator of the Timelock. There can only be one administrator at a time. Only the existing administrator can update it.

This means there must be a proposal where the existing Governor transfers the Timelock’s administrator role over to the new Governor. It must be done in one step. And in the unlikely event that something went wrong with that step, such that the new Governor could not properly queue transactions to the Timelock, the funds there would be locked.

That’s why we wrote all the tests as part of this process. These tests effectively simulate the upgrade process in an environment that mimics the network state of mainnet.

I’ll let @kyle comment, but I think he meant this as an example of how we could go too far in attempting mitigations.

As I said, I do think moving some proportion of the funds out of the treasury and into some other wallet is a reasonable disaster mitigation plan. We can incorporate this into the proposal itself and test the transfer out.

That said, it’s important to remember that the mitigation itself is not risk free, may actually be riskier than the upgrade. What is more likely: the carefully planned and extensively tested/simulated upgrade process fails? Or a well intentioned multi-sig member fat-fingering the address in the Safe UI when they go to transfer the funds back?

This is why my personal recommendation would be to only move some percentage of the funds out. I think you’d want to transfer enough to allow the DAO to continue to function and recover in the very unlikely event that something went wrong, but also an amount small enough that the DAO could afford to lose it should something go wrong moving the funds back.

1 Like

Hey all, I wanted to share that we just disclosed and patched a bug with Flexible Voting. The bug was found during an audit of Frax Finance’s governance contracts, which use Flexible Voting. PoolTogether has also funded ScopeLift to execute their upgrade to an Flexible Voting Governor.

It’s important to note that this bug was an edge case, would not have put treasury funds at risk, and would not have impacted Gitcoin directly, as signature based voting is not (currently) leveraged by the DAO.

All that said, the fact this bug was discovered does emphasize the very valid concerns the community has had around security and safety of funds.

Next week, we will deploy a new candidate Governor—with the fix included—for Gitcoin to consider, and update the repository to reflect this change. We hope the community can come to some consensus soon on a path forward, in particular on:

  • How much of the treasury funds to move out before executing the upgrade and…
  • Where to move the backup funds to
2 Likes

Thanks, Ben for the update!

When are the the PoolTogether contract upgrades going live?

1 Like

Thanks for the update @bendi

Considering the fact that bugs are being found on an ongoing basis, I suggest we move all non-native tokens to another address. The reason being that non-native tokens cannot be recovered but a new contract can be deployed for native tokens

Assuming that these responsibilities still hold true for CSDO CSDO Charter V1 I suggest them to make a separate address to hold these funds. Alternatively a time dependant contract can escrow but I am not sure if it’s worth the operational effort for you all.

In either case, I am happy to add my feedback on the draft proposal if you want. Thanks once again @bendi

Hey Kyle, we’re working on the engineering side of this now. Realistically I think a proposal for PoolTogether could be live in 4-5 weeks.

I understand this reaction, and if the Gitcoin community decides to move 100% of non-native funds, we are happy to support it. I just want to reiterate a few facts:

  1. The bug in question would not have resulted in lost or locked funds
  2. The contracts have now gone through two in-depth audits
  3. Moving of the funds is not a risk free endeavor, as they have to be sent back to the DAO by the entity that holds them, such as the CSDO as you are suggesting, and errors can happen in that process as well.

With regards to the CSDO as a suggested entity to custody the funds, I have a couple of quick practical questions:

  1. How many people are on the CSDO
  2. Does the CSDO currently have a multisig it operates on mainnet, and how is that multisig configured if so? What is its address?

Thanks for the suggestion! Hoping we can move this forward and come to a consensus soon :slight_smile:

1 Like

Hey all - I want to bump this thread again for everyone.

We recently withdrew $3MM USDC from the treasury to support the workstreams and moved that into the gnosis safe.

I wold love to revive these conversations and move this to a vote on snapshot. If the snapshot vote passes, I will work with ben to ensure we test this together a few times prior to que`ing up the Tally vote.

Hopefully folks are interested in supporting the upgrade!

1 Like

Thanks for moving this forward Kyle. As always, happy to answer any questions anyone might have!

We’re gratified to see the snapshot vote on the upgrade is going well. As promised, the updated candidate Governor has been deployed:

2 Likes