fix(utxo): avoid index out of bounds panics in tx_details_by_hash (#1915

)

This commit addresses index out of bounds errors in the `tx_details_by_hash` functions. The changes replace direct array access with safer methods, avoiding potential panics due to out-of-range indices. In addition, error handling around the function was improved to log and skip transactions in case of errors.

mm2src/coins/utxo/bch.rs

@@ -371,7 +371,16 @@ impl BchCoin {
                 params.my_addresses,
             )
             .await?;
-        let maybe_op_return: Script = tx.outputs[0].script_pubkey.clone().into();
+        let maybe_op_return: Script = tx
+            .outputs
+            .get(0)
+            .ok_or(UtxoTxDetailsError::Internal(format!(
+                "Transaction {} has no outputs",
+                params.hash
+            )))?
+            .script_pubkey
+            .clone()
+            .into();
         if !(maybe_op_return.is_pay_to_public_key_hash()
             || maybe_op_return.is_pay_to_public_key()
             || maybe_op_return.is_pay_to_script_hash())

mm2src/coins/utxo/utxo_common.rs

@@ -2928,11 +2928,20 @@ where
                     if e.get().should_update_timestamp() || e.get().firo_negative_fee() {
                         mm_counter!(ctx.metrics, "tx.history.request.count", 1, "coin" => coin.as_ref().conf.ticker.clone(), "method" => "tx_detail_by_hash");
 
-                        if let Ok(tx_details) = coin.tx_details_by_hash(&txid.0, &mut input_transactions).await {
-                            mm_counter!(ctx.metrics, "tx.history.response.count", 1, "coin" => coin.as_ref().conf.ticker.clone(), "method" => "tx_detail_by_hash");
-                            // replace with new tx details in case we need to update any data
-                            e.insert(tx_details);
-                            updated = true;
+                        match coin.tx_details_by_hash(&txid.0, &mut input_transactions).await {
+                            Ok(tx_details) => {
+                                mm_counter!(ctx.metrics, "tx.history.response.count", 1, "coin" => coin.as_ref().conf.ticker.clone(), "method" => "tx_detail_by_hash");
+                                // replace with new tx details in case we need to update any data
+                                e.insert(tx_details);
+                                updated = true;
+                            },
+                            Err(e) => log_tag!(
+                                ctx,
+                                "",
+                                "tx_history",
+                                "coin" => coin.as_ref().conf.ticker;
+                                fmt = "Error {:?} on getting the details of {:?}, skipping the tx", e, txid
+                            ),
                         }
                     }
                 },
@@ -3116,16 +3125,19 @@ pub async fn tx_details_by_hash<T: UtxoCommonOps>(
         let prev_tx = &mut prev_tx.tx;
         prev_tx.tx_hash_algo = coin.as_ref().tx_hash_algo;
 
-        let prev_tx_value = prev_tx.outputs[input.previous_output.index as usize].value;
-        input_amount += prev_tx_value;
-        let from: Vec<Address> = try_s!(coin.addresses_from_script(
-            &prev_tx.outputs[input.previous_output.index as usize]
-                .script_pubkey
-                .clone()
-                .into()
-        ));
+        let prev_output_index: usize = try_s!(input.previous_output.index.try_into());
+        let prev_tx_output = prev_tx.outputs.get(prev_output_index).ok_or(ERRL!(
+            "Previous output index is out of bound: coin={}, prev_output_index={}, prev_tx_hash={}, tx_hash={}, tx_hex={:02x}",
+            ticker,
+            prev_output_index,
+            prev_tx_hash,
+            hash,
+            verbose_tx.hex,
+        ))?;
+        input_amount += prev_tx_output.value;
+        let from: Vec<Address> = try_s!(coin.addresses_from_script(&prev_tx_output.script_pubkey.clone().into()));
         if from.contains(my_address) {
-            spent_by_me += prev_tx_value;
+            spent_by_me += prev_tx_output.value;
         }
         from_addresses.extend(from.into_iter());
     }

mm2src/coins/utxo/utxo_common/utxo_tx_history_v2_common.rs

@@ -9,8 +9,8 @@ use crate::utxo::utxo_tx_history_v2::{UtxoMyAddressesHistoryError, UtxoTxDetails
                                       UtxoTxHistoryOps};
 use crate::utxo::{output_script, RequestTxHistoryResult, UtxoCoinFields, UtxoCommonOps, UtxoHDAccount};
 use crate::{big_decimal_from_sat_unsigned, compare_transactions, BalanceResult, CoinWithDerivationMethod,
-            DerivationMethod, HDAccountAddressId, MarketCoinOps, TransactionDetails, TxFeeDetails, TxIdHeight,
-            UtxoFeeDetails, UtxoTx};
+            DerivationMethod, HDAccountAddressId, MarketCoinOps, NumConversError, TransactionDetails, TxFeeDetails,
+            TxIdHeight, UtxoFeeDetails, UtxoTx};
 use common::jsonrpc_client::JsonRpcErrorType;
 use crypto::Bip44Chain;
 use futures::compat::Future01CompatExt;
@@ -22,8 +22,9 @@ use mm2_number::BigDecimal;
 use rpc::v1::types::{TransactionInputEnum, H256 as H256Json};
 use serialization::deserialize;
 use std::collections::{HashMap, HashSet};
-use std::convert::TryFrom;
+use std::convert::{TryFrom, TryInto};
 use std::iter;
+use std::num::TryFromIntError;
 
 /// [`CoinWithTxHistoryV2::history_wallet_id`] implementation.
 pub fn history_wallet_id(coin: &UtxoCoinFields) -> WalletId { WalletId::new(coin.conf.ticker.clone()) }
@@ -190,13 +191,20 @@ where
 
         let prev_tx = coin.tx_from_storage_or_rpc(&prev_tx_hash, params.storage).await?;
 
-        let prev_output_index = input.previous_output.index as usize;
-        let prev_tx_value = prev_tx.outputs[prev_output_index].value;
-        let prev_script = prev_tx.outputs[prev_output_index].script_pubkey.clone().into();
+        let prev_output_index: usize = input.previous_output.index.try_into().map_to_mm(|e: TryFromIntError| {
+            UtxoTxDetailsError::NumConversionErr(NumConversError::new(e.to_string()))
+        })?;
+        let prev_tx_output = prev_tx.outputs.get(prev_output_index).ok_or_else(|| {
+            UtxoTxDetailsError::Internal(format!(
+                "Previous output index is out of bound: coin={}, prev_output_index={}, prev_tx_hash={}, tx_hash={}, tx_hex={:02x}",
+                ticker, prev_output_index, prev_tx_hash, params.hash, verbose_tx.hex
+            ))
+        })?;
 
-        input_amount += prev_tx_value;
-        let amount = big_decimal_from_sat_unsigned(prev_tx_value, decimals);
+        input_amount += prev_tx_output.value;
+        let amount = big_decimal_from_sat_unsigned(prev_tx_output.value, decimals);
 
+        let prev_script = prev_tx_output.script_pubkey.clone().into();
         let from: Vec<Address> = coin
             .addresses_from_script(&prev_script)
             .map_to_mm(UtxoTxDetailsError::TxAddressDeserializationError)?;

mm2src/mm2_main/src/lp_swap/taker_swap.rs

@@ -324,7 +324,7 @@ impl TakerSavedSwap {
             Some(event) => match &event.event {
                 TakerSwapEvent::Negotiated(neg) => {
                     let Some(key) = neg.maker_coin_htlc_pubkey else {
-                         return ERR!("maker's pubkey is empty");
+                        return ERR!("maker's pubkey is empty");
                     };
                     key.to_string()
                 },