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

Create an ObservedAddrManager and add an AddressMapper in AutonatService and AutoRelayService #871

Merged
merged 49 commits into from
Mar 24, 2023

Conversation

diegomrsantos
Copy link
Collaborator

@diegomrsantos diegomrsantos commented Mar 2, 2023

  • Creates an ObservedAddrManager which keeps track of the most observed addresses reported by remote peers.
  • Creates an AddressMapper in AutonatService which tries to identify the peer public IP/Port when there's a manual port forward configured.
  • Creates an AddressMapper in AutonRelayService which adds the relayed address.

@diegomrsantos diegomrsantos marked this pull request as ready for review March 6, 2023 12:28
Copy link
Contributor

@Menduist Menduist left a comment

Choose a reason for hiding this comment

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

We will also have to check what impact this has on the existing addressMapper in waku

.pinned Outdated Show resolved Hide resolved
libp2p/observedaddrmanager.nim Outdated Show resolved Hide resolved
libp2p/observedaddrmanager.nim Outdated Show resolved Hide resolved
libp2p/observedaddrmanager.nim Show resolved Hide resolved
libp2p/observedaddrmanager.nim Outdated Show resolved Hide resolved
libp2p/peerstore.nim Outdated Show resolved Hide resolved
libp2p/protocols/connectivity/autonat/service.nim Outdated Show resolved Hide resolved
libp2p/protocols/connectivity/autonat/service.nim Outdated Show resolved Hide resolved
Base automatically changed from upgraderefacto to unstable March 8, 2023 11:30
let observedAddrManager = ObservedAddrManager.new(minCount = 3)

# Calculate the most oberserved IP4 correctly
let mostObservedIP4AndPort = MultiAddress.init("/ip4/1.2.3.0/tcp/1").get()
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you test with invalid multiaddresses, seems like it will crash

## Returns the most observed IP address or none if the number of observations are less than minCount.
return self.identify.getMostObservedIP(ipVersion)

proc replaceMAIpByMostObserved*(
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is used by guessDialableAddrs which is used in hpservice and the dcutr server.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, but this is unrelated to the peer store

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Where would you add it? I think it is related to the observed addresses.

Copy link
Contributor

Choose a reason for hiding this comment

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

ObservedAddressManager?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@diegomrsantos diegomrsantos changed the title Create an ObservedAddrManager and add an AddressMapper in AutonatService Create an ObservedAddrManager and add an AddressMapper in AutonatService and AutoRelayService Mar 23, 2023
@diegomrsantos diegomrsantos enabled auto-merge (squash) March 24, 2023 14:45
@diegomrsantos diegomrsantos merged commit af5299f into unstable Mar 24, 2023
@diegomrsantos diegomrsantos deleted the observed-addr-manager branch March 24, 2023 15:42
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.

2 participants