feat: proper threshold handling for robust ecdsa, adapted tests

Author: gilcu3

crates/contract/src/primitives/thresholds.rs

@@ -61,8 +61,8 @@ impl ThresholdParameters {
         if k.value() < MIN_THRESHOLD_ABSOLUTE {
             return Err(InvalidThreshold::MinAbsRequirementFailed.into());
         }
-        // TODO: this is just a temporary fix else I cannot make tests work for robust-ecdsa
-        let percentage_bound = (2 * n_shares).div_ceil(5); // minimum 60%
+
+        let percentage_bound = (3 * n_shares).div_ceil(5); // minimum 60%
         if k.value() < percentage_bound {
             return Err(InvalidThreshold::MinRelRequirementFailed.message(format!(
                 "require at least {}, found {:?}",
@@ -183,7 +183,7 @@ mod tests {
     #[test]
     fn test_validate_threshold() {
         let n = rand::thread_rng().gen_range(2..600) as u64;
-        let min_threshold = ((n as f64) * 0.4).ceil() as u64;
+        let min_threshold = ((n as f64) * 0.6).ceil() as u64;
         for k in 0..min_threshold {
             assert!(ThresholdParameters::validate_threshold(n, Threshold::new(k)).is_err());
         }
@@ -196,7 +196,7 @@ mod tests {
     #[test]
     fn test_threshold_parameters_constructor() {
         let n: usize = rand::thread_rng().gen_range(2..600);
-        let min_threshold = ((n as f64) * 0.4).ceil() as usize;
+        let min_threshold = ((n as f64) * 0.6).ceil() as usize;
 
         let participants = gen_participants(n);
         for k in 1..min_threshold {

crates/node/src/key_events.rs

@@ -68,7 +68,6 @@ pub async fn keygen_computation_inner(
         SignatureScheme::V2Secp256k1 => {
             let keyshare =
                 RobustEcdsaSignatureProvider::run_key_generation_client(threshold, channel).await?;
-            println!("did we compute?");
             let public_key = keyshare.public_key.into_contract_interface_type();
             (KeyshareData::V2Secp256k1(keyshare), public_key)
         }

crates/node/src/providers/robust_ecdsa.rs

@@ -129,7 +129,10 @@ impl SignatureProvider for RobustEcdsaSignatureProvider {
         threshold: usize,
         channel: NetworkTaskChannel,
     ) -> anyhow::Result<Self::KeygenOutput> {
-        EcdsaSignatureProvider::run_key_generation_client_internal(threshold, channel).await
+        let number_of_participants = channel.participants().len();
+        let robust_ecdsa_threshold = translate_threshold(threshold, number_of_participants);
+        EcdsaSignatureProvider::run_key_generation_client_internal(robust_ecdsa_threshold, channel)
+            .await
     }
 
     async fn run_key_resharing_client(
@@ -139,11 +142,23 @@ impl SignatureProvider for RobustEcdsaSignatureProvider {
         old_participants: &ParticipantsConfig,
         channel: NetworkTaskChannel,
     ) -> anyhow::Result<Self::KeygenOutput> {
+        let number_of_participants = channel.participants().len();
+        let new_robust_ecdsa_threshold = translate_threshold(new_threshold, number_of_participants);
+
+        // This is a bad hack, but cannot think of a better way to solve it, as the struct
+        // comes directly from generic implementations, so probably this is the best place
+        // to do so anyway
+        let mut old_participants_patched = old_participants.clone();
+        old_participants_patched.threshold = translate_threshold(
+            old_participants.threshold as usize,
+            old_participants.participants.len(),
+        ) as u64;
+
         EcdsaSignatureProvider::run_key_resharing_client_internal(
-            new_threshold,
+            new_robust_ecdsa_threshold,
             my_share,
             public_key,
-            old_participants,
+            &old_participants_patched,
             channel,
         )
         .await
@@ -205,3 +220,19 @@ impl SignatureProvider for RobustEcdsaSignatureProvider {
         Ok(())
     }
 }
+
+pub(super) fn get_number_of_signers(threshold: usize, _number_of_participants: usize) -> usize {
+    // TODO: this is the case for the other schemes, might not be our best choice for this one
+    threshold
+}
+
+/// This function translates the current threshold from the contract
+/// to the threshold expected by the robust-ecdsa scheme, which
+/// is semantically different.
+/// The function should be no longer needed when these issues are solved:
+/// https://github.com/near/threshold-signatures/issues/255
+/// https://github.com/near/mpc/issues/1649
+pub(super) fn translate_threshold(threshold: usize, number_of_participants: usize) -> usize {
+    let number_of_signers = get_number_of_signers(threshold, number_of_participants);
+    (number_of_signers - 1) / 2
+}

crates/node/src/providers/robust_ecdsa/presign.rs

@@ -7,7 +7,8 @@ use crate::network::{MeshNetworkClient, NetworkTaskChannel};
 use crate::primitives::{ParticipantId, UniqueId};
 use crate::protocol::run_protocol;
 use crate::providers::robust_ecdsa::{
-    KeygenOutput, RobustEcdsaSignatureProvider, RobustEcdsaTaskId,
+    get_number_of_signers, translate_threshold, KeygenOutput, RobustEcdsaSignatureProvider,
+    RobustEcdsaTaskId,
 };
 use crate::providers::HasParticipants;
 use crate::tracking::AutoAbortTaskCollection;
@@ -92,9 +93,9 @@ impl RobustEcdsaSignatureProvider {
             .collect();
 
         let threshold = mpc_config.participants.threshold as usize;
-
-        // TODO: we need to guarantee this is less than the total number of participants in the contract
-        anyhow::ensure!(running_participants.len() >= 2 * threshold + 1);
+        let num_signers = get_number_of_signers(threshold, running_participants.len());
+        let robust_ecdsa_threshold = translate_threshold(threshold, running_participants.len());
+        anyhow::ensure!(robust_ecdsa_threshold * 2 + 1 <= num_signers);
 
         loop {
             progress_tracker.update_progress();
@@ -114,13 +115,13 @@ impl RobustEcdsaSignatureProvider {
             {
                 let id = presignature_store.generate_and_reserve_id();
                 let participants = match client.select_random_active_participants_including_me(
-                    2 * threshold + 1,
+                    num_signers,
                     &running_participants,
                 ) {
                     Ok(participants) => participants,
                     Err(e) => {
                         tracing::warn!(
-                            "Can't choose active participants for a triple: {}. Sleeping.",
+                            "Can't choose active participants for a robust-ecdsa presignature: {}. Sleeping.",
                             e
                         );
                         // that should not happen often, so sleeping here is okay
@@ -139,7 +140,7 @@ impl RobustEcdsaSignatureProvider {
                     let _in_flight = in_flight;
                     let _semaphore_guard = parallelism_limiter.acquire().await?;
                     let presignature = PresignComputation {
-                        threshold,
+                        threshold: robust_ecdsa_threshold,
                         keygen_out,
                     }
                     .perform_leader_centric_computation(
@@ -177,8 +178,12 @@ impl RobustEcdsaSignatureProvider {
     ) -> anyhow::Result<()> {
         let domain_data = self.domain_data(domain_id)?;
 
+        let number_of_participants = self.mpc_config.participants.participants.len();
+        let threshold = self.mpc_config.participants.threshold as usize;
+        let robust_ecdsa_threshold = translate_threshold(threshold, number_of_participants);
+
         FollowerPresignComputation {
-            threshold: self.mpc_config.participants.threshold as usize,
+            threshold: robust_ecdsa_threshold,
             keygen_out: domain_data.keyshare,
             out_presignature_store: domain_data.presignature_store,
             out_presignature_id: id,
@@ -224,6 +229,11 @@ impl MpcLeaderCentricComputation<PresignOutput> for PresignComputation {
             .map(Participant::from)
             .collect::<Vec<_>>();
         let me = channel.my_participant_id();
+        println!(
+            "computing presign {} {}",
+            self.threshold,
+            cs_participants.len()
+        );
         let protocol = presign(
             &cs_participants,
             me.into(),

crates/node/src/tests/basic_cluster.rs

@@ -14,8 +14,8 @@ use near_time::Clock;
 #[tokio::test]
 async fn test_basic_cluster() {
     init_integration_logger();
-    const NUM_PARTICIPANTS: usize = 5;
-    const THRESHOLD: usize = 2;
+    const NUM_PARTICIPANTS: usize = 6;
+    const THRESHOLD: usize = 5;
     const TXN_DELAY_BLOCKS: u64 = 1;
     let temp_dir = tempfile::tempdir().unwrap();
     let mut setup: IntegrationTestSetup = IntegrationTestSetup::new(
@@ -50,15 +50,17 @@ async fn test_basic_cluster() {
         scheme: SignatureScheme::V2Secp256k1,
     };
 
+    let domains = vec![
+        signature_domain_ecdsa.clone(),
+        signature_domain_eddsa.clone(),
+        ckd_domain.clone(),
+        signature_domain_robust_ecdsa.clone(),
+    ];
+
     {
         let mut contract = setup.indexer.contract_mut().await;
         contract.initialize(setup.participants.clone());
-        contract.add_domains(vec![
-            signature_domain_ecdsa.clone(),
-            signature_domain_eddsa.clone(),
-            ckd_domain.clone(),
-            signature_domain_robust_ecdsa.clone(),
-        ]);
+        contract.add_domains(domains.clone());
     }
 
     let _runs = setup
@@ -71,7 +73,7 @@ async fn test_basic_cluster() {
         .indexer
         .wait_for_contract_state(
             |state| matches!(state, ContractState::Running(_)),
-            DEFAULT_MAX_PROTOCOL_WAIT_TIME * 3,
+            DEFAULT_MAX_PROTOCOL_WAIT_TIME * domains.len() as u32,
         )
         .await
         .expect("timeout waiting for keygen to complete");

crates/node/src/tests/multidomain.rs

@@ -14,8 +14,8 @@ use near_time::Clock;
 #[tokio::test]
 async fn test_basic_multidomain() {
     init_integration_logger();
-    const NUM_PARTICIPANTS: usize = 7;
-    const THRESHOLD: usize = 3;
+    const NUM_PARTICIPANTS: usize = 6;
+    const THRESHOLD: usize = 5;
     const TXN_DELAY_BLOCKS: u64 = 1;
     let temp_dir = tempfile::tempdir().unwrap();
     let mut setup = IntegrationTestSetup::new(
@@ -65,7 +65,7 @@ async fn test_basic_multidomain() {
         .indexer
         .wait_for_contract_state(
             |state| matches!(state, ContractState::Running(_)),
-            DEFAULT_MAX_PROTOCOL_WAIT_TIME * 3,
+            DEFAULT_MAX_PROTOCOL_WAIT_TIME * domains.len() as u32,
         )
         .await
         .expect("must not exceed timeout");
@@ -97,29 +97,29 @@ async fn test_basic_multidomain() {
             }
         }
     }
+    let new_domains = vec![
+        DomainConfig {
+            id: DomainId(4),
+            scheme: SignatureScheme::Ed25519,
+        },
+        DomainConfig {
+            id: DomainId(5),
+            scheme: SignatureScheme::Secp256k1,
+        },
+        DomainConfig {
+            id: DomainId(6),
+            scheme: SignatureScheme::Bls12381,
+        },
+        DomainConfig {
+            id: DomainId(7),
+            scheme: SignatureScheme::V2Secp256k1,
+        },
+    ];
 
     {
-        let new_domains = vec![
-            DomainConfig {
-                id: DomainId(4),
-                scheme: SignatureScheme::Ed25519,
-            },
-            DomainConfig {
-                id: DomainId(5),
-                scheme: SignatureScheme::Secp256k1,
-            },
-            DomainConfig {
-                id: DomainId(6),
-                scheme: SignatureScheme::Bls12381,
-            },
-            DomainConfig {
-                id: DomainId(7),
-                scheme: SignatureScheme::V2Secp256k1,
-            },
-        ];
         let mut contract = setup.indexer.contract_mut().await;
         contract.add_domains(new_domains.clone());
-        domains.extend(new_domains);
+        domains.extend(new_domains.clone());
     }
     setup
         .indexer
@@ -134,7 +134,7 @@ async fn test_basic_multidomain() {
         .indexer
         .wait_for_contract_state(
             |state| matches!(state, ContractState::Running(_)),
-            DEFAULT_MAX_PROTOCOL_WAIT_TIME * 3,
+            DEFAULT_MAX_PROTOCOL_WAIT_TIME * new_domains.len() as u32,
         )
         .await
         .expect("must not exceed timeout");
@@ -183,7 +183,7 @@ async fn test_basic_multidomain() {
                     }
                 }
             },
-            DEFAULT_MAX_PROTOCOL_WAIT_TIME * 4,
+            DEFAULT_MAX_PROTOCOL_WAIT_TIME * domains.len() as u32,
         )
         .await
         .expect("must not exceed timeout");

crates/node/src/tests/resharing.rs

@@ -17,8 +17,8 @@ use super::DEFAULT_BLOCK_TIME;
 #[tokio::test]
 async fn test_key_resharing_simple() {
     init_integration_logger();
-    const NUM_PARTICIPANTS: usize = 5;
-    const THRESHOLD: usize = 2;
+    const NUM_PARTICIPANTS: usize = 6;
+    const THRESHOLD: usize = 5;
     const TXN_DELAY_BLOCKS: u64 = 1;
     let temp_dir = tempfile::tempdir().unwrap();
     let mut setup = IntegrationTestSetup::new(
@@ -76,6 +76,15 @@ async fn test_key_resharing_simple() {
         .map(|config| AutoAbortTask::from(tokio::spawn(config.run())))
         .collect::<Vec<_>>();
 
+    setup
+        .indexer
+        .wait_for_contract_state(
+            |state| matches!(state, ContractState::Running(_)),
+            DEFAULT_MAX_PROTOCOL_WAIT_TIME * domains.len() as u32,
+        )
+        .await
+        .expect("must not exceed timeout");
+
     // Sanity check.
     assert!(request_signature_and_await_response(
         &mut setup.indexer,
@@ -102,7 +111,7 @@ async fn test_key_resharing_simple() {
                 }
                 _ => false,
             },
-            DEFAULT_MAX_PROTOCOL_WAIT_TIME,
+            DEFAULT_MAX_PROTOCOL_WAIT_TIME * domains.len() as u32,
         )
         .await
         .expect("Timeout waiting for resharing to complete");