txindex: return optional tx and block hash from FindTx

Co-authored-by: l0rinc <pap.lorinc@gmail.com>

src/index/txindex.cpp

@@ -24,6 +24,7 @@
 #include <cstdint>
 #include <cstdio>
 #include <exception>
+#include <optional>
 #include <string>
 #include <utility>
 #include <vector>
@@ -90,31 +91,31 @@ bool TxIndex::CustomAppend(const interfaces::BlockInfo& block)
 
 BaseIndex::DB& TxIndex::GetDB() const { return *m_db; }
 
-bool TxIndex::FindTx(const Txid& tx_hash, uint256& block_hash, CTransactionRef& tx) const
+std::optional<TxIndexResult> TxIndex::FindTx(const Txid& tx_hash) const
 {
     CDiskTxPos postx;
     if (!m_db->ReadTxPos(tx_hash, postx)) {
-        return false;
+        return std::nullopt;
     }
 
-    AutoFile file{m_chainstate->m_blockman.OpenBlockFile(postx, true)};
+    AutoFile file{m_chainstate->m_blockman.OpenBlockFile(postx, /*fReadOnly=*/true)};
     if (file.IsNull()) {
         LogError("OpenBlockFile failed");
-        return false;
+        return std::nullopt;
     }
     CBlockHeader header;
+    CTransactionRef tx;
     try {
         file >> header;
         file.seek(postx.nTxOffset, SEEK_CUR);
         file >> TX_WITH_WITNESS(tx);
     } catch (const std::exception& e) {
         LogError("Deserialize or I/O error - %s", e.what());
-        return false;
+        return std::nullopt;
     }
     if (tx->GetHash() != tx_hash) {
         LogError("txid mismatch");
-        return false;
+        return std::nullopt;
     }
-    block_hash = header.GetHash();
-    return true;
+    return TxIndexResult{header.GetHash(), std::move(tx)};
 }

src/index/txindex.h

@@ -7,17 +7,24 @@
 
 #include <index/base.h>
 #include <primitives/transaction.h>
+#include <uint256.h>
 
 #include <cstddef>
 #include <memory>
+#include <optional>
 
-class uint256;
 namespace interfaces {
 class Chain;
 }
 
 static constexpr bool DEFAULT_TXINDEX{false};
 
+/// A found transaction and the hash of the block that contains it.
+struct TxIndexResult {
+    uint256 block_hash;
+    CTransactionRef tx;
+};
+
 /**
  * TxIndex is used to look up transactions included in the blockchain by hash.
  * The index is written to a LevelDB database and records the filesystem
@@ -48,10 +55,8 @@ public:
     /// Look up a transaction by hash.
     ///
     /// @param[in]   tx_hash  The hash of the transaction to be returned.
-    /// @param[out]  block_hash  The hash of the block the transaction is found in.
-    /// @param[out]  tx  The transaction itself.
-    /// @return  true if transaction is found, false otherwise
-    bool FindTx(const Txid& tx_hash, uint256& block_hash, CTransactionRef& tx) const;
+    /// @return  the transaction and containing block hash, or nullopt if it is not found
+    std::optional<TxIndexResult> FindTx(const Txid& tx_hash) const;
 };
 
 /// The global transaction index, used in GetTransaction. May be null.

src/node/transaction.cpp

@@ -146,15 +146,13 @@ CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMe
         if (ptx) return ptx;
     }
     if (g_txindex) {
-        CTransactionRef tx;
-        uint256 block_hash;
-        if (g_txindex->FindTx(hash, block_hash, tx)) {
-            if (!block_index || block_index->GetBlockHash() == block_hash) {
+        if (auto result{g_txindex->FindTx(hash)}) {
+            if (!block_index || block_index->GetBlockHash() == result->block_hash) {
                 // Don't return the transaction if the provided block hash doesn't match.
                 // The case where a transaction appears in multiple blocks (e.g. reorgs or
                 // BIP30) is handled by the block lookup below.
-                hashBlock = block_hash;
-                return tx;
+                hashBlock = result->block_hash;
+                return result->tx;
             }
         }
     }

src/rpc/rawtransaction.cpp

@@ -152,8 +152,7 @@ PartiallySignedTransaction ProcessPSBT(const std::string& psbt_string, const std
 
         // Look in the txindex
         if (g_txindex) {
-            uint256 block_hash;
-            g_txindex->FindTx(psbt_input.prev_txid, block_hash, tx);
+            if (auto result{g_txindex->FindTx(psbt_input.prev_txid)}) tx = result->tx;
         }
         // If we still don't have it look in the mempool
         if (!tx) {

src/test/txindex_tests.cpp

@@ -19,12 +19,9 @@ BOOST_FIXTURE_TEST_CASE(txindex_initial_sync, TestChain100Setup)
     TxIndex txindex(interfaces::MakeChain(m_node), 1_MiB, true);
     BOOST_REQUIRE(txindex.Init());
 
-    CTransactionRef tx_disk;
-    uint256 block_hash;
-
     // Transaction should not be found in the index before it is started.
     for (const auto& txn : m_coinbase_txns) {
-        BOOST_CHECK(!txindex.FindTx(txn->GetHash(), block_hash, tx_disk));
+        BOOST_CHECK(!txindex.FindTx(txn->GetHash()));
     }
 
     // BlockUntilSyncedToCurrentChain should return false before txindex is started.
@@ -35,14 +32,15 @@ BOOST_FIXTURE_TEST_CASE(txindex_initial_sync, TestChain100Setup)
     // Check that txindex excludes genesis block transactions.
     const CBlock& genesis_block = Params().GenesisBlock();
     for (const auto& txn : genesis_block.vtx) {
-        BOOST_CHECK(!txindex.FindTx(txn->GetHash(), block_hash, tx_disk));
+        BOOST_CHECK(!txindex.FindTx(txn->GetHash()));
     }
 
     // Check that txindex has all txs that were in the chain before it started.
     for (const auto& txn : m_coinbase_txns) {
-        if (!txindex.FindTx(txn->GetHash(), block_hash, tx_disk)) {
+        const auto result{txindex.FindTx(txn->GetHash())};
+        if (!result) {
             BOOST_ERROR("FindTx failed");
-        } else if (tx_disk->GetHash() != txn->GetHash()) {
+        } else if (result->tx->GetHash() != txn->GetHash()) {
             BOOST_ERROR("Read incorrect tx");
         }
     }
@@ -55,9 +53,10 @@ BOOST_FIXTURE_TEST_CASE(txindex_initial_sync, TestChain100Setup)
         const CTransaction& txn = *block.vtx[0];
 
         BOOST_CHECK(txindex.BlockUntilSyncedToCurrentChain());
-        if (!txindex.FindTx(txn.GetHash(), block_hash, tx_disk)) {
+        const auto result{txindex.FindTx(txn.GetHash())};
+        if (!result) {
             BOOST_ERROR("FindTx failed");
-        } else if (tx_disk->GetHash() != txn.GetHash()) {
+        } else if (result->tx->GetHash() != txn.GetHash()) {
             BOOST_ERROR("Read incorrect tx");
         }
     }