[bls12381/dkg] Check Commitments (Tolerate Equivocation Attack) (#1813)

Author: patrick-ogrady

cryptography/src/bls12381/benches/dkg_recovery.rs

@@ -8,7 +8,7 @@ use commonware_cryptography::{
 use commonware_utils::quorum;
 use criterion::{criterion_group, BatchSize, Criterion};
 use rand::{rngs::StdRng, SeedableRng};
-use std::{collections::HashMap, hint::black_box};
+use std::{collections::BTreeMap, hint::black_box};
 
 /// Concurrency isn't used in DKG recovery, so we set it to 1.
 const CONCURRENCY: usize = 1;
@@ -43,7 +43,7 @@ fn benchmark_dkg_recovery(c: &mut Criterion) {
                     );
 
                     // Create commitments and send shares to player
-                    let mut commitments = HashMap::new();
+                    let mut commitments = BTreeMap::new();
                     for (idx, dealer) in contributors.iter().take(t as usize).enumerate() {
                         let (_, commitment, shares) =
                             Dealer::<_, MinSig>::new(&mut rng, None, contributors.clone());
@@ -55,7 +55,7 @@ fn benchmark_dkg_recovery(c: &mut Criterion) {
                     (player, commitments)
                 },
                 |(player, commitments)| {
-                    black_box(player.finalize(commitments, HashMap::new()).unwrap());
+                    black_box(player.finalize(commitments, BTreeMap::new()).unwrap());
                 },
                 BatchSize::SmallInput,
             );

cryptography/src/bls12381/benches/dkg_reshare_recovery.rs

@@ -8,7 +8,7 @@ use commonware_cryptography::{
 use commonware_utils::quorum;
 use criterion::{criterion_group, BatchSize, Criterion};
 use rand::{rngs::StdRng, SeedableRng};
-use std::{collections::HashMap, hint::black_box};
+use std::{collections::BTreeMap, hint::black_box};
 
 // Configure contributors based on context
 #[cfg(not(full_bench))]
@@ -45,7 +45,7 @@ fn benchmark_dkg_reshare_recovery(c: &mut Criterion) {
 
         // Create commitments and send shares to players
         let t = quorum(n as u32);
-        let mut commitments = HashMap::new();
+        let mut commitments = BTreeMap::new();
         for (dealer_idx, dealer) in contributors.iter().take(t as usize).enumerate() {
             let (_, commitment, shares) =
                 Dealer::<_, MinSig>::new(&mut rng, None, contributors.clone());
@@ -66,7 +66,7 @@ fn benchmark_dkg_reshare_recovery(c: &mut Criterion) {
         for player in players {
             outputs.push(
                 player
-                    .finalize(commitments.clone(), HashMap::new())
+                    .finalize(commitments.clone(), BTreeMap::new())
                     .unwrap(),
             );
         }
@@ -88,7 +88,7 @@ fn benchmark_dkg_reshare_recovery(c: &mut Criterion) {
                             );
 
                             // Create commitments and send shares to player
-                            let mut commitments = HashMap::new();
+                            let mut commitments = BTreeMap::new();
                             for (idx, dealer) in contributors.iter().take(t as usize).enumerate() {
                                 let (_, commitment, shares) = Dealer::<_, MinSig>::new(
                                     &mut rng,
@@ -103,7 +103,7 @@ fn benchmark_dkg_reshare_recovery(c: &mut Criterion) {
                             (player, commitments)
                         },
                         |(player, commitments)| {
-                            black_box(player.finalize(commitments, HashMap::new()).unwrap());
+                            black_box(player.finalize(commitments, BTreeMap::new()).unwrap());
                         },
                         BatchSize::SmallInput,
                     );

cryptography/src/bls12381/dkg/arbiter.rs

@@ -56,10 +56,10 @@ pub struct Output<V: Variant> {
     pub public: poly::Public<V>,
 
     /// `2f + 1` commitments used to derive group polynomial.
-    pub commitments: HashMap<u32, poly::Public<V>>,
+    pub commitments: BTreeMap<u32, poly::Public<V>>,
 
     /// Reveals published by dealers of selected commitments.
-    pub reveals: HashMap<u32, Vec<Share>>,
+    pub reveals: BTreeMap<u32, Vec<Share>>,
 }
 
 /// Gather commitments, acknowledgements, and reveals from all dealers.
@@ -233,58 +233,47 @@ impl<P: PublicKey, V: Variant> Arbiter<P, V> {
 
         // If there exist more than `2f + 1` commitments, take the first `2f + 1`
         // sorted by dealer index.
-        let selected = self
-            .commitments
-            .into_iter()
-            .take(dealer_threshold)
-            .collect::<Vec<_>>();
+        let mut commitments = BTreeMap::new();
+        let mut reveals = BTreeMap::new();
+        for (dealer_idx, (commitment, _, shares)) in
+            self.commitments.into_iter().take(dealer_threshold)
+        {
+            commitments.insert(dealer_idx, commitment);
+
+            // If there are no reveals for dealer, skip
+            if shares.is_empty() {
+                continue;
+            }
+            reveals.insert(dealer_idx, shares);
+        }
 
         // Recover group
         let public = match self.previous {
             Some(previous) => {
-                let mut commitments = BTreeMap::new();
-                for (dealer_idx, (commitment, _, _)) in &selected {
-                    commitments.insert(*dealer_idx, commitment.clone());
-                }
                 match recover_public::<V>(
                     &previous,
-                    commitments,
+                    &commitments,
                     self.player_threshold,
                     self.concurrency,
                 ) {
                     Ok(public) => public,
                     Err(e) => return (Err(e), self.disqualified),
                 }
             }
-            None => {
-                let mut commitments = Vec::new();
-                for (_, (commitment, _, _)) in &selected {
-                    commitments.push(commitment.clone());
-                }
-                match construct_public::<V>(commitments, self.player_threshold) {
-                    Ok(public) => public,
-                    Err(e) => return (Err(e), self.disqualified),
-                }
-            }
-        };
-
-        // Generate output
-        let mut commitments = HashMap::new();
-        let mut reveals = HashMap::new();
-        for (dealer_idx, (commitment, _, shares)) in selected {
-            commitments.insert(dealer_idx, commitment);
-            if shares.is_empty() {
-                continue;
-            }
-            reveals.insert(dealer_idx, shares);
-        }
-        let output = Output {
-            public,
-            commitments,
-            reveals,
+            None => match construct_public::<V>(commitments.values(), self.player_threshold) {
+                Ok(public) => public,
+                Err(e) => return (Err(e), self.disqualified),
+            },
         };
 
         // Return output
-        (Ok(output), self.disqualified)
+        (
+            Ok(Output {
+                public,
+                commitments,
+                reveals,
+            }),
+            self.disqualified,
+        )
     }
 }