Repository: tangle-network/gadget (language: rust)

Issue:
[TASK] Signer Selections
### Goal

Given an $`n`$ amount of participants, choose $`t`$ participant _randomly_ to participate and run the signing protocol.

### Random* Selection

This algorithm is not cryptographically random, instead it will use a seed of different values while ensuring that these values are the same across different nodes. Let $`S`$ is the seed, where $`S = keccak256(jobId, retryId, now)`$ then, we can use that seed to pick random participants from the participants list.





Discussion / hints:
Is `now` going to be the same across parties? Also I thought we solved this a long time ago, why not just choose what we did before?
> Is `now` going to be the same across parties? Also I thought we solved this a long time ago, why not just choose what we did before?

Yes, Now is the finality notification block number.
And No, It did not work as expected. Needs a bit of tweaking to get it to work with CGGMP21 Implementation.

Fix patch:
diff --git a/Cargo.lock b/Cargo.lock
index 8daebad7a..3c533e55b 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2143,6 +2143,7 @@ dependencies = [
  "parity-scale-codec 3.6.9",
  "protocol-macros",
  "rand 0.8.5",
+ "rand_chacha 0.3.1",
  "sc-client-api",
  "serde",
  "sp-api",
diff --git a/Cargo.toml b/Cargo.toml
index ee9ba472c..d9c711a51 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -120,6 +120,7 @@ strum = "0.25.0"
 clap = "4.0.32"
 hex-literal = "0.4.1"
 rand = "0.8.5"
+rand_chacha = "0.3.1"
 jsonrpsee = "0.16.2"
 linked-hash-map = "0.5.6"
 k256 = "0.13.2"
diff --git a/gadget-common/src/gadget/mod.rs b/gadget-common/src/gadget/mod.rs
index 28de89e96..1e5461c26 100644
--- a/gadget-common/src/gadget/mod.rs
+++ b/gadget-common/src/gadget/mod.rs
@@ -115,23 +115,23 @@ where
             .query_jobs_by_validator(notification.hash, *self.protocol.account_id())
             .await?;
 
-        log::info!(target: "gadget", "[{}] Found {} jobs for initialization", self.protocol.name(), jobs.len());
+        log::trace!(target: "gadget", "[{}] Found {} jobs for initialization", self.protocol.name(), jobs.len());
         let mut relevant_jobs = Vec::new();
 
         for job in jobs {
             // Job is expired.
             if job.expiry < now_header {
-                log::warn!(target: "gadget", "[{}] The job requested for initialization is expired, skipping submission", self.protocol.name());
+                log::trace!(target: "gadget", "[{}] The job requested for initialization is expired, skipping submission", self.protocol.name());
                 continue;
             }
             // Job is not for this role
             if !self.protocol.role_filter(job.job_type.get_role_type()) {
-                log::debug!(target: "gadget", "[{}] The job {} requested for initialization is not for this role {:?}, skipping submission", self.protocol.name(), job.job_id, job.job_type.get_role_type());
+                log::trace!(target: "gadget", "[{}] The job {} requested for initialization is not for this role {:?}, skipping submission", self.protocol.name(), job.job_id, job.job_type.get_role_type());
                 continue;
             }
             // Job is not for this phase
             if !self.protocol.phase_filter(job.job_type.clone()) {
-                log::debug!(target: "gadget", "[{}] The job {} requested for initialization is not for this phase {:?}, skipping submission", self.protocol.name(), job.job_id, job.job_type);
+                log::trace!(target: "gadget", "[{}] The job {} requested for initialization is not for this phase {:?}, skipping submission", self.protocol.name(), job.job_id, job.job_type);
                 continue;
             }
 
@@ -182,7 +182,7 @@ where
         for relevant_job in relevant_jobs {
             let task_id = relevant_job.task_id;
             let retry_id = relevant_job.retry_id;
-            log::info!(target: "gadget", "[{}] Creating job for task {task_id} with retry id {retry_id}", self.protocol.name(), task_id = hex::encode(task_id), retry_id = retry_id);
+            log::trace!(target: "gadget", "[{}] Creating job for task {task_id} with retry id {retry_id}", self.protocol.name(), task_id = hex::encode(task_id), retry_id = retry_id);
             match self.protocol.create_next_job(relevant_job).await {
                 Ok(params) => {
                     match self
@@ -215,6 +215,10 @@ where
                     }
                 }
 
+                Err(Error::ParticipantNotSelected { id, reason }) => {
+                    log::debug!(target: "gadget", "[{}] Participant {id} not selected for job {task_id} with retry id {retry_id} because {reason}", self.protocol.name(), id = id, task_id = hex::encode(task_id), retry_id = retry_id, reason = reason);
+                }
+
                 Err(err) => {
                     self.protocol
                         .logger()
@@ -266,6 +270,8 @@ where
 {
     /// Given an input of a valid and relevant job, return the parameters needed to start the async protocol
     /// Note: the parameters returned must be relevant to the `AsyncProtocol` implementation of this protocol
+    ///
+    /// In case the participant is not selected for some reason, return an [`Error::ParticipantNotSelected`]
     async fn create_next_job(
         &self,
         job: JobInitMetadata<B>,
diff --git a/gadget-common/src/lib.rs b/gadget-common/src/lib.rs
index 6d7355cf9..845b861bb 100644
--- a/gadget-common/src/lib.rs
+++ b/gadget-common/src/lib.rs
@@ -47,6 +47,7 @@ pub enum Error {
     MissingNetworkId,
     PeerNotFound { id: AccountId },
     JoinError { err: JoinError },
+    ParticipantNotSelected { id: AccountId, reason: String },
 }
 
 impl Display for Error {
diff --git a/protocols/dfns-cggmp21/Cargo.toml b/protocols/dfns-cggmp21/Cargo.toml
index c175be97f..5478fcdb1 100644
--- a/protocols/dfns-cggmp21/Cargo.toml
+++ b/protocols/dfns-cggmp21/Cargo.toml
@@ -33,6 +33,7 @@ parity-scale-codec = { workspace = true }
 
 serde = { version = "1.0.193", features = ["derive"] }
 rand = { workspace = true }
+rand_chacha = { workspace = true }
 hex = { workspace = true }
 
 [dev-dependencies]
diff --git a/protocols/dfns-cggmp21/src/protocols/key_refresh.rs b/protocols/dfns-cggmp21/src/protocols/key_refresh.rs
index d1b6c32d5..c3bc32c6f 100644
--- a/protocols/dfns-cggmp21/src/protocols/key_refresh.rs
+++ b/protocols/dfns-cggmp21/src/protocols/key_refresh.rs
@@ -274,7 +274,7 @@ where
                 let perf_report = tracer.get_report().map_err(|err| JobError {
                     reason: format!("KeyRefresh protocol error: {err:?}"),
                 })?;
-                logger.debug(format!("KeyRefresh protocol report: {perf_report}"));
+                logger.trace(format!("KeyRefresh protocol report: {perf_report}"));
 
                 logger.debug("Finished AsyncProtocol - KeyRefresh");
 
diff --git a/protocols/dfns-cggmp21/src/protocols/key_rotate.rs b/protocols/dfns-cggmp21/src/protocols/key_rotate.rs
index 79ea1eb54..e37539ede 100644
--- a/protocols/dfns-cggmp21/src/protocols/key_rotate.rs
+++ b/protocols/dfns-cggmp21/src/protocols/key_rotate.rs
@@ -86,20 +86,15 @@ where
 
         let phase1_job = job.phase1_job.expect("Should exist for a phase 2 job");
         let participants = phase1_job.clone().get_participants().expect("Should exist");
-        let threshold = phase1_job.get_threshold().expect("Should exist") as u16;
-
-        let i = participants
-            .iter()
-            .position(|p| p == &self.account_id)
-            .expect("Should exist") as u16;
-        let signers = participants
-            .iter()
-            .enumerate()
-            .take(threshold as usize)
-            .map(|(i, _)| i as u16)
-            .collect::<Vec<_>>();
-
-        log::debug!(target: "gadget", "Fetching key for job ID: {new_phase_one_id}");
+        let t = phase1_job.get_threshold().expect("Should exist") as u16;
+
+        let seed =
+            keccak_256(&[&job_id.to_be_bytes()[..], &job.retry_id.to_be_bytes()[..]].concat());
+        let mut rng = rand_chacha::ChaChaRng::from_seed(seed);
+
+        let (i, signers, mapping) =
+            super::util::choose_signers(&mut rng, &self.account_id, &participants, t)?;
+
         let new_phase_one_result = self
             .client
             .query_job_result(job.at, job.role_type, new_phase_one_id)
@@ -128,38 +123,21 @@ where
                 err: format!("No key found for job ID: {job_id:?}"),
             })?;
 
-        if participants.contains(&self.account_id) && signers.contains(&i) {
-            let user_id_to_account_id_mapping = Arc::new(
-                participants
-                    .clone()
-                    .into_iter()
-                    .enumerate()
-                    .map(|r| (r.0 as UserID, r.1))
-                    .collect(),
-            );
-
-            let params = DfnsCGGMP21KeyRotateExtraParams {
-                i,
-                t: threshold,
-                signers,
-                job_id,
-                phase_one_id,
-                new_phase_one_id,
-                role_type: job.role_type,
-                key,
-                new_key,
-                user_id_to_account_id_mapping,
-            };
-            Ok(params)
-        } else {
-            Err(gadget_common::Error::ClientError {
-                err: format!(
-                    "Account ID {account_id:?} is not a participant or signer for job {job_id:?}",
-                    account_id = self.account_id,
-                    job_id = job_id
-                ),
-            })
-        }
+        let user_id_to_account_id_mapping = Arc::new(mapping);
+
+        let params = DfnsCGGMP21KeyRotateExtraParams {
+            i,
+            t,
+            signers,
+            job_id,
+            phase_one_id,
+            new_phase_one_id,
+            role_type: job.role_type,
+            key,
+            new_key,
+            user_id_to_account_id_mapping,
+        };
+        Ok(params)
     }
 
     async fn process_block_import_notification(
diff --git a/protocols/dfns-cggmp21/src/protocols/keygen.rs b/protocols/dfns-cggmp21/src/protocols/keygen.rs
index 74b46479d..40606531c 100644
--- a/protocols/dfns-cggmp21/src/protocols/keygen.rs
+++ b/protocols/dfns-cggmp21/src/protocols/keygen.rs
@@ -257,7 +257,7 @@ where
                 let perf_report = tracer.get_report().map_err(|err| JobError {
                     reason: format!("Keygen protocol error: {err:?}"),
                 })?;
-                logger.debug(format!("Incomplete Keygen protocol report: {perf_report}"));
+                logger.trace(format!("Incomplete Keygen protocol report: {perf_report}"));
                 logger.debug("Finished AsyncProtocol - Incomplete Keygen");
 
                 let (
@@ -298,7 +298,7 @@ where
                 let perf_report = tracer.get_report().map_err(|err| JobError {
                     reason: format!("Aux info protocol error: {err:?}"),
                 })?;
-                logger.debug(format!("Aux info protocol report: {perf_report}"));
+                logger.trace(format!("Aux info protocol report: {perf_report}"));
                 logger.debug("Finished AsyncProtocol - Aux Info");
 
                 let key_share = dfns_cggmp21::KeyShare::make(incomplete_key_share, aux_info)
diff --git a/protocols/dfns-cggmp21/src/protocols/sign.rs b/protocols/dfns-cggmp21/src/protocols/sign.rs
index 30de863ea..2fa2e771b 100644
--- a/protocols/dfns-cggmp21/src/protocols/sign.rs
+++ b/protocols/dfns-cggmp21/src/protocols/sign.rs
@@ -86,21 +86,14 @@ where
 
         let phase1_job = job.phase1_job.expect("Should exist for a phase 2 job");
         let participants = phase1_job.clone().get_participants().expect("Should exist");
-        let threshold = phase1_job.get_threshold().expect("Should exist") as u16;
+        let t = phase1_job.get_threshold().expect("Should exist") as u16;
 
-        let i = participants
-            .iter()
-            .position(|p| p == &self.account_id)
-            .expect("Should exist") as u16;
-        // TODO: decide how we will pick the signers.
-        // For now, we will just pick the first t participants.
-        let signers = participants
-            .iter()
-            .enumerate()
-            .take(threshold as usize)
-            .map(|(i, _)| i as u16)
-            .collect::<Vec<_>>();
+        let seed =
+            keccak_256(&[&job_id.to_be_bytes()[..], &job.retry_id.to_be_bytes()[..]].concat());
+        let mut rng = rand_chacha::ChaChaRng::from_seed(seed);
 
+        let (i, signers, mapping) =
+            super::util::choose_signers(&mut rng, &self.account_id, &participants, t)?;
         let key = self
             .key_store
             .get_job_result(previous_job_id)
@@ -112,36 +105,19 @@ where
                 err: format!("No key found for job ID: {job_id:?}"),
             })?;
 
-        if participants.contains(&self.account_id) && signers.contains(&i) {
-            let user_id_to_account_id_mapping = Arc::new(
-                participants
-                    .clone()
-                    .into_iter()
-                    .enumerate()
-                    .map(|r| (r.0 as UserID, r.1))
-                    .collect(),
-            );
+        let user_id_to_account_id_mapping = Arc::new(mapping);
 
-            let params = DfnsCGGMP21SigningExtraParams {
-                i,
-                t: threshold,
-                signers,
-                job_id,
-                role_type: RoleType::Tss(ThresholdSignatureRoleType::DfnsCGGMP21Secp256k1),
-                key,
-                input_data_to_sign,
-                user_id_to_account_id_mapping,
-            };
-            Ok(params)
-        } else {
-            Err(gadget_common::Error::ClientError {
-                err: format!(
-                    "Account ID {account_id:?} is not a participant or signer for job {job_id:?}",
-                    account_id = self.account_id,
-                    job_id = job_id
-                ),
-            })
-        }
+        let params = DfnsCGGMP21SigningExtraParams {
+            i,
+            t,
+            signers,
+            job_id,
+            role_type: job.role_type,
+            key,
+            input_data_to_sign,
+            user_id_to_account_id_mapping,
+        };
+        Ok(params)
     }
 
     async fn process_block_import_notification(
@@ -292,7 +268,7 @@ where
                 let perf_report = tracer.get_report().map_err(|err| JobError {
                     reason: format!("Signing protocol error: {err:?}"),
                 })?;
-                logger.debug(format!("Signing protocol report: {perf_report}"));
+                logger.trace(format!("Signing protocol report: {perf_report}"));
                 // Normalize the signature
                 let signature = signature.normalize_s();
                 logger.debug("Finished AsyncProtocol - Signing");
diff --git a/protocols/dfns-cggmp21/src/protocols/util.rs b/protocols/dfns-cggmp21/src/protocols/util.rs
index 0bba23ff8..3146752be 100644
--- a/protocols/dfns-cggmp21/src/protocols/util.rs
+++ b/protocols/dfns-cggmp21/src/protocols/util.rs
@@ -8,6 +8,7 @@ use gadget_common::gadget::message::{GadgetProtocolMessage, UserID};
 use gadget_common::gadget::network::Network;
 use gadget_common::gadget::work_manager::WorkManager;
 use gadget_core::job_manager::WorkManagerInterface;
+use rand::seq::SliceRandom;
 use serde::de::DeserializeOwned;
 use serde::{Deserialize, Serialize};
 use std::collections::HashMap;
@@ -451,3 +452,56 @@ fn get_to_and_from_account_id(
 
     (to_account_id, from_account_id)
 }
+
+/// Given a list of participants, choose `t` of them and return the index of the current participant
+/// and the indices of the chosen participants, as well as a mapping from the index to the account
+/// id.
+///
+/// # Errors
+/// If we are not selected to sign the message it will return an error
+/// [`gadget_common::Error::ParticipantNotSelected`].
+///
+/// # Panics
+/// If the current participant is not in the list of participants it will panic.
+pub fn choose_signers<R: rand::Rng>(
+    rng: &mut R,
+    my_account_id: &AccountId,
+    participants: &[AccountId],
+    t: u16,
+) -> Result<(u16, Vec<u16>, HashMap<UserID, AccountId>), gadget_common::Error> {
+    let selected_participants = participants
+        .choose_multiple(rng, t as usize)
+        .cloned()
+        .collect::<Vec<_>>();
+
+    let selected_participants_indices = selected_participants
+        .iter()
+        .map(|p| participants.iter().position(|x| x == p).unwrap() as u16)
+        .collect::<Vec<_>>();
+
+    let j = participants
+        .iter()
+        .position(|p| p == my_account_id)
+        .expect("Should exist") as u16;
+
+    let i = selected_participants_indices
+        .iter()
+        .position(|p| p == &j)
+        .map(|i| i as u16)
+        .ok_or_else(|| gadget_common::Error::ParticipantNotSelected {
+            id: *my_account_id,
+            reason: String::from("we are not selected to sign"),
+        })?;
+
+    let user_id_to_account_id_mapping = selected_participants
+        .clone()
+        .into_iter()
+        .enumerate()
+        .map(|(i, p)| (i as UserID, p))
+        .collect();
+    Ok((
+        i,
+        selected_participants_indices,
+        user_id_to_account_id_mapping,
+    ))
+}


Test patch: