use actual DKG participants when finalizing DKG

src/consensus/dkg.rs

@@ -44,31 +44,23 @@ pub fn generate_secret_key_set(rng: &mut MainRng, participants: usize) -> bls::S
 #[derive(Clone)]
 /// DKG result
 pub struct DkgResult {
+    /// Actual participants of the DKG. Excludes the non-participating nodes.
+    pub participants: BTreeSet<PublicId>,
     /// Public key set to verify threshold signatures
     pub public_key_set: PublicKeySet,
     /// Secret Key share: None if the node was not participating in the DKG and did not receive
     /// encrypted shares.
     pub secret_key_share: Option<SecretKeyShare>,
 }
 
-impl DkgResult {
-    /// Create DkgResult from components
-    pub fn new(public_key_set: PublicKeySet, secret_key_share: Option<SecretKeyShare>) -> Self {
-        Self {
-            public_key_set,
-            secret_key_share,
-        }
-    }
-}
-
 impl Debug for DkgResult {
     fn fmt(&self, formatter: &mut Formatter) -> fmt::Result {
-        write!(
-            formatter,
-            "DkgResult({:?}, {})",
-            self.public_key_set,
-            self.secret_key_share.is_some()
-        )
+        formatter
+            .debug_struct("DkgResult")
+            .field("participants", &self.participants)
+            .field("public_key", &self.public_key_set.public_key())
+            .field("secret_key_share", &self.secret_key_share.is_some())
+            .finish()
     }
 }
 
@@ -113,11 +105,12 @@ impl DkgVoter {
         for (key, key_gen) in self.key_gen_map.iter_mut() {
             if key_gen.is_finalized() {
                 let dkg_key = (key.0.clone(), key.1);
-                if let Some((_participants, dkg_outcome)) = key_gen.generate_keys() {
-                    let dkg_result = DkgResult::new(
-                        dkg_outcome.public_key_set,
-                        Some(dkg_outcome.secret_key_share),
-                    );
+                if let Some((participants, dkg_outcome)) = key_gen.generate_keys() {
+                    let dkg_result = DkgResult {
+                        participants,
+                        public_key_set: dkg_outcome.public_key_set,
+                        secret_key_share: Some(dkg_outcome.secret_key_share),
+                    };
                     let _ = completed.insert(dkg_key.clone(), dkg_result);
                 }
             }

src/messages/variant.rs

@@ -10,13 +10,15 @@ use super::{AccumulatingMessage, Message, MessageHash, VerifyStatus};
 use crate::{
     consensus::{DkgKey, ProofShare, Proven, Vote},
     error::{Result, RoutingError},
+    id::PublicId,
     relocation::{RelocateDetails, RelocatePayload, RelocatePromise},
     section::{EldersInfo, SectionProofChain, SharedState, TrustStatus},
 };
 use bytes::Bytes;
 use hex_fmt::HexFmt;
 use serde::Serialize;
 use std::{
+    collections::BTreeSet,
     fmt::{self, Debug, Formatter},
     net::SocketAddr,
 };
@@ -84,6 +86,8 @@ pub(crate) enum Variant {
     DKGOldElders {
         /// The identifier of the key_gen instance this message is about.
         dkg_key: DkgKey,
+        /// The actual DKG participants.
+        participants: BTreeSet<PublicId>,
         /// Public key set that got consensused
         public_key_set: bls::PublicKeySet,
     },
@@ -168,18 +172,18 @@ impl Debug for Variant {
                 .finish(),
             Self::DKGMessage { dkg_key, message } => f
                 .debug_struct("DKGMessage")
-                .field("participants", &dkg_key.0)
-                .field("section_key_index", &dkg_key.1)
+                .field("dkg_key", &dkg_key)
                 .field("message_hash", &MessageHash::from_bytes(message))
                 .finish(),
             Self::DKGOldElders {
                 dkg_key,
+                participants,
                 public_key_set,
             } => f
                 .debug_struct("DKGOldElders")
-                .field("participants", &dkg_key.0)
-                .field("section_key_index", &dkg_key.1)
-                .field("public_key_set", public_key_set)
+                .field("dkg_key", &dkg_key)
+                .field("participants", &participants)
+                .field("public_key", &public_key_set.public_key())
                 .finish(),
             Self::Vote {
                 content,

src/node/mod.rs

@@ -703,11 +703,13 @@ impl Node {
                 }
                 Variant::DKGOldElders {
                     dkg_key,
+                    participants,
                     public_key_set,
                 } => {
                     stage.handle_dkg_old_elders(
                         &mut self.core,
                         dkg_key,
+                        participants.clone(),
                         public_key_set.clone(),
                         *msg.src().as_node()?,
                     )?;

src/node/stage/approved.rs

@@ -267,7 +267,12 @@ impl Approved {
 
         for (dkg_key, dkg_result) in completed {
             debug!("Completed DKG {:?}", dkg_key);
-            self.notify_old_elders(core, &dkg_key, dkg_result.public_key_set.clone());
+            self.notify_old_elders(
+                core,
+                &dkg_key,
+                &dkg_result.participants,
+                &dkg_result.public_key_set,
+            );
 
             if let Err(err) = self.handle_dkg_result_event(core, &dkg_key, &dkg_result) {
                 debug!("Failed handle DKG result of {:?} - {:?}", dkg_key, err);
@@ -818,29 +823,32 @@ impl Approved {
         )
     }
 
-    // TODO: accumulate at least quorum of these messages
-    //       and only then proceed to handle the DKG result.
     pub fn handle_dkg_old_elders(
         &mut self,
         core: &mut Core,
         dkg_key: &DkgKey,
+        participants: BTreeSet<PublicId>,
         public_key_set: bls::PublicKeySet,
         src_id: PublicId,
     ) -> Result<()> {
         debug!(
             "notified by DKG participants {:?} to vote for SectionInfo",
-            dkg_key.0
+            participants,
         );
 
         // Accumulate quorum notifications then carry out the further process.
         if !self
             .dkg_voter
-            .add_old_elders_notification(&dkg_key.0, &src_id)
+            .add_old_elders_notification(&participants, &src_id)
         {
             return Ok(());
         }
 
-        let dkg_result = DkgResult::new(public_key_set, None);
+        let dkg_result = DkgResult {
+            participants,
+            public_key_set,
+            secret_key_share: None,
+        };
         if self.dkg_voter.has_info(
             dkg_key,
             &dkg_result,
@@ -1299,28 +1307,33 @@ impl Approved {
     }
 
     fn notify_old_elders(
-        &mut self,
+        &self,
         core: &mut Core,
         dkg_key: &DkgKey,
-        public_key_set: bls::PublicKeySet,
+        participants: &BTreeSet<PublicId>,
+        public_key_set: &bls::PublicKeySet,
     ) {
-        let src = SrcLocation::Node(*core.id().name());
-        let variant = Variant::DKGOldElders {
-            dkg_key: dkg_key.clone(),
-            public_key_set,
-        };
-        let elder_ids: Vec<PublicId> = self.shared_state.our_info().elder_ids().copied().collect();
+        let recipients = self
+            .shared_state
+            .our_info()
+            .elders
+            .values()
+            .filter(|p2p_node| !participants.contains(p2p_node.public_id()));
 
-        for elder_id in elder_ids {
-            if !dkg_key.0.contains(&elder_id) {
-                trace!(
-                    "notify {:?} for the completion of DKG {:?}",
-                    elder_id,
-                    dkg_key.0
-                );
-                let dst = DstLocation::Node(*elder_id.name());
-                let _ = self.send_routing_message(core, src, dst, variant.clone(), None);
-            }
+        trace!(
+            "notify {{{:?}}} for the completion of DKG {:?}",
+            recipients.clone().format(", "),
+            participants,
+        );
+
+        for recipient in recipients {
+            let variant = Variant::DKGOldElders {
+                dkg_key: dkg_key.clone(),
+                participants: participants.clone(),
+                public_key_set: public_key_set.clone(),
+            };
+
+            core.send_direct_message(recipient.peer_addr(), variant)
         }
     }
 
@@ -1340,11 +1353,8 @@ impl Approved {
         if let Some(index) = self.shared_state.our_history.index_of(&public_key) {
             // Our shared state is already up to date, so no need to vote. Just finalize the DKG so
             // we can start using the new secret key share.
-            self.section_keys_provider.finalise_dkg(
-                index - 1,
-                core.name(),
-                self.shared_state.our_info(),
-            );
+            self.section_keys_provider
+                .finalise_dkg(index - 1, core.name());
 
             return Ok(());
         }
@@ -1487,11 +1497,8 @@ impl Approved {
         let old_last_key_index = self.shared_state.our_history.last_key_index();
         let old_prefix = *self.shared_state.our_prefix();
 
-        self.section_keys_provider.finalise_dkg(
-            self.shared_state.our_history.last_key_index(),
-            core.name(),
-            update.our_info(),
-        );
+        self.section_keys_provider
+            .finalise_dkg(self.shared_state.our_history.last_key_index(), core.name());
 
         let neighbour_elders_removed = NeighbourEldersRemoved::builder(&self.shared_state.sections);
 

src/node/tests/elder.rs

@@ -195,6 +195,7 @@ impl Env {
             .collect();
 
         let dkg_result = DkgResult {
+            participants: new_elders_info.elder_ids().copied().collect(),
             public_key_set: new_pk_set.clone(),
             secret_key_share: new_elders_info
                 .position(self.subject.name())

src/section/elders_info.rs

@@ -44,6 +44,7 @@ impl EldersInfo {
     /// Returns the index of the elder with `name` in this set of elders.
     /// This is useful for BLS signatures where the signature share needs to be mapped to a
     /// "field element" which is typically a numeric index.
+    #[cfg(all(test, feature = "mock"))]
     pub(crate) fn position(&self, name: &XorName) -> Option<usize> {
         self.elders.keys().position(|other_name| other_name == name)
     }

src/section/section_keys.rs

@@ -6,7 +6,6 @@
 // KIND, either express or implied. Please review the Licences for the specific language governing
 // permissions and limitations relating to use of the SAFE Network Software.
 
-use super::elders_info::EldersInfo;
 use crate::{
     consensus::DkgResult,
     error::{Result, RoutingError},
@@ -68,32 +67,31 @@ impl SectionKeysProvider {
         });
     }
 
-    pub fn finalise_dkg(
-        &mut self,
-        section_key_index: u64,
-        our_name: &XorName,
-        elders_info: &EldersInfo,
-    ) {
+    pub fn finalise_dkg(&mut self, section_key_index: u64, our_name: &XorName) {
         let dkg_result = if let Some(result) = self.pending.remove(&section_key_index) {
             result
         } else {
             trace!("missing pending DKG result #{}", section_key_index);
             return;
         };
 
-        let public_key_set = dkg_result.public_key_set;
+        let DkgResult {
+            participants,
+            public_key_set,
+            secret_key_share,
+        } = dkg_result;
 
         trace!(
             "finalise DKG result #{}: {:?}",
             section_key_index,
             public_key_set.public_key()
         );
 
-        self.current = dkg_result
-            .secret_key_share
+        self.current = secret_key_share
             .and_then(|secret_key_share| {
-                elders_info
-                    .position(our_name)
+                participants
+                    .iter()
+                    .position(|id| id.name() == our_name)
                     .map(|index| (index, secret_key_share))
             })
             .map(|(index, secret_key_share)| SectionKeyShare {