TLDR

• We deployed V3 with the same Vote with Refund bug we fixed in V2.1.
• This proposal upgrades the DAO with a minor fix.
• This version is not audited; we’re confident enough with the internal review we conducted.

Fix summary

Change the refund destination such that multisig voters that pay for gas get refunded, rather than sending the refund to the multisig contract. Technically, this change involves refunding tx.origin instead of msg.sender.

Exact changes can be seen in this pull request.

Because this change is very minor, we're not going through an official audit process. This change has been reviewed by solimander.

Why this happened

We started work on V3 before the V2.1 fix, and V3 is in different files so syncing the master branch into the V3 branch did not apply the fix to V3 code.

Here’s a short sequence of events:

1. September 22, 2022 [v3 branch]: the branch that later becomes DAO V3 is created with initial code experiments.
2. October 25, 2022 [dao 2.1 branch]: refund bug fixed.
3. November 2, 2022 [v3 branch]: we sync from master and create a new DAO V3 file.
4. November 9, 2022 [master] refund fix merged into master, to be deployed.
5. January 4, 2023 [v3 branch] we sync from master, the refund fix is updated in the DAO V2 file, but we are focused on V3 and do not examine the merge carefully and miss the opportunity to copy the fix into V3.
6. January 11, 2023 [v3 branch] we split DAO V3 into library files to overcome the contract size limit; this change makes the bug harder to discover, as we lose the ability to directly compare V3 to V2 line for line, as we still could have done between V2 and V1.

Another problem was that we did not copy or change all V1/V2 tests to test V3; most of the tests were copied to run on V3 as well, but we dropped the ball on the gas refund test.

How we’re preventing it from happening again

1. Make sure tests are always running against the latest version.
2. When merging a fix to master, make sure to correctly merge it into any open branch.

Running all tests on the latest version is the most important takeaway. As part of working on this fix we made sure all DAO tests are now running on the latest version, and removed old duplicate tests that were testing older versions. We believe we’ve reduced the chances of this happening again significantly.

The second point is helpful as an extra layer of safety, mostly helpful if we don’t have sufficient tests or failed to run all tests against the latest version we’re working on. We will certainly be more mindful in future merges.

Transaction

The transaction sets the DAO proxy implementation to be the fixed version deployed at 0xe3caa436461DBa00CFBE1749148C9fa7FA1F5344.

Thanks,

the verbs ⌐◨-◨