Skip to content

chore(coprocessor): map verify_and_expand panics (if any) to an error #1271

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: georgi/sns/minor-fixes
Choose a base branch
from
Open

chore(coprocessor): map verify_and_expand panics (if any) to an error #1271

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions coprocessor/fhevm-engine/zkproof-worker/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ pub enum ExecutionError {

#[error("Too many inputs: {0}")]
TooManyInputs(usize),

#[error("Panic occurred during verify and expand: {0}")]
VerifyExpandPanic(String),
}

impl From<ExecutionError> for ServiceError {
Expand Down
21 changes: 20 additions & 1 deletion coprocessor/fhevm-engine/zkproof-worker/src/verifier.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use fhevm_engine_common::tfhe_ops::{current_ciphertext_version, extract_ct_list}
use fhevm_engine_common::types::SupportedFheCiphertexts;

use fhevm_engine_common::utils::safe_deserialize_conformant;
use fhevm_engine_common::with_panic_guard;
use hex::encode;
use lru::LruCache;
use sha3::Digest;
Expand Down Expand Up @@ -353,7 +354,7 @@ pub(crate) fn verify_proof(
set_server_key(keys.server_key.clone());

let mut s = span.child_span("verify_and_expand");
let mut cts = match try_verify_and_expand_ciphertext_list(request_id, raw_ct, keys, aux_data) {
let mut cts = match verify_and_expand_with_guard(request_id, raw_ct, keys, aux_data) {
Ok(cts) => {
telemetry::attribute(&mut s, "count", cts.len().to_string());
telemetry::end_span(s);
Expand Down Expand Up @@ -382,6 +383,24 @@ pub(crate) fn verify_proof(
Ok((cts, blob_hash))
}

fn verify_and_expand_with_guard(
request_id: i64,
raw_ct: &[u8],
keys: &FetchTenantKeyResult,
aux_data: &auxiliary::ZkData,
) -> Result<Vec<SupportedFheCiphertexts>, ExecutionError> {
with_panic_guard!(try_verify_and_expand_ciphertext_list(
request_id, raw_ct, keys, aux_data
))
.map_err(|err| {
// Map panic to VerifyExpandPanic
ExecutionError::VerifyExpandPanic(format!(
"Panic occurred while verifying and expanding: {}",
err
))
})?
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The with_panic_guard! macro returns Result<T, String> on success (wrapping the inner result), so this code performs double error handling. The ? operator at line 401 will propagate the outer Result<Vec<SupportedFheCiphertexts>, ExecutionError> from map_err, but the inner Result<Vec<SupportedFheCiphertexts>, ExecutionError> from try_verify_and_expand_ciphertext_list is still wrapped. This should be .map_err(...)? followed by another ? to unwrap both layers, or the entire expression should use ??. Compare with the sns-worker implementation at lines 766-776 which correctly uses ?? to handle both the panic result and the inner result.

Suggested change
})?
})??

Copilot uses AI. Check for mistakes.
}

fn try_verify_and_expand_ciphertext_list(
request_id: i64,
raw_ct: &[u8],
Expand Down
Loading