diff --git a/.github/workflows/contracts-upgrade-hygiene.yml b/.github/workflows/contracts-upgrade-hygiene.yml new file mode 100644 index 0000000000..341565d665 --- /dev/null +++ b/.github/workflows/contracts-upgrade-hygiene.yml @@ -0,0 +1,106 @@ +name: contracts-upgrade-hygiene + +permissions: {} + +on: + pull_request: + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: ${{ github.ref != 'refs/heads/main' }} + +jobs: + check-changes: + name: contracts-upgrade-hygiene/check-changes + permissions: + contents: 'read' # Required to checkout repository code + pull-requests: 'read' # Required to read pull request for paths-filter + runs-on: ubuntu-latest + outputs: + packages: ${{ steps.filter.outputs.changes }} + steps: + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + persist-credentials: 'false' + - uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2 + id: filter + with: + filters: | + host-contracts: + - .github/workflows/contracts-upgrade-hygiene.yml + - ci/check-upgrade-hygiene.ts + - ci/stubs/host-contracts/** + - host-contracts/** + gateway-contracts: + - .github/workflows/contracts-upgrade-hygiene.yml + - ci/check-upgrade-hygiene.ts + - ci/stubs/gateway-contracts/** + - gateway-contracts/** + + check: + name: contracts-upgrade-hygiene/${{ matrix.package }} (bpr) + needs: check-changes + if: ${{ needs.check-changes.outputs.packages != '[]' }} + permissions: + contents: 'read' # Required to checkout repository code + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + package: ${{ fromJSON(needs.check-changes.outputs.packages) }} + include: + - package: host-contracts + extra-deps: forge soldeer install + - package: gateway-contracts + extra-deps: '' + steps: + - name: Checkout PR branch + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + persist-credentials: 'false' + + - name: Checkout main branch + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: main + path: main-branch + persist-credentials: 'false' + + - name: Install Bun + uses: oven-sh/setup-bun@735343b667d3e6f658f44d0eca948eb6282f2b76 # v2.0.2 + + - name: Install Foundry + uses: foundry-rs/foundry-toolchain@82dee4ba654bd2146511f85f0d013af94670c4de # v1.4.0 + + - name: Install PR dependencies + working-directory: ${{ matrix.package }} + run: npm ci + + - name: Install main dependencies + working-directory: main-branch/${{ matrix.package }} + run: npm ci + + - name: Install Forge dependencies + if: matrix.extra-deps != '' + env: + PACKAGE: ${{ matrix.package }} + EXTRA_DEPS: ${{ matrix.extra-deps }} + run: | + (cd "$PACKAGE" && $EXTRA_DEPS) + (cd "main-branch/$PACKAGE" && $EXTRA_DEPS) + + - name: Setup compilation + env: + PACKAGE: ${{ matrix.package }} + run: | + # Use PR's foundry.toml for both so compiler settings match (cbor_metadata, bytecode_hash) + cp "$PACKAGE/foundry.toml" "main-branch/$PACKAGE/foundry.toml" + for dir in "$PACKAGE" "main-branch/$PACKAGE"; do + mkdir -p "$dir/addresses" + cp ci/stubs/"$PACKAGE"/*.sol "$dir/addresses/" + done + + - name: Run upgrade hygiene check + env: + PACKAGE: ${{ matrix.package }} + run: bun ci/check-upgrade-hygiene.ts "main-branch/$PACKAGE" "$PACKAGE" diff --git a/ci/check-upgrade-hygiene.ts b/ci/check-upgrade-hygiene.ts new file mode 100644 index 0000000000..2d49bba71f --- /dev/null +++ b/ci/check-upgrade-hygiene.ts @@ -0,0 +1,144 @@ +#!/usr/bin/env bun +// Checks that upgradeable contracts have proper version bumps when bytecode changes. +// Usage: bun ci/check-upgrade-hygiene.ts + +import { readFileSync, existsSync } from "fs"; +import { execSync } from "child_process"; +import { join } from "path"; + +const [mainDir, prDir] = process.argv.slice(2); +if (!mainDir || !prDir) { + console.error("Usage: bun ci/check-upgrade-hygiene.ts "); + process.exit(1); +} + +const manifestPath = join(prDir, "upgrade-manifest.json"); +if (!existsSync(manifestPath)) { + console.error(`::error::upgrade-manifest.json not found in ${prDir}`); + process.exit(1); +} + +const VERSION_RE = /(?REINITIALIZER_VERSION|MAJOR_VERSION|MINOR_VERSION|PATCH_VERSION)\s*=\s*(?\d+)/g; + +function extractVersions(filePath: string) { + const source = readFileSync(filePath, "utf-8"); + const versions: Record = {}; + for (const { groups } of source.matchAll(VERSION_RE)) { + versions[groups!.name] = Number(groups!.value); + } + return { versions, source }; +} + +function forgeInspect(contract: string, root: string): string | null { + try { + return execSync(`forge inspect "contracts/${contract}.sol:${contract}" --root "${root}" deployedBytecode`, { + encoding: "utf-8", + stdio: ["pipe", "pipe", "pipe"], + }).trim(); + } catch (e: any) { + if (e.stderr) console.error(String(e.stderr)); + return null; + } +} + +const contracts: string[] = JSON.parse(readFileSync(manifestPath, "utf-8")); +let errors = 0; + +for (const name of contracts) { + console.log(`::group::Checking ${name}`); + try { + const mainSol = join(mainDir, "contracts", `${name}.sol`); + const prSol = join(prDir, "contracts", `${name}.sol`); + + if (!existsSync(mainSol)) { + console.log(`Skipping ${name} (new contract, not on main)`); + continue; + } + + if (!existsSync(prSol)) { + console.error(`::error::${name} listed in upgrade-manifest.json but missing in PR`); + errors++; + continue; + } + + const { versions: mainV } = extractVersions(mainSol); + const { versions: prV, source: prSrc } = extractVersions(prSol); + + let parseFailed = false; + for (const key of ["REINITIALIZER_VERSION", "MAJOR_VERSION", "MINOR_VERSION", "PATCH_VERSION"]) { + if (mainV[key] == null || prV[key] == null) { + console.error(`::error::Failed to parse ${key} for ${name}`); + errors++; + parseFailed = true; + } + } + if (parseFailed) continue; + + const mainBytecode = forgeInspect(name, mainDir); + if (mainBytecode == null) { + console.error(`::error::Failed to compile ${name} on main`); + errors++; + continue; + } + + const prBytecode = forgeInspect(name, prDir); + if (prBytecode == null) { + console.error(`::error::Failed to compile ${name} on PR`); + errors++; + continue; + } + + const bytecodeChanged = mainBytecode !== prBytecode; + const reinitChanged = mainV.REINITIALIZER_VERSION !== prV.REINITIALIZER_VERSION; + const versionChanged = + mainV.MAJOR_VERSION !== prV.MAJOR_VERSION || + mainV.MINOR_VERSION !== prV.MINOR_VERSION || + mainV.PATCH_VERSION !== prV.PATCH_VERSION; + + if (!bytecodeChanged) { + console.log(`${name}: bytecode unchanged`); + if (reinitChanged) { + console.error( + `::error::${name} REINITIALIZER_VERSION bumped (${mainV.REINITIALIZER_VERSION} -> ${prV.REINITIALIZER_VERSION}) but bytecode is unchanged`, + ); + errors++; + } + continue; + } + + console.log(`${name}: bytecode CHANGED`); + + if (!reinitChanged) { + console.error( + `::error::${name} bytecode changed but REINITIALIZER_VERSION was not bumped (still ${prV.REINITIALIZER_VERSION})`, + ); + errors++; + } else { + // Convention: reinitializeV{N-1} for REINITIALIZER_VERSION=N + const expectedFn = `reinitializeV${prV.REINITIALIZER_VERSION - 1}`; + const uncommented = prSrc.replace(/\/\*[\s\S]*?\*\//g, "").replace(/\/\/.*$/gm, ""); + if (!new RegExp(`function\\s+${expectedFn}\\s*\\(`).test(uncommented)) { + console.error( + `::error::${name} has REINITIALIZER_VERSION=${prV.REINITIALIZER_VERSION} but no ${expectedFn}() function found`, + ); + errors++; + } + } + + if (!versionChanged) { + console.error( + `::error::${name} bytecode changed but semantic version was not bumped (still v${prV.MAJOR_VERSION}.${prV.MINOR_VERSION}.${prV.PATCH_VERSION})`, + ); + errors++; + } + } finally { + console.log("::endgroup::"); + } +} + +if (errors > 0) { + console.error(`::error::Upgrade hygiene check failed with ${errors} error(s)`); + process.exit(1); +} + +console.log("All contracts passed upgrade hygiene checks"); diff --git a/ci/stubs/gateway-contracts/GatewayAddresses.sol b/ci/stubs/gateway-contracts/GatewayAddresses.sol new file mode 100644 index 0000000000..cd5400e287 --- /dev/null +++ b/ci/stubs/gateway-contracts/GatewayAddresses.sol @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: BSD-3-Clause-Clear +// Stub: the real addresses/ files are generated at deploy time and gitignored. +// These dummy values let forge compile the contracts for bytecode comparison in CI. +pragma solidity ^0.8.24; +address constant gatewayConfigAddress = 0x0000000000000000000000000000000000000001; +address constant decryptionAddress = 0x0000000000000000000000000000000000000002; +address constant ciphertextCommitsAddress = 0x0000000000000000000000000000000000000003; +address constant inputVerificationAddress = 0x0000000000000000000000000000000000000004; +address constant kmsGenerationAddress = 0x0000000000000000000000000000000000000005; +address constant protocolPaymentAddress = 0x0000000000000000000000000000000000000006; +address constant pauserSetAddress = 0x0000000000000000000000000000000000000007; diff --git a/ci/stubs/gateway-contracts/PaymentBridgingAddresses.sol b/ci/stubs/gateway-contracts/PaymentBridgingAddresses.sol new file mode 100644 index 0000000000..74a035fa1e --- /dev/null +++ b/ci/stubs/gateway-contracts/PaymentBridgingAddresses.sol @@ -0,0 +1,6 @@ +// SPDX-License-Identifier: BSD-3-Clause-Clear +// Stub: the real addresses/ files are generated at deploy time and gitignored. +// These dummy values let forge compile the contracts for bytecode comparison in CI. +pragma solidity ^0.8.24; +address constant feesSenderToBurnerAddress = 0x0000000000000000000000000000000000000008; +address constant zamaOFTAddress = 0x0000000000000000000000000000000000000009; diff --git a/ci/stubs/host-contracts/FHEVMHostAddresses.sol b/ci/stubs/host-contracts/FHEVMHostAddresses.sol new file mode 100644 index 0000000000..1ed52d2f66 --- /dev/null +++ b/ci/stubs/host-contracts/FHEVMHostAddresses.sol @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: BSD-3-Clause-Clear +// Stub: the real addresses/ files are generated at deploy time and gitignored. +// These dummy values let forge compile the contracts for bytecode comparison in CI. +pragma solidity ^0.8.24; +address constant aclAdd = 0x0000000000000000000000000000000000000001; +address constant fhevmExecutorAdd = 0x0000000000000000000000000000000000000002; +address constant kmsVerifierAdd = 0x0000000000000000000000000000000000000003; +address constant inputVerifierAdd = 0x0000000000000000000000000000000000000004; +address constant hcuLimitAdd = 0x0000000000000000000000000000000000000005; +address constant pauserSetAdd = 0x0000000000000000000000000000000000000006; diff --git a/gateway-contracts/foundry.toml b/gateway-contracts/foundry.toml index c77611f5d9..f8c901533d 100644 --- a/gateway-contracts/foundry.toml +++ b/gateway-contracts/foundry.toml @@ -2,3 +2,10 @@ src = 'contracts' libs = ['node_modules'] solc = '0.8.24' +cbor_metadata = false +bytecode_hash = 'none' + +remappings = [ + '@openzeppelin/contracts/=node_modules/@openzeppelin/contracts/', + '@openzeppelin/contracts-upgradeable/=node_modules/@openzeppelin/contracts-upgradeable/', +] diff --git a/host-contracts/foundry.toml b/host-contracts/foundry.toml index 8e95a13d12..79268ce7f9 100644 --- a/host-contracts/foundry.toml +++ b/host-contracts/foundry.toml @@ -5,6 +5,8 @@ libs = ['node_modules', 'lib', "dependencies"] test = 'test' cache_path = 'cache_forge' solc = '0.8.24' +cbor_metadata = false +bytecode_hash = 'none' remappings = [ '@fhevm-foundry/=./fhevm-foundry/',