From e74152d5b48bdca8dfc2217f35842dbb24b2746c Mon Sep 17 00:00:00 2001 From: dergoegge Date: Wed, 19 Jul 2023 15:56:37 +0200 Subject: [PATCH] Use type-safe txid types in orphanage --- src/net_processing.cpp | 5 +++-- src/test/orphanage_tests.cpp | 5 +++-- src/txorphanage.cpp | 33 +++++++++++++++++---------------- src/txorphanage.h | 11 ++++++----- 4 files changed, 29 insertions(+), 25 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 89236b54f199a..f53235cadab99 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -41,6 +41,7 @@ #include // For NDEBUG compile time check #include #include +#include #include #include @@ -2918,8 +2919,8 @@ bool PeerManagerImpl::ProcessOrphanTx(Peer& peer) while (CTransactionRef porphanTx = m_orphanage.GetTxToReconsider(peer.m_id)) { const MempoolAcceptResult result = m_chainman.ProcessTransaction(porphanTx); const TxValidationState& state = result.m_state; - const uint256& orphanHash = porphanTx->GetHash(); - const uint256& orphan_wtxid = porphanTx->GetWitnessHash(); + const Txid& orphanHash = porphanTx->GetHash(); + const Wtxid& orphan_wtxid = porphanTx->GetWitnessHash(); if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) { LogPrint(BCLog::TXPACKAGES, " accepted orphan tx %s (wtxid=%s)\n", orphanHash.ToString(), orphan_wtxid.ToString()); diff --git a/src/test/orphanage_tests.cpp b/src/test/orphanage_tests.cpp index af53737fecb1d..fe64d48396810 100644 --- a/src/test/orphanage_tests.cpp +++ b/src/test/orphanage_tests.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -29,8 +30,8 @@ class TxOrphanageTest : public TxOrphanage CTransactionRef RandomOrphan() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { LOCK(m_mutex); - std::map::iterator it; - it = m_orphans.lower_bound(InsecureRand256()); + std::map::iterator it; + it = m_orphans.lower_bound(Txid::FromUint256(InsecureRand256())); if (it == m_orphans.end()) it = m_orphans.begin(); return it->second.tx; diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp index 7455d914e806c..d4c07b054372c 100644 --- a/src/txorphanage.cpp +++ b/src/txorphanage.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include @@ -20,8 +21,8 @@ bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer) { LOCK(m_mutex); - const uint256& hash = tx->GetHash(); - const uint256& wtxid = tx->GetWitnessHash(); + const Txid& hash = tx->GetHash(); + const Wtxid& wtxid = tx->GetWitnessHash(); if (m_orphans.count(hash)) return false; @@ -53,16 +54,16 @@ bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer) return true; } -int TxOrphanage::EraseTx(const uint256& txid) +int TxOrphanage::EraseTx(const Txid& txid) { LOCK(m_mutex); return EraseTxNoLock(txid); } -int TxOrphanage::EraseTxNoLock(const uint256& txid) +int TxOrphanage::EraseTxNoLock(const Txid& txid) { AssertLockHeld(m_mutex); - std::map::iterator it = m_orphans.find(txid); + auto it = m_orphans.find(txid); if (it == m_orphans.end()) return 0; for (const CTxIn& txin : it->second.tx->vin) @@ -100,10 +101,10 @@ void TxOrphanage::EraseForPeer(NodeId peer) m_peer_work_set.erase(peer); int nErased = 0; - std::map::iterator iter = m_orphans.begin(); + auto iter = m_orphans.begin(); while (iter != m_orphans.end()) { - std::map::iterator maybeErase = iter++; // increment to avoid iterator becoming invalid + auto maybeErase = iter++; // increment to avoid iterator becoming invalid if (maybeErase->second.fromPeer == peer) { nErased += EraseTxNoLock(maybeErase->second.tx->GetHash()); @@ -123,10 +124,10 @@ void TxOrphanage::LimitOrphans(unsigned int max_orphans) // Sweep out expired orphan pool entries: int nErased = 0; int64_t nMinExpTime = nNow + ORPHAN_TX_EXPIRE_TIME - ORPHAN_TX_EXPIRE_INTERVAL; - std::map::iterator iter = m_orphans.begin(); + auto iter = m_orphans.begin(); while (iter != m_orphans.end()) { - std::map::iterator maybeErase = iter++; + auto maybeErase = iter++; if (maybeErase->second.nTimeExpire <= nNow) { nErased += EraseTxNoLock(maybeErase->second.tx->GetHash()); } else { @@ -159,7 +160,7 @@ void TxOrphanage::AddChildrenToWorkSet(const CTransaction& tx) for (const auto& elem : it_by_prev->second) { // Get this source peer's work set, emplacing an empty set if it didn't exist // (note: if this peer wasn't still connected, we would have removed the orphan tx already) - std::set& orphan_work_set = m_peer_work_set.try_emplace(elem->second.fromPeer).first->second; + std::set& orphan_work_set = m_peer_work_set.try_emplace(elem->second.fromPeer).first->second; // Add this tx to the work set orphan_work_set.insert(elem->first); LogPrint(BCLog::TXPACKAGES, "added %s (wtxid=%s) to peer %d workset\n", @@ -173,9 +174,9 @@ bool TxOrphanage::HaveTx(const GenTxid& gtxid) const { LOCK(m_mutex); if (gtxid.IsWtxid()) { - return m_wtxid_to_orphan_it.count(gtxid.GetHash()); + return m_wtxid_to_orphan_it.count(Wtxid::FromUint256(gtxid.GetHash())); } else { - return m_orphans.count(gtxid.GetHash()); + return m_orphans.count(Txid::FromUint256(gtxid.GetHash())); } } @@ -187,7 +188,7 @@ CTransactionRef TxOrphanage::GetTxToReconsider(NodeId peer) if (work_set_it != m_peer_work_set.end()) { auto& work_set = work_set_it->second; while (!work_set.empty()) { - uint256 txid = *work_set.begin(); + Txid txid = *work_set.begin(); work_set.erase(work_set.begin()); const auto orphan_it = m_orphans.find(txid); @@ -215,7 +216,7 @@ void TxOrphanage::EraseForBlock(const CBlock& block) { LOCK(m_mutex); - std::vector vOrphanErase; + std::vector vOrphanErase; for (const CTransactionRef& ptx : block.vtx) { const CTransaction& tx = *ptx; @@ -226,7 +227,7 @@ void TxOrphanage::EraseForBlock(const CBlock& block) if (itByPrev == m_outpoint_to_orphan_it.end()) continue; for (auto mi = itByPrev->second.begin(); mi != itByPrev->second.end(); ++mi) { const CTransaction& orphanTx = *(*mi)->second.tx; - const uint256& orphanHash = orphanTx.GetHash(); + const auto& orphanHash = orphanTx.GetHash(); vOrphanErase.push_back(orphanHash); } } @@ -235,7 +236,7 @@ void TxOrphanage::EraseForBlock(const CBlock& block) // Erase orphan transactions included or precluded by this block if (vOrphanErase.size()) { int nErased = 0; - for (const uint256& orphanHash : vOrphanErase) { + for (const auto& orphanHash : vOrphanErase) { nErased += EraseTxNoLock(orphanHash); } LogPrint(BCLog::TXPACKAGES, "Erased %d orphan tx included or conflicted by block\n", nErased); diff --git a/src/txorphanage.h b/src/txorphanage.h index a4705bf382bd1..2361356402382 100644 --- a/src/txorphanage.h +++ b/src/txorphanage.h @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -34,7 +35,7 @@ class TxOrphanage { CTransactionRef GetTxToReconsider(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); /** Erase an orphan by txid */ - int EraseTx(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); + int EraseTx(const Txid& txid) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); /** Erase all orphans announced by a peer (eg, after that peer disconnects) */ void EraseForPeer(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); @@ -71,10 +72,10 @@ class TxOrphanage { /** Map from txid to orphan transaction record. Limited by * -maxorphantx/DEFAULT_MAX_ORPHAN_TRANSACTIONS */ - std::map m_orphans GUARDED_BY(m_mutex); + std::map m_orphans GUARDED_BY(m_mutex); /** Which peer provided the orphans that need to be reconsidered */ - std::map> m_peer_work_set GUARDED_BY(m_mutex); + std::map> m_peer_work_set GUARDED_BY(m_mutex); using OrphanMap = decltype(m_orphans); @@ -96,10 +97,10 @@ class TxOrphanage { /** Index from wtxid into the m_orphans to lookup orphan * transactions using their witness ids. */ - std::map m_wtxid_to_orphan_it GUARDED_BY(m_mutex); + std::map m_wtxid_to_orphan_it GUARDED_BY(m_mutex); /** Erase an orphan by txid */ - int EraseTxNoLock(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(m_mutex); + int EraseTxNoLock(const Txid& txid) EXCLUSIVE_LOCKS_REQUIRED(m_mutex); }; #endif // BITCOIN_TXORPHANAGE_H