Skip to content

Commit

Permalink
additional cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
wildmolasses committed Aug 28, 2024
1 parent becb8b1 commit d3750a5
Show file tree
Hide file tree
Showing 9 changed files with 194 additions and 18 deletions.
4 changes: 4 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
@@ -1 +1,5 @@
# RPC for running governor v2 upgrade tests
ARBITRUM_ONE_RPC_URL=

# Proposer address for governor v2 upgrade
PROPOSER_ADDRESS=
1 change: 0 additions & 1 deletion remappings.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,4 @@ solady/=lib/solady/src/
@gnosis.pm/safe-contracts/=node_modules/@gnosis.pm/safe-contracts/
ds-test/=lib/forge-std/lib/ds-test/src/
openzeppelin-v5/=lib/openzeppelin-contracts-v5/contracts
openzeppelin-v4/=lib/openzeppelin-contracts-v4/contracts
openzeppelin-upgradeable-v5/=lib/openzeppelin-contracts-upgradeable-v5/contracts
4 changes: 2 additions & 2 deletions script/DeployTimelockRolesUpgrader.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ contract DeployTimelockRolesUpgrader is BaseDeployer, SharedGovernorConstants {
timelockRolesUpgrader = new TimelockRolesUpgrader(
L2_CORE_GOVERNOR_TIMELOCK,
L2_CORE_GOVERNOR,
L2_CORE_GOVERNOR_ONCHAIN,
L2_CORE_GOVERNOR_NEW_DEPLOY,
L2_TREASURY_GOVERNOR_TIMELOCK,
L2_TREASURY_GOVERNOR,
L2_TREASURY_GOVERNOR_ONCHAIN
L2_TREASURY_GOVERNOR_NEW_DEPLOY
);
vm.stopBroadcast();
}
Expand Down
4 changes: 2 additions & 2 deletions script/SharedGovernorConstants.sol
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ contract SharedGovernorConstants {
uint256 public constant L1_TIMELOCK_MIN_DELAY = 259_200; // TODO: Make sure this is up to date.
address public constant L1_ARB_ONE_DELAYED_INBOX = 0x4Dbd4fc535Ac27206064B68FfCf827b0A60BAB3f;

address public constant L2_CORE_GOVERNOR_ONCHAIN = 0x7796F378B3c56ceD57350B938561D8c52256456b;
address public constant L2_TREASURY_GOVERNOR_ONCHAIN =
address public constant L2_CORE_GOVERNOR_NEW_DEPLOY = 0x7796F378B3c56ceD57350B938561D8c52256456b;
address public constant L2_TREASURY_GOVERNOR_NEW_DEPLOY =
0x4fd1216c8b5E72b22785169Ae5C1e8f3b30C19E4;
bool public constant UPGRADE_PROPOSAL_PASSED_ONCHAIN = false; // TODO: Update after the upgrade proposal is passed.

Expand Down
4 changes: 2 additions & 2 deletions script/SubmitUpgradeProposalScript.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ contract SubmitUpgradeProposalScript is Script, SharedGovernorConstants, CreateL
\
### **Technical Details** \
- The new Governor contracts have been deployed on Arbitrum One at the following addresses: \
TODO: [Insert new Core Governor address] \
TODO: [Insert new Treasury Governor address] \
Core Governor: 0x7796F378B3c56ceD57350B938561D8c52256456b \
Treasury Governor: 0x4fd1216c8b5E72b22785169Ae5C1e8f3b30C19E4 \
\
- These new contracts include the following enhancements: \
1. Proposal Cancellation: Allows the delegate who submitted a proposal to cancel it during the delay phase, before voting begins. \
Expand Down
2 changes: 1 addition & 1 deletion src/L2ArbitrumGovernorV2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {GovernorUpgradeable} from "openzeppelin-upgradeable-v5/governance/Govern
import {GovernorSettingsUpgradeable} from
"openzeppelin-upgradeable-v5/governance/extensions/GovernorSettingsUpgradeable.sol";
import {GovernorCountingFractionalUpgradeable} from
"./lib/GovernorCountingFractionalUpgradeable.sol";
"src/lib/GovernorCountingFractionalUpgradeable.sol";
import {GovernorTimelockControlUpgradeable} from
"openzeppelin-upgradeable-v5/governance/extensions/GovernorTimelockControlUpgradeable.sol";
import {GovernorVotesQuorumFractionUpgradeable} from
Expand Down
12 changes: 6 additions & 6 deletions test/L2ArbitrumGovernorV2.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@ abstract contract L2ArbitrumGovernorV2Test is SetupNewGovernors {

function _skipToPostUpgrade() internal {
if (
UPGRADE_PROPOSAL_PASSED_ONCHAIN && L2_CORE_GOVERNOR_ONCHAIN != address(0)
&& L2_TREASURY_GOVERNOR_ONCHAIN != address(0)
UPGRADE_PROPOSAL_PASSED_ONCHAIN && L2_CORE_GOVERNOR_NEW_DEPLOY != address(0)
&& L2_TREASURY_GOVERNOR_NEW_DEPLOY != address(0)
) {
return;
}
Expand Down Expand Up @@ -171,9 +171,9 @@ abstract contract CoreGovernorBase is L2ArbitrumGovernorV2Test {
function setUp() public virtual override {
super.setUp();
// If no deployed governor address is set, we use the locally deployed governor
governor = L2_CORE_GOVERNOR_ONCHAIN == address(0)
governor = L2_CORE_GOVERNOR_NEW_DEPLOY == address(0)
? newCoreGovernor
: L2ArbitrumGovernorV2(payable(L2_CORE_GOVERNOR_ONCHAIN));
: L2ArbitrumGovernorV2(payable(L2_CORE_GOVERNOR_NEW_DEPLOY));
_oldGovernor = currentCoreGovernor;
timelock = currentCoreTimelock;
proxyDeployer = proxyCoreGovernorDeployer;
Expand Down Expand Up @@ -208,9 +208,9 @@ abstract contract TreasuryGovernorBase is L2ArbitrumGovernorV2Test {
function setUp() public virtual override {
super.setUp();
// If no deployed governor address is set, we use the locally deployed governor
governor = L2_TREASURY_GOVERNOR_ONCHAIN == address(0)
governor = L2_TREASURY_GOVERNOR_NEW_DEPLOY == address(0)
? newTreasuryGovernor
: L2ArbitrumGovernorV2(payable(L2_TREASURY_GOVERNOR_ONCHAIN));
: L2ArbitrumGovernorV2(payable(L2_TREASURY_GOVERNOR_NEW_DEPLOY));
_oldGovernor = currentTreasuryGovernor;
timelock = currentTreasuryTimelock;
proxyDeployer = proxyTreasuryGovernorDeployer;
Expand Down
173 changes: 173 additions & 0 deletions test/SubmitUpgradeProposal.t.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
// SPDX-License-Identifier: AGPL-3.0-only
pragma solidity 0.8.26;

import {Test, console2} from "forge-std/Test.sol";
import {SubmitUpgradeProposalScript} from "script/SubmitUpgradeProposalScript.s.sol";
import {IGovernor} from "openzeppelin-v5/governance/IGovernor.sol";
import {TimelockRolesUpgrader} from
"src/gov-action-contracts/gov-upgrade-contracts/update-timelock-roles/TimelockRolesUpgrader.sol";
import {SetupNewGovernors} from "test/util/SetupNewGovernors.sol";

contract SubmitUpgradeProposalTest is SetupNewGovernors {
function test_SuccessfullyExecuteUpgradeProposal() public {
TimelockRolesUpgrader timelockRolesUpgrader = new TimelockRolesUpgrader(
L2_CORE_GOVERNOR_TIMELOCK,
L2_CORE_GOVERNOR,
L2_CORE_GOVERNOR_NEW_DEPLOY,
L2_TREASURY_GOVERNOR_TIMELOCK,
L2_TREASURY_GOVERNOR,
L2_TREASURY_GOVERNOR_NEW_DEPLOY
);

// Propose
(
address[] memory _targets,
uint256[] memory _values,
bytes[] memory _calldatas,
string memory _description,
uint256 _proposalId
) = submitUpgradeProposalScript.run(address(timelockRolesUpgrader), L1_TIMELOCK_MIN_DELAY);
assertEq(
uint256(currentCoreGovernor.state(_proposalId)),
uint256(IGovernor.ProposalState.Pending)
);
vm.roll(vm.getBlockNumber() + currentCoreGovernor.votingDelay() + 1);
assertEq(
uint256(currentCoreGovernor.state(_proposalId)), uint256(IGovernor.ProposalState.Active)
);

// Vote
for (uint256 i; i < _majorDelegates.length; i++) {
vm.prank(_majorDelegates[i]);
currentCoreGovernor.castVote(_proposalId, uint8(VoteType.For));
}

// Success
vm.roll(vm.getBlockNumber() + currentCoreGovernor.votingPeriod() + 1);
assertEq(
uint256(currentCoreGovernor.state(_proposalId)),
uint256(IGovernor.ProposalState.Succeeded)
);

// Queue
currentCoreGovernor.queue(_targets, _values, _calldatas, keccak256(bytes(_description)));
assertEq(
uint256(currentCoreGovernor.state(_proposalId)), uint256(IGovernor.ProposalState.Queued)
);
vm.warp(vm.getBlockTimestamp() + currentCoreTimelock.getMinDelay() + 1);

// Execute
currentCoreGovernor.execute(_targets, _values, _calldatas, keccak256(bytes(_description)));
assertEq(
uint256(currentCoreGovernor.state(_proposalId)),
uint256(IGovernor.ProposalState.Executed)
);

assertEq(
currentCoreTimelock.hasRole(keccak256("PROPOSER_ROLE"), address(newCoreGovernor)), true
);
assertEq(
currentCoreTimelock.hasRole(keccak256("CANCELLER_ROLE"), address(newCoreGovernor)), true
);
assertEq(currentCoreTimelock.hasRole(keccak256("PROPOSER_ROLE"), L2_CORE_GOVERNOR), false);
assertEq(currentCoreTimelock.hasRole(keccak256("CANCELLER_ROLE"), L2_CORE_GOVERNOR), false);

assertEq(
currentTreasuryTimelock.hasRole(
keccak256("PROPOSER_ROLE"), address(newTreasuryGovernor)
),
true
);
assertEq(
currentTreasuryTimelock.hasRole(
keccak256("CANCELLER_ROLE"), address(newTreasuryGovernor)
),
true
);
assertEq(
currentTreasuryTimelock.hasRole(keccak256("PROPOSER_ROLE"), L2_TREASURY_GOVERNOR), false
);
assertEq(
currentTreasuryTimelock.hasRole(keccak256("CANCELLER_ROLE"), L2_TREASURY_GOVERNOR),
false
);
}

function test_DefeatedExecuteUpgradeProposalDoesNotChangeRoles() public {
TimelockRolesUpgrader timelockRolesUpgrader = new TimelockRolesUpgrader(
L2_CORE_GOVERNOR_TIMELOCK,
L2_CORE_GOVERNOR,
address(newCoreGovernor),
L2_TREASURY_GOVERNOR_TIMELOCK,
L2_TREASURY_GOVERNOR,
address(newTreasuryGovernor)
);

// Propose
(
/*address[] memory _targets*/
,
/*uint256[] memory _values*/
,
/*bytes[] memory _calldatas*/
,
/*string memory _description*/
,
uint256 _proposalId
) = submitUpgradeProposalScript.run(address(timelockRolesUpgrader), L1_TIMELOCK_MIN_DELAY);
assertEq(
uint256(currentCoreGovernor.state(_proposalId)),
uint256(IGovernor.ProposalState.Pending)
);
vm.roll(vm.getBlockNumber() + currentCoreGovernor.votingDelay() + 1);
assertEq(
uint256(currentCoreGovernor.state(_proposalId)), uint256(IGovernor.ProposalState.Active)
);

// Vote
for (uint256 i; i < _majorDelegates.length; i++) {
vm.prank(_majorDelegates[i]);
currentCoreGovernor.castVote(_proposalId, uint8(VoteType.Against));
}

// Defeat
vm.roll(vm.getBlockNumber() + currentCoreGovernor.votingPeriod() + 1);
assertEq(
uint256(currentCoreGovernor.state(_proposalId)),
uint256(IGovernor.ProposalState.Defeated)
);

assertEq(
currentCoreTimelock.hasRole(keccak256("PROPOSER_ROLE"), address(newCoreGovernor)), false
);
assertEq(
currentCoreTimelock.hasRole(keccak256("CANCELLER_ROLE"), address(newCoreGovernor)),
false
);
assertEq(currentCoreTimelock.hasRole(keccak256("PROPOSER_ROLE"), L2_CORE_GOVERNOR), true);
assertEq(currentCoreTimelock.hasRole(keccak256("CANCELLER_ROLE"), L2_CORE_GOVERNOR), true);

assertEq(
currentTreasuryTimelock.hasRole(
keccak256("PROPOSER_ROLE"), address(newTreasuryGovernor)
),
false
);
assertEq(
currentTreasuryTimelock.hasRole(
keccak256("CANCELLER_ROLE"), address(newTreasuryGovernor)
),
false
);
assertEq(
currentTreasuryTimelock.hasRole(keccak256("PROPOSER_ROLE"), L2_TREASURY_GOVERNOR), true
);
assertEq(
currentTreasuryTimelock.hasRole(keccak256("CANCELLER_ROLE"), L2_TREASURY_GOVERNOR), true
);
}
}

interface IUpgradeExecutor {
function execute(address to, bytes calldata data) external payable;
}
8 changes: 4 additions & 4 deletions test/util/SetupNewGovernors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,12 @@ abstract contract SetupNewGovernors is SharedGovernorConstants, Test {
proxyTreasuryGovernorDeployer.setUp();

// Deploy Governor proxy contracts
newCoreGovernor = L2_CORE_GOVERNOR_ONCHAIN == address(0)
newCoreGovernor = L2_CORE_GOVERNOR_NEW_DEPLOY == address(0)
? proxyCoreGovernorDeployer.run(_implementation)
: L2ArbitrumGovernorV2(payable(L2_CORE_GOVERNOR_ONCHAIN));
newTreasuryGovernor = L2_TREASURY_GOVERNOR_ONCHAIN == address(0)
: L2ArbitrumGovernorV2(payable(L2_CORE_GOVERNOR_NEW_DEPLOY));
newTreasuryGovernor = L2_TREASURY_GOVERNOR_NEW_DEPLOY == address(0)
? proxyTreasuryGovernorDeployer.run(_implementation)
: L2ArbitrumGovernorV2(payable(L2_TREASURY_GOVERNOR_ONCHAIN));
: L2ArbitrumGovernorV2(payable(L2_TREASURY_GOVERNOR_NEW_DEPLOY));

// Current governors and timelocks
currentCoreGovernor = GovernorUpgradeable(payable(L2_CORE_GOVERNOR));
Expand Down

0 comments on commit d3750a5

Please sign in to comment.