- Wait until HTLCs are irrevocably removed before cleaning up their
data structures (MPP and forwarding)
- keep methods maybe_cleanup_mpp and maybe_cleanup_forwarding separate
- perform cleanup in htlc_switch, so that process_unfulfilled_htlc
has less side effects
- In htlc_switch, we blank the onion_packet_hex field to signal that
an HTLC has been processed. An item of chan.unfulfilled_htlcs may
go through 4 stages:
- 1. not forwarded yet: (None, onion_packet_hex)
- 2. forwarded: (forwarding_key, onion_packet_hex)
- 3. processed: (forwarding_key, None), not irrevocably removed yet
- 4. done: (forwarding_key, None), irrevocably removed
- in test_lnpeer, an extra iteration of htlc_switch has been added to
trampoline forwarding tests
If we accept a MPP and we forward the payment (trampoline or swap),
we need to persist the payment accepted status, or we might wrongly
release htlcs on the next restart.
lnworker.received_mpp_htlcs used to be cleaned up in maybe_cleanup_forwarding,
which only applies to forwarded payments. However, since we now
persist this dict, we need to clean it up also in the case of
payments received by us. This part of maybe_cleanup_forwarding has
been migrated to lnworker.maybe_cleanup_mpp
This will be useful if we decide to ship lntransport as a separate
package. It is also a conceptual cleanup.
Notes:
- lntransport still requires crypto.py
- parsing node id from a bolt11 invoice is not supported.
```
=============================== warnings summary ===============================
tests/test_wizard.py::ServerConnectWizardTestCase::test_no_advanced
tests/test_wizard.py::ServerConnectWizardTestCase::test_proxy
tests/test_wizard.py::ServerConnectWizardTestCase::test_proxy_and_server
tests/test_wizard.py::ServerConnectWizardTestCase::test_server
tests/test_wizard.py::WalletWizardTestCase::test_2fa
tests/test_wizard.py::WalletWizardTestCase::test_create_standard_wallet_haveseed_bip39
tests/test_wizard.py::WalletWizardTestCase::test_create_standard_wallet_haveseed_electrum
tests/test_wizard.py::WalletWizardTestCase::test_create_standard_wallet_newseed
/tmp/cirrus-ci-build/.tox/py3/lib/python3.10/site-packages/_pytest/threadexception.py:82: PytestUnhandledThreadExceptionWarning: Exception in thread Plugins
Traceback (most recent call last):
File "/usr/local/lib/python3.10/threading.py", line 1016, in _bootstrap_inner
self.run()
File "/tmp/cirrus-ci-build/electrum/plugin.py", line 360, in run
self.on_stop()
File "/tmp/cirrus-ci-build/electrum/util.py", line 430, in on_stop
loop = get_asyncio_loop()
File "/tmp/cirrus-ci-build/electrum/util.py", line 1578, in get_asyncio_loop
raise Exception("event loop not created yet")
Exception: event loop not created yet
warnings.warn(pytest.PytestUnhandledThreadExceptionWarning(msg))
```
This is most useful if the user wants to import a significant number of keys,
as every invocation of `importprivkey` results in rewriting the wallet file to disk.
The API of "importprivkey" is left unchanged for the single privkey case.
The history tab would show an incorrect feerate for partial/unsigned (local) txs,
if they had any p2sh/p2wsh txins. We would just guess the script is p2wpkh, and
use that for the size calc. Now with calling add_info_from_wallet, the correct
size is used to calculate the feerate.
(The gui tx dialogs call add_info_from_wallet independently, so the size/feerate
shown there were already correct.)
The messages are sometimes logged and sometimes shown to the user,
- for logging we might not want to truncate or have higher limits,
- but when shown to the user, we definitely want to truncate the error text.
It is simplest to just do the truncation here, at the lowest level.
Note that we usually prepend the error text with a header e.g. "[DO NOT TRUST THIS MESSAGE]"
and if the error text is too long, this header at the beginning might get "lost" in some way.
Hence we should truncate the error text.
```
./tests/test_mnemonic.py:249:9: B017 `assertRaises(Exception)` and `pytest.raises(Exception)` should be considered evil. They can lead to your test passing even if the code being tested is never executed due to a typo. Assert for a more specific exception (builtin or custom), or use `assertRaisesRegex` (if using `assertRaises`), or add the `match` keyword argument (if using `pytest.raises`), or use the context manager form with a target.
with self.assertRaises(Exception):
^
1 B017 `assertRaises(Exception)` and `pytest.raises(Exception)` should be considered evil. They can lead to your test passing even if the code being tested is never executed due to a typo. Assert for a more specific exception (builtin or custom), or use `assertRaisesRegex` (if using `assertRaises`), or add the `match` keyword argument (if using `pytest.raises`), or use the context manager form with a target.
```
Some functions have an argument named "seed_type" in which it was annoying to call the seed_type() fn.
(especially for functions inside the same module)
Add support for key-path-spending taproot utxos into transaction.py.
- no wallet support yet
- add some psbt, and minimal descriptor support
- preliminary work towards script-path spends
The low-S rule for ecdsa signatures is mandated by Bitcoin Core policy/standardness (though not by consensus). If we get signatures from untrusted sources, we should mandate they obey the policy rules. (e.g. from LN peers)
Note that we normalize the signatures in the sig format conversion methods (DER <-> (r,s) <-> sig64).
The BOLTs treat high-S signatures as invalid, and this changes our behaviour to that.
(previously we would silently normalize the S value)
see https://github.com/bitcoin/bitcoin/pull/6769
see https://github.com/lightning/bolts/pull/807
get_preimage_script should really have been private API...
looks like everywhere it is used outside of transaction.py, it is actually abused :/
Other existing usages in plugin code I don't dare to touch without lots of manual testing...
Instead of some functions operating with hex strings,
and others using bytes, this consolidates most things to use bytes.
This mainly focuses on bitcoin.py and transaction.py,
and then adapts the API usages in other files.
Notably,
- scripts,
- pubkeys,
- signatures
should be bytes in almost all places now.
- `bitstring` started depending on `bitarray` in version 4.1 [0]
- that would mean one additional dependency for us (from yet another maintainer), which is not even pure python
- we only use bitstring for bolt11-parsing
- hence this PR rewrites the bolt11-parsing and removes `bitstring` as dependency
- note: I benchmarked lndecode using [1], and the new code performs better,
taking around 80% time needed for old code (when using bitstring 3.1.9, pure python).
Though the variance is quite large in both cases.
[0]: 95ee533ee4/release_notes.txt (L108)
[1]: d7597d96d0
I decided to use the stdlib (hashlib) instead of libsecp for this,
as it is simple enough, and the former is faster on my PC.
Added a unit test that compares the two.
The transaction dict can also contain PSBTs (in addition to complete raw hex txs).
This is the case if the user has saved a partial (e.g. unsigned) tx as local into the history.
fixes https://github.com/spesmilo/electrum/issues/8913
ThomasV
Sander van Grieken
SomberNight
shangchenglumetro
Ondřej Vejpustek
momantech