[Feature request] Proper error code when peer is not connected

[NicolasDorier]

Issue and Steps to Reproduce

At every release, lightningd is breaking clients due to not using proper error code when funding a channel to a peer that is not connected.

Workaround

Testing on several possible different error message thrown at each version of clightning.

btcpayserver/BTCPayServer.Lightning@f78bd5b#diff-3aa3aa916b8d25b5d374f2953506b989L357-R362

[cdecker]

Would checking the error code not be more sensible? We can definitely assign a
more unique numeric code to "Not connected", "No address" and "Unknown peer",
and you could better match those and take the appropriate actions (looking up
addresses, reporting the details up to the user, etc).

The errors really are distinct as they report various different causes that
led the fundchannel to fail, and we can't just merge them into a "something
went wrong" error.

fwiw the "Unable to connect" error in response to fundchannel is new only
because we introduced an autoconnect feature in v0.8.0, since users reported
that it was strange to have a peer in the list, know its address, and don't
just connect in case we aren't.

[NicolasDorier]

@cdecker anything I can compare other than an error message would make sense.

[vasild]

This should help #3395?

The error "Unable to connect, no address known for peer" (https://github.com/ElementsProject/lightning/blob/f6ff5e5/connectd/connectd.c#L1431) is not returned during funding, but during connect. How to add a code for it? connect_failed() is only sending a textual error message to the master daemon.

[ZmnSCPxj]

How to add a code for it?

You will need to modify the message from connectd to lightningd, in file connectd/connect_wire.csv, to add an error code from connectd for the connect_failed message. This changes all fromwire_connectctl_connect_failed and towire_connectctl_connect_failed calls (there should just be one of each, fromwire in lightningd/connect_control.c and towire somewhere in connectd/).

The type to use for the error code is a bit iffy though. You could cast to a u64 for the message but that is bad style; unfortunately it seems there is not yet any use for an ordinary int for messaging yet. You could probably just create some kind of fromwire_int and towire_int in wire/fromwire.c and wire/towire.c, and do some magic in tools/generate-wire.py (add it to assignables array I think).

[vasild]

@ZmnSCPxj thanks for the explanation! Done in #3397, as a separate PR because it is not related to funding. I used u32 with an ugly typecast to int for the error code type.

[vasild]

This can be closed now, after #3395 and #3397 have been merged, @NicolasDorier?

[NicolasDorier]

@vasild thank you, closing it. I have not tested your PRs though, but it seems to be what I needed.