feat: make committee size to be dynamic [skip-line-limit]#1556
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a committee build axis and enforcement (CLI, stamps, check-committee), generates per-committee Noir configs/parity, propagates an honest H-subset through public-key aggregation and plaintext gating, updates ZK helpers/keyshare/node-actor logic to be committee-aware, and makes benchmarks, reports, tests, and SDK tooling committee-aware. Cohort: Committee axis and honest-subset integration
Estimated code review effort: Possibly related PRs:
Suggested labels: Suggested reviewers:
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![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: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
crates/zk-helpers/src/circuits/threshold/pk_generation/codegen.rs (1)
158-197:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCommittee size mismatch in test: Small vs Medium.
The test generates a sample using
CiphernodesCommitteeSize::Small(line 158) but then computes bounds and bits usingCiphernodesCommitteeSize::Medium(line 195). This inconsistency could lead to incorrect test validation since the committee size affects bound calculations.🔧 Proposed fix to use consistent committee size
#[test] fn test_toml_generation_and_structure() { - let committee = CiphernodesCommitteeSize::Small.values(); + let committee = CiphernodesCommitteeSize::Medium.values(); let sample = PkGenerationCircuitData::generate_sample(BfvPreset::InsecureThreshold512, committee) .unwrap();Or alternatively, if Small is the intended size:
use crate::ciphernodes_committee::CiphernodesCommitteeSize; - let committee = CiphernodesCommitteeSize::Medium.values(); + let committee = CiphernodesCommitteeSize::Small.values(); let bounds = Bounds::compute(BfvPreset::InsecureThreshold512, &committee).unwrap();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/zk-helpers/src/circuits/threshold/pk_generation/codegen.rs` around lines 158 - 197, The test mixes committee sizes: it generates the sample with CiphernodesCommitteeSize::Small via PkGenerationCircuit::generate_sample but later computes bounds and bits using CiphernodesCommitteeSize::Medium; make them consistent by using the same enum variant for both generation and validation (replace CiphernodesCommitteeSize::Medium with CiphernodesCommitteeSize::Small before calling Bounds::compute and Bits::compute) so Bounds::compute(...) and Bits::compute(...) use the same committee values as the sample.circuits/benchmarks/README.md (1)
191-191:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse committee-suffixed secure results paths consistently.
Line 191 still points to
results_secure_{agg|no_agg}/, but the documented layout now includes_<committee>. This can misdirect report regeneration commands.Suggested doc fix
-For secure mode, use `--mode secure` and the `results_secure_{agg|no_agg}/` directories. +For secure mode, use `--mode secure` and the `results_secure_{agg|no_agg}_<committee>/` directories.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@circuits/benchmarks/README.md` at line 191, Update the README reference that currently points to results_secure_{agg|no_agg}/ so it matches the new documented layout by using committee-suffixed paths (e.g., results_secure_{agg|no_agg}_<committee>/); change every occurrence of results_secure_{agg|no_agg}/ in the secure-mode example text to include the _{committee} suffix and ensure the example command snippets and any mention of report regeneration refer to results_secure_{agg|no_agg}_<committee>/ consistently.crates/aggregator/src/threshold_plaintext_aggregator.rs (1)
304-358:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftExpulsions never shrink the honest-share target.
required_sharescomes fromself.honest_committee_addresses.len(), but this method only removes the expelled party fromsharesandc6_proofs. If an honest member is expelled before contributing, the actor still waits for the oldHand can get stuck inCollectingforever.Store enough party-id↔honest-roster mapping to remove expelled members from the honest roster before recomputing
required_shares.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/aggregator/src/threshold_plaintext_aggregator.rs` around lines 304 - 358, The code in handle_member_expelled uses required_shares (derived from self.honest_committee_addresses.len()) but never updates that honest roster when a party is expelled, so the actor can still wait for the old H; modify handle_member_expelled to remove the expelled party_id from the honest roster (or from whatever party-id↔honest-roster mapping you maintain) before recomputing required_shares, then recompute required_shares and use that updated value for the subsequent comparisons (instead of the earlier captured required_shares). Ensure you update the same mapping used by self.aggregated_committee_n() (or remove from self.honest_committee_addresses) so that the checks around shares.len() and the transition to VerifyingC6 use the shrunk honest-share target.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@circuits/benchmarks/scripts/ensure_circuit_preset_built.sh`:
- Around line 19-22: The --committee case currently assigns COMMITTEE="$2"
without validating that a value exists or that $2 isn't another flag; add a
guard in the --committee branch to check that "$2" is non-empty and does not
start with '-' (treat as missing), and if invalid print the script usage and
exit non‑zero; only then set COMMITTEE="$2" and shift 2. Also update the
script's usage/help text to include the --committee option so the displayed
usage remains consistent. Ensure the same validation pattern is applied to the
other flag handling instance referenced (the other --committee handling at the
later switch).
In `@circuits/benchmarks/scripts/generate_prover_toml.sh`:
- Line 114: Before building the CMD array that runs zk_cli, validate that the
COMMITTEE_NAME variable is set and non-empty; if empty, print an error to stderr
and exit with non-zero status. Specifically, add a check prior to the CMD=(...)
line that tests COMMITTEE_NAME (e.g., [ -n "$COMMITTEE_NAME" ]), and on failure
call echo >&2 with a clear message like "COMMITTEE_NAME is not set" and exit 1
so the subsequent use of COMMITTEE_NAME in the CMD array cannot proceed
silently.
In `@circuits/benchmarks/scripts/load_default_committee.sh`:
- Around line 57-60: After extracting COMMITTEE_N, COMMITTEE_T, and COMMITTEE_H
from "$committee_file", add a guard that checks if any of these variables are
empty and fail fast: if any of COMMITTEE_N, COMMITTEE_T, or COMMITTEE_H is
unset/empty, print an error to stderr including the committee_file path and the
parsed values, then exit 1 before the export; this ensures the script does not
continue with empty env vars (reference the COMMITTEE_N/COMMITTEE_T/COMMITTEE_H
variables and the export COMMITTEE_NAME COMMITTEE_N COMMITTEE_T COMMITTEE_H
line).
In `@circuits/benchmarks/scripts/regenerate_report.sh`:
- Around line 58-65: The script currently assigns OUTPUT_COMMITTEE from
COMMITTEE_OVERRIDE without validating it, which can let bad values fall through
into benchmark_results_dir_path and pick wrong legacy directories; change the
flow so that when COMMITTEE_OVERRIDE is set you validate it (e.g., call an
existing validation helper or a new function like validate_committee_name or
load_committee_from_override) before setting OUTPUT_COMMITTEE, and if validation
fails exit with a non‑zero status and an explanatory error; if the override is
valid keep using OUTPUT_COMMITTEE="$COMMITTEE_OVERRIDE", otherwise fall back to
the existing load_default_committee/COMMITTEE_NAME path only after explicit
validation.
In `@circuits/benchmarks/scripts/run_benchmarks.sh`:
- Around line 153-163: When --skip-compile is used together with a CLI
--committee (COMMITTEE_OVERRIDE) ensure the script validates that the active
committee on disk matches OUTPUT_COMMITTEE: call load_default_committee (or
otherwise read COMMITTEE_NAME from disk) to get the current on-disk committee
and, if it differs from COMMITTEE_OVERRIDE/OUTPUT_COMMITTEE, exit non‑zero with
a clear error; apply the same guard in both places where OUTPUT_COMMITTEE is
resolved (the block using COMMITTEE_OVERRIDE/ load_default_committee and the
similar block around lines 217-231) so results cannot be mis-labeled when
skipping compilation.
In `@circuits/lib/src/configs/committee/small/mod.nr`:
- Around line 9-18: The module declarations pub mod parity_insecure and pub mod
parity_secure in the small committee config will cause a compile error because
those generated parity files do not exist; fix this by either (a) gating those
declarations with a cfg flag (e.g., add #[cfg(feature = "parity_files")] above
both pub mod parity_insecure; pub mod parity_secure; and enable that feature
only after parity generation) or (b) temporarily remove/comment out those pub
mod lines until the parity matrix generator produces the files, and ensure
committee::active = small is only used when the files/feature are present;
update your build (Cargo.toml features or build script) to set the feature or
invoke the generator prior to building.
In `@crates/keyshare/src/threshold_keyshare.rs`:
- Around line 1215-1225: Replace the debug-only check with a runtime-enforced
check: instead of debug_assert_eq!(recipient_pks.len(),
derived_committee_size.values().n as usize, ...), perform a real conditional
(e.g. if recipient_pks.len() != derived_committee_size.values().n as usize) and
return an error (or use ensure!/bail! if using anyhow/thiserror) with a clear
message including recipient_pks.len() and derived_committee_size.values().n;
this ensures encrypt_all_extended_for_share_indices and the C3/C4 NodeFold slot
sizing is not corrupted by a roster/committee size drift.
In `@crates/test-helpers/src/ciphernode_system.rs`:
- Around line 252-267: The loop in take_history_until_last_event checks total_to
only before awaiting, so an awaited call to take_history_with_timeouts can
exceed total_to; compute the remaining total timeout each iteration (remaining =
total_to.saturating_sub(start.elapsed())), bail if remaining is zero/expired,
and pass the minimum of event_to and remaining as the per-call timeout to
take_history_with_timeouts (use the function name take_history_with_timeouts and
the variables start, total_to, event_to, index, collected to locate the spot).
This ensures each await honors the overall total_to and avoids overrunning it.
In `@crates/tests/tests/integration.rs`:
- Around line 166-213: active_committee() reads circuits/bin/.active-preset.json
but setup_test_zk_backend() may prefer dist/circuits/<preset>/, causing
mismatched committee/threshold metadata; fix by centralizing parameter
selection: add a single helper (e.g., resolve_active_preset()) that first
detects which preset path is actually being used by setup_test_zk_backend (check
for dist/circuits/<preset>/ existence) and then reads the corresponding
.active-preset.json from that resolved directory, and change both
active_committee() and setup_test_zk_backend() to call this helper so
committee_h, threshold_m/n and benchmark metadata come from the same stamp.
In `@crates/zk-prover/src/actors/node_proof_aggregator.rs`:
- Around line 123-136: When build_pair_for_preset(...) fails inside
NodeProofAggregator (the Err(e) branch that currently calls
self.pending_inner_proofs.remove(&e3_id) and returns), instead of returning
silently you must publish a terminal failure for that round: remove the
prebuffered proof as now, update/emit the appropriate terminal event (E3Failed
for the given e3_id) and transition any aggregator state so the round completes
(so downstream logic can emit DKGRecursiveAggregationComplete if applicable).
Concretely, in the Err(e) branch around build_pair_for_preset, call the existing
event-emission path that emits E3Failed with the e3_id and error info, ensure
any per-e3 state is cleaned (pending_inner_proofs.remove(&e3_id)), and do not
simply return without emitting the terminal event.
In `@scripts/build-circuits.ts`:
- Around line 481-487: Extract the block that runs
setNoirConfigPreset(modNrPath, preset) plus the conditional
setNoirCommittee(this.options.committee),
regenerateParityMatrices(this.options.committee), and
patchUtilsTs(this.options.committee) into a shared helper (e.g.,
syncPresetAndCommittee(modNrPath, preset, committee)) and replace the inline
block with a call to that helper; then invoke this helper before any
early-return hydrate-only/fast-path branches (including the --hydrate-bin-only
and "dist is current; hydrating circuits/bin from dist" path) so the files
referenced by modNrPath, default/mod.nr, committee/active.nr, and
packages/enclave-contracts/scripts/utils.ts stay in sync with
circuits/bin/.active-preset.json.
- Around line 109-117: The code currently only warns when
isPresetCommitteeSupported(this.options.preset, this.options.committee) is
false, which lets the build produce broken artifacts; change this in the
build-circuits flow to fail fast: after detecting the unsupported pair in the
same if block (referencing this.options.preset and this.options.committee and
the isPresetCommitteeSupported helper) log an error and abort the process with a
non-zero exit (throw an Error or call process.exit(1)) unless an explicit force
flag is present (e.g. this.options.force or a --force CLI flag) — if a force
flag exists allow the warning and continue, otherwise treat the condition as a
hard error.
In `@scripts/check-committee.sh`:
- Line 160: The final summary prints stamp/parity text based only on whether
STAMP and GEN_BIN variables are set, which misrepresents skipped checks;
introduce boolean flags (e.g., RAN_STAMP_CHECK and RAN_PARITY_CHECK) that are
set to true at the points where the stamp check and parity (parity_*.nr /
GEN_BIN) checks actually run, then change the echo line that references STAMP
and GEN_BIN to conditionally include ", .active-preset.json" and ", parity_*.nr"
only when the corresponding RAN_STAMP_CHECK or RAN_PARITY_CHECK is true; keep
the existing ACTIVE_COMMITTEE, EXPECTED_H, EXPECTED_T variables in the message
and ensure the new flags are initialized to false before any checks.
- Around line 102-129: The function format_parity_matrices_for_committee
overwrites live parity files before running "nargo fmt" and does not guarantee
restoration on failure; modify it to perform the live-file swap inside a guarded
section that registers a trap to restore originals (using the backup variables:
backup, live, fresh, formatted) on EXIT or ERR, or alternatively avoid touching
live files at all by formatting only the temp copies (fresh → formatted) and
then atomically replacing live with formatted only after nargo fmt succeeds;
ensure the trap is removed or no-op'd after successful completion so backups are
not restored unnecessarily.
In `@scripts/generate-verifiers.ts`:
- Around line 140-145: The constructor currently sets this.verifierDir using
options.committee before the effective committee is resolved (via
targetCommittee()/active-preset.json), which can cause writes to the canonical
honk/ when no --committee is passed; change the flow so the effective committee
is resolved first (call targetCommittee() or read active-preset.json) and then
compute this.verifierDir, or alternatively defer computing this.verifierDir
until generate() and recompute it there when options.outputDir is not provided;
update any code that references this.verifierDir (constructor, generate(), and
targetCommittee() call sites) to use the resolved committee value.
---
Outside diff comments:
In `@circuits/benchmarks/README.md`:
- Line 191: Update the README reference that currently points to
results_secure_{agg|no_agg}/ so it matches the new documented layout by using
committee-suffixed paths (e.g., results_secure_{agg|no_agg}_<committee>/);
change every occurrence of results_secure_{agg|no_agg}/ in the secure-mode
example text to include the _{committee} suffix and ensure the example command
snippets and any mention of report regeneration refer to
results_secure_{agg|no_agg}_<committee>/ consistently.
In `@crates/aggregator/src/threshold_plaintext_aggregator.rs`:
- Around line 304-358: The code in handle_member_expelled uses required_shares
(derived from self.honest_committee_addresses.len()) but never updates that
honest roster when a party is expelled, so the actor can still wait for the old
H; modify handle_member_expelled to remove the expelled party_id from the honest
roster (or from whatever party-id↔honest-roster mapping you maintain) before
recomputing required_shares, then recompute required_shares and use that updated
value for the subsequent comparisons (instead of the earlier captured
required_shares). Ensure you update the same mapping used by
self.aggregated_committee_n() (or remove from self.honest_committee_addresses)
so that the checks around shares.len() and the transition to VerifyingC6 use the
shrunk honest-share target.
In `@crates/zk-helpers/src/circuits/threshold/pk_generation/codegen.rs`:
- Around line 158-197: The test mixes committee sizes: it generates the sample
with CiphernodesCommitteeSize::Small via PkGenerationCircuit::generate_sample
but later computes bounds and bits using CiphernodesCommitteeSize::Medium; make
them consistent by using the same enum variant for both generation and
validation (replace CiphernodesCommitteeSize::Medium with
CiphernodesCommitteeSize::Small before calling Bounds::compute and
Bits::compute) so Bounds::compute(...) and Bits::compute(...) use the same
committee values as the sample.
🪄 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: CHILL
Plan: Pro
Run ID: fcf53983-759e-4880-9dc0-6c1c393655dd
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (63)
.husky/pre-pushagent/RULES.mdcircuits/benchmarks/README.mdcircuits/benchmarks/results_insecure_agg/benchmark_run_meta.jsoncircuits/benchmarks/results_insecure_agg/crisp_verify_gas.jsoncircuits/benchmarks/results_insecure_agg/integration_summary.jsoncircuits/benchmarks/results_insecure_agg/report.mdcircuits/benchmarks/results_insecure_no_agg/benchmark_run_meta.jsoncircuits/benchmarks/results_insecure_no_agg/crisp_verify_gas.jsoncircuits/benchmarks/results_insecure_no_agg/integration_summary.jsoncircuits/benchmarks/results_insecure_no_agg/report.mdcircuits/benchmarks/scripts/benchmark_output_dir.shcircuits/benchmarks/scripts/ensure_circuit_preset_built.shcircuits/benchmarks/scripts/generate_prover_toml.shcircuits/benchmarks/scripts/generate_report.shcircuits/benchmarks/scripts/load_default_committee.shcircuits/benchmarks/scripts/regenerate_report.shcircuits/benchmarks/scripts/run_benchmarks.shcircuits/lib/src/configs/committee/active.nrcircuits/lib/src/configs/committee/medium/mod.nrcircuits/lib/src/configs/committee/medium/parity_insecure.nrcircuits/lib/src/configs/committee/medium/parity_secure.nrcircuits/lib/src/configs/committee/micro.nrcircuits/lib/src/configs/committee/micro/mod.nrcircuits/lib/src/configs/committee/micro/parity_insecure.nrcircuits/lib/src/configs/committee/micro/parity_secure.nrcircuits/lib/src/configs/committee/mod.nrcircuits/lib/src/configs/committee/small.nrcircuits/lib/src/configs/committee/small/mod.nrcircuits/lib/src/configs/committee/small/parity_insecure.nrcircuits/lib/src/configs/committee/small/parity_secure.nrcircuits/lib/src/configs/default/mod.nrcircuits/lib/src/configs/insecure/dkg.nrcircuits/lib/src/configs/secure/dkg.nrcircuits/lib/src/math/committee_hash.nrcrates/aggregator/src/ext.rscrates/aggregator/src/publickey_aggregator.rscrates/aggregator/src/threshold_plaintext_aggregator.rscrates/events/src/enclave_event/publickey_aggregated.rscrates/keyshare/src/threshold_keyshare.rscrates/test-helpers/src/ciphernode_system.rscrates/tests/Cargo.tomlcrates/tests/tests/integration.rscrates/zk-helpers/src/bin/generate_parity_matrices.rscrates/zk-helpers/src/bin/zk_cli.rscrates/zk-helpers/src/ciphernodes_committee.rscrates/zk-helpers/src/circuits/dkg/share_computation/computation.rscrates/zk-helpers/src/circuits/threshold/pk_generation/codegen.rscrates/zk-helpers/src/circuits/threshold/pk_generation/computation.rscrates/zk-prover/src/actors/node_proof_aggregator.rscrates/zk-prover/tests/common/helpers.rscrates/zk-prover/tests/common/mod.rscrates/zk-prover/tests/fold_accumulators_e2e_tests.rscrates/zk-prover/tests/local_e2e_tests.rscrates/zk-prover/tests/node_fold_correlated_e2e_tests.rspackage.jsonpackages/enclave-contracts/scripts/utils.tspackages/enclave-contracts/test/fixtures/bfv_vk_binding/folded_artifacts.jsonscripts/README.mdscripts/build-circuits.tsscripts/check-committee.shscripts/circuit-constants.tsscripts/generate-verifiers.ts
💤 Files with no reviewable changes (4)
- circuits/benchmarks/results_insecure_no_agg/benchmark_run_meta.json
- circuits/benchmarks/results_insecure_no_agg/crisp_verify_gas.json
- circuits/lib/src/configs/committee/micro.nr
- circuits/lib/src/configs/committee/small.nr
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
agent/flow-trace/04_DKG_AND_COMPUTATION.md (1)
561-576:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlign C5 wording with the new canonical-H aggregation rule.
This block correctly says C5 uses H keyshares, but nearby C5 prose still refers to aggregating from “all pk_shares.” Please update that wording so the section is internally consistent with the H-subset model.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agent/flow-trace/04_DKG_AND_COMPUTATION.md` around lines 561 - 576, Update the C5 wording to consistently reference the H-canonical subset rather than "all pk_shares": change any prose that says aggregating from "all pk_shares" to state aggregation from the H canonical honest parties (e.g., when describing Fhe::get_aggregate_public_key, PublicKeyShare::aggregate, aggregate_pk, pk_hash and the PkAggregationProofRequest/ PkAggregationProofPending proof_request) so the text clearly indicates C5 uses H keyshares for compute_pk_commitment and the PkAggregationProofRequest.
♻️ Duplicate comments (1)
scripts/check-committee.sh (1)
113-145:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRegister restore trap before the first live-file overwrite.
The restore guard is installed only at Line 139, after live files are already swapped. If a copy fails earlier in the loop, the script can exit with mutated tracked files.
Suggested fix
format_parity_matrices_for_committee() { local committee="$1" local tmp="$2" local variant live fresh backup formatted local -a swapped_live=() local -a swapped_backup=() _restore_swapped_parity_live() { local i for i in "${!swapped_live[@]}"; do if [[ -f "${swapped_backup[$i]}" ]]; then cp "${swapped_backup[$i]}" "${swapped_live[$i]}" fi done } + trap '_restore_swapped_parity_live' ERR for variant in insecure secure; do live="$NOIR_LIB/src/configs/committee/$committee/parity_${variant}.nr" fresh="$tmp/$committee/parity_${variant}.nr" [[ -f "$live" && -f "$fresh" ]] || continue backup="$tmp/$committee/parity_${variant}.live.bak" formatted="$tmp/$committee/parity_${variant}.formatted.nr" cp "$live" "$backup" cp "$fresh" "$live" swapped_live+=("$live") swapped_backup+=("$backup") done - ((${`#swapped_live`[@]} == 0)) && return 0 - - trap '_restore_swapped_parity_live' ERR + if ((${`#swapped_live`[@]} == 0)); then + trap - ERR + return 0 + fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/check-committee.sh` around lines 113 - 145, The trap to restore swapped files is registered too late (after files are already overwritten); move the trap registration so it runs before any live->backup swaps occur: register trap '_restore_swapped_parity_live' ERR immediately before the for-variant loop that performs cp into "$live" (so any early failure will call _restore_swapped_parity_live using the swapped_live and swapped_backup arrays), and keep the existing explicit calls to _restore_swapped_parity_live and "trap - ERR" on the error path to avoid double-restores; ensure the trap is only removed after successful nargo fmt completes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@agent/flow-trace/04_DKG_AND_COMPUTATION.md`:
- Around line 561-576: Update the C5 wording to consistently reference the
H-canonical subset rather than "all pk_shares": change any prose that says
aggregating from "all pk_shares" to state aggregation from the H canonical
honest parties (e.g., when describing Fhe::get_aggregate_public_key,
PublicKeyShare::aggregate, aggregate_pk, pk_hash and the
PkAggregationProofRequest/ PkAggregationProofPending proof_request) so the text
clearly indicates C5 uses H keyshares for compute_pk_commitment and the
PkAggregationProofRequest.
---
Duplicate comments:
In `@scripts/check-committee.sh`:
- Around line 113-145: The trap to restore swapped files is registered too late
(after files are already overwritten); move the trap registration so it runs
before any live->backup swaps occur: register trap
'_restore_swapped_parity_live' ERR immediately before the for-variant loop that
performs cp into "$live" (so any early failure will call
_restore_swapped_parity_live using the swapped_live and swapped_backup arrays),
and keep the existing explicit calls to _restore_swapped_parity_live and "trap -
ERR" on the error path to avoid double-restores; ensure the trap is only removed
after successful nargo fmt completes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f785d06c-e65e-4e60-8829-d2b48c5af489
📒 Files selected for processing (32)
agent/flow-trace/04_DKG_AND_COMPUTATION.mdcircuits/benchmarks/results_insecure_agg/crisp_verify_gas.jsoncircuits/benchmarks/results_insecure_agg/integration_summary.jsoncircuits/benchmarks/results_insecure_agg_micro/benchmark_run_meta.jsoncircuits/benchmarks/results_insecure_agg_micro/crisp_verify_gas.jsoncircuits/benchmarks/results_insecure_agg_micro/integration_summary.jsoncircuits/benchmarks/results_insecure_agg_micro/report.mdcircuits/benchmarks/results_insecure_no_agg_micro/benchmark_run_meta.jsoncircuits/benchmarks/results_insecure_no_agg_micro/crisp_verify_gas.jsoncircuits/benchmarks/results_insecure_no_agg_micro/integration_summary.jsoncircuits/benchmarks/results_insecure_no_agg_micro/report.mdcircuits/benchmarks/results_secure_agg/benchmark_run_meta.jsoncircuits/benchmarks/results_secure_agg/crisp_verify_gas.jsoncircuits/benchmarks/results_secure_agg/integration_summary.jsoncircuits/benchmarks/results_secure_no_agg/crisp_verify_gas.jsoncircuits/benchmarks/results_secure_no_agg/report.mdcircuits/benchmarks/scripts/ensure_circuit_preset_built.shcircuits/benchmarks/scripts/generate_prover_toml.shcircuits/benchmarks/scripts/load_default_committee.shcircuits/benchmarks/scripts/regenerate_report.shcircuits/benchmarks/scripts/run_benchmarks.shcircuits/lib/src/configs/committee/small/mod.nrcircuits/lib/src/configs/default/mod.nrcircuits/lib/src/configs/default/mod.nr.benchmark_backupcrates/aggregator/src/publickey_aggregator.rscrates/keyshare/src/threshold_keyshare.rscrates/test-helpers/src/ciphernode_system.rscrates/tests/tests/integration.rscrates/zk-prover/src/actors/node_proof_aggregator.rsscripts/build-circuits.tsscripts/check-committee.shscripts/generate-verifiers.ts
💤 Files with no reviewable changes (7)
- circuits/benchmarks/results_secure_agg/benchmark_run_meta.json
- circuits/benchmarks/results_secure_agg/crisp_verify_gas.json
- circuits/benchmarks/results_secure_agg/integration_summary.json
- circuits/benchmarks/results_insecure_agg/crisp_verify_gas.json
- circuits/benchmarks/results_insecure_agg/integration_summary.json
- circuits/benchmarks/results_secure_no_agg/crisp_verify_gas.json
- circuits/benchmarks/results_secure_no_agg/report.md
✅ Files skipped from review due to trivial changes (5)
- circuits/lib/src/configs/default/mod.nr.benchmark_backup
- circuits/benchmarks/results_insecure_agg_micro/integration_summary.json
- circuits/benchmarks/results_insecure_no_agg_micro/report.md
- circuits/lib/src/configs/committee/small/mod.nr
- circuits/benchmarks/results_insecure_agg_micro/crisp_verify_gas.json
🚧 Files skipped from review as they are similar to previous changes (12)
- circuits/lib/src/configs/default/mod.nr
- circuits/benchmarks/scripts/generate_prover_toml.sh
- crates/keyshare/src/threshold_keyshare.rs
- circuits/benchmarks/scripts/load_default_committee.sh
- circuits/benchmarks/scripts/ensure_circuit_preset_built.sh
- circuits/benchmarks/scripts/regenerate_report.sh
- circuits/benchmarks/scripts/run_benchmarks.sh
- crates/zk-prover/src/actors/node_proof_aggregator.rs
- scripts/generate-verifiers.ts
- crates/tests/tests/integration.rs
- crates/aggregator/src/publickey_aggregator.rs
- scripts/build-circuits.ts
ctrlc03
reviewed
May 27, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/enclave-sdk/package.json (1)
49-49: ⚡ Quick winCircuit compilation on every test run may impact developer experience.
The
pretesthook compiles circuits before everypnpm testinvocation. While this ensures circuits are up-to-date, it may significantly slow down the test feedback loop during development, especially if circuit compilation takes seconds or minutes.Consider:
- Adding a check to skip compilation if circuits are already built and up-to-date
- Documenting the expected compilation time
- Providing a
test:quickscript that skips circuit compilation for rapid iteration🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/enclave-sdk/package.json` at line 49, The pretest hook currently runs "compile:circuits" on every pnpm test which slows iteration; update package.json scripts to (1) change or augment the "pretest" behavior so compilation is skipped when circuits are already up-to-date by wrapping the existing "compile:circuits" invocation with a pre-check (e.g., a small script or npm script that compares artifact timestamps or exits early if build outputs exist) and (2) add a new "test:quick" script that runs "pnpm test" without invoking the circuit compilation step so developers can run fast tests; refer to the "pretest" script name and the "compile:circuits" script as the symbols to modify and add the "test:quick" script entry in package.json.crates/data/src/data_store.rs (2)
251-260: ⚡ Quick winConsider testing actual persistence behavior, not just scope string formation.
The test validates that chained
scope()calls produce the expected string ("//foo/bar/baz"), but doesn't verify that data written to that scope can be read back correctly. Given that the relevant code snippets show scope affects bothDataStorereads andRepositoryoperations, consider adding assertions that:
- Write a value to the scoped store
- Read it back and verify correctness
- Ensure the scope isolation works (data in
"/foo"doesn't appear in"/foo/bar")This would catch any issues in the scope byte handling beyond string formation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/data/src/data_store.rs` around lines 251 - 260, The test scope_normalizes_slashes only asserts the scope string; extend it to verify persistence by writing and reading through the scoped DataStore to ensure scope bytes are applied: after creating scoped via DataStore::from(&addr).base("//foo").scope("bar").scope("/baz"), use the store's write API (e.g., put/save) to store a test key/value in that scoped store, then read it back with the store's read API (e.g., get/load) and assert the value matches; also write a different value into the parent scope (e.g., store.base("//foo").put(...)) and assert it is not visible from the child scoped store to confirm isolation. Ensure you use the same unique symbols from this file (InMemStore::new, DataStore::from, base, scope, get_scope) when locating where to add the write/read assertions.
130-131: 💤 Low valueClarify documentation example with double leading slash.
The example uses
base("//foo")with a double leading slash. While this may be intentional to test edge cases, it could confuse readers about the expected usage pattern. Consider:
- Using a single leading slash if that's the typical pattern:
base("/foo")- Adding a comment explaining that multiple leading slashes are preserved verbatim
- Clarifying whether
"//foo"is a realistic use case or just an edge-case demonstrationThe phrasing "Non-leading segments get a
/separator" could also be more precise: "Segments not starting with/get a/separator prepended before concatenation."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/data/src/data_store.rs` around lines 130 - 131, Update the doc comment on the path builder (the example using base("//foo").scope("bar").scope("/baz")) to avoid confusion: either change the example to use a typical single-leading-slash input like base("/foo").scope("bar").scope("/baz"), or add a brief note that multiple leading slashes are preserved verbatim (i.e., "//foo" is an allowed edge-case) so readers know it's intentional; also rephrase the sentence "Non-leading segments get a `/` separator" to "Segments not starting with `/` get a `/` separator prepended before concatenation" to be more precise; make these edits in the doc block that documents base(...) and scope(...).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/enclave-sdk/package.json`:
- Line 43: The package hardcodes `--committee micro` in the `compile:circuits`
script and the SDK defaults `committeeSize: 0` in
`packages/enclave-sdk/src/utils.ts` while tests use
`require_micro_circuits()`—either parameterize the `compile:circuits` script to
accept a committee preset via environment/shared config/`.active-preset.json`
(so `compile:circuits` uses that value instead of `--committee micro`) and
propagate that preset into `utils.ts` (set `committeeSize` from the same
source), or add a runtime fast-fail in the SDK (e.g., in the module that reads
`.active-preset.json` or in the init path that loads
`circuits/bin/*/user_data_encryption*.json`) that checks the active preset is
`micro` and throws a clear error if not; update tests/helpers
(`require_micro_circuits()`) to rely on the new parameter or the explicit
fast-fail.
---
Nitpick comments:
In `@crates/data/src/data_store.rs`:
- Around line 251-260: The test scope_normalizes_slashes only asserts the scope
string; extend it to verify persistence by writing and reading through the
scoped DataStore to ensure scope bytes are applied: after creating scoped via
DataStore::from(&addr).base("//foo").scope("bar").scope("/baz"), use the store's
write API (e.g., put/save) to store a test key/value in that scoped store, then
read it back with the store's read API (e.g., get/load) and assert the value
matches; also write a different value into the parent scope (e.g.,
store.base("//foo").put(...)) and assert it is not visible from the child scoped
store to confirm isolation. Ensure you use the same unique symbols from this
file (InMemStore::new, DataStore::from, base, scope, get_scope) when locating
where to add the write/read assertions.
- Around line 130-131: Update the doc comment on the path builder (the example
using base("//foo").scope("bar").scope("/baz")) to avoid confusion: either
change the example to use a typical single-leading-slash input like
base("/foo").scope("bar").scope("/baz"), or add a brief note that multiple
leading slashes are preserved verbatim (i.e., "//foo" is an allowed edge-case)
so readers know it's intentional; also rephrase the sentence "Non-leading
segments get a `/` separator" to "Segments not starting with `/` get a `/`
separator prepended before concatenation" to be more precise; make these edits
in the doc block that documents base(...) and scope(...).
In `@packages/enclave-sdk/package.json`:
- Line 49: The pretest hook currently runs "compile:circuits" on every pnpm test
which slows iteration; update package.json scripts to (1) change or augment the
"pretest" behavior so compilation is skipped when circuits are already
up-to-date by wrapping the existing "compile:circuits" invocation with a
pre-check (e.g., a small script or npm script that compares artifact timestamps
or exits early if build outputs exist) and (2) add a new "test:quick" script
that runs "pnpm test" without invoking the circuit compilation step so
developers can run fast tests; refer to the "pretest" script name and the
"compile:circuits" script as the symbols to modify and add the "test:quick"
script entry in package.json.
🪄 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: CHILL
Plan: Pro
Run ID: 0c85718f-8c69-4429-9c1e-4c8ddc1f1c8d
📒 Files selected for processing (14)
agent/flow-trace/04_DKG_AND_COMPUTATION.mdcircuits/benchmarks/results_secure_agg_micro/benchmark_run_meta.jsoncircuits/benchmarks/results_secure_agg_micro/crisp_verify_gas.jsoncircuits/benchmarks/results_secure_agg_micro/integration_summary.jsoncircuits/benchmarks/results_secure_agg_micro/report.mdcircuits/benchmarks/results_secure_no_agg_micro/benchmark_run_meta.jsoncircuits/benchmarks/results_secure_no_agg_micro/crisp_verify_gas.jsoncircuits/benchmarks/results_secure_no_agg_micro/integration_summary.jsoncircuits/benchmarks/results_secure_no_agg_micro/report.mdcircuits/lib/src/configs/default/mod.nrcrates/aggregator/src/threshold_plaintext_aggregator.rscrates/data/src/data_store.rspackages/enclave-sdk/package.jsonscripts/check-committee.sh
✅ Files skipped from review due to trivial changes (6)
- circuits/benchmarks/results_secure_no_agg_micro/crisp_verify_gas.json
- circuits/benchmarks/results_secure_agg_micro/report.md
- circuits/benchmarks/results_secure_agg_micro/crisp_verify_gas.json
- circuits/benchmarks/results_secure_no_agg_micro/integration_summary.json
- circuits/benchmarks/results_secure_no_agg_micro/report.md
- circuits/benchmarks/results_secure_agg_micro/integration_summary.json
🚧 Files skipped from review as they are similar to previous changes (4)
- circuits/lib/src/configs/default/mod.nr
- agent/flow-trace/04_DKG_AND_COMPUTATION.md
- scripts/check-committee.sh
- crates/aggregator/src/threshold_plaintext_aggregator.rs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/aggregator/src/publickey_aggregator.rs (2)
1121-1135:⚠️ Potential issue | 🟠 Major | ⚡ Quick winScope no-aggregation detection to honest parties only.
all_proofs_are_nonecurrently inspects every buffered proof; a non-honestSome(proof)can prevent publish even when all honest entries areNone.Suggested fix
- Some(all_present && dkg_node_proofs.values().all(|p| p.is_none())) + let honest_all_none = honest_party_ids.iter().all(|id| { + dkg_node_proofs + .get(id) + .map(|p| p.is_none()) + .unwrap_or(false) + }); + Some(all_present && honest_all_none)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/aggregator/src/publickey_aggregator.rs` around lines 1121 - 1135, The check computing all_proofs_are_none currently uses dkg_node_proofs.values().all(|p| p.is_none()) which includes non-honest parties; change the logic to only consider honest_party_ids: after confirming all_present, test that every honest id maps to Some(None) in dkg_node_proofs (e.g. honest_party_ids.iter().all(|id| matches!(dkg_node_proofs.get(id), Some(None)))). Update the branch inside PublicKeyAggregatorState::GeneratingC5Proof to replace the values() check with this honest-only iteration so that non-honest Some(proof) entries do not block publishing.
785-791:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not drop honest DKG fold messages when committee dims are unset.
This early return skips verification and skips buffering, which can deadlock progress after state recovery/migration where dims are temporarily unset.
Suggested fix
- if committee_n == 0 || committee_h == 0 { - warn!( - party_id = msg.party_id, - "DKG fold attestation verify skipped — circuit committee dims unset" - ); - return Ok(()); - } - if let Err(e) = verify_dkg_fold_attestation( - &self.e3_id, - msg.party_id, - proof, - attestation, - expected_node, - committee_n, - committee_h, - n_moduli, - ) { - warn!( - party_id = msg.party_id, - error = %e, - "DKG fold attestation verification failed — rejecting" - ); - return Ok(()); - } + if committee_n == 0 || committee_h == 0 { + warn!( + party_id = msg.party_id, + "DKG fold attestation verify skipped — circuit committee dims unset; accepting proof for buffering" + ); + } else if let Err(e) = verify_dkg_fold_attestation( + &self.e3_id, + msg.party_id, + proof, + attestation, + expected_node, + committee_n, + committee_h, + n_moduli, + ) { + warn!( + party_id = msg.party_id, + error = %e, + "DKG fold attestation verification failed — rejecting" + ); + return Ok(()); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/aggregator/src/publickey_aggregator.rs` around lines 785 - 791, The current code returns early when committee_n or committee_h are zero, which drops the DKG fold attestation (msg.party_id) and can deadlock progress after recovery; instead of returning Ok(()), keep the warn(...) but enqueue/buffer the incoming DKG fold attestation for later verification (when committee_n and committee_h are set). Replace the early return with logic that calls the existing buffering mechanism (e.g., push into the pending DKG fold/attestation queue or call the module's buffer/enqueue function used elsewhere), ensuring the message is persisted in the same pending storage used for other delayed verifications, then return Ok(()) after buffering. Ensure the buffered entry includes msg.party_id and the original message payload so verification can run once committee_n and committee_h are nonzero.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/aggregator/src/ext.rs`:
- Around line 206-219: load_honest_committee_addresses currently treats an empty
Vec from context or state as valid; update the loader (function
load_honest_committee_addresses) to enforce non-empty honest committee: when
reading ctx.get_dependency(HONEST_COMMITTEE_ADDRESSES_KEY) treat Some(addrs) as
valid only if !addrs.is_empty(), otherwise continue to repo read; likewise after
obtaining state.honest_committee_addresses(), return Ok(addrs.to_vec()) only if
!addrs.is_empty(), and return
Err(anyhow!(ERROR_TRBFV_PLAINTEXT_HONEST_COMMITTEE_MISSING)) when the vector is
empty so restart/hydrate cannot create a plaintext aggregator with an empty
roster.
---
Outside diff comments:
In `@crates/aggregator/src/publickey_aggregator.rs`:
- Around line 1121-1135: The check computing all_proofs_are_none currently uses
dkg_node_proofs.values().all(|p| p.is_none()) which includes non-honest parties;
change the logic to only consider honest_party_ids: after confirming
all_present, test that every honest id maps to Some(None) in dkg_node_proofs
(e.g. honest_party_ids.iter().all(|id| matches!(dkg_node_proofs.get(id),
Some(None)))). Update the branch inside
PublicKeyAggregatorState::GeneratingC5Proof to replace the values() check with
this honest-only iteration so that non-honest Some(proof) entries do not block
publishing.
- Around line 785-791: The current code returns early when committee_n or
committee_h are zero, which drops the DKG fold attestation (msg.party_id) and
can deadlock progress after recovery; instead of returning Ok(()), keep the
warn(...) but enqueue/buffer the incoming DKG fold attestation for later
verification (when committee_n and committee_h are set). Replace the early
return with logic that calls the existing buffering mechanism (e.g., push into
the pending DKG fold/attestation queue or call the module's buffer/enqueue
function used elsewhere), ensuring the message is persisted in the same pending
storage used for other delayed verifications, then return Ok(()) after
buffering. Ensure the buffered entry includes msg.party_id and the original
message payload so verification can run once committee_n and committee_h are
nonzero.
🪄 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: CHILL
Plan: Pro
Run ID: 90ebd2ed-a40f-432c-ae64-12d8eb349407
📒 Files selected for processing (15)
agent/flow-trace/04_DKG_AND_COMPUTATION.mdcircuits/benchmarks/README.mdcircuits/benchmarks/scripts/ensure_circuit_preset_built.shcircuits/benchmarks/scripts/extract_crisp_verify_gas.shcircuits/benchmarks/scripts/generate_prover_toml.shcircuits/benchmarks/scripts/load_default_committee.shcircuits/benchmarks/scripts/replay_folded_verify_gas.shcircuits/benchmarks/scripts/run_benchmarks.shcrates/aggregator/src/ext.rscrates/aggregator/src/publickey_aggregator.rscrates/keyshare/src/threshold_keyshare.rscrates/zk-helpers/src/ciphernodes_committee.rscrates/zk-helpers/src/circuits/threshold/pk_generation/computation.rspackages/enclave-contracts/scripts/utils.tsscripts/build-circuits.ts
✅ Files skipped from review due to trivial changes (2)
- packages/enclave-contracts/scripts/utils.ts
- circuits/benchmarks/README.md
🚧 Files skipped from review as they are similar to previous changes (4)
- circuits/benchmarks/scripts/generate_prover_toml.sh
- circuits/benchmarks/scripts/run_benchmarks.sh
- crates/zk-helpers/src/circuits/threshold/pk_generation/computation.rs
- scripts/build-circuits.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/enclave-sdk/src/circuits/assert-micro-circuits.ts`:
- Around line 26-27: The code sets the module-level flag checked to true before
validation completes, so if validation throws later callers will be skipped;
change the flow so the early-return if (checked) stays, but move checked = true
to execute only after the validation call completes successfully (i.e., after
the await/try block that runs the checks and not inside a catch), so that
checked is only set when validation has not thrown.
🪄 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: CHILL
Plan: Pro
Run ID: 33663d37-2040-4a59-8971-e4fa8a5517f8
📒 Files selected for processing (11)
circuits/benchmarks/README.mdcircuits/benchmarks/results_secure_agg_small/benchmark_run_meta.jsoncircuits/benchmarks/results_secure_agg_small/crisp_verify_gas.jsoncircuits/benchmarks/results_secure_agg_small/integration_summary.jsoncircuits/benchmarks/results_secure_agg_small/report.mdcrates/aggregator/src/ext.rspackages/enclave-sdk/package.jsonpackages/enclave-sdk/scripts/compile-circuits.shpackages/enclave-sdk/src/circuits/assert-micro-circuits.tspackages/enclave-sdk/src/crypto/user-data-encryption.tspackages/enclave-sdk/src/utils.ts
✅ Files skipped from review due to trivial changes (6)
- packages/enclave-sdk/src/utils.ts
- circuits/benchmarks/results_secure_agg_small/crisp_verify_gas.json
- circuits/benchmarks/results_secure_agg_small/report.md
- circuits/benchmarks/results_secure_agg_small/benchmark_run_meta.json
- circuits/benchmarks/README.md
- circuits/benchmarks/results_secure_agg_small/integration_summary.json
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/aggregator/src/ext.rs
ctrlc03
approved these changes
May 27, 2026
This was referenced Jun 4, 2026
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
2 participants
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 by CodeRabbit
New Features
Improvements
Chores