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

swarm: fix DialPeer behaviour for transient connections #2547

Merged
merged 2 commits into from
Sep 18, 2023

Conversation

sukunrt
Copy link
Member

@sukunrt sukunrt commented Aug 31, 2023

This fixes a bug where the first call to swarm.DialPeer succeeds and returns a transient connection with no error while the second call to swarm.DialPeer returns (nil, network.ErrTransientConn).

For dialing, we now only rely on network.WithForceDirectDial to force a direct connection.
For new stream, we open a stream on a transient connection only if network.WithUseTransient is used.

This feels correct to me since once we fix #1603 we can have NewStream called without network.WithUseTransient or network.WithForceDirectDial and it'll wait for the transient connection to be upgraded to a direct connection before returning.

I'm not fixing it the other way round, i.e. Failing if network.WithUseTransient is not used, because there are users of this API who are probably relying on the first call to succeed. https://github.com/search?q=Network%28%29.DialPeer&type=code

Comment on lines 61 to 60
// swarm.DialPeer should connect over transient connections
conn1, err := h1.Network().DialPeer(context.Background(), h2.ID())
require.NoError(t, err)
require.NotNil(t, conn1)

conn2, err := h1.Network().DialPeer(context.Background(), h2.ID())
require.NoError(t, err)
require.NotNil(t, conn2)
Copy link
Member Author

Choose a reason for hiding this comment

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

On current master, the first call succeeds while the second one fails.

This fixes a bug where the first call to `swarm.DialPeer` succeeds and
returns a transient connection with no error while the second call to
DialPeer returns `(nil, network.ErrTransientConn)`.

For dialing, we now only rely on `network.WithForceDirectDial` to force
a direct connection.
For new stream, we open a stream on a transient connection only if
`network.WithUseTransient` is used.
p2p/net/swarm/swarm.go Outdated Show resolved Hide resolved
p2p/test/swarm/swarm_test.go Show resolved Hide resolved
p2p/net/swarm/swarm.go Outdated Show resolved Hide resolved
@sukunrt sukunrt merged commit 70d31da into master Sep 18, 2023
11 checks passed
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

Successfully merging this pull request may close these issues.

Connect should wait for an acceptable connection
2 participants