[Identify] How to integrate signed peer records?

[thomaseizinger]

The /identify protocol was conceived before signed peer records were added.

Is there a plan of integrating signed peer records into /identify? I think there are a number of ways:

1. Create /identify/1.1.0 where listenAddrs is deprecated and a new field listenPeerRecords is added.
2. Create an entirely new protocol that is built on top of peer records instead of plain addresses and simply deprecate the old identify protocol.

(1) could be implemented in a backwards-compatible way where upgraded peers always populate both fields.
(2) could also be a good chance to do #45

Given the simplicity of the identify protocol, it might just be easier to design a completely new one. Nodes can always stay backwards compatible by supporting both protocols simultaneously.

[mxinden]

Is there a plan of integrating signed peer records into /identify?

Yes, very much, i.e. it is on our roadmap (see ### Peer Routing Records in #339).

As far as I can tell go-libp2p already does via a combination of the two suggestions above, more specifically via:

• A new protocol name: /p2p/id/1.1.0.
• A new field in the existing ProtoBuf: optional bytes signedPeerRecord = 8;.

See libp2p/go-libp2p#747 for details.

Unfortunately the identify protocol specification has not been updated.

[thomaseizinger]

A new protocol name: /p2p/id/1.1.0.

Is there any reasoning why version 1.1.0 was chosen? Is there a /p2p/id/1.0.0 as well?

Unfortunately the identify protocol specification has not been updated.

I might set out and try to update the spec then.

[mxinden]

Is there any reasoning why version 1.1.0 was chosen? Is there a /p2p/id/1.0.0 as well?

I would guess that /p2p/id/1.1.0 was chosen instead of /p2p/id/1.0.0 to avoid confusion with /ipfs/id/1.0.0.

I might set out and try to update the spec then.

That would be very much appreciated. Thank you Thomas!

[thomaseizinger]

I tried to update the spec and I am in-between two minds.

For one, we are defining a completely new protocol here (i.e. /p2p/id/1.1.0) but at the same time, what the go implementation defines as /p2p/id/1.1.0 carries old legacy stuff around like listenAddrs and un-specified functionality like delta.

I am tempted to - instead of specifying go's variation of identify as /p2p/id/1.1.0 - specify a new protocol /p2p/id/2.0.0 that removes old legacy fields and is purely based on signed peer records.

Thoughts?

[Stebalien]

A new protocol name: /p2p/id/1.1.0.

It doesn't have a new protocol ID. Unfortunately, because we run identify before we know the peer's protocols (at the moment) negotiating between two versions would take a full round-trip and add a round-trip to connection initiation.

Instead, we just added the field to the the protocol. It's backwards compatible, and old versions will simply ignore it.

[mxinden]

I think the move to signed peer records in the identify protocol should happen in a non-breaking way at the Protocol Buffer level and not in a "breaking" way at the protocol name level. In other words, I am in favor of the go-libp2p way, adding the signedPeerRecord to the Protocol Buffer schema instead of changing the protocol name.

I agree that the additional field in the Protocol Buffer schema adds complexity to the protocol, though I would argue that (a) complexity is rather small, especially since we can ultimately deprecate the old listenAddrs field and (b) the work required to role out a new protocol, while not very large, still outweights the complexity of the additional signedPeerRecord field.

Given that listenAddrs is a repeated field and thus can be left empty once most nodes of a network use signedPeerRecord I don't think the non-breaking approach (adding the signedPeerRecord field) has any notable performance impact.

With the above in mind, what do you think @thomaseizinger? I am sorry for (a) this not being properly specified before rolled out in go-libp2p and (b) the extra work you already did in #350.

[thomaseizinger]

It doesn't have a new protocol ID.

Alright, that completely changes the game then :)

I'll drop #350 then and update the spec to reflect what /ipfs/id/1.1.0 is.

Migration to a new protocol identifier can be done at a later stage (if ever).

[mxinden]

I'll drop #350 then and update the spec to reflect what /ipfs/id/1.1.0 is.

Friendly ping. Help here would still be very much appreciated @thomaseizinger 😇

[thomaseizinger]

I'll drop #350 then and update the spec to reflect what /ipfs/id/1.1.0 is.

Friendly ping. Help here would still be very much appreciated @thomaseizinger 😇

Thanks! I did not forget about it, just got a lot on my plate at the moment :)

Safe to say that this is up for grabs if anyone else wants to pick it up 😅

[GGAlanSmithee]

@thomaseizinger

Migration to a new protocol identifier can be done at a later stage (if ever).

Speaking as a new user of libp2p, the remaining references to ipfs is a bit confusing. Of course, I do not know the techincal difficulties of changing the protocol id to use p2p instead of ipfs, but disregarding that, I think using p2p would add clarity, given that p2p is used in multiaddr and other protocols.

[thomaseizinger]

A new protocol name: /p2p/id/1.1.0.

It doesn't have a new protocol ID. Unfortunately, because we run identify before we know the peer's protocols (at the moment) negotiating between two versions would take a full round-trip and add a round-trip to connection initiation.

Instead, we just added the field to the the protocol. It's backwards compatible, and old versions will simply ignore it.

@Stebalien Are there any other issues apart from this that are blockers for an upgrade to a new ID?

It is my understanding that this is primarily a go-libp2p specific problem. Over at rust-libp2p, we don't specialize the identify protocol.

I am not familiar with the go-libp2p codebase but would it be possible to run two versions of the identify protocol in parallel? This way, there is no extra roundtrip, just more bandwidth being consumed and over time, you could phase out the old one once enough nodes have updated.

[Stebalien]

go-libp2p added signed peer records (https://github.com/libp2p/go-libp2p/blob/8cf67ba1d06271050c4211eb383189214bb535c1/p2p/protocol/identify/pb/identify.proto#L45) ages ago (with the assumption that the spec change would be made).

However, we never changed the protocol name because that's an unrelated issue.

In terms of how to implement the protocol name change... there isn't actually a round-trip issue as long as we prefer the old name (/ipfs/id/1.0.0) for now. I.e.:

1. The client tries to negotiate both protocols, but prefers the old protocol.
2. The server supports both protocol names.

Eventually, once most of the network has upgraded, we can switch the clients to preferring the new protocol.

[thomaseizinger]

Cool that is good to know!

It is OT for this issue but I might open another one soon then where we can discuss moving to a new identifier.