From eac5a614944d432bed170baee2923ad65fd823cd Mon Sep 17 00:00:00 2001 From: Ben DiFrancesco Date: Mon, 19 Dec 2022 18:04:26 -0500 Subject: [PATCH 01/27] Set up and run one end-to-end test with the new Governor Create test harness for testing that the new Governor can execute proposals after the upgrade has completed. Write one test demonstrating the Governor's ability to send GTC already held by the timelock. --- test/GitcoinGovernor.t.sol | 249 +++++++++++++++++++++++++++++-------- 1 file changed, 199 insertions(+), 50 deletions(-) diff --git a/test/GitcoinGovernor.t.sol b/test/GitcoinGovernor.t.sol index 9aeed85..0f4b0b7 100644 --- a/test/GitcoinGovernor.t.sol +++ b/test/GitcoinGovernor.t.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.17; import "forge-std/Test.sol"; import {IERC20} from "openzeppelin-contracts/interfaces/IERC20.sol"; +import {IGovernor} from "openzeppelin-contracts/governance/IGovernor.sol"; import {GitcoinGovernor, ICompoundTimelock} from "src/GitcoinGovernor.sol"; import {DeployInput, DeployScript} from "script/Deploy.s.sol"; import {IGovernorAlpha} from "src/interfaces/IGovernorAlpha.sol"; @@ -47,7 +48,7 @@ contract GitcoinGovernorProposalTestHelper is GitcoinGovernorTestHelper { IGTC gtcToken = IGTC(GTC_TOKEN); ICompoundTimelock timelock = ICompoundTimelock(payable(TIMELOCK)); uint256 initialProposalCount; - uint256 proposalId; + uint256 upgradeProposalId; address[] delegates = [ PROPOSER, // kbw.eth (~1.8M) 0x2df9a188fBE231B0DC36D14AcEb65dEFbB049479, // janineleger.eth (~1.53M) @@ -71,61 +72,67 @@ contract GitcoinGovernorProposalTestHelper is GitcoinGovernorTestHelper { initialProposalCount = governorAlpha.proposalCount(); ProposeScript _proposeScript = new ProposeScript(); - proposalId = _proposeScript.run(governor); + upgradeProposalId = _proposeScript.run(governor); } //--------------- HELPERS ---------------// - function proposalStartBlock() public view returns (uint256) { - (,,, uint256 _startBlock,,,,,) = governorAlpha.proposals(proposalId); + function upgradeProposalStartBlock() public view returns (uint256) { + (,,, uint256 _startBlock,,,,,) = governorAlpha.proposals(upgradeProposalId); return _startBlock; } - function proposalEndBlock() public view returns (uint256) { - (,,,, uint256 _endBlock,,,,) = governorAlpha.proposals(proposalId); + function upgradeProposalEndBlock() public view returns (uint256) { + (,,,, uint256 _endBlock,,,,) = governorAlpha.proposals(upgradeProposalId); return _endBlock; } - function proposalEta() public view returns (uint256) { - (,, uint256 _eta,,,,,,) = governorAlpha.proposals(proposalId); + function upgradeProposalEta() public view returns (uint256) { + (,, uint256 _eta,,,,,,) = governorAlpha.proposals(upgradeProposalId); return _eta; } - function jumpToActiveProposal() public { - vm.roll(proposalStartBlock() + 1); + function jumpToActiveUpgradeProposal() public { + vm.roll(upgradeProposalStartBlock() + 1); } - function jumpToVoteComplete() public { - vm.roll(proposalEndBlock() + 1); + function jumpToUpgradeVoteComplete() public { + vm.roll(upgradeProposalEndBlock() + 1); } function jumpPastProposalEta() public { vm.roll(block.number + 1); // move up one block so we're not in the same block as when queued - vm.warp(proposalEta() + 1); // jump past the eta timestamp + vm.warp(upgradeProposalEta() + 1); // jump past the eta timestamp } - function delegatesVoteOnProposal(bool _support) public { + function delegatesVoteOnUpgradeProposal(bool _support) public { for (uint256 _index = 0; _index < delegates.length; _index++) { vm.prank(delegates[_index]); - governorAlpha.castVote(proposalId, _support); + governorAlpha.castVote(upgradeProposalId, _support); } } - function passProposal() public { - jumpToActiveProposal(); - delegatesVoteOnProposal(true); - jumpToVoteComplete(); + function passUpgradeProposal() public { + jumpToActiveUpgradeProposal(); + delegatesVoteOnUpgradeProposal(true); + jumpToUpgradeVoteComplete(); } - function defeatProposal() public { - jumpToActiveProposal(); - delegatesVoteOnProposal(false); - jumpToVoteComplete(); + function defeatUpgradeProposal() public { + jumpToActiveUpgradeProposal(); + delegatesVoteOnUpgradeProposal(false); + jumpToUpgradeVoteComplete(); } - function passAndQueueProposal() public { - passProposal(); - governorAlpha.queue(proposalId); + function passAndQueueUpgradeProposal() public { + passUpgradeProposal(); + governorAlpha.queue(upgradeProposalId); + } + + function passQueueAndExecuteUpgradeProposal() public { + passAndQueueUpgradeProposal(); + jumpPastProposalEta(); + governorAlpha.execute(upgradeProposalId); } } @@ -135,7 +142,7 @@ contract GitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { assertEq(governorAlpha.proposalCount(), initialProposalCount + 1); // Proposal is in the expected state - uint8 _state = governorAlpha.state(proposalId); + uint8 _state = governorAlpha.state(upgradeProposalId); assertEq(_state, PENDING); // Proposal actions correspond to Governor upgrade @@ -144,7 +151,7 @@ contract GitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { uint256[] memory _values, string[] memory _signatures, bytes[] memory _calldatas - ) = governorAlpha.getActions(proposalId); + ) = governorAlpha.getActions(upgradeProposalId); assertEq(_targets.length, 2); assertEq(_targets[0], TIMELOCK); assertEq(_targets[1], address(governor)); @@ -160,48 +167,48 @@ contract GitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { } function test_proposalActiveAfterDelay() public { - jumpToActiveProposal(); + jumpToActiveUpgradeProposal(); // Ensure proposal has become active the block after the voting delay - uint8 _state = governorAlpha.state(proposalId); + uint8 _state = governorAlpha.state(upgradeProposalId); assertEq(_state, ACTIVE); } function testFuzz_ProposerCanCastVote(bool _willSupport) public { - jumpToActiveProposal(); - uint256 _proposerVotes = gtcToken.getPriorVotes(PROPOSER, proposalStartBlock()); + jumpToActiveUpgradeProposal(); + uint256 _proposerVotes = gtcToken.getPriorVotes(PROPOSER, upgradeProposalStartBlock()); vm.prank(PROPOSER); - governorAlpha.castVote(proposalId, _willSupport); + governorAlpha.castVote(upgradeProposalId, _willSupport); - IGovernorAlpha.Receipt memory _receipt = governorAlpha.getReceipt(proposalId, PROPOSER); + IGovernorAlpha.Receipt memory _receipt = governorAlpha.getReceipt(upgradeProposalId, PROPOSER); assertEq(_receipt.hasVoted, true); assertEq(_receipt.support, _willSupport); assertEq(_receipt.votes, _proposerVotes); } function test_ProposalSucceedsWhenAllDelegatesVoteFor() public { - passProposal(); + passUpgradeProposal(); // Ensure proposal state is now succeeded - uint8 _state = governorAlpha.state(proposalId); + uint8 _state = governorAlpha.state(upgradeProposalId); assertEq(_state, SUCCEEDED); } function test_ProposalDefeatedWhenAllDelegatesVoteAgainst() public { - defeatProposal(); + defeatUpgradeProposal(); // Ensure proposal state is now defeated - uint8 _state = governorAlpha.state(proposalId); + uint8 _state = governorAlpha.state(upgradeProposalId); assertEq(_state, DEFEATED); } function test_ProposalCanBeQueuedAfterSucceeding() public { - passProposal(); - governorAlpha.queue(proposalId); + passUpgradeProposal(); + governorAlpha.queue(upgradeProposalId); // Ensure proposal can be queued after success - uint8 _state = governorAlpha.state(proposalId); + uint8 _state = governorAlpha.state(upgradeProposalId); assertEq(_state, QUEUED); ( @@ -209,7 +216,7 @@ contract GitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { uint256[] memory _values, string[] memory _signatures, bytes[] memory _calldatas - ) = governorAlpha.getActions(proposalId); + ) = governorAlpha.getActions(upgradeProposalId); uint256 _eta = block.timestamp + timelock.delay(); @@ -226,14 +233,14 @@ contract GitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { } function test_ProposalCanBeExecutedAfterDelay() public { - passAndQueueProposal(); + passAndQueueUpgradeProposal(); jumpPastProposalEta(); // Execute the proposal - governorAlpha.execute(proposalId); + governorAlpha.execute(upgradeProposalId); // Ensure the proposal is now executed - uint8 _state = governorAlpha.state(proposalId); + uint8 _state = governorAlpha.state(upgradeProposalId); assertEq(_state, EXECUTED); // Ensure the governor is now the admin of the timelock @@ -265,7 +272,7 @@ contract GitcoinGovernorAlphaPostProposalTest is GitcoinGovernorProposalTestHelp uint256 _initialGtcBalance = gtcToken.balanceOf(_gtcReceiver); // Defeat the proposal to upgrade the Governor - defeatProposal(); + defeatUpgradeProposal(); // Craft a new proposal to send GTC address[] memory _targets = new address[](1); @@ -330,7 +337,7 @@ contract GitcoinGovernorAlphaPostProposalTest is GitcoinGovernorProposalTestHelp uint256 _initialRadBalance = radToken.balanceOf(_radReceiver); // Defeat the proposal to upgrade the Governor - defeatProposal(); + defeatUpgradeProposal(); // Craft a new proposal to send amounts of all three tokens address[] memory _targets = new address[](2); @@ -392,9 +399,7 @@ contract GitcoinGovernorAlphaPostProposalTest is GitcoinGovernorProposalTestHelp _gtcAmount = bound(_gtcAmount, 0, _timelockGtcBalance); // Pass and execute the proposal to upgrade the Governor - passAndQueueProposal(); - jumpPastProposalEta(); - governorAlpha.execute(proposalId); + passQueueAndExecuteUpgradeProposal(); // Craft a new proposal to send GTC address[] memory _targets = new address[](1); @@ -427,3 +432,147 @@ contract GitcoinGovernorAlphaPostProposalTest is GitcoinGovernorProposalTestHelp governorAlpha.queue(_newProposalId); } } + +contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { + // From GovernorCountingSimple + uint8 constant AGAINST = 0; + uint8 constant FOR = 1; + uint8 constant ABSTAIN = 2; + + function assumeReceiver(address _receiver) public { + vm.assume(_receiver != TIMELOCK && _receiver != address(0x0)); + } + + function buildProposalData(string memory _signature, bytes memory _calldata) + public + pure + returns (bytes memory) + { + return abi.encodePacked(bytes4(keccak256(bytes(_signature))), _calldata); + } + + function jumpToActiveProposal(uint256 _proposalId) public { + uint256 _snapshot = governor.proposalSnapshot(_proposalId); + vm.roll(_snapshot + 1); + } + + function jumpToVotingComplete(uint256 _proposalId) public { + // Jump one block past the proposal voting deadline + uint256 _deadline = governor.proposalDeadline(_proposalId); + vm.roll(_deadline + 1); + } + + function jumpPastProposalEta(uint256 _proposalId) public { + uint256 _eta = governor.proposalEta(_proposalId); + vm.roll(block.number + 1); + vm.warp(_eta + 1); + } + + function delegatesVoteOnProposal(uint256 _proposalId, uint8 _support) public { + assertLt(_support, 3, "Invalid value for support"); + + for (uint256 _index = 0; _index < delegates.length; _index++) { + vm.prank(delegates[_index]); + governor.castVote(_proposalId, _support); + } + } + + function submitGtcSendProposal(uint256 _gtcAmount, address _gtcReceiver) + public + returns (uint256, address[] memory, uint256[] memory, bytes[] memory, string memory) + { + // Craft a new proposal to send GTC + address[] memory _targets = new address[](1); + uint256[] memory _values = new uint256[](1); + bytes[] memory _calldatas = new bytes[](1); + + _targets[0] = GTC_TOKEN; + _values[0] = 0; + _calldatas[0] = + buildProposalData("transfer(address,uint256)", abi.encode(_gtcReceiver, _gtcAmount)); + string memory _description = "Transfer some GTC from the old Governor"; + + // Submit the new proposal + vm.prank(PROPOSER); + uint256 _newProposalId = governor.propose(_targets, _values, _calldatas, _description); + + return (_newProposalId, _targets, _values, _calldatas, _description); + } + + function assertEq(IGovernor.ProposalState _actual, IGovernor.ProposalState _expected) public { + assertEq(uint8(_actual), uint8(_expected)); + } + + function setUp() public override { + GitcoinGovernorProposalTestHelper.setUp(); + } + + function testFuzz_NewGovernorCanReceiveNewProposal(uint256 _gtcAmount, address _gtcReceiver) + public + { + assumeReceiver(_gtcReceiver); + passQueueAndExecuteUpgradeProposal(); + (uint256 _newProposalId,,,,) = submitGtcSendProposal(_gtcAmount, _gtcReceiver); + + // Ensure proposal is in the expected state + IGovernor.ProposalState _state = governor.state(_newProposalId); + assertEq(_state, IGovernor.ProposalState.Pending); + } + + function testFuzz_NewGovernorCanPassProposalAndSendGtc(uint256 _gtcAmount, address _gtcReceiver) + public + { + assumeReceiver(_gtcReceiver); + uint256 _timelockGtcBalance = gtcToken.balanceOf(TIMELOCK); + + // bound by the number of tokens the timelock currently controls + _gtcAmount = bound(_gtcAmount, 0, _timelockGtcBalance); + uint256 _initialGtcBalance = gtcToken.balanceOf(_gtcReceiver); + + passQueueAndExecuteUpgradeProposal(); + ( + uint256 _newProposalId, + address[] memory _targets, + uint256[] memory _values, + bytes[] memory _calldatas, + string memory _description + ) = submitGtcSendProposal(_gtcAmount, _gtcReceiver); + + // Ensure proposal is in the expected state + IGovernor.ProposalState _state = governor.state(_newProposalId); + assertEq(_state, IGovernor.ProposalState.Pending); + + jumpToActiveProposal(_newProposalId); + + // Ensure the proposal is now Active + _state = governor.state(_newProposalId); + assertEq(_state, IGovernor.ProposalState.Active); + + delegatesVoteOnProposal(_newProposalId, FOR); + jumpToVotingComplete(_newProposalId); + + // Ensure the proposal has succeeded + _state = governor.state(_newProposalId); + assertEq(_state, IGovernor.ProposalState.Succeeded); + + // Queue the proposal + governor.queue(_targets, _values, _calldatas, keccak256(bytes(_description))); + + // Ensure the proposal is queued + _state = governor.state(_newProposalId); + assertEq(_state, IGovernor.ProposalState.Queued); + + jumpPastProposalEta(_newProposalId); + + // Execute the proposal + governor.execute(_targets, _values, _calldatas, keccak256(bytes(_description))); + + // Ensure the proposal is executed + _state = governor.state(_newProposalId); + assertEq(_state, IGovernor.ProposalState.Executed); + + // Ensure the tokens have been transferred + assertEq(gtcToken.balanceOf(_gtcReceiver), _initialGtcBalance + _gtcAmount); + assertEq(gtcToken.balanceOf(TIMELOCK), _timelockGtcBalance - _gtcAmount); + } +} From dd4b4d69d5721cdb2f321d43a1cba3341d96b9e3 Mon Sep 17 00:00:00 2001 From: David Laprade Date: Thu, 12 Jan 2023 11:22:39 -0500 Subject: [PATCH 02/27] Add fuzz seed to .env template to reduce RPC calls --- .env.template | 1 + 1 file changed, 1 insertion(+) diff --git a/.env.template b/.env.template index fb5d2d7..18f9d12 100644 --- a/.env.template +++ b/.env.template @@ -1,3 +1,4 @@ DEPLOYER_PRIVATE_KEY= PROPOSER_PRIVATE_KEY= MAINNET_RPC_URL= +FOUNDRY_FUZZ_SEED= From 98fb2429a4a8db597cf611e933363ea21163d741 Mon Sep 17 00:00:00 2001 From: David Laprade Date: Thu, 12 Jan 2023 14:48:27 -0500 Subject: [PATCH 03/27] Generalize governorBravo token send test --- test/GitcoinGovernor.t.sol | 43 +++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/test/GitcoinGovernor.t.sol b/test/GitcoinGovernor.t.sol index 0f4b0b7..dff6511 100644 --- a/test/GitcoinGovernor.t.sol +++ b/test/GitcoinGovernor.t.sol @@ -46,6 +46,10 @@ contract GitcoinGovernorProposalTestHelper is GitcoinGovernorTestHelper { IGovernorAlpha governorAlpha = IGovernorAlpha(0xDbD27635A534A3d3169Ef0498beB56Fb9c937489); IGTC gtcToken = IGTC(GTC_TOKEN); + address constant USDC_ADDRESS = 0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48; + address constant RAD_ADDRESS = 0x31c8EAcBFFdD875c74b94b077895Bd78CF1E64A3; + IERC20 usdcToken = IERC20(USDC_ADDRESS); + IERC20 radToken = IERC20(RAD_ADDRESS); ICompoundTimelock timelock = ICompoundTimelock(payable(TIMELOCK)); uint256 initialProposalCount; uint256 upgradeProposalId; @@ -249,16 +253,8 @@ contract GitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { } contract GitcoinGovernorAlphaPostProposalTest is GitcoinGovernorProposalTestHelper { - address constant USDC_ADDRESS = 0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48; - address constant RAD_ADDRESS = 0x31c8EAcBFFdD875c74b94b077895Bd78CF1E64A3; - - IERC20 usdcToken = IERC20(USDC_ADDRESS); - IERC20 radToken = IERC20(RAD_ADDRESS); - function setUp() public override { GitcoinGovernorProposalTestHelper.setUp(); - usdcToken = IERC20(USDC_ADDRESS); - radToken = IERC20(RAD_ADDRESS); } function testFuzz_OldGovernorSendsGTCAfterProposalIsDefeated( @@ -477,7 +473,7 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { } } - function submitGtcSendProposal(uint256 _gtcAmount, address _gtcReceiver) + function submitTokenSendProposal(address _token, uint256 _amount, address _receiver) public returns (uint256, address[] memory, uint256[] memory, bytes[] memory, string memory) { @@ -486,11 +482,11 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { uint256[] memory _values = new uint256[](1); bytes[] memory _calldatas = new bytes[](1); - _targets[0] = GTC_TOKEN; + _targets[0] = _token; _values[0] = 0; _calldatas[0] = - buildProposalData("transfer(address,uint256)", abi.encode(_gtcReceiver, _gtcAmount)); - string memory _description = "Transfer some GTC from the old Governor"; + buildProposalData("transfer(address,uint256)", abi.encode(_receiver, _amount)); + string memory _description = "Transfer some token from the new Governor"; // Submit the new proposal vm.prank(PROPOSER); @@ -512,22 +508,27 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { { assumeReceiver(_gtcReceiver); passQueueAndExecuteUpgradeProposal(); - (uint256 _newProposalId,,,,) = submitGtcSendProposal(_gtcAmount, _gtcReceiver); + (uint256 _newProposalId,,,,) = submitTokenSendProposal(address(gtcToken), _gtcAmount, _gtcReceiver); // Ensure proposal is in the expected state IGovernor.ProposalState _state = governor.state(_newProposalId); assertEq(_state, IGovernor.ProposalState.Pending); } - function testFuzz_NewGovernorCanPassProposalAndSendGtc(uint256 _gtcAmount, address _gtcReceiver) + function testFuzz_NewGovernorCanPassProposalAndSendToken(uint256 _amount, address _receiver) public { - assumeReceiver(_gtcReceiver); - uint256 _timelockGtcBalance = gtcToken.balanceOf(TIMELOCK); + IERC20 _token; + if (_amount % 3 == 0) _token = IERC20(address(gtcToken)); + if (_amount % 3 == 1) _token = usdcToken; + if (_amount % 3 == 2) _token = radToken; + + assumeReceiver(_receiver); + uint256 _timelockTokenBalance = _token.balanceOf(TIMELOCK); // bound by the number of tokens the timelock currently controls - _gtcAmount = bound(_gtcAmount, 0, _timelockGtcBalance); - uint256 _initialGtcBalance = gtcToken.balanceOf(_gtcReceiver); + _amount = bound(_amount, 0, _timelockTokenBalance); + uint256 _initialTokenBalance = _token.balanceOf(_receiver); passQueueAndExecuteUpgradeProposal(); ( @@ -536,7 +537,7 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { uint256[] memory _values, bytes[] memory _calldatas, string memory _description - ) = submitGtcSendProposal(_gtcAmount, _gtcReceiver); + ) = submitTokenSendProposal(address(_token), _amount, _receiver); // Ensure proposal is in the expected state IGovernor.ProposalState _state = governor.state(_newProposalId); @@ -572,7 +573,7 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { assertEq(_state, IGovernor.ProposalState.Executed); // Ensure the tokens have been transferred - assertEq(gtcToken.balanceOf(_gtcReceiver), _initialGtcBalance + _gtcAmount); - assertEq(gtcToken.balanceOf(TIMELOCK), _timelockGtcBalance - _gtcAmount); + assertEq(_token.balanceOf(_receiver), _initialTokenBalance + _amount); + assertEq(_token.balanceOf(TIMELOCK), _timelockTokenBalance - _amount); } } From d9069d1e00ce875c525cf8a0d3784c4ede2f0d53 Mon Sep 17 00:00:00 2001 From: David Laprade Date: Thu, 12 Jan 2023 15:02:00 -0500 Subject: [PATCH 04/27] Test that proposals can be defeated for the new governor --- test/GitcoinGovernor.t.sol | 39 ++++++++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/test/GitcoinGovernor.t.sol b/test/GitcoinGovernor.t.sol index dff6511..dd073c1 100644 --- a/test/GitcoinGovernor.t.sol +++ b/test/GitcoinGovernor.t.sol @@ -515,14 +515,41 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { assertEq(_state, IGovernor.ProposalState.Pending); } - function testFuzz_NewGovernorCanPassProposalAndSendToken(uint256 _amount, address _receiver) + function _randomERC20Token(uint256 _seed) internal view returns(IERC20 _token) { + if (_seed % 3 == 0) _token = IERC20(address(gtcToken)); + if (_seed % 3 == 1) _token = usdcToken; + if (_seed % 3 == 2) _token = radToken; + } + + function testFuzz_NewGovernorCanDefeatProposal(uint256 _amount, address _receiver) public { + IERC20 _token = _randomERC20Token(_amount); + assumeReceiver(_receiver); + + passQueueAndExecuteUpgradeProposal(); + (uint256 _newProposalId,,,,) = submitTokenSendProposal(address(_token), _amount, _receiver); + + // Ensure proposal is in the expected state + IGovernor.ProposalState _state = governor.state(_newProposalId); + assertEq(_state, IGovernor.ProposalState.Pending); + + jumpToActiveProposal(_newProposalId); + + // Ensure the proposal is now Active + _state = governor.state(_newProposalId); + assertEq(_state, IGovernor.ProposalState.Active); + + delegatesVoteOnProposal(_newProposalId, AGAINST); + jumpToVotingComplete(_newProposalId); + + // Ensure the proposal has failed + _state = governor.state(_newProposalId); + assertEq(_state, IGovernor.ProposalState.Defeated); + } + + function testFuzz_NewGovernorCanPassProposalToSendToken(uint256 _amount, address _receiver) public { - IERC20 _token; - if (_amount % 3 == 0) _token = IERC20(address(gtcToken)); - if (_amount % 3 == 1) _token = usdcToken; - if (_amount % 3 == 2) _token = radToken; - + IERC20 _token = _randomERC20Token(_amount); assumeReceiver(_receiver); uint256 _timelockTokenBalance = _token.balanceOf(TIMELOCK); From 5dfd792f20cea090111505f44439fad2199b3560 Mon Sep 17 00:00:00 2001 From: David Laprade Date: Thu, 12 Jan 2023 15:42:27 -0500 Subject: [PATCH 05/27] passQueueAndExecuteUpgradeProposal --> upgradeToBravoGovernor --- test/GitcoinGovernor.t.sol | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/GitcoinGovernor.t.sol b/test/GitcoinGovernor.t.sol index dd073c1..0b41e51 100644 --- a/test/GitcoinGovernor.t.sol +++ b/test/GitcoinGovernor.t.sol @@ -133,7 +133,7 @@ contract GitcoinGovernorProposalTestHelper is GitcoinGovernorTestHelper { governorAlpha.queue(upgradeProposalId); } - function passQueueAndExecuteUpgradeProposal() public { + function upgradeToBravoGovernor() public { passAndQueueUpgradeProposal(); jumpPastProposalEta(); governorAlpha.execute(upgradeProposalId); @@ -395,7 +395,7 @@ contract GitcoinGovernorAlphaPostProposalTest is GitcoinGovernorProposalTestHelp _gtcAmount = bound(_gtcAmount, 0, _timelockGtcBalance); // Pass and execute the proposal to upgrade the Governor - passQueueAndExecuteUpgradeProposal(); + upgradeToBravoGovernor(); // Craft a new proposal to send GTC address[] memory _targets = new address[](1); @@ -507,7 +507,7 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { public { assumeReceiver(_gtcReceiver); - passQueueAndExecuteUpgradeProposal(); + upgradeToBravoGovernor(); (uint256 _newProposalId,,,,) = submitTokenSendProposal(address(gtcToken), _gtcAmount, _gtcReceiver); // Ensure proposal is in the expected state @@ -525,7 +525,7 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { IERC20 _token = _randomERC20Token(_amount); assumeReceiver(_receiver); - passQueueAndExecuteUpgradeProposal(); + upgradeToBravoGovernor(); (uint256 _newProposalId,,,,) = submitTokenSendProposal(address(_token), _amount, _receiver); // Ensure proposal is in the expected state @@ -557,7 +557,7 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { _amount = bound(_amount, 0, _timelockTokenBalance); uint256 _initialTokenBalance = _token.balanceOf(_receiver); - passQueueAndExecuteUpgradeProposal(); + upgradeToBravoGovernor(); ( uint256 _newProposalId, address[] memory _targets, From 470d880b9d4f42ff8ae1ce82ac557cab09f29327 Mon Sep 17 00:00:00 2001 From: David Laprade Date: Thu, 12 Jan 2023 15:43:01 -0500 Subject: [PATCH 06/27] Test that settings can be updated by new governor --- test/GitcoinGovernor.t.sol | 69 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/test/GitcoinGovernor.t.sol b/test/GitcoinGovernor.t.sol index 0b41e51..a841c93 100644 --- a/test/GitcoinGovernor.t.sol +++ b/test/GitcoinGovernor.t.sol @@ -603,4 +603,73 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { assertEq(_token.balanceOf(_receiver), _initialTokenBalance + _amount); assertEq(_token.balanceOf(TIMELOCK), _timelockTokenBalance - _amount); } + + function testFuzz_NewGovernorCanUpdateSettingsViaSuccessfulProposal( + uint256 _newDelay, + uint256 _newVotingPeriod, + uint256 _newProposalThreshold + ) public { + // The upper bounds are arbitrary here. + _newDelay = bound(_newDelay, 0, 50_000); // about a week at 1 block per 12s + _newVotingPeriod = bound(_newVotingPeriod, 1, 200_000); // about a month + _newProposalThreshold = bound(_newProposalThreshold, 0, 42 ether); + + upgradeToBravoGovernor(); + + address[] memory _targets = new address[](3); + uint256[] memory _values = new uint256[](3); + bytes[] memory _calldatas = new bytes[](3); + string memory _description = "Update governance settings"; + + _targets[0] = address(governor); + _calldatas[0] = buildProposalData("setVotingDelay(uint256)", abi.encode(_newDelay)); + + _targets[1] = address(governor); + _calldatas[1] = buildProposalData("setVotingPeriod(uint256)", abi.encode(_newVotingPeriod)); + + _targets[2] = address(governor); + _calldatas[2] = buildProposalData("setProposalThreshold(uint256)", abi.encode(_newProposalThreshold)); + + // Submit the new proposal + vm.prank(PROPOSER); + uint256 _newProposalId = governor.propose(_targets, _values, _calldatas, _description); + + // Ensure proposal is in the expected state + IGovernor.ProposalState _state = governor.state(_newProposalId); + assertEq(_state, IGovernor.ProposalState.Pending); + + jumpToActiveProposal(_newProposalId); + + // Ensure the proposal is now Active + _state = governor.state(_newProposalId); + assertEq(_state, IGovernor.ProposalState.Active); + + delegatesVoteOnProposal(_newProposalId, FOR); + jumpToVotingComplete(_newProposalId); + + // Ensure the proposal has succeeded + _state = governor.state(_newProposalId); + assertEq(_state, IGovernor.ProposalState.Succeeded); + + // Queue the proposal + governor.queue(_targets, _values, _calldatas, keccak256(bytes(_description))); + + // Ensure the proposal is queued + _state = governor.state(_newProposalId); + assertEq(_state, IGovernor.ProposalState.Queued); + + jumpPastProposalEta(_newProposalId); + + // Execute the proposal + governor.execute(_targets, _values, _calldatas, keccak256(bytes(_description))); + + // Ensure the proposal is executed + _state = governor.state(_newProposalId); + assertEq(_state, IGovernor.ProposalState.Executed); + + // Confirm that governance settings have updated. + assertEq(governor.votingDelay(), _newDelay); + assertEq(governor.votingPeriod(), _newVotingPeriod); + assertEq(governor.proposalThreshold(), _newProposalThreshold); + } } From ddd1c6abbd82a728852eee5eca5dc9b89c06d4d3 Mon Sep 17 00:00:00 2001 From: David Laprade Date: Thu, 12 Jan 2023 16:31:57 -0500 Subject: [PATCH 07/27] Test mixed voting results --- test/GitcoinGovernor.t.sol | 137 +++++++++++++++++++++++++++++++++++-- 1 file changed, 132 insertions(+), 5 deletions(-) diff --git a/test/GitcoinGovernor.t.sol b/test/GitcoinGovernor.t.sol index a841c93..ddfcc5f 100644 --- a/test/GitcoinGovernor.t.sol +++ b/test/GitcoinGovernor.t.sol @@ -464,7 +464,7 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { vm.warp(_eta + 1); } - function delegatesVoteOnProposal(uint256 _proposalId, uint8 _support) public { + function delegatesVoteOnBravoGovernor(uint256 _proposalId, uint8 _support) public { assertLt(_support, 3, "Invalid value for support"); for (uint256 _index = 0; _index < delegates.length; _index++) { @@ -526,7 +526,13 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { assumeReceiver(_receiver); upgradeToBravoGovernor(); - (uint256 _newProposalId,,,,) = submitTokenSendProposal(address(_token), _amount, _receiver); + ( + uint256 _newProposalId, + address[] memory _targets, + uint256[] memory _values, + bytes[] memory _calldatas, + string memory _description + ) = submitTokenSendProposal(address(_token), _amount, _receiver); // Ensure proposal is in the expected state IGovernor.ProposalState _state = governor.state(_newProposalId); @@ -538,12 +544,16 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { _state = governor.state(_newProposalId); assertEq(_state, IGovernor.ProposalState.Active); - delegatesVoteOnProposal(_newProposalId, AGAINST); + delegatesVoteOnBravoGovernor(_newProposalId, AGAINST); jumpToVotingComplete(_newProposalId); // Ensure the proposal has failed _state = governor.state(_newProposalId); assertEq(_state, IGovernor.ProposalState.Defeated); + + // It should not be possible to queue the proposal + vm.expectRevert("Governor: proposal not successful"); + governor.queue(_targets, _values, _calldatas, keccak256(bytes(_description))); } function testFuzz_NewGovernorCanPassProposalToSendToken(uint256 _amount, address _receiver) @@ -576,7 +586,7 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { _state = governor.state(_newProposalId); assertEq(_state, IGovernor.ProposalState.Active); - delegatesVoteOnProposal(_newProposalId, FOR); + delegatesVoteOnBravoGovernor(_newProposalId, FOR); jumpToVotingComplete(_newProposalId); // Ensure the proposal has succeeded @@ -644,7 +654,7 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { _state = governor.state(_newProposalId); assertEq(_state, IGovernor.ProposalState.Active); - delegatesVoteOnProposal(_newProposalId, FOR); + delegatesVoteOnBravoGovernor(_newProposalId, FOR); jumpToVotingComplete(_newProposalId); // Ensure the proposal has succeeded @@ -672,4 +682,121 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { assertEq(governor.votingPeriod(), _newVotingPeriod); assertEq(governor.proposalThreshold(), _newProposalThreshold); } + + function testFuzz_NewGovernorCanPassMixedProposal(uint256 _amount, address _receiver) public { + IERC20 _token = _randomERC20Token(_amount); + assumeReceiver(_receiver); + uint256 _timelockTokenBalance = _token.balanceOf(TIMELOCK); + + // bound by the number of tokens the timelock currently controls + _amount = bound(_amount, 0, _timelockTokenBalance); + uint256 _initialTokenBalance = _token.balanceOf(_receiver); + + upgradeToBravoGovernor(); + ( + uint256 _newProposalId, + address[] memory _targets, + uint256[] memory _values, + bytes[] memory _calldatas, + string memory _description + ) = submitTokenSendProposal(address(_token), _amount, _receiver); + + // Ensure proposal is in the expected state + IGovernor.ProposalState _state = governor.state(_newProposalId); + assertEq(_state, IGovernor.ProposalState.Pending); + + jumpToActiveProposal(_newProposalId); + + // Ensure the proposal is now Active + _state = governor.state(_newProposalId); + assertEq(_state, IGovernor.ProposalState.Active); + + // Delegates vote with a mix of For/Against/Abstain with For winning. + vm.prank(PROPOSER); // kbw.eth (~1.8M votes) + governor.castVote(_newProposalId, FOR); + vm.prank(0x2df9a188fBE231B0DC36D14AcEb65dEFbB049479); // janineleger.eth (~1.53M) + governor.castVote(_newProposalId, FOR); + vm.prank(0x4Be88f63f919324210ea3A2cCAD4ff0734425F91); // kevinolsen.eth (~1.35M) + governor.castVote(_newProposalId, FOR); + vm.prank(0x34aA3F359A9D614239015126635CE7732c18fDF3); // Austin Griffith (~1.05M) + governor.castVote(_newProposalId, AGAINST); + vm.prank(0x7E052Ef7B4bB7E5A45F331128AFadB1E589deaF1); // Kris Is (~1.05M) + governor.castVote(_newProposalId, ABSTAIN); + vm.prank(0x5e349eca2dc61aBCd9dD99Ce94d04136151a09Ee); // Linda Xie (~1.02M) + governor.castVote(_newProposalId, AGAINST); + + jumpToVotingComplete(_newProposalId); + + // Ensure the proposal has succeeded + _state = governor.state(_newProposalId); + assertEq(_state, IGovernor.ProposalState.Succeeded); + + // Queue the proposal + governor.queue(_targets, _values, _calldatas, keccak256(bytes(_description))); + + jumpPastProposalEta(_newProposalId); + + // Execute the proposal + governor.execute(_targets, _values, _calldatas, keccak256(bytes(_description))); + + // Ensure the proposal is executed + _state = governor.state(_newProposalId); + assertEq(_state, IGovernor.ProposalState.Executed); + + // Ensure the tokens have been transferred + assertEq(_token.balanceOf(_receiver), _initialTokenBalance + _amount); + assertEq(_token.balanceOf(TIMELOCK), _timelockTokenBalance - _amount); + } + + function testFuzz_NewGovernorCanDefeatMixedProposal(uint256 _amount, address _receiver) public { + IERC20 _token = _randomERC20Token(_amount); + assumeReceiver(_receiver); + uint256 _timelockTokenBalance = _token.balanceOf(TIMELOCK); + + // bound by the number of tokens the timelock currently controls + _amount = bound(_amount, 0, _timelockTokenBalance); + + upgradeToBravoGovernor(); + ( + uint256 _newProposalId, + address[] memory _targets, + uint256[] memory _values, + bytes[] memory _calldatas, + string memory _description + ) = submitTokenSendProposal(address(_token), _amount, _receiver); + + // Ensure proposal is in the expected state + IGovernor.ProposalState _state = governor.state(_newProposalId); + assertEq(_state, IGovernor.ProposalState.Pending); + + jumpToActiveProposal(_newProposalId); + + // Ensure the proposal is now Active + _state = governor.state(_newProposalId); + assertEq(_state, IGovernor.ProposalState.Active); + + // Delegates vote with a mix of For/Against/Abstain with Against/Abstain winning. + vm.prank(PROPOSER); // kbw.eth (~1.8M votes) + governor.castVote(_newProposalId, ABSTAIN); + vm.prank(0x2df9a188fBE231B0DC36D14AcEb65dEFbB049479); // janineleger.eth (~1.53M) + governor.castVote(_newProposalId, FOR); + vm.prank(0x4Be88f63f919324210ea3A2cCAD4ff0734425F91); // kevinolsen.eth (~1.35M) + governor.castVote(_newProposalId, FOR); + vm.prank(0x34aA3F359A9D614239015126635CE7732c18fDF3); // Austin Griffith (~1.05M) + governor.castVote(_newProposalId, AGAINST); + vm.prank(0x7E052Ef7B4bB7E5A45F331128AFadB1E589deaF1); // Kris Is (~1.05M) + governor.castVote(_newProposalId, AGAINST); + vm.prank(0x5e349eca2dc61aBCd9dD99Ce94d04136151a09Ee); // Linda Xie (~1.02M) + governor.castVote(_newProposalId, AGAINST); + + jumpToVotingComplete(_newProposalId); + + // Ensure the proposal has failed + _state = governor.state(_newProposalId); + assertEq(_state, IGovernor.ProposalState.Defeated); + + // It should not be possible to queue the proposal + vm.expectRevert("Governor: proposal not successful"); + governor.queue(_targets, _values, _calldatas, keccak256(bytes(_description))); + } } From 8e601736c1fc1a3b1de9865d05228664b1bac3f6 Mon Sep 17 00:00:00 2001 From: David Laprade Date: Fri, 13 Jan 2023 15:08:13 -0500 Subject: [PATCH 08/27] Test that alpha votes don't affect bravo votes --- test/GitcoinGovernor.t.sol | 85 +++++++++++++++++++++++++++++++++++++- 1 file changed, 84 insertions(+), 1 deletion(-) diff --git a/test/GitcoinGovernor.t.sol b/test/GitcoinGovernor.t.sol index ddfcc5f..87dbce3 100644 --- a/test/GitcoinGovernor.t.sol +++ b/test/GitcoinGovernor.t.sol @@ -486,7 +486,7 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { _values[0] = 0; _calldatas[0] = buildProposalData("transfer(address,uint256)", abi.encode(_receiver, _amount)); - string memory _description = "Transfer some token from the new Governor"; + string memory _description = "Transfer some tokens from the new Governor"; // Submit the new proposal vm.prank(PROPOSER); @@ -799,4 +799,87 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { vm.expectRevert("Governor: proposal not successful"); governor.queue(_targets, _values, _calldatas, keccak256(bytes(_description))); } + + struct NewGovernorUnaffectedByVotesOnOldGovernorVars { + uint256 alphaProposalId; + address[] alphaTargets; + uint256[] alphaValues; + string[] alphaSignatures; + bytes[] alphaCalldatas; + string alphaDescription; + uint256 bravoProposalId; + address[] bravoTargets; + uint256[] bravoValues; + bytes[] bravoCalldatas; + string bravoDescription; + } + + function testFuzz_NewGovernorUnaffectedByVotesOnOldGovernor(uint256 _amount, address _receiver) public { + NewGovernorUnaffectedByVotesOnOldGovernorVars memory _vars; + IERC20 _token = _randomERC20Token(_amount); + assumeReceiver(_receiver); + + upgradeToBravoGovernor(); + + // Create a new proposal to send the token. + _vars.alphaTargets = new address[](1); + _vars.alphaValues = new uint256[](1); + _vars.alphaSignatures = new string [](1); + _vars.alphaCalldatas = new bytes[](1); + _vars.alphaDescription = "Transfer some tokens from the new Governor"; + + _vars.alphaTargets[0] = address(_token); + _vars.alphaSignatures[0] = "transfer(address,uint256)"; + _vars.alphaCalldatas[0] = abi.encode(_receiver, _amount); + + // Submit the new proposal to Governor Alpha, which is now deprecated. + vm.prank(PROPOSER); + _vars.alphaProposalId = governorAlpha.propose( + _vars.alphaTargets, + _vars.alphaValues, + _vars.alphaSignatures, + _vars.alphaCalldatas, + _vars.alphaDescription + ); + + // Now construct and submit an identical proposal on Governor Bravo, which is active. + ( + _vars.bravoProposalId, + _vars.bravoTargets, + _vars.bravoValues, + _vars.bravoCalldatas, + _vars.bravoDescription + ) = submitTokenSendProposal(address(_token), _amount, _receiver); + + assertEq(governor.state(_vars.bravoProposalId), IGovernor.ProposalState.Pending); + assertEq( + uint8(governorAlpha.state(_vars.alphaProposalId)), + uint8(governor.state(_vars.bravoProposalId)) + ); + + jumpToActiveProposal(_vars.bravoProposalId); + + // Defeat the proposal on Bravo. + assertEq(governor.state(_vars.bravoProposalId), IGovernor.ProposalState.Active); + delegatesVoteOnBravoGovernor(_vars.bravoProposalId, AGAINST); + + // Pass the proposal on Alpha. + for (uint256 _index = 0; _index < delegates.length; _index++) { + vm.prank(delegates[_index]); + governorAlpha.castVote(_vars.alphaProposalId, true); + } + + jumpToVotingComplete(_vars.bravoProposalId); + + // Ensure the Bravo proposal has failed and Alpha has succeeded. + assertEq(governor.state(_vars.bravoProposalId), IGovernor.ProposalState.Defeated); + assertEq(governorAlpha.state(_vars.alphaProposalId), uint8(IGovernor.ProposalState.Succeeded)); + + // It should not be possible to queue either proposal, confirming that votes + // on alpha do not affect votes on bravo. + vm.expectRevert("Governor: proposal not successful"); + governor.queue(_vars.bravoTargets, _vars.bravoValues, _vars.bravoCalldatas, keccak256(bytes(_vars.bravoDescription))); + vm.expectRevert("Timelock::queueTransaction: Call must come from admin."); + governorAlpha.queue(_vars.alphaProposalId); + } } From 659f1491fb785a45cd68add65654fd0a4003c969 Mon Sep 17 00:00:00 2001 From: David Laprade Date: Fri, 13 Jan 2023 15:13:19 -0500 Subject: [PATCH 09/27] s/governor/governorBravo/g --- test/GitcoinGovernor.t.sol | 146 ++++++++++++++++++------------------- 1 file changed, 73 insertions(+), 73 deletions(-) diff --git a/test/GitcoinGovernor.t.sol b/test/GitcoinGovernor.t.sol index 87dbce3..280d793 100644 --- a/test/GitcoinGovernor.t.sol +++ b/test/GitcoinGovernor.t.sol @@ -16,7 +16,7 @@ contract GitcoinGovernorTestHelper is Test, DeployInput { address constant TIMELOCK = 0x57a8865cfB1eCEf7253c27da6B4BC3dAEE5Be518; address constant PROPOSER = 0xc2E2B715d9e302947Ec7e312fd2384b5a1296099; // kbw.eth - GitcoinGovernor governor; + GitcoinGovernor governorBravo; function setUp() public virtual { uint256 _forkBlock = 15_980_096; // The latest block when this test was written @@ -24,20 +24,20 @@ contract GitcoinGovernorTestHelper is Test, DeployInput { DeployScript _deployScript = new DeployScript(); _deployScript.setUp(); - governor = _deployScript.run(); + governorBravo = _deployScript.run(); } } contract GitcoinGovernorDeployTest is GitcoinGovernorTestHelper { function testFuzz_deployment(uint256 _blockNumber) public { - assertEq(governor.name(), "GTC Governor Bravo"); - assertEq(address(governor.token()), GTC_TOKEN); - assertEq(governor.votingDelay(), INITIAL_VOTING_DELAY); - assertEq(governor.votingPeriod(), INITIAL_VOTING_PERIOD); - assertEq(governor.proposalThreshold(), INITIAL_PROPOSAL_THRESHOLD); - assertEq(governor.quorum(_blockNumber), QUORUM); - assertEq(governor.timelock(), TIMELOCK); - assertEq(governor.COUNTING_MODE(), "support=bravo&quorum=for,abstain"); + assertEq(governorBravo.name(), "GTC Governor Bravo"); + assertEq(address(governorBravo.token()), GTC_TOKEN); + assertEq(governorBravo.votingDelay(), INITIAL_VOTING_DELAY); + assertEq(governorBravo.votingPeriod(), INITIAL_VOTING_PERIOD); + assertEq(governorBravo.proposalThreshold(), INITIAL_PROPOSAL_THRESHOLD); + assertEq(governorBravo.quorum(_blockNumber), QUORUM); + assertEq(governorBravo.timelock(), TIMELOCK); + assertEq(governorBravo.COUNTING_MODE(), "support=bravo&quorum=for,abstain"); } } @@ -76,7 +76,7 @@ contract GitcoinGovernorProposalTestHelper is GitcoinGovernorTestHelper { initialProposalCount = governorAlpha.proposalCount(); ProposeScript _proposeScript = new ProposeScript(); - upgradeProposalId = _proposeScript.run(governor); + upgradeProposalId = _proposeScript.run(governorBravo); } //--------------- HELPERS ---------------// @@ -158,7 +158,7 @@ contract GitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { ) = governorAlpha.getActions(upgradeProposalId); assertEq(_targets.length, 2); assertEq(_targets[0], TIMELOCK); - assertEq(_targets[1], address(governor)); + assertEq(_targets[1], address(governorBravo)); assertEq(_values.length, 2); assertEq(_values[0], 0); assertEq(_values[1], 0); @@ -166,7 +166,7 @@ contract GitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { assertEq(_signatures[0], "setPendingAdmin(address)"); assertEq(_signatures[1], "__acceptAdmin()"); assertEq(_calldatas.length, 2); - assertEq(_calldatas[0], abi.encode(address(governor))); + assertEq(_calldatas[0], abi.encode(address(governorBravo))); assertEq(_calldatas[1], ""); } @@ -247,8 +247,8 @@ contract GitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { uint8 _state = governorAlpha.state(upgradeProposalId); assertEq(_state, EXECUTED); - // Ensure the governor is now the admin of the timelock - assertEq(timelock.admin(), address(governor)); + // Ensure the governorBravo is now the admin of the timelock + assertEq(timelock.admin(), address(governorBravo)); } } @@ -448,18 +448,18 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { } function jumpToActiveProposal(uint256 _proposalId) public { - uint256 _snapshot = governor.proposalSnapshot(_proposalId); + uint256 _snapshot = governorBravo.proposalSnapshot(_proposalId); vm.roll(_snapshot + 1); } function jumpToVotingComplete(uint256 _proposalId) public { // Jump one block past the proposal voting deadline - uint256 _deadline = governor.proposalDeadline(_proposalId); + uint256 _deadline = governorBravo.proposalDeadline(_proposalId); vm.roll(_deadline + 1); } function jumpPastProposalEta(uint256 _proposalId) public { - uint256 _eta = governor.proposalEta(_proposalId); + uint256 _eta = governorBravo.proposalEta(_proposalId); vm.roll(block.number + 1); vm.warp(_eta + 1); } @@ -469,7 +469,7 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { for (uint256 _index = 0; _index < delegates.length; _index++) { vm.prank(delegates[_index]); - governor.castVote(_proposalId, _support); + governorBravo.castVote(_proposalId, _support); } } @@ -490,7 +490,7 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { // Submit the new proposal vm.prank(PROPOSER); - uint256 _newProposalId = governor.propose(_targets, _values, _calldatas, _description); + uint256 _newProposalId = governorBravo.propose(_targets, _values, _calldatas, _description); return (_newProposalId, _targets, _values, _calldatas, _description); } @@ -511,7 +511,7 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { (uint256 _newProposalId,,,,) = submitTokenSendProposal(address(gtcToken), _gtcAmount, _gtcReceiver); // Ensure proposal is in the expected state - IGovernor.ProposalState _state = governor.state(_newProposalId); + IGovernor.ProposalState _state = governorBravo.state(_newProposalId); assertEq(_state, IGovernor.ProposalState.Pending); } @@ -535,25 +535,25 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { ) = submitTokenSendProposal(address(_token), _amount, _receiver); // Ensure proposal is in the expected state - IGovernor.ProposalState _state = governor.state(_newProposalId); + IGovernor.ProposalState _state = governorBravo.state(_newProposalId); assertEq(_state, IGovernor.ProposalState.Pending); jumpToActiveProposal(_newProposalId); // Ensure the proposal is now Active - _state = governor.state(_newProposalId); + _state = governorBravo.state(_newProposalId); assertEq(_state, IGovernor.ProposalState.Active); delegatesVoteOnBravoGovernor(_newProposalId, AGAINST); jumpToVotingComplete(_newProposalId); // Ensure the proposal has failed - _state = governor.state(_newProposalId); + _state = governorBravo.state(_newProposalId); assertEq(_state, IGovernor.ProposalState.Defeated); // It should not be possible to queue the proposal vm.expectRevert("Governor: proposal not successful"); - governor.queue(_targets, _values, _calldatas, keccak256(bytes(_description))); + governorBravo.queue(_targets, _values, _calldatas, keccak256(bytes(_description))); } function testFuzz_NewGovernorCanPassProposalToSendToken(uint256 _amount, address _receiver) @@ -577,36 +577,36 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { ) = submitTokenSendProposal(address(_token), _amount, _receiver); // Ensure proposal is in the expected state - IGovernor.ProposalState _state = governor.state(_newProposalId); + IGovernor.ProposalState _state = governorBravo.state(_newProposalId); assertEq(_state, IGovernor.ProposalState.Pending); jumpToActiveProposal(_newProposalId); // Ensure the proposal is now Active - _state = governor.state(_newProposalId); + _state = governorBravo.state(_newProposalId); assertEq(_state, IGovernor.ProposalState.Active); delegatesVoteOnBravoGovernor(_newProposalId, FOR); jumpToVotingComplete(_newProposalId); // Ensure the proposal has succeeded - _state = governor.state(_newProposalId); + _state = governorBravo.state(_newProposalId); assertEq(_state, IGovernor.ProposalState.Succeeded); // Queue the proposal - governor.queue(_targets, _values, _calldatas, keccak256(bytes(_description))); + governorBravo.queue(_targets, _values, _calldatas, keccak256(bytes(_description))); // Ensure the proposal is queued - _state = governor.state(_newProposalId); + _state = governorBravo.state(_newProposalId); assertEq(_state, IGovernor.ProposalState.Queued); jumpPastProposalEta(_newProposalId); // Execute the proposal - governor.execute(_targets, _values, _calldatas, keccak256(bytes(_description))); + governorBravo.execute(_targets, _values, _calldatas, keccak256(bytes(_description))); // Ensure the proposal is executed - _state = governor.state(_newProposalId); + _state = governorBravo.state(_newProposalId); assertEq(_state, IGovernor.ProposalState.Executed); // Ensure the tokens have been transferred @@ -631,56 +631,56 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { bytes[] memory _calldatas = new bytes[](3); string memory _description = "Update governance settings"; - _targets[0] = address(governor); + _targets[0] = address(governorBravo); _calldatas[0] = buildProposalData("setVotingDelay(uint256)", abi.encode(_newDelay)); - _targets[1] = address(governor); + _targets[1] = address(governorBravo); _calldatas[1] = buildProposalData("setVotingPeriod(uint256)", abi.encode(_newVotingPeriod)); - _targets[2] = address(governor); + _targets[2] = address(governorBravo); _calldatas[2] = buildProposalData("setProposalThreshold(uint256)", abi.encode(_newProposalThreshold)); // Submit the new proposal vm.prank(PROPOSER); - uint256 _newProposalId = governor.propose(_targets, _values, _calldatas, _description); + uint256 _newProposalId = governorBravo.propose(_targets, _values, _calldatas, _description); // Ensure proposal is in the expected state - IGovernor.ProposalState _state = governor.state(_newProposalId); + IGovernor.ProposalState _state = governorBravo.state(_newProposalId); assertEq(_state, IGovernor.ProposalState.Pending); jumpToActiveProposal(_newProposalId); // Ensure the proposal is now Active - _state = governor.state(_newProposalId); + _state = governorBravo.state(_newProposalId); assertEq(_state, IGovernor.ProposalState.Active); delegatesVoteOnBravoGovernor(_newProposalId, FOR); jumpToVotingComplete(_newProposalId); // Ensure the proposal has succeeded - _state = governor.state(_newProposalId); + _state = governorBravo.state(_newProposalId); assertEq(_state, IGovernor.ProposalState.Succeeded); // Queue the proposal - governor.queue(_targets, _values, _calldatas, keccak256(bytes(_description))); + governorBravo.queue(_targets, _values, _calldatas, keccak256(bytes(_description))); // Ensure the proposal is queued - _state = governor.state(_newProposalId); + _state = governorBravo.state(_newProposalId); assertEq(_state, IGovernor.ProposalState.Queued); jumpPastProposalEta(_newProposalId); // Execute the proposal - governor.execute(_targets, _values, _calldatas, keccak256(bytes(_description))); + governorBravo.execute(_targets, _values, _calldatas, keccak256(bytes(_description))); // Ensure the proposal is executed - _state = governor.state(_newProposalId); + _state = governorBravo.state(_newProposalId); assertEq(_state, IGovernor.ProposalState.Executed); // Confirm that governance settings have updated. - assertEq(governor.votingDelay(), _newDelay); - assertEq(governor.votingPeriod(), _newVotingPeriod); - assertEq(governor.proposalThreshold(), _newProposalThreshold); + assertEq(governorBravo.votingDelay(), _newDelay); + assertEq(governorBravo.votingPeriod(), _newVotingPeriod); + assertEq(governorBravo.proposalThreshold(), _newProposalThreshold); } function testFuzz_NewGovernorCanPassMixedProposal(uint256 _amount, address _receiver) public { @@ -702,45 +702,45 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { ) = submitTokenSendProposal(address(_token), _amount, _receiver); // Ensure proposal is in the expected state - IGovernor.ProposalState _state = governor.state(_newProposalId); + IGovernor.ProposalState _state = governorBravo.state(_newProposalId); assertEq(_state, IGovernor.ProposalState.Pending); jumpToActiveProposal(_newProposalId); // Ensure the proposal is now Active - _state = governor.state(_newProposalId); + _state = governorBravo.state(_newProposalId); assertEq(_state, IGovernor.ProposalState.Active); // Delegates vote with a mix of For/Against/Abstain with For winning. vm.prank(PROPOSER); // kbw.eth (~1.8M votes) - governor.castVote(_newProposalId, FOR); + governorBravo.castVote(_newProposalId, FOR); vm.prank(0x2df9a188fBE231B0DC36D14AcEb65dEFbB049479); // janineleger.eth (~1.53M) - governor.castVote(_newProposalId, FOR); + governorBravo.castVote(_newProposalId, FOR); vm.prank(0x4Be88f63f919324210ea3A2cCAD4ff0734425F91); // kevinolsen.eth (~1.35M) - governor.castVote(_newProposalId, FOR); + governorBravo.castVote(_newProposalId, FOR); vm.prank(0x34aA3F359A9D614239015126635CE7732c18fDF3); // Austin Griffith (~1.05M) - governor.castVote(_newProposalId, AGAINST); + governorBravo.castVote(_newProposalId, AGAINST); vm.prank(0x7E052Ef7B4bB7E5A45F331128AFadB1E589deaF1); // Kris Is (~1.05M) - governor.castVote(_newProposalId, ABSTAIN); + governorBravo.castVote(_newProposalId, ABSTAIN); vm.prank(0x5e349eca2dc61aBCd9dD99Ce94d04136151a09Ee); // Linda Xie (~1.02M) - governor.castVote(_newProposalId, AGAINST); + governorBravo.castVote(_newProposalId, AGAINST); jumpToVotingComplete(_newProposalId); // Ensure the proposal has succeeded - _state = governor.state(_newProposalId); + _state = governorBravo.state(_newProposalId); assertEq(_state, IGovernor.ProposalState.Succeeded); // Queue the proposal - governor.queue(_targets, _values, _calldatas, keccak256(bytes(_description))); + governorBravo.queue(_targets, _values, _calldatas, keccak256(bytes(_description))); jumpPastProposalEta(_newProposalId); // Execute the proposal - governor.execute(_targets, _values, _calldatas, keccak256(bytes(_description))); + governorBravo.execute(_targets, _values, _calldatas, keccak256(bytes(_description))); // Ensure the proposal is executed - _state = governor.state(_newProposalId); + _state = governorBravo.state(_newProposalId); assertEq(_state, IGovernor.ProposalState.Executed); // Ensure the tokens have been transferred @@ -766,38 +766,38 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { ) = submitTokenSendProposal(address(_token), _amount, _receiver); // Ensure proposal is in the expected state - IGovernor.ProposalState _state = governor.state(_newProposalId); + IGovernor.ProposalState _state = governorBravo.state(_newProposalId); assertEq(_state, IGovernor.ProposalState.Pending); jumpToActiveProposal(_newProposalId); // Ensure the proposal is now Active - _state = governor.state(_newProposalId); + _state = governorBravo.state(_newProposalId); assertEq(_state, IGovernor.ProposalState.Active); // Delegates vote with a mix of For/Against/Abstain with Against/Abstain winning. vm.prank(PROPOSER); // kbw.eth (~1.8M votes) - governor.castVote(_newProposalId, ABSTAIN); + governorBravo.castVote(_newProposalId, ABSTAIN); vm.prank(0x2df9a188fBE231B0DC36D14AcEb65dEFbB049479); // janineleger.eth (~1.53M) - governor.castVote(_newProposalId, FOR); + governorBravo.castVote(_newProposalId, FOR); vm.prank(0x4Be88f63f919324210ea3A2cCAD4ff0734425F91); // kevinolsen.eth (~1.35M) - governor.castVote(_newProposalId, FOR); + governorBravo.castVote(_newProposalId, FOR); vm.prank(0x34aA3F359A9D614239015126635CE7732c18fDF3); // Austin Griffith (~1.05M) - governor.castVote(_newProposalId, AGAINST); + governorBravo.castVote(_newProposalId, AGAINST); vm.prank(0x7E052Ef7B4bB7E5A45F331128AFadB1E589deaF1); // Kris Is (~1.05M) - governor.castVote(_newProposalId, AGAINST); + governorBravo.castVote(_newProposalId, AGAINST); vm.prank(0x5e349eca2dc61aBCd9dD99Ce94d04136151a09Ee); // Linda Xie (~1.02M) - governor.castVote(_newProposalId, AGAINST); + governorBravo.castVote(_newProposalId, AGAINST); jumpToVotingComplete(_newProposalId); // Ensure the proposal has failed - _state = governor.state(_newProposalId); + _state = governorBravo.state(_newProposalId); assertEq(_state, IGovernor.ProposalState.Defeated); // It should not be possible to queue the proposal vm.expectRevert("Governor: proposal not successful"); - governor.queue(_targets, _values, _calldatas, keccak256(bytes(_description))); + governorBravo.queue(_targets, _values, _calldatas, keccak256(bytes(_description))); } struct NewGovernorUnaffectedByVotesOnOldGovernorVars { @@ -851,16 +851,16 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { _vars.bravoDescription ) = submitTokenSendProposal(address(_token), _amount, _receiver); - assertEq(governor.state(_vars.bravoProposalId), IGovernor.ProposalState.Pending); + assertEq(governorBravo.state(_vars.bravoProposalId), IGovernor.ProposalState.Pending); assertEq( uint8(governorAlpha.state(_vars.alphaProposalId)), - uint8(governor.state(_vars.bravoProposalId)) + uint8(governorBravo.state(_vars.bravoProposalId)) ); jumpToActiveProposal(_vars.bravoProposalId); // Defeat the proposal on Bravo. - assertEq(governor.state(_vars.bravoProposalId), IGovernor.ProposalState.Active); + assertEq(governorBravo.state(_vars.bravoProposalId), IGovernor.ProposalState.Active); delegatesVoteOnBravoGovernor(_vars.bravoProposalId, AGAINST); // Pass the proposal on Alpha. @@ -872,13 +872,13 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { jumpToVotingComplete(_vars.bravoProposalId); // Ensure the Bravo proposal has failed and Alpha has succeeded. - assertEq(governor.state(_vars.bravoProposalId), IGovernor.ProposalState.Defeated); + assertEq(governorBravo.state(_vars.bravoProposalId), IGovernor.ProposalState.Defeated); assertEq(governorAlpha.state(_vars.alphaProposalId), uint8(IGovernor.ProposalState.Succeeded)); // It should not be possible to queue either proposal, confirming that votes // on alpha do not affect votes on bravo. vm.expectRevert("Governor: proposal not successful"); - governor.queue(_vars.bravoTargets, _vars.bravoValues, _vars.bravoCalldatas, keccak256(bytes(_vars.bravoDescription))); + governorBravo.queue(_vars.bravoTargets, _vars.bravoValues, _vars.bravoCalldatas, keccak256(bytes(_vars.bravoDescription))); vm.expectRevert("Timelock::queueTransaction: Call must come from admin."); governorAlpha.queue(_vars.alphaProposalId); } From 815c79b243defdd0cf77d3448833ed1f9065f04f Mon Sep 17 00:00:00 2001 From: David Laprade Date: Fri, 13 Jan 2023 15:14:15 -0500 Subject: [PATCH 10/27] scopelint --- test/GitcoinGovernor.t.sol | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/test/GitcoinGovernor.t.sol b/test/GitcoinGovernor.t.sol index 280d793..0450458 100644 --- a/test/GitcoinGovernor.t.sol +++ b/test/GitcoinGovernor.t.sol @@ -484,8 +484,7 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { _targets[0] = _token; _values[0] = 0; - _calldatas[0] = - buildProposalData("transfer(address,uint256)", abi.encode(_receiver, _amount)); + _calldatas[0] = buildProposalData("transfer(address,uint256)", abi.encode(_receiver, _amount)); string memory _description = "Transfer some tokens from the new Governor"; // Submit the new proposal @@ -508,14 +507,15 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { { assumeReceiver(_gtcReceiver); upgradeToBravoGovernor(); - (uint256 _newProposalId,,,,) = submitTokenSendProposal(address(gtcToken), _gtcAmount, _gtcReceiver); + (uint256 _newProposalId,,,,) = + submitTokenSendProposal(address(gtcToken), _gtcAmount, _gtcReceiver); // Ensure proposal is in the expected state IGovernor.ProposalState _state = governorBravo.state(_newProposalId); assertEq(_state, IGovernor.ProposalState.Pending); } - function _randomERC20Token(uint256 _seed) internal view returns(IERC20 _token) { + function _randomERC20Token(uint256 _seed) internal view returns (IERC20 _token) { if (_seed % 3 == 0) _token = IERC20(address(gtcToken)); if (_seed % 3 == 1) _token = usdcToken; if (_seed % 3 == 2) _token = radToken; @@ -638,7 +638,8 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { _calldatas[1] = buildProposalData("setVotingPeriod(uint256)", abi.encode(_newVotingPeriod)); _targets[2] = address(governorBravo); - _calldatas[2] = buildProposalData("setProposalThreshold(uint256)", abi.encode(_newProposalThreshold)); + _calldatas[2] = + buildProposalData("setProposalThreshold(uint256)", abi.encode(_newProposalThreshold)); // Submit the new proposal vm.prank(PROPOSER); @@ -814,7 +815,9 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { string bravoDescription; } - function testFuzz_NewGovernorUnaffectedByVotesOnOldGovernor(uint256 _amount, address _receiver) public { + function testFuzz_NewGovernorUnaffectedByVotesOnOldGovernor(uint256 _amount, address _receiver) + public + { NewGovernorUnaffectedByVotesOnOldGovernorVars memory _vars; IERC20 _token = _randomERC20Token(_amount); assumeReceiver(_receiver); @@ -878,7 +881,12 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { // It should not be possible to queue either proposal, confirming that votes // on alpha do not affect votes on bravo. vm.expectRevert("Governor: proposal not successful"); - governorBravo.queue(_vars.bravoTargets, _vars.bravoValues, _vars.bravoCalldatas, keccak256(bytes(_vars.bravoDescription))); + governorBravo.queue( + _vars.bravoTargets, + _vars.bravoValues, + _vars.bravoCalldatas, + keccak256(bytes(_vars.bravoDescription)) + ); vm.expectRevert("Timelock::queueTransaction: Call must come from admin."); governorAlpha.queue(_vars.alphaProposalId); } From 64f2a4379d4ff1983eb529fd43d9b7e0e552afca Mon Sep 17 00:00:00 2001 From: David Laprade Date: Fri, 13 Jan 2023 16:03:35 -0500 Subject: [PATCH 11/27] Cache fork requests https://twitter.com/ultrasecreth/status/1603430600442101761/photo/1 --- .github/workflows/ci.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c5a0bb4..fa4b35a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,6 +40,14 @@ jobs: with: version: nightly + - name: Cache fork requests + uses: actions/cache@v3 + with: + path: ~/.foundry/cache + key: ${{ runner.os }}-foundry-network-fork-${{ github.sha }} + restore-keys: | + ${{ runner.os }}-foundry-network-fork- + - name: Run tests run: forge test From f10554f4e500db61db5a09db155209e8d0c67f58 Mon Sep 17 00:00:00 2001 From: David Laprade Date: Fri, 13 Jan 2023 16:36:49 -0500 Subject: [PATCH 12/27] Add a seed to fuzz deterministically in CI --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fa4b35a..b610856 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,6 +32,7 @@ jobs: DEPLOYER_PRIVATE_KEY: ${{ secrets.DEPLOYER_PRIVATE_KEY }} MAINNET_RPC_URL: ${{ secrets.MAINNET_RPC_URL }} PROPOSER_PRIVATE_KEY: ${{ secrets.PROPOSER_PRIVATE_KEY }} + FOUNDRY_FUZZ_SEED: 58636 steps: - uses: actions/checkout@v3 From b41f7ad88f1ea2fd117586431a19cd7cd16b435c Mon Sep 17 00:00:00 2001 From: David Laprade Date: Mon, 16 Jan 2023 13:24:03 -0500 Subject: [PATCH 13/27] scopelint --- src/GitcoinGovernor.sol | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/GitcoinGovernor.sol b/src/GitcoinGovernor.sol index b9fc14f..395208f 100644 --- a/src/GitcoinGovernor.sol +++ b/src/GitcoinGovernor.sol @@ -54,7 +54,7 @@ contract GitcoinGovernor is public view virtual - override (Governor, GovernorTimelockCompound) + override(Governor, GovernorTimelockCompound) returns (bool) { return GovernorTimelockCompound.supportsInterface(interfaceId); @@ -65,7 +65,7 @@ contract GitcoinGovernor is public view virtual - override (Governor, GovernorSettings) + override(Governor, GovernorSettings) returns (uint256) { return GovernorSettings.proposalThreshold(); @@ -76,7 +76,7 @@ contract GitcoinGovernor is public view virtual - override (Governor, GovernorTimelockCompound) + override(Governor, GovernorTimelockCompound) returns (ProposalState) { return GovernorTimelockCompound.state(proposalId); @@ -96,7 +96,7 @@ contract GitcoinGovernor is uint256[] memory values, bytes[] memory calldatas, bytes32 descriptionHash - ) internal virtual override (Governor, GovernorTimelockCompound) { + ) internal virtual override(Governor, GovernorTimelockCompound) { return GovernorTimelockCompound._execute(proposalId, targets, values, calldatas, descriptionHash); } @@ -107,7 +107,7 @@ contract GitcoinGovernor is uint256[] memory values, bytes[] memory calldatas, bytes32 descriptionHash - ) internal virtual override (Governor, GovernorTimelockCompound) returns (uint256) { + ) internal virtual override(Governor, GovernorTimelockCompound) returns (uint256) { return GovernorTimelockCompound._cancel(targets, values, calldatas, descriptionHash); } @@ -116,7 +116,7 @@ contract GitcoinGovernor is internal view virtual - override (Governor, GovernorTimelockCompound) + override(Governor, GovernorTimelockCompound) returns (address) { return GovernorTimelockCompound._executor(); From cce818555e8a52bf3199aa322ba49a9cc52a79a5 Mon Sep 17 00:00:00 2001 From: David Laprade Date: Mon, 16 Jan 2023 14:48:16 -0500 Subject: [PATCH 14/27] Troubleshoot CI --- .github/workflows/ci.yml | 2 +- foundry.toml | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b610856..6135908 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,7 +32,7 @@ jobs: DEPLOYER_PRIVATE_KEY: ${{ secrets.DEPLOYER_PRIVATE_KEY }} MAINNET_RPC_URL: ${{ secrets.MAINNET_RPC_URL }} PROPOSER_PRIVATE_KEY: ${{ secrets.PROPOSER_PRIVATE_KEY }} - FOUNDRY_FUZZ_SEED: 58636 + FOUNDRY_FUZZ_SEED: 1673481600 steps: - uses: actions/checkout@v3 diff --git a/foundry.toml b/foundry.toml index 0cb3b6b..e6f2fb5 100644 --- a/foundry.toml +++ b/foundry.toml @@ -5,11 +5,11 @@ verbosity = 3 [profile.ci] - fuzz = { runs = 5000 } - invariant = { runs = 1000 } + fuzz = { runs = 5 } + invariant = { runs = 5 } [profile.lite] - fuzz = { runs = 50 } + fuzz = { runs = 50, seed = 1673481600 } invariant = { runs = 10 } # Speed up compilation and tests during development. optimizer = false From bc88087c41d0cc5d031f944924c8b6a101db2602 Mon Sep 17 00:00:00 2001 From: David Laprade Date: Mon, 16 Jan 2023 15:02:11 -0500 Subject: [PATCH 15/27] Cycle CI fuzz seeds each week --- .github/workflows/ci.yml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6135908..3d8919f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,7 +32,6 @@ jobs: DEPLOYER_PRIVATE_KEY: ${{ secrets.DEPLOYER_PRIVATE_KEY }} MAINNET_RPC_URL: ${{ secrets.MAINNET_RPC_URL }} PROPOSER_PRIVATE_KEY: ${{ secrets.PROPOSER_PRIVATE_KEY }} - FOUNDRY_FUZZ_SEED: 1673481600 steps: - uses: actions/checkout@v3 @@ -49,6 +48,13 @@ jobs: restore-keys: | ${{ runner.os }}-foundry-network-fork- + # https://twitter.com/PaulRBerg/status/1611116650664796166 + - name: Generate fuzz seed with 1 week TTL + run: > + echo "FOUNDRY_FUZZ_SEED=$( + echo $(($EPOCHSECONDS - $EPOCHSECONDS % 604800)) + )" >> $GITHUB_ENV + - name: Run tests run: forge test From 4d58ea0c5e5262f0b7c673142ebb88d2f0fe6475 Mon Sep 17 00:00:00 2001 From: David Laprade Date: Mon, 16 Jan 2023 15:02:59 -0500 Subject: [PATCH 16/27] Bump fuzz tests in CI --- foundry.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/foundry.toml b/foundry.toml index e6f2fb5..27eec7d 100644 --- a/foundry.toml +++ b/foundry.toml @@ -5,8 +5,8 @@ verbosity = 3 [profile.ci] - fuzz = { runs = 5 } - invariant = { runs = 5 } + fuzz = { runs = 500 } + invariant = { runs = 500 } [profile.lite] fuzz = { runs = 50, seed = 1673481600 } From 629f1ca934c04cdbea0c5bc383e185ce85a869d8 Mon Sep 17 00:00:00 2001 From: David Laprade Date: Mon, 16 Jan 2023 15:20:15 -0500 Subject: [PATCH 17/27] Remove fuzz seed from .env.template --- .env.template | 1 - 1 file changed, 1 deletion(-) diff --git a/.env.template b/.env.template index 18f9d12..fb5d2d7 100644 --- a/.env.template +++ b/.env.template @@ -1,4 +1,3 @@ DEPLOYER_PRIVATE_KEY= PROPOSER_PRIVATE_KEY= MAINNET_RPC_URL= -FOUNDRY_FUZZ_SEED= From 8d1ac767b13e9e5b51f451455cf48d15c844aa63 Mon Sep 17 00:00:00 2001 From: David Laprade Date: Mon, 16 Jan 2023 16:05:58 -0500 Subject: [PATCH 18/27] Modify based on PR feedback --- test/GitcoinGovernor.t.sol | 54 ++++++++++++++++++++++++-------------- 1 file changed, 35 insertions(+), 19 deletions(-) diff --git a/test/GitcoinGovernor.t.sol b/test/GitcoinGovernor.t.sol index 0450458..25a6cf3 100644 --- a/test/GitcoinGovernor.t.sol +++ b/test/GitcoinGovernor.t.sol @@ -436,6 +436,12 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { uint8 constant ABSTAIN = 2; function assumeReceiver(address _receiver) public { + // We don't want the receiver to be the Timelock, as that would make our + // assertions less meaningful -- most of our tests want to confirm that + // proposals can cause tokens to be sent *from* the timelock to somewhere + // else. We also don't want the zero address as a receiver -- if our + // governor is sending tokens to the zero address it could just be due to a + // bug. vm.assume(_receiver != TIMELOCK && _receiver != address(0x0)); } @@ -465,7 +471,7 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { } function delegatesVoteOnBravoGovernor(uint256 _proposalId, uint8 _support) public { - assertLt(_support, 3, "Invalid value for support"); + require(_support < 3, "Invalid value for support"); for (uint256 _index = 0; _index < delegates.length; _index++) { vm.prank(delegates[_index]); @@ -498,10 +504,6 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { assertEq(uint8(_actual), uint8(_expected)); } - function setUp() public override { - GitcoinGovernorProposalTestHelper.setUp(); - } - function testFuzz_NewGovernorCanReceiveNewProposal(uint256 _gtcAmount, address _gtcReceiver) public { @@ -521,8 +523,10 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { if (_seed % 3 == 2) _token = radToken; } - function testFuzz_NewGovernorCanDefeatProposal(uint256 _amount, address _receiver) public { - IERC20 _token = _randomERC20Token(_amount); + function testFuzz_NewGovernorCanDefeatProposal(uint256 _amount, address _receiver, uint256 _seed) + public + { + IERC20 _token = _randomERC20Token(_seed); assumeReceiver(_receiver); upgradeToBravoGovernor(); @@ -556,10 +560,12 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { governorBravo.queue(_targets, _values, _calldatas, keccak256(bytes(_description))); } - function testFuzz_NewGovernorCanPassProposalToSendToken(uint256 _amount, address _receiver) - public - { - IERC20 _token = _randomERC20Token(_amount); + function testFuzz_NewGovernorCanPassProposalToSendToken( + uint256 _amount, + address _receiver, + uint256 _seed + ) public { + IERC20 _token = _randomERC20Token(_seed); assumeReceiver(_receiver); uint256 _timelockTokenBalance = _token.balanceOf(TIMELOCK); @@ -684,8 +690,12 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { assertEq(governorBravo.proposalThreshold(), _newProposalThreshold); } - function testFuzz_NewGovernorCanPassMixedProposal(uint256 _amount, address _receiver) public { - IERC20 _token = _randomERC20Token(_amount); + function testFuzz_NewGovernorCanPassMixedProposal( + uint256 _amount, + address _receiver, + uint256 _seed + ) public { + IERC20 _token = _randomERC20Token(_seed); assumeReceiver(_receiver); uint256 _timelockTokenBalance = _token.balanceOf(TIMELOCK); @@ -749,8 +759,12 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { assertEq(_token.balanceOf(TIMELOCK), _timelockTokenBalance - _amount); } - function testFuzz_NewGovernorCanDefeatMixedProposal(uint256 _amount, address _receiver) public { - IERC20 _token = _randomERC20Token(_amount); + function testFuzz_NewGovernorCanDefeatMixedProposal( + uint256 _amount, + address _receiver, + uint256 _seed + ) public { + IERC20 _token = _randomERC20Token(_seed); assumeReceiver(_receiver); uint256 _timelockTokenBalance = _token.balanceOf(TIMELOCK); @@ -815,11 +829,13 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { string bravoDescription; } - function testFuzz_NewGovernorUnaffectedByVotesOnOldGovernor(uint256 _amount, address _receiver) - public - { + function testFuzz_NewGovernorUnaffectedByVotesOnOldGovernor( + uint256 _amount, + address _receiver, + uint256 _seed + ) public { NewGovernorUnaffectedByVotesOnOldGovernorVars memory _vars; - IERC20 _token = _randomERC20Token(_amount); + IERC20 _token = _randomERC20Token(_seed); assumeReceiver(_receiver); upgradeToBravoGovernor(); From d7b457000a76cc66417b7adeb4c124e4f8c2a062 Mon Sep 17 00:00:00 2001 From: David Laprade Date: Mon, 16 Jan 2023 16:14:25 -0500 Subject: [PATCH 19/27] Foundry toolchain caches fork requests for us --- .github/workflows/ci.yml | 8 -------- 1 file changed, 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3d8919f..3500bd1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,14 +40,6 @@ jobs: with: version: nightly - - name: Cache fork requests - uses: actions/cache@v3 - with: - path: ~/.foundry/cache - key: ${{ runner.os }}-foundry-network-fork-${{ github.sha }} - restore-keys: | - ${{ runner.os }}-foundry-network-fork- - # https://twitter.com/PaulRBerg/status/1611116650664796166 - name: Generate fuzz seed with 1 week TTL run: > From adffed2a9ab4502ed573c5ded89ec961b9f06eb2 Mon Sep 17 00:00:00 2001 From: Matt Solomon Date: Tue, 17 Jan 2023 07:38:38 -0800 Subject: [PATCH 20/27] ci: test if manually creating cache dir solves the issue --- .github/workflows/ci.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3500bd1..6a49a21 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,6 +40,9 @@ jobs: with: version: nightly + - name: Create cache folder + run: mkdir -p ~/.foundry/cache/rpc + # https://twitter.com/PaulRBerg/status/1611116650664796166 - name: Generate fuzz seed with 1 week TTL run: > From 3f56e3d6cebd5b1edca7cd0a101682ccc169d92e Mon Sep 17 00:00:00 2001 From: David Laprade Date: Tue, 17 Jan 2023 11:30:59 -0500 Subject: [PATCH 21/27] Fix coverage workflow --- .github/workflows/ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6a49a21..121f25b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -54,6 +54,8 @@ jobs: run: forge test coverage: + env: + MAINNET_RPC_URL: ${{ secrets.MAINNET_RPC_URL }} runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 From 114ccca2a306105330b5c615b7e217d63deb160c Mon Sep 17 00:00:00 2001 From: David Laprade Date: Tue, 17 Jan 2023 11:30:59 -0500 Subject: [PATCH 22/27] Fix coverage workflow --- .github/workflows/ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 121f25b..ceceb00 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -55,7 +55,9 @@ jobs: coverage: env: + DEPLOYER_PRIVATE_KEY: ${{ secrets.DEPLOYER_PRIVATE_KEY }} MAINNET_RPC_URL: ${{ secrets.MAINNET_RPC_URL }} + PROPOSER_PRIVATE_KEY: ${{ secrets.PROPOSER_PRIVATE_KEY }} runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 From 751dc77245f129f4c76dd0da85e0f7d986742136 Mon Sep 17 00:00:00 2001 From: David Laprade Date: Tue, 17 Jan 2023 11:35:26 -0500 Subject: [PATCH 23/27] Revert "ci: test if manually creating cache dir solves the issue" This reverts commit adffed2a9ab4502ed573c5ded89ec961b9f06eb2. --- .github/workflows/ci.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ceceb00..ac2e89a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,9 +40,6 @@ jobs: with: version: nightly - - name: Create cache folder - run: mkdir -p ~/.foundry/cache/rpc - # https://twitter.com/PaulRBerg/status/1611116650664796166 - name: Generate fuzz seed with 1 week TTL run: > From 9a5c0f7f1c03db42278968f8ae3bf6027ffe4b97 Mon Sep 17 00:00:00 2001 From: David Laprade Date: Tue, 17 Jan 2023 11:35:37 -0500 Subject: [PATCH 24/27] Revert "Foundry toolchain caches fork requests for us" This reverts commit d7b457000a76cc66417b7adeb4c124e4f8c2a062. --- .github/workflows/ci.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ac2e89a..e005ff5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,6 +40,14 @@ jobs: with: version: nightly + - name: Cache fork requests + uses: actions/cache@v3 + with: + path: ~/.foundry/cache + key: ${{ runner.os }}-foundry-network-fork-${{ github.sha }} + restore-keys: | + ${{ runner.os }}-foundry-network-fork- + # https://twitter.com/PaulRBerg/status/1611116650664796166 - name: Generate fuzz seed with 1 week TTL run: > From 8f10d5770c86cbf2da66ddc97fdb45b7a21ad09f Mon Sep 17 00:00:00 2001 From: David Laprade Date: Tue, 17 Jan 2023 11:37:25 -0500 Subject: [PATCH 25/27] Use cached fork requests on coverage --- .github/workflows/ci.yml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e005ff5..21b19ab 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -72,6 +72,21 @@ jobs: with: version: nightly + - name: Cache fork requests + uses: actions/cache@v3 + with: + path: ~/.foundry/cache + key: ${{ runner.os }}-foundry-network-fork-${{ github.sha }} + restore-keys: | + ${{ runner.os }}-foundry-network-fork- + + # https://twitter.com/PaulRBerg/status/1611116650664796166 + - name: Recycle the fuzz seed from the test run + run: > + echo "FOUNDRY_FUZZ_SEED=$( + echo $(($EPOCHSECONDS - $EPOCHSECONDS % 604800)) + )" >> $GITHUB_ENV + - name: Run coverage run: forge coverage --report summary --report lcov From b69b4ebe457b5a98d8a40c3edb2e13cb33adb621 Mon Sep 17 00:00:00 2001 From: David Laprade Date: Tue, 17 Jan 2023 12:01:53 -0500 Subject: [PATCH 26/27] Modify based on PR feedback --- test/GitcoinGovernor.t.sol | 102 ++++++++++++++++++------------------- 1 file changed, 51 insertions(+), 51 deletions(-) diff --git a/test/GitcoinGovernor.t.sol b/test/GitcoinGovernor.t.sol index 25a6cf3..16d13a9 100644 --- a/test/GitcoinGovernor.t.sol +++ b/test/GitcoinGovernor.t.sol @@ -81,61 +81,67 @@ contract GitcoinGovernorProposalTestHelper is GitcoinGovernorTestHelper { //--------------- HELPERS ---------------// - function upgradeProposalStartBlock() public view returns (uint256) { + function _randomERC20Token(uint256 _seed) internal view returns (IERC20 _token) { + if (_seed % 3 == 0) _token = IERC20(address(gtcToken)); + if (_seed % 3 == 1) _token = usdcToken; + if (_seed % 3 == 2) _token = radToken; + } + + function _upgradeProposalStartBlock() internal view returns (uint256) { (,,, uint256 _startBlock,,,,,) = governorAlpha.proposals(upgradeProposalId); return _startBlock; } - function upgradeProposalEndBlock() public view returns (uint256) { + function _upgradeProposalEndBlock() internal view returns (uint256) { (,,,, uint256 _endBlock,,,,) = governorAlpha.proposals(upgradeProposalId); return _endBlock; } - function upgradeProposalEta() public view returns (uint256) { + function _upgradeProposalEta() internal view returns (uint256) { (,, uint256 _eta,,,,,,) = governorAlpha.proposals(upgradeProposalId); return _eta; } - function jumpToActiveUpgradeProposal() public { - vm.roll(upgradeProposalStartBlock() + 1); + function _jumpToActiveUpgradeProposal() internal { + vm.roll(_upgradeProposalStartBlock() + 1); } - function jumpToUpgradeVoteComplete() public { - vm.roll(upgradeProposalEndBlock() + 1); + function _jumpToUpgradeVoteComplete() internal { + vm.roll(_upgradeProposalEndBlock() + 1); } - function jumpPastProposalEta() public { + function _jumpPastProposalEta() internal { vm.roll(block.number + 1); // move up one block so we're not in the same block as when queued - vm.warp(upgradeProposalEta() + 1); // jump past the eta timestamp + vm.warp(_upgradeProposalEta() + 1); // jump past the eta timestamp } - function delegatesVoteOnUpgradeProposal(bool _support) public { + function _delegatesVoteOnUpgradeProposal(bool _support) internal { for (uint256 _index = 0; _index < delegates.length; _index++) { vm.prank(delegates[_index]); governorAlpha.castVote(upgradeProposalId, _support); } } - function passUpgradeProposal() public { - jumpToActiveUpgradeProposal(); - delegatesVoteOnUpgradeProposal(true); - jumpToUpgradeVoteComplete(); + function _passUpgradeProposal() internal { + _jumpToActiveUpgradeProposal(); + _delegatesVoteOnUpgradeProposal(true); + _jumpToUpgradeVoteComplete(); } - function defeatUpgradeProposal() public { - jumpToActiveUpgradeProposal(); - delegatesVoteOnUpgradeProposal(false); - jumpToUpgradeVoteComplete(); + function _defeatUpgradeProposal() internal { + _jumpToActiveUpgradeProposal(); + _delegatesVoteOnUpgradeProposal(false); + _jumpToUpgradeVoteComplete(); } - function passAndQueueUpgradeProposal() public { - passUpgradeProposal(); + function _passAndQueueUpgradeProposal() internal { + _passUpgradeProposal(); governorAlpha.queue(upgradeProposalId); } - function upgradeToBravoGovernor() public { - passAndQueueUpgradeProposal(); - jumpPastProposalEta(); + function _upgradeToBravoGovernor() internal { + _passAndQueueUpgradeProposal(); + _jumpPastProposalEta(); governorAlpha.execute(upgradeProposalId); } } @@ -171,7 +177,7 @@ contract GitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { } function test_proposalActiveAfterDelay() public { - jumpToActiveUpgradeProposal(); + _jumpToActiveUpgradeProposal(); // Ensure proposal has become active the block after the voting delay uint8 _state = governorAlpha.state(upgradeProposalId); @@ -179,8 +185,8 @@ contract GitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { } function testFuzz_ProposerCanCastVote(bool _willSupport) public { - jumpToActiveUpgradeProposal(); - uint256 _proposerVotes = gtcToken.getPriorVotes(PROPOSER, upgradeProposalStartBlock()); + _jumpToActiveUpgradeProposal(); + uint256 _proposerVotes = gtcToken.getPriorVotes(PROPOSER, _upgradeProposalStartBlock()); vm.prank(PROPOSER); governorAlpha.castVote(upgradeProposalId, _willSupport); @@ -192,7 +198,7 @@ contract GitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { } function test_ProposalSucceedsWhenAllDelegatesVoteFor() public { - passUpgradeProposal(); + _passUpgradeProposal(); // Ensure proposal state is now succeeded uint8 _state = governorAlpha.state(upgradeProposalId); @@ -200,7 +206,7 @@ contract GitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { } function test_ProposalDefeatedWhenAllDelegatesVoteAgainst() public { - defeatUpgradeProposal(); + _defeatUpgradeProposal(); // Ensure proposal state is now defeated uint8 _state = governorAlpha.state(upgradeProposalId); @@ -208,7 +214,7 @@ contract GitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { } function test_ProposalCanBeQueuedAfterSucceeding() public { - passUpgradeProposal(); + _passUpgradeProposal(); governorAlpha.queue(upgradeProposalId); // Ensure proposal can be queued after success @@ -237,8 +243,8 @@ contract GitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { } function test_ProposalCanBeExecutedAfterDelay() public { - passAndQueueUpgradeProposal(); - jumpPastProposalEta(); + _passAndQueueUpgradeProposal(); + _jumpPastProposalEta(); // Execute the proposal governorAlpha.execute(upgradeProposalId); @@ -268,7 +274,7 @@ contract GitcoinGovernorAlphaPostProposalTest is GitcoinGovernorProposalTestHelp uint256 _initialGtcBalance = gtcToken.balanceOf(_gtcReceiver); // Defeat the proposal to upgrade the Governor - defeatUpgradeProposal(); + _defeatUpgradeProposal(); // Craft a new proposal to send GTC address[] memory _targets = new address[](1); @@ -333,7 +339,7 @@ contract GitcoinGovernorAlphaPostProposalTest is GitcoinGovernorProposalTestHelp uint256 _initialRadBalance = radToken.balanceOf(_radReceiver); // Defeat the proposal to upgrade the Governor - defeatUpgradeProposal(); + _defeatUpgradeProposal(); // Craft a new proposal to send amounts of all three tokens address[] memory _targets = new address[](2); @@ -395,7 +401,7 @@ contract GitcoinGovernorAlphaPostProposalTest is GitcoinGovernorProposalTestHelp _gtcAmount = bound(_gtcAmount, 0, _timelockGtcBalance); // Pass and execute the proposal to upgrade the Governor - upgradeToBravoGovernor(); + _upgradeToBravoGovernor(); // Craft a new proposal to send GTC address[] memory _targets = new address[](1); @@ -464,7 +470,7 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { vm.roll(_deadline + 1); } - function jumpPastProposalEta(uint256 _proposalId) public { + function _jumpPastProposalEta(uint256 _proposalId) public { uint256 _eta = governorBravo.proposalEta(_proposalId); vm.roll(block.number + 1); vm.warp(_eta + 1); @@ -508,7 +514,7 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { public { assumeReceiver(_gtcReceiver); - upgradeToBravoGovernor(); + _upgradeToBravoGovernor(); (uint256 _newProposalId,,,,) = submitTokenSendProposal(address(gtcToken), _gtcAmount, _gtcReceiver); @@ -517,19 +523,13 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { assertEq(_state, IGovernor.ProposalState.Pending); } - function _randomERC20Token(uint256 _seed) internal view returns (IERC20 _token) { - if (_seed % 3 == 0) _token = IERC20(address(gtcToken)); - if (_seed % 3 == 1) _token = usdcToken; - if (_seed % 3 == 2) _token = radToken; - } - function testFuzz_NewGovernorCanDefeatProposal(uint256 _amount, address _receiver, uint256 _seed) public { IERC20 _token = _randomERC20Token(_seed); assumeReceiver(_receiver); - upgradeToBravoGovernor(); + _upgradeToBravoGovernor(); ( uint256 _newProposalId, address[] memory _targets, @@ -573,7 +573,7 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { _amount = bound(_amount, 0, _timelockTokenBalance); uint256 _initialTokenBalance = _token.balanceOf(_receiver); - upgradeToBravoGovernor(); + _upgradeToBravoGovernor(); ( uint256 _newProposalId, address[] memory _targets, @@ -606,7 +606,7 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { _state = governorBravo.state(_newProposalId); assertEq(_state, IGovernor.ProposalState.Queued); - jumpPastProposalEta(_newProposalId); + _jumpPastProposalEta(_newProposalId); // Execute the proposal governorBravo.execute(_targets, _values, _calldatas, keccak256(bytes(_description))); @@ -630,7 +630,7 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { _newVotingPeriod = bound(_newVotingPeriod, 1, 200_000); // about a month _newProposalThreshold = bound(_newProposalThreshold, 0, 42 ether); - upgradeToBravoGovernor(); + _upgradeToBravoGovernor(); address[] memory _targets = new address[](3); uint256[] memory _values = new uint256[](3); @@ -675,7 +675,7 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { _state = governorBravo.state(_newProposalId); assertEq(_state, IGovernor.ProposalState.Queued); - jumpPastProposalEta(_newProposalId); + _jumpPastProposalEta(_newProposalId); // Execute the proposal governorBravo.execute(_targets, _values, _calldatas, keccak256(bytes(_description))); @@ -703,7 +703,7 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { _amount = bound(_amount, 0, _timelockTokenBalance); uint256 _initialTokenBalance = _token.balanceOf(_receiver); - upgradeToBravoGovernor(); + _upgradeToBravoGovernor(); ( uint256 _newProposalId, address[] memory _targets, @@ -745,7 +745,7 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { // Queue the proposal governorBravo.queue(_targets, _values, _calldatas, keccak256(bytes(_description))); - jumpPastProposalEta(_newProposalId); + _jumpPastProposalEta(_newProposalId); // Execute the proposal governorBravo.execute(_targets, _values, _calldatas, keccak256(bytes(_description))); @@ -771,7 +771,7 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { // bound by the number of tokens the timelock currently controls _amount = bound(_amount, 0, _timelockTokenBalance); - upgradeToBravoGovernor(); + _upgradeToBravoGovernor(); ( uint256 _newProposalId, address[] memory _targets, @@ -838,7 +838,7 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { IERC20 _token = _randomERC20Token(_seed); assumeReceiver(_receiver); - upgradeToBravoGovernor(); + _upgradeToBravoGovernor(); // Create a new proposal to send the token. _vars.alphaTargets = new address[](1); From 4216a168c97d7111604d4f0da06fb655cae7283c Mon Sep 17 00:00:00 2001 From: David Laprade Date: Tue, 17 Jan 2023 14:07:05 -0500 Subject: [PATCH 27/27] Update comment --- test/GitcoinGovernor.t.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/GitcoinGovernor.t.sol b/test/GitcoinGovernor.t.sol index 16d13a9..0eff158 100644 --- a/test/GitcoinGovernor.t.sol +++ b/test/GitcoinGovernor.t.sol @@ -445,9 +445,9 @@ contract NewGitcoinGovernorProposalTest is GitcoinGovernorProposalTestHelper { // We don't want the receiver to be the Timelock, as that would make our // assertions less meaningful -- most of our tests want to confirm that // proposals can cause tokens to be sent *from* the timelock to somewhere - // else. We also don't want the zero address as a receiver -- if our - // governor is sending tokens to the zero address it could just be due to a - // bug. + // else. We also can't have the receiver be the zero address because GTC + // blocks transfers to the zero address -- see line 546: + // https://etherscan.io/address/0xDe30da39c46104798bB5aA3fe8B9e0e1F348163F#code vm.assume(_receiver != TIMELOCK && _receiver != address(0x0)); }