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

Default to no_std in all crates #2413

Open
tcharding opened this issue Jan 28, 2024 · 11 comments · May be fixed by #2543
Open

Default to no_std in all crates #2413

tcharding opened this issue Jan 28, 2024 · 11 comments · May be fixed by #2543

Comments

@tcharding
Copy link
Member

tcharding commented Jan 28, 2024

Explicit trumps implicit any day, we should use the more explicit #![no_std] attribute in all crates except bitcoin as we did in units.

ref: #2408 (comment)

@Kixunil
Copy link
Collaborator

Kixunil commented Jan 29, 2024

Why except bitcoin?

@tcharding
Copy link
Member Author

Oh, I got mixed up with no-std and requiring an allocator, you are correct, including bitcoin.

@krisbitney
Copy link

This may be slightly off-topic, but I'm wondering how close rust-bitcoin is to full support in a no_std environment like Wasm? Is it currently possible to sign a taproot transaction with a local private key in no_std mode?

@apoelstra
Copy link
Member

I believe the answer is "yes" as far as no-std goes, but we may have WASM-specific problems with rust-secp256k1 because it links to a C library.

@krisbitney
Copy link

Amazing, thank you. I know that Substrate uses the secp256k1 rust library in a Wasm environment. At least, I saw it on their dependency list.

They also use this crate, which they wrote for no_std:
https://crates.io/crates/libsecp256k1

I'm not sure of the specifics. I'm just doing research. I'll have to do some testing. Thank you again!

@Kixunil Kixunil changed the title Default to no_std in all but bitcoin crate. Default to no_std in all crates Feb 29, 2024
@Kixunil
Copy link
Collaborator

Kixunil commented Feb 29, 2024

This may be slightly off-topic, but I'm wondering how close rust-bitcoin is to full support in a no_std environment like Wasm?

FTR you don't actually need no_std for wasm to work. I made a production-ready piece that also performs taproot signing using this crate with std enabled. :)

@krisbitney
Copy link

This may be slightly off-topic, but I'm wondering how close rust-bitcoin is to full support in a no_std environment like Wasm?

FTR you don't actually need no_std for wasm to work. I made a production-ready piece that also performs taproot signing using this crate with std enabled. :)

Amazing! Thank you.

@TheBlueMatt
Copy link
Member

FTR you don't actually need no_std for wasm to work. I made a production-ready piece that also performs taproot signing using this crate with std enabled. :)

Do we want to support that, though? WASM+std is some crazy half-working Frankenstein monster where everything compiles but you call the wrong function (mostly SystemTime) and suddenly you panic. It's not really something practical to support IMO without building a custom static checking framework.

@dpc
Copy link
Contributor

dpc commented Feb 29, 2024

Do we want to support that, though? WASM+std is some crazy half-working Frankenstein monster where everything compiles but you call the wrong function (mostly SystemTime) and suddenly you panic. It's not really something practical to support IMO without building a custom static checking framework.

@TheBlueMatt : We're using in Fedimint/Fedi and it works good enough. System time is PITA, but we have semgrep lints; wasm unit, integration and e2e tests; and recently we've figured a way to "grep" the wasm binary and flag banned function call, which should be fast and reliable. (twiggy paths --regex wasm_bg.wasm 'std::time::Instant::now.*').

I'm aware there are some risks w.r.t miscompilation of ABI for rust-secp256k1, which i guess we'll just have to live with for now.

@TheBlueMatt
Copy link
Member

Certainly not saying its not doable, only asking if we want to officially support that in rust-bitcoin or not. Now that std/no-std is de-tied from this stupid global where you have to set the same one on LDK/rust-bitcoin/etc/etc, there's not a lot of reason users can't use no-std on LDK+rust-bitcoin but std on BDK or whatever. That means we can pick whether we want to support WASM+std or if we just want to tell WASM users to use no-std (which has basically no fewer features in rust-bitcoin). Users can do whatever they want, but its on them if we break it and dont support it :)

@Kixunil
Copy link
Collaborator

Kixunil commented Mar 1, 2024

In my case the only reason I use std is because I don't want to bother with secp256k1 contexts. bitcoin doesn't call any such functions and the only way it can do IO is via IO traits (I use byte slices/vecs).

But I'd absolutely love to get rid of it for the peace of mind and potential space optimization. If we can get secp256k1 have static contexts without std that'd be great.

only asking if we want to officially support that in rust-bitcoin or not

I think we can and should at least until no_std is more ergonomic. Calling any system function (other than alloc) should be immediately obvious anyway because there shouldn't be a reason to need it. The check @dpc suggests sounds great and we should add it to CI.

@tcharding tcharding linked a pull request Mar 6, 2024 that will close this issue
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 a pull request may close this issue.

6 participants