Add increment to skipped_items if we hit an Exception in mempool (#19286)

* add skip to exception

* fix test as last coin gets skipped

* Add skip to exception update (#19339)

* don't throw exception when skipping FF or Dedup coin in mempool

* skipping a dedup because of different solution is not an error

* correct logging and tests

* unit test skipping FF after 3 errors

---------

Co-authored-by: Arvid Norberg <[email protected]>

Author: matt-o-how

chia/_tests/core/mempool/test_mempool.py

@@ -7,6 +7,7 @@
 
 import pytest
 from chia_rs import (
+    ELIGIBLE_FOR_FF,
     ENABLE_KECCAK,
     ENABLE_KECCAK_OPS_OUTSIDE_GUARD,
     AugSchemeMPL,
@@ -2931,6 +2932,39 @@ def test_items_by_feerate(items: list[MempoolItem], expected: list[Coin]) -> Non
         last_fpc = mi.fee_per_cost
 
 
+# make sure that after failing to pick 3 fast-forward spends, we skip
+# FF spends
+@pytest.mark.anyio
+async def test_skip_error_items() -> None:
+    fee_estimator = create_bitcoin_fee_estimator(uint64(11000000000))
+    mempool_info = MempoolInfo(
+        CLVMCost(uint64(11000000000 * 3)),
+        FeeRate(uint64(1000000)),
+        CLVMCost(uint64(11000000000)),
+    )
+    mempool = Mempool(mempool_info, fee_estimator)
+
+    # all 50 items support fast forward
+    for i in range(50):
+        item = mk_item(coins[i : i + 1], flags=[ELIGIBLE_FOR_FF], fee=0, cost=50)
+        add_info = mempool.add_to_pool(item)
+        assert add_info.error is None
+
+    called = 0
+
+    async def local_get_unspent_lineage_info(ph: bytes32) -> Optional[UnspentLineageInfo]:
+        nonlocal called
+        called += 1
+        raise RuntimeError("failed to find fast forward coin")
+
+    result = await mempool.create_block_generator(local_get_unspent_lineage_info, DEFAULT_CONSTANTS, uint32(10))
+    assert result is not None
+    generator, _, _ = result
+
+    assert called == 3
+    assert generator.program == SerializedProgram.from_bytes(bytes.fromhex("ff01ff8080"))
+
+
 def rand_hash() -> bytes32:
     rng = random.Random()
     ret = bytearray(32)

chia/_tests/core/mempool/test_mempool_manager.py

@@ -52,6 +52,7 @@
     DedupCoinSpend,
     EligibilityAndAdditions,
     EligibleCoinSpends,
+    SkipDedup,
     UnspentLineageInfo,
     run_for_cost,
 )
@@ -1185,10 +1186,7 @@ async def make_and_send_big_cost_sb(coin: Coin) -> None:
     assert result is not None
     agg, additions = result
     skipped_due_to_eligible_coins = sum(
-        1
-        for line in caplog.text.split("\n")
-        if "Exception while checking a mempool item for deduplication: Skipping transaction with eligible coin(s)"
-        in line
+        1 for line in caplog.text.split("\n") if "Skipping transaction with dedup or FF spends" in line
     )
     if num_skipped_items == PRIORITY_TX_THRESHOLD:
         # We skipped enough big cost items to reach `PRIORITY_TX_THRESHOLD`,
@@ -1511,7 +1509,7 @@ def test_dedup_info_eligible_but_different_solution() -> None:
     conditions = [[ConditionOpcode.CREATE_COIN, IDENTITY_PUZZLE_HASH, TEST_COIN_AMOUNT]]
     sb = spend_bundle_from_conditions(conditions, TEST_COIN)
     mempool_item = mempool_item_from_spendbundle(sb)
-    with pytest.raises(ValueError, match="Solution is different from what we're deduplicating on"):
+    with pytest.raises(SkipDedup, match="Solution is different from what we're deduplicating on"):
         eligible_coin_spends.get_deduplication_info(
             bundle_coin_spends=mempool_item.bundle_coin_spends, max_cost=mempool_item.conds.cost
         )

chia/full_node/mempool.py

@@ -19,7 +19,7 @@
 from chia.types.blockchain_format.serialized_program import SerializedProgram
 from chia.types.clvm_cost import CLVMCost
 from chia.types.coin_spend import CoinSpend
-from chia.types.eligible_coin_spends import EligibleCoinSpends, UnspentLineageInfo
+from chia.types.eligible_coin_spends import EligibleCoinSpends, SkipDedup, UnspentLineageInfo
 from chia.types.generator_types import BlockGenerator
 from chia.types.internal_mempool_item import InternalMempoolItem
 from chia.types.mempool_item import MempoolItem
@@ -558,11 +558,18 @@ async def create_bundle_from_mempool_items(
                     # we want to keep looking for smaller transactions that
                     # might fit, but we also want to avoid spending too much
                     # time on potentially expensive ones, hence this shortcut.
+                    if any(
+                        map(
+                            lambda spend_data: (spend_data.eligible_for_dedup or spend_data.eligible_for_fast_forward),
+                            item.bundle_coin_spends.values(),
+                        )
+                    ):
+                        log.info("Skipping transaction with dedup or FF spends {item.name}")
+                        continue
+
                     unique_coin_spends = []
                     unique_additions = []
                     for spend_data in item.bundle_coin_spends.values():
-                        if spend_data.eligible_for_dedup or spend_data.eligible_for_fast_forward:
-                            raise Exception(f"Skipping transaction with eligible coin(s): {name.hex()}")
                         unique_coin_spends.append(spend_data.coin_spend)
                         unique_additions.extend(spend_data.additions)
                     cost_saving = 0
@@ -610,8 +617,12 @@ async def create_bundle_from_mempool_items(
                 # find transactions small enough to fit at this point
                 if self.mempool_info.max_block_clvm_cost - cost_sum < MIN_COST_THRESHOLD:
                     break
+            except SkipDedup as e:
+                log.info(f"{e}")
+                continue
             except Exception as e:
                 log.info(f"Exception while checking a mempool item for deduplication: {e}")
+                skipped_items += 1
                 continue
         if processed_spend_bundles == 0:
             return None

chia/types/eligible_coin_spends.py

@@ -143,6 +143,11 @@ def perform_the_fast_forward(
     return new_coin_spend, patched_additions
 
 
+@dataclasses.dataclass
+class SkipDedup(BaseException):
+    msg: str
+
+
 @dataclasses.dataclass(frozen=True)
 class EligibleCoinSpends:
     deduplication_spends: dict[bytes32, DedupCoinSpend] = dataclasses.field(default_factory=dict)
@@ -200,7 +205,7 @@ def get_deduplication_info(
                 # even if they end up saving more cost, as we're going for the first
                 # solution we see from the relatively highest FPC item, to avoid
                 # severe performance and/or time-complexity impact
-                raise ValueError("Solution is different from what we're deduplicating on")
+                raise SkipDedup("Solution is different from what we're deduplicating on")
             # Let's calculate the saved cost if we never did that before
             if duplicate_cost is None:
                 # See first if this mempool item had this cost computed before