Handle mon update completion actions even with update(s) is blocked

If we complete a `ChannelMonitorUpdate` persistence but there are
blocked `ChannelMonitorUpdate`s in the channel, we'll skip all the
post-monitor-update logic entirely. While its correct that we can't
resume the channel (as it expected the monitor updates it generated
to complete, even if they ended up blocked), the post-update
actions are a `channelmanager.rs` concept - they cannot be tied to
blocked updates because `channelmanager.rs` doesn't even see
blocked updates.

This can lead to a channel getting stuck waiting on itself. In a
production environment, an LDK user saw a case where:
 (a) an MPP payment was received over several channels, let's call
     them A + B.
 (b) channel B got into `AwaitingRAA` due to unrelated operations,
 (c) the MPP payment was claimed, with async monitor updating,
 (d) the `revoke_and_ack` we were waiting on was delivered, but the
     resulting `ChannelMonitorUpdate` was blocked due to the
     pending claim having inserted an RAA-blocking action,
 (e) the preimage `ChannelMonitorUpdate` generated for channel B
     completed persistence, which did nothing due to the blocked
     `ChannelMonitorUpdate`.
 (f) the `Event::PaymentClaimed` event was handled but it, too,
     failed to unblock the channel.

Instead, here, we simply process post-update actions when an update
completes, even if there are pending blocked updates. We do not
fully unblock the channel, of course.

Backport of 8f4a4d2

Conflicts due to upstream macro rewrite resolved in:
 * lightning/src/ln/channelmanager.rs

Author: TheBlueMatt

lightning/src/ln/chanmon_update_fail_tests.rs

@@ -19,12 +19,13 @@ use crate::chain::transaction::OutPoint;
 use crate::chain::{ChannelMonitorUpdateStatus, Listen, Watch};
 use crate::events::{ClosureReason, Event, HTLCHandlingFailureType, PaymentPurpose};
 use crate::ln::channel::AnnouncementSigsState;
-use crate::ln::channelmanager::{PaymentId, RAACommitmentOrder, RecipientOnionFields};
+use crate::ln::channelmanager::{PaymentId, RAACommitmentOrder, RecipientOnionFields, Retry};
 use crate::ln::msgs;
 use crate::ln::msgs::{
 	BaseMessageHandler, ChannelMessageHandler, MessageSendEvent, RoutingMessageHandler,
 };
 use crate::ln::types::ChannelId;
+use crate::routing::router::{PaymentParameters, RouteParameters};
 use crate::sign::NodeSigner;
 use crate::util::native_async::FutureQueue;
 use crate::util::persist::{
@@ -3542,7 +3543,7 @@ fn do_test_blocked_chan_preimage_release(completion_mode: BlockedUpdateComplMode
 		assert!(a.is_none());
 
 		nodes[1].node.handle_revoke_and_ack(node_a_id, &raa);
-		check_added_monitors(&nodes[1], 0);
+		check_added_monitors(&nodes[1], 1);
 		assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
 	}
 
@@ -3561,8 +3562,8 @@ fn do_test_blocked_chan_preimage_release(completion_mode: BlockedUpdateComplMode
 		panic!();
 	}
 
-	// The event processing should release the last RAA updates on both channels.
-	check_added_monitors(&nodes[1], 2);
+	// The event processing should release the last RAA update.
+	check_added_monitors(&nodes[1], 1);
 
 	// When we fetch the next update the message getter will generate the next update for nodes[2],
 	// generating a further monitor update.
@@ -5058,3 +5059,111 @@ fn native_async_persist() {
 		panic!();
 	}
 }
+
+#[test]
+fn test_mpp_claim_to_holding_cell() {
+	// Previously, if an MPP payment was claimed while one channel was AwaitingRAA (causing the
+	// HTLC claim to go into the holding cell), and the RAA came in before the async monitor
+	// update with the preimage completed, the channel could hang waiting on itself.
+	// This tests that behavior.
+	let chanmon_cfgs = create_chanmon_cfgs(4);
+	let node_cfgs = create_node_cfgs(4, &chanmon_cfgs);
+	let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, None, None]);
+	let nodes = create_network(4, &node_cfgs, &node_chanmgrs);
+
+	let node_b_id = nodes[1].node.get_our_node_id();
+	let node_c_id = nodes[2].node.get_our_node_id();
+	let node_d_id = nodes[3].node.get_our_node_id();
+
+	// First open channels in a diamond and deliver the MPP payment.
+	let chan_1_scid = create_announced_chan_between_nodes(&nodes, 0, 1).0.contents.short_channel_id;
+	let chan_2_scid = create_announced_chan_between_nodes(&nodes, 0, 2).0.contents.short_channel_id;
+	let (chan_3_update, _, chan_3_id, ..) = create_announced_chan_between_nodes(&nodes, 1, 3);
+	let chan_3_scid = chan_3_update.contents.short_channel_id;
+	let (chan_4_update, _, chan_4_id, ..) = create_announced_chan_between_nodes(&nodes, 2, 3);
+	let chan_4_scid = chan_4_update.contents.short_channel_id;
+
+	let (mut route, paymnt_hash_1, preimage_1, payment_secret) =
+		get_route_and_payment_hash!(&nodes[0], nodes[3], 500_000);
+	let path = route.paths[0].clone();
+	route.paths.push(path);
+	route.paths[0].hops[0].pubkey = node_b_id;
+	route.paths[0].hops[0].short_channel_id = chan_1_scid;
+	route.paths[0].hops[1].short_channel_id = chan_3_scid;
+	route.paths[0].hops[1].fee_msat = 250_000;
+	route.paths[1].hops[0].pubkey = node_c_id;
+	route.paths[1].hops[0].short_channel_id = chan_2_scid;
+	route.paths[1].hops[1].short_channel_id = chan_4_scid;
+	route.paths[1].hops[1].fee_msat = 250_000;
+	let paths = &[&[&nodes[1], &nodes[3]][..], &[&nodes[2], &nodes[3]][..]];
+	send_along_route_with_secret(&nodes[0], route, paths, 500_000, paymnt_hash_1, payment_secret);
+
+	// Put the C <-> D channel into AwaitingRaa
+	let (preimage_2, paymnt_hash_2, payment_secret_2) = get_payment_preimage_hash!(nodes[3]);
+	let onion = RecipientOnionFields::secret_only(payment_secret_2);
+	let id = PaymentId([42; 32]);
+	let pay_params = PaymentParameters::from_node_id(node_d_id, TEST_FINAL_CLTV);
+	let route_params = RouteParameters::from_payment_params_and_value(pay_params, 400_000);
+	nodes[2].node.send_payment(paymnt_hash_2, onion, id, route_params, Retry::Attempts(0)).unwrap();
+	check_added_monitors(&nodes[2], 1);
+
+	let mut payment_event = SendEvent::from_node(&nodes[2]);
+	nodes[3].node.handle_update_add_htlc(node_c_id, &payment_event.msgs[0]);
+	nodes[3].node.handle_commitment_signed_batch_test(node_c_id, &payment_event.commitment_msg);
+	check_added_monitors(&nodes[3], 1);
+
+	let (raa, cs) = get_revoke_commit_msgs(&nodes[3], &node_c_id);
+	nodes[2].node.handle_revoke_and_ack(node_d_id, &raa);
+	check_added_monitors(&nodes[2], 1);
+
+	nodes[2].node.handle_commitment_signed_batch_test(node_d_id, &cs);
+	check_added_monitors(&nodes[2], 1);
+
+	let cs_raa = get_event_msg!(nodes[2], MessageSendEvent::SendRevokeAndACK, node_d_id);
+
+	// Now claim the payment, completing both channel monitor updates async
+	// In the current code, the C <-> D channel happens to be the `durable_preimage_channel`,
+	// improving coverage somewhat but it isn't strictly critical to the test.
+	chanmon_cfgs[3].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
+	chanmon_cfgs[3].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
+	nodes[3].node.claim_funds(preimage_1);
+	check_added_monitors(&nodes[3], 2);
+
+	// Complete the B <-> D monitor update, freeing the first fulfill.
+	let (latest_id, _) = get_latest_mon_update_id(&nodes[3], chan_3_id);
+	nodes[3].chain_monitor.chain_monitor.channel_monitor_updated(chan_3_id, latest_id).unwrap();
+	let mut b_claim = get_htlc_update_msgs(&nodes[3], &node_b_id);
+
+	// When we deliver the pre-claim RAA, node D will shove the monitor update into the blocked
+	// state since we have a pending MPP payment which is blocking RAA monitor updates.
+	nodes[3].node.handle_revoke_and_ack(node_c_id, &cs_raa);
+	check_added_monitors(&nodes[3], 0);
+
+	// Finally, complete the C <-> D monitor update. Previously, this unlock failed to be processed
+	// due to the existence of the blocked RAA update above.
+	let (latest_id, _) = get_latest_mon_update_id(&nodes[3], chan_4_id);
+	nodes[3].chain_monitor.chain_monitor.channel_monitor_updated(chan_4_id, latest_id).unwrap();
+	// Once we process monitor events (in this case by checking for the `PaymentClaimed` event, the
+	// RAA monitor update blocked above will be released.
+	expect_payment_claimed!(nodes[3], paymnt_hash_1, 500_000);
+	check_added_monitors(&nodes[3], 1);
+
+	// After the RAA monitor update completes, the C <-> D channel will be able to generate its
+	// fulfill updates as well.
+	let mut c_claim = get_htlc_update_msgs(&nodes[3], &node_c_id);
+	check_added_monitors(&nodes[3], 1);
+
+	// Finally, clear all the pending payments.
+	let path = [&[&nodes[1], &nodes[3]][..], &[&nodes[2], &nodes[3]][..]];
+	let mut args = ClaimAlongRouteArgs::new(&nodes[0], &path[..], preimage_1);
+	let b_claim_msgs = (b_claim.update_fulfill_htlcs.pop().unwrap(), b_claim.commitment_signed);
+	let c_claim_msgs = (c_claim.update_fulfill_htlcs.pop().unwrap(), c_claim.commitment_signed);
+	let claims = vec![(b_claim_msgs, node_b_id), (c_claim_msgs, node_c_id)];
+	pass_claimed_payment_along_route_from_ev(250_000, claims, args);
+
+	expect_payment_sent(&nodes[0], preimage_1, None, true, true);
+
+	expect_and_process_pending_htlcs(&nodes[3], false);
+	expect_payment_claimable!(nodes[3], paymnt_hash_2, payment_secret_2, 400_000);
+	claim_payment(&nodes[2], &[&nodes[3]], preimage_2);
+}