chore: minor changes moby/stairway#564
Open
titouantanguy wants to merge 30 commits intomainfrom
Open
Conversation
Consolidated Tests Results 2026-05-05 - 16:51:17Test Results
Details
test-reporter: Run #1917
🎉 All tests passed!TestsView All Tests
🍂 No flaky tests in this run. Github Test Reporter by CTRF 💚 🔄 This comment has been updated |
titouantanguy
force-pushed
the
titouan/chore/minor-nist-bench
branch
from
9469e5d to
6b32d44
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors and expands the core/experiments reproducible benchmark tooling for TFHE and BGV, adds NIST-oriented runner/parser scripts for session stats, and introduces small runtime/CLI changes to support multi-ciphertext decryption flows and improved stats collection.
Changes:
- Factor TFHE reproducible test logic into a shared
tfhe_reproducible_common.shand add small/large/malicious wrappers; add a BGV reproducible script. - Add NIST scripts to run threshold reproducible tests and parse per-party
session_statsinto CSV; wiresession_statsfolder creation/mounting into experiment setup and docker compose. - Extend CLIs and runtime code paths to handle “N ciphertexts per preprocess/decrypt” and to fail on mismatched expected plaintexts.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| core/experiments/test_scripts/tfhe_reproducible_small_session.sh | TFHE small-session wrapper now delegates to shared common script. |
| core/experiments/test_scripts/tfhe_reproducible_small_session_malicious.sh | Adds malicious small-session wrapper for TFHE reproducible runs. |
| core/experiments/test_scripts/tfhe_reproducible_large_session.sh | Adds TFHE large-session wrapper delegating to shared common script. |
| core/experiments/test_scripts/tfhe_reproducible_common.sh | New shared TFHE reproducible flow (keygen, CRS, reshare, multi-ctxt decrypt checks). |
| core/experiments/test_scripts/bgv_reproducible.sh | New BGV reproducible runner using stairwayctl, including expected-hash and DDec loops. |
| core/experiments/src/choreography/tfhe_rs/grpc.rs | Adds cached modswitched private keyset for Z64 and routes bitdec to it. |
| core/experiments/src/choreography/server_utils.rs | Writes stats under session_stats/ and creates parent directory if missing. |
| core/experiments/src/bin/threshold-tfhe/mobygo.rs | Adds --num-ctxts to decrypt-from-file, parses comma-delimited expected values, exits non-zero on mismatch. |
| core/experiments/src/bin/threshold-bgv/stairwayctl.rs | Adds encrypt + threshold-decrypt-from-file commands and expected-value checking with non-zero exit on mismatch. |
| core/experiments/NIST_scripts/threshold-test-params.sh | New script orchestrating TFHE/BGV reproducible threshold tests under docker. |
| core/experiments/NIST_scripts/session-stats-parser.py | New parser aggregating per-party session stats into averaged CSV outputs. |
| core/experiments/NIST_scripts/non-threshold-zk-pok-bench.sh | Adds a size bench output file and runs the new ZK PoK size bench. |
| core/experiments/NIST_scripts/non-threshold-tfhe-bench.sh | Improves logging/UX for running and parsing non-threshold benches. |
| core/experiments/NIST_scripts/non-threshold-parser.py | Extends CSV output to include serialized proof sizes. |
| core/experiments/NIST_scripts/build.sh | Fixes path quoting for the experiments target dir. |
| core/experiments/NIST_scripts/.gitignore | Ignores threshold/ output directory. |
| core/experiments/Makefile.toml | Adds setup task for telemetry + session stats files; adjusts bench-run tasks; start-parties waits for health. |
| core/experiments/experiments/templates/docker-compose.yml.j2 | Passes expected parties/threshold args; mounts temp/session_stats into containers. |
| core/experiments/Cargo.toml | Removes cdylib crate-type; registers new size bench target. |
| core/experiments/benches/non-threshold/utilities.rs | Adds shared SAMPLE_SIZE constant for Criterion configuration. |
| core/experiments/benches/non-threshold/tfhe-zk-pok/speed.rs | Uses shared SAMPLE_SIZE for speed benches. |
| core/experiments/benches/non-threshold/tfhe-zk-pok/size.rs | New bench printing serialized proof sizes for parsing. |
| core/experiments/benches/non-threshold/tfhe-rs/speed/keygen.rs | Uses shared SAMPLE_SIZE. |
| core/experiments/benches/non-threshold/tfhe-rs/speed/erc20.rs | Uses shared SAMPLE_SIZE. |
| core/experiments/benches/non-threshold/tfhe-rs/speed/basic_ops.rs | Uses shared SAMPLE_SIZE. |
| core/experiments/benches/non-threshold/bgv/speed/keygen.rs | Uses shared SAMPLE_SIZE. |
| core/experiments/benches/non-threshold/bgv/speed/basic_ops.rs | Uses shared SAMPLE_SIZE. |
| Cargo.toml | Reduces bench profile debug info to line-tables-only. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1
to
+3
| SESSION_TYPE="small" | ||
| EXPECTED_KEY_HASH="86e53c36c19d03ef7794486d982e7cc8bba4ddea704b7b9587e43aed5e7a804e" | ||
| EXPECTED_RESHARED_KEY_HASH="b7f36bba2da051dca4740721a66da374c07cc657ede4bee3cea4ec198bd15b33" |
Comment on lines
+1
to
+3
| SESSION_TYPE="large" | ||
| EXPECTED_KEY_HASH="a1420068eea9184e833f31c859c1b10621e648966cff3697b9e151ab744f03df" | ||
| EXPECTED_RESHARED_KEY_HASH="5b13254b3c2dcb604b6653902a7ed74f0263abef98fa4a0b133972f40d84c7ef" |
Comment on lines
+1
to
+3
| SESSION_TYPE="small" | ||
| EXPECTED_KEY_HASH="86e53c36c19d03ef7794486d982e7cc8bba4ddea704b7b9587e43aed5e7a804e" | ||
| EXPECTED_RESHARED_KEY_HASH="b7f36bba2da051dca4740721a66da374c07cc657ede4bee3cea4ec198bd15b33" |
| DDEC_MODES="noise-flood-small bit-dec-small" | ||
| MAIN_PATH="./temp/tfhe_small_reproducible_malicious" | ||
| PARAMS="params-test-bk-sns" | ||
| SEED=42 |
|
|
||
|
|
||
|
|
||
| if [ $2 = "GEN" ]; then |
Comment on lines
+331
to
+341
| let m: Vec<u32> = (0..N65536::VALUE) | ||
| .map(|i| ((params.value + i as u64) % PLAINTEXT_MODULUS.get().0) as u32) | ||
| .collect(); | ||
|
|
||
| let ctxt = bgv_pk_encrypt(&mut rng, &m, &pk); | ||
|
|
||
| println!("Encrypted value {}", params.value); | ||
|
|
||
| let serialized = bc2wrap::serialize(&(m, ctxt))?; | ||
| std::fs::write(¶ms.output_file, serialized)?; | ||
| println!("Ciphertexts stored in {}", params.output_file); |
|
|
||
| /// Optional argument to check the received plaintext against an expected value. | ||
| /// The expected plaintext polynomial is value, value+1, value+2, ... | ||
| /// If more than one plaintext were decrypted, we make sure they are all equal to this epxectect plaintext polynomial. |
Comment on lines
+2
to
+11
| //! Speed benchmarks for ZK proof-of-knowledge operations. | ||
| //! | ||
| //! Measures wall-clock time for: | ||
| //! - CRS generation | ||
| //! - Proof generation (CRS pre-computed) | ||
| //! - Proof verification in TwoSteps mode (proof pre-computed) | ||
| //! - Proof verification in Batched mode (proof pre-computed) | ||
| //! | ||
| //! Run with: | ||
| //! cargo bench --bench non-threshold_tfhe-zk-pok_speed |
Comment on lines
+12
to
+16
| TFHE_RUN_4P_NAME = "tfhe-bench-run-4p" | ||
| TFHE_RUN_5P_NAME = "tfhe-bench-run-5p" | ||
| TFHE_RUN_NAMES = [TFHE_RUN_4P_NAME, TFHE_RUN_5P_NAME] | ||
| BGV_RUN_NAME = "bgv-bench-run" | ||
|
|
Comment on lines
+19
to
+23
| EXPECTED_LINES_PER_RUN = { | ||
| TFHE_RUN_4P_NAME: 34, | ||
| TFHE_RUN_5P_NAME: 34, | ||
| BGV_RUN_NAME: 11, | ||
| } |
dd23
reviewed
May 6, 2026
Comment on lines
+24
to
+26
| if [ "$(docker ps -a -q)" ]; then | ||
| echo "Cleaning up docker containers..." | ||
| docker stop $(docker ps -a -q) && docker rm $(docker ps -a -q) |
Member
There was a problem hiding this comment.
should we limit this to just known docker containers? Maybe it's fine as is, if we assume we only run the benchmarks on this machine.
dd23
reviewed
May 6, 2026
|
|
||
|
|
||
| #Bring parties alive based on provided yml file | ||
| [tasks.start-parties] |
Member
There was a problem hiding this comment.
can we update the related documentation in https://github.com/zama-ai/kms/blob/main/docs/guides/threshold-benchmark.md?
dd23
reviewed
May 6, 2026
Comment on lines
+43
to
+44
| TFHE_RUN_4P_NAME: build_tfhe_operation_labels(), | ||
| TFHE_RUN_5P_NAME: build_tfhe_operation_labels(), |
Member
There was a problem hiding this comment.
I haven't fully figured out how it works, but this means we run the exact same operations for both 4 and 5 parties, correct?
dd23
reviewed
May 6, 2026
| #Run the full test suite for tfhe with centralized dkg | ||
| [tasks.bgv-test-fake-dkg] | ||
| command = "sh" | ||
| args = ["test_scripts/bgv_test_script_fake_dkg.sh", "temp/${EXPERIMENT_NAME}.toml"] |
Member
There was a problem hiding this comment.
should we also delete the script, or do we want to keep it?
dd23
reviewed
May 6, 2026
| #Checking every 30s | ||
| $MOBYGO_EXEC -c $1 status-check --sid $CURR_SID --keep-retry true --interval 30 | ||
| #Get the key | ||
| mkdir -p $KEY_PATH |
dd23
reviewed
May 6, 2026
| params.session_id_decrypt | ||
| ); | ||
| } else { | ||
| println!( |
Member
There was a problem hiding this comment.
maybe this is clearer?
Suggested change
| println!( | |
| eprintln!( |
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
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.
Description of changes
Issue ticket number and link
PR Checklist
I attest that all checked items are satisfied. Any deviation is clearly justified above.
chore: ...).TODO(#issue).unwrap/expect/paniconly in tests or for invariant bugs (documented if present).devopslabel + infra notified + infra-team reviewer assigned.!and affected teams notified.Zeroize+ZeroizeOnDropimplemented.unsafe; if unavoidable: minimal, justified, documented, and test/fuzz covered.Dependency Update Questionnaire (only if deps changed or added)
Answer in the
Cargo.tomlnext to the dependency (or here if updating):More details and explanations for the checklist and dependency updates can be found in CONTRIBUTING.md