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

Bootstrap panics if no config has no xBackupBootstrapPeers functions #10030

Closed
3 tasks done
gammazero opened this issue Jul 26, 2023 · 4 comments
Closed
3 tasks done

Bootstrap panics if no config has no xBackupBootstrapPeers functions #10030

gammazero opened this issue Jul 26, 2023 · 4 comments
Labels
kind/bug A bug in existing code (including security flaws) need/triage Needs initial labeling and prioritization

Comments

@gammazero
Copy link
Contributor

gammazero commented Jul 26, 2023

Checklist

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/[email protected]/core/bootstrap/bootstrap.go:248 +0x190
github.com/ipfs/kubo/core/bootstrap.Bootstrap.func1({0x1065d54f0?, 0x1400074b0e0?})
        /home/ajg/go/pkg/mod/github.com/ipfs/[email protected]/core/bootstrap/bootstrap.go:102 +0xcc
github.com/jbenet/goprocess.(*process).Go.func1()
        /home/ajg/go/pkg/mod/github.com/jbenet/[email protected]/impl-mutex.go:134 +0x3c
created by github.com/jbenet/goprocess.(*process).Go
        /home/ajg/go/pkg/mod/github.com/jbenet/[email protected]/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

@gammazero gammazero added kind/bug A bug in existing code (including security flaws) need/triage Needs initial labeling and prioritization labels Jul 26, 2023
@Jorropo
Copy link
Contributor

Jorropo commented Jul 26, 2023

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
Copy link
Contributor Author

gammazero commented Jul 26, 2023

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
Copy link
Contributor

Jorropo commented Jul 27, 2023

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 ipfs/boxo#419.

I'll look at your pull request thx.

gammazero added a commit to ipni/storetheindex that referenced this issue Aug 2, 2023
Need this because of ipfs/kubo#10030.

Can be removed when ipfs/kubo#10029 is available.
gammazero added a commit to ipni/storetheindex that referenced this issue Aug 2, 2023
@gammazero
Copy link
Contributor Author

Fixed in #10029

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A bug in existing code (including security flaws) need/triage Needs initial labeling and prioritization
Projects
None yet
Development

No branches or pull requests

2 participants