Skip to content

feat(contracts): add contract upgrade version check#2112

Merged
mergify[bot] merged 8 commits intomainfrom
upgrade-ci-detection-v2
Mar 16, 2026
Merged

feat(contracts): add contract upgrade version check#2112
mergify[bot] merged 8 commits intomainfrom
upgrade-ci-detection-v2

Conversation

@Eikix
Copy link
Copy Markdown
Contributor

@Eikix Eikix commented Mar 13, 2026

Summary

  • CI workflow + TypeScript script that detects when an upgradeable contract's compiled bytecode changes without required version bumps
  • Compares PR bytecode against the last deployed release (UPGRADE_FROM_TAG, currently v0.11.0) — not main — so multiple PRs touching the same contract between deployments only require one reinitializer bump
  • Enforces: REINITIALIZER_VERSION bump, reinitializeV{N-1}() function exists (not commented out), and MAJOR/MINOR/PATCH version bump
  • Flags spurious REINITIALIZER_VERSION bumps when bytecode is unchanged
  • Uses matrix strategy for both host-contracts and gateway-contracts

Changes

  1. host-contracts/foundry.toml, gateway-contracts/foundry.tomlcbor_metadata = false + bytecode_hash = 'none' for deterministic bytecode output
  2. ci/check-upgrade-versions.ts — Bun script: reads upgrade-manifest.json, extracts version constants from source, compares forge inspect deployedBytecode between baseline tag and PR
  3. ci/merge-address-constants.ts — Merges generated address constants from both baseline and PR so both sides compile. Needed because contracts may be added/removed between versions — generating addresses on only one side would leave the other unable to compile
  4. .github/workflows/contracts-upgrade-version-check.yml — Workflow with UPGRADE_FROM_TAG env var (same as *-upgrade-tests.yml), paths-filter, matrix strategy

Test plan

  • host-contracts version check passes
  • gateway-contracts version check passes (Decryption bump already on main, GatewayConfig unchanged)
  • Zizmor code scanning has no new findings

@cla-bot cla-bot bot added the cla-signed label Mar 13, 2026
@mergify
Copy link
Copy Markdown

mergify bot commented Mar 13, 2026

🧪 CI Insights

Here's what we observed from your CI run for 88bce8d.

🟢 All jobs passed!

But CI Insights is watching 👀

@Eikix
Copy link
Copy Markdown
Contributor Author

Eikix commented Mar 13, 2026

Local validation

Ran the script locally against host-contracts with 6 scenarios:

# Scenario Expected Result
1 No contract changes pass pass
2 Added external function, no version bumps fail: missing reinit + semver fail (2 errors)
3 Bumped versions but no reinitializeV3() fail: missing function fail (1 error)
4 Bumped versions + added reinitializeV3() pass pass
5 reinitializeV3() commented out with // fail: function not found fail (1 error)
6 Only bumped REINITIALIZER_VERSION, no semver or function fail: missing semver + function fail (2 errors)

Also confirmed: forge compilation errors are surfaced in CI output (not swallowed).

@zmalatrax
Copy link
Copy Markdown
Contributor

@claude /review

@Eikix Eikix marked this pull request as draft March 13, 2026 13:48
@Eikix
Copy link
Copy Markdown
Contributor Author

Eikix commented Mar 13, 2026

Live test

Pushed 6ef7673d — adds an external function to ACL without bumping versions. Expecting the hygiene check to fail with:

  • REINITIALIZER_VERSION was not bumped
  • semantic version was not bumped

Will revert once CI confirms.

@Eikix Eikix force-pushed the upgrade-ci-detection-v2 branch 3 times, most recently from 868e7fb to c08b695 Compare March 13, 2026 14:47
@Eikix Eikix marked this pull request as ready for review March 13, 2026 14:47
@Eikix Eikix requested review from a team as code owners March 13, 2026 14:47
@Eikix Eikix force-pushed the upgrade-ci-detection-v2 branch from 1ffb7c2 to 55562df Compare March 13, 2026 16:25
Eikix added 3 commits March 13, 2026 18:07
Add `cbor_metadata = false` and `bytecode_hash = 'none'` to both
host-contracts and gateway-contracts foundry.toml. This strips
compiler metadata from deployed bytecode, making bytecode comparison
across branches reliable for upgrade hygiene checks.

Also add OZ remappings to gateway-contracts for Foundry compatibility.
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).
Compare PR bytecode against UPGRADE_FROM_TAG (v0.11.0) instead of main.
This avoids unnecessary reinitializer bumps when multiple PRs modify the
same contract between deployments — only the first change after a release
requires a bump. Keeps the tag in sync with *-upgrade-tests.yml workflows.
@Eikix Eikix force-pushed the upgrade-ci-detection-v2 branch from 55562df to 9231fe5 Compare March 13, 2026 17:21
Eikix added 3 commits March 16, 2026 10:28
When comparing against the v0.11.0 baseline, forge tries to compile all
.sol files including ones that were deleted in the PR (e.g.
MultichainACLChecks.sol). These reference removed address constants and
break compilation of unrelated contracts like GatewayConfig, causing
false positives. Strip deleted files from the baseline before compiling.
When forge can't compile the baseline (e.g. deleted contracts break
unrelated imports), compare source files directly instead of blindly
treating all contracts as changed. If the contract source is identical
between baseline and PR, bytecode is unchanged — no version bump needed.

Fixes false positive for GatewayConfig which is unchanged since v0.11.0
but couldn't be compiled because MultichainACLChecks.sol (deleted in PR)
was still imported by other v0.11.0 files.
When comparing bytecode between the baseline tag and the PR, both sides
must compile with identical address constants (they're embedded in
bytecode).  Previously we generated addresses only on the PR side and
copied them to the baseline.  This broke when contracts were removed
between versions — the baseline still had source files importing the
deleted constant, causing forge compilation to fail for the entire
project, including unrelated unchanged contracts.

Add ci/merge-address-constants.ts: generates addresses on both sides
independently, then merges them.  PR values win for shared constants,
baseline-only constants (removed contracts) are preserved so the
baseline compiles, and PR-only constants (new contracts) are preserved
so the PR compiles.

Reverts the source-comparison fallback in check-upgrade-hygiene.ts —
both sides now compile successfully so we always compare real bytecode.
@Eikix Eikix changed the title feat(contracts): add contract upgrade hygiene check feat(contracts): add contract upgrade version check Mar 16, 2026
Eikix added 2 commits March 16, 2026 10:58
Rename workflow, script, job names, and all references from the
ambiguous "upgrade hygiene" to the descriptive "upgrade version check".
Copy link
Copy Markdown
Contributor

@zmalatrax zmalatrax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Eikix
Copy link
Copy Markdown
Contributor Author

Eikix commented Mar 16, 2026

@Mergifyio queue

@mergify
Copy link
Copy Markdown

mergify bot commented Mar 16, 2026

Merge Queue Status

This pull request spent 2 hours 45 minutes 29 seconds in the queue, including 1 hour 44 minutes 59 seconds running CI.

Required conditions to merge
  • #approved-reviews-by >= 1 [🛡 GitHub branch protection]
  • #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
  • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
  • branch-protection-review-decision = APPROVED [🛡 GitHub branch protection]
  • check-success = run-e2e-tests / fhevm-e2e-test
  • any of [🛡 GitHub branch protection]:
    • check-success = common-pull-request/lint (bpr)
    • check-neutral = common-pull-request/lint (bpr)
    • check-skipped = common-pull-request/lint (bpr)
  • any of [🛡 GitHub branch protection]:
    • check-skipped = coprocessor-cargo-listener-tests/cargo-tests (bpr)
    • check-neutral = coprocessor-cargo-listener-tests/cargo-tests (bpr)
    • check-success = coprocessor-cargo-listener-tests/cargo-tests (bpr)
  • any of [🛡 GitHub branch protection]:
    • check-success = coprocessor-cargo-test/cargo-tests (bpr)
    • check-neutral = coprocessor-cargo-test/cargo-tests (bpr)
    • check-skipped = coprocessor-cargo-test/cargo-tests (bpr)
  • any of [🛡 GitHub branch protection]:
    • check-success = coprocessor-dependency-analysis/dependencies-check (bpr)
    • check-neutral = coprocessor-dependency-analysis/dependencies-check (bpr)
    • check-skipped = coprocessor-dependency-analysis/dependencies-check (bpr)
  • any of [🛡 GitHub branch protection]:
    • check-success = gateway-contracts-deployment-tests/sc-deploy (bpr)
    • check-neutral = gateway-contracts-deployment-tests/sc-deploy (bpr)
    • check-skipped = gateway-contracts-deployment-tests/sc-deploy (bpr)
  • any of [🛡 GitHub branch protection]:
    • check-skipped = kms-connector-tests/test-connector (bpr)
    • check-neutral = kms-connector-tests/test-connector (bpr)
    • check-success = kms-connector-tests/test-connector (bpr)

mergify bot added a commit that referenced this pull request Mar 16, 2026
@mergify mergify bot removed the merge-queued label Mar 16, 2026
@mergify mergify bot merged commit e561ebd into main Mar 16, 2026
66 checks passed
@mergify mergify bot deleted the upgrade-ci-detection-v2 branch March 16, 2026 13:43
@mergify mergify bot removed the queued label Mar 16, 2026
dartdart26 pushed a commit that referenced this pull request Mar 16, 2026
* feat(contracts): enable deterministic bytecode output

Add `cbor_metadata = false` and `bytecode_hash = 'none'` to both
host-contracts and gateway-contracts foundry.toml. This strips
compiler metadata from deployed bytecode, making bytecode comparison
across branches reliable for upgrade hygiene checks.

Also add OZ remappings to gateway-contracts for Foundry compatibility.

* 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).

* feat(ci): compare bytecode against last deployed release tag

Compare PR bytecode against UPGRADE_FROM_TAG (v0.11.0) instead of main.
This avoids unnecessary reinitializer bumps when multiple PRs modify the
same contract between deployments — only the first change after a release
requires a bump. Keeps the tag in sync with *-upgrade-tests.yml workflows.

* fix(ci): remove deleted source files from baseline before compiling

When comparing against the v0.11.0 baseline, forge tries to compile all
.sol files including ones that were deleted in the PR (e.g.
MultichainACLChecks.sol). These reference removed address constants and
break compilation of unrelated contracts like GatewayConfig, causing
false positives. Strip deleted files from the baseline before compiling.

* fix(ci): fall back to source comparison when baseline compilation fails

When forge can't compile the baseline (e.g. deleted contracts break
unrelated imports), compare source files directly instead of blindly
treating all contracts as changed. If the contract source is identical
between baseline and PR, bytecode is unchanged — no version bump needed.

Fixes false positive for GatewayConfig which is unchanged since v0.11.0
but couldn't be compiled because MultichainACLChecks.sol (deleted in PR)
was still imported by other v0.11.0 files.

* fix(ci): merge address constants from both baseline and PR

When comparing bytecode between the baseline tag and the PR, both sides
must compile with identical address constants (they're embedded in
bytecode).  Previously we generated addresses only on the PR side and
copied them to the baseline.  This broke when contracts were removed
between versions — the baseline still had source files importing the
deleted constant, causing forge compilation to fail for the entire
project, including unrelated unchanged contracts.

Add ci/merge-address-constants.ts: generates addresses on both sides
independently, then merges them.  PR values win for shared constants,
baseline-only constants (removed contracts) are preserved so the
baseline compiles, and PR-only constants (new contracts) are preserved
so the PR compiles.

Reverts the source-comparison fallback in check-upgrade-hygiene.ts —
both sides now compile successfully so we always compare real bytecode.

* refactor(ci): rename upgrade hygiene to upgrade version check

Rename workflow, script, job names, and all references from the
ambiguous "upgrade hygiene" to the descriptive "upgrade version check".

* chore: remove accidentally committed plan file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants