From abc57e1f0882a1a2bb20474648419979af6e383d Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Thu, 22 Jul 2021 15:01:14 +0200 Subject: [PATCH 1/2] refactor: move `GetTransaction(...)` to node/transaction.cpp can be reviewed with --color-moved --- src/node/transaction.cpp | 35 +++++++++++++++++++++++++ src/node/transaction.h | 20 ++++++++++++++ src/validation.cpp | 33 ----------------------- src/validation.h | 15 +---------- test/lint/lint-circular-dependencies.sh | 1 - 5 files changed, 56 insertions(+), 48 deletions(-) diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp index f21b3909150..0227618edff 100644 --- a/src/node/transaction.cpp +++ b/src/node/transaction.cpp @@ -4,9 +4,12 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include +#include #include +#include #include #include #include @@ -104,3 +107,35 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t return TransactionError::OK; } + +CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMemPool* const mempool, const uint256& hash, const Consensus::Params& consensusParams, uint256& hashBlock) +{ + LOCK(cs_main); + + if (mempool && !block_index) { + CTransactionRef ptx = mempool->get(hash); + if (ptx) return ptx; + } + if (g_txindex) { + CTransactionRef tx; + uint256 block_hash; + if (g_txindex->FindTx(hash, block_hash, tx)) { + if (!block_index || block_index->GetBlockHash() == block_hash) { + hashBlock = block_hash; + return tx; + } + } + } + if (block_index) { + CBlock block; + if (ReadBlockFromDisk(block, block_index, consensusParams)) { + for (const auto& tx : block.vtx) { + if (tx->GetHash() == hash) { + hashBlock = block_index->GetBlockHash(); + return tx; + } + } + } + } + return nullptr; +} diff --git a/src/node/transaction.h b/src/node/transaction.h index 0c016ff04e1..aed519cf7fa 100644 --- a/src/node/transaction.h +++ b/src/node/transaction.h @@ -10,7 +10,12 @@ #include #include +class CBlockIndex; +class CTxMemPool; struct NodeContext; +namespace Consensus { +struct Params; +} /** Maximum fee rate for sendrawtransaction and testmempoolaccept RPC calls. * Also used by the GUI when broadcasting a completed PSBT. @@ -38,4 +43,19 @@ static const CFeeRate DEFAULT_MAX_RAW_TX_FEE_RATE{COIN / 10}; */ [[nodiscard]] TransactionError BroadcastTransaction(NodeContext& node, CTransactionRef tx, std::string& err_string, const CAmount& max_tx_fee, bool relay, bool wait_callback); +/** + * Return transaction with a given hash. + * If mempool is provided and block_index is not provided, check it first for the tx. + * If -txindex is available, check it next for the tx. + * Finally, if block_index is provided, check for tx by reading entire block from disk. + * + * @param[in] block_index The block to read from disk, or nullptr + * @param[in] mempool If provided, check mempool for tx + * @param[in] hash The txid + * @param[in] consensusParams The params + * @param[out] hashBlock The block hash, if the tx was found via -txindex or block_index + * @returns The tx if found, otherwise nullptr + */ +CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMemPool* const mempool, const uint256& hash, const Consensus::Params& consensusParams, uint256& hashBlock); + #endif // BITCOIN_NODE_TRANSACTION_H diff --git a/src/validation.cpp b/src/validation.cpp index 6a145a088ac..3e5cc1e77cb 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -18,7 +18,6 @@ #include #include #include -#include #include #include #include @@ -1155,38 +1154,6 @@ PackageMempoolAcceptResult ProcessNewPackage(CChainState& active_chainstate, CTx return result; } -CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMemPool* const mempool, const uint256& hash, const Consensus::Params& consensusParams, uint256& hashBlock) -{ - LOCK(cs_main); - - if (mempool && !block_index) { - CTransactionRef ptx = mempool->get(hash); - if (ptx) return ptx; - } - if (g_txindex) { - CTransactionRef tx; - uint256 block_hash; - if (g_txindex->FindTx(hash, block_hash, tx)) { - if (!block_index || block_index->GetBlockHash() == block_hash) { - hashBlock = block_hash; - return tx; - } - } - } - if (block_index) { - CBlock block; - if (ReadBlockFromDisk(block, block_index, consensusParams)) { - for (const auto& tx : block.vtx) { - if (tx->GetHash() == hash) { - hashBlock = block_index->GetBlockHash(); - return tx; - } - } - } - } - return nullptr; -} - CAmount GetBlockSubsidy(int nHeight, const Consensus::Params& consensusParams) { int halvings = nHeight / consensusParams.nSubsidyHalvingInterval; diff --git a/src/validation.h b/src/validation.h index 0cebf3967d5..573246ddb6b 100644 --- a/src/validation.h +++ b/src/validation.h @@ -141,20 +141,7 @@ void UnloadBlockIndex(CTxMemPool* mempool, ChainstateManager& chainman); void StartScriptCheckWorkerThreads(int threads_num); /** Stop all of the script checking worker threads */ void StopScriptCheckWorkerThreads(); -/** - * Return transaction with a given hash. - * If mempool is provided and block_index is not provided, check it first for the tx. - * If -txindex is available, check it next for the tx. - * Finally, if block_index is provided, check for tx by reading entire block from disk. - * - * @param[in] block_index The block to read from disk, or nullptr - * @param[in] mempool If provided, check mempool for tx - * @param[in] hash The txid - * @param[in] consensusParams The params - * @param[out] hashBlock The block hash, if the tx was found via -txindex or block_index - * @returns The tx if found, otherwise nullptr - */ -CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMemPool* const mempool, const uint256& hash, const Consensus::Params& consensusParams, uint256& hashBlock); + CAmount GetBlockSubsidy(int nHeight, const Consensus::Params& consensusParams); bool AbortNode(BlockValidationState& state, const std::string& strMessage, const bilingual_str& userMessage = bilingual_str{}); diff --git a/test/lint/lint-circular-dependencies.sh b/test/lint/lint-circular-dependencies.sh index f8f24bb1ffe..df5051720b3 100755 --- a/test/lint/lint-circular-dependencies.sh +++ b/test/lint/lint-circular-dependencies.sh @@ -10,7 +10,6 @@ export LC_ALL=C EXPECTED_CIRCULAR_DEPENDENCIES=( "chainparamsbase -> util/system -> chainparamsbase" - "index/txindex -> validation -> index/txindex" "node/blockstorage -> validation -> node/blockstorage" "index/blockfilterindex -> node/blockstorage -> validation -> index/blockfilterindex" "index/base -> validation -> index/blockfilterindex -> index/base" From f685a13bef0418663015ea6d8f448f075510c0ec Mon Sep 17 00:00:00 2001 From: John Newbery Date: Thu, 22 Jul 2021 20:32:04 +0200 Subject: [PATCH 2/2] doc: GetTransaction()/getrawtransaction follow-ups to #22383 --- src/node/transaction.cpp | 3 +++ src/rpc/rawtransaction.cpp | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp index 0227618edff..4de0b302e64 100644 --- a/src/node/transaction.cpp +++ b/src/node/transaction.cpp @@ -121,6 +121,9 @@ CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMe uint256 block_hash; if (g_txindex->FindTx(hash, block_hash, tx)) { if (!block_index || block_index->GetBlockHash() == block_hash) { + // Don't return the transaction if the provided block hash doesn't match. + // The case where a transaction appears in multiple blocks (e.g. reorgs or + // BIP30) is handled by the block lookup below. hashBlock = block_hash; return tx; } diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 30addf8af6d..ed5e1d8c460 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -76,8 +76,8 @@ static RPCHelpMan getrawtransaction() "\nBy default, this call only returns a transaction if it is in the mempool. If -txindex is enabled\n" "and no blockhash argument is passed, it will return the transaction if it is in the mempool or any block.\n" - "If -txindex is not enabled and a blockhash argument is passed, it will return the transaction if\n" - "the specified block is available and the transaction is found in that block.\n" + "If a blockhash argument is passed, it will return the transaction if\n" + "the specified block is available and the transaction is in that block.\n" "\nHint: Use gettransaction for wallet transactions.\n" "\nIf verbose is 'true', returns an Object with information about 'txid'.\n"