Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add and link circuit relay spec #6

Merged
merged 5 commits into from
Feb 13, 2017
Merged

Add and link circuit relay spec #6

merged 5 commits into from
Feb 13, 2017

Conversation

ghost
Copy link

@ghost ghost commented Feb 8, 2017

Review of the spec itself concluded in libp2p/go-libp2p-circuit#1 -- the additional changes in this PR are purely linking it into place.

I chose to categorize the relay primarily as a transport, since that is what it looks like to the layers using it -- you use it by means of /p2p-circuit multiaddrs, just like other transports. The fact that it's also a swarm protocol is an implementation detail from this point of view.

@ghost ghost added the needs review label Feb 8, 2017
@ghost ghost requested a review from daviddias February 8, 2017 03:24
@ghost ghost added the in progress label Feb 8, 2017
@@ -13,6 +13,10 @@ It is recommended that implementations use one of the many NAT traversal librari

Unfortunately, due to symmetric NATs, container and VM NATs, and other impossible-to-bypass NATs, `libp2p` MUST fallback to relaying communication to establish a full connectivity graph. To be complete, implementations MUST support relay, though it SHOULD be optional and able to be turned off by end users.

Connection relaying SHOULD be implemented as a transport, in order to be transparent to upper layers.

For an instantiation of relaying, see the [/ipfs/relay/circuit transport](transports/circuit-relay.md).
Copy link
Member

Choose a reason for hiding this comment

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

/ipfs/ or /libp2p/?

@@ -71,6 +71,8 @@ Protocol multiplexing is done through [`multistream`](https://github.com/jbenet/

### 4.2.6 Relay

See [/ipfs/relay/circuit transport](transports/circuit-relay.md).
Copy link
Member

Choose a reason for hiding this comment

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

same as above?

7-properties.md Outdated
multiplex | /mplex/6.7.0 |
identify | /ipfs/id/1.0.0 |
ping | /ipfs/ping/1.0.0 |
relay | /ipfs/relay/line/0.1.0 | discontinued
Copy link
Member

Choose a reason for hiding this comment

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

just delete it, as it 'never actually started' :)

@daviddias
Copy link
Member

Still a great fan of the dramatization :) I would say that we should take this opportunity to make sure that the protocol strings are very clear and well scoped (e.g relay is a libp2p thing and not an ipfs one)

@daviddias
Copy link
Member

@lgierth I've updated the links and the multicodec in the multicodecs table. I understand that his spec is still a WIP, but let's make it a WIP with a placeholder in master, so that we have an entry point for it and then update the spec as the implemention develops. (Trying to avoid having 50 spec fragments everywhere)

@daviddias daviddias merged commit 2bfb69c into master Feb 13, 2017
@daviddias daviddias deleted the feat/relay branch February 13, 2017 16:06
@ghost
Copy link
Author

ghost commented Feb 13, 2017

Yes totally agree that we should merge this. While the spec is WIP and will be for a while longer, it's definitely good enough to be merged as a first draft.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant