feat: add fork-tested in-place upgrade scripts for the deployed proxies

.github/workflows/push_checking.yml

@@ -78,15 +78,6 @@ jobs:
 
       - run: bun install
 
-      # Only the unit-fuzz job needs the fork adapter, and only when fork tests
-      # actually exist in the tree. Fork tests are PR-scoped, so `test/fork/` is
-      # empty on master and `hashFiles` returns '' (step skipped, no docker build).
-      - name: Start revive-eth-rpc (paseo_local fork target)
-        if: matrix.kind.id == 'unit-fuzz' && hashFiles('test/fork/**') != ''
-        run: |
-          docker compose up -d --build
-          bash scripts/shell/wait-for-eth-rpc.sh
-
       - name: Run tests
         id: run
         env:
@@ -122,20 +113,6 @@ jobs:
             exit 1
           fi
 
-      # Fork tests run against the revive-eth-rpc adapter started above, only
-      # when present. The preceding `forge build` already produced full build-info
-      # for the OZ upgrade validator, so this reuses it.
-      - name: Run fork tests
-        if: matrix.kind.id == 'unit-fuzz' && hashFiles('test/fork/**') != ''
-        env:
-          FOUNDRY_DISABLE_NIGHTLY_WARNING: "1"
-          FOUNDRY_PROFILE: "ci"
-        run: forge test -vv --match-path 'test/fork/**'
-
-      - name: Tear down revive-eth-rpc
-        if: always() && matrix.kind.id == 'unit-fuzz' && hashFiles('test/fork/**') != ''
-        run: docker compose down --volumes --remove-orphans
-
       - name: Stage matrix-kind output for the aggregator
         if: always()
         run: |
@@ -163,9 +140,111 @@ jobs:
       - if: steps.run.outputs.result && contains(steps.run.outputs.result, 'Failed')
         run: exit 1
 
+  # Fork tests validate the PR-scoped upgrade scripts against live Paseo Asset Hub
+  # state through the ETH-RPC adapter. They exist only during an upgrade PR, so a
+  # cheap detect job decides whether the heavy fork runner is provisioned at all:
+  # nothing sets up on a PR without `test/fork/**`.
+  detect-fork:
+    if: github.event.action != 'closed'
+    runs-on: ubuntu-latest
+    outputs:
+      has_fork: ${{ steps.detect.outputs.has_fork }}
+    steps:
+      - uses: actions/checkout@v4
+      - id: detect
+        run: |
+          if ls test/fork/*.t.sol > /dev/null 2>&1; then
+            echo "has_fork=true" >> "$GITHUB_OUTPUT"
+          else
+            echo "has_fork=false" >> "$GITHUB_OUTPUT"
+          fi
+
+  fork:
+    name: Upgrade Fork Tests
+    needs: detect-fork
+    if: github.event.action != 'closed' && needs.detect-fork.outputs.has_fork == 'true'
+    # The fork job builds the revive ETH-RPC adapter image and runs the upgrade
+    # suite, so it is kept off the shared test runner and onto the Parity XL runner.
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          submodules: recursive
+
+      - uses: ./.github/actions/setup-foundry
+
+      - uses: oven-sh/setup-bun@v2
+        with:
+          bun-version: "1.2.6"
+          no-cache: true
+
+      - uses: actions/setup-node@v4
+        with:
+          node-version: "20"
+
+      - uses: actions/setup-python@v5
+        with:
+          python-version: "3.12"
+
+      - uses: actions/cache@v4
+        with:
+          path: node_modules
+          key: bun-${{ hashFiles('bun.lock') }}
+          restore-keys: bun-
+
+      - run: bun install
+
+      # Reuse the repository's docker compose eth-rpc service as the paseo_local
+      # fork target.
+      - name: Start revive-eth-rpc (paseo_local fork target)
+        run: |
+          docker compose up -d --build
+          bash scripts/shell/wait-for-eth-rpc.sh
+
+      - name: Run fork tests
+        id: fork
+        env:
+          FOUNDRY_DISABLE_NIGHTLY_WARNING: "1"
+          FOUNDRY_PROFILE: "ci"
+        run: |
+          set +e
+          # A clean build-info keeps the OZ upgrade validator (vm.ffi) from reading
+          # a partial JSON on a warm self-hosted runner.
+          rm -rf out/build-info
+          forge test -vv --match-path 'test/fork/**' 2>&1 | tee fork.log
+          FORK=${PIPESTATUS[0]}
+          if [ "$FORK" -eq 0 ]; then
+            echo "result=Passed" >> "$GITHUB_OUTPUT"
+          else
+            echo "result=Failed" >> "$GITHUB_OUTPUT"
+          fi
+          exit "$FORK"
+
+      - name: Tear down revive-eth-rpc
+        if: always()
+        run: docker compose down --volumes --remove-orphans
+
+      # Report the outcome as its own CI Summary row through a shard artifact the
+      # report job renders. `always()` so a failed run still uploads the Failed row
+      # and the fork log for the reviewer.
+      - name: Stage upgrade-fork output for the aggregator
+        if: always()
+        run: |
+          mkdir -p forkshard
+          printf 'Upgrade Fork Tests|%s|false\n' \
+            "${{ steps.fork.outputs.result || 'Failed' }}" > forkshard/result.txt
+          [ -f fork.log ] && cp fork.log forkshard/fork.log || true
+
+      - uses: actions/upload-artifact@v4
+        if: always()
+        with:
+          name: test-shard-fork
+          path: forkshard
+          retention-days: 7
+
   report:
     name: Report Test Results
-    needs: test
+    needs: [test, fork]
     if: always() && github.event_name == 'pull_request' && github.event.action != 'closed'
     runs-on: ubuntu-latest
     steps:
@@ -267,7 +346,7 @@ jobs:
               }
             }
 
-            const order = ['4naly3er Analysis', 'Slither Analysis', 'Contract Tests (Unit + Fuzz)', 'Contract Tests (Invariant)', 'Gas Report', 'Coverage', 'Documentation', 'Format & Lint', 'File Validation', 'Deploy Contracts', 'PR Title', 'Labels'];
+            const order = ['4naly3er Analysis', 'Slither Analysis', 'Contract Tests (Unit + Fuzz)', 'Contract Tests (Invariant)', 'Upgrade Fork Tests', 'Gas Report', 'Coverage', 'Documentation', 'Format & Lint', 'File Validation', 'Deploy Contracts', 'PR Title', 'Labels'];
             const sortedKeys = Object.keys(rows).sort((a, b) => (order.indexOf(a) === -1 ? 999 : order.indexOf(a)) - (order.indexOf(b) === -1 ? 999 : order.indexOf(b)));
             let table = `| Check | Result |\n|:------|:-------|\n`;
             for (const key of sortedKeys) table += `| ${key} | ${rows[key]} |\n`;

CONTRIBUTING.md

@@ -259,16 +259,36 @@ The `Old.sol` convention has a fixed shape. For a contract `Foo.sol` declaring `
 
 **`Old.sol` snapshots are PR-scoped and must never land on `master`.** They exist only for the upgrade PR that introduces them, so CI and local `forge build` can diff the new layout against the pre-upgrade layout. **Before the PR merges, every `Old.sol` (and every matching `I*Old.sol`) must be deleted, along with the `referenceContract` wiring in the upgrade script.** Once the upgrade is live, the "old" layout is the on-chain deployment, not a file in the repository; keeping the snapshot around after merge would create a phantom contract that future diffs would treat as real code. Reviewers should refuse any PR that ships `Old.sol` files to `master`.
 
+### The upgrade script
+
+Each upgraded proxy has one `Upgrade<Name>.s.sol` under `scripts/deploy/`, paired with its fork test. The script resolves the target proxy from the on-disk manifest, runs the layout diff against the `Old.sol` snapshot, and swaps the implementation through `Upgrades.upgradeProxy`. A beacon-backed store rotates its shared beacon through the factory's upgrade entrypoint after `Upgrades.validateUpgrade`, rather than a per-proxy call. The `referenceContract` is always supplied, so the layout diff is mandatory and fails closed; there is no environment switch that turns it off.
+
+The script asserts the broadcaster owns the proxy, or for a beacon the factory that owns it, before the swap, so a wrong signer fails fast with a clear message rather than reverting inside the upgrade call. It passes no initialiser data unless the new implementation adds storage that needs seeding.
+
 ### Fork tests
 
 Fork tests are upgrade-PR scoped. They live in `test/fork/` for the duration of an upgrade PR, paired 1:1 with the upgrade script under `scripts/deploy/`. They run against a local Paseo Asset Hub fork via the ETH-RPC adapter described in the README's deployment note, and they are deleted alongside the upgrade script and the matching `Old.sol` snapshots before merge. Between upgrade PRs the directory is empty.
 
+Each fork test forks live Asset Hub state, seeds or reads real on-chain state through the deployed implementation, runs the upgrade script, and asserts that state and every P0 path survive on the new implementation. Assertions exercise the real flows rather than bare mints, so a layout regression in a live slot fails the test. The `Old.sol` snapshot reproduces the layout of the implementation currently deployed on-chain, and the fork test is what confirms it: a snapshot that diverged from the live implementation makes the preserved-state assertions fail.
+
+CI wires this in automatically, so an upgrade PR adds fork tests without touching any workflow. The `push_checking` workflow detects `test/fork/**`: when fork tests are present it brings up the ETH-RPC adapter and runs them on a dedicated job that reports an `Upgrade Fork Tests` row in the CI summary; when the directory is empty that job is skipped and no adapter starts. Locally, run the same suite with `bun run test:fork`.
+
 While a fork test is in flight, skip it with:
 
 ```bash
 forge test --no-match-path 'test/fork/**'
 ```
 
+### Broadcasting the upgrade
+
+Broadcast one upgrade at a time with `scripts/deploy/upgrade.sh`, which is permanent tooling and stays on `master`:
+
+```bash
+SCRIPT=UpgradeRegistrar ACCOUNT_NAME=<keystore> RPC_URL=<network> ./scripts/deploy/upgrade.sh
+```
+
+It resolves the deployer account and reuses the shared forge flags (`--legacy`, `--slow`, and the gas limit matching the block gas limit), so an upgrade broadcast cannot drift from the deploy pipeline. The simulation is never skipped.
+
 ### Cleanup checklist before merging an upgrade PR
 
 1. Delete the upgrade script under `scripts/deploy/`.

contracts/registrars/DotnsPopController.sol

@@ -142,8 +142,9 @@ contract DotnsPopController is
     ///      boundary, which is the question it is asking.
     mapping(string label => bool issued) internal _popIssued;
 
-    /// @dev Reserved storage space to allow for layout changes in future upgrades.
-    uint256[50] private __gap;
+    /// @dev Reserved storage space to allow for layout changes in future upgrades. The
+    ///      `_popIssued` mapping consumes one of the reserved slots, so the gap holds 49.
+    uint256[49] private __gap;
 
     /// @notice Restricts calls to a substrate Root origin.
     modifier onlyRoot() {

contracts/registrars/DotnsRegistrar.sol

@@ -61,8 +61,10 @@ contract DotnsRegistrar is
     /// @custom:function quoteTransferFee.
     mapping(uint256 tokenId => bool soulbound) private _soulbound;
 
-    /// @dev Reserved storage space to allow for layout changes in the future.
-    uint256[50] private __gap;
+    /// @dev Reserved storage space to allow for layout changes in the future. `_soulbound` occupies
+    /// one reserved slot, so the gap holds 49 slots and the contract keeps a fixed 51-slot
+    /// footprint.
+    uint256[49] private __gap;
 
     /// @notice Restricts function access to authorised controllers.
     modifier onlyController() {

contracts/registrars/DotnsRegistrarController.sol

@@ -50,6 +50,24 @@ contract DotnsRegistrarController is
     using StringUtils for *;
     using StoreUtils for IStoreFactory;
 
+    /// @notice One role's membership and its admin role.
+    /// @dev Member of the reserved AccessControl namespace, unused because gating is Root only.
+    /// @param hasRole Whether an account holds the role.
+    /// @param adminRole Admin role that manages the role.
+    struct RoleData {
+        mapping(address account => bool) hasRole;
+        bytes32 adminRole;
+    }
+
+    /// @notice Reserved OpenZeppelin access-control namespace held at its ERC-7201 slot.
+    /// @dev Declared and left unused so the namespace stays present in the layout. Its slot derives
+    ///      from the label, disjoint from the sequential slots below, so it consumes none of them.
+    /// @param _roles Role data keyed by role identifier.
+    /// @custom:storage-location erc7201:openzeppelin.storage.AccessControl
+    struct AccessControlStorage {
+        mapping(bytes32 role => RoleData) _roles;
+    }
+
     /// @notice Upper bound for commitment validity to cap storage griefing risk.
     uint256 public constant MAX_ALLOWED_COMMITMENT_AGE = 7 days;
 
@@ -69,11 +87,17 @@ contract DotnsRegistrarController is
     ///      from this stamp.
     mapping(bytes32 hash => uint256 version) public committedPricingVersion;
 
+    /// @dev Reserved slot held so the sequential storage layout stays fixed across the in-place
+    ///      upgrade. Unused: name eligibility lives in @custom:contract DotnsNameWhitelist.
+    /// @custom:oz-renamed-from whiteList
+    mapping(address account => bool retained) private __whiteListSlot;
+
     /// @notice Protocol-level address registry for all DotNS contracts.
     IDotnsProtocolRegistry public protocolRegistry;
 
-    /// @dev Reserved storage space to allow for layout changes in the future.
-    uint256[50] private __gap;
+    /// @dev Reserved storage space to allow for layout changes in the future. The retained
+    ///      whitelist slot above holds one slot, so the gap holds 49 to keep the footprint fixed.
+    uint256[49] private __gap;
 
     /// @custom:oz-upgrades-unsafe-allow constructor
     constructor() {

contracts/whitelist/DotnsNameWhitelist.sol

@@ -48,6 +48,24 @@ contract DotnsNameWhitelist is
     using EnumerableSet for EnumerableSet.AddressSet;
     using EnumerableSet for EnumerableSet.Bytes32Set;
 
+    /// @notice One role's membership and its admin role.
+    /// @dev Member of the reserved AccessControl namespace, unused because gating is Root only.
+    /// @param hasRole Whether an account holds the role.
+    /// @param adminRole Admin role that manages the role.
+    struct RoleData {
+        mapping(address account => bool) hasRole;
+        bytes32 adminRole;
+    }
+
+    /// @notice Reserved OpenZeppelin access-control namespace held at its ERC-7201 slot.
+    /// @dev Declared and left unused so the namespace stays present in the layout. Its slot derives
+    ///      from the label, disjoint from the sequential slots below, so it consumes none of them.
+    /// @param _roles Role data keyed by role identifier.
+    /// @custom:storage-location erc7201:openzeppelin.storage.AccessControl
+    struct AccessControlStorage {
+        mapping(bytes32 role => RoleData) _roles;
+    }
+
     /// @notice Protocol-level address registry for all DotNS contracts.
     IDotnsProtocolRegistry public protocolRegistry;
 

package.json

@@ -5,6 +5,7 @@
   "type": "module",
   "scripts": {
     "test": "forge test -vvvvv",
+    "test:fork": "./scripts/shell/fork-tests.sh",
     "deploy": "./scripts/deploy/run.sh",
     "deploy:anvil": "forge clean && forge build && ./scripts/deploy/run.sh",
     "deploy:testnet": "./scripts/deploy/run.sh '--timeout 1000'",

scripts/deploy/upgrade.sh

@@ -0,0 +1,34 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+# Broadcasts a single in-place upgrade script against the resolved deployer
+# account. It reuses the shared forge flags from _account.sh (legacy, slow, and
+# the gas limit matching the block gas limit) so an upgrade broadcast cannot
+# drift from the deploy pipeline. The upgrade script resolves its target proxy
+# from the on-disk manifest and runs the OpenZeppelin layout diff before the
+# swap; the simulation is never skipped.
+#
+# Usage:
+#   SCRIPT=UpgradeRegistrar ACCOUNT_NAME=<keystore> RPC_URL=<network> ./scripts/deploy/upgrade.sh
+#
+#   SCRIPT              Upgrade script name, for example UpgradeRegistrar.
+#   ACCOUNT_NAME        Foundry keystore account, passed to forge as --account.
+#   RPC_URL             Network RPC alias or URL, same meaning as the deploy runner.
+#   DEPLOYMENT_NETWORK  Optional manifest subdirectory override, same meaning as
+#                       the deploy runner.
+
+ROOT="$(cd "$(dirname "$0")/../.." && pwd)"
+cd "$ROOT"
+
+SCRIPT="${SCRIPT:?set SCRIPT to an upgrade script name, for example UpgradeRegistrar}"
+
+# shellcheck source=scripts/deploy/_account.sh
+. "$(dirname "$0")/_account.sh"
+
+# Exported only when set so the forge script resolves the same manifest folder the
+# deploy runner does; sourcing .env does not auto-export.
+if [ -n "${DEPLOYMENT_NETWORK:-}" ]; then
+  export DEPLOYMENT_NETWORK
+fi
+
+forge script "scripts/deploy/${SCRIPT}.s.sol:${SCRIPT}" "${FORGE_DEPLOY_ARGS[@]}" -vvvvv

scripts/shell/fork-tests.sh

@@ -0,0 +1,44 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+# Single-command fork-test runner. Brings up the local revive ETH-RPC adapter (or
+# reuses one already answering on the RPC url), waits for it to be healthy, does a
+# clean build, then runs the test/fork/** suite against it. Fork tests validate each
+# upgrade script against live Paseo Asset Hub state; see CONTRIBUTING.md (Upgrade-PR
+# workflow) and DEPLOYMENTS.md (Local ETH-RPC adapter). Between upgrade PRs test/fork
+# is empty and the suite runs zero tests, which is a pass.
+#
+# Usage:
+#   bun run test:fork                 # default verbosity (-vvv)
+#   bun run test:fork -- -vvvvv       # pass extra forge args through
+#   RPC_URL=http://127.0.0.1:8545 bun run test:fork
+
+ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"
+cd "$ROOT"
+
+RPC_URL="${RPC_URL:-http://127.0.0.1:8545}"
+
+adapter_is_up() {
+  curl -sf -X POST -H 'Content-Type: application/json' \
+    --data '{"jsonrpc":"2.0","method":"eth_chainId","params":[],"id":1}' \
+    "$RPC_URL" > /dev/null 2>&1
+}
+
+if adapter_is_up; then
+  echo "fork-tests: reusing the ETH-RPC adapter already answering on $RPC_URL"
+else
+  echo "fork-tests: starting the eth-rpc adapter (docker compose up --build -d eth-rpc)"
+  docker compose up --build -d eth-rpc
+  scripts/shell/wait-for-eth-rpc.sh "$RPC_URL"
+fi
+
+# The OpenZeppelin upgrade validator reads Foundry build-info, and a stale incremental
+# build trips it with "Found multiple contracts with name ...". Start from a clean
+# build so every upgrade script's storage-layout diff resolves against fresh artefacts.
+echo "fork-tests: forge clean"
+forge clean
+
+echo "fork-tests: running test/fork/** against $RPC_URL"
+forge test --match-path 'test/fork/**' "${@:--vvv}"
+
+echo "fork-tests: done. Stop the adapter with: docker compose down"