-
Notifications
You must be signed in to change notification settings - Fork 891
remove deprecated UDP TPU socket bindings #9291
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
base: master
Are you sure you want to change the base?
Conversation
|
hi @alexpyattaev sir the tests are failing here because this PR depends on the changes in #9299 should i merge the changes from #9299 into this branch, or would you prefer happy to adjust based on what's easiest for review. thanks! |
This is fine, but in such cases it is best to keep such PRs as drafts and mention that it goes on top of something else to avoid confusion. For example:
Thank you! |
|
I think you did it backwards:) |
d154ea2 to
b815f5b
Compare
alexpyattaev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important: some functions that used to return TPU UDP address/port should generally not start returning TPU QUIC address/port. This may cause confusion and bugs. Also in many cases we already have a quic-specific getter.
|
This looks good, but now that tvu quic was removed the conflicts must be resolved and it should be good to go. |
b562488 to
3d8e79c
Compare
alexpyattaev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple more instances to remove and we should be good to go!
alexpyattaev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The diff LGTM (except for the added typo), please squash all commits into one with a clean message, thank you!
bf41b9e to
b51a2f5
Compare
|
Probably need to rebase to get CI to clear - error has nothing to do with this PR. Diff looks good. |
b51a2f5 to
79cb808
Compare
alexpyattaev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
CI red due to incorrect const update in gossip unittests
Co-Authored-By: Alex Pyattaev <[email protected]>
79cb808 to
d449dc0
Compare
|
@gregcusack ended up editing this to wrap it up before 4.0 is cut. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9291 +/- ##
=========================================
- Coverage 82.5% 82.5% -0.1%
=========================================
Files 844 844
Lines 316364 316322 -42
=========================================
- Hits 261207 261172 -35
+ Misses 55157 55150 -7 🚀 New features to boost your workflow:
|
Problem
The validator binds an excessive number of network ports. UDP TPU path (transaction ingress via UDP) is deprecated since
version 3.0 and no longer used - the system now exclusively uses QUIC for TPU transaction ingress. Removing these unused socket bindings reduces resource consumption and simplifies network configuration.
Summary of Changes
Removes deprecated UDP socket bindings for TPU and TPU forwards paths that are no longer in use:
tpu_portandtpu_socketsUDP socket binding code fromNode::new_with_client()tpu_forwards_portandtpu_forwards_socketsUDP socket binding codetpu: Vec<UdpSocket>andtpu_forwards: Vec<UdpSocket>fields fromSocketsstructtest_tpu_forwards_quic_uses_correct_porttest to only verify QUIC paths (UDP variant no longer exists)CLUSTER_INFO_TRACE_LENGTHconstant to reflect the changes in ContactInfo trace outputsockets.tpufieldFixes #7525 (partially)