Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 85 additions & 0 deletions .github/workflows/minimize-pragma.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
# Keeps Solidity pragma floors minimal (`npm run pragma`), opening a PR when any can be lowered.
#
# SETUP (one-time, org admin): a short-lived GitHub App token is minted so the opened PR triggers CI
# (the default GITHUB_TOKEN can't — events it creates don't start workflow runs). Create a GitHub App
# with Contents + Pull requests set to Read-and-write, install it on this repo, and add two secrets:
# PRAGMA_BOT_APP_ID and PRAGMA_BOT_PRIVATE_KEY. Reuse an existing org CI-bot App if one exists.
name: minimize-pragma

on:
schedule:
- cron: '0 6 * * 1' # Mondays 06:00 UTC
workflow_dispatch: {}

concurrency:
group: minimize-pragma
cancel-in-progress: false

permissions:
contents: read # checkout only; the App token pushes the branch and opens the PR

env:
NODE_OPTIONS: --max_old_space_size=8192
DEFAULT_BRANCH: master

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit: ${{ github.event.repository.default_branch }} avoids hardcoding master.


jobs:
minimize-pragma:
runs-on: ubuntu-latest

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No timeout-minutes: npm run pragma compiles every file against every candidate solc, so a stalled solc download would hold a runner for the default 6h (and with cancel-in-progress: false a manual dispatch would queue behind it).

if: github.event_name == 'workflow_dispatch' || github.repository == 'OpenZeppelin/openzeppelin-contracts'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why disable the cron if we aren't in vanilla? I'd like to symlink this into confidential and have the same behavior.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes I thought about that but I wanted something pretty scoped for this first try. If updated here we want to skip if upgradeable repo or other vanilla forks which didn't opt-in for that.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I believe that forks automatically have actions disabled. Upgradeable also has actions disabled. If a dev has explicitly enabled actions, I don't see why there would be an issue with running this weekly.

steps:
- uses: actions/checkout@v7
Comment thread
coderabbitai[bot] marked this conversation as resolved.
with:
ref: ${{ env.DEFAULT_BRANCH }}
submodules: recursive # foundry libs, needed by the pragma tooling
# Read-only checkout: don't persist the token. The write-capable App token is minted after
# the build (below), so nothing usable sits in .git/config while untrusted deps run.
persist-credentials: false
- name: Set up environment
uses: ./.github/actions/setup
with:
foundry: 'on' # needed by the pragma scripts (see checks.yml)

- name: Minimize pragmas
run: npm run pragma

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit: might be worth running npm run test:pragma after this, as a sanity check before opening the PR, so a bad output fails here rather than in the PR's CI.


- name: Detect changes
id: diff
continue-on-error: true # exits 1 when pragmas changed
run: git diff --quiet -- contracts

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

When there's no diff the job exits early, but it doesn't close an already-open bot PR. If the pragmas get fixed manually on master (as in #6825) while the bot PR is open, that PR stays around with stale/conflicting changes and could get merged by mistake. Maybe close it (gh pr close) in a step gated on steps.diff.outcome == 'success'.

Also, git diff --quiet ignores untracked files, but git add contracts (L74) stages them. git add -u contracts would keep the two consistent.


# Minted after the build and only when there's a PR to open, so build deps never see it.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

"build deps never see it" isn't really true: npm ci / the hardhat build already ran dependency code on this same runner. A malicious postinstall can drop a .git/hooks/pre-push, set a credential helper in ~/.gitconfig, or put a git/gh shim on PATH, and it will be invoked later with GH_TOKEN in the env and in the remote URL. persist-credentials: false doesn't help with that.

To actually isolate the token, split into two jobs: one that runs npm run pragma and uploads the diff (git diff > pragma.patch) as an artifact, and one that never runs npm ci, applies the patch on a fresh checkout, mints the token and pushes.

# Scoped to the two permissions needed, not everything the (possibly shared) App installation grants.
- name: Generate app token
if: steps.diff.outcome == 'failure'
id: app-token
uses: actions/create-github-app-token@v2
with:
app-id: ${{ secrets.PRAGMA_BOT_APP_ID }}
private-key: ${{ secrets.PRAGMA_BOT_PRIVATE_KEY }}
permission-contents: write # push the automation branch
permission-pull-requests: write # open/update the PR

- name: Open or update PR
if: steps.diff.outcome == 'failure'
env:
GH_TOKEN: ${{ steps.app-token.outputs.token }} # App token: opens the PR and lets its CI run
APP_SLUG: ${{ steps.app-token.outputs.app-slug }}
BRANCH: automation/minimize-pragma
BASE: ${{ env.DEFAULT_BRANCH }}
run: |
uid="$(gh api "/users/${APP_SLUG}[bot]" --jq .id)"
git config user.name "${APP_SLUG}[bot]"
git config user.email "${uid}+${APP_SLUG}[bot]@users.noreply.github.com"
git switch -c "$BRANCH"
git add contracts
git commit -m "chore: minimize Solidity pragmas"
# Auth via the token in the remote URL, not persisted git credentials.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Rather than embedding the token in the remote URL (where it ends up on the git command line, visible in /proc, and possibly echoed in error output), gh auth setup-git + a plain git push origin would reuse the GH_TOKEN that's already exported.

git push --force "https://x-access-token:${GH_TOKEN}@github.com/${GITHUB_REPOSITORY}.git" "$BRANCH"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Two issues with the unconditional force-push:

  • The branch is recreated from master every run, so any commit a maintainer pushed to the open PR (e.g. reverting one file whose lowered pragma we don't want) gets silently wiped and the rejected change comes back.
  • Even when the diff is identical to what's already on the branch, the new commit has a new SHA, so the whole CI suite reruns weekly and existing approvals are dismissed.

Suggest comparing against origin/$BRANCH first (git fetch origin "$BRANCH" && git diff --quiet "origin/$BRANCH" -- contracts) and skipping the push when nothing changed.

if [ -n "$(gh pr list --head "$BRANCH" --base "$BASE" --state open --json number --jq '.[0].number')" ]; then

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Only --state open is checked, so if we deliberately close the bot PR, it gets recreated the following Monday. Probably worth checking for a closed PR on the same branch (or a label) and bailing out in that case.

echo "PR already open for $BRANCH -> $BASE; the force-push updated it."
else
gh pr create --base "$BASE" --head "$BRANCH" \
--label ignore-changeset \
--title "chore: minimize Solidity pragmas" \
--body "Automated \`npm run pragma\`: lowers pragma floors to the lowest compiler that still builds each file."
fi
Loading