P2P Circuit Addressing #72

Stebalien · 2018-06-12T19:21:56Z

Currently in go-libp2p, when we encounter a p2p circuit multiaddr we treat the entire multiaddr as part of the transport (including the /ipfs/QmId part).

Basically, we considered /p2p-circuit/ipfs/QmId to be short for /p2p-circuit/ipfs/QmId/ipfs/QmId (where the first is the target for the circuit relay and the second is the peer) which would resolve to /p2p-circuit/ip4/.../tcp/.../ipfs/QmId. However, according to the p2p-circuit multiaddr definition, this is incorrect; the /p2p-circuit/ multiaddr takes no arguments.

We did this because we (used to) only pass the "transport" part to the transport. We didn't pass the peer ID.

However, now that we've refactored go transports, we do pass in the peer ID.

After reconsidering, I realized that treating /p2p-circuit/ as the full transport is probably more correct. For example, when we encounter /ipfs/QmId by itself in the swarm, we don't say that the "transport" part is, /ipfs/QmId, we say that no transport-level address has been specified and we look it up. In this case, the /p2p-circuit/ transport has been (completely) specified but the transport of the connection after the the circuit part has not (it's not /ipfs/QmId).

I bring this up because:

It's a wart that I've never fully been able to get over.
We implement this as a special-case hack in go-ipfs-addr.
We don't have this hack in go-libp2p-peerstore.InfoFromP2pAddr. At the very least, we need to be consistent.

Proposed solution: Stop doing this. That is, treat /p2p-circuit/ as a full transport address (like /ip4/xxx/tcp/yyy). This will simplify a bunch of code and remove a nasty hack.

Downside: We have to be careful not to break published PeerInfo records. That is, if an old go-ipfs node (not sure about js) sees:

PeerInfo {
  ID: "QmId",
  Addrs: ["/p2p-circuit/"],
}

It won't be able to use this p2p circuit address.

Note: removing the hack in go-ipfs-addr won't introduce this problem as long as the go-libp2p-circuit transport continues to return the full /p2p-circuit/ipfs/QmId addresses.

@dryajov how does js-libp2p handle this? If I give js-libp2p an address of the form /p2p-circuit/ipfs/QmId, what kind of PeerInfo struct would I get back out?

The text was updated successfully, but these errors were encountered:

dryajov · 2018-06-13T20:16:05Z

This is a bit of a rabbit hole, but I'm glad this conversation is starting 👍

In general I agree that /p2p-circuit is a valid address, tho on its own it's not necessarily clear what the address represents. Can we come up with a concise meaning for it?

To me, it always meant that this peer is able to accept relayed connections - but afaik, that isn't documented anywhere and its very arbitrary at best. Moreover, a number of questions pop up immediately, such as - which addresses (transports) does this peer listen on, all of them (/p2p-circuit/ipfs/QmPeer) or specific transports only (/p2p-circuit/ip4/.../tcp/.../ipfs/QmId), etc...

As far as the js implementation goes, we should be fine, I made sure that /p2p-circuit was a valid address.

Stebalien · 2018-06-13T22:07:16Z

In general I agree that /p2p-circuit is a valid address, tho on its own it's not necessarily clear what the address represents. Can we come up with a concise meaning for it?

So, there are two questions here:

The "transport part" is /p2p-circuit versus /p2p-circuit/ipfs/QmId
The "transport part" must name the relay (/p2p-circuit/... versus /ipfs/QmRelay/p2p-circuit/...)

The second one basically says "just use some circuit". IMO, this is only useful on the CLI where a user may want to be able to specify "please use a relay". That's not possible on go right now but should be.

The first one is the question at hand. Basically, I see the the transport part as telling libp2p to:

Connect to the "transport part".
From there, connect to the peer.

So,

For /ip4/.../tcp/.../ipfs/QmId, the transport part (/ip4/.../tcp/...) says:
1. "connect to /ip4/.../tcp/..."
2. "connect to the peer QmId"
For /ipfs/QmRelay/p2p-circuit/ipfs/QmId, the transport part (/ipfs/QmRelay/p2p-circuit) says:
1. "connect to /ipfs/QmRelay"
2. "connect to the p2p-circuit service"
3. "tell it to connect to QmId"
For /ipfs/QmRelay/p2p-circuit/ip4/xxx/tcp/yyy/ipfs/QmId, the transport part (/ipfs/QmRelay/p2p-circuit/ip4/xxx/tcp/yyy) says:
1. "connect to /ipfs/QmRelay"
2. "connect to the p2p-circuit service"
3. "tell it to connect to QmId through /ip4/xxx/tcp/yyy"

Concisely, /p2p-circuit, means "use some relay". /ipfs/QmRelay/p2p-circuit means "use relay QmRelay".

ghost · 2018-09-25T15:30:08Z

Agreed to all you said in your second comment @Stebalien -- everything that comes before the last /p2p (or /ipfs) part, is the transport to get to said node.

Let's collect the places where this matters, e.g. go-ipfs-addr, and figure out how to maneuver out of this.

vyzo · 2018-09-25T16:46:36Z

As the author of the hack in go-ipfs-addr, I concur. Let's be consistent.

ghost · 2018-09-26T16:53:22Z

My preliminary hackery is here: libp2p/go-libp2p-circuit#48

The original reason for having the /ipfs/QmFoo part in the transport address, isn't relevant anymore, since Transport.Dial() now also takes a PeerID.

The PR doesn't quite work yet.

Stebalien · 2018-09-26T16:56:05Z

ipfs swarm addrs listen currently spits out:

/ip4/127.0.0.1/tcp/4001
/ip4/127.0.0.1/udp/4001/quic
/ip4/192.168.0.129/tcp/4001
/ip4/192.168.0.129/udp/4001/quic
/ip6/2601:647:5801:4add:25da:afaa:316a:b04c/tcp/4001
/ip6/2601:647:5801:4add:25da:afaa:316a:b04c/udp/4001/quic
/ip6/::1/tcp/4001
/ip6/::1/udp/4001/quic
/p2p-circuit/ipfs/QmS3zcG7LhYZYSJMhyRZvTddvbNUqtt8BJpaSs6mi1K5Va

Notice:

The non-relay addresses don't include a peer ID.
The peer ID included in the relay address is my peer ID, not a relay's peer ID.

If we were to listen on a specific relay, we'd get:

/ipfs/QmMyChosenRelay/p2p-circuit/ipfs/QmS3zcG7LhYZYSJMhyRZvTddvbNUqtt8BJpaSs6mi1K5Va

With the changes proposed here, we'd now list:

/ip4/127.0.0.1/tcp/4001
/ip4/127.0.0.1/udp/4001/quic
/ip4/192.168.0.129/tcp/4001
/ip4/192.168.0.129/udp/4001/quic
/ip6/2601:647:5801:4add:25da:afaa:316a:b04c/tcp/4001
/ip6/2601:647:5801:4add:25da:afaa:316a:b04c/udp/4001/quic
/ip6/::1/tcp/4001
/ip6/::1/udp/4001/quic
/p2p-circuit
/ipfs/QmMyChosenRelay/p2p-circuit # with a "chosen" relay

Regardless, nether ipfs swarm peers nor ipfs id would change. We'd continue printing /ip4/.../tcp/.../ipfs/QmRelay/p2p-circuit/ipfs/QmTarget.

ghost · 2018-09-26T17:38:21Z

ipfs id nothing is printed here yet -- we should eventually add the /p2p-circuit addr to Host.Addrs(). In cases where we want to listen on circuit, but not announce that, we'll set Addresses.NoAnnounce = ["/p2p-circuit"] as with any other address.

ghost · 2018-10-02T03:44:36Z

Let's collect the places where this matters, e.g. go-ipfs-addr, and figure out how to maneuver out of this.

These three are ready for CR:

Then need to bubble the changes up to go-ipfs.

ghost · 2018-10-02T04:00:56Z

You can also try this out in go-ipfs by doing gx-go link go-libp2p-circuit go-libp2p-swarm go-ipfs-addr (don't forget gx-go link -r -a afterwards).

vyzo · 2018-10-02T08:45:47Z

Also, can we finally add P_CIRCUIT to multiaddr?

ghost · 2018-10-02T16:29:07Z

Also, can we finally add P_CIRCUIT to multiaddr?

I'm already way deep into this rabbit hole, that will have to happen another time :) A simple update to go-ipfs-config brought me here and I need to finally wrap that up.

ghost · 2018-10-03T06:24:39Z

I've worked through the update, but unfortunately there are more unrelated changes neccessary: ipfs/kubo#5553

Help very welcome 🚁

vyzo · 2018-10-06T08:12:49Z

So what's our compatibility story?
When we deploy the change, there will still be older nodes using full addresses with the peer included in the multiaddr.
I think we need a compatibility shim so that we can still dial those addrs.

vyzo · 2018-10-06T08:16:24Z

Seems the shim should be unnecessary and the dialer will still work though (by looking at the code).

We used to have to do this as these addresses had `/ipfs/QmId` appended at the *transport* layer. However, we were able to remove this with the transport refactor so we can now remove this check. fixes libp2p/specs#72 License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

Stebalien assigned ghost , dryajov, daviddias and whyrusleeping Jun 12, 2018

This was referenced Sep 27, 2018

Announce relay addresses by default ipfs/kubo#4830

Closed

[RFC] Delay dials to relay addresses libp2p/go-libp2p-swarm#57

Open

Stebalien mentioned this issue Sep 28, 2018

deprecate package ipfs-inactive/go-ipfs-addr#6

Closed

Stebalien mentioned this issue Oct 11, 2018

Use ipfs-addr to parse IPFS addrs filecoin-project/venus#1119

Closed

Stebalien mentioned this issue Oct 11, 2018

Use ipfs-addr to parse IPFS addrs filecoin-project/venus#1120

Merged

Stebalien mentioned this issue Oct 25, 2018

remove p2p-circuit addr hack in ipfs swarm peers ipfs/kubo#5645

Merged

Stebalien closed this as completed in ipfs/kubo#5645 Oct 25, 2018

Stebalien mentioned this issue Feb 14, 2019

Initial implementation ipfs/go-ipfs-http-client#1

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

P2P Circuit Addressing #72

P2P Circuit Addressing #72

Stebalien commented Jun 12, 2018

dryajov commented Jun 13, 2018

Stebalien commented Jun 13, 2018 •

edited

Loading

ghost commented Sep 25, 2018

vyzo commented Sep 25, 2018

ghost commented Sep 26, 2018

Stebalien commented Sep 26, 2018 •

edited

Loading

ghost commented Sep 26, 2018

ghost commented Oct 2, 2018 •

edited by ghost

Loading

ghost commented Oct 2, 2018

vyzo commented Oct 2, 2018

ghost commented Oct 2, 2018

ghost commented Oct 3, 2018

vyzo commented Oct 6, 2018

vyzo commented Oct 6, 2018

P2P Circuit Addressing #72

P2P Circuit Addressing #72

Comments

Stebalien commented Jun 12, 2018

dryajov commented Jun 13, 2018

Stebalien commented Jun 13, 2018 • edited Loading

ghost commented Sep 25, 2018

vyzo commented Sep 25, 2018

ghost commented Sep 26, 2018

Stebalien commented Sep 26, 2018 • edited Loading

ghost commented Sep 26, 2018

ghost commented Oct 2, 2018 • edited by ghost Loading

ghost commented Oct 2, 2018

vyzo commented Oct 2, 2018

ghost commented Oct 2, 2018

ghost commented Oct 3, 2018

vyzo commented Oct 6, 2018

vyzo commented Oct 6, 2018

Stebalien commented Jun 13, 2018 •

edited

Loading

Stebalien commented Sep 26, 2018 •

edited

Loading

ghost commented Oct 2, 2018 •

edited by ghost

Loading