integritychain / fips203

Pure Rust implementation of FIPS 203 Module-Lattice-based Key-Encapsulation Mechanism Standard for server, desktop, browser and embedded applications.
Apache License 2.0
7 stars 1 forks source link

relax regex and zeroize dependencies #3

Closed dkg closed 8 months ago

dkg commented 8 months ago

These dependencies were bumped without comment or other code change in ff4beaf8ef69f81035fd4ba481926169579ee9dc. This reverts that part of that commit. "cargo build" and "cargo test" still both pass.

Neither upstream change was necessary to make the package build or test correctly, from what i can see, and there was no backward-incompatible API change (the versions are semver-compatible).

There is nothing wrong with raising a dependency when it's needed, but raising a dependency above the necessary base version makes it harder to build against older installations.

dkg commented 8 months ago

I think the tests that are failing here are failing due to sub-dependencies that want a higher MSRV. If the cargo resolver were flexible enough to pick a satisfiable dependency that doesn't tickle this sub-depedency, i think they would pass.

At any rate, i'm building and running the tests successfully against rust 1.70. So i don't know what the Rust ecosystem expectations are about how strictly MSRV has to be matched. :shrug: it's possible that you'll just want to cherry-pick a232e7e and leave behind the other commits on this series.

integritychain commented 8 months ago

Apologies for the gymnastics here. The GA tests were (earlier) not configured to run on merge requests so didn't initially pick up the issue. I spent a bunch of time earlier trying to unwind it a bit without success. However, I have a few new(er) ideas...give me a day/two to tweak a bit

integritychain commented 8 months ago

dkg, please could you have a look at the latest commit [3923c98] and let me know if it works for you? The GA are flaky on 1.70 but I can test successfully on my machine as well. I am happy to publish 0.1.3 if that helps you. thanks!

dkg commented 8 months ago

@integritychain thanks for this work! 3923c98fd302ba02bdaa1ddecfadc1a5bd3cea0f looks great to me and differs only in how it defines the dev-dependency on criterion, which i think i can work around as needed.

I would love a 0.1.3 release with these changes if it's not too much of a hassle.

What do you think about releasing the ffi subdir to crates.io as fips203-ffi ? Is that something you want to do? I don't know enough about how to integrate the tests i wrote into Cargo's testing framework, but if you or anyone else wants to polish that up, i'd be happy to learn from that work.

dkg commented 8 months ago

I'm closing this MR to keep your list of open requests clean; if i figure out a way to loosen the criterion dev-dep later in a way that doesn't break your CI, i can do that with a separate MR. I really appreciate your responsiveness here!

integritychain commented 8 months ago

Thanks for your help and understanding on this - I published 0.1.3 just now. I do have an interest in polishing up the FFI and supporting bindings much more broadly, but will need to clear out some time for this. For now, I inserted a sentence into the top of the README promoting their availability. This code provides some (non-Rust) examples worthy of study https://github.com/supranational/blst/tree/master/bindings