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

core/transport: Improve docs for Transport::address_translation #2976

Merged
merged 4 commits into from Oct 9, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 16 additions & 3 deletions core/src/transport.rs
Expand Up @@ -149,9 +149,22 @@ pub trait Transport {
cx: &mut Context<'_>,
) -> Poll<TransportEvent<Self::ListenerUpgrade, Self::Error>>;

/// Performs a transport-specific mapping of an address `observed` by
/// a remote onto a local `listen` address to yield an address for
/// the local node that may be reachable for other peers.
/// Performs a transport-specific mapping of an address `observed` by a remote onto a
/// local `listen` address to yield an address for the local node that may be reachable
/// for other peers.
///
/// This is relevant for transports where Network Address Translation (NAT) can occur
/// so that e.g. the peer is observed at a different IP than the IP of the local
/// listening address. See also [`address_translation`][crate::address_translation].
///
/// Within [`libp2p::Swarm`](<https://docs.rs/libp2p/latest/libp2p/struct.Swarm.html>) this is
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a better way to link to items from crates that are not a dependency?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not aware of a way. Maybe @thomaseizinger?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Me neither but I also think it is kind of an odd thing to do. This crate shouldn't "know" about Swarm. Do we have to include this sentence here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Me neither but I also think it is kind of an odd thing to do. This crate shouldn't "know" about Swarm. Do we have to include this sentence here?

Good point. But in practice transports are commonly used in a swarm, so I think its okay to mention this here. When I was looking at it for Quic it helped me to know how it is used in the swarm.

/// used when extending the listening addresses of the local peer with external addresses
/// observed by remote peers.
/// On transports where this is not relevant (i.e. no NATs are present) `None` should be
/// returned for the sake of de-duplication.
///
/// Note: if the listen or observed address is not a valid address of this transport,
/// `None` should be returned as well.
fn address_translation(&self, listen: &Multiaddr, observed: &Multiaddr) -> Option<Multiaddr>;

/// Boxes the transport, including custom transport errors.
Expand Down