Merge bitcoin/bitcoin#35728: rpc: Properly throw on internal I/O errors in GetTransaction

### doc/release-notes-35728.md
@@ -0,0 +1,17 @@
+Updated RPCs
+------------
+
+- The `getrawtransaction` and `gettxoutproof` RPCs now return an internal error
+  when block files or the transaction index cannot be read or are corrupt.
+  Previously, they reported the transaction as missing.
+
+- Similarly, the `utxoupdatepsbt` and `descriptorprocesspsbt` RPCs now return
+  an internal error in this case, instead of silently continuing without the
+  transaction data.
+
+Updated REST APIs
+-----------------
+
+- The `/rest/tx/<TX-HASH>` endpoint now returns an internal error when block
+  files or the transaction index cannot be read or are corrupt.
+  Previously, it reported the transaction as not found.

### src/index/txindex.cpp
@@ -20,9 +20,11 @@
 #include <serialize.h>
 #include <streams.h>
 #include <sync.h>
+#include <tinyformat.h>
 #include <uint256.h>
 #include <util/fs.h>
 #include <util/log.h>
+#include <util/string.h>
 #include <validation.h>
 
 #include <algorithm>
@@ -34,6 +36,7 @@
 #include <functional>
 #include <memory>
 #include <optional>
+#include <stdexcept>
 #include <string>
 #include <utility>
 #include <vector>
@@ -156,6 +159,14 @@ bool TxIndex::CustomAppend(const interfaces::BlockInfo& block)
 
 BaseIndex::DB& TxIndex::GetDB() const { return *m_db; }
 
+template <typename... Args>
+[[noreturn]] static void WarnAndThrowErr(util::ConstevalFormatString<sizeof...(Args)> fmt, const Args&... args)
+{
+    const std::string err{tfm::format(fmt, args...)};
+    LogWarning("%s", err);
+    throw std::runtime_error{err};
+}
+
 std::optional<TxIndexResult> TxIndex::FindTx(const Txid& tx_hash) const
 {
     struct Candidate {
@@ -175,38 +186,45 @@ std::optional<TxIndexResult> TxIndex::FindTx(const Txid& tx_hash) const
         for (it->Seek(key); it->Valid() && it->GetKey(key) && key.hash_prefix == prefix; it->Next()) {
             uint256 candidate_block_hash;
             if (!m_db->Read(txindex::BlockSeqKey{key.pos.block_seq}, candidate_block_hash)) {
-                LogWarning("Block sequence %u not found for txid %s", key.pos.block_seq, tx_hash.ToString());
-                continue;
+                // Read may throw dbwrapper_error.
+                // Read returns false for internal deserialize errors or a
+                // missing key. A missing key can only happen after corruption.
+                WarnAndThrowErr("%s: Corrupt index: block sequence %u not found for txid %s",
+                                GetName(), key.pos.block_seq, tx_hash.ToString());
             }
             LOCK(cs_main);
             const CBlockIndex* block_index{m_chainstate->m_blockman.LookupBlockIndex(candidate_block_hash)};
-            if (!block_index) {
-                LogWarning("Block index entry %s not found for txid %s", candidate_block_hash.ToString(), tx_hash.ToString());
-                continue;
+            // Stale entries can remain after an unclean shutdown. Unknown blocks
+            // cannot be in the active chain, so skip them.
+            if (!block_index) continue;
+            const bool in_active_chain{m_chainstate->m_chain.Contains(*block_index)};
+            if (!(block_index->nStatus & BLOCK_HAVE_DATA)) {
+                // A stale entry may also refer to a known block without data.
+                // Skip it if it is not in the active chain.
+                if (!in_active_chain) continue;
+                WarnAndThrowErr("%s: Block data %s unexpectedly unavailable for txid %s. Caused by data corruption?",
+                                GetName(), candidate_block_hash.ToString(), tx_hash.ToString());
             }
-            if (!(block_index->nStatus & BLOCK_HAVE_DATA)) continue;
             const FlatFilePos tx_position{block_index->nFile, block_index->nDataPos + key.pos.tx_offset_in_block};
-            candidates.emplace_back(tx_position, candidate_block_hash, key.pos.block_seq, m_chainstate->m_chain.Contains(*block_index));
+            candidates.emplace_back(tx_position, candidate_block_hash, key.pos.block_seq, in_active_chain);
         }
     }
 
     // Prefer active-chain matches, then later-connected blocks.
-    std::ranges::sort(candidates, std::greater{}, [](const Candidate& c) {
-        return std::pair{c.in_active_chain, c.block_seq};
-    });
+    std::ranges::sort(candidates, std::greater{}, [](const Candidate& c) { return std::pair{c.in_active_chain, c.block_seq}; });
 
     for (const auto& candidate : candidates) {
         AutoFile file{m_chainstate->m_blockman.OpenBlockFile(candidate.tx_position, /*fReadOnly=*/true)};
         if (file.IsNull()) {
-            LogWarning("OpenBlockFile failed for txid %s", tx_hash.ToString());
-            continue;
+            WarnAndThrowErr("%s: Failed to open block file for txid %s at %s in block %s",
+                            GetName(), tx_hash.ToString(), candidate.tx_position.ToString(), candidate.block_hash.ToString());
         }
         CTransactionRef tx;
         try {
             file >> TX_WITH_WITNESS(tx);
         } catch (const std::exception& e) {
-            LogWarning("Deserialize or I/O error - %s", e.what());
-            continue;
+            WarnAndThrowErr("%s: Deserialize or I/O error (%s). txid %s at %s in block %s",
+                            GetName(), e.what(), tx_hash.ToString(), candidate.tx_position.ToString(), candidate.block_hash.ToString());
         }
         if (tx->GetHash() == tx_hash) {
             return TxIndexResult{candidate.block_hash, std::move(tx)};
@@ -220,14 +238,19 @@ std::optional<TxIndexResult> TxIndex::FindTx(const Txid& tx_hash) const
 std::optional<TxIndexResult> TxIndex::FindLegacyTx(const Txid& tx_hash) const
 {
     CDiskTxPos postx;
-    if (!m_db->Read(txindex::LegacyTxKey(tx_hash), postx)) {
+    const auto read{m_db->TryRead(txindex::LegacyTxKey(tx_hash), postx)};
+    if (!read) {
+        WarnAndThrowErr("%s: Failed to read legacy entry for txid %s: %s",
+                        GetName(), tx_hash.ToString(), read.error().err_msg);
+    }
+    if (!read.value()) {
         return std::nullopt;
     }
 
     AutoFile file{m_chainstate->m_blockman.OpenBlockFile(postx, /*fReadOnly=*/true)};
     if (file.IsNull()) {
-        LogWarning("%s: OpenBlockFile failed", GetName());
-        return std::nullopt;
+        WarnAndThrowErr("%s: Failed to open block file for txid %s at %s",
+                        GetName(), tx_hash.ToString(), postx.ToString());
     }
     CBlockHeader header;
     CTransactionRef tx;
@@ -236,12 +259,12 @@ std::optional<TxIndexResult> TxIndex::FindLegacyTx(const Txid& tx_hash) const
         file.seek(postx.nTxOffset, SEEK_CUR);
         file >> TX_WITH_WITNESS(tx);
     } catch (const std::exception& e) {
-        LogWarning("%s: Deserialize or I/O error - %s", GetName(), e.what());
-        return std::nullopt;
+        WarnAndThrowErr("%s: Deserialize or I/O error (%s). txid %s at %s",
+                        GetName(), e.what(), tx_hash.ToString(), postx.ToString());
     }
     if (tx->GetHash() != tx_hash) {
-        LogWarning("%s: txid mismatch", GetName());
-        return std::nullopt;
+        WarnAndThrowErr("%s: txid mismatch in legacy index. txid %s at %s",
+                        GetName(), tx_hash.ToString(), postx.ToString());
     }
     return TxIndexResult{header.GetHash(), std::move(tx)};
 }

### src/node/transaction.cpp
@@ -13,10 +13,13 @@
 #include <node/context.h>
 #include <node/types.h>
 #include <policy/feerate.h>
+#include <tinyformat.h>
 #include <txmempool.h>
 #include <validation.h>
 #include <validationinterface.h>
 
+#include <stdexcept>
+
 namespace node {
 static TransactionError HandleATMPError(const TxValidationState& state, std::string& err_string_out)
 {
@@ -158,12 +161,13 @@ CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMe
     }
     if (block_index) {
         CBlock block;
-        if (blockman.ReadBlock(block, *block_index)) {
-            for (const auto& tx : block.vtx) {
-                if (tx->GetHash() == hash) {
-                    hashBlock = block_index->GetBlockHash();
-                    return tx;
-                }
+        if (!blockman.ReadBlock(block, *block_index)) {
+            throw std::runtime_error{strprintf("I/O error reading block data for block %s", block_index->GetBlockHash().ToString())};
+        }
+        for (const auto& tx : block.vtx) {
+            if (tx->GetHash() == hash) {
+                hashBlock = block_index->GetBlockHash();
+                return tx;
             }
         }
     }

### src/rpc/rawtransaction.cpp
@@ -3,8 +3,6 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
-#include <rpc/register.h> // IWYU pragma: associated
-
 #include <addresstype.h>
 #include <base58.h>
 #include <chain.h>
@@ -30,8 +28,10 @@
 #include <psbt.h>
 #include <pubkey.h>
 #include <random.h>
+#include <rpc/blockchain.h>
 #include <rpc/protocol.h>
 #include <rpc/rawtransaction_util.h>
+#include <rpc/register.h> // IWYU pragma: associated
 #include <rpc/request.h>
 #include <rpc/server.h>
 #include <rpc/server_util.h>
@@ -69,6 +69,7 @@
 #include <optional>
 #include <set>
 #include <span>
+#include <stdexcept>
 #include <tuple>
 #include <utility>
 #include <vector>
@@ -174,16 +175,8 @@ PartiallySignedTransaction ProcessPSBT(const std::string& psbt_string, const std
         // The `non_witness_utxo` is the whole previous transaction
         if (psbt_input.non_witness_utxo) continue;
 
-        CTransactionRef tx;
-
-        // Look in the txindex
-        if (g_txindex) {
-            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) {
-            tx = node.mempool->get(psbt_input.prev_txid);
-        }
+        uint256 block_hash;
+        CTransactionRef tx{GetTransaction(/*block_index=*/nullptr, node.mempool.get(), psbt_input.prev_txid, node.chainman->m_blockman, block_hash)};
         if (tx) {
             psbt_input.non_witness_utxo = tx;
         } else {
@@ -327,14 +320,16 @@ static RPCMethod getrawtransaction()
     }
 
     uint256 hash_block;
-    const CTransactionRef tx = GetTransaction(blockindex, node.mempool.get(), txid, chainman.m_blockman, hash_block);
+    CTransactionRef tx;
+    try {
+        tx = GetTransaction(blockindex, node.mempool.get(), txid, chainman.m_blockman, hash_block);
+    } catch (const std::runtime_error&) {
+        if (blockindex) WITH_LOCK(::cs_main, CheckBlockDataAvailability(chainman.m_blockman, *blockindex, /*check_for_undo=*/false));
+        throw;
+    }
     if (!tx) {
         std::string errmsg;
         if (blockindex) {
-            const bool block_has_data = WITH_LOCK(::cs_main, return blockindex->nStatus & BLOCK_HAVE_DATA);
-            if (!block_has_data) {
-                throw JSONRPCError(RPC_MISC_ERROR, "Block not available");
-            }
             errmsg = "No such transaction found in the provided block";
         } else if (!g_txindex) {
             errmsg = "No such mempool transaction. Use -txindex or provide a block hash to enable blockchain transaction queries";

### test/functional/feature_io_errors.py
@@ -0,0 +1,63 @@
+#!/usr/bin/env python3
+# Copyright (c) The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or https://opensource.org/license/mit/.
+"""Test handling of I/O errors."""
+
+from contextlib import closing
+import http.client
+import urllib.parse
+
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import (
+    assert_equal,
+    assert_raises_rpc_error,
+)
+from test_framework.wallet import MiniWallet
+
+
+class IOErrorTest(BitcoinTestFramework):
+    def set_test_params(self):
+        self.num_nodes = 1
+        self.uses_wallet = None
+
+    def test_blk_dat_io_error(self):
+        node = self.nodes[0]
+        self.restart_node(0, ["-txindex", "-rest"])
+        self.wait_until(lambda: node.getindexinfo()["txindex"]["synced"])
+        wallet = MiniWallet(node)
+
+        tx = wallet.send_self_transfer(from_node=node)
+        txid = tx["txid"]
+        tx2 = wallet.send_self_transfer(from_node=node, utxo_to_spend=tx["new_utxo"])
+        self.generate(node, 1)
+        psbt = node.createpsbt(inputs=[{"txid": txid, "vout": 0}], outputs=[])
+
+        blk_dat = node.blocks_path / "blk00000.dat"
+        blk_dat_moved = node.blocks_path / "blk00000.dat.moved"
+        blk_dat.rename(blk_dat_moved)
+
+        self.log.info("Check tx RPCs throw on I/O errors")
+        msg = "Can't read block from disk"
+        assert_raises_rpc_error(-32603, msg, node.gettxoutproof, [tx2["txid"]])
+        msg = "txindex: Failed to open block file"
+        assert_raises_rpc_error(-1, msg, node.gettxoutproof, [txid])
+        assert_raises_rpc_error(-1, msg, node.getrawtransaction, txid)
+        assert_raises_rpc_error(-1, msg, node.utxoupdatepsbt, psbt)
+
+        self.log.info("Check REST /tx/ on I/O errors")
+        url = urllib.parse.urlparse(node.url)
+        with closing(http.client.HTTPConnection(url.hostname, url.port)) as conn:
+            conn.request("GET", f"/rest/tx/{txid}.json")
+            response = conn.getresponse()
+            assert_equal(response.status, 500)
+            assert_equal(response.read().decode("utf-8")[:len(msg)], msg)
+
+        blk_dat_moved.rename(blk_dat)
+
+    def run_test(self):
+        self.test_blk_dat_io_error()
+
+
+if __name__ == "__main__":
+    IOErrorTest(__file__).main()

### test/functional/rpc_rawtransaction.py
@@ -16,6 +16,10 @@
 from decimal import Decimal
 from itertools import product
 
+from test_framework.blocktools import (
+    NORMAL_GBT_REQUEST_PARAMS,
+    create_block,
+)
 from test_framework.messages import (
     MAX_BIP125_RBF_SEQUENCE,
     COIN,
@@ -82,6 +86,7 @@ def setup_network(self):
     def run_test(self):
         self.wallet = MiniWallet(self.nodes[0])
 
+        self.test_getrawtransaction_block_data_errors()
         self.getrawtransaction_tests()
         self.createrawtransaction_tests()
         self.sendrawtransaction_tests()
@@ -90,6 +95,23 @@ def run_test(self):
         self.transaction_version_number_tests()
         self.getrawtransaction_verbosity_tests()
 
+    def test_getrawtransaction_block_data_errors(self):
+        node = self.nodes[0]
+        self.log.info("Test getrawtransaction with a header-only block")
+        block = create_block(tmpl=node.getblocktemplate(NORMAL_GBT_REQUEST_PARAMS))
+        block.solve()
+        node.submitheader(block.serialize()[:80].hex())
+
+        assert_raises_rpc_error(-1, "Block not available (not fully downloaded)", node.getrawtransaction, txid="a" * 64, blockhash=block.hash_hex)
+
+        self.log.info("Test getrawtransaction when block data cannot be read")
+        blk_dat = node.blocks_path / "blk00000.dat"
+        blk_dat_moved = node.blocks_path / "blk00000.dat.moved"
+        blk_dat.rename(blk_dat_moved)
+        assert_raises_rpc_error(-1, "I/O error reading block data", node.getrawtransaction, txid="a" * 64, blockhash=node.getblockhash(1))
+        blk_dat_moved.rename(blk_dat)
+
+
     def getrawtransaction_tests(self):
         tx = self.wallet.send_self_transfer(from_node=self.nodes[0])
         self.generate(self.nodes[0], 1)

### test/functional/test_runner.py
@@ -197,6 +197,7 @@
     'wallet_txn_clone.py',
     'wallet_txn_clone.py --segwit',
     'rpc_getchaintips.py',
+    'feature_io_errors.py',
     'rpc_misc.py',
     'p2p_1p1c_network.py',
     'interface_rest.py',