Build/poland reg

[Vishalkulkarni45]

Summary by CodeRabbit

• Chores
  • Expanded circuit registration support to include a new identity circuit configuration combining SHA512, SHA256, and RSA PSS encryption with specific cryptographic parameters.

[coderabbitai]

Walkthrough

Two build scripts are updated to register a new circuit identifier register_id_sha512_sha512_sha256_rsapss_65537_32_2048 in their respective REGISTER_ID_CIRCUITS arrays, enabling this circuit to be processed during build operations.

Changes

Cohort / File(s)	Summary
Register ID Circuits Configuration circuits/scripts/build/build_cpp.sh, circuits/scripts/build/build_single_circuit.sh	Adds new circuit identifier entry register_id_sha512_sha512_sha256_rsapss_65537_32_2048 to REGISTER_ID_CIRCUITS arrays (with :true flag in build_cpp.sh); expands supported circuit registrations

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

• selfxyz/self#682: Modifies the same REGISTER_ID_CIRCUITS configuration to add register_id circuit instances
• selfxyz/self#683: Introduces and populates the register_id build mode and circuit arrays in the same build scripts
• selfxyz/self#1028: Directly conflicts—removes the same register_id_sha512_sha512_sha256_rsapss_65537_32_2048 entry from REGISTER_ID_CIRCUITS

Suggested reviewers

• Nesopie

Poem

🔧 A circuit joins the roster bright,
SHA-512 and RSA in flight,
Two scripts now know this ID well,
Building circuits—time will tell! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title Check	❓ Inconclusive	The pull request title "Build/poland reg" is vague and uses non-descriptive terminology that obscures the actual changes. While "Build" does connect to the modified build scripts and "reg" likely refers to the register-related circuit additions, the title lacks clarity about the specific change being made. The term "poland" is unexplained and cryptic, and the abbreviation "reg" is ambiguous—a teammate scanning the repository history would not understand that this PR specifically adds a new circuit registration entry for SHA512/SHA256/RSA-PSS algorithm combinations to the build system.	Consider revising the title to be more explicit and descriptive, such as "Add SHA512/SHA256/RSA-PSS circuit registration to build scripts" or similar. This would clearly communicate the primary change to reviewers without requiring them to examine the diff. If "poland" is an internal project reference or code name, consider providing context or using more standard terminology to ensure the title is self-explanatory.

✅ 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 build/poland-reg

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

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 5d04870 and 944cbba.

📒 Files selected for processing (2)

• circuits/scripts/build/build_cpp.sh (1 hunks)
• circuits/scripts/build/build_single_circuit.sh (1 hunks)

🔇 Additional comments (1)

circuits/scripts/build/build_single_circuit.sh (1)

108-109: Approve code change: circuit instance file verified
The file circuits/circuits/register_id/instances/register_id_sha512_sha512_sha256_rsapss_65537_32_2048.circom exists and the addition is correct.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Enablement looks good; fix silent-failure risk in parallel build.

With more enabled circuits, a failed compile still yields “All circuits compiled successfully!” because only the last wait’s status is surfaced. Make the script fail fast if any job fails.

Apply this change near the end to aggregate exit codes:

-echo "Waiting for all circuits to compile..."
-wait "${pids[@]}"
-echo "All circuits compiled successfully!"
+echo "Waiting for all circuits to compile..."
+failures=0
+for pid in "${pids[@]}"; do
+  if ! wait "$pid"; then
+    echo "❌ Compile process $pid failed"
+    failures=$((failures+1))
+  fi
+done
+if (( failures > 0 )); then
+  echo "❌ $failures circuit(s) failed to compile"
+  exit 1
+fi
+echo "✅ All circuits compiled successfully!"

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents

In circuits/scripts/build/build_cpp.sh around lines 72-73, the script starts
parallel compile jobs but only checks the last wait status, causing a false "All
circuits compiled successfully!" when earlier jobs fail; modify the end to
record PIDs of all background jobs, loop over them calling wait PID and collect
each exit code, and if any wait returns non-zero exit immediately (or after
aggregating) with a non-zero exit status and do not print the success message;
optionally add a trap to kill child jobs on early exit.

[shazarre]

🇵🇱

[Nesopie]

lgtm