Skip to content

Commit feb1667

Browse files
committed
refactor: deduplicate workflow with matrix, extract stubs to files
- Replace two near-identical jobs with a single matrix job (host-contracts, gateway-contracts), cutting workflow from 151 to 92 lines - Move inline Solidity address heredocs to committed ci/stubs/ files - Add missing PaymentBridgingAddresses.sol stub (was breaking gateway CI) - Drop forge build pre-step: forge inspect already compiles and caches on first call, and forge build required ALL stubs including non-manifest contracts which caused failures - Trim script from 161 to 110 lines: shorter header, flattened control flow
1 parent 2133dfe commit feb1667

File tree

5 files changed

+76
-165
lines changed

5 files changed

+76
-165
lines changed

.github/workflows/contracts-upgrade-hygiene.yml

Lines changed: 28 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,7 @@ jobs:
1717
pull-requests: 'read'
1818
runs-on: ubuntu-latest
1919
outputs:
20-
host-contracts: ${{ steps.filter.outputs.host-contracts }}
21-
gateway-contracts: ${{ steps.filter.outputs.gateway-contracts }}
20+
packages: ${{ steps.filter.outputs.changes }}
2221
steps:
2322
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
2423
with:
@@ -30,19 +29,30 @@ jobs:
3029
host-contracts:
3130
- .github/workflows/contracts-upgrade-hygiene.yml
3231
- ci/check-upgrade-hygiene.sh
32+
- ci/stubs/host-contracts/**
3333
- host-contracts/**
3434
gateway-contracts:
3535
- .github/workflows/contracts-upgrade-hygiene.yml
3636
- ci/check-upgrade-hygiene.sh
37+
- ci/stubs/gateway-contracts/**
3738
- gateway-contracts/**
3839
39-
host-contracts:
40-
name: contracts-upgrade-hygiene/host-contracts
40+
check:
41+
name: contracts-upgrade-hygiene/${{ matrix.package }}
4142
needs: check-changes
42-
if: ${{ needs.check-changes.outputs.host-contracts == 'true' }}
43+
if: ${{ needs.check-changes.outputs.packages != '[]' }}
4344
permissions:
4445
contents: 'read'
4546
runs-on: ubuntu-latest
47+
strategy:
48+
fail-fast: false
49+
matrix:
50+
package: ${{ fromJSON(needs.check-changes.outputs.packages) }}
51+
include:
52+
- package: host-contracts
53+
extra-deps: forge soldeer install
54+
- package: gateway-contracts
55+
extra-deps: ''
4656
steps:
4757
- name: Checkout PR branch
4858
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
@@ -59,93 +69,24 @@ jobs:
5969
- name: Install Foundry
6070
uses: foundry-rs/foundry-toolchain@82dee4ba654bd2146511f85f0d013af94670c4de # v1.4.0
6171

62-
- name: Install PR dependencies
63-
working-directory: host-contracts
72+
- name: Install dependencies
6473
run: |
65-
npm ci
66-
forge soldeer install
67-
68-
- name: Install main dependencies
69-
working-directory: main-branch/host-contracts
70-
run: |
71-
npm ci
72-
forge soldeer install
73-
74-
- name: Align compilation settings
75-
run: cp host-contracts/foundry.toml main-branch/host-contracts/foundry.toml
76-
77-
- name: Generate address stubs
78-
run: |
79-
# Host contracts need addresses/FHEVMHostAddresses.sol (generated at deploy time).
80-
# Create identical stubs in both copies so bytecode comparison is not affected.
81-
mkdir -p host-contracts/addresses main-branch/host-contracts/addresses
82-
cat > /tmp/FHEVMHostAddresses.sol << 'SOL'
83-
// SPDX-License-Identifier: BSD-3-Clause-Clear
84-
pragma solidity ^0.8.24;
85-
address constant aclAdd = 0x0000000000000000000000000000000000000001;
86-
address constant fhevmExecutorAdd = 0x0000000000000000000000000000000000000002;
87-
address constant kmsVerifierAdd = 0x0000000000000000000000000000000000000003;
88-
address constant inputVerifierAdd = 0x0000000000000000000000000000000000000004;
89-
address constant hcuLimitAdd = 0x0000000000000000000000000000000000000005;
90-
address constant pauserSetAdd = 0x0000000000000000000000000000000000000006;
91-
SOL
92-
cp /tmp/FHEVMHostAddresses.sol host-contracts/addresses/FHEVMHostAddresses.sol
93-
cp /tmp/FHEVMHostAddresses.sol main-branch/host-contracts/addresses/FHEVMHostAddresses.sol
94-
95-
- name: Run upgrade hygiene check
96-
run: ./ci/check-upgrade-hygiene.sh main-branch/host-contracts host-contracts
97-
98-
gateway-contracts:
99-
name: contracts-upgrade-hygiene/gateway-contracts
100-
needs: check-changes
101-
if: ${{ needs.check-changes.outputs.gateway-contracts == 'true' }}
102-
permissions:
103-
contents: 'read'
104-
runs-on: ubuntu-latest
105-
steps:
106-
- name: Checkout PR branch
107-
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
108-
with:
109-
persist-credentials: 'false'
110-
111-
- name: Checkout main branch
112-
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
113-
with:
114-
ref: main
115-
path: main-branch
116-
persist-credentials: 'false'
117-
118-
- name: Install Foundry
119-
uses: foundry-rs/foundry-toolchain@82dee4ba654bd2146511f85f0d013af94670c4de # v1.4.0
120-
121-
- name: Install PR dependencies
122-
working-directory: gateway-contracts
123-
run: npm ci
124-
125-
- name: Install main dependencies
126-
working-directory: main-branch/gateway-contracts
127-
run: npm ci
74+
npm --prefix ${{ matrix.package }} ci
75+
npm --prefix main-branch/${{ matrix.package }} ci
76+
if [ -n "${{ matrix.extra-deps }}" ]; then
77+
(cd ${{ matrix.package }} && ${{ matrix.extra-deps }})
78+
(cd main-branch/${{ matrix.package }} && ${{ matrix.extra-deps }})
79+
fi
12880
12981
- name: Align compilation settings
130-
run: cp gateway-contracts/foundry.toml main-branch/gateway-contracts/foundry.toml
82+
run: cp ${{ matrix.package }}/foundry.toml main-branch/${{ matrix.package }}/foundry.toml
13183

13284
- name: Generate address stubs
13385
run: |
134-
# Gateway contracts need addresses/GatewayAddresses.sol (generated at deploy time).
135-
mkdir -p gateway-contracts/addresses main-branch/gateway-contracts/addresses
136-
cat > /tmp/GatewayAddresses.sol << 'SOL'
137-
// SPDX-License-Identifier: BSD-3-Clause-Clear
138-
pragma solidity ^0.8.24;
139-
address constant gatewayConfigAddress = 0x0000000000000000000000000000000000000001;
140-
address constant decryptionAddress = 0x0000000000000000000000000000000000000002;
141-
address constant ciphertextCommitsAddress = 0x0000000000000000000000000000000000000003;
142-
address constant inputVerificationAddress = 0x0000000000000000000000000000000000000004;
143-
address constant kmsGenerationAddress = 0x0000000000000000000000000000000000000005;
144-
address constant protocolPaymentAddress = 0x0000000000000000000000000000000000000006;
145-
address constant pauserSetAddress = 0x0000000000000000000000000000000000000007;
146-
SOL
147-
cp /tmp/GatewayAddresses.sol gateway-contracts/addresses/GatewayAddresses.sol
148-
cp /tmp/GatewayAddresses.sol main-branch/gateway-contracts/addresses/GatewayAddresses.sol
86+
for dir in ${{ matrix.package }} main-branch/${{ matrix.package }}; do
87+
mkdir -p "$dir/addresses"
88+
cp ci/stubs/${{ matrix.package }}/*.sol "$dir/addresses/"
89+
done
14990
15091
- name: Run upgrade hygiene check
151-
run: ./ci/check-upgrade-hygiene.sh main-branch/gateway-contracts gateway-contracts
92+
run: ./ci/check-upgrade-hygiene.sh main-branch/${{ matrix.package }} ${{ matrix.package }}

ci/check-upgrade-hygiene.sh

Lines changed: 27 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,6 @@
11
#!/usr/bin/env bash
2-
# ci/check-upgrade-hygiene.sh
3-
#
4-
# Validates that upgradeable contracts have proper version bumps when bytecode changes.
5-
# Compares deployed bytecodes between two copies of a contract package (e.g. main vs PR branch).
6-
#
7-
# Usage:
8-
# ./ci/check-upgrade-hygiene.sh <main-pkg-dir> <pr-pkg-dir>
9-
#
10-
# Example:
11-
# ./ci/check-upgrade-hygiene.sh main-branch/host-contracts host-contracts
12-
#
13-
# Requires: forge (Foundry), jq
14-
# Both directories must have:
15-
# - foundry.toml with cbor_metadata=false and bytecode_hash='none'
16-
# - upgrade-manifest.json listing contract names
17-
# - contracts/<Name>.sol for each manifest entry
18-
# - addresses/ stub (generated file) so contracts compile
19-
2+
# Checks that upgradeable contracts have proper version bumps when bytecode changes.
3+
# Usage: ./ci/check-upgrade-hygiene.sh <main-pkg-dir> <pr-pkg-dir>
204
set -euo pipefail
215

226
MAIN_DIR="$1"
@@ -29,8 +13,7 @@ fi
2913

3014
ERRORS=0
3115

32-
# Extract all four version constants from a .sol file in a single pass.
33-
# Returns: REINITIALIZER_VERSION MAJOR_VERSION MINOR_VERSION PATCH_VERSION
16+
# Extract REINITIALIZER_VERSION, MAJOR_VERSION, MINOR_VERSION, PATCH_VERSION in one pass.
3417
extract_versions() {
3518
awk '
3619
function val_after_eq(line) { sub(/.*=[[:space:]]*/, "", line); sub(/[^0-9].*/, "", line); return line }
@@ -42,35 +25,25 @@ extract_versions() {
4225
' "$1"
4326
}
4427

45-
# Pre-compile both roots in parallel so all forge inspect calls are cache hits.
46-
forge build --root "$MAIN_DIR" &
47-
pid_main=$!
48-
forge build --root "$PR_DIR" &
49-
pid_pr=$!
50-
wait "$pid_main" || { echo "::error::forge build failed for $MAIN_DIR"; exit 1; }
51-
wait "$pid_pr" || { echo "::error::forge build failed for $PR_DIR"; exit 1; }
52-
5328
for name in $(jq -r '.[]' "$PR_DIR/upgrade-manifest.json"); do
5429
echo "::group::Checking $name"
5530

5631
main_sol="$MAIN_DIR/contracts/${name}.sol"
5732
pr_sol="$PR_DIR/contracts/${name}.sol"
5833

59-
# Skip contracts not present on main (newly added)
6034
if [ ! -f "$main_sol" ]; then
6135
echo "Skipping $name (new contract, not on main)"
6236
echo "::endgroup::"
6337
continue
6438
fi
6539

6640
if [ ! -f "$pr_sol" ]; then
67-
echo "::error::$name is in upgrade-manifest.json but contracts/${name}.sol not found in PR"
41+
echo "::error::$name listed in upgrade-manifest.json but missing in PR"
6842
ERRORS=$((ERRORS + 1))
6943
echo "::endgroup::"
7044
continue
7145
fi
7246

73-
# --- Extract version constants from both (single pass per file) ---
7447
read -r main_reinit main_major main_minor main_patch < <(extract_versions "$main_sol")
7548
read -r pr_reinit pr_major pr_minor pr_patch < <(extract_versions "$pr_sol")
7649

@@ -83,73 +56,49 @@ for name in $(jq -r '.[]' "$PR_DIR/upgrade-manifest.json"); do
8356
fi
8457
done
8558

86-
# --- Compare bytecodes (paths relative to --root, builds are cached) ---
87-
# Capture only stdout for bytecode; stderr goes to /dev/null on success (warnings),
88-
# but on failure we re-run to capture the error message.
59+
# forge inspect compiles on first call and caches; stderr suppressed to avoid warning noise.
8960
if ! main_bytecode=$(forge inspect "contracts/${name}.sol:$name" --root "$MAIN_DIR" deployedBytecode 2>/dev/null); then
90-
echo "::error::Failed to inspect $name on main:$(forge inspect "contracts/${name}.sol:$name" --root "$MAIN_DIR" deployedBytecode 2>&1 || true)"
61+
echo "::error::Failed to compile $name on main"
9162
ERRORS=$((ERRORS + 1))
9263
echo "::endgroup::"
9364
continue
9465
fi
9566
if ! pr_bytecode=$(forge inspect "contracts/${name}.sol:$name" --root "$PR_DIR" deployedBytecode 2>/dev/null); then
96-
echo "::error::Failed to inspect $name on PR:$(forge inspect "contracts/${name}.sol:$name" --root "$PR_DIR" deployedBytecode 2>&1 || true)"
67+
echo "::error::Failed to compile $name on PR"
9768
ERRORS=$((ERRORS + 1))
9869
echo "::endgroup::"
9970
continue
10071
fi
10172

102-
bytecode_changed=false
103-
if [ "$main_bytecode" != "$pr_bytecode" ]; then
104-
bytecode_changed=true
105-
fi
106-
107-
version_changed=false
108-
if [ "$main_major" != "$pr_major" ] || [ "$main_minor" != "$pr_minor" ] || [ "$main_patch" != "$pr_patch" ]; then
109-
version_changed=true
110-
fi
111-
112-
reinit_changed=false
113-
if [ "$main_reinit" != "$pr_reinit" ]; then
114-
reinit_changed=true
115-
fi
116-
117-
if [ "$bytecode_changed" = true ]; then
118-
echo "$name: bytecode CHANGED"
119-
120-
# Check 1: REINITIALIZER_VERSION must be bumped
121-
if [ "$reinit_changed" = false ]; then
122-
echo "::error::$name bytecode changed but REINITIALIZER_VERSION was not bumped (still $pr_reinit)"
73+
if [ "$main_bytecode" = "$pr_bytecode" ]; then
74+
echo "$name: bytecode unchanged"
75+
if [ "$main_reinit" != "$pr_reinit" ]; then
76+
echo "::error::$name REINITIALIZER_VERSION bumped ($main_reinit -> $pr_reinit) but bytecode is unchanged"
12377
ERRORS=$((ERRORS + 1))
12478
fi
79+
echo "::endgroup::"
80+
continue
81+
fi
12582

126-
# Check 2: reinitializeVN function must exist (convention: N = REINITIALIZER_VERSION - 1)
127-
if [ "$reinit_changed" = true ]; then
128-
expected_n=$((pr_reinit - 1))
129-
expected_fn="reinitializeV${expected_n}"
130-
# Look for function declaration (not just any mention)
131-
if ! grep -qE "function[[:space:]]+${expected_fn}[[:space:]]*\(" "$pr_sol"; then
132-
echo "::error::$name has REINITIALIZER_VERSION=$pr_reinit but no $expected_fn() function found"
133-
ERRORS=$((ERRORS + 1))
134-
fi
135-
fi
136-
137-
# Check 3: Semantic version must be bumped
138-
if [ "$version_changed" = false ]; then
139-
echo "::error::$name bytecode changed but semantic version was not bumped (still v${pr_major}.${pr_minor}.${pr_patch})"
140-
ERRORS=$((ERRORS + 1))
141-
fi
83+
echo "$name: bytecode CHANGED"
14284

85+
if [ "$main_reinit" = "$pr_reinit" ]; then
86+
echo "::error::$name bytecode changed but REINITIALIZER_VERSION was not bumped (still $pr_reinit)"
87+
ERRORS=$((ERRORS + 1))
14388
else
144-
echo "$name: bytecode unchanged"
145-
146-
# Inverse check: reinitializer should NOT be bumped if bytecode didn't change
147-
if [ "$reinit_changed" = true ]; then
148-
echo "::error::$name REINITIALIZER_VERSION bumped ($main_reinit -> $pr_reinit) but bytecode is unchanged"
89+
# Convention: reinitializeV{N-1} for REINITIALIZER_VERSION=N
90+
expected_fn="reinitializeV$((pr_reinit - 1))"
91+
if ! grep -qE "function[[:space:]]+${expected_fn}[[:space:]]*\(" "$pr_sol"; then
92+
echo "::error::$name has REINITIALIZER_VERSION=$pr_reinit but no $expected_fn() function found"
14993
ERRORS=$((ERRORS + 1))
15094
fi
15195
fi
15296

97+
if [ "$main_major" = "$pr_major" ] && [ "$main_minor" = "$pr_minor" ] && [ "$main_patch" = "$pr_patch" ]; then
98+
echo "::error::$name bytecode changed but semantic version was not bumped (still v${pr_major}.${pr_minor}.${pr_patch})"
99+
ERRORS=$((ERRORS + 1))
100+
fi
101+
153102
echo "::endgroup::"
154103
done
155104

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// SPDX-License-Identifier: BSD-3-Clause-Clear
2+
pragma solidity ^0.8.24;
3+
address constant gatewayConfigAddress = 0x0000000000000000000000000000000000000001;
4+
address constant decryptionAddress = 0x0000000000000000000000000000000000000002;
5+
address constant ciphertextCommitsAddress = 0x0000000000000000000000000000000000000003;
6+
address constant inputVerificationAddress = 0x0000000000000000000000000000000000000004;
7+
address constant kmsGenerationAddress = 0x0000000000000000000000000000000000000005;
8+
address constant protocolPaymentAddress = 0x0000000000000000000000000000000000000006;
9+
address constant pauserSetAddress = 0x0000000000000000000000000000000000000007;
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
// SPDX-License-Identifier: BSD-3-Clause-Clear
2+
pragma solidity ^0.8.24;
3+
address constant feesSenderToBurnerAddress = 0x0000000000000000000000000000000000000008;
4+
address constant zamaOFTAddress = 0x0000000000000000000000000000000000000009;
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// SPDX-License-Identifier: BSD-3-Clause-Clear
2+
pragma solidity ^0.8.24;
3+
address constant aclAdd = 0x0000000000000000000000000000000000000001;
4+
address constant fhevmExecutorAdd = 0x0000000000000000000000000000000000000002;
5+
address constant kmsVerifierAdd = 0x0000000000000000000000000000000000000003;
6+
address constant inputVerifierAdd = 0x0000000000000000000000000000000000000004;
7+
address constant hcuLimitAdd = 0x0000000000000000000000000000000000000005;
8+
address constant pauserSetAdd = 0x0000000000000000000000000000000000000006;

0 commit comments

Comments
 (0)