From 04da9f4834e1651da65ceb6379950cef9450591c Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Sat, 26 Jan 2019 14:34:00 -0800 Subject: [PATCH] [RPC] Update getrawtransaction interface --- doc/release-notes.md | 6 ++++++ src/rest.cpp | 2 +- src/rpc/blockchain.cpp | 2 +- src/rpc/rawtransaction.cpp | 15 +++++++-------- src/validation.cpp | 19 +++++-------------- src/validation.h | 2 +- test/functional/feature_segwit.py | 4 ++++ test/functional/interface_rest.py | 3 ++- test/functional/rpc_psbt.py | 2 ++ test/functional/rpc_rawtransaction.py | 3 ++- test/functional/wallet_abandonconflict.py | 3 ++- test/functional/wallet_basic.py | 2 ++ 12 files changed, 35 insertions(+), 28 deletions(-) diff --git a/doc/release-notes.md b/doc/release-notes.md index 53b5a2119f3..5a5a566715e 100644 --- a/doc/release-notes.md +++ b/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) ------------------------------ diff --git a/src/rest.cpp b/src/rest.cpp index 4f26e3afb5c..44f04ee2bc3 100644 --- a/src/rest.cpp +++ b/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) { diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 315f69d46be..94be223858f 100644 --- a/src/rpc/blockchain.cpp +++ b/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)")); } diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index bf00870107c..6ccead2fdd2 100644 --- a/src/rpc/rawtransaction.cpp +++ b/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) { diff --git a/src/validation.cpp b/src/validation.cpp index a18e449af6b..71dda006218 100644 --- a/src/validation.cpp +++ b/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; } } diff --git a/src/validation.h b/src/validation.h index b5548a92934..1d00124ab78 100644 --- a/src/validation.h +++ b/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 * diff --git a/test/functional/feature_segwit.py b/test/functional/feature_segwit.py index 4bcdf9af55e..1efc50e71fa 100755 --- a/test/functional/feature_segwit.py +++ b/test/functional/feature_segwit.py @@ -43,22 +43,26 @@ class SegWitTest(BitcoinTestFramework): 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" ], ] diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py index afa9de580f0..72455a0788c 100755 --- a/test/functional/interface_rest.py +++ b/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() diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 272ebe65cbc..1e10280e603 100755 --- a/test/functional/rpc_psbt.py +++ b/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() diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py index 5b9dbef68d7..a97d7536266 100755 --- a/test/functional/rpc_rawtransaction.py +++ b/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() diff --git a/test/functional/wallet_abandonconflict.py b/test/functional/wallet_abandonconflict.py index e5ac2c8bd45..2b684349ced 100755 --- a/test/functional/wallet_abandonconflict.py +++ b/test/functional/wallet_abandonconflict.py @@ -18,7 +18,8 @@ from test_framework.util import assert_equal, assert_raises_rpc_error, connect_n 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 index 7184bb8cb68..daae2ed3c02 100755 --- a/test/functional/wallet_basic.py +++ b/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()