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

📜 Proposals
, ,
37

@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
38

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.

40

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
41

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
42

Thanks, Ben for the update!

When are the the PoolTogether contract upgrades going live?

1 Like
43

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

44

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
45

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
46

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

47

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
48

Vote Yes to upgrade the Governor Contract to the contracts ScopeLift has deployed

1 Like
49

The upgrade proposal is now live onchain. We have done two additional security/safety checks since the proposal was put onchain by @kyle.

First, we updated our tests & simulations to run with actual deployed proposal data, to ensure no errors were introduced when the proposal was put onchain. See results here.

Secondly, we modified Seatbelt to simulate the proposal and analyze its effects. You can see the report here.

Both checks indicate that the proposal will behave as expected, that Gitcoin’s Governor will be properly upgraded, and that the DAO’s onchain governance will continue to function as expected afterwards.

Given all this, we encourage DAO Delegates to vote in favor of the proposal. Tally | Gitcoin Proposal

Thanks again to the Gitcoin community for entrusting ScopeLift with this sensitive work. We look forward to seeing the upgrade completed and what will get built on top of it afterwards.

As always, I’m happy to answer any additional questions anyone has.

1 Like
50

Excited to see the proposal going live @bendi ! Appreciate I’m late to the party, but I was wondering if you could share the links to the audits of the GitcoinGovernor contracts? I’m probably overlooking something obvious, but can’t find them myself :sweat_smile:

3 Likes
51

Hey @garm, thanks! And no worries at all for the late question.

The final Gitcoin Governor contract is assembled from fully audited components, namely: ScopeLift’s Flexible Voting extension and the battle tested OpenZeppelin Governor implementation.

Flexible Voting was audited twice, but only one of the audits was made public, which you can see here. The code was audited a second time by Trail of Bits as part of that projects Governance system audit. We have seen the final report but it has not (yet?) been published.

As you can see by looking at the Gitcoin Governor, there is no new code—only the inheriting of audited contracts and the configuration of parameters as appropriate for Gitcoin.

Most of the work for the upgrade actually comes from the tests/simulations we wrote to ensure the upgrade would execute properly and that the new Governor contract we put in place would function as expected afterwards.

Please let me know if you have any further questions! We are also excited to see this upgrade coming to fruition.

1 Like
52

Hi all, as most of you have no doubt seen, the upgrade has now executed successfully. The new Governor is available on Tally and one new proposal has already been submitted to it. I again reiterate my thanks to the Gitcoin community for trusting ScopeLift with this work. We’re excited to see what comes next, and eager for the opportunity for continued collaboration. Congrats to Gitcoin!

2 Likes
53

Kudos on the successful upgrade. (err seemingly successful? Does the new Tally proposal have to pass and successfully execute to 100% prove it works?)

Looking forward to seeing what kind of things are built here. I’d love to wrap my mind around the possibilities enabled.

Kyle teased some possibilities here.

54

Hey Kevin, good question. We can say with high confidence that the Governor is working based on the execution of the proposal and the queuing of a new one. But you’re right. We’ll be able to say with total confidence that it worked only after the first proposal clears through it!

55

Excited to say that one proposal has now executed through the new Governor, with several others lined up to do so soon. It is now possible to say with full confidence that the upgrade has gone as expected. Once more, thanks to the Gitcoin community for entrusting us with this work!

3 Likes
56

As Ben mentioned, the Upgrade has been successful!

We are going to spend a bit of time outlining where there are opportunities to introduce novel voting schemes in the future.

Just a heads up, Tally will continue to show the details for the old governor, and now also the new governor. All voting and on-chain interaction will happen using the new page and new governor.

Let us know if you have any questions :slight_smile:

3 Likes
57

Kudos to everyone who worked on this on a successful upgrade.

2 Likes