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

Add inbound connections to client pool #318

Closed
LLFourn opened this Issue Oct 8, 2018 · 4 comments

Comments

Projects
4 participants
@LLFourn
Copy link
Contributor

LLFourn commented Oct 8, 2018

BamClientPool: https://github.com/tenx-tech/swap/blob/3d3bd5816ddb42108baf291977de06f59dfbc417/application/comit_node/src/comit_client/bam.rs

Is meant to be the source of connections for other nodes. However if you get connected to by another party and then want to send them a message you will have to create a new connection because the existing one is not stored.

When the server receives a new connection it should add the client to BamClientPool under its SocketAddress and we should index the clients by socket address (for now).

DoD:

  • GET /peers on Node2 returns Node1 network details once Node1 has sent a SWAP request to Node2
  • GET /peers on Node1 and Node2 returns only node network details once Node2 has sent a SWAP request to Node1 (this goes after test above)

Blocked by #555
Blocked by #620
Blocked by Noise and Pubkeys (@LLFourn to open Noise ticket)

Note:

Ensuring that an old connection can be reused is out of scope for this ticket, because it requires connections to be uniquely identified.

@LLFourn LLFourn added this to To Refine in Swap Beta via automation Oct 8, 2018

@LLFourn LLFourn added this to the Open-Source milestone Oct 8, 2018

@D4nte

This comment has been minimized.

Copy link
Member

D4nte commented Oct 12, 2018

I suggest we wait to implement public keys to identify nodes before optimizing this behaviour so that the indexing is done per public key.
Am I understanding this correctly?

@LLFourn

This comment has been minimized.

Copy link
Contributor Author

LLFourn commented Oct 15, 2018

@D4nte I think we can do this before then. First indexed by IP then by public key.

@bonomat bonomat removed this from the Open-Source milestone Oct 29, 2018

@D4nte D4nte modified the milestone: TenX Xmas Summit Demo Nov 7, 2018

@D4nte

This comment has been minimized.

Copy link
Member

D4nte commented Nov 14, 2018

Only needed once a previously Bob node becomes a Alice node.

@D4nte D4nte added Epic and removed Epic labels Nov 14, 2018

@D4nte D4nte added the icebox label Nov 22, 2018

@D4nte D4nte changed the title Connections made by server aren't added to BamClientPool Allow re-use of inbound connections Jan 7, 2019

@D4nte D4nte added this to the Sprint 5 📜🔓 milestone Jan 9, 2019

@luckysori luckysori self-assigned this Jan 9, 2019

@luckysori luckysori changed the title Allow re-use of inbound connections Add inbound connections to client pool Jan 9, 2019

@luckysori luckysori referenced this issue Jan 9, 2019

Merged

Add inbound connections to client pool #630

1 of 1 task complete

@wafflebot wafflebot bot added review and removed work-in-progress labels Jan 9, 2019

@D4nte

This comment has been minimized.

Copy link
Member

D4nte commented Jan 10, 2019

As discussed:

  • can't index by IP address only (e.g. all node have same IP in tests), need port
  • can't index by IP and port as it depends on the direction of the connection: Alice may listen to 1.1.1.1:1000 but if she connects to Bob, Bob will see her as 1.1.1.1:20000 (random outbound port).

Hence, the node id needs to be done first.

@mergify mergify bot closed this in #630 Jan 14, 2019

@wafflebot wafflebot bot removed the review label Jan 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment