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-docs): do not dial invalid peers #2470

Merged
merged 6 commits into from
Jul 15, 2024
Merged

Conversation

dignifiedquire
Copy link
Contributor

Description

Breaking Changes

Notes & open questions

Change checklist

  • Self-review.
  • Documentation updates following the style guide, if relevant.
  • Tests if relevant.
  • All breaking changes documented.

@dignifiedquire dignifiedquire added this to the v0.20.0 milestone Jul 8, 2024
@dignifiedquire
Copy link
Contributor Author

okay, sync tests are broken, this seems to trigger either another bug, or this fix is incorrect

@dignifiedquire dignifiedquire modified the milestones: v0.20.0, v0.21.0 Jul 8, 2024
@Frando
Copy link
Member

Frando commented Jul 9, 2024

The fix was incorrect, pushed two commits:

  • we need to join gossip also if no peers are set.
  • we may not skip node addrs that have empty address info.

For the last one - We maybe should change iroh_net so that Endpoint::add_node_addr does not error if called with a NodeAddr with emtpy AddrInfo (only NodeId set) because it seems cumbersome to check at the user level if the addr contains addr info or not, usually you get these from a ticket or some protocol and just want to add them in case they contain useful info and don't care if not because node discovery is enabled. But that can be a followup.

@dignifiedquire dignifiedquire added this pull request to the merge queue Jul 15, 2024
Merged via the queue into main with commit 7579caa Jul 15, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants