Merge pull request #2630 from TheBlueMatt/2023-09-117-rc1

Bump crate versions to 0.0.117-rc1/invoice 0.25-rc1

Merge pull request #2610 from wpaulino/missing-htlc-claim-balance

Fix matching of second-stage HTLC claim in get_htlc_balance

Merge pull request #2621 from G8XSU/dont-persist-erroneous-update

Persist entire monitor if there is an error while applying monitor_update

Bump crate versions to 0.0.117-rc1/invoice 0.25-rc1

Fix wrong node broadcaster in test_yield_anchors_events

Add anchors coverage to test_revoked_counterparty_aggregated_claims

Add anchors coverage to test_revoked_counterparty_htlc_tx_balances

Add update_persisted_channel doc to indicate rare case for None update

Persist full monitor if there is an error while applying monitor_update

Motivation: When there is an error while applying monitor_update to a
channel_monitor,  we don't want to persist a 'monitor_update' which
results in a failure to apply later while reading 'channel_monitor' with
updates from storage. Instead, we should persist the entire 'channel_monitor'
here.

Correctly mark chain_sync updates in test_utils

We were incorrectly marking updates as chain_sync
or not in test_utils based on whether monitor_update
is None or not. Instead, use UpdateOrigin to determine it.

Add anchors coverage to test_revoked_counterparty_commitment_balances

Add anchors coverage to test_balances_on_local_commitment_htlcs

Add anchors coverage to test_claim_value_force_close

Add anchors coverage to chanmon_claim_value_coop_close

Add test util to handle bump HTLC events

Fix matching of second-stage HTLC claim in get_htlc_balance

We incorrectly assumed that the descriptor's output index from
second-stage HTLC transaction would always match the HTLC's output index
in the commitment transaction. This doesn't make any sense though, we
need to make sure we map the descriptor to it's corresponding HTLC in
the commitment. Instead, we check that the transaction from which the
descriptor originated from spends the HTLC in question.

Note that pre-anchors, second-stage HTLC transactions are always 1
input-1 output, so previously we would only match if the HTLC was the
first output in the commitment transaction. Post-anchors, they are
malleable, so  we can aggregate multiple HTLC claims into a single
transaction making this even more likely to happen. Unfortunately, we
lacked proper coverage in this area so the bug went unnoticed. To
address this, we aim to extend our existing coverage of
`get_claimable_balances` to anchor outputs channels in the following
commits.

Merge pull request #2629 from jkczyz/2023-09-invreqfailed

Config-guard `Event::InvoiceRequestFailed`

Merge pull request #2605 from wpaulino/anchors-monitor-track-to-remote-script

Use correct to_remote script in counterparty commitments

Remove unused imports

Fix build warning for unused method

Remove an unnecessary enumerate

Config-guard Event::InvoiceRequestFailed

The event cannot be generated publicly, so remove it for now to avoid
users needing to handle it.

Merge pull request #2628 from wvanlint/fix_multiple_shutdown_results

Fix handling multiple ShutdownResults

Expose witness_script for StaticPaymentOutputDescriptor

Fix incorrect anchors `counterparty_payment_script` upon deserialization

Merge pull request #2622 from wpaulino/funding-and-commitment-tx-confirm-same-block

Avoid early return upon confirmation of channel funding

Merge pull request #2626 from TheBlueMatt/2023-09-revert-2476

Revert "Remove AvailableBalances::balance_msat"

Fix off-by-one max witness estimate for P2WPKH StaticPaymentDescriptor

We were not accounting for the extra byte denoting the number of items
in the witness stack.

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.

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.

Merge pull request #2624 from wpaulino/2609-follow-up

Address 2609 follow-up comments

Fix handling multiple ShutdownResults

Merge pull request #2623 from wpaulino/htlc-claim-receive-preimage-after-close

Claim HTLCs with preimage from currently confirmed commitment

Avoid early return upon confirmation of channel funding

This early return is only possible if the channel requires a single
confirmation, allowing a `channel_ready` message to go out. This can be
problematic though if a commitment transaction (specifically from the
counterparty, as the channel would be immediately closed if a local
commitment is broadcast) also confirms within the same block. The
`ChannelMonitor` will detect both, but it won't inform the
`ChannelManager` at all. Luckily, while the channel still is considered
open to the `ChannelManager`, the `ChannelMonitor` will reject any
further updates to the channel state.

Revert "Remove AvailableBalances::balance_msat"

While removing the `balance_msat` field absolutely makes sense -
it is, at best, confusing - we really need a solid replacement for
it before we can do so. While one such replacement is in progress,
it is not complete and we'd like to not block our current release
on its completion.

This reverts commit ef5be580f51d6756612fea710516c0297203f4dc.

Only yield DelayedPaymentOutput descriptors once their delay expires

Otherwise, we could give users a descriptor ahead of time that will
result in an invalid transaction spend/broadcast.

Merge pull request #2617 from wpaulino/no-persist-same-channel-update

Avoid persisting on same counterparty's ChannelUpdate

Merge pull request #2591 from TheBlueMatt/2023-09-2562-followups

Doc and comment followups to #2562

Note required levels of descendant transactions in get_spendable_outputs

Three levels of descendant transactions starting from the channel's
funding transaction should cover all potential spendable outputs.

The first level covers the commitment transaction.

The second level covers the to_self claims, to_remote claims,
second-stage HTLC claims and justice transactions.

The third levels covers the justice transactions on second-stage HTLCs,
and to_self claims on second-stage HTLCs.

Test preimage claim after reorg of counterparty commitment

This test adds coverage for receiving a preimage after seeing a
counterparty commitment confirm, followed by a reorg and the
confirmation of a different commitment instead.

The first test covers the case where a holder commitment confirms after
the counterparty commitment reorg.

The second test covers the case where a previous counterparty commitment
confirms after the latest counterparty commitment reorg.

Claim HTLCs with preimage from currently confirmed commitment

We should always claim HTLCs from the currently confirmed commitment,
rather than always claiming from the latest or previous counterparty
commitment if we've seen either confirm onchain at a prior point.

Avoid persisting on same counterparty's ChannelUpdate

Some nodes may rebroadcast their `ChannelUpdate` to their counterparty
on every connection establishment, which leads to us doing an additional
persist most of the time when nothing has changed. Now, we'll only
persist if we receive an update that changes anything.

Merge pull request #2609 from wpaulino/monitor-get-spendable-output

Allow retrieval of SpendableOutputDescriptors from relevant transactions

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

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.

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.

Add more details about async persistence completion/backgrounding

This clarifies somewhat that async persistence should run
indefinitely or keep trying via polling, and that either is
acceptable.

Log max routing fee before we start pathfinding

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

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.

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.

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.

Merge pull request #2612 from TheBlueMatt/2023-09-namespace-split

Refer to top-level persistence namespaces as primary_namespace

Merge pull request #2575 from tnull/2023-09-fix-debug-panic

Various router fixes and #2417 follow-ups

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`

Update remaining references to primary/secondary namespaces

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

Update storage constants to new PRIMARY/SECONDARY namespace terms

Rename the persistence `sub_namespace` to `secondary_namespace`

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

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.

Assert query's and route's `final_value_msat` are equal

Refactor `mpp_retry[_overpay]` tests to use `SendEvent::from_event`

.. as a follow-up from #2417.

Make ignored candidate counts `u32`

.. as a follow-up from #2417.

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.

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.

Merge pull request #2607 from DhananjayPurohit/civkit-branch

Issue in adding rust lightning as dependency on windows

Merge pull request #2606 from wpaulino/anchors-counterparty-htlc-claim-sequence

Use correct input sequence for HTLC claims from counterparty commitments

Log full channel_update messages received for our channels

Extend logging of ignored candidates

Use `saturating_add` when adding inflight HTLCs values

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

chore: remove time_utils.rs

No longer needed time_utils in lightning-invoice

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.

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>`.

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.

Merge pull request #2570 from valentinewallace/2023-08-router-fuzz-fail

Fix router fuzz failure

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.

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.

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.

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.

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.

get_route: fix outdated var name

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

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.

Remove trailing whitespace in get_route

Because my text editor loves to do that.

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.

Pathfinding: log when we ignore one hop blinded route hints

Merge pull request #2601 from TheBlueMatt/2023-09-117-alpha2

Bump versions to 0.0.117-alpha2/invoice 0.25.0-alpha2

Merge pull request #2603 from TheBlueMatt/2023-09-default-route-limit

Set a default max_total_routing_fee_msat of 1% + 50sats

Merge pull request #2602 from TheBlueMatt/2023-09-descriptor-hash

Derive `Hash` for `SpendableOutputDescriptor`

Merge pull request #2359 from domZippilli/2023-06-monitor-updating-persister

Monitor updating persister

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>

Merge pull request #2486 from wvanlint/batch_channel_opens

Batch funding for v1 channel establishments

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.

Derive `Debug` and `Display` for `Route`

Derive `Hash` for `SpendableOutputDescriptor`

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

Batch funding for v1 channel establishments

Rename finish_force_close_channel to finish_close_channel

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.

Bump versions to 0.0.117-alpha2/invoice 0.25.0-alpha2

Merge pull request #2417 from tnull/2023-07-max-total-fee

Add config option to set maximum total routing fee

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.

Check `max_total_routing_fee` is accounted for in `test_threaded_payment_retries`

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.