SwarmBuilder generics are too restricting

[sirandreww-starkware]

Description

SwarmBuilder is too restricting when I want to conditionally user some features. For example:

let mut swarm = SwarmBuilder::with_existing_identity(key_pair)
            .with_tokio()
            .with_tcp(Default::default(), noise::Config::new, yamux::Config::default)
            .expect("Error building TCP transport")
            .with_dns()
            .expect("Error building DNS transport")
            .with_behaviour(|key| {
                mixed_behaviour::MixedBehaviour::new(
                    key.clone(),
                    bootstrap_peer_multiaddr,
                    sqmr::Config { session_timeout },
                    chain_id,
                    node_version,
                    discovery_config,
                    peer_manager_config,
                )
            })
            .expect("Error while building the swarm")
            .with_swarm_config(|cfg| cfg.with_idle_connection_timeout(idle_connection_timeout))
            .build();

If I want to sometimes use QUIC instead of TCP, and sometimes log, and sometimes use this behaviour or another I cannot do this incrementally by adding to the builder.

Motivation

It allows for making a slightly different swarm depending on the required parameters.

Current Implementation

pub struct SwarmBuilder<Provider, Phase> {
    keypair: libp2p_identity::Keypair,
    phantom: PhantomData<Provider>,
    phase: Phase,
}

The builder takes a generic provider and phase which change the object after setting each layer, which means that after diverging with one layer I can no longer have a variable that holds the builder because the type is ambiguous

Are you planning to do it yourself in a pull request?

Yes

[elenaf9]

Hi! There are several combinators, like Either or Toggle that allow you to switch between transports or only use a behavior optionally. With those you can have a single unambiguous type while still allowing the design that you described above. Does that help?

[dariusc93]

Hey! What youre experiencing with SwarmBuilder is actually intended, though not ideal for most who want the most flexibility. A option would be to have multiple swarmbuilder to build swarm based on some option you set/want (ie, one using tcp, another using quic, another using both, etc). I think in your case, it might be better to use Swarm::new directly, though this will require you to construct and configure the transports yourself, but would give you the most flexibility long term.

EDIT: As @elenaf9 mentioned, Either could be used in this case, though this would require you to use SwarmBuilder::with_other_transport to my knowledge

[sirandreww-starkware]

1. What about cases where I want to call with_bandwidth_metrics conditionally?
2. Why is this intentional? It's not like the produced swarm will have a generic type that is connected with the intermediate builder types.
3. Couldn't one make a more flexible system by making SwarmBuilder one object that contains toggles and have the build function figure out what to do, and return an error if this is not possible?

[dariusc93]

1. What about cases where I want to call with_bandwidth_metrics conditionally?

You can construct a transport as intended and whenever you need to conditionally enable the transport for bandwidth metrics, you can use OrTransport with Either if true, otherwise you would use Either to pass the current transport on. For example, you can look at how I enable relay as it would use OrTransport with Either. The same would probably be applicable for libp2p_metrics::BandwidthTransport.

2. Why is this intentional? It's not like the produced swarm will have a generic type that is connected with the intermediate builder types.

This falls back to how rust works since as well as how SwarmBuilder handles types. As you make a specific function call (ie SwarmBuilder::with_quic), it would use a specific type apart of SwarmBuilder that would conflict if you try to make it conditional as it is now. There might be some ways around it to mimic what you do when constructing multiple transport, but may not be idiomatic and would just be overly complex compared to just building everything yourself and passing it to Swarm vs Swarm::new.

3. Couldn't one make a more flexible system by making SwarmBuilder one object that contains toggles and have the build function figure out what to do, and return an error if this is not possible?

From what I would assume, it may be more complex internally.

[sirandreww-starkware]

I see, so one can always fall back on using Swarm::new when push comes to shove.

1. Am I the first to request this? Has this been an issue for literally no one 😆 ?
2. If I made a more dynamic version of SwarmBuilder would you approve it?

[sirandreww-starkware]

Another question, Is there an AND type that stacks two Transports together?

[dariusc93]

Another question, Is there an AND type that stacks two Transports together?

I believe OrTransport would be the answer?

[sirandreww-starkware]

@dariusc93 OrTransport seems to not be what I want. I want to have two transports "stacked" on top of each other, and so when one fails both fail. I would imagine there would be an AndTransport?

[elenaf9]

@dariusc93 OrTransport seems to not be what I want. I want to have two transports "stacked" on top of each other, and so when one fails both fail. I would imagine there would be an AndTransport?

Would you mind expanding a bit on your use-case? What kind of transports would you want to combine like this?

[sirandreww-starkware]

I'm exploring the best way to implement a custom security layer, which I'll call Cryptonite, that performs its own handshake after the base transport's security handshake has completed.

My goal is to use QUIC (or TCP + Noise) to handle the primary connection and its associated security, and then stack my Cryptonite layer on top of it to manage a secondary handshake. This is a follow-up to the discussion in https://github.com/libp2p/rust-libp2p/discussions/6104.

The core challenge is composing these two layers. Since transports often encapsulate their own security (e.g., TLS is part of QUIC), it's not immediately clear how to chain them.

The desired network stack would look like this:

Using QUIC:

\+-----------------+
|   Cryptonite    |
\+-----------------+
|      QUIC       |
\+-----------------+
|       IP        |
\+-----------------+

Using TCP:

\+-----------------+
|   Cryptonite    |
\+-----------------+
|      Noise      |
\+-----------------+
|       TCP       |
\+-----------------+
|       IP        |
\+-----------------+

Theoretically, TCP + Noise is a good example of stacking security on a transport. The question is: does a generic mechanism exist in rust-libp2p to stack two Transport implementations in this "AND" fashion? This would be similar to how OrTransport selects between two transports, but instead, it would ensure both successfully run in sequence.

Any guidance on how to achieve this transport composition would be greatly appreciated.

[elenaf9]

You probably want to wrap the inner transport in that case.

There was a similar discussion in #5818.
What you could do is use Transport::map (or manually warp the boxed inner transport) to apply custom logic to an established connection (i.e., map the connection-future). This custom logic could then perform your handshake on the connection that was established by the inner transport.

[sirandreww-starkware]

You've been super helpful, thank you so much 🙏
I'll try it out (possibly add this as en example) I'll let you know how it goes!!!