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(iroh-net): enforce storing a single derp region per peer #1607

Merged

Conversation

divagant-martian
Copy link
Contributor

@divagant-martian divagant-martian commented Oct 11, 2023

Description

The Endpoint contained SendAddr instead of ip, port pairs. This is problematic for a couple of reasons:

  • it makes it harder to reason about direct addresses and relay addresses
  • makes it harder to properly update the derp-related information
  • in principle, we shouldn't be storing multiple derp regions per peer

So this PR enforces storing a single derp region per peer. Some relevant note with these changes:

Notes & open questions

  • this proves we never ping a peer via their relay address. I left some TODOs that I intend to address in a later PR when it's clear how we should be doing this
  • I also noticed updating a peer's region is different between "updating from node" to receiving a ping. Maybe this is the right thing to do but it's also peculiar. If a peer pinged us via relay we update their derp region, if we add a ticket or receive info from a gossip peer exchange we only update the region if we don't have any. To me this makes sense, but again, it draws my attention. I left a comment about it in the code
    Basically TLDR, next step is adding ping via relays with some appropriate tests. This should at a minimum solve the second part of clean up output for node connections #1576

Change checklist

  • Self-review.
  • Documentation updates if relevant.
  • Tests if relevant.

@dignifiedquire
Copy link
Contributor

this looks pretty good, what is missing for this to be ready?

@divagant-martian
Copy link
Contributor Author

@dignifiedquire was built on top of #1606 so was waiting for that. Otherwise I think it's ready. I'll update the description with the relevant info

@divagant-martian divagant-martian marked this pull request as ready for review October 11, 2023 19:59
@divagant-martian divagant-martian changed the title [wip] enforce storing a single derp region per peer fix(iroh-net): enforce storing a single derp region per peer Oct 11, 2023
@divagant-martian divagant-martian added this pull request to the merge queue Oct 11, 2023
Merged via the queue into n0-computer:main with commit bfcce3d Oct 12, 2023
15 checks passed
@b5 b5 added this to the v0.7.1 milestone Oct 12, 2023
@divagant-martian divagant-martian deleted the store-relay-latency-appart branch December 10, 2023 02:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants