-
Notifications
You must be signed in to change notification settings - Fork 250
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
equihash: Fix some Rust API issues #1088
equihash: Fix some Rust API issues #1088
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are the major changes I've made to the code. The solution allocation type fix is required, everything else is optional.
(There are also a bunch of lints and comments that can be ignored, I'll clean them up when we get a valid solution from the code.)
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## equihash-solver-tromp #1088 +/- ##
=========================================================
+ Coverage 66.52% 66.71% +0.18%
=========================================================
Files 115 115
Lines 10881 10943 +62
=========================================================
+ Hits 7239 7301 +62
Misses 3642 3642 ☔ View full report in Codecov by Sentry. |
2510983
to
cc2f2d7
Compare
It looks like Windows and macOS need a portable Also Windows didn't like the thread support, which we're not using anyway, so I deleted it. |
@@ -30,23 +31,30 @@ extern "C" { | |||
fn equi_digiteven(eq: *mut CEqui, r: u32, id: u32); | |||
fn equi_digitK(eq: *mut CEqui, id: u32); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're not using threads, we could remove id
from all these functions and replace it with zero in C.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes generally LGTM. I'm going to merge this and then do some refactoring / squashing in #1083, to in particular ensure that this is also usable by zcashd
(so we can deduplicate the code therein).
This is a fully-functional draft, but it contains some extra lint handling and review comments.