feat: add fetch protocol

[stbrody]

No description provided.

[stbrody]

all these changes to the random autogenerated protobuf code happened when I ran npm run build:proto && npm run build:proto-types on a totally clean checkout before I even started making any of my changes.

[stbrody]

If you want me to leave those changes out of this PR just let me know and I can unstage those files.

[achingbrain]

Yes please!

[stbrody]

Yep will do. I'm still worried though that the code generated for the new fetch protocol protobufs is going to have the same problems as clearly there's something in my local setup causing it to generate protobufs differently than what yall have here.

[stbrody]

@achingbrain @vasco-santos @aschmahmann PTAL!

CC @oed @smrz2001

[aschmahmann]

@vasco-santos @achingbrain this is needed to implement the updated IPNS over PubSub spec (from 2019) and might also be useful for other folks such as Ceramic who just need a basic "give me the data corresponding to key" protocol.

You may already be doing this, but the main technical note I'd give is that you should give room to pull from multiple data sources.

Side note: @stbrody I'm also realizing that it might be useful to work on a version of the fetch spec that can work with a single stream (i.e. not one stream per request) to more easily respect things like back pressure. It might not be a big deal for your use case, but something to consider.

[stbrody]

the main technical note I'd give is that you should give room to pull from multiple data sources

Yeah I thought about this a little bit. The API right now is very simple - when the protocol is registered the user provides a callback to map key to value. If an app developer wanted to use two different subsystems that both relied on the fetch protocol, the dev could easily wrap the two lookup functions from each subsystem into a single one that dispatches based on the key.

I feel like if we try to build some way to register multiple lookup callbacks to different ranges of the keyspace, it's just going to complicate the API and may wind up being too brittle and prescriptive for every use case. I think it might genuinely make sense to leave this to users, but I could be swayed if you disagree @aschmahmann

EDIT: actually I thought of a way that isn't too bad - if we force each subsystem to have a fixed prefix on all keys (instead of trying to do something like a range match or regex which would be too complicated I think). I can give a try to making that update when I'm back from vacation next week.

[stbrody]

I seem to be getting failures generating type information, but the errors are all coming from the autogenerated protocol buffer code: https://github.com/libp2p/js-libp2p/runs/4316690228?check_suite_focus=true

I'm wondering if this is related to why a bunch of autogenerated protobuf code got updated when I ran the build:proto commands locally even on a clean build. Maybe my local setup is using a different version of whatever package is generating the protobuf code somehow? Any advice where to look here to fix this would be appreciated.

[stbrody]

Okay, updated to be more modular and allow registering multiple key lookup functions for different fixed prefixes of the keyspace. Hopefully this addresses your main concern @aschmahmann

@achingbrain @vasco-santos

[stbrody]

@aschmahmann, I had an idea that I think would make this a bit simpler to use and understand. Instead of having a key prefix to dispatch request handlers, what if we added a new string field namespace to the fetch request protocol buffer message, alongside the existing identifier field, and registered handlers based on namespace? Then we don't have to worry about collisions on the identifier so long as they are in different namespaces. The one thing is that this would cause a bit of a divergence from what is currently implemented in go-libp2p, but I think it'd be a cleaner design longer term? What do you think?

[achingbrain]

This is great, thanks so much for submitting it. Just a few minor comments for consistency.

The implementation needs to be mounted in https://github.com/libp2p/js-libp2p/blob/master/src/index.js and exposed on the libp2p class to let consumers configure lookup functions - see how .indentifyService and the ping protocol are configured for an example.

The generated .js file also needs to be added to the exclude list in https://github.com/libp2p/js-libp2p/blob/master/tsconfig.json - this plus reverting the changes to the generated files should get linting passing in CI.

Finally please merge master/rebase as it's had some changes made that make the build a lot more stable.

Ideally some tests should be added to https://github.com/libp2p/interop to ensure it works across implementations but I don't know how feasible that is as it uses https://github.com/libp2p/go-libp2p-daemon which is a bit out of date.

[stbrody]

Okay, ready for another round of review. I reverted the auto-generated protobuf code, merged in master, and added error codes. PTAL

[achingbrain]

Thanks so much for adding this. I've tided some things up to make it more consistent with the existing code, added some extra tests and docs, and exposed the .fetch command as a top-level libp2p method.

I've also changed the protocol name to be consistent with go-libp2p, otherwise we won't have interop.

[achingbrain]

@stbrody this is now available in libp2p under the next tag - could you please try it out and report back any observed issues?

$ npm install libp2p@next

[stbrody]

All changes look great, thanks @achingbrain!!! I'm excited to try this out. Hopefully I'll have time to check it out in the upcoming 2 week sprint.

I'm still curious though what you and @aschmahmann think about the idea of using an explicit namespace instead of the 'key prefix' concept that is how it's implemented today. Doing string parsing always makes me feel gross - I think it'd be much cleaner and easier to understand and reason about with an explicit 'namespace' instead.