feat: integrate proof of correct share decryption [skip-line-limit]#1362
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 DKG share‑decryption and batch proof verification: new ZK request/response variants, per‑party signed‑proof collection and propagation, multi‑stage verification in keyshare, C4 proof orchestration, DecryptionKeyShared event, and end‑to‑end publication of per‑party decryption‑key documents. Changes
Sequence DiagramsequenceDiagram
participant Collector as ThresholdShareCollector
participant TK as ThresholdKeyshare
participant ZK as ZK Prover
participant Prover as C4 Proof Generator
participant Pub as DocumentPublisher
Collector->>TK: AllThresholdSharesCollected(shares, proofs)
TK->>TK: Inspect per‑party proofs
alt proofs present
TK->>ZK: VerifyShareProofsRequest(party_proofs)
ZK->>ZK: Batch‑verify C2/C3 proofs
ZK->>TK: VerifyShareProofsResponse(results)
TK->>TK: Build honest_party_set
else no proofs
TK->>TK: Assume all parties honest
end
TK->>Prover: CalculateDecryptionKeyRequest(honest_shares)
Prover->>Prover: Generate DkgShareDecryption proofs (C4a/C4b)
Prover->>TK: DkgShareDecryptionProofResponse(c4a/c4b)
TK->>TK: Accumulate C4 proofs, form DecryptionKeyShared
TK->>Pub: Publish DecryptionKeyShared (per‑party document)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
ctrlc03
force-pushed
the
feat/c4-integration
branch
from
dba68a3 to
ee46730
Compare
ctrlc03
force-pushed
the
feat/c4-integration
branch
from
ee46730 to
f4b87ea
Compare
ctrlc03
changed the title
ctrlc03
force-pushed
the
feat/c4-integration
branch
from
f4b87ea to
f20c585
Compare
![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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/trbfv/src/shares/bfv_encrypted.rs (1)
142-160:⚠️ Potential issue | 🟡 MinorDocumentation block is misplaced after inserting new methods.
The doc comment at lines 142-150 describes the
decryptmethod, but it now precedes the newly addedciphertext_bytes()andnum_moduli()accessors. The doc comment fordecryptshould be moved directly above thedecryptmethod at line 161.📝 Proposed fix
- /// Decrypt an encrypted share using the recipient's secret key. - /// - /// # Arguments - /// * `sk` - The recipient's BFV secret key - /// * `params` - BFV parameters for share encryption - /// * `degree` - Polynomial degree (for reconstructing the share matrix) - /// - /// # Returns - /// The decrypted Shamir share /// Get the raw ciphertext bytes (one per CRT modulus). pub fn ciphertext_bytes(&self) -> &[ArcBytes] { &self.ciphertexts } /// Number of CRT moduli (ciphertexts per share). pub fn num_moduli(&self) -> usize { self.ciphertexts.len() } + /// Decrypt an encrypted share using the recipient's secret key. + /// + /// # Arguments + /// * `sk` - The recipient's BFV secret key + /// * `params` - BFV parameters for share encryption + /// * `degree` - Polynomial degree (for reconstructing the share matrix) + /// + /// # Returns + /// The decrypted Shamir share pub fn decrypt(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/trbfv/src/shares/bfv_encrypted.rs` around lines 142 - 160, The doc comment describing decryption is misplaced above the new accessors; move that documentation block so it sits immediately above the decrypt method (the function named decrypt) instead of above ciphertext_bytes() and num_moduli(); ensure ciphertext_bytes() and num_moduli() keep their own short comments (or none) and that decrypt retains the full docblock describing sk, params, degree and return value.
🧹 Nitpick comments (6)
crates/zk-prover/tests/onchain_verification_tests.rs (1)
175-190: Verify public signals byte length assumption.The chunking logic pads partial chunks with trailing zeros. This works correctly when
proof.public_signalsis exactly 32 bytes, but if it's shorter, the padding direction (right-padding) may be incorrect for big-endian encoded values. The assertion on line 185 catches mismatches at runtime, but consider adding a debug assertion on the raw byte length to catch encoding issues earlier:debug_assert_eq!( proof.public_signals.len(), 32, "expected pk commitment to be exactly 32 bytes" );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/zk-prover/tests/onchain_verification_tests.rs` around lines 175 - 190, Add a debug assertion to validate the raw byte length of proof.public_signals before chunking: check that proof.public_signals.len() == 32 (or the expected byte-length for the pk commitment) to fail fast on incorrect encoding/lengths; place this check just before the code that builds public_inputs (i.e., right above the chunking/map that constructs FixedBytes<32>), referencing the proof.public_signals variable and the public_inputs construction so the assertion triggers earlier than the assert_eq! on public_inputs.len().crates/events/src/enclave_event/decryption_key_shared.rs (1)
16-36: LGTM with minor suggestion: Consider consistent Debug formatting fores_poly_sum.The struct is well-designed. Note that
sk_poly_sumuseshexfformatter via derivative, butes_poly_sum(alsoVec<ArcBytes>) uses default Debug formatting. For consistency, you may want to apply similar formatting toes_poly_sum.✨ Optional: consistent formatting
/// Lagrange-interpolated aggregated SK polynomial (serialized). #[derivative(Debug(format_with = "e3_utils::formatters::hexf"))] pub sk_poly_sum: ArcBytes, /// Lagrange-interpolated aggregated E_SM polynomials (serialized), one per smudging noise. + #[derivative(Debug = "ignore")] // or a custom formatter for Vec<ArcBytes> pub es_poly_sum: Vec<ArcBytes>,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/events/src/enclave_event/decryption_key_shared.rs` around lines 16 - 36, The Debug formatting for DecryptionKeyShared is inconsistent: sk_poly_sum uses #[derivative(Debug(format_with = "e3_utils::formatters::hexf"))] but es_poly_sum does not; update the DecryptionKeyShared struct so es_poly_sum (Vec<ArcBytes>) also uses #[derivative(Debug(format_with = "e3_utils::formatters::hexf"))] to match sk_poly_sum formatting, keeping the same attribute style and imports.crates/zk-prover/src/actors/proof_request.rs (1)
446-461: Underscore-prefixed variables are actually used — misleading naming.Variables like
_recipient,_row, and_esiuse the underscore prefix convention that typically indicates unused variables, but they are actually used as map keys. Consider removing the underscores for clarity.✏️ Suggested rename
- for ((_recipient, _row), proof) in &pending.sk_share_encryption_proofs { + for ((recipient, _row), proof) in &pending.sk_share_encryption_proofs { if let Some(signed) = self.sign_proof(e3_id, ProofType::C3aSkShareEncryption, proof.clone()) { - signed_c3a_map.entry(*_recipient).or_default().push(signed); + signed_c3a_map.entry(*recipient).or_default().push(signed); } } // Sign C3b proofs (ESmShareEncryption) — keyed by (esi_index, recipient, row) let mut signed_c3b_map: HashMap<usize, Vec<SignedProofPayload>> = HashMap::new(); - for ((_esi, _recipient, _row), proof) in &pending.e_sm_share_encryption_proofs { + for ((_esi, recipient, _row), proof) in &pending.e_sm_share_encryption_proofs { if let Some(signed) = self.sign_proof(e3_id, ProofType::C3bESmShareEncryption, proof.clone()) { - signed_c3b_map.entry(*_recipient).or_default().push(signed); + signed_c3b_map.entry(*recipient).or_default().push(signed); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/zk-prover/src/actors/proof_request.rs` around lines 446 - 461, The loop variables in the blocks iterating pending.sk_share_encryption_proofs and pending.e_sm_share_encryption_proofs are misleadingly named with leading underscores even though they are used as keys; rename those bindings (e.g., change _recipient -> recipient, _row -> row, and _esi -> esi) and update all occurrences in the same loop bodies (including where you dereference them for signed_c3a_map.entry(*recipient) and signed_c3b_map.entry(*recipient)) so the code clearly expresses that these values are used (keep the same semantics with the existing sign_proof calls and ProofType::C3aSkShareEncryption / ProofType::C3bESmShareEncryption).crates/keyshare/src/threshold_keyshare.rs (3)
1341-1377: Underscore-prefixed variable_ecis used — consider renaming.On line 1343,
_ecis destructured with an underscore prefix (typically indicating an unused variable), but it's actually used on line 1373. While valid Rust, this could confuse readers.♻️ Suggested rename for clarity
- let (msg, _ec) = msg.into_components(); + let (msg, ec) = msg.into_components(); ... - self.try_publish_decryption_key_shared(_ec)?; + self.try_publish_decryption_key_shared(ec)?;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/keyshare/src/threshold_keyshare.rs` around lines 1341 - 1377, The variable named `_ec` in handle_c4_proof_response is destructured with a leading underscore (implying it is unused) but is actually passed into try_publish_decryption_key_shared, which is confusing; rename `_ec` to a non-underscored name like `ec` in the destructuring and update its usage in the call to try_publish_decryption_key_shared so the intent is clear and the symbol matches expected usage.
1632-1640: TODO: Future verification of C4 proofs on received DecryptionKeyShared.The handler correctly filters for external messages and logs receipt. The TODO on line 1638 indicates C4 proof verification for received messages is pending implementation.
Would you like me to open an issue to track the implementation of C4 proof verification for received
DecryptionKeySharedmessages?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/keyshare/src/threshold_keyshare.rs` around lines 1632 - 1640, The handler for EnclaveEventData::DecryptionKeyShared currently logs external messages but does not verify C4 proofs; implement verification by adding a proof-check step inside the EnclaveEventData::DecryptionKeyShared match arm when data.external is true: call the existing C4 proof verification routine (or add one named verify_c4_proof / verify_c4_proof_for_decryption_key) using data's proof fields, reject or log and drop messages that fail verification, and only store the key material for threshold decryption after successful verification; update any storage function (e.g., where you persist shared keys) to require the verification result.
1148-1156: Consider emitting an E3Failed event for insufficient honest parties.When the honest party count drops below threshold (line 1148-1155), the code logs a warning and silently returns. The TODO comment indicates intent to emit an
E3Failedevent. Without this, upstream components may not know the E3 has failed.Would you like me to draft the
E3Failedevent emission for this error path?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/keyshare/src/threshold_keyshare.rs` around lines 1148 - 1156, When honest_count < threshold in threshold_keyshare.rs (the block that currently logs a warning and sets self.pending_verification_shares = None then returns), emit the E3Failed event before clearing state and returning so upstream components are notified; construct the event with the failing e3_id and a reason/metadata (e.g., HonestCountBelowThreshold including honest_count and threshold) and call the same event-emission pathway used elsewhere in this module (the code path that normally emits events for E3 outcomes), then clear self.pending_verification_shares and return Ok(()) as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/keyshare/src/threshold_keyshare.rs`:
- Around line 1303-1315: The C4 proof requests are passing self.share_enc_preset
(a DKG preset) but the handlers/circuit expect a threshold preset; fix by
converting self.share_enc_preset to its threshold counterpart before building
the requests: call self.share_enc_preset.threshold_counterpart().ok_or_else(||
anyhow!("No threshold counterpart for {:?}", self.share_enc_preset))? to obtain
threshold_preset, then pass threshold_preset into the
DkgShareDecryptionProofRequest (used in ComputeRequest::zk /
ZkRequest::DkgShareDecryption) for both the C4a and the C4b requests (mirror the
conversion used at the other sites like where build_pair_for_preset is called).
In `@crates/multithread/src/multithread.rs`:
- Around line 721-802: In handle_dkg_share_decryption_proof, convert
req.params_preset to its threshold counterpart before calling circuit.prove:
call req.params_preset.threshold_counterpart().ok_or_else(|| /* return
appropriate ComputeRequestError via make_zk_error or same pattern as
threshold_keyshare.rs */) to get a threshold_preset and pass that
threshold_preset (not req.params_preset) into circuit.prove; mirror the error
construction used in threshold_keyshare.rs so a missing threshold mapping
produces the same ComputeRequestError flow.
---
Outside diff comments:
In `@crates/trbfv/src/shares/bfv_encrypted.rs`:
- Around line 142-160: The doc comment describing decryption is misplaced above
the new accessors; move that documentation block so it sits immediately above
the decrypt method (the function named decrypt) instead of above
ciphertext_bytes() and num_moduli(); ensure ciphertext_bytes() and num_moduli()
keep their own short comments (or none) and that decrypt retains the full
docblock describing sk, params, degree and return value.
---
Nitpick comments:
In `@crates/events/src/enclave_event/decryption_key_shared.rs`:
- Around line 16-36: The Debug formatting for DecryptionKeyShared is
inconsistent: sk_poly_sum uses #[derivative(Debug(format_with =
"e3_utils::formatters::hexf"))] but es_poly_sum does not; update the
DecryptionKeyShared struct so es_poly_sum (Vec<ArcBytes>) also uses
#[derivative(Debug(format_with = "e3_utils::formatters::hexf"))] to match
sk_poly_sum formatting, keeping the same attribute style and imports.
In `@crates/keyshare/src/threshold_keyshare.rs`:
- Around line 1341-1377: The variable named `_ec` in handle_c4_proof_response is
destructured with a leading underscore (implying it is unused) but is actually
passed into try_publish_decryption_key_shared, which is confusing; rename `_ec`
to a non-underscored name like `ec` in the destructuring and update its usage in
the call to try_publish_decryption_key_shared so the intent is clear and the
symbol matches expected usage.
- Around line 1632-1640: The handler for EnclaveEventData::DecryptionKeyShared
currently logs external messages but does not verify C4 proofs; implement
verification by adding a proof-check step inside the
EnclaveEventData::DecryptionKeyShared match arm when data.external is true: call
the existing C4 proof verification routine (or add one named verify_c4_proof /
verify_c4_proof_for_decryption_key) using data's proof fields, reject or log and
drop messages that fail verification, and only store the key material for
threshold decryption after successful verification; update any storage function
(e.g., where you persist shared keys) to require the verification result.
- Around line 1148-1156: When honest_count < threshold in threshold_keyshare.rs
(the block that currently logs a warning and sets
self.pending_verification_shares = None then returns), emit the E3Failed event
before clearing state and returning so upstream components are notified;
construct the event with the failing e3_id and a reason/metadata (e.g.,
HonestCountBelowThreshold including honest_count and threshold) and call the
same event-emission pathway used elsewhere in this module (the code path that
normally emits events for E3 outcomes), then clear
self.pending_verification_shares and return Ok(()) as before.
In `@crates/zk-prover/src/actors/proof_request.rs`:
- Around line 446-461: The loop variables in the blocks iterating
pending.sk_share_encryption_proofs and pending.e_sm_share_encryption_proofs are
misleadingly named with leading underscores even though they are used as keys;
rename those bindings (e.g., change _recipient -> recipient, _row -> row, and
_esi -> esi) and update all occurrences in the same loop bodies (including where
you dereference them for signed_c3a_map.entry(*recipient) and
signed_c3b_map.entry(*recipient)) so the code clearly expresses that these
values are used (keep the same semantics with the existing sign_proof calls and
ProofType::C3aSkShareEncryption / ProofType::C3bESmShareEncryption).
In `@crates/zk-prover/tests/onchain_verification_tests.rs`:
- Around line 175-190: Add a debug assertion to validate the raw byte length of
proof.public_signals before chunking: check that proof.public_signals.len() ==
32 (or the expected byte-length for the pk commitment) to fail fast on incorrect
encoding/lengths; place this check just before the code that builds
public_inputs (i.e., right above the chunking/map that constructs
FixedBytes<32>), referencing the proof.public_signals variable and the
public_inputs construction so the assertion triggers earlier than the assert_eq!
on public_inputs.len().
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
crates/events/src/enclave_event/compute_request/mod.rscrates/events/src/enclave_event/compute_request/zk.rscrates/events/src/enclave_event/decryption_key_shared.rscrates/events/src/enclave_event/mod.rscrates/events/src/enclave_event/threshold_share_created.rscrates/keyshare/src/threshold_keyshare.rscrates/keyshare/src/threshold_share_collector.rscrates/multithread/src/multithread.rscrates/net/src/document_publisher.rscrates/trbfv/src/shares/bfv_encrypted.rscrates/zk-prover/src/actors/proof_request.rscrates/zk-prover/tests/onchain_verification_tests.rstemplates/default/tests/integration.spec.ts
ctrlc03
force-pushed
the
feat/c4-integration
branch
from
f20c585 to
fbdb43c
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
crates/multithread/src/multithread.rs (1)
785-786:⚠️ Potential issue | 🔴 CriticalUse a threshold preset for
ShareDecryptionCircuit::prove(Line 785).
proveis currently called withreq.params_presetdirectly. If that value is a DKG preset, this path can fail at runtime in the share-decryption circuit stack.🔧 Proposed fix
- let proof = circuit - .prove(prover, &req.params_preset, &circuit_data, &e3_id_str) + let proof_preset = req + .params_preset + .threshold_counterpart() + .unwrap_or(req.params_preset); + let proof = circuit + .prove(prover, &proof_preset, &circuit_data, &e3_id_str) .map_err(|e| { ComputeRequestError::new( ComputeRequestErrorKind::Zk(ZkEventError::ProofGenerationFailed(e.to_string())), request.clone(), ) })?;#!/bin/bash set -euo pipefail # 1) Confirm current prove call uses req.params_preset directly rg -nC3 'handle_dkg_share_decryption_proof|\.prove\(prover, &req\.params_preset' crates/multithread/src/multithread.rs # 2) Confirm circuit codegen expectation (threshold preset -> dkg counterpart unwrap path) rg -nC5 'dkg_counterpart\(\)\.unwrap|share_decryption' crates/zk-helpers/src/circuits/dkg/share_decryption/codegen.rs # 3) Confirm request construction preset source in keyshare flow rg -nC4 'DkgShareDecryptionProofRequest|params_preset|share_enc_preset' crates/keyshare/src/threshold_keyshare.rsBased on learnings: The share-decryption circuit codegen accepts only threshold BFV presets and intentionally unwraps
preset.dkg_counterpart().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/multithread/src/multithread.rs` around lines 785 - 786, The prove call for ShareDecryptionCircuit should receive a threshold (non-DKG) BFV preset instead of possibly a DKG preset; update the call to pass the threshold counterpart of req.params_preset (e.g. use req.params_preset.dkg_counterpart().unwrap() or the project’s helper that returns the threshold preset) to ShareDecryptionCircuit::prove so the circuit codegen that unwraps dkg_counterpart() will not panic; modify the argument at the .prove(prover, &req.params_preset, &circuit_data, &e3_id_str) site accordingly.crates/keyshare/src/threshold_keyshare.rs (1)
1303-1311:⚠️ Potential issue | 🔴 CriticalUse threshold counterpart preset for C4 proof requests.
At Line 1310 and Line 1330,
params_presetis set toself.share_enc_preset, but this C4 path expects threshold BFV presets. Passing a DKG preset can break proof generation/verification compatibility.🧩 Proposed fix
+ let threshold_preset = self + .share_enc_preset + .threshold_counterpart() + .ok_or_else(|| anyhow!("No threshold counterpart for {:?}", self.share_enc_preset))?; + let c4a_request = ComputeRequest::zk( ZkRequest::DkgShareDecryption(DkgShareDecryptionProofRequest { sk_bfv: current.sk_bfv.clone(), honest_ciphertexts_raw: sk_ciphertexts_raw, num_honest_parties: num_honest, num_moduli: num_moduli_sk, dkg_input_type: DkgInputType::SecretKey, - params_preset: self.share_enc_preset.clone(), + params_preset: threshold_preset, }), CorrelationId::new(), e3_id.clone(), ); @@ let c4b_request = ComputeRequest::zk( ZkRequest::DkgShareDecryption(DkgShareDecryptionProofRequest { sk_bfv: current.sk_bfv.clone(), honest_ciphertexts_raw: esi_cts, num_honest_parties: num_honest, num_moduli: num_moduli_esi, dkg_input_type: DkgInputType::SmudgingNoise, - params_preset: self.share_enc_preset.clone(), + params_preset: threshold_preset, }), CorrelationId::new(), e3_id.clone(), );#!/bin/bash set -euo pipefail echo "1) Confirm current C4 request preset usage:" rg -n -C2 'DkgShareDecryptionProofRequest|params_preset:\s*self\.share_enc_preset' crates/keyshare/src/threshold_keyshare.rs echo echo "2) Confirm preset expectations in helper/codegen paths:" rg -n -C2 'build_pair_for_preset|threshold_counterpart|dkg_counterpart|MissingPair' crates/fhe-params/src crates/zk-helpers/src echo echo "3) Inspect share_decryption codegen precondition directly:" sed -n '54,72p' crates/zk-helpers/src/circuits/dkg/share_decryption/codegen.rsBased on learnings: the share-decryption codegen is intentionally designed to accept threshold BFV presets, not DKG presets.
Also applies to: 1324-1331
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/keyshare/src/threshold_keyshare.rs` around lines 1303 - 1311, The DkgShareDecryptionProofRequest is being passed a DKG preset (params_preset: self.share_enc_preset) but the C4/threshold share-decryption path requires a threshold BFV preset; replace params_preset:self.share_enc_preset with the corresponding threshold preset (e.g. params_preset: self.threshold_enc_preset) or compute it with the helper that returns the threshold counterpart (e.g. threshold_counterpart(self.share_enc_preset) or build_pair_for_preset(self.share_enc_preset).threshold) so DkgShareDecryptionProofRequest and other C4 requests use the threshold BFV preset instead of the DKG preset.
🧹 Nitpick comments (1)
crates/zk-prover/src/actors/proof_request.rs (1)
444-543: Sign C3 proofs once and reuse those signed payloads.Current flow signs C3a/C3b proofs twice (map-building and event publishing). Consolidating to a single signing pass will simplify control flow and avoid drift.
♻️ Refactor sketch
- // Sign C3a proofs ... build map - // ... - // Sign and publish C3a proofs ... sign again + // Sign once, then: + // 1) store by recipient for ThresholdShareCreated + // 2) publish DkgProofSigned using the same SignedProofPayload🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/zk-prover/src/actors/proof_request.rs` around lines 444 - 543, The code currently calls self.sign_proof twice for C3a/C3b (once to populate signed_c3a_map/signed_c3b_map and again in the publishing loops); change to sign each proof only once by: iterate pending.sk_share_encryption_proofs and pending.e_sm_share_encryption_proofs once, store the Some(signed) results in signed_c3a_map and signed_c3b_map (log and return on None), then in the publishing phase iterate those maps to publish DkgProofSigned events (using the stored SignedProofPayloads) instead of calling self.sign_proof again; update error messages to reference signing failures from sign_proof and publishing failures when calling bus.publish.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/events/src/enclave_event/threshold_share_created.rs`:
- Around line 67-74: The new proof fields on ThresholdShareCreated must be
annotated with serde defaults for backward-compatible deserialization: add
#[serde(default)] to the signed_c2a_proof and signed_c2b_proof
Option<SignedProofPayload> fields and to the signed_c3a_proofs and
signed_c3b_proofs Vec<SignedProofPayload> fields in the ThresholdShareCreated
struct so old payloads missing these fields deserialize safely.
In `@crates/keyshare/src/threshold_keyshare.rs`:
- Around line 1069-1074: The code currently omits senders with empty
signed_proofs from verification (signed_proofs.is_empty()) but later still
includes their shares in honest_shares; change the logic so that shares with no
proofs are treated as dishonest: when signed_proofs.is_empty() push a
PartyProofsToVerify entry for share.party_id (so verification can fail/record
it) or otherwise mark/record that share.party_id is dishonest, and ensure you do
NOT add that share to honest_shares later (remove it from the honest_shares
path). Update the blocks that build party_proofs_to_verify and the honest_shares
assignment (referencing signed_proofs, party_proofs_to_verify,
PartyProofsToVerify, honest_shares, and share.party_id) so missing proofs cannot
silently count toward decryption-key aggregation.
In `@crates/multithread/src/multithread.rs`:
- Around line 816-819: The verification flow currently calls
prover.verify(proof, &e3_id_str, sender) for each signed_proof but never
validates the SignedProofPayload signature; update the loop over
party.signed_proofs to first call signed_proof.payload.verify_address() (or the
appropriate method on SignedProofPayload) and capture the recovered
signer/address, then ensure that recovered signer matches the expected sender
(or the derived sender_party_id) before calling prover.verify; if
verify_address() fails or the recovered signer does not match the claimed
party_id, return an error or skip/fail verification immediately so proofs cannot
be misattributed.
---
Duplicate comments:
In `@crates/keyshare/src/threshold_keyshare.rs`:
- Around line 1303-1311: The DkgShareDecryptionProofRequest is being passed a
DKG preset (params_preset: self.share_enc_preset) but the C4/threshold
share-decryption path requires a threshold BFV preset; replace
params_preset:self.share_enc_preset with the corresponding threshold preset
(e.g. params_preset: self.threshold_enc_preset) or compute it with the helper
that returns the threshold counterpart (e.g.
threshold_counterpart(self.share_enc_preset) or
build_pair_for_preset(self.share_enc_preset).threshold) so
DkgShareDecryptionProofRequest and other C4 requests use the threshold BFV
preset instead of the DKG preset.
In `@crates/multithread/src/multithread.rs`:
- Around line 785-786: The prove call for ShareDecryptionCircuit should receive
a threshold (non-DKG) BFV preset instead of possibly a DKG preset; update the
call to pass the threshold counterpart of req.params_preset (e.g. use
req.params_preset.dkg_counterpart().unwrap() or the project’s helper that
returns the threshold preset) to ShareDecryptionCircuit::prove so the circuit
codegen that unwraps dkg_counterpart() will not panic; modify the argument at
the .prove(prover, &req.params_preset, &circuit_data, &e3_id_str) site
accordingly.
---
Nitpick comments:
In `@crates/zk-prover/src/actors/proof_request.rs`:
- Around line 444-543: The code currently calls self.sign_proof twice for
C3a/C3b (once to populate signed_c3a_map/signed_c3b_map and again in the
publishing loops); change to sign each proof only once by: iterate
pending.sk_share_encryption_proofs and pending.e_sm_share_encryption_proofs
once, store the Some(signed) results in signed_c3a_map and signed_c3b_map (log
and return on None), then in the publishing phase iterate those maps to publish
DkgProofSigned events (using the stored SignedProofPayloads) instead of calling
self.sign_proof again; update error messages to reference signing failures from
sign_proof and publishing failures when calling bus.publish.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
crates/events/src/enclave_event/compute_request/mod.rscrates/events/src/enclave_event/compute_request/zk.rscrates/events/src/enclave_event/decryption_key_shared.rscrates/events/src/enclave_event/mod.rscrates/events/src/enclave_event/threshold_share_created.rscrates/keyshare/src/threshold_keyshare.rscrates/keyshare/src/threshold_share_collector.rscrates/multithread/src/multithread.rscrates/net/src/document_publisher.rscrates/trbfv/src/shares/bfv_encrypted.rscrates/zk-prover/src/actors/proof_request.rscrates/zk-prover/tests/onchain_verification_tests.rstemplates/default/tests/integration.spec.ts
✅ Files skipped from review due to trivial changes (1)
- crates/zk-prover/tests/onchain_verification_tests.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- templates/default/tests/integration.spec.ts
- crates/trbfv/src/shares/bfv_encrypted.rs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
crates/keyshare/src/threshold_keyshare.rs (1)
1338-1365:⚠️ Potential issue | 🔴 CriticalUse threshold preset for C4 requests, not
self.share_enc_preset.Line 1344 and Line 1364 still pass
self.share_enc_presetintoDkgShareDecryptionProofRequest. This should use the threshold counterpart, otherwise the C4 path can fail for DKG presets.🛠️ Proposed fix
+ let threshold_preset = self + .share_enc_preset + .threshold_counterpart() + .ok_or_else(|| anyhow!("No threshold counterpart for {:?}", self.share_enc_preset))?; + let c4a_request = ComputeRequest::zk( ZkRequest::DkgShareDecryption(DkgShareDecryptionProofRequest { sk_bfv: current.sk_bfv.clone(), honest_ciphertexts_raw: sk_ciphertexts_raw, num_honest_parties: num_honest, num_moduli: num_moduli_sk, dkg_input_type: DkgInputType::SecretKey, - params_preset: self.share_enc_preset.clone(), + params_preset: threshold_preset, }), CorrelationId::new(), e3_id.clone(), ); @@ let c4b_request = ComputeRequest::zk( ZkRequest::DkgShareDecryption(DkgShareDecryptionProofRequest { sk_bfv: current.sk_bfv.clone(), honest_ciphertexts_raw: esi_cts, num_honest_parties: num_honest, num_moduli: num_moduli_esi, dkg_input_type: DkgInputType::SmudgingNoise, - params_preset: self.share_enc_preset.clone(), + params_preset: threshold_preset, }), CorrelationId::new(), e3_id.clone(), );#!/bin/bash set -euo pipefail # Verify C4 request preset usage in threshold_keyshare rg -n 'DkgShareDecryptionProofRequest|params_preset' crates/keyshare/src/threshold_keyshare.rs -A4 -B6 # Verify share-decryption circuit precondition around preset handling sed -n '54,72p' crates/zk-helpers/src/circuits/dkg/share_decryption/codegen.rsBased on learnings: the share-decryption circuit codegen is designed to accept only threshold BFV presets, not DKG presets.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/keyshare/src/threshold_keyshare.rs` around lines 1338 - 1365, The C4 DkgShareDecryptionProofRequest calls are using self.share_enc_preset (lines with DkgShareDecryptionProofRequest and params_preset), but the share-decryption circuit requires the threshold BFV preset; replace the preset argument in both C4a and C4b requests to use the threshold preset field (e.g., self.threshold_share_enc_preset) instead of self.share_enc_preset so DkgShareDecryptionProofRequest.params_preset supplies the threshold preset expected by the circuit.
🧹 Nitpick comments (1)
crates/keyshare/src/threshold_keyshare.rs (1)
1666-1673: Please track the externalDecryptionKeySharedverification TODO.Line 1672 leaves a critical follow-up in-flow. If you want, I can draft a concrete issue checklist (verification inputs, storage shape, and failure policy) to unblock implementation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/keyshare/src/threshold_keyshare.rs` around lines 1666 - 1673, The match arm handling EnclaveEventData::DecryptionKeyShared currently leaves a TODO; implement verification of the C4 proofs and durable storage for threshold decryption: when data.external is true (inside the EnclaveEventData::DecryptionKeyShared branch referencing data.party_id and data.e3_id), call the appropriate C4 proof verification routine (e.g., the crate's verify_c4_proof / verify_proof function or verifier module) using the proof fields in data, reject or mark the share invalid and log an error if verification fails, and on success persist the verified decryption share into the threshold decryption store (e.g., the struct/map used for decryption keys or shares such as threshold_store/decryption_keys) with keys indexed by e3_id and party_id; ensure the storage shape matches the threshold decryption consumer expectations and surface deterministic error handling (logged + observable state entry for invalid shares) so downstream threshold decryption can proceed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/keyshare/src/threshold_keyshare.rs`:
- Around line 1352-1370: The C4b proofs are being collected in arrival order
which can misalign with es_poly_sum indices; modify the collection logic so that
when sending ComputeRequest::zk ZkRequest::DkgShareDecryption (the loop over
esi_ciphertexts_raw creating c4b_request and calling self.bus.publish) you
attach or record a deterministic key (use the loop's esi_idx or store a
correlation-id→esi_idx map) and push each returned proof into a map keyed by
that deterministic key (not an append-only Vec); then, before publishing
c4b_proofs (the code that currently iterates over c4b_proofs near where
es_poly_sum is used), iterate the keys in index order and assemble the final
vector in esi_idx order so publishing aligns with es_poly_sum semantics. Ensure
references to current.sk_bfv, DkgInputType::SmudgingNoise, and params_preset are
unchanged while only changing how responses are stored and ordered.
---
Duplicate comments:
In `@crates/keyshare/src/threshold_keyshare.rs`:
- Around line 1338-1365: The C4 DkgShareDecryptionProofRequest calls are using
self.share_enc_preset (lines with DkgShareDecryptionProofRequest and
params_preset), but the share-decryption circuit requires the threshold BFV
preset; replace the preset argument in both C4a and C4b requests to use the
threshold preset field (e.g., self.threshold_share_enc_preset) instead of
self.share_enc_preset so DkgShareDecryptionProofRequest.params_preset supplies
the threshold preset expected by the circuit.
---
Nitpick comments:
In `@crates/keyshare/src/threshold_keyshare.rs`:
- Around line 1666-1673: The match arm handling
EnclaveEventData::DecryptionKeyShared currently leaves a TODO; implement
verification of the C4 proofs and durable storage for threshold decryption: when
data.external is true (inside the EnclaveEventData::DecryptionKeyShared branch
referencing data.party_id and data.e3_id), call the appropriate C4 proof
verification routine (e.g., the crate's verify_c4_proof / verify_proof function
or verifier module) using the proof fields in data, reject or mark the share
invalid and log an error if verification fails, and on success persist the
verified decryption share into the threshold decryption store (e.g., the
struct/map used for decryption keys or shares such as
threshold_store/decryption_keys) with keys indexed by e3_id and party_id; ensure
the storage shape matches the threshold decryption consumer expectations and
surface deterministic error handling (logged + observable state entry for
invalid shares) so downstream threshold decryption can proceed.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
crates/events/src/enclave_event/threshold_share_created.rscrates/keyshare/src/threshold_keyshare.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/events/src/enclave_event/threshold_share_created.rs
ctrlc03
force-pushed
the
feat/c4-integration
branch
from
f1381a7 to
cf75b8f
Compare
hmzakhalid
reviewed
Feb 27, 2026
hmzakhalid
left a comment
hmzakhalid
left a comment
Collaborator
There was a problem hiding this comment.
Thanks Ale, left a few comments
Also, for the verification of ECDSA signatures of the C1-C4 proofs, I can do them in the fault attribution PR. Otherwise ill have to refactor a lot of it
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
crates/keyshare/src/threshold_keyshare.rs (1)
1411-1412:⚠️ Potential issue | 🔴 CriticalUse threshold preset for C4 requests, not
self.share_enc_preset.At Line 1411 and Line 1433,
DkgShareDecryptionProofRequest.params_presetis stillself.share_enc_preset. This can break C4 proof generation ifself.share_enc_presetis a DKG preset.🔧 Proposed fix
+ let threshold_preset = self + .share_enc_preset + .threshold_counterpart() + .ok_or_else(|| anyhow!("No threshold counterpart for {:?}", self.share_enc_preset))?; + let c4a_request = ComputeRequest::zk( ZkRequest::DkgShareDecryption(DkgShareDecryptionProofRequest { sk_bfv: current.sk_bfv.clone(), honest_ciphertexts_raw: sk_ciphertexts_raw, num_honest_parties: num_honest, num_moduli: num_moduli_sk, dkg_input_type: DkgInputType::SecretKey, - params_preset: self.share_enc_preset.clone(), + params_preset: threshold_preset, }), CorrelationId::new(), e3_id.clone(), ); @@ let c4b_request = ComputeRequest::zk( ZkRequest::DkgShareDecryption(DkgShareDecryptionProofRequest { sk_bfv: current.sk_bfv.clone(), honest_ciphertexts_raw: esi_cts, num_honest_parties: num_honest, num_moduli: num_moduli_esi, dkg_input_type: DkgInputType::SmudgingNoise, - params_preset: self.share_enc_preset.clone(), + params_preset: threshold_preset, }), correlation_id, e3_id.clone(), );#!/bin/bash set -euo pipefail echo "Inspect C4 request presets in threshold_keyshare:" sed -n '1398,1440p' crates/keyshare/src/threshold_keyshare.rs echo echo "Inspect share_decryption codegen preset expectation:" sed -n '54,72p' crates/zk-helpers/src/circuits/dkg/share_decryption/codegen.rs echo echo "Inspect DkgShareDecryption handling path:" sed -n '724,740p' crates/multithread/src/multithread.rsBased on learnings, the share-decryption circuit codegen is designed to accept only threshold BFV presets (crates/zk-helpers/src/circuits/dkg/share_decryption/codegen.rs:54-72).
Also applies to: 1433-1434
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/keyshare/src/threshold_keyshare.rs` around lines 1411 - 1412, DkgShareDecryptionProofRequest currently sets params_preset to self.share_enc_preset but the share-decryption C4/codegen only accepts a threshold BFV preset; replace the use of self.share_enc_preset with the node's threshold BFV preset (the same threshold preset used for C4/share-decryption elsewhere, e.g., self.threshold_bfv_preset or the equivalent field) in both DkgShareDecryptionProofRequest constructions so DkgShareDecryptionProofRequest.params_preset passes the proper threshold preset to the share_decryption circuit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/keyshare/src/threshold_keyshare.rs`:
- Around line 1213-1226: The loop over resp.party_results only marks parties
explicitly present with all_verified == false as dishonest, but parties missing
from resp.party_results are currently treated as honest; fix by collecting the
set of seen sender IDs from resp.party_results (using result.sender_party_id)
and then iterate the expected senders for this VerifyShareProofs request (the
original list of party IDs we queried) and insert any expected sender not in the
seen set into dishonest_parties; keep the existing handling for present results
(check result.all_verified and insert when false) and ensure you reference
resp.party_results, result.sender_party_id, and dishonest_parties when
implementing this.
---
Duplicate comments:
In `@crates/keyshare/src/threshold_keyshare.rs`:
- Around line 1411-1412: DkgShareDecryptionProofRequest currently sets
params_preset to self.share_enc_preset but the share-decryption C4/codegen only
accepts a threshold BFV preset; replace the use of self.share_enc_preset with
the node's threshold BFV preset (the same threshold preset used for
C4/share-decryption elsewhere, e.g., self.threshold_bfv_preset or the equivalent
field) in both DkgShareDecryptionProofRequest constructions so
DkgShareDecryptionProofRequest.params_preset passes the proper threshold preset
to the share_decryption circuit.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
crates/events/src/enclave_event/threshold_share_created.rscrates/keyshare/src/threshold_keyshare.rscrates/zk-prover/src/actors/proof_request.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/events/src/enclave_event/threshold_share_created.rs
ctrlc03
force-pushed
the
feat/c4-integration
branch
from
b77c0e7 to
d7d2955
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
crates/keyshare/src/threshold_keyshare.rs (1)
1432-1462:⚠️ Potential issue | 🔴 CriticalUse threshold preset for C4 proof requests (current preset can break share-decryption proof generation).
Line 1439 and Line 1461 pass
self.share_enc_presetdirectly intoDkgShareDecryptionProofRequest. This flow should use the threshold counterpart preset.Based on learnings, the share-decryption circuit codegen is designed to accept only threshold BFV presets and intentionally panics for DKG presets.🔧 Proposed fix
+ let threshold_preset = self + .share_enc_preset + .threshold_counterpart() + .ok_or_else(|| anyhow!("No threshold counterpart for {:?}", self.share_enc_preset))?; + let c4a_request = ComputeRequest::zk( ZkRequest::DkgShareDecryption(DkgShareDecryptionProofRequest { sk_bfv: current.sk_bfv.clone(), honest_ciphertexts_raw: sk_ciphertexts_raw, num_honest_parties: num_honest, num_moduli: num_moduli_sk, dkg_input_type: DkgInputType::SecretKey, - params_preset: self.share_enc_preset.clone(), + params_preset: threshold_preset.clone(), }), CorrelationId::new(), e3_id.clone(), ); @@ let c4b_request = ComputeRequest::zk( ZkRequest::DkgShareDecryption(DkgShareDecryptionProofRequest { sk_bfv: current.sk_bfv.clone(), honest_ciphertexts_raw: esi_cts, num_honest_parties: num_honest, num_moduli: num_moduli_esi, dkg_input_type: DkgInputType::SmudgingNoise, - params_preset: self.share_enc_preset.clone(), + params_preset: threshold_preset.clone(), }), correlation_id, e3_id.clone(), );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/keyshare/src/threshold_keyshare.rs` around lines 1432 - 1462, The DkgShareDecryptionProofRequest is being passed the DKG BFV preset (self.share_enc_preset) which the share-decryption circuit rejects; update both places where DkgShareDecryptionProofRequest is constructed (the C4a request and each C4b request) to pass the threshold BFV preset instead (for example use self.threshold_share_enc_preset or obtain the threshold preset via a helper like self.get_threshold_preset()), and if that field/helper does not exist add it so the proof requests use the threshold preset rather than self.share_enc_preset.
🧹 Nitpick comments (2)
crates/keyshare/src/threshold_keyshare.rs (2)
1792-1793: C4 verification for externalDecryptionKeySharedis still TODO.If this path is in scope soon, I can draft a follow-up patch/issue to validate incoming
c4a_proof/c4b_proofsbefore accepting persisted external key docs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/keyshare/src/threshold_keyshare.rs` around lines 1792 - 1793, Implement C4 proof verification before accepting/persisting external DecryptionKeyShared documents: in the code path that handles incoming DecryptionKeyShared (the block containing the TODO comment), validate the included c4a_proof and each entry in c4b_proofs against the expected public inputs and keys, reject or return an error on any invalid proof, and only store the DecryptionKeyShared after all proofs pass; reference the DecryptionKeyShared struct fields c4a_proof and c4b_proofs and perform verification using the existing C4 verification utilities (or add a helper like verify_c4_proofs) so persistence logic runs only after successful verification.
1484-1493: Harden C4a response handling with correlation validation (match C4b safety model).Line 1485-L1488 accepts any
SecretKeyC4 response without checking correlation ID, while C4b is correlation-bound. Storing/checking a dedicated C4a correlation ID would prevent stale/stray responses from overwritingc4a_proof.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/keyshare/src/threshold_keyshare.rs` around lines 1484 - 1493, The SecretKey branch currently accepts any C4a response and sets self.c4a_proof without validating correlation_id; add correlation validation like the SmudgingNoise case by checking and removing an expected correlation entry before accepting the proof. Concretely, introduce/usage of a correlation store (e.g., self.c4a_correlation_map or similar) and in the DkgInputType::SecretKey arm call self.c4a_correlation_map.remove(&correlation_id).ok_or_else(...) and only then assign self.c4a_proof = Some(resp.proof); ensure the error path mirrors the existing correlation error handling used for c4b (c4b_correlation_map) so stray/stale C4a responses cannot overwrite c4a_proof.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@crates/keyshare/src/threshold_keyshare.rs`:
- Around line 1432-1462: The DkgShareDecryptionProofRequest is being passed the
DKG BFV preset (self.share_enc_preset) which the share-decryption circuit
rejects; update both places where DkgShareDecryptionProofRequest is constructed
(the C4a request and each C4b request) to pass the threshold BFV preset instead
(for example use self.threshold_share_enc_preset or obtain the threshold preset
via a helper like self.get_threshold_preset()), and if that field/helper does
not exist add it so the proof requests use the threshold preset rather than
self.share_enc_preset.
---
Nitpick comments:
In `@crates/keyshare/src/threshold_keyshare.rs`:
- Around line 1792-1793: Implement C4 proof verification before
accepting/persisting external DecryptionKeyShared documents: in the code path
that handles incoming DecryptionKeyShared (the block containing the TODO
comment), validate the included c4a_proof and each entry in c4b_proofs against
the expected public inputs and keys, reject or return an error on any invalid
proof, and only store the DecryptionKeyShared after all proofs pass; reference
the DecryptionKeyShared struct fields c4a_proof and c4b_proofs and perform
verification using the existing C4 verification utilities (or add a helper like
verify_c4_proofs) so persistence logic runs only after successful verification.
- Around line 1484-1493: The SecretKey branch currently accepts any C4a response
and sets self.c4a_proof without validating correlation_id; add correlation
validation like the SmudgingNoise case by checking and removing an expected
correlation entry before accepting the proof. Concretely, introduce/usage of a
correlation store (e.g., self.c4a_correlation_map or similar) and in the
DkgInputType::SecretKey arm call
self.c4a_correlation_map.remove(&correlation_id).ok_or_else(...) and only then
assign self.c4a_proof = Some(resp.proof); ensure the error path mirrors the
existing correlation error handling used for c4b (c4b_correlation_map) so
stray/stale C4a responses cannot overwrite c4a_proof.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
crates/keyshare/src/threshold_keyshare.rscrates/zk-prover/src/actors/proof_request.rs
hmzakhalid
approved these changes
Feb 27, 2026
This was referenced Feb 27, 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.
fix #1361
Summary by CodeRabbit
New Features
Tests