Merge #6450: feat: show human friendly error if missing spentindex, txindex or addressindex

a275bda266 refactor: removed duplicated code with errors messages for txindex, timestampindex, spentindex (Konstantin Akimov)
dd1b36636c feat: show human friendly error if missing spentindex, txindex or addressindex (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented

  Currently user receive error `Unable to get spent info` or even worse `No information available for address` which doesn't say anything about required extra indexes.

  Also, every call of RPC `getrawtransaction` causes this error logs on high level if spent index is disabled, but actually it just means that no couple extra fields are hidden in output which is expected if no index.

      node0 2024-12-03T18:54:33.349605Z [      http] [httpserver.cpp:248] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:40052
      node0 2024-12-03T18:54:33.349634Z [httpworker.3] [rpc/request.cpp:180] [parse] [rpc] ThreadRPCServer method=getrawtransaction user=__cookie__
      node0 2024-12-03T18:54:33.349729Z [httpworker.3] [util/system.h:57] [error] ERROR: Spent index not enabled
      node0 2024-12-03T18:54:33.349735Z [httpworker.3] [util/system.h:57] [error] ERROR: Spent index not enabled
      node0 2024-12-03T18:54:33.349738Z [httpworker.3] [util/system.h:57] [error] ERROR: Spent index not enabled
      node0 2024-12-03T18:54:33.349808Z [httpworker.3] [httprpc.cpp:93] [~RpcHttpRequest] [bench] HTTP RPC request handled: user=__cookie__ command=getrawtransaction external=false status=200 elapsed_time_ms=0
      node0 2024-12-03T18:54:33.349998Z [      http] [httpserver.cpp:248] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:40052
      node0 2024-12-03T18:54:33.350027Z [httpworker.0] [rpc/request.cpp:180] [parse] [rpc] ThreadRPCServer method=getrawtransaction user=__cookie__
      node0 2024-12-03T18:54:33.350128Z [httpworker.0] [util/system.h:57] [error] ERROR: Spent index not enabled
      node0 2024-12-03T18:54:33.350133Z [httpworker.0] [util/system.h:57] [error] ERROR: Spent index not enabled
      node0 2024-12-03T18:54:33.350137Z [httpworker.0] [util/system.h:57] [error] ERROR: Spent index not enabled

  ## What was done?
  Improved all usages of extra indexes `spentindex`, `txindex` and `addressindex` in RPC implementation.

  Affected RPCc:
   - getaddressmempool
   - getaddressutxos
   - getaddressdeltas
   - getaddressbalance
   - getaddresstxids
   - getspentinfo
   - getblockhashes

  ## How Has This Been Tested?
  Run unit&functional tests.
  ```
  $ dash-cli getaddressutxos '["yW4kiSd2pytXC2erbjm6crt1PGBvbwS4YX"]'
  Address index is disabled. You should run Dash Core with -addressindex (requires reindex) (code -32600)
  ```

  Check logs after calling getrawtransaction:
  ```
      node0 2024-12-03T19:10:00.378770Z [httpworker.3] [rpc/request.cpp:180] [parse] [rpc] ThreadRPCServer method=getrawtransaction user=__cookie__
      node0 2024-12-03T19:10:00.378861Z [httpworker.3] [httprpc.cpp:93] [~RpcHttpRequest] [bench] HTTP RPC request handled: user=__cookie__ command=getrawtransaction external=false status=500 elapsed_time_ms=0
      node0 2024-12-03T19:10:00.379017Z [      http] [httpserver.cpp:248] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:44984
      node0 2024-12-03T19:10:00.379036Z [httpworker.0] [rpc/request.cpp:180] [parse] [rpc] ThreadRPCServer method=getrawtransaction user=__cookie__
      node0 2024-12-03T19:10:00.379090Z [httpworker.0] [httprpc.cpp:93] [~RpcHttpRequest] [bench] HTTP RPC request handled: user=__cookie__ command=getrawtransaction external=false status=500 elapsed_time_ms=0
      node0 2024-12-03T19:10:00.379240Z [      http] [httpserver.cpp:248] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:44984
  ```

  ## Breaking Changes
  Error type and message changed. It's no more `RPC_INVALID_ADDRESS_OR_KEY` but `RPC_INVALID_REQUEST`.

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  kwvg:
    utACK a275bda266
  UdjinM6:
    utACK a275bda266

Tree-SHA512: d53373aba794035173375811e333e940efb1081bed46e18846b2b54d60036ee52487c65f9b84ac687b2be2a30f3a68fb75afbb2c89e52b0774740892863a04df

src/rpc/index_util.cpp

@@ -6,21 +6,29 @@
 #include <rpc/index_util.h>
 
 #include <node/blockstorage.h>
+#include <rpc/protocol.h>
+#include <rpc/request.h>
 #include <txdb.h>
 #include <txmempool.h>
 #include <uint256.h>
 
+static void EnsureAddressIndexAvailable()
+{
+    if (!fAddressIndex) {
+        throw JSONRPCError(RPC_INVALID_REQUEST, "Address index is disabled. You should run Dash Core with -addressindex (requires reindex)");
+    }
+}
+
 bool GetAddressIndex(CBlockTreeDB& block_tree_db, const uint160& addressHash, const AddressType type,
                      std::vector<CAddressIndexEntry>& addressIndex,
                      const int32_t start, const int32_t end)
 {
     AssertLockHeld(::cs_main);
+    EnsureAddressIndexAvailable();
 
-    if (!fAddressIndex)
+    if (!block_tree_db.ReadAddressIndex(addressHash, type, addressIndex, start, end)) {
-        return error("Address index not enabled");
-
-    if (!block_tree_db.ReadAddressIndex(addressHash, type, addressIndex, start, end))
         return error("Unable to get txids for address");
+    }
 
     return true;
 }
@@ -29,9 +37,7 @@ bool GetAddressUnspentIndex(CBlockTreeDB& block_tree_db, const uint160& addressH
                             std::vector<CAddressUnspentIndexEntry>& unspentOutputs, const bool height_sort)
 {
     AssertLockHeld(::cs_main);
-
+    EnsureAddressIndexAvailable();
-    if (!fAddressIndex)
-        return error("Address index not enabled");
 
     if (!block_tree_db.ReadAddressUnspentIndex(addressHash, type, unspentOutputs))
         return error("Unable to get txids for address");
@@ -51,8 +57,7 @@ bool GetMempoolAddressDeltaIndex(const CTxMemPool& mempool,
                                  std::vector<CMempoolAddressDeltaEntry>& addressDeltaEntries,
                                  const bool timestamp_sort)
 {
-    if (!fAddressIndex)
+    EnsureAddressIndexAvailable();
-        return error("Address index not enabled");
 
     if (!mempool.getAddressIndex(addressDeltaIndex, addressDeltaEntries))
         return error("Unable to get address delta information");
@@ -72,8 +77,9 @@ bool GetSpentIndex(CBlockTreeDB& block_tree_db, const CTxMemPool& mempool, const
 {
     AssertLockHeld(::cs_main);
 
-    if (!fSpentIndex)
+    if (!fSpentIndex) {
-        return error("Spent index not enabled");
+        throw JSONRPCError(RPC_INVALID_REQUEST, "Spent index is disabled. You should run Dash Core with -spentindex (requires reindex)");
+    }
 
     if (mempool.getSpentIndex(key, value))
         return true;
@@ -89,8 +95,9 @@ bool GetTimestampIndex(CBlockTreeDB& block_tree_db, const uint32_t high, const u
 {
     AssertLockHeld(::cs_main);
 
-    if (!fTimestampIndex)
+    if (!fTimestampIndex) {
-        return error("Timestamp index not enabled");
+        throw JSONRPCError(RPC_INVALID_REQUEST, "Timestamp index is disabled. You should run Dash Core with -timestampindex (requires reindex)");
+    }
 
     if (!block_tree_db.ReadTimestampIndex(high, low, hashes))
         return error("Unable to get hashes for timestamps");

src/rpc/index_util.h

@@ -24,20 +24,27 @@ enum class AddressType : uint8_t;
 
 extern RecursiveMutex cs_main;
 
+//! throws JSONRPCError if address index is unavailable
 bool GetAddressIndex(CBlockTreeDB& block_tree_db, const uint160& addressHash, const AddressType type,
                      std::vector<CAddressIndexEntry>& addressIndex,
                      const int32_t start = 0, const int32_t end = 0)
     EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+//! throws JSONRPCError if address index is unavailable
 bool GetAddressUnspentIndex(CBlockTreeDB& block_tree_db, const uint160& addressHash, const AddressType type,
                             std::vector<CAddressUnspentIndexEntry>& unspentOutputs, const bool height_sort = false)
     EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+//! throws JSONRPCError if address index is unavailable
 bool GetMempoolAddressDeltaIndex(const CTxMemPool& mempool,
                                  const std::vector<CMempoolAddressDeltaKey>& addressDeltaIndex,
                                  std::vector<CMempoolAddressDeltaEntry>& addressDeltaEntries,
                                  const bool timestamp_sort = false);
+
+//! throws JSONRPCError if spent index is unavailable
 bool GetSpentIndex(CBlockTreeDB& block_tree_db, const CTxMemPool& mempool, const CSpentIndexKey& key,
                    CSpentIndexValue& value)
     EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+
+//! throws JSONRPCError if timestamp index is unavailable
 bool GetTimestampIndex(CBlockTreeDB& block_tree_db, const uint32_t high, const uint32_t low,
                        std::vector<uint256>& hashes)
     EXCLUSIVE_LOCKS_REQUIRED(::cs_main);

src/rpc/misc.cpp

@@ -810,9 +810,7 @@ static RPCHelpMan getaddressutxos()
         },
     [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-
     std::vector<std::pair<uint160, AddressType> > addresses;
-
     if (!getAddressesFromParams(request.params, addresses)) {
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid address");
     }
@@ -883,8 +881,6 @@ static RPCHelpMan getaddressdeltas()
         },
     [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-
-
     UniValue startValue = find_value(request.params[0].get_obj(), "start");
     UniValue endValue = find_value(request.params[0].get_obj(), "end");
 
@@ -975,7 +971,6 @@ static RPCHelpMan getaddressbalance()
         },
     [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-
     std::vector<std::pair<uint160, AddressType> > addresses;
 
     if (!getAddressesFromParams(request.params, addresses)) {
@@ -1048,7 +1043,6 @@ static RPCHelpMan getaddresstxids()
         },
     [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-
     std::vector<std::pair<uint160, AddressType> > addresses;
 
     if (!getAddressesFromParams(request.params, addresses)) {
@@ -1138,7 +1132,6 @@ static RPCHelpMan getspentinfo()
         },
     [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-
     UniValue txidValue = find_value(request.params[0].get_obj(), "txid");
     UniValue indexValue = find_value(request.params[0].get_obj(), "index");
 

src/rpc/rawtransaction.cpp

@@ -71,27 +71,32 @@ void TxToJSON(const CTransaction& tx, const uint256 hashBlock, CTxMemPool& mempo
     // data into the returned UniValue.
 
     uint256 txid = tx.GetHash();
+    CSpentIndexTxInfo *txSpentInfoPtr{nullptr};
 
     // Add spent information if spentindex is enabled
     CSpentIndexTxInfo txSpentInfo;
-    for (const auto& txin : tx.vin) {
+    if (fSpentIndex) {
-        if (!tx.IsCoinBase()) {
+        txSpentInfo = CSpentIndexTxInfo{};
+        for (const auto& txin : tx.vin) {
+            if (!tx.IsCoinBase()) {
+                CSpentIndexValue spentInfo;
+                CSpentIndexKey spentKey(txin.prevout.hash, txin.prevout.n);
+                if (GetSpentIndex(*active_chainstate.m_blockman.m_block_tree_db, mempool, spentKey, spentInfo)) {
+                    txSpentInfo.mSpentInfo.emplace(spentKey, spentInfo);
+                }
+            }
+        }
+        for (unsigned int i = 0; i < tx.vout.size(); i++) {
             CSpentIndexValue spentInfo;
-            CSpentIndexKey spentKey(txin.prevout.hash, txin.prevout.n);
+            CSpentIndexKey spentKey(txid, i);
             if (GetSpentIndex(*active_chainstate.m_blockman.m_block_tree_db, mempool, spentKey, spentInfo)) {
                 txSpentInfo.mSpentInfo.emplace(spentKey, spentInfo);
             }
         }
-    }
+        txSpentInfoPtr = &txSpentInfo;
-    for (unsigned int i = 0; i < tx.vout.size(); i++) {
-        CSpentIndexValue spentInfo;
-        CSpentIndexKey spentKey(txid, i);
-        if (GetSpentIndex(*active_chainstate.m_blockman.m_block_tree_db, mempool, spentKey, spentInfo)) {
-            txSpentInfo.mSpentInfo.emplace(spentKey, spentInfo);
-        }
     }
 
-    TxToUniv(tx, uint256(), entry, true, /* txundo = */ nullptr, &txSpentInfo);
+    TxToUniv(tx, uint256(), entry, true, /* txundo = */ nullptr, txSpentInfoPtr);
 
     bool chainLock = false;
     if (!hashBlock.IsNull()) {