Rewrite removeForReorg to avoid using sets

Also improve test coverage for removeForReorg by creating a scenario where
there are in-mempool descendants that are only invalidated due to an in-mempool
parent no longer spending a mature coin.

src/txmempool.cpp

@@ -352,15 +352,19 @@ void CTxMemPool::removeForReorg(CChain& chain, std::function<bool(txiter)> check
     AssertLockHeld(::cs_main);
     Assume(!m_have_changeset);
 
-    setEntries txToRemove;
+    std::vector<const TxGraph::Ref*> to_remove;
-    for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) {
+    for (txiter it = mapTx.begin(); it != mapTx.end(); it++) {
-        if (check_final_and_mature(it)) txToRemove.insert(it);
+        if (check_final_and_mature(it)) {
+            to_remove.emplace_back(&*it);
+        }
     }
-    setEntries setAllRemoves;
+
-    for (txiter it : txToRemove) {
+    auto all_to_remove = m_txgraph->GetDescendantsUnion(to_remove, TxGraph::Level::MAIN);
-        CalculateDescendants(it, setAllRemoves);
+
+    for (auto ref : all_to_remove) {
+        auto it = mapTx.iterator_to(static_cast<const CTxMemPoolEntry&>(*ref));
+        removeUnchecked(it, MemPoolRemovalReason::REORG);
     }
-    RemoveStaged(setAllRemoves, MemPoolRemovalReason::REORG);
     for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) {
         assert(TestLockPointValidity(chain, it->GetLockPoints()));
     }

test/functional/mempool_reorg.py

@@ -146,7 +146,7 @@ class MempoolCoinbaseTest(BitcoinTestFramework):
         assert_raises_rpc_error(-26, "non-final", self.nodes[0].sendrawtransaction, timelock_tx)
 
         self.log.info("Broadcast and mine spend_2 and spend_3")
-        wallet.sendrawtransaction(from_node=self.nodes[0], tx_hex=spend_2['hex'])
+        spend_2_id = wallet.sendrawtransaction(from_node=self.nodes[0], tx_hex=spend_2['hex'])
         wallet.sendrawtransaction(from_node=self.nodes[0], tx_hex=spend_3['hex'])
         self.log.info("Generate a block")
         self.generate(self.nodes[0], 1)
@@ -154,7 +154,7 @@ class MempoolCoinbaseTest(BitcoinTestFramework):
         assert_raises_rpc_error(-26, 'non-final', self.nodes[0].sendrawtransaction, timelock_tx)
 
         self.log.info("Create spend_2_1 and spend_3_1")
-        spend_2_1 = wallet.create_self_transfer(utxo_to_spend=spend_2["new_utxo"])
+        spend_2_1 = wallet.create_self_transfer(utxo_to_spend=spend_2["new_utxo"], version=1)
         spend_3_1 = wallet.create_self_transfer(utxo_to_spend=spend_3["new_utxo"])
 
         self.log.info("Broadcast and mine spend_3_1")
@@ -181,6 +181,24 @@ class MempoolCoinbaseTest(BitcoinTestFramework):
         self.log.info("spend_3_1 has been re-orged out of the chain and is back in the mempool")
         assert_equal(set(self.nodes[0].getrawmempool()), {spend_1_id, spend_2_1_id, spend_3_1_id})
 
+        self.log.info("Reorg out enough blocks to get spend_2 back in the mempool, along with its child")
+
+        while (spend_2_id not in self.nodes[0].getrawmempool()):
+            b = self.nodes[0].getbestblockhash()
+            for node in self.nodes:
+                node.invalidateblock(b)
+
+        assert(spend_2_id in self.nodes[0].getrawmempool())
+        assert(spend_2_1_id in self.nodes[0].getrawmempool())
+
+        # Chain 10 more transactions off of spend_2_1
+        self.log.info("Give spend_2 some more descendants by creating a chain of 10 transactions spending from it")
+        parent_utxo = spend_2_1["new_utxo"]
+        for i in range(10):
+            tx = wallet.create_self_transfer(utxo_to_spend=parent_utxo, version=1)
+            self.nodes[0].sendrawtransaction(tx['hex'])
+            parent_utxo = tx["new_utxo"]
+
         self.log.info("Use invalidateblock to re-org back and make all those coinbase spends immature/invalid")
         b = self.nodes[0].getblockhash(first_block + 100)
         for node in self.nodes: