Skip to content

Conversation

arvidn
Copy link
Contributor

@arvidn arvidn commented Jul 10, 2025

Purpose:

This simplification is in preparation for another improvement to the mempool dedup feature.
We have the same list of CoinSpends in two different places, once in SpendBundle and once in the result from running the generator SpendBundleConditions. We need to merge the information from both of these (and ensure that they match each other).

Current Behavior:

While iterating over SpendBundleConditions, build a new dictionary mapping coin ID to EligibilityAndAdditions.
Then iterate over SpendBundle and perform lookups in the dictionary.

New Behavior:

Build a map of coin ID -> SpendCondition.
While iterating over SpendBundle, look up the corresponding condition from the map.
This moves all logic into a single loop.

@arvidn arvidn requested a review from a team as a code owner July 10, 2025 13:29
@arvidn arvidn changed the title simplify MempoolManager by removing EligibilityAndAdditions [CHIA-2827] simplify MempoolManager by removing EligibilityAndAdditions Jul 10, 2025
@arvidn arvidn added Changed Required label for PR that categorizes merge commit message as "Changed" for changelog Exclude_Notes Use this label if the changes in the PR should be excluded from the release notes labels Jul 10, 2025
@arvidn arvidn requested a review from AmineKhaldi July 10, 2025 13:34
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, left some suggestions.

Copy link

coveralls-official bot commented Jul 10, 2025

Pull Request Test Coverage Report for Build 16227671941

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

  • 22 of 22 (100.0%) changed or added relevant lines in 2 files are covered.
  • 198 unchanged lines in 22 files lost coverage.
  • Overall coverage decreased (-0.06%) to 91.353%

Files with Coverage Reduction New Missed Lines %
chia/_tests/core/mempool/test_mempool_manager.py 1 99.93%
chia/_tests/wallet/cat_wallet/test_trades.py 1 99.8%
chia/wallet/cat_wallet/cat_outer_puzzle.py 1 98.81%
chia/wallet/nft_wallet/metadata_outer_puzzle.py 1 97.14%
chia/wallet/nft_wallet/singleton_outer_puzzle.py 1 98.77%
chia/_tests/wallet/cat_wallet/test_cat_wallet.py 2 99.58%
chia/wallet/vc_wallet/vc_drivers.py 2 99.23%
chia/wallet/nft_wallet/transfer_program_puzzle.py 3 88.89%
chia/full_node/weight_proof.py 4 90.6%
chia/_tests/core/full_node/test_full_node.py 4 98.48%
Totals Coverage Status
Change from base Build 16184231739: -0.06%
Covered Lines: 102351
Relevant Lines: 111916

💛 - Coveralls

@arvidn arvidn requested a review from AmineKhaldi July 10, 2025 14:59
@arvidn arvidn requested a review from wjblanke July 11, 2025 22:08
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

@cmmarslender cmmarslender merged commit 46bc14b into main Jul 14, 2025
519 of 520 checks passed
@cmmarslender cmmarslender deleted the eligibility-and-additions branch July 14, 2025 13:52
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 Exclude_Notes Use this label if the changes in the PR should be excluded from the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants