chore: improve ci workflows#1113
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughIntroduces NOIR_TOOLCHAIN env var in CI, replaces hard-coded Noir toolchain references, removes some early Rust setup steps, adds a new build_crisp_sdk job and artifact handling, updates job dependencies (crisp_unit, crisp_e2e), and simplifies CRISP SDK test and e2e scripts. Changes
Sequence DiagramsequenceDiagram
participant GHA as GitHub Actions
participant BuildSDK as build_crisp_sdk (Job)
participant BuildEnclave as build_enclave_cli (Job)
participant Unit as crisp_unit (Job)
participant E2E as crisp_e2e (Job)
participant Artifacts as Artifact Storage
GHA->>BuildSDK: trigger (uses NOIR_TOOLCHAIN)
BuildSDK->>BuildSDK: build SDK, compile circuits
BuildSDK->>Artifacts: upload crisp-sdk-artifacts
GHA->>BuildEnclave: trigger
BuildEnclave->>Artifacts: upload enclave-artifacts
GHA->>Unit: trigger (needs: build_crisp_sdk)
Artifacts->>Unit: download crisp-sdk-artifacts
Unit->>Unit: run tests (vitest --run)
Unit->>Artifacts: upload unit artifacts
GHA->>E2E: trigger (needs: build_enclave_cli, build_crisp_sdk)
Artifacts->>E2E: download SDK & enclave artifacts
E2E->>E2E: prepare env, run dev.sh and e2e tests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (8)📓 Common learnings📚 Learning: 2025-08-27T13:49:03.811ZApplied to files:
📚 Learning: 2025-10-10T12:56:40.538ZApplied to files:
📚 Learning: 2024-10-23T01:59:42.967ZApplied to files:
📚 Learning: 2024-10-23T01:59:27.215ZApplied to files:
📚 Learning: 2024-10-23T02:03:02.008ZApplied to files:
📚 Learning: 2025-11-05T14:12:57.814ZApplied to files:
📚 Learning: 2025-08-25T10:28:56.174ZApplied to files:
⏰ 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). (7)
🔇 Additional comments (4)
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. Comment |
cedoor
changed the title
cedoor
force-pushed
the
ci/improvements-3
branch
from
5b3f86a to
1b30fd0
Compare
cedoor
force-pushed
the
ci/improvements-3
branch
from
c26295d to
225c994
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/ci.yml (2)
24-24: Consider using "stable" toolchain instead of pinned version.Based on learnings from this repo, the preference has been to use "stable" toolchain with
noir-lang/noiruprather than pinning to specific versions. If there's a specific reason for pinning tov1.0.0-beta.15(e.g., compatibility requirements with the circuits), please document it in a comment.
736-736: Consider usingerrorinstead ofwarnfor artifact upload.Using
if-no-files-found: warncould mask build failures. If the SDK build fails to produce artifacts, downstream jobs (crisp_unit,crisp_e2e) will fail with unclear errors when trying to download missing artifacts.- if-no-files-found: warn + if-no-files-found: error
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/ci.yml(10 hunks)examples/CRISP/packages/crisp-sdk/package.json(1 hunks)examples/CRISP/scripts/test_e2e.sh(1 hunks)
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: 0xjei
Repo: gnosisguild/enclave PR: 648
File: .github/workflows/ci.yml:202-206
Timestamp: 2025-08-27T13:49:03.811Z
Learning: In GitHub Actions workflows for Noir circuits, the user prefers to use "stable" toolchain with noir-lang/noirup action rather than pinning to specific versions like 1.0.0-beta.11, and prefers to update the noirup action version (e.g., to 0.1.4) to get tooling improvements.
📚 Learning: 2024-09-26T04:26:00.779Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 107
File: tests/basic_integration/test.sh:142-142
Timestamp: 2024-09-26T04:26:00.779Z
Learning: In the `tests/basic_integration/test.sh` script, using fixed sleep durations is acceptable, and adding more complex waiting mechanisms may introduce unnecessary complexity for the test.
Applied to files:
examples/CRISP/scripts/test_e2e.sh
📚 Learning: 2025-10-29T23:35:30.146Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 936
File: scripts/run-crisp-test.sh:1-3
Timestamp: 2025-10-29T23:35:30.146Z
Learning: In the scripts/run-crisp-test.sh file, the use of `rm -rf *` is acceptable as it's intentionally designed as a definitive reset-and-test script for clean checkouts.
Applied to files:
examples/CRISP/scripts/test_e2e.sh
📚 Learning: 2024-09-26T04:12:09.345Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 107
File: tests/basic_integration/test.sh:103-114
Timestamp: 2024-09-26T04:12:09.345Z
Learning: In `tests/basic_integration/test.sh`, the user prefers not to refactor the ciphernode addition section to reduce duplication.
Applied to files:
examples/CRISP/scripts/test_e2e.sh
📚 Learning: 2024-09-26T04:15:32.544Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 107
File: tests/basic_integration/test.sh:81-83
Timestamp: 2024-09-26T04:15:32.544Z
Learning: In `tests/basic_integration/test.sh`, it's acceptable to wait indefinitely for the EVM node to start without a timeout, as it's unlikely to fail here.
Applied to files:
examples/CRISP/scripts/test_e2e.sh
📚 Learning: 2025-08-27T13:49:03.811Z
Learnt from: 0xjei
Repo: gnosisguild/enclave PR: 648
File: .github/workflows/ci.yml:202-206
Timestamp: 2025-08-27T13:49:03.811Z
Learning: In GitHub Actions workflows for Noir circuits, the user prefers to use "stable" toolchain with noir-lang/noirup action rather than pinning to specific versions like 1.0.0-beta.11, and prefers to update the noirup action version (e.g., to 0.1.4) to get tooling improvements.
Applied to files:
.github/workflows/ci.yml
📚 Learning: 2025-10-10T12:56:40.538Z
Learnt from: 0xjei
Repo: gnosisguild/enclave PR: 830
File: templates/default/README.md:123-128
Timestamp: 2025-10-10T12:56:40.538Z
Learning: In the Enclave repository, the hard-coded Hardhat development private key `0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80` is acceptable in template README files and documentation for local testing/interaction purposes.
Applied to files:
.github/workflows/ci.yml
📚 Learning: 2024-10-23T01:59:42.967Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 156
File: packages/ciphernode/tests/tests/test_aggregation_and_decryption.rs:274-274
Timestamp: 2024-10-23T01:59:42.967Z
Learning: In the `packages/ciphernode/tests/tests/test_aggregation_and_decryption.rs` file and other test files within this project, hardcoding `CIPHERNODE_SECRET` is acceptable for testing purposes.
Applied to files:
.github/workflows/ci.yml
📚 Learning: 2024-10-23T01:59:27.215Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 156
File: tests/basic_integration/test.sh:21-21
Timestamp: 2024-10-23T01:59:27.215Z
Learning: In `tests/basic_integration/test.sh`, the hardcoded `CIPHERNODE_SECRET` is acceptable for testing purposes and does not need to be changed.
Applied to files:
.github/workflows/ci.yml
📚 Learning: 2024-10-23T02:03:02.008Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 156
File: packages/ciphernode/keyshare/src/encryption.rs:45-45
Timestamp: 2024-10-23T02:03:02.008Z
Learning: In the `packages/ciphernode/keyshare/src/encryption.rs` file, the environment variable `CIPHERNODE_SECRET` is used for the encryption password. A secure secret management solution is not currently available, but may be considered in future iterations.
Applied to files:
.github/workflows/ci.yml
📚 Learning: 2025-11-05T14:12:57.814Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 963
File: examples/CRISP/client/package.json:25-25
Timestamp: 2025-11-05T14:12:57.814Z
Learning: In the Enclave/CRISP codebase, `enclave-e3/sdk` and `crisp-e3/sdk` are different packages: `enclave-e3/sdk` is the general Enclave SDK, while `crisp-e3/sdk` is the CRISP-specific SDK. The CRISP client (`examples/CRISP/client`) intentionally depends on `enclave-e3/sdk`, not `crisp-e3/sdk`.
Applied to files:
.github/workflows/ci.yml
📚 Learning: 2025-08-25T10:28:56.174Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 657
File: Cargo.toml:32-34
Timestamp: 2025-08-25T10:28:56.174Z
Learning: The examples/CRISP directory has its own Cargo.toml workspace configuration with members like "server", "wasm-crypto", "program/core", "program/client", etc. The root workspace intentionally excludes "examples/CRISP/server", "examples/CRISP/program", and "examples/CRISP/wasm-crypto" to prevent double workspace membership, which is the correct approach for self-contained example workspaces.
Applied to files:
.github/workflows/ci.yml
🔇 Additional comments (6)
examples/CRISP/scripts/test_e2e.sh (1)
17-17: LGTM! Simplified concurrent execution aligns with the new artifact-based workflow.The removal of
setup.shfrom the parallel execution is consistent with the CI changes where thebuild_crisp_sdkjob now handles SDK building separately. This simplifies the E2E test script by relying on pre-built artifacts.Note that when running locally, developers must ensure the SDK artifacts exist (via
pnpm build:sdkor similar) before executing this script.examples/CRISP/packages/crisp-sdk/package.json (1)
27-27: LGTM! Test script simplified to assume pre-built artifacts.The removal of build steps from the test script is appropriate given the CI workflow now builds artifacts separately in the
build_crisp_sdkjob. For local development, developers should runpnpm build(line 26) before running tests..github/workflows/ci.yml (4)
408-410: LGTM! Job dependency correctly updated.The
crisp_unitjob now properly depends onbuild_crisp_sdk, ensuring artifacts are available before tests run.
481-483: LGTM! Job dependencies correctly updated for E2E tests.The
crisp_e2ejob now depends on bothbuild_enclave_cliandbuild_crisp_sdk, ensuring all required binaries and SDK artifacts are available.
463-467: LGTM! Artifact download step correctly configured.The download path (
./examples/CRISP) matches the artifact upload paths frombuild_crisp_sdk, ensuring files are restored to the correct locations.
723-725: Thepnpm build:sdkscript exists but only builds the crisp-sdk package.The script in
examples/CRISP/package.jsonis defined as"pnpm -C packages/crisp-sdk build". This only builds the crisp-sdk package in isolation. Circuits are compiled separately via thecompile:circuitsscript, not as part ofbuild:sdk.Likely an incorrect or invalid review comment.
2 participants
Updates CI to pin the Noir toolchain, add a dedicated CRISP SDK build + artifact upload job, and make CRISP unit/E2E tests reuse those artifacts (and drop unnecessary installs), plus a small cleanup to the CRISP E2E test runner script.
Saved around 2/3 mins.
Summary by CodeRabbit
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.