search

hyperledger-archives / ursa

Hyperledger Ursa (a shared cryptographic library) has moved to end-of-life status, with the components of Ursa still in use moved to their relevant Hyperledger projects (AnonCreds, Indy, Aries and Iroha).
https://wiki.hyperledger.org/display/ursa
Apache License 2.0
321 stars 142 forks source link

Remove libsodium and time dependencies #200

Closed andrewwhitehead closed 2 years ago

andrewwhitehead commented 2 years ago

NB I haven't built and tested the docker images.

andrewwhitehead commented 2 years ago

Somewhat related but I don't think this repo should have a Cargo.lock. It causes older dependency versions to be pinned when running tests (and the audit) where they would be updated to the latest patch versions when the library is used.

mac-arrap commented 2 years ago

We discussed during the last URSA meeting that we wanted to switch from libsodium to Dalek. This seems to just remove where libsodium is called but doesn't replace it.

andrewwhitehead commented 2 years ago

libsodium wasn't used for ed25519 except in tests (verifying the Dalek backend) and in benchmarks.

dcmiddle commented 2 years ago

Somewhat related but I don't think this repo should have a Cargo.lock. It causes older dependency versions to be pinned when running tests (and the audit) where they would be updated to the latest patch versions when the library is used.

I don't like the lock file in here either. Official guidance is that libraries should not commit the lock file[1] unless the library is a static or cdylib[2]. Ursa is setup as a static, cdylib[3] ... but why? I'm not sure. If you want to remove it, I'm supportive but please create a separate issue so we can make sure we understand why ursa is setup this way.

[1]https://doc.rust-lang.org/cargo/faq.html#why-do-binaries-have-cargolock-in-version-control-but-not-libraries [2]https://doc.rust-lang.org/cargo/guide/cargo-toml-vs-cargo-lock.html [3]https://github.com/hyperledger/ursa/blob/main/libursa/Cargo.toml#L35

dcmiddle commented 2 years ago

fixes #196

mac-arrap commented 2 years ago

If it's agreed that this benchmarking can be removed then I see no problem. The only reason I stated the above was per the request given to me on the last URSA call. But I do agree that it's not useful to just test another libraries backend. @hartm please review