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-1101: update BlockGenerator type #18508

Merged
merged 1 commit into from
Aug 22, 2024
Merged

CHIA-1101: update BlockGenerator type #18508

merged 1 commit into from
Aug 22, 2024

Conversation

arvidn
Copy link
Contributor

@arvidn arvidn commented Aug 21, 2024

Purpose:

Simplify and optimize code around block validation.

The BlockGenerator class contains the block generator for a specific block as well as the actual generators for blocks it references.

This PR:

  • change the type of generator_refs to List[bytes] instead of List[SerializedProgram]. The only place these items are used are in generators, in CLVM. We always pass plain bytes into CLVM.
  • Avoid passing the block generator twice to the worker process for validation. Currently we pass both the FullBLock and the BlockGenerator. By only passing the previous generators (List[bytes]) instead of BlockGenerator, we avoid copying the generator twice.

Current Behavior:

Pass 2 copies of the block generator to pre_validate_blocks_multiprocessing(), once in the FullBlock and once in the BlockGenerator

New Behavior:

Only pass the block generator once to pre_validate_blocks_multiprocessing(), in the FullBlock. Instead of the BlockGenerator just pass the generator_refs.

@arvidn arvidn added the Changed Required label for PR that categorizes merge commit message as "Changed" for changelog label Aug 21, 2024
@arvidn arvidn marked this pull request as ready for review August 21, 2024 11:21
@arvidn arvidn requested a review from a team as a code owner August 21, 2024 11:21
…Program]. Also, avoid passing the block generator twice to the worker process for validation
@arvidn
Copy link
Contributor Author

arvidn commented Aug 21, 2024

The performance improvement is found in chia/consensus/multiprocess_validation.py. Otherwise there's a mixture of updates to tests with some more simplifications in chia/full_node/mempool_check_conditions.py

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

@arvidn arvidn added the ready_to_merge Submitter and reviewers think this is ready label Aug 22, 2024
@pmaslana pmaslana merged commit d1844b6 into main Aug 22, 2024
375 checks passed
@pmaslana pmaslana deleted the block-generator-class branch August 22, 2024 17:08
@altendky altendky mentioned this pull request Sep 3, 2024
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