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)
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).
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.
- risk_of_burning_coins_as_fees is turned into a private (helper) method, only called by check_sighash.
UIs should only care about check_sighash.
- check_sighash returns instance of new class "TxSighashDanger" instead of tuple
- made warning levels more fine-grained (FEE_WARNING_SKIPCONFIRM vs FEE_WARNING_NEEDCONFIRM)
- this became more complicated than I had hoped for but I think it is worth it to ~merge
check_sighash and risk_of_burning_coins_as_fees into one.
related https://github.com/spesmilo/electrum/pull/8699
qml: use backend sighash check and add user confirmation path
qt: use backend sighash check and add user confirmation path
qml: include get_warning_for_risk_of_burning_coins_as_fees test in txdetails
qt: add warning icon to sighash warning
add sighash and fee checks to wallet.sign_transaction, making all warnings fatal unless ignore_warnings is set to True
tests: test sign_transaction on both code paths with ignore_warnings True and False,
raise specific exceptions TransactionPotentiallyDangerousException and TransactionDangerousException
gui/qt/rbf_dialog.py (old) lines 57-64 were implementing logic that should not be part of GUI code.
Case in point, gui/qml/qetxfinalizer.py (old) lines 511-513 duplicated half of that logic but not the other half.
That logic is now moved to wallet.get_bumpfee_strategies_for_tx().
More context: a user on irc got confused when using the qml gui. They were sending "max" and wanted to bump_fee.
The qml gui selected the "preserve_payment" strategy by default, using which there was no solution, and the user
did not notice that the strategy can be altered (via the "method" dropdown). The qt gui had logic to select
"decrease_payment" by default in such a case (which does find a solution to bump) but this logic was not
duplicated in the qml gui.
Instead of duplicating the logic, this commit moves it to shared lib code.
- all forwarding types use the same flow
- forwarding callback returns a htlc_key or None
- forwarding info is persisted in lnworker:
- ongoing_forwardings
- downstream to upstream htlc_key
- htlc_key -> error_bytes
- a node scid alias is derived from the node ID
- the channel opening fee is sent in a TLV field of open_channel
- the server requires htlc settlement before broadcasting
(server does not trust client)
This is a regression from 7ca89f56ee, which introduced StoredList.
The newly added test was failing without the change.
```
Traceback (most recent call last):
File "/home/user/wspace/electrum/electrum/gui/qt/main_window.py", line 1786, in new_contact_dialog
self.set_contact(line2.text(), line1.text())
File "/home/user/wspace/electrum/electrum/gui/qt/main_window.py", line 1435, in set_contact
self.contacts[address] = ('address', label)
File "/home/user/wspace/electrum/electrum/contacts.py", line 75, in __setitem__
self.save()
File "/home/user/wspace/electrum/electrum/contacts.py", line 62, in save
self.db.put('contacts', dict(self))
File "/home/user/wspace/electrum/electrum/json_db.py", line 42, in wrapper
return func(self, *args, **kwargs)
File "/home/user/wspace/electrum/electrum/json_db.py", line 318, in put
self.data[key] = copy.deepcopy(value)
File "/usr/lib/python3.10/copy.py", line 146, in deepcopy
y = copier(x, memo)
File "/usr/lib/python3.10/copy.py", line 231, in _deepcopy_dict
y[deepcopy(key, memo)] = deepcopy(value, memo)
File "/usr/lib/python3.10/copy.py", line 172, in deepcopy
y = _reconstruct(x, memo, *rv)
File "/usr/lib/python3.10/copy.py", line 271, in _reconstruct
state = deepcopy(state, memo)
File "/usr/lib/python3.10/copy.py", line 146, in deepcopy
y = copier(x, memo)
File "/usr/lib/python3.10/copy.py", line 231, in _deepcopy_dict
y[deepcopy(key, memo)] = deepcopy(value, memo)
File "/usr/lib/python3.10/copy.py", line 172, in deepcopy
y = _reconstruct(x, memo, *rv)
File "/usr/lib/python3.10/copy.py", line 271, in _reconstruct
state = deepcopy(state, memo)
File "/usr/lib/python3.10/copy.py", line 146, in deepcopy
y = copier(x, memo)
File "/usr/lib/python3.10/copy.py", line 231, in _deepcopy_dict
y[deepcopy(key, memo)] = deepcopy(value, memo)
File "/usr/lib/python3.10/copy.py", line 161, in deepcopy
rv = reductor(4)
TypeError: cannot pickle '_thread.RLock' object
```
The wizard should technically disallows this at creation time,
but this second layer sanity check could not hurt.
Also, looks like the wizard check is not working properly atm
(regression from qt wizard refactor).
- introduce PaymentFeeBudget, which contains limits for fee budget and cltv budget
- when splitting a payment,
- the fee budget is linearly distributed between the parts
- this resolves a FIXME in lnrouter ("FIXME in case of MPP")
- the cltv budget is simply copied
- we could also add other kinds of budgets later, e.g. for the num in-flight htlcs
- resolves TODO in lnworker ("todo: compare to the fee of the actual route we found")
- Refactor _run_mpp so that it takes only one set of arguments.
Success and failure conditions are now tested by calling
_run_mpp multiple times.
- In test_payment_multipart with_timeout, check that the received
failure message actually is MPP_TIMEOUT, and not a generic
failure. Since the onion is obfuscated by the forwarding node,
this tests that obfuscate_onion_packet and decode_onion_packet
work as expected.
