diff --git a/doc/REST-interface.md b/doc/REST-interface.md --- a/doc/REST-interface.md +++ b/doc/REST-interface.md @@ -13,7 +13,8 @@ Given a transaction hash: returns a transaction in binary, hex-encoded binary, or JSON formats. -For full TX query capability, one must enable the transaction index via "txindex=1" command line / configuration option. +By default, this endpoint will only search the mempool. +To query for a confirmed transaction, enable the transaction index via "txindex=1" command line / configuration option. #### Blocks `GET /rest/block/.` 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 " @@ -109,7 +106,8 @@ RPCResult{ "if verbose is set to true", "{\n" - " \"in_active_chain\": b, (bool) Whether specified block " + " \"in_active_chain\": b, (bool) Whether specified " + "block " "is in " "the active chain or not (only present with explicit " "\"blockhash\" " @@ -117,10 +115,12 @@ " \"hex\" : \"data\", (string) The serialized, " "hex-encoded " "data for 'txid'\n" - " \"txid\" : \"id\", (string) The transaction id " + " \"txid\" : \"id\", (string) The transaction " + "id " "(same as " "provided)\n" - " \"hash\" : \"id\", (string) The transaction hash " + " \"hash\" : \"id\", (string) The transaction " + "hash " "(differs from txid for witness transactions)\n" " \"size\" : n, (numeric) The serialized " "transaction " @@ -129,35 +129,42 @@ " \"locktime\" : ttt, (numeric) The lock time\n" " \"vin\" : [ (array of json objects)\n" " {\n" - " \"txid\": \"id\", (string) The transaction id\n" + " \"txid\": \"id\", (string) The transaction " + "id\n" " \"vout\": n, (numeric) \n" " \"scriptSig\": { (json object) The script\n" " \"asm\": \"asm\", (string) asm\n" " \"hex\": \"hex\" (string) hex\n" " },\n" - " \"sequence\": n (numeric) The script sequence " + " \"sequence\": n (numeric) The script " + "sequence " "number\n" " }\n" " ,...\n" " ],\n" " \"vout\" : [ (array of json objects)\n" " {\n" - " \"value\" : x.xxx, (numeric) The value " + " \"value\" : x.xxx, (numeric) The " + "value " "in " + CURRENCY_UNIT + "\n" - " \"n\" : n, (numeric) index\n" - " \"scriptPubKey\" : { (json object)\n" + " \"n\" : n, (numeric) " + "index\n" + " \"scriptPubKey\" : { (json " + "object)\n" " \"asm\" : \"asm\", (string) the " "asm\n" " \"hex\" : \"hex\", (string) the " "hex\n" - " \"reqSigs\" : n, (numeric) The " + " \"reqSigs\" : n, (numeric) " + "The " "required sigs\n" " \"type\" : \"pubkeyhash\", (string) The " "type, eg " "'pubkeyhash'\n" - " \"addresses\" : [ (json array of " + " \"addresses\" : [ (json array " + "of " "string)\n" " \"address\" (string) bitcoin " "address\n" @@ -230,7 +237,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) { @@ -356,7 +363,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() @@ -92,15 +93,18 @@ txid = self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), 0.1) self.sync_all() - self.nodes[1].generatetoaddress(1, not_related_address) - self.sync_all() - bb_hash = self.nodes[0].getbestblockhash() - assert_equal(self.nodes[1].getbalance(), Decimal("0.1")) - - self.log.info("Load the transaction using the /tx URI") + self.log.info("Test the /tx URI") json_obj = self.test_rest_request("/tx/{}".format(txid)) + assert_equal(json_obj['txid'], txid) + + # Check hex format response + hex_response = self.test_rest_request( + "/tx/{}".format(txid), req_type=ReqType.HEX, ret_type=RetType.OBJ) + assert_greater_than_or_equal(int(hex_response.getheader('content-length')), + json_obj['size'] * 2) + # Get the vin to later check for utxo (should be spent by then) spent = (json_obj['vin'][0]['txid'], json_obj['vin'][0]['vout']) # Get n of 0.1 outpoint @@ -109,9 +113,14 @@ self.log.info("Query an unspent TXO using the /getutxos URI") - json_obj = self.test_rest_request("/getutxos/{}-{}".format(*spending)) + self.nodes[1].generatetoaddress(1, not_related_address) + self.sync_all() + bb_hash = self.nodes[0].getbestblockhash() + + assert_equal(self.nodes[1].getbalance(), Decimal("0.1")) # Check chainTip response + json_obj = self.test_rest_request("/getutxos/{}-{}".format(*spending)) assert_equal(json_obj['chaintipHash'], bb_hash) # Make sure there is one utxo @@ -276,18 +285,6 @@ # Now we should have 5 header objects assert_equal(len(json_obj), 5) - self.log.info("Test the /tx URI") - - tx_hash = block_json_obj['tx'][0]['txid'] - json_obj = self.test_rest_request("/tx/{}".format(tx_hash)) - assert_equal(json_obj['txid'], tx_hash) - - # Check hex format response - hex_response = self.test_rest_request( - "/tx/{}".format(tx_hash), req_type=ReqType.HEX, ret_type=RetType.OBJ) - assert_greater_than_or_equal( - int(hex_response.getheader('content-length')), json_obj['size'] * 2) - self.log.info("Test tx inclusion in the /mempool and /block URIs") # Make 3 tx and mine them on node 1 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 @@ -24,6 +24,7 @@ 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,7 @@ def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 + 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"]] def skip_test_if_missing_module(self): self.skip_if_no_wallet() @@ -123,10 +125,10 @@ node2_addr = self.nodes[2].getnewaddress() txid1 = self.nodes[0].sendtoaddress(node1_addr, 13) txid2 = self.nodes[0].sendtoaddress(node2_addr, 13) - self.nodes[0].generate(6) + blockhash = self.nodes[0].generate(6)[0] self.sync_all() - vout1 = find_output(self.nodes[1], txid1, 13) - vout2 = find_output(self.nodes[2], txid2, 13) + vout1 = find_output(self.nodes[1], txid1, 13, blockhash=blockhash) + vout2 = find_output(self.nodes[2], txid2, 13, blockhash=blockhash) # Create a psbt spending outputs from nodes 1 and 2 psbt_orig = self.nodes[0].createpsbt([{"txid": txid1, "vout": vout1}, { @@ -281,9 +283,9 @@ # Newly created PSBT needs UTXOs and updating addr = self.nodes[1].getnewaddress("") txid = self.nodes[0].sendtoaddress(addr, 7) - self.nodes[0].generate(6) + blockhash = self.nodes[0].generate(6)[0] self.sync_all() - vout = find_output(self.nodes[0], txid, 7) + vout = find_output(self.nodes[0], txid, 7, blockhash=blockhash) psbt = self.nodes[1].createpsbt([{"txid": txid, "vout": vout}], { self.nodes[0].getnewaddress(""): Decimal('6.999')}) analyzed = self.nodes[0].analyzepsbt(psbt) 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/test_framework/util.py b/test/functional/test_framework/util.py --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -470,12 +470,12 @@ ############################# -def find_output(node, txid, amount): +def find_output(node, txid, amount, *, blockhash=None): """ Return index to output of txid with value amount Raises exception if there is none. """ - txdata = node.getrawtransaction(txid, 1) + txdata = node.getrawtransaction(txid, 1, blockhash) for i in range(len(txdata["vout"])): if txdata["vout"][i]["value"] == amount: return i 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,10 @@ 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"], + ["-txindex", "-acceptnonstdtxn=1"], ["-acceptnonstdtxn=1"]] def skip_test_if_missing_module(self): self.skip_if_no_wallet()