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/txmempool.h b/src/txmempool.h
index 8764bbcf13..664fb5986a 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -587,10 +587,9 @@ private:
     std::vector<indexed_transaction_set::const_iterator> GetSortedDepthAndScore() const EXCLUSIVE_LOCKS_REQUIRED(cs);
 
     /**
-     * track locally submitted transactions to periodically retry initial broadcast
-     * map of txid -> wtxid
+     * Track locally submitted transactions to periodically retry initial broadcast.
      */
-    std::map<uint256, uint256> m_unbroadcast_txids GUARDED_BY(cs);
+    std::set<uint256> m_unbroadcast_txids GUARDED_BY(cs);
 
 public:
     indirectmap<COutPoint, const CTransaction*> mapNextTx GUARDED_BY(cs);
@@ -752,19 +751,20 @@ public:
     size_t DynamicMemoryUsage() const;
 
     /** Adds a transaction to the unbroadcast set */
-    void AddUnbroadcastTx(const uint256& txid, const uint256& wtxid) {
+    void AddUnbroadcastTx(const uint256& txid)
+    {
         LOCK(cs);
-        // Sanity Check: the transaction should also be in the mempool
-        if (exists(txid)) {
-            m_unbroadcast_txids[txid] = wtxid;
-        }
-    }
+        // Sanity check the transaction is in the mempool & insert into
+        // unbroadcast set.
+        if (exists(txid)) m_unbroadcast_txids.insert(txid);
+    };
 
     /** Removes a transaction from the unbroadcast set */
     void RemoveUnbroadcastTx(const uint256& txid, const bool unchecked = false);
 
     /** Returns transactions in unbroadcast set */
-    std::map<uint256, uint256> GetUnbroadcastTxs() const {
+    std::set<uint256> GetUnbroadcastTxs() const
+    {
         LOCK(cs);
         return m_unbroadcast_txids;
     }