Merge pull request #617 from input-output-hk/jpraynaud/585-deprecate-unverified-signer-registration

Deprecate uncertified signer registration

Author: jpraynaud

docs/root/manual/getting-started/run-signer-node.md

@@ -35,10 +35,10 @@ For more information about the **Mithril Protocol**, please refer to the [About
 ## What you'll need
 
 * Operating a **Cardano Node** as a **Stake Pool**:
-  * **Stable**: The Cardano `Pool Id` in a `BECH32` format such as `pool1frevxe70aqw2ce58c0muyesnahl88nfjjsp25h85jwakzgd2g2l`
-  * **Experimental**:
+  * **Stable**:
     * The Cardano `Operational Certificate` file of the pool
     * The Cardano `KES Secret Key` file of the pool
+  * **Deprecated**: The Cardano `Pool Id` in a `BECH32` format such as `pool1frevxe70aqw2ce58c0muyesnahl88nfjjsp25h85jwakzgd2g2l`
 
 * Access to the file system of a `relay` **Cardano Node** running on the `testnet`:
   * Read rights on the `Database` folder (`--database-path` setting of the **Cardano Node**)
@@ -62,21 +62,21 @@ Your feedback is very important, so feel free to contact us on the #moria channe
 
 :::
 
-### Stable mode: Declare your Pool Id
-
-In this stable mode, the Cardano `Pool Id` that you specify is not strictly verified. It is associated to Cardano stakes based on your declaration. This mode is soon to be deprecated and replaced by the certification mode below.
-
-This is the default mode presented in the setup of this document, it is displayed with a specific **Stable** mention.
-
-### Experimental mode: Certify your Pool Id
+### Stable mode: Certify your Pool Id
 
 In this mode, you declare your Cardano `Operational Certificate` file and `KES Secret Key` file which allows to:
 
 * Compute automatically the `PoolId`
 * Verify that you are the owner of the `PoolId`, and thus of the associated stakes used by Mithril protocol
 * Verify that you are the owner of the Mithril `Signer Secret Key`, and thus allowed to contribute to the multi-signatures and certificate production of the Mithril network
 
-This mode is displayed with a specific **Experimental** mention in this document.
+This mode is displayed with a specific **Stable** mention in this document.
+
+### Deprecated mode: Declare your Pool Id
+
+In this mode, the Cardano `Pool Id` that you specify is not strictly verified. It is associated to Cardano stakes based on your declaration. This mode is deprecated and replaced by the certification mode above.
+
+This mode is presented in the setup of this document with a specific **Deprecated** mention.
 
 ## Building your own executable
 
@@ -165,16 +165,16 @@ sudo mv mithril-signer /opt/mithril
 Replace this value with the correct user. We assume that the user used to run the **Cardano Node** is `cardano`. The **Mithril Signer** must imperatively run with the same user.
 
 * **Stable mode**: in the `/opt/mithril/mithril-signer/service.env` env file:
-  * `PARTY_ID=YOUR_POOL_ID_BECH32`: replace `YOUR_POOL_ID_BECH32` with your BECH32 `Pool Id`
+  * `KES_SECRET_KEY_PATH=/cardano/keys/kes.skey`: replace `/cardano/keys/kes.skey` with the path to your Cardano `KES Secret Key` file
+  * `OPERATIONAL_CERTIFICATE_PATH=/cardano/cert/opcert.cert`: replace `/cardano/cert/opcert.cert` with the path to your Cardano `Operational Certificate` file
   * `DB_DIRECTORY=/cardano/db`: replace `/cardano/db` with the path to the database folder of the **Cardano Node** (the one in `--database-path`)
   * `CARDANO_NODE_SOCKET_PATH=/cardano/ipc/node.socket`: replace with the path to the IPC file (`CARDANO_NODE_SOCKET_PATH` env var)
   * `CARDANO_CLI_PATH=/app/bin/cardano-cli`: replace with the path to the `cardano-cli` executable
   * `DATA_STORES_DIRECTORY=/opt/mithril/stores`: replace with the path to a folder where the **Mithril Signer** will store its data (`/opt/mithril/stores` e.g.)
   * `STORE_RETENTION_LIMIT`: if set, this will limit the number of records in some internal stores (5 is a good fit).
 
-* :boom: **Experimental mode**: in the `/opt/mithril/mithril-signer/service.env` env file:
-  * `KES_SECRET_KEY_PATH=/cardano/keys/kes.skey`: replace `/cardano/keys/kes.skey` with the path to your Cardano `KES Secret Key` file
-  * `OPERATIONAL_CERTIFICATE_PATH=/cardano/cert/pool.cert`: replace `/cardano/cert/pool.cert` with the path to your Cardano `Operational Certificate` file
+* **Deprecated mode**: in the `/opt/mithril/mithril-signer/service.env` env file:
+  * `PARTY_ID=YOUR_POOL_ID_BECH32`: replace `YOUR_POOL_ID_BECH32` with your BECH32 `Pool Id`
   * `DB_DIRECTORY=/cardano/db`: replace `/cardano/db` with the path to the database folder of the **Cardano Node** (the one in `--database-path`)
   * `CARDANO_NODE_SOCKET_PATH=/cardano/ipc/node.socket`: replace with the path to the IPC file (`CARDANO_NODE_SOCKET_PATH` env var)
   * `CARDANO_CLI_PATH=/app/bin/cardano-cli`: replace with the path to the `cardano-cli` executable
@@ -189,7 +189,8 @@ First create an env file that will be used by the service:
 
 ```bash
 sudo bash -c 'cat > /opt/mithril/mithril-signer.env << EOF
-PARTY_ID=**YOUR_POOL_ID_BECH32**
+KES_SECRET_KEY_PATH=**YOUR_KES_SECRET_KEY_PATH**
+OPERATIONAL_CERTIFICATE_PATH=**YOUR_OPERATIONAL_CERTIFICATE_PATH**
 NETWORK=**YOUR_CARDANO_NETWORK**
 AGGREGATOR_ENDPOINT=**YOUR_AGGREGATOR_ENDPOINT**
 RUN_INTERVAL=60000
@@ -201,12 +202,11 @@ STORE_RETENTION_LIMIT=5
 EOF'
 ```
 
-* :boom: **Experimental mode**:
+* **Deprecated mode**:
 
 ```bash
 sudo bash -c 'cat > /opt/mithril/mithril-signer.env << EOF
-KES_SECRET_KEY_PATH=**YOUR_KES_SECRET_KEY_PATH**
-OPERATIONAL_CERTIFICATE_PATH=**YOUR_OPERATIONAL_CERTIFICATE_PATH**
+PARTY_ID=**YOUR_POOL_ID_BECH32**
 NETWORK=**YOUR_CARDANO_NETWORK**
 AGGREGATOR_ENDPOINT=**YOUR_AGGREGATOR_ENDPOINT**
 RUN_INTERVAL=60000

mithril-common/Cargo.toml

@@ -54,6 +54,8 @@ mithril-stm = { path = "../mithril-stm", default-features = false, features = ["
 slog-scope = "4.4.0"
 
 [features]
+default = ["allow_skip_signer_certification"]
 portable = ["mithril-stm/portable"]
 test_only = []
 allow_skip_signer_certification = []
+

mithril-common/src/crypto_helper/cardano/key_certification.rs

@@ -43,6 +43,10 @@ pub enum ProtocolRegistrationErrorWrapper {
     #[error("party id does not exist in the stake distribution")]
     PartyIdNonExisting,
 
+    /// Error raised when the operational certificate is missing
+    #[error("missing operational certificate")]
+    OpCertMissing,
+
     /// Error raised when an operational certificate is invalid
     #[error("invalid operational certificate")]
     OpCertInvalid,
@@ -245,7 +249,10 @@ impl KeyRegWrapper {
                 opcert.start_kes_period,
             ))?
         } else {
-            println!("WARNING: Signer certification is skipped! {:?}", party_id);
+            if cfg!(not(feature = "allow_skip_signer_certification")) {
+                Err(ProtocolRegistrationErrorWrapper::OpCertMissing)?
+            }
+            println!("WARNING: Uncertified signer regsitration by providing a Pool Id is deprecated and will be removed soon! (Pool Id: {:?})", party_id);
             party_id.ok_or(ProtocolRegistrationErrorWrapper::PartyIdMissing)?
         };
 

mithril-common/src/crypto_helper/tests_setup.rs

@@ -15,7 +15,8 @@ use std::{cmp::min, fs, sync::Arc};
 
 use std::{collections::HashMap, path::PathBuf};
 
-fn setup_temp_directory_for_signer(
+/// Create or retrieve a temporary directory for storing cryptographic material for a signer, use this for tests only.
+pub fn setup_temp_directory_for_signer(
     party_id: &ProtocolPartyId,
     auto_create: bool,
 ) -> Option<PathBuf> {
@@ -62,8 +63,11 @@ pub fn setup_signers(
     let stake_distribution = (0..total)
         .into_iter()
         .map(|party_idx| {
-            let party_id = if party_idx % 2 == 0 {
-                // 50% of signers with key certification
+            let party_id = if party_idx % 2 == 0
+                || cfg!(not(feature = "allow_skip_signer_certification"))
+            {
+                // 50% of signers with key certification if allow unverified signer registration
+                // Or 100% of signers otherwise
                 let keypair = ColdKeyGenerator::create_deterministic_keypair([party_idx as u8; 32]);
                 let (kes_secret_key, kes_verification_key) = Sum6Kes::keygen(&mut kes_keys_seed);
                 let operational_certificate = OpCert::new(kes_verification_key, 0, 0, keypair);
@@ -77,14 +81,15 @@ pub fn setup_signers(
                         .to_file(temp_dir.join("kes.sk"))
                         .expect("KES secret key file export should not fail");
                 }
-                if !temp_dir.join("pool.cert").exists() {
+                if !temp_dir.join("opcert.cert").exists() {
                     operational_certificate
-                        .to_file(temp_dir.join("pool.cert"))
+                        .to_file(temp_dir.join("opcert.cert"))
                         .expect("operational certificate file export should not fail");
                 }
                 party_id
             } else {
-                // 50% of signers without key certification (legacy)
+                // 50% of signers without key certification (legacy) if allow unverified signer registration
+                // Or 0% of signers otherwise
                 // TODO: Should be removed once the signer certification is fully deployed
                 format!("{:<032}", party_idx)
             };
@@ -132,7 +137,7 @@ pub fn setup_signers_from_stake_distribution(
         .for_each(|(party_id, _stake, protocol_initializer)| {
             let temp_dir = setup_temp_directory_for_signer(party_id, false);
             let operational_certificate = temp_dir.as_ref().map(|dir| {
-                OpCert::from_file(dir.join("pool.cert"))
+                OpCert::from_file(dir.join("opcert.cert"))
                     .expect("operational certificate decoding should not fail")
             });
             let verification_key = protocol_initializer.verification_key();
@@ -154,7 +159,7 @@ pub fn setup_signers_from_stake_distribution(
         .map(|(party_id, stake, protocol_initializer)| {
             let temp_dir = setup_temp_directory_for_signer(&party_id, false);
             let operational_certificate: Option<OpCert> = temp_dir.as_ref().map(|dir| {
-                OpCert::from_file(dir.join("pool.cert"))
+                OpCert::from_file(dir.join("opcert.cert"))
                     .expect("operational certificate decoding should not fail")
             });
             let kes_period = 0;

mithril-signer/tests/state_machine.rs

@@ -1,13 +1,20 @@
 mod test_extensions;
 
-use mithril_common::entities::Epoch;
+use mithril_common::{
+    crypto_helper::tests_setup,
+    entities::{Epoch, SignerWithStake},
+};
 
 use test_extensions::StateMachineTester;
 
 #[rustfmt::skip]
 #[tokio::test]
 async fn test_create_single_signature() {
-    let mut tester = StateMachineTester::init().await;
+
+    let protocol_parameters = tests_setup::setup_protocol_parameters();
+    let signers = tests_setup::setup_signers(10, &protocol_parameters);
+    let signers_with_stake = signers.iter().map(|(signer_with_stake, _, _)| signer_with_stake.to_owned()).collect::<Vec<SignerWithStake>>();
+    let mut tester = StateMachineTester::init(&signers_with_stake).await.expect("state machine tester init should not fail");
 
     tester
         .comment("state machine starts and remains in Unregistered state until a epoch settings is got")
@@ -25,7 +32,7 @@ async fn test_create_single_signature() {
         .comment("getting an epoch settings changes the state → Registered")
         .aggregator_send_epoch_settings().await
         .cycle_registered().await.unwrap()
-        .register_signers(2).await.unwrap()
+        .register_signers(&signers_with_stake[..2]).await.unwrap()
         .check_protocol_initializer(Epoch(3)).await.unwrap()
         .check_stake_store(Epoch(3)).await.unwrap()
 

mithril-signer/tests/test_extensions/state_machine_tester.rs

@@ -45,20 +45,30 @@ pub struct StateMachineTester {
 }
 
 impl StateMachineTester {
-    pub async fn init() -> Self {
+    pub async fn init(signers_with_stake: &[SignerWithStake]) -> Result<Self> {
+        let selected_signer_with_stake = signers_with_stake.first().ok_or_else(|| {
+            TestError::AssertFailed("there should be at least one signer with stakes".to_string())
+        })?;
+        let selected_signer_party_id = selected_signer_with_stake.party_id.clone();
+        let selected_signer_temp_dir =
+            tests_setup::setup_temp_directory_for_signer(&selected_signer_party_id, false);
         let config = Config {
             aggregator_endpoint: "http://0.0.0.0:8000".to_string(),
             cardano_cli_path: PathBuf::new(),
             cardano_node_socket_path: PathBuf::new(),
             db_directory: PathBuf::new(),
             network: "devnet".to_string(),
             network_magic: Some(42),
-            party_id: Some("99999999999999999999999999999999".to_string()),
+            party_id: Some(selected_signer_party_id),
             run_interval: 5000,
             data_stores_directory: PathBuf::new(),
             store_retention_limit: None,
-            kes_secret_key_path: None,
-            operational_certificate_path: None,
+            kes_secret_key_path: selected_signer_temp_dir
+                .as_ref()
+                .map(|dir| dir.join("kes.sk")),
+            operational_certificate_path: selected_signer_temp_dir
+                .as_ref()
+                .map(|dir| dir.join("opcert.cert")),
         };
 
         let decorator = slog_term::PlainDecorator::new(slog_term::TestStdoutWriter);
@@ -103,22 +113,9 @@ impl StateMachineTester {
             stake_store: stake_store.clone(),
         };
         // set up stake distribution
-        let protocol_parameters = tests_setup::setup_protocol_parameters();
-        let mut signers: Vec<SignerWithStake> =
-            tests_setup::setup_signers(10, &protocol_parameters)
-                .into_iter()
-                .map(|(signer_with_stake, _, _)| signer_with_stake)
-                .collect();
-        signers.push(SignerWithStake {
-            party_id: "99999999999999999999999999999999".to_string(),
-            stake: 999,
-            verification_key: "".to_string(),
-            verification_key_signature: None,
-            operational_certificate: None,
-            kes_period: None,
-        });
-
-        chain_observer.set_signers(signers).await;
+        chain_observer
+            .set_signers(signers_with_stake.to_owned())
+            .await;
 
         let runner = Box::new(SignerRunner::new(config, services));
 
@@ -128,7 +125,7 @@ impl StateMachineTester {
             Duration::from_secs(5),
         );
 
-        StateMachineTester {
+        Ok(StateMachineTester {
             state_machine,
             immutable_observer,
             chain_observer,
@@ -137,7 +134,7 @@ impl StateMachineTester {
             stake_store,
             comment_no: 0,
             _logs_guard: logs_guard,
-        }
+        })
     }
 
     fn assert(&mut self, condition: bool, description: String) -> Result<&mut Self> {
@@ -279,11 +276,11 @@ impl StateMachineTester {
     }
 
     /// register the signer in the certificate handler
-    pub async fn register_signers(&mut self, count: u64) -> Result<&mut Self> {
-        let protocol_parameters = tests_setup::setup_protocol_parameters();
-        for (signer_with_stake, _signer, _protocol_initializer) in
-            tests_setup::setup_signers(count, &protocol_parameters)
-        {
+    pub async fn register_signers(
+        &mut self,
+        signers_with_stake: &[SignerWithStake],
+    ) -> Result<&mut Self> {
+        for signer_with_stake in signers_with_stake {
             self.certificate_handler
                 .register_signer(&signer_with_stake.to_owned().into())
                 .await

mithril-test-lab/mithril-end-to-end/Cargo.toml

@@ -27,4 +27,6 @@ tokio = { version = "1.17.0", features = ["full"] }
 tokio-util = { version = "0.7.1", features = ["codec"] }
 
 [features]
+default = ["allow_skip_signer_certification"]
 portable = ["mithril-common/portable"]
+allow_skip_signer_certification = []

mithril-test-lab/mithril-end-to-end/src/mithril/infrastructure.rs

@@ -39,9 +39,11 @@ impl MithrilInfrastructure {
 
         let mut signers: Vec<Signer> = vec![];
         for (index, pool_node) in devnet_topology.pool_nodes.iter().enumerate() {
-            // 50% of signers with key certification
-            // TODO: Should be removed 100% once the signer certification is fully deployed
-            let enable_certification = index % 2 == 0;
+            // 50% of signers with key certification if allow unverified signer registration
+            // Or 100% of signers otherwise
+            // TODO: Should be removed once the signer certification is fully deployed
+            let enable_certification =
+                index % 2 == 0 || cfg!(not(feature = "allow_skip_signer_certification"));
             let mut signer = Signer::new(
                 aggregator.endpoint(),
                 pool_node,