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

NewStream failure right after startup #546

Open
cpacia opened this issue Oct 29, 2023 · 1 comment
Open

NewStream failure right after startup #546

cpacia opened this issue Oct 29, 2023 · 1 comment

Comments

@cpacia
Copy link

cpacia commented Oct 29, 2023

This took me forever debug, but here's the issue. If you start up two nodes and immediately join a topic they will fail discover each other in the pubsub. Here's why:

Node1

  • Create host
  • Create pubsub

Node2

  • Create host
    • This causes an immediate connection to Node1 but the pubsub protocol is not yet registered.
    • Node1's pubsub detects the new connection, tries to open a stream, and NewStream fails because Node2 doesn't yet speak pubsub.
  • Create pubsub
    • Node2's pubsub doesn't detect the connection since it was established before the pubsub was created.

Nodes 1 and 2 never connect :(

Fortunately I was able to come up with a fix that doesn't require any changes in libp2p. Basically detect the protocolupdate event and force it think it received a new connection:

        protocolUpdatedSub, err := host.EventBus().Subscribe(new(event.EvtPeerProtocolsUpdated))
	if err != nil {
		return nil, err
	}
	go func(sub event.Subscription) {
		for {
			select {
			case evt, ok := <-sub.Out():
				if !ok {
					return
				}
				var updated bool
				for _, proto := range evt.(event.EvtPeerProtocolsUpdated).Added {
					if proto == cfg.params.ProtocolPrefix+pubsub.GossipSubID_v11 {
						updated = true
						break
					}
				}
				if updated {
					for _, c := range host.Network().ConnsToPeer(evt.(event.EvtPeerProtocolsUpdated).Peer) {
						(*pubsub.PubSubNotif)(ps).Connected(host.Network(), c)
					}
				}
			}
		}
	}(protocolUpdatedSub)

But it would be nice if this was built in as it's definitely not expected behavior.

@cpacia
Copy link
Author

cpacia commented Oct 29, 2023

Looks like there is an open PR attempting to address this issue.

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

1 participant