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.

diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 666c48442668e2992e16585fb8d6d7f796fcd9e1..5c0c570782d2004750ec079e8c8e6e923edad906 100644 (file)
--- 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 c2ff9da67f38cc169c048ca7279952109f346d3d..5035384c18d7bc9b352c3da28ab66d88c9f5ad14 100644 (file)
--- 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<PaymentSecret>,
        },
 }
 #[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<Vec<RouteHop>> = Some(Vec::new());
                                let mut payment_id = None;
-                               let mut payment_secret = None;
                                let mut payment_params: Option<PaymentParameters> = 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<W: Writer>(&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::<PaymentParameters>, 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 (file)
index 0000000..694e368
--- /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).