Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src/transport.rs: Consider using V1Lazy #12

Open
mxinden opened this issue Jul 5, 2021 · 2 comments
Open

src/transport.rs: Consider using V1Lazy #12

mxinden opened this issue Jul 5, 2021 · 2 comments

Comments

@mxinden
Copy link

mxinden commented Jul 5, 2021

Unsolicited feedback. Feel free to ignore.

Given that you are supporting a single authentication protocol only, you might want to consider using Version::V1Lazy instead of V1, potentially saving you one round trip. See V1Lazy docs for details.

/// "Completes" a transport by applying the authentication and multiplexing
/// upgrades.
///
/// Even though the actual transport technology in use might be different, for
/// two libp2p applications to be compatible, the authentication and
/// multiplexing upgrades need to be compatible.
pub fn authenticate_and_multiplex<T>(
transport: Boxed<T>,
identity: &identity::Keypair,
) -> Result<Boxed<(PeerId, StreamMuxerBox)>>
where
T: AsyncRead + AsyncWrite + Unpin + Send + 'static,
{
let auth_upgrade = {
let noise_identity = noise::Keypair::<X25519Spec>::new().into_authentic(identity)?;
NoiseConfig::xx(noise_identity).into_authenticated()
};
let multiplex_upgrade = SelectUpgrade::new(yamux::YamuxConfig::default(), MplexConfig::new());
let transport = transport
.upgrade(Version::V1)
.authenticate(auth_upgrade)
.multiplex(multiplex_upgrade)
.timeout(Duration::from_secs(20))
.map(|(peer, muxer), _| (peer, StreamMuxerBox::new(muxer)))
.boxed();
Ok(transport)
}

@rishflab
Copy link
Member

rishflab commented Jul 8, 2021

Given that you are supporting a single authentication protocol only, you might want to consider using Version::V1Lazy instead of V1, potentially saving you one round trip. See V1Lazy docs for details.

In the docs it says V1Lazy only applies to the dialer. Since the rendezvous point never dials anyone, I don't think it is needed here?

This strategy is only applicable for the node with the role of “dialer” in the negotiation and only if the dialer supports just a single application protocol. In that case the dialer immedidately “settles” on that protocol, buffering the negotiation messages to be sent with the first round of application protocol data (or an attempt is made to read from the Negotiated I/O stream).

@mxinden
Copy link
Author

mxinden commented Jul 8, 2021

In the docs it says V1Lazy only applies to the dialer. Since the rendezvous point never dials anyone, I don't think it is needed here?

My bad. Yes you are right. I didn't think about this being a rendezvous-only binary.

You could consider setting Swarm::substream_upgrade_protocol_override to Version::V1Lazy to speed up e.g. libp2p-ping substream negotiations, might not be worth the performance gain in the case of libp2p-ping though. Also obviously not worth it in case you decide to remote libp2p-ping.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants