-
Notifications
You must be signed in to change notification settings - Fork 220
Initial Rust no_std support. #153
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
Closed
Closed
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
dd2b5c1
bindings/rust/src/lib.rs: mask std-dependent functions and engage no_…
dot-asm 8a7e760
bindings/rust: add pippenger-no_std.rs [and deduplicate tests].
dot-asm 8b45986
bindings/rust/build.rs: refine no_std detection.
dot-asm 3289650
.github/workflows/ci.yml: exercise no-std [with limited stack size].
dot-asm File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,146 @@ | ||
| // Copyright Supranational LLC | ||
| // Licensed under the Apache License, Version 2.0, see LICENSE for details. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| use core::ops::{Index, IndexMut}; | ||
| use core::slice::SliceIndex; | ||
|
|
||
| macro_rules! pippenger_mult_impl { | ||
| ( | ||
| $points:ident, | ||
| $point:ty, | ||
| $point_affine:ty, | ||
| $to_affines:ident, | ||
| $scratch_sizeof:ident, | ||
| $multi_scalar_mult:ident, | ||
| $tile_mult:ident, | ||
| $add_or_double:ident, | ||
| $double:ident, | ||
| $test_mod:ident, | ||
| $generator:ident, | ||
| $mult:ident, | ||
| $add:ident, | ||
| ) => { | ||
| pub struct $points { | ||
| points: Vec<$point_affine>, | ||
| } | ||
|
|
||
| impl<I: SliceIndex<[$point_affine]>> Index<I> for $points { | ||
| type Output = I::Output; | ||
|
|
||
| #[inline] | ||
| fn index(&self, i: I) -> &Self::Output { | ||
| &self.points[i] | ||
| } | ||
| } | ||
| impl<I: SliceIndex<[$point_affine]>> IndexMut<I> for $points { | ||
| #[inline] | ||
| fn index_mut(&mut self, i: I) -> &mut Self::Output { | ||
| &mut self.points[i] | ||
| } | ||
| } | ||
|
|
||
| impl $points { | ||
| #[inline] | ||
| pub fn as_slice(&self) -> &[$point_affine] { | ||
| self.points.as_slice() | ||
| } | ||
|
|
||
| pub fn from(points: &[$point]) -> Self { | ||
| let npoints = points.len(); | ||
| let mut ret = Self { | ||
| points: Vec::with_capacity(npoints), | ||
| }; | ||
| unsafe { ret.points.set_len(npoints) }; | ||
|
|
||
| let p: [*const $point; 2] = [&points[0], ptr::null()]; | ||
| unsafe { $to_affines(&mut ret.points[0], &p[0], npoints) }; | ||
| ret | ||
| } | ||
|
|
||
| pub fn mult(&self, scalars: &[u8], nbits: usize) -> $point { | ||
| let npoints = self.points.len(); | ||
| let nbytes = (nbits + 7) / 8; | ||
|
|
||
| if scalars.len() < nbytes * npoints { | ||
| panic!("scalars length mismatch"); | ||
| } | ||
|
|
||
| let p: [*const $point_affine; 2] = | ||
| [&self.points[0], ptr::null()]; | ||
| let s: [*const u8; 2] = [&scalars[0], ptr::null()]; | ||
|
|
||
| let mut ret = <$point>::default(); | ||
| unsafe { | ||
| let mut scratch: Vec<u64> = | ||
| Vec::with_capacity($scratch_sizeof(npoints) / 8); | ||
| scratch.set_len(scratch.capacity()); | ||
| $multi_scalar_mult( | ||
| &mut ret, | ||
| &p[0], | ||
| npoints, | ||
| &s[0], | ||
| nbits, | ||
| &mut scratch[0], | ||
| ); | ||
| } | ||
| ret | ||
| } | ||
|
|
||
| pub fn add(&self) -> $point { | ||
| let npoints = self.points.len(); | ||
|
|
||
| let p: [*const _; 2] = [&self.points[0], ptr::null()]; | ||
| let mut ret = <$point>::default(); | ||
| unsafe { $add(&mut ret, &p[0], npoints) }; | ||
|
|
||
| ret | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| pippenger_test_mod!( | ||
| $test_mod, | ||
| $points, | ||
| $point, | ||
| $add_or_double, | ||
| $generator, | ||
| $mult, | ||
| ); | ||
| }; | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| include!("pippenger-test_mod.rs"); | ||
|
|
||
| pippenger_mult_impl!( | ||
| p1_affines, | ||
| blst_p1, | ||
| blst_p1_affine, | ||
| blst_p1s_to_affine, | ||
| blst_p1s_mult_pippenger_scratch_sizeof, | ||
| blst_p1s_mult_pippenger, | ||
| blst_p1s_tile_pippenger, | ||
| blst_p1_add_or_double, | ||
| blst_p1_double, | ||
| p1_multi_scalar, | ||
| blst_p1_generator, | ||
| blst_p1_mult, | ||
| blst_p1s_add, | ||
| ); | ||
|
|
||
| pippenger_mult_impl!( | ||
| p2_affines, | ||
| blst_p2, | ||
| blst_p2_affine, | ||
| blst_p2s_to_affine, | ||
| blst_p2s_mult_pippenger_scratch_sizeof, | ||
| blst_p2s_mult_pippenger, | ||
| blst_p2s_tile_pippenger, | ||
| blst_p2_add_or_double, | ||
| blst_p2_double, | ||
| p2_multi_scalar, | ||
| blst_p2_generator, | ||
| blst_p2_mult, | ||
| blst_p2s_add, | ||
| ); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
You should define features in
Cargo.toml. Even though it will work like that, it is not easily discoverable.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.
It's a conscious choice. There is no reason to make it discoverable. If user attempts to compile on a bare-metal platform it's simply engaged in the build script.
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.
But why should
blstbe unlike any other ordinary Rust crate? There is an established pattern in the ecosystem how this things are done, what is the point of deviating from it?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.
Without having this feature defined in
Cargo.tomland set by default you essentially compileno_stdversion for ALL PLATFORMS. Users will have to setstdfeature to avoid this, but due to "conscious choice" they will have a hard time knowing it exists in the first place.You can probably define it from within
build.rsbut you've literally fighting the best practices in the ecosystem for the sake of confusing users.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.
No. build.rs enables
stdfeature on ALL PLATFORMS but bare-metal ones.No. Users won't have to do anything. Nor would they have to do anything special when using blst as dependency on a bare-metal platform.
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.
And they can do that. Nobody actually did to my knowledge, defaulting to multi-threading appears to be generally appreciated.
This is not true. blst uses a private fixed-sized threadpool.
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.
What I'm trying to say is
blstcan't possibly know and doesn't really need to know what works and what doesn't. I have given a few examples like support for threads where you think there isn't such support and forcingstdon users that can, but don't want to depend on it. Both of those are not possible to achieve with current design of the library.Simply offering two features enabled by default (
stdandthreads) allows any imaginable supported platform will work under full control of the developer using the library.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.
And what I'm saying is that if we don't know something, we make no assumptions and don't really want users to make such assumptions either. [As effectively reflected in README.md.] But as already said, it's not right spot to discuss this. Why-not-this-way is off-topic in this specific context. Open an issue or choose another channel to communicate with supranational.
Uh oh!
There was an error while loading. Please reload this page.
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.
For the record (just to volunteer relevant info, I don’t have a horse in this race), MystenLabs/fastcrypto#176
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.
I was thinking to replace manual thread pool management with rayon, which inherits thread pool from outer context and that way you can change number of threads from the outside (or not bother and use global default thread pool), but didn't have time to do the change and bench it.