- rename trampoline_forwardings -> final_onion_forwardings,
because this dict is used for both trampoline and hold invoices
- remove timeout from hold_invoice_callbacks (redundant with invoice)
- add test_failure boolean parameter to TestPeer._test_simple_payment,
in order to test correct propagation of OnionRoutingFailures.
- maybe_fulfill_htlc: raise an OnionRoutingFailure if we do not have
the preimage for a payment that does not have a hold invoice callback.
Without this, the above unit tests stall when we use test_failure=True
- introduce SentHtlcInfo named tuple
- some previously unnamed tuples are now much shorter:
create_routes_for_payment no longer returns an 8-tuple!
- sent_htlcs_q (renamed from sent_htlcs), is now keyed on payment_hash+payment_secret
(needed for proper trampoline forwarding)
- add RecvMPPResolution enum for possible states of a pending incoming MPP,
and use it in check_mpp_status
- new state: "FAILED", to allow nicely failing back the whole MPP set
- key more things with payment_hash+payment_secret, for consistency
(just payment_hash is insufficient for trampoline forwarding)
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
(invoices for which we do not have the preimage)
Callbacks and timeouts are registered with lnworker. If the
preimage is not known after the timeout has expired, the payment
is failed with MPP_TIMEOUT.
A new config API is introduced, and ~all of the codebase is adapted to it.
The old API is kept but mainly only for dynamic usage where its extra flexibility is needed.
Using examples, the old config API looked this:
```
>>> config.get("request_expiry", 86400)
604800
>>> config.set_key("request_expiry", 86400)
>>>
```
The new config API instead:
```
>>> config.WALLET_PAYREQ_EXPIRY_SECONDS
604800
>>> config.WALLET_PAYREQ_EXPIRY_SECONDS = 86400
>>>
```
The old API operated on arbitrary string keys, the new one uses
a static ~enum-like list of variables.
With the new API:
- there is a single centralised list of config variables, as opposed to
these being scattered all over
- no more duplication of default values (in the getters)
- there is now some (minimal for now) type-validation/conversion for
the config values
closes https://github.com/spesmilo/electrum/pull/5640
closes https://github.com/spesmilo/electrum/pull/5649
Note: there is yet a third API added here, for certain niche/abstract use-cases,
where we need a reference to the config variable itself.
It should only be used when needed:
```
>>> var = config.cv.WALLET_PAYREQ_EXPIRY_SECONDS
>>> var
<ConfigVarWithConfig key='request_expiry'>
>>> var.get()
604800
>>> var.set(3600)
>>> var.get_default_value()
86400
>>> var.is_set()
True
>>> var.is_modifiable()
True
```
Inheritance was overkill here, and now we can use inheritance for new functionality X
without having to create classes for all combinations of {X, is_testnet}.
- save remote alias for use in invoices
- derive local alias from wallet xpub
- send channel_type without the option_scid_alias bit
(apparently LND does not like it)
Upon receiving UNKNOWN_NEXT_PEER, TEMPORARY_NODE_FAILURE
or TEMPORARY_CHANNEL_FAILURE, remember the trampoline route
that was used, and try other routes before raising the
trampoline fee.
Before this commit, we used to raise the trampoline fee
upon receiving any error message, in order to ensure
termination of the payment loop.
Note that we will still retry failing routes after we have
raised the trampoline fee. This choice is questionable, it
is unclear if doing so significantly increases the probability
of success.
Tests: add a test for trampoline handling of UNKNOWN_NEXT_PEER
get_unused_addresses() has been broken since #7730, because
addresses are considered as permanently used if they are in
the list of keys of receive_requests. This is true even if
an address is used as fallback for a lightning payment. This
means that the number of lightning payments we can receive
is constrained by the gap limit.
If a payment succeeds off-chain, we want to be able to reuse
its fallback address in other requests (this does not reduce
privacy, because invoices already share the same public key).
This implies that we should not use the onchain address as key
for lightning-enabled requests in wallet.receive_requests. If
we did, paid invoices would be overwritten when the address is
reused. That is the reason for the wallet_db upgrade.
Related: a3faf85e3c
- add new index: requests_rhash_to_key (fixes#7845)
- when creating a request, do not save its description in labels.
Instead, return it as default value in wallet.get_label_by_rhash
lnworker:
- rename 'payments' to 'payment_info'
- add note to delete_payment_info
commands: rename 'rmrequest' to 'delete_request'
- separate AddressSynchronizer from Wallet and LNWatcher
- the AddressSynchronizer class is referred to as 'adb' (address database)
- Use callbacks to replace overloaded methods
When replaying messages during channel-reestablishment,
previously we first resent all update messages, along with potential commitment_signed messages,
and then we potentially resent a single revoke_and_ack.
This can result in incorrect behaviour in case both a commitment_signed and a revoke_and_ack needs to be resent.
When replaying messages, the relative order of commitment_signed and revoke_and_messages needs to be preserved.
(the order of updates (htlc/fee) in relation to the revack messages does not matter)
implements https://github.com/lightning/bolts/pull/810
The logic here is somewhat based on what c-lightning does:
01e5f1886e/channeld/channeld.c (L3059)
These methods return a list of channels that can be rebalanced,
in order to receive or send a given amount.
Also add 'channels' parameter to submarine swaps.
Previously, swaps were not considering which channel to use.
When we do not have liquidity to pay an invoice:
- add 'rebalance' option in order to pay an invoice
- use the suggested channel in the 'swap' option
When we do not have the liquidity to receive an invoice:
- add 'Rebalance' and 'Swap' buttons to the receive tab
follow-up https://github.com/spesmilo/electrum/pull/7818
- note it matters whether a sender pays us end-to-end-trampoline or just via legacy
- consider: Alice has 0.1 BTC recv cap in chan1 and 1 BTC recv cap in chan2, both with border-node T1
- if sender is paying e2e trampoline, it can realistically pay even ~1.1 BTC, as T1 can resplit the HTLCs
- if sender is paying legacy, it will have a hard time trying to pay more than 1 BTC, in practice
- although note if T1 has implemented non-strict-forwarding (see BOLT-04), achieving 1 BTC is easy,
as T1 can redirect HTLCs (but cannot split them, in this case)
- to make num_sats_can_receive realistic, it assumes the legacy case
- To calc num_sats_can_receive, we sort our channels in decreasing order of receive-capacities, iterate over them
and calculate a running sum - we stop adding channels when the next chan's recv cap is small compared to
the running total.
- When putting routing hints in an invoice, we do the same, with the added condition that we keep adding channels
if their recv cap is larger than the invoice amount.
- consider: Alice has 0.1 BTC recv cap in chan1 with Bob, and 1 BTC recv cap in chan2 with Carol
- if Alice wants to recv 100 sats, it is useful to add hints for both channels into the invoice, for redundancy
- if Alice wants to recv 0.9 BTC, it is questionable whether adding the smaller chan is useful - the code here won't add it
- increase MPP_RECEIVE_CUTOFF from 5 to 20 percent
- filter channels by node_id, not channel_id
- make num_sats_can_receive consistent with routing hints
