-
Notifications
You must be signed in to change notification settings - Fork 13
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
Chain TryFrom
errors rather than calling unwrap
on them
#91
base: main
Are you sure you want to change the base?
Conversation
@@ -1,6 +1,6 @@ | |||
[package] | |||
name = "wsts" | |||
version = "9.1.0" | |||
version = "10.0.0" |
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.
If we're going to bump the version, should we remove that unused error variant?
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.
Yeah we can go wild here.
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.
Done
/// threshold, where the degree of each poly is (t-1) | ||
t: u32, | ||
t: usize, |
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.
Curious, usize
instead of u64
or u32
? Same question for neg_shares
.
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.
The goal was to get rid of the conversions inside the C callback, since we can’t return errors from there.
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.
Oh wow get_point
still has an as
cast 😮
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.
Ahhh I see. Got it.
We use usize
because we often feed things into functions in the p256k1
library. That library wraps the libsecp256k1 C library via the rust-bindgen crate. And bindgen assumes size_t
is usize
(which is the default). We can change that default but keeping things as is allows p256k1
to be used on rust targets where the pointer size is not 64-bits.
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.
Yeah we need to do another pass on usize vs explicitly sized types. Anything that doesn’t go over the network is probably better s as usize.
… BadPolyCommitmentLen
Can we also replace the |
Yeah. I had aspirations of making this |
Ahh I see. Yeah, I was wondering why that was used. Well, if that ends up being a goal again we can change it back or something. |
Fixes #89