Skip to content
This repository has been archived by the owner on Dec 17, 2024. It is now read-only.

Add bootstrap node and peer list exchange and automatique connection #40

Merged
merged 6 commits into from
Jan 21, 2024

Conversation

musitdev
Copy link
Contributor

Add node peers list propagation.
When a node connects to another one, they exchange their peer list. If there's a missing connection, the node connects to the missing node. So one node connected to another, it gets all not connected nodes and connect to them.

To start, a bootstrap node must be specified. This part has not been changed. The node use the config to get one peer to connect to. When it connects, it gets all peer list and open a connection to them.

It's a raw version but seems to work.

Some edge cases are not completely managed. For example, if 2 nodes (2,3) connect at the same time to one node 1. Node 2 and 3 get the other address from node 1 and try to connect each other at the same time. So it opens 2 connections between both nodes, which is not allowed. So the lib close one. From my tests, only one is closed and not both.

Node disconnection is not completely tested. From my tests, to be notified of a disconnection, the disconnect function must be called. I'll do more test with real host to see how it works. Node disconnection remove unused nodes from the exchanged peer list.

I've tested using the unit test. I create 2 tests that seems to work locally.

@musitdev musitdev requested a review from tuommaki January 18, 2024 20:49
Copy link
Contributor

@tuommaki tuommaki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor notes. In general LGTM 🙂

@@ -22,6 +22,7 @@ hex = "0.4"
home = "0.5"
jsonrpsee = { version = "0.20", features = [ "client", "server" ] }
libsecp256k1 = "0.7"
log = "0.4.20"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accidental leftover?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I miss it. Removed.

crates/node/src/networking/p2p.rs Show resolved Hide resolved
crates/node/src/networking/p2p.rs Outdated Show resolved Hide resolved
crates/node/src/networking/p2p.rs Show resolved Hide resolved
crates/node/src/networking/p2p.rs Show resolved Hide resolved
crates/node/src/networking/p2p.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@tuommaki tuommaki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides the log dependency, this LGTM 👍

@tuommaki tuommaki merged commit 6246c4e into proto Jan 21, 2024
4 checks passed
@tuommaki tuommaki deleted the add_bootstrap_node branch January 21, 2024 17:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants