We must not process incoming updates for a given channel until we ~finished reestablishing it.
Consider both parties have some unacked updates they want to replay during reestablish.
If Bob reacts to Alice's replayed stuff before he himself replays his stuff, madness ensues.
I think this should fix the remaining part of https://github.com/spesmilo/electrum/pull/8778
(timing issues when running the unit tests with py3.12)
This allows making any message handler async in lnpeer.
Note: `process_message` is only called from `_message_loop`.
There are(would be) basically three types of message handlers:
1. "traditional blocking msg handlers". non-async ones. When these handlers are called, `process_message` naturally blocks until the handler returns, which means `_message_loop` also blocks until the message is fully processed before starting the next iteration.
2. "async blocking msg handlers". async ones where we want the previous property, i.e. we want the `_message_loop` to wait until the handler finishes. We await the handler inside `process_message`, and `_message_loop` awaits `process_message`.
3. "async non-blocking msg handlers". async message handlers that can be spawned e.g. onto `Peer.taskgroup` and the loop is free to start processing subsequent messages. e.g. msg handlers that start a negotiation, such as `on_shutdown` and `on_open_channel`.
Any non-async message handler (`def on_...`) automatically goes into category 1.
An async message handler, by default, goes into category 2, "blocking";
to go into category 3 ("non-blocking"), we use the `runs_in_taskgroup` function decorator.
- scenario:
- reuse same seed between two devices, LN enabled on both
- open channel using device1, import chan backup on device2
- local-force-close channel using device1
- tx1 (ctx) gets into mempool (or even mined), tx2 (sweep tx for to_local) is a local (future) tx
- history tab on device1 shows tx1 and tx2
- history tab on device2 was showing only tx2, and no info about tx1
- device2 knows about tx1, it was just not showing it previously.
With this commit, tx1 is now shown in the history.
- note: tx1 might linger in the mempool for an indeterminate amount of time, or even become local.
During that time, it is confusing on device2 not to show any feedback. Also,
if tx1 becomes local, it is useful to be able to rebroadcast it.
was getting log spam when running the pycharm debugger in certain cases:
```
3.29 | E | concurrent.futures | exception calling callback for <Future at 0x1d4987bcc70 state=cancelled>
Traceback (most recent call last):
File "...\Python310\lib\concurrent\futures\_base.py", line 342, in _invoke_callbacks
callback(self)
File "...\electrum\electrum\util.py", line 1785, in on_done
if exc := fut_.exception():
File "...\Python310\lib\concurrent\futures\_base.py", line 485, in exception
raise CancelledError()
concurrent.futures._base.CancelledError
```
```
$ ./electrum/scripts/quick_start.py
Traceback (most recent call last):
File "/home/user/wspace/electrum/./electrum/scripts/quick_start.py", line 32, in <module>
wallet.start_network(network)
File "/home/user/wspace/electrum/electrum/wallet.py", line 620, in start_network
assert self.network is None, "already started"
AssertionError: already started
```
just the sanity check added in 6ac3f84095
user complained:
> when you want to sweep an address. it's very unclear to me as a noob who is getting the money
> might want a little picture with an arrow to show which direction the funds are going
user on bitcointalk [0] tried to create wallet with name "w/o 2FA".
Before this, one would only get an error after the last page of the wizard.
With this, the "Next" button does not even get enabled if the name does not look ok.
(and as in comment, maybe we should be even stricter re what is allowed)
[0]: https://bitcointalk.org/index.php?topic=5483514.msg63584789#msg63584789
fixes https://github.com/spesmilo/electrum/issues/8837
Alternatively we could do a config upgrade (convert_version_*) to fix that specific issue,
but they are not safe against downgrade-upgrade-again, so probably not the best choice for
crash-inducing values.
And this kind of converter is a generic solution that can be useful for other configvars later
(though for most usages I expect we will also need a converter for the setter).
fixes https://github.com/spesmilo/electrum/issues/8813
regression from eef9680743
We started setting the witness field in above commit to be able to provide the witnesses for already pre-signed
external inputs to the device, e.g. for a coinjoin.
Trezor One fw has pretty strict limits on the witness field: max 109 bytes,
probably because that's a ~tight upper bound for a p2wpkh witness:
<num_witness_items> <len(sig)> <sig> <len(pubkey)> <pubkey>, it comes out to 3+73(high-S and high-R)+33.
ed1785a985/legacy/firmware/protob/messages-bitcoin.options (L35)
Trezor model T seems to have higher limits.
tx_inputs is called for the tx being signed (for_sig=True), and for its parents/prev_txes (for_sig=False).
The witness is only useful for the tx being signed, I think.
Users reported seeing a "DataError: bytes overflow" exception when using a Trezor One to sign 2of3 p2wsh multisig txs.
There were no external inputs involved so for the tx being signed all witnesses were None, however we were also
setting the witness for the inputs of prev_txes.
The witness for a 2of3 pw2sh multisig input is around ~253 bytes.
To sidestep the problem, we now only set the witness in the for_sig=True case.
Note that this means if someone tries to do a coinjoin with a Trezor One involving non-trivial external inputs,
they will run into the same limit and exception.
