Skip to content

Commit c08b695

Browse files
committed
feat(contracts): add contract upgrade hygiene CI check
New workflow + Bun/TypeScript script that compares deployed bytecode between main and PR for every contract in upgrade-manifest.json. When bytecode changes, the check enforces: - REINITIALIZER_VERSION must be bumped - A reinitializeV{N-1}() function must exist (not just commented out) - At least one of MAJOR/MINOR/PATCH_VERSION must be bumped When bytecode is unchanged, spurious REINITIALIZER_VERSION bumps are flagged. Uses matrix strategy for host-contracts and gateway-contracts. Addresses are generated via `make ensure-addresses` (same as upgrade tests).
1 parent 3134984 commit c08b695

File tree

2 files changed

+252
-0
lines changed

2 files changed

+252
-0
lines changed
Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
name: contracts-upgrade-hygiene
2+
3+
permissions: {}
4+
5+
on:
6+
pull_request:
7+
8+
concurrency:
9+
group: ${{ github.workflow }}-${{ github.ref }}
10+
cancel-in-progress: ${{ github.ref != 'refs/heads/main' }}
11+
12+
jobs:
13+
check-changes:
14+
name: contracts-upgrade-hygiene/check-changes
15+
permissions:
16+
contents: 'read' # Required to checkout repository code
17+
pull-requests: 'read' # Required to read pull request for paths-filter
18+
runs-on: ubuntu-latest
19+
outputs:
20+
packages: ${{ steps.filter.outputs.changes }}
21+
steps:
22+
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
23+
with:
24+
persist-credentials: 'false'
25+
- uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2
26+
id: filter
27+
with:
28+
filters: |
29+
host-contracts:
30+
- .github/workflows/contracts-upgrade-hygiene.yml
31+
- ci/check-upgrade-hygiene.ts
32+
- host-contracts/**
33+
gateway-contracts:
34+
- .github/workflows/contracts-upgrade-hygiene.yml
35+
- ci/check-upgrade-hygiene.ts
36+
- gateway-contracts/**
37+
38+
check:
39+
name: contracts-upgrade-hygiene/${{ matrix.package }} (bpr)
40+
needs: check-changes
41+
if: ${{ needs.check-changes.outputs.packages != '[]' }}
42+
permissions:
43+
contents: 'read' # Required to checkout repository code
44+
runs-on: ubuntu-latest
45+
strategy:
46+
fail-fast: false
47+
matrix:
48+
package: ${{ fromJSON(needs.check-changes.outputs.packages) }}
49+
include:
50+
- package: host-contracts
51+
extra-deps: forge soldeer install
52+
- package: gateway-contracts
53+
extra-deps: ''
54+
steps:
55+
- name: Checkout PR branch
56+
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
57+
with:
58+
persist-credentials: 'false'
59+
60+
- name: Checkout main branch
61+
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
62+
with:
63+
ref: main
64+
path: main-branch
65+
persist-credentials: 'false'
66+
67+
- name: Install Bun
68+
uses: oven-sh/setup-bun@735343b667d3e6f658f44d0eca948eb6282f2b76 # v2.0.2
69+
70+
- name: Install Foundry
71+
uses: foundry-rs/foundry-toolchain@82dee4ba654bd2146511f85f0d013af94670c4de # v1.4.0
72+
73+
- name: Install PR dependencies
74+
working-directory: ${{ matrix.package }}
75+
run: npm ci
76+
77+
- name: Install main dependencies
78+
working-directory: main-branch/${{ matrix.package }}
79+
run: npm ci
80+
81+
- name: Install Forge dependencies
82+
if: matrix.extra-deps != ''
83+
env:
84+
PACKAGE: ${{ matrix.package }}
85+
EXTRA_DEPS: ${{ matrix.extra-deps }}
86+
run: |
87+
(cd "$PACKAGE" && $EXTRA_DEPS)
88+
(cd "main-branch/$PACKAGE" && $EXTRA_DEPS)
89+
90+
- name: Setup compilation
91+
env:
92+
PACKAGE: ${{ matrix.package }}
93+
run: |
94+
# Generate addresses once and share — both sides must use identical values
95+
# because address constants are compiled into bytecode
96+
(cd "$PACKAGE" && make ensure-addresses)
97+
cp -r "$PACKAGE/addresses" "main-branch/$PACKAGE/addresses"
98+
# Use PR's foundry.toml for both so compiler settings match (cbor_metadata, bytecode_hash)
99+
cp "$PACKAGE/foundry.toml" "main-branch/$PACKAGE/foundry.toml"
100+
101+
- name: Run upgrade hygiene check
102+
env:
103+
PACKAGE: ${{ matrix.package }}
104+
run: bun ci/check-upgrade-hygiene.ts "main-branch/$PACKAGE" "$PACKAGE"

ci/check-upgrade-hygiene.ts

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
#!/usr/bin/env bun
2+
// Checks that upgradeable contracts have proper version bumps when bytecode changes.
3+
// Usage: bun ci/check-upgrade-hygiene.ts <main-pkg-dir> <pr-pkg-dir>
4+
5+
import { readFileSync, existsSync } from "fs";
6+
import { execSync } from "child_process";
7+
import { join } from "path";
8+
9+
const [mainDir, prDir] = process.argv.slice(2);
10+
if (!mainDir || !prDir) {
11+
console.error("Usage: bun ci/check-upgrade-hygiene.ts <main-pkg-dir> <pr-pkg-dir>");
12+
process.exit(1);
13+
}
14+
15+
const manifestPath = join(prDir, "upgrade-manifest.json");
16+
if (!existsSync(manifestPath)) {
17+
console.error(`::error::upgrade-manifest.json not found in ${prDir}`);
18+
process.exit(1);
19+
}
20+
21+
const VERSION_RE = /(?<name>REINITIALIZER_VERSION|MAJOR_VERSION|MINOR_VERSION|PATCH_VERSION)\s*=\s*(?<value>\d+)/g;
22+
23+
function extractVersions(filePath: string) {
24+
const source = readFileSync(filePath, "utf-8");
25+
const versions: Record<string, number> = {};
26+
for (const { groups } of source.matchAll(VERSION_RE)) {
27+
versions[groups!.name] = Number(groups!.value);
28+
}
29+
return { versions, source };
30+
}
31+
32+
function forgeInspect(contract: string, root: string): string | null {
33+
try {
34+
const raw = execSync(`forge inspect "contracts/${contract}.sol:${contract}" --root "${root}" deployedBytecode`, {
35+
encoding: "utf-8",
36+
stdio: ["pipe", "pipe", "pipe"],
37+
env: { ...process.env, NO_COLOR: "1" },
38+
});
39+
// Extract hex bytecode — forge may prepend ANSI codes or compilation progress to stdout
40+
const match = raw.match(/0x[0-9a-fA-F]+/);
41+
return match ? match[0] : null;
42+
} catch (e: any) {
43+
if (e.stderr) console.error(String(e.stderr));
44+
return null;
45+
}
46+
}
47+
48+
const contracts: string[] = JSON.parse(readFileSync(manifestPath, "utf-8"));
49+
let errors = 0;
50+
51+
for (const name of contracts) {
52+
console.log(`::group::Checking ${name}`);
53+
try {
54+
const mainSol = join(mainDir, "contracts", `${name}.sol`);
55+
const prSol = join(prDir, "contracts", `${name}.sol`);
56+
57+
if (!existsSync(mainSol)) {
58+
console.log(`Skipping ${name} (new contract, not on main)`);
59+
continue;
60+
}
61+
62+
if (!existsSync(prSol)) {
63+
console.error(`::error::${name} listed in upgrade-manifest.json but missing in PR`);
64+
errors++;
65+
continue;
66+
}
67+
68+
const { versions: mainV } = extractVersions(mainSol);
69+
const { versions: prV, source: prSrc } = extractVersions(prSol);
70+
71+
let parseFailed = false;
72+
for (const key of ["REINITIALIZER_VERSION", "MAJOR_VERSION", "MINOR_VERSION", "PATCH_VERSION"]) {
73+
if (mainV[key] == null || prV[key] == null) {
74+
console.error(`::error::Failed to parse ${key} for ${name}`);
75+
errors++;
76+
parseFailed = true;
77+
}
78+
}
79+
if (parseFailed) continue;
80+
81+
const mainBytecode = forgeInspect(name, mainDir);
82+
if (mainBytecode == null) {
83+
console.error(`::error::Failed to compile ${name} on main`);
84+
errors++;
85+
continue;
86+
}
87+
88+
const prBytecode = forgeInspect(name, prDir);
89+
if (prBytecode == null) {
90+
console.error(`::error::Failed to compile ${name} on PR`);
91+
errors++;
92+
continue;
93+
}
94+
95+
const bytecodeChanged = mainBytecode !== prBytecode;
96+
const reinitChanged = mainV.REINITIALIZER_VERSION !== prV.REINITIALIZER_VERSION;
97+
const versionChanged =
98+
mainV.MAJOR_VERSION !== prV.MAJOR_VERSION ||
99+
mainV.MINOR_VERSION !== prV.MINOR_VERSION ||
100+
mainV.PATCH_VERSION !== prV.PATCH_VERSION;
101+
102+
if (!bytecodeChanged) {
103+
console.log(`${name}: bytecode unchanged`);
104+
if (reinitChanged) {
105+
console.error(
106+
`::error::${name} REINITIALIZER_VERSION bumped (${mainV.REINITIALIZER_VERSION} -> ${prV.REINITIALIZER_VERSION}) but bytecode is unchanged`,
107+
);
108+
errors++;
109+
}
110+
continue;
111+
}
112+
113+
console.log(`${name}: bytecode CHANGED`);
114+
115+
if (!reinitChanged) {
116+
console.error(
117+
`::error::${name} bytecode changed but REINITIALIZER_VERSION was not bumped (still ${prV.REINITIALIZER_VERSION})`,
118+
);
119+
errors++;
120+
} else {
121+
// Convention: reinitializeV{N-1} for REINITIALIZER_VERSION=N
122+
const expectedFn = `reinitializeV${prV.REINITIALIZER_VERSION - 1}`;
123+
const uncommented = prSrc.replace(/\/\*[\s\S]*?\*\//g, "").replace(/\/\/.*$/gm, "");
124+
if (!new RegExp(`function\\s+${expectedFn}\\s*\\(`).test(uncommented)) {
125+
console.error(
126+
`::error::${name} has REINITIALIZER_VERSION=${prV.REINITIALIZER_VERSION} but no ${expectedFn}() function found`,
127+
);
128+
errors++;
129+
}
130+
}
131+
132+
if (!versionChanged) {
133+
console.error(
134+
`::error::${name} bytecode changed but semantic version was not bumped (still v${prV.MAJOR_VERSION}.${prV.MINOR_VERSION}.${prV.PATCH_VERSION})`,
135+
);
136+
errors++;
137+
}
138+
} finally {
139+
console.log("::endgroup::");
140+
}
141+
}
142+
143+
if (errors > 0) {
144+
console.error(`::error::Upgrade hygiene check failed with ${errors} error(s)`);
145+
process.exit(1);
146+
}
147+
148+
console.log("All contracts passed upgrade hygiene checks");

0 commit comments

Comments
 (0)