Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CHIA-2381 Validate fast forward spends before adding their spend bundle to the mempool #19272

Conversation

AmineKhaldi
Copy link
Contributor

Purpose:

Make sure that all fast forward spends of a spend bundle would still have unspent coins.

Current Behavior:

We can accept a spend bundle with fast forward spends that do not currently have latest unspent coins.

New Behavior:

We reject spend bundles like that with DOUBLE_SPEND error.

@AmineKhaldi AmineKhaldi added the Changed Required label for PR that categorizes merge commit message as "Changed" for changelog label Feb 15, 2025
@AmineKhaldi AmineKhaldi self-assigned this Feb 15, 2025
@AmineKhaldi AmineKhaldi changed the title Validate fast forward spends before adding their spend bundle to the mempool CHIA-2381 Validate fast forward spends before adding their spend bundle to the mempool Feb 15, 2025
@AmineKhaldi AmineKhaldi marked this pull request as ready for review February 15, 2025 20:36
@AmineKhaldi AmineKhaldi requested a review from a team as a code owner February 15, 2025 20:36
@AmineKhaldi AmineKhaldi force-pushed the ensure_ff_spends_have_latest_unspent branch from 01f2443 to 313c6e4 Compare February 15, 2025 20:57
Copy link
Contributor

@arvidn arvidn left a comment

Choose a reason for hiding this comment

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

my main comment is around the relationship of is_eligible_for_ff and ff_puzzle_hash. It seems like it could be stricter, or maybe merged into a single field

chia/types/eligible_coin_spends.py Show resolved Hide resolved
chia/full_node/mempool_manager.py Outdated Show resolved Hide resolved
chia/full_node/mempool_manager.py Outdated Show resolved Hide resolved
chia/full_node/mempool_manager.py Outdated Show resolved Hide resolved
@AmineKhaldi AmineKhaldi force-pushed the ensure_ff_spends_have_latest_unspent branch from 313c6e4 to 81b6e5d Compare February 16, 2025 10:55
@AmineKhaldi AmineKhaldi force-pushed the ensure_ff_spends_have_latest_unspent branch from 81b6e5d to 51339fb Compare February 16, 2025 13:00
@AmineKhaldi AmineKhaldi force-pushed the ensure_ff_spends_have_latest_unspent branch from 51339fb to 1639fdd Compare February 16, 2025 20:14
arvidn
arvidn previously approved these changes Feb 17, 2025
@arvidn arvidn added the ready_to_merge Submitter and reviewers think this is ready label Feb 17, 2025
emlowe
emlowe previously approved these changes Feb 17, 2025
@pmaslana pmaslana closed this Feb 17, 2025
@pmaslana pmaslana reopened this Feb 17, 2025
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the merge_conflict Branch has conflicts that prevent merge to main label Feb 17, 2025
…mempool.

Make sure that all fast forward spends of a spend bundle would still have unspent coins.
@AmineKhaldi AmineKhaldi dismissed stale reviews from emlowe and arvidn via a6758ca February 17, 2025 18:59
@AmineKhaldi AmineKhaldi force-pushed the ensure_ff_spends_have_latest_unspent branch from 1639fdd to a6758ca Compare February 17, 2025 18:59
@github-actions github-actions bot removed the merge_conflict Branch has conflicts that prevent merge to main label Feb 17, 2025
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

@pmaslana pmaslana merged commit de0b0d8 into Chia-Network:release/2.5.2 Feb 17, 2025
861 checks passed
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