From 2f798f6cc0099a10f2f8e4a3ea0f3fab6d51a612 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 22 Mar 2023 19:15:52 +0000 Subject: [PATCH] Remove the `PaymentSecret` field from `HTLCSource::OutboundRoute` Many of the fields in `HTLCSource::OutboundRoute` are used to rebuild the pending-outbound-payment map on reload if the `ChannelManager` was not serialized though `ChannelMonitor`(s) were after an HTLC was sent. As of 0.0.114, however, such payments are not retryable without allowing them to fail and doing a full, fresh, send. Thus, some of the fields can be safely removed - we only really care about having enough information to provide the user a failure event, not being able to retry. Here we drop one such field - the `payment_secret`, making our `ChannelMonitorUpdate`s another handful of bytes smaller. --- lightning/src/ln/channel.rs | 1 - lightning/src/ln/channelmanager.rs | 17 +++++------------ pending_changelog/matt-rm-retryable-secret.txt | 3 +++ 3 files changed, 8 insertions(+), 13 deletions(-) create mode 100644 pending_changelog/matt-rm-retryable-secret.txt diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 666c48442..5c0c57078 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -7158,7 +7158,6 @@ mod tests { session_priv: SecretKey::from_slice(&hex::decode("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap()[..]).unwrap(), first_hop_htlc_msat: 548, payment_id: PaymentId([42; 32]), - payment_secret: None, } }); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index c2ff9da67..5035384c1 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -286,7 +286,6 @@ pub(crate) enum HTLCSource { /// doing a double-pass on route when we get a failure back first_hop_htlc_msat: u64, payment_id: PaymentId, - payment_secret: Option, }, } #[allow(clippy::derive_hash_xor_eq)] // Our Hash is faithful to the data, we just don't have SecretKey::hash @@ -297,12 +296,11 @@ impl core::hash::Hash for HTLCSource { 0u8.hash(hasher); prev_hop_data.hash(hasher); }, - HTLCSource::OutboundRoute { path, session_priv, payment_id, payment_secret, first_hop_htlc_msat } => { + HTLCSource::OutboundRoute { path, session_priv, payment_id, first_hop_htlc_msat } => { 1u8.hash(hasher); path.hash(hasher); session_priv[..].hash(hasher); payment_id.hash(hasher); - payment_secret.hash(hasher); first_hop_htlc_msat.hash(hasher); }, } @@ -317,7 +315,6 @@ impl HTLCSource { session_priv: SecretKey::from_slice(&[1; 32]).unwrap(), first_hop_htlc_msat: 0, payment_id: PaymentId([2; 32]), - payment_secret: None, } } } @@ -2556,7 +2553,6 @@ where session_priv: session_priv.clone(), first_hop_htlc_msat: htlc_msat, payment_id, - payment_secret: payment_secret.clone(), }, onion_packet, &self.logger); match break_chan_entry!(self, send_res, chan) { Some(monitor_update) => { @@ -6900,13 +6896,11 @@ impl Readable for HTLCSource { let mut first_hop_htlc_msat: u64 = 0; let mut path: Option> = Some(Vec::new()); let mut payment_id = None; - let mut payment_secret = None; let mut payment_params: Option = None; read_tlv_fields!(reader, { (0, session_priv, required), (1, payment_id, option), (2, first_hop_htlc_msat, required), - (3, payment_secret, option), (4, path, vec_type), (5, payment_params, (option: ReadableArgs, 0)), }); @@ -6929,7 +6923,6 @@ impl Readable for HTLCSource { first_hop_htlc_msat, path, payment_id: payment_id.unwrap(), - payment_secret, }) } 1 => Ok(HTLCSource::PreviousHopData(Readable::read(reader)?)), @@ -6941,14 +6934,14 @@ impl Readable for HTLCSource { impl Writeable for HTLCSource { fn write(&self, writer: &mut W) -> Result<(), crate::io::Error> { match self { - HTLCSource::OutboundRoute { ref session_priv, ref first_hop_htlc_msat, ref path, payment_id, payment_secret } => { + HTLCSource::OutboundRoute { ref session_priv, ref first_hop_htlc_msat, ref path, payment_id } => { 0u8.write(writer)?; let payment_id_opt = Some(payment_id); write_tlv_fields!(writer, { (0, session_priv, required), (1, payment_id_opt, option), (2, first_hop_htlc_msat, required), - (3, payment_secret, option), + // 3 was previously used to write a PaymentSecret for the payment. (4, *path, vec_type), (5, None::, option), // payment_params in LDK versions prior to 0.0.115 }); @@ -7611,7 +7604,7 @@ where for (_, monitor) in args.channel_monitors.iter() { if id_to_peer.get(&monitor.get_funding_txo().0.to_channel_id()).is_none() { for (htlc_source, (htlc, _)) in monitor.get_pending_or_resolved_outbound_htlcs() { - if let HTLCSource::OutboundRoute { payment_id, session_priv, path, payment_secret, .. } = htlc_source { + if let HTLCSource::OutboundRoute { payment_id, session_priv, path, .. } = htlc_source { if path.is_empty() { log_error!(args.logger, "Got an empty path for a pending payment"); return Err(DecodeError::InvalidValue); @@ -7634,7 +7627,7 @@ where payment_params: None, session_privs: [session_priv_bytes].iter().map(|a| *a).collect(), payment_hash: htlc.payment_hash, - payment_secret, + payment_secret: None, // only used for retries, and we'll never retry on startup keysend_preimage: None, // only used for retries, and we'll never retry on startup pending_amt_msat: path_amt, pending_fee_msat: Some(path_fee), diff --git a/pending_changelog/matt-rm-retryable-secret.txt b/pending_changelog/matt-rm-retryable-secret.txt new file mode 100644 index 000000000..694e3686a --- /dev/null +++ b/pending_changelog/matt-rm-retryable-secret.txt @@ -0,0 +1,3 @@ +## Backwards Compatibility + * Payments sent with the legacy `*_with_route` methods on LDK 0.0.115+ will no + longer be retryable via the LDK 0.0.114- `retry_payment` method (#XXXX). -- 2.39.5