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.

Sign up for GitHub

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

Jump to bottom

CHIA-193: Rust bindings to chiapos #439

Merged
merged 25 commits into from
Aug 15, 2024
Merged

CHIA-193: Rust bindings to chiapos #439

merged 25 commits into from
Aug 15, 2024

Conversation

Rigidity
Copy link
Contributor

@Rigidity Rigidity commented Aug 12, 2024 •
edited
Loading

Adds a Rust crate with a memory safe (at least, according to fuzz tests and unit tests I've written) binding to validate_proof. This is part of an effort to port block prevalidation to Rust.

@Rigidity Rigidity marked this pull request as draft August 12, 2024 17:24
@coveralls-official Coveralls Official
Copy link

coveralls-official bot commented Aug 12, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 10407235767

Details

  • 16 of 16 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.08%) to 79.028%
Totals Coverage Status
Change from base Build 9946993550: 0.08%
Covered Lines: 3497
Relevant Lines: 4425
💛 - Coveralls

@Rigidity Rigidity marked this pull request as ready for review August 13, 2024 20:06
@Rigidity Rigidity changed the title Initial draft Rust binding Rust bindings to chiapos Aug 13, 2024
@Rigidity Rigidity changed the title Rust bindings to chiapos CHIA-193: Rust bindings to chiapos Aug 13, 2024
@Rigidity Rigidity requested review from arvidn and haorldbchi August 13, 2024 20:12
arvidn
arvidn reviewed Aug 14, 2024
View reviewed changes
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.

looks good, just a few comments

CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
c-bindings/wrapper.cpp Outdated Show resolved Hide resolved
c-bindings/wrapper.h Outdated Show resolved Hide resolved
c-bindings/wrapper.h Outdated Show resolved Hide resolved
@Rigidity Rigidity requested a review from arvidn August 14, 2024 12:53
arvidn
arvidn reviewed Aug 14, 2024
View reviewed changes
c-bindings/wrapper.cpp Outdated Show resolved Hide resolved
c-bindings/wrapper.h Outdated Show resolved Hide resolved
rust-bindings/src/lib.rs Outdated Show resolved Hide resolved
arvidn
arvidn previously approved these changes Aug 14, 2024
View reviewed changes
@Rigidity Rigidity requested a review from arvidn August 15, 2024 09:24
arvidn
arvidn reviewed Aug 15, 2024
View reviewed changes
rust-bindings/src/lib.rs Outdated Show resolved Hide resolved
rust-bindings/src/lib.rs Show resolved Hide resolved
rust-bindings/test_proofs.txt Show resolved Hide resolved
@Rigidity Rigidity requested a review from arvidn August 15, 2024 09:47
arvidn
arvidn reviewed Aug 15, 2024
View reviewed changes
tests/test.cpp Outdated Show resolved Hide resolved
@Rigidity Rigidity requested a review from arvidn August 15, 2024 10:12
arvidn
arvidn reviewed Aug 15, 2024
View reviewed changes
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.

looks good to me. do you want Harold to have time to review it as well?

@Rigidity
Copy link
Contributor Author

looks good to me. do you want Harold to have time to review it as well?

Sounds like a good idea if he's able to, I'm not in a rush to release this but this week would be nice

haorldbchi
haorldbchi previously requested changes Aug 15, 2024
View reviewed changes
tools/generate_rust_proof_test.py Show resolved Hide resolved
rust-bindings/src/lib.rs Outdated Show resolved Hide resolved
c-bindings/wrapper.cpp Outdated Show resolved Hide resolved
c-bindings/wrapper.h Outdated Show resolved Hide resolved
@Rigidity Rigidity requested review from arvidn and haorldbchi August 15, 2024 16:47
@Rigidity Rigidity dismissed haorldbchi’s stale review August 15, 2024 16:49

The comments are implemented & Harold is fine with me resolving them

arvidn
arvidn reviewed Aug 15, 2024
View reviewed changes
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.

good call on the GetSize() check

c-bindings/wrapper.cpp Show resolved Hide resolved
@Rigidity Rigidity merged commit 84a28b7 into main Aug 15, 2024
49 checks passed
@Rigidity Rigidity deleted the rust-binding branch August 15, 2024 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@harold-b harold-b harold-b left review comments

@haorldbchi haorldbchi haorldbchi approved these changes

@arvidn arvidn arvidn approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Rigidity @arvidn @harold-b @haorldbchi