Initial Rust no_std support. by dot-asm · Pull Request #153 · supranational/blst

dot-asm · 2023-03-10T19:31:27Z

No description provided.

nazar-pc · 2023-03-10T19:42:54Z

bindings/rust/src/lib.rs

 // Licensed under the Apache License, Version 2.0, see LICENSE for details.
 // SPDX-License-Identifier: Apache-2.0

+#![cfg_attr(not(feature = "std"), no_std)]


You should define features in Cargo.toml. Even though it will work like that, it is not easily discoverable.

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.

But why should blst be 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?

Without having this feature defined in Cargo.toml and set by default you essentially compile no_std version for ALL PLATFORMS. Users will have to set std feature 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.rs but you've literally fighting the best practices in the ecosystem for the sake of confusing users.

you essentially compile no_std version for ALL PLATFORMS.

No. build.rs enables std feature on ALL PLATFORMS but bare-metal ones.

Users will have to set std feature to avoid this,

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.

They may want to have no threads either because for their use case threading makes more sense on higher level

And they can do that. Nobody actually did to my knowledge, defaulting to multi-threading appears to be generally appreciated.

and blst would just cause quadratic number of threads only making things worse.

This is not true. blst uses a private fixed-sized threadpool.

blst is about what actually works, not what might work.

What I'm trying to say is blst can'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 forcing std on 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 (std and threads) allows any imaginable supported platform will work under full control of the developer using the library.

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.

They may want to have no threads either because for their use case threading makes more sense on higher level

And they can do that. Nobody actually did to my knowledge, defaulting to multi-threading appears to be generally appreciated.

For the record (just to volunteer relevant info, I don’t have a horse in this race), MystenLabs/fastcrypto#176

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.

nazar-pc · 2023-03-10T19:43:34Z

.github/workflows/ci.yml

        fi
        cargo test --release
-        cargo test --release --features=no-threads
+        env BLST_TEST_NO_STD= cargo test --release


This is not how Rust ecosystem does it. It should be controlled by features (there is --no-default-features for instance). See #150 on how to do it in idiomatic way.

This is not how Rust ecosystem does it.

I don't really care. I want to exercise the no_std build on an std platform without offering an idiomatic way.

What is the point? It just confuses the heck out of people.

I get that in other languages that might be a norm, but in Rust there are conventions that make like so much easier, I don't quite understand the rationale behind deliberately making it harder instead 😕

IMHO Rust API should look like idiomatic Rust, Golang API should look like idiomatic Golang and so on for every language to the degree possible.

What is the point?

??? It says just one line above, "exercise the no_std build on an std platform." It's not for users, but for us as a way to ensure that no_std doesn't go under the CI radar unchecked.

My question was about your comment. What is the point of intentionally doing non-idiomatic way with environment variables and other trickery if you can just use idiomatic --no-default-features like most other projects do.

We plan to use this package in Subspace and I care about the quality of Rust binding and willing to help, so seeing deliberately non-idiomatic code is confusing to me and doesn't make sense.

you can just use idiomatic --no-default-features

To do what? To run the test. But what do I tell no_std, a.k.a. bare-metal platform users who want to use blst as dependency? Or let me rephrase. Provided that I don't actually have to tell them anything special, why would I want to?

We plan to use this package in Subspace

Just in case, quoting README.md, "Problem reports for these [non-x86_64 and ARM64 hardware platforms] will be considered and are likely to be addressed."

To do what? To run the test. But what do I tell no_std, a.k.a. bare-metal platform users who want to use blst as dependency? Or let me rephrase. Provided that I don't actually have to tell them anything special, why would I want to?

Because it is up to the user whether they want to use std or not (it is not free, they might run on a platform where they don't want to pull std even if they technically could from CPU architecture point of view). This might include use cases like building a static library. By forcing decisions that you don't want to tell and don't want to expose you're locking them out of ability to use the library like they use any other idiomatic Rust library.

Just in case, quoting README.md, "Problem reports for these [non-x86_64 and ARM64 hardware platforms] will be considered and are likely to be addressed."

It is certainly appreciated, but my point is that if you create a typical idiomatic Rust crate you don't even need to have that in the readme. The beauty of Rust ecosystem is that most of the packages follow the same versioning, naming and features conventions and everyone knows what it is and how to use it. By default. No extra docs necessary.

nazar-pc · 2023-03-16T14:32:06Z

bindings/rust/src/pippenger-no_std.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,
+);


These should really be generics and type aliases for convenience rather than macros. Macros destroy IDE support and make it hard to debug things when something goes wrong both for maintainers and users. This crate seems to go to extreme by having almost all code under macros, which is a big usability issue.

dot-asm · 2023-03-18T18:46:04Z

Just in case. Questions/rants that go beyond the specific subject matter will remain unanswered. The subject is that the suggested method is deemed to work reliably. Without disrupting current users and providing a straightforward path for potential no_std users.

nazar-pc · 2023-03-18T19:24:51Z

Without disrupting current users and providing a straightforward path for potential no_std users.

#150 is also fully backwords compatible and supports no_std for 100% of APIs at the same time 🤷‍♂️

rupansh · 2023-06-02T17:20:46Z

Hello are there any plans to merge this soon?

…std.

dot-asm · 2023-06-16T19:40:20Z

Merged.

This was referenced Mar 10, 2023

Rust: no_std support #150

Closed

better no_std support #125

Closed

nazar-pc reviewed Mar 10, 2023

View reviewed changes

dot-asm force-pushed the no_std branch from 3bcc481 to b01506c Compare March 15, 2023 17:47

nazar-pc reviewed Mar 16, 2023

View reviewed changes

dot-asm force-pushed the no_std branch from 8520f64 to 18d36c3 Compare April 24, 2023 17:43

dot-asm force-pushed the no_std branch 5 times, most recently from 6cf5c27 to 4516f94 Compare May 8, 2023 20:43

alxiong mentioned this pull request May 10, 2023

cross compilation to wasm32 for blst EspressoSystems/jellyfish#252

Closed

dot-asm force-pushed the no_std branch from 4516f94 to 7bfdbdb Compare May 18, 2023 17:32

dot-asm force-pushed the no_std branch from 7bfdbdb to 82099d2 Compare May 25, 2023 20:32

dot-asm force-pushed the no_std branch 4 times, most recently from 7ed2b4d to 734c562 Compare June 16, 2023 11:30

dot-asm added 2 commits June 16, 2023 15:39

bindings/rust/src/lib.rs: mask std-dependent functions and engage no_…

dd2b5c1

…std.

bindings/rust: add pippenger-no_std.rs [and deduplicate tests].

8a7e760

dot-asm force-pushed the no_std branch 2 times, most recently from be64c71 to d3dbaab Compare June 16, 2023 17:39

dot-asm added 2 commits June 16, 2023 20:32

bindings/rust/build.rs: refine no_std detection.

8b45986

.github/workflows/ci.yml: exercise no-std [with limited stack size].

3289650

dot-asm force-pushed the no_std branch from d3dbaab to 3289650 Compare June 16, 2023 18:32

dot-asm closed this Jun 16, 2023

Conversation

dot-asm commented Mar 10, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

huitseeker Aug 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dot-asm commented Mar 18, 2023

Uh oh!

nazar-pc commented Mar 18, 2023

Uh oh!

rupansh commented Jun 2, 2023

Uh oh!

dot-asm commented Jun 16, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

huitseeker Aug 9, 2023 •

edited

Loading