gossipd: Turn off IP discovery when we already have useable addresses.

[m-schmoock]

Currently, IP discovery will be active (unless turned off) also when users configure announced addresses manually.
When the manual configuration uses a non-default port, this will lead to the strange behavior of announcing the same address twice. One with the correct and one with the guessed default port.

The idea is that we turn off IP discovery when a usable announcement has been defined via commandline, config or autobind.
Also this PR sets the 'guessed' port depending on what network (Mainnet, Testnet, ...) we are on.

Followup

• Make the switch --disable-ip-discovery obsolete it and maybe add a --ip-discovery true/false(/auto) switch to force it on or off or keeping it automatically, if not defined.

Note:

We could/should also enable IP discovery when we only have a working TOR address (but without --always-use-proxy ofc). This will give nodes an option to have a bootstrap way to be reachable until IP discovery can do the job in a more stable way.
Keep in mind its easier for most users to punch a hole in a NAT router than to setup TOR correctly.
If/Once we have a --ip-discovery=true switch the user can also do that manually if he wants to.

Addresses #5305

[vincenzopalazzo]

Concept ACK

[rustyrussell]

This will make the switch '--disable-ip-discovery' more or less pointless (except for edge cases). We can think about adding a '--enable-ip-discovery' switch instead or also.

This is why --enable/disable flags are generally poor design: --ip-discovery= is better, since it can be overridden later in commandline too.

[niftynei]

@m-schmoock we're looking to cut an RC this week, and I'd love to get this added. It looks like this just needs a rebase.

[m-schmoock]

@m-schmoock we're looking to cut an RC this week, and I'd love to get this added. It looks like this just needs a rebase.

Ok, I'll try to finish it. It's a little bit more though because we also need to depreciate the '--disable-ip-discovery' flag, update docs and tests etc.

[m-schmoock]

Note: Needed to fix some outdated gossmap tests that broke CI

[m-schmoock]

@rustyrussell
So now IP discovery is only active if no other addresses are defined (or usable). I would like the option to have a TOR address announced but also IP discovery enabled. How can we achieve that?

1. TODO anyway: Deprecate --disable-ip-discovery. Whats the proper way of deprecating a paramter? Just remove it or raise a warning/error?
2. Add a --ip-discovery=true/false(/auto) switch. But this can't be a strict bool, because we want IP discovery to kick in as a fallback when no addresses are defined. Therefore we don't want to default to true nor do we want to default to false but 'auto'. Whats the proper way to do that? For example in lightningd/lightningd.h:

enum config_autobool {
    AUTOBOOL_AUTO,
    AUTOBOOL_ON,
    AUTOBOOL_OFF
};

// ... plus the corresponding helpers to register this as an arg...

3. Alternatively to 2. we can have it as a string arg with value checking. But that's ugly too for something that's more like an optional<boolean>.
4. Alternatively to 2. and 3. we can enable annoucement of discovered IPs when there are only non-IP addresses announced (e.g. just a TOR address).

[rustyrussell]

Number 2 is correct. And default to AUTO.

[m-schmoock]

Number 2 is correct. And default to AUTO.

@rustyrussell
Okay will add that.
And how about the deprecation of the --disable-ip-discovery flag? just drop it or fail/warn?

[rustyrussell]

Number 2 is correct. And default to AUTO.

@rustyrussell Okay will add that. And how about the deprecation of the --disable-ip-discovery flag? just drop it or fail/warn?

You need to deprecate it: accept it if deprecated_apis is false with a log_unusual() warning, put it in Changelog, remove it from documentation...

[rustyrussell]

Ack f02e510

[vincenzopalazzo]

ACK f02e510