Bootstrap panics if no config has no xBackupBootstrapPeers functions

gammazero commented 1 year ago

Checklist

[X] This is a bug report, not a question. Ask questions on discuss.ipfs.io.
[X] I have searched on the issue tracker for my bug.
[X] I am running the latest kubo version or have an issue updating.

Installation method

built from source

Version

Not running kubo, importing boostrap package from version:
v0.21.0

Config

N/A

Description

I am using the bootstrap package to establish libp2p connections to a set of hosts that I need to exchange gossip pubsub messages with. I am creating the bootstrap config using BootstrapConfigWithPeers. When starting the bootstrapper, with:

bootCfg := BootstrapConfigWithPeers(peerList)
bootCfg.MinPeerThreshold = 2
bootstrapper, err := Bootstrap(peerID, p2pHost, nil, bootCfg)

A panic occurs when the first bootstrap round runs because these functions are not assigned in the configuration:

LoadBackupBootstrapPeers
SaveBackupBootstrapPeers

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x0 pc=0x1051bbd20]

goroutine 2259 [running]:
github.com/ipfs/kubo/core/bootstrap.bootstrapRound({0x1065c7bf0?, 0x140008820f0?}, {0x1065d9000, 0x140000f2600}, {0x8, 0x6fc23ac00, 0x2540be400, 0x14000760040, 0x34630b8a000, 0x14, ...})
        /home/ajg/go/pkg/mod/github.com/ipfs/kubo@v0.21.0/core/bootstrap/bootstrap.go:248 +0x190
github.com/ipfs/kubo/core/bootstrap.Bootstrap.func1({0x1065d54f0?, 0x1400074b0e0?})
        /home/ajg/go/pkg/mod/github.com/ipfs/kubo@v0.21.0/core/bootstrap/bootstrap.go:102 +0xcc
github.com/jbenet/goprocess.(*process).Go.func1()
        /home/ajg/go/pkg/mod/github.com/jbenet/goprocess@v0.1.4/impl-mutex.go:134 +0x3c
created by github.com/jbenet/goprocess.(*process).Go
        /home/ajg/go/pkg/mod/github.com/jbenet/goprocess@v0.1.4/impl-mutex.go:133 +0x218

If these functions are expected to be defined, then an error should be returned from Boosstrap. If these functions can be nil, then they should not be called.

I have created a PR to suggest a possible fix: #10029

Related discussion and issue

Issue #9876 Kubo, invalid memory address or nil pointer dereference from IpfsNode.Bootstrap

Jorropo commented 1 year ago

Any reason you are using this package ? We don't want people importing parts of kubo anymore, would be nice to steer boxo into what peoples need.

Also the DHT already have bootstrap code I would use the bootstrap option on the dht directly, there are multiple implementations of bootstrap inside kubo for historic reasons I want to remove this.

gammazero commented 1 year ago

Any reason you are using this package ?

This is being used to bootstrap IPNI indexers with known gateway nodes that relay pubsub messages from Filecoin storage providers announcing that new index data is available. It is necessary to maintain connections with at least some gateways in order to maintain the gossip mesh over which the messages are communicated. Seemed better to use an existing tested package that was part of go-ipfs (now kubo) that to write one that is nearly identical. The peering package is also used similarly. Both imported here: https://github.com/ipni/storetheindex/blob/main/command/daemon.go#L19-L20

Also the DHT already have bootstrap code I would use the bootstrap option on the dht directly

I am not currently using DHT in my project, and I am bootstrapping against a known set of gateway nodes that can authenticate Filecoin storage providers and relay gossip pubsub. It seems like using DHT and its bootstrapping is not what I want.

there are multiple implementations of bootstrap inside kubo for historic reasons I want to remove this.

Can I expect one to be available in boxo soon? It seems that the basic bootstrapping service should be in boxo, with the specific implementation of the Load/SaveBackupBootstrapPeers functions being provided by kubo.

Should I fork the bootstrap and the peering services and not use what is provided by kubo? Should I create a PR that moves the non-kubo-specific portions of those services into boxo?

Jorropo commented 1 year ago

I did not thought about the fact you would bootstrap something else than a DHT. There are no plans to move this into boxo yet, I've created https://github.com/ipfs/boxo/issues/419.

I'll look at your pull request thx.

ipfs / kubo