refactor transports to be fully featured #29
Conversation
ghost
assigned
| @@ -1,53 +0,0 @@ | |||
| package transport | |||
There was a problem hiding this comment.
Removed without replacement (the libp2p constructor will add-in the tcp transport by default if no other transports are specified.
transport.go
Outdated
| Protocols() []int | ||
|
|
||
| // Proxy returns true if this transport proxies. | ||
| // TODO: Make this a part of the go-multiaddr protocol instead? |
There was a problem hiding this comment.
I think it's fine to leave this here for now but we may want to move it later. Proxy transports are transports like p2p-circuit that handle part of the multiaddr and forward the rest to some other system.
| @@ -50,9 +74,10 @@ type Listener interface { | |||
| Multiaddr() ma.Multiaddr | |||
| } | |||
|
|
|||
| // DialOpt is an option used for configuring dialer behaviour | |||
| type DialOpt interface{} | |||
There was a problem hiding this comment.
All configuration options are passed to the transport on construction.
| @@ -24,19 +40,27 @@ type Conn interface { | |||
| // but many more can be implemented, sctp, audio signals, sneakernet, UDT, a | |||
| // network of drones carrying usb flash drives, and so on. | |||
| type Transport interface { | |||
| Dialer(laddr ma.Multiaddr, opts ...DialOpt) (Dialer, error) | |||
There was a problem hiding this comment.
We no longer have "dialers". Instead, we just call Dial on the transport and the transport decides how to dial (e.g., which source address to use). In the past, we constructed one dialer per listen address and picked the dialer randomly. That was obviously not an optimal solution but the swarm just didn't have enough information to make an informed decision (without special-casing transports). On the other hand, the transport does have enough information.
| Dialer(laddr ma.Multiaddr, opts ...DialOpt) (Dialer, error) | ||
| // Dial dials a remote peer. It should try to reuse local listener | ||
| // addresses if possible but it may choose not to. | ||
| Dial(ctx context.Context, raddr ma.Multiaddr, p peer.ID) (Conn, error) |
There was a problem hiding this comment.
Given that we're breaking things, I got rid of the distinction between Dial and DialContext. Just pass a context.
Also, this now takes a peer ID. While we can technically get away with not having this, it allows us to validate connections early (given that transport security now happens inside the transport).
README.md
Outdated
| @@ -21,26 +21,6 @@ This is the 'base' layer for any transport that wants to be used by libp2p and i | |||
| > gx-go rewrite | |||
| ``` | |||
|
|
|||
| ## Usage | |||
There was a problem hiding this comment.
no new usage text? @Stebalien tsk tsk tsk
There was a problem hiding this comment.
One generally doesn't use transports directly like this. One implements them.
... fine. I'll add an example.
|
edit: punt, see #30 So, during the refactor, I inverted the transport/net dependency. That is, instead of go-libp2p-net depending on go-libp2p-transport, go-libp2p-transport now depends on go-libp2p-net. My reasoning was that transports are one way to implement a libp2p networks. That's why this package has a However, go-libp2p-net contains the One solution is to:
Personally, I'm inclined to finish this refactor and punt on this. Unlike the current refactor, moving these interfaces around won't be a major operation. |
Stebalien
force-pushed
the
feat/refactor
branch
from
6634ab7
to
88e91d2
Compare
Stebalien
force-pushed
the
feat/refactor
branch
from
88e91d2
to
ee6b5b9
Compare
You don't really *use* this package in that way.
* explain that transports are fully-featured. * note how to use them (and link to the upgrader package).
…listening The one for listening is the reverse of the one for dialing.
DO NOT MERGE (yet)
Part of: libp2p/go-libp2p#297