> These kind of checks look incorrect and confused/confusing for me:
> 43a5d03426/electrum/gui/qml/qeinvoice.py (L388)
> 43a5d03426/electrum/gui/qt/main_window.py (L1801)
> For example, consider creating a local tx that spends all UTXOs in the wallet, and consolidates them to another ismine address.
> Such a tx basically does not change what wallet.get_balance() returns.
> but to the checks listed above, the confirmed balance of the wallet I guess should be 0?
- new version of flake8-bugbear (24.1.16) introduced new tests (B036), which we were failing
- that's fine, but our CI should not suddenly break because of this
- better to do explicit manual version bumps of the linter
In particular, lnd sends both chan_reest and then an error ("sync error").
It is critical we process the chan_reest and transition to WE_ARE_TOXIC before
processing the error (which would trigger us to force-close).
see spec 8a64c6a1ce/02-peer-protocol.md (L1504-L1506) :
> - upon reconnection:
> [...]
> - MUST transmit channel_reestablish for each channel.
> - MUST wait to receive the other node's channel_reestablish message before sending any other messages for that channel.
if the remote does not know we are behind, they will try to send us chan_reest
```
38.52 | W | P/lnpeer.Peer.[LNWallet, 034cd7a09f-ed50cf95] | channel_reestablish (533x1x1): remote is ahead of us! They should force-close. Remote PCP: 03857eabd943eec820d56b94d2f162763294565627cc42c4a0db0e7c6b77da46be
38.52 | E | P/lnpeer.Peer.[LNWallet, 034cd7a09f-ed50cf95] | Exception in main_loop: Exception('Transition not allowed: WE_ARE_TOXIC -> WE_ARE_TOXIC')
Traceback (most recent call last):
File "/home/user/wspace/electrum/electrum/util.py", line 1148, in wrapper
return await func(*args, **kwargs)
File "/home/user/wspace/electrum/electrum/lnpeer.py", line 426, in wrapper_func
return await func(self, *args, **kwargs)
File "/home/user/wspace/electrum/electrum/lnpeer.py", line 440, in main_loop
async with self.taskgroup as group:
File "/home/user/.local/lib/python3.10/site-packages/aiorpcx/curio.py", line 297, in __aexit__
await self.join()
File "/home/user/wspace/electrum/electrum/util.py", line 1319, in join
task.result()
File "/home/user/wspace/electrum/electrum/lnpeer.py", line 1231, in on_channel_reestablish
chan.set_state(ChannelState.WE_ARE_TOXIC)
File "/home/user/wspace/electrum/electrum/lnchannel.py", line 215, in set_state
raise Exception(f"Transition not allowed: {old_state.name} -> {state.name}")
Exception: Transition not allowed: WE_ARE_TOXIC -> WE_ARE_TOXIC
```
Assume Alice and Bob have a channel, and Alice is on an old state,
but neither of them knows the latter yet.
Timing scenarios:
1. Alice sends reest first, and Bob receives it before sending reest himself
- old code: Bob realises Alice is behind, Bob will force-close,
Bob won't send reest to Alice, so Alice does not learn she is behind
- new code: Bob realises Alice is behind, Bob will force-close,
Bob will still send reest to Alice, so Alice learns she is behind.
2. Bob sends reest first, and Alice receives it before sending reest herself
- old code: Alice learns she is behind. Alice won't send reest to Bob,
so Bob does not learn he is ahead, so Bob won't force-close.
- new code: Alice learns she is behind. Alice will still send reest to Bob
though with ctn=0 instead of actual. Bob learns he is ahead, so
Bob will force close.
3. Alice and Bob both send reest, and then they both receive what the other sent
- no change: Alice and Bob both learn Alice is behind. Bob will force-close.
lnworker.suggest_splits for non-trampoline case tries to split amts over 5000 sat
but mpp_split.suggest_splits does not return splits where any part is smaller than 10000 sat.
So in practice, without trampoline, invoices between 5k and ~20k sats could not be paid.
This suggests that the logic should not be scattered in multiple places but merged into mpp_split.py...
This commit just does a quick fix though, to try again without splitting if there was no solution.
SomberNight
Sander van Grieken
ThomasV