feat: add production ready sets for trbfv and bfv#942
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughAdds an optional error2_variance field into BFV parameter definition, encoding/decoding ABI, and parameter-construction APIs; introduces BfvParamSet presets and new build_bfv_params_from_set* helpers; updates call sites and CLI to pass or default the variance; adds a workspace dependency to bfv-helpers Cargo.toml. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as pack_e3_params CLI
participant TS as Enclave SDK / Caller
participant Helpers as bfv_helpers
participant Encoder as encode_bfv_params
CLI->>TS: optional --error2-variance
TS->>Helpers: build_bfv_params_from_set_arc(param_set) or build_bfv_params_arc(..., Some/None)
activate Helpers
Note right of Helpers `#DDEBF7`: internal variance = provided OR default "10"
Helpers-->>TS: Arc<BfvParameters (with error2_variance)>
deactivate Helpers
TS->>Encoder: encode_bfv_params(params)
activate Encoder
Encoder-->>TS: encoded payload { degree, plaintext_modulus, moduli, error2_variance }
deactivate Encoder
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (9)📓 Common learnings📚 Learning: 2025-05-07T15:18:20.056ZApplied to files:
📚 Learning: 2024-10-22T02:10:34.864ZApplied to files:
📚 Learning: 2024-10-23T01:59:42.967ZApplied to files:
📚 Learning: 2024-10-03T23:02:41.732ZApplied to files:
📚 Learning: 2024-09-26T03:11:29.311ZApplied to files:
📚 Learning: 2024-10-23T02:03:02.008ZApplied to files:
📚 Learning: 2024-11-05T06:48:58.177ZApplied to files:
📚 Learning: 2025-09-19T11:16:53.825ZApplied to files:
🧬 Code graph analysis (6)templates/default/program/src/lib.rs (1)
crates/test-helpers/src/bin/fake_encrypt.rs (1)
examples/CRISP/server/src/cli/commands.rs (1)
crates/bfv-helpers/src/client.rs (1)
crates/test-helpers/src/lib.rs (1)
crates/bfv-helpers/src/lib.rs (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
🔇 Additional comments (13)
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 |
|
ah maybe is failing because I need to update the encoded values for the BFV parameter set. I will work on this on Monday! BTW, @ryardley feel free to start getting a look around if you like the approach / design. |
0xjei
force-pushed
the
params/128b-sets
branch
from
4280a1d to
edf2cd3
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/bfv-helpers/src/lib.rs (1)
191-249: Decoding breaks pre-upgrade payloads.
tuple_typenow hard-requires theerror2_variancestring, so any BFV params encoded before this PR (only three fields) will panic at decode. We need a backward-compatible branch (e.g., try 3-field decode first, default variance to “10”) or a versioned payload so existing snapshots/chain data don’t brick.
🧹 Nitpick comments (1)
crates/bfv-helpers/src/lib.rs (1)
35-55: Clarify tuple shapes for the new presets.
SET_2048_1032193_1omitserror2_variancewhileSET_8192_1000_4includes it. The mixed tuple arities make it easy to misuse these constants. Please either supply both variances (e.g.,(usize, u64, [u64; N], &str)for every preset) or expose helper accessors so downstream code can’t accidentally ignoreerror2_variancefor TRBFV.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockexamples/CRISP/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
crates/bfv-helpers/Cargo.toml(1 hunks)crates/bfv-helpers/src/client.rs(6 hunks)crates/bfv-helpers/src/lib.rs(16 hunks)crates/fhe/src/fhe.rs(1 hunks)crates/fhe/src/utils.rs(1 hunks)crates/test-helpers/src/bin/fake_encrypt.rs(1 hunks)crates/test-helpers/src/bin/pack_e3_params.rs(1 hunks)crates/tests/tests/integration.rs(1 hunks)crates/trbfv/tests/integration.rs(1 hunks)examples/CRISP/server/src/cli/commands.rs(1 hunks)examples/CRISP/server/src/server/routes/rounds.rs(1 hunks)packages/enclave-sdk/src/utils.ts(4 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: 0xjei
Repo: gnosisguild/enclave PR: 388
File: packages/commons/src/bfv/mod.rs:38-48
Timestamp: 2025-05-07T15:18:20.056Z
Learning: The `build_bfv_params` and related functions in the commons BFV utilities intentionally use panic for error handling as a temporary solution. The team plans to refactor these to use proper Result-based error handling in the future.
📚 Learning: 2025-05-07T15:18:20.056Z
Learnt from: 0xjei
Repo: gnosisguild/enclave PR: 388
File: packages/commons/src/bfv/mod.rs:38-48
Timestamp: 2025-05-07T15:18:20.056Z
Learning: The `build_bfv_params` and related functions in the commons BFV utilities intentionally use panic for error handling as a temporary solution. The team plans to refactor these to use proper Result-based error handling in the future.
Applied to files:
crates/fhe/src/utils.rscrates/tests/tests/integration.rscrates/test-helpers/src/bin/pack_e3_params.rsexamples/CRISP/server/src/cli/commands.rscrates/bfv-helpers/src/lib.rscrates/test-helpers/src/bin/fake_encrypt.rscrates/bfv-helpers/src/client.rs
📚 Learning: 2024-10-08T01:48:49.778Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 139
File: packages/ciphernode/aggregator/src/publickey_aggregator.rs:46-46
Timestamp: 2024-10-08T01:48:49.778Z
Learning: In `packages/ciphernode/router/src/hooks.rs`, the `src_chain_id` parameter in `PublicKeyAggregator::new` is correctly handled, even if not explicitly provided during instantiation.
Applied to files:
examples/CRISP/server/src/server/routes/rounds.rs
📚 Learning: 2025-08-25T10:28:56.174Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 657
File: Cargo.toml:32-34
Timestamp: 2025-08-25T10:28:56.174Z
Learning: The examples/CRISP directory has its own Cargo.toml workspace configuration with members like "server", "wasm-crypto", "program/core", "program/client", etc. The root workspace intentionally excludes "examples/CRISP/server", "examples/CRISP/program", and "examples/CRISP/wasm-crypto" to prevent double workspace membership, which is the correct approach for self-contained example workspaces.
Applied to files:
crates/bfv-helpers/Cargo.toml
📚 Learning: 2024-10-22T02:10:34.864Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 145
File: packages/ciphernode/tests/tests/test_aggregation_and_decryption.rs:82-83
Timestamp: 2024-10-22T02:10:34.864Z
Learning: In the file `packages/ciphernode/tests/tests/test_aggregation_and_decryption.rs`, when reviewing test functions like `generate_pk_share`, minor performance optimizations (e.g., minimizing mutex locks) are not a priority.
Applied to files:
crates/bfv-helpers/src/lib.rscrates/test-helpers/src/bin/fake_encrypt.rs
📚 Learning: 2024-10-03T23:02:41.732Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 133
File: packages/ciphernode/tests/tests/test_aggregation_and_decryption.rs:137-139
Timestamp: 2024-10-03T23:02:41.732Z
Learning: In `packages/ciphernode/tests/tests/test_aggregation_and_decryption.rs`, using the same RNG instance `rng_test` for generating multiple key shares without advancing its state is acceptable.
Applied to files:
crates/bfv-helpers/src/lib.rs
🧬 Code graph analysis (10)
crates/fhe/src/utils.rs (1)
crates/bfv-helpers/src/lib.rs (1)
build_bfv_params_arc(121-143)
crates/tests/tests/integration.rs (1)
crates/bfv-helpers/src/lib.rs (1)
build_bfv_params_arc(121-143)
crates/trbfv/tests/integration.rs (1)
crates/bfv-helpers/src/lib.rs (1)
build_bfv_params_arc(121-143)
crates/fhe/src/fhe.rs (1)
crates/bfv-helpers/src/lib.rs (1)
build_bfv_params_arc(121-143)
crates/test-helpers/src/bin/pack_e3_params.rs (2)
crates/test-helpers/src/bin/fake_encrypt.rs (1)
main(29-52)crates/bfv-helpers/src/lib.rs (1)
build_bfv_params_arc(121-143)
examples/CRISP/server/src/server/routes/rounds.rs (1)
crates/bfv-helpers/src/lib.rs (2)
encode_bfv_params(157-171)build_bfv_params_arc(121-143)
examples/CRISP/server/src/cli/commands.rs (1)
crates/bfv-helpers/src/lib.rs (1)
build_bfv_params_arc(121-143)
crates/bfv-helpers/src/lib.rs (2)
crates/trbfv/src/trbfv_config.rs (1)
params(34-36)templates/default/program/src/lib.rs (1)
test(37-72)
crates/test-helpers/src/bin/fake_encrypt.rs (1)
crates/bfv-helpers/src/lib.rs (1)
build_bfv_params_arc(121-143)
crates/bfv-helpers/src/client.rs (2)
crates/trbfv/src/trbfv_config.rs (1)
params(34-36)crates/bfv-helpers/src/lib.rs (1)
build_bfv_params_arc(121-143)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: crisp_e2e
🔇 Additional comments (1)
packages/enclave-sdk/src/utils.ts (1)
96-123: The review concern about contract decoders is incorrect; remove it.The contract stores
e3ProgramParamsas opaquebytesand does not decode or validate the tuple structure. The structE3RequestParamsdefines it asbytes, and the contract passes it through without interpretation. This means the change from 3-field to 4-field encoding in the SDK is transparent to on-chain logic and will not cause silent parsing failures.The backend (Rust
crates/bfv-helpers) already expects a 4-field tuple witherror2_variance, so the SDK change correctly aligns with that expectation. The contract test scripts that still encode 3 fields are separate test setup code and do not indicate a real incompatibility.No action is required from the developer regarding on-chain decoder synchronization—the concern does not apply here.
Likely an incorrect or invalid review comment.
|
@ryardley ready! |
2 participants
This PR adds 128bit security parameters set (production ready) for
trbfvandbfv.Please, note that
trbfvrequires an explicit set oferror_2. Botherror_1anderror_2variances are default to 10 in other cases. To avoid breaking changes, I madeerror_2an option to the build and we can just passNoneto default to 10.Summary by CodeRabbit
New Features
Chores