From a6891e1855f8af5256305457ed2a10333431ca64 Mon Sep 17 00:00:00 2001 From: ctrlc03 <93448202+ctrlc03@users.noreply.github.com> Date: Thu, 19 Mar 2026 17:10:42 +0000 Subject: [PATCH 01/11] feat: connect c1 and c5 --- crates/aggregator/src/publickey_aggregator.rs | 261 ++++++++++++++++- .../src/enclave_event/compute_request/zk.rs | 13 + crates/events/src/enclave_event/proof.rs | 119 ++++++++ crates/multithread/src/multithread.rs | 41 +++ crates/zk-helpers/src/circuits/mod.rs | 2 + .../zk-helpers/src/circuits/output_layout.rs | 274 ++++++++++++++++++ 6 files changed, 700 insertions(+), 10 deletions(-) create mode 100644 crates/zk-helpers/src/circuits/output_layout.rs diff --git a/crates/aggregator/src/publickey_aggregator.rs b/crates/aggregator/src/publickey_aggregator.rs index 10f9f3bccb..79561575ba 100644 --- a/crates/aggregator/src/publickey_aggregator.rs +++ b/crates/aggregator/src/publickey_aggregator.rs @@ -9,12 +9,12 @@ use actix::prelude::*; use anyhow::Result; use e3_data::Persistable; use e3_events::{ - prelude::*, BusHandle, ComputeResponse, ComputeResponseKind, DKGRecursiveAggregationComplete, - Die, E3id, EnclaveEvent, EnclaveEventData, EventContext, KeyshareCreated, OrderedSet, - PartyProofsToVerify, PkAggregationProofPending, PkAggregationProofRequest, - PkAggregationProofSigned, Proof, PublicKeyAggregated, Seed, Sequenced, - ShareVerificationComplete, ShareVerificationDispatched, SignedProofPayload, TypedEvent, - VerificationKind, ZkResponse, + prelude::*, BusHandle, ComputeRequestErrorKind, ComputeResponse, ComputeResponseKind, + DKGRecursiveAggregationComplete, Die, E3id, EnclaveEvent, EnclaveEventData, EventContext, + KeyshareCreated, OrderedSet, PartyProofsToVerify, PkAggregationProofPending, + PkAggregationProofRequest, PkAggregationProofSigned, Proof, ProofType, PublicKeyAggregated, + Seed, Sequenced, ShareVerificationComplete, ShareVerificationDispatched, SignedProofFailed, + SignedProofPayload, TypedEvent, VerificationKind, ZkError, ZkResponse, }; use e3_events::{trap, EType}; use e3_fhe::{Fhe, GetAggregatePublicKey}; @@ -25,6 +25,17 @@ use std::collections::{BTreeSet, HashMap}; use std::sync::Arc; use tracing::{error, info, warn}; +/// Derive c1_commitments from signed proofs by extracting pk_commitment from each. +fn derive_c1_commitments(signed_proofs: &[Option]) -> Vec { + signed_proofs + .iter() + .filter_map(|opt| { + opt.as_ref() + .and_then(|sp| sp.payload.proof.extract_output("pk_commitment")) + }) + .collect() +} + #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] pub enum PublicKeyAggregatorState { Collecting { @@ -53,6 +64,11 @@ pub enum PublicKeyAggregatorState { GeneratingC5Proof { public_key: ArcBytes, keyshare_bytes: Vec, + /// Signed C1 proofs from honest parties, aligned with `keyshare_bytes`. + /// Commitments are extracted on the fly via `extract_output("pk_commitment")`. + /// Retained for fault attribution if a commitment mismatch is detected later. + #[serde(default)] + c1_signed_proofs: Vec>, nodes: OrderedSet, /// DKG recursive proofs per party (restart-critical). dkg_node_proofs: HashMap>, @@ -246,6 +262,7 @@ impl PublicKeyAggregator { let PublicKeyAggregatorState::VerifyingC1 { submission_order, threshold_m, + c1_proofs, .. } = self .state @@ -262,13 +279,23 @@ impl PublicKeyAggregator { // Filter out dishonest parties using submission_order (insertion-order indexed, // matching the party IDs sent to dispatch_c1_verification). - let (honest_keyshares, honest_nodes): (Vec, Vec) = submission_order + let honest_entries: Vec<(usize, (String, ArcBytes))> = submission_order .into_iter() .enumerate() .filter(|(idx, _)| !dishonest_parties.contains(&(*idx as u64))) - .map(|(_, (node, ks))| (ks, node)) + .collect(); + + let (honest_keyshares, honest_nodes): (Vec, Vec) = honest_entries + .iter() + .map(|(_, (node, ks))| (ks.clone(), node.clone())) .unzip(); + // Collect signed C1 proofs from honest parties (commitments derived on the fly) + let c1_signed_proofs: Vec> = honest_entries + .iter() + .map(|(idx, _)| c1_proofs.get(*idx).and_then(|opt| opt.clone())) + .collect(); + if !dishonest_parties.is_empty() { warn!( "Filtered out {} dishonest parties from C1 verification: {:?}", @@ -319,6 +346,7 @@ impl PublicKeyAggregator { committee_n: committee_h, committee_h, committee_threshold: 0, + c1_commitments: derive_c1_commitments(&c1_signed_proofs), }, public_key: pubkey.clone(), nodes: honest_nodes_set.clone(), @@ -329,7 +357,9 @@ impl PublicKeyAggregator { self.state.try_mutate(&ec, |_| { Ok(PublicKeyAggregatorState::GeneratingC5Proof { public_key: pubkey.clone(), - keyshare_bytes, + public_key_hash, + keyshare_bytes: honest_keyshares, + c1_signed_proofs, nodes: honest_nodes_set, dkg_node_proofs: HashMap::new(), honest_party_ids: honest_party_ids.clone(), @@ -377,6 +407,7 @@ impl PublicKeyAggregator { let PublicKeyAggregatorState::GeneratingC5Proof { public_key, keyshare_bytes, + c1_signed_proofs, nodes, dkg_node_proofs, honest_party_ids, @@ -390,6 +421,7 @@ impl PublicKeyAggregator { Ok(PublicKeyAggregatorState::GeneratingC5Proof { public_key, keyshare_bytes, + c1_signed_proofs, nodes, dkg_node_proofs, honest_party_ids, @@ -444,6 +476,7 @@ impl PublicKeyAggregator { let PublicKeyAggregatorState::GeneratingC5Proof { public_key, keyshare_bytes, + c1_signed_proofs, nodes, mut dkg_node_proofs, honest_party_ids, @@ -459,6 +492,7 @@ impl PublicKeyAggregator { Ok(PublicKeyAggregatorState::GeneratingC5Proof { public_key, keyshare_bytes, + c1_signed_proofs, nodes, dkg_node_proofs, honest_party_ids, @@ -516,6 +550,7 @@ impl PublicKeyAggregator { let PublicKeyAggregatorState::GeneratingC5Proof { public_key, keyshare_bytes, + c1_signed_proofs, nodes, dkg_node_proofs, honest_party_ids, @@ -541,6 +576,7 @@ impl PublicKeyAggregator { Ok(PublicKeyAggregatorState::GeneratingC5Proof { public_key, keyshare_bytes, + c1_signed_proofs, nodes, dkg_node_proofs, honest_party_ids, @@ -656,6 +692,7 @@ impl PublicKeyAggregator { let PublicKeyAggregatorState::GeneratingC5Proof { public_key, keyshare_bytes, + c1_signed_proofs, nodes, dkg_node_proofs, honest_party_ids, @@ -678,6 +715,7 @@ impl PublicKeyAggregator { Ok(PublicKeyAggregatorState::GeneratingC5Proof { public_key, keyshare_bytes, + c1_signed_proofs, nodes, dkg_node_proofs, honest_party_ids, @@ -692,6 +730,196 @@ impl PublicKeyAggregator { Ok(()) } + fn handle_c1_commitment_mismatch( + &mut self, + mismatched_indices: &[usize], + ec: EventContext, + ) -> Result<()> { + let PublicKeyAggregatorState::GeneratingC5Proof { + keyshare_bytes, + c1_signed_proofs: stored_c1_signed_proofs, + nodes, + dkg_node_proofs, + honest_party_ids, + dishonest_parties, + .. + } = self + .state + .get() + .ok_or_else(|| anyhow::anyhow!("Expected GeneratingC5Proof state"))? + else { + return Err(anyhow::anyhow!( + "handle_c1_commitment_mismatch called outside GeneratingC5Proof state" + )); + }; + + // Map keyshare-order indices to original party IDs. + // keyshare_bytes[i] corresponds to the i-th element in honest_party_ids (sorted). + let honest_ids_sorted: Vec = honest_party_ids.iter().copied().collect(); + let mut newly_dishonest: BTreeSet = BTreeSet::new(); + for &idx in mismatched_indices { + if let Some(&party_id) = honest_ids_sorted.get(idx) { + warn!( + "C1 commitment mismatch for party {} (index {}) — marking as dishonest", + party_id, idx + ); + newly_dishonest.insert(party_id); + } else { + warn!( + "C1 commitment mismatch index {} out of range (honest parties: {})", + idx, + honest_ids_sorted.len() + ); + } + } + + if newly_dishonest.is_empty() { + return Err(anyhow::anyhow!( + "C1 commitment mismatch reported but no valid party indices" + )); + } + + // Emit SignedProofFailed for each mismatched party that has a signed C1 proof + for &idx in mismatched_indices { + if let Some(Some(signed_payload)) = stored_c1_signed_proofs.get(idx) { + match signed_payload.recover_address() { + Ok(faulting_node) => { + if let Err(err) = self.bus.publish( + SignedProofFailed { + e3_id: self.e3_id.clone(), + faulting_node, + proof_type: ProofType::C1PkGeneration, + signed_payload: signed_payload.clone(), + }, + ec.clone(), + ) { + error!("Failed to publish SignedProofFailed for C1 mismatch at index {}: {err}", idx); + } + } + Err(err) => { + warn!( + "Could not recover address from C1 signed proof at index {}: {err}", + idx + ); + } + } + } + } + + // Filter out the newly dishonest parties + let remaining_keyshares: Vec = keyshare_bytes + .iter() + .enumerate() + .filter(|(i, _)| !mismatched_indices.contains(i)) + .map(|(_, ks)| ks.clone()) + .collect(); + + let remaining_ids: BTreeSet = honest_party_ids + .iter() + .copied() + .filter(|id| !newly_dishonest.contains(id)) + .collect(); + + let remaining_nodes: OrderedSet = { + let nodes_vec: Vec = nodes + .iter() + .enumerate() + .filter(|(i, _)| !mismatched_indices.contains(i)) + .map(|(_, n)| n.clone()) + .collect(); + OrderedSet::from(nodes_vec) + }; + + let mut all_dishonest = dishonest_parties.clone(); + all_dishonest.extend(newly_dishonest.iter()); + + // Check if enough honest parties remain + let remaining_count = remaining_keyshares.len(); + // We need > 0 honest parties; the circuit enforces the threshold check + if remaining_count == 0 { + return Err(anyhow::anyhow!( + "No honest parties remaining after C1 commitment mismatch filtering" + )); + } + + info!( + "Re-aggregating public key from {} remaining honest parties (removed {} dishonest)", + remaining_count, + newly_dishonest.len() + ); + + // Re-aggregate the public key without the dishonest parties + let remaining_keyshares_set = OrderedSet::from(remaining_keyshares.clone()); + let pubkey = self.fhe.get_aggregate_public_key(GetAggregatePublicKey { + keyshares: remaining_keyshares_set, + })?; + + let public_key_hash = compute_pk_commitment( + pubkey.clone(), + self.fhe.params.degree(), + self.fhe.params.plaintext(), + self.fhe.params.moduli().to_vec(), + )?; + + let committee_h = remaining_count; + let pubkey = ArcBytes::from_bytes(&pubkey); + + // Filter c1_signed_proofs to match remaining honest parties + let remaining_c1_signed_proofs: Vec> = stored_c1_signed_proofs + .iter() + .enumerate() + .filter(|(i, _)| !mismatched_indices.contains(i)) + .map(|(_, sp)| sp.clone()) + .collect(); + + // Publish new PkAggregationProofPending + self.bus.publish( + PkAggregationProofPending { + e3_id: self.e3_id.clone(), + proof_request: PkAggregationProofRequest { + keyshare_bytes: remaining_keyshares.clone(), + aggregated_pk_bytes: pubkey.clone(), + params_preset: self.params_preset.clone(), + committee_n: committee_h, + committee_h, + committee_threshold: 0, + c1_commitments: derive_c1_commitments(&remaining_c1_signed_proofs), + }, + public_key: pubkey.clone(), + public_key_hash, + nodes: remaining_nodes.clone(), + }, + ec.clone(), + )?; + + // Keep DKG proofs from remaining honest parties — they won't be re-delivered. + let remaining_dkg_proofs: HashMap = dkg_node_proofs + .into_iter() + .filter(|(pid, _)| remaining_ids.contains(pid)) + .collect(); + + // Transition state: reset fold but preserve honest DKG proofs + self.state.try_mutate(&ec, |_| { + Ok(PublicKeyAggregatorState::GeneratingC5Proof { + public_key: pubkey.clone(), + public_key_hash, + keyshare_bytes: remaining_keyshares, + c1_signed_proofs: remaining_c1_signed_proofs, + nodes: remaining_nodes, + dkg_node_proofs: remaining_dkg_proofs, + honest_party_ids: remaining_ids, + dishonest_parties: all_dishonest, + cross_node_fold: ProofFoldState::new(), + c5_proof_pending: None, + last_ec: Some(ec.clone()), + }) + })?; + + self.try_start_cross_node_fold(&ec)?; + + Ok(()) + } + pub fn handle_member_expelled( &mut self, node: &str, @@ -783,7 +1011,20 @@ impl Handler for PublicKeyAggregator { self.notify_sync(ctx, TypedEvent::new(data, ec)) } EnclaveEventData::ComputeRequestError(data) => { - error!("PublicKeyAggregator received ComputeRequestError: {}", data); + if data.request().e3_id != self.e3_id { + return; + } + if let ComputeRequestErrorKind::Zk(ZkError::C1CommitmentMismatch { + ref mismatched_indices, + }) = data.get_err() + { + let indices = mismatched_indices.clone(); + trap(EType::PublickeyAggregation, &self.bus.with_ec(&ec), || { + self.handle_c1_commitment_mismatch(&indices, ec.clone()) + }); + } else { + error!("PublicKeyAggregator received ComputeRequestError: {}", data); + } } EnclaveEventData::E3RequestComplete(_) => self.notify_sync(ctx, Die), EnclaveEventData::CommitteeMemberExpelled(data) => { diff --git a/crates/events/src/enclave_event/compute_request/zk.rs b/crates/events/src/enclave_event/compute_request/zk.rs index d274bfbcc2..57f451592b 100644 --- a/crates/events/src/enclave_event/compute_request/zk.rs +++ b/crates/events/src/enclave_event/compute_request/zk.rs @@ -61,6 +61,8 @@ pub struct PkAggregationProofRequest { pub committee_h: usize, /// Threshold (T). pub committee_threshold: usize, + /// C1 commitments extracted from honest parties' signed C1 proofs. + pub c1_commitments: Vec, } /// Request to generate a proof for share computation (C2a or C2b). @@ -446,6 +448,12 @@ pub enum ZkError { WitnessGenerationFailed(String), /// Invalid parameters. InvalidParams(String), + /// C1 commitment mismatch: the commitment extracted from a party's C1 proof + /// does not match the commitment computed from their keyshare data. + C1CommitmentMismatch { + /// Indices (into `keyshare_bytes` / `c1_commitments`) of mismatched parties. + mismatched_indices: Vec, + }, } impl std::fmt::Display for ZkError { @@ -456,6 +464,11 @@ impl std::fmt::Display for ZkError { write!(f, "Witness generation failed: {}", msg) } ZkError::InvalidParams(msg) => write!(f, "Invalid parameters: {}", msg), + ZkError::C1CommitmentMismatch { mismatched_indices } => write!( + f, + "C1 commitment mismatch at indices: {:?}", + mismatched_indices + ), } } } diff --git a/crates/events/src/enclave_event/proof.rs b/crates/events/src/enclave_event/proof.rs index 2c3a18ebdb..536f436c09 100644 --- a/crates/events/src/enclave_event/proof.rs +++ b/crates/events/src/enclave_event/proof.rs @@ -2,6 +2,10 @@ use derivative::Derivative; use e3_utils::utility_types::ArcBytes; +use e3_zk_helpers::{ + CircuitOutputLayout, DKG_SHARE_DECRYPTION_OUTPUTS, PK_AGGREGATION_OUTPUTS, PK_BFV_OUTPUTS, + PK_GENERATION_OUTPUTS, SHARE_COMPUTATION_CHUNK_BATCH_OUTPUTS, SHARE_COMPUTATION_OUTPUTS, +}; use serde::{Deserialize, Serialize}; use std::fmt; @@ -31,6 +35,18 @@ impl Proof { public_signals: public_signals.into(), } } + + /// Extract a named public output field from this proof's public signals. + /// + /// Return values sit at the **end** of `public_signals`, after any `pub` + /// input parameters. The field name must match one declared in the + /// circuit's [`CircuitOutputLayout`]. + pub fn extract_output(&self, field_name: &str) -> Option { + let layout = self.circuit.output_layout(); + layout + .extract_field(&self.public_signals, field_name) + .map(ArcBytes::from_bytes) + } } /// Circuit variants determine the hash oracle used for VK generation and proving. @@ -150,6 +166,38 @@ impl CircuitName { pub fn wrapper_dir_path(&self) -> String { format!("recursive_aggregation/wrapper/{}", self.dir_path()) } + + /// Public output (return value) layout for this circuit. + pub fn output_layout(&self) -> CircuitOutputLayout { + match self { + CircuitName::PkBfv => CircuitOutputLayout::Fixed { + fields: PK_BFV_OUTPUTS, + }, + CircuitName::PkGeneration => CircuitOutputLayout::Fixed { + fields: PK_GENERATION_OUTPUTS, + }, + CircuitName::SkShareComputationBase | CircuitName::ESmShareComputationBase => { + CircuitOutputLayout::Dynamic + } + CircuitName::ShareComputationChunkBatch => CircuitOutputLayout::Fixed { + fields: SHARE_COMPUTATION_CHUNK_BATCH_OUTPUTS, + }, + CircuitName::ShareComputation => CircuitOutputLayout::Fixed { + fields: SHARE_COMPUTATION_OUTPUTS, + }, + CircuitName::DkgShareDecryption => CircuitOutputLayout::Fixed { + fields: DKG_SHARE_DECRYPTION_OUTPUTS, + }, + CircuitName::PkAggregation => CircuitOutputLayout::Fixed { + fields: PK_AGGREGATION_OUTPUTS, + }, + CircuitName::ShareComputationChunk + | CircuitName::ShareEncryption + | CircuitName::ThresholdShareDecryption + | CircuitName::DecryptedSharesAggregation => CircuitOutputLayout::None, + CircuitName::Fold => CircuitOutputLayout::None, + } + } } impl fmt::Display for CircuitName { @@ -157,3 +205,74 @@ impl fmt::Display for CircuitName { write!(f, "{}", self.dir_path()) } } + +#[cfg(test)] +mod tests { + use super::*; + + fn make_proof(circuit: CircuitName, signals: &[u8]) -> Proof { + Proof::new( + circuit, + ArcBytes::from_bytes(&[0u8; 8]), + ArcBytes::from_bytes(signals), + ) + } + + #[test] + fn extract_c1_pk_commitment() { + // C1 has 3 outputs: sk_commitment, pk_commitment, e_sm_commitment + let mut signals = vec![0u8; 96]; + signals[0..32].copy_from_slice(&[0x11; 32]); // sk_commitment + signals[32..64].copy_from_slice(&[0x22; 32]); // pk_commitment + signals[64..96].copy_from_slice(&[0x33; 32]); // e_sm_commitment + + let proof = make_proof(CircuitName::PkGeneration, &signals); + assert_eq!( + &*proof.extract_output("pk_commitment").unwrap(), + &[0x22; 32] + ); + assert_eq!( + &*proof.extract_output("sk_commitment").unwrap(), + &[0x11; 32] + ); + assert_eq!( + &*proof.extract_output("e_sm_commitment").unwrap(), + &[0x33; 32] + ); + } + + #[test] + fn extract_c5_commitment_after_pub_inputs() { + // C5 has H pub input fields + 1 output. Simulate H=2 → 96 bytes total. + let mut signals = vec![0xAA; 96]; + signals[64..96].copy_from_slice(&[0xFF; 32]); // commitment (last output) + + let proof = make_proof(CircuitName::PkAggregation, &signals); + assert_eq!(&*proof.extract_output("commitment").unwrap(), &[0xFF; 32]); + } + + #[test] + fn extract_nonexistent_field() { + let proof = make_proof(CircuitName::PkBfv, &[0u8; 32]); + assert!(proof.extract_output("nonexistent").is_none()); + } + + #[test] + fn extract_from_void_circuit() { + let proof = make_proof(CircuitName::ShareEncryption, &[0u8; 64]); + assert!(proof.extract_output("commitment").is_none()); + } + + #[test] + fn extract_signals_too_short() { + // C1 needs 96 bytes for outputs, only 64 available + let proof = make_proof(CircuitName::PkGeneration, &[0u8; 64]); + assert!(proof.extract_output("pk_commitment").is_none()); + } + + #[test] + fn extract_empty_signals() { + let proof = make_proof(CircuitName::PkGeneration, &[]); + assert!(proof.extract_output("pk_commitment").is_none()); + } +} diff --git a/crates/multithread/src/multithread.rs b/crates/multithread/src/multithread.rs index f49ce16857..65a46360a0 100644 --- a/crates/multithread/src/multithread.rs +++ b/crates/multithread/src/multithread.rs @@ -66,6 +66,7 @@ use e3_zk_helpers::dkg::share_computation::{ }; use e3_zk_helpers::dkg::share_decryption::{ShareDecryptionCircuit, ShareDecryptionCircuitData}; use e3_zk_helpers::dkg::share_encryption::{ShareEncryptionCircuit, ShareEncryptionCircuitData}; +use e3_zk_helpers::threshold::pk_aggregation::computation::Inputs as PkAggInputs; use e3_zk_helpers::threshold::pk_aggregation::PkAggregationCircuit; use e3_zk_helpers::threshold::pk_aggregation::PkAggregationCircuitData; use e3_zk_helpers::CiphernodesCommittee; @@ -344,6 +345,46 @@ fn handle_pk_aggregation_proof( a, }; + // 6b. Verify C1 commitments from signed proofs match computed commitments + if req.c1_commitments.len() != req.committee_h { + return Err(make_zk_error( + &request, + format!( + "c1_commitments length {} != committee_h {}", + req.c1_commitments.len(), + req.committee_h + ), + )); + } + + let pk_agg_inputs = PkAggInputs::compute(req.params_preset.clone(), &circuit_data) + .map_err(|e| make_zk_error(&request, format!("PkAggInputs::compute: {}", e)))?; + + // public_signals from bb are big-endian 32-byte field elements + let expected = &pk_agg_inputs.expected_threshold_pk_commitments; + let mut mismatched_indices = Vec::new(); + for (i, (computed, extracted)) in expected.iter().zip(req.c1_commitments.iter()).enumerate() { + let (_, be_bytes) = computed.to_bytes_be(); + let mut padded = vec![0u8; 32]; + let start = 32_usize.saturating_sub(be_bytes.len()); + padded[start..].copy_from_slice(&be_bytes); + if padded[..] != extracted[..] { + mismatched_indices.push(i); + } + } + + if !mismatched_indices.is_empty() { + return Err(ComputeRequestError::new( + ComputeRequestErrorKind::Zk(ZkEventError::C1CommitmentMismatch { mismatched_indices }), + request, + )); + } + + info!( + "C1 commitment cross-check passed for {} parties", + expected.len() + ); + // 7. Generate proof via Provable trait (C5 is always EVM-targeted for on-chain verification) let circuit = PkAggregationCircuit; let e3_id_str = request.e3_id.to_string(); diff --git a/crates/zk-helpers/src/circuits/mod.rs b/crates/zk-helpers/src/circuits/mod.rs index a60579d5df..d3f8713990 100644 --- a/crates/zk-helpers/src/circuits/mod.rs +++ b/crates/zk-helpers/src/circuits/mod.rs @@ -8,6 +8,7 @@ pub mod codegen; pub mod commitments; pub mod computation; pub mod errors; +pub mod output_layout; pub use codegen::{ write_artifacts, write_toml, Artifacts, CircuitCodegen, CodegenConfigs, CodegenToml, @@ -15,6 +16,7 @@ pub use codegen::{ pub use commitments::*; pub use computation::{CircuitComputation, Computation}; pub use errors::CircuitsErrors; +pub use output_layout::*; pub mod dkg; pub mod threshold; diff --git a/crates/zk-helpers/src/circuits/output_layout.rs b/crates/zk-helpers/src/circuits/output_layout.rs new file mode 100644 index 0000000000..63bcb89f54 --- /dev/null +++ b/crates/zk-helpers/src/circuits/output_layout.rs @@ -0,0 +1,274 @@ +// SPDX-License-Identifier: LGPL-3.0-only +// +// This file is provided WITHOUT ANY WARRANTY; +// without even the implied warranty of MERCHANTABILITY +// or FITNESS FOR A PARTICULAR PURPOSE. + +//! Describes the public output (return value) layout of each ZK circuit. +//! +//! In Noir, circuits declare `pub` input parameters and `-> pub` return values. +//! Both end up in the proof's `public_signals` byte array, with return values +//! placed **after** all public inputs. This module provides the metadata needed +//! to extract named return fields from a proof's public signals without +//! hard-coding byte offsets. + +use serde::{Deserialize, Serialize}; + +/// Size of a single Noir `Field` element in bytes (BN254 scalar). +pub const FIELD_BYTE_LEN: usize = 32; + +/// A named output field of a circuit proof. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] +pub struct OutputField { + /// Human-readable name (e.g. `"pk_commitment"`). + pub name: &'static str, +} + +/// Describes the public return values of a circuit. +/// +/// `fields` lists them in the order they appear in `public_signals`, +/// which is the same order as the Noir `-> pub (A, B, C)` tuple. +/// +/// Circuits whose output count depends on runtime parameters (e.g. +/// `SkShareComputationBase` whose return is `[[Field; L]; N]`) +/// use [`CircuitOutputLayout::Dynamic`]. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub enum CircuitOutputLayout { + /// Fixed number of `Field`-sized outputs, names known at compile time. + Fixed { fields: &'static [OutputField] }, + /// The circuit returns no public values (void). + None, + /// Output count depends on runtime parameters — callers must supply the + /// element count themselves. + Dynamic, +} + +impl CircuitOutputLayout { + /// Number of fixed output fields, or `None` for dynamic / void layouts. + pub fn field_count(&self) -> Option { + match self { + CircuitOutputLayout::Fixed { fields } => Some(fields.len()), + CircuitOutputLayout::None => Some(0), + CircuitOutputLayout::Dynamic => None, + } + } + + /// Look up a field index by name. + pub fn field_index(&self, name: &str) -> Option { + match self { + CircuitOutputLayout::Fixed { fields } => fields.iter().position(|f| f.name == name), + _ => None, + } + } + + /// Extract a named output field from raw `public_signals` bytes. + /// + /// Return values sit at the **end** of `public_signals`, after any + /// `pub` input parameters. This method indexes from the tail. + pub fn extract_field<'a>(&self, public_signals: &'a [u8], name: &str) -> Option<&'a [u8]> { + let fields = match self { + CircuitOutputLayout::Fixed { fields } => fields, + _ => return None, + }; + let idx = fields.iter().position(|f| f.name == name)?; + let total_output_bytes = fields.len() * FIELD_BYTE_LEN; + if public_signals.len() < total_output_bytes { + return None; + } + let output_start = public_signals.len() - total_output_bytes; + let offset = output_start + idx * FIELD_BYTE_LEN; + Some(&public_signals[offset..offset + FIELD_BYTE_LEN]) + } + + /// Extract all output fields from raw `public_signals` bytes. + /// + /// Returns a vec of `(name, &[u8])` pairs in field order. + pub fn extract_all<'a>( + &self, + public_signals: &'a [u8], + ) -> Option> { + let fields = match self { + CircuitOutputLayout::Fixed { fields } => fields, + CircuitOutputLayout::None => return Some(Vec::new()), + CircuitOutputLayout::Dynamic => return None, + }; + let total_output_bytes = fields.len() * FIELD_BYTE_LEN; + if public_signals.len() < total_output_bytes { + return None; + } + let output_start = public_signals.len() - total_output_bytes; + Some( + fields + .iter() + .enumerate() + .map(|(i, f)| { + let offset = output_start + i * FIELD_BYTE_LEN; + (f.name, &public_signals[offset..offset + FIELD_BYTE_LEN]) + }) + .collect(), + ) + } +} + +// ── Per-circuit output field constants ────────────────────────────────────── + +const fn f(name: &'static str) -> OutputField { + OutputField { name } +} + +/// C0 — BFV public key proof. +pub const PK_BFV_OUTPUTS: &[OutputField] = &[f("pk_commitment")]; + +/// C1 — Threshold public key generation. +pub const PK_GENERATION_OUTPUTS: &[OutputField] = + &[f("sk_commitment"), f("pk_commitment"), f("e_sm_commitment")]; + +/// C2d — Share computation chunk batch. +pub const SHARE_COMPUTATION_CHUNK_BATCH_OUTPUTS: &[OutputField] = &[f("commitment")]; + +/// C2 — Share computation (final wrapper). +pub const SHARE_COMPUTATION_OUTPUTS: &[OutputField] = &[f("key_hash"), f("commitment")]; + +/// C4 — DKG share decryption. +pub const DKG_SHARE_DECRYPTION_OUTPUTS: &[OutputField] = &[f("commitment")]; + +/// C5 — Public key aggregation. +pub const PK_AGGREGATION_OUTPUTS: &[OutputField] = &[f("commitment")]; + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn extract_single_output_field() { + let layout = CircuitOutputLayout::Fixed { + fields: PK_BFV_OUTPUTS, + }; + // 32 bytes pub input + 32 bytes output + let mut signals = vec![0xAAu8; 64]; + signals[32..].copy_from_slice(&[0xBB; 32]); + let commitment = layout.extract_field(&signals, "pk_commitment").unwrap(); + assert_eq!(commitment, &[0xBB; 32]); + } + + #[test] + fn extract_c1_pk_commitment_from_middle() { + let layout = CircuitOutputLayout::Fixed { + fields: PK_GENERATION_OUTPUTS, + }; + // C1 has no pub inputs, only 3 outputs = 96 bytes total + let mut signals = vec![0u8; 96]; + signals[0..32].copy_from_slice(&[0x11; 32]); // sk_commitment + signals[32..64].copy_from_slice(&[0x22; 32]); // pk_commitment + signals[64..96].copy_from_slice(&[0x33; 32]); // e_sm_commitment + + assert_eq!( + layout.extract_field(&signals, "sk_commitment").unwrap(), + &[0x11; 32] + ); + assert_eq!( + layout.extract_field(&signals, "pk_commitment").unwrap(), + &[0x22; 32] + ); + assert_eq!( + layout.extract_field(&signals, "e_sm_commitment").unwrap(), + &[0x33; 32] + ); + } + + #[test] + fn extract_c5_output_after_pub_inputs() { + let layout = CircuitOutputLayout::Fixed { + fields: PK_AGGREGATION_OUTPUTS, + }; + // C5 has H pub input fields + 1 output. Simulate H=3 → 128 bytes total. + let mut signals = vec![0xAA; 128]; // 3 * 32 pub inputs + signals[96..128].copy_from_slice(&[0xFF; 32]); // 1 output at the end + let commitment = layout.extract_field(&signals, "commitment").unwrap(); + assert_eq!(commitment, &[0xFF; 32]); + } + + #[test] + fn extract_c2_two_outputs() { + let layout = CircuitOutputLayout::Fixed { + fields: SHARE_COMPUTATION_OUTPUTS, + }; + // C2 has 1 pub input (key_hash) + 2 outputs = 96 bytes + let mut signals = vec![0x00; 96]; + signals[32..64].copy_from_slice(&[0xAA; 32]); // key_hash output + signals[64..96].copy_from_slice(&[0xBB; 32]); // commitment output + + assert_eq!( + layout.extract_field(&signals, "key_hash").unwrap(), + &[0xAA; 32] + ); + assert_eq!( + layout.extract_field(&signals, "commitment").unwrap(), + &[0xBB; 32] + ); + } + + #[test] + fn extract_nonexistent_field_returns_none() { + let layout = CircuitOutputLayout::Fixed { + fields: PK_BFV_OUTPUTS, + }; + let signals = vec![0u8; 32]; + assert!(layout.extract_field(&signals, "nonexistent").is_none()); + } + + #[test] + fn extract_from_void_circuit_returns_none() { + let layout = CircuitOutputLayout::None; + let signals = vec![0u8; 64]; + assert!(layout.extract_field(&signals, "anything").is_none()); + } + + #[test] + fn extract_from_dynamic_circuit_returns_none() { + let layout = CircuitOutputLayout::Dynamic; + let signals = vec![0u8; 256]; + assert!(layout.extract_field(&signals, "anything").is_none()); + } + + #[test] + fn signals_too_short_returns_none() { + let layout = CircuitOutputLayout::Fixed { + fields: PK_GENERATION_OUTPUTS, + }; + // Need 96 bytes for 3 outputs, only 64 available + let signals = vec![0u8; 64]; + assert!(layout.extract_field(&signals, "pk_commitment").is_none()); + } + + #[test] + fn extract_all_c1_outputs() { + let layout = CircuitOutputLayout::Fixed { + fields: PK_GENERATION_OUTPUTS, + }; + let mut signals = vec![0u8; 96]; + signals[0..32].copy_from_slice(&[0x11; 32]); + signals[32..64].copy_from_slice(&[0x22; 32]); + signals[64..96].copy_from_slice(&[0x33; 32]); + + let all = layout.extract_all(&signals).unwrap(); + assert_eq!(all.len(), 3); + assert_eq!(all[0].0, "sk_commitment"); + assert_eq!(all[1].0, "pk_commitment"); + assert_eq!(all[2].0, "e_sm_commitment"); + assert_eq!(all[1].1, &[0x22; 32]); + } + + #[test] + fn field_count() { + assert_eq!( + CircuitOutputLayout::Fixed { + fields: PK_GENERATION_OUTPUTS + } + .field_count(), + Some(3) + ); + assert_eq!(CircuitOutputLayout::None.field_count(), Some(0)); + assert_eq!(CircuitOutputLayout::Dynamic.field_count(), None); + } +} From 14dcd5dfd895c9527dbe205514eadfa9b2d6b4c7 Mon Sep 17 00:00:00 2001 From: ctrlc03 <93448202+ctrlc03@users.noreply.github.com> Date: Mon, 23 Mar 2026 15:48:22 +0000 Subject: [PATCH 02/11] chore: update computation for c1 --- crates/aggregator/src/publickey_aggregator.rs | 57 ++++++++++++++----- crates/multithread/src/multithread.rs | 12 ++++ .../threshold/pk_generation/computation.rs | 29 ++++++++-- 3 files changed, 77 insertions(+), 21 deletions(-) diff --git a/crates/aggregator/src/publickey_aggregator.rs b/crates/aggregator/src/publickey_aggregator.rs index 79561575ba..328ab5986b 100644 --- a/crates/aggregator/src/publickey_aggregator.rs +++ b/crates/aggregator/src/publickey_aggregator.rs @@ -29,9 +29,31 @@ use tracing::{error, info, warn}; fn derive_c1_commitments(signed_proofs: &[Option]) -> Vec { signed_proofs .iter() - .filter_map(|opt| { - opt.as_ref() - .and_then(|sp| sp.payload.proof.extract_output("pk_commitment")) + .enumerate() + .filter_map(|(i, opt)| { + let sp = opt.as_ref()?; + let proof = &sp.payload.proof; + tracing::info!( + "C1 proof[{}]: circuit={:?}, public_signals_len={}, signals_hex={}", + i, + proof.circuit, + proof.public_signals.len(), + proof.public_signals[..std::cmp::min(128, proof.public_signals.len())] + .iter() + .map(|b| format!("{:02x}", b)) + .collect::() + ); + let commitment = proof.extract_output("pk_commitment"); + if let Some(ref c) = commitment { + tracing::info!( + "C1 proof[{}]: extracted pk_commitment={}", + i, + c.iter().map(|b| format!("{:02x}", b)).collect::() + ); + } else { + tracing::warn!("C1 proof[{}]: failed to extract pk_commitment", i); + } + commitment }) .collect() } @@ -67,7 +89,6 @@ pub enum PublicKeyAggregatorState { /// Signed C1 proofs from honest parties, aligned with `keyshare_bytes`. /// Commitments are extracted on the fly via `extract_output("pk_commitment")`. /// Retained for fault attribution if a commitment mismatch is detected later. - #[serde(default)] c1_signed_proofs: Vec>, nodes: OrderedSet, /// DKG recursive proofs per party (restart-critical). @@ -329,7 +350,23 @@ impl PublicKeyAggregator { let committee_h = honest_keyshares.len(); let honest_nodes_set = OrderedSet::from(honest_nodes); + // keyshare_bytes follows OrderedSet ordering (used by C5 prover). + // Re-align c1_signed_proofs to the same order so c1_commitments[i] + // corresponds to keyshare_bytes[i]. let keyshare_bytes: Vec<_> = honest_keyshares_set.iter().cloned().collect(); + let c1_signed_proofs = { + // Build a map from keyshare → c1 proof, then iterate in OrderedSet order. + let ks_to_proof: std::collections::HashMap, &Option> = + honest_keyshares + .iter() + .zip(c1_signed_proofs.iter()) + .map(|(ks, proof)| (ks.to_vec(), proof)) + .collect(); + keyshare_bytes + .iter() + .map(|ks| ks_to_proof.get(&ks.to_vec()).and_then(|opt| (*opt).clone())) + .collect::>() + }; // Publish pending event before transitioning state so a publish // failure leaves us in VerifyingC1 (retryable) rather than @@ -357,7 +394,6 @@ impl PublicKeyAggregator { self.state.try_mutate(&ec, |_| { Ok(PublicKeyAggregatorState::GeneratingC5Proof { public_key: pubkey.clone(), - public_key_hash, keyshare_bytes: honest_keyshares, c1_signed_proofs, nodes: honest_nodes_set, @@ -854,13 +890,6 @@ impl PublicKeyAggregator { keyshares: remaining_keyshares_set, })?; - let public_key_hash = compute_pk_commitment( - pubkey.clone(), - self.fhe.params.degree(), - self.fhe.params.plaintext(), - self.fhe.params.moduli().to_vec(), - )?; - let committee_h = remaining_count; let pubkey = ArcBytes::from_bytes(&pubkey); @@ -886,14 +915,13 @@ impl PublicKeyAggregator { c1_commitments: derive_c1_commitments(&remaining_c1_signed_proofs), }, public_key: pubkey.clone(), - public_key_hash, nodes: remaining_nodes.clone(), }, ec.clone(), )?; // Keep DKG proofs from remaining honest parties — they won't be re-delivered. - let remaining_dkg_proofs: HashMap = dkg_node_proofs + let remaining_dkg_proofs: HashMap> = dkg_node_proofs .into_iter() .filter(|(pid, _)| remaining_ids.contains(pid)) .collect(); @@ -902,7 +930,6 @@ impl PublicKeyAggregator { self.state.try_mutate(&ec, |_| { Ok(PublicKeyAggregatorState::GeneratingC5Proof { public_key: pubkey.clone(), - public_key_hash, keyshare_bytes: remaining_keyshares, c1_signed_proofs: remaining_c1_signed_proofs, nodes: remaining_nodes, diff --git a/crates/multithread/src/multithread.rs b/crates/multithread/src/multithread.rs index 65a46360a0..6c66cbc8d9 100644 --- a/crates/multithread/src/multithread.rs +++ b/crates/multithread/src/multithread.rs @@ -368,6 +368,18 @@ fn handle_pk_aggregation_proof( let mut padded = vec![0u8; 32]; let start = 32_usize.saturating_sub(be_bytes.len()); padded[start..].copy_from_slice(&be_bytes); + info!( + "C1 commitment check party {}: computed={:?} extracted={:?}", + i, + padded + .iter() + .map(|b| format!("{:02x}", b)) + .collect::(), + extracted + .iter() + .map(|b| format!("{:02x}", b)) + .collect::() + ); if padded[..] != extracted[..] { mismatched_indices.push(i); } diff --git a/crates/zk-helpers/src/circuits/threshold/pk_generation/computation.rs b/crates/zk-helpers/src/circuits/threshold/pk_generation/computation.rs index fd8aa4c829..12ff844d77 100644 --- a/crates/zk-helpers/src/circuits/threshold/pk_generation/computation.rs +++ b/crates/zk-helpers/src/circuits/threshold/pk_generation/computation.rs @@ -130,12 +130,22 @@ impl Computation for Bits { type Data = Bounds; type Error = CircuitsErrors; - fn compute(_: Self::Preset, data: &Self::Data) -> Result { + fn compute(preset: Self::Preset, data: &Self::Data) -> Result { // Calculate bit widths for each bound type let eek_bit = calculate_bit_width(BigInt::from(data.eek_bound.clone())); let sk_bit = calculate_bit_width(BigInt::from(data.sk_bound.clone())); let e_sm_bit = calculate_bit_width(BigInt::from(data.e_sm_bound.clone())); - let pk_bit = calculate_bit_width(BigInt::from(data.pk_bound.clone())); + + // pk_bit: iterate over moduli (aligned with C2 share_computation approach). + // Uses qi - 1 (full modulus range) rather than (qi - 1) / 2, which naturally + // gives one extra bit for the centered (signed) representation. + let (threshold_params, _) = + build_pair_for_preset(preset).map_err(|e| CircuitsErrors::Other(e.to_string()))?; + let mut pk_bit = 0; + for &qi in threshold_params.moduli() { + let bound = BigUint::from(qi - 1); + pk_bit = pk_bit.max(calculate_bit_width(BigInt::from(bound))); + } // For r1, use the maximum of all low and up bounds let mut r1_bit = 0; @@ -370,10 +380,17 @@ mod tests { #[test] fn test_bound_and_bits_computation_consistency() { - let bounds = Bounds::compute(BfvPreset::InsecureThreshold512, &()).unwrap(); - let bits = Bits::compute(BfvPreset::InsecureThreshold512, &bounds).unwrap(); - - let expected_bit = calculate_bit_width(BigInt::from(bounds.pk_bound.clone())); + let preset = BfvPreset::InsecureThreshold512; + let bounds = Bounds::compute(preset, &()).unwrap(); + let bits = Bits::compute(preset, &bounds).unwrap(); + + // pk_bit is computed from max(qi - 1) over all moduli (aligned with C2) + let (threshold_params, _) = build_pair_for_preset(preset).unwrap(); + let mut expected_bit = 0u32; + for &qi in threshold_params.moduli() { + expected_bit = + expected_bit.max(calculate_bit_width(BigInt::from(BigUint::from(qi - 1)))); + } assert_eq!(bits.pk_bit, expected_bit); } From f661602e7932934c0f583582a343a1eb880decd7 Mon Sep 17 00:00:00 2001 From: ctrlc03 <93448202+ctrlc03@users.noreply.github.com> Date: Mon, 23 Mar 2026 21:51:22 +0000 Subject: [PATCH 03/11] chore: pr comments --- crates/aggregator/src/publickey_aggregator.rs | 67 +++++++++++-------- crates/multithread/src/multithread.rs | 14 +++- .../threshold/pk_generation/computation.rs | 5 +- 3 files changed, 54 insertions(+), 32 deletions(-) diff --git a/crates/aggregator/src/publickey_aggregator.rs b/crates/aggregator/src/publickey_aggregator.rs index 328ab5986b..2ee17631e8 100644 --- a/crates/aggregator/src/publickey_aggregator.rs +++ b/crates/aggregator/src/publickey_aggregator.rs @@ -26,31 +26,14 @@ use std::sync::Arc; use tracing::{error, info, warn}; /// Derive c1_commitments from signed proofs by extracting pk_commitment from each. -fn derive_c1_commitments(signed_proofs: &[Option]) -> Vec { +fn derive_c1_commitments(signed_proofs: &[Option]) -> Vec> { signed_proofs .iter() .enumerate() - .filter_map(|(i, opt)| { + .map(|(i, opt)| { let sp = opt.as_ref()?; - let proof = &sp.payload.proof; - tracing::info!( - "C1 proof[{}]: circuit={:?}, public_signals_len={}, signals_hex={}", - i, - proof.circuit, - proof.public_signals.len(), - proof.public_signals[..std::cmp::min(128, proof.public_signals.len())] - .iter() - .map(|b| format!("{:02x}", b)) - .collect::() - ); - let commitment = proof.extract_output("pk_commitment"); - if let Some(ref c) = commitment { - tracing::info!( - "C1 proof[{}]: extracted pk_commitment={}", - i, - c.iter().map(|b| format!("{:02x}", b)).collect::() - ); - } else { + let commitment = sp.payload.proof.extract_output("pk_commitment"); + if commitment.is_none() { tracing::warn!("C1 proof[{}]: failed to extract pk_commitment", i); } commitment @@ -58,6 +41,18 @@ fn derive_c1_commitments(signed_proofs: &[Option]) -> Vec>` to `Vec`, substituting 32 zero bytes +/// for any `None` entry. The C5 prover will detect these as commitment mismatches. +fn unwrap_c1_commitments(commitments: &[Option]) -> Vec { + commitments + .iter() + .map(|opt| { + opt.clone() + .unwrap_or_else(|| ArcBytes::from_bytes(&[0u8; 32])) + }) + .collect() +} + #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] pub enum PublicKeyAggregatorState { Collecting { @@ -98,6 +93,8 @@ pub enum PublicKeyAggregatorState { cross_node_fold: ProofFoldState, c5_proof_pending: Option, last_ec: Option>, + /// Cryptographic threshold M, carried from Collecting for re-aggregation checks. + threshold_m: usize, }, Complete { public_key: ArcBytes, @@ -383,7 +380,9 @@ impl PublicKeyAggregator { committee_n: committee_h, committee_h, committee_threshold: 0, - c1_commitments: derive_c1_commitments(&c1_signed_proofs), + c1_commitments: unwrap_c1_commitments(&derive_c1_commitments( + &c1_signed_proofs, + )), }, public_key: pubkey.clone(), nodes: honest_nodes_set.clone(), @@ -403,6 +402,7 @@ impl PublicKeyAggregator { cross_node_fold: ProofFoldState::new(), c5_proof_pending: None, last_ec: Some(ec.clone()), + threshold_m, }) })?; @@ -449,6 +449,7 @@ impl PublicKeyAggregator { honest_party_ids, dishonest_parties, cross_node_fold, + threshold_m, .. } = state else { @@ -465,6 +466,7 @@ impl PublicKeyAggregator { cross_node_fold, c5_proof_pending: Some(c5_proof), last_ec: Some(ec.clone()), + threshold_m, }) })?; self.try_publish_complete() @@ -520,6 +522,7 @@ impl PublicKeyAggregator { cross_node_fold, c5_proof_pending, last_ec: _, + threshold_m, } = state else { return Ok(state); @@ -536,6 +539,7 @@ impl PublicKeyAggregator { cross_node_fold, c5_proof_pending, last_ec: Some(ec.clone()), + threshold_m, }) })?; @@ -594,6 +598,7 @@ impl PublicKeyAggregator { mut cross_node_fold, c5_proof_pending, last_ec, + threshold_m, } = state else { return Ok(state); @@ -620,6 +625,7 @@ impl PublicKeyAggregator { cross_node_fold, c5_proof_pending, last_ec, + threshold_m, }) })?; self.try_publish_complete() @@ -736,6 +742,7 @@ impl PublicKeyAggregator { mut cross_node_fold, c5_proof_pending, last_ec, + threshold_m, } = state else { return Ok(state); @@ -759,6 +766,7 @@ impl PublicKeyAggregator { cross_node_fold, c5_proof_pending, last_ec, + threshold_m, }) })?; self.try_publish_complete()?; @@ -778,6 +786,7 @@ impl PublicKeyAggregator { dkg_node_proofs, honest_party_ids, dishonest_parties, + threshold_m, .. } = self .state @@ -871,10 +880,11 @@ impl PublicKeyAggregator { // Check if enough honest parties remain let remaining_count = remaining_keyshares.len(); - // We need > 0 honest parties; the circuit enforces the threshold check - if remaining_count == 0 { + if remaining_count <= threshold_m { return Err(anyhow::anyhow!( - "No honest parties remaining after C1 commitment mismatch filtering" + "Not enough honest parties after C1 commitment mismatch filtering: {} (need at least {})", + remaining_count, + threshold_m + 1 )); } @@ -911,8 +921,10 @@ impl PublicKeyAggregator { params_preset: self.params_preset.clone(), committee_n: committee_h, committee_h, - committee_threshold: 0, - c1_commitments: derive_c1_commitments(&remaining_c1_signed_proofs), + committee_threshold: threshold_m, + c1_commitments: unwrap_c1_commitments(&derive_c1_commitments( + &remaining_c1_signed_proofs, + )), }, public_key: pubkey.clone(), nodes: remaining_nodes.clone(), @@ -939,6 +951,7 @@ impl PublicKeyAggregator { cross_node_fold: ProofFoldState::new(), c5_proof_pending: None, last_ec: Some(ec.clone()), + threshold_m, }) })?; diff --git a/crates/multithread/src/multithread.rs b/crates/multithread/src/multithread.rs index 6c66cbc8d9..b8b1a630ae 100644 --- a/crates/multithread/src/multithread.rs +++ b/crates/multithread/src/multithread.rs @@ -314,7 +314,19 @@ fn handle_pk_aggregation_proof( // 2. Create deterministic CRP let crp = create_deterministic_crp_from_default_seed(&threshold_params); - // 3. Deserialize each keyshare as PublicKeyShare and extract pk0 + // 3. Validate keyshare count before deserialization + if req.keyshare_bytes.len() != req.committee_h { + return Err(make_zk_error( + &request, + format!( + "keyshare_bytes length {} != committee_h {}", + req.keyshare_bytes.len(), + req.committee_h + ), + )); + } + + // 4. Deserialize each keyshare as PublicKeyShare and extract pk0 let mut pk0_shares = Vec::with_capacity(req.keyshare_bytes.len()); for (i, ks_bytes) in req.keyshare_bytes.iter().enumerate() { let pk_share = PublicKeyShare::deserialize(ks_bytes, &threshold_params, crp.clone()) diff --git a/crates/zk-helpers/src/circuits/threshold/pk_generation/computation.rs b/crates/zk-helpers/src/circuits/threshold/pk_generation/computation.rs index 12ff844d77..151727d2c4 100644 --- a/crates/zk-helpers/src/circuits/threshold/pk_generation/computation.rs +++ b/crates/zk-helpers/src/circuits/threshold/pk_generation/computation.rs @@ -135,10 +135,7 @@ impl Computation for Bits { let eek_bit = calculate_bit_width(BigInt::from(data.eek_bound.clone())); let sk_bit = calculate_bit_width(BigInt::from(data.sk_bound.clone())); let e_sm_bit = calculate_bit_width(BigInt::from(data.e_sm_bound.clone())); - - // pk_bit: iterate over moduli (aligned with C2 share_computation approach). - // Uses qi - 1 (full modulus range) rather than (qi - 1) / 2, which naturally - // gives one extra bit for the centered (signed) representation. + let (threshold_params, _) = build_pair_for_preset(preset).map_err(|e| CircuitsErrors::Other(e.to_string()))?; let mut pk_bit = 0; From 3d23146e8118487b223c2a0a10b80ddb2abbd512 Mon Sep 17 00:00:00 2001 From: ctrlc03 <93448202+ctrlc03@users.noreply.github.com> Date: Tue, 24 Mar 2026 14:28:14 +0000 Subject: [PATCH 04/11] chore: throw error in unwrap signal --- crates/aggregator/src/publickey_aggregator.rs | 22 ++++++++++++------- .../threshold/pk_generation/computation.rs | 2 +- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/crates/aggregator/src/publickey_aggregator.rs b/crates/aggregator/src/publickey_aggregator.rs index 2ee17631e8..3d8c45bbab 100644 --- a/crates/aggregator/src/publickey_aggregator.rs +++ b/crates/aggregator/src/publickey_aggregator.rs @@ -41,14 +41,20 @@ fn derive_c1_commitments(signed_proofs: &[Option]) -> Vec>` to `Vec`, substituting 32 zero bytes -/// for any `None` entry. The C5 prover will detect these as commitment mismatches. -fn unwrap_c1_commitments(commitments: &[Option]) -> Vec { +/// Convert `Vec>` to `Vec`, failing if any entry is `None`. +/// A `None` means pk_commitment extraction failed for that party's C1 proof — +/// this is a bug or a corrupted proof, not a normal mismatch. +fn unwrap_c1_commitments(commitments: &[Option]) -> Result> { commitments .iter() - .map(|opt| { - opt.clone() - .unwrap_or_else(|| ArcBytes::from_bytes(&[0u8; 32])) + .enumerate() + .map(|(i, opt)| { + opt.clone().ok_or_else(|| { + anyhow::anyhow!( + "Failed to extract pk_commitment from C1 proof at index {}", + i + ) + }) }) .collect() } @@ -382,7 +388,7 @@ impl PublicKeyAggregator { committee_threshold: 0, c1_commitments: unwrap_c1_commitments(&derive_c1_commitments( &c1_signed_proofs, - )), + ))?, }, public_key: pubkey.clone(), nodes: honest_nodes_set.clone(), @@ -924,7 +930,7 @@ impl PublicKeyAggregator { committee_threshold: threshold_m, c1_commitments: unwrap_c1_commitments(&derive_c1_commitments( &remaining_c1_signed_proofs, - )), + ))?, }, public_key: pubkey.clone(), nodes: remaining_nodes.clone(), diff --git a/crates/zk-helpers/src/circuits/threshold/pk_generation/computation.rs b/crates/zk-helpers/src/circuits/threshold/pk_generation/computation.rs index 151727d2c4..0e2d7f55d4 100644 --- a/crates/zk-helpers/src/circuits/threshold/pk_generation/computation.rs +++ b/crates/zk-helpers/src/circuits/threshold/pk_generation/computation.rs @@ -135,7 +135,7 @@ impl Computation for Bits { let eek_bit = calculate_bit_width(BigInt::from(data.eek_bound.clone())); let sk_bit = calculate_bit_width(BigInt::from(data.sk_bound.clone())); let e_sm_bit = calculate_bit_width(BigInt::from(data.e_sm_bound.clone())); - + let (threshold_params, _) = build_pair_for_preset(preset).map_err(|e| CircuitsErrors::Other(e.to_string()))?; let mut pk_bit = 0; From b84a8e327911b0496b9f12d29217441864380fbe Mon Sep 17 00:00:00 2001 From: ctrlc03 <93448202+ctrlc03@users.noreply.github.com> Date: Wed, 25 Mar 2026 09:26:17 +0000 Subject: [PATCH 05/11] chore: pr comments --- crates/aggregator/src/publickey_aggregator.rs | 146 ++---------------- crates/multithread/src/multithread.rs | 12 -- 2 files changed, 9 insertions(+), 149 deletions(-) diff --git a/crates/aggregator/src/publickey_aggregator.rs b/crates/aggregator/src/publickey_aggregator.rs index 3d8c45bbab..a7bd8c5aed 100644 --- a/crates/aggregator/src/publickey_aggregator.rs +++ b/crates/aggregator/src/publickey_aggregator.rs @@ -399,7 +399,7 @@ impl PublicKeyAggregator { self.state.try_mutate(&ec, |_| { Ok(PublicKeyAggregatorState::GeneratingC5Proof { public_key: pubkey.clone(), - keyshare_bytes: honest_keyshares, + keyshare_bytes, c1_signed_proofs, nodes: honest_nodes_set, dkg_node_proofs: HashMap::new(), @@ -786,13 +786,8 @@ impl PublicKeyAggregator { ec: EventContext, ) -> Result<()> { let PublicKeyAggregatorState::GeneratingC5Proof { - keyshare_bytes, c1_signed_proofs: stored_c1_signed_proofs, - nodes, - dkg_node_proofs, honest_party_ids, - dishonest_parties, - threshold_m, .. } = self .state @@ -804,34 +799,17 @@ impl PublicKeyAggregator { )); }; - // Map keyshare-order indices to original party IDs. - // keyshare_bytes[i] corresponds to the i-th element in honest_party_ids (sorted). + // Map keyshare-order indices to party IDs for logging let honest_ids_sorted: Vec = honest_party_ids.iter().copied().collect(); - let mut newly_dishonest: BTreeSet = BTreeSet::new(); - for &idx in mismatched_indices { - if let Some(&party_id) = honest_ids_sorted.get(idx) { - warn!( - "C1 commitment mismatch for party {} (index {}) — marking as dishonest", - party_id, idx - ); - newly_dishonest.insert(party_id); - } else { - warn!( - "C1 commitment mismatch index {} out of range (honest parties: {})", - idx, - honest_ids_sorted.len() - ); - } - } - - if newly_dishonest.is_empty() { - return Err(anyhow::anyhow!( - "C1 commitment mismatch reported but no valid party indices" - )); - } - // Emit SignedProofFailed for each mismatched party that has a signed C1 proof + // Emit SignedProofFailed for each mismatched party for &idx in mismatched_indices { + let party_id = honest_ids_sorted.get(idx).copied().unwrap_or(u64::MAX); + warn!( + "C1 commitment mismatch for party {} (index {}) — reporting fault", + party_id, idx + ); + if let Some(Some(signed_payload)) = stored_c1_signed_proofs.get(idx) { match signed_payload.recover_address() { Ok(faulting_node) => { @@ -857,112 +835,6 @@ impl PublicKeyAggregator { } } - // Filter out the newly dishonest parties - let remaining_keyshares: Vec = keyshare_bytes - .iter() - .enumerate() - .filter(|(i, _)| !mismatched_indices.contains(i)) - .map(|(_, ks)| ks.clone()) - .collect(); - - let remaining_ids: BTreeSet = honest_party_ids - .iter() - .copied() - .filter(|id| !newly_dishonest.contains(id)) - .collect(); - - let remaining_nodes: OrderedSet = { - let nodes_vec: Vec = nodes - .iter() - .enumerate() - .filter(|(i, _)| !mismatched_indices.contains(i)) - .map(|(_, n)| n.clone()) - .collect(); - OrderedSet::from(nodes_vec) - }; - - let mut all_dishonest = dishonest_parties.clone(); - all_dishonest.extend(newly_dishonest.iter()); - - // Check if enough honest parties remain - let remaining_count = remaining_keyshares.len(); - if remaining_count <= threshold_m { - return Err(anyhow::anyhow!( - "Not enough honest parties after C1 commitment mismatch filtering: {} (need at least {})", - remaining_count, - threshold_m + 1 - )); - } - - info!( - "Re-aggregating public key from {} remaining honest parties (removed {} dishonest)", - remaining_count, - newly_dishonest.len() - ); - - // Re-aggregate the public key without the dishonest parties - let remaining_keyshares_set = OrderedSet::from(remaining_keyshares.clone()); - let pubkey = self.fhe.get_aggregate_public_key(GetAggregatePublicKey { - keyshares: remaining_keyshares_set, - })?; - - let committee_h = remaining_count; - let pubkey = ArcBytes::from_bytes(&pubkey); - - // Filter c1_signed_proofs to match remaining honest parties - let remaining_c1_signed_proofs: Vec> = stored_c1_signed_proofs - .iter() - .enumerate() - .filter(|(i, _)| !mismatched_indices.contains(i)) - .map(|(_, sp)| sp.clone()) - .collect(); - - // Publish new PkAggregationProofPending - self.bus.publish( - PkAggregationProofPending { - e3_id: self.e3_id.clone(), - proof_request: PkAggregationProofRequest { - keyshare_bytes: remaining_keyshares.clone(), - aggregated_pk_bytes: pubkey.clone(), - params_preset: self.params_preset.clone(), - committee_n: committee_h, - committee_h, - committee_threshold: threshold_m, - c1_commitments: unwrap_c1_commitments(&derive_c1_commitments( - &remaining_c1_signed_proofs, - ))?, - }, - public_key: pubkey.clone(), - nodes: remaining_nodes.clone(), - }, - ec.clone(), - )?; - - // Keep DKG proofs from remaining honest parties — they won't be re-delivered. - let remaining_dkg_proofs: HashMap> = dkg_node_proofs - .into_iter() - .filter(|(pid, _)| remaining_ids.contains(pid)) - .collect(); - - // Transition state: reset fold but preserve honest DKG proofs - self.state.try_mutate(&ec, |_| { - Ok(PublicKeyAggregatorState::GeneratingC5Proof { - public_key: pubkey.clone(), - keyshare_bytes: remaining_keyshares, - c1_signed_proofs: remaining_c1_signed_proofs, - nodes: remaining_nodes, - dkg_node_proofs: remaining_dkg_proofs, - honest_party_ids: remaining_ids, - dishonest_parties: all_dishonest, - cross_node_fold: ProofFoldState::new(), - c5_proof_pending: None, - last_ec: Some(ec.clone()), - threshold_m, - }) - })?; - - self.try_start_cross_node_fold(&ec)?; - Ok(()) } diff --git a/crates/multithread/src/multithread.rs b/crates/multithread/src/multithread.rs index b8b1a630ae..6ffd4de716 100644 --- a/crates/multithread/src/multithread.rs +++ b/crates/multithread/src/multithread.rs @@ -380,18 +380,6 @@ fn handle_pk_aggregation_proof( let mut padded = vec![0u8; 32]; let start = 32_usize.saturating_sub(be_bytes.len()); padded[start..].copy_from_slice(&be_bytes); - info!( - "C1 commitment check party {}: computed={:?} extracted={:?}", - i, - padded - .iter() - .map(|b| format!("{:02x}", b)) - .collect::(), - extracted - .iter() - .map(|b| format!("{:02x}", b)) - .collect::() - ); if padded[..] != extracted[..] { mismatched_indices.push(i); } From 765422dcfe181a280ce0bc4f912178db20babe6c Mon Sep 17 00:00:00 2001 From: ctrlc03 <93448202+ctrlc03@users.noreply.github.com> Date: Wed, 25 Mar 2026 10:39:22 +0000 Subject: [PATCH 06/11] chore: pr comments --- crates/aggregator/src/publickey_aggregator.rs | 32 +++-- crates/multithread/src/multithread.rs | 10 +- crates/tests/tests/integration.rs | 132 ++++++++++++++++++ 3 files changed, 160 insertions(+), 14 deletions(-) diff --git a/crates/aggregator/src/publickey_aggregator.rs b/crates/aggregator/src/publickey_aggregator.rs index a7bd8c5aed..a376766982 100644 --- a/crates/aggregator/src/publickey_aggregator.rs +++ b/crates/aggregator/src/publickey_aggregator.rs @@ -99,8 +99,6 @@ pub enum PublicKeyAggregatorState { cross_node_fold: ProofFoldState, c5_proof_pending: Option, last_ec: Option>, - /// Cryptographic threshold M, carried from Collecting for re-aggregation checks. - threshold_m: usize, }, Complete { public_key: ArcBytes, @@ -408,7 +406,6 @@ impl PublicKeyAggregator { cross_node_fold: ProofFoldState::new(), c5_proof_pending: None, last_ec: Some(ec.clone()), - threshold_m, }) })?; @@ -455,7 +452,6 @@ impl PublicKeyAggregator { honest_party_ids, dishonest_parties, cross_node_fold, - threshold_m, .. } = state else { @@ -472,7 +468,6 @@ impl PublicKeyAggregator { cross_node_fold, c5_proof_pending: Some(c5_proof), last_ec: Some(ec.clone()), - threshold_m, }) })?; self.try_publish_complete() @@ -528,7 +523,6 @@ impl PublicKeyAggregator { cross_node_fold, c5_proof_pending, last_ec: _, - threshold_m, } = state else { return Ok(state); @@ -545,7 +539,6 @@ impl PublicKeyAggregator { cross_node_fold, c5_proof_pending, last_ec: Some(ec.clone()), - threshold_m, }) })?; @@ -604,7 +597,6 @@ impl PublicKeyAggregator { mut cross_node_fold, c5_proof_pending, last_ec, - threshold_m, } = state else { return Ok(state); @@ -631,7 +623,6 @@ impl PublicKeyAggregator { cross_node_fold, c5_proof_pending, last_ec, - threshold_m, }) })?; self.try_publish_complete() @@ -748,7 +739,6 @@ impl PublicKeyAggregator { mut cross_node_fold, c5_proof_pending, last_ec, - threshold_m, } = state else { return Ok(state); @@ -772,7 +762,6 @@ impl PublicKeyAggregator { cross_node_fold, c5_proof_pending, last_ec, - threshold_m, }) })?; self.try_publish_complete()?; @@ -1092,3 +1081,24 @@ impl Handler for PublicKeyAggregator { ctx.stop(); } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn unwrap_c1_commitments_succeeds_when_all_present() { + let commitments = vec![ + Some(ArcBytes::from_bytes(&[0x11; 32])), + Some(ArcBytes::from_bytes(&[0x22; 32])), + ]; + assert_eq!(unwrap_c1_commitments(&commitments).unwrap().len(), 2); + } + + #[test] + fn unwrap_c1_commitments_fails_on_missing_entry() { + let commitments = vec![Some(ArcBytes::from_bytes(&[0x11; 32])), None]; + let err = unwrap_c1_commitments(&commitments).unwrap_err(); + assert!(err.to_string().contains("index 1")); + } +} diff --git a/crates/multithread/src/multithread.rs b/crates/multithread/src/multithread.rs index 6ffd4de716..946e671fc5 100644 --- a/crates/multithread/src/multithread.rs +++ b/crates/multithread/src/multithread.rs @@ -377,9 +377,13 @@ fn handle_pk_aggregation_proof( let mut mismatched_indices = Vec::new(); for (i, (computed, extracted)) in expected.iter().zip(req.c1_commitments.iter()).enumerate() { let (_, be_bytes) = computed.to_bytes_be(); - let mut padded = vec![0u8; 32]; - let start = 32_usize.saturating_sub(be_bytes.len()); - padded[start..].copy_from_slice(&be_bytes); + if be_bytes.len() > 32 { + // Commitment exceeds field size — treat as mismatch + mismatched_indices.push(i); + continue; + } + let mut padded = [0u8; 32]; + padded[32 - be_bytes.len()..].copy_from_slice(&be_bytes); if padded[..] != extracted[..] { mismatched_indices.push(i); } diff --git a/crates/tests/tests/integration.rs b/crates/tests/tests/integration.rs index ae7e495501..bcaf2bce12 100644 --- a/crates/tests/tests/integration.rs +++ b/crates/tests/tests/integration.rs @@ -555,6 +555,138 @@ impl Report { } } +/// Test C1→C5 commitment mismatch detection. +/// +/// Verifies the full flow: when the C5 prover detects that C1 pk_commitments +/// don't match the computed commitments from keyshare data, it returns a +/// C1CommitmentMismatch error, which causes the aggregator to emit +/// SignedProofFailed for the faulting parties and fail the E3. +#[actix::test] +#[serial_test::serial] +async fn test_c1_c5_commitment_mismatch() -> Result<()> { + use e3_events::{CircuitName, GetEvents, ProofPayload, ProofType, SignedProofPayload}; + use e3_utils::utility_types::ArcBytes; + + let _guard = with_tracing("info"); + let (bus, _rng, _seed, _params, _crp, _errors, history) = + e3_test_helpers::get_common_setup(None)?; + + let e3_id = E3id::new("99", 1); + + // Create mock C1 proofs with known pk_commitments + let make_c1_proof = |pk: &[u8; 32]| { + let mut signals = vec![0u8; 96]; + signals[0..32].copy_from_slice(&[0xAA; 32]); // sk_commitment + signals[32..64].copy_from_slice(pk); // pk_commitment + signals[64..96].copy_from_slice(&[0xCC; 32]); // e_sm_commitment + e3_events::Proof::new( + CircuitName::PkGeneration, + ArcBytes::from_bytes(&[0u8; 8]), + ArcBytes::from_bytes(&signals), + ) + }; + + let sign_proof = |proof: e3_events::Proof| -> SignedProofPayload { + let signer = PrivateKeySigner::random(); + let payload = ProofPayload { + e3_id: e3_id.clone(), + proof_type: ProofType::C1PkGeneration, + proof, + }; + SignedProofPayload::sign(payload, &signer).unwrap() + }; + + // Verify extraction works: derive pk_commitments from signed C1 proofs + let c1_proofs = vec![ + Some(sign_proof(make_c1_proof(&[0x11; 32]))), + Some(sign_proof(make_c1_proof(&[0x22; 32]))), + Some(sign_proof(make_c1_proof(&[0x33; 32]))), + ]; + + // Extract pk_commitments — should match what we put in + for (i, proof_opt) in c1_proofs.iter().enumerate() { + let proof = &proof_opt.as_ref().unwrap().payload.proof; + let extracted = proof.extract_output("pk_commitment"); + assert!( + extracted.is_some(), + "Failed to extract pk_commitment from C1 proof[{}]", + i + ); + } + + // Verify that mismatched commitments are detectable: + // If C5 prover computes commitment X for party 0, but C1 proof has commitment Y, + // the comparison fails. + let c1_commitment_from_proof = c1_proofs[0] + .as_ref() + .unwrap() + .payload + .proof + .extract_output("pk_commitment") + .unwrap(); + let wrong_commitment = ArcBytes::from_bytes(&[0xFF; 32]); + assert_ne!( + c1_commitment_from_proof[..], + wrong_commitment[..], + "Sanity check: commitments should differ" + ); + + // Verify the ComputeRequestError with C1CommitmentMismatch can be constructed + // and carries the correct indices + let error = e3_events::ComputeRequestError::new( + e3_events::ComputeRequestErrorKind::Zk(e3_events::ZkError::C1CommitmentMismatch { + mismatched_indices: vec![0, 2], + }), + e3_events::ComputeRequest::zk( + e3_events::ZkRequest::PkAggregation(e3_events::PkAggregationProofRequest { + keyshare_bytes: vec![], + aggregated_pk_bytes: ArcBytes::from_bytes(&[]), + params_preset: e3_fhe_params::BfvPreset::InsecureThreshold512, + committee_n: 3, + committee_h: 3, + committee_threshold: 1, + c1_commitments: vec![], + }), + e3_events::CorrelationId::new(), + e3_id.clone(), + ), + ); + + // Publish the error on the bus + bus.publish_without_context(error.clone())?; + + // Give actix time to process + tokio::time::sleep(std::time::Duration::from_millis(200)).await; + + // The error should be visible in history as ComputeRequestError + let events = history.send(GetEvents::new()).await?; + let error_events: Vec<_> = events + .iter() + .filter(|e| e.event_type() == "ComputeRequestError") + .collect(); + assert!( + !error_events.is_empty(), + "Expected ComputeRequestError in history, got: {:?}", + events.iter().map(|e| e.event_type()).collect::>() + ); + + // Verify the error kind is C1CommitmentMismatch + if let EnclaveEventData::ComputeRequestError(ref err) = error_events[0].get_data() { + match err.get_err() { + e3_events::ComputeRequestErrorKind::Zk(e3_events::ZkError::C1CommitmentMismatch { + ref mismatched_indices, + }) => { + assert_eq!(mismatched_indices, &[0, 2]); + } + other => bail!("Expected C1CommitmentMismatch, got: {:?}", other), + } + } else { + bail!("Expected ComputeRequestError event data"); + } + + Ok(()) +} + /// Test trbfv #[actix::test] #[serial_test::serial] From efe076da33a13cf8b17841f97c625dd9c7acb1fe Mon Sep 17 00:00:00 2001 From: ctrlc03 <93448202+ctrlc03@users.noreply.github.com> Date: Wed, 25 Mar 2026 16:56:03 +0000 Subject: [PATCH 07/11] refactor: simplify and ensure we retry aggregation --- crates/aggregator/src/publickey_aggregator.rs | 268 ++++++++--------- .../src/enclave_event/compute_request/zk.rs | 18 +- crates/multithread/src/multithread.rs | 47 ++- crates/tests/tests/integration.rs | 274 ++++++++++++------ .../threshold/pk_generation/computation.rs | 16 +- crates/zk-prover/src/actors/proof_request.rs | 78 ++++- 6 files changed, 410 insertions(+), 291 deletions(-) diff --git a/crates/aggregator/src/publickey_aggregator.rs b/crates/aggregator/src/publickey_aggregator.rs index a376766982..05509cfd07 100644 --- a/crates/aggregator/src/publickey_aggregator.rs +++ b/crates/aggregator/src/publickey_aggregator.rs @@ -9,12 +9,12 @@ use actix::prelude::*; use anyhow::Result; use e3_data::Persistable; use e3_events::{ - prelude::*, BusHandle, ComputeRequestErrorKind, ComputeResponse, ComputeResponseKind, - DKGRecursiveAggregationComplete, Die, E3id, EnclaveEvent, EnclaveEventData, EventContext, - KeyshareCreated, OrderedSet, PartyProofsToVerify, PkAggregationProofPending, - PkAggregationProofRequest, PkAggregationProofSigned, Proof, ProofType, PublicKeyAggregated, - Seed, Sequenced, ShareVerificationComplete, ShareVerificationDispatched, SignedProofFailed, - SignedProofPayload, TypedEvent, VerificationKind, ZkError, ZkResponse, + prelude::*, BusHandle, ComputeRequestError, ComputeRequestErrorKind, ComputeRequestKind, + ComputeResponse, ComputeResponseKind, DKGRecursiveAggregationComplete, Die, E3id, EnclaveEvent, + EnclaveEventData, EventContext, KeyshareCreated, OrderedSet, PartyProofsToVerify, + PkAggregationProofPending, PkAggregationProofRequest, PkAggregationProofSigned, Proof, + PublicKeyAggregated, Seed, Sequenced, ShareVerificationComplete, ShareVerificationDispatched, + SignedProofPayload, TypedEvent, VerificationKind, ZkError, ZkRequest, ZkResponse, }; use e3_events::{trap, EType}; use e3_fhe::{Fhe, GetAggregatePublicKey}; @@ -25,40 +25,6 @@ use std::collections::{BTreeSet, HashMap}; use std::sync::Arc; use tracing::{error, info, warn}; -/// Derive c1_commitments from signed proofs by extracting pk_commitment from each. -fn derive_c1_commitments(signed_proofs: &[Option]) -> Vec> { - signed_proofs - .iter() - .enumerate() - .map(|(i, opt)| { - let sp = opt.as_ref()?; - let commitment = sp.payload.proof.extract_output("pk_commitment"); - if commitment.is_none() { - tracing::warn!("C1 proof[{}]: failed to extract pk_commitment", i); - } - commitment - }) - .collect() -} - -/// Convert `Vec>` to `Vec`, failing if any entry is `None`. -/// A `None` means pk_commitment extraction failed for that party's C1 proof — -/// this is a bug or a corrupted proof, not a normal mismatch. -fn unwrap_c1_commitments(commitments: &[Option]) -> Result> { - commitments - .iter() - .enumerate() - .map(|(i, opt)| { - opt.clone().ok_or_else(|| { - anyhow::anyhow!( - "Failed to extract pk_commitment from C1 proof at index {}", - i - ) - }) - }) - .collect() -} - #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] pub enum PublicKeyAggregatorState { Collecting { @@ -87,10 +53,6 @@ pub enum PublicKeyAggregatorState { GeneratingC5Proof { public_key: ArcBytes, keyshare_bytes: Vec, - /// Signed C1 proofs from honest parties, aligned with `keyshare_bytes`. - /// Commitments are extracted on the fly via `extract_output("pk_commitment")`. - /// Retained for fault attribution if a commitment mismatch is detected later. - c1_signed_proofs: Vec>, nodes: OrderedSet, /// DKG recursive proofs per party (restart-critical). dkg_node_proofs: HashMap>, @@ -312,12 +274,22 @@ impl PublicKeyAggregator { .map(|(_, (node, ks))| (ks.clone(), node.clone())) .unzip(); - // Collect signed C1 proofs from honest parties (commitments derived on the fly) - let c1_signed_proofs: Vec> = honest_entries + // Collect signed C1 proofs from honest parties (submission order). + // All honest parties should have a signed C1 proof (parties without + // proofs are marked dishonest in dispatch_c1_verification). + let c1_signed_proofs_submission_order: Vec = honest_entries .iter() - .map(|(idx, _)| c1_proofs.get(*idx).and_then(|opt| opt.clone())) + .filter_map(|(idx, _)| c1_proofs.get(*idx).and_then(|opt| opt.clone())) .collect(); + if c1_signed_proofs_submission_order.len() != honest_keyshares.len() { + return Err(anyhow::anyhow!( + "C1 proof count ({}) != honest keyshare count ({}) — data misalignment", + c1_signed_proofs_submission_order.len(), + honest_keyshares.len() + )); + } + if !dishonest_parties.is_empty() { warn!( "Filtered out {} dishonest parties from C1 verification: {:?}", @@ -352,26 +324,21 @@ impl PublicKeyAggregator { let committee_h = honest_keyshares.len(); let honest_nodes_set = OrderedSet::from(honest_nodes); // keyshare_bytes follows OrderedSet ordering (used by C5 prover). - // Re-align c1_signed_proofs to the same order so c1_commitments[i] - // corresponds to keyshare_bytes[i]. + // Re-align c1_signed_proofs to the same order. let keyshare_bytes: Vec<_> = honest_keyshares_set.iter().cloned().collect(); - let c1_signed_proofs = { - // Build a map from keyshare → c1 proof, then iterate in OrderedSet order. - let ks_to_proof: std::collections::HashMap, &Option> = + let c1_signed_proofs: Vec = { + let ks_to_proof: std::collections::HashMap, &SignedProofPayload> = honest_keyshares .iter() - .zip(c1_signed_proofs.iter()) - .map(|(ks, proof)| (ks.to_vec(), proof)) + .zip(c1_signed_proofs_submission_order.iter()) + .map(|(ks, sp)| (ks.to_vec(), sp)) .collect(); keyshare_bytes .iter() - .map(|ks| ks_to_proof.get(&ks.to_vec()).and_then(|opt| (*opt).clone())) - .collect::>() + .filter_map(|ks| ks_to_proof.get(&ks.to_vec()).map(|sp| (*sp).clone())) + .collect() }; - // Publish pending event before transitioning state so a publish - // failure leaves us in VerifyingC1 (retryable) rather than - // GeneratingC5Proof (no retry path). let pubkey = ArcBytes::from_bytes(&pubkey); info!("Publishing PkAggregationProofPending for C5 proof generation..."); self.bus.publish( @@ -384,9 +351,7 @@ impl PublicKeyAggregator { committee_n: committee_h, committee_h, committee_threshold: 0, - c1_commitments: unwrap_c1_commitments(&derive_c1_commitments( - &c1_signed_proofs, - ))?, + c1_signed_proofs, }, public_key: pubkey.clone(), nodes: honest_nodes_set.clone(), @@ -398,7 +363,6 @@ impl PublicKeyAggregator { Ok(PublicKeyAggregatorState::GeneratingC5Proof { public_key: pubkey.clone(), keyshare_bytes, - c1_signed_proofs, nodes: honest_nodes_set, dkg_node_proofs: HashMap::new(), honest_party_ids: honest_party_ids.clone(), @@ -446,7 +410,6 @@ impl PublicKeyAggregator { let PublicKeyAggregatorState::GeneratingC5Proof { public_key, keyshare_bytes, - c1_signed_proofs, nodes, dkg_node_proofs, honest_party_ids, @@ -460,7 +423,6 @@ impl PublicKeyAggregator { Ok(PublicKeyAggregatorState::GeneratingC5Proof { public_key, keyshare_bytes, - c1_signed_proofs, nodes, dkg_node_proofs, honest_party_ids, @@ -515,7 +477,6 @@ impl PublicKeyAggregator { let PublicKeyAggregatorState::GeneratingC5Proof { public_key, keyshare_bytes, - c1_signed_proofs, nodes, mut dkg_node_proofs, honest_party_ids, @@ -531,7 +492,6 @@ impl PublicKeyAggregator { Ok(PublicKeyAggregatorState::GeneratingC5Proof { public_key, keyshare_bytes, - c1_signed_proofs, nodes, dkg_node_proofs, honest_party_ids, @@ -589,7 +549,6 @@ impl PublicKeyAggregator { let PublicKeyAggregatorState::GeneratingC5Proof { public_key, keyshare_bytes, - c1_signed_proofs, nodes, dkg_node_proofs, honest_party_ids, @@ -615,7 +574,6 @@ impl PublicKeyAggregator { Ok(PublicKeyAggregatorState::GeneratingC5Proof { public_key, keyshare_bytes, - c1_signed_proofs, nodes, dkg_node_proofs, honest_party_ids, @@ -731,7 +689,6 @@ impl PublicKeyAggregator { let PublicKeyAggregatorState::GeneratingC5Proof { public_key, keyshare_bytes, - c1_signed_proofs, nodes, dkg_node_proofs, honest_party_ids, @@ -754,7 +711,6 @@ impl PublicKeyAggregator { Ok(PublicKeyAggregatorState::GeneratingC5Proof { public_key, keyshare_bytes, - c1_signed_proofs, nodes, dkg_node_proofs, honest_party_ids, @@ -769,61 +725,106 @@ impl PublicKeyAggregator { Ok(()) } - fn handle_c1_commitment_mismatch( + /// Handle C5 proof error. On C1CommitmentMismatch, re-aggregate without + /// the faulting parties and re-dispatch C5. On other errors, just log. + fn handle_c5_error( &mut self, - mismatched_indices: &[usize], + error: ComputeRequestError, ec: EventContext, ) -> Result<()> { - let PublicKeyAggregatorState::GeneratingC5Proof { - c1_signed_proofs: stored_c1_signed_proofs, - honest_party_ids, - .. - } = self - .state - .get() - .ok_or_else(|| anyhow::anyhow!("Expected GeneratingC5Proof state"))? + let ComputeRequestErrorKind::Zk(ZkError::C1CommitmentMismatch { + ref mismatched_indices, + }) = error.get_err() else { - return Err(anyhow::anyhow!( - "handle_c1_commitment_mismatch called outside GeneratingC5Proof state" - )); + error!( + "PublicKeyAggregator received ComputeRequestError: {}", + error + ); + return Ok(()); }; - // Map keyshare-order indices to party IDs for logging - let honest_ids_sorted: Vec = honest_party_ids.iter().copied().collect(); + // Extract the original request from the error + let pk_req = match &error.request().request { + ComputeRequestKind::Zk(ZkRequest::PkAggregation(req)) => req.clone(), + _ => { + error!("C1CommitmentMismatch error with non-PkAggregation request"); + return Ok(()); + } + }; - // Emit SignedProofFailed for each mismatched party - for &idx in mismatched_indices { - let party_id = honest_ids_sorted.get(idx).copied().unwrap_or(u64::MAX); - warn!( - "C1 commitment mismatch for party {} (index {}) — reporting fault", - party_id, idx - ); + warn!( + "C1 commitment mismatch at indices {:?} — re-aggregating without faulting parties", + mismatched_indices + ); - if let Some(Some(signed_payload)) = stored_c1_signed_proofs.get(idx) { - match signed_payload.recover_address() { - Ok(faulting_node) => { - if let Err(err) = self.bus.publish( - SignedProofFailed { - e3_id: self.e3_id.clone(), - faulting_node, - proof_type: ProofType::C1PkGeneration, - signed_payload: signed_payload.clone(), - }, - ec.clone(), - ) { - error!("Failed to publish SignedProofFailed for C1 mismatch at index {}: {err}", idx); - } - } - Err(err) => { - warn!( - "Could not recover address from C1 signed proof at index {}: {err}", - idx - ); - } - } - } + // Filter out mismatched parties + let remaining_keyshares: Vec = pk_req + .keyshare_bytes + .iter() + .enumerate() + .filter(|(i, _)| !mismatched_indices.contains(i)) + .map(|(_, ks)| ks.clone()) + .collect(); + let remaining_c1_proofs: Vec = pk_req + .c1_signed_proofs + .iter() + .enumerate() + .filter(|(i, _)| !mismatched_indices.contains(i)) + .map(|(_, sp)| sp.clone()) + .collect(); + + if remaining_keyshares.len() <= pk_req.committee_threshold { + error!( + "Not enough honest parties after C1 commitment mismatch filtering: {} (need > {})", + remaining_keyshares.len(), + pk_req.committee_threshold + ); + self.bus.publish( + e3_events::E3Failed { + e3_id: self.e3_id.clone(), + failed_at_stage: e3_events::E3Stage::CommitteeFinalized, + reason: e3_events::FailureReason::DKGInvalidShares, + }, + ec, + )?; + return Ok(()); } + // Re-aggregate the public key from remaining honest keyshares + let remaining_set = OrderedSet::from(remaining_keyshares.clone()); + let pubkey = self.fhe.get_aggregate_public_key(GetAggregatePublicKey { + keyshares: remaining_set, + })?; + + let committee_h = remaining_keyshares.len(); + let pubkey = ArcBytes::from_bytes(&pubkey); + + // Re-dispatch C5 with the filtered data + self.bus.publish( + PkAggregationProofPending { + e3_id: self.e3_id.clone(), + proof_request: PkAggregationProofRequest { + keyshare_bytes: remaining_keyshares, + aggregated_pk_bytes: pubkey.clone(), + params_preset: self.params_preset.clone(), + committee_n: committee_h, + committee_h, + committee_threshold: 0, + c1_signed_proofs: remaining_c1_proofs, + }, + public_key: pubkey, + nodes: self + .state + .get() + .map(|s| match s { + PublicKeyAggregatorState::GeneratingC5Proof { nodes, .. } => nodes, + _ => OrderedSet::new(), + }) + .unwrap_or_default(), + }, + ec, + )?; + Ok(()) } @@ -921,17 +922,9 @@ impl Handler for PublicKeyAggregator { if data.request().e3_id != self.e3_id { return; } - if let ComputeRequestErrorKind::Zk(ZkError::C1CommitmentMismatch { - ref mismatched_indices, - }) = data.get_err() - { - let indices = mismatched_indices.clone(); - trap(EType::PublickeyAggregation, &self.bus.with_ec(&ec), || { - self.handle_c1_commitment_mismatch(&indices, ec.clone()) - }); - } else { - error!("PublicKeyAggregator received ComputeRequestError: {}", data); - } + trap(EType::PublickeyAggregation, &self.bus.with_ec(&ec), || { + self.handle_c5_error(data, ec.clone()) + }); } EnclaveEventData::E3RequestComplete(_) => self.notify_sync(ctx, Die), EnclaveEventData::CommitteeMemberExpelled(data) => { @@ -1081,24 +1074,3 @@ impl Handler for PublicKeyAggregator { ctx.stop(); } } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn unwrap_c1_commitments_succeeds_when_all_present() { - let commitments = vec![ - Some(ArcBytes::from_bytes(&[0x11; 32])), - Some(ArcBytes::from_bytes(&[0x22; 32])), - ]; - assert_eq!(unwrap_c1_commitments(&commitments).unwrap().len(), 2); - } - - #[test] - fn unwrap_c1_commitments_fails_on_missing_entry() { - let commitments = vec![Some(ArcBytes::from_bytes(&[0x11; 32])), None]; - let err = unwrap_c1_commitments(&commitments).unwrap_err(); - assert!(err.to_string().contains("index 1")); - } -} diff --git a/crates/events/src/enclave_event/compute_request/zk.rs b/crates/events/src/enclave_event/compute_request/zk.rs index 57f451592b..d954662d18 100644 --- a/crates/events/src/enclave_event/compute_request/zk.rs +++ b/crates/events/src/enclave_event/compute_request/zk.rs @@ -61,8 +61,10 @@ pub struct PkAggregationProofRequest { pub committee_h: usize, /// Threshold (T). pub committee_threshold: usize, - /// C1 commitments extracted from honest parties' signed C1 proofs. - pub c1_commitments: Vec, + /// Signed C1 proofs per party, aligned with `keyshare_bytes`. + /// The C5 prover extracts pk_commitment from each proof for cross-checking, + /// and returns mismatched indices for fault attribution. + pub c1_signed_proofs: Vec, } /// Request to generate a proof for share computation (C2a or C2b). @@ -448,12 +450,10 @@ pub enum ZkError { WitnessGenerationFailed(String), /// Invalid parameters. InvalidParams(String), - /// C1 commitment mismatch: the commitment extracted from a party's C1 proof - /// does not match the commitment computed from their keyshare data. - C1CommitmentMismatch { - /// Indices (into `keyshare_bytes` / `c1_commitments`) of mismatched parties. - mismatched_indices: Vec, - }, + /// C1 commitment mismatch: not enough honest parties remain after filtering. + /// The mismatched indices identify which parties' C1 proofs are inconsistent + /// with their keyshare data. + C1CommitmentMismatch { mismatched_indices: Vec }, } impl std::fmt::Display for ZkError { @@ -466,7 +466,7 @@ impl std::fmt::Display for ZkError { ZkError::InvalidParams(msg) => write!(f, "Invalid parameters: {}", msg), ZkError::C1CommitmentMismatch { mismatched_indices } => write!( f, - "C1 commitment mismatch at indices: {:?}", + "C1 commitment mismatch at indices {:?} — not enough honest parties", mismatched_indices ), } diff --git a/crates/multithread/src/multithread.rs b/crates/multithread/src/multithread.rs index 946e671fc5..e6d3277706 100644 --- a/crates/multithread/src/multithread.rs +++ b/crates/multithread/src/multithread.rs @@ -83,7 +83,7 @@ use ndarray::Array2; use num_bigint::BigInt; use rand::rngs::OsRng; use rand::Rng; -use tracing::{error, info}; +use tracing::{error, info, warn}; /// Multithread actor pub struct Multithread { @@ -357,34 +357,33 @@ fn handle_pk_aggregation_proof( a, }; - // 6b. Verify C1 commitments from signed proofs match computed commitments - if req.c1_commitments.len() != req.committee_h { - return Err(make_zk_error( - &request, - format!( - "c1_commitments length {} != committee_h {}", - req.c1_commitments.len(), - req.committee_h - ), - )); - } - + // 6b. Verify C1 commitments match computed commitments. + // On mismatch, return C1CommitmentMismatch error with the faulting indices. + // The ProofRequestActor emits SignedProofFailed for each faulting party, + // and the PublicKeyAggregator re-aggregates without them and retries. let pk_agg_inputs = PkAggInputs::compute(req.params_preset.clone(), &circuit_data) .map_err(|e| make_zk_error(&request, format!("PkAggInputs::compute: {}", e)))?; - // public_signals from bb are big-endian 32-byte field elements let expected = &pk_agg_inputs.expected_threshold_pk_commitments; let mut mismatched_indices = Vec::new(); - for (i, (computed, extracted)) in expected.iter().zip(req.c1_commitments.iter()).enumerate() { - let (_, be_bytes) = computed.to_bytes_be(); - if be_bytes.len() > 32 { - // Commitment exceeds field size — treat as mismatch - mismatched_indices.push(i); - continue; - } - let mut padded = [0u8; 32]; - padded[32 - be_bytes.len()..].copy_from_slice(&be_bytes); - if padded[..] != extracted[..] { + for (i, sp) in req.c1_signed_proofs.iter().enumerate() { + let matches = if let Some(extracted) = sp.payload.proof.extract_output("pk_commitment") { + if let Some(computed) = expected.get(i) { + let (_, be_bytes) = computed.to_bytes_be(); + if be_bytes.len() > 32 { + false + } else { + let mut padded = [0u8; 32]; + padded[32 - be_bytes.len()..].copy_from_slice(&be_bytes); + padded[..] == extracted[..] + } + } else { + false + } + } else { + false + }; + if !matches { mismatched_indices.push(i); } } diff --git a/crates/tests/tests/integration.rs b/crates/tests/tests/integration.rs index bcaf2bce12..68a09b35f2 100644 --- a/crates/tests/tests/integration.rs +++ b/crates/tests/tests/integration.rs @@ -555,87 +555,155 @@ impl Report { } } -/// Test C1→C5 commitment mismatch detection. +// ── C1→C5 Commitment Connection Tests ──────────────────────────────────────── +// +// These tests verify the C1→C5 proof connection: ensuring the keyshare data +// used in C5 (PK aggregation) matches what each party committed to in their +// C1 (PK generation) proof. +// +// The flow: +// 1. Aggregator extracts pk_commitment from each party's signed C1 proof +// 2. Passes signed C1 proofs to the C5 prover via PkAggregationProofRequest +// 3. C5 prover computes expected commitments from keyshare data and compares +// 4. On mismatch: returns C1CommitmentMismatch error → ProofRequestActor +// emits SignedProofFailed → aggregator re-aggregates and retries +// 5. On success: proof is generated with the honest subset + +/// Helper: build a mock C1 proof with known (sk, pk, esm) commitments. +/// C1 has 3 output fields of 32 bytes each in public_signals. +fn make_c1_proof(e3_id: &E3id, pk_commitment: &[u8; 32]) -> e3_events::SignedProofPayload { + use e3_events::{CircuitName, ProofPayload, ProofType, SignedProofPayload}; + + let mut signals = vec![0u8; 96]; + signals[0..32].copy_from_slice(&[0xAA; 32]); // sk_commitment + signals[32..64].copy_from_slice(pk_commitment); // pk_commitment + signals[64..96].copy_from_slice(&[0xCC; 32]); // e_sm_commitment + let proof = e3_events::Proof::new( + CircuitName::PkGeneration, + ArcBytes::from_bytes(&[0u8; 8]), + ArcBytes::from_bytes(&signals), + ); + let signer = PrivateKeySigner::random(); + let payload = ProofPayload { + e3_id: e3_id.clone(), + proof_type: ProofType::C1PkGeneration, + proof, + }; + SignedProofPayload::sign(payload, &signer).unwrap() +} + +/// Scenario 1: All C1 commitments match — happy path. /// -/// Verifies the full flow: when the C5 prover detects that C1 pk_commitments -/// don't match the computed commitments from keyshare data, it returns a -/// C1CommitmentMismatch error, which causes the aggregator to emit -/// SignedProofFailed for the faulting parties and fail the E3. +/// When all parties' C1 pk_commitments match the computed commitments from +/// their keyshare data, the C5 proof is generated successfully. #[actix::test] #[serial_test::serial] -async fn test_c1_c5_commitment_mismatch() -> Result<()> { - use e3_events::{CircuitName, GetEvents, ProofPayload, ProofType, SignedProofPayload}; - use e3_utils::utility_types::ArcBytes; - +async fn test_c1_c5_all_commitments_match() -> Result<()> { let _guard = with_tracing("info"); - let (bus, _rng, _seed, _params, _crp, _errors, history) = - e3_test_helpers::get_common_setup(None)?; + let e3_id = E3id::new("100", 1); - let e3_id = E3id::new("99", 1); - - // Create mock C1 proofs with known pk_commitments - let make_c1_proof = |pk: &[u8; 32]| { - let mut signals = vec![0u8; 96]; - signals[0..32].copy_from_slice(&[0xAA; 32]); // sk_commitment - signals[32..64].copy_from_slice(pk); // pk_commitment - signals[64..96].copy_from_slice(&[0xCC; 32]); // e_sm_commitment - e3_events::Proof::new( - CircuitName::PkGeneration, - ArcBytes::from_bytes(&[0u8; 8]), - ArcBytes::from_bytes(&signals), - ) - }; - - let sign_proof = |proof: e3_events::Proof| -> SignedProofPayload { - let signer = PrivateKeySigner::random(); - let payload = ProofPayload { - e3_id: e3_id.clone(), - proof_type: ProofType::C1PkGeneration, - proof, - }; - SignedProofPayload::sign(payload, &signer).unwrap() - }; - - // Verify extraction works: derive pk_commitments from signed C1 proofs - let c1_proofs = vec![ - Some(sign_proof(make_c1_proof(&[0x11; 32]))), - Some(sign_proof(make_c1_proof(&[0x22; 32]))), - Some(sign_proof(make_c1_proof(&[0x33; 32]))), + // Build 3 signed C1 proofs with distinct pk_commitments + let proofs = vec![ + make_c1_proof(&e3_id, &[0x11; 32]), + make_c1_proof(&e3_id, &[0x22; 32]), + make_c1_proof(&e3_id, &[0x33; 32]), ]; - // Extract pk_commitments — should match what we put in - for (i, proof_opt) in c1_proofs.iter().enumerate() { - let proof = &proof_opt.as_ref().unwrap().payload.proof; - let extracted = proof.extract_output("pk_commitment"); + // Verify extract_output("pk_commitment") returns the correct values + for (i, sp) in proofs.iter().enumerate() { + let extracted = sp.payload.proof.extract_output("pk_commitment"); assert!( extracted.is_some(), - "Failed to extract pk_commitment from C1 proof[{}]", + "extract_output failed for proof[{}]", i ); } + assert_eq!( + proofs[0] + .payload + .proof + .extract_output("pk_commitment") + .unwrap()[..], + [0x11; 32] + ); + assert_eq!( + proofs[1] + .payload + .proof + .extract_output("pk_commitment") + .unwrap()[..], + [0x22; 32] + ); + assert_eq!( + proofs[2] + .payload + .proof + .extract_output("pk_commitment") + .unwrap()[..], + [0x33; 32] + ); - // Verify that mismatched commitments are detectable: - // If C5 prover computes commitment X for party 0, but C1 proof has commitment Y, - // the comparison fails. - let c1_commitment_from_proof = c1_proofs[0] - .as_ref() - .unwrap() - .payload - .proof - .extract_output("pk_commitment") - .unwrap(); - let wrong_commitment = ArcBytes::from_bytes(&[0xFF; 32]); - assert_ne!( - c1_commitment_from_proof[..], - wrong_commitment[..], - "Sanity check: commitments should differ" + Ok(()) +} + +/// Scenario 2: Partial mismatch — C1CommitmentMismatch error carries indices. +/// +/// When some parties' C1 pk_commitments don't match, the C5 prover returns +/// a C1CommitmentMismatch error with the specific faulting indices. The +/// ProofRequestActor uses these indices to emit SignedProofFailed for each +/// faulting party, and the aggregator re-aggregates without them. +#[actix::test] +#[serial_test::serial] +async fn test_c1_c5_partial_mismatch_error() -> Result<()> { + let _guard = with_tracing("info"); + + // Construct a C1CommitmentMismatch error with indices [0, 2] + let mismatch_err = e3_events::ZkError::C1CommitmentMismatch { + mismatched_indices: vec![0, 2], + }; + + // Error message should contain the indices for debugging + let msg = mismatch_err.to_string(); + assert!( + msg.contains("[0, 2]"), + "Error should contain indices: {}", + msg + ); + + // ComputeRequestErrorKind should wrap it correctly + let kind = e3_events::ComputeRequestErrorKind::Zk(mismatch_err); + assert!( + matches!( + kind, + e3_events::ComputeRequestErrorKind::Zk(e3_events::ZkError::C1CommitmentMismatch { .. }) + ), + "Should match C1CommitmentMismatch variant" ); - // Verify the ComputeRequestError with C1CommitmentMismatch can be constructed - // and carries the correct indices + Ok(()) +} + +/// Scenario 3: Total mismatch — E3 fails when not enough honest parties. +/// +/// When ALL parties have mismatched commitments, the aggregator cannot +/// re-aggregate (0 remaining ≤ threshold). It publishes E3Failed to +/// properly clean up the E3. +#[actix::test] +#[serial_test::serial] +async fn test_c1_c5_total_mismatch_fails_e3() -> Result<()> { + use e3_events::GetEvents; + + let _guard = with_tracing("info"); + let (bus, _rng, _seed, _params, _crp, _errors, history) = + e3_test_helpers::get_common_setup(None)?; + + let e3_id = E3id::new("101", 1); + + // Publish a C1CommitmentMismatch ComputeRequestError on the bus. + // In the real flow, this comes from the multithread prover. let error = e3_events::ComputeRequestError::new( e3_events::ComputeRequestErrorKind::Zk(e3_events::ZkError::C1CommitmentMismatch { - mismatched_indices: vec![0, 2], + mismatched_indices: vec![0, 1, 2], }), e3_events::ComputeRequest::zk( e3_events::ZkRequest::PkAggregation(e3_events::PkAggregationProofRequest { @@ -645,20 +713,17 @@ async fn test_c1_c5_commitment_mismatch() -> Result<()> { committee_n: 3, committee_h: 3, committee_threshold: 1, - c1_commitments: vec![], + c1_signed_proofs: vec![], }), e3_events::CorrelationId::new(), e3_id.clone(), ), ); + bus.publish_without_context(error)?; - // Publish the error on the bus - bus.publish_without_context(error.clone())?; + tokio::time::sleep(Duration::from_millis(200)).await; - // Give actix time to process - tokio::time::sleep(std::time::Duration::from_millis(200)).await; - - // The error should be visible in history as ComputeRequestError + // The error should be visible in history let events = history.send(GetEvents::new()).await?; let error_events: Vec<_> = events .iter() @@ -670,18 +735,61 @@ async fn test_c1_c5_commitment_mismatch() -> Result<()> { events.iter().map(|e| e.event_type()).collect::>() ); - // Verify the error kind is C1CommitmentMismatch - if let EnclaveEventData::ComputeRequestError(ref err) = error_events[0].get_data() { - match err.get_err() { - e3_events::ComputeRequestErrorKind::Zk(e3_events::ZkError::C1CommitmentMismatch { - ref mismatched_indices, - }) => { - assert_eq!(mismatched_indices, &[0, 2]); - } - other => bail!("Expected C1CommitmentMismatch, got: {:?}", other), - } - } else { - bail!("Expected ComputeRequestError event data"); + Ok(()) +} + +/// Scenario 4: Signed C1 proofs are carried in PkAggregationProofRequest. +/// +/// The request carries signed C1 proofs alongside keyshare bytes so that: +/// - The C5 prover can extract pk_commitment from each proof for cross-checking +/// - The ProofRequestActor can emit SignedProofFailed with full evidence +/// (address recovery + signed proof) when mismatches are detected +#[actix::test] +#[serial_test::serial] +async fn test_c1_c5_signed_proofs_in_request() -> Result<()> { + let _guard = with_tracing("info"); + let e3_id = E3id::new("102", 1); + + let proofs = vec![ + make_c1_proof(&e3_id, &[0x11; 32]), + make_c1_proof(&e3_id, &[0x22; 32]), + ]; + + // Build a PkAggregationProofRequest with signed proofs + let request = e3_events::PkAggregationProofRequest { + keyshare_bytes: vec![ + ArcBytes::from_bytes(&[1u8; 32]), + ArcBytes::from_bytes(&[2u8; 32]), + ], + aggregated_pk_bytes: ArcBytes::from_bytes(&[0u8; 32]), + params_preset: e3_fhe_params::BfvPreset::InsecureThreshold512, + committee_n: 2, + committee_h: 2, + committee_threshold: 1, + c1_signed_proofs: proofs.clone(), + }; + + // Proofs are aligned with keyshare_bytes + assert_eq!(request.c1_signed_proofs.len(), request.keyshare_bytes.len()); + + // Each proof's pk_commitment is extractable + for (i, sp) in request.c1_signed_proofs.iter().enumerate() { + let commitment = sp.payload.proof.extract_output("pk_commitment"); + assert!( + commitment.is_some(), + "Should extract pk_commitment from proof[{}]", + i + ); + } + + // Addresses are recoverable from signed proofs (for fault attribution) + for (i, sp) in request.c1_signed_proofs.iter().enumerate() { + let addr = sp.recover_address(); + assert!( + addr.is_ok(), + "Should recover address from signed proof[{}]", + i + ); } Ok(()) diff --git a/crates/zk-helpers/src/circuits/threshold/pk_generation/computation.rs b/crates/zk-helpers/src/circuits/threshold/pk_generation/computation.rs index 0e2d7f55d4..1136e19b5d 100644 --- a/crates/zk-helpers/src/circuits/threshold/pk_generation/computation.rs +++ b/crates/zk-helpers/src/circuits/threshold/pk_generation/computation.rs @@ -136,13 +136,11 @@ impl Computation for Bits { let sk_bit = calculate_bit_width(BigInt::from(data.sk_bound.clone())); let e_sm_bit = calculate_bit_width(BigInt::from(data.e_sm_bound.clone())); + // pk_bit: centered representation uses (max(qi) - 1) / 2 as the bound, + // matching compute_modulus_bit() used in C5 (pk_aggregation). let (threshold_params, _) = build_pair_for_preset(preset).map_err(|e| CircuitsErrors::Other(e.to_string()))?; - let mut pk_bit = 0; - for &qi in threshold_params.moduli() { - let bound = BigUint::from(qi - 1); - pk_bit = pk_bit.max(calculate_bit_width(BigInt::from(bound))); - } + let pk_bit = crate::compute_modulus_bit(&threshold_params); // For r1, use the maximum of all low and up bounds let mut r1_bit = 0; @@ -381,13 +379,9 @@ mod tests { let bounds = Bounds::compute(preset, &()).unwrap(); let bits = Bits::compute(preset, &bounds).unwrap(); - // pk_bit is computed from max(qi - 1) over all moduli (aligned with C2) + // pk_bit uses compute_modulus_bit: (max(qi) - 1) / 2 for centered representation let (threshold_params, _) = build_pair_for_preset(preset).unwrap(); - let mut expected_bit = 0u32; - for &qi in threshold_params.moduli() { - expected_bit = - expected_bit.max(calculate_bit_width(BigInt::from(BigUint::from(qi - 1)))); - } + let expected_bit = crate::compute_modulus_bit(&threshold_params); assert_eq!(bits.pk_bit, expected_bit); } diff --git a/crates/zk-prover/src/actors/proof_request.rs b/crates/zk-prover/src/actors/proof_request.rs index d8eb23991c..1ea2260719 100644 --- a/crates/zk-prover/src/actors/proof_request.rs +++ b/crates/zk-prover/src/actors/proof_request.rs @@ -16,10 +16,10 @@ use e3_events::{ DecryptionShareProofsPending, DecryptionshareCreated, DkgProofSigned, E3Failed, E3Stage, E3id, EnclaveEvent, EnclaveEventData, EncryptionKey, EncryptionKeyCreated, EncryptionKeyPending, EventContext, EventPublisher, EventSubscriber, EventType, FailureReason, - PkAggregationProofPending, PkAggregationProofSigned, PkBfvProofRequest, - PkGenerationProofSigned, Proof, ProofPayload, ProofType, Sequenced, - ShareDecryptionProofPending, SignedProofPayload, ThresholdShare, ThresholdShareCreated, - ThresholdSharePending, TypedEvent, ZkRequest, ZkResponse, + PkAggregationProofPending, PkAggregationProofRequest, PkAggregationProofSigned, + PkBfvProofRequest, PkGenerationProofSigned, Proof, ProofPayload, ProofType, Sequenced, + ShareDecryptionProofPending, SignedProofFailed, SignedProofPayload, ThresholdShare, + ThresholdShareCreated, ThresholdSharePending, TypedEvent, ZkRequest, ZkResponse, }; use e3_utils::utility_types::ArcBytes; use e3_utils::NotifySync; @@ -181,6 +181,7 @@ impl PendingDecryptionProofs { #[derive(Clone, Debug)] struct PendingPkAggregationProof { ec: EventContext, + request: PkAggregationProofRequest, } /// Pending C6 (ShareDecryptionProof) proof generation state. @@ -905,8 +906,13 @@ impl ProofRequestActor { return; } - self.pending_pk_aggregation - .insert(e3_id.clone(), PendingPkAggregationProof { ec: ec.clone() }); + self.pending_pk_aggregation.insert( + e3_id.clone(), + PendingPkAggregationProof { + ec: ec.clone(), + request: msg.proof_request.clone(), + }, + ); let correlation_id = CorrelationId::new(); self.pk_aggregation_correlation @@ -1466,16 +1472,56 @@ impl ProofRequestActor { "C5 proof request failed for E3 {}: {err} — PkAggregationProofSigned will not be published", e3_id ); - self.pending_pk_aggregation.remove(&e3_id); - if let Err(e) = self.bus.publish( - E3Failed { - e3_id, - failed_at_stage: E3Stage::CommitteeFinalized, - reason: FailureReason::DKGInvalidShares, - }, - ec.clone(), - ) { - error!("Failed to publish E3Failed for C5 error: {e}"); + let pending = self.pending_pk_aggregation.remove(&e3_id); + + // Emit SignedProofFailed for C1 commitment mismatches before failing E3 + if let ComputeRequestErrorKind::Zk(e3_events::ZkError::C1CommitmentMismatch { + ref mismatched_indices, + }) = msg.get_err() + { + if let Some(ref pending) = pending { + for &idx in mismatched_indices { + if let Some(signed_c1) = pending.request.c1_signed_proofs.get(idx) { + match signed_c1.recover_address() { + Ok(faulting_node) => { + if let Err(e) = self.bus.publish( + SignedProofFailed { + e3_id: e3_id.clone(), + faulting_node, + proof_type: ProofType::C1PkGeneration, + signed_payload: signed_c1.clone(), + }, + ec.clone(), + ) { + error!("Failed to publish SignedProofFailed: {e}"); + } + } + Err(e) => warn!( + "Could not recover address from C1 proof at index {idx}: {e}" + ), + } + } + } + } + } + + // Don't publish E3Failed for C1CommitmentMismatch — the aggregator + // will re-aggregate without the faulting parties and retry C5. + let is_mismatch = matches!( + msg.get_err(), + ComputeRequestErrorKind::Zk(e3_events::ZkError::C1CommitmentMismatch { .. }) + ); + if !is_mismatch { + if let Err(e) = self.bus.publish( + E3Failed { + e3_id, + failed_at_stage: E3Stage::CommitteeFinalized, + reason: FailureReason::DKGInvalidShares, + }, + ec.clone(), + ) { + error!("Failed to publish E3Failed for C5 error: {e}"); + } } return; } From b217570726ba533fbe3368d5ef8c6b5551be1197 Mon Sep 17 00:00:00 2001 From: ctrlc03 <93448202+ctrlc03@users.noreply.github.com> Date: Wed, 25 Mar 2026 17:10:26 +0000 Subject: [PATCH 08/11] chore: shorten test timeout --- crates/tests/tests/integration.rs | 240 -------------------- templates/default/tests/integration.spec.ts | 2 +- 2 files changed, 1 insertion(+), 241 deletions(-) diff --git a/crates/tests/tests/integration.rs b/crates/tests/tests/integration.rs index 68a09b35f2..ae7e495501 100644 --- a/crates/tests/tests/integration.rs +++ b/crates/tests/tests/integration.rs @@ -555,246 +555,6 @@ impl Report { } } -// ── C1→C5 Commitment Connection Tests ──────────────────────────────────────── -// -// These tests verify the C1→C5 proof connection: ensuring the keyshare data -// used in C5 (PK aggregation) matches what each party committed to in their -// C1 (PK generation) proof. -// -// The flow: -// 1. Aggregator extracts pk_commitment from each party's signed C1 proof -// 2. Passes signed C1 proofs to the C5 prover via PkAggregationProofRequest -// 3. C5 prover computes expected commitments from keyshare data and compares -// 4. On mismatch: returns C1CommitmentMismatch error → ProofRequestActor -// emits SignedProofFailed → aggregator re-aggregates and retries -// 5. On success: proof is generated with the honest subset - -/// Helper: build a mock C1 proof with known (sk, pk, esm) commitments. -/// C1 has 3 output fields of 32 bytes each in public_signals. -fn make_c1_proof(e3_id: &E3id, pk_commitment: &[u8; 32]) -> e3_events::SignedProofPayload { - use e3_events::{CircuitName, ProofPayload, ProofType, SignedProofPayload}; - - let mut signals = vec![0u8; 96]; - signals[0..32].copy_from_slice(&[0xAA; 32]); // sk_commitment - signals[32..64].copy_from_slice(pk_commitment); // pk_commitment - signals[64..96].copy_from_slice(&[0xCC; 32]); // e_sm_commitment - let proof = e3_events::Proof::new( - CircuitName::PkGeneration, - ArcBytes::from_bytes(&[0u8; 8]), - ArcBytes::from_bytes(&signals), - ); - let signer = PrivateKeySigner::random(); - let payload = ProofPayload { - e3_id: e3_id.clone(), - proof_type: ProofType::C1PkGeneration, - proof, - }; - SignedProofPayload::sign(payload, &signer).unwrap() -} - -/// Scenario 1: All C1 commitments match — happy path. -/// -/// When all parties' C1 pk_commitments match the computed commitments from -/// their keyshare data, the C5 proof is generated successfully. -#[actix::test] -#[serial_test::serial] -async fn test_c1_c5_all_commitments_match() -> Result<()> { - let _guard = with_tracing("info"); - let e3_id = E3id::new("100", 1); - - // Build 3 signed C1 proofs with distinct pk_commitments - let proofs = vec![ - make_c1_proof(&e3_id, &[0x11; 32]), - make_c1_proof(&e3_id, &[0x22; 32]), - make_c1_proof(&e3_id, &[0x33; 32]), - ]; - - // Verify extract_output("pk_commitment") returns the correct values - for (i, sp) in proofs.iter().enumerate() { - let extracted = sp.payload.proof.extract_output("pk_commitment"); - assert!( - extracted.is_some(), - "extract_output failed for proof[{}]", - i - ); - } - assert_eq!( - proofs[0] - .payload - .proof - .extract_output("pk_commitment") - .unwrap()[..], - [0x11; 32] - ); - assert_eq!( - proofs[1] - .payload - .proof - .extract_output("pk_commitment") - .unwrap()[..], - [0x22; 32] - ); - assert_eq!( - proofs[2] - .payload - .proof - .extract_output("pk_commitment") - .unwrap()[..], - [0x33; 32] - ); - - Ok(()) -} - -/// Scenario 2: Partial mismatch — C1CommitmentMismatch error carries indices. -/// -/// When some parties' C1 pk_commitments don't match, the C5 prover returns -/// a C1CommitmentMismatch error with the specific faulting indices. The -/// ProofRequestActor uses these indices to emit SignedProofFailed for each -/// faulting party, and the aggregator re-aggregates without them. -#[actix::test] -#[serial_test::serial] -async fn test_c1_c5_partial_mismatch_error() -> Result<()> { - let _guard = with_tracing("info"); - - // Construct a C1CommitmentMismatch error with indices [0, 2] - let mismatch_err = e3_events::ZkError::C1CommitmentMismatch { - mismatched_indices: vec![0, 2], - }; - - // Error message should contain the indices for debugging - let msg = mismatch_err.to_string(); - assert!( - msg.contains("[0, 2]"), - "Error should contain indices: {}", - msg - ); - - // ComputeRequestErrorKind should wrap it correctly - let kind = e3_events::ComputeRequestErrorKind::Zk(mismatch_err); - assert!( - matches!( - kind, - e3_events::ComputeRequestErrorKind::Zk(e3_events::ZkError::C1CommitmentMismatch { .. }) - ), - "Should match C1CommitmentMismatch variant" - ); - - Ok(()) -} - -/// Scenario 3: Total mismatch — E3 fails when not enough honest parties. -/// -/// When ALL parties have mismatched commitments, the aggregator cannot -/// re-aggregate (0 remaining ≤ threshold). It publishes E3Failed to -/// properly clean up the E3. -#[actix::test] -#[serial_test::serial] -async fn test_c1_c5_total_mismatch_fails_e3() -> Result<()> { - use e3_events::GetEvents; - - let _guard = with_tracing("info"); - let (bus, _rng, _seed, _params, _crp, _errors, history) = - e3_test_helpers::get_common_setup(None)?; - - let e3_id = E3id::new("101", 1); - - // Publish a C1CommitmentMismatch ComputeRequestError on the bus. - // In the real flow, this comes from the multithread prover. - let error = e3_events::ComputeRequestError::new( - e3_events::ComputeRequestErrorKind::Zk(e3_events::ZkError::C1CommitmentMismatch { - mismatched_indices: vec![0, 1, 2], - }), - e3_events::ComputeRequest::zk( - e3_events::ZkRequest::PkAggregation(e3_events::PkAggregationProofRequest { - keyshare_bytes: vec![], - aggregated_pk_bytes: ArcBytes::from_bytes(&[]), - params_preset: e3_fhe_params::BfvPreset::InsecureThreshold512, - committee_n: 3, - committee_h: 3, - committee_threshold: 1, - c1_signed_proofs: vec![], - }), - e3_events::CorrelationId::new(), - e3_id.clone(), - ), - ); - bus.publish_without_context(error)?; - - tokio::time::sleep(Duration::from_millis(200)).await; - - // The error should be visible in history - let events = history.send(GetEvents::new()).await?; - let error_events: Vec<_> = events - .iter() - .filter(|e| e.event_type() == "ComputeRequestError") - .collect(); - assert!( - !error_events.is_empty(), - "Expected ComputeRequestError in history, got: {:?}", - events.iter().map(|e| e.event_type()).collect::>() - ); - - Ok(()) -} - -/// Scenario 4: Signed C1 proofs are carried in PkAggregationProofRequest. -/// -/// The request carries signed C1 proofs alongside keyshare bytes so that: -/// - The C5 prover can extract pk_commitment from each proof for cross-checking -/// - The ProofRequestActor can emit SignedProofFailed with full evidence -/// (address recovery + signed proof) when mismatches are detected -#[actix::test] -#[serial_test::serial] -async fn test_c1_c5_signed_proofs_in_request() -> Result<()> { - let _guard = with_tracing("info"); - let e3_id = E3id::new("102", 1); - - let proofs = vec![ - make_c1_proof(&e3_id, &[0x11; 32]), - make_c1_proof(&e3_id, &[0x22; 32]), - ]; - - // Build a PkAggregationProofRequest with signed proofs - let request = e3_events::PkAggregationProofRequest { - keyshare_bytes: vec![ - ArcBytes::from_bytes(&[1u8; 32]), - ArcBytes::from_bytes(&[2u8; 32]), - ], - aggregated_pk_bytes: ArcBytes::from_bytes(&[0u8; 32]), - params_preset: e3_fhe_params::BfvPreset::InsecureThreshold512, - committee_n: 2, - committee_h: 2, - committee_threshold: 1, - c1_signed_proofs: proofs.clone(), - }; - - // Proofs are aligned with keyshare_bytes - assert_eq!(request.c1_signed_proofs.len(), request.keyshare_bytes.len()); - - // Each proof's pk_commitment is extractable - for (i, sp) in request.c1_signed_proofs.iter().enumerate() { - let commitment = sp.payload.proof.extract_output("pk_commitment"); - assert!( - commitment.is_some(), - "Should extract pk_commitment from proof[{}]", - i - ); - } - - // Addresses are recoverable from signed proofs (for fault attribution) - for (i, sp) in request.c1_signed_proofs.iter().enumerate() { - let addr = sp.recover_address(); - assert!( - addr.is_ok(), - "Should recover address from signed proof[{}]", - i - ); - } - - Ok(()) -} - /// Test trbfv #[actix::test] #[serial_test::serial] diff --git a/templates/default/tests/integration.spec.ts b/templates/default/tests/integration.spec.ts index baec0bb5f6..dedf593598 100644 --- a/templates/default/tests/integration.spec.ts +++ b/templates/default/tests/integration.spec.ts @@ -189,7 +189,7 @@ describe('Integration', () => { const { waitForEvent } = await setupEventListeners(sdk, store) const committeeSize = CommitteeSize.Micro - const duration = 600 + const duration = 500 const inputWindow = await calculateInputWindow(publicClient, duration) const thresholdBfvParams = await sdk.getThresholdBfvParamsSet() const e3ProgramParams = encodeBfvParams(thresholdBfvParams) From 38c2fe6bda6c97f3ce9ae518cfd9e733c4bfd7be Mon Sep 17 00:00:00 2001 From: ctrlc03 <93448202+ctrlc03@users.noreply.github.com> Date: Wed, 25 Mar 2026 18:17:51 +0000 Subject: [PATCH 09/11] test: add partially mocked tests --- Cargo.lock | 2 + crates/tests/Cargo.toml | 2 + crates/tests/tests/integration.rs | 460 +++++++++++++++++++++++++++++- 3 files changed, 460 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5dafd131ae..a5bc8e35e5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3770,12 +3770,14 @@ dependencies = [ "e3-logger", "e3-multithread", "e3-net", + "e3-polynomial", "e3-request", "e3-sdk", "e3-sortition", "e3-test-helpers", "e3-trbfv", "e3-utils", + "e3-zk-helpers", "e3-zk-prover", "fhe", "fhe-traits", diff --git a/crates/tests/Cargo.toml b/crates/tests/Cargo.toml index 11f0575485..9856129b97 100644 --- a/crates/tests/Cargo.toml +++ b/crates/tests/Cargo.toml @@ -36,6 +36,8 @@ e3-bfv-client = { workspace = true } e3-fhe-params = { workspace = true } e3-utils = { workspace = true } e3-zk-prover = { workspace = true } +e3-zk-helpers = { workspace = true } +e3-polynomial = { workspace = true } fhe-traits = { workspace = true } fhe-util = { workspace = true } fhe = { workspace = true } diff --git a/crates/tests/tests/integration.rs b/crates/tests/tests/integration.rs index ae7e495501..1e4aebba5d 100644 --- a/crates/tests/tests/integration.rs +++ b/crates/tests/tests/integration.rs @@ -14,14 +14,17 @@ use e3_config::BBPath; use e3_crypto::Cipher; use e3_events::{ prelude::*, BusHandle, CiphertextOutputPublished, CommitteeFinalized, ConfigurationUpdated, - E3Requested, E3id, EnclaveEvent, EnclaveEventData, OperatorActivationChanged, - PlaintextAggregated, Seed, TakeEvents, TicketBalanceUpdated, + E3Requested, E3id, EffectsEnabled, EnclaveEvent, EnclaveEventData, EventType, GetEvents, + HistoryCollector, OperatorActivationChanged, OrderedSet, PkAggregationProofPending, + PkAggregationProofRequest, PlaintextAggregated, Seed, TakeEvents, TicketBalanceUpdated, }; use e3_fhe_params::DEFAULT_BFV_PRESET; -use e3_fhe_params::{encode_bfv_params, BfvParamSet}; +use e3_fhe_params::{build_pair_for_preset, create_deterministic_crp_from_default_seed}; +use e3_fhe_params::{encode_bfv_params, BfvParamSet, BfvPreset}; use e3_multithread::{Multithread, MultithreadReport, ToReport}; use e3_net::events::{GossipData, NetEvent}; use e3_net::NetEventTranslator; +use e3_polynomial::CrtPolynomial; use e3_sortition::{calculate_buffer_size, RegisteredNode, ScoreSortition, Ticket}; use e3_test_helpers::ciphernode_system::CiphernodeSystemBuilder; use e3_test_helpers::{ @@ -30,11 +33,15 @@ use e3_test_helpers::{ use e3_trbfv::helpers::calculate_error_size; use e3_utils::utility_types::ArcBytes; use e3_utils::{colorize, rand_eth_addr, Color}; +use e3_zk_helpers::{compute_modulus_bit, compute_threshold_pk_commitment}; use e3_zk_prover::test_utils::get_tempdir; -use e3_zk_prover::ZkBackend; +use e3_zk_prover::{ProofRequestActor, ZkBackend}; use fhe::bfv::PublicKey; +use fhe::bfv::SecretKey; +use fhe::mbfv::{AggregateIter, PublicKeyShare}; use fhe_traits::{DeserializeParametrized, Serialize}; use num_bigint::BigUint; +use rand::rngs::OsRng; use rand::SeedableRng; use rand_chacha::ChaCha20Rng; use std::time::{Duration, Instant}; @@ -555,6 +562,451 @@ impl Report { } } +// ── C1->C5 Commitment Connection Tests +// +// These tests verify the C1→C5 proof connection end-to-end through real actors: +// 1. Generate real BFV threshold keyshares +// 2. Compute correct pk_commitments from keyshare data +// 3. Build signed C1 proofs with correct or tampered commitments +// 4. Dispatch PkAggregationProofPending through the event bus +// 5. Verify the Multithread prover detects mismatches +// 6. Verify ProofRequestActor emits SignedProofFailed for faulting parties +// 7. Verify E3Failed is NOT emitted on mismatch (aggregator retries) + +/// Generate BFV threshold keyshares and compute the correct pk_commitment +/// for each party, matching what the C5 prover expects. +fn generate_keyshares_and_commitments( + num_parties: usize, +) -> Result<( + Vec, // keyshare_bytes (serialized PublicKeyShare) + ArcBytes, // aggregated_pk_bytes + Vec<[u8; 32]>, // correct pk_commitments per party + Arc, +)> { + let preset = BfvPreset::InsecureThreshold512; + let (threshold_params, _) = + build_pair_for_preset(preset).map_err(|e| anyhow::anyhow!("{e}"))?; + let crp = create_deterministic_crp_from_default_seed(&threshold_params); + + let mut keyshare_bytes = Vec::with_capacity(num_parties); + let mut pk_shares = Vec::with_capacity(num_parties); + + for _ in 0..num_parties { + let sk = SecretKey::random(&threshold_params, &mut OsRng); + let pk_share = PublicKeyShare::new(&sk, crp.clone(), &mut OsRng)?; + keyshare_bytes.push(ArcBytes::from_bytes(&pk_share.to_bytes())); + pk_shares.push(pk_share); + } + + let public_key: PublicKey = pk_shares.iter().cloned().aggregate()?; + let aggregated_pk_bytes = ArcBytes::from_bytes(&public_key.to_bytes()); + + // Compute correct pk_commitments using the same logic as PkAggInputs::compute + let bit_pk = compute_modulus_bit(&threshold_params); + let mut commitments = Vec::with_capacity(num_parties); + + for pk_share in &pk_shares { + let mut pk0 = CrtPolynomial::from_fhe_polynomial(&pk_share.p0_share()); + let mut pk1 = CrtPolynomial::from_fhe_polynomial(&crp.poly()); + pk0.reverse(); + pk0.center(threshold_params.moduli()) + .map_err(|e| anyhow::anyhow!("{e}"))?; + pk1.reverse(); + pk1.center(threshold_params.moduli()) + .map_err(|e| anyhow::anyhow!("{e}"))?; + + let commitment = compute_threshold_pk_commitment(&pk0, &pk1, bit_pk); + let (_, be_bytes) = commitment.to_bytes_be(); + let mut padded = [0u8; 32]; + let start = 32usize.saturating_sub(be_bytes.len()); + padded[start..].copy_from_slice(&be_bytes[..be_bytes.len().min(32)]); + commitments.push(padded); + } + + Ok(( + keyshare_bytes, + aggregated_pk_bytes, + commitments, + threshold_params, + )) +} + +/// Build a signed C1 proof with a specific pk_commitment in public_signals. +/// C1 (PkGeneration) has 3 output fields of 32 bytes each: +/// [sk_commitment, pk_commitment, e_sm_commitment] +fn build_signed_c1_proof( + e3_id: &E3id, + pk_commitment: &[u8; 32], + signer: &PrivateKeySigner, +) -> e3_events::SignedProofPayload { + use e3_events::{CircuitName, ProofPayload, ProofType, SignedProofPayload}; + + let mut signals = vec![0u8; 96]; + signals[0..32].copy_from_slice(&[0xAA; 32]); // sk_commitment (dummy) + signals[32..64].copy_from_slice(pk_commitment); // pk_commitment (real) + signals[64..96].copy_from_slice(&[0xCC; 32]); // e_sm_commitment (dummy) + + let proof = e3_events::Proof::new( + CircuitName::PkGeneration, + ArcBytes::from_bytes(&[0u8; 8]), + ArcBytes::from_bytes(&signals), + ); + let payload = ProofPayload { + e3_id: e3_id.clone(), + proof_type: ProofType::C1PkGeneration, + proof, + }; + SignedProofPayload::sign(payload, signer).unwrap() +} + +/// Set up the minimal actor stack for C1->C5 tests: +/// event bus + Multithread (with dummy ZkBackend) + ProofRequestActor + HistoryCollector. +async fn setup_c1_c5_actors() -> Result<( + BusHandle, + actix::Addr>, + tempfile::TempDir, // keep alive so paths stay valid +)> { + let system = EventSystem::new().with_fresh_bus(); + let bus = system.handle()?.enable("c1c5test"); + + let history = HistoryCollector::::new().start(); + bus.subscribe(EventType::All, history.clone().recipient()); + + // Dummy ZkBackend — the commitment check runs before any proof generation, + // so the bb binary and circuit files are never accessed on the mismatch path. + let temp = tempfile::tempdir()?; + let temp_path = temp.path(); + let dummy_backend = ZkBackend::new( + BBPath::Default(temp_path.join("bb")), + temp_path.join("circuits"), + temp_path.join("work"), + ); + + // Multithread actor with ZK prover + let rng = create_shared_rng_from_u64(99); + let cipher = Arc::new(Cipher::from_password("c1c5-test-key").await?); + let task_pool = Multithread::create_taskpool(2, 1); + Multithread::attach_with_zk(&bus, rng, cipher, task_pool, None, &dummy_backend); + + // ProofRequestActor handles PkAggregationProofPending → ComputeRequest dispatch, + // and ComputeRequestError → SignedProofFailed emission. + let signer = PrivateKeySigner::random(); + ProofRequestActor::setup(&bus, signer); + + // Enable effects so Multithread subscribes to ComputeRequest + bus.publish_without_context(EffectsEnabled::new())?; + sleep(Duration::from_millis(50)).await; + + Ok((bus, history, temp)) +} + +/// C1->C5: Tampered commitment is detected as C1CommitmentMismatch. +/// +/// Generates 3 real BFV keyshares, builds C1 proofs with correct commitments +/// for parties 0 and 1 but a WRONG commitment for party 2, dispatches +/// PkAggregationProofPending through the actor system, and verifies: +/// - ComputeRequestError with C1CommitmentMismatch appears in history +/// - The mismatched index includes party 2 +/// - SignedProofFailed is emitted for the faulting party +/// - E3Failed is NOT emitted (aggregator can retry with honest subset) +#[actix::test] +#[serial_test::serial] +async fn test_c1_c5_tampered_commitment_detected() -> Result<()> { + let _guard = with_tracing("info"); + let (bus, history, _temp) = setup_c1_c5_actors().await?; + + let e3_id = E3id::new("9001", 1); + let num_parties = 3; + + // Generate real BFV keyshares and correct commitments + let (keyshare_bytes, aggregated_pk_bytes, correct_commitments, _params) = + generate_keyshares_and_commitments(num_parties)?; + + // Build signed C1 proofs: parties 0,1 get correct commitments, party 2 gets a tampered one + let signers: Vec<_> = (0..num_parties) + .map(|_| PrivateKeySigner::random()) + .collect(); + let mut c1_proofs = Vec::with_capacity(num_parties); + for i in 0..num_parties { + let commitment = if i == 2 { + [0xFFu8; 32] // tampered commitment + } else { + correct_commitments[i] + }; + c1_proofs.push(build_signed_c1_proof(&e3_id, &commitment, &signers[i])); + } + + // Dispatch PkAggregationProofPending + let proof_request = PkAggregationProofRequest { + keyshare_bytes, + aggregated_pk_bytes, + params_preset: BfvPreset::InsecureThreshold512, + committee_n: num_parties, + committee_h: num_parties, + committee_threshold: 1, + c1_signed_proofs: c1_proofs, + }; + + bus.publish_without_context(PkAggregationProofPending { + e3_id: e3_id.clone(), + proof_request, + public_key: ArcBytes::from_bytes(&[]), + nodes: OrderedSet::from(vec![ + signers[0].address().to_string(), + signers[1].address().to_string(), + signers[2].address().to_string(), + ]), + })?; + + // Wait for the async actor chain to complete: + // PkAggregationProofPending -> ProofRequestActor -> ComputeRequest + // → Multithread -> ComputeRequestError -> ProofRequestActor -> SignedProofFailed + sleep(Duration::from_secs(5)).await; + + let events = history.send(GetEvents::new()).await?; + let event_types: Vec = events.iter().map(|e| e.event_type()).collect(); + + // 1. ComputeRequestError should be in history (from Multithread) + assert!( + event_types.contains(&"ComputeRequestError".to_string()), + "Expected ComputeRequestError in history, got: {:?}", + event_types + ); + + // 2. SignedProofFailed should be emitted for the faulting party + let signed_proof_failed: Vec<_> = events + .iter() + .filter(|e| e.event_type() == "SignedProofFailed") + .collect(); + assert!( + !signed_proof_failed.is_empty(), + "Expected SignedProofFailed in history, got: {:?}", + event_types + ); + + // 3. ProofRequestActor should NOT emit E3Failed for C1CommitmentMismatch — + // it suppresses E3Failed so the PublicKeyAggregator can retry with honest parties. + assert!( + !event_types.contains(&"E3Failed".to_string()), + "ProofRequestActor should not emit E3Failed on C1CommitmentMismatch" + ); + + Ok(()) +} + +/// C1->C5: All commitments correct passes the check (gets past commitment verification). +/// +/// With all correct commitments, the commitment check passes and the prover +/// proceeds to actual proof generation — which fails because we use a dummy +/// ZkBackend. We verify the error is NOT a C1CommitmentMismatch. +#[actix::test] +#[serial_test::serial] +async fn test_c1_c5_correct_commitments_pass_check() -> Result<()> { + let _guard = with_tracing("info"); + let (bus, history, _temp) = setup_c1_c5_actors().await?; + + let e3_id = E3id::new("9002", 1); + let num_parties = 3; + + let (keyshare_bytes, aggregated_pk_bytes, correct_commitments, _params) = + generate_keyshares_and_commitments(num_parties)?; + + // All C1 proofs get correct commitments + let signers: Vec<_> = (0..num_parties) + .map(|_| PrivateKeySigner::random()) + .collect(); + let c1_proofs: Vec<_> = (0..num_parties) + .map(|i| build_signed_c1_proof(&e3_id, &correct_commitments[i], &signers[i])) + .collect(); + + let proof_request = PkAggregationProofRequest { + keyshare_bytes, + aggregated_pk_bytes, + params_preset: BfvPreset::InsecureThreshold512, + committee_n: num_parties, + committee_h: num_parties, + committee_threshold: 1, + c1_signed_proofs: c1_proofs, + }; + + bus.publish_without_context(PkAggregationProofPending { + e3_id: e3_id.clone(), + proof_request, + public_key: ArcBytes::from_bytes(&[]), + nodes: OrderedSet::from( + signers + .iter() + .map(|s| s.address().to_string()) + .collect::>(), + ), + })?; + + // Wait for processing — the commitment check passes but proof generation + // fails (dummy backend, no bb binary). We just check it's NOT a mismatch. + sleep(Duration::from_secs(5)).await; + + let events = history.send(GetEvents::new()).await?; + let event_types: Vec = events.iter().map(|e| e.event_type()).collect(); + + // Should NOT have SignedProofFailed (no commitment mismatch) + assert!( + !event_types.contains(&"SignedProofFailed".to_string()), + "SignedProofFailed should NOT appear when all commitments match, got: {:?}", + event_types + ); + + Ok(()) +} + +/// C1->C5: All commitments tampered — all indices detected. +/// +/// When every party's C1 commitment is wrong, the prover returns +/// C1CommitmentMismatch with all indices. SignedProofFailed is emitted +/// for each party. +#[actix::test] +#[serial_test::serial] +async fn test_c1_c5_all_tampered_all_detected() -> Result<()> { + let _guard = with_tracing("info"); + let (bus, history, _temp) = setup_c1_c5_actors().await?; + + let e3_id = E3id::new("9003", 1); + let num_parties = 3; + + let (keyshare_bytes, aggregated_pk_bytes, _correct_commitments, _params) = + generate_keyshares_and_commitments(num_parties)?; + + // ALL C1 proofs get wrong commitments + let signers: Vec<_> = (0..num_parties) + .map(|_| PrivateKeySigner::random()) + .collect(); + let c1_proofs: Vec<_> = (0..num_parties) + .map(|i| { + let bad_commitment = [(i + 1) as u8; 32]; // unique wrong value per party + build_signed_c1_proof(&e3_id, &bad_commitment, &signers[i]) + }) + .collect(); + + let proof_request = PkAggregationProofRequest { + keyshare_bytes, + aggregated_pk_bytes, + params_preset: BfvPreset::InsecureThreshold512, + committee_n: num_parties, + committee_h: num_parties, + committee_threshold: 1, + c1_signed_proofs: c1_proofs, + }; + + bus.publish_without_context(PkAggregationProofPending { + e3_id: e3_id.clone(), + proof_request, + public_key: ArcBytes::from_bytes(&[]), + nodes: OrderedSet::from( + signers + .iter() + .map(|s| s.address().to_string()) + .collect::>(), + ), + })?; + + sleep(Duration::from_secs(5)).await; + + let events = history.send(GetEvents::new()).await?; + let event_types: Vec = events.iter().map(|e| e.event_type()).collect(); + + // All 3 parties should have SignedProofFailed events + let failed_count = event_types + .iter() + .filter(|t| *t == "SignedProofFailed") + .count(); + assert_eq!( + failed_count, num_parties, + "Expected {} SignedProofFailed events (one per tampered party), got {}. Events: {:?}", + num_parties, failed_count, event_types + ); + + // NOTE: In the full system, PublicKeyAggregator would receive the + // ComputeRequestError, determine 0 honest parties remain, and emit + // E3Failed. This test only covers ProofRequestActor + Multithread + // (no aggregator), so E3Failed emission from the retry-or-fail path + // is not exercised here. + + Ok(()) +} + +/// C1->C5: Fault attribution recovers the correct signer address. +/// +/// Verifies that the SignedProofFailed event contains the correct faulting +/// node address (recovered from the ECDSA signature on the tampered C1 proof). +#[actix::test] +#[serial_test::serial] +async fn test_c1_c5_fault_attribution_address_recovery() -> Result<()> { + let _guard = with_tracing("info"); + let (bus, history, _temp) = setup_c1_c5_actors().await?; + + let e3_id = E3id::new("9004", 1); + let num_parties = 2; + + let (keyshare_bytes, aggregated_pk_bytes, correct_commitments, _params) = + generate_keyshares_and_commitments(num_parties)?; + + // Party 0 correct, party 1 tampered + let faulting_signer = PrivateKeySigner::random(); + let honest_signer = PrivateKeySigner::random(); + let expected_faulting_addr = faulting_signer.address(); + + let c1_proofs = vec![ + build_signed_c1_proof(&e3_id, &correct_commitments[0], &honest_signer), + build_signed_c1_proof(&e3_id, &[0xBB; 32], &faulting_signer), // tampered + ]; + + let proof_request = PkAggregationProofRequest { + keyshare_bytes, + aggregated_pk_bytes, + params_preset: BfvPreset::InsecureThreshold512, + committee_n: num_parties, + committee_h: num_parties, + committee_threshold: 1, + c1_signed_proofs: c1_proofs, + }; + + bus.publish_without_context(PkAggregationProofPending { + e3_id: e3_id.clone(), + proof_request, + public_key: ArcBytes::from_bytes(&[]), + nodes: OrderedSet::from(vec![ + honest_signer.address().to_string(), + faulting_signer.address().to_string(), + ]), + })?; + + sleep(Duration::from_secs(5)).await; + + let events = history.send(GetEvents::new()).await?; + + // Find SignedProofFailed and verify the recovered address + let failed_events: Vec<_> = events + .iter() + .filter_map(|e| match e.clone().into_data() { + EnclaveEventData::SignedProofFailed(spf) => Some(spf), + _ => None, + }) + .collect(); + + assert_eq!( + failed_events.len(), + 1, + "Expected exactly 1 SignedProofFailed, got {}", + failed_events.len() + ); + + assert_eq!( + failed_events[0].faulting_node, expected_faulting_addr, + "SignedProofFailed should contain the faulting party's address" + ); + + Ok(()) +} + /// Test trbfv #[actix::test] #[serial_test::serial] From 171819090b15b290fb2da8df40fba2e057cf018f Mon Sep 17 00:00:00 2001 From: ctrlc03 <93448202+ctrlc03@users.noreply.github.com> Date: Wed, 25 Mar 2026 21:18:42 +0000 Subject: [PATCH 10/11] refactor: commitment checks before c5 --- crates/aggregator/src/publickey_aggregator.rs | 256 ++++------ .../src/enclave_event/compute_request/zk.rs | 13 - crates/multithread/src/multithread.rs | 46 +- crates/tests/tests/integration.rs | 445 ------------------ crates/zk-helpers/src/circuits/commitments.rs | 80 ++++ crates/zk-prover/src/actors/proof_request.rs | 59 +-- 6 files changed, 190 insertions(+), 709 deletions(-) diff --git a/crates/aggregator/src/publickey_aggregator.rs b/crates/aggregator/src/publickey_aggregator.rs index 05509cfd07..4b791e9944 100644 --- a/crates/aggregator/src/publickey_aggregator.rs +++ b/crates/aggregator/src/publickey_aggregator.rs @@ -9,12 +9,12 @@ use actix::prelude::*; use anyhow::Result; use e3_data::Persistable; use e3_events::{ - prelude::*, BusHandle, ComputeRequestError, ComputeRequestErrorKind, ComputeRequestKind, - ComputeResponse, ComputeResponseKind, DKGRecursiveAggregationComplete, Die, E3id, EnclaveEvent, - EnclaveEventData, EventContext, KeyshareCreated, OrderedSet, PartyProofsToVerify, - PkAggregationProofPending, PkAggregationProofRequest, PkAggregationProofSigned, Proof, - PublicKeyAggregated, Seed, Sequenced, ShareVerificationComplete, ShareVerificationDispatched, - SignedProofPayload, TypedEvent, VerificationKind, ZkError, ZkRequest, ZkResponse, + prelude::*, BusHandle, ComputeResponse, ComputeResponseKind, DKGRecursiveAggregationComplete, + Die, E3Failed, E3Stage, E3id, EnclaveEvent, EnclaveEventData, EventContext, FailureReason, + KeyshareCreated, OrderedSet, PartyProofsToVerify, PkAggregationProofPending, + PkAggregationProofRequest, PkAggregationProofSigned, Proof, ProofType, PublicKeyAggregated, + Seed, Sequenced, ShareVerificationComplete, ShareVerificationDispatched, SignedProofFailed, + SignedProofPayload, TypedEvent, VerificationKind, ZkResponse, }; use e3_events::{trap, EType}; use e3_fhe::{Fhe, GetAggregatePublicKey}; @@ -258,42 +258,97 @@ impl PublicKeyAggregator { )); }; - let dishonest_parties = &msg.dishonest_parties; + let mut dishonest_parties = msg.dishonest_parties.clone(); let total_parties = submission_order.len(); - // Filter out dishonest parties using submission_order (insertion-order indexed, - // matching the party IDs sent to dispatch_c1_verification). - let honest_entries: Vec<(usize, (String, ArcBytes))> = submission_order + // Filter out parties that failed C1 ZK verification. + let mut honest_entries: Vec<(usize, (String, ArcBytes))> = submission_order .into_iter() .enumerate() .filter(|(idx, _)| !dishonest_parties.contains(&(*idx as u64))) .collect(); + // Cross-check: verify each party's keyshare matches their C1 pk_commitment. + // Parties that fail are marked dishonest and reported via SignedProofFailed. + let mut commitment_dishonest = Vec::new(); + for (party_idx, (_node, ks)) in &honest_entries { + let signed_proof = match c1_proofs.get(*party_idx).and_then(|opt| opt.as_ref()) { + Some(proof) => proof, + None => { + // No C1 proof for this party — should already be in dishonest_parties. + // If not, treat as dishonest now (defensive). + warn!( + "Party {} has no C1 proof but was not marked dishonest", + party_idx + ); + dishonest_parties.insert(*party_idx as u64); + continue; + } + }; + let ok = match e3_zk_helpers::compute_pk_commitment_from_keyshare_bytes( + ks, + &self.fhe.params, + &self.fhe.crp, + ) { + Ok(computed) => signed_proof + .payload + .proof + .extract_output("pk_commitment") + .map_or(false, |extracted| extracted[..] == computed[..]), + Err(e) => { + warn!( + "Failed to compute pk_commitment for party {}: {}", + party_idx, e + ); + false + } + }; + if !ok { + commitment_dishonest.push((*party_idx as u64, signed_proof.clone())); + } + } + + // Emit SignedProofFailed for each commitment-mismatched party + for (party_idx, signed_proof) in &commitment_dishonest { + dishonest_parties.insert(*party_idx); + match signed_proof.recover_address() { + Ok(faulting_node) => { + if let Err(e) = self.bus.publish( + SignedProofFailed { + e3_id: self.e3_id.clone(), + faulting_node, + proof_type: ProofType::C1PkGeneration, + signed_payload: signed_proof.clone(), + }, + ec.clone(), + ) { + error!("Failed to publish SignedProofFailed: {e}"); + } + } + Err(e) => warn!( + "Could not recover address from C1 proof for party {}: {e}", + party_idx + ), + } + } + + if !commitment_dishonest.is_empty() { + warn!( + "C1 commitment mismatch for {} parties — filtering before aggregation", + commitment_dishonest.len() + ); + // Re-filter honest_entries after commitment check + honest_entries.retain(|(idx, _)| !dishonest_parties.contains(&(*idx as u64))); + } + let (honest_keyshares, honest_nodes): (Vec, Vec) = honest_entries .iter() .map(|(_, (node, ks))| (ks.clone(), node.clone())) .unzip(); - // Collect signed C1 proofs from honest parties (submission order). - // All honest parties should have a signed C1 proof (parties without - // proofs are marked dishonest in dispatch_c1_verification). - let c1_signed_proofs_submission_order: Vec = honest_entries - .iter() - .filter_map(|(idx, _)| c1_proofs.get(*idx).and_then(|opt| opt.clone())) - .collect(); - - if c1_signed_proofs_submission_order.len() != honest_keyshares.len() { - return Err(anyhow::anyhow!( - "C1 proof count ({}) != honest keyshare count ({}) — data misalignment", - c1_signed_proofs_submission_order.len(), - honest_keyshares.len() - )); - } - if !dishonest_parties.is_empty() { warn!( - "Filtered out {} dishonest parties from C1 verification: {:?}", - dishonest_parties.len(), + "Total dishonest parties (ZK + commitment): {:?}", dishonest_parties ); } @@ -304,11 +359,20 @@ impl PublicKeyAggregator { // Need at least threshold + 1 honest parties for aggregation if honest_keyshares.len() <= threshold_m { - return Err(anyhow::anyhow!( - "Not enough honest parties after C1 verification: {} (need at least {})", + error!( + "Not enough honest parties after filtering: {} (need > {})", honest_keyshares.len(), - threshold_m + 1 - )); + threshold_m + ); + self.bus.publish( + E3Failed { + e3_id: self.e3_id.clone(), + failed_at_stage: E3Stage::CommitteeFinalized, + reason: FailureReason::DKGInvalidShares, + }, + ec, + )?; + return Ok(()); } // Synchronous aggregation @@ -322,22 +386,8 @@ impl PublicKeyAggregator { })?; let committee_h = honest_keyshares.len(); - let honest_nodes_set = OrderedSet::from(honest_nodes); - // keyshare_bytes follows OrderedSet ordering (used by C5 prover). - // Re-align c1_signed_proofs to the same order. + let honest_nodes_set = OrderedSet::from(honest_nodes.clone()); let keyshare_bytes: Vec<_> = honest_keyshares_set.iter().cloned().collect(); - let c1_signed_proofs: Vec = { - let ks_to_proof: std::collections::HashMap, &SignedProofPayload> = - honest_keyshares - .iter() - .zip(c1_signed_proofs_submission_order.iter()) - .map(|(ks, sp)| (ks.to_vec(), sp)) - .collect(); - keyshare_bytes - .iter() - .filter_map(|ks| ks_to_proof.get(&ks.to_vec()).map(|sp| (*sp).clone())) - .collect() - }; let pubkey = ArcBytes::from_bytes(&pubkey); info!("Publishing PkAggregationProofPending for C5 proof generation..."); @@ -351,7 +401,6 @@ impl PublicKeyAggregator { committee_n: committee_h, committee_h, committee_threshold: 0, - c1_signed_proofs, }, public_key: pubkey.clone(), nodes: honest_nodes_set.clone(), @@ -725,109 +774,6 @@ impl PublicKeyAggregator { Ok(()) } - /// Handle C5 proof error. On C1CommitmentMismatch, re-aggregate without - /// the faulting parties and re-dispatch C5. On other errors, just log. - fn handle_c5_error( - &mut self, - error: ComputeRequestError, - ec: EventContext, - ) -> Result<()> { - let ComputeRequestErrorKind::Zk(ZkError::C1CommitmentMismatch { - ref mismatched_indices, - }) = error.get_err() - else { - error!( - "PublicKeyAggregator received ComputeRequestError: {}", - error - ); - return Ok(()); - }; - - // Extract the original request from the error - let pk_req = match &error.request().request { - ComputeRequestKind::Zk(ZkRequest::PkAggregation(req)) => req.clone(), - _ => { - error!("C1CommitmentMismatch error with non-PkAggregation request"); - return Ok(()); - } - }; - - warn!( - "C1 commitment mismatch at indices {:?} — re-aggregating without faulting parties", - mismatched_indices - ); - - // Filter out mismatched parties - let remaining_keyshares: Vec = pk_req - .keyshare_bytes - .iter() - .enumerate() - .filter(|(i, _)| !mismatched_indices.contains(i)) - .map(|(_, ks)| ks.clone()) - .collect(); - let remaining_c1_proofs: Vec = pk_req - .c1_signed_proofs - .iter() - .enumerate() - .filter(|(i, _)| !mismatched_indices.contains(i)) - .map(|(_, sp)| sp.clone()) - .collect(); - - if remaining_keyshares.len() <= pk_req.committee_threshold { - error!( - "Not enough honest parties after C1 commitment mismatch filtering: {} (need > {})", - remaining_keyshares.len(), - pk_req.committee_threshold - ); - self.bus.publish( - e3_events::E3Failed { - e3_id: self.e3_id.clone(), - failed_at_stage: e3_events::E3Stage::CommitteeFinalized, - reason: e3_events::FailureReason::DKGInvalidShares, - }, - ec, - )?; - return Ok(()); - } - - // Re-aggregate the public key from remaining honest keyshares - let remaining_set = OrderedSet::from(remaining_keyshares.clone()); - let pubkey = self.fhe.get_aggregate_public_key(GetAggregatePublicKey { - keyshares: remaining_set, - })?; - - let committee_h = remaining_keyshares.len(); - let pubkey = ArcBytes::from_bytes(&pubkey); - - // Re-dispatch C5 with the filtered data - self.bus.publish( - PkAggregationProofPending { - e3_id: self.e3_id.clone(), - proof_request: PkAggregationProofRequest { - keyshare_bytes: remaining_keyshares, - aggregated_pk_bytes: pubkey.clone(), - params_preset: self.params_preset.clone(), - committee_n: committee_h, - committee_h, - committee_threshold: 0, - c1_signed_proofs: remaining_c1_proofs, - }, - public_key: pubkey, - nodes: self - .state - .get() - .map(|s| match s { - PublicKeyAggregatorState::GeneratingC5Proof { nodes, .. } => nodes, - _ => OrderedSet::new(), - }) - .unwrap_or_default(), - }, - ec, - )?; - - Ok(()) - } - pub fn handle_member_expelled( &mut self, node: &str, @@ -918,14 +864,6 @@ impl Handler for PublicKeyAggregator { EnclaveEventData::ComputeResponse(data) => { self.notify_sync(ctx, TypedEvent::new(data, ec)) } - EnclaveEventData::ComputeRequestError(data) => { - if data.request().e3_id != self.e3_id { - return; - } - trap(EType::PublickeyAggregation, &self.bus.with_ec(&ec), || { - self.handle_c5_error(data, ec.clone()) - }); - } EnclaveEventData::E3RequestComplete(_) => self.notify_sync(ctx, Die), EnclaveEventData::CommitteeMemberExpelled(data) => { // Only process raw events from chain (party_id not yet resolved). diff --git a/crates/events/src/enclave_event/compute_request/zk.rs b/crates/events/src/enclave_event/compute_request/zk.rs index d954662d18..d274bfbcc2 100644 --- a/crates/events/src/enclave_event/compute_request/zk.rs +++ b/crates/events/src/enclave_event/compute_request/zk.rs @@ -61,10 +61,6 @@ pub struct PkAggregationProofRequest { pub committee_h: usize, /// Threshold (T). pub committee_threshold: usize, - /// Signed C1 proofs per party, aligned with `keyshare_bytes`. - /// The C5 prover extracts pk_commitment from each proof for cross-checking, - /// and returns mismatched indices for fault attribution. - pub c1_signed_proofs: Vec, } /// Request to generate a proof for share computation (C2a or C2b). @@ -450,10 +446,6 @@ pub enum ZkError { WitnessGenerationFailed(String), /// Invalid parameters. InvalidParams(String), - /// C1 commitment mismatch: not enough honest parties remain after filtering. - /// The mismatched indices identify which parties' C1 proofs are inconsistent - /// with their keyshare data. - C1CommitmentMismatch { mismatched_indices: Vec }, } impl std::fmt::Display for ZkError { @@ -464,11 +456,6 @@ impl std::fmt::Display for ZkError { write!(f, "Witness generation failed: {}", msg) } ZkError::InvalidParams(msg) => write!(f, "Invalid parameters: {}", msg), - ZkError::C1CommitmentMismatch { mismatched_indices } => write!( - f, - "C1 commitment mismatch at indices {:?} — not enough honest parties", - mismatched_indices - ), } } } diff --git a/crates/multithread/src/multithread.rs b/crates/multithread/src/multithread.rs index e6d3277706..6357ede622 100644 --- a/crates/multithread/src/multithread.rs +++ b/crates/multithread/src/multithread.rs @@ -66,7 +66,6 @@ use e3_zk_helpers::dkg::share_computation::{ }; use e3_zk_helpers::dkg::share_decryption::{ShareDecryptionCircuit, ShareDecryptionCircuitData}; use e3_zk_helpers::dkg::share_encryption::{ShareEncryptionCircuit, ShareEncryptionCircuitData}; -use e3_zk_helpers::threshold::pk_aggregation::computation::Inputs as PkAggInputs; use e3_zk_helpers::threshold::pk_aggregation::PkAggregationCircuit; use e3_zk_helpers::threshold::pk_aggregation::PkAggregationCircuitData; use e3_zk_helpers::CiphernodesCommittee; @@ -357,48 +356,9 @@ fn handle_pk_aggregation_proof( a, }; - // 6b. Verify C1 commitments match computed commitments. - // On mismatch, return C1CommitmentMismatch error with the faulting indices. - // The ProofRequestActor emits SignedProofFailed for each faulting party, - // and the PublicKeyAggregator re-aggregates without them and retries. - let pk_agg_inputs = PkAggInputs::compute(req.params_preset.clone(), &circuit_data) - .map_err(|e| make_zk_error(&request, format!("PkAggInputs::compute: {}", e)))?; - - let expected = &pk_agg_inputs.expected_threshold_pk_commitments; - let mut mismatched_indices = Vec::new(); - for (i, sp) in req.c1_signed_proofs.iter().enumerate() { - let matches = if let Some(extracted) = sp.payload.proof.extract_output("pk_commitment") { - if let Some(computed) = expected.get(i) { - let (_, be_bytes) = computed.to_bytes_be(); - if be_bytes.len() > 32 { - false - } else { - let mut padded = [0u8; 32]; - padded[32 - be_bytes.len()..].copy_from_slice(&be_bytes); - padded[..] == extracted[..] - } - } else { - false - } - } else { - false - }; - if !matches { - mismatched_indices.push(i); - } - } - - if !mismatched_indices.is_empty() { - return Err(ComputeRequestError::new( - ComputeRequestErrorKind::Zk(ZkEventError::C1CommitmentMismatch { mismatched_indices }), - request, - )); - } - - info!( - "C1 commitment cross-check passed for {} parties", - expected.len() - ); + // C1 commitment consistency is verified by the PublicKeyAggregator before + // dispatching this request (pre-aggregation check). By the time we reach + // the prover, all keyshares are guaranteed to match their C1 proofs. // 7. Generate proof via Provable trait (C5 is always EVM-targeted for on-chain verification) let circuit = PkAggregationCircuit; diff --git a/crates/tests/tests/integration.rs b/crates/tests/tests/integration.rs index 1e4aebba5d..22962d6a01 100644 --- a/crates/tests/tests/integration.rs +++ b/crates/tests/tests/integration.rs @@ -562,451 +562,6 @@ impl Report { } } -// ── C1->C5 Commitment Connection Tests -// -// These tests verify the C1→C5 proof connection end-to-end through real actors: -// 1. Generate real BFV threshold keyshares -// 2. Compute correct pk_commitments from keyshare data -// 3. Build signed C1 proofs with correct or tampered commitments -// 4. Dispatch PkAggregationProofPending through the event bus -// 5. Verify the Multithread prover detects mismatches -// 6. Verify ProofRequestActor emits SignedProofFailed for faulting parties -// 7. Verify E3Failed is NOT emitted on mismatch (aggregator retries) - -/// Generate BFV threshold keyshares and compute the correct pk_commitment -/// for each party, matching what the C5 prover expects. -fn generate_keyshares_and_commitments( - num_parties: usize, -) -> Result<( - Vec, // keyshare_bytes (serialized PublicKeyShare) - ArcBytes, // aggregated_pk_bytes - Vec<[u8; 32]>, // correct pk_commitments per party - Arc, -)> { - let preset = BfvPreset::InsecureThreshold512; - let (threshold_params, _) = - build_pair_for_preset(preset).map_err(|e| anyhow::anyhow!("{e}"))?; - let crp = create_deterministic_crp_from_default_seed(&threshold_params); - - let mut keyshare_bytes = Vec::with_capacity(num_parties); - let mut pk_shares = Vec::with_capacity(num_parties); - - for _ in 0..num_parties { - let sk = SecretKey::random(&threshold_params, &mut OsRng); - let pk_share = PublicKeyShare::new(&sk, crp.clone(), &mut OsRng)?; - keyshare_bytes.push(ArcBytes::from_bytes(&pk_share.to_bytes())); - pk_shares.push(pk_share); - } - - let public_key: PublicKey = pk_shares.iter().cloned().aggregate()?; - let aggregated_pk_bytes = ArcBytes::from_bytes(&public_key.to_bytes()); - - // Compute correct pk_commitments using the same logic as PkAggInputs::compute - let bit_pk = compute_modulus_bit(&threshold_params); - let mut commitments = Vec::with_capacity(num_parties); - - for pk_share in &pk_shares { - let mut pk0 = CrtPolynomial::from_fhe_polynomial(&pk_share.p0_share()); - let mut pk1 = CrtPolynomial::from_fhe_polynomial(&crp.poly()); - pk0.reverse(); - pk0.center(threshold_params.moduli()) - .map_err(|e| anyhow::anyhow!("{e}"))?; - pk1.reverse(); - pk1.center(threshold_params.moduli()) - .map_err(|e| anyhow::anyhow!("{e}"))?; - - let commitment = compute_threshold_pk_commitment(&pk0, &pk1, bit_pk); - let (_, be_bytes) = commitment.to_bytes_be(); - let mut padded = [0u8; 32]; - let start = 32usize.saturating_sub(be_bytes.len()); - padded[start..].copy_from_slice(&be_bytes[..be_bytes.len().min(32)]); - commitments.push(padded); - } - - Ok(( - keyshare_bytes, - aggregated_pk_bytes, - commitments, - threshold_params, - )) -} - -/// Build a signed C1 proof with a specific pk_commitment in public_signals. -/// C1 (PkGeneration) has 3 output fields of 32 bytes each: -/// [sk_commitment, pk_commitment, e_sm_commitment] -fn build_signed_c1_proof( - e3_id: &E3id, - pk_commitment: &[u8; 32], - signer: &PrivateKeySigner, -) -> e3_events::SignedProofPayload { - use e3_events::{CircuitName, ProofPayload, ProofType, SignedProofPayload}; - - let mut signals = vec![0u8; 96]; - signals[0..32].copy_from_slice(&[0xAA; 32]); // sk_commitment (dummy) - signals[32..64].copy_from_slice(pk_commitment); // pk_commitment (real) - signals[64..96].copy_from_slice(&[0xCC; 32]); // e_sm_commitment (dummy) - - let proof = e3_events::Proof::new( - CircuitName::PkGeneration, - ArcBytes::from_bytes(&[0u8; 8]), - ArcBytes::from_bytes(&signals), - ); - let payload = ProofPayload { - e3_id: e3_id.clone(), - proof_type: ProofType::C1PkGeneration, - proof, - }; - SignedProofPayload::sign(payload, signer).unwrap() -} - -/// Set up the minimal actor stack for C1->C5 tests: -/// event bus + Multithread (with dummy ZkBackend) + ProofRequestActor + HistoryCollector. -async fn setup_c1_c5_actors() -> Result<( - BusHandle, - actix::Addr>, - tempfile::TempDir, // keep alive so paths stay valid -)> { - let system = EventSystem::new().with_fresh_bus(); - let bus = system.handle()?.enable("c1c5test"); - - let history = HistoryCollector::::new().start(); - bus.subscribe(EventType::All, history.clone().recipient()); - - // Dummy ZkBackend — the commitment check runs before any proof generation, - // so the bb binary and circuit files are never accessed on the mismatch path. - let temp = tempfile::tempdir()?; - let temp_path = temp.path(); - let dummy_backend = ZkBackend::new( - BBPath::Default(temp_path.join("bb")), - temp_path.join("circuits"), - temp_path.join("work"), - ); - - // Multithread actor with ZK prover - let rng = create_shared_rng_from_u64(99); - let cipher = Arc::new(Cipher::from_password("c1c5-test-key").await?); - let task_pool = Multithread::create_taskpool(2, 1); - Multithread::attach_with_zk(&bus, rng, cipher, task_pool, None, &dummy_backend); - - // ProofRequestActor handles PkAggregationProofPending → ComputeRequest dispatch, - // and ComputeRequestError → SignedProofFailed emission. - let signer = PrivateKeySigner::random(); - ProofRequestActor::setup(&bus, signer); - - // Enable effects so Multithread subscribes to ComputeRequest - bus.publish_without_context(EffectsEnabled::new())?; - sleep(Duration::from_millis(50)).await; - - Ok((bus, history, temp)) -} - -/// C1->C5: Tampered commitment is detected as C1CommitmentMismatch. -/// -/// Generates 3 real BFV keyshares, builds C1 proofs with correct commitments -/// for parties 0 and 1 but a WRONG commitment for party 2, dispatches -/// PkAggregationProofPending through the actor system, and verifies: -/// - ComputeRequestError with C1CommitmentMismatch appears in history -/// - The mismatched index includes party 2 -/// - SignedProofFailed is emitted for the faulting party -/// - E3Failed is NOT emitted (aggregator can retry with honest subset) -#[actix::test] -#[serial_test::serial] -async fn test_c1_c5_tampered_commitment_detected() -> Result<()> { - let _guard = with_tracing("info"); - let (bus, history, _temp) = setup_c1_c5_actors().await?; - - let e3_id = E3id::new("9001", 1); - let num_parties = 3; - - // Generate real BFV keyshares and correct commitments - let (keyshare_bytes, aggregated_pk_bytes, correct_commitments, _params) = - generate_keyshares_and_commitments(num_parties)?; - - // Build signed C1 proofs: parties 0,1 get correct commitments, party 2 gets a tampered one - let signers: Vec<_> = (0..num_parties) - .map(|_| PrivateKeySigner::random()) - .collect(); - let mut c1_proofs = Vec::with_capacity(num_parties); - for i in 0..num_parties { - let commitment = if i == 2 { - [0xFFu8; 32] // tampered commitment - } else { - correct_commitments[i] - }; - c1_proofs.push(build_signed_c1_proof(&e3_id, &commitment, &signers[i])); - } - - // Dispatch PkAggregationProofPending - let proof_request = PkAggregationProofRequest { - keyshare_bytes, - aggregated_pk_bytes, - params_preset: BfvPreset::InsecureThreshold512, - committee_n: num_parties, - committee_h: num_parties, - committee_threshold: 1, - c1_signed_proofs: c1_proofs, - }; - - bus.publish_without_context(PkAggregationProofPending { - e3_id: e3_id.clone(), - proof_request, - public_key: ArcBytes::from_bytes(&[]), - nodes: OrderedSet::from(vec![ - signers[0].address().to_string(), - signers[1].address().to_string(), - signers[2].address().to_string(), - ]), - })?; - - // Wait for the async actor chain to complete: - // PkAggregationProofPending -> ProofRequestActor -> ComputeRequest - // → Multithread -> ComputeRequestError -> ProofRequestActor -> SignedProofFailed - sleep(Duration::from_secs(5)).await; - - let events = history.send(GetEvents::new()).await?; - let event_types: Vec = events.iter().map(|e| e.event_type()).collect(); - - // 1. ComputeRequestError should be in history (from Multithread) - assert!( - event_types.contains(&"ComputeRequestError".to_string()), - "Expected ComputeRequestError in history, got: {:?}", - event_types - ); - - // 2. SignedProofFailed should be emitted for the faulting party - let signed_proof_failed: Vec<_> = events - .iter() - .filter(|e| e.event_type() == "SignedProofFailed") - .collect(); - assert!( - !signed_proof_failed.is_empty(), - "Expected SignedProofFailed in history, got: {:?}", - event_types - ); - - // 3. ProofRequestActor should NOT emit E3Failed for C1CommitmentMismatch — - // it suppresses E3Failed so the PublicKeyAggregator can retry with honest parties. - assert!( - !event_types.contains(&"E3Failed".to_string()), - "ProofRequestActor should not emit E3Failed on C1CommitmentMismatch" - ); - - Ok(()) -} - -/// C1->C5: All commitments correct passes the check (gets past commitment verification). -/// -/// With all correct commitments, the commitment check passes and the prover -/// proceeds to actual proof generation — which fails because we use a dummy -/// ZkBackend. We verify the error is NOT a C1CommitmentMismatch. -#[actix::test] -#[serial_test::serial] -async fn test_c1_c5_correct_commitments_pass_check() -> Result<()> { - let _guard = with_tracing("info"); - let (bus, history, _temp) = setup_c1_c5_actors().await?; - - let e3_id = E3id::new("9002", 1); - let num_parties = 3; - - let (keyshare_bytes, aggregated_pk_bytes, correct_commitments, _params) = - generate_keyshares_and_commitments(num_parties)?; - - // All C1 proofs get correct commitments - let signers: Vec<_> = (0..num_parties) - .map(|_| PrivateKeySigner::random()) - .collect(); - let c1_proofs: Vec<_> = (0..num_parties) - .map(|i| build_signed_c1_proof(&e3_id, &correct_commitments[i], &signers[i])) - .collect(); - - let proof_request = PkAggregationProofRequest { - keyshare_bytes, - aggregated_pk_bytes, - params_preset: BfvPreset::InsecureThreshold512, - committee_n: num_parties, - committee_h: num_parties, - committee_threshold: 1, - c1_signed_proofs: c1_proofs, - }; - - bus.publish_without_context(PkAggregationProofPending { - e3_id: e3_id.clone(), - proof_request, - public_key: ArcBytes::from_bytes(&[]), - nodes: OrderedSet::from( - signers - .iter() - .map(|s| s.address().to_string()) - .collect::>(), - ), - })?; - - // Wait for processing — the commitment check passes but proof generation - // fails (dummy backend, no bb binary). We just check it's NOT a mismatch. - sleep(Duration::from_secs(5)).await; - - let events = history.send(GetEvents::new()).await?; - let event_types: Vec = events.iter().map(|e| e.event_type()).collect(); - - // Should NOT have SignedProofFailed (no commitment mismatch) - assert!( - !event_types.contains(&"SignedProofFailed".to_string()), - "SignedProofFailed should NOT appear when all commitments match, got: {:?}", - event_types - ); - - Ok(()) -} - -/// C1->C5: All commitments tampered — all indices detected. -/// -/// When every party's C1 commitment is wrong, the prover returns -/// C1CommitmentMismatch with all indices. SignedProofFailed is emitted -/// for each party. -#[actix::test] -#[serial_test::serial] -async fn test_c1_c5_all_tampered_all_detected() -> Result<()> { - let _guard = with_tracing("info"); - let (bus, history, _temp) = setup_c1_c5_actors().await?; - - let e3_id = E3id::new("9003", 1); - let num_parties = 3; - - let (keyshare_bytes, aggregated_pk_bytes, _correct_commitments, _params) = - generate_keyshares_and_commitments(num_parties)?; - - // ALL C1 proofs get wrong commitments - let signers: Vec<_> = (0..num_parties) - .map(|_| PrivateKeySigner::random()) - .collect(); - let c1_proofs: Vec<_> = (0..num_parties) - .map(|i| { - let bad_commitment = [(i + 1) as u8; 32]; // unique wrong value per party - build_signed_c1_proof(&e3_id, &bad_commitment, &signers[i]) - }) - .collect(); - - let proof_request = PkAggregationProofRequest { - keyshare_bytes, - aggregated_pk_bytes, - params_preset: BfvPreset::InsecureThreshold512, - committee_n: num_parties, - committee_h: num_parties, - committee_threshold: 1, - c1_signed_proofs: c1_proofs, - }; - - bus.publish_without_context(PkAggregationProofPending { - e3_id: e3_id.clone(), - proof_request, - public_key: ArcBytes::from_bytes(&[]), - nodes: OrderedSet::from( - signers - .iter() - .map(|s| s.address().to_string()) - .collect::>(), - ), - })?; - - sleep(Duration::from_secs(5)).await; - - let events = history.send(GetEvents::new()).await?; - let event_types: Vec = events.iter().map(|e| e.event_type()).collect(); - - // All 3 parties should have SignedProofFailed events - let failed_count = event_types - .iter() - .filter(|t| *t == "SignedProofFailed") - .count(); - assert_eq!( - failed_count, num_parties, - "Expected {} SignedProofFailed events (one per tampered party), got {}. Events: {:?}", - num_parties, failed_count, event_types - ); - - // NOTE: In the full system, PublicKeyAggregator would receive the - // ComputeRequestError, determine 0 honest parties remain, and emit - // E3Failed. This test only covers ProofRequestActor + Multithread - // (no aggregator), so E3Failed emission from the retry-or-fail path - // is not exercised here. - - Ok(()) -} - -/// C1->C5: Fault attribution recovers the correct signer address. -/// -/// Verifies that the SignedProofFailed event contains the correct faulting -/// node address (recovered from the ECDSA signature on the tampered C1 proof). -#[actix::test] -#[serial_test::serial] -async fn test_c1_c5_fault_attribution_address_recovery() -> Result<()> { - let _guard = with_tracing("info"); - let (bus, history, _temp) = setup_c1_c5_actors().await?; - - let e3_id = E3id::new("9004", 1); - let num_parties = 2; - - let (keyshare_bytes, aggregated_pk_bytes, correct_commitments, _params) = - generate_keyshares_and_commitments(num_parties)?; - - // Party 0 correct, party 1 tampered - let faulting_signer = PrivateKeySigner::random(); - let honest_signer = PrivateKeySigner::random(); - let expected_faulting_addr = faulting_signer.address(); - - let c1_proofs = vec![ - build_signed_c1_proof(&e3_id, &correct_commitments[0], &honest_signer), - build_signed_c1_proof(&e3_id, &[0xBB; 32], &faulting_signer), // tampered - ]; - - let proof_request = PkAggregationProofRequest { - keyshare_bytes, - aggregated_pk_bytes, - params_preset: BfvPreset::InsecureThreshold512, - committee_n: num_parties, - committee_h: num_parties, - committee_threshold: 1, - c1_signed_proofs: c1_proofs, - }; - - bus.publish_without_context(PkAggregationProofPending { - e3_id: e3_id.clone(), - proof_request, - public_key: ArcBytes::from_bytes(&[]), - nodes: OrderedSet::from(vec![ - honest_signer.address().to_string(), - faulting_signer.address().to_string(), - ]), - })?; - - sleep(Duration::from_secs(5)).await; - - let events = history.send(GetEvents::new()).await?; - - // Find SignedProofFailed and verify the recovered address - let failed_events: Vec<_> = events - .iter() - .filter_map(|e| match e.clone().into_data() { - EnclaveEventData::SignedProofFailed(spf) => Some(spf), - _ => None, - }) - .collect(); - - assert_eq!( - failed_events.len(), - 1, - "Expected exactly 1 SignedProofFailed, got {}", - failed_events.len() - ); - - assert_eq!( - failed_events[0].faulting_node, expected_faulting_addr, - "SignedProofFailed should contain the faulting party's address" - ); - - Ok(()) -} - /// Test trbfv #[actix::test] #[serial_test::serial] diff --git a/crates/zk-helpers/src/circuits/commitments.rs b/crates/zk-helpers/src/circuits/commitments.rs index 15c8b34098..0b2a8e2006 100644 --- a/crates/zk-helpers/src/circuits/commitments.rs +++ b/crates/zk-helpers/src/circuits/commitments.rs @@ -217,6 +217,46 @@ pub fn compute_threshold_pk_commitment( BigInt::from_bytes_le(num_bigint::Sign::Plus, &commitment_bytes) } +/// Compute the pk_commitment for a serialized `PublicKeyShare`, matching what the C1 circuit outputs. +/// +/// Deserializes the keyshare, extracts the pk0 polynomial, and hashes it +/// together with the CRP (pk1) to produce the commitment. Returns 32 +/// big-endian bytes, ready to compare against +/// `Proof::extract_output("pk_commitment")` from a C1 proof. +/// +/// The caller supplies pre-built `params` and `crp` so that batch calls +/// (multiple keyshares with the same parameters) don't rebuild them each time. +pub fn compute_pk_commitment_from_keyshare_bytes( + keyshare_bytes: &[u8], + params: &std::sync::Arc, + crp: &fhe::mbfv::CommonRandomPoly, +) -> Result<[u8; 32], crate::CircuitsErrors> { + let bit_pk = crate::compute_modulus_bit(params); + let moduli = params.moduli(); + + let pk_share = fhe::mbfv::PublicKeyShare::deserialize(keyshare_bytes, params, crp.clone()) + .map_err(|e| { + crate::CircuitsErrors::Other(format!("PublicKeyShare deserialize: {:?}", e)) + })?; + + let mut pk0 = CrtPolynomial::from_fhe_polynomial(&pk_share.p0_share()); + pk0.reverse(); + pk0.center(moduli) + .map_err(|e| crate::CircuitsErrors::Other(format!("pk0 center: {}", e)))?; + + let mut pk1 = CrtPolynomial::from_fhe_polynomial(&crp.poly()); + pk1.reverse(); + pk1.center(moduli) + .map_err(|e| crate::CircuitsErrors::Other(format!("pk1 center: {}", e)))?; + + let commitment = compute_threshold_pk_commitment(&pk0, &pk1, bit_pk); + let (_, be_bytes) = commitment.to_bytes_be(); + let mut padded = [0u8; 32]; + let start = 32usize.saturating_sub(be_bytes.len()); + padded[start..].copy_from_slice(&be_bytes[..be_bytes.len().min(32)]); + Ok(padded) +} + /// Compute a commitment to the threshold secret key share by flattening it and hashing. /// /// This matches the Noir `compute_share_computation_sk_commitment` function exactly. @@ -674,4 +714,44 @@ mod tests { let expected = compute_commitments(vk_hashes.clone(), super::DS_VK_HASH, io_pattern)[0]; assert_eq!(compute_vk_hash(vk_hashes), expected); } + + #[test] + fn compute_pk_commitment_from_keyshare_roundtrip() { + use e3_fhe_params::{ + build_pair_for_preset, create_deterministic_crp_from_default_seed, BfvPreset, + }; + use fhe::bfv::SecretKey; + use fhe::mbfv::PublicKeyShare; + use fhe_traits::Serialize; + use rand::rngs::OsRng; + + let preset = BfvPreset::InsecureThreshold512; + let (params, _) = build_pair_for_preset(preset).unwrap(); + let crp = create_deterministic_crp_from_default_seed(¶ms); + + // Generate a real keyshare + let sk = SecretKey::random(¶ms, &mut OsRng); + let pk_share = PublicKeyShare::new(&sk, crp.clone(), &mut OsRng).unwrap(); + let ks_bytes = pk_share.to_bytes(); + + // Compute commitment via the helper + let commitment = + compute_pk_commitment_from_keyshare_bytes(&ks_bytes, ¶ms, &crp).unwrap(); + + // Compute commitment manually (same steps as PkAggInputs::compute) + let bit_pk = crate::compute_modulus_bit(¶ms); + let mut pk0 = CrtPolynomial::from_fhe_polynomial(&pk_share.p0_share()); + pk0.reverse(); + pk0.center(params.moduli()).unwrap(); + let mut pk1 = CrtPolynomial::from_fhe_polynomial(&crp.poly()); + pk1.reverse(); + pk1.center(params.moduli()).unwrap(); + let expected = compute_threshold_pk_commitment(&pk0, &pk1, bit_pk); + let (_, be_bytes) = expected.to_bytes_be(); + let mut expected_padded = [0u8; 32]; + let start = 32usize.saturating_sub(be_bytes.len()); + expected_padded[start..].copy_from_slice(&be_bytes[..be_bytes.len().min(32)]); + + assert_eq!(commitment, expected_padded); + } } diff --git a/crates/zk-prover/src/actors/proof_request.rs b/crates/zk-prover/src/actors/proof_request.rs index 1ea2260719..61fc4ac134 100644 --- a/crates/zk-prover/src/actors/proof_request.rs +++ b/crates/zk-prover/src/actors/proof_request.rs @@ -1472,56 +1472,17 @@ impl ProofRequestActor { "C5 proof request failed for E3 {}: {err} — PkAggregationProofSigned will not be published", e3_id ); - let pending = self.pending_pk_aggregation.remove(&e3_id); - - // Emit SignedProofFailed for C1 commitment mismatches before failing E3 - if let ComputeRequestErrorKind::Zk(e3_events::ZkError::C1CommitmentMismatch { - ref mismatched_indices, - }) = msg.get_err() - { - if let Some(ref pending) = pending { - for &idx in mismatched_indices { - if let Some(signed_c1) = pending.request.c1_signed_proofs.get(idx) { - match signed_c1.recover_address() { - Ok(faulting_node) => { - if let Err(e) = self.bus.publish( - SignedProofFailed { - e3_id: e3_id.clone(), - faulting_node, - proof_type: ProofType::C1PkGeneration, - signed_payload: signed_c1.clone(), - }, - ec.clone(), - ) { - error!("Failed to publish SignedProofFailed: {e}"); - } - } - Err(e) => warn!( - "Could not recover address from C1 proof at index {idx}: {e}" - ), - } - } - } - } - } + self.pending_pk_aggregation.remove(&e3_id); - // Don't publish E3Failed for C1CommitmentMismatch — the aggregator - // will re-aggregate without the faulting parties and retry C5. - let is_mismatch = matches!( - msg.get_err(), - ComputeRequestErrorKind::Zk(e3_events::ZkError::C1CommitmentMismatch { .. }) - ); - if !is_mismatch { - if let Err(e) = self.bus.publish( - E3Failed { - e3_id, - failed_at_stage: E3Stage::CommitteeFinalized, - reason: FailureReason::DKGInvalidShares, - }, - ec.clone(), - ) { - error!("Failed to publish E3Failed for C5 error: {e}"); - } + if let Err(e) = self.bus.publish( + E3Failed { + e3_id, + failed_at_stage: E3Stage::CommitteeFinalized, + reason: FailureReason::DKGInvalidShares, + }, + ec.clone(), + ) { + error!("Failed to publish E3Failed for C5 error: {e}"); } return; } From 1ffff4ed2ac3c3813b56d033fc0e7591d824d910 Mon Sep 17 00:00:00 2001 From: ctrlc03 <93448202+ctrlc03@users.noreply.github.com> Date: Thu, 26 Mar 2026 10:48:38 +0000 Subject: [PATCH 11/11] chore: cleanup --- crates/aggregator/src/publickey_aggregator.rs | 6 +-- .../proof_verification_passed.rs | 2 +- .../actors/commitment_consistency_checker.rs | 14 +++---- .../src/actors/commitment_links/c1_to_c5.rs | 42 ++++++++----------- .../src/actors/proof_verification.rs | 2 +- .../src/actors/share_verification.rs | 4 +- 6 files changed, 32 insertions(+), 38 deletions(-) diff --git a/crates/aggregator/src/publickey_aggregator.rs b/crates/aggregator/src/publickey_aggregator.rs index 4b791e9944..09791cb83a 100644 --- a/crates/aggregator/src/publickey_aggregator.rs +++ b/crates/aggregator/src/publickey_aggregator.rs @@ -12,9 +12,9 @@ use e3_events::{ prelude::*, BusHandle, ComputeResponse, ComputeResponseKind, DKGRecursiveAggregationComplete, Die, E3Failed, E3Stage, E3id, EnclaveEvent, EnclaveEventData, EventContext, FailureReason, KeyshareCreated, OrderedSet, PartyProofsToVerify, PkAggregationProofPending, - PkAggregationProofRequest, PkAggregationProofSigned, Proof, ProofType, PublicKeyAggregated, - Seed, Sequenced, ShareVerificationComplete, ShareVerificationDispatched, SignedProofFailed, - SignedProofPayload, TypedEvent, VerificationKind, ZkResponse, + PkAggregationProofRequest, PkAggregationProofSigned, Proof, ProofType, ProofVerificationPassed, + PublicKeyAggregated, Seed, Sequenced, ShareVerificationComplete, ShareVerificationDispatched, + SignedProofFailed, SignedProofPayload, TypedEvent, VerificationKind, ZkResponse, }; use e3_events::{trap, EType}; use e3_fhe::{Fhe, GetAggregatePublicKey}; diff --git a/crates/events/src/enclave_event/proof_verification_passed.rs b/crates/events/src/enclave_event/proof_verification_passed.rs index aed00f2338..15511f54e4 100644 --- a/crates/events/src/enclave_event/proof_verification_passed.rs +++ b/crates/events/src/enclave_event/proof_verification_passed.rs @@ -33,7 +33,7 @@ pub struct ProofVerificationPassed { /// keccak256 hash of the received data + proof bytes — for equivocation detection. pub data_hash: [u8; 32], /// Raw public signals from the verified proof — for commitment consistency checks. - pub public_outputs: ArcBytes, + pub public_signals: ArcBytes, } impl Display for ProofVerificationPassed { diff --git a/crates/zk-prover/src/actors/commitment_consistency_checker.rs b/crates/zk-prover/src/actors/commitment_consistency_checker.rs index 634c1bafaa..30642cb45a 100644 --- a/crates/zk-prover/src/actors/commitment_consistency_checker.rs +++ b/crates/zk-prover/src/actors/commitment_consistency_checker.rs @@ -36,7 +36,7 @@ use tracing::{info, warn}; struct VerifiedProofData { party_id: u64, address: Address, - public_outputs: ArcBytes, + public_signals: ArcBytes, } /// Per-E3 actor that enforces cross-circuit commitment consistency. @@ -102,8 +102,8 @@ impl CommitmentConsistencyChecker { let target = self.verified.get(&(address, tgt_type)); if let (Some(src), Some(tgt)) = (source, target) { - let source_values = link.extract_source_values(&src.public_outputs); - if !link.check_consistency(&source_values, &tgt.public_outputs) { + let source_values = link.extract_source_values(&src.public_signals); + if !link.check_consistency(&source_values, &tgt.public_signals) { warn!( "[{}] Commitment mismatch for E3 {} — party {} ({}): \ source {:?} vs target {:?} from same address", @@ -146,12 +146,12 @@ impl CommitmentConsistencyChecker { // For each (source, target) pair, check consistency. for src in &sources { - let source_values = link.extract_source_values(&src.public_outputs); + let source_values = link.extract_source_values(&src.public_signals); if source_values.is_empty() { continue; } for tgt in &targets { - if !link.check_consistency(&source_values, &tgt.public_outputs) { + if !link.check_consistency(&source_values, &tgt.public_signals) { warn!( "[{}] Commitment mismatch for E3 {} — source party {} ({}) {:?} \ not consistent with target party {} ({}) {:?}", @@ -205,14 +205,14 @@ impl Handler> for CommitmentConsistencyCheck let proof_type = data.proof_type; let address = data.address; - let public_outputs = data.public_outputs; + let public_signals = data.public_signals; self.verified.insert( (address, proof_type), VerifiedProofData { party_id: data.party_id, address, - public_outputs, + public_signals, }, ); diff --git a/crates/zk-prover/src/actors/commitment_links/c1_to_c5.rs b/crates/zk-prover/src/actors/commitment_links/c1_to_c5.rs index 89169a4ee5..db5519cdb0 100644 --- a/crates/zk-prover/src/actors/commitment_links/c1_to_c5.rs +++ b/crates/zk-prover/src/actors/commitment_links/c1_to_c5.rs @@ -23,10 +23,8 @@ //! `expected_threshold_pk_commitments` array. use super::{CommitmentLink, FieldValue, LinkScope}; -use e3_events::ProofType; - -/// Size of one BN254 field element in bytes. -const FIELD_SIZE: usize = 32; +use e3_events::{CircuitName, ProofType}; +use e3_zk_helpers::{CircuitOutputLayout, FIELD_BYTE_LEN}; /// C1 → C5 pk_commitment consistency link. pub struct C1ToC5PkCommitmentLink; @@ -49,13 +47,12 @@ impl CommitmentLink for C1ToC5PkCommitmentLink { } fn extract_source_values(&self, public_signals: &[u8]) -> Vec { - // C1 outputs: (sk_commitment, pk_commitment, e_sm_commitment) — 3 fields, no public inputs - // pk_commitment is at field index 1 (bytes 32..64) - if public_signals.len() < 3 * FIELD_SIZE { + let layout = CircuitName::PkGeneration.output_layout(); + let Some(bytes) = layout.extract_field(public_signals, "pk_commitment") else { return vec![]; - } - let mut value = [0u8; FIELD_SIZE]; - value.copy_from_slice(&public_signals[FIELD_SIZE..2 * FIELD_SIZE]); + }; + let mut value = [0u8; FIELD_BYTE_LEN]; + value.copy_from_slice(bytes); vec![value] } @@ -65,30 +62,27 @@ impl CommitmentLink for C1ToC5PkCommitmentLink { target_public_signals: &[u8], ) -> bool { if source_values.is_empty() { - // No source values to check — vacuously consistent. return true; } - if target_public_signals.len() < 2 * FIELD_SIZE { - // Target proof is present but has malformed/truncated signals — non-consistent. + // C5 public_signals layout: [pub inputs: pk_commitments[0..H]] [output: commitment] + // The output count comes from the circuit layout; everything before it is public inputs. + let output_count = CircuitName::PkAggregation + .output_layout() + .field_count() + .unwrap_or(1); + let total_fields = target_public_signals.len() / FIELD_BYTE_LEN; + if total_fields <= output_count { return false; } + let h = total_fields - output_count; let source_pk_commitment = &source_values[0]; - // C5 public_signals: [expected_pk_commitments[0..H], pk_agg_commitment] - // H = total_fields - 1 (last field is the output) - let total_fields = target_public_signals.len() / FIELD_SIZE; - if total_fields < 2 { - // Target proof present but not enough fields — non-consistent. - return false; - } - let h = total_fields - 1; - // Check if the source pk_commitment appears in any of the H input fields for i in 0..h { - let offset = i * FIELD_SIZE; - if target_public_signals[offset..offset + FIELD_SIZE] == *source_pk_commitment { + let offset = i * FIELD_BYTE_LEN; + if target_public_signals[offset..offset + FIELD_BYTE_LEN] == *source_pk_commitment { return true; } } diff --git a/crates/zk-prover/src/actors/proof_verification.rs b/crates/zk-prover/src/actors/proof_verification.rs index 10b1aa0869..d9d1371509 100644 --- a/crates/zk-prover/src/actors/proof_verification.rs +++ b/crates/zk-prover/src/actors/proof_verification.rs @@ -251,7 +251,7 @@ impl Handler> for ProofVerificationActor { address: recovered_signer, proof_type: ProofType::C0PkBfv, data_hash, - public_outputs: signed_payload.payload.proof.public_signals.clone(), + public_signals: signed_payload.payload.proof.public_signals.clone(), }, ec, ) { diff --git a/crates/zk-prover/src/actors/share_verification.rs b/crates/zk-prover/src/actors/share_verification.rs index 89ff8c33cb..38a24c9b83 100644 --- a/crates/zk-prover/src/actors/share_verification.rs +++ b/crates/zk-prover/src/actors/share_verification.rs @@ -460,7 +460,7 @@ impl ShareVerificationActor { .unwrap_or_default(); let signals = pending.party_public_signals.get(&result.sender_party_id); for (i, &(proof_type, data_hash)) in hashes.iter().enumerate() { - let public_outputs = signals + let public_signals = signals .and_then(|s| s.get(i)) .map(|(_, ps)| ps.clone()) .unwrap_or_default(); @@ -471,7 +471,7 @@ impl ShareVerificationActor { address: addr, proof_type, data_hash, - public_outputs, + public_signals, }, pending.ec.clone(), ) {