2e9cf9d txindex: return pruned block hashes from FindTx:
I find result.GetFound() and result.GetMiss() easier to read than repeating std::get_if at each call site. Deriving from the variant, like GenTxid does, keeps the found/miss distinction and the existing std::get calls.
<details>
<summary>Give the lookup result named accessors</summary>
diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp
index 60338e9af0..48fe0016b7 100644
--- a/src/node/transaction.cpp
+++ b/src/node/transaction.cpp
@@ -19,7 +19,6 @@
#include <validationinterface.h>
#include <utility>
-#include <variant>
namespace node {
static TransactionError HandleATMPError(const TxValidationState& state, std::string& err_string_out)
@@ -146,21 +145,15 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t
TxLookupResult GetTransaction(const CBlockIndex* const block_index, const CTxMemPool* const mempool, const Txid& hash, const BlockManager& blockman)
{
if (mempool && !block_index) {
- CTransactionRef ptx = mempool->get(hash);
- if (ptx) return TxFound{std::move(ptx), {}};
+ if (auto ptx{mempool->get(hash)}) return TxFound{std::move(ptx), {}};
}
if (g_txindex) {
- TxLookupResult result{g_txindex->FindTx(hash)};
- if (const auto* found{std::get_if<TxFound>(&result)}) {
- if (!block_index || block_index->GetBlockHash() == found->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.
- return result;
- }
- } else if (!block_index) {
- return result;
- }
+ auto result{g_txindex->FindTx(hash)};
+ if (!block_index) return result;
+ // 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.
+ if (const auto* found{result.GetFound()}; found && block_index->GetBlockHash() == found->block_hash) return result;
}
if (block_index) {
CBlock block;
diff --git a/src/node/tx_lookup_result.h b/src/node/tx_lookup_result.h
index 9c1808d42c..e1c8e56b12 100644
--- a/src/node/tx_lookup_result.h
+++ b/src/node/tx_lookup_result.h
@@ -26,6 +26,14 @@ struct TxMiss {
};
/// Result of a transaction lookup that may be answered by the txindex.
-using TxLookupResult = std::variant<TxFound, TxMiss>;
+struct TxLookupResult : public std::variant<TxFound, TxMiss>
+{
+ using variant::variant;
+
+ TxFound* GetFound() { return std::get_if<TxFound>(this); }
+ const TxFound* GetFound() const { return std::get_if<TxFound>(this); }
+ TxMiss* GetMiss() { return std::get_if<TxMiss>(this); }
+ const TxMiss* GetMiss() const { return std::get_if<TxMiss>(this); }
+};
#endif // BITCOIN_NODE_TX_LOOKUP_RESULT_H
diff --git a/src/rest.cpp b/src/rest.cpp
index b583bb1abe..004e31778a 100644
--- a/src/rest.cpp
+++ b/src/rest.cpp
@@ -903,7 +903,7 @@ static bool rest_tx(const std::any& context, HTTPRequest* req, const std::string
const NodeContext* const node = GetNodeContext(context, req);
if (!node) return false;
const TxLookupResult result{GetTransaction(/*block_index=*/nullptr, node->mempool.get(), *hash, node->chainman->m_blockman)};
- if (const auto* miss{std::get_if<TxMiss>(&result)}) {
+ if (const auto* miss{result.GetMiss()}) {
if (!miss->pruned_block_hashes.empty()) {
return RESTERR(req, HTTP_NOT_FOUND, PrunedBlocksErrorMessage(miss->pruned_block_hashes));
}
diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp
index 3714ee26cc..77b75cb18b 100644
--- a/src/rpc/rawtransaction.cpp
+++ b/src/rpc/rawtransaction.cpp
@@ -178,8 +178,7 @@ PartiallySignedTransaction ProcessPSBT(const std::string& psbt_string, const std
// The `non_witness_utxo` is the whole previous transaction
if (psbt_input.non_witness_utxo) continue;
- TxLookupResult result{GetTransaction(/*block_index=*/nullptr, node.mempool.get(), psbt_input.prev_txid, chainman.m_blockman)};
- if (auto* found{std::get_if<TxFound>(&result)}) {
+ if (auto result{GetTransaction(/*block_index=*/nullptr, node.mempool.get(), psbt_input.prev_txid, chainman.m_blockman)}; auto* found{result.GetFound()}) {
psbt_input.non_witness_utxo = std::move(found->tx);
} else {
coins[psbt_input.GetOutPoint()]; // Create empty map entry keyed by prevout
@@ -326,7 +325,7 @@ static RPCMethod getrawtransaction()
}
const TxLookupResult tx_result{GetTransaction(blockindex, node.mempool.get(), txid, chainman.m_blockman)};
- if (const auto* miss{std::get_if<TxMiss>(&tx_result)}) {
+ if (const auto* miss{tx_result.GetMiss()}) {
if (!miss->pruned_block_hashes.empty()) {
throw JSONRPCPrunedBlocksError(miss->pruned_block_hashes);
}
diff --git a/src/rpc/txoutproof.cpp b/src/rpc/txoutproof.cpp
index 57f1cb2bb3..e433578075 100644
--- a/src/rpc/txoutproof.cpp
+++ b/src/rpc/txoutproof.cpp
@@ -32,7 +32,6 @@
#include <span>
#include <string>
#include <utility>
-#include <variant>
#include <vector>
using node::GetTransaction;
@@ -108,9 +107,9 @@ static RPCMethod gettxoutproof()
if (pblockindex == nullptr) {
const TxLookupResult result{GetTransaction(/*block_index=*/nullptr, /*mempool=*/nullptr, *setTxids.begin(), chainman.m_blockman)};
- const auto* found{std::get_if<TxFound>(&result)};
+ const auto* found{result.GetFound()};
if (!found || found->block_hash.IsNull()) {
- if (const auto* miss{std::get_if<TxMiss>(&result)}; miss && !miss->pruned_block_hashes.empty()) {
+ if (const auto* miss{result.GetMiss()}; miss && !miss->pruned_block_hashes.empty()) {
throw JSONRPCPrunedBlocksError(miss->pruned_block_hashes);
}
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not yet in block");
diff --git a/src/test/txindex_tests.cpp b/src/test/txindex_tests.cpp
index d57a7074a4..e1a3666da1 100644
--- a/src/test/txindex_tests.cpp
+++ b/src/test/txindex_tests.cpp
@@ -35,7 +35,6 @@
#include <string>
#include <string_view>
#include <utility>
-#include <variant>
#include <vector>
#include <boost/test/unit_test.hpp>
@@ -88,16 +87,15 @@ FlatFilePos BlockFilePos(const ChainstateManager& chainman, uint32_t height)
uint256 LookupTx(const TxIndex& txindex, const Txid& txid)
{
const auto result{txindex.FindTx(txid)};
- const auto* found{std::get_if<TxFound>(&result)};
- BOOST_CHECK(Assert(found)->tx->GetHash() == txid);
- return found->block_hash;
+ const auto& [tx, block_hash]{*Assert(result.GetFound())};
+ BOOST_CHECK(tx->GetHash() == txid);
+ return block_hash;
}
void CheckNotFound(const TxIndex& txindex, const Txid& txid, const std::set<uint256>& expected_pruned_block_hashes = {})
{
const auto result{txindex.FindTx(txid)};
- const auto* miss{std::get_if<TxMiss>(&result)};
- BOOST_CHECK(Assert(miss)->pruned_block_hashes == expected_pruned_block_hashes);
+ BOOST_CHECK(Assert(result.GetMiss())->pruned_block_hashes == expected_pruned_block_hashes);
}
void InvalidateBlock(ChainstateManager& chainman, const uint256& block_hash)
</details>
