Skip to content

Commit f692379

Browse files
committed
additional cleanup
1 parent becb8b1 commit f692379

9 files changed

+195
-20
lines changed

.env.example

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,5 @@
1+
# RPC for running governor v2 upgrade tests
12
ARBITRUM_ONE_RPC_URL=
3+
4+
# Proposer address for governor v2 upgrade
5+
PROPOSER_ADDRESS=

remappings.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,4 @@ solady/=lib/solady/src/
88
@gnosis.pm/safe-contracts/=node_modules/@gnosis.pm/safe-contracts/
99
ds-test/=lib/forge-std/lib/ds-test/src/
1010
openzeppelin-v5/=lib/openzeppelin-contracts-v5/contracts
11-
openzeppelin-v4/=lib/openzeppelin-contracts-v4/contracts
1211
openzeppelin-upgradeable-v5/=lib/openzeppelin-contracts-upgradeable-v5/contracts

script/DeployTimelockRolesUpgrader.s.sol

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@ contract DeployTimelockRolesUpgrader is BaseDeployer, SharedGovernorConstants {
1414
timelockRolesUpgrader = new TimelockRolesUpgrader(
1515
L2_CORE_GOVERNOR_TIMELOCK,
1616
L2_CORE_GOVERNOR,
17-
L2_CORE_GOVERNOR_ONCHAIN,
17+
L2_CORE_GOVERNOR_NEW_DEPLOY,
1818
L2_TREASURY_GOVERNOR_TIMELOCK,
1919
L2_TREASURY_GOVERNOR,
20-
L2_TREASURY_GOVERNOR_ONCHAIN
20+
L2_TREASURY_GOVERNOR_NEW_DEPLOY
2121
);
2222
vm.stopBroadcast();
2323
}

script/SharedGovernorConstants.sol

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,11 @@ contract SharedGovernorConstants {
2323
address public constant L2_SECURITY_COUNCIL_9 = 0x423552c0F05baCCac5Bfa91C6dCF1dc53a0A1641;
2424

2525
address public constant L1_TIMELOCK = 0xE6841D92B0C345144506576eC13ECf5103aC7f49;
26-
uint256 public constant L1_TIMELOCK_MIN_DELAY = 259_200; // TODO: Make sure this is up to date.
26+
uint256 public constant L1_TIMELOCK_MIN_DELAY = 259_200;
2727
address public constant L1_ARB_ONE_DELAYED_INBOX = 0x4Dbd4fc535Ac27206064B68FfCf827b0A60BAB3f;
2828

29-
address public constant L2_CORE_GOVERNOR_ONCHAIN = 0x7796F378B3c56ceD57350B938561D8c52256456b;
30-
address public constant L2_TREASURY_GOVERNOR_ONCHAIN =
29+
address public constant L2_CORE_GOVERNOR_NEW_DEPLOY = 0x7796F378B3c56ceD57350B938561D8c52256456b;
30+
address public constant L2_TREASURY_GOVERNOR_NEW_DEPLOY =
3131
0x4fd1216c8b5E72b22785169Ae5C1e8f3b30C19E4;
3232
bool public constant UPGRADE_PROPOSAL_PASSED_ONCHAIN = false; // TODO: Update after the upgrade proposal is passed.
3333

script/SubmitUpgradeProposalScript.s.sol

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ contract SubmitUpgradeProposalScript is Script, SharedGovernorConstants, CreateL
3535
uint256 _proposalId
3636
)
3737
{
38-
// TODO: Before deployment update `_description` with the new governor contract addresses.
3938
_description =
4039
"# Proposal to Upgrade Governor Contracts \
4140
\
@@ -55,8 +54,8 @@ contract SubmitUpgradeProposalScript is Script, SharedGovernorConstants, CreateL
5554
\
5655
### **Technical Details** \
5756
- The new Governor contracts have been deployed on Arbitrum One at the following addresses: \
58-
TODO: [Insert new Core Governor address] \
59-
TODO: [Insert new Treasury Governor address] \
57+
Core Governor: 0x7796F378B3c56ceD57350B938561D8c52256456b \
58+
Treasury Governor: 0x4fd1216c8b5E72b22785169Ae5C1e8f3b30C19E4 \
6059
\
6160
- These new contracts include the following enhancements: \
6261
1. Proposal Cancellation: Allows the delegate who submitted a proposal to cancel it during the delay phase, before voting begins. \

src/L2ArbitrumGovernorV2.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import {GovernorUpgradeable} from "openzeppelin-upgradeable-v5/governance/Govern
66
import {GovernorSettingsUpgradeable} from
77
"openzeppelin-upgradeable-v5/governance/extensions/GovernorSettingsUpgradeable.sol";
88
import {GovernorCountingFractionalUpgradeable} from
9-
"./lib/GovernorCountingFractionalUpgradeable.sol";
9+
"src/lib/GovernorCountingFractionalUpgradeable.sol";
1010
import {GovernorTimelockControlUpgradeable} from
1111
"openzeppelin-upgradeable-v5/governance/extensions/GovernorTimelockControlUpgradeable.sol";
1212
import {GovernorVotesQuorumFractionUpgradeable} from

test/L2ArbitrumGovernorV2.t.sol

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,8 @@ abstract contract L2ArbitrumGovernorV2Test is SetupNewGovernors {
112112

113113
function _skipToPostUpgrade() internal {
114114
if (
115-
UPGRADE_PROPOSAL_PASSED_ONCHAIN && L2_CORE_GOVERNOR_ONCHAIN != address(0)
116-
&& L2_TREASURY_GOVERNOR_ONCHAIN != address(0)
115+
UPGRADE_PROPOSAL_PASSED_ONCHAIN && L2_CORE_GOVERNOR_NEW_DEPLOY != address(0)
116+
&& L2_TREASURY_GOVERNOR_NEW_DEPLOY != address(0)
117117
) {
118118
return;
119119
}
@@ -171,9 +171,9 @@ abstract contract CoreGovernorBase is L2ArbitrumGovernorV2Test {
171171
function setUp() public virtual override {
172172
super.setUp();
173173
// If no deployed governor address is set, we use the locally deployed governor
174-
governor = L2_CORE_GOVERNOR_ONCHAIN == address(0)
174+
governor = L2_CORE_GOVERNOR_NEW_DEPLOY == address(0)
175175
? newCoreGovernor
176-
: L2ArbitrumGovernorV2(payable(L2_CORE_GOVERNOR_ONCHAIN));
176+
: L2ArbitrumGovernorV2(payable(L2_CORE_GOVERNOR_NEW_DEPLOY));
177177
_oldGovernor = currentCoreGovernor;
178178
timelock = currentCoreTimelock;
179179
proxyDeployer = proxyCoreGovernorDeployer;
@@ -208,9 +208,9 @@ abstract contract TreasuryGovernorBase is L2ArbitrumGovernorV2Test {
208208
function setUp() public virtual override {
209209
super.setUp();
210210
// If no deployed governor address is set, we use the locally deployed governor
211-
governor = L2_TREASURY_GOVERNOR_ONCHAIN == address(0)
211+
governor = L2_TREASURY_GOVERNOR_NEW_DEPLOY == address(0)
212212
? newTreasuryGovernor
213-
: L2ArbitrumGovernorV2(payable(L2_TREASURY_GOVERNOR_ONCHAIN));
213+
: L2ArbitrumGovernorV2(payable(L2_TREASURY_GOVERNOR_NEW_DEPLOY));
214214
_oldGovernor = currentTreasuryGovernor;
215215
timelock = currentTreasuryTimelock;
216216
proxyDeployer = proxyTreasuryGovernorDeployer;

test/SubmitUpgradeProposal.t.sol

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
// SPDX-License-Identifier: AGPL-3.0-only
2+
pragma solidity 0.8.26;
3+
4+
import {Test, console2} from "forge-std/Test.sol";
5+
import {SubmitUpgradeProposalScript} from "script/SubmitUpgradeProposalScript.s.sol";
6+
import {IGovernor} from "openzeppelin-v5/governance/IGovernor.sol";
7+
import {TimelockRolesUpgrader} from
8+
"src/gov-action-contracts/gov-upgrade-contracts/update-timelock-roles/TimelockRolesUpgrader.sol";
9+
import {SetupNewGovernors} from "test/util/SetupNewGovernors.sol";
10+
11+
contract SubmitUpgradeProposalTest is SetupNewGovernors {
12+
function test_SuccessfullyExecuteUpgradeProposal() public {
13+
TimelockRolesUpgrader timelockRolesUpgrader = new TimelockRolesUpgrader(
14+
L2_CORE_GOVERNOR_TIMELOCK,
15+
L2_CORE_GOVERNOR,
16+
L2_CORE_GOVERNOR_NEW_DEPLOY,
17+
L2_TREASURY_GOVERNOR_TIMELOCK,
18+
L2_TREASURY_GOVERNOR,
19+
L2_TREASURY_GOVERNOR_NEW_DEPLOY
20+
);
21+
22+
// Propose
23+
(
24+
address[] memory _targets,
25+
uint256[] memory _values,
26+
bytes[] memory _calldatas,
27+
string memory _description,
28+
uint256 _proposalId
29+
) = submitUpgradeProposalScript.run(address(timelockRolesUpgrader), L1_TIMELOCK_MIN_DELAY);
30+
assertEq(
31+
uint256(currentCoreGovernor.state(_proposalId)),
32+
uint256(IGovernor.ProposalState.Pending)
33+
);
34+
vm.roll(vm.getBlockNumber() + currentCoreGovernor.votingDelay() + 1);
35+
assertEq(
36+
uint256(currentCoreGovernor.state(_proposalId)), uint256(IGovernor.ProposalState.Active)
37+
);
38+
39+
// Vote
40+
for (uint256 i; i < _majorDelegates.length; i++) {
41+
vm.prank(_majorDelegates[i]);
42+
currentCoreGovernor.castVote(_proposalId, uint8(VoteType.For));
43+
}
44+
45+
// Success
46+
vm.roll(vm.getBlockNumber() + currentCoreGovernor.votingPeriod() + 1);
47+
assertEq(
48+
uint256(currentCoreGovernor.state(_proposalId)),
49+
uint256(IGovernor.ProposalState.Succeeded)
50+
);
51+
52+
// Queue
53+
currentCoreGovernor.queue(_targets, _values, _calldatas, keccak256(bytes(_description)));
54+
assertEq(
55+
uint256(currentCoreGovernor.state(_proposalId)), uint256(IGovernor.ProposalState.Queued)
56+
);
57+
vm.warp(vm.getBlockTimestamp() + currentCoreTimelock.getMinDelay() + 1);
58+
59+
// Execute
60+
currentCoreGovernor.execute(_targets, _values, _calldatas, keccak256(bytes(_description)));
61+
assertEq(
62+
uint256(currentCoreGovernor.state(_proposalId)),
63+
uint256(IGovernor.ProposalState.Executed)
64+
);
65+
66+
assertEq(
67+
currentCoreTimelock.hasRole(keccak256("PROPOSER_ROLE"), address(newCoreGovernor)), true
68+
);
69+
assertEq(
70+
currentCoreTimelock.hasRole(keccak256("CANCELLER_ROLE"), address(newCoreGovernor)), true
71+
);
72+
assertEq(currentCoreTimelock.hasRole(keccak256("PROPOSER_ROLE"), L2_CORE_GOVERNOR), false);
73+
assertEq(currentCoreTimelock.hasRole(keccak256("CANCELLER_ROLE"), L2_CORE_GOVERNOR), false);
74+
75+
assertEq(
76+
currentTreasuryTimelock.hasRole(
77+
keccak256("PROPOSER_ROLE"), address(newTreasuryGovernor)
78+
),
79+
true
80+
);
81+
assertEq(
82+
currentTreasuryTimelock.hasRole(
83+
keccak256("CANCELLER_ROLE"), address(newTreasuryGovernor)
84+
),
85+
true
86+
);
87+
assertEq(
88+
currentTreasuryTimelock.hasRole(keccak256("PROPOSER_ROLE"), L2_TREASURY_GOVERNOR), false
89+
);
90+
assertEq(
91+
currentTreasuryTimelock.hasRole(keccak256("CANCELLER_ROLE"), L2_TREASURY_GOVERNOR),
92+
false
93+
);
94+
}
95+
96+
function test_DefeatedExecuteUpgradeProposalDoesNotChangeRoles() public {
97+
TimelockRolesUpgrader timelockRolesUpgrader = new TimelockRolesUpgrader(
98+
L2_CORE_GOVERNOR_TIMELOCK,
99+
L2_CORE_GOVERNOR,
100+
address(newCoreGovernor),
101+
L2_TREASURY_GOVERNOR_TIMELOCK,
102+
L2_TREASURY_GOVERNOR,
103+
address(newTreasuryGovernor)
104+
);
105+
106+
// Propose
107+
(
108+
/*address[] memory _targets*/
109+
,
110+
/*uint256[] memory _values*/
111+
,
112+
/*bytes[] memory _calldatas*/
113+
,
114+
/*string memory _description*/
115+
,
116+
uint256 _proposalId
117+
) = submitUpgradeProposalScript.run(address(timelockRolesUpgrader), L1_TIMELOCK_MIN_DELAY);
118+
assertEq(
119+
uint256(currentCoreGovernor.state(_proposalId)),
120+
uint256(IGovernor.ProposalState.Pending)
121+
);
122+
vm.roll(vm.getBlockNumber() + currentCoreGovernor.votingDelay() + 1);
123+
assertEq(
124+
uint256(currentCoreGovernor.state(_proposalId)), uint256(IGovernor.ProposalState.Active)
125+
);
126+
127+
// Vote
128+
for (uint256 i; i < _majorDelegates.length; i++) {
129+
vm.prank(_majorDelegates[i]);
130+
currentCoreGovernor.castVote(_proposalId, uint8(VoteType.Against));
131+
}
132+
133+
// Defeat
134+
vm.roll(vm.getBlockNumber() + currentCoreGovernor.votingPeriod() + 1);
135+
assertEq(
136+
uint256(currentCoreGovernor.state(_proposalId)),
137+
uint256(IGovernor.ProposalState.Defeated)
138+
);
139+
140+
assertEq(
141+
currentCoreTimelock.hasRole(keccak256("PROPOSER_ROLE"), address(newCoreGovernor)), false
142+
);
143+
assertEq(
144+
currentCoreTimelock.hasRole(keccak256("CANCELLER_ROLE"), address(newCoreGovernor)),
145+
false
146+
);
147+
assertEq(currentCoreTimelock.hasRole(keccak256("PROPOSER_ROLE"), L2_CORE_GOVERNOR), true);
148+
assertEq(currentCoreTimelock.hasRole(keccak256("CANCELLER_ROLE"), L2_CORE_GOVERNOR), true);
149+
150+
assertEq(
151+
currentTreasuryTimelock.hasRole(
152+
keccak256("PROPOSER_ROLE"), address(newTreasuryGovernor)
153+
),
154+
false
155+
);
156+
assertEq(
157+
currentTreasuryTimelock.hasRole(
158+
keccak256("CANCELLER_ROLE"), address(newTreasuryGovernor)
159+
),
160+
false
161+
);
162+
assertEq(
163+
currentTreasuryTimelock.hasRole(keccak256("PROPOSER_ROLE"), L2_TREASURY_GOVERNOR), true
164+
);
165+
assertEq(
166+
currentTreasuryTimelock.hasRole(keccak256("CANCELLER_ROLE"), L2_TREASURY_GOVERNOR), true
167+
);
168+
}
169+
}
170+
171+
interface IUpgradeExecutor {
172+
function execute(address to, bytes calldata data) external payable;
173+
}

test/util/SetupNewGovernors.sol

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,12 @@ abstract contract SetupNewGovernors is SharedGovernorConstants, Test {
5252
proxyTreasuryGovernorDeployer.setUp();
5353

5454
// Deploy Governor proxy contracts
55-
newCoreGovernor = L2_CORE_GOVERNOR_ONCHAIN == address(0)
55+
newCoreGovernor = L2_CORE_GOVERNOR_NEW_DEPLOY == address(0)
5656
? proxyCoreGovernorDeployer.run(_implementation)
57-
: L2ArbitrumGovernorV2(payable(L2_CORE_GOVERNOR_ONCHAIN));
58-
newTreasuryGovernor = L2_TREASURY_GOVERNOR_ONCHAIN == address(0)
57+
: L2ArbitrumGovernorV2(payable(L2_CORE_GOVERNOR_NEW_DEPLOY));
58+
newTreasuryGovernor = L2_TREASURY_GOVERNOR_NEW_DEPLOY == address(0)
5959
? proxyTreasuryGovernorDeployer.run(_implementation)
60-
: L2ArbitrumGovernorV2(payable(L2_TREASURY_GOVERNOR_ONCHAIN));
60+
: L2ArbitrumGovernorV2(payable(L2_TREASURY_GOVERNOR_NEW_DEPLOY));
6161

6262
// Current governors and timelocks
6363
currentCoreGovernor = GovernorUpgradeable(payable(L2_CORE_GOVERNOR));

0 commit comments

Comments
 (0)