Repository: input-output-hk/mithril
Pull request #472 (2022-09-01 12:32:11)

Avoid panics in 'StmInitializer'
This PR attempts to fix some panics that occur when a `StmInitializer` creates a `StmSigner` if the initializer is not registered.

In that case, the `new_signer` will return an error instead.


Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 528a4ab47..5b306071b 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1835,6 +1835,7 @@ dependencies = [
  "config",
  "hex",
  "httpmock",
+ "mithril",
  "mithril-common",
  "mockall",
  "rand_chacha 0.3.1",
diff --git a/demo/protocol-demo/src/demonstrator.rs b/demo/protocol-demo/src/demonstrator.rs
index 54476af3a..b11c32084 100644
--- a/demo/protocol-demo/src/demonstrator.rs
+++ b/demo/protocol-demo/src/demonstrator.rs
@@ -107,7 +107,7 @@ impl Party {
         let seed = [0u8; 32];
         let mut rng = ChaCha20Rng::from_seed(seed);
         let p = ProtocolInitializer::setup(self.params.unwrap(), self.stake, &mut rng);
-        self.signer = Some(p.new_signer(closed_reg));
+        self.signer = Some(p.new_signer(closed_reg).unwrap());
         self.clerk = Some(ProtocolClerk::from_signer(self.signer.as_ref().unwrap()));
     }
 
diff --git a/mithril-core/benches/size_benches.rs b/mithril-core/benches/size_benches.rs
index c6b4a6c9f..00d7a223d 100644
--- a/mithril-core/benches/size_benches.rs
+++ b/mithril-core/benches/size_benches.rs
@@ -39,7 +39,7 @@ where
 
     let closed_reg = key_reg.close::<H>();
 
-    let signer = ps[0].clone().new_signer(closed_reg);
+    let signer = ps[0].clone().new_signer(closed_reg).unwrap();
     let sig = signer.sign(&msg).unwrap();
 
     println!(
diff --git a/mithril-core/benches/stm.rs b/mithril-core/benches/stm.rs
index 0023c6585..ea8820f3a 100644
--- a/mithril-core/benches/stm.rs
+++ b/mithril-core/benches/stm.rs
@@ -55,7 +55,7 @@ where
 
     let signers = initializers
         .into_par_iter()
-        .map(|p| p.new_signer(closed_reg.clone()))
+        .map(|p| p.new_signer(closed_reg.clone()).unwrap())
         .collect::<Vec<StmSigner<H>>>();
 
     group.bench_function(BenchmarkId::new("Play all lotteries", &param_string), |b| {
diff --git a/mithril-core/examples/key_registration.rs b/mithril-core/examples/key_registration.rs
index 5f448f920..50a8b8282 100644
--- a/mithril-core/examples/key_registration.rs
+++ b/mithril-core/examples/key_registration.rs
@@ -59,10 +59,10 @@ fn main() {
 
     // Now, with information of all participating parties (we can create the Merkle Tree), the
     // signers can be initialised.
-    let party_0 = party_0_init.new_signer(key_reg_0);
-    let party_1 = party_1_init.new_signer(key_reg_1);
-    let party_2 = party_2_init.new_signer(key_reg_2);
-    let party_3 = party_3_init.new_signer(key_reg_3);
+    let party_0 = party_0_init.new_signer(key_reg_0).unwrap();
+    let party_1 = party_1_init.new_signer(key_reg_1).unwrap();
+    let party_2 = party_2_init.new_signer(key_reg_2).unwrap();
+    let party_3 = party_3_init.new_signer(key_reg_3).unwrap();
 
     /////////////////////
     // operation phase //
diff --git a/mithril-core/src/error.rs b/mithril-core/src/error.rs
index f1a79e6f9..ec250b6c5 100644
--- a/mithril-core/src/error.rs
+++ b/mithril-core/src/error.rs
@@ -100,7 +100,7 @@ pub enum MerkleTreeError<D: Digest + FixedOutput> {
 }
 
 /// Errors which can be outputted by key registration.
-#[derive(Debug, Clone, thiserror::Error)]
+#[derive(Debug, Clone, thiserror::Error, PartialEq, Eq)]
 pub enum RegisterError {
     /// This key has already been registered by a participant
     #[error("This key has already been registered.")]
@@ -113,6 +113,10 @@ pub enum RegisterError {
     /// Serialization error
     #[error("Serialization error")]
     SerializationError,
+
+    /// UnregisteredInitializer error
+    #[error("Initializer not registered. Cannot participate as a signer.")]
+    UnregisteredInitializer,
 }
 
 impl<D: Digest + FixedOutput> From<RegisterError> for StmSignatureError<D> {
@@ -121,6 +125,7 @@ impl<D: Digest + FixedOutput> From<RegisterError> for StmSignatureError<D> {
             RegisterError::SerializationError => Self::SerializationError,
             RegisterError::KeyInvalid(e) => Self::IvkInvalid(e.vk),
             RegisterError::KeyRegistered(_) => unreachable!(),
+            RegisterError::UnregisteredInitializer => unreachable!(),
         }
     }
 }
diff --git a/mithril-core/src/key_reg.rs b/mithril-core/src/key_reg.rs
index 3e986e69b..c8a267807 100644
--- a/mithril-core/src/key_reg.rs
+++ b/mithril-core/src/key_reg.rs
@@ -134,6 +134,7 @@ mod tests {
                         assert!(a.check().is_err());
                     }
                     Err(RegisterError::SerializationError) => unreachable!(),
+                    Err(RegisterError::UnregisteredInitializer) => unreachable!(),
                 }
             }
 
diff --git a/mithril-core/src/lib.rs b/mithril-core/src/lib.rs
index 92103f24e..e36df9a13 100644
--- a/mithril-core/src/lib.rs
+++ b/mithril-core/src/lib.rs
@@ -10,4 +10,4 @@ pub mod stm;
 
 mod multi_sig;
 
-pub use crate::error::AggregationError;
+pub use crate::error::{AggregationError, RegisterError};
diff --git a/mithril-core/src/stm.rs b/mithril-core/src/stm.rs
index 8791a684d..028c324d6 100644
--- a/mithril-core/src/stm.rs
+++ b/mithril-core/src/stm.rs
@@ -63,11 +63,11 @@
 //! // Close the key registration.
 //! let closed_reg = key_reg.close();
 //!
-//! // Finialize the StmInitializer and turn it into a StmSigner, which can execute the
+//! // Finalize the StmInitializer and turn it into a StmSigner, which can execute the
 //! // rest of the protocol.
 //! let ps = ps
 //!     .into_par_iter()
-//!     .map(|p| p.new_signer(closed_reg.clone()))
+//!     .map(|p| p.new_signer(closed_reg.clone()).unwrap())
 //!     .collect::<Vec<StmSigner<D>>>();
 //!
 //! /////////////////////
@@ -288,12 +288,12 @@ impl StmInitializer {
     /// * this `StmSigner`'s parameter valuation
     /// * the `avk` as built from the current registered parties (according to the registration service)
     /// * the current total stake (according to the registration service)
-    /// # Panics
+    /// Returns an error
     /// If the initializer is not registered.
     pub fn new_signer<D: Digest + FixedOutput + Clone>(
         self,
         closed_reg: ClosedKeyReg<D>,
-    ) -> StmSigner<D> {
+    ) -> Result<StmSigner<D>, RegisterError> {
         let mut my_index = None;
         for (i, rp) in closed_reg.reg_parties.iter().enumerate() {
             if rp.0 == self.pk.vk {
@@ -301,15 +301,18 @@ impl StmInitializer {
                 break;
             }
         }
-        StmSigner {
-            mt_index: my_index
-                .expect("Initializer not registered. Cannot participate as a signer."),
+        if my_index.is_none() {
+            return Err(RegisterError::UnregisteredInitializer);
+        }
+
+        Ok(StmSigner {
+            mt_index: my_index.unwrap(),
             stake: self.stake,
             params: self.params,
             sk: self.sk,
             vk: self.pk.vk,
             closed_reg,
-        }
+        })
     }
 
     /// Convert to bytes
@@ -454,8 +457,8 @@ impl<D: Clone + Digest + FixedOutput> StmSigner<D> {
     ///     // signers can be initialised (we can create the Merkle Tree).
     ///     // The (closed) key registration is consumed, to ensure that it
     ///     // is not used to initialise a signer at a different epoch.
-    ///     let party_0 = party_0_init_e1.new_signer(party_0_key_reg_e1);
-    ///     let party_1 = party_1_init_e1.new_signer(party_1_key_reg_e1);
+    ///     let party_0 = party_0_init_e1.new_signer(party_0_key_reg_e1).unwrap();
+    ///     let party_1 = party_1_init_e1.new_signer(party_1_key_reg_e1).unwrap();
     ///
     ///     ////////////////////////////////
     ///     //////// EPOCH 2 ///////////////
@@ -904,7 +907,7 @@ mod tests {
             .collect::<Vec<_>>();
         let closed_reg = kr.close();
         ps.into_iter()
-            .map(|p| p.new_signer(closed_reg.clone()))
+            .map(|p| p.new_signer(closed_reg.clone()).unwrap())
             .collect()
     }
 
diff --git a/mithril-signer/Cargo.toml b/mithril-signer/Cargo.toml
index 1b5149c2b..f20f20ade 100644
--- a/mithril-signer/Cargo.toml
+++ b/mithril-signer/Cargo.toml
@@ -11,6 +11,7 @@ blake2      = "0.9.2"
 clap = { version = "3.1.6", features = ["derive"] }
 config = "0.13.1"
 hex = "0.4.3"
+mithril = { path = "../mithril-core" }
 mithril-common = { path = "../mithril-common" }
 rand_chacha = "0.3.1"
 rand_core   = "0.6.3"
diff --git a/mithril-signer/src/single_signer.rs b/mithril-signer/src/single_signer.rs
index 7dbb9e61a..0452fd6d3 100644
--- a/mithril-signer/src/single_signer.rs
+++ b/mithril-signer/src/single_signer.rs
@@ -2,6 +2,7 @@ use hex::ToHex;
 use slog_scope::{info, trace, warn};
 use thiserror::Error;
 
+use mithril::RegisterError;
 use mithril_common::crypto_helper::{
     key_decode_hex, key_encode_hex, ProtocolClerk, ProtocolInitializer, ProtocolKeyRegistration,
     ProtocolSigner,
@@ -80,6 +81,10 @@ pub enum SingleSignerError {
     #[error("the protocol initializer is missing")]
     ProtocolInitializerMissing(),
 
+    /// Could not fetch a signer from a protocol initializer.
+    #[error("the protocol initializer is not registered")]
+    ProtocolInitializerNotRegistered(#[from] RegisterError),
+
     /// Encoding / Decoding error.
     #[error("codec error: '{0}'")]
     Codec(String),
@@ -123,7 +128,7 @@ impl MithrilSingleSigner {
         }
         let closed_reg = key_reg.close();
 
-        Ok(protocol_initializer.to_owned().new_signer(closed_reg))
+        Ok(protocol_initializer.to_owned().new_signer(closed_reg)?)
     }
 }
 


Test patch:
diff --git a/mithril-common/src/crypto_helper/tests_setup.rs b/mithril-common/src/crypto_helper/tests_setup.rs
index 2922d672f..1a6940ec3 100644
--- a/mithril-common/src/crypto_helper/tests_setup.rs
+++ b/mithril-common/src/crypto_helper/tests_setup.rs
@@ -104,7 +104,8 @@ pub fn setup_signers_from_parties(
                 protocol_initializer.verification_key(),
                 protocol_initializer
                     .clone()
-                    .new_signer(closed_key_registration.clone()),
+                    .new_signer(closed_key_registration.clone())
+                    .unwrap(),
                 protocol_initializer,
             )
         })
diff --git a/mithril-core/tests/integration.rs b/mithril-core/tests/integration.rs
index 26fb94ab3..28cf4da02 100644
--- a/mithril-core/tests/integration.rs
+++ b/mithril-core/tests/integration.rs
@@ -44,7 +44,7 @@ fn test_full_protocol() {
 
     let ps = ps
         .into_par_iter()
-        .map(|p| p.new_signer(closed_reg.clone()))
+        .map(|p| p.new_signer(closed_reg.clone()).unwrap())
         .collect::<Vec<StmSigner<H>>>();
 
     /////////////////////