refactor: cleanup & add proptests - #32
Merged
Merged
Conversation
- hash_to_field: const-generic msg/dst lengths make the RFC 9380 DST limit and scratch capacity compile-time asserts, removing the runtime error path; reduction and serialization now avoid arkworks' heap paths. Add an RFC-pseudocode reference expander validated against all 10 CFRG appendix K.1 vectors and a differential grid test of the production L=48 expander against it. - lib: gate extern crate alloc and the gnark vk parser off SBF builds so any allocation reintroduced into on-chain code fails to compile. - groth16: verify_commitment_pok fails closed via exhaustive match; a partial commitment/pok/key state now returns the new Bsb22InconsistentCommitmentState error (code 21) instead of silently skipping the check. Commitment fields are unconditional so the default-feature build compiles again.
…nark fixtures Adapt the benchmark program to the light-program-profiler pattern and generate BENCHMARKS.md covering plain Groth16 and BSB22 (one Pedersen commitment) verification at 1/2/4/8 public inputs. - tests/program (was tests/bsb22-program): selector byte dispatches to one baked vk per variant; #[profile]-instrumented verify / verify_with_bsb22_commitment behind a profile-program feature - tests/program/tests/bench_cu.rs: mollusk harness with register_profiling_syscalls + CuBenchmark, writes BENCHMARKS.md - litesvm_cu.rs: all 8 variants end to end with per-variant CU envelopes (last measured + ~10%), plus negative tests - deterministic fixtures: swap crypto/rand.Reader for a per-variant seeded ChaCha8 stream so gnark Setup/Prove are byte-reproducible; build.rs regenerates fixtures via go run ./cmd/benchgen, so the SBF and host builds derive identical bytes and no fixture binaries are committed (tests/fixtures/ removed; lib unit tests use baked hex in tests/gnark-ffi/test_fixtures.rs, included via #[path]) - rename tests/bsb22 -> tests/gnark-ffi (crate groth16-solana-gnark-ffi), drop the obsolete TestDumpFullFixture Measured: plain 78k/83k/91k/109k CU, BSB22 211k/216k/225k/242k CU for 1/2/4/8 public inputs.
The bench fixtures and mollusk runtime are deterministic, so the regenerated BENCHMARKS.md is byte-reproducible. CI now rebuilds the profiled .so, reruns the bench, and fails on any uncommitted BENCHMARKS.md diff - every CU change must be re-baselined by committing the regenerated file. - remove tests/program/tests/litesvm_cu.rs and the per-variant max_cu envelope asserts - drop the litesvm + solana 3.x dev-dependency stack; mollusk is the only test harness left - drop the unprofiled build-sbf CI step (nothing consumes that .so in CI anymore)
tests/program/tests/failing.rs runs every negative case on-chain under mollusk and pins the exact error a client would see: - parsing gates: unknown selector, empty instruction data, one byte short/long payloads on all 8 variants, cross-mode payloads -> ProgramError::InvalidInstructionData - plain groth16: mutated public input, non-negated proof_a, off-curve proof point, proof for a different vk, swapped public inputs -> Custom(ProofVerificationFailed); input >= Fr modulus -> Custom(PublicInputGreaterThanFieldSize) - bsb22: mutated input, on-curve substituted commitment, swapped commitment/pok -> Custom(ProofVerificationFailed); off-curve commitment -> Custom(Bsb22InvalidCommitmentPoint); mutated pok -> Custom(CommitmentPokVerificationFailed) - layout sanity test pinning the splice offsets to PROOF_LEN The tests register the profiling syscalls, so they run against either .so build. CI now builds the profiled .so before the workspace tests (failing.rs loads it) and reuses it for the BENCHMARKS.md bench.
Replace the two bare vk_ic indexings with .get()/.first() mapped to InvalidPublicInputsLength. The constructor length checks already made them unreachable, but the bound is now enforced locally and fails closed instead of panicking (or silently truncating, as a zip would) if that invariant ever breaks. Re-baselines BENCHMARKS.md: the bounds checks shift each variant by a few CU in both directions (plain_8 and bsb22_8 drop ~175 CU).
- rename the `vk` feature to `circom-vk`; move the gnark vk parser (vk::gnark) out of `bsb22` into a standalone `gnark-vk` feature. The parser-only error variants move with it; u32 error codes are unchanged. tests/program's build-dependency slims to gnark-vk. - remove the dead Bsb22HashToFieldFailed variant (hash_to_field is infallible); code 14 stays unassigned like 0/4/5. - delete parse_vk_to_rust.js and the root npm files; the README now points to the Rust generators in src/vk/ instead. - restructure the README around a per-feature list and correct the CU claim (78k-109k plain, 212k-242k BSB22) and the alt_bn128 syscall name. - prose cleanup across docs and comments; BENCHMARKS.md description regenerated to match bench_cu.rs.
- justfile mirroring the zolana conventions: build/test/bench/lint recipes; CI collapses to a single `just ci` step (lint, circuit build, workspace build, profiled SBF build, tests, all-features unit run, BENCHMARKS.md regeneration + diff check). - lint = fmt check + clippy + feature-matrix compile (none, each feature alone, all) + README sync. Fixed all findings: five clippy warnings and the fmt churn on the generated tests/rust-vk/src/verifying_key.rs, whose generator now emits a header comment and #[rustfmt::skip]. - README.md is generated from the src/lib.rs crate docs with cargo-rdme (markers + .cargo-rdme.toml; intralinks stripped to avoid the nightly toolchain). `just readme` regenerates, `just check-readme` gates. - crate docs gain the per-input CU table and exact cost ranges; the new readme_benchmarks test pins both to the BENCHMARKS.md totals so a re-baselined bench forces a docs update. - CLAUDE.md documents the test coverage per suite with numbered test lists.
Analyzed eight files, diff |
- SOLANA_VERSION pinned instead of "stable": platform-tools codegen determines the BENCHMARKS.md CU numbers, so the CI toolchain must match the one that produced the committed baseline (v3.1.15, platform-tools v1.52). Bumps re-baseline via `just bench`. - `just check-nostd` (in `just ci`) compiles the solana-target dependency graph against a core+alloc-only sysroot (nightly build-std, os = "solana" target spec in ci/solana-nostd.json), so any std usage in the on-chain surface fails to compile. CI installs nightly + rust-src for it.
release.anza.xyz has no v3.1.15 install; v3.1.14 pins the same platform-tools v1.52 as the local toolchain that produced the BENCHMARKS.md baselines.
parse_gnark_vk_bytes trusted the untrusted nbK length field as a Vec::with_capacity size before checking the buffer holds nbK 64-byte K entries, so a malformed vk blob claiming nbK = u32::MAX forced a huge allocation before the cursor bounds check could reject it.
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.
No description provided.