From 8278d2df3fa6a8dfc71f768cf819a2b9f7626cb4 Mon Sep 17 00:00:00 2001 From: Ben DiFrancesco Date: Wed, 23 Sep 2026 17:38:23 -0400 Subject: [PATCH] Update test suite to execute with actual onchain upgrade proposal --- AGENTS.md | 53 ++++++--- README.md | 31 +++-- .../GovernorUpgradeProposal.integration.t.sol | 41 +++++-- ...radeFranchiserDelegation.integration.t.sol | 37 +++++- ...tUpgradeFranchiserDeploy.integration.t.sol | 26 +++++ ...tUpgradeFranchiserExpiry.integration.t.sol | 37 +++++- ...tUpgradeFranchiserRecall.integration.t.sol | 26 +++++ test/PostUpgradeGovernance.integration.t.sol | 39 ++++++- ...tUpgradeProposalGuardian.integration.t.sol | 22 ++++ ...ostUpgradeQuorumBehavior.integration.t.sol | 22 ++++ .../GitcoinGovernorUpgradeTestBase.sol | 108 ++++++++++++++---- 11 files changed, 377 insertions(+), 65 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 7e54ffe..5b882fe 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -228,7 +228,10 @@ secret). CI supplies it via the `MAINNET_RPC_URL` repository secret. is reserved for the claims a test makes about the system under test; checks on test assumptions or scaffolding (setUp fork state, helper lifecycle checkpoints, scenario preconditions) instead revert with a developer-aimed message naming the broken assumption, so a failure reads as - "repair the test setup," not "a behavior regressed." + "repair the test setup," not "a behavior regressed." Fuzz-run caps for the fork suites go on + each provenance concrete as contract-level `/// forge-config:` lines: forge silently ignores + inline config on test functions inherited from an abstract suite, so a function-level cap there + does nothing and the fuzz test falls back to the profile's run count (5,000 under `ci`). - **Keep docs current.** `README.md` is intentionally lightweight and reflects the project's in-progress status. As scripts, tests, and contracts mature, update the README in the **same change** that introduces them — document a script's usage when the script lands, and drop the "under @@ -252,18 +255,37 @@ secret). CI supplies it via the `MAINNET_RPC_URL` repository secret. **upgrade proposal script** (`ProposeGovernorUpgrade[Mainnet].s.sol`) are in place. The proposal concrete now points at the deployed Governor and still carries `TODO`s for the proposer and final proposal text. +- **The upgrade proposal is on-chain.** It was submitted to the old Governor directly by kev.eth, + not through the proposal script, in block `26_041_897` (transaction + `0xb589c7fcb916860feefeef54da017800787e48952fc739b6d1ab50b34ca77e89`), with id + `0xefb5ffee46ab14020294f926e242a2fa79f096de6577be73d69429207a464489`. Voting runs from block + `26_055_037` to `26_095_357`. Its two actions match the proposal script's exactly; its + description is the stakeholders' own text. - The **mainnet fork integration suite** (`test/*.integration.t.sol`) is in place: it deploys the new Governor with the real deploy script, submits the upgrade proposal with the real proposal script, and exercises the upgrade lifecycle, post-upgrade governance, quorum behavior - (settable + late-quorum), and the Proposal Guardian. Shared helpers live in `test/helpers/`; - the four Governor suites have both `…MainnetScript` and `…MainnetDeployed` provenance concretes. - The former fork from before deployment and run the production deploy script; the latter fork from - the first block after deployment and bind to the production bytecode. Proposals are voted through - by an electorate of **real delegates** whose live weights are read from the fork in `setUp`. The - suite pins both `GOVERNOR_PRE_DEPLOYMENT_BLOCK` and `GOVERNOR_POST_DEPLOYMENT_BLOCK` in - `test/helpers/GitcoinGovernorUpgradeTestBase.sol`; when bumping either, re-verify the `PROPOSER` - delegate still clears the proposal threshold and the electorate still clears quorum (`setUp` - asserts both weights loudly, and quorum-boundary tests assert their own weight preconditions). + (settable + late-quorum), and the Proposal Guardian. Shared helpers live in `test/helpers/`. + Three provenance concretes exist, chosen through the `_setUpNetwork`, `_fetchOrDeploySystem`, + and `_fetchOrSubmitUpgradeProposal` hooks: + - `…MainnetScript` forks from before deployment, runs the production deploy script, and submits + the upgrade proposal with the proposal script. + - `…MainnetDeployed` (the four Governor suites only) forks from the first block after + deployment, binds to the production bytecode, and submits the upgrade proposal with the + proposal script. + - `…MainnetProposed` forks from the first block after the upgrade proposal's submission, binds + to the production bytecode, and adopts the live proposal. Its id and description are read from + the old Governor's `ProposalCreated` event via `vm.eth_getLogs`. Its actions come from the + tests' independent `_upgradeProposalDetails` mirror, so the id matching proves the live + proposal carries exactly the expected actions. + + Proposals are voted through by an electorate of **real delegates** whose live weights are read + from the fork in `setUp`. The suite pins `GOVERNOR_PRE_DEPLOYMENT_BLOCK`, + `GOVERNOR_POST_DEPLOYMENT_BLOCK`, and `UPGRADE_PROPOSAL_POST_SUBMISSION_BLOCK` (alongside + `UPGRADE_PROPOSAL_SUBMISSION_BLOCK` and `SUBMITTED_UPGRADE_PROPOSAL_ID`) in + `test/helpers/GitcoinGovernorUpgradeTestBase.sol`; when bumping a fork block, re-verify the + `PROPOSER` delegate still clears the proposal threshold and the electorate still clears quorum + (`setUp` asserts both weights loudly, and quorum-boundary tests assert their own weight + preconditions). - The Franchiser contracts are in place as the `lib/franchiser-expiry` submodule (ScopeLift fork, `repo-updates` branch), and all four **Franchiser script pairs** are written: `DeployFranchiser[Mainnet]`, `ProposeFranchiserDelegation[Mainnet]`, @@ -271,17 +293,18 @@ secret). CI supplies it via the `MAINNET_RPC_URL` repository secret. dry-runs clean against a mainnet fork; the proposal and sweep concretes carry `TODO`s (factory and new-Governor addresses, proposer, per-round delegations) and revert until those are set. - The **Franchiser fork integration suites** (`test/PostUpgradeFranchiser*.integration.t.sol`) - are in place: each runs the full Governor upgrade in `setUp` (the production sequence), deploys - the Franchiser system with the real deploy script via a `_fetchOrDeployFranchiser` provenance - hook, and drives the operations scripts through constructor-injected test configs in + are in place, each with `…MainnetScript` and `…MainnetProposed` concretes. Each runs the full + Governor upgrade in `setUp` (the production sequence), deploys the Franchiser system with the + real deploy script via a `_fetchOrDeployFranchiser` provenance hook, and drives the operations + scripts through constructor-injected test configs in `test/helpers/`. Coverage spans delegation rounds (fresh/existing delegatees, top-ups and expiration overwrites, zero-amount adjustments, defeats), early recalls (including sub-delegation clawback and the in-flight-snapshot property — a recall cannot strip weight from proposals already snapshotted), expiry sweeps (permissionless, candidate filtering, weight persists until swept), and the scripts' validation reverts. Shared helpers live in `test/helpers/PostUpgradeFranchiserTestBase.sol`. -- Up next: confirm the upgrade proposal's proposer and final text with Gitcoin stakeholders, then - run the upgrade proposal (see [Deliverables](#deliverables)). +- Up next: the DAO votes on the upgrade proposal; once it executes, Franchiser adoption follows + (see [Deliverables](#deliverables)). - CI runs `forge build`, `forge test`, and `scopelint check`. Coverage and Slither jobs are scaffolded but commented out in `.github/workflows/ci.yml`. diff --git a/README.md b/README.md index 319a0dd..ced6ef7 100644 --- a/README.md +++ b/README.md @@ -221,11 +221,19 @@ forge script script/RecallExpiredFranchisersMainnet.s.sol:RecallExpiredFranchise ## Testing The integration tests (`test/*.integration.t.sol`) run against forks of Ethereum mainnet pinned to -fixed blocks. The `…MainnetScript` provenance forks from before the real deployment and runs the -production deploy script. The `…MainnetDeployed` provenance forks from the first block after the -deployment and binds to Gitcoin Governor Charlie's live mainnet bytecode. Both provenances then -submit the upgrade proposal to the currently active Governor and vote it through to execution before -exercising the upgraded Governor in place: +fixed blocks, under three provenances: + +- `…MainnetScript` forks from before the real deployment, runs the production deploy script, and + submits the upgrade proposal with the proposal script. +- `…MainnetDeployed` forks from the first block after the deployment, binds to Gitcoin Governor + Charlie's live mainnet bytecode, and submits the upgrade proposal with the proposal script. +- `…MainnetProposed` forks from the first block after the upgrade proposal was submitted on-chain + (in block 26,041,897), binds to the live Governor, and adopts the live proposal. Its id and + description are read from the chain, while the actions it must carry come from the tests' own + independent copy, so this provenance verifies the exact proposal delegates are voting on. + +Each provenance then votes the upgrade proposal through to execution on the currently active +Governor before exercising the upgraded Governor in place: - `GovernorUpgradeProposal` — the upgrade proposal's lifecycle on the active Governor: passing it hands the Timelock to the new Governor; defeating it leaves the current Governor in control. @@ -254,16 +262,19 @@ the real deploy script and drive the operations scripts end-to-end: on-chain candidate filtering, the weight that persists until a sweep actually runs, and the guard protecting live positions. -Each suite is written against an abstract base that leaves *how the system comes into being* to a -small concrete contract at the bottom of the file. The four Governor suites have both -`…MainnetScript` and `…MainnetDeployed` concretes. Run the deployed-bytecode acceptance suites with: +Each suite is written against an abstract base that leaves *how the system and its upgrade +proposal come into being* to a small concrete contract at the bottom of the file. Every suite has +`…MainnetScript` and `…MainnetProposed` concretes, and the four Governor suites also have +`…MainnetDeployed`. Run the deployed-bytecode acceptance suites, or the suites against the live +upgrade proposal, with: ```sh forge test --match-contract '.*MainnetDeployed' +forge test --match-contract '.*MainnetProposed' ``` -The Franchiser suites currently retain their `…MainnetScript` provenance; deployed concretes will -be added once the Franchiser system is live on mainnet. +The Franchiser suites deploy the Franchiser system with its deploy script under every provenance; +deployed concretes will be added once the Franchiser system is live on mainnet. ## License diff --git a/test/GovernorUpgradeProposal.integration.t.sol b/test/GovernorUpgradeProposal.integration.t.sol index 5b7e9b7..ef51fa5 100644 --- a/test/GovernorUpgradeProposal.integration.t.sol +++ b/test/GovernorUpgradeProposal.integration.t.sol @@ -5,9 +5,10 @@ import {IGovernor} from "@openzeppelin/contracts/governance/IGovernor.sol"; import {GitcoinGovernorWithGuardian} from "src/GitcoinGovernorWithGuardian.sol"; import {GitcoinGovernorUpgradeTestBase} from "test/helpers/GitcoinGovernorUpgradeTestBase.sol"; -// Exercises the upgrade itself: the new Governor is deployed by the real deploy script, and the -// upgrade proposal — submitted to the old Governor by the real proposal script — is walked -// through passing, failing, and post-upgrade outcomes for control of the Timelock. +// Exercises the upgrade itself: the new Governor, deployed by the real deploy script or bound to +// the live deployment, and the upgrade proposal, submitted to the old Governor by the real +// proposal script or bound to the one live on mainnet, are walked through passing, failing, and +// post-upgrade outcomes for control of the Timelock. abstract contract GovernorUpgradeProposalTest is GitcoinGovernorUpgradeTestBase { function test_NewGovernorHasTheMainnetConfiguration() external view { assertEq(governor.name(), "Gitcoin Governor Charlie"); @@ -40,11 +41,13 @@ abstract contract GovernorUpgradeProposalTest is GitcoinGovernorUpgradeTestBase assertTrue(governor.proposalNeedsQueuing(type(uint256).max)); } - function test_SubmitsTheUpgradeProposalWithTheExpectedActions() external { - _submitUpgradeProposal(); + function test_UpgradeProposalCarriesTheExpectedActions() external { + // Calls the provenance hook directly rather than through _proposeUpgrade, whose scaffolding + // guard would preempt the assertion this test exists to make. + (upgradeProposalId, upgradeProposalDescription) = _fetchOrSubmitUpgradeProposal(); // The id the old Governor assigned matches the id computed from the actions the proposal is - // expected to carry, proving the script proposed exactly the setPendingAdmin + __acceptAdmin + // expected to carry, proving the proposal holds exactly the setPendingAdmin + __acceptAdmin // pair targeting this deployment. assertEq(upgradeProposalId, _upgradeProposalDetails().id); assertEq(OLD_GOVERNOR.state(upgradeProposalId), IGovernor.ProposalState.Pending); @@ -52,7 +55,7 @@ abstract contract GovernorUpgradeProposalTest is GitcoinGovernorUpgradeTestBase } function test_PassedUpgradeProposalTransfersTimelockControlToTheNewGovernor() external { - _submitUpgradeProposal(); + _proposeUpgrade(); // The electorate passes the proposal. _passUpgradeProposal(); @@ -82,7 +85,7 @@ abstract contract GovernorUpgradeProposalTest is GitcoinGovernorUpgradeTestBase } function test_DefeatedUpgradeProposalLeavesTheOldGovernorGoverning() external { - _submitUpgradeProposal(); + _proposeUpgrade(); // The electorate votes the upgrade down. _defeatUpgradeProposal(); @@ -154,6 +157,10 @@ contract GovernorUpgradeProposalMainnetScript is GovernorUpgradeProposalTest { function _fetchOrDeploySystem() internal override returns (GitcoinGovernorWithGuardian) { return _deployGovernorWithMainnetScript(); } + + function _fetchOrSubmitUpgradeProposal() internal override returns (uint256, string memory) { + return _submitUpgradeProposalWithScript(); + } } contract GovernorUpgradeProposalMainnetDeployed is GovernorUpgradeProposalTest { @@ -164,4 +171,22 @@ contract GovernorUpgradeProposalMainnetDeployed is GovernorUpgradeProposalTest { function _fetchOrDeploySystem() internal view override returns (GitcoinGovernorWithGuardian) { return _fetchDeployedGovernor(); } + + function _fetchOrSubmitUpgradeProposal() internal override returns (uint256, string memory) { + return _submitUpgradeProposalWithScript(); + } +} + +contract GovernorUpgradeProposalMainnetProposed is GovernorUpgradeProposalTest { + function _setUpNetwork() internal override { + _createMainnetUpgradeProposalPostSubmissionFork(); + } + + function _fetchOrDeploySystem() internal view override returns (GitcoinGovernorWithGuardian) { + return _fetchDeployedGovernor(); + } + + function _fetchOrSubmitUpgradeProposal() internal view override returns (uint256, string memory) { + return _fetchSubmittedUpgradeProposal(); + } } diff --git a/test/PostUpgradeFranchiserDelegation.integration.t.sol b/test/PostUpgradeFranchiserDelegation.integration.t.sol index fa8f840..3bd1993 100644 --- a/test/PostUpgradeFranchiserDelegation.integration.t.sol +++ b/test/PostUpgradeFranchiserDelegation.integration.t.sol @@ -53,9 +53,8 @@ abstract contract PostUpgradeFranchiserDelegationTest is PostUpgradeFranchiserTe assertEq(_forVotes, _amount); } - /// forge-config: default.fuzz.runs = 25 - /// forge-config: ci.fuzz.runs = 25 - /// forge-config: lite.fuzz.runs = 5 + // This suite's fuzz-run caps sit on the provenance concretes at the bottom of the file: forge + // ignores inline config on test functions inherited from an abstract suite. function testFuzz_PassedDelegationProposalFundsAFreshDelegateeWithAnyTreasuryAmount( uint256 _amount, uint256 _expiration @@ -333,6 +332,9 @@ abstract contract PostUpgradeFranchiserDelegationTest is PostUpgradeFranchiserTe } } +/// forge-config: default.fuzz.runs = 25 +/// forge-config: ci.fuzz.runs = 25 +/// forge-config: lite.fuzz.runs = 5 contract PostUpgradeFranchiserDelegationMainnetScript is PostUpgradeFranchiserDelegationTest { function _setUpNetwork() internal override { _createMainnetFork(); @@ -342,6 +344,35 @@ contract PostUpgradeFranchiserDelegationMainnetScript is PostUpgradeFranchiserDe return _deployGovernorWithMainnetScript(); } + function _fetchOrSubmitUpgradeProposal() internal override returns (uint256, string memory) { + return _submitUpgradeProposalWithScript(); + } + + function _fetchOrDeployFranchiser() + internal + override + returns (FranchiserExpiryFactory, FranchiserLens) + { + return _deployFranchiserWithMainnetScript(); + } +} + +/// forge-config: default.fuzz.runs = 25 +/// forge-config: ci.fuzz.runs = 25 +/// forge-config: lite.fuzz.runs = 5 +contract PostUpgradeFranchiserDelegationMainnetProposed is PostUpgradeFranchiserDelegationTest { + function _setUpNetwork() internal override { + _createMainnetUpgradeProposalPostSubmissionFork(); + } + + function _fetchOrDeploySystem() internal view override returns (GitcoinGovernorWithGuardian) { + return _fetchDeployedGovernor(); + } + + function _fetchOrSubmitUpgradeProposal() internal view override returns (uint256, string memory) { + return _fetchSubmittedUpgradeProposal(); + } + function _fetchOrDeployFranchiser() internal override diff --git a/test/PostUpgradeFranchiserDeploy.integration.t.sol b/test/PostUpgradeFranchiserDeploy.integration.t.sol index 99ceab8..8a3fac9 100644 --- a/test/PostUpgradeFranchiserDeploy.integration.t.sol +++ b/test/PostUpgradeFranchiserDeploy.integration.t.sol @@ -47,6 +47,32 @@ contract PostUpgradeFranchiserDeployMainnetScript is PostUpgradeFranchiserDeploy return _deployGovernorWithMainnetScript(); } + function _fetchOrSubmitUpgradeProposal() internal override returns (uint256, string memory) { + return _submitUpgradeProposalWithScript(); + } + + function _fetchOrDeployFranchiser() + internal + override + returns (FranchiserExpiryFactory, FranchiserLens) + { + return _deployFranchiserWithMainnetScript(); + } +} + +contract PostUpgradeFranchiserDeployMainnetProposed is PostUpgradeFranchiserDeployTest { + function _setUpNetwork() internal override { + _createMainnetUpgradeProposalPostSubmissionFork(); + } + + function _fetchOrDeploySystem() internal view override returns (GitcoinGovernorWithGuardian) { + return _fetchDeployedGovernor(); + } + + function _fetchOrSubmitUpgradeProposal() internal view override returns (uint256, string memory) { + return _fetchSubmittedUpgradeProposal(); + } + function _fetchOrDeployFranchiser() internal override diff --git a/test/PostUpgradeFranchiserExpiry.integration.t.sol b/test/PostUpgradeFranchiserExpiry.integration.t.sol index d63e025..ad57747 100644 --- a/test/PostUpgradeFranchiserExpiry.integration.t.sol +++ b/test/PostUpgradeFranchiserExpiry.integration.t.sol @@ -37,9 +37,8 @@ abstract contract PostUpgradeFranchiserExpiryTest is PostUpgradeFranchiserTestBa assertEq(GTC_TOKEN.getCurrentVotes(_delegatee), 0); } - /// forge-config: default.fuzz.runs = 25 - /// forge-config: ci.fuzz.runs = 25 - /// forge-config: lite.fuzz.runs = 5 + // This suite's fuzz-run caps sit on the provenance concretes at the bottom of the file: forge + // ignores inline config on test functions inherited from an abstract suite. function testFuzz_AnyAccountCanSweepAnExpiredPosition(address _caller) external { address _delegatee = makeAddr("expiringDelegatee"); uint256 _amount = 500_000e18; @@ -136,6 +135,9 @@ abstract contract PostUpgradeFranchiserExpiryTest is PostUpgradeFranchiserTestBa } } +/// forge-config: default.fuzz.runs = 25 +/// forge-config: ci.fuzz.runs = 25 +/// forge-config: lite.fuzz.runs = 5 contract PostUpgradeFranchiserExpiryMainnetScript is PostUpgradeFranchiserExpiryTest { function _setUpNetwork() internal override { _createMainnetFork(); @@ -145,6 +147,35 @@ contract PostUpgradeFranchiserExpiryMainnetScript is PostUpgradeFranchiserExpiry return _deployGovernorWithMainnetScript(); } + function _fetchOrSubmitUpgradeProposal() internal override returns (uint256, string memory) { + return _submitUpgradeProposalWithScript(); + } + + function _fetchOrDeployFranchiser() + internal + override + returns (FranchiserExpiryFactory, FranchiserLens) + { + return _deployFranchiserWithMainnetScript(); + } +} + +/// forge-config: default.fuzz.runs = 25 +/// forge-config: ci.fuzz.runs = 25 +/// forge-config: lite.fuzz.runs = 5 +contract PostUpgradeFranchiserExpiryMainnetProposed is PostUpgradeFranchiserExpiryTest { + function _setUpNetwork() internal override { + _createMainnetUpgradeProposalPostSubmissionFork(); + } + + function _fetchOrDeploySystem() internal view override returns (GitcoinGovernorWithGuardian) { + return _fetchDeployedGovernor(); + } + + function _fetchOrSubmitUpgradeProposal() internal view override returns (uint256, string memory) { + return _fetchSubmittedUpgradeProposal(); + } + function _fetchOrDeployFranchiser() internal override diff --git a/test/PostUpgradeFranchiserRecall.integration.t.sol b/test/PostUpgradeFranchiserRecall.integration.t.sol index 2fe73ee..f20c55c 100644 --- a/test/PostUpgradeFranchiserRecall.integration.t.sol +++ b/test/PostUpgradeFranchiserRecall.integration.t.sol @@ -215,6 +215,32 @@ contract PostUpgradeFranchiserRecallMainnetScript is PostUpgradeFranchiserRecall return _deployGovernorWithMainnetScript(); } + function _fetchOrSubmitUpgradeProposal() internal override returns (uint256, string memory) { + return _submitUpgradeProposalWithScript(); + } + + function _fetchOrDeployFranchiser() + internal + override + returns (FranchiserExpiryFactory, FranchiserLens) + { + return _deployFranchiserWithMainnetScript(); + } +} + +contract PostUpgradeFranchiserRecallMainnetProposed is PostUpgradeFranchiserRecallTest { + function _setUpNetwork() internal override { + _createMainnetUpgradeProposalPostSubmissionFork(); + } + + function _fetchOrDeploySystem() internal view override returns (GitcoinGovernorWithGuardian) { + return _fetchDeployedGovernor(); + } + + function _fetchOrSubmitUpgradeProposal() internal view override returns (uint256, string memory) { + return _fetchSubmittedUpgradeProposal(); + } + function _fetchOrDeployFranchiser() internal override diff --git a/test/PostUpgradeGovernance.integration.t.sol b/test/PostUpgradeGovernance.integration.t.sol index af45004..0f56e06 100644 --- a/test/PostUpgradeGovernance.integration.t.sol +++ b/test/PostUpgradeGovernance.integration.t.sol @@ -9,9 +9,8 @@ import {GitcoinGovernorPostUpgradeTestBase} from "test/helpers/GitcoinGovernorUp // and defeating proposals that move treasury assets held by the Timelock, updating the // Governor's own settings, fractional and by-signature voting, and Timelock expiry. abstract contract PostUpgradeGovernanceTest is GitcoinGovernorPostUpgradeTestBase { - /// forge-config: default.fuzz.runs = 25 - /// forge-config: ci.fuzz.runs = 25 - /// forge-config: lite.fuzz.runs = 5 + // This suite's fuzz-run caps sit on the provenance concretes at the bottom of the file: forge + // ignores inline config on test functions inherited from an abstract suite. function testFuzz_PassedProposalSendsGtcHeldByTheTimelock(uint256 _amount) external { // The proposal draws on the GTC the Timelock genuinely holds at the fork block. uint256 _initialTimelockBalance = GTC_TOKEN.balanceOf(address(TIMELOCK)); @@ -26,9 +25,6 @@ abstract contract PostUpgradeGovernanceTest is GitcoinGovernorPostUpgradeTestBas assertEq(GTC_TOKEN.balanceOf(address(TIMELOCK)), _initialTimelockBalance - _amount); } - /// forge-config: default.fuzz.runs = 25 - /// forge-config: ci.fuzz.runs = 25 - /// forge-config: lite.fuzz.runs = 5 function testFuzz_PassedProposalSendsEthHeldByTheTimelock(uint256 _amount) external { // The proposal draws on the ETH the Timelock genuinely holds at the fork block. uint256 _initialTimelockBalance = address(TIMELOCK).balance; @@ -290,6 +286,9 @@ abstract contract PostUpgradeGovernanceTest is GitcoinGovernorPostUpgradeTestBas } } +/// forge-config: default.fuzz.runs = 25 +/// forge-config: ci.fuzz.runs = 25 +/// forge-config: lite.fuzz.runs = 5 contract PostUpgradeGovernanceMainnetScript is PostUpgradeGovernanceTest { function _setUpNetwork() internal override { _createMainnetFork(); @@ -298,8 +297,15 @@ contract PostUpgradeGovernanceMainnetScript is PostUpgradeGovernanceTest { function _fetchOrDeploySystem() internal override returns (GitcoinGovernorWithGuardian) { return _deployGovernorWithMainnetScript(); } + + function _fetchOrSubmitUpgradeProposal() internal override returns (uint256, string memory) { + return _submitUpgradeProposalWithScript(); + } } +/// forge-config: default.fuzz.runs = 25 +/// forge-config: ci.fuzz.runs = 25 +/// forge-config: lite.fuzz.runs = 5 contract PostUpgradeGovernanceMainnetDeployed is PostUpgradeGovernanceTest { function _setUpNetwork() internal override { _createMainnetGovernorPostDeploymentFork(); @@ -308,4 +314,25 @@ contract PostUpgradeGovernanceMainnetDeployed is PostUpgradeGovernanceTest { function _fetchOrDeploySystem() internal view override returns (GitcoinGovernorWithGuardian) { return _fetchDeployedGovernor(); } + + function _fetchOrSubmitUpgradeProposal() internal override returns (uint256, string memory) { + return _submitUpgradeProposalWithScript(); + } +} + +/// forge-config: default.fuzz.runs = 25 +/// forge-config: ci.fuzz.runs = 25 +/// forge-config: lite.fuzz.runs = 5 +contract PostUpgradeGovernanceMainnetProposed is PostUpgradeGovernanceTest { + function _setUpNetwork() internal override { + _createMainnetUpgradeProposalPostSubmissionFork(); + } + + function _fetchOrDeploySystem() internal view override returns (GitcoinGovernorWithGuardian) { + return _fetchDeployedGovernor(); + } + + function _fetchOrSubmitUpgradeProposal() internal view override returns (uint256, string memory) { + return _fetchSubmittedUpgradeProposal(); + } } diff --git a/test/PostUpgradeProposalGuardian.integration.t.sol b/test/PostUpgradeProposalGuardian.integration.t.sol index ece5b1a..29a1a59 100644 --- a/test/PostUpgradeProposalGuardian.integration.t.sol +++ b/test/PostUpgradeProposalGuardian.integration.t.sol @@ -222,6 +222,10 @@ contract PostUpgradeProposalGuardianMainnetScript is PostUpgradeProposalGuardian function _fetchOrDeploySystem() internal override returns (GitcoinGovernorWithGuardian) { return _deployGovernorWithMainnetScript(); } + + function _fetchOrSubmitUpgradeProposal() internal override returns (uint256, string memory) { + return _submitUpgradeProposalWithScript(); + } } contract PostUpgradeProposalGuardianMainnetDeployed is PostUpgradeProposalGuardianTest { @@ -232,4 +236,22 @@ contract PostUpgradeProposalGuardianMainnetDeployed is PostUpgradeProposalGuardi function _fetchOrDeploySystem() internal view override returns (GitcoinGovernorWithGuardian) { return _fetchDeployedGovernor(); } + + function _fetchOrSubmitUpgradeProposal() internal override returns (uint256, string memory) { + return _submitUpgradeProposalWithScript(); + } +} + +contract PostUpgradeProposalGuardianMainnetProposed is PostUpgradeProposalGuardianTest { + function _setUpNetwork() internal override { + _createMainnetUpgradeProposalPostSubmissionFork(); + } + + function _fetchOrDeploySystem() internal view override returns (GitcoinGovernorWithGuardian) { + return _fetchDeployedGovernor(); + } + + function _fetchOrSubmitUpgradeProposal() internal view override returns (uint256, string memory) { + return _fetchSubmittedUpgradeProposal(); + } } diff --git a/test/PostUpgradeQuorumBehavior.integration.t.sol b/test/PostUpgradeQuorumBehavior.integration.t.sol index 038a3e9..ded63e8 100644 --- a/test/PostUpgradeQuorumBehavior.integration.t.sol +++ b/test/PostUpgradeQuorumBehavior.integration.t.sol @@ -245,6 +245,10 @@ contract PostUpgradeQuorumBehaviorMainnetScript is PostUpgradeQuorumBehaviorTest function _fetchOrDeploySystem() internal override returns (GitcoinGovernorWithGuardian) { return _deployGovernorWithMainnetScript(); } + + function _fetchOrSubmitUpgradeProposal() internal override returns (uint256, string memory) { + return _submitUpgradeProposalWithScript(); + } } contract PostUpgradeQuorumBehaviorMainnetDeployed is PostUpgradeQuorumBehaviorTest { @@ -255,4 +259,22 @@ contract PostUpgradeQuorumBehaviorMainnetDeployed is PostUpgradeQuorumBehaviorTe function _fetchOrDeploySystem() internal view override returns (GitcoinGovernorWithGuardian) { return _fetchDeployedGovernor(); } + + function _fetchOrSubmitUpgradeProposal() internal override returns (uint256, string memory) { + return _submitUpgradeProposalWithScript(); + } +} + +contract PostUpgradeQuorumBehaviorMainnetProposed is PostUpgradeQuorumBehaviorTest { + function _setUpNetwork() internal override { + _createMainnetUpgradeProposalPostSubmissionFork(); + } + + function _fetchOrDeploySystem() internal view override returns (GitcoinGovernorWithGuardian) { + return _fetchDeployedGovernor(); + } + + function _fetchOrSubmitUpgradeProposal() internal view override returns (uint256, string memory) { + return _fetchSubmittedUpgradeProposal(); + } } diff --git a/test/helpers/GitcoinGovernorUpgradeTestBase.sol b/test/helpers/GitcoinGovernorUpgradeTestBase.sol index 19e7978..05c2050 100644 --- a/test/helpers/GitcoinGovernorUpgradeTestBase.sol +++ b/test/helpers/GitcoinGovernorUpgradeTestBase.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: AGPL-3.0-only pragma solidity ^0.8.35; -import {Test} from "forge-std/Test.sol"; +import {Test, Vm} from "forge-std/Test.sol"; import {IGovernor} from "@openzeppelin/contracts/governance/IGovernor.sol"; import {ICompoundTimelock} from "@openzeppelin/contracts/vendor/compound/ICompoundTimelock.sol"; import {GitcoinGovernorWithGuardian} from "src/GitcoinGovernorWithGuardian.sol"; @@ -13,8 +13,9 @@ import {IGtc} from "test/helpers/IGtc.sol"; import {ProposeGovernorUpgradeTestConfig} from "test/helpers/ProposeGovernorUpgradeTestConfig.sol"; // Shared base for the mainnet fork integration suites. Holds the provenance abstraction (how the -// system under test comes into being), the real-delegate electorate, and step helpers for -// walking proposals through their lifecycle on both the old and the new Governor. +// system under test and its upgrade proposal come into being), the real-delegate electorate, and +// step helpers for walking proposals through their lifecycle on both the old and the new +// Governor. abstract contract GitcoinGovernorUpgradeTestBase is Test { // Vote support values shared by both governors' bravo-style counting. uint8 constant AGAINST = 0; @@ -25,6 +26,10 @@ abstract contract GitcoinGovernorUpgradeTestBase is Test { uint256 constant GOVERNOR_PRE_DEPLOYMENT_BLOCK = 25_453_000; uint256 constant GOVERNOR_POST_DEPLOYMENT_BLOCK = 25_776_743; + // The upgrade proposal was submitted to the old Governor on mainnet in transaction + // 0xb589c7fcb916860feefeef54da017800787e48952fc739b6d1ab50b34ca77e89. + uint256 constant UPGRADE_PROPOSAL_SUBMISSION_BLOCK = 26_041_897; + uint256 constant UPGRADE_PROPOSAL_POST_SUBMISSION_BLOCK = 26_041_898; IGovernorBravo constant OLD_GOVERNOR = IGovernorBravo(0x9D4C63565D5618310271bF3F3c01b2954C1D1639); GitcoinGovernorWithGuardian constant DEPLOYED_GOVERNOR = @@ -33,6 +38,8 @@ abstract contract GitcoinGovernorUpgradeTestBase is Test { ICompoundTimelock constant TIMELOCK = ICompoundTimelock(payable(0x57a8865cfB1eCEf7253c27da6B4BC3dAEE5Be518)); address constant PROPOSAL_GUARDIAN = 0x5743E35477363241300FcEdc2F5eB0195F300817; + uint256 constant SUBMITTED_UPGRADE_PROPOSAL_ID = + 0xefb5ffee46ab14020294f926e242a2fa79f096de6577be73d69429207a464489; // kbw.eth — a real delegate whose voting weight (~485k GTC at the pre-deployment block) clears // the 150k @@ -59,6 +66,8 @@ abstract contract GitcoinGovernorUpgradeTestBase is Test { uint256 constant MIN_TIMELOCK_GTC_BALANCE = 1_000_000e18; uint256 constant MIN_TIMELOCK_ETH_BALANCE = 10 ether; + // The description the proposal script submits under the provenances that submit the upgrade + // proposal themselves; the proposal already on mainnet carries its own. string constant UPGRADE_PROPOSAL_DESCRIPTION = "Upgrade the Gitcoin Governor to GitcoinGovernorWithGuardian"; @@ -72,7 +81,10 @@ abstract contract GitcoinGovernorUpgradeTestBase is Test { } GitcoinGovernorWithGuardian governor; + // The upgrade proposal on the old Governor, set by _proposeUpgrade from the concrete's + // provenance. The description is tracked alongside the id because it is hashed into it. uint256 upgradeProposalId; + string upgradeProposalDescription; // The electorate: real delegates whose live voting weights are read from the fork in setUp. // Combined they must clear the 1.5M quorum — asserted in setUp so that a fork-block bump that @@ -152,8 +164,13 @@ abstract contract GitcoinGovernorUpgradeTestBase is Test { function _fetchOrDeploySystem() internal virtual returns (GitcoinGovernorWithGuardian); + // Produces the upgrade proposal on the old Governor and returns its id and description. Unlike + // the two hooks above, setUp does not call this one: tests reach it through _proposeUpgrade at + // the point in their flow where the upgrade is proposed. + function _fetchOrSubmitUpgradeProposal() internal virtual returns (uint256, string memory); + //-------------------------- Provenance implementation helpers --------------------------// - // Provenance concretes at the bottom of each test file implement the two methods above with + // Provenance concretes at the bottom of each test file implement the hooks above with // one-line delegations to these helpers. function _createMainnetFork() internal { @@ -164,6 +181,10 @@ abstract contract GitcoinGovernorUpgradeTestBase is Test { vm.createSelectFork("mainnet", GOVERNOR_POST_DEPLOYMENT_BLOCK); } + function _createMainnetUpgradeProposalPostSubmissionFork() internal { + vm.createSelectFork("mainnet", UPGRADE_PROPOSAL_POST_SUBMISSION_BLOCK); + } + // Deploys the new Governor onto the fork by running the real mainnet deploy script, exactly as // the production deployment will run it. function _deployGovernorWithMainnetScript() internal returns (GitcoinGovernorWithGuardian) { @@ -186,6 +207,56 @@ abstract contract GitcoinGovernorUpgradeTestBase is Test { return DEPLOYED_GOVERNOR; } + // Submits the upgrade proposal by running the proposal script, exactly as a delegate would. + function _submitUpgradeProposalWithScript() internal returns (uint256, string memory) { + ProposeGovernorUpgradeTestConfig _proposeScript = new ProposeGovernorUpgradeTestConfig( + OLD_GOVERNOR, governor, PROPOSER, UPGRADE_PROPOSAL_DESCRIPTION + ); + _proposeScript.disableLogging(); + _proposeScript.run(); + return (_proposeScript.proposalId(), UPGRADE_PROPOSAL_DESCRIPTION); + } + + // Binds to the upgrade proposal already submitted on mainnet, reading its id and description + // from the ProposalCreated event the old Governor emitted. The event's actions are left unread + // on purpose: the tests pair the live description with the actions the _upgradeProposalDetails + // mirror expects, so the ids matching proves the live proposal carries exactly those actions. + function _fetchSubmittedUpgradeProposal() internal view returns (uint256, string memory) { + // The old Governor emits ProposalCreated with the same signature as OZ v5's IGovernor. + bytes32[] memory _topics = new bytes32[](1); + _topics[0] = IGovernor.ProposalCreated.selector; + Vm.EthGetLogs[] memory _logs = vm.eth_getLogs( + UPGRADE_PROPOSAL_SUBMISSION_BLOCK, + UPGRADE_PROPOSAL_SUBMISSION_BLOCK, + address(OLD_GOVERNOR), + _topics + ); + if (_logs.length != 1) { + revert( + string.concat( + "Expected one ProposalCreated event from the old Governor at " + "UPGRADE_PROPOSAL_SUBMISSION_BLOCK but found ", + vm.toString(_logs.length), + "; repair the pinned submission block" + ) + ); + } + (uint256 _proposalId,,,,,,,, string memory _description) = abi.decode( + _logs[0].data, + (uint256, address, address[], uint256[], string[], bytes[], uint256, uint256, string) + ); + if (_proposalId != SUBMITTED_UPGRADE_PROPOSAL_ID) { + revert( + string.concat( + "The proposal created at UPGRADE_PROPOSAL_SUBMISSION_BLOCK has id ", + vm.toString(_proposalId), + " rather than SUBMITTED_UPGRADE_PROPOSAL_ID; repair the pinned block or id" + ) + ); + } + return (_proposalId, _description); + } + //---------------------------------- Scaffolding guards ----------------------------------// // Guards on the state the step helpers expect as they drive proposals through their // lifecycle. Like the setUp guards, these revert rather than assert: a failure means a test @@ -315,9 +386,9 @@ abstract contract GitcoinGovernorUpgradeTestBase is Test { ); } - // The two actions of the upgrade proposal, mirroring what the proposal script builds. The - // tests assert the mirror is faithful by recomputing the script-returned proposal id from - // these actions. + // The two actions of the upgrade proposal, mirroring what the proposal script builds, paired + // with the proposal's description. The tests assert the mirror is faithful by recomputing the + // upgrade proposal's id from these actions. function _upgradeProposalDetails() internal view returns (ProposalDetails memory _proposal) { _proposal.targets = new address[](2); _proposal.values = new uint256[](2); @@ -326,24 +397,21 @@ abstract contract GitcoinGovernorUpgradeTestBase is Test { _proposal.calldatas[0] = abi.encodeCall(ICompoundTimelock.setPendingAdmin, (address(governor))); _proposal.targets[1] = address(governor); _proposal.calldatas[1] = abi.encodeCall(governor.__acceptAdmin, ()); - _proposal.description = UPGRADE_PROPOSAL_DESCRIPTION; + _proposal.description = upgradeProposalDescription; _proposal.id = _hashProposal(_proposal); } //----------------------------- Upgrade proposal (old Governor) -----------------------------// - // Submits the upgrade proposal by running the proposal script, exactly as a delegate would. - function _submitUpgradeProposal() internal { - ProposeGovernorUpgradeTestConfig _proposeScript = new ProposeGovernorUpgradeTestConfig( - OLD_GOVERNOR, governor, PROPOSER, UPGRADE_PROPOSAL_DESCRIPTION - ); - _proposeScript.disableLogging(); - _proposeScript.run(); - upgradeProposalId = _proposeScript.proposalId(); + // Brings the upgrade proposal into being through the provenance hook (submitting it with the + // real proposal script, or binding to the one already on mainnet) and guards that it carries + // the actions the _upgradeProposalDetails mirror expects. + function _proposeUpgrade() internal { + (upgradeProposalId, upgradeProposalDescription) = _fetchOrSubmitUpgradeProposal(); _guardProposalId( upgradeProposalId, _upgradeProposalDetails().id, - "the upgrade proposal script vs the _upgradeProposalDetails mirror" + "the upgrade proposal vs the _upgradeProposalDetails mirror" ); } @@ -397,10 +465,10 @@ abstract contract GitcoinGovernorUpgradeTestBase is Test { ); } - // The full upgrade journey: submit via the proposal script, pass, queue, wait out the - // Timelock delay, execute, and confirm the new Governor now controls the Timelock. + // The full upgrade journey: propose, pass, queue, wait out the Timelock delay, execute, and + // confirm the new Governor now controls the Timelock. function _upgradeToNewGovernor() internal { - _submitUpgradeProposal(); + _proposeUpgrade(); _passUpgradeProposal(); _queueUpgradeProposal(); _jumpPastUpgradeProposalEta();