]> git.bitcoin.ninja Git - rust-lightning/commitdiff
Remove payment_release_secret from async payments messages.
authorValentine Wallace <vwallace@protonmail.com>
Wed, 11 Sep 2024 14:20:58 +0000 (10:20 -0400)
committerValentine Wallace <vwallace@protonmail.com>
Fri, 13 Sep 2024 14:40:06 +0000 (10:40 -0400)
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
lightning/src/ln/channelmanager.rs
lightning/src/ln/outbound_payment.rs
lightning/src/onion_message/async_payments.rs

index 26bb0bc0ee45e00ccd138ad4709e3c163dc80bf7..5cd45238df2e841fb2e0c299de8cdae81df4da37 100644 (file)
@@ -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) {}
 }
index 63faa91f583938bd1d4ebad19cd70fd6cc19a529..03164e04fb57cb7f5ec5328f99fbfbb182aa5abd 100644 (file)
@@ -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
                                );
                        }
                }
index ac2578a67214840217dc8dc3968ad98943e88456..7a850889d4c57946ccc37a42e6f765b3be657a3d 100644 (file)
@@ -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);
index e2a7f7bf74e2307b31c0f5b0c463f312897ed68d..cc4ca5edfb0cf433efe6f28f1fee62bc4638c338 100644 (file)
@@ -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.