From c041650a0279224789e149f3de2b386efe695cd8 Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Thu, 22 Oct 2026 11:45:38 -0300 Subject: [PATCH 01/11] docs: add design for removing packages/contracts --- docs/contracts-removal/design.md | 394 +++++++++++++++++++++++++++++++ 1 file changed, 394 insertions(+) create mode 100644 docs/contracts-removal/design.md diff --git a/docs/contracts-removal/design.md b/docs/contracts-removal/design.md new file mode 100644 index 000000000..7065830be --- /dev/null +++ b/docs/contracts-removal/design.md @@ -0,0 +1,394 @@ +# Removing `packages/contracts` — Design + +Status: draft for review (revision 3, after two review passes) +Date: 2026-10-22 + +## Goal + +Delete `packages/contracts` (`@graphprotocol/contracts`) and `packages/contracts-test` from the monorepo. Every contract that is still in use moves to the package that owns its domain, and its unit tests are rewritten in Foundry in that package. Dead code is deleted. Nothing changes onchain: moves are mechanical and must produce identical bytecode, ignoring the metadata hash. + +Out of scope: + +- Porting contracts to Solidity 0.8. +- Any change to contract behaviour. +- Bringing `billing-contracts` into the monorepo. That is a separate sub-project with its own spec. + +## Decisions + +### 1. npm + +- `@graphprotocol/contracts` is frozen at 7.3.0, marked deprecated with `npm deprecate`, and never published again. The git tag `@graphprotocol/contracts@7.3.0` preserves its source, CHANGELOG and docs (`DEPLOYMENT.md`, `TESTING.md`). +- `@graphprotocol/sdk` is already deprecated in its README (which points to `@graphprotocol/toolshed`). It also gets `npm deprecate`. +- Known external consumers (graph-network-subgraph, subgraph-studio, network-monitor) pin old versions and are migrated as needed. Addresses come from `@graphprotocol/address-book`, ABIs from `@graphprotocol/interfaces`. + +### 2. Package layout + +Final dependency graph (arrow = "depends on"): + +``` +graph-core → interfaces +graph-token → graph-core, interfaces +subgraph-discovery → graph-token, graph-core, interfaces +issuance → graph-core, interfaces +toolshed → issuance (exists today) +horizon → subgraph-discovery, graph-token, graph-core, issuance, toolshed, interfaces +subgraph-service → horizon, subgraph-discovery, graph-token, graph-core, toolshed, interfaces +testing → graph-core, horizon, issuance, subgraph-service, interfaces +deployment → graph-core, graph-token, subgraph-discovery, issuance, horizon, subgraph-service + +legacy (new, private, no workspace dependencies) +``` + +Rules that keep this acyclic: + +- `issuance` depends on `graph-core` and `interfaces` only. Its code and tests never use horizon, subgraph-discovery or graph-token, because `horizon → toolshed → issuance` already exists. +- The 0.7.6 packages (`graph-core`, `graph-token`, `subgraph-discovery`, `legacy`) do not use `toolshed` or `hardhat-graph-protocol`. They have their own small Hardhat configs. +- `horizon → issuance` is only for deploying RewardsManager from Ignition. It already exists transitively through toolshed. + +| Package | Contracts | Compiler | +| -------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------ | +| `graph-core` | Controller, Governed, Pausable, Managed, GraphProxy, GraphProxyAdmin, GraphProxyStorage, GraphUpgradeable | solc 0.7.6 only (see §3) | +| `graph-token` | GraphToken (mainnet), L2GraphToken, GraphTokenUpgradeable, TokenUtils, GraphTokenGateway (base), L1GraphTokenGateway, L2GraphTokenGateway, BridgeEscrow, L1ArbitrumMessenger, L2ArbitrumMessenger, AddressAliasHelper | solc 0.7.6 only | +| `subgraph-discovery` | L2GNS, L2GNSStorage, GNS, GNSStorage, L2Curation, CurationStorage, GraphCurationToken, SubgraphNFT, SubgraphNFTDescriptor, HexStrings, Base58Encoder, Multicall | solc 0.7.6 only | +| `horizon` | EpochManager, EpochManagerStorage (in `contracts/legacy-0.7/`) | mixed | +| `issuance` | RewardsManager, RewardsManagerStorage, MathUtils (in `contracts/legacy-0.7/`) | mixed | +| `legacy` | Copies of the mainnet-only contracts and everything they need (see "legacy package" below) | solc 0.7.6 only | + +Why `subgraph-discovery` is its own package: + +- These contracts need OpenZeppelin 3.4.2 under its real name. `ISubgraphNFT` and `IGraphCurationToken` in `@graphprotocol/interfaces` import `@openzeppelin/contracts/token/ERC721/IERC721.sol` and `@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol`. Inside horizon or subgraph-service (Hardhat 2, OpenZeppelin 5), those imports resolve to v5 and fail. An alias cannot fix imports inside `interfaces`. +- horizon's Ignition modules deploy these contracts, so the package must sit below horizon. + +Deleted, with no replacement: + +- Staking: L2Staking. The Arbitrum proxy now runs HorizonStaking. +- AllocationExchange, EthereumDIDRegistry, SubgraphAvailabilityManager. +- The Arbitrum DisputeManager and ServiceRegistry, along with their storage contracts. +- Mocks under `contracts/tests/` that no ported test needs: ArbSysMock, `tests/ens/*`, LegacyGNSMock, and both L1GraphTokenLockTransferTool mocks. +- Each mainnet-only contract is deleted from `packages/contracts` only after its copy exists in `legacy`. + +#### legacy package + +- A frozen, private archive of the mainnet-only contracts: L1GNS, Curation and L1Staking, plus their closure. +- It has no Solidity imports from the workspace. Its closure is copied in, interfaces included, so later edits to `@graphprotocol/interfaces` can never break it. +- Contract files (20): BancorFormula, Multicall, Curation, CurationStorage, GNS, GNSStorage, L1GNS, L1GNSStorage, Managed, L1Staking, L1StakingStorage, Staking, StakingExtension, StakingStorage, Exponential, LibFixedMath, MathUtils, Stakes, GraphUpgradeable, TokenUtils. +- Interface files (copied from `interfaces`): ITokenGateway, IMulticall, ICuration, IGraphCurationToken, IGNS, ISubgraphNFT, IEthereumDIDRegistry, IEpochManager, ICallhookReceiver, IController, IManaged, IL2GNS, IL2StakingTypes, IRewardsIssuer, IRewardsManager, IL1GraphTokenLockTransferTool, IL1StakingBase, IStaking, IStakingBase, IStakingData, IStakingExtension, IStakes, IGraphToken, IGraphProxy. +- It has its own Hardhat config (0.7.6, OpenZeppelin 3.4.2) and a CI build job, so it can't silently stop compiling. +- It has no tests. Instead, CI checks its bytecode against the mainnet implementations (see Phase 0c). +- Historical audit reports from `packages/contracts/audits/` (29 PDFs) move to `packages/legacy/audits/`. + +### 3. Compiler: keep 0.7.6, port only when upgrading + +The source in the repo must match what is deployed, so contracts move unchanged. A contract is ported to 0.8 only when it gets a real upgrade, and the port ships and is audited together with that upgrade. Some contracts are not upgradeable and stay 0.7.6 forever: + +- GraphToken (mainnet) +- Controller +- GraphProxyAdmin +- SubgraphNFT +- GraphCurationToken (the clone master) + +The 0.7.6 settings must reproduce the current build exactly: solc 0.7.6, optimizer enabled, 200 runs, no `evmVersion`, no `viaIR`, default metadata. Everything else follows from that. + +#### graph-core, graph-token, subgraph-discovery and legacy + +- Each has a single Hardhat compiler: 0.7.6 with the settings above. +- graph-token, subgraph-discovery and legacy depend on OpenZeppelin `contracts` and `contracts-upgradeable` 3.4.2 under their real names. +- graph-core has no OpenZeppelin imports of its own. It only reaches `IERC20` through `IGraphToken`, so it depends on OpenZeppelin `^5.4`, whose `IERC20` uses `>=0.4.16`. That keeps graph-core importable from 0.7.6 code, 0.8 code and 0.8 Foundry tests. (EpochManager, RewardsManager and L2GNS were verified to produce identical bytecode with v5's `IERC20`.) +- Ignition and `packages/deployment` always use these packages' own 0.7.6 artifacts. That includes GraphProxy, GraphProxyAdmin and Controller, which are deployed from 0.7.6 builds. +- The graph-core files keep the dual pragma `^0.7.6 || ^0.8.27`, so 0.8 code can import them as source. Managed moves from `^0.7.6` to that dual pragma; it was verified to compile under 0.8.35. +- graph-token adds `arbos-precompiles` (needed by L2ArbitrumMessenger). +- `TokenUtils` (in graph-token) also keeps its dual pragma. It is imported as source by 0.8 code in horizon (HorizonStaking, GraphPayments, PaymentsEscrow) and subgraph-service (SubgraphService, DisputeManager, AllocationManager, AllocationHandler). + +#### horizon and issuance (mixed) + +- Moved files live under `contracts/legacy-0.7/`. +- OpenZeppelin 3.4.2 is installed as the npm aliases `@openzeppelin/contracts-v3` and `@openzeppelin/contracts-upgradeable-v3`. The moved files' own OpenZeppelin imports are rewritten to those aliases (SafeMath, IERC165). + - The aliased upgradeable package never imports `@openzeppelin/contracts` internally, so the alias is safe. + - In horizon (Hardhat 2, which resolves every import from the project root), EpochManager reaches `IERC20` through `IGraphToken` and gets horizon's OpenZeppelin 5 `IERC20` (`>=0.4.16`). This was verified to compile with identical bytecode. horizon's `@openzeppelin/contracts` floor goes from `^5.0.2` to `^5.4.0`, because that pragma depends on it. + - In issuance (Hardhat 3, which resolves imports per package), `IGraphToken` inside `interfaces` probably resolves to interfaces' own OpenZeppelin 3.4.2. That's fine for the 0.7.6 RewardsManager, but step 5 must confirm it with a real build. Rule: issuance's 0.8 code never imports graph-core's Managed or graph-token's TokenUtils. (issuance already has its own `contracts/common/IGraphToken.sol`.) +- Hardhat: + - horizon (Hardhat 2 with the toolshed base config) gets `solidity.compilers: [, <0.7.6 settings>]`. Hardhat 2 has no per-folder override; it selects 0.7.6 from the `^0.7.6` pragma. + - issuance (Hardhat 3, `issuance/hardhat.base.config.ts`) adds a 0.7.6 entry next to its existing 0.8 compiler. + - The 0.7.6 entry must not inherit `viaIR` or `evmVersion: cancun`. +- Foundry: `skip = ["contracts/legacy-0.7/**"]`. `lint:forge`, solhint and prettier also ignore `legacy-0.7/`. +- CI rule: no file outside `legacy-0.7/` may import the `-v3` aliases (grep check). +- horizon already has its own `Managed` (`contracts/staking/utilities/Managed.sol`). Consumers must refer to graph-core's Managed by its fully qualified name in artifacts, typechain and Ignition. + +### 4. Tests + +- `packages/contracts-test` is deleted. Each package owns its unit tests, written in **Foundry**, following the existing suites in horizon, subgraph-service and issuance: + - a shared base test contract (`GraphBase.t.sol`, `SubgraphBaseTest.t.sol`); + - `shared/` helpers and `mocks/`; + - the same directory layout, naming and fuzzing style. + + Each new package copies the conventions of its nearest neighbour. + +- Tests are written **before** the contracts move (Phase T): + - Test files are 0.8, because forge-std needs `>=0.8.13`. They never import 0.7.6 source. + - They deploy the contracts under test from compiled artifacts: + - **Before the move:** the Hardhat JSON in `@graphprotocol/contracts/artifacts`, read with `vm.readFile` + `vm.parseJsonBytes(".bytecode")` and deployed with `create`. `packages/testing/test/harness/RealRewardsHarness.t.sol` is the template. + - **After the move:** + - In graph-core, graph-token and subgraph-discovery, Foundry compiles the 0.7.6 sources itself (it handles several solc versions), and tests switch to `deployCode("X.sol:X")` so coverage registers. + - In horizon and issuance, tests keep reading the package's own Hardhat artifact under `legacy-0.7/`. + - Tests interact through interfaces they can compile under 0.8: + - In graph-core and in OpenZeppelin-5 packages (horizon, issuance), they use `@graphprotocol/interfaces`. + - In graph-token and subgraph-discovery (OpenZeppelin 3.4.2), `IGraphToken`, `ISubgraphNFT` and `IGraphCurationToken` can't be imported from 0.8. Tests there use local minimal interfaces or OpenZeppelin 5's `IERC20` from a test-only remapping. + - Mocks are 0.8 contracts in the test tree. If a mock needs a 0.7.6 base, it is deployed from an artifact instead. + - Each package lists its artifact folders explicitly in `foundry.toml` `fs_permissions`, because pnpm symlinks point outside the package. + - `forge test` depends on Hardhat artifacts, so each package's `test` script builds first. That already holds where `test` runs `pnpm build`. +- **Shared fixture.** Upgradeable contracts have an `onlyImpl` `initialize`, so they must sit behind a GraphProxy and be activated through GraphProxyAdmin `upgrade` + `acceptProxyAndCall`. Each package's base test contract has a small helper that: + - deploys Controller and GraphProxyAdmin from graph-core's artifacts; + - unpauses the protocol (the Controller constructor leaves it paused); + - registers mocks with `setContractProxy` and calls `syncAllContracts`; + - deploys the contract under test behind a proxy. + + Contract-specific setup the ports must reproduce: + - **L2GNS:** SubgraphNFT (minter and descriptor set), a real L2Curation with a GraphCurationToken master, an L2GraphToken with a minter, and a registered L2 gateway. + - **L2Curation:** `setSubgraphService` before `collect`. + - **L2GraphTokenGateway:** `vm.mockCall` / `vm.etch` at ArbSys `0x64`. + +- **Unit tests use mocks, not the full network.** A test deploys only the contract under test plus mocks for its dependencies. Issuance tests in particular mock EpochManager, Curation, Staking, GRT and the subgraph service, and never load artifacts from horizon or subgraph-discovery (see the acyclicity rules in §2). Test cases that really exercise a cross-package flow, such as the allocation path through staking, curation and rewards, move to `packages/testing`, the existing cross-package Foundry integration suite. +- **Coverage.** Phase 0d records per-contract **line** coverage from `contracts-test` (solidity-coverage) before anything changes. Branch numbers aren't comparable between solidity-coverage and `forge coverage`, so they're informative only. + - In graph-core, graph-token and subgraph-discovery, `forge coverage` line coverage after the move must be at least the baseline. + - EpochManager and RewardsManager live in Foundry-skipped `legacy-0.7/` folders, so Foundry can't measure coverage for them. For those two, completeness is enforced by the old-to-new test mapping table. + +#### Test assignment + +| Destination | Source tests (`contracts-test/tests/unit`) | Approx. lines | +| ---------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------- | +| issuance (+ `packages/testing` for allocation flows) | `rewards/*` except `subgraphAvailability.test.ts` | 6.8k | +| subgraph-discovery | `l2/l2GNS.test.ts`, `l2/l2Curation.test.ts`, `lib/gnsUtils.ts` | 1.8k | +| graph-token | `gateway/*`, `l2/l2GraphToken.test.ts`, `l2/l2GraphTokenGateway.test.ts`, `l2/l2ArbitrumMessengerMock.ts`, `graphToken.test.ts`, `lib/graphTokenTests.ts` | 1.6k | +| graph-core | `governance/*`, `upgrade/admin.test.ts` | 0.5k | +| horizon | `epochs.test.ts` | 0.2k | + +Notes: + +- Today `l2GNS.test.ts` stakes and allocates through L2Staking. Its port uses a staking mock instead. +- `controller.test.ts`, `pausing.test.ts` and `admin.test.ts` use EpochManager, Staking and Curation. Their ports use Managed or GraphUpgradeable mocks instead. +- `l2GraphTokenGateway.test.ts` uses RewardsManager. Its port uses a plain callhook receiver mock. +- `lib/gnsUtils.ts` is also imported by `gns.test.ts`, which is dropped but stays until D1. T3 deletes `gns.test.ts` together with `gnsUtils.ts`. + +Dropped, because their contracts are deleted or legacy: + +- `gns.test.ts`: it tests L1GNS and is already `describe.skip`. +- `curation/*` (L1 Curation), `disputes/*`, `payments/*`, `serviceRegisty.test.ts`. +- `staking/*`, `l2/l2Staking.test.ts`, `rewards/subgraphAvailability.test.ts`. + +#### Mocks needed by ported tests + +| Mock | Destination | +| ------------------------------------------------------------------------------------ | --------------------------------------------------------------------------------------- | +| GovernedMock | graph-core | +| BridgeMock, InboxMock, OutboxMock, CallhookReceiverMock | graph-token | +| MockERC165, MockIssuanceAllocator, MockRewardsEligibilityOracle, MockSubgraphService | issuance (reuse its existing 0.8 mocks where they exist; otherwise rewrite them in 0.8) | + +Every test-port PR includes a table mapping each old test case to its new Foundry test (or to `packages/testing`), or to "dropped" with a reason. + +### 5. Tooling and infrastructure + +#### Ignition + +- horizon's periphery modules switch their artifact imports: + - Curation and GNS: to `subgraph-discovery`. + - RewardsManager: to `issuance`. + - GraphToken and GraphTokenGateway: to `graph-token`. + - Controller, GraphProxyAdmin and GraphProxy: to `graph-core`. + - EpochManager: to horizon's own artifacts. + + The same applies to `proxy/GraphProxy.ts`, `migrate/migrate-4.ts` and `core/HorizonStaking.ts`. + +- subgraph-service's `ignition/modules/GNS.ts` (SubgraphNFT, L2GNS) and `Curation.ts` (L2Curation) switch to `subgraph-discovery`. +- issuance's artifacts are Hardhat 3 format (`hh3-artifact-1`). Step 5 confirms that horizon's Hardhat 2 Ignition deploy, the journal check, `pre-verify` and verification all work with them. If not, the fix is to ship a Hardhat 2-shaped copy of the RewardsManager artifact from issuance. + +- subgraph-service's `ignition/modules/DisputeManager.ts` and `ServiceRegistry.ts` use the legacy DisputeManager and ServiceRegistry only through `contractAt`. They switch to artifacts compiled by `@graphprotocol/interfaces` for `IDisputeManager` / `IServiceRegistry`. If those aren't suitable, the ABIs are committed as artifact-shaped JSON under `subgraph-service/ignition/`. +- Existing deployment journals under `ignition/deployments/*` reference the old artifacts. The first move PR verifies that re-running the horizon and subgraph-service modules against the existing Arbitrum Sepolia deployment plans no new futures, i.e. the changed metadata hash doesn't trigger a redeploy. If it does, the fix is documented and applied before any other move. + +#### Verification and scripts + +- `horizon/scripts/pre-verify` and `subgraph-service/scripts/pre-verify` copy `../contracts/artifacts`. They switch to the new packages. +- `horizon/tenderly.config.json` `externalArtifacts` switches the same way. + +#### `packages/deployment` + +- The `'contracts'` artifact source is replaced by one source per package: `graph-core`, `graph-token`, `subgraph-discovery`, `issuance`, `horizon`. This touches: + - `lib/artifact-loaders.ts`, `lib/contract-registry.ts`, `lib/deploy-implementation.ts`, `lib/deployment-validation.ts` + - `scripts/generate-abis.ts`, `test/bytecode-comparison.test.ts` + - `docs/Design.md`, and the `package.json` dependency and `build:clean` script +- `tasks/verify-contract.ts` gets cases for the new packages and supports nested source paths (`contracts/legacy-0.7/...`). Each new package has a Hardhat config with `hardhat-verify`. + +#### `packages/testing` + +- Imports switch to `graph-core`. +- The RewardsManager artifact path moves to issuance (`artifacts/contracts/legacy-0.7/...`), along with `foundry.toml` `fs_permissions`. +- The PR checks that `RealRewardsHarness` loads the real RewardsManager. Today it silently falls back to a mock if the artifact is missing. + +#### Removed tooling and CI + +- `packages/contracts/task` (`@graphprotocol/contracts-task`, the old sdk-based ops tooling) is deleted, not ported. `packages/deployment` replaces it. +- `.github/workflows/verifydeployed.yml` is deleted. It depends on contracts-task. +- `publish.yml` drops `contracts` and adds `graph-core`, `graph-token` and `subgraph-discovery`. +- The root `lint:yaml` drops `packages/contracts/task/config`. +- `count-patterns.txt` paths are updated. +- `@graphprotocol/sdk` is removed from the `pnpm-workspace.yaml` catalog and from `minimumReleaseAgeExclude`. +- `build-test.yml` coverage collection is updated for the removed packages and the new ones. +- The Docker image (`Dockerfile`, `publish-image.yml`) is checked for use of `packages/contracts` or the sdk before deletion. + +#### token-distribution + +- Its three tests that import `Staking` / `Staking__factory` switch to an interface from `@graphprotocol/interfaces` or a local mock. +- The `@graphprotocol/contracts` dependency is removed. + +#### address-book + +Address books are symlinked from each package and listed in `address-book/scripts/sources.js`. + +- New `packages/legacy/addresses.json` with every L1 entry from `packages/contracts/addresses.json`: + - mainnet (1) and Sepolia (11155111); + - including Controller, EpochManager, RewardsManager, DisputeManager, ServiceRegistry, GraphProxyAdmin and SubgraphNFT. It is the L1 deployment record, even for contracts whose source isn't in `legacy`. +- `'legacy'` is added to `sources.js`. +- GraphCurationToken and SubgraphNFTDescriptor (live on Arbitrum One and Arbitrum Sepolia, today only in `packages/contracts/addresses.json`) are added to `horizon/addresses.json`. +- L2 token and gateway entries stay in `horizon/`, where they are today. +- Dropped: + - Goerli (5) and Arbitrum Goerli (421613); + - Arbitrum entries for deleted contracts; + - `arbitrum-addresses.json` (Arbitrum bridge infra, used only by the sdk); + - `addresses-staging.json` and `addresses-local.json`. + +#### Publishing + +`graph-core`, `graph-token` and `subgraph-discovery` follow issuance's package setup: + +- `files`: contracts, artifacts, types and README +- `license: GPL-2.0-or-later` +- starting version 1.0.0 +- an initial changeset + +horizon and subgraph-service add the new packages as devDependencies, the same way they list `@graphprotocol/contracts` today. They publish build output only. `legacy` is `"private": true`. + +#### Docs and references updated + +- Root `CLAUDE.md`, `README.md` and `.gitignore`. +- `docs/IGraphProxyAdminInterfaceFix.md`, `docs/ForgeLintSymlinkIssue.md`, `docs/Linting.md`. +- `issuance/docs/testing/reo/*TestPlan.md`. +- toolshed's `src/deployments/{horizon,subgraph-service}/contracts.ts` comments. + +## PR sequence + +Tests are ported first, against the contracts where they are today. Then dead code and the old suite are removed. Only then do the live contracts move, bottom-up. Because of this order, `packages/contracts` never imports from a package that depends on it, so there are no cyclic workspace dependencies. Every PR leaves the build, all test suites and `packages/deployment` working. + +### Phase 0: preparation + +- **0a.** Delete `packages/contracts/task`, `verifydeployed.yml`, and the root `lint:yaml` entry. +- **0b.** Move token-distribution's tests off the `Staking` types and drop its `@graphprotocol/contracts` dependency. +- **0c.** Add a bytecode equivalence check that builds on the existing `scripts/compare-repo-contract-bytecode-excluding-metadata.mjs` and `scripts/bytecode-diff.sh`. It compares bytecode with metadata stripped for: + - every moved 0.7.6 contract, against a reference committed from the current `packages/contracts` build; + - the 0.8 contracts whose imports change (HorizonStaking, GraphPayments, PaymentsEscrow, SubgraphService, DisputeManager and their libraries), against the current horizon and subgraph-service builds; + - `legacy` and graph-token's mainnet contracts (GraphToken, L1GraphTokenGateway, BridgeEscrow, L1GNS, Curation, L1Staking, StakingExtension), against the onchain implementation bytecode on mainnet. This part applies from D2, when `legacy` exists. If the current source no longer reproduces a mainnet implementation, `legacy` takes that contract's source from the git tag or commit it was deployed from, and the mismatch is recorded in its README. + + Every later PR that moves or copies contracts must pass it. + +- **0d.** Run `contracts-test` coverage and commit the per-contract line coverage as the parity baseline. + +### Phase T: Foundry test ports (parallel) + +Each PR: + +- creates the destination package skeleton if needed: `package.json`, `foundry.toml`, a 0.7.6 Hardhat config (graph-core, graph-token and subgraph-discovery don't use toolshed), `test/`; +- adds `@graphprotocol/contracts` as a temporary devDependency; +- writes the Foundry tests against its artifacts, following §4; +- deletes the matching TypeScript tests from `contracts-test`. + +New packages aren't published yet: they stay `"private": true` until their contracts move in Phase 1. + +- **T1.** graph-core. Also proves the 0.8-tests-against-0.7.6-artifacts pattern and the shared proxy/Controller fixture. +- **T2.** graph-token. +- **T3.** subgraph-discovery. Also deletes `gns.test.ts` and `lib/gnsUtils.ts`. +- **T4.** horizon (epochs). +- **T5.** issuance (rewards, with mocks only) plus the allocation-flow cases in `packages/testing`. This PR may be split by test file group. + +### Phase D: removal of dead code + +- **D1.** Delete `packages/contracts-test`, `packages/contracts/test/` (tx logs, reports), and the `test` / `test:coverage` scripts in `packages/contracts`. Remove `@graphprotocol/sdk` (catalog and `minimumReleaseAgeExclude`). Update `build-test.yml` coverage collection. All live tests have been ported by now. +- **D2.** Create `legacy` as copies of the mainnet-only contracts, verified by 0c against mainnet bytecode. Add `legacy/addresses.json`, the `sources.js` entry, its CI build job, and move the audits to `legacy/audits/`. +- **D3.** Delete from `packages/contracts` exactly these contract files, plus their storage contracts where listed: + - L1GNS, L1GNSStorage, Curation, BancorFormula; + - Staking, L1Staking, L1StakingStorage, StakingExtension, StakingStorage, Stakes, Exponential, LibFixedMath, L2Staking; + - DisputeManager, DisputeManagerStorage, ServiceRegistry, ServiceRegistryStorage; + - AllocationExchange, EthereumDIDRegistry, SubgraphAvailabilityManager. + + Also delete: + - the unused mocks listed in §2; + - `addresses-staging.json`, `addresses-local.json`, `arbitrum-addresses.json`; + - `config/`, and `configDir` in `index.js`. + + Live files that also have copies in `legacy` stay: Multicall, GNS, GNSStorage, CurationStorage, Managed, GraphUpgradeable, TokenUtils and MathUtils. Nothing that remains imports a deleted file. The same PR updates subgraph-service's legacy DisputeManager and ServiceRegistry Ignition imports (§5). + +After D3, `packages/contracts` contains only live contracts. + +### Phase 1: moves, bottom-up (one PR each, `git mv` to keep history) + +Each PR: + +- moves the files; +- switches every consumer's Solidity imports, Ignition artifacts, `deployment` sources and test artifact paths; +- passes 0c; +- removes the moved package's temporary `@graphprotocol/contracts` devDependency; +- flips the moved package to published (drops `"private"`, adds a `publish.yml` option and an initial changeset). + +Dependency edges added in each step (removing the devDependency is what prevents a cycle): + +| Step | Moves | Edges added | +| ---- | --------------------------------------- | -------------------------------------------------------------------------------------------------------- | +| 1 | graph-core | contracts, horizon, subgraph-service, testing, deployment → graph-core | +| 2 | graph-token | contracts, horizon, subgraph-service, deployment → graph-token. graph-token → graph-core | +| 3 | subgraph-discovery | horizon, subgraph-service, deployment → subgraph-discovery. subgraph-discovery → graph-token, graph-core | +| 4 | EpochManager into horizon | horizon → graph-core (already present) | +| 5 | RewardsManager, MathUtils into issuance | issuance → graph-core. horizon, testing, deployment → issuance | + +Per-step notes: + +1. **graph-core.** + - Generalizes `deployment`'s artifact loader to one source per package, and adds the new package cases to `tasks/verify-contract.ts`. + - Updates `pre-verify` and `tenderly.config.json`. + - Switches `packages/testing`'s Solidity imports. + - Verifies that the Ignition journals see no new futures. +2. **graph-token.** +3. **subgraph-discovery.** Also adds GraphCurationToken and SubgraphNFTDescriptor to `horizon/addresses.json`, and switches subgraph-service's GNS and Curation Ignition modules. +4. **horizon gets EpochManager.** + - Proves the two-compiler config, the `-v3` alias, the Foundry skip, the lint ignores and the `-v3` CI grep rule. + - Bumps horizon's OpenZeppelin floor to `^5.4.0`. +5. **issuance gets RewardsManager and MathUtils.** + - Confirms the Hardhat 3 compile and artifact compatibility (§3, §5). + - Moves `packages/testing`'s RewardsManager artifact path and `fs_permissions`. + - Scheduled when no RewardsManager PRs are in flight. + +### Phase 2: final removal + +- **2a.** Delete the now-empty `packages/contracts` and every remaining reference: + - `count-patterns.txt` and the `publish.yml` `contracts` option; + - the docs listed in §5; + - after checking that the Docker image no longer uses `packages/contracts`. +- **2b.** Run `npm deprecate` on `@graphprotocol/contracts` and `@graphprotocol/sdk`. This is a manual step after 2a. + +## Verification + +- **Every move or copy PR:** + - 0c passes. + - `pnpm build` and `pnpm test` pass across the workspace. + - `packages/deployment` sync/status against an Arbitrum Sepolia fork reports no code changes. +- **Step 1 additionally:** the Ignition modules plan no new futures against the existing deployments. +- **Test-port PRs:** + - the mapping table is complete; + - for packages Foundry compiles, coverage is at least the 0d baseline after the move. +- **Final:** a repo-wide search for `@graphprotocol/contracts` (outside historical Ignition build-info and CHANGELOGs) returns nothing. + +## Risks + +- **Merge conflicts.** RewardsManager, L2GNS and L2Curation are under active development. Keep T3, T5 and Phase 1 steps 3 and 5 short-lived, and coordinate timing with open work. +- **Ignition reconciliation.** If Ignition treats the metadata change as a modified artifact for executed futures, step 1 must resolve it before other moves land. +- **Two OpenZeppelin versions in horizon and issuance.** Limited to `legacy-0.7/`, and enforced by the CI grep rule. In issuance, Hardhat 3 resolves imports per package, so the import rule in §3 also matters there. +- **Hardhat 3 artifacts consumed by Hardhat 2 tooling.** horizon's Ignition and verification tooling read the RewardsManager artifact from issuance. Step 5 validates this, with a Hardhat 2-shaped copy as the fallback. +- **Mainnet source drift.** Current source may not reproduce every mainnet implementation exactly. D2's onchain comparison catches this, and `legacy` then uses the source from the deployed tag. +- **Test port fidelity.** A rewrite can silently drop behaviours. The mapping table, the coverage baseline and keeping `packages/testing` integration cases guard against this. +- **Library link references.** Only L1Staking (legacy) links a library (`Exponential`). The 0c check covers it. None of the live moved contracts link libraries. From 0b7bd28cfb297cb8c103573f7ed28aa77dd16d30 Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Thu, 22 Oct 2026 11:45:40 -0300 Subject: [PATCH 02/11] docs: add phase 0 implementation plan for contracts removal --- docs/contracts-removal/phase-0-plan.md | 372 +++++++++++++++++++++++++ 1 file changed, 372 insertions(+) create mode 100644 docs/contracts-removal/phase-0-plan.md diff --git a/docs/contracts-removal/phase-0-plan.md b/docs/contracts-removal/phase-0-plan.md new file mode 100644 index 000000000..8e1dfe09f --- /dev/null +++ b/docs/contracts-removal/phase-0-plan.md @@ -0,0 +1,372 @@ +# Contracts Removal — Phase 0 (Preparation) Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Land the four preparation PRs that every later phase depends on: + +- 0a: delete the legacy task package. +- 0b: decouple token-distribution. +- 0c: a bytecode equivalence check. +- 0d: a coverage baseline. + +**Architecture:** + +- 0a and 0b are pure removals and import swaps. +- 0c adds a small Node script with a committed manifest (contract → artifact path) and reference (contract → hashes of metadata-stripped bytecode). Every later move PR updates the manifest and must keep the hashes identical. CI runs it after the build. +- 0d adds an lcov summarizer and commits the per-file line coverage of `contracts-test` as the parity baseline for the Foundry ports. + +**Tech Stack:** pnpm workspaces, Node 24 (`node:test`, `node:crypto`), Hardhat 2 / solidity-coverage (contracts-test), GitHub Actions. + +**Spec:** `docs/contracts-removal/design.md` (sections "PR sequence → Phase 0", §5 "Removed tooling and CI", §5 "token-distribution", §4 "Coverage"). + +Later phases (T, D, 1, 2) get their own plans, written once Phase 0 has landed, because they depend on what 0c and 0d produce. + +## Global Constraints + +- **Prerequisite:** merge the `docs/contracts-package-removal-spec` branch (spec and this plan) to `main` before starting, so every task branch has both documents. +- Package manager: pnpm. Run `pnpm install` after any `package.json` change and commit `pnpm-lock.yaml`. +- One PR per task. Branch off `main`: `chore/contracts-removal-0a-delete-task`, `…-0b-token-distribution`, `…-0c-bytecode-check`, `…-0d-coverage-baseline`. +- Commits use conventional messages (`chore:`, `test:`, `feat:`, `ci:`). Do NOT add `Co-Authored-By` lines. +- No contract source changes in Phase 0. +- Bytecode comparison strips the exact CBOR metadata blob `a264697066735822<68 hex>64736f6c6343<6 hex>0033`, wherever it occurs. Any other difference, including bytes after the metadata, is a failure. +- `reference.json` hashes never change. Later PRs may change `manifest.json` paths. Entries are removed from both files only when their contract is deleted. +- Migration helper files live in `scripts/contracts-migration/`. They are deleted in Phase 2a. +- Node test runner: always pass a glob such as `node --test 'scripts/**/*.test.mjs'`. On Node 24, a directory argument is treated as a single test file and fails. + +## Review Focus + +1. **Missing artifact** (package not built, or a stale manifest path). `check-bytecode` must exit 1 and name the missing path on stderr. It must never skip the entry and pass. → Task 3 test `fails when an artifact is missing`. +2. **Partial reference on `--write`.** If any manifest artifact is missing, `--write` must exit 1 without touching `reference.json`. → Task 3 test `write refuses to produce a partial reference`. +3. **Metadata-only vs code differences.** + - Artifacts that differ only in the metadata hash must pass. + - A one-byte difference before the metadata must fail, and so must one after it (GraphProxy, PaymentsEscrow and others have real bytes after the metadata in their creation bytecode). + - → Task 3 tests `ignores metadata-only differences`, `detects code differences` and `detects differences after the metadata`. +4. **Empty bytecode.** A manifest entry pointing at an interface or abstract contract (`bytecode` = `0x`) must fail, not hash the empty string. → Task 3 test `rejects empty bytecode`. +5. **lcov path normalization.** solidity-coverage writes absolute `SF:` paths whose ancestors can also be named `contracts`; locally and in CI the repo root itself is `contracts`. Keys must be `contracts/…` measured from the _last_ `contracts` segment. `LF:0` gets `pct: null`. → Task 4 tests. + +--- + +### Task 1 (PR 0a): Delete `packages/contracts/task` and `verifydeployed.yml` + +**Files:** + +- Delete: `packages/contracts/task/` (entire directory; package `@graphprotocol/contracts-task`) +- Delete: `.github/workflows/verifydeployed.yml` +- Modify: `package.json` (root) — in `scripts.lint:yaml`, remove `packages/contracts/task/config/*.yml` +- Modify: `packages/testing/foundry.toml:35-38` — the comment's example path mentions `@graphprotocol/contracts/task/...`. Reword it to "this package's pnpm-hoisted node_modules" without that example, and keep `lint_on_build = false`. +- Modify: `pnpm-lock.yaml` (regenerated) + +**Interfaces:** + +- Consumes: nothing. +- Produces: no `@graphprotocol/contracts-task` package in the workspace. + +- [ ] **Step 1: Confirm nothing else depends on the package** + +Run: `grep -rn "contracts-task\|contracts/task" --exclude-dir=node_modules --exclude-dir=.git --exclude-dir=artifacts --exclude-dir=cache . | grep -v '^./packages/contracts/task/' | grep -v '^./docs/contracts-removal/'` +Expected: only `package.json` (lint:yaml), `pnpm-lock.yaml` and `packages/testing/foundry.toml`. + +- [ ] **Step 2: Delete the directory and workflow, then edit the root `lint:yaml` and the `testing/foundry.toml` comment as listed above** + +- [ ] **Step 3: Reinstall and build** + +Run: `pnpm install && pnpm build` +Expected: install succeeds with no reference to `@graphprotocol/contracts-task`, and the build succeeds. + +- [ ] **Step 4: Run the affected suites** + +Run: `pnpm --filter @graphprotocol/contracts-tests test:self && pnpm --filter @graphprotocol/testing test:self && pnpm lint:yaml` +Expected: all pass. contracts-tests only uses `configDir` from `@graphprotocol/contracts`, never the task package. + +- [ ] **Step 5: Commit and open PR** + +```bash +git add -A packages/contracts/task .github/workflows/verifydeployed.yml package.json packages/testing/foundry.toml pnpm-lock.yaml +git commit -m "chore: remove legacy contracts-task package and verifydeployed workflow" +``` + +PR description: + +- The task package is replaced by `packages/deployment`. +- `verifydeployed.yml` is already broken: its `verify-defender` task exists only in the task package. Its network options include dead networks. + +--- + +### Task 2 (PR 0b): Move token-distribution tests off `@graphprotocol/contracts` + +**Files:** + +- Modify: `packages/token-distribution/test/tokenLockWallet.test.ts:4,215,263` +- Modify: `packages/token-distribution/test/l2TokenLockManager.test.ts:4,78` +- Modify: `packages/token-distribution/test/l1TokenLockTransferTool.test.ts:4` +- Modify: `packages/token-distribution/package.json` — remove the `"@graphprotocol/contracts": "workspace:^"` devDependency +- Modify: `pnpm-lock.yaml` + +**Interfaces:** + +- Consumes: the IStaking ABI from `@graphprotocol/interfaces/artifacts/contracts/contracts/staking/IStaking.sol/IStaking.json`, and ethers v5's `Contract` (token-distribution's own `ethers`, 5.8.0). The tests call only `stake(uint256)` and `setController(address)` through `lockAsStaking`, plus the `.address` property. +- Do NOT use `@graphprotocol/interfaces/types-v5`. At runtime its factories `require("ethers")` from the interfaces package, which is v6, and return v6 contracts without `.address`. Under token-distribution's node10 resolution it also fails to type-check. +- Produces: token-distribution no longer depends on `@graphprotocol/contracts`. + +- [ ] **Step 1: Record the current state on `main`** + +Run: + +```bash +cd packages/token-distribution && pnpm build && npx tsc --noEmit --incremental false > /tmp/td-tsc-before.txt +STUDIO_API_KEY= pnpm test:self 2>&1 | tail -40 +``` + +Expected: `td-tsc-before.txt` holds the existing type errors, which are pre-existing and unrelated. Record the mocha `passing` count. + +- [ ] **Step 2: Swap the imports in all three files** + +- Remove the `@graphprotocol/contracts/types` import. +- Add `import { Contract } from 'ethers'` (merge it into an existing `ethers` import if there is one) and `import IStakingArtifact from '@graphprotocol/interfaces/artifacts/contracts/contracts/staking/IStaking.sol/IStaking.json'`. +- Replace each `Staking__factory.connect(tokenLock.address, deployer.signer)` with `new Contract(tokenLock.address, IStakingArtifact.abi, deployer.signer)`. +- Type `lockAsStaking` as `Contract` (in `l2TokenLockManager.test.ts:78`, replace `Staking`). + +If `resolveJsonModule` is not enabled in token-distribution's `tsconfig.json`, use `require(...)` the same way other JSON is loaded in that package, rather than changing the tsconfig. + +- [ ] **Step 3: Remove the dependency and reinstall** + +Remove the devDependency from `packages/token-distribution/package.json`, then run `pnpm install`. +Expected: the lockfile no longer links `@graphprotocol/contracts` into token-distribution. + +- [ ] **Step 4: Type-check for new errors only** + +Run: `cd packages/token-distribution && npx tsc --noEmit --incremental false > /tmp/td-tsc-after.txt; diff /tmp/td-tsc-before.txt /tmp/td-tsc-after.txt` +Expected: no lines that appear only in the "after" file. Hardhat's ts-node runs without type checking, so this is the only type gate. + +- [ ] **Step 5: Run the tests** + +Run: `cd packages/token-distribution && STUDIO_API_KEY= pnpm test:self 2>&1 | tail -40` +Expected: the same `passing` count as Step 1, with 0 failing and a non-zero exit code on failure. Without `STUDIO_API_KEY` the script prints a warning and exits 0 without running anything. If no key is available locally, say so in the PR and confirm the count in CI's log, where the key is set. + +- [ ] **Step 6: Commit and open PR** + +```bash +git add packages/token-distribution/test packages/token-distribution/package.json pnpm-lock.yaml +git commit -m "test(token-distribution): use IStaking ABI from interfaces instead of contracts types" +``` + +PR description: mention that `scripts/test` skips everything and exits 0 when `STUDIO_API_KEY` is unset. That's out of scope here. + +--- + +### Task 3 (PR 0c): Bytecode equivalence check + +**Files:** + +- Create: `scripts/lib/bytecode.mjs` +- Modify: `scripts/compare-repo-contract-bytecode-excluding-metadata.mjs` — delete its local `stripMetadata` and import it from `./lib/bytecode.mjs` +- Create: `scripts/contracts-migration/check-bytecode.mjs` +- Create: `scripts/contracts-migration/check-bytecode.test.mjs` +- Create: `scripts/contracts-migration/manifest.json` +- Create: `scripts/contracts-migration/reference.json` (generated) +- Modify: `package.json` (root) — add scripts `"check:bytecode": "node scripts/contracts-migration/check-bytecode.mjs"` and `"test:scripts": "node --test 'scripts/**/*.test.mjs'"` +- Modify: `.github/workflows/build-test.yml` — add a step `Check bytecode equivalence` (`pnpm check:bytecode`) right after "Build all packages", and a step `Test scripts` (`pnpm test:scripts`) + +**Interfaces:** + +- Produces, from `scripts/lib/bytecode.mjs`: + - `stripMetadata(bytecode: string): string`. Lowercases the input, strips a leading `0x`, then removes every match of `/a264697066735822[0-9a-f]{68}64736f6c6343[0-9a-f]{6}0033/g`. That pattern covers the 0.7.6 suffix (`…430007060033`) and the 0.8.35 suffix (`…430008230033`). Bytes before or after a blob are kept. + - `bytecodeHash(bytecode: string): string`. Returns the sha256 hex of `stripMetadata(bytecode)`. +- Produces, from `scripts/contracts-migration/check-bytecode.mjs`: + - `computeHashes(manifest: Record, repoRoot: string): { hashes: Record, missing: string[], empty: string[] }`. + - Reads each artifact JSON's `bytecode` and `deployedBytecode`. + - A missing file goes to `missing`. + - A missing, `''` or `'0x'` `bytecode` or `deployedBytecode` goes to `empty`. + - `compare(reference, hashes): string[]`. Returns one line per mismatch (`EpochManager: deployedBytecode differs`) and one per entry present on only one side (`X: not in reference` / `X: not in manifest`). + - CLI, optional flags `--root --manifest --reference `; defaults are the repo root and the two files next to the script. + - Without `--write`: if `missing` or `empty` is non-empty, print each entry to **stderr** and exit 1. Otherwise print the mismatches to stderr and exit 1 if there are any, else print `OK: contracts match` to stdout and exit 0. + - With `--write`: exit 1 without writing if `missing` or `empty` is non-empty. Otherwise write `reference.json` with keys sorted, 2-space indent and a trailing newline. + - Run the CLI only when the file is executed directly: `import.meta.url === pathToFileURL(process.argv[1]).href`. +- Produces `manifest.json`: contract name → artifact path relative to the repo root. + +Initial `manifest.json` (26 entries; every path was confirmed to exist after `pnpm build`): + +| Name | Artifact path | +| --------------------- | ------------------------------------------------------------------------------------------------------------ | +| Controller | `packages/contracts/artifacts/contracts/governance/Controller.sol/Controller.json` | +| GraphProxy | `packages/contracts/artifacts/contracts/upgrades/GraphProxy.sol/GraphProxy.json` | +| GraphProxyAdmin | `packages/contracts/artifacts/contracts/upgrades/GraphProxyAdmin.sol/GraphProxyAdmin.json` | +| GraphToken | `packages/contracts/artifacts/contracts/token/GraphToken.sol/GraphToken.json` | +| L2GraphToken | `packages/contracts/artifacts/contracts/l2/token/L2GraphToken.sol/L2GraphToken.json` | +| L1GraphTokenGateway | `packages/contracts/artifacts/contracts/gateway/L1GraphTokenGateway.sol/L1GraphTokenGateway.json` | +| L2GraphTokenGateway | `packages/contracts/artifacts/contracts/l2/gateway/L2GraphTokenGateway.sol/L2GraphTokenGateway.json` | +| BridgeEscrow | `packages/contracts/artifacts/contracts/gateway/BridgeEscrow.sol/BridgeEscrow.json` | +| L2GNS | `packages/contracts/artifacts/contracts/l2/discovery/L2GNS.sol/L2GNS.json` | +| L2Curation | `packages/contracts/artifacts/contracts/l2/curation/L2Curation.sol/L2Curation.json` | +| GraphCurationToken | `packages/contracts/artifacts/contracts/curation/GraphCurationToken.sol/GraphCurationToken.json` | +| SubgraphNFT | `packages/contracts/artifacts/contracts/discovery/SubgraphNFT.sol/SubgraphNFT.json` | +| SubgraphNFTDescriptor | `packages/contracts/artifacts/contracts/discovery/SubgraphNFTDescriptor.sol/SubgraphNFTDescriptor.json` | +| EpochManager | `packages/contracts/artifacts/contracts/epochs/EpochManager.sol/EpochManager.json` | +| RewardsManager | `packages/contracts/artifacts/contracts/rewards/RewardsManager.sol/RewardsManager.json` | +| L1GNS | `packages/contracts/artifacts/contracts/discovery/L1GNS.sol/L1GNS.json` | +| Curation | `packages/contracts/artifacts/contracts/curation/Curation.sol/Curation.json` | +| L1Staking | `packages/contracts/artifacts/contracts/staking/L1Staking.sol/L1Staking.json` | +| StakingExtension | `packages/contracts/artifacts/contracts/staking/StakingExtension.sol/StakingExtension.json` | +| LibExponential | `packages/contracts/artifacts/contracts/staking/libs/Exponential.sol/LibExponential.json` | +| HorizonStaking | `packages/horizon/build/contracts/contracts/staking/HorizonStaking.sol/HorizonStaking.json` | +| GraphPayments | `packages/horizon/build/contracts/contracts/payments/GraphPayments.sol/GraphPayments.json` | +| PaymentsEscrow | `packages/horizon/build/contracts/contracts/payments/PaymentsEscrow.sol/PaymentsEscrow.json` | +| SubgraphService | `packages/subgraph-service/build/contracts/contracts/SubgraphService.sol/SubgraphService.json` | +| DisputeManager | `packages/subgraph-service/build/contracts/contracts/DisputeManager.sol/DisputeManager.json` | +| AllocationHandler | `packages/subgraph-service/build/contracts/contracts/libraries/AllocationHandler.sol/AllocationHandler.json` | + +The horizon and subgraph-service paths are Hardhat outputs (`build/contracts`, mirroring the source tree), not Foundry's flat `build/`. + +Library link placeholders (L1Staking → LibExponential; SubgraphService → AllocationHandler and others) encode the library's fully qualified source path. They stay stable as long as those libraries don't move; a later PR that moves a linked library must expect to update the reference and justify it in its description. + +- [ ] **Step 1: Write the failing tests in `scripts/contracts-migration/check-bytecode.test.mjs`** + +Use `node:test` and `node:assert/strict`. Write fixtures to a temp dir (`fs.mkdtempSync`). Constants: + +- `META_A = 'a264697066735822' + '1220' + 'ab'.repeat(32) + '64736f6c63430007060033'` +- `META_B`: the same with `'cd'.repeat(32)` + +Tests: + +- `stripMetadata removes the CBOR blob`: `stripMetadata('0x6080' + META_A) === '6080'`. +- `stripMetadata keeps bytes after the blob`: `stripMetadata('0x6080' + META_A + 'ff'.repeat(32)) === '6080' + 'ff'.repeat(32)`. +- `stripMetadata leaves code without metadata unchanged`: `stripMetadata('0x6080') === '6080'`. +- `ignores metadata-only differences`: + - artifact A `{bytecode:'0x6080'+META_A, deployedBytecode:'0x60aa'+META_A}`, artifact B the same with `META_B`; + - `a = computeHashes({X: 'a.json'}, dir)` and `b = computeHashes({X: 'b.json'}, dir)`; + - `compare(a.hashes, b.hashes)` deep-equals `[]`. +- `detects code differences`: as above, but B's `deployedBytecode` is `'0x60ab'+META_B`. `compare` deep-equals `['X: deployedBytecode differs']`. +- `detects differences after the metadata`: B's `bytecode` is `'0x6080'+META_B+'01'` versus A's `'0x6080'+META_A+'00'`. `compare` deep-equals `['X: bytecode differs']`. +- `rejects empty bytecode`: an artifact with `bytecode: '0x'`. `computeHashes(...).empty` deep-equals `['X']`. +- `fails when an artifact is missing`: + - `computeHashes({X: 'nope.json'}, dir).missing` deep-equals `['nope.json']`; + - spawning the CLI (`child_process.spawnSync(process.execPath, [script, '--root', dir, '--manifest', m, '--reference', r])`) exits with status 1, and stderr contains `nope.json`. +- `write refuses to produce a partial reference`: write a reference file with content `'{"keep":true}\n'`, then spawn the CLI with `--write` and a manifest containing a missing entry. It exits with status 1, and the reference file still reads `'{"keep":true}\n'`. + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `node --test 'scripts/contracts-migration/*.test.mjs'` +Expected: FAIL with `ERR_MODULE_NOT_FOUND` for `check-bytecode.mjs` / `../lib/bytecode.mjs`. + +- [ ] **Step 3: Implement `scripts/lib/bytecode.mjs` and `scripts/contracts-migration/check-bytecode.mjs` per the Interfaces block, and switch the compare script to the shared `stripMetadata`** + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `node --test 'scripts/**/*.test.mjs'` +Expected: all PASS, including the existing `scripts/verify-solhint-disables.test.mjs`. + +- [ ] **Step 5: Check that the compare script still works** + +The script refuses to compare identical paths, so give it a symlink: + +```bash +ln -sfn "$PWD" /tmp/repo-link && node scripts/compare-repo-contract-bytecode-excluding-metadata.mjs . /tmp/repo-link +``` + +Expected: `FUNCTIONAL DIFFERENCES (0 contracts)`. + +- [ ] **Step 6: Build and generate the reference** + +Run: `pnpm build && pnpm check:bytecode --write && pnpm check:bytecode` +Expected: `reference.json` has 26 entries, each with two 64-hex-character hashes, and the check prints `OK: 26 contracts match`. + +- [ ] **Step 7: Prove determinism** + +Run: `pnpm clean && pnpm build && pnpm check:bytecode` +Expected: `OK: 26 contracts match`. + +- [ ] **Step 8: Wire into CI** + +Add the two steps to `.github/workflows/build-test.yml` as listed in Files. + +- [ ] **Step 9: Commit and open PR** + +```bash +git add scripts/lib/bytecode.mjs scripts/compare-repo-contract-bytecode-excluding-metadata.mjs scripts/contracts-migration package.json .github/workflows/build-test.yml +git commit -m "ci: add bytecode equivalence check for the contracts package migration" +``` + +PR description: + +- Later move PRs update `manifest.json` paths only. +- `reference.json` hashes never change. Entries are removed only when their contract is deleted. +- The shared `stripMetadata` is now exact (it matches the full blob, not "everything to the end"). The repo comparison script is therefore slightly stricter: it no longer ignores the bytes some contracts have after the metadata in their creation bytecode. +- The onchain comparison for `legacy` and graph-token's mainnet contracts comes in Phase D (D2). + +--- + +### Task 4 (PR 0d): Coverage baseline from `contracts-test` + +**Files:** + +- Create: `scripts/contracts-migration/summarize-coverage.mjs` +- Create: `scripts/contracts-migration/summarize-coverage.test.mjs` +- Create: `scripts/contracts-migration/coverage-baseline.json` (generated) + +**Interfaces:** + +- Produces `parseLcov(text: string): Record`: + - Each record starts at `SF:` and ends at `end_of_record`. + - The key is the `SF:` path from its **last** path segment that equals `contracts`. For example, `/home/runner/work/contracts/contracts/packages/contracts-test/contracts/rewards/RewardsManager.sol` becomes `contracts/rewards/RewardsManager.sol`. Paths that already start with `contracts/` are unchanged. + - The values come from the record's `LF:` and `LH:` lines, with `pct = linesFound === 0 ? null : Math.round(linesHit / linesFound * 10000) / 100`. +- Produces a CLI: `node scripts/contracts-migration/summarize-coverage.mjs ` writes `parseLcov` output with keys sorted, 2-space indent and a trailing newline. Later phases run the same parser on `forge coverage --report lcov` output. + +- [ ] **Step 1: Write the failing tests in `summarize-coverage.test.mjs`** + +- `parses one record with an absolute CI path`: `SF:/home/runner/work/contracts/contracts/packages/contracts-test/contracts/rewards/RewardsManager.sol`, `LF:200`, `LH:150`, `end_of_record` gives `{ 'contracts/rewards/RewardsManager.sol': { linesFound: 200, linesHit: 150, pct: 75 } }`. +- `keeps relative contracts paths`: `SF:contracts/upgrades/GraphProxy.sol` gives the key `contracts/upgrades/GraphProxy.sol`. +- `keeps records separate`: two records (`LF:10/LH:10` and `LF:4/LH:1`) give two keys with `pct` 100 and 25. +- `zero lines yields null pct`: `LF:0`, `LH:0` gives `pct: null`. + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `node --test 'scripts/contracts-migration/summarize-coverage.test.mjs'` +Expected: FAIL with `ERR_MODULE_NOT_FOUND`. + +- [ ] **Step 3: Implement `parseLcov` and the CLI in `summarize-coverage.mjs`** + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `node --test 'scripts/**/*.test.mjs'` +Expected: all PASS. + +- [ ] **Step 5: Run contracts-test coverage** + +`scripts/coverage` does not create the address book that the sdk fixture needs; only `scripts/test` does. So run the regular tests first: + +```bash +pnpm --filter @graphprotocol/contracts-tests test:self +pnpm --filter @graphprotocol/contracts-tests test:coverage:self +``` + +Expected: both complete. solidity-coverage writes to `packages/coverage/lcov.info`, because `.solcover.js` sets `istanbulFolder: '../coverage'`, even though its log says `./coverage/`. If it runs out of memory, retry with `NODE_OPTIONS=--max-old-space-size=8192`. Don't run `pnpm install` between the two commands; it can remove the address book again. + +- [ ] **Step 6: Generate the baseline** + +Run: `node scripts/contracts-migration/summarize-coverage.mjs packages/coverage/lcov.info scripts/contracts-migration/coverage-baseline.json` +Expected: the JSON has entries with `linesFound > 0` for at least these files: + +- `contracts/rewards/RewardsManager.sol` +- `contracts/l2/discovery/L2GNS.sol` +- `contracts/discovery/GNS.sol` +- `contracts/l2/curation/L2Curation.sol` +- `contracts/governance/Controller.sol` +- `contracts/upgrades/GraphProxyAdmin.sol` +- `contracts/epochs/EpochManager.sol` +- `contracts/l2/gateway/L2GraphTokenGateway.sol` +- `contracts/gateway/L1GraphTokenGateway.sol` + +No key contains `packages/`. + +- [ ] **Step 7: Commit and open PR** + +```bash +git add scripts/contracts-migration/summarize-coverage.mjs scripts/contracts-migration/summarize-coverage.test.mjs scripts/contracts-migration/coverage-baseline.json +git commit -m "test: record contracts-test line coverage baseline for the Foundry ports" +``` + +PR description: + +- List the baseline line % for every file that will move to graph-core, graph-token and subgraph-discovery, and for EpochManager and RewardsManager. These are the parity targets from spec §4. +- Note that `.solcover.js` skips `contracts/arbitrum/` and `contracts/tests/`. L1ArbitrumMessenger, L2ArbitrumMessenger and AddressAliasHelper therefore have no baseline, and their parity is checked by the test mapping table only. +- Delete `packages/coverage/` locally after generating; it's not committed. From 6f855a69a6cebce265b0ce784472a8055bb278fe Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Fri, 23 Oct 2026 15:49:14 -0300 Subject: [PATCH 03/11] docs: amend contracts removal design for phase T decisions --- docs/contracts-removal/design.md | 38 ++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/docs/contracts-removal/design.md b/docs/contracts-removal/design.md index 7065830be..4937d8001 100644 --- a/docs/contracts-removal/design.md +++ b/docs/contracts-removal/design.md @@ -147,20 +147,20 @@ The 0.7.6 settings must reproduce the current build exactly: solc 0.7.6, optimiz - **L2Curation:** `setSubgraphService` before `collect`. - **L2GraphTokenGateway:** `vm.mockCall` / `vm.etch` at ArbSys `0x64`. -- **Unit tests use mocks, not the full network.** A test deploys only the contract under test plus mocks for its dependencies. Issuance tests in particular mock EpochManager, Curation, Staking, GRT and the subgraph service, and never load artifacts from horizon or subgraph-discovery (see the acyclicity rules in §2). Test cases that really exercise a cross-package flow, such as the allocation path through staking, curation and rewards, move to `packages/testing`, the existing cross-package Foundry integration suite. +- **Unit tests use mocks, not the full network.** A test deploys only the contract under test plus mocks for its dependencies. Issuance tests in particular mock EpochManager, Curation, Staking, GRT and the subgraph service, and never load artifacts from horizon or subgraph-discovery (see the acyclicity rules in §2). The old rewards tests that went through legacy Staking (restaking, rewards destination, delegation cut) keep their RewardsManager assertions as mock-based unit tests. Their Staking assertions are dropped: that is legacy Staking behaviour, and the Horizon equivalent is already tested in subgraph-service. `packages/testing` stays the home for new cross-package flows. - **Coverage.** Phase 0d records per-contract **line** coverage from `contracts-test` (solidity-coverage) before anything changes. Branch numbers aren't comparable between solidity-coverage and `forge coverage`, so they're informative only. - In graph-core, graph-token and subgraph-discovery, `forge coverage` line coverage after the move must be at least the baseline. - EpochManager and RewardsManager live in Foundry-skipped `legacy-0.7/` folders, so Foundry can't measure coverage for them. For those two, completeness is enforced by the old-to-new test mapping table. #### Test assignment -| Destination | Source tests (`contracts-test/tests/unit`) | Approx. lines | -| ---------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------- | -| issuance (+ `packages/testing` for allocation flows) | `rewards/*` except `subgraphAvailability.test.ts` | 6.8k | -| subgraph-discovery | `l2/l2GNS.test.ts`, `l2/l2Curation.test.ts`, `lib/gnsUtils.ts` | 1.8k | -| graph-token | `gateway/*`, `l2/l2GraphToken.test.ts`, `l2/l2GraphTokenGateway.test.ts`, `l2/l2ArbitrumMessengerMock.ts`, `graphToken.test.ts`, `lib/graphTokenTests.ts` | 1.6k | -| graph-core | `governance/*`, `upgrade/admin.test.ts` | 0.5k | -| horizon | `epochs.test.ts` | 0.2k | +| Destination | Source tests (`contracts-test/tests/unit`) | Approx. lines | +| ------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------- | +| issuance | `rewards/*` except `subgraphAvailability.test.ts` | 6.8k | +| subgraph-discovery | `l2/l2GNS.test.ts`, `l2/l2Curation.test.ts`, `lib/gnsUtils.ts` | 1.8k | +| graph-token | `gateway/*`, `l2/l2GraphToken.test.ts`, `l2/l2GraphTokenGateway.test.ts`, `l2/l2ArbitrumMessengerMock.ts`, `graphToken.test.ts`, `lib/graphTokenTests.ts` | 1.6k | +| graph-core | `governance/*`, `upgrade/admin.test.ts` | 0.5k | +| horizon | `epochs.test.ts` | 0.2k | Notes: @@ -177,13 +177,15 @@ Dropped, because their contracts are deleted or legacy: #### Mocks needed by ported tests -| Mock | Destination | -| ------------------------------------------------------------------------------------ | --------------------------------------------------------------------------------------- | -| GovernedMock | graph-core | -| BridgeMock, InboxMock, OutboxMock, CallhookReceiverMock | graph-token | -| MockERC165, MockIssuanceAllocator, MockRewardsEligibilityOracle, MockSubgraphService | issuance (reuse its existing 0.8 mocks where they exist; otherwise rewrite them in 0.8) | +| Mock | Destination | +| ------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------ | +| GovernedMock, ManagedMock (new, 0.7.6) | graph-core | +| BridgeMock, InboxMock, OutboxMock, CallhookReceiverMock, TokenUtilsMock (new, 0.7.6) | graph-token | +| MockERC165, MockIssuanceAllocator, MockRewardsEligibilityOracle, MockSubgraphService | issuance (reuse its existing 0.8 mocks where they exist; otherwise rewrite them in 0.8; MockSubgraphService becomes a rewards-issuer mock) | -Every test-port PR includes a table mapping each old test case to its new Foundry test (or to `packages/testing`), or to "dropped" with a reason. +ManagedMock and TokenUtilsMock are added to `packages/contracts/contracts/tests/` in Phase T, because 0.8 tests can't compile a Managed contract or call the TokenUtils library. They move with their packages in Phase 1. ArbSysMock is not used (ArbSys is faked with `vm.mockCall`), so it stays on the delete list. + +Every test-port PR includes a table mapping each old test case to its new Foundry test, or to "dropped" with a reason. ### 5. Tooling and infrastructure @@ -294,7 +296,7 @@ Tests are ported first, against the contracts where they are today. Then dead co Each PR: -- creates the destination package skeleton if needed: `package.json`, `foundry.toml`, a 0.7.6 Hardhat config (graph-core, graph-token and subgraph-discovery don't use toolshed), `test/`; +- creates the destination package skeleton if needed: `package.json`, `foundry.toml` and `test/`. The 0.7.6 Hardhat config (graph-core, graph-token and subgraph-discovery don't use toolshed) comes in Phase 1 with the contracts, since there is nothing to compile before then; - adds `@graphprotocol/contracts` as a temporary devDependency; - writes the Foundry tests against its artifacts, following §4; - deletes the matching TypeScript tests from `contracts-test`. @@ -305,7 +307,9 @@ New packages aren't published yet: they stay `"private": true` until their contr - **T2.** graph-token. - **T3.** subgraph-discovery. Also deletes `gns.test.ts` and `lib/gnsUtils.ts`. - **T4.** horizon (epochs). -- **T5.** issuance (rewards, with mocks only) plus the allocation-flow cases in `packages/testing`. This PR may be split by test file group. +- **T5.** issuance (rewards, with mocks only). This PR may be split by test file group. + +The detailed task split is in `phase-t-plan.md`. ### Phase D: removal of dead code @@ -390,5 +394,5 @@ Per-step notes: - **Two OpenZeppelin versions in horizon and issuance.** Limited to `legacy-0.7/`, and enforced by the CI grep rule. In issuance, Hardhat 3 resolves imports per package, so the import rule in §3 also matters there. - **Hardhat 3 artifacts consumed by Hardhat 2 tooling.** horizon's Ignition and verification tooling read the RewardsManager artifact from issuance. Step 5 validates this, with a Hardhat 2-shaped copy as the fallback. - **Mainnet source drift.** Current source may not reproduce every mainnet implementation exactly. D2's onchain comparison catches this, and `legacy` then uses the source from the deployed tag. -- **Test port fidelity.** A rewrite can silently drop behaviours. The mapping table, the coverage baseline and keeping `packages/testing` integration cases guard against this. +- **Test port fidelity.** A rewrite can silently drop behaviours. The mapping table and the coverage baseline guard against this. - **Library link references.** Only L1Staking (legacy) links a library (`Exponential`). The 0c check covers it. None of the live moved contracts link libraries. From 205a4d1464abab650170348eff219834abccec65 Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Fri, 23 Oct 2026 15:49:16 -0300 Subject: [PATCH 04/11] docs: add phase T implementation plan for contracts removal --- docs/contracts-removal/phase-t-plan.md | 962 +++++++++++++++++++++++++ 1 file changed, 962 insertions(+) create mode 100644 docs/contracts-removal/phase-t-plan.md diff --git a/docs/contracts-removal/phase-t-plan.md b/docs/contracts-removal/phase-t-plan.md new file mode 100644 index 000000000..9de44d09c --- /dev/null +++ b/docs/contracts-removal/phase-t-plan.md @@ -0,0 +1,962 @@ +# Contracts Removal — Phase T (Foundry Test Ports) Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Rewrite every live `contracts-test` unit test as a Foundry test in the package that will own the contract, and delete the TypeScript originals. The tests run against the `@graphprotocol/contracts` artifacts, so no contract moves yet. + +**Architecture:** + +- Each destination package gets a copy of `ProtocolFixture`, a small test base contract. It deploys 0.7.6 contracts from Hardhat JSON artifacts, sets up Controller and GraphProxyAdmin, and puts contracts behind a GraphProxy. +- Tests are Solidity 0.8 and reach the contracts through 0.8-compilable interfaces. +- Every dependency that isn't under test is a mock. +- Artifact paths are always built from two constants, `CORE_ARTIFACTS` (Controller, GraphProxy, GraphProxyAdmin) and `PKG_ARTIFACTS` (the package's own contracts and mocks). In Phase T both point at `@graphprotocol/contracts`. Phase 1 repoints them, and in graph-core, graph-token and subgraph-discovery switches `_deployArtifact` to `deployCode`. The test bodies don't change. + +**Tech Stack:** Foundry 1.8.3 (CI's version: `foundryup -i 1.8.3`), forge-std 1.14, solc 0.8.35, pnpm workspaces, Hardhat 2 artifacts from `packages/contracts`, Node 24 (T0 script). + +**Spec:** `docs/contracts-removal/design.md`: + +- §4 "Tests" (test rules, shared fixture, test assignment, mocks); +- "PR sequence → Phase T"; +- "Verification → Test-port PRs". + +The executors read the spec and this plan together. + +## Global Constraints + +### Branching + +- Base branch: `chore/contracts-removal-phase-0` (draft PR #1373). Nothing merges to `main` until the audit. +- Every task branches off the current tip of that branch, and its PR targets it: `gh pr create --draft --base chore/contracts-removal-phase-0`. +- The spec and plans are on branch `docs/contracts-package-removal-spec` (PR #1372), not on the base branch. Read them from the main checkout at `~/dev/the-graph/contracts`. +- One PR per task. Branch names are `test/contracts-removal-`, where `` is one of: + - `t0-coverage-functions` + - `t1-graph-core` + - `t2a-graph-token`, `t2b-gateways` + - `t3a-l2-curation`, `t3b-l2-gns` + - `t4-epochs` + - `t5a-rewards-config`, `t5b-rewards-distribution`, `t5c-rewards-issuers`, `t5d-rewards-eligibility`, `t5e-rewards-accounting`, `t5f-rewards-legacy-suite` +- Order: + 1. T0, then T1. + 2. T2a, T3a, T4 and T5a in parallel. + 3. T2b after T2a. T3b after T3a. T5b–T5e after T5a; they can run in parallel with each other. + 4. T5f after T5b–T5e. +- When a sibling PR lands first, rebase onto the new base. For a `pnpm-lock.yaml` conflict: + + ```bash + git checkout origin/chore/contracts-removal-phase-0 -- pnpm-lock.yaml && pnpm install && git add pnpm-lock.yaml + ``` + +### Commits and PRs + +- Conventional commits (`test:`, `chore:`). Do NOT add `Co-Authored-By` lines, and do not add any Claude or "Generated with" attribution to commits or PR bodies. +- Run `pnpm install` after any `package.json` change and commit `pnpm-lock.yaml`. + +### Scope + +- No production contract changes. The only changes to `packages/contracts` in this phase are two test mocks under `contracts/tests/`: `ManagedMock` (T1) and `TokenUtilsMock` (T2a). +- `pnpm check:bytecode` must still print `OK: 26 contracts match`. +- New packages (`graph-core`, `graph-token`, `subgraph-discovery`) are `"private": true` and `"version": "0.0.0"`. Phase 1 makes them publishable. +- They have no Hardhat config yet (nothing to compile). Their Foundry `src` is `test`. This deviates from the spec's Phase T skeleton list: the Hardhat config arrives in Phase 1 with the contracts. +- New packages have no `test:coverage:self`. Foundry can't measure coverage of artifact-deployed bytecode. Coverage is measured in Phase 1. + +### Test-file rules (all tasks) + +- **Header.** `// SPDX-License-Identifier: MIT` and `pragma solidity ^0.8.27;`, the same as the horizon and issuance tests. +- **No 0.7-only imports.** Never import 0.7-only source. Deploy contracts under test only through `_deployArtifact`. +- **Imports in graph-token and subgraph-discovery:** + - Test files never import `@openzeppelin/*`, or any `@graphprotocol/interfaces` file that imports `@openzeppelin/*` (`IGraphToken`, `ISubgraphNFT`, `IGraphCurationToken`). + - They use the local interfaces under `test/unit/interfaces/`. + - Allowed `@graphprotocol/interfaces` files: + - `contracts/contracts/governance/{IController,IGoverned,IManaged}.sol` + - `contracts/contracts/upgrades/{IGraphProxy,IGraphProxyAdmin}.sol` + - `contracts/contracts/arbitrum/{ITokenGateway,IBridge,IInbox,IOutbox}.sol` + - `contracts/contracts/gateway/ICallhookReceiver.sol` + - These packages don't depend on OpenZeppelin, so a violation fails to compile. +- **Naming:** + - Test functions are `test__` and `test__RevertWhen_`, where `` is the old `describe` name in PascalCase. + - The test contract is named after its file (`Controller.t.sol` → `ControllerTest`). + - Shared setup lives in `shared.t.sol` or `shared/`. +- **Translating a TS test:** + - Each `it` becomes one test function with the same assertions. + - `revertedWith("X")` becomes `vm.expectRevert(bytes("X"))`. A bare `.reverted` becomes `vm.expectRevert()`. + - `vm.expectRevert` must come right before an external call. To expect a revert from an internal helper, call it through an external wrapper (`this.wrapper(...)`). + - `emit(...).withArgs(...)` becomes `vm.expectEmit(address(target))` plus a locally declared event with the same signature. `not.emit` becomes `vm.recordLogs` plus an assertion that the event's topic is absent. + - `beforeEach` becomes `setUp` or a modifier. Snapshot and restore are dropped, because Foundry isolates each test. +- **Blocks and time:** + - Hardhat mines a block per transaction. Foundry runs a whole test in one block until `vm.roll`. + - Every `helpers.mine`, `mineUpTo` or `mineEpoch` becomes an explicit `vm.roll`. + - Hand-counted block totals in the old tests are replaced by values computed from the explicit rolls. + - Every `setUp` calls `vm.warp(1_700_000_000)`. Foundry's default timestamp is 1, which defeats deadline checks. +- **Accounts.** Use `makeAddr` / `makeAddrAndKey`. Random values in the old tests become fixed values, or fuzz parameters where the old test was explicitly randomized. +- **Tolerances.** Keep each old tolerance (`assertApproxEqAbs` / `assertApproxEqRel`). Where Foundry makes a value exact, assert it exactly. +- **No new behaviour tests,** except the ones this plan names and the coverage additions. +- **Formatting.** All Solidity in this plan is shown unformatted. Files are formatted by the repo's prettier (4 spaces, double quotes), and "copy verbatim" means copy the formatted file. + +### Mapping tables + +- Each port PR adds `scripts/contracts-migration/test-mapping/.md` (for example `t2a.md`). Phase 2a deletes the folder with the other migration helpers. +- The table has one row per deleted `it`, with columns `Old file | Old test (describe › it) | New test (file::function) | Note`. + - `Old file` is the repo-relative path. `New test` is `test/unit/.../.t.sol::`, relative to the package. + - A dropped test says `dropped: `. A duplicate points at the test that already covers it. + - T3b may map all of `gns.test.ts` in one `dropped` row, with the note `88 cases, L1GNS`. +- The file ends with a `## Coverage` section (see the checklist below). +- **Checks** (run them in every port PR): + + ```bash + BASE=origin/chore/contracts-removal-phase-0 + MAP=scripts/contracts-migration/test-mapping/.md + # 1. cases in the deleted files (repo-relative paths) + for f in ; do git show "$BASE:$f" | grep -cE '\bit(\.only|\.skip)?\('; done + # 2. rows in the table + grep -c '^| packages/contracts-test' "$MAP" + # 3. every referenced test function exists + grep -oE '[A-Za-z0-9_/.-]+\.t\.sol::[A-Za-z0-9_]+' "$MAP" | while IFS=: read -r file _ fn; do grep -q "function $fn(" "packages//$file" || echo "MISSING $file::$fn"; done + ``` + + Expected: check 1 sums to the PR's case count (in each task), check 2 matches it (T3b: 35 + 1), and check 3 prints nothing. + +### Coverage checklist + +- After T0, `coverage-baseline.json` lists, per contract, the functions the old suite executed (`functionsHit`). +- **Rule.** Every listed external or public function of the contracts a PR owns is called directly by at least one new test. Internal functions and modifiers in the list count as covered when a test reaches them through a public path. +- The list is a floor. It undercounts: some old tests deployed uninstrumented sdk artifacts. The mapping table is the real completeness guard. +- Print the list, and the external/public names no test mentions yet: + + ```bash + node -e 'const b=require("./scripts/contracts-migration/coverage-baseline.json");for(const k of process.argv.slice(1))console.log(k, b[k].functionsHit.join(" "))' + for fn in ; do grep -rqE "\.$fn\(" packages//test || echo "MISSING $fn"; done + ``` + +- The `## Coverage` section of the mapping file lists every `MISSING` name with a reason (for example: only reached through a dropped legacy contract). An empty section means none. + +## Shared fixture (`test/unit/shared/ProtocolFixture.t.sol` in each package) + +Exact API. T1 writes it. T2a, T3a, T4 and T5a copy it verbatim. + +```solidity +abstract contract ProtocolFixture is Test { + string internal constant CORE_ARTIFACTS = 'node_modules/@graphprotocol/contracts/artifacts/contracts/'; + string internal constant PKG_ARTIFACTS = 'node_modules/@graphprotocol/contracts/artifacts/contracts/'; + + address internal governor; // makeAddr("governor"), set in _deployProtocolBase + IController internal controller; // @graphprotocol/interfaces/contracts/contracts/governance/IController.sol + IGraphProxyAdmin internal proxyAdmin; // @graphprotocol/interfaces/contracts/contracts/upgrades/IGraphProxyAdmin.sol + + /// path is a full artifact path, e.g. string.concat(CORE_ARTIFACTS, "governance/Controller.sol/Controller.json"). + /// Deploys `.bytecode ++ args` with CREATE (honours an active vm.prank). + /// Reverts "artifact needs linking" if `.linkReferences` has any key (vm.parseJsonKeys), + /// and "artifact deploy failed" if CREATE returns 0. + function _deployArtifact(string memory path, bytes memory args) internal returns (address); + + /// As governor: deploys Controller and GraphProxyAdmin from CORE_ARTIFACTS, then controller.setPaused(false). + /// Must be called with no active prank. + function _deployProtocolBase() internal; + + /// As governor: deploys GraphProxy(address(0), proxyAdmin) from CORE_ARTIFACTS, + /// proxyAdmin.upgrade(proxy, impl), proxyAdmin.acceptProxyAndCall(impl, proxy, initData). + /// Must be called with no active prank. + function _deployBehindProxy(address impl, bytes memory initData) internal returns (address proxy); + + /// As governor: controller.setContractProxy(keccak256(bytes(name)), addr). + function _registerContract(string memory name, address addr) internal; +} +``` + +Each `foundry.toml` gets an explicit read permission for the artifact folder. pnpm symlinks point outside the package, so `"./"` does not cover them: + +```toml +fs_permissions = [{ access = "read", path = "node_modules/@graphprotocol/contracts/artifacts" }] +``` + +(Existing packages add it to their current `fs_permissions` list.) + +### Shared local token interface (`test/unit/interfaces/IGraphTokenLike.sol`, T2a and T3a) + +Both packages write the same file, from this list, so they don't depend on each other: + +- ERC20: `totalSupply`, `balanceOf`, `transfer`, `transferFrom`, `approve`, `allowance`, and the `Transfer` and `Approval` events. +- Burning: `burn(uint256)`, `burnFrom(address,uint256)`. +- Minting: `mint(address,uint256)`, `addMinter`, `removeMinter`, `renounceMinter`, `isMinter`, and the `MinterAdded(address indexed)` and `MinterRemoved(address indexed)` events. +- Permit: `permit(address owner, address spender, uint256 value, uint256 deadline, uint8 v, bytes32 r, bytes32 s)`, `nonces(address)`. +- Governance: `governor()`, `transferOwnership`, `acceptOwnership`. + +The domain separator is private in both tokens. Tests compute it locally (see T2a). + +## Review Focus + +1. **Missing or stale artifact.** A missing artifact must fail loudly. It must never skip, and never fall back to a mock (the `RealRewardsHarness` fallback is the anti-pattern). A missing `fs_permissions` entry also makes reads revert, which must not pass for a missing-file test. + - → T1 `test_Fixture_ArtifactReadable`: `assertTrue(vm.isFile())`. + - → T1 `test_Fixture_RevertWhen_ArtifactMissing`: an external wrapper around `_deployArtifact` on a nonexistent path, expecting a revert. +2. **Constructor or initializer wiring silently wrong.** GraphProxy args are appended in the wrong order, or the implementation is never accepted, so calls hit a proxy with no implementation. + - → T1 `test_Fixture_ProxyWiredToImplementation` asserts `getProxyAdmin(proxy) == proxyAdmin`, `getProxyImplementation(proxy) == impl` and `getProxyPendingImplementation(proxy) == address(0)`. +3. **Controller left paused.** The Controller constructor pauses the protocol. Any `notPaused` path would then revert with "Paused" and mask the behaviour under test. + - → T1 `test_Fixture_ControllerUnpaused`. +4. **Same-block semantics.** A port that forgets `vm.roll` asserts zero accrual where the old test expected per-block accrual, or the reverse. Fails silently if the expected value is copied from the TS test instead of derived from the rolls. + - → T4 `test_EpochManager_SameBlockDoesNotAdvance`. + - → T5b `test_Rewards_ZeroWhenNoBlocksElapsed`: two calls with no roll in between return an equal `accRewardsPerSignal`. +5. **Default timestamp of 1.** A deadline of 1 is not expired in Foundry, so an "expired permit" test passes for the wrong reason. + - → T2a `test_Permit_RevertWhen_Expired` warps to `1_700_000_000` and uses `deadline = block.timestamp - 1`. + +--- + +### Task 0 (PR T0): Record covered functions in the coverage baseline + +**Files:** + +- Modify: `scripts/contracts-migration/summarize-coverage.mjs` +- Modify: `scripts/contracts-migration/summarize-coverage.test.mjs` +- Modify: `scripts/contracts-migration/coverage-baseline.json` (regenerated) + +**Interfaces:** + +- Consumes: nothing. +- Produces: `parseLcov(text)` returns `{ [key]: { linesFound, linesHit, pct, functionsHit: string[] } }`. + - `functionsHit` is sorted and de-duplicated. + - Empty names are left out (solidity-coverage gives fallback and receive an empty name). + - Every later task reads `functionsHit`. + +The current `parseLcov` reads `LF`/`LH` and overwrites repeated `SF:` records for the same key. The new one computes from `DA:` and `FNDA:` records and merges duplicates: + +- a line counts as found if any record has it; +- it counts as hit if any record hit it; +- functions are the union of `FNDA:,` with `n > 0`. + +`LF`/`LH` records are ignored. + +- [ ] **Step 1: Write failing tests** in `summarize-coverage.test.mjs`. Keep the existing tests. + + ```js + test('computes lines from DA records', ...) // DA:1,3 DA:2,0 DA:3,1 → linesFound 3, linesHit 2, pct 66.67 + test('merges duplicate SF records', ...) // same SF twice: {DA:1,0 DA:2,1} and {DA:1,2 DA:3,0} → found 3, hit 2 + test('lists hit functions sorted and unique', ...) // FNDA:2,b FNDA:0,c FNDA:1,a (+ dup record FNDA:1,b) → ['a','b'] + test('drops empty function names', ...) // FNDA:1, (empty name) → [] + test('file without functions has empty functionsHit', ...) + ``` + +- [ ] **Step 2: Run** `pnpm test:scripts`. Expected: the new tests fail. +- [ ] **Step 3: Implement** the new `parseLcov` (same exported name and signature). `main` is unchanged. +- [ ] **Step 4: Run** `pnpm test:scripts`. Expected: all pass. +- [ ] **Step 5: Regenerate the baseline** from the unmodified suite on this branch. solidity-coverage writes `packages/coverage/lcov.info` (`istanbulFolder: '../coverage'`). + + ```bash + pnpm build && pnpm --filter @graphprotocol/contracts-tests test:coverage:self + node scripts/contracts-migration/summarize-coverage.mjs packages/coverage/lcov.info scripts/contracts-migration/coverage-baseline.json + ``` + + Expected: `git diff` only adds `functionsHit` arrays. `linesFound` and `linesHit` stay the same for every key, except keys that had duplicate `SF:` records. + - If anything else changes, stop and report the keys. + - If duplicates changed a value, keep the merged value and list those keys in the PR body. + +- [ ] **Step 6: Commit and open the PR.** + + ```bash + git commit -am "test: record covered functions in the contracts-test coverage baseline" + ``` + +--- + +### Task 1 (PR T1): graph-core package and governance/upgrade tests + +**Cases:** 36 (`governed` 2, `controller` 12, `pausing` 8, `admin` 14). + +**Files:** + +- Create: + - `packages/graph-core/package.json` + - `packages/graph-core/foundry.toml` +- Create, under `packages/graph-core/test/unit/`: + - `shared/ProtocolFixture.t.sol` + - `shared/GraphCoreTest.t.sol` (base: `_deployProtocolBase` plus a `ManagedMock` behind a proxy) + - `shared/Fixture.t.sol` (Review Focus 1–3) + - `interfaces/IManagedMock.sol` + - `governance/Governed.t.sol` + - `governance/Controller.t.sol` + - `governance/Managed.t.sol` + - `governance/Pausing.t.sol` + - `upgrades/GraphProxyAdmin.t.sol` + - `upgrades/GraphProxy.t.sol` +- Create: `packages/contracts/contracts/tests/ManagedMock.sol` +- Create: `scripts/contracts-migration/test-mapping/t1.md` +- Delete, under `packages/contracts-test/tests/unit/`: + - `governance/governed.test.ts` + - `governance/controller.test.ts` + - `governance/pausing.test.ts` + - `upgrade/admin.test.ts` +- Modify: `pnpm-lock.yaml` + +**Interfaces:** + +- Consumes: T0 `functionsHit`. +- Produces: + - the `ProtocolFixture` API above; + - `ManagedMock` at `PKG_ARTIFACTS + "tests/ManagedMock.sol/ManagedMock.json"`; + - the `package.json` and `foundry.toml` templates, which T2a and T3a copy, changing only the name, description and devDependencies. + +`package.json`: + +```json +{ + "name": "@graphprotocol/graph-core", + "version": "0.0.0", + "private": true, + "description": "The Graph governance and proxy contracts", + "license": "GPL-2.0-or-later", + "scripts": { + "build": "pnpm build:dep", + "build:dep": "pnpm --filter '@graphprotocol/graph-core^...' run build:self", + "clean": "rm -rf forge-artifacts/ cache_forge/", + "test": "pnpm build && pnpm test:self", + "test:self": "forge test" + }, + "devDependencies": { + "@graphprotocol/contracts": "workspace:^", + "@graphprotocol/interfaces": "workspace:^", + "forge-std": "catalog:" + } +} +``` + +`foundry.toml`: + +- Same as `packages/testing/foundry.toml`, except: + - `src = 'test'` + - `auto_detect_remappings = false` + - remappings `@graphprotocol/=node_modules/@graphprotocol/` and `forge-std/=node_modules/forge-std/src/` only + - the `fs_permissions` line above + - `ignored_warnings_from = ["node_modules"]` +- Keep `solc_version = '0.8.35'`, `evm_version = 'cancun'` and the `[lint]` block. +- No `[profile.prod]`. + +`ManagedMock.sol` (0.7.6, in `contracts/tests/` next to `GovernedMock.sol`): + +- Header: the SPDX line, `pragma solidity ^0.7.6;` and `// solhint-disable use-natspec`. +- No `TODO` line: `pnpm todo` (part of `lint:sol`) fails on it. + +```solidity +contract ManagedMock is Managed, GraphUpgradeable { + function initialize(address _controller) external onlyImpl { + Managed._initialize(_controller); + } + function checkNotPartialPaused() external view notPartialPaused {} + function checkNotPaused() external view notPaused {} + function checkOnlyGovernor() external view onlyGovernor {} + /// curation, epochManager, rewardsManager, staking, graphToken, graphTokenGateway, gns — in that order + function resolveAll() external view returns (address[7] memory); +} +``` + +It replaces the EpochManager, Staking and Curation contracts the old tests used as "some Managed / some GraphUpgradeable contract" (spec §4 notes). Phase 1 moves it to graph-core with `GovernedMock`. + +- [ ] **Step 1: Write the fixture and its tests.** Write `ProtocolFixture`, `IManagedMock`, `GraphCoreTest` and `shared/Fixture.t.sol` (the four Review Focus 1–3 tests). + - `GraphCoreTest.setUp`: `vm.warp(1_700_000_000)`, `_deployProtocolBase()`, then two `ManagedMock` implementations (`implA`, `implB`). + - `managed = IManagedMock(_deployBehindProxy(implA, abi.encodeCall(IManagedMock.initialize, (address(controller)))))`. +- [ ] **Step 2: Run** `pnpm --filter @graphprotocol/graph-core test`. Expected: the build succeeds and the four fixture tests pass. +- [ ] **Step 3: Port the four TS files.** Old → new: + + | Old | New | + | -------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | + | `governed.test.ts` | `governance/Governed.t.sol`. Deploys `PKG_ARTIFACTS + "tests/GovernedMock.sol/GovernedMock.json"` as governor. | + | `controller.test.ts` (`describe('Managed')`) | `governance/Controller.t.sol`. `setUp` registers the `ManagedMock` proxy under the `EpochManager` id. Without it, `updateController` calls address zero. `setController` is called directly on the proxy. | + | `pausing.test.ts` | `governance/Pausing.t.sol`. "fails when partialPause is set": `checkNotPartialPaused` reverts `Partial-paused`. "fails when pause is set": `checkNotPartialPaused` and `checkNotPaused` both revert `Paused`. | + | `admin.test.ts` | `upgrades/GraphProxyAdmin.t.sol`. "Staking proxy" becomes the `ManagedMock` proxy, and "Curation implementation" becomes `implB`. Reading `implementation()` / `admin()` / `pendingImplementation()` directly from a non-admin reverts. | + +- [ ] **Step 4: Cover the checklist.** Run the checklist commands for: + - `contracts/governance/Controller.sol` + - `contracts/governance/Governed.sol` + - `contracts/governance/Managed.sol` + - `contracts/governance/Pausable.sol` + - `contracts/upgrades/GraphProxy.sol` + - `contracts/upgrades/GraphProxyAdmin.sol` + - `contracts/upgrades/GraphProxyStorage.sol` + - `contracts/upgrades/GraphUpgradeable.sol` + + Add a test for every `MISSING` name. Expected additions: + - `Managed.t.sol`: + - `syncAllContracts` emits `ContractSynced` per changed id, and nothing on a second call; + - `resolveAll` returns the controller's addresses before the sync and the cached addresses after it; + - `checkOnlyGovernor` reverts `Only Controller governor`. + - `GraphProxy.t.sol`: + - `acceptUpgradeAndCall` reverts `Impl call failed` on failing calldata; + - the admin calling the fallback reverts `Cannot fallback to proxy target`; + - `setAdmin(0)` reverts `Admin cant be the zero address`; + - `acceptUpgrade` with `vm.prank(address(proxyAdmin))` and no pending implementation reverts `Impl cannot be zero address`; + - from a non-pending implementation, it reverts `Only pending implementation`. + - `Governed.t.sol`: `transferOwnership(0)` reverts `Governor must be set`. + +- [ ] **Step 5: Run** `pnpm --filter @graphprotocol/graph-core test` and `pnpm --filter @graphprotocol/contracts-tests test:self`. Expected: all pass. +- [ ] **Step 6: Lint and check bytecode.** Run: + - `pnpm check:bytecode` (expect `OK: 26 contracts match`); + - `(cd packages/contracts && npx solhint --max-warnings=0 contracts/tests/ManagedMock.sol)` (expect exit 0); + - `pnpm lint:sol && git status --short` (expect exit 0 and no files changed; commit any formatting fixes first). +- [ ] **Step 7:** Write `t1.md` and run the mapping checks. Expected: 36 cases, 36 rows, no `MISSING`. +- [ ] **Step 8: Commit and open the PR.** + + ```bash + git commit -m "test(graph-core): port governance and proxy tests to Foundry" + ``` + +--- + +### Task 2a (PR T2a): graph-token package, tokens, BridgeEscrow and TokenUtils + +**Cases:** 30 (`lib/graphTokenTests.ts` 15, `l2/l2GraphToken.test.ts` 11, `gateway/bridgeEscrow.test.ts` 4, `graphToken.test.ts` 0). + +**Files:** + +- Create: + - `packages/graph-token/package.json` + - `packages/graph-token/foundry.toml` +- Create, under `packages/graph-token/test/unit/`: + - `shared/ProtocolFixture.t.sol` (copy of T1's) + - `interfaces/IGraphTokenLike.sol` (shared list above) + - `interfaces/IL2GraphTokenLike.sol` + - `interfaces/IBridgeEscrowLike.sol` + - `interfaces/ITokenUtilsMock.sol` + - `token/GraphTokenShared.t.sol` (abstract) + - `token/GraphToken.t.sol` + - `token/L2GraphToken.t.sol` + - `gateway/BridgeEscrow.t.sol` + - `utils/TokenUtils.t.sol` +- Create: `packages/contracts/contracts/tests/TokenUtilsMock.sol` +- Create: `scripts/contracts-migration/test-mapping/t2a.md` +- Delete, under `packages/contracts-test/tests/unit/`: + - `graphToken.test.ts` + - `lib/graphTokenTests.ts` + - `l2/l2GraphToken.test.ts` + - `gateway/bridgeEscrow.test.ts` + +**Interfaces:** + +- Consumes: `ProtocolFixture`. +- Produces: the graph-token package with `ProtocolFixture` and `IGraphTokenLike`, which T2b uses. +- devDependencies: `@graphprotocol/contracts`, `@graphprotocol/interfaces`, `forge-std`. No OpenZeppelin. +- `IL2GraphTokenLike is IGraphTokenLike`, adding: + - `initialize(address)`, `gateway()`, `l1Address()`, `setGateway`, `setL1Address`, `bridgeMint`, `bridgeBurn`; + - the `GatewaySet`, `L1AddressSet`, `BridgeMinted` and `BridgeBurned` events, with the contract's signatures. +- `IBridgeEscrowLike`: `initialize(address)`, `approveAll(address)`, `revokeAll(address)`. +- `TokenUtilsMock.sol` (0.7.6, in `contracts/tests/`, same header rules as `ManagedMock`): + + ```solidity + contract TokenUtilsMock { + function pullTokens(IGraphToken token, address from, uint256 amount) external { + TokenUtils.pullTokens(token, from, amount); + } + function pushTokens(IGraphToken token, address to, uint256 amount) external { + TokenUtils.pushTokens(token, to, amount); + } + function burnTokens(IGraphToken token, uint256 amount) external { + TokenUtils.burnTokens(token, amount); + } + } + ``` + + - In the live code, TokenUtils is only used by GNS and L2Curation, which aren't in graph-token. Without this mock, graph-token can't reach TokenUtils' baseline coverage in Phase 1. + - Phase 1 moves the mock to graph-token with TokenUtils. + +**Artifacts and setup** (`PKG_ARTIFACTS` +): + +| Contract | Path | +| -------------- | ---------------------------------------------- | +| GraphToken | `token/GraphToken.sol/GraphToken.json` | +| L2GraphToken | `l2/token/L2GraphToken.sol/L2GraphToken.json` | +| BridgeEscrow | `gateway/BridgeEscrow.sol/BridgeEscrow.json` | +| TokenUtilsMock | `tests/TokenUtilsMock.sol/TokenUtilsMock.json` | + +- **GraphToken** is not proxied: deploy as governor with `abi.encode(10_000_000_000 ether)`. +- **L2GraphToken** is behind a proxy: `initialize(governor)`. +- **BridgeEscrow** is behind a proxy: `initialize(controller)`. Register `GraphToken` in the controller first, then call `syncAllContracts()`. +- **`GraphTokenSharedTest`** carries every `lib/graphTokenTests.ts` case once: + - abstract hooks `_token() returns (IGraphTokenLike)` and `_salt() returns (bytes32)`; + - L1 salt `0x51f3d585afe6dfeb2af01bba0889a36c1db03beec88c6a4d0c53817069026afa`; + - L2 salt `0xe33842a7acd1d5a1d28f25a931703e5605152dc48d64dc4716efdae1f5659591`; + - `_domainSeparator()` computes `keccak256(abi.encode(keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract,bytes32 salt)"), keccak256("Graph Token"), keccak256("0"), block.chainid, address(_token()), _salt()))`; + - the permit digest is `keccak256(abi.encodePacked("\x19\x01", _domainSeparator(), keccak256(abi.encode(keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)"), owner, spender, value, nonce, deadline))))`; + - permits are signed with `vm.sign` using the key from `makeAddrAndKey("me")`; + - `GraphToken.t.sol` and `L2GraphToken.t.sol` inherit it. The L2 file also holds the "Extended L2 behavior" cases, with the gateway and L1 address as plain `makeAddr` accounts. +- **`TokenUtils.t.sol`**: + - `pullTokens` moves tokens from an approved holder to the mock; + - `pullTokens` and `pushTokens` with amount 0 do nothing; + - `pushTokens` moves tokens from the mock; + - `burnTokens` lowers `totalSupply`. + +Steps: + +- [ ] **Step 1: Scaffold.** Write the package, copy `ProtocolFixture`, and write the interfaces and `GraphTokenSharedTest`, including Review Focus 5. +- [ ] **Step 2: Run** `pnpm --filter @graphprotocol/graph-token test`. Expected: the shared tests pass for both tokens. +- [ ] **Step 3: Port** the L2 extensions and BridgeEscrow. Write `TokenUtilsMock` and `TokenUtils.t.sol`. Run the package tests after each file. Expected: pass. +- [ ] **Step 4: Cover the checklist** for: + - `contracts/token/GraphToken.sol` + - `contracts/l2/token/L2GraphToken.sol` + - `contracts/l2/token/GraphTokenUpgradeable.sol` + - `contracts/gateway/BridgeEscrow.sol` + - `contracts/utils/TokenUtils.sol` +- [ ] **Step 5: Run** the package tests, `pnpm --filter @graphprotocol/contracts-tests test:self`, `pnpm check:bytecode`, `(cd packages/contracts && npx solhint --max-warnings=0 contracts/tests/TokenUtilsMock.sol)`, and `pnpm lint:sol && git status --short`. Expected: all pass, and no files changed. +- [ ] **Step 6:** Write `t2a.md` and run the mapping checks. Expected: 30 cases, 30 rows (the two identically titled permit cases are two rows), no `MISSING`. +- [ ] **Step 7: Commit and open the PR.** + + ```bash + git commit -m "test(graph-token): port token, escrow and token utils tests to Foundry" + ``` + +--- + +### Task 2b (PR T2b): graph-token gateway tests + +**Cases:** 74 (`gateway/l1GraphTokenGateway.test.ts` 46, `l2/l2GraphTokenGateway.test.ts` 28). + +**Files:** + +- Create, under `packages/graph-token/test/unit/`: + - `interfaces/IL1GraphTokenGatewayLike.sol` + - `interfaces/IL2GraphTokenGatewayLike.sol` + - `interfaces/IArbitrumMocksLike.sol` + - `gateway/L1GraphTokenGatewayShared.t.sol` + - `gateway/L1GraphTokenGatewayConfig.t.sol` + - `gateway/L1GraphTokenGatewayTransfers.t.sol` + - `gateway/L2GraphTokenGatewayShared.t.sol` + - `gateway/L2GraphTokenGatewayConfig.t.sol` + - `gateway/L2GraphTokenGatewayTransfers.t.sol` + - `mocks/NoCallhookReceiver.sol` (an empty 0.8 contract: no fallback, so any call reverts) +- Create: `scripts/contracts-migration/test-mapping/t2b.md` +- Delete, under `packages/contracts-test/tests/unit/`: + - `gateway/l1GraphTokenGateway.test.ts` + - `l2/l2GraphTokenGateway.test.ts` + - `l2/l2ArbitrumMessengerMock.ts` (imported by nothing) + +**Interfaces:** + +- Consumes: T2a's package, `ProtocolFixture` and `IGraphTokenLike`. +- Produces: nothing for other tasks. +- `IL1GraphTokenGatewayLike`: + - `initialize(address)`; + - `setArbitrumAddresses(address inbox, address router)`, `setL2TokenAddress`, `setL2CounterpartAddress`, `setEscrowAddress`; + - `addToCallhookAllowlist`, `removeFromCallhookAllowlist`, `callhookAllowlist(address)`; + - `updateL2MintAllowance(uint256 l2IssuancePerBlock, uint256 updateBlockNum)`, `setL2MintAllowanceParametersManual(uint256 accumulatedAllowance, uint256 l2IssuancePerBlock, uint256 lastUpdateBlock)`, `accumulatedL2MintAllowanceAtBlock(uint256)`; + - getters: `accumulatedL2MintAllowanceSnapshot`, `l2MintAllowancePerBlock`, `lastL2MintAllowanceUpdateBlock`, `totalMintedFromL2`, `l2GRT`, `inbox`, `l1Router`, `l2Counterpart`, `escrow`, `counterpartGateway`, `calculateL2TokenAddress`; + - pause: `setPaused`, `paused`, `setPauseGuardian`, `pauseGuardian`; + - transfers: `outboundTransfer` and `finalizeInboundTransfer` with `ITokenGateway`'s signatures; + - every event the contract declares. +- `IL2GraphTokenGatewayLike`: + - `initialize(address)`; + - `setL2Router`, `setL1TokenAddress`, `setL1CounterpartAddress`, their getters, and `calculateL2TokenAddress`; + - the 4-argument `outboundTransfer(address l1Token, address to, uint256 amount, bytes data)` and the `ITokenGateway` overloads; + - the same pause functions as L1; + - every event the contract declares (`DepositFinalized`, `WithdrawalInitiated`, `TxToL1`, `L2RouterSet`, …). +- `IArbitrumMocksLike`: + - BridgeMock: `setInbox(address,bool)`, `setOutbox(address,bool)`; + - InboxMock and OutboxMock: `setBridge(address)`; + - OutboxMock: `executeTransaction(...)`, with the mock's exact parameter list; + - CallhookReceiverMock: the `TransferReceived(address from, uint256 amount, uint256 foo, uint256 bar)` event. +- Copy exact parameter lists and event signatures from the 0.7.6 sources in `packages/contracts/contracts/`. + +**Artifacts and setup** (`PKG_ARTIFACTS` +): + +| Contract | Path | +| --------------------------------- | ------------------------------------------------------------- | +| L1GraphTokenGateway | `gateway/L1GraphTokenGateway.sol/L1GraphTokenGateway.json` | +| L2GraphTokenGateway | `l2/gateway/L2GraphTokenGateway.sol/L2GraphTokenGateway.json` | +| BridgeMock, InboxMock, OutboxMock | `tests/arbitrum/.sol/.json` | +| CallhookReceiverMock | `tests/CallhookReceiverMock.sol/CallhookReceiverMock.json` | + +- **Gateways:** behind a proxy, `initialize(controller)`. Register `GraphToken` and `GraphTokenGateway`, then call `syncAllContracts()`. Gateways start paused. +- **Counterpart contracts** (the other layer's token and gateway, routers): `makeAddr` plus `vm.etch(addr, hex"1234")`. +- **L1 bridge mocks:** + - Wiring: `bridge.setInbox(inbox, true)`, `bridge.setOutbox(outbox, true)`, `inbox.setBridge(bridge)`, `outbox.setBridge(bridge)`. + - Inbound calls go through `outbox.executeTransaction(...)`. +- **ArbSys:** don't use the ArbSysMock artifact. The spec deletes it in D3. + - `vm.etch(address(0x64), hex"00")` gives the address code, which 0.7.6's extcodesize check needs. + - `vm.mockCall(address(0x64), abi.encodeWithSignature("sendTxToL1(address,bytes)"), abi.encode(uint256(1)))`. + - Outbound tests add `vm.expectCall(address(0x64), abi.encodeWithSignature("sendTxToL1(address,bytes)", l1Gateway, data))`. +- **L2 inbound:** `vm.prank(address(uint160(l1Counterpart) + uint160(0x1111000000000000000000000000000000001111)))`, as an unchecked add. +- **"Any contract" for the callhook allowlist:** any deployed address. Replaces RewardsManager. +- **"Contract without onTokenTransfer":** `NoCallhookReceiver`, asserted with a bare `vm.expectRevert()`. +- **Mint allowance test:** derive block numbers from explicit `vm.roll`s. For example: update the allowance at block B, roll to B + 4, then expect an allowance of 8 GRT at 2 GRT per block. + +Steps: + +- [ ] **Step 1: Write the interfaces and the L1 shared base.** Port the L1 `> immediately after deploy` cases into `L1GraphTokenGatewayConfig.t.sol`. Run `pnpm --filter @graphprotocol/graph-token test`. Expected: pass. +- [ ] **Step 2: Port** the L1 `> after configuring and unpausing` cases into `L1GraphTokenGatewayTransfers.t.sol`. Run the package tests. Expected: pass. +- [ ] **Step 3: Port** the L2 gateway the same way, split into config and transfers. Run the package tests. Expected: pass. +- [ ] **Step 4: Cover the checklist** for: + - `contracts/gateway/GraphTokenGateway.sol` + - `contracts/gateway/L1GraphTokenGateway.sol` + - `contracts/l2/gateway/L2GraphTokenGateway.sol` + + The L2 gateway's baseline lacks `getOutboundCalldata`, because the old test skipped it under coverage. Cover it anyway; it's in the TS test. + +- [ ] **Step 5: Run** the package tests, `pnpm --filter @graphprotocol/contracts-tests test:self`, `pnpm check:bytecode` and `pnpm lint:sol && git status --short`. Expected: all pass, and no files changed. +- [ ] **Step 6:** Write `t2b.md` and run the mapping checks. Expected: 74 cases, 74 rows, no `MISSING`. +- [ ] **Step 7: Commit and open the PR.** + + ```bash + git commit -m "test(graph-token): port gateway tests to Foundry" + ``` + +--- + +### Task 3a (PR T3a): subgraph-discovery package and L2Curation tests + +**Cases:** 43 (`l2/l2Curation.test.ts`). + +**Files:** + +- Create: + - `packages/subgraph-discovery/package.json` (devDependencies as in T2a) + - `packages/subgraph-discovery/foundry.toml` +- Create, under `packages/subgraph-discovery/test/unit/`: + - `shared/ProtocolFixture.t.sol` + - `shared/DiscoveryTest.t.sol` (full setup below; used by T3b too) + - `mocks/L2GatewayMock.sol` + - `interfaces/IGraphTokenLike.sol` (shared list above) + - `interfaces/IL2GNSLike.sol` + - `interfaces/IL2CurationLike.sol` + - `interfaces/ISubgraphNFTLike.sol` + - `interfaces/IERC20Like.sol` (`balanceOf`, `totalSupply`; for curation-token balances) + - under `l2-curation/`: + - `Config.t.sol` + - `BondingCurve.t.sol` + - `Curate.t.sol` + - `CurateTaxFree.t.sol` + - `Collect.t.sol` + - `Burn.t.sol` + - `Conservation.t.sol` + - `MultipleMinting.t.sol` +- Create: `scripts/contracts-migration/test-mapping/t3a.md` +- Delete: `packages/contracts-test/tests/unit/l2/l2Curation.test.ts` + +**Interfaces:** + +- Consumes: `ProtocolFixture`. +- Produces, used by T3b: + - `DiscoveryTest`; + - `L2GatewayMock`; + - `IL2GNSLike`, `IL2CurationLike`, `ISubgraphNFTLike`. +- `DiscoveryTest` fields: + - contracts: `grt` (`IGraphTokenLike`), `curation` (`IL2CurationLike`), `gns` (`IL2GNSLike`), `nft` (`ISubgraphNFTLike`), `gatewayMock`, `curationTokenMaster`; + - accounts, all from `makeAddr`: `me`, `other`, `curator`, `attacker`, `l1GNS`, `subgraphService`. +- `IL2GNSLike`: + - the entries of `IGNS` and `IL2GNS` the tests call, both overloads of `publishNewVersion`, and `onTokenTransfer`; + - `initialize(address,address)`, `approveAll()`, `setCounterpartGNSAddress(address)`, `counterpartGNSAddress()`; + - `ownerTaxPercentage()`, `setOwnerTaxPercentage(uint32)`, `nextAccountSeqID(address)`; + - `subgraphs(uint256) returns (uint256 vSignal, uint256 nSignal, bytes32 subgraphDeploymentID, uint32 reserveRatio, bool disabled, uint256 withdrawableGRT)`; + - `subgraphL2TransferData(uint256) returns (uint256 tokens, bool l2Done, uint256 subgraphReceivedOnL2BlockNumber)`; + - `getAliasedL2SubgraphID(uint256)`; + - every event the tests assert. +- `IL2CurationLike`: + - the entries of `ICuration` the tests call; + - `initialize(address,address,uint32,uint256)`; + - `pools(bytes32) returns (uint256 tokens, uint32 reserveRatio, address gcs)`; + - `defaultReserveRatio()`, `setDefaultReserveRatio(uint32)`; + - `minimumCurationDeposit()`, `setMinimumCurationDeposit(uint256)`, `setCurationTaxPercentage(uint32)`; + - `curationTokenMaster()`, `setCurationTokenMaster(address)`; + - `subgraphService()`, `setSubgraphService(address)`, `collect(bytes32,uint256)`; + - `mintTaxFree`, `tokensToSignalNoTax`, `tokensToSignalToTokensNoTax`; + - the `Signalled`, `Burned` and `Collected` events. +- `ISubgraphNFTLike`: `ownerOf`, `setMinter`, `setTokenDescriptor`, `tokenURI`. +- `L2GatewayMock` (0.8): + - `constructor(address token)`; + - `function deliver(address gns, address l1From, uint256 amount, bytes calldata data) external`. It calls `IGraphTokenLike(token).mint(gns, amount)` and then `ICallhookReceiver(gns).onTokenTransfer(l1From, amount, data)`. Mint first, then forward, like `L2GraphTokenGateway.finalizeInboundTransfer`. + +**`DiscoveryTest.setUp`** (spec §4 "L2GNS" and "L2Curation" setup), all artifacts from `PKG_ARTIFACTS`: + +1. `vm.warp(1_700_000_000)`, then `_deployProtocolBase()`. +2. L2GraphToken behind a proxy (`initialize(governor)`). As governor, `addMinter(gatewayMock)` and `addMinter(governor)`. +3. GraphCurationToken master (`curation/GraphCurationToken.sol/GraphCurationToken.json`, deployed uninitialized). +4. L2Curation behind a proxy: `initialize(controller, curationTokenMaster, 0, 1)`. +5. SubgraphNFT (`discovery/SubgraphNFT.sol/SubgraphNFT.json`, `abi.encode(governor)`). Deploy SubgraphNFTDescriptor (`discovery/SubgraphNFTDescriptor.sol/SubgraphNFTDescriptor.json`) and set it with `setTokenDescriptor`. +6. L2GNS behind a proxy: `initialize(controller, nft)`. `nft.setMinter(gns)`. +7. Register `GraphToken`, `Curation`, `GNS` and `GraphTokenGateway` (= `gatewayMock`), then call `syncAllContracts()` on curation and GNS. +8. `gns.approveAll()`, `gns.setCounterpartGNSAddress(l1GNS)`, `curation.setSubgraphService(subgraphService)`. + +- There is no staking, rewards manager or epoch manager. L2Curation skips the rewards hook when RewardsManager resolves to zero (`L2Curation.sol` `_updateRewards`). +- Tests that need curation as the caller use `vm.prank(address(gns))` (it replaces the old impersonation of the GNS proxy). Tests that need curator funds mint as governor. + +Steps: + +- [ ] **Step 1: Scaffold.** Write the package, `DiscoveryTest`, `L2GatewayMock` and the interfaces. Add `test_Setup_Wired`: `nft` minter is `gns`, `curation.subgraphService() == subgraphService`, and `gns.counterpartGNSAddress() == l1GNS`. +- [ ] **Step 2: Run** `pnpm --filter @graphprotocol/subgraph-discovery test`. Expected: pass. +- [ ] **Step 3: Port** `l2Curation.test.ts`, one `describe` block per file, as listed above. + - `Config.t.sol`: the `L2Curation:Config` describe. + - `MultipleMinting.t.sol`: the `multiple minting` describe. + - The leftover `setContractProxy("Staking")` lines in the collect tests are dropped, because `collect` only checks `subgraphService`. + - `Conservation.t.sol` is pinned as `testFuzz_Conservation_DepositsAndRedeemsMatch(uint256[10] memory depositCuts, uint256[10] memory redeemCuts)`: + - Split exactly `1_000_000_000 ether` into 10 deposit chunks, cutting at `bound(cut, 0, remaining)`; the last chunk takes the remainder. + - Skip zero chunks (no call). + - Split the curator's resulting signal into 10 redeem chunks the same way. + - Assert that the pool and the curator end with exactly the initial token balances. +- [ ] **Step 4: Cover the checklist** for: + - `contracts/l2/curation/L2Curation.sol` + - `contracts/curation/GraphCurationToken.sol` +- [ ] **Step 5: Run** the package tests, `pnpm --filter @graphprotocol/contracts-tests test:self`, `pnpm check:bytecode` and `pnpm lint:sol && git status --short`. Expected: all pass, and no files changed. +- [ ] **Step 6:** Write `t3a.md` and run the mapping checks. Expected: 43 cases, 43 rows, no `MISSING`. +- [ ] **Step 7: Commit and open the PR.** + + ```bash + git commit -m "test(subgraph-discovery): port L2Curation tests to Foundry" + ``` + +--- + +### Task 3b (PR T3b): L2GNS tests + +**Cases:** 35 live (`l2/l2GNS.test.ts`), plus 88 dropped (`gns.test.ts`, in one row). + +**Files:** + +- Create, under `packages/subgraph-discovery/test/unit/`: + - `shared/GNSHelpers.t.sol` + - under `l2-gns/`: + - `PublishNewVersion.t.sol` + - `ReceiveSubgraphFromL1.t.sol` (also holds the `onTokenTransfer with invalid codes` case) + - `FinishSubgraphTransfer.t.sol` + - `ReceiveCuratorBalance.t.sol` + - `AliasedSubgraphId.t.sol` +- Create: `scripts/contracts-migration/test-mapping/t3b.md` +- Delete, under `packages/contracts-test/tests/unit/`: + - `l2/l2GNS.test.ts` + - `lib/gnsUtils.ts` + - `gns.test.ts` (L1GNS, already `describe.skip`; spec §4) + +**Interfaces:** + +- Consumes: T3a's `DiscoveryTest`, `L2GatewayMock` and interfaces. +- Produces: nothing for other tasks. +- `abstract contract GNSHelpers is DiscoveryTest`. It ports the `lib/gnsUtils.ts` helpers that `l2GNS.test.ts` uses, assertions included: + - `_publishNewSubgraph(address account, bytes32 deploymentId) returns (uint256 subgraphId)`. The id is `uint256(keccak256(abi.encodePacked(account, gns.nextAccountSeqID(account), block.chainid)))`. + - `_publishNewVersion(address account, uint256 subgraphId, bytes32 newDeploymentId)` + - `_mintSignal(address account, uint256 subgraphId, uint256 tokensIn)` + - `_burnSignal(address account, uint256 subgraphId)` + - `_deprecateSubgraph(address account, uint256 subgraphId)` + - Helpers act as `account` with `vm.startPrank` / `vm.stopPrank`. +- Callhook data: `abi.encode(uint8 code, uint256 l1SubgraphID, address beneficiary)`, with code `0` for a subgraph and `1` for a curator balance. L1 deliveries go through `gatewayMock.deliver(address(gns), l1GNS, amount, data)`. + +Steps: + +- [ ] **Step 1: Write `GNSHelpers`** and `test_Setup_PublishNewSubgraph`. Run `pnpm --filter @graphprotocol/subgraph-discovery test`. Expected: pass. +- [ ] **Step 2: Port** `l2GNS.test.ts`, one `describe` block per file, as listed above. + - **Front-running test:** the slippage case keeps its curation inflation (mint to curation, then `collect` as `subgraphService`). It drops the staking lines, which never affected the assertions. + - **`DepositFinalized`:** the old assertions on `emit(l2GraphTokenGateway, 'DepositFinalized')` map to `covered by t2b L2GraphTokenGatewayTransfers`. The mock gateway doesn't emit it. + - **`getAliasedL2SubgraphID`:** keeps the exact L1 id `68799548758199140224151701590582019137924969401915573086349306511960790045480` and the wrap-around case. +- [ ] **Step 3: Cover the checklist** for: + - `contracts/l2/discovery/L2GNS.sol` + - `contracts/discovery/GNS.sol` + - `contracts/discovery/SubgraphNFT.sol` + - `contracts/base/Multicall.sol` + - `contracts/utils/TokenUtils.sol`, reached through GNS and L2Curation + + GNS functions that only L1GNS reached go in `## Coverage` with a reason. Add `test_GNS_Multicall` if `multicall` is listed. + +- [ ] **Step 4: Run** the package tests, `pnpm --filter @graphprotocol/contracts-tests test:self`, `pnpm check:bytecode` and `pnpm lint:sol && git status --short`. Expected: all pass, and no files changed. +- [ ] **Step 5:** Write `t3b.md` and run the mapping checks. Expected: 123 cases, 36 rows (35 plus the `gns.test.ts` row), no `MISSING`. +- [ ] **Step 6: Commit and open the PR.** + + ```bash + git commit -m "test(subgraph-discovery): port L2GNS tests to Foundry" + ``` + +--- + +### Task 4 (PR T4): EpochManager tests in horizon + +**Cases:** 11 (`epochs.test.ts`). + +**Files:** + +- Modify: `packages/horizon/foundry.toml` (add the artifact read permission to `fs_permissions`) +- Create, under `packages/horizon/test/unit/`: + - `shared/ProtocolFixture.t.sol` + - `epochs/IEpochManagerLike.sol` + - `epochs/EpochManager.t.sol` +- Create: `scripts/contracts-migration/test-mapping/t4.md` +- Delete: `packages/contracts-test/tests/unit/epochs.test.ts` + +**Interfaces:** + +- Consumes: `ProtocolFixture`. +- horizon already depends on `@graphprotocol/contracts`. +- EpochManager: `PKG_ARTIFACTS + "epochs/EpochManager.sol/EpochManager.json"`, behind a proxy, `initialize(controller, 60)`. 60 is the old fixture's `lengthInBlocks`. +- `IEpochManagerLike is IEpochManager`, adding: + - `initialize(address,uint256)`; + - `epochLength()`, `lastRunEpoch()`, `lastLengthUpdateEpoch()`, `lastLengthUpdateBlock()`; + - `event EpochRun(uint256 indexed epoch, address caller)`; + - `event EpochLengthUpdate(uint256 indexed epoch, uint256 epochLength)`. + +Steps: + +- [ ] **Step 1: Write `EpochManager.t.sol`.** Include every `epochs.test.ts` case, plus Review Focus 4: `test_EpochManager_SameBlockDoesNotAdvance` (no roll means the same `currentEpoch` and `currentEpochBlock`). + - The lifecycle block uses `setEpochLength(3)`. + - Block positions use `vm.roll` relative to `currentEpochBlock()`. +- [ ] **Step 2: Run** `cd packages/horizon && forge test --match-path 'test/unit/epochs/*'`. Expected: pass. Then run `pnpm --filter @graphprotocol/horizon test`. Expected: the full horizon suite still passes. +- [ ] **Step 3: Cover the checklist** for `contracts/epochs/EpochManager.sol`. The baseline lacks `runEpoch` and `isCurrentEpochRun`, because the old test used uninstrumented sdk artifacts. The ported cases cover them anyway. Expected additions: + - `setEpochLength` with the same value reverts `Epoch length must be different to current`; + - `blockHash` reverts `Can only retrieve past block hashes` and `Can only retrieve hashes for last 256 blocks`; + - `epochsSince`, `epochsSinceUpdate` and `currentEpochBlockSinceStart` return the expected values after the rolls. +- [ ] **Step 4: Run** `pnpm --filter @graphprotocol/contracts-tests test:self`, `pnpm check:bytecode` and `pnpm lint:sol && git status --short`. Then write `t4.md` and run the mapping checks. Expected: all pass, and 11 cases, 11 rows, no `MISSING`. +- [ ] **Step 5: Commit and open the PR.** + + ```bash + git commit -m "test(horizon): port EpochManager tests to Foundry" + ``` + +--- + +### Task 5a (PR T5a): RewardsManager fixture, mocks, interface and config tests in issuance + +**Cases:** 35 (`rewards-interface` 14, `rewards-config` 21). + +**Files:** + +- Modify: + - `packages/issuance/package.json`: + - add the devDependency `"@graphprotocol/contracts": "workspace:^"` (no cycle: contracts depends only on interfaces); + - change `"test"` to `"pnpm build && pnpm test:self"`. Spec §4: `test` builds first, so the RewardsManager artifact exists. + - `packages/issuance/foundry.toml` (add the artifact read permission) + - `packages/issuance/test/unit/mocks/MockGraphToken.sol` (add `function burn(uint256 amount) external`, burning from `msg.sender`) +- Create, under `packages/issuance/test/unit/`: + - `shared/ProtocolFixture.t.sol` + - under `rewards-manager/`: + - `shared.t.sol` (`RewardsManagerSharedTest`) + - `IRewardsManagerLike.sol` + - `mocks/RMCurationMock.sol` + - `mocks/RMIssuerMock.sol` + - `mocks/RMIssuanceAllocatorMock.sol` + - `mocks/RMEligibilityOracleMock.sol` + - `interface.t.sol` + - `config.t.sol` +- Create: `scripts/contracts-migration/test-mapping/t5a.md` +- Delete, under `packages/contracts-test/tests/unit/rewards/`: + - `rewards-interface.test.ts` + - `rewards-config.test.ts` + +**Interfaces:** + +- Consumes: `ProtocolFixture`, and issuance's `contracts/test/allocate/MockERC165.sol` and `test/unit/mocks/MockGraphToken.sol`. +- Mock names carry an `RM` prefix, because issuance already has `MockIssuanceAllocator`, `MockSubgraphService` and `RewardsEligibilityOracleMock`. +- Produces, used by T5b–T5f: + - `RewardsManagerSharedTest`: + - fields: `rm` (`IRewardsManagerLike`), `token` (`MockGraphToken`), `curation` (`RMCurationMock`), `issuer` (`RMIssuerMock`), `allocator` (`RMIssuanceAllocatorMock`), `oracle` (`RMEligibilityOracleMock`), and `indexer` (`makeAddr`); + - constants: `ISSUANCE_PER_BLOCK = 200 ether`, `SIGNAL = 1500 ether`, `ALLOCATION = 12_500 ether`. + - Helpers on `RewardsManagerSharedTest`: + - `_signal(bytes32 sd, uint256 tokens)` calls `curation.signal`. + - `_allocate(address allocationId, bytes32 sd, uint256 tokens)` calls `issuer.allocate(allocationId, indexer, sd, tokens)`. + - `_close(address allocationId) returns (uint256)` calls `issuer.close`. + - `_expectedRewardsPerSignal(uint256 blocks, uint256 totalSignal) returns (uint256)` is `blocks * ISSUANCE_PER_BLOCK * 1e18 / totalSignal`. + - `IRewardsManagerLike is IRewardsManager, IRewardsManagerDeprecated, IManaged, IIssuanceTarget, IProviderEligibilityManagement, IERC165`. Leave out any base that `IRewardsManager` already inherits. + - All bases come from `@graphprotocol/interfaces`, except `IERC165`, which comes from OZ5 `utils/introspection/IERC165.sol`. + - It adds: + - `initialize(address)`; + - `accRewardsPerSignal()`, `accRewardsPerSignalLastBlockUpdated()`; + - `subgraphAvailabilityOracle()`, `subgraphs(bytes32)`, `denylist(bytes32)`, `minimumSubgraphSignal()`; + - `event ParameterUpdated(string param)`. + - Interface-id tests use `type(IRewardsManager).interfaceId` etc., never the `Like` type. + - `RMCurationMock(MockGraphToken token)`: + - `setRewardsManager(address)`; + - `signal(bytes32 sd, uint256 tokens)`: calls `rm.onSubgraphSignalUpdate(sd)`, then bumps pool tokens and mints `tokens` to itself; + - `unsignal(bytes32 sd, uint256 tokens)`: hook first, then lowers pool tokens and burns `tokens` from itself; + - `addFees(bytes32 sd, uint256 tokens)`: no hook; bumps pool tokens and mints; + - `getCurationPoolTokens(bytes32) view returns (uint256)`. + - `RMIssuerMock is IRewardsIssuer`: + - `setRewardsManager(address)`; + - `allocate(address allocationId, address indexer, bytes32 sd, uint256 tokens)`: `snapshot = rm.onSubgraphAllocationUpdate(sd)`, stores an active allocation `(indexer, sd, tokens, snapshot, pending 0)`, then bumps allocated tokens. + - `close(address allocationId) returns (uint256)`: `rm.takeRewards(allocationId)` while still active, then deactivates and lowers allocated tokens. Doc comment: legacy Staking called `onSubgraphAllocationUpdate` instead for a zero POI or an allocation under one epoch old. The ported tests never hit that case. + - `setAllocation(address allocationId, bool isActive, address indexer, bytes32 sd, uint256 tokens, uint256 accRewardsPerAllocatedToken, uint256 accRewardsPending)`; + - `setSubgraphAllocatedTokens(bytes32, uint256)`; + - `reclaim(bytes32 reason, address allocationId) returns (uint256)`: calls `rm.reclaimRewards`; + - `getAllocationData` and `getSubgraphAllocatedTokens`, per `IRewardsIssuer`. + - `RMIssuanceAllocatorMock` and `RMEligibilityOracleMock`: 0.8 ports of `packages/contracts/contracts/tests/` `MockIssuanceAllocator.sol` and `MockRewardsEligibilityOracle.sol`, with the same functions and ERC-165 answers. + +**`setUp`:** + +1. `vm.warp(1_700_000_000)`, `vm.roll(1_000_000)`, `_deployProtocolBase()`. +2. RewardsManager from `PKG_ARTIFACTS + "rewards/RewardsManager.sol/RewardsManager.json"` behind a proxy: `initialize(controller)`. +3. Register `GraphToken` (the MockGraphToken) and `Curation` (RMCurationMock), then call `rm.syncAllContracts()`. +4. `curation.setRewardsManager(rm)`, `issuer.setRewardsManager(rm)`. +5. As governor: `setSubgraphService(issuer)`, `setIssuancePerBlock(ISSUANCE_PER_BLOCK)`. + +No artifact from horizon, subgraph-discovery or graph-token is loaded (spec §2 acyclicity). + +Steps: + +- [ ] **Step 1: Scaffold.** Write the mocks, `IRewardsManagerLike` and `RewardsManagerSharedTest`, plus `test_Setup_RewardsManagerWired`: `subgraphService() == issuer` and `issuancePerBlock() == 200 ether`. +- [ ] **Step 2: Run** `pnpm --filter @graphprotocol/issuance test`. Expected: the new test and the existing issuance suite pass. +- [ ] **Step 3: Port** `rewards-interface.test.ts` into `interface.t.sol`. Keep the exact interface ids: IERC165 `0x01ffc9a7`, IIssuanceTarget `0x19f6601a`, IRewardsManager `0x8469b577`. +- [ ] **Step 4: Port** `rewards-config.test.ts` into `config.t.sol`. The "initialize on implementation" case calls `initialize` on the proxy, as the old test did, and expects `Only implementation`. +- [ ] **Step 5: Run** the package tests, `pnpm --filter @graphprotocol/contracts-tests test:self`, `pnpm check:bytecode` and `pnpm lint:sol && git status --short`. Then write `t5a.md` and run the mapping checks. Expected: all pass, and 35 cases, 35 rows. +- [ ] **Step 6: Commit and open the PR.** + + ```bash + git commit -m "test(issuance): add RewardsManager Foundry fixture and port interface and config tests" + ``` + +--- + +### Tasks 5b–5f (PRs T5b–T5f): remaining RewardsManager tests + +Each PR: + +- adds `.t.sol` files under `packages/issuance/test/unit/rewards-manager/`; +- writes its own mapping file (`t5b.md` … `t5f.md`); +- deletes its TS files. + +Use T5a's `RewardsManagerSharedTest`. Add a helper there only when two files need it. If two parallel PRs add the same helper, the second to rebase keeps one. + +| PR | Cases | New files | Deleted TS files (`packages/contracts-test/tests/unit/rewards/`) | +| --- | ----- | ----------------------------------------------------------------------------------- | -------------------------------------------------------------------------------- | +| T5b | 25 | `calculations.t.sol`, `distribution.t.sol` | `rewards-calculations.test.ts`, `rewards-distribution.test.ts` | +| T5c | 32 | `issuanceAllocator.t.sol`, `subgraphService.t.sol` | `rewards-issuance-allocator.test.ts`, `rewards-subgraph-service.test.ts` | +| T5d | 56 | `eligibility.t.sol`, `reclaim.t.sol` (split further by describe if over ~800 lines) | `rewards-eligibility-oracle.test.ts`, `rewards-reclaim.test.ts` | +| T5e | 21 | `signalAllocationUpdate.t.sol`, `snapshotInversion.t.sol` | `rewards-signal-allocation-update.test.ts`, `rewards-snapshot-inversion.test.ts` | +| T5f | 42 | `legacySuite.t.sol` (unique cases only) | `rewards.test.ts` | + +Decisions each port must follow: + +- **Translations:** + - `staking.allocateFrom` becomes `_allocate`, and `closeAllocation` becomes `_close`. + - `curation.mint` becomes `_signal`. + - `helpers.mineEpoch` and `mine(n)` become `vm.roll`. + - The old suite set the legacy Staking contract as the subgraph service. `issuer` plays that role. + - `staking.collect` query fees become `curation.addFees` with the curation share only. The old test set `setCurationPercentage(100000)`, so collecting 10,000 GRT is `addFees(sd, 1_000 ether)`. +- **Legacy-Staking cases** (`should distribute rewards on closed allocation and stake`, `…and send to destination`, `…w/delegators`, in T5b's distribution file and again in T5f): + - Port the RewardsManager half: the `HorizonRewardsAssigned` amount, the mint to the issuer and the total supply. + - Drop the stake, destination and delegation-pool assertions as `dropped: legacy Staking behaviour; Horizon equivalent in subgraph-service test/unit/subgraphService/collect/indexing/indexing.t.sol`. + - Name the matching test functions there, such as `test_SubgraphService_Collect_Indexing_WithDelegation`. +- **`two allocations in the same block with a GRT burn in the middle`:** use `MockGraphToken.burn`. +- **T5b** also adds Review Focus 4: `test_Rewards_ZeroWhenNoBlocksElapsed`. +- **Snapshot inversion (T5e):** `vm.store` replaces `hardhat_setStorageAt`. + - The `subgraphs` mapping is at slot 16, with fields at `+0..+3` (`accRewardsForSubgraph`, `accRewardsForSubgraphSnapshot`, `accRewardsPerSignalSnapshot`, `accRewardsPerAllocatedToken`) from `keccak256(abi.encode(sd, uint256(16)))`. + - `test_SnapshotInversion_StorageSlotsMatchLayout` checks this before the other tests rely on it. +- **Same-block cases (T5e):** batched Hardhat automine cases become consecutive calls with no `vm.roll` in between. +- **`rewards.test.ts` (T5f).** Most of its cases duplicate files T5a–T5e already ported. + - For each case, find the same-titled test and diff the assertions, not just the titles. + - If the assertions match, the row points at the existing test. If they differ, port the difference into `legacySuite.t.sol`. + - Known unique cases: + - the 75/25 split variant of `accrued for each subgraph` (±1%); + - `should handle zero rewards with denylist and reclaim address`; + - `should handle zero rewards with eligibility oracle and reclaim address`; + - `should allow collecting pre-denial rewards after undeny`; + - `allocation created while denied should only earn post-undeny rewards`; + - `should reclaim denied-period rewards via onSubgraphAllocationUpdate`. +- **Coverage checklist.** + - Each of T5b–T5e runs it for `contracts/rewards/RewardsManager.sol` and records its `MISSING` names. + - T5f closes it: every listed RewardsManager function is reached, or listed with a reason. + - For `contracts/staking/libs/MathUtils.sol`, only `diffOrZero` is reachable from RewardsManager. `min` and `weightedAverageRoundingUp` are listed as `reached only through legacy Staking`. + +Steps (each of T5b–T5f): + +- [ ] **Step 1: Port** the first file listed for the PR. Run `cd packages/issuance && forge test --match-path 'test/unit/rewards-manager/'`. Expected: pass. +- [ ] **Step 2: Port** the second file, if the PR has one. Run the same command for it. Expected: pass. +- [ ] **Step 3: Run** `pnpm --filter @graphprotocol/issuance test`, `pnpm --filter @graphprotocol/contracts-tests test:self`, `pnpm check:bytecode` and `pnpm lint:sol && git status --short`. Expected: all pass, and no files changed. +- [ ] **Step 4:** Write the mapping file and run the mapping checks. Expected: rows equal the PR's case count in the table, and no `MISSING`. +- [ ] **Step 5: Commit and open the PR.** + + ```bash + git commit -m "test(issuance): port RewardsManager tests to Foundry" + ``` + +--- + +## After Phase T + +- `contracts-test` contains only tests for deleted or legacy contracts, plus `subgraphAvailability.test.ts`. D1 deletes them. +- The temporary `@graphprotocol/contracts` devDependencies are removed one by one by the Phase 1 moves. +- The spec (`design.md`) was amended to match this plan: ManagedMock and TokenUtilsMock in the mocks table, ArbSysMock still deleted, no Hardhat config before Phase 1, and no allocation-flow cases moved to `packages/testing`. From 5b8177b8e6223bdbda0e14ddec1ca3ddcb93aac0 Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Fri, 23 Oct 2026 20:29:50 -0300 Subject: [PATCH 05/11] docs: clarify phase T naming and foundry.toml lint comment --- docs/contracts-removal/phase-t-plan.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/contracts-removal/phase-t-plan.md b/docs/contracts-removal/phase-t-plan.md index 9de44d09c..ab213c22b 100644 --- a/docs/contracts-removal/phase-t-plan.md +++ b/docs/contracts-removal/phase-t-plan.md @@ -73,7 +73,7 @@ The executors read the spec and this plan together. - `contracts/contracts/gateway/ICallhookReceiver.sol` - These packages don't depend on OpenZeppelin, so a violation fails to compile. - **Naming:** - - Test functions are `test__` and `test__RevertWhen_`, where `` is the old `describe` name in PascalCase. + - Test functions are `test__` and `test__RevertWhen_`, where `` is the innermost old `describe` name in PascalCase (as in T1's `Controller.t.sol`). - The test contract is named after its file (`Controller.t.sol` → `ControllerTest`). - Shared setup lives in `shared.t.sol` or `shared/`. - **Translating a TS test:** @@ -324,7 +324,7 @@ The current `parseLcov` reads `LF`/`LH` and overwrites repeated `SF:` records fo - remappings `@graphprotocol/=node_modules/@graphprotocol/` and `forge-std/=node_modules/forge-std/src/` only - the `fs_permissions` line above - `ignored_warnings_from = ["node_modules"]` -- Keep `solc_version = '0.8.35'`, `evm_version = 'cancun'` and the `[lint]` block. +- Keep `solc_version = '0.8.35'`, `evm_version = 'cancun'` and the `[lint]` block. Reword the `[lint]` comment so it doesn't mention `forge lint contracts/` (there is no `contracts/` folder yet). - No `[profile.prod]`. `ManagedMock.sol` (0.7.6, in `contracts/tests/` next to `GovernedMock.sol`): From c1245739426a1768d865929452efa1db557d0d64 Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Tue, 27 Oct 2026 10:48:14 -0300 Subject: [PATCH 06/11] docs: note L2GraphToken artifact path change in phase 1 step 3 --- docs/contracts-removal/design.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/contracts-removal/design.md b/docs/contracts-removal/design.md index 4937d8001..bb38c63c8 100644 --- a/docs/contracts-removal/design.md +++ b/docs/contracts-removal/design.md @@ -358,7 +358,9 @@ Per-step notes: - Switches `packages/testing`'s Solidity imports. - Verifies that the Ignition journals see no new futures. 2. **graph-token.** -3. **subgraph-discovery.** Also adds GraphCurationToken and SubgraphNFTDescriptor to `horizon/addresses.json`, and switches subgraph-service's GNS and Curation Ignition modules. +3. **subgraph-discovery.** + - Adds GraphCurationToken and SubgraphNFTDescriptor to `horizon/addresses.json`, and switches subgraph-service's GNS and Curation Ignition modules. + - Changes the L2GraphToken path in `DiscoveryTest.t.sol`. Its tests load L2GraphToken from `PKG_ARTIFACTS`, but L2GraphToken moved to graph-token in step 2, and Foundry does not compile it in subgraph-discovery. Add a `TOKEN_ARTIFACTS` constant pointing at graph-token's artifacts and load it from there with `_deployArtifact`, including its `fs_permissions` entry. This is the one place where Phase 1 changes a test body. 4. **horizon gets EpochManager.** - Proves the two-compiler config, the `-v3` alias, the Foundry skip, the lint ignores and the `-v3` CI grep rule. - Bumps horizon's OpenZeppelin floor to `^5.4.0`. From 823a105bd7697db48fa61e9916382739f52be39c Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Tue, 27 Oct 2026 11:10:13 -0300 Subject: [PATCH 07/11] docs: keep ignition modules frozen on published contracts 7.3.0 --- docs/contracts-removal/design.md | 44 +++++++++++++------------------- 1 file changed, 18 insertions(+), 26 deletions(-) diff --git a/docs/contracts-removal/design.md b/docs/contracts-removal/design.md index bb38c63c8..f5724c6d4 100644 --- a/docs/contracts-removal/design.md +++ b/docs/contracts-removal/design.md @@ -18,6 +18,7 @@ Out of scope: ### 1. npm - `@graphprotocol/contracts` is frozen at 7.3.0, marked deprecated with `npm deprecate`, and never published again. The git tag `@graphprotocol/contracts@7.3.0` preserves its source, CHANGELOG and docs (`DEPLOYMENT.md`, `TESTING.md`). +- horizon's and subgraph-service's Ignition modules stay as they are, as the record of how Horizon was deployed. They keep importing their artifacts from the published 7.3.0 (§5). - `@graphprotocol/sdk` is already deprecated in its README (which points to `@graphprotocol/toolshed`). It also gets `npm deprecate`. - Known external consumers (graph-network-subgraph, subgraph-studio, network-monitor) pin old versions and are migrated as needed. Addresses come from `@graphprotocol/address-book`, ABIs from `@graphprotocol/interfaces`. @@ -43,7 +44,7 @@ Rules that keep this acyclic: - `issuance` depends on `graph-core` and `interfaces` only. Its code and tests never use horizon, subgraph-discovery or graph-token, because `horizon → toolshed → issuance` already exists. - The 0.7.6 packages (`graph-core`, `graph-token`, `subgraph-discovery`, `legacy`) do not use `toolshed` or `hardhat-graph-protocol`. They have their own small Hardhat configs. -- `horizon → issuance` is only for deploying RewardsManager from Ignition. It already exists transitively through toolshed. +- `horizon → issuance` is only for RewardsManager's verification artifacts (`pre-verify`, `tenderly.config.json`). It already exists transitively through toolshed. | Package | Contracts | Compiler | | -------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------ | @@ -57,7 +58,7 @@ Rules that keep this acyclic: Why `subgraph-discovery` is its own package: - These contracts need OpenZeppelin 3.4.2 under its real name. `ISubgraphNFT` and `IGraphCurationToken` in `@graphprotocol/interfaces` import `@openzeppelin/contracts/token/ERC721/IERC721.sol` and `@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol`. Inside horizon or subgraph-service (Hardhat 2, OpenZeppelin 5), those imports resolve to v5 and fail. An alias cannot fix imports inside `interfaces`. -- horizon's Ignition modules deploy these contracts, so the package must sit below horizon. +- horizon's verification tooling reads these contracts' artifacts, so the package must sit below horizon. Deleted, with no replacement: @@ -94,7 +95,7 @@ The 0.7.6 settings must reproduce the current build exactly: solc 0.7.6, optimiz - Each has a single Hardhat compiler: 0.7.6 with the settings above. - graph-token, subgraph-discovery and legacy depend on OpenZeppelin `contracts` and `contracts-upgradeable` 3.4.2 under their real names. - graph-core has no OpenZeppelin imports of its own. It only reaches `IERC20` through `IGraphToken`, so it depends on OpenZeppelin `^5.4`, whose `IERC20` uses `>=0.4.16`. That keeps graph-core importable from 0.7.6 code, 0.8 code and 0.8 Foundry tests. (EpochManager, RewardsManager and L2GNS were verified to produce identical bytecode with v5's `IERC20`.) -- Ignition and `packages/deployment` always use these packages' own 0.7.6 artifacts. That includes GraphProxy, GraphProxyAdmin and Controller, which are deployed from 0.7.6 builds. +- `packages/deployment` always uses these packages' own 0.7.6 artifacts. That includes GraphProxy, GraphProxyAdmin and Controller, which are deployed from 0.7.6 builds. Ignition keeps the 7.3.0 artifacts (§5). - The graph-core files keep the dual pragma `^0.7.6 || ^0.8.27`, so 0.8 code can import them as source. Managed moves from `^0.7.6` to that dual pragma; it was verified to compile under 0.8.35. - graph-token adds `arbos-precompiles` (needed by L2ArbitrumMessenger). - `TokenUtils` (in graph-token) also keeps its dual pragma. It is imported as source by 0.8 code in horizon (HorizonStaking, GraphPayments, PaymentsEscrow) and subgraph-service (SubgraphService, DisputeManager, AllocationManager, AllocationHandler). @@ -112,7 +113,7 @@ The 0.7.6 settings must reproduce the current build exactly: solc 0.7.6, optimiz - The 0.7.6 entry must not inherit `viaIR` or `evmVersion: cancun`. - Foundry: `skip = ["contracts/legacy-0.7/**"]`. `lint:forge`, solhint and prettier also ignore `legacy-0.7/`. - CI rule: no file outside `legacy-0.7/` may import the `-v3` aliases (grep check). -- horizon already has its own `Managed` (`contracts/staking/utilities/Managed.sol`). Consumers must refer to graph-core's Managed by its fully qualified name in artifacts, typechain and Ignition. +- horizon already has its own `Managed` (`contracts/staking/utilities/Managed.sol`). Consumers must refer to graph-core's Managed by its fully qualified name in artifacts and typechain. ### 4. Tests @@ -191,25 +192,18 @@ Every test-port PR includes a table mapping each old test case to its new Foundr #### Ignition -- horizon's periphery modules switch their artifact imports: - - Curation and GNS: to `subgraph-discovery`. - - RewardsManager: to `issuance`. - - GraphToken and GraphTokenGateway: to `graph-token`. - - Controller, GraphProxyAdmin and GraphProxy: to `graph-core`. - - EpochManager: to horizon's own artifacts. +Ignition deployed Horizon, but new deployments go through `packages/deployment`. The Ignition modules and journals stay unchanged as the record of that deployment. - The same applies to `proxy/GraphProxy.ts`, `migrate/migrate-4.ts` and `core/HorizonStaking.ts`. - -- subgraph-service's `ignition/modules/GNS.ts` (SubgraphNFT, L2GNS) and `Curation.ts` (L2Curation) switch to `subgraph-discovery`. -- issuance's artifacts are Hardhat 3 format (`hh3-artifact-1`). Step 5 confirms that horizon's Hardhat 2 Ignition deploy, the journal check, `pre-verify` and verification all work with them. If not, the fix is to ship a Hardhat 2-shaped copy of the RewardsManager artifact from issuance. - -- subgraph-service's `ignition/modules/DisputeManager.ts` and `ServiceRegistry.ts` use the legacy DisputeManager and ServiceRegistry only through `contractAt`. They switch to artifacts compiled by `@graphprotocol/interfaces` for `IDisputeManager` / `IServiceRegistry`. If those aren't suitable, the ABIs are committed as artifact-shaped JSON under `subgraph-service/ignition/`. -- Existing deployment journals under `ignition/deployments/*` reference the old artifacts. The first move PR verifies that re-running the horizon and subgraph-service modules against the existing Arbitrum Sepolia deployment plans no new futures, i.e. the changed metadata hash doesn't trigger a redeploy. If it does, the fix is documented and applied before any other move. +- horizon and subgraph-service import 14 artifacts in their modules from `@graphprotocol/contracts/artifacts/...`. Each package's `tasks/deploy.ts` imports the modules, and `loadTasks` loads it in every built Hardhat run. Both tsconfigs also include `ignition/**/*.ts`. The imports must therefore keep resolving after the workspace package is gone. +- D3 switches horizon's and subgraph-service's dependency from `workspace:^` to `npm:@graphprotocol/contracts@7.3.0`. That PR deletes DisputeManager and ServiceRegistry, the first artifacts the modules import that disappear from the workspace. The published 7.3.0 tarball ships every artifact the modules import. The import paths don't change, so no module file changes. +- Until each Phase 1 step switches them, horizon's and subgraph-service's remaining Solidity imports from `@graphprotocol/contracts` resolve to the 7.3.0 sources, and so does the EpochManager test artifact. These are live files that are unchanged since 7.3.0. 0c confirms that the horizon and subgraph-service bytecode doesn't change. +- Phase 1 never removes this dependency from horizon or subgraph-service. It is the only `@graphprotocol/contracts` reference left after Phase 2. #### Verification and scripts - `horizon/scripts/pre-verify` and `subgraph-service/scripts/pre-verify` copy `../contracts/artifacts`. They switch to the new packages. - `horizon/tenderly.config.json` `externalArtifacts` switches the same way. +- issuance's artifacts are Hardhat 3 format (`hh3-artifact-1`). Step 5 confirms that `pre-verify` and verification work with them. If not, the fix is to ship a Hardhat 2-shaped copy of the RewardsManager artifact from issuance. #### `packages/deployment` @@ -326,7 +320,7 @@ The detailed task split is in `phase-t-plan.md`. - `addresses-staging.json`, `addresses-local.json`, `arbitrum-addresses.json`; - `config/`, and `configDir` in `index.js`. - Live files that also have copies in `legacy` stay: Multicall, GNS, GNSStorage, CurationStorage, Managed, GraphUpgradeable, TokenUtils and MathUtils. Nothing that remains imports a deleted file. The same PR updates subgraph-service's legacy DisputeManager and ServiceRegistry Ignition imports (§5). + Live files that also have copies in `legacy` stay: Multicall, GNS, GNSStorage, CurationStorage, Managed, GraphUpgradeable, TokenUtils and MathUtils. Nothing that remains imports a deleted file. The same PR pins horizon's and subgraph-service's `@graphprotocol/contracts` dependency to the published 7.3.0 (§5). After D3, `packages/contracts` contains only live contracts. @@ -335,9 +329,9 @@ After D3, `packages/contracts` contains only live contracts. Each PR: - moves the files; -- switches every consumer's Solidity imports, Ignition artifacts, `deployment` sources and test artifact paths; +- switches every consumer's Solidity imports, `deployment` sources and test artifact paths (not the Ignition modules, §5); - passes 0c; -- removes the moved package's temporary `@graphprotocol/contracts` devDependency; +- removes the moved package's temporary `@graphprotocol/contracts` devDependency (horizon keeps its 7.3.0 pin for Ignition); - flips the moved package to published (drops `"private"`, adds a `publish.yml` option and an initial changeset). Dependency edges added in each step (removing the devDependency is what prevents a cycle): @@ -356,10 +350,9 @@ Per-step notes: - Generalizes `deployment`'s artifact loader to one source per package, and adds the new package cases to `tasks/verify-contract.ts`. - Updates `pre-verify` and `tenderly.config.json`. - Switches `packages/testing`'s Solidity imports. - - Verifies that the Ignition journals see no new futures. 2. **graph-token.** 3. **subgraph-discovery.** - - Adds GraphCurationToken and SubgraphNFTDescriptor to `horizon/addresses.json`, and switches subgraph-service's GNS and Curation Ignition modules. + - Adds GraphCurationToken and SubgraphNFTDescriptor to `horizon/addresses.json`. - Changes the L2GraphToken path in `DiscoveryTest.t.sol`. Its tests load L2GraphToken from `PKG_ARTIFACTS`, but L2GraphToken moved to graph-token in step 2, and Foundry does not compile it in subgraph-discovery. Add a `TOKEN_ARTIFACTS` constant pointing at graph-token's artifacts and load it from there with `_deployArtifact`, including its `fs_permissions` entry. This is the one place where Phase 1 changes a test body. 4. **horizon gets EpochManager.** - Proves the two-compiler config, the `-v3` alias, the Foundry skip, the lint ignores and the `-v3` CI grep rule. @@ -383,18 +376,17 @@ Per-step notes: - 0c passes. - `pnpm build` and `pnpm test` pass across the workspace. - `packages/deployment` sync/status against an Arbitrum Sepolia fork reports no code changes. -- **Step 1 additionally:** the Ignition modules plan no new futures against the existing deployments. - **Test-port PRs:** - the mapping table is complete; - for packages Foundry compiles, coverage is at least the 0d baseline after the move. -- **Final:** a repo-wide search for `@graphprotocol/contracts` (outside historical Ignition build-info and CHANGELOGs) returns nothing. +- **Final:** a repo-wide search for `@graphprotocol/contracts` returns nothing outside the Ignition modules and journals, horizon's and subgraph-service's 7.3.0 pin, the lockfile and the CHANGELOGs. ## Risks - **Merge conflicts.** RewardsManager, L2GNS and L2Curation are under active development. Keep T3, T5 and Phase 1 steps 3 and 5 short-lived, and coordinate timing with open work. -- **Ignition reconciliation.** If Ignition treats the metadata change as a modified artifact for executed futures, step 1 must resolve it before other moves land. +- **Ignition modules are frozen.** They stay on the 7.3.0 artifacts. Re-running them would deploy 7.3.0 code, not the moved packages. New deployments go through `packages/deployment`. - **Two OpenZeppelin versions in horizon and issuance.** Limited to `legacy-0.7/`, and enforced by the CI grep rule. In issuance, Hardhat 3 resolves imports per package, so the import rule in §3 also matters there. -- **Hardhat 3 artifacts consumed by Hardhat 2 tooling.** horizon's Ignition and verification tooling read the RewardsManager artifact from issuance. Step 5 validates this, with a Hardhat 2-shaped copy as the fallback. +- **Hardhat 3 artifacts consumed by Hardhat 2 tooling.** horizon's verification tooling reads the RewardsManager artifact from issuance. Step 5 validates this, with a Hardhat 2-shaped copy as the fallback. - **Mainnet source drift.** Current source may not reproduce every mainnet implementation exactly. D2's onchain comparison catches this, and `legacy` then uses the source from the deployed tag. - **Test port fidelity.** A rewrite can silently drop behaviours. The mapping table and the coverage baseline guard against this. - **Library link references.** Only L1Staking (legacy) links a library (`Exponential`). The 0c check covers it. None of the live moved contracts link libraries. From 5d9abfbed2c089901ea39fca485d147229c42b7c Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Fri, 30 Oct 2026 10:33:14 -0300 Subject: [PATCH 08/11] docs: note L2GraphToken exception in phase T plan --- docs/contracts-removal/phase-t-plan.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/contracts-removal/phase-t-plan.md b/docs/contracts-removal/phase-t-plan.md index ab213c22b..8fd3b7984 100644 --- a/docs/contracts-removal/phase-t-plan.md +++ b/docs/contracts-removal/phase-t-plan.md @@ -9,7 +9,7 @@ - Each destination package gets a copy of `ProtocolFixture`, a small test base contract. It deploys 0.7.6 contracts from Hardhat JSON artifacts, sets up Controller and GraphProxyAdmin, and puts contracts behind a GraphProxy. - Tests are Solidity 0.8 and reach the contracts through 0.8-compilable interfaces. - Every dependency that isn't under test is a mock. -- Artifact paths are always built from two constants, `CORE_ARTIFACTS` (Controller, GraphProxy, GraphProxyAdmin) and `PKG_ARTIFACTS` (the package's own contracts and mocks). In Phase T both point at `@graphprotocol/contracts`. Phase 1 repoints them, and in graph-core, graph-token and subgraph-discovery switches `_deployArtifact` to `deployCode`. The test bodies don't change. +- Artifact paths are always built from two constants, `CORE_ARTIFACTS` (Controller, GraphProxy, GraphProxyAdmin) and `PKG_ARTIFACTS` (the package's own contracts and mocks). In Phase T both point at `@graphprotocol/contracts`. Phase 1 repoints them, and in graph-core, graph-token and subgraph-discovery switches `_deployArtifact` to `deployCode`. The test bodies don't change, except subgraph-discovery's `DiscoveryTest.t.sol`, which loads L2GraphToken from graph-token's artifacts (design, Phase 1 step 3). **Tech Stack:** Foundry 1.8.3 (CI's version: `foundryup -i 1.8.3`), forge-std 1.14, solc 0.8.35, pnpm workspaces, Hardhat 2 artifacts from `packages/contracts`, Node 24 (T0 script). From 032de975a5b244c639a0bb591cde0730dc64f878 Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Fri, 30 Oct 2026 11:29:14 -0300 Subject: [PATCH 09/11] docs: add phase D plan and correct ignition and legacy design --- docs/contracts-removal/design.md | 99 +++++----- docs/contracts-removal/phase-d-plan.md | 250 +++++++++++++++++++++++++ 2 files changed, 298 insertions(+), 51 deletions(-) create mode 100644 docs/contracts-removal/phase-d-plan.md diff --git a/docs/contracts-removal/design.md b/docs/contracts-removal/design.md index f5724c6d4..c74411325 100644 --- a/docs/contracts-removal/design.md +++ b/docs/contracts-removal/design.md @@ -18,7 +18,7 @@ Out of scope: ### 1. npm - `@graphprotocol/contracts` is frozen at 7.3.0, marked deprecated with `npm deprecate`, and never published again. The git tag `@graphprotocol/contracts@7.3.0` preserves its source, CHANGELOG and docs (`DEPLOYMENT.md`, `TESTING.md`). -- horizon's and subgraph-service's Ignition modules stay as they are, as the record of how Horizon was deployed. They keep importing their artifacts from the published 7.3.0 (§5). +- horizon's and subgraph-service's Ignition modules stay as the record of how Horizon was deployed. They read their artifacts from the published 7.3.0 through an npm alias (§5). - `@graphprotocol/sdk` is already deprecated in its README (which points to `@graphprotocol/toolshed`). It also gets `npm deprecate`. - Known external consumers (graph-network-subgraph, subgraph-studio, network-monitor) pin old versions and are migrated as needed. Addresses come from `@graphprotocol/address-book`, ABIs from `@graphprotocol/interfaces`. @@ -43,8 +43,7 @@ legacy (new, private, no workspace dependencies) Rules that keep this acyclic: - `issuance` depends on `graph-core` and `interfaces` only. Its code and tests never use horizon, subgraph-discovery or graph-token, because `horizon → toolshed → issuance` already exists. -- The 0.7.6 packages (`graph-core`, `graph-token`, `subgraph-discovery`, `legacy`) do not use `toolshed` or `hardhat-graph-protocol`. They have their own small Hardhat configs. -- `horizon → issuance` is only for RewardsManager's verification artifacts (`pre-verify`, `tenderly.config.json`). It already exists transitively through toolshed. +- The 0.7.6 packages (`graph-core`, `graph-token`, `subgraph-discovery`) do not use `toolshed` or `hardhat-graph-protocol`. They have their own small Hardhat configs. `legacy` has no Hardhat config. | Package | Contracts | Compiler | | -------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------ | @@ -53,34 +52,33 @@ Rules that keep this acyclic: | `subgraph-discovery` | L2GNS, L2GNSStorage, GNS, GNSStorage, L2Curation, CurationStorage, GraphCurationToken, SubgraphNFT, SubgraphNFTDescriptor, HexStrings, Base58Encoder, Multicall | solc 0.7.6 only | | `horizon` | EpochManager, EpochManagerStorage (in `contracts/legacy-0.7/`) | mixed | | `issuance` | RewardsManager, RewardsManagerStorage, MathUtils (in `contracts/legacy-0.7/`) | mixed | -| `legacy` | Copies of the mainnet-only contracts and everything they need (see "legacy package" below) | solc 0.7.6 only | +| `legacy` | Verified source of the seven mainnet implementations (see "legacy package" below) | solc 0.7.4 and 0.7.6 | Why `subgraph-discovery` is its own package: - These contracts need OpenZeppelin 3.4.2 under its real name. `ISubgraphNFT` and `IGraphCurationToken` in `@graphprotocol/interfaces` import `@openzeppelin/contracts/token/ERC721/IERC721.sol` and `@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol`. Inside horizon or subgraph-service (Hardhat 2, OpenZeppelin 5), those imports resolve to v5 and fail. An alias cannot fix imports inside `interfaces`. -- horizon's verification tooling reads these contracts' artifacts, so the package must sit below horizon. Deleted, with no replacement: - Staking: L2Staking. The Arbitrum proxy now runs HorizonStaking. - AllocationExchange, EthereumDIDRegistry, SubgraphAvailabilityManager. - The Arbitrum DisputeManager and ServiceRegistry, along with their storage contracts. -- Mocks under `contracts/tests/` that no ported test needs: ArbSysMock, `tests/ens/*`, LegacyGNSMock, and both L1GraphTokenLockTransferTool mocks. -- Each mainnet-only contract is deleted from `packages/contracts` only after its copy exists in `legacy`. +- Mocks under `contracts/tests/` that no ported test needs: ArbSysMock, `tests/ens/*`, LegacyGNSMock, MockERC165, MockSubgraphService, and both L1GraphTokenLockTransferTool mocks. +- Each mainnet-only contract is deleted from `packages/contracts` only after its verified source exists in `legacy`. #### legacy package -- A frozen, private archive of the mainnet-only contracts: L1GNS, Curation and L1Staking, plus their closure. -- It has no Solidity imports from the workspace. Its closure is copied in, interfaces included, so later edits to `@graphprotocol/interfaces` can never break it. -- Contract files (20): BancorFormula, Multicall, Curation, CurationStorage, GNS, GNSStorage, L1GNS, L1GNSStorage, Managed, L1Staking, L1StakingStorage, Staking, StakingExtension, StakingStorage, Exponential, LibFixedMath, MathUtils, Stakes, GraphUpgradeable, TokenUtils. -- Interface files (copied from `interfaces`): ITokenGateway, IMulticall, ICuration, IGraphCurationToken, IGNS, ISubgraphNFT, IEthereumDIDRegistry, IEpochManager, ICallhookReceiver, IController, IManaged, IL2GNS, IL2StakingTypes, IRewardsIssuer, IRewardsManager, IL1GraphTokenLockTransferTool, IL1StakingBase, IStaking, IStakingBase, IStakingData, IStakingExtension, IStakes, IGraphToken, IGraphProxy. -- It has its own Hardhat config (0.7.6, OpenZeppelin 3.4.2) and a CI build job, so it can't silently stop compiling. -- It has no tests. Instead, CI checks its bytecode against the mainnet implementations (see Phase 0c). +- A frozen, private archive of the source that actually runs on mainnet, for GraphToken, L1GraphTokenGateway, BridgeEscrow, L1GNS, Curation, L1Staking and StakingExtension. +- Current source reproduces only Curation. GraphToken was built with solc 0.7.4, and nine files shared between the implementations, including `Managed.sol`, differ from one implementation to the next. One shared source tree can't hold them, so each contract has its own bundle. +- Each bundle is `verified//`: the Sourcify-verified sources, unchanged, with the compiler version and settings. All seven were compiled locally and reproduce mainnet, ignoring metadata and immutable values. L1Staking's linked library address comes from its settings, so it is part of the check. +- It has no Solidity imports from the workspace, no Hardhat config and no tests of contract behaviour. Formatters and linters must not touch `verified/`. +- `test:self` checks every source file against a committed hash, compiles every bundle with its exact compiler (solc-js from npm) and compares the result with the hash of the on-chain code committed in `mainnet.json`, so CI needs no RPC secret. Recording those hashes needs an RPC URL once. The source hashes are needed because comments and formatting don't change bytecode. GraphToken and L1Staking are Sourcify partial matches, so their sources may differ from the deployed ones outside the code. +- graph-token keeps its current source for GraphToken, L1GraphTokenGateway and BridgeEscrow, and its README says this is not the code running on mainnet. - Historical audit reports from `packages/contracts/audits/` (29 PDFs) move to `packages/legacy/audits/`. ### 3. Compiler: keep 0.7.6, port only when upgrading -The source in the repo must match what is deployed, so contracts move unchanged. A contract is ported to 0.8 only when it gets a real upgrade, and the port ships and is audited together with that upgrade. Some contracts are not upgradeable and stay 0.7.6 forever: +Contracts move unchanged, so moving them changes no bytecode. For mainnet-only implementations, the record of what is deployed is `legacy`, not the package that holds the current source. A contract is ported to 0.8 only when it gets a real upgrade, and the port ships and is audited together with that upgrade. Some contracts are not upgradeable and stay 0.7.6 forever: - GraphToken (mainnet) - Controller @@ -90,12 +88,12 @@ The source in the repo must match what is deployed, so contracts move unchanged. The 0.7.6 settings must reproduce the current build exactly: solc 0.7.6, optimizer enabled, 200 runs, no `evmVersion`, no `viaIR`, default metadata. Everything else follows from that. -#### graph-core, graph-token, subgraph-discovery and legacy +#### graph-core, graph-token and subgraph-discovery - Each has a single Hardhat compiler: 0.7.6 with the settings above. -- graph-token, subgraph-discovery and legacy depend on OpenZeppelin `contracts` and `contracts-upgradeable` 3.4.2 under their real names. +- graph-token and subgraph-discovery depend on OpenZeppelin `contracts` and `contracts-upgradeable` 3.4.2 under their real names. - graph-core has no OpenZeppelin imports of its own. It only reaches `IERC20` through `IGraphToken`, so it depends on OpenZeppelin `^5.4`, whose `IERC20` uses `>=0.4.16`. That keeps graph-core importable from 0.7.6 code, 0.8 code and 0.8 Foundry tests. (EpochManager, RewardsManager and L2GNS were verified to produce identical bytecode with v5's `IERC20`.) -- `packages/deployment` always uses these packages' own 0.7.6 artifacts. That includes GraphProxy, GraphProxyAdmin and Controller, which are deployed from 0.7.6 builds. Ignition keeps the 7.3.0 artifacts (§5). +- `packages/deployment` always uses these packages' own 0.7.6 artifacts. That includes GraphProxy, GraphProxyAdmin and Controller, which are deployed from 0.7.6 builds. Ignition uses the 7.3.0 artifacts (§5). - The graph-core files keep the dual pragma `^0.7.6 || ^0.8.27`, so 0.8 code can import them as source. Managed moves from `^0.7.6` to that dual pragma; it was verified to compile under 0.8.35. - graph-token adds `arbos-precompiles` (needed by L2ArbitrumMessenger). - `TokenUtils` (in graph-token) also keeps its dual pragma. It is imported as source by 0.8 code in horizon (HorizonStaking, GraphPayments, PaymentsEscrow) and subgraph-service (SubgraphService, DisputeManager, AllocationManager, AllocationHandler). @@ -178,11 +176,11 @@ Dropped, because their contracts are deleted or legacy: #### Mocks needed by ported tests -| Mock | Destination | -| ------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------ | -| GovernedMock, ManagedMock (new, 0.7.6) | graph-core | -| BridgeMock, InboxMock, OutboxMock, CallhookReceiverMock, TokenUtilsMock (new, 0.7.6) | graph-token | -| MockERC165, MockIssuanceAllocator, MockRewardsEligibilityOracle, MockSubgraphService | issuance (reuse its existing 0.8 mocks where they exist; otherwise rewrite them in 0.8; MockSubgraphService becomes a rewards-issuer mock) | +| Mock | Destination | +| ------------------------------------------------------------------------------------ | --------------------------------------------------------------------------------------- | +| GovernedMock, ManagedMock (new, 0.7.6) | graph-core | +| BridgeMock, InboxMock, OutboxMock, CallhookReceiverMock, TokenUtilsMock (new, 0.7.6) | graph-token | +| MockIssuanceAllocator, MockRewardsEligibilityOracle | issuance (reuse its existing 0.8 mocks where they exist; otherwise rewrite them in 0.8) | ManagedMock and TokenUtilsMock are added to `packages/contracts/contracts/tests/` in Phase T, because 0.8 tests can't compile a Managed contract or call the TokenUtils library. They move with their packages in Phase 1. ArbSysMock is not used (ArbSys is faked with `vm.mockCall`), so it stays on the delete list. @@ -192,18 +190,18 @@ Every test-port PR includes a table mapping each old test case to its new Foundr #### Ignition -Ignition deployed Horizon, but new deployments go through `packages/deployment`. The Ignition modules and journals stay unchanged as the record of that deployment. +Ignition deployed Horizon, but new deployments go through `packages/deployment`. The Ignition modules and journals stay as the record of that deployment, with one mechanical change to their imports. -- horizon and subgraph-service import 14 artifacts in their modules from `@graphprotocol/contracts/artifacts/...`. Each package's `tasks/deploy.ts` imports the modules, and `loadTasks` loads it in every built Hardhat run. Both tsconfigs also include `ignition/**/*.ts`. The imports must therefore keep resolving after the workspace package is gone. -- D3 switches horizon's and subgraph-service's dependency from `workspace:^` to `npm:@graphprotocol/contracts@7.3.0`. That PR deletes DisputeManager and ServiceRegistry, the first artifacts the modules import that disappear from the workspace. The published 7.3.0 tarball ships every artifact the modules import. The import paths don't change, so no module file changes. -- Until each Phase 1 step switches them, horizon's and subgraph-service's remaining Solidity imports from `@graphprotocol/contracts` resolve to the 7.3.0 sources, and so does the EpochManager test artifact. These are live files that are unchanged since 7.3.0. 0c confirms that the horizon and subgraph-service bytecode doesn't change. -- Phase 1 never removes this dependency from horizon or subgraph-service. It is the only `@graphprotocol/contracts` reference left after Phase 2. +- horizon and subgraph-service import 14 artifacts in 15 module files from `@graphprotocol/contracts/artifacts/...`. Each package's `tasks/deploy.ts` imports the modules, and `loadTasks` loads it in every built Hardhat run. Both tsconfigs also include `ignition/**/*.ts`. The imports must therefore keep resolving after the workspace package loses those artifacts. +- Both packages can't simply pin `@graphprotocol/contracts` to 7.3.0. Their Solidity imports from it still need the workspace version until Phase 1: the published sources use the pragma `^0.7.6 || 0.8.27`, and these packages compile with 0.8.35. +- Before D3, both packages add the devDependency `"@graphprotocol/contracts-7.3.0": "npm:@graphprotocol/contracts@7.3.0"`, and the module imports change only the package name to `@graphprotocol/contracts-7.3.0`. The tarball ships every imported artifact. Journals and module logic don't change. +- Their `@graphprotocol/contracts` workspace dependency stays for Solidity imports and is removed in Phase 1 as usual. +- The modules now hold 7.3.0 bytecode. It differs from current source for RewardsManager, L2GNS and L2Curation. `test:integration`, which runs the modules on a fork and isn't in CI, therefore deploys those 7.3.0 versions. #### Verification and scripts -- `horizon/scripts/pre-verify` and `subgraph-service/scripts/pre-verify` copy `../contracts/artifacts`. They switch to the new packages. -- `horizon/tenderly.config.json` `externalArtifacts` switches the same way. -- issuance's artifacts are Hardhat 3 format (`hh3-artifact-1`). Step 5 confirms that `pre-verify` and verification work with them. If not, the fix is to ship a Hardhat 2-shaped copy of the RewardsManager artifact from issuance. +- `horizon/scripts/pre-verify`, `subgraph-service/scripts/pre-verify` and `horizon/tenderly.config.json` `externalArtifacts` read `../contracts/artifacts` for verifying Ignition deployments. They switch to the 7.3.0 alias with the modules, so they keep matching the bytecode the modules deploy. +- issuance's artifacts are Hardhat 3 format (`hh3-artifact-1`). Step 5 confirms that `packages/deployment` and verification work with them. If not, the fix is to ship a Hardhat 2-shaped copy of the RewardsManager artifact from issuance. #### `packages/deployment` @@ -226,7 +224,7 @@ Ignition deployed Horizon, but new deployments go through `packages/deployment`. - `publish.yml` drops `contracts` and adds `graph-core`, `graph-token` and `subgraph-discovery`. - The root `lint:yaml` drops `packages/contracts/task/config`. - `count-patterns.txt` paths are updated. -- `@graphprotocol/sdk` is removed from the `pnpm-workspace.yaml` catalog and from `minimumReleaseAgeExclude`. +- `@graphprotocol/sdk` is removed from the `pnpm-workspace.yaml` catalog and from `minumumReleaseAgeExclude` (the key's real, misspelled name). - `build-test.yml` coverage collection is updated for the removed packages and the new ones. - The Docker image (`Dockerfile`, `publish-image.yml`) is checked for use of `packages/contracts` or the sdk before deletion. @@ -237,7 +235,7 @@ Ignition deployed Horizon, but new deployments go through `packages/deployment`. #### address-book -Address books are symlinked from each package and listed in `address-book/scripts/sources.js`. +Address books are copied from each package into `address-book/src//addresses.json`, tracked as regular files, and listed in `address-book/scripts/sources.js`. - New `packages/legacy/addresses.json` with every L1 entry from `packages/contracts/addresses.json`: - mainnet (1) and Sepolia (11155111); @@ -280,7 +278,7 @@ Tests are ported first, against the contracts where they are today. Then dead co - **0c.** Add a bytecode equivalence check that builds on the existing `scripts/compare-repo-contract-bytecode-excluding-metadata.mjs` and `scripts/bytecode-diff.sh`. It compares bytecode with metadata stripped for: - every moved 0.7.6 contract, against a reference committed from the current `packages/contracts` build; - the 0.8 contracts whose imports change (HorizonStaking, GraphPayments, PaymentsEscrow, SubgraphService, DisputeManager and their libraries), against the current horizon and subgraph-service builds; - - `legacy` and graph-token's mainnet contracts (GraphToken, L1GraphTokenGateway, BridgeEscrow, L1GNS, Curation, L1Staking, StakingExtension), against the onchain implementation bytecode on mainnet. This part applies from D2, when `legacy` exists. If the current source no longer reproduces a mainnet implementation, `legacy` takes that contract's source from the git tag or commit it was deployed from, and the mismatch is recorded in its README. + - the verified source of the mainnet implementations (GraphToken, L1GraphTokenGateway, BridgeEscrow, L1GNS, Curation, L1Staking, StakingExtension), against the on-chain bytecode. This part lives in `legacy` from D2 and ignores metadata and immutable values. Every later PR that moves or copies contracts must pass it. @@ -307,8 +305,8 @@ The detailed task split is in `phase-t-plan.md`. ### Phase D: removal of dead code -- **D1.** Delete `packages/contracts-test`, `packages/contracts/test/` (tx logs, reports), and the `test` / `test:coverage` scripts in `packages/contracts`. Remove `@graphprotocol/sdk` (catalog and `minimumReleaseAgeExclude`). Update `build-test.yml` coverage collection. All live tests have been ported by now. -- **D2.** Create `legacy` as copies of the mainnet-only contracts, verified by 0c against mainnet bytecode. Add `legacy/addresses.json`, the `sources.js` entry, its CI build job, and move the audits to `legacy/audits/`. +- **D1.** Delete `packages/contracts-test` and the `test` / `test:coverage` scripts in `packages/contracts`. Remove `@graphprotocol/sdk` (catalog and `minumumReleaseAgeExclude`). `build-test.yml` finds coverage files itself, so it needs no change. All live tests have been ported by now. +- **D2.** Create `legacy` from the verified bundles, checked by its `test:self` against mainnet bytecode. Add `legacy/addresses.json` and the `sources.js` entry, and move the audits to `legacy/audits/`. - **D3.** Delete from `packages/contracts` exactly these contract files, plus their storage contracts where listed: - L1GNS, L1GNSStorage, Curation, BancorFormula; - Staking, L1Staking, L1StakingStorage, StakingExtension, StakingStorage, Stakes, Exponential, LibFixedMath, L2Staking; @@ -317,10 +315,10 @@ The detailed task split is in `phase-t-plan.md`. Also delete: - the unused mocks listed in §2; - - `addresses-staging.json`, `addresses-local.json`, `arbitrum-addresses.json`; + - `addresses-staging.json`, `arbitrum-addresses.json`; - `config/`, and `configDir` in `index.js`. - Live files that also have copies in `legacy` stay: Multicall, GNS, GNSStorage, CurationStorage, Managed, GraphUpgradeable, TokenUtils and MathUtils. Nothing that remains imports a deleted file. The same PR pins horizon's and subgraph-service's `@graphprotocol/contracts` dependency to the published 7.3.0 (§5). + Live files that also appear in `legacy` bundles stay: Multicall, GNS, GNSStorage, CurationStorage, Managed, GraphUpgradeable, TokenUtils and MathUtils. Nothing that remains imports a deleted file. A PR just before it adds the Ignition alias (§5). L1GNS, Curation, L1Staking, StakingExtension and LibExponential leave the 0c manifest, because `legacy` now checks them. After D3, `packages/contracts` contains only live contracts. @@ -331,24 +329,23 @@ Each PR: - moves the files; - switches every consumer's Solidity imports, `deployment` sources and test artifact paths (not the Ignition modules, §5); - passes 0c; -- removes the moved package's temporary `@graphprotocol/contracts` devDependency (horizon keeps its 7.3.0 pin for Ignition); +- removes the moved package's temporary `@graphprotocol/contracts` devDependency; - flips the moved package to published (drops `"private"`, adds a `publish.yml` option and an initial changeset). Dependency edges added in each step (removing the devDependency is what prevents a cycle): -| Step | Moves | Edges added | -| ---- | --------------------------------------- | -------------------------------------------------------------------------------------------------------- | -| 1 | graph-core | contracts, horizon, subgraph-service, testing, deployment → graph-core | -| 2 | graph-token | contracts, horizon, subgraph-service, deployment → graph-token. graph-token → graph-core | -| 3 | subgraph-discovery | horizon, subgraph-service, deployment → subgraph-discovery. subgraph-discovery → graph-token, graph-core | -| 4 | EpochManager into horizon | horizon → graph-core (already present) | -| 5 | RewardsManager, MathUtils into issuance | issuance → graph-core. horizon, testing, deployment → issuance | +| Step | Moves | Edges added | +| ---- | --------------------------------------- | ---------------------------------------------------------------------------------------- | +| 1 | graph-core | contracts, horizon, subgraph-service, testing, deployment → graph-core | +| 2 | graph-token | contracts, horizon, subgraph-service, deployment → graph-token. graph-token → graph-core | +| 3 | subgraph-discovery | deployment → subgraph-discovery. subgraph-discovery → graph-token, graph-core | +| 4 | EpochManager into horizon | horizon → graph-core (already present) | +| 5 | RewardsManager, MathUtils into issuance | issuance → graph-core. testing, deployment → issuance | Per-step notes: 1. **graph-core.** - Generalizes `deployment`'s artifact loader to one source per package, and adds the new package cases to `tasks/verify-contract.ts`. - - Updates `pre-verify` and `tenderly.config.json`. - Switches `packages/testing`'s Solidity imports. 2. **graph-token.** 3. **subgraph-discovery.** @@ -379,14 +376,14 @@ Per-step notes: - **Test-port PRs:** - the mapping table is complete; - for packages Foundry compiles, coverage is at least the 0d baseline after the move. -- **Final:** a repo-wide search for `@graphprotocol/contracts` returns nothing outside the Ignition modules and journals, horizon's and subgraph-service's 7.3.0 pin, the lockfile and the CHANGELOGs. +- **Final:** a repo-wide search for `@graphprotocol/contracts` returns nothing outside the root `@graphprotocol/contracts-monorepo` name, the Ignition journals, the `@graphprotocol/contracts-7.3.0` alias and its imports, the lockfile and the CHANGELOGs. ## Risks - **Merge conflicts.** RewardsManager, L2GNS and L2Curation are under active development. Keep T3, T5 and Phase 1 steps 3 and 5 short-lived, and coordinate timing with open work. -- **Ignition modules are frozen.** They stay on the 7.3.0 artifacts. Re-running them would deploy 7.3.0 code, not the moved packages. New deployments go through `packages/deployment`. +- **Ignition modules are frozen.** They stay on the 7.3.0 artifacts. Re-running them, including through `test:integration`, deploys 7.3.0 code, not the moved packages. New deployments go through `packages/deployment`. - **Two OpenZeppelin versions in horizon and issuance.** Limited to `legacy-0.7/`, and enforced by the CI grep rule. In issuance, Hardhat 3 resolves imports per package, so the import rule in §3 also matters there. -- **Hardhat 3 artifacts consumed by Hardhat 2 tooling.** horizon's verification tooling reads the RewardsManager artifact from issuance. Step 5 validates this, with a Hardhat 2-shaped copy as the fallback. -- **Mainnet source drift.** Current source may not reproduce every mainnet implementation exactly. D2's onchain comparison catches this, and `legacy` then uses the source from the deployed tag. +- **Hardhat 3 artifacts consumed by Hardhat 2 tooling.** `packages/deployment` reads the RewardsManager artifact from issuance. Step 5 validates this, with a Hardhat 2-shaped copy as the fallback. +- **Mainnet source drift.** Current source does not reproduce most mainnet implementations. `legacy` therefore archives the verified source, and graph-token's README says its mainnet contracts aren't the deployed code. - **Test port fidelity.** A rewrite can silently drop behaviours. The mapping table and the coverage baseline guard against this. -- **Library link references.** Only L1Staking (legacy) links a library (`Exponential`). The 0c check covers it. None of the live moved contracts link libraries. +- **Library link references.** Only L1Staking (legacy) links a library (`Exponential`). `legacy`'s check covers it. None of the live moved contracts link libraries. diff --git a/docs/contracts-removal/phase-d-plan.md b/docs/contracts-removal/phase-d-plan.md new file mode 100644 index 000000000..5f0b83ac5 --- /dev/null +++ b/docs/contracts-removal/phase-d-plan.md @@ -0,0 +1,250 @@ +# Contracts Removal — Phase D (Dead Code Removal) Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Delete the old test suite and every dead contract from `packages/contracts`, after archiving the source that actually runs on mainnet in a new private `legacy` package. + +**Architecture:** + +- D1 removes `contracts-test` and `@graphprotocol/sdk`. +- D2a creates `legacy` as per-contract bundles of Sourcify-verified source, each compiled with its exact compiler and compared with committed hashes of the on-chain code. D2b adds its address book and audits. +- D3a points the Ignition modules at an npm alias of `@graphprotocol/contracts@7.3.0`, so they survive the deletions. D3b deletes the dead contracts. + +**Tech Stack:** pnpm workspaces, Node 24 (`node:test`, `node:crypto`), solc-js 0.7.4 and 0.7.6 from npm, Hardhat 2, Foundry 1.8.3. + +**Spec:** `docs/contracts-removal/design.md` (§2 "legacy package", §5 "Ignition", §5 "address-book", "PR sequence → Phase D"). + +## Global Constraints + +- Each task is a draft PR in one chain, the same way as Phase T: D1 targets `chore/contracts-removal-phase-0` (#1373), D2a targets D1's branch, then D2b, D3a and D3b each target the one before. Nothing goes to main before the audit. +- Conventional commits, no Co-Authored-By line and no Claude attribution in commits or PRs. +- Sources under `packages/legacy/verified/` are never edited, formatted or linted. +- No RPC URL or API key is ever written to a file, commit, report or PR. RPC URLs are passed only as inline env vars. +- D3b's list comes from the spec. Do not delete any other contract. +- `packages/legacy` never gets a `.solhint.json`: CI's changed-files mode runs solhint from that package and passes only because it loads no rules there. +- D1 starts after #1387 merges into the base branch, so the stack starts from the integration tip. +- Ignition module changes are limited to the package name in import lines. Journals under `ignition/deployments/` don't change. + +**Verification for every PR**, run from the repo root after committing. It runs what CI runs, except coverage: + +```bash +pnpm install --frozen-lockfile +pnpm -r --sequential run build +pnpm test:scripts +pnpm check:bytecode +pnpm -r --sequential run test:self +pnpm lint:sol && git status --short +``` + +Expected: all exit 0, and `git status` prints nothing. + +## Review Focus + +1. **A formatter or linter rewrites a verified source.** `lint:sol`, the lint-staged commit hook and CI's Solidity lint must all leave `verified/` alone. Compiling can't detect this, because formatting doesn't change bytecode, so D2a compares source hashes after the commit. +2. **A changed verified source goes unnoticed.** Verification must fail, whether or not the code changes. D2a has a test for each case. +3. **An implementation on mainnet has been replaced since the addresses were listed.** `record-mainnet` must fail rather than hash a stale one. It reads each proxy's slot, and for StakingExtension, which has no proxy, L1Staking's `extensionImpl`. +4. **The alias changes what Solidity compiles.** Foundry's remapping auto-detection or Hardhat could pick up the alias's sources. D3a pins this with `check:bytecode` and the horizon and subgraph-service suites. +5. **Modules fail only when loaded.** Hardhat loads `tasks/deploy.ts` only in a built project, so a broken import can pass `build`. D3a and D3b run `npx hardhat --help` in both packages after the build. + +--- + +### Task D1: Delete `contracts-test` and the sdk + +**Files:** + +- Delete: `packages/contracts-test/` (with `rm -rf`, because `git rm` leaves `node_modules`, `artifacts` and `types`) +- Delete: `packages/contracts/.mocharc.json` +- Delete: `packages/contracts/scripts/ops/20240208-migrate-legacy-subgraphs/`, `packages/contracts/scripts/ops/testDisputeConflict/` (their only sdk users outside `contracts-test`; the package doesn't depend on the sdk, so they can't run today) +- Modify: `packages/contracts/package.json` (remove the `test` and `test:coverage` scripts, and `test/node_modules/` from `clean`) +- Modify: `packages/contracts/tsconfig.json` (remove the `test/**/*.ts` include), `packages/contracts/hardhat.config.ts` (remove `paths.tests`) +- Modify: `pnpm-workspace.yaml` (remove `'@graphprotocol/sdk'` from the catalog, and the whole `minumumReleaseAgeExclude` key, whose only entry it is) +- Modify: `.gitignore` (remove `packages/contracts-test/types/`) +- Modify: `pnpm-lock.yaml` + +**Interfaces:** + +- Consumes: nothing. +- Produces: no package runs `contracts-test`. `scripts/contracts-migration/coverage-baseline.json` and `summarize-coverage.mjs` stay, for the Phase 1 coverage gate. + +`packages/contracts/test/` no longer exists, and `build-test.yml` finds coverage files with `find`, so neither needs a change. Say so in the PR. + +- [ ] **Step 1: Confirm the deleted suite covers only dead code.** Run `pnpm --filter @graphprotocol/contracts-tests test:self`. Expected: `361 passing`, 0 failing. Run `git ls-files packages/contracts-test/tests`. Expected: only `curation/`, `disputes/`, `l2/l2Staking.test.ts`, `lib/`, `payments/`, `rewards/subgraphAvailability.test.ts`, `serviceRegisty.test.ts` and `staking/`. +- [ ] **Step 2: Make the deletions and edits above.** +- [ ] **Step 3: Update the lockfile.** Run `pnpm install --no-frozen-lockfile`. Expected: `@graphprotocol/sdk` no longer appears in `pnpm-lock.yaml`. +- [ ] **Step 4: Check no reference is left.** Run `git grep -nE "contracts-tests?\b|@graphprotocol/sdk" -- ':!docs/contracts-removal' ':!**/CHANGELOG.md' ':!scripts/contracts-migration'`. Expected: no output. +- [ ] **Step 5: Commit.** `chore: remove contracts-test and the sdk` +- [ ] **Step 6: Run the verification block.** Expected: `test:self` no longer runs `contracts-tests`, and `check:bytecode` prints `OK: 26 contracts match`. + +--- + +### Task D2a: Create `legacy` with verified bundles + +**Files:** + +- Create: `packages/legacy/package.json`, `packages/legacy/README.md` +- Create: `packages/legacy/scripts/lib.mjs`, `scripts/fetch-verified.mjs`, `scripts/record-mainnet.mjs`, `scripts/verify.mjs`, `scripts/lib.test.mjs` +- Create: `packages/legacy/targets.json`, `packages/legacy/mainnet.json` +- Create: `packages/legacy/verified//compiler.json`, `verified//sources/**` +- Modify: `.prettierignore`, `.solhintignore` (create it), both adding `packages/legacy/verified/` +- Modify: `scripts/lint-staged-run.sh` (add `"/packages/legacy/verified/"` to `IGNORE_SUBSTRINGS`), `package.json` (wrap both `*.sol` lint-staged commands in `scripts/lint-staged-run.sh`). Without this the commit hook fails: `solhint` exits 255 when every staged file is ignored +- Modify: `pnpm-lock.yaml` + +**Interfaces:** + +- Consumes: nothing. +- Produces: `pnpm --filter @graphprotocol/legacy test:self`, which later tasks rely on to prove the archive still reproduces mainnet. + +`package.json`: name `@graphprotocol/legacy`, `"private": true`, version `0.0.0`, license `GPL-2.0-or-later`, devDependencies exactly `"solc-0.7.4": "npm:solc@0.7.4"` and `"solc-0.7.6": "npm:solc@0.7.6"`, script `"test:self": "node --test 'scripts/*.test.mjs' && node scripts/verify.mjs"`. No `build` script. CI's `pnpm -r run test:self` is its CI job, so no workflow changes. + +`targets.json`, the implementations to archive: + +| Contract | Implementation | Proxy | solc | +| ------------------- | -------------------------------------------- | -------------------------------------------- | ----- | +| GraphToken | `0xc944E90C64B2c07662A292be6244BDf05Cda44a7` | none | 0.7.4 | +| L1GraphTokenGateway | `0xD41ca6A1d034D178c196DFa916f22f7D1a1B8222` | `0x01cDC91B0A9bA741903aA3699BF4CE31d6C5cC06` | 0.7.6 | +| BridgeEscrow | `0xBcD54513aa593646d72aEA31406c633C235Ad6EA` | `0x36aFF7001294daE4C2ED4fDEfC478a00De77F090` | 0.7.6 | +| L1GNS | `0x83A5B6C542aB20436087Fc79724bdF06ea80f836` | `0xaDcA0dd4729c8BA3aCf3E99F3A9f471EF37b6825` | 0.7.6 | +| Curation | `0xDeb46851907fd85DD475780CcE2eE0D67c969825` | `0x8FE00a685Bcb3B2cc296ff6FfEaB10acA4CE1538` | 0.7.6 | +| L1Staking | `0x20a14D78848BC8B3F3d4B77239F2adC3C0259A10` | `0xF55041E37E12cD407ad00CE2910B8269B01263b9` | 0.7.6 | +| StakingExtension | `0xA479c00cDa8C07bce458D7a826C7b091672EB92C` | none | 0.7.6 | + +Functions in `scripts/lib.mjs`: + +- `normalizeRuntime(hex: string, ranges: {start: number, length: number}[]): string`: lowercases, drops `0x`, zeroes each byte range, and removes the trailing CBOR metadata using the length in the last 2 bytes. +- `runtimeRanges(deployedBytecode): {start, length}[]`: every entry of `immutableReferences` and `linkReferences`. +- `compileBundle(dir: string): {object: string, ranges: {start, length}[]}`: builds a solc standard-JSON input from `compiler.json` and the files under `sources/`, with each path relative to `sources/` as the source key and `settings.outputSelection` set to `evm.deployedBytecode.{object,immutableReferences,linkReferences}`. It sets `language: 'Solidity'`, loads `solc-0.7.4` or `solc-0.7.6` (the version without `+commit...`) through `createRequire`, asserts `solc.version()` starts with the full version string, throws on any error-severity output, and returns the target's output. +- `runtimeHash(normalized: string): string`: sha256 hex. + +- `checkSources(dir: string): string[]`: compares the files under `sources/` with `sourceHashes` in `compiler.json` and returns each path that is missing, extra or changed. + +`compiler.json` keeps `version` (the full `0.7.6+commit.7338295f` string), `settings` (everything from Sourcify except `outputSelection`, including `evmVersion` and `libraries`), `target` (`{source, name}`, split from `fullyQualifiedName`), `sourcifyMatch` and `sourceHashes` (path to sha256). + +- [ ] **Step 1: Write the failing tests** in `scripts/lib.test.mjs`: + +```js +test('normalizeRuntime zeroes ranges and removes metadata', () => { + // 4 code bytes, then a 2-byte CBOR blob and its 2-byte length + assert.equal(normalizeRuntime('0xAABBCCDD' + 'eeff' + '0002', [{ start: 1, length: 2 }]), 'aa0000dd') +}) + +test('runtimeRanges reads immutable and link references', () => { + const ranges = runtimeRanges({ + immutableReferences: { 1: [{ start: 3, length: 32 }] }, + linkReferences: { 'a.sol': { Lib: [{ start: 40, length: 20 }] } }, + }) + assert.deepEqual(ranges, [ + { start: 3, length: 32 }, + { start: 40, length: 20 }, + ]) +}) + +test('verified BridgeEscrow matches mainnet.json', () => { + const { object, ranges } = compileBundle(BRIDGE_ESCROW_DIR) + assert.equal(runtimeHash(normalizeRuntime(object, ranges)), mainnet.BridgeEscrow.runtimeHash) +}) + +test('a code change no longer matches mainnet.json', () => { + // Copy the bundle to a temp dir and insert a function right after the target contract's opening brace + const { object, ranges } = compileBundle(copyOfBridgeEscrowWithFunctionInserted()) + assert.notEqual(runtimeHash(normalizeRuntime(object, ranges)), mainnet.BridgeEscrow.runtimeHash) +}) + +test('a comment-only change fails the source check', () => { + // Bytecode doesn't change here, so only the hashes can catch it + assert.deepEqual(checkSources(copyOfBridgeEscrowWithCommentAdded()), [TARGET_SOURCE_PATH]) +}) +``` + +- [ ] **Step 2: Run them.** `cd packages/legacy && node --test scripts/lib.test.mjs`. Expected: FAIL, with `lib.mjs` not implemented. +- [ ] **Step 3: Implement `scripts/lib.mjs`.** The first two tests pass. +- [ ] **Step 4: Implement and run `scripts/fetch-verified.mjs`.** For each target, it reads `https://sourcify.dev/server/v2/contract/1/?fields=stdJsonInput,compilation` and writes `compiler.json`, including `sourceHashes`, and each source's `content` byte for byte. Expected: 7 folders. `verified/GraphToken/sources/` has 1 file, and the file counts of the others are, in the table's order, 26, 18, 38, 33, 35 and 23. +- [ ] **Step 5: Implement and run `scripts/record-mainnet.mjs`.** It requires `MAINNET_RPC_URL` and exits 1 without it. For each target with a proxy, it reads the EIP-1967 slot `0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc` and exits 1 if it isn't the target's implementation (Review Focus 3). For StakingExtension, it reads `extensionImpl` on the L1Staking proxy, with the slot taken from L1Staking's compiler `storageLayout`, and exits 1 if it isn't the target. It then calls `eth_getCode`, normalizes with the ranges from `compileBundle`, and writes `mainnet.json` as `{ : { implementation, proxy, runtimeHash } }`. Run `MAINNET_RPC_URL= node scripts/record-mainnet.mjs`. Any public mainnet RPC works, because the data is public. +- [ ] **Step 6: Implement `scripts/verify.mjs`.** For each folder under `verified/` it runs `checkSources`, then compiles, normalizes and compares with `mainnet.json`. It prints each mismatch and exits 1 on any, and fails if `verified/` and `mainnet.json` don't list the same contracts. On success it prints `OK: 7 mainnet implementations reproduce`. +- [ ] **Step 7: Run it.** `pnpm --filter @graphprotocol/legacy test:self`. Expected: all tests pass and the `OK` line prints. +- [ ] **Step 8: Add the ignore entries,** and write `README.md`: what the archive is, the table above, that current source reproduces only Curation, that GraphToken and L1Staking are Sourcify partial matches so their sources may differ from the deployed ones outside the code, how to re-run the fetch and record scripts, and that the sources must stay unformatted. +- [ ] **Step 9: Commit,** with the hook enabled so lint-staged runs (never `--no-verify`). `feat(legacy): archive verified mainnet implementations` +- [ ] **Step 10: Prove the hook and linters left the sources alone.** Run `pnpm --filter @graphprotocol/legacy test:self`, then the verification block. Expected: the `OK` line prints, so every source hash still matches, and `git status` stays empty. If the hook reformatted a source, fix the ignore file and re-fetch before amending. + +--- + +### Task D2b: Add the legacy address book and audits + +**Files:** + +- Create: `packages/legacy/addresses.json` +- Create: `packages/address-book/src/legacy/addresses.json` +- Move: `packages/contracts/audits/` to `packages/legacy/audits/` (`git mv`) +- Modify: `packages/address-book/scripts/sources.js` (add `'legacy'`, and correct its comment), `packages/address-book/README.md` (the sources are regular files, not symlinks) +- Create: a changeset for `@graphprotocol/address-book`, which gains a published export +- Modify: `packages/legacy/README.md` + +**Interfaces:** + +- Consumes: the `legacy` package from D2a. +- Produces: `@graphprotocol/address-book` exposes `src/legacy/addresses.json`. + +`legacy/addresses.json` holds the `1` and `11155111` entries of `packages/contracts/addresses.json`, unchanged and complete, including Controller, EpochManager, RewardsManager, DisputeManager, ServiceRegistry, GraphProxyAdmin and SubgraphNFT. Copy `packages/legacy/addresses.json` to `packages/address-book/src/legacy/addresses.json` as a regular file, like the other four (mode 100644, byte-identical to their sources). Don't run `restore-symlinks`. + +Adding GraphCurationToken and SubgraphNFTDescriptor to `horizon/addresses.json` is Phase 1 step 3, not this task. + +- [ ] **Step 1: Generate `addresses.json`** with a one-off Node command, then compare: `node -e` that deep-equals `addresses.json['1']` and `['11155111']` with `packages/contracts/addresses.json`. Expected: equal, and the top-level keys are exactly `1` and `11155111`. +- [ ] **Step 2: Make the other changes.** +- [ ] **Step 3: Check the audits moved.** Run `ls packages/legacy/audits | wc -l` and compare with the count before the move. Run `git grep -n "contracts/audits" -- ':!**/CHANGELOG.md' ':!docs/contracts-removal'`, and update any link it finds. +- [ ] **Step 4: Commit.** `feat(legacy): add address book and historical audits` +- [ ] **Step 5: Run the verification block.** + +--- + +### Task D3a: Point Ignition at the 7.3.0 alias + +**Files:** + +- Modify: `packages/horizon/package.json`, `packages/subgraph-service/package.json` (add the devDependency `"@graphprotocol/contracts-7.3.0": "npm:@graphprotocol/contracts@7.3.0"`; keep the `@graphprotocol/contracts` workspace dependency) +- Modify: the 15 import-holding files under `ignition/modules/`: in horizon `core/HorizonStaking.ts`, `migrate/migrate-4.ts`, `proxy/GraphProxy.ts` and `periphery/{Controller,Curation,EpochManager,GNS,GraphProxyAdmin,GraphToken,GraphTokenGateway,RewardsManager}.ts`; in subgraph-service `Curation.ts`, `DisputeManager.ts`, `GNS.ts`, `ServiceRegistry.ts` +- Modify: `packages/horizon/scripts/pre-verify`, `packages/subgraph-service/scripts/pre-verify`, `packages/horizon/tenderly.config.json` (`../contracts/artifacts` becomes `node_modules/@graphprotocol/contracts-7.3.0/artifacts`; the tarball ships `artifacts/build-info`) +- Modify: `pnpm-lock.yaml` + +**Interfaces:** + +- Consumes: nothing. +- Produces: the modules and their verification tooling no longer read artifacts from the workspace package, so D3b and Phase 1 can remove them. + +- [ ] **Step 1: Add the dependency,** and run `pnpm install --no-frozen-lockfile`. +- [ ] **Step 2: Replace the package name.** First run `git grep -l "@graphprotocol/contracts/artifacts" -- 'packages/*/ignition/modules/**' | wc -l`. Expected: `15`. In the listed files only, `@graphprotocol/contracts/artifacts/` becomes `@graphprotocol/contracts-7.3.0/artifacts/`. +- [ ] **Step 3: Check the diff is only that.** Run `git diff -- 'packages/*/ignition/**' | grep -E '^[-+][^-+]' | grep -v "@graphprotocol/contracts"`. Expected: no output. Run `git diff --stat -- 'packages/*/ignition/deployments/**'`. Expected: no output. Run `git grep -n "@graphprotocol/contracts/artifacts" -- 'packages/*/ignition/modules/**'`. Expected: no output. +- [ ] **Step 4: Commit.** `chore(ignition): read deployment artifacts from published contracts 7.3.0` +- [ ] **Step 5: Run the verification block.** Expected: `check:bytecode` still prints `OK: 26 contracts match`, and horizon's and subgraph-service's suites pass unchanged (Review Focus 4). +- [ ] **Step 6: Load the modules.** Run `npx hardhat --help` and `npx tsc --noEmit -p .` in `packages/horizon` and `packages/subgraph-service`. Expected: `--help` exits 0 in both, and `tsc` reports no error mentioning `contracts-7.3.0`. + +--- + +### Task D3b: Delete the dead contracts + +**Files:** + +- Delete, under `packages/contracts/contracts/`: + - `discovery/L1GNS.sol`, `discovery/L1GNSStorage.sol`, `curation/Curation.sol`, `bancor/BancorFormula.sol` + - `staking/Staking.sol`, `staking/L1Staking.sol`, `staking/L1StakingStorage.sol`, `staking/StakingExtension.sol`, `staking/StakingStorage.sol`, `staking/libs/Stakes.sol`, `staking/libs/Exponential.sol`, `staking/libs/LibFixedMath.sol`, `l2/staking/L2Staking.sol` + - `disputes/DisputeManager.sol`, `disputes/DisputeManagerStorage.sol`, `discovery/ServiceRegistry.sol`, `discovery/ServiceRegistryStorage.sol` + - `payments/AllocationExchange.sol`, `discovery/erc1056/EthereumDIDRegistry.sol`, `rewards/SubgraphAvailabilityManager.sol` + - `tests/arbitrum/ArbSysMock.sol`, `tests/ens/IENS.sol`, `tests/ens/IPublicResolver.sol`, `tests/ens/ITestRegistrar.sol`, `tests/LegacyGNSMock.sol`, `tests/MockERC165.sol`, `tests/MockSubgraphService.sol`, `tests/L1GraphTokenLockTransferToolMock.sol`, `tests/L1GraphTokenLockTransferToolBadMock.sol` +- Delete: `packages/contracts/addresses-staging.json`, `packages/contracts/arbitrum-addresses.json`, `packages/contracts/config/` (`addresses-local.json` is not tracked) +- Modify: `packages/contracts/index.js`, `packages/contracts/index.d.ts` (remove `configDir`) +- Modify: `packages/deployment/test/bytecode-comparison.test.ts:235,239` (load `('l2/curation', 'L2Curation')` and `('l2/token', 'L2GraphToken')` instead of the deleted Staking and Curation) +- Modify: `packages/contracts/hardhat.config.ts` (remove the `StakingExtension` warnings entry) +- Modify: `scripts/contracts-migration/manifest.json`, `scripts/contracts-migration/reference.json` (remove `L1GNS`, `Curation`, `L1Staking`, `StakingExtension`, `LibExponential`) + +**Interfaces:** + +- Consumes: `legacy` from D2a, and the Ignition alias from D3a. +- Produces: `packages/contracts` holds only live contracts and the mocks ported tests use: 45 Solidity files. Other leftovers that point at deleted things (`.env.example`, `scripts/upgrade`) stay until Phase 2a deletes the package. + +Edit `reference.json` by removing those 5 keys. Don't regenerate it with `--write`: leaving the other 21 hashes untouched is what shows the deletions changed no live bytecode. + +- [ ] **Step 1: Make the deletions and edits.** +- [ ] **Step 2: Rebuild from clean.** Run `pnpm --filter @graphprotocol/contracts clean && pnpm --filter @graphprotocol/contracts build`. Expected: exit 0. A stale artifact mustn't hide a broken import. +- [ ] **Step 3: Commit.** `refactor(contracts): delete dead contracts now archived in legacy` +- [ ] **Step 4: Run the verification block.** Expected: `check:bytecode` prints `OK: 21 contracts match`, and `test:self` includes `OK: 7 mainnet implementations reproduce`. +- [ ] **Step 5: Count what's left.** Run `git ls-files 'packages/contracts/contracts/*.sol' | wc -l`. Expected: `45`. +- [ ] **Step 6: Load the modules.** Run `npx hardhat --help` in `packages/horizon` and `packages/subgraph-service`. Expected: exit 0 (Review Focus 5). +- [ ] **Step 7: Check nothing else consumed them.** Run `git grep -nE "@graphprotocol/contracts/(artifacts/)?contracts/(staking/(Staking|L1Staking|StakingExtension|StakingStorage)|disputes/|payments/|curation/Curation\.|bancor/|discovery/(L1GNS|ServiceRegistry))" -- ':!**/CHANGELOG.md' ':!docs/contracts-removal'` and `git grep -nE "loadContractsArtifact\('(staking|curation|disputes|bancor|payments)'" -- packages/deployment`. Expected: no output. From 1e4f50f5d0d564a07df7c6e4b7b4738d22a14024 Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Fri, 30 Oct 2026 12:18:38 -0300 Subject: [PATCH 10/11] docs: apply phase D plan review fixes --- docs/contracts-removal/design.md | 22 +++++----- docs/contracts-removal/phase-d-plan.md | 56 ++++++++++++++------------ 2 files changed, 43 insertions(+), 35 deletions(-) diff --git a/docs/contracts-removal/design.md b/docs/contracts-removal/design.md index c74411325..7f0e2c071 100644 --- a/docs/contracts-removal/design.md +++ b/docs/contracts-removal/design.md @@ -63,7 +63,7 @@ Deleted, with no replacement: - Staking: L2Staking. The Arbitrum proxy now runs HorizonStaking. - AllocationExchange, EthereumDIDRegistry, SubgraphAvailabilityManager. - The Arbitrum DisputeManager and ServiceRegistry, along with their storage contracts. -- Mocks under `contracts/tests/` that no ported test needs: ArbSysMock, `tests/ens/*`, LegacyGNSMock, MockERC165, MockSubgraphService, and both L1GraphTokenLockTransferTool mocks. +- Mocks under `contracts/tests/` that no ported test needs: ArbSysMock, `tests/ens/*`, LegacyGNSMock, MockERC165, MockIssuanceAllocator, MockRewardsEligibilityOracle, MockSubgraphService, and both L1GraphTokenLockTransferTool mocks. - Each mainnet-only contract is deleted from `packages/contracts` only after its verified source exists in `legacy`. #### legacy package @@ -166,7 +166,7 @@ Notes: - Today `l2GNS.test.ts` stakes and allocates through L2Staking. Its port uses a staking mock instead. - `controller.test.ts`, `pausing.test.ts` and `admin.test.ts` use EpochManager, Staking and Curation. Their ports use Managed or GraphUpgradeable mocks instead. - `l2GraphTokenGateway.test.ts` uses RewardsManager. Its port uses a plain callhook receiver mock. -- `lib/gnsUtils.ts` is also imported by `gns.test.ts`, which is dropped but stays until D1. T3 deletes `gns.test.ts` together with `gnsUtils.ts`. +- `lib/gnsUtils.ts` is also imported by `gns.test.ts`, which is dropped. T3 deletes `gns.test.ts` together with `gnsUtils.ts`. Dropped, because their contracts are deleted or legacy: @@ -176,11 +176,11 @@ Dropped, because their contracts are deleted or legacy: #### Mocks needed by ported tests -| Mock | Destination | -| ------------------------------------------------------------------------------------ | --------------------------------------------------------------------------------------- | -| GovernedMock, ManagedMock (new, 0.7.6) | graph-core | -| BridgeMock, InboxMock, OutboxMock, CallhookReceiverMock, TokenUtilsMock (new, 0.7.6) | graph-token | -| MockIssuanceAllocator, MockRewardsEligibilityOracle | issuance (reuse its existing 0.8 mocks where they exist; otherwise rewrite them in 0.8) | +| Mock | Destination | +| ------------------------------------------------------------------------------------ | -------------------- | +| GovernedMock, ManagedMock (new, 0.7.6) | graph-core | +| BridgeMock, InboxMock, OutboxMock, CallhookReceiverMock, TokenUtilsMock (new, 0.7.6) | graph-token | +| New 0.8 test mocks for the issuance allocator, eligibility oracle and rewards issuer | issuance's test tree | ManagedMock and TokenUtilsMock are added to `packages/contracts/contracts/tests/` in Phase T, because 0.8 tests can't compile a Managed contract or call the TokenUtils library. They move with their packages in Phase 1. ArbSysMock is not used (ArbSys is faked with `vm.mockCall`), so it stays on the delete list. @@ -192,7 +192,7 @@ Every test-port PR includes a table mapping each old test case to its new Foundr Ignition deployed Horizon, but new deployments go through `packages/deployment`. The Ignition modules and journals stay as the record of that deployment, with one mechanical change to their imports. -- horizon and subgraph-service import 14 artifacts in 15 module files from `@graphprotocol/contracts/artifacts/...`. Each package's `tasks/deploy.ts` imports the modules, and `loadTasks` loads it in every built Hardhat run. Both tsconfigs also include `ignition/**/*.ts`. The imports must therefore keep resolving after the workspace package loses those artifacts. +- horizon and subgraph-service import 14 artifacts in 15 module files from `@graphprotocol/contracts/artifacts/...`. Each package's `tasks/deploy.ts` imports the modules (`migrate/*` dynamically), and `loadTasks` loads it in every built Hardhat run. Both tsconfigs also include `ignition/**/*.ts`. The imports must therefore keep resolving after the workspace package loses those artifacts. - Both packages can't simply pin `@graphprotocol/contracts` to 7.3.0. Their Solidity imports from it still need the workspace version until Phase 1: the published sources use the pragma `^0.7.6 || 0.8.27`, and these packages compile with 0.8.35. - Before D3, both packages add the devDependency `"@graphprotocol/contracts-7.3.0": "npm:@graphprotocol/contracts@7.3.0"`, and the module imports change only the package name to `@graphprotocol/contracts-7.3.0`. The tarball ships every imported artifact. Journals and module logic don't change. - Their `@graphprotocol/contracts` workspace dependency stays for Solidity imports and is removed in Phase 1 as usual. @@ -225,7 +225,7 @@ Ignition deployed Horizon, but new deployments go through `packages/deployment`. - The root `lint:yaml` drops `packages/contracts/task/config`. - `count-patterns.txt` paths are updated. - `@graphprotocol/sdk` is removed from the `pnpm-workspace.yaml` catalog and from `minumumReleaseAgeExclude` (the key's real, misspelled name). -- `build-test.yml` coverage collection is updated for the removed packages and the new ones. +- `build-test.yml` finds coverage files itself, so it needs no change for the removed packages or the new ones. - The Docker image (`Dockerfile`, `publish-image.yml`) is checked for use of `packages/contracts` or the sdk before deletion. #### token-distribution @@ -320,7 +320,9 @@ The detailed task split is in `phase-t-plan.md`. Live files that also appear in `legacy` bundles stay: Multicall, GNS, GNSStorage, CurationStorage, Managed, GraphUpgradeable, TokenUtils and MathUtils. Nothing that remains imports a deleted file. A PR just before it adds the Ignition alias (§5). L1GNS, Curation, L1Staking, StakingExtension and LibExponential leave the 0c manifest, because `legacy` now checks them. -After D3, `packages/contracts` contains only live contracts. +After D3, `packages/contracts` contains only live contracts and the mocks ported tests use. + +Phase D moves nothing and leaves the live bytecode hashes unchanged, so the fork check under Verification starts with Phase 1. ### Phase 1: moves, bottom-up (one PR each, `git mv` to keep history) diff --git a/docs/contracts-removal/phase-d-plan.md b/docs/contracts-removal/phase-d-plan.md index 5f0b83ac5..b8929ac21 100644 --- a/docs/contracts-removal/phase-d-plan.md +++ b/docs/contracts-removal/phase-d-plan.md @@ -22,10 +22,10 @@ - No RPC URL or API key is ever written to a file, commit, report or PR. RPC URLs are passed only as inline env vars. - D3b's list comes from the spec. Do not delete any other contract. - `packages/legacy` never gets a `.solhint.json`: CI's changed-files mode runs solhint from that package and passes only because it loads no rules there. -- D1 starts after #1387 merges into the base branch, so the stack starts from the integration tip. +- The spec's Arbitrum Sepolia fork check for `packages/deployment` doesn't run in this phase: nothing moves, and the unchanged bytecode hashes show no live bytecode changed. It starts with Phase 1. - Ignition module changes are limited to the package name in import lines. Journals under `ignition/deployments/` don't change. -**Verification for every PR**, run from the repo root after committing. It runs what CI runs, except coverage: +**Verification for every PR**, run from the repo root after committing. It runs CI's build and test steps, except coverage, and doesn't replace CI's lint job (`lint.yml`): ```bash pnpm install --frozen-lockfile @@ -36,7 +36,7 @@ pnpm -r --sequential run test:self pnpm lint:sol && git status --short ``` -Expected: all exit 0, and `git status` prints nothing. +Expected: all exit 0, and `git status` prints nothing. `lint:sol`'s exit code only reflects its final `pnpm todo`, so `git status` is the real check on that line: nothing was reformatted. ## Review Focus @@ -44,7 +44,7 @@ Expected: all exit 0, and `git status` prints nothing. 2. **A changed verified source goes unnoticed.** Verification must fail, whether or not the code changes. D2a has a test for each case. 3. **An implementation on mainnet has been replaced since the addresses were listed.** `record-mainnet` must fail rather than hash a stale one. It reads each proxy's slot, and for StakingExtension, which has no proxy, L1Staking's `extensionImpl`. 4. **The alias changes what Solidity compiles.** Foundry's remapping auto-detection or Hardhat could pick up the alias's sources. D3a pins this with `check:bytecode` and the horizon and subgraph-service suites. -5. **Modules fail only when loaded.** Hardhat loads `tasks/deploy.ts` only in a built project, so a broken import can pass `build`. D3a and D3b run `npx hardhat --help` in both packages after the build. +5. **Modules fail only when loaded.** Hardhat loads `tasks/deploy.ts` only in a built project, so a broken import can pass `build`. `npx hardhat --help` catches most modules, but `migrate/*` and subgraph-service's `ServiceRegistry.ts` load dynamically, so D3a also runs `tsc`, which catches those. --- @@ -68,7 +68,7 @@ Expected: all exit 0, and `git status` prints nothing. `packages/contracts/test/` no longer exists, and `build-test.yml` finds coverage files with `find`, so neither needs a change. Say so in the PR. -- [ ] **Step 1: Confirm the deleted suite covers only dead code.** Run `pnpm --filter @graphprotocol/contracts-tests test:self`. Expected: `361 passing`, 0 failing. Run `git ls-files packages/contracts-test/tests`. Expected: only `curation/`, `disputes/`, `l2/l2Staking.test.ts`, `lib/`, `payments/`, `rewards/subgraphAvailability.test.ts`, `serviceRegisty.test.ts` and `staking/`. +- [ ] **Step 1: Confirm the deleted suite covers only dead code.** Run `pnpm -r --sequential run build`, then `pnpm --filter @graphprotocol/contracts-tests test:self`. Expected: `361 passing`, 0 failing. Run `git ls-files packages/contracts-test/tests`. Expected: only `curation/`, `disputes/`, `l2/l2Staking.test.ts`, `lib/`, `payments/`, `rewards/subgraphAvailability.test.ts`, `serviceRegisty.test.ts` and `staking/`. - [ ] **Step 2: Make the deletions and edits above.** - [ ] **Step 3: Update the lockfile.** Run `pnpm install --no-frozen-lockfile`. Expected: `@graphprotocol/sdk` no longer appears in `pnpm-lock.yaml`. - [ ] **Step 4: Check no reference is left.** Run `git grep -nE "contracts-tests?\b|@graphprotocol/sdk" -- ':!docs/contracts-removal' ':!**/CHANGELOG.md' ':!scripts/contracts-migration'`. Expected: no output. @@ -86,6 +86,7 @@ Expected: all exit 0, and `git status` prints nothing. - Create: `packages/legacy/targets.json`, `packages/legacy/mainnet.json` - Create: `packages/legacy/verified//compiler.json`, `verified//sources/**` - Modify: `.prettierignore`, `.solhintignore` (create it), both adding `packages/legacy/verified/` +- Create: `.gitattributes` with `packages/legacy/verified/** -text`. `GraphToken.sol` has CRLF line endings, and with `core.autocrlf=input` git would rewrite it on commit and fail its source hash - Modify: `scripts/lint-staged-run.sh` (add `"/packages/legacy/verified/"` to `IGNORE_SUBSTRINGS`), `package.json` (wrap both `*.sol` lint-staged commands in `scripts/lint-staged-run.sh`). Without this the commit hook fails: `solhint` exits 255 when every staged file is ignored - Modify: `pnpm-lock.yaml` @@ -96,7 +97,7 @@ Expected: all exit 0, and `git status` prints nothing. `package.json`: name `@graphprotocol/legacy`, `"private": true`, version `0.0.0`, license `GPL-2.0-or-later`, devDependencies exactly `"solc-0.7.4": "npm:solc@0.7.4"` and `"solc-0.7.6": "npm:solc@0.7.6"`, script `"test:self": "node --test 'scripts/*.test.mjs' && node scripts/verify.mjs"`. No `build` script. CI's `pnpm -r run test:self` is its CI job, so no workflow changes. -`targets.json`, the implementations to archive: +`targets.json`, the implementations to archive, as `{ "": { "implementation": "0x...", "proxy": "0x..." | null } }`: | Contract | Implementation | Proxy | solc | | ------------------- | -------------------------------------------- | -------------------------------------------- | ----- | @@ -112,12 +113,12 @@ Functions in `scripts/lib.mjs`: - `normalizeRuntime(hex: string, ranges: {start: number, length: number}[]): string`: lowercases, drops `0x`, zeroes each byte range, and removes the trailing CBOR metadata using the length in the last 2 bytes. - `runtimeRanges(deployedBytecode): {start, length}[]`: every entry of `immutableReferences` and `linkReferences`. -- `compileBundle(dir: string): {object: string, ranges: {start, length}[]}`: builds a solc standard-JSON input from `compiler.json` and the files under `sources/`, with each path relative to `sources/` as the source key and `settings.outputSelection` set to `evm.deployedBytecode.{object,immutableReferences,linkReferences}`. It sets `language: 'Solidity'`, loads `solc-0.7.4` or `solc-0.7.6` (the version without `+commit...`) through `createRequire`, asserts `solc.version()` starts with the full version string, throws on any error-severity output, and returns the target's output. +- `compileBundle(dir: string): {object: string, ranges: {start, length}[], storageLayout}`: builds a solc standard-JSON input from `compiler.json` and the files under `sources/`, with each path relative to `sources/` as the source key and `settings.outputSelection` set to `evm.deployedBytecode.{object,immutableReferences,linkReferences}` and `storageLayout`. It sets `language: 'Solidity'`, loads `solc-0.7.4` or `solc-0.7.6` (the version without `+commit...`) through `createRequire`, asserts `solc.version()` starts with the full version string, throws on any error-severity output, and returns the target's output. - `runtimeHash(normalized: string): string`: sha256 hex. - `checkSources(dir: string): string[]`: compares the files under `sources/` with `sourceHashes` in `compiler.json` and returns each path that is missing, extra or changed. -`compiler.json` keeps `version` (the full `0.7.6+commit.7338295f` string), `settings` (everything from Sourcify except `outputSelection`, including `evmVersion` and `libraries`), `target` (`{source, name}`, split from `fullyQualifiedName`), `sourcifyMatch` and `sourceHashes` (path to sha256). +`compiler.json` keeps `version` (the full `0.7.6+commit.7338295f` string), `settings` (everything from Sourcify except `outputSelection`, including `evmVersion` and `libraries`), `target` (`{source, name}`, split from `fullyQualifiedName`), `sourcifyMatch` (Sourcify's `match` field: a partial match reads `match`, an exact one `exact_match`) and `sourceHashes` (path to sha256). - [ ] **Step 1: Write the failing tests** in `scripts/lib.test.mjs`: @@ -155,13 +156,15 @@ test('a comment-only change fails the source check', () => { }) ``` -- [ ] **Step 2: Run them.** `cd packages/legacy && node --test scripts/lib.test.mjs`. Expected: FAIL, with `lib.mjs` not implemented. +Read `mainnet.json` inside the tests that use it, because it doesn't exist until Step 5. + +- [ ] **Step 2: Run them.** Create `package.json`, run `pnpm install --no-frozen-lockfile` to install the solc aliases, then `cd packages/legacy && node --test scripts/lib.test.mjs`. Expected: FAIL, with `lib.mjs` not implemented. - [ ] **Step 3: Implement `scripts/lib.mjs`.** The first two tests pass. -- [ ] **Step 4: Implement and run `scripts/fetch-verified.mjs`.** For each target, it reads `https://sourcify.dev/server/v2/contract/1/?fields=stdJsonInput,compilation` and writes `compiler.json`, including `sourceHashes`, and each source's `content` byte for byte. Expected: 7 folders. `verified/GraphToken/sources/` has 1 file, and the file counts of the others are, in the table's order, 26, 18, 38, 33, 35 and 23. -- [ ] **Step 5: Implement and run `scripts/record-mainnet.mjs`.** It requires `MAINNET_RPC_URL` and exits 1 without it. For each target with a proxy, it reads the EIP-1967 slot `0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc` and exits 1 if it isn't the target's implementation (Review Focus 3). For StakingExtension, it reads `extensionImpl` on the L1Staking proxy, with the slot taken from L1Staking's compiler `storageLayout`, and exits 1 if it isn't the target. It then calls `eth_getCode`, normalizes with the ranges from `compileBundle`, and writes `mainnet.json` as `{ : { implementation, proxy, runtimeHash } }`. Run `MAINNET_RPC_URL= node scripts/record-mainnet.mjs`. Any public mainnet RPC works, because the data is public. -- [ ] **Step 6: Implement `scripts/verify.mjs`.** For each folder under `verified/` it runs `checkSources`, then compiles, normalizes and compares with `mainnet.json`. It prints each mismatch and exits 1 on any, and fails if `verified/` and `mainnet.json` don't list the same contracts. On success it prints `OK: 7 mainnet implementations reproduce`. +- [ ] **Step 4: Implement and run `scripts/fetch-verified.mjs`.** For each target, it reads `https://sourcify.dev/server/v2/contract/1/?fields=stdJsonInput,compilation` and writes `compiler.json`, including `sourceHashes`, and each source's `content` byte for byte. It reads `stdJsonInput.{sources,settings}`, `compilation.{compilerVersion,fullyQualifiedName}` and `match`. Expected: 7 folders. `verified/GraphToken/sources/` has 1 file, and the file counts of the others are, in the table's order, 26, 18, 38, 33, 35 and 23. +- [ ] **Step 5: Implement and run `scripts/record-mainnet.mjs`.** It requires `MAINNET_RPC_URL` and exits 1 without it. For each target with a proxy, it reads the EIP-1967 slot `0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc` and exits 1 if it isn't the target's implementation (Review Focus 3). For StakingExtension, it reads `extensionImpl` on the L1Staking proxy, with the slot taken from L1Staking's compiler `storageLayout`, and exits 1 if the slot's low 20 bytes aren't the target (the slot is packed with other values). It reads everything at one pinned block and records that block in `mainnet.json`. It then calls `eth_getCode`, normalizes with the ranges from `compileBundle`, and writes `mainnet.json` as `{ "block": , "contracts": { : { implementation, proxy, runtimeHash } } }`. Run `MAINNET_RPC_URL= node scripts/record-mainnet.mjs`. Any public mainnet RPC works, because the data is public. +- [ ] **Step 6: Implement `scripts/verify.mjs`.** For each folder under `verified/` it runs `checkSources`, then compiles, normalizes and compares with `mainnet.json`. It prints each mismatch and exits 1 on any, and fails if `targets.json`, `verified/` and `mainnet.json` don't list the same contracts. On success it prints the real count: `OK: 7 mainnet implementations reproduce`. - [ ] **Step 7: Run it.** `pnpm --filter @graphprotocol/legacy test:self`. Expected: all tests pass and the `OK` line prints. -- [ ] **Step 8: Add the ignore entries,** and write `README.md`: what the archive is, the table above, that current source reproduces only Curation, that GraphToken and L1Staking are Sourcify partial matches so their sources may differ from the deployed ones outside the code, how to re-run the fetch and record scripts, and that the sources must stay unformatted. +- [ ] **Step 8: Add the ignore entries and `.gitattributes`,** run `npx eslint packages/legacy/scripts` by hand (CI's changed-files filter skips paths containing `lib`), and write `README.md`: what the archive is, the table above, that current source reproduces only Curation, that GraphToken and L1Staking are Sourcify partial matches so their sources may differ from the deployed ones outside the code, how to re-run the fetch and record scripts, and that the sources must stay unformatted. - [ ] **Step 9: Commit,** with the hook enabled so lint-staged runs (never `--no-verify`). `feat(legacy): archive verified mainnet implementations` - [ ] **Step 10: Prove the hook and linters left the sources alone.** Run `pnpm --filter @graphprotocol/legacy test:self`, then the verification block. Expected: the `OK` line prints, so every source hash still matches, and `git status` stays empty. If the hook reformatted a source, fix the ignore file and re-fetch before amending. @@ -175,7 +178,7 @@ test('a comment-only change fails the source check', () => { - Create: `packages/address-book/src/legacy/addresses.json` - Move: `packages/contracts/audits/` to `packages/legacy/audits/` (`git mv`) - Modify: `packages/address-book/scripts/sources.js` (add `'legacy'`, and correct its comment), `packages/address-book/README.md` (the sources are regular files, not symlinks) -- Create: a changeset for `@graphprotocol/address-book`, which gains a published export +- Create: a minor changeset for `@graphprotocol/address-book`, which gains a published export - Modify: `packages/legacy/README.md` **Interfaces:** @@ -183,13 +186,13 @@ test('a comment-only change fails the source check', () => { - Consumes: the `legacy` package from D2a. - Produces: `@graphprotocol/address-book` exposes `src/legacy/addresses.json`. -`legacy/addresses.json` holds the `1` and `11155111` entries of `packages/contracts/addresses.json`, unchanged and complete, including Controller, EpochManager, RewardsManager, DisputeManager, ServiceRegistry, GraphProxyAdmin and SubgraphNFT. Copy `packages/legacy/addresses.json` to `packages/address-book/src/legacy/addresses.json` as a regular file, like the other four (mode 100644, byte-identical to their sources). Don't run `restore-symlinks`. +`legacy/addresses.json` holds the `1` and `11155111` entries of `packages/contracts/addresses.json`, unchanged and complete, including Controller, EpochManager, RewardsManager, DisputeManager, ServiceRegistry, GraphProxyAdmin and SubgraphNFT. Copy `packages/legacy/addresses.json` to `packages/address-book/src/legacy/addresses.json` as a regular file, like the other four (mode 100644, byte-identical to their sources). `postpublish` still runs `restore-symlinks`, so the README should say the sources are tracked as regular files and become symlinks only after a publish, and its Features and usage sections should list `legacy`. Don't run `restore-symlinks`. Adding GraphCurationToken and SubgraphNFTDescriptor to `horizon/addresses.json` is Phase 1 step 3, not this task. -- [ ] **Step 1: Generate `addresses.json`** with a one-off Node command, then compare: `node -e` that deep-equals `addresses.json['1']` and `['11155111']` with `packages/contracts/addresses.json`. Expected: equal, and the top-level keys are exactly `1` and `11155111`. +- [ ] **Step 1: Generate `addresses.json`** with a one-off Node command, then compare: `node -e` that deep-equals `addresses.json['1']` and `['11155111']` with `packages/contracts/addresses.json`. Expected: equal, and the top-level keys are exactly `1` and `11155111`. Run prettier on both copies, then `cmp packages/legacy/addresses.json packages/address-book/src/legacy/addresses.json`. Expected: identical. - [ ] **Step 2: Make the other changes.** -- [ ] **Step 3: Check the audits moved.** Run `ls packages/legacy/audits | wc -l` and compare with the count before the move. Run `git grep -n "contracts/audits" -- ':!**/CHANGELOG.md' ':!docs/contracts-removal'`, and update any link it finds. +- [ ] **Step 3: Check the audits moved.** Run `git ls-files packages/legacy/audits | wc -l`. Expected: `29`. Run `git grep -n "contracts/audits" -- ':!**/CHANGELOG.md' ':!docs/contracts-removal'`, and update any link it finds. - [ ] **Step 4: Commit.** `feat(legacy): add address book and historical audits` - [ ] **Step 5: Run the verification block.** @@ -212,9 +215,11 @@ Adding GraphCurationToken and SubgraphNFTDescriptor to `horizon/addresses.json` - [ ] **Step 1: Add the dependency,** and run `pnpm install --no-frozen-lockfile`. - [ ] **Step 2: Replace the package name.** First run `git grep -l "@graphprotocol/contracts/artifacts" -- 'packages/*/ignition/modules/**' | wc -l`. Expected: `15`. In the listed files only, `@graphprotocol/contracts/artifacts/` becomes `@graphprotocol/contracts-7.3.0/artifacts/`. - [ ] **Step 3: Check the diff is only that.** Run `git diff -- 'packages/*/ignition/**' | grep -E '^[-+][^-+]' | grep -v "@graphprotocol/contracts"`. Expected: no output. Run `git diff --stat -- 'packages/*/ignition/deployments/**'`. Expected: no output. Run `git grep -n "@graphprotocol/contracts/artifacts" -- 'packages/*/ignition/modules/**'`. Expected: no output. -- [ ] **Step 4: Commit.** `chore(ignition): read deployment artifacts from published contracts 7.3.0` -- [ ] **Step 5: Run the verification block.** Expected: `check:bytecode` still prints `OK: 26 contracts match`, and horizon's and subgraph-service's suites pass unchanged (Review Focus 4). -- [ ] **Step 6: Load the modules.** Run `npx hardhat --help` and `npx tsc --noEmit -p .` in `packages/horizon` and `packages/subgraph-service`. Expected: `--help` exits 0 in both, and `tsc` reports no error mentioning `contracts-7.3.0`. +- [ ] **Step 4: Point the verification tooling at the alias.** Edit both `pre-verify` scripts and `tenderly.config.json` as listed under Files. CI never runs them, so check them here. Run `git grep -n '\.\./contracts/artifacts' -- packages/horizon packages/subgraph-service`. Expected: no output. +- [ ] **Step 5: Commit.** `chore(ignition): read deployment artifacts from published contracts 7.3.0` +- [ ] **Step 6: Run the verification block.** Expected: `check:bytecode` still prints `OK: 26 contracts match`, and horizon's and subgraph-service's suites pass unchanged (Review Focus 4). +- [ ] **Step 7: Run `pre-verify`.** In `packages/horizon` and `packages/subgraph-service`, run `bash scripts/pre-verify review-tmp && rm -rf ignition/deployments/review-tmp`. Expected: exit 0, and `git status` stays empty. +- [ ] **Step 8: Load the modules.** Run `npx hardhat --help` and `npx tsc --noEmit -p .` in `packages/horizon` and `packages/subgraph-service`. Expected: `--help` exits 0 in both, and `tsc` reports no error mentioning `contracts-7.3.0`. --- @@ -227,17 +232,18 @@ Adding GraphCurationToken and SubgraphNFTDescriptor to `horizon/addresses.json` - `staking/Staking.sol`, `staking/L1Staking.sol`, `staking/L1StakingStorage.sol`, `staking/StakingExtension.sol`, `staking/StakingStorage.sol`, `staking/libs/Stakes.sol`, `staking/libs/Exponential.sol`, `staking/libs/LibFixedMath.sol`, `l2/staking/L2Staking.sol` - `disputes/DisputeManager.sol`, `disputes/DisputeManagerStorage.sol`, `discovery/ServiceRegistry.sol`, `discovery/ServiceRegistryStorage.sol` - `payments/AllocationExchange.sol`, `discovery/erc1056/EthereumDIDRegistry.sol`, `rewards/SubgraphAvailabilityManager.sol` - - `tests/arbitrum/ArbSysMock.sol`, `tests/ens/IENS.sol`, `tests/ens/IPublicResolver.sol`, `tests/ens/ITestRegistrar.sol`, `tests/LegacyGNSMock.sol`, `tests/MockERC165.sol`, `tests/MockSubgraphService.sol`, `tests/L1GraphTokenLockTransferToolMock.sol`, `tests/L1GraphTokenLockTransferToolBadMock.sol` + - `tests/arbitrum/ArbSysMock.sol`, `tests/ens/IENS.sol`, `tests/ens/IPublicResolver.sol`, `tests/ens/ITestRegistrar.sol`, `tests/LegacyGNSMock.sol`, `tests/MockERC165.sol`, `tests/MockIssuanceAllocator.sol`, `tests/MockRewardsEligibilityOracle.sol`, `tests/MockSubgraphService.sol`, `tests/L1GraphTokenLockTransferToolMock.sol`, `tests/L1GraphTokenLockTransferToolBadMock.sol` - Delete: `packages/contracts/addresses-staging.json`, `packages/contracts/arbitrum-addresses.json`, `packages/contracts/config/` (`addresses-local.json` is not tracked) - Modify: `packages/contracts/index.js`, `packages/contracts/index.d.ts` (remove `configDir`) -- Modify: `packages/deployment/test/bytecode-comparison.test.ts:235,239` (load `('l2/curation', 'L2Curation')` and `('l2/token', 'L2GraphToken')` instead of the deleted Staking and Curation) +- Modify: `packages/deployment/test/bytecode-comparison.test.ts:235,239` (load `('l2/curation', 'L2Curation')` and `('l2/token', 'L2GraphToken')` instead of the deleted Staking and Curation, and rename the two variables to match) +- Modify: `packages/issuance/test/unit/rewards-manager/mocks/RMEligibilityOracleMock.sol`, `RMIssuanceAllocatorMock.sol` (their `///` comments name the deleted mocks; say they are the test's own mocks without pointing at a deleted file) - Modify: `packages/contracts/hardhat.config.ts` (remove the `StakingExtension` warnings entry) - Modify: `scripts/contracts-migration/manifest.json`, `scripts/contracts-migration/reference.json` (remove `L1GNS`, `Curation`, `L1Staking`, `StakingExtension`, `LibExponential`) **Interfaces:** - Consumes: `legacy` from D2a, and the Ignition alias from D3a. -- Produces: `packages/contracts` holds only live contracts and the mocks ported tests use: 45 Solidity files. Other leftovers that point at deleted things (`.env.example`, `scripts/upgrade`) stay until Phase 2a deletes the package. +- Produces: `packages/contracts` holds only live contracts and the mocks ported tests use: 43 Solidity files. Other leftovers that point at deleted things (`.env.example`, `scripts/upgrade`) stay until Phase 2a deletes the package. Edit `reference.json` by removing those 5 keys. Don't regenerate it with `--write`: leaving the other 21 hashes untouched is what shows the deletions changed no live bytecode. @@ -245,6 +251,6 @@ Edit `reference.json` by removing those 5 keys. Don't regenerate it with `--writ - [ ] **Step 2: Rebuild from clean.** Run `pnpm --filter @graphprotocol/contracts clean && pnpm --filter @graphprotocol/contracts build`. Expected: exit 0. A stale artifact mustn't hide a broken import. - [ ] **Step 3: Commit.** `refactor(contracts): delete dead contracts now archived in legacy` - [ ] **Step 4: Run the verification block.** Expected: `check:bytecode` prints `OK: 21 contracts match`, and `test:self` includes `OK: 7 mainnet implementations reproduce`. -- [ ] **Step 5: Count what's left.** Run `git ls-files 'packages/contracts/contracts/*.sol' | wc -l`. Expected: `45`. +- [ ] **Step 5: Count what's left.** Run `git ls-files 'packages/contracts/contracts/*.sol' | wc -l`. Expected: `43`. - [ ] **Step 6: Load the modules.** Run `npx hardhat --help` in `packages/horizon` and `packages/subgraph-service`. Expected: exit 0 (Review Focus 5). -- [ ] **Step 7: Check nothing else consumed them.** Run `git grep -nE "@graphprotocol/contracts/(artifacts/)?contracts/(staking/(Staking|L1Staking|StakingExtension|StakingStorage)|disputes/|payments/|curation/Curation\.|bancor/|discovery/(L1GNS|ServiceRegistry))" -- ':!**/CHANGELOG.md' ':!docs/contracts-removal'` and `git grep -nE "loadContractsArtifact\('(staking|curation|disputes|bancor|payments)'" -- packages/deployment`. Expected: no output. +- [ ] **Step 7: Check nothing else consumed them.** Run `git grep -nE "(@graphprotocol/contracts/(artifacts/)?contracts/|_ARTIFACTS, \"|ARTIFACTS \+ \"|loadContractsArtifact\(')(staking/(Staking|L1Staking|StakingExtension|StakingStorage|libs/(Stakes|Exponential|LibFixedMath))|staking'|l2/staking|disputes|payments|curation/Curation\.|curation'|bancor|discovery/(L1GNS|ServiceRegistry|erc1056)|rewards/SubgraphAvailability|tests/(arbitrum/ArbSysMock|ens/|LegacyGNSMock|MockERC165|MockIssuanceAllocator|MockRewardsEligibilityOracle|MockSubgraphService|L1GraphTokenLock))" -- ':!**/CHANGELOG.md' ':!docs/contracts-removal' ':!packages/*/ignition/deployments/**' ':!scripts/contracts-migration/test-mapping/**'`. Expected: no output. From 6f83c56f4224040adcec55ee8340f42183c61ee2 Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Fri, 30 Oct 2026 17:08:32 -0300 Subject: [PATCH 11/11] docs: correct phase 1 and ignition design after phase D review --- docs/contracts-removal/design.md | 44 +++++++++++++++----------- docs/contracts-removal/phase-d-plan.md | 12 +++---- 2 files changed, 31 insertions(+), 25 deletions(-) diff --git a/docs/contracts-removal/design.md b/docs/contracts-removal/design.md index 7f0e2c071..65bbaae8a 100644 --- a/docs/contracts-removal/design.md +++ b/docs/contracts-removal/design.md @@ -18,7 +18,7 @@ Out of scope: ### 1. npm - `@graphprotocol/contracts` is frozen at 7.3.0, marked deprecated with `npm deprecate`, and never published again. The git tag `@graphprotocol/contracts@7.3.0` preserves its source, CHANGELOG and docs (`DEPLOYMENT.md`, `TESTING.md`). -- horizon's and subgraph-service's Ignition modules stay as the record of how Horizon was deployed. They read their artifacts from the published 7.3.0 through an npm alias (§5). +- horizon's and subgraph-service's Ignition modules stay as the record of how Horizon was deployed. Most read their artifacts from the published 7.3.0 through an npm alias, and three that have changed since then read live artifacts (§5). - `@graphprotocol/sdk` is already deprecated in its README (which points to `@graphprotocol/toolshed`). It also gets `npm deprecate`. - Known external consumers (graph-network-subgraph, subgraph-studio, network-monitor) pin old versions and are migrated as needed. Addresses come from `@graphprotocol/address-book`, ABIs from `@graphprotocol/interfaces`. @@ -44,6 +44,7 @@ Rules that keep this acyclic: - `issuance` depends on `graph-core` and `interfaces` only. Its code and tests never use horizon, subgraph-discovery or graph-token, because `horizon → toolshed → issuance` already exists. - The 0.7.6 packages (`graph-core`, `graph-token`, `subgraph-discovery`) do not use `toolshed` or `hardhat-graph-protocol`. They have their own small Hardhat configs. `legacy` has no Hardhat config. +- `horizon → issuance` is only for deploying RewardsManager from Ignition. It already exists transitively through toolshed. | Package | Contracts | Compiler | | -------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------ | @@ -57,6 +58,7 @@ Rules that keep this acyclic: Why `subgraph-discovery` is its own package: - These contracts need OpenZeppelin 3.4.2 under its real name. `ISubgraphNFT` and `IGraphCurationToken` in `@graphprotocol/interfaces` import `@openzeppelin/contracts/token/ERC721/IERC721.sol` and `@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol`. Inside horizon or subgraph-service (Hardhat 2, OpenZeppelin 5), those imports resolve to v5 and fail. An alias cannot fix imports inside `interfaces`. +- horizon's and subgraph-service's Ignition modules deploy L2GNS and L2Curation, so the package must sit below them. Deleted, with no replacement: @@ -93,7 +95,7 @@ The 0.7.6 settings must reproduce the current build exactly: solc 0.7.6, optimiz - Each has a single Hardhat compiler: 0.7.6 with the settings above. - graph-token and subgraph-discovery depend on OpenZeppelin `contracts` and `contracts-upgradeable` 3.4.2 under their real names. - graph-core has no OpenZeppelin imports of its own. It only reaches `IERC20` through `IGraphToken`, so it depends on OpenZeppelin `^5.4`, whose `IERC20` uses `>=0.4.16`. That keeps graph-core importable from 0.7.6 code, 0.8 code and 0.8 Foundry tests. (EpochManager, RewardsManager and L2GNS were verified to produce identical bytecode with v5's `IERC20`.) -- `packages/deployment` always uses these packages' own 0.7.6 artifacts. That includes GraphProxy, GraphProxyAdmin and Controller, which are deployed from 0.7.6 builds. Ignition uses the 7.3.0 artifacts (§5). +- `packages/deployment` always uses these packages' own 0.7.6 artifacts. That includes GraphProxy, GraphProxyAdmin and Controller, which are deployed from 0.7.6 builds. Ignition uses the 7.3.0 artifacts where their bytecode is identical (§5). - The graph-core files keep the dual pragma `^0.7.6 || ^0.8.27`, so 0.8 code can import them as source. Managed moves from `^0.7.6` to that dual pragma; it was verified to compile under 0.8.35. - graph-token adds `arbos-precompiles` (needed by L2ArbitrumMessenger). - `TokenUtils` (in graph-token) also keeps its dual pragma. It is imported as source by 0.8 code in horizon (HorizonStaking, GraphPayments, PaymentsEscrow) and subgraph-service (SubgraphService, DisputeManager, AllocationManager, AllocationHandler). @@ -190,18 +192,19 @@ Every test-port PR includes a table mapping each old test case to its new Foundr #### Ignition -Ignition deployed Horizon, but new deployments go through `packages/deployment`. The Ignition modules and journals stay as the record of that deployment, with one mechanical change to their imports. +Ignition deployed Horizon, but new deployments go through `packages/deployment`. The Ignition modules and journals stay as the record of that deployment, and must keep deploying what they deploy today, because `test:integration` and local-network still build stacks with them. The only change is where they read artifacts from. - horizon and subgraph-service import 14 artifacts in 15 module files from `@graphprotocol/contracts/artifacts/...`. Each package's `tasks/deploy.ts` imports the modules (`migrate/*` dynamically), and `loadTasks` loads it in every built Hardhat run. Both tsconfigs also include `ignition/**/*.ts`. The imports must therefore keep resolving after the workspace package loses those artifacts. - Both packages can't simply pin `@graphprotocol/contracts` to 7.3.0. Their Solidity imports from it still need the workspace version until Phase 1: the published sources use the pragma `^0.7.6 || 0.8.27`, and these packages compile with 0.8.35. -- Before D3, both packages add the devDependency `"@graphprotocol/contracts-7.3.0": "npm:@graphprotocol/contracts@7.3.0"`, and the module imports change only the package name to `@graphprotocol/contracts-7.3.0`. The tarball ships every imported artifact. Journals and module logic don't change. -- Their `@graphprotocol/contracts` workspace dependency stays for Solidity imports and is removed in Phase 1 as usual. -- The modules now hold 7.3.0 bytecode. It differs from current source for RewardsManager, L2GNS and L2Curation. `test:integration`, which runs the modules on a fork and isn't in CI, therefore deploys those 7.3.0 versions. +- Before D3, both packages add the devDependency `"@graphprotocol/contracts-7.3.0": "npm:@graphprotocol/contracts@7.3.0"`. Module imports of the 11 artifacts whose 7.3.0 bytecode equals current source change only the package name to `@graphprotocol/contracts-7.3.0`. The tarball ships them all, and deployments are unaffected. +- RewardsManager, L2GNS and L2Curation differ from 7.3.0, so their module imports stay on live artifacts. They can't use 7.3.0: current SubgraphService calls `reclaimRewards`, which the 7.3.0 RewardsManager doesn't have, so closing an allocation would revert. These imports move with the contracts in Phase 1 steps 3 and 5. +- Their `@graphprotocol/contracts` workspace dependency stays for Solidity imports and these three artifacts, and is removed in Phase 1 as usual. #### Verification and scripts -- `horizon/scripts/pre-verify`, `subgraph-service/scripts/pre-verify` and `horizon/tenderly.config.json` `externalArtifacts` read `../contracts/artifacts` for verifying Ignition deployments. They switch to the 7.3.0 alias with the modules, so they keep matching the bytecode the modules deploy. -- issuance's artifacts are Hardhat 3 format (`hh3-artifact-1`). Step 5 confirms that `packages/deployment` and verification work with them. If not, the fix is to ship a Hardhat 2-shaped copy of the RewardsManager artifact from issuance. +- `horizon/scripts/pre-verify` and `subgraph-service/scripts/pre-verify` read `../contracts/artifacts` for verifying Ignition deployments. They copy from the 7.3.0 alias, then the three live artifacts and their build-info over it, so they keep matching what the modules deploy. Steps 3 and 5 update the live paths. +- `horizon/tenderly.config.json` `externalArtifacts` stays on live artifacts, because its only external contracts are L2Curation and RewardsManager. It holds one `source` and one `buildInfo` (`toolshed/src/hardhat/tenderly.ts`), and from step 3 those two contracts live in different packages, so step 3 needs a toolshed change or a split config. +- issuance's artifacts are Hardhat 3 format (`hh3-artifact-1`). Step 5 confirms that horizon's Hardhat 2 Ignition deploy, `pre-verify`, `packages/deployment` and verification all work with them. If not, the fix is to ship a Hardhat 2-shaped copy of the RewardsManager artifact from issuance. #### `packages/deployment` @@ -329,35 +332,38 @@ Phase D moves nothing and leaves the live bytecode hashes unchanged, so the fork Each PR: - moves the files; -- switches every consumer's Solidity imports, `deployment` sources and test artifact paths (not the Ignition modules, §5); +- switches every consumer's Solidity imports, `deployment` sources and test artifact paths, and, for steps 3 and 5, the Ignition imports and verification paths that read live artifacts (§5); - passes 0c; - removes the moved package's temporary `@graphprotocol/contracts` devDependency; - flips the moved package to published (drops `"private"`, adds a `publish.yml` option and an initial changeset). Dependency edges added in each step (removing the devDependency is what prevents a cycle): -| Step | Moves | Edges added | -| ---- | --------------------------------------- | ---------------------------------------------------------------------------------------- | -| 1 | graph-core | contracts, horizon, subgraph-service, testing, deployment → graph-core | -| 2 | graph-token | contracts, horizon, subgraph-service, deployment → graph-token. graph-token → graph-core | -| 3 | subgraph-discovery | deployment → subgraph-discovery. subgraph-discovery → graph-token, graph-core | -| 4 | EpochManager into horizon | horizon → graph-core (already present) | -| 5 | RewardsManager, MathUtils into issuance | issuance → graph-core. testing, deployment → issuance | +| Step | Moves | Edges added | +| ---- | --------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------- | +| 1 | graph-core | contracts, horizon, subgraph-service, testing, deployment → graph-core. graph-token, subgraph-discovery, issuance → graph-core (test devDependency) | +| 2 | graph-token | contracts, horizon, subgraph-service, deployment → graph-token. graph-token → graph-core. subgraph-discovery → graph-token (test devDependency) | +| 3 | subgraph-discovery | horizon, subgraph-service, deployment → subgraph-discovery. subgraph-discovery → graph-token, graph-core | +| 4 | EpochManager into horizon | horizon → graph-core (already present) | +| 5 | RewardsManager, MathUtils into issuance | issuance → graph-core. horizon, testing, deployment → issuance | Per-step notes: 1. **graph-core.** - Generalizes `deployment`'s artifact loader to one source per package, and adds the new package cases to `tasks/verify-contract.ts`. + - Points `CORE_ARTIFACTS` in the Foundry fixtures of graph-token, subgraph-discovery and issuance at graph-core, with `fs_permissions` entries, because Controller, GraphProxy and GraphProxyAdmin leave `packages/contracts` here while those packages' own contracts don't. - Switches `packages/testing`'s Solidity imports. 2. **graph-token.** + - Changes the L2GraphToken path in subgraph-discovery's `DiscoveryTest.t.sol`. Its tests load L2GraphToken from `PKG_ARTIFACTS`, and nothing left in `packages/contracts` imports it, so its artifact disappears at this step. Add a `TOKEN_ARTIFACTS` constant pointing at graph-token's artifacts, with its `fs_permissions` entry and a test devDependency. 3. **subgraph-discovery.** - Adds GraphCurationToken and SubgraphNFTDescriptor to `horizon/addresses.json`. - - Changes the L2GraphToken path in `DiscoveryTest.t.sol`. Its tests load L2GraphToken from `PKG_ARTIFACTS`, but L2GraphToken moved to graph-token in step 2, and Foundry does not compile it in subgraph-discovery. Add a `TOKEN_ARTIFACTS` constant pointing at graph-token's artifacts and load it from there with `_deployArtifact`, including its `fs_permissions` entry. This is the one place where Phase 1 changes a test body. + - Switches the Ignition L2GNS and L2Curation imports and their verification paths to this package. 4. **horizon gets EpochManager.** - Proves the two-compiler config, the `-v3` alias, the Foundry skip, the lint ignores and the `-v3` CI grep rule. - Bumps horizon's OpenZeppelin floor to `^5.4.0`. 5. **issuance gets RewardsManager and MathUtils.** - Confirms the Hardhat 3 compile and artifact compatibility (§3, §5). + - Switches the Ignition RewardsManager import and its verification paths to issuance. - Moves `packages/testing`'s RewardsManager artifact path and `fs_permissions`. - Scheduled when no RewardsManager PRs are in flight. @@ -383,9 +389,9 @@ Per-step notes: ## Risks - **Merge conflicts.** RewardsManager, L2GNS and L2Curation are under active development. Keep T3, T5 and Phase 1 steps 3 and 5 short-lived, and coordinate timing with open work. -- **Ignition modules are frozen.** They stay on the 7.3.0 artifacts. Re-running them, including through `test:integration`, deploys 7.3.0 code, not the moved packages. New deployments go through `packages/deployment`. +- **Ignition keeps two artifact sources.** Most modules read 7.3.0, and three read live artifacts. A contract changed later and left on the alias could pair badly with live code, as RewardsManager did, so a change to a contract deployed from the alias must move its import to live artifacts. - **Two OpenZeppelin versions in horizon and issuance.** Limited to `legacy-0.7/`, and enforced by the CI grep rule. In issuance, Hardhat 3 resolves imports per package, so the import rule in §3 also matters there. -- **Hardhat 3 artifacts consumed by Hardhat 2 tooling.** `packages/deployment` reads the RewardsManager artifact from issuance. Step 5 validates this, with a Hardhat 2-shaped copy as the fallback. +- **Hardhat 3 artifacts consumed by Hardhat 2 tooling.** horizon's Ignition and verification tooling, and `packages/deployment`, read the RewardsManager artifact from issuance. Step 5 validates this, with a Hardhat 2-shaped copy as the fallback. - **Mainnet source drift.** Current source does not reproduce most mainnet implementations. `legacy` therefore archives the verified source, and graph-token's README says its mainnet contracts aren't the deployed code. - **Test port fidelity.** A rewrite can silently drop behaviours. The mapping table and the coverage baseline guard against this. - **Library link references.** Only L1Staking (legacy) links a library (`Exponential`). `legacy`'s check covers it. None of the live moved contracts link libraries. diff --git a/docs/contracts-removal/phase-d-plan.md b/docs/contracts-removal/phase-d-plan.md index b8929ac21..0734c2211 100644 --- a/docs/contracts-removal/phase-d-plan.md +++ b/docs/contracts-removal/phase-d-plan.md @@ -8,7 +8,7 @@ - D1 removes `contracts-test` and `@graphprotocol/sdk`. - D2a creates `legacy` as per-contract bundles of Sourcify-verified source, each compiled with its exact compiler and compared with committed hashes of the on-chain code. D2b adds its address book and audits. -- D3a points the Ignition modules at an npm alias of `@graphprotocol/contracts@7.3.0`, so they survive the deletions. D3b deletes the dead contracts. +- D3a points the Ignition modules whose bytecode is unchanged since 7.3.0 at an npm alias of it, so they survive the deletions. D3b deletes the dead contracts. **Tech Stack:** pnpm workspaces, Node 24 (`node:test`, `node:crypto`), solc-js 0.7.4 and 0.7.6 from npm, Hardhat 2, Foundry 1.8.3. @@ -23,7 +23,7 @@ - D3b's list comes from the spec. Do not delete any other contract. - `packages/legacy` never gets a `.solhint.json`: CI's changed-files mode runs solhint from that package and passes only because it loads no rules there. - The spec's Arbitrum Sepolia fork check for `packages/deployment` doesn't run in this phase: nothing moves, and the unchanged bytecode hashes show no live bytecode changed. It starts with Phase 1. -- Ignition module changes are limited to the package name in import lines. Journals under `ignition/deployments/` don't change. +- Ignition module changes are limited to the package name in import lines, for the 11 artifacts whose bytecode is unchanged since 7.3.0. RewardsManager, L2GNS and L2Curation stay on live artifacts. Journals under `ignition/deployments/` don't change. **Verification for every PR**, run from the repo root after committing. It runs CI's build and test steps, except coverage, and doesn't replace CI's lint job (`lint.yml`): @@ -204,7 +204,7 @@ Adding GraphCurationToken and SubgraphNFTDescriptor to `horizon/addresses.json` - Modify: `packages/horizon/package.json`, `packages/subgraph-service/package.json` (add the devDependency `"@graphprotocol/contracts-7.3.0": "npm:@graphprotocol/contracts@7.3.0"`; keep the `@graphprotocol/contracts` workspace dependency) - Modify: the 15 import-holding files under `ignition/modules/`: in horizon `core/HorizonStaking.ts`, `migrate/migrate-4.ts`, `proxy/GraphProxy.ts` and `periphery/{Controller,Curation,EpochManager,GNS,GraphProxyAdmin,GraphToken,GraphTokenGateway,RewardsManager}.ts`; in subgraph-service `Curation.ts`, `DisputeManager.ts`, `GNS.ts`, `ServiceRegistry.ts` -- Modify: `packages/horizon/scripts/pre-verify`, `packages/subgraph-service/scripts/pre-verify`, `packages/horizon/tenderly.config.json` (`../contracts/artifacts` becomes `node_modules/@graphprotocol/contracts-7.3.0/artifacts`; the tarball ships `artifacts/build-info`) +- Modify: `packages/horizon/scripts/pre-verify`, `packages/subgraph-service/scripts/pre-verify` (copy from `node_modules/@graphprotocol/contracts-7.3.0/artifacts`, then the three live contracts' folders and the workspace `build-info` over them). `tenderly.config.json` doesn't change, because its only external contracts are L2Curation and RewardsManager - Modify: `pnpm-lock.yaml` **Interfaces:** @@ -213,9 +213,9 @@ Adding GraphCurationToken and SubgraphNFTDescriptor to `horizon/addresses.json` - Produces: the modules and their verification tooling no longer read artifacts from the workspace package, so D3b and Phase 1 can remove them. - [ ] **Step 1: Add the dependency,** and run `pnpm install --no-frozen-lockfile`. -- [ ] **Step 2: Replace the package name.** First run `git grep -l "@graphprotocol/contracts/artifacts" -- 'packages/*/ignition/modules/**' | wc -l`. Expected: `15`. In the listed files only, `@graphprotocol/contracts/artifacts/` becomes `@graphprotocol/contracts-7.3.0/artifacts/`. -- [ ] **Step 3: Check the diff is only that.** Run `git diff -- 'packages/*/ignition/**' | grep -E '^[-+][^-+]' | grep -v "@graphprotocol/contracts"`. Expected: no output. Run `git diff --stat -- 'packages/*/ignition/deployments/**'`. Expected: no output. Run `git grep -n "@graphprotocol/contracts/artifacts" -- 'packages/*/ignition/modules/**'`. Expected: no output. -- [ ] **Step 4: Point the verification tooling at the alias.** Edit both `pre-verify` scripts and `tenderly.config.json` as listed under Files. CI never runs them, so check them here. Run `git grep -n '\.\./contracts/artifacts' -- packages/horizon packages/subgraph-service`. Expected: no output. +- [ ] **Step 2: Replace the package name.** First run `git grep -l "@graphprotocol/contracts/artifacts" -- 'packages/*/ignition/modules/**' | wc -l`. Expected: `15`. In the listed files only, `@graphprotocol/contracts/artifacts/` becomes `@graphprotocol/contracts-7.3.0/artifacts/`, except imports of RewardsManager, L2GNS and L2Curation, which stay on live artifacts. +- [ ] **Step 3: Check the diff is only that.** Run `git diff -- 'packages/*/ignition/**' | grep -E '^[-+][^-+]' | grep -v "@graphprotocol/contracts"`. Expected: no output. Run `git diff --stat -- 'packages/*/ignition/deployments/**'`. Expected: no output. Run `git grep -n "@graphprotocol/contracts/artifacts" -- 'packages/*/ignition/modules/**'`. Expected: exactly 5 lines, the three live contracts. +- [ ] **Step 4: Point the verification tooling at the alias.** Edit both `pre-verify` scripts as listed under Files. CI never runs them, so Step 7 checks them. - [ ] **Step 5: Commit.** `chore(ignition): read deployment artifacts from published contracts 7.3.0` - [ ] **Step 6: Run the verification block.** Expected: `check:bytecode` still prints `OK: 26 contracts match`, and horizon's and subgraph-service's suites pass unchanged (Review Focus 4). - [ ] **Step 7: Run `pre-verify`.** In `packages/horizon` and `packages/subgraph-service`, run `bash scripts/pre-verify review-tmp && rm -rf ignition/deployments/review-tmp`. Expected: exit 0, and `git status` stays empty.