search

libp2p / go-libp2p-pubsub

The PubSub implementation for go-libp2p
https://github.com/libp2p/specs/tree/master/pubsub
Other
309 stars 178 forks source link

use event bus for monitoring peer connections and protocol updates #536

Open jefft0 opened 1 year ago

jefft0 commented 1 year ago

This fixes a bug where another peer is not added to a topic after subscribing. This bug happens, for example, when a discovery system (like MDNS) is set up before pubsub initialization. Even though the peers have been discovered, they are not added to the new pubsub topics.

The bug can be seen by running TestNotifyPeerProtocolsUpdated in the master branch:

git clone https://github.com/libp2p/go-libp2p-pubsub
cd go-libp2p-pubsub
curl https://raw.githubusercontent.com/jefft0/go-libp2p-pubsub/fix/notify-protocol-update/notify_test.go -o notify_test.go
go test -run TestNotifyPeerProtocolsUpdated .

It prints "topic1 should at least have 1 peer". In our code we had to workaround this by disabling discovery (MDNS) and deferring discovery until after the pubsub protocols are registered. However, this pull request fixes the problem so that a workaround is not needed. It uses the event bus to monitor for peer connections and protocol updates. When hosts[1] joins topic1, the other peer is automatically added and the test passes.

jefft0 commented 1 year ago

Some background if it helps: We discovered and created a fix for this issue while working on this simple example of our networking library that is built on libp2p. https://wesh.network/posts/share-contact-and-send-message

jefft0 commented 2 months ago

Hello @vyzo . You suggested moving AddPeers to inside startMonitoring before the goroutine as p.AddPeers(p.host.Network().Peers()...) . But with this change, TestSimpleDiscovery hangs and times out. What do you suggest to do to resolve this? (Right now we have to use a fork of this repo with the fix.)

vyzo commented 2 months ago

Uhm, fix the deadlock?

Maybe add an extra goroutine for startup.