Simplify create_bundle_from_mempool's handling of removals (#15056)

Simplify create_bundle_from_mempool's handling of removals.

They're already taken care of by aggregation.

chia/clvm/spend_sim.py

@@ -254,13 +254,13 @@ class SpendSim:
                 result = self.mempool_manager.create_bundle_from_mempool(peak.header_hash, item_inclusion_filter)
 
                 if result is not None:
-                    bundle, additions, removals = result
+                    bundle, additions = result
                     generator_bundle = bundle
                     return_additions = additions
-                    return_removals = removals
+                    return_removals = bundle.removals()
 
                     await self.coin_store._add_coin_records([self.new_coin_record(addition) for addition in additions])
-                    await self.coin_store._set_spent([r.name() for r in removals], uint32(self.block_height + 1))
+                    await self.coin_store._set_spent([r.name() for r in return_removals], uint32(self.block_height + 1))
 
         # SimBlockRecord is created
         generator: Optional[BlockGenerator] = await self.generate_transaction_generator(generator_bundle)

chia/full_node/full_node_api.py

@@ -755,9 +755,8 @@ class FullNodeAPI:
                         self.full_node.log.error(f"Error making spend bundle {e} peak: {peak}")
                         mempool_bundle = None
                     if mempool_bundle is not None:
-                        spend_bundle = mempool_bundle[0]
-                        additions = mempool_bundle[1]
-                        removals = mempool_bundle[2]
+                        spend_bundle, additions = mempool_bundle
+                        removals = spend_bundle.removals()
                         self.full_node.log.info(f"Add rem: {len(additions)} {len(removals)}")
                         aggregate_signature = spend_bundle.aggregated_signature
                         if self.full_node.full_node_store.previous_generator is not None:

chia/full_node/mempool.py

@@ -376,11 +376,10 @@ class Mempool:
 
     def create_bundle_from_mempool_items(
         self, item_inclusion_filter: Callable[[bytes32], bool]
-    ) -> Optional[Tuple[SpendBundle, List[Coin], List[Coin]]]:
+    ) -> Optional[Tuple[SpendBundle, List[Coin]]]:
         cost_sum = 0  # Checks that total cost does not exceed block maximum
         fee_sum = 0  # Checks that total fees don't exceed 64 bits
         spend_bundles: List[SpendBundle] = []
-        removals: List[Coin] = []
         additions: List[Coin] = []
         log.info(f"Starting to make block, max cost: {self.mempool_info.max_block_clvm_cost}")
         for item in self.spends_by_feerate():
@@ -395,7 +394,6 @@ class Mempool:
             spend_bundles.append(item.spend_bundle)
             cost_sum += item.cost
             fee_sum += item.fee
-            removals.extend(item.removals)
             if item.npc_result.conds is not None:
                 for spend in item.npc_result.conds.spends:
                     for puzzle_hash, amount, _ in spend.create_coin:
@@ -408,4 +406,4 @@ class Mempool:
             f"full: {cost_sum / self.mempool_info.max_block_clvm_cost}"
         )
         agg = SpendBundle.aggregate(spend_bundles)
-        return agg, additions, removals
+        return agg, additions

chia/full_node/mempool_manager.py

@@ -210,10 +210,8 @@ class MempoolManager:
         self.pool.shutdown(wait=True)
 
     def create_bundle_from_mempool(
-        self,
-        last_tb_header_hash: bytes32,
-        item_inclusion_filter: Optional[Callable[[bytes32], bool]] = None,
-    ) -> Optional[Tuple[SpendBundle, List[Coin], List[Coin]]]:
+        self, last_tb_header_hash: bytes32, item_inclusion_filter: Optional[Callable[[bytes32], bool]] = None
+    ) -> Optional[Tuple[SpendBundle, List[Coin]]]:
         """
         Returns aggregated spendbundle that can be used for creating new block,
         additions and removals in that spend_bundle

tests/blockchain/test_blockchain_transactions.py

@@ -58,10 +58,9 @@ class TestBlockchainTransactions:
         assert sb == spend_bundle
 
         last_block = blocks[-1]
-        next_spendbundle, additions, removals = full_node_1.mempool_manager.create_bundle_from_mempool(
-            last_block.header_hash
-        )
-        assert next_spendbundle is not None
+        result = full_node_1.mempool_manager.create_bundle_from_mempool(last_block.header_hash)
+        assert result is not None
+        next_spendbundle, _ = result
 
         new_blocks = bt.get_consecutive_blocks(
             1,

tests/core/mempool/test_mempool_manager.py

@@ -919,12 +919,12 @@ async def test_create_bundle_from_mempool_on_max_cost() -> None:
     assert mempool_manager.peak is not None
     result = mempool_manager.create_bundle_from_mempool(mempool_manager.peak.header_hash)
     assert result is not None
-    agg, additions, removals = result
+    agg, additions = result
     # The second spend bundle has a higher FPC so it should get picked first
     assert agg == sb2
     # The first spend bundle hits the maximum block clvm cost and gets skipped
     assert additions == [Coin(coins[1].name(), IDENTITY_PUZZLE_HASH, coins[1].amount - 2)]
-    assert removals == [coins[1]]
+    assert agg.removals() == [coins[1]]
 
 
 @pytest.mark.parametrize(