Skip to content

gateway: expose commands for js-libp2p delegated routing - #4595

Closed
ghost wants to merge 1 commit into
masterfrom
feat/delegated-routing
Closed

ghost wants to merge 1 commit into
masterfrom
feat/delegated-routing

Conversation

@ghost

@ghost ghost commented Jan 19, 2018

Copy link
Copy Markdown

This enables js-libp2p/js-ipfs to delegate peer routing and content routing to a go-ipfs gateway (usually ipfs.io). It covers:

  • Peer routing lookups (dht/findpeer)
  • Content routing lookups (dht/findprovs)
  • Content routing writes -- go-libp2p-kad-dht rejects provider records which originate from a different node than the message sender, so we're doing a creative hack here: we'll tell the delegatee to connect to us (swarm/connect with a /p2p-circuit address), and then to fetch the data from us (refs?r=true). This isn't technically content routing, but it makes the data available to the network which is all we need.

Peer routing writes will be a new command that'll be covered in a future PR.

(@diasdavid catch!)

@ghost ghost added topic/libp2p Topic libp2p topic/commands Topic commands topic/gateway Topic gateway topic/interop Interoperability labels Jan 19, 2018
@ghost ghost self-assigned this Jan 19, 2018
@ghost ghost added the status/in-progress In progress label Jan 19, 2018
@ghost

ghost commented Jan 19, 2018

Copy link
Copy Markdown
Author

Oh and this is already running on ipfs.io, so go take a shot at it @diasdavid

@Stebalien

Copy link
Copy Markdown
Member

Would putting these behind a config option be problematic? I worry about increasing the default gateway API a bit.

@ghost

ghost commented Jan 28, 2018

Copy link
Copy Markdown
Author

Would putting these behind a config option be problematic? I worry about increasing the default gateway API a bit.

Not at all, you have a point. I'll update this in the next few days.

@Kubuxu

Kubuxu commented Mar 9, 2018

Copy link
Copy Markdown
Member

@magik6k mind taking over?

@Kubuxu Kubuxu added need/author-input Needs input from the original author and removed status/in-progress In progress labels Mar 9, 2018
@magik6k magik6k self-assigned this Mar 9, 2018
@magik6k

magik6k commented Mar 9, 2018

Copy link
Copy Markdown
Member

I don't see any reasonably simple way to put this behind a config option (rootROOldSubcommands is set up in init), one 'hack' I have in mind is to create a utility command which would perform that check and proxy to the proper command, does that sound reasonable?

@Stebalien

Copy link
Copy Markdown
Member

Can the command itself not check and return a permission denied error? Not the best solution but it works.

@magik6k

magik6k commented Mar 9, 2018

Copy link
Copy Markdown
Member

It would have to check if it was called from the gateway port in that case which I'm not sure is even possible.

@ghost

ghost commented Mar 24, 2018

Copy link
Copy Markdown
Author

We could maybe eliminate the root/rootRO duality and have :8080 call the same root cmds.Command as :5001, but with a configurable allowlist in front. The current rootRO would then be default config for that.

@ghost

ghost commented Mar 24, 2018

Copy link
Copy Markdown
Author

but with a configurable allowlist in front

and a hardcoded list of commands that should not ever be exposed to the public.

@ghost
ghost force-pushed the feat/delegated-routing branch from 514af5e to 0dc0a04 Compare April 18, 2018 17:04
@ghost
ghost requested a review from Kubuxu as a code owner April 18, 2018 17:04
@ghost ghost added the status/in-progress In progress label Apr 18, 2018
@ghost
ghost force-pushed the feat/delegated-routing branch from 0dc0a04 to 287cbed Compare April 21, 2018 08:59
ghost pushed a commit to ipfs/infra that referenced this pull request Apr 23, 2018
Exposes API methods for delegated routing on the gateway.

License: MIT
Signed-off-by: Lars Gierth <larsg@systemli.org>
ghost pushed a commit to ipfs/infra that referenced this pull request Apr 23, 2018
@ghost
ghost force-pushed the feat/delegated-routing branch from 287cbed to 6c76de3 Compare August 22, 2018 15:54
@ghost

ghost commented Aug 28, 2018

Copy link
Copy Markdown
Author

I'm picking this back up this week -- let me know if there's reservations about the approach mentioned back in March:

We could maybe eliminate the root/rootRO duality and have :8080 call the same root cmds.Command as :5001, but with a configurable allowlist in front. The current rootRO would then be default config for that.

License: MIT
Signed-off-by: Lars Gierth <larsg@systemli.org>
@ghost
ghost force-pushed the feat/delegated-routing branch from 6c76de3 to 23a68cd Compare August 31, 2018 13:32
@jacobheun

Copy link
Copy Markdown
Contributor

@lgierth I think we may need to add some limiters on findProviders. I am testing this locally with an example of the current delegated routing implementation in js-libp2p. Other than limiting the number of providers there isn't currently any other limiter to the call. If I run ipfs dht findprovs for a newer CID and the number of providers is below 20 (the default), or the query takes a while to find 20, the connection stays open until at least 20 providers can be found.

For an http request to the delegated node, this can mean multiple http requests open for minutes at a time. For content I just added to my local go-ipfs instance, I've seen it exceed 30minutes for an open connection in browser.

@ghost

ghost commented Sep 4, 2018

Copy link
Copy Markdown
Author

@jacobheun In go-ipfs there's a -n/--num-providers (default=20) option on ipfs dht findprovs. But nothing similar on ipfs dht findpeer yet.

@ghost

ghost commented Sep 12, 2018

Copy link
Copy Markdown
Author

@jacobheun Just to confirm, do you need limiting on findprovs too?

@jacobheun

Copy link
Copy Markdown
Contributor

@lgierth my primary concern is too many open connections to the server. The current code for findprovs keeps the connection open until the default 20 providers are returned.

In the event that findprovs is called for new content by several peers, the requests from those peers will stay open until resolved (which can take a significant amount of time).

Even though I can lower --num-providers, it's not the default so most people won't use it, unless we explicitly lower it in the js module.

I'm wondering if a default timeout is needed for these requests?

@ghost

ghost commented Sep 14, 2018

Copy link
Copy Markdown
Author

Yeah sounds like some combination of --num-providers and --timeout is needed, and a reasonable default should be used in the delegated routing module.

@ghost

ghost commented Sep 14, 2018

Copy link
Copy Markdown
Author

@jacobheun Just to confirm, do you need limiting on findprovs too?

Eeeh what I meant to say here: do you need the limiting on findpeer too?

@jacobheun

Copy link
Copy Markdown
Contributor

@lgierth I think --timeout limiting for findpeer would be great.

@ghost

ghost commented Sep 14, 2018

Copy link
Copy Markdown
Author

Okay, cool -- --timeout already exists in go-ipfs, it's an option on the root command: ipfs --help

@jacobheun

Copy link
Copy Markdown
Contributor

Perfect, I can work on adding support and sane defaults for those in the routing modules.

@jacobheun

Copy link
Copy Markdown
Contributor

I've got the js-libp2p delegated routing work all ready to go and in review. I've got the timeouts configurable and currently defaulting to a max of 30 seconds. It's been verified against the example I demo'd a few weeks back, which is also in the PR for js-libp2p libp2p/js-libp2p#242

@ghost

ghost commented Sep 27, 2018

Copy link
Copy Markdown
Author

Update on the go side is that configurable Gateway API commands (aka "readonly API" commands) are ready, but I'm dealing with some nasty dependency issues - hopefully done by tomorrow.

@ghost ghost mentioned this pull request Oct 5, 2018
@ghost

ghost commented Oct 5, 2018

Copy link
Copy Markdown
Author

Closing this in favor of #5565

@ghost ghost closed this Oct 5, 2018
@ghost ghost removed the status/in-progress In progress label Oct 5, 2018
@ghost
ghost deleted the feat/delegated-routing branch October 5, 2018 02:52
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

need/author-input Needs input from the original author topic/commands Topic commands topic/gateway Topic gateway topic/interop Interoperability topic/libp2p Topic libp2p

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants