Skip to content

fix(code/discovery): connection upgrades and multiaddr issues #1060

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

bastienfaivre
Copy link
Member

@bastienfaivre bastienfaivre commented May 27, 2025

Closes: #1056


PR author checklist

For all contributors

For external contributors

@bastienfaivre bastienfaivre marked this pull request as draft May 27, 2025 10:01
Copy link

codecov bot commented May 27, 2025

Codecov Report

Attention: Patch coverage is 76.50273% with 86 lines in your changes missing coverage. Please review.

Project coverage is 76.95%. Comparing base (82186b9) to head (a893a29).

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1060      +/-   ##
==========================================
+ Coverage   76.31%   76.95%   +0.64%     
==========================================
  Files         159      159              
  Lines       16512    16588      +76     
  Branches    16512    16588      +76     
==========================================
+ Hits        12600    12764     +164     
+ Misses       3101     2998     -103     
- Partials      811      826      +15     
Flag Coverage Δ
integration 76.74% <76.50%> (+0.63%) ⬆️
mbt 8.03% <0.00%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
core 85.45% <ø> (ø)
engine 74.59% <ø> (-0.08%) ⬇️
app 86.32% <ø> (ø)
starknet 73.25% <100.00%> (-0.06%) ⬇️

@bastienfaivre
Copy link
Member Author

Discovery now supports all the listening addresses of a node, especially when listening on 0.0.0.0. Moreover, the connection upgrade issue has been fixed by changing the connection philosophy: we now follow the same principle as in libp2p, where there can be multiple connections to a single peer. Therefore, the number of connections is now counted at the peer level, not the connection level. This introduces a new parameter max_connections_per_peer in the discovery config.

@bastienfaivre bastienfaivre marked this pull request as ready for review June 5, 2025 23:06
@bastienfaivre bastienfaivre requested a review from adizere as a code owner June 5, 2025 23:06
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.

code: Discovery issues
1 participant