[RPC] Update getrawtransaction interface

doc/release-notes.md

@@ -246,6 +246,12 @@ in the Low-level Changes section below.
 
 - See the [Mining](#mining) section for changes to `getblocktemplate`.
 
+- The `getrawtransaction` RPC no longer checks the unspent UTXO set for
+ a transaction. The remaining behaviors are as follows: 1. If a
+ blockhash is provided, check the corresponding block. 2. If no
+ blockhash is provided, check the mempool. 3. If no blockhash is
+ provided but txindex is enabled, also check txindex.
+
 Graphical User Interface (GUI)
 ------------------------------
 

src/rest.cpp

@@ -352,7 +352,7 @@ static bool rest_tx(HTTPRequest* req, const std::string& strURIPart)
 
  CTransactionRef tx;
  uint256 hashBlock = uint256();
- if (!GetTransaction(hash, tx, Params().GetConsensus(), hashBlock, true))
+ if (!GetTransaction(hash, tx, Params().GetConsensus(), hashBlock))
  return RESTERR(req, HTTP_NOT_FOUND, hashStr + " not found");
 
  switch (rf) {

src/rpc/blockchain.cpp

@@ -1918,7 +1918,7 @@ 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)) {
+ if (!GetTransaction(in.prevout.hash, tx_in, Params().GetConsensus(), hashBlock)) {
  throw JSONRPCError(RPC_INTERNAL_ERROR, std::string("Unexpected internal error (tx index seems corrupt)"));
  }
 

src/rpc/rawtransaction.cpp

@@ -67,12 +67,11 @@ static UniValue getrawtransaction(const JSONRPCRequest& request)
  if (request.fHelp || request.params.size() < 1 || request.params.size() > 3)
  throw std::runtime_error(
  RPCHelpMan{"getrawtransaction",
- "\nNOTE: By default this function only works for mempool transactions. If the -txindex option is\n"
- "enabled, it also works for blockchain transactions. If the block which contains the transaction\n"
- "is known, its hash can be provided even for nodes without -txindex. Note that if a blockhash is\n"
- "provided, only that block will be searched and if the transaction is in the mempool or other\n"
- "blocks, or if this node does not have the given block available, the transaction will not be found.\n"
- "DEPRECATED: for now, it also works for transactions with unspent outputs.\n"
+ "\nBy default this function only works for mempool transactions. When called with a blockhash\n"
+ "argument, getrawtransaction will return the transaction if the specified block is available and\n"
+ "the transaction is found in that block. When called without a blockhash argument, getrawtransaction\n"
+ "will return the transaction if it is in the mempool, or if -txindex is enabled and the transaction\n"
+ "is in a block in the blockchain.\n"
 
  "\nReturn the raw transaction data.\n"
  "\nIf verbose is 'true', returns an Object with information about 'txid'.\n"
@@ -175,7 +174,7 @@ static UniValue getrawtransaction(const JSONRPCRequest& request)
 
  CTransactionRef tx;
  uint256 hash_block;
- if (!GetTransaction(hash, tx, Params().GetConsensus(), hash_block, true, blockindex)) {
+ if (!GetTransaction(hash, tx, Params().GetConsensus(), hash_block, blockindex)) {
  std::string errmsg;
  if (blockindex) {
  if (!(blockindex->nStatus & BLOCK_HAVE_DATA)) {
@@ -270,7 +269,7 @@ static UniValue gettxoutproof(const JSONRPCRequest& request)
  if (pblockindex == nullptr)
  {
  CTransactionRef tx;
- if (!GetTransaction(oneTxid, tx, Params().GetConsensus(), hashBlock, false) || hashBlock.IsNull())
+ if (!GetTransaction(oneTxid, tx, Params().GetConsensus(), hashBlock) || hashBlock.IsNull())
  throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not yet in block");
  pblockindex = LookupBlockIndex(hashBlock);
  if (!pblockindex) {

src/validation.cpp

@@ -994,13 +994,11 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa
  * 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)
+bool GetTransaction(const uint256& hash, CTransactionRef& txOut, const Consensus::Params& consensusParams, uint256& hashBlock, const CBlockIndex* const block_index)
 {
- CBlockIndex* pindexSlow = blockIndex;
-
  LOCK(cs_main);
 
- if (!blockIndex) {
+ if (!block_index) {
  CTransactionRef ptx = mempool.get(hash);
  if (ptx) {
  txOut = ptx;
@@ -1010,20 +1008,13 @@ bool GetTransaction(const uint256& hash, CTransactionRef& txOut, const Consensus
  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) {
+ } else {
  CBlock block;
- if (ReadBlockFromDisk(block, pindexSlow, consensusParams)) {
+ if (ReadBlockFromDisk(block, block_index, consensusParams)) {
  for (const auto& tx : block.vtx) {
  if (tx->GetHash() == hash) {
  txOut = tx;
- hashBlock = pindexSlow->GetBlockHash();
+ hashBlock = block_index->GetBlockHash();
  return true;
  }
  }

src/validation.h

@@ -269,7 +269,7 @@ 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);
+bool GetTransaction(const uint256& hash, CTransactionRef& tx, const Consensus::Params& params, uint256& hashBlock, const CBlockIndex* const blockIndex = nullptr);
 /**
  * Find the best known block, and make it the tip of the block chain
  *

test/functional/feature_segwit.py

@@ -43,22 +43,26 @@ def set_test_params(self):
  self.setup_clean_chain = True
  self.num_nodes = 3
  # This test tests SegWit both pre and post-activation, so use the normal BIP9 activation.
+ # TODO: remove -txindex. Currently required for getrawtransaction call.
  self.extra_args = [
  [
  "-rpcserialversion=0",
  "-vbparams=segwit:0:999999999999",
  "-addresstype=legacy",
+ "-txindex"
  ],
  [
  "-blockversion=4",
  "-rpcserialversion=1",
  "-vbparams=segwit:0:999999999999",
  "-addresstype=legacy",
+ "-txindex"
  ],
  [
  "-blockversion=536870915",
  "-vbparams=segwit:0:999999999999",
  "-addresstype=legacy",
+ "-txindex"
  ],
  ]
 

test/functional/interface_rest.py

@@ -41,7 +41,8 @@ class RESTTest (BitcoinTestFramework):
  def set_test_params(self):
  self.setup_clean_chain = True
  self.num_nodes = 2
- self.extra_args = [["-rest"], []]
+ # TODO: remove -txindex. Currently required for getrawtransaction call.
+ self.extra_args = [["-rest", "-txindex"], []]
 
  def skip_test_if_missing_module(self):
  self.skip_if_no_wallet()

test/functional/rpc_psbt.py

@@ -19,6 +19,8 @@ class PSBTTest(BitcoinTestFramework):
  def set_test_params(self):
  self.setup_clean_chain = False
  self.num_nodes = 3
+ # TODO: remove -txindex. Currently required for getrawtransaction call.
+ self.extra_args = [[], ["-txindex"], ["-txindex"]]
 
  def skip_test_if_missing_module(self):
  self.skip_if_no_wallet()

test/functional/rpc_rawtransaction.py

@@ -42,7 +42,8 @@ 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"]]
+ # TODO: remove -txindex. Currently required for getrawtransaction call.
+ self.extra_args = [["-addresstype=legacy", "-txindex"], ["-addresstype=legacy", "-txindex"], ["-addresstype=legacy", "-txindex"]]
 
  def skip_test_if_missing_module(self):
  self.skip_if_no_wallet()

test/functional/wallet_abandonconflict.py

@@ -18,7 +18,8 @@
 class AbandonConflictTest(BitcoinTestFramework):
  def set_test_params(self):
  self.num_nodes = 2
- self.extra_args = [["-minrelaytxfee=0.00001"], []]
+ # TODO: remove -txindex. Currently required for getrawtransaction call.
+ self.extra_args = [["-minrelaytxfee=0.00001", "-txindex"], []]
 
  def skip_test_if_missing_module(self):
  self.skip_if_no_wallet()

test/functional/wallet_basic.py

@@ -23,6 +23,8 @@ class WalletTest(BitcoinTestFramework):
  def set_test_params(self):
  self.num_nodes = 4
  self.setup_clean_chain = True
+ # TODO: remove -txindex. Currently required for getrawtransaction call.
+ self.extra_args = [[], [], ["-txindex"], []]
 
  def skip_test_if_missing_module(self):
  self.skip_if_no_wallet()