Kademlia Records

[montekki]

No description provided.

[tomaka]

A few remarks:

• As explained in Implement Kademlia record store #146, I think that it should be the responsibility of the user to store records.
• The Kademlia iterative query should stop whenever it finds a record. However this is vulnerable to misbehaviour (a node can return a bad record and stop the iterative query by itself). We need some sort of way to ask the user whether a record is satisfactory.

However I think it is fine as a first draft and this can be merged before doing these.

@montekki Did you try if it works?

[montekki]

@tomaka

• ok
• It doesn't look so hard to implement the quorum logic from the specs actually

About works: There are some tests and there is also a more complicated test (not pushed yet) based off the chain topology in query_iter test. It works but I am not really satisfied with how, i.e. in the topology of a chain the value is now only distributed to the "next" neighbor that we are connected to, that I am trying to find out what to do with now.

I would argue for at least

• implementing the logic of PUT_VALUE in a way that data is distributed to some decent amount of nodes
• implementing the logic of GET_VALUE with quorum-ing
• probably implementing validation

before merging this

[romanb]

I would argue for at least

• implementing the logic of PUT_VALUE in a way that data is distributed to some decent amount of nodes

But that is already happening, as far as I can tell. What you implemented is the standard iterative PUT_VALUE that stores the record on the (up to) k closest nodes to the key. Of course this is generally already assuming a properly bootstrapped / initialised routing table. For persistence, the record would need to be replicated and regularly republished, of course, but that is maybe beyond the scope of this PR. I do think, however, that there should be at least either already an expiration on the records, or the size of the record store (current hashmap) should be capped, which may anyway be necessary eventually.

• implementing the logic of GET_VALUE with quorum-ing

I think it would be fine to just stop on the first result for now, which is the "standard" Kademlia. The quorum scheme with the Q parameter seems to be an extension of libp2p that could be added later.

• probably implementing validation

What would you want to do to validate the records? Validation seems to be something that is just offered as an API to user code, i.e. such that user code can do whatever validation it likes. Do you mean you would already want to provide an interface for validation?

[montekki]

What would you want to do to validate the records? Validation seems to be something that is just offered as an API to user code, i.e. such that user code can do whatever validation it likes. Do you mean you would already want to provide an interface for validation?

The Record has some fields for signatures being passed around doesn't it? We probably should check them and when we are asked for some value, we should send out the record with the signatures

[romanb]

What would you want to do to validate the records? Validation seems to be something that is just offered as an API to user code, i.e. such that user code can do whatever validation it likes. Do you mean you would already want to provide an interface for validation?

The Record has some fields for signatures being passed around doesn't it?

Not any longer, as far as I know. See the latest protobuf

[tomaka]

The Kademlia iterative query should stop whenever it finds a record. However this is vulnerable to misbehaviour (a node can return a bad record and stop the iterative query by itself). We need some sort of way to ask the user whether a record is satisfactory.

I just realized the "stupidity" of that remark.

In the regular Kademlia protocol, the key of a record is its hash, which eliminates the question of knowing whether a record is valid.

In the situation of Polkadot, a record would be valid if it contains a signature using the key of the record as a public key.

In order to solve this question, I think that we can simply introduce a trait that checks the validity in a self-contained way (ie. a pure function).
We could also add a template parameter to Kademlia representing the type of a record, and require a TryFrom<UnverifiedRecord> trait implementation.

[romanb]

There are still some correctness issues with get_value that need to be addressed.

[romanb]

Thanks for getting this started.

[tomaka]

Good job! A couple minor nits that should be quick to fix.

[tomaka]

As mentioned, I think the methods here should be asynchronous. We should open an issue about that when the PR gets merged.

[montekki]

When this is merged, it would be great to avoid including whole history of 120+ commits into the squashed commit btw