use per-puzzle cost to estimate DEDUP savings, rather than rerunning puzzles

Author: arvidn

chia/_tests/core/mempool/test_mempool.py

@@ -42,7 +42,6 @@
 from chia.consensus.cost_calculator import NPCResult
 from chia.consensus.default_constants import DEFAULT_CONSTANTS
 from chia.full_node.bitcoin_fee_estimator import create_bitcoin_fee_estimator
-from chia.full_node.eligible_coin_spends import run_for_cost
 from chia.full_node.fee_estimation import EmptyMempoolInfo, MempoolInfo
 from chia.full_node.full_node_api import FullNodeAPI
 from chia.full_node.mempool import Mempool
@@ -3135,15 +3134,11 @@ def make_test_spendbundle(coin: Coin, *, fee: int = 0, with_higher_cost: bool =
 
 
 def test_aggregating_on_a_solution_then_a_more_cost_saving_one_appears() -> None:
-    def agg_and_add_sb_returning_cost_info(mempool: Mempool, spend_bundles: list[SpendBundle]) -> uint64:
+    def agg_and_add_sb_returning_cost_info(mempool: Mempool, spend_bundles: list[SpendBundle]) -> None:
         sb = SpendBundle.aggregate(spend_bundles)
         mi = mempool_item_from_spendbundle(sb)
         mempool.add_to_pool(mi)
         invariant_check_mempool(mempool)
-        saved_cost = run_for_cost(
-            sb.coin_spends[0].puzzle_reveal, sb.coin_spends[0].solution, len(mi.additions), mi.cost
-        )
-        return saved_cost
 
     fee_estimator = create_bitcoin_fee_estimator(uint64(11000000000))
     mempool_info = MempoolInfo(
@@ -3163,7 +3158,7 @@ def agg_and_add_sb_returning_cost_info(mempool: Mempool, spend_bundles: list[Spe
     invariant_check_mempool(mempool)
     # Create a ~2 FPC item that spends the eligible coin using the same solution A
     sb_low_rate = make_test_spendbundle(coins[2], fee=highest_fee // 5)
-    saved_cost_on_solution_A = agg_and_add_sb_returning_cost_info(mempool, [sb_A, sb_low_rate])
+    agg_and_add_sb_returning_cost_info(mempool, [sb_A, sb_low_rate])
     invariant_check_mempool(mempool)
     result = mempool.create_bundle_from_mempool_items(test_constants, uint32(0))
     assert result is not None
@@ -3177,10 +3172,8 @@ def agg_and_add_sb_returning_cost_info(mempool: Mempool, spend_bundles: list[Spe
         # We're picking this fee to get a ~3 FPC, and get picked after sb_A1
         # (which has ~10 FPC) but before sb_A2 (which has ~2 FPC)
         sb_mid_rate = make_test_spendbundle(coins[i], fee=38004852 - i)
-        saved_cost_on_solution_B = agg_and_add_sb_returning_cost_info(mempool, [sb_B, sb_mid_rate])
+        agg_and_add_sb_returning_cost_info(mempool, [sb_B, sb_mid_rate])
         invariant_check_mempool(mempool)
-    # We'd save more cost if we went with solution B instead of A
-    assert saved_cost_on_solution_B > saved_cost_on_solution_A
     # If we process everything now, the 3 x ~3 FPC items get skipped because
     # sb_A1 gets picked before them (~10 FPC), so from then on only sb_A2 (~2 FPC)
     # would get picked

chia/_tests/core/mempool/test_mempool_manager.py

@@ -34,7 +34,6 @@
     DedupCoinSpend,
     IdenticalSpendDedup,
     SkipDedup,
-    run_for_cost,
 )
 from chia.full_node.mempool import MAX_SKIPPED_ITEMS, PRIORITY_TX_THRESHOLD
 from chia.full_node.mempool_manager import (
@@ -535,7 +534,7 @@ def spend_bundle_from_conditions(
 
 async def add_spendbundle(
     mempool_manager: MempoolManager, sb: SpendBundle, sb_name: bytes32
-) -> tuple[Optional[uint64], MempoolInclusionStatus, Optional[Err]]:
+) -> tuple[uint64, MempoolInclusionStatus, Optional[Err]]:
     sbc = await mempool_manager.pre_validate_spendbundle(sb, sb_name)
     ret = await mempool_manager.add_spend_bundle(sb, sbc, sb_name, TEST_HEIGHT)
     invariant_check_mempool(mempool_manager.mempool)
@@ -547,7 +546,7 @@ async def generate_and_add_spendbundle(
     conditions: list[list[Any]],
     coin: Coin = TEST_COIN,
     aggsig: G2Element = G2Element(),
-) -> tuple[SpendBundle, bytes32, tuple[Optional[uint64], MempoolInclusionStatus, Optional[Err]]]:
+) -> tuple[SpendBundle, bytes32, tuple[uint64, MempoolInclusionStatus, Optional[Err]]]:
     sb = spend_bundle_from_conditions(conditions, coin, aggsig)
     sb_name = sb.name()
     result = await add_spendbundle(mempool_manager, sb, sb_name)
@@ -579,6 +578,7 @@ def make_bundle_spends_map_and_fee(
             eligible_for_dedup=bool(spend_conds.flags & ELIGIBLE_FOR_DEDUP),
             eligible_for_fast_forward=bool(spend_conds.flags & ELIGIBLE_FOR_FF),
             additions=additions,
+            cost=uint64(spend_conds.condition_cost + spend_conds.execution_cost),
             latest_singleton_lineage=UnspentLineageInfo(coin_id, coin_spend.coin.parent_coin_info, bytes32([0] * 32))
             if bool(spend_conds.flags & ELIGIBLE_FOR_FF)
             else None,
@@ -703,7 +703,7 @@ async def get_coin_records(_: Collection[bytes32]) -> list[CoinRecord]:
     mempool_manager = await instantiate_mempool_manager(get_coin_records)
     conditions = [[ConditionOpcode.CREATE_COIN, IDENTITY_PUZZLE_HASH, 1]]
     _, _, result = await generate_and_add_spendbundle(mempool_manager, conditions)
-    assert result == (None, MempoolInclusionStatus.FAILED, Err.UNKNOWN_UNSPENT)
+    assert result == (0, MempoolInclusionStatus.FAILED, Err.UNKNOWN_UNSPENT)
 
 
 @pytest.mark.anyio
@@ -885,6 +885,7 @@ def mk_bcs(coin_spend: CoinSpend, flags: int = 0) -> BundleCoinSpend:
         eligible_for_dedup=bool(flags & ELIGIBLE_FOR_DEDUP),
         eligible_for_fast_forward=bool(flags & ELIGIBLE_FOR_FF),
         additions=[],
+        cost=uint64(0),
     )
 
 
@@ -1578,21 +1579,6 @@ async def test_replacing_one_with_an_eligible_coin() -> None:
     assert_sb_in_pool(mempool_manager, sb123e4)
 
 
-@pytest.mark.parametrize("amount", [0, 1])
-def test_run_for_cost(amount: int) -> None:
-    conditions = [[ConditionOpcode.CREATE_COIN, IDENTITY_PUZZLE_HASH, amount]]
-    solution = SerializedProgram.to(conditions)
-    cost = run_for_cost(IDENTITY_PUZZLE, solution, additions_count=1, max_cost=uint64(10000000))
-    assert cost == uint64(1800044)
-
-
-def test_run_for_cost_max_cost() -> None:
-    conditions = [[ConditionOpcode.CREATE_COIN, IDENTITY_PUZZLE_HASH, 1]]
-    solution = SerializedProgram.to(conditions)
-    with pytest.raises(ValueError, match="cost exceeded"):
-        run_for_cost(IDENTITY_PUZZLE, solution, additions_count=1, max_cost=uint64(43))
-
-
 def test_dedup_info_nothing_to_do() -> None:
     # No eligible coins, nothing to deduplicate, item gets considered normally
 
@@ -1636,7 +1622,9 @@ def test_dedup_info_eligible_1st_time() -> None:
         Coin(TEST_COIN_ID, IDENTITY_PUZZLE_HASH, uint64(1)),
         Coin(TEST_COIN_ID, IDENTITY_PUZZLE_HASH, uint64(TEST_COIN_AMOUNT - 1)),
     }
-    assert dedup_coin_spends == IdenticalSpendDedup({TEST_COIN_ID: DedupCoinSpend(solution=solution, cost=None)})
+    assert dedup_coin_spends == IdenticalSpendDedup(
+        {TEST_COIN_ID: DedupCoinSpend(solution=solution, cost=uint64(3600044))}
+    )
 
 
 def test_dedup_info_eligible_but_different_solution() -> None:
@@ -1646,7 +1634,7 @@ def test_dedup_info_eligible_but_different_solution() -> None:
         [ConditionOpcode.CREATE_COIN, IDENTITY_PUZZLE_HASH, TEST_COIN_AMOUNT],
     ]
     initial_solution = SerializedProgram.to(initial_conditions)
-    dedup_coin_spends = IdenticalSpendDedup({TEST_COIN_ID: DedupCoinSpend(solution=initial_solution, cost=None)})
+    dedup_coin_spends = IdenticalSpendDedup({TEST_COIN_ID: DedupCoinSpend(solution=initial_solution, cost=uint64(10))})
     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)
@@ -1663,7 +1651,9 @@ def test_dedup_info_eligible_2nd_time_and_another_1st_time() -> None:
         [ConditionOpcode.CREATE_COIN, IDENTITY_PUZZLE_HASH, TEST_COIN_AMOUNT - 1],
     ]
     initial_solution = SerializedProgram.to(initial_conditions)
-    dedup_coin_spends = IdenticalSpendDedup({TEST_COIN_ID: DedupCoinSpend(solution=initial_solution, cost=None)})
+    dedup_coin_spends = IdenticalSpendDedup(
+        {TEST_COIN_ID: DedupCoinSpend(solution=initial_solution, cost=uint64(1337))}
+    )
     sb1 = spend_bundle_from_conditions(initial_conditions, TEST_COIN)
     second_conditions = [[ConditionOpcode.CREATE_COIN, IDENTITY_PUZZLE_HASH, TEST_COIN_AMOUNT2]]
     second_solution = SerializedProgram.to(second_conditions)
@@ -1676,15 +1666,14 @@ def test_dedup_info_eligible_2nd_time_and_another_1st_time() -> None:
     )
     # Only the eligible one that we encountered more than once gets deduplicated
     assert unique_coin_spends == sb2.coin_spends
-    saved_cost = uint64(3600044)
-    assert cost_saving == saved_cost
+    assert cost_saving == uint64(1337)
     assert unique_additions == [Coin(TEST_COIN_ID2, IDENTITY_PUZZLE_HASH, TEST_COIN_AMOUNT2)]
     # The coin we encountered a second time has its cost and additions properly updated
     # The coin we encountered for the first time gets cost None and an empty set of additions
     expected_dedup_coin_spends = IdenticalSpendDedup(
         {
-            TEST_COIN_ID: DedupCoinSpend(solution=initial_solution, cost=saved_cost),
-            TEST_COIN_ID2: DedupCoinSpend(solution=second_solution, cost=None),
+            TEST_COIN_ID: DedupCoinSpend(solution=initial_solution, cost=uint64(1337)),
+            TEST_COIN_ID2: DedupCoinSpend(solution=second_solution, cost=uint64(1800044)),
         }
     )
     assert dedup_coin_spends == expected_dedup_coin_spends
@@ -1703,7 +1692,7 @@ def test_dedup_info_eligible_3rd_time_another_2nd_time_and_one_non_eligible() ->
     dedup_coin_spends = IdenticalSpendDedup(
         {
             TEST_COIN_ID: DedupCoinSpend(solution=initial_solution, cost=saved_cost),
-            TEST_COIN_ID2: DedupCoinSpend(solution=second_solution, cost=None),
+            TEST_COIN_ID2: DedupCoinSpend(solution=second_solution, cost=uint64(1337)),
         }
     )
     sb1 = spend_bundle_from_conditions(initial_conditions, TEST_COIN)
@@ -1723,7 +1712,7 @@ def test_dedup_info_eligible_3rd_time_another_2nd_time_and_one_non_eligible() ->
         bundle_coin_spends=mempool_item.bundle_coin_spends, max_cost=mempool_item.conds.cost
     )
     assert unique_coin_spends == sb3.coin_spends
-    saved_cost2 = uint64(1800044)
+    saved_cost2 = uint64(1337)
     assert cost_saving == saved_cost + saved_cost2
     assert unique_additions == [Coin(TEST_COIN_ID3, IDENTITY_PUZZLE_HASH, TEST_COIN_AMOUNT3)]
     expected_dedup_coin_spends = IdenticalSpendDedup(
@@ -1805,12 +1794,14 @@ async def test_bundle_coin_spends() -> None:
             eligible_for_dedup=False,
             eligible_for_fast_forward=False,
             additions=[Coin(coins[i].name(), IDENTITY_PUZZLE_HASH, coins[i].amount)],
+            cost=uint64(3000044),
         )
     assert mi123e.bundle_coin_spends[coins[3].name()] == BundleCoinSpend(
         coin_spend=eligible_sb.coin_spends[0],
         eligible_for_dedup=True,
         eligible_for_fast_forward=False,
         additions=[Coin(coins[3].name(), IDENTITY_PUZZLE_HASH, coins[3].amount)],
+        cost=uint64(1800044),
     )
 
 
@@ -2162,7 +2153,7 @@ async def get_coin_records(coin_ids: Collection[bytes32]) -> list[CoinRecord]:
         result = await add_spendbundle(mempool_manager, bundle, bundle_name)
         print(result)
         if expected is not None:
-            assert result == (None, MempoolInclusionStatus.FAILED, expected)
+            assert result == (0, MempoolInclusionStatus.FAILED, expected)
         else:
             assert result[0] is not None
             assert result[1] != MempoolInclusionStatus.FAILED
@@ -2453,7 +2444,7 @@ async def test_new_peak_ff_eviction(
 
     bundle_add_info = await mempool_manager.add_spend_bundle(
         bundle,
-        make_test_conds(spend_ids=[(singleton_spend.coin, ELIGIBLE_FOR_FF), (TEST_COIN, 0)], cost=1000000),
+        make_test_conds(spend_ids=[(singleton_spend.coin, ELIGIBLE_FOR_FF), (TEST_COIN, 0)], cost=uint64(1000000)),
         bundle.name(),
         first_added_height=uint32(1),
     )
@@ -2541,7 +2532,7 @@ async def test_multiple_ff(use_optimization: bool) -> None:
                 (singleton_spend2.coin, ELIGIBLE_FOR_FF),
                 (TEST_COIN, 0),
             ],
-            cost=1000000,
+            cost=uint64(1000000),
         ),
         bundle.name(),
         first_added_height=uint32(1),

chia/_tests/core/mempool/test_singleton_fast_forward.py

@@ -168,7 +168,7 @@ def test_perform_the_fast_forward() -> None:
         "517b0dadb0c310ded24dd86dff8205398080ff808080"
     )
     test_coin_spend = CoinSpend(test_coin, test_puzzle_reveal, test_solution)
-    test_spend_data = BundleCoinSpend(test_coin_spend, False, True, [test_child_coin])
+    test_spend_data = BundleCoinSpend(test_coin_spend, False, True, [test_child_coin], uint64(0))
     test_unspent_lineage_info = UnspentLineageInfo(
         coin_id=latest_unspent_coin.name(),
         parent_id=latest_unspent_coin.parent_coin_info,

chia/full_node/eligible_coin_spends.py

@@ -1,34 +1,22 @@
 from __future__ import annotations
 
 import dataclasses
-from typing import Optional
 
 from chia_rs import CoinSpend, ConsensusConstants, SpendBundle, fast_forward_singleton, get_conditions_from_spendbundle
 from chia_rs.sized_bytes import bytes32
 from chia_rs.sized_ints import uint32, uint64
 
-from chia.consensus.condition_costs import ConditionCost
 from chia.types.blockchain_format.coin import Coin
-from chia.types.blockchain_format.program import run_mempool_with_cost
 from chia.types.blockchain_format.serialized_program import SerializedProgram
 from chia.types.internal_mempool_item import InternalMempoolItem
 from chia.types.mempool_item import BundleCoinSpend, UnspentLineageInfo
 from chia.util.errors import Err
 
 
-def run_for_cost(
-    puzzle_reveal: SerializedProgram, solution: SerializedProgram, additions_count: int, max_cost: int
-) -> uint64:
-    create_coins_cost = additions_count * ConditionCost.CREATE_COIN.value
-    clvm_cost, _ = run_mempool_with_cost(puzzle_reveal, max_cost, solution)
-    saved_cost = uint64(clvm_cost + create_coins_cost)
-    return saved_cost
-
-
 @dataclasses.dataclass(frozen=True)
 class DedupCoinSpend:
     solution: SerializedProgram
-    cost: Optional[uint64]
+    cost: uint64
 
 
 def set_next_singleton_version(
@@ -165,13 +153,12 @@ def get_deduplication_info(
             if dedup_coin_spend is None:
                 # We didn't process an item with this coin before. If we end up including
                 # this item, add this pair to deduplication_spends
-                new_dedup_spends[coin_id] = DedupCoinSpend(spend_data.coin_spend.solution, None)
+                new_dedup_spends[coin_id] = DedupCoinSpend(spend_data.coin_spend.solution, spend_data.cost)
                 unique_coin_spends.append(spend_data.coin_spend)
                 unique_additions.extend(spend_data.additions)
                 continue
             # See if the solution was identical
-            current_solution, duplicate_cost = dataclasses.astuple(dedup_coin_spend)
-            if current_solution != spend_data.coin_spend.solution:
+            if dedup_coin_spend.solution != spend_data.coin_spend.solution:
                 # It wasn't, so let's skip this whole item because it's relying on
                 # spending this coin with a different solution and that would
                 # conflict with the coin spends that we're deduplicating already
@@ -180,30 +167,7 @@ def get_deduplication_info(
                 # solution we see from the relatively highest FPC item, to avoid
                 # severe performance and/or time-complexity impact
                 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
-                # This can happen if this item didn't get included in the previous block
-                spend_cost = spend_data.cost
-                if spend_cost is None:
-                    spend_cost = run_for_cost(
-                        puzzle_reveal=spend_data.coin_spend.puzzle_reveal,
-                        solution=spend_data.coin_spend.solution,
-                        additions_count=len(spend_data.additions),
-                        max_cost=max_cost,
-                    )
-                    # Update this mempool item's coin spends map
-                    bundle_coin_spends[coin_id] = BundleCoinSpend(
-                        coin_spend=spend_data.coin_spend,
-                        eligible_for_dedup=spend_data.eligible_for_dedup,
-                        eligible_for_fast_forward=spend_data.eligible_for_fast_forward,
-                        additions=spend_data.additions,
-                        cost=spend_cost,
-                    )
-                duplicate_cost = spend_cost
-                # If we end up including this item, update this entry's cost
-                new_dedup_spends[coin_id] = DedupCoinSpend(current_solution, duplicate_cost)
-            cost_saving += duplicate_cost
+            cost_saving += dedup_coin_spend.cost
         # Update the eligible coin spends data
         self.deduplication_spends.update(new_dedup_spends)
         return unique_coin_spends, uint64(cost_saving), unique_additions

chia/full_node/mempool_manager.py

@@ -124,7 +124,7 @@ def compute_assert_height(
 
 @dataclass
 class SpendBundleAddInfo:
-    cost: Optional[uint64]
+    cost: uint64
     status: MempoolInclusionStatus
     removals: list[MempoolRemoveInfo]
     error: Optional[Err]
@@ -545,7 +545,7 @@ async def add_spend_bundle(
             return SpendBundleAddInfo(item.cost, MempoolInclusionStatus.PENDING, [], err)
         else:
             # Cannot add to the mempool or pending pool.
-            return SpendBundleAddInfo(None, MempoolInclusionStatus.FAILED, [], err)
+            return SpendBundleAddInfo(uint64(0), MempoolInclusionStatus.FAILED, [], err)
 
     async def validate_spend_bundle(
         self,
@@ -625,6 +625,7 @@ async def validate_spend_bundle(
                 eligible_for_dedup=bool(spend_conds.flags & ELIGIBLE_FOR_DEDUP),
                 eligible_for_fast_forward=eligible_for_ff,
                 additions=spend_additions,
+                cost=uint64(spend_conds.condition_cost + spend_conds.execution_cost),
                 latest_singleton_lineage=lineage_info,
             )
 

chia/types/mempool_item.py

@@ -24,8 +24,9 @@ class BundleCoinSpend:
     eligible_for_dedup: bool
     eligible_for_fast_forward: bool
     additions: list[Coin]
-    # cost on the specific solution in this item
-    cost: Optional[uint64] = None
+    # cost on the specific solution in this item. The cost includes execution
+    # cost and conditions cost, not byte-cost.
+    cost: uint64
 
     # if this spend is eligible for fast forward, this may be set to the
     # current unspent lineage belonging to this singleton, that we would rebase