Local Discovery Support with MDNS

[amydevs]

Description

As of polykey 1.2.1-alpha.20, js-mdns has been integrated for the sake of local discovery. Changes to nodes/CommandFind and related tests will need to be changed in order to support the scopes property on NodeAddress, and that some RPCCalls now return Array<NodeAddress> rather than NodeAddress.

Issues Fixed

None

Tasks

• 1. Change nodes/CommandFind to output multiple addresses
• 2. Amend related tests

Final checklist

• Domain specific tests
• Full tests
• Updated inline-comment documentation
• Lint fixed
• Squash and rebased
• Sanity check the final build

[ghost]

👇 Click on the image for a new way to code review

Legend

[CMCDragonkai]

Is this in relation to CI failure?

[CMCDragonkai]

Does this need to be merged BEFORE or AFTER #50?

[CMCDragonkai]

@amydevs what's the status of this now that #50 was merged?

[amydevs]

Am in the process of completing this.

I had the impression that the original findNode function used the ctx to determine how long getClosestNodes would be executed for. I had implemented findNodeLocal this way assuming this was the case. But now the command for nodes find fails on rpc timeout instead of ErrorCommandFindNodeNotFound or something similar. So i have to shorten the time out for the findNodeLocal function, so that it's shorter than ctx. But im not sure what to set it to.

[CMCDragonkai]

Regarding scopes, please read what I wrote here: MatrixAI/Polykey#537 (comment)

[CMCDragonkai]

I'm not really sure findNode is properly implemented in consideration of all the changes and features we have. I'm going over the entire nodes domain over in MatrixAI/Polykey#618 - probably major changes coming into there to the nodes domain @tegefaulkes

[amydevs]

I'm not really sure findNode is properly implemented in consideration of all the changes and features we have. I'm going over the entire nodes domain over in MatrixAI/Polykey#618 - probably major changes coming into there to the nodes domain @tegefaulkes

i've went over how my findNodeLocal works and created a config option for specifically the timeout of finding a local node. This was done, as there is a similar config option for finding nodes via kademlllia based on the no. of hops. I cannot use the ctx of the handler, as that is the ctx of the RPC call. Tthe RPC call will timeout before my MDNS resolves the known addresses if I use that as my ctx.

MatrixAI/Polykey#537 (comment)

[amydevs]

all tests are passing on CI, everything seems to work as intended. Merge is on the way.

[CMCDragonkai]

You forgot to do the final checklist.

[CMCDragonkai]

i've went over how my findNodeLocal works and created a config option for specifically the timeout of finding a local node. This was done, as there is a similar config option for finding nodes via kademlllia based on the no. of hops. I cannot use the ctx of the handler, as that is the ctx of the RPC call. Tthe RPC call will timeout before my MDNS resolves the known addresses if I use that as my ctx.

RPC call timeouts are relevant to the client. If the client times out before your MDNS finds the addresses, then either the client cares about the result, maybe you need to find it faster, or extend the timeout.

Or you should consider it a long-running call, where one get get results as a list over a long time, rather than a unary call. I would imagine it it's more of a server streaming call rather than a unary call.