`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.
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.
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.
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 ef5be580f5.
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.
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.
Previously we only asserted the `final_value_msat` matches. Looking at
it again we can _of course_ assert the full equality of looked-for and
included route params after all (duh, not sure what I was thinking...).
This cleans up the prior misunderstanding and fixes a bunch of tests
that would now fail otherwise.
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.
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 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.
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.
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.
