The `WE_ARE_TOXIC` state is added as a sanity check to ensure that if
the remote has proven that we have lost state we do not accidentally
do a local force-close. E.g. if we receive an "error" message for the
channel, we might normally do an automatic force-close. Manually
force-closing in such a state is not offered anymore by the GUI.
The `REQUESTED_FCLOSE` state is added as it is quite likely that
we receive an error message from the remote after requesting a fclose,
e.g. during a later chan-reestablish. In such a scenario, we should
not do an auto-local-fclose, however the manual option of a local-fclose
should still be offered.
- separate AddressSynchronizer from Wallet and LNWatcher
- the AddressSynchronizer class is referred to as 'adb' (address database)
- Use callbacks to replace overloaded methods
- Do not send ping if messages have been received recently.
- Do not send more than one ping.
- Await pong before sending commitment_signed (per BOLT-2)
- Lower ping time to 30s
When replaying messages during channel-reestablishment,
previously we first resent all update messages, along with potential commitment_signed messages,
and then we potentially resent a single revoke_and_ack.
This can result in incorrect behaviour in case both a commitment_signed and a revoke_and_ack needs to be resent.
When replaying messages, the relative order of commitment_signed and revoke_and_messages needs to be preserved.
(the order of updates (htlc/fee) in relation to the revack messages does not matter)
implements https://github.com/lightning/bolts/pull/810
The logic here is somewhat based on what c-lightning does:
01e5f1886e/channeld/channeld.c (L3059)
- rm the `_get_channel_ids` abstraction as each of its usages needs subtle differences.
Some code duplication is preferable in this case.
- raise exceptions in `wait_for_message`, so that callers such as the GUI can show user-feedback
- on_error/on_warning were dropping messages with temp_chan_ids if they were not stored in
`temp_id_to_id` - which was only done once the mapping was known (so the normal chan_id was known).
To fix this, we now store temp_chan_ids into `temp_id_to_id` early.
- `schedule_force_closing` only works if the chan_id is already in `channels`
related:
https://github.com/spesmilo/electrum/pull/7645 (and related commits)
-----
example before commit:
```
D/P | lnpeer.Peer.[LNWallet, 03933884aa-3b53e4ab] | Sending OPEN_CHANNEL
D/P | lnpeer.Peer.[LNWallet, 03933884aa-3b53e4ab] | Received ERROR
I/P | lnpeer.Peer.[LNWallet, 03933884aa-3b53e4ab] | remote peer sent error [DO NOT TRUST THIS MESSAGE]: invalid funding_satoshis=10000 sat (min=400000 sat max=1500000000 sat)
E | gui.qt.main_window.[test_segwit_2] | Could not open channel
Traceback (most recent call last):
File "...\electrum\electrum\util.py", line 1160, in wrapper
return await func(*args, **kwargs)
File "...\electrum\electrum\lnpeer.py", line 661, in wrapper
return await func(self, *args, **kwargs)
File "...\electrum\electrum\lnpeer.py", line 742, in channel_establishment_flow
payload = await self.wait_for_message('accept_channel', temp_channel_id) #
File "...\electrum\electrum\lnpeer.py", line 315, in wait_for_message
name, payload = await asyncio.wait_for(q.get(), LN_P2P_NETWORK_TIMEOUT)
File "...\Python39\lib\asyncio\tasks.py", line 468, in wait_for
await waiter
asyncio.exceptions.CancelledError
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "...\Python39\lib\asyncio\tasks.py", line 492, in wait_for
fut.result()
asyncio.exceptions.CancelledError
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "...\electrum\electrum\gui\qt\util.py", line 914, in run
result = task.task()
File "...\electrum\electrum\gui\qt\main_window.py", line 1875, in task
return self.wallet.lnworker.open_channel(
File "...\electrum\electrum\lnworker.py", line 1075, in open_channel
chan, funding_tx = fut.result()
File "...\Python39\lib\concurrent\futures\_base.py", line 445, in result
return self.__get_result()
File "...\Python39\lib\concurrent\futures\_base.py", line 390, in __get_result
raise self._exception
File "...\electrum\electrum\util.py", line 1160, in wrapper
return await func(*args, **kwargs)
File "...\electrum\electrum\lnworker.py", line 1006, in _open_channel_coroutine
chan, funding_tx = await asyncio.wait_for(coro, LN_P2P_NETWORK_TIMEOUT)
File "...\Python39\lib\asyncio\tasks.py", line 494, in wait_for
raise exceptions.TimeoutError() from exc
asyncio.exceptions.TimeoutError
```
example after commit:
```
D/P | lnpeer.Peer.[LNWallet, 03933884aa-ff3a866f] | Sending OPEN_CHANNEL
D/P | lnpeer.Peer.[LNWallet, 03933884aa-ff3a866f] | Received ERROR
I/P | lnpeer.Peer.[LNWallet, 03933884aa-ff3a866f] | remote peer sent error [DO NOT TRUST THIS MESSAGE]: invalid funding_satoshis=10000 sat (min=400000 sat max=1500000000 sat). chan_id=124ca21fa6aa2993430ad71f465f0d44731ef87f7478e4b31327e4459b5a3988
E | lnworker.LNWallet.[test_segwit_2] | Exception in _open_channel_coroutine: GracefulDisconnect('remote peer sent error [DO NOT TRUST THIS MESSAGE]: invalid funding_satoshis=10000 sat (min=400000 sat max=1500000000 sat)')
Traceback (most recent call last):
File "...\electrum\electrum\util.py", line 1160, in wrapper
return await func(*args, **kwargs)
File "...\electrum\electrum\lnworker.py", line 1006, in _open_channel_coroutine
chan, funding_tx = await asyncio.wait_for(coro, LN_P2P_NETWORK_TIMEOUT)
File "...\Python39\lib\asyncio\tasks.py", line 481, in wait_for
return fut.result()
File "...\electrum\electrum\lnpeer.py", line 673, in wrapper
return await func(self, *args, **kwargs)
File "...\electrum\electrum\lnpeer.py", line 755, in channel_establishment_flow
payload = await self.wait_for_message('accept_channel', temp_channel_id)
File "...\electrum\electrum\lnpeer.py", line 326, in wait_for_message
raise GracefulDisconnect(
electrum.interface.GracefulDisconnect: remote peer sent error [DO NOT TRUST THIS MESSAGE]: invalid funding_satoshis=10000 sat (min=400000 sat max=1500000000 sat)
I/P | lnpeer.Peer.[LNWallet, 03933884aa-ff3a866f] | Disconnecting: GracefulDisconnect()
```
- the fee negotiation is split into smaller functions, reducing the scope of variables.
- the while loop logic is condensed in a few lines, so it is easier to understand termination conditions.
- removed code that was never executed
Updates the closing fee negotiation to comply with most recent spec
changes, see https://github.com/lightning/bolts/pull/847
The closing negotiation is backwards compatible with the old
negotiation.
Messages are sent in reestablish_channel (async)
Message checks and force_close are performed in on_channel_reestablish (not async).
That task should not be cancelled if the connection is closed.
Revert 57583c05cf
* Refactor `create_trampoline_route`.
* Enables end-to-end multi-trampoline multipart payments.
Trampoline-to-legacy payments are still not enabled, as this is
currently not supported by Eclair.
* Reverts to a global trampoline fee level, as trampoline failures
are currently not handled properly, see (#7648), which doubles
fee rates.
In particular, in the regtests, with incoming peers, we can have multiple transports open with the same node simultaneously
(see e.g. lnworker._request_force_close_from_backup).
We now use the first few bytes of peer_pubkey, as that is potentially familiar to users,
and the first few bytes of sha256(id(self)) to mitigate collisions in case the peer_pubkeys collide.
log excerpt:
```
I/P | lnpeer.Peer.[LNWallet, 030f0bf260-e0b33756] | handshake done for 030f0bf260acdbd3edcad84d7588ec7c5df4711e87e6a23016f989b8d3a4147230@163.172.94.64:9735
D/P | lnpeer.Peer.[LNWallet, 030f0bf260-e0b33756] | Sending INIT
I/P | lnpeer.Peer.[LNWallet, 03933884aa-5e5dce45] | handshake done for 03933884aaf1d6b108397e5efe5c86bcf2d8ca8d2f700eda99db9214fc2712b134@34.250.234.192:9735
D/P | lnpeer.Peer.[LNWallet, 03933884aa-5e5dce45] | Sending INIT
D/P | lnpeer.Peer.[LNWallet, 030f0bf260-e0b33756] | Received INIT
I/P | lnpeer.Peer.[LNWallet, 02651acf4a-79696c42] | handshake done for 02651acf4a7096091bf42baad19b3643ea318d6979f6dcc16ebaec43d5b0f4baf2@82.119.233.36:19735
D/P | lnpeer.Peer.[LNWallet, 02651acf4a-79696c42] | Sending INIT
D/P | lnpeer.Peer.[LNWallet, 03933884aa-5e5dce45] | Received INIT
I/P | lnpeer.Peer.[LNWallet, 030f0bf260-e0b33756] | saved remote_update
D/P | lnpeer.Peer.[LNWallet, 030f0bf260-e0b33756] | Received CHANNEL_REESTABLISH
```
aiorpcx 0.20 changed the behaviour/API of TaskGroups.
When used as a context manager, TaskGroups no longer propagate
exceptions raised by their tasks. Instead, the calling code has
to explicitly check the results of tasks and decide whether to re-raise
any exceptions.
This is a significant change, and so this commit introduces "OldTaskGroup",
which should behave as the TaskGroup class of old aiorpcx. All existing
usages of TaskGroup are replaced with OldTaskGroup.
closes https://github.com/spesmilo/electrum/issues/7446
I think this was originally needed due to incorrect management of group lifecycles,
which our current code is doing better.
also note that if we needed this, in newer aiorpcx, the name of
the field was ~changed from `_closed` to `joined`:
239002689a
`drop_to_remote` can be False and at the same time the to_remote output
is not present, because it is below dust. Therefore, we have to explicitly
check if to_remote is present when checking for the allowed script types and
dust limits. This affects channels which have sent only dust values, they
can't be closed unilaterally without this fix.
Fixes a regression introduced by 947693c90d.
* on channel opening we verify that the peer's dust limit is above 354
sat, the limit for unknown segwit versions
* we constrain the allowed scriptpubkey types for channel closing
* we check that the remote's output is above the relay dust limit for
the collaborative close case
- better error handling: previously we stopped all attempts on any of
TRAMPOLINE_EXPIRY_TOO_SOON, UNKNOWN_NEXT_PEER, TEMPORARY_NODE_FAILURE.
Instead we should retry (but see code comments).
- previously payments failed if ALL of the following criteria applied:
- sender is paying via trampoline, but not via the ACINQ node (which is
special cased)
- receiver only has private channels and has put r_tags into invoice, along
with setting the trampoline feature bit in the invoice, however the receiver
is not connected to any trampoline forwarders directly
The sender would then assume that the private routing hints in the invoice
correspond to trampoline forwarders.
- also, previously if both the sender and the recipient used trampoline and
they shared a trampoline forwarder (that they were both connected to), the
private channels the recipient had (with nodes other than the shared TF)
would never be attempted.
Previously for incoming transports, the diagnostic_name (for log messages)
was just "responder" -- not sufficient to distinguish peers.
We now use the pubkey instead.
For outgoing transports it is f"{host}:{port}" (unchanged).
We could just use the pubkey for both uniformly; but it is quite long, and
it is hard to distinguish them at a glance.
(related to prev commit, but really another bug)
If we had two peers with the same pubkey (peer A in the process of teardown, peer B ~freshly connected),
peer A might remove peer B from lnworker.peers via close_and_cleanup().
rm `close_and_cleanup()` call from reestablish_channel - it was added
as a workaround for this bug (in 8b95b2127d)
before we understood the cause.
Note that this will not fix the value for already existing channels
that have been created with onchain backup; one would need a wallet_db
upgrade in order to fix them (probably not worth the effort).
SomberNight
ThomasV
bitromortac
sgmoore