backend support for 'close reason'

[wdiffman]

Closes #10389 following @SomberNight 's suggestion. Used reason instead of diagnosis, though.
In the test, bob's close reason isn't guaranteed to be set by the time alice's close_channel returns, so I used a short sleep; please advise if there is a better way.

[SomberNight]

Hey. Thanks for the PR. Added some comments.

[SomberNight]

You should also set the close reason in the LOCAL case here.
First of all, it is nicer to make the code (in the local scope) symmetric/homogeneous.
But also, ultimately the final close reason should reflect what gets mined and weird chains of events could happen: e.g. local force-close gets broadcast to mempool first, then remote force-close replaces it in mempool, but then local force-close gets mined. If you call save_close_reason here, it will self-heal.

We could keep the other call in lnworker._force_close_channel in addition to this one, as that one runs much earlier.

[SomberNight]

I think it does not make sense to add a column for this. It takes a lot of horizontal space and adds noise.
Isn't it enough to show this in the channel details dialog?

[wdiffman]

Agree. I only added because I thought someone might want to search for channels closed with a given reason. But if that's the case, we can fix it later. I will remove.

[SomberNight]

note: this would make the names of the enum members part of the external API

also see other comment re wallet db upgrades

[SomberNight]

re the comment # Harder to read, but keeps it within a single line. -- ok, but the comment itself is not needed IMO

[SomberNight]

This would make the names of the enum members part of the wallet db. That is, if someone later renames e.g. ChanCloseReason.LOCAL_FORCE to ChanCloseReason.LOCAL_FORCE_CLOSE, they need to do a wallet db upgrade.

IMO it is preferable to use the names and not the int values in the wallet db, so this is fine, but it is completely unintuitive and error-prone for the programmer, so at least a comment must be added.

Just duplicate this comment:

electrum/electrum/lnchannel.py

Lines 77 to 78 in 1235b4a

	# Note: these states are persisted by name (for a given channel) in the wallet file,
	# so consider doing a wallet db upgrade when changing them.

[SomberNight]

On my machine, the test passes even with this sleep completely removed. Is that not the case for you?

Anyway, the simplest less-wasteful thing that could be done is:

            async with util.async_timeout(1):
                while not bob_channel.is_closed_or_closing():
                    await asyncio.sleep(0.01)

Something more complicated:

diff --git a/tests/test_lnpeer.py b/tests/test_lnpeer.py
index a4950ba9d5..b65f78a041 100644
--- a/tests/test_lnpeer.py
+++ b/tests/test_lnpeer.py
@@ -1636,12 +1636,21 @@ class TestPeerDirect(TestPeer):
         w2.network.config.TEST_SHUTDOWN_FEE = 100
         w1.network.config.TEST_SHUTDOWN_LEGACY = True
         w2.network.config.TEST_SHUTDOWN_LEGACY = True
+
+        any_chan_changed = asyncio.Event()
+        async def on_chan_changed(*args):
+            any_chan_changed.set()
+            any_chan_changed.clear()
+        util.register_callback(on_chan_changed, ["channel"])
+
         async def action():
             await util.wait_for2(p1.initialized, 1)
             await util.wait_for2(p2.initialized, 1)
             await p1.close_channel(alice_channel.channel_id)
             # alice's side is completed, but bob need to wait to see the reason.
-            await asyncio.sleep(1)  # FIXME: use a better wait
+            async with util.async_timeout(1):
+                while not bob_channel.is_closed_or_closing():
+                    await any_chan_changed.wait()
             self.assertEqual(alice_channel.get_close_reason(), ChanCloseReason.LOCAL_COOP)
             self.assertEqual(bob_channel.get_close_reason(), ChanCloseReason.REMOTE_COOP)
             gath.cancel()

[wdiffman]

Indeed it does, but that's will not be necessarily true for all runs.
I will use the simple wait, but thanks for sharing the more complete version.