]> git.bitcoin.ninja Git - rust-lightning/log
rust-lightning
13 months agoSupport signing to_remote anchors variant for StaticPaymentOutput
Wilmer Paulino [Mon, 25 Sep 2023 23:55:22 +0000 (16:55 -0700)]
Support signing to_remote anchors variant for StaticPaymentOutput

`to_remote` outputs on commitment transactions with anchor outputs have
an additional `1 CSV` constraint on its spending condition,
transitioning away from the previous P2WPKH script to a P2WSH.

Since our `ChannelMonitor` was never updated to track the proper
`to_remote` script on anchor outputs channels, we also missed updating
our signer to handle the new script changes.

13 months agoUse correct to_remote script in counterparty commitments
Wilmer Paulino [Mon, 25 Sep 2023 23:53:42 +0000 (16:53 -0700)]
Use correct to_remote script in counterparty commitments

While our commitment transactions did use the correct `to_remote`
script, the `ChannelMonitor`'s was not as it is tracked separately. This
would lead to users never receiving an `Event::SpendableOutputs` with a
`StaticPaymentOutput` descriptor to claim the funds.

Luckily, any users affected which had channel closures confirmed by a
counterparty commitment just need to replay the closing transaction to
receive the event.

13 months agoMerge pull request #2609 from wpaulino/monitor-get-spendable-output
Matt Corallo [Fri, 29 Sep 2023 01:29:47 +0000 (01:29 +0000)]
Merge pull request #2609 from wpaulino/monitor-get-spendable-output

Allow retrieval of SpendableOutputDescriptors from relevant transactions

13 months agoMerge pull request #2604 from TheBlueMatt/2023-09-route-overpay-limit
Matt Corallo [Fri, 29 Sep 2023 00:19:04 +0000 (00:19 +0000)]
Merge pull request #2604 from TheBlueMatt/2023-09-route-overpay-limit

Try to overpay the recipient if we fail to find a path at all and limit overpay

13 months agoRetrieve all possible spendable outputs from transactions
Wilmer Paulino [Thu, 28 Sep 2023 18:42:13 +0000 (11:42 -0700)]
Retrieve all possible spendable outputs from transactions

Assuming our keys haven't been compromised, and that random transactions
aren't learning of these scripts somehow and sending funds to them, it
was only possible for one spendable output to exist within a
transaction.

- `shutdown_script` can only exist in co-op close transactions.
- `counterparty_payment_script` can only exist in counterparty
  commitment transactions.
- `broadcasted_holder_revokable_script` can only exist in holder
  commitment/HTLC transactions.
- `destination_script` can exist in any other type of claim we support.

Now that we're exposing this API to users such that they can rescan any
relevant transactions, there's no harm in allowing them to claim more
funds from spendable outputs than we expected.

13 months agoAllow retrieval of SpendableOutputDescriptors from relevant transactions
Wilmer Paulino [Wed, 27 Sep 2023 17:27:44 +0000 (10:27 -0700)]
Allow retrieval of SpendableOutputDescriptors from relevant transactions

Currently, our API will only expose `SpendableOutputDescriptor`s once
after they are no longer under reorg risk (see `ANTI_REORG_DELAY`).
Users have often requested they'd like the ability to retrieve these in
some other way, either for historical purposes, or to handle replaying
any in the event of a failure.

13 months agoLog max routing fee before we start pathfinding 2023-09-route-overpay-limit
Matt Corallo [Thu, 28 Sep 2023 17:51:05 +0000 (17:51 +0000)]
Log max routing fee before we start pathfinding

This may be useful in debugging routing failures in the future.

13 months agoInclude any recipient overpayment amounts in the route fee limit
Matt Corallo [Thu, 28 Sep 2023 18:19:36 +0000 (18:19 +0000)]
Include any recipient overpayment amounts in the route fee limit

If the user told us to limit their total fee exposure, we should
do so including any potential overpayment to the recipient, which
is ultimately a part of the "fee" as far as the user is concerned.

13 months agoTry to overpay the recipient if we fail to find a path at all
Matt Corallo [Tue, 26 Sep 2023 21:58:53 +0000 (21:58 +0000)]
Try to overpay the recipient if we fail to find a path at all

Previously we'd only try to overpay if we managed to find a path
to the recipient which was sufficient. However, if we fail to find
any path to the recipient at all we should still retry overpaying
the recipient. Ultimately we should be silling to pay whatever
reasonable performance penalty if the alternative is not finding a
path at all, which we do here.

13 months agoAllow routefinding again due to HTLC limit based on >= not >
Matt Corallo [Thu, 28 Sep 2023 18:19:50 +0000 (18:19 +0000)]
Allow routefinding again due to HTLC limit based on >= not >

While this doesn't matter much in practice, if we go around again
when route-finding to try to meet an htlc_minimum_msat, we use the
`recommended_value_msat` which can work if we meet the
`htlc_minimum_msat` on a channel exactly, so using >= rather than >
can capture cases with 1msat more.

13 months agoMerge pull request #2612 from TheBlueMatt/2023-09-namespace-split
Matt Corallo [Thu, 28 Sep 2023 20:37:54 +0000 (20:37 +0000)]
Merge pull request #2612 from TheBlueMatt/2023-09-namespace-split

Refer to top-level persistence namespaces as primary_namespace

13 months agoMerge pull request #2575 from tnull/2023-09-fix-debug-panic
Matt Corallo [Thu, 28 Sep 2023 20:30:29 +0000 (20:30 +0000)]
Merge pull request #2575 from tnull/2023-09-fix-debug-panic

Various router fixes and #2417 follow-ups

13 months agoMerge pull request #2611 from TheBlueMatt/2023-09-no-close-on-bad-update
Matt Corallo [Thu, 28 Sep 2023 19:06:24 +0000 (19:06 +0000)]
Merge pull request #2611 from TheBlueMatt/2023-09-no-close-on-bad-update

Don't FC when a `channel_update` has a bogus `htlc_minimum_msat`

13 months agoUpdate remaining references to primary/secondary namespaces 2023-09-namespace-split
Matt Corallo [Thu, 28 Sep 2023 17:28:04 +0000 (17:28 +0000)]
Update remaining references to primary/secondary namespaces

Update various variables, error strings, and the pending changelog
entry to refer to new namespace terminology.

13 months agoUpdate storage constants to new PRIMARY/SECONDARY namespace terms
Matt Corallo [Thu, 28 Sep 2023 17:06:20 +0000 (17:06 +0000)]
Update storage constants to new PRIMARY/SECONDARY namespace terms

13 months agoRename the persistence `sub_namespace` to `secondary_namespace`
Matt Corallo [Thu, 28 Sep 2023 16:36:52 +0000 (16:36 +0000)]
Rename the persistence `sub_namespace` to `secondary_namespace`

With the top-level namespace now called "primary", "secondary"
makes more sense than "sub".

13 months agoDon't retry overpaid values for `PartialFailure`s
Elias Rohrer [Wed, 27 Sep 2023 10:43:31 +0000 (12:43 +0200)]
Don't retry overpaid values for `PartialFailure`s

Previously, if an overpaid path would fail immediately, we'd retry a
`PartialFailure` with the full path amount, _including_ any overpayment.

Here, we now subtract the succeeded paths' values from the
net. value to exclude the overpaid amounts on retry.

13 months agoAssert query's and route's `final_value_msat` are equal
Elias Rohrer [Thu, 28 Sep 2023 08:04:35 +0000 (10:04 +0200)]
Assert query's and route's `final_value_msat` are equal

13 months agoRefactor `mpp_retry[_overpay]` tests to use `SendEvent::from_event`
Elias Rohrer [Wed, 27 Sep 2023 07:57:20 +0000 (09:57 +0200)]
Refactor `mpp_retry[_overpay]` tests to use `SendEvent::from_event`

.. as a follow-up from #2417.

13 months agoMake ignored candidate counts `u32`
Elias Rohrer [Wed, 27 Sep 2023 07:28:43 +0000 (09:28 +0200)]
Make ignored candidate counts `u32`

.. as a follow-up from #2417.

13 months agoAvoid adding duplicate hint candidates if they are first hops
Elias Rohrer [Fri, 22 Sep 2023 14:38:30 +0000 (16:38 +0200)]
Avoid adding duplicate hint candidates if they are first hops

If we have a direct channel to a node generating an invoice with route
hints, we'd previously happily add multiple candidates that all refer to
the same channel. To keep our candidate set small and unify our tracking
where possible, we now check if its `short_channel_id` is an
`outbound_scid_alias` of any of our first hops and refrain from adding
another candidate if it's the case.

13 months agoAlso add route hints if we are the source
Elias Rohrer [Fri, 22 Sep 2023 13:56:07 +0000 (15:56 +0200)]
Also add route hints if we are the source

Previously, we would only consider route hints if we had a direct
channel to the first node in the hint or if the first node in the hint
was part of the public network graph.

However, this left out the possiblity of us being part of the first hop,
especially if our own node is not announced and part of the graph.

13 months agoMerge pull request #2607 from DhananjayPurohit/civkit-branch
Matt Corallo [Thu, 28 Sep 2023 16:54:44 +0000 (16:54 +0000)]
Merge pull request #2607 from DhananjayPurohit/civkit-branch

Issue in adding rust lightning as dependency on windows

13 months agoMerge pull request #2606 from wpaulino/anchors-counterparty-htlc-claim-sequence
Matt Corallo [Thu, 28 Sep 2023 16:32:18 +0000 (16:32 +0000)]
Merge pull request #2606 from wpaulino/anchors-counterparty-htlc-claim-sequence

Use correct input sequence for HTLC claims from counterparty commitments

13 months agoLog full channel_update messages received for our channels 2023-09-no-close-on-bad-update
Matt Corallo [Thu, 28 Sep 2023 16:30:02 +0000 (16:30 +0000)]
Log full channel_update messages received for our channels

13 months agoExtend logging of ignored candidates
Elias Rohrer [Fri, 22 Sep 2023 13:00:43 +0000 (15:00 +0200)]
Extend logging of ignored candidates

13 months agoUse `saturating_add` when adding inflight HTLCs values
Elias Rohrer [Thu, 14 Sep 2023 12:14:03 +0000 (14:14 +0200)]
Use `saturating_add` when adding inflight HTLCs values

Previously this calculation could overflow, leading to panicking in `debug`.

13 months agochore: remove time_utils.rs
DhananjayPurohit [Tue, 26 Sep 2023 09:24:00 +0000 (14:54 +0530)]
chore: remove time_utils.rs

No longer needed time_utils in lightning-invoice

13 months agoRefer to top-level persistence namespaces as `primary_namespace`
Matt Corallo [Thu, 28 Sep 2023 02:40:07 +0000 (02:40 +0000)]
Refer to top-level persistence namespaces as `primary_namespace`

This fixes a bindings build error as `namespace` is a C++ keyword
which cannot be used as an argument, and while this could be fixed
in the bindings rather than here, separating the term `namespace`
between the concept (which refers to the primary and sub
namespaces) and the primary namespace makes the documentation more
readable.

13 months agoUse `Result<_, io::Error>` over `io::Result<_>`
Matt Corallo [Wed, 27 Sep 2023 22:26:57 +0000 (22:26 +0000)]
Use `Result<_, io::Error>` over `io::Result<_>`

Personally I've always found the overload of a prelude enum to be
confusing, and never bothered to handle it properly in bindings as
a result. To avoid needing to do so now, we simply move the
newly-introduced `io::Result` usages over to
`Result<_, io::Error>`.

13 months agoDon't FC when a `channel_update` has a bogus `htlc_minimum_msat`
Matt Corallo [Thu, 28 Sep 2023 00:46:31 +0000 (00:46 +0000)]
Don't FC when a `channel_update` has a bogus `htlc_minimum_msat`

If a peer sends us a `channel_update` for our own channel with an
`htlc_minimum_msat` which is more than the channel's amount, that's
dumb, but there's no reason to force-close the channel. We don't
even use the field.

Here we simply drop the unnecessary check.

13 months agoMerge pull request #2570 from valentinewallace/2023-08-router-fuzz-fail
Matt Corallo [Wed, 27 Sep 2023 20:40:32 +0000 (20:40 +0000)]
Merge pull request #2570 from valentinewallace/2023-08-router-fuzz-fail

Fix router fuzz failure

13 months agoUse correct input sequence for HTLC claims from counterparty commitments
Wilmer Paulino [Mon, 25 Sep 2023 23:57:04 +0000 (16:57 -0700)]
Use correct input sequence for HTLC claims from counterparty commitments

HTLC outputs, like the `to_remote` output, in commitment transactions
with anchor outputs also have an additional `1 CSV` constraint on the
counterparty. When spending such outputs, their corresponding input
needs to have their sequence set to 1. This was done for HTLC claims
from holder commitments, but unfortunately not for counterparty
commitments as we were lacking test coverage.

13 months agoget_route: fix path value contribution to include min htlc overpay 2023-09-2570-fuzz-test
Valentine Wallace [Wed, 27 Sep 2023 03:44:27 +0000 (23:44 -0400)]
get_route: fix path value contribution to include min htlc overpay

Previously, the fuzzer hit a debug panic because we wouldn't include the amount
overpaid to meet a last hop's min_htlc in the total collected paths value. We
now include this value and also penalize hops along the overpaying path to
ensure that it gets deprioritized in path selection.

13 months agoget_route: fix path_min for first_hop<>network_node candidates
Valentine Wallace [Wed, 27 Sep 2023 02:21:15 +0000 (22:21 -0400)]
get_route: fix path_min for first_hop<>network_node candidates

Previously, we would add a first_hop<>network_node channel that did not have
enough contribution amount to cover the next channel's min htlc plus fees,
because we were storing the next hop as having a path_min that did not include
fees, and would add a connecting first_hop node that did not have enough
contribution amount, leading to a debug panic upon invalid path construction.

13 months agoget_route: fix path_min for first_hop<>blinded_hint candidates
Valentine Wallace [Tue, 26 Sep 2023 22:57:02 +0000 (18:57 -0400)]
get_route: fix path_min for first_hop<>blinded_hint candidates

See previous commit, but the bug where we would underestimate how much a first
hop candidate needed to be able to relay was also present in blinded paths.

13 months agoget_route: fix path_min when adding first_hop<>route_hint candidates
Valentine Wallace [Wed, 27 Sep 2023 01:19:33 +0000 (21:19 -0400)]
get_route: fix path_min when adding first_hop<>route_hint candidates

Previously, we would add a candidate hop to the list of potential hops even
though its available contribution wasn't sufficient to meet the next hop's
min_htlc. We'd subsequently build an invalid path using this hop and hit a
debug assertion.

13 months agoget_route: fix outdated var name
Valentine Wallace [Fri, 22 Sep 2023 20:13:36 +0000 (16:13 -0400)]
get_route: fix outdated var name

Previously this variable was a bool, but has since been updated to be an
Option, so rename accordingly.

13 months agoPathfinding: ignore blinded route hints where we are the intro node
Valentine Wallace [Tue, 12 Sep 2023 18:57:59 +0000 (14:57 -0400)]
Pathfinding: ignore blinded route hints where we are the intro node

See tests, but the fuzzer found several panics from not fully ignoring these
hints.

We should support these route hints eventually, but it will involve some
reworking of the Path/BlindedTail structs.

13 months agoRemove trailing whitespace in get_route
Valentine Wallace [Tue, 12 Sep 2023 19:01:53 +0000 (15:01 -0400)]
Remove trailing whitespace in get_route

Because my text editor loves to do that.

13 months agoRefuse to pathfind when provided our_node_id matches internal dummy pk
Valentine Wallace [Tue, 12 Sep 2023 18:56:54 +0000 (14:56 -0400)]
Refuse to pathfind when provided our_node_id matches internal dummy pk

The fuzzer managed to hit this and it causes some invalid paths to be generated
internally.

13 months agoPathfinding: log when we ignore one hop blinded route hints
Valentine Wallace [Tue, 12 Sep 2023 18:55:36 +0000 (14:55 -0400)]
Pathfinding: log when we ignore one hop blinded route hints

14 months agoMerge pull request #2601 from TheBlueMatt/2023-09-117-alpha2 v0.0.117-alpha2
Matt Corallo [Wed, 27 Sep 2023 03:51:12 +0000 (03:51 +0000)]
Merge pull request #2601 from TheBlueMatt/2023-09-117-alpha2

Bump versions to 0.0.117-alpha2/invoice 0.25.0-alpha2

14 months agoMerge pull request #2603 from TheBlueMatt/2023-09-default-route-limit
Matt Corallo [Wed, 27 Sep 2023 03:47:16 +0000 (03:47 +0000)]
Merge pull request #2603 from TheBlueMatt/2023-09-default-route-limit

Set a default max_total_routing_fee_msat of 1% + 50sats

14 months agoMerge pull request #2602 from TheBlueMatt/2023-09-descriptor-hash
Matt Corallo [Wed, 27 Sep 2023 03:47:09 +0000 (03:47 +0000)]
Merge pull request #2602 from TheBlueMatt/2023-09-descriptor-hash

Derive `Hash` for `SpendableOutputDescriptor`

14 months agoMerge pull request #2359 from domZippilli/2023-06-monitor-updating-persister
Matt Corallo [Wed, 27 Sep 2023 03:46:39 +0000 (03:46 +0000)]
Merge pull request #2359 from domZippilli/2023-06-monitor-updating-persister

Monitor updating persister

14 months agoAdd MonitorUpdatingPersister
Dom Zippilli [Wed, 30 Aug 2023 00:42:07 +0000 (17:42 -0700)]
Add MonitorUpdatingPersister

MonitorUpdatingPersister is an implementation of Persister that stores
ChannelMonitorUpdates separately from ChannelMonitors. Its RFC is
in #2545, at https://github.com/orgs/lightningdevkit/discussions/2545.

Co-Authored-By: Elias Rohrer <dev@tnull.de>
14 months agoMerge pull request #2486 from wvanlint/batch_channel_opens
Matt Corallo [Tue, 26 Sep 2023 22:54:12 +0000 (22:54 +0000)]
Merge pull request #2486 from wvanlint/batch_channel_opens

Batch funding for v1 channel establishments

14 months agoSet a default `max_total_routing_fee_msat` of 1% + 50sats 2023-09-default-route-limit
Matt Corallo [Tue, 26 Sep 2023 21:28:29 +0000 (21:28 +0000)]
Set a default `max_total_routing_fee_msat` of 1% + 50sats

When using the normal default constructors, we should have some
fee maximum to ensure our default behavior is safe. Here we pick
1% + 50 sats to ensure we're always willing to pay
reasonabl(y high) fees, but not anything too wild.

14 months agoDerive `Debug` and `Display` for `Route`
Matt Corallo [Tue, 26 Sep 2023 21:12:46 +0000 (21:12 +0000)]
Derive `Debug` and `Display` for `Route`

14 months agoDerive `Hash` for `SpendableOutputDescriptor` 2023-09-descriptor-hash
Matt Corallo [Tue, 26 Sep 2023 20:41:49 +0000 (20:41 +0000)]
Derive `Hash` for `SpendableOutputDescriptor`

This allows for easy descriptor de-duplication before building a
claiming transaction.

14 months agoBatch funding for v1 channel establishments
Willem Van Lint [Sat, 29 Jul 2023 00:21:47 +0000 (17:21 -0700)]
Batch funding for v1 channel establishments

14 months agoRename finish_force_close_channel to finish_close_channel
Willem Van Lint [Tue, 26 Sep 2023 05:39:26 +0000 (22:39 -0700)]
Rename finish_force_close_channel to finish_close_channel

14 months agoAlways call finish_force_close_channel on closure
Willem Van Lint [Tue, 26 Sep 2023 05:36:26 +0000 (22:36 -0700)]
Always call finish_force_close_channel on closure

This is a step towards more unified closing of channels, and provides a
place where the per_peer_state lock is not held.

14 months agoBump versions to 0.0.117-alpha2/invoice 0.25.0-alpha2 2023-09-117-alpha2
Matt Corallo [Tue, 26 Sep 2023 20:21:08 +0000 (20:21 +0000)]
Bump versions to 0.0.117-alpha2/invoice 0.25.0-alpha2

14 months agoMerge pull request #2417 from tnull/2023-07-max-total-fee
Matt Corallo [Tue, 26 Sep 2023 20:07:52 +0000 (20:07 +0000)]
Merge pull request #2417 from tnull/2023-07-max-total-fee

Add config option to set maximum total routing fee

14 months agoTest `max_total_routing_fee_msat` handling when retrying overpaid paths
Elias Rohrer [Tue, 26 Sep 2023 13:30:39 +0000 (15:30 +0200)]
Test `max_total_routing_fee_msat` handling when retrying overpaid paths

We setup an MPP scenario with two paths in which we need to overpay to
reach `htlc_minimum_msat`. We then fail the overpaid path and check that
on retry our `max_total_routing_fee_msat` only accounts for the path
fees, but not for the fees overpaid in the first attempt.

14 months agoCheck `max_total_routing_fee` is accounted for in `test_threaded_payment_retries`
Elias Rohrer [Tue, 26 Sep 2023 08:48:33 +0000 (10:48 +0200)]
Check `max_total_routing_fee` is accounted for in `test_threaded_payment_retries`

14 months agoCheck `max_total_routing_fee` is reduced in `mpp_retry` test
Elias Rohrer [Wed, 20 Sep 2023 13:32:37 +0000 (15:32 +0200)]
Check `max_total_routing_fee` is reduced in `mpp_retry` test

We check that the `RouteParameters::max_total_routing_fee` field is reduced accordingly
to our previously used fees.

14 months agoAccount for leftover fee budget when retrying via `check_retry_payment`
Elias Rohrer [Mon, 4 Sep 2023 12:51:42 +0000 (14:51 +0200)]
Account for leftover fee budget when retrying via `check_retry_payment`

14 months agoMerge pull request #2597 from TheBlueMatt/2023-09-finish-force-close-deadlocks
Matt Corallo [Tue, 26 Sep 2023 16:36:16 +0000 (16:36 +0000)]
Merge pull request #2597 from TheBlueMatt/2023-09-finish-force-close-deadlocks

Fix potential peer_state deadlocks in `finish_force_close_channel`

14 months agoAccount for leftover fee budget when retrying `PartialFailure`s
Elias Rohrer [Tue, 18 Jul 2023 13:46:06 +0000 (15:46 +0200)]
Account for leftover fee budget when retrying `PartialFailure`s

14 months agoTest we adhere to `max_total_routing_fee_msat`
Elias Rohrer [Fri, 14 Jul 2023 14:46:52 +0000 (16:46 +0200)]
Test we adhere to `max_total_routing_fee_msat`

14 months agoConsider `RouteParameters::max_total_routing_fee_msat` in `get_route`
Elias Rohrer [Fri, 14 Jul 2023 11:25:33 +0000 (13:25 +0200)]
Consider `RouteParameters::max_total_routing_fee_msat` in `get_route`

We exclude any candidate hops if we find that using them would let the
aggregated path routing fees exceed `max_total_routing_fee_msat`.

Moreover, we return an error if the aggregated fees over all paths of
the selected route would surpass `max_total_routing_fee_msat`.

14 months agoIntroduce `RouteParameters::max_total_routing_fee_msat`
Elias Rohrer [Fri, 14 Jul 2023 09:47:22 +0000 (11:47 +0200)]
Introduce `RouteParameters::max_total_routing_fee_msat`

Currently, users have no means to upper-bound the total fees accruing
when finding a route. Here, we add a corresponding field to
`RouteParameters` which will be used to limit the candidate set during
path finding in the following commits.

14 months agoProvide some test coverage of `shutdown` msgs for unfunded chans 2023-09-finish-force-close-deadlocks
Matt Corallo [Mon, 25 Sep 2023 19:49:15 +0000 (19:49 +0000)]
Provide some test coverage of `shutdown` msgs for unfunded chans

We have code to handle receiving `shutdown` messages on unfudned
channels. However, it had no test coverage, which we add here.

14 months agoFix potential peer_state deadlocks in `finish_force_close_channel`
Matt Corallo [Mon, 25 Sep 2023 18:05:53 +0000 (18:05 +0000)]
Fix potential peer_state deadlocks in `finish_force_close_channel`

`ChannelManager::finish_force_close_channel` exists to do cleanups
which must happen without the `per_peer_state` mutex held. However,
because it lacked lock assertions, several changes snuck in
recently which resulted in it running with peer-state locks held,
risking a deadlock if some HTLCs need to be failed.

14 months agoMerge pull request #2583 from Evanfeenstra/pub-make-onion
Matt Corallo [Mon, 25 Sep 2023 17:08:41 +0000 (17:08 +0000)]
Merge pull request #2583 from Evanfeenstra/pub-make-onion

Pub make onion

14 months agoMerge pull request #2576 from valentinewallace/2023-09-fix-outbound-bp-fail-ev
Matt Corallo [Mon, 25 Sep 2023 16:56:03 +0000 (16:56 +0000)]
Merge pull request #2576 from valentinewallace/2023-09-fix-outbound-bp-fail-ev

Fix `PaymentPathFailed::payment_failed_permanently` on blinded path fail

14 months agoMerge pull request #2594 from benthecarman/debug-monitor-update-id
Matt Corallo [Mon, 25 Sep 2023 16:00:04 +0000 (16:00 +0000)]
Merge pull request #2594 from benthecarman/debug-monitor-update-id

Implement Debug for MonitorUpdateId

14 months agoImplement Debug for MonitorUpdateId
benthecarman [Sun, 24 Sep 2023 05:34:27 +0000 (00:34 -0500)]
Implement Debug for MonitorUpdateId

14 months agoBlame outbound channel on UPDATE onion failure with 0-len update
Valentine Wallace [Fri, 15 Sep 2023 20:55:12 +0000 (16:55 -0400)]
Blame outbound channel on UPDATE onion failure with 0-len update

We've run into this several times in the wild, likely due to
https://github.com/ElementsProject/lightning/issues/6200 wherein a node on the
path will error with 0x1000 but not provide a channel update (a spec
violation).

Previously, we would blame the inbound edge even though the buggy peer wanted
us to blame the outbound edge. Since this issue seems to be recurring and our
blaming the inbound edge is causing us to punish innocent channels, trust the
peer that the outbound edge is the one to blame.

14 months agoFix PaymentPathFailed::payment_failed_permanently on blinded path fail
Valentine Wallace [Thu, 14 Sep 2023 15:46:02 +0000 (11:46 -0400)]
Fix PaymentPathFailed::payment_failed_permanently on blinded path fail

Previously this value would be incorrectly set to true because we wouldn't
account for blinded hops when determining if we were processing the last hop's
failure packet.

14 months agoCorrect DecodedOnionFailure when processing we-are-intro-node path
Valentine Wallace [Thu, 14 Sep 2023 15:41:35 +0000 (11:41 -0400)]
Correct DecodedOnionFailure when processing we-are-intro-node path

We don't support sending to paths where we are the intro node yet, but may as
well set the failure correctly now.

14 months agoDecodedOnionFailure::payment_retryable -> ::payment_failed_permanently
Valentine Wallace [Thu, 14 Sep 2023 15:33:01 +0000 (11:33 -0400)]
DecodedOnionFailure::payment_retryable -> ::payment_failed_permanently

Our ultimate goal with this field is to set
PaymentPathFailed::payment_failed_permanently, so use this name rather than
flipping a bool back and forth across methods.

14 months agoStruct-ify onion util internal result type
Valentine Wallace [Wed, 20 Sep 2023 18:49:58 +0000 (14:49 -0400)]
Struct-ify onion util internal result type

Improves readability.

14 months agoRename onion util internal var
Valentine Wallace [Thu, 14 Sep 2023 03:52:11 +0000 (23:52 -0400)]
Rename onion util internal var

This variable is ultimately for setting
PaymentPathFailed::payment_failed_permanently, so use this name rather than
flipping a bool back and forth.

14 months agoMerge pull request #2589 from ErikDeSmedt/reexport_route_hint_hop
Elias Rohrer [Fri, 22 Sep 2023 07:09:41 +0000 (09:09 +0200)]
Merge pull request #2589 from ErikDeSmedt/reexport_route_hint_hop

Reexport RouteHintHop

14 months agoMerge pull request #2592 from TheBlueMatt/2023-09-117-alpha v0.0.117-alpha1
Matt Corallo [Thu, 21 Sep 2023 23:20:17 +0000 (23:20 +0000)]
Merge pull request #2592 from TheBlueMatt/2023-09-117-alpha

Update crate version numbers to 0.0.117-alpha1/invoice 0.25-alpha1

14 months agoMerge pull request #2590 from TheBlueMatt/2023-09-default-score-params
Matt Corallo [Thu, 21 Sep 2023 20:40:13 +0000 (20:40 +0000)]
Merge pull request #2590 from TheBlueMatt/2023-09-default-score-params

Use `Default::default()` to construct `()` as a test scoring param

14 months agoUpdate crate version numbers to 0.0.117-alpha1/invoice 0.25-alpha1 2023-09-117-alpha
Matt Corallo [Thu, 21 Sep 2023 20:27:12 +0000 (20:27 +0000)]
Update crate version numbers to 0.0.117-alpha1/invoice 0.25-alpha1

14 months agoMerge pull request #2562 from TheBlueMatt/2023-08-no-perm-fail
Matt Corallo [Thu, 21 Sep 2023 20:22:16 +0000 (20:22 +0000)]
Merge pull request #2562 from TheBlueMatt/2023-08-no-perm-fail

Drop the ChannelMonitorUpdateStatus::PermanentFailure variant

14 months agoAdd an `UnrecoverableError` variant to `ChannelMonitorUpdateStatus` 2023-08-no-perm-fail
Matt Corallo [Thu, 14 Sep 2023 20:02:46 +0000 (20:02 +0000)]
Add an `UnrecoverableError` variant to `ChannelMonitorUpdateStatus`

While there is no great way to handle a true failure to persist a
`ChannelMonitorUpdate`, it is confusing for users for there to be
no error variant at all on an I/O operation.

Thus, here we re-add the error variant removed over the past
handful of commits, but rather than handle it in a truly unsafe
way, we simply panic, optimizing for maximum mutex poisoning to
ensure any future operations fail and return immediately.

In the future, we may consider changing the handling of this to
instead set some "disconnect all peers and fail all operations"
bool to give the user a better chance to shutdown in a semi-orderly
fashion, but there's only so much that can be done in lightning if
we truly cannot persist new updates.

14 months agoDrop doc comments on `ChainMonitor` trait impl methods
Matt Corallo [Sun, 10 Sep 2023 20:21:50 +0000 (20:21 +0000)]
Drop doc comments on `ChainMonitor` trait impl methods

In general, doc comments on trait impl blocks are not very visible
in rustdoc output, and unless they provide useful information they
should be elided.

Here we drop useless doc comments on `ChainMonitor`'s `Watch` impl
methods.

14 months agoDrop error handling in `handle_new_monitor_update`
Matt Corallo [Sun, 10 Sep 2023 20:05:43 +0000 (20:05 +0000)]
Drop error handling in `handle_new_monitor_update`

Now that `handle_new_monitor_update` can no longer return an error,
it similarly no longer needs any code to handle errors. Here we
remove that code, substantially reducing macro variants.

14 months agoClean up code flow in `ChannelManager`
Matt Corallo [Sun, 10 Sep 2023 17:48:49 +0000 (17:48 +0000)]
Clean up code flow in `ChannelManager`

In the previous commit various dead code was removed. Here we
finish that cleanup by removing uneccessary indentation and syntax.

14 months agoDrop `PermamentFailure` persistence handling in ChannelManager
Matt Corallo [Sun, 10 Sep 2023 17:39:24 +0000 (17:39 +0000)]
Drop `PermamentFailure` persistence handling in ChannelManager

14 months agoUpdate `ChannelMonitorUpdateStatus` documentation with async support
Matt Corallo [Fri, 8 Sep 2023 00:05:56 +0000 (00:05 +0000)]
Update `ChannelMonitorUpdateStatus` documentation with async support

Since we now (almost) support async monitor update persistence, the
documentation on `ChannelMonitorUpdateStatus` can be updated to no
longer suggest users must keep a local copy that persists before
returning. However, because there are still a few remaining issues,
we note that async support is currently beta and explicily warn of
potential for funds-loss.

Fixes #1684

14 months agoRename `MonitorEvent::CommitmentTxConfirmed` to `HolderForceClosed`
Matt Corallo [Wed, 30 Aug 2023 18:22:33 +0000 (18:22 +0000)]
Rename `MonitorEvent::CommitmentTxConfirmed` to `HolderForceClosed`

The `MonitorEvent::CommitmentTxConfirmed` has always been a result
of us force-closing the channel, not the counterparty doing so.
Thus, it was always a bit of a misnomer. Worse, it carried over
into the channel's `ClosureReason` in the event API.

Here we simply rename it and use the proper `ClosureReason`.

14 months agoDrop `ChannelMonitorUpdate::UpdateFailed` as its now unused
Matt Corallo [Wed, 30 Aug 2023 18:19:35 +0000 (18:19 +0000)]
Drop `ChannelMonitorUpdate::UpdateFailed` as its now unused

14 months agoDrop `channel_perm_failed` tracking in `ChainMonitor`
Matt Corallo [Wed, 30 Aug 2023 18:16:03 +0000 (18:16 +0000)]
Drop `channel_perm_failed` tracking in `ChainMonitor`

Now that `PermanentFailure` is not a possible return value, we can
simply remove handling of it in `ChannelMonitor`.

14 months agoDrop the `ChannelMonitorUpdateStatus::PermanentFailure` variant
Matt Corallo [Sun, 10 Sep 2023 17:14:32 +0000 (17:14 +0000)]
Drop the `ChannelMonitorUpdateStatus::PermanentFailure` variant

When a `ChannelMonitorUpdate` fails to apply, it generally means
we cannot reach our storage backend. This, in general, is a
critical issue, but is often only a transient issue.

Sadly, users see the failure variant and return it on any I/O
error, resulting in channel force-closures due to transient issues.

Users don't generally expect force-closes in most cases, and
luckily with async `ChannelMonitorUpdate`s supported we don't take
any risk by "delaying" the `ChannelMonitorUpdate` indefinitely.

Thus, here we drop the `PermanentFailure` variant entirely, making
all failures instead be "the update is in progress, but won't ever
complete", which is equivalent if we do not close the channel
automatically.

14 months agoRewrite failure payment retry tests to avoid perm-fail storage
Matt Corallo [Sun, 10 Sep 2023 22:11:56 +0000 (22:11 +0000)]
Rewrite failure payment retry tests to avoid perm-fail storage

Two tests in the payment tests currently rely on failing to persist
ChannelMonitorUpdates as their method of failing payments before
they even get out the door.

In the coming commits we'll drop the persist failure error codes,
so here rewrite these tests to rely on trying to send more than is
available in a channel.

14 months agoReexport RouteHintHop
Erik De Smedt [Tue, 19 Sep 2023 18:57:40 +0000 (20:57 +0200)]
Reexport RouteHintHop

Earlier @benthecarman re-exported `RouteHint` to make life-easier
for developpers that use `lightning-invoice` and don't use the
`lightning`-crate.

This only solved part of the issue. To create a `RouteHint` the
developer must also have access to `RouteHintHop`.

See also:
  PR https://github.com/lightningdevkit/rust-lightning/pull/2572
commit 79b426f49b08a66e404669ce7d1332c3977c5d95

14 months agoUse `Default::default()` to construct `()` as a test scoring param 2023-09-default-score-params
Matt Corallo [Thu, 21 Sep 2023 01:44:23 +0000 (01:44 +0000)]
Use `Default::default()` to construct `()` as a test scoring param

In bindings, we can't use unbounded generic types, and thus have to
rip out the `ScoreParams` and replace them with static
`ProbabilisticScoringFeeParams` universally. To make this easier,
using `Default::default()` everywhere allows the type to change out
from under the test without the test needing to change.

14 months agoMerge pull request #2547 from TheBlueMatt/2023-04-nonlinear-scoring
Matt Corallo [Wed, 20 Sep 2023 22:21:02 +0000 (22:21 +0000)]
Merge pull request #2547 from TheBlueMatt/2023-04-nonlinear-scoring

Add an option to make the success probability estimation nonlinear

14 months agopublic make_onion_message static method on OnionMessenger
Evan Feenstra [Fri, 15 Sep 2023 22:47:23 +0000 (15:47 -0700)]
public make_onion_message static method on OnionMessenger

14 months agoAvoid unnecessary newline in middle of log statement 2023-04-nonlinear-scoring
Matt Corallo [Sat, 16 Sep 2023 17:04:11 +0000 (17:04 +0000)]
Avoid unnecessary newline in middle of log statement

14 months agoAdd an option to make the success probability estimation nonlinear
Matt Corallo [Mon, 24 Apr 2023 01:52:41 +0000 (01:52 +0000)]
Add an option to make the success probability estimation nonlinear

Our "what is the success probability of paying over a channel with
the given liquidity bounds" calculation currently assumes the
probability of where the liquidity lies in a channel is constant
across the entire capacity of a channel. This is obviously a
somewhat dubious assumption given most nodes don't materially
rebalance and flows within the network often push liquidity
"towards the edges".

Here we add an option to consider this when scoring channels during
routefinding. Specifically, if a new `linear_success_probability`
flag is unset on `ProbabilisticScoringFeeParameters`, rather than
assuming a PDF of `1` (across the channel's capacity scaled from 0
to 1), we use `(x - 0.5)^2`.

This assumes liquidity is likely to be near the edges, which
matches experimental results. Further, calculating the CDF (i.e.
integral) between arbitrary points on the PDF is trivial, which we
do as our main scoring function.

While this (finally) introduces floats in our scoring, its not
practical to exponentiate using fixed-precision, and benchmarks
show this is a performance regression, but not a huge one, more
than made up for by the increase in payment success rates.

14 months agoScore in-flight amounts as amounts, not a capacity reduction
Matt Corallo [Sat, 16 Sep 2023 18:39:03 +0000 (18:39 +0000)]
Score in-flight amounts as amounts, not a capacity reduction

When we started considering the in-flight amounts when scoring, we
took the approach of considering the in-flight amount as an
effective reduction in the channel's total capacity. When we were
scoring using a flat success probability PDF, that was fine,
however in the next commit we'll move to a highly nonlinear one,
which makes this a pretty confusing heuristic.

Here, instead, we move to considering the in-flight amount as
simply an extension of the amount we're trying to send over the
channel, which is equivalent for the flat success probability PDF,
but makes much more sense in a nonlinear world.