fix: empty Bootstrap list no longer dials stale backup peers - #11453
fix: empty Bootstrap list no longer dials stale backup peers#11453karawitan wants to merge 1 commit into
Conversation
02dcc04 to
4a03e7e
Compare
There was a problem hiding this comment.
Thanks for digging into this. The root cause you found is right. The fix needs more work.
Feels like a wrong place: Routing.Type and Bootstrap are two separate settings. Some people run Routing.Type=none together with their own Bootstrap peers (private or static setups), and with this change those nodes would never connect. The check also misses ipfs daemon --routing=none, since that flag does not change the config file.
The backup peer list exists so a node can recover when its configured bootstrap peers are down (#8856). If Bootstrap is empty, there is nothing to recover from, so the backup list should not be used at all. The simplest fix is in boxo, in bootstrap.bootstrapRound: when cfg.BootstrapPeers() returns nothing, stop there instead of trying the backup list. That covers your Bootstrap: null case (it becomes an empty list), works with the --routing flag, and keeps none + explicit bootstrap peers working. No new config option is needed.
If you open that change against ipfs/boxo, this PR can become: the boxo bump, the changelog line, and one sentence under Bootstrap in docs/config.md saying that an empty list turns off all bootstrap dialing, including saved backup peers.
Two smaller things: the changelog entry landed in v0.43.md (already released) instead of v0.44.md, and both files now have a broken emoji (�) in the headings, probably from the editor's encoding. The new config.md wording "will not attempt to dial any external peers" also promises too much, since Peering.Peers and mDNS still dial.
Hope this helps.
ps. since this should be v0.44, be mindful it may take some time before new maintainer is identified and picks this up
Rework of ipfs#11453 per maintainer review (lidel): the fix belongs in boxo (bootstrap.bootstrapRound skips the backup list when no bootstrap peers are configured), not behind a Routing.Type=none guard in kubo. This PR is now the companion to ipfs/boxo#1213: - Pin the boxo fix via a temporary replace directive pointing at the boxo PR branch. Once ipfs/boxo#1213 merges, repoint at boxo main and convert to a pseudo-version pin. - Add a kubo-level regression test (TestBootstrapWithEmptyPeerListAndStaleBackupPeers) verifying IpfsNode.Bootstrap runs with an empty Bootstrap config and a populated TempBootstrapPeersKey without error. - Document under Bootstrap in docs/config.md that an empty list disables all bootstrap dialing, including saved backup peers. - Add a v0.44 changelog highlight. Drop the v0.43 entry and the broken emoji from the original PR. Closes ipfs#11452 Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
4a03e7e to
9446212
Compare
|
Thanks @lidel — reworked this PR per your review. Fix moved to boxo: ipfs/boxo#1213 adds an early return in This PR is now the companion kubo PR for ipfs/boxo#1213:
Marked as draft while it pins the unmerged boxo branch. |
Summary
Rework of this PR per @lidel's review in #11453 (review). The fix belongs in boxo (
bootstrap.bootstrapRoundskips the backup list when no bootstrap peers are configured), not behind aRouting.Type=noneguard in kubo. This PR is now the companion kubo PR for ipfs/boxo#1213.What changed here
go.mod/go.sum: pin the boxo fix via a temporaryreplacedirective pointing at the boxo PR branch (karawitan/boxo@76cf61a). Once ipfs/boxo#1213 merges, this repoints at boxomainand converts to a pseudo-version pin.core/core_test.go: addTestBootstrapWithEmptyPeerListAndStaleBackupPeers, a kubo-level regression test for #11452. It verifiesIpfsNode.Bootstrapruns without error with an emptyBootstrapconfig and a populatedTempBootstrapPeersKey(stale backup peers from a previous run), and starts a bootstrapper whose rounds are no-ops for the backup list. The dialing behavior itself is fixed and unit-tested in boxo.docs/config.md: document underBootstrapthat an empty list disables all bootstrap dialing, including saved backup peers. Note thatPeering.Peersand mDNS are independent and may still dial. (Reverts the overpromisingRouting.Type=nonewording from the original PR —Peering.Peersand mDNS still dial regardless.)docs/changelogs/v0.44.md: add a v0.44 highlight. Drops the v0.43 entry (already released) and the broken emoji from the original PR.What was removed from the original PR
Routing.Type=noneguard incore/core.gois gone. Per lidel:Routing.TypeandBootstrapare separate settings; some users runRouting.Type=nonewith their ownBootstrappeers (private/static swarms), and the guard would silently break those. The guard also missedipfs daemon --routing=none, which overrides routing at runtime without changing the config file.TestBootstrapSkippedWhenRoutingNonetest is replaced byTestBootstrapWithEmptyPeerListAndStaleBackupPeers.Behavior change
Previously, a node with default routing and an empty
Bootstraplist would still dial backup peers persisted from previous runs. After this change (boxo fix), it does not. This aligns behavior with operator intent: an emptyBootstraplist now means "no bootstrap dialing at all, including saved backup peers." Nodes that configure explicitBootstrappeers are unaffected; the backup-list fallback still runs when those peers fail to connect.Test plan
go test ./core/ -run TestBootstrapWithEmptyPeerListAndStaleBackupPeers -v -count=1passesgo test ./core/ -count=1passes (all existing core tests green)go vet ./core/...passesgofmt -l core/cleanmake mod_tidyrun (all three go.mod files tidied)References
Generated with Devin