diff --git a/doc/release-notes.md b/doc/release-notes.md --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -3,3 +3,13 @@ This release includes the following features and fixes: + +Updated RPCs +------------ + +- 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. + diff --git a/src/rest.cpp b/src/rest.cpp --- a/src/rest.cpp +++ b/src/rest.cpp @@ -394,7 +394,7 @@ CTransactionRef tx; BlockHash hashBlock; if (!GetTransaction(txid, tx, config.GetChainParams().GetConsensus(), - hashBlock, true)) { + hashBlock)) { return RESTERR(req, HTTP_NOT_FOUND, hashStr + " not found"); } diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -2365,7 +2365,7 @@ CTransactionRef tx_in; BlockHash hashBlock; if (!GetTransaction(in.prevout.GetTxId(), tx_in, params, - hashBlock, false)) { + hashBlock)) { throw JSONRPCError(RPC_INTERNAL_ERROR, std::string("Unexpected internal error " "(tx index seems corrupt)")); diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -73,18 +73,15 @@ 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 " @@ -223,7 +220,7 @@ CTransactionRef tx; BlockHash hash_block; - if (!GetTransaction(txid, tx, params.GetConsensus(), hash_block, true, + if (!GetTransaction(txid, tx, params.GetConsensus(), hash_block, blockindex)) { std::string errmsg; if (blockindex) { @@ -349,7 +346,7 @@ if (pblockindex == nullptr) { CTransactionRef tx; - if (!GetTransaction(oneTxId, tx, params, hashBlock, false) || + if (!GetTransaction(oneTxId, tx, params, hashBlock) || hashBlock.IsNull()) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not yet in block"); diff --git a/src/validation.h b/src/validation.h --- a/src/validation.h +++ b/src/validation.h @@ -381,7 +381,6 @@ */ bool GetTransaction(const TxId &txid, CTransactionRef &txOut, const Consensus::Params ¶ms, BlockHash &hashBlock, - bool fAllowSlow = false, const CBlockIndex *const blockIndex = nullptr); /** diff --git a/src/validation.cpp b/src/validation.cpp --- a/src/validation.cpp +++ b/src/validation.cpp @@ -703,12 +703,10 @@ */ bool GetTransaction(const TxId &txid, CTransactionRef &txOut, const Consensus::Params ¶ms, BlockHash &hashBlock, - bool fAllowSlow, const CBlockIndex *const blockIndex) { - CBlockIndex const *pindexSlow = blockIndex; - + const CBlockIndex *const block_index) { LOCK(cs_main); - if (!blockIndex) { + if (block_index == nullptr) { CTransactionRef ptx = g_mempool.get(txid); if (ptx) { txOut = ptx; @@ -718,24 +716,13 @@ if (g_txindex) { return g_txindex->FindTx(txid, hashBlock, txOut); } - - // use coin database to locate block that contains transaction, and scan - // it - if (fAllowSlow) { - const Coin &coin = AccessByTxid(*pcoinsTip, txid); - if (!coin.IsSpent()) { - pindexSlow = ::ChainActive()[coin.GetHeight()]; - } - } - } - - if (pindexSlow) { + } else { CBlock block; - if (ReadBlockFromDisk(block, pindexSlow, params)) { + if (ReadBlockFromDisk(block, block_index, params)) { for (const auto &tx : block.vtx) { if (tx->GetId() == txid) { txOut = tx; - hashBlock = pindexSlow->GetBlockHash(); + hashBlock = block_index->GetBlockHash(); return true; } } diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py --- a/test/functional/interface_rest.py +++ b/test/functional/interface_rest.py @@ -44,7 +44,8 @@ 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() diff --git a/test/functional/mining_prioritisetransaction.py b/test/functional/mining_prioritisetransaction.py --- a/test/functional/mining_prioritisetransaction.py +++ b/test/functional/mining_prioritisetransaction.py @@ -21,9 +21,12 @@ def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 2 + # TODO: remove -txindex. Currently required for getrawtransaction call + # (called by calculate_fee_from_txid) self.extra_args = [[ "-printpriority=1", "-acceptnonstdtxn=1", + "-txindex" ]] * self.num_nodes def skip_test_if_missing_module(self): diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -51,6 +51,8 @@ def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 + # TODO: remove -txindex. Currently required for getrawtransaction call. + self.extra_args = [["-txindex"]] def run_test(self): self.mine_chain() diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -24,6 +24,8 @@ 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"], ["-txindex"]] def skip_test_if_missing_module(self): self.skip_if_no_wallet() diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py --- a/test/functional/rpc_rawtransaction.py +++ b/test/functional/rpc_rawtransaction.py @@ -56,6 +56,8 @@ def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 3 + # TODO: remove -txindex. Currently required for getrawtransaction call. + self.extra_args = [["-txindex"], ["-txindex"], ["-txindex"]] def skip_test_if_missing_module(self): self.skip_if_no_wallet() diff --git a/test/functional/wallet_abandonconflict.py b/test/functional/wallet_abandonconflict.py --- a/test/functional/wallet_abandonconflict.py +++ b/test/functional/wallet_abandonconflict.py @@ -27,7 +27,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() diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -23,10 +23,12 @@ class WalletTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 4 - self.extra_args = [ - ["-acceptnonstdtxn=1"], - ] * self.num_nodes self.setup_clean_chain = True + # TODO: remove -txindex. Currently required for getrawtransaction call. + self.extra_args = [["-acceptnonstdtxn=1"], + ["-acceptnonstdtxn=1"], + ["-acceptnonstdtxn=1", "-txindex"], + ["-acceptnonstdtxn=1"]] def skip_test_if_missing_module(self): self.skip_if_no_wallet()