Remove payment_release_secret from async payments messages.

This field isn't necessary because we already authenticate the messages via the
blinded reply paths payment_id, nonce and HMAC.

Author: valentinewallace

fuzz/src/onion_message.rs

@@ -126,10 +126,7 @@ impl AsyncPaymentsMessageHandler for TestAsyncPaymentsMessageHandler {
 			Some(resp) => resp,
 			None => return None,
 		};
-		Some((
-			ReleaseHeldHtlc { payment_release_secret: message.payment_release_secret },
-			responder.respond(),
-		))
+		Some((ReleaseHeldHtlc {}, responder.respond()))
 	}
 	fn release_held_htlc(&self, _message: ReleaseHeldHtlc, _context: AsyncPaymentsContext) {}
 }

lightning/src/ln/channelmanager.rs

@@ -4359,8 +4359,8 @@ where
 				invoice, payment_id, features, best_block_height, &*self.entropy_source,
 				&self.pending_events
 			);
-			let payment_release_secret = match outbound_pmts_res {
-				Ok(secret) => secret,
+			match outbound_pmts_res {
+				Ok(()) => {},
 				Err(Bolt12PaymentError::UnexpectedInvoice) | Err(Bolt12PaymentError::DuplicateInvoice) => {
 					res = outbound_pmts_res.map(|_| ());
 					return NotifyOption::SkipPersistNoEvents
@@ -4397,9 +4397,7 @@ where
 						destination: Destination::BlindedPath(invoice_path.clone()),
 						reply_path: reply_path.clone(),
 					};
-					let message = AsyncPaymentsMessage::HeldHtlcAvailable(
-						HeldHtlcAvailable { payment_release_secret }
-					);
+					let message = AsyncPaymentsMessage::HeldHtlcAvailable(HeldHtlcAvailable {});
 					pending_async_payments_messages.push((message, instructions));
 				});
 
@@ -4411,16 +4409,15 @@ where
 
 	#[cfg(async_payments)]
 	fn send_payment_for_static_invoice(
-		&self, payment_id: PaymentId, payment_release_secret: [u8; 32]
+		&self, payment_id: PaymentId
 	) -> Result<(), Bolt12PaymentError> {
 		let best_block_height = self.best_block.read().unwrap().height;
 		let mut res = Ok(());
 		PersistenceNotifierGuard::optionally_notify(self, || {
 			let outbound_pmts_res = self.pending_outbound_payments.send_payment_for_static_invoice(
-				payment_id, payment_release_secret, &self.router, self.list_usable_channels(),
-				|| self.compute_inflight_htlcs(), &self.entropy_source, &self.node_signer, &self,
-				&self.secp_ctx, best_block_height, &self.logger, &self.pending_events,
-				|args| self.send_payment_along_path(args)
+				payment_id, &self.router, self.list_usable_channels(), || self.compute_inflight_htlcs(),
+				&self.entropy_source, &self.node_signer, &self, &self.secp_ctx, best_block_height,
+				&self.logger, &self.pending_events, |args| self.send_payment_along_path(args)
 			);
 			match outbound_pmts_res {
 				Err(Bolt12PaymentError::UnexpectedInvoice) | Err(Bolt12PaymentError::DuplicateInvoice) => {
@@ -11239,10 +11236,9 @@ where
 		#[cfg(async_payments)] {
 			let AsyncPaymentsContext::OutboundPayment { payment_id, hmac, nonce } = _context;
 			if payment_id.verify_for_async_payment(hmac, nonce, &self.inbound_payment_key).is_err() { return }
-			if let Err(e) = self.send_payment_for_static_invoice(payment_id, _message.payment_release_secret) {
+			if let Err(e) = self.send_payment_for_static_invoice(payment_id) {
 				log_trace!(
-					self.logger, "Failed to release held HTLC with payment id {} and release secret {:02x?}: {:?}",
-					payment_id, _message.payment_release_secret, e
+					self.logger, "Failed to release held HTLC with payment id {}: {:?}", payment_id, e
 				);
 			}
 		}

lightning/src/ln/outbound_payment.rs

@@ -82,7 +82,6 @@ pub(crate) enum PendingOutboundPayment {
 		payment_hash: PaymentHash,
 		keysend_preimage: PaymentPreimage,
 		retry_strategy: Retry,
-		payment_release_secret: [u8; 32],
 		route_params: RouteParameters,
 	},
 	Retryable {
@@ -984,7 +983,7 @@ impl OutboundPayments {
 		&self, invoice: &StaticInvoice, payment_id: PaymentId, features: Bolt12InvoiceFeatures,
 		best_block_height: u32, entropy_source: ES,
 		pending_events: &Mutex<VecDeque<(events::Event, Option<EventCompletionAction>)>>
-	) -> Result<[u8; 32], Bolt12PaymentError> where ES::Target: EntropySource {
+	) -> Result<(), Bolt12PaymentError> where ES::Target: EntropySource {
 		macro_rules! abandon_with_entry {
 			($payment: expr, $reason: expr) => {
 				$payment.get_mut().mark_abandoned($reason);
@@ -1029,7 +1028,6 @@ impl OutboundPayments {
 					};
 					let keysend_preimage = PaymentPreimage(entropy_source.get_secure_random_bytes());
 					let payment_hash = PaymentHash(Sha256::hash(&keysend_preimage.0).to_byte_array());
-					let payment_release_secret = entropy_source.get_secure_random_bytes();
 					let pay_params = PaymentParameters::from_static_invoice(invoice);
 					let mut route_params = RouteParameters::from_payment_params_and_value(pay_params, amount_msat);
 					route_params.max_total_routing_fee_msat = *max_total_routing_fee_msat;
@@ -1046,10 +1044,9 @@ impl OutboundPayments {
 						payment_hash,
 						keysend_preimage,
 						retry_strategy: *retry_strategy,
-						payment_release_secret,
 						route_params,
 					};
-					return Ok(payment_release_secret)
+					return Ok(())
 				},
 				_ => return Err(Bolt12PaymentError::DuplicateInvoice),
 			},
@@ -1061,9 +1058,9 @@ impl OutboundPayments {
 	pub(super) fn send_payment_for_static_invoice<
 		R: Deref, ES: Deref, NS: Deref, NL: Deref, IH, SP, L: Deref
 	>(
-		&self, payment_id: PaymentId, payment_release_secret: [u8; 32], router: &R,
-		first_hops: Vec<ChannelDetails>, inflight_htlcs: IH, entropy_source: &ES, node_signer: &NS,
-		node_id_lookup: &NL, secp_ctx: &Secp256k1<secp256k1::All>, best_block_height: u32, logger: &L,
+		&self, payment_id: PaymentId, router: &R, first_hops: Vec<ChannelDetails>, inflight_htlcs: IH,
+		entropy_source: &ES, node_signer: &NS, node_id_lookup: &NL,
+		secp_ctx: &Secp256k1<secp256k1::All>, best_block_height: u32, logger: &L,
 		pending_events: &Mutex<VecDeque<(events::Event, Option<EventCompletionAction>)>>,
 		send_payment_along_path: SP,
 	) -> Result<(), Bolt12PaymentError>
@@ -1080,12 +1077,8 @@ impl OutboundPayments {
 			match self.pending_outbound_payments.lock().unwrap().entry(payment_id) {
 				hash_map::Entry::Occupied(entry) => match entry.get() {
 					PendingOutboundPayment::StaticInvoiceReceived {
-						payment_hash, payment_release_secret: release_secret, route_params, retry_strategy,
-						keysend_preimage, ..
+						payment_hash, route_params, retry_strategy, keysend_preimage, ..
 					} => {
-						if payment_release_secret != *release_secret {
-							return Err(Bolt12PaymentError::UnexpectedInvoice)
-						}
 						(*payment_hash, *keysend_preimage, route_params.clone(), *retry_strategy)
 					},
 					_ => return Err(Bolt12PaymentError::DuplicateInvoice),
@@ -2196,8 +2189,7 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
 		(0, payment_hash, required),
 		(2, keysend_preimage, required),
 		(4, retry_strategy, required),
-		(6, payment_release_secret, required),
-		(8, route_params, required),
+		(6, route_params, required),
 	},
 );
 
@@ -2785,7 +2777,6 @@ mod tests {
 			payment_hash,
 			keysend_preimage: PaymentPreimage([0; 32]),
 			retry_strategy: Retry::Attempts(0),
-			payment_release_secret: [0; 32],
 			route_params,
 		};
 		outbounds.insert(payment_id, outbound);
@@ -2832,7 +2823,6 @@ mod tests {
 			payment_hash,
 			keysend_preimage: PaymentPreimage([0; 32]),
 			retry_strategy: Retry::Attempts(0),
-			payment_release_secret: [0; 32],
 			route_params,
 		};
 		outbounds.insert(payment_id, outbound);

lightning/src/onion_message/async_payments.rs

@@ -61,18 +61,11 @@ pub enum AsyncPaymentsMessage {
 /// accompanying this onion message should be used to send a [`ReleaseHeldHtlc`] response, which
 /// will cause the upstream HTLC to be released.
 #[derive(Clone, Debug)]
-pub struct HeldHtlcAvailable {
-	/// The secret that will be used by the recipient of this message to release the held HTLC.
-	pub payment_release_secret: [u8; 32],
-}
+pub struct HeldHtlcAvailable {}
 
 /// Releases the HTLC corresponding to an inbound [`HeldHtlcAvailable`] message.
 #[derive(Clone, Debug)]
-pub struct ReleaseHeldHtlc {
-	/// Used to release the HTLC held upstream if it matches the corresponding
-	/// [`HeldHtlcAvailable::payment_release_secret`].
-	pub payment_release_secret: [u8; 32],
-}
+pub struct ReleaseHeldHtlc {}
 
 impl OnionMessageContents for ReleaseHeldHtlc {
 	fn tlv_type(&self) -> u64 {
@@ -88,13 +81,9 @@ impl OnionMessageContents for ReleaseHeldHtlc {
 	}
 }
 
-impl_writeable_tlv_based!(HeldHtlcAvailable, {
-	(0, payment_release_secret, required),
-});
+impl_writeable_tlv_based!(HeldHtlcAvailable, {});
 
-impl_writeable_tlv_based!(ReleaseHeldHtlc, {
-	(0, payment_release_secret, required),
-});
+impl_writeable_tlv_based!(ReleaseHeldHtlc, {});
 
 impl AsyncPaymentsMessage {
 	/// Returns whether `tlv_type` corresponds to a TLV record for async payment messages.