CHIA-2382 Create a mempool item out of a copy of the input one when processing fast forward spends (#19273)

Create a fast forward mempool item out of a copy of the input one.

Author: AmineKhaldi

chia/_tests/core/mempool/test_singleton_fast_forward.py

@@ -1,10 +1,12 @@
 from __future__ import annotations
 
+import copy
 import dataclasses
 from typing import Any, Optional
 
 import pytest
 from chia_rs import AugSchemeMPL, G1Element, G2Element, PrivateKey
+from chiabip158 import PyBIP158
 
 from chia._tests.clvm.test_puzzles import public_key_for_index, secret_exponent_for_index
 from chia._tests.core.mempool.test_mempool_manager import (
@@ -57,14 +59,14 @@ async def get_unspent_lineage_info_for_puzzle_hash(_: bytes32) -> Optional[Unspe
     internal_mempool_item = InternalMempoolItem(sb, item.conds, item.height_added_to_mempool, item.bundle_coin_spends)
     original_version = dataclasses.replace(internal_mempool_item)
     eligible_coin_spends = EligibleCoinSpends()
-    await eligible_coin_spends.process_fast_forward_spends(
+    bundle_coin_spends = await eligible_coin_spends.process_fast_forward_spends(
         mempool_item=internal_mempool_item,
         get_unspent_lineage_info_for_puzzle_hash=get_unspent_lineage_info_for_puzzle_hash,
         height=TEST_HEIGHT,
         constants=DEFAULT_CONSTANTS,
     )
     assert eligible_coin_spends == EligibleCoinSpends()
-    assert internal_mempool_item == original_version
+    assert bundle_coin_spends == original_version.bundle_coin_spends
 
 
 @pytest.mark.anyio
@@ -130,7 +132,7 @@ async def get_unspent_lineage_info_for_puzzle_hash(puzzle_hash: bytes32) -> Opti
     internal_mempool_item = InternalMempoolItem(sb, item.conds, item.height_added_to_mempool, item.bundle_coin_spends)
     original_version = dataclasses.replace(internal_mempool_item)
     eligible_coin_spends = EligibleCoinSpends()
-    await eligible_coin_spends.process_fast_forward_spends(
+    bundle_coin_spends = await eligible_coin_spends.process_fast_forward_spends(
         mempool_item=internal_mempool_item,
         get_unspent_lineage_info_for_puzzle_hash=get_unspent_lineage_info_for_puzzle_hash,
         height=TEST_HEIGHT,
@@ -149,7 +151,7 @@ async def get_unspent_lineage_info_for_puzzle_hash(puzzle_hash: bytes32) -> Opti
     # We have set the next version from our additions to chain ff spends
     assert eligible_coin_spends.fast_forward_spends == expected_fast_forward_spends
     # We didn't need to fast forward the item so it stays as is
-    assert internal_mempool_item == original_version
+    assert bundle_coin_spends == original_version.bundle_coin_spends
 
 
 def test_perform_the_fast_forward() -> None:
@@ -607,3 +609,86 @@ async def test_singleton_fast_forward_same_block() -> None:
         assert unspent_lineage_info.parent_id == latest_singleton.parent_coin_info
         # The one before it should have the second last random amount
         assert unspent_lineage_info.parent_amount == random_amounts[-2]
+
+
+@pytest.mark.anyio
+async def test_mempool_items_immutability_on_ff() -> None:
+    """
+    This tests processing singleton fast forward spends for mempool items using
+    modified copies, without altering those original mempool items.
+    """
+    SINGLETON_AMOUNT = uint64(1337)
+    async with sim_and_client() as (sim, sim_client):
+        singleton, eve_coin_spend, inner_puzzle, remaining_coin = await prepare_and_test_singleton(
+            sim, sim_client, True, SINGLETON_AMOUNT, SINGLETON_AMOUNT
+        )
+        singleton_name = singleton.name()
+        singleton_puzzle_hash = eve_coin_spend.coin.puzzle_hash
+        inner_puzzle_hash = inner_puzzle.get_tree_hash()
+        sk = AugSchemeMPL.key_gen(b"1" * 32)
+        g1 = sk.get_g1()
+        sig = AugSchemeMPL.sign(sk, b"foobar", g1)
+        inner_conditions: list[list[Any]] = [
+            [ConditionOpcode.AGG_SIG_UNSAFE, bytes(g1), b"foobar"],
+            [ConditionOpcode.CREATE_COIN, inner_puzzle_hash, SINGLETON_AMOUNT],
+        ]
+        singleton_coin_spend, singleton_signing_puzzle = make_singleton_coin_spend(
+            eve_coin_spend, singleton, inner_puzzle, inner_conditions
+        )
+        remaining_spend_solution = SerializedProgram.from_program(
+            Program.to([[ConditionOpcode.CREATE_COIN, IDENTITY_PUZZLE_HASH, remaining_coin.amount]])
+        )
+        remaining_coin_spend = CoinSpend(remaining_coin, IDENTITY_PUZZLE, remaining_spend_solution)
+        await make_and_send_spend_bundle(
+            sim,
+            sim_client,
+            [remaining_coin_spend, singleton_coin_spend],
+            signing_puzzle=singleton_signing_puzzle,
+            signing_coin=singleton,
+            aggsig=sig,
+        )
+        unspent_lineage_info = await sim_client.service.coin_store.get_unspent_lineage_info_for_puzzle_hash(
+            singleton_puzzle_hash
+        )
+        singleton_child, [remaining_coin] = await get_singleton_and_remaining_coins(sim)
+        singleton_child_name = singleton_child.name()
+        assert singleton_child.amount == SINGLETON_AMOUNT
+        assert unspent_lineage_info == UnspentLineageInfo(
+            coin_id=singleton_child_name,
+            coin_amount=singleton_child.amount,
+            parent_id=singleton_name,
+            parent_amount=singleton.amount,
+            parent_parent_id=eve_coin_spend.coin.name(),
+        )
+        # Now let's spend the first version again (despite being already spent
+        # by now) to exercise its fast forward.
+        remaining_spend_solution = SerializedProgram.from_program(
+            Program.to([[ConditionOpcode.CREATE_COIN, IDENTITY_PUZZLE_HASH, remaining_coin.amount]])
+        )
+        remaining_coin_spend = CoinSpend(remaining_coin, IDENTITY_PUZZLE, remaining_spend_solution)
+        sb = SpendBundle([remaining_coin_spend, singleton_coin_spend], sig)
+        sb_name = sb.name()
+        status, error = await sim_client.push_tx(sb)
+        assert status == MempoolInclusionStatus.SUCCESS
+        assert error is None
+        original_item = copy.copy(sim_client.service.mempool_manager.get_mempool_item(sb_name))
+        original_filter = sim_client.service.mempool_manager.get_filter()
+        # Let's trigger the fast forward by creating a mempool bundle
+        result = await sim.mempool_manager.create_bundle_from_mempool(
+            sim_client.service.block_records[-1].header_hash,
+            sim_client.service.coin_store.get_unspent_lineage_info_for_puzzle_hash,
+        )
+        assert result is not None
+        bundle, _ = result
+        # Make sure the mempool bundle we created contains the result of our
+        # fast forward, instead of our original spend.
+        assert any(cs.coin.name() == singleton_child_name for cs in bundle.coin_spends)
+        assert not any(cs.coin.name() == singleton_name for cs in bundle.coin_spends)
+        # We should have processed our item without modifying it in-place
+        new_item = copy.copy(sim_client.service.mempool_manager.get_mempool_item(sb_name))
+        new_filter = sim_client.service.mempool_manager.get_filter()
+        assert new_item == original_item
+        assert new_filter == original_filter
+        sb_filter = PyBIP158(bytearray(original_filter))
+        items_not_in_sb_filter = sim_client.service.mempool_manager.get_items_not_in_filter(sb_filter)
+        assert len(items_not_in_sb_filter) == 0

chia/full_node/mempool.py

@@ -526,14 +526,14 @@ async def create_bundle_from_mempool_items(
                         unique_additions.extend(spend_data.additions)
                     cost_saving = 0
                 else:
-                    await eligible_coin_spends.process_fast_forward_spends(
+                    bundle_coin_spends = await eligible_coin_spends.process_fast_forward_spends(
                         mempool_item=item,
                         get_unspent_lineage_info_for_puzzle_hash=get_unspent_lineage_info_for_puzzle_hash,
                         height=height,
                         constants=constants,
                     )
                     unique_coin_spends, cost_saving, unique_additions = eligible_coin_spends.get_deduplication_info(
-                        bundle_coin_spends=item.bundle_coin_spends, max_cost=cost
+                        bundle_coin_spends=bundle_coin_spends, max_cost=cost
                     )
                 item_cost = cost - cost_saving
                 log.info(

chia/types/eligible_coin_spends.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import copy
 import dataclasses
 from collections.abc import Awaitable
 from typing import Callable, Optional
@@ -233,26 +234,36 @@ async def process_fast_forward_spends(
         get_unspent_lineage_info_for_puzzle_hash: Callable[[bytes32], Awaitable[Optional[UnspentLineageInfo]]],
         height: uint32,
         constants: ConsensusConstants,
-    ) -> None:
+    ) -> dict[bytes32, BundleCoinSpend]:
         """
-        Provides the caller with an in-place internal mempool item that has a
-        proper state of fast forwarded coin spends and additions starting from
+        Provides the caller with a `bundle_coin_spends` map that has a proper
+        state of fast forwarded coin spends and additions starting from
         the most recent unspent versions of the related singleton spends.
 
         Args:
-            mempool_item: in-out parameter for the internal mempool item to process
+            mempool_item: The internal mempool item to process
             get_unspent_lineage_info_for_puzzle_hash: to lookup the most recent
                 version of the singleton from the coin store
             constants: needed in order to refresh the mempool item if needed
             height: needed in order to refresh the mempool item if needed
 
+        Returns:
+            The resulting `bundle_coin_spends` map of coin IDs to coin spends
+            and metadata, after fast forwarding
+
         Raises:
             If a fast forward cannot proceed, to prevent potential double spends
         """
+
+        # Let's first create a copy of the mempool item's `bundle_coin_spends`
+        # map to work on and return. This way we avoid the possibility of
+        # propagating a modified version of this item through the network.
+        bundle_coin_spends = copy.copy(mempool_item.bundle_coin_spends)
         new_coin_spends = []
+        # Map of rebased singleton coin ID to coin spend and metadata
         ff_bundle_coin_spends = {}
         replaced_coin_ids = []
-        for coin_id, spend_data in mempool_item.bundle_coin_spends.items():
+        for coin_id, spend_data in bundle_coin_spends.items():
             if not spend_data.eligible_for_fast_forward:
                 # Nothing to do for this spend, moving on
                 new_coin_spends.append(spend_data.coin_spend)
@@ -326,21 +337,17 @@ async def process_fast_forward_spends(
             new_coin_spends.append(new_coin_spend)
         if len(ff_bundle_coin_spends) == 0:
             # This item doesn't have any fast forward coins, nothing to do here
-            return
+            return bundle_coin_spends
         # Update the mempool item after validating the new spend bundle
         new_sb = SpendBundle(
             coin_spends=new_coin_spends, aggregated_signature=mempool_item.spend_bundle.aggregated_signature
         )
-        # We need to run the new spend bundle to make sure it remains valid
         assert mempool_item.conds is not None
         try:
-            new_conditions = get_conditions_from_spendbundle(
-                new_sb,
-                mempool_item.conds.cost,
-                constants,
-                height,
-            )
-        # validate_clvm_and_signature raises a TypeError with an error code
+            # Run the new spend bundle to make sure it remains valid. What we
+            # care about here is whether this call throws or not.
+            get_conditions_from_spendbundle(new_sb, mempool_item.conds.cost, constants, height)
+        # get_conditions_from_spendbundle raises a TypeError with an error code
         except TypeError as e:
             # Convert that to a ValidationError
             if len(e.args) > 0:
@@ -351,15 +358,9 @@ async def process_fast_forward_spends(
                     "Mempool item became invalid after singleton fast forward with an unspecified error."
                 )  # pragma: no cover
 
-        # Update bundle_coin_spends using the collected data
+        # Update bundle_coin_spends using the map of rebased singleton coin ID
+        # to coin spend and metadata.
         for coin_id in replaced_coin_ids:
-            mempool_item.bundle_coin_spends.pop(coin_id, None)
-        mempool_item.bundle_coin_spends.update(ff_bundle_coin_spends)
-        # Update the mempool item with the new spend bundle related information
-        # NOTE: From this point on, in `create_bundle_from_mempool_items`, we rely
-        # on `bundle_coin_spends` and we don't use this updated spend bundle
-        # information, as we'll only need `aggregated_signature` which doesn't
-        # change. Still, it's good form to update the spend bundle with the
-        # new coin spends
-        mempool_item.spend_bundle = new_sb
-        mempool_item.conds = new_conditions
+            bundle_coin_spends.pop(coin_id, None)
+        bundle_coin_spends.update(ff_bundle_coin_spends)
+        return bundle_coin_spends

chia/types/internal_mempool_item.py

@@ -9,7 +9,7 @@
 from chia.util.ints import uint32
 
 
-@dataclass
+@dataclass(frozen=True)
 class InternalMempoolItem:
     spend_bundle: SpendBundle
     conds: SpendBundleConditions