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

feat: adds transitive sharing of peer information #618

Merged
merged 2 commits into from
Dec 9, 2024

Conversation

nathanielc
Copy link
Collaborator

With this change all the pieces are in place such that peers can discover peers from connected peers and connect to those peers.

Fixes: #605
Fixes: #606

@nathanielc nathanielc force-pushed the feat/peer-recon-ring branch 2 times, most recently from 5b74e9d to 9e8d80e Compare November 26, 2024 21:14
@nathanielc nathanielc force-pushed the feat/pairwise-peer-sync branch from a9931a3 to 285bc5f Compare November 26, 2024 23:04
assert!(is_bi_connected(&test_runner_a, &test_runner_b).await?);
assert!(is_bi_connected(&test_runner_b, &test_runner_c).await?);
// We expect that a and c find each other through b and become connected
assert!(is_bi_connected(&test_runner_a, &test_runner_c).await?);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the key test, it shows peers can find each other transitively and connect.

@nathanielc nathanielc force-pushed the feat/peer-recon-ring branch from 4892678 to d9cdbec Compare December 4, 2024 22:04
@nathanielc nathanielc force-pushed the feat/pairwise-peer-sync branch from 285bc5f to 2d040d1 Compare December 5, 2024 19:36
@nathanielc nathanielc marked this pull request as ready for review December 5, 2024 20:22
@nathanielc nathanielc requested a review from a team as a code owner December 5, 2024 20:22
@nathanielc nathanielc requested review from stbrody and dav1do and removed request for a team December 5, 2024 20:22
Base automatically changed from feat/peer-recon-ring to main December 5, 2024 20:49
With this change all the pieces are in place such that peers can
discover peers from connected peers and connect to those peers.

Fixes: #605
Fixes: #606
Copy link
Contributor

@dav1do dav1do left a comment

Choose a reason for hiding this comment

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

One question about the tests but overall looks good!

p2p/src/peers.rs Show resolved Hide resolved
p2p/tests/node.rs Outdated Show resolved Hide resolved
p2p/tests/node.rs Outdated Show resolved Hide resolved
@nathanielc nathanielc enabled auto-merge December 6, 2024 21:24
@nathanielc nathanielc added this pull request to the merge queue Dec 6, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 6, 2024
@nathanielc nathanielc added this pull request to the merge queue Dec 6, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 6, 2024
@dav1do dav1do added this pull request to the merge queue Dec 8, 2024
@dav1do
Copy link
Contributor

dav1do commented Dec 8, 2024

This PR failed the merge queue due to a test I wrote. Looked at it to see if there was potential flaky behavior and I don't see any. It's been passing and I can't reproduce so I'm a bit confused.. somehow the last event wasn't ordering in the failure. Submitted it again to see if it fails again.

Merged via the queue into main with commit becc8b0 Dec 9, 2024
5 checks passed
@dav1do dav1do deleted the feat/pairwise-peer-sync branch December 9, 2024 00:22
@smrz2001 smrz2001 mentioned this pull request Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants