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

upgrade dependencies. #233

Merged
merged 7 commits into from
Aug 19, 2020
Merged

upgrade dependencies. #233

merged 7 commits into from
Aug 19, 2020

Conversation

raulk
Copy link
Member

@raulk raulk commented May 19, 2020

Doesn't build due to go-libp2p-kad-dht API breaking changes.

⟩ go build ./...
# github.com/libp2p/go-libp2p-daemon
./dht.go:91:18: d.dht.FindPeersConnectedToPeer undefined (type *dht.IpfsDHT has no field or method FindPeersConnectedToPeer)
./dht.go:123:11: undefined: dht.KValue

@aarshkshah1992
Copy link
Contributor

aarshkshah1992 commented May 19, 2020

@Stebalien @aschmahmann

I see we removed the FindPeersConnectedToPeer API in the https://github.com/libp2p/go-libp2p-kad-dht/releases/tag/v0.6.0 release but the release notes don't mention it and I can't seem to find an issue that explains why we removed this.

On the other hand, I'm not quite sure it even makes to have that API in the DHT and if it's implementation was even correct.

Can I go ahead and remove this API from go-libp2p-daemon as well ?

@vyzo
Copy link
Collaborator

vyzo commented May 19, 2020

So now it's impossible to write a crawler?

@aschmahmann
Copy link

aschmahmann commented May 19, 2020

So now it's impossible to write a crawler?

Nope, crawlers are working just fine and people keep making more of them. Now that routing table membership is disassociated from connection state using only a view of connected peers is both an unnecessary information leak and will result in an incorrect view of the network.

Looking through open source projects (e.g. https://grep.app/search?q=.FindPeersConnectedToPeer&case=true&filter[lang][0]=Go) there does not seem to be any external community members trying to do anything useful with this other than just re-expose the functionality like this libp2p-daemon does.

@vyzo did you have any other use case in mind that would warrant re-exposing this internal state?

Can I go ahead and remove this API from go-libp2p-daemon as well ?

Yes

@vyzo
Copy link
Collaborator

vyzo commented May 19, 2020

No, it was only useful for crawling and diagnosing the network.

@aschmahmann
Copy link

@raulk depending on what people actually use the libp2p-daemon for I'd probably switch the DHT to use the dual subpackage (e.g. dual.New()) instead of the regular DHT (kaddht.New()). This is more likely to make things "just work" out of the box and is what is used by go-ipfs.

@vasco-santos
Copy link
Member

vasco-santos commented Jun 2, 2020

Hey all!

IPFS is experiencing some interop issues with the latest gossipsub work . Some references:

I would like to get the interop tests running on libp2p/interop, so that I can try to debug what is going on. Can we get this PR ready?

cc @achingbrain

kvalue has nothing to do with the number of providers a user might want and this
constant has been removed from the DHT.
This feature has been removed from the DHT and supporting this feature here was
an historical mistake. We need to move towards abstracting over an abstract
"router" type instead of hard-coding DHT concepts.
@jacobheun jacobheun mentioned this pull request Aug 19, 2020
72 tasks
@Stebalien Stebalien merged commit 9ecfdc5 into master Aug 19, 2020
@Stebalien Stebalien deleted the update branch August 19, 2020 20:16
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.

7 participants