anon/tornado-governance
1
0
Fork 0
Code Pull Requests Activity

Implement suggestion: https://hackmd.io/@mLAPku2WQZmso5oX4kzPOg/Bkn8SBKSn

Browse Source 
Signed-off-by: AlienTornadosaurusHex <>
This commit is contained in:
AlienTornadosaurusHex 2023-05-22 20:25:59 +00:00
parent 4f141a5bb6
commit 68f5be64e2
3 changed files with 7 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
contracts/v4-patch/GovernancePatchUpgrade.sol
View File

@ -9,8 +9,6 @@ import "../v3-relayer-registry/GovernanceStakingUpgrade.sol";
contract GovernancePatchUpgrade is GovernanceStakingUpgrade {
mapping(uint256 => bytes32) public proposalCodehashes;
event CodehashDifferent(address target, bytes32 oldHash, bytes32 newHash);
// The stakingRewardsAddress sets the immutable to the new staking contract
constructor(
address stakingRewardsAddress,
@ -32,18 +30,9 @@ contract GovernancePatchUpgrade is GovernanceStakingUpgrade {
proposalCodehash := extcodehash(target)
}
if (proposalCodehash == proposalCodehashes[proposalId]) {
super.execute(proposalId);
} else {
// So this should guarantee that the proposal should not be able to be executed in future
// But also, the proposal will be skipped each time. So the question is whether this, a silent
// skip, or a require can be used to stay "concise", and abandon the safety of having also this assign
// Outside dependencies, which don't exist as of yet, might believe that the proposal code was executed
proposal.executed = true;
require(proposalCodehash == proposalCodehashes[proposalId], "Governance::propose: metamorphic contracts not allowed");
// Let the event signify it was metamorphic
emit CodehashDifferent(target, proposalCodehashes[proposalId], proposalCodehash);
}
super.execute(proposalId);
}
// This should store the proposal extcodehash

3
package.json
View File

@ -17,7 +17,8 @@
"test:f": "yarn prettier:fix && yarn test",
"clean": "yarn prettier:fix && yarn lint",
"compile": "yarn prettier:fix && yarn hardhat compile",
"coverage": "yarn hardhat coverage"
"coverage": "yarn hardhat coverage",
"layout": "yarn hardhat check"
},
"author": "Tornado.cash team <hello@tornado.cash>",
"license": "MIT",

21
test/patch/patch.test.js
View File

@ -264,25 +264,10 @@ describe('Gov Exploit Patch Upgrade Tests', () => {
// Load the contract
const maliciousProposal = await ethers.getContractAt('MaliciousProposal', maliciousProposalAddress)
// Check that the malicious proposer is the deployer
const deployer = await maliciousProposal.deployer()
// Get bal before
const deployerBalanceBefore = await torn.balanceOf(deployer)
const governanceBalanceBefore = await torn.balanceOf(governance.address)
expect(governanceBalanceBefore).to.be.gt(zero)
// Now execute
await governance.execute(proposalId)
// Check bal after
const deployerBalanceAfter = await torn.balanceOf(deployer)
const governanceBalanceAfter = await torn.balanceOf(governance.address)
// Protected
expect(deployerBalanceAfter).to.equal(deployerBalanceBefore)
expect(governanceBalanceAfter).to.equal(governanceBalanceBefore)
await expect(governance.execute(proposalId)).to.be.revertedWith(
'Governance::propose: metamorphic contracts not allowed',
)
// Terminate the contract for the next test
await maliciousProposal.emergencyStop()