fix(network): stop zero-connection recovery re-dialing orphaned peers - #100
Draft
lukyrys wants to merge 107 commits into
Draft
fix(network): stop zero-connection recovery re-dialing orphaned peers#100lukyrys wants to merge 107 commits into
lukyrys wants to merge 107 commits into
Conversation
…ale-discovered-peers # Conflicts: # backend/src/protocol/network.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stops a node with no connections from re-dialing peers that are long gone.
Zero-connection recovery walks a list of bootstrap addresses every 30 seconds and dials each one with a 10 second timeout. That list was append-only: an address learned from gossip went in before any dial proved the peer existed, and nothing ever took it out again. Once libp2p's peerStore aged the peer out after two hours, re-dial maintenance — which walks the peerStore — could not see it either, so the address stayed on the recovery list for good and every tick spent its budget on dead hosts.
Follow-up needed once #72 is merged, written out here so it does not have to be worked out again.
#72 introduces an
unreachableQuarantinemap (peer ID to the epoch ms it was evicted at, held forUNREACHABLE_QUARANTINE_MS). That map does not exist on main, so this PR cannot consult it — recovery currently paces a dead discovered peer through the backoff only. After #72 lands, recovery should skip a quarantined peer outright:isRecoveryDialEligibleinbackend/src/protocol/network.tstwo more parameters, the quarantine map and its window, and return false for a discovered entry whose peer ID has a quarantine stamp newer than the window; configured entries stay exempt exactly as they are for the backoffthis.unreachableQuarantineat the single call site inrunZeroConnectionRecoverybootstrapMultiaddrsarray tobootstrapEntries(now objects, not bare multiaddrs), so the two places fix(network): expire stale and unreachable peers from the network participant list #72 rebuilds that array need adapting:purgeStalePeerbecomesthis.bootstrapEntries = this.bootstrapEntries.filter(e => e.peerID !== peerID), and the identity-mismatch filter inaddBootstrapPeersmatches one.peerIDande.ma.toString()instead of the raw multiaddrbackend/tests/unit/protocol/bootstrap-recovery.test.ts: a quarantined discovered entry is skipped by recovery, and a quarantined configured entry is still dialledExpect a merge conflict in
runZeroConnectionRecoveryandaddBootstrapPeerswhichever of the two merges second — both touch the same lines, and the resolution is the rename above.