fix(discovery): makes pong send only IP without hostname #2279
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jurajpiar
force-pushed
the
jurajpiar/no_dns_for_find_node
branch
3 times, most recently
from
c7b58f0 to
bec94de
Compare
asoto-iov
previously approved these changes
Apr 1, 2024
jurajpiar
force-pushed
the
jurajpiar/no_dns_for_find_node
branch
3 times, most recently
from
ffac707 to
875b454
Compare
asoto-iov
reviewed
Apr 1, 2024
rskj-core/src/test/java/co/rsk/net/discovery/PeerExplorerTest.java
Outdated
Show resolved
Hide resolved
rskj-core/src/test/java/co/rsk/net/discovery/PeerExplorerTest.java
Outdated
Show resolved
Hide resolved
fmacleal
previously approved these changes
Apr 1, 2024
There was a problem hiding this comment.
Good job. I don't have any specific remark and I think that what @asoto-iov commented already cover an important part.
I am just wondering if makes sense some more small tests on the places I have commented below. Let me know your opinion, it doesn't prevent the approval.
jurajpiar
force-pushed
the
jurajpiar/no_dns_for_find_node
branch
from
875b454 to
1354af3
Compare
|
pipeline:run |
fmacleal
previously approved these changes
Apr 3, 2024
asoto-iov
previously approved these changes
Apr 9, 2024
jurajpiar
force-pushed
the
jurajpiar/no_dns_for_find_node
branch
from
4fb6f63 to
a985e90
Compare
squashme: makes safer test
jurajpiar
force-pushed
the
jurajpiar/no_dns_for_find_node
branch
from
a985e90 to
2bd1e0d
Compare
|
|
pipeline:run |
1 similar comment
|
pipeline:run |
Vovchyk
approved these changes
Apr 18, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As RSK user I want IP addresses (instead of DNS names) to be sent to other peers in response to FIND_NODE peer discovery message
Description
Instead of DNS hostnames a node always sends IP addresses in response to the FIND_NODE peer discovery message
Motivation and Context
As of now, in the peer discovery protocol, when a peer asks for neighbours by sending FIND_NODE message in response it gets a sub list of other discovered peers in a network. Sometimes, instead of IP addresses of remote peers that list might include DNS names resolved via reverse DNS lookup in a context of the peer that returns that response. In some cases this might lead to issues when the reverse DNS resolution happens in different network contexts, for instance, when inside AWS VPC an IP can be resolved to ip-10-10-133-54.us-west-2.compute.internal, but outside that host name would make no sense.
How Has This Been Tested?
Unit test included.
Types of changes
Checklist: