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

feat: resolve hostname to multiple addresses when connecting to a node #484

Closed
tegefaulkes opened this issue Oct 19, 2022 · 8 comments · Fixed by #491
Closed

feat: resolve hostname to multiple addresses when connecting to a node #484

tegefaulkes opened this issue Oct 19, 2022 · 8 comments · Fixed by #491
Assignees
Labels
development Standard development r&d:polykey:core activity 4 End to End Networking behind Consumer NAT Devices

Comments

@tegefaulkes
Copy link
Contributor

tegefaulkes commented Oct 19, 2022

Specification

We need to expand the functionality of establishing connections to handle multiple possible addresses when resolving a host name. The most notable use case here is when connecting to the test net. The hostname testnet.polykey.io can resolve to all of our seed nodes. We need to resolve this to multiple A or AAAA records and attempt connections to each one when connecting to a node.

Additional context

Tasks

  1. Add ability to resolve a hostname to multiple addresses.
  2. add functionality to connect to multiple addresses when connecting to a node.
@CMCDragonkai
Copy link
Member

The resolveHost function in network/utils right now takes Host | Hostname. This should be changed to resolveHostname and it should just take a single Hostname and return Promise<Array<Host>>.

Or AsyncGenerator<Host> depending on how the module works. I'll try both.

It's also referenced by NodeConnectionManager, NodeManager and tests/network/utils.test.ts.

@CMCDragonkai
Copy link
Member

The current implementation uses dns.lookup which is using the OS's facility to do a DNS lookup and thus is affected by OS configuration.

But if we use dns.resolve, it always uses the DNS protocol does its own thing.

I think the dns.resolve is a better solution so as an application we don't get weird errors with respect to misconfiguration on the OS.

On the other hand, having the OS /etc/hosts actually affect the DNS lookup could be useful in certain cases. Like for manual configuration.

I think for general use case, it should be using dns.resolve. We can bubble up an option up to the front of PK agent start that allows one to switch to using dns.lookup as it's more a developer option.

@CMCDragonkai
Copy link
Member

Note that once we start using dns.resolve, we have the ability to use SRV records. If we ever want to have dynamic ports rather than fixing the ports of all the PK nodes, the SRV records would be a useful way. This is something to investigate later.

@CMCDragonkai
Copy link
Member

There is one problem, the if we use dns.resolve we have to preconfigure our DNS servers, and we aren't relying on the underlying OS's DNS servers anymore.

This could be solved with some global addresses like the ones we use here:

  [
    "1.1.1.1"                    # cloudflare 1
    "1.0.0.1"                    # cloudflare 2
    "8.8.8.8"                    # google 1
    "8.8.4.4"                    # google 2
    "2606:4700:4700::1111"       # cloudflare ipv6 1
    "2606:4700:4700::1001"       # cloudflare ipv6 2
    "2001:4860:4860::8888"       # google ipv6 1
    "2001:4860:4860::8844"       # google ipv6 2
  ]

@CMCDragonkai
Copy link
Member

Actually I've found by default the resolver has 127.0.0.1 set as the default resolver.

However if I give it random useless IP, it actually still works.

I'm wondering now if resolve actually fallsback on the OS.

But if I give it no addresses at all, it then comes with an error... so how does this work?

@CMCDragonkai
Copy link
Member

I've had to ask a question about this: https://stackoverflow.com/questions/74260036/how-does-nodejs-resolve-dns-with-fallback-when-all-servers-are-incorrect


So for now, I'm going to use the resolver, but I'm going to set the addresses statically for now, and add 127.0.0.1 as the fallback at the end. Perhaps by just calling getServers() first and spreading whatever is considered default.

@CMCDragonkai
Copy link
Member

Ok instead I can use dns.getServers(), it actually inherits the default DNS settings for the OS. That means I don't need to set custom DNS servers above.

For example on my windows computer, it says ['1.1.1.1', '1.0.0.1', ... ]. So at least here we get some inheritance of the configuration at the OS level.

@CMCDragonkai
Copy link
Member

The DNS resolve timeout is the timeout PER dns packet. The number of times it tries multiplied by the timeout is actually what determines the maximum time spent doing the DNS query.

The maximum timeout is largest 32 bit number 2147483647.

The default of -1 actually results in an exponential backoff, starting at 5 seconds and doubling each time. So 5, 10, 20, 40 seconds... etc.

We can leave the timeout as is with -1 being whatever is the default.

If we want to integrate it into our timedCancellable mechanism, it would make sense to instead give a very large tries number, and do a signal based cancellation instead with resolver.cancel().

The maximum number is again the largest 32 bit number of 2147483647.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Standard development r&d:polykey:core activity 4 End to End Networking behind Consumer NAT Devices
Development

Successfully merging a pull request may close this issue.

2 participants