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

identify: Don't filter addr if remote is neither public nor private #2820

Merged
merged 3 commits into from
Jun 5, 2024

Conversation

MarcoPolo
Copy link
Contributor

Updates the filterAddrs logic to no-op if the address is neither public nor private.

This fixes an issue in mocknet that assigns each node an address in the IPv6 discard prefix space. That doesn't interact well with this logic in identify.

The issue mocknet hits is that it filters out all received listen addresses and then doesn't remember any address for the peer.

@MarcoPolo MarcoPolo requested a review from sukunrt June 3, 2024 21:11
Copy link
Member

@sukunrt sukunrt left a comment

Choose a reason for hiding this comment

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

Should we classify blackhole prefix addresses as Private for manet.IsPrivate?

@MarcoPolo
Copy link
Contributor Author

I don't think so. I think this logic is correct. It's neither public nor private.

Also this might be what we want anyways if this code ever runs into non-IP multiaddrs.

@sukunrt sukunrt merged commit 2769070 into master Jun 5, 2024
11 checks passed
sukunrt pushed a commit that referenced this pull request Jun 8, 2024
…2820)

Updates the filterAddrs logic to no-op if the address is neither public nor private.

This fixes an issue in mocknet that assigns each node an address in the IPv6 discard prefix space. That doesn't interact well with this logic in identify.

The issue mocknet hits is that it filters out all received listen addresses and then doesn't remember any address for the peer.
MarcoPolo added a commit that referenced this pull request Jun 12, 2024
* identify: Don't filter addr if remote is neither public nor private (#2820)

Updates the filterAddrs logic to no-op if the address is neither public nor private.

This fixes an issue in mocknet that assigns each node an address in the IPv6 discard prefix space. That doesn't interact well with this logic in identify.

The issue mocknet hits is that it filters out all received listen addresses and then doesn't remember any address for the peer.

* identify: fix bug in observed address handling (#2825)

* identify: add test for observed address handling (#2828)

This modifies TestObservedAddrManager to verify the fix in #2825

* libp2phttp: workaround for ResponseWriter's CloseNotifier (#2821)

* libp2phttp: workaround for CloseNotifier

* Add lintignore

* circuitv2: improve voucher validation (#2826)

* webrtc: fix ufrag prefix for dialing (#2832)

* webrtc: add a test for establishing many connections (#2801)

Update pion/ice to include the fix for out of order 
ConnectionState update callbacks

* release v0.35.1

---------

Co-authored-by: Marco Munizaga <git@marcopolo.io>
Co-authored-by: Ivan Shvedunov <ivan4th@users.noreply.github.com>
MarcoPolo added a commit that referenced this pull request Jun 13, 2024
…2820)

Updates the filterAddrs logic to no-op if the address is neither public nor private.

This fixes an issue in mocknet that assigns each node an address in the IPv6 discard prefix space. That doesn't interact well with this logic in identify.

The issue mocknet hits is that it filters out all received listen addresses and then doesn't remember any address for the peer.
MarcoPolo added a commit that referenced this pull request Jun 13, 2024
…2820)

Updates the filterAddrs logic to no-op if the address is neither public nor private.

This fixes an issue in mocknet that assigns each node an address in the IPv6 discard prefix space. That doesn't interact well with this logic in identify.

The issue mocknet hits is that it filters out all received listen addresses and then doesn't remember any address for the peer.
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