-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Identify protocol: SignedPeerRecords not being added to the CertifiedAddrBook #2754
Comments
If that is the expected behavior, how should applications handle the use of Gossipsub |
Not sure yet if it's the expected behavior but I confirm that was the problem for my case, just forked the code and modified Now every peer has the signed records of every peers and Code modification here: PedrobyJoao@9504ce7 |
I think it's fine to add the signed peer record to the CertifiedAddrBook and remove it when the peer disconnects. We should specify this though. Currently the identify spec says nothing about signed peer records: https://github.com/libp2p/specs/blob/master/identify/README.md |
First off, thank you @PedrobyJoao . This is a great example of a well crafted issue and bug report. I appreciate it! The quick fix is certainly to start putting things back in the certified address book. But here are some reasons against it:
I don't think the solution is to have Identify be responsible for updating the certified address book. Here's my suggestion on how to fix it:
Even without 2, applications would still be able to update the CertifiedAddressBook themselves from events emitted by Identify. It would be even better if we could allow go-libp2p to run modular components/services (as mentioned in #1993). Then if multiple protocols needed a certain kind of certified address book, they could construct it and share the same resource, but this can happen later. |
Following up here to give a quick update:
|
Version Information
Hey! I have been trying to use Gossipsub
Peer Exchange (PX)
feature for discovery/routing purposes, however when automatically trying to connect to the discovered peers (based on PX done by bootstrap peers), the same error happens for all new connection tries (for the returned peers):2024-03-26T08:53:25.568-0300 DEBUG pubsub [email protected]/gossipsub.go:966 error connecting to QmeYAxZaJ9C3Tw8yp6XNopaJP2ctyrNysReJjhzFeiASJt: failed to dial: failed to dial QmeYAxZaJ9C3Tw8yp6XNopaJP2ctyrNysReJjhzFeiASJt: no addresses
After investigating a little bit, I saw that addresses will only be send within PX if they come from
SignedPeerRecords
.I didn't know anything about
SignedPeerRecords
so I supposed I had to create and send them manually, until I realized that they were being automatically created when instantiatingbasicHost
and being sent within the identify family of protocols.I had setup a basic network for tests where there is one bootstrap peers and
n
other normal peers, they're all connecting to the bootstrap peer only (star topology). For debugging purposes, I added the following that runs every 30 seconds:Output in a nutshell: the host peer has only information about its own signed peer record! While for all the other connected peers,
Peer has NO signed peer record
is logged.Why is that happening: Identify protocol
When receiving messages through the Identify protocols, the peer records are indeed processed BUT they are not added to the CertifiedAddrBook. See:
It seems that is the expected behavior?
So my question is: is this really an expected behavior?
What is the utility of processing SignedPeerRecords within the Identify protocols if they're being treated as normal unsigned records? Applications can not differentiate if the received listening addresses from Identify are signed or unsigned currently.
The text was updated successfully, but these errors were encountered: