Skip to content

Conversation

arvidn
Copy link
Contributor

@arvidn arvidn commented Jul 14, 2025

Purpose:

The most recent chia_rs update included a feature where run_block_generator2() records the execution cost and condition cost per puzzle/spend. Use that cost in MempoolItem to estimate the cost savings when deduplicating spends (in the dedup feature).

Today we run the puzzle again in order to know the cost. This both simplifies and optimizes the DEDUP feature.

This is a step towards exploring using the "puzzle fingerprint" rather than solution bytes to compare equality of spends. i.e. to compare the condition outputs rather than the actual solution.

Current Behavior:

  • We use run_for_cost(), per puzzle, to compute the per-puzzle cost. Used in dedup.
  • The data structure supports optional cost, and computing it lazily

New Behavior:

  • We use just populate the MempoolItem's cost field directly from the result of validating the spend bundle.
  • The data structure has the cost as non-optional and is always available

@arvidn arvidn added the Changed Required label for PR that categorizes merge commit message as "Changed" for changelog label Jul 14, 2025
@arvidn arvidn force-pushed the per-puzzle-cost branch from 7ea6bb4 to eda986d Compare July 14, 2025 18:29
@arvidn arvidn marked this pull request as ready for review July 14, 2025 18:47
@arvidn arvidn requested a review from a team as a code owner July 14, 2025 18:47
@arvidn arvidn requested a review from AmineKhaldi July 14, 2025 18:47
Copy link

coveralls-official bot commented Jul 14, 2025

Pull Request Test Coverage Report for Build 16573949364

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 62 of 62 (100.0%) changed or added relevant lines in 5 files are covered.
  • 420 unchanged lines in 23 files lost coverage.
  • Overall coverage decreased (-0.05%) to 91.324%

Files with Coverage Reduction New Missed Lines %
chia/daemon/client.py 1 74.72%
chia/daemon/server.py 1 83.57%
chia/_tests/wallet/cat_wallet/test_cat_wallet.py 1 99.58%
chia/types/blockchain_format/program.py 1 91.67%
chia/consensus/pot_iterations.py 2 94.44%
chia/_tests/core/full_node/stores/test_block_store.py 2 99.53%
chia/timelord/timelord_launcher.py 2 90.71%
chia/wallet/transaction_record.py 2 95.69%
chia/wallet/wallet_state_manager.py 2 96.52%
chia/rpc/rpc_server.py 3 88.54%
Totals Coverage Status
Change from base Build 16332491766: -0.05%
Covered Lines: 101834
Relevant Lines: 111387

💛 - Coveralls

AmineKhaldi added a commit to AmineKhaldi/chia-blockchain that referenced this pull request Jul 15, 2025
Copy link
Contributor

@AmineKhaldi AmineKhaldi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Due to the amount of review suggestions I sent an addendum (PR #19816) to take care of all of them.

…x_cost parameter from get_deduplication_info()
@arvidn arvidn requested a review from AmineKhaldi July 15, 2025 17:14
@arvidn arvidn force-pushed the per-puzzle-cost branch from 5577dba to cbaed7a Compare July 17, 2025 21:47
Addendum to use per-puzzle cost to estimate DEDUP savings.
@arvidn arvidn requested a review from AmineKhaldi July 28, 2025 20:42
@arvidn arvidn added the ready_to_merge Submitter and reviewers think this is ready label Jul 29, 2025
@arvidn arvidn requested a review from wjblanke July 30, 2025 03:35
Copy link
Contributor

@wjblanke wjblanke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aok

@Starttoaster Starttoaster merged commit 33a0212 into main Jul 31, 2025
350 checks passed
@Starttoaster Starttoaster deleted the per-puzzle-cost branch July 31, 2025 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Changed Required label for PR that categorizes merge commit message as "Changed" for changelog ready_to_merge Submitter and reviewers think this is ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants