Refactor utxo return address result

components/consensusmanager/src/session.rs

@@ -4,15 +4,15 @@
 
 use kaspa_consensus_core::{
     acceptance_data::AcceptanceData,
-    api::{BlockCount, BlockValidationFutures, ConsensusApi, ConsensusStats, DynConsensus},
+    api::{BlockCount, BlockValidationFutures, ConsensusApi, ConsensusStats, DynConsensus, ReturnAddress},
     block::Block,
     blockstatus::BlockStatus,
     daa_score_timestamp::DaaScoreTimestamp,
     errors::consensus::ConsensusResult,
     header::Header,
     pruning::{PruningPointProof, PruningPointTrustedData, PruningPointsList},
     trusted::{ExternalGhostdagData, TrustedBlock},
-    tx::{MutableTransaction, ScriptPublicKey, Transaction, TransactionOutpoint, UtxoEntry},
+    tx::{MutableTransaction, Transaction, TransactionOutpoint, UtxoEntry},
     BlockHashSet, BlueWorkType, ChainPath, Hash,
 };
 use kaspa_utils::sync::rwlock::*;
@@ -308,12 +308,8 @@ impl ConsensusSessionOwned {
         self.clone().spawn_blocking(|c| c.get_chain_block_samples()).await
     }
 
-    pub async fn async_get_utxo_return_script_public_key(
-        &self,
-        txid: Hash,
-        accepting_block_daa_score: u64,
-    ) -> Option<ScriptPublicKey> {
-        self.clone().spawn_blocking(move |c| c.get_utxo_return_script_public_key(txid, accepting_block_daa_score)).await
+    pub async fn async_get_utxo_return_script_public_key(&self, txid: Hash, accepting_block_daa_score: u64) -> ReturnAddress {
+        self.clone().spawn_blocking(move |c| c.get_utxo_return_address(txid, accepting_block_daa_score)).await
     }
 
     /// Returns the antipast of block `hash` from the POV of `context`, i.e. `antipast(hash) ∩ past(context)`.

consensus/core/src/api/mod.rs

@@ -1,6 +1,10 @@
 use futures_util::future::BoxFuture;
+use kaspa_addresses::Address;
 use kaspa_muhash::MuHash;
-use std::sync::Arc;
+use std::{
+    fmt::{Display, Formatter},
+    sync::Arc,
+};
 
 use crate::{
     acceptance_data::AcceptanceData,
@@ -19,7 +23,7 @@ use crate::{
     header::Header,
     pruning::{PruningPointProof, PruningPointTrustedData, PruningPointsList},
     trusted::{ExternalGhostdagData, TrustedBlock},
-    tx::{MutableTransaction, ScriptPublicKey, Transaction, TransactionOutpoint, UtxoEntry},
+    tx::{MutableTransaction, Transaction, TransactionOutpoint, UtxoEntry},
     BlockHashSet, BlueWorkType, ChainPath,
 };
 use kaspa_hashes::Hash;
@@ -43,6 +47,31 @@ pub struct BlockValidationFutures {
     pub virtual_state_task: BlockValidationFuture,
 }
 
+#[derive(Debug, Clone)]
+pub enum ReturnAddress {
+    Found(Address),
+    AlreadyPruned,
+    TxFromCoinbase,
+    NoTxAtScore,
+    NonStandard,
+    NotFound(String),
+}
+
+impl Display for ReturnAddress {
+    #[inline]
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        let s = match self {
+            ReturnAddress::AlreadyPruned => "Transaction is already pruned".to_string(),
+            ReturnAddress::NoTxAtScore => "Transaction not found at given accepting daa score".to_string(),
+            ReturnAddress::NonStandard => "Transaction was found but not standard".to_string(),
+            ReturnAddress::TxFromCoinbase => "Transaction return address is coinbase".to_string(),
+            ReturnAddress::NotFound(reason) => format!("Transaction return address not found: {}", reason),
+            ReturnAddress::Found(address) => address.to_string(),
+        };
+        f.write_str(&s)
+    }
+}
+
 /// Abstracts the consensus external API
 #[allow(unused_variables)]
 pub trait ConsensusApi: Send + Sync {
@@ -165,7 +194,7 @@ pub trait ConsensusApi: Send + Sync {
         unimplemented!()
     }
 
-    fn get_utxo_return_script_public_key(&self, txid: Hash, daa_score: u64) -> Option<ScriptPublicKey> {
+    fn get_utxo_return_address(&self, txid: Hash, daa_score: u64) -> ReturnAddress {
         unimplemented!()
     }
 

consensus/src/consensus/mod.rs

@@ -46,7 +46,7 @@ use kaspa_consensus_core::{
     api::{
         args::{TransactionValidationArgs, TransactionValidationBatchArgs},
         stats::BlockCount,
-        BlockValidationFutures, ConsensusApi, ConsensusStats,
+        BlockValidationFutures, ConsensusApi, ConsensusStats, ReturnAddress,
     },
     block::{Block, BlockTemplate, TemplateBuildMode, TemplateTransactionSelector, VirtualStateApproxId},
     blockhash::BlockHashExtensions,
@@ -66,7 +66,7 @@ use kaspa_consensus_core::{
     network::NetworkType,
     pruning::{PruningPointProof, PruningPointTrustedData, PruningPointsList},
     trusted::{ExternalGhostdagData, TrustedBlock},
-    tx::{MutableTransaction, ScriptPublicKey, Transaction, TransactionOutpoint, UtxoEntry},
+    tx::{MutableTransaction, Transaction, TransactionOutpoint, UtxoEntry},
     BlockHashSet, BlueWorkType, ChainPath, HashMapCustomHasher,
 };
 use kaspa_consensus_notify::root::ConsensusNotificationRoot;
@@ -81,7 +81,7 @@ use kaspa_core::{trace, warn};
 use kaspa_database::prelude::StoreResultExtensions;
 use kaspa_hashes::Hash;
 use kaspa_muhash::MuHash;
-use kaspa_txscript::caches::TxScriptCacheCounters;
+use kaspa_txscript::{caches::TxScriptCacheCounters, extract_script_pub_key_address};
 
 use std::{
     cmp::Reverse,
@@ -688,13 +688,20 @@ impl ConsensusApi for Consensus {
         sample_headers
     }
 
-    fn get_utxo_return_script_public_key(&self, txid: Hash, target_daa_score: u64) -> Option<ScriptPublicKey> {
+    fn get_utxo_return_address(&self, txid: Hash, target_daa_score: u64) -> ReturnAddress {
         // We need consistency between the past pruning points, selected chain and header store reads
         let _guard = self.pruning_lock.blocking_read();
 
         let sc_read = self.selected_chain_store.read();
 
         let pp_hash = self.pruning_point_store.read().get().unwrap().pruning_point;
+
+        // Pruning Point hash is always expected to be in get_compact_header_data so unwrap should never fail
+        if target_daa_score < self.headers_store.get_compact_header_data(pp_hash).unwrap().daa_score {
+            // Early exit if target daa score is lower than that of pruning point's daa score:
+            return ReturnAddress::AlreadyPruned;
+        }
+
         let pp_index = sc_read.get_by_hash(pp_hash).unwrap();
         let (tip_index, tip_hash) = sc_read.get_tip().unwrap();
         let tip_daa_score = self.headers_store.get_compact_header_data(tip_hash).unwrap().daa_score;
@@ -708,29 +715,28 @@ impl ConsensusApi for Consensus {
             let mid = low_index + (high_index - low_index) / 2;
 
             // 1. Get the chain block hash at that index. Error if we don't find a hash at an index
-            let hash = sc_read
-                .get_by_index(mid)
-                .map_err(|err| {
+            let hash = match sc_read.get_by_index(mid) {
+                Ok(hash) => hash,
+                Err(_) => {
                     trace!("Did not find a hash at index {}", mid);
-                    err
-                })
-                .ok()?;
+                    return ReturnAddress::NotFound(format!("Did not find a hash at index {}", mid));
+                }
+            };
 
             // 2. Get the compact header so we have access to the daa_score. Error if we
-            let compact_header = self
-                .headers_store
-                .get_compact_header_data(hash)
-                .map_err(|err| {
+            let compact_header = match self.headers_store.get_compact_header_data(hash) {
+                Ok(compact_header) => compact_header,
+                Err(_) => {
                     trace!("Did not find a compact header with hash {}", hash);
-                    err
-                })
-                .ok()?;
+                    return ReturnAddress::NotFound(format!("Did not find a compact header with hash {}", hash));
+                }
+            };
 
             // 3. Compare block daa score to our target
             match compact_header.daa_score.cmp(&target_daa_score) {
                 cmp::Ordering::Equal => {
                     // We found the chain block we need
-                    break Some(hash);
+                    break hash;
                 }
                 cmp::Ordering::Greater => {
                     high_index = mid - 1;
@@ -741,38 +747,59 @@ impl ConsensusApi for Consensus {
             }
 
             if low_index > high_index {
-                break None;
+                return ReturnAddress::NoTxAtScore;
             }
-        }?;
+        };
 
-        let acceptance_data = self.acceptance_data_store.get(matching_chain_block_hash).ok()?;
-        let (index, containing_acceptance) = acceptance_data.iter().find_map(|mbad| {
+        let acceptance_data = match self.acceptance_data_store.get(matching_chain_block_hash) {
+            Ok(acceptance_data) => acceptance_data,
+            Err(_) => {
+                return ReturnAddress::NotFound("Did not find acceptance data".to_string());
+            }
+        };
+        let (index, containing_acceptance) = match acceptance_data.iter().find_map(|mbad| {
             let tx_arr_index =
                 mbad.accepted_transactions.iter().enumerate().find_map(|(index, tx)| (tx.transaction_id == txid).then_some(index));
             tx_arr_index.map(|index| (index, mbad.clone()))
-        })?;
+        }) {
+            Some((index, containing_acceptance)) => (index, containing_acceptance),
+            None => {
+                return ReturnAddress::NotFound("Did not find containing_acceptance".to_string());
+            }
+        };
 
         // Found Merged block containing the TXID
         let tx = &self.block_transactions_store.get(containing_acceptance.block_hash).unwrap()[index];
 
         if tx.id() != txid {
             // Should never happen, but do a sanity check. This would mean something went wrong with storing block transactions
-            // Sanity check is necessary to guarantee that this function will never give back a wrong address (err on the side of None)
+            // Sanity check is necessary to guarantee that this function will never give back a wrong address (err on the side of NotFound)
             warn!("Expected {} to match {} when checking block_transaction_store using array index of transaction", tx.id(), txid);
-            return None;
+            return ReturnAddress::NotFound(format!(
+                "Expected {} to match {} when checking block_transaction_store using array index of transaction",
+                tx.id(),
+                txid
+            ));
         }
 
         if tx.inputs.is_empty() {
             // A transaction may have no inputs (like a coinbase transaction)
-            return None;
+            return ReturnAddress::TxFromCoinbase;
         }
 
         let first_input_prev_outpoint = &tx.inputs[0].previous_outpoint;
         // Expected to never fail, since we found the acceptance data and therefore there must be matching diff
         let utxo_diff = self.utxo_diffs_store.get(matching_chain_block_hash).unwrap();
         let removed_diffs = utxo_diff.removed();
 
-        Some(removed_diffs.get(first_input_prev_outpoint)?.script_public_key.clone())
+        if let Ok(address) = extract_script_pub_key_address(
+            &removed_diffs.get(first_input_prev_outpoint).unwrap().script_public_key,
+            self.config.prefix(),
+        ) {
+            ReturnAddress::Found(address)
+        } else {
+            ReturnAddress::NonStandard
+        }
     }
 
     fn get_virtual_parents(&self) -> BlockHashSet {

rpc/core/src/error.rs

@@ -1,4 +1,4 @@
-use kaspa_consensus_core::{subnets::SubnetworkConversionError, tx::TransactionId};
+use kaspa_consensus_core::{api::ReturnAddress, subnets::SubnetworkConversionError, tx::TransactionId};
 use kaspa_utils::networking::IpAddress;
 use std::{net::AddrParseError, num::TryFromIntError};
 use thiserror::Error;
@@ -130,6 +130,9 @@ pub enum RpcError {
 
     #[error(transparent)]
     ConsensusClient(#[from] kaspa_consensus_client::error::Error),
+
+    #[error("utxo return address could not be found -> {0}")]
+    UtxoReturnAddressNotFound(ReturnAddress),
 }
 
 impl From<String> for RpcError {

rpc/service/src/service.rs

@@ -6,6 +6,7 @@ use crate::converter::{consensus::ConsensusConverter, index::IndexConverter, pro
 use crate::service::NetworkType::{Mainnet, Testnet};
 use async_trait::async_trait;
 use kaspa_consensus_core::api::counters::ProcessingCounters;
+use kaspa_consensus_core::api::ReturnAddress;
 use kaspa_consensus_core::errors::block::RuleError;
 use kaspa_consensus_core::{
     block::Block,
@@ -789,13 +790,11 @@ NOTE: This error usually indicates an RPC conversion error between the node and
         let session = self.consensus_manager.consensus().session().await;
 
         // Convert a SPK to an Address
-        let return_address = if let Some(spk) =
-            session.async_get_utxo_return_script_public_key(request.txid, request.accepting_block_daa_score).await
-        {
-            extract_script_pub_key_address(&spk, self.config.prefix()).ok()
-        } else {
-            None
-        };
+        let return_address =
+            match session.async_get_utxo_return_script_public_key(request.txid, request.accepting_block_daa_score).await {
+                ReturnAddress::Found(address) => Some(address),
+                other => return Err(RpcError::UtxoReturnAddressNotFound(other)),
+            };
 
         Ok(GetUtxoReturnAddressResponse { return_address })
     }