From 741f245c6d229ce819356ce1d1334d64d36a8e91 Mon Sep 17 00:00:00 2001 From: tomasarrachea Date: Wed, 25 Sep 2024 15:05:16 -0300 Subject: [PATCH 1/3] fix invalid signature bug --- crates/services/bls_aggregation/src/bls_agg.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/crates/services/bls_aggregation/src/bls_agg.rs b/crates/services/bls_aggregation/src/bls_agg.rs index 28b7de9f8..c2e428f41 100644 --- a/crates/services/bls_aggregation/src/bls_agg.rs +++ b/crates/services/bls_aggregation/src/bls_agg.rs @@ -322,6 +322,7 @@ impl BlsAggregatorService &operator_state_avs, ) .await; + let verification_failed = verification_result.is_err(); signed_task_digest .signature_verification_channel @@ -329,6 +330,10 @@ impl BlsAggregatorService .await .map_err(|_| BlsAggregationServiceError::ChannelError)?; + if verification_failed { + continue; + } + let operator_state = operator_state_avs .get(&signed_task_digest.operator_id) .unwrap(); @@ -1789,5 +1794,18 @@ mod tests { )), result ); + + // Also test that the aggregator service is not affected by the invalid signature, so the task should expire + let response = bls_agg_service + .aggregated_response_receiver + .lock() + .await + .recv() + .await; + + assert_eq!( + Err(BlsAggregationServiceError::TaskExpired), + response.unwrap() + ); } } From 63e230d0e791c2136cd3ed3e0b73d027bea9b222 Mon Sep 17 00:00:00 2001 From: tomasarrachea Date: Wed, 25 Sep 2024 15:07:22 -0300 Subject: [PATCH 2/3] add duplicate signature failing test --- .../services/bls_aggregation/src/bls_agg.rs | 101 ++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/crates/services/bls_aggregation/src/bls_agg.rs b/crates/services/bls_aggregation/src/bls_agg.rs index c2e428f41..48358d712 100644 --- a/crates/services/bls_aggregation/src/bls_agg.rs +++ b/crates/services/bls_aggregation/src/bls_agg.rs @@ -668,6 +668,107 @@ mod tests { assert_eq!(task_index, response.unwrap().unwrap().task_index); } + #[tokio::test] + async fn test_1_quorum_2_operator_2_duplicated_signatures() { + let test_operator_1 = TestOperator { + operator_id: U256::from(1).into(), + stake_per_quorum: HashMap::from([(0u8, U256::from(100))]), + bls_keypair: BlsKeyPair::new(PRIVATE_KEY_1.into()).unwrap(), + }; + let test_operator_2 = TestOperator { + operator_id: U256::from(2).into(), + stake_per_quorum: HashMap::from([(0u8, U256::from(100))]), + bls_keypair: BlsKeyPair::new(PRIVATE_KEY_2.into()).unwrap(), + }; + let test_operators = vec![test_operator_1.clone(), test_operator_2.clone()]; + let block_number = 1; + let task_index: TaskIndex = 0; + let quorum_numbers = vec![0]; + let quorum_threshold_percentages: QuorumThresholdPercentages = vec![100]; + let time_to_expiry = Duration::from_secs(1); + let task_response = 123; // Initialize with appropriate data + let task_response_digest = hash(task_response); + + let fake_avs_registry_service = + FakeAvsRegistryService::new(block_number, test_operators.clone()); + let bls_agg_service = BlsAggregatorService::new(fake_avs_registry_service); + bls_agg_service + .initialize_new_task( + task_index, + block_number as u32, + quorum_numbers, + quorum_threshold_percentages, + time_to_expiry, + ) + .await + .unwrap(); + + let bls_signature_1 = test_operator_1 + .bls_keypair + .sign_message(task_response_digest.as_ref()); + bls_agg_service + .process_new_signature( + task_index, + task_response_digest, + bls_signature_1.clone(), + test_operator_1.operator_id, + ) + .await + .unwrap(); + + bls_agg_service + .process_new_signature( + task_index, + task_response_digest, + bls_signature_1.clone(), + test_operator_1.operator_id, + ) + .await + .unwrap(); + + let bls_signature_2 = test_operator_2 + .bls_keypair + .sign_message(task_response_digest.as_ref()); + bls_agg_service + .process_new_signature( + task_index, + task_response_digest, + bls_signature_2.clone(), + test_operator_2.operator_id, + ) + .await + .unwrap(); + + let quorum_apks_g1 = aggregate_g1_public_keys(&test_operators); + let signers_apk_g2 = aggregate_g2_public_keys(&test_operators); + let signers_agg_sig_g1 = aggregate_g1_signatures(&[bls_signature_1, bls_signature_2]); + let expected_agg_service_response = BlsAggregationServiceResponse { + task_index, + task_response_digest, + non_signers_pub_keys_g1: vec![], + quorum_apks_g1: vec![quorum_apks_g1], + signers_apk_g2, + signers_agg_sig_g1, + non_signer_quorum_bitmap_indices: vec![], + quorum_apk_indices: vec![], + total_stake_indices: vec![], + non_signer_stake_indices: vec![], + }; + + let response = bls_agg_service + .aggregated_response_receiver + .lock() + .await + .recv() + .await; + + assert_eq!( + expected_agg_service_response, + response.clone().unwrap().unwrap() + ); + assert_eq!(task_index, response.unwrap().unwrap().task_index); + } + #[tokio::test] async fn test_1_quorum_3_operator_3_correct_signatures() { let test_operator_1 = TestOperator { From 2edb7fa0d8e74415fe80ab58ee91f3ee053ec73e Mon Sep 17 00:00:00 2001 From: tomasarrachea Date: Wed, 25 Sep 2024 15:51:43 -0300 Subject: [PATCH 3/3] prevent duplicate signature processing --- .../services/bls_aggregation/src/bls_agg.rs | 32 ++++++++++++++++--- crates/types/src/avs.rs | 2 ++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/crates/services/bls_aggregation/src/bls_agg.rs b/crates/services/bls_aggregation/src/bls_agg.rs index 48358d712..80147481c 100644 --- a/crates/services/bls_aggregation/src/bls_agg.rs +++ b/crates/services/bls_aggregation/src/bls_agg.rs @@ -316,6 +316,24 @@ impl BlsAggregatorService }) .map_err(|_| BlsAggregationServiceError::TaskExpired)? { + // check if the operator has already signed for this digest + if aggregated_operators + .get(&signed_task_digest.task_response_digest) + .map(|operators| { + operators + .signers_operator_ids_set + .contains_key(&signed_task_digest.operator_id) + }) + .unwrap_or(false) + { + signed_task_digest + .signature_verification_channel + .send(Err(SignatureVerificationError::DuplicateSignature)) + .await + .map_err(|_| BlsAggregationServiceError::ChannelError)?; + continue; + } + let verification_result = BlsAggregatorService::::verify_signature( task_index, &signed_task_digest, @@ -548,7 +566,7 @@ mod tests { use alloy_primitives::{B256, U256}; use eigen_crypto_bls::{BlsG1Point, BlsG2Point, BlsKeyPair, Signature}; use eigen_services_avsregistry::fake_avs_registry_service::FakeAvsRegistryService; - use eigen_types::avs::SignatureVerificationError::IncorrectSignature; + use eigen_types::avs::SignatureVerificationError::{DuplicateSignature, IncorrectSignature}; use eigen_types::operator::{QuorumNum, QuorumThresholdPercentages}; use eigen_types::{avs::TaskIndex, test::TestOperator}; use sha2::{Digest, Sha256}; @@ -716,15 +734,21 @@ mod tests { .await .unwrap(); - bls_agg_service + let second_signature_processing_result = bls_agg_service .process_new_signature( task_index, task_response_digest, bls_signature_1.clone(), test_operator_1.operator_id, ) - .await - .unwrap(); + .await; + + assert_eq!( + second_signature_processing_result, + Err(BlsAggregationServiceError::SignatureVerificationError( + DuplicateSignature + )) + ); let bls_signature_2 = test_operator_2 .bls_keypair diff --git a/crates/types/src/avs.rs b/crates/types/src/avs.rs index 24a457c5f..1052fc188 100644 --- a/crates/types/src/avs.rs +++ b/crates/types/src/avs.rs @@ -15,6 +15,8 @@ pub enum SignatureVerificationError { OperatorPublicKeyNotFound, #[error("operator not found")] OperatorNotFound, + #[error("duplicate signature error")] + DuplicateSignature, } /// Represents a signed task response digest