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