Remove PassthroughDigest and update crypto dependencies #385
Conversation
sbihel
force-pushed
the
fix/remove-passthroughdigest
branch
from
a430169
to
5a74c89
Compare
|
Checked on this again, it seems the original issue is resolved, but now |
Ran `cargo hack test --each-feature --workspace --exclude-features "tor-tests" -- --test-threads=8` 3 tests are failing, maybe from older changes Update CI to run tests with cargo hack
sbihel
force-pushed
the
fix/remove-passthroughdigest
branch
from
5a74c89
to
ab205fa
Compare
sbihel
changed the title
| #[error("Curve not implemented: '{0}'")] | ||
| CurveNotImplemented(String), | ||
| /// Errors from p256, k256 and ed25519-dalek | ||
| #[cfg(feature = "k256")] |
There was a problem hiding this comment.
should these features be "secp256k1" and "secp256r1"?
There was a problem hiding this comment.
I was asking myself the same question -- I came to the conclusion that it doesn't matter as long as we make sure to always use features and never optional dependencies as features
There was a problem hiding this comment.
never mind I forgot about dependencies being enabled by other crates
There was a problem hiding this comment.
I've opened #472 to reduce the scope
| let hash = openssl::hash::hash(digest, &data)?; | ||
| let sig = openssl::ecdsa::EcdsaSig::sign(&hash, &sk)?; | ||
| [sig.r().to_vec(), sig.s().to_vec()].concat() | ||
| #[cfg(feature = "p256")] |
There was a problem hiding this comment.
looks like an oversight from #457, should this say "secp256r1"?
ssi-vc/src/lib.rs
Outdated
| @@ -2925,7 +2925,9 @@ _:c14n0 <https://w3id.org/security#verificationMethod> <https://example.org/foo/ | |||
| assert_eq!(n_proofs, 4); | |||
| } | |||
|
|
|||
| // Can't deserialise the URIs as verification methods (?) | |||
There was a problem hiding this comment.
this relies on the testing flag for ssi-dids to be enabled (see ssi-dids/src/lib.rs#973)
.github/workflows/build.yml
Outdated
|
|
||
| - name: Test DID Resolution HTTP(S) Binding | ||
| run: cargo test --manifest-path ssi-dids/Cargo.toml --features http | ||
| run: cargo hack test --each-feature --workspace --exclude-features "tor-tests" -- --test-threads=4 |
There was a problem hiding this comment.
do we want to do --feature-powerset or --group-features instead of --each-feature? I think this may exclude some combinations and tests
There was a problem hiding this comment.
Depends if you want to run tests for 17,421 combinations -- I checked and there didn't seem to be obvious combination of features. I will double check on tests though, making sure e.g. secp256k1 tests run for both the secp256k1 and ring features
There was a problem hiding this comment.
I added another step in the CI to run the tests with all features enabled, that should cover any build/test problem
Ah my mistake, I thought it didn't do any crypto stuff (only decoding/parsing keys) as we are using ssi-jws but here's the default default = ["compression", "crypto-nettle"]
|
|
I replaced the pgp library for one with more users and more recent versions of RustCrypto crates. Had to modify a tests assertions but the changes are valid, and tested with gpg. |
This test was purely a regression test. k256 version change in #385 changed the recoverable/ethereum signatures generated (both variants are valid and verify successfully). Ref: RustCrypto/elliptic-curves#552
This test was purely a regression test. k256 version change in #385 changed the recoverable/ethereum signatures generated (both variants are valid and verify successfully). Ref: RustCrypto/elliptic-curves#552
This test was purely a regression test. k256 version change in #385 changed the recoverable/ethereum signatures generated (both variants are valid and verify successfully). Ref: RustCrypto/elliptic-curves#552
This test was purely a regression test. k256 version change in #385 changed the recoverable/ethereum signatures generated (both variants are valid and verify successfully). Ref: RustCrypto/elliptic-curves#552
This test was purely a regression test. k256 version change in #385 changed the recoverable/ethereum signatures generated (both variants are valid and verify successfully). Ref: RustCrypto/elliptic-curves#552
Ran
cargo hack test --each-feature --workspace --exclude-features "tor-tests" -- --test-threads=83 tests are failing, maybe from older changes
Update CI to run tests with cargo hack