Adds the invoice features to the `PaymentInfo` class so we can check if
the sender respects our requested features (e.g. if they tried to send
mpp if we requested no mpp).
Allows storing two different payment info of the same payment hash by
including the direction into the db key.
We create and store PaymentInfo for sending attempts and for requests (receiving),
if we try to pay ourself (e.g. through a channel rebalance) the checks
in `save_payment_info` would prevent this and throw an exception.
By storing the PaymentInfos of outgoing and incoming payments separately in
the db this collision is avoided and it makes it easier to reason about
which PaymentInfo belongs where.
Splits `LNWallet.dont_settle_htlcs` into `LNWallet.dont_settle_htlcs`
and `LNWallet.dont_expire_htlcs`.
Registering a payment hash in dont_settle_htlcs will prevent it from
getting fulfilled if we have the preimage stored. The preimage will not
be released before the the payment hash gets removed from
dont_settle_htlcs. Htlcs can still get expired as usual or failed if no
preimage is known.
This is only used by Just-in-time channel openings.
Registering a payment hash in dont_expire_htlcs allows to overwrite the
minimum final cltv delta value after which htlcs would usually get
expired. This allows to delay expiry of htlcs or, if the value in the
dont_settle_htlcs dict is None, completely prevent expiry and let the
htlc get expired onchain.
Splitting this up in two different dicts makes it more explicit and
easier to reason about what they are actually doing.
Please enter the commit message for your changes. Lines starting
Adds test_payment_bundle_with_hold_invoice to simulate the use of a
payment bundle in which one invoice of the bundle needs to trigger a hold invoice
callback (similar to submarine swaps).
Also modifies the test helper _test_simple_payment() to compare the
results of all payment attempts instead of just returning after the
first (of multiple) payments raises its result causing the test to miss
if all payments were successful or not.
Add test `test_hold_invoice_set_doesnt_get_expired` to test_lnpeer to
ensure a mpp set on which a hold invoice callback doesn't get expired
automatically if the cltv_abs falls below MIN_FINAL_CLTV_DELTA_ACCEPTED
as these sets should only get failed if the htlcs are safe to fail by
the target of the hold invoice callback (e.g. swap got refunded
successfully).
Adds test_forwarder_fails_for_inconsistent_trampoline_onions
which checks that a forwarder compares the trampoline onions of a mpp
set and fails the set if the onions are not similar.
In the test alice sends a mpp through bob with 2 htlcs, in one
trampoline onion amt_to_forward is off by 1 msat so bob fails the htlc
set instead of initiating the trampoline forwarding.
Add sanity check that bob is not forwarding more sats to carol if than
he receives from alice. (he only forwards once and doesn't try to
forward multiple times).
This should get caught by asserts in lnworker/lnpeer, nevertheless it
seems to make sense to just add this test to prevent regressions of this
kind.
1. Alice sends two HTLCs to Bob, not reaching total_msat,
and eventually they MPP_TIMEOUT
2. Bob fails both HTLCs
3. Alice then retries and sends HTLCs again to Bob, for the same RHASH,
this time reaching total_msat, and the payment succeeds
Test that the sets are properly cleaned up after MPP_TIMEOUT
and the sender gets a second chance to pay the same invoice.
Add `test_reject_invalid_min_final_cltv_delta` which is supposed to test
that the peer rejects incoming htlcs with final cltv delta differing
from what has been requested in the lightning invoice.
refactor `htlc_switch` to new architecture to make it more robust
against partial settlement of htlc sets and increase maintainability.
Htlcs are now processed in two steps, first the htlcs are collected into
sets from the channels, and potentially failed on their own already.
Then a second loop iterates over the htlc sets and finalizes only on
whole sets.
# Conflicts:
# electrum/lnpeer.py
Add unittest to TestPeerForwarding which sends a multi trampoline
payment.
Wait another htlc_switch iteration in tests because trampolines might have different delays
There is a race when initiating multiple lightning payments concurrently
(e.g. when doing a reverse swap with prepayment + swap payment).
suggest_splits might overallocate
split amounts for a channel as the splitting of both invoice amounts runs
concurrently and before acutal htlcs that reduce the channels balance
have been added to the channel yet. This results in a "not
enough balance" PaymentFailure once we try to send the htlcs and the
other payment attempt already reduced the available balance of the
channel.
This fix takes a lock from splitting the amount until the htlcs are
put on the channel, so suggest_splits always acts on the correct channel
balance.
Enforce that the information used to create a bolt11 invoice using
`get_bolt11_invoice()` is similar to the related instance of PaymentInfo
by requiring a PaymentInfo as argument for `get_bolt11_invoice()`.
This way the invoice cannot differ from the created PaymentInfo.
This allows to use the information in PaymentInfo for validation of
incoming htlcs more reliably.
To cover all required information for the creation of a b11 invoice the
PaymentInfo class has to be extended with a expiry and
min_final_cltv_expiry. This requires a db upgrade.
util.callback_mgr.callbacks was not getting properly cleared between tests.
Every time an Abstract_Wallet or an LNWorker (or many other subclasses of EventListener) is instantiated,
self.register_callbacks() is called in __init__, which puts callbacks into util.callback_mgr.callbacks.
These are only cleaned up if we explicitly call Abstract_Wallet.stop() or LNWorker.stop() later, which we usually do not do in the tests.
As a result, when running multiple unit tests in a row, lots of objects created in a given testcase are never GC-ed and leak into subsequent tests. This is not only a memory leak, but wastes compute too: when events are triggered and cbs get called, these old objects also have their cbs called.
After running all (~1061) unit tests, I observe util.callback_mgr.callbacks had 30 events with a total of 3156 callbacks stored.
On my laptop, running all unit tests previously took ~115 sec, and now it takes ~73 sec.
part configs
I noticed certain ln payments become very unreliable. These payments are ~21k sat, from gossip to gossip sender, with direct, unannounced channel.
Due to the recent fix https://github.com/spesmilo/electrum/pull/9723 `LNPathFinder.get_shortest_path_hops()` will not use channels for the last hop of a route anymore that aren't also passed to it in `my_sending_channels`:
```python
if edge_startnode == nodeA and my_sending_channels: # payment outgoing, on our channel
if edge_channel_id not in my_sending_channels:
continue
```
Conceptually this makes sense as we only want to send through `my_sending_channels`, however if the only channel between us and the receiver is a direct channel that we got from the r_tag and it's not passed in `my_sending_channel` it's not able to construct a route now.
Previously this type of payment worked as `get_shortest_path_hops()` knew of the direct channel between us and `nodeB` from the invoices r_tag and then just used this channel as the route, even though it was (often) not contained in `my_sending_channels`.
`my_sending_channels`, passed in `LNWallet.create_routes_for_payment()` is either a single channel or all our channels, depending on `is_multichan_mpp`:
```python
for sc in split_configurations:
is_multichan_mpp = len(sc.config.items()) > 1
```
This causes the unreliable, random behavior as `LNWallet.suggest_splits()` is supposed to `exclude_single_part_payments` if the amount is > `MPP_SPLIT_PART_MINAMT_SAT` (5000 sat).
As `mpp_split.py suggest_splits()` is selecting channels randomly, and then removes single part configs, it sometimes doesn't return a single configuration, as it removes single part splits, and also removes multi part splits if a part is below 10 000 sat:
```python
if target_parts > 1 and config.is_any_amount_smaller_than_min_part_size():
continue
```
This will result in a fallback to allow single part payments:
```python
split_configurations = get_splits()
if not split_configurations and exclude_single_part_payments:
exclude_single_part_payments = False
split_configurations = get_splits()
```
Then the payment works as all our channels are passed as `my_sending_channels` to `LNWallet.create_routes_for_payment()`.
However sometimes this fallback doesn't happen, because a few mpp configurations found in the first iteration of `suggest_splits` have been kept, e.g. [10500, 10500], but at the same time most others have been removed as they crossed the limit, e.g. [11001, 9999], (which happens sometimes with payments ~20k sat), this makes `suggest_splits` return very few usable channels/configurations (sometimes just one or two, even with way more available channels).
This makes payments in this range unreliable as we do not retry to generate new split configurations if the following path finding fails with `NoPathFound()`, and there is no single part configuration that allows the path finding to access all channels. Also this does not only affect direct channel payments, but all gossip payments in this amount range.
There seem to be multiple ways to fix this, i think one simple approach is to just disable `exclude_single_part_payments` if the splitting loop already begins to sort out configs on the second iteration (the first split), as this indicates that the amount may be too small to split within the given limits, and prevents the issue of having only few valid splits returned and not going into the fallback. However this also results in increased usage of single part payments.
the update_fee logic for lightning channels was not adapted to anchor
channels causing us to send update_fee with a eta target of 2 blocks.
This causes force closes when there are mempool spikes as the fees we
try to update to are a lot higher than e.g. eclair uses. Eclair will
force close if our fee is 10x > than their fee.
- Wallet.make_unsigned_transaction takes a FeePolicy parameter
- fee sliders act on a FeePolicy instead of config
- different fee policies may be used for different purposes
- do not detect dust outputs in lnsweep, delegate that to lnwatcher
Adds a new test case for e2e trampoline payment, where there are multiple
Trampoline Forwarders which themselves are not directly connected.
This adds a regression test for https://github.com/spesmilo/electrum/pull/9431dffdebf778
