fix: wrong committee threshold for C5 agg [skip-line-limit]

[0xjei]

this will make N != H committee size work

Summary by CodeRabbit

• Bug Fixes

  • Corrected aggregation proof generation to use canonical committee dimensions and thresholds, improving proof accuracy and reliability.

• Tests

  • Expanded test coverage for non-square committees; added a test validating canonical committee dimensions and keyshare sizing in pending aggregation proofs.

• Chores

  • Added development dependencies to support the new tests.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
crisp	Ready	Preview, Comment	Jun 6, 2026 10:42am
enclave-docs	Ready	Preview, Comment	Jun 6, 2026 10:42am
enclave-enclave-dashboard	Ready	Preview, Comment	Jun 6, 2026 10:42am

[coderabbitai]

PR changed again? Review this PR in Change Stack to compare snapshots and stay oriented.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 19f7181e-e970-4d1a-9981-90ae365378b6

📥 Commits

Reviewing files that changed from the base of the PR and between 4f59223 and 5e658a6.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (2)

• crates/aggregator/Cargo.toml
• crates/aggregator/src/actors/publickey_aggregator.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• crates/aggregator/src/actors/publickey_aggregator.rs

---

📝 Walkthrough

Walkthrough

PkAggregationProofRequest construction for C5 proof pending now uses canonical committee dimensions: committee_n = threshold_n, committee_h preserved from circuit, and committee_threshold = threshold_m. Tests and helpers were added/refactored to validate non-square committees; dev-deps were added for those tests.

Changes

C5 Proof Request Dimensions

Layer / File(s)	Summary
PkAggregationProofRequest dimension fix crates/aggregator/src/actors/publickey_aggregator.rs	When emitting PkAggregationProofPending for C5, committee_n is taken from threshold_n (not committee_h) and committee_threshold is set to threshold_m (not 0); inline comment updated to separate witness keyshare dimensions from canonical artifact lookup dims.
Tests and helpers for non-square committees crates/aggregator/src/actors/publickey_aggregator.rs, crates/aggregator/Cargo.toml	Test import formatting and builder wiring refactored; helpers added to build non-square verification state and proof payloads; new test pk_aggregation_proof_pending_carries_canonical_committee_dims asserts committee_n == threshold_n, committee_h == circuit_h, committee_threshold == threshold_m, and keyshare_bytes length matches circuit_h. Dev-dependencies fhe, fhe-traits, and rand added for tests.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• gnosisguild/enclave#1537: Touches PublicKeyAggregator C5 proof-generation/pending flow and related error/terminal emission behavior.

Suggested reviewers

• ctrlc03

Poem

🐰 In circuits where the numbers play,
I hop the paths of N, H, T today.
Thresholds find their rightful place,
Keyshares trot with measured pace.
Proofs align — a tidy, hopping trace.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: wrong committee threshold for C5 agg' accurately describes the main change—correcting the committee threshold logic for C5 aggregation to support N != H configurations, which matches the core objective of the PR.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/h

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

crates/aggregator/src/actors/publickey_aggregator.rs (1)

299-306: ⚡ Quick win

Add a regression test for the N != H request shape.

This fixes the constructor, but the file still doesn't pin the emitted PkAggregationProofRequest for the non-square committee case that broke here. A small test around the published PkAggregationProofPending would lock in committee_n = threshold_n, committee_h = H, and committee_threshold = threshold_m so the artifact-lookup contract can't drift again. Based on the downstream PkAggregationProofRequest contract and C5 handler behavior in the supplied snippets.

🤖 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/actors/publickey_aggregator.rs` around lines 299 - 306,
Add a regression test that publishes a non-square (N != H) committee scenario
and asserts the emitted PkAggregationProofPending contains a
PkAggregationProofRequest with committee_n == threshold_n, committee_h == H, and
committee_threshold == threshold_m; locate where PkAggregationProofPending is
emitted in publickey_aggregator (search for PkAggregationProofPending and
PkAggregationProofRequest constructors) and create a test that drives that path,
captures the published event, and asserts those three fields exactly to lock the
artifact-lookup contract.

🤖 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.

Nitpick comments:
In `@crates/aggregator/src/actors/publickey_aggregator.rs`:
- Around line 299-306: Add a regression test that publishes a non-square (N !=
H) committee scenario and asserts the emitted PkAggregationProofPending contains
a PkAggregationProofRequest with committee_n == threshold_n, committee_h == H,
and committee_threshold == threshold_m; locate where PkAggregationProofPending
is emitted in publickey_aggregator (search for PkAggregationProofPending and
PkAggregationProofRequest constructors) and create a test that drives that path,
captures the published event, and asserts those three fields exactly to lock the
artifact-lookup contract.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 42c94f4c-ffcf-4e5d-8de5-6af7a56aa23e

📥 Commits

Reviewing files that changed from the base of the PR and between edb1477 and 4f59223.

📒 Files selected for processing (1)

• crates/aggregator/src/actors/publickey_aggregator.rs

[ctrlc03]

utACK