feat p2p_daemon: add API to call peer handle #181
Conversation
hivemind/p2p/p2p_daemon.py
Outdated
|
|
||
| def _initialize(self, proc_args: tp.List[str]) -> None: | ||
| self._host_port = find_open_port() | ||
| self._daemon_listen_port = find_open_port() |
There was a problem hiding this comment.
Note: these ports can randomly become equal.
I do not insist, but i'd recommend:
- supporting an option to specify host/listen port manually
- switching to a with find_open_port(...) version that guarantees that the port is unique
Both of these can be done here or in a subsequent PR, whichever you prefer.
|
[note: we can merge this despite RTFD build failure because this failure will be automatically resolved in #183 . The target branch for this merge is libp2p, not master.] |
hivemind/p2p/p2p_daemon.py
Outdated
| chunks.append(data) | ||
| curr_length += len(data) | ||
|
|
||
| return pickle.loads(b''.join(chunks)) |
There was a problem hiding this comment.
If the stream data may come from another computer (now or in future), unpickling it may lead to remote code execution, a serious security risk (see #155). To overcome this, you can use MSGPackSerializer and implement sending exceptions as a special case.
There was a problem hiding this comment.
I suggest that we discuss possible security risk at the meeting, and fix this issue later if necessary
| configargparse>=1.2.3 | ||
| trio==0.16.0 |
There was a problem hiding this comment.
nit: Replace == to >= to allow updating trio and modules depending on it?
There was a problem hiding this comment.
You have to install exactly this version to make p2pclient work with python 3.8. Otherwise import errors arise.
|
|
||
| while curr_length < content_length: | ||
| async def _receive_exactly(self, stream, n_bytes, max_bytes=1 << 16): | ||
| while len(self._buffer) < n_bytes: | ||
| data = await stream.receive_some(max_bytes) |
There was a problem hiding this comment.
I suggest to replace self._buffer with a local variable buffer, remove max_bytes, and just write:
data = await stream.receive_some(n_bytes - len(buffer))Then you'll never get more data than needed, so you won't need to save the remainder in self._buffer.
There was a problem hiding this comment.
@skobellev Please note this comment as well :)
(I've approved the PR in advance while assuming that you'll fix it :) )
* Extend P2P api * Add tests for new api * Add p2pclient dependencies * Test P2P from different processes * Fix typo in tests * Add default initialization * Fix daemon ports assignment * Replace del with __del__ in tests * Read from input stream with receive_exactly Co-authored-by: Ilya Kobelev <ilya.kobellev@gmail.com>
* Extend P2P api * Add tests for new api * Add p2pclient dependencies * Test P2P from different processes * Fix typo in tests * Add default initialization * Fix daemon ports assignment * Replace del with __del__ in tests * Read from input stream with receive_exactly Co-authored-by: Ilya Kobelev <ilya.kobellev@gmail.com>
* Extend P2P api * Add tests for new api * Add p2pclient dependencies * Test P2P from different processes * Fix typo in tests * Add default initialization * Fix daemon ports assignment * Replace del with __del__ in tests * Read from input stream with receive_exactly Co-authored-by: Ilya Kobelev <ilya.kobellev@gmail.com>
* Extend P2P api * Add tests for new api * Add p2pclient dependencies * Test P2P from different processes * Fix typo in tests * Add default initialization * Fix daemon ports assignment * Replace del with __del__ in tests * Read from input stream with receive_exactly Co-authored-by: Ilya Kobelev <ilya.kobellev@gmail.com>
Relates: #177