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-200: Rust bindings #180

Merged
merged 37 commits into from
Aug 9, 2024
Merged

CHIA-200: Rust bindings #180

merged 37 commits into from
Aug 9, 2024

Conversation

Rigidity
Copy link
Contributor

No description provided.

Copy link

'This PR has been flagged as stale due to no activity for over 60
days. It will not be automatically closed, but it has been given
a stale-pr label and should be manually reviewed.'

@Rigidity Rigidity changed the title Draft bindings to chiavdf in Rust Rust bindings Jul 3, 2024
@Rigidity Rigidity changed the title Rust bindings CHIA-200: Rust bindings Jul 3, 2024
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 so far! I think it would be really good to run the fuzzers for 30 seconds on CI as well. like we do in chia_rs and clvm_rs

rust_bindings/src/lib.rs Show resolved Hide resolved
src/c_bindings/c_wrapper.cpp Outdated Show resolved Hide resolved
src/c_bindings/c_wrapper.cpp Outdated Show resolved Hide resolved
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.

Do we use MPIR for windows builds? I suspect the rust build logic might have to be a bit more complex to build chiavdf for windows. You do build chiavdf, right? You don't just link against a binary built with the existing build tools.

rust_bindings/build.rs Outdated Show resolved Hide resolved
.github/workflows/rust.yml Show resolved Hide resolved
.github/workflows/rust.yml Show resolved Hide resolved
@Rigidity Rigidity marked this pull request as ready for review July 15, 2024 03:33
@Rigidity
Copy link
Contributor Author

looks good so far! I think it would be really good to run the fuzzers for 30 seconds on CI as well. like we do in chia_rs and clvm_rs

I changed it to 30 instead of 20 (I think that's seconds in the workflow?)

@Rigidity
Copy link
Contributor Author

Do we use MPIR for windows builds? I suspect the rust build logic might have to be a bit more complex to build chiavdf for windows. You do build chiavdf, right? You don't just link against a binary built with the existing build tools.

Yes, it builds chiavdf via CMake using our existing configuration within the build.rs file. It should work on windows since we use it in chia-blockchain via Python bindings today.

@Rigidity Rigidity requested a review from arvidn July 15, 2024 03:35
@github-actions github-actions bot removed the stale-pr label Jul 15, 2024
rust_bindings/src/lib.rs Outdated Show resolved Hide resolved
rust_bindings/src/lib.rs Outdated Show resolved Hide resolved
src/c_bindings/c_wrapper.cpp Outdated Show resolved Hide resolved
src/c_bindings/c_wrapper.cpp Outdated Show resolved Hide resolved
src/c_bindings/c_wrapper.cpp Outdated Show resolved Hide resolved
src/c_bindings/c_wrapper.cpp Outdated Show resolved Hide resolved
src/c_bindings/c_wrapper.cpp Outdated Show resolved Hide resolved
rust_bindings/src/lib.rs Outdated Show resolved Hide resolved
@Rigidity Rigidity requested a review from arvidn August 5, 2024 17:14
src/c_bindings/c_wrapper.cpp Outdated Show resolved Hide resolved
src/c_bindings/c_wrapper.cpp Outdated Show resolved Hide resolved
src/c_bindings/c_wrapper.cpp Outdated Show resolved Hide resolved
rust_bindings/src/lib.rs Outdated Show resolved Hide resolved
@Rigidity Rigidity requested a review from arvidn August 8, 2024 15:13
arvidn
arvidn previously approved these changes Aug 9, 2024
@Rigidity Rigidity merged commit 7d737f5 into main Aug 9, 2024
53 checks passed
@Rigidity Rigidity deleted the rust-bindings branch August 9, 2024 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants