From 33f85549860c518b8840052b8c58bb23c5096d54 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 20 Jun 2025 14:03:52 +0700 Subject: [PATCH 1/6] refactor: drop circular dependency over evo/mnhftx.h by removing non-used include --- src/validation.cpp | 1 - test/lint/lint-circular-dependencies.py | 1 - 2 files changed, 2 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index bc9b2acbbf10..95a8cb7cac1a 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -53,7 +53,6 @@ #include #include #include -#include #include #include #include diff --git a/test/lint/lint-circular-dependencies.py b/test/lint/lint-circular-dependencies.py index 5c96da1e7298..f7ae34637f76 100755 --- a/test/lint/lint-circular-dependencies.py +++ b/test/lint/lint-circular-dependencies.py @@ -52,7 +52,6 @@ "evo/deterministicmns -> llmq/utils -> net -> evo/deterministicmns", "evo/deterministicmns -> validationinterface -> evo/deterministicmns", "evo/deterministicmns -> validationinterface -> governance/vote -> evo/deterministicmns", - "evo/mnhftx -> validation -> evo/mnhftx", "evo/simplifiedmns -> llmq/blockprocessor -> llmq/utils -> llmq/snapshot -> evo/simplifiedmns", "evo/specialtxman -> validation -> evo/specialtxman", "governance/governance -> governance/object -> governance/governance", From 696e1643efe4d935e1620d83589b4556ddb6416b Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 20 Jun 2025 15:33:57 +0700 Subject: [PATCH 2/6] refactor: drop circular dependency over evo/chainhelper.h --- src/evo/chainhelper.cpp | 11 ----------- src/evo/chainhelper.h | 7 ------- src/evo/deterministicmns.cpp | 7 +++++-- src/governance/object.cpp | 13 ++++++++++--- src/llmq/chainlocks.cpp | 4 +++- src/llmq/instantsend.cpp | 4 +++- src/node/transaction.cpp | 7 +++++++ src/node/transaction.h | 9 +++++++++ src/validation.cpp | 3 +++ test/lint/lint-circular-dependencies.py | 5 ----- 10 files changed, 40 insertions(+), 30 deletions(-) diff --git a/src/evo/chainhelper.cpp b/src/evo/chainhelper.cpp index d946785ea1b5..c58c942b535c 100644 --- a/src/evo/chainhelper.cpp +++ b/src/evo/chainhelper.cpp @@ -6,14 +6,9 @@ #include #include -#include #include #include #include -#include -#include - -using node::GetTransaction; CChainstateHelper::CChainstateHelper(CCreditPoolManager& cpoolman, CDeterministicMNManager& dmnman, CMNHFManager& mnhfman, CGovernanceManager& govman, llmq::CInstantSendManager& isman, @@ -64,9 +59,3 @@ bool CChainstateHelper::RemoveConflictingISLockByTx(const CTransaction& tx) bool CChainstateHelper::ShouldInstantSendRejectConflicts() const { return isman.RejectConflictingBlocks(); } -std::pair 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}; -} diff --git a/src/evo/chainhelper.h b/src/evo/chainhelper.h index 374d59b4f128..6dd94bf1941f 100644 --- a/src/evo/chainhelper.h +++ b/src/evo/chainhelper.h @@ -18,7 +18,6 @@ class CGovernanceManager; class CSpecialTxProcessor; class CSporkManager; class CTransaction; -class CTxMemPool; class uint256; namespace Consensus { struct Params; } @@ -30,8 +29,6 @@ class CQuorumManager; class CQuorumSnapshotManager; } -using CTransactionRef = std::shared_ptr; - class CChainstateHelper { private: @@ -66,8 +63,4 @@ class CChainstateHelper const std::unique_ptr special_tx; }; -/* Retrieve transaction and block from txindex (or mempool) */ -std::pair GetTransactionBlock(const uint256& hash, - const CTxMemPool* const mempool = nullptr); - #endif // BITCOIN_EVO_CHAINHELPER_H diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp index 38192ca25539..36f6f42e6f4d 100644 --- a/src/evo/deterministicmns.cpp +++ b/src/evo/deterministicmns.cpp @@ -2,7 +2,6 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include #include #include #include @@ -27,6 +26,10 @@ #include #include +// Forward declaration to break dependency over node/transaction.h +class CTxMemPool; +std::pair GetTransactionBlock(const uint256& hash, const CTxMemPool* const mempool); + static const std::string DB_LIST_SNAPSHOT = "dmn_S3"; static const std::string DB_LIST_DIFF = "dmn_D3"; @@ -50,7 +53,7 @@ UniValue CDeterministicMN::ToJson() const obj.pushKV("collateralHash", collateralOutpoint.hash.ToString()); obj.pushKV("collateralIndex", (int)collateralOutpoint.n); - auto [collateralTx, _] = GetTransactionBlock(collateralOutpoint.hash); + auto [collateralTx, _] = GetTransactionBlock(collateralOutpoint.hash, nullptr); if (collateralTx) { CTxDestination dest; if (ExtractDestination(collateralTx->vout[collateralOutpoint.n].scriptPubKey, dest)) { diff --git a/src/governance/object.cpp b/src/governance/object.cpp index 53c2db8a9761..d896bd5b9d93 100644 --- a/src/governance/object.cpp +++ b/src/governance/object.cpp @@ -7,10 +7,10 @@ #include #include #include -#include #include #include #include +#include #include #include #include @@ -453,8 +453,15 @@ bool CGovernanceObject::IsCollateralValid(const ChainstateManager& chainman, std fMissingConfirmations = false; uint256 nExpectedHash = GetHash(); - // RETRIEVE TRANSACTION IN QUESTION - auto [txCollateral, nBlockHash] = GetTransactionBlock(m_obj.collateralHash); + CTransactionRef txCollateral; + uint256 nBlockHash; + if (g_txindex) { + g_txindex->FindTx(m_obj.collateralHash, nBlockHash, txCollateral); + } else { + strError = "TxIndex is not available"; + return false; + } + if (!txCollateral) { strError = strprintf("Can't find collateral tx %s", m_obj.collateralHash.ToString()); LogPrintf("CGovernanceObject::IsCollateralValid -- %s\n", strError); diff --git a/src/llmq/chainlocks.cpp b/src/llmq/chainlocks.cpp index 3364fa3b6637..3b53a7f5c740 100644 --- a/src/llmq/chainlocks.cpp +++ b/src/llmq/chainlocks.cpp @@ -10,7 +10,6 @@ #include #include #include -#include #include #include #include @@ -26,6 +25,9 @@ using node::ReadBlockFromDisk; +// Forward declaration to break dependency over node/transaction.h +std::pair GetTransactionBlock(const uint256& hash, const CTxMemPool* const mempool); + static bool ChainLocksSigningEnabled(const CSporkManager& sporkman) { return sporkman.GetSporkValue(SPORK_19_CHAINLOCKS_ENABLED) == 0; diff --git a/src/llmq/instantsend.cpp b/src/llmq/instantsend.cpp index f80221d51e2f..cf91a4815ca6 100644 --- a/src/llmq/instantsend.cpp +++ b/src/llmq/instantsend.cpp @@ -13,7 +13,6 @@ #include #include #include -#include #include #include #include @@ -31,6 +30,9 @@ using node::fImporting; using node::fReindex; +// Forward declaration to break dependency over node/transaction.h +std::pair GetTransactionBlock(const uint256& hash, const CTxMemPool* const mempool); + namespace llmq { diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp index a66c036bbfda..63ebce06cfb7 100644 --- a/src/node/transaction.cpp +++ b/src/node/transaction.cpp @@ -150,3 +150,10 @@ CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMe return nullptr; } } // namespace node + +std::pair 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}; +} diff --git a/src/node/transaction.h b/src/node/transaction.h index 3156e841e594..b3fc46accf2d 100644 --- a/src/node/transaction.h +++ b/src/node/transaction.h @@ -60,4 +60,13 @@ static const CFeeRate DEFAULT_MAX_RAW_TX_FEE_RATE{COIN / 10}; CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMemPool* const mempool, const uint256& hash, const Consensus::Params& consensusParams, uint256& hashBlock); } // namespace node +/** + * Retrieve transaction and block from txindex (or mempool) + * That's just a wrapper over GetTransaction to break circular dependencies + * and to simplify caller for case if no block_index + */ +std::pair GetTransactionBlock(const uint256& hash, + const CTxMemPool* const mempool); + + #endif // BITCOIN_NODE_TRANSACTION_H diff --git a/src/validation.cpp b/src/validation.cpp index 95a8cb7cac1a..865931613496 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -137,6 +137,9 @@ int64_t nMaxTipAge = DEFAULT_MAX_TIP_AGE; uint256 hashAssumeValid; arith_uint256 nMinimumChainWork; +// Forward declaration to break dependency over node/transaction.h +std::pair GetTransactionBlock(const uint256& hash, const CTxMemPool* const mempool); + const CBlockIndex* CChainState::FindForkInGlobalIndex(const CBlockLocator& locator) const { AssertLockHeld(cs_main); diff --git a/test/lint/lint-circular-dependencies.py b/test/lint/lint-circular-dependencies.py index f7ae34637f76..c0746f6006b5 100755 --- a/test/lint/lint-circular-dependencies.py +++ b/test/lint/lint-circular-dependencies.py @@ -38,12 +38,7 @@ "dsnotificationinterface -> llmq/chainlocks -> node/blockstorage -> dsnotificationinterface", "evo/assetlocktx -> llmq/signing -> net_processing -> txmempool -> evo/assetlocktx", "evo/cbtx -> evo/simplifiedmns -> evo/cbtx", - "evo/chainhelper -> evo/specialtxman -> evo/deterministicmns -> evo/chainhelper", "evo/chainhelper -> evo/specialtxman -> validation -> evo/chainhelper", - "evo/chainhelper -> llmq/chainlocks -> evo/chainhelper", - "evo/chainhelper -> llmq/instantsend -> evo/chainhelper", - "evo/chainhelper -> masternode/payments -> governance/classes -> governance/object -> evo/chainhelper", - "evo/chainhelper -> node/transaction -> node/context -> evo/chainhelper", "evo/deterministicmns -> llmq/commitment -> evo/deterministicmns", "evo/deterministicmns -> llmq/commitment -> validation -> evo/deterministicmns", "evo/deterministicmns -> llmq/commitment -> validation -> txmempool -> evo/deterministicmns", From 33020e97b843210292e95fee1e12094cfeb6ea8b Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Tue, 24 Jun 2025 20:43:41 +0300 Subject: [PATCH 3/6] refactor: replace GetTransactionBlock to direct usage of txindex --- src/evo/deterministicmns.cpp | 19 ++++++++++--------- src/governance/object.cpp | 3 --- test/lint/lint-circular-dependencies.py | 6 ++++-- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp index 36f6f42e6f4d..e38e699f5270 100644 --- a/src/evo/deterministicmns.cpp +++ b/src/evo/deterministicmns.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -26,10 +27,6 @@ #include #include -// Forward declaration to break dependency over node/transaction.h -class CTxMemPool; -std::pair GetTransactionBlock(const uint256& hash, const CTxMemPool* const mempool); - static const std::string DB_LIST_SNAPSHOT = "dmn_S3"; static const std::string DB_LIST_DIFF = "dmn_D3"; @@ -53,11 +50,15 @@ UniValue CDeterministicMN::ToJson() const obj.pushKV("collateralHash", collateralOutpoint.hash.ToString()); obj.pushKV("collateralIndex", (int)collateralOutpoint.n); - auto [collateralTx, _] = GetTransactionBlock(collateralOutpoint.hash, nullptr); - if (collateralTx) { - CTxDestination dest; - if (ExtractDestination(collateralTx->vout[collateralOutpoint.n].scriptPubKey, dest)) { - obj.pushKV("collateralAddress", EncodeDestination(dest)); + if (g_txindex) { + CTransactionRef collateralTx; + uint256 nBlockHash; + g_txindex->FindTx(collateralOutpoint.hash, nBlockHash, collateralTx); + if (collateralTx) { + CTxDestination dest; + if (ExtractDestination(collateralTx->vout[collateralOutpoint.n].scriptPubKey, dest)) { + obj.pushKV("collateralAddress", EncodeDestination(dest)); + } } } diff --git a/src/governance/object.cpp b/src/governance/object.cpp index d896bd5b9d93..6c57af78aa86 100644 --- a/src/governance/object.cpp +++ b/src/governance/object.cpp @@ -457,9 +457,6 @@ bool CGovernanceObject::IsCollateralValid(const ChainstateManager& chainman, std uint256 nBlockHash; if (g_txindex) { g_txindex->FindTx(m_obj.collateralHash, nBlockHash, txCollateral); - } else { - strError = "TxIndex is not available"; - return false; } if (!txCollateral) { diff --git a/test/lint/lint-circular-dependencies.py b/test/lint/lint-circular-dependencies.py index c0746f6006b5..921420032204 100755 --- a/test/lint/lint-circular-dependencies.py +++ b/test/lint/lint-circular-dependencies.py @@ -39,9 +39,11 @@ "evo/assetlocktx -> llmq/signing -> net_processing -> txmempool -> evo/assetlocktx", "evo/cbtx -> evo/simplifiedmns -> evo/cbtx", "evo/chainhelper -> evo/specialtxman -> validation -> evo/chainhelper", + "evo/deterministicmns -> index/txindex -> node/blockstorage -> evo/deterministicmns", + "evo/deterministicmns -> index/txindex -> node/blockstorage -> masternode/node -> evo/deterministicmns", + "evo/deterministicmns -> index/txindex -> validation -> evo/deterministicmns", + "evo/deterministicmns -> index/txindex -> validation -> txmempool -> evo/deterministicmns", "evo/deterministicmns -> llmq/commitment -> evo/deterministicmns", - "evo/deterministicmns -> llmq/commitment -> validation -> evo/deterministicmns", - "evo/deterministicmns -> llmq/commitment -> validation -> txmempool -> evo/deterministicmns", "evo/deterministicmns -> llmq/utils -> evo/deterministicmns", "evo/deterministicmns -> llmq/utils -> llmq/snapshot -> evo/simplifiedmns -> evo/deterministicmns", "evo/deterministicmns -> llmq/utils -> net -> evo/deterministicmns", From c5b941f26879af9fc0aea04aeb1fad84f0ba618e Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Wed, 25 Jun 2025 01:50:10 +0700 Subject: [PATCH 4/6] refactor: nuke dependency of node/blockstorage on evo/deterministicmns and dsnotificationinterface --- src/init.cpp | 26 +++++++++++++++++++++- src/node/blockstorage.cpp | 29 +------------------------ src/node/blockstorage.h | 6 +---- test/lint/lint-circular-dependencies.py | 3 --- 4 files changed, 27 insertions(+), 37 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index f942e0f93ef4..1b0a8aa8605a 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -2327,7 +2327,31 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } chainman.m_load_block = std::thread(&util::TraceThread, "loadblk", [=, &args, &chainman, &node] { - ThreadImport(chainman, *node.dmnman, *g_ds_notification_interface, vImportFiles, node.mn_activeman.get(), args); + ThreadImport(chainman, vImportFiles, args); + + // force UpdatedBlockTip to initialize nCachedBlockHeight for DS, MN payments and budgets + // but don't call it directly to prevent triggering of other listeners like zmq etc. + // GetMainSignals().UpdatedBlockTip(::ChainActive().Tip()); + g_ds_notification_interface->InitializeCurrentBlockTip(); + + { + // Get all UTXOs for each MN collateral in one go so that we can fill coin cache early + // and reduce further locking overhead for cs_main in other parts of code including GUI + LogPrintf("Filling coin cache with masternode UTXOs...\n"); + LOCK(cs_main); + const auto start{SteadyClock::now()}; + const auto mnList{node.dmnman->GetListAtChainTip()}; + mnList.ForEachMN(false, [&](auto& dmn) { + Coin coin; + GetUTXOCoin(chainman.ActiveChainstate(), dmn.collateralOutpoint, coin); + }); + LogPrintf("Filling coin cache with masternode UTXOs: done in %dms\n", Ticks(SteadyClock::now() - start)); + } + + if (node.mn_activeman != nullptr) { + node.mn_activeman->Init(chainman.ActiveTip()); + } + }); #ifdef ENABLE_WALLET if (!args.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) { diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index e97e4ac6f3c9..0a5ca022fa73 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -8,12 +8,9 @@ #include #include #include -#include -#include #include #include #include -#include #include #include #include @@ -825,8 +822,7 @@ struct CImportingNow { } }; -void ThreadImport(ChainstateManager& chainman, CDeterministicMNManager& dmnman, CDSNotificationInterface& dsnfi, - std::vector vImportFiles, CActiveMasternodeManager* const mn_activeman, const ArgsManager& args) +void ThreadImport(ChainstateManager& chainman, std::vector vImportFiles, const ArgsManager& args) { ScheduleBatchPriority(); @@ -899,29 +895,6 @@ void ThreadImport(ChainstateManager& chainman, CDeterministicMNManager& dmnman, } } // End scope of CImportingNow - // force UpdatedBlockTip to initialize nCachedBlockHeight for DS, MN payments and budgets - // but don't call it directly to prevent triggering of other listeners like zmq etc. - // GetMainSignals().UpdatedBlockTip(::ChainActive().Tip()); - dsnfi.InitializeCurrentBlockTip(); - - { - // Get all UTXOs for each MN collateral in one go so that we can fill coin cache early - // and reduce further locking overhead for cs_main in other parts of code including GUI - LogPrintf("Filling coin cache with masternode UTXOs...\n"); - LOCK(cs_main); - const auto start{SteadyClock::now()}; - auto mnList = dmnman.GetListAtChainTip(); - mnList.ForEachMN(false, [&](auto& dmn) { - Coin coin; - GetUTXOCoin(chainman.ActiveChainstate(), dmn.collateralOutpoint, coin); - }); - LogPrintf("Filling coin cache with masternode UTXOs: done in %dms\n", Ticks(SteadyClock::now() - start)); - } - - if (mn_activeman != nullptr) { - mn_activeman->Init(chainman.ActiveTip()); - } - chainman.ActiveChainstate().LoadMempool(args); } } // namespace node diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index 128fb884ac4b..b627c26162fd 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -18,7 +18,6 @@ extern RecursiveMutex cs_main; -class CActiveMasternodeManager; class ArgsManager; class BlockValidationState; class CBlock; @@ -26,8 +25,6 @@ class CBlockUndo; class CChain; class CChainParams; class CChainState; -class CDeterministicMNManager; -class CDSNotificationInterface; class ChainstateManager; struct CCheckpointData; struct FlatFilePos; @@ -229,8 +226,7 @@ bool ReadBlockFromDisk(CBlock& block, const CBlockIndex* pindex, const Consensus bool UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex* pindex); -void ThreadImport(ChainstateManager& chainman, CDeterministicMNManager& dmnman, CDSNotificationInterface& dsnfi, - std::vector vImportFiles, CActiveMasternodeManager* const mn_activeman, const ArgsManager& args); +void ThreadImport(ChainstateManager& chainman, std::vector vImportFiles, const ArgsManager& args); } // namespace node #endif // BITCOIN_NODE_BLOCKSTORAGE_H diff --git a/test/lint/lint-circular-dependencies.py b/test/lint/lint-circular-dependencies.py index 921420032204..a1d7ef640e33 100755 --- a/test/lint/lint-circular-dependencies.py +++ b/test/lint/lint-circular-dependencies.py @@ -35,12 +35,9 @@ "consensus/tx_verify -> evo/assetlocktx -> llmq/commitment -> validation -> consensus/tx_verify", "consensus/tx_verify -> evo/assetlocktx -> llmq/signing -> net_processing -> txmempool -> consensus/tx_verify", "core_io -> evo/cbtx -> evo/simplifiedmns -> core_io", - "dsnotificationinterface -> llmq/chainlocks -> node/blockstorage -> dsnotificationinterface", "evo/assetlocktx -> llmq/signing -> net_processing -> txmempool -> evo/assetlocktx", "evo/cbtx -> evo/simplifiedmns -> evo/cbtx", "evo/chainhelper -> evo/specialtxman -> validation -> evo/chainhelper", - "evo/deterministicmns -> index/txindex -> node/blockstorage -> evo/deterministicmns", - "evo/deterministicmns -> index/txindex -> node/blockstorage -> masternode/node -> evo/deterministicmns", "evo/deterministicmns -> index/txindex -> validation -> evo/deterministicmns", "evo/deterministicmns -> index/txindex -> validation -> txmempool -> evo/deterministicmns", "evo/deterministicmns -> llmq/commitment -> evo/deterministicmns", From 3a83b063be34b811223c61cc892af06ed70100ed Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 26 Jun 2025 02:06:30 +0700 Subject: [PATCH 5/6] fix: copyright for dsnotificationinterface.h --- src/dsnotificationinterface.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dsnotificationinterface.h b/src/dsnotificationinterface.h index d58d178a1b62..92ee04aac1d3 100644 --- a/src/dsnotificationinterface.h +++ b/src/dsnotificationinterface.h @@ -1,4 +1,4 @@ -// Copyright (c) 2015 The Bitcoin Core developers +// Copyright (c) 2015 The Dash Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. From 1f4a5a6e251b2332664fe01f5444bf90e95e451e Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 27 Jun 2025 22:03:43 +0700 Subject: [PATCH 6/6] refactor: replace helper GetTransactionBlock to direct usage of GetTransaction --- src/llmq/chainlocks.cpp | 11 ++++++++--- src/llmq/instantsend.cpp | 17 +++++++++++++---- src/node/transaction.cpp | 7 ------- src/node/transaction.h | 9 --------- src/validation.cpp | 9 +++++++-- 5 files changed, 28 insertions(+), 25 deletions(-) diff --git a/src/llmq/chainlocks.cpp b/src/llmq/chainlocks.cpp index 3b53a7f5c740..f25628dfcc15 100644 --- a/src/llmq/chainlocks.cpp +++ b/src/llmq/chainlocks.cpp @@ -26,7 +26,12 @@ using node::ReadBlockFromDisk; // Forward declaration to break dependency over node/transaction.h -std::pair GetTransactionBlock(const uint256& hash, const CTxMemPool* const mempool); +namespace node +{ +CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMemPool* const mempool, + const uint256& hash, const Consensus::Params& consensusParams, uint256& hashBlock); +} // namespace node +using node::GetTransaction; static bool ChainLocksSigningEnabled(const CSporkManager& sporkman) { @@ -640,8 +645,8 @@ void CChainLocksHandler::Cleanup() } } for (auto it = txFirstSeenTime.begin(); it != txFirstSeenTime.end(); ) { - auto [tx, hashBlock] = GetTransactionBlock(it->first, &mempool); - if (!tx) { + uint256 hashBlock; + if (auto tx = GetTransaction(nullptr, &mempool, it->first, Params().GetConsensus(), hashBlock); !tx) { // tx has vanished, probably due to conflicts it = txFirstSeenTime.erase(it); } else if (!hashBlock.IsNull()) { diff --git a/src/llmq/instantsend.cpp b/src/llmq/instantsend.cpp index cf91a4815ca6..ff4ed81f2d1f 100644 --- a/src/llmq/instantsend.cpp +++ b/src/llmq/instantsend.cpp @@ -31,7 +31,12 @@ using node::fImporting; using node::fReindex; // Forward declaration to break dependency over node/transaction.h -std::pair GetTransactionBlock(const uint256& hash, const CTxMemPool* const mempool); +namespace node +{ +CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMemPool* const mempool, + const uint256& hash, const Consensus::Params& consensusParams, uint256& hashBlock); +} // namespace node +using node::GetTransaction; namespace llmq { @@ -580,7 +585,8 @@ bool CInstantSendManager::CheckCanLock(const COutPoint& outpoint, bool printDebu return false; } - auto [tx, hashBlock] = GetTransactionBlock(outpoint.hash, &mempool); + uint256 hashBlock{}; + const auto tx = GetTransaction(nullptr, &mempool, outpoint.hash, params, hashBlock); // this relies on enabled txindex and won't work if we ever try to remove the requirement for txindex for masternodes if (!tx) { if (printDebug) { @@ -637,7 +643,9 @@ void CInstantSendManager::HandleNewInputLockRecoveredSig(const CRecoveredSig& re g_txindex->BlockUntilSyncedToCurrentChain(); } - auto [tx, hashBlock] = GetTransactionBlock(txid, &mempool); + + uint256 hashBlock{}; + const auto tx = GetTransaction(nullptr, &mempool, txid, Params().GetConsensus(), hashBlock); if (!tx) { return; } @@ -1019,7 +1027,8 @@ void CInstantSendManager::ProcessInstantSendLock(NodeId from, PeerManager& peerm return; } - auto [tx, hashBlock] = GetTransactionBlock(islock->txid, &mempool); + uint256 hashBlock{}; + const auto tx = GetTransaction(nullptr, &mempool, islock->txid, Params().GetConsensus(), hashBlock); const CBlockIndex* pindexMined{nullptr}; // we ignore failure here as we must be able to propagate the lock even if we don't have the TX locally if (tx && !hashBlock.IsNull()) { diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp index 63ebce06cfb7..a66c036bbfda 100644 --- a/src/node/transaction.cpp +++ b/src/node/transaction.cpp @@ -150,10 +150,3 @@ CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMe return nullptr; } } // namespace node - -std::pair 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}; -} diff --git a/src/node/transaction.h b/src/node/transaction.h index b3fc46accf2d..3156e841e594 100644 --- a/src/node/transaction.h +++ b/src/node/transaction.h @@ -60,13 +60,4 @@ static const CFeeRate DEFAULT_MAX_RAW_TX_FEE_RATE{COIN / 10}; CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMemPool* const mempool, const uint256& hash, const Consensus::Params& consensusParams, uint256& hashBlock); } // namespace node -/** - * Retrieve transaction and block from txindex (or mempool) - * That's just a wrapper over GetTransaction to break circular dependencies - * and to simplify caller for case if no block_index - */ -std::pair GetTransactionBlock(const uint256& hash, - const CTxMemPool* const mempool); - - #endif // BITCOIN_NODE_TRANSACTION_H diff --git a/src/validation.cpp b/src/validation.cpp index 865931613496..823c2ca5d354 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -138,7 +138,11 @@ uint256 hashAssumeValid; arith_uint256 nMinimumChainWork; // Forward declaration to break dependency over node/transaction.h -std::pair GetTransactionBlock(const uint256& hash, const CTxMemPool* const mempool); +namespace node +{ +CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMemPool* const mempool, const uint256& hash, const Consensus::Params& consensusParams, uint256& hashBlock); +} // namespace node +using node::GetTransaction; const CBlockIndex* CChainState::FindForkInGlobalIndex(const CBlockLocator& locator) const { @@ -754,7 +758,8 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) if (auto conflictLockOpt = m_chain_helper.ConflictingISLockIfAny(tx); conflictLockOpt.has_value()) { auto& [_, conflict_txid] = conflictLockOpt.value(); - if (auto [txConflict, _] = GetTransactionBlock(conflict_txid, &m_pool); txConflict) { + uint256 hashBlock; + if (auto txConflict = GetTransaction(nullptr, &m_pool, conflict_txid, chainparams.GetConsensus(), hashBlock); txConflict) { GetMainSignals().NotifyInstantSendDoubleSpendAttempt(ptx, txConflict); } LogPrintf("ERROR: AcceptToMemoryPool : Transaction %s conflicts with locked TX %s\n", hash.ToString(), conflict_txid.ToString());