perf: optimize Rust build times — eliminate C build bottlenecks and ark-ff duplicate compilation#2356
Open
ananas-block wants to merge 8 commits intomainfrom
Open
perf: optimize Rust build times — eliminate C build bottlenecks and ark-ff duplicate compilation#2356ananas-block wants to merge 8 commits intomainfrom
ananas-block wants to merge 8 commits intomainfrom
Conversation
Install libzstd-dev on CI and set ZSTD_SYS_USE_PKG_CONFIG=1 so zstd-sys uses pkg-config to find the system library instead of compiling from C source. zstd-sys (~7.9s) is a transitive dep via Solana's reqwest 0.11 and cannot be removed from the dep graph, but the C build can be avoided with a system library present. Also includes lld, clang, and libssl-dev from previous optimization work (already in the working tree from the prior plan). Entire-Checkpoint: 549cca13d530
…rofile Add [profile.dev.package.*] overrides for syn, proc-macro2, quote, serde_derive, ark-ff-macros, and ark-ff-asm. These crates execute at build time; compiling them at full optimization reduces the time they spend generating code for downstream crates.
Patch groth16-solana via [patch.crates-io] with the local version which: - Uses ark-ff 0.5 (same as workspace) instead of 0.4 - Bumps solana-bn254 from 2.x to 3.x (which uses ark 0.5) - Sets default-features = false for ark/thiserror/serde deps Also bump workspace solana-bn254 from "2.2" to "3.2.1" to match. Fix typo in 104 light-verifier verifying_keys files: vk_gamme_g2 -> vk_gamma_g2 (the typo fix was shipped in groth16-solana PR #29). TODO: replace path patch with git dep once groth16-solana changes are pushed and merged (branch: jorrit/chore-bump-deps, commit: 4e6cacf).
Replace local path patch with git dep pointing to the pushed commit, making the ark-ff 0.4 elimination work on CI.
Contributor
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughMass rename of a misspelled struct field ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
solana-bn254 v3 deprecated the unversioned compress/decompress functions
in favour of explicit _be variants. Update prover/client/src/proof.rs to
use alt_bn128_g{1,2}_{compress,decompress}_be.
Also fix the vk_gamme_g2 -> vk_gamma_g2 typo in xtask/src/create_vkeyrs_from_gnark_key.rs
which was missed in the bulk rename (this file both constructs a Groth16Verifyingkey
and generates source code using quote!).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@prover/client/src/proof.rs`:
- Around line 87-89: The code in compress_proof currently calls
alt_bn128_g1_compress_be/alt_bn128_g2_compress_be and unwraps their Results,
which can panic on malformed prover data; change compress_proof to return
Result<([u8;32],[u8;64],[u8;32]), ProverClientError>, replace the .unwrap()
calls with map_err to convert compression errors into ProverClientError, and
update the two existing callsites that already return Result to propagate the
error using the ? operator (or handle explicitly) so compression failures are
returned instead of panicking.
In `@sdk-libs/photon-api/src/lib.rs`:
- Around line 14-16: The ensure_ring_provider function currently calls
rustls::crypto::ring::default_provider().install_default() on every
Configuration construction; change it to perform the installation exactly once
using a static std::sync::Once: declare a static ONCE: Once = Once::new() and
inside ensure_ring_provider call ONCE.call_once(|| { let _ =
rustls::crypto::ring::default_provider().install_default(); }); keep discarding
the Result as before but ensure the install attempt only runs one time per
process to avoid redundant calls.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 82f69761-a08c-4b77-a091-c99650d7bab7
⛔ Files ignored due to path filters (8)
.github/actions/setup-and-build/action.ymlis excluded by none and included by none.github/workflows/forester-tests.ymlis excluded by none and included by none.github/workflows/programs.ymlis excluded by none and included by none.github/workflows/rust.ymlis excluded by none and included by none.github/workflows/sdk-tests.ymlis excluded by none and included by noneCargo.lockis excluded by!**/*.lockand included by noneCargo.tomlis excluded by none and included by nonextask/src/create_vkeyrs_from_gnark_key.rsis excluded by none and included by none
📒 Files selected for processing (107)
program-libs/verifier/src/verifying_keys/batch_address_append_40_10.rsprogram-libs/verifier/src/verifying_keys/batch_address_append_40_250.rsprogram-libs/verifier/src/verifying_keys/batch_append_32_10.rsprogram-libs/verifier/src/verifying_keys/batch_append_32_500.rsprogram-libs/verifier/src/verifying_keys/batch_update_32_10.rsprogram-libs/verifier/src/verifying_keys/batch_update_32_500.rsprogram-libs/verifier/src/verifying_keys/v1_combined_26_26_1_1.rsprogram-libs/verifier/src/verifying_keys/v1_combined_26_26_1_2.rsprogram-libs/verifier/src/verifying_keys/v1_combined_26_26_1_4.rsprogram-libs/verifier/src/verifying_keys/v1_combined_26_26_1_8.rsprogram-libs/verifier/src/verifying_keys/v1_combined_26_26_2_1.rsprogram-libs/verifier/src/verifying_keys/v1_combined_26_26_2_2.rsprogram-libs/verifier/src/verifying_keys/v1_combined_26_26_2_4.rsprogram-libs/verifier/src/verifying_keys/v1_combined_26_26_2_8.rsprogram-libs/verifier/src/verifying_keys/v1_combined_26_26_3_1.rsprogram-libs/verifier/src/verifying_keys/v1_combined_26_26_3_2.rsprogram-libs/verifier/src/verifying_keys/v1_combined_26_26_3_4.rsprogram-libs/verifier/src/verifying_keys/v1_combined_26_26_3_8.rsprogram-libs/verifier/src/verifying_keys/v1_combined_26_26_4_1.rsprogram-libs/verifier/src/verifying_keys/v1_combined_26_26_4_2.rsprogram-libs/verifier/src/verifying_keys/v1_combined_26_26_4_4.rsprogram-libs/verifier/src/verifying_keys/v1_combined_26_26_4_8.rsprogram-libs/verifier/src/verifying_keys/v1_combined_26_26_8_1.rsprogram-libs/verifier/src/verifying_keys/v1_combined_26_26_8_2.rsprogram-libs/verifier/src/verifying_keys/v1_combined_26_26_8_4.rsprogram-libs/verifier/src/verifying_keys/v1_combined_26_26_8_8.rsprogram-libs/verifier/src/verifying_keys/v1_inclusion_26_1.rsprogram-libs/verifier/src/verifying_keys/v1_inclusion_26_2.rsprogram-libs/verifier/src/verifying_keys/v1_inclusion_26_3.rsprogram-libs/verifier/src/verifying_keys/v1_inclusion_26_4.rsprogram-libs/verifier/src/verifying_keys/v1_inclusion_26_8.rsprogram-libs/verifier/src/verifying_keys/v1_non_inclusion_26_1.rsprogram-libs/verifier/src/verifying_keys/v1_non_inclusion_26_2.rsprogram-libs/verifier/src/verifying_keys/v1_non_inclusion_26_3.rsprogram-libs/verifier/src/verifying_keys/v1_non_inclusion_26_4.rsprogram-libs/verifier/src/verifying_keys/v1_non_inclusion_26_8.rsprogram-libs/verifier/src/verifying_keys/v2_combined_32_40_1_1.rsprogram-libs/verifier/src/verifying_keys/v2_combined_32_40_1_2.rsprogram-libs/verifier/src/verifying_keys/v2_combined_32_40_1_3.rsprogram-libs/verifier/src/verifying_keys/v2_combined_32_40_1_4.rsprogram-libs/verifier/src/verifying_keys/v2_combined_32_40_2_1.rsprogram-libs/verifier/src/verifying_keys/v2_combined_32_40_2_2.rsprogram-libs/verifier/src/verifying_keys/v2_combined_32_40_2_3.rsprogram-libs/verifier/src/verifying_keys/v2_combined_32_40_2_4.rsprogram-libs/verifier/src/verifying_keys/v2_combined_32_40_3_1.rsprogram-libs/verifier/src/verifying_keys/v2_combined_32_40_3_2.rsprogram-libs/verifier/src/verifying_keys/v2_combined_32_40_3_3.rsprogram-libs/verifier/src/verifying_keys/v2_combined_32_40_3_4.rsprogram-libs/verifier/src/verifying_keys/v2_combined_32_40_4_1.rsprogram-libs/verifier/src/verifying_keys/v2_combined_32_40_4_2.rsprogram-libs/verifier/src/verifying_keys/v2_combined_32_40_4_3.rsprogram-libs/verifier/src/verifying_keys/v2_combined_32_40_4_4.rsprogram-libs/verifier/src/verifying_keys/v2_inclusion_32_1.rsprogram-libs/verifier/src/verifying_keys/v2_inclusion_32_10.rsprogram-libs/verifier/src/verifying_keys/v2_inclusion_32_11.rsprogram-libs/verifier/src/verifying_keys/v2_inclusion_32_12.rsprogram-libs/verifier/src/verifying_keys/v2_inclusion_32_13.rsprogram-libs/verifier/src/verifying_keys/v2_inclusion_32_14.rsprogram-libs/verifier/src/verifying_keys/v2_inclusion_32_15.rsprogram-libs/verifier/src/verifying_keys/v2_inclusion_32_16.rsprogram-libs/verifier/src/verifying_keys/v2_inclusion_32_17.rsprogram-libs/verifier/src/verifying_keys/v2_inclusion_32_18.rsprogram-libs/verifier/src/verifying_keys/v2_inclusion_32_19.rsprogram-libs/verifier/src/verifying_keys/v2_inclusion_32_2.rsprogram-libs/verifier/src/verifying_keys/v2_inclusion_32_20.rsprogram-libs/verifier/src/verifying_keys/v2_inclusion_32_3.rsprogram-libs/verifier/src/verifying_keys/v2_inclusion_32_4.rsprogram-libs/verifier/src/verifying_keys/v2_inclusion_32_5.rsprogram-libs/verifier/src/verifying_keys/v2_inclusion_32_6.rsprogram-libs/verifier/src/verifying_keys/v2_inclusion_32_7.rsprogram-libs/verifier/src/verifying_keys/v2_inclusion_32_8.rsprogram-libs/verifier/src/verifying_keys/v2_inclusion_32_9.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_1.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_10.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_11.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_12.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_13.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_14.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_15.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_16.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_17.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_18.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_19.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_2.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_20.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_21.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_22.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_23.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_24.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_25.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_26.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_27.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_28.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_29.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_3.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_30.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_31.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_32.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_4.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_5.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_6.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_7.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_8.rsprogram-libs/verifier/src/verifying_keys/v2_non_inclusion_40_9.rsprover/client/src/proof.rssdk-libs/photon-api/Cargo.tomlsdk-libs/photon-api/src/lib.rs
Comment on lines
+87
to
+89
| let proof_a = alt_bn128_g1_compress_be(proof_a).unwrap(); | ||
| let proof_b = alt_bn128_g2_compress_be(proof_b).unwrap(); | ||
| let proof_c = alt_bn128_g1_compress_be(proof_c).unwrap(); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify all callsites that must be updated if compress_proof returns Result.
# Expected: each callsite handles `?` / match / map_err, and no `.unwrap()` remains on compress_proof.
rg -nP --type=rust -C3 '\bcompress_proof\s*\('
rg -nP --type=rust '\bcompress_proof\s*\([^;]*\)\.unwrap\s*\('Repository: Lightprotocol/light-protocol
Length of output: 2745
🏁 Script executed:
cat -n prover/client/src/proof.rs | sed -n '75,95p'Repository: Lightprotocol/light-protocol
Length of output: 955
🏁 Script executed:
cat -n prover/client/src/proof_client.rs | sed -n '655,670p'Repository: Lightprotocol/light-protocol
Length of output: 675
🏁 Script executed:
cat -n program-tests/utils/src/e2e_test_env.rs | sed -n '834,850p'Repository: Lightprotocol/light-protocol
Length of output: 1330
🏁 Script executed:
cat -n sdk-libs/program-test/src/indexer/test_indexer.rs | sed -n '2480,2495p'Repository: Lightprotocol/light-protocol
Length of output: 1130
🏁 Script executed:
rg -nP --type=rust -B10 'compress_proof\(&proof_a' prover/client/src/proof_client.rs | head -30Repository: Lightprotocol/light-protocol
Length of output: 679
🏁 Script executed:
rg -nP --type=rust -B20 'compress_proof\(&proof_a' program-tests/utils/src/e2e_test_env.rs | head -40Repository: Lightprotocol/light-protocol
Length of output: 1505
🏁 Script executed:
fd CLAUDE.md --type f | head -20Repository: Lightprotocol/light-protocol
Length of output: 684
🏁 Script executed:
rg -i 'proof.*compress' --type=md | head -20Repository: Lightprotocol/light-protocol
Length of output: 5068
🏁 Script executed:
cat -n prover/client/src/proof_client.rs | sed -n '630,670p'Repository: Lightprotocol/light-protocol
Length of output: 2026
🏁 Script executed:
cat -n program-tests/utils/src/e2e_test_env.rs | sed -n '800,860p'Repository: Lightprotocol/light-protocol
Length of output: 4145
🏁 Script executed:
cat -n sdk-libs/program-test/src/indexer/test_indexer.rs | sed -n '2475,2510p'Repository: Lightprotocol/light-protocol
Length of output: 1924
🏁 Script executed:
fd -e rs -x grep -l "alt_bn128_g1_compress_be\|alt_bn128_g2_compress_be" {} \;Repository: Lightprotocol/light-protocol
Length of output: 100
🏁 Script executed:
cat -n prover/client/src/proof.rs | sed -n '1,35p'Repository: Lightprotocol/light-protocol
Length of output: 1272
🏁 Script executed:
rg -nP "ProverClientError" prover/client/src/proof.rsRepository: Lightprotocol/light-protocol
Length of output: 185
🏁 Script executed:
cat -n prover/client/src/proof.rs | sed -n '1,15p'Repository: Lightprotocol/light-protocol
Length of output: 706
Propagate compression errors instead of panicking.
compress_proof uses .unwrap() on elliptic curve compression operations. Since this path consumes untrusted prover response data, malformed points will panic instead of returning a typed error in production contexts.
The function appears in three callsites: two production paths (proof_client.rs:659 and test_indexer.rs:2486) where functions already return Result types, making error propagation feasible.
Suggested approach: change compress_proof to return Result<([u8; 32], [u8; 64], [u8; 32]), ProverClientError>, propagating errors with map_err, and update callsites to handle the Result (using ? operator or explicit matching).
Suggested fix
-pub fn compress_proof(
+pub fn compress_proof(
proof_a: &[u8; 64],
proof_b: &[u8; 128],
proof_c: &[u8; 64],
-) -> ([u8; 32], [u8; 64], [u8; 32]) {
- let proof_a = alt_bn128_g1_compress_be(proof_a).unwrap();
- let proof_b = alt_bn128_g2_compress_be(proof_b).unwrap();
- let proof_c = alt_bn128_g1_compress_be(proof_c).unwrap();
- (proof_a, proof_b, proof_c)
+) -> Result<([u8; 32], [u8; 64], [u8; 32]), ProverClientError> {
+ let proof_a = alt_bn128_g1_compress_be(proof_a).map_err(|e| {
+ ProverClientError::ProverServerError(format!("Failed to compress proof.a: {e}"))
+ })?;
+ let proof_b = alt_bn128_g2_compress_be(proof_b).map_err(|e| {
+ ProverClientError::ProverServerError(format!("Failed to compress proof.b: {e}"))
+ })?;
+ let proof_c = alt_bn128_g1_compress_be(proof_c).map_err(|e| {
+ ProverClientError::ProverServerError(format!("Failed to compress proof.c: {e}"))
+ })?;
+ Ok((proof_a, proof_b, proof_c))
}Update callsites to handle the Result—proof_client.rs:659 and test_indexer.rs:2486 can use the ? operator since they already return Result types.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@prover/client/src/proof.rs` around lines 87 - 89, The code in compress_proof
currently calls alt_bn128_g1_compress_be/alt_bn128_g2_compress_be and unwraps
their Results, which can panic on malformed prover data; change compress_proof
to return Result<([u8;32],[u8;64],[u8;32]), ProverClientError>, replace the
.unwrap() calls with map_err to convert compression errors into
ProverClientError, and update the two existing callsites that already return
Result to propagate the error using the ? operator (or handle explicitly) so
compression failures are returned instead of panicking.
…ailure Solana 2.2.15 ships platform-tools v1.46 (cargo 1.84.0), which cannot parse Cargo.toml manifests that use `edition = "2024"`. time-macros 0.2.27 (transitively required via solana-streamer -> x509-parser -> asn1-rs -> time 0.3.47) uses edition 2024. Main CI was not failing because its build cache was warm. Our Cargo.lock changes (solana-bn254 2.2 -> 3.2.1, groth16-solana patch) bust the cache, causing a fresh compile of time-macros 0.2.27 which then fails. Solana 2.3.13 ships platform-tools v1.48 (Rust 1.86+) which supports edition 2024. This also aligns the CLI version with the workspace library crates that are already pinned to "2.3".
Use std::sync::Once in ensure_ring_provider so that install_default() is only attempted on the first call per process, avoiding redundant global-state mutations on subsequent calls.
time 0.3.38+ depends on time-macros 0.2.20+ which uses edition = "2024"
in its Cargo.toml. The Solana platform-tools ship Cargo 1.84.0 which
cannot parse edition 2024 manifests. Pin time to 0.3.37 (time-macros
0.2.19) so cargo test-sbf can parse the full dep graph.
Affects: e2e-test via solana-client -> solana-streamer -> x509-parser
-> asn1-rs -> time -> time-macros
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
libzstd-devon CI and setZSTD_SYS_USE_PKG_CONFIG=1to skip the zstd-sys C build (~7.9s) — zstd-sys is a transitive dep via Solana's reqwest 0.11 and cannot be removed, but the C build is avoided with a system libraryCARGO_INCREMENTAL=0to all CI workflows to prevent incremental artifact accumulation across cached builds[profile.dev.package.*]opt-level 3 overrides for proc-macro crates (syn,proc-macro2,quote,serde_derive,ark-ff-macros,ark-ff-asm) — they run at build time so faster generated code reduces downstream compile timegroth16-solanavia[patch.crates-io]to use commit 4e6cacf which bumpssolana-bn254from v2 to v3 and switches all ark deps todefault-features = false— eliminates the duplicateark-ff 0.4compilation (~9–10s) caused by the old groth16-solana pulling in a separate ark 0.4 build alongside the workspace's ark 0.5solana-bn254from2.2to3.2.1to matchvk_gamme_g2→vk_gamma_g2in 104light-verifierverifying key files (the typo fix was shipped upstream in groth16-solana PR add custom errors #29, required by the patched version)CI Build Time Savings
Comparison of baseline (main branch
7075dbce) vs this PR (86881992), both triggering full Rust compilation.Lint & JS Workflows
SDK / Integration Tests (examples-tests workflow)
Average improvement: ~19% (range 9%--29%). Total cumulative savings across all 10 jobs: ~36 minutes.
Changes
libzstd-dev+ZSTD_SYS_USE_PKG_CONFIG=1CARGO_INCREMENTAL=0in all workflow filesopt-level = 3forsyn,proc-macro2,quote,serde_derive,ark-ff-macros,ark-ff-asmark-ff 0.4build via[patch.crates-io]vk_gamme_g2→vk_gamma_g2in 104 verifying key filesSummary by CodeRabbit
Bug Fixes
Chores