Fix some findings from the Rust week (iotaledger#428)

* Return error instead of panic, use .replace(), use consts

* use consts for fixed values

* improve readability

bindings/nodejs/native/src/classes/client/builder.rs

@@ -57,7 +57,7 @@ declare_types! {
                 let mut this = cx.this();
                 let guard = cx.lock();
                 let auth = &mut this.borrow_mut(&guard).auth;
-                *auth = Some((node_url, name, password));
+                auth.replace((node_url, name, password));
             }
             Ok(cx.this().upcast())
         }
@@ -112,7 +112,7 @@ declare_types! {
                 let mut this = cx.this();
                 let guard = cx.lock();
                 let network = &mut this.borrow_mut(&guard).network;
-                *network = Some(network_name);
+                network.replace(network_name);
             }
             Ok(cx.this().upcast())
         }

iota-client/src/api/address.rs

@@ -43,7 +43,7 @@ impl<'a> GetAddressesBuilder<'a> {
 
     /// Provide a client to get the bech32_hrp from the node
     pub fn with_client(mut self, client: &'a Client) -> Self {
-        self.client = Some(client);
+        self.client.replace(client);
         self
     }
 
@@ -61,7 +61,7 @@ impl<'a> GetAddressesBuilder<'a> {
 
     /// Set bech32 human readable part (hrp)
     pub fn with_bech32_hrp(mut self, bech32_hrp: String) -> Self {
-        self.bech32_hrp = Some(bech32_hrp);
+        self.bech32_hrp.replace(bech32_hrp);
         self
     }
 

iota-client/src/api/balance.rs

@@ -25,13 +25,13 @@ impl<'a> GetBalanceBuilder<'a> {
 
     /// Sets the account index.
     pub fn with_account_index(mut self, account_index: usize) -> Self {
-        self.account_index = Some(account_index);
+        self.account_index.replace(account_index);
         self
     }
 
     /// Sets the index of the address to start looking for balance.
     pub fn with_initial_address_index(mut self, initial_address_index: usize) -> Self {
-        self.initial_address_index = Some(initial_address_index);
+        self.initial_address_index.replace(initial_address_index);
         self
     }
 

iota-client/src/api/message_builder.rs

@@ -21,6 +21,7 @@ use std::{
 
 const MAX_ALLOWED_DUST_OUTPUTS: i64 = 100;
 const ADDRESS_GAP_LIMIT: usize = 20;
+const DUST_THRESHOLD: u64 = 1_000_000;
 
 /// Structure for sorting of UnlockBlocks
 // TODO: move the sorting process to the `Message` crate
@@ -64,19 +65,19 @@ impl<'a> ClientMessageBuilder<'a> {
 
     /// Sets the seed.
     pub fn with_seed(mut self, seed: &'a Seed) -> Self {
-        self.seed = Some(seed);
+        self.seed.replace(seed);
         self
     }
 
     /// Sets the account index.
     pub fn with_account_index(mut self, account_index: usize) -> Self {
-        self.account_index = Some(account_index);
+        self.account_index.replace(account_index);
         self
     }
 
     /// Sets the index of the address to start looking for balance.
     pub fn with_initial_address_index(mut self, initial_address_index: usize) -> Self {
-        self.initial_address_index = Some(initial_address_index);
+        self.initial_address_index.replace(initial_address_index);
         self
     }
 
@@ -108,7 +109,7 @@ impl<'a> ClientMessageBuilder<'a> {
 
     /// Set a dust allowance transfer to the builder, address needs to be Bech32 encoded
     pub fn with_dust_allowance_output(mut self, address: &Bech32Address, amount: u64) -> Result<Self> {
-        if amount < 1_000_000 {
+        if amount < DUST_THRESHOLD {
             return Err(Error::DustError(
                 "Amount for SignatureLockedDustAllowanceOutput needs to be >= 1_000_000".into(),
             ));
@@ -128,13 +129,13 @@ impl<'a> ClientMessageBuilder<'a> {
 
     /// Set indexation to the builder
     pub fn with_index<I: AsRef<[u8]>>(mut self, index: I) -> Self {
-        self.index = Some(index.as_ref().into());
+        self.index.replace(index.as_ref().into());
         self
     }
 
     /// Set data to the builder
     pub fn with_data(mut self, data: Vec<u8>) -> Self {
-        self.data = Some(data);
+        self.data.replace(data);
         self
     }
 
@@ -143,7 +144,7 @@ impl<'a> ClientMessageBuilder<'a> {
         if !(1..=8).contains(&parent_ids.len()) {
             return Err(Error::InvalidParentsAmount(parent_ids.len()));
         }
-        self.parents = Some(parent_ids);
+        self.parents.replace(parent_ids);
         Ok(self)
     }
 
@@ -188,7 +189,7 @@ impl<'a> ClientMessageBuilder<'a> {
             match output {
                 Output::SignatureLockedSingle(x) => {
                     total_to_spend += x.amount();
-                    if x.amount() < 1_000_000 {
+                    if x.amount() < DUST_THRESHOLD {
                         dust_and_allowance_recorders.push((x.amount(), *x.address(), true));
                     }
                 }
@@ -210,12 +211,14 @@ impl<'a> ClientMessageBuilder<'a> {
                     if let Ok(output) = self.client.get_output(&input).await {
                         if !output.is_spent {
                             let (output_amount, output_address) = match output.output {
-                                OutputDto::Treasury(_) => panic!("Can't be used as input"),
+                                OutputDto::Treasury(_) => {
+                                    return Err(Error::OutputError("Treasury output is no supported"))
+                                }
                                 OutputDto::SignatureLockedSingle(r) => match r.address {
                                     AddressDto::Ed25519(addr) => {
                                         let output_address = Address::from(Ed25519Address::from_str(&addr.address)?);
                                         // Only add dust
-                                        if r.amount < 1_000_000 {
+                                        if r.amount < DUST_THRESHOLD {
                                             dust_and_allowance_recorders.push((r.amount, output_address, false));
                                         }
                                         (r.amount, output_address)
@@ -263,7 +266,7 @@ impl<'a> ClientMessageBuilder<'a> {
                             // Output the remaining tokens back to the original address
                             if total_already_spent > total_to_spend {
                                 let remaining_balance = total_already_spent - total_to_spend;
-                                if remaining_balance < 1_000_000 {
+                                if remaining_balance < DUST_THRESHOLD {
                                     dust_and_allowance_recorders.push((remaining_balance, output_address, true));
                                 }
                                 outputs_for_essence
@@ -306,10 +309,12 @@ impl<'a> ClientMessageBuilder<'a> {
                         }
                         for (_offset, output) in outputs.into_iter().enumerate() {
                             let output_amount = match output.output {
-                                OutputDto::Treasury(_) => panic!("Can't be used as input"),
+                                OutputDto::Treasury(_) => {
+                                    return Err(Error::OutputError("Treasury output is no supported"))
+                                }
                                 OutputDto::SignatureLockedSingle(r) => match r.address {
                                     AddressDto::Ed25519(addr) => {
-                                        if r.amount < 1_000_000 {
+                                        if r.amount < DUST_THRESHOLD {
                                             let output_address =
                                                 Address::from(Ed25519Address::from_str(&addr.address)?);
                                             dust_and_allowance_recorders.push((r.amount, output_address, false));
@@ -357,7 +362,7 @@ impl<'a> ClientMessageBuilder<'a> {
                                         });
                                         if total_already_spent > total_to_spend {
                                             let remaining_balance = total_already_spent - total_to_spend;
-                                            if remaining_balance < 1_000_000 {
+                                            if remaining_balance < DUST_THRESHOLD {
                                                 dust_and_allowance_recorders.push((
                                                     remaining_balance,
                                                     Address::try_from_bech32(address)?,
@@ -549,15 +554,15 @@ async fn is_dust_allowed(client: &Client, address: Bech32Address, outputs: Vec<(
         match output.2 {
             // add newly created outputs
             true => {
-                if output.0 >= 1_000_000 {
+                if output.0 >= DUST_THRESHOLD {
                     dust_allowance_balance += output.0 as i64;
                 } else {
                     dust_outputs_amount += 1
                 }
             }
             // remove consumed outputs
             false => {
-                if output.0 >= 1_000_000 {
+                if output.0 >= DUST_THRESHOLD {
                     dust_allowance_balance -= output.0 as i64;
                 } else {
                     dust_outputs_amount -= 1;
@@ -575,7 +580,7 @@ async fn is_dust_allowed(client: &Client, address: Bech32Address, outputs: Vec<(
                 dust_allowance_balance += d_a_o.amount as i64;
             }
             OutputDto::SignatureLockedSingle(s_o) => {
-                if s_o.amount < 1_000_000 {
+                if s_o.amount < DUST_THRESHOLD {
                     dust_outputs_amount += 1;
                 }
             }

iota-client/src/api/unspent.rs

@@ -5,6 +5,8 @@ use crate::{Client, Error, Result};
 use bee_message::prelude::Bech32Address;
 use crypto::keys::slip10::Seed;
 
+const ADDRESS_GAP_LIMIT: usize = 20;
+
 /// Builder of get_unspent_address API
 pub struct GetUnspentAddressBuilder<'a> {
     client: &'a Client,
@@ -26,13 +28,13 @@ impl<'a> GetUnspentAddressBuilder<'a> {
 
     /// Sets the account index.
     pub fn with_account_index(mut self, account_index: usize) -> Self {
-        self.account_index = Some(account_index);
+        self.account_index.replace(account_index);
         self
     }
 
     /// Sets the index of the address to start looking for balance.
     pub fn with_initial_address_index(mut self, initial_address_index: usize) -> Self {
-        self.initial_address_index = Some(initial_address_index);
+        self.initial_address_index.replace(initial_address_index);
         self
     }
 
@@ -47,7 +49,7 @@ impl<'a> GetUnspentAddressBuilder<'a> {
                 .client
                 .get_addresses(self.seed)
                 .with_account_index(account_index)
-                .with_range(index..index + 20)
+                .with_range(index..index + ADDRESS_GAP_LIMIT)
                 .finish()
                 .await?;
 
@@ -57,7 +59,7 @@ impl<'a> GetUnspentAddressBuilder<'a> {
                 let address_balance = self.client.get_address().balance(&a).await?;
                 match address_balance.balance {
                     0 => {
-                        address = Some(a);
+                        address.replace(a);
                         break;
                     }
                     _ => index += 1,

iota-client/src/builder.rs

@@ -21,6 +21,8 @@ pub(crate) const GET_API_TIMEOUT: Duration = Duration::from_millis(2000);
 const NODE_SYNC_INTERVAL: Duration = Duration::from_secs(60);
 // Interval in seconds when new tips will be requested during PoW
 const TIPS_INTERVAL: u64 = 15;
+const DEFAULT_MIN_POW: f64 = 4000f64;
+const DEFAULT_BECH32_HRP: &str = "iota";
 
 /// Struct containing network and PoW related information
 #[derive(Serialize, Deserialize, Clone, Debug, PartialEq)]
@@ -67,9 +69,9 @@ impl Default for ClientBuilder {
             network_info: NetworkInfo {
                 network: None,
                 network_id: None,
-                min_pow_score: 4000f64,
+                min_pow_score: DEFAULT_MIN_POW,
                 local_pow: true,
-                bech32_hrp: "iota".into(),
+                bech32_hrp: DEFAULT_BECH32_HRP.into(),
                 tips_interval: TIPS_INTERVAL,
             },
             request_timeout: DEFAULT_REQUEST_TIMEOUT,
@@ -144,7 +146,7 @@ impl ClientBuilder {
     /// Nodes that don't belong to this network are ignored. Default nodes are only used when no other nodes are
     /// provided.
     pub fn with_network(mut self, network: &str) -> Self {
-        self.network_info.network = Some(network.into());
+        self.network_info.network.replace(network.into());
         self
     }
 

iota-client/src/error.rs

@@ -116,4 +116,7 @@ pub enum Error {
     /// Blake2b256 Error
     #[error("{0}")]
     Blake2b256Error(&'static str),
+    /// Output Error
+    #[error("Output error: {0}")]
+    OutputError(&'static str),
 }

iota-client/src/node/address.rs

@@ -9,6 +9,9 @@ use bee_rest_api::types::responses::{BalanceForAddressResponse, OutputsForAddres
 
 use std::convert::TryInto;
 
+const OUTPUT_ID_LENGTH: usize = 68;
+const TRANSACTION_ID_LENGTH: usize = 64;
+
 /// Output type filter.
 #[derive(Clone)]
 pub enum OutputType {
@@ -45,9 +48,10 @@ impl OutputsOptions {
         if let Some(output_type) = self.output_type {
             params.push(format!("type={}", u16::from(output_type)))
         }
-        match params.len() {
-            0 => None,
-            _ => Some(params.join("&")),
+        if params.is_empty() {
+            None
+        } else {
+            Some(params.join("&"))
         }
     }
 }
@@ -112,14 +116,18 @@ impl<'a> GetAddressBuilder<'a> {
             .output_ids
             .iter()
             .map(|s| {
-                let mut transaction_id = [0u8; 32];
-                hex::decode_to_slice(&s[..64], &mut transaction_id)?;
-                let index = u16::from_le_bytes(
-                    hex::decode(&s[64..]).map_err(|_| Error::InvalidParameter("index"))?[..]
-                        .try_into()
-                        .map_err(|_| Error::InvalidParameter("index"))?,
-                );
-                Ok(UTXOInput::new(TransactionId::new(transaction_id), index)?)
+                if s.len() == OUTPUT_ID_LENGTH {
+                    let mut transaction_id = [0u8; 32];
+                    hex::decode_to_slice(&s[..TRANSACTION_ID_LENGTH], &mut transaction_id)?;
+                    let index = u16::from_le_bytes(
+                        hex::decode(&s[TRANSACTION_ID_LENGTH..]).map_err(|_| Error::InvalidParameter("index"))?[..]
+                            .try_into()
+                            .map_err(|_| Error::InvalidParameter("index"))?,
+                    );
+                    Ok(UTXOInput::new(TransactionId::new(transaction_id), index)?)
+                } else {
+                    Err(Error::OutputError("Invalid output length from API response"))
+                }
             })
             .collect::<Result<Box<[UTXOInput]>>>()
     }

iota-client/src/node/message.rs

@@ -94,7 +94,7 @@ impl<'a> GetMessageBuilder<'a> {
         Ok(resp.data)
     }
 
-    /// GET /api/v1/messages/{messageID}/children endpoint
+    /// GET /api/v1/messages/{messageID}/raw endpoint
     /// Consume the builder and find a message by its identifer. This method returns the given message raw data.
     pub async fn raw(self, message_id: &MessageId) -> Result<String> {
         let mut url = self.client.get_node().await?;
@@ -111,6 +111,7 @@ impl<'a> GetMessageBuilder<'a> {
         Ok(resp)
     }
 
+    /// GET /api/v1/messages/{messageID}/children endpoint
     /// Consume the builder and returns the list of message IDs that reference a message by its identifier.
     pub async fn children(self, message_id: &MessageId) -> Result<Box<[MessageId]>> {
         let mut url = self.client.get_node().await?;

iota-client/src/node/mqtt.rs

@@ -89,7 +89,7 @@ async fn get_mqtt_client(client: &mut Client) -> Result<&mut MqttClient> {
                 // if we found a valid mqtt connection, loop it on a separate thread
                 if got_ack {
                     let (mqtt_client, connection) = MqttClient::new(mqttoptions, 10);
-                    client.mqtt_client = Some(mqtt_client);
+                    client.mqtt_client.replace(mqtt_client);
                     poll_mqtt(client.mqtt_topic_handlers.clone(), connection);
                 }
             }
@@ -213,11 +213,9 @@ impl<'a> MqttManager<'a> {
             client.disconnect().await?;
             self.client.mqtt_client = None;
 
-            {
-                let mqtt_topic_handlers = &self.client.mqtt_topic_handlers;
-                let mut mqtt_topic_handlers = mqtt_topic_handlers.write().await;
-                mqtt_topic_handlers.clear()
-            }
+            let mqtt_topic_handlers = &self.client.mqtt_topic_handlers;
+            let mut mqtt_topic_handlers = mqtt_topic_handlers.write().await;
+            mqtt_topic_handlers.clear()
         }
 
         Ok(())