-
Notifications
You must be signed in to change notification settings - Fork 19
Add DHTRouter interface #93
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
base: master
Are you sure you want to change the base?
Conversation
The implementation of ipfs/specs#476 suggests that content routers should support a DHT-specific operations (GetClosestPeers). Content routers depend on routing interfaces defined in the `routing` package and some decorator interfaces defined here. So it seems like the natural place to add yet another interface for this type of router.
|
This interface will be used from boxo and kubo, it would be weird that it lives in boxo as that would create yet another place where routing interfaces live. As long as I have not misunderstood things. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This interface doesn't match kad-dht GetClosestPeers.
This only seems to be used by ipfs/specs#476, in which closerThan and count are currently not used, because implemented neither in Go nor JS DHT.
We could nonetheless use the count and closerThan parameters in boxo to return less than 20 peers, by filtering the results of the kad-dht GetClosestPeers(). But I wouldn't make a generic DHT interface including these unsupported parameters.
It is certainly fine to define this interface in boxo, since it is only used in the routing API so far. We can reevaluate later when/if this interface is used somewhere else.
|
Also, other routing API funcs aren't defined in |
Those are slightly different. The contentRouter there implements the methods that Kubo/Boxo work with, which are those in libp2p/routing and routing-helpers.
Yes, I need to respect the spec, and those options should be handled as deep as possible. I know DHT doesn't have them now, so the layer right above (Kubo in this case) will have to deal with them and ignore them or respect them... I'm also fine with the spec being more aligned with reality and dropping the options or limiting them. |
|
@hsanjuan my vote would be to remove unsupported parameters from spec (ipfs/specs#476 (comment)) |
|
@lidel sounds good, I will circle back to this work eventually and carry out the parameter removal accross the board. |
The implementation of ipfs/specs#476 suggests that content routers should support a DHT-specific operations (GetClosestPeers).
Content routers depend on routing interfaces defined in the
routingpackage and some decorator interfaces defined here. So it seems like the natural place to add yet another interface for this type of router.