Skip to content
This repository has been archived by the owner on Sep 9, 2022. It is now read-only.

Update for transport refactor #33

Merged
merged 3 commits into from
Jun 6, 2018
Merged

Update for transport refactor #33

merged 3 commits into from
Jun 6, 2018

Conversation

Stebalien
Copy link
Member

@Stebalien Stebalien commented Mar 13, 2018

DO NOT MERGE (yet)
Part of: libp2p/go-libp2p#297

(tests failing as gx deps need updating)

@ghost ghost assigned Stebalien Mar 13, 2018
@ghost ghost added the status/in-progress In progress label Mar 13, 2018
@Stebalien Stebalien requested a review from vyzo March 14, 2018 19:49
@Stebalien Stebalien force-pushed the feat/refactor branch 11 times, most recently from 501ede1 to 58c1e1e Compare March 15, 2018 05:29
panic(err)
}

for _, tp := range tps {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there equivalent code for this or is it handled in a different manner now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's handled by the swarm internally. See: libp2p/go-addr-util#11

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the swarm explicitly aware of relay addresses or does it ask the transports dynamically to respond whether they support the address?
If so, how are encapsulations handled?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The swarm asks its transports.

For listening, we just figure out which transport is responsible using the algorithm described here and ask it to listen on the address.

For dialing, we lookup the appropriate transport (same algorithm) and call the CanDial function on it (code). We could just try dialing but this way allows us to filter out addresses we definitely can't dial higher up the stack.

Copy link
Contributor

@vyzo vyzo Mar 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not clear it will have the same behaviour -- for example we want to pick up relay for /ip4/X.X.X.X/tcp/YY/ipfs/Qmfoo//p2p-circuit/ip4/X.X.X.X/tcp/YY/ipfs/Qmbar (that's explicit relay dialing).
What will the algorithm pick here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how that's related to this function but it will pick the relay transport. It'll:

  1. Iterate over the protocols (ip4, tcp, ipfs, p2p-circuit, ip4, tcp, ipfs).
  2. When it gets to the p2p-circuit protocol, it will notice that the associated transport is a Proxy transport and will pass the address off to that transport.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, being a proxy is the key feature.

@Stebalien Stebalien force-pushed the feat/refactor branch 3 times, most recently from deb6334 to 22305ab Compare March 22, 2018 03:29
transport.go Outdated
if !t.Matches(laddr) {
func (t *RelayTransport) Listen(laddr ma.Multiaddr) (tpt.Listener, error) {
protos := laddr.Protocols()
if len(protos) == 0 || protos[len(protos)-1].Code == P_CIRCUIT {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am confused by this -- you fail with "not a relay address" if it's the passive relay address?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting... So, I intended to write != however, that wouldn't have worked because we listen with /p2p-circuit/ipfs/QmId. Really, it should support both.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I wanted the check to be a bit more specific than "has a p2p-circuit protocol". However, I'll have to consider carefully how to do this properly.

@Stebalien Stebalien force-pushed the feat/refactor branch 2 times, most recently from c799cc4 to 7fcacee Compare March 26, 2018 21:40
@Stebalien Stebalien requested a review from vyzo March 27, 2018 00:53
@@ -54,8 +57,9 @@ func (e RelayError) Error() string {
return fmt.Sprintf("error opening relay circuit: %s (%d)", pb.CircuitRelay_Status_name[int32(e.Code)], e.Code)
}

func NewRelay(ctx context.Context, h host.Host, opts ...RelayOpt) (*Relay, error) {
func NewRelay(ctx context.Context, h host.Host, upgrader *tptu.Upgrader, opts ...RelayOpt) (*Relay, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where does one get a transport upgrader?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@vyzo vyzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alright, this looks good.

@Stebalien Stebalien force-pushed the feat/refactor branch 2 times, most recently from e603515 to c24509e Compare March 29, 2018 08:28
@Stebalien Stebalien merged commit c7790e8 into master Jun 6, 2018
@ghost ghost removed the status/in-progress In progress label Jun 6, 2018
@Stebalien Stebalien deleted the feat/refactor branch June 6, 2018 07:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants