core: introduce {In,Out}boundConnectionUpgrade traits

[thomaseizinger]

The InboundUpgrade and OutboundUpgrade traits currently fulfill two needs:

• Upgrading connections: i.e. going from an unencrypted connection to an encrypted connection with libp2p-noise or libp2p-tls but also going from a single-stream connection to a multiplexed connection using libp2p-yamux or libp2p-mdns
• Upgrading streams: i.e. going from a negotiated stream to some kind of message. This isn't always possible which is why we want to remove the concept of stream upgrades to allow for a more uniform handling. See Simplify ConnectionHandler trait by removing as many associated types as possible #2863 for details.

To finish #2863, we need to deprecate the implementations of {In,Out}boundUpgrade for stream upgrades but without affecting implementations of connection upgrades.

Thus, we need to create a dedicated set of traits that is used just for connection upgrades: {In,Out}boundConnectionUpgrade. They need a blanket impl over everything that implements {In,Out}boundUpgrade for this to be backwards-compatible.

We can then change the "transport upgrade" infrastructure to require these traits instead of the current ones. For example here:

rust-libp2p/core/src/transport/upgrade.rs

Lines 104 to 105 in 02dc432

	U: InboundUpgrade<Negotiated<C>, Output = (PeerId, D), Error = E>,
	U: OutboundUpgrade<Negotiated<C>, Output = (PeerId, D), Error = E> + Clone,

For it actually be a backwards-compatible change, we have to rely on the blanket impl in all the dependent crates, like libp2p-noise etc. Changing those to the new traits will have to be in the next breaking release.

[PanGan21]

Hi! I am looking into this issue in order to start understand it better and maybe start contributing. :)
From my understanding it needs something like the following. Please correct me if I am wrong:

pub trait InboundConnectionUpgrade<T> {
    type Output;
    type Error;
    type Future: Future<Output = Result<Self::Output, Self::Error>>;

    fn upgrade_inbound(self, socket: C, info: Self::Info) -> Self::Future;
}

pub trait OutboundConnectionUpgrade<T> {
    type Output;
    type Error;
    type Future: Future<Output = Result<Self::Output, Self::Error>>;

    fn upgrade_outbound(self, socket: C, info: Self::Info) -> Self::Future;
}

// Blanket implementation for InboundConnectionUpgrade based on InboundUpgrade
impl<U, T> InboundConnectionUpgrade<T> for U
where
    U: InboundUpgrade<T>,
{
    type Output = <U as InboundUpgrade<T>>::Output;
    type Error = <U as InboundUpgrade<T>>::Error;

    fn upgrade_inbound_connection(self, socket: C, info: Self::Info) -> Self::Future {
        Box::pin(self.upgrade_inbound(socket, info))
    }
}

// Blanket implementation for OutboundConnectionUpgrade based on OutboundUpgrade
impl<U, T> OutboundConnectionUpgrade<T> for U
where
    U: OutboundUpgrade<T>,
{
    type Output = <U as OutboundUpgrade<T>>::Output;
    type Error = <U as OutboundUpgrade<T>>::Error;

    fn upgrade_outbound_connection(self, socket: C, info: Self::Info) -> Self::Future {
        Box::pin(self.upgrade_outbound(socket, info))
    }
}

[thomaseizinger]

Hi! I am looking into this issue in order to start understand it better and maybe start contributing. :)

Great to hear! Thank you for your help :)

From my understanding it needs something like the following. Please correct me if I am wrong:

pub trait InboundConnectionUpgrade<T> {
    type Output;
    type Error;
    type Future: Future<Output = Result<Self::Output, Self::Error>>;

    fn upgrade_inbound(self, socket: C, info: Self::Info) -> Self::Future;
}

pub trait OutboundConnectionUpgrade<T> {
    type Output;
    type Error;
    type Future: Future<Output = Result<Self::Output, Self::Error>>;

    fn upgrade_outbound(self, socket: C, info: Self::Info) -> Self::Future;
}

// Blanket implementation for InboundConnectionUpgrade based on InboundUpgrade
impl<U, T> InboundConnectionUpgrade<T> for U
where
    U: InboundUpgrade<T>,
{
    type Output = <U as InboundUpgrade<T>>::Output;
    type Error = <U as InboundUpgrade<T>>::Error;

    fn upgrade_inbound_connection(self, socket: C, info: Self::Info) -> Self::Future {
        Box::pin(self.upgrade_inbound(socket, info))
    }
}

// Blanket implementation for OutboundConnectionUpgrade based on OutboundUpgrade
impl<U, T> OutboundConnectionUpgrade<T> for U
where
    U: OutboundUpgrade<T>,
{
    type Output = <U as OutboundUpgrade<T>>::Output;
    type Error = <U as OutboundUpgrade<T>>::Error;

    fn upgrade_outbound_connection(self, socket: C, info: Self::Info) -> Self::Future {
        Box::pin(self.upgrade_outbound(socket, info))
    }
}

Yes, I think that all we need is a direct copy of the existing traits.

Do we necessarily need the boxing or can we do without?

Best would be to open a PR and see what CI has to say about this! An important goal here is backwards-compatibility because we want to release this as a non-breaking change.

[PanGan21]

Just opened a draft pr! Let me know your thoughts. I hope I understood the requirements correctly