Merge pull request #2319 from valentinewallace/2023-05-forward-less-than-onion

Allow forwarding less than the amount in the onion

Author: tnull

lightning/src/events/mod.rs

@@ -385,8 +385,25 @@ pub enum Event {
 		///
 		/// Payments received on LDK versions prior to 0.0.115 will have this field unset.
 		onion_fields: Option<RecipientOnionFields>,
-		/// The value, in thousandths of a satoshi, that this payment is for.
+		/// The value, in thousandths of a satoshi, that this payment is claimable for. May be greater
+		/// than the invoice amount.
+		///
+		/// May be less than the invoice amount if [`ChannelConfig::accept_underpaying_htlcs`] is set
+		/// and the previous hop took an extra fee.
+		///
+		/// # Note
+		/// If [`ChannelConfig::accept_underpaying_htlcs`] is set and you claim without verifying this
+		/// field, you may lose money!
+		///
+		/// [`ChannelConfig::accept_underpaying_htlcs`]: crate::util::config::ChannelConfig::accept_underpaying_htlcs
 		amount_msat: u64,
+		/// The value, in thousands of a satoshi, that was skimmed off of this payment as an extra fee
+		/// taken by our channel counterparty.
+		///
+		/// Will always be 0 unless [`ChannelConfig::accept_underpaying_htlcs`] is set.
+		///
+		/// [`ChannelConfig::accept_underpaying_htlcs`]: crate::util::config::ChannelConfig::accept_underpaying_htlcs
+		counterparty_skimmed_fee_msat: u64,
 		/// Information for claiming this received payment, based on whether the purpose of the
 		/// payment is to pay an invoice or to send a spontaneous payment.
 		purpose: PaymentPurpose,
@@ -428,7 +445,8 @@ pub enum Event {
 		/// The payment hash of the claimed payment. Note that LDK will not stop you from
 		/// registering duplicate payment hashes for inbound payments.
 		payment_hash: PaymentHash,
-		/// The value, in thousandths of a satoshi, that this payment is for.
+		/// The value, in thousandths of a satoshi, that this payment is for. May be greater than the
+		/// invoice amount.
 		amount_msat: u64,
 		/// The purpose of the claimed payment, i.e. whether the payment was for an invoice or a
 		/// spontaneous payment.
@@ -621,6 +639,7 @@ pub enum Event {
 		inbound_amount_msat: u64,
 		/// How many msats the payer intended to route to the next node. Depending on the reason you are
 		/// intercepting this payment, you might take a fee by forwarding less than this amount.
+		/// Forwarding less than this amount may break compatibility with LDK versions prior to 0.0.116.
 		///
 		/// Note that LDK will NOT check that expected fees were factored into this value. You MUST
 		/// check that whatever fee you want has been included here or subtract it as required. Further,
@@ -830,8 +849,8 @@ impl Writeable for Event {
 				// We never write out FundingGenerationReady events as, upon disconnection, peers
 				// drop any channels which have not yet exchanged funding_signed.
 			},
-			&Event::PaymentClaimable { ref payment_hash, ref amount_msat, ref purpose,
-				ref receiver_node_id, ref via_channel_id, ref via_user_channel_id,
+			&Event::PaymentClaimable { ref payment_hash, ref amount_msat, counterparty_skimmed_fee_msat,
+				ref purpose, ref receiver_node_id, ref via_channel_id, ref via_user_channel_id,
 				ref claim_deadline, ref onion_fields
 			} => {
 				1u8.write(writer)?;
@@ -846,6 +865,8 @@ impl Writeable for Event {
 						payment_preimage = Some(*preimage);
 					}
 				}
+				let skimmed_fee_opt = if counterparty_skimmed_fee_msat == 0 { None }
+					else { Some(counterparty_skimmed_fee_msat) };
 				write_tlv_fields!(writer, {
 					(0, payment_hash, required),
 					(1, receiver_node_id, option),
@@ -857,6 +878,7 @@ impl Writeable for Event {
 					(7, claim_deadline, option),
 					(8, payment_preimage, option),
 					(9, onion_fields, option),
+					(10, skimmed_fee_opt, option),
 				});
 			},
 			&Event::PaymentSent { ref payment_id, ref payment_preimage, ref payment_hash, ref fee_paid_msat } => {
@@ -1056,6 +1078,7 @@ impl MaybeReadable for Event {
 					let mut payment_preimage = None;
 					let mut payment_secret = None;
 					let mut amount_msat = 0;
+					let mut counterparty_skimmed_fee_msat_opt = None;
 					let mut receiver_node_id = None;
 					let mut _user_payment_id = None::<u64>; // Used in 0.0.103 and earlier, no longer written in 0.0.116+.
 					let mut via_channel_id = None;
@@ -1073,6 +1096,7 @@ impl MaybeReadable for Event {
 						(7, claim_deadline, option),
 						(8, payment_preimage, option),
 						(9, onion_fields, option),
+						(10, counterparty_skimmed_fee_msat_opt, option),
 					});
 					let purpose = match payment_secret {
 						Some(secret) => PaymentPurpose::InvoicePayment {
@@ -1086,6 +1110,7 @@ impl MaybeReadable for Event {
 						receiver_node_id,
 						payment_hash,
 						amount_msat,
+						counterparty_skimmed_fee_msat: counterparty_skimmed_fee_msat_opt.unwrap_or(0),
 						purpose,
 						via_channel_id,
 						via_user_channel_id,

lightning/src/ln/channel.rs

@@ -224,6 +224,7 @@ struct OutboundHTLCOutput {
 	payment_hash: PaymentHash,
 	state: OutboundHTLCState,
 	source: HTLCSource,
+	skimmed_fee_msat: Option<u64>,
 }
 
 /// See AwaitingRemoteRevoke ChannelState for more info
@@ -235,6 +236,8 @@ enum HTLCUpdateAwaitingACK {
 		payment_hash: PaymentHash,
 		source: HTLCSource,
 		onion_routing_packet: msgs::OnionPacket,
+		// The extra fee we're skimming off the top of this HTLC.
+		skimmed_fee_msat: Option<u64>,
 	},
 	ClaimHTLC {
 		payment_preimage: PaymentPreimage,
@@ -3052,8 +3055,13 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
 				// handling this case better and maybe fulfilling some of the HTLCs while attempting
 				// to rebalance channels.
 				match &htlc_update {
-					&HTLCUpdateAwaitingACK::AddHTLC {amount_msat, cltv_expiry, ref payment_hash, ref source, ref onion_routing_packet, ..} => {
-						match self.send_htlc(amount_msat, *payment_hash, cltv_expiry, source.clone(), onion_routing_packet.clone(), false, logger) {
+					&HTLCUpdateAwaitingACK::AddHTLC {
+						amount_msat, cltv_expiry, ref payment_hash, ref source, ref onion_routing_packet,
+						skimmed_fee_msat, ..
+					} => {
+						match self.send_htlc(amount_msat, *payment_hash, cltv_expiry, source.clone(),
+							onion_routing_packet.clone(), false, skimmed_fee_msat, logger)
+						{
 							Ok(update_add_msg_option) => update_add_htlcs.push(update_add_msg_option.unwrap()),
 							Err(e) => {
 								match e {
@@ -3695,6 +3703,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
 					payment_hash: htlc.payment_hash,
 					cltv_expiry: htlc.cltv_expiry,
 					onion_routing_packet: (**onion_packet).clone(),
+					skimmed_fee_msat: htlc.skimmed_fee_msat,
 				});
 			}
 		}
@@ -5042,11 +5051,13 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
 	/// commitment update.
 	///
 	/// `Err`s will only be [`ChannelError::Ignore`].
-	pub fn queue_add_htlc<L: Deref>(&mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32, source: HTLCSource,
-		onion_routing_packet: msgs::OnionPacket, logger: &L)
-	-> Result<(), ChannelError> where L::Target: Logger {
+	pub fn queue_add_htlc<L: Deref>(
+		&mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32, source: HTLCSource,
+		onion_routing_packet: msgs::OnionPacket, skimmed_fee_msat: Option<u64>, logger: &L
+	) -> Result<(), ChannelError> where L::Target: Logger {
 		self
-			.send_htlc(amount_msat, payment_hash, cltv_expiry, source, onion_routing_packet, true, logger)
+			.send_htlc(amount_msat, payment_hash, cltv_expiry, source, onion_routing_packet, true,
+				skimmed_fee_msat, logger)
 			.map(|msg_opt| assert!(msg_opt.is_none(), "We forced holding cell?"))
 			.map_err(|err| {
 				if let ChannelError::Ignore(_) = err { /* fine */ }
@@ -5071,9 +5082,11 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
 	/// on this [`Channel`] if `force_holding_cell` is false.
 	///
 	/// `Err`s will only be [`ChannelError::Ignore`].
-	fn send_htlc<L: Deref>(&mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32, source: HTLCSource,
-		onion_routing_packet: msgs::OnionPacket, mut force_holding_cell: bool, logger: &L)
-	-> Result<Option<msgs::UpdateAddHTLC>, ChannelError> where L::Target: Logger {
+	fn send_htlc<L: Deref>(
+		&mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32, source: HTLCSource,
+		onion_routing_packet: msgs::OnionPacket, mut force_holding_cell: bool,
+		skimmed_fee_msat: Option<u64>, logger: &L
+	) -> Result<Option<msgs::UpdateAddHTLC>, ChannelError> where L::Target: Logger {
 		if (self.context.channel_state & (ChannelState::ChannelReady as u32 | BOTH_SIDES_SHUTDOWN_MASK)) != (ChannelState::ChannelReady as u32) {
 			return Err(ChannelError::Ignore("Cannot send HTLC until channel is fully established and we haven't started shutting down".to_owned()));
 		}
@@ -5125,6 +5138,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
 				cltv_expiry,
 				source,
 				onion_routing_packet,
+				skimmed_fee_msat,
 			});
 			return Ok(None);
 		}
@@ -5136,6 +5150,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
 			cltv_expiry,
 			state: OutboundHTLCState::LocalAnnounced(Box::new(onion_routing_packet.clone())),
 			source,
+			skimmed_fee_msat,
 		});
 
 		let res = msgs::UpdateAddHTLC {
@@ -5145,6 +5160,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
 			payment_hash,
 			cltv_expiry,
 			onion_routing_packet,
+			skimmed_fee_msat,
 		};
 		self.context.next_holder_htlc_id += 1;
 
@@ -5283,8 +5299,12 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
 	///
 	/// Shorthand for calling [`Self::send_htlc`] followed by a commitment update, see docs on
 	/// [`Self::send_htlc`] and [`Self::build_commitment_no_state_update`] for more info.
-	pub fn send_htlc_and_commit<L: Deref>(&mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32, source: HTLCSource, onion_routing_packet: msgs::OnionPacket, logger: &L) -> Result<Option<&ChannelMonitorUpdate>, ChannelError> where L::Target: Logger {
-		let send_res = self.send_htlc(amount_msat, payment_hash, cltv_expiry, source, onion_routing_packet, false, logger);
+	pub fn send_htlc_and_commit<L: Deref>(
+		&mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32, source: HTLCSource,
+		onion_routing_packet: msgs::OnionPacket, skimmed_fee_msat: Option<u64>, logger: &L
+	) -> Result<Option<&ChannelMonitorUpdate>, ChannelError> where L::Target: Logger {
+		let send_res = self.send_htlc(amount_msat, payment_hash, cltv_expiry, source,
+			onion_routing_packet, false, skimmed_fee_msat, 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(_) => {
@@ -6609,9 +6629,10 @@ impl<Signer: WriteableEcdsaChannelSigner> Writeable for Channel<Signer> {
 		}
 
 		let mut preimages: Vec<&Option<PaymentPreimage>> = vec![];
+		let mut pending_outbound_skimmed_fees: Vec<Option<u64>> = Vec::new();
 
 		(self.context.pending_outbound_htlcs.len() as u64).write(writer)?;
-		for htlc in self.context.pending_outbound_htlcs.iter() {
+		for (idx, htlc) in self.context.pending_outbound_htlcs.iter().enumerate() {
 			htlc.htlc_id.write(writer)?;
 			htlc.amount_msat.write(writer)?;
 			htlc.cltv_expiry.write(writer)?;
@@ -6647,18 +6668,37 @@ impl<Signer: WriteableEcdsaChannelSigner> Writeable for Channel<Signer> {
 					reason.write(writer)?;
 				}
 			}
+			if let Some(skimmed_fee) = htlc.skimmed_fee_msat {
+				if pending_outbound_skimmed_fees.is_empty() {
+					for _ in 0..idx { pending_outbound_skimmed_fees.push(None); }
+				}
+				pending_outbound_skimmed_fees.push(Some(skimmed_fee));
+			} else if !pending_outbound_skimmed_fees.is_empty() {
+				pending_outbound_skimmed_fees.push(None);
+			}
 		}
 
+		let mut holding_cell_skimmed_fees: Vec<Option<u64>> = Vec::new();
 		(self.context.holding_cell_htlc_updates.len() as u64).write(writer)?;
-		for update in self.context.holding_cell_htlc_updates.iter() {
+		for (idx, update) in self.context.holding_cell_htlc_updates.iter().enumerate() {
 			match update {
-				&HTLCUpdateAwaitingACK::AddHTLC { ref amount_msat, ref cltv_expiry, ref payment_hash, ref source, ref onion_routing_packet } => {
+				&HTLCUpdateAwaitingACK::AddHTLC {
+					ref amount_msat, ref cltv_expiry, ref payment_hash, ref source, ref onion_routing_packet,
+					skimmed_fee_msat,
+				} => {
 					0u8.write(writer)?;
 					amount_msat.write(writer)?;
 					cltv_expiry.write(writer)?;
 					payment_hash.write(writer)?;
 					source.write(writer)?;
 					onion_routing_packet.write(writer)?;
+
+					if let Some(skimmed_fee) = skimmed_fee_msat {
+						if holding_cell_skimmed_fees.is_empty() {
+							for _ in 0..idx { holding_cell_skimmed_fees.push(None); }
+						}
+						holding_cell_skimmed_fees.push(Some(skimmed_fee));
+					} else if !holding_cell_skimmed_fees.is_empty() { holding_cell_skimmed_fees.push(None); }
 				},
 				&HTLCUpdateAwaitingACK::ClaimHTLC { ref payment_preimage, ref htlc_id } => {
 					1u8.write(writer)?;
@@ -6825,6 +6865,8 @@ impl<Signer: WriteableEcdsaChannelSigner> Writeable for Channel<Signer> {
 			(29, self.context.temporary_channel_id, option),
 			(31, channel_pending_event_emitted, option),
 			(33, self.context.pending_monitor_updates, vec_type),
+			(35, pending_outbound_skimmed_fees, optional_vec),
+			(37, holding_cell_skimmed_fees, optional_vec),
 		});
 
 		Ok(())
@@ -6935,6 +6977,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
 					},
 					_ => return Err(DecodeError::InvalidValue),
 				},
+				skimmed_fee_msat: None,
 			});
 		}
 
@@ -6948,6 +6991,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
 					payment_hash: Readable::read(reader)?,
 					source: Readable::read(reader)?,
 					onion_routing_packet: Readable::read(reader)?,
+					skimmed_fee_msat: None,
 				},
 				1 => HTLCUpdateAwaitingACK::ClaimHTLC {
 					payment_preimage: Readable::read(reader)?,
@@ -7103,6 +7147,9 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
 
 		let mut pending_monitor_updates = Some(Vec::new());
 
+		let mut pending_outbound_skimmed_fees_opt: Option<Vec<Option<u64>>> = None;
+		let mut holding_cell_skimmed_fees_opt: Option<Vec<Option<u64>>> = None;
+
 		read_tlv_fields!(reader, {
 			(0, announcement_sigs, option),
 			(1, minimum_depth, option),
@@ -7126,6 +7173,8 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
 			(29, temporary_channel_id, option),
 			(31, channel_pending_event_emitted, option),
 			(33, pending_monitor_updates, vec_type),
+			(35, pending_outbound_skimmed_fees_opt, optional_vec),
+			(37, holding_cell_skimmed_fees_opt, optional_vec),
 		});
 
 		let (channel_keys_id, holder_signer) = if let Some(channel_keys_id) = channel_keys_id {
@@ -7180,6 +7229,25 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
 
 		let holder_max_accepted_htlcs = holder_max_accepted_htlcs.unwrap_or(DEFAULT_MAX_HTLCS);
 
+		if let Some(skimmed_fees) = pending_outbound_skimmed_fees_opt {
+			let mut iter = skimmed_fees.into_iter();
+			for htlc in pending_outbound_htlcs.iter_mut() {
+				htlc.skimmed_fee_msat = iter.next().ok_or(DecodeError::InvalidValue)?;
+			}
+			// We expect all skimmed fees to be consumed above
+			if iter.next().is_some() { return Err(DecodeError::InvalidValue) }
+		}
+		if let Some(skimmed_fees) = holding_cell_skimmed_fees_opt {
+			let mut iter = skimmed_fees.into_iter();
+			for htlc in holding_cell_htlc_updates.iter_mut() {
+				if let HTLCUpdateAwaitingACK::AddHTLC { ref mut skimmed_fee_msat, .. } = htlc {
+					*skimmed_fee_msat = iter.next().ok_or(DecodeError::InvalidValue)?;
+				}
+			}
+			// We expect all skimmed fees to be consumed above
+			if iter.next().is_some() { return Err(DecodeError::InvalidValue) }
+		}
+
 		Ok(Channel {
 			context: ChannelContext {
 				user_id,
@@ -7522,7 +7590,8 @@ mod tests {
 				session_priv: SecretKey::from_slice(&hex::decode("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap()[..]).unwrap(),
 				first_hop_htlc_msat: 548,
 				payment_id: PaymentId([42; 32]),
-			}
+			},
+			skimmed_fee_msat: None,
 		});
 
 		// Make sure when Node A calculates their local commitment transaction, none of the HTLCs pass
@@ -8079,6 +8148,7 @@ mod tests {
 				payment_hash: PaymentHash([0; 32]),
 				state: OutboundHTLCState::Committed,
 				source: HTLCSource::dummy(),
+				skimmed_fee_msat: None,
 			};
 			out.payment_hash.0 = Sha256::hash(&hex::decode("0202020202020202020202020202020202020202020202020202020202020202").unwrap()).into_inner();
 			out
@@ -8091,6 +8161,7 @@ mod tests {
 				payment_hash: PaymentHash([0; 32]),
 				state: OutboundHTLCState::Committed,
 				source: HTLCSource::dummy(),
+				skimmed_fee_msat: None,
 			};
 			out.payment_hash.0 = Sha256::hash(&hex::decode("0303030303030303030303030303030303030303030303030303030303030303").unwrap()).into_inner();
 			out
@@ -8492,6 +8563,7 @@ mod tests {
 				payment_hash: PaymentHash([0; 32]),
 				state: OutboundHTLCState::Committed,
 				source: HTLCSource::dummy(),
+				skimmed_fee_msat: None,
 			};
 			out.payment_hash.0 = Sha256::hash(&hex::decode("0505050505050505050505050505050505050505050505050505050505050505").unwrap()).into_inner();
 			out
@@ -8504,6 +8576,7 @@ mod tests {
 				payment_hash: PaymentHash([0; 32]),
 				state: OutboundHTLCState::Committed,
 				source: HTLCSource::dummy(),
+				skimmed_fee_msat: None,
 			};
 			out.payment_hash.0 = Sha256::hash(&hex::decode("0505050505050505050505050505050505050505050505050505050505050505").unwrap()).into_inner();
 			out