fix(network): disconnect exclusive peers and disable downloads on leave-network - #59
fix(network): disconnect exclusive peers and disable downloads on leave-network#59lukyrys wants to merge 51 commits into
Conversation
lukyrys
changed the title
e0aff62 to
c1fb566
Compare
lukyrys
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b5f61c57d8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (const [lishID, dl] of activeDownloaders) { | ||
| const ids = dl.getNetworkIDs?.() ?? []; | ||
| if (!ids.includes(networkID)) continue; | ||
| if (ids.some(id => networks.isJoined(id))) continue; |
There was a problem hiding this comment.
When a download was created while networks A and B were enabled, leaving A will hit this continue because B remains joined, but the Downloader still keeps its original networkIDs snapshot and later broadcasts WANTs over every id in that list (downloader.ts:556-563). Because handleWant still answers based on upload state rather than verifying the requester is subscribed to the topic, the node can keep discovering/downloading from the lishnet it just left. Remove the left network from the active downloader or rebuild it with only currently joined networks instead of simply continuing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in deb2971. Downloader.networkIDs is now mutable and onNetworkLeft calls dl.removeNetwork(networkID) when another joined lishnet remains, so it stops broadcasting WANTs and probing peers on the left topic instead of just continuing.
| if (sharesNetworkWith && remotePeerID && !sharesNetworkWith(remotePeerID)) { | ||
| trace(`[PROTO] getLishs from ${remotePeer} refused: no shared joined lishnet`); | ||
| const gated: LISHGetLishsResponse = { type: 'getLishs-result', lishs: [] }; | ||
| sendLengthPrefixed(stream, codecEncode(gated)); | ||
| continue; |
There was a problem hiding this comment.
The unicast search fallback in api/search.ts:160-168 intentionally queries peers that are connected before gossipsub has propagated their SUBSCRIBE, but in exactly that window sharesNetworkWith is false because it only checks getSubscribers; this guard returns an empty list, the requester treats it as no result, and there is no later retry after the subscriber appears. This regresses searches for freshly connected peers, so the gate should either wait for topic membership before querying or distinguish "not yet visible" from "not shared".
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid — deferring as a follow-up. This branch is 80+ commits behind main, which meanwhile added a recently-seen topic-member cache with TTL (topicMembers) precisely for this raw-getSubscribers race. Fixing the gate here would duplicate/conflict with it; better to unify the leave snapshot and the serve/search gate onto that shared membership source after merge.
| continue; | ||
| } | ||
| const peerID = ma.getComponents().find(c => c.code === 421)?.value ?? null; | ||
| if (peerID && origin === 'configured') this.configuredBootstrapPeerIDs.add(peerID); |
There was a problem hiding this comment.
Once a peer ID is added here for a configured bootstrap entry, nothing removes it when updateBootstrapPeers replaces/removes that multiaddr, so isBootstrapOrRelayPeer will keep treating the old peer as infrastructure for the rest of the process. If the user removes a bootstrap peer from a joined lishnet and later leaves that lishnet, the stale entry makes leaveNetwork skip the hang-up even though the peer is no longer configured or shared with another joined network. Prune this set when bootstrap configs change or when stale peers are purged.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in deb2971. Removing a bootstrap entry via updateBootstrapPeers, and leaving a lishnet, now prune the peer from the configured-bootstrap exemption set unless it is still configured for another joined network (network.pruneConfiguredBootstrapPeer).
| if (!ids.includes(networkID)) continue; | ||
| if (ids.some(id => networks.isJoined(id))) continue; | ||
| console.log(`[Transfer] ${lishID.slice(0, 8)}: last joined lishnet left, disabling download`); | ||
| dl.disable(); |
There was a problem hiding this comment.
When a LISH has an error-recovery timer pending (for example after an upload or download I/O error), leaving its lishnet only disables the active downloader here and deletes the runtime flag; it does not call recovery.stop like the normal disableDownload path does. ErrorRecovery later uses the captured downloadWasEnabled value to call enableDownload regardless of downloadEnabledLishs, so after the directory becomes accessible it can silently restart a download the user just stopped by leaving the network, potentially on any other enabled lishnet. Stop recovery for the LISH as part of this leave-network disable path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in deb2971. The last-lishnet-left disable path now calls recovery.stop(lishID), so a pending recovery timer can no longer re-enable a download the user stopped by leaving the network.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: deb2971025
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // revive it after verification while no usable lishnet is joined. The | ||
| // DB flag stays untouched, so an app restart with the lishnet re-joined | ||
| // resumes the download. | ||
| downloadEnabledLishs.delete(lishID); |
There was a problem hiding this comment.
When the last joined lishnet for an enabled download is left, this removes the LISH from the in-memory enabled set while intentionally leaving the DB flag enabled. In the same process, re-enabling that lishnet only runs joinNetwork/bootstrap dialing; the startup auto-resume timer has already run and nothing reloads downloadEnabledLishs or calls enableDownload, so the download stays stopped despite being persisted as enabled until the user manually toggles it or restarts after rejoining. Either persist the disabled state or re-enable these DB-enabled downloads when their network becomes joined again.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7dba87b — LISHs disabled because their last joined lishnet was left are tracked as network-suspended; a new onNetworkJoined hook re-enables them when a bound lishnet is re-joined in-process. An explicit user enable/disable clears the suspension so a rejoin never overrides a deliberate user action.
| */ | ||
| private registerPeerDisconnectHandler(): void { | ||
| this.peerDisconnectDisposer?.(); | ||
| this.peerDisconnectDisposer = this.network.onPeerDisconnect(peerID => this.dropPeer(peerID)); |
There was a problem hiding this comment.
Registering a network-level peer-disconnect handler here leaves it installed for any downloader that later reaches setError: that path unregisters HAVE handlers and closes peers, then the transfer promise catch removes the downloader from activeDownloaders, but destroy() is never called, so the new peerDisconnectHandlers set retains a closure to every failed downloader. Repeated failed downloads will accumulate stale listeners and downloader instances; dispose this subscription in the terminal error path as well as in destroy().
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f151caf — the terminal setError path now disposes the peer-disconnect subscription (mirroring the HAVE handler unregister), so errored downloaders dropped from the active map no longer leak the handler closure.
| await this.node.peerStore.merge(pid, { | ||
| tags: { 'keep-alive-fleet': undefined, [KEEP_ALIVE]: undefined }, | ||
| }); |
There was a problem hiding this comment.
When leaveNetwork disconnects an exclusive content peer, this only clears keep-alive tags before hangUp while deliberately keeping the peerStore entry. I checked runRedialMaintenance in this same file: every 30 seconds it iterates all peerStore entries with reachable addresses and dials them regardless of tags, then re-stamps keep-alive-fleet, so peers that were just disconnected for a left lishnet can be reconnected almost immediately. The leave path needs to either remove/suppress these peerStore entries from maintenance until rejoin or have maintenance filter by currently shared lishnets.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 97bbabf — disconnectPeer now adds the peer to a redial-suppression set that runRedialMaintenance skips; the entry is cleared the moment the peer is observed connected again (own re-dial or mesh reconnection after rejoin) and pruned with the peerStore so the set stays bounded.
| continue; | ||
| } | ||
| const peerID = ma.getComponents().find(c => c.code === 421)?.value ?? null; | ||
| if (peerID && origin === 'configured') this.configuredBootstrapPeerIDs.add(peerID); |
There was a problem hiding this comment.
When a configured bootstrap address is relayed (.../p2p/<relay>/p2p-circuit/p2p/<bootstrapPeer>), find(c => c.code === 421) records the relay id in the new configured-bootstrap exemption instead of the actual bootstrap peer id. If that target peer is also configured for another joined lishnet, leaveNetwork later checks isBootstrapOrRelayPeer with the topic peer's final id and the exemption does not match, so the configured bootstrap peer can be treated as ordinary content and hung up; use the last /p2p component here and in the lishnet-side parser.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0cac337 — both the network-side exemption recorder and the lishnet-side parser now take the LAST /p2p component of a relayed bootstrap multiaddr (the target peer), with a relayed-address unit fixture covering it.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 709c3d3d77
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| // Keep redial maintenance from re-dialing this just-left peer on the next | ||
| // status tick. Cleared automatically once it reconnects legitimately. | ||
| this.redialSuppressed.add(peerID); |
There was a problem hiding this comment.
When a configured bootstrap peer is hung up on leave, this marker survives the later re-join path: joinNetwork calls addBootstrapPeers for one explicit dial, but neither that path nor peer:connect clears redialSuppressed. If the peer is offline during that re-join dial, or connects and drops before the next 30s maintenance tick observes it, the redial/zero-connection/promote loops keep skipping the now-configured bootstrap peer indefinitely, so the rejoined lishnet may never recover until restart or another manual edit. Clear the suppression when the peer becomes configured/joined again, before relying on the maintenance skips.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in fe15912 — addBootstrapPeers clears redialSuppressed for a re-configured bootstrap peer (scoped to origin==='configured'), so a rejoin no longer leaves the peer permanently skipped by the maintenance loops when the one explicit dial fails or drops before the next tick.
| await this.node.peerStore.merge(pid, { | ||
| tags: { 'keep-alive-fleet': undefined, [KEEP_ALIVE]: undefined }, | ||
| }); |
There was a problem hiding this comment.
This removes keep-alive tags and records the peer as suppressed, but I checked the peer:discovery handler and it still stamps keep-alive-fleet and dials every discovered address without consulting redialSuppressed. In LAN installs where mDNS keeps rediscovering a peer after its exclusive lishnet was left, that path reconnects the just-disconnected peer and re-adds the keep-alive tag, defeating the leave-network disconnect. Check suppression before tagging/dialing discovered peers, except for an explicit rejoin path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b8e90dc — the peer:discovery handler now returns early for a left-suppressed peer before stamping keep-alive-fleet or dialing, so mDNS rediscovery on a LAN no longer defeats the leave-network disconnect. Suppression lifts on a legitimate inbound reconnect or on rejoin.
| if (stillConfigured.has(pid)) continue; | ||
| this.network.pruneConfiguredBootstrapPeer(pid); | ||
| if (this.network.isBootstrapOrRelayPeer(pid)) continue; | ||
| await this.network.disconnectPeer(pid); |
There was a problem hiding this comment.
When a peer is configured as a bootstrap only for the lishnet being left but is also a subscriber on another still-joined lishnet, this early bootstrap cleanup disconnects it before the stillJoinedPeers check below can protect shared content peers. The result is that leaving network A can hang up and suppress a peer still used by network B simply because B does not list it as a configured bootstrap; compute the other-topic subscriber set first and skip these bootstrap disconnects when the peer is still present on another joined topic.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c4a1caa — the still-joined subscriber set is now computed before the bootstrap cleanup, and a bootstrap peer of the left lishnet that still subscribes another joined lishnet is kept (exemption pruned, but not disconnected).
| downloadEnabledLishs.delete(lishID); | ||
| // Remember it as suspended-by-leave so onNetworkJoined can resume it if a | ||
| // bound lishnet is re-joined in-process (without waiting for a restart). | ||
| networkSuspended.add(lishID); |
There was a problem hiding this comment.
This unconditionally records every active downloader as rejoin-suspended, but the same file still exposes transfer.download, which creates an active downloader without adding the LISH to downloadEnabledLishs. If such a one-off download is interrupted by leaving its only lishnet, the later onNetworkJoined path calls enableDownload, which adds the LISH to the runtime enabled set and may persist it, changing a transient download into an auto-resumed enabled one. Capture whether the LISH was enabled before deleting the flag and only put those downloads in networkSuspended (or resume transient downloads without marking them enabled).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4b459ba — onNetworkLeft captures wasEnabled before deleting the flag and only adds the LISH to networkSuspended when it was actually enabled, so a transient one-off download from the download handler is never turned into a persisted enabled download on rejoin.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b459baff1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // A peer we deliberately left (leave-network) must not be re-tagged or | ||
| // re-dialed by discovery (mDNS/identify/PX) — that would beat the disconnect. | ||
| // Suppression lifts on a legitimate inbound reconnect or on network rejoin. | ||
| if (this.isRedialSuppressed(peerID)) return; |
There was a problem hiding this comment.
When a user leaves and then re-joins a lishnet whose peers are ordinary content peers (for example mDNS/peer-announce peers, not configured bootstraps), those peer IDs remain in redialSuppressed; rejoin only clears configured bootstrap IDs in addBootstrapPeers. This early return means subsequent mDNS/identify discoveries for the same cached peers are ignored, and the redial/promote loops also skip suppressed IDs, so if the remote side does not initiate an inbound connection (or both sides left/rejoined) resumed downloads can never reconnect until restart/peerstore pruning. Clear the suppression for peers that become eligible again when the lishnet is rejoined instead of unconditionally skipping discovery.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4196863 — joinNetwork now lifts ALL redial suppression via Network.clearRedialSuppression(): a rejoin is an explicit request for the network's peers back, covering content peers (mDNS/peer-announce), not just configured bootstrap IDs. Suppression is a flat set, so the wholesale clear was chosen over per-network tracking; the over-clear is benign — a still-left network stays unsubscribed and its content serve-gated.
| // Serve the shared-LISH list only to peers we share a joined | ||
| // lishnet with. A bare transport connection (e.g. the peer re-dialed | ||
| // us right after we left its network) must not reveal what we share. | ||
| if (serveGateBlocks(sharesNetworkWith, remotePeerID)) { |
There was a problem hiding this comment.
runUnicastFallback in backend/src/api/search.ts deliberately sends getLishs to connected peers whose gossipsub SUBSCRIBE has not yet shown up in pubsub.getSubscribers(topic), but this gate uses that same subscriber view via sharesJoinedTopicWith and returns an empty list when it is false. In the fresh mDNS/just-connected window the fallback now marks the peer queried and gets no results, so searches can miss available LISHs until the user retries after pubsub catches up. Keep this fallback path from depending on the subscriber set, or retry once the peer is visible.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in aa607be — getLishs now uses a softer listing gate (canListSharesTo): served to any authenticated peer while we are joined to at least one lishnet, EXCEPT peers we deliberately left (still redial-suppressed), without requiring a synced gossipsub SUBSCRIBE — so the unicast search fallback reaches freshly-connected peers again. Data requests (getLish/getChunk) stay on the strict fail-closed subscriber gate, and unknown peer ids remain refused.
| } | ||
| // Keep redial maintenance from re-dialing this just-left peer on the next | ||
| // status tick. Cleared automatically once it reconnects legitimately. | ||
| this.redialSuppressed.add(peerID); |
There was a problem hiding this comment.
When a lishnet is left and the app/node restarts before it is rejoined, this in-memory suppression is lost (and stop() clears it) while disconnectPeer intentionally keeps the peerStore entry for that peer. On the next start, runRedialMaintenance iterates the persisted peerStore and dials every unsuppressed known peer, so peers exclusive to the disabled lishnet can be reconnected even though the user already left that network. Persist/recompute the leave suppression or remove those peerStore entries on leave.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a22c309 — disconnectPeer now also forgets the persisted peerStore entry (purgeStalePeer); its sole caller (leaveNetwork) only passes peers with no remaining reason to stay, so a leave now survives a restart — maintenance has nothing to redial. A rejoin re-acquires the entry via bootstrap config or discovery.
| continue; | ||
| } | ||
| console.log(`[Transfer] ${lishID.slice(0, 8)}: last joined lishnet left, disabling download`); | ||
| dl.disable(); |
There was a problem hiding this comment.
When leaving the last joined lishnet for an enabled download, this disables the downloader and removes the in-memory enabled flag, but leaves the downloader in activeDownloaders. getActiveTransfers() reports every entry in that map as type: 'downloading', so after the transfer.download:disabled event the same LISH can still appear as an active download until it is rejoined or destroyed. Remove suspended downloaders from the active list or have the active-transfer view skip disabled downloaders.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 25f826f — getActiveTransfers skips disabled downloaders, so a download suspended by leaving its last lishnet no longer appears as an active download; the downloader itself stays in the map for resume on rejoin.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 25f826f50f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Prune backoff / suppression entries for peers no longer in peerStore to prevent unbounded growth. | ||
| const storeSet = new Set(allPeers.map(p => p.id.toString())); | ||
| for (const pid of this.redialBackoff.keys()) if (!storeSet.has(pid)) this.redialBackoff.delete(pid); | ||
| for (const pid of this.redialSuppressed) if (!storeSet.has(pid)) this.redialSuppressed.delete(pid); |
There was a problem hiding this comment.
When disconnectPeer() leaves a lishnet it immediately purges that peer from the peerStore, so this maintenance cleanup removes the same peer from redialSuppressed on the next status pass. For peers that are rediscovered later via mDNS/identify while the lishnet is still disabled, the peer:discovery guard no longer sees suppression and can re-tag/re-dial them, undoing the leave within one tick. Keep leave suppression until an observed reconnect or an explicit rejoin instead of pruning it solely from the peerStore snapshot.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 871173d — suppression is now tracked per lishnet (Map keyed by networkID) and is no longer pruned against the peerStore snapshot, so purging the left peer from the store cannot drop its suppression; mDNS/identify rediscovery keeps being skipped. Suppression lifts on an observed reconnect (any path) or on rejoin of that lishnet, and is cleared in stop().
| const wasEnabled = downloadEnabledLishs.has(lishID); | ||
| downloadEnabledLishs.delete(lishID); | ||
| // Remember it as suspended-by-leave so onNetworkJoined can resume it if a | ||
| // bound lishnet is re-joined in-process (without waiting for a restart). | ||
| if (wasEnabled) networkSuspended.add(lishID); |
There was a problem hiding this comment.
When the last lishnet is left for a download started through transfer.download rather than persisted via enableDownload, wasEnabled is false, so this path disables the downloader but neither marks it for resume nor removes/destroys it. That leaves its download() promise unresolved and its network handlers registered for the process; starting the same LISH again can replace the map entry without disposing the old downloader. Destroy and delete non-resumable downloaders here instead of retaining them.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d3b88db — a transient download (started via the download handler, never persisted) is now destroyed and removed from activeDownloaders when its last lishnet is left: its download() promise settles and the network handlers are disposed. Persisted downloads keep the disable-and-retain suspend behaviour.
| clearRedialSuppression(): void { | ||
| this.redialSuppressed.clear(); |
There was a problem hiding this comment.
If a user leaves multiple lishnets and then rejoins only one of them, this unconditional clear also unsuppresses peers from the lishnets that are still left. Those peers can then be rediscovered/redialed by maintenance, and canListSharesTo() no longer has the suppression marker it relies on to refuse listing requests from left peers, so leaving the other lishnet is effectively undone without rejoining it. Track suppression per lishnet or clear only peers associated with the network being joined.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 871173d — clearing is now scoped: joinNetwork lifts only the rejoined lishnet's suppression set. Peers of other still-left lishnets stay suppressed, and canListSharesTo keeps refusing them, so leaving another lishnet is no longer undone by an unrelated rejoin. An observed reconnect still lifts a peer from all sets.
| const dl = activeDownloaders.get(lishID); | ||
| if (dl && !dl.getNetworkIDs?.().includes(networkID)) continue; | ||
| enableDownload({ lishID }) |
There was a problem hiding this comment.
When a suspended entry has no retained downloader (for example after startup auto-resume ran with no joined lishnets), this check only filters the retained-downloader case, so enableDownload() rebinds the download to whatever lishnet is currently joined. A user can leave lishnet A, restart with no joined lishnets, then join lishnet B and have the A download automatically broadcast WANTs on B. Persist the original network binding or require a matching rejoin before resuming these no-downloader suspensions.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 86dc71e for the in-process case — networkSuspended now maps each LISH to its ORIGINAL lishnets (stable snapshot, not the removeNetwork-shrunk set) and onNetworkJoined resumes only when a bound lishnet rejoins. The restart variant is not fully fixable at this layer: the DB stores only a download_enabled boolean with no per-download network binding, so a post-restart suspension has no binding source (recorded as resume-on-any, matching what a fresh enableDownload would bind anyway). Making that strict needs a schema change — noted as a follow-up.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 86dc71e990
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (const peer of allPeers) { | ||
| const pid = peer.id.toString(); | ||
| if (pid === myID) continue; | ||
| if (this.isRedialSuppressed(pid)) continue; // deliberately left — don't promote it back to bootstrap |
There was a problem hiding this comment.
Fresh evidence in the current code: this suppression check only filters maStrings; the gossipsub.direct loop immediately below still iterates allPeers and adds every peer ID. If a left peer remains or reappears in the peerStore while suppressed (for example, the best-effort purge fails or discovery repopulates it), the fifth status tick can put it into the direct set, whose nearby comment says it reconnects on a directConnect cadence, so the leave-network disconnect can be undone. Apply the same isRedialSuppressed(pid) guard before adding to gossipsub.direct.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 304a417 — the gossipsub.direct promotion loop now applies the same isRedialSuppressed guard as the bootstrap promotion, so a left peer that lingers/reappears in the peerStore is not added to the direct set (whose fast reconnect cadence would otherwise undo the leave).
| } | ||
| // A peer that reconnects (inbound or otherwise) is legitimately back — | ||
| // lift any leave-network redial suppression so maintenance resumes for it. | ||
| this.clearRedialSuppressionForPeer(peerID); |
There was a problem hiding this comment.
When a peer we deliberately disconnected can still open an inbound connection (for example, its own keep-alive or mDNS dials us after we leave its only lishnet), this unconditional clear removes the only marker canListSharesTo() uses to reject left peers. Because canListSharesTo() then serves getLishs to any authenticated peer while we have any lishnet joined, that peer can browse our shared list without rejoining a shared topic; only clear suppression on an explicit rejoin or after verifying sharesJoinedTopicWith(peerID).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 304a417 — a mere reconnect (inbound keep-alive/mDNS) no longer clears leave suppression: both the peer:connect handler and the maintenance connected-branch clear it only when sharesJoinedTopicWith(peer) is true, so canListSharesTo keeps refusing a left peer until it genuinely rejoins a shared topic.
| // so ReconnectQueue does not immediately re-dial it). | ||
| for (const pid of leftPeers) { | ||
| if (stillJoinedPeers.has(pid)) continue; | ||
| if (this.network.isBootstrapOrRelayPeer(pid)) continue; |
There was a problem hiding this comment.
When an exclusive left-lishnet peer is an active relay/bootstrap that we intentionally keep connected, this continue skips disconnectPeer, so no redial suppression is recorded. The new listing gate refuses left peers only via isRedialSuppressed, so a kept relay from a network the user just left can still call getLishs and receive the shared catalog while we remain in any other lishnet. Record suppression without hanging up, or make canListSharesTo require current topic sharing for these kept infrastructure peers.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 302ed5b — canListSharesTo now requires a kept infrastructure peer (active relay/bootstrap, isBootstrapOrRelayPeer) to currently share a joined topic; a relay of a network we just left can no longer browse our catalog. Ordinary content peers still get the soft gate for the SUBSCRIBE-lag search-fallback window.
| // Persisted download — retain the disabled downloader and remember it as | ||
| // suspended-by-leave so onNetworkJoined can resume it after rejoin. | ||
| console.log(`[Transfer] ${lishID.slice(0, 8)}: last joined lishnet left, disabling download`); | ||
| dl.disable(); |
There was a problem hiding this comment.
Fresh evidence in the current code: this path retains a disabled downloader in activeDownloaders, and the existing enableDownload active-downloader branch calls dl.enable() without checking that any of its networkIDs is joined. If the user clicks enable after leaving the last lishnet but before rejoining it, the downloader resumes and broadcasts/probes on the left topic instead of staying suspended; keep it suspended or remove/rebuild it unless a bound network is joined.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ddda6e2 — enableDownload now refuses to resume a leave-suspended download (in networkSuspended) unless one of its ORIGINAL bound lishnets is currently joined; it drops the runtime flag while keeping the DB intent, so a manual enable after leaving the last lishnet keeps it suspended instead of broadcasting on the left topic.
|
|
||
| // Snapshot the topic subscribers BEFORE unsubscribing — unsubscribeTopic | ||
| // tears the topic out of pubsub, after which getTopicPeers(id) returns []. | ||
| const leftPeers = this.network.getTopicPeers(id); |
There was a problem hiding this comment.
This snapshot only includes peers currently visible as topic subscribers. If an ordinary content peer for the lishnet is disconnected at leave time but still has a peerStore entry from earlier discovery/keep-alive, it is not in leftPeers and is not covered by the configured-bootstrap loop below, so runRedialMaintenance later treats it as unsuppressed and can dial it back after the lishnet was left. Track/reuse recently seen topic members or purge/suppress peerStore entries associated with the left lishnet, not only live subscribers.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9651703 — leaveNetwork now unions the live subscriber snapshot with recently-seen topic members (peer-announce's TTL cache, exposed via getRecentTopicMembers), so a content peer momentarily offline at leave time but still in the peerStore is suppressed too and maintenance cannot redial it back.
…-disconnect # Conflicts: # backend/src/api/transfer.ts
1 participant
Summary
Test plan