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 implementation of SerializedProgram #17297

Merged
merged 1 commit into from Jan 19, 2024

Conversation

arvidn
Copy link
Contributor

@arvidn arvidn commented Jan 11, 2024

Purpose:

This replaces the SerializedProgram class with the counterpart from chia_rs. The long-term objective is to make FullBlock able to live entirely in rust, to move validation to threads. The transactions_generator field is a serialized CLVM program.

The main challenge in the python binding for this class, is how python arguments are converted into CLVM structures before invoking the program (e.g. run_with_cost()). This conversion is partly implemented in _serialize(), here as well as SExp.to, in the clvm wheel.

Note how there's a special case when passing a SerializedProgram, its bytes are used directly. i.e. a serialized program is passed as the CLVM structure it represents, not as a blob of bytes (i.e. a single atom). This special handling no longer exists when entering the SExp.to() path.

The rust implementation of this behavior can be found here (the _serialize() counterpart) and here (the SExp.to() counterpart).

Current Behavior:

SerializedProgram is implemented in python (but CLVM is run by rust)

New Behavior:

SerializedProgram is implemented in rust

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

Pull Request Test Coverage Report for Build 7554530142

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

Totals Coverage Status
Change from base Build 7549452453: -0.01%
Covered Lines: 94735
Relevant Lines: 104543

💛 - Coveralls

@arvidn arvidn marked this pull request as ready for review January 17, 2024 17:45
@arvidn arvidn requested a review from a team as a code owner January 17, 2024 17:45
Copy link
Contributor

@Rigidity Rigidity left a comment

Choose a reason for hiding this comment

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

Love to see it

@cmmarslender cmmarslender merged commit 42762bc into main Jan 19, 2024
267 checks passed
@cmmarslender cmmarslender deleted the rust-serialized-program branch January 19, 2024 17:38
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