Skip to content

fix(ci): only run bytecode check on PRs targeting main#7802

Open
larryob wants to merge 2 commits intomainfrom
fix-bytecode-filter
Open

fix(ci): only run bytecode check on PRs targeting main#7802
larryob wants to merge 2 commits intomainfrom
fix-bytecode-filter

Conversation

@larryob
Copy link
Contributor

@larryob larryob commented Jan 15, 2026

Description

Added branch filter to bytecode-analysis workflow so it only runs on pull requests targeting the main branch.

🤖 Generated with Claude Code

Testing

Manual.
See #7803 (failing run) and #7804 (passing run)

Summary by CodeRabbit

  • Chores
    • Updated workflow configuration to run analysis checks on main branch pull requests.
    • Modified build configuration to adjust bytecode generation settings.

✏️ Tip: You can customize this high-level summary in your review settings.

Added branch filter to bytecode-analysis workflow so it only runs
on pull requests targeting the main branch.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@changeset-bot
Copy link

changeset-bot bot commented Jan 15, 2026

⚠️ No Changeset found

Latest commit: ffcc5f6

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-project-automation github-project-automation bot moved this to In Review in Hyperlane Tasks Jan 15, 2026
The ci profile sets cbor_metadata=false but was missing bytecode_hash.
When cbor_metadata is false, bytecode_hash must be "none" per the
Solidity compiler specification. This fixes the bytecode check workflow.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@larryob larryob marked this pull request as ready for review January 15, 2026 23:01
@larryob larryob requested review from ltyu and yorhodes as code owners January 15, 2026 23:01
@codecov
Copy link

codecov bot commented Jan 15, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.02%. Comparing base (d90525a) to head (ffcc5f6).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7802   +/-   ##
=======================================
  Coverage   77.02%   77.02%           
=======================================
  Files         117      117           
  Lines        2651     2651           
  Branches      244      244           
=======================================
  Hits         2042     2042           
  Misses        593      593           
  Partials       16       16           
Flag Coverage Δ
solidity 77.02% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
core 87.80% <ø> (ø)
hooks 71.86% <ø> (ø)
isms 81.10% <ø> (ø)
token 86.67% <ø> (ø)
middlewares 84.98% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@claude
Copy link
Contributor

claude bot commented Jan 15, 2026

Claude finished @larryob's task in 41s —— View job


PR Review: fix(ci): only run bytecode check on PRs targeting main

  • Analyze PR changes and context
  • Review workflow file changes for correctness
  • Review foundry.toml changes
  • Check for security implications
  • Provide summary and feedback

Summary

This PR makes two changes to improve the bytecode analysis CI workflow:

  1. Workflow branch filter (.github/workflows/bytecode-analysis.yml): Adds a branches: [main] filter so the bytecode check only runs on PRs targeting main, not on intermediate branches.

  2. Foundry config (solidity/foundry.toml): Adds bytecode_hash = "none" to the [profile.ci] section to ensure deterministic bytecode comparison.

Review Findings

✅ Workflow Changes - LGTM

The branch filter is correctly placed under the pull_request trigger:

on:
  pull_request:
    branches:
      - main
    paths:
      - 'solidity/**'

This correctly ensures the workflow only triggers when:

  • The event is a pull request AND
  • The PR targets main AND
  • Files in solidity/ or the workflow itself are changed

The workflow_dispatch trigger is preserved, allowing manual runs when needed.

✅ Foundry Config Changes - LGTM

Adding bytecode_hash = "none" alongside the existing cbor_metadata = false in the [profile.ci] section is the correct approach for deterministic bytecode comparison. This disables the metadata hash that Solidity appends to bytecode, which can vary between builds even with identical source code.

Both settings work together:

  • cbor_metadata = false - Disables CBOR-encoded metadata
  • bytecode_hash = "none" - Disables the metadata hash entirely

Security Analysis

No security concerns identified. This PR only modifies CI configuration:

  • No smart contract code changes
  • No changes to contract logic, access control, or message handling
  • The changes improve CI efficiency without affecting deployed contract behavior

Final Notes

Verdict: Approve

Comment on lines +5 to +6
branches:
- main
Copy link
Collaborator

Choose a reason for hiding this comment

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

would we not want to do this for audit branch PRs?

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 15, 2026

📝 Walkthrough

Walkthrough

Two simple configuration adjustments to ensure bytecode analysis runs under the right conditions. The workflow now filters to main branch PRs, and Foundry's CI profile gains a bytecode hash setting for deterministic builds.

Changes

Cohort / File(s) Summary
CI Workflow Trigger
.github/workflows/bytecode-analysis.yml
Added branch filter to pull_request trigger, restricting bytecode-analysis workflow execution to PRs targeting the main branch
Foundry Configuration
solidity/foundry.toml
Added bytecode_hash = "none" configuration option under [profile.ci] to control bytecode hash generation in CI environments

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • #7588: Modifies bytecode-analysis CI workflow and Foundry-related CI settings alongside this PR
  • #7500: Originally added the bytecode-analysis workflow; this PR refines its PR trigger conditions
  • #7702: Makes bytecode comparisons deterministic by adjusting the same [profile.ci] section in foundry.toml

Suggested reviewers

  • ltyu
  • paulbalaji

Poem

Like an ogre's careful layering, 🧅
Branch filters and config stack true,
Bytecode flows where it's meant to go,
Deterministic builds, not a hash askew! 🔨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding a branch filter to run bytecode checks only on PRs targeting main.
Description check ✅ Passed The description covers the main change and testing approach, though it's missing some optional template sections like drive-by changes and backward compatibility assessment.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-bytecode-filter


📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d90525a and ffcc5f6.

📒 Files selected for processing (2)
  • .github/workflows/bytecode-analysis.yml
  • solidity/foundry.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
  • GitHub Check: cli-evm-e2e-matrix (warp-deploy-2)
  • GitHub Check: aleo-sdk-e2e-run
  • GitHub Check: e2e-matrix (evm)
  • GitHub Check: aleo-sdk-e2e-run
  • GitHub Check: aleo-sdk-e2e-run
  • GitHub Check: cli-e2e
  • GitHub Check: aleo-sdk-e2e-run
  • GitHub Check: cli-e2e
  • GitHub Check: aleo-sdk-e2e-run
  • GitHub Check: cli-e2e
  • GitHub Check: aleo-sdk-e2e-run
  • GitHub Check: cli-e2e
  • GitHub Check: aleo-sdk-e2e-run
  • GitHub Check: aleo-sdk-e2e-run
  • GitHub Check: aleo-sdk-e2e-run
  • GitHub Check: aleo-sdk-e2e-run
🔇 Additional comments (2)
.github/workflows/bytecode-analysis.yml (1)

3-9: Get out of my swamp... unless you're heading to main.

The branch filter is properly nested under pull_request. Now the bytecode check only kicks in when PRs target main, which is what you want - no point checking bytecode differences for feature branches that ain't going anywhere important yet. The paths filter still works in tandem, so both conditions need to be true.

Manual runs via workflow_dispatch (line 10-15) remain available for ad-hoc needs, which is good for flexibility.

solidity/foundry.toml (1)

18-22: This is the right move for bytecode comparison.

When you set cbor_metadata = false, you've gotta pair it with bytecode_hash = "none". Otherwise the Solidity compiler gets grumpy and throws warnings. This config keeps your bytecode consistent and metadata-free, just like you wanted. All good here.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Comment on lines 4 to +6
pull_request:
branches:
- main
Copy link
Member

Choose a reason for hiding this comment

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

we do want this to run on all branches following #7801

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

3 participants