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

Eliminate GetOfflineLinkService method from LinkService interface #4152

Closed
wants to merge 1 commit into from

Conversation

kevina
Copy link
Contributor

@kevina kevina commented Aug 19, 2017

This eliminates the GetOfflineLinkService method to make ipfs/go-ipld-format#8 easier and also avoid the need for #4009, at least for now.

It does it by keeping the method but instead exposing the type create by NewDAGService so that implementation specific methods can be called, with one of them being GetOfflineLinkService. The GC function now expects an offline LinkService. To help reinforce this notion a trivial type OfflineLinkService is used.

License: MIT
Signed-off-by: Kevin Atkinson <k@kevina.org>
@Stebalien
Copy link
Member

I'm not sure about replacing that interface with a concrete type; I believe we want the dag service to remain replacable. Also, I've run into the need for an offline NodeGetter/DAGService elsewhere (pubsub) so we'll probably want some form of "don't go online" switch anyways.

@kevina
Copy link
Contributor Author

kevina commented Aug 19, 2017

The idea behind this p.r. is to get things moving until we can figure out a better interface. If the time comes when we want to swap out the DAGService with something else in an IPFS node (as that is the only place the concrete type is used) we can worry about it then.

@kevina
Copy link
Contributor Author

kevina commented Aug 19, 2017

Also specific examples of where an offline NodeGetter/DAGService is used in pubsub will be helpful.

@Stebalien
Copy link
Member

Also specific examples of where an offline NodeGetter/DAGService is used in pubsub will be helpful.

So, for background:

  1. I've been working on adapting PubSub to use IPLD for messages. Instead of sending arbitrary data, one would send a CID of an IPLD object along with (optionally) a dag rooted at this CID.
  2. For security (DoS) reasons, we need to be able to validate pubsub messages before we forward them (otherwise, it's trivial to DoS pubsub).

I'd like to be able to validate received DAGs using both blocks attached to the messages and blocks already present on the local node but I do not want nodes to start bitswaping for blocks during validation; otherwise, peers could trivially trick other peers into downloading arbitrary data.

Really, on thinking about this, fetching these nodes isn't the primary issue, it's persisting and providing them. This brings us back to that issue about adding hints to contexts.

@kevina
Copy link
Contributor Author

kevina commented Sep 1, 2017

@Stebalien another option is to do what is done within IPFS and get an offline NodeGetter via out of band means.

@Stebalien
Copy link
Member

I won't have access to that from within pubsub. When the pubsub is constructed, it will be passed a DAGService (or something like that depending on how that refactor falls out) and it won't really have access to the rest of the system.

This problem actually extends past just pubsub/offline. Any library operating over IPLD will want access to a DAGService and will need to be able to pass hints like:

  • Don't persist these, they're temporary.
  • Persist this, I need it (i.e., pinning).
  • Just give this to me if you have it, don't do any work getting it.
  • Give this to me, here are my credentials.
  • Store this but only make it accessible with certain credentials.

That's one of the nice things about stuffing this stuff in a context.


Actually, one way to do this could be to pass a "dead" (already canceled) context (and fix everything to check if the context has been canceled before performing any network requests). Really, this should already work.

@Stebalien Stebalien mentioned this pull request Jan 25, 2018
1 task
@dirkmc dirkmc mentioned this pull request Jan 27, 2018
@Stebalien
Copy link
Member

Superseded by #4610.

@Stebalien Stebalien closed this Jan 30, 2018
@Stebalien Stebalien deleted the kevina/kill-get-offline branch February 28, 2019 21:56
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.

2 participants