[question] protocols/plaintext: How to use plaintext1 with builder pattern

[mxinden]

When calling Transport::upgrade one retrieves a
core::transport::upgrade::Builder. Calling authenticate on this Builder
requires the InboundUpgrade::Output and OutboundUprade::Output
implementation of the passed struct to be a tuple of
(ConnectionInfo, AsyncRead + AsyncWrite).

One can not use the above builder pattern upgrade with
libp2p_plaintext::Plaintext1, given that its InboundUpgrade and
OutboundUpgrade don't define a tuple as their Output, but just return the
plain Negotiated<C>. In addition, given that Plaintext1 is just a simple
wrapper around Negotiated<C> it is not aware of the identity of the other
side, and can thus not fulfill the ConnectionInfo trait.

Plaintext2 does a full handshake to retrieve the PeerId of the other side to
satisfy the ConnectionInfo trait.

This patch makes Plaintext1 simply return a random PeerId. Whether this is a
valid solution is questionable. It rather serves the purpose of showcasing the
issue.

What is the correct way of upgrading a transport with Plaintext1?

Relates to #1240.

[romanb]

What is the correct way of upgrading a transport with Plaintext1?

There isn't any. Plaintext1 was never directly compatible with the Swarm API, at least not since that API required a pair of (PeerId, AsyncRead + AsyncWrite) as the Transport output. Note that the transport::upgrade::Builder does not constrain what can be done with upgrades, it just encodes the "standard" upgrade procedure that yields a transport that is directly usable with the Network/Swarm APIs. You can always do "ad-hoc" upgrades using Transport::and_then, which is often done in tests, for example:

TcpConfig::new()
    .and_then(move |io, endpoint| {
        upgrade::apply(io, Plaintext1, endpoint, upgrade::Version::V1)
     })
    .map(|plaintext, _endpoint| {
        let peer_id = summon_peer_id();
        (some_peer_id, plaintext)
    })

In other words, to use Plaintext1 with a Network/Swarm you have to summon a peer ID from somewhere and when dialing it needs to be the peer ID that is being dialed, otherwise the Swarm will abort the connection. The libp2p specs for the plaintext protocol have a related remark:

An earlier version, /plaintext/1.0.0, was implemented in several languages, but it did not include any exchange of public keys or peer ids. This led to undefined behavior in parts of libp2p that assumed the presence of a peer id.

So to summarise, I don't think Plaintext1 is really good for anything, especially with Plaintext2 in place.

[mxinden]

Thanks, that makes sense @romanb.

So to summarise, I don't think Plaintext1 is really good for anything, especially with Plaintext2 in place.

To reduce confusion I would suggest removing it. With #1236 (review) in mind, do you think strongly about keeping it @tomaka? If so I will add proper comments to the module for future readers.

[tomaka]

It used to be possible to use plaintext v1 using the IdentityTransport which has now been removed.

While plaintext v1 isn't compatible with the swarm API, I also don't think it hurts to keep it.

[mxinden]

While plaintext v1 isn't compatible with the swarm API, I also don't think it hurts to keep it.

Sounds reasonable. I have added #1286 to add some documentation.

Thanks you two for the answers!