Force enqueue second forward event for phantom receives

This was a regression resulting from f2453b7 since we now process events
in a loop until there aren't any left. Phantom receives rely on two
calls to `ChannelManager::process_pending_htlc_forwards` before emiting
the `PaymentClaimable` event, providing better privacy to make them look
"real" by taking more time.

Author: wpaulino

lightning-invoice/src/utils.rs

@@ -792,12 +792,13 @@ fn prefer_current_channel(min_inbound_capacity_msat: Option<u64>, current_channe
 
 #[cfg(test)]
 mod test {
+	use core::cell::RefCell;
 	use core::time::Duration;
 	use crate::{Currency, Description, InvoiceDescription, SignOrCreationError, CreationError};
 	use bitcoin_hashes::{Hash, sha256};
 	use bitcoin_hashes::sha256::Hash as Sha256;
 	use lightning::sign::PhantomKeysManager;
-	use lightning::events::{MessageSendEvent, MessageSendEventsProvider, Event};
+	use lightning::events::{MessageSendEvent, MessageSendEventsProvider, Event, EventsProvider};
 	use lightning::ln::{PaymentPreimage, PaymentHash};
 	use lightning::ln::channelmanager::{PhantomRouteHints, MIN_FINAL_CLTV_EXPIRY_DELTA, PaymentId, RecipientOnionFields, Retry};
 	use lightning::ln::functional_test_utils::*;
@@ -1357,13 +1358,20 @@ mod test {
 		// Note that we have to "forward pending HTLCs" twice before we see the PaymentClaimable as
 		// this "emulates" the payment taking two hops, providing some privacy to make phantom node
 		// payments "look real" by taking more time.
-		expect_pending_htlcs_forwardable_ignore!(nodes[fwd_idx]);
-		nodes[fwd_idx].node.process_pending_htlc_forwards();
-		expect_pending_htlcs_forwardable_ignore!(nodes[fwd_idx]);
-		nodes[fwd_idx].node.process_pending_htlc_forwards();
+		let other_events = RefCell::new(Vec::new());
+		let forward_event_handler = |event: Event| {
+			if let Event::PendingHTLCsForwardable { .. } = event {
+				nodes[fwd_idx].node.process_pending_htlc_forwards();
+			} else {
+				other_events.borrow_mut().push(event);
+			}
+		};
+		nodes[fwd_idx].node.process_pending_events(&forward_event_handler);
+		nodes[fwd_idx].node.process_pending_events(&forward_event_handler);
 
 		let payment_preimage_opt = if user_generated_pmt_hash { None } else { Some(payment_preimage) };
-		expect_payment_claimable!(&nodes[fwd_idx], payment_hash, payment_secret, payment_amt, payment_preimage_opt, invoice.recover_payee_pub_key());
+		assert_eq!(other_events.borrow().len(), 1);
+		check_payment_claimable(&other_events.borrow()[0], payment_hash, payment_secret, payment_amt, payment_preimage_opt, invoice.recover_payee_pub_key());
 		do_claim_payment_along_route(&nodes[0], &[&vec!(&nodes[fwd_idx])[..]], false, payment_preimage);
 		let events = nodes[0].node.get_and_clear_pending_events();
 		assert_eq!(events.len(), 2);

lightning/src/ln/channelmanager.rs

@@ -5607,14 +5607,21 @@ where
 	fn forward_htlcs(&self, per_source_pending_forwards: &mut [(u64, OutPoint, u128, Vec<(PendingHTLCInfo, u64)>)]) {
 		for &mut (prev_short_channel_id, prev_funding_outpoint, prev_user_channel_id, ref mut pending_forwards) in per_source_pending_forwards {
 			let mut push_forward_event = false;
+			let mut has_phantom_receive = false;
 			let mut new_intercept_events = VecDeque::new();
 			let mut failed_intercept_forwards = Vec::new();
 			if !pending_forwards.is_empty() {
 				for (forward_info, prev_htlc_id) in pending_forwards.drain(..) {
 					let scid = match forward_info.routing {
 						PendingHTLCRouting::Forward { short_channel_id, .. } => short_channel_id,
-						PendingHTLCRouting::Receive { .. } => 0,
-						PendingHTLCRouting::ReceiveKeysend { .. } => 0,
+						PendingHTLCRouting::Receive { .. } => {
+							has_phantom_receive = true;
+							0
+						},
+						PendingHTLCRouting::ReceiveKeysend { .. } => {
+							has_phantom_receive = true;
+							0
+						},
 					};
 					// Pull this now to avoid introducing a lock order with `forward_htlcs`.
 					let is_our_scid = self.short_to_chan_info.read().unwrap().contains_key(&scid);
@@ -5682,7 +5689,22 @@ where
 				let mut events = self.pending_events.lock().unwrap();
 				events.append(&mut new_intercept_events);
 			}
-			if push_forward_event { self.push_pending_forwards_ev() }
+			if push_forward_event {
+				let mut pending_events = self.pending_events.lock().unwrap();
+				let num_forward_events = pending_events.iter().filter(|(ev, _)|
+					if let Event::PendingHTLCsForwardable { .. } = ev { true } else { false }
+				).count();
+				// Processing events is done in batches and they are not removed until we're done
+				// processing each batch. Since handling a `PendingHTLCsForwardable` event will call
+				// back into the `ChannelManager`, we'll still see the original forwarding event not
+				// removed. Phantom payments will need an additional forwarding event before being
+				// claimed to make them look real by taking more time.
+				if (has_phantom_receive && num_forward_events <= 1) || num_forward_events < 1 {
+					pending_events.push_back((Event::PendingHTLCsForwardable {
+						time_forwardable: Duration::from_millis(MIN_HTLC_RELAY_HOLDING_CELL_MILLIS),
+					}, None));
+				}
+			}
 		}
 	}
 

lightning/src/ln/functional_test_utils.rs

@@ -1806,6 +1806,28 @@ macro_rules! get_route_and_payment_hash {
 	}}
 }
 
+pub fn check_payment_claimable(
+	event: &Event, expected_payment_hash: PaymentHash, expected_payment_secret: PaymentSecret,
+	expected_recv_value: u64, expected_payment_preimage: Option<PaymentPreimage>,
+	expected_receiver_node_id: PublicKey,
+) {
+	match event {
+		Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, .. } => {
+			assert_eq!(expected_payment_hash, *payment_hash);
+			assert_eq!(expected_recv_value, *amount_msat);
+			assert_eq!(expected_receiver_node_id, receiver_node_id.unwrap());
+			match purpose {
+				PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
+					assert_eq!(&expected_payment_preimage, payment_preimage);
+					assert_eq!(expected_payment_secret, *payment_secret);
+				},
+				_ => {},
+			}
+		},
+		_ => panic!("Unexpected event"),
+	}
+}
+
 #[macro_export]
 #[cfg(any(test, ldk_bench, feature = "_test_utils"))]
 macro_rules! expect_payment_claimable {
@@ -1815,22 +1837,8 @@ macro_rules! expect_payment_claimable {
 	($node: expr, $expected_payment_hash: expr, $expected_payment_secret: expr, $expected_recv_value: expr, $expected_payment_preimage: expr, $expected_receiver_node_id: expr) => {
 		let events = $node.node.get_and_clear_pending_events();
 		assert_eq!(events.len(), 1);
-		match events[0] {
-			$crate::events::Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, .. } => {
-				assert_eq!($expected_payment_hash, *payment_hash);
-				assert_eq!($expected_recv_value, amount_msat);
-				assert_eq!($expected_receiver_node_id, receiver_node_id.unwrap());
-				match purpose {
-					$crate::events::PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
-						assert_eq!(&$expected_payment_preimage, payment_preimage);
-						assert_eq!($expected_payment_secret, *payment_secret);
-					},
-					_ => {},
-				}
-			},
-			_ => panic!("Unexpected event"),
-		}
-	}
+		$crate::ln::functional_test_utils::check_payment_claimable(&events[0], $expected_payment_hash, $expected_payment_secret, $expected_recv_value, $expected_payment_preimage, $expected_receiver_node_id)
+	};
 }
 
 #[macro_export]