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.

Sign up for GitHub

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

Jump to bottom

Add Transient as a "connectedness" state #2692

Closed
Stebalien opened this issue Jan 26, 2024 · 2 comments · Fixed by #2696
Closed

Add Transient as a "connectedness" state #2692

Stebalien opened this issue Jan 26, 2024 · 2 comments · Fixed by #2696
Labels
kind/enhancement A net-new feature or improvement to an existing feature

Comments

@Stebalien
Copy link
Member

This will let users:

  1. Quickly detect if the connection to a peer is transient.
  2. Detect transitions from transient to "full" connections (and back) via events.
@Stebalien Stebalien mentioned this issue Jan 26, 2024
Draft
Stebalien added a commit that referenced this issue Jan 29, 2024
@Stebalien
Add a "transient" network connectivity state
d624653 
Previously, we'd consider "transiently" connected peers to be connected.
This meant:

1. We wouldn't fire a second event when transitioning to "really
connected". The only option for users was to listen on the old-style
per-connection notifications.
2. "Connectedness" checks would be a little too eager to treat a peer as
connected.

For 99% of users, "transient" peers should be treated as disconnected.
So while it's technically a breaking change to split-out "transient"
connectivity into a separate state, I expect it's more likely to fix
bugs than anything.

Unfortunately, this change _did_ require several changes to go-libp2p
itself because go-libp2p _does_ care about transient connections:

1. We want to keep peerstore information for transient peers.
2. We may sometimes want to treat peers as "connected" in the host.
3. Identify still needs to run over transient connections.

fixes #2692
@Stebalien Stebalien mentioned this issue Jan 29, 2024
Merged
Stebalien added a commit that referenced this issue Jan 29, 2024
@Stebalien
Add a "transient" network connectivity state
11515f0 
Previously, we'd consider "transiently" connected peers to be connected.
This meant:

1. We wouldn't fire a second event when transitioning to "really
connected". The only option for users was to listen on the old-style
per-connection notifications.
2. "Connectedness" checks would be a little too eager to treat a peer as
connected.

For 99% of users, "transient" peers should be treated as disconnected.
So while it's technically a breaking change to split-out "transient"
connectivity into a separate state, I expect it's more likely to fix
bugs than anything.

Unfortunately, this change _did_ require several changes to go-libp2p
itself because go-libp2p _does_ care about transient connections:

1. We want to keep peerstore information for transient peers.
2. We may sometimes want to treat peers as "connected" in the host.
3. Identify still needs to run over transient connections.

fixes #2692
Stebalien added a commit that referenced this issue Jan 29, 2024
@Stebalien
Add a "transient" network connectivity state
57736f1 
Previously, we'd consider "transiently" connected peers to be connected.
This meant:

1. We wouldn't fire a second event when transitioning to "really
connected". The only option for users was to listen on the old-style
per-connection notifications.
2. "Connectedness" checks would be a little too eager to treat a peer as
connected.

For 99% of users, "transient" peers should be treated as disconnected.
So while it's technically a breaking change to split-out "transient"
connectivity into a separate state, I expect it's more likely to fix
bugs than anything.

Unfortunately, this change _did_ require several changes to go-libp2p
itself because go-libp2p _does_ care about transient connections:

1. We want to keep peerstore information for transient peers.
2. We may sometimes want to treat peers as "connected" in the host.
3. Identify still needs to run over transient connections.

fixes #2692
@Stebalien
Copy link
Member Author

Needed for ipfs/boxo#526

@sukunrt
Copy link
Member

sukunrt commented Feb 5, 2024 •
edited
Loading

Can we do?

Detect transitions from transient to "full" connections Can we do?
Detect transitions from transient to "full" connections (and back) via events.

I'm not sure if we can guarantee ordering between notifications for two different connections. The only guarantee right now is that disconnect on a connection is after connect. This behavior I believe was changed in https://github.com/libp2p/go-libp2p/pull/2373/files(and back) via events.

Ignore this. It does work correct for hole punching.

@sukunrt sukunrt added the kind/enhancement A net-new feature or improvement to an existing feature label Feb 22, 2024
sukunrt pushed a commit that referenced this issue May 5, 2024
@Stebalien @sukunrt
Add a "transient" network connectivity state
374ec48 
Previously, we'd consider "transiently" connected peers to be connected.
This meant:

1. We wouldn't fire a second event when transitioning to "really
connected". The only option for users was to listen on the old-style
per-connection notifications.
2. "Connectedness" checks would be a little too eager to treat a peer as
connected.

For 99% of users, "transient" peers should be treated as disconnected.
So while it's technically a breaking change to split-out "transient"
connectivity into a separate state, I expect it's more likely to fix
bugs than anything.

Unfortunately, this change _did_ require several changes to go-libp2p
itself because go-libp2p _does_ care about transient connections:

1. We want to keep peerstore information for transient peers.
2. We may sometimes want to treat peers as "connected" in the host.
3. Identify still needs to run over transient connections.

fixes #2692
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
kind/enhancement A net-new feature or improvement to an existing feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add a "transient" network connectivity state libp2p/go-libp2p
2 participants
@Stebalien @sukunrt