Require that all transports be "fully featured"

[Stebalien]

This is an alternative to #17.

Instead of introspecting transports in https://github.com/libp2p/go-libp2p-conn to see what features they support, we can require that all transports support all features: pnet (pre-shared key), encryption/authentication (e.g., secio), multiplexing, (packets?). To avoid forcing every transport to implement all of these features, we can use composition. That is, provide a set of helper types/functions that can be used to plugin in default implementations of these features.

This:

1. Makes go-libp2p-conn much simpler (my only real motivation).
2. Allows us to avoid having to rely on runtime type casting to figure out what type of connection a transport returns.
3. Gives transports full control. We'll never need to add new interfaces for, e.g., secure duplex connections, insecure multiplex connections, pnet but unauthenticated duplex connections (e.g., TCP over IPSEC-PSK), etc...

---

The large drawback here is backwards compatibility with old transports (adding a new feature means changing existing transports). However, this will often be the case anyways if we want to be able to pass in new parameters when we construct connections (unless we version the dial functions but, IMO, that's no better).

[Stebalien]

Also, I believe this would allow us to more easily punt some decisions around QUIC down the road.

[daviddias]

That is also how js-libp2p is structured:

• https://github.com/libp2p/interface-transport (pretty much simple and dial interface)
• https://github.com/libp2p/interface-connection (the interface we get, pretty much a duplex stream)
• https://github.com/libp2p/js-libp2p-swarm (glues everything together. We know Swarm is a bad name -> https://github.com/libp2p/js-libp2p-swarm/issues/40)

[Stebalien]

First, there's a glaring motivation I missed: different transports will need different arguments. For example, secure connections will need keys. That makes fixing this a priority (as-is, our transport interfaces won't cut it).

Second, an alternative (preferred by @whyrusleeping) is to add transports for every level. That is,

• Transport - fully multiplexed and secured
• SecureTransport - secure but not multiplexed
• StreamTransport (or DuplexTransport?) - reliable stream, no crypto
• PacketTransport (someday...)

This solves the problem of needing to be able to pass different arguments when creating connections at different levels without forcing every transport to be fully-featured. Users would be able to register any kind of transport and have it type-checked at registration time (not compile time but still better than checking on connect).

Personally, I'm not a fan of introducing so many interfaces but this does solve the problem (and, arguably, makes it easier for programmers to develop new transports).

[whyrusleeping]

The second approach is also much closer to what @marten-seemann has been doing

[Stebalien]

Yeah, it just requires at least 6 more interfaces (*Dial, *Listen, *Transport, *Conn). However, I'm probably just lazy.

[marten-seemann]

@Stebalien: I'm not sure I understand your proposal. Can you explain?
I never really liked the accept-time conn-type assertion I made in go-libp2p-conn, so I'm happy if we find a clean way to get rid of that.
What is the packet feature, is that unreliable delivery?

Composition might be hard to do. Take the pnet for example. Making this work for multiplex connections required a lot of changes: libp2p/go-libp2p-pnet#8. (Or maybe this is a bad example, since we can replace pnet by TLS 1.3 PSK mode anyway).

[Stebalien]

Ignore the packet feature, that was just me trying to ensure we don't have to redesign this stuff when we get unreliable packet-based transports.

First of all, for context, @whyrusleeping's proposal is to define multiple transport interfaces along with the associated Dialer/Listener/Conn interfaces (not shown):

trait Transport interface {
    // Intentionally vague...
    // NOTE: this is why *something* needs to change. We need to be able to pass crypto stuff when constructing dialers/listeners for dialers/listeners that do crypto.
    func Dialer(cryptoStuff CryptoOptions, lid peer.ID, laddr peer.ID, opts ...DialerOpt) Dialer
    func Listener(cryptoStuff CryptoOptions, lid peer.ID, laddr peer.ID, opts ...ListenerOpt) Listener
    func Matches(maddr Multiaddr) bool
}

trait SecureTransport interface {
    func Dialer(cryptoStuff CryptoOptions, lid peer.ID, laddr peer.ID, opts ...SecureDialerOpt) DuplexDialer
    func Listener(cryptoStuff CryptoOptions, lid peer.ID, laddr peer.ID, opts ...SecureListenerOpt) SecureListener
    func Matches(maddr Multiaddr) bool
}

trait BasicTransport interface {
    func Dialer(laddr peer.ID, opts ...BasicDialerOpt) BasicDialer
    func Listener(laddr peer.ID, opts ...BasicListenerOpt) BasicListener
    func Matches(maddr Multiaddr) bool
}

Programmers would register their transports at start (probably in module-level init functions). We could either have a single RegisterTransport(interface{}) error function or multiple Register*Transport(*Transport) functions.

My only real objection was that this needs 12 interfaces. An alternative that only requires 4 interfaces and no casting is to just define the Transport interface, Conn, SecureConn, and DuplexConn and have implementers call helper functions to secure/multiplex their own connections. The helper functions would be: func SecureConnDefault(DuplexConn) SecureConn and func MultiplexConnDefault(SecureConn) Conn. This would also allow users to, e.g., attempt to secure their own connections using some connection-specific scheme and fall back on calling SecureConnDefault. However, the drawbacks: we don't get to do all of this in one place (harder to manage) and implementers of new transports have to get this right (they can't just implement what they care about and hand off the rest to us. For the second drawback, I'd argue that we can write foolproof functions that make it impossible to screw up (the types won't check) but that's debatable.

In summary,

Pros

• 4 interfaces versus 12
• Transports make explicit choices (they explicitly choose to use the defaults and can, e.g., pass custom options to the default multiplexer).

Cons

• More work for the transports
• We can't add features by just adding a new level (feature-set) and providing a reasonable default implementation.
• Not centrally managed (harder to rate-limit, track, etc.). Note: We can't centrally manage crypto handshakes for, e.g., QUIC so I don't find this argument very convincing.

Honestly, @whyrusleeping's approach is probably better in the long run (largely due to the second con). It just requires a bit more thought up-front (to get all the interfaces right).

Making this work for multiplex connections required a lot of changes

I'm not sure I follow. What changes?

[vyzo]

this seems to be complicating a lot -- 12 interfaces is insane!

[Stebalien]

State: In progress.

Branch feat/refactor in:

• libp2p/go-libp2p-net
• libp2p/go-libp2p-net
• libp2p/go-libp2p
• libp2p/go-libp2p-circuit
• libp2p/go-libp2p-transport
• libp2p/go-libp2p-swarm
• libp2p/go-libp2p-kad-dht -- netutil changes + changed notification guarantees.
• libp2p/go-floodsub -- netutil changes
• libp2p/go-addr-util -- Remove AddrUsable (the swarm should be responsible for this...)
• libp2p/go-libp2p-secio
• libp2p/go-libp2p-interface-pnet
• libp2p/go-tcp-transport
• libp2p/go-ws-transport
• libp2p/go-libp2p-pnet

Branch master in the new repos:

• libp2p/go-conn-security-multistream: multistream security multiplexer
• libp2p/go-conn-security: stream security interfaces
• libp2p/go-reuseport-transport: base transport for all TCP reuseport transports
• libp2p/go-libp2p-transport-upgrader: stream transport to full libp2p transport upgrader

Deprecated packages:

• libp2p/go-libp2p-interface-conn: will merge what is left with go-libp2p-net, the distinction between this package, go-libp2p-net, and go-libp2p-transport always confused me anyways so I'm glad to see one of them go.
• libp2p/go-libp2p-conn: mostly replaced by go-stream-transport
• libp2p/go-peerstream: merged into swarm

TODO:

• go-libp2p-host (no changes needed for now, will do this later)
• go-libp2p-circuit
• go-ws-transport (will use go-reuseport-transport) (will use go-reuseport-transport later)
• go-libp2p

(and probably more)

However, those changes should be fairly small.

Known Bugs:

• There's an issue somewhere in go-reuseport-transport that's causing the swarm tests to fail when reuseport is disabled. I'll look into this, reuseport is... special. (simultaneous dial issue)

@marten-seemann,

Hopefully this should give you an idea of what's going on if you want to jump in. With the exception of go-reuseport-transport, this code should be significantly cleaner and easier to understand (not having piles of bug fixes on top of piles of bug fixes helps quite a bit...).

[Stebalien]

Note: We went with the original fully-featured transport proposal, not the many traits version.

[Stebalien]

Moved to: libp2p/go-libp2p#297