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

Fix direct peers #5427

Merged
merged 5 commits into from
Sep 15, 2023
Merged

Fix direct peers #5427

merged 5 commits into from
Sep 15, 2023

Conversation

Menduist
Copy link
Contributor

Ref #5278

The libp2p sides fixes are here: vacp2p/nim-libp2p#949

I would like to add a test to the CI to makes sure it actually works, but not sure what's the best way to do so

@Menduist Menduist marked this pull request as draft September 13, 2023 14:12
@arnetheduck
Copy link
Member

best way to do so

I imagine making one of the peers in the local testnet script become a direct peer and disable all of the other ways it has of getting peers is the easiest thing to do - this "should" cause the local testnet to fail if the connection is not maintained.

@github-actions
Copy link

github-actions bot commented Sep 13, 2023

Unit Test Results

         9 files  ±0    1 089 suites  ±0   45m 14s ⏱️ + 7m 42s
  3 847 tests ±0    3 568 ✔️ ±0  279 💤 ±0  0 ±0 
15 892 runs  ±0  15 587 ✔️ ±0  305 💤 ±0  0 ±0 

Results for commit 72454d7. ± Comparison against base commit d7aaf1b.

♻️ This comment has been updated with latest results.

@Menduist
Copy link
Contributor Author

Done, though was a bit bloody as we need to know the ENRs of two nodes before-hand.
Will merge the libp2p fix, and then this one will be ready for merge

connection to. This requires a not random netkey-file. In the multiaddress
format like: /ip4/<address>/tcp/<port>/p2p/<peerId-public-key>, or enr format
(enr:-xx). Peering agreements are established out of band and must be
reciprocal..
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
reciprocal..
reciprocal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, confutils is adding a dot, funny

@Menduist Menduist marked this pull request as ready for review September 15, 2023 15:26
@arnetheduck arnetheduck enabled auto-merge (squash) September 15, 2023 15:33
@arnetheduck arnetheduck merged commit 4918a4e into unstable Sep 15, 2023
10 checks passed
@arnetheduck arnetheduck deleted the fixdirectpeers branch September 15, 2023 18:45
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.

None yet

2 participants