tendermint-rs icon indicating copy to clipboard operation
tendermint-rs copied to clipboard

Suggestions/ideas for crypto code: feature gating, extract module/crate

Open tony-iqlusion opened this issue 4 years ago • 3 comments

This is one of those issues that's probably my fault to begin with, but I thought I'd give some suggestions.

At iqlusion we're writing more and more libraries which use tendermint-rs purely for data types and domain modeling.

Right now it has a whole slew of cryptographic dependencies which take fairly long to compile. When using tendermint-rs merely for things like RPC, protos, or domain modeling, it'd be nice to be able to shut those dependencies off.

Here are some suggestions I think might be helpful:

Make crypto an optional feature of the tendermint crate

Presently the only crypto you can shut off is the secp256k1 feature, which I think was introduced when ECDSA/secp256k1 support used an FFI binding to a C library, a concern that has since gone away after migrating to the pure Rust k256 crate.

I think it would be nice to have a crypto feature (on-by-default) which can be used to switch off all cryptographic functionality. If implemented, I'd suggest getting rid of the secp256k1 feature, and just having a coarser-grained crypto feature.

Extract cryptographic functionality into tendermint::crypto module or tendermint-crypto crate

There's a lot of cryptographic functionality floating around in the toplevel namespace. I think it'd be nice to refactor all of that into a separate module/crate.

In addition to eliminating pollution of the toplevel namespace, it makes clear which functionality is specifically related to cryptography, which is helpful for anyone interested in auditing the cryptographic functionality.

tony-iqlusion avatar Aug 25 '21 13:08 tony-iqlusion

At a high level this makes sense. I have some questions for clarification:

  1. Would you like this option only for the tendermint crate, or are there any other crates for which this would make sense for your use case?
  2. Do you perhaps have a list of the dependencies you'd like to avoid? That'd make it easier to see what functionality would be disabled by introducing a crypto feature and having it turned off.

thanethomson avatar Aug 26 '21 14:08 thanethomson

Would you like this option only for the tendermint crate, or are there any other crates for which this would make sense for your use case?

I think it makes sense for just the tendermint crate for now, although if the code were factored out into a separate tendermint-crypto crate, it'd be pretty easy to feature-gate other crates which would like to use the cryptographic code directly.

Crates that don't have a hard requirement on cryptographic functionality (e.g. tendermint-rpc) could use default-features = false when importing tendermint as well, at least as a baseline.

Do you perhaps have a list of the dependencies you'd like to avoid? That'd make it easier to see what functionality would be disabled by introducing a crypto feature and having it turned off.

Here's a list:

  • ed25519
  • ed25519-dalek
  • k256
  • ripemd160
  • sha2
  • signature
  • subtle
  • zeroize

If those dependencies were optional, it would reduce the baseline set of total dependencies (including transitive ones) by 48 crates by my estimation.

tony-iqlusion avatar Aug 26 '21 17:08 tony-iqlusion

Thanks @tony-iqlusion! That helps a lot, and makes sense.

If those dependencies were optional, it would reduce the baseline set of total dependencies (including transitive ones) by 48 crates by my estimation.

Wow, that's massive :smile:

thanethomson avatar Aug 26 '21 17:08 thanethomson