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

pure-rust via cfg("aegis_backend" = "..") vs cfg(feature = "..") #5

Open
pinkforest opened this issue Jun 26, 2024 · 1 comment
Open

Comments

@pinkforest
Copy link

pinkforest commented Jun 26, 2024

Hey thanks for working on this! Great stuff

Would you be open to moving the cfg(feature = "pure-rust") into as:

cfg("aegis_backend" = "libaegis" | "rust" | ..)

Happy to send a PR - ideally the selection is automatic but we provide "overrides" in curve25519_dalek f.ex.

We had extensive discussion over here re: features vs cfg when we bumped to curve25519 to v4:

I would recommend having this mutual exclusive override as cfg due to;

  • Allows top-level binary composing the primitive as the overarching choice and transients not having any say
  • Transient dependencies don't need to "relay" it down - dalek itself (via ed/x f.ex.) long time didn't relay the overrides
  • The compiler / cargo has now taken official stance on it to support the pattern
  • Composes as mutually exclusive feature vs features are not mutually exclusive
  • Other issues discussed in the long winded ref'd issue

The compiler now supports validating cfg (requires high MSRV though so we've ignored it until MSRV is high enough)

Ref:

@jedisct1
Copy link
Owner

Hi!

I'm not fluent enough in Rust to understand the difference

However, unless it removes a bug that can't be addressed otherwise, or makes things faster, or adds a feature, what I really care about is avoiding breaking changes.
People should be able to update the library version without having to change anything to their code. This includes compiling to WebAssembly without requiring any special tweaks.
Maybe what you propose is just some internal changes that wouldn't change any user-facing feature. In that case why not. Rust has so many completely different ways to do the same thing.

The pure Rust implementation was added for people wanting to compile it to WebAssembly, that don't necessarily have a WebAssembly C compiler installed. But it's incomplete and maintaining two implementations in the same project is not fun. So, the plan is rather to precompile libaegis as a webasssembly module, and just link it on that platform, as done by other projects. So that all platforms can share the same code.

If you need to cross-compile to a target that isn't supported by cargo-zigbuild, or don't have a C compiler for some reason, crypto-rust has all the AEGIS variants implemented in pure Rust. It's better than the Rust code we have here.

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

No branches or pull requests

2 participants