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

Correct the bootstrap.libp2p.io default bootstrap peer entry #270

Closed
wants to merge 1 commit into from

Conversation

anacrolix
Copy link
Contributor

I'm not sure why the dnsaddr set was getting around in the state it was. I believe there's only a single P2P ID that works at this address, and this is it.

@anacrolix anacrolix requested review from a user, Stebalien and raulk February 15, 2019 04:30
@ghost ghost assigned anacrolix Feb 15, 2019
@ghost ghost added the status/in-progress In progress label Feb 15, 2019
@Stebalien
Copy link
Member

Odd. Yeah, drill TXT _dnsaddr.bootstrap.libp2p.io only returns QmSoLer265... @lgierth any context on this?

@ghost
Copy link

ghost commented Feb 15, 2019

There's three classes of bootstrappers in that list:

  • The old Digitalocean bootstrappers, with PeerIDs starting in QmSoL
    • These used to be called "solarnet" early on, and the character following "L" indicates which host it is, e.g. V=venus.i.ipfs.io, e=earth, S=saturn, and so on.
  • @whyrusleeping's personal bootstrapper, QmaCp..LuvuJ, also with Digitalocean
  • The newer bootstrap.libp2p.io addrs

We didn't move the new bootstrappers beyond the stage of having their addresses in the config, so far. I have the respective private keys on my backup disks.

The motivation to move away from the Digitalocean bootstrappers is mainly that early on we made the mistake of not using Floating IPs in these addresses - that means if there's a happy accident with one of those VPSes, that bootstrapper is permanently lost because we very likely don't get that same IP assigned again.

I'd also like to point out that why's personal bootstrapper is neither automated nor monitored -- he wasn't a fan of changing that in the past, but that might have changed by now. We could also just do it.

I'd vote to keep the bootstrap list here consistent with go-ipfs and js-ipfs until there's another plan.

@ghost
Copy link

ghost commented Feb 15, 2019

Odd. Yeah, drill TXT _dnsaddr.bootstrap.libp2p.io only returns QmSoLer265... @lgierth any context on this?

I don't remember why there's that one particular dnsaddr in bootstrap.libp2p.io.

@ghost
Copy link

ghost commented Feb 15, 2019

PR back in the day: ipfs/kubo#4127


We didn't move the new bootstrappers beyond the stage of having their addresses in the config, so far. I have the respective private keys on my backup disks.

Where should I stick those private keys for the time being?

@raulk
Copy link
Member

raulk commented Feb 15, 2019

@lgierth this bootstrap peer was never even dialled because the mismatch in the peer ID between the TXT record and the hardcoded multiaddrs made this check fail 🙃 As a result, the multiaddr dns resolution returned an empty address set.

Before we merge this change:

  1. Yeah, let's sync up on how to handle that key.
  2. Let's do a connectivity and capacity check on that host. nc 178.62.158.247 4001 is green for me but I can't check the IPv6 addr. Someone who can do that?

@ghost
Copy link

ghost commented Feb 15, 2019

Before we merge this change:

What I meant was: let's keep the bootstrap configs in sync, as in, let's keep this as it is, or make a new plan for bootstrap addressing. The dnsaddrs currently in there are correct as per ipfs/kubo#4127, their hosts are just not deployed yet.

It's useful to have non-QmSoL PeerIDs in the bootstrap list because it makes incidents with the existing Digitalocean-based bootstrappers less bad. Even if those hosts aren't running at the moment, it lets us spin them up quickly when needed.

The non-QmSoL PeerIDs here are our upgrade path for the bootstrap config.

this bootstrap peer was never even dialled because the mismatch in the peer ID between the TXT record and the hardcoded multiaddrs made this check fail

We can probably just remove these existing TXT records. I feel it doesn't make much sense to put the QmSoL PeerIDs in dnsaddrs. They're already in the list based on their IP addresses anyway, and we can't move these PeerIDs to other IP addresses because that'll break bootstrap for everyone up until go-ipfs v0.4.10. (In js-ipfs we've used dns right from the start.)

@anacrolix
Copy link
Contributor Author

It's actually good having some bootstrap nodes that don't exist, for my purposes of debugging. So it sounds like I should leave all the addrs in, and also add the new extra one? Is there any way to have these addresses without specifying a peer ID, so that we just connect to whoever is at bootstrap.libp2p.io?

@anacrolix
Copy link
Contributor Author

@raulk yes that exact line took me a very long time to find, and eventually led to this PR.

@ghost
Copy link

ghost commented Feb 16, 2019

So it sounds like I should leave all the addrs in, and also add the new extra one

Just leave them as-is, the QmSoL addresses don't make much sense as a dnsaddr. (Just because they're bound to the exact IP address for semi-historic reasons.)

Is there any way to have these addresses without specifying a peer ID, so that we just connect to whoever is at bootstrap.libp2p.io?

Yeah, just drop the part after /dnsaddr/$domain -- whatever comes after /dnsaddr/$domain is used for optionally matching the returned addresses.

@ghost
Copy link

ghost commented Feb 16, 2019

Unless there's someone against it, I'll remove these /dnsaddr/bootstrap.libp2p.io/ipfs/QmSoLer265NRgSp2LA3dPaeykiS1J6DifTC88f5uVQKNAd TXT records. I'm cleaning up and fixing DNS anyway.

@anacrolix
Copy link
Contributor Author

@lgierth removing the /ipfs/$peerid from the end of addresses made them undialable (rejected for not having a peer ID specified or a mismatch I can't remember). That was my first instinct after poking through the code to see how the resolutions occur.

So what exactly is the consensus on changes to the bootstrap address list. Should I leave it unchanged for now?

@Kubuxu
Copy link
Member

Kubuxu commented Feb 19, 2019

removing the /ipfs/$peerid from the end of addresses made them undialable

We need those peer IDs at the end to prevent MitM attacks.

@Kubuxu
Copy link
Member

Kubuxu commented Feb 19, 2019

So what exactly is the consensus on changes to the bootstrap address list. Should I leave it unchanged for now?

IMO leave them as it is, given that we have keys from them we can deploy them in future (with people already having them in the config).

@anacrolix
Copy link
Contributor Author

What about adding just "/dnsaddr/bootstrap.libp2p.io/ipfs/QmSoLer265NRgSp2LA3dPaeykiS1J6DifTC88f5uVQKNAd"?

@ghost
Copy link

ghost commented Feb 20, 2019

What about adding just "/dnsaddr/bootstrap.libp2p.io/ipfs/QmSoLer265NRgSp2LA3dPaeykiS1J6DifTC88f5uVQKNAd"?

It doesn't help because the QmSoL PeerIDs are bound to their current IP addresses - too many existing nodes have them in their configs for an IP address change to be doable. And if we can't change their IP address, DNS doesn't help.

@anacrolix
Copy link
Contributor Author

I'm very confused, let me lay out what I understand:

  • The existing *.i.ipfs.io entries with explicit IPs are all correct, and should remain.
  • The existing /dnsaddr/bootstrap.libp2p.io/ entries are correct, but not currently active, and should remain in the list.
  • The currently active /dnsaddr/bootstrap.libp2p.io/ipfs/QmSoLer265NRgSp2LA3dPaeykiS1J6DifTC88f5uVQKNAd node is temporary, should not be relied on, and should not be added to the bootstrap list.

Is that correct?

@anacrolix anacrolix closed this Feb 26, 2019
@ghost ghost removed the status/in-progress In progress label Feb 26, 2019
@anacrolix anacrolix deleted the default-bootstrap-dnsaddrs branch February 28, 2019 07:30
@Stebalien
Copy link
Member

@lgierth can we remove the current dnsaddr records for these domains?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants