client-core: clean up gateway registration (#2827)

* client-core: clean up gateway registration

* client-core: tidy docs and clippy

* clients: tidy output

clients/client-core/src/config/mod.rs

@@ -88,6 +88,7 @@ impl<T> Config<T> {
         Config::default().with_id(id)
     }
 
+    #[must_use]
     pub fn with_id<S: Into<String>>(mut self, id: S) -> Self
     where
         T: NymConfig,

clients/client-core/src/init/helpers.rs

@@ -18,7 +18,7 @@ use url::Url;
 
 pub(super) async fn query_gateway_details(
     validator_servers: Vec<Url>,
-    chosen_gateway_id: Option<String>,
+    chosen_gateway_id: Option<identity::PublicKey>,
 ) -> Result<gateway::Node, ClientCoreError> {
     let nym_api = validator_servers
         .choose(&mut thread_rng())
@@ -40,7 +40,7 @@ pub(super) async fn query_gateway_details(
     if let Some(gateway_id) = chosen_gateway_id {
         filtered_gateways
             .iter()
-            .find(|gateway| gateway.identity_key.to_base58_string() == gateway_id)
+            .find(|gateway| gateway.identity_key == gateway_id)
             .ok_or_else(|| ClientCoreError::NoGatewayWithId(gateway_id.to_string()))
             .cloned()
     } else {

clients/client-core/src/init/mod.rs

@@ -69,42 +69,21 @@ pub fn new_client_keys() -> KeyManager {
     KeyManager::new(&mut rng)
 }
 
-/// Convenience function for setting up the gateway for a client. Depending on the arguments given
-/// it will do the sensible thing.
-pub async fn setup_gateway<C, T>(
-    register_gateway: bool,
-    // TODO: this should get refactored to instead take Option<identity::PublicKey>
-    user_chosen_gateway_id: Option<String>,
-    config: &Config<T>,
-) -> Result<GatewayEndpointConfig, ClientCoreError>
-where
-    C: NymConfig + ClientCoreConfigTrait,
-    T: NymConfig,
-{
-    let id = config.get_id();
-    if register_gateway {
-        register_with_gateway_and_store(user_chosen_gateway_id, config).await
-    } else if let Some(user_chosen_gateway_id) = user_chosen_gateway_id {
-        config_gateway_with_existing_keys(user_chosen_gateway_id, config).await
-    } else {
-        reuse_existing_gateway_config::<C>(&id)
-    }
-}
-
-/// Get the gateway details by querying the validator-api. Either pick one at random or use
-/// the chosen one if it's among the available ones.
+/// Authenticate and register with a gateway.
+/// Either pick one at random by querying the available gateways from the nym-api, or use the
+/// chosen one if it's among the available ones.
+/// The shared key is added to the supplied `KeyManager` and the endpoint details are returned.
 pub async fn register_with_gateway(
     key_manager: &mut KeyManager,
     nym_api_endpoints: Vec<Url>,
-    chosen_gateway_id: Option<String>,
+    chosen_gateway_id: Option<identity::PublicKey>,
 ) -> Result<GatewayEndpointConfig, ClientCoreError> {
-    // Our identity is derived from our key
-    let our_identity = key_manager.identity_keypair();
-
     // Get the gateway details of the gateway we will use
     let gateway = helpers::query_gateway_details(nym_api_endpoints, chosen_gateway_id).await?;
     log::debug!("Querying gateway gives: {}", gateway);
 
+    let our_identity = key_manager.identity_keypair();
+
     // Establish connection, authenticate and generate keys for talking with the gateway
     let shared_keys = helpers::register_with_gateway(&gateway, our_identity).await?;
     key_manager.insert_gateway_shared_key(shared_keys);
@@ -112,59 +91,64 @@ pub async fn register_with_gateway(
     Ok(gateway.into())
 }
 
-/// Get the gateway details by querying the validator-api. Either pick one at random or use
-/// the chosen one if it's among the available ones.
-/// Saves keys to disk, specified by the paths in `config`.
-pub async fn register_with_gateway_and_store<T>(
-    chosen_gateway_id: Option<String>,
+/// Convenience function for setting up the gateway for a client given a `Config`. Depending on the
+/// arguments given it will do the sensible thing. Either it will
+///
+/// a. Reuse existing gateway configuration from storage.
+/// b. Create a new gateway configuration but keep existing keys. This assumes that the caller
+///    knows what they are doing and that the keys match the requested gateway.
+/// c. Create a new gateway configuration with a newly registered gateway and keys.
+pub async fn setup_gateway_from_config<C, T>(
+    register_gateway: bool,
+    user_chosen_gateway_id: Option<identity::PublicKey>,
     config: &Config<T>,
 ) -> Result<GatewayEndpointConfig, ClientCoreError>
 where
+    C: NymConfig + ClientCoreConfigTrait,
     T: NymConfig,
 {
-    println!("Configuring gateway");
-    let mut key_manager = new_client_keys();
+    let id = config.get_id();
 
-    let gateway = register_with_gateway(
-        &mut key_manager,
-        config.get_nym_api_endpoints(),
-        chosen_gateway_id,
-    )
-    .await?;
+    // If we are not going to register gateway, and an explicitly chosed gateway is not passed in,
+    // load the existing configuration file
+    if !register_gateway && user_chosen_gateway_id.is_none() {
+        println!("Not registering gateway, will reuse existing config and keys");
+        return load_existing_gateway_config::<C>(&id);
+    }
 
-    helpers::store_keys(&key_manager, config)?;
-    println!("Saved all generated keys");
-
-    Ok(gateway)
-}
-
-/// Set the gateway using the usual procedue of querying the validator-api, but don't register or
-/// create any keys.
-/// This assumes that the user knows what they are doing, and that the existing keys are valid for
-/// the gateway being used
-pub async fn config_gateway_with_existing_keys<T>(
-    user_chosen_gateway_id: String,
-    config: &Config<T>,
-) -> Result<GatewayEndpointConfig, ClientCoreError>
-where
-    T: NymConfig,
-{
-    println!("Using gateway provided by user, keeping existing keys");
-    let gateway = helpers::query_gateway_details(
-        config.get_nym_api_endpoints(),
-        Some(user_chosen_gateway_id),
-    )
-    .await?;
+    // Else, we preceed by querying the nym-api
+    let gateway =
+        helpers::query_gateway_details(config.get_nym_api_endpoints(), user_chosen_gateway_id)
+            .await?;
     log::debug!("Querying gateway gives: {}", gateway);
+
+    // If we are not registering, just return this and assume the caller has the keys already and
+    // wants to keep the,
+    if user_chosen_gateway_id.is_some() {
+        println!("Using gateway provided by user, keeping existing keys");
+        return Ok(gateway.into());
+    }
+
+    // Create new keys and derive our identity
+    let mut key_manager = new_client_keys();
+    let our_identity = key_manager.identity_keypair();
+
+    // Establish connection, authenticate and generate keys for talking with the gateway
+    println!("Registering with new gateway");
+    let shared_keys = helpers::register_with_gateway(&gateway, our_identity).await?;
+    key_manager.insert_gateway_shared_key(shared_keys);
+
+    // Write all keys to storage and just return the gateway endpoint config. It is assumed that we
+    // will load keys from storage when actually connecting.
+    helpers::store_keys(&key_manager, config)?;
     Ok(gateway.into())
 }
 
 /// Read and reuse the existing gateway configuration from a file that was generate earlier.
-pub fn reuse_existing_gateway_config<T>(id: &str) -> Result<GatewayEndpointConfig, ClientCoreError>
+pub fn load_existing_gateway_config<T>(id: &str) -> Result<GatewayEndpointConfig, ClientCoreError>
 where
     T: NymConfig + ClientCoreConfigTrait,
 {
-    println!("Not registering gateway, will reuse existing config and keys");
     T::load_from_file(Some(id))
         .map(|existing_config| existing_config.get_gateway_endpoint().clone())
         .map_err(|err| {
@@ -242,9 +226,9 @@ where
 pub fn output_to_json<T: Serialize>(init_results: &T, output_file: &str) {
     match std::fs::File::create(output_file) {
         Ok(file) => match serde_json::to_writer_pretty(file, init_results) {
-            Ok(_) => println!("Saved: {}", output_file),
-            Err(err) => eprintln!("Could not save {}: {err}", output_file),
+            Ok(_) => println!("Saved: {output_file}"),
+            Err(err) => eprintln!("Could not save {output_file}: {err}"),
         },
-        Err(err) => eprintln!("Could not save {}: {err}", output_file),
+        Err(err) => eprintln!("Could not save {output_file}: {err}"),
     }
 }

clients/native/src/commands/init.rs

@@ -114,16 +114,15 @@ pub(crate) async fn execute(args: &Init) -> Result<(), ClientError> {
 
     let already_init = Config::default_config_file_path(Some(id)).exists();
     if already_init {
-        println!(
-            "Client \"{}\" was already initialised before! \
-            Config information will be overwritten (but keys will be kept)!",
-            id
-        );
+        println!("Client \"{id}\" was already initialised before");
     }
 
     // Usually you only register with the gateway on the first init, however you can force
     // re-registering if wanted.
     let user_wants_force_register = args.force_register_gateway;
+    if user_wants_force_register {
+        println!("Instructed to force registering gateway. This might overwrite keys!");
+    }
 
     // If the client was already initialized, don't generate new keys and don't re-register with
     // the gateway (because this would create a new shared key).
@@ -138,9 +137,9 @@ pub(crate) async fn execute(args: &Init) -> Result<(), ClientError> {
 
     // Setup gateway by either registering a new one, or creating a new config from the selected
     // one but with keys kept, or reusing the gateway configuration.
-    let gateway = client_core::init::setup_gateway::<Config, _>(
+    let gateway = client_core::init::setup_gateway_from_config::<Config, _>(
         register_gateway,
-        user_chosen_gateway_id.map(|id| id.to_base58_string()),
+        user_chosen_gateway_id,
         config.get_base(),
     )
     .await

clients/socks5/src/commands/init.rs

@@ -124,16 +124,15 @@ pub(crate) async fn execute(args: &Init) -> Result<(), Socks5ClientError> {
 
     let already_init = Config::default_config_file_path(Some(id)).exists();
     if already_init {
-        println!(
-            "SOCKS5 client \"{}\" was already initialised before! \
-            Config information will be overwritten (but keys will be kept)!",
-            id
-        );
+        println!("SOCKS5 client \"{id}\" was already initialised before");
     }
 
     // Usually you only register with the gateway on the first init, however you can force
     // re-registering if wanted.
     let user_wants_force_register = args.force_register_gateway;
+    if user_wants_force_register {
+        println!("Instructed to force registering gateway. This might overwrite keys!");
+    }
 
     // If the client was already initialized, don't generate new keys and don't re-register with
     // the gateway (because this would create a new shared key).
@@ -151,9 +150,9 @@ pub(crate) async fn execute(args: &Init) -> Result<(), Socks5ClientError> {
 
     // Setup gateway by either registering a new one, or creating a new config from the selected
     // one but with keys kept, or reusing the gateway configuration.
-    let gateway = client_core::init::setup_gateway::<Config, _>(
+    let gateway = client_core::init::setup_gateway_from_config::<Config, _>(
         register_gateway,
-        user_chosen_gateway_id.map(|id| id.to_base58_string()),
+        user_chosen_gateway_id,
         config.get_base(),
     )
     .await

nym-connect/src-tauri/src/config/mod.rs

@@ -4,6 +4,7 @@ use crate::{
 };
 use client_core::config::Config as BaseConfig;
 use config_common::NymConfig;
+use crypto::asymmetric::identity;
 use nym_socks5::client::config::Config as Socks5Config;
 use std::path::PathBuf;
 use std::sync::Arc;
@@ -109,11 +110,7 @@ pub async fn init_socks5_config(provider_address: String, chosen_gateway_id: Str
     );
     let already_init = Config::config_file_location(&id)?.exists();
     if already_init {
-        log::info!(
-            "SOCKS5 client \"{}\" was already initialised before! \
-            Config information will be overwritten (but keys will be kept)!",
-            id
-        );
+        log::info!("SOCKS5 client \"{id}\" was already initialised before");
     }
 
     // Future proofing. This flag exists for the other clients
@@ -133,8 +130,11 @@ pub async fn init_socks5_config(provider_address: String, chosen_gateway_id: Str
             .set_custom_nym_apis(config_common::parse_urls(&raw_validators));
     }
 
+    let chosen_gateway_id = identity::PublicKey::from_base58_string(chosen_gateway_id)
+        .map_err(|_| BackendError::UnableToParseGateway)?;
+
     // Setup gateway by either registering a new one, or reusing exiting keys
-    let gateway = client_core::init::setup_gateway::<Socks5Config, _>(
+    let gateway = client_core::init::setup_gateway_from_config::<Socks5Config, _>(
         register_gateway,
         Some(chosen_gateway_id),
         config.get_base(),

nym-connect/src-tauri/src/error.rs

@@ -68,6 +68,8 @@ pub enum BackendError {
     CouldNotLoadExistingGatewayConfiguration(std::io::Error),
     #[error("Unable to open a new window")]
     NewWindowError,
+    #[error("Unable to parse the specified gateway")]
+    UnableToParseGateway,
 }
 
 impl Serialize for BackendError {

sdk/rust/nym-sdk/src/error.rs

@@ -8,6 +8,9 @@ pub enum Error {
     TomlSerializationError(#[from] toml::ser::Error),
     #[error("toml deserialization error: {0}")]
     TomlDeserializationError(#[from] toml::de::Error),
+    #[error("Ed25519 error: {0}")]
+    Ed25519RecoveryError(#[from] crypto::asymmetric::identity::Ed25519RecoveryError),
+
     #[error(transparent)]
     ClientCoreError(#[from] client_core::error::ClientCoreError),
 

sdk/rust/nym-sdk/src/mixnet/client.rs

@@ -13,13 +13,15 @@ use client_core::{
     },
     config::{persistence::key_pathfinder::ClientKeyPathfinder, GatewayEndpointConfig},
 };
-use futures::StreamExt;
+use crypto::asymmetric::identity;
 use nymsphinx::{
     addressing::clients::{ClientIdentity, Recipient},
     receiver::ReconstructedMessage,
 };
 use task::TaskManager;
 
+use futures::StreamExt;
+
 use super::{connection_state::BuilderState, Config, GatewayKeyMode, Keys, KeysArc, StoragePaths};
 use crate::error::{Error, Result};
 
@@ -124,10 +126,17 @@ impl ClientBuilder {
             "can only setup gateway when in `New` connection state"
         );
 
+        let user_chosen_gateway = self
+            .config
+            .user_chosen_gateway
+            .as_ref()
+            .map(identity::PublicKey::from_base58_string)
+            .transpose()?;
+
         let gateway_config = client_core::init::register_with_gateway(
             &mut self.key_manager,
             self.config.nym_api_endpoints.clone(),
-            self.config.user_chosen_gateway.clone(),
+            user_chosen_gateway,
         )
         .await?;