diff --git a/src/index/txindex.cpp b/src/index/txindex.cpp index 5e71e6bb77a..4e4094e73e4 100644 --- a/src/index/txindex.cpp +++ b/src/index/txindex.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -90,31 +91,31 @@ bool TxIndex::CustomAppend(const interfaces::BlockInfo& block) BaseIndex::DB& TxIndex::GetDB() const { return *m_db; } -bool TxIndex::FindTx(const Txid& tx_hash, uint256& block_hash, CTransactionRef& tx) const +std::optional TxIndex::FindTx(const Txid& tx_hash) const { CDiskTxPos postx; if (!m_db->ReadTxPos(tx_hash, postx)) { - return false; + return std::nullopt; } - AutoFile file{m_chainstate->m_blockman.OpenBlockFile(postx, true)}; + AutoFile file{m_chainstate->m_blockman.OpenBlockFile(postx, /*fReadOnly=*/true)}; if (file.IsNull()) { LogError("OpenBlockFile failed"); - return false; + return std::nullopt; } CBlockHeader header; + CTransactionRef tx; try { file >> header; file.seek(postx.nTxOffset, SEEK_CUR); file >> TX_WITH_WITNESS(tx); } catch (const std::exception& e) { LogError("Deserialize or I/O error - %s", e.what()); - return false; + return std::nullopt; } if (tx->GetHash() != tx_hash) { LogError("txid mismatch"); - return false; + return std::nullopt; } - block_hash = header.GetHash(); - return true; + return TxIndexResult{header.GetHash(), std::move(tx)}; } diff --git a/src/index/txindex.h b/src/index/txindex.h index 0358e0ae49f..b4648817443 100644 --- a/src/index/txindex.h +++ b/src/index/txindex.h @@ -7,17 +7,24 @@ #include #include +#include #include #include +#include -class uint256; namespace interfaces { class Chain; } static constexpr bool DEFAULT_TXINDEX{false}; +/// A found transaction and the hash of the block that contains it. +struct TxIndexResult { + uint256 block_hash; + CTransactionRef tx; +}; + /** * TxIndex is used to look up transactions included in the blockchain by hash. * The index is written to a LevelDB database and records the filesystem @@ -48,10 +55,8 @@ public: /// Look up a transaction by hash. /// /// @param[in] tx_hash The hash of the transaction to be returned. - /// @param[out] block_hash The hash of the block the transaction is found in. - /// @param[out] tx The transaction itself. - /// @return true if transaction is found, false otherwise - bool FindTx(const Txid& tx_hash, uint256& block_hash, CTransactionRef& tx) const; + /// @return the transaction and containing block hash, or nullopt if it is not found + std::optional FindTx(const Txid& tx_hash) const; }; /// The global transaction index, used in GetTransaction. May be null. diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp index d331ae05aa6..0df9718bbb7 100644 --- a/src/node/transaction.cpp +++ b/src/node/transaction.cpp @@ -146,15 +146,13 @@ CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMe 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) { + if (auto result{g_txindex->FindTx(hash)}) { + if (!block_index || block_index->GetBlockHash() == result->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; + hashBlock = result->block_hash; + return result->tx; } } } diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 054721b0124..686761f26ae 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -152,8 +152,7 @@ PartiallySignedTransaction ProcessPSBT(const std::string& psbt_string, const std // Look in the txindex if (g_txindex) { - uint256 block_hash; - g_txindex->FindTx(psbt_input.prev_txid, block_hash, tx); + if (auto result{g_txindex->FindTx(psbt_input.prev_txid)}) tx = result->tx; } // If we still don't have it look in the mempool if (!tx) { diff --git a/src/test/txindex_tests.cpp b/src/test/txindex_tests.cpp index 46711e080c6..0dd21fae507 100644 --- a/src/test/txindex_tests.cpp +++ b/src/test/txindex_tests.cpp @@ -19,12 +19,9 @@ BOOST_FIXTURE_TEST_CASE(txindex_initial_sync, TestChain100Setup) TxIndex txindex(interfaces::MakeChain(m_node), 1_MiB, true); BOOST_REQUIRE(txindex.Init()); - CTransactionRef tx_disk; - uint256 block_hash; - // Transaction should not be found in the index before it is started. for (const auto& txn : m_coinbase_txns) { - BOOST_CHECK(!txindex.FindTx(txn->GetHash(), block_hash, tx_disk)); + BOOST_CHECK(!txindex.FindTx(txn->GetHash())); } // BlockUntilSyncedToCurrentChain should return false before txindex is started. @@ -35,14 +32,15 @@ BOOST_FIXTURE_TEST_CASE(txindex_initial_sync, TestChain100Setup) // Check that txindex excludes genesis block transactions. const CBlock& genesis_block = Params().GenesisBlock(); for (const auto& txn : genesis_block.vtx) { - BOOST_CHECK(!txindex.FindTx(txn->GetHash(), block_hash, tx_disk)); + BOOST_CHECK(!txindex.FindTx(txn->GetHash())); } // Check that txindex has all txs that were in the chain before it started. for (const auto& txn : m_coinbase_txns) { - if (!txindex.FindTx(txn->GetHash(), block_hash, tx_disk)) { + const auto result{txindex.FindTx(txn->GetHash())}; + if (!result) { BOOST_ERROR("FindTx failed"); - } else if (tx_disk->GetHash() != txn->GetHash()) { + } else if (result->tx->GetHash() != txn->GetHash()) { BOOST_ERROR("Read incorrect tx"); } } @@ -55,9 +53,10 @@ BOOST_FIXTURE_TEST_CASE(txindex_initial_sync, TestChain100Setup) const CTransaction& txn = *block.vtx[0]; BOOST_CHECK(txindex.BlockUntilSyncedToCurrentChain()); - if (!txindex.FindTx(txn.GetHash(), block_hash, tx_disk)) { + const auto result{txindex.FindTx(txn.GetHash())}; + if (!result) { BOOST_ERROR("FindTx failed"); - } else if (tx_disk->GetHash() != txn.GetHash()) { + } else if (result->tx->GetHash() != txn.GetHash()) { BOOST_ERROR("Read incorrect tx"); } }