Commit graph

6 commits

Author SHA1 Message Date
Matt Corallo
f4ab077a69
Merge pull request #1857 from TheBlueMatt/2022-11-reload-htlc
Fail HTLCs which were removed from a channel but not persisted
2022-12-05 22:54:08 +00:00
Matt Corallo
dbe4aadb89 Fail HTLCs which were removed from a channel but not persisted
When a channel is force-closed, if a `ChannelMonitor` update is
completed but a `ChannelManager` persist has not yet happened,
HTLCs which were removed in the latest (persisted) `ChannelMonitor`
update will not be failed even though they do not appear in the
commitment transaction which went on chain. This is because the
`ChannelManager` thinks the `ChannelMonitor` is responsible for
them (as it is stale), but the `ChannelMonitor` has no knowledge of
the HTLC at all (as it is not stale).

The fix for this is relatively simple - we need to check for this
specific case and fail back such HTLCs when deserializing a
`ChannelManager`
2022-12-05 20:27:35 +00:00
Matt Corallo
0bb87ddad7 Avoid attempting to forward to a closed chan on stale-data reload
If, after forwarding a payment to our counterparty, we restart with
a ChannelMonitor update having been persisted, but the
corresponding ChannelManager update was not persisted, we'll still
have the forwarded HTLC in the `forward_htlcs` map on start. This
will cause us to generate a (spurious) `PendingHTLCsForwardable`
event. However, when we go to forward said HTLC, we'll notice the
channel has been closed and leave it up to the `ChannelMontior` to
finalize the HTLC.

This is all fine today - we won't lose any funds, we'll just
generate an excess forwardable event and then fail to forward.
However, in the future when we allow for forward-time channel
changes this could break. Thus, its worth adding tests for this
behavior today, and, while we're at it, removing the spurious
forwardable HTLCs event.
2022-12-05 20:27:35 +00:00
Elias Rohrer
22d74bf28b
Rename PaymentReceived to PaymentClaimable 2022-12-01 09:39:33 +01:00
Matt Corallo
7e9b88a5cd Wait to free the holding cell during channel_reestablish handling
When we process a `channel_reestablish` message we free the HTLC
update holding cell as things may have changed while we were
disconnected. However, some time ago, to handle freeing from the
holding cell when a monitor update completes, we added a holding
cell freeing check in `get_and_clear_pending_msg_events`. This
leaves the in-`channel_reestablish` holding cell clear redundant,
as doing it immediately or is `get_and_clear_pending_msg_events` is
not a user-visible difference.

Thus, we remove the redundant code here, substantially simplifying
`handle_chan_restoration_locked` while we're at it.
2022-11-17 17:57:17 +00:00
Matt Corallo
97b210dd97 Move restart-related tests to their own file 2022-11-15 22:38:12 +00:00