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-1291 Switch mempool TX prevalidation to the Rust version #18557

Merged

Conversation

AmineKhaldi
Copy link
Contributor

Purpose:

Leverage the Rust version of validate_clvm_and_signature with the added benefit of being able to use ThreadPools instead of ProcessPools.

Current Behavior:

Mempool item prevalidation currently happens with a Python version of validate_clvm_and_signature through ProcessPools.

New Behavior:

Mempool item prevalidation happens with a Rust version of validate_clvm_and_signature through ThreadPools.

@AmineKhaldi AmineKhaldi added the Changed Required label for PR that categorizes merge commit message as "Changed" for changelog label Sep 5, 2024
@AmineKhaldi AmineKhaldi self-assigned this Sep 5, 2024
@AmineKhaldi AmineKhaldi marked this pull request as ready for review September 5, 2024 11:30
@AmineKhaldi AmineKhaldi requested a review from a team as a code owner September 5, 2024 11:30
@arvidn
Copy link
Contributor

arvidn commented Sep 5, 2024

Missing coverage:

chia/full_node/mempool_manager.py (91.7%): Missing lines 288
chia/types/eligible_coin_spends.py (54.5%): Missing lines 342-345,347

Are these OK Amine?

arvidn
arvidn previously approved these changes Sep 5, 2024
@AmineKhaldi
Copy link
Contributor Author

AmineKhaldi commented Sep 5, 2024

Missing coverage:

chia/full_node/mempool_manager.py (91.7%): Missing lines 288
chia/types/eligible_coin_spends.py (54.5%): Missing lines 342-345,347

Are these OK Amine?

Yes:

  • For
    chia/full_node/mempool_manager.py:288 and chia/types/eligible_coin_spends.py:347 these are just defensive. I documented the failure path and marked these with the no cover pragma.

  • For chia/types/eligible_coin_spends.py:342-345 it's existing logic, and I didn't mark it with the no cover pragma because this can technically happen, we just don't currently have a realistic test scenario that we can add, where a singleton would be somehow broken after it gets fast forwarded. This is therefore fine by me to continue exempting.

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants