From cb79b9dbf4cd06e17c8c65b36bf15c3ea2641de4 Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Mon, 27 Jul 2020 21:30:50 -0700 Subject: [mempool] Revert unbroadcast set to tracking just txid When I originally implemented the unbroadcast set in 18038, it just tracked txids. After 18038 was merged, I offered a patch to 18044 to make the unbroadcast changes compatible with wtxid relay. In this patch, I updated `unbroadcast_txids` to a map of txid -> wtxid. Post merge review comments shed light on the fact that this update was unnecessary, and distracting. So, this commit updates the unbroadcast ids back to a set. --- src/net_processing.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index ce4ac3cd7..e1007e071 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -889,15 +889,16 @@ void PeerLogicValidation::InitializeNode(CNode *pnode) { void PeerLogicValidation::ReattemptInitialBroadcast(CScheduler& scheduler) const { - std::map unbroadcast_txids = m_mempool.GetUnbroadcastTxs(); + std::set unbroadcast_txids = m_mempool.GetUnbroadcastTxs(); - for (const auto& elem : unbroadcast_txids) { - // Sanity check: all unbroadcast txns should exist in the mempool - if (m_mempool.exists(elem.first)) { + for (const auto& txid : unbroadcast_txids) { + CTransactionRef tx = m_mempool.get(txid); + + if (tx != nullptr) { LOCK(cs_main); - RelayTransaction(elem.first, elem.second, m_connman); + RelayTransaction(txid, tx->GetWitnessHash(), m_connman); } else { - m_mempool.RemoveUnbroadcastTx(elem.first, true); + m_mempool.RemoveUnbroadcastTx(txid, true); } } -- cgit v1.2.3 From fc66d0a65cdc52a3b259effe0c29b5eafb1b5ff5 Mon Sep 17 00:00:00 2001 From: Adam Jonas Date: Mon, 17 Aug 2020 16:35:03 -0400 Subject: [p2p] Check for nullptr before dereferencing pointer --- src/net_processing.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index e1007e071..578d93150 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1509,8 +1509,9 @@ void RelayTransaction(const uint256& txid, const uint256& wtxid, const CConnman& { LockAssertion lock(::cs_main); - CNodeState &state = *State(pnode->GetId()); - if (state.m_wtxid_relay) { + CNodeState* state = State(pnode->GetId()); + if (state == nullptr) return; + if (state->m_wtxid_relay) { pnode->PushTxInventory(wtxid); } else { pnode->PushTxInventory(txid); -- cgit v1.2.3 From 125c0381266e0e05a408f8e1818501ab73d29110 Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Mon, 24 Aug 2020 17:00:05 -0700 Subject: [p2p] Remove dead code The else clause is dead code because the only way to not enter the if branch is if TX_WITNESS_STRIPPED is true. In that case, it would not have a witness to match the `tx.HasWitness()` else condition. Co-authored-by: Adam Jonas Co-authored-by: John Newbery --- src/net_processing.cpp | 2 -- 1 file changed, 2 deletions(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 578d93150..3135fcb20 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3121,8 +3121,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty if (RecursiveDynamicUsage(*ptx) < 100000) { AddToCompactExtraTransactions(ptx); } - } else if (tx.HasWitness() && RecursiveDynamicUsage(*ptx) < 100000) { - AddToCompactExtraTransactions(ptx); } if (pfrom.HasPermission(PF_FORCERELAY)) { -- cgit v1.2.3