Correctly fail back blinded HTLCs when they're failed back to us

Author: valentinewallace

lightning/src/ln/blinded_payment_tests.rs

@@ -10,12 +10,14 @@
 use bitcoin::secp256k1::Secp256k1;
 use crate::blinded_path::BlindedPath;
 use crate::blinded_path::payment::{ForwardTlvs, PaymentConstraints, PaymentRelay, ReceiveTlvs};
-use crate::events::MessageSendEventsProvider;
+use crate::events::{Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider};
 use crate::ln::channelmanager;
 use crate::ln::channelmanager::{PaymentId, RecipientOnionFields, RetryableSendFailure};
 use crate::ln::features::{BlindedHopFeatures, Bolt12InvoiceFeatures};
 use crate::ln::functional_test_utils::*;
+use crate::ln::msgs;
 use crate::ln::msgs::ChannelMessageHandler;
+use crate::ln::onion_utils::INVALID_ONION_BLINDING;
 use crate::ln::outbound_payment::Retry;
 use crate::prelude::*;
 use crate::routing::router::{PaymentParameters, RouteParameters};
@@ -439,3 +441,124 @@ fn high_prop_fees() {
 	pass_along_route(&nodes[0], &[&[&nodes[1], &nodes[2], &nodes[3], &nodes[4]]], amt_msat, payment_hash, payment_secret);
 	claim_payment(&nodes[0], &[&nodes[1], &nodes[2], &nodes[3], &nodes[4]], payment_preimage);
 }
+
+#[test]
+fn fail_blinded_payment() {
+	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);
+	create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0);
+	create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 0);
+	let chan_upd = create_announced_chan_between_nodes_with_value(&nodes, 2, 3, 1_000_000, 0).0.contents;
+
+	let amt_msat = 5000;
+	let (_, payment_hash, payment_secret) = get_payment_preimage_hash(&nodes[3], Some(amt_msat), None);
+	let intermediate_nodes = vec![(nodes[2].node.get_our_node_id(), ForwardTlvs {
+		short_channel_id: chan_upd.short_channel_id,
+		payment_relay: PaymentRelay {
+			cltv_expiry_delta: chan_upd.cltv_expiry_delta,
+			fee_proportional_millionths: chan_upd.fee_proportional_millionths,
+			fee_base_msat: chan_upd.fee_base_msat,
+		},
+		payment_constraints: PaymentConstraints {
+			max_cltv_expiry: u32::max_value(),
+			htlc_minimum_msat: chan_upd.htlc_minimum_msat,
+		},
+		features: BlindedHopFeatures::empty(),
+	})];
+	let payee_tlvs = ReceiveTlvs {
+		payment_secret,
+		payment_constraints: PaymentConstraints {
+			max_cltv_expiry: u32::max_value(),
+			htlc_minimum_msat: chan_upd.htlc_minimum_msat,
+		},
+	};
+	let mut secp_ctx = Secp256k1::new();
+	let blinded_path = BlindedPath::new_for_payment(
+		&intermediate_nodes[..], nodes[3].node.get_our_node_id(), payee_tlvs,
+		chan_upd.htlc_maximum_msat, &chanmon_cfgs[3].keys_manager, &secp_ctx
+	).unwrap();
+
+	let route_params = RouteParameters {
+		payment_params: PaymentParameters::blinded(vec![blinded_path]),
+		final_value_msat: amt_msat
+	};
+	nodes[0].node.send_payment(payment_hash, RecipientOnionFields::spontaneous_empty(),
+		PaymentId(payment_hash.0), route_params, Retry::Attempts(0)).unwrap();
+	check_added_monitors(&nodes[0], 1);
+	pass_along_route(&nodes[0], &[&[&nodes[1], &nodes[2], &nodes[3]]], amt_msat, payment_hash,
+		payment_secret);
+
+	nodes[3].node.fail_htlc_backwards(&payment_hash);
+	expect_pending_htlcs_forwardable_conditions(nodes[3].node.get_and_clear_pending_events(),
+		&[HTLCDestination::FailedPayment { payment_hash }]);
+	nodes[3].node.process_pending_htlc_forwards();
+
+	// The last node should fail back with malformed since it's not the intro node.
+	check_added_monitors!(nodes[3], 1);
+	let (update_fail_malformed, commitment_signed) = {
+		let msg_events = nodes[3].node.get_and_clear_pending_msg_events();
+		assert_eq!(msg_events.len(), 1);
+		match msg_events[0] {
+			MessageSendEvent::UpdateHTLCs { updates: msgs::CommitmentUpdate {
+				ref update_fail_malformed_htlcs, ref commitment_signed, ..
+			}, .. } => {
+				assert_eq!(update_fail_malformed_htlcs.len(), 1);
+				(update_fail_malformed_htlcs[0].clone(), commitment_signed.clone())
+			},
+			_ => panic!("Unexpected event"),
+		}
+	};
+	assert_eq!(update_fail_malformed.sha256_of_onion, [0; 32]);
+	assert_eq!(update_fail_malformed.failure_code, INVALID_ONION_BLINDING);
+	nodes[2].node.handle_update_fail_malformed_htlc(&nodes[3].node.get_our_node_id(), &update_fail_malformed);
+	do_commitment_signed_dance(&nodes[2], &nodes[3], &commitment_signed, true, false);
+
+	// The intro node fails back with the invalid_onion_blinding error.
+	let (update_fail, commitment_signed) = {
+		let msg_events = nodes[2].node.get_and_clear_pending_msg_events();
+		assert_eq!(msg_events.len(), 1);
+		match msg_events[0] {
+			MessageSendEvent::UpdateHTLCs { updates: msgs::CommitmentUpdate {
+				ref update_fail_htlcs, ref commitment_signed, ..
+			}, .. } => {
+				assert_eq!(update_fail_htlcs.len(), 1);
+				(update_fail_htlcs[0].clone(), commitment_signed.clone())
+			},
+			_ => panic!("Unexpected event"),
+		}
+	};
+	nodes[1].node.handle_update_fail_htlc(&nodes[2].node.get_our_node_id(), &update_fail);
+	do_commitment_signed_dance(&nodes[1], &nodes[2], &commitment_signed, true, false);
+
+	let (final_update_fail, commitment_signed) = {
+		let msg_events = nodes[1].node.get_and_clear_pending_msg_events();
+		assert_eq!(msg_events.len(), 1);
+		match msg_events[0] {
+			MessageSendEvent::UpdateHTLCs { updates: msgs::CommitmentUpdate {
+				ref update_fail_htlcs, ref commitment_signed, ..
+			}, .. } => {
+				assert_eq!(update_fail_htlcs.len(), 1);
+				(update_fail_htlcs[0].clone(), commitment_signed.clone())
+			},
+			_ => panic!("Unexpected event"),
+		}
+	};
+	nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &final_update_fail);
+	do_commitment_signed_dance(&nodes[0], &nodes[1], &commitment_signed, false, false);
+	let failure_evs = nodes[0].node.get_and_clear_pending_events();
+	assert_eq!(failure_evs.len(), 2);
+	match &failure_evs[0] {
+		Event::PaymentPathFailed {
+			payment_hash: ref ev_payment_hash, payment_failed_permanently, ref error_code, ref error_data,
+			..
+		} => {
+			assert_eq!(payment_hash, *ev_payment_hash);
+			assert!(!payment_failed_permanently);
+			assert_eq!(error_code, &Some(INVALID_ONION_BLINDING));
+			assert_eq!(error_data, &Some(vec![0; 32]));
+		},
+			_ => panic!("Unexpected event"),
+	}
+}

lightning/src/ln/channel.rs

@@ -30,7 +30,7 @@ use crate::ln::script::{self, ShutdownScript};
 use crate::ln::channelmanager::{self, CounterpartyForwardingInfo, PendingHTLCStatus, HTLCSource, SentHTLCId, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT, ChannelShutdownState};
 use crate::ln::chan_utils::{CounterpartyCommitmentSecrets, TxCreationKeys, HTLCOutputInCommitment, htlc_success_tx_weight, htlc_timeout_tx_weight, make_funding_redeemscript, ChannelPublicKeys, CommitmentTransaction, HolderCommitmentTransaction, ChannelTransactionParameters, CounterpartyChannelTransactionParameters, MAX_HTLCS, get_commitment_transaction_number_obscure_factor, ClosingTransaction};
 use crate::ln::chan_utils;
-use crate::ln::onion_utils::HTLCFailReason;
+use crate::ln::onion_utils::{HTLCFailReason, INVALID_ONION_BLINDING};
 use crate::chain::BestBlock;
 use crate::chain::chaininterface::{FeeEstimator, ConfirmationTarget, LowerBoundedFeeEstimator};
 use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, LATENCY_GRACE_PERIOD_BLOCKS, CLOSED_CHANNEL_UPDATE_ID};
@@ -248,6 +248,9 @@ enum HTLCUpdateAwaitingACK {
 		htlc_id: u64,
 		err_packet: msgs::OnionErrorPacket,
 	},
+	FailMalformedHTLC {
+		htlc_id: u64,
+	},
 }
 
 /// There are a few "states" and then a number of flags which can be applied:
@@ -2055,6 +2058,25 @@ struct CommitmentTxInfoCached {
 	feerate: u32,
 }
 
+trait FailHTLCMessage {
+	fn new(htlc_id: u64, channel_id: ChannelId, reason: msgs::OnionErrorPacket) -> Self;
+}
+impl FailHTLCMessage for msgs::UpdateFailHTLC {
+	fn new(htlc_id: u64, channel_id: ChannelId, reason: msgs::OnionErrorPacket) -> Self {
+		msgs::UpdateFailHTLC { htlc_id, channel_id, reason }
+	}
+}
+impl FailHTLCMessage for msgs::UpdateFailMalformedHTLC {
+	fn new(htlc_id: u64, channel_id: ChannelId, _reason: msgs::OnionErrorPacket) -> Self {
+		msgs::UpdateFailMalformedHTLC {
+			htlc_id,
+			channel_id,
+			sha256_of_onion: [0; 32],
+			failure_code: INVALID_ONION_BLINDING,
+		}
+	}
+}
+
 impl<SP: Deref> Channel<SP> where
 	SP::Target: SignerProvider,
 	<SP::Target as SignerProvider>::Signer: WriteableEcdsaChannelSigner
@@ -2279,7 +2301,9 @@ impl<SP: Deref> Channel<SP> where
 							return UpdateFulfillFetch::DuplicateClaim {};
 						}
 					},
-					&HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } => {
+					&HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } |
+						&HTLCUpdateAwaitingACK::FailMalformedHTLC { htlc_id, .. } =>
+					{
 						if htlc_id_arg == htlc_id {
 							log_warn!(logger, "Have preimage and want to fulfill HTLC with pending failure against channel {}", &self.context.channel_id());
 							// TODO: We may actually be able to switch to a fulfill here, though its
@@ -2372,7 +2396,15 @@ impl<SP: Deref> Channel<SP> where
 	/// [`ChannelError::Ignore`].
 	pub fn queue_fail_htlc<L: Deref>(&mut self, htlc_id_arg: u64, err_packet: msgs::OnionErrorPacket, logger: &L)
 	-> Result<(), ChannelError> where L::Target: Logger {
-		self.fail_htlc(htlc_id_arg, err_packet, true, logger)
+		self.fail_htlc::<L, msgs::UpdateFailHTLC>(htlc_id_arg, Some(err_packet), true, logger)
+			.map(|msg_opt| assert!(msg_opt.is_none(), "We forced holding cell?"))
+	}
+
+	/// Used for failing back blinded payments if we are not the intro node. See
+	/// [`Self::queue_fail_htlc`] for more info.
+	pub fn queue_fail_blinded_intermed_htlc<L: Deref>(&mut self, htlc_id_arg: u64, logger: &L)
+	-> Result<(), ChannelError> where L::Target: Logger {
+		self.fail_htlc::<L, msgs::UpdateFailMalformedHTLC>(htlc_id_arg, None, true, logger)
 			.map(|msg_opt| assert!(msg_opt.is_none(), "We forced holding cell?"))
 	}
 
@@ -2384,8 +2416,10 @@ impl<SP: Deref> Channel<SP> where
 	/// If we do fail twice, we `debug_assert!(false)` and return `Ok(None)`. Thus, this will always
 	/// return `Ok(_)` if preconditions are met. In any case, `Err`s will only be
 	/// [`ChannelError::Ignore`].
-	fn fail_htlc<L: Deref>(&mut self, htlc_id_arg: u64, err_packet: msgs::OnionErrorPacket, mut force_holding_cell: bool, logger: &L)
-	-> Result<Option<msgs::UpdateFailHTLC>, ChannelError> where L::Target: Logger {
+	fn fail_htlc<L: Deref, M: FailHTLCMessage>(
+		&mut self, htlc_id_arg: u64, err_packet: Option<msgs::OnionErrorPacket>,
+		mut force_holding_cell: bool, logger: &L
+	) -> Result<Option<M>, ChannelError> where L::Target: Logger {
 		if (self.context.channel_state & (ChannelState::ChannelReady as u32)) != (ChannelState::ChannelReady as u32) {
 			panic!("Was asked to fail an HTLC when channel was not in an operational state");
 		}
@@ -2439,7 +2473,9 @@ impl<SP: Deref> Channel<SP> where
 							return Ok(None);
 						}
 					},
-					&HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } => {
+					&HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } |
+						&HTLCUpdateAwaitingACK::FailMalformedHTLC { htlc_id } =>
+					{
 						if htlc_id_arg == htlc_id {
 							debug_assert!(false, "Tried to fail an HTLC that was already failed");
 							return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID".to_owned()));
@@ -2448,25 +2484,34 @@ impl<SP: Deref> Channel<SP> where
 					_ => {}
 				}
 			}
-			log_trace!(logger, "Placing failure for HTLC ID {} in holding cell in channel {}.", htlc_id_arg, &self.context.channel_id());
-			self.context.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::FailHTLC {
-				htlc_id: htlc_id_arg,
-				err_packet,
-			});
+			log_trace!(logger, "Placing failure for HTLC ID {} in holding cell in channel {}.", htlc_id_arg, self.context.channel_id());
+			if let Some(err_packet) = err_packet {
+				self.context.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::FailHTLC {
+					htlc_id: htlc_id_arg,
+					err_packet,
+				});
+			} else {
+				self.context.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::FailMalformedHTLC {
+					htlc_id: htlc_id_arg,
+				});
+			}
 			return Ok(None);
 		}
 
-		log_trace!(logger, "Failing HTLC ID {} back with a update_fail_htlc message in channel {}.", htlc_id_arg, &self.context.channel_id());
+		log_trace!(logger, "Failing HTLC ID {} back with a update_fail_{}htlc message in channel {}.",
+			htlc_id_arg, if err_packet.is_none() { "malformed_" } else { "" }, self.context.channel_id());
 		{
 			let htlc = &mut self.context.pending_inbound_htlcs[pending_idx];
-			htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailRelay(err_packet.clone()));
+			if let Some(ref err_packet) = err_packet  {
+				htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailRelay(err_packet.clone()));
+			} else {
+				htlc.state = InboundHTLCState::LocalRemoved(
+					InboundHTLCRemovalReason::FailMalformed(([0; 32], INVALID_ONION_BLINDING)));
+			}
 		}
 
-		Ok(Some(msgs::UpdateFailHTLC {
-			channel_id: self.context.channel_id(),
-			htlc_id: htlc_id_arg,
-			reason: err_packet
-		}))
+		let err_packet = err_packet.unwrap_or(msgs::OnionErrorPacket { data: Vec::new()});
+		Ok(Some(M::new(htlc_id_arg, self.context.channel_id(), err_packet)))
 	}
 
 	// Message handlers:
@@ -3164,7 +3209,9 @@ impl<SP: Deref> Channel<SP> where
 						monitor_update.updates.append(&mut additional_monitor_update.updates);
 					},
 					&HTLCUpdateAwaitingACK::FailHTLC { htlc_id, ref err_packet } => {
-						match self.fail_htlc(htlc_id, err_packet.clone(), false, logger) {
+						match self.fail_htlc::<L, msgs::UpdateFailHTLC>(
+							htlc_id, Some(err_packet.clone()), false, logger)
+						{
 							Ok(update_fail_msg_option) => {
 								// If an HTLC failure was previously added to the holding cell (via
 								// `queue_fail_htlc`) then generating the fail message itself must
@@ -3182,6 +3229,22 @@ impl<SP: Deref> Channel<SP> where
 							}
 						}
 					},
+					&HTLCUpdateAwaitingACK::FailMalformedHTLC { htlc_id } => {
+						match self.fail_htlc::<L, msgs::UpdateFailMalformedHTLC>(
+							htlc_id, None, false, logger)
+						{
+							Ok(update_fail_malformed_opt) => {
+								debug_assert!(update_fail_malformed_opt.is_some()); // See above comment
+								update_fail_count += 1;
+							},
+							Err(e) => {
+								if let ChannelError::Ignore(_) = e {}
+								else {
+									panic!("Got a non-IgnoreError action trying to fail holding cell HTLC");
+								}
+							}
+						}
+					},
 				}
 			}
 			if update_add_count == 0 && update_fulfill_count == 0 && update_fail_count == 0 && self.context.holding_cell_update_fee.is_none() {
@@ -6853,6 +6916,10 @@ impl<SP: Deref> Writeable for Channel<SP> where SP::Target: SignerProvider {
 					htlc_id.write(writer)?;
 					err_packet.write(writer)?;
 				}
+				&HTLCUpdateAwaitingACK::FailMalformedHTLC { ref htlc_id } => {
+					3u8.write(writer)?;
+					htlc_id.write(writer)?;
+				}
 			}
 		}
 
@@ -7148,6 +7215,9 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
 					htlc_id: Readable::read(reader)?,
 					err_packet: Readable::read(reader)?,
 				},
+				3 => HTLCUpdateAwaitingACK::FailMalformedHTLC {
+					htlc_id: Readable::read(reader)?,
+				},
 				_ => return Err(DecodeError::InvalidValue),
 			});
 		}