refactor: use GetTransactionBlock() as narrow GetTransaction() proxy

Currently, we rely on redefinition to avoid an explosion in reported
circular dependencies but that is verboten with `-Wredundant-decls`. To
trim down the amount of circular dependencies that would happen once
we remove the circular definition, we have to proxy `GetTransaction()`
through another function located elsewhere.

Since all Dash-specific invocations either rely on the txindex or the
mempool, we can safely trim down the number of arguments and add a
fast-fail. Since we do (sometimes) care about the block the transaction
is in, we return that as well.

Author: kwvg

src/coinjoin/client.cpp

@@ -4,6 +4,7 @@
 
 #include <coinjoin/client.h>
 
+#include <chain.h>
 #include <chainparams.h>
 #include <coinjoin/options.h>
 #include <consensus/validation.h>
@@ -21,7 +22,6 @@
 #include <util/ranges.h>
 #include <util/system.h>
 #include <util/translation.h>
-#include <validation.h>
 #include <version.h>
 #include <wallet/coincontrol.h>
 #include <wallet/fees.h>

src/coinjoin/util.cpp

@@ -7,7 +7,6 @@
 #include <policy/fees.h>
 #include <policy/policy.h>
 #include <script/sign.h>
-#include <validation.h>
 #include <wallet/fees.h>
 #include <wallet/wallet.h>
 #include <util/translation.h>

src/evo/assetlocktx.cpp

@@ -14,9 +14,9 @@
 #include <consensus/validation.h>
 #include <deploymentstatus.h>
 #include <logging.h>
+#include <node/blockstorage.h>
 #include <tinyformat.h>
 #include <util/ranges_set.h>
-#include <validation.h>
 
 #include <algorithm>
 

src/evo/cbtx.cpp

@@ -18,7 +18,6 @@
 #include <chainparams.h>
 #include <consensus/merkle.h>
 #include <deploymentstatus.h>
-#include <validation.h>
 
 
 bool CheckCbTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidationState& state)

src/evo/chainhelper.cpp

@@ -6,9 +6,12 @@
 
 #include <consensus/params.h>
 #include <evo/specialtxman.h>
+#include <index/txindex.h>
 #include <llmq/chainlocks.h>
 #include <llmq/instantsend.h>
 #include <masternode/payments.h>
+#include <node/transaction.h>
+#include <txmempool.h>
 
 CChainstateHelper::CChainstateHelper(CCreditPoolManager& cpoolman, CDeterministicMNManager& dmnman,
                                      CMNHFManager& mnhfman, CGovernanceManager& govman, llmq::CInstantSendManager& isman,
@@ -58,3 +61,10 @@ bool CChainstateHelper::RemoveConflictingISLockByTx(const CTransaction& tx)
 }
 
 bool CChainstateHelper::ShouldInstantSendRejectConflicts() const { return isman.RejectConflictingBlocks(); }
+
+std::pair<CTransactionRef, uint256> GetTransactionBlock(const uint256& hash, const CTxMemPool* const mempool)
+{
+    uint256 hashBlock{};
+    if (!g_txindex && !mempool) return {nullptr, hashBlock}; // Fast-fail as we don't have any other way to search
+    return {GetTransaction(/*block_index=*/nullptr, mempool, hash, Params().GetConsensus(), hashBlock), hashBlock};
+}

src/evo/chainhelper.h

@@ -18,6 +18,7 @@ class CGovernanceManager;
 class CSpecialTxProcessor;
 class CSporkManager;
 class CTransaction;
+class CTxMemPool;
 class uint256;
 
 namespace Consensus { struct Params; }
@@ -29,6 +30,8 @@ class CQuorumManager;
 class CQuorumSnapshotManager;
 }
 
+using CTransactionRef = std::shared_ptr<const CTransaction>;
+
 class CChainstateHelper
 {
 private:
@@ -63,4 +66,8 @@ class CChainstateHelper
     const std::unique_ptr<CSpecialTxProcessor> special_tx;
 };
 
+/* Retrieve transaction and block from txindex (or mempool) */
+std::pair</*tx=*/CTransactionRef, /*hash_block=*/uint256> GetTransactionBlock(const uint256& hash,
+                                                                              const CTxMemPool* const mempool = nullptr);
+
 #endif // BITCOIN_EVO_CHAINHELPER_H

src/evo/deterministicmns.cpp

@@ -2,6 +2,7 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
+#include <evo/chainhelper.h>
 #include <evo/deterministicmns.h>
 #include <evo/dmn_types.h>
 #include <evo/dmnstate.h>
@@ -13,15 +14,15 @@
 
 #include <base58.h>
 #include <chainparams.h>
+#include <coins.h>
 #include <consensus/validation.h>
 #include <deploymentstatus.h>
-#include <script/standard.h>
-#include <validation.h>
-#include <validationinterface.h>
-#include <univalue.h>
 #include <messagesigner.h>
-#include <uint256.h>
+#include <script/standard.h>
 #include <stats/client.h>
+#include <uint256.h>
+#include <univalue.h>
+#include <validationinterface.h>
 
 #include <optional>
 #include <memory>
@@ -51,8 +52,7 @@ UniValue CDeterministicMN::ToJson() const
     obj.pushKV("collateralHash", collateralOutpoint.hash.ToString());
     obj.pushKV("collateralIndex", (int)collateralOutpoint.n);
 
-    uint256 tmpHashBlock;
-    CTransactionRef collateralTx = GetTransaction(/* block_index */ nullptr,  /* mempool */ nullptr, collateralOutpoint.hash, Params().GetConsensus(), tmpHashBlock);
+    auto [collateralTx, _] = GetTransactionBlock(collateralOutpoint.hash);
     if (collateralTx) {
         CTxDestination dest;
         if (ExtractDestination(collateralTx->vout[collateralOutpoint.n].scriptPubKey, dest)) {

src/governance/object.cpp

@@ -7,6 +7,7 @@
 #include <bls/bls.h>
 #include <chainparams.h>
 #include <core_io.h>
+#include <evo/chainhelper.h>
 #include <evo/deterministicmns.h>
 #include <governance/governance.h>
 #include <governance/validators.h>
@@ -467,8 +468,7 @@ bool CGovernanceObject::IsCollateralValid(const ChainstateManager& chainman, std
     uint256 nExpectedHash = GetHash();
 
     // RETRIEVE TRANSACTION IN QUESTION
-    uint256 nBlockHash;
-    CTransactionRef txCollateral = GetTransaction(/* block_index */ nullptr, /* mempool */ nullptr, m_obj.collateralHash, Params().GetConsensus(), nBlockHash);
+    auto [txCollateral, nBlockHash] = GetTransactionBlock(m_obj.collateralHash);
     if (!txCollateral) {
         strError = strprintf("Can't find collateral tx %s", m_obj.collateralHash.ToString());
         LogPrintf("CGovernanceObject::IsCollateralValid -- %s\n", strError);

src/governance/vote.cpp

@@ -15,7 +15,6 @@
 #include <timedata.h>
 #include <util/string.h>
 #include <util/system.h>
-#include <validation.h>
 
 #include <evo/deterministicmns.h>
 

src/llmq/chainlocks.cpp

@@ -10,6 +10,7 @@
 #include <chain.h>
 #include <chainparams.h>
 #include <consensus/validation.h>
+#include <evo/chainhelper.h>
 #include <masternode/sync.h>
 #include <node/blockstorage.h>
 #include <node/interface_ui.h>
@@ -635,8 +636,7 @@ void CChainLocksHandler::Cleanup()
         }
     }
     for (auto it = txFirstSeenTime.begin(); it != txFirstSeenTime.end(); ) {
-        uint256 hashBlock;
-        CTransactionRef tx = GetTransaction(/* block_index */ nullptr, &mempool, it->first, Params().GetConsensus(), hashBlock);
+        auto [tx, hashBlock] = GetTransactionBlock(it->first, &mempool);
         if (!tx) {
             // tx has vanished, probably due to conflicts
             it = txFirstSeenTime.erase(it);