Fix a debug panic caused by receiving MPP parts after a failure 2022-01-fix-double-fail-panic
authorMatt Corallo <git@bluematt.me>
Wed, 19 Jan 2022 03:52:56 +0000 (03:52 +0000)
committerMatt Corallo <git@bluematt.me>
Wed, 16 Feb 2022 21:40:11 +0000 (21:40 +0000)
commitbe57e828b8452ac8cfe12c6bcd8c2afd7e1f02a5
tree060cdbb675fa4a573045dfa2b7a2e68336407892
parent0df247d6328ea78a6c1daa3d01f5c311e82f06a6
Fix a debug panic caused by receiving MPP parts after a failure

Prior to cryptographic payment secrets, when we process a received
payment in `process_pending_htlc_fowards` we'd remove its entry
from the `pending_inbound_payments` map and give the user a
`PaymentReceived` event.

Thereafter, if a second HTLC came in with the same payment hash, it
would find no entry in the `pending_inbound_payments` map and be
immediately failed in `process_pending_htlc_forwards`.

Thus, each HTLC will either result in a `PaymentReceived` event or
be failed, with no possibility for both.

As of 846487555556d8465c5b7b811f976e78f265c48f, we no longer
materially have a pending-inbound-payments map, and thus
more-than-happily accept a second payment with the same payment
hash even if we just failed a previous one for having mis-matched
payment data.

This can cause an issue if the two HTLCs are received back-to-back,
with the first being accepted as valid, generating a
`PaymentReceived` event. Then, when the second comes in we'll hit
the "total value {} ran over expected value" condition and fail
*all* pending HTLCs with the same payment hash. At this point,
we'll have a pending failure for both HTLCs, as well as a
`PaymentReceived` event for the user.

Thereafter, if the user attempts to fail the HTLC in response to
the `PaymentReceived`, they'll get a debug panic at channel.rs:1657
'Tried to fail an HTLC that was already failed'.

The solution is to avoid bulk-failing all pending HTLCs for a
payment. This feels like the right thing to do anyway - if a sender
accidentally sends an extra HTLC after a payment has ben fully
paid, we shouldn't fail the entire payment.

Found by the `chanmon_consistency` fuzz test.
lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_tests.rs