Skip to content

Conversation

@Vishalkulkarni45
Copy link
Collaborator

@Vishalkulkarni45 Vishalkulkarni45 commented Oct 8, 2025

Summary by CodeRabbit

  • New Features

    • Added support for a new DSC circuit variant: SHA-384 RSA-PSS (65537, 3072).
  • Refactor

    • Circuit build now resolves dependencies from an isolated circuits dependency location for improved isolation and stability.
  • Chores

    • CI workflow can download build artifacts by specifying a run ID.
    • CI permissions updated to allow artifact access and use a new artifact-download action.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 8, 2025

Walkthrough

Adds a run-id workflow_dispatch input and actions: read permission to the circuits-build workflow, switches artifact download to dawidd6/action-download-artifact@v6 conditioned on run-id, updates Circom include paths to circuits/node_modules, and adds a new DSC circuit variant.

Changes

Cohort / File(s) Summary
GitHub Actions: circuits build workflow
.github/workflows/circuits-build.yml
Added workflow_dispatch input run-id (string, default ''), granted actions: read permission to the build job, replaced artifact download step with dawidd6/action-download-artifact@v6 using run_id: ${{ inputs.run-id }} and made download conditional on inputs.run-id != ''.
Circom build scripts: include paths and DSC entries
circuits/scripts/build/build_cpp.sh, circuits/scripts/build/build_single_circuit.sh, circuits/scripts/build/build_r1cs_wasm.sh
Switched Circom include paths from top-level node_modules to circuits/node_modules (including @zk-kit and circomlib paths). Added new DSC circuit entry dsc_sha384_rsapss_65537_48_3072 (and ...:true variant where present).

Sequence Diagram(s)

sequenceDiagram
    actor Dev as Developer
    participant GH as GitHub Actions
    participant Job as build job
    participant DL as dawidd6/download-artifact@v6
    participant Art as Artifact Storage

    Dev->>GH: Trigger workflow (workflow_dispatch run-id)
    GH->>Job: Start build (permissions: actions:read)
    alt run-id provided (not '')
        Job->>DL: request download with run_id
        DL->>Art: fetch artifacts for run_id
        Art-->>DL: return artifacts
        DL-->>Job: write artifacts to workspace
    else run-id empty
        Job-->>Job: skip artifact download
    end
    Job->>Job: run Circom build (includes from circuits/node_modules)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • Nesopie

Poem

A run-id set, the CI breathes light,
Artifacts fall into place by night.
Circom finds its modules near,
A new DSC joins the sphere.
Builds hum on — small sparks, bright.

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title Check ❓ Inconclusive The title “Custom build circuit” is too generic to convey the pull request’s key changes such as adding a run-id input to the build workflow, updating permissions, adjusting Circom include paths, and registering a new DSC circuit entry. It does not specify the primary enhancements implemented, making it unclear to reviewers what the main focus of the changeset is. Because of its lack of specificity, the title does not accurately summarize the most important updates. Please revise the title to clearly reflect the core modifications, for example “Add run-id input to build workflow and register new DSC circuit in build scripts,” so that the pull request’s main changes are immediately apparent.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/serbia-dsc-build

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 907fa88 and 9916cb9.

📒 Files selected for processing (3)
  • circuits/scripts/build/build_cpp.sh (2 hunks)
  • circuits/scripts/build/build_r1cs_wasm.sh (1 hunks)
  • circuits/scripts/build/build_single_circuit.sh (2 hunks)
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: run_circuit_tests

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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5d63c60 and 907fa88.

📒 Files selected for processing (3)
  • .github/workflows/circuits-build.yml (3 hunks)
  • circuits/scripts/build/build_cpp.sh (2 hunks)
  • circuits/scripts/build/build_single_circuit.sh (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.github/workflows/**/*.{yml,yaml}

📄 CodeRabbit inference engine (AGENTS.md)

.github/workflows/**/*.{yml,yaml}: In GitHub workflows, use the shared composite actions in .github/actions for dependency caching instead of calling actions/cache directly
Use the cache-yarn composite action for Yarn dependency caching in workflows
Use the cache-bundler composite action for Ruby gems caching in workflows
Use the cache-gradle composite action for Gradle caching in workflows
Use the cache-pods composite action for CocoaPods caching in workflows

Files:

  • .github/workflows/circuits-build.yml
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: run_circuit_tests

Comment on lines +23 to +27
run-id:
description: 'Run ID to download artifacts .'
required: false
type: string
default: ''
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Fix invalid expression for run-id input.

inputs.run-id is parsed as inputs.run - id, so the condition will never behave correctly and the download step passes an empty run ID. Use bracket notation for hyphenated workflow inputs.

-      run-id:
+      run-id:
         description: 'Run ID to download artifacts .'
         required: false
         type: string
         default: ''
…
-      - name: Download previous artifacts
-        if: github.event_name == 'workflow_dispatch' && inputs.run-id != ''
+      - name: Download previous artifacts
+        if: github.event_name == 'workflow_dispatch' && inputs['run-id'] != ''
         uses: dawidd6/action-download-artifact@v6
         with:
           name: circuits
           path: output/
-          run_id: ${{ inputs.run-id }}
+          run_id: ${{ inputs['run-id'] }}

Also applies to: 160-166

🤖 Prompt for AI Agents
In .github/workflows/circuits-build.yml around lines 23 to 27 (and also apply
the same change at lines 160-166), the workflow references the hyphenated input
as inputs.run-id which is parsed as inputs.run - id and yields incorrect/empty
values; update all expressions that reference this input to use bracket notation
inputs['run-id'] (and any other hyphenated input names) so the condition and
artifact download receive the correct run ID.

@Nesopie Nesopie merged commit c5d0e5d into dev Oct 8, 2025
14 checks passed
@Nesopie Nesopie deleted the feat/serbia-dsc-build branch October 8, 2025 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants