Non-additive feature flags are problematic

[djc]

Currently you cannot compile curv with more than one of the specific curve features enabled due to the top-level FE, GE, PK, SK aliases. Please note that this is generally discouraged, because in a complex application there might be a dependency graph that wants to enable different curves, and this will result in compile failures in curv.

(Also, I really dislike these two-letter type names and the fact that there are two sets of aliases for each curve. IMO, using PublicKey and SecretKey consistently instead would make code using curv much more readable.)

[omershlo]

Enabling only a single curve is by design. No cryptographic system should use multiple curves simultaneously.
About the aliases: maybe, I think it's a matter of personal taste :)

[djc]

There is an established idiom for naming things in Rust libraries, though:

https://rust-lang-nursery.github.io/api-guidelines/naming.html

IMO following that consistently should generally be preferred over personal taste (though obviously there are backwards compatibility concerns at this stage).

[omershlo]

I think that because of the backwards compatibility issue it will not be possible to change it now. But we should definitely follow the established idiom . Thanks!

[omershlo]

having said that - I would love to see an additive feature version of Curv library. It would be really interesting and useful (we get this request a lot)

[djc]

One version to do this would be to split this repository into five separate crates, one per curve + one base crate that only holds the common traits. This would make things more obvious. Then you could keep the shared curv crate as a high-level interface that depends on all of these others.

Still, this adds quite a bit of extra complexity. IMO it would be better to bite the bullet and simply stop providing the top-level types. With some reorganization, this means that dependencies add an extra path component to their imports (use curv::GE -> use curv::secp256k1::GE).

[omershlo]

cc: @oleiba exactly what we have talking about.
Another bonus is that we will be able to put some of the crates at crates.io

[oleiba]

We worked around this issue, by having the depending library to use curv of different tag, and if needed a different package alias in Cargo.toml.

For example, we have https://github.com/KZen-networks/gotham-city which depends both on https://github.com/KZen-networks/curv directly with feature ec_secp256k1, and on https://github.com/KZen-networks/multi-party-eddsa which depends on curv with feature ec_ed25519.

So we added an additional tag on curv e.g. v0.2.0-ed25519 (on the same commit as v0.2.0), and https://github.com/KZen-networks/multi-party-eddsa uses this tag with feature ed_25519. That way https://github.com/KZen-networks/gotham-city does not break and is able to import both features of curv.

[djc]

So if you use that as a backwards compatibility shim for now, then it should also be fine to change things to not have the top-level exports in a 0.3 release?

[oleiba]

Yes, we now follow semver semantics so you can break backwards compatibility and update the version accordingly, e.g. 0.2.0 -> 0.3.0.