triggered a payment forwarding.
Final onions may trigger a payment forwarding, through the callback
returned by maybe_fulfill_htlc. In that case, we should not fail the
HTLC later; doing so might result in fund loss.
Remove test_simple_payment_with_hold_invoice_timing_out: once we
have accepted to forward a payment HTLC with a hold invoice, we
do not want to time it out, for the same reason.
- maybe_fulfill_htlc returns a forwarding callback that
covers both cases.
- previously, the callback of hold invoices was called as a
side-effect of lnworker.check_mpp_status.
- the same data structures (lnworker.trampoline_forwardings,
lnworker.trampoline_forwarding_errors) are used for both
trampoline forwardings and hold invoices.
- maybe_fulfill_htlc still recursively calls itself to perform
checks on trampoline onion. This is ugly, but ugliness is now
contained to that method.
- fix parameters passed to maybe_forward_trampoline
- use lnworker.trampoline_forwardings as a semaphore for ongoing
trampoline payments
- if a trampoline payment fails, fail all received HTLCs
This clears up log spam for regtest tests.
related:
- https://bugs.python.org/issue44665
- https://github.com/python/cpython/issues/88831
- https://textual.textualize.io/blog/2023/02/11/the-heisenbug-lurking-in-your-async-code/
- https://github.com/python/cpython/issues/91887#issuecomment-1434816045
- "Task was destroyed but it is pending!"
Perhaps we should inspect all our usages of
- asyncio.create_task
- loop.create_task
- asyncio.ensure_future
- asyncio.run_coroutine_threadsafe
?
Example log for running a regtest test:
```
$ python3 -m unittest electrum.tests.regtest.TestLightningAB.test_collaborative_close
***** test_collaborative_close ******
initializing alice
--- Logging error ---
Traceback (most recent call last):
File "/usr/lib/python3.10/logging/__init__.py", line 1100, in emit
msg = self.format(record)
File "/usr/lib/python3.10/logging/__init__.py", line 943, in format
return fmt.format(record)
File "/home/user/wspace/electrum/electrum/logging.py", line 44, in format
record = copy.copy(record) # avoid mutating arg
File "/usr/lib/python3.10/copy.py", line 92, in copy
rv = reductor(4)
ImportError: sys.meta_path is None, Python is likely shutting down
Call stack:
File "/usr/lib/python3.10/asyncio/base_events.py", line 1781, in call_exception_handler
self._exception_handler(self, context)
File "/home/user/wspace/electrum/electrum/util.py", line 1535, in on_exception
loop.default_exception_handler(context)
File "/usr/lib/python3.10/asyncio/base_events.py", line 1744, in default_exception_handler
logger.error('\n'.join(log_lines), exc_info=exc_info)
Message: "Task was destroyed but it is pending!\ntask: <Task pending name='Task-2' coro=<Abstract_Wallet.on_event_adb_set_up_to_date() running at /home/user/wspace/electrum/electrum/wallet.py:485> wait_for=<Future finished result=0> cb=[_chain_future.<locals>._call_set_state() at /usr/lib/python3.10/asyncio/futures.py:392]>"
Arguments: ()
[--- SNIP --- more of the same --- SNIP ---]
--- Logging error ---
Traceback (most recent call last):
File "/usr/lib/python3.10/logging/__init__.py", line 1100, in emit
msg = self.format(record)
File "/usr/lib/python3.10/logging/__init__.py", line 943, in format
return fmt.format(record)
File "/home/user/wspace/electrum/electrum/logging.py", line 44, in format
record = copy.copy(record) # avoid mutating arg
File "/usr/lib/python3.10/copy.py", line 92, in copy
rv = reductor(4)
ImportError: sys.meta_path is None, Python is likely shutting down
Call stack:
File "/usr/lib/python3.10/asyncio/base_events.py", line 1781, in call_exception_handler
self._exception_handler(self, context)
File "/home/user/wspace/electrum/electrum/util.py", line 1535, in on_exception
loop.default_exception_handler(context)
File "/usr/lib/python3.10/asyncio/base_events.py", line 1744, in default_exception_handler
logger.error('\n'.join(log_lines), exc_info=exc_info)
Message: "Task was destroyed but it is pending!\ntask: <Task pending name='Task-31' coro=<Abstract_Wallet.on_event_adb_set_up_to_date() running at /home/user/wspace/electrum/electrum/wallet.py:485> wait_for=<Future pending cb=[_chain_future.<locals>._call_check_cancel() at /usr/lib/python3.10/asyncio/futures.py:385, Task.task_wakeup()]> cb=[_chain_future.<locals>._call_set_state() at /usr/lib/python3.10/asyncio/futures.py:392]>"
Arguments: ()
true
true
true
true
funding alice
```
scenario:
- user opens a lightning channel and exports an "imported channel backup"
- user closes channel via local-force-close
- local ctx is published, to_local output has user's funds and they are CSV-locked for days
- user restores wallet file from seed and imports channel backup
- new wallet file should be able to sweep coins from to_local output (after CSV expires)
This was not working previously, as the local_payment_basepoint was not included in the
imported channel backups, and the code was interpreting the lack of this as the channel not
having option_static_remotekey enabled. This resulted in lnutil.extract_ctn_from_tx
using an incorrect funder_payment_basepoint, and lnsweep not recognising the ctx due to
the garbage ctn value.
The imported channel backup serialisation format is slightly changed to include the
previously missing field, and its version number is bumped (0->1). We allow importing
both version 0 and version 1 backups, however v0 backups cannot handle the above
described scenario (they can only be used to request a remote-force-close).
Note that we were/are setting the missing local_payment_basepoint to the pubkey of
one of the wallet change addresses, which is bruteforceable if necessary, but I
think it is not worth the complexity to add this bruteforce logic. Also note
that the bruteforcing could only be done after the local-force-close was broadcast.
Ideally people with existing channels and already exported v0 backups should re-export
v1 backups... Not sure how to handle this.
closes https://github.com/spesmilo/electrum/issues/8516
Was getting error:
```
>>> import copy
>>> from electrum.simple_config import SimpleConfig
>>> copy.deepcopy(SimpleConfig.EXPERIMENTAL_LN_FORWARD_PAYMENTS)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "...\Python\Python310\lib\copy.py", line 161, in deepcopy
rv = reductor(4)
TypeError: cannot pickle 'ConfigVar' object
```
This is useful in tests/test_lnpeer.py, to deepcopy the GRAPH_DEFINITIONS dict.
```
229.18 | E | gui.qt.main_window.[test_segwit_2] | on_error
Traceback (most recent call last):
File "...\electrum\gui\qt\util.py", line 917, in run
result = task.task()
File "...\electrum\gui\qt\send_tab.py", line 681, in broadcast_thread
if self.payto_e.payment_identifier.has_expired():
AttributeError: 'NoneType' object has no attribute 'has_expired'
```
In SendTab.broadcast_transaction.broadcast_thread, self.payto_e.payment_identifier was referenced -
but do_clear() has already cleared it by then.
E.g. consider SendTab.pay_onchain_dialog: it calls save_pending_invoice(), which calls do_clear(),
and later (in sign_done), it calls window.broadcast_or_show, which will call SendTab.broadcast_transaction().
As there might be multiple independent transaction dialogs open simultaneously, the single shared state
send_tab.payto_e.payment_identifier approach was problematic -- I think it is conceptually nicer to
pass around the payment_identifiers as needed, as done with this change.
However, this change is not a full proper fix, as it still somewhat relies on
send_tab.payto_e.payment_identifier (e.g. in pay_onchain_dialog). Hence, e.g. when using
the invoice_list context menu "Pay..." item, as payto_e.payment_identifier is not set,
payment_identifier will be None in broadcast_transaction.
but at least we handle PI being None gracefully -- before this change, broadcast_transaction
expected PI to be set, and it was never set to the correct thing (as do_clear() already ran by then):
depending on timing it was either None or a new empty PI. In the former case, producing the above
traceback and hard failing (not only for bip70 stuff!), and in the latter, silently ignoring the logic bug.
regression from https://github.com/spesmilo/electrum/pull/8462
- pr.verify() was called in qml, but not in qt gui
- we now call pr.verify() in get_payment_request(), to make the API less error-prone
- it is now ok to call pr.verify() multiple times, the result is cached
