Don't build secp256k1 by default.

[dvc94ch]

any features required to make a network behaviour work will be specified manually. for testing it is common to just add libp2p to the dev-dependencies to get a development-transport. This PR disables all the features that aren't required for building a development transport.

[thomaseizinger]

Personally, I always use rust-libp2p with default-features = false. Given the additive nature of Cargo's features, I would suggest to go even one step further and have everything off by default. To make things like the development_transport easier to use, we could have a dedicated feature for that (testing for example) that gives you those things.

[mxinden]

I am fine with removing secp256k1 from the default feature set.

I would suggest to go even one step further and have everything off by default.

I see the default features as the equivalent to batteries included, allowing newcomers to get started without having to tinker with any cargo features. After all, getting started with rust-libp2p is already hard enough. I assume that advanced users will set default-features = false as a first optimization step either way. With that in mind, I am not in favor of "everything off by default".

[thomaseizinger]

I assume that advanced users will set default-features = false as a first optimization step either way.

That works if your crate is the only one pulling in libp2p. The annoying thing is that once turned on, features in a dependency graph can never be turned off again. Thus, if you are pulling in a dependency that already depends on libp2p and that dependency did not turn off all default features, you are somewhat screwed.

Now, how often this case happens is debatable :)

But by having all features turned off by default, this is much less likely to happen. I really like how tokio approached this from version 1 onwards: selective features so you can pick what you need. If you don't want to bother, just enable full.

Regarding batteries included: The batteries are still there, you just need to plug them in :P

There is certainly potential for making things easier for newcomers. Cargo features and conditional compilation are a fact of life in the Rust world though, it is not like rust-libp2p is doing something completely different here.

At the risk of derailing the discussion completely: One thing that IMO we could do much better at is supporting different executors. For example, at the moment it possible to use tcp-tokio with async-std-dns. That doesn't make a whole lot of sense though. It would be nicer if the user would pick features such as: tokio, dns, tcp and tokio forwards to libp2p-tcp/tokio and libp2p-dns/tokio while tcp just enables libp2p-tcp without an executor. Generally, it has always been a bit of an annoyance to use rust-libp2p with the tokio runtime without pulling in all of the async-std dependencies. It is possible but the configuration always felt very fragile, especially because of the default choices.

[dvc94ch]

I am fine with removing secp256k1 from the default feature set.

I agree with @thomaseizinger, although I think this would already be an improvement as it pulls in three crates for a feature that is likely not used very often. So to turn your suggestion into a rule would be the following if I understand correctly:

• protocols are enabled by default for easy discovery but other optional features aren't

[mxinden]

I am torn.

On the one hand, also after reading through tokio-rs/tokio#1811, I like the idea of an empty default and an additional full which would enable all features. Users would use full if they just don't care or are just getting started and hand-pick their features otherwise. Missing a specific feature would simply surface as a compile time error which is easy to fix.

On the other hand there are special features, like secp256k1. Say I am a user that uses ed25519 for their own key, though still wanting to support remote peers using secp256k1. Accidentally not enabling the secp256k1 feature would not surface as a compile time error.

Do you know of more crates that follow the empty default and additional full pattern?

In the end, I think I am fine with either way, as long as:

• It is well documented.
• It is well communicated via corresponding CHANGELOG.md entries.
• It is consistent across the whole rust-libp2p repository.

Thanks for all the input!

[thomaseizinger]

Say I am a user that uses ed25519 for their own key, though still wanting to support remote peers using secp256k1.

I firmly believe that requirements like this have to come with a test, preferably e2e. At that point, we don't have a compile error yes, but we will fail in the next bigger feedback loop: testing.

In the end, I think I am fine with either way, as long as:

• It is well documented.
• It is well communicated via corresponding CHANGELOG.md entries.
• It is consistent across the whole rust-libp2p repository.

That is exciting!

@dvc94ch Unless you are happy to do it, I am fine with taking on these tasks given that I brought up this idea. Let me know your preference!

[dvc94ch]

Sorry I haven't gotten around to it yet. But if you I'm happy to take you up on your offer. I'll see if I can add tls to libp2p-quic this weekend.