go-libp2p-daemon icon indicating copy to clipboard operation
go-libp2p-daemon copied to clipboard

feat: add peerstore spec and protobuf

Open jacobheun opened this issue 5 years ago • 10 comments

This adds a subset of commands to interact with the Peerstore.

  • ADD_PROTOCOLS, GET_PROTOCOLS, GET_PEER_INFO

Resolves https://github.com/libp2p/go-libp2p-daemon/issues/89

jacobheun avatar Mar 20 '19 11:03 jacobheun

There are more functions in the Peerstore interface. Also, don't expose SET_PROTOCOLS, this is used internally by identify I think.

vyzo avatar Mar 20 '19 12:03 vyzo

The only other two methods I can see being useful to expose would be

  • GET_PEER_INFOS, get PeerInfo's for a list of peer id's and,
  • GET_PEERS, get a list of peer id's in the Peerstore.

Probably also the majority of the AddrBook.

SupportsProtocols seems unnecessary since we're exposing GetProtocols. Close and SetProtocols (as you mentioned) probably shouldn't be exposed.

jacobheun avatar Mar 20 '19 12:03 jacobheun

There are more interfaces it implements. And again, we don't want to expose the SET_PROTOCOLS method, as it is pointless to set it unless the peer advertises a protocol -- expose SUPPORTS_PROTOCOLS instead.

vyzo avatar Mar 20 '19 13:03 vyzo

And again, we don't want to expose the SET_PROTOCOLS method

It's not implemented and I don't intend to add it, are you referring to ADD_PROTOCOLS?

Is SUPPORTS_PROTOCOLS even necessary though? It's just a convenience check on top of GET_PROTOCOLS the client could easily perform themselves.

jacobheun avatar Mar 20 '19 13:03 jacobheun

It's not implemented and I don't intend to add it, are you referring to ADD_PROTOCOLS?

Yes, both are not to be used willy nilly.

Is SUPPORTS_PROTOCOLS even necessary though? It's just a convenience check on top of GET_PROTOCOLS the client could easily perform themselves.

fair enough.

vyzo avatar Mar 20 '19 14:03 vyzo

I've added some more types based on the available interfaces. If those look good I will add accompanying specs for those requests. I didn't include the PeerMetadata interface for now as I'm not quite clear what that would look like at the moment.

jacobheun avatar Mar 20 '19 14:03 jacobheun

Now this is probably too much; we need to be a little more selective for utility here :)

vyzo avatar Mar 20 '19 15:03 vyzo

Here's an updated list. I removed metrics as that's probably unnecessary at this point. I think this gives us a good base for interacting with the Peerstore.

Peer operations GET_PROTOCOLS so we can retrieve all known protocols for a peer GET_PEER_INFOS so we can get PeerInfo for any number of PeerIds GET_PEERS get all peers currently in the Peerstore. LIST_PEERS currently only returns connected peers.

Address operations These should account for basic address operation needs ADD_ADDRS SET_ADDRS GET_ADDRS

Key Operations I think this may be the only thing we may want to expose from the Keybook PUB_KEY

jacobheun avatar Mar 20 '19 17:03 jacobheun

That looks better. @bigs opinions?

vyzo avatar Mar 21 '19 07:03 vyzo

cc @raulk

vyzo avatar Mar 21 '19 11:03 vyzo