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

autonat: don't use autonat for address discovery #2148

Merged
merged 2 commits into from
Feb 27, 2023

Conversation

sukunrt
Copy link
Member

@sukunrt sukunrt commented Feb 27, 2023

fixes: #2102

@sukunrt sukunrt marked this pull request as draft February 27, 2023 11:54
@sukunrt sukunrt marked this pull request as ready for review February 27, 2023 12:30
p2p/host/autonat/autonat_test.go Outdated Show resolved Hide resolved
if as.confidence > 1 {
as.confidence--
}
// On local address update, reduce confidence from maximum so that we schedule
Copy link
Contributor

Choose a reason for hiding this comment

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

What is a local address update?

Copy link
Member Author

@sukunrt sukunrt Feb 27, 2023

Choose a reason for hiding this comment

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

this is any change in the host address list(host.AllAddrs()). I used the phrase from the event EvtLocalAddressesUpdated.

I didn't change this logic much since it was not related to this PR, but the correct thing to do here would be to only reduce confidence if there are any addresses removed from the host address list and our status is public or if there are addresses added and our status is private.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, this is fine. Sorry, I messed this up, didn't realize this was literally in the name of the event ;)

@marten-seemann marten-seemann changed the title autonat: dont use autonat for address discovery autonat: don't use autonat for address discovery Feb 27, 2023
@marten-seemann marten-seemann merged commit c738a48 into libp2p:master Feb 27, 2023
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.

autonat: don't use AutoNAT to discover addresses
2 participants