Commit Graph

222 Commits

Author SHA1 Message Date
ThomasV
1cc92d4890 trampoline forwarding: before failing payment, wait until all htcs
have failed and session is not longer active.
2023-11-18 16:08:11 +01:00
ThomasV
9b1c40e396 Refactor payment forwarding:
- 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
2023-11-18 16:03:18 +01:00
SomberNight
6506abf583 lnworker: use PaymentFeeBudget
- 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")
2023-10-27 16:01:23 +00:00
ThomasV
4a6c55ef7b test_lnpeer: add more tests, combining forwarding and hold invoices 2023-10-27 16:09:56 +02:00
ThomasV
b0dbdfd5e4 test_lnpeer: split some tests, remove redundant test 2023-10-23 13:02:02 +02:00
ThomasV
f8781364d2 test_lnpeer:
- 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.
2023-10-20 11:36:50 +02:00
SomberNight
22a8348303 renames: use consistent naming of cltv delta vs cltv abs
to avoid confusing relative vs absolute cltvs
(see b0401a6386)
2023-10-19 16:40:05 +00:00
ThomasV
b645da6514 test_lnpeer: separate graph creation from run_trampoline 2023-10-19 18:32:04 +02:00
ThomasV
7ea2e68507 follow-up 4c42840c1c 2023-10-19 16:05:54 +02:00
SomberNight
04d8eec3be tests: change fake block height in test_lnpeer. 0 might hide bugs. 2023-10-18 17:09:53 +00:00
SomberNight
30c863d32c lnaddr: don't call validate_features in parser
- see comment in lnaddr.py
- Previously we used feature bit 50/51 for trampoline.
  The spec subsequently defined fbit 50/51 as option_zeroconf, which
  requires fbit 46/47 (option_scid_alias) to also be set.
  We moved the non-standard trampoline fbit to a different int.
  However, old wallets might have old invoices saved that set fbit 50/51
  for trampoline, and those would not have the dependent bit set.
  Invoices are parsed at wallet-open, so if the parser ran these checks,
  those wallets could not be opened.
- note: we could potentially also run lnaddr.validate_and_compare_features
  when saving new invoices into the wallet but this is not done here
2023-10-16 17:10:49 +00:00
ThomasV
89023cc123 Remove trampoline hints from invoices
This has been deprecated for a long time, was kept only for
compatibility with old electrum clients.
2023-10-11 09:47:36 +02:00
ThomasV
2f8325ca09 follow-up 455167136d (variable rename) 2023-10-09 12:31:23 +02:00
ThomasV
4e76ed6952 move INITIAL_TRAMPOLINE_FEE_LEVEL to config 2023-10-08 15:06:19 +02:00
ThomasV
4bfd3b2f46 test_lnpeer:
- do not use needs_test_with_all_chacha20_implementation,
   this is slow and not really useful here.
 - split TestPeer class in two classes, depending on the type
   of graph we use.
2023-09-26 10:44:43 +02:00
ThomasV
e206d264c8 trampoline forwarding: use routing hints
unit tests:
 - remove 'drop_dave' flag, replace it by depleted channel
 - add test for trampoline forwarding using routing hints
 - lower attempts to 2
2023-09-26 10:07:41 +02:00
SomberNight
139eb632d7 (trivial) follow-up paysession.use_two_trampolines arg change
follow-up b2053c68f1

- use_two_trampolines is already passed as arg to init
- strings in gui/messages.py should not end with a newline
2023-09-12 14:05:12 +00:00
SomberNight
6819f685d7 tests: add tests for "recv mpp confusion" bug
see https://github.com/spesmilo/electrum/security/advisories/GHSA-8r85-vp7r-hjxf
2023-09-12 13:58:57 +00:00
ThomasV
136978e9d0 submarine swaps: fail received HTLCs of normal swap htlcs if
the swap is still unfunded and the refund delay has expired.
2023-09-08 16:38:08 +02:00
ThomasV
b2053c68f1 make use_to_trampolines a user visible option, disable it by default 2023-08-27 11:43:17 +02:00
SomberNight
98bda60c01 lnworker: move sent_buckets into PaySession 2023-08-11 22:01:19 +00:00
SomberNight
00e88c4e50 lnworker: introduce PaySession cls, refactor pay_to_node 2023-08-11 22:01:16 +00:00
SomberNight
98bea49a3c lnworker.pay_to_node: make trampoline fee_level and failed_routes local
multiple instances of pay_to_node might run concurrently, esp for trampoline forwarding
2023-08-11 22:01:11 +00:00
SomberNight
35c9ac8f31 lnworker: MPP send: more aggressively split large htlcs
related: https://github.com/spesmilo/electrum/issues/7987#issuecomment-1670002482
2023-08-11 22:01:08 +00:00
ThomasV
bf86cd6761 lnpeer and lnworker cleanup:
- 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
2023-08-09 13:23:26 +02:00
SomberNight
afac158c80 lnworker: clean-up sent_htlcs_q and sent_htlcs_info
- 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)
2023-08-08 16:37:50 +00:00
SomberNight
44bdd20ccc lnworker: add RecvMPPResolution with "FAILED" state
- 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)
2023-08-08 16:37:46 +00:00
SomberNight
c527ef8967 lnpeer: refuse to forward htlcs that correspond to payreq we created 2023-08-07 18:57:04 +00:00
SomberNight
d51f00e2a3 asyncio.wait_for() is too buggy. use util.wait_for2() instead
wasted some time because asyncio.wait_for() was suppressing cancellations. [0][1][2]
deja vu... [3]

Looks like this is finally getting fixed in cpython 3.12 [4]
So far away...
In attempt to avoid encountering this again, let's try using
asyncio.timeout in 3.11, which is how upstream reimplemented wait_for in 3.12 [4], and
aiorpcx.timeout_after in 3.8-3.10.

[0] https://github.com/python/cpython/issues/86296
[1] https://bugs.python.org/issue42130
[2] https://bugs.python.org/issue45098
[3] https://github.com/kyuupichan/aiorpcX/issues/44
[4] https://github.com/python/cpython/pull/98518
2023-08-04 18:18:21 +00:00
ThomasV
141cd524bc lnpeer: do not run maybe_fulfill_htlc more than once, if it
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.
2023-07-24 12:44:56 +02:00
ThomasV
017186d107 Refactor trampoline forwarding and hold invoices.
- 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.
2023-07-21 13:40:10 +02:00
ThomasV
e124ff7ee7 Trampoline MPP consolidation:
- 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
2023-07-19 10:48:44 +02:00
ThomasV
aeaf9c71df Add unit test for trampoline MPP consolidation
This tests that a trampoline waits until all incoming HTLCs are
received, and fail or succeed them together
2023-07-19 09:49:05 +02:00
ThomasV
1e3b3b82d5 test_lnpeer: deepcopy graph definitions in test setup 2023-07-19 09:43:18 +02:00
SomberNight
fc6486ecdb lnaddr: make payment_secret field mandatory, in both directions
we now require payment_secret both for sending and for receiving
(previously was optional for both)

see
https://github.com/lightning/bolts/pull/898
https://github.com/ACINQ/eclair/pull/1810
https://github.com/ElementsProject/lightning/pull/4646

note: payment_secret depends on var_onion_optin, so that becomes mandatory as well,
however this commit does not yet remove the ability of creating legacy onions
2023-06-29 14:34:02 +00:00
ThomasV
7caa6ccf57 test_lnpeer: fix variable names after rename 2023-06-28 13:22:42 +02:00
ThomasV
c4eb7d8321 lnworker: bundled payments
- htlcs of bundled payments must arrive in the same MPP_TIMEOUT
window, or they will be failed
- add correspoding tests
2023-06-28 13:01:37 +02:00
ThomasV
6c231e1d07 test_lnpeer: factorize code into TestPeer._activate_trampoline 2023-06-28 12:50:44 +02:00
ThomasV
24296ca7c0 test_lnpeer: follow-up 21e06b7065 2023-06-28 12:49:30 +02:00
ThomasV
14efb401d6 test_lnpeer: refactor tests for hold invoices 2023-06-28 12:10:30 +02:00
ThomasV
1acf426fa9 lnworker: add support for hold invoices
(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.
2023-06-26 11:09:47 +02:00
ThomasV
df5b98792e lnworker: always call check_received_htlc (no only for MPP)
This will be a generic placeholder to decide if we need to wait
before settling a payment (to be used with hold invoices and
bundled payments)
2023-06-26 09:29:40 +02:00
ThomasV
21e06b7065 lnpeer: new payment secret, derived without preimage.
(this is needed for hold invoices)
2023-06-25 19:15:52 +02:00
SomberNight
24980feab7 config: introduce ConfigVars
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
```
2023-05-25 17:39:48 +00:00
ThomasV
719b468eee Refresh bolt11 routing hints when channel liquidity changes:
- wallet_db update: separate Invoices and Requests.
 - do not store bolt11 invoice in Request
2023-02-28 15:33:17 +01:00
SomberNight
d4338fb503 tests: clean-up use of asyncio 2023-02-20 16:53:44 +00:00
SomberNight
c5bdd5007c tests: rework testnet
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}.
2023-02-18 11:29:54 +00:00
SomberNight
373db76ac9 util: kill bh2u
no longer useful, and the name is so confusing...
2023-02-17 11:43:11 +00:00
ThomasV
b9393b0603 Support scid alias:
- 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)
2023-01-13 15:47:30 +01:00
ThomasV
d95f3e5622 Use different trampoline bits than Eclair. Fixes #8141 2023-01-13 12:48:13 +01:00