kayabaNerve commented 1 year ago

Motivation

Currently compiling sp-core with std will compile:

2 secp256k1 libraries (one in C, one in Rust)
An Ed25519 lib
A Ristretto lib
The Arkworks suite
BLS12-381 (arkworks)
Bandersnatch (also arkworks)

And that's for elliptic curves alone. Several hash algorithms/transcripts also get pulled in.

Request

Decrease tree size, ideally not only via feature flags yet completely in order to ensure minimal Cargo.locks (making supply chain evaluations much easier).

Solution

Move all of these literals into their own primitives crates.

For secp256k1/Ed25519/Ristretto specifically, their use is almost fully abstracted behind traits. Accordingly, it should be feasible to have sp-schnorrkel, sp-ed25519, and sp-ecdsa preventing sp-core compilations from forcing inclusion of so many dependencies.

I'd hope to see this expanded to the Arkworks section, yet I know that's under active development and don't want to throw a wrench at it while it moves.

I'd be fine helping with this yet I'm unwilling and/or not-reasonably-able to sign the CLA at this time, sorry.

Are you willing to help with this request?

Maybe (please elaborate above)

Status

[x] Move hashing procedures to sp-crypto-hashing (https://github.com/paritytech/polkadot-sdk/pull/2306)
- Deprecation of sp-core-hashing and `sp-core-hashing-proc-macro (scheduled removal Jun 2024)
[ ] Move crypto primitives in crypto folder
- Features for each primitive

kayabaNerve commented 1 year ago

133 relevant.

davxy commented 1 year ago

This is something I've been thinking about for a while.

Even though it doesn't help with the supply chain size I'd like to first identify and move all the crypto-related stuff in the substrate/primitives/crypto folder (is already present and for the moment it just contains EC related host functions). This at least will separate it from non-crypto core stuff.

As a potential follow-up we can then consider "one crate per primitive" or "feature gated primitives"..

For sure it'd help reducing the not required deps and build times in the consumers
It'd be a bit more annoying as you have to pick the crate of interest instead of including sp-crypto in one shot

Maybe feature flags are better than separate crates here (don't know... just asking for opinions)?

Let's collect some opinions first (cc @paritytech/sdk-node)

koute commented 1 year ago

Let's collect some opinions first

Personally I'd probably move all crypto related stuff into sp-crypto, make all of them disabled by default (so by default only the generic interfaces would be compiled, without any actual crypto code/dependencies) and add feature flags for each of them.

michalkucharczyk commented 1 year ago

My two cents:

sounds good, sp-crypto makes sense.
I am thinking of explicit randomness feature. Now the functionality which requires randomness is implicitly hidden behind std feature.
would be also nice to have tests working for different subsets of features.

paritytech / polkadot-sdk