pkc%feat(bls): pin Chia and IETF scheme behaviors in unit tests, implement weighted aggregation API, Chia arm of aggregate verification, import aggregate corpus

[kwvg]

Additional Information

• Related to pkc%fix(bls): implement Fr,Fp{,2}, G{1,2}{,Affine} wrappers, use draft-03 keygen, improve conformance with bls-signatures #20

• Depends on pkc%feat(bls): migrate operational types to Bls{PublicKey,SecretKey,BlsSignature}<S>, reap bls_{chia,ietf} modules for unified bls module #25 and resolves some review comments (here, here and here)

  Description	Notes	Reference	Test
  Chia decodes a G1 public key outside the prime-order subgroup and re-serializes it to the input, IETF refuses it.	The validity check is gated on the scheme, so such a key is already in circulation and deemed valid	Valid on any successful decode (bls.h:102), the check gated on the scheme (elements.cpp:24)	off_subgroup_public_key_policy
  Chia decodes an off-subgroup G2 signature and re-serializes it to the input, IETF refuses it	See above	elements.cpp:245	off_subgroup_signature_policy
  Chia weights an off-subgroup peer key in DH, exposing the scalar mod the cofactor's small factors	Reachable with keys the consumer treats as valid.	Dash Core admits it (bls.cpp:206), there is no separate consensus rule (providertx.cpp:83), and the multiply is a bare g1_mul (privatekey.cpp:184)	off_subgroup_peer_policy
  IETF refuses that peer key at the decoder, so DH is never handed one	Such a key cannot be exercised since it's filtered out by the decoder itself.	elements.cpp:24	off_subgroup_peer_policy
  Both schemes refuse an x at or above the field prime, from the prime itself to an all-ones coordinate	Product of emergent behavior in Dash Core that results in effective rejection.	the read error is swallowed off the IETF path (elements.cpp:85), but the residue re-serializes to zeros, so the malleability check refuses it (bls.h:212) before ProRegTx ever sees a key (bls.h:492, providertx.cpp:83)	out_of_range_coordinate_rejected
  Chia accepts a repeated message in a per-signer-message aggregate; IETF does not.	-	Reaches that path under the legacy flag (bls.cpp:333), where it has no guard (schemes.cpp:695) while std::set is used in basic scheme (schemes.cpp:428)	verify_policy_duplicate_messages, aggregate_verify_matches_vectors
  Neither scheme decodes an identity public key or signature, in the canonical form or as zeros	-	Consumer rejects a decoded identity (bls.h:114) that FromBytesUnchecked hands back (elements.cpp:46)	identity_public_key_rejected, identity_signature_rejected
  A Chia identity signature encodes to 0xc0 over zeros, which no decoder accepts	The identity is reachable by computation and not off the wire, so emitting it and still refusing it is right	the infinity branch of the G2 encoder (elements.cpp:406)	chia_identity_encodes_canonically
  An aggregate cancelled to the identity verifies against any message under Chia, and none under IETF	-	nothing is checked before the pairing under Chia (schemes.cpp:670), and the IETF arm does check (schemes.cpp:140) but IsValid admits infinity (elements.cpp:358)	identity_cancellation_follows_scheme
  Chia masks stray high bits on a public key, and bits 6 and 7 together read as the infinity marker it refuses	Single-scheme, since IETF's high bits are meaningful flags rather than maskable padding	erased before the read (elements.cpp:41)	chia_masks_stray_public_key_bits, chia_masks_stray_bit_six
  Chia refuses stray high bits on a signature rather than masking them	Stricter than the G2 decoder, which has no mask and lets the bits fall into the coordinate, but a point read that way never verifies	copied in unmasked (elements.cpp:262), where the IETF path erases the top three (:266) and only the sign bit is taken back out (:285)	chia_rejects_stray_signature_bits
  Both schemes refuse an empty aggregation instead of returning the identity	The library would call zero keys over an identity signature good, so this refusal is the consumer's, given a type	refused at each entry point (bls.cpp:155, :341) rather than in the library (schemes.cpp:61)	aggregate_empty_fails, verify_aggregate_distinct_messages
  A weighted aggregate over a set holding one key twice depends on the argument order	-	std::sort on the key encodings (schemes.cpp:203)	secure_aggregate_duplicate_key_pairing
  A secret share derived at an id exposes the public share the verification vector gives for that id	The two are the same evaluation over different groups, which is the tie that makes the secret share checkable	secret at bls.cpp:100, public at :196, one call for both (threshold.cpp:270)	sk_share_matches_pk_share
  Share derivation refuses a verification vector shorter than two coefficients and an id that reduces to zero	Both contracts come from the shared evaluation, which reduces the id before Horner	threshold.cpp:166	derive_share_rejects_short_verification_vector, split_rejects_zero_reducing_ids, split_rejects_congruent_ids

Breaking Changes

See above.

How Has This Been Tested?

cargo fmt --check
cargo test --features full
cargo clippy --features full --all-targets
./contrib/lint_all.py --exclude lint_codeql
./contrib/lint/lint_codeql.py --with-suite rust-security-and-quality

Checklist

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated relevant unit/integration/functional tests
• I have made corresponding changes to the documentation (note: N/A)
• I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: da3ad7df-2d04-47f3-8337-ecd097f09bb2

📥 Commits

Reviewing files that changed from the base of the PR and between 8834b89 and 6045c99.

📒 Files selected for processing (6)

• contrib/codeql/lib/policy.qll
• pkgs/pkc/src/bls/dh_bytes.rs
• pkgs/pkc/src/bls/mod.rs
• pkgs/pkc/src/bls/public_ops.rs
• pkgs/pkc/src/bls/secret_ops.rs
• pkgs/pkc/src/bls/sig_basic.rs

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

• pkgs/pkc/src/bls/public_ops.rs

---

📝 Walkthrough

Walkthrough

The PR generalizes BLS aggregate operations across Chia and IETF schemes. It adds secure aggregation, secret-share derivation, cross-scheme conversion, and serialized DH output. It also expands subgroup, encoding, threshold, serialization, and benchmark coverage.

Changes

BLS operations

Layer / File(s)	Summary
Aggregate verification and secure aggregation pkgs/pkc/src/bls/error.rs, pkgs/pkc/src/bls/blst_ffi.rs, pkgs/pkc/src/bls/scheme_*.rs, pkgs/pkc/src/bls/scheme_ops.rs, pkgs/pkc/src/bls/sig_aggregate.rs	Aggregate verification is available through BlsScheme for both schemes. It validates counts, empty inputs, duplicate messages, and pairing results. Secure aggregation applies deterministic public-key weights.
Secret-share derivation and recovery pkgs/pkc/src/bls/scheme_ops.rs, pkgs/pkc/src/bls/share_ops.rs, pkgs/pkc/src/bls/sig_threshold.rs	Secret shares derive from master secret polynomials. Tests cover derived keys, signature shares, threshold recovery, hashing, equality, and serde.
Encoding, conversion, and shared-secret bytes pkgs/pkc/src/bls/dh_bytes.rs, pkgs/pkc/src/bls/mod.rs, pkgs/pkc/src/bls/public_ops.rs, pkgs/pkc/src/bls/secret_ops.rs, pkgs/pkc/src/bls/sig_basic.rs, pkgs/pkc/src/bls/tests.rs, contrib/codeql/lib/policy.qll	BLS keys and signatures support cross-scheme conversion. DH exchange returns BlsDhBytes. Tests cover subgroup policies, identity encodings, coordinates, compression bits, signature variants, round trips, and hexadecimal JSON output.
Generic benchmark coverage pkgs/pkc/bench/bls.rs, pkgs/pkc/src/bls/tests.rs	Benchmarks use full-width indices and cover distinct-message verification, secure aggregation, and share derivation for both schemes.

Sequence Diagram(s)

sequenceDiagram
  participant BlsSignature
  participant BlsScheme
  participant BlsScChia
  participant blst_pairing
  BlsSignature->>BlsScheme: verify_aggregates(sig, msgs, pks)
  BlsScheme->>BlsScChia: validate inputs and hash messages
  BlsScChia->>blst_pairing: pairings_equal_with_g1_generator_prod
  blst_pairing-->>BlsScChia: pairing result
  BlsScChia-->>BlsScheme: verification result

Loading

Mergeability Score: ⚪ Minimal · up to 6045c

The PR adds BLS aggregation and scheme-specific behavior coverage, and no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the PR's main BLS testing, weighted aggregation, Chia verification, and corpus changes.
Description check	✅ Passed	The description directly explains the BLS behavior changes, related issues, tests, breaking changes, and validation commands.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

---

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

[github-actions]

Note

This pull request has no conflicts! 🎊 🎉 🎊

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (1)

pkgs/pkc/src/bls/scheme_ops.rs (1)

402-410: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider a wider index encoding in secure_weights.

Line 405 encodes the index as u32. The index comes from sorted_pks.len(), which is a usize. The cast is lossless for any realistic signer count, and the encoding is part of the wire format that the corpus vectors pin, so do not change it now. Add a short note that the four-byte index is a fixed protocol constant, so a future reader does not "fix" the cast and silently change every weight.

Confirm that the reference implementation also uses a four-byte big-endian index, because a mismatch would only surface against external vectors.

🤖 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 `@pkgs/pkc/src/bls/scheme_ops.rs` around lines 402 - 410, Add a concise comment
beside the index encoding in secure_weights explaining that the u32 big-endian
representation is a fixed protocol constant and must not be widened, preserving
wire-format compatibility. Verify the reference implementation uses the same
four-byte big-endian index encoding, but do not alter the hashing logic.

🤖 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 `@pkgs/pkc/bench/bls.rs`:
- Around line 208-209: Correct the benchmark documentation above the relevant
benchmark in bls.rs to describe only the BlsSecretKey::derive_share operation
that is actually executed; remove claims about deriving a public share or
comparing public evaluations unless the benchmark is updated to perform those
operations.

In `@pkgs/pkc/src/bls/scheme_chia.rs`:
- Around line 266-281: Add rustdoc to the Chia implementation’s
verify_aggregates method documenting that repeated messages are accepted
intentionally for consensus compatibility and that DuplicateMessage is not
returned. Keep the verification behavior unchanged and make the policy explicit
at this implementation site.

In `@pkgs/pkc/src/bls/sig_aggregate.rs`:
- Around line 305-325: Add a non-empty assertion for the
secure_verify_aggregates vectors in assert_secure_aggregate_matches_vectors,
matching the guard used by assert_aggregate_verify_matches_vectors, before
iterating over vecs.

---

Nitpick comments:
In `@pkgs/pkc/src/bls/scheme_ops.rs`:
- Around line 402-410: Add a concise comment beside the index encoding in
secure_weights explaining that the u32 big-endian representation is a fixed
protocol constant and must not be widened, preserving wire-format compatibility.
Verify the reference implementation uses the same four-byte big-endian index
encoding, but do not alter the hashing logic.

🪄 Autofix

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: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: acde3527-75db-4542-bcd3-5912e1a64b86

📥 Commits

Reviewing files that changed from the base of the PR and between 0b0770d and 805bce8.

⛔ Files ignored due to path filters (2)

• pkgs/pkc/corpus/bls_chia_aggregate.json5 is excluded by !**/*.json5
• pkgs/pkc/corpus/bls_ietf_aggregate.json5 is excluded by !**/*.json5

📒 Files selected for processing (13)

• pkgs/pkc/bench/bls.rs
• pkgs/pkc/src/bls/blst_ffi.rs
• pkgs/pkc/src/bls/error.rs
• pkgs/pkc/src/bls/public_ops.rs
• pkgs/pkc/src/bls/scheme_chia.rs
• pkgs/pkc/src/bls/scheme_ietf.rs
• pkgs/pkc/src/bls/scheme_ops.rs
• pkgs/pkc/src/bls/secret_ops.rs
• pkgs/pkc/src/bls/share_ops.rs
• pkgs/pkc/src/bls/sig_aggregate.rs
• pkgs/pkc/src/bls/sig_basic.rs
• pkgs/pkc/src/bls/sig_threshold.rs
• pkgs/pkc/src/bls/tests.rs