Expand HTLCDestination

Author: valentinewallace

lightning/src/events/mod.rs

@@ -466,8 +466,39 @@ impl_writeable_tlv_based_enum_upgradable!(ClosureReason,
 );
 
 /// Intended destination of a failed HTLC as indicated in [`Event::HTLCHandlingFailed`].
-#[derive(Clone, Debug, PartialEq, Eq)]
+#[derive(Clone, Debug, PartialEq, Eq, Hash)]
 pub enum HTLCDestination {
+	DustLimitReached { holder_commitment: bool },
+	FeeSpikeBuffer,
+	ShutdownSent,
+	PrivateChannelForward,
+	RealSCIDForward,
+	ChannelNotReady,
+	FeeInsufficient,
+	IncorrectCLTVDelta,
+	ChannelDisabled,
+	HTLCBelowMinimum,
+	InvalidKeysendPreimage,
+	MissingPaymentSecret,
+	IncorrectHTLCAmount,
+	CLTVExpiryTooClose,
+	CLTVExpiryTooFar,
+	OutgoingCLTVTooClose,
+	NodeIDForward,
+	DownstreamHopFailed { node_id: Option<PublicKey>, channel_id: ChannelId },
+	NextHopChannelClosed { node_id: Option<PublicKey>, channel_id: ChannelId },
+	FailedPhantomPayment { payment_hash: PaymentHash },
+	AmountExceedsChannelValue {
+		amount_msat: u64,
+		channel_value_msat: u64,
+	},
+	InvalidPaymentAmount {
+		amount_msat: u64,
+		next_outbound_min_msat: u64,
+		next_outbound_max_msat: u64,
+	},
+	BlindedConstraintsViolated,
+	IncorrectFinalCltvExpiry,
 	/// We tried forwarding to a channel but failed to do so. An example of such an instance is when
 	/// there is insufficient capacity in our outbound channel.
 	NextHopChannel {
@@ -507,22 +538,22 @@ pub enum HTLCDestination {
 	},
 }
 
-impl_writeable_tlv_based_enum_upgradable!(HTLCDestination,
-	(0, NextHopChannel) => {
-		(0, node_id, required),
-		(2, channel_id, required),
-	},
-	(1, InvalidForward) => {
-		(0, requested_forward_scid, required),
-	},
-	(2, UnknownNextHop) => {
-		(0, requested_forward_scid, required),
-	},
-	(3, InvalidOnion) => {},
-	(4, FailedPayment) => {
-		(0, payment_hash, required),
-	},
-);
+// impl_writeable_tlv_based_enum_upgradable!(HTLCDestination,
+//   (0, NextHopChannel) => {
+//     (0, node_id, required),
+//     (2, channel_id, required),
+//   },
+//   (1, InvalidForward) => {
+//     (0, requested_forward_scid, required),
+//   },
+//   (2, UnknownNextHop) => {
+//     (0, requested_forward_scid, required),
+//   },
+//   (3, InvalidOnion) => {},
+//   (4, FailedPayment) => {
+//     (0, payment_hash, required),
+//   },
+// );
 
 /// Will be used in [`Event::HTLCIntercepted`] to identify the next hop in the HTLC's path.
 /// Currently only used in serialization for the sake of maintaining compatibility. More variants

lightning/src/ln/channel.rs

@@ -57,7 +57,7 @@ use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, Channel
 use crate::chain::transaction::{OutPoint, TransactionData};
 use crate::sign::ecdsa::EcdsaChannelSigner;
 use crate::sign::{EntropySource, ChannelSigner, SignerProvider, NodeSigner, Recipient};
-use crate::events::{ClosureReason, Event};
+use crate::events::{ClosureReason, Event, HTLCDestination};
 use crate::events::bump_transaction::BASE_INPUT_WEIGHT;
 use crate::routing::gossip::NodeId;
 use crate::util::ser::{Readable, ReadableArgs, TransactionU16LenLimited, Writeable, Writer};
@@ -5888,21 +5888,14 @@ impl<SP: Deref> FundedChannel<SP> where
 								update_add_count += 1;
 							},
 							Err(e) => {
-								match e {
-									ChannelError::Ignore(ref msg) => {
-										log_info!(logger, "Failed to send HTLC with payment_hash {} due to {} in channel {}", &payment_hash, msg, &self.context.channel_id());
-										// If we fail to send here, then this HTLC should
-										// be failed backwards. Failing to send here
-										// indicates that this HTLC may keep being put back
-										// into the holding cell without ever being
-										// successfully forwarded/failed/fulfilled, causing
-										// our counterparty to eventually close on us.
-										htlcs_to_fail.push((source.clone(), *payment_hash));
-									},
-									_ => {
-										panic!("Got a non-IgnoreError action trying to send holding cell HTLC");
-									},
-								}
+								log_info!(logger, "Failed to send HTLC with payment_hash {} due to {:?} in channel {}", &payment_hash, e, &self.context.channel_id());
+								// If we fail to send here, then this HTLC should
+								// be failed backwards. Failing to send here
+								// indicates that this HTLC may keep being put back
+								// into the holding cell without ever being
+								// successfully forwarded/failed/fulfilled, causing
+								// our counterparty to eventually close on us.
+								htlcs_to_fail.push((source.clone(), *payment_hash));
 							}
 						}
 						None
@@ -7580,18 +7573,20 @@ impl<SP: Deref> FundedChannel<SP> where
 
 	fn internal_htlc_satisfies_config(
 		&self, htlc: &msgs::UpdateAddHTLC, amt_to_forward: u64, outgoing_cltv_value: u32, config: &ChannelConfig,
-	) -> Result<(), (&'static str, u16)> {
+	) -> Result<(), (HTLCDestination, &'static str, u16)> {
 		let fee = amt_to_forward.checked_mul(config.forwarding_fee_proportional_millionths as u64)
 			.and_then(|prop_fee| (prop_fee / 1000000).checked_add(config.forwarding_fee_base_msat as u64));
 		if fee.is_none() || htlc.amount_msat < fee.unwrap() ||
 			(htlc.amount_msat - fee.unwrap()) < amt_to_forward {
 			return Err((
+				HTLCDestination::FeeInsufficient,
 				"Prior hop has deviated from specified fees parameters or origin node has obsolete ones",
 				0x1000 | 12, // fee_insufficient
 			));
 		}
 		if (htlc.cltv_expiry as u64) < outgoing_cltv_value as u64 + config.cltv_expiry_delta as u64 {
 			return Err((
+				HTLCDestination::OutgoingCLTVTooClose,
 				"Forwarding node has tampered with the intended HTLC values or origin node has an obsolete cltv_expiry_delta",
 				0x1000 | 13, // incorrect_cltv_expiry
 			));
@@ -7604,7 +7599,7 @@ impl<SP: Deref> FundedChannel<SP> where
 	/// unsuccessful, falls back to the previous one if one exists.
 	pub fn htlc_satisfies_config(
 		&self, htlc: &msgs::UpdateAddHTLC, amt_to_forward: u64, outgoing_cltv_value: u32,
-	) -> Result<(), (&'static str, u16)> {
+	) -> Result<(), (HTLCDestination, &'static str, u16)> {
 		self.internal_htlc_satisfies_config(&htlc, amt_to_forward, outgoing_cltv_value, &self.context.config())
 			.or_else(|err| {
 				if let Some(prev_config) = self.context.prev_config() {
@@ -7619,13 +7614,13 @@ impl<SP: Deref> FundedChannel<SP> where
 	/// this function determines whether to fail the HTLC, or forward / claim it.
 	pub fn can_accept_incoming_htlc<F: Deref, L: Deref>(
 		&self, msg: &msgs::UpdateAddHTLC, fee_estimator: &LowerBoundedFeeEstimator<F>, logger: L
-	) -> Result<(), (&'static str, u16)>
+	) -> Result<(), (HTLCDestination, u16)>
 	where
 		F::Target: FeeEstimator,
 		L::Target: Logger
 	{
 		if self.context.channel_state.is_local_shutdown_sent() {
-			return Err(("Shutdown was already sent", 0x4000|8))
+			return Err((HTLCDestination::ShutdownSent, 0x4000|8))
 		}
 
 		let dust_exposure_limiting_feerate = self.context.get_dust_exposure_limiting_feerate(&fee_estimator);
@@ -7636,7 +7631,7 @@ impl<SP: Deref> FundedChannel<SP> where
 			// Note that the total dust exposure includes both the dust HTLCs and the excess mining fees of the counterparty commitment transaction
 			log_info!(logger, "Cannot accept value that would put our total dust exposure at {} over the limit {} on counterparty commitment tx",
 				on_counterparty_tx_dust_htlc_exposure_msat, max_dust_htlc_exposure_msat);
-			return Err(("Exceeded our total dust exposure limit on counterparty commitment tx", 0x1000|7))
+			return Err((HTLCDestination::DustLimitReached { holder_commitment: false }, 0x1000|7))
 		}
 		let htlc_success_dust_limit = if self.context.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
 			0
@@ -7650,7 +7645,7 @@ impl<SP: Deref> FundedChannel<SP> where
 			if on_holder_tx_dust_htlc_exposure_msat > max_dust_htlc_exposure_msat {
 				log_info!(logger, "Cannot accept value that would put our exposure to dust HTLCs at {} over the limit {} on holder commitment tx",
 					on_holder_tx_dust_htlc_exposure_msat, max_dust_htlc_exposure_msat);
-				return Err(("Exceeded our dust exposure limit on holder commitment tx", 0x1000|7))
+				return Err((HTLCDestination::DustLimitReached { holder_commitment: true }, 0x1000|7))
 			}
 		}
 
@@ -7688,7 +7683,7 @@ impl<SP: Deref> FundedChannel<SP> where
 			}
 			if pending_remote_value_msat.saturating_sub(self.funding.holder_selected_channel_reserve_satoshis * 1000).saturating_sub(anchor_outputs_value_msat) < remote_fee_cost_incl_stuck_buffer_msat {
 				log_info!(logger, "Attempting to fail HTLC due to fee spike buffer violation in channel {}. Rebalancing is required.", &self.context.channel_id());
-				return Err(("Fee spike buffer violation", 0x1000|7));
+				return Err((HTLCDestination::FeeSpikeBuffer, 0x1000|7));
 			}
 		}
 
@@ -8564,18 +8559,13 @@ impl<SP: Deref> FundedChannel<SP> where
 		&mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32, source: HTLCSource,
 		onion_routing_packet: msgs::OnionPacket, skimmed_fee_msat: Option<u64>,
 		blinding_point: Option<PublicKey>, fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L
-	) -> Result<(), ChannelError>
+	) -> Result<(), HTLCDestination>
 	where F::Target: FeeEstimator, L::Target: Logger
 	{
 		self
 			.send_htlc(amount_msat, payment_hash, cltv_expiry, source, onion_routing_packet, true,
 				skimmed_fee_msat, blinding_point, fee_estimator, logger)
 			.map(|msg_opt| assert!(msg_opt.is_none(), "We forced holding cell?"))
-			.map_err(|err| {
-				if let ChannelError::Ignore(_) = err { /* fine */ }
-				else { debug_assert!(false, "Queueing cannot trigger channel failure"); }
-				err
-			})
 	}
 
 	/// Adds a pending outbound HTLC to this channel, note that you probably want
@@ -8599,33 +8589,42 @@ impl<SP: Deref> FundedChannel<SP> where
 		onion_routing_packet: msgs::OnionPacket, mut force_holding_cell: bool,
 		skimmed_fee_msat: Option<u64>, blinding_point: Option<PublicKey>,
 		fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L
-	) -> Result<Option<msgs::UpdateAddHTLC>, ChannelError>
+	) -> Result<Option<msgs::UpdateAddHTLC>, HTLCDestination>
 	where F::Target: FeeEstimator, L::Target: Logger
 	{
 		if !matches!(self.context.channel_state, ChannelState::ChannelReady(_)) ||
 			self.context.channel_state.is_local_shutdown_sent() ||
 			self.context.channel_state.is_remote_shutdown_sent()
 		{
-			return Err(ChannelError::Ignore("Cannot send HTLC until channel is fully established and we haven't started shutting down".to_owned()));
+			return Err(HTLCDestination::ChannelNotReady);
 		}
 		let channel_total_msat = self.funding.get_value_satoshis() * 1000;
 		if amount_msat > channel_total_msat {
-			return Err(ChannelError::Ignore(format!("Cannot send amount {}, because it is more than the total value of the channel {}", amount_msat, channel_total_msat)));
+			return Err(HTLCDestination::AmountExceedsChannelValue {
+				amount_msat,
+				channel_value_msat: channel_total_msat,
+			})
 		}
 
 		if amount_msat == 0 {
-			return Err(ChannelError::Ignore("Cannot send 0-msat HTLC".to_owned()));
+			return Err(HTLCDestination::HTLCBelowMinimum)
 		}
 
 		let available_balances = self.context.get_available_balances(&self.funding, fee_estimator);
 		if amount_msat < available_balances.next_outbound_htlc_minimum_msat {
-			return Err(ChannelError::Ignore(format!("Cannot send less than our next-HTLC minimum - {} msat",
-				available_balances.next_outbound_htlc_minimum_msat)));
+			return Err(HTLCDestination::InvalidPaymentAmount {
+				amount_msat,
+				next_outbound_min_msat: available_balances.next_outbound_htlc_minimum_msat,
+				next_outbound_max_msat: available_balances.next_outbound_htlc_limit_msat,
+			});
 		}
 
 		if amount_msat > available_balances.next_outbound_htlc_limit_msat {
-			return Err(ChannelError::Ignore(format!("Cannot send more than our next-HTLC maximum - {} msat",
-				available_balances.next_outbound_htlc_limit_msat)));
+			return Err(HTLCDestination::InvalidPaymentAmount {
+				amount_msat,
+				next_outbound_min_msat: available_balances.next_outbound_htlc_minimum_msat,
+				next_outbound_max_msat: available_balances.next_outbound_htlc_limit_msat,
+			});
 		}
 
 		if self.context.channel_state.is_peer_disconnected() {
@@ -8635,7 +8634,7 @@ impl<SP: Deref> FundedChannel<SP> where
 			// disconnected during the time the previous hop was doing the commitment dance we may
 			// end up getting here after the forwarding delay. In any case, returning an
 			// IgnoreError will get ChannelManager to do the right thing and fail backwards now.
-			return Err(ChannelError::Ignore("Cannot send an HTLC while disconnected from channel counterparty".to_owned()));
+			return Err(HTLCDestination::ChannelDisabled)
 		}
 
 		let need_holding_cell = !self.context.channel_state.can_generate_new_commitment();
@@ -8850,12 +8849,11 @@ impl<SP: Deref> FundedChannel<SP> where
 		&mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32,
 		source: HTLCSource, onion_routing_packet: msgs::OnionPacket, skimmed_fee_msat: Option<u64>,
 		fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L
-	) -> Result<Option<ChannelMonitorUpdate>, ChannelError>
+	) -> Result<Option<ChannelMonitorUpdate>, HTLCDestination>
 	where F::Target: FeeEstimator, L::Target: Logger
 	{
 		let send_res = self.send_htlc(amount_msat, payment_hash, cltv_expiry, source,
 			onion_routing_packet, false, skimmed_fee_msat, None, fee_estimator, logger);
-		if let Err(e) = &send_res { if let ChannelError::Ignore(_) = e {} else { debug_assert!(false, "Sending cannot trigger channel failure"); } }
 		match send_res? {
 			Some(_) => {
 				let monitor_update = self.build_commitment_no_status_check(logger);