RPC: Improve error messages on RPC endpoints that use GetTransaction

src/rest.cpp

@@ -12,6 +12,7 @@
 #include <validation.h>
 #include <httpserver.h>
 #include <rpc/blockchain.h>
+#include <rpc/rawtransaction.h>
 #include <rpc/server.h>
 #include <streams.h>
 #include <sync.h>
@@ -351,14 +352,13 @@ static bool rest_tx(HTTPRequest* req, const std::string& strURIPart)
     if (!ParseHashStr(hashStr, hash))
         return RESTERR(req, HTTP_BAD_REQUEST, "Invalid hash: " + hashStr);
 
-    if (g_txindex) {
-        g_txindex->BlockUntilSyncedToCurrentChain();
-    }
-
     CTransactionRef tx;
     uint256 hashBlock = uint256();
-    if (!GetTransaction(hash, tx, Params().GetConsensus(), hashBlock, true))
-        return RESTERR(req, HTTP_NOT_FOUND, hashStr + " not found");
+    int error_code;
+    std::string errmsg;
+    if (!GetTransaction(hash, tx, hashBlock, error_code, errmsg, true)) {
+        return RESTERR(req, HTTP_NOT_FOUND, errmsg);
+    }
 
     CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION | RPCSerializationFlags());
     ssTx << tx;

src/rpc/blockchain.cpp

@@ -16,6 +16,7 @@
 #include <policy/feerate.h>
 #include <policy/policy.h>
 #include <primitives/transaction.h>
+#include <rpc/rawtransaction.h>
 #include <rpc/server.h>
 #include <streams.h>
 #include <sync.h>
@@ -1822,7 +1823,9 @@ static UniValue getblockstats(const JSONRPCRequest& request)
             for (const CTxIn& in : tx->vin) {
                 CTransactionRef tx_in;
                 uint256 hashBlock;
-                if (!GetTransaction(in.prevout.hash, tx_in, Params().GetConsensus(), hashBlock, false)) {
+                int dummy_error_code;
+                std::string dummy_errmsg;
+                if (!GetTransaction(in.prevout.hash, tx_in, hashBlock, dummy_error_code, dummy_errmsg, false)) {
                     throw JSONRPCError(RPC_INTERNAL_ERROR, std::string("Unexpected internal error (tx index seems corrupt)"));
                 }
 

src/rpc/protocol.h

@@ -57,6 +57,7 @@ enum RPCErrorCode
     RPC_VERIFY_ALREADY_IN_CHAIN     = -27, //!< Transaction already in chain
     RPC_IN_WARMUP                   = -28, //!< Client still warming up
     RPC_METHOD_DEPRECATED           = -32, //!< RPC method is deprecated
+    RPC_DATA_UNAVAILABLE            = -33, //!< Requested data is unavailable due to application configuration or state
 
     //! Aliases for backward compatibility
     RPC_TRANSACTION_ERROR           = RPC_VERIFY_ERROR,

src/rpc/rawtransaction.cpp

@@ -35,6 +35,67 @@
 
 #include <univalue.h>
 
+bool GetTransaction(const uint256& tx_hash, CTransactionRef& tx, uint256& block_hash,
+                    int& error_code, std::string& errmsg, bool allow_slow)
+{
+    // First check for an unconfirmed transaction in the mempool.
+    if (tx = mempool.get(tx_hash)) {
+        block_hash.SetNull();
+        return true;
+    }
+
+    // Search for transaction in the txindex if it is enabled and up-to-date.
+    bool f_txindex_ready = g_txindex && g_txindex->BlockUntilSyncedToCurrentChain();
+    if (f_txindex_ready && g_txindex->FindTx(tx_hash, block_hash, tx)) {
+        return true;
+    }
+
+    // If txindex is not available, try a best-effort lookup using the UTXO set cache.
+    if (!f_txindex_ready && allow_slow) {
+        const CBlockIndex* block_index = nullptr;
+        {
+            LOCK(cs_main);
+            const Coin& coin = AccessByTxid(*pcoinsTip, tx_hash);
+            if (!coin.IsSpent()) {
+                block_index = chainActive[coin.nHeight];
+            }
+        }
+
+        if (block_index && GetTransactionInBlock(tx_hash, block_index, tx)) {
+            block_hash = block_index->GetBlockHash();
+            return true;
+        }
+    }
+
+    if (!g_txindex) {
+        error_code = RPC_DATA_UNAVAILABLE;
+        errmsg = "No such mempool transaction. Use -txindex to enable blockchain transaction queries.";
+    } else if (!f_txindex_ready) {
+        error_code = RPC_DATA_UNAVAILABLE;
+        errmsg = "No such mempool transaction. Blockchain transactions are still in the process of being indexed.";
+    } else {
+        error_code = RPC_INVALID_ADDRESS_OR_KEY;
+        errmsg = "No such mempool or blockchain transaction.";
+    }
+    return false;
+}
+
+bool GetTransactionInBlock(const uint256& tx_hash, const CBlockIndex* block_index, CTransactionRef& tx)
+{
+    CBlock block;
+    if (!ReadBlockFromDisk(block, block_index, Params().GetConsensus())) {
+        return false;
+    }
+
+    for (const auto& block_tx : block.vtx) {
+        if (block_tx->GetHash() == tx_hash) {
+            tx = block_tx;
+            return true;
+        }
+    }
+
+    return false;
+}
 
 static void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue& entry)
 {
@@ -168,28 +229,23 @@ static UniValue getrawtransaction(const JSONRPCRequest& request)
         in_active_chain = chainActive.Contains(blockindex);
     }
 
-    bool f_txindex_ready = false;
-    if (g_txindex && !blockindex) {
-        f_txindex_ready = g_txindex->BlockUntilSyncedToCurrentChain();
-    }
-
     CTransactionRef tx;
     uint256 hash_block;
-    if (!GetTransaction(hash, tx, Params().GetConsensus(), hash_block, true, blockindex)) {
+    if (blockindex) {
+        if (GetTransactionInBlock(hash, blockindex, tx)) {
+            hash_block = blockindex->GetBlockHash();
+        } else if (!(blockindex->nStatus & BLOCK_HAVE_DATA)) {
+            throw JSONRPCError(RPC_DATA_UNAVAILABLE, "Block not available");
+         } else {
+            throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY,
+                               "No such transaction found in the provided block.");
+        }
+    } else {
+        int error_code;
         std::string errmsg;
-        if (blockindex) {
-            if (!(blockindex->nStatus & BLOCK_HAVE_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 to enable blockchain transaction queries";
-        } else if (!f_txindex_ready) {
-            errmsg = "No such mempool transaction. Blockchain transactions are still in the process of being indexed";
-        } else {
-            errmsg = "No such mempool or blockchain transaction";
+        if (!GetTransaction(hash, tx, hash_block, error_code, errmsg, true)) {
+            throw JSONRPCError(error_code, errmsg + " Use gettransaction for wallet transactions.");
         }
-        throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, errmsg + ". Use gettransaction for wallet transactions.");
     }
 
     if (!fVerbose) {
@@ -259,19 +315,29 @@ static UniValue gettxoutproof(const JSONRPCRequest& request)
         }
     }
 
-
     // Allow txindex to catch up if we need to query it and before we acquire cs_main.
+    bool txindex_ready = false;
     if (g_txindex && !pblockindex) {
-        g_txindex->BlockUntilSyncedToCurrentChain();
+        txindex_ready = g_txindex->BlockUntilSyncedToCurrentChain();
     }
 
     LOCK(cs_main);
 
     if (pblockindex == nullptr)
     {
+        if (!g_txindex) {
+            throw JSONRPCError(RPC_DATA_UNAVAILABLE,
+                               "Use -txindex to enable blockchain transaction queries");
+        } else if (!txindex_ready) {
+            throw JSONRPCError(RPC_DATA_UNAVAILABLE,
+                               "Blockchain transactions are still in the process of being indexed");
+        }
+
         CTransactionRef tx;
-        if (!GetTransaction(oneTxid, tx, Params().GetConsensus(), hashBlock, false) || hashBlock.IsNull())
-            throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not yet in block");
+        if (!g_txindex->FindTx(oneTxid, hashBlock, tx)) {
+            throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY,
+                               "Transaction not found in block: " + oneTxid.ToString());
+        }
         pblockindex = LookupBlockIndex(hashBlock);
         if (!pblockindex) {
             throw JSONRPCError(RPC_INTERNAL_ERROR, "Transaction index corrupt");

src/rpc/rawtransaction.h

@@ -5,11 +5,44 @@
 #ifndef BITCOIN_RPC_RAWTRANSACTION_H
 #define BITCOIN_RPC_RAWTRANSACTION_H
 
+#include <primitives/transaction.h>
+#include <uint256.h>
+
 class CBasicKeyStore;
+class CBlockIndex;
 struct CMutableTransaction;
 class UniValue;
 
 /** Sign a transaction with the given keystore and previous transactions */
 UniValue SignTransaction(CMutableTransaction& mtx, const UniValue& prevTxs, CBasicKeyStore *keystore, bool tempKeystore, const UniValue& hashType);
 
+/**
+ * Look up a transaction by hash. This checks for transactions in the mempool,
+ * the tx index if enabled, and the UTXO set cache on a best-effort basis. If
+ * the transaction is not found, this returns false.
+ *
+ * @param[in]   tx_hash  The hash of the transaction to be returned.
+ * @param[out]  tx  The transaction itself.
+ * @param[out]  block_hash  The hash of the block the transaction is found in.
+ * @param[out]  error_code  RPC code explaining why transaction could not be found.
+ * @param[out]  errmsg  Reason why transaction could not be found.
+ * @param[in]   allow_slow  An option to search a slow, unreliable source for transactions.
+ * @return  true if transaction is found, false otherwise
+ */
+bool GetTransaction(const uint256& tx_hash, CTransactionRef& tx, uint256& block_hash,
+                    int& error_code, std::string& errmsg, bool allow_slow);
+
+/**
+ * Look up a raw transaction by hash within a specified block. This loads the
+ * block itself from disk and return true if the transaction is contained within
+ * it.
+ *
+ * @param[in]   tx_hash  The hash of the transaction to be returned.
+ * @param[in]   block_index  The block index of the block to search.
+ * @param[out]  tx  The raw transaction itself.
+ * @return  true if transaction is found, false otherwise
+ */
+bool GetTransactionInBlock(const uint256& tx_hash, const CBlockIndex* block_index,
+                           CTransactionRef& tx);
+
 #endif // BITCOIN_RPC_RAWTRANSACTION_H

src/validation.cpp

@@ -16,7 +16,6 @@
 #include <consensus/validation.h>
 #include <cuckoocache.h>
 #include <hash.h>
-#include <index/txindex.h>
 #include <policy/fees.h>
 #include <policy/policy.h>
 #include <policy/rbf.h>
@@ -1017,53 +1016,6 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa
     return AcceptToMemoryPoolWithTime(chainparams, pool, state, tx, pfMissingInputs, GetTime(), plTxnReplaced, bypass_limits, nAbsurdFee, test_accept);
 }
 
-/**
- * Return transaction in txOut, and if it was found inside a block, its hash is placed in hashBlock.
- * If blockIndex is provided, the transaction is fetched from the corresponding block.
- */
-bool GetTransaction(const uint256& hash, CTransactionRef& txOut, const Consensus::Params& consensusParams, uint256& hashBlock, bool fAllowSlow, CBlockIndex* blockIndex)
-{
-    CBlockIndex* pindexSlow = blockIndex;
-
-    LOCK(cs_main);
-
-    if (!blockIndex) {
-        CTransactionRef ptx = mempool.get(hash);
-        if (ptx) {
-            txOut = ptx;
-            return true;
-        }
-
-        if (g_txindex) {
-            return g_txindex->FindTx(hash, hashBlock, txOut);
-        }
-
-        if (fAllowSlow) { // use coin database to locate block that contains transaction, and scan it
-            const Coin& coin = AccessByTxid(*pcoinsTip, hash);
-            if (!coin.IsSpent()) pindexSlow = chainActive[coin.nHeight];
-        }
-    }
-
-    if (pindexSlow) {
-        CBlock block;
-        if (ReadBlockFromDisk(block, pindexSlow, consensusParams)) {
-            for (const auto& tx : block.vtx) {
-                if (tx->GetHash() == hash) {
-                    txOut = tx;
-                    hashBlock = pindexSlow->GetBlockHash();
-                    return true;
-                }
-            }
-        }
-    }
-
-    return false;
-}
-
-
-
-
-
 
 //////////////////////////////////////////////////////////////////////////////
 //

src/validation.h

@@ -278,8 +278,6 @@ void UnloadBlockIndex();
 void ThreadScriptCheck();
 /** Check whether we are doing an initial block download (synchronizing from disk or network) */
 bool IsInitialBlockDownload();
-/** Retrieve a transaction (from memory pool, or from disk, if possible) */
-bool GetTransaction(const uint256& hash, CTransactionRef& tx, const Consensus::Params& params, uint256& hashBlock, bool fAllowSlow = false, CBlockIndex* blockIndex = nullptr);
 /**
  * Find the best known block, and make it the tip of the block chain
  *

test/functional/rpc_rawtransaction.py

@@ -41,7 +41,7 @@ class RawTransactionsTest(BitcoinTestFramework):
     def set_test_params(self):
         self.setup_clean_chain = True
         self.num_nodes = 3
-        self.extra_args = [["-addresstype=legacy"], ["-addresstype=legacy"], ["-addresstype=legacy"]]
+        self.extra_args = [["-addresstype=legacy"], ["-addresstype=legacy"], ["-addresstype=legacy", "-txindex"]]
 
     def setup_network(self, split=False):
         super().setup_network()
@@ -231,6 +231,28 @@ def run_test(self):
         self.nodes[0].reconsiderblock(block1)
         assert_equal(self.nodes[0].getbestblockhash(), block2)
 
+        # Create new transaction and don't broadcast.
+        to_address = self.nodes[0].getnewaddress()
+        prevout_index = [i for i, out in enumerate(gottx['vout']) if out['value'] == 1][0]
+        inputs = [{"txid": tx, "vout": prevout_index}]
+        outputs = {to_address: 0.9999}
+        spending_rawtx = self.nodes[1].createrawtransaction(inputs, outputs)
+        spending_tx = self.nodes[1].signrawtransactionwithwallet(spending_rawtx)['hex']
+        spending_txid = bytes_to_hex_str(hash256(hex_str_to_bytes(spending_tx)))
+
+        # Searching unknown transaction without -txindex.
+        assert_raises_rpc_error(-33, "No such mempool transaction. Use -txindex to enable blockchain transaction queries.", self.nodes[1].getrawtransaction, spending_txid)
+
+        # Searching unknown transaction with txindex.
+        assert_raises_rpc_error(-5, "No such mempool or blockchain transaction.", self.nodes[2].getrawtransaction, spending_txid)
+
+        # Broadcast transaction to mempool.
+        self.nodes[0].sendrawtransaction(spending_tx)
+        self.sync_all()
+
+        gottx = self.nodes[2].getrawtransaction(spending_txid, True)
+        assert_equal(gottx['txid'], spending_txid)
+
         #########################
         # RAW TX MULTISIG TESTS #
         #########################

test/functional/rpc_txoutproof.py

@@ -40,7 +40,7 @@ def run_test(self):
         tx2 = self.nodes[0].createrawtransaction([node0utxos.pop()], {self.nodes[1].getnewaddress(): 49.99})
         txid2 = self.nodes[0].sendrawtransaction(self.nodes[0].signrawtransactionwithwallet(tx2)["hex"])
         # This will raise an exception because the transaction is not yet in a block
-        assert_raises_rpc_error(-5, "Transaction not yet in block", self.nodes[0].gettxoutproof, [txid1])
+        assert_raises_rpc_error(-5, "Transaction not found in block", self.nodes[3].gettxoutproof, [txid1])
 
         self.nodes[0].generate(1)
         blockhash = self.nodes[0].getblockhash(chain_height + 1)
@@ -64,8 +64,8 @@ def run_test(self):
         txid_spent = txin_spent["txid"]
         txid_unspent = txid1 if txin_spent["txid"] != txid1 else txid2
 
-        # We can't find the block from a fully-spent tx
-        assert_raises_rpc_error(-5, "Transaction not yet in block", self.nodes[2].gettxoutproof, [txid_spent])
+        # We can't find the block from a fully-spent tx without txindex
+        assert_raises_rpc_error(-33, "Use -txindex to enable blockchain transaction queries", self.nodes[2].gettxoutproof, [txid_spent])
         # We can get the proof if we specify the block
         assert_equal(self.nodes[2].verifytxoutproof(self.nodes[2].gettxoutproof([txid_spent], blockhash)), [txid_spent])
         # We can't get the proof if we specify a non-existent block