Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

feat: add delegate routers to libp2p config #2195

Merged
merged 4 commits into from
Jun 28, 2019
Merged

Conversation

jacobheun
Copy link
Contributor

Adds the delegate routing modules needed for #2155.

This enables js-ipfs nodes to leverage external nodes to provide content, find content, and find peers.

I added a crude randomizer for the preload nodes which are supporting delegate routing. Ideally this would be per request, but the modules will need to be updated to take multiple hosts to do so.

A note: If the DHT is enabled on your node, libp2p will attempt to perform the action using the DHT first. If that is not successful, it will then use the delegates to perform the request. This gives nodes the ability to fallback to the external nodes while they are building up their DHT records/peers.

Copy link
Contributor

@dirkmc dirkmc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the hostnames be in config?
Otherwise LGTM 👍

@jacobheun
Copy link
Contributor Author

jacobheun commented Jun 24, 2019

Should the hostnames be in config?

Yeah, we should probably have the multiaddrs in there and then pull out the http endpoints.

This will cause the config to deviate from go, as we'll likely add a new address field like Addresses.Delegates. This should be fine as it's more of a js thing, but if we do that we should check to make sure go doesn't care about the extra field.

Does Addresses.Delegates sound like the right place for this to go?

Addresses: {
  ...
  Delegates: [
    '/dns4/node0.delegate.ipfs.io/tcp/443/https',
    '/dns4/node1.delegate.ipfs.io/tcp/443/https'
  ]
}

(updated addrs to match ones from #2291@lidel)

@dirkmc
Copy link
Contributor

dirkmc commented Jun 24, 2019

Discovery and Bootstrap are at the top level, so it may make sense for Delegates to be as well, and thereby less likely to conflict with go config

@alanshaw alanshaw mentioned this pull request Jun 24, 2019
54 tasks
@jacobheun
Copy link
Contributor Author

I've updated this to only add delegated routing if the delegates multiaddrs are actually added to the Config Addresses.Delegates property. I also added docs and a couple tests.

Copy link
Member

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry just noticed the link to the example is not working when rendered.

Separately the example says this depends on a branch of go ipfs - does it still?

Also, we need to document in the example what API endpoints need to be exposed on a go ipfs node for this to work.

@alanshaw alanshaw force-pushed the feat/delegated-routing branch from 6ecbdc4 to d36cf47 Compare June 27, 2019 22:18
@alanshaw
Copy link
Member

@jacobheun I fixed the link and rebased

@alanshaw alanshaw merged commit 1aaaab9 into master Jun 28, 2019
@alanshaw alanshaw deleted the feat/delegated-routing branch June 28, 2019 06:04
lidel added a commit that referenced this pull request Jul 12, 2019
Adds Delegates as optional array in node config.
Context: #2195

License: MIT
Signed-off-by: Marcin Rataj <[email protected]>
alanshaw pushed a commit that referenced this pull request Jul 17, 2019
This PR adds `Addresses.Delegates` (added in  #2195) as an optional array to the schema used for config validation. 

It fixes the error below:

![2019-07-12--21-00-53](https://user-images.githubusercontent.com/157609/61152758-92e00180-a4e9-11e9-8ef1-c6c1200575ee.png)

ps. I tested it in Brave (embedded js-ipfs + preload nodes) and delegated DHT queries produce expected responses 👌 

> ![2019-07-12--21-27-35](https://user-images.githubusercontent.com/157609/61153580-eeab8a00-a4eb-11e9-9152-0dcb51e2a67d.png)
> ![2019-07-12--21-27-52](https://user-images.githubusercontent.com/157609/61153579-eeab8a00-a4eb-11e9-8478-71f68d3330d4.png)

License: MIT
Signed-off-by: Marcin Rataj <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants