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

xud-level P2P reconnections are attempted from both sides #1135

Closed
kilrau opened this issue Aug 8, 2019 · 3 comments
Closed

xud-level P2P reconnections are attempted from both sides #1135

kilrau opened this issue Aug 8, 2019 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@kilrau
Copy link
Contributor

kilrau commented Aug 8, 2019

Background

xud-level P2P reconnections are attempted from both sides of a former p2p connection.

Your environment

  • version of xud: latest master
  • which operating system (uname -a on *Nix): ubuntu 18
  • version of btcd, bitcoind, ltcd, litecoind, geth, parity, raiden, lnd or other chain backend:
  • any other relevant environment details:

Steps to reproduce

Connect two xud instances via manual connect on xud level. Restart both instances with e.g. network off and watch both instances trying to reconnect.

Expected behaviour

Only the connection initiator tries to reconnect

Actual behaviour

Both peers try to reconnect

@kilrau kilrau added the bug Something isn't working label Aug 8, 2019
@erkarl erkarl self-assigned this Aug 8, 2019
@sangaman
Copy link
Collaborator

@kilrau Are both nodes being restarted at the same time? Consider the following situation:

  1. Node A connects to Node B and advertises a listening connection.
  2. Node B is shut down.
  3. Node B comes back online but no longer accepts incoming connections.
    4A. (Current behavior) Node B reconnects to Node A using the advertised address from step 1.
    4B. (Proposed behavior) The nodes will not reconnect.

I'm wondering what is the downside of having potentially two connection attempts in a case like this. The second one should fail gracefully due to the nodes already being connected, if not that seems like the real issue.

See my comments on #1145 for more details about my concerns.

@erkarl
Copy link
Collaborator

erkarl commented Aug 16, 2019

Thank you @sangaman for your input. What you said makes a lot of sense and I think we shouldn't restrict connection attempts to outbound connections only.

@erkarl erkarl closed this as completed Aug 16, 2019
@kilrau
Copy link
Contributor Author

kilrau commented Aug 16, 2019

Even though originally we did not intend to re-connect to advertised URIs we never connected to before, now with TOR this behavior is actually desirable (since probability of advertised URIs being reachable is high) and this improves reconnections as per @sangaman description above. No need to change anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants