search

paritytech / polkadot-sdk

The Parity Polkadot Blockchain SDK
https://polkadot.com/
1.94k stars 710 forks source link

network: Require validators to provide CLI param `--public-addresses` for starting nodes #5266

Open lexnv opened 3 months ago

lexnv commented 3 months ago

Requiring validators to provide public addresses ensures the authority can be discovered sooner.

This should happen in one of the following releases, after the release that includes:

bkchr commented 3 months ago

Why? I still don't get this. You should just be able to query the local interfaces to find out the public addr? (Assuming they are not behind a NAT, which we don't support any way for validators)

nicolasochem commented 2 months ago

in a datacenter environment, it's pretty common to be behind a load balancer, in which case the public ip is never discoverable from a local interface.

but libp2p has various mechanisms to find the "real" ip so I also think it should not be mandatory to force-set it.

bkchr commented 2 months ago

in a datacenter environment, it's pretty common to be behind a load balancer, in which case the public ip is never discoverable from a local interface.

What kind of load balancer? I mean you can only have 1 validator running, so what is it load balancing?

lexnv commented 2 months ago

My initial thought was that we may have validators running under a NAT. Similar to the comment above, a node operator might start a node on their machine and will probably be behind a firewall. Some authority DHT records provided addresses that were unreachable last year when I experimented with collecting records.

I think we can change the behavior of the authority discovery to:

I double checked the listen addresses reported by litep2p running in the cloud, they will look similar to:

/ip6/[valid-ipv6-addr/tcp/3333/ws/p2p/...
/ip6/::1/tcp/30333/ws/p2p/...
/ip4/[valid-ip-v4]/tcp/30333/ws/p2p/...
/ip4/127.0.0.1/tcp/30333/...

From these details, it looks like we can find at least 2 global addresses, so in most of the cases we'll never emit the warning.

I'll come with a follow-up to the initial PR and only warn if we cannot find global ips in our listen addresses: https://github.com/paritytech/polkadot-sdk/pull/5240

Thanks for the feedback, let me know if that sounds like a plan 🙏

bkchr commented 1 month ago

My initial thought was that we may have validators running under a NAT.

At least for Polkadot we would not support this right now. AFAIK we don't support NAT hole punching and this would be problematic for collators who need a direct connection to the validators.

  • publish [public addresses (first) ++ local interface if global (second) ++ network discovered via Identify protocol]

Yeah that sounds reasonable. I think I had proposed somewhere already.

  • emit this warning if no public addresses are provided and no global ips are discovered from the local interfaces

Yeah also a good point.