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(sync): fix PeerData encoding, neighbor events, better & predictable tests #1513

Merged
merged 4 commits into from
Sep 22, 2023

Conversation

Frando
Copy link
Member

@Frando Frando commented Sep 22, 2023

Description

  • fix(gossip): properly encode peer data. refactor(*): rework NodeAddr #1506 introduced a bug: The PeerData was encoded from the PeerAddr (including the peer id) but decoded to AddrInfo (without the peer id). So it failed, and dialing peers failed. It only did not matter much because most tests use tickets separately, so do not rely on the PeerData gossip.
  • feat: expose neighbor events through document subscriptions. for now used to write better and less flakey tests. also useful for stats like usecases, and potentially others.
  • tests: improve sync tests and make sync_full_basic not flakey anymore (hopefully). the main change is that for some events, we don't care about the exact order in tests anymore, because the exact order is too unpredictable timing-wise for things that happen concurrently. instead they are matched in chunks.

Change checklist

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

Replaces #1512

@ramfox ramfox added this to the v0.6.0 milestone Sep 22, 2023
@ramfox ramfox added c-iroh-sync fix Fixes a bug labels Sep 22, 2023
@Frando Frando added this pull request to the merge queue Sep 22, 2023
Merged via the queue into main with commit 779e470 Sep 22, 2023
15 checks passed
@dignifiedquire dignifiedquire deleted the sync-full-basic-fix2 branch November 1, 2023 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants