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

use rust types for slots, SubEpochSummary and SubEpochData #17298

Merged
merged 3 commits into from
Jan 16, 2024

Conversation

arvidn
Copy link
Contributor

@arvidn arvidn commented Jan 11, 2024

These commits are best reviewed one at a time.

Purpose:

This PR is another part of the long-term effort of transitioning FullBlock to the rust type. This improves serialization/deserialization speed as well as allow more of the validation to live entirely on the rust side.

Some of the performance issues are highlighted here.

This PR is especially targeting serializing and deserializing of SubEpochChallengeSegments.
image

This PR transitions the following types to rust:

  • ChallengeBlockInfo
  • ChallengeChainSubSlot
  • InfusedChallengeChainSubSlot
  • RewardChainSubSlot
  • SubSlotProofs
  • SubEpochSummary
  • EndOfSubSlotBundle
  • SubEpochData
  • SubEpochChallengeSegment
  • SubEpochSegments
  • SubSlotData

The rust definition of these types can be found here, here, here and here.

As with earlier types being moved to rust, the integer types are exposed as plain int to python, even though the underlying types are fixed width. This allows mypy to highlight all the places where fields need to be "cast" (or really, where our fixed-width integer type needs to be constructed).

A utility function on StructStream (the base class for our fixed-width integers) was added in the previous PR, construct_optional() which essentially casts an Optional[int] -> Optional[<StructStream>]. This is used for optional members of the rust types, which are exposed as Optional[int] even though the underlying value is restricted by the width of the field.

Since native types implemented in rust cannot (at least to my knowlege) be dataclasses, the free function dataclasses.replace() has to be replaced by a member function replace(), which our rust classes implement.

Current Behavior:

Serializing weight proof messages and deserializing SubEpochChallengeSegments is slow and may block other (more time critical tasks).

New Behavior:

Serializing weight proof messages and deserializing SubEpochChallengeSegments is fast.

@arvidn arvidn added the Changed Required label for PR that categorizes merge commit message as "Changed" for changelog label Jan 11, 2024
Copy link

Pull Request Test Coverage Report for Build 7489697254

Warning: This coverage report may be inaccurate.

We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
To ensure accuracy in future PRs, please see these guidelines.
A quick fix for this PR: rebase it; your next report should be accurate.

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.1%) to 90.496%

Totals Coverage Status
Change from base Build 7481196968: -0.1%
Covered Lines: 94420
Relevant Lines: 104290

💛 - Coveralls

Copy link
Contributor

File Coverage Missing Lines
chia/full_node/full_node_api.py 0.0% lines 883, 885
chia/timelord/timelord_state.py 0.0% lines 90, 93-94
Total Missing Coverage
52 lines 5 lines 90%

@arvidn arvidn marked this pull request as ready for review January 12, 2024 12:29
@arvidn arvidn requested a review from a team as a code owner January 12, 2024 12:29
@arvidn arvidn requested a review from altendky January 16, 2024 10:45
@cmmarslender cmmarslender merged commit c6d5f7f into main Jan 16, 2024
263 of 264 checks passed
@cmmarslender cmmarslender deleted the rust-sub-epoch branch January 16, 2024 17:04
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.

None yet

4 participants