[fix] #1811: added tests and checks to dedup trusted peer keys.

Signed-off-by: Aleksandr <a-p-petrosyan@yandex.ru>

core/src/config.rs

@@ -3,18 +3,15 @@ use std::{fmt::Debug, fs::File, io::BufReader, path::Path};
 
 use eyre::{Result, WrapErr};
 use iroha_config::derive::Configurable;
-use iroha_crypto::{KeyPair, PrivateKey, PublicKey};
+use iroha_crypto::{PrivateKey, PublicKey};
 use iroha_data_model::prelude::*;
 use iroha_logger::Configuration as LoggerConfiguration;
 use serde::{Deserialize, Serialize};
 
 use crate::{
-    block_sync::config::BlockSyncConfiguration,
-    genesis::config::GenesisConfiguration,
-    kura::config::KuraConfiguration,
-    queue::Configuration as QueueConfiguration,
-    sumeragi::config::{SumeragiConfiguration, TrustedPeers},
-    torii::config::ToriiConfiguration,
+    block_sync::config::BlockSyncConfiguration, genesis::config::GenesisConfiguration,
+    kura::config::KuraConfiguration, queue::Configuration as QueueConfiguration,
+    sumeragi::config::SumeragiConfiguration, torii::config::ToriiConfiguration,
     wsv::config::Configuration as WorldStateViewConfiguration,
 };
 
@@ -85,51 +82,70 @@ impl Default for NetworkConfiguration {
 }
 
 impl Configuration {
-    /// This method will build `Configuration` from a json *pretty* formatted file (without `:` in
-    /// key names).
+    /// Construct [`Self`] from a path-like object.
     ///
-    /// # Panics
-    /// This method will panic if configuration file presented, but has incorrect scheme or format.
+    /// The method has automatic de-duplication for items that are
+    /// represented as [`HashSet`]s, and errors if a duplicate is
+    /// found for [`crate::sumeragi::config::TrustedPeers`], i.e. if
+    /// two peers have the same public key.
     ///
     /// # Errors
-    /// This method will return error if system will fail to find a file or read it's content.
-    pub fn from_path<P: AsRef<Path> + Debug>(path: P) -> Result<Configuration> {
-        let file = File::open(path).wrap_err("Failed to open the config file")?;
+    /// - File not found.
+    /// - File found, but peer configuration parsing failed.
+    /// - File re-opening failed.
+    /// - Length of the array in raw json representation is different
+    /// to the lenght of the array in
+    /// [`self.sumeragi.trusted_peers.peers`], most likely due to two
+    /// (or more) peers having the same public key.
+    pub fn from_path<P: AsRef<Path> + Debug + Clone>(path: P) -> Result<Configuration> {
+        let file = File::open(path.clone())
+            .wrap_err(format!("Failed to open the config file {:?}", path))?;
         let reader = BufReader::new(file);
-        let mut configuration: Configuration =
-            serde_json::from_reader(reader).wrap_err("Failed to deserialize json from reader")?;
-        configuration.sumeragi.key_pair = KeyPair {
-            public_key: configuration.public_key.clone(),
-            private_key: configuration.private_key.clone(),
-        };
-        configuration.sumeragi.peer_id =
-            PeerId::new(&configuration.torii.p2p_addr, &configuration.public_key);
+        let mut configuration: Configuration = serde_json::from_reader(reader).wrap_err(
+            format!("Failed to parse {:?} as Iroha peer configuration.", path),
+        )?;
+        // TODO: While re-reading the file isn't too much of a problem,
+        // from_reader drops the file, it's safe to re-read.
+        let second_reader = BufReader::new(File::open(path.clone())?);
+        let raw_json: serde_json::Value = serde_json::from_reader(second_reader).wrap_err(format!("Failed to read raw json from {:?}. However, parsing the file as a Configuration structure succeeded. ", path))?;
+        if let serde_json::Value::Array(arr) = &raw_json["SUMERAGI"]["TRUSTED_PEERS"] {
+            if arr.len() != configuration.sumeragi.trusted_peers.peers.len() {
+                let err = eyre::eyre!(
+                    "Raw JSON value {:?} vs. {:?}",
+                    arr,
+                    configuration.sumeragi.trusted_peers
+                );
+                return Err(err.wrap_err("Possible duplication of trusted peers. Make sure that each public key appears only once. "));
+            }
+        } else {
+            // If you get this error, you most likely updated the
+            // configuration.sumeragi.trusted_peers,
+            return Err(eyre::eyre!(
+                "configuration's TRUSTED_PEERS was not an array type, instead got {}",
+                &raw_json
+            ));
+        }
+        configuration.finalise();
         Ok(configuration)
     }
 
-    /// Loads configuration from environment
-    /// # Errors
-    /// Fails if fails to deserialize configuration from env variables
-    pub fn load_environment(&mut self) -> Result<()> {
-        iroha_config::Configurable::load_environment(self)?;
-        self.sumeragi.key_pair = KeyPair {
-            public_key: self.public_key.clone(),
-            private_key: self.private_key.clone(),
-        };
+    /// Enforces a few invariants
+    fn finalise(&mut self) {
+        self.sumeragi.key_pair = self.key_pair().into();
         self.sumeragi.peer_id = PeerId::new(&self.torii.p2p_addr, &self.public_key.clone());
-        Ok(())
     }
 
-    /// Load trusted peers variables from a json *pretty* formatted file.
+    /// Loads configuration from environment
     ///
     /// # Errors
-    /// Fails if can not load [`TrustedPeers`] from `path`.
-    pub fn load_trusted_peers_from_path<P: AsRef<Path> + Debug>(&mut self, path: P) -> Result<()> {
-        self.sumeragi.trusted_peers = TrustedPeers::from_path(&path)?;
+    /// Fails if fails to deserialize configuration from env variables
+    pub fn load_environment(&mut self) -> Result<()> {
+        iroha_config::Configurable::load_environment(self)?;
+        self.finalise();
         Ok(())
     }
 
-    /// Gets `public_key` and `private_key` configuration parameters.
+    /// Get `public_key` and `private_key` configuration parameters.
     pub fn key_pair(&self) -> (PublicKey, PrivateKey) {
         (self.public_key.clone(), self.private_key.clone())
     }
@@ -156,6 +172,9 @@ mod tests {
 
     #[test]
     fn parse_example_trusted_peers_json() -> Result<(), String> {
+        // NOTE: this test will fail if you touch
+        // `configuration.sumeragi.trusted_peers`.  If this is the
+        // only failure, consider looking at the raw output.
         let configuration = Configuration::from_path(CONFIGURATION_PATH)
             .map_err(|e| format!("Failed to read configuration from example config: {}", e))?;
         let public_key1 = PublicKey {

core/src/sumeragi/mod.rs

@@ -1851,7 +1851,7 @@ pub mod config {
         ///
         /// # Errors
         /// Fails if there is no file or if file is not valid json
-        pub fn from_path<P: AsRef<Path> + Debug>(path: P) -> Result<TrustedPeers> {
+        pub fn from_path<P: AsRef<Path> + Debug>(path: P) -> Result<Self> {
             let file = File::open(&path)
                 .wrap_err_with(|| format!("Failed to open trusted peers file {:?}", &path))?;
             let reader = BufReader::new(file);
@@ -1870,7 +1870,7 @@ pub mod config {
         }
     }
 
-    // Allowed because `HashSet::new()` is not const yet.
+    // `HashSet::new()` is not const yet.
     fn default_empty_trusted_peers() -> TrustedPeers {
         TrustedPeers {
             peers: HashSet::new(),

crypto/src/lib.rs

@@ -160,6 +160,15 @@ pub struct KeyPair {
     pub private_key: PrivateKey,
 }
 
+impl From<(PublicKey, PrivateKey)> for KeyPair {
+    fn from((public_key, private_key): (PublicKey, PrivateKey)) -> Self {
+        Self {
+            public_key,
+            private_key,
+        }
+    }
+}
+
 /// Error when dealing with cryptographic functions
 #[derive(Debug, Display)]
 pub enum Error {