Discv5 issues

[zilm13]

We've finished implementation of discv5 without topics in Harmony and I have some feedback about the spec. A lot of work done, @fjl ,and we could move it further. Could we improve the spec in some of these ways?

1. FINDNODE message replies only with peers with exact distance from recipient. So, how could we find 50 closest nodes, for example? Spamming messages with 0-200 distances, all with empty answers, and then, messages with nodes starting to appear. Of course, we could start from 255 and go down, but we need closest, yeah?
2. Node ID derivation function is not defined anywhere in v5. Maybe it's somewhere in v4, but I have not found it in v5 spec.
3. Spec isn't written in DDD way and a lot of things have different names in spec and in Go implementation. For example, initiatorKey and recipientKey are called writeKey and readKey respectively. It's much better when all things have the same names in specification and in code, especially when you need to refer to the implementation to clarify understanding.
4. id field in ENR (not this spec, but related) could appear in any place. And it looks a bit oddly: you have the signature, but you don't have its type nearby, you parse pairs one by one until you have id. There is a chance, that your implementation is not compatible with such kind of id. So, my proposal is to put it next to the signature without id key pair: [signature, id, seq, k, v...]

Any thoughts on these topics?

[Mikerah]

The NodeID derivation can be found in the ENR spec. It's simply taking the keccak256 hash of the uncompressed public key. This is meant to be backwards compatible with the v4 spec.

[zilm13]

@Mikerah Oh I see, thank you. I'd highlight this.

[fjl]

Node ID derivation function is not defined anywhere in v5. Maybe it's somewhere in v4, but I have not found it in v5 spec.

It is important to keep this flexible in your client because other identity schemes may be defined later.

[fjl]

id field in ENR (not this spec, but related) could appear in any place. And it looks a bit oddly: you have the signature, but you don't have its type nearby, you parse pairs one by one until you have id

I understand your concern and we could've changed it the way you suggest. Unfortunately it is too late now. EIP-778 is two years old and we have already deployed it on the network.

[fjl]

FINDNODE message replies only with peers with exact distance from recipient. So, how could we find 50 closest nodes, for example? Spamming messages with 0-200 distances, all with empty answers, and then, messages with nodes starting to appear. Of course, we could start from 255 and go down, but we need closest, yeah?

See https://github.com/ethereum/devp2p/issues/79 and this comment for previous discussion about this. I think the FINDNODE parameter is good the way it is because there is no way to get the answer wrong. Getting nodes at exact distance is simplest to implement and also simplest to verify on the 'client' side.

The downside is that getting many neighbors from a single node is inefficient. But maybe that's fine because you're supposed to use more than one node for lookup anyway. Getting nodes at all distances from another node is pretty rare I think.

An alternative idea would be to return nodes with distance <= d.

We can still make changes to FINDNODE because discv5 is not released yet.

[zilm13]

@fjl so there are 2 simplest ways of implementation: 1) spamming node with hundreds of messages, 2) requesting 255 and down until first empty bucket/enough peers from this host. Why you may need nodes with distance 113 in any real network?

Generally, I don't see anything bad in requesting furthermost nodes from the peer (anyway distance is an artificial value), due to bucket definition you receive freshest peers in this case. But it looks a bit odd why all peers are requesting furthest nodes and requires some comments in specification.

[fjl]

And it looks a bit oddly: you have the signature, but you don't have its type nearby, you parse pairs one by one until you have id

Some more about this: In general, the identity scheme may use any fields from the ENR. The "v4" scheme uses "secp256k1" for verification and for deriving the ID. So you need to parse all fields before attempting to derive the ID anyway and "id" is nothing special.

[fjl]

so there are 2 simplest ways of implementation: 1) spamming node with hundreds of messages

I don't understand where your "hundreds of messages" are coming from. Can you explain how that would happen.

[fjl]

Can you link your lookup code maybe?

[zilm13]

so there are 2 simplest ways of implementation: 1) spamming node with hundreds of messages

I don't understand where your "hundreds of messages" are coming from. Can you explain how that would happen.

I want, say, 50 peers from every new node. If I start with distance 0 and go upwards, most replies would be empty. So, the best strategy for me is to start from 255 and go downwards.

[zilm13]

And it looks a bit oddly: you have the signature, but you don't have its type nearby, you parse pairs one by one until you have id

Some more about this: In general, the identity scheme may use any fields from the ENR. The "v4" scheme uses "secp256k1" for verification and for deriving the ID. So you need to parse all fields before attempting to derive the ID anyway and "id" is nothing special.

I understand this. The issue is that: you receive ENR with id, say, vX and to choose appropriate verifier/nodeId translator you need to parse whole message, though you may not support such id at all.

[zilm13]

@fjl Finally I think >= looks like reliable fix: For any distance N greater than 0 requested with FINDNODE message, we return nodes from buckets starting from N and going higher until k (value defining size of k-bucket) nodes found or buckets are over (256+).

With such approach we always could calculate from last node, where did it come from (from which bucket). And we could get all nodes from peer with several requests as bucket is not greater than k. And, of course, it fixes the need of sending a lot of requests for empty buckets.

[AgeManning]

We pick a random peer and search for buckets of that distance. If not enough peers are returned, we send two more subsequent requests, which go either side of the distance. If distance to the random peer was 254, next request would be 255 and next would be 253. We only send 3 and if we don't get enough peers, we try again later.

I borrowed this approach from the go-code, although not sure if it has changed since.

[zilm13]

@AgeManning I think it's crucial to expand network using closest peers. It will be more randomized compared to using furthermost peers

[fjl]

I don't know how to resolve this issue. @zilm13 can you explain your problem better? I'm not entirely opposed to changing FINDNODE so it returns nodes with distance >= the given one, just need to think it through some more.

[zilm13]

@fjl I'm trying to simulate both cases, I will ping back when result is available

[zilm13]

@fjl yeah, we'd better close this one, I'll re-open it targeted when new info is available.