Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Check if identity is already taken #511

Merged
merged 3 commits into from
Oct 8, 2022
Merged

Conversation

borzunov
Copy link
Member

@borzunov borzunov commented Oct 8, 2022

While using scripts built with hivemind, users often run two peers with the same identity by accident (e.g., if they forget to change the CLI command or copied the same identity file to another host via scp). Now, this leads to undefined behavior of libp2p.

This PR makes hivemind.P2P check if the identity is already taken, thus solving this issue in all applications at once. See the example below:

Screenshot 2022-10-08 at 05 22 11

Future work:

  • If initial_peers are not defined and use_ipfs = False (i.e., we are the first peer), there is no need to check anything

@borzunov borzunov requested a review from justheuristic October 8, 2022 01:04
@borzunov borzunov force-pushed the check-if-identity-taken branch 2 times, most recently from 8f75580 to 9ae7bef Compare October 8, 2022 01:14
@borzunov borzunov marked this pull request as ready for review October 8, 2022 01:15
@borzunov borzunov force-pushed the check-if-identity-taken branch from 9ae7bef to ff3a4cd Compare October 8, 2022 01:20
@borzunov borzunov force-pushed the check-if-identity-taken branch from ff3a4cd to 23719b1 Compare October 8, 2022 04:32
@codecov
Copy link

codecov bot commented Oct 8, 2022

Codecov Report

Merging #511 (e2fba4d) into master (e9f35b5) will increase coverage by 0.00%.
The diff coverage is 96.77%.

@@           Coverage Diff           @@
##           master     #511   +/-   ##
=======================================
  Coverage   75.98%   75.99%           
=======================================
  Files          81       81           
  Lines        7919     7947   +28     
=======================================
+ Hits         6017     6039   +22     
- Misses       1902     1908    +6     
Impacted Files Coverage Δ
hivemind/p2p/p2p_daemon_bindings/datastructures.py 75.42% <92.30%> (+1.83%) ⬆️
hivemind/p2p/p2p_daemon.py 94.70% <100.00%> (+0.24%) ⬆️
hivemind/dht/node.py 90.99% <0.00%> (-1.19%) ⬇️

@borzunov borzunov merged commit 64a6c30 into master Oct 8, 2022
@borzunov borzunov deleted the check-if-identity-taken branch October 8, 2022 15:34
borzunov added a commit that referenced this pull request Oct 11, 2022
- In `hivemind.Server`, use the graceful shutdown for `ConnectionHandler`
- In `hivemind.P2P`, if we are the first peer, skip checking if the provided identity is free
mryab pushed a commit that referenced this pull request Oct 19, 2022
While using scripts built with hivemind, users often run two peers with the same identity by accident (e.g., if they forget to change the CLI command or copied the same identity file to another host via `scp`). Now, this leads to undefined behavior of libp2p.

This PR makes `hivemind.P2P` check if the identity is already taken, thus solving this issue in all applications at once.

(cherry picked from commit 64a6c30)
mryab pushed a commit that referenced this pull request Oct 19, 2022
- In `hivemind.Server`, use the graceful shutdown for `ConnectionHandler`
- In `hivemind.P2P`, if we are the first peer, skip checking if the provided identity is free

(cherry picked from commit 13cdd13)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants