If we don't currently have the preimage for an inbound HTLC, that
does not guarantee we can never claim it, but instead only that we
cannot claim it unless we receive the preimage from the channel we
forwarded the channel out on.
Thus, we cannot consider a channel to have no claimable balances if
the only remaining output on the commitment ransaction is an
inbound HTLC for which we do not have the preimage, as we may be
able to claim it in the future.
This commit addresses this issue by adding a new `Balance` variant
- `MaybePreimageClaimableHTLCAwaitingTimeout`, which is generated
until the HTLC output is spent.
Fixes#1620
This uses the various new tracking added in the prior commits to
expose a new `Balance` type - `CounterpartyRevokedOutputClaimable`.
Some nontrivial work is required, however, as we now have to track
HTLC outputs as spendable in a transaction that comes *after* an
HTLC-Success/HTLC-Timeout transaction, which we previously didn't
need to do. Thus, we have to check if an
`onchain_events_awaiting_threshold_conf` event spends a commitment
transaction's HTLC output while walking events. Further, because
we now need to track HTLC outputs after the
HTLC-Success/HTLC-Timeout confirms, and because we have to track
the counterparty's `to_self` output as a contentious output which
could be claimed by either party, we have to examine the
`OnchainTxHandler`'s set of outputs to spend when determining if
certain outputs are still spendable.
Two new tests are added which test various different transaction
formats, and hopefully provide good test coverage of the various
revoked output paths.
When we see a counterparty revoked commitment transaction on-chain
we shouldn't immediately queue up HTLCs present in it for
resolution until we have spent the HTLC outputs in some kind of
claim transaction.
In order to do so, we first have to change the
`fail_unbroadcast_htlcs!()` call to provide it with the HTLCs which
are present in the (revoked) commitment transaction which was
broadcast. However, this is not sufficient - because all of those
HTLCs had their `HTLCSource` removed when the commitment
transaction was revoked, we also have to update
`fail_unbroadcast_htlcs` to check the payment hash and amount when
the `HTLCSource` is `None`.
Somewhat surprisingly, several tests actually explicitly tested for
the old behavior, which required amending to pass with the new
changes.
Finally, this adds a debug assertion when writing `ChannelMonitor`s
to ensure `HTLCSource`s do not leak.
When handling the broadcast of a local commitment transactions
(with associated CSV delays prior to spendability), we incorrectly
handled the CSV delays on HTLC transactions. This caused us to miss
spendable outputs for HTLCs which were awaiting a CSV delay.
Further, because of this, we could hit an assertion as
`get_claimable_balances` asserted that HTLCs were resolved after
the funding spend was resolved, which was not true if the HTLC did
not have a CSV delay attached (due to the above bug or due to it
being an HTLC claim by our counterparty).
This fixes both bugs, also converting some assertions to
`debug_assert`s to avoid future issues as balance mis-calculation
is not currently an indication of potential funds loss.
Thanks to Cash App for reporting this bug.
A single PaymentSent event is generated when a payment is fulfilled.
This is occurs when the preimage is revealed on the first claimed HTLC.
For subsequent HTLCs, the event is not generated.
In order to score channels involved with a successful payments, the
scorer must be notified of each successful path involved in the payment.
Add a PaymentPathSuccessful event for this purpose. Generate it whenever
a part is removed from a pending outbound payment. This avoids duplicate
events when reconnecting to a peer.
Exposing a `RwLock<HashMap<>>` directly was always a bit strange,
and in upcoming changes we'd like to change the internal
datastructure in `ChainMonitor`.
Further, the use of `RwLock` and `HashMap` meant we weren't able
to expose the ChannelMonitors themselves to users in bindings,
leaving a bindings/rust API gap.
Thus, we take this opportunity go expose ChannelMonitors directly
via a wrapper, hiding the internals of `ChainMonitor` behind
getters. We also update tests to use the new API.
The interface for get_route will change to take a scorer. Using
get_route_and_payment_hash whenever possible allows for keeping the
scorer inside get_route_and_payment_hash rather than at every call site.
Replace get_route with get_route_and_payment_hash wherever possible.
Additionally, update get_route_and_payment_hash to use the known invoice
features and the sending node's logger.
When we detect a channel `is_shutdown()` or call on it
`force_shutdown()`, we notify the user with a Event::ChannelClosed
informing about the id and closure reason.
In general, we should always allow users to query for how much is
currently in-flight being claimed on-chain at any time.
This does so by examining the confirmed claims on-chain and
breaking down what is left to be claimed into a new
`ClaimableBalance` enum.
Fixes#995.
MessageSendEvent::PaymentFailureNetworkUpdate served as a hack to pass
an HTLCFailChannelUpdate from ChannelManager to NetGraphMsgHandler via
PeerManager. Instead, remove the event entirely and move the contained
data (renamed NetworkUpdate) to Event::PaymentFailed to be processed by
an event handler.
If we forward an HTLC to our counterparty, but we force-closed the
channel before our counterparty provides us an updated commitment
transaction, we'll end up with a commitment transaction that does
not contain the HTLC which we attempted to forward. In this case,
we need to wait `ANTI_REORG_DELAY` blocks and then fail back the
HTLC as there is no way for us to learn the preimage and the
confirmed commitment transaction paid us the value of the HTLC.
However, check_spend_holder_transaction did not do this - it
instead only looked for dust HTLCs in the confirmed commitment
transaction, paying no attention to what other HTLCs may exist that
are missed.
This will eventually lead to channel force-closure as the channel
on which we received the inbound HTLC to forward will be closed in
time for the initial sender to claim the HTLC on-chain.
