Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Expose PING and FINDNODE rpc to public API #90

Closed
ogenev opened this issue Oct 24, 2021 · 6 comments
Closed

Expose PING and FINDNODE rpc to public API #90

ogenev opened this issue Oct 24, 2021 · 6 comments

Comments

@ogenev
Copy link
Contributor

ogenev commented Oct 24, 2021

We at Trin would benefit greatly if we have public API access to raw PING and FINDNODE RPC via Discv5 pub struct. For example, something like send_ping(enr: Enr) and send_findnode(enr: Enr, distances: Vec<u64>) would be great.

Are you guys open to such a change? If you agree, we can discuss the implementation approach and I can work on a PR to commit this upstream?

@AgeManning
Copy link
Member

I'd be curious to know a bit more about the purpose of these. In a way, you already have access to these, just not via the discv5 struct.

The design currently has been split into layers, from top down:

  1. discv5 - This is the user-level API where you can do basic functions used for discovery. Users who want a discovery protocol essentially use this struct for searching for peers etc.
  2. service - This builds all DHT information and peer storage etc. It adds the 'kademalia' part of discv5 and can exist on top of any lower level connection or RPC framework. i.e This can be used without the discv5 protocol itself.
  3. handler - This layer manages all the discv5 protocol stuff. It handles encryption/decryption and all the specific discv5 RPC protocols. It can do raw request/responses such as PING and FINDNODE and handles all the protocol-level encryption for you.
  4. Socket - This guy handles all the raw udp traffic and packet management.

I'm not sure about your exact needs, but the design was such that anyone can come along and just spawn a handler This is a service, that you can send PING, FINDNODE and all the RPC requests to and obtain responses from. It has no knowledge of all the DHT and fancy query logic. It might suit your needs to just run this service as is.

For example you can run the service and call this function: https://github.com/sigp/discv5/blob/master/src/handler/mod.rs#L452 and set the request to be either a PING or a FINDNODE and it will return the responses on the return channel it creates when you spawn it: https://github.com/sigp/discv5/blob/master/src/handler/mod.rs#L240

@ogenev
Copy link
Contributor Author

ogenev commented Oct 26, 2021

Hey @AgeManning, thank you for the thoughtful examples! The layered architecture looks really cool!

Although, I'm afraid that we want to use all DIscovery v5 functionalities including discovery protocol, the service for managing the DHT, and to have the possibility to send raw PING and FINDNODE messages.

To accomplish this, I guess we need to spawn at least a service and a handler. It looks like Service::spawn is spawning a handler for us in the background, but we don't have access to it. If I understand this correctly, we can send ServiceRequest but then we need to implement upstream modified QueryKind to match PING and FINDNODE {distances: Vec) requests.

Is there any workaround I'm missing here to spawn a custom handler with a service for managing the DHT?

@AgeManning
Copy link
Member

AgeManning commented Oct 27, 2021

The current thread model is:

discv5 <-> service <-> handler <-> socket.

So there are usually 4 long-running tasks. The <-> represent channels between the async tasks. The service creates a handler task and provides all the discv5 basic query logic (as you've pointed out). We could still send the recv events that the service gets from the handler to the discv5 struct.

There's a few issues with doing this:

  • We'd have to be careful with the impact of FindNode requests. If the user could randomly send these, the service would add relevant peers to its local DHT which could have undesired effects. We might have to tag the request to avoid all DHT related functionality and just send the response back to the discv5 struct.
  • It would break the semi-nice property that the higher tasks are somewhat independent of the tasks beneath them. i.e The discv5 public functions are somewhat generic and not tied to the discv5 RPC protocols.

We could still do it however, with not too much effort. An alternative would be to spawn a separate handler. Although this would likely be on a separate UDP port. I'm not sure if you need to to look like a single node identity.

If you explain the use-case a bit more, maybe there are some alternative options also

@AgeManning
Copy link
Member

Another thing we need to be careful of is that exposing a public function like send_findnode() is somewhat dangerous to users.

For a discovery mechanism, users should not be finding or using results from a FINDNODE directly. It opens them up to possible eclipse attacks if they use this function as a discovery query.

Perhaps we gate this functionality behind a feature flag so the average user doesn't have these raw functions exposed

@ogenev
Copy link
Contributor Author

ogenev commented Nov 1, 2021

Thanks for the discussion, it is really helpful!

It seems our use cases are not very clear right now. The next milestone for Portal Network is running a test network on top of discv5 protocol and our assumption is that we may need to send raw PING and FINDNODE base layer discv5 RPCs mainly for testing and development purposes.

Maybe we should wait and see if we will really need those raw base layer RPCs and then look for a way to implement them.

@AgeManning
Copy link
Member

Sure. It sounds to me like if you need them in conjunction with all the DHT logic and everything provided in the service, then we can add it, but we should do it via a compile-time feature flag, such that they are not exposed to regular users.

Let's re-open this when you need.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants