The Circuit Relay Specification: The first iteration #15

daviddias · 2017-03-28T15:01:24Z

In an attempt to converge all the relay spec notes and discussions, I'm listing them here, so that they can all be considered for the revamp of the relay spec. (Order represents the timeline of how theses appear, last should be considered as last 'discussed')

first iteration of the spec - https://github.com/libp2p/specs/blob/master/relay/README.md
question: wire protocol message format - Clarify/confirm <src><dst><data> multistream flow in relay #12
question: relay vs tunnel - Clarify the difference between a tunnel and a relay #8
implementation notes - https://github.com/libp2p/js-libp2p-circuit/blob/master/implementation-notes.md
Why's concerns about current implementation - https://github.com/libp2p/js-libp2p-circuit/issues/8
Relay use cases - https://github.com/libp2p/js-libp2p-circuit/issues/6
moar notes - https://github.com/libp2p/js-libp2p-circuit/issues/2
moar discussion - https://github.com/libp2p/js-libp2p-circuit/issues/4
errors table: https://gist.github.com/whyrusleeping/7665164d04e43171e6cb7b8aa6205645#file-errors-md
wire protocol: https://gist.github.com/whyrusleeping/7665164d04e43171e6cb7b8aa6205645#file-relay-md

daviddias · 2017-03-28T15:06:50Z

relay/README.md

 - `/p2p-circuit`
  - Use relay discovery to find a suitable relay node. (Neither specified nor implemented.)
  - Listen for relayed connections.
  - Dial through the discovered relay node for any `/p2p-circuit` multiaddr.

-TODO: figure out forced addresses.
-TODO: figure out nested relayed connections.
+TODO: figure out forced addresses. -> what is forced addresses?


@lgierth Can you elaborate on what "forced addresses" are?

daviddias · 2017-03-28T15:12:23Z

relay/README.md

+- `/p2p/QmRelay/p2p-circuit`
+  - Same as previous example, but use peer routing to find an address for QmRelay.
+
+?? I believe we don't need this one:
 - `/p2p-circuit`
  - Use relay discovery to find a suitable relay node. (Neither specified nor implemented.)
  - Listen for relayed connections.
  - Dial through the discovered relay node for any `/p2p-circuit` multiaddr.


To me, the act of 'enabling relayed connections' is a config value and not a multiaddr. Do you agree @lgierth ?

@diasdavid @lgierth @whyrusleeping

I think it depends on how you look at relay. To me, relay is just another transport, hence making it behave like any other transport makes sense - here, I'm specifically talking about the dialer/listener (/stop) parts of the relay. The circuit-relay itself, can/should be controlled by config options.

Example:

Swarm: { Addresses: [ '/p2p-circuit/<multiaddr of the Relay>' // or just plain /p2p-circuit enables circuit for listening (/stop) and dialing ] }, Relay: { Circuit: { Enabled: false, // enable/disable circuit-relaying list-peers: false // I think this will be moved to its own proto that implements `ls` Proactive: false // should be renamed `Active`? (Active/Passive) naming } }

Edit:

Thinking about this a bit more, I think we should enable circuit transport by default, no need for an explicit p2p-circuit in the swarm addrs.

daviddias · 2017-03-28T15:22:17Z

relay/README.md

-  - The relay node will use peer routing to find an address for QmTwo.
+  - Also makes QmRelay available for relayed dialing, based on how listeners currently relate to dialers.
+- `/p2p/QmRelay/p2p-circuit`
+  - Same as previous example, but use peer routing to find an address for QmRelay.


Just to confirm, these addresses exist to have a way to make the node always dial to the relay peer (punch out), correct?

daviddias · 2017-03-28T15:27:50Z

From now on, all comments, reviews, edits and so on, should be applied directly to the document so that we keep everything in place :)

daviddias · 2017-03-28T15:55:25Z

relay/README.md

+| 260   | "passive relay has no connection to dst" | |
+| 261   | "active relay could not connect to dst: connection refused" | relay could not form new connection to target peer |
+| 262   | "could not open new stream to dst: BAD ERROR" | relay has connection to dst, but failed to open a new stream |
+| 270   | "<dst> does not support relay" | |


Do we really want to use 2XX+3XX for errors? Can we follow the pattern laid by HTTP:

- 1×× Informational - 2×× Success - 3×× Redirection - 4×× Client Error - 5×× Server Error

👍HTTP codes are well known, which makes them more intuitive and expected.

It doesnt map exactly though, i'm fine moving the 100 OK to 200 OK.

I guess we could map all the 200 errors to 400 'client' errors, and map all the 300 errors to 500 'server' errors

@whyrusleeping exactly what I was hoping for ❤️

@diasdavid @whyrusleeping

After, re-reading the table... They don't really map to HTTP at all... 200s are relay codes and 300 are dst errors. So, I think 1xx, 2xx, 3xx make sense here.

daviddias · 2017-03-28T15:58:21Z

relay/README.md

+| 261   | "active relay could not connect to dst: connection refused" | relay could not form new connection to target peer |
+| 262   | "could not open new stream to dst: BAD ERROR" | relay has connection to dst, but failed to open a new stream |
+| 270   | "<dst> does not support relay" | |
+| 320   | "src address too long" | |


From IRC:

14:13 <@daviddias> I don't understand why the 'send the src' addr keeps appearing as a thing for relay 16:05 <+dryajov> diasdavid re srs addr: I dont think we need it anymore, you are right. We can always do getPeerInfo on the circuited conn and should be able to get the src addrs. It would only make sense, if the full relay addr is sent so that the relay can be rebuilt from the dst to the src, if that ever makes sense? 16:23 <@daviddias> " It would only make sense, if the full relay addr is sent so that the relay can be rebuilt from the dst to the src, if that ever makes sense?" even in that case, we can always verify who dialed us with the getPeerInfo call 16:26 <+dryajov> True, that would be a swarm addr and should come back with the getPeerInfo. We don't need it.

dryajov · 2017-03-28T16:11:57Z

relay/README.md

+
+This opens the room for multiple hop relay, where the first relay is encapsulated in the seconf relay multiaddr, such as:
+
+`/p2p-circuit/p2p-circuit/<first relay>/<first hop multiaddr>/<destination peer multiaddr>`


To me it makes a lot more sense to do something like - /p2p-circuit<first relay>/<first hop multiaddr>/p2p-circuit<second relay>/<second hop multiaddr>/<destination peer multiaddr>

daviddias · 2017-03-28T16:38:08Z

relay/README.md

+Once it receives that, it checks if the status code is `OK`. If it is, it passes the new connection to its `NewConnHandler`.
+Otherwise, it returns the error message to the caller.
+
+### 'hop' protocol handler


By hop, you mean /libp2p/circuit/relay ?

No, i mean /libp2p/circuit/relay/1.0.0/hop

daviddias · 2017-03-28T16:38:47Z

relay/README.md

+<uvarint 'OK'><uvarint len(msg)><string "OK">
+```
+
+And passes the relayed connection into its `NewConnHandler`.


I'm confused by this stop handler. What case is it enabling exactly?

Thats the end point in the relay flow, hop is the relay listener, stop listens for relayed connections coming from hop. This is needed because you can be both a relay and listener on the same node - hence hop and stop sub protos.

Understood.

daviddias · 2017-03-28T17:09:46Z

One thing that the current spec is missing is versioning. Right now we have:

/libp2p/circuit/relay/hop
/libp2p/circuit/relay/stop

We the possibility of having a /libp2p/circuit/relay/ls, still on discussion.

If we were to add versioning, it makes it a tad complicated because it would create a version for each of these protocols (and we already know that assuming that something will never change is an antipattern™), ending up on:

/libp2p/circuit/relay/hop/1.0.0
/libp2p/circuit/relay/stop/1.0.0

This can be ok (and maybe a question of preference), but I would rather have a single /libp2p/circuit/relay/1.0.0 to maintain a single versioning over the circuit relay protocol, having the selection of {hop, stop, ls} as part of the RPC.

// cbor encoded json
{
  type: `hop`
  dstAddr: <some multiaddr>
}

It is not that we are trying to avoid RPC messages, we are already sending the multiaddr after first dial and we have the status codes going on.

whyrusleeping · 2017-03-28T17:15:11Z

relay/README.md

+
+A `/p2p-circuit` circuit address, is formated following:
+
+`/p2p-circuit[<relay peer multiaddr>]<destination peer multiaddr>`


maybe put slashes in here?

Also, after @lgierth clarifications this should be <relay peer multiaddr>/p2p-circuit/<destination peer multiaddr>

dryajov · 2017-03-28T17:20:41Z

@diasdavid re versioning:

<•lgierth>  /libp2p/relay/circuit/1.0.0/{hop,stop}
15:37 W<•whyrusleeping> because theyre sub-endpoints of the same protocol
15:38 L<•lgierth> yeah and they're different semantics on the same wire format
15:38 D<•dryajov> ah, sure… I had the impression that the version would always come at the end
15:39 that makes sense tho
15:39 L<•lgierth> well i guess until now it always has
15:39 "it's always been like this" is usually a good reason to do it differently eventually :D
15:39 D<•dryajov> problem is that now its non deterministic… if its always at the end, you can easily extract it, if its not, you need to define syntax around it
15:40 L<•lgierth> no you just go through all the mounted protocols you have, and pick the one which supports /libp2p/relay/circuit/$versionRange
15:40 W— •whyrusleeping checks his parsing code
15:40 L<•lgierth> hah yeah better check
15:40 W— •whyrusleeping is sure that this will break his parsing code
15:40 W<•whyrusleeping> but no worries
15:41 we have to update that package anyways
15:41 can fix it along the way
15:41 or just use explicit protocol matching for now
15:41 D<•dryajov> yeah… but… how do you know whats a version and whats not :D?
15:41 L<•lgierth> your mount protocols know
15:41 W<•whyrusleeping> Yeah, that does get hard
15:41 L<•lgierth> transports work similarly, just without a version
15:41 D<•dryajov> ah… I see what you mean
15:42 L<•lgierth> each transport has a Matches() func
15:42 D<•dryajov> yeah, you don’t really need to check anything, just do an exact match
15:42 :D

Boils down to /libp2p/relay/circuit/1.0.0/{hop,stop} with the version not coming at the end anymore. This works if you're doing exact matching, but makes it hard to extract if you try to do semantic versioning, specially since semver supports tags in version numbers.

whyrusleeping · 2017-03-28T17:39:33Z

what should the 'remote multiaddr' look like on a relay connection?

My proposal is:

<src maddr, from header>/p2p-circuit/QmRelayPeerID

daviddias · 2017-03-28T17:41:59Z

@whyrusleeping can you enlighten me on the src being something that is transferred here? See #15 (comment)

whyrusleeping · 2017-03-28T17:46:05Z

Irc logs for context: http://lets.git.sexy/relaynotes

dryajov · 2017-03-28T17:50:07Z

@whyrusleeping can you clarify what's the

src maddr
from header

in this - <src maddr, from header>

whyrusleeping · 2017-03-28T17:50:58Z

@dryajov I mean the source multiaddr that is sent by 'A' in their initial relay request

whyrusleeping · 2017-03-28T17:55:13Z

Another question, is it /p2p-relay/PEERID or /p2p-relay/<multiaddr> ?

dryajov · 2017-03-28T17:56:03Z

@whyrusleeping - both. In the first case it's peer routing, the second is over an explicit transport.

Oh, and its /p2p-circuit not p2p-crelay :)

daviddias · 2017-03-28T17:56:16Z

2017-03-27 17:48:29	+dryajov	hsanhuan brought some valid points there, basically, the idea is that you can rebuild the circuit from dst to src
2017-03-27 17:48:47	+dryajov	thats why src address is written to dst,
2017-03-27 17:48:54	+dryajov	otherwise it would work
2017-03-27 17:49:27	@whyrusleeping	Then we should go back to lgierth's original spec with both a src and a dst

I see you want to the full initial multiaddr, including any IPs, ports and so on. I question the usefulness of this, as you would only really need the peer Id (which you get from secio) and the relay addr (which you know, because you also have a conn established with it). But I guess it doesn't hurt.

dryajov · 2017-03-28T17:59:08Z

@whyrusleeping or do you mean,

/p2p-circuit/QmPeer (not a valid mr as it is right now?)
/p2p-circuit/ipfs/QmPeer - peer routing ma
/p2p-circuit/ip4/127.0.0.1/tcp/80/ipfs/QmPeer

?

whyrusleeping · 2017-03-28T18:00:14Z

as you would only really need the peer Id (which you get from secio)

No, we reallllllly have to stop assuming "we will just get it from secio". Thats an assumption that hurts us

I think its important to know where the connection is coming from. Though I think with the two separate protocols we could now get away with a single address, with different meaning per protocol (hop vs stop)

whyrusleeping · 2017-03-28T18:05:48Z

@dryajov

/p2p-circuit/QmPeer (not a valid mr as it is right now?)

/p2p-circuit/ipfs/QmPeer - peer routing ma

/p2p-circuit/ip4/127.0.0.1/tcp/80/ipfs/QmPeer

The issue here is that <anything>/ipfs/QmPeerID implies that this multiaddress is connecting to that peer. I think doing <anything>/p2p-circuit/QmPeerID makes the most sense, as its saying we are relaying through peer QmPeerID.

/p2p-circuit/ip4/127.0.0.1/tcp/80/ipfs/QmPeer is very confusing. Consider the following:
``/p2p-circuit/ip4/127.0.0.1/tcp/80/ipfs/QmPeer/p2p-circuit/ip4/127.0.0.1/tcp/123/ipfs/QmOtherPeer`

Am I relaying through QmPeer to QmOtherPeer? Or is this a relay whose exit point is QmOtherPeer?

In a diagram so i can more clearly explain myself:

A -> [QmPeer] -> [QmOtherPeer] -> ?
or
A -> [QmPeer] -> QmOtherPeer
or
A -> [QmPeer -> [QmOtherPeer]] -> ?

whyrusleeping · 2017-03-28T18:25:50Z

relay/README.md

+
+`/p2p-circuit[<relay peer multiaddr>]<destination peer multiaddr>`
+
+This opens the room for multiple hop relay, where the first relay is encapsulated in the seconf relay multiaddr, such as:


typo: s/seconf/second/

dryajov · 2017-03-29T19:08:31Z

relay/README.md

+
+Specify a relay:
+
+- `/p2p-circuit/p2p/QmRelay/p2p/QmTwo`


Should be:

/p2p/QmRelay/p2p-circuit/p2p/QmTwo

dryajov · 2017-03-29T19:09:06Z

relay/README.md

+  - Dial QmTwo, through QmRelay.
+  - Use peer routing to find an address for QmRelay.
+  - The relay node will also use peer routing, to find an address for QmTwo.
+- `/p2p-circuit/ip4/../tcp/../p2p/QmRelay/p2p/QmTwo`


Should be:

/ip4/../tcp/../p2p/QmRelay/p2p-circuit/p2p/QmTwo

dryajov · 2017-03-29T20:46:14Z

relay/README.md

+- R writes `/ipfs/QmB` on `sRB`
+- B receives stream `sRB` and reads `/ipfs/QmB` from it.
+- B sees that the multiaddr it read is its own and chooses to handle this stream as an endpoint instead of attempting to relay further
+- TODO: step for R to send back status code to A


Thoughts on status codes:

R doesn't send any success codes to A, only failure codes

R would potentially also send err codes from B if the circuit is not yet established

A would get a success or failure code from B once the circuit is established

* feat: adding err code 280 "can't relay to itlsef" * doc: reworded 280 description

dryajov

I've added some more comments and clarifications. Lets get this reviewed one more time.

daviddias · 2017-05-08T18:22:11Z

I've reviewed:

Overview
Dramatization
Addressing

Missing:

Wire Protocol review Wire protocol section from the relay spec #18
Interfaces
Implementation details

I'll review these last 3 in separate PRs to this one to make sure I don't merge anything to the PR without having everyone in the same page.

ghost · 2017-05-08T18:27:26Z

Three comments, then LGTM 👍

daviddias · 2017-05-09T17:41:35Z

Applied @lgierth review, now onto the remaining bits.

daviddias · 2017-05-09T18:44:31Z

#18 up for review

ghost

Let's merge this one, thanks @diasdavid!

dryajov

Lets get this merged! 👍

daviddias · 2017-05-21T19:12:18Z

Not yet, #15 (comment) needs to happen. The spec is not ready.

ghost · 2017-05-22T14:45:47Z

@diasdavid there's an inaccurate spec already merged though -- this PR is a great improvement over the existing document, and isn't technically blocked by the remaining tasks. Anyhow I added a note about this work-in-progress in #20 as an alternative.

review Wire protocol section from the relay spec

daviddias · 2017-07-08T10:20:18Z

A new chapter

Following up the discussion on IRC (#ipfs-dev@freenode), I'll be working on finishing this spec to my best understanding and contemplating all the improvements discussed. Once this revamp is done, I'll open a new PR for formal review.

daviddias · 2017-07-08T16:07:45Z

It is here! #22

Guess what, using a protobuf simplifies so much the 'under the microscope section' :)

daviddias added 2 commits March 28, 2017 15:04

first pass

63897d6

update addresses examples

4c5a51c

daviddias added the in progress label Mar 28, 2017

daviddias mentioned this pull request Mar 28, 2017

One 'relay spec' to rule them all #14

Closed

daviddias commented Mar 28, 2017

View reviewed changes

add error table

b74a39e

daviddias commented Mar 28, 2017

View reviewed changes

dryajov reviewed Mar 28, 2017

View reviewed changes

update wire format section with why notes

5ad252d

daviddias commented Mar 28, 2017

View reviewed changes

whyrusleeping reviewed Mar 28, 2017

View reviewed changes

dryajov reviewed Mar 29, 2017

View reviewed changes

relay/README.md Outdated

Specify a relay:

- `/p2p-circuit/p2p/QmRelay/p2p/QmTwo`

Copy link

Member

dryajov Mar 29, 2017

Choose a reason for hiding this comment

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

Should be:

/p2p/QmRelay/p2p-circuit/p2p/QmTwo

dryajov reviewed Mar 29, 2017

View reviewed changes

Feat/update relay spec (#16)

d65d541

* feat: adding err code 280 "can't relay to itlsef" * doc: reworded 280 description

dryajov approved these changes Mar 30, 2017

View reviewed changes

dryajov suggested changes Mar 30, 2017

View reviewed changes

ghost self-requested a review April 14, 2017 19:42

dryajov mentioned this pull request Apr 16, 2017

WIP - Awesome Relay Endeavour ipfs/js-ipfs#833

Closed

9 tasks

reviewed Overview, Dramatization and Addressing

5105b41

apply lars CR

9ca982b

review Wire protocol section from the relay spec

1759516

ghost approved these changes May 10, 2017

View reviewed changes

dryajov approved these changes May 20, 2017

View reviewed changes

Merge pull request #18 from libp2p/review/relay/wire-protocol

76dfb72

review Wire protocol section from the relay spec

daviddias changed the title ~~Update the relay spec~~ The Circuit Relay Specification Jul 8, 2017

Merge branch 'master' into feat/update-relay-spec

a758d06

daviddias changed the title ~~The Circuit Relay Specification~~ The Circuit Relay Specification: The first iteration Jul 8, 2017

daviddias closed this Jul 8, 2017

daviddias mentioned this pull request Jul 8, 2017

The Circuit Relay Specification #22

Merged

dryajov mentioned this pull request Aug 17, 2017

P2P Circuit Advertisements #33

Open

dryajov mentioned this pull request Sep 1, 2017

Awesome Endeavour: Circuit Relay ipfs/js-ipfs#830

Merged

9 tasks

daviddias deleted the feat/update-relay-spec branch February 20, 2018 22:25


		This opens the room for multiple hop relay, where the first relay is encapsulated in the seconf relay multiaddr, such as:

		`/p2p-circuit/p2p-circuit/<first relay>/<first hop multiaddr>/<destination peer multiaddr>`


		A `/p2p-circuit` circuit address, is formated following:

		`/p2p-circuit[<relay peer multiaddr>]<destination peer multiaddr>`

The Circuit Relay Specification: The first iteration #15

The Circuit Relay Specification: The first iteration #15

Conversation

daviddias commented Mar 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dryajov Mar 29, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daviddias commented Mar 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dryajov Mar 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daviddias commented Mar 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dryajov commented Mar 28, 2017 • edited Loading

whyrusleeping commented Mar 28, 2017

daviddias commented Mar 28, 2017

whyrusleeping commented Mar 28, 2017

dryajov commented Mar 28, 2017 • edited Loading

whyrusleeping commented Mar 28, 2017

whyrusleeping commented Mar 28, 2017

dryajov commented Mar 28, 2017 • edited Loading

daviddias commented Mar 28, 2017

dryajov commented Mar 28, 2017

whyrusleeping commented Mar 28, 2017

whyrusleeping commented Mar 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dryajov Mar 29, 2017 • edited Loading

Choose a reason for hiding this comment

dryajov left a comment

Choose a reason for hiding this comment

daviddias commented May 8, 2017 • edited Loading

ghost commented May 8, 2017

daviddias commented May 9, 2017

daviddias commented May 9, 2017

ghost left a comment

Choose a reason for hiding this comment

dryajov left a comment

Choose a reason for hiding this comment

daviddias commented May 21, 2017 • edited Loading

ghost commented May 22, 2017

daviddias commented Jul 8, 2017 • edited Loading

A new chapter

daviddias commented Jul 8, 2017

dryajov Mar 29, 2017 •

edited

Loading

dryajov Mar 28, 2017 •

edited

Loading

dryajov commented Mar 28, 2017 •

edited

Loading

dryajov commented Mar 28, 2017 •

edited

Loading

dryajov commented Mar 28, 2017 •

edited

Loading

dryajov Mar 29, 2017 •

edited

Loading

daviddias commented May 8, 2017 •

edited

Loading

daviddias commented May 21, 2017 •

edited

Loading

daviddias commented Jul 8, 2017 •

edited

Loading