Peer dialing to self

[hsanjuan]

Is it do-able to enable peers to dial themselves?

We have Raft using a libp2p-transport and it wants to dial itself all the time. It does not seem to cause undesired side-effects, but it does not seem crazy that any cluster-application using RPCs ends up trying to contact itself for some things. (same issue in ipfs-cluster RPC, but we worked around it).

Perhaps we can provide some shortcut at a higher level so that a peer contacting itself does not even attempt touch the network?

Pointers appreciated...

[vyzo]

perhaps we should have a virtual network interface for this?
I don't think it makes sense to dial oneself and setup the entire network stack, it's a smell.

[hsanjuan]

If you point me to the right repos, I can put it on my to-do list

[vyzo]

@Stebalien thoughts and pointers?

[Stebalien]

So, we'd have to be very careful here. All of our services (DHT, bitswap, identify, etc.) will break if we start dialing ourself.

Somewhat off topic...

We may need some form of "self dial" in the future for a libp2p daemon however, it won't look like a true self-dial. Basically, given a daemon and some service registered to handle protocol A, the some other local service may ask the daemon to dial itself and then speak protocol A. However, this is really just a special case that needs to be handled by the libp2p daemon dialer proxy.

[hsanjuan]

We may need some form of "self dial" in the future for a libp2p

I need it, now..

Happy if it is a non-default option.

[bigs]

@hsanjuan @Stebalien Would this have a special API (i.e. DialSelf) or do we want to be able to dial our own peer ID? Perhaps we make an option for the latter and fail to dial ourselves when it's not enabled.

[hsanjuan]

We want to be able to dial our own peer ID.

[Stebalien]

but it does not seem crazy that any cluster-application using RPCs ends up trying to contact itself for some things

In my experience, self-RPCs tend to cause more trouble than they're worth (mostly deadlocks).

I assume you're trying to vote for yourself? The correct way to do this is to just vote for yourself internally before sending any RPCs. You should be doing this anyways so that you don't ask a bunch of peers to vote for you but then end up voting for a different peer.

---

Basically, we can support this but I don't want to build you a footgun.

[hsanjuan]

I didn't implement Raft, we just plugged-in the libp2p transport. I haven't looked exactly why it tries to contact itself, but it does and produces a continues stream of errors (even if it seems to work as normal).

The thing is you normally you do Broadcast(op, Peers()) and that Peers() includes yourself. And this is just more streamlined than broadcasting to all peers except yourself and then (or before) calling the operation in yourself internally and then merging the results of all the operations. Or simply you have chosen a random peer to do an operation and it turns out to be yourself.

We handled this "doing things internally" in cluster for cluster RPC, but things would be nicer if the shortcut happened in libp2p.

[Stebalien]

@hsanjuan so, I take it this is blocking you?

[hsanjuan]

@Stebalien not blocking me, but I'd like to address it rather sooner than later. I'm willing to take the stab if given a few pointers.

[Stebalien]

Pointers:

1. Add a swarm and libp2p option to enable dialing self.
2. Add a special immortal loopback conn to the connection set on init.

Given the fact that the connection is immortal, we'll have to be careful with things that expect to be able to close it.

[phritz]

This issue prevents filecoin miners from making deals with themselves, which is functionality that we'd really like to support. We'd love to be able to dial self to enable that functionality, see filecoin-project/venus#1917.

[raulk]

Solution proposal

First of all, let me point out that self-dialling sounds like a bit of a smell. There are usually more efficient ways of carrying out a transaction with yourself (by tackling this in business logic). However, I do recognise that requires special casing based on identity, so I lean towards providing a solution inside libp2p seeing that two projects already require this functionality, so there's likely more to come.

In projects like Apache Camel, such abstraction was achieved by a special in-memory transport. We can find inspiration in that.

Concretely I propose the following high-level design:

1. Make self-dialling an explicit opt-in option in the libp2p Host. Percolate this option down to the swarm.
2. When enabled, attach a new, in-memory transport to the host on construction with two sides: dialling and listening.
3. When enabled, host.Connect() calls to self will always succeed.
4. Make protocols supporting self-dialling declare so explicitly:
   • either when registering themselves by extending SetStreamHandler(pid protocol.ID, handler inet.StreamHandler) with an Option... arg.
   • by adding a method somewhere else: EnableSelfDial(pid protocol.ID).
5. Swarm: calls host.NewStream(context.Context, peer.ID, protocol.ID) with peer.ID = us will error if the protocol.ID has not been enlisted as supporting self-dial, or will always succeed if it did. Behind the scenes, we're establishing an io.Pipe. I think we'll need a custom upgrader to bypass encryption and muxing negotiation.

@Stebalien @hsanjuan – thoughts?

[Stebalien]

How should network.ClosePeer(self) behave? Should we always have an implicit connection to ourself.

[raulk]

@Stebalien IMO network.ClosePeer(self) should close all streams and keep the implicit connection "alive". In the model I proposed, there's really no connection, it's just a transport that's acting like a bridge, tracking open streams and piping bytes for each, and it's "always on".

If self-dialling is enabled host-level:

1. Calling host.Connect(self) always succeeds, and nothing really happens underneath.
2. The swarm network.DialPeer(self) short-circuits always succeeding, as the connection is implicit.
   • We return a new implementation of tpt.Conn that does the heavy-lifting: it opens in-memory streams returning a custom implementation of inet.Stream that's watching the protocol negotiation and doing a Reset() if a non-participating protocol is negotiated.
3. Identify returns all protocols supporting self-dial.
4. We need to define the behaviour of SetConnHandler(), and probably a few other methods, under this scenario.

---

EDIT: added more colour in point 2.

[Stebalien]

SGTM. Maybe we should just throw away SetConnHandler and use notifications?

[hsanjuan]

4. Make protocols supporting self-dialling declare so explicitly:
   either when registering themselves by extending SetStreamHandler(pid protocol.ID, handler inet.StreamHandler) with an Option... arg.
   by adding a method somewhere else: EnableSelfDial(pid protocol.ID).

Seems redundant to have to declare the peer as self-dialing, and then the protocols, from a user point of view. Allowing or disallowing stream handling depending on its source sounds very much like adding an authorization mechanism (which could be generalized into whitelist/blacklists?).

I don't know if there is a use-case for declaring a peer as self-dialable and then disallowing most protocols. Unless we are scared of current protocols causing some unintended breakage, we can just simply process those streams as normal (when self-dialing is enabled for the host). The user can always choose to abort such stream their handlers, as needed. This would also simplify implementation a little bit.

In any case, the proposed solution also works for us.

[raulk]

The rationale is that protocol handlers rely on the assumption that incoming streams are never local. Suddenly allowing any protocol to receive local streams is a breakage of implied contract, and will cause regressions. Some of those protocols we control within libp2p (DHT, Pubsub, Relay); most we don’t. That’s why — IMO — protocols should opt into this behaviour.

[Stebalien]

The rationale is that protocol handlers rely on the assumption that incoming streams are never local.

Honestly, we should fix these services. Really, I think the main issue is that we need:

1. Peers() to return peers (not including self).
2. PeerConnected/PeerDisconnected events that notify us when some other node transitions to/from the "connected" state.

That should fix most of the issues around dialing self.