crate-crypto / rust-eth-kzg

Apache License 2.0
12 stars 5 forks source link

Dependency on `subtle >= 2.5.0` is not codified #235

Closed michaelsproul closed 3 weeks ago

michaelsproul commented 3 weeks ago

rust_eth_kzg depends on subtle >= v2.5.0 due to its use of expect here:

https://github.com/crate-crypto/rust-eth-kzg/blob/7157789f08f636f18f9d02226344fb6c08bb4628/cryptography/bls12_381/src/batch_inversion.rs#L45-L48

expect was only added to CtOption in subtle v2.5.0 and does not exist in v2.4.1 as evidenced by:

This causes strange compilation failures if some package has constrained or locked the version of subtle at a prior version, as is the case with the aes-gcm v0.9.4 package:

Oddly, aes-gcm v0.9.2 did not place this upper bound on subtle, so this is a pretty dodgy change on the part of aes-gcm to introduce in a patch version! That version is 3yo at this point, so projects (like Lighthouse) should probably move off it. However, it would still help clarify things if rust_eth_kzg declared its minimum dependency as subtle v2.5.

At the moment the compilation error in Lighthouse after updating aes-gcm to 0.9.4 is very counterintuitive:

error[E0599]: no method named `expect` found for struct `subtle::CtOption` in the current scope
  --> /home/michael/.cargo/registry/src/index.crates.io-6f17d22bba15001f/crate_crypto_internal_eth_kzg_bls12_381-0.3.4/src/batch_inversion.rs:48:10
   |
46 |       tmp = tmp
   |  ___________-
47 | |         .invert()
48 | |         .expect("guaranteed to be non-zero since we filtered out zero field elements");
   | |         -^^^^^^ method not found in `CtOption<F>`
   | |_________|
   |

error[E0599]: no method named `expect` found for struct `subtle::CtOption` in the current scope
  --> /home/michael/.cargo/registry/src/index.crates.io-6f17d22bba15001f/crate_crypto_internal_eth_kzg_bls12_381-0.3.4/src/lib.rs:51:35
   |
51 |     Scalar::from_bytes_be(&bytes).expect("254 bit integer should have been reducible to a scalar")
   |                                   ^^^^^^ method not found in `CtOption<Scalar>`

With a minimum subtle version specified I think the compilation error would be something clearer like:

$ cargo update -p subtle --precise 2.6.1
    Updating crates.io index
error: failed to select a version for the requirement `subtle = ">=2, <2.5"`
candidate versions found which didn't match: 2.6.1
location searched: crates.io index
required by package `aes-gcm v0.9.4`
    ... which satisfies dependency `aes-gcm = "^0.9"` (locked to 0.9.4) of package `discv5 v0.4.1`
    ... which satisfies dependency `discv5 = "^0.4.1"` (locked to 0.4.1) of package `eth2_network_config v0.2.0 (https://github.com/sigp/lighthouse?rev=6566705505b417965dbaeafe107367cdb45bdf08#65667055)`
kevaundray commented 3 weeks ago

Thanks for the detailed writeup!

Yep it seems that aes-gcm 0.9.4 introduced a breaking change and it was added as a patch :(

For aes-gcm 0.10.3, it seems that they've fixed it, so your other issue of updating aes-gcm should fix the overall issue. If that issue takes too long to close and is blocking, I'll be happy to apply a patch to 0.3.4 of rust-eth-kzg to make it compatible with subtle 2.4.1.

I've opened up a PR here: https://github.com/crate-crypto/rust-eth-kzg/pull/237 to codify the subtle versions we depend on!