Merge #19879: [p2p] miscellaneous wtxid followups

a8a64acaf32ac21feeb885671772282b531ef9a2 [BroadcastTransaction] Remove unsafe move operator (Amiti Uttarwar)
125c0381266e0e05a408f8e1818501ab73d29110 [p2p] Remove dead code (Amiti Uttarwar)
fc66d0a65cdc52a3b259effe0c29b5eafb1b5ff5 [p2p] Check for nullptr before dereferencing pointer (Adam Jonas)
cb79b9dbf4cd06e17c8c65b36bf15c3ea2641de4 [mempool] Revert unbroadcast set to tracking just txid (Amiti Uttarwar)

Pull request description:

  Addresses some outstanding review comments from #18044

  - reverts unbroadcast txids to a set instead of a map (simpler, communicates intent better, takes less space, no efficiency advantages of map)
  - adds safety around two touchpoints (check for nullptr before dereferencing pointer, remove an inaccurate std::move operator)
  - removes some dead code

  Links to comments on wtxid PR: [1](https://github.com/bitcoin/bitcoin/pull/18044#discussion_r460495254) [2](https://github.com/bitcoin/bitcoin/pull/18044#discussion_r460496023) [3](https://github.com/bitcoin/bitcoin/pull/18044#discussion_r463532611)

  thanks to jnewbery & adamjonas for flagging these ! !

ACKs for top commit:
  sdaftuar:
    utACK a8a64acaf32ac21feeb885671772282b531ef9a2
  naumenkogs:
    utACK a8a64acaf32ac21feeb885671772282b531ef9a2
  jnewbery:
    utACK a8a64acaf32ac21feeb885671772282b531ef9a2

Tree-SHA512: 7be669cb30cc17fb9e06b50e636ef7887c6a27354697987e4e4d38dba4b8f50e175647587430cd9bc3295bec01ce8b1e6639a50a4249d8fff9b1ca1b9ead3277

1 files changed, 10 insertions, 10 deletions

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 1d386b1ad8..166adc05bb 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -889,15 +889,16 @@ void PeerManager::InitializeNode(CNode *pnode) {
 
 void PeerManager::ReattemptInitialBroadcast(CScheduler& scheduler) const
 {
-    std::map<uint256, uint256> unbroadcast_txids = m_mempool.GetUnbroadcastTxs();
+    std::set<uint256> 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);
         }
     }
 
@@ -1492,8 +1493,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);
@@ -3105,8 +3107,6 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat
                 if (RecursiveDynamicUsage(*ptx) < 100000) {
                     AddToCompactExtraTransactions(ptx);
                 }
-            } else if (tx.HasWitness() && RecursiveDynamicUsage(*ptx) < 100000) {
-                AddToCompactExtraTransactions(ptx);
             }
 
             if (pfrom.HasPermission(PF_FORCERELAY)) {