From 48c3e53806eba361337fe2b6ce8c6bf57526e33e Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Wed, 11 Sep 2024 10:20:58 -0400 Subject: [PATCH] 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. --- fuzz/src/onion_message.rs | 5 +--- lightning/src/ln/channelmanager.rs | 22 +++++++---------- lightning/src/ln/outbound_payment.rs | 24 ++++++------------- lightning/src/onion_message/async_payments.rs | 19 ++++----------- 4 files changed, 21 insertions(+), 49 deletions(-) diff --git a/fuzz/src/onion_message.rs b/fuzz/src/onion_message.rs index d1ceb04f5b..113d5668dc 100644 --- a/fuzz/src/onion_message.rs +++ b/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) {} } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 8bd1fa6d0a..c36dee79a7 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4325,8 +4325,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 @@ -4363,9 +4363,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)); }); @@ -4377,16 +4375,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) => { @@ -11142,10 +11139,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 ); } } diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 52aad71958..6d88ccb3e1 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -80,7 +80,6 @@ pub(crate) enum PendingOutboundPayment { payment_hash: PaymentHash, keysend_preimage: PaymentPreimage, retry_strategy: Retry, - payment_release_secret: [u8; 32], route_params: RouteParameters, }, Retryable { @@ -966,7 +965,7 @@ impl OutboundPayments { &self, invoice: &StaticInvoice, payment_id: PaymentId, features: Bolt12InvoiceFeatures, best_block_height: u32, entropy_source: ES, pending_events: &Mutex)>> - ) -> 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); @@ -1008,7 +1007,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; @@ -1025,10 +1023,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), }, @@ -1040,9 +1037,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, inflight_htlcs: IH, entropy_source: &ES, node_signer: &NS, - node_id_lookup: &NL, secp_ctx: &Secp256k1, best_block_height: u32, logger: &L, + &self, payment_id: PaymentId, router: &R, first_hops: Vec, inflight_htlcs: IH, + entropy_source: &ES, node_signer: &NS, node_id_lookup: &NL, + secp_ctx: &Secp256k1, best_block_height: u32, logger: &L, pending_events: &Mutex)>>, send_payment_along_path: SP, ) -> Result<(), Bolt12PaymentError> @@ -1059,12 +1056,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), @@ -2147,8 +2140,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), }, ); @@ -2735,7 +2727,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); @@ -2782,7 +2773,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); diff --git a/lightning/src/onion_message/async_payments.rs b/lightning/src/onion_message/async_payments.rs index e2a7f7bf74..cc4ca5edfb 100644 --- a/lightning/src/onion_message/async_payments.rs +++ b/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.