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.
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:
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.
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.
The transaction sets the DAO proxy implementation to be the fixed version deployed at 0xe3caa436461DBa00CFBE1749148C9fa7FA1F5344.
Thanks,
the verbs ⌐◨-◨
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.
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:
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.
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.
The transaction sets the DAO proxy implementation to be the fixed version deployed at 0xe3caa436461DBa00CFBE1749148C9fa7FA1F5344.
Thanks,
the verbs ⌐◨-◨
🥌 x FOR ↳ benbodhi.eth
Throwing weight behind zero weight votes with reason.
Took a look at the code, fix is indeed minor. Good change for multisigs and other smart contract wallets.
For other voters, don't take this as an audit -- there are many other steps involved that I didn't do, like checking the logic of setImplementation() and diffing the deployed versions on Etherscan -- but do take it as confirmation that the changes are small and wouldn't normally be audited.
sent from voter.wtf
https://www.lilnouns.wtf/vote/nounsdao/443/votes
FOR 85 VOTES 0xae4705dC0816ee6d8a13F1C72780Ec5021915Fed | "thanks index"
AGAINST 0 VOTES
ABSTAINS 99 VOTES
0x6CF8568007eD13AE79d4CCD138a58F5Fa06F582 | "I am not a dev and think protocol changes should be audited, but they say its not a major change, prefer to abstain " 0x2eE0485f71764bcD2062A84d9455688c581B90f8 | "" gif of lizard laughing "" 0x9e0e9D25a5ED9bc773f91691f0b45599255257B1 | "I'll only vote on proposals that require explanations once the proposer attends our weekly calls to explain them."
FOR - 34 VOTES
benbodhi | "Verbs - the doing words. Thanks for doing this."
AGAINST - 0 VOTES
ABSTAINS - 2 VOTES