Improve error handling in bech32 decoding (#323)

* Improve error handling in bech32 decoding

Refactored the bech32 decoding function to improve error handling. Previously, an error occurred if the REV_CHARSET returned a value not within its range, due to direct indexing. This has now been altered to a safer "get" method, which outputs a default value of 100 in case of out-of-range input. This improvement increases the reliability and robustness of the code, effectively reducing the possibility of runtime failures.

* Add tests for invalid characters in bech32 decoding

Expanded the testing suite for the bech32 decoding function by including test cases with invalid characters. These additional tests check the function's error-handling capabilities when faced with unexpected characters, ensuring better response to erroneous inputs and thereby improving the robustness of the decoding process.

* Clippy fix. Optimize utxos iterations in main.rs

Changed the order of cloning and filtering iterations over utxos in main.rs, enhancing efficiency. By filtering prior to cloning, we reduce the number of objects we need to clone. This change results in less memory usage and quicker processing time.

* fix and supress clippy

* fmt fixes

* Update code by reinstating imports and suppressing clippy lints

* avoid using char keyword as variable name

* remove unneeded clippy allows

cli/src/modules/estimate.rs

@@ -16,7 +16,7 @@ impl Estimate {
             return Ok(());
         }
 
-        let amount_sompi = try_parse_required_nonzero_kaspa_as_sompi_u64(argv.get(0))?;
+        let amount_sompi = try_parse_required_nonzero_kaspa_as_sompi_u64(argv.first())?;
         let priority_fee_sompi = try_parse_optional_kaspa_as_sompi_i64(argv.get(1))?.unwrap_or(0);
         let abortable = Abortable::default();
 

cli/src/modules/export.rs

@@ -9,12 +9,12 @@ impl Export {
     async fn main(self: Arc<Self>, ctx: &Arc<dyn Context>, argv: Vec<String>, _cmd: &str) -> Result<()> {
         let ctx = ctx.clone().downcast_arc::<KaspaCli>()?;
 
-        if argv.is_empty() || argv.get(0) == Some(&"help".to_string()) {
+        if argv.is_empty() || argv.first() == Some(&"help".to_string()) {
             tprintln!(ctx, "usage: export [mnemonic]");
             return Ok(());
         }
 
-        let what = argv.get(0).unwrap();
+        let what = argv.first().unwrap();
         match what.as_str() {
             "mnemonic" => {
                 let account = ctx.account().await?;

cli/src/modules/message.rs

@@ -29,7 +29,7 @@ impl Message {
             return self.display_help(ctx, argv).await;
         }
 
-        match argv.get(0).unwrap().as_str() {
+        match argv.first().unwrap().as_str() {
             "sign" => {
                 if argv.len() != 2 {
                     return self.display_help(ctx, argv).await;

cli/src/modules/send.rs

@@ -16,7 +16,7 @@ impl Send {
             return Ok(());
         }
 
-        let address = Address::try_from(argv.get(0).unwrap().as_str())?;
+        let address = Address::try_from(argv.first().unwrap().as_str())?;
         let amount_sompi = try_parse_required_nonzero_kaspa_as_sompi_u64(argv.get(1))?;
         let priority_fee_sompi = try_parse_optional_kaspa_as_sompi_i64(argv.get(2))?.unwrap_or(0);
         let outputs = PaymentOutputs::from((address.clone(), amount_sompi));

cli/src/modules/transfer.rs

@@ -15,7 +15,7 @@ impl Transfer {
             return Ok(());
         }
 
-        let target_account = argv.get(0).unwrap();
+        let target_account = argv.first().unwrap();
         let target_account = ctx.find_accounts_by_name_or_id(target_account).await?;
         if target_account.id() == account.id() {
             return Err("Cannot transfer to the same account".into());

consensus/src/processes/ghostdag/protocol.rs

@@ -96,7 +96,7 @@ impl<T: GhostdagStoreReader, S: RelationsStoreReader, U: ReachabilityService, V:
         assert!(!parents.is_empty(), "genesis must be added via a call to init");
 
         // Run the GHOSTDAG parent selection algorithm
-        let selected_parent = self.find_selected_parent(&mut parents.iter().copied());
+        let selected_parent = self.find_selected_parent(parents.iter().copied());
         // Initialize new GHOSTDAG block data with the selected parent
         let mut new_block_data = GhostdagData::new_with_selected_parent(selected_parent, self.k);
         // Get the mergeset in consensus-agreed topological order (topological here means forward in time from blocks to children)

consensus/wasm/src/signable.rs

@@ -41,7 +41,7 @@ impl SignableTransaction {
 
     #[wasm_bindgen(js_name=getScriptHashes)]
     pub fn script_hashes(&self) -> Result<JsValue, JsError> {
-        let hashes = script_hashes(self.clone().try_into()?)?;
+        let hashes = script_hashes(self.clone().into())?;
         Ok(to_value(&hashes)?)
     }
 

crypto/addresses/src/bech32.rs

@@ -57,8 +57,8 @@ fn conv8to5(payload: &[u8]) -> Vec<u8> {
 
     let mut buff = 0u16;
     let mut bits = 0;
-    for char in payload.iter() {
-        buff = (buff << 8) | *char as u16;
+    for c in payload.iter() {
+        buff = (buff << 8) | *c as u16;
         bits += 8;
         while bits >= 5 {
             bits -= 5;
@@ -80,8 +80,8 @@ fn conv5to8(payload: &[u8]) -> Vec<u8> {
 
     let mut buff = 0u16;
     let mut bits = 0;
-    for char in payload.iter() {
-        buff = (buff << 5) | *char as u16;
+    for c in payload.iter() {
+        buff = (buff << 5) | *c as u16;
         bits += 5;
         while bits >= 8 {
             bits -= 8;
@@ -113,7 +113,7 @@ impl Address {
         let address_u5 = address
             .as_bytes()
             .iter()
-            .scan(&mut err, |err, b| match REV_CHARSET[*b as usize] {
+            .scan(&mut err, |err, b| match *REV_CHARSET.get(*b as usize).unwrap_or(&100) {
                 100 => {
                     **err = Err(AddressError::DecodingError(*b as char));
                     None

crypto/addresses/src/lib.rs

@@ -583,6 +583,16 @@ mod tests {
         let address: Result<Address, AddressError> = address_str.try_into();
         assert_eq!(Err(AddressError::DecodingError('1')), address);
 
+        let invalid_char = 124u8 as char;
+        let address_str: String = format!("kaspa:qqqqqqqqqqqqq{invalid_char}qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqkx9awp4e");
+        let address: Result<Address, AddressError> = address_str.try_into();
+        assert_eq!(Err(AddressError::DecodingError(invalid_char)), address);
+
+        let invalid_char = 129u8 as char;
+        let address_str: String = format!("kaspa:qqqqqqqqqqqqq{invalid_char}qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqkx9awp4e");
+        let address: Result<Address, AddressError> = address_str.try_into();
+        assert!(matches!(address, Err(AddressError::DecodingError(_))));
+
         let address_str: String = "kaspa1:qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqkx9awp4e".to_string();
         let address: Result<Address, AddressError> = address_str.try_into();
         assert_eq!(Err(AddressError::InvalidPrefix("kaspa1".into())), address);

protocol/flows/src/v5/request_pp_proof.rs

@@ -40,7 +40,7 @@ impl RequestPruningPointProofFlow {
             self.router
                 .enqueue(make_response!(
                     Payload::PruningPointProof,
-                    PruningPointProofMessage { headers: proof.iter().map(|headers| headers.try_into().unwrap()).collect() },
+                    PruningPointProofMessage { headers: proof.iter().map(|headers| headers.into()).collect() },
                     request_id
                 ))
                 .await?;

rothschild/src/main.rs

@@ -338,7 +338,7 @@ fn select_utxos(
     const MAX_UTXOS: usize = 84;
     let mut selected_amount: u64 = 0;
     let mut selected = Vec::new();
-    for (outpoint, entry) in utxos.iter().cloned().filter(|(op, _)| !pending.contains_key(op)) {
+    for (outpoint, entry) in utxos.iter().filter(|(op, _)| !pending.contains_key(op)).cloned() {
         selected_amount += entry.amount;
         selected.push((outpoint, entry));
 

rpc/core/src/lib.rs

@@ -15,10 +15,7 @@ pub mod prelude {
 }
 
 pub use api::notifications::*;
-pub use convert::block::*;
-pub use convert::notification::*;
-pub use convert::tx::*;
-pub use convert::utxo::*;
+pub use convert::{block::*, notification::*, tx::*, utxo::*};
 pub use error::*;
 pub use model::script_class::*;
 pub use model::*;

rpc/core/src/wasm/mod.rs

@@ -5,7 +5,7 @@ use std::sync::Arc;
 
 impl From<RpcUtxosByAddressesEntry> for UtxoEntry {
     fn from(entry: RpcUtxosByAddressesEntry) -> UtxoEntry {
-        UtxoEntry { address: entry.address, outpoint: entry.outpoint.try_into().unwrap(), entry: entry.utxo_entry }
+        UtxoEntry { address: entry.address, outpoint: entry.outpoint.into(), entry: entry.utxo_entry }
     }
 }
 

wallet/core/src/derivation/mod.rs

@@ -134,7 +134,7 @@ impl AddressManager {
         let is_multisig = manager_length > 1;
 
         if !is_multisig {
-            let keys = manager_keys.get(0).unwrap().clone();
+            let keys = manager_keys.first().unwrap().clone();
             let mut addresses = vec![];
             for key in keys {
                 addresses.push(self.create_address(vec![key])?);

wallet/core/src/storage/mod.rs

@@ -92,7 +92,7 @@ mod tests {
 
         assert_eq!(payload_json, serde_json::to_string(w2payload.as_ref())?);
 
-        let w2keydata1 = w2payload.as_ref().prv_key_data.get(0).unwrap();
+        let w2keydata1 = w2payload.as_ref().prv_key_data.first().unwrap();
         let w2keydata1_payload = w2keydata1.payload.decrypt(Some(&payment_secret)).unwrap();
         let first_mnemonic = &w2keydata1_payload.as_ref().as_mnemonic()?.unwrap().phrase_string();
         // println!("first mnemonic (plain): {}", hex_string(first_mnemonic.as_ref()));

wallet/core/src/storage/transaction.rs

@@ -420,7 +420,7 @@ impl TransactionRecord {
                 }
 
                 if include_utxos {
-                    for (_n, input) in transaction.inputs.iter().enumerate() {
+                    for input in transaction.inputs.iter() {
                         let TransactionInput { previous_outpoint, signature_script: _, sequence, sig_op_count } = input;
                         let TransactionOutpoint { transaction_id, index } = previous_outpoint;