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: peer_control getinfo to show correct port on discovered IPs #5585

Merged

Conversation

m-schmoock
Copy link
Collaborator

@m-schmoock m-schmoock commented Sep 12, 2022

Before this patch, getinfo shows incorrect ports on getinfo auto discovered IP addresses.
It also just showed (not announced) unconfirmed addresses.

Addresses #5553

Changelog-Fixed: peer_control: getinfo shows the correct port on discovered IPs

@m-schmoock m-schmoock changed the title fix peer_control getinfo tp show correct port on discovered IPs fix peer_control getinfo to show correct port on discovered IPs Sep 12, 2022
Changelog-Fixed: peer_control: getinfo shows the correct port on discovered IPs
Copy link
Collaborator

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

ACK bcb6d32

@m-schmoock m-schmoock changed the title fix peer_control getinfo to show correct port on discovered IPs fix: peer_control getinfo to show correct port on discovered IPs Sep 13, 2022
This is cleaner because, the `remote_addr` and `discovered_ip` are
related but two different things.

Within connectd and lightningd we use the peers `remote_addr` feature
to validate (and guess a port) to be used for IP discovery.

Also when a peer reports us a `remote_addr`, this is given to the plugin API
via the `peer_connected` hook. The network port here is not modified for
godd reason! This can be used i.e. to detect if we are behind a NAT.

But once lightningd figures enough peers report the same `remote_addr`,
it sets the port to the selected network and tells gossipd to use that for
`node_announcement` updates.

Hence, within gossipd, there is no (should not be) `remote_addr`.

Changelog-None
Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

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

Ack 8fb6ed8

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

3 participants